Skip to content

SOLR-18085: Review use of "wt=standard" and see if we can remove it. #4080

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
epugh wants to merge 7 commits into
base: main
Choose a base branch
from
Open

SOLR-18085: Review use of "wt=standard" and see if we can remove it. #4080

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
99a5114
Commenting out code and adding a content type --> wt converter appear…
epugh Jan 27, 2026
d976a1a
don't overwrite the wt if it's been set.
epugh Jan 27, 2026
63d4284
Change the logic so that if you use a bad wt then instead of a 400 no…
epugh Jan 27, 2026
90dd987
json is the new standard
epugh Jan 27, 2026
947464b
No longer supporting a "default" since we specify what we want everyw…
epugh Jan 27, 2026
5753e33
Merge remote-tracking branch 'upstream/main' into SOLR-18085
epugh Jan 31, 2026
c76ad67
fix merge
epugh Jan 31, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion solr/core/src/java/org/apache/solr/core/RequestHandlers.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,8 @@ void initHandlersFromConfig(SolrConfig config) {
modifiedInfos.add(applyInitParams(config, info));
}
handlers.init(Collections.emptyMap(), core, modifiedInfos);
handlers.alias(handlers.getDefault(), "");
// Curious if this is actually needed!
// handlers.alias(handlers.getDefault(), "");
Copy link
Contributor Author

@epugh epugh Jan 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tests all pass!

Comment on lines +120 to +121
Copy link

Copilot AI Jan 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Commenting out the aliasing of the configured default handler to the empty path ("") can change runtime behavior for configs that set a non-/select handler as the default. For example, solr/modules/langid/src/test-files/langid/solr/collection1/conf/solrconfig-languageidentifier.xml defines a default handler named "search"; without this aliasing, requests to the core root will be routed to /select instead of the declared default. Consider restoring alias(handlers.getDefault(), "") (or otherwise ensuring plugin-bag default takes precedence over the /select fallback).

Suggested change
// Curious if this is actually needed!
// handlers.alias(handlers.getDefault(), "");
// Ensure the configured default handler (if any) is aliased to the empty path ("")
String defaultHandler = handlers.getDefault();
if (defaultHandler != null) {
handlers.alias(defaultHandler, "");
}

Copilot uses AI. Check for mistakes.
if (log.isDebugEnabled()) {
log.debug("Registered paths: {}", StrUtils.join(new ArrayList<>(handlers.keySet()), ','));
}
Expand Down
6 changes: 3 additions & 3 deletions solr/core/src/java/org/apache/solr/core/SolrCore.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3133,12 +3133,12 @@ private void initWriters() {
responseWriters.init(defaultWriters, this);

// configure the default response writer; this one should never be null
if (responseWriters.getDefault() == null) responseWriters.setDefault("standard");
// if (responseWriters.getDefault() == null) responseWriters.setDefault("standard");
Copy link

Copilot AI Jan 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The default response writer is no longer being set when none is explicitly marked default in solrconfig. This can cause core.getQueryResponseWriter(req) / core.getQueryResponseWriter(null) to return null, which leads to NullPointerExceptions in call sites that assume a non-null default (e.g., GeoTransformerFactory uses req.getCore().getQueryResponseWriter(req) and then calls getClass() on it). Consider restoring a non-null default (likely "json" now that "standard" is being removed) and keep unknown-wt validation separate from default selection.

Suggested change
// if (responseWriters.getDefault() == null) responseWriters.setDefault("standard");
if (responseWriters.getDefault() == null) {
// Use "json" as the default writer now that the legacy "standard" writer is being removed.
responseWriters.setDefault("json");
}

Copilot uses AI. Check for mistakes.
}

/** Finds a writer by name, or returns the default writer if not found. */
/** Finds a writer by name, or null if not found. */
public final QueryResponseWriter getQueryResponseWriter(String writerName) {
return responseWriters.get(writerName, true);
return responseWriters.get(writerName, false);
}

