Skip to content

Commit

Permalink
fix: clean overrides for removed dependencies (#13170) (#13171)
Browse files Browse the repository at this point in the history 
Overrides should also automatically
be cleaned from the package.json
if there is no dependency that the
override would be targeting.

Co-authored-by: caalador <mikael.grankvist@vaadin.com>
  • Loading branch information
@vaadin-bot @caalador
vaadin-bot and caalador committed Feb 28, 2022
1 parent 3e3554c commit 035b1fc
Show file tree
Hide file tree
Showing 3 changed files with 91 additions and 46 deletions.
23 changes: 10 additions & 13 deletions flow-server/src/main/java/com/vaadin/flow/server/frontend/NodeUpdater.java
View file
Expand Up @@ -188,7 +188,8 @@ protected File getPackageLockFile() {
* Gets the platform pinned versions that are not overridden by the user in
* package.json.
*
* @return json object with the dependencies or {@code null}
* @return {@code JsonObject} with the dependencies or empty
* {@code JsonObject} if file doesn't exist
* @throws IOException
* when versions file could not be read
*/
Expand All @@ -198,20 +199,16 @@ JsonObject getPlatformPinnedDependencies() throws IOException {
log().info("Couldn't find {} file to pin dependency versions."
+ " Transitive dependencies won't be pinned for pnpm.",
Constants.VAADIN_VERSIONS_JSON);
return Json.createObject();
}

JsonObject versionsJson = null;
try (InputStream content = resource == null ? null
: resource.openStream()) {

if (content != null) {
VersionsJsonConverter convert = new VersionsJsonConverter(
Json.parse(IOUtils.toString(content,
StandardCharsets.UTF_8)));
versionsJson = convert.getConvertedJson();
versionsJson = new VersionsJsonFilter(getPackageJson(),
DEPENDENCIES).getFilteredVersions(versionsJson);
}
JsonObject versionsJson;
try (InputStream content = resource.openStream()) {
VersionsJsonConverter convert = new VersionsJsonConverter(Json
.parse(IOUtils.toString(content, StandardCharsets.UTF_8)));
versionsJson = convert.getConvertedJson();
versionsJson = new VersionsJsonFilter(getPackageJson(),
DEPENDENCIES).getFilteredVersions(versionsJson);
}
return versionsJson;
}
Expand Down
54 changes: 31 additions & 23 deletions flow-server/src/main/java/com/vaadin/flow/server/frontend/TaskUpdatePackages.java
View file
Expand Up @@ -137,19 +137,29 @@ boolean lockVersionForNpm(JsonObject packageJson, String versionsPath)
final JsonObject versionsJson = Json.parse(FileUtils.readFileToString(
generatedVersionsFile, StandardCharsets.UTF_8));

if (versionsJson != null) {
JsonObject overridesSection = getOverridesSection(packageJson);
final JsonObject dependencies = packageJson.getObject(DEPENDENCIES);
for (String dependency : versionsJson.keys()) {
if (!overridesSection.hasKey(dependency)
&& dependencies.hasKey(dependency)
&& !isInternalPseudoDependency(
versionsJson.getString(dependency))) {
overridesSection.put(dependency, "$" + dependency);
versionLockingUpdated = true;
}
JsonObject overridesSection = getOverridesSection(packageJson);
final JsonObject dependencies = packageJson.getObject(DEPENDENCIES);
final JsonObject devDependencies = packageJson
.getObject(DEV_DEPENDENCIES);
for (String dependency : versionsJson.keys()) {
if (!overridesSection.hasKey(dependency)
&& (dependencies.hasKey(dependency)
|| devDependencies.hasKey(dependency))
&& !isInternalPseudoDependency(
versionsJson.getString(dependency))) {
overridesSection.put(dependency, "$" + dependency);
versionLockingUpdated = true;
}
}
for (String dependency : overridesSection.keys()) {
if (!dependencies.hasKey(dependency)
&& !devDependencies.hasKey(dependency)
&& overridesSection.getString(dependency).startsWith("$")) {
overridesSection.remove(dependency);
versionLockingUpdated = true;
}
}

return versionLockingUpdated;
}

Expand Down Expand Up @@ -223,19 +233,17 @@ private boolean updatePackageJsonDependencies(JsonObject packageJson,
*/
List<String> pinnedPlatformDependencies = new ArrayList<>();
final JsonObject platformPinnedDependencies = getPlatformPinnedDependencies();
if (platformPinnedDependencies != null) {
for (String key : platformPinnedDependencies.keys()) {
// need to double check that not overriding a scanned
// dependency since add-ons should be able to downgrade
// version through exclusion
if (!applicationDependencies.containsKey(key)
&& pinPlatformDependency(packageJson,
platformPinnedDependencies, key)) {
added++;
}
// make sure platform pinned dependency is not cleared
pinnedPlatformDependencies.add(key);
for (String key : platformPinnedDependencies.keys()) {
// need to double check that not overriding a scanned
// dependency since add-ons should be able to downgrade
// version through exclusion
if (!applicationDependencies.containsKey(key)
&& pinPlatformDependency(packageJson,
platformPinnedDependencies, key)) {
added++;
}
// make sure platform pinned dependency is not cleared
pinnedPlatformDependencies.add(key);
}

if (added > 0) {
Expand Down
60 changes: 50 additions & 10 deletions flow-server/src/test/java/com/vaadin/flow/server/frontend/TaskUpdatePackagesNpmTest.java
View file
Expand Up @@ -18,7 +18,6 @@

import java.io.File;
import java.io.IOException;
import java.net.MalformedURLException;
import java.nio.charset.StandardCharsets;
import java.util.ArrayList;
import java.util.Arrays;
Expand Down Expand Up @@ -247,6 +246,40 @@ public void npmIsInUse_versionJsonHasBadVersion_noFailureNothingAdded()
verifyVersionLockingWithNpmOverrides(true, true, false);
}

@Test
public void npmIsInUse_executionAfterDependencyRemoved_overlayIsCleanedOfDependency()
throws IOException {
createVaadinVersionsJson(PLATFORM_DIALOG_VERSION,
PLATFORM_ELEMENT_MIXIN_VERSION, PLATFORM_OVERLAY_VERSION);

final Map<String, String> applicationDependencies = createApplicationDependencies();
applicationDependencies.put(VAADIN_ELEMENT_MIXIN,
PLATFORM_ELEMENT_MIXIN_VERSION);
applicationDependencies.put(VAADIN_OVERLAY, PLATFORM_OVERLAY_VERSION);
TaskUpdatePackages task = createTask(applicationDependencies);
task.execute();
Assert.assertTrue("Updates not picked", task.modified);

verifyVersionLockingWithNpmOverrides(true, true, true);

// Remove platform lock for vaadin-element-mixin
final JsonObject versions = Json.parse(FileUtils
.readFileToString(versionJsonFile, StandardCharsets.UTF_8));
versions.getObject("core").remove("vaadin-element-mixin");
FileUtils.writeStringToFile(versionJsonFile, versions.toJson(),
StandardCharsets.UTF_8);

// Remove VAADIN_ELEMENT_MIXIN from the application dependencies
applicationDependencies.remove(VAADIN_ELEMENT_MIXIN);
task = createTask(applicationDependencies);

task.execute();

Assert.assertTrue("Updates not picked", task.modified);

verifyVersionLockingWithNpmOverrides(true, false, true);
}

@Test
public void npmIsInUse_versionsJsonHasSnapshotVersions_notAddedToPackageJson()
throws IOException {
Expand Down Expand Up @@ -456,15 +489,18 @@ private void createVaadinVersionsJson(String dialogVersion,
String elementMixinVersion, String overlayVersion) {
// testing with exact versions json content instead of mocking parsing
String versionJsonString = //@formatter:off
"{ \"core\": {" + "\"vaadin-dialog\": {\n"
"{ \"core\": {"
+ "\"vaadin-dialog\": {\n"
+ " \"component\": true,\n"
+ " \"javaVersion\": \"{{version}}\",\n"
+ " \"jsVersion\": \"" + dialogVersion + "\",\n"
+ " \"npmName\": \"" + VAADIN_DIALOG + "\"\n"
+ "},\n" + "\"vaadin-element-mixin\": {\n"
+ "},\n"
+ "\"vaadin-element-mixin\": {\n"
+ " \"jsVersion\": \"" + elementMixinVersion
+ "\",\n" + " \"npmName\": \"" + VAADIN_ELEMENT_MIXIN
+ "\"\n" + "},\n" + "\"vaadin-overlay\": {\n"
+ "\"\n" + "},\n"
+ "\"vaadin-overlay\": {\n"
+ " \"jsVersion\": \"" + overlayVersion + "\",\n"
+ " \"npmName\": \"" + VAADIN_OVERLAY + "\",\n"
+ " \"releasenotes\": true\n"
Expand Down Expand Up @@ -503,10 +539,10 @@ private TaskUpdatePackages createTask(

private JsonObject getOrCreatePackageJson() throws IOException {
File packageJson = new File(npmFolder, PACKAGE_JSON);
if (packageJson.exists())
if (packageJson.exists()) {
return Json.parse(FileUtils.readFileToString(packageJson,
StandardCharsets.UTF_8));
else {
} else {
final JsonObject packageJsonJson = Json.createObject();
packageJsonJson.put(DEPENDENCIES, Json.createObject());
FileUtils.writeStringToFile(new File(npmFolder, PACKAGE_JSON),
Expand Down Expand Up @@ -552,24 +588,28 @@ private void verifyVersionLockingWithNpmOverrides(boolean hasDialogLocking,
Assert.assertEquals("$" + VAADIN_DIALOG,
overrides.getString(VAADIN_DIALOG));
} else {
Assert.assertNull(overrides.get(VAADIN_DIALOG));
Assert.assertNull("vaadin-dialog dependency should not be present",
overrides.get(VAADIN_DIALOG));
}
if (hasElementMixinLocking) {
Assert.assertEquals("$" + VAADIN_ELEMENT_MIXIN,
overrides.getString(VAADIN_ELEMENT_MIXIN));
} else {
Assert.assertNull(overrides.get(VAADIN_ELEMENT_MIXIN));
Assert.assertNull(
"vaadin-element-mixin dependency should not be present",
overrides.get(VAADIN_ELEMENT_MIXIN));
}
if (hasOverlayLocking) {
Assert.assertEquals("$" + VAADIN_OVERLAY,
overrides.getString(VAADIN_OVERLAY));
} else {
Assert.assertNull(overrides.get(VAADIN_OVERLAY));
Assert.assertNull("vaadin-overlay dependency should not be present",
overrides.get(VAADIN_OVERLAY));
}
}

private void verifyPlatformDependenciesAreAdded(boolean enablePnpm)
throws MalformedURLException, IOException {
throws IOException {
Mockito.when(finder.getResource(Constants.VAADIN_VERSIONS_JSON))
.thenReturn(versionJsonFile.toURI().toURL());
final String newVersion = "20.0.0";
Expand Down

0 comments on commit 035b1fc

Please sign in to comment.