This is an automated email from the ASF dual-hosted git repository.
dsmiley pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/solr.git
The following commit(s) were added to refs/heads/main by this push:
new f43171e145a SolrJ: simplify ResponseParser.getContentType processing
(#4528)
f43171e145a is described below
commit f43171e145a3562509a9dadeb57c50adbd0a271b
Author: David Smiley <[email protected]>
AuthorDate: Tue Jun 23 14:48:32 2026 -0400
SolrJ: simplify ResponseParser.getContentType processing (#4528)
ResponseParser.getContentType must now return a Set, statically defined.
Enforced by RP's constructor.
---
.../PR#4528-ResponseParser_getContentType.yml | 8 ++++++
.../org/apache/solr/search/TestSmileRequest.java | 8 ++++--
.../client/solrj/jetty/HttpJettySolrClient.java | 28 ------------------
.../solr/client/solrj/impl/HttpJdkSolrClient.java | 33 ----------------------
.../solr/client/solrj/impl/HttpSolrClient.java | 25 ++++------------
.../solrj/response/JavaBinResponseParser.java | 8 ++++--
.../solr/client/solrj/response/ResponseParser.java | 33 +++++++++++++++++++---
.../client/solrj/response/XMLResponseParser.java | 4 ++-
.../json/JacksonDataBindResponseParser.java | 9 +++---
.../solrj/response/json/JsonMapResponseParser.java | 7 +++--
.../solr/client/solrj/SolrExampleCborTest.java | 5 +++-
.../client/solrj/impl/HttpJdkSolrClientTest.java | 23 ---------------
12 files changed, 68 insertions(+), 123 deletions(-)
diff --git a/changelog/unreleased/PR#4528-ResponseParser_getContentType.yml
b/changelog/unreleased/PR#4528-ResponseParser_getContentType.yml
new file mode 100644
index 00000000000..e9de829b06d
--- /dev/null
+++ b/changelog/unreleased/PR#4528-ResponseParser_getContentType.yml
@@ -0,0 +1,8 @@
+title: >
+ Simplify SolrJ ResponseParser.getContentType processing. getContentType
must now return a Set.
+type: other
+authors:
+ - name: David Smiley
+links:
+ - name: PR#4528
+ url: https://github.com/apache/solr/pull/4528
diff --git a/solr/core/src/test/org/apache/solr/search/TestSmileRequest.java
b/solr/core/src/test/org/apache/solr/search/TestSmileRequest.java
index 79c9e0a35c3..a3bb792496b 100644
--- a/solr/core/src/test/org/apache/solr/search/TestSmileRequest.java
+++ b/solr/core/src/test/org/apache/solr/search/TestSmileRequest.java
@@ -18,7 +18,6 @@ package org.apache.solr.search;
import java.io.IOException;
import java.io.InputStream;
-import java.util.Collection;
import java.util.Map;
import java.util.Set;
import org.apache.solr.JSONTestUtil;
@@ -106,9 +105,12 @@ public class TestSmileRequest extends SolrTestCaseJ4 {
return new NamedList(m);
}
+ private static final Set<String> CONTENT_TYPES =
+ Set.of("application/x-jackson-smile", "application/octet-stream");
+
@Override
- public Collection<String> getContentTypes() {
- return Set.of("application/x-jackson-smile", "application/octet-stream");
+ public Set<String> getContentTypes() {
+ return CONTENT_TYPES;
}
}
}
diff --git
a/solr/solrj-jetty/src/java/org/apache/solr/client/solrj/jetty/HttpJettySolrClient.java
b/solr/solrj-jetty/src/java/org/apache/solr/client/solrj/jetty/HttpJettySolrClient.java
index d0d8e034c22..fbc276cc992 100644
---
a/solr/solrj-jetty/src/java/org/apache/solr/client/solrj/jetty/HttpJettySolrClient.java
+++
b/solr/solrj-jetty/src/java/org/apache/solr/client/solrj/jetty/HttpJettySolrClient.java
@@ -26,7 +26,6 @@ import java.util.ArrayList;
import java.util.Collection;
import java.util.Iterator;
import java.util.List;
-import java.util.Locale;
import java.util.Map;
import java.util.concurrent.CancellationException;
import java.util.concurrent.CompletableFuture;
@@ -37,7 +36,6 @@ import java.util.concurrent.RejectedExecutionException;
import java.util.concurrent.Semaphore;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.TimeoutException;
-import java.util.stream.Collectors;
import org.apache.solr.client.api.util.SolrVersion;
import org.apache.solr.client.solrj.SolrClient;
import org.apache.solr.client.solrj.SolrRequest;
@@ -161,7 +159,6 @@ public class HttpJettySolrClient extends HttpSolrClient {
this.listenerFactory = new ArrayList<>(0);
}
- updateDefaultMimeTypeForParser();
this.idleTimeoutMillis = builder.getIdleTimeoutMillis();
try {
@@ -807,31 +804,6 @@ public class HttpJettySolrClient extends HttpSolrClient {
return httpClient.isFollowRedirects();
}
- @Override
- protected boolean processorAcceptsMimeType(
- Collection<String> processorSupportedContentTypes, String mimeType) {
-
- return processorSupportedContentTypes.stream()
- .map(ct -> MimeTypes.getContentTypeWithoutCharset(ct).trim())
- .anyMatch(mimeType::equalsIgnoreCase);
- }
-
- @Override
- protected void updateDefaultMimeTypeForParser() {
- defaultParserMimeTypes =
- parser.getContentTypes().stream()
- .map(ct ->
MimeTypes.getContentTypeWithoutCharset(ct).trim().toLowerCase(Locale.ROOT))
- .collect(Collectors.toSet());
- }
-
- @Override
- protected String allProcessorSupportedContentTypesCommaDelimited(
- Collection<String> processorSupportedContentTypes) {
- return processorSupportedContentTypes.stream()
- .map(ct ->
MimeTypes.getContentTypeWithoutCharset(ct).trim().toLowerCase(Locale.ROOT))
- .collect(Collectors.joining(", "));
- }
-
/**
* An HttpJettySolrClient that doesn't close or cleanup any resources
*
diff --git
a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpJdkSolrClient.java
b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpJdkSolrClient.java
index 5489cac7058..1f88adc519f 100644
---
a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpJdkSolrClient.java
+++
b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpJdkSolrClient.java
@@ -37,7 +37,6 @@ import java.util.Collection;
import java.util.HashMap;
import java.util.Locale;
import java.util.Map;
-import java.util.Objects;
import java.util.concurrent.BlockingQueue;
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.ExecutorService;
@@ -46,7 +45,6 @@ import java.util.concurrent.LinkedBlockingQueue;
import java.util.concurrent.TimeUnit;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
-import java.util.stream.Collectors;
import javax.net.ssl.SSLContext;
import org.apache.solr.client.api.util.SolrVersion;
import org.apache.solr.client.solrj.SolrRequest;
@@ -139,7 +137,6 @@ public class HttpJdkSolrClient extends HttpSolrClient {
ProxySelector.of(new InetSocketAddress(builder.getProxyHost(),
builder.getProxyPort())));
}
this.httpClient = b.build();
- updateDefaultMimeTypeForParser();
assert ObjectReleaseTracker.track(this);
}
@@ -537,36 +534,6 @@ public class HttpJdkSolrClient extends HttpSolrClient {
return httpClient.followRedirects() != HttpClient.Redirect.NEVER;
}
- @Override
- protected boolean processorAcceptsMimeType(
- Collection<String> processorSupportedContentTypes, String mimeType) {
- return processorSupportedContentTypes.stream()
- .map(this::contentTypeToMimeType)
- .filter(Objects::nonNull)
- .map(String::trim)
- .anyMatch(mimeType::equalsIgnoreCase);
- }
-
- @Override
- protected void updateDefaultMimeTypeForParser() {
- defaultParserMimeTypes =
- parser.getContentTypes().stream()
- .map(this::contentTypeToMimeType)
- .filter(Objects::nonNull)
- .map(s -> s.toLowerCase(Locale.ROOT).trim())
- .collect(Collectors.toSet());
- }
-
- @Override
- protected String allProcessorSupportedContentTypesCommaDelimited(
- Collection<String> processorSupportedContentTypes) {
- return processorSupportedContentTypes.stream()
- .map(this::contentTypeToMimeType)
- .filter(Objects::nonNull)
- .map(s -> s.toLowerCase(Locale.ROOT).trim())
- .collect(Collectors.joining(", "));
- }
-
@Override
protected BuilderBase<?, ?> toBuilder(String baseUrl) {
return new HttpJdkSolrClient.Builder(baseUrl).withHttpClient(this);
diff --git
a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpSolrClient.java
b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpSolrClient.java
index d1e69255bab..bcf5eb6b3c1 100644
--- a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpSolrClient.java
+++ b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpSolrClient.java
@@ -27,6 +27,7 @@ import java.nio.charset.Charset;
import java.nio.charset.StandardCharsets;
import java.util.Base64;
import java.util.Collection;
+import java.util.Locale;
import java.util.Objects;
import java.util.Set;
import java.util.concurrent.CompletableFuture;
@@ -72,11 +73,8 @@ public abstract class HttpSolrClient extends SolrClient {
protected RequestWriter requestWriter = new JavaBinRequestWriter();
- // updating parser instance needs to go via the setter to ensure update of
defaultParserMimeTypes
protected ResponseParser parser = new JavaBinResponseParser();
- protected Set<String> defaultParserMimeTypes;
-
protected final String basicAuthAuthorizationStr;
protected HttpSolrClient(String serverBaseUrl, BuilderBase<?, ?> builder) {
@@ -266,12 +264,6 @@ public abstract class HttpSolrClient extends SolrClient {
return processor == null || processor instanceof InputStreamResponseParser;
}
- protected abstract boolean processorAcceptsMimeType(
- Collection<String> processorSupportedContentTypes, String mimeType);
-
- protected abstract String allProcessorSupportedContentTypesCommaDelimited(
- Collection<String> processorSupportedContentTypes);
-
/**
* Validates that the content type in the response can be processed by the
Response Parser. Throws
* a {@code RemoteSolrException} if not.
@@ -283,19 +275,15 @@ public abstract class HttpSolrClient extends SolrClient {
String encoding,
int httpStatus,
String urlExceptionMessage) {
- if (mimeType == null
- || (processor == this.parser &&
defaultParserMimeTypes.contains(mimeType))) {
- // Shortcut the default scenario
+ if (mimeType == null) {
return;
}
final Collection<String> processorSupportedContentTypes =
processor.getContentTypes();
if (!processorSupportedContentTypes.isEmpty()) {
- boolean processorAcceptsMimeType =
- processorAcceptsMimeType(processorSupportedContentTypes, mimeType);
- if (!processorAcceptsMimeType) {
+ final String normalizedMimeType =
mimeType.toLowerCase(Locale.ROOT).trim();
+ if (!processorSupportedContentTypes.contains(normalizedMimeType)) {
// unexpected mime type
- final String allSupportedTypes =
-
allProcessorSupportedContentTypesCommaDelimited(processorSupportedContentTypes);
+ final String allSupportedTypes = String.join(", ",
processorSupportedContentTypes);
String prefix =
"Expected mime type in [" + allSupportedTypes + "] but got " +
mimeType + ". ";
String exceptionEncoding = encoding != null ? encoding :
FALLBACK_CHARSET.name();
@@ -322,11 +310,8 @@ public abstract class HttpSolrClient extends SolrClient {
protected void setParser(ResponseParser parser) {
this.parser = parser;
- updateDefaultMimeTypeForParser();
}
- protected abstract void updateDefaultMimeTypeForParser();
-
/**
* Executes a SolrRequest using the provided URL to temporarily override any
"base URL" currently
* used by this client
diff --git
a/solr/solrj/src/java/org/apache/solr/client/solrj/response/JavaBinResponseParser.java
b/solr/solrj/src/java/org/apache/solr/client/solrj/response/JavaBinResponseParser.java
index 0fd83fa1554..ded08f6d3c3 100644
---
a/solr/solrj/src/java/org/apache/solr/client/solrj/response/JavaBinResponseParser.java
+++
b/solr/solrj/src/java/org/apache/solr/client/solrj/response/JavaBinResponseParser.java
@@ -18,7 +18,6 @@ package org.apache.solr.client.solrj.response;
import java.io.IOException;
import java.io.InputStream;
-import java.util.Collection;
import java.util.Set;
import org.apache.solr.common.util.JavaBinCodec;
import org.apache.solr.common.util.NamedList;
@@ -52,8 +51,11 @@ public class JavaBinResponseParser extends ResponseParser {
return new JavaBinCodec(null, stringCache);
}
+ private static final Set<String> CONTENT_TYPES =
+ Set.of(JAVABIN_CONTENT_TYPE, JAVABIN_CONTENT_TYPE_V2);
+
@Override
- public Collection<String> getContentTypes() {
- return Set.of(JAVABIN_CONTENT_TYPE, JAVABIN_CONTENT_TYPE_V2);
+ public Set<String> getContentTypes() {
+ return CONTENT_TYPES;
}
}
diff --git
a/solr/solrj/src/java/org/apache/solr/client/solrj/response/ResponseParser.java
b/solr/solrj/src/java/org/apache/solr/client/solrj/response/ResponseParser.java
index 9d2fbc7a67e..9884d8a1ef5 100644
---
a/solr/solrj/src/java/org/apache/solr/client/solrj/response/ResponseParser.java
+++
b/solr/solrj/src/java/org/apache/solr/client/solrj/response/ResponseParser.java
@@ -19,6 +19,8 @@ package org.apache.solr.client.solrj.response;
import java.io.IOException;
import java.io.InputStream;
import java.util.Collection;
+import java.util.Locale;
+import java.util.Set;
import org.apache.solr.common.util.NamedList;
/**
@@ -28,6 +30,23 @@ import org.apache.solr.common.util.NamedList;
*/
public abstract class ResponseParser {
+ protected ResponseParser() {
+ assert validateContentTypes();
+ }
+
+ private boolean validateContentTypes() {
+ Collection<String> contentTypes = getContentTypes();
+ assert contentTypes == getContentTypes()
+ : getClass().getName() + ".getContentTypes() must return the same
instance on every call";
+ for (String ct : contentTypes) {
+ assert !ct.contains(";") && ct.equals(ct.toLowerCase(Locale.ROOT))
+ : getClass().getName()
+ + ".getContentTypes() must return lowercase MIME types without
semicolons, got: "
+ + ct;
+ }
+ return true;
+ }
+
/** The writer type placed onto the request as the {@code wt} param. */
public abstract String getWriterType(); // for example: wt=XML, JSON, etc
@@ -35,10 +54,16 @@ public abstract class ResponseParser {
throws IOException;
/**
- * A well-behaved ResponseParser will return the content-types it supports.
+ * Returns the MIME types this parser supports. Return an empty set to
disable. Implementations
+ * must:
+ *
+ * <ul>
+ * <li>Returns the same instance (same reference on every call).
+ * <li>Use only lowercase MIME types without charset or other parameters
(no semicolons)
+ * <li>
+ * </ul>
*
- * @return the content-type values that this parser is capable of parsing.
Never null. Empty means
- * no enforcement.
+ * @return the MIME types that this parser is capable of parsing. Never null.
*/
- public abstract Collection<String> getContentTypes();
+ public abstract Set<String> getContentTypes();
}
diff --git
a/solr/solrj/src/java/org/apache/solr/client/solrj/response/XMLResponseParser.java
b/solr/solrj/src/java/org/apache/solr/client/solrj/response/XMLResponseParser.java
index 8e378989668..5bc358b4f5e 100644
---
a/solr/solrj/src/java/org/apache/solr/client/solrj/response/XMLResponseParser.java
+++
b/solr/solrj/src/java/org/apache/solr/client/solrj/response/XMLResponseParser.java
@@ -80,9 +80,11 @@ public class XMLResponseParser extends ResponseParser {
return "xml";
}
+ private static final Set<String> CONTENT_TYPES = Set.of("application/xml");
+
@Override
public Set<String> getContentTypes() {
- return Set.of(XML_CONTENT_TYPE);
+ return CONTENT_TYPES;
}
public NamedList<Object> processResponse(Reader in) {
diff --git
a/solr/solrj/src/java/org/apache/solr/client/solrj/response/json/JacksonDataBindResponseParser.java
b/solr/solrj/src/java/org/apache/solr/client/solrj/response/json/JacksonDataBindResponseParser.java
index e98eb104cae..a09ac3d2155 100644
---
a/solr/solrj/src/java/org/apache/solr/client/solrj/response/json/JacksonDataBindResponseParser.java
+++
b/solr/solrj/src/java/org/apache/solr/client/solrj/response/json/JacksonDataBindResponseParser.java
@@ -20,9 +20,8 @@ package org.apache.solr.client.solrj.response.json;
import java.io.IOException;
import java.io.InputStream;
import java.io.InputStreamReader;
-import java.util.Collection;
-import java.util.List;
import java.util.Map;
+import java.util.Set;
import org.apache.solr.client.api.model.SolrJerseyResponse;
import org.apache.solr.client.solrj.request.json.JacksonContentWriter;
import org.apache.solr.client.solrj.response.ResponseParser;
@@ -47,9 +46,11 @@ public class JacksonDataBindResponseParser<T> extends
ResponseParser {
return "json";
}
+ private static final Set<String> CONTENT_TYPES = Set.of("application/json");
+
@Override
- public Collection<String> getContentTypes() {
- return List.of("application/json");
+ public Set<String> getContentTypes() {
+ return CONTENT_TYPES;
}
// TODO it'd be nice if the ResponseParser could receive the mime type so it
can parse
diff --git
a/solr/solrj/src/java/org/apache/solr/client/solrj/response/json/JsonMapResponseParser.java
b/solr/solrj/src/java/org/apache/solr/client/solrj/response/json/JsonMapResponseParser.java
index 8410612e0a8..e1848ba44ff 100644
---
a/solr/solrj/src/java/org/apache/solr/client/solrj/response/json/JsonMapResponseParser.java
+++
b/solr/solrj/src/java/org/apache/solr/client/solrj/response/json/JsonMapResponseParser.java
@@ -20,7 +20,6 @@ package org.apache.solr.client.solrj.response.json;
import java.io.IOException;
import java.io.InputStream;
import java.io.InputStreamReader;
-import java.util.Collection;
import java.util.Map;
import java.util.Set;
import org.apache.solr.client.solrj.response.ResponseParser;
@@ -55,8 +54,10 @@ public class JsonMapResponseParser extends ResponseParser {
return list;
}
+ private static final Set<String> CONTENT_TYPES = Set.of("application/json");
+
@Override
- public Collection<String> getContentTypes() {
- return Set.of("application/json");
+ public Set<String> getContentTypes() {
+ return CONTENT_TYPES;
}
}
diff --git
a/solr/solrj/src/test/org/apache/solr/client/solrj/SolrExampleCborTest.java
b/solr/solrj/src/test/org/apache/solr/client/solrj/SolrExampleCborTest.java
index dda3a57426d..ff2d325579b 100644
--- a/solr/solrj/src/test/org/apache/solr/client/solrj/SolrExampleCborTest.java
+++ b/solr/solrj/src/test/org/apache/solr/client/solrj/SolrExampleCborTest.java
@@ -281,9 +281,12 @@ public class SolrExampleCborTest extends SolrExampleTests {
return "cbor";
}
+ private static final Set<String> CONTENT_TYPES =
+ Set.of("application/cbor", "application/octet-stream");
+
@Override
public Set<String> getContentTypes() {
- return Set.of("application/cbor", "application/octet-stream");
+ return CONTENT_TYPES;
}
@Override
diff --git
a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/HttpJdkSolrClientTest.java
b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/HttpJdkSolrClientTest.java
index fe159e4ad3f..73ff00474bb 100644
---
a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/HttpJdkSolrClientTest.java
+++
b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/HttpJdkSolrClientTest.java
@@ -437,29 +437,6 @@ public class HttpJdkSolrClientTest extends
HttpSolrClientTestBase {
}
}
- @Test
- public void testProcessorMimeTypes() throws Exception {
- ResponseParser rp = new XMLResponseParser();
-
- try (HttpJdkSolrClient client =
- builder(solrTestRule.getBaseUrl()).withResponseParser(rp).build()) {
- assertTrue(client.processorAcceptsMimeType(rp.getContentTypes(),
"application/xml"));
- assertFalse(client.processorAcceptsMimeType(rp.getContentTypes(),
"application/json"));
- queryToHelpJdkReleaseThreads(client);
- }
-
- rp = new JavaBinResponseParser();
- try (HttpJdkSolrClient client =
- builder(solrTestRule.getBaseUrl()).withResponseParser(rp).build()) {
- assertTrue(
- client.processorAcceptsMimeType(
- rp.getContentTypes(), "application/vnd.apache.solr.javabin"));
- assertTrue(client.processorAcceptsMimeType(rp.getContentTypes(),
"application/octet-stream"));
- assertFalse(client.processorAcceptsMimeType(rp.getContentTypes(),
"application/xml"));
- queryToHelpJdkReleaseThreads(client);
- }
- }
-
@Test
public void testContentTypeToEncoding() throws Exception {
try (HttpJdkSolrClient client =
builder(solrTestRule.getBaseUrl()).build()) {