/**
Expand Down
24 changes: 12 additions & 12 deletions solr/core/src/java/org/apache/solr/servlet/SolrRequestParsers.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,11 +68,11 @@ public class SolrRequestParsers {
private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());

// Should these constants be in a more public place?
public static final String MULTIPART = "multipart";
public static final String FORMDATA = "formdata";
public static final String RAW = "raw";
public static final String SIMPLE = "simple";
public static final String STANDARD = "standard";
// public static final String MULTIPART = "multipart";
// public static final String FORMDATA = "formdata";
Copy link
Contributor Author

@epugh epugh Jan 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so many unused statics!

// public static final String RAW = "raw";
// public static final String SIMPLE = "simple";
// public static final String STANDARD = "standard";
Comment on lines 70 to +75
Copy link

Copilot AI Jan 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change leaves large blocks of commented-out code (public constants and the parser registry) rather than removing them. Since these are now dead code paths, it’s better to delete them to avoid confusion and reduce maintenance overhead (or keep them active if they’re still intended to be supported).

Copilot uses AI. Check for mistakes.

private static final Charset CHARSET_US_ASCII = StandardCharsets.US_ASCII;

Expand All @@ -81,7 +81,7 @@ public class SolrRequestParsers {

public static final String REQUEST_TIMER_SERVLET_ATTRIBUTE = "org.apache.solr.RequestTimer";

private final HashMap<String, SolrRequestParser> parsers = new HashMap<>();
// private final HashMap<String, SolrRequestParser> parsers = new HashMap<>();
private StandardRequestParser standard;

/**
Expand Down Expand Up @@ -120,12 +120,12 @@ private void init(int multipartUploadLimitKB, int formUploadLimitKB) {

// I don't see a need to have this publicly configured just yet
// adding it is trivial
parsers.put(MULTIPART, multi);
parsers.put(FORMDATA, formdata);
parsers.put(RAW, raw);
parsers.put(SIMPLE, new SimpleRequestParser());
parsers.put(STANDARD, standard);
parsers.put("", standard);
// parsers.put(MULTIPART, multi);
// parsers.put(FORMDATA, formdata);
// parsers.put(RAW, raw);
// parsers.put(SIMPLE, new SimpleRequestParser());
// parsers.put(STANDARD, standard);
// parsers.put("", standard);
}

private static RTimerTree getRequestTimer(HttpServletRequest req) {
Expand Down
2 changes: 1 addition & 1 deletion solr/core/src/test/org/apache/solr/OutputWriterTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ public static void beforeClass() throws Exception {
@Test
public void testSOLR59responseHeaderVersions() {
// default results in "new" responseHeader
lrf.args.put("wt", "standard");
lrf.args.put("wt", "json");
assertQ(req("foo"), "/response/lst[@name='responseHeader']/int[@name='status'][.='0']");
lrf.args.remove("wt");
assertQ(req("foo"), "/response/lst[@name='responseHeader']/int[@name='QTime']");
Expand Down
25 changes: 13 additions & 12 deletions solr/core/src/test/org/apache/solr/handler/V2ApiIntegrationTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,26 +114,27 @@ public void testIntrospect() throws Exception {
}

@Test
public void testWTParam() throws Exception {
public void testInvalidWTParamReturnsError() throws Exception {
V2Request request = new V2Request.Builder("/c/" + COLL_NAME + "/get/_introspect").build();
// TODO: If possible do this in a better way
// Using an invalid wt parameter should return a 500 error
Copy link
Contributor Author

@epugh epugh Jan 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the Big Change.

request.setResponseParser(new InputStreamResponseParser("bleh"));
NamedList<Object> res = cluster.getSolrClient().request(request);
String respString = InputStreamResponseParser.consumeResponseToString(res);

assertFalse(respString.contains("<body><h2>HTTP ERROR 500</h2>"));
assertFalse(respString.contains("500"));
assertFalse(respString.contains("NullPointerException"));
assertFalse(
respString.contains(
"<p>Problem accessing /solr/____v2/c/collection1/get/_introspect. Reason:"));
// since no-op response writer is used, doing contains match
assertTrue(respString.contains("/c/collection1/get"));
// Should get a 500 Server Error for unknown writer type
assertTrue(
"Expected error message about unknown writer type",
respString.contains("Unknown response writer type"));
assertTrue("Expected 500 error code", respString.contains("500"));
}

// no response parser
@Test
public void testWTParam() throws Exception {
// When no response parser is set, the default JSON writer should be used
V2Request request = new V2Request.Builder("/c/" + COLL_NAME + "/get/_introspect").build();
request.setResponseParser(null);
Map<?, ?> resp = resAsMap(cluster.getSolrClient(), request);
respString = resp.toString();
String respString = resp.toString();

assertFalse(respString.contains("<body><h2>HTTP ERROR 500</h2>"));
assertFalse(
Expand Down
3 changes: 2 additions & 1 deletion solr/core/src/test/org/apache/solr/response/TestPrometheusResponseWriter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,8 @@ public void testUnsupportedMetricsFormat() throws Exception {

try (SolrClient adminClient = getHttpSolrClient(solrTestRule.getBaseUrl())) {
NamedList<Object> res = adminClient.request(req);
assertEquals(400, res.get("responseStatus"));
// Unknown wt parameter should return a 500 error
assertEquals(500, res.get("responseStatus"));
}
}
}
24 changes: 11 additions & 13 deletions solr/core/src/test/org/apache/solr/response/TestRawResponseWriter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ public class TestRawResponseWriter extends SolrTestCaseJ4 {
private static RawResponseWriter writerJsonBase;
private static RawResponseWriter writerBinBase;
private static RawResponseWriter writerCborBase;
private static RawResponseWriter writerNoBase;
// private static RawResponseWriter writerNoBase;

private static RawResponseWriter[] allWriters;

Expand All @@ -56,24 +56,22 @@ public static void setupCoreAndWriters() throws Exception {
// we spin up.
initCore("solrconfig.xml", "schema.xml");

writerNoBase = newRawResponseWriter(null); /* defaults to standard writer as base */
// writerNoBase = newRawResponseWriter(null); /* defaults to standard writer as base */
writerXmlBase = newRawResponseWriter("xml");
writerJsonBase = newRawResponseWriter("json");
writerBinBase = newRawResponseWriter("javabin");
writerCborBase = newRawResponseWriter("cbor");

