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 753e8f6d3a914b5c25328bacaff9010b7b195059 Author: Benoit TELLIER <[email protected]> AuthorDate: Fri Nov 10 19:46:40 2023 +0100 JAMES-2586 Small codestyle refactorings --- .../backends/postgres/PostgresConfiguration.java | 62 +++++++++++----------- .../james/backends/postgres/PostgresTable.java | 24 ++++----- .../backends/postgres/PostgresTableManager.java | 51 ++++++++++-------- .../postgres/PostgresConfigurationTest.java | 6 +-- .../james/backends/postgres/PostgresExtension.java | 4 +- .../backends/postgres/PostgresExtensionTest.java | 4 +- .../postgres/PostgresTableManagerTest.java | 24 ++++----- .../postgres/user/PostgresSubscriptionModule.java | 2 +- 8 files changed, 90 insertions(+), 87 deletions(-) diff --git a/backends-common/postgres/src/main/java/org/apache/james/backends/postgres/PostgresConfiguration.java b/backends-common/postgres/src/main/java/org/apache/james/backends/postgres/PostgresConfiguration.java index 73dbf36211..7ffeb8be40 100644 --- a/backends-common/postgres/src/main/java/org/apache/james/backends/postgres/PostgresConfiguration.java +++ b/backends-common/postgres/src/main/java/org/apache/james/backends/postgres/PostgresConfiguration.java @@ -61,7 +61,7 @@ public class PostgresConfiguration { private Optional<String> url = Optional.empty(); private Optional<String> databaseName = Optional.empty(); private Optional<String> databaseSchema = Optional.empty(); - private Optional<Boolean> rlsEnabled = Optional.empty(); + private Optional<Boolean> rowLevelSecurityEnabled = Optional.empty(); public Builder url(String url) { this.url = Optional.of(url); @@ -88,13 +88,13 @@ public class PostgresConfiguration { return this; } - public Builder rlsEnabled(boolean rlsEnabled) { - this.rlsEnabled = Optional.of(rlsEnabled); + public Builder rowLevelSecurityEnabled(boolean rlsEnabled) { + this.rowLevelSecurityEnabled = Optional.of(rlsEnabled); return this; } - public Builder rlsEnabled() { - this.rlsEnabled = Optional.of(true); + public Builder rowLevelSecurityEnabled() { + this.rowLevelSecurityEnabled = Optional.of(true); return this; } @@ -106,7 +106,7 @@ public class PostgresConfiguration { parseCredential(postgresURI), databaseName.orElse(DATABASE_NAME_DEFAULT_VALUE), databaseSchema.orElse(DATABASE_SCHEMA_DEFAULT_VALUE), - rlsEnabled.orElse(false)); + rowLevelSecurityEnabled.orElse(false)); } private Credential parseCredential(URI postgresURI) { @@ -140,7 +140,7 @@ public class PostgresConfiguration { .url(propertiesConfiguration.getString(URL, null)) .databaseName(Optional.ofNullable(propertiesConfiguration.getString(DATABASE_NAME))) .databaseSchema(Optional.ofNullable(propertiesConfiguration.getString(DATABASE_SCHEMA))) - .rlsEnabled(propertiesConfiguration.getBoolean(RLS_ENABLED, false)) + .rowLevelSecurityEnabled(propertiesConfiguration.getBoolean(RLS_ENABLED, false)) .build(); } @@ -148,33 +148,14 @@ public class PostgresConfiguration { private final Credential credential; private final String databaseName; private final String databaseSchema; - private final boolean rlsEnabled; + private final boolean rowLevelSecurityEnabled; - private PostgresConfiguration(URI uri, Credential credential, String databaseName, String databaseSchema, boolean rlsEnabled) { + private PostgresConfiguration(URI uri, Credential credential, String databaseName, String databaseSchema, boolean rowLevelSecurityEnabled) { this.uri = uri; this.credential = credential; this.databaseName = databaseName; this.databaseSchema = databaseSchema; - this.rlsEnabled = rlsEnabled; - } - - @Override - public final boolean equals(Object o) { - if (o instanceof PostgresConfiguration) { - PostgresConfiguration that = (PostgresConfiguration) o; - - return Objects.equals(this.rlsEnabled, that.rlsEnabled) - && Objects.equals(this.uri, that.uri) - && Objects.equals(this.credential, that.credential) - && Objects.equals(this.databaseName, that.databaseName) - && Objects.equals(this.databaseSchema, that.databaseSchema); - } - return false; - } - - @Override - public final int hashCode() { - return Objects.hash(uri, credential, databaseName, databaseSchema, rlsEnabled); + this.rowLevelSecurityEnabled = rowLevelSecurityEnabled; } public URI getUri() { @@ -193,7 +174,26 @@ public class PostgresConfiguration { return databaseSchema; } - public boolean rlsEnabled() { - return rlsEnabled; + public boolean rowLevelSecurityEnabled() { + return rowLevelSecurityEnabled; + } + + @Override + public final boolean equals(Object o) { + if (o instanceof PostgresConfiguration) { + PostgresConfiguration that = (PostgresConfiguration) o; + + return Objects.equals(this.rowLevelSecurityEnabled, that.rowLevelSecurityEnabled) + && Objects.equals(this.uri, that.uri) + && Objects.equals(this.credential, that.credential) + && Objects.equals(this.databaseName, that.databaseName) + && Objects.equals(this.databaseSchema, that.databaseSchema); + } + return false; + } + + @Override + public final int hashCode() { + return Objects.hash(uri, credential, databaseName, databaseSchema, rowLevelSecurityEnabled); } } 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 1956d3c5e8..933a7810df 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 @@ -27,13 +27,11 @@ import org.jooq.DSLContext; import com.google.common.base.Preconditions; public class PostgresTable { - @FunctionalInterface public interface RequireCreateTableStep { RequireRowLevelSecurity createTableStep(CreateTableFunction createTableFunction); } - @FunctionalInterface public interface CreateTableFunction { DDLQuery createTable(DSLContext dsl, String tableName); @@ -41,30 +39,30 @@ public class PostgresTable { @FunctionalInterface public interface RequireRowLevelSecurity { - PostgresTable enableRowLevelSecurity(boolean enableRowLevelSecurity); + PostgresTable supportsRowLevelSecurity(boolean rowLevelSecurityEnabled); - default PostgresTable noRLS() { - return enableRowLevelSecurity(false); + default PostgresTable disableRowLevelSecurity() { + return supportsRowLevelSecurity(false); } - default PostgresTable enableRowLevelSecurity() { - return enableRowLevelSecurity(true); + default PostgresTable supportsRowLevelSecurity() { + return supportsRowLevelSecurity(true); } } public static RequireCreateTableStep name(String tableName) { Preconditions.checkNotNull(tableName); - return createTableFunction -> enableRowLevelSecurity -> new PostgresTable(tableName, enableRowLevelSecurity, dsl -> createTableFunction.createTable(dsl, tableName)); + return createTableFunction -> supportsRowLevelSecurity -> new PostgresTable(tableName, supportsRowLevelSecurity, dsl -> createTableFunction.createTable(dsl, tableName)); } private final String name; - private final boolean enableRowLevelSecurity; + private final boolean supportsRowLevelSecurity; private final Function<DSLContext, DDLQuery> createTableStepFunction; - private PostgresTable(String name, boolean enableRowLevelSecurity, Function<DSLContext, DDLQuery> createTableStepFunction) { + private PostgresTable(String name, boolean supportsRowLevelSecurity, Function<DSLContext, DDLQuery> createTableStepFunction) { this.name = name; - this.enableRowLevelSecurity = enableRowLevelSecurity; + this.supportsRowLevelSecurity = supportsRowLevelSecurity; this.createTableStepFunction = createTableStepFunction; } @@ -77,7 +75,7 @@ public class PostgresTable { return createTableStepFunction; } - public boolean isEnableRowLevelSecurity() { - return enableRowLevelSecurity; + public boolean supportsRowLevelSecurity() { + return supportsRowLevelSecurity; } } 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 78eb5170f6..eaa4aa7994 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 @@ -40,7 +40,7 @@ public class PostgresTableManager implements Startable { private static final Logger LOGGER = LoggerFactory.getLogger(PostgresTableManager.class); private final PostgresExecutor postgresExecutor; private final PostgresModule module; - private final boolean rlsEnabled; + private final boolean rowLevelSecurityEnabled; @Inject public PostgresTableManager(JamesPostgresConnectionFactory postgresConnectionFactory, @@ -48,34 +48,36 @@ public class PostgresTableManager implements Startable { PostgresConfiguration postgresConfiguration) { this.postgresExecutor = new PostgresExecutor(postgresConnectionFactory.getConnection(Optional.empty())); this.module = module; - this.rlsEnabled = postgresConfiguration.rlsEnabled(); + this.rowLevelSecurityEnabled = postgresConfiguration.rowLevelSecurityEnabled(); } @VisibleForTesting - public PostgresTableManager(PostgresExecutor postgresExecutor, PostgresModule module, boolean rlsEnabled) { + public PostgresTableManager(PostgresExecutor postgresExecutor, PostgresModule module, boolean rowLevelSecurityEnabled) { this.postgresExecutor = postgresExecutor; this.module = module; - this.rlsEnabled = rlsEnabled; + this.rowLevelSecurityEnabled = rowLevelSecurityEnabled; } public Mono<Void> initializeTables() { return postgresExecutor.dslContext() .flatMap(dsl -> Flux.fromIterable(module.tables()) .flatMap(table -> Mono.from(table.getCreateTableStepFunction().apply(dsl)) - .then(alterTableEnableRLSIfNeed(table)) + .then(alterTableIfNeeded(table)) .doOnSuccess(any -> LOGGER.info("Table {} created", table.getName())) - .onErrorResume(DataAccessException.class, exception -> { - if (exception.getMessage().contains(String.format("\"%s\" already exists", table.getName()))) { - return Mono.empty(); - } - return Mono.error(exception); - }) - .doOnError(e -> LOGGER.error("Error while creating table {}", table.getName(), e))) + .onErrorResume(exception -> handleTableCreationException(table, exception))) .then()); } - private Mono<Void> alterTableEnableRLSIfNeed(PostgresTable table) { - if (rlsEnabled && table.isEnableRowLevelSecurity()) { + private Mono<Void> handleTableCreationException(PostgresTable table, Throwable e) { + if (e instanceof DataAccessException && e.getMessage().contains(String.format("\"%s\" already exists", table.getName()))) { + return Mono.empty(); + } + LOGGER.error("Error while creating table {}", table.getName(), e); + return Mono.error(e); + } + + private Mono<Void> alterTableIfNeeded(PostgresTable table) { + if (rowLevelSecurityEnabled && table.supportsRowLevelSecurity()) { return alterTableEnableRLS(table); } return Mono.empty(); @@ -83,12 +85,13 @@ public class PostgresTableManager implements Startable { public Mono<Void> alterTableEnableRLS(PostgresTable table) { return postgresExecutor.connection() - .flatMapMany(con -> con.createStatement(getAlterRLSStatement(table.getName())).execute()) + .flatMapMany(connection -> connection.createStatement(rowLevelSecurityAlterStatement(table.getName())) + .execute()) .flatMap(Result::getRowsUpdated) .then(); } - private String getAlterRLSStatement(String tableName) { + private String rowLevelSecurityAlterStatement(String tableName) { return "SET app.current_domain = ''; ALTER TABLE " + tableName + " ADD DOMAIN varchar(255) not null DEFAULT current_setting('app.current_domain')::text;" + "ALTER TABLE " + tableName + " ENABLE ROW LEVEL SECURITY; " + "CREATE POLICY DOMAIN_" + tableName + "_POLICY ON " + tableName + " USING (DOMAIN = current_setting('app.current_domain')::text);"; @@ -108,13 +111,15 @@ public class PostgresTableManager implements Startable { .flatMap(dsl -> Flux.fromIterable(module.tableIndexes()) .concatMap(index -> Mono.from(index.getCreateIndexStepFunction().apply(dsl)) .doOnSuccess(any -> LOGGER.info("Index {} created", index.getName())) - .onErrorResume(DataAccessException.class, exception -> { - if (exception.getMessage().contains(String.format("\"%s\" already exists", index.getName()))) { - return Mono.empty(); - } - return Mono.error(exception); - }) - .doOnError(e -> LOGGER.error("Error while creating index {}", index.getName(), e))) + .onErrorResume(e -> handleIndexCreationException(index, e))) .then()); } + + private Mono<? extends Integer> handleIndexCreationException(PostgresIndex index, Throwable e) { + if (e instanceof DataAccessException && e.getMessage().contains(String.format("\"%s\" already exists", index.getName()))) { + return Mono.empty(); + } + LOGGER.error("Error while creating index {}", index.getName(), e); + return Mono.error(e); + } } diff --git a/backends-common/postgres/src/test/java/org/apache/james/backends/postgres/PostgresConfigurationTest.java b/backends-common/postgres/src/test/java/org/apache/james/backends/postgres/PostgresConfigurationTest.java index b324ec527a..248eb0dd66 100644 --- a/backends-common/postgres/src/test/java/org/apache/james/backends/postgres/PostgresConfigurationTest.java +++ b/backends-common/postgres/src/test/java/org/apache/james/backends/postgres/PostgresConfigurationTest.java @@ -71,7 +71,7 @@ class PostgresConfigurationTest { .url("postgresql://username:password@postgreshost:5672") .build(); - assertThat(configuration.rlsEnabled()).isFalse(); + assertThat(configuration.rowLevelSecurityEnabled()).isFalse(); } @Test @@ -96,13 +96,13 @@ class PostgresConfigurationTest { void shouldReturnCorrespondingProperties() { PostgresConfiguration configuration = PostgresConfiguration.builder() .url("postgresql://username:password@postgreshost:5672") - .rlsEnabled() + .rowLevelSecurityEnabled() .databaseName("databaseName") .databaseSchema("databaseSchema") .build(); SoftAssertions.assertSoftly(softly -> { - softly.assertThat(configuration.rlsEnabled()).isEqualTo(true); + softly.assertThat(configuration.rowLevelSecurityEnabled()).isEqualTo(true); softly.assertThat(configuration.getDatabaseName()).isEqualTo("databaseName"); softly.assertThat(configuration.getDatabaseSchema()).isEqualTo("databaseSchema"); }); 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 682fc49696..a1ae9b9aba 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 @@ -80,7 +80,7 @@ public class PostgresExtension implements GuiceModuleTestExtension { .toString()) .databaseName(PostgresFixture.Database.DB_NAME) .databaseSchema(PostgresFixture.Database.SCHEMA) - .rlsEnabled(rlsEnabled) + .rowLevelSecurityEnabled(rlsEnabled) .build(); connectionFactory = new PostgresqlConnectionFactory(PostgresqlConnectionConfiguration.builder() @@ -149,7 +149,7 @@ public class PostgresExtension implements GuiceModuleTestExtension { } private void initTablesAndIndexes() { - PostgresTableManager postgresTableManager = new PostgresTableManager(postgresExecutor, postgresModule, postgresConfiguration.rlsEnabled()); + PostgresTableManager postgresTableManager = new PostgresTableManager(postgresExecutor, postgresModule, postgresConfiguration.rowLevelSecurityEnabled()); postgresTableManager.initializeTables().block(); postgresTableManager.initializeTableIndexes().block(); } diff --git a/backends-common/postgres/src/test/java/org/apache/james/backends/postgres/PostgresExtensionTest.java b/backends-common/postgres/src/test/java/org/apache/james/backends/postgres/PostgresExtensionTest.java index 406ba4b6bc..ca3a641ead 100644 --- a/backends-common/postgres/src/test/java/org/apache/james/backends/postgres/PostgresExtensionTest.java +++ b/backends-common/postgres/src/test/java/org/apache/james/backends/postgres/PostgresExtensionTest.java @@ -38,7 +38,7 @@ class PostgresExtensionTest { .column("column1", SQLDataType.UUID.notNull()) .column("column2", SQLDataType.INTEGER) .column("column3", SQLDataType.VARCHAR(255).notNull())) - .noRLS(); + .disableRowLevelSecurity(); static PostgresIndex INDEX_1 = PostgresIndex.name("index1") .createIndexStep((dslContext, indexName) -> dslContext.createIndex(indexName) @@ -47,7 +47,7 @@ class PostgresExtensionTest { static PostgresTable TABLE_2 = PostgresTable.name("table2") .createTableStep((dslContext, tableName) -> dslContext.createTable(tableName) .column("column1", SQLDataType.INTEGER)) - .noRLS(); + .disableRowLevelSecurity(); static PostgresIndex INDEX_2 = PostgresIndex.name("index2") .createIndexStep((dslContext, indexName) -> dslContext.createIndex(indexName) 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 c7d98cb915..ac5d73c2d7 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 @@ -53,7 +53,7 @@ class PostgresTableManagerTest { .column("colum1", SQLDataType.UUID.notNull()) .column("colum2", SQLDataType.INTEGER) .column("colum3", SQLDataType.VARCHAR(255).notNull())) - .noRLS(); + .disableRowLevelSecurity(); PostgresModule module = PostgresModule.table(table); @@ -75,12 +75,12 @@ class PostgresTableManagerTest { PostgresTable table1 = PostgresTable.name(tableName1) .createTableStep((dsl, tbn) -> dsl.createTable(tbn) - .column("columA", SQLDataType.UUID.notNull())).noRLS(); + .column("columA", SQLDataType.UUID.notNull())).disableRowLevelSecurity(); String tableName2 = "tableName2"; PostgresTable table2 = PostgresTable.name(tableName2) .createTableStep((dsl, tbn) -> dsl.createTable(tbn) - .column("columB", SQLDataType.INTEGER)).noRLS(); + .column("columB", SQLDataType.INTEGER)).disableRowLevelSecurity(); PostgresTableManager testee = tableManagerFactory.apply(PostgresModule.table(table1, table2)); @@ -101,7 +101,7 @@ class PostgresTableManagerTest { PostgresTable table1 = PostgresTable.name(tableName1) .createTableStep((dsl, tbn) -> dsl.createTable(tbn) - .column("columA", SQLDataType.UUID.notNull())).noRLS(); + .column("columA", SQLDataType.UUID.notNull())).disableRowLevelSecurity(); PostgresTableManager testee = tableManagerFactory.apply(PostgresModule.table(table1)); @@ -117,7 +117,7 @@ class PostgresTableManagerTest { String tableName1 = "tableName1"; PostgresTable table1 = PostgresTable.name(tableName1) .createTableStep((dsl, tbn) -> dsl.createTable(tbn) - .column("columA", SQLDataType.UUID.notNull())).noRLS(); + .column("columA", SQLDataType.UUID.notNull())).disableRowLevelSecurity(); tableManagerFactory.apply(PostgresModule.table(table1)) .initializeTables() @@ -125,7 +125,7 @@ class PostgresTableManagerTest { PostgresTable table1Changed = PostgresTable.name(tableName1) .createTableStep((dsl, tbn) -> dsl.createTable(tbn) - .column("columB", SQLDataType.INTEGER)).noRLS(); + .column("columB", SQLDataType.INTEGER)).disableRowLevelSecurity(); tableManagerFactory.apply(PostgresModule.table(table1Changed)) .initializeTables() @@ -145,7 +145,7 @@ class PostgresTableManagerTest { .column("colum1", SQLDataType.UUID.notNull()) .column("colum2", SQLDataType.INTEGER) .column("colum3", SQLDataType.VARCHAR(255).notNull())) - .noRLS(); + .disableRowLevelSecurity(); String indexName = "idx_test_1"; PostgresIndex index = PostgresIndex.name(indexName) @@ -178,7 +178,7 @@ class PostgresTableManagerTest { .column("colum1", SQLDataType.UUID.notNull()) .column("colum2", SQLDataType.INTEGER) .column("colum3", SQLDataType.VARCHAR(255).notNull())) - .noRLS(); + .disableRowLevelSecurity(); String indexName1 = "idx_test_1"; PostgresIndex index1 = PostgresIndex.name(indexName1) @@ -216,7 +216,7 @@ class PostgresTableManagerTest { .column("colum1", SQLDataType.UUID.notNull()) .column("colum2", SQLDataType.INTEGER) .column("colum3", SQLDataType.VARCHAR(255).notNull())) - .noRLS(); + .disableRowLevelSecurity(); String indexName = "idx_test_1"; PostgresIndex index = PostgresIndex.name(indexName) @@ -244,7 +244,7 @@ class PostgresTableManagerTest { String tableName1 = "tbn1"; PostgresTable table1 = PostgresTable.name(tableName1) .createTableStep((dsl, tbn) -> dsl.createTable(tbn) - .column("column1", SQLDataType.INTEGER.notNull())).noRLS(); + .column("column1", SQLDataType.INTEGER.notNull())).disableRowLevelSecurity(); PostgresTableManager testee = tableManagerFactory.apply(PostgresModule.table(table1)); testee.initializeTables() @@ -286,7 +286,7 @@ class PostgresTableManagerTest { .createTableStep((dsl, tbn) -> dsl.createTable(tbn) .column("clm1", SQLDataType.UUID.notNull()) .column("clm2", SQLDataType.VARCHAR(255).notNull())) - .enableRowLevelSecurity(); + .supportsRowLevelSecurity(); PostgresModule module = PostgresModule.table(table); @@ -326,7 +326,7 @@ class PostgresTableManagerTest { .createTableStep((dsl, tbn) -> dsl.createTable(tbn) .column("clm1", SQLDataType.UUID.notNull()) .column("clm2", SQLDataType.VARCHAR(255).notNull())) - .enableRowLevelSecurity(); + .supportsRowLevelSecurity(); PostgresModule module = PostgresModule.table(table); boolean disabledRLS = false; diff --git a/mailbox/postgres/src/main/java/org/apache/james/mailbox/postgres/user/PostgresSubscriptionModule.java b/mailbox/postgres/src/main/java/org/apache/james/mailbox/postgres/user/PostgresSubscriptionModule.java index 3f07843eab..68c8eca1d0 100644 --- a/mailbox/postgres/src/main/java/org/apache/james/mailbox/postgres/user/PostgresSubscriptionModule.java +++ b/mailbox/postgres/src/main/java/org/apache/james/mailbox/postgres/user/PostgresSubscriptionModule.java @@ -43,7 +43,7 @@ public interface PostgresSubscriptionModule { .column(MAILBOX) .column(USER) .constraint(DSL.unique(MAILBOX, USER)))) - .enableRowLevelSecurity(); + .supportsRowLevelSecurity(); PostgresIndex INDEX = PostgresIndex.name("subscription_user_index") .createIndexStep((dsl, indexName) -> dsl.createIndex(indexName) .on(TABLE_NAME, USER)); --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
