diff --git a/api/src/main/java/org/openmrs/liquibase/ChangeLogDetective.java b/api/src/main/java/org/openmrs/liquibase/ChangeLogDetective.java index f08df226397b..9cf059fe5f5f 100644 --- a/api/src/main/java/org/openmrs/liquibase/ChangeLogDetective.java +++ b/api/src/main/java/org/openmrs/liquibase/ChangeLogDetective.java @@ -83,38 +83,16 @@ public String getInitialLiquibaseSnapshotVersion(String context, LiquibaseProvid log.info("looking for un-run change sets in snapshot version '{}'", version); List changeSets = snapshotCombinations.get(version); - Liquibase liquibase = null; - try { - for (String filename : changeSets) { - String scopeId = LiquibaseScopeHandling.enterLiquibaseUILoggingService(); - - liquibase = liquibaseProvider.getLiquibase(filename); - - List rawUnrunChangeSets = new StatusCommandStep() - .listUnrunChangeSets(new Contexts(context), - new LabelExpression(), liquibase.getDatabaseChangeLog(), liquibase.getDatabase()); - - - LiquibaseScopeHandling.exitLiquibaseScope(scopeId); - liquibase.close(); - - List refinedUnrunChangeSets = excludeVintageChangeSets(filename, rawUnrunChangeSets); - - log.info("file '{}' contains {} un-run change sets", filename, refinedUnrunChangeSets.size()); - logUnRunChangeSetDetails(filename, refinedUnrunChangeSets); - - unrunChangeSetsCount += refinedUnrunChangeSets.size(); - } - } - finally { - if (liquibase != null) { - try { - liquibase.close(); - } - catch (Exception e) { - // ignore exceptions triggered by closing liquibase a second time - } - } + Contexts contexts = new Contexts(context); + for (String filename : changeSets) { + List rawUnrunChangeSets = getUnrunChangeSets(filename, contexts, liquibaseProvider); + + List refinedUnrunChangeSets = excludeVintageChangeSets(filename, rawUnrunChangeSets); + + log.info("file '{}' contains {} un-run change sets", filename, refinedUnrunChangeSets.size()); + logUnRunChangeSetDetails(filename, refinedUnrunChangeSets); + + unrunChangeSetsCount += refinedUnrunChangeSets.size(); } if (unrunChangeSetsCount == 0) { @@ -146,35 +124,15 @@ public List getUnrunLiquibaseUpdateFileNames(String snapshotVersion, Str List updateVersions = changeLogVersionFinder.getUpdateVersionsGreaterThan(snapshotVersion); List updateFileNames = changeLogVersionFinder.getUpdateFileNames(updateVersions); - Liquibase liquibase = null; - try { - for (String filename : updateFileNames) { - String scopeId = LiquibaseScopeHandling.enterLiquibaseUILoggingService(); - liquibase = liquibaseProvider.getLiquibase(filename); + Contexts contexts = new Contexts(context); + for (String filename : updateFileNames) { + List unrunChangeSets = getUnrunChangeSets(filename, contexts, liquibaseProvider); - List unrunChangeSets = new StatusCommandStep() - .listUnrunChangeSets(new Contexts(context), - new LabelExpression(), liquibase.getDatabaseChangeLog(), liquibase.getDatabase()); - - LiquibaseScopeHandling.exitLiquibaseScope(scopeId); - liquibase.close(); - - log.info("file '{}' contains {} un-run change sets", filename, unrunChangeSets.size()); - logUnRunChangeSetDetails(filename, unrunChangeSets); - - if (!unrunChangeSets.isEmpty()) { - unrunLiquibaseUpdates.add(filename); - } - } - } - finally { - if (liquibase != null) { - try { - liquibase.close(); - } - catch (Exception e) { - // ignore exceptions triggered by closing liquibase a second time - } + log.info("file '{}' contains {} un-run change sets", filename, unrunChangeSets.size()); + logUnRunChangeSetDetails(filename, unrunChangeSets); + + if (!unrunChangeSets.isEmpty()) { + unrunLiquibaseUpdates.add(filename); } } @@ -183,7 +141,7 @@ public List getUnrunLiquibaseUpdateFileNames(String snapshotVersion, Str List getSnapshotVersionsInDescendingOrder(Map> snapshotCombinations) { List versions = new ArrayList<>(snapshotCombinations.keySet()); - Collections.sort(versions, Collections.reverseOrder()); + versions.sort(Collections.reverseOrder()); return versions; } @@ -197,15 +155,29 @@ List excludeVintageChangeSets(String filename, List change return result; } - boolean isVintageChangeSet(String filename, ChangeSet changeSet) { - if (filename != null && filename.contains(LIQUIBASE_CORE_DATA_1_9_X_FILENAME) - && changeSet.getId().equals(DISABLE_FOREIGN_KEY_CHECKS) && changeSet.getAuthor().equals(BEN)) { - return true; + List getUnrunChangeSets(String filename, Contexts context, LiquibaseProvider liquibaseProvider) throws Exception { + String scopeId = LiquibaseScopeHandling.enterLiquibaseUILoggingService(); + Liquibase liquibase = liquibaseProvider.getLiquibase(filename); + + List unrunChangeSets; + try { + unrunChangeSets = new StatusCommandStep() + .listUnrunChangeSets(context, + new LabelExpression(), liquibase.getDatabaseChangeLog(), liquibase.getDatabase()); + + } finally { + LiquibaseScopeHandling.exitLiquibaseScope(scopeId); + liquibase.close(); } - if (filename != null && filename.contains(LIQUIBASE_CORE_DATA_1_9_X_FILENAME) - && changeSet.getId().equals(ENABLE_FOREIGN_KEY_CHECKS) && changeSet.getAuthor().equals(BEN)) { - return true; + + return unrunChangeSets; + } + + boolean isVintageChangeSet(String filename, ChangeSet changeSet) { + if (filename != null && filename.contains(LIQUIBASE_CORE_DATA_1_9_X_FILENAME) && changeSet.getAuthor().equals(BEN)) { + return changeSet.getId().equals(DISABLE_FOREIGN_KEY_CHECKS) || changeSet.getId().equals(ENABLE_FOREIGN_KEY_CHECKS); } + return false; } @@ -218,10 +190,13 @@ boolean isVintageChangeSet(String filename, ChangeSet changeSet) { boolean logUnRunChangeSetDetails(String filename, List changeSets) { if (changeSets.size() < MAX_NUMBER_OF_CHANGE_SETS_TO_LOG && (filename.contains(LIQUIBASE_CORE_DATA_1_9_X_FILENAME) || filename.contains(LIQUIBASE_SCHEMA_ONLY_1_9_X_FILENAME))) { - for (ChangeSet changeSet : changeSets) { - log.info("file '{}' contains un-run change set with id '{}' by author '{}'", filename, changeSet.getId(), - changeSet.getAuthor()); + if (log.isInfoEnabled()) { + for (ChangeSet changeSet : changeSets) { + log.info("file '{}' contains un-run change set with id '{}' by author '{}'", filename, changeSet.getId(), + changeSet.getAuthor()); + } } + return true; } return false; diff --git a/api/src/main/java/org/openmrs/liquibase/LiquibaseScopeHandling.java b/api/src/main/java/org/openmrs/liquibase/LiquibaseScopeHandling.java index 971cf4c72294..d67249a5f8aa 100644 --- a/api/src/main/java/org/openmrs/liquibase/LiquibaseScopeHandling.java +++ b/api/src/main/java/org/openmrs/liquibase/LiquibaseScopeHandling.java @@ -22,7 +22,7 @@ public static String enterLiquibaseUILoggingService() throws Exception { // Temp workaround to silence liquibase writing to stdout - https://github.com/liquibase/liquibase/issues/2396, // https://github.com/liquibase/liquibase/issues/3651 LoggerUIService loggerUIService = new LoggerUIService(); - loggerUIService.setStandardLogLevel(Level.FINE); + loggerUIService.setStandardLogLevel(Level.INFO); Map m = Collections.singletonMap(Scope.Attr.ui.name(), loggerUIService); return Scope.enter(m); } diff --git a/api/src/main/java/org/openmrs/module/ModuleUtil.java b/api/src/main/java/org/openmrs/module/ModuleUtil.java index 60f5e072e94b..d3b72cf7f442 100644 --- a/api/src/main/java/org/openmrs/module/ModuleUtil.java +++ b/api/src/main/java/org/openmrs/module/ModuleUtil.java @@ -71,7 +71,7 @@ public static void startup(Properties props) throws ModuleMustStartException { String moduleListString = props.getProperty(ModuleConstants.RUNTIMEPROPERTY_MODULE_LIST_TO_LOAD); - if (moduleListString == null || moduleListString.length() == 0) { + if (moduleListString == null || moduleListString.isEmpty()) { // Attempt to get all of the modules from the modules folder // and store them in the modules list log.debug("Starting all modules"); diff --git a/api/src/main/java/org/openmrs/util/LocaleUtility.java b/api/src/main/java/org/openmrs/util/LocaleUtility.java index 547c1e0384d4..2cff8e531e50 100644 --- a/api/src/main/java/org/openmrs/util/LocaleUtility.java +++ b/api/src/main/java/org/openmrs/util/LocaleUtility.java @@ -34,7 +34,7 @@ public class LocaleUtility implements GlobalPropertyListener { * Cached version of the default locale. This is cached so that we don't have to look it up in * the global property table every page load */ - private static Locale defaultLocaleCache = null; + private static volatile Locale defaultLocaleCache = null; /** * Cached version of the localeAllowedList. This is cached so that we don't have to look it up @@ -55,36 +55,40 @@ public class LocaleUtility implements GlobalPropertyListener { */ public static Locale getDefaultLocale() { if (defaultLocaleCache == null) { - if (Context.isSessionOpen()) { - try { - String locale = Context.getAdministrationService().getGlobalProperty( - OpenmrsConstants.GLOBAL_PROPERTY_DEFAULT_LOCALE); - - if (StringUtils.hasLength(locale)) { + synchronized (LocaleUtility.class) { + if (defaultLocaleCache == null) { + if (Context.isSessionOpen()) { try { - defaultLocaleCache = fromSpecification(locale); + Context.addProxyPrivilege(PrivilegeConstants.GET_GLOBAL_PROPERTIES); + String locale = Context.getAdministrationService().getGlobalProperty( + OpenmrsConstants.GLOBAL_PROPERTY_DEFAULT_LOCALE); + + if (StringUtils.hasLength(locale)) { + try { + defaultLocaleCache = fromSpecification(locale); + } catch (Exception t) { + log.warn("Unable to parse default locale global property value: {}", locale, t); + } + } + } catch (Exception e) { + // don't print the full stack-trace by default + log.warn("Unable to get locale global property value. {}", e.getMessage()); + log.debug("Exception caught while capturing locale global property value.", e); + } finally { + Context.removeProxyPrivilege(PrivilegeConstants.GET_GLOBAL_PROPERTIES); } - catch (Exception t) { - log.warn("Unable to parse default locale global property value: " + locale, t); + + // if we weren't able to load the locale from the global property, + // use the default one + if (defaultLocaleCache == null) { + defaultLocaleCache = fromSpecification(OpenmrsConstants.GLOBAL_PROPERTY_DEFAULT_LOCALE_DEFAULT_VALUE); } + } else { + // if session is not open, return the default locale without caching + return fromSpecification(OpenmrsConstants.GLOBAL_PROPERTY_DEFAULT_LOCALE_DEFAULT_VALUE); } } - catch (Exception e) { - // swallow most of the stack trace for most users - log.warn("Unable to get locale global property value. " + e.getMessage()); - log.trace("Unable to get locale global property value", e); - } - - // if we weren't able to load the locale from the global property, - // use the default one - if (defaultLocaleCache == null) { - defaultLocaleCache = fromSpecification(OpenmrsConstants.GLOBAL_PROPERTY_DEFAULT_LOCALE_DEFAULT_VALUE); - } - } else { - // if session is not open, return the default locale without caching - return fromSpecification(OpenmrsConstants.GLOBAL_PROPERTY_DEFAULT_LOCALE_DEFAULT_VALUE); } - } return defaultLocaleCache;