cpoerschke commented on a change in pull request #736:
URL: https://github.com/apache/solr/pull/736#discussion_r825802770



##########
File path: solr/core/src/test/org/apache/solr/handler/TestContainerPlugin.java
##########
@@ -548,16 +473,28 @@ public void m2(SolrQueryRequest req, SolrQueryResponse 
rsp) {
     }
   }
 
+  private Callable<V2Response> getPlugin(String path) {

Review comment:
       3/3 Annotated examples of use and non-use of `forceV2(true)` before the 
change to the getPlugin call.

##########
File path: solr/core/src/test/org/apache/solr/handler/TestContainerPlugin.java
##########
@@ -69,353 +69,274 @@
   private Phaser phaser;
 
   @Before
-  public void setup() {
+  public void setup() throws Exception {
     System.setProperty("enable.packages", "true");
     phaser = new Phaser();
+
+    int nodes = TEST_NIGHTLY ? 4 : 2;
+    cluster = configureCluster(nodes).withJettyConfig(jetty -> 
jetty.enableV2(true)).configure();
+    
cluster.getOpenOverseer().getCoreContainer().getContainerPluginsRegistry().setPhaser(phaser);
   }
 
   @After
-  public void teardown() {
+  public void teardown() throws Exception {
+    shutdownCluster();
     System.clearProperty("enable.packages");
   }
 
   @Test
   public void testApi() throws Exception {
-    MiniSolrCloudCluster cluster =
-        configureCluster(4).withJettyConfig(jetty -> 
jetty.enableV2(true)).configure();
-    ContainerPluginsRegistry pluginsRegistry =
-        
cluster.getOpenOverseer().getCoreContainer().getContainerPluginsRegistry();
-    pluginsRegistry.setPhaser(phaser);
-
     int version = phaser.getPhase();
 
-    String errPath = "/error/details[0]/errorMessages[0]";
-    try {
-      PluginMeta plugin = new PluginMeta();
+    PluginMeta plugin = new PluginMeta();
+    V2Request addPlugin =
+        new V2Request.Builder("/cluster/plugin")
+            .forceV2(true)
+            .POST()
+            .withPayload(singletonMap("add", plugin))
+            .build();
+
+    // test with an invalid class
+    try (ErrorLogMuter errors = 
ErrorLogMuter.substring("TestContainerPlugin$C2")) {
       plugin.name = "testplugin";
       plugin.klass = C2.class.getName();
-      // test with an invalid class
-      V2Request req =
-          new V2Request.Builder("/cluster/plugin")
-              .forceV2(true)
-              .POST()
-              .withPayload(singletonMap("add", plugin))
-              .build();
-      expectError(req, cluster.getSolrClient(), errPath, "No method with 
@Command in class");
-
-      // test with a valid class. This should succeed now
-      plugin.klass = C3.class.getName();
-      req.process(cluster.getSolrClient());
-
-      version = phaser.awaitAdvanceInterruptibly(version, 10, 
TimeUnit.SECONDS);
-
-      // just check if the plugin is indeed registered
-      V2Request readPluginState =
-          new V2Request.Builder("/cluster/plugin").forceV2(true).GET().build();
-      V2Response rsp = readPluginState.process(cluster.getSolrClient());
-      assertEquals(C3.class.getName(), rsp._getStr("/plugin/testplugin/class", 
null));
-
-      // let's test the plugin
-      TestDistribPackageStore.assertResponseValues(
-          10,
-          () ->
-              new V2Request.Builder("/plugin/my/plugin")
-                  .forceV2(true)
-                  .GET()
-                  .build()
-                  .process(cluster.getSolrClient()),
-          ImmutableMap.of("/testkey", "testval"));
-
-      // now remove the plugin
-      new V2Request.Builder("/cluster/plugin")
-          .POST()
-          .forceV2(true)
-          .withPayload("{remove : testplugin}")
-          .build()
-          .process(cluster.getSolrClient());
-
-      version = phaser.awaitAdvanceInterruptibly(version, 10, 
TimeUnit.SECONDS);
-
-      // verify it is removed
-      rsp = readPluginState.process(cluster.getSolrClient());
-      assertEquals(null, rsp._get("/plugin/testplugin/class", null));
+      expectError(addPlugin, "No method with @Command in class");
+      assertEquals(1, errors.getCount());
+    }
+
+    // test with a valid class. This should succeed now
+    plugin.klass = C3.class.getName();
+    addPlugin.process(cluster.getSolrClient());
+
+    version = phaser.awaitAdvanceInterruptibly(version, 10, TimeUnit.SECONDS);
 
+    // just check if the plugin is indeed registered
+    Callable<V2Response> readPluginState = getPlugin("/cluster/plugin");
+    V2Response rsp = readPluginState.call();
+    assertEquals(C3.class.getName(), rsp._getStr("/plugin/testplugin/class", 
null));
+
+    // let's test the plugin
+    TestDistribPackageStore.assertResponseValues(
+        getPlugin("/plugin/my/plugin"), Map.of("/testkey", "testval"));
+
+    // now remove the plugin
+    new V2Request.Builder("/cluster/plugin")
+        .POST()
+        .forceV2(true)
+        .withPayload("{remove : testplugin}")
+        .build()
+        .process(cluster.getSolrClient());
+
+    version = phaser.awaitAdvanceInterruptibly(version, 10, TimeUnit.SECONDS);
+
+    // verify it is removed
+    rsp = readPluginState.call();
+    assertNull(rsp._get("/plugin/testplugin/class", null));
+
+    try (ErrorLogMuter errors = 
ErrorLogMuter.substring("TestContainerPlugin$C4")) {
       // test with a class  @EndPoint methods. This also uses a template in 
the path name
       plugin.klass = C4.class.getName();
       plugin.name = "collections";
       plugin.pathPrefix = "collections";
-      expectError(
-          req, cluster.getSolrClient(), errPath, "path must not have a prefix: 
collections");
-
-      plugin.name = "my-random-name";
-      plugin.pathPrefix = "my-random-prefix";
-
-      req.process(cluster.getSolrClient());
-      version = phaser.awaitAdvanceInterruptibly(version, 10, 
TimeUnit.SECONDS);
-
-      // let's test the plugin
-      TestDistribPackageStore.assertResponseValues(
-          10,
-          () ->
-              new V2Request.Builder("/my-random-name/my/plugin")
-                  .forceV2(true)
-                  .GET()
-                  .build()
-                  .process(cluster.getSolrClient()),
-          ImmutableMap.of("/method.name", "m1"));
-
-      TestDistribPackageStore.assertResponseValues(
-          10,
-          () ->
-              new V2Request.Builder("/my-random-prefix/their/plugin")
-                  .forceV2(true)
-                  .GET()
-                  .build()
-                  .process(cluster.getSolrClient()),
-          ImmutableMap.of("/method.name", "m2"));
-      // now remove the plugin
-      new V2Request.Builder("/cluster/plugin")
-          .POST()
-          .forceV2(true)
-          .withPayload("{remove : my-random-name}")
-          .build()
-          .process(cluster.getSolrClient());
-
-      version = phaser.awaitAdvanceInterruptibly(version, 10, 
TimeUnit.SECONDS);
-
-      expectFail(
-          () ->
-              new V2Request.Builder("/my-random-prefix/their/plugin")
-                  .forceV2(true)
-                  .GET()
-                  .build()
-                  .process(cluster.getSolrClient()));
-      expectFail(
-          () ->
-              new V2Request.Builder("/my-random-prefix/their/plugin")
-                  .forceV2(true)
-                  .GET()
-                  .build()
-                  .process(cluster.getSolrClient()));
-
-      // test ClusterSingleton plugin
-      plugin.name = "clusterSingleton";
-      plugin.klass = C6.class.getName();
-      req.process(cluster.getSolrClient());
-      version = phaser.awaitAdvanceInterruptibly(version, 10, 
TimeUnit.SECONDS);
-
-      // just check if the plugin is indeed registered
-      readPluginState = new 
V2Request.Builder("/cluster/plugin").forceV2(true).GET().build();
-      rsp = readPluginState.process(cluster.getSolrClient());
-      assertEquals(C6.class.getName(), 
rsp._getStr("/plugin/clusterSingleton/class", null));
-
-      assertTrue("ccProvided", C6.ccProvided);
-      assertTrue("startCalled", C6.startCalled);
-      assertFalse("stopCalled", C6.stopCalled);
-
-      assertEquals(CConfig.class, ContainerPluginsRegistry.getConfigClass(new 
CC()));
-      assertEquals(CConfig.class, ContainerPluginsRegistry.getConfigClass(new 
CC1()));
-      assertEquals(CConfig.class, ContainerPluginsRegistry.getConfigClass(new 
CC2()));
-
-      CConfig cfg = new CConfig();
-      cfg.boolVal = Boolean.TRUE;
-      cfg.strVal = "Something";
-      cfg.longVal = 1234L;
-      PluginMeta p = new PluginMeta();
-      p.name = "hello";
-      p.klass = CC.class.getName();
-      p.config = cfg;
-
-      new V2Request.Builder("/cluster/plugin")
-          .forceV2(true)
-          .POST()
-          .withPayload(singletonMap("add", p))
-          .build()
-          .process(cluster.getSolrClient());
-
-      version = phaser.awaitAdvanceInterruptibly(version, 10, 
TimeUnit.SECONDS);
-
-      TestDistribPackageStore.assertResponseValues(
-          10,
-          () ->
-              new V2Request.Builder("hello/plugin")
-                  .forceV2(true)
-                  .GET()
-                  .build()
-                  .process(cluster.getSolrClient()),
-          ImmutableMap.of(
-              "/config/boolVal", "true", "/config/strVal", "Something", 
"/config/longVal", "1234"));
-
-      cfg.strVal = "Something else";
-      new V2Request.Builder("/cluster/plugin")
-          .forceV2(true)
-          .POST()
-          .withPayload(singletonMap("update", p))
-          .build()
-          .process(cluster.getSolrClient());
-      version = phaser.awaitAdvanceInterruptibly(version, 10, 
TimeUnit.SECONDS);
-
-      TestDistribPackageStore.assertResponseValues(
-          10,
-          () ->
-              new V2Request.Builder("hello/plugin")
-                  .forceV2(true)
-                  .GET()
-                  .build()
-                  .process(cluster.getSolrClient()),
-          ImmutableMap.of(
-              "/config/boolVal", "true", "/config/strVal", cfg.strVal, 
"/config/longVal", "1234"));
-
-      // kill the Overseer leader
-      for (JettySolrRunner jetty : cluster.getJettySolrRunners()) {
-        if 
(!jetty.getCoreContainer().getZkController().getOverseer().isClosed()) {
-          cluster.stopJettySolrRunner(jetty);
-          cluster.waitForJettyToStop(jetty);
-        }
-      }
-      assertTrue("stopCalled", C6.stopCalled);
-    } finally {
-      cluster.shutdown();
+      expectError(addPlugin, "path must not have a prefix: collections");
+      assertEquals(1, errors.getCount());
     }
-  }
 
-  private void expectFail(ThrowingRunnable runnable) throws Exception {
-    for (int i = 0; i < 20; i++) {
-      try {
-        runnable.run();
-      } catch (Throwable throwable) {
-        return;
+    plugin.name = "my-random-name";
+    plugin.pathPrefix = "my-random-prefix";
+
+    addPlugin.process(cluster.getSolrClient());
+    version = phaser.awaitAdvanceInterruptibly(version, 10, TimeUnit.SECONDS);
+
+    // let's test the plugin
+    TestDistribPackageStore.assertResponseValues(
+        getPlugin("/my-random-name/my/plugin"), Map.of("/method.name", "m1"));
+
+    TestDistribPackageStore.assertResponseValues(
+        getPlugin("/my-random-prefix/their/plugin"), Map.of("/method.name", 
"m2"));
+    // now remove the plugin
+    new V2Request.Builder("/cluster/plugin")
+        .POST()
+        .forceV2(true)
+        .withPayload("{remove : my-random-name}")
+        .build()
+        .process(cluster.getSolrClient());
+
+    version = phaser.awaitAdvanceInterruptibly(version, 10, TimeUnit.SECONDS);
+
+    try (ErrorLogMuter errors = 
ErrorLogMuter.substring("/my-random-prefix/their/plugin")) {
+      RemoteExecutionException e =
+          assertThrows(
+              RemoteExecutionException.class,
+              () -> getPlugin("/my-random-prefix/their/plugin").call());
+      assertEquals(404, e.code());
+      assertThat(
+          e.getMetaData().findRecursive("error", "msg").toString(),
+          containsString("no core retrieved"));
+      // V2HttpCall will separately log the path and stack trace, probably 
could be fixed
+      assertEquals(2, errors.getCount());
+    }
+
+    // test ClusterSingleton plugin
+    plugin.name = "clusterSingleton";
+    plugin.klass = C6.class.getName();
+    addPlugin.process(cluster.getSolrClient());
+    version = phaser.awaitAdvanceInterruptibly(version, 10, TimeUnit.SECONDS);
+
+    // just check if the plugin is indeed registered
+    rsp = readPluginState.call();
+    assertEquals(C6.class.getName(), 
rsp._getStr("/plugin/clusterSingleton/class", null));
+
+    assertTrue("ccProvided", C6.ccProvided);
+    assertTrue("startCalled", C6.startCalled);
+    assertFalse("stopCalled", C6.stopCalled);
+
+    assertEquals(CConfig.class, ContainerPluginsRegistry.getConfigClass(new 
CC()));
+    assertEquals(CConfig.class, ContainerPluginsRegistry.getConfigClass(new 
CC1()));
+    assertEquals(CConfig.class, ContainerPluginsRegistry.getConfigClass(new 
CC2()));
+
+    CConfig cfg = new CConfig();
+    cfg.boolVal = Boolean.TRUE;
+    cfg.strVal = "Something";
+    cfg.longVal = 1234L;
+    PluginMeta p = new PluginMeta();
+    p.name = "hello";
+    p.klass = CC.class.getName();
+    p.config = cfg;
+
+    new V2Request.Builder("/cluster/plugin")
+        .forceV2(true)
+        .POST()
+        .withPayload(singletonMap("add", p))
+        .build()
+        .process(cluster.getSolrClient());
+
+    version = phaser.awaitAdvanceInterruptibly(version, 10, TimeUnit.SECONDS);
+
+    TestDistribPackageStore.assertResponseValues(
+        getPlugin("hello/plugin"),
+        Map.of(
+            "/config/boolVal", "true", "/config/strVal", "Something", 
"/config/longVal", "1234"));
+
+    cfg.strVal = "Something else";
+    new V2Request.Builder("/cluster/plugin")
+        .forceV2(true)
+        .POST()
+        .withPayload(singletonMap("update", p))
+        .build()
+        .process(cluster.getSolrClient());
+    version = phaser.awaitAdvanceInterruptibly(version, 10, TimeUnit.SECONDS);
+
+    TestDistribPackageStore.assertResponseValues(
+        getPlugin("hello/plugin"),
+        Map.of("/config/boolVal", "true", "/config/strVal", cfg.strVal, 
"/config/longVal", "1234"));
+
+    // kill the Overseer leader
+    for (JettySolrRunner jetty : cluster.getJettySolrRunners()) {
+      if 
(!jetty.getCoreContainer().getZkController().getOverseer().isClosed()) {
+        cluster.stopJettySolrRunner(jetty);
+        cluster.waitForJettyToStop(jetty);
       }
-      Thread.sleep(100);
     }
-    fail("should have failed with an exception");
+    assertTrue("stopCalled", C6.stopCalled);
   }
 
   @Test
   public void testApiFromPackage() throws Exception {
-    MiniSolrCloudCluster cluster =
-        configureCluster(4).withJettyConfig(jetty -> 
jetty.enableV2(true)).configure();
     String FILE1 = "/myplugin/v1.jar";
     String FILE2 = "/myplugin/v2.jar";
-    ContainerPluginsRegistry pluginsRegistry =
-        
cluster.getOpenOverseer().getCoreContainer().getContainerPluginsRegistry();
-    pluginsRegistry.setPhaser(phaser);
 
     int version = phaser.getPhase();
 
-    String errPath = "/error/details[0]/errorMessages[0]";
-    try {
-      byte[] derFile = readFile("cryptokeys/pub_key512.der");
-      uploadKey(derFile, PackageStoreAPI.KEYS_DIR + "/pub_key512.der", 
cluster);
-      TestPackages.postFileAndWait(
-          cluster,
-          "runtimecode/containerplugin.v.1.jar.bin",
-          FILE1,
-          
"pmrmWCDafdNpYle2rueAGnU2J6NYlcAey9mkZYbqh+5RdYo2Ln+llLF9voyRj+DDivK9GV1XdtKvD9rgCxlD7Q==");
-      TestPackages.postFileAndWait(
-          cluster,
-          "runtimecode/containerplugin.v.2.jar.bin",
-          FILE2,
-          
"StR3DmqaUSL7qjDOeVEiCqE+ouiZAkW99fsL48F9oWG047o7NGgwwZ36iGgzDC3S2tPaFjRAd9Zg4UK7OZLQzg==");
-
-      // We have two versions of the plugin in 2 different jar files. they are 
already uploaded to
-      // the package store
-      Package.AddVersion add = new Package.AddVersion();
-      add.version = "1.0";
-      add.pkg = "mypkg";
-      add.files = singletonList(FILE1);
-      V2Request addPkgVersionReq =
-          new V2Request.Builder("/cluster/package")
-              .forceV2(true)
-              .POST()
-              .withPayload(singletonMap("add", add))
-              .build();
-      addPkgVersionReq.process(cluster.getSolrClient());
-
-      waitForAllNodesToSync(
-          cluster,
-          "/cluster/package",
-          Map.of(
-              ":result:packages:mypkg[0]:version",
-              "1.0",
-              ":result:packages:mypkg[0]:files[0]",
-              FILE1));
-
-      // Now lets create a plugin using v1 jar file
-      PluginMeta plugin = new PluginMeta();
-      plugin.name = "myplugin";
-      plugin.klass = "mypkg:org.apache.solr.handler.MyPlugin";
-      plugin.version = add.version;
-      final V2Request req1 =
-          new V2Request.Builder("/cluster/plugin")
-              .forceV2(true)
-              .POST()
-              .withPayload(singletonMap("add", plugin))
-              .build();
-      req1.process(cluster.getSolrClient());
-      version = phaser.awaitAdvanceInterruptibly(version, 10, 
TimeUnit.SECONDS);
-
-      // verify the plugin creation
-      TestDistribPackageStore.assertResponseValues(
-          10,
-          () ->
-              new V2Request.Builder("/cluster/plugin")
-                  .GET()
-                  .build()
-                  .process(cluster.getSolrClient()),
-          ImmutableMap.of(
-              "/plugin/myplugin/class", plugin.klass,
-              "/plugin/myplugin/version", plugin.version));
-      // let's test this now
-      Callable<NavigableObject> invokePlugin =
-          () ->
-              new V2Request.Builder("/plugin/my/path")
-                  .forceV2(true)

Review comment:
       1/3 Here's an example of where before the change to the `getPlugin` call 
the `forceV2(true)` was used.

##########
File path: solr/core/src/test/org/apache/solr/handler/TestContainerPlugin.java
##########
@@ -69,353 +69,274 @@
   private Phaser phaser;
 
   @Before
-  public void setup() {
+  public void setup() throws Exception {
     System.setProperty("enable.packages", "true");
     phaser = new Phaser();
+
+    int nodes = TEST_NIGHTLY ? 4 : 2;
+    cluster = configureCluster(nodes).withJettyConfig(jetty -> 
jetty.enableV2(true)).configure();
+    
cluster.getOpenOverseer().getCoreContainer().getContainerPluginsRegistry().setPhaser(phaser);
   }
 
   @After
-  public void teardown() {
+  public void teardown() throws Exception {
+    shutdownCluster();
     System.clearProperty("enable.packages");
   }
 
   @Test
   public void testApi() throws Exception {
-    MiniSolrCloudCluster cluster =
-        configureCluster(4).withJettyConfig(jetty -> 
jetty.enableV2(true)).configure();
-    ContainerPluginsRegistry pluginsRegistry =
-        
cluster.getOpenOverseer().getCoreContainer().getContainerPluginsRegistry();
-    pluginsRegistry.setPhaser(phaser);
-
     int version = phaser.getPhase();
 
-    String errPath = "/error/details[0]/errorMessages[0]";
-    try {
-      PluginMeta plugin = new PluginMeta();
+    PluginMeta plugin = new PluginMeta();
+    V2Request addPlugin =
+        new V2Request.Builder("/cluster/plugin")
+            .forceV2(true)
+            .POST()
+            .withPayload(singletonMap("add", plugin))
+            .build();
+
+    // test with an invalid class
+    try (ErrorLogMuter errors = 
ErrorLogMuter.substring("TestContainerPlugin$C2")) {
       plugin.name = "testplugin";
       plugin.klass = C2.class.getName();
-      // test with an invalid class
-      V2Request req =
-          new V2Request.Builder("/cluster/plugin")
-              .forceV2(true)
-              .POST()
-              .withPayload(singletonMap("add", plugin))
-              .build();
-      expectError(req, cluster.getSolrClient(), errPath, "No method with 
@Command in class");
-
-      // test with a valid class. This should succeed now
-      plugin.klass = C3.class.getName();
-      req.process(cluster.getSolrClient());
-
-      version = phaser.awaitAdvanceInterruptibly(version, 10, 
TimeUnit.SECONDS);
-
-      // just check if the plugin is indeed registered
-      V2Request readPluginState =
-          new V2Request.Builder("/cluster/plugin").forceV2(true).GET().build();
-      V2Response rsp = readPluginState.process(cluster.getSolrClient());
-      assertEquals(C3.class.getName(), rsp._getStr("/plugin/testplugin/class", 
null));
-
-      // let's test the plugin
-      TestDistribPackageStore.assertResponseValues(
-          10,
-          () ->
-              new V2Request.Builder("/plugin/my/plugin")
-                  .forceV2(true)
-                  .GET()
-                  .build()
-                  .process(cluster.getSolrClient()),
-          ImmutableMap.of("/testkey", "testval"));
-
-      // now remove the plugin
-      new V2Request.Builder("/cluster/plugin")
-          .POST()
-          .forceV2(true)
-          .withPayload("{remove : testplugin}")
-          .build()
-          .process(cluster.getSolrClient());
-
-      version = phaser.awaitAdvanceInterruptibly(version, 10, 
TimeUnit.SECONDS);
-
-      // verify it is removed
-      rsp = readPluginState.process(cluster.getSolrClient());
-      assertEquals(null, rsp._get("/plugin/testplugin/class", null));
+      expectError(addPlugin, "No method with @Command in class");
+      assertEquals(1, errors.getCount());
+    }
+
+    // test with a valid class. This should succeed now
+    plugin.klass = C3.class.getName();
+    addPlugin.process(cluster.getSolrClient());
+
+    version = phaser.awaitAdvanceInterruptibly(version, 10, TimeUnit.SECONDS);
 
+    // just check if the plugin is indeed registered
+    Callable<V2Response> readPluginState = getPlugin("/cluster/plugin");
+    V2Response rsp = readPluginState.call();
+    assertEquals(C3.class.getName(), rsp._getStr("/plugin/testplugin/class", 
null));
+
+    // let's test the plugin
+    TestDistribPackageStore.assertResponseValues(
+        getPlugin("/plugin/my/plugin"), Map.of("/testkey", "testval"));
+
+    // now remove the plugin
+    new V2Request.Builder("/cluster/plugin")
+        .POST()
+        .forceV2(true)
+        .withPayload("{remove : testplugin}")
+        .build()
+        .process(cluster.getSolrClient());
+
+    version = phaser.awaitAdvanceInterruptibly(version, 10, TimeUnit.SECONDS);
+
+    // verify it is removed
+    rsp = readPluginState.call();
+    assertNull(rsp._get("/plugin/testplugin/class", null));
+
+    try (ErrorLogMuter errors = 
ErrorLogMuter.substring("TestContainerPlugin$C4")) {
       // test with a class  @EndPoint methods. This also uses a template in 
the path name
       plugin.klass = C4.class.getName();
       plugin.name = "collections";
       plugin.pathPrefix = "collections";
-      expectError(
-          req, cluster.getSolrClient(), errPath, "path must not have a prefix: 
collections");
-
-      plugin.name = "my-random-name";
-      plugin.pathPrefix = "my-random-prefix";
-
-      req.process(cluster.getSolrClient());
-      version = phaser.awaitAdvanceInterruptibly(version, 10, 
TimeUnit.SECONDS);
-
-      // let's test the plugin
-      TestDistribPackageStore.assertResponseValues(
-          10,
-          () ->
-              new V2Request.Builder("/my-random-name/my/plugin")
-                  .forceV2(true)
-                  .GET()
-                  .build()
-                  .process(cluster.getSolrClient()),
-          ImmutableMap.of("/method.name", "m1"));
-
-      TestDistribPackageStore.assertResponseValues(
-          10,
-          () ->
-              new V2Request.Builder("/my-random-prefix/their/plugin")
-                  .forceV2(true)
-                  .GET()
-                  .build()
-                  .process(cluster.getSolrClient()),
-          ImmutableMap.of("/method.name", "m2"));
-      // now remove the plugin
-      new V2Request.Builder("/cluster/plugin")
-          .POST()
-          .forceV2(true)
-          .withPayload("{remove : my-random-name}")
-          .build()
-          .process(cluster.getSolrClient());
-
-      version = phaser.awaitAdvanceInterruptibly(version, 10, 
TimeUnit.SECONDS);
-
-      expectFail(
-          () ->
-              new V2Request.Builder("/my-random-prefix/their/plugin")
-                  .forceV2(true)
-                  .GET()
-                  .build()
-                  .process(cluster.getSolrClient()));
-      expectFail(
-          () ->
-              new V2Request.Builder("/my-random-prefix/their/plugin")
-                  .forceV2(true)
-                  .GET()
-                  .build()
-                  .process(cluster.getSolrClient()));
-
-      // test ClusterSingleton plugin
-      plugin.name = "clusterSingleton";
-      plugin.klass = C6.class.getName();
-      req.process(cluster.getSolrClient());
-      version = phaser.awaitAdvanceInterruptibly(version, 10, 
TimeUnit.SECONDS);
-
-      // just check if the plugin is indeed registered
-      readPluginState = new 
V2Request.Builder("/cluster/plugin").forceV2(true).GET().build();
-      rsp = readPluginState.process(cluster.getSolrClient());
-      assertEquals(C6.class.getName(), 
rsp._getStr("/plugin/clusterSingleton/class", null));
-
-      assertTrue("ccProvided", C6.ccProvided);
-      assertTrue("startCalled", C6.startCalled);
-      assertFalse("stopCalled", C6.stopCalled);
-
-      assertEquals(CConfig.class, ContainerPluginsRegistry.getConfigClass(new 
CC()));
-      assertEquals(CConfig.class, ContainerPluginsRegistry.getConfigClass(new 
CC1()));
-      assertEquals(CConfig.class, ContainerPluginsRegistry.getConfigClass(new 
CC2()));
-
-      CConfig cfg = new CConfig();
-      cfg.boolVal = Boolean.TRUE;
-      cfg.strVal = "Something";
-      cfg.longVal = 1234L;
-      PluginMeta p = new PluginMeta();
-      p.name = "hello";
-      p.klass = CC.class.getName();
-      p.config = cfg;
-
-      new V2Request.Builder("/cluster/plugin")
-          .forceV2(true)
-          .POST()
-          .withPayload(singletonMap("add", p))
-          .build()
-          .process(cluster.getSolrClient());
-
-      version = phaser.awaitAdvanceInterruptibly(version, 10, 
TimeUnit.SECONDS);
-
-      TestDistribPackageStore.assertResponseValues(
-          10,
-          () ->
-              new V2Request.Builder("hello/plugin")
-                  .forceV2(true)
-                  .GET()
-                  .build()
-                  .process(cluster.getSolrClient()),
-          ImmutableMap.of(
-              "/config/boolVal", "true", "/config/strVal", "Something", 
"/config/longVal", "1234"));
-
-      cfg.strVal = "Something else";
-      new V2Request.Builder("/cluster/plugin")
-          .forceV2(true)
-          .POST()
-          .withPayload(singletonMap("update", p))
-          .build()
-          .process(cluster.getSolrClient());
-      version = phaser.awaitAdvanceInterruptibly(version, 10, 
TimeUnit.SECONDS);
-
-      TestDistribPackageStore.assertResponseValues(
-          10,
-          () ->
-              new V2Request.Builder("hello/plugin")
-                  .forceV2(true)
-                  .GET()
-                  .build()
-                  .process(cluster.getSolrClient()),
-          ImmutableMap.of(
-              "/config/boolVal", "true", "/config/strVal", cfg.strVal, 
"/config/longVal", "1234"));
-
-      // kill the Overseer leader
-      for (JettySolrRunner jetty : cluster.getJettySolrRunners()) {
-        if 
(!jetty.getCoreContainer().getZkController().getOverseer().isClosed()) {
-          cluster.stopJettySolrRunner(jetty);
-          cluster.waitForJettyToStop(jetty);
-        }
-      }
-      assertTrue("stopCalled", C6.stopCalled);
-    } finally {
-      cluster.shutdown();
+      expectError(addPlugin, "path must not have a prefix: collections");
+      assertEquals(1, errors.getCount());
     }
-  }
 
-  private void expectFail(ThrowingRunnable runnable) throws Exception {
-    for (int i = 0; i < 20; i++) {
-      try {
-        runnable.run();
-      } catch (Throwable throwable) {
-        return;
+    plugin.name = "my-random-name";
+    plugin.pathPrefix = "my-random-prefix";
+
+    addPlugin.process(cluster.getSolrClient());
+    version = phaser.awaitAdvanceInterruptibly(version, 10, TimeUnit.SECONDS);
+
+    // let's test the plugin
+    TestDistribPackageStore.assertResponseValues(
+        getPlugin("/my-random-name/my/plugin"), Map.of("/method.name", "m1"));
+
+    TestDistribPackageStore.assertResponseValues(
+        getPlugin("/my-random-prefix/their/plugin"), Map.of("/method.name", 
"m2"));
+    // now remove the plugin
+    new V2Request.Builder("/cluster/plugin")
+        .POST()
+        .forceV2(true)
+        .withPayload("{remove : my-random-name}")
+        .build()
+        .process(cluster.getSolrClient());
+
+    version = phaser.awaitAdvanceInterruptibly(version, 10, TimeUnit.SECONDS);
+
+    try (ErrorLogMuter errors = 
ErrorLogMuter.substring("/my-random-prefix/their/plugin")) {
+      RemoteExecutionException e =
+          assertThrows(
+              RemoteExecutionException.class,
+              () -> getPlugin("/my-random-prefix/their/plugin").call());
+      assertEquals(404, e.code());
+      assertThat(
+          e.getMetaData().findRecursive("error", "msg").toString(),
+          containsString("no core retrieved"));
+      // V2HttpCall will separately log the path and stack trace, probably 
could be fixed
+      assertEquals(2, errors.getCount());
+    }
+
+    // test ClusterSingleton plugin
+    plugin.name = "clusterSingleton";
+    plugin.klass = C6.class.getName();
+    addPlugin.process(cluster.getSolrClient());
+    version = phaser.awaitAdvanceInterruptibly(version, 10, TimeUnit.SECONDS);
+
+    // just check if the plugin is indeed registered
+    rsp = readPluginState.call();
+    assertEquals(C6.class.getName(), 
rsp._getStr("/plugin/clusterSingleton/class", null));
+
+    assertTrue("ccProvided", C6.ccProvided);
+    assertTrue("startCalled", C6.startCalled);
+    assertFalse("stopCalled", C6.stopCalled);
+
+    assertEquals(CConfig.class, ContainerPluginsRegistry.getConfigClass(new 
CC()));
+    assertEquals(CConfig.class, ContainerPluginsRegistry.getConfigClass(new 
CC1()));
+    assertEquals(CConfig.class, ContainerPluginsRegistry.getConfigClass(new 
CC2()));
+
+    CConfig cfg = new CConfig();
+    cfg.boolVal = Boolean.TRUE;
+    cfg.strVal = "Something";
+    cfg.longVal = 1234L;
+    PluginMeta p = new PluginMeta();
+    p.name = "hello";
+    p.klass = CC.class.getName();
+    p.config = cfg;
+
+    new V2Request.Builder("/cluster/plugin")
+        .forceV2(true)
+        .POST()
+        .withPayload(singletonMap("add", p))
+        .build()
+        .process(cluster.getSolrClient());
+
+    version = phaser.awaitAdvanceInterruptibly(version, 10, TimeUnit.SECONDS);
+
+    TestDistribPackageStore.assertResponseValues(
+        getPlugin("hello/plugin"),
+        Map.of(
+            "/config/boolVal", "true", "/config/strVal", "Something", 
"/config/longVal", "1234"));
+
+    cfg.strVal = "Something else";
+    new V2Request.Builder("/cluster/plugin")
+        .forceV2(true)
+        .POST()
+        .withPayload(singletonMap("update", p))
+        .build()
+        .process(cluster.getSolrClient());
+    version = phaser.awaitAdvanceInterruptibly(version, 10, TimeUnit.SECONDS);
+
+    TestDistribPackageStore.assertResponseValues(
+        getPlugin("hello/plugin"),
+        Map.of("/config/boolVal", "true", "/config/strVal", cfg.strVal, 
"/config/longVal", "1234"));
+
+    // kill the Overseer leader
+    for (JettySolrRunner jetty : cluster.getJettySolrRunners()) {
+      if 
(!jetty.getCoreContainer().getZkController().getOverseer().isClosed()) {
+        cluster.stopJettySolrRunner(jetty);
+        cluster.waitForJettyToStop(jetty);
       }
-      Thread.sleep(100);
     }
-    fail("should have failed with an exception");
+    assertTrue("stopCalled", C6.stopCalled);
   }
 
   @Test
   public void testApiFromPackage() throws Exception {
-    MiniSolrCloudCluster cluster =
-        configureCluster(4).withJettyConfig(jetty -> 
jetty.enableV2(true)).configure();
     String FILE1 = "/myplugin/v1.jar";
     String FILE2 = "/myplugin/v2.jar";
-    ContainerPluginsRegistry pluginsRegistry =
-        
cluster.getOpenOverseer().getCoreContainer().getContainerPluginsRegistry();
-    pluginsRegistry.setPhaser(phaser);
 
     int version = phaser.getPhase();
 
-    String errPath = "/error/details[0]/errorMessages[0]";
-    try {
-      byte[] derFile = readFile("cryptokeys/pub_key512.der");
-      uploadKey(derFile, PackageStoreAPI.KEYS_DIR + "/pub_key512.der", 
cluster);
-      TestPackages.postFileAndWait(
-          cluster,
-          "runtimecode/containerplugin.v.1.jar.bin",
-          FILE1,
-          
"pmrmWCDafdNpYle2rueAGnU2J6NYlcAey9mkZYbqh+5RdYo2Ln+llLF9voyRj+DDivK9GV1XdtKvD9rgCxlD7Q==");
-      TestPackages.postFileAndWait(
-          cluster,
-          "runtimecode/containerplugin.v.2.jar.bin",
-          FILE2,
-          
"StR3DmqaUSL7qjDOeVEiCqE+ouiZAkW99fsL48F9oWG047o7NGgwwZ36iGgzDC3S2tPaFjRAd9Zg4UK7OZLQzg==");
-
-      // We have two versions of the plugin in 2 different jar files. they are 
already uploaded to
-      // the package store
-      Package.AddVersion add = new Package.AddVersion();
-      add.version = "1.0";
-      add.pkg = "mypkg";
-      add.files = singletonList(FILE1);
-      V2Request addPkgVersionReq =
-          new V2Request.Builder("/cluster/package")
-              .forceV2(true)
-              .POST()
-              .withPayload(singletonMap("add", add))
-              .build();
-      addPkgVersionReq.process(cluster.getSolrClient());
-
-      waitForAllNodesToSync(
-          cluster,
-          "/cluster/package",
-          Map.of(
-              ":result:packages:mypkg[0]:version",
-              "1.0",
-              ":result:packages:mypkg[0]:files[0]",
-              FILE1));
-
-      // Now lets create a plugin using v1 jar file
-      PluginMeta plugin = new PluginMeta();
-      plugin.name = "myplugin";
-      plugin.klass = "mypkg:org.apache.solr.handler.MyPlugin";
-      plugin.version = add.version;
-      final V2Request req1 =
-          new V2Request.Builder("/cluster/plugin")
-              .forceV2(true)
-              .POST()
-              .withPayload(singletonMap("add", plugin))
-              .build();
-      req1.process(cluster.getSolrClient());
-      version = phaser.awaitAdvanceInterruptibly(version, 10, 
TimeUnit.SECONDS);
-
-      // verify the plugin creation
-      TestDistribPackageStore.assertResponseValues(
-          10,
-          () ->
-              new V2Request.Builder("/cluster/plugin")
-                  .GET()

Review comment:
       2/3 Here's an example of where before the change to the `getPlugin` call 
the `forceV2(true)` was _not_ used.




-- 
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: issues-unsubscr...@solr.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org
For additional commands, e-mail: issues-h...@solr.apache.org

Reply via email to