From 2e60e1fbbd3f72526b2975398f3943c398637c21 Mon Sep 17 00:00:00 2001
From: noblepaul <noble.paul@gmail.com>
Date: Thu, 2 Jul 2020 15:33:02 +1000
Subject: [PATCH] SOLR-14404: Unregister was not working for plugins with
 $path-prefix

---
 .../solr/api/CustomContainerPlugins.java      | 19 ++++++++++--
 .../solr/handler/TestContainerPlugin.java     | 30 +++++++++++++++++++
 2 files changed, 46 insertions(+), 3 deletions(-)

diff --git a/solr/core/src/java/org/apache/solr/api/CustomContainerPlugins.java b/solr/core/src/java/org/apache/solr/api/CustomContainerPlugins.java
index 6e2e0fbc7c1..fe537d9e516 100644
--- a/solr/core/src/java/org/apache/solr/api/CustomContainerPlugins.java
+++ b/solr/core/src/java/org/apache/solr/api/CustomContainerPlugins.java
@@ -32,6 +32,7 @@ import com.fasterxml.jackson.databind.ObjectMapper;
 import org.apache.lucene.analysis.util.ResourceLoaderAware;
 import org.apache.solr.client.solrj.SolrRequest;
 import org.apache.solr.client.solrj.request.beans.PluginMeta;
+import org.apache.solr.common.MapWriter;
 import org.apache.solr.common.SolrException;
 import org.apache.solr.common.annotation.JsonProperty;
 import org.apache.solr.common.cloud.ClusterPropertiesListener;
@@ -52,7 +53,7 @@ import org.slf4j.LoggerFactory;
 import static org.apache.lucene.util.IOUtils.closeWhileHandlingException;
 import static org.apache.solr.common.util.Utils.makeMap;
 
-public class CustomContainerPlugins implements ClusterPropertiesListener {
+public class CustomContainerPlugins implements ClusterPropertiesListener, MapWriter {
   private final ObjectMapper mapper = SolrJacksonAnnotationInspector.createObjectMapper();
   private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
 
@@ -71,6 +72,11 @@ public class CustomContainerPlugins implements ClusterPropertiesListener {
     this.containerApiBag = apiBag;
   }
 
+  @Override
+  public void writeMap(EntryWriter ew) throws IOException {
+    currentPlugins.forEach(ew.getBiConsumer());
+  }
+
   public synchronized void refresh() {
     Map<String, Object> pluginInfos = null;
     try {
@@ -101,7 +107,8 @@ public class CustomContainerPlugins implements ClusterPropertiesListener {
         ApiInfo apiInfo = currentPlugins.remove(e.getKey());
         if (apiInfo == null) continue;
         for (ApiHolder holder : apiInfo.holders) {
-          Api old = containerApiBag.unregister(holder.api.getEndPoint().method()[0], holder.api.getEndPoint().path()[0]);
+          Api old = containerApiBag.unregister(holder.api.getEndPoint().method()[0],
+              getActualPath(apiInfo, holder.api.getEndPoint().path()[0]));
           if (old instanceof Closeable) {
             closeWhileHandlingException((Closeable) old);
           }
@@ -141,7 +148,7 @@ public class CustomContainerPlugins implements ClusterPropertiesListener {
           if (old != null) {
             for (ApiHolder holder : old.holders) {
               if (replaced.contains(holder)) continue;// this path is present in the new one as well. so it already got replaced
-              containerApiBag.unregister(holder.getMethod(), holder.getPath());
+              containerApiBag.unregister(holder.getMethod(),getActualPath(old, holder.getPath()));
             }
             if (old instanceof Closeable) {
               closeWhileHandlingException((Closeable) old);
@@ -153,6 +160,12 @@ public class CustomContainerPlugins implements ClusterPropertiesListener {
     }
   }
 
+  private static String getActualPath(ApiInfo apiInfo, String path) {
+    path = path.replaceAll("\\$path-prefix", apiInfo.info.pathPrefix);
+    path = path.replaceAll("\\$plugin-name", apiInfo.info.name);
+    return path;
+  }
+
   @SuppressWarnings({"rawtypes", "unchecked"})
   private static  Map<String, String> getTemplateVars(PluginMeta pluginMeta) {
     Map result = makeMap("plugin-name", pluginMeta.name, "path-prefix", pluginMeta.pathPrefix);
diff --git a/solr/core/src/test/org/apache/solr/handler/TestContainerPlugin.java b/solr/core/src/test/org/apache/solr/handler/TestContainerPlugin.java
index d137b102c43..9bfe22e5b33 100644
--- a/solr/core/src/test/org/apache/solr/handler/TestContainerPlugin.java
+++ b/solr/core/src/test/org/apache/solr/handler/TestContainerPlugin.java
@@ -20,6 +20,7 @@ package org.apache.solr.handler;
 import java.io.IOException;
 import java.io.InputStream;
 import java.nio.ByteBuffer;
+import java.util.Collections;
 import java.util.Map;
 import java.util.concurrent.Callable;
 
@@ -152,11 +153,40 @@ public class TestContainerPlugin extends SolrCloudTestCase {
               .withMethod(GET)
               .build().process(cluster.getSolrClient()),
           ImmutableMap.of("/method.name", "m2"));
+      //now remove the plugin
+      new V2Request.Builder("/cluster/plugin")
+          .withMethod(POST)
+          .forceV2(true)
+          .withPayload("{remove : my-random-name}")
+          .build()
+          .process(cluster.getSolrClient());
 
+      expectFail( () -> new V2Request.Builder("/my-random-prefix/their/plugin")
+          .forceV2(true)
+          .withMethod(GET)
+          .build()
+          .process(cluster.getSolrClient()));
+      expectFail(() -> new V2Request.Builder("/my-random-prefix/their/plugin")
+          .forceV2(true)
+          .withMethod(GET)
+          .build()
+          .process(cluster.getSolrClient()));
     } finally {
       cluster.shutdown();
     }
   }
+
+  private void expectFail(ThrowingRunnable runnable) throws Exception {
+    for(int i=0;i< 20;i++) {
+      try {
+        runnable.run();
+      } catch (Throwable throwable) {
+        return;
+      }
+      Thread.sleep(100);
+    }
+    fail("should have failed with an exception");
+  }
   @Test
   public void testApiFromPackage() throws Exception {
     MiniSolrCloudCluster cluster =