This is an automated email from the ASF dual-hosted git repository. dsmiley pushed a commit to branch branch_10x in repository https://gitbox.apache.org/repos/asf/solr.git
commit 605dfc1e8785a754802597c84dcc0840f751c7ba 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. (cherry picked from commit f43171e145a3562509a9dadeb57c50adbd0a271b) --- .../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 954db55f10c..df363449fba 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()) {
