Skip to content

Commit c721ef9

Browse files
IamMujuziMosesrkorytkowski
IamMujuziMoses
authored and
rkorytkowski
committed
TRUNK-6418: Run liquibase checks and data imports only when version of core or modules changes
1 parent 7162ed7 commit c721ef9

File tree

9 files changed

+176
-227
lines changed

9 files changed

+176
-227
lines changed

‎api/src/main/java/org/openmrs/api/AdministrationService.java‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,10 +17,12 @@
1717

1818
import org.openmrs.GlobalProperty;
1919
import org.openmrs.ImplementationId;
20+
import org.openmrs.module.Module;
2021
import org.openmrs.OpenmrsObject;
2122
import org.openmrs.User;
2223
import org.openmrs.annotation.Authorized;
2324
import org.openmrs.api.db.AdministrationDAO;
25+
import org.openmrs.util.DatabaseUpdateException;
2426
import org.openmrs.util.HttpClient;
2527
import org.openmrs.util.OpenmrsConstants;
2628
import org.openmrs.util.PrivilegeConstants;
@@ -420,4 +422,33 @@ public interface AdministrationService extends OpenmrsService {
420422
* <strong>Should</strong> return default common classes if no GPs defined
421423
*/
422424
List<String> getSerializerWhitelistTypes();
425+
426+
/**
427+
* Checks whether a core setup needs to be run due to a version change.
428+
*
429+
* @return true if core setup should be executed because of a version change, false otherwise
430+
*/
431+
public boolean isCoreSetupOnVersionChangeNeeded();
432+
433+
/**
434+
* Checks whether a module setup needs to be run due to a version change.
435+
*
436+
* @param moduleId the identifier of the module to check
437+
* @return true if the module setup should be executed because of a version change, false otherwise
438+
*/
439+
public boolean isModuleSetupOnVersionChangeNeeded(String moduleId);
440+
441+
/**
442+
* Executes the core setup procedures required after a core version change.
443+
*
444+
* @throws DatabaseUpdateException
445+
*/
446+
public void runCoreSetupOnVersionChange() throws DatabaseUpdateException;
447+
448+
/**
449+
* Executes the setup procedures required for a module after a module version change.
450+
*
451+
* @param module the module for which the setup should be executed
452+
*/
453+
public void runModuleSetupOnVersionChange(Module module);
423454
}

‎api/src/main/java/org/openmrs/api/context/Context.java‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1311,13 +1311,18 @@ private static void checkForDatabaseUpdates(Properties props) throws DatabaseUpd
13111311

13121312
// this must be the first thing run in case it changes database mappings
13131313
if (updatesRequired) {
1314-
if (DatabaseUpdater.allowAutoUpdate()) {
1315-
DatabaseUpdater.executeChangelog();
1316-
} else {
1314+
if (!DatabaseUpdater.allowAutoUpdate()) {
13171315
throw new DatabaseUpdateException(
13181316
"Database updates are required. Call Context.updateDatabase() before .startup() to continue.");
13191317
}
13201318
}
1319+
1320+
if (getAdministrationService().isCoreSetupOnVersionChangeNeeded()) {
1321+
log.info("Detected core version change. Running core setup hooks and Liquibase.");
1322+
getAdministrationService().runCoreSetupOnVersionChange();
1323+
}
1324+
1325+
log.info("Database update check completed.");
13211326
}
13221327

