From dd22dbef8958311c2ec5e99822269405d59f8eba Mon Sep 17 00:00:00 2001 From: Arafat2198 Date: Fri, 15 Nov 2024 17:33:43 +0530 Subject: [PATCH] HDDS-11615. Add Upgrade Action for Initial Schema Constraints for Unhealthy Container Table in Recon. (#7372) --- .../schema/ContainerSchemaDefinition.java | 35 +--- .../hadoop/ozone/recon/ReconServer.java | 4 +- .../ReconStorageContainerManagerFacade.java | 12 +- .../InitialConstraintUpgradeAction.java | 114 +++++++++++ .../upgrade/ReconLayoutVersionManager.java | 5 +- .../recon/upgrade/ReconUpgradeAction.java | 4 +- .../TestInitialConstraintUpgradeAction.java | 192 ++++++++++++++++++ .../TestReconLayoutVersionManager.java | 41 ++-- 8 files changed, 358 insertions(+), 49 deletions(-) create mode 100644 hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/upgrade/InitialConstraintUpgradeAction.java create mode 100644 hadoop-ozone/recon/src/test/java/org/apache/hadoop/ozone/recon/upgrade/TestInitialConstraintUpgradeAction.java diff --git a/hadoop-ozone/recon-codegen/src/main/java/org/hadoop/ozone/recon/schema/ContainerSchemaDefinition.java b/hadoop-ozone/recon-codegen/src/main/java/org/hadoop/ozone/recon/schema/ContainerSchemaDefinition.java index 0882de3bf4f..0c778aead5d 100644 --- a/hadoop-ozone/recon-codegen/src/main/java/org/hadoop/ozone/recon/schema/ContainerSchemaDefinition.java +++ b/hadoop-ozone/recon-codegen/src/main/java/org/hadoop/ozone/recon/schema/ContainerSchemaDefinition.java @@ -31,7 +31,6 @@ import javax.sql.DataSource; import java.sql.Connection; import java.sql.SQLException; -import java.util.Arrays; /** * Class used to create tables that are required for tracking containers. @@ -70,39 +69,11 @@ public enum UnHealthyContainerStates { public void initializeSchema() throws SQLException { Connection conn = dataSource.getConnection(); dslContext = DSL.using(conn); - - if (TABLE_EXISTS_CHECK.test(conn, UNHEALTHY_CONTAINERS_TABLE_NAME)) { - // Drop the existing constraint if it exists - String constraintName = UNHEALTHY_CONTAINERS_TABLE_NAME + "ck1"; - dslContext.alterTable(UNHEALTHY_CONTAINERS_TABLE_NAME) - .dropConstraint(constraintName) - .execute(); - - // Add the updated constraint with all enum states - addUpdatedConstraint(); - } else { - // Create the table if it does not exist + if (!TABLE_EXISTS_CHECK.test(conn, UNHEALTHY_CONTAINERS_TABLE_NAME)) { createUnhealthyContainersTable(); } } - /** - * Add the updated constraint to the table. - */ - private void addUpdatedConstraint() { - // Get all enum values as a list of strings - String[] enumStates = Arrays.stream(UnHealthyContainerStates.values()) - .map(Enum::name) - .toArray(String[]::new); - - // Alter the table to add the updated constraint - dslContext.alterTable(UNHEALTHY_CONTAINERS_TABLE_NAME) - .add(DSL.constraint(UNHEALTHY_CONTAINERS_TABLE_NAME + "ck1") - .check(field(name("container_state")) - .in(enumStates))) - .execute(); - } - /** * Create the Missing Containers table. */ @@ -126,4 +97,8 @@ private void createUnhealthyContainersTable() { public DSLContext getDSLContext() { return dslContext; } + + public DataSource getDataSource() { + return dataSource; + } } diff --git a/hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/ReconServer.java b/hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/ReconServer.java index 7c9564c23b1..24b5c10952a 100644 --- a/hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/ReconServer.java +++ b/hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/ReconServer.java @@ -165,7 +165,9 @@ public Void call() throws Exception { ReconLayoutVersionManager layoutVersionManager = new ReconLayoutVersionManager(versionTableManager, reconContext); // Run the upgrade framework to finalize layout features if needed - layoutVersionManager.finalizeLayoutFeatures(); + ReconStorageContainerManagerFacade reconStorageContainerManagerFacade = + (ReconStorageContainerManagerFacade) this.getReconStorageContainerManager(); + layoutVersionManager.finalizeLayoutFeatures(reconStorageContainerManagerFacade); LOG.info("Initializing support of Recon Features..."); FeatureProvider.initFeatureSupport(configuration); diff --git a/hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/scm/ReconStorageContainerManagerFacade.java b/hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/scm/ReconStorageContainerManagerFacade.java index ea1a3440160..eff68848a2f 100644 --- a/hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/scm/ReconStorageContainerManagerFacade.java +++ b/hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/scm/ReconStorageContainerManagerFacade.java @@ -131,6 +131,8 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import javax.sql.DataSource; + /** * Recon's 'lite' version of SCM. */ @@ -156,6 +158,7 @@ public class ReconStorageContainerManagerFacade private final SCMHAManager scmhaManager; private final SequenceIdGenerator sequenceIdGen; private final ContainerHealthTask containerHealthTask; + private final DataSource dataSource; private DBStore dbStore; private ReconNodeManager nodeManager; @@ -188,7 +191,8 @@ public ReconStorageContainerManagerFacade(OzoneConfiguration conf, ReconContainerMetadataManager reconContainerMetadataManager, ReconUtils reconUtils, ReconSafeModeManager safeModeManager, - ReconContext reconContext) throws IOException { + ReconContext reconContext, + DataSource dataSource) throws IOException { reconNodeDetails = reconUtils.getReconNodeDetails(conf); this.threadNamePrefix = reconNodeDetails.threadNamePrefix(); this.eventQueue = new EventQueue(threadNamePrefix); @@ -285,6 +289,8 @@ public ReconStorageContainerManagerFacade(OzoneConfiguration conf, containerCountBySizeDao, utilizationSchemaDefinition); + this.dataSource = dataSource; + StaleNodeHandler staleNodeHandler = new ReconStaleNodeHandler(nodeManager, pipelineManager, conf, pipelineSyncTask); @@ -754,4 +760,8 @@ public ContainerCountBySizeDao getContainerCountBySizeDao() { public ReconContext getReconContext() { return reconContext; } + + public DataSource getDataSource() { + return dataSource; + } } diff --git a/hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/upgrade/InitialConstraintUpgradeAction.java b/hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/upgrade/InitialConstraintUpgradeAction.java new file mode 100644 index 00000000000..e75efd2116a --- /dev/null +++ b/hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/upgrade/InitialConstraintUpgradeAction.java @@ -0,0 +1,114 @@ +/* + * 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.hadoop.ozone.recon.upgrade; + +import com.google.common.annotations.VisibleForTesting; +import org.apache.hadoop.ozone.recon.scm.ReconStorageContainerManagerFacade; +import org.hadoop.ozone.recon.schema.ContainerSchemaDefinition; +import org.jooq.DSLContext; +import org.jooq.impl.DSL; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import javax.sql.DataSource; +import java.sql.Connection; +import java.sql.SQLException; +import java.util.Arrays; + +import static org.apache.hadoop.ozone.recon.upgrade.ReconLayoutFeature.INITIAL_VERSION; +import static org.apache.hadoop.ozone.recon.upgrade.ReconUpgradeAction.UpgradeActionType.FINALIZE; +import static org.hadoop.ozone.recon.codegen.SqlDbUtils.TABLE_EXISTS_CHECK; +import static org.hadoop.ozone.recon.schema.ContainerSchemaDefinition.UNHEALTHY_CONTAINERS_TABLE_NAME; +import static org.jooq.impl.DSL.field; +import static org.jooq.impl.DSL.name; + +/** + * Upgrade action for the INITIAL schema version, which manages constraints + * for the UNHEALTHY_CONTAINERS table. + */ +@UpgradeActionRecon(feature = INITIAL_VERSION, type = FINALIZE) +public class InitialConstraintUpgradeAction implements ReconUpgradeAction { + + private static final Logger LOG = LoggerFactory.getLogger(InitialConstraintUpgradeAction.class); + private DataSource dataSource; + private DSLContext dslContext; + + @Override + public void execute(ReconStorageContainerManagerFacade scmFacade) throws SQLException { + this.dataSource = scmFacade.getDataSource(); + try (Connection conn = dataSource.getConnection()) { + if (!TABLE_EXISTS_CHECK.test(conn, UNHEALTHY_CONTAINERS_TABLE_NAME)) { + return; + } + dslContext = DSL.using(conn); + // Drop the existing constraint + dropConstraint(); + // Add the updated constraint with all enum states + addUpdatedConstraint(); + } catch (SQLException e) { + throw new SQLException("Failed to execute InitialConstraintUpgradeAction", e); + } + } + + /** + * Drops the existing constraint from the UNHEALTHY_CONTAINERS table. + */ + private void dropConstraint() { + String constraintName = UNHEALTHY_CONTAINERS_TABLE_NAME + "ck1"; + dslContext.alterTable(UNHEALTHY_CONTAINERS_TABLE_NAME) + .dropConstraint(constraintName) + .execute(); + LOG.debug("Dropped the existing constraint: {}", constraintName); + } + + /** + * Adds the updated constraint directly within this class. + */ + private void addUpdatedConstraint() { + String[] enumStates = Arrays + .stream(ContainerSchemaDefinition.UnHealthyContainerStates.values()) + .map(Enum::name) + .toArray(String[]::new); + + dslContext.alterTable(ContainerSchemaDefinition.UNHEALTHY_CONTAINERS_TABLE_NAME) + .add(DSL.constraint(ContainerSchemaDefinition.UNHEALTHY_CONTAINERS_TABLE_NAME + "ck1") + .check(field(name("container_state")) + .in(enumStates))) + .execute(); + + LOG.info("Added the updated constraint to the UNHEALTHY_CONTAINERS table for enum state values: {}", + Arrays.toString(enumStates)); + } + + @Override + public UpgradeActionType getType() { + return FINALIZE; + } + + @VisibleForTesting + public void setDataSource(DataSource dataSource) { + this.dataSource = dataSource; + } + + @VisibleForTesting + public void setDslContext(DSLContext dslContext) { + this.dslContext = dslContext; + } +} diff --git a/hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/upgrade/ReconLayoutVersionManager.java b/hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/upgrade/ReconLayoutVersionManager.java index b646f6d9a86..e9f7fc9650d 100644 --- a/hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/upgrade/ReconLayoutVersionManager.java +++ b/hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/upgrade/ReconLayoutVersionManager.java @@ -21,6 +21,7 @@ import org.apache.hadoop.ozone.recon.ReconContext; import org.apache.hadoop.ozone.recon.ReconSchemaVersionTableManager; +import org.apache.hadoop.ozone.recon.scm.ReconStorageContainerManagerFacade; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -78,7 +79,7 @@ private int determineSLV() { * Finalizes the layout features that need to be upgraded, by executing the upgrade action for each * feature that is registered for finalization. */ - public void finalizeLayoutFeatures() { + public void finalizeLayoutFeatures(ReconStorageContainerManagerFacade scmFacade) { // Get features that need finalization, sorted by version List featuresToFinalize = getRegisteredFeatures(); @@ -88,7 +89,7 @@ public void finalizeLayoutFeatures() { Optional action = feature.getAction(ReconUpgradeAction.UpgradeActionType.FINALIZE); if (action.isPresent()) { // Execute the upgrade action & update the schema version in the DB - action.get().execute(); + action.get().execute(scmFacade); updateSchemaVersion(feature.getVersion()); LOG.info("Feature versioned {} finalized successfully.", feature.getVersion()); } diff --git a/hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/upgrade/ReconUpgradeAction.java b/hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/upgrade/ReconUpgradeAction.java index f09cdf8e1f2..d5fdbdacb7c 100644 --- a/hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/upgrade/ReconUpgradeAction.java +++ b/hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/upgrade/ReconUpgradeAction.java @@ -19,6 +19,8 @@ package org.apache.hadoop.ozone.recon.upgrade; +import org.apache.hadoop.ozone.recon.scm.ReconStorageContainerManagerFacade; + /** * ReconUpgradeAction is an interface for executing upgrade actions in Recon. */ @@ -40,7 +42,7 @@ enum UpgradeActionType { /** * Execute the upgrade action. */ - void execute() throws Exception; + void execute(ReconStorageContainerManagerFacade scmFacade) throws Exception; /** * Provides the type of upgrade phase (e.g., FINALIZE). diff --git a/hadoop-ozone/recon/src/test/java/org/apache/hadoop/ozone/recon/upgrade/TestInitialConstraintUpgradeAction.java b/hadoop-ozone/recon/src/test/java/org/apache/hadoop/ozone/recon/upgrade/TestInitialConstraintUpgradeAction.java new file mode 100644 index 00000000000..b2399f42362 --- /dev/null +++ b/hadoop-ozone/recon/src/test/java/org/apache/hadoop/ozone/recon/upgrade/TestInitialConstraintUpgradeAction.java @@ -0,0 +1,192 @@ +/* + * 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.hadoop.ozone.recon.upgrade; + +import org.apache.hadoop.ozone.recon.persistence.AbstractReconSqlDBTest; +import org.apache.hadoop.ozone.recon.scm.ReconStorageContainerManagerFacade; +import org.hadoop.ozone.recon.schema.ContainerSchemaDefinition; +import org.jooq.DSLContext; +import org.jooq.impl.DSL; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +import javax.sql.DataSource; +import java.sql.Connection; +import java.sql.DatabaseMetaData; +import java.sql.ResultSet; +import java.sql.SQLException; + +import static org.hadoop.ozone.recon.schema.ContainerSchemaDefinition.UNHEALTHY_CONTAINERS_TABLE_NAME; +import static org.jooq.impl.DSL.field; +import static org.jooq.impl.DSL.name; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +/** + * Test class for InitialConstraintUpgradeAction. + */ +public class TestInitialConstraintUpgradeAction extends AbstractReconSqlDBTest { + + private InitialConstraintUpgradeAction upgradeAction; + private DSLContext dslContext; + private ReconStorageContainerManagerFacade mockScmFacade; + + @BeforeEach + public void setUp() throws SQLException { + // Initialize the DSLContext + dslContext = getDslContext(); + + // Initialize the upgrade action + upgradeAction = new InitialConstraintUpgradeAction(); + + // Mock the SCM facade to provide the DataSource + mockScmFacade = mock(ReconStorageContainerManagerFacade.class); + DataSource dataSource = getInjector().getInstance(DataSource.class); + when(mockScmFacade.getDataSource()).thenReturn(dataSource); + + // Set the DataSource and DSLContext directly + upgradeAction.setDataSource(dataSource); + upgradeAction.setDslContext(dslContext); + + // Check if the table already exists + try (Connection conn = dataSource.getConnection()) { + DatabaseMetaData dbMetaData = conn.getMetaData(); + ResultSet tables = dbMetaData.getTables(null, null, UNHEALTHY_CONTAINERS_TABLE_NAME, null); + if (!tables.next()) { + // Create the initial table if it does not exist + dslContext.createTable(UNHEALTHY_CONTAINERS_TABLE_NAME) + .column("container_id", org.jooq.impl.SQLDataType.BIGINT + .nullable(false)) + .column("container_state", org.jooq.impl.SQLDataType.VARCHAR(16) + .nullable(false)) + .constraint(DSL.constraint("pk_container_id") + .primaryKey("container_id", "container_state")) + .execute(); + } + } + } + + @Test + public void testUpgradeAppliesConstraintModificationForAllStates() throws SQLException { + // Run the upgrade action + upgradeAction.execute(mockScmFacade); + + // Iterate over all valid states and insert records + for (ContainerSchemaDefinition.UnHealthyContainerStates state : + ContainerSchemaDefinition.UnHealthyContainerStates.values()) { + dslContext.insertInto(DSL.table(UNHEALTHY_CONTAINERS_TABLE_NAME)) + .columns( + field(name("container_id")), + field(name("container_state")), + field(name("in_state_since")), + field(name("expected_replica_count")), + field(name("actual_replica_count")), + field(name("replica_delta")), + field(name("reason")) + ) + .values( + System.currentTimeMillis(), // Unique container_id for each record + state.name(), System.currentTimeMillis(), 3, 2, 1, "Replica count mismatch" + ) + .execute(); + } + + // Verify that the number of inserted records matches the number of enum values + int count = dslContext.fetchCount(DSL.table(UNHEALTHY_CONTAINERS_TABLE_NAME)); + assertEquals(ContainerSchemaDefinition.UnHealthyContainerStates.values().length, + count, "Expected one record for each valid state"); + + // Try inserting an invalid state (should fail due to constraint) + assertThrows(org.jooq.exception.DataAccessException.class, () -> + dslContext.insertInto(DSL.table(UNHEALTHY_CONTAINERS_TABLE_NAME)) + .columns( + field(name("container_id")), + field(name("container_state")), + field(name("in_state_since")), + field(name("expected_replica_count")), + field(name("actual_replica_count")), + field(name("replica_delta")), + field(name("reason")) + ) + .values(999L, "INVALID_STATE", System.currentTimeMillis(), 3, 2, 1, + "Invalid state test").execute(), + "Inserting an invalid container_state should fail due to the constraint"); + } + + @Test + public void testInsertionWithNullContainerState() { + assertThrows(org.jooq.exception.DataAccessException.class, () -> { + dslContext.insertInto(DSL.table(UNHEALTHY_CONTAINERS_TABLE_NAME)) + .columns( + field(name("container_id")), + field(name("container_state")), + field(name("in_state_since")), + field(name("expected_replica_count")), + field(name("actual_replica_count")), + field(name("replica_delta")), + field(name("reason")) + ) + .values( + 100L, // container_id + null, // container_state is NULL + System.currentTimeMillis(), 3, 2, 1, "Testing NULL state" + ) + .execute(); + }, "Inserting a NULL container_state should fail due to the NOT NULL constraint"); + } + + @Test + public void testDuplicatePrimaryKeyInsertion() throws SQLException { + // Insert the first record + dslContext.insertInto(DSL.table(UNHEALTHY_CONTAINERS_TABLE_NAME)) + .columns( + field(name("container_id")), + field(name("container_state")), + field(name("in_state_since")), + field(name("expected_replica_count")), + field(name("actual_replica_count")), + field(name("replica_delta")), + field(name("reason")) + ) + .values(200L, "MISSING", System.currentTimeMillis(), 3, 2, 1, "First insertion" + ) + .execute(); + + // Try inserting a duplicate record with the same primary key + assertThrows(org.jooq.exception.DataAccessException.class, () -> { + dslContext.insertInto(DSL.table(UNHEALTHY_CONTAINERS_TABLE_NAME)) + .columns( + field(name("container_id")), + field(name("container_state")), + field(name("in_state_since")), + field(name("expected_replica_count")), + field(name("actual_replica_count")), + field(name("replica_delta")), + field(name("reason")) + ) + .values(200L, "MISSING", System.currentTimeMillis(), 3, 2, 1, "Duplicate insertion" + ) + .execute(); + }, "Inserting a duplicate primary key should fail due to the primary key constraint"); + } + +} diff --git a/hadoop-ozone/recon/src/test/java/org/apache/hadoop/ozone/recon/upgrade/TestReconLayoutVersionManager.java b/hadoop-ozone/recon/src/test/java/org/apache/hadoop/ozone/recon/upgrade/TestReconLayoutVersionManager.java index 1da4c48a94c..e1a949b6d15 100644 --- a/hadoop-ozone/recon/src/test/java/org/apache/hadoop/ozone/recon/upgrade/TestReconLayoutVersionManager.java +++ b/hadoop-ozone/recon/src/test/java/org/apache/hadoop/ozone/recon/upgrade/TestReconLayoutVersionManager.java @@ -21,6 +21,7 @@ import org.apache.hadoop.ozone.recon.ReconContext; import org.apache.hadoop.ozone.recon.ReconSchemaVersionTableManager; +import org.apache.hadoop.ozone.recon.scm.ReconStorageContainerManagerFacade; import org.mockito.InOrder; import org.mockito.MockedStatic; import org.junit.jupiter.api.BeforeEach; @@ -107,7 +108,8 @@ public void testInitializationWithMockedValues() { */ @Test public void testFinalizeLayoutFeaturesWithMockedValues() throws SQLException { - layoutVersionManager.finalizeLayoutFeatures(); + layoutVersionManager.finalizeLayoutFeatures(mock( + ReconStorageContainerManagerFacade.class)); // Verify that schema versions are updated for our custom features verify(schemaVersionTableManager, times(1)).updateSchemaVersion(1); @@ -137,7 +139,8 @@ public void testGetRegisteredFeaturesWithMockedValues() { @Test public void testNoLayoutFeatures() throws SQLException { mockedEnum.when(ReconLayoutFeature::values).thenReturn(new ReconLayoutFeature[]{}); - layoutVersionManager.finalizeLayoutFeatures(); + layoutVersionManager.finalizeLayoutFeatures(mock( + ReconStorageContainerManagerFacade.class)); verify(schemaVersionTableManager, never()).updateSchemaVersion(anyInt()); } @@ -155,8 +158,11 @@ public void testUpgradeActionFailure() throws Exception { when(feature1.getVersion()).thenReturn(1); ReconUpgradeAction action1 = mock(ReconUpgradeAction.class); + // Create a consistent mock instance for the SCM facade + ReconStorageContainerManagerFacade scmFacadeMock = mock(ReconStorageContainerManagerFacade.class); + // Simulate an exception being thrown during the upgrade action execution - doThrow(new RuntimeException("Upgrade failed")).when(action1).execute(); + doThrow(new RuntimeException("Upgrade failed")).when(action1).execute(scmFacadeMock); when(feature1.getAction(ReconUpgradeAction.UpgradeActionType.FINALIZE)) .thenReturn(Optional.of(action1)); @@ -165,9 +171,11 @@ public void testUpgradeActionFailure() throws Exception { // Execute the layout feature finalization try { - layoutVersionManager.finalizeLayoutFeatures(); + layoutVersionManager.finalizeLayoutFeatures(scmFacadeMock); } catch (Exception e) { + // Exception is expected, so it's fine to catch and ignore it here } + // Verify that schema version update was never called due to the exception verify(schemaVersionTableManager, never()).updateSchemaVersion(anyInt()); } @@ -203,14 +211,17 @@ public void testUpgradeActionExecutionOrder() throws Exception { // Mock the static values method to return custom features in a jumbled order mockedEnum.when(ReconLayoutFeature::values).thenReturn(new ReconLayoutFeature[]{feature2, feature3, feature1}); + // Create a consistent mock instance for SCM facade + ReconStorageContainerManagerFacade scmFacadeMock = mock(ReconStorageContainerManagerFacade.class); + // Execute the layout feature finalization - layoutVersionManager.finalizeLayoutFeatures(); + layoutVersionManager.finalizeLayoutFeatures(scmFacadeMock); // Verify that the actions were executed in the correct order using InOrder InOrder inOrder = inOrder(action1, action2, action3); - inOrder.verify(action1).execute(); // Should be executed first - inOrder.verify(action2).execute(); // Should be executed second - inOrder.verify(action3).execute(); // Should be executed third + inOrder.verify(action1).execute(scmFacadeMock); // Should be executed first + inOrder.verify(action2).execute(scmFacadeMock); // Should be executed second + inOrder.verify(action3).execute(scmFacadeMock); // Should be executed third } /** @@ -221,7 +232,8 @@ public void testUpgradeActionExecutionOrder() throws Exception { public void testNoUpgradeActionsNeeded() throws SQLException { when(schemaVersionTableManager.getCurrentSchemaVersion()).thenReturn(2); layoutVersionManager = new ReconLayoutVersionManager(schemaVersionTableManager, mock(ReconContext.class)); - layoutVersionManager.finalizeLayoutFeatures(); + layoutVersionManager.finalizeLayoutFeatures(mock( + ReconStorageContainerManagerFacade.class)); verify(schemaVersionTableManager, never()).updateSchemaVersion(anyInt()); } @@ -252,8 +264,9 @@ public void testFinalizingNewFeatureWithoutReFinalizingPreviousFeatures() throws mockedEnum.when(ReconLayoutFeature::values).thenReturn(new ReconLayoutFeature[]{feature1, feature2}); + ReconStorageContainerManagerFacade scmFacadeMock = mock(ReconStorageContainerManagerFacade.class); // Finalize the first two features. - layoutVersionManager.finalizeLayoutFeatures(); + layoutVersionManager.finalizeLayoutFeatures(scmFacadeMock); // Verify that the schema versions for the first two features were updated. verify(schemaVersionTableManager, times(1)).updateSchemaVersion(1); @@ -272,17 +285,17 @@ public void testFinalizingNewFeatureWithoutReFinalizingPreviousFeatures() throws when(schemaVersionTableManager.getCurrentSchemaVersion()).thenReturn(2); // Finalize again, but only feature 3 should be finalized. - layoutVersionManager.finalizeLayoutFeatures(); + layoutVersionManager.finalizeLayoutFeatures(scmFacadeMock); // Verify that the schema version for feature 3 was updated. verify(schemaVersionTableManager, times(1)).updateSchemaVersion(3); // Verify that action1 and action2 were not executed again. - verify(action1, times(1)).execute(); // Still should have been executed only once - verify(action2, times(1)).execute(); // Still should have been executed only once + verify(action1, times(1)).execute(scmFacadeMock); + verify(action2, times(1)).execute(scmFacadeMock); // Verify that the upgrade action for feature 3 was executed. - verify(action3, times(1)).execute(); + verify(action3, times(1)).execute(scmFacadeMock); } }