diff --git a/src/java/org/apache/cassandra/tools/nodetool/InvalidatePermissionsCache.java b/src/java/org/apache/cassandra/tools/nodetool/InvalidatePermissionsCache.java index e1fcc6b26a..7f1372b33d 100644 --- a/src/java/org/apache/cassandra/tools/nodetool/InvalidatePermissionsCache.java +++ b/src/java/org/apache/cassandra/tools/nodetool/InvalidatePermissionsCache.java @@ -38,7 +38,7 @@ import static com.google.common.base.Preconditions.checkArgument; @Command(name = "invalidatepermissionscache", description = "Invalidate the permissions cache") public class InvalidatePermissionsCache extends AbstractCommand { - @Parameters(paramLabel = "role", description = "A role for which permissions to specified resources need to be invalidated", arity = "0..1", index = "0") + @Parameters(paramLabel = "role_name", description = "A role for which permissions to specified resources need to be invalidated", arity = "0..1", index = "0") private String roleName; // Data Resources diff --git a/test/resources/nodetool/help/invalidatepermissionscache b/test/resources/nodetool/help/invalidatepermissionscache index 67566462d1..8d956bdca0 100644 --- a/test/resources/nodetool/help/invalidatepermissionscache +++ b/test/resources/nodetool/help/invalidatepermissionscache @@ -10,7 +10,7 @@ SYNOPSIS [--all-tables] [--function ] [--functions-in-keyspace ] [--keyspace ] [--mbean ] [--role ] - [--table ] [--] + [--table
] [--] OPTIONS --all-functions @@ -69,6 +69,6 @@ OPTIONS list of argument, (useful when arguments might be mistaken for command-line options - + A role for which permissions to specified resources need to be invalidated diff --git a/test/unit/org/apache/cassandra/tools/nodetool/NodetoolClassHierarchyTest.java b/test/unit/org/apache/cassandra/tools/nodetool/NodetoolClassHierarchyTest.java index 4734a45c70..66e764aecf 100644 --- a/test/unit/org/apache/cassandra/tools/nodetool/NodetoolClassHierarchyTest.java +++ b/test/unit/org/apache/cassandra/tools/nodetool/NodetoolClassHierarchyTest.java @@ -20,8 +20,11 @@ package org.apache.cassandra.tools.nodetool; import java.lang.reflect.Field; import java.util.ArrayList; +import java.util.LinkedHashMap; +import java.util.LinkedHashSet; import java.util.List; import java.util.Map; +import java.util.Set; import java.util.TreeMap; import java.util.function.Consumer; @@ -143,6 +146,82 @@ public class NodetoolClassHierarchyTest extends CQLTester failedCommands.isEmpty()); } + /** + * When command arguments are addressed by name rather than by CLI position, all arguments + * of a command share a single flat, case-insensitive key namespace: options are keyed by + * their normalized {@code paramLabel()} and names, positional parameters by their paramLabel. + * No two arguments of the same command may claim the same normalized key, otherwise one value + * is silently overwritten on write or misrouted on read (e.g. a command field and a + * {@code @Mixin} field that produce the same paramLabel). + */ + @Test + public void testCommandArgumentKeysAreUnambiguous() + { + CommandLine root = new CommandLine(NodetoolCommand.class); + Map> affected = new TreeMap<>(); + + commandTreeWalker(root, cmd -> { + List violations = collectAmbiguousArgumentKeys(cmd); + if (!violations.isEmpty()) + affected.put(fullCommandName(cmd), violations); + }); + + assertTrue("The following commands have ambiguous argument keys (options are keyed by " + + "normalized paramLabel and names, positional parameters by paramLabel, all in " + + "one case-insensitive namespace). Rename the field or set an explicit, unique " + + "paramLabel:\n" + + buildAffectedCommandMessage(affected), + affected.isEmpty()); + } + + private static List collectAmbiguousArgumentKeys(CommandLine cmd) + { + List violations = new ArrayList<>(); + Map keyOwners = new LinkedHashMap<>(); + + for (CommandLine.Model.OptionSpec option : cmd.getCommandSpec().options()) + { + if (option.usageHelp() || option.versionHelp()) + continue; + + String owner = "option " + String.join("/", option.names()); + // An option is addressable by its paramLabel and by every name/alias, so all of them + // must stay unambiguous, a set tolerates a paramLabel equal to one of its own names. + Set keys = new LinkedHashSet<>(); + keys.add(normalizeOptionName(option.paramLabel()).toLowerCase()); + for (String name : option.names()) + keys.add(normalizeOptionName(name).toLowerCase()); + + for (String key : keys) + claimArgumentKey(key, owner, keyOwners, violations); + } + + for (CommandLine.Model.PositionalParamSpec param : cmd.getCommandSpec().positionalParameters()) + { + String owner = String.format("parameter index=%s (%s)", param.index(), param.paramLabel()); + claimArgumentKey(normalizeOptionName(param.paramLabel()).toLowerCase(), owner, keyOwners, violations); + } + + return violations; + } + + private static void claimArgumentKey(String key, String owner, Map keyOwners, List violations) + { + String previousOwner = keyOwners.putIfAbsent(key, owner); + if (previousOwner != null) + violations.add(String.format("key '%s' is claimed by both %s and %s", key, previousOwner, owner)); + } + + /** Strips the leading dashes from an option name. */ + private static String normalizeOptionName(String name) + { + if (name.startsWith("--")) + return name.substring(2); + else if (name.startsWith("-")) + return name.substring(1); + return name; + } + /** * For a given command, follows the {@code @ParentCommand} chain and collects all * options and parameters declared on each parent.