This is an automated email from the ASF dual-hosted git repository. rcordier pushed a commit to branch postgresql in repository https://gitbox.apache.org/repos/asf/james-project.git
commit 7622770cb6322bb77761893290da244c88236b9b Author: Tung Tran <[email protected]> AuthorDate: Tue Mar 19 16:14:34 2024 +0700 JAMES-2586 [REFACTORING] - PostgresTableManager - fix incorrect log - Do not print log "Table {} created", "Index {} created" when it already exists and James does nothing --- .../james/backends/postgres/PostgresIndex.java | 3 +- .../james/backends/postgres/PostgresTable.java | 3 +- .../backends/postgres/PostgresTableManager.java | 59 ++++++++++++++++++---- .../james/backends/postgres/PostgresExtension.java | 18 ++----- .../postgres/PostgresTableManagerTest.java | 10 ++-- .../vacation/postgres/PostgresVacationModule.java | 4 +- 6 files changed, 64 insertions(+), 33 deletions(-) diff --git a/backends-common/postgres/src/main/java/org/apache/james/backends/postgres/PostgresIndex.java b/backends-common/postgres/src/main/java/org/apache/james/backends/postgres/PostgresIndex.java index db41be4e35..c1a41f2947 100644 --- a/backends-common/postgres/src/main/java/org/apache/james/backends/postgres/PostgresIndex.java +++ b/backends-common/postgres/src/main/java/org/apache/james/backends/postgres/PostgresIndex.java @@ -40,8 +40,9 @@ public class PostgresIndex { public static RequireCreateIndexStep name(String indexName) { Preconditions.checkNotNull(indexName); + String strategyIndexName = indexName.toLowerCase(); - return createIndexFunction -> new PostgresIndex(indexName, dsl -> createIndexFunction.createIndex(dsl, indexName)); + return createIndexFunction -> new PostgresIndex(strategyIndexName, dsl -> createIndexFunction.createIndex(dsl, strategyIndexName)); } private final String name; diff --git a/backends-common/postgres/src/main/java/org/apache/james/backends/postgres/PostgresTable.java b/backends-common/postgres/src/main/java/org/apache/james/backends/postgres/PostgresTable.java index 517ff411bb..db37fcdf9d 100644 --- a/backends-common/postgres/src/main/java/org/apache/james/backends/postgres/PostgresTable.java +++ b/backends-common/postgres/src/main/java/org/apache/james/backends/postgres/PostgresTable.java @@ -80,8 +80,9 @@ public class PostgresTable { public static RequireCreateTableStep name(String tableName) { Preconditions.checkNotNull(tableName); + String strategyName = tableName.toLowerCase(); - return createTableFunction -> supportsRowLevelSecurity -> new FinalStage(tableName, supportsRowLevelSecurity, dsl -> createTableFunction.createTable(dsl, tableName)); + return createTableFunction -> supportsRowLevelSecurity -> new FinalStage(strategyName, supportsRowLevelSecurity, dsl -> createTableFunction.createTable(dsl, strategyName)); } private final String name; diff --git a/backends-common/postgres/src/main/java/org/apache/james/backends/postgres/PostgresTableManager.java b/backends-common/postgres/src/main/java/org/apache/james/backends/postgres/PostgresTableManager.java index 84e2bc7fe6..313bc8bc72 100644 --- a/backends-common/postgres/src/main/java/org/apache/james/backends/postgres/PostgresTableManager.java +++ b/backends-common/postgres/src/main/java/org/apache/james/backends/postgres/PostgresTableManager.java @@ -19,11 +19,15 @@ package org.apache.james.backends.postgres; +import java.util.List; + import javax.inject.Inject; import org.apache.james.backends.postgres.utils.PostgresExecutor; import org.apache.james.lifecycle.api.Startable; +import org.jooq.DSLContext; import org.jooq.exception.DataAccessException; +import org.jooq.impl.DSL; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -73,12 +77,28 @@ public class PostgresTableManager implements Startable { public Mono<Void> initializeTables() { return postgresExecutor.dslContext() - .flatMap(dsl -> Flux.fromIterable(module.tables()) - .flatMap(table -> Mono.from(table.getCreateTableStepFunction().apply(dsl)) - .then(alterTableIfNeeded(table)) - .doOnSuccess(any -> LOGGER.info("Table {} created", table.getName())) - .onErrorResume(exception -> handleTableCreationException(table, exception))) - .then()); + .flatMapMany(dsl -> listExistTables() + .flatMapMany(existTables -> Flux.fromIterable(module.tables()) + .filter(table -> !existTables.contains(table.getName())) + .flatMap(table -> createAndAlterTable(table, dsl)))) + .then(); + } + + private Mono<Void> createAndAlterTable(PostgresTable table, DSLContext dsl) { + return Mono.from(table.getCreateTableStepFunction().apply(dsl)) + .then(alterTableIfNeeded(table)) + .doOnSuccess(any -> LOGGER.info("Table {} created", table.getName())) + .onErrorResume(exception -> handleTableCreationException(table, exception)); + } + + public Mono<List<String>> listExistTables() { + return postgresExecutor.dslContext() + .flatMapMany(d -> Flux.from(d.select(DSL.field("tablename")) + .from("pg_tables") + .where(DSL.field("schemaname") + .eq(DSL.currentSchema())))) + .map(r -> r.get(0, String.class)) + .collectList(); } private Mono<Void> handleTableCreationException(PostgresTable table, Throwable e) { @@ -148,11 +168,28 @@ public class PostgresTableManager implements Startable { public Mono<Void> initializeTableIndexes() { return postgresExecutor.dslContext() - .flatMap(dsl -> Flux.fromIterable(module.tableIndexes()) - .concatMap(index -> Mono.from(index.getCreateIndexStepFunction().apply(dsl)) - .doOnSuccess(any -> LOGGER.info("Index {} created", index.getName())) - .onErrorResume(e -> handleIndexCreationException(index, e))) - .then()); + .flatMapMany(dsl -> listExistIndexes() + .flatMapMany(existIndexes -> Flux.fromIterable(module.tableIndexes()) + .filter(index -> !existIndexes.contains(index.getName())) + .flatMap(index -> createTableIndex(index, dsl)))) + .then(); + } + + public Mono<List<String>> listExistIndexes() { + return postgresExecutor.dslContext() + .flatMapMany(dsl -> Flux.from(dsl.select(DSL.field("indexname")) + .from("pg_indexes") + .where(DSL.field("schemaname") + .eq(DSL.currentSchema())))) + .map(r -> r.get(0, String.class)) + .collectList(); + } + + private Mono<Void> createTableIndex(PostgresIndex index, DSLContext dsl) { + return Mono.from(index.getCreateIndexStepFunction().apply(dsl)) + .doOnSuccess(any -> LOGGER.info("Index {} created", index.getName())) + .onErrorResume(e -> handleIndexCreationException(index, e)) + .then(); } private Mono<? extends Integer> handleIndexCreationException(PostgresIndex index, Throwable e) { diff --git a/backends-common/postgres/src/test/java/org/apache/james/backends/postgres/PostgresExtension.java b/backends-common/postgres/src/test/java/org/apache/james/backends/postgres/PostgresExtension.java index e21f846a1b..1f6f0a200c 100644 --- a/backends-common/postgres/src/test/java/org/apache/james/backends/postgres/PostgresExtension.java +++ b/backends-common/postgres/src/test/java/org/apache/james/backends/postgres/PostgresExtension.java @@ -23,9 +23,7 @@ import static org.apache.james.backends.postgres.PostgresFixture.Database.DEFAUL import static org.apache.james.backends.postgres.PostgresFixture.Database.ROW_LEVEL_SECURITY_DATABASE; import java.io.IOException; -import java.net.URISyntaxException; import java.util.List; -import java.util.Optional; import java.util.stream.Collectors; import org.apache.james.GuiceModuleTestExtension; @@ -69,6 +67,7 @@ public class PostgresExtension implements GuiceModuleTestExtension { private PostgresExecutor nonRLSPostgresExecutor; private PostgresqlConnectionFactory connectionFactory; private PostgresExecutor.Factory executorFactory; + private PostgresTableManager postgresTableManager; public void pause() { PG_CONTAINER.getDockerClient().pauseContainerCmd(PG_CONTAINER.getContainerId()) @@ -159,6 +158,8 @@ public class PostgresExtension implements GuiceModuleTestExtension { } else { nonRLSPostgresExecutor = postgresExecutor; } + + this.postgresTableManager = new PostgresTableManager(postgresExecutor, postgresModule, postgresConfiguration); } @Override @@ -225,13 +226,13 @@ public class PostgresExtension implements GuiceModuleTestExtension { } private void initTablesAndIndexes() { - PostgresTableManager postgresTableManager = new PostgresTableManager(postgresExecutor, postgresModule, postgresConfiguration.rowLevelSecurityEnabled()); postgresTableManager.initializeTables().block(); postgresTableManager.initializeTableIndexes().block(); } private void resetSchema() { - dropTables(listAllTables()); + List<String> tables = postgresTableManager.listExistTables().block(); + dropTables(tables); } private void dropTables(List<String> tables) { @@ -246,15 +247,6 @@ public class PostgresExtension implements GuiceModuleTestExtension { .block(); } - private List<String> listAllTables() { - return postgresExecutor.connection() - .flatMapMany(connection -> connection.createStatement(String.format("SELECT tablename FROM pg_tables WHERE schemaname = '%s'", selectedDatabase.schema())) - .execute()) - .flatMap(result -> result.map((row, rowMetadata) -> row.get(0, String.class))) - .collectList() - .block(); - } - private void dropAllConnections() { postgresExecutor.connection() .flatMapMany(connection -> connection.createStatement(String.format("SELECT pg_terminate_backend(pid) " + diff --git a/backends-common/postgres/src/test/java/org/apache/james/backends/postgres/PostgresTableManagerTest.java b/backends-common/postgres/src/test/java/org/apache/james/backends/postgres/PostgresTableManagerTest.java index 0068fd1566..e1414906dc 100644 --- a/backends-common/postgres/src/test/java/org/apache/james/backends/postgres/PostgresTableManagerTest.java +++ b/backends-common/postgres/src/test/java/org/apache/james/backends/postgres/PostgresTableManagerTest.java @@ -45,7 +45,7 @@ class PostgresTableManagerTest { @Test void initializeTableShouldSuccessWhenModuleHasSingleTable() { - String tableName = "tableName1"; + String tableName = "tablename1"; PostgresTable table = PostgresTable.name(tableName) .createTableStep((dsl, tbn) -> dsl.createTable(tbn) @@ -71,14 +71,14 @@ class PostgresTableManagerTest { @Test void initializeTableShouldSuccessWhenModuleHasMultiTables() { - String tableName1 = "tableName1"; + String tableName1 = "tablename1"; PostgresTable table1 = PostgresTable.name(tableName1) .createTableStep((dsl, tbn) -> dsl.createTable(tbn) .column("columA", SQLDataType.UUID.notNull())).disableRowLevelSecurity() .build(); - String tableName2 = "tableName2"; + String tableName2 = "tablename2"; PostgresTable table2 = PostgresTable.name(tableName2) .createTableStep((dsl, tbn) -> dsl.createTable(tbn) .column("columB", SQLDataType.INTEGER)).disableRowLevelSecurity() @@ -99,7 +99,7 @@ class PostgresTableManagerTest { @Test void initializeTableShouldNotThrowWhenTableExists() { - String tableName1 = "tableName1"; + String tableName1 = "tablename1"; PostgresTable table1 = PostgresTable.name(tableName1) .createTableStep((dsl, tbn) -> dsl.createTable(tbn) @@ -117,7 +117,7 @@ class PostgresTableManagerTest { @Test void initializeTableShouldNotChangeTableStructureOfExistTable() { - String tableName1 = "tableName1"; + String tableName1 = "tablename1"; PostgresTable table1 = PostgresTable.name(tableName1) .createTableStep((dsl, tbn) -> dsl.createTable(tbn) .column("columA", SQLDataType.UUID.notNull())).disableRowLevelSecurity() diff --git a/server/data/data-postgres/src/main/java/org/apache/james/vacation/postgres/PostgresVacationModule.java b/server/data/data-postgres/src/main/java/org/apache/james/vacation/postgres/PostgresVacationModule.java index f306651822..14fb05df0a 100644 --- a/server/data/data-postgres/src/main/java/org/apache/james/vacation/postgres/PostgresVacationModule.java +++ b/server/data/data-postgres/src/main/java/org/apache/james/vacation/postgres/PostgresVacationModule.java @@ -74,11 +74,11 @@ public interface PostgresVacationModule { .supportsRowLevelSecurity() .build(); - PostgresIndex ACCOUNT_ID_INDEX = PostgresIndex.name("vacation_notification_registry_accountId_index") + PostgresIndex ACCOUNT_ID_INDEX = PostgresIndex.name("vacation_notification_registry_accountid_index") .createIndexStep((dsl, indexName) -> dsl.createIndexIfNotExists(indexName) .on(TABLE_NAME, ACCOUNT_ID)); - PostgresIndex FULL_COMPOSITE_INDEX = PostgresIndex.name("vacation_notification_registry_accountId_recipientId_expiryDate_index") + PostgresIndex FULL_COMPOSITE_INDEX = PostgresIndex.name("vnr_accountid_recipientid_expirydate_index") .createIndexStep((dsl, indexName) -> dsl.createIndexIfNotExists(indexName) .on(TABLE_NAME, ACCOUNT_ID, RECIPIENT_ID, EXPIRY_DATE)); } --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
