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/06/02 00:18:53 UTC

[lucene-solr] branch branch_8x updated: SOLR-14525: SolrCoreAware, ResourceLoaderAware should be honored for plugin loaded from packages

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

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


The following commit(s) were added to refs/heads/branch_8x by this push:
     new e0b7984  SOLR-14525: SolrCoreAware, ResourceLoaderAware should be honored for plugin loaded from packages
e0b7984 is described below

commit e0b7984b140c4ecc9f435a22fd557fbcea30b171
Author: Noble Paul <no...@users.noreply.github.com>
AuthorDate: Tue Jun 2 10:01:39 2020 +1000

    SOLR-14525: SolrCoreAware, ResourceLoaderAware should be honored for plugin loaded from packages
---
 solr/CHANGES.txt                                   |   2 +
 .../org/apache/solr/core/SolrResourceLoader.java   |  63 ++++++---
 .../java/org/apache/solr/pkg/PackageLoader.java    |  41 ++++--
 .../org/apache/solr/pkg/PackagePluginHolder.java   |  34 ++++-
 .../src/test/org/apache/solr/pkg/TestPackages.java | 141 ++++++++++++++++-----
 5 files changed, 213 insertions(+), 68 deletions(-)

diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index 0aef16a..f975e08 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -134,6 +134,8 @@ Bug Fixes
 
 * SOLR-14491: Intercepting internode requests in KerberosPlugin when HTTP/2 client is used (Ishan Chattopadhyaya, Moshe Bla)
 
+* SOLR-14525: SolrCoreAware, ResourceLoaderAware should be honored for plugin loaded from packages (noble)
+
 
 Other Changes
 ---------------------
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 5a3faf1..3c6318c 100644
--- a/solr/core/src/java/org/apache/solr/core/SolrResourceLoader.java
+++ b/solr/core/src/java/org/apache/solr/core/SolrResourceLoader.java
@@ -16,12 +16,7 @@
  */
 package org.apache.solr.core;
 
-import java.io.Closeable;
-import java.io.File;
-import java.io.FileOutputStream;
-import java.io.IOException;
-import java.io.InputStream;
-import java.io.OutputStream;
+import java.io.*;
 import java.lang.invoke.MethodHandles;
 import java.lang.reflect.Constructor;
 import java.net.MalformedURLException;
@@ -34,24 +29,13 @@ import java.nio.file.DirectoryStream;
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.nio.file.PathMatcher;
-import java.util.ArrayList;
-import java.util.Arrays;
-import java.util.Collections;
-import java.util.HashMap;
-import java.util.List;
-import java.util.Map;
-import java.util.Properties;
+import java.util.*;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 import java.util.stream.Collectors;
-
 import org.apache.lucene.analysis.WordlistLoader;
-import org.apache.lucene.analysis.util.CharFilterFactory;
-import org.apache.lucene.analysis.util.ResourceLoader;
-import org.apache.lucene.analysis.util.ResourceLoaderAware;
-import org.apache.lucene.analysis.util.TokenFilterFactory;
-import org.apache.lucene.analysis.util.TokenizerFactory;
+import org.apache.lucene.analysis.util.*;
 import org.apache.lucene.codecs.Codec;
 import org.apache.lucene.codecs.DocValuesFormat;
 import org.apache.lucene.codecs.PostingsFormat;
@@ -622,9 +606,48 @@ public class SolrResourceLoader implements ResourceLoader,Closeable
       }
     }
 
+    addToCoreAware(obj);
+    addToResourceLoaderAware(obj);
+    addToInfoBeans(obj);
     return obj;
   }
 
