Skip to content

Commit

Permalink
TRUNK-5388 Make ModuleFileParser MessageSourceService dependency expl…
Browse files Browse the repository at this point in the history 
…icit

new
* add constructor ModuleFileParser(MessageSourceService) to clearly
state dependencies, which enables easier testing by passing in a mock or
use of a different implementation of a MessageSourceService
* add method ModuleFileParser.parse(File)
* add method ModuleFileparser.parse(InputStream)
which provide the same functionality/use the same implementation
underneath

deprecate
* deprecate all other constructors (3 in total)
* deprecate parse() in favor of parse(File), there is no need to keep
the file as state. This way we can create one parser and reuse it by
just passing in a different file to parse

tests
* move all tests which do not need the Context to be there to the
ModuleFileParserUnitTest which is now BaseContextMockTest and uses mocks
of the MessageSourceService
* one test still needs to stay in the BaseContextSensitive test since
the Context.getLocale() is still called. Did not add Locale as
dependency since I think this specific method getMessage(String key,
Object[] args) should be implementated in the MessageSourceService
as is getMessage(String) which abstracts away the users current locale
for us
  • Loading branch information
@teleivo
teleivo committed Apr 14, 2018
1 parent 4b7461d commit e4be754
Show file tree
Hide file tree
Showing 3 changed files with 1,644 additions and 1,478 deletions.
150 changes: 109 additions & 41 deletions api/src/main/java/org/openmrs/module/ModuleFileParser.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import java.util.IdentityHashMap;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Set;
import java.util.jar.JarFile;
import java.util.zip.ZipEntry;
Expand All @@ -35,6 +36,7 @@
import org.openmrs.Privilege;
import org.openmrs.api.context.Context;
import org.openmrs.customdatatype.CustomDatatype;
import org.openmrs.messagesource.MessageSourceService;
import org.openmrs.util.OpenmrsUtil;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
Expand All @@ -46,17 +48,21 @@
import org.xml.sax.InputSource;

