Skip to content

Commit

Permalink
TRUNK-4004: Expose start_before_modules functionality better (#4575)
Browse files Browse the repository at this point in the history
  • Loading branch information
@ibacher
ibacher committed Mar 6, 2024
1 parent c41cead commit cca3ac0
Show file tree
Hide file tree
Showing 5 changed files with 146 additions and 20 deletions.
20 changes: 18 additions & 2 deletions api/src/main/java/org/openmrs/module/ModuleFileParser.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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>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
Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -375,7 +389,9 @@ private Map<String, String> extractAwareOfModules(Element configRoot) {
}

private Map<String, String> extractStartBeforeModules(Element configRoot) {
return extractModulesWithVersionAttribute(configRoot, "module", "start_before_modules");
Map<String, String> result = extractModulesWithVersionAttribute(configRoot, "module", "start_before_modules");
result.putAll(extractModulesWithVersionAttribute(configRoot, "start_before_module", "start_before_modules"));
return result;
}

private Map<String, String> extractModulesWithVersionAttribute(Element configRoot, String elementName,
Expand Down
2 changes: 1 addition & 1 deletion api/src/main/resources/org/openmrs/module/dtd/README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ your config. Like so
```xml
<?xml version="1.0" encoding="UTF-8"?>
<!DOCTYPE module PUBLIC "-//OpenMRS//DTD OpenMRS Config 1.0//EN"
"http://resources.openmrs.org/doctype/config-1.5.dtd">
"https://resources.openmrs.org/doctype/config-1.5.dtd">
<module configVersion="1.5">
...
</module>
Expand Down
5 changes: 5 additions & 0 deletions api/src/main/resources/org/openmrs/module/dtd/config-1.6.dtd
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
(require_database_version?),
(require_modules?),
(aware_of_modules?),
(start_before_modules?),
(mandatory?),
(library*),
(extension*),
Expand Down Expand Up @@ -51,6 +52,10 @@
<!ELEMENT aware_of_module (#PCDATA)>
<!ATTLIST aware_of_module version CDATA #IMPLIED>

<!ELEMENT start_before_modules (module+)>
<!ELEMENT start_before_module (#PCDATA)>
<!ATTLIST start_before_module version CDATA #IMPLIED>

<!ELEMENT mandatory (#PCDATA)>

<!ELEMENT library EMPTY>
Expand Down
24 changes: 9 additions & 15 deletions api/src/test/java/org/openmrs/module/ModuleFileParserTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"));
Expand Down Expand Up @@ -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());

Expand Down
115 changes: 113 additions & 2 deletions api/src/test/java/org/openmrs/module/ModuleFileParserUnitTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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 {

Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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<String, String> children = new HashMap<>();
if (name != null) {
Expand Down

2 comments on commit cca3ac0

@mogoodrich
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fyi @ibacher you saw the test/build failures due to this?

@ibacher
Copy link
Member Author

@ibacher ibacher commented on cca3ac0 Mar 7, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. Fixed that.

Please sign in to comment.