+  public <T> void addToInfoBeans(T obj) {
+    if(!live) {
+      if (obj instanceof SolrInfoBean) {
+        infoMBeans.add((SolrInfoBean) obj);
+      }
+    }
+  }
+
+  public <T> boolean addToResourceLoaderAware(T obj) {
+    if (!live) {
+      if (obj instanceof ResourceLoaderAware) {
+        assertAwareCompatibility(ResourceLoaderAware.class, obj);
+        waitingForResources.add((ResourceLoaderAware) obj);
+      }
+      return true;
+    } else {
+      return false;
+    }
+  }
+
+  /** the inform() callback should be invoked on the listener.
+   * If this is 'live', the callback is not called so currently this returns 'false'
+   *
+   */
+  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;
+    }
+  }
+
 
   /**
    * Tell all {@link SolrCoreAware} instances about the SolrCore
@@ -760,7 +783,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 ) {
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..7dcc968 100644
--- a/solr/core/src/java/org/apache/solr/pkg/PackageLoader.java
+++ b/solr/core/src/java/org/apache/solr/pkg/PackageLoader.java
@@ -22,18 +22,9 @@ import java.io.IOException;
 import java.lang.invoke.MethodHandles;
 import java.nio.file.Path;
 import java.nio.file.Paths;
-import java.util.ArrayList;
-import java.util.Collection;
-import java.util.Collections;
-import java.util.HashMap;
-import java.util.HashSet;
-import java.util.List;
-import java.util.Map;
-import java.util.Objects;
-import java.util.Set;
+import java.util.*;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.CopyOnWriteArrayList;
-
 import org.apache.solr.common.MapWriter;
 import org.apache.solr.common.cloud.ZkStateReader;
 import org.apache.solr.core.CoreContainer;
@@ -269,7 +260,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 +292,34 @@ 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 not aware of a SolrCore and it is totally not tied to
+      // the lifecycle of SolrCore. So, this returns 'false' & it should be
+      // taken care of by the caller
+      return false;
+    }
+
+    @Override
+    public <T> boolean addToResourceLoaderAware(T obj) {
+      // do not do anything
+      // this should be invoked only after the init() is invoked.
+      // The caller should take care of that
+      return false;
+    }
+
+    @Override
+    public  <T> void addToInfoBeans(T obj) {
+      //do not do anything. It should be handled externally
+    }
+  }
 
   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..78a1658 100644
--- a/solr/core/src/java/org/apache/solr/pkg/PackagePluginHolder.java
+++ b/solr/core/src/java/org/apache/solr/pkg/PackagePluginHolder.java
@@ -17,13 +17,12 @@
 
 package org.apache.solr.pkg;
 
+import java.io.IOException;
 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.lucene.analysis.util.ResourceLoaderAware;
+import org.apache.solr.common.SolrException;
+import org.apache.solr.core.*;
+import org.apache.solr.util.plugin.SolrCoreAware;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -98,7 +97,7 @@ public class PackagePluginHolder<T> extends PluginBag.PluginHolder<T> {
 
     if (pkgVersion != null) {
       if (newest == pkgVersion) {
-        //I'm already using the latest classloder in the package. nothing to do
+        //I'm already using the latest classloader in the package. nothing to do
         return;
       }
     }
@@ -117,6 +116,7 @@ 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);
+    handleAwareCallbacks(newest.getLoader(), instance);
     T old = inst;
     inst = (T) instance;
     if (old instanceof AutoCloseable) {
@@ -129,4 +129,24 @@ public class PackagePluginHolder<T> extends PluginBag.PluginHolder<T> {
     }
   }
 
+  private void handleAwareCallbacks(SolrResourceLoader loader, Object instance) {
+    if (instance instanceof SolrCoreAware) {
+      SolrCoreAware coreAware = (SolrCoreAware) instance;
+      if (!core.getResourceLoader().addToCoreAware(coreAware)) {
+        coreAware.inform(core);
+      }
+    }
+    if (instance instanceof ResourceLoaderAware) {
+      SolrResourceLoader.assertAwareCompatibility(ResourceLoaderAware.class, instance);
+      try {
+        ((ResourceLoaderAware) instance).inform(loader);
+      } catch (IOException e) {
+        throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, e);
+      }
+    }
+    if (instance instanceof SolrInfoBean) {
+      core.getResourceLoader().addToInfoBeans(instance);
+    }
+  }
+
 }
\ No newline at end of file
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 0d38bd9..a99a9e4 100644
--- a/solr/core/src/test/org/apache/solr/pkg/TestPackages.java
+++ b/solr/core/src/test/org/apache/solr/pkg/TestPackages.java
@@ -21,10 +21,12 @@ import java.io.IOException;
 import java.nio.ByteBuffer;
 import java.util.Arrays;
 import java.util.Collections;
+import java.util.LinkedHashMap;
 import java.util.Map;
 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;
@@ -32,27 +34,31 @@ import org.apache.solr.client.solrj.SolrServerException;
 import org.apache.solr.client.solrj.embedded.JettySolrRunner;
 import org.apache.solr.client.solrj.impl.BaseHttpSolrClient;
 import org.apache.solr.client.solrj.impl.HttpSolrClient;
-import org.apache.solr.client.solrj.request.CollectionAdminRequest;
-import org.apache.solr.client.solrj.request.GenericSolrRequest;
-import org.apache.solr.client.solrj.request.RequestWriter;
-import org.apache.solr.client.solrj.request.UpdateRequest;
-import org.apache.solr.client.solrj.request.V2Request;
+import org.apache.solr.client.solrj.request.*;
 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;
@@ -62,9 +68,7 @@ import static org.apache.solr.common.cloud.ZkStateReader.SOLR_PKGS_PATH;
 import static org.apache.solr.common.params.CommonParams.JAVABIN;
 import static org.apache.solr.common.params.CommonParams.WT;
 import static org.apache.solr.core.TestDynamicLoading.getFileContent;
-import static org.apache.solr.filestore.TestDistribPackageStore.readFile;
-import static org.apache.solr.filestore.TestDistribPackageStore.uploadKey;
-import static org.apache.solr.filestore.TestDistribPackageStore.waitForAllNodesHaveFile;
+import static org.apache.solr.filestore.TestDistribPackageStore.*;
 
 @LogLevel("org.apache.solr.pkg.PackageLoader=DEBUG;org.apache.solr.pkg.PackageAPI=DEBUG")
 //@org.apache.lucene.util.LuceneTestCase.AwaitsFix(bugUrl="https://issues.apache.org/jira/browse/SOLR-13822") // leaks files
@@ -74,12 +78,18 @@ public class TestPackages extends SolrCloudTestCase {
   public void setup() {
     System.setProperty("enable.packages", "true");
   }
-  
+
   @After
   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 =
@@ -97,8 +107,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==");
 
@@ -135,20 +143,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",
@@ -372,6 +399,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
@@ -463,13 +511,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
@@ -550,6 +596,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);