From 7e6b0842b3b53648adfbd4e6de27bbb6e89c0a87 Mon Sep 17 00:00:00 2001 From: Simon Brandhof Date: Tue, 6 Dec 2016 20:56:01 +0100 Subject: [PATCH] SONAR-5471 share validations of index/table/constraint names --- .../sonar/db/version/AddColumnsBuilder.java | 3 +- .../sonar/db/version/CreateTableBuilder.java | 27 +++++----- .../sonar/db/version/DropTableBuilder.java | 5 +- .../org/sonar/db/version/Validations.java | 51 ++++++++++++++++++- .../db/version/CreateTableBuilderTest.java | 12 ++--- .../org/sonar/db/version/ValidationsTest.java | 22 ++++++++ 6 files changed, 94 insertions(+), 26 deletions(-) diff --git a/sonar-db/src/main/java/org/sonar/db/version/AddColumnsBuilder.java b/sonar-db/src/main/java/org/sonar/db/version/AddColumnsBuilder.java index ed2628aa65a..f61a30dd3df 100644 --- a/sonar-db/src/main/java/org/sonar/db/version/AddColumnsBuilder.java +++ b/sonar-db/src/main/java/org/sonar/db/version/AddColumnsBuilder.java @@ -25,6 +25,7 @@ import org.sonar.db.dialect.MsSql; import org.sonar.db.dialect.PostgreSql; import static com.google.common.collect.Lists.newArrayList; +import static org.sonar.db.version.Validations.validateTableName; /** * Generate a SQL query to add multiple columns on a table @@ -36,7 +37,7 @@ public class AddColumnsBuilder { private List columnDefs = newArrayList(); public AddColumnsBuilder(Dialect dialect, String tableName) { - this.tableName = tableName; + this.tableName = validateTableName(tableName); this.dialect = dialect; } diff --git a/sonar-db/src/main/java/org/sonar/db/version/CreateTableBuilder.java b/sonar-db/src/main/java/org/sonar/db/version/CreateTableBuilder.java index 955106533af..84b34fda5b4 100644 --- a/sonar-db/src/main/java/org/sonar/db/version/CreateTableBuilder.java +++ b/sonar-db/src/main/java/org/sonar/db/version/CreateTableBuilder.java @@ -41,9 +41,8 @@ import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.base.Preconditions.checkState; import static java.util.Objects.requireNonNull; import static java.util.stream.Stream.of; -import static org.sonar.db.version.Validations.CONSTRAINT_NAME_MAX_SIZE; -import static org.sonar.db.version.Validations.TABLE_NAME_MAX_SIZE; -import static org.sonar.db.version.Validations.checkDbIdentifier; +import static org.sonar.db.version.Validations.validateConstraintName; +import static org.sonar.db.version.Validations.validateTableName; public class CreateTableBuilder { @@ -57,7 +56,7 @@ public class CreateTableBuilder { public CreateTableBuilder(Dialect dialect, String tableName) { this.dialect = requireNonNull(dialect, "dialect can't be null"); - this.tableName = checkDbIdentifier(tableName, "Table name", TABLE_NAME_MAX_SIZE); + this.tableName = validateTableName(tableName); } public List build() { @@ -102,7 +101,7 @@ public class CreateTableBuilder { } public CreateTableBuilder withPkConstraintName(String pkConstraintName) { - this.pkConstraintName = checkDbIdentifier(pkConstraintName, "Primary key constraint name", CONSTRAINT_NAME_MAX_SIZE); + this.pkConstraintName = validateConstraintName(pkConstraintName); return this; } @@ -237,8 +236,8 @@ public class CreateTableBuilder { return Stream.empty(); } return pkColumnDefs.stream() - .filter(this::isAutoIncrement) - .flatMap(columnDef -> of(createSequenceFor(tableName), createTriggerFor(tableName))); + .filter(this::isAutoIncrement) + .flatMap(columnDef -> of(createSequenceFor(tableName), createTriggerFor(tableName))); } private static String createSequenceFor(String tableName) { @@ -247,13 +246,13 @@ public class CreateTableBuilder { private static String createTriggerFor(String tableName) { return "CREATE OR REPLACE TRIGGER " + tableName + "_idt" + - " BEFORE INSERT ON " + tableName + - " FOR EACH ROW" + - " BEGIN" + - " IF :new.id IS null THEN" + - " SELECT " + tableName + "_seq.nextval INTO :new.id FROM dual;" + - " END IF;" + - " END;"; + " BEFORE INSERT ON " + tableName + + " FOR EACH ROW" + + " BEGIN" + + " IF :new.id IS null THEN" + + " SELECT " + tableName + "_seq.nextval INTO :new.id FROM dual;" + + " END IF;" + + " END;"; } public enum ColumnFlag { diff --git a/sonar-db/src/main/java/org/sonar/db/version/DropTableBuilder.java b/sonar-db/src/main/java/org/sonar/db/version/DropTableBuilder.java index de573851f44..029d3a1b474 100644 --- a/sonar-db/src/main/java/org/sonar/db/version/DropTableBuilder.java +++ b/sonar-db/src/main/java/org/sonar/db/version/DropTableBuilder.java @@ -26,8 +26,7 @@ import org.sonar.db.dialect.Oracle; import static java.util.Arrays.asList; import static java.util.Collections.singletonList; import static java.util.Objects.requireNonNull; -import static org.sonar.db.version.Validations.TABLE_NAME_MAX_SIZE; -import static org.sonar.db.version.Validations.checkDbIdentifier; +import static org.sonar.db.version.Validations.validateTableName; public class DropTableBuilder { @@ -36,7 +35,7 @@ public class DropTableBuilder { public DropTableBuilder(Dialect dialect, String tableName) { this.dialect = requireNonNull(dialect, "dialect can't be null"); - this.tableName = checkDbIdentifier(tableName, "Table name", TABLE_NAME_MAX_SIZE); + this.tableName = validateTableName(tableName); } public List build() { diff --git a/sonar-db/src/main/java/org/sonar/db/version/Validations.java b/sonar-db/src/main/java/org/sonar/db/version/Validations.java index cf75f9ff18d..a21f5413741 100644 --- a/sonar-db/src/main/java/org/sonar/db/version/Validations.java +++ b/sonar-db/src/main/java/org/sonar/db/version/Validations.java @@ -30,8 +30,9 @@ import static java.util.Objects.requireNonNull; public class Validations { - static final int TABLE_NAME_MAX_SIZE = 25; - static final int CONSTRAINT_NAME_MAX_SIZE = 30; + private static final int TABLE_NAME_MAX_SIZE = 25; + private static final int CONSTRAINT_NAME_MAX_SIZE = 30; + private static final int INDEX_NAME_MAX_SIZE = 30; private static final CharMatcher DIGIT_CHAR_MATCHER = inRange('0', '9'); private static final CharMatcher LOWER_CASE_ASCII_LETTERS_CHAR_MATCHER = inRange('a', 'z'); @@ -41,12 +42,58 @@ public class Validations { // Only static stuff here } + /** + * Ensure {@code columnName} is a valid name for a column. + * @throws NullPointerException if {@code columnName} is {@code null} + * @throws IllegalArgumentException if {@code columnName} is not valid + * @return the same {@code columnName} + * @see #checkDbIdentifier(String, String, int) + */ static String validateColumnName(@Nullable String columnName) { String name = requireNonNull(columnName, "Column name cannot be null"); checkDbIdentifierCharacters(columnName, "Column name"); return name; } + /** + * Ensure {@code tableName} is a valid name for a table. + * @throws NullPointerException if {@code tableName} is {@code null} + * @throws IllegalArgumentException if {@code tableName} is not valid + * @return the same {@code tableName} + * @see #checkDbIdentifier(String, String, int) + */ + static String validateTableName(@Nullable String tableName) { + requireNonNull(tableName, "Table name cannot be null"); + checkDbIdentifier(tableName, "Table name", TABLE_NAME_MAX_SIZE); + return tableName; + } + + /** + * Ensure {@code constraintName} is a valid name for a constraint. + * @throws NullPointerException if {@code constraintName} is {@code null} + * @throws IllegalArgumentException if {@code constraintName} is not valid + * @return the same {@code constraintName} + * @see #checkDbIdentifier(String, String, int) + */ + static String validateConstraintName(@Nullable String constraintName) { + requireNonNull(constraintName, "Constraint name cannot be null"); + checkDbIdentifier(constraintName, "Constraint name", CONSTRAINT_NAME_MAX_SIZE); + return constraintName; + } + + /** + * Ensure {@code indexName} is a valid name for an index. + * @throws NullPointerException if {@code indexName} is {@code null} + * @throws IllegalArgumentException if {@code indexName} is not valid + * @return the same {@code indexName} + * @see #checkDbIdentifier(String, String, int) + */ + static String validateIndexName(@Nullable String indexName) { + requireNonNull(indexName, "Index name cannot be null"); + checkDbIdentifier(indexName, "Index name", INDEX_NAME_MAX_SIZE); + return indexName; + } + /** * Ensure {@code identifier} is a valid DB identifier. * diff --git a/sonar-db/src/test/java/org/sonar/db/version/CreateTableBuilderTest.java b/sonar-db/src/test/java/org/sonar/db/version/CreateTableBuilderTest.java index 2b11ff4eb89..12b32882f05 100644 --- a/sonar-db/src/test/java/org/sonar/db/version/CreateTableBuilderTest.java +++ b/sonar-db/src/test/java/org/sonar/db/version/CreateTableBuilderTest.java @@ -73,7 +73,7 @@ public class CreateTableBuilderTest { @Test public void constructor_fails_with_NPE_if_tablename_is_null() { expectedException.expect(NullPointerException.class); - expectedException.expectMessage("Table name can't be null"); + expectedException.expectMessage("Table name cannot be null"); new CreateTableBuilder(mock(Dialect.class), null); } @@ -313,7 +313,7 @@ public class CreateTableBuilderTest { @Test public void withPkConstraintName_throws_NPE_if_name_is_null() { expectedException.expect(NullPointerException.class); - expectedException.expectMessage("Primary key constraint name can't be null"); + expectedException.expectMessage("Constraint name cannot be null"); underTest.withPkConstraintName(null); } @@ -321,7 +321,7 @@ public class CreateTableBuilderTest { @Test public void withPkConstraintName_throws_IAE_if_name_is_not_lowercase() { expectedException.expect(IllegalArgumentException.class); - expectedException.expectMessage("Primary key constraint name must be lower case and contain only alphanumeric chars or '_', got 'Too'"); + expectedException.expectMessage("Constraint name must be lower case and contain only alphanumeric chars or '_', got 'Too'"); underTest.withPkConstraintName("Too"); } @@ -329,7 +329,7 @@ public class CreateTableBuilderTest { @Test public void withPkConstraintName_throws_IAE_if_name_is_more_than_30_char_long() { expectedException.expect(IllegalArgumentException.class); - expectedException.expectMessage("Primary key constraint name length can't be more than 30"); + expectedException.expectMessage("Constraint name length can't be more than 30"); underTest.withPkConstraintName("abcdefghijklmnopqrstuvwxyzabcdf"); } @@ -337,7 +337,7 @@ public class CreateTableBuilderTest { @Test public void withPkConstraintName_throws_IAE_if_name_starts_with_underscore() { expectedException.expect(IllegalArgumentException.class); - expectedException.expectMessage("Primary key constraint name must not start by a number or '_', got '_a'"); + expectedException.expectMessage("Constraint name must not start by a number or '_', got '_a'"); underTest.withPkConstraintName("_a"); } @@ -346,7 +346,7 @@ public class CreateTableBuilderTest { @UseDataProvider("digitCharsDataProvider") public void withPkConstraintName_throws_IAE_if_name_starts_with_number(char number) { expectedException.expect(IllegalArgumentException.class); - expectedException.expectMessage("Primary key constraint name must not start by a number or '_', got '" + number + "a'"); + expectedException.expectMessage("Constraint name must not start by a number or '_', got '" + number + "a'"); underTest.withPkConstraintName(number + "a"); } diff --git a/sonar-db/src/test/java/org/sonar/db/version/ValidationsTest.java b/sonar-db/src/test/java/org/sonar/db/version/ValidationsTest.java index bd66ad3f3de..bb1913a6e4a 100644 --- a/sonar-db/src/test/java/org/sonar/db/version/ValidationsTest.java +++ b/sonar-db/src/test/java/org/sonar/db/version/ValidationsTest.java @@ -23,7 +23,9 @@ import org.junit.Rule; import org.junit.Test; import org.junit.rules.ExpectedException; +import static org.assertj.core.api.Assertions.assertThat; import static org.sonar.db.version.Validations.validateColumnName; +import static org.sonar.db.version.Validations.validateIndexName; public class ValidationsTest { @@ -60,4 +62,24 @@ public class ValidationsTest { validateColumnName("date-in/ms"); } + @Test + public void validateIndexName_throws_IAE_when_index_name_contains_invalid_characters() { + thrown.expect(IllegalArgumentException.class); + thrown.expectMessage("Index name must be lower case and contain only alphanumeric chars or '_', got '(not/valid)'"); + + validateIndexName("(not/valid)"); + } + + @Test + public void validateIndexName_throws_NPE_when_index_name_is_null() { + thrown.expect(NullPointerException.class); + thrown.expectMessage("Index name cannot be null"); + + validateIndexName(null); + } + + @Test + public void validateIndexName_returns_valid_name() { + assertThat(validateIndexName("foo")).isEqualTo("foo"); + } } -- 2.39.5