epugh commented on code in PR #4073:
URL: https://github.com/apache/solr/pull/4073#discussion_r2724142169


##########
solr/core/src/java/org/apache/solr/core/SolrCore.java:
##########
@@ -3148,11 +3116,49 @@ default String getContentType() {
   }
 
   /**
-   * Configure the query response writers. There will always be a default 
writer; additional writers
-   * may also be configured.
+   * Gets a response writer suitable for admin/container-level requests.
+   *
+   * @param writerName the writer name, or null for default
+   * @return the response writer, never null
+   * @deprecated Use {@link
+   *     
org.apache.solr.response.BuiltInResponseWriterRegistry#getWriter(String)} 
instead.
+   */
+  @Deprecated
+  public static QueryResponseWriter getAdminResponseWriter(String writerName) {
+    return 
org.apache.solr.response.BuiltInResponseWriterRegistry.getWriter(writerName);
+  }
+
+  /**
+   * Initializes query response writers. Response writers from {@code 
ImplicitPlugins.json} may also
+   * be configured.
    */
   private void initWriters() {
-    responseWriters.init(DEFAULT_RESPONSE_WRITERS, this);
+    // Build default writers map from implicit plugins
+    Map<String, QueryResponseWriter> defaultWriters = new HashMap<>();
+
+    // Load writers from ImplicitPlugins.json
+    List<PluginInfo> implicitWriters = getImplicitResponseWriters();
+    for (PluginInfo info : implicitWriters) {
+      try {
+        QueryResponseWriter writer =
+            createInstance(
+                info.className,
+                QueryResponseWriter.class,
+                "queryResponseWriter",
+                null,

Review Comment:
   This is interesting, and I *think* a separte issue.  None of the 
QueryResponseWriter's take a core.  RawResponseWriter looks up a core from a 
specific request, but doesn't take it directly.  There are. a few calls to the 
createInstance and they mostly pass a `null` value.  The only place I see a 
core being passed in is the call from `PackagePluginHolder.initNewInstance`.  I 
don't know if there is a refactoring to have two createInstances?  Or one that 
you call that doesn't take a core and then calls this one for a null?  Probably 
for another PR.   So, based on this analysis, I am going to leave this null.  
This is just a bit icky.



-- 
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]

Reply via email to