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.

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

Do not run npm install if generated package json content has not changed #6385

Merged
merged 2 commits into from
Sep 6, 2019
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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,
Expand All @@ -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);
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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(
Copy link
Collaborator

Choose a reason for hiding this comment

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

MAJOR Define and throw a dedicated exception instead of using a generic one. rule

"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();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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 {

Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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;
}

}