You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@james.apache.org by "vttranlina (via GitHub)" <gi...@apache.org> on 2023/10/31 12:05:47 UTC
[PR] JAMES-2586 - Implement PostgresTableManager [james-project]
vttranlina opened a new pull request, #1769:
URL: https://github.com/apache/james-project/pull/1769
jira Jira https://issues.apache.org/jira/browse/JAMES-2586
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
Re: [PR] JAMES-2586 - Implement PostgresTableManager [james-project]
Posted by "vttranlina (via GitHub)" <gi...@apache.org>.
vttranlina closed pull request #1769: JAMES-2586 - Implement PostgresTableManager
URL: https://github.com/apache/james-project/pull/1769
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
Re: [PR] JAMES-2586 - Implement PostgresTableManager [james-project]
Posted by "quantranhong1999 (via GitHub)" <gi...@apache.org>.
quantranhong1999 commented on code in PR #1769:
URL: https://github.com/apache/james-project/pull/1769#discussion_r1378361540
##########
backends-common/postgres/src/main/java/org/apache/james/backends/postgres/PostgresModule.java:
##########
@@ -0,0 +1,129 @@
+/****************************************************************
+ * Licensed to the Apache Software Foundation (ASF) under one *
+ * or more contributor license agreements. See the NOTICE file *
+ * distributed with this work for additional information *
+ * regarding copyright ownership. The ASF licenses this file *
+ * to you under the Apache License, Version 2.0 (the *
+ * "License"); you may not use this file except in compliance *
+ * with the License. You may obtain a copy of the License at *
+ * *
+ * http://www.apache.org/licenses/LICENSE-2.0 *
+ * *
+ * Unless required by applicable law or agreed to in writing, *
+ * software distributed under the License is distributed on an *
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY *
+ * KIND, either express or implied. See the License for the *
+ * specific language governing permissions and limitations *
+ * under the License. *
+ ****************************************************************/
+
+package org.apache.james.backends.postgres;
+
+
+import java.util.List;
+
+import com.google.common.collect.ImmutableList;
+
+public interface PostgresModule {
+
+ static PostgresModule aggregateModules(PostgresModule... modules) {
+ return builder()
+ .modules(modules)
+ .build();
+ }
+
+ static PostgresModule aggregateModules(List<PostgresModule> modules) {
+ return builder()
+ .modules(modules)
+ .build();
+ }
+
+ PostgresModule EMPTY_MODULE = builder().build();
+
+ List<PostgresTable> tables();
+
+ List<PostgresIndex> tableIndexes();
+
+ class Impl implements PostgresModule {
+ private final List<PostgresTable> tables;
+ private final List<PostgresIndex> tableIndexes;
+
+ private Impl(List<PostgresTable> tables, List<PostgresIndex> tableIndexes) {
+ this.tables = tables;
+ this.tableIndexes = tableIndexes;
+ }
+
+ @Override
+ public List<PostgresTable> tables() {
+ return tables;
+ }
+
+ @Override
+ public List<PostgresIndex> tableIndexes() {
+ return tableIndexes;
+ }
+ }
+
+ class Builder {
+ private final ImmutableList.Builder<PostgresTable> tables;
+ private final ImmutableList.Builder<PostgresIndex> tableIndexes;
+
+ public Builder() {
+ tables = ImmutableList.builder();
+ tableIndexes = ImmutableList.builder();
+ }
+
+ public Builder table(PostgresTable... table) {
+ tables.add(table);
+ return this;
+ }
Review Comment:
Q: Should we change `table` -> `addTable`?
Today:
```
builder
.table(tableA)
.table(tableB)
```
My impression when use that code is tableB would override `tableA`, and only `tableB` is created.
##########
backends-common/postgres/src/test/java/org/apache/james/backends/postgres/PostgresTableManagerTest.java:
##########
@@ -0,0 +1,310 @@
+/****************************************************************
+ * Licensed to the Apache Software Foundation (ASF) under one *
+ * or more contributor license agreements. See the NOTICE file *
+ * distributed with this work for additional information *
+ * regarding copyright ownership. The ASF licenses this file *
+ * to you under the Apache License, Version 2.0 (the *
+ * "License"); you may not use this file except in compliance *
+ * with the License. You may obtain a copy of the License at *
+ * *
+ * http://www.apache.org/licenses/LICENSE-2.0 *
+ * *
+ * Unless required by applicable law or agreed to in writing, *
+ * software distributed under the License is distributed on an *
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY *
+ * KIND, either express or implied. See the License for the *
+ * specific language governing permissions and limitations *
+ * under the License. *
+ ****************************************************************/
+
+package org.apache.james.backends.postgres;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatCode;
+
+import java.util.List;
+import java.util.function.Function;
+
+import org.apache.commons.lang3.tuple.Pair;
+import org.apache.james.backends.postgres.utils.PostgresExecutor;
+import org.jooq.impl.DSL;
+import org.jooq.impl.SQLDataType;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.testcontainers.containers.GenericContainer;
+import org.testcontainers.junit.jupiter.Container;
+import org.testcontainers.junit.jupiter.Testcontainers;
+
+import io.r2dbc.postgresql.PostgresqlConnectionConfiguration;
+import io.r2dbc.postgresql.PostgresqlConnectionFactory;
+import io.r2dbc.postgresql.api.PostgresqlResult;
+import io.r2dbc.spi.Connection;
+import reactor.core.publisher.Flux;
+import reactor.core.publisher.Mono;
+
+@Testcontainers
+public class PostgresTableManagerTest {
+
+ @Container
+ private static final GenericContainer<?> pgContainer = PostgresFixture.PG_CONTAINER.get();
+
+ private PostgresqlConnectionFactory connectionFactory;
+
+ @BeforeEach
+ void beforeAll() {
+ connectionFactory = new PostgresqlConnectionFactory(PostgresqlConnectionConfiguration.builder()
+ .host(pgContainer.getHost())
+ .port(pgContainer.getMappedPort(PostgresFixture.PORT))
+ .username(PostgresFixture.Database.DB_USER)
+ .password(PostgresFixture.Database.DB_PASSWORD)
+ .database(PostgresFixture.Database.DB_NAME)
+ .schema(PostgresFixture.Database.SCHEMA)
+ .build());
+ }
+
+ @AfterEach
+ void afterEach() {
+ // clean data
+ Flux.usingWhen(connectionFactory.create(),
+ connection -> Mono.from(connection.createStatement("DROP SCHEMA " + PostgresFixture.Database.SCHEMA + " CASCADE").execute())
+ .then(Mono.from(connection.createStatement("CREATE SCHEMA " + PostgresFixture.Database.SCHEMA).execute()))
+ .flatMap(PostgresqlResult::getRowsUpdated),
+ Connection::close)
+ .collectList()
+ .block();
+ }
+
+ Function<PostgresModule, PostgresTableManager> tableManagerFactory = module -> new PostgresTableManager(new PostgresExecutor(connectionFactory.create()
+ .map(c -> c)), module);
+
+ @Test
+ void initializeTableShouldSuccessWhenModuleHasSingleTable() {
+ String tableName = "tableName1";
+
+ PostgresTable table = PostgresTable.name(tableName)
+ .creteTableStep(dslContext -> dslContext.createTable(tableName)
Review Comment:
What happens if `PostgresTable.name(tableA)` and `dslContext.createTable(tableB)`?
Maybe `dslContext.createTable(tableName)` should be implicitly set following `PostgresTable.name(tableName)` (like Cassandra), no?
##########
backends-common/postgres/src/main/java/org/apache/james/backends/postgres/PostgresTable.java:
##########
@@ -0,0 +1,58 @@
+/****************************************************************
+ * Licensed to the Apache Software Foundation (ASF) under one *
+ * or more contributor license agreements. See the NOTICE file *
+ * distributed with this work for additional information *
+ * regarding copyright ownership. The ASF licenses this file *
+ * to you under the Apache License, Version 2.0 (the *
+ * "License"); you may not use this file except in compliance *
+ * with the License. You may obtain a copy of the License at *
+ * *
+ * http://www.apache.org/licenses/LICENSE-2.0 *
+ * *
+ * Unless required by applicable law or agreed to in writing, *
+ * software distributed under the License is distributed on an *
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY *
+ * KIND, either express or implied. See the License for the *
+ * specific language governing permissions and limitations *
+ * under the License. *
+ ****************************************************************/
+
+package org.apache.james.backends.postgres;
+
+import java.util.function.Function;
+
+import org.jooq.DDLQuery;
+import org.jooq.DSLContext;
+
+public class PostgresTable {
+
+ @FunctionalInterface
+ public interface RequireCreateTableStep {
+ PostgresTable creteTableStep(Function<DSLContext, DDLQuery> createTableStep);
Review Comment:
```suggestion
PostgresTable createTableStep(Function<DSLContext, DDLQuery> createTableStep);
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
Re: [PR] JAMES-2586 - Implement PostgresTableManager [james-project]
Posted by "vttranlina (via GitHub)" <gi...@apache.org>.
vttranlina commented on code in PR #1769:
URL: https://github.com/apache/james-project/pull/1769#discussion_r1378548504
##########
backends-common/postgres/src/main/java/org/apache/james/backends/postgres/PostgresModule.java:
##########
@@ -0,0 +1,129 @@
+/****************************************************************
+ * Licensed to the Apache Software Foundation (ASF) under one *
+ * or more contributor license agreements. See the NOTICE file *
+ * distributed with this work for additional information *
+ * regarding copyright ownership. The ASF licenses this file *
+ * to you under the Apache License, Version 2.0 (the *
+ * "License"); you may not use this file except in compliance *
+ * with the License. You may obtain a copy of the License at *
+ * *
+ * http://www.apache.org/licenses/LICENSE-2.0 *
+ * *
+ * Unless required by applicable law or agreed to in writing, *
+ * software distributed under the License is distributed on an *
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY *
+ * KIND, either express or implied. See the License for the *
+ * specific language governing permissions and limitations *
+ * under the License. *
+ ****************************************************************/
+
+package org.apache.james.backends.postgres;
+
+
+import java.util.List;
+
+import com.google.common.collect.ImmutableList;
+
+public interface PostgresModule {
+
+ static PostgresModule aggregateModules(PostgresModule... modules) {
+ return builder()
+ .modules(modules)
+ .build();
+ }
+
+ static PostgresModule aggregateModules(List<PostgresModule> modules) {
+ return builder()
+ .modules(modules)
+ .build();
+ }
+
+ PostgresModule EMPTY_MODULE = builder().build();
+
+ List<PostgresTable> tables();
+
+ List<PostgresIndex> tableIndexes();
+
+ class Impl implements PostgresModule {
+ private final List<PostgresTable> tables;
+ private final List<PostgresIndex> tableIndexes;
+
+ private Impl(List<PostgresTable> tables, List<PostgresIndex> tableIndexes) {
+ this.tables = tables;
+ this.tableIndexes = tableIndexes;
+ }
+
+ @Override
+ public List<PostgresTable> tables() {
+ return tables;
+ }
+
+ @Override
+ public List<PostgresIndex> tableIndexes() {
+ return tableIndexes;
+ }
+ }
+
+ class Builder {
+ private final ImmutableList.Builder<PostgresTable> tables;
+ private final ImmutableList.Builder<PostgresIndex> tableIndexes;
+
+ public Builder() {
+ tables = ImmutableList.builder();
+ tableIndexes = ImmutableList.builder();
+ }
+
+ public Builder table(PostgresTable... table) {
+ tables.add(table);
+ return this;
+ }
Review Comment:
https://github.com/apache/james-project/pull/1770/commits/8089a3cb1adc20353817e213364a8ff45b9a8559
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
Re: [PR] JAMES-2586 - Implement PostgresTableManager [james-project]
Posted by "vttranlina (via GitHub)" <gi...@apache.org>.
vttranlina commented on code in PR #1769:
URL: https://github.com/apache/james-project/pull/1769#discussion_r1378373915
##########
backends-common/postgres/src/test/java/org/apache/james/backends/postgres/PostgresTableManagerTest.java:
##########
@@ -0,0 +1,310 @@
+/****************************************************************
+ * Licensed to the Apache Software Foundation (ASF) under one *
+ * or more contributor license agreements. See the NOTICE file *
+ * distributed with this work for additional information *
+ * regarding copyright ownership. The ASF licenses this file *
+ * to you under the Apache License, Version 2.0 (the *
+ * "License"); you may not use this file except in compliance *
+ * with the License. You may obtain a copy of the License at *
+ * *
+ * http://www.apache.org/licenses/LICENSE-2.0 *
+ * *
+ * Unless required by applicable law or agreed to in writing, *
+ * software distributed under the License is distributed on an *
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY *
+ * KIND, either express or implied. See the License for the *
+ * specific language governing permissions and limitations *
+ * under the License. *
+ ****************************************************************/
+
+package org.apache.james.backends.postgres;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatCode;
+
+import java.util.List;
+import java.util.function.Function;
+
+import org.apache.commons.lang3.tuple.Pair;
+import org.apache.james.backends.postgres.utils.PostgresExecutor;
+import org.jooq.impl.DSL;
+import org.jooq.impl.SQLDataType;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.testcontainers.containers.GenericContainer;
+import org.testcontainers.junit.jupiter.Container;
+import org.testcontainers.junit.jupiter.Testcontainers;
+
+import io.r2dbc.postgresql.PostgresqlConnectionConfiguration;
+import io.r2dbc.postgresql.PostgresqlConnectionFactory;
+import io.r2dbc.postgresql.api.PostgresqlResult;
+import io.r2dbc.spi.Connection;
+import reactor.core.publisher.Flux;
+import reactor.core.publisher.Mono;
+
+@Testcontainers
+public class PostgresTableManagerTest {
+
+ @Container
+ private static final GenericContainer<?> pgContainer = PostgresFixture.PG_CONTAINER.get();
+
+ private PostgresqlConnectionFactory connectionFactory;
+
+ @BeforeEach
+ void beforeAll() {
+ connectionFactory = new PostgresqlConnectionFactory(PostgresqlConnectionConfiguration.builder()
+ .host(pgContainer.getHost())
+ .port(pgContainer.getMappedPort(PostgresFixture.PORT))
+ .username(PostgresFixture.Database.DB_USER)
+ .password(PostgresFixture.Database.DB_PASSWORD)
+ .database(PostgresFixture.Database.DB_NAME)
+ .schema(PostgresFixture.Database.SCHEMA)
+ .build());
+ }
+
+ @AfterEach
+ void afterEach() {
+ // clean data
+ Flux.usingWhen(connectionFactory.create(),
+ connection -> Mono.from(connection.createStatement("DROP SCHEMA " + PostgresFixture.Database.SCHEMA + " CASCADE").execute())
+ .then(Mono.from(connection.createStatement("CREATE SCHEMA " + PostgresFixture.Database.SCHEMA).execute()))
+ .flatMap(PostgresqlResult::getRowsUpdated),
+ Connection::close)
+ .collectList()
+ .block();
+ }
+
+ Function<PostgresModule, PostgresTableManager> tableManagerFactory = module -> new PostgresTableManager(new PostgresExecutor(connectionFactory.create()
+ .map(c -> c)), module);
+
+ @Test
+ void initializeTableShouldSuccessWhenModuleHasSingleTable() {
+ String tableName = "tableName1";
+
+ PostgresTable table = PostgresTable.name(tableName)
+ .creteTableStep(dslContext -> dslContext.createTable(tableName)
Review Comment:
I thought it at first, But I can't not find a way to do it by code
Let me try one more time
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
Re: [PR] JAMES-2586 - Implement PostgresTableManager [james-project]
Posted by "quantranhong1999 (via GitHub)" <gi...@apache.org>.
quantranhong1999 commented on PR #1769:
URL: https://github.com/apache/james-project/pull/1769#issuecomment-1788398842
Should target [apache:postgresql](https://github.com/apache/james-project/tree/postgresql) instead of master.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
Re: [PR] JAMES-2586 - Implement PostgresTableManager [james-project]
Posted by "vttranlina (via GitHub)" <gi...@apache.org>.
vttranlina commented on PR #1769:
URL: https://github.com/apache/james-project/pull/1769#issuecomment-1788631360
> Should target [apache:postgresql](https://github.com/apache/james-project/tree/postgresql) instead of master.
I created a new pr to `postgresql` branch at https://github.com/apache/james-project/pull/1770
And add a new fixup commit
I will close this pr
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org