From 1f7bc8418c339e8be811756dde166587b9c888c9 Mon Sep 17 00:00:00 2001 From: Denis Anisimov Date: Tue, 3 Sep 2019 14:36:32 +0300 Subject: [PATCH 1/2] Do not run npm install if generated package json content has not changed Stores hash of the generated package json file in the main package json (inside node_modules, outside target folder) and updates it every time when the generated content is changed. TaskUpdatePackages modified status is changed only if the hash value is changed. It allows do not run npm install even if the generated package.json file is regenerated. Fixes #6151 --- .../flow/server/frontend/NodeUpdater.java | 12 ++-- .../server/frontend/TaskUpdatePackages.java | 58 ++++++++++++++--- .../frontend/NodeUpdatePackagesTest.java | 64 +++++++++++++++++++ 3 files changed, 121 insertions(+), 13 deletions(-) diff --git a/flow-server/src/main/java/com/vaadin/flow/server/frontend/NodeUpdater.java b/flow-server/src/main/java/com/vaadin/flow/server/frontend/NodeUpdater.java index 7f86a70c313..2ccfc36454e 100644 --- a/flow-server/src/main/java/com/vaadin/flow/server/frontend/NodeUpdater.java +++ b/flow-server/src/main/java/com/vaadin/flow/server/frontend/NodeUpdater.java @@ -259,16 +259,18 @@ void writeMainPackageFile(JsonObject packageJson) throws IOException { writePackageFile(packageJson, new File(npmFolder, PACKAGE_JSON)); } - void writeAppPackageFile(JsonObject packageJson) throws IOException { - writePackageFile(packageJson, new File(generatedFolder, PACKAGE_JSON)); + String writeAppPackageFile(JsonObject packageJson) throws IOException { + return writePackageFile(packageJson, + new File(generatedFolder, PACKAGE_JSON)); } - void writePackageFile(JsonObject json, File packageFile) + String writePackageFile(JsonObject json, File packageFile) throws IOException { log().info("Updated npm {}.", packageFile.getAbsolutePath()); FileUtils.forceMkdirParent(packageFile); - FileUtils.writeStringToFile(packageFile, stringify(json, 2) + "\n", - UTF_8.name()); + String content = stringify(json, 2) + "\n"; + FileUtils.writeStringToFile(packageFile, content, UTF_8.name()); + return content; } Logger log() { diff --git a/flow-server/src/main/java/com/vaadin/flow/server/frontend/TaskUpdatePackages.java b/flow-server/src/main/java/com/vaadin/flow/server/frontend/TaskUpdatePackages.java index bbdf2b90b7f..c2b35976d86 100644 --- a/flow-server/src/main/java/com/vaadin/flow/server/frontend/TaskUpdatePackages.java +++ b/flow-server/src/main/java/com/vaadin/flow/server/frontend/TaskUpdatePackages.java @@ -19,11 +19,14 @@ import java.io.IOException; import java.io.Serializable; import java.io.UncheckedIOException; +import java.nio.charset.StandardCharsets; import java.nio.file.FileVisitResult; import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.SimpleFileVisitor; import java.nio.file.attribute.BasicFileAttributes; +import java.security.MessageDigest; +import java.security.NoSuchAlgorithmException; import java.util.Map; import java.util.Map.Entry; import java.util.Objects; @@ -44,6 +47,7 @@ */ public class TaskUpdatePackages extends NodeUpdater { + static final String APP_PACKAGE_HASH = "vaadinAppPackageHash"; private static final String VALUE = "value"; private static final String SHRINK_WRAP = "@vaadin/vaadin-shrinkwrap"; private boolean forceCleanUp; @@ -78,9 +82,8 @@ public FileVisitResult postVisitDirectory(Path dir, IOException exc) * @param generatedPath * folder where flow generated files will be placed. * @param forceCleanUp - * forces the clean up process to be run. If {@code false}, - * clean up will be performed when platform version update is - * detected. + * forces the clean up process to be run. If {@code false}, clean + * up will be performed when platform version update is detected. */ TaskUpdatePackages(ClassFinder finder, FrontendDependencies frontendDependencies, File npmFolder, @@ -97,9 +100,11 @@ public void execute() { if (packageJson == null) { packageJson = Json.createObject(); } - modified = updatePackageJsonDependencies(packageJson, deps); - if (modified) { - writeAppPackageFile(packageJson); + boolean isModified = updatePackageJsonDependencies(packageJson, + deps); + if (isModified) { + String content = writeAppPackageFile(packageJson); + modified = updateAppPackageHash(getHash(content)); } } catch (IOException e) { throw new UncheckedIOException(e); @@ -153,8 +158,7 @@ private boolean ensureReleaseVersion(JsonObject dependencies) shrinkWrapVersion = dependencies.getString(SHRINK_WRAP); } - return Objects.equals(shrinkWrapVersion, - getCurrentShrinkWrapVersion()); + return Objects.equals(shrinkWrapVersion, getCurrentShrinkWrapVersion()); } private void cleanUp() throws IOException { @@ -245,4 +249,42 @@ private String getShrinkWrapVersion(JsonObject packageJson) } return null; } + + private String getHash(String content) { + try { + MessageDigest digest = MessageDigest.getInstance("SHA-256"); + return bytesToHex( + digest.digest(content.getBytes(StandardCharsets.UTF_8))); + } catch (NoSuchAlgorithmException e) { + // Unrecoverable runime exception, it may not happen + throw new RuntimeException( + "Unable to find a provider for SHA-256 algorithm", e); + } + } + + private boolean updateAppPackageHash(String hash) throws IOException { + JsonObject mainContent = getMainPackageJson(); + if (mainContent == null) { + mainContent = Json.createObject(); + } + boolean modified = !mainContent.hasKey(APP_PACKAGE_HASH) + || !hash.equals(mainContent.getString(APP_PACKAGE_HASH)); + if (modified) { + mainContent.put(APP_PACKAGE_HASH, hash); + writeMainPackageFile(mainContent); + } + return modified; + } + + private String bytesToHex(byte[] hash) { + StringBuilder result = new StringBuilder(); + for (byte bit : hash) { + String hex = Integer.toHexString(0xff & bit); + if (hex.length() == 1) { + result.append('0'); + } + result.append(hex); + } + return result.toString(); + } } diff --git a/flow-server/src/test/java/com/vaadin/flow/server/frontend/NodeUpdatePackagesTest.java b/flow-server/src/test/java/com/vaadin/flow/server/frontend/NodeUpdatePackagesTest.java index 6e1cbf1089d..97eec220dde 100644 --- a/flow-server/src/test/java/com/vaadin/flow/server/frontend/NodeUpdatePackagesTest.java +++ b/flow-server/src/test/java/com/vaadin/flow/server/frontend/NodeUpdatePackagesTest.java @@ -22,6 +22,7 @@ import java.nio.file.Files; import java.util.Collections; +import org.apache.commons.io.FileUtils; import org.junit.Assert; import org.junit.Before; import org.junit.Rule; @@ -35,6 +36,7 @@ import static com.vaadin.flow.server.Constants.PACKAGE_JSON; import static com.vaadin.flow.server.frontend.FrontendUtils.DEFAULT_GENERATED_DIR; import static elemental.json.impl.JsonUtil.stringify; +import static java.nio.charset.StandardCharsets.UTF_8; public class NodeUpdatePackagesTest extends NodeUpdateTestUtil { @@ -242,6 +244,58 @@ public void versionsMatch_forceCleanUp_cleanUp() throws IOException { assertCleanUp(); } + @Test + public void generateAppPackageJsonFromScratch_updaterIsModified() + throws IOException { + packageCreator.execute(); + packageUpdater.execute(); + + JsonObject mainJson = getPackageJson(mainPackageJson); + Assert.assertTrue(mainJson.hasKey(TaskUpdatePackages.APP_PACKAGE_HASH)); + + Assert.assertTrue(packageUpdater.modified); + } + + @Test + public void regenerateAppPackageJson_sameContent_updaterIsNotModified() + throws IOException { + packageCreator.execute(); + packageUpdater.execute(); + + System.out.println("xx " + getPackageJson(appPackageJson)); + + // delete generated file + appPackageJson.delete(); + + // regenerate it (with the same content) + packageCreator.execute(); + packageUpdater.execute(); + + // the modified flag should be false (because the hash written in the + // main package json matches the content of the generated file) and "npm + // install" won't be executed + // as a result of this flag value + Assert.assertFalse(packageUpdater.modified); + } + + @Test + public void generateAppPackageJson_changedContent_updaterIsModified() + throws IOException { + packageCreator.execute(); + packageUpdater.execute(); + + System.out.println("xx " + getPackageJson(appPackageJson)); + + // delete generated file + appPackageJson.delete(); + + // generate it one more time, the content will be different since + // packageCreator has not added its content + packageUpdater.execute(); + + Assert.assertTrue(packageUpdater.modified); + } + private void makeNodeModulesAndPackageLock() throws IOException { // Make two node_modules folders and package lock mainNodeModules.mkdirs(); @@ -320,4 +374,14 @@ private JsonObject makePackageLock(String version) { return object; } + JsonObject getPackageJson(File packageFile) throws IOException { + JsonObject packageJson = null; + if (packageFile.exists()) { + String fileContent = FileUtils.readFileToString(packageFile, + UTF_8.name()); + packageJson = Json.parse(fileContent); + } + return packageJson; + } + } From d7df66f432e821cbab971d892a4df398ce1878ab Mon Sep 17 00:00:00 2001 From: Denis Anisimov Date: Fri, 6 Sep 2019 10:01:03 +0300 Subject: [PATCH 2/2] Remove sysouts. --- .../java/com/vaadin/flow/server/frontend/NodeUpdater.java | 4 ++-- .../vaadin/flow/server/frontend/NodeUpdatePackagesTest.java | 4 ---- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/flow-server/src/main/java/com/vaadin/flow/server/frontend/NodeUpdater.java b/flow-server/src/main/java/com/vaadin/flow/server/frontend/NodeUpdater.java index 2ccfc36454e..b4c61093a30 100644 --- a/flow-server/src/main/java/com/vaadin/flow/server/frontend/NodeUpdater.java +++ b/flow-server/src/main/java/com/vaadin/flow/server/frontend/NodeUpdater.java @@ -255,8 +255,8 @@ boolean addDependency(JsonObject json, String key, String pkg, return false; } - void writeMainPackageFile(JsonObject packageJson) throws IOException { - writePackageFile(packageJson, new File(npmFolder, PACKAGE_JSON)); + String writeMainPackageFile(JsonObject packageJson) throws IOException { + return writePackageFile(packageJson, new File(npmFolder, PACKAGE_JSON)); } String writeAppPackageFile(JsonObject packageJson) throws IOException { diff --git a/flow-server/src/test/java/com/vaadin/flow/server/frontend/NodeUpdatePackagesTest.java b/flow-server/src/test/java/com/vaadin/flow/server/frontend/NodeUpdatePackagesTest.java index 97eec220dde..dadb0ad25a1 100644 --- a/flow-server/src/test/java/com/vaadin/flow/server/frontend/NodeUpdatePackagesTest.java +++ b/flow-server/src/test/java/com/vaadin/flow/server/frontend/NodeUpdatePackagesTest.java @@ -262,8 +262,6 @@ public void regenerateAppPackageJson_sameContent_updaterIsNotModified() packageCreator.execute(); packageUpdater.execute(); - System.out.println("xx " + getPackageJson(appPackageJson)); - // delete generated file appPackageJson.delete(); @@ -284,8 +282,6 @@ public void generateAppPackageJson_changedContent_updaterIsModified() packageCreator.execute(); packageUpdater.execute(); - System.out.println("xx " + getPackageJson(appPackageJson)); - // delete generated file appPackageJson.delete();