From d8d9e5c4c20458a0142eb41514f3d5f28f8c9b75 Mon Sep 17 00:00:00 2001 From: David Smiley Date: Sun, 14 Jun 2026 13:57:48 -0400 Subject: [PATCH 1/2] Simplify ResponseParser.getContentType processing Especially helps HttpSolrClient and implementations. --- .../apache/solr/search/TestSmileRequest.java | 8 +++-- .../solrj/jetty/HttpJettySolrClient.java | 28 ---------------- .../client/solrj/impl/HttpJdkSolrClient.java | 33 ------------------- .../client/solrj/impl/HttpSolrClient.java | 25 +++----------- .../solrj/response/JavaBinResponseParser.java | 8 +++-- .../client/solrj/response/ResponseParser.java | 33 ++++++++++++++++--- .../solrj/response/XMLResponseParser.java | 4 ++- .../json/JacksonDataBindResponseParser.java | 9 ++--- .../response/json/JsonMapResponseParser.java | 7 ++-- .../client/solrj/SolrExampleCborTest.java | 5 ++- .../solrj/impl/HttpJdkSolrClientTest.java | 23 ------------- 11 files changed, 60 insertions(+), 123 deletions(-) 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 79c9e0a35c3c..a3bb792496b2 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 @@ 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 NamedList processResponse(InputStream body, String encoding) thro return new NamedList(m); } + private static final Set CONTENT_TYPES = + Set.of("application/x-jackson-smile", "application/octet-stream"); + @Override - public Collection getContentTypes() { - return Set.of("application/x-jackson-smile", "application/octet-stream"); + public Set 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 d0d8e034c22c..fbc276cc9927 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.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.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 @@ protected HttpJettySolrClient(String serverBaseUrl, Builder builder) { this.listenerFactory = new ArrayList<>(0); } - updateDefaultMimeTypeForParser(); this.idleTimeoutMillis = builder.getIdleTimeoutMillis(); try { @@ -807,31 +804,6 @@ protected boolean isFollowRedirects() { return httpClient.isFollowRedirects(); } - @Override - protected boolean processorAcceptsMimeType( - Collection 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 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 5489cac70589..1f88adc519fd 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.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.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 @@ protected HttpJdkSolrClient(String serverBaseUrl, HttpJdkSolrClient.Builder buil ProxySelector.of(new InetSocketAddress(builder.getProxyHost(), builder.getProxyPort()))); } this.httpClient = b.build(); - updateDefaultMimeTypeForParser(); assert ObjectReleaseTracker.track(this); } @@ -537,36 +534,6 @@ protected boolean isFollowRedirects() { return httpClient.followRedirects() != HttpClient.Redirect.NEVER; } - @Override - protected boolean processorAcceptsMimeType( - Collection 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 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 93ae9e332f15..2ac096eaf3fb 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 @@ -28,6 +28,7 @@ import java.nio.charset.StandardCharsets; import java.util.Base64; import java.util.Collection; +import java.util.Locale; import java.util.Set; import java.util.concurrent.CompletableFuture; import java.util.concurrent.ExecutorService; @@ -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 defaultParserMimeTypes; - protected final String basicAuthAuthorizationStr; protected HttpSolrClient(String serverBaseUrl, BuilderBase builder) { @@ -268,12 +266,6 @@ protected boolean wantStream(final ResponseParser processor) { return processor == null || processor instanceof InputStreamResponseParser; } - protected abstract boolean processorAcceptsMimeType( - Collection processorSupportedContentTypes, String mimeType); - - protected abstract String allProcessorSupportedContentTypesCommaDelimited( - Collection processorSupportedContentTypes); - /** * Validates that the content type in the response can be processed by the Response Parser. Throws * a {@code RemoteSolrException} if not. @@ -285,19 +277,15 @@ private void checkContentType( 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 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(); @@ -324,11 +312,8 @@ protected static String basicAuthCredentialsToAuthorizationString(String user, S 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 0fd83fa1554a..ded08f6d3c37 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 @@ 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 @@ protected JavaBinCodec createCodec() { return new JavaBinCodec(null, stringCache); } + private static final Set CONTENT_TYPES = + Set.of(JAVABIN_CONTENT_TYPE, JAVABIN_CONTENT_TYPE_V2); + @Override - public Collection getContentTypes() { - return Set.of(JAVABIN_CONTENT_TYPE, JAVABIN_CONTENT_TYPE_V2); + public Set 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 9d2fbc7a67e0..9884d8a1ef57 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 @@ 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 @@ */ public abstract class ResponseParser { + protected ResponseParser() { + assert validateContentTypes(); + } + + private boolean validateContentTypes() { + Collection 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 NamedList processResponse(InputStream body, String encod 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: + * + *
    + *
  • Returns the same instance (same reference on every call). + *
  • Use only lowercase MIME types without charset or other parameters (no semicolons) + *
  • + *
* - * @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 getContentTypes(); + public abstract Set 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 8e378989668d..5bc358b4f5e7 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 String getWriterType() { return "xml"; } + private static final Set CONTENT_TYPES = Set.of("application/xml"); + @Override public Set getContentTypes() { - return Set.of(XML_CONTENT_TYPE); + return CONTENT_TYPES; } public NamedList 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 e98eb104caef..a09ac3d2155a 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 @@ 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 String getWriterType() { return "json"; } + private static final Set CONTENT_TYPES = Set.of("application/json"); + @Override - public Collection getContentTypes() { - return List.of("application/json"); + public Set 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 8410612e0a8a..e1848ba44ff0 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 @@ 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 NamedList processResponse(InputStream body, String encoding) thro return list; } + private static final Set CONTENT_TYPES = Set.of("application/json"); + @Override - public Collection getContentTypes() { - return Set.of("application/json"); + public Set 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 6456aa5824ca..1fe235b17d4a 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 String getWriterType() { return "cbor"; } + private static final Set CONTENT_TYPES = + Set.of("application/cbor", "application/octet-stream"); + @Override public Set 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 fe159e4ad3f3..73ff00474bb1 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 void testUseOptionalCredentialsWithNull() throws Exception { } } - @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()) { From c27e2d15f9a3dae18e5cf18e709f765e65dddd16 Mon Sep 17 00:00:00 2001 From: David Smiley Date: Thu, 18 Jun 2026 11:18:37 -0400 Subject: [PATCH 2/2] changelog --- .../PR#4528-ResponseParser_getContentType.yml | 32 +++++++++++++++++++ 1 file changed, 32 insertions(+) create mode 100644 changelog/unreleased/PR#4528-ResponseParser_getContentType.yml diff --git a/changelog/unreleased/PR#4528-ResponseParser_getContentType.yml b/changelog/unreleased/PR#4528-ResponseParser_getContentType.yml new file mode 100644 index 000000000000..8ad60b9c6aa1 --- /dev/null +++ b/changelog/unreleased/PR#4528-ResponseParser_getContentType.yml @@ -0,0 +1,32 @@ +# (DELETE ALL COMMENTS UP HERE AFTER FILLING THIS IN + +# See https://github.com/apache/solr/blob/main/dev-docs/changelog.adoc + +# If the change is minor, don't bother adding a changelog entry. +# For `other` type entries, the threshold to bother with a changelog entry should be even higher. + +# title: +# * The audience is end-users and administrators, not committers. +# * Be short and focused on the user impact. Multiple sentences is fine! +# * For technical/geeky details, prefer the commit message instead of changelog. +# * Reference JIRA issues like `SOLR-12345`, or if no JIRA but have a GitHub PR then `PR#12345`. + +# type: +# `added` for new features/improvements, opt-in by the user typically documented in the ref guide +# `changed` for improvements; not opt-in +# `fixed` for improvements that are deemed to have fixed buggy behavior +# `deprecated` for marking things deprecated +# `removed` for code removed +# `dependency_update` for updates to dependencies +# `other` for anything else, like large/significant refactorings, build changes, +# test infrastructure, or documentation. +# Most such changes are too small/minor to bother with a changelog entry. + +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