SDK-365 and SDK-369 - Ensure content package validation occurs prior … #318
Conversation
…to destructive changes, and do not automatically change distribution/server based on content package requirements.
| verifier.resetStreams(); | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
This allows us to create any content packages we wish for testing, without relying on published versions in the cloud
| assertLogContains("requires module org.openmrs.module:o3forms-omod version >=2.2.0"); | ||
| assertLogContains("requires esm @openmrs/esm-patient-chart-app version >=8.1.0"); | ||
| } | ||
|
|
There was a problem hiding this comment.
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.
| @@ -54,6 +54,9 @@ public void upgradeToDistro(Server server, Distribution distribution, boolean ig | |||
| return; | |||
| } | |||
| } | |||
|
|
|||
| parentTask.distroHelper.validateDistribution(distroProperties); | |||
There was a problem hiding this comment.
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.
| try { | ||
| FileUtils.deleteDirectory(tempDirectory); | ||
| } catch (IOException e) { | ||
| log.warn("Failed to delete temporary directory: {}", tempDirectory.getAbsolutePath(), e); |
There was a problem hiding this comment.
This is a convenience method that gets the missing dependencies, iterates over them, formats these into friendly error messages for display, and throws an exception with this.
| throws MojoExecutionException { | ||
|
|
||
| Properties contentProperties = new Properties(); | ||
|
|
There was a problem hiding this comment.
This is what actually retrieves and calculates missing dependencies. If we wanted to add back in some interactivity with the wizard and allow the user to choose whether to upgrade their server/distribution to the necessary artifacts and versions, we would change the goal from calling the validateDistribution method to calling this method and then using the list of missing dependencies to drive the wizard and support the user upgrading.
| String latestVersion = findLatestMatchingVersion(dependency, versionRange); | ||
| if (latestVersion == null) { | ||
| throw new MojoExecutionException( | ||
| "No matching version found for dependency " + dependency + " in " + zipFileName); |
There was a problem hiding this comment.
This method is a bit of a hack to support what PIH is currently doing, which is building our frontend in a separate project and publishing it to Maven, and then including that Maven zip in our distribution. In this case, we don't have the spa-assemble or spa-build config files in that artifact (though as an alternative to this below, we could potentially add it in). This downloads the artifact, unpacks it, and inspects the file/directory contents, translating the directory names back into ESM names and versions. Interested in review/thoughts on this in particular @ibacher . There is an integration test in DistroHelperIT that demonstrates this.
| } | ||
| } catch (SemverException e) { | ||
| throw new MojoExecutionException("Invalid version range format for " + contentDependencyKey | ||
| + " in content package " + contentPackageName + ": " + contentDependencyVersionRange, e); |
There was a problem hiding this comment.
Per slack discussion this new method is what checks for version incompatibility, accounting for things like "next", and also allowing snapshot and pre-releases that are otherwise higher than a release version to satisfy that release version. See unit test in DistroHelperTest for examples.
…to destructive changes, and do not automatically change distribution/server based on content package requirements.
see https://openmrs.atlassian.net/browse/SDK-365
see https://openmrs.atlassian.net/browse/SDK-369