Merge pull request #261 from mfvanek/feature/possible-object-name-ove…

…rflow Add a check for identifiers with maximum length
mfvanek · Nov 2, 2024 · 3ffe107 · 3ffe107
2 parents ebb0c90 + 7c232a8
commit 3ffe107
Show file tree

Hide file tree

Showing 15 changed files with 215 additions and 135 deletions.
diff --git a/build.gradle.kts b/build.gradle.kts
@@ -9,7 +9,7 @@ plugins {
 
 allprojects {
     group = "io.github.mfvanek"
-    version = "0.13.1"
+    version = "0.13.2"
 
     repositories {
         mavenLocal()

diff --git a/buildSrc/build.gradle.kts b/buildSrc/build.gradle.kts
@@ -8,7 +8,7 @@ repositories {
 
 dependencies {
     implementation("org.sonarsource.scanner.gradle:sonarqube-gradle-plugin:5.1.0.4882")
-    implementation("com.github.spotbugs.snom:spotbugs-gradle-plugin:6.0.24")
+    implementation("com.github.spotbugs.snom:spotbugs-gradle-plugin:6.0.25")
     implementation("net.ltgt.gradle:gradle-errorprone-plugin:4.0.1")
     implementation("info.solidsoft.gradle.pitest:gradle-pitest-plugin:1.15.0")
     implementation("org.gradle:test-retry-gradle-plugin:1.6.0")

diff --git a/buildSrc/src/main/kotlin/pg-index-health-demo.java-compilation.gradle.kts b/buildSrc/src/main/kotlin/pg-index-health-demo.java-compilation.gradle.kts
@@ -5,9 +5,8 @@ plugins {
 }
 
 java {
-    toolchain {
-        languageVersion = JavaLanguageVersion.of(17)
-    }
+    sourceCompatibility = JavaVersion.VERSION_21
+    targetCompatibility = JavaVersion.VERSION_21
 }
 
 tasks {

diff --git a/buildSrc/src/main/kotlin/pg-index-health-demo.java-conventions.gradle.kts b/buildSrc/src/main/kotlin/pg-index-health-demo.java-conventions.gradle.kts
@@ -22,16 +22,16 @@ configurations.configureEach {
 }
 
 dependencies {
-    implementation(platform("io.github.mfvanek:pg-index-health-bom:0.13.1"))
-    implementation(platform("org.testcontainers:testcontainers-bom:1.20.2"))
+    implementation(platform("io.github.mfvanek:pg-index-health-bom:0.13.2"))
+    implementation(platform("org.testcontainers:testcontainers-bom:1.20.3"))
 
     implementation("com.google.code.findbugs:jsr305:3.0.2")
     implementation("org.postgresql:postgresql:42.7.4")
 
-    testImplementation(platform("org.junit:junit-bom:5.11.2"))
+    testImplementation(platform("org.junit:junit-bom:5.11.3"))
     testImplementation(platform("org.assertj:assertj-bom:3.26.3"))
 
-    errorprone("com.google.errorprone:error_prone_core:2.33.0")
+    errorprone("com.google.errorprone:error_prone_core:2.35.1")
     errorprone("jp.skypencil.errorprone.slf4j:errorprone-slf4j:0.1.28")
 
     spotbugsSlf4j("org.slf4j:slf4j-simple:2.0.16")
@@ -77,7 +77,7 @@ checkstyle {
 }
 
 pmd {
-    toolVersion = "7.6.0"
+    toolVersion = "7.7.0"
     isConsoleOutput = true
     ruleSetFiles = files("${rootDir}/config/pmd/pmd.xml")
     ruleSets = listOf()

diff --git a/buildSrc/src/main/kotlin/pg-index-health-demo.pitest.gradle.kts b/buildSrc/src/main/kotlin/pg-index-health-demo.pitest.gradle.kts
@@ -14,7 +14,7 @@ dependencies {
 pitest {
     verbosity = "DEFAULT"
     junit5PluginVersion = "1.2.1"
-    pitestVersion = "1.16.1"
+    pitestVersion = "1.17.0"
     threads = 4
     if (System.getenv("STRYKER_DASHBOARD_API_KEY") != null) {
         outputFormats.set(setOf("stryker-dashboard"))

diff --git a/config/pmd/pmd.xml b/config/pmd/pmd.xml
@@ -6,8 +6,8 @@
     <description>PMD configuration</description>
 
     <rule ref="category/java/bestpractices.xml">
-        <exclude name="JUnitTestContainsTooManyAsserts"/>
-        <exclude name="JUnitAssertionsShouldIncludeMessage"/>
+        <exclude name="UnitTestContainsTooManyAsserts"/>
+        <exclude name="UnitTestAssertionsShouldIncludeMessage"/>
         <exclude name="GuardLogStatement"/>
         <exclude name="CheckResultSet"/> <!-- false positive. broken since v.7.0.0 -->
     </rule>

diff --git a/db-migrations/src/main/resources/db/changelog/db.changelog-master.yaml b/db-migrations/src/main/resources/db/changelog/db.changelog-master.yaml
@@ -1,6 +1,8 @@
 databaseChangeLog:
     - include:
         file: db/changelog/sql/schema.sql
+    - include:
+          file: db/changelog/sql/warehouse.sql
     - include:
         file: db/changelog/sql/buyer.sql
     - include:

diff --git a/db-migrations/src/main/resources/db/changelog/sql/courier.sql b/db-migrations/src/main/resources/db/changelog/sql/courier.sql
@@ -15,3 +15,7 @@ comment on column demo.courier.first_name is 'Courier''s given name';
 comment on column demo.courier.last_name is 'Courier''s last name';
 comment on column demo.courier.phone is 'Courier''s phone number';
 comment on column demo.courier.email is 'Courier''s email address';
+
+--changeset ivan.vakhrushev:2024.10.20:courier.indexes runInTransaction:false
+create unique index concurrently if not exists idx_courier_phone_and_email_should_be_unique_very_long_name_that_will_be_truncated
+    on demo.courier (phone, email);
diff --git a/db-migrations/src/main/resources/db/changelog/sql/order_item.sql b/db-migrations/src/main/resources/db/changelog/sql/order_item.sql
@@ -37,3 +37,6 @@ create index if not exists order_item_categories_idx on demo.order_item(categori
 
 --changeset ivan.vakhrushev:2020.10.10:order_item.duplicated_foreign_key
 alter table demo.order_item add constraint order_item_order_id_fk_duplicate foreign key (order_id) references demo.orders (id);
+
+--changeset ivan.vakhrushev:2024.11.01:warehouse.reference
+alter table demo.order_item add constraint order_item_warehouse_id_fk foreign key (warehouse_id) references demo.warehouse (id);
diff --git a/db-migrations/src/main/resources/db/changelog/sql/warehouse.sql b/db-migrations/src/main/resources/db/changelog/sql/warehouse.sql
@@ -0,0 +1,12 @@
+--liquibase formatted sql
+
+--changeset ivan.vakhrushev:2024.11.01:warehouse.table
+create table if not exists demo.warehouse
+(
+    id   bigint primary key generated always as identity,
+    name varchar(255) not null
+);
+
+comment on table demo.warehouse is 'Information about the warehouses';
+comment on column demo.warehouse.id is 'Unique identifier of the warehouse';
+comment on column demo.warehouse.name is 'Human readable name of the warehouse';
diff --git a/...t-spring/src/test/java/io/github/mfvanek/pg/index/health/demo/IndexesMaintenanceTest.java b/...t-spring/src/test/java/io/github/mfvanek/pg/index/health/demo/IndexesMaintenanceTest.java
@@ -14,17 +14,21 @@
 import io.github.mfvanek.pg.checks.host.DuplicatedForeignKeysCheckOnHost;
 import io.github.mfvanek.pg.checks.host.DuplicatedIndexesCheckOnHost;
 import io.github.mfvanek.pg.checks.host.ForeignKeysNotCoveredWithIndexCheckOnHost;
+import io.github.mfvanek.pg.checks.host.ForeignKeysWithUnmatchedColumnTypeCheckOnHost;
 import io.github.mfvanek.pg.checks.host.FunctionsWithoutDescriptionCheckOnHost;
 import io.github.mfvanek.pg.checks.host.IndexesWithBooleanCheckOnHost;
 import io.github.mfvanek.pg.checks.host.IndexesWithNullValuesCheckOnHost;
 import io.github.mfvanek.pg.checks.host.IntersectedForeignKeysCheckOnHost;
 import io.github.mfvanek.pg.checks.host.IntersectedIndexesCheckOnHost;
 import io.github.mfvanek.pg.checks.host.InvalidIndexesCheckOnHost;
 import io.github.mfvanek.pg.checks.host.NotValidConstraintsCheckOnHost;
+import io.github.mfvanek.pg.checks.host.PossibleObjectNameOverflowCheckOnHost;
 import io.github.mfvanek.pg.checks.host.PrimaryKeysWithSerialTypesCheckOnHost;
 import io.github.mfvanek.pg.checks.host.SequenceOverflowCheckOnHost;
+import io.github.mfvanek.pg.checks.host.TablesNotLinkedToOthersCheckOnHost;
 import io.github.mfvanek.pg.checks.host.TablesWithoutDescriptionCheckOnHost;
 import io.github.mfvanek.pg.checks.host.TablesWithoutPrimaryKeyCheckOnHost;
+import io.github.mfvanek.pg.common.maintenance.CheckTypeAware;
 import io.github.mfvanek.pg.common.maintenance.DatabaseCheckOnHost;
 import io.github.mfvanek.pg.common.maintenance.Diagnostic;
 import io.github.mfvanek.pg.connection.PgConnection;
@@ -43,8 +47,11 @@
 import io.github.mfvanek.pg.model.index.IndexWithColumns;
 import io.github.mfvanek.pg.model.index.IndexWithNulls;
 import io.github.mfvanek.pg.model.index.IndexWithSize;
+import io.github.mfvanek.pg.model.object.AnyObject;
+import io.github.mfvanek.pg.model.object.PgObjectType;
 import io.github.mfvanek.pg.model.sequence.SequenceState;
 import io.github.mfvanek.pg.model.table.Table;
+import org.assertj.core.api.ListAssert;
 import org.junit.jupiter.api.DisplayName;
 import org.junit.jupiter.api.Test;
 
@@ -89,7 +96,10 @@ class IndexesMaintenanceTest extends DatabaseAwareTestBase {
             new SequenceOverflowCheckOnHost(pgConnection),
             new PrimaryKeysWithSerialTypesCheckOnHost(pgConnection),
             new DuplicatedForeignKeysCheckOnHost(pgConnection),
-            new IntersectedForeignKeysCheckOnHost(pgConnection)
+            new IntersectedForeignKeysCheckOnHost(pgConnection),
+            new PossibleObjectNameOverflowCheckOnHost(pgConnection),
+            new TablesNotLinkedToOthersCheckOnHost(pgConnection),
+            new ForeignKeysWithUnmatchedColumnTypeCheckOnHost(pgConnection)
         );
     }
 
@@ -107,26 +117,35 @@ void checkPostgresVersion() throws SQLException {
     }
 
     @Test
-    void databaseStructureCheckForPublicSchema() {
+    void completenessTest() {
         assertThat(checks)
-            .hasSize(Diagnostic.values().length - SKIPPED_CHECKS_COUNT);
+            .hasSize(Diagnostic.values().length - SKIPPED_CHECKS_COUNT)
+            .filteredOn(c -> c.getDiagnostic() != Diagnostic.SEQUENCE_OVERFLOW)
+            .as("Only static checks should present in list")
+            .allMatch(CheckTypeAware::isStatic);
+    }
 
+    @SuppressWarnings("PMD.UnitTestShouldIncludeAssert")
+    @Test
+    void databaseStructureCheckForPublicSchema() {
         checks.forEach(check -> {
-            final List<? extends DbObject> checkResult = check.check();
+            final ListAssert<? extends DbObject> checksAssert = assertThat(check.check())
+                .as(check.getDiagnostic().name());
+
             switch (check.getDiagnostic()) {
-                case TABLES_WITHOUT_PRIMARY_KEY, TABLES_WITHOUT_DESCRIPTION -> assertThat(checkResult)
+                case TABLES_WITHOUT_PRIMARY_KEY, TABLES_WITHOUT_DESCRIPTION, TABLES_NOT_LINKED_TO_OTHERS -> checksAssert
                     .asInstanceOf(list(Table.class))
                     .hasSize(1)
                     // HOW TO FIX: just add liquibase table to exclusions
                     .containsExactly(Table.of("databasechangelog", 0L));
 
-                case COLUMNS_WITHOUT_DESCRIPTION -> assertThat(checkResult)
+                case COLUMNS_WITHOUT_DESCRIPTION -> checksAssert
                     .asInstanceOf(list(Column.class))
                     // HOW TO FIX: just add liquibase table to exclusions
                     .hasSize(14)
                     .allMatch(c -> "databasechangelog".equals(c.getTableName()));
 
-                default -> assertThat(checkResult).isEmpty();
+                default -> checksAssert.isEmpty();
             }
         });
     }
@@ -138,23 +157,25 @@ void databaseStructureCheckForDemoSchema() {
             .hasSize(Diagnostic.values().length - SKIPPED_CHECKS_COUNT);
 
         checks.forEach(check -> {
-            final List<? extends DbObject> checkResult = check.check(demoSchema);
+            final ListAssert<? extends DbObject> checksAssert = assertThat(check.check(demoSchema))
+                .as(check.getDiagnostic().name());
+
             switch (check.getDiagnostic()) {
-                case INVALID_INDEXES -> assertThat(checkResult)
+                case INVALID_INDEXES -> checksAssert
                     .asInstanceOf(list(Index.class))
                     .hasSize(1)
                     // HOW TO FIX: drop index concurrently, fix data in table, then create index concurrently again
                     .containsExactly(Index.of(BUYER_TABLE, "demo.i_buyer_email"));
 
-                case DUPLICATED_INDEXES -> assertThat(checkResult)
+                case DUPLICATED_INDEXES -> checksAssert
                     .asInstanceOf(list(DuplicatedIndexes.class))
                     .hasSize(1)
                     // HOW TO FIX: do not manually create index for column with unique constraint
                     .containsExactly(DuplicatedIndexes.of(
                         IndexWithSize.of(ORDER_ITEM_TABLE, "demo.i_order_item_sku_order_id_unique", 8_192L),
                         IndexWithSize.of(ORDER_ITEM_TABLE, "demo.order_item_sku_order_id_key", 8_192L)));
 
-                case INTERSECTED_INDEXES -> assertThat(checkResult)
+                case INTERSECTED_INDEXES -> checksAssert
                     .asInstanceOf(list(DuplicatedIndexes.class))
                     .hasSize(2)
                     // HOW TO FIX: consider using an index with a different column order or just delete unnecessary indexes
@@ -166,64 +187,77 @@ void databaseStructureCheckForDemoSchema() {
                             IndexWithSize.of(BUYER_TABLE, "demo.i_buyer_first_name", 1L),
                             IndexWithSize.of(BUYER_TABLE, "demo.i_buyer_names", 1L)));
 
-                case FOREIGN_KEYS_WITHOUT_INDEX -> assertThat(checkResult)
+                case FOREIGN_KEYS_WITHOUT_INDEX -> checksAssert
                     .asInstanceOf(list(ForeignKey.class))
-                    .hasSize(4)
+                    .hasSize(5)
                     // HOW TO FIX: create indexes on columns under foreign key constraint
                     .containsExactlyInAnyOrder(
                         ForeignKey.ofNotNullColumn(ORDER_ITEM_TABLE, "order_item_order_id_fkey", ORDER_ID_COLUMN),
                         ForeignKey.ofNotNullColumn(ORDER_ITEM_TABLE, "order_item_order_id_fk_duplicate", ORDER_ID_COLUMN),
+                        ForeignKey.ofNotNullColumn(ORDER_ITEM_TABLE, "order_item_warehouse_id_fk", "warehouse_id"),
                         ForeignKey.ofNotNullColumn(ORDERS_TABLE, "orders_buyer_id_fkey", "buyer_id"),
                         ForeignKey.ofNullableColumn("demo.payment", "payment_order_id_fkey", ORDER_ID_COLUMN));
 
-                case TABLES_WITHOUT_PRIMARY_KEY -> assertThat(checkResult)
+                case TABLES_WITHOUT_PRIMARY_KEY -> checksAssert
                     .asInstanceOf(list(Table.class))
                     .hasSize(1)
                     // HOW TO FIX: add primary key to the table
                     .containsExactly(Table.of("demo.payment", 1L));
 
-                case INDEXES_WITH_NULL_VALUES -> assertThat(checkResult)
+                case INDEXES_WITH_NULL_VALUES -> checksAssert
                     .asInstanceOf(list(IndexWithNulls.class))
                     .hasSize(1)
                     // HOW TO FIX: consider excluding null values from index if it's possible
                     .containsExactly(IndexWithNulls.of(BUYER_TABLE, "demo.i_buyer_middle_name", 1L, "middle_name"));
 
-                case INDEXES_WITH_BOOLEAN -> assertThat(checkResult)
+                case INDEXES_WITH_BOOLEAN -> checksAssert
                     .asInstanceOf(list(IndexWithColumns.class))
                     .hasSize(1)
                     .contains(IndexWithColumns.ofSingle(ORDERS_TABLE, "demo.i_orders_preorder", 1L,
                         Column.ofNotNull(ORDERS_TABLE, "preorder")));
 
-                case NOT_VALID_CONSTRAINTS -> assertThat(checkResult)
+                case NOT_VALID_CONSTRAINTS -> checksAssert
                     .asInstanceOf(list(Constraint.class))
                     .hasSize(1)
                     .contains(Constraint.ofType(ORDER_ITEM_TABLE, "order_item_amount_less_than_100", ConstraintType.CHECK));
 
-                case BTREE_INDEXES_ON_ARRAY_COLUMNS -> assertThat(checkResult)
+                case BTREE_INDEXES_ON_ARRAY_COLUMNS -> checksAssert
                     .asInstanceOf(list(IndexWithColumns.class))
                     .hasSize(1)
                     .containsExactly(IndexWithColumns.ofSingle(ORDER_ITEM_TABLE, "demo.order_item_categories_idx", 8_192L,
                         Column.ofNullable(ORDER_ITEM_TABLE, "categories")));
 
-                case SEQUENCE_OVERFLOW -> assertThat(checkResult)
+                case SEQUENCE_OVERFLOW -> checksAssert
                     .asInstanceOf(list(SequenceState.class))
                     .hasSize(1)
                     .containsExactly(SequenceState.of("demo.payment_num_seq", "smallint", 8.44));
 
-                case PRIMARY_KEYS_WITH_SERIAL_TYPES -> assertThat(checkResult)
+                case PRIMARY_KEYS_WITH_SERIAL_TYPES -> checksAssert
                     .asInstanceOf(list(ColumnWithSerialType.class))
                     .hasSize(1)
                     .containsExactly(ColumnWithSerialType.ofBigSerial(Column.ofNotNull("demo.courier", "id"), "demo.courier_id_seq"));
 
-                case DUPLICATED_FOREIGN_KEYS -> assertThat(checkResult)
+                case DUPLICATED_FOREIGN_KEYS -> checksAssert
                     .asInstanceOf(list(DuplicatedForeignKeys.class))
                     .hasSize(1)
                     .containsExactly(DuplicatedForeignKeys.of(
                         ForeignKey.ofNotNullColumn(ORDER_ITEM_TABLE, "order_item_order_id_fk_duplicate", ORDER_ID_COLUMN),
                         ForeignKey.ofNotNullColumn(ORDER_ITEM_TABLE, "order_item_order_id_fkey", ORDER_ID_COLUMN))
                     );
 
-                default -> assertThat(checkResult).isEmpty();
+                case POSSIBLE_OBJECT_NAME_OVERFLOW -> checksAssert
+                    .asInstanceOf(list(AnyObject.class))
+                    .hasSize(1)
+                    .containsExactly(
+                        AnyObject.ofType("demo.idx_courier_phone_and_email_should_be_unique_very_long_name_tha", PgObjectType.INDEX));
+
+                case FOREIGN_KEYS_WITH_UNMATCHED_COLUMN_TYPE -> checksAssert
+                    .asInstanceOf(list(ForeignKey.class))
+                    .hasSize(1)
+                    .containsExactly(ForeignKey.ofNotNullColumn(ORDER_ITEM_TABLE, "order_item_warehouse_id_fk", "warehouse_id"));
+
+                default -> checksAssert
+                    .isEmpty();
             }
         });
     }

diff --git a/...g/src/test/java/io/github/mfvanek/pg/index/health/demo/utils/HealthDataCollectorTest.java b/...g/src/test/java/io/github/mfvanek/pg/index/health/demo/utils/HealthDataCollectorTest.java
@@ -24,7 +24,7 @@ void shouldCollectHealthData() {
             "db_indexes_health\tduplicated_indexes\t1",
             "db_indexes_health\tintersected_indexes\t2",
             "db_indexes_health\tunused_indexes\t0",
-            "db_indexes_health\tforeign_keys_without_index\t4",
+            "db_indexes_health\tforeign_keys_without_index\t5",
             "db_indexes_health\ttables_with_missing_indexes\t0",
             "db_indexes_health\ttables_without_primary_key\t1",
             "db_indexes_health\tindexes_with_null_values\t1",
@@ -41,7 +41,10 @@ void shouldCollectHealthData() {
             "db_indexes_health\tsequence_overflow\t1",
             "db_indexes_health\tprimary_keys_with_serial_types\t1",
             "db_indexes_health\tduplicated_foreign_keys\t1",
-            "db_indexes_health\tintersected_foreign_keys\t0");
+            "db_indexes_health\tintersected_foreign_keys\t0",
+            "db_indexes_health\tpossible_object_name_overflow\t1",
+            "db_indexes_health\ttables_not_linked_to_others\t0",
+            "db_indexes_health\tforeign_keys_with_unmatched_column_type\t1");
         final List<String> healthData = HealthDataCollector.collectHealthData(getConnectionFactory(), getConnectionCredentials());
         assertThat(healthData)
             .hasSameSizeAs(Diagnostic.values())

diff --git a/...g/src/test/java/io/github/mfvanek/pg/index/health/demo/utils/MigrationsGeneratorTest.java b/...g/src/test/java/io/github/mfvanek/pg/index/health/demo/utils/MigrationsGeneratorTest.java
@@ -35,7 +35,7 @@ static void restoreSchema() {
     void generateMigrationsShouldWork() {
         final List<ForeignKey> foreignKeys = MigrationsGenerator.getForeignKeysNotCoveredWithIndex(getConnectionFactory(), getConnectionCredentials());
         assertThat(foreignKeys)
-            .hasSize(4);
+            .hasSize(5);
         MigrationsGenerator.generateMigrations(getDataSource(), foreignKeys);
         assertThat(MigrationsGenerator.getForeignKeysNotCoveredWithIndex(getConnectionFactory(), getConnectionCredentials()))
             .isEmpty();