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.
eclipse-dash · Oct 2, 2024 · 04d95c1 · 04d95c1
1 parent 43125cb
commit 04d95c1
Show file tree

Hide file tree

Showing 4 changed files with 36 additions and 112 deletions.
diff --git a/README.md b/README.md
@@ -560,28 +560,24 @@ The CLI tool does provide help.
 $ java -jar target/org.eclipse.dash.licenses-<version>.jar -help
 usage: org.eclipse.dash.licenses.cli.Main [options] <file> ...
 Sort out the licenses and approval of dependencies.
- -batch <int>                      Batch size (number of entries sent per
-                                   API call)
- -cd,--clearly-defined-api <url>   Clearly Defined API URL
- -confidence <int>                 Confidence threshold expressed as
-                                   integer percent (0-100)
- -ef,--foundation-api <url>        Eclipse Foundation license check API
-                                   URL.
- -excludeSources <sources>         Exclude values from specific sources
- -help,--help                      Display help
- -lic,--licenses <url>             Approved Licenses List URL
- -project <shortname>              Process the request in the context of
-                                   an Eclipse project (e.g.,
-                                   technology.dash)
- -repo <url>                       The Eclipse Project repository that is
-                                   the source of the request
- -review                           Must also specify the project and token
- -summary <file>                   Output a summary to a file
- -timeout <seconds>                Timeout for HTTP calls (in seconds)
- -token <token>                    The GitLab authentication token
+ -batch <int>                Batch size (number of entries sent per API
+                             call)
+ -confidence <int>           Confidence threshold expressed as integer
+                             percent (0-100)
+ -excludeSources <sources>   Exclude values from specific sources
+ -help,--help                Display help
+ -project <shortname>        Process the request in the context of an
+                             Eclipse project (e.g., technology.dash)
+ -repo <url>                 The Eclipse Project repository that is the
+                             source of the request
+ -review                     Must also specify the project and token
+ -summary <file>             Output a summary to a file
+ -timeout <seconds>          Timeout for HTTP calls (in seconds)
+ -token <token>              The GitLab authentication token
 
 <file> 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
@@ -101,7 +101,11 @@ default int getTimeout() {
 	 * @return the token or <code>null</code> 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");
 	}
 }
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
@@ -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
@@ -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" });