allWriters =
new RawResponseWriter[] {
writerXmlBase, writerJsonBase, writerBinBase, writerCborBase, writerNoBase
};
new RawResponseWriter[] {writerXmlBase, writerJsonBase, writerBinBase, writerCborBase};
}

@AfterClass
public static void cleanupWriters() {
writerXmlBase = null;
writerJsonBase = null;
writerBinBase = null;
writerNoBase = null;
// writerNoBase = null;
writerCborBase = null;
allWriters = null;
}
Expand Down Expand Up @@ -120,7 +118,7 @@ public void testRawStringContentStream() throws IOException {
// we should have UTF-8 Bytes if we use an OutputStream
ByteArrayOutputStream bout = new ByteArrayOutputStream();
writer.write(bout, req(), rsp);
assertEquals(data, bout.toString(StandardCharsets.UTF_8.toString()));
assertEquals(data, bout.toString(StandardCharsets.UTF_8));
}
}

Expand All @@ -133,7 +131,7 @@ public void testStructuredDataViaBaseWriters() throws IOException {
rsp.add("foo", "bar");

// check Content-Type against each writer
assertEquals("application/xml; charset=UTF-8", writerNoBase.getContentType(req(), rsp));
// assertEquals("application/xml; charset=UTF-8", writerNoBase.getContentType(req(), rsp));
assertEquals("application/xml; charset=UTF-8", writerXmlBase.getContentType(req(), rsp));
assertEquals("application/json; charset=UTF-8", writerJsonBase.getContentType(req(), rsp));
assertEquals("application/octet-stream", writerBinBase.getContentType(req(), rsp));
Expand All @@ -153,13 +151,13 @@ public void testStructuredDataViaBaseWriters() throws IOException {
assertEquals(xml, writerXmlBase.writeToString(req(), rsp));
ByteArrayOutputStream xmlBout = new ByteArrayOutputStream();
writerXmlBase.write(xmlBout, req(), rsp);
assertEquals(xml, xmlBout.toString(StandardCharsets.UTF_8.toString()));
assertEquals(xml, xmlBout.toString(StandardCharsets.UTF_8));

//
assertEquals(xml, writerNoBase.writeToString(req(), rsp));
ByteArrayOutputStream noneBout = new ByteArrayOutputStream();
writerNoBase.write(noneBout, req(), rsp);
assertEquals(xml, noneBout.toString(StandardCharsets.UTF_8.toString()));
// assertEquals(xml, writerNoBase.writeToString(req(), rsp));
// ByteArrayOutputStream noneBout = new ByteArrayOutputStream();
// writerNoBase.write(noneBout, req(), rsp);
// assertEquals(xml, noneBout.toString(StandardCharsets.UTF_8.toString()));

// json
String json = "{\n" + " \"content\":\"test\",\n" + " \"foo\":\"bar\"}\n";
Expand Down
1 change: 1 addition & 0 deletions solr/test-framework/src/java/org/apache/solr/util/TestHarness.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -419,6 +419,7 @@ public LocalRequestFactory() {}
@SuppressWarnings({"unchecked"})
public LocalSolrQueryRequest makeRequest(String... q) {
if (q.length == 1) {
args.computeIfAbsent("wt", k -> "xml");
return new LocalSolrQueryRequest(
TestHarness.this.getCore(), q[0], qtype, start, limit, args);
}
Expand Down
Loading