13231328
/**

‎api/src/main/java/org/openmrs/api/impl/AdministrationServiceImpl.java‎

Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
import java.util.List;
2626
import java.util.Locale;
2727
import java.util.Map;
28+
import java.util.Objects;
2829
import java.util.Properties;
2930
import java.util.Set;
3031
import java.util.SortedMap;
@@ -59,6 +60,8 @@
5960
import org.openmrs.module.ModuleUtil;
6061
import org.openmrs.obs.ComplexData;
6162
import org.openmrs.person.PersonMergeLogData;
63+
import org.openmrs.util.DatabaseUpdateException;
64+
import org.openmrs.util.DatabaseUpdater;
6265
import org.openmrs.util.HttpClient;
6366
import org.openmrs.util.LocaleUtility;
6467
import org.openmrs.util.OpenmrsConstants;
@@ -1010,4 +1013,99 @@ public List<Class<?>> getRefTypes() {
10101013
return Arrays.asList(GlobalProperty.class);
10111014
}
10121015

1016+
/**
1017+
* @see org.openmrs.api.AdministrationService#isCoreSetupOnVersionChangeNeeded()
1018+
*/
1019+
@Override
1020+
public boolean isCoreSetupOnVersionChangeNeeded() {
1021+
String stored = getStoredCoreVersion();
1022+
String current = OpenmrsConstants.OPENMRS_VERSION_SHORT;
1023+
boolean forceSetup = Boolean.parseBoolean(getGlobalProperty("force.setup", "false"));
1024+
1025+
return forceSetup || !Objects.equals(stored, current);
1026+
}
1027+
1028+
/**
1029+
* @see org.openmrs.api.AdministrationService#isModuleSetupOnVersionChangeNeeded(String)
1030+
*/
1031+
@Override
1032+
public boolean isModuleSetupOnVersionChangeNeeded(String moduleId) {
1033+
String stored = getStoredModuleVersion(moduleId);
1034+
Module module = ModuleFactory.getModuleById(moduleId);
1035+
if (module == null) {
1036+
return false;
1037+
}
1038+
String current = module.getVersion();
1039+
boolean forceSetup = Boolean.parseBoolean(getGlobalProperty("force.setup", "false"));
1040+
1041+
return forceSetup || !Objects.equals(stored, current);
1042+
}
1043+
1044+
/**
1045+
* @see org.openmrs.api.AdministrationService#runCoreSetupOnVersionChange()
1046+
*/
1047+
@Override
1048+
@Transactional
1049+
public void runCoreSetupOnVersionChange() throws DatabaseUpdateException {
1050+
DatabaseUpdater.executeChangelog();
1051+
storeCoreVersion();
1052+
}
1053+
1054+
/**
1055+
* @see org.openmrs.api.AdministrationService#runModuleSetupOnVersionChange(Module)
1056+
*/
1057+
@Override
1058+
@Transactional
1059+
public void runModuleSetupOnVersionChange(Module module) {
1060+
if (module == null) {
1061+
return;
1062+
}
1063+
1064+
String moduleId = module.getModuleId();
1065+
String prevCoreVersion = getStoredCoreVersion() != null ? getStoredCoreVersion() : OpenmrsConstants.OPENMRS_VERSION_SHORT;
1066+
String prevModuleVersion = getStoredModuleVersion(moduleId);
1067+
1068+
module.getModuleActivator().setupOnVersionChangeBeforeSchemaChanges(prevCoreVersion, prevModuleVersion);
1069+
ModuleFactory.runLiquibaseForModule(module);
1070+
module.getModuleActivator().setupOnVersionChange(prevCoreVersion, prevModuleVersion);
1071+
1072+
storeModuleVersion(moduleId, module.getVersion());
1073+
}
1074+
1075+
protected String getStoredCoreVersion() {
1076+
return getGlobalProperty("core.version");
1077+
}
1078+
1079+
protected String getStoredModuleVersion(String moduleId) {
1080+
return getGlobalProperty("module." + moduleId + ".version");
1081+
}
1082+
1083+
protected void storeCoreVersion() {
1084+
saveGlobalProperty("core.version", OpenmrsConstants.OPENMRS_VERSION_SHORT, "Saved the state of this core version for future restarts");
1085+
}
1086+
1087+
protected void storeModuleVersion(String moduleId, String version) {
1088+
String propertyName = "module." + moduleId + ".version";
1089+
saveGlobalProperty(propertyName, version, "Saved the state of this module version for future restarts");
1090+
}
1091+
1092+
/**
1093+
* Convenience method to save a global property with the given value. Proxy privileges are added so
1094+
* that this can occur at startup.
1095+
*/
1096+
protected void saveGlobalProperty(String key, String value, String desc) {
1097+
try {
1098+
GlobalProperty gp = getGlobalPropertyObject(key);
1099+
if (gp == null) {
1100+
gp = new GlobalProperty(key, value, desc);
1101+
} else {
1102+
gp.setPropertyValue(value);
1103+
}
1104+
1105+
saveGlobalProperty(gp);
1106+
}
1107+
catch (Exception e) {
1108+
log.warn("Unable to save the global property", e);
1109+
}
1110+
}
10131111
}

‎api/src/main/java/org/openmrs/module/ModuleFactory.java‎

