From 1fefe4411559fb183a7c3cb87ceda5f9185cbbc4 Mon Sep 17 00:00:00 2001 From: Ian Date: Tue, 24 Sep 2024 14:50:56 -0400 Subject: [PATCH] Code review and some improvements --- .../openmrs/maven/plugins/BuildDistro.java | 5 +- .../java/org/openmrs/maven/plugins/Setup.java | 7 +- .../maven/plugins/utility/ContentHelper.java | 95 ++++++------------- .../plugins/utility/ModuleInstaller.java | 42 +++++--- .../plugins/model/BaseSdkProperties.java | 15 ++- 5 files changed, 80 insertions(+), 84 deletions(-) diff --git a/maven-plugin/src/main/java/org/openmrs/maven/plugins/BuildDistro.java b/maven-plugin/src/main/java/org/openmrs/maven/plugins/BuildDistro.java index c10fbb1d..91ef6bde 100644 --- a/maven-plugin/src/main/java/org/openmrs/maven/plugins/BuildDistro.java +++ b/maven-plugin/src/main/java/org/openmrs/maven/plugins/BuildDistro.java @@ -324,9 +324,10 @@ private String buildDistro(File targetDirectory, Artifact distroArtifact, Distro File configDir = new File(web, SDKConstants.OPENMRS_SERVER_CONFIGURATION); configDir.mkdir(); setConfigFolder(configDir, distroProperties, distroArtifact); + ContentHelper.downloadAndMoveContentBackendConfig(web, distroProperties, moduleInstaller, wizard); + spaInstaller.installFromDistroProperties(web, distroProperties, ignorePeerDependencies, overrideReuseNodeCache); - ContentHelper.deleteTempContentFolder(web); File owasDir = new File(web, "owa"); owasDir.mkdir(); @@ -513,7 +514,7 @@ private void writeTemplatedFile(File targetDirectory, String version, String pat } private String adjustImageName(String part) { - return part.replaceAll("\\s+", "").toLowerCase(); + return part != null ? part.replaceAll("\\s+", "").toLowerCase() : ""; } private void copyDbDump(File targetDirectory, InputStream stream) throws MojoExecutionException { diff --git a/maven-plugin/src/main/java/org/openmrs/maven/plugins/Setup.java b/maven-plugin/src/main/java/org/openmrs/maven/plugins/Setup.java index e1f46070..b71f17fa 100644 --- a/maven-plugin/src/main/java/org/openmrs/maven/plugins/Setup.java +++ b/maven-plugin/src/main/java/org/openmrs/maven/plugins/Setup.java @@ -279,13 +279,16 @@ public void setup(Server server, DistroProperties distroProperties) throws MojoE setServerVersionsFromDistroProperties(server, distroProperties); distroHelper.parseContentProperties(distroProperties); moduleInstaller.installModulesForDistro(server, distroProperties, distroHelper); - setConfigFolder(server, distroProperties); + ContentHelper.downloadAndMoveContentBackendConfig(server.getServerDirectory(), distroProperties, moduleInstaller, wizard); + if (spaInstaller != null) { spaInstaller.installFromDistroProperties(server.getServerDirectory(), distroProperties, ignorePeerDependencies, overrideReuseNodeCache); } - ContentHelper.deleteTempContentFolder(server.getServerDirectory()); + installOWAs(server, distroProperties); + + setConfigFolder(server, distroProperties); } else { moduleInstaller.installDefaultModules(server); } diff --git a/maven-plugin/src/main/java/org/openmrs/maven/plugins/utility/ContentHelper.java b/maven-plugin/src/main/java/org/openmrs/maven/plugins/utility/ContentHelper.java index f0602793..06168e42 100644 --- a/maven-plugin/src/main/java/org/openmrs/maven/plugins/utility/ContentHelper.java +++ b/maven-plugin/src/main/java/org/openmrs/maven/plugins/utility/ContentHelper.java @@ -3,9 +3,8 @@ import java.io.File; import java.io.IOException; import java.nio.file.Files; -import java.nio.file.StandardCopyOption; -import java.util.ArrayList; -import java.util.Collection; +import java.nio.file.Path; +import java.nio.file.Paths; import java.util.List; import org.apache.commons.io.FileUtils; @@ -18,78 +17,46 @@ */ public class ContentHelper { - public static final String TEMP_CONTENT_FOLDER = "temp-content"; - public static final String FRONTEND_CONFIG_FOLDER = File.separator + "configs" + File.separator + "frontend_config"; - public static final String BACKEND_CONFIG_FOLDER = "configs" + File.separator + "backend_config"; + public static final String FRONTEND_CONFIG_FOLDER = Paths.get("configs", "frontend_config").toString(); + public static final String BACKEND_CONFIG_FOLDER = Paths.get("configs", "backend_config").toString(); - public static void downloadContent(File serverDirectory, Artifact contentArtifact, ModuleInstaller moduleInstaller, File targetDir) throws MojoExecutionException { - + public static void downloadContent(Artifact contentArtifact, ModuleInstaller moduleInstaller, File targetDir) throws MojoExecutionException { String artifactId = contentArtifact.getArtifactId(); - //create a artifact folder under temp_content - File sourceDir = new File(serverDirectory, TEMP_CONTENT_FOLDER + File.separator + artifactId); - sourceDir.mkdirs(); - - moduleInstaller.installAndUnpackModule(contentArtifact, sourceDir.getAbsolutePath()); + // create a temporary artifact folder + File sourceDir; + try { + sourceDir = Files.createTempDirectory("openmrs-sdk-" + artifactId + "-").toFile(); + } catch (IOException e) { + throw new MojoExecutionException("Exception while trying to create temporary directory", e); + } + + moduleInstaller.installAndUnpackModule(contentArtifact, sourceDir.getAbsolutePath()); moveBackendConfig(artifactId, sourceDir, targetDir); + + FileUtils.deleteQuietly(sourceDir); } private static void moveBackendConfig(String artifactId, File sourceDir, File targetDir) throws MojoExecutionException { try { - File backendConfigFiles = new File(sourceDir, BACKEND_CONFIG_FOLDER); - - for (File config : backendConfigFiles.listFiles()) { - // Find a corresponding directory in the configurationDir with the same name as backendConfig - File matchingConfigurationDir = new File(targetDir, config.getName()); - File destDir = null; - if (!matchingConfigurationDir.exists() || !matchingConfigurationDir.isDirectory()) { - // Folder doesn't exist so create it - matchingConfigurationDir.mkdirs(); - destDir = matchingConfigurationDir; + File backendConfigFiles = sourceDir.toPath().resolve(BACKEND_CONFIG_FOLDER).toFile(); + Path targetPath = targetDir.toPath().toAbsolutePath(); + + if (backendConfigFiles.exists()) { + File[] configDirectories = backendConfigFiles.listFiles(File::isDirectory); + if (configDirectories != null) { + for (File config : configDirectories) { + Path destDir = targetPath.resolve(config.getName()).resolve(artifactId); + Files.createDirectories(destDir); + + //copy config files to the matching configuration folder + FileUtils.copyDirectory(config, destDir.toFile()); + } } - // Create a new artifact folder under the matching configuration folder - destDir = new File(matchingConfigurationDir, artifactId); - - //copy config files to the matching configuration folder - FileUtils.copyDirectory(config, destDir); } } catch (IOException e) { throw new MojoExecutionException("Error copying backend configuration: " + e.getMessage(), e); } - } - - public static List getContentFrontendConfigFiles(DistroProperties distroProperties, File serverDirectory) { - List contentConfigFiles = new ArrayList(); - - List artifacts = distroProperties.getContentArtifacts(); - if (artifacts != null) { - for (Artifact artifact : artifacts) { - String artifactId = artifact.getArtifactId(); - File artifactFrontendDir = new File(serverDirectory, - TEMP_CONTENT_FOLDER + File.separator + artifactId + FRONTEND_CONFIG_FOLDER); - - // Add all files from the artifact directory - if (artifactFrontendDir.exists() && artifactFrontendDir.isDirectory()) { - //just include json files and set recursive to true - Collection files = FileUtils.listFiles(artifactFrontendDir, null, true); - contentConfigFiles.addAll(files); - } - } - } - return contentConfigFiles; - } - - public static void deleteTempContentFolder(File serverDirectory) throws MojoExecutionException { - File tempContentDir = new File(serverDirectory, TEMP_CONTENT_FOLDER); - try { - if (tempContentDir.exists()) { - FileUtils.deleteDirectory(tempContentDir); - } - } - catch (IOException e) { - throw new MojoExecutionException("Failed to delete temporary content folder at: " - + tempContentDir.getAbsolutePath() + " due to: " + e.getMessage(), e); - } } public static void downloadAndMoveContentBackendConfig(File serverDirectory, DistroProperties distroProperties, ModuleInstaller moduleInstaller, Wizard wizard) throws MojoExecutionException { @@ -97,10 +64,10 @@ public static void downloadAndMoveContentBackendConfig(File serverDirectory, Dis File targetDir = new File(serverDirectory, SDKConstants.OPENMRS_SERVER_CONFIGURATION); List contents = distroProperties.getContentArtifacts(); - if (!contents.isEmpty()) { + if (contents != null) { for (Artifact content : contents) { wizard.showMessage("Downloading Content: " + content + "\n"); - ContentHelper.downloadContent(serverDirectory, content, moduleInstaller, targetDir); + ContentHelper.downloadContent(content, moduleInstaller, targetDir); } } } diff --git a/maven-plugin/src/main/java/org/openmrs/maven/plugins/utility/ModuleInstaller.java b/maven-plugin/src/main/java/org/openmrs/maven/plugins/utility/ModuleInstaller.java index 856e0174..07b33789 100644 --- a/maven-plugin/src/main/java/org/openmrs/maven/plugins/utility/ModuleInstaller.java +++ b/maven-plugin/src/main/java/org/openmrs/maven/plugins/utility/ModuleInstaller.java @@ -1,5 +1,6 @@ package org.openmrs.maven.plugins.utility; +import org.apache.commons.io.FileUtils; import org.apache.maven.execution.MavenSession; import org.apache.maven.plugin.BuildPluginManager; import org.apache.maven.plugin.MojoExecutionException; @@ -10,7 +11,11 @@ import org.twdata.maven.mojoexecutor.MojoExecutor; import java.io.File; +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; import java.util.ArrayList; +import java.util.Arrays; import java.util.List; import static org.twdata.maven.mojoexecutor.MojoExecutor.Element; @@ -29,6 +34,8 @@ */ public class ModuleInstaller { + private static final String GOAL_COPY = "copy"; + private static final String GOAL_UNPACK = "unpack"; final MavenProject mavenProject; @@ -74,23 +81,31 @@ public void installModulesForDistro(Server server, DistroProperties properties, } public void installModules(List artifacts, String outputDir) throws MojoExecutionException { - final String goal = "copy"; - prepareModules(artifacts.toArray(new Artifact[0]), outputDir, goal); + prepareModules(artifacts.toArray(new Artifact[0]), outputDir, GOAL_COPY); } public void installModules(Artifact[] artifacts, String outputDir) throws MojoExecutionException { - final String goal = "copy"; - prepareModules(artifacts, outputDir, goal); + prepareModules(artifacts, outputDir, GOAL_COPY); } public void installModule(Artifact artifact, String outputDir) throws MojoExecutionException { - final String goal = "copy"; - prepareModules(new Artifact[] { artifact }, outputDir, goal); + prepareModules(new Artifact[] { artifact }, outputDir, GOAL_COPY); } - + public void installAndUnpackModule(Artifact artifact, String outputDir) throws MojoExecutionException { - final String goal = GOAL_UNPACK; - prepareModules(new Artifact[] { artifact }, outputDir, goal); + Path markersDirectory; + try { + markersDirectory = Files.createTempDirectory("openmrs-sdk-markers"); + } catch (IOException e) { + throw new MojoExecutionException("Error creating markers directory", e); + } + + prepareModules(new Artifact[] { artifact }, outputDir, GOAL_UNPACK, + element("overWriteSnapshots", "true"), + element("overWriteReleases", "true"), + element("markersDirectory", markersDirectory.toAbsolutePath().toString())); + + FileUtils.deleteQuietly(markersDirectory.toFile()); } /** @@ -100,7 +115,7 @@ public void installAndUnpackModule(Artifact artifact, String outputDir) throws M * @param goal * @throws MojoExecutionException */ - private void prepareModules(Artifact[] artifacts, String outputDir, String goal) throws MojoExecutionException { + private void prepareModules(Artifact[] artifacts, String outputDir, String goal, MojoExecutor.Element... additionalConfiguration) throws MojoExecutionException { MojoExecutor.Element[] artifactItems = new MojoExecutor.Element[artifacts.length]; for (int index = 0; index < artifacts.length; index++) { artifactItems[index] = artifacts[index].toElement(outputDir); @@ -108,10 +123,9 @@ private void prepareModules(Artifact[] artifacts, String outputDir, String goal) List configuration = new ArrayList<>(); configuration.add(element("artifactItems", artifactItems)); - if (goal.equals(GOAL_UNPACK)) { - configuration.add(element("overWriteSnapshots", "true")); - configuration.add(element("overWriteReleases", "true")); - } + + configuration.addAll(Arrays.asList(additionalConfiguration)); + executeMojo( plugin( groupId(SDKConstants.DEPENDENCY_PLUGIN_GROUP_ID), diff --git a/sdk-commons/src/main/java/org/openmrs/maven/plugins/model/BaseSdkProperties.java b/sdk-commons/src/main/java/org/openmrs/maven/plugins/model/BaseSdkProperties.java index 8e0b154a..a75cce6d 100644 --- a/sdk-commons/src/main/java/org/openmrs/maven/plugins/model/BaseSdkProperties.java +++ b/sdk-commons/src/main/java/org/openmrs/maven/plugins/model/BaseSdkProperties.java @@ -82,7 +82,7 @@ public void setVersion(String version){ } public String getName(){ - return getParam("name"); + return getParam("name", "openmrs"); } public void setName(String name){ @@ -249,7 +249,18 @@ else if (key.equals("distro.referenceapplication")) { * @param key * @return */ - public String getParam(String key) {return properties.getProperty(key); } + public String getParam(String key) { + return properties.getProperty(key); + } + + /** + * get param from properties + * @param key + * @return + */ + public String getParam(String key, String defaultValue) { + return properties.getProperty(key, defaultValue); + } public Artifact getModuleArtifact(String artifactId){ String key = TYPE_OMOD + "." + artifactId;