You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by no...@apache.org on 2020/05/31 09:54:23 UTC

[lucene-solr] 01/01: For components loaded from packages SolrCoreAware, ResourceLoaderAware are not honored

This is an automated email from the ASF dual-hosted git repository.

noble pushed a commit to branch jira/solr14525
in repository https://gitbox.apache.org/repos/asf/lucene-solr.git

commit 3ebe3861d079a70431dd165ce65f099cde156521
Author: noble <no...@apache.org>
AuthorDate: Sun May 31 19:53:33 2020 +1000

    For components loaded from packages SolrCoreAware, ResourceLoaderAware are not honored
---
 .../org/apache/solr/core/SolrResourceLoader.java   |  41 ++++--
 .../java/org/apache/solr/pkg/PackageLoader.java    |  31 ++++-
 .../org/apache/solr/pkg/PackagePluginHolder.java   |  18 ++-
 .../src/test/org/apache/solr/pkg/TestPackages.java | 146 +++++++++++++++------
 4 files changed, 178 insertions(+), 58 deletions(-)

diff --git a/solr/core/src/java/org/apache/solr/core/SolrResourceLoader.java b/solr/core/src/java/org/apache/solr/core/SolrResourceLoader.java
index afe07cd..991b1c7 100644
--- a/solr/core/src/java/org/apache/solr/core/SolrResourceLoader.java
+++ b/solr/core/src/java/org/apache/solr/core/SolrResourceLoader.java
@@ -538,7 +538,7 @@ public class SolrResourceLoader implements ResourceLoader, Closeable {
     Class<? extends T> clazz = findClass(cName, expectedType, subPackages);
     if (clazz == null) {
       throw new SolrException(SolrException.ErrorCode.SERVER_ERROR,
-          "Can not find class: " + cName + " in " + classLoader);
+              "Can not find class: " + cName + " in " + classLoader);
     }
 
     T obj = null;
@@ -566,25 +566,42 @@ public class SolrResourceLoader implements ResourceLoader, Closeable {
 
     } catch (Exception e) {
       throw new SolrException(SolrException.ErrorCode.SERVER_ERROR,
-          "Error instantiating class: '" + clazz.getName() + "'", e);
+              "Error instantiating class: '" + clazz.getName() + "'", e);
     }
 