/**
* This class will parse a file into an org.openmrs.module.Module object
* This class will parse an OpenMRS module, specifically its {@code config.xml} file into a {@link org.openmrs.module.Module} object.
* <p>Typical usage is:
* <ol>
* <li>Create a {@code ModuleFileParser} with {@link #ModuleFileParser(MessageSourceService)}.
* <li>Parse the module by passing the file to {@link #parse(File)}.</li>
* </ol>
* Note that the parser does not validate the {@code config.xml} file against the document type definition's (DTD).
*/
public class ModuleFileParser {

private static final Logger log = LoggerFactory.getLogger(ModuleFileParser.class);

private static final String MODULE_CONFIG_XML_FILENAME = "config.xml";

private static final String OPENMRS_MODULE_FILE_EXTENSION_TEST = ".omod";

private static final String OPENMRS_MODULE_FILE_EXTENSION = "omod";
private static final String OPENMRS_MODULE_FILE_EXTENSION = ".omod";

/**
* List out all of the possible version numbers for config files that openmrs has DTDs for.
Expand All @@ -74,35 +80,69 @@ public class ModuleFileParser {
validConfigVersions.add("1.6");
}

// TODO - remove this field once ModuleFileParser(File), ModuleFileParser(InputStream) are removed.
// There is no need to keep the file as state since moduleFileParser.parse(File) does not need it.
// this is also why all private methods that need access to the file for parsing or error message get it as a parameter
private File moduleFile;

private MessageSourceService messageSourceService;

/**
* Creates a ModuleFileParser.
*
* @param messageSourceService the message source used for error messages
* @since 2.2.0
*/
public ModuleFileParser(MessageSourceService messageSourceService) {
this.messageSourceService = Objects.requireNonNull(messageSourceService, "messageSourceService must not be null");
}

/**
* Constructor
*
* @param moduleFile the module (jar)file that will be parsed
* @deprecated since 2.2.0 use {@link #ModuleFileParser(MessageSourceService)}
*/
@Deprecated
public ModuleFileParser(File moduleFile) {
if (moduleFile == null) {
throw new ModuleException(Context.getMessageSourceService().getMessage("Module.error.fileCannotBeNull"));
}

if (!moduleFile.getName().endsWith(OPENMRS_MODULE_FILE_EXTENSION_TEST)) {
if (!moduleFile.getName().endsWith(OPENMRS_MODULE_FILE_EXTENSION)) {
throw new ModuleException(Context.getMessageSourceService().getMessage("Module.error.invalidFileExtension"),
moduleFile.getName());
moduleFile.getName());
}

this.moduleFile = moduleFile;
this.messageSourceService = Context.getMessageSourceService();
}

/**
* Convenience constructor to parse the given inputStream file into an omod. <br>
* This copies the stream into a temporary file just so things can be parsed.<br>
*
* @param inputStream the inputStream pointing to an omod file
* @deprecated since 2.2.0 use {@link #ModuleFileParser(MessageSourceService)}
*/
@Deprecated
public ModuleFileParser(InputStream inputStream) {
moduleFile = createTempFile("moduleUpgrade", OPENMRS_MODULE_FILE_EXTENSION);
this.messageSourceService = Context.getMessageSourceService();
this.moduleFile = createTempFile("moduleUpgrade", OPENMRS_MODULE_FILE_EXTENSION);
copyInputStreamToFile(inputStream, this.moduleFile);
}

/**
* Parses the given {@code InputStream} of an OpenMRS module into a {@code Module}.
* This copies the stream into a temporary file and close given {@code InputStream}.
*
* @param inputStream the inputStream pointing to an omod file
* @since 2.2.0
*/
public Module parse(InputStream inputStream) {
File moduleFile = createTempFile("moduleUpgrade", OPENMRS_MODULE_FILE_EXTENSION);
copyInputStreamToFile(inputStream, moduleFile);
return parse(moduleFile);
}

private File createTempFile(String prefix, String suffix) {
Expand All @@ -111,7 +151,7 @@ private File createTempFile(String prefix, String suffix) {
file = File.createTempFile(prefix, suffix);
}
catch (IOException e) {
throw new ModuleException(Context.getMessageSourceService().getMessage("Module.error.cannotCreateFile"), e);
throw new ModuleException(messageSourceService.getMessage("Module.error.cannotCreateFile"), e);
}
return file;
}
Expand All @@ -121,7 +161,7 @@ private void copyInputStreamToFile(InputStream inputStream, File file) {
OpenmrsUtil.copyFile(inputStream, outputStream);
}
catch (IOException e) {
throw new ModuleException(Context.getMessageSourceService().getMessage("Module.error.cannotCreateFile"), e);
throw new ModuleException(messageSourceService.getMessage("Module.error.cannotCreateFile"), e);
}
finally {
try {
Expand All @@ -131,55 +171,83 @@ private void copyInputStreamToFile(InputStream inputStream, File file) {
}
}


/**
* This constructor was created for testing purposes and is now deprecated.
* DO NOT USE.
*
* @deprecated since 2.2.0 use {@link #ModuleFileParser(MessageSourceService)}
*/
@Deprecated
ModuleFileParser() {
}

/**
* Get the module
* Get the module.
* If you use this method only do so together with {@link #ModuleFileParser(File)} or {@link #ModuleFileParser(InputStream)}.
* Best use {@link #ModuleFileParser(MessageSourceService)} and {@link #parse(File)}
* since this method is deprecated.
*
* @return new module object
* @deprecated since 2.2.0 use {@link #parse(File)}
*/
@Deprecated
public Module parse() throws ModuleException {

return createModule(getModuleConfigXml());
return parse(this.moduleFile);
}

/**
* Get the module from an OpenMRS module file.
*
* @param moduleFile the module file to be parsed
* @return new module object
* @since 2.2.0
*/
public Module parse(File moduleFile) {
if (moduleFile == null) {
throw new ModuleException(messageSourceService.getMessage("Module.error.fileCannotBeNull"));
}
if (!moduleFile.getName().endsWith(".omod")) {
throw new ModuleException(messageSourceService.getMessage("Module.error.invalidFileExtension"),
moduleFile.getName());
}
return createModule(getModuleConfigXml(moduleFile), moduleFile);
}

private Document getModuleConfigXml() {
private Document getModuleConfigXml(File moduleFile) {
Document config;
try (JarFile jarfile = new JarFile(moduleFile)) {
ZipEntry configEntry = getConfigXmlZipEntry(jarfile);
config = parseConfigXml(jarfile, configEntry);
ZipEntry configEntry = getConfigXmlZipEntry(jarfile, moduleFile);
config = parseConfigXml(jarfile, configEntry, moduleFile);
}
catch (IOException e) {
throw new ModuleException(Context.getMessageSourceService().getMessage("Module.error.cannotGetJarFile"),
throw new ModuleException(messageSourceService.getMessage("Module.error.cannotGetJarFile"),
moduleFile.getName(), e);
}
return config;
}

private ZipEntry getConfigXmlZipEntry(JarFile jarfile) {
private ZipEntry getConfigXmlZipEntry(JarFile jarfile, File moduleFile) {
ZipEntry config = jarfile.getEntry(MODULE_CONFIG_XML_FILENAME);
if (config == null) {
throw new ModuleException(Context.getMessageSourceService().getMessage("Module.error.noConfigFile"),
throw new ModuleException(messageSourceService.getMessage("Module.error.noConfigFile"),
moduleFile.getName());
}
return config;
}
private Document parseConfigXml(JarFile jarfile, ZipEntry configEntry) {

private Document parseConfigXml(JarFile jarfile, ZipEntry configEntry, File moduleFile) {
Document config;
try (InputStream configStream = jarfile.getInputStream(configEntry)) {
config = parseConfigXmlStream(configStream);
config = parseConfigXmlStream(configStream, moduleFile);
}
catch (IOException e) {
throw new ModuleException(Context.getMessageSourceService().getMessage(
throw new ModuleException(messageSourceService.getMessage(
"Module.error.cannotGetConfigFileStream"), moduleFile.getName(), e);
}
return config;
}
private Document parseConfigXmlStream(InputStream configStream) {

private Document parseConfigXmlStream(InputStream configStream, File moduleFile) {
Document config;
try {
DocumentBuilder db = newDocumentBuilder();
Expand All @@ -203,9 +271,8 @@ private Document parseConfigXmlStream(InputStream configStream) {
}

log.error("{} content: {}", MODULE_CONFIG_XML_FILENAME, output);
throw new ModuleException(
Context.getMessageSourceService().getMessage("Module.error.cannotParseConfigFile"), moduleFile
.getName(), e);
throw new ModuleException(messageSourceService.getMessage("Module.error.cannotParseConfigFile"),
moduleFile.getName(), e);
}
return config;
}
Expand All @@ -223,12 +290,12 @@ private DocumentBuilder newDocumentBuilder() throws ParserConfigurationException
return db;
}

private Module createModule(Document config) {
private Module createModule(Document config, File moduleFile) {
Element configRoot = config.getDocumentElement();

String configVersion = ensureValidModuleConfigVersion(configRoot);
String configVersion = ensureValidModuleConfigVersion(configRoot, moduleFile);

String name = ensureNonEmptyName(configRoot);
String name = ensureNonEmptyName(configRoot, moduleFile);
String moduleId = ensureNonEmptyId(configRoot, name);
String packageName = ensureNonEmptyPackage(configRoot, name);

Expand Down Expand Up @@ -260,21 +327,22 @@ private Module createModule(Document config) {

return module;
}
private String ensureValidModuleConfigVersion(Element configRoot) {

private String ensureValidModuleConfigVersion(Element configRoot, File moduleFile) {
String configVersion = configRoot.getAttribute("configVersion").trim();
validateModuleConfigVersion(configVersion);
validateModuleConfigVersion(configVersion, moduleFile);
return configVersion;
}
private void validateModuleConfigVersion(String version) {

private void validateModuleConfigVersion(String version, File moduleFile) {
if (!validConfigVersions.contains(version)) {
throw new ModuleException(Context.getMessageSourceService().getMessage("Module.error.invalidConfigVersion",
new Object[] { version, String.join(", ", validConfigVersions) }, Context.getLocale()), moduleFile.getName());
new Object[] { version, String.join(", ", validConfigVersions) }, Context.getLocale()),
moduleFile.getName());
}
}

private String ensureNonEmptyName(Element configRoot) {
private String ensureNonEmptyName(Element configRoot, File moduleFile) {
return getTrimmedElementOrFail(configRoot, "name", "Module.error.nameCannotBeEmpty", moduleFile.getName());
}

Expand Down Expand Up @@ -525,7 +593,7 @@ private Collection<String> splitTagContentByWhitespace(Element rootNode, String
private String getTrimmedElementOrFail(Element rootNode, String elementName, String errorMessageKey, String moduleName) {
String element = getElementTrimmed(rootNode, elementName);
if (element == null || element.length() == 0) {
throw new ModuleException(Context.getMessageSourceService().getMessage(errorMessageKey),
throw new ModuleException(messageSourceService.getMessage(errorMessageKey),
moduleName);
}
return element;
Expand Down
Loading

0 comments on commit e4be754

Please sign in to comment.