From 44a83b04ffcbd6bcd8a7fe27ffceb5d0d6796ec3 Mon Sep 17 00:00:00 2001 From: David Smiley Date: Sun, 2 Feb 2025 10:41:11 -0500 Subject: [PATCH] Also clarify SolrRequest.getParams nad QParser params --- .../java/org/apache/solr/handler/api/V2ApiUtils.java | 3 --- .../org/apache/solr/request/SolrQueryRequest.java | 6 +++--- .../apache/solr/request/SolrQueryRequestBase.java | 7 ++++--- .../src/java/org/apache/solr/search/QParser.java | 12 +++++++----- .../java/org/apache/solr/search/QParserPlugin.java | 6 +++++- .../org/apache/solr/handler/api/V2ApiUtilsTest.java | 5 ++--- .../BaseTestRuleBasedAuthorizationPlugin.java | 2 +- 7 files changed, 22 insertions(+), 19 deletions(-) diff --git a/solr/core/src/java/org/apache/solr/handler/api/V2ApiUtils.java b/solr/core/src/java/org/apache/solr/handler/api/V2ApiUtils.java index 22f492abf92..9a96b34afc0 100644 --- a/solr/core/src/java/org/apache/solr/handler/api/V2ApiUtils.java +++ b/solr/core/src/java/org/apache/solr/handler/api/V2ApiUtils.java @@ -91,9 +91,6 @@ public static void squashIntoNamedListWithoutHeader( } public static String getMediaTypeFromWtParam(SolrParams params, String defaultMediaType) { - if (params == null) { - return defaultMediaType; - } final String wtParam = params.get(WT); if (StrUtils.isBlank(wtParam)) return defaultMediaType; diff --git a/solr/core/src/java/org/apache/solr/request/SolrQueryRequest.java b/solr/core/src/java/org/apache/solr/request/SolrQueryRequest.java index 4b2a4d543d4..52ab91fcf9f 100644 --- a/solr/core/src/java/org/apache/solr/request/SolrQueryRequest.java +++ b/solr/core/src/java/org/apache/solr/request/SolrQueryRequest.java @@ -75,7 +75,7 @@ static boolean disallowPartialResults(SolrParams params) { return !allowPartialResults(params); } - /** returns the current request parameters */ + /** The parameters for this request; never null. Use {@link #setParams(SolrParams)} to change. */ SolrParams getParams(); /** @@ -88,8 +88,8 @@ static boolean disallowPartialResults(SolrParams params) { Iterable getContentStreams(); /** - * Returns the original request parameters. As this does not normally include configured defaults - * it's more suitable for logging. + * The original request parameters; never null. As this does not normally include configured + * defaults, it's more suitable for logging. */ SolrParams getOriginalParams(); diff --git a/solr/core/src/java/org/apache/solr/request/SolrQueryRequestBase.java b/solr/core/src/java/org/apache/solr/request/SolrQueryRequestBase.java index 363022c3979..806a08e43e8 100644 --- a/solr/core/src/java/org/apache/solr/request/SolrQueryRequestBase.java +++ b/solr/core/src/java/org/apache/solr/request/SolrQueryRequestBase.java @@ -22,6 +22,7 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Objects; import org.apache.solr.api.ApiBag; import org.apache.solr.common.SolrException; import org.apache.solr.common.params.SolrParams; @@ -62,8 +63,8 @@ public abstract class SolrQueryRequestBase implements SolrQueryRequest, Closeabl public SolrQueryRequestBase(SolrCore core, SolrParams params, RTimerTree requestTimer) { this.core = core; this.schema = null == core ? null : core.getLatestSchema(); - this.params = this.origParams = params; - this.requestTimer = requestTimer; + this.params = this.origParams = Objects.requireNonNull(params); + this.requestTimer = Objects.requireNonNull(requestTimer); this.startTime = System.currentTimeMillis(); } @@ -90,7 +91,7 @@ public SolrParams getOriginalParams() { @Override public void setParams(SolrParams params) { - this.params = params; + this.params = Objects.requireNonNull(params); } // Get the start time of this request in milliseconds diff --git a/solr/core/src/java/org/apache/solr/search/QParser.java b/solr/core/src/java/org/apache/solr/search/QParser.java index e6723cff3bf..09c7a05fe84 100644 --- a/solr/core/src/java/org/apache/solr/search/QParser.java +++ b/solr/core/src/java/org/apache/solr/search/QParser.java @@ -21,6 +21,7 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Objects; import org.apache.lucene.search.Query; import org.apache.solr.common.SolrException; import org.apache.solr.common.params.CommonParams; @@ -62,10 +63,11 @@ public abstract class QParser { /** * Constructor for the QParser * - * @param qstr The part of the query string specific to this parser - * @param localParams The set of parameters that are specific to this QParser. See + * @param qstr The query string to be parsed + * @param localParams Params scoped to this query, customizing the parsing. Null if none. Most + * params the parser needs are resolved here first, overlaying the request. See * https://solr.apache.org/guide/solr/latest/query-guide/local-params.html - * @param params The rest of the {@link org.apache.solr.common.params.SolrParams} + * @param params Params for the request, taken from {@link SolrQueryRequest#getParams()}; not null * @param req The original {@link org.apache.solr.request.SolrQueryRequest}. */ public QParser(String qstr, SolrParams localParams, SolrParams params, SolrQueryRequest req) { @@ -97,7 +99,7 @@ public QParser(String qstr, SolrParams localParams, SolrParams params, SolrQuery } } - this.params = params; + this.params = Objects.requireNonNull(params); this.req = req; } @@ -160,7 +162,7 @@ public SolrParams getParams() { } public void setParams(SolrParams params) { - this.params = params; + this.params = Objects.requireNonNull(params); } public SolrQueryRequest getReq() { diff --git a/solr/core/src/java/org/apache/solr/search/QParserPlugin.java b/solr/core/src/java/org/apache/solr/search/QParserPlugin.java index 8146c585cf0..f87b35b55dc 100644 --- a/solr/core/src/java/org/apache/solr/search/QParserPlugin.java +++ b/solr/core/src/java/org/apache/solr/search/QParserPlugin.java @@ -95,7 +95,11 @@ public abstract class QParserPlugin implements NamedListInitializedPlugin, SolrI standardPlugins = Collections.unmodifiableMap(map); } - /** return a {@link QParser} */ + /** + * Creates the {@link QParser}. + * + * @see QParser#QParser(String, SolrParams, SolrParams, SolrQueryRequest) + */ public abstract QParser createParser( String qstr, SolrParams localParams, SolrParams params, SolrQueryRequest req); diff --git a/solr/core/src/test/org/apache/solr/handler/api/V2ApiUtilsTest.java b/solr/core/src/test/org/apache/solr/handler/api/V2ApiUtilsTest.java index 78f2f8d5fb9..9a4945cfb94 100644 --- a/solr/core/src/test/org/apache/solr/handler/api/V2ApiUtilsTest.java +++ b/solr/core/src/test/org/apache/solr/handler/api/V2ApiUtilsTest.java @@ -22,6 +22,7 @@ import jakarta.ws.rs.core.MediaType; import org.apache.solr.SolrTestCaseJ4; import org.apache.solr.common.params.ModifiableSolrParams; +import org.apache.solr.common.params.SolrParams; import org.junit.Test; public class V2ApiUtilsTest extends SolrTestCaseJ4 { @@ -44,9 +45,7 @@ public void testReadsDisableV2ApiSysprop() { @Test public void testConvertsWtToMediaTypeString() { - assertEquals( - "someDefault", - V2ApiUtils.getMediaTypeFromWtParam(new ModifiableSolrParams(), "someDefault")); + assertEquals("someDefault", V2ApiUtils.getMediaTypeFromWtParam(SolrParams.of(), "someDefault")); var params = new ModifiableSolrParams(); params.add("wt", "json"); diff --git a/solr/core/src/test/org/apache/solr/security/BaseTestRuleBasedAuthorizationPlugin.java b/solr/core/src/test/org/apache/solr/security/BaseTestRuleBasedAuthorizationPlugin.java index 196120ca628..c861dc2458c 100644 --- a/solr/core/src/test/org/apache/solr/security/BaseTestRuleBasedAuthorizationPlugin.java +++ b/solr/core/src/test/org/apache/solr/security/BaseTestRuleBasedAuthorizationPlugin.java @@ -840,7 +840,7 @@ public MockAuthorizationContext(Map values) { @Override public SolrParams getParams() { SolrParams params = (SolrParams) values.get("params"); - return params == null ? new MapSolrParams(new HashMap<>()) : params; + return params == null ? SolrParams.of() : params; } @Override