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