jaykay12 commented on code in PR #4739:
URL: https://github.com/apache/solr/pull/4739#discussion_r4172743728


##########
solr/core/src/java/org/apache/solr/cli/PackageTool.java:
##########
@@ -113,185 +206,203 @@ public String getName() {
               + "don't print stack traces, hence special treatment is needed 
here."
               + "Need to turn off logging, and SLF4J doesn't seem to provide 
for a way.")
   public void runImpl(CommandLine cli) throws Exception {
+    String solrUrl = CLIUtils.normalizeSolrUrl(cli);
+    String zkHost = CLIUtils.getZkHost(cli);
+    String credentials = 
cli.getOptionValue(CommonCLIOptions.CREDENTIALS_OPTION);
+    String command = cli.getArgs()[0];
+    String[] cmdArgs = Arrays.copyOfRange(cli.getArgs(), 1, 
cli.getArgs().length);
+    PackageFlags packageFlags =
+        new PackageFlags(
+            cli.getOptionValue(COLLECTIONS_OPTION),
+            cli.hasOption(CLUSTER_OPTION),
+            cli.getOptionValues(PARAM_OPTION),
+            cli.hasOption(UPDATE_OPTION),
+            cli.getOptionValue(COLLECTION_OPTION),
+            cli.hasOption(NO_PROMPT_OPTION));
+
+    executePackage(solrUrl, zkHost, credentials, command, cmdArgs, 
packageFlags);
+  }
+
+  private void executePackage(
+      String solrUrl,
+      String zkHost,
+      String credentials,
+      String command,
+      String[] cmdArgs,
+      PackageFlags packageFlags)
+      throws Exception {
 
     // Need a logging free, clean output going through to the user.
     Level oldLevel = 
LoggerContext.getContext(false).getRootLogger().getLevel();
     Configurator.setRootLevel(Level.OFF);
 
     try {
-      String solrUrl = CLIUtils.normalizeSolrUrl(cli);
-      String zkHost = CLIUtils.getZkHost(cli);
       if (zkHost == null) {
         throw new SolrException(ErrorCode.INVALID_STATE, "Package manager runs 
only in SolrCloud");
       }
 
       log.info("ZK: {}", zkHost);
 
-      String cmd = cli.getArgs()[0];
-
-      try (SolrClient solrClient = CLIUtils.getSolrClient(cli, true)) {
+      try (SolrClient solrClient = CLIUtils.getSolrClient(solrUrl, 
credentials, true)) {
         packageManager = new PackageManager(runtime, solrClient, solrUrl, 
zkHost);
         try {
           repositoryManager = new RepositoryManager(solrClient, 
packageManager);
-
-          switch (cmd) {
-            case "add-repo":
-              String repoName = cli.getArgs()[1];
-              String repoUrl = cli.getArgs()[2];
-              repositoryManager.addRepository(repoName, repoUrl);
-              printGreen("Added repository: " + repoName);
-              break;
-            case "add-key":
-              String keyFilename = cli.getArgs()[1];
-              Path path = Path.of(keyFilename);
-              repositoryManager.addKey(Files.readAllBytes(path), 
path.getFileName().toString());
-              break;
-            case "list-installed":
-              printGreen("Installed packages:\n-----");
-              for (SolrPackageInstance pkg : 
packageManager.fetchInstalledPackageInstances()) {
-                printGreen(pkg);
-              }
-              break;
-            case "list-available":
-              printGreen("Available packages:\n-----");
-              for (SolrPackage pkg : repositoryManager.getPackages()) {
-                printGreen(pkg.name + " \t\t" + pkg.description);
-                for (SolrPackageRelease version : pkg.versions) {
-                  printGreen("\tVersion: " + version.version);
-                }
-              }
-              break;
-            case "list-deployed":
-              if (cli.hasOption(COLLECTION_OPTION)) {
-                String collection = cli.getOptionValue(COLLECTION_OPTION);
-                Map<String, SolrPackageInstance> packages =
-                    packageManager.getPackagesDeployed(collection);
-                printGreen("Packages deployed on " + collection + ":");
-                for (String packageName : packages.keySet()) {
-                  printGreen("\t" + packages.get(packageName));
-                }
-              } else {
-                // nuance that we use an arg here instead of requiring a 
--package parameter with a
-                // value
-                // in this code path
-                String packageName = cli.getArgs()[1];
-                Map<String, String> deployedCollections =
-                    packageManager.getDeployedCollections(packageName);
-                if (!deployedCollections.isEmpty()) {
-                  printGreen("Collections on which package " + packageName + " 
was deployed:");
-                  for (String collection : deployedCollections.keySet()) {
-                    printGreen(
-                        "\t"
-                            + collection
-                            + "("
-                            + packageName
-                            + ":"
-                            + deployedCollections.get(collection)
-                            + ")");
-                  }
-                } else {
-                  printGreen("Package " + packageName + " not deployed on any 
collection.");
-                }
-              }
-              break;
-            case "install":
-              {
-                Pair<String, String> parsedVersion = 
parsePackageVersion(cli.getArgList().get(1));
-                String packageName = parsedVersion.first();
-                String version = parsedVersion.second();
-                boolean success = repositoryManager.install(packageName, 
version);
-                if (success) {
-                  printGreen(packageName + " installed.");
-                } else {
-                  printRed(packageName + " installation failed.");
-                }
-                break;
-              }
-            case "deploy":
-              {
-                if (cli.hasOption(CLUSTER_OPTION) || 
cli.hasOption(COLLECTIONS_OPTION)) {
-                  Pair<String, String> parsedVersion = 
parsePackageVersion(cli.getArgList().get(1));
-                  String packageName = parsedVersion.first();
-                  String version = parsedVersion.second();
-                  boolean noPrompt = cli.hasOption(NO_PROMPT_OPTION);
-                  boolean isUpdate = cli.hasOption(UPDATE_OPTION);
-                  String[] collections =
-                      cli.hasOption(COLLECTIONS_OPTION)
-                          ? PackageUtils.validateCollections(
-                              
cli.getOptionValue(COLLECTIONS_OPTION).split(","))
-                          : new String[] {};
-                  String[] parameters = cli.getOptionValues(PARAM_OPTION);
-                  packageManager.deploy(
-                      packageName,
-                      version,
-                      collections,
-                      cli.hasOption(CLUSTER_OPTION),
-                      parameters,
-                      isUpdate,
-                      noPrompt);
-                } else {
-                  printRed(
-                      "Either specify --cluster to deploy cluster level 
plugins or --collections <list-of-collections> to deploy collection level 
plugins");
-                }
-                break;
-              }
-            case "undeploy":
-              {
-                if (cli.hasOption(CLUSTER_OPTION) || 
cli.hasOption(COLLECTIONS_OPTION)) {
-                  Pair<String, String> parsedVersion = 
parsePackageVersion(cli.getArgList().get(1));
-                  if (parsedVersion.second() != null) {
-                    throw new SolrException(
-                        ErrorCode.BAD_REQUEST,
-                        "Only package name expected, without a version. 
Actual: "
-                            + cli.getArgList().get(1));
-                  }
-                  String packageName = parsedVersion.first();
-                  String[] collections =
-                      cli.hasOption(COLLECTIONS_OPTION)
-                          ? PackageUtils.validateCollections(
-                              
cli.getOptionValue(COLLECTIONS_OPTION).split(","))
-                          : new String[] {};
-                  packageManager.undeploy(packageName, collections, 
cli.hasOption(CLUSTER_OPTION));
-                } else {
-                  printRed(
-                      "Either specify --cluster to undeploy cluster level 
plugins or -collections <list-of-collections> to undeploy collection level 
plugins");
-                }
-                break;
-              }
-            case "uninstall":
-              {
-                Pair<String, String> parsedVersion = 
parsePackageVersion(cli.getArgList().get(1));
-                if (parsedVersion.second() == null) {
-                  throw new SolrException(
-                      ErrorCode.BAD_REQUEST,
-                      "Package name and version are both required. Actual: "
-                          + cli.getArgList().get(1));
-                }
-                String packageName = parsedVersion.first();
-                String version = parsedVersion.second();
-                packageManager.uninstall(packageName, version);
-                break;
-              }
-            default:
-              throw new RuntimeException("Unrecognized command: " + cmd);
-          }
+          handleCommand(command, cmdArgs, packageFlags);
         } finally {
           packageManager.close();
         }
       }
       log.info("Finished: {}", cmd);
 
-    } catch (Exception ex) {
+    } catch (Exception exception) {
       // We need to print this since SolrCLI drops the stack trace in favour
       // of brevity. Package tool should surely print the full stacktrace!
-      ex.printStackTrace();
-      throw ex;
+      exception.printStackTrace();
+      throw exception;
     } finally {
       // Restore the old logging level
       Configurator.setRootLevel(oldLevel);
     }
   }
 
+  private void handleCommand(String command, String[] cmdArgs, PackageFlags 
packageFlags)
+      throws Exception {
+    switch (command) {
+      case "add-repo":
+        String repoName = cmdArgs[0];
+        String repoUrl = cmdArgs[1];
+        repositoryManager.addRepository(repoName, repoUrl);
+        printGreen("Added repository: " + repoName);
+        break;
+      case "add-key":
+        String keyFilename = cmdArgs[0];
+        Path path = Path.of(keyFilename);
+        repositoryManager.addKey(Files.readAllBytes(path), 
path.getFileName().toString());
+        break;
+      case "list-installed":
+        printGreen("Installed packages:\n-----");
+        for (SolrPackageInstance pkg : 
packageManager.fetchInstalledPackageInstances()) {
+          printGreen(pkg);
+        }
+        break;
+      case "list-available":
+        printGreen("Available packages:\n-----");
+        for (SolrPackage pkg : repositoryManager.getPackages()) {
+          printGreen(pkg.name + " \t\t" + pkg.description);
+          for (SolrPackageRelease version : pkg.versions) {
+            printGreen("\tVersion: " + version.version);
+          }
+        }
+        break;
+      case "list-deployed":
+        if (packageFlags.collection() != null) {
+          String collection = packageFlags.collection();
+          Map<String, SolrPackageInstance> packages =
+              packageManager.getPackagesDeployed(collection);
+          printGreen("Packages deployed on " + collection + ":");
+          for (String packageName : packages.keySet()) {
+            printGreen("\t" + packages.get(packageName));
+          }
+        } else {
+          // nuance that we use an arg here instead of requiring a --package 
parameter with a
+          // value
+          // in this code path
+          String packageName = cmdArgs[0];
+          Map<String, String> deployedCollections =
+              packageManager.getDeployedCollections(packageName);
+          if (!deployedCollections.isEmpty()) {
+            printGreen("Collections on which package " + packageName + " was 
deployed:");
+            for (String collection : deployedCollections.keySet()) {
+              printGreen(
+                  "\t"
+                      + collection
+                      + "("
+                      + packageName
+                      + ":"
+                      + deployedCollections.get(collection)
+                      + ")");
+            }
+          } else {
+            printGreen("Package " + packageName + " not deployed on any 
collection.");
+          }
+        }
+        break;
+      case "install":
+        {
+          Pair<String, String> parsedVersion = parsePackageVersion(cmdArgs[0]);
+          String packageName = parsedVersion.first();
+          String version = parsedVersion.second();
+          boolean success = repositoryManager.install(packageName, version);
+          if (success) {
+            printGreen(packageName + " installed.");
+          } else {
+            printRed(packageName + " installation failed.");
+          }
+          break;
+        }
+      case "deploy":
+        {
+          if (packageFlags.cluster() || packageFlags.collections() != null) {
+            Pair<String, String> parsedVersion = 
parsePackageVersion(cmdArgs[0]);
+            String packageName = parsedVersion.first();
+            String version = parsedVersion.second();
+            String[] collections =
+                packageFlags.collections() != null
+                    ? 
PackageUtils.validateCollections(packageFlags.collections().split(","))
+                    : new String[] {};
+            packageManager.deploy(
+                packageName,
+                version,
+                collections,
+                packageFlags.cluster(),
+                packageFlags.parameters(),
+                packageFlags.update(),
+                packageFlags.noPrompt());
+          } else {
+            printRed(
+                "Either specify --cluster to deploy cluster level plugins or 
--collections <list-of-collections> to deploy collection level plugins");
+          }
+          break;
+        }
+      case "undeploy":
+        {
+          if (packageFlags.cluster() || packageFlags.collections() != null) {
+            Pair<String, String> parsedVersion = 
parsePackageVersion(cmdArgs[0]);
+            if (parsedVersion.second() != null) {
+              throw new SolrException(
+                  ErrorCode.BAD_REQUEST,
+                  "Only package name expected, without a version. Actual: " + 
cmdArgs[0]);
+            }
+            String packageName = parsedVersion.first();
+            String[] collections =
+                packageFlags.collections() != null
+                    ? 
PackageUtils.validateCollections(packageFlags.collections().split(","))
+                    : new String[] {};
+            packageManager.undeploy(packageName, collections, 
packageFlags.cluster());
+          } else {
+            printRed(
+                "Either specify --cluster to undeploy cluster level plugins or 
-collections <list-of-collections> to undeploy collection level plugins");

Review Comment:
   Fixed on the picocli Undeploy leaf & this place as well.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to