Repository: zeppelin Updated Branches: refs/heads/master 9bc4dce98 -> d61cd99db
[ZEPPELIN-1969] Can not change visualization package version. ### What is this PR for? Changing visualization package version from helium menu, sometimes fail. This PR fixes the problem and providing a unittest. ### What type of PR is it? Bug Fix ### Todos * [x] - remove package from node_module and let npm download again before bundle the package. * [x] - add unittest. ### What is the Jira issue? https://issues.apache.org/jira/browse/ZEPPELIN-1969 ### How should this be tested? Unittest HeliumVisualizationFactoryTest.switchVersion() ensure the fix. ### Questions: * Does the licenses files need update? no * Is there breaking changes for older versions? no * Does this needs documentation? no Author: Lee moon soo <m...@apache.org> Closes #1900 from Leemoonsoo/fix_helium_version_switch and squashes the following commits: 540497f [Lee moon soo] fix style e9f2811 [Lee moon soo] Make download package everytime bundle to workaround inconsistent behavior of npm install Project: http://git-wip-us.apache.org/repos/asf/zeppelin/repo Commit: http://git-wip-us.apache.org/repos/asf/zeppelin/commit/d61cd99d Tree: http://git-wip-us.apache.org/repos/asf/zeppelin/tree/d61cd99d Diff: http://git-wip-us.apache.org/repos/asf/zeppelin/diff/d61cd99d Branch: refs/heads/master Commit: d61cd99dbb5c63078b3e20e94183355e2c401d5f Parents: 9bc4dce Author: Lee moon soo <m...@apache.org> Authored: Mon Jan 16 07:40:44 2017 -0800 Committer: Lee moon soo <m...@apache.org> Committed: Mon Jan 16 09:44:06 2017 -0800 ---------------------------------------------------------------------- .../helium/HeliumVisualizationFactory.java | 31 ++++++++++++--- .../helium/HeliumVisualizationFactoryTest.java | 42 ++++++++++++++++++-- 2 files changed, 65 insertions(+), 8 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/zeppelin/blob/d61cd99d/zeppelin-zengine/src/main/java/org/apache/zeppelin/helium/HeliumVisualizationFactory.java ---------------------------------------------------------------------- diff --git a/zeppelin-zengine/src/main/java/org/apache/zeppelin/helium/HeliumVisualizationFactory.java b/zeppelin-zengine/src/main/java/org/apache/zeppelin/helium/HeliumVisualizationFactory.java index a06c18b..1c1d25a 100644 --- a/zeppelin-zengine/src/main/java/org/apache/zeppelin/helium/HeliumVisualizationFactory.java +++ b/zeppelin-zengine/src/main/java/org/apache/zeppelin/helium/HeliumVisualizationFactory.java @@ -104,6 +104,7 @@ public class HeliumVisualizationFactory { URL pkgUrl = Resources.getResource("helium/package.json"); String pkgJson = Resources.toString(pkgUrl, Charsets.UTF_8); StringBuilder dependencies = new StringBuilder(); + StringBuilder cacheKeyBuilder = new StringBuilder(); FileFilter npmPackageCopyFilter = new FileFilter() { @Override @@ -127,18 +128,25 @@ public class HeliumVisualizationFactory { dependencies.append(",\n"); } dependencies.append("\"" + moduleNameVersion[0] + "\": \"" + moduleNameVersion[1] + "\""); + cacheKeyBuilder.append(pkg.getName() + pkg.getArtifact()); + + File pkgInstallDir = new File(workingDirectory, "node_modules/" + pkg.getName()); + if (pkgInstallDir.exists()) { + FileUtils.deleteDirectory(pkgInstallDir); + } if (isLocalPackage(pkg)) { FileUtils.copyDirectory( new File(pkg.getArtifact()), - new File(workingDirectory, "node_modules/" + pkg.getName()), + pkgInstallDir, npmPackageCopyFilter); } } pkgJson = pkgJson.replaceFirst("DEPENDENCIES", dependencies.toString()); // check if we can use previous bundle or not - if (dependencies.toString().equals(bundleCacheKey) && currentBundle.isFile() && !forceRefresh) { + if (cacheKeyBuilder.toString().equals(bundleCacheKey) + && currentBundle.isFile() && !forceRefresh) { return currentBundle; } @@ -177,7 +185,10 @@ public class HeliumVisualizationFactory { // install tabledata module File tabledataModuleInstallPath = new File(workingDirectory, "node_modules/zeppelin-tabledata"); - if (tabledataModulePath != null && !tabledataModuleInstallPath.exists()) { + if (tabledataModulePath != null) { + if (tabledataModuleInstallPath.exists()) { + FileUtils.deleteDirectory(tabledataModuleInstallPath); + } FileUtils.copyDirectory( tabledataModulePath, tabledataModuleInstallPath, @@ -187,7 +198,17 @@ public class HeliumVisualizationFactory { // install visualization module File visModuleInstallPath = new File(workingDirectory, "node_modules/zeppelin-vis"); - if (visualizationModulePath != null && !visModuleInstallPath.exists()) { + if (visualizationModulePath != null) { + if (visModuleInstallPath.exists()) { + // when zeppelin-vis and zeppelin-table package is published to npm repository + // we don't need to remove module because npm install command will take care + // dependency version change. However, when two dependencies are copied manually + // into node_modules directory, changing vis package version results inconsistent npm + // install behavior. + // + // Remote vis package everytime and let npm download every time bundle as a workaround + FileUtils.deleteDirectory(visModuleInstallPath); + } FileUtils.copyDirectory(visualizationModulePath, visModuleInstallPath, npmPackageCopyFilter); } @@ -210,7 +231,7 @@ public class HeliumVisualizationFactory { synchronized (this) { currentBundle.delete(); FileUtils.moveFile(visBundleJs, currentBundle); - bundleCacheKey = dependencies.toString(); + bundleCacheKey = cacheKeyBuilder.toString(); } return currentBundle; } http://git-wip-us.apache.org/repos/asf/zeppelin/blob/d61cd99d/zeppelin-zengine/src/test/java/org/apache/zeppelin/helium/HeliumVisualizationFactoryTest.java ---------------------------------------------------------------------- diff --git a/zeppelin-zengine/src/test/java/org/apache/zeppelin/helium/HeliumVisualizationFactoryTest.java b/zeppelin-zengine/src/test/java/org/apache/zeppelin/helium/HeliumVisualizationFactoryTest.java index 47af409..e5a61ed 100644 --- a/zeppelin-zengine/src/test/java/org/apache/zeppelin/helium/HeliumVisualizationFactoryTest.java +++ b/zeppelin-zengine/src/test/java/org/apache/zeppelin/helium/HeliumVisualizationFactoryTest.java @@ -30,9 +30,7 @@ import java.net.URL; import java.util.LinkedList; import java.util.List; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertNull; -import static org.junit.Assert.assertTrue; +import static org.junit.Assert.*; public class HeliumVisualizationFactoryTest { private File tmpDir; @@ -154,4 +152,42 @@ public class HeliumVisualizationFactoryTest { } assertNull(bundle); } + + @Test + public void switchVersion() throws IOException, TaskRunnerException { + URL res = Resources.getResource("helium/webpack.config.js"); + String resDir = new File(res.getFile()).getParent(); + + HeliumPackage pkgV1 = new HeliumPackage( + HeliumPackage.Type.VISUALIZATION, + "zeppelin-bubblechart", + "zeppelin-bubblechart", + "zeppelin-bubblechart@0.0.3", + "", + null, + "license", + "icon" + ); + + HeliumPackage pkgV2 = new HeliumPackage( + HeliumPackage.Type.VISUALIZATION, + "zeppelin-bubblechart", + "zeppelin-bubblechart", + "zeppelin-bubblechart@0.0.1", + "", + null, + "license", + "icon" + ); + List<HeliumPackage> pkgsV1 = new LinkedList<>(); + pkgsV1.add(pkgV1); + + List<HeliumPackage> pkgsV2 = new LinkedList<>(); + pkgsV2.add(pkgV2); + + File bundle1 = hvf.bundle(pkgsV1); + File bundle2 = hvf.bundle(pkgsV2); + + assertNotSame(bundle1.lastModified(), bundle2.lastModified()); + } }
