Add makeVariableName method, update unit tests

Author: vbabich

packages/console/src/csv/CsvInputBar.tsx

@@ -89,7 +89,7 @@ class CsvInputBar extends Component<CsvInputBarProps, CsvInputBarState> {
     // Set the table name from a file
     if (!tableNameSet && file != null && !tableName) {
       const dotIndex = file.name.lastIndexOf('.');
-      const fileTableName = DbNameValidator.legalizeTableName(
+      const fileTableName = DbNameValidator.makeVariableName(
         file.name.substring(0, dotIndex)
       );
       this.setState({

packages/utils/src/DbNameValidator.test.ts

@@ -3,10 +3,27 @@ import DbNameValidator from './DbNameValidator';
 const TABLE_PREFIX = 'table_';
 const COLUMN_PREFIX = 'column_';
 
-const VALID_TABLE_NAME = '$+@abc-123_ABC';
-const CLEANED_VALID_TABLE_NAME = 'abc_123_ABC';
-const INVALID_TABLE_NAMES = ['%^&ab-c', '-a_b c', '-'];
-const CLEANED_INVALID_TABLE_NAMES = ['ab_c', '_a_b_c', '_'];
+// $ is not a valid table name in Deephaven, but this behavior is consistent with the Java class
+const VALID_TABLE_NAMES = ['$+@abc-123_ABC', '$'];
+const VARIABLE_NAMES_FROM_VALID = ['$__abc_123_ABC', '$'];
+
+const INVALID_TABLE_NAMES = ['%^&ab-c', '-a_b c', '-', '0', '%', ''];
+const LEGALIZED_INVALID_TABLE_NAMES = [
+  'ab-c',
+  'table_-a_b_c',
+  'table_-',
+  'table_0',
+  'table_0',
+  'table_0',
+];
+const VARIABLE_NAMES_FROM_INVALID = [
+  'ab_c',
+  'table__a_b_c',
+  'table__',
+  'table_0',
+  'table_0',
+  'table_0',
+];
 
 const VALID_COL_NAME = 'abc123_ABC';
 const INVALID_COL_NAME = '@abc123_ABC-123';
@@ -17,8 +34,8 @@ const RESERVED_JAVA_WORD = 'return';
 const RESERVED_DEEPHAVEN_WORD = 'not';
 
 describe('Table name validation', () => {
-  it('Returns true on valid table names', () => {
-    expect(DbNameValidator.isValidTableName(VALID_TABLE_NAME)).toBe(true);
+  it.each(VALID_TABLE_NAMES)('Returns true on valid table name %s', name => {
+    expect(DbNameValidator.isValidTableName(name)).toBe(true);
   });
 
   it.each(INVALID_TABLE_NAMES)(
@@ -50,14 +67,15 @@ describe('Column name validation', () => {
 });
 
 describe('legalizeTableName', () => {
-  it('Sanitizes a table name even if it is valid', () => {
-    expect(DbNameValidator.legalizeTableName(VALID_TABLE_NAME)).toBe(
-      CLEANED_VALID_TABLE_NAME
-    );
+  it.each(VALID_TABLE_NAMES)('Does not change a valid table name $s', name => {
+    expect(DbNameValidator.legalizeTableName(name)).toBe(name);
   });
 
   it.each(
-    INVALID_TABLE_NAMES.map((name, i) => [name, CLEANED_INVALID_TABLE_NAMES[i]])
+    INVALID_TABLE_NAMES.map((name, i) => [
+      name,
+      LEGALIZED_INVALID_TABLE_NAMES[i],
+    ])
   )('Legalize an invalid table name %s > %s', (invalid, cleaned) => {
     expect(DbNameValidator.legalizeTableName(invalid)).toBe(cleaned);
   });
@@ -73,6 +91,25 @@ describe('legalizeTableName', () => {
   });
 });
 
+describe('makeVariableName', () => {
+  it.each(
+    VALID_TABLE_NAMES.map((name, i) => [name, VARIABLE_NAMES_FROM_VALID[i]])
+  )(
+    'Makes a variable name for a valid table name %s > %s',
+    (invalid, variableName) => {
+      expect(DbNameValidator.makeVariableName(invalid)).toBe(variableName);
+    }
+  );
+  it.each(
+    INVALID_TABLE_NAMES.map((name, i) => [name, VARIABLE_NAMES_FROM_INVALID[i]])
+  )(
+    'Makes a variable name for an invalid table name %s > %s',
+    (invalid, variableName) => {
+      expect(DbNameValidator.makeVariableName(invalid)).toBe(variableName);
+    }
+  );
+});
+
 describe('legalizeColumnName', () => {
   it('Does not change a valid column name', () => {
     expect(DbNameValidator.legalizeColumnName(VALID_COL_NAME)).toBe(

packages/utils/src/DbNameValidator.ts

@@ -69,7 +69,19 @@ const STERILE_TABLE_AND_NAMESPACE_REGEX = /[^a-zA-Z0-9_$\-+@]/g;
 const STERILE_COLUMN_AND_QUERY_REGEX = /[^A-Za-z0-9_$]/g;
 
 // From io.deephaven.db.tables.utils.DBNameValidator#TABLE_NAME_PATTERN
-const TABLE_NAME_PATTERN = /^[a-zA-Z_$][a-zA-Z0-9_$\-+@]*$/g;
+const TABLE_NAME_PATTERN = /^[a-zA-Z_$][a-zA-Z0-9_$\-+@]*$/;
+
+const STERILE_VARIABLE_NAME_REGEX = /[^a-zA-Z0-9_$]/g;
+
+function columnNameReplacer(input: string): string {
+  // Replace all dashes and spaces with underscores
+  return input.replace(/[ -]/g, '_');
+}
+
+function tableNameReplacer(input: string): string {
+  // Replace spaces with underscores
+  return input.replace(/\s/g, '_');
+}
 
 /**
  * Similar to DBNameValidator.java, this class has utilities for validating and legalizing
@@ -78,12 +90,13 @@ const TABLE_NAME_PATTERN = /^[a-zA-Z_$][a-zA-Z0-9_$\-+@]*$/g;
 class DbNameValidator {
   static legalize = (
     name: string,
+    replace: (input: string) => string,
     prefix: string,
     regex: RegExp,
     checkReserved: boolean,
     i: number
   ): string => {
-    let legalName = name.trim().replace(/[ -]/g, '_');
+    let legalName = replace(name.trim());
 
     // Add prefix to reserved names
     if (
@@ -97,28 +110,49 @@ class DbNameValidator {
     // Remove illegal characters
     legalName = legalName.replace(regex, '');
 
-    // Check if the name ended up blank
+    // Check if the name ended up blank and append the prefix.
+    // Note, io.deephaven.db.tables.utils.DBNameValidator throws an exception in this case.
     if (!legalName) {
-      legalName = prefix + i;
+      return prefix + i;
     }
 
-    // If name starts with a number, append prefix to the front
-    if (!Number.isNaN(Number(legalName.charAt(0)))) {
+    // If name starts with a number or a dash, append the prefix.
+    // Note, io.deephaven.db.tables.utils.DBNameValidator throws an exception for names starting with a dash.
+    if (/^[0-9-]/.test(legalName)) {
       legalName = prefix + legalName;
     }
 
     return legalName;
   };
 
+  /**
+   * Get a legal table name based on the passed in string.
+   * Follows the same rules as DBNameValidator.java except that here
+   * we prepend a prefix in cases where the Java class throws an exception.
+   * @param name The name to legalize
+   * @returns Legalized table name
+   */
   static legalizeTableName = (name: string): string =>
     DbNameValidator.legalize(
       name,
+      tableNameReplacer,
       TABLE_PREFIX,
       STERILE_TABLE_AND_NAMESPACE_REGEX,
       false,
       0
     );
 
+  /**
+   * Get a variable name based on the passed in string.
+   * @param name The name to get the variable name for
+   * @returns Variable name
+   */
+  static makeVariableName = (name: string): string =>
+    DbNameValidator.legalizeTableName(name).replace(
+      STERILE_VARIABLE_NAME_REGEX,
+      '_'
+    );
+
   static legalizeColumnNames = (headers: string[]): string[] => {
     const legalHeaders: string[] = [];
     headers.forEach((header, i) => {
@@ -138,6 +172,7 @@ class DbNameValidator {
     // Replace all dashes and spaces with underscores
     DbNameValidator.legalize(
       header,
+      columnNameReplacer,
       COLUMN_PREFIX,
       STERILE_COLUMN_AND_QUERY_REGEX,
       true,