From 5909163aab00a70ecf583254f04e441c97ff17bd Mon Sep 17 00:00:00 2001
From: Valery Kharseko <vharseko@3a-systems.ru>
Date: Wed, 29 Jul 2026 14:18:29 +0000
Subject: [PATCH] Remove the never-populated globalArgumentMap from SubCommandArgumentParser (#786)
---
opendj-cli/src/test/java/com/forgerock/opendj/cli/TestSubCommandArgumentParserTestCase.java | 57 ++++++++++++++++++++++++++++
opendj-cli/src/main/java/com/forgerock/opendj/cli/SubCommand.java | 5 --
opendj-cli/src/main/java/com/forgerock/opendj/cli/SubCommandArgumentParser.java | 17 ++------
3 files changed, 63 insertions(+), 16 deletions(-)
diff --git a/opendj-cli/src/main/java/com/forgerock/opendj/cli/SubCommand.java b/opendj-cli/src/main/java/com/forgerock/opendj/cli/SubCommand.java
index e6d3c82..72bfb3c 100644
--- a/opendj-cli/src/main/java/com/forgerock/opendj/cli/SubCommand.java
+++ b/opendj-cli/src/main/java/com/forgerock/opendj/cli/SubCommand.java
@@ -13,6 +13,7 @@
*
* Copyright 2006-2008 Sun Microsystems, Inc.
* Portions Copyright 2014-2016 ForgeRock AS.
+ * Portions Copyright 2026 3A Systems, LLC.
*/
package com.forgerock.opendj.cli;
@@ -226,10 +227,6 @@
throw new ArgumentException(ERR_ARG_SUBCOMMAND_DUPLICATE_ARGUMENT_NAME.get(name, argumentLongID));
}
- if (parser.hasGlobalArgument(argumentLongID)) {
- throw new ArgumentException(ERR_ARG_SUBCOMMAND_ARGUMENT_GLOBAL_CONFLICT.get(argumentLongID, name));
- }
-
Character shortID = argument.getShortIdentifier();
if (shortID != null) {
if (shortIDMap.containsKey(shortID)) {
diff --git a/opendj-cli/src/main/java/com/forgerock/opendj/cli/SubCommandArgumentParser.java b/opendj-cli/src/main/java/com/forgerock/opendj/cli/SubCommandArgumentParser.java
index 06c3b74..495b03b 100644
--- a/opendj-cli/src/main/java/com/forgerock/opendj/cli/SubCommandArgumentParser.java
+++ b/opendj-cli/src/main/java/com/forgerock/opendj/cli/SubCommandArgumentParser.java
@@ -13,7 +13,7 @@
*
* Copyright 2006-2010 Sun Microsystems, Inc.
* Portions Copyright 2011-2016 ForgeRock AS.
- * Portions Copyright 2018-2024 3A Systems, LLC.
+ * Portions Copyright 2018-2026 3A Systems, LLC.
*/
package com.forgerock.opendj.cli;
@@ -63,8 +63,6 @@
private final Map<Character, Argument> globalShortIDMap = new HashMap<>();
/** The set of global arguments defined for this parser, referenced by long ID. */
private final Map<String, Argument> globalLongIDMap = new HashMap<>();
- /** The set of global arguments defined for this parser, referenced by argument name. */
- private final Map<String, Argument> globalArgumentMap = new HashMap<>();
/** The total set of global arguments defined for this parser. */
private final List<Argument> globalArgumentList = new LinkedList<>();
/** The set of subcommands defined for this parser, referenced by subcommand name. */
@@ -98,7 +96,7 @@
* @return <CODE>true</CODE> if a global argument exists with the specified name, or <CODE>false</CODE> if not.
*/
public boolean hasGlobalArgument(String argumentName) {
- return globalArgumentMap.containsKey(argumentName);
+ return getGlobalArgumentForLongID(formatLongIdentifier(argumentName)) != null;
}
/**
@@ -199,9 +197,6 @@
*/
public void addGlobalArgument(Argument argument, ArgumentGroup group) throws ArgumentException {
String longID = argument.getLongIdentifier();
- if (globalArgumentMap.containsKey(longID)) {
- throw new ArgumentException(ERR_SUBCMDPARSER_DUPLICATE_GLOBAL_ARG_NAME.get(longID));
- }
for (SubCommand s : subCommands.values()) {
if (s.getArgumentForLongIdentifier(longID) != null) {
throw new ArgumentException(ERR_SUBCMDPARSER_GLOBAL_ARG_NAME_SUBCMD_CONFLICT.get(
@@ -226,11 +221,9 @@
}
}
- if (!longArgumentsCaseSensitive()) {
- longID = toLowerCase(longID);
- if (globalLongIDMap.containsKey(longID)) {
- throw new ArgumentException(ERR_SUBCMDPARSER_DUPLICATE_GLOBAL_ARG_LONG_ID.get(longID));
- }
+ longID = formatLongIdentifier(longID);
+ if (globalLongIDMap.containsKey(longID)) {
+ throw new ArgumentException(ERR_SUBCMDPARSER_DUPLICATE_GLOBAL_ARG_LONG_ID.get(longID));
}
for (SubCommand s : subCommands.values()) {
diff --git a/opendj-cli/src/test/java/com/forgerock/opendj/cli/TestSubCommandArgumentParserTestCase.java b/opendj-cli/src/test/java/com/forgerock/opendj/cli/TestSubCommandArgumentParserTestCase.java
index 863fa2b..1b7d3db 100644
--- a/opendj-cli/src/test/java/com/forgerock/opendj/cli/TestSubCommandArgumentParserTestCase.java
+++ b/opendj-cli/src/test/java/com/forgerock/opendj/cli/TestSubCommandArgumentParserTestCase.java
@@ -13,6 +13,7 @@
*
* Copyright 2008 Sun Microsystems, Inc.
* Portions Copyright 2014-2016 ForgeRock AS.
+ * Portions Copyright 2026 3A Systems, LLC.
*/
package com.forgerock.opendj.cli;
@@ -157,4 +158,60 @@
SubCommandArgumentParser.indentAndWrap(indent, buffer, wrapColumn, LocalizableMessage.raw(text));
Assertions.assertThat(buffer.toString()).isEqualTo(expected);
}
+
+ private static SubCommandArgumentParser newParser(final boolean longArgumentsCaseSensitive) {
+ return new SubCommandArgumentParser(TestSubCommandArgumentParserTestCase.class.getName(),
+ LocalizableMessage.raw("test description"), longArgumentsCaseSensitive);
+ }
+
+ private static Argument booleanArg(final String longID) throws ArgumentException {
+ return BooleanArgument.builder(longID).description(LocalizableMessage.raw(longID)).buildArgument();
+ }
+
+ /** A registered global argument must be discoverable by its long identifier. */
+ @Test
+ public void testHasGlobalArgument() throws Exception {
+ final SubCommandArgumentParser caseInsensitive = newParser(false);
+ caseInsensitive.addGlobalArgument(booleanArg("globalArg"));
+ Assertions.assertThat(caseInsensitive.hasGlobalArgument("globalArg")).isTrue();
+ Assertions.assertThat(caseInsensitive.hasGlobalArgument("GLOBALARG")).isTrue();
+ Assertions.assertThat(caseInsensitive.hasGlobalArgument("otherArg")).isFalse();
+
+ final SubCommandArgumentParser caseSensitive = newParser(true);
+ caseSensitive.addGlobalArgument(booleanArg("globalArg"));
+ Assertions.assertThat(caseSensitive.hasGlobalArgument("globalArg")).isTrue();
+ Assertions.assertThat(caseSensitive.hasGlobalArgument("GLOBALARG")).isFalse();
+ }
+
+ /** Two global arguments sharing a long identifier must be rejected, whatever the case sensitivity. */
+ @Test
+ public void testDuplicateGlobalArgumentIsRejected() throws Exception {
+ for (final boolean caseSensitive : new boolean[] { false, true }) {
+ final SubCommandArgumentParser aParser = newParser(caseSensitive);
+ aParser.addGlobalArgument(booleanArg("globalArg"));
+ try {
+ aParser.addGlobalArgument(booleanArg("globalArg"));
+ Assert.fail("A duplicate global argument should have been rejected "
+ + "(longArgumentsCaseSensitive=" + caseSensitive + ")");
+ } catch (final ArgumentException expected) {
+ // Expected.
+ }
+ }
+ }
+
+ /** A sub-command argument must not shadow a global argument. */
+ @Test
+ public void testSubCommandArgumentConflictingWithGlobalIsRejected() throws Exception {
+ final SubCommandArgumentParser aParser = newParser(false);
+ aParser.addGlobalArgument(booleanArg("globalArg"));
+
+ final SubCommand subCommand =
+ new SubCommand(aParser, "a-sub-command", LocalizableMessage.raw("a-sub-command"));
+ try {
+ subCommand.addArgument(booleanArg("globalArg"));
+ Assert.fail("A sub-command argument conflicting with a global argument should have been rejected");
+ } catch (final ArgumentException expected) {
+ // Expected.
+ }
+ }
}
--
Gitblit v1.10.0