Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SDK-365 and SDK-369 - Ensure content package validation occurs prior … #318

Merged
merged 1 commit into from
Dec 10, 2024
Merged

SDK-365 and SDK-369 - Ensure content package validation occurs prior … #318

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 17 additions & 1 deletion integration-tests/src/test/java/org/openmrs/maven/plugins/AbstractSdkIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ static synchronized int nextCounter() {
return counter++;
}

public String setupTestServer(String distro) throws Exception{
public String setupTestServer(String distro) throws Exception {
Verifier setupServer = new Verifier(testDirectory.getAbsolutePath());
String serverId = UUID.randomUUID().toString();
try {
Expand All @@ -171,6 +171,22 @@ public String setupTestServer(String distro) throws Exception{
return serverId;
}

public void setupContentPackage(String contentPackageDir) throws Exception {
Path sourcePath = testResourceDir.resolve(TEST_DIRECTORY).resolve("contentpackages").resolve(contentPackageDir);
Path targetPath = testDirectoryPath.resolve("contentpackages").resolve(contentPackageDir);
FileUtils.copyDirectory(sourcePath.toFile(), targetPath.toFile());
Verifier setupContentPackage = new Verifier(targetPath.toFile().getAbsolutePath());
try {
addAnswer("y");
addTaskParam(setupContentPackage, BATCH_ANSWERS, getAnswers());
setupContentPackage.executeGoal(resolveSdkArtifact() + ":build");
}
finally {
cleanAnswers();
verifier.resetStreams();
}
}

Copy link
Member Author

Choose a reason for hiding this comment

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

This allows us to create any content packages we wish for testing, without relying on published versions in the cloud

private void cleanAnswers() {
batchAnswers.clear();
}
Expand Down
25 changes: 25 additions & 0 deletions integration-tests/src/test/java/org/openmrs/maven/plugins/BuildDistroIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@
import java.nio.file.Path;
import java.nio.file.Paths;

import static org.junit.Assert.assertNotNull;

public class BuildDistroIT extends AbstractSdkIT {

@Test
Expand Down Expand Up @@ -211,4 +213,27 @@ public void testBuildDistroWithWithContentPackageWithNoNamespace() throws Except
assertFilePresent("target", "web", "openmrs_config", "conceptsources", "conceptsources.csv");
assertFilePresent("target", "web", "openmrs_config", "encountertypes", "encountertypes.csv");
}

@Test
public void testBuildDistroWithMissingContentDependencies() throws Exception {
setupContentPackage("testpackage1");

includeDistroPropertiesFile("openmrs-distro-content-package-missing-dependencies.properties");
addTaskParam("dir", "distro");
addTaskParam("ignorePeerDependencies", "false");
Exception exception = null;
try {
executeTask("build-distro");
}
catch (Exception e) {
exception = e;
}
assertNotNull(exception);
assertLogContains("requires module org.openmrs.module:fhir2-omod version >=1.8.0");
assertLogContains("requires module org.openmrs.module:webservices.rest-omod version >=2.42.0");
assertLogContains("requires module org.openmrs.module:openconceptlab-omod version >=2.3.0");
assertLogContains("requires module org.openmrs.module:initializer-omod version >=2.5.2");
assertLogContains("requires module org.openmrs.module:o3forms-omod version >=2.2.0");
assertLogContains("requires esm @openmrs/esm-patient-chart-app version >=8.1.0");
}
}
27 changes: 27 additions & 0 deletions integration-tests/src/test/java/org/openmrs/maven/plugins/DeployIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -348,4 +348,31 @@ public void deploy_shouldDeployOwas() throws Exception {
server = Server.loadServer(testDirectoryPath.resolve(testServerId));
assertThat(server.getOwaArtifacts().size(), equalTo(3));
}

@Test
public void testDeployWithMissingContentDependencies() throws Exception {
setupContentPackage("testpackage1");

testServerId = setupTestServer("referenceapplication:2.2");

includeDistroPropertiesFile("openmrs-distro-content-package-missing-dependencies.properties");
addAnswer(testServerId);
addAnswer("y");
addAnswer("y");

Exception exception = null;
try {
executeTask("deploy");
}
catch (Exception e) {
exception = e;
}
assertNotNull(exception);
assertLogContains("requires module org.openmrs.module:fhir2-omod version >=1.8.0");
assertLogContains("requires module org.openmrs.module:webservices.rest-omod version >=2.42.0");
assertLogContains("requires module org.openmrs.module:openconceptlab-omod version >=2.3.0");
assertLogContains("requires module org.openmrs.module:initializer-omod version >=2.5.2");
assertLogContains("requires module org.openmrs.module:o3forms-omod version >=2.2.0");
assertLogContains("requires esm @openmrs/esm-patient-chart-app version >=8.1.0");
}
}
30 changes: 30 additions & 0 deletions integration-tests/src/test/java/org/openmrs/maven/plugins/SetupIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import static org.hamcrest.CoreMatchers.nullValue;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.core.Is.is;
import static org.junit.Assert.assertNotNull;
import static org.openmrs.maven.plugins.SdkMatchers.hasModuleVersion;
import static org.openmrs.maven.plugins.SdkMatchers.hasPropertyEqualTo;
import static org.openmrs.maven.plugins.SdkMatchers.hasPropertyThatContains;
Expand Down Expand Up @@ -527,6 +528,35 @@ public void setup_shouldInstallWithContentPackageWithNoNamespace() throws Except
assertFilePresent(serverId, "configuration", "encountertypes", "encountertypes.csv");
}

