From 4315e8577159f0af29dc6eba177bf5ba10932a71 Mon Sep 17 00:00:00 2001 From: nielm Date: Tue, 30 Jan 2024 14:30:45 +0100 Subject: [PATCH] Add Errorprone and fix errors --- pom.xml | 15 +++ .../spannerddl/diff/ASTTreeUtils.java | 5 +- .../solutions/spannerddl/diff/DdlDiff.java | 99 +++++++++---------- .../spannerddl/diff/DdlDiffOptions.java | 77 ++++++++------- .../parser/ASTalter_database_statement.java | 5 + .../parser/ASTalter_table_statement.java | 5 + .../parser/ASTchange_stream_for_clause.java | 5 + .../parser/ASTcheck_constraint.java | 8 +- .../spannerddl/parser/ASTcolumn_def.java | 6 +- .../parser/ASTcolumn_default_clause.java | 5 + .../spannerddl/parser/ASTcolumn_length.java | 5 + .../spannerddl/parser/ASTcolumn_type.java | 5 +- .../spannerddl/parser/ASTcolumns.java | 1 - .../ASTcreate_change_stream_statement.java | 9 +- .../parser/ASTcreate_index_statement.java | 11 ++- .../parser/ASTcreate_table_statement.java | 16 +-- .../spannerddl/parser/ASTddl_statement.java | 5 + .../spannerddl/parser/ASTexpression.java | 5 + .../spannerddl/parser/ASTforeign_key.java | 6 ++ .../parser/ASTgeneration_clause.java | 1 + .../solutions/spannerddl/parser/ASTkey.java | 1 - .../spannerddl/parser/ASTkey_part.java | 6 +- .../spannerddl/parser/ASTlength.java | 6 +- .../spannerddl/parser/ASTon_delete.java | 5 + .../spannerddl/parser/ASToptions_clause.java | 1 - .../parser/ASTreferential_action.java | 5 + .../parser/ASTrow_deletion_policy_clause.java | 5 + .../parser/ASTstored_column_list.java | 1 - .../spannerddl/diff/DdlDiffFromFilesTest.java | 28 +++--- .../parser/DDLParserFromFileTest.java | 6 +- .../testUtils/ReadTestDatafile.java | 12 +-- 31 files changed, 230 insertions(+), 140 deletions(-) diff --git a/pom.xml b/pom.xml index 3e95b08..86e9b4c 100644 --- a/pom.xml +++ b/pom.xml @@ -38,6 +38,8 @@ 3.1.1 3.5.0 1.10.4 + 2.24.1 + 3.3.1 @@ -108,14 +110,27 @@ + org.apache.maven.plugins maven-compiler-plugin + true + true + + -Xlint:-options + -XDcompilePolicy=simple + -Xplugin:ErrorProne -XepExcludedPaths:${project.build.directory}/generated-sources/.* -XepDisableWarningsInGeneratedCode + com.google.auto.value auto-value ${auto-value.version} + + com.google.errorprone + error_prone_core + ${error-prone.version} + diff --git a/src/main/java/com/google/cloud/solutions/spannerddl/diff/ASTTreeUtils.java b/src/main/java/com/google/cloud/solutions/spannerddl/diff/ASTTreeUtils.java index fca12a1..b4d6b30 100644 --- a/src/main/java/com/google/cloud/solutions/spannerddl/diff/ASTTreeUtils.java +++ b/src/main/java/com/google/cloud/solutions/spannerddl/diff/ASTTreeUtils.java @@ -22,6 +22,7 @@ import com.google.cloud.solutions.spannerddl.parser.Token; import java.util.Arrays; import java.util.List; +import java.util.Locale; import java.util.Set; import java.util.stream.Collectors; import java.util.stream.StreamSupport; @@ -88,7 +89,7 @@ public static String tokensToString( Token t = firstToken; while (t != lastToken) { String tok = t.toString(); - sb.append(isReservedWord(tok) && upperCaseReserved ? tok.toUpperCase() : tok); + sb.append(isReservedWord(tok) && upperCaseReserved ? tok.toUpperCase(Locale.ROOT) : tok); if (t.next != null && !t.next.toString().equals(",") @@ -100,7 +101,7 @@ public static String tokensToString( } // append last token String tok = t.toString(); - sb.append(isReservedWord(tok) && upperCaseReserved ? tok.toUpperCase() : tok); + sb.append(isReservedWord(tok) && upperCaseReserved ? tok.toUpperCase(Locale.ROOT) : tok); return sb.toString(); } diff --git a/src/main/java/com/google/cloud/solutions/spannerddl/diff/DdlDiff.java b/src/main/java/com/google/cloud/solutions/spannerddl/diff/DdlDiff.java index 3c2d907..c22dee5 100644 --- a/src/main/java/com/google/cloud/solutions/spannerddl/diff/DdlDiff.java +++ b/src/main/java/com/google/cloud/solutions/spannerddl/diff/DdlDiff.java @@ -39,6 +39,7 @@ import com.google.cloud.solutions.spannerddl.parser.SimpleNode; import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Joiner; +import com.google.common.base.Splitter; import com.google.common.base.Strings; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; @@ -341,10 +342,10 @@ static List generateAlterTableStatements( } if (left.getInterleaveClause().isPresent() - && !(left.getInterleaveClause() + && !left.getInterleaveClause() .get() .getParentTableName() - .equals(right.getInterleaveClause().get().getParentTableName()))) { + .equals(right.getInterleaveClause().get().getParentTableName())) { throw new DdlDiffException( "Cannot change interleaved parent of table " + left.getTableName()); } @@ -356,10 +357,10 @@ static List generateAlterTableStatements( // On delete changed if (left.getInterleaveClause().isPresent() - && !(left.getInterleaveClause() + && !left.getInterleaveClause() .get() .getOnDelete() - .equals(right.getInterleaveClause().get().getOnDelete()))) { + .equals(right.getInterleaveClause().get().getOnDelete())) { alterStatements.add( "ALTER TABLE " + left.getTableName() @@ -391,9 +392,7 @@ static List generateAlterTableStatements( } private static void addColumnDiffs( - String tableName, - ArrayList alterStatements, - ValueDifference columnDiff) + String tableName, List alterStatements, ValueDifference columnDiff) throws DdlDiffException { // check for compatible type changes. @@ -599,8 +598,8 @@ private static String getDatabaseNameFromAlterDatabase(List st @VisibleForTesting static List parseDDL(String original) throws DdlDiffException { // Remove "--" comments and split by ";" - String[] statements = original.replaceAll("--.*(\n|$)", "").split(";"); - ArrayList ddlStatements = new ArrayList<>(statements.length); + List statements = Splitter.on(';').splitToList(original.replaceAll("--.*(\n|$)", "")); + ArrayList ddlStatements = new ArrayList<>(statements.size()); for (String statement : statements) { statement = statement.trim(); @@ -612,40 +611,38 @@ static List parseDDL(String original) throws DdlDiffException int statementType = ddlStatement.jjtGetChild(0).getId(); switch (statementType) { - case (DdlParserTreeConstants.JJTALTER_TABLE_STATEMENT): - { - ASTalter_table_statement alterTableStatement = - (ASTalter_table_statement) ddlStatement.jjtGetChild(0); - // child 0 = table name - // child 1 = alter statement. Only ASTforeign_key is supported - if (!(alterTableStatement.jjtGetChild(1) instanceof ASTforeign_key) - && !(alterTableStatement.jjtGetChild(1) instanceof ASTcheck_constraint) - && !(alterTableStatement.jjtGetChild(1) instanceof ASTadd_row_deletion_policy)) { - throw new IllegalArgumentException( - "Unsupported statement:\n" - + statement - + "\n" - + "Can only create diffs from 'CREATE TABLE, CREATE INDEX and 'ALTER TABLE" - + " table_name ADD [constraint|row deletion policy]' DDL statements"); - } - if (alterTableStatement.jjtGetChild(1) instanceof ASTforeign_key - && ((ASTforeign_key) alterTableStatement.jjtGetChild(1)) - .getName() - .equals(ASTcreate_table_statement.ANONYMOUS_NAME)) { - throw new IllegalArgumentException( - "Unsupported statement:\n" - + statement - + "\nCan not create diffs when anonymous constraints are used."); - } - if (alterTableStatement.jjtGetChild(1) instanceof ASTcheck_constraint - && ((ASTcheck_constraint) alterTableStatement.jjtGetChild(1)) - .getName() - .equals(ASTcreate_table_statement.ANONYMOUS_NAME)) { - throw new IllegalArgumentException( - "Unsupported statement:\n" - + statement - + "\nCan not create diffs when anonymous constraints are used."); - } + case DdlParserTreeConstants.JJTALTER_TABLE_STATEMENT: + ASTalter_table_statement alterTableStatement = + (ASTalter_table_statement) ddlStatement.jjtGetChild(0); + // child 0 = table name + // child 1 = alter statement. Only ASTforeign_key is supported + if (!(alterTableStatement.jjtGetChild(1) instanceof ASTforeign_key) + && !(alterTableStatement.jjtGetChild(1) instanceof ASTcheck_constraint) + && !(alterTableStatement.jjtGetChild(1) instanceof ASTadd_row_deletion_policy)) { + throw new IllegalArgumentException( + "Unsupported statement:\n" + + statement + + "\n" + + "Can only create diffs from 'CREATE TABLE, CREATE INDEX and 'ALTER TABLE" + + " table_name ADD [constraint|row deletion policy]' DDL statements"); + } + if (alterTableStatement.jjtGetChild(1) instanceof ASTforeign_key + && ((ASTforeign_key) alterTableStatement.jjtGetChild(1)) + .getName() + .equals(ASTcreate_table_statement.ANONYMOUS_NAME)) { + throw new IllegalArgumentException( + "Unsupported statement:\n" + + statement + + "\nCan not create diffs when anonymous constraints are used."); + } + if (alterTableStatement.jjtGetChild(1) instanceof ASTcheck_constraint + && ((ASTcheck_constraint) alterTableStatement.jjtGetChild(1)) + .getName() + .equals(ASTcreate_table_statement.ANONYMOUS_NAME)) { + throw new IllegalArgumentException( + "Unsupported statement:\n" + + statement + + "\nCan not create diffs when anonymous constraints are used."); } break; case DdlParserTreeConstants.JJTCREATE_TABLE_STATEMENT: @@ -703,8 +700,10 @@ public static void main(String[] args) { System.exit(0); } catch (IOException e) { System.err.println("Cannot read DDL file: " + e); + System.exit(1); } catch (DdlDiffException e) { - e.printStackTrace(); + System.err.println("Failed to generate a diff: " + e.getMessage()); + System.exit(1); } } } @@ -835,15 +834,15 @@ static DatbaseDefinition create(List statements) { ImmutableMap.copyOf(alterDatabaseOptions)); } - abstract Map tablesInCreationOrder(); + abstract ImmutableMap tablesInCreationOrder(); - abstract Map indexes(); + abstract ImmutableMap indexes(); - abstract Map constraints(); + abstract ImmutableMap constraints(); - abstract Map ttls(); + abstract ImmutableMap ttls(); - abstract Map changeStreams(); + abstract ImmutableMap changeStreams(); - abstract Map alterDatabaseOptions(); + abstract ImmutableMap alterDatabaseOptions(); } diff --git a/src/main/java/com/google/cloud/solutions/spannerddl/diff/DdlDiffOptions.java b/src/main/java/com/google/cloud/solutions/spannerddl/diff/DdlDiffOptions.java index 0f8b399..52969b8 100644 --- a/src/main/java/com/google/cloud/solutions/spannerddl/diff/DdlDiffOptions.java +++ b/src/main/java/com/google/cloud/solutions/spannerddl/diff/DdlDiffOptions.java @@ -15,14 +15,17 @@ */ package com.google.cloud.solutions.spannerddl.diff; +import static java.nio.charset.StandardCharsets.UTF_8; + import com.google.auto.value.AutoValue; import com.google.common.annotations.VisibleForTesting; import com.google.common.collect.ImmutableMap; +import java.io.BufferedWriter; import java.io.File; +import java.io.OutputStreamWriter; import java.io.PrintWriter; import java.nio.file.InvalidPathException; import java.nio.file.Path; -import java.util.Map; import org.apache.commons.cli.CommandLine; import org.apache.commons.cli.DefaultParser; import org.apache.commons.cli.HelpFormatter; @@ -40,7 +43,7 @@ public abstract class DdlDiffOptions { public abstract Path outputDdlPath(); - public abstract Map args(); + public abstract ImmutableMap args(); @VisibleForTesting static Options buildOptions() { @@ -93,40 +96,40 @@ static Options buildOptions() { } static void printHelpAndExit(int exitStatus) { - try (PrintWriter pw = new PrintWriter(System.err)) { - new HelpFormatter() - .printHelp( - pw, - 132, - "DdlDiff", - "Compares original and new DDL files and creates a DDL file with DROP, CREATE and" - + " ALTER statements which convert the original Schema to the new Schema.\n" - + "\n" - + "Incompatible table changes (table hierarchy changes. column type changes) are" - + " not supported and will cause this tool to fail.\n" - + "\n" - + "To prevent accidental data loss, and to make it easier to apply DDL changes," - + " DROP statements are not generated for removed tables, columns, indexes and" - + " change streams. This can be overridden using the " - + DdlDiff.ALLOW_DROP_STATEMENTS_OPT - + " command line argument.\n" - + "\n" - + "By default, changes to indexes will also cause a failure. The " - + DdlDiff.ALLOW_RECREATE_INDEXES_OPT - + " command line option enables index changes by" - + " generating statements to drop and recreate the index.\n" - + "\n" - + "By default, changes to foreign key constraints will also cause a failure. The " - + DdlDiff.ALLOW_RECREATE_CONSTRAINTS_OPT - + " command line option enables foreign key changes by" - + " generating statements to drop and recreate the constraint.\n" - + "\n", - buildOptions(), - 1, - 4, - "", - true); - } + PrintWriter pw = new PrintWriter(new BufferedWriter(new OutputStreamWriter(System.err, UTF_8))); + new HelpFormatter() + .printHelp( + pw, + 132, + "DdlDiff", + "Compares original and new DDL files and creates a DDL file with DROP, CREATE and" + + " ALTER statements which convert the original Schema to the new Schema.\n" + + "\n" + + "Incompatible table changes (table hierarchy changes. column type changes) are" + + " not supported and will cause this tool to fail.\n" + + "\n" + + "To prevent accidental data loss, and to make it easier to apply DDL changes," + + " DROP statements are not generated for removed tables, columns, indexes and" + + " change streams. This can be overridden using the " + + DdlDiff.ALLOW_DROP_STATEMENTS_OPT + + " command line argument.\n" + + "\n" + + "By default, changes to indexes will also cause a failure. The " + + DdlDiff.ALLOW_RECREATE_INDEXES_OPT + + " command line option enables index changes by" + + " generating statements to drop and recreate the index.\n" + + "\n" + + "By default, changes to foreign key constraints will also cause a failure. The " + + DdlDiff.ALLOW_RECREATE_CONSTRAINTS_OPT + + " command line option enables foreign key changes by" + + " generating statements to drop and recreate the constraint.\n" + + "\n", + buildOptions(), + 1, + 4, + "", + true); + pw.flush(); System.exit(exitStatus); } @@ -143,7 +146,7 @@ public static DdlDiffOptions parseCommandLine(String[] args) { Path outputDdlPath = new File(commandLine.getOptionValue(DdlDiff.OUTPUT_DDL_FILE_OPT)).toPath(); - Map argsMap = + ImmutableMap argsMap = ImmutableMap.of( DdlDiff.ALLOW_RECREATE_INDEXES_OPT, commandLine.hasOption(DdlDiff.ALLOW_RECREATE_INDEXES_OPT), diff --git a/src/main/java/com/google/cloud/solutions/spannerddl/parser/ASTalter_database_statement.java b/src/main/java/com/google/cloud/solutions/spannerddl/parser/ASTalter_database_statement.java index 62299e5..c583efc 100644 --- a/src/main/java/com/google/cloud/solutions/spannerddl/parser/ASTalter_database_statement.java +++ b/src/main/java/com/google/cloud/solutions/spannerddl/parser/ASTalter_database_statement.java @@ -43,4 +43,9 @@ public String toString() { public boolean equals(Object other) { return other instanceof ASTalter_table_statement && this.toString().equals(other.toString()); } + + @Override + public int hashCode() { + return toString().hashCode(); + } } diff --git a/src/main/java/com/google/cloud/solutions/spannerddl/parser/ASTalter_table_statement.java b/src/main/java/com/google/cloud/solutions/spannerddl/parser/ASTalter_table_statement.java index 30510d3..aafa3b1 100644 --- a/src/main/java/com/google/cloud/solutions/spannerddl/parser/ASTalter_table_statement.java +++ b/src/main/java/com/google/cloud/solutions/spannerddl/parser/ASTalter_table_statement.java @@ -59,4 +59,9 @@ public String toString() { } return ret.toString(); } + + @Override + public int hashCode() { + return toString().hashCode(); + } } diff --git a/src/main/java/com/google/cloud/solutions/spannerddl/parser/ASTchange_stream_for_clause.java b/src/main/java/com/google/cloud/solutions/spannerddl/parser/ASTchange_stream_for_clause.java index 664aba3..55128cf 100644 --- a/src/main/java/com/google/cloud/solutions/spannerddl/parser/ASTchange_stream_for_clause.java +++ b/src/main/java/com/google/cloud/solutions/spannerddl/parser/ASTchange_stream_for_clause.java @@ -35,4 +35,9 @@ public String toString() { } return "FOR ALL"; } + + @Override + public int hashCode() { + return toString().hashCode(); + } } diff --git a/src/main/java/com/google/cloud/solutions/spannerddl/parser/ASTcheck_constraint.java b/src/main/java/com/google/cloud/solutions/spannerddl/parser/ASTcheck_constraint.java index 40d3310..7897f05 100644 --- a/src/main/java/com/google/cloud/solutions/spannerddl/parser/ASTcheck_constraint.java +++ b/src/main/java/com/google/cloud/solutions/spannerddl/parser/ASTcheck_constraint.java @@ -41,9 +41,10 @@ public String getExpression() { if (children[0] instanceof ASTconstraint_name) { child++; } - return (ASTTreeUtils.tokensToString((ASTcheck_constraint_expression) children[child], false)); + return ASTTreeUtils.tokensToString((ASTcheck_constraint_expression) children[child], false); } + @Override public String toString() { return "CONSTRAINT " + getName() + " CHECK (" + getExpression() + ")"; } @@ -55,4 +56,9 @@ public boolean equals(Object other) { } return false; } + + @Override + public int hashCode() { + return toString().hashCode(); + } } diff --git a/src/main/java/com/google/cloud/solutions/spannerddl/parser/ASTcolumn_def.java b/src/main/java/com/google/cloud/solutions/spannerddl/parser/ASTcolumn_def.java index b2e0fff..27f1fdf 100644 --- a/src/main/java/com/google/cloud/solutions/spannerddl/parser/ASTcolumn_def.java +++ b/src/main/java/com/google/cloud/solutions/spannerddl/parser/ASTcolumn_def.java @@ -16,7 +16,6 @@ package com.google.cloud.solutions.spannerddl.parser; -/** Abstract Syntax Tree parser object for column definitions */ import com.google.cloud.solutions.spannerddl.diff.ASTTreeUtils; import com.google.common.base.Joiner; import javax.annotation.Nullable; @@ -129,4 +128,9 @@ public boolean equals(Object other) { } return false; } + + @Override + public int hashCode() { + return toString().hashCode(); + } } diff --git a/src/main/java/com/google/cloud/solutions/spannerddl/parser/ASTcolumn_default_clause.java b/src/main/java/com/google/cloud/solutions/spannerddl/parser/ASTcolumn_default_clause.java index 73da171..52a9316 100644 --- a/src/main/java/com/google/cloud/solutions/spannerddl/parser/ASTcolumn_default_clause.java +++ b/src/main/java/com/google/cloud/solutions/spannerddl/parser/ASTcolumn_default_clause.java @@ -37,4 +37,9 @@ public String toString() { + ASTTreeUtils.tokensToString((ASTcolumn_default_expression) jjtGetChild(0)) + ")"; } + + @Override + public int hashCode() { + return toString().hashCode(); + } } diff --git a/src/main/java/com/google/cloud/solutions/spannerddl/parser/ASTcolumn_length.java b/src/main/java/com/google/cloud/solutions/spannerddl/parser/ASTcolumn_length.java index 05180cd..8faa905 100644 --- a/src/main/java/com/google/cloud/solutions/spannerddl/parser/ASTcolumn_length.java +++ b/src/main/java/com/google/cloud/solutions/spannerddl/parser/ASTcolumn_length.java @@ -33,4 +33,9 @@ public ASTcolumn_length(DdlParser p, int id) { public String toString() { return ASTTreeUtils.tokensToString(this); } + + @Override + public int hashCode() { + return toString().hashCode(); + } } diff --git a/src/main/java/com/google/cloud/solutions/spannerddl/parser/ASTcolumn_type.java b/src/main/java/com/google/cloud/solutions/spannerddl/parser/ASTcolumn_type.java index ce1b1b1..8002286 100644 --- a/src/main/java/com/google/cloud/solutions/spannerddl/parser/ASTcolumn_type.java +++ b/src/main/java/com/google/cloud/solutions/spannerddl/parser/ASTcolumn_type.java @@ -17,6 +17,7 @@ package com.google.cloud.solutions.spannerddl.parser; import com.google.cloud.solutions.spannerddl.diff.ASTTreeUtils; +import java.util.Locale; /** Abstract Syntax Tree parser object for "column_type" token */ public class ASTcolumn_type extends SimpleNode { @@ -61,7 +62,7 @@ public String toString() { case "ARRAY": return "ARRAY<" + ((ASTcolumn_type) children[0]) + ">"; case "PG": // PG.pgtype - return ASTTreeUtils.tokensToString(this).toUpperCase(); + return ASTTreeUtils.tokensToString(this).toUpperCase(Locale.ROOT); case "STRUCT": if (jjtGetNumChildren() > 0) { return ASTTreeUtils.tokensToString(this); @@ -74,6 +75,6 @@ public String toString() { } public String getTypeName() { - return jjtGetFirstToken().toString().toUpperCase(); + return jjtGetFirstToken().toString().toUpperCase(Locale.ROOT); } } diff --git a/src/main/java/com/google/cloud/solutions/spannerddl/parser/ASTcolumns.java b/src/main/java/com/google/cloud/solutions/spannerddl/parser/ASTcolumns.java index f94f2d7..322cdff 100644 --- a/src/main/java/com/google/cloud/solutions/spannerddl/parser/ASTcolumns.java +++ b/src/main/java/com/google/cloud/solutions/spannerddl/parser/ASTcolumns.java @@ -16,7 +16,6 @@ package com.google.cloud.solutions.spannerddl.parser; -/** Abstract Syntax Tree parser object for "columns" token */ import com.google.cloud.solutions.spannerddl.diff.ASTTreeUtils; import com.google.common.base.Joiner; diff --git a/src/main/java/com/google/cloud/solutions/spannerddl/parser/ASTcreate_change_stream_statement.java b/src/main/java/com/google/cloud/solutions/spannerddl/parser/ASTcreate_change_stream_statement.java index 1b026d8..66ccafa 100644 --- a/src/main/java/com/google/cloud/solutions/spannerddl/parser/ASTcreate_change_stream_statement.java +++ b/src/main/java/com/google/cloud/solutions/spannerddl/parser/ASTcreate_change_stream_statement.java @@ -18,10 +18,6 @@ import com.google.cloud.solutions.spannerddl.diff.ASTTreeUtils; import com.google.common.base.Joiner; -/** - * @link - * https://cloud.google.com/spanner/docs/reference/standard-sql/data-definition-language#create-change-stream - */ public class ASTcreate_change_stream_statement extends SimpleNode { public ASTcreate_change_stream_statement(int id) { super(id); @@ -55,4 +51,9 @@ public boolean equals(Object other) { return (other instanceof ASTcreate_change_stream_statement) && this.toString().equals(other.toString()); } + + @Override + public int hashCode() { + return toString().hashCode(); + } } diff --git a/src/main/java/com/google/cloud/solutions/spannerddl/parser/ASTcreate_index_statement.java b/src/main/java/com/google/cloud/solutions/spannerddl/parser/ASTcreate_index_statement.java index 4a7b0c1..201ba0a 100644 --- a/src/main/java/com/google/cloud/solutions/spannerddl/parser/ASTcreate_index_statement.java +++ b/src/main/java/com/google/cloud/solutions/spannerddl/parser/ASTcreate_index_statement.java @@ -42,11 +42,7 @@ public String toString() { return toStringOptionalExistClause(true); } - /** - * Create string version, optionally including the IF NOT EXISTS clause - * - * @param includeExists - */ + /** Create string version, optionally including the IF NOT EXISTS clause */ public String toStringOptionalExistClause(boolean includeExists) { validateChildren(); ASTindex_interleave_clause interleave = @@ -101,4 +97,9 @@ public boolean equals(Object other) { } return false; } + + @Override + public int hashCode() { + return toStringOptionalExistClause(false).hashCode(); + } } diff --git a/src/main/java/com/google/cloud/solutions/spannerddl/parser/ASTcreate_table_statement.java b/src/main/java/com/google/cloud/solutions/spannerddl/parser/ASTcreate_table_statement.java index f551594..a8c0747 100644 --- a/src/main/java/com/google/cloud/solutions/spannerddl/parser/ASTcreate_table_statement.java +++ b/src/main/java/com/google/cloud/solutions/spannerddl/parser/ASTcreate_table_statement.java @@ -23,6 +23,7 @@ import java.util.ArrayList; import java.util.LinkedHashMap; import java.util.List; +import java.util.Map; import java.util.Objects; import java.util.Optional; import java.util.stream.Collectors; @@ -45,7 +46,7 @@ public String getTableName() { return ASTTreeUtils.tokensToString(ASTTreeUtils.getChildByType(children, ASTname.class)); } - public LinkedHashMap getColumns() { + public Map getColumns() { LinkedHashMap columns = new LinkedHashMap<>(); for (Node child : children) { if (child instanceof ASTcolumn_def) { @@ -56,7 +57,7 @@ public LinkedHashMap getColumns() { return columns; } - public LinkedHashMap getConstraints() { + public Map getConstraints() { LinkedHashMap constraints = new LinkedHashMap<>(); for (Node child : children) { if (child instanceof ASTforeign_key) { @@ -94,11 +95,7 @@ public String toString() { return toStringOptionalExistClause(true); } - /** - * Create string version, optionally including the IF NOT EXISTS clause - * - * @param includeExists - */ + /** Create string version, optionally including the IF NOT EXISTS clause */ public String toStringOptionalExistClause(boolean includeExists) { verifyTableElements(); @@ -156,4 +153,9 @@ public boolean equals(Object other) { } return false; } + + @Override + public int hashCode() { + return toStringOptionalExistClause(false).hashCode(); + } } diff --git a/src/main/java/com/google/cloud/solutions/spannerddl/parser/ASTddl_statement.java b/src/main/java/com/google/cloud/solutions/spannerddl/parser/ASTddl_statement.java index ff6e63a..fd07dc3 100644 --- a/src/main/java/com/google/cloud/solutions/spannerddl/parser/ASTddl_statement.java +++ b/src/main/java/com/google/cloud/solutions/spannerddl/parser/ASTddl_statement.java @@ -36,4 +36,9 @@ public String toString() { public boolean equals(Object obj) { return obj instanceof ASTddl_statement && this.toString().equals(obj.toString()); } + + @Override + public int hashCode() { + return toString().hashCode(); + } } diff --git a/src/main/java/com/google/cloud/solutions/spannerddl/parser/ASTexpression.java b/src/main/java/com/google/cloud/solutions/spannerddl/parser/ASTexpression.java index bcfa742..0226cea 100644 --- a/src/main/java/com/google/cloud/solutions/spannerddl/parser/ASTexpression.java +++ b/src/main/java/com/google/cloud/solutions/spannerddl/parser/ASTexpression.java @@ -31,4 +31,9 @@ public ASTexpression(DdlParser p, int id) { public String toString() { return ASTTreeUtils.tokensToString(this, false); } + + @Override + public int hashCode() { + return toString().hashCode(); + } } diff --git a/src/main/java/com/google/cloud/solutions/spannerddl/parser/ASTforeign_key.java b/src/main/java/com/google/cloud/solutions/spannerddl/parser/ASTforeign_key.java index ad04489..c75e022 100644 --- a/src/main/java/com/google/cloud/solutions/spannerddl/parser/ASTforeign_key.java +++ b/src/main/java/com/google/cloud/solutions/spannerddl/parser/ASTforeign_key.java @@ -79,6 +79,7 @@ public String getDeleteOption() { } } + @Override public String toString() { return "CONSTRAINT " + getName() @@ -99,4 +100,9 @@ public boolean equals(Object other) { } return false; } + + @Override + public int hashCode() { + return toString().hashCode(); + } } diff --git a/src/main/java/com/google/cloud/solutions/spannerddl/parser/ASTgeneration_clause.java b/src/main/java/com/google/cloud/solutions/spannerddl/parser/ASTgeneration_clause.java index e5feffc..a35d7e8 100644 --- a/src/main/java/com/google/cloud/solutions/spannerddl/parser/ASTgeneration_clause.java +++ b/src/main/java/com/google/cloud/solutions/spannerddl/parser/ASTgeneration_clause.java @@ -25,6 +25,7 @@ public ASTgeneration_clause(DdlParser p, int id) { super(p, id); } + @Override public String toString() { final ASTexpression exp = (ASTexpression) children[0]; return "AS ( " + exp.toString() + " ) STORED"; diff --git a/src/main/java/com/google/cloud/solutions/spannerddl/parser/ASTkey.java b/src/main/java/com/google/cloud/solutions/spannerddl/parser/ASTkey.java index 40cb177..b2698b9 100644 --- a/src/main/java/com/google/cloud/solutions/spannerddl/parser/ASTkey.java +++ b/src/main/java/com/google/cloud/solutions/spannerddl/parser/ASTkey.java @@ -16,7 +16,6 @@ package com.google.cloud.solutions.spannerddl.parser; -/** Abstract Syntax Tree parser object for "key" token */ import com.google.cloud.solutions.spannerddl.diff.ASTTreeUtils; import com.google.common.base.Joiner; diff --git a/src/main/java/com/google/cloud/solutions/spannerddl/parser/ASTkey_part.java b/src/main/java/com/google/cloud/solutions/spannerddl/parser/ASTkey_part.java index c811caa..4dd1458 100644 --- a/src/main/java/com/google/cloud/solutions/spannerddl/parser/ASTkey_part.java +++ b/src/main/java/com/google/cloud/solutions/spannerddl/parser/ASTkey_part.java @@ -16,6 +16,8 @@ package com.google.cloud.solutions.spannerddl.parser; +import java.util.Locale; + /** Abstract Syntax Tree parser object for "key_part" token */ public class ASTkey_part extends SimpleNode { @@ -37,7 +39,9 @@ public String toString() { return ((ASTpath) children[0]).toString() + " ASC"; // key name without direction ; } else { // key name and ASC/DESC - return ((ASTpath) children[0]).toString() + " " + children[1].toString().toUpperCase(); + return ((ASTpath) children[0]).toString() + + " " + + children[1].toString().toUpperCase(Locale.ROOT); } } } diff --git a/src/main/java/com/google/cloud/solutions/spannerddl/parser/ASTlength.java b/src/main/java/com/google/cloud/solutions/spannerddl/parser/ASTlength.java index 55c32e9..ec61481 100644 --- a/src/main/java/com/google/cloud/solutions/spannerddl/parser/ASTlength.java +++ b/src/main/java/com/google/cloud/solutions/spannerddl/parser/ASTlength.java @@ -16,6 +16,8 @@ package com.google.cloud.solutions.spannerddl.parser; +import java.util.Locale; + /** Abstract Syntax Tree parser object for "length" token */ public class ASTlength extends SimpleNode { @@ -31,10 +33,10 @@ public ASTlength(DdlParser p, int id) { public String toString() { if (children == null) { // MAX - return jjtGetFirstToken().toString().toUpperCase(); + return jjtGetFirstToken().toString().toUpperCase(Locale.ROOT); } else { // Length - return children[0].toString().toUpperCase(); + return children[0].toString().toUpperCase(Locale.ROOT); } } } diff --git a/src/main/java/com/google/cloud/solutions/spannerddl/parser/ASTon_delete.java b/src/main/java/com/google/cloud/solutions/spannerddl/parser/ASTon_delete.java index 16c03a0..8477444 100644 --- a/src/main/java/com/google/cloud/solutions/spannerddl/parser/ASTon_delete.java +++ b/src/main/java/com/google/cloud/solutions/spannerddl/parser/ASTon_delete.java @@ -37,4 +37,9 @@ public String toString() { public boolean equals(Object other) { return (other instanceof ASTon_delete && this.toString().equals(other.toString())); } + + @Override + public int hashCode() { + return toString().hashCode(); + } } diff --git a/src/main/java/com/google/cloud/solutions/spannerddl/parser/ASToptions_clause.java b/src/main/java/com/google/cloud/solutions/spannerddl/parser/ASToptions_clause.java index e396131..8086490 100644 --- a/src/main/java/com/google/cloud/solutions/spannerddl/parser/ASToptions_clause.java +++ b/src/main/java/com/google/cloud/solutions/spannerddl/parser/ASToptions_clause.java @@ -16,7 +16,6 @@ package com.google.cloud.solutions.spannerddl.parser; -/** Abstract Syntax Tree parser object for "options_clause" token */ import com.google.cloud.solutions.spannerddl.diff.ASTTreeUtils; import com.google.common.base.Joiner; import java.util.Map; diff --git a/src/main/java/com/google/cloud/solutions/spannerddl/parser/ASTreferential_action.java b/src/main/java/com/google/cloud/solutions/spannerddl/parser/ASTreferential_action.java index f1f9837..4d1730d 100644 --- a/src/main/java/com/google/cloud/solutions/spannerddl/parser/ASTreferential_action.java +++ b/src/main/java/com/google/cloud/solutions/spannerddl/parser/ASTreferential_action.java @@ -35,4 +35,9 @@ public String toString() { public boolean equals(Object other) { return (other instanceof ASTreferential_action && this.toString().equals(other.toString())); } + + @Override + public int hashCode() { + return toString().hashCode(); + } } diff --git a/src/main/java/com/google/cloud/solutions/spannerddl/parser/ASTrow_deletion_policy_clause.java b/src/main/java/com/google/cloud/solutions/spannerddl/parser/ASTrow_deletion_policy_clause.java index ff4e3d9..a0b5446 100644 --- a/src/main/java/com/google/cloud/solutions/spannerddl/parser/ASTrow_deletion_policy_clause.java +++ b/src/main/java/com/google/cloud/solutions/spannerddl/parser/ASTrow_deletion_policy_clause.java @@ -43,4 +43,9 @@ public boolean equals(Object other) { return (other instanceof ASTrow_deletion_policy_clause && this.toString().equals(other.toString())); } + + @Override + public int hashCode() { + return toString().hashCode(); + } } diff --git a/src/main/java/com/google/cloud/solutions/spannerddl/parser/ASTstored_column_list.java b/src/main/java/com/google/cloud/solutions/spannerddl/parser/ASTstored_column_list.java index 05bdb60..661cfb4 100644 --- a/src/main/java/com/google/cloud/solutions/spannerddl/parser/ASTstored_column_list.java +++ b/src/main/java/com/google/cloud/solutions/spannerddl/parser/ASTstored_column_list.java @@ -16,7 +16,6 @@ package com.google.cloud.solutions.spannerddl.parser; -/** Abstract Syntax Tree parser object for "stored_column_list" token */ import com.google.cloud.solutions.spannerddl.diff.ASTTreeUtils; import com.google.common.base.Joiner; diff --git a/src/test/java/com/google/cloud/solutions/spannerddl/diff/DdlDiffFromFilesTest.java b/src/test/java/com/google/cloud/solutions/spannerddl/diff/DdlDiffFromFilesTest.java index 0f28870..be4cfae 100644 --- a/src/test/java/com/google/cloud/solutions/spannerddl/diff/DdlDiffFromFilesTest.java +++ b/src/test/java/com/google/cloud/solutions/spannerddl/diff/DdlDiffFromFilesTest.java @@ -7,13 +7,13 @@ import static org.junit.Assert.fail; import com.google.cloud.solutions.spannerddl.testUtils.ReadTestDatafile; +import com.google.common.base.Splitter; import com.google.common.collect.ImmutableMap; +import com.google.common.collect.Iterables; import java.io.IOException; import java.util.Arrays; import java.util.Collections; import java.util.Iterator; -import java.util.LinkedHashMap; -import java.util.LinkedList; import java.util.List; import java.util.Map; import java.util.regex.Pattern; @@ -30,11 +30,10 @@ public void compareDddTextFiles() throws IOException { // Uses 3 files: 2 containing DDL segments to run diffs on, 1 with the expected results // if allowRecreateIndexes and allowDropStatements are set. - LinkedHashMap originalSegments = + Map originalSegments = ReadTestDatafile.readDdlSegmentsFromFile("originalDdl.txt"); - LinkedHashMap newSegments = - ReadTestDatafile.readDdlSegmentsFromFile("newDdl.txt"); - LinkedHashMap expectedOutputs = + Map newSegments = ReadTestDatafile.readDdlSegmentsFromFile("newDdl.txt"); + Map expectedOutputs = ReadTestDatafile.readDdlSegmentsFromFile("expectedDdlDiff.txt"); Iterator> originalSegmentIt = originalSegments.entrySet().iterator(); @@ -50,12 +49,13 @@ public void compareDddTextFiles() throws IOException { Map.Entry expectedOutput = expectedOutputIt.next(); // verify segment name order for sanity. - assertWithMessage("mismatched section names in newDdl.txt") - .that(newSegment.getKey()) - .isEqualTo(segmentName); - assertWithMessage("mismatched section names in expectedDdlDiff.txt") - .that(expectedOutput.getKey()) - .isEqualTo(segmentName); + assertWithMessage("section name in newDdl.txt differs from originalDdl.txt") + .that(segmentName) + .isEqualTo(newSegment.getKey()); + assertWithMessage("section name in expectedDdlDiff.txt differs from originalDdl.txt") + .that(segmentName) + .isEqualTo(expectedOutput.getKey()); + List expectedDiff = expectedOutput.getValue() != null ? Arrays.asList(expectedOutput.getValue().split("\n")) @@ -82,14 +82,14 @@ public void compareDddTextFiles() throws IOException { List expectedDiffNoDrops = expectedDiff.stream() .filter(statement -> !statement.matches(".*DROP (TABLE|COLUMN|CHANGE STREAM).*")) - .collect(Collectors.toCollection(LinkedList::new)); + .collect(Collectors.toList()); // remove any drop indexes from the expectedResults if they do not have an equivalent // CREATE statement. This is because we are allowing recreation of indexes, but not allowing // dropping of removed indexes. for (String statement : expectedDiff) { if (statement.startsWith("DROP INDEX ")) { - String indexName = statement.split(" ")[2]; + String indexName = Iterables.get(Splitter.on(' ').split(statement), 2); // see if there is a matching create statement Pattern p = Pattern.compile("CREATE .*INDEX " + indexName + " "); if (expectedDiffNoDrops.stream().noneMatch(s -> p.matcher(s).find())) { diff --git a/src/test/java/com/google/cloud/solutions/spannerddl/parser/DDLParserFromFileTest.java b/src/test/java/com/google/cloud/solutions/spannerddl/parser/DDLParserFromFileTest.java index 52ccad3..1d47af8 100644 --- a/src/test/java/com/google/cloud/solutions/spannerddl/parser/DDLParserFromFileTest.java +++ b/src/test/java/com/google/cloud/solutions/spannerddl/parser/DDLParserFromFileTest.java @@ -7,7 +7,6 @@ import java.io.IOException; import java.io.StringReader; import java.util.Iterator; -import java.util.LinkedHashMap; import java.util.Map; import java.util.Map.Entry; import org.junit.Test; @@ -17,8 +16,7 @@ public class DDLParserFromFileTest { @Test public void validateDDLfromFile() throws IOException { - LinkedHashMap tests = - ReadTestDatafile.readDdlSegmentsFromFile("ddlParserValidation.txt"); + Map tests = ReadTestDatafile.readDdlSegmentsFromFile("ddlParserValidation.txt"); Iterator> testIt = tests.entrySet().iterator(); @@ -53,7 +51,7 @@ public void validateDDLfromFile() throws IOException { @Test public void validateUnsupportedDDLfromFile() throws Exception { - LinkedHashMap tests = + Map tests = ReadTestDatafile.readDdlSegmentsFromFile("ddlParserUnsupported.txt"); Iterator> testIt = tests.entrySet().iterator(); diff --git a/src/test/java/com/google/cloud/solutions/spannerddl/testUtils/ReadTestDatafile.java b/src/test/java/com/google/cloud/solutions/spannerddl/testUtils/ReadTestDatafile.java index 104bb25..5162c5f 100644 --- a/src/test/java/com/google/cloud/solutions/spannerddl/testUtils/ReadTestDatafile.java +++ b/src/test/java/com/google/cloud/solutions/spannerddl/testUtils/ReadTestDatafile.java @@ -15,27 +15,27 @@ */ package com.google.cloud.solutions.spannerddl.testUtils; +import static java.nio.charset.StandardCharsets.UTF_8; + import java.io.BufferedReader; import java.io.File; -import java.io.FileReader; import java.io.IOException; +import java.nio.file.Files; import java.util.LinkedHashMap; +import java.util.Map; public abstract class ReadTestDatafile { /** * Reads the test data file, parsing out the test titles and data from the file. * - * @param filename * @return LinkedHashMap of segment name => contents - * @throws IOException */ - public static LinkedHashMap readDdlSegmentsFromFile(String filename) - throws IOException { + public static Map readDdlSegmentsFromFile(String filename) throws IOException { File file = new File("src/test/resources/" + filename).getAbsoluteFile(); LinkedHashMap output = new LinkedHashMap<>(); - try (BufferedReader in = new BufferedReader(new FileReader(file))) { + try (BufferedReader in = Files.newBufferedReader(file.toPath(), UTF_8)) { String sectionName = null; StringBuilder section = new StringBuilder();