Address review comments

apache · Dec 17, 2024 · 0be3e14 · 0be3e14
1 parent 8252831
commit 0be3e14
Show file tree

Hide file tree

Showing 7 changed files with 129 additions and 77 deletions.
diff --git 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=<data-index-db-url e.g. jdbc:postgresql://host.docker.internal:5432/di> \
    --env QUARKUS_DATASOURCE_DATAINDEX_USERNAME=<data-index-db-user> \
    --env QUARKUS_DATASOURCE_DATAINDEX_PASSWORD=<data-index-db-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=<jobs-service-db-url e.g. jdbc:postgresql://host.docker.internal:5432/js> \
    --env QUARKUS_DATASOURCE_JOBSSERVICE_USERNAME=<jobs-service-db-user> \
    --env QUARKUS_DATASOURCE_JOBSSERVICE_PASSWORD=<jobs-service-db-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/.../kogito-db-migrator-tool-image/resources/incubator-kie-kogito-db-migrator-tool-image.yaml b/.../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/.../kogito-db-migrator-tool/src/main/java/org/kie/kogito/postgresql/migrator/DBMigrator.java b/.../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,24 +57,38 @@ 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) {
             try {
                 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;
     }
 }
diff --git 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
diff --git a/...grator-tool/src/test/java/org/kie/kogito/postgresql/migrator/DBConnectionCheckerTest.java b/...grator-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/...ito-db-migrator-tool/src/test/java/org/kie/kogito/postgresql/migrator/DBMigratorTest.java b/...ito-db-migrator-tool/src/test/java/org/kie/kogito/postgresql/migrator/DBMigratorTest.java
@@ -1,36 +1,40 @@
 /*
- * 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;
 
 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;
 
 import java.sql.SQLException;
 
 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,33 +45,41 @@ 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;
         dbMigrator.service = migrationService;
 
         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();
     }
 }