You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by is...@apache.org on 2019/12/15 05:11:12 UTC
[lucene-solr] branch jira/solr-13662-fixes updated: SOLR-13662:
Review comments addressed
This is an automated email from the ASF dual-hosted git repository.
ishan pushed a commit to branch jira/solr-13662-fixes
in repository https://gitbox.apache.org/repos/asf/lucene-solr.git
The following commit(s) were added to refs/heads/jira/solr-13662-fixes by this push:
new 82a7d36 SOLR-13662: Review comments addressed
82a7d36 is described below
commit 82a7d364b6f91a8ad8c8e42f0a367a7e23afd28d
Author: Ishan Chattopadhyaya <is...@apache.org>
AuthorDate: Sun Dec 15 10:40:53 2019 +0530
SOLR-13662: Review comments addressed
---
.../packagemanager/DefaultPackageRepository.java | 3 +-
.../apache/solr/packagemanager/PackageManager.java | 63 +++++++++++-----------
.../apache/solr/packagemanager/PackageUtils.java | 7 +++
.../solr/packagemanager/RepositoryManager.java | 18 +++----
4 files changed, 50 insertions(+), 41 deletions(-)
diff --git a/solr/core/src/java/org/apache/solr/packagemanager/DefaultPackageRepository.java b/solr/core/src/java/org/apache/solr/packagemanager/DefaultPackageRepository.java
index 79cc1fa..b55dc71 100644
--- a/solr/core/src/java/org/apache/solr/packagemanager/DefaultPackageRepository.java
+++ b/solr/core/src/java/org/apache/solr/packagemanager/DefaultPackageRepository.java
@@ -26,6 +26,7 @@ import java.util.HashMap;
import java.util.Map;
import org.apache.commons.io.FileUtils;
+import org.apache.commons.io.FilenameUtils;
import org.apache.http.impl.client.CloseableHttpClient;
import org.apache.http.impl.client.HttpClientBuilder;
import org.apache.solr.common.SolrException;
@@ -83,7 +84,7 @@ public class DefaultPackageRepository extends PackageRepository {
Path tmpDirectory = Files.createTempDirectory("solr-packages");
tmpDirectory.toFile().deleteOnExit();
URL url = new URL(new URL(repositoryURL.endsWith("/")? repositoryURL: repositoryURL+"/"), artifactName);
- String fileName = url.getPath().substring(url.getPath().lastIndexOf('/') + 1);
+ String fileName = FilenameUtils.getName(url.getPath());
Path destination = tmpDirectory.resolve(fileName);
switch (url.getProtocol()) {
diff --git a/solr/core/src/java/org/apache/solr/packagemanager/PackageManager.java b/solr/core/src/java/org/apache/solr/packagemanager/PackageManager.java
index 493e00a..0d7a7d6 100644
--- a/solr/core/src/java/org/apache/solr/packagemanager/PackageManager.java
+++ b/solr/core/src/java/org/apache/solr/packagemanager/PackageManager.java
@@ -26,15 +26,19 @@ import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.HashMap;
+import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Scanner;
+import java.util.Set;
+import java.util.stream.Collectors;
import org.apache.solr.client.solrj.impl.HttpSolrClient;
import org.apache.solr.common.NavigableObject;
import org.apache.solr.common.SolrException;
import org.apache.solr.common.SolrException.ErrorCode;
import org.apache.solr.common.cloud.SolrZkClient;
+import org.apache.solr.common.cloud.ZkStateReader;
import org.apache.solr.common.util.Utils;
import org.apache.solr.packagemanager.SolrPackage.Command;
import org.apache.solr.packagemanager.SolrPackage.Manifest;
@@ -84,9 +88,9 @@ public class PackageManager implements Closeable {
try {
Map packagesZnodeMap = null;
- if (zkClient.exists("/packages.json", true) == true) {
+ if (zkClient.exists(ZkStateReader.SOLR_PKGS_PATH, true) == true) {
packagesZnodeMap = (Map)getMapper().readValue(
- new String(zkClient.getData("/packages.json", null, null, true), "UTF-8"), Map.class).get("packages");
+ new String(zkClient.getData(ZkStateReader.SOLR_PKGS_PATH, null, null, true), "UTF-8"), Map.class).get("packages");
for (Object packageName: packagesZnodeMap.keySet()) {
List pkg = (List)packagesZnodeMap.get(packageName);
for (Map pkgVersion: (List<Map>)pkg) {
@@ -112,18 +116,16 @@ public class PackageManager implements Closeable {
Map<String, String> packages = null;
try {
NavigableObject result = (NavigableObject) Utils.executeGET(solrClient.getHttpClient(),
- solrBaseUrl+"/api/collections/"+collection+"/config/params/PKG_VERSIONS?omitHeader=true&wt=javabin", Utils.JAVABINCONSUMER);
+ solrBaseUrl + PackageUtils.getCollectionParamsPath(collection) + "/PKG_VERSIONS?omitHeader=true&wt=javabin", Utils.JAVABINCONSUMER);
packages = (Map<String, String>) result._get("/response/params/PKG_VERSIONS", Collections.emptyMap());
} catch (PathNotFoundException ex) {
// Don't worry if PKG_VERSION wasn't found. It just means this collection was never touched by the package manager.
}
if (packages == null) return Collections.emptyMap();
- Map<String, SolrPackageInstance> ret = new HashMap<String, SolrPackageInstance>();
+ Map<String, SolrPackageInstance> ret = new HashMap<>();
for (String packageName: packages.keySet()) {
- if (Strings.isNullOrEmpty(packageName) == false) { // There can be an empty key, storing the version here
- if (packages.get(packageName) == null) { // This means the package was undeployed from this package before
- continue;
- }
+ if (Strings.isNullOrEmpty(packageName) == false && // There can be an empty key, storing the version here
+ packages.get(packageName) != null) { // null means the package was undeployed from this package before
ret.put(packageName, getPackageInstance(packageName, packages.get(packageName)));
}
}
@@ -133,10 +135,10 @@ public class PackageManager implements Closeable {
private void ensureCollectionsExist(List<String> collections) {
try {
List<String> existingCollections = zkClient.getChildren("/collections", null, true);
- for (String c: collections) {
- if (existingCollections.contains(c) == false) {
- throw new SolrException(ErrorCode.BAD_REQUEST, "Collection " + c + " doesn't exist.");
- }
+ Set<String> nonExistent = new HashSet<>(collections);
+ nonExistent.removeAll(existingCollections);
+ if (nonExistent.isEmpty() == false) {
+ throw new SolrException(ErrorCode.BAD_REQUEST, "Collection(s) doesn't exist: " + nonExistent.toString());
}
} catch (KeeperException | InterruptedException e) {
throw new SolrException(ErrorCode.SERVER_ERROR, "Unable to fetch list of collections from ZK.");
@@ -145,20 +147,20 @@ public class PackageManager implements Closeable {
private boolean deployPackage(SolrPackageInstance packageInstance, boolean pegToLatest, boolean isUpdate, boolean noprompt,
List<String> collections, String overrides[]) {
- List<String> alreadyDeployed = new ArrayList<>(); // collections where package is already deployed in
+ List<String> previouslyDeployed = new ArrayList<>(); // collections where package is already deployed in
for (String collection: collections) {
SolrPackageInstance deployedPackage = getPackagesDeployed(collection).get(packageInstance.name);
if (packageInstance.equals(deployedPackage)) {
if (!pegToLatest) {
PackageUtils.printRed("Package " + packageInstance + " already deployed on "+collection);
- alreadyDeployed.add(collection);
+ previouslyDeployed.add(collection);
continue;
}
} else {
if (deployedPackage != null && !isUpdate) {
PackageUtils.printRed("Package " + deployedPackage + " already deployed on "+collection+". To update to "+packageInstance+", pass --update parameter.");
- alreadyDeployed.add(collection);
+ previouslyDeployed.add(collection);
continue;
}
}
@@ -167,9 +169,9 @@ public class PackageManager implements Closeable {
// Get package params
try {
- boolean packageParamsExist = ((Map)PackageUtils.getJson(solrClient.getHttpClient(), solrBaseUrl + "/api/collections/"+collection+"/config/params/packages", Map.class)
+ boolean packageParamsExist = ((Map)PackageUtils.getJson(solrClient.getHttpClient(), solrBaseUrl + PackageUtils.getCollectionParamsPath(collection) + "/packages", Map.class)
.getOrDefault("response", Collections.emptyMap())).containsKey("params");
- SolrCLI.postJsonToSolr(solrClient, "/api/collections/" + collection + "/config/params",
+ SolrCLI.postJsonToSolr(solrClient, PackageUtils.getCollectionParamsPath(collection),
getMapper().writeValueAsString(Collections.singletonMap(packageParamsExist? "update": "set",
Collections.singletonMap("packages", Collections.singletonMap(packageInstance.name, collectionParameterOverrides)))));
} catch (Exception e) {
@@ -178,7 +180,7 @@ public class PackageManager implements Closeable {
// Set the package version in the collection's parameters
try {
- SolrCLI.postJsonToSolr(solrClient, "/api/collections/" + collection + "/config/params",
+ SolrCLI.postJsonToSolr(solrClient, PackageUtils.getCollectionParamsPath(collection),
"{set:{PKG_VERSIONS:{" + packageInstance.name+": '" + (pegToLatest? PackagePluginHolder.LATEST: packageInstance.version)+"'}}}");
} catch (Exception ex) {
throw new SolrException(ErrorCode.SERVER_ERROR, ex);
@@ -187,7 +189,7 @@ public class PackageManager implements Closeable {
// If updating, refresh the package version for this to take effect
if (isUpdate || pegToLatest) {
try {
- SolrCLI.postJsonToSolr(solrClient, "/api/cluster/package", "{\"refresh\": \"" + packageInstance.name + "\"}");
+ SolrCLI.postJsonToSolr(solrClient, PackageUtils.PACKAGE_PATH, "{\"refresh\": \"" + packageInstance.name + "\"}");
} catch (Exception ex) {
throw new SolrException(ErrorCode.SERVER_ERROR, ex);
}
@@ -232,15 +234,14 @@ public class PackageManager implements Closeable {
// Set the package version in the collection's parameters
try {
- SolrCLI.postJsonToSolr(solrClient, "/api/collections/" + collection + "/config/params",
+ SolrCLI.postJsonToSolr(solrClient, PackageUtils.getCollectionParamsPath(collection),
"{update:{PKG_VERSIONS:{'" + packageInstance.name + "' : '" + (pegToLatest? PackagePluginHolder.LATEST: packageInstance.version) + "'}}}");
} catch (Exception ex) {
throw new SolrException(ErrorCode.SERVER_ERROR, ex);
}
}
- List<String> deployedCollections = new ArrayList<String>();
- for (String c: collections) if (alreadyDeployed.contains(c) == false) deployedCollections.add(c);
+ List<String> deployedCollections = collections.stream().filter(c -> !previouslyDeployed.contains(c)).collect(Collectors.toList());
boolean success = true;
if (deployedCollections.isEmpty() == false) {
@@ -250,10 +251,10 @@ public class PackageManager implements Closeable {
PackageUtils.printGreen("Deployed on " + deployedCollections + " and verified package: " + packageInstance.name + ", version: " + packageInstance.version);
}
}
- if (alreadyDeployed.isEmpty() == false) {
- PackageUtils.printRed("Already Deployed on " + alreadyDeployed + ", package: " + packageInstance.name + ", version: " + packageInstance.version);
+ if (previouslyDeployed.isEmpty() == false) {
+ PackageUtils.printRed("Already Deployed on " + previouslyDeployed + ", package: " + packageInstance.name + ", version: " + packageInstance.version);
}
- return alreadyDeployed.isEmpty()==false? false: success;
+ return previouslyDeployed.isEmpty() && success;
}
private Map<String,String> getCollectionParameterOverrides(SolrPackageInstance packageInstance, boolean isUpdate,
@@ -271,7 +272,7 @@ public class PackageManager implements Closeable {
Map<String, String> getPackageParams(String packageName, String collection) {
try {
return (Map<String, String>)((Map)((Map)((Map)
- PackageUtils.getJson(solrClient.getHttpClient(), solrBaseUrl + "/api/collections/" + collection + "/config/params/packages", Map.class)
+ PackageUtils.getJson(solrClient.getHttpClient(), solrBaseUrl + PackageUtils.getCollectionParamsPath(collection) + "/packages", Map.class)
.get("response"))
.get("params"))
.get("packages")).get(packageName);
@@ -288,7 +289,6 @@ public class PackageManager implements Closeable {
public boolean verify(SolrPackageInstance pkg, List<String> collections) {
boolean success = true;
for (Plugin plugin: pkg.plugins) {
- // PackageUtils.printGreen(plugin.verifyCommand);
for (String collection: collections) {
Map<String, String> collectionParameterOverrides = getPackageParams(pkg.name, collection);
Command cmd = plugin.verifyCommand;
@@ -410,8 +410,9 @@ public class PackageManager implements Closeable {
// Set the package version in the collection's parameters
try {
- SolrCLI.postJsonToSolr(solrClient, "/api/collections/" + collection + "/config/params", "{set: {PKG_VERSIONS: {"+packageName+": null}}}"); // Is it better to "unset"? If so, build support in params API for "unset"
- SolrCLI.postJsonToSolr(solrClient, "/api/cluster/package", "{\"refresh\": \"" + packageName + "\"}");
+ SolrCLI.postJsonToSolr(solrClient, PackageUtils.getCollectionParamsPath(collection),
+ "{set: {PKG_VERSIONS: {"+packageName+": null}}}"); // Is it better to "unset"? If so, build support in params API for "unset"
+ SolrCLI.postJsonToSolr(solrClient, PackageUtils.PACKAGE_PATH, "{\"refresh\": \"" + packageName + "\"}");
} catch (Exception ex) {
throw new SolrException(ErrorCode.SERVER_ERROR, ex);
}
@@ -427,14 +428,14 @@ public class PackageManager implements Closeable {
public Map<String, String> getDeployedCollections(String packageName) {
List<String> allCollections;
try {
- allCollections = zkClient.getChildren("/collections", null, true);
+ allCollections = zkClient.getChildren(ZkStateReader.COLLECTIONS_ZKNODE, null, true);
} catch (KeeperException | InterruptedException e) {
throw new SolrException(ErrorCode.SERVICE_UNAVAILABLE, e);
}
Map<String, String> deployed = new HashMap<String, String>();
for (String collection: allCollections) {
// Check package version installed
- String paramsJson = PackageUtils.getJsonStringFromUrl(solrClient.getHttpClient(), solrBaseUrl + "/api/collections/" + collection + "/config/params/PKG_VERSIONS?omitHeader=true");
+ String paramsJson = PackageUtils.getJsonStringFromUrl(solrClient.getHttpClient(), solrBaseUrl + PackageUtils.getCollectionParamsPath(collection) + "/PKG_VERSIONS?omitHeader=true");
String version = null;
try {
version = JsonPath.parse(paramsJson, PackageUtils.jsonPathConfiguration())
diff --git a/solr/core/src/java/org/apache/solr/packagemanager/PackageUtils.java b/solr/core/src/java/org/apache/solr/packagemanager/PackageUtils.java
index 77ae057..11859a2 100644
--- a/solr/core/src/java/org/apache/solr/packagemanager/PackageUtils.java
+++ b/solr/core/src/java/org/apache/solr/packagemanager/PackageUtils.java
@@ -58,6 +58,9 @@ public class PackageUtils {
* Represents a version which denotes the latest version available at the moment.
*/
public static String LATEST = "latest";
+
+ public static String PACKAGE_PATH = "/api/cluster/package";
+ public static String REPOSITORIES_ZK_PATH = "/repositories.json";
public static Configuration jsonPathConfiguration() {
MappingProvider provider = new JacksonMappingProvider();
@@ -235,4 +238,8 @@ public class PackageUtils {
}
return collections;
}
+
+ public static String getCollectionParamsPath(String collection) {
+ return "/api/collections/" + collection + "/config/params";
+ }
}
diff --git a/solr/core/src/java/org/apache/solr/packagemanager/RepositoryManager.java b/solr/core/src/java/org/apache/solr/packagemanager/RepositoryManager.java
index 66b59f5..3d6075b 100644
--- a/solr/core/src/java/org/apache/solr/packagemanager/RepositoryManager.java
+++ b/solr/core/src/java/org/apache/solr/packagemanager/RepositoryManager.java
@@ -122,23 +122,23 @@ public class RepositoryManager {
List<PackageRepository> repos = getMapper().readValue(existingRepositoriesJson, List.class);
repos.add(new DefaultPackageRepository(name, uri));
- if (packageManager.zkClient.exists("/repositories.json", true) == false) {
- packageManager.zkClient.create("/repositories.json", getMapper().writeValueAsString(repos).getBytes("UTF-8"), CreateMode.PERSISTENT, true);
+ if (packageManager.zkClient.exists(PackageUtils.REPOSITORIES_ZK_PATH, true) == false) {
+ packageManager.zkClient.create(PackageUtils.REPOSITORIES_ZK_PATH, getMapper().writeValueAsString(repos).getBytes("UTF-8"), CreateMode.PERSISTENT, true);
} else {
- packageManager.zkClient.setData("/repositories.json", getMapper().writeValueAsString(repos).getBytes("UTF-8"), true);
+ packageManager.zkClient.setData(PackageUtils.REPOSITORIES_ZK_PATH, getMapper().writeValueAsString(repos).getBytes("UTF-8"), true);
}
if (packageManager.zkClient.exists("/keys", true)==false) packageManager.zkClient.create("/keys", new byte[0], CreateMode.PERSISTENT, true);
if (packageManager.zkClient.exists("/keys/exe", true)==false) packageManager.zkClient.create("/keys/exe", new byte[0], CreateMode.PERSISTENT, true);
- if (packageManager.zkClient.exists("/keys/exe/"+name+".der", true)==false) {
- packageManager.zkClient.create("/keys/exe/"+name+".der", new byte[0], CreateMode.PERSISTENT, true);
+ if (packageManager.zkClient.exists("/keys/exe/" + name + ".der", true)==false) {
+ packageManager.zkClient.create("/keys/exe/" + name + ".der", new byte[0], CreateMode.PERSISTENT, true);
}
- packageManager.zkClient.setData("/keys/exe/"+name+".der", IOUtils.toByteArray(new URL(uri+"/publickey.der").openStream()), true);
+ packageManager.zkClient.setData("/keys/exe/" + name + ".der", IOUtils.toByteArray(new URL(uri + "/publickey.der").openStream()), true);
}
private String getRepositoriesJson(SolrZkClient zkClient) throws UnsupportedEncodingException, KeeperException, InterruptedException {
- if (zkClient.exists("/repositories.json", true)) {
- return new String(zkClient.getData("/repositories.json", null, null, true), "UTF-8");
+ if (zkClient.exists(PackageUtils.REPOSITORIES_ZK_PATH, true)) {
+ return new String(zkClient.getData(PackageUtils.REPOSITORIES_ZK_PATH, null, null, true), "UTF-8");
}
return "[]";
}
@@ -195,7 +195,7 @@ public class RepositoryManager {
add.manifest = "/package/" + packageName + "/" + version + "/manifest.json";
add.manifestSHA512 = manifestSHA512;
- V2Request req = new V2Request.Builder("/api/cluster/package")
+ V2Request req = new V2Request.Builder(PackageUtils.PACKAGE_PATH)
.forceV2(true)
.withMethod(SolrRequest.METHOD.POST)
.withPayload(Collections.singletonMap("add", add))