cpoerschke commented on code in PR #931:
URL: https://github.com/apache/solr/pull/931#discussion_r925510124


##########
solr/core/src/java/org/apache/solr/handler/admin/CoreAdminHandler.java:
##########
@@ -399,19 +424,16 @@ public void shutdown() {
     if (parallelExecutor != null) 
ExecutorUtil.shutdownAndAwaitTermination(parallelExecutor);
   }
 
-  private static final Map<String, CoreAdminOperation> opMap = new HashMap<>();
+  private static final Map<String, CoreAdminOp> opMap = new HashMap<>();
 
-  static class CallInfo {
-    final CoreAdminHandler handler;
-    final SolrQueryRequest req;
-    final SolrQueryResponse rsp;
-    final CoreAdminOperation op;
+  public static class CallInfo {
+    public final CoreAdminHandler handler;
+    public final SolrQueryRequest req;
+    public final SolrQueryResponse rsp;
+    public final CoreAdminOp op;
 
     CallInfo(
-        CoreAdminHandler handler,
-        SolrQueryRequest req,
-        SolrQueryResponse rsp,
-        CoreAdminOperation op) {
+        CoreAdminHandler handler, SolrQueryRequest req, SolrQueryResponse rsp, 
CoreAdminOp op) {

Review Comment:
   > ... for some reason when I try to run tests task it takes a huge amount of 
time and I have never succeeded to wait until it is ends ...
   
   Yes, running the full test suite can take quite a bit of time and even if it 
completes not all the tests pass all the time.
   
   > ... If you have some nice example of tests to base it on please advice ...
   
   * Tests for the configuration being read correctly: `git grep adminHandler` 
finds `solr/core/src/test-files/solr/solr-50-all.xml` as a match and `git grep 
solr-50-all.xml` finds 
`solr/core/src/test/org/apache/solr/core/TestSolrXml.java` i.e. `./gradlew test 
--tests TestSolrXml`
   * Tests for existing custom core admin handler: `git grep -l 
handleCustomAction` finds no match i.e. no existing test coverage it seems
   * Tests for existing core admin handler: `CoreAdminHandlerTest.java` looks 
like a potential match and 
https://github.com/apache/solr/blob/releases/solr/9.0.0/solr/test-framework/src/java/org/apache/solr/SolrTestCaseJ4.java#L858
 looks like it would provide a way to build a test without a `solr.xml` file 
specific to that test.



##########
solr/core/src/java/org/apache/solr/core/CoreContainer.java:
##########
@@ -822,6 +824,16 @@ public void load() {
     coreAdminHandler =
         createHandler(CORES_HANDLER_PATH, cfg.getCoreAdminHandlerClass(), 
CoreAdminHandler.class);
 
+    Map<String, CoreAdminOp> coreAdminHandlerActions =
+        cfg.getCoreAdminHandlerActions().entrySet().stream()
+            .collect(
+                Collectors.toMap(
+                    item -> item.getKey(),
+                    item -> createCoreAdminHandlerOperation(item.getValue(), 
CoreAdminOp.class)));

Review Comment:
   ```suggestion
                       item -> loader.newInstance(item.getValue(), 
CoreAdminOp.class)));
   ```



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