From 04d95c1e10738fb8ea81a9b2cd3093e7d1a06464 Mon Sep 17 00:00:00 2001 From: Wayne Beaton Date: Wed, 2 Oct 2024 15:34:55 -0400 Subject: [PATCH] Clean/tighten up the CLI-based settings implementation Updating commons-cli introduced some method resolution ambiguities that this patch cleans up. This commit also removes the CLI options to specify the location of the license file, the ClearlyDefined Server, and the EF server. These options are still available as "-D" configuration via the JVM in ISettings. --- README.md | 36 ++++----- .../org/eclipse/dash/licenses/ISettings.java | 8 +- .../licenses/cli/CommandLineSettings.java | 81 +++---------------- .../tests/CommandLineSettingsTest.java | 23 +----- 4 files changed, 36 insertions(+), 112 deletions(-) diff --git a/README.md b/README.md index d737e954..26dd6e17 100644 --- a/README.md +++ b/README.md @@ -560,28 +560,24 @@ The CLI tool does provide help. $ java -jar target/org.eclipse.dash.licenses-.jar -help usage: org.eclipse.dash.licenses.cli.Main [options] ... Sort out the licenses and approval of dependencies. - -batch Batch size (number of entries sent per - API call) - -cd,--clearly-defined-api Clearly Defined API URL - -confidence Confidence threshold expressed as - integer percent (0-100) - -ef,--foundation-api Eclipse Foundation license check API - URL. - -excludeSources Exclude values from specific sources - -help,--help Display help - -lic,--licenses Approved Licenses List URL - -project Process the request in the context of - an Eclipse project (e.g., - technology.dash) - -repo The Eclipse Project repository that is - the source of the request - -review Must also specify the project and token - -summary Output a summary to a file - -timeout Timeout for HTTP calls (in seconds) - -token The GitLab authentication token + -batch Batch size (number of entries sent per API + call) + -confidence Confidence threshold expressed as integer + percent (0-100) + -excludeSources Exclude values from specific sources + -help,--help Display help + -project Process the request in the context of an + Eclipse project (e.g., technology.dash) + -repo The Eclipse Project repository that is the + source of the request + -review Must also specify the project and token + -summary Output a summary to a file + -timeout Timeout for HTTP calls (in seconds) + -token The GitLab authentication token is the path to a file, or "-" to indicate stdin. -For more help and examples, see https://github.com/eclipse-dash/dash-licenses +For more help and examples, see +https://github.com/eclipse-dash/dash-licenses ``` ### Errors diff --git a/core/src/main/java/org/eclipse/dash/licenses/ISettings.java b/core/src/main/java/org/eclipse/dash/licenses/ISettings.java index febd72b7..45461bf6 100644 --- a/core/src/main/java/org/eclipse/dash/licenses/ISettings.java +++ b/core/src/main/java/org/eclipse/dash/licenses/ISettings.java @@ -101,7 +101,11 @@ default int getTimeout() { * @return the token or null if no value is available. */ default String getIpLabToken() { - return System.getProperty("org.eclipse.dash.token"); + String token = System.getProperty("org.eclipse.dash.token"); + if (token == null) { + return System.getenv("DASH_TOKEN"); + } + return token; } /** @@ -132,6 +136,6 @@ default File getSummaryFile() { } default String getRepository() { - return null; + return System.getProperty("org.eclipse.dash.repo"); } } \ No newline at end of file diff --git a/core/src/main/java/org/eclipse/dash/licenses/cli/CommandLineSettings.java b/core/src/main/java/org/eclipse/dash/licenses/cli/CommandLineSettings.java index cb75bf79..a63ce64f 100644 --- a/core/src/main/java/org/eclipse/dash/licenses/cli/CommandLineSettings.java +++ b/core/src/main/java/org/eclipse/dash/licenses/cli/CommandLineSettings.java @@ -1,5 +1,5 @@ /************************************************************************* - * Copyright (c) 2019,2021 The Eclipse Foundation and others. + * Copyright (c) 2019 The Eclipse Foundation and others. * * This program and the accompanying materials are made available under * the terms of the Eclipse Public License 2.0 which accompanies this @@ -23,10 +23,7 @@ public class CommandLineSettings implements ISettings { private static final String HELP_OPTION = "help"; - private static final String CD_URL_OPTION = "cd"; - private static final String EF_URL_OPTION = "ef"; private static final String TIMEOUT_OPTION = "timeout"; - private static final String APPROVED_LICENSES_URL_OPTION = "lic"; private static final String BATCH_OPTION = "batch"; private static final String CONFIDENCE_OPTION = "confidence"; private static final String SUMMARY_OPTION = "summary"; @@ -41,26 +38,14 @@ public class CommandLineSettings implements ISettings { @Override public int getBatchSize() { - if (!commandLine.hasOption(BATCH_OPTION)) - return ISettings.DEFAULT_BATCH; try { - return ((Number) commandLine.getParsedOptionValue(BATCH_OPTION)).intValue(); + return ((Number) commandLine.getParsedOptionValue(BATCH_OPTION, () -> ISettings.super.getBatchSize())).intValue(); } catch (ParseException e) { // TODO Deal with this throw new RuntimeException(e); } } - @Override - public String getLicenseCheckUrl() { - return commandLine.getOptionValue(EF_URL_OPTION, ISettings.DEFAULT_IPZILLA_URL); - } - - @Override - public String getClearlyDefinedDefinitionsUrl() { - return commandLine.getOptionValue(CD_URL_OPTION, ISettings.DEFAULT_CLEARLYDEFINED_URL); - } - @Override public int getTimeout() { // FIXME This gets called multiple times; consider caching. @@ -75,17 +60,10 @@ public int getTimeout() { } } - @Override - public String getApprovedLicensesUrl() { - return commandLine.getOptionValue(APPROVED_LICENSES_URL_OPTION, ISettings.DEFAULT_APPROVED_LICENSES_URL); - } - @Override public int getConfidenceThreshold() { - if (!commandLine.hasOption(CONFIDENCE_OPTION)) - return ISettings.DEFAULT_THRESHOLD; try { - return ((Number) commandLine.getParsedOptionValue(CONFIDENCE_OPTION)).intValue(); + return ((Number) commandLine.getParsedOptionValue(CONFIDENCE_OPTION, () -> ISettings.super.getConfidenceThreshold())).intValue(); } catch (ParseException e) { // TODO Deal with this throw new RuntimeException(e); @@ -94,12 +72,7 @@ public int getConfidenceThreshold() { @Override public String getIpLabToken() { - String value = commandLine.getOptionValue(TOKEN_OPTION); - if (value == null) { - // FIXME generalize this (maybe rethink the whole settings thing) - value = System.getenv("DASH_TOKEN"); - } - return value; + return commandLine.getOptionValue(TOKEN_OPTION, () -> ISettings.super.getIpLabToken()); } public boolean isValid() { @@ -177,23 +150,6 @@ private static CommandLine getCommandLine(final String[] args) { private static Options getOptions() { final Options options = new Options(); - // @formatter:off - options.addOption(Option.builder(EF_URL_OPTION) - .longOpt("foundation-api") - .required(false) - .hasArg() - .argName("url") - .desc("Eclipse Foundation license check API URL.") - .build()); - - options.addOption(Option.builder(CD_URL_OPTION) - .longOpt("clearly-defined-api") - .required(false) - .hasArg() - .argName("url") - .desc("Clearly Defined API URL") - .build()); - options.addOption(Option.builder(TIMEOUT_OPTION) .required(false) .hasArg() @@ -202,14 +158,6 @@ private static Options getOptions() { .desc("Timeout for HTTP calls (in seconds)") .build()); - options.addOption(Option.builder(APPROVED_LICENSES_URL_OPTION) - .longOpt("licenses") - .required(false) - .hasArg() - .argName("url") - .desc("Approved Licenses List URL") - .build()); - options.addOption(Option.builder(BATCH_OPTION) .required(false) .hasArg() @@ -241,11 +189,11 @@ private static Options getOptions() { .build()); options.addOption(Option.builder(EXCLUDE_SOURCES_OPTION) - .required(false) - .hasArg(true) - .argName("sources") - .desc("Exclude values from specific sources") - .build()); + .required(false) + .hasArg(true) + .argName("sources") + .desc("Exclude values from specific sources") + .build()); options.addOption(Option.builder(TOKEN_OPTION) .required(false) @@ -277,7 +225,6 @@ private static Options getOptions() { .hasArg(false) .desc("Display help") .build()); - // @formatter:on return options; } @@ -304,22 +251,20 @@ public static void printHelp(PrintStream out) { @Override public String getSummaryFilePath() { - return commandLine.getOptionValue(SUMMARY_OPTION, null); + return commandLine.getOptionValue(SUMMARY_OPTION, () -> ISettings.super.getSummaryFilePath()); } @Override public String getProjectId() { - return commandLine.getOptionValue(PROJECT_OPTION, null); + return commandLine.getOptionValue(PROJECT_OPTION, () -> ISettings.super.getProjectId()); } @Override public String getRepository() { - var repository = commandLine.getOptionValue(REPO_OPTION, ""); - if (repository.isBlank()) return null; - return repository; + return commandLine.getOptionValue(REPO_OPTION, () -> ISettings.super.getRepository()); } public String getExcludedSources() { - return commandLine.getOptionValue(EXCLUDE_SOURCES_OPTION); + return commandLine.getOptionValue(EXCLUDE_SOURCES_OPTION, ""); } } diff --git a/core/src/test/java/org/eclipse/dash/licenses/tests/CommandLineSettingsTest.java b/core/src/test/java/org/eclipse/dash/licenses/tests/CommandLineSettingsTest.java index 0dc52a50..df3ae26e 100644 --- a/core/src/test/java/org/eclipse/dash/licenses/tests/CommandLineSettingsTest.java +++ b/core/src/test/java/org/eclipse/dash/licenses/tests/CommandLineSettingsTest.java @@ -35,28 +35,7 @@ void testInvalidBatchSize() { CommandLineSettings settings = CommandLineSettings.getSettings(new String[] { "-batch", "xx" }); assertFalse(settings.isValid()); } - - @Test - void testCustomLicenseCheckUrl() { - String url = "http://localhost/license.php"; - ISettings settings = CommandLineSettings.getSettings(new String[] { "-ef", url }); - assertEquals(url, settings.getLicenseCheckUrl()); - } - - @Test - void testCustomClearlyDefinedDefinitionsUrl() { - String url = "http://localhost/license.php"; - ISettings settings = CommandLineSettings.getSettings(new String[] { "-cd", url }); - assertEquals(url, settings.getClearlyDefinedDefinitionsUrl()); - } - - @Test - void testCustomApprovedLicensesUrl() { - String url = "http://localhost/license.php"; - ISettings settings = CommandLineSettings.getSettings(new String[] { "-lic", url }); - assertEquals(url, settings.getApprovedLicensesUrl()); - } - + @Test void testCustomConfidence() { ISettings settings = CommandLineSettings.getSettings(new String[] { "-confidence", "42" });