From f5f71b7fae4e184debc24ea3d9375afb8491df41 Mon Sep 17 00:00:00 2001 From: Stefan Miklosovic Date: Mon, 21 Jul 2025 15:07:00 +0200 Subject: [PATCH] Rework / simplification of nodetool get/setguardrailsconfig commands patch by Stefan Miklosovic; reviewed by Maxim Muzafarov for CASSANDRA-20778 --- CHANGES.txt | 1 + .../nodetool/GuardrailsConfigCommand.java | 350 +++++++++++------- .../GuardrailsConfigCommandsTest.java | 214 +++++------ 3 files changed, 317 insertions(+), 248 deletions(-) diff --git a/CHANGES.txt b/CHANGES.txt index 5e4703f828..8958d7d02d 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,4 +1,5 @@ 4.1.10 + * Rework / simplification of nodetool get/setguardrailsconfig commands (CASSANDRA-20778) * IntrusiveStack.accumulate is not accumulating correctly (CASSANDRA-20670) * Add nodetool get/setguardrailsconfig commands (CASSANDRA-19552) Merged from 4.0: diff --git a/src/java/org/apache/cassandra/tools/nodetool/GuardrailsConfigCommand.java b/src/java/org/apache/cassandra/tools/nodetool/GuardrailsConfigCommand.java index 7e1d1e3400..e9aa5f8ef5 100644 --- a/src/java/org/apache/cassandra/tools/nodetool/GuardrailsConfigCommand.java +++ b/src/java/org/apache/cassandra/tools/nodetool/GuardrailsConfigCommand.java @@ -22,12 +22,14 @@ import java.io.PrintStream; import java.lang.reflect.Method; import java.util.ArrayList; import java.util.Arrays; +import java.util.Collections; import java.util.HashMap; import java.util.HashSet; import java.util.LinkedHashMap; import java.util.LinkedHashSet; import java.util.List; import java.util.Map; +import java.util.Objects; import java.util.Set; import java.util.function.Function; import java.util.regex.Pattern; @@ -59,7 +61,11 @@ public abstract class GuardrailsConfigCommand extends NodeTool.NodeToolCmd allowedValues = { "values", "thresholds", "flags", "others" }) private String guardrailCategory; - @Arguments(description = "Specific names or guardrails to get configuration of.") + @Option(name = { "--expand" }, + description = "Expand all guardrail names so they reflect their counterparts in cassandra.yaml") + private boolean expand = false; + + @Arguments(description = "Specific name of a guardrail to get configuration of.") private List args = new ArrayList<>(); @Override @@ -67,43 +73,100 @@ public abstract class GuardrailsConfigCommand extends NodeTool.NodeToolCmd { GuardrailCategory categoryEnum = GuardrailCategory.parseCategory(guardrailCategory, probe.output().out); - if (!args.isEmpty() && categoryEnum != null) + if (args.size() > 1) + throw new IllegalStateException("Specify only one guardrail name to get the configuration of or no name to get the configuration of all of them."); + + String guardrailName = !args.isEmpty() ? args.get(0) : null; + + if (guardrailName != null && categoryEnum != null) throw new IllegalStateException("Do not specify additional arguments when --category/-c is set."); - List allGetters = stream(probe.getGuardrailsMBean().getClass().getDeclaredMethods()) - .filter(method -> method.getName().startsWith("get") - && !method.getName().endsWith("CSV")) - .filter(method -> args.isEmpty() || args.contains(toSnakeCase(method.getName().substring(3)))) - .sorted(comparing(Method::getName)) - .collect(toList()); + Map> allGetters = parseGuardrailNames(probe.getGuardrailsMBean().getClass().getDeclaredMethods(), guardrailName); - display(probe, allGetters, categoryEnum); + if (allGetters.isEmpty()) + { + assert guardrailName != null; + throw new IllegalStateException(format("Guardrail %s not found.", guardrailName)); + } + + display(probe, allGetters, categoryEnum, expand); + } + + @VisibleForTesting + public static Map> parseGuardrailNames(Method[] guardrailsMethods, String guardrailName) + { + Map> allGetters = stream(guardrailsMethods) + .filter(method -> method.getName().startsWith("get") + && !method.getName().endsWith("CSV") + && !(method.getName().endsWith("WarnThreshold") || method.getName().endsWith("FailThreshold"))) + .filter(method -> guardrailName == null || guardrailName.equals(toSnakeCase(method.getName().substring(3)))) + .collect(Collectors.groupingBy(method -> toSnakeCase(method.getName().substring(3)))); + + Map> thresholds = stream(guardrailsMethods) + .filter(method -> method.getName().startsWith("get") + && !method.getName().endsWith("CSV") + && (method.getName().endsWith("WarnThreshold") || method.getName().endsWith("FailThreshold"))) + .filter(method -> { + if (guardrailName == null) + return true; + + String snakeCase = toSnakeCase(method.getName().substring(3)); + String snakeCaseSuccinct = snakeCase.replace("_warn_", "_") + .replace("_fail_", "_"); + + return guardrailName.equals(snakeCase) || guardrailName.equals(snakeCaseSuccinct); + }) + .sorted(comparing(Method::getName)) + .collect(Collectors.groupingBy(method -> { + String methodName = method.getName().substring(3); + String snakeCase = toSnakeCase(methodName); + if (snakeCase.endsWith("warn_threshold")) + return snakeCase.replaceAll("_warn_", "_"); + else + return snakeCase.replaceAll("_fail_", "_"); + })); + + allGetters.putAll(thresholds); + + return allGetters.entrySet() + .stream() + .sorted(Map.Entry.comparingByKey()) + .collect(Collectors.toMap(Map.Entry::getKey, + Map.Entry::getValue, + (e1, e2) -> e1, + LinkedHashMap::new)); } @Override - public void addRow(List bucket, GuardrailsMBean mBean, Method method, String guardrailName) throws Throwable + public void addRow(List bucket, GuardrailsMBean mBean, List methods, String guardrailName) throws Throwable { - Class returnType = method.getReturnType(); - Object value = method.invoke(mBean); + List values = new ArrayList<>(); + for (Method method : methods) + { + Class returnType = method.getReturnType(); + Object value = method.invoke(mBean); - if (returnType.equals(int.class) || returnType.equals(Integer.class) - || returnType.equals(long.class) || returnType.equals(Long.class) - || returnType.equals(boolean.class) || returnType.equals(Boolean.class) - || returnType.equals(Set.class)) - { - constructRow(bucket, guardrailName, value.toString()); - } - else if (returnType.equals(String.class)) - { - if (value == null || value.toString().isEmpty()) - constructRow(bucket, guardrailName, "null"); + if (returnType.equals(int.class) || returnType.equals(Integer.class) + || returnType.equals(long.class) || returnType.equals(Long.class) + || returnType.equals(boolean.class) || returnType.equals(Boolean.class) + || returnType.equals(Set.class)) + { + values.add(value.toString()); + } + else if (returnType.equals(String.class)) + { + if (value == null || value.toString().isEmpty()) + values.add("null"); + else + values.add(value.toString()); + } else - constructRow(bucket, guardrailName, value.toString()); - } - else - { - throw new RuntimeException("unhandled return type: " + returnType.getTypeName()); + { + throw new RuntimeException("Unhandled return type: " + returnType.getTypeName()); + } } + + constructRow(bucket, guardrailName, values.size() == 1 ? values.get(0) : values.toString()); } } @@ -112,78 +175,34 @@ public abstract class GuardrailsConfigCommand extends NodeTool.NodeToolCmd { private static final Pattern SETTER_PATTERN = Pattern.compile("^set"); - @Option(name = { "--list", "-l" }, - description = "List all available guardrails setters") - private boolean list; - - @Option(name = { "--category", "-c" }, - description = "Category of guardrails to filter, can be one of 'values', 'thresholds', 'flags', 'others'.", - allowedValues = { "values", "thresholds", "flags", "others" }) - private String guardrailCategory; - @Arguments(usage = "[ ...]", description = "For flags, possible values are 'true' or 'false'. " + - "For thresholds, two values are expected, first for warning, second for failure. " + - "For values, one value is expected, multiple values separated by comma.") + "For thresholds, two values are expected, first for failure, second for warning. " + + "For values, enumeration of values expected or one value where multiple items are separated by comma. " + + "Setting for thresholds accepting strings and value guardrails are reset by specifying 'null' or '[]' value. " + + "For thresholds accepting integers, the reset value is -1.") private final List args = new ArrayList<>(); @Override public void execute(NodeProbe probe) { - if (!list && guardrailCategory != null) - throw new IllegalStateException("--category/-c can be specified only together with --list/-l"); - - GuardrailCategory categoryEnum = GuardrailCategory.parseCategory(guardrailCategory, probe.output().out); - - if (args.isEmpty() && !list) + if (args.isEmpty()) throw new IllegalStateException("No arguments."); - if (list) - display(probe, getAllSetters(probe), categoryEnum); - else - executeSetter(probe); - } - - @Override - public void addRow(List bucket, GuardrailsMBean mBean, Method method, String guardrailName) throws Throwable - { - if (method.getParameterTypes().length == 1) - constructRow(bucket, sanitizeSetterName(method), method.getParameterTypes()[0].getName()); - else - constructRow(bucket, sanitizeSetterName(method), stream(method.getParameterTypes()).map(Class::getName).collect(toList()).toString()); - } - - private List getAllSetters(NodeProbe probe) - { - return stream(probe.getGuardrailsMBean().getClass().getDeclaredMethods()) - .filter(method -> method.getName().startsWith("set") && !method.getName().endsWith("CSV")) - .filter(method -> args.isEmpty() || args.contains(toSnakeCase(method.getName().substring(3)))) - .sorted(comparing(Method::getName)) - .collect(toList()); - } - - private String sanitizeSetterName(Method setter) - { - return toSnakeCase(SETTER_PATTERN.matcher(setter.getName()).replaceAll("")); - } - - private void executeSetter(NodeProbe nodeProbe) - { String snakeCaseName = args.get(0); - String setterName = toCamelCase(args.get(0).startsWith("set_") ? args.get(0) : "set_" + args.get(0)); - Method setter = getAllSetters(nodeProbe).stream() + Method setter = getAllSetters(probe).entrySet().stream() .findFirst() - .orElseThrow(() -> new IllegalStateException(format("Setter method %s not found. " + - "Run nodetool setguardrailsconfig --list " + - "to see available setters", setterName))); + .map(o -> o.getValue().get(0)) + .orElseThrow(() -> new IllegalStateException(format("Guardrail %s not found.", snakeCaseName))); + sanitizeArguments(setter, args); validateArguments(setter, snakeCaseName, args); List methodArgs = args.subList(1, args.size()); try { - setter.invoke(nodeProbe.getGuardrailsMBean(), prepareArguments(methodArgs, setter)); + setter.invoke(probe.getGuardrailsMBean(), prepareArguments(methodArgs, setter)); } catch (Exception ex) { @@ -198,6 +217,57 @@ public abstract class GuardrailsConfigCommand extends NodeTool.NodeToolCmd } } + @Override + public void addRow(List bucket, GuardrailsMBean mBean, List methods, String guardrailName) throws Throwable + { + if (methods.size() == 1) + { + Method method = methods.get(0); + if (method.getParameterTypes().length == 1) + constructRow(bucket, sanitizeSetterName(method), method.getParameterTypes()[0].getName()); + else + constructRow(bucket, sanitizeSetterName(method), stream(method.getParameterTypes()).map(Class::getName).collect(toList()).toString()); + } + } + + private Map> getAllSetters(NodeProbe probe) + { + return stream(probe.getGuardrailsMBean().getClass().getDeclaredMethods()) + .filter(method -> method.getName().startsWith("set") && !method.getName().endsWith("CSV")) + .filter(method -> args.isEmpty() || args.contains(toSnakeCase(method.getName().substring(3)))) + .sorted(comparing(Method::getName)) + .collect(Collectors.groupingBy(method -> toSnakeCase(method.getName().substring(3)))) + .entrySet() + .stream() + .sorted(Map.Entry.comparingByKey()) + .collect(Collectors.toMap(Map.Entry::getKey, + Map.Entry::getValue, + (e1, e2) -> e1, + LinkedHashMap::new)); + } + + private String sanitizeSetterName(Method setter) + { + return toSnakeCase(SETTER_PATTERN.matcher(setter.getName()).replaceAll("")); + } + + private void sanitizeArguments(Method setter, List args) + { + Class[] parameterTypes = setter.getParameterTypes(); + if (parameterTypes.length == 1 && parameterTypes[0] == Set.class) + { + if (args.size() > 2) + { + String guardrail = args.get(0); + // replace multiple arguments with one which is separated by a single comma + String collectedArguments = String.join(",", args.subList(1, args.size())); + args.clear(); + args.add(guardrail); + args.add(collectedArguments); + } + } + } + private void validateArguments(Method setter, String setterName, List args) { if (args.size() != setter.getParameterCount() + 1) @@ -217,6 +287,13 @@ public abstract class GuardrailsConfigCommand extends NodeTool.NodeToolCmd for (int i = 0; i < args.size(); i++) arguments[i] = castType(parameterTypes[i], args.get(i)); + if (method.getName().endsWith("Threshold")) + { + List thresholdArgs = Arrays.asList(arguments); + Collections.reverse(thresholdArgs); + arguments = thresholdArgs.toArray(); + } + return arguments; } @@ -239,12 +316,10 @@ public abstract class GuardrailsConfigCommand extends NodeTool.NodeToolCmd } else if (targetType == Set.class) { - if (value == null || value.equals("null")) + if (value == null || value.equals("null") || value.equals("[]")) return new HashSet<>(); else - { return new LinkedHashSet<>(Arrays.asList(value.split(","))); - } } else { @@ -281,11 +356,6 @@ public abstract class GuardrailsConfigCommand extends NodeTool.NodeToolCmd put("FieldsPerUDTThreshold", "fields_per_udt_threshold"); }}; - private static final Map toCamelCaseTranslationMap = new HashMap() - {{ - put("set_fields_per_udt_threshold", "setFieldsPerUDTThreshold"); - }}; - @VisibleForTesting public enum GuardrailCategory { @@ -316,7 +386,7 @@ public abstract class GuardrailsConfigCommand extends NodeTool.NodeToolCmd } } - void display(NodeProbe probe, List methods, GuardrailCategory userCategory) + void display(NodeProbe probe, Map> methods, GuardrailCategory userCategory, boolean verbose) { try { @@ -325,24 +395,37 @@ public abstract class GuardrailsConfigCommand extends NodeTool.NodeToolCmd List values = new ArrayList<>(); List others = new ArrayList<>(); - for (Method method : methods) + for (Map.Entry> entry : methods.entrySet()) { - String guardrailName = toSnakeCase(method.getName().substring(3)); - + String key = entry.getKey(); List bucket; - if (guardrailName.endsWith("_enabled")) + if (key.endsWith("_enabled")) bucket = flags; - else if (guardrailName.endsWith("_threshold")) - bucket = thresholds; - else if (guardrailName.endsWith("_disallowed") || - guardrailName.endsWith("_ignored") || - guardrailName.endsWith("_warned")) + else if (key.endsWith("_threshold")) + { + if (!verbose) + { + addRow(thresholds, probe.getGuardrailsMBean(), entry.getValue(), entry.getKey()); + } + else + { + for (Method method : entry.getValue()) + { + String guardrailName = toSnakeCase(method.getName().substring(3)); + addRow(thresholds, probe.getGuardrailsMBean(), method, guardrailName); + } + } + continue; + } + else if (key.endsWith("_disallowed") || + key.endsWith("_ignored") || + key.endsWith("_warned")) bucket = values; else bucket = others; - addRow(bucket, probe.getGuardrailsMBean(), method, guardrailName); + addRow(bucket, probe.getGuardrailsMBean(), entry.getValue().get(0), key); } TableBuilder tb = new TableBuilder(); @@ -396,7 +479,14 @@ public abstract class GuardrailsConfigCommand extends NodeTool.NodeToolCmd bucket.add(new InternalRow(guardrailName, value)); } - abstract void addRow(List bucket, GuardrailsMBean mBean, Method method, String guardrailName) throws Throwable; + void addRow(List bucket, GuardrailsMBean mBean, Method method, String guardrailName) throws Throwable + { + List methods = new ArrayList<>(); + methods.add(method); + addRow(bucket, mBean, methods, guardrailName); + } + + abstract void addRow(List bucket, GuardrailsMBean mBean, List method, String guardrailName) throws Throwable; public static class InternalRow { @@ -408,9 +498,34 @@ public abstract class GuardrailsConfigCommand extends NodeTool.NodeToolCmd this.name = name; this.value = value; } + + @Override + public boolean equals(Object o) + { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + InternalRow that = (InternalRow) o; + return Objects.equals(name, that.name) && Objects.equals(value, that.value); + } + + @Override + public int hashCode() + { + return Objects.hash(name, value); + } + + @Override + public String toString() + { + return "InternalRow{" + + "name='" + name + '\'' + + ", value='" + value + '\'' + + '}'; + } } - private static String toSnakeCase(String camelCase) + @VisibleForTesting + public static String toSnakeCase(String camelCase) { if (camelCase == null || camelCase.isEmpty()) return camelCase; @@ -423,33 +538,4 @@ public abstract class GuardrailsConfigCommand extends NodeTool.NodeToolCmd return CAMEL_PATTERN.matcher(camelCase).replaceAll("$1_$2").toLowerCase(); } } - - private static String toCamelCase(String snakeCase) - { - if (snakeCase == null || snakeCase.isEmpty()) - return snakeCase; - - String maybeCamelCase = toCamelCaseTranslationMap.get(snakeCase); - if (maybeCamelCase != null) - return maybeCamelCase; - - StringBuilder result = new StringBuilder(); - boolean toUpper = false; - - for (int i = 0; i < snakeCase.length(); i++) - { - char c = snakeCase.charAt(i); - if (c == '_') - { - toUpper = true; - } - else - { - result.append(toUpper ? Character.toUpperCase(c) : c); - toUpper = false; - } - } - - return result.toString(); - } } diff --git a/test/unit/org/apache/cassandra/tools/nodetool/GuardrailsConfigCommandsTest.java b/test/unit/org/apache/cassandra/tools/nodetool/GuardrailsConfigCommandsTest.java index a4320da548..683f2a6c07 100644 --- a/test/unit/org/apache/cassandra/tools/nodetool/GuardrailsConfigCommandsTest.java +++ b/test/unit/org/apache/cassandra/tools/nodetool/GuardrailsConfigCommandsTest.java @@ -18,23 +18,29 @@ package org.apache.cassandra.tools.nodetool; -import java.util.ArrayList; +import java.lang.reflect.Field; +import java.lang.reflect.Method; +import java.lang.reflect.Modifier; import java.util.Arrays; +import java.util.HashSet; import java.util.List; +import java.util.Map; +import java.util.Set; import org.junit.BeforeClass; import org.junit.Test; +import org.apache.cassandra.config.Config; import org.apache.cassandra.cql3.CQLTester; +import org.apache.cassandra.db.guardrails.GuardrailsMBean; import org.apache.cassandra.tools.ToolRunner.ToolResult; -import org.apache.cassandra.tools.nodetool.GuardrailsConfigCommand.GuardrailCategory; +import org.apache.cassandra.tools.nodetool.GuardrailsConfigCommand.GetGuardrailsConfig; import static org.apache.cassandra.tools.ToolRunner.invokeNodetool; import static org.junit.Assert.assertArrayEquals; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; -import static org.junit.Assert.fail; public class GuardrailsConfigCommandsTest extends CQLTester { @@ -52,6 +58,10 @@ public class GuardrailsConfigCommandsTest extends CQLTester getResult.asserts().success(); assertEquals(removeMultipleSpaces(ALL_GUARDRAILS_GETTER_OUTPUT), getOutput(getResult)); + ToolResult getResultVerbose = invokeNodetool("getguardrailsconfig", "--expand"); + getResultVerbose.asserts().success(); + assertEquals(removeMultipleSpaces(ALL_GUARDRAILS_GETTER_VERBOSE_OUTPUT), getOutput(getResultVerbose)); + ToolResult getFlagsResult = invokeNodetool("getguardrailsconfig", "-c", "flags"); getFlagsResult.asserts().success(); assertEquals(removeMultipleSpaces(ALL_FLAGS_GETTER_OUTPUT), getOutput(getFlagsResult)); @@ -61,7 +71,7 @@ public class GuardrailsConfigCommandsTest extends CQLTester assertEquals(removeMultipleSpaces(ALL_VALUES_GETTER_OUTPUT), getOutput(getValuesResult)); ToolResult getThresholdsResult = invokeNodetool("getguardrailsconfig", "-c", "thresholds"); - getValuesResult.asserts().success(); + getThresholdsResult.asserts().success(); assertEquals(removeMultipleSpaces(ALL_THRESHOLDS_GETTER_OUTPUT), getOutput(getThresholdsResult)); ToolResult wrongCategory = invokeNodetool("getguardrailsconfig", "-c", "nonsense"); @@ -70,84 +80,26 @@ public class GuardrailsConfigCommandsTest extends CQLTester // individual guardrail ToolResult individualResult = invokeNodetool("getguardrailsconfig", "group_by_enabled"); - getResult.asserts().success(); + individualResult.asserts().success(); assertEquals("true\n", getOutput(individualResult)); - // more than one result + // more than one guardrail ToolResult multipleResult = invokeNodetool("getguardrailsconfig", "group_by_enabled", "keyspaces_fail_threshold"); - getResult.asserts().success(); - assertEquals("group_by_enabled true\nkeyspaces_fail_threshold -1\n", getOutput(multipleResult)); + multipleResult.asserts().failure(); + assertTrue(getOutput(multipleResult).contains("Specify only one guardrail name to get the configuration of or no name to get the configuration of all of them.")); // category with individual ToolResult categoryWithIndividualResult = invokeNodetool("getguardrailsconfig", "-c", "values", "group_by_enabled"); categoryWithIndividualResult.asserts().failure(); assertTrue(categoryWithIndividualResult.getStdout().contains("Do not specify additional arguments when --category/-c is set.")); - // get config of all guardrails enumerated on command line - String[] allLines = ALL_GUARDRAILS_GETTER_OUTPUT.split("\n"); - - List argsForAllGuardrails = new ArrayList<>(); - argsForAllGuardrails.add("getguardrailsconfig"); - - for (String line : allLines) - argsForAllGuardrails.add(line.split(" ")[0]); - - ToolResult allGuardrails = invokeNodetool(argsForAllGuardrails); - allGuardrails.asserts().success(); - assertEquals(ALL_GUARDRAILS_GETTER_OUTPUT, removeMultipleSpaces(allGuardrails.getStdout())); - // set ToolResult setResultNoArgs = invokeNodetool("setguardrailsconfig"); setResultNoArgs.asserts().failure(); assertTrue(getOutput(setResultNoArgs).contains("No arguments.")); - ToolResult setResultList = invokeNodetool("setguardrailsconfig", "--list"); - setResultList.asserts().success(); - setResultList = invokeNodetool("setguardrailsconfig", "-l"); - setResultList.asserts().success(); - assertEquals(removeMultipleSpaces(ALL_GUARDRAILS_SETTER_OUTPUT), getOutput(setResultList)); - - ToolResult categoryList = invokeNodetool("setguardrailsconfig", "--list", "--category", "values"); - categoryList.asserts().success(); - - for (GuardrailCategory category : GuardrailCategory.values()) - { - categoryList = invokeNodetool("setguardrailsconfig", "-l", "-c", category.name()); - categoryList.asserts().success(); - - String expectedOutput = null; - switch (category) - { - case flags: - expectedOutput = ALL_FLAGS_SETTER_OUTPUT; - break; - case values: - expectedOutput = ALL_VALUES_SETTER_OUTPUT; - break; - case thresholds: - expectedOutput = ALL_THRESHOLDS_SETTER_OUTPUT; - break; - case others: - expectedOutput = ALL_OTHER_SETTER_OUTPUT; - break; - default: - fail("Untested category " + category); - } - assertEquals(removeMultipleSpaces(expectedOutput), getOutput(categoryList)); - } - - // test -c without -l does not make sense - ToolResult emptyCategoryListing = invokeNodetool("setguardrailsconfig", "-l", "-c"); - emptyCategoryListing.asserts().failure(); - assertTrue(getOutput(emptyCategoryListing).contains("Required values for option 'guardrailCategory' not provided")); - - // test -c alone does not make sense - ToolResult onlyCategory = invokeNodetool("setguardrailsconfig", "-c", "values"); - onlyCategory.asserts().failure(); - assertTrue(getOutput(onlyCategory).contains("--category/-c can be specified only together with --list/-l")); - - // it would be quite cumbersome to test all guardrails are settable so we will + // it would be quite cumbersome to test all guardrails are settable, so we will // set one from each category to prove the point // flag @@ -161,9 +113,13 @@ public class GuardrailsConfigCommandsTest extends CQLTester assertArrayEquals(new String[]{ "comment", "cdc" }, getValues("table_properties_warned")); setValues("table_properties_warned", "null"); assertArrayEquals(new String[0], getValues("table_properties_warned")); + setValues("table_properties_warned", "comment", "cdc"); + assertArrayEquals(new String[]{ "comment", "cdc" }, getValues("table_properties_warned")); + setValues("table_properties_warned", "[]"); + assertArrayEquals(new String[0], getValues("table_properties_warned")); // threshold - setThresholds("keyspaces_threshold", "10", "20"); + setThresholds("keyspaces_threshold", "20", "10"); assertEquals("20", getThreshold("keyspaces_fail_threshold")); assertEquals("10", getThreshold("keyspaces_warn_threshold")); setThresholds("keyspaces_threshold", "-1", "-1"); @@ -175,10 +131,13 @@ public class GuardrailsConfigCommandsTest extends CQLTester invalidNumberOfArgsForThreshold.asserts().failure(); assertTrue(invalidNumberOfArgsForThreshold.getStdout().contains("keyspaces_threshold is expecting 2 argument values. Getting 3 instead.")); - // not separated by comma - ToolResult invalidNumberOfArgsForValues = invokeNodetool("setguardrailsconfig", "table_properties_warned", "comment", "cdc"); - invalidNumberOfArgsForValues.asserts().failure(); - assertTrue(invalidNumberOfArgsForValues.getStdout().contains("table_properties_warned is expecting 1 argument values. Getting 2 instead.")); + // separated by comma + ToolResult argumentsForValuesSeparatedByComma = invokeNodetool("setguardrailsconfig", "table_properties_warned", "comment,cdc"); + argumentsForValuesSeparatedByComma.asserts().success(); + + // enumerated + ToolResult argumentsForValuesEnumerated = invokeNodetool("setguardrailsconfig", "table_properties_warned", "comment", "cdc"); + argumentsForValuesEnumerated.asserts().success(); // invalid boolean ToolResult invalidBooleanForFlags = invokeNodetool("setguardrailsconfig", "allow_filtering_enabled", "nonsense"); @@ -189,7 +148,52 @@ public class GuardrailsConfigCommandsTest extends CQLTester ToolResult nonsenseSetterArgs = invokeNodetool("setguardrailsconfig", "keyspaces_threshold", "-10", "-20"); nonsenseSetterArgs.asserts().failure(); assertTrue(nonsenseSetterArgs.getStdout().contains("Error occured when setting the config for setter keyspaces_threshold with arguments [-10, -20]: " + - "Invalid value -10 for keyspaces_warn_threshold: negative values are not allowed, outside of -1 which disables the guardrail")); + "Invalid value -20 for keyspaces_warn_threshold: negative values are not allowed, outside of -1 which disables the guardrail")); + + // invalid set name + ToolResult setInvalidName = invokeNodetool("setguardrailsconfig", "non_sense", "10"); + setInvalidName.asserts().failure(); + assertTrue(setInvalidName.getStdout().contains("Guardrail non_sense not found.")); + + // invalid get name + ToolResult getInvalidName = invokeNodetool("getguardrailsconfig", "non_sense"); + getInvalidName.asserts().failure(); + assertTrue(getInvalidName.getStdout().contains("Guardrail non_sense not found.")); + } + + @Test + public void testParsedGuardrailNamesFromMBeanExistInCassandraYaml() + { + Set configFieldNames = getConfigFieldNames(); + Map> snakeCaseGuardrailsMap = GetGuardrailsConfig.parseGuardrailNames(GuardrailsMBean.class.getDeclaredMethods(), null); + + for (Map.Entry> entry : snakeCaseGuardrailsMap.entrySet()) + { + for (Method method : entry.getValue()) + { + String guardrailName = GuardrailsConfigCommand.toSnakeCase(method.getName().substring(3)); + if (entry.getValue().size() == 1) + assertEquals(entry.getKey(), guardrailName); + // else it is threshold, so it does not match the key + + // assert converted snake-case guardrail name is actually in Config / cassandra.yaml + assertTrue(configFieldNames.contains(guardrailName)); + } + } + } + + private Set getConfigFieldNames() + { + Set variableNames = new HashSet<>(); + for (Field field : Config.class.getFields()) + { + // ignore the constants + if (Modifier.isFinal(field.getModifiers())) + continue; + variableNames.add(field.getName()); + } + + return variableNames; } private static final String ALL_FLAGS_GETTER_OUTPUT = @@ -203,6 +207,21 @@ public class GuardrailsConfigCommandsTest extends CQLTester "user_timestamps_enabled true\n"; private static final String ALL_THRESHOLDS_GETTER_OUTPUT = + "collection_size_threshold [null, null]\n" + + "columns_per_table_threshold [-1, -1] \n" + + "data_disk_usage_percentage_threshold [-1, -1] \n" + + "fields_per_udt_threshold [-1, -1] \n" + + "in_select_cartesian_product_threshold [-1, -1] \n" + + "items_per_collection_threshold [-1, -1] \n" + + "keyspaces_threshold [-1, -1] \n" + + "materialized_views_per_table_threshold [-1, -1] \n" + + "minimum_replication_factor_threshold [-1, -1] \n" + + "page_size_threshold [-1, -1] \n" + + "partition_keys_in_select_threshold [-1, -1] \n" + + "secondary_indexes_per_table_threshold [-1, -1] \n" + + "tables_threshold [-1, -1] \n"; + + private static final String ALL_THRESHOLDS_GETTER_VERBOSE_OUTPUT = "collection_size_fail_threshold null\n" + "collection_size_warn_threshold null\n" + "columns_per_table_fail_threshold -1 \n" + @@ -242,53 +261,16 @@ public class GuardrailsConfigCommandsTest extends CQLTester private static final String ALL_OTHER_GETTER_OUTPUT = "data_disk_usage_max_disk_size null \n"; - private static final String ALL_FLAGS_SETTER_OUTPUT = - "allow_filtering_enabled boolean\n" + - "compact_tables_enabled boolean\n" + - "drop_truncate_table_enabled boolean\n" + - "group_by_enabled boolean\n" + - "read_before_write_list_operations_enabled boolean\n" + - "secondary_indexes_enabled boolean\n" + - "uncompressed_tables_enabled boolean\n" + - "user_timestamps_enabled boolean\n"; - - private static final String ALL_THRESHOLDS_SETTER_OUTPUT = - "collection_size_threshold [java.lang.String, java.lang.String]\n" + - "columns_per_table_threshold [int, int] \n" + - "data_disk_usage_percentage_threshold [int, int] \n" + - "fields_per_udt_threshold [int, int] \n" + - "in_select_cartesian_product_threshold [int, int] \n" + - "items_per_collection_threshold [int, int] \n" + - "keyspaces_threshold [int, int] \n" + - "materialized_views_per_table_threshold [int, int] \n" + - "minimum_replication_factor_threshold [int, int] \n" + - "page_size_threshold [int, int] \n" + - "partition_keys_in_select_threshold [int, int] \n" + - "secondary_indexes_per_table_threshold [int, int] \n" + - "tables_threshold [int, int] \n"; - - private static final String ALL_VALUES_SETTER_OUTPUT = - "read_consistency_levels_disallowed java.util.Set\n" + - "read_consistency_levels_warned java.util.Set\n" + - "table_properties_disallowed java.util.Set\n" + - "table_properties_ignored java.util.Set\n" + - "table_properties_warned java.util.Set\n" + - "write_consistency_levels_disallowed java.util.Set\n" + - "write_consistency_levels_warned java.util.Set\n"; - - private static final String ALL_OTHER_SETTER_OUTPUT = - "data_disk_usage_max_disk_size java.lang.String \n"; - private static final String ALL_GUARDRAILS_GETTER_OUTPUT = removeMultipleSpaces(ALL_FLAGS_GETTER_OUTPUT + ALL_THRESHOLDS_GETTER_OUTPUT + ALL_VALUES_GETTER_OUTPUT + ALL_OTHER_GETTER_OUTPUT); - private static final String ALL_GUARDRAILS_SETTER_OUTPUT = removeMultipleSpaces(ALL_FLAGS_SETTER_OUTPUT + - ALL_THRESHOLDS_SETTER_OUTPUT + - ALL_VALUES_SETTER_OUTPUT + - ALL_OTHER_SETTER_OUTPUT); + private static final String ALL_GUARDRAILS_GETTER_VERBOSE_OUTPUT = removeMultipleSpaces(ALL_FLAGS_GETTER_OUTPUT + + ALL_THRESHOLDS_GETTER_VERBOSE_OUTPUT + + ALL_VALUES_GETTER_OUTPUT + + ALL_OTHER_GETTER_OUTPUT); private static String removeMultipleSpaces(String input) { @@ -305,9 +287,9 @@ public class GuardrailsConfigCommandsTest extends CQLTester return invokeNodetool("setguardrailsconfig", name, flag.toString()); } - private ToolResult setThresholds(String name, String warn, String fail) + private ToolResult setThresholds(String name, String fail, String warn) { - return invokeNodetool("setguardrailsconfig", name, warn, fail); + return invokeNodetool("setguardrailsconfig", name, fail, warn); } private ToolResult setValues(String name, String... values)