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 09:53:51 UTC
[lucene] 05/08: incorporating several review feedbacks. Added more
tests. Changed the config object type
This is an automated email from the ASF dual-hosted git repository.
dweiss pushed a commit to branch jira/solr14977
in repository https://gitbox.apache.org/repos/asf/lucene.git
commit b7983a765c883014ad47d7a1735d7d337047846e
Author: noblepaul <no...@gmail.com>
AuthorDate: Wed Nov 11 10:58:27 2020 +1100
incorporating several review feedbacks. Added more tests. Changed the config object type
---
.../src/java/org/apache/solr/api/AnnotatedApi.java | 5 ++--
.../org/apache/solr/api/ConfigurablePlugin.java | 2 +-
.../apache/solr/api/ContainerPluginsRegistry.java | 23 +++++++++++++----
.../src/java/org/apache/solr/api/V2HttpCall.java | 2 +-
.../solr/handler/admin/ContainerPluginsApi.java | 12 +++++----
.../apache/solr/handler/TestContainerPlugin.java | 29 ++++++++++++++++++----
6 files changed, 54 insertions(+), 19 deletions(-)
diff --git a/solr/core/src/java/org/apache/solr/api/AnnotatedApi.java b/solr/core/src/java/org/apache/solr/api/AnnotatedApi.java
index df373ea..9ec86ce 100644
--- a/solr/core/src/java/org/apache/solr/api/AnnotatedApi.java
+++ b/solr/core/src/java/org/apache/solr/api/AnnotatedApi.java
@@ -63,6 +63,8 @@ import org.slf4j.LoggerFactory;
public class AnnotatedApi extends Api implements PermissionNameProvider , Closeable {
private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+ private static final ObjectMapper mapper = SolrJacksonAnnotationInspector.createObjectMapper()
+ .configure(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES, false);
public static final String ERR = "Error executing commands :";
private EndPoint endPoint;
@@ -223,8 +225,7 @@ public class AnnotatedApi extends Api implements PermissionNameProvider , Closea
final String command;
final MethodHandle method;
final Object obj;
- ObjectMapper mapper = SolrJacksonAnnotationInspector.createObjectMapper()
- .configure(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES, false);
+
int paramsCount;
@SuppressWarnings({"rawtypes"})
Class parameterClass;
diff --git a/solr/core/src/java/org/apache/solr/api/ConfigurablePlugin.java b/solr/core/src/java/org/apache/solr/api/ConfigurablePlugin.java
index 969cc8d..e549bac 100644
--- a/solr/core/src/java/org/apache/solr/api/ConfigurablePlugin.java
+++ b/solr/core/src/java/org/apache/solr/api/ConfigurablePlugin.java
@@ -27,5 +27,5 @@ public interface ConfigurablePlugin<T> {
*
* @param cfg value deserialized from JSON
*/
- void initConfig(T cfg);
+ void configure(T cfg);
}
diff --git a/solr/core/src/java/org/apache/solr/api/ContainerPluginsRegistry.java b/solr/core/src/java/org/apache/solr/api/ContainerPluginsRegistry.java
index 883767a..62dab20 100644
--- a/solr/core/src/java/org/apache/solr/api/ContainerPluginsRegistry.java
+++ b/solr/core/src/java/org/apache/solr/api/ContainerPluginsRegistry.java
@@ -126,6 +126,20 @@ public class ContainerPluginsRegistry implements ClusterPropertiesListener, MapW
this.original = original;
meta = mapper.readValue(Utils.toJSON(original), PluginMeta.class);
}
+
+ @Override
+ public boolean equals(Object obj) {
+ if (obj instanceof PluginMetaHolder) {
+ PluginMetaHolder that = (PluginMetaHolder) obj;
+ return Objects.equals(this.original,that.original);
+ }
+ return false;
+ }
+
+ @Override
+ public int hashCode() {
+ return original.hashCode();
+ }
}
@SuppressWarnings("unchecked")
public synchronized void refresh() {
@@ -365,7 +379,7 @@ public class ContainerPluginsRegistry implements ClusterPropertiesListener, MapW
}
}
- @SuppressWarnings({"rawtypes","unchecked"})
+ @SuppressWarnings({"rawtypes", "unchecked"})
public void init() throws Exception {
if (this.holders != null) return;
Constructor constructor = klas.getConstructors()[0];
@@ -378,11 +392,10 @@ public class ContainerPluginsRegistry implements ClusterPropertiesListener, MapW
}
if (instance instanceof ConfigurablePlugin) {
Class c = getConfigClass((ConfigurablePlugin<?>) instance);
- if(c != null) {
- Object initVal = mapper.readValue(Utils.toJSON(holder.original), c);
- ((ConfigurablePlugin) instance).initConfig(initVal);
+ if (c != null) {
+ Object initVal = mapper.readValue(Utils.toJSON(holder.original), c);
+ ((ConfigurablePlugin) instance).configure(initVal);
}
-
}
if (instance instanceof ResourceLoaderAware) {
try {
diff --git a/solr/core/src/java/org/apache/solr/api/V2HttpCall.java b/solr/core/src/java/org/apache/solr/api/V2HttpCall.java
index 5eae502..504db1f 100644
--- a/solr/core/src/java/org/apache/solr/api/V2HttpCall.java
+++ b/solr/core/src/java/org/apache/solr/api/V2HttpCall.java
@@ -136,7 +136,7 @@ public class V2HttpCall extends HttpSolrCall {
initAdminRequest(path);
return;
} else {
- throw new SolrException(SolrException.ErrorCode.NOT_FOUND, "no core retrieved for " + origCorename);
+ throw new SolrException(SolrException.ErrorCode.NOT_FOUND, "no core retrieved for core name: " + origCorename + ". Path : "+ path);
}
}
diff --git a/solr/core/src/java/org/apache/solr/handler/admin/ContainerPluginsApi.java b/solr/core/src/java/org/apache/solr/handler/admin/ContainerPluginsApi.java
index 4e3f7f5..57bcbef 100644
--- a/solr/core/src/java/org/apache/solr/handler/admin/ContainerPluginsApi.java
+++ b/solr/core/src/java/org/apache/solr/handler/admin/ContainerPluginsApi.java
@@ -19,10 +19,7 @@ package org.apache.solr.handler.admin;
import java.io.IOException;
import java.lang.invoke.MethodHandles;
-import java.util.ArrayList;
-import java.util.LinkedHashMap;
-import java.util.List;
-import java.util.Map;
+import java.util.*;
import java.util.function.Function;
import java.util.function.Supplier;
@@ -122,7 +119,12 @@ public class ContainerPluginsApi {
payload.addError("No such plugin: " + info.name);
return null;
} else {
- map.put(info.name, info);
+ Map<String, Object> jsonObj = payload.getDataMap();
+ if(Objects.equals(jsonObj, existing)) {
+ //no need to change anything
+ return null;
+ }
+ map.put(info.name, jsonObj);
return map;
}
});
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 7f9a927..01ab39f 100644
--- a/solr/core/src/test/org/apache/solr/handler/TestContainerPlugin.java
+++ b/solr/core/src/test/org/apache/solr/handler/TestContainerPlugin.java
@@ -44,6 +44,7 @@ import org.apache.solr.cloud.MiniSolrCloudCluster;
import org.apache.solr.cloud.SolrCloudTestCase;
import org.apache.solr.common.NavigableObject;
import org.apache.solr.common.annotation.JsonProperty;
+import org.apache.solr.common.util.ReflectMapWriter;
import org.apache.solr.common.util.Utils;
import org.apache.solr.core.CoreContainer;
import org.apache.solr.core.SolrResourceLoader;
@@ -212,8 +213,22 @@ public class TestContainerPlugin extends SolrCloudTestCase {
.build().process(cluster.getSolrClient()),
ImmutableMap.of("/config/boolVal", "true", "/config/strVal", "Something","/config/longVal", "1234" ));
-
- // kill the Overseer leader
+ p.strVal = "Something else";
+ new V2Request.Builder("/cluster/plugin")
+ .forceV2(true)
+ .POST()
+ .withPayload(singletonMap("update", p))
+ .build()
+ .process(cluster.getSolrClient());
+
+ TestDistribPackageStore.assertResponseValues(10,
+ () -> new V2Request.Builder("hello/plugin")
+ .forceV2(true)
+ .GET()
+ .build().process(cluster.getSolrClient()),
+ ImmutableMap.of("/config/boolVal", "true", "/config/strVal", p.strVal,"/config/longVal", "1234" ));
+
+ // kill the Overseer leader
for (JettySolrRunner jetty : cluster.getJettySolrRunners()) {
if (!jetty.getCoreContainer().getZkController().getOverseer().isClosed()) {
cluster.stopJettySolrRunner(jetty);
@@ -352,7 +367,7 @@ public class TestContainerPlugin extends SolrCloudTestCase {
@Override
- public void initConfig(CConfig cfg) {
+ public void configure(CConfig cfg) {
this.cfg = cfg;
}
@@ -366,7 +381,7 @@ public class TestContainerPlugin extends SolrCloudTestCase {
}
- public static class CConfig extends PluginMeta {
+ public static class CConfig implements ReflectMapWriter {
@JsonProperty
public String strVal;
@@ -377,6 +392,11 @@ public class TestContainerPlugin extends SolrCloudTestCase {
@JsonProperty
public Boolean boolVal;
+ @JsonProperty
+ public String name;
+
+ @JsonProperty(value = "class", required = true)
+ public String klass;
}
public static class C6 implements ClusterSingleton {
@@ -423,7 +443,6 @@ public class TestContainerPlugin extends SolrCloudTestCase {
private SolrResourceLoader resourceLoader;
@Override
- @SuppressWarnings("unchecked")
public void inform(ResourceLoader loader) throws IOException {
this.resourceLoader = (SolrResourceLoader) loader;
try {