Lines changed: 10 additions & 94 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@
2525
import java.util.List;
2626
import java.util.Map;
2727
import java.util.Map.Entry;
28-
import java.util.Objects;
2928
import java.util.Set;
3029
import java.util.SortedMap;
3130
import java.util.concurrent.ConcurrentHashMap;
@@ -53,24 +52,16 @@
5352
import org.slf4j.Logger;
5453
import org.slf4j.LoggerFactory;
5554
import org.springframework.aop.Advisor;
56-
import org.springframework.beans.factory.annotation.Autowired;
5755
import org.springframework.context.support.AbstractRefreshableApplicationContext;
58-
import org.springframework.stereotype.Component;
59-
import org.springframework.transaction.PlatformTransactionManager;
60-
import org.springframework.transaction.TransactionManager;
61-
import org.springframework.transaction.support.TransactionTemplate;
6256
import org.springframework.util.StringUtils;
6357

6458
import liquibase.Contexts;
6559

6660
/**
6761
* Methods for loading, starting, stopping, and storing OpenMRS modules
6862
*/
69-
@Component
7063
public class ModuleFactory {
7164

72-
private static TransactionManager txManager;
73-
7465
private ModuleFactory() {
7566
}
7667

@@ -97,11 +88,6 @@ private ModuleFactory() {
9788

9889
private static final Set<String> actualStartupOrder = new LinkedHashSet<>();
9990

100-
@Autowired
101-
public void setTransactionManager(TransactionManager transactionManager) {
102-
ModuleFactory.txManager = transactionManager;
103-
}
104-
10591
/**
10692
* Add a module (in the form of a jar file) to the list of openmrs modules Returns null if an error
10793
* occurred and/or module was not successfully loaded
@@ -286,7 +272,6 @@ public static void startModules() {
286272
notifySuperUsersAboutModuleFailure(mod);
287273
}
288274
}
289-
storeCoreVersion();
290275
}
291276
}
292277

@@ -709,31 +694,9 @@ public static Module startModuleInternal(Module module, boolean isOpenmrsStartup
709694
Context.removeProxyPrivilege("");
710695
}
711696

712-
// Run version-change hooks + liquibase if necessary
713-
String prevCoreVersion = getStoredCoreVersion() != null ? getStoredCoreVersion() : OpenmrsConstants.OPENMRS_VERSION_SHORT;
714-
String currentCoreVersion = OpenmrsConstants.OPENMRS_VERSION_SHORT;
715-
716-
boolean forceSetup = Boolean.parseBoolean(
717-
Context.getAdministrationService().getGlobalProperty("force.setup", "false"));
718-
719-
String prevModuleVersion = getStoredModuleVersion(module);
720-
String currentModuleVersion = module.getVersion();
721-
722-
boolean versionChanged = forceSetup
723-
|| versionChanged(prevCoreVersion, currentCoreVersion, prevModuleVersion, currentModuleVersion);
724-
725-
if (versionChanged) {
726-
runInSingleTransaction(() -> {
727-
module.getModuleActivator().setupOnVersionChangeBeforeSchemaChanges(prevCoreVersion, prevModuleVersion);
728-
729-
// run module's optional liquibase.xml immediately after sqldiff.xml
730-
log.debug("Run module liquibase: {}", module.getModuleId());
731-
runLiquibase(module);
732-
733-
module.getModuleActivator().setupOnVersionChange(prevCoreVersion, prevModuleVersion);
734-
});
735-
736-
storeModuleVersion(module.getModuleId(), currentModuleVersion);
697+
if (Context.getAdministrationService().isModuleSetupOnVersionChangeNeeded(module.getModuleId())) {
698+
log.info("Detected version change for module {}. Running setup hooks and module Liquibase.", module.getModuleId());
699+
Context.getAdministrationService().runModuleSetupOnVersionChange(module);
737700
}
738701

739702
// effectively mark this module as started successfully
@@ -983,6 +946,13 @@ private static void runDiff(Module module, String version, String sql) {
983946
}
984947

985948
}
949+
950+
/**
951+
* This is a convenience method that exposes the private {@link #runLiquibase(Module)} method.
952+
*/
953+
public static void runLiquibaseForModule(Module module) {
954+
runLiquibase(module);
955+
}
986956

987957
/**
988958
* Execute all not run changeSets in liquibase.xml for the given module
@@ -1662,58 +1632,4 @@ public static List<String> getDependencies(String moduleId) {
16621632
}
16631633
return dependentModules;
16641634
}
1665-
1666-
private static boolean versionChanged(String prevCore, String currentCore, String prevModule, String currentModule) {
1667-
return !Objects.equals(prevCore, currentCore) || !Objects.equals(prevModule, currentModule);
1668-
}
1669-
1670-
protected static String getStoredCoreVersion() {
1671-
return Context.getAdministrationService().getGlobalProperty("core.version");
1672-
}
1673-
1674-
protected static String getStoredModuleVersion(Module mod) {
1675-
return Context.getAdministrationService().getGlobalProperty("module." + mod.getModuleId() + ".version");
1676-
}
1677-
1678-
protected static void storeCoreVersion() {
1679-
try {
1680-
Context.addProxyPrivilege(PrivilegeConstants.MANAGE_GLOBAL_PROPERTIES);
1681-
1682-
Context.getAdministrationService().setGlobalProperty("core.version", OpenmrsConstants.OPENMRS_VERSION_SHORT);
1683-
} finally {
1684-
Context.removeProxyPrivilege(PrivilegeConstants.MANAGE_GLOBAL_PROPERTIES);
1685-
}
1686-
}
1687-
1688-
protected static void storeModuleVersion(String moduleId, String version) {
1689-
// Clear session so that the first-level cache is empty
1690-
Context.flushSession();
1691-
Context.clearSession();
1692-
try {
1693-
Context.addProxyPrivilege(PrivilegeConstants.MANAGE_GLOBAL_PROPERTIES);
1694-
String propertyName = "module." + moduleId + ".version";
1695-
1696-
AdministrationService adminService = Context.getAdministrationService();
1697-
GlobalProperty gp = adminService.getGlobalPropertyObject(propertyName);
1698-
1699-
if (gp == null) {
1700-
gp = new GlobalProperty(propertyName, version);
1701-
} else {
1702-
gp.setPropertyValue(version);
1703-
}
1704-
1705-
adminService.saveGlobalProperty(gp);
1706-
1707-
} finally {
1708-
Context.removeProxyPrivilege(PrivilegeConstants.MANAGE_GLOBAL_PROPERTIES);
1709-
}
1710-
}
1711-
1712-
private static void runInSingleTransaction(Runnable action) {
1713-
TransactionTemplate txTemplate = new TransactionTemplate((PlatformTransactionManager) txManager);
1714-
txTemplate.execute(status -> {
1715-
action.run();
1716-
return null;
1717-
});
1718-
}
17191635
}

‎api/src/test/java/org/openmrs/api/AdministrationServiceTest.java‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
import static org.junit.jupiter.api.Assertions.assertThrows;
2828
import static org.junit.jupiter.api.Assertions.assertTrue;
2929
import static org.mockito.Mockito.mock;
30+
import static org.mockito.Mockito.verify;
3031

3132
import java.util.ArrayList;
3233
import java.util.Arrays;
@@ -51,6 +52,8 @@
5152
import org.openmrs.customdatatype.datatype.DateDatatype;
5253
import org.openmrs.messagesource.MutableMessageSource;
5354
import org.openmrs.messagesource.impl.MutableResourceBundleMessageSource;
55+
import org.openmrs.module.Module;
56+
import org.openmrs.module.ModuleActivator;
5457
import org.openmrs.test.jupiter.BaseContextSensitiveTest;
5558
import org.openmrs.util.HttpClient;
5659
import org.openmrs.util.LocaleUtility;
@@ -1160,4 +1163,29 @@ public void getSerializerWhitelistTypes_shouldReturnDefaultCommonClassesIfNoGPS(
11601163
"hierarchyOf:org.openmrs.messagesource.PresentationMessage",
11611164
"hierarchyOf:org.openmrs.person.PersonMergeLogData"));
11621165
}
1166+
1167+
@Test
1168+
public void runModuleSetupOnVersionChange_shouldExecuteLiquibaseAndStoreNewVersion() {
1169+
// old version
1170+
adminService.setGlobalProperty("module.testmodule.version", "1.0.0");
1171+
assertEquals("1.0.0", adminService.getGlobalProperty("module.testmodule.version"));
1172+
1173+
String previousModuleVersion = "1.0.0";
1174+
String previousCoreVersion = OpenmrsConstants.OPENMRS_VERSION_SHORT;
1175+
1176+
Module module = new Module("Test Module");
1177+
module.setModuleId("testmodule");
1178+
module.setVersion("1.2.3");
1179+
1180+
ModuleActivator activator = mock(ModuleActivator.class);
1181+
module.setModuleActivator(activator);
1182+
1183+
adminService.runModuleSetupOnVersionChange(module);
1184+
1185+
assertEquals("1.2.3", adminService.getGlobalProperty("module.testmodule.version"));
1186+
1187+
// verify hook methods must be called
1188+
verify(activator).setupOnVersionChangeBeforeSchemaChanges(previousCoreVersion, previousModuleVersion);
1189+
verify(activator).setupOnVersionChange(previousCoreVersion, previousModuleVersion);
1190+
}
11631191
}

0 commit comments

Comments
 (0)