-    if (!live) {
-      if (obj instanceof SolrCoreAware) {
-        assertAwareCompatibility(SolrCoreAware.class, obj);
-        waitingForCore.add((SolrCoreAware) obj);
+    addToCoreAware(obj);
+    addToResourceLoaderAware(obj);
+    if(!live) {
+      if (obj instanceof SolrInfoBean) {
+        //TODO: Assert here?
+        infoMBeans.add((SolrInfoBean) obj);
       }
+    }
+    return obj;
+  }
+
+  public <T> boolean addToResourceLoaderAware(T obj) {
+    if (!live) {
       if (obj instanceof ResourceLoaderAware) {
         assertAwareCompatibility(ResourceLoaderAware.class, obj);
         waitingForResources.add((ResourceLoaderAware) obj);
       }
-      if (obj instanceof SolrInfoBean) {
-        //TODO: Assert here?
-        infoMBeans.add((SolrInfoBean) obj);
-      }
+      return true;
+    } else {
+      return false;
     }
+  }
 
-    return obj;
+  public <T> boolean addToCoreAware(T obj) {
+    if (!live) {
+      if (obj instanceof SolrCoreAware) {
+        assertAwareCompatibility(SolrCoreAware.class, obj);
+        waitingForCore.add((SolrCoreAware) obj);
+      }
+      return true;
+    } else {
+      return false;
+    }
   }
 
 
@@ -713,7 +730,7 @@ public class SolrResourceLoader implements ResourceLoader, Closeable {
   /**
    * Utility function to throw an exception if the class is invalid
    */
-  static void assertAwareCompatibility(Class aware, Object obj) {
+  public static void assertAwareCompatibility(Class aware, Object obj) {
     Class[] valid = awareCompatibility.get(aware);
     if (valid == null) {
       throw new SolrException(SolrException.ErrorCode.SERVER_ERROR,
diff --git a/solr/core/src/java/org/apache/solr/pkg/PackageLoader.java b/solr/core/src/java/org/apache/solr/pkg/PackageLoader.java
index b38628e..6b230ac 100644
--- a/solr/core/src/java/org/apache/solr/pkg/PackageLoader.java
+++ b/solr/core/src/java/org/apache/solr/pkg/PackageLoader.java
@@ -34,11 +34,14 @@ import java.util.Set;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.CopyOnWriteArrayList;
 
+import org.apache.lucene.analysis.util.ResourceLoaderAware;
 import org.apache.solr.common.MapWriter;
+import org.apache.solr.common.SolrException;
 import org.apache.solr.common.cloud.ZkStateReader;
 import org.apache.solr.core.CoreContainer;
 import org.apache.solr.core.SolrCore;
 import org.apache.solr.core.SolrResourceLoader;
+import org.apache.solr.util.plugin.SolrCoreAware;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -269,7 +272,7 @@ public class PackageLoader implements Closeable {
           paths.add(coreContainer.getPackageStoreAPI().getPackageStore().getRealpath(file));
         }
 
-        loader = new SolrResourceLoader(
+        loader = new PackageResourceLoader(
             "PACKAGE_LOADER: " + parent.name() + ":" + version,
             paths,
             Paths.get(coreContainer.getSolrHome()),
@@ -301,6 +304,32 @@ public class PackageLoader implements Closeable {
       }
     }
   }
+  static class PackageResourceLoader extends SolrResourceLoader {
+
+    PackageResourceLoader(String name, List<Path> classpath, Path instanceDir, ClassLoader parent) {
+      super(name, classpath, instanceDir, parent);
+    }
+
+    @Override
+    public <T> boolean addToCoreAware(T obj) {
+      //do not do anything
+      //this class is
+      return false;
+    }
+
+    @Override
+    public <T> boolean addToResourceLoaderAware(T obj) {
+      if (obj instanceof ResourceLoaderAware) {
+        assertAwareCompatibility(ResourceLoaderAware.class, obj);
+        try {
+          ((ResourceLoaderAware) obj).inform(this);
+        } catch (IOException e) {
+          throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, e);
+        }
+      }
+     return true;
+    }
+  }
 
   private static String findBiggest(String lessThan, List<String> sortedList) {
     String latest = null;
diff --git a/solr/core/src/java/org/apache/solr/pkg/PackagePluginHolder.java b/solr/core/src/java/org/apache/solr/pkg/PackagePluginHolder.java
index cf2382d..852daf4 100644
--- a/solr/core/src/java/org/apache/solr/pkg/PackagePluginHolder.java
+++ b/solr/core/src/java/org/apache/solr/pkg/PackagePluginHolder.java
@@ -17,16 +17,13 @@
 
 package org.apache.solr.pkg;
 
-import java.lang.invoke.MethodHandles;
-
-import org.apache.solr.core.PluginBag;
-import org.apache.solr.core.PluginInfo;
-import org.apache.solr.core.RequestParams;
-import org.apache.solr.core.SolrConfig;
-import org.apache.solr.core.SolrCore;
+import org.apache.solr.core.*;
+import org.apache.solr.util.plugin.SolrCoreAware;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import java.lang.invoke.MethodHandles;
+
 public class PackagePluginHolder<T> extends PluginBag.PluginHolder<T> {
   private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
   public static final String LATEST = "$LATEST";
@@ -117,6 +114,13 @@ public class PackagePluginHolder<T> extends PluginBag.PluginHolder<T> {
     Object instance = SolrCore.createInstance(pluginInfo.className,
         pluginMeta.clazz, pluginMeta.getCleanTag(), core, newest.getLoader());
     PluginBag.initInstance(instance, pluginInfo);
+    if (instance instanceof SolrCoreAware) {
+      SolrCoreAware coreAware = (SolrCoreAware) instance;
+      if (!core.getResourceLoader().addToCoreAware(coreAware)) {
+        //returns false if the core was fully initialized
+        coreAware.inform(core);
+      }
+    }
     T old = inst;
     inst = (T) instance;
     if (old instanceof AutoCloseable) {
diff --git a/solr/core/src/test/org/apache/solr/pkg/TestPackages.java b/solr/core/src/test/org/apache/solr/pkg/TestPackages.java
index 55f3c1b..5a5476e 100644
--- a/solr/core/src/test/org/apache/solr/pkg/TestPackages.java
+++ b/solr/core/src/test/org/apache/solr/pkg/TestPackages.java
@@ -19,12 +19,12 @@ package org.apache.solr.pkg;
 
 import java.io.IOException;
 import java.nio.ByteBuffer;
-import java.util.Arrays;
-import java.util.Collections;
-import java.util.Map;
+import java.util.*;
 import java.util.concurrent.Callable;
 
 import org.apache.commons.codec.digest.DigestUtils;
+import org.apache.lucene.analysis.util.ResourceLoader;
+import org.apache.lucene.analysis.util.ResourceLoaderAware;
 import org.apache.solr.client.solrj.SolrClient;
 import org.apache.solr.client.solrj.SolrQuery;
 import org.apache.solr.client.solrj.SolrRequest;
@@ -40,19 +40,27 @@ import org.apache.solr.client.solrj.request.V2Request;
 import org.apache.solr.client.solrj.request.beans.Package;
 import org.apache.solr.client.solrj.response.QueryResponse;
 import org.apache.solr.client.solrj.util.ClientUtils;
-import org.apache.solr.cloud.ConfigRequest;
 import org.apache.solr.cloud.MiniSolrCloudCluster;
 import org.apache.solr.cloud.SolrCloudTestCase;
 import org.apache.solr.common.MapWriterMap;
 import org.apache.solr.common.NavigableObject;
 import org.apache.solr.common.SolrInputDocument;
+import org.apache.solr.common.annotation.JsonProperty;
 import org.apache.solr.common.params.MapSolrParams;
 import org.apache.solr.common.params.ModifiableSolrParams;
 import org.apache.solr.common.params.SolrParams;
+import org.apache.solr.common.util.ReflectMapWriter;
 import org.apache.solr.common.util.Utils;
+import org.apache.solr.core.SolrCore;
 import org.apache.solr.filestore.PackageStoreAPI;
 import org.apache.solr.filestore.TestDistribPackageStore;
+import org.apache.solr.handler.RequestHandlerBase;
+import org.apache.solr.request.SolrQueryRequest;
+import org.apache.solr.response.SolrQueryResponse;
+import org.apache.solr.search.QParser;
+import org.apache.solr.search.QParserPlugin;
 import org.apache.solr.util.LogLevel;
+import org.apache.solr.util.plugin.SolrCoreAware;
 import org.apache.zookeeper.data.Stat;
 import org.junit.After;
 import org.junit.Before;
@@ -80,7 +88,13 @@ public class TestPackages extends SolrCloudTestCase {
   public void teardown() {
     System.clearProperty("enable.packages");
   }
-  
+  public static class ConfigPlugin implements ReflectMapWriter {
+    @JsonProperty
+    public String name;
+
+    @JsonProperty("class")
+    public String klass;
+  }
   @Test
   public void testPluginLoading() throws Exception {
     MiniSolrCloudCluster cluster =
@@ -98,8 +112,6 @@ public class TestPackages extends SolrCloudTestCase {
       String COLLECTION_NAME = "testPluginLoadingColl";
       byte[] derFile = readFile("cryptokeys/pub_key512.der");
       uploadKey(derFile, PackageStoreAPI.KEYS_DIR+"/pub_key512.der", cluster);
-//      cluster.getZkClient().makePath("/keys/exe", true);
-//      cluster.getZkClient().create("/keys/exe/pub_key512.der", derFile, CreateMode.PERSISTENT, true);
       postFileAndWait(cluster, "runtimecode/runtimelibs.jar.bin", FILE1,
           "L3q/qIGs4NaF6JiO0ZkMUFa88j0OmYc+I6O7BOdNuMct/xoZ4h73aZHZGc0+nmI1f/U3bOlMPINlSOM6LK3JpQ==");
 
@@ -136,20 +148,39 @@ public class TestPackages extends SolrCloudTestCase {
               ":result:packages:mypkg[0]:version", "1.0",
               ":result:packages:mypkg[0]:files[0]", FILE1
           ));
-
-      String payload = "{\n" +
-          "'create-requesthandler' : { 'name' : '/runtime', 'class': 'mypkg:org.apache.solr.core.RuntimeLibReqHandler' }," +
-          "'create-searchcomponent' : { 'name' : 'get', 'class': 'mypkg:org.apache.solr.core.RuntimeLibSearchComponent'  }," +
-          "'create-queryResponseWriter' : { 'name' : 'json1', 'class': 'mypkg:org.apache.solr.core.RuntimeLibResponseWriter' }" +
-          "'create-updateProcessor' : { 'name' : 'myurp', 'class': 'mypkg:org.apache.solr.update.TestVersionedURP' }," +
-          " create-expressible: {name: mincopy , class: 'mypkg:org.apache.solr.client.solrj.io.stream.metrics.MinCopyMetric'}" +
-          "}";
-      cluster.getSolrClient().request(new ConfigRequest(payload) {
-        @Override
-        public String getCollection() {
-          return COLLECTION_NAME;
-        }
-      });
+      Map<String,ConfigPlugin> plugins = new LinkedHashMap<>();
+      ConfigPlugin p = new ConfigPlugin();
+      p.klass = "mypkg:org.apache.solr.core.RuntimeLibReqHandler";
+      p.name = "/runtime";
+      plugins.put("create-requesthandler", p);
+
+      p = new ConfigPlugin();
+      p.klass = "mypkg:org.apache.solr.core.RuntimeLibSearchComponent";
+      p.name = "get";
+      plugins.put("create-searchcomponent", p);
+
+      p = new ConfigPlugin();
+      p.klass = "mypkg:org.apache.solr.core.RuntimeLibResponseWriter";
+      p.name = "json1";
+      plugins.put("create-queryResponseWriter", p);
+
+      p = new ConfigPlugin();
+      p.klass = "mypkg:org.apache.solr.update.TestVersionedURP";
+      p.name = "myurp";
+      plugins.put("create-updateProcessor", p);
+
+      p = new ConfigPlugin();
+      p.klass = "mypkg:org.apache.solr.client.solrj.io.stream.metrics.MinCopyMetric";
+      p.name = "mincopy";
+      plugins.put("create-expressible", p);
+
+
+      V2Request v2r = new V2Request.Builder( "/c/"+COLLECTION_NAME+ "/config")
+              .withMethod(SolrRequest.METHOD.POST)
+              .withPayload(plugins)
+              .forceV2(true)
+              .build();
+      cluster.getSolrClient().request(v2r);
 
       verifyCmponent(cluster.getSolrClient(),
           COLLECTION_NAME, "queryResponseWriter", "json1",
@@ -373,6 +404,27 @@ public class TestPackages extends SolrCloudTestCase {
       verifyCmponent(cluster.getSolrClient(),
           COLLECTION_NAME, "requestHandler", "/runtime",
           "mypkg", "2.1" );
+
+      plugins.clear();
+      p = new ConfigPlugin();
+      p.name = "/rt_2";
+      p.klass = "mypkg:"+ C.class.getName();
+      plugins.put("create-requesthandler", p);
+
+      p = new ConfigPlugin();
+      p.name = "qp1";
+      p.klass = "mypkg:"+ C2.class.getName();
+      plugins.put("create-queryparser", p);
+
+      v2r = new V2Request.Builder( "/c/"+COLLECTION_NAME+ "/config")
+              .withMethod(SolrRequest.METHOD.POST)
+              .withPayload(plugins)
+              .forceV2(true)
+              .build();
+      cluster.getSolrClient().request(v2r);
+      assertTrue(C.informCalled);
+      assertTrue(C2.informCalled);
+
       //we create a new node. This node does not have the packages. But it should download it from another node
       JettySolrRunner jetty = cluster.startJettySolrRunner();
       //create a new replica for this collection. it should end up
@@ -390,21 +442,6 @@ public class TestPackages extends SolrCloudTestCase {
     }
 
   }
-   /* new V2Request.Builder("/c/"+COLLECTIONORALIAS+"/config").withMethod(SolrRequest.METHOD.POST)
-        .withPayload("{add-expressible: {name: mincopy , class: org.apache.solr.client.solrj.io.stream.metrics.MinCopyMetric}}")
-    .build().process(cluster.getSolrClient());
-
-  ModifiableSolrParams _params = new ModifiableSolrParams();
-  QueryRequest query = new QueryRequest(new MapSolrParams("action","plugins", "collection", COLLECTIONORALIAS, "wt", "javabin"));
-    query.setPath("/stream");
-  NamedList<Object> rsp = cluster.getSolrClient().request(query);
-  assertEquals("org.apache.solr.client.solrj.io.stream.metrics.MinCopyMetric", rsp._getStr("/plugins/mincopy", null));
-  _params = new ModifiableSolrParams();
-  query = new QueryRequest(new MapSolrParams("componentName","mincopy", "meta" ,"true", "collection", COLLECTIONORALIAS, "wt", "javabin"));
-    query.setPath("/config/expressible");
-  rsp = cluster.getSolrClient().request(query);
-
-    System.out.println();*/
 
   private void executeReq(String uri, JettySolrRunner jetty, Utils.InputStreamConsumer parser, Map expected) throws Exception {
     try(HttpSolrClient client = (HttpSolrClient) jetty.newClient()){
@@ -480,13 +517,11 @@ public class TestPackages extends SolrCloudTestCase {
           "L3q/qIGs4NaF6JiO0ZkMUFa88j0OmYc+I6O7BOdNuMct/xoZ4h73aZHZGc0+nmI1f/U3bOlMPINlSOM6LK3JpQ==");
       // with correct signature
       //after uploading the file, let's delete the keys to see if we get proper error message
-//      cluster.getZkClient().delete("/keys/exe/pub_key512.der", -1, true);
       add.files = Arrays.asList(new String[]{FILE2});
       /*expectError(req, cluster.getSolrClient(), errPath,
           "ZooKeeper does not have any public keys");*/
 
       //Now lets' put the keys back
-//      cluster.getZkClient().create("/keys/exe/pub_key512.der", derFile, CreateMode.PERSISTENT, true);
 
       //this time we have a file with proper signature, public keys are in ZK
       // so the add {} command should succeed
@@ -567,6 +602,41 @@ public class TestPackages extends SolrCloudTestCase {
       cluster.shutdown();
     }
   }
+  public static class C extends RequestHandlerBase implements SolrCoreAware   {
+    static boolean informCalled = false;
+
+    @Override
+    public void inform(SolrCore core) {
+      informCalled = true;
+
+    }
+
+    @Override
+    public void handleRequestBody(SolrQueryRequest req, SolrQueryResponse rsp) {
+
+    }
+
+    @Override
+    public String getDescription() {
+      return "test";
+    }
+  }
+
+  public static class C2 extends QParserPlugin implements ResourceLoaderAware {
+    static boolean informCalled = false;
+
+
+    @Override
+    public void inform(ResourceLoader loader) throws IOException {
+      informCalled = true;
+
+    }
+
+    @Override
+    public QParser createParser(String qstr, SolrParams localParams, SolrParams params, SolrQueryRequest req) {
+      return null;
+    }
+  }
 
   static void postFileAndWait(MiniSolrCloudCluster cluster, String fname, String path, String sig) throws Exception {
     ByteBuffer fileContent = getFileContent(fname);