From 9f99e72aae812b86d277883601450bc0e7bb9463 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9s=20de=20la=20Pe=C3=B1a?= Date: Fri, 4 Nov 2022 12:39:21 +0000 Subject: [PATCH] Add reasons to guardrail messages and consider guardrails in the error message for needed ALLOW FILTERING MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Add optional reason for guarding an operation to every guardrail * Add reason for disabling ALLOW FILTERING into cassandra.yaml * Consider allow_filtering_enabled guardrail in the error message for needed ALLOW FILTERING patch by Andrés de la Peña; reviewed by Berenguer Blasi and Josh McKenzie for CASSANDRA-17967 --- CHANGES.txt | 1 + conf/cassandra.yaml | 1 + .../restrictions/StatementRestrictions.java | 37 +++++++---- .../cql3/statements/BatchStatement.java | 2 +- .../cql3/statements/DeleteStatement.java | 7 +- .../statements/ModificationStatement.java | 14 ++-- .../cql3/statements/SelectStatement.java | 12 ++-- .../cql3/statements/UpdateStatement.java | 19 ++++-- .../schema/CreateViewStatement.java | 3 +- .../cassandra/db/guardrails/EnableFlag.java | 19 +++++- .../cassandra/db/guardrails/Guardrail.java | 19 +++++- .../cassandra/db/guardrails/Guardrails.java | 34 +++++++++- .../cassandra/db/guardrails/MaxThreshold.java | 6 +- .../cassandra/db/guardrails/MinThreshold.java | 6 +- .../db/guardrails/PercentageThreshold.java | 6 +- .../cassandra/db/guardrails/Predicates.java | 4 +- .../cassandra/db/guardrails/Threshold.java | 4 +- .../cassandra/db/guardrails/Values.java | 4 +- .../org/apache/cassandra/db/view/View.java | 3 +- .../distributed/fuzz/SSTableGenerator.java | 3 +- .../GuardrailAllowFilteringTest.java | 27 +++++++- .../db/guardrails/GuardrailTester.java | 23 +++++-- .../GuardrailsConfigProviderTest.java | 1 + .../db/guardrails/GuardrailsTest.java | 64 +++++++++++-------- 24 files changed, 240 insertions(+), 79 deletions(-) diff --git a/CHANGES.txt b/CHANGES.txt index 1728a8db8d..b1b3788dc4 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,4 +1,5 @@ 4.2 + * Add reasons to guardrail messages and consider guardrails in the error message for needed ALLOW FILTERING (CASSANDRA-17967) * Add support for CQL functions on collections, tuples and UDTs (CASSANDRA-17811) * Add flag to exclude nodes from local DC when running nodetool rebuild (CASSANDRA-17870) * Adding endpoint verification option to client_encryption_options (CASSANDRA-18034) diff --git a/conf/cassandra.yaml b/conf/cassandra.yaml index ff074bddd7..ab15c87ebc 100644 --- a/conf/cassandra.yaml +++ b/conf/cassandra.yaml @@ -1814,6 +1814,7 @@ drop_compact_storage_enabled: false # items_per_collection_fail_threshold: -1 # # Guardrail to allow/disallow querying with ALLOW FILTERING. Defaults to true. +# ALLOW FILTERING can potentially visit all the data in the table and have unpredictable performance. # allow_filtering_enabled: true # # Guardrail to allow/disallow setting SimpleStrategy via keyspace creation or alteration. Defaults to true. diff --git a/src/java/org/apache/cassandra/cql3/restrictions/StatementRestrictions.java b/src/java/org/apache/cassandra/cql3/restrictions/StatementRestrictions.java index 8f8be94e1f..371bc589c3 100644 --- a/src/java/org/apache/cassandra/cql3/restrictions/StatementRestrictions.java +++ b/src/java/org/apache/cassandra/cql3/restrictions/StatementRestrictions.java @@ -28,6 +28,7 @@ import org.apache.cassandra.cql3.statements.Bound; import org.apache.cassandra.cql3.statements.StatementType; import org.apache.cassandra.db.*; import org.apache.cassandra.db.filter.RowFilter; +import org.apache.cassandra.db.guardrails.Guardrails; import org.apache.cassandra.db.marshal.AbstractType; import org.apache.cassandra.dht.*; import org.apache.cassandra.exceptions.InvalidRequestException; @@ -50,10 +51,15 @@ import static org.apache.cassandra.cql3.statements.RequestValidations.invalidReq */ public final class StatementRestrictions { - public static final String REQUIRES_ALLOW_FILTERING_MESSAGE = - "Cannot execute this query as it might involve data filtering and " + - "thus may have unpredictable performance. If you want to execute " + - "this query despite the performance unpredictability, use ALLOW FILTERING"; + private static final String ALLOW_FILTERING_MESSAGE = + "Cannot execute this query as it might involve data filtering and thus may have unpredictable performance. "; + + public static final String REQUIRES_ALLOW_FILTERING_MESSAGE = ALLOW_FILTERING_MESSAGE + + "If you want to execute this query despite the performance unpredictability, use ALLOW FILTERING"; + + public static final String CANNOT_USE_ALLOW_FILTERING_MESSAGE = ALLOW_FILTERING_MESSAGE + + "Executing this query despite the performance unpredictability with ALLOW FILTERING has been disabled " + + "by the allow_filtering_enabled property in cassandra.yaml"; /** * The type of statement @@ -125,7 +131,8 @@ public final class StatementRestrictions this.notNullColumns = new HashSet<>(); } - public StatementRestrictions(StatementType type, + public StatementRestrictions(ClientState state, + StatementType type, TableMetadata table, WhereClause whereClause, VariableSpecifications boundNames, @@ -133,14 +140,15 @@ public final class StatementRestrictions boolean allowFiltering, boolean forView) { - this(type, table, whereClause, boundNames, selectsOnlyStaticColumns, type.allowUseOfSecondaryIndices(), allowFiltering, forView); + this(state, type, table, whereClause, boundNames, selectsOnlyStaticColumns, type.allowUseOfSecondaryIndices(), allowFiltering, forView); } /* * We want to override allowUseOfSecondaryIndices flag from the StatementType for MV statements * to avoid initing the Keyspace and SecondaryIndexManager. */ - public StatementRestrictions(StatementType type, + public StatementRestrictions(ClientState state, + StatementType type, TableMetadata table, WhereClause whereClause, VariableSpecifications boundNames, @@ -214,7 +222,7 @@ public final class StatementRestrictions } // At this point, the select statement if fully constructed, but we still have a few things to validate - processPartitionKeyRestrictions(hasQueriableIndex, allowFiltering, forView); + processPartitionKeyRestrictions(state, hasQueriableIndex, allowFiltering, forView); // Some but not all of the partition key columns have been specified; // hence we need turn these restrictions into a row filter. @@ -267,7 +275,7 @@ public final class StatementRestrictions if (hasQueriableIndex) usesSecondaryIndexing = true; else if (!allowFiltering) - throw invalidRequest(StatementRestrictions.REQUIRES_ALLOW_FILTERING_MESSAGE); + throw invalidRequest(allowFilteringMessage(state)); filterRestrictions.add(nonPrimaryKeyRestrictions); } @@ -408,7 +416,7 @@ public final class StatementRestrictions return this.usesSecondaryIndexing; } - private void processPartitionKeyRestrictions(boolean hasQueriableIndex, boolean allowFiltering, boolean forView) + private void processPartitionKeyRestrictions(ClientState state, boolean hasQueriableIndex, boolean allowFiltering, boolean forView) { if (!type.allowPartitionKeyRanges()) { @@ -445,7 +453,7 @@ public final class StatementRestrictions if (partitionKeyRestrictions.needFiltering(table)) { if (!allowFiltering && !forView && !hasQueriableIndex) - throw new InvalidRequestException(REQUIRES_ALLOW_FILTERING_MESSAGE); + throw new InvalidRequestException(allowFilteringMessage(state)); isKeyRange = true; usesSecondaryIndexing = hasQueriableIndex; @@ -874,4 +882,11 @@ public final class StatementRestrictions { return ToStringBuilder.reflectionToString(this, ToStringStyle.SHORT_PREFIX_STYLE); } + + private static String allowFilteringMessage(ClientState state) + { + return Guardrails.allowFilteringEnabled.isEnabled(state) + ? REQUIRES_ALLOW_FILTERING_MESSAGE + : CANNOT_USE_ALLOW_FILTERING_MESSAGE; + } } diff --git a/src/java/org/apache/cassandra/cql3/statements/BatchStatement.java b/src/java/org/apache/cassandra/cql3/statements/BatchStatement.java index 61e4934864..ab761ad9c1 100644 --- a/src/java/org/apache/cassandra/cql3/statements/BatchStatement.java +++ b/src/java/org/apache/cassandra/cql3/statements/BatchStatement.java @@ -647,7 +647,7 @@ public class BatchStatement implements CQLStatement public BatchStatement prepare(ClientState state) { List statements = new ArrayList<>(parsedStatements.size()); - parsedStatements.forEach(s -> statements.add(s.prepare(bindVariables))); + parsedStatements.forEach(s -> statements.add(s.prepare(state, bindVariables))); Attributes prepAttrs = attrs.prepare("[batch]", "[batch]"); prepAttrs.collectMarkerSpecification(bindVariables); diff --git a/src/java/org/apache/cassandra/cql3/statements/DeleteStatement.java b/src/java/org/apache/cassandra/cql3/statements/DeleteStatement.java index be01481a45..fff6dd33df 100644 --- a/src/java/org/apache/cassandra/cql3/statements/DeleteStatement.java +++ b/src/java/org/apache/cassandra/cql3/statements/DeleteStatement.java @@ -31,6 +31,7 @@ import org.apache.cassandra.db.partitions.PartitionUpdate; import org.apache.cassandra.exceptions.InvalidRequestException; import org.apache.cassandra.schema.ColumnMetadata; import org.apache.cassandra.schema.TableMetadata; +import org.apache.cassandra.service.ClientState; import org.apache.cassandra.utils.Pair; import org.apache.commons.lang3.builder.ToStringBuilder; import org.apache.commons.lang3.builder.ToStringStyle; @@ -140,7 +141,8 @@ public class DeleteStatement extends ModificationStatement @Override - protected ModificationStatement prepareInternal(TableMetadata metadata, + protected ModificationStatement prepareInternal(ClientState state, + TableMetadata metadata, VariableSpecifications bindVariables, Conditions conditions, Attributes attrs) @@ -160,7 +162,8 @@ public class DeleteStatement extends ModificationStatement operations.add(op); } - StatementRestrictions restrictions = newRestrictions(metadata, + StatementRestrictions restrictions = newRestrictions(state, + metadata, bindVariables, operations, whereClause, diff --git a/src/java/org/apache/cassandra/cql3/statements/ModificationStatement.java b/src/java/org/apache/cassandra/cql3/statements/ModificationStatement.java index ab36ec971b..c69cf4945a 100644 --- a/src/java/org/apache/cassandra/cql3/statements/ModificationStatement.java +++ b/src/java/org/apache/cassandra/cql3/statements/ModificationStatement.java @@ -940,10 +940,10 @@ public abstract class ModificationStatement implements CQLStatement.SingleKeyspa public ModificationStatement prepare(ClientState state) { - return prepare(bindVariables); + return prepare(state, bindVariables); } - public ModificationStatement prepare(VariableSpecifications bindVariables) + public ModificationStatement prepare(ClientState state, VariableSpecifications bindVariables) { TableMetadata metadata = Schema.instance.validateTable(keyspace(), name()); @@ -952,7 +952,7 @@ public abstract class ModificationStatement implements CQLStatement.SingleKeyspa Conditions preparedConditions = prepareConditions(metadata, bindVariables); - return prepareInternal(metadata, bindVariables, preparedConditions, preparedAttributes); + return prepareInternal(state, metadata, bindVariables, preparedConditions, preparedAttributes); } /** @@ -1011,7 +1011,8 @@ public abstract class ModificationStatement implements CQLStatement.SingleKeyspa return builder.build(); } - protected abstract ModificationStatement prepareInternal(TableMetadata metadata, + protected abstract ModificationStatement prepareInternal(ClientState state, + TableMetadata metadata, VariableSpecifications bindVariables, Conditions conditions, Attributes attrs); @@ -1026,7 +1027,8 @@ public abstract class ModificationStatement implements CQLStatement.SingleKeyspa * @param conditions the conditions * @return the restrictions */ - protected StatementRestrictions newRestrictions(TableMetadata metadata, + protected StatementRestrictions newRestrictions(ClientState state, + TableMetadata metadata, VariableSpecifications boundNames, Operations operations, WhereClause where, @@ -1036,7 +1038,7 @@ public abstract class ModificationStatement implements CQLStatement.SingleKeyspa throw new InvalidRequestException(CUSTOM_EXPRESSIONS_NOT_ALLOWED); boolean applyOnlyToStaticColumns = appliesOnlyToStaticColumns(operations, conditions); - return new StatementRestrictions(type, metadata, where, boundNames, applyOnlyToStaticColumns, false, false); + return new StatementRestrictions(state, type, metadata, where, boundNames, applyOnlyToStaticColumns, false, false); } public List> getConditions() diff --git a/src/java/org/apache/cassandra/cql3/statements/SelectStatement.java b/src/java/org/apache/cassandra/cql3/statements/SelectStatement.java index 1b2e937189..75c12f065e 100644 --- a/src/java/org/apache/cassandra/cql3/statements/SelectStatement.java +++ b/src/java/org/apache/cassandra/cql3/statements/SelectStatement.java @@ -1075,17 +1075,17 @@ public class SelectStatement implements CQLStatement.SingleKeyspaceCqlStatement { // Cache locally for use by Guardrails this.state = state; - return prepare(false); + return prepare(state, false); } - public SelectStatement prepare(boolean forView) throws InvalidRequestException + public SelectStatement prepare(ClientState state, boolean forView) throws InvalidRequestException { TableMetadata table = Schema.instance.validateTable(keyspace(), name()); List selectables = RawSelector.toSelectables(selectClause, table); boolean containsOnlyStaticColumns = selectOnlyStaticColumns(table, selectables); - StatementRestrictions restrictions = prepareRestrictions(table, bindVariables, containsOnlyStaticColumns, forView); + StatementRestrictions restrictions = prepareRestrictions(state, table, bindVariables, containsOnlyStaticColumns, forView); // If we order post-query, the sorted column needs to be in the ResultSet for sorting, // even if we don't ultimately ship them to the client (CASSANDRA-4911). @@ -1215,12 +1215,14 @@ public class SelectStatement implements CQLStatement.SingleKeyspaceCqlStatement * @return the restrictions * @throws InvalidRequestException if a problem occurs while building the restrictions */ - private StatementRestrictions prepareRestrictions(TableMetadata metadata, + private StatementRestrictions prepareRestrictions(ClientState state, + TableMetadata metadata, VariableSpecifications boundNames, boolean selectsOnlyStaticColumns, boolean forView) throws InvalidRequestException { - return new StatementRestrictions(StatementType.SELECT, + return new StatementRestrictions(state, + StatementType.SELECT, metadata, whereClause, boundNames, diff --git a/src/java/org/apache/cassandra/cql3/statements/UpdateStatement.java b/src/java/org/apache/cassandra/cql3/statements/UpdateStatement.java index 20df151849..258fbf354e 100644 --- a/src/java/org/apache/cassandra/cql3/statements/UpdateStatement.java +++ b/src/java/org/apache/cassandra/cql3/statements/UpdateStatement.java @@ -32,6 +32,7 @@ import org.apache.cassandra.db.Slice; import org.apache.cassandra.db.partitions.PartitionUpdate; import org.apache.cassandra.schema.ColumnMetadata; import org.apache.cassandra.schema.TableMetadata; +import org.apache.cassandra.service.ClientState; import org.apache.cassandra.utils.ByteBufferUtil; import org.apache.cassandra.utils.Pair; import org.apache.commons.lang3.builder.ToStringBuilder; @@ -135,7 +136,8 @@ public class UpdateStatement extends ModificationStatement } @Override - protected ModificationStatement prepareInternal(TableMetadata metadata, + protected ModificationStatement prepareInternal(ClientState state, + TableMetadata metadata, VariableSpecifications bindVariables, Conditions conditions, Attributes attrs) @@ -176,7 +178,8 @@ public class UpdateStatement extends ModificationStatement boolean applyOnlyToStaticColumns = !hasClusteringColumnsSet && appliesOnlyToStaticColumns(operations, conditions); - StatementRestrictions restrictions = new StatementRestrictions(type, + StatementRestrictions restrictions = new StatementRestrictions(state, + type, metadata, whereClause.build(), bindVariables, @@ -210,7 +213,8 @@ public class UpdateStatement extends ModificationStatement } @Override - protected ModificationStatement prepareInternal(TableMetadata metadata, + protected ModificationStatement prepareInternal(ClientState state, + TableMetadata metadata, VariableSpecifications bindVariables, Conditions conditions, Attributes attrs) @@ -244,7 +248,8 @@ public class UpdateStatement extends ModificationStatement boolean applyOnlyToStaticColumns = !hasClusteringColumnsSet && appliesOnlyToStaticColumns(operations, conditions); - StatementRestrictions restrictions = new StatementRestrictions(type, + StatementRestrictions restrictions = new StatementRestrictions(state, + type, metadata, whereClause.build(), bindVariables, @@ -291,7 +296,8 @@ public class UpdateStatement extends ModificationStatement } @Override - protected ModificationStatement prepareInternal(TableMetadata metadata, + protected ModificationStatement prepareInternal(ClientState state, + TableMetadata metadata, VariableSpecifications bindVariables, Conditions conditions, Attributes attrs) @@ -309,7 +315,8 @@ public class UpdateStatement extends ModificationStatement operations.add(operation); } - StatementRestrictions restrictions = newRestrictions(metadata, + StatementRestrictions restrictions = newRestrictions(state, + metadata, bindVariables, operations, whereClause, diff --git a/src/java/org/apache/cassandra/cql3/statements/schema/CreateViewStatement.java b/src/java/org/apache/cassandra/cql3/statements/schema/CreateViewStatement.java index 145c8fc838..dce054faa4 100644 --- a/src/java/org/apache/cassandra/cql3/statements/schema/CreateViewStatement.java +++ b/src/java/org/apache/cassandra/cql3/statements/schema/CreateViewStatement.java @@ -272,7 +272,8 @@ public final class CreateViewStatement extends AlterSchemaStatement throw ire("WHERE clause for materialized view '%s' cannot contain custom index expressions", viewName); StatementRestrictions restrictions = - new StatementRestrictions(StatementType.SELECT, + new StatementRestrictions(state, + StatementType.SELECT, table, whereClause, VariableSpecifications.empty(), diff --git a/src/java/org/apache/cassandra/db/guardrails/EnableFlag.java b/src/java/org/apache/cassandra/db/guardrails/EnableFlag.java index aba013a65f..7b5a63eb99 100644 --- a/src/java/org/apache/cassandra/db/guardrails/EnableFlag.java +++ b/src/java/org/apache/cassandra/db/guardrails/EnableFlag.java @@ -39,18 +39,31 @@ public class EnableFlag extends Guardrail * Creates a new {@link EnableFlag} guardrail. * * @param name the identifying name of the guardrail + * @param reason the optional description of the reason for guarding the operation * @param enabled a {@link ClientState}-based supplier of boolean indicating whether the feature guarded by this * guardrail is enabled. * @param featureName The feature that is guarded by this guardrail (for reporting in error messages), {@link * EnableFlag#ensureEnabled(String, ClientState)} can specify a different {@code featureName}. */ - public EnableFlag(String name, Predicate enabled, String featureName) + public EnableFlag(String name, @Nullable String reason, Predicate enabled, String featureName) { - super(name); + super(name, reason); this.enabled = enabled; this.featureName = featureName; } + /** + * Returns whether the guarded feature is enabled or not. + * + * @param state The client state, used to skip the check if the query is internal or is done by a superuser. + * A {@code null} value means that the check should be done regardless of the query. + * @return {@code true} is the feature is enabled, {@code false} otherwise. + */ + public boolean isEnabled(@Nullable ClientState state) + { + return !enabled(state) || enabled.test(state); + } + /** * Aborts the operation if this guardrail is not enabled. * @@ -80,7 +93,7 @@ public class EnableFlag extends Guardrail */ public void ensureEnabled(String featureName, @Nullable ClientState state) { - if (enabled(state) && !enabled.test(state)) + if (!isEnabled(state)) fail(featureName + " is not allowed", state); } } diff --git a/src/java/org/apache/cassandra/db/guardrails/Guardrail.java b/src/java/org/apache/cassandra/db/guardrails/Guardrail.java index c058f10784..fbd1a5b880 100644 --- a/src/java/org/apache/cassandra/db/guardrails/Guardrail.java +++ b/src/java/org/apache/cassandra/db/guardrails/Guardrail.java @@ -51,6 +51,10 @@ public abstract class Guardrail /** A name identifying the guardrail (mainly for shipping with diagnostic events). */ public final String name; + /** An optional description of the reason for guarding the operation. */ + @Nullable + public final String reason; + /** Minimum logging and triggering interval to avoid spamming downstream. */ private long minNotifyIntervalInMs = 0; @@ -60,9 +64,10 @@ public abstract class Guardrail /** Time of last failure in milliseconds. */ private volatile long lastFailInMs = 0; - Guardrail(String name) + Guardrail(String name, @Nullable String reason) { this.name = name; + this.reason = reason; } /** @@ -138,8 +143,16 @@ public abstract class Guardrail @VisibleForTesting String decorateMessage(String message) { - // Add a prefix to error message so user knows what threw the warning or cause the failure - return String.format("Guardrail %s violated: %s", name, message); + // Add a prefix to error message so user knows what threw the warning or cause the failure. + String decoratedMessage = String.format("Guardrail %s violated: %s", name, message); + + // Add the reason for the guardrail triggering, if there is any. + if (reason != null) + { + decoratedMessage += (message.endsWith(".") ? ' ' : ". ") + reason; + } + + return decoratedMessage; } /** diff --git a/src/java/org/apache/cassandra/db/guardrails/Guardrails.java b/src/java/org/apache/cassandra/db/guardrails/Guardrails.java index 633f396ce5..4ea6312b4f 100644 --- a/src/java/org/apache/cassandra/db/guardrails/Guardrails.java +++ b/src/java/org/apache/cassandra/db/guardrails/Guardrails.java @@ -57,6 +57,7 @@ public final class Guardrails implements GuardrailsMBean */ public static final MaxThreshold keyspaces = new MaxThreshold("keyspaces", + null, state -> CONFIG_PROVIDER.getOrCreate(state).getKeyspacesWarnThreshold(), state -> CONFIG_PROVIDER.getOrCreate(state).getKeyspacesFailThreshold(), (isWarning, what, value, threshold) -> @@ -70,6 +71,7 @@ public final class Guardrails implements GuardrailsMBean */ public static final MaxThreshold tables = new MaxThreshold("tables", + null, state -> CONFIG_PROVIDER.getOrCreate(state).getTablesWarnThreshold(), state -> CONFIG_PROVIDER.getOrCreate(state).getTablesFailThreshold(), (isWarning, what, value, threshold) -> @@ -83,6 +85,7 @@ public final class Guardrails implements GuardrailsMBean */ public static final MaxThreshold columnsPerTable = new MaxThreshold("columns_per_table", + null, state -> CONFIG_PROVIDER.getOrCreate(state).getColumnsPerTableWarnThreshold(), state -> CONFIG_PROVIDER.getOrCreate(state).getColumnsPerTableFailThreshold(), (isWarning, what, value, threshold) -> @@ -93,6 +96,7 @@ public final class Guardrails implements GuardrailsMBean public static final MaxThreshold secondaryIndexesPerTable = new MaxThreshold("secondary_indexes_per_table", + null, state -> CONFIG_PROVIDER.getOrCreate(state).getSecondaryIndexesPerTableWarnThreshold(), state -> CONFIG_PROVIDER.getOrCreate(state).getSecondaryIndexesPerTableFailThreshold(), (isWarning, what, value, threshold) -> @@ -106,6 +110,7 @@ public final class Guardrails implements GuardrailsMBean */ public static final EnableFlag createSecondaryIndexesEnabled = new EnableFlag("secondary_indexes", + null, state -> CONFIG_PROVIDER.getOrCreate(state).getSecondaryIndexesEnabled(), "User creation of secondary indexes"); @@ -114,6 +119,7 @@ public final class Guardrails implements GuardrailsMBean */ public static final MaxThreshold materializedViewsPerTable = new MaxThreshold("materialized_views_per_table", + null, state -> CONFIG_PROVIDER.getOrCreate(state).getMaterializedViewsPerTableWarnThreshold(), state -> CONFIG_PROVIDER.getOrCreate(state).getMaterializedViewsPerTableFailThreshold(), (isWarning, what, value, threshold) -> @@ -127,6 +133,7 @@ public final class Guardrails implements GuardrailsMBean */ public static final Values tableProperties = new Values<>("table_properties", + null, state -> CONFIG_PROVIDER.getOrCreate(state).getTablePropertiesWarned(), state -> CONFIG_PROVIDER.getOrCreate(state).getTablePropertiesIgnored(), state -> CONFIG_PROVIDER.getOrCreate(state).getTablePropertiesDisallowed(), @@ -137,11 +144,13 @@ public final class Guardrails implements GuardrailsMBean */ public static final EnableFlag userTimestampsEnabled = new EnableFlag("user_timestamps", + null, state -> CONFIG_PROVIDER.getOrCreate(state).getUserTimestampsEnabled(), "User provided timestamps (USING TIMESTAMP)"); public static final EnableFlag groupByEnabled = new EnableFlag("group_by", + null, state -> CONFIG_PROVIDER.getOrCreate(state).getGroupByEnabled(), "GROUP BY functionality"); @@ -150,14 +159,16 @@ public final class Guardrails implements GuardrailsMBean */ public static final EnableFlag alterTableEnabled = new EnableFlag("alter_table", - state -> CONFIG_PROVIDER.getOrCreate(state).getAlterTableEnabled(), - "User access to ALTER TABLE statement for column mutation"); + null, + state -> CONFIG_PROVIDER.getOrCreate(state).getAlterTableEnabled(), + "User access to ALTER TABLE statement for column mutation"); /** * Guardrail disabling DROP / TRUNCATE TABLE behavior */ public static final EnableFlag dropTruncateTableEnabled = new EnableFlag("drop_truncate_table_enabled", + null, state -> CONFIG_PROVIDER.getOrCreate(state).getDropTruncateTableEnabled(), "DROP and TRUNCATE TABLE functionality"); @@ -166,6 +177,7 @@ public final class Guardrails implements GuardrailsMBean */ public static final EnableFlag dropKeyspaceEnabled = new EnableFlag("drop_keyspace_enabled", + null, state -> CONFIG_PROVIDER.getOrCreate(state).getDropKeyspaceEnabled(), "DROP KEYSPACE functionality"); @@ -174,6 +186,7 @@ public final class Guardrails implements GuardrailsMBean */ public static final EnableFlag uncompressedTablesEnabled = new EnableFlag("uncompressed_tables_enabled", + null, state -> CONFIG_PROVIDER.getOrCreate(state).getUncompressedTablesEnabled(), "Uncompressed table"); @@ -182,6 +195,7 @@ public final class Guardrails implements GuardrailsMBean */ public static final EnableFlag compactTablesEnabled = new EnableFlag("compact_tables", + null, state -> CONFIG_PROVIDER.getOrCreate(state).getCompactTablesEnabled(), "Creation of new COMPACT STORAGE tables"); @@ -190,6 +204,7 @@ public final class Guardrails implements GuardrailsMBean */ public static final MaxThreshold pageSize = new MaxThreshold("page_size", + null, state -> CONFIG_PROVIDER.getOrCreate(state).getPageSizeWarnThreshold(), state -> CONFIG_PROVIDER.getOrCreate(state).getPageSizeFailThreshold(), (isWarning, what, value, threshold) -> @@ -203,6 +218,7 @@ public final class Guardrails implements GuardrailsMBean */ public static final MaxThreshold partitionKeysInSelect = new MaxThreshold("partition_keys_in_select", + null, state -> CONFIG_PROVIDER.getOrCreate(state).getPartitionKeysInSelectWarnThreshold(), state -> CONFIG_PROVIDER.getOrCreate(state).getPartitionKeysInSelectFailThreshold(), (isWarning, what, value, threshold) -> @@ -218,6 +234,7 @@ public final class Guardrails implements GuardrailsMBean */ public static final EnableFlag readBeforeWriteListOperationsEnabled = new EnableFlag("read_before_write_list_operations", + null, state -> CONFIG_PROVIDER.getOrCreate(state).getReadBeforeWriteListOperationsEnabled(), "List operation requiring read before write"); @@ -226,6 +243,7 @@ public final class Guardrails implements GuardrailsMBean */ public static final EnableFlag allowFilteringEnabled = new EnableFlag("allow_filtering", + "ALLOW FILTERING can potentially visit all the data in the table and have unpredictable performance.", state -> CONFIG_PROVIDER.getOrCreate(state).getAllowFilteringEnabled(), "Querying with ALLOW FILTERING"); @@ -234,6 +252,7 @@ public final class Guardrails implements GuardrailsMBean */ public static final EnableFlag simpleStrategyEnabled = new EnableFlag("simplestrategy", + null, state -> CONFIG_PROVIDER.getOrCreate(state).getSimpleStrategyEnabled(), "SimpleStrategy"); @@ -242,6 +261,7 @@ public final class Guardrails implements GuardrailsMBean */ public static final MaxThreshold inSelectCartesianProduct = new MaxThreshold("in_select_cartesian_product", + null, state -> CONFIG_PROVIDER.getOrCreate(state).getInSelectCartesianProductWarnThreshold(), state -> CONFIG_PROVIDER.getOrCreate(state).getInSelectCartesianProductFailThreshold(), (isWarning, what, value, threshold) -> @@ -257,6 +277,7 @@ public final class Guardrails implements GuardrailsMBean */ public static final Values readConsistencyLevels = new Values<>("read_consistency_levels", + null, state -> CONFIG_PROVIDER.getOrCreate(state).getReadConsistencyLevelsWarned(), state -> Collections.emptySet(), state -> CONFIG_PROVIDER.getOrCreate(state).getReadConsistencyLevelsDisallowed(), @@ -267,6 +288,7 @@ public final class Guardrails implements GuardrailsMBean */ public static final Values writeConsistencyLevels = new Values<>("write_consistency_levels", + null, state -> CONFIG_PROVIDER.getOrCreate(state).getWriteConsistencyLevelsWarned(), state -> Collections.emptySet(), state -> CONFIG_PROVIDER.getOrCreate(state).getWriteConsistencyLevelsDisallowed(), @@ -277,6 +299,7 @@ public final class Guardrails implements GuardrailsMBean */ public static final MaxThreshold columnValueSize = new MaxThreshold("column_value_size", + null, state -> sizeToBytes(CONFIG_PROVIDER.getOrCreate(state).getColumnValueSizeWarnThreshold()), state -> sizeToBytes(CONFIG_PROVIDER.getOrCreate(state).getColumnValueSizeFailThreshold()), (isWarning, what, value, threshold) -> @@ -288,6 +311,7 @@ public final class Guardrails implements GuardrailsMBean */ public static final MaxThreshold collectionSize = new MaxThreshold("collection_size", + null, state -> sizeToBytes(CONFIG_PROVIDER.getOrCreate(state).getCollectionSizeWarnThreshold()), state -> sizeToBytes(CONFIG_PROVIDER.getOrCreate(state).getCollectionSizeFailThreshold()), (isWarning, what, value, threshold) -> @@ -299,6 +323,7 @@ public final class Guardrails implements GuardrailsMBean */ public static final MaxThreshold itemsPerCollection = new MaxThreshold("items_per_collection", + null, state -> CONFIG_PROVIDER.getOrCreate(state).getItemsPerCollectionWarnThreshold(), state -> CONFIG_PROVIDER.getOrCreate(state).getItemsPerCollectionFailThreshold(), (isWarning, what, value, threshold) -> @@ -310,6 +335,7 @@ public final class Guardrails implements GuardrailsMBean */ public static final MaxThreshold fieldsPerUDT = new MaxThreshold("fields_per_udt", + null, state -> CONFIG_PROVIDER.getOrCreate(state).getFieldsPerUDTWarnThreshold(), state -> CONFIG_PROVIDER.getOrCreate(state).getFieldsPerUDTFailThreshold(), (isWarning, what, value, threshold) -> @@ -324,6 +350,7 @@ public final class Guardrails implements GuardrailsMBean */ public static final PercentageThreshold localDataDiskUsage = new PercentageThreshold("local_data_disk_usage", + null, state -> CONFIG_PROVIDER.getOrCreate(state).getDataDiskUsagePercentageWarnThreshold(), state -> CONFIG_PROVIDER.getOrCreate(state).getDataDiskUsagePercentageFailThreshold(), (isWarning, what, value, threshold) -> @@ -339,6 +366,7 @@ public final class Guardrails implements GuardrailsMBean */ public static final Predicates replicaDiskUsage = new Predicates<>("replica_disk_usage", + null, state -> DiskUsageBroadcaster.instance::isStuffed, state -> DiskUsageBroadcaster.instance::isFull, // not using `value` because it represents replica address which should be hidden from client. @@ -359,6 +387,7 @@ public final class Guardrails implements GuardrailsMBean */ public static final MinThreshold minimumReplicationFactor = new MinThreshold("minimum_replication_factor", + null, state -> CONFIG_PROVIDER.getOrCreate(state).getMinimumReplicationFactorWarnThreshold(), state -> CONFIG_PROVIDER.getOrCreate(state).getMinimumReplicationFactorFailThreshold(), (isWarning, what, value, threshold) -> @@ -370,6 +399,7 @@ public final class Guardrails implements GuardrailsMBean */ public static final MaxThreshold maximumReplicationFactor = new MaxThreshold("maximum_replication_factor", + null, state -> CONFIG_PROVIDER.getOrCreate(state).getMaximumReplicationFactorWarnThreshold(), state -> CONFIG_PROVIDER.getOrCreate(state).getMaximumReplicationFactorFailThreshold(), (isWarning, what, value, threshold) -> diff --git a/src/java/org/apache/cassandra/db/guardrails/MaxThreshold.java b/src/java/org/apache/cassandra/db/guardrails/MaxThreshold.java index badaff784b..6203ee2bf5 100644 --- a/src/java/org/apache/cassandra/db/guardrails/MaxThreshold.java +++ b/src/java/org/apache/cassandra/db/guardrails/MaxThreshold.java @@ -19,6 +19,8 @@ package org.apache.cassandra.db.guardrails; import java.util.function.ToLongFunction; +import javax.annotation.Nullable; + import org.apache.cassandra.service.ClientState; /** @@ -30,16 +32,18 @@ public class MaxThreshold extends Threshold * Creates a new threshold guardrail. * * @param name the identifying name of the guardrail + * @param reason the optional description of the reason for guarding the operation * @param warnThreshold a {@link ClientState}-based provider of the value above which a warning should be triggered. * @param failThreshold a {@link ClientState}-based provider of the value above which the operation should be aborted. * @param messageProvider a function to generate the warning or error message if the guardrail is triggered */ public MaxThreshold(String name, + @Nullable String reason, ToLongFunction warnThreshold, ToLongFunction failThreshold, Threshold.ErrorMessageProvider messageProvider) { - super(name, warnThreshold, failThreshold, messageProvider); + super(name, reason, warnThreshold, failThreshold, messageProvider); } @Override diff --git a/src/java/org/apache/cassandra/db/guardrails/MinThreshold.java b/src/java/org/apache/cassandra/db/guardrails/MinThreshold.java index 427f2777e8..72704cc51c 100644 --- a/src/java/org/apache/cassandra/db/guardrails/MinThreshold.java +++ b/src/java/org/apache/cassandra/db/guardrails/MinThreshold.java @@ -19,6 +19,8 @@ package org.apache.cassandra.db.guardrails; import java.util.function.ToLongFunction; +import javax.annotation.Nullable; + import org.apache.cassandra.service.ClientState; /** @@ -30,16 +32,18 @@ public class MinThreshold extends Threshold * Creates a new minimum threshold guardrail. * * @param name the identifying name of the guardrail + * @param reason the optional description of the reason for guarding the operation * @param warnThreshold a {@link ClientState}-based provider of the value above which a warning should be triggered. * @param failThreshold a {@link ClientState}-based provider of the value above which the operation should be aborted. * @param messageProvider a function to generate the warning or error message if the guardrail is triggered */ public MinThreshold(String name, + @Nullable String reason, ToLongFunction warnThreshold, ToLongFunction failThreshold, Threshold.ErrorMessageProvider messageProvider) { - super(name, warnThreshold, failThreshold, messageProvider); + super(name, reason, warnThreshold, failThreshold, messageProvider); } @Override diff --git a/src/java/org/apache/cassandra/db/guardrails/PercentageThreshold.java b/src/java/org/apache/cassandra/db/guardrails/PercentageThreshold.java index 6f866c6ec0..c316276596 100644 --- a/src/java/org/apache/cassandra/db/guardrails/PercentageThreshold.java +++ b/src/java/org/apache/cassandra/db/guardrails/PercentageThreshold.java @@ -20,6 +20,8 @@ package org.apache.cassandra.db.guardrails; import java.util.function.ToLongFunction; +import javax.annotation.Nullable; + import org.apache.cassandra.service.ClientState; /** @@ -33,16 +35,18 @@ public class PercentageThreshold extends MaxThreshold * Creates a new threshold guardrail. * * @param name the identifying name of the guardrail + * @param reason the optional description of the reason for guarding the operation * @param warnThreshold a {@link ClientState}-based provider of the value above which a warning should be triggered. * @param failThreshold a {@link ClientState}-based provider of the value above which the operation should be aborted. * @param messageProvider a function to generate the warning or error message if the guardrail is triggered */ public PercentageThreshold(String name, + @Nullable String reason, ToLongFunction warnThreshold, ToLongFunction failThreshold, ErrorMessageProvider messageProvider) { - super(name, warnThreshold, failThreshold, messageProvider); + super(name, reason, warnThreshold, failThreshold, messageProvider); } @Override diff --git a/src/java/org/apache/cassandra/db/guardrails/Predicates.java b/src/java/org/apache/cassandra/db/guardrails/Predicates.java index 13be9e9302..ab08560218 100644 --- a/src/java/org/apache/cassandra/db/guardrails/Predicates.java +++ b/src/java/org/apache/cassandra/db/guardrails/Predicates.java @@ -56,16 +56,18 @@ public class Predicates extends Guardrail * Creates a new {@link Predicates} guardrail. * * @param name the identifying name of the guardrail + * @param reason the optional description of the reason for guarding the operation * @param warnPredicate a {@link ClientState}-based predicate provider that is used to check if given value should trigger a warning. * @param failurePredicate a {@link ClientState}-based predicate provider that is used to check if given value should trigger a failure. * @param messageProvider a function to generate the warning or error message if the guardrail is triggered */ Predicates(String name, + @Nullable String reason, Function> warnPredicate, Function> failurePredicate, MessageProvider messageProvider) { - super(name); + super(name, reason); this.warnPredicate = warnPredicate; this.failurePredicate = failurePredicate; this.messageProvider = messageProvider; diff --git a/src/java/org/apache/cassandra/db/guardrails/Threshold.java b/src/java/org/apache/cassandra/db/guardrails/Threshold.java index b671907c62..257ab013b7 100644 --- a/src/java/org/apache/cassandra/db/guardrails/Threshold.java +++ b/src/java/org/apache/cassandra/db/guardrails/Threshold.java @@ -41,16 +41,18 @@ public abstract class Threshold extends Guardrail * Creates a new threshold guardrail. * * @param name the identifying name of the guardrail + * @param reason the optional description of the reason for guarding the operation * @param warnThreshold a {@link ClientState}-based provider of the value above which a warning should be triggered. * @param failThreshold a {@link ClientState}-based provider of the value above which the operation should be aborted. * @param messageProvider a function to generate the warning or error message if the guardrail is triggered */ public Threshold(String name, + @Nullable String reason, ToLongFunction warnThreshold, ToLongFunction failThreshold, ErrorMessageProvider messageProvider) { - super(name); + super(name, reason); this.warnThreshold = warnThreshold; this.failThreshold = failThreshold; this.messageProvider = messageProvider; diff --git a/src/java/org/apache/cassandra/db/guardrails/Values.java b/src/java/org/apache/cassandra/db/guardrails/Values.java index f46e3af035..9504a3d63b 100644 --- a/src/java/org/apache/cassandra/db/guardrails/Values.java +++ b/src/java/org/apache/cassandra/db/guardrails/Values.java @@ -47,18 +47,20 @@ public class Values extends Guardrail * Creates a new values guardrail. * * @param name the identifying name of the guardrail + * @param reason the optional description of the reason for guarding the operation * @param warnedValues a {@link ClientState}-based provider of the values for which a warning is triggered. * @param ignoredValues a {@link ClientState}-based provider of the values that are ignored. * @param disallowedValues a {@link ClientState}-based provider of the values that are disallowed. * @param what The feature that is guarded by this guardrail (for reporting in error messages). */ public Values(String name, + @Nullable String reason, Function> warnedValues, Function> ignoredValues, Function> disallowedValues, String what) { - super(name); + super(name, reason); this.warnedValues = warnedValues; this.ignoredValues = ignoredValues; this.disallowedValues = disallowedValues; diff --git a/src/java/org/apache/cassandra/db/view/View.java b/src/java/org/apache/cassandra/db/view/View.java index d813d0e668..a3ecc33d79 100644 --- a/src/java/org/apache/cassandra/db/view/View.java +++ b/src/java/org/apache/cassandra/db/view/View.java @@ -35,6 +35,7 @@ import org.apache.cassandra.schema.KeyspaceMetadata; import org.apache.cassandra.schema.Schema; import org.apache.cassandra.schema.TableMetadataRef; import org.apache.cassandra.schema.ViewMetadata; +import org.apache.cassandra.service.ClientState; import org.apache.cassandra.utils.FBUtilities; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -177,7 +178,7 @@ public class View rawSelect.setBindVariables(Collections.emptyList()); - select = rawSelect.prepare(true); + select = rawSelect.prepare(ClientState.forInternalCalls(), true); } return select; diff --git a/test/distributed/org/apache/cassandra/distributed/fuzz/SSTableGenerator.java b/test/distributed/org/apache/cassandra/distributed/fuzz/SSTableGenerator.java index 627d3606da..c7d0fbbc96 100644 --- a/test/distributed/org/apache/cassandra/distributed/fuzz/SSTableGenerator.java +++ b/test/distributed/org/apache/cassandra/distributed/fuzz/SSTableGenerator.java @@ -306,7 +306,8 @@ public class SSTableGenerator new AbstractMarker.Raw(values.size() - 1))); } - StatementRestrictions restrictions = new StatementRestrictions(StatementType.DELETE, + StatementRestrictions restrictions = new StatementRestrictions(null, + StatementType.DELETE, metadata, builder.build(), new VariableSpecifications(variableNames), diff --git a/test/unit/org/apache/cassandra/db/guardrails/GuardrailAllowFilteringTest.java b/test/unit/org/apache/cassandra/db/guardrails/GuardrailAllowFilteringTest.java index c46498c01f..5eca8393a9 100644 --- a/test/unit/org/apache/cassandra/db/guardrails/GuardrailAllowFilteringTest.java +++ b/test/unit/org/apache/cassandra/db/guardrails/GuardrailAllowFilteringTest.java @@ -22,8 +22,11 @@ import org.junit.After; import org.junit.Before; import org.junit.Test; +import org.apache.cassandra.cql3.restrictions.StatementRestrictions; +import org.apache.cassandra.exceptions.InvalidRequestException; import org.apache.cassandra.schema.SchemaConstants; import org.apache.cassandra.schema.SchemaKeyspaceTables; +import org.apache.cassandra.service.ClientState; public class GuardrailAllowFilteringTest extends GuardrailTester { @@ -33,7 +36,7 @@ public class GuardrailAllowFilteringTest extends GuardrailTester public void setupTest() { createTable("CREATE TABLE %s (k int PRIMARY KEY, a int, b int)"); - enableState = getGuardrial(); + enableState = getGuardrail(); } @After @@ -47,7 +50,7 @@ public class GuardrailAllowFilteringTest extends GuardrailTester guardrails().setAllowFilteringEnabled(allowFilteringEnabled); } - private boolean getGuardrial() + private boolean getGuardrail() { return guardrails().getAllowFilteringEnabled(); } @@ -91,4 +94,24 @@ public class GuardrailAllowFilteringTest extends GuardrailTester SchemaKeyspaceTables.TABLES, currentTable())); } + + @Test + public void testRequiredAllowFiltering() + { + setGuardrail(true); + assertRequiredAllowFilteringThrows(systemClientState, StatementRestrictions.REQUIRES_ALLOW_FILTERING_MESSAGE); + assertRequiredAllowFilteringThrows(superClientState, StatementRestrictions.REQUIRES_ALLOW_FILTERING_MESSAGE); + assertRequiredAllowFilteringThrows(userClientState, StatementRestrictions.REQUIRES_ALLOW_FILTERING_MESSAGE); + + setGuardrail(false); + assertRequiredAllowFilteringThrows(systemClientState, StatementRestrictions.REQUIRES_ALLOW_FILTERING_MESSAGE); + assertRequiredAllowFilteringThrows(superClientState, StatementRestrictions.REQUIRES_ALLOW_FILTERING_MESSAGE); + assertRequiredAllowFilteringThrows(userClientState, StatementRestrictions.CANNOT_USE_ALLOW_FILTERING_MESSAGE); + } + + private void assertRequiredAllowFilteringThrows(ClientState state, String message) + { + String query = "SELECT * FROM %s WHERE a = 5"; + assertThrows(() -> execute(state, query), InvalidRequestException.class, message); + } } diff --git a/test/unit/org/apache/cassandra/db/guardrails/GuardrailTester.java b/test/unit/org/apache/cassandra/db/guardrails/GuardrailTester.java index 54523743b7..5e0cae224d 100644 --- a/test/unit/org/apache/cassandra/db/guardrails/GuardrailTester.java +++ b/test/unit/org/apache/cassandra/db/guardrails/GuardrailTester.java @@ -353,9 +353,17 @@ public abstract class GuardrailTester extends CQLTester if (guardrail != null) { - String prefix = guardrail.decorateMessage(""); - assertTrue(format("Full error message '%s' doesn't start with the prefix '%s'", e.getMessage(), prefix), - e.getMessage().startsWith(prefix)); + String message = e.getMessage(); + String prefix = guardrail.decorateMessage("").replace(". " + guardrail.reason, ""); + assertTrue(format("Full error message '%s' doesn't start with the prefix '%s'", message, prefix), + message.startsWith(prefix)); + + String reason = guardrail.reason; + if (reason != null) + { + assertTrue(format("Full error message '%s' doesn't end with the reason '%s'", message, reason), + message.endsWith(reason)); + } } assertTrue(format("Full error message '%s' does not contain expected message '%s'", e.getMessage(), failMessage), @@ -412,9 +420,16 @@ public abstract class GuardrailTester extends CQLTester String warning = warnings.get(i); if (guardrail != null) { - String prefix = guardrail.decorateMessage(""); + String prefix = guardrail.decorateMessage("").replace(". " + guardrail.reason, ""); assertTrue(format("Warning log message '%s' doesn't start with the prefix '%s'", warning, prefix), warning.startsWith(prefix)); + + String reason = guardrail.reason; + if (reason != null) + { + assertTrue(format("Warning log message '%s' doesn't end with the reason '%s'", warning, reason), + warning.endsWith(reason)); + } } assertTrue(format("Warning log message '%s' does not contain expected message '%s'", warning, message), diff --git a/test/unit/org/apache/cassandra/db/guardrails/GuardrailsConfigProviderTest.java b/test/unit/org/apache/cassandra/db/guardrails/GuardrailsConfigProviderTest.java index e99b736b03..36fbc7cd08 100644 --- a/test/unit/org/apache/cassandra/db/guardrails/GuardrailsConfigProviderTest.java +++ b/test/unit/org/apache/cassandra/db/guardrails/GuardrailsConfigProviderTest.java @@ -37,6 +37,7 @@ public class GuardrailsConfigProviderTest extends GuardrailTester String name = getClass().getCanonicalName() + '$' + CustomProvider.class.getSimpleName(); GuardrailsConfigProvider provider = GuardrailsConfigProvider.build(name); MaxThreshold guard = new MaxThreshold("test_guardrail", + "Some reason", state -> provider.getOrCreate(state).getTablesWarnThreshold(), state -> provider.getOrCreate(state).getTablesFailThreshold(), (isWarn, what, v, t) -> format("%s: for %s, %s > %s", diff --git a/test/unit/org/apache/cassandra/db/guardrails/GuardrailsTest.java b/test/unit/org/apache/cassandra/db/guardrails/GuardrailsTest.java index 5c7e724abb..9cea89b4b5 100644 --- a/test/unit/org/apache/cassandra/db/guardrails/GuardrailsTest.java +++ b/test/unit/org/apache/cassandra/db/guardrails/GuardrailsTest.java @@ -38,7 +38,7 @@ import static org.junit.Assert.assertTrue; public class GuardrailsTest extends GuardrailTester { public static final int DISABLED = -1; - + public static final String REASON = "Testing"; private void testDisabledThreshold(Threshold guard) throws Throwable { @@ -58,17 +58,18 @@ public class GuardrailsTest extends GuardrailTester public void testDisabledMaxThreshold() throws Throwable { Threshold.ErrorMessageProvider errorMessageProvider = (isWarn, what, v, t) -> "Should never trigger"; - testDisabledThreshold(new MaxThreshold("x", state -> DISABLED, state -> DISABLED, errorMessageProvider)); + testDisabledThreshold(new MaxThreshold("x", REASON, state -> DISABLED, state -> DISABLED, errorMessageProvider)); } @Test public void testMaxThreshold() throws Throwable { MaxThreshold guard = new MaxThreshold("x", - state -> 10, - state -> 100, - (isWarn, featureName, v, t) -> format("%s: for %s, %s > %s", - isWarn ? "Warning" : "Aborting", featureName, v, t)); + REASON, + state -> 10, + state -> 100, + (isWarn, featureName, v, t) -> format("%s: for %s, %s > %s", + isWarn ? "Warning" : "Aborting", featureName, v, t)); assertTrue(guard.enabled(userClientState)); @@ -91,10 +92,11 @@ public class GuardrailsTest extends GuardrailTester public void testWarnOnlyMaxThreshold() throws Throwable { MaxThreshold guard = new MaxThreshold("x", - state -> 10, - state -> DISABLED, - (isWarn, featureName, v, t) -> format("%s: for %s, %s > %s", - isWarn ? "Warning" : "Aborting", featureName, v, t)); + REASON, + state -> 10, + state -> DISABLED, + (isWarn, featureName, v, t) -> format("%s: for %s, %s > %s", + isWarn ? "Warning" : "Aborting", featureName, v, t)); assertTrue(guard.enabled(userClientState)); @@ -109,10 +111,11 @@ public class GuardrailsTest extends GuardrailTester public void testFailOnlyMaxThreshold() throws Throwable { MaxThreshold guard = new MaxThreshold("x", - state -> DISABLED, - state -> 10, - (isWarn, featureName, v, t) -> format("%s: for %s, %s > %s", - isWarn ? "Warning" : "Aborting", featureName, v, t)); + REASON, + state -> DISABLED, + state -> 10, + (isWarn, featureName, v, t) -> format("%s: for %s, %s > %s", + isWarn ? "Warning" : "Aborting", featureName, v, t)); assertTrue(guard.enabled(userClientState)); @@ -127,10 +130,11 @@ public class GuardrailsTest extends GuardrailTester public void testMaxThresholdUsers() throws Throwable { MaxThreshold guard = new MaxThreshold("x", - state -> 10, - state -> 100, - (isWarn, featureName, v, t) -> format("%s: for %s, %s > %s", - isWarn ? "Warning" : "Failure", featureName, v, t)); + REASON, + state -> 10, + state -> 100, + (isWarn, featureName, v, t) -> format("%s: for %s, %s > %s", + isWarn ? "Warning" : "Failure", featureName, v, t)); // value under both thresholds assertValid(() -> guard.guard(5, "x", false, null)); @@ -156,13 +160,14 @@ public class GuardrailsTest extends GuardrailTester public void testDisabledMinThreshold() throws Throwable { Threshold.ErrorMessageProvider errorMessageProvider = (isWarn, what, v, t) -> "Should never trigger"; - testDisabledThreshold(new MinThreshold("x", state -> DISABLED, state -> DISABLED, errorMessageProvider)); + testDisabledThreshold(new MinThreshold("x", REASON, state -> DISABLED, state -> DISABLED, errorMessageProvider)); } @Test public void testMinThreshold() throws Throwable { MinThreshold guard = new MinThreshold("x", + REASON, state -> 100, state -> 10, (isWarn, what, v, t) -> format("%s: for %s, %s < %s", @@ -189,6 +194,7 @@ public class GuardrailsTest extends GuardrailTester public void testWarnOnlyMinThreshold() throws Throwable { MinThreshold guard = new MinThreshold("x", + REASON, state -> 10, state -> DISABLED, (isWarn, what, v, t) -> format("%s: for %s, %s < %s", @@ -207,6 +213,7 @@ public class GuardrailsTest extends GuardrailTester public void testFailOnlyMinThreshold() throws Throwable { MinThreshold guard = new MinThreshold("x", + REASON, state -> DISABLED, state -> 10, (isWarn, what, v, t) -> format("%s: for %s, %s < %s", @@ -225,6 +232,7 @@ public class GuardrailsTest extends GuardrailTester public void testMinThresholdUsers() throws Throwable { MinThreshold guard = new MinThreshold("x", + REASON, state -> 100, state -> 10, (isWarn, what, v, t) -> format("%s: for %s, %s < %s", @@ -253,23 +261,23 @@ public class GuardrailsTest extends GuardrailTester @Test public void testEnableFlag() throws Throwable { - assertFails(() -> new EnableFlag("x", state -> false, "X").ensureEnabled(userClientState), "X is not allowed"); - assertValid(() -> new EnableFlag("x", state -> true, "X").ensureEnabled(userClientState)); + assertFails(() -> new EnableFlag("x", REASON, state -> false, "X").ensureEnabled(userClientState), "X is not allowed"); + assertValid(() -> new EnableFlag("x", REASON, state -> true, "X").ensureEnabled(userClientState)); - assertFails(() -> new EnableFlag("x", state -> false, "X").ensureEnabled("Y", userClientState), "Y is not allowed"); - assertValid(() -> new EnableFlag("x", state -> true, "X").ensureEnabled("Y", userClientState)); + assertFails(() -> new EnableFlag("x", REASON, state -> false, "X").ensureEnabled("Y", userClientState), "Y is not allowed"); + assertValid(() -> new EnableFlag("x", REASON, state -> true, "X").ensureEnabled("Y", userClientState)); } @Test public void testEnableFlagUsers() throws Throwable { - EnableFlag enabled = new EnableFlag("x", state -> true, "X"); + EnableFlag enabled = new EnableFlag("x", REASON, state -> true, "X"); assertValid(() -> enabled.ensureEnabled(null)); assertValid(() -> enabled.ensureEnabled(userClientState)); assertValid(() -> enabled.ensureEnabled(systemClientState)); assertValid(() -> enabled.ensureEnabled(superClientState)); - EnableFlag disabled = new EnableFlag("x", state -> false, "X"); + EnableFlag disabled = new EnableFlag("x", REASON, state -> false, "X"); assertFails(() -> disabled.ensureEnabled(userClientState), "X is not allowed"); assertValid(() -> disabled.ensureEnabled(systemClientState)); assertValid(() -> disabled.ensureEnabled(superClientState)); @@ -280,6 +288,7 @@ public class GuardrailsTest extends GuardrailTester { // Using a sorted set below to ensure the order in the warning message checked below is not random Values warned = new Values<>("x", + REASON, state -> insertionOrderedSet(4, 6, 20), state -> Collections.emptySet(), state -> Collections.emptySet(), @@ -300,6 +309,7 @@ public class GuardrailsTest extends GuardrailTester { // Using a sorted set below to ensure the order in the error message checked below are not random Values ignored = new Values<>("x", + REASON, state -> Collections.emptySet(), state -> insertionOrderedSet(4, 6, 20), state -> Collections.emptySet(), @@ -334,6 +344,7 @@ public class GuardrailsTest extends GuardrailTester { // Using a sorted set below to ensure the order in the error message checked below are not random Values disallowed = new Values<>("x", + REASON, state -> Collections.emptySet(), state -> Collections.emptySet(), state -> insertionOrderedSet(4, 6, 20), @@ -359,6 +370,7 @@ public class GuardrailsTest extends GuardrailTester public void testValuesUsers() throws Throwable { Values disallowed = new Values<>("x", + REASON, state -> Collections.singleton(2), state -> Collections.singleton(3), state -> Collections.singleton(4), @@ -396,6 +408,7 @@ public class GuardrailsTest extends GuardrailTester public void testPredicates() throws Throwable { Predicates guard = new Predicates<>("x", + REASON, state -> x -> x > 10, state -> x -> x > 100, (isWarn, value) -> format("%s: %s", isWarn ? "Warning" : "Aborting", value)); @@ -412,6 +425,7 @@ public class GuardrailsTest extends GuardrailTester public void testPredicatesUsers() throws Throwable { Predicates guard = new Predicates<>("x", + REASON, state -> x -> x > 10, state -> x -> x > 100, (isWarn, value) -> format("%s: %s", isWarn ? "Warning" : "Aborting", value));