CNDB-12683 make name validation functions clear

src/java/org/apache/cassandra/cql3/statements/schema/AlterSchemaStatement.java

@@ -28,7 +28,6 @@
 import org.apache.cassandra.config.DatabaseDescriptor;
 import org.apache.cassandra.cql3.CQLStatement;
 import org.apache.cassandra.cql3.QueryOptions;
-import org.apache.cassandra.cql3.statements.RawKeyspaceAwareStatement;
 import org.apache.cassandra.exceptions.InvalidRequestException;
 import org.apache.cassandra.schema.KeyspaceMetadata;
 import org.apache.cassandra.schema.Keyspaces.KeyspacesDiff;
@@ -147,7 +146,7 @@ public ResultMessage execute(QueryState state, boolean locally)
 
     private void validateKeyspaceName()
     {
-        if (!SchemaConstants.isValidName(keyspaceName))
+        if (!SchemaConstants.isNameSafeForFilename(keyspaceName))
         {
             throw ire("Keyspace name must not be empty, more than %d characters long, " +
                       "or contain non-alphanumeric-underscore characters (got '%s')",

src/java/org/apache/cassandra/cql3/statements/schema/CreateIndexStatement.java

@@ -224,7 +224,7 @@ private void validateIndexTarget(TableMetadata table, IndexMetadata.Kind kind, I
         if (null == column)
             throw ire("Column '%s' doesn't exist", target.column);
 
-        if ((kind == IndexMetadata.Kind.CUSTOM) && !SchemaConstants.isValidName(target.column.toString()))
+        if ((kind == IndexMetadata.Kind.CUSTOM) && !SchemaConstants.isNameSafeForFilename(target.column.toString()))
             throw ire("Column '%s' is longer than the permissible name length of %d characters or" +
                       " contains non-alphanumeric-underscore characters", target.column, SchemaConstants.NAME_LENGTH);
 

src/java/org/apache/cassandra/db/compaction/unified/AdaptiveController.java

@@ -210,15 +210,15 @@ static Controller fromOptions(Environment env,
         {
             logger.warn("Unable to parse saved options. Using starting value instead:", e);
         }
-        catch (FSError e)
-        {
-            logger.warn("Unable to read controller config file. Using starting value instead:", e);
-        }
-        catch (Throwable e)
-        {
-            logger.warn("Unable to read controller config file. Using starting value instead:", e);
-            JVMStabilityInspector.inspectThrowable(e);
-        }
+//        catch (FSError e)
+//        {
+//            logger.warn("Unable to read controller config file. Using starting value instead:", e);
+//        }
+//        catch (Throwable e)
+//        {
+//            logger.warn("Unable to read controller config file. Using starting value instead:", e);
+//            JVMStabilityInspector.inspectThrowable(e);
+//        }
 
         if (scalingParameters == null)
         {

src/java/org/apache/cassandra/db/compaction/unified/Controller.java

@@ -425,15 +425,15 @@ public static void storeOptions(String keyspaceName, String tableName, int[] sca
 
             logger.debug(String.format("Writing current scaling parameters and flush size to file %s: %s", f.toPath().toString(), jsonObject));
         }
-        catch (IOException | FSError e)
+        catch (IOException e)
         {
             logger.warn("Unable to save current scaling parameters and flush size. Current controller configuration will be lost if a node restarts: ", e);
         }
-        catch (Throwable e)
-        {
-            logger.warn("Unable to save current scaling parameters and flush size. Current controller configuration will be lost if a node restarts: ", e);
-            JVMStabilityInspector.inspectThrowable(e);
-        }
+//        catch (Throwable e)
+//        {
+//            logger.warn("Unable to save current scaling parameters and flush size. Current controller configuration will be lost if a node restarts: ", e);
+//            JVMStabilityInspector.inspectThrowable(e);
+//        }
     }
 
     public abstract void storeControllerConfig();

src/java/org/apache/cassandra/db/compaction/unified/StaticController.java

@@ -152,15 +152,15 @@ static Controller fromOptions(Environment env,
         {
             logger.warn("Unable to parse saved flush size. Using starting value instead:", e);
         }
-        catch (FSError e)
-        {
-            logger.warn("Unable to read controller config file. Using starting value instead:", e);
-        }
-        catch (Throwable e)
-        {
-            logger.warn("Unable to read controller config file. Using starting value instead:", e);
-            JVMStabilityInspector.inspectThrowable(e);
-        }
+//        catch (FSError e)
+//        {
+//            logger.warn("Unable to read controller config file. Using starting value instead:", e);
+//        }
+//        catch (Throwable e)
+//        {
+//            logger.warn("Unable to read controller config file. Using starting value instead:", e);
+//            JVMStabilityInspector.inspectThrowable(e);
+//        }
         return new StaticController(env,
                                     scalingParameters,
                                     survivalFactors,

src/java/org/apache/cassandra/schema/IndexMetadata.java

@@ -46,6 +46,8 @@
 import org.apache.cassandra.utils.FBUtilities;
 import org.apache.cassandra.utils.UUIDSerializer;
 
+import static org.apache.cassandra.schema.SchemaConstants.isValidCharsName;
+
 /**
  * An immutable representation of secondary index metadata.
  */
@@ -108,11 +110,6 @@ public static IndexMetadata fromIndexTargets(List<IndexTarget> targets,
         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("");
@@ -125,7 +122,7 @@ public static String generateDefaultIndexName(String table)
 
     public void validate(TableMetadata table)
     {
-        if (!isNameValid(name))
+        if (!isValidCharsName(name))
             throw new ConfigurationException("Illegal index name " + name);
 
         if (kind == null)

src/java/org/apache/cassandra/schema/KeyspaceMetadata.java

@@ -363,7 +363,7 @@ public String toCqlString(boolean withInternals, boolean ifNotExists)
 
     public void validate()
     {
-        if (!SchemaConstants.isValidName(name))
+        if (!SchemaConstants.isNameSafeForFilename(name))
         {
             throw new ConfigurationException(format("Keyspace name must not be empty, more than %s characters long, "
                                                     + "or contain non-alphanumeric-underscore characters (got \"%s\")",

src/java/org/apache/cassandra/schema/SchemaConstants.java

@@ -72,9 +72,31 @@ public final class SchemaConstants
 
     public static final List<String> LEGACY_AUTH_TABLES = Arrays.asList("credentials", "users", "permissions");
 
-    public static boolean isValidName(String name)
+    public static boolean isSafeLengthForFilename(String name)
     {
-        return name != null && !name.isEmpty() && name.length() <= NAME_LENGTH && PATTERN_WORD_CHARS.matcher(name).matches();
+        return name.length() <= NAME_LENGTH;
+    }
+
+    /**
+     * Names such as keyspace, table, index names are used in file paths and file names,
+     * so, they need to be safe for the use there, i.e., short enough and
+     * containing only alphanumeric characters and underscores.
+     * @param name the name to check
+     * @return whether the name is safe for use in file paths and file names
+     */
+    public static boolean isNameSafeForFilename(String name)
+    {
+        return name != null && !name.isEmpty() && isSafeLengthForFilename(name) && PATTERN_WORD_CHARS.matcher(name).matches();
+    }
+
+    /**
+     * Checks if the name contains only alphanumeric characters and underscores and is not empty.
+     * @param name the name to check
+     * @return true if the name is valid, false otherwise
+     */
+    public static boolean isValidCharsName(String name)
+    {
+        return name != null && !name.isEmpty() && PATTERN_WORD_CHARS.matcher(name).matches();
     }
 
     static

src/java/org/apache/cassandra/schema/TableMetadata.java

@@ -51,7 +51,7 @@
 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.schema.SchemaConstants.isValidCharsName;
 
 @Unmetered
 public class TableMetadata implements SchemaElement
@@ -439,11 +439,11 @@ public final void validate()
 
     public void validate(boolean durationLegacyMode)
     {
-        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);
+        if (!isValidCharsName(keyspace))
+            except("Keyspace name must not be empty or contain non-alphanumeric-underscore characters (got \"%s\")", SchemaConstants.NAME_LENGTH, keyspace);
 
-        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);
+        if (!isValidCharsName(name))
+            except("Table name must not be empty or contain non-alphanumeric-underscore characters (got \"%s\")", SchemaConstants.NAME_LENGTH, name);
 
         params.validate();
 

test/unit/org/apache/cassandra/schema/CreateTableValidationTest.java

@@ -20,6 +20,7 @@
 
 import org.apache.cassandra.cql3.CQLTester;
 import org.apache.cassandra.cql3.UntypedResultSet;
+import org.apache.cassandra.cql3.functions.types.ParseUtils;
 import org.apache.cassandra.exceptions.ConfigurationException;
 import org.apache.cassandra.exceptions.InvalidRequestException;
 
@@ -105,7 +106,7 @@ public void testCreateTableWithMissingClusteringColumn()
     @Test
     public void testCreatingTableWithLongName() throws Throwable
     {
-        String keyspace = "g38373639353166362d356631322d343864652d393063362d653862616534343165333764_tpch";
+        String keyspace = "\"38373639353166362d356631322d343864652d393063362d653862616534343165333764_tpch\"";
         String table = "test_create_k8yq1r75bpzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzz";
 
         execute(String.format("CREATE KEYSPACE %s with replication = " +
@@ -116,7 +117,7 @@ public void testCreatingTableWithLongName() throws Throwable
                                            "val int)", keyspace, table));
 
         execute(String.format("INSERT INTO %s.%s (key,val) VALUES (1,1)", keyspace, table));
-        flush(keyspace, table);
+        flush(ParseUtils.unDoubleQuote(keyspace), table);
         UntypedResultSet result = execute(String.format("SELECT * from %s.%s", keyspace, table));
         assertThat(result.size()).isEqualTo(1);
     }

test/unit/org/apache/cassandra/schema/IndexMetadataTest.java

@@ -25,27 +25,28 @@
 
 import org.apache.cassandra.cql3.ColumnIdentifier;
 
+import static org.apache.cassandra.schema.SchemaConstants.isValidCharsName;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
 
 public class IndexMetadataTest {
 
     @Test
-    public void testIsNameValidPositive()
+    public void testIsValidCharsNamePositive()
     {
-        assertTrue(IndexMetadata.isNameValid("abcdefghijklmnopqrstuvwxyz"));
-        assertTrue(IndexMetadata.isNameValid("ABCDEFGHIJKLMNOPQRSTUVWXYZ"));
-        assertTrue(IndexMetadata.isNameValid("_01234567890"));
+        assertTrue(isValidCharsName("abcdefghijklmnopqrstuvwxyz"));
+        assertTrue(isValidCharsName("ABCDEFGHIJKLMNOPQRSTUVWXYZ"));
+        assertTrue(isValidCharsName("_01234567890"));
     }
 
     @Test
-    public void testIsNameValidNegative()
+    public void testIsValidCharsNameNegative()
     {
-        assertFalse(IndexMetadata.isNameValid(null));
-        assertFalse(IndexMetadata.isNameValid(""));
-        assertFalse(IndexMetadata.isNameValid(" "));
-        assertFalse(IndexMetadata.isNameValid("@"));
-        assertFalse(IndexMetadata.isNameValid("!"));
+        assertFalse(isValidCharsName(null));
+        assertFalse(isValidCharsName(""));
+        assertFalse(isValidCharsName(" "));
+        assertFalse(isValidCharsName("@"));
+        assertFalse(isValidCharsName("!"));
     }
 
     @Test

test/unit/org/apache/cassandra/schema/MigrationManagerTest.java

@@ -138,11 +138,11 @@ public void testInvalidNames()
     {
         String[] valid = {"1", "a", "_1", "b_", "__", "1_a"};
         for (String s : valid)
-            assertTrue(SchemaConstants.isValidName(s));
+            assertTrue(SchemaConstants.isNameSafeForFilename(s));
 
         String[] invalid = {"b@t", "dash-y", "", " ", "dot.s", ".hidden"};
         for (String s : invalid)
-            assertFalse(SchemaConstants.isValidName(s));
+            assertFalse(SchemaConstants.isNameSafeForFilename(s));
     }
 
     @Test

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.isNameSafeForFilename("abcdefghijklmnopqrstuvwxyz"));
+         assertTrue(SchemaConstants.isNameSafeForFilename("ABCDEFGHIJKLMNOPQRSTUVWXYZ"));
+         assertTrue(SchemaConstants.isNameSafeForFilename("_01234567890"));
     }
 
     @Test
     public void testIsNameValidNegative()
     {
-        assertFalse(SchemaConstants.isValidName(null));
-        assertFalse(SchemaConstants.isValidName(""));
-        assertFalse(SchemaConstants.isValidName(" "));
-        assertFalse(SchemaConstants.isValidName("@"));
-        assertFalse(SchemaConstants.isValidName("!"));
+        assertFalse(SchemaConstants.isNameSafeForFilename(null));
+        assertFalse(SchemaConstants.isNameSafeForFilename(""));
+        assertFalse(SchemaConstants.isNameSafeForFilename(" "));
+        assertFalse(SchemaConstants.isNameSafeForFilename("@"));
+        assertFalse(SchemaConstants.isNameSafeForFilename("!"));
     }
 
     private static Set<String> primitiveTypes =