From cca3ac0d20268ab0dca94aac5c58ffe902f9b79b Mon Sep 17 00:00:00 2001 From: Ian <52504170+ibacher@users.noreply.github.com> Date: Wed, 6 Mar 2024 12:54:23 -0500 Subject: [PATCH] TRUNK-4004: Expose start_before_modules functionality better (#4575) --- .../org/openmrs/module/ModuleFileParser.java | 20 ++- .../org/openmrs/module/dtd/README.md | 2 +- .../org/openmrs/module/dtd/config-1.6.dtd | 5 + .../openmrs/module/ModuleFileParserTest.java | 24 ++-- .../module/ModuleFileParserUnitTest.java | 115 +++++++++++++++++- 5 files changed, 146 insertions(+), 20 deletions(-) diff --git a/api/src/main/java/org/openmrs/module/ModuleFileParser.java b/api/src/main/java/org/openmrs/module/ModuleFileParser.java index 8d22c406213e..167e53517848 100644 --- a/api/src/main/java/org/openmrs/module/ModuleFileParser.java +++ b/api/src/main/java/org/openmrs/module/ModuleFileParser.java @@ -29,6 +29,8 @@ import java.util.Objects; import java.util.Set; import java.util.jar.JarFile; +import java.util.regex.Matcher; +import java.util.regex.Pattern; import java.util.zip.ZipEntry; import org.apache.commons.lang3.StringUtils; @@ -37,6 +39,7 @@ import org.openmrs.api.context.Context; import org.openmrs.customdatatype.CustomDatatype; import org.openmrs.messagesource.MessageSourceService; +import org.openmrs.util.OpenmrsClassLoader; import org.openmrs.util.OpenmrsUtil; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -64,6 +67,9 @@ public class ModuleFileParser { private static final String OPENMRS_MODULE_FILE_EXTENSION = ".omod"; + //https://resources.openmrs.org/doctype/config-1.5.dtd + private static final Pattern OPENMRS_DTD_SYSTEM_ID_PATTERN = Pattern.compile("https?://resources.openmrs.org/doctype/(?config-[0-9.]+\\.dtd)"); + /** * List out all of the possible version numbers for config files that openmrs has DTDs for. * These are usually stored at http://resources.openmrs.org/doctype/config-x.x.dt @@ -286,7 +292,15 @@ private DocumentBuilder newDocumentBuilder() throws ParserConfigurationException // DTD) we return an InputSource // with no data at the end, causing the parser to ignore // the DTD. - db.setEntityResolver((publicId, systemId) -> new InputSource(new StringReader(""))); + db.setEntityResolver((publicId, systemId) -> { + Matcher dtdMatcher = OPENMRS_DTD_SYSTEM_ID_PATTERN.matcher(systemId); + if (dtdMatcher.matches()) { + String dtdFile = dtdMatcher.group("config"); + return new InputSource(OpenmrsClassLoader.getInstance().getResourceAsStream("org/openmrs/module/dtd/" + dtdFile)); + } + return new InputSource(new StringReader("")); + }); + return db; } @@ -375,7 +389,9 @@ private Map extractAwareOfModules(Element configRoot) { } private Map extractStartBeforeModules(Element configRoot) { - return extractModulesWithVersionAttribute(configRoot, "module", "start_before_modules"); + Map result = extractModulesWithVersionAttribute(configRoot, "module", "start_before_modules"); + result.putAll(extractModulesWithVersionAttribute(configRoot, "start_before_module", "start_before_modules")); + return result; } private Map extractModulesWithVersionAttribute(Element configRoot, String elementName, diff --git a/api/src/main/resources/org/openmrs/module/dtd/README.md b/api/src/main/resources/org/openmrs/module/dtd/README.md index 1eba2e830c6c..7ef74754f2c4 100644 --- a/api/src/main/resources/org/openmrs/module/dtd/README.md +++ b/api/src/main/resources/org/openmrs/module/dtd/README.md @@ -13,7 +13,7 @@ your config. Like so ```xml + "https://resources.openmrs.org/doctype/config-1.5.dtd"> ... diff --git a/api/src/main/resources/org/openmrs/module/dtd/config-1.6.dtd b/api/src/main/resources/org/openmrs/module/dtd/config-1.6.dtd index 1af510a4e688..95bfdef266c5 100644 --- a/api/src/main/resources/org/openmrs/module/dtd/config-1.6.dtd +++ b/api/src/main/resources/org/openmrs/module/dtd/config-1.6.dtd @@ -15,6 +15,7 @@ (require_database_version?), (require_modules?), (aware_of_modules?), + (start_before_modules?), (mandatory?), (library*), (extension*), @@ -51,6 +52,10 @@ + + + + diff --git a/api/src/test/java/org/openmrs/module/ModuleFileParserTest.java b/api/src/test/java/org/openmrs/module/ModuleFileParserTest.java index 453d808cb82d..acaa07a2e672 100644 --- a/api/src/test/java/org/openmrs/module/ModuleFileParserTest.java +++ b/api/src/test/java/org/openmrs/module/ModuleFileParserTest.java @@ -70,35 +70,32 @@ public static void setUp() throws ParserConfigurationException { @Test public void moduleFileParser_shouldFailCreatingParserFromFileIfGivenNull() { - expectModuleExceptionWithTranslatedMessage(() -> new ModuleFileParser((File) null), "Module.error.fileCannotBeNull"); } @Test public void moduleFileParser_shouldFailCreatingParserFromFileIfNotEndingInOmod() { - expectModuleExceptionWithTranslatedMessage(() -> new ModuleFileParser(new File("reporting.jar")), "Module.error.invalidFileExtension"); } @Test public void moduleFileParser_shouldFailCreatingParserFromFileIfInputStreamClosed() throws IOException { - File moduleFile = new File(getClass().getClassLoader().getResource(LOGIC_MODULE_PATH).getPath()); - InputStream inputStream = new FileInputStream(moduleFile); - inputStream.close(); - - expectModuleExceptionWithTranslatedMessage(() -> new ModuleFileParser(inputStream), "Module.error.cannotCreateFile"); + try (InputStream inputStream = new FileInputStream(moduleFile)) { + inputStream.close(); + expectModuleExceptionWithTranslatedMessage(() -> new ModuleFileParser(inputStream), "Module.error.cannotCreateFile"); + } } @Test public void parse_shouldParseValidXmlConfigCreatedFromInputStream() throws IOException { - File moduleFile = new File(getClass().getClassLoader().getResource(LOGIC_MODULE_PATH).getPath()); - ModuleFileParser parser = new ModuleFileParser(new FileInputStream(moduleFile)); - - Module module = parser.parse(); + Module module; + try (FileInputStream moduleFileInputStream = new FileInputStream(moduleFile)) { + module = new ModuleFileParser().parse(moduleFileInputStream); + } assertThat(module.getModuleId(), is("logic")); assertThat(module.getVersion(), is("0.2")); @@ -129,14 +126,11 @@ public void parse_shouldFailIfModuleHasConfigInvalidConfigVersion() throws Excep configXml.appendChild(root); configXml.getDocumentElement().setAttribute("configVersion", invalidConfigVersion); - ModuleFileParser parser = new ModuleFileParser(writeConfigXmlToFile(configXml)); - - expectModuleExceptionWithMessage(() -> parser.parse(), expectedMessage); + expectModuleExceptionWithMessage(() -> new ModuleFileParser().parse(writeConfigXmlToFile(configXml)), expectedMessage); } @Test public void parse_shouldParseValidLogicModuleFromFile() { - File moduleFile = new File(getClass().getClassLoader().getResource(LOGIC_MODULE_PATH).getPath()); ModuleFileParser parser = new ModuleFileParser(Context.getMessageSourceService()); diff --git a/api/src/test/java/org/openmrs/module/ModuleFileParserUnitTest.java b/api/src/test/java/org/openmrs/module/ModuleFileParserUnitTest.java index a3c56652f513..42b0d54e7395 100644 --- a/api/src/test/java/org/openmrs/module/ModuleFileParserUnitTest.java +++ b/api/src/test/java/org/openmrs/module/ModuleFileParserUnitTest.java @@ -14,6 +14,7 @@ import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.hasEntry; import static org.hamcrest.Matchers.hasItems; +import static org.hamcrest.Matchers.hasSize; import static org.hamcrest.Matchers.instanceOf; import static org.hamcrest.Matchers.is; import static org.hamcrest.core.StringStartsWith.startsWith; @@ -532,6 +533,107 @@ public void parse_shouldParseAwareOfModulesContainingMultipleChildren() throws I hasItems("org.openmrs.module.serialization.xstream", "org.openmrs.module.legacyui")); } + @Test + public void parse_shouldParseStartBeforeModulesWithoutChildren() throws IOException { + Document config = buildOnValidConfigXml() + .withStartBeforeModules() + .build(); + + Module module = parser.parse(writeConfigXmlToFile(config)); + + assertThat(module.getStartBeforeModules(), is(equalTo(Collections.EMPTY_LIST))); + } + + @Test + public void parse_shouldParseStartBeforeModulesOnlyContainingText() throws IOException { + Document config = buildOnValidConfigXml() + .withTextNode("start_before_modules", "will be ignored") + .build(); + + Module module = parser.parse(writeConfigXmlToFile(config)); + + assertThat(module.getStartBeforeModules(), is(equalTo(Collections.EMPTY_LIST))); + } + + @Test + public void parse_shouldParseStartBeforeModulesContainingDuplicatesAndKeepOnlyOneModule() + throws IOException { + + Document config = buildOnValidConfigXml() + .withStartBeforeModules( + "org.openmrs.module.serialization.xstream", + "org.openmrs.module.serialization.xstream" + ) + .build(); + + Module module = parser.parse(writeConfigXmlToFile(config)); + + assertThat(module.getStartBeforeModules(), hasSize(1)); + assertThat(module.getStartBeforeModules(), hasItems("org.openmrs.module.serialization.xstream")); + } + + @Test + public void parse_shouldParseStartBeforeModulesContainingMultipleChildren() throws IOException { + Document config = buildOnValidConfigXml() + .withStartBeforeModules( + "org.openmrs.module.serialization.xstream", + "org.openmrs.module.legacyui" + ) + .build(); + + Module module = parser.parse(writeConfigXmlToFile(config)); + + assertThat(module.getStartBeforeModules(), hasSize(2)); + assertThat(module.getStartBeforeModules(), + hasItems("org.openmrs.module.serialization.xstream", "org.openmrs.module.legacyui")); + } + + @Test + public void parse_shouldParseStartBeforeModulesContainingModuleChildren() throws IOException { + Document config = buildOnValidConfigXml().build(); + + Element startBeforeModules = config.createElement("start_before_modules"); + for (String module : new String[] { "org.openmrs.module.serialization.xstream", "org.openmrs.module.legacyui" }) { + Element startBeforeModule = config.createElement("module"); + startBeforeModule.setTextContent(module); + startBeforeModules.appendChild(startBeforeModule); + } + config.getDocumentElement().appendChild(startBeforeModules); + + Module module = parser.parse(writeConfigXmlToFile(config)); + + assertThat(module.getStartBeforeModules(), hasSize(2)); + assertThat(module.getStartBeforeModules(), + hasItems("org.openmrs.module.serialization.xstream", "org.openmrs.module.legacyui")); + } + + @Test + public void parse_shouldParseStartBeforeModulesContainingMixedChildren() throws IOException { + Document config = buildOnValidConfigXml().build(); + + Element startBeforeModules = config.createElement("start_before_modules"); + for (String module : new String[] { "org.openmrs.module.xforms", "org.openmrs.module.idgen" }) { + Element startBeforeModule = config.createElement("start_before_module"); + startBeforeModule.setTextContent(module); + startBeforeModules.appendChild(startBeforeModule); + } + + for (String module : new String[] { "org.openmrs.module.serialization.xstream", "org.openmrs.module.legacyui" }) { + Element startBeforeModule = config.createElement("module"); + startBeforeModule.setTextContent(module); + startBeforeModules.appendChild(startBeforeModule); + } + + config.getDocumentElement().appendChild(startBeforeModules); + + Module module = parser.parse(writeConfigXmlToFile(config)); + + assertThat(module.getStartBeforeModules(), hasSize(4)); + assertThat(module.getStartBeforeModules(), + hasItems("org.openmrs.module.serialization.xstream", "org.openmrs.module.legacyui", + "org.openmrs.module.xforms", "org.openmrs.module.idgen")); + } + @Test public void parse_shouldParseExtensions() throws IOException { @@ -1175,12 +1277,10 @@ private void whenGettingMessageFromMessageSourceServiceWithKeyReturnSameKey(Stri } private ModuleConfigXmlBuilder buildOnValidConfigXml() { - return buildOnValidConfigXml("1.6"); } private ModuleConfigXmlBuilder buildOnValidConfigXml(String version) { - return new ModuleConfigXmlBuilder(documentBuilder) .withModuleRoot() .withConfigVersion(version) @@ -1266,6 +1366,17 @@ public ModuleConfigXmlBuilder withAwareOfModules(String... modules) { return this; } + public ModuleConfigXmlBuilder withStartBeforeModules(String... modules) { + Element startBeforeModules = configXml.createElement("start_before_modules"); + for (String module : modules) { + Element startBeforeModule = configXml.createElement("start_before_module"); + startBeforeModule.setTextContent(module); + startBeforeModules.appendChild(startBeforeModule); + } + configXml.getDocumentElement().appendChild(startBeforeModules); + return this; + } + public ModuleConfigXmlBuilder withPrivilege(String name, String description) { Map children = new HashMap<>(); if (name != null) {