dsmiley commented on code in PR #4101:
URL: https://github.com/apache/solr/pull/4101#discussion_r2778188033
##########
solr/core/src/java/org/apache/solr/request/SolrQueryRequestBase.java:
##########
@@ -191,7 +192,23 @@ public void setJSON(Map<String, Object> json) {
@Override
public Principal getUserPrincipal() {
- return null;
+ if (userPrincipalName == null) {
+ return null;
+ }
+ return new LocalPrincipal(userPrincipalName);
+ }
+
+ /**
+ * Allows setting the 'name' of the User Principal for the purposes of
creating local requests in
+ * a solr node when security is enabled. It is experimental and subject to
removal
+ *
+ * @see PKIAuthenticationPlugin#NODE_IS_USER
+ * @see #getUserPrincipal
+ * @lucene.internal
+ * @lucene.experimental
Review Comment:
Don't pick both. Based on what you said about this, `lucene.internal` is
right. Thus also remove the javadocs you added about "experimental". Just let
the annotation speak for itself instead of basically repeating in words what
the annotation is supposed to do to save you from writing the words you wrote!
##########
solr/core/src/java/org/apache/solr/request/SolrQueryRequestBase.java:
##########
@@ -137,7 +171,7 @@ public SolrCore getCore() {
// The index schema associated with this request
@Override
public IndexSchema getSchema() {
- // a request for a core admin will no have a core
+ // a request for a core admin will not have a core
Review Comment:
getCore() and getSchema() here (maybe more) are only applicable to requests
to a core. Node requests will return null.
##########
solr/core/src/java/org/apache/solr/request/SolrQueryRequestBase.java:
##########
@@ -209,11 +226,15 @@ public List<CommandOperation> getCommands(boolean
validateInput) {
return CommandOperation.clone(parsedCommands);
}
- protected ValidatingJsonMap getSpec() {
- return null;
- }
-
protected Map<String, JsonSchemaValidator> getValidators() {
return Collections.emptyMap();
}
+
+ private record LocalPrincipal(String user) implements Principal {
Review Comment:
ah; a record :-)
##########
solr/core/src/test/org/apache/solr/handler/admin/V2ApiMappingTest.java:
##########
@@ -146,70 +147,6 @@ protected SolrParams captureConvertedV1Params(String path,
String method, String
return captureConvertedV1Request(path, method, v2RequestBody).getParams();
}
- protected SolrParams captureConvertedV1Params(
Review Comment:
what happened here?
##########
solr/core/src/java/org/apache/solr/request/SolrQueryRequestBase.java:
##########
@@ -137,7 +171,7 @@ public SolrCore getCore() {
// The index schema associated with this request
@Override
public IndexSchema getSchema() {
- // a request for a core admin will no have a core
+ // a request for a core admin will not have a core
Review Comment:
I don't think this spot here is the right spot for such a comment. For one
thing; this "Base" class implements an interface, so if something about this is
to be said, it should be there.
##########
solr/test-framework/src/java/org/apache/solr/util/TestHarness.java:
##########
@@ -434,7 +463,7 @@ public LocalSolrQueryRequest makeRequest(String... q) {
@SuppressWarnings({"rawtypes"})
NamedList nl = new NamedList(entries);
if (nl.get("wt") == null) nl.add("wt", "xml");
- return new LocalSolrQueryRequest(TestHarness.this.getCore(), nl);
+ return new SolrQueryRequestBase(TestHarness.this.getCore(),
nl.toSolrParams());
Review Comment:
tiny scope creep request: remove most of these lines with a simple call to
SolrTestCaseJ4.params(q). No need for the NamedList
##########
solr/test-framework/src/java/org/apache/solr/util/TestHarness.java:
##########
@@ -190,6 +192,34 @@ public static NodeConfig buildTestNodeConfig(Path
solrHome) {
.build();
}
+ /**
+ * Utility method to build SolrParams from individual query components. This
is a convenience
+ * method for legacy code that needs to construct params from separate
query, qtype, start, limit,
+ * and additional args.
+ *
+ * @param query the query string (added as "q" param)
+ * @param qtype the query type (added as "qt" param)
+ * @param start the start offset (added as "start" param)
+ * @param limit the row limit (added as "rows" param)
+ * @param args additional parameters as a map
+ * @return SolrParams containing all the specified parameters
+ */
Review Comment:
lets deprecate this
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
