diff --git a/CHANGES.txt b/CHANGES.txt index 75f12f8f0b..60eddc6442 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -9,6 +9,7 @@ * IntrusiveStack.accumulate is not accumulating correctly (CASSANDRA-20670) * Add nodetool get/setguardrailsconfig commands (CASSANDRA-19552) Merged from 4.0: + * Prevent too long table names not fitting file names (CASSANDRA-20389) * Fix IndexOutOfBoundsException in sstablemetadata tool when a range tombstone is a max clustering value (CASSANDRA-20855) * Update Jackson to 2.19.2 (CASSANDRA-20848) * Update commons-lang3 to 3.18.0 (CASSANDRA-20849) @@ -7841,5 +7842,3 @@ Full list of issues resolved in 0.4 is at https://issues.apache.org/jira/secure/ * Combined blocking and non-blocking versions of insert APIs * Added FlushPeriodInMinutes configuration parameter to force flushing of infrequently-updated ColumnFamilies - - diff --git a/src/java/org/apache/cassandra/cql3/statements/schema/AlterSchemaStatement.java b/src/java/org/apache/cassandra/cql3/statements/schema/AlterSchemaStatement.java index c3e7d8a166..dd740b8008 100644 --- a/src/java/org/apache/cassandra/cql3/statements/schema/AlterSchemaStatement.java +++ b/src/java/org/apache/cassandra/cql3/statements/schema/AlterSchemaStatement.java @@ -36,6 +36,8 @@ import org.apache.cassandra.transport.Dispatcher; import org.apache.cassandra.transport.Event.SchemaChange; import org.apache.cassandra.transport.messages.ResultMessage; +import static org.apache.cassandra.schema.KeyspaceMetadata.validateKeyspaceName; + abstract public class AlterSchemaStatement implements CQLStatement.SingleKeyspaceCqlStatement, SchemaTransformation { protected final String keyspaceName; // name of the keyspace affected by the statement @@ -109,7 +111,7 @@ abstract public class AlterSchemaStatement implements CQLStatement.SingleKeyspac if (null != keyspace && keyspace.isVirtual()) throw ire("Virtual keyspace '%s' is not user-modifiable", keyspaceName); - validateKeyspaceName(); + validateKeyspaceName(keyspaceName, AlterSchemaStatement::ire); SchemaTransformationResult result = Schema.instance.transform(this, locally); @@ -132,16 +134,6 @@ abstract public class AlterSchemaStatement implements CQLStatement.SingleKeyspac return new ResultMessage.SchemaChange(schemaChangeEvent(result.diff)); } - private void validateKeyspaceName() - { - if (!SchemaConstants.isValidName(keyspaceName)) - { - throw ire("Keyspace name must not be empty, more than %d characters long, " + - "or contain non-alphanumeric-underscore characters (got '%s')", - SchemaConstants.NAME_LENGTH, keyspaceName); - } - } - private void grantPermissionsOnResource(IResource resource, AuthenticatedUser user) { try diff --git a/src/java/org/apache/cassandra/db/Directories.java b/src/java/org/apache/cassandra/db/Directories.java index 3b20d7af2e..01756ba305 100644 --- a/src/java/org/apache/cassandra/db/Directories.java +++ b/src/java/org/apache/cassandra/db/Directories.java @@ -102,6 +102,7 @@ public class Directories public static final String SNAPSHOT_SUBDIR = "snapshots"; public static final String TMP_SUBDIR = "tmp"; public static final String SECONDARY_INDEX_NAME_SEPARATOR = "."; + public static final String TABLE_DIRECTORY_NAME_SEPARATOR = "-"; /** * The directories used to store keyspaces data. @@ -209,14 +210,11 @@ public class Directories this.metadata = metadata; this.paths = paths; ImmutableMap.Builder canonicalPathsBuilder = ImmutableMap.builder(); - String tableId = metadata.id.toHexString(); - int idx = metadata.name.indexOf(SECONDARY_INDEX_NAME_SEPARATOR); - String cfName = idx >= 0 ? metadata.name.substring(0, idx) : metadata.name; - String indexNameWithDot = idx >= 0 ? metadata.name.substring(idx) : null; + String indexNameWithDot = metadata.getIndexNameWithDot(); this.dataPaths = new File[paths.length]; // If upgraded from version less than 2.1, use existing directories - String oldSSTableRelativePath = join(metadata.keyspace, cfName); + String oldSSTableRelativePath = join(metadata.keyspace, metadata.getTableName()); for (int i = 0; i < paths.length; ++i) { // check if old SSTable directory exists @@ -229,7 +227,7 @@ public class Directories { canonicalPathsBuilder = ImmutableMap.builder(); // use 2.1+ style - String newSSTableRelativePath = join(metadata.keyspace, cfName + '-' + tableId); + String newSSTableRelativePath = join(metadata.keyspace, metadata.getTableDirectoryName()); for (int i = 0; i < paths.length; ++i) { File dataPath = new File(paths[i].location, newSSTableRelativePath); diff --git a/src/java/org/apache/cassandra/schema/IndexMetadata.java b/src/java/org/apache/cassandra/schema/IndexMetadata.java index 4adbce170b..a82923d538 100644 --- a/src/java/org/apache/cassandra/schema/IndexMetadata.java +++ b/src/java/org/apache/cassandra/schema/IndexMetadata.java @@ -21,7 +21,6 @@ package org.apache.cassandra.schema; import java.io.IOException; import java.lang.reflect.InvocationTargetException; import java.util.*; -import java.util.regex.Pattern; import java.util.stream.Collectors; import com.google.common.base.Objects; @@ -42,6 +41,9 @@ import org.apache.cassandra.io.util.DataOutputPlus; import org.apache.cassandra.utils.FBUtilities; import org.apache.cassandra.utils.UUIDSerializer; +import static org.apache.cassandra.schema.SchemaConstants.PATTERN_NON_WORD_CHAR; +import static org.apache.cassandra.schema.SchemaConstants.isValidCharsName; + /** * An immutable representation of secondary index metadata. */ @@ -49,10 +51,6 @@ public final class IndexMetadata { private static final Logger logger = LoggerFactory.getLogger(IndexMetadata.class); - private static final Pattern PATTERN_NON_WORD_CHAR = Pattern.compile("\\W"); - private static final Pattern PATTERN_WORD_CHARS = Pattern.compile("\\w+"); - - public static final Serializer serializer = new Serializer(); public enum Kind @@ -94,11 +92,6 @@ public final class IndexMetadata return new IndexMetadata(name, newOptions, kind); } - public static boolean isNameValid(String name) - { - return name != null && !name.isEmpty() && PATTERN_WORD_CHARS.matcher(name).matches(); - } - public static String generateDefaultIndexName(String table, ColumnIdentifier column) { return PATTERN_NON_WORD_CHAR.matcher(table + "_" + column.toString() + "_idx").replaceAll(""); @@ -111,7 +104,8 @@ public final class IndexMetadata public void validate(TableMetadata table) { - if (!isNameValid(name)) + // TODO: address validating the length by CASSANDRA-20445 + if (!isValidCharsName(name)) throw new ConfigurationException("Illegal index name " + name); if (kind == null) diff --git a/src/java/org/apache/cassandra/schema/KeyspaceMetadata.java b/src/java/org/apache/cassandra/schema/KeyspaceMetadata.java index 3e631e310a..388465a6b6 100644 --- a/src/java/org/apache/cassandra/schema/KeyspaceMetadata.java +++ b/src/java/org/apache/cassandra/schema/KeyspaceMetadata.java @@ -34,6 +34,7 @@ import org.apache.cassandra.cql3.functions.UDAggregate; import org.apache.cassandra.cql3.functions.UDFunction; import org.apache.cassandra.db.marshal.UserType; import org.apache.cassandra.exceptions.ConfigurationException; +import org.apache.cassandra.exceptions.RequestValidationException; import org.apache.cassandra.locator.AbstractReplicationStrategy; import org.apache.cassandra.schema.Functions.FunctionsDiff; import org.apache.cassandra.schema.Tables.TablesDiff; @@ -44,12 +45,30 @@ import org.apache.cassandra.service.StorageService; import static java.lang.String.format; import static com.google.common.collect.Iterables.any; +import static org.apache.cassandra.schema.SchemaConstants.TABLE_NAME_LENGTH; /** * An immutable representation of keyspace metadata (name, params, tables, types, and functions). */ public final class KeyspaceMetadata implements SchemaElement { + /** + * Validates the keyspace name for valid characters and correct length. + * Throws an exception if it's invalid. + * + * @param keyspaceName The name of the keyspace to validate + * @param exceptionBuilder The exception constructor to throw if validation fails + */ + public static void validateKeyspaceName(String keyspaceName, java.util.function.Function exceptionBuilder) + { + if (!SchemaConstants.isValidCharsName(keyspaceName)) + throw exceptionBuilder.apply(format("Keyspace name must not be empty and must contain alphanumeric or underscore characters only (got \"%s\")", + keyspaceName)); + if (keyspaceName.length() > SchemaConstants.NAME_LENGTH) + throw exceptionBuilder.apply(format("Keyspace name must not be more than %d characters long (got %d characters for \"%s\")", + TABLE_NAME_LENGTH, keyspaceName.length(), keyspaceName)); + } + public enum Kind { REGULAR, VIRTUAL @@ -306,16 +325,8 @@ public final class KeyspaceMetadata implements SchemaElement public void validate() { - if (!SchemaConstants.isValidName(name)) - { - throw new ConfigurationException(format("Keyspace name must not be empty, more than %s characters long, " - + "or contain non-alphanumeric-underscore characters (got \"%s\")", - SchemaConstants.NAME_LENGTH, - name)); - } - + validateKeyspaceName(name, ConfigurationException::new); params.validate(name, null); - tablesAndViews().forEach(TableMetadata::validate); Set indexNames = new HashSet<>(); diff --git a/src/java/org/apache/cassandra/schema/SchemaConstants.java b/src/java/org/apache/cassandra/schema/SchemaConstants.java index d4cf888a9f..939a3b17ea 100644 --- a/src/java/org/apache/cassandra/schema/SchemaConstants.java +++ b/src/java/org/apache/cassandra/schema/SchemaConstants.java @@ -29,12 +29,15 @@ import com.google.common.collect.Sets; import org.apache.cassandra.db.Digest; +import static org.apache.cassandra.db.Directories.TABLE_DIRECTORY_NAME_SEPARATOR; + /** * When adding new String keyspace names here, double check if it needs to be added to PartitionDenylist.canDenylistKeyspace */ public final class SchemaConstants { public static final Pattern PATTERN_WORD_CHARS = Pattern.compile("\\w+"); + public static final Pattern PATTERN_NON_WORD_CHAR = Pattern.compile("\\W"); public static final String SYSTEM_KEYSPACE_NAME = "system"; public static final String SCHEMA_KEYSPACE_NAME = "system_schema"; @@ -66,14 +69,37 @@ public final class SchemaConstants */ public static final int NAME_LENGTH = 48; + /** + * Longest acceptable file name. Longer names lead to too long file name error. + */ + public static final int FILENAME_LENGTH = 255; + + /** + * Length of a table uuid as a hex string. + */ + public static final int TABLE_UUID_AS_HEX_LENGTH = 32; + + /** + * Longest acceptable table name, so it can be used in a directory + * name constructed with a suffix of a table id and a separator. + */ + public static final int TABLE_NAME_LENGTH = FILENAME_LENGTH - TABLE_UUID_AS_HEX_LENGTH - TABLE_DIRECTORY_NAME_SEPARATOR.length(); + // 59adb24e-f3cd-3e02-97f0-5b395827453f public static final UUID emptyVersion; public static final List LEGACY_AUTH_TABLES = Arrays.asList("credentials", "users", "permissions"); - public static boolean isValidName(String name) + /** + * Validates that a name is not empty and contains only alphanumeric characters or + * underscore, so it can be used in file or directory names. + * + * @param name the name to check + * @return whether the non-empty name contains only valid characters + */ + public static boolean isValidCharsName(String name) { - return name != null && !name.isEmpty() && name.length() <= NAME_LENGTH && PATTERN_WORD_CHARS.matcher(name).matches(); + return name != null && !name.isEmpty() && PATTERN_WORD_CHARS.matcher(name).matches(); } static diff --git a/src/java/org/apache/cassandra/schema/TableMetadata.java b/src/java/org/apache/cassandra/schema/TableMetadata.java index 3234998244..d3a01ebc96 100644 --- a/src/java/org/apache/cassandra/schema/TableMetadata.java +++ b/src/java/org/apache/cassandra/schema/TableMetadata.java @@ -49,7 +49,12 @@ import static com.google.common.collect.Iterables.transform; import static java.lang.String.format; import static java.util.stream.Collectors.toList; import static java.util.stream.Collectors.toSet; -import static org.apache.cassandra.schema.IndexMetadata.isNameValid; +import static org.apache.cassandra.db.Directories.SECONDARY_INDEX_NAME_SEPARATOR; +import static org.apache.cassandra.db.Directories.TABLE_DIRECTORY_NAME_SEPARATOR; +import static org.apache.cassandra.schema.KeyspaceMetadata.validateKeyspaceName; +import static org.apache.cassandra.schema.SchemaConstants.FILENAME_LENGTH; +import static org.apache.cassandra.schema.SchemaConstants.TABLE_NAME_LENGTH; +import static org.apache.cassandra.schema.SchemaConstants.isValidCharsName; @Unmetered public class TableMetadata implements SchemaElement @@ -416,11 +421,9 @@ public class TableMetadata implements SchemaElement public void validate() { - if (!isNameValid(keyspace)) - except("Keyspace name must not be empty, more than %s characters long, or contain non-alphanumeric-underscore characters (got \"%s\")", SchemaConstants.NAME_LENGTH, keyspace); + validateKeyspaceName(keyspace, this::prepareConfigurationException); - if (!isNameValid(name)) - except("Table name must not be empty, more than %s characters long, or contain non-alphanumeric-underscore characters (got \"%s\")", SchemaConstants.NAME_LENGTH, name); + validateTableName(); params.validate(); @@ -448,6 +451,17 @@ public class TableMetadata implements SchemaElement indexes.validate(this); } + private void validateTableName() + { + if (!isValidCharsName(name)) + except("Table name must not be empty or not contain non-alphanumeric-underscore characters (got \"%s\")", name); + + if (name.length() > TABLE_NAME_LENGTH) + except("Table name must not be more than %d characters long (got %d characters for \"%s\")", TABLE_NAME_LENGTH, name.length(), name); + + assert getTableDirectoryName().length() <= FILENAME_LENGTH : String.format("Generated directory name for a table of %d characters doesn't fit the max filename legnth of %s. This unexpectedly wasn't prevented by check of the table name length, %d, to fit %d characters (got table name \"%s\" and generated directory name \"%s\"", getTableDirectoryName().length(), FILENAME_LENGTH, name.length(), TABLE_NAME_LENGTH, name, getTableDirectoryName()); + } + /** * To support backward compatibility with thrift super columns in the C* 3.0+ storage engine, we encode said super * columns as a CQL {@code map}. To ensure the name of this map did not conflict with any other user @@ -544,7 +558,41 @@ public class TableMetadata implements SchemaElement public String indexTableName(IndexMetadata info) { // TODO simplify this when info.index_name is guaranteed to be set - return name + Directories.SECONDARY_INDEX_NAME_SEPARATOR + info.name; + return name + SECONDARY_INDEX_NAME_SEPARATOR + info.name; + } + + /** + * Returns the table part of the index table name or the entire table name + * if not an index table. + * @return table name part + */ + public String getTableName() + { + int idx = name.indexOf(SECONDARY_INDEX_NAME_SEPARATOR); + return idx >= 0 ? name.substring(0, idx) : name; + } + + /** + * Generates a directory name for the table by using table part of + * the (index) table name and table id. + * @return directory name + */ + public String getTableDirectoryName() + { + return getTableName() + TABLE_DIRECTORY_NAME_SEPARATOR + id.toHexString(); + } + + /** + * Returns the index name from the name of an index table + * including the dot prefixing the index name, see {@link #indexTableName}. + * If not an index table, returns null. + * @return index name prefixed with dot prefix or null + */ + @Nullable + public String getIndexNameWithDot() + { + int idx = name.indexOf(SECONDARY_INDEX_NAME_SEPARATOR); + return idx >= 0 ? name.substring(idx) : null; } /** @@ -602,9 +650,14 @@ public class TableMetadata implements SchemaElement return builder.build(); } + private ConfigurationException prepareConfigurationException(String format, Object... args) + { + return new ConfigurationException(keyspace + '.' + name + ": " + format(format, args)); + } + protected void except(String format, Object... args) { - throw new ConfigurationException(keyspace + "." + name + ": " + format(format, args)); + throw prepareConfigurationException(format, args); } @Override diff --git a/test/unit/org/apache/cassandra/cql3/CQLTester.java b/test/unit/org/apache/cassandra/cql3/CQLTester.java index c20a0cea2a..7615de598b 100644 --- a/test/unit/org/apache/cassandra/cql3/CQLTester.java +++ b/test/unit/org/apache/cassandra/cql3/CQLTester.java @@ -47,6 +47,7 @@ import com.google.common.base.Strings; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Iterables; +import org.apache.commons.lang3.ArrayUtils; import org.junit.*; import org.slf4j.Logger; @@ -663,6 +664,21 @@ public abstract class CQLTester Util.flush(store); } + public void flush(String keyspace, String table1, String... tables) + { + tables = ArrayUtils.add(tables, table1); + for (ColumnFamilyStore store : getTables(keyspace, tables)) + Util.flush(store); + } + + private List getTables(String keyspace, String[] tables) + { + List stores = new ArrayList<>(tables.length); + for (String name : tables) + stores.add(getColumnFamilyStore(keyspace, name)); + return stores; + } + public void disableCompaction(String keyspace) { ColumnFamilyStore store = getCurrentColumnFamilyStore(keyspace); diff --git a/test/unit/org/apache/cassandra/schema/CreateTableValidationTest.java b/test/unit/org/apache/cassandra/schema/CreateTableValidationTest.java index 9682b4f57d..23b17bdb2b 100644 --- a/test/unit/org/apache/cassandra/schema/CreateTableValidationTest.java +++ b/test/unit/org/apache/cassandra/schema/CreateTableValidationTest.java @@ -21,6 +21,8 @@ package org.apache.cassandra.schema; import java.io.IOException; import java.util.List; +import org.apache.commons.lang3.StringUtils; + import org.apache.cassandra.config.DatabaseDescriptor; import org.apache.cassandra.cql3.CQLTester; import org.apache.cassandra.cql3.QueryOptions; @@ -33,6 +35,10 @@ import org.apache.cassandra.transport.messages.QueryMessage; import org.junit.Test; +import static org.apache.cassandra.schema.SchemaConstants.FILENAME_LENGTH; +import static org.apache.cassandra.schema.SchemaConstants.NAME_LENGTH; +import static org.apache.cassandra.schema.SchemaConstants.TABLE_NAME_LENGTH; +import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatExceptionOfType; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; @@ -157,6 +163,42 @@ public class CreateTableValidationTest extends CQLTester "Missing CLUSTERING ORDER for column ck1"); } + @Test + public void testCreatingTableWithLongName() throws Throwable + { + int tableIdSuffix = "-1b255f4def2540a60000000000000000".length(); + String keyspaceName = StringUtils.repeat("k", NAME_LENGTH); + String tableName = StringUtils.repeat("t", FILENAME_LENGTH - tableIdSuffix); + String tooLongTableName = StringUtils.repeat("l", FILENAME_LENGTH - tableIdSuffix + 1); + + execute(String.format("CREATE KEYSPACE %s with replication = " + + "{ 'class' : 'SimpleStrategy', 'replication_factor' : 1 }", + keyspaceName)); + + assertInvalidMessage(String.format("%s.%s: Table name must not be more than %d characters long (got %d characters for \"%2$s\")", + keyspaceName, tooLongTableName, TABLE_NAME_LENGTH, tooLongTableName.length()), + String.format("CREATE TABLE %s.%s (" + + "key int PRIMARY KEY," + + "val int)", keyspaceName, tooLongTableName)); + + createTable(String.format("CREATE TABLE %s.%s (" + + "key int PRIMARY KEY," + + "val int)", keyspaceName, tableName)); + execute(String.format("INSERT INTO %s.%s (key,val) VALUES (1,1)", keyspaceName, tableName)); + assertThat(execute(String.format("SELECT * from %s.%s", keyspaceName, tableName))).hasSize(1); + flush(keyspaceName, tableName); + assertThat(execute(String.format("SELECT * from %s.%s", keyspaceName, tableName))).hasSize(1); + } + + @Test + public void testNonAlphanummericTableName() throws Throwable + { + assertInvalidMessage(String.format("%s.d-3: Table name must not be empty or not contain non-alphanumeric-underscore characters (got \"d-3\")", KEYSPACE), + String.format("CREATE TABLE %s.\"d-3\" (key int PRIMARY KEY, val int)", KEYSPACE)); + assertInvalidMessage(String.format("%s. : Table name must not be empty or not contain non-alphanumeric-underscore characters (got \" \")", KEYSPACE), + String.format("CREATE TABLE %s.\" \" (key int PRIMARY KEY, val int)", KEYSPACE)); + } + private void expectedFailure(String statement, String errorMsg) { diff --git a/test/unit/org/apache/cassandra/schema/IndexMetadataTest.java b/test/unit/org/apache/cassandra/schema/IndexMetadataTest.java index c9e0d52a0e..0bcd0dfc28 100644 --- a/test/unit/org/apache/cassandra/schema/IndexMetadataTest.java +++ b/test/unit/org/apache/cassandra/schema/IndexMetadataTest.java @@ -25,29 +25,8 @@ import org.junit.Test; import org.apache.cassandra.cql3.ColumnIdentifier; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertTrue; - -public class IndexMetadataTest { - - @Test - public void testIsNameValidPositive() - { - assertTrue(IndexMetadata.isNameValid("abcdefghijklmnopqrstuvwxyz")); - assertTrue(IndexMetadata.isNameValid("ABCDEFGHIJKLMNOPQRSTUVWXYZ")); - assertTrue(IndexMetadata.isNameValid("_01234567890")); - } - - @Test - public void testIsNameValidNegative() - { - assertFalse(IndexMetadata.isNameValid(null)); - assertFalse(IndexMetadata.isNameValid("")); - assertFalse(IndexMetadata.isNameValid(" ")); - assertFalse(IndexMetadata.isNameValid("@")); - assertFalse(IndexMetadata.isNameValid("!")); - } - +public class IndexMetadataTest +{ @Test public void testGetDefaultIndexName() { diff --git a/test/unit/org/apache/cassandra/schema/MigrationManagerTest.java b/test/unit/org/apache/cassandra/schema/MigrationManagerTest.java index e1556b7a52..b37fe986fe 100644 --- a/test/unit/org/apache/cassandra/schema/MigrationManagerTest.java +++ b/test/unit/org/apache/cassandra/schema/MigrationManagerTest.java @@ -25,6 +25,10 @@ import java.util.function.Supplier; import com.google.common.collect.ImmutableMap; import org.apache.cassandra.io.util.File; + +import org.apache.commons.lang3.StringUtils; + +import org.apache.cassandra.exceptions.InvalidRequestException; import org.junit.BeforeClass; import org.junit.Rule; import org.junit.Test; @@ -51,6 +55,8 @@ import org.apache.cassandra.utils.FBUtilities; import static org.apache.cassandra.Util.throwAssert; import static org.apache.cassandra.cql3.CQLTester.assertRows; import static org.apache.cassandra.cql3.CQLTester.row; +import static org.apache.cassandra.schema.SchemaConstants.NAME_LENGTH; +import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; @@ -130,13 +136,21 @@ public class MigrationManagerTest @Test public void testInvalidNames() { - String[] valid = {"1", "a", "_1", "b_", "__", "1_a"}; + String[] valid = { "1", "a", "_1", "b_", "__", "1_a" }; for (String s : valid) - assertTrue(SchemaConstants.isValidName(s)); - - String[] invalid = {"b@t", "dash-y", "", " ", "dot.s", ".hidden"}; + { + assertTrue(SchemaConstants.isValidCharsName(s)); + KeyspaceMetadata.validateKeyspaceName(s, InvalidRequestException::new); + } + String[] invalid = { "b@t", "dash-y", "", " ", "dot.s", ".hidden" }; for (String s : invalid) - assertFalse(SchemaConstants.isValidName(s)); + { + assertFalse(SchemaConstants.isValidCharsName(s)); + assertThatThrownBy(() -> KeyspaceMetadata.validateKeyspaceName(s, InvalidRequestException::new)).isInstanceOf(InvalidRequestException.class); + } + + KeyspaceMetadata.validateKeyspaceName(StringUtils.repeat("k", NAME_LENGTH), InvalidRequestException::new); + assertThatThrownBy(() -> KeyspaceMetadata.validateKeyspaceName(StringUtils.repeat("k", NAME_LENGTH + 1), InvalidRequestException::new)).isInstanceOf(InvalidRequestException.class); } @Test @@ -477,14 +491,22 @@ public class MigrationManagerTest } @Test - public void testValidateNullKeyspace() throws Exception + public void testTableMetadataBuilderWithInvalidKeyspace() { - TableMetadata.Builder builder = TableMetadata.builder(null, TABLE1).addPartitionKeyColumn("partitionKey", BytesType.instance); - - TableMetadata table1 = builder.build(); - thrown.expect(ConfigurationException.class); - thrown.expectMessage(null + "." + TABLE1 + ": Keyspace name must not be empty"); - table1.validate(); + String[][] keyspaces = + { + { null, "Keyspace name must not be empty" }, + { "a@b", "Keyspace name must not be empty and must contain alphanumeric or underscore characters only" }, + { StringUtils.repeat("k", NAME_LENGTH), String.format("Keyspace name must not be more than %d characters long", NAME_LENGTH) } + }; + for (String[] keyspace : keyspaces) + { + TableMetadata.Builder builder = TableMetadata.builder(keyspace[0], TABLE1).addPartitionKeyColumn("partitionKey", BytesType.instance); + TableMetadata table1 = builder.build(); + thrown.expect(ConfigurationException.class); + thrown.expectMessage(String.format("%s.%s: %s", keyspace[0], TABLE1, keyspace[1])); + table1.validate(); + } } @Test diff --git a/test/unit/org/apache/cassandra/schema/ValidationTest.java b/test/unit/org/apache/cassandra/schema/ValidationTest.java index 8eb1247c5b..7af8112d82 100644 --- a/test/unit/org/apache/cassandra/schema/ValidationTest.java +++ b/test/unit/org/apache/cassandra/schema/ValidationTest.java @@ -32,19 +32,19 @@ public class ValidationTest @Test public void testIsNameValidPositive() { - assertTrue(SchemaConstants.isValidName("abcdefghijklmnopqrstuvwxyz")); - assertTrue(SchemaConstants.isValidName("ABCDEFGHIJKLMNOPQRSTUVWXYZ")); - assertTrue(SchemaConstants.isValidName("_01234567890")); + assertTrue(SchemaConstants.isValidCharsName("abcdefghijklmnopqrstuvwxyz")); + assertTrue(SchemaConstants.isValidCharsName("ABCDEFGHIJKLMNOPQRSTUVWXYZ")); + assertTrue(SchemaConstants.isValidCharsName("_01234567890")); } @Test public void testIsNameValidNegative() { - assertFalse(SchemaConstants.isValidName(null)); - assertFalse(SchemaConstants.isValidName("")); - assertFalse(SchemaConstants.isValidName(" ")); - assertFalse(SchemaConstants.isValidName("@")); - assertFalse(SchemaConstants.isValidName("!")); + assertFalse(SchemaConstants.isValidCharsName(null)); + assertFalse(SchemaConstants.isValidCharsName("")); + assertFalse(SchemaConstants.isValidCharsName(" ")); + assertFalse(SchemaConstants.isValidCharsName("@")); + assertFalse(SchemaConstants.isValidCharsName("!")); } private static Set primitiveTypes =