From 0be3e14ceed84c6f9614a05d59084b17e53c7f37 Mon Sep 17 00:00:00 2001 From: rhkp Date: Tue, 17 Dec 2024 23:08:59 +0000 Subject: [PATCH] Address review comments --- .../kogito-db-migrator-tool-image/README.md | 10 ++-- ...tor-kie-kogito-db-migrator-tool-image.yaml | 2 +- .../postgresql/migrator/DBMigrator.java | 37 +++++++++--- .../src/main/resources/application.properties | 4 +- .../migrator/DBConnectionCheckerTest.java | 42 ++++++++------ .../postgresql/migrator/DBMigratorTest.java | 58 ++++++++++++------- .../migrator/MigrationServiceTest.java | 53 ++++++++++------- 7 files changed, 129 insertions(+), 77 deletions(-) diff --git a/packages/kogito-db-migrator-tool-image/README.md b/packages/kogito-db-migrator-tool-image/README.md index dca5cd3bca1..4af61293cea 100644 --- a/packages/kogito-db-migrator-tool-image/README.md +++ b/packages/kogito-db-migrator-tool-image/README.md @@ -63,12 +63,12 @@ pnpm -F @kie-tools/kogito-db-migrator-tool-image... build:prod | QUARKUS_DATASOURCE_DATAINDEX_JDBC_URL | Data index database url e.g. jdbc:postgresql://host.docker.internal:5432/di | jdbc:postgresql://localhost:5432/postgres | | QUARKUS_DATASOURCE_DATAINDEX_USERNAME | Data index database username | postgres | | QUARKUS_DATASOURCE_DATAINDEX_PASSWORD | Data index database password | postgres | -| QUARKUS_FLYWAY_DATAINDEX_SCHEMAS | Data index database schema | dataindex | +| QUARKUS_FLYWAY_DATAINDEX_SCHEMAS | Data index database schema | data-index-service | | MIGRATE_DB_JOBSSERVICE | Set to true if you want to migrate jobs service database, set to false otherwise | false | | QUARKUS_DATASOURCE_JOBSSERVICE_JDBC_URL | Jobs service database url e.g. jdbc:postgresql://host.docker.internal:5432/js | jdbc:postgresql://localhost:5432/postgres | | QUARKUS_DATASOURCE_JOBSSERVICE_USERNAME | Jobs service database username | postgres | | QUARKUS_DATASOURCE_JOBSSERVICE_PASSWORD | Jobs service database password | postgres | -| QUARKUS_FLYWAY_JOBSSERVICE_SCHEMAS | Jobs service database schema | jobsservice | +| QUARKUS_FLYWAY_JOBSSERVICE_SCHEMAS | Jobs service database schema | jobs-service | ### Example @@ -80,13 +80,13 @@ An example to use diverse environment variables --env QUARKUS_DATASOURCE_DATAINDEX_JDBC_URL= \ --env QUARKUS_DATASOURCE_DATAINDEX_USERNAME= \ --env QUARKUS_DATASOURCE_DATAINDEX_PASSWORD= \ - --env QUARKUS_FLYWAY_DATAINDEX_SCHEMAS=dataindex \ + --env QUARKUS_FLYWAY_DATAINDEX_SCHEMAS=data-index-service \ --env MIGRATE_DB_JOBSSERVICE=true \ --env QUARKUS_DATASOURCE_JOBSSERVICE_JDBC_URL= \ --env QUARKUS_DATASOURCE_JOBSSERVICE_USERNAME= \ --env QUARKUS_DATASOURCE_JOBSSERVICE_PASSWORD= \ - --env QUARKUS_FLYWAY_JOBSSERVICE_SCHEMAS=jobsservice \ - docker.io/apache/incubator-kie-kogito-service-db-migration-postgresql:999-SNAPSHOT + --env QUARKUS_FLYWAY_JOBSSERVICE_SCHEMAS=jobs-service \ + docker.io/apache/incubator-kie-kogito-db-migrator-tool:main ``` --- diff --git a/packages/kogito-db-migrator-tool-image/resources/incubator-kie-kogito-db-migrator-tool-image.yaml b/packages/kogito-db-migrator-tool-image/resources/incubator-kie-kogito-db-migrator-tool-image.yaml index 506e5bc5d24..fd29a2f39e5 100644 --- a/packages/kogito-db-migrator-tool-image/resources/incubator-kie-kogito-db-migrator-tool-image.yaml +++ b/packages/kogito-db-migrator-tool-image/resources/incubator-kie-kogito-db-migrator-tool-image.yaml @@ -19,7 +19,7 @@ name: "docker.io/apache/incubator-kie-kogito-db-migrator-tool" version: "main" from: registry.access.redhat.com/ubi8/openjdk-17-runtime:1.20 -description: Flyway image for Data Index and Jobs Service database migration +description: DBMigratorTool image for Data Index and Jobs Service database migration labels: - name: "org.kie.kogito.version" diff --git a/packages/kogito-db-migrator-tool/src/main/java/org/kie/kogito/postgresql/migrator/DBMigrator.java b/packages/kogito-db-migrator-tool/src/main/java/org/kie/kogito/postgresql/migrator/DBMigrator.java index bb69266f4e0..67c3f5e46a9 100644 --- a/packages/kogito-db-migrator-tool/src/main/java/org/kie/kogito/postgresql/migrator/DBMigrator.java +++ b/packages/kogito-db-migrator-tool/src/main/java/org/kie/kogito/postgresql/migrator/DBMigrator.java @@ -23,6 +23,7 @@ import io.quarkus.runtime.annotations.QuarkusMain; import jakarta.inject.Inject; import io.quarkus.logging.Log; +import org.flywaydb.core.api.FlywayException; import java.sql.SQLException; @@ -31,6 +32,12 @@ @QuarkusMain public class DBMigrator implements QuarkusApplication { + private int SUCCESS_DB_MIGRATION = 0; + private int ERR_DATA_INDEX_DB_CONN = -1; + private int ERR_JOBS_SERVICE_DB_CONN = -2; + private int ERR_DATA_INDEX_MIGRATION = -3; + private int ERR_JOBS_SERVICE_MIGRATION = -4; + @Inject MigrationService service; @@ -50,10 +57,17 @@ public int run(String... args) { dbConnectionChecker.checkDataIndexDBConnection(); } catch (SQLException e) { Log.error( "Error obtaining data index database connection. Cannot proceed, exiting."); - Quarkus.asyncExit(-1); - return -1; + Quarkus.asyncExit(ERR_DATA_INDEX_DB_CONN); + return ERR_DATA_INDEX_DB_CONN; + } + + try{ + service.migrateDataIndex(); + } catch ( FlywayException fe ){ + Log.error( "Error migrating data index database, flyway service exception occured, please check logs."); + Quarkus.asyncExit(ERR_DATA_INDEX_MIGRATION); + return ERR_DATA_INDEX_MIGRATION; } - service.migrateDataIndex(); } if (migrateJobsService) { @@ -61,13 +75,20 @@ public int run(String... args) { dbConnectionChecker.checkJobsServiceDBConnection(); } catch (SQLException e) { Log.error( "Error obtaining jobs service database connection. Cannot proceed, exiting."); - Quarkus.asyncExit(-2); - return -2; + Quarkus.asyncExit(ERR_JOBS_SERVICE_DB_CONN); + return ERR_JOBS_SERVICE_DB_CONN; + } + + try{ + service.migrateJobsService(); + } catch ( FlywayException fe ){ + Log.error( "Error migrating jobs service database, flyway service exception occured, please check logs."); + Quarkus.asyncExit(ERR_JOBS_SERVICE_MIGRATION); + return ERR_JOBS_SERVICE_MIGRATION; } - service.migrateJobsService(); } - Quarkus.asyncExit(0); - return 0; + Quarkus.asyncExit(SUCCESS_DB_MIGRATION); + return SUCCESS_DB_MIGRATION; } } \ No newline at end of file diff --git a/packages/kogito-db-migrator-tool/src/main/resources/application.properties b/packages/kogito-db-migrator-tool/src/main/resources/application.properties index 59b8c2382a5..5f18a83ff28 100644 --- a/packages/kogito-db-migrator-tool/src/main/resources/application.properties +++ b/packages/kogito-db-migrator-tool/src/main/resources/application.properties @@ -24,7 +24,7 @@ quarkus.datasource.dataindex.username=postgres quarkus.datasource.dataindex.password=postgres quarkus.datasource.dataindex.jdbc.url=jdbc:postgresql://localhost:5432/postgres quarkus.flyway.dataindex.locations=classpath:postgresql/data-index -quarkus.flyway.dataindex.schemas=dataindex +quarkus.flyway.dataindex.schemas=data-index-service quarkus.flyway.dataindex.migrate-at-start=false quarkus.flyway.dataindex.clean-at-start=false @@ -35,6 +35,6 @@ quarkus.datasource.jobsservice.username=postgres quarkus.datasource.jobsservice.password=postgres quarkus.datasource.jobsservice.jdbc.url=jdbc:postgresql://localhost:5432/postgres quarkus.flyway.jobsservice.locations=classpath:postgresql/jobs-service -quarkus.flyway.jobsservice.schemas=jobsservice +quarkus.flyway.jobsservice.schemas=jobs-service quarkus.flyway.jobsservice.migrate-at-start=false quarkus.flyway.jobsservice.clean-at-start=false \ No newline at end of file diff --git a/packages/kogito-db-migrator-tool/src/test/java/org/kie/kogito/postgresql/migrator/DBConnectionCheckerTest.java b/packages/kogito-db-migrator-tool/src/test/java/org/kie/kogito/postgresql/migrator/DBConnectionCheckerTest.java index 935eed5c382..77e23482f76 100644 --- a/packages/kogito-db-migrator-tool/src/test/java/org/kie/kogito/postgresql/migrator/DBConnectionCheckerTest.java +++ b/packages/kogito-db-migrator-tool/src/test/java/org/kie/kogito/postgresql/migrator/DBConnectionCheckerTest.java @@ -1,17 +1,20 @@ /* - * Copyright 2024 Apache Software Foundation (ASF) - * - * Licensed 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. + * 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.kie.kogito.postgresql.migrator; @@ -26,23 +29,24 @@ import java.sql.DriverManager; import java.sql.SQLException; -import static org.junit.jupiter.api.Assertions.*; +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.Mockito.mockStatic; -public class DBConnectionCheckerTest { +class DBConnectionCheckerTest { DBConnectionChecker dbConnectionChecker = new DBConnectionChecker(); @Mock static DriverManager driverManager; @BeforeAll - public static void init() { + static void init() { mockStatic(DriverManager.class); } @BeforeEach - public void setupEach() { + void setupEach() { dbConnectionChecker.dataIndexDBURL = "jdbc:postgresql://db-service:5432/di"; dbConnectionChecker.dataIndexDBUserName = "postgres"; dbConnectionChecker.dataIndexDBPassword = "postgres"; @@ -53,14 +57,14 @@ public void setupEach() { } @Test - public void testCheckDBConnections() throws SQLException { + void testCheckDBConnections() throws SQLException { Mockito.when(driverManager.getConnection(anyString(), anyString(), anyString())).thenReturn(Mockito.mock(Connection.class)); assertDoesNotThrow(() -> dbConnectionChecker.checkDataIndexDBConnection()); assertDoesNotThrow(() -> dbConnectionChecker.checkJobsServiceDBConnection()); } @Test - public void testCheckDBConnectionsThrowSQLException() throws SQLException { + void testCheckDBConnectionsThrowSQLException() throws SQLException { Mockito.when(driverManager.getConnection(anyString(), anyString(), anyString())).thenThrow(SQLException.class); assertThrows(SQLException.class, () -> dbConnectionChecker.checkDataIndexDBConnection()); assertThrows(SQLException.class, () -> dbConnectionChecker.checkJobsServiceDBConnection()); diff --git a/packages/kogito-db-migrator-tool/src/test/java/org/kie/kogito/postgresql/migrator/DBMigratorTest.java b/packages/kogito-db-migrator-tool/src/test/java/org/kie/kogito/postgresql/migrator/DBMigratorTest.java index 3ee9c6e0555..8fe45292b79 100644 --- a/packages/kogito-db-migrator-tool/src/test/java/org/kie/kogito/postgresql/migrator/DBMigratorTest.java +++ b/packages/kogito-db-migrator-tool/src/test/java/org/kie/kogito/postgresql/migrator/DBMigratorTest.java @@ -1,17 +1,20 @@ /* - * Copyright 2024 Apache Software Foundation (ASF) - * - * Licensed 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. + * 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.kie.kogito.postgresql.migrator; @@ -19,7 +22,6 @@ import io.quarkus.test.Mock; import org.junit.Rule; import org.junit.contrib.java.lang.system.ExpectedSystemExit; -import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -27,10 +29,12 @@ import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.times; -public class DBMigratorTest { +class DBMigratorTest { @Rule - public final ExpectedSystemExit exitRule = ExpectedSystemExit.none(); + final ExpectedSystemExit exitRule = ExpectedSystemExit.none(); @Mock MigrationService migrationService; @@ -41,22 +45,26 @@ public class DBMigratorTest { DBMigrator dbMigrator = new DBMigrator(); @BeforeEach - public void setupEach() { + void setupEach() { migrationService = mock(MigrationService.class); dbConnectionChecker = mock(DBConnectionChecker.class); } @Test - public void testMigratorWithNoMigrations() throws Exception { + void testMigratorWithNoMigrations() throws Exception { dbMigrator.migrateDataIndex = false; dbMigrator.migrateJobsService = false; exitRule.expectSystemExitWithStatus(0); dbMigrator.run(); + verify(dbConnectionChecker, times(0)).checkDataIndexDBConnection(); + verify(dbConnectionChecker, times(0)).checkJobsServiceDBConnection(); + verify(migrationService, times(0)).migrateDataIndex(); + verify(migrationService, times(0)).migrateJobsService(); } @Test - public void testMigratorWithAllMigrations() throws Exception { + void testMigratorWithAllMigrations() throws Exception { dbMigrator.migrateDataIndex = true; dbMigrator.migrateJobsService = true; dbMigrator.dbConnectionChecker = dbConnectionChecker; @@ -64,10 +72,14 @@ public void testMigratorWithAllMigrations() throws Exception { exitRule.expectSystemExitWithStatus(0); dbMigrator.run(); + verify(dbConnectionChecker, times(1)).checkDataIndexDBConnection(); + verify(dbConnectionChecker, times(1)).checkJobsServiceDBConnection(); + verify(migrationService, times(1)).migrateDataIndex(); + verify(migrationService, times(1)).migrateJobsService(); } @Test - public void testDataIndexMigrationWithException() throws Exception { + void testDataIndexMigrationWithException() throws Exception { dbMigrator.migrateDataIndex = true; dbMigrator.migrateJobsService = false; dbMigrator.dbConnectionChecker = dbConnectionChecker; @@ -77,10 +89,12 @@ public void testDataIndexMigrationWithException() throws Exception { exitRule.expectSystemExitWithStatus(-1); dbMigrator.run(); + verify(migrationService, times(0)).migrateDataIndex(); + verify(migrationService, times(0)).migrateJobsService(); } @Test - public void testJobsServiceWithException() throws Exception { + void testJobsServiceWithException() throws Exception { dbMigrator.migrateDataIndex = false; dbMigrator.migrateJobsService = true; dbMigrator.dbConnectionChecker = dbConnectionChecker; @@ -90,5 +104,7 @@ public void testJobsServiceWithException() throws Exception { exitRule.expectSystemExitWithStatus(-2); dbMigrator.run(); + verify(migrationService, times(0)).migrateDataIndex(); + verify(migrationService, times(0)).migrateJobsService(); } } diff --git a/packages/kogito-db-migrator-tool/src/test/java/org/kie/kogito/postgresql/migrator/MigrationServiceTest.java b/packages/kogito-db-migrator-tool/src/test/java/org/kie/kogito/postgresql/migrator/MigrationServiceTest.java index 2506952c2b3..b64f23f105d 100644 --- a/packages/kogito-db-migrator-tool/src/test/java/org/kie/kogito/postgresql/migrator/MigrationServiceTest.java +++ b/packages/kogito-db-migrator-tool/src/test/java/org/kie/kogito/postgresql/migrator/MigrationServiceTest.java @@ -1,17 +1,20 @@ /* - * Copyright 2024 Apache Software Foundation (ASF) - * - * Licensed 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. + * 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.kie.kogito.postgresql.migrator; @@ -20,50 +23,58 @@ import org.flywaydb.core.Flyway; import org.flywaydb.core.api.output.CleanResult; import org.flywaydb.core.api.output.MigrateResult; -import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; -import static org.mockito.Mockito.*; +import static org.mockito.Mockito.when; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.times; -public class MigrationServiceTest { +class MigrationServiceTest { @Mock Flyway flyway; MigrationService migrationService = new MigrationService(); @BeforeEach - public void setupEach() { + void setupEach() { flyway = mock(Flyway.class); when(flyway.migrate()).thenReturn(new MigrateResult("flywayVersion", "db", "schema")); when(flyway.clean()).thenReturn(new CleanResult("flywayVersion", "db")); } @Test - public void testMigrateDataIndexWithNoClean() { + void testMigrateDataIndexWithNoClean() { migrationService.cleanDataIndex = false; migrationService.flywayDataIndex = flyway; migrationService.migrateDataIndex(); + verify(flyway, times(1)).migrate(); } @Test - public void testMigrateDataIndexWithClean() { + void testMigrateDataIndexWithClean() { migrationService.cleanDataIndex = true; migrationService.flywayDataIndex = flyway; migrationService.migrateDataIndex(); + verify(flyway, times(1)).clean(); + verify(flyway, times(1)).migrate(); } @Test - public void testMigrateJobsServiceWithNoClean() { + void testMigrateJobsServiceWithNoClean() { migrationService.cleanJobsService = false; migrationService.flywayJobsService = flyway; migrationService.migrateJobsService(); + verify(flyway, times(1)).migrate(); } @Test - public void testMigrateJobsServiceWithClean() { + void testMigrateJobsServiceWithClean() { migrationService.cleanJobsService = true; migrationService.flywayJobsService = flyway; migrationService.migrateJobsService(); + verify(flyway, times(1)).clean(); + verify(flyway, times(1)).migrate(); } }