@Test
public void testSetupWithMissingContentDependencies() throws Exception {
setupContentPackage("testpackage1");

includeDistroPropertiesFile("openmrs-distro-content-package-missing-dependencies.properties");
addTaskParam("distro", testDirectory.toString() + File.separator + "openmrs-distro.properties");
addMockDbSettings();

String serverId = UUID.randomUUID().toString();
addAnswer(serverId);
addAnswer("1044");
addAnswer("8080");

Exception exception = null;
try {
executeTask("setup");
}
catch (Exception e) {
exception = e;
}
assertNotNull(exception);
assertLogContains("requires module org.openmrs.module:fhir2-omod version >=1.8.0");
assertLogContains("requires module org.openmrs.module:webservices.rest-omod version >=2.42.0");
assertLogContains("requires module org.openmrs.module:openconceptlab-omod version >=2.3.0");
assertLogContains("requires module org.openmrs.module:initializer-omod version >=2.5.2");
assertLogContains("requires module org.openmrs.module:o3forms-omod version >=2.2.0");
assertLogContains("requires esm @openmrs/esm-patient-chart-app version >=8.1.0");
}

Copy link
Member Author

Choose a reason for hiding this comment

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

I have added integration tests for the 3 impacted goals with respect to content package validation. In each case, the new default behavior is to fail, and provide a clear message as to why the build failed and what is needed to fix it.

