diff --git a/api/src/main/java/org/openmrs/api/AdministrationService.java b/api/src/main/java/org/openmrs/api/AdministrationService.java index 64b5f32bbe1b..5ccd782653b7 100644 --- a/api/src/main/java/org/openmrs/api/AdministrationService.java +++ b/api/src/main/java/org/openmrs/api/AdministrationService.java @@ -17,10 +17,12 @@ import org.openmrs.GlobalProperty; import org.openmrs.ImplementationId; +import org.openmrs.module.Module; import org.openmrs.OpenmrsObject; import org.openmrs.User; import org.openmrs.annotation.Authorized; import org.openmrs.api.db.AdministrationDAO; +import org.openmrs.util.DatabaseUpdateException; import org.openmrs.util.HttpClient; import org.openmrs.util.OpenmrsConstants; import org.openmrs.util.PrivilegeConstants; @@ -420,4 +422,37 @@ public interface AdministrationService extends OpenmrsService { * Should return default common classes if no GPs defined */ List getSerializerWhitelistTypes(); + + /** + * Checks whether a core setup needs to be run due to a version change. + * + * @since 2.9.0 + * @return true if core setup should be executed because of a version change, false otherwise + */ + boolean isCoreSetupOnVersionChangeNeeded(); + + /** + * Checks whether a module setup needs to be run due to a version change. + * + * @since 2.9.0 + * @param moduleId the identifier of the module to check + * @return true if the module setup should be executed because of a version change, false otherwise + */ + boolean isModuleSetupOnVersionChangeNeeded(String moduleId); + + /** + * Executes the core setup procedures required after a core version change. + * + * @since 2.9.0 + * @throws DatabaseUpdateException if the core setup fails + */ + void runCoreSetupOnVersionChange() throws DatabaseUpdateException; + + /** + * Executes the setup procedures required for a module after a module version change. + * + * @since 2.9.0 + * @param module the module for which the setup should be executed + */ + void runModuleSetupOnVersionChange(Module module); } diff --git a/api/src/main/java/org/openmrs/api/context/Context.java b/api/src/main/java/org/openmrs/api/context/Context.java index 9ebf169a030a..ec940b4554b8 100644 --- a/api/src/main/java/org/openmrs/api/context/Context.java +++ b/api/src/main/java/org/openmrs/api/context/Context.java @@ -1009,20 +1009,22 @@ public static synchronized void startup(Properties props) throws DatabaseUpdateE // do any context database specific startup getContextDAO().startup(props); - // find/set/check whether the current database version is compatible - checkForDatabaseUpdates(props); + if (getAdministrationService().isCoreSetupOnVersionChangeNeeded()) { + log.info("Detected core version change. Running core setup hooks and Liquibase."); + getAdministrationService().runCoreSetupOnVersionChange(); + } // this should be first in the startup routines so that the application // data directory can be set from the runtime properties OpenmrsUtil.startup(props); - + openSession(); clearSession(); // add any privileges/roles that /must/ exist for openmrs to work // correctly. checkCoreDataset(); - + getContextDAO().setupSearchIndex(); // Loop over each module and startup each with these custom properties @@ -1282,39 +1284,6 @@ public static void checkCoreDataset() { OpenmrsConstants.GP_ALLERGEN_OTHER_NON_CODED_UUID)); } - /** - * Runs any needed updates on the current database if the user has the allow_auto_update runtime - * property set to true. If not set to true, then {@link #updateDatabase(Map)} must be called.
- *
- * If an {@link InputRequiredException} is thrown, a call to {@link #updateDatabase(Map)} is - * required with a mapping from question prompt to user answer. - * - * @param props the runtime properties - * @throws InputRequiredException if the {@link DatabaseUpdater} has determined that updates - * cannot continue without input from the user - * @see InputRequiredException#getRequiredInput() InputRequiredException#getRequiredInput() for - * the required question/datatypes - */ - private static void checkForDatabaseUpdates(Properties props) throws DatabaseUpdateException, InputRequiredException { - boolean updatesRequired; - try { - updatesRequired = DatabaseUpdater.updatesRequired(); - } - catch (Exception e) { - throw new DatabaseUpdateException("Unable to check if database updates are required", e); - } - - // this must be the first thing run in case it changes database mappings - if (updatesRequired) { - if (DatabaseUpdater.allowAutoUpdate()) { - DatabaseUpdater.executeChangelog(); - } else { - throw new DatabaseUpdateException( - "Database updates are required. Call Context.updateDatabase() before .startup() to continue."); - } - } - } - /** * Updates the openmrs database to the latest. This is only needed if using the API alone.
*
diff --git a/api/src/main/java/org/openmrs/api/db/hibernate/HibernateAdministrationDAO.java b/api/src/main/java/org/openmrs/api/db/hibernate/HibernateAdministrationDAO.java index 3a0a6a217541..53d250303b25 100644 --- a/api/src/main/java/org/openmrs/api/db/hibernate/HibernateAdministrationDAO.java +++ b/api/src/main/java/org/openmrs/api/db/hibernate/HibernateAdministrationDAO.java @@ -113,10 +113,16 @@ public GlobalProperty getGlobalPropertyObject(String propertyName) { query.where(condition); - return session.createQuery(query).uniqueResult(); - } else { - return session.get(GlobalProperty.class, propertyName); + GlobalProperty gp = session.createQuery(query).uniqueResult(); + if (gp != null) { + // GP may be null, but the session may contain an unflushed gp so + // we will do a final check with session.get below. It may happen, + // if flush is set to manual and running in a larger transaction. + return gp; + } } + + return session.get(GlobalProperty.class, propertyName); } @Override diff --git a/api/src/main/java/org/openmrs/api/impl/AdministrationServiceImpl.java b/api/src/main/java/org/openmrs/api/impl/AdministrationServiceImpl.java index 9ade8296d71a..8995f5e210cd 100644 --- a/api/src/main/java/org/openmrs/api/impl/AdministrationServiceImpl.java +++ b/api/src/main/java/org/openmrs/api/impl/AdministrationServiceImpl.java @@ -25,6 +25,7 @@ import java.util.List; import java.util.Locale; import java.util.Map; +import java.util.Objects; import java.util.Properties; import java.util.Set; import java.util.SortedMap; @@ -58,6 +59,8 @@ import org.openmrs.module.ModuleUtil; import org.openmrs.obs.ComplexData; import org.openmrs.person.PersonMergeLogData; +import org.openmrs.util.DatabaseUpdateException; +import org.openmrs.util.DatabaseUpdater; import org.openmrs.util.HttpClient; import org.openmrs.util.LocaleUtility; import org.openmrs.util.OpenmrsConstants; @@ -982,4 +985,122 @@ public static List> getSerializerDefaultWhitelistHierarchyTypes() { PersonMergeLogData.class); return types; } + + /** + * @see org.openmrs.api.AdministrationService#isCoreSetupOnVersionChangeNeeded() + */ + @Override + public boolean isCoreSetupOnVersionChangeNeeded() { + boolean forceSetup = Boolean.parseBoolean(Context.getRuntimeProperties().getProperty("force.setup", "false")); + if (forceSetup) { + return true; + } + + String stored = getStoredCoreVersion(); + String current = OpenmrsConstants.OPENMRS_VERSION_SHORT; + return !Objects.equals(stored, current); + } + + /** + * @see org.openmrs.api.AdministrationService#isModuleSetupOnVersionChangeNeeded(String) + */ + @Override + public boolean isModuleSetupOnVersionChangeNeeded(String moduleId) { + Module module = ModuleFactory.getModuleById(moduleId); + if (module == null) { + log.info("{} module is no longer installed, skipping setup", moduleId); + return false; + } + + if (isCoreSetupOnVersionChangeNeeded()) { + return true; + } + + String stored = getStoredModuleVersion(moduleId); + String current = module.getVersion(); + if (!Objects.equals(stored, current)) { + return true; + } else { + log.info("{} module did not change, skipping setup", moduleId); + return false; + } + } + + /** + * @see org.openmrs.api.AdministrationService#runCoreSetupOnVersionChange() + */ + @Override + @Transactional + public void runCoreSetupOnVersionChange() throws DatabaseUpdateException { + if (!ModuleFactory.getLoadedModules().isEmpty()) { + String prevCoreVersion = getStoredCoreVersion(); + + for (Module module : ModuleFactory.getLoadedModules()) { + String prevModuleVersion = getStoredModuleVersion(module.getModuleId()); + + module.getModuleActivator().setupOnVersionChangeBeforeSchemaChanges(prevCoreVersion, prevModuleVersion); + } + } + + boolean updatesRequired; + try { + updatesRequired = DatabaseUpdater.updatesRequired(); + } + catch (Exception e) { + throw new DatabaseUpdateException("Unable to check if database updates are required", e); + } + + // this must be the first thing run in case it changes database mappings + if (updatesRequired) { + if (!DatabaseUpdater.allowAutoUpdate()) { + throw new DatabaseUpdateException( + "Database updates are required. Call Context.updateDatabase() before .startup() to continue."); + } + } + + DatabaseUpdater.executeChangelog(); + + storeCoreVersion(); + } + + /** + * @see org.openmrs.api.AdministrationService#runModuleSetupOnVersionChange(Module) + */ + @Override + @Transactional + public void runModuleSetupOnVersionChange(Module module) { + if (module == null) { + return; + } + + String moduleId = module.getModuleId(); + String prevCoreVersion = getStoredCoreVersion(); + String prevModuleVersion = getStoredModuleVersion(moduleId); + + ModuleFactory.runLiquibaseForModule(module); + module.getModuleActivator().setupOnVersionChange(prevCoreVersion, prevModuleVersion); + + storeModuleVersion(moduleId, module.getVersion()); + } + + protected String getStoredCoreVersion() { + return dao.getGlobalProperty("core.version"); + } + + protected String getStoredModuleVersion(String moduleId) { + return dao.getGlobalProperty("module." + moduleId + ".version"); + } + + protected void storeCoreVersion() { + String propertyName = "core.version"; + GlobalProperty gp = new GlobalProperty(propertyName, OpenmrsConstants.OPENMRS_VERSION_SHORT, + "Saved core version for future restarts"); + dao.saveGlobalProperty(gp); + } + + protected void storeModuleVersion(String moduleId, String version) { + String propertyName = "module." + moduleId + ".version"; + GlobalProperty gp = new GlobalProperty(propertyName, version, "Saved module version for future restarts"); + dao.saveGlobalProperty(gp); + } } diff --git a/api/src/main/java/org/openmrs/module/BaseModuleActivator.java b/api/src/main/java/org/openmrs/module/BaseModuleActivator.java index f1c1eda0233f..3d29a936c338 100644 --- a/api/src/main/java/org/openmrs/module/BaseModuleActivator.java +++ b/api/src/main/java/org/openmrs/module/BaseModuleActivator.java @@ -61,4 +61,17 @@ public void willStart() { public void willStop() { } + /** + * @see org.openmrs.module.ModuleActivator#setupOnVersionChangeBeforeSchemaChanges(String, String) + */ + @Override + public void setupOnVersionChangeBeforeSchemaChanges(String previousCoreVersion, String previousModuleVersion) { + } + + /** + * @see org.openmrs.module.ModuleActivator#setupOnVersionChange(String, String) + */ + @Override + public void setupOnVersionChange(String previousCoreVersion, String previousModuleVersion) { + } } diff --git a/api/src/main/java/org/openmrs/module/ModuleActivator.java b/api/src/main/java/org/openmrs/module/ModuleActivator.java index 434d7b8b1acb..00294153d3f9 100644 --- a/api/src/main/java/org/openmrs/module/ModuleActivator.java +++ b/api/src/main/java/org/openmrs/module/ModuleActivator.java @@ -56,4 +56,22 @@ public interface ModuleActivator { */ public void stopped(); + /** + * Called before Liquibase runs, but only if core or this module version changed. + * + * @param previousCoreVersion previous core version or null if first install + * @param previousModuleVersion previous module version or null if first install + * @since 2.9.0 + */ + default void setupOnVersionChangeBeforeSchemaChanges(String previousCoreVersion, String previousModuleVersion) {} + + /** + * Called after Liquibase runs, but only if core or this module version changed. + * + * @param previousCoreVersion previous core version or null if first install + * @param previousModuleVersion previous module version or null if first install + * @since 2.9.0 + */ + default void setupOnVersionChange(String previousCoreVersion, String previousModuleVersion) {} + } diff --git a/api/src/main/java/org/openmrs/module/ModuleFactory.java b/api/src/main/java/org/openmrs/module/ModuleFactory.java index 995978e73f14..5f95152c33e2 100644 --- a/api/src/main/java/org/openmrs/module/ModuleFactory.java +++ b/api/src/main/java/org/openmrs/module/ModuleFactory.java @@ -691,8 +691,10 @@ public static Module startModuleInternal(Module module, boolean isOpenmrsStartup Context.removeProxyPrivilege(""); } - // run module's optional liquibase.xml immediately after sqldiff.xml - runLiquibase(module); + if (Context.getAdministrationService().isModuleSetupOnVersionChangeNeeded(module.getModuleId())) { + log.info("Module {} changed, running setup.", module.getModuleId()); + Context.getAdministrationService().runModuleSetupOnVersionChange(module); + } // effectively mark this module as started successfully getStartedModulesMap().put(moduleId, module); @@ -750,7 +752,7 @@ public static Module startModuleInternal(Module module, boolean isOpenmrsStartup module.clearStartupError(); } catch (Exception e) { - log.warn("Error while trying to start module: " + moduleId, e); + log.error("Error while trying to start module: {}", moduleId, e); module.setStartupErrorMessage("Error while trying to start module", e); notifySuperUsersAboutModuleFailure(module); // undo all of the actions in startup @@ -766,7 +768,7 @@ public static Module startModuleInternal(Module module, boolean isOpenmrsStartup catch (Exception e2) { // this will probably occur about the same place as the // error in startup - log.debug("Error while stopping module: " + moduleId, e2); + log.debug("Error while stopping module: {}", moduleId, e2); } } @@ -939,6 +941,14 @@ private static void runDiff(Module module, String version, String sql) { } } + + /** + * This is a convenience method that exposes the private {@link #runLiquibase(Module)} method. + * @since 2.9.0 + */ + public static void runLiquibaseForModule(Module module) { + runLiquibase(module); + } /** * Execute all not run changeSets in liquibase.xml for the given module diff --git a/api/src/main/java/org/openmrs/util/DatabaseUpdater.java b/api/src/main/java/org/openmrs/util/DatabaseUpdater.java index 9eb507a15337..f5336454ec26 100644 --- a/api/src/main/java/org/openmrs/util/DatabaseUpdater.java +++ b/api/src/main/java/org/openmrs/util/DatabaseUpdater.java @@ -56,6 +56,8 @@ import java.nio.charset.StandardCharsets; import java.sql.Connection; import java.sql.DriverManager; +import java.sql.PreparedStatement; +import java.sql.ResultSet; import java.sql.SQLException; import java.util.ArrayList; import java.util.Arrays; @@ -66,6 +68,7 @@ import java.util.LinkedList; import java.util.List; import java.util.Map; +import java.util.Objects; import java.util.Properties; import java.util.Set; @@ -278,13 +281,34 @@ public static List executeChangelog(String changeLogFile, Contexts conte } /** - * Ask Liquibase if it needs to do any updates. + * Determine if Liquibase updates are required. If OpenMRS Core version did not change, then do not run any checks + * unless force.setup runtime property is set to true. * * @return true/false whether database updates are required * @throws Exception when an exception is raised while processing Liquibase changelog files */ public static boolean updatesRequired() throws Exception { - log.debug("checking for updates"); + String storedCoreVersion = null; + // Using raw SQL to not rely on Context being initialized for this check. + try (Connection con = getConnection()) { + try (PreparedStatement ps = con.prepareStatement("SELECT property_value from global_property " + + "where property = ?")) { + ps.setString(1, "core.version"); + ResultSet resultSet = ps.executeQuery(); + if (resultSet.next()) { + storedCoreVersion = resultSet.getString(1); + } + } + } + String currentCoreVersion = OpenmrsConstants.OPENMRS_VERSION_SHORT; + boolean forceSetup = Boolean.parseBoolean(Context.getRuntimeProperties().getProperty("force.setup", "false")); + + if (!forceSetup && Objects.equals(storedCoreVersion, currentCoreVersion)) { + log.info("No core version changed. Skipping database updates."); + return false; + } + + log.debug("Checking for database updates"); List changesets = getUnrunDatabaseChanges(new DatabaseUpdaterLiquibaseProvider()); // if the db is locked, it means there was a crash @@ -295,7 +319,7 @@ public static boolean updatesRequired() throws Exception { // if there is a db lock but there are no db changes we undo the // lock DatabaseUpdater.releaseDatabaseLock(); - log.debug("db lock found and released automatically"); + log.debug("DB lock found and released automatically"); return false; } diff --git a/api/src/test/java/org/openmrs/annotation/StartModuleAnnotationJUnit4Test.java b/api/src/test/java/org/openmrs/annotation/StartModuleAnnotationJUnit4Test.java index 18d37b00d8e3..f95533a5b727 100644 --- a/api/src/test/java/org/openmrs/annotation/StartModuleAnnotationJUnit4Test.java +++ b/api/src/test/java/org/openmrs/annotation/StartModuleAnnotationJUnit4Test.java @@ -21,7 +21,6 @@ public class StartModuleAnnotationJUnit4Test extends BaseContextSensitiveTest { @Test public void shouldStartModules() throws ClassNotFoundException { - Class test1ServiceClass = Context.loadClass("org.openmrs.module.test1.api.Test1Service"); Class test2ServiceClass = Context.loadClass("org.openmrs.module.test2.api.Test2Service"); assertNotNull(test1ServiceClass); diff --git a/api/src/test/java/org/openmrs/annotation/StartModuleAnnotationReuseJUnit4Test.java b/api/src/test/java/org/openmrs/annotation/StartModuleAnnotationReuseJUnit4Test.java index 02d81c4e3bbd..0a994974ae5c 100644 --- a/api/src/test/java/org/openmrs/annotation/StartModuleAnnotationReuseJUnit4Test.java +++ b/api/src/test/java/org/openmrs/annotation/StartModuleAnnotationReuseJUnit4Test.java @@ -16,8 +16,9 @@ @StartModule("org/openmrs/module/include/test1-1.0-SNAPSHOT.omod") public class StartModuleAnnotationReuseJUnit4Test extends BaseContextSensitiveTest { - @Test - public void shouldPass() { - Assert.assertTrue(true); - } + + @Test + public void shouldPass() { + Assert.assertTrue(true); + } } diff --git a/api/src/test/java/org/openmrs/api/AdministrationServiceTest.java b/api/src/test/java/org/openmrs/api/AdministrationServiceTest.java index 92e12ce4c796..3dc468c6258e 100644 --- a/api/src/test/java/org/openmrs/api/AdministrationServiceTest.java +++ b/api/src/test/java/org/openmrs/api/AdministrationServiceTest.java @@ -27,6 +27,7 @@ import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; import java.util.ArrayList; import java.util.Arrays; @@ -51,6 +52,8 @@ import org.openmrs.customdatatype.datatype.DateDatatype; import org.openmrs.messagesource.MutableMessageSource; import org.openmrs.messagesource.impl.MutableResourceBundleMessageSource; +import org.openmrs.module.Module; +import org.openmrs.module.ModuleActivator; import org.openmrs.test.jupiter.BaseContextSensitiveTest; import org.openmrs.util.HttpClient; import org.openmrs.util.LocaleUtility; @@ -1160,4 +1163,28 @@ public void getSerializerWhitelistTypes_shouldReturnDefaultCommonClassesIfNoGPS( "hierarchyOf:org.openmrs.messagesource.PresentationMessage", "hierarchyOf:org.openmrs.person.PersonMergeLogData")); } + + @Test + public void runModuleSetupOnVersionChange_shouldExecuteLiquibaseAndStoreNewVersion() { + // old version + adminService.setGlobalProperty("module.testmodule.version", "1.0.0"); + assertEquals("1.0.0", adminService.getGlobalProperty("module.testmodule.version")); + + String previousModuleVersion = "1.0.0"; + String previousCoreVersion = null; + + Module module = new Module("Test Module"); + module.setModuleId("testmodule"); + module.setVersion("1.2.3"); + + ModuleActivator activator = mock(ModuleActivator.class); + module.setModuleActivator(activator); + + adminService.runModuleSetupOnVersionChange(module); + + assertEquals("1.2.3", adminService.getGlobalProperty("module.testmodule.version")); + + // verify hook methods must be called + verify(activator).setupOnVersionChange(previousCoreVersion, previousModuleVersion); + } } diff --git a/api/src/test/java/org/openmrs/module/ModuleTestData.java b/api/src/test/java/org/openmrs/module/ModuleTestData.java index 9ed0ee3e5a2d..17141ca92f51 100644 --- a/api/src/test/java/org/openmrs/module/ModuleTestData.java +++ b/api/src/test/java/org/openmrs/module/ModuleTestData.java @@ -27,6 +27,10 @@ public class ModuleTestData { private Map stoppedCallCount = new HashMap<>(); + private Map setupOnVersionChangeBeforeSchemaChangesCallCount = new HashMap<>(); + + private Map setupOnVersionChangeCallCount = new HashMap<>(); + private Map willRefreshContextCallTime = new HashMap<>(); private Map contextRefreshedCallTime = new HashMap<>(); @@ -39,6 +43,10 @@ public class ModuleTestData { private Map stoppedCallTime = new HashMap<>(); + private Map setupOnVersionChangeBeforeSchemaChangesCallTime = new HashMap<>(); + + private Map setupOnVersionChangeCallTime = new HashMap<>(); + private ModuleTestData() { } @@ -59,6 +67,8 @@ public synchronized void init(String moduleId) { startedCallCount.put(moduleId, 0); willStopCallCount.put(moduleId, 0); stoppedCallCount.put(moduleId, 0); + setupOnVersionChangeBeforeSchemaChangesCallCount.put(moduleId, 0); + setupOnVersionChangeCallCount.put(moduleId, 0); willRefreshContextCallTime.put(moduleId, 0L); contextRefreshedCallTime.put(moduleId, 0L); @@ -66,6 +76,8 @@ public synchronized void init(String moduleId) { startedCallTime.put(moduleId, 0L); willStopCallTime.put(moduleId, 0L); stoppedCallTime.put(moduleId, 0L); + setupOnVersionChangeBeforeSchemaChangesCallTime.put(moduleId, 0L); + setupOnVersionChangeCallTime.put(moduleId, 0L); } public synchronized Integer getWillRefreshContextCallCount(String moduleId) { @@ -116,6 +128,22 @@ public synchronized Integer getStoppedCallCount(String moduleId) { return count; } + public synchronized Integer getSetupOnVersionChangeBeforeSchemaChangesCallCount(String moduleId) { + Integer count = setupOnVersionChangeBeforeSchemaChangesCallCount.get(moduleId); + if (count == null) { + count = 0; + } + return count; + } + + public synchronized Integer getSetupOnVersionChangeCallCount(String moduleId) { + Integer count = setupOnVersionChangeCallCount.get(moduleId); + if (count == null) { + count = 0; + } + return count; + } + public synchronized void willRefreshContext(String moduleId) { willRefreshContextCallTime.put(moduleId, new Date().getTime()); @@ -176,6 +204,26 @@ public synchronized void stopped(String moduleId) { stoppedCallCount.put(moduleId, count + 1); } + public synchronized void setupOnVersionChangeBeforeSchemaChanges(String moduleId) { + setupOnVersionChangeBeforeSchemaChangesCallTime.put(moduleId, new Date().getTime()); + + Integer count = setupOnVersionChangeBeforeSchemaChangesCallCount.get(moduleId); + if (count == null) { + count = 0; + } + setupOnVersionChangeBeforeSchemaChangesCallCount.put(moduleId, count + 1); + } + + public synchronized void setupOnVersionChange(String moduleId) { + setupOnVersionChangeCallTime.put(moduleId, new Date().getTime()); + + Integer count = setupOnVersionChangeCallCount.get(moduleId); + if (count == null) { + count = 0; + } + setupOnVersionChangeCallCount.put(moduleId, count + 1); + } + public synchronized Long getWillRefreshContextCallTime(String moduleId) { return willRefreshContextCallTime.get(moduleId); } @@ -199,4 +247,12 @@ public synchronized Long getWillStopCallTime(String moduleId) { public synchronized Long getStoppedCallTime(String moduleId) { return stoppedCallTime.get(moduleId); } + + public synchronized Long getSetupOnVersionChangeBeforeSchemaChangesCallTime(String moduleId) { + return setupOnVersionChangeBeforeSchemaChangesCallTime.get(moduleId); + } + + public synchronized Long getSetupOnVersionChangeCallTime(String moduleId) { + return setupOnVersionChangeCallTime.get(moduleId); + } } diff --git a/api/src/test/java/org/openmrs/test/BaseContextSensitiveTest.java b/api/src/test/java/org/openmrs/test/BaseContextSensitiveTest.java index fbf6cd9bcc86..a4c0870a26de 100644 --- a/api/src/test/java/org/openmrs/test/BaseContextSensitiveTest.java +++ b/api/src/test/java/org/openmrs/test/BaseContextSensitiveTest.java @@ -315,7 +315,7 @@ public Properties getRuntimeProperties() { // properties if (useInMemoryDatabase()) { runtimeProperties.setProperty(Environment.DIALECT, H2Dialect.class.getName()); - String url = "jdbc:h2:mem:openmrs;DB_CLOSE_DELAY=30;LOCK_TIMEOUT=10000"; + String url = "jdbc:h2:mem:openmrs;DB_CLOSE_DELAY=30;LOCK_TIMEOUT=10000;IGNORECASE=TRUE"; runtimeProperties.setProperty(Environment.URL, url); runtimeProperties.setProperty(Environment.DRIVER, "org.h2.Driver"); runtimeProperties.setProperty(Environment.USER, "sa"); @@ -925,7 +925,7 @@ public void baseSetupWithStandardDataAndAuthentication() throws SQLException { Context.openSession(); } - // The skipBaseSetup flag is controlled by the @SkipBaseSetup annotation. if (useInMemoryDatabase()) { + // The skipBaseSetup flag is controlled by the @SkipBaseSetup annotation. if (!skipBaseSetup) { if (!isBaseSetup) { diff --git a/api/src/test/java/org/openmrs/test/jupiter/BaseContextSensitiveTest.java b/api/src/test/java/org/openmrs/test/jupiter/BaseContextSensitiveTest.java index d0496db738bc..64729fc1bf00 100644 --- a/api/src/test/java/org/openmrs/test/jupiter/BaseContextSensitiveTest.java +++ b/api/src/test/java/org/openmrs/test/jupiter/BaseContextSensitiveTest.java @@ -953,7 +953,7 @@ public void baseSetupWithStandardDataAndAuthentication() throws SQLException { Context.openSession(); } - // The skipBaseSetup flag is controlled by the @SkipBaseSetup annotation. if (useInMemoryDatabase()) { + // The skipBaseSetup flag is controlled by the @SkipBaseSetup annotation. if (!skipBaseSetup) { if (!isBaseSetup) { diff --git a/api/src/test/java/org/openmrs/test/jupiter/StartModuleExecutionListener.java b/api/src/test/java/org/openmrs/test/jupiter/StartModuleExecutionListener.java index 1a0dd45e2dd1..20ce4d6986bd 100644 --- a/api/src/test/java/org/openmrs/test/jupiter/StartModuleExecutionListener.java +++ b/api/src/test/java/org/openmrs/test/jupiter/StartModuleExecutionListener.java @@ -71,7 +71,6 @@ public void prepareTestInstance(TestContext testContext) throws Exception { // if the developer listed some modules with the @StartModule annotation on the class if (startModuleAnnotation != null) { - if (!lastClassRun.equals(testContext.getTestClass().getSimpleName())) { // mark this with our class so that the services are only restarted once lastClassRun = testContext.getTestClass().getSimpleName(); @@ -79,7 +78,6 @@ public void prepareTestInstance(TestContext testContext) throws Exception { if (!Context.isSessionOpen()) Context.openSession(); - ModuleUtil.shutdown(); // load the omods that the dev defined for this class diff --git a/test-suite/module/api/src/main/java/org/openmrs/module/testmodule/TestModuleActivator.java b/test-suite/module/api/src/main/java/org/openmrs/module/testmodule/TestModuleActivator.java index e4e7f54412c7..6ee58eb7ce65 100644 --- a/test-suite/module/api/src/main/java/org/openmrs/module/testmodule/TestModuleActivator.java +++ b/test-suite/module/api/src/main/java/org/openmrs/module/testmodule/TestModuleActivator.java @@ -62,5 +62,19 @@ public void willStop() { public void stopped() { log.info("Test Module stopped"); } + + /** + * @see ModuleActivator#setupOnVersionChangeBeforeSchemaChanges(String, String) + */ + public void setupOnVersionChangeBeforeSchemaChanges(String previousCoreVersion, String previousModuleVersion) { + log.info("Preparing upgrade for module from: " + previousModuleVersion); + } + + /** + * @see ModuleActivator#setupOnVersionChange(String, String) + */ + public void setupOnVersionChange(String previousCoreVersion, String previousModuleVersion) { + log.info("Running post-schema setup after upgrade"); + } } diff --git a/test-suite/performance/src/test/java/org/openmrs/StartupPerformanceIT.java b/test-suite/performance/src/test/java/org/openmrs/StartupPerformanceIT.java index 2e6917d1191f..af2b83aa0f69 100644 --- a/test-suite/performance/src/test/java/org/openmrs/StartupPerformanceIT.java +++ b/test-suite/performance/src/test/java/org/openmrs/StartupPerformanceIT.java @@ -27,13 +27,17 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.List; +import java.util.concurrent.atomic.AtomicLong; import java.util.function.Consumer; import org.jetbrains.annotations.NotNull; +import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; import org.openmrs.test.Containers; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import org.springframework.util.unit.DataSize; +import org.springframework.util.unit.DataUnit; import org.testcontainers.containers.BindMode; import org.testcontainers.containers.GenericContainer; import org.testcontainers.containers.MariaDBContainer; @@ -42,6 +46,7 @@ import org.testcontainers.containers.output.OutputFrame; import org.testcontainers.containers.output.Slf4jLogConsumer; import org.testcontainers.containers.wait.strategy.Wait; +import org.testcontainers.images.PullPolicy; import org.testcontainers.junit.jupiter.Container; import org.testcontainers.junit.jupiter.Testcontainers; import org.testcontainers.utility.MountableFile; @@ -66,22 +71,24 @@ public class StartupPerformanceIT { .withNetworkAliases("mariadb"); @Test + @Disabled("No need to test openmrs-core alone since running with modules is enough") public void shouldFailIfStartupTimeOfCoreIncreases() throws SQLException, IOException { compareStartupPerformance("openmrs/openmrs-core:" + FROM_VERSION, - "openmrs/openmrs-core:" + TO_VERSION, Duration.ofSeconds(0)); + "openmrs/openmrs-core:" + TO_VERSION, 0); } @Test public void shouldFailIfStartupTimeOfPlatformIncreases() throws SQLException, IOException { compareStartupPerformance("openmrs/openmrs-platform:" + FROM_VERSION, - "openmrs/openmrs-platform:" + TO_VERSION, Duration.ofSeconds(0)); + "openmrs/openmrs-platform:" + TO_VERSION, 0); } @Test public void shouldFailIfStartupTimeOfO3Increases() throws SQLException, IOException { - //Using O3 3.6.x as a reference, which is running on openmrs-core 2.8.x + // Using O3 3.6.x as a reference, which is running on openmrs-core 2.8.x + // 2.9.x should start at least 10% faster than 2.8.x compareStartupPerformance("openmrs/openmrs-reference-application-3-backend:3.6.x-no-demo", - "openmrs/openmrs-reference-application-3-backend:3.6.x-no-demo", Duration.ofSeconds(0)); + "openmrs/openmrs-reference-application-3-backend:3.6.x-no-demo", -10); } @@ -121,19 +128,34 @@ public void shouldFailIfStartupTimeOfO3Increases() throws SQLException, IOExcept * Consumes only lines starting with a level according to the OpenMRS Log4j2 configuration. * It won't accept Tomcat or startup bash script logs, which do not match the pattern and are * wrongly interpreted as errors. + *

+ * It records the time when the first log entry is received to measure the startup time of an application + * without container creation. */ - public static class OpenMRSLogConsumer extends Slf4jLogConsumer { + public static class LogConsumer extends Slf4jLogConsumer { + + private final AtomicLong startTime = new AtomicLong(0); - public OpenMRSLogConsumer(Logger logger) { + public LogConsumer(Logger logger) { super(logger); + withSeparateOutputStreams(); } - public OpenMRSLogConsumer(Logger logger, boolean separateOutputStreams) { + public LogConsumer(Logger logger, boolean separateOutputStreams) { super(logger, separateOutputStreams); } + + public Long getStartTime() { + return startTime.get(); + } + public void resetStartTime() { + startTime.set(0); + } + @Override public void accept(OutputFrame outputFrame) { + startTime.compareAndSet(0, System.nanoTime()); if (!outputFrame.getUtf8String().startsWith("ERROR") || !outputFrame.getUtf8String().startsWith("WARN") || !outputFrame.getUtf8String().startsWith("INFO") || !outputFrame.getUtf8String().startsWith("DEBUG") || !outputFrame.getUtf8String().startsWith("TRACE")) { @@ -148,60 +170,68 @@ public void accept(OutputFrame outputFrame) { * * @param fromImage docker distro image to compare * @param toImage docker distro image to compare with a war file replaced with the one from the current build - * @param timeDiffAccepted set to expected speed-up or slow-down between versions + * @param diffInPercent set to expected speed-up or slow-down between versions * @throws SQLException if fails to access DB */ - private void compareStartupPerformance(String fromImage, String toImage, Duration timeDiffAccepted) throws IOException, SQLException { + private void compareStartupPerformance(String fromImage, String toImage, int diffInPercent) throws IOException, SQLException { clearDB(); - Consumer logConsumer = new OpenMRSLogConsumer(containerLogger).withSeparateOutputStreams(); + LogConsumer logConsumer = new LogConsumer(containerLogger); long fromContainerStartupTime; long toContainerStartupTime; File tempDirectory = Files.createTempDirectory("test").toFile(); - try (GenericContainer fromContainer = newOpenMRSContainer(fromImage, logConsumer)) { - fromContainer.addFileSystemBind(tempDirectory.getAbsolutePath(), "/openmrs/data/", BindMode.READ_WRITE, - SelinuxContext.SHARED); - + try (GenericContainer fromContainer = newOpenMRSContainer(fromImage, tempDirectory, logConsumer)) { // Do not measure initial setup + logConsumer.resetStartTime(); fromContainer.start(); + long startupTime = Duration.ofNanos(System.nanoTime() - logConsumer.getStartTime()).toMillis(); + logger.info("{} installed in {}ms", fromContainer.getDockerImageName(), startupTime); fromContainer.stop(); - fromContainerStartupTime = measureMeanStartupTime(fromContainer); + fromContainerStartupTime = measureMeanStartupTime(fromContainer, logConsumer); // Overwrite the war file from the image to the one that was just built instead of using an image created // on the fly from code with ImageFromDockerfile. // ImageFromDockerfile runs into some issue when building an image and there is no easy way to debug. - try (GenericContainer toContainer = newOpenMRSContainer(toImage, logConsumer)) { + try (GenericContainer toContainer = newOpenMRSContainer(toImage, tempDirectory, logConsumer)) { //toContainer is re-using DB and OpenMRS application data to do upgrade instead of fresh install - toContainer.addFileSystemBind(tempDirectory.getAbsolutePath(), "/openmrs/data/", BindMode.READ_WRITE, - SelinuxContext.SHARED); assertThat("The test must run after webapp is packaged", Files.exists(Paths.get("../../webapp/target/openmrs.war")), is(true)); toContainer.withCopyFileToContainer(MountableFile.forHostPath("../../webapp/target/openmrs.war"), "/openmrs/distribution/openmrs_core/openmrs.war"); // Do not measure initial setup + logConsumer.resetStartTime(); toContainer.start(); + startupTime = Duration.ofNanos(System.nanoTime() - logConsumer.getStartTime()).toMillis(); + logger.info("{} upgraded in {}ms", toContainer.getDockerImageName(), startupTime); toContainer.stop(); - toContainerStartupTime = measureMeanStartupTime(toContainer); + toContainerStartupTime = measureMeanStartupTime(toContainer, logConsumer); } } finally { tempDirectory.delete(); } + + long diff = toContainerStartupTime - fromContainerStartupTime; + int actualDiffInPercent = diff == 0 ? 0 : Math.round(((float) diff) / fromContainerStartupTime * 100); + String describeDifference = actualDiffInPercent == 0 ? "almost the same" : + String.format("%d%% (%dms) %s", Math.abs(actualDiffInPercent), Math.abs(diff), (diff < 0 ? "faster" : "slower")); + logger.info("{} started up on average in {}ms, while {} started up in {}ms, which is {}", fromImage, + fromContainerStartupTime, toImage, toContainerStartupTime, describeDifference); + + // 15% is an accepted variation between runs + long acceptedDiff = Math.round(((double) fromContainerStartupTime) * (diffInPercent + 15) / 100); - long diff = Duration.ofNanos(toContainerStartupTime).getSeconds() - Duration.ofNanos(fromContainerStartupTime).getSeconds(); - logger.info("{} started up in {}s, while {} started up in {}s with the latter starting {} by {}s", fromImage, - Duration.ofNanos(fromContainerStartupTime).getSeconds(), toImage, - Duration.ofNanos(toContainerStartupTime).getSeconds(), diff < 0 ? "faster" : "slower", Math.abs(diff)); - - assertThat(diff, lessThan(timeDiffAccepted.getSeconds() + 10)); //10s is an accepted variation between runs + assertThat("Fail if slower than " + diffInPercent + "% + 15% (accepted variation between runs)", diff, lessThan(acceptedDiff)); } - - private long measureMeanStartupTime(GenericContainer container) { + + private long measureMeanStartupTime(GenericContainer container, LogConsumer logConsumer) { List times = new ArrayList<>(); for (int i = 0; i < 3; i++) { - long start = System.nanoTime(); + logConsumer.resetStartTime(); container.start(); - times.add(System.nanoTime() - start); + long startupTime = Duration.ofNanos(System.nanoTime() - logConsumer.getStartTime()).toMillis(); + logger.info("{} started up in {}ms", container.getDockerImageName(), startupTime); + times.add(startupTime); container.stop(); } return (long) times.stream().mapToLong(Long::longValue).average().orElse(0); @@ -218,17 +248,27 @@ private void clearDB() throws SQLException { } } - private GenericContainer newOpenMRSContainer(String image, Consumer logConsumer) { - return new GenericContainer<>(image) - .withExposedPorts(8080) - .withNetwork(dbContainer.getNetwork()) - .withEnv("OMRS_DB", "mariadb") - .withEnv("OMRS_DB_HOSTNAME", "mariadb") - .withEnv("OMRS_DB_NAME", dbContainer.getDatabaseName()) - .withEnv("OMRS_DB_USERNAME", dbContainer.getUsername()) - .withEnv("OMRS_DB_PASSWORD", dbContainer.getPassword()) - .withEnv("OMRS_DB_PORT", "3306") - .waitingFor(Wait.forHttp("/openmrs/health/started").withStartupTimeout(Duration.ofMinutes(30))) - .withLogConsumer(logConsumer); + private GenericContainer newOpenMRSContainer(String image, File dataDir, Consumer logConsumer) { + GenericContainer container = new GenericContainer<>(image) + .withImagePullPolicy(PullPolicy.alwaysPull()) + .withExposedPorts(8080) + .withNetwork(dbContainer.getNetwork()) + .withEnv("OMRS_DB", "mariadb") + .withEnv("OMRS_DB_HOSTNAME", "mariadb") + .withEnv("OMRS_DB_NAME", dbContainer.getDatabaseName()) + .withEnv("OMRS_DB_USERNAME", dbContainer.getUsername()) + .withEnv("OMRS_DB_PASSWORD", dbContainer.getPassword()) + .withEnv("OMRS_DB_PORT", "3306") + .withCreateContainerCmdModifier(cmd -> { + cmd.getHostConfig() // Simulate lower specs + .withMemory(DataSize.of(2, DataUnit.GIGABYTES).toBytes()) + .withCpuCount(1L); + }) + .waitingFor(Wait.forHttp("/openmrs/health/started").withStartupTimeout(Duration.ofMinutes(30))) + .withLogConsumer(logConsumer); + container.addFileSystemBind(dataDir.getAbsolutePath(), "/openmrs/data/", BindMode.READ_WRITE, + SelinuxContext.SHARED); + + return container; } } diff --git a/web/src/main/java/org/openmrs/web/filter/update/UpdateFilter.java b/web/src/main/java/org/openmrs/web/filter/update/UpdateFilter.java index 3dd4e09c45d6..0f94cdea1be0 100644 --- a/web/src/main/java/org/openmrs/web/filter/update/UpdateFilter.java +++ b/web/src/main/java/org/openmrs/web/filter/update/UpdateFilter.java @@ -702,42 +702,45 @@ public void executing(ChangeSet changeSet, int numChangeSetsToRun) { } } + try { - setMessage("Updating the database to the latest version"); - - ChangeLogDetective changeLogDetective = ChangeLogDetective.getInstance(); - ChangeLogVersionFinder changeLogVersionFinder = new ChangeLogVersionFinder(); - - List changelogs = new ArrayList<>(); - List warnings = new ArrayList<>(); - - String version = changeLogDetective.getInitialLiquibaseSnapshotVersion(DatabaseUpdater.CONTEXT, - new DatabaseUpdaterLiquibaseProvider()); - - log.debug( - "updating the database with versions of liquibase-update-to-latest files greater than '{}'", - version); - - changelogs.addAll(changeLogVersionFinder - .getUpdateFileNames(changeLogVersionFinder.getUpdateVersionsGreaterThan(version))); - - log.debug("found applicable Liquibase update change logs: {}", changelogs); - - for (String changelog : changelogs) { - log.debug("applying Liquibase changelog '{}'", changelog); - - List currentWarnings = DatabaseUpdater.executeChangelog(changelog, - new PrintingChangeSetExecutorCallback("executing Liquibase changelog :" + changelog)); - - if (currentWarnings != null) { - warnings.addAll(currentWarnings); + if (DatabaseUpdater.updatesRequired()) { + setMessage("Updating the database to the latest version"); + + ChangeLogDetective changeLogDetective = ChangeLogDetective.getInstance(); + ChangeLogVersionFinder changeLogVersionFinder = new ChangeLogVersionFinder(); + + List changelogs = new ArrayList<>(); + List warnings = new ArrayList<>(); + + String version = changeLogDetective.getInitialLiquibaseSnapshotVersion(DatabaseUpdater.CONTEXT, + new DatabaseUpdaterLiquibaseProvider()); + + log.debug( + "updating the database with versions of liquibase-update-to-latest files greater than '{}'", + version); + + changelogs.addAll(changeLogVersionFinder + .getUpdateFileNames(changeLogVersionFinder.getUpdateVersionsGreaterThan(version))); + + log.debug("found applicable Liquibase update change logs: {}", changelogs); + + for (String changelog : changelogs) { + log.debug("applying Liquibase changelog '{}'", changelog); + + List currentWarnings = DatabaseUpdater.executeChangelog(changelog, + new PrintingChangeSetExecutorCallback("executing Liquibase changelog :" + changelog)); + + if (currentWarnings != null) { + warnings.addAll(currentWarnings); + } + } + executingChangesetId = null; // clear out the last changeset + + if (CollectionUtils.isNotEmpty(warnings)) { + reportWarnings(warnings); } - } - executingChangesetId = null; // clear out the last changeset - - if (CollectionUtils.isNotEmpty(warnings)) { - reportWarnings(warnings); } } catch (InputRequiredException inputRequired) { diff --git a/web/src/main/java/org/openmrs/web/filter/update/UpdateFilterModel.java b/web/src/main/java/org/openmrs/web/filter/update/UpdateFilterModel.java index 0f3c50fc944f..b8cb04a9ac47 100644 --- a/web/src/main/java/org/openmrs/web/filter/update/UpdateFilterModel.java +++ b/web/src/main/java/org/openmrs/web/filter/update/UpdateFilterModel.java @@ -9,9 +9,12 @@ */ package org.openmrs.web.filter.update; +import javax.xml.crypto.Data; +import java.util.Collections; import java.util.List; import org.openmrs.liquibase.LiquibaseProvider; +import org.openmrs.util.DatabaseUpdater; import org.openmrs.util.DatabaseUpdater.OpenMRSChangeSet; import org.openmrs.util.DatabaseUpdaterLiquibaseProvider; import org.openmrs.util.OpenmrsConstants; @@ -68,13 +71,12 @@ public UpdateFilterModel(LiquibaseProvider liquibaseProvider, DatabaseUpdaterWra this.liquibaseProvider = liquibaseProvider; this.databaseUpdaterWrapper = databaseUpdaterWrapper; - updateChanges(); - try { - if (changes != null && !changes.isEmpty()) { - updateRequired = true; - } else { - updateRequired = databaseUpdaterWrapper.updatesRequired(); + updateRequired = databaseUpdaterWrapper.updatesRequired(); + + if (updateRequired) { + updateChanges(); + updateRequired = changes != null; } } catch (Exception e) { @@ -89,11 +91,13 @@ public UpdateFilterModel(LiquibaseProvider liquibaseProvider, DatabaseUpdaterWra public void updateChanges() { log.debug("executing updateChanges()..."); try { - changes = databaseUpdaterWrapper.getUnrunDatabaseChanges(liquibaseProvider); - - // not sure why this is necessary... - if (changes == null && databaseUpdaterWrapper.isLocked()) { + if (updateRequired) { changes = databaseUpdaterWrapper.getUnrunDatabaseChanges(liquibaseProvider); + + // not sure why this is necessary... + if (changes == null && databaseUpdaterWrapper.isLocked()) { + changes = databaseUpdaterWrapper.getUnrunDatabaseChanges(liquibaseProvider); + } } } catch (Exception e) { diff --git a/web/src/test/java/org/openmrs/web/filter/update/UpdateFilterModelTest.java b/web/src/test/java/org/openmrs/web/filter/update/UpdateFilterModelTest.java index d7fce315c97d..3255711689ce 100644 --- a/web/src/test/java/org/openmrs/web/filter/update/UpdateFilterModelTest.java +++ b/web/src/test/java/org/openmrs/web/filter/update/UpdateFilterModelTest.java @@ -10,12 +10,14 @@ package org.openmrs.web.filter.update; import static org.hamcrest.CoreMatchers.is; +import static org.hamcrest.CoreMatchers.nullValue; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.collection.IsEmptyCollection.empty; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.isNull; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; import static org.mockito.Mockito.times; @@ -53,6 +55,7 @@ public void createUpdateFilterModel_shouldrequireAnUpdateAndSetChangesToUnrunDat throws Exception { List changes = Arrays.asList(mock(OpenMRSChangeSet.class)); + when(databaseUpdaterWrapper.updatesRequired()).thenReturn(true); when(databaseUpdaterWrapper.getUnrunDatabaseChanges(any(LiquibaseProvider.class))).thenReturn(changes); when(databaseUpdaterWrapper.isLocked()).thenReturn(false); @@ -62,7 +65,7 @@ public void createUpdateFilterModel_shouldrequireAnUpdateAndSetChangesToUnrunDat assertThat(model.changes, is(changes)); verify( databaseUpdaterWrapper, times(1)).getUnrunDatabaseChanges( liquibaseProvider ); - verify( databaseUpdaterWrapper, never()).updatesRequired(); + verify( databaseUpdaterWrapper, times(1)).updatesRequired(); } @Test @@ -95,9 +98,9 @@ public void createUpdateFilterModel_shouldNotRequireAnUpdateIfChangesAreEmptyAnd model = new UpdateFilterModel(liquibaseProvider, databaseUpdaterWrapper); assertFalse(model.updateRequired, "should not require an update"); - assertThat(model.changes, is(empty())); + assertThat(model.changes, is(nullValue())); - verify( databaseUpdaterWrapper, times(1)).getUnrunDatabaseChanges( liquibaseProvider ); + verify( databaseUpdaterWrapper, times(0)).getUnrunDatabaseChanges( liquibaseProvider ); verify( databaseUpdaterWrapper, times(1)).updatesRequired(); } @@ -114,7 +117,7 @@ public void createUpdateFilterModel_shouldNotRequireAnUpdateIfChangesAreNullAndD assertFalse(model.updateRequired, "should not require an update"); assertNull(model.changes, "should not have changes"); - verify( databaseUpdaterWrapper, times(1)).getUnrunDatabaseChanges( liquibaseProvider ); + verify( databaseUpdaterWrapper, times(0)).getUnrunDatabaseChanges( liquibaseProvider ); verify( databaseUpdaterWrapper, times(1)).updatesRequired(); } @@ -131,7 +134,7 @@ public void createUpdateFilterModel_shouldNotRequireAnUpdateIfDatabaseUpdaterIsL assertFalse(model.updateRequired, "should not require an update"); assertNull(model.changes, "should not have changes"); - verify( databaseUpdaterWrapper, times(2)).getUnrunDatabaseChanges( liquibaseProvider ); + verify( databaseUpdaterWrapper, times(0)).getUnrunDatabaseChanges( liquibaseProvider ); verify( databaseUpdaterWrapper, times(1)).updatesRequired(); } @@ -140,6 +143,7 @@ public void createUpdateFilterModel_shouldRequireAnUpdateIfDatabaseUpdaterIsLock throws Exception { List changes = Arrays.asList(mock(OpenMRSChangeSet.class)); + when(databaseUpdaterWrapper.updatesRequired()).thenReturn(true); when(databaseUpdaterWrapper.getUnrunDatabaseChanges(any(LiquibaseProvider.class))).thenReturn(changes); when(databaseUpdaterWrapper.isLocked()).thenReturn(true); @@ -149,6 +153,6 @@ public void createUpdateFilterModel_shouldRequireAnUpdateIfDatabaseUpdaterIsLock assertThat(model.changes, is(changes)); verify( databaseUpdaterWrapper, times(1)).getUnrunDatabaseChanges( liquibaseProvider ); - verify( databaseUpdaterWrapper, never()).updatesRequired(); + verify( databaseUpdaterWrapper, times(1)).updatesRequired(); } }