Skip to content

Commit e1e7fad

Browse files
mseatonmseaton
authored
SDK-365 and SDK-369 - Ensure content package validation occurs prior to destructive changes, and do not automatically change distribution/server based on content package requirements. (#318)
1 parent e9477b0 commit e1e7fad

File tree

22 files changed

+595
-264
lines changed

22 files changed

+595
-264
lines changed

‎integration-tests/src/test/java/org/openmrs/maven/plugins/AbstractSdkIT.java

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -145,7 +145,7 @@ static synchronized int nextCounter() {
145145
return counter++;
146146
}
147147

148-
public String setupTestServer(String distro) throws Exception{
148+
public String setupTestServer(String distro) throws Exception {
149149
Verifier setupServer = new Verifier(testDirectory.getAbsolutePath());
150150
String serverId = UUID.randomUUID().toString();
151151
try {
@@ -171,6 +171,22 @@ public String setupTestServer(String distro) throws Exception{
171171
return serverId;
172172
}
173173

174+
public void setupContentPackage(String contentPackageDir) throws Exception {
175+
Path sourcePath = testResourceDir.resolve(TEST_DIRECTORY).resolve("contentpackages").resolve(contentPackageDir);
176+
Path targetPath = testDirectoryPath.resolve("contentpackages").resolve(contentPackageDir);
177+
FileUtils.copyDirectory(sourcePath.toFile(), targetPath.toFile());
178+
Verifier setupContentPackage = new Verifier(targetPath.toFile().getAbsolutePath());
179+
try {
180+
addAnswer("y");
181+
addTaskParam(setupContentPackage, BATCH_ANSWERS, getAnswers());
182+
setupContentPackage.executeGoal(resolveSdkArtifact() + ":build");
183+
}
184+
finally {
185+
cleanAnswers();
186+
verifier.resetStreams();
187+
}
188+
}
189+
174190
private void cleanAnswers() {
175191
batchAnswers.clear();
176192
}

‎integration-tests/src/test/java/org/openmrs/maven/plugins/BuildDistroIT.java

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@
77
import java.nio.file.Path;
88
import java.nio.file.Paths;
99

10+
import static org.junit.Assert.assertNotNull;
11+
1012
public class BuildDistroIT extends AbstractSdkIT {
1113

1214
@Test
@@ -211,4 +213,27 @@ public void testBuildDistroWithWithContentPackageWithNoNamespace() throws Except
211213
assertFilePresent("target", "web", "openmrs_config", "conceptsources", "conceptsources.csv");
212214
assertFilePresent("target", "web", "openmrs_config", "encountertypes", "encountertypes.csv");
213215
}
216+
217+
@Test
218+
public void testBuildDistroWithMissingContentDependencies() throws Exception {
219+
setupContentPackage("testpackage1");
220+
221+
includeDistroPropertiesFile("openmrs-distro-content-package-missing-dependencies.properties");
222+
addTaskParam("dir", "distro");
223+
addTaskParam("ignorePeerDependencies", "false");
224+
Exception exception = null;
225+
try {
226+
executeTask("build-distro");
227+
}
228+
catch (Exception e) {
229+
exception = e;
230+
}
231+
assertNotNull(exception);
232+
assertLogContains("requires module org.openmrs.module:fhir2-omod version >=1.8.0");
233+
assertLogContains("requires module org.openmrs.module:webservices.rest-omod version >=2.42.0");
234+
assertLogContains("requires module org.openmrs.module:openconceptlab-omod version >=2.3.0");
235+
assertLogContains("requires module org.openmrs.module:initializer-omod version >=2.5.2");
236+
assertLogContains("requires module org.openmrs.module:o3forms-omod version >=2.2.0");
237+
assertLogContains("requires esm @openmrs/esm-patient-chart-app version >=8.1.0");
238+
}
214239
}

‎integration-tests/src/test/java/org/openmrs/maven/plugins/DeployIT.java

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -348,4 +348,31 @@ public void deploy_shouldDeployOwas() throws Exception {
348348
server = Server.loadServer(testDirectoryPath.resolve(testServerId));
349349
assertThat(server.getOwaArtifacts().size(), equalTo(3));
350350
}
351+
352+
@Test
353+
public void testDeployWithMissingContentDependencies() throws Exception {
354+
setupContentPackage("testpackage1");
355+
356+
testServerId = setupTestServer("referenceapplication:2.2");
357+
358+
includeDistroPropertiesFile("openmrs-distro-content-package-missing-dependencies.properties");
359+
addAnswer(testServerId);
360+
addAnswer("y");
361+
addAnswer("y");
362+
363+
Exception exception = null;
364+
try {
365+
executeTask("deploy");
366+
}
367+
catch (Exception e) {
368+
exception = e;
369+
}
370+
assertNotNull(exception);
371+
assertLogContains("requires module org.openmrs.module:fhir2-omod version >=1.8.0");
372+
assertLogContains("requires module org.openmrs.module:webservices.rest-omod version >=2.42.0");
373+
assertLogContains("requires module org.openmrs.module:openconceptlab-omod version >=2.3.0");
374+
assertLogContains("requires module org.openmrs.module:initializer-omod version >=2.5.2");
375+
assertLogContains("requires module org.openmrs.module:o3forms-omod version >=2.2.0");
376+
assertLogContains("requires esm @openmrs/esm-patient-chart-app version >=8.1.0");
377+
}
351378
}

‎integration-tests/src/test/java/org/openmrs/maven/plugins/SetupIT.java

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
import static org.hamcrest.CoreMatchers.nullValue;
2222
import static org.hamcrest.MatcherAssert.assertThat;
2323
import static org.hamcrest.core.Is.is;
24+
import static org.junit.Assert.assertNotNull;
2425
import static org.openmrs.maven.plugins.SdkMatchers.hasModuleVersion;
2526
import static org.openmrs.maven.plugins.SdkMatchers.hasPropertyEqualTo;
2627
import static org.openmrs.maven.plugins.SdkMatchers.hasPropertyThatContains;
@@ -527,6 +528,35 @@ public void setup_shouldInstallWithContentPackageWithNoNamespace() throws Except
527528
assertFilePresent(serverId, "configuration", "encountertypes", "encountertypes.csv");
528529
}
529530

531+
@Test
532+
public void testSetupWithMissingContentDependencies() throws Exception {
533+
setupContentPackage("testpackage1");
534+
535+
includeDistroPropertiesFile("openmrs-distro-content-package-missing-dependencies.properties");
536+
addTaskParam("distro", testDirectory.toString() + File.separator + "openmrs-distro.properties");
537+
addMockDbSettings();
538+
539+
String serverId = UUID.randomUUID().toString();
540+
addAnswer(serverId);
541+
addAnswer("1044");
542+
addAnswer("8080");
543+
544+
Exception exception = null;
545+
try {
546+
executeTask("setup");
547+
}
548+
catch (Exception e) {
549+
exception = e;
550+
}
551+
assertNotNull(exception);
552+
assertLogContains("requires module org.openmrs.module:fhir2-omod version >=1.8.0");
553+
assertLogContains("requires module org.openmrs.module:webservices.rest-omod version >=2.42.0");
554+
assertLogContains("requires module org.openmrs.module:openconceptlab-omod version >=2.3.0");
555+
assertLogContains("requires module org.openmrs.module:initializer-omod version >=2.5.2");
556+
assertLogContains("requires module org.openmrs.module:o3forms-omod version >=2.2.0");
557+
assertLogContains("requires esm @openmrs/esm-patient-chart-app version >=8.1.0");
558+
}
559+
530560
private String readValueFromPropertyKey(File propertiesFile, String key) throws Exception {
531561

532562
InputStream in = new FileInputStream(propertiesFile);

‎integration-tests/src/test/java/org/openmrs/maven/plugins/utility/DistroHelperIT.java

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
package org.openmrs.maven.plugins.utility;
2+
3+
4+
import lombok.Getter;
5+
import lombok.Setter;
6+
import org.junit.Test;
7+
import org.openmrs.maven.plugins.AbstractMavenIT;
8+
import org.openmrs.maven.plugins.model.Artifact;
9+
10+
import java.util.Map;
11+
12+
import static org.hamcrest.Matchers.equalTo;
13+
import static org.junit.Assert.assertThat;
14+
15+
@Getter @Setter
16+
public class DistroHelperIT extends AbstractMavenIT {
17+
18+
@Test
19+
public void getFrontendModulesFromArtifact_shouldGetFrontendModulesAndVersions() throws Exception {
20+
executeTest(() -> {
21+
DistroHelper distroHelper = new DistroHelper(getMavenEnvironment());
22+
Artifact artifact = new Artifact("openmrs-frontend-pihemr", "1.8.0", "org.pih.openmrs", "zip");
23+
Map<String, String> m = distroHelper.getFrontendModulesFromArtifact(artifact, "openmrs-frontend-pihemr-1.8.0");
24+
assertThat(m.size(), equalTo(39));
25+
assertThat(m.get("@openmrs/esm-ward-app"), equalTo("8.0.3-pre.4390"));
26+
assertThat(m.get("@openmrs/esm-login-app"), equalTo("5.8.2-pre.2483"));
27+
});
28+
}
29+
}

‎integration-tests/src/test/resources/integration-test/contentpackages/testpackage1/assembly.xml

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
<assembly>
2+
<id>assemble-content</id>
3+
<formats>
4+
<format>zip</format>
5+
</formats>
6+
<includeBaseDirectory>false</includeBaseDirectory>
7+
<fileSets>
8+
<fileSet>
9+
<directory>${project.build.directory}</directory>
10+
<includes>
11+
<include>content.properties</include>
12+
</includes>
13+
<outputDirectory>/</outputDirectory>
14+
</fileSet>
15+
<fileSet>
16+
<directory>${project.basedir}/configuration</directory>
17+
<includes>
18+
<include>**/*</include>
19+
</includes>
20+
</fileSet>
21+
</fileSets>
22+
</assembly>

‎integration-tests/src/test/resources/integration-test/contentpackages/testpackage1/configuration/.gitkeep

Whitespace-only changes.

‎integration-tests/src/test/resources/integration-test/contentpackages/testpackage1/content.properties

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
# content.properties - Metadata for the content package and its dependencies
2+
3+
name=${project.artifactId}
4+
version=${project.version}
5+
6+
# Platform minimum version
7+
war.openmrs=>=2.4.0
8+
9+
# Backend modules
10+
omod.fhir2=>=1.8.0
11+
omod.webservices.rest=>=2.42.0
12+
omod.initializer=>=2.5.2
13+
omod.openconceptlab=>=2.3.0
14+
omod.o3forms=>=2.2.0
15+
16+
# Frontend modules
17+
spa.frontendModules.@openmrs/esm-patient-chart-app=>=8.1.0

‎integration-tests/src/test/resources/integration-test/contentpackages/testpackage1/pom.xml

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
1+
<project
2+
xmlns="http://maven.apache.org/POM/4.0.0"
3+
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
4+
xsi:schemaLocation="http://maven.apache.org/POM/4.0.0
5+
http://maven.apache.org/xsd/maven-4.0.0.xsd">
6+
<modelVersion>4.0.0</modelVersion>
7+
8+
<groupId>org.openmrs.maven.sdk.test</groupId>
9+
<artifactId>testpackage1</artifactId>
10+
<version>1.0.0</version>
11+
<packaging>pom</packaging>
12+
13+
<build>
14+
<plugins>
15+
<plugin>
16+
<artifactId>maven-resources-plugin</artifactId>
17+
<version>3.3.1</version>
18+
<executions>
19+
<execution>
20+
<id>copy-resources</id>
21+
<phase>process-resources</phase>
22+
<goals>
23+
<goal>resources</goal>
24+
</goals>
25+
<configuration>
26+
<outputDirectory>${project.build.directory}</outputDirectory>
27+
<resources>
28+
<resource>
29+
<directory>${project.basedir}</directory>
30+
<includes>
31+
<include>content.properties</include>
32+
</includes>
33+
<filtering>true</filtering>
34+
</resource>
35+
</resources>
36+
</configuration>
37+
</execution>
38+
</executions>
39+
</plugin>
40+
<plugin>
41+
<artifactId>maven-assembly-plugin</artifactId>
42+
<version>3.7.1</version>
43+
<configuration>
44+
<appendAssemblyId>false</appendAssemblyId>
45+
<descriptors>
46+
<descriptor>${project.basedir}/assembly.xml</descriptor>
47+
</descriptors>
48+
</configuration>
49+
<executions>
50+
<execution>
51+
<id>make-assembly</id>
52+
<phase>package</phase>
53+
<goals>
54+
<goal>single</goal>
55+
</goals>
56+
</execution>
57+
</executions>
58+
</plugin>
59+
</plugins>
60+
</build>
61+
62+
<repositories>
63+
<repository>
64+
<id>openmrs-repo</id>
65+
<name>OpenMRS repository</name>
66+
<url>https://mavenrepo.openmrs.org/nexus/content/repositories/public</url>
67+
</repository>
68+
</repositories>
69+
70+
</project>

‎integration-tests/src/test/resources/integration-test/openmrs-distro-content-package-missing-dependencies.properties

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
name=Content Package Example
2+
version=1.0.0
3+
war.openmrs=2.6.9
4+
content.testpackage1.groupId=org.openmrs.maven.sdk.test
5+
content.testpackage1.type=zip
6+
content.testpackage1=1.0.0
7+
db.h2.supported=true

‎maven-plugin/src/main/java/org/openmrs/maven/plugins/BuildDistro.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -246,7 +246,7 @@ private String buildDistro(File targetDirectory, Distribution distribution) thro
246246
DistroProperties distroProperties = distribution.getEffectiveProperties();
247247

248248
// First do content package validation
249-
distroHelper.parseContentProperties(distroProperties);
249+
distroHelper.validateDistribution(distroProperties);
250250

251251
InputStream dbDumpStream;
252252
wizard.showMessage("Downloading modules...\n");

‎maven-plugin/src/main/java/org/openmrs/maven/plugins/ServerUpgrader.java

Lines changed: 5 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,9 @@ public void upgradeToDistro(Server server, Distribution distribution, boolean ig
5454
return;
5555
}
5656
}
57+
58+
parentTask.distroHelper.validateDistribution(distroProperties);
59+
5760
server.saveBackupProperties();
5861
server.deleteServerTmpDirectory();
5962

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

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

131-
// TODO: This should happen in a higher-level validation stage and this method needs refactoring
132-
parentTask.distroHelper.parseContentProperties(distroProperties);
133-
134134
File configDir = new File(server.getServerDirectory(), SDKConstants.OPENMRS_SERVER_CONFIGURATION);
135135
if (configDir.exists()) {
136136
parentTask.wizard.showMessage("Removing existing configuration and content packages");
@@ -226,14 +226,8 @@ public UpgradeDifferential calculateUpdateDifferential(Server server, Distributi
226226
upgradeDifferential.setConfigChanges(new UpgradeDifferential.ArtifactChanges(oldConfig, newConfig));
227227

228228
// Content
229-
List<Artifact> oldContent = new ArrayList<>();
230-
for (ContentPackage contentPackage : server.getContentPackages()) {
231-
oldContent.add(contentPackage.getArtifact());
232-
}
233-
List<Artifact> newContent = new ArrayList<>();
234-
for (ContentPackage contentPackage : distroProperties.getContentPackages()) {
235-
newContent.add(contentPackage.getArtifact());
236-
}
229+
List<Artifact> oldContent = server.getContentPackageArtifacts();
230+
List<Artifact> newContent = distroProperties.getContentPackageArtifacts();
237231
upgradeDifferential.setContentChanges(new UpgradeDifferential.ArtifactChanges(oldContent, newContent));
238232

239233
return upgradeDifferential;

‎maven-plugin/src/main/java/org/openmrs/maven/plugins/Setup.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -281,7 +281,7 @@ public void setup(Server server, DistroProperties distroProperties) throws MojoE
281281
distroHelper.savePropertiesToServer(distroProperties, server);
282282

283283
setServerVersionsFromDistroProperties(server, distroProperties);
284-
distroHelper.parseContentProperties(distroProperties);
284+
distroHelper.validateDistribution(distroProperties);
285285
moduleInstaller.installModulesForDistro(server, distroProperties);
286286

287287
File configurationDir = new File(server.getServerDirectory(), SDKConstants.OPENMRS_SERVER_CONFIGURATION);

‎pom.xml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -407,7 +407,7 @@
407407
<dependency>
408408
<groupId>org.semver4j</groupId>
409409
<artifactId>semver4j</artifactId>
410-
<version>5.3.0</version>
410+
<version>5.4.0</version>
411411
</dependency>
412412

413413
<!-- console input and output -->

0 commit comments

Comments
 (0)