From 537445e77d41276e3f743ca2a20c5f6dae823909 Mon Sep 17 00:00:00 2001 From: Ivan Vakhrushev Date: Sun, 20 Oct 2024 18:23:44 +0400 Subject: [PATCH 1/5] Add a check for identifiers with maximum length --- build.gradle.kts | 2 +- buildSrc/build.gradle.kts | 2 +- ...ex-health-demo.java-compilation.gradle.kts | 5 +- ...ex-health-demo.java-conventions.gradle.kts | 2 +- .../pg-index-health-demo.pitest.gradle.kts | 2 +- .../resources/db/changelog/sql/courier.sql | 4 + .../health/demo/IndexesMaintenanceTest.java | 24 ++- .../demo/utils/HealthDataCollectorTest.java | 3 +- .../health/demo/IndexesMaintenanceTest.java | 191 +++++++++--------- .../controller/DbHealthControllerTest.java | 3 +- 10 files changed, 135 insertions(+), 103 deletions(-) diff --git a/build.gradle.kts b/build.gradle.kts index a89b23f..7f9cd62 100644 --- 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 index cc833a8..92f9d94 100644 --- 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 index aa7571a..3067dd0 100644 --- 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 index d5a0c7f..b46bae4 100644 --- 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,7 +22,7 @@ configurations.configureEach { } dependencies { - implementation(platform("io.github.mfvanek:pg-index-health-bom:0.13.1")) + implementation(platform("io.github.mfvanek:pg-index-health-bom:0.13.2-SNAPSHOT")) implementation(platform("org.testcontainers:testcontainers-bom:1.20.2")) implementation("com.google.code.findbugs:jsr305:3.0.2") 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 index 8e1bc01..3339107 100644 --- 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/db-migrations/src/main/resources/db/changelog/sql/courier.sql b/db-migrations/src/main/resources/db/changelog/sql/courier.sql index 00246f6..f9c16d9 100644 --- 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/pg-index-health-demo-without-spring/src/test/java/io/github/mfvanek/pg/index/health/demo/IndexesMaintenanceTest.java b/pg-index-health-demo-without-spring/src/test/java/io/github/mfvanek/pg/index/health/demo/IndexesMaintenanceTest.java index fc72fbe..d1fa28a 100644 --- a/pg-index-health-demo-without-spring/src/test/java/io/github/mfvanek/pg/index/health/demo/IndexesMaintenanceTest.java +++ b/pg-index-health-demo-without-spring/src/test/java/io/github/mfvanek/pg/index/health/demo/IndexesMaintenanceTest.java @@ -21,10 +21,12 @@ 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.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,6 +45,8 @@ 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.junit.jupiter.api.DisplayName; @@ -89,7 +93,8 @@ class IndexesMaintenanceTest extends DatabaseAwareTestBase { new SequenceOverflowCheckOnHost(pgConnection), new PrimaryKeysWithSerialTypesCheckOnHost(pgConnection), new DuplicatedForeignKeysCheckOnHost(pgConnection), - new IntersectedForeignKeysCheckOnHost(pgConnection) + new IntersectedForeignKeysCheckOnHost(pgConnection), + new PossibleObjectNameOverflowCheckOnHost(pgConnection) ); } @@ -107,10 +112,17 @@ 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.JUnitTestsShouldIncludeAssert") + @Test + void databaseStructureCheckForPublicSchema() { checks.forEach(check -> { final List checkResult = check.check(); switch (check.getDiagnostic()) { @@ -223,6 +235,12 @@ void databaseStructureCheckForDemoSchema() { ForeignKey.ofNotNullColumn(ORDER_ITEM_TABLE, "order_item_order_id_fkey", ORDER_ID_COLUMN)) ); + case POSSIBLE_OBJECT_NAME_OVERFLOW -> assertThat(checkResult) + .asInstanceOf(list(AnyObject.class)) + .hasSize(1) + .containsExactly( + AnyObject.ofType("demo.idx_courier_phone_and_email_should_be_unique_very_long_name_tha", PgObjectType.INDEX)); + default -> assertThat(checkResult).isEmpty(); } }); diff --git a/pg-index-health-demo-without-spring/src/test/java/io/github/mfvanek/pg/index/health/demo/utils/HealthDataCollectorTest.java b/pg-index-health-demo-without-spring/src/test/java/io/github/mfvanek/pg/index/health/demo/utils/HealthDataCollectorTest.java index 0354140..05b2e36 100644 --- a/pg-index-health-demo-without-spring/src/test/java/io/github/mfvanek/pg/index/health/demo/utils/HealthDataCollectorTest.java +++ b/pg-index-health-demo-without-spring/src/test/java/io/github/mfvanek/pg/index/health/demo/utils/HealthDataCollectorTest.java @@ -41,7 +41,8 @@ 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"); final List healthData = HealthDataCollector.collectHealthData(getConnectionFactory(), getConnectionCredentials()); assertThat(healthData) .hasSameSizeAs(Diagnostic.values()) diff --git a/pg-index-health-spring-boot-demo/src/test/java/io/github/mfvanek/pg/index/health/demo/IndexesMaintenanceTest.java b/pg-index-health-spring-boot-demo/src/test/java/io/github/mfvanek/pg/index/health/demo/IndexesMaintenanceTest.java index 3c7dffa..e076acd 100644 --- a/pg-index-health-spring-boot-demo/src/test/java/io/github/mfvanek/pg/index/health/demo/IndexesMaintenanceTest.java +++ b/pg-index-health-spring-boot-demo/src/test/java/io/github/mfvanek/pg/index/health/demo/IndexesMaintenanceTest.java @@ -23,6 +23,8 @@ 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.junit.jupiter.api.DisplayName; @@ -85,96 +87,103 @@ void databaseStructureCheckForDemoSchema() { assertThat(checks) .hasSize(Diagnostic.values().length); - checks.forEach(check -> { - final List checkResult = check.check(demoSchema); - switch (check.getDiagnostic()) { - case INVALID_INDEXES -> assertThat(checkResult) - .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) - .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) - .asInstanceOf(list(DuplicatedIndexes.class)) - .hasSize(2) - // HOW TO FIX: consider using an index with a different column order or just delete unnecessary indexes - .containsExactlyInAnyOrder( - DuplicatedIndexes.of( - IndexWithSize.of(BUYER_TABLE, "demo.buyer_pkey", 1L), - IndexWithSize.of(BUYER_TABLE, "demo.i_buyer_id_phone", 1L)), - DuplicatedIndexes.of( - 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) - .asInstanceOf(list(ForeignKey.class)) - .hasSize(4) - // 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(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) - .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) - .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) - .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) - .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) - .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) - .asInstanceOf(list(SequenceState.class)) - .hasSize(1) - .containsExactly(SequenceState.of("demo.payment_num_seq", "smallint", 8.44)); - - case PRIMARY_KEYS_WITH_SERIAL_TYPES -> assertThat(checkResult) - .asInstanceOf(list(ColumnWithSerialType.class)) - .hasSize(1) - .containsExactly(ColumnWithSerialType.ofBigSerial(Column.ofNotNull("demo.courier", "id"), "demo.courier_id_seq")); - - case UNUSED_INDEXES -> assertThat(checkResult).isNotEmpty(); // TODO Just ignore this "runtime" check https://github.com/mfvanek/pg-index-health/issues/456 - - case DUPLICATED_FOREIGN_KEYS -> assertThat(checkResult) - .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(); - } - }); + checks.stream() + // Skip all runtime checks except SEQUENCE_OVERFLOW + .filter(check -> check.getDiagnostic() == Diagnostic.SEQUENCE_OVERFLOW || check.isStatic()) + .forEach(check -> { + final List checkResult = check.check(demoSchema); + switch (check.getDiagnostic()) { + case INVALID_INDEXES -> assertThat(checkResult) + .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) + .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) + .asInstanceOf(list(DuplicatedIndexes.class)) + .hasSize(2) + // HOW TO FIX: consider using an index with a different column order or just delete unnecessary indexes + .containsExactlyInAnyOrder( + DuplicatedIndexes.of( + IndexWithSize.of(BUYER_TABLE, "demo.buyer_pkey", 1L), + IndexWithSize.of(BUYER_TABLE, "demo.i_buyer_id_phone", 1L)), + DuplicatedIndexes.of( + 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) + .asInstanceOf(list(ForeignKey.class)) + .hasSize(4) + // 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(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) + .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) + .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) + .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) + .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) + .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) + .asInstanceOf(list(SequenceState.class)) + .hasSize(1) + .containsExactly(SequenceState.of("demo.payment_num_seq", "smallint", 8.44)); + + case PRIMARY_KEYS_WITH_SERIAL_TYPES -> assertThat(checkResult) + .asInstanceOf(list(ColumnWithSerialType.class)) + .hasSize(1) + .containsExactly(ColumnWithSerialType.ofBigSerial(Column.ofNotNull("demo.courier", "id"), "demo.courier_id_seq")); + + case DUPLICATED_FOREIGN_KEYS -> assertThat(checkResult) + .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)) + ); + + case POSSIBLE_OBJECT_NAME_OVERFLOW -> assertThat(checkResult) + .asInstanceOf(list(AnyObject.class)) + .hasSize(1) + .containsExactly( + AnyObject.ofType("demo.idx_courier_phone_and_email_should_be_unique_very_long_name_tha", PgObjectType.INDEX)); + + default -> assertThat(checkResult).isEmpty(); + } + }); } } diff --git a/pg-index-health-spring-boot-demo/src/test/java/io/github/mfvanek/pg/index/health/demo/controller/DbHealthControllerTest.java b/pg-index-health-spring-boot-demo/src/test/java/io/github/mfvanek/pg/index/health/demo/controller/DbHealthControllerTest.java index 02d5af0..136b365 100644 --- a/pg-index-health-spring-boot-demo/src/test/java/io/github/mfvanek/pg/index/health/demo/controller/DbHealthControllerTest.java +++ b/pg-index-health-spring-boot-demo/src/test/java/io/github/mfvanek/pg/index/health/demo/controller/DbHealthControllerTest.java @@ -52,6 +52,7 @@ void collectHealthDataShouldReturnOk() { "sequence_overflow:1", "primary_keys_with_serial_types:1", "duplicated_foreign_keys:1", - "intersected_foreign_keys:0"); + "intersected_foreign_keys:0", + "possible_object_name_overflow:1"); } } From 14345fd094fd7d66113318b1d6c4e762f08efd25 Mon Sep 17 00:00:00 2001 From: Ivan Vakhrushev Date: Tue, 22 Oct 2024 23:06:58 +0400 Subject: [PATCH 2/5] Add check "Tables are not linked to other tables" --- .../pg/index/health/demo/IndexesMaintenanceTest.java | 6 ++++-- .../pg/index/health/demo/utils/HealthDataCollectorTest.java | 3 ++- .../pg/index/health/demo/IndexesMaintenanceTest.java | 2 +- .../health/demo/controller/DbHealthControllerTest.java | 3 ++- 4 files changed, 9 insertions(+), 5 deletions(-) diff --git a/pg-index-health-demo-without-spring/src/test/java/io/github/mfvanek/pg/index/health/demo/IndexesMaintenanceTest.java b/pg-index-health-demo-without-spring/src/test/java/io/github/mfvanek/pg/index/health/demo/IndexesMaintenanceTest.java index d1fa28a..ea33507 100644 --- a/pg-index-health-demo-without-spring/src/test/java/io/github/mfvanek/pg/index/health/demo/IndexesMaintenanceTest.java +++ b/pg-index-health-demo-without-spring/src/test/java/io/github/mfvanek/pg/index/health/demo/IndexesMaintenanceTest.java @@ -24,6 +24,7 @@ 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; @@ -94,7 +95,8 @@ class IndexesMaintenanceTest extends DatabaseAwareTestBase { new PrimaryKeysWithSerialTypesCheckOnHost(pgConnection), new DuplicatedForeignKeysCheckOnHost(pgConnection), new IntersectedForeignKeysCheckOnHost(pgConnection), - new PossibleObjectNameOverflowCheckOnHost(pgConnection) + new PossibleObjectNameOverflowCheckOnHost(pgConnection), + new TablesNotLinkedToOthersCheckOnHost(pgConnection) ); } @@ -126,7 +128,7 @@ void databaseStructureCheckForPublicSchema() { checks.forEach(check -> { final List checkResult = check.check(); 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 -> assertThat(checkResult) .asInstanceOf(list(Table.class)) .hasSize(1) // HOW TO FIX: just add liquibase table to exclusions diff --git a/pg-index-health-demo-without-spring/src/test/java/io/github/mfvanek/pg/index/health/demo/utils/HealthDataCollectorTest.java b/pg-index-health-demo-without-spring/src/test/java/io/github/mfvanek/pg/index/health/demo/utils/HealthDataCollectorTest.java index 05b2e36..911f43e 100644 --- a/pg-index-health-demo-without-spring/src/test/java/io/github/mfvanek/pg/index/health/demo/utils/HealthDataCollectorTest.java +++ b/pg-index-health-demo-without-spring/src/test/java/io/github/mfvanek/pg/index/health/demo/utils/HealthDataCollectorTest.java @@ -42,7 +42,8 @@ void shouldCollectHealthData() { "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\tpossible_object_name_overflow\t1"); + "db_indexes_health\tpossible_object_name_overflow\t1", + "db_indexes_health\ttables_not_linked_to_others\t0"); final List healthData = HealthDataCollector.collectHealthData(getConnectionFactory(), getConnectionCredentials()); assertThat(healthData) .hasSameSizeAs(Diagnostic.values()) diff --git a/pg-index-health-spring-boot-demo/src/test/java/io/github/mfvanek/pg/index/health/demo/IndexesMaintenanceTest.java b/pg-index-health-spring-boot-demo/src/test/java/io/github/mfvanek/pg/index/health/demo/IndexesMaintenanceTest.java index e076acd..d0c99bb 100644 --- a/pg-index-health-spring-boot-demo/src/test/java/io/github/mfvanek/pg/index/health/demo/IndexesMaintenanceTest.java +++ b/pg-index-health-spring-boot-demo/src/test/java/io/github/mfvanek/pg/index/health/demo/IndexesMaintenanceTest.java @@ -64,7 +64,7 @@ void databaseStructureCheckForPublicSchema() { checks.forEach(check -> { final List checkResult = check.check(); 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 -> assertThat(checkResult) .asInstanceOf(list(Table.class)) .hasSize(1) // HOW TO FIX: just add liquibase table to exclusions diff --git a/pg-index-health-spring-boot-demo/src/test/java/io/github/mfvanek/pg/index/health/demo/controller/DbHealthControllerTest.java b/pg-index-health-spring-boot-demo/src/test/java/io/github/mfvanek/pg/index/health/demo/controller/DbHealthControllerTest.java index 136b365..25fe4b7 100644 --- a/pg-index-health-spring-boot-demo/src/test/java/io/github/mfvanek/pg/index/health/demo/controller/DbHealthControllerTest.java +++ b/pg-index-health-spring-boot-demo/src/test/java/io/github/mfvanek/pg/index/health/demo/controller/DbHealthControllerTest.java @@ -53,6 +53,7 @@ void collectHealthDataShouldReturnOk() { "primary_keys_with_serial_types:1", "duplicated_foreign_keys:1", "intersected_foreign_keys:0", - "possible_object_name_overflow:1"); + "possible_object_name_overflow:1", + "tables_not_linked_to_others:0"); } } From 35bfced29a783f0f8638f4a3080138f964da4931 Mon Sep 17 00:00:00 2001 From: Ivan Vakhrushev Date: Fri, 1 Nov 2024 11:52:31 +0400 Subject: [PATCH 3/5] Add check "The type of the foreign key must match the type of column in the target table" --- ...ex-health-demo.java-conventions.gradle.kts | 2 +- .../db/changelog/db.changelog-master.yaml | 2 + .../resources/db/changelog/sql/order_item.sql | 3 + .../resources/db/changelog/sql/warehouse.sql | 12 ++++ .../health/demo/IndexesMaintenanceTest.java | 56 ++++++++++++------- .../health/demo/IndexesMaintenanceTest.java | 51 ++++++++++------- 6 files changed, 84 insertions(+), 42 deletions(-) create mode 100644 db-migrations/src/main/resources/db/changelog/sql/warehouse.sql 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 index b46bae4..8296628 100644 --- 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,7 +22,7 @@ configurations.configureEach { } dependencies { - implementation(platform("io.github.mfvanek:pg-index-health-bom:0.13.2-SNAPSHOT")) + implementation(platform("io.github.mfvanek:pg-index-health-bom:0.13.2")) implementation(platform("org.testcontainers:testcontainers-bom:1.20.2")) implementation("com.google.code.findbugs:jsr305:3.0.2") 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 index 5afafb7..f826475 100644 --- 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/order_item.sql b/db-migrations/src/main/resources/db/changelog/sql/order_item.sql index 504b979..8364358 100644 --- 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 new file mode 100644 index 0000000..09b1776 --- /dev/null +++ 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/pg-index-health-demo-without-spring/src/test/java/io/github/mfvanek/pg/index/health/demo/IndexesMaintenanceTest.java b/pg-index-health-demo-without-spring/src/test/java/io/github/mfvanek/pg/index/health/demo/IndexesMaintenanceTest.java index ea33507..c7906f0 100644 --- a/pg-index-health-demo-without-spring/src/test/java/io/github/mfvanek/pg/index/health/demo/IndexesMaintenanceTest.java +++ b/pg-index-health-demo-without-spring/src/test/java/io/github/mfvanek/pg/index/health/demo/IndexesMaintenanceTest.java @@ -14,6 +14,7 @@ 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; @@ -50,6 +51,7 @@ 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; @@ -96,7 +98,8 @@ class IndexesMaintenanceTest extends DatabaseAwareTestBase { new DuplicatedForeignKeysCheckOnHost(pgConnection), new IntersectedForeignKeysCheckOnHost(pgConnection), new PossibleObjectNameOverflowCheckOnHost(pgConnection), - new TablesNotLinkedToOthersCheckOnHost(pgConnection) + new TablesNotLinkedToOthersCheckOnHost(pgConnection), + new ForeignKeysWithUnmatchedColumnTypeCheckOnHost(pgConnection) ); } @@ -126,21 +129,23 @@ void completenessTest() { @Test void databaseStructureCheckForPublicSchema() { checks.forEach(check -> { - final List checkResult = check.check(); + final ListAssert checksAssert = assertThat(check.check()) + .as(check.getDiagnostic().name()); + switch (check.getDiagnostic()) { - case TABLES_WITHOUT_PRIMARY_KEY, TABLES_WITHOUT_DESCRIPTION, TABLES_NOT_LINKED_TO_OTHERS -> 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(); } }); } @@ -152,15 +157,17 @@ void databaseStructureCheckForDemoSchema() { .hasSize(Diagnostic.values().length - SKIPPED_CHECKS_COUNT); checks.forEach(check -> { - final List checkResult = check.check(demoSchema); + final ListAssert 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 @@ -168,7 +175,7 @@ void databaseStructureCheckForDemoSchema() { 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 @@ -180,56 +187,57 @@ 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( @@ -237,13 +245,19 @@ void databaseStructureCheckForDemoSchema() { ForeignKey.ofNotNullColumn(ORDER_ITEM_TABLE, "order_item_order_id_fkey", ORDER_ID_COLUMN)) ); - case POSSIBLE_OBJECT_NAME_OVERFLOW -> assertThat(checkResult) + 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)); - default -> assertThat(checkResult).isEmpty(); + 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/pg-index-health-spring-boot-demo/src/test/java/io/github/mfvanek/pg/index/health/demo/IndexesMaintenanceTest.java b/pg-index-health-spring-boot-demo/src/test/java/io/github/mfvanek/pg/index/health/demo/IndexesMaintenanceTest.java index d0c99bb..7039693 100644 --- a/pg-index-health-spring-boot-demo/src/test/java/io/github/mfvanek/pg/index/health/demo/IndexesMaintenanceTest.java +++ b/pg-index-health-spring-boot-demo/src/test/java/io/github/mfvanek/pg/index/health/demo/IndexesMaintenanceTest.java @@ -27,6 +27,7 @@ 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; import org.springframework.beans.factory.annotation.Autowired; @@ -62,21 +63,23 @@ void databaseStructureCheckForPublicSchema() { .hasSize(Diagnostic.values().length); checks.forEach(check -> { - final List checkResult = check.check(); + final ListAssert checksAssert = assertThat(check.check()) + .as(check.getDiagnostic().name()); + switch (check.getDiagnostic()) { - case TABLES_WITHOUT_PRIMARY_KEY, TABLES_WITHOUT_DESCRIPTION, TABLES_NOT_LINKED_TO_OTHERS -> 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(); } }); } @@ -91,15 +94,17 @@ void databaseStructureCheckForDemoSchema() { // Skip all runtime checks except SEQUENCE_OVERFLOW .filter(check -> check.getDiagnostic() == Diagnostic.SEQUENCE_OVERFLOW || check.isStatic()) .forEach(check -> { - final List checkResult = check.check(demoSchema); + final ListAssert 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 @@ -107,7 +112,7 @@ void databaseStructureCheckForDemoSchema() { 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 @@ -119,56 +124,57 @@ 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( @@ -176,13 +182,18 @@ void databaseStructureCheckForDemoSchema() { ForeignKey.ofNotNullColumn(ORDER_ITEM_TABLE, "order_item_order_id_fkey", ORDER_ID_COLUMN)) ); - case POSSIBLE_OBJECT_NAME_OVERFLOW -> assertThat(checkResult) + 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)); - default -> assertThat(checkResult).isEmpty(); + 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(); } }); } From db3c498b0b0eda38785168be76e699700870c282 Mon Sep 17 00:00:00 2001 From: Ivan Vakhrushev Date: Fri, 1 Nov 2024 12:02:36 +0400 Subject: [PATCH 4/5] Update PMD --- .../pg-index-health-demo.java-conventions.gradle.kts | 8 ++++---- config/pmd/pmd.xml | 4 ++-- .../pg/index/health/demo/IndexesMaintenanceTest.java | 2 +- 3 files changed, 7 insertions(+), 7 deletions(-) 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 index 8296628..3818edb 100644 --- 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 @@ -23,15 +23,15 @@ configurations.configureEach { dependencies { implementation(platform("io.github.mfvanek:pg-index-health-bom:0.13.2")) - implementation(platform("org.testcontainers:testcontainers-bom:1.20.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/config/pmd/pmd.xml b/config/pmd/pmd.xml index 98bc208..8c752af 100644 --- a/config/pmd/pmd.xml +++ b/config/pmd/pmd.xml @@ -6,8 +6,8 @@ PMD configuration - - + + diff --git a/pg-index-health-demo-without-spring/src/test/java/io/github/mfvanek/pg/index/health/demo/IndexesMaintenanceTest.java b/pg-index-health-demo-without-spring/src/test/java/io/github/mfvanek/pg/index/health/demo/IndexesMaintenanceTest.java index c7906f0..e66333c 100644 --- a/pg-index-health-demo-without-spring/src/test/java/io/github/mfvanek/pg/index/health/demo/IndexesMaintenanceTest.java +++ b/pg-index-health-demo-without-spring/src/test/java/io/github/mfvanek/pg/index/health/demo/IndexesMaintenanceTest.java @@ -125,7 +125,7 @@ void completenessTest() { .allMatch(CheckTypeAware::isStatic); } - @SuppressWarnings("PMD.JUnitTestsShouldIncludeAssert") + @SuppressWarnings("PMD.UnitTestShouldIncludeAssert") @Test void databaseStructureCheckForPublicSchema() { checks.forEach(check -> { From 7c232a84c50de1d86025f928a86ec40822be96b8 Mon Sep 17 00:00:00 2001 From: Ivan Vakhrushev Date: Fri, 1 Nov 2024 12:16:35 +0400 Subject: [PATCH 5/5] Fix tests --- .../pg/index/health/demo/utils/HealthDataCollectorTest.java | 5 +++-- .../pg/index/health/demo/utils/MigrationsGeneratorTest.java | 2 +- .../index/health/demo/controller/DbHealthControllerTest.java | 5 +++-- 3 files changed, 7 insertions(+), 5 deletions(-) diff --git a/pg-index-health-demo-without-spring/src/test/java/io/github/mfvanek/pg/index/health/demo/utils/HealthDataCollectorTest.java b/pg-index-health-demo-without-spring/src/test/java/io/github/mfvanek/pg/index/health/demo/utils/HealthDataCollectorTest.java index 911f43e..7d7adc6 100644 --- a/pg-index-health-demo-without-spring/src/test/java/io/github/mfvanek/pg/index/health/demo/utils/HealthDataCollectorTest.java +++ b/pg-index-health-demo-without-spring/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", @@ -43,7 +43,8 @@ void shouldCollectHealthData() { "db_indexes_health\tduplicated_foreign_keys\t1", "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\ttables_not_linked_to_others\t0", + "db_indexes_health\tforeign_keys_with_unmatched_column_type\t1"); final List healthData = HealthDataCollector.collectHealthData(getConnectionFactory(), getConnectionCredentials()); assertThat(healthData) .hasSameSizeAs(Diagnostic.values()) diff --git a/pg-index-health-demo-without-spring/src/test/java/io/github/mfvanek/pg/index/health/demo/utils/MigrationsGeneratorTest.java b/pg-index-health-demo-without-spring/src/test/java/io/github/mfvanek/pg/index/health/demo/utils/MigrationsGeneratorTest.java index 6ea546d..9a8650e 100644 --- a/pg-index-health-demo-without-spring/src/test/java/io/github/mfvanek/pg/index/health/demo/utils/MigrationsGeneratorTest.java +++ b/pg-index-health-demo-without-spring/src/test/java/io/github/mfvanek/pg/index/health/demo/utils/MigrationsGeneratorTest.java @@ -35,7 +35,7 @@ static void restoreSchema() { void generateMigrationsShouldWork() { final List foreignKeys = MigrationsGenerator.getForeignKeysNotCoveredWithIndex(getConnectionFactory(), getConnectionCredentials()); assertThat(foreignKeys) - .hasSize(4); + .hasSize(5); MigrationsGenerator.generateMigrations(getDataSource(), foreignKeys); assertThat(MigrationsGenerator.getForeignKeysNotCoveredWithIndex(getConnectionFactory(), getConnectionCredentials())) .isEmpty(); diff --git a/pg-index-health-spring-boot-demo/src/test/java/io/github/mfvanek/pg/index/health/demo/controller/DbHealthControllerTest.java b/pg-index-health-spring-boot-demo/src/test/java/io/github/mfvanek/pg/index/health/demo/controller/DbHealthControllerTest.java index 25fe4b7..47e07dd 100644 --- a/pg-index-health-spring-boot-demo/src/test/java/io/github/mfvanek/pg/index/health/demo/controller/DbHealthControllerTest.java +++ b/pg-index-health-spring-boot-demo/src/test/java/io/github/mfvanek/pg/index/health/demo/controller/DbHealthControllerTest.java @@ -35,7 +35,7 @@ void collectHealthDataShouldReturnOk() { "duplicated_indexes:1", "intersected_indexes:2", "unused_indexes:0", - "foreign_keys_without_index:4", + "foreign_keys_without_index:5", "tables_with_missing_indexes:0", "tables_without_primary_key:1", "indexes_with_null_values:1", @@ -54,6 +54,7 @@ void collectHealthDataShouldReturnOk() { "duplicated_foreign_keys:1", "intersected_foreign_keys:0", "possible_object_name_overflow:1", - "tables_not_linked_to_others:0"); + "tables_not_linked_to_others:0", + "foreign_keys_with_unmatched_column_type:1"); } }