You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by dw...@apache.org on 2021/03/10 10:09:57 UTC
[lucene] 25/33: SOLR-15016: Use '.' prefix for predefined plugins.
Prevent errors when using unquoted JSON with string with a leading dot :)
Make sure to process only plugin configs with the predefined names.
This is an automated email from the ASF dual-hosted git repository.
dweiss pushed a commit to branch jira/solr-15016
in repository https://gitbox.apache.org/repos/asf/lucene.git
commit 88083a44f8ff6c3896404059662cd6a13191a68e
Author: Andrzej Bialecki <ab...@apache.org>
AuthorDate: Wed Dec 2 14:09:53 2020 +0100
SOLR-15016: Use '.' prefix for predefined plugins.
Prevent errors when using unquoted JSON with string with a leading dot :)
Make sure to process only plugin configs with the predefined names.
---
.../solr/cluster/events/ClusterEventProducer.java | 2 +-
.../events/impl/ClusterEventProducerFactory.java | 32 ++++++++++++++--------
.../cluster/placement/PlacementPluginFactory.java | 2 +-
.../impl/PlacementPluginFactoryLoader.java | 17 ++++++++++--
.../impl/PlacementPluginIntegrationTest.java | 30 +++++++++++++++-----
.../client/solrj/request/beans/PluginMeta.java | 6 ++++
6 files changed, 66 insertions(+), 23 deletions(-)
diff --git a/solr/core/src/java/org/apache/solr/cluster/events/ClusterEventProducer.java b/solr/core/src/java/org/apache/solr/cluster/events/ClusterEventProducer.java
index d3b0ee7..aa36fd7 100644
--- a/solr/core/src/java/org/apache/solr/cluster/events/ClusterEventProducer.java
+++ b/solr/core/src/java/org/apache/solr/cluster/events/ClusterEventProducer.java
@@ -26,7 +26,7 @@ import java.io.Closeable;
public interface ClusterEventProducer extends ClusterSingleton, Closeable {
/** Unique name for the registration of a plugin-based implementation. */
- String PLUGIN_NAME = "cluster-event-producer";
+ String PLUGIN_NAME = ".cluster-event-producer";
@Override
default String getName() {
diff --git a/solr/core/src/java/org/apache/solr/cluster/events/impl/ClusterEventProducerFactory.java b/solr/core/src/java/org/apache/solr/cluster/events/impl/ClusterEventProducerFactory.java
index 85f1410..609f65c 100644
--- a/solr/core/src/java/org/apache/solr/cluster/events/impl/ClusterEventProducerFactory.java
+++ b/solr/core/src/java/org/apache/solr/cluster/events/impl/ClusterEventProducerFactory.java
@@ -128,13 +128,17 @@ public class ClusterEventProducerFactory extends ClusterEventProducerBase {
ClusterEventListener listener = (ClusterEventListener) instance;
clusterEventProducer.registerListener(listener);
} else if (instance instanceof ClusterEventProducer) {
- // replace the existing impl
- if (cc.getClusterEventProducer() instanceof DelegatingClusterEventProducer) {
- ((DelegatingClusterEventProducer) cc.getClusterEventProducer())
- .setDelegate((ClusterEventProducer) instance);
+ if (ClusterEventProducer.PLUGIN_NAME.equals(plugin.getInfo().name)) {
+ // replace the existing impl
+ if (cc.getClusterEventProducer() instanceof DelegatingClusterEventProducer) {
+ ((DelegatingClusterEventProducer) cc.getClusterEventProducer())
+ .setDelegate((ClusterEventProducer) instance);
+ } else {
+ log.warn("Can't configure plugin-based ClusterEventProducer while CoreContainer is still loading - " +
+ " using existing implementation {}", cc.getClusterEventProducer().getClass().getName());
+ }
} else {
- log.warn("Can't configure plugin-based ClusterEventProducer while CoreContainer is still loading - " +
- " using existing implementation {}", cc.getClusterEventProducer().getClass().getName());
+ log.warn("Ignoring ClusterEventProducer config with non-standard name: " + plugin.getInfo());
}
}
}
@@ -149,13 +153,17 @@ public class ClusterEventProducerFactory extends ClusterEventProducerBase {
ClusterEventListener listener = (ClusterEventListener) instance;
clusterEventProducer.unregisterListener(listener);
} else if (instance instanceof ClusterEventProducer) {
- // replace the existing impl with NoOp
- if (cc.getClusterEventProducer() instanceof DelegatingClusterEventProducer) {
- ((DelegatingClusterEventProducer) cc.getClusterEventProducer())
- .setDelegate(new NoOpProducer(cc));
+ if (ClusterEventProducer.PLUGIN_NAME.equals(plugin.getInfo().name)) {
+ // replace the existing impl with NoOp
+ if (cc.getClusterEventProducer() instanceof DelegatingClusterEventProducer) {
+ ((DelegatingClusterEventProducer) cc.getClusterEventProducer())
+ .setDelegate(new NoOpProducer(cc));
+ } else {
+ log.warn("Can't configure plugin-based ClusterEventProducer while CoreContainer is still loading - " +
+ " using existing implementation {}", cc.getClusterEventProducer().getClass().getName());
+ }
} else {
- log.warn("Can't configure plugin-based ClusterEventProducer while CoreContainer is still loading - " +
- " using existing implementation {}", cc.getClusterEventProducer().getClass().getName());
+ log.warn("Ignoring ClusterEventProducer config with non-standard name: " + plugin.getInfo());
}
}
}
diff --git a/solr/core/src/java/org/apache/solr/cluster/placement/PlacementPluginFactory.java b/solr/core/src/java/org/apache/solr/cluster/placement/PlacementPluginFactory.java
index abdd7b9..fc537ca 100644
--- a/solr/core/src/java/org/apache/solr/cluster/placement/PlacementPluginFactory.java
+++ b/solr/core/src/java/org/apache/solr/cluster/placement/PlacementPluginFactory.java
@@ -28,7 +28,7 @@ public interface PlacementPluginFactory {
/**
* The key in the plugins registry under which this plugin and its configuration are defined.
*/
- String PLUGIN_NAME = "placement-plugin";
+ String PLUGIN_NAME = ".placement-plugin";
/**
* Returns an instance of the plugin that will be repeatedly (and concurrently) be called to compute placement. Multiple
diff --git a/solr/core/src/java/org/apache/solr/cluster/placement/impl/PlacementPluginFactoryLoader.java b/solr/core/src/java/org/apache/solr/cluster/placement/impl/PlacementPluginFactoryLoader.java
index 8207279..46e6435 100644
--- a/solr/core/src/java/org/apache/solr/cluster/placement/impl/PlacementPluginFactoryLoader.java
+++ b/solr/core/src/java/org/apache/solr/cluster/placement/impl/PlacementPluginFactoryLoader.java
@@ -3,12 +3,17 @@ package org.apache.solr.cluster.placement.impl;
import org.apache.solr.api.ContainerPluginsRegistry;
import org.apache.solr.cluster.placement.PlacementPlugin;
import org.apache.solr.cluster.placement.PlacementPluginFactory;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.lang.invoke.MethodHandles;
/**
* Utility class to load the configured {@link PlacementPluginFactory} plugin and
* then keep it up to date as the plugin configuration changes.
*/
public class PlacementPluginFactoryLoader {
+ private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
public static PlacementPluginFactory load(ContainerPluginsRegistry plugins) {
final DelegatingPlacementPluginFactory pluginFactory = new DelegatingPlacementPluginFactory();
@@ -24,7 +29,11 @@ public class PlacementPluginFactoryLoader {
}
Object instance = plugin.getInstance();
if (instance instanceof PlacementPluginFactory) {
- pluginFactory.setDelegate((PlacementPluginFactory) instance);
+ if (PlacementPluginFactory.PLUGIN_NAME.equals(plugin.getInfo().name)) {
+ pluginFactory.setDelegate((PlacementPluginFactory) instance);
+ } else {
+ log.warn("Ignoring PlacementPluginFactory plugin with non-standard name: " + plugin.getInfo());
+ }
}
}
@@ -35,7 +44,11 @@ public class PlacementPluginFactoryLoader {
}
Object instance = plugin.getInstance();
if (instance instanceof PlacementPluginFactory) {
- pluginFactory.setDelegate(null);
+ if (PlacementPluginFactory.PLUGIN_NAME.equals(plugin.getInfo().name)) {
+ pluginFactory.setDelegate(null);
+ } else {
+ log.warn("Ignoring PlacementPluginFactory plugin with non-standard name: " + plugin.getInfo());
+ }
}
}
diff --git a/solr/core/src/test/org/apache/solr/cluster/placement/impl/PlacementPluginIntegrationTest.java b/solr/core/src/test/org/apache/solr/cluster/placement/impl/PlacementPluginIntegrationTest.java
index e3c9a27..6159358 100644
--- a/solr/core/src/test/org/apache/solr/cluster/placement/impl/PlacementPluginIntegrationTest.java
+++ b/solr/core/src/test/org/apache/solr/cluster/placement/impl/PlacementPluginIntegrationTest.java
@@ -84,7 +84,7 @@ public class PlacementPluginIntegrationTest extends SolrCloudTestCase {
req = new V2Request.Builder("/cluster/plugin")
.forceV2(true)
.POST()
- .withPayload("{remove: " + PlacementPluginFactory.PLUGIN_NAME + "}")
+ .withPayload("{remove: '" + PlacementPluginFactory.PLUGIN_NAME + "'}")
.build();
req.process(cluster.getSolrClient());
}
@@ -161,7 +161,7 @@ public class PlacementPluginIntegrationTest extends SolrCloudTestCase {
.build();
req.process(cluster.getSolrClient());
- version = waitForVersionChange(version, wrapper);
+ version = waitForVersionChange(version, wrapper, 10);
factory = wrapper.getDelegate();
assertTrue("wrong type " + factory.getClass().getName(), factory instanceof AffinityPlacementFactory);
@@ -178,27 +178,43 @@ public class PlacementPluginIntegrationTest extends SolrCloudTestCase {
.build();
req.process(cluster.getSolrClient());
- version = waitForVersionChange(version, wrapper);
+ version = waitForVersionChange(version, wrapper, 10);
factory = wrapper.getDelegate();
assertTrue("wrong type " + factory.getClass().getName(), factory instanceof AffinityPlacementFactory);
config = ((AffinityPlacementFactory) factory).getConfig();
assertEquals("minimalFreeDiskGB", 3, config.minimalFreeDiskGB);
assertEquals("prioritizedFreeDiskGB", 4, config.prioritizedFreeDiskGB);
+ // add plugin of the right type but with the wrong name
+ plugin.name = "myPlugin";
+ req = new V2Request.Builder("/cluster/plugin")
+ .forceV2(true)
+ .POST()
+ .withPayload(singletonMap("add", plugin))
+ .build();
+ req.process(cluster.getSolrClient());
+ try {
+ int newVersion = waitForVersionChange(version, wrapper, 5);
+ if (newVersion != version) {
+ fail("factory configuration updated but plugin name was wrong: " + plugin);
+ }
+ } catch (TimeoutException te) {
+ // expected
+ }
// remove plugin
req = new V2Request.Builder("/cluster/plugin")
.forceV2(true)
.POST()
- .withPayload("{remove: " + PlacementPluginFactory.PLUGIN_NAME + "}")
+ .withPayload("{remove: '" + PlacementPluginFactory.PLUGIN_NAME + "'}")
.build();
req.process(cluster.getSolrClient());
- version = waitForVersionChange(version, wrapper);
+ version = waitForVersionChange(version, wrapper, 10);
factory = wrapper.getDelegate();
assertNull("no factory should be present", factory);
}
- private int waitForVersionChange(int currentVersion, PlacementPluginFactoryLoader.DelegatingPlacementPluginFactory wrapper) throws Exception {
- TimeOut timeout = new TimeOut(60, TimeUnit.SECONDS, TimeSource.NANO_TIME);
+ private int waitForVersionChange(int currentVersion, PlacementPluginFactoryLoader.DelegatingPlacementPluginFactory wrapper, int timeoutSec) throws Exception {
+ TimeOut timeout = new TimeOut(timeoutSec, TimeUnit.SECONDS, TimeSource.NANO_TIME);
while (!timeout.hasTimedOut()) {
int newVersion = wrapper.getVersion();
diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/request/beans/PluginMeta.java b/solr/solrj/src/java/org/apache/solr/client/solrj/request/beans/PluginMeta.java
index 5bee19f..3586ffa 100644
--- a/solr/solrj/src/java/org/apache/solr/client/solrj/request/beans/PluginMeta.java
+++ b/solr/solrj/src/java/org/apache/solr/client/solrj/request/beans/PluginMeta.java
@@ -21,6 +21,7 @@ import java.util.Objects;
import org.apache.solr.common.annotation.JsonProperty;
import org.apache.solr.common.util.ReflectMapWriter;
+import org.apache.solr.common.util.Utils;
/**
* POJO for a plugin metadata used in container plugins
@@ -71,4 +72,9 @@ public class PluginMeta implements ReflectMapWriter {
public int hashCode() {
return Objects.hash(name, version, klass);
}
+
+ @Override
+ public String toString() {
+ return Utils.toJSONString(this);
+ }
}