private String readValueFromPropertyKey(File propertiesFile, String key) throws Exception {

InputStream in = new FileInputStream(propertiesFile);
Expand Down
29 changes: 29 additions & 0 deletions integration-tests/src/test/java/org/openmrs/maven/plugins/utility/DistroHelperIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
package org.openmrs.maven.plugins.utility;


import lombok.Getter;
import lombok.Setter;
import org.junit.Test;
import org.openmrs.maven.plugins.AbstractMavenIT;
import org.openmrs.maven.plugins.model.Artifact;

import java.util.Map;

import static org.hamcrest.Matchers.equalTo;
import static org.junit.Assert.assertThat;

@Getter @Setter
public class DistroHelperIT extends AbstractMavenIT {

@Test
public void getFrontendModulesFromArtifact_shouldGetFrontendModulesAndVersions() throws Exception {
executeTest(() -> {
DistroHelper distroHelper = new DistroHelper(getMavenEnvironment());
Artifact artifact = new Artifact("openmrs-frontend-pihemr", "1.8.0", "org.pih.openmrs", "zip");
Map<String, String> m = distroHelper.getFrontendModulesFromArtifact(artifact, "openmrs-frontend-pihemr-1.8.0");
assertThat(m.size(), equalTo(39));
assertThat(m.get("@openmrs/esm-ward-app"), equalTo("8.0.3-pre.4390"));
assertThat(m.get("@openmrs/esm-login-app"), equalTo("5.8.2-pre.2483"));
});
}
}
22 changes: 22 additions & 0 deletions ...ation-tests/src/test/resources/integration-test/contentpackages/testpackage1/assembly.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
<assembly>
<id>assemble-content</id>
<formats>
<format>zip</format>
</formats>
<includeBaseDirectory>false</includeBaseDirectory>
<fileSets>
<fileSet>
<directory>${project.build.directory}</directory>
<includes>
<include>content.properties</include>
</includes>
<outputDirectory>/</outputDirectory>
</fileSet>
<fileSet>
<directory>${project.basedir}/configuration</directory>
<includes>
<include>**/*</include>
</includes>
</fileSet>
</fileSets>
</assembly>
Empty file added ...s/src/test/resources/integration-test/contentpackages/testpackage1/configuration/.gitkeep
View file
Open in desktop
Empty file.
17 changes: 17 additions & 0 deletions ...tests/src/test/resources/integration-test/contentpackages/testpackage1/content.properties
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
# content.properties - Metadata for the content package and its dependencies

name=${project.artifactId}
version=${project.version}

# Platform minimum version
war.openmrs=>=2.4.0

# Backend modules
omod.fhir2=>=1.8.0
omod.webservices.rest=>=2.42.0
omod.initializer=>=2.5.2
omod.openconceptlab=>=2.3.0
omod.o3forms=>=2.2.0

# Frontend modules
spa.frontendModules.@openmrs/esm-patient-chart-app=>=8.1.0
70 changes: 70 additions & 0 deletions integration-tests/src/test/resources/integration-test/contentpackages/testpackage1/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
<project
xmlns="http://maven.apache.org/POM/4.0.0"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://maven.apache.org/POM/4.0.0
http://maven.apache.org/xsd/maven-4.0.0.xsd">
<modelVersion>4.0.0</modelVersion>

<groupId>org.openmrs.maven.sdk.test</groupId>
<artifactId>testpackage1</artifactId>
<version>1.0.0</version>
<packaging>pom</packaging>

<build>
<plugins>
<plugin>
<artifactId>maven-resources-plugin</artifactId>
<version>3.3.1</version>
<executions>
<execution>
<id>copy-resources</id>
<phase>process-resources</phase>
<goals>
<goal>resources</goal>
</goals>
<configuration>
<outputDirectory>${project.build.directory}</outputDirectory>
<resources>
<resource>
<directory>${project.basedir}</directory>
<includes>
<include>content.properties</include>
</includes>
<filtering>true</filtering>
</resource>
</resources>
</configuration>
</execution>
</executions>
</plugin>
<plugin>
<artifactId>maven-assembly-plugin</artifactId>
<version>3.7.1</version>
<configuration>
<appendAssemblyId>false</appendAssemblyId>
<descriptors>
<descriptor>${project.basedir}/assembly.xml</descriptor>
</descriptors>
</configuration>
<executions>
<execution>
<id>make-assembly</id>
<phase>package</phase>
<goals>
<goal>single</goal>
</goals>
</execution>
</executions>
</plugin>
</plugins>
</build>

<repositories>
<repository>
<id>openmrs-repo</id>
<name>OpenMRS repository</name>
<url>https://mavenrepo.openmrs.org/nexus/content/repositories/public</url>
</repository>
</repositories>

</project>
7 changes: 7 additions & 0 deletions ...resources/integration-test/openmrs-distro-content-package-missing-dependencies.properties
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
name=Content Package Example
version=1.0.0
war.openmrs=2.6.9
content.testpackage1.groupId=org.openmrs.maven.sdk.test
content.testpackage1.type=zip
content.testpackage1=1.0.0
db.h2.supported=true
2 changes: 1 addition & 1 deletion maven-plugin/src/main/java/org/openmrs/maven/plugins/BuildDistro.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -246,7 +246,7 @@ private String buildDistro(File targetDirectory, Distribution distribution) thro
DistroProperties distroProperties = distribution.getEffectiveProperties();

// First do content package validation
distroHelper.parseContentProperties(distroProperties);
distroHelper.validateDistribution(distroProperties);

InputStream dbDumpStream;
wizard.showMessage("Downloading modules...\n");
Expand Down
16 changes: 5 additions & 11 deletions maven-plugin/src/main/java/org/openmrs/maven/plugins/ServerUpgrader.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,9 @@ public void upgradeToDistro(Server server, Distribution distribution, boolean ig
return;
}
}

parentTask.distroHelper.validateDistribution(distroProperties);
Copy link
Member Author

Choose a reason for hiding this comment

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

Per SDK-365, this moves the validation earlier in the process. Previously this was not running until after war/module/etc changes had already been applied to the server.


server.saveBackupProperties();
server.deleteServerTmpDirectory();

Expand Down Expand Up @@ -128,9 +131,6 @@ public void upgradeToDistro(Server server, Distribution distribution, boolean ig

if (configChanges.hasChanges() || contentChanges.hasChanges()) {

// TODO: This should happen in a higher-level validation stage and this method needs refactoring
parentTask.distroHelper.parseContentProperties(distroProperties);

File configDir = new File(server.getServerDirectory(), SDKConstants.OPENMRS_SERVER_CONFIGURATION);
if (configDir.exists()) {
parentTask.wizard.showMessage("Removing existing configuration and content packages");
Expand Down Expand Up @@ -226,14 +226,8 @@ public UpgradeDifferential calculateUpdateDifferential(Server server, Distributi
upgradeDifferential.setConfigChanges(new UpgradeDifferential.ArtifactChanges(oldConfig, newConfig));

// Content
List<Artifact> oldContent = new ArrayList<>();
for (ContentPackage contentPackage : server.getContentPackages()) {
oldContent.add(contentPackage.getArtifact());
}
List<Artifact> newContent = new ArrayList<>();
for (ContentPackage contentPackage : distroProperties.getContentPackages()) {
newContent.add(contentPackage.getArtifact());
}
List<Artifact> oldContent = server.getContentPackageArtifacts();
List<Artifact> newContent = distroProperties.getContentPackageArtifacts();
upgradeDifferential.setContentChanges(new UpgradeDifferential.ArtifactChanges(oldContent, newContent));

return upgradeDifferential;
Expand Down
2 changes: 1 addition & 1 deletion maven-plugin/src/main/java/org/openmrs/maven/plugins/Setup.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -281,7 +281,7 @@ public void setup(Server server, DistroProperties distroProperties) throws MojoE
distroHelper.savePropertiesToServer(distroProperties, server);

setServerVersionsFromDistroProperties(server, distroProperties);
distroHelper.parseContentProperties(distroProperties);
distroHelper.validateDistribution(distroProperties);
moduleInstaller.installModulesForDistro(server, distroProperties);

File configurationDir = new File(server.getServerDirectory(), SDKConstants.OPENMRS_SERVER_CONFIGURATION);
Expand Down
2 changes: 1 addition & 1 deletion pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -407,7 +407,7 @@
<dependency>
<groupId>org.semver4j</groupId>
<artifactId>semver4j</artifactId>
<version>5.3.0</version>
<version>5.4.0</version>
</dependency>

<!-- console input and output -->
Expand Down
Loading
Loading