refactor: Create API call for config

zeppelin-server/src/main/java/org/apache/zeppelin/rest/HeliumRestApi.java

@@ -18,9 +18,7 @@
 package org.apache.zeppelin.rest;
 
 import com.google.gson.Gson;
-import com.google.gson.JsonObject;
 import com.google.gson.JsonParseException;
-import com.google.gson.JsonSyntaxException;
 import com.google.gson.reflect.TypeToken;
 import org.apache.commons.io.FileUtils;
 import org.apache.commons.lang3.StringUtils;
@@ -191,6 +189,30 @@ public class HeliumRestApi {
     return new JsonResponse(Response.Status.OK, order).build();
   }
 
+  @GET
+  @Path("spell/config/{packageName}")
+  public Response getSpellConfigUsingMagic(@PathParam("packageName") String packageName) {
+    if (StringUtils.isEmpty(packageName)) {
+      return new JsonResponse(Response.Status.BAD_REQUEST,
+          "packageName is empty" ).build();
+    }
+
+    try {
+      Map<String, Map<String, Object>> config =
+          helium.getSpellConfig(packageName);
+
+      if (config == null) {
+        return new JsonResponse(Response.Status.BAD_REQUEST,
+            "Failed to find enabled package for " + packageName).build();
+      }
+
+      return new JsonResponse(Response.Status.OK, config).build();
+    } catch (RuntimeException e) {
+      logger.error(e.getMessage(), e);
+      return new JsonResponse(Response.Status.INTERNAL_SERVER_ERROR, e.getMessage()).build();
+    }
+  }
+
   @GET
   @Path("config")
   public Response getAllPackageConfigs() {
@@ -204,17 +226,24 @@ public class HeliumRestApi {
   }
 
   @GET
-  @Path("config/{artifact}")
-  public Response getPackageConfig(@PathParam("artifact") String artifact) {
-
-    if (StringUtils.isEmpty(artifact)) {
+  @Path("config/{packageName}/{artifact}")
+  public Response getPackageConfig(@PathParam("packageName") String packageName,
+                                   @PathParam("artifact") String artifact) {
+    if (StringUtils.isEmpty(packageName) || StringUtils.isEmpty(artifact)) {
       return new JsonResponse(Response.Status.BAD_REQUEST,
-          "package name or version is empty"
+          "package name or artifact is empty"
       ).build();
     }
 
     try {
-      Map<String, Object> config = helium.getPackageConfig(artifact);
+      Map<String, Map<String, Object>> config =
+          helium.getPackageConfig(packageName, artifact);
+
+      if (config == null) {
+        return new JsonResponse(Response.Status.BAD_REQUEST,
+            "Failed to find package for " + artifact).build();
+      }
+
       return new JsonResponse(Response.Status.OK, config).build();
     } catch (RuntimeException e) {
       logger.error(e.getMessage(), e);
@@ -223,10 +252,17 @@ public class HeliumRestApi {
   }
 
   @POST
-  @Path("config/{artifact}")
-  public Response updatePackageConfig(@PathParam("artifact") String artifact,
+  @Path("config/{packageName}/{artifact}")
+  public Response updatePackageConfig(@PathParam("packageName") String packageName,
+                                      @PathParam("artifact") String artifact,
                                       String rawConfig) {
 
+    if (StringUtils.isEmpty(packageName) || StringUtils.isEmpty(artifact)) {
+      return new JsonResponse(Response.Status.BAD_REQUEST,
+          "package name or artifact is empty"
+      ).build();
+    }
+
     Map<String, Object> packageConfig = null;
 
     try {

zeppelin-web/src/app/helium/helium.controller.js

@@ -223,7 +223,6 @@ export default function HeliumCtrl($scope, $rootScope, $sce,
       .then(confs => {
         $scope.defaultPackageConfigs[pkgName] = confs;
         pkgSearchResult.configFetching = false;
-        $scope.$digest(); // to trigger view update
       });
   };
 

zeppelin-web/src/components/helium/helium-conf.js

@@ -41,18 +41,6 @@ export function mergePersistedConfWithSpec(persisted, spec) {
   return confs;
 }
 
-export function createSinglePackageConfigs(pkg, persistedConf) {
-  const spec = pkg.config;
-  if (!spec) { return []; }
-
-  const version = pkg.version;
-  if (!version) { return []; }
-
-  if (!persistedConf) { persistedConf = {}; }
-
-  return mergePersistedConfWithSpec(persistedConf, spec);
-}
-
 export function createAllPackageConfigs(defaultPackages, persistedConfs) {
   let packageConfs = {};
 

zeppelin-web/src/components/helium/helium-package.js

@@ -12,8 +12,6 @@
  * limitations under the License.
  */
 
-import { HeliumType, } from './helium-type';
-
 export function createDefaultPackage(pkgSearchResult, sce) {
   for (let pkgIdx in pkgSearchResult) {
     const pkg = pkgSearchResult[pkgIdx];
@@ -47,32 +45,3 @@ export function createDefaultPackages(pkgSearchResults, sce) {
 
   return defaultPackages;
 }
-
-/**
- * @param defaultPackages {name, pkgSearchResult}
- * @param magic
- * @returns {pkgSearchResult}
- */
-export function findPackageByMagic(defaultPackages, magic) {
-  for (let name in defaultPackages) {
-    const pkgSearchResult = defaultPackages[name];
-    if (pkgSearchResult.enabled &&
-      pkgSearchResult.pkg.type === HeliumType.SPELL &&
-      pkgSearchResult.pkg.spell.magic === magic) {
-      return pkgSearchResult;
-    }
-  }
-
-  return undefined;
-}
-
-/**
- * @param singlePkgSearchResults list of PkgSearchResult for a single package
- * @param version
- * @returns {T} found PkgSearchResult otherwise returns `undefined`
- */
-export function findPackageByVersion(singlePkgSearchResults, version) {
-  return singlePkgSearchResults.find(psr => {
-    return psr.pkg.version === version;
-  });
-}

zeppelin-web/src/components/helium/helium.service.js

@@ -15,13 +15,11 @@
 import { HeliumType, } from './helium-type';
 import {
   createAllPackageConfigs,
-  createSinglePackageConfigs,
   createPersistableConfig,
+  mergePersistedConfWithSpec,
 } from './helium-conf';
 import {
   createDefaultPackages,
-  findPackageByVersion,
-  findPackageByMagic,
 } from './helium-package';
 
 angular.module('zeppelinWebApp').service('heliumService', heliumService);
@@ -38,6 +36,8 @@ export default function heliumService($http, $sce, baseUrlSrv) {
   var heliumBundles = [];
   // map for `{ magic: interpreter }`
   let spellPerMagic = {};
+  // map for `{ magic: package-name }`
+  let pkgNamePerMagic = {}
   let visualizationBundles = [];
 
   // load should be promise
@@ -50,7 +50,9 @@ export default function heliumService($http, $sce, baseUrlSrv) {
       heliumBundles.map(b => {
         if (b.type === HeliumType.SPELL) {
           const spell = new b.class(); // eslint-disable-line new-cap
+          const pkgName = b.id;
           spellPerMagic[spell.getMagic()] = spell;
+          pkgNamePerMagic[spell.getMagic()] = pkgName;
         } else if (b.type === HeliumType.VISUALIZATION) {
           visualizationBundles.push(b);
         }
@@ -125,19 +127,18 @@ export default function heliumService($http, $sce, baseUrlSrv) {
   };
 
   this.saveConfig = function(pkg , defaultPackageConfig) {
-    let pkgArtifact = pkg.artifact;
+    // in case of local package, it will include `/`
+    const pkgArtifact = encodeURIComponent(pkg.artifact);
+    const pkgName = pkg.name;
     const filtered = createPersistableConfig(defaultPackageConfig);
 
-    if (!pkgArtifact|| !filtered) {
+    if (!pkgName || !pkgArtifact|| !filtered) {
       console.error(
         `Can't save config for helium package '${pkgArtifact}'`, filtered);
       return;
     }
 
-    // in case of local package, it will include `/`
-    pkgArtifact = encodeURIComponent(pkgArtifact);
-
-    const url = `${baseUrlSrv.getRestApiBase()}/helium/config/${pkgArtifact}`;
+    const url = `${baseUrlSrv.getRestApiBase()}/helium/config/${pkgName}/${pkgArtifact}`;
     return $http.post(url, filtered);
   };
 
@@ -154,25 +155,6 @@ export default function heliumService($http, $sce, baseUrlSrv) {
       });
   };
 
-  /**
-   * @returns {Promise<Array>} which including package info list for all versions
-   */
-  this.getSinglePackageInfo = function(pkgName, pkgVersion) {
-    return $http.get(`${baseUrlSrv.getRestApiBase()}/helium/package/${pkgName}`)
-      .then(response => {
-        return response.data.body;
-      })
-      .then(singlePkgSearchResults => {
-        const found = findPackageByVersion(singlePkgSearchResults, pkgVersion);
-
-        if (!found) {
-          throw new Error(`Could not find package info for '${pkgName}@${pkgVersion}'`);
-        }
-
-        return found;
-      });
-  };
-
   this.getDefaultPackages = function() {
     return this.getAllPackageInfo()
       .then(pkgSearchResults => {
@@ -221,49 +203,38 @@ export default function heliumService($http, $sce, baseUrlSrv) {
   this.getSinglePackageConfigs = function(pkg) {
     const pkgName = pkg.name;
     const pkgVersion = pkg.version;
-    let pkgArtifact = pkg.artifact;
+    // in case of local package, it will include `/`
+    const pkgArtifact = encodeURIComponent(pkg.artifact);
 
     if (!pkgName || !pkgVersion || !pkgArtifact) {
       console.error('Failed to fetch config for\n', pkg);
       return Promise.resolve([]);
     }
 
-    const promisedPkgSearchResult = this.getSinglePackageInfo(pkgName, pkgVersion);
-
-    // in case of local package, it will include `/`
-    pkgArtifact = encodeURIComponent(pkgArtifact);
-
-    const confUrl = `${baseUrlSrv.getRestApiBase()}/helium/config/${pkgArtifact}`;
+    const confUrl = `${baseUrlSrv.getRestApiBase()}/helium/config/${pkgName}/${pkgArtifact}`;
     const promisedConf = $http.get(confUrl)
       .then(function(response, status) {
         return response.data.body;
       });
 
-    return Promise.all([promisedPkgSearchResult, promisedConf])
-      .then(values => {
-        const pkgSearchResult = values[0];
-        const persistedConf = values[1];
-
-        const merged = createSinglePackageConfigs(pkgSearchResult.pkg, persistedConf);
-        return merged;
-      })
-      .catch(function(error) {
-        console.error(`Failed to get package config for '${pkgName}@${pkgVersion}'`, error);
-      });
+    return promisedConf.then(({confSpec, confPersisted}) => {
+      const merged = mergePersistedConfWithSpec(confPersisted, confSpec)
+      return merged;
+    });
   };
 
   this.getSinglePackageConfigUsingMagic = function(magic) {
-    const promised = this.getDefaultPackages()
-      .then(defaultPackages => {
-        const pkgSearchResult = findPackageByMagic(defaultPackages, magic)
+    const pkgName = pkgNamePerMagic[magic];
 
-        // return empty confs if failed to find pkg
-        if (!pkgSearchResult) {
-          return [];
-        }
-        return this.getSinglePackageConfigs(pkgSearchResult.pkg);
+    const confUrl = `${baseUrlSrv.getRestApiBase()}/helium/spell/config/${pkgName}`;
+    const promisedConf = $http.get(confUrl)
+      .then(function(response, status) {
+        return response.data.body;
       });
 
-    return promised;
-  };
+    return promisedConf.then(({confSpec, confPersisted}) => {
+      const merged = mergePersistedConfWithSpec(confPersisted, confSpec)
+      return merged;
+    });
+  }
 }

zeppelin-zengine/src/main/java/org/apache/zeppelin/helium/Helium.java

@@ -248,11 +248,24 @@ public class Helium {
     return result.get(packageName);
   }
 
-  public HeliumPackageSearchResult getPackageInfo(String name, String artifact) {
-    Map<String, List<HeliumPackageSearchResult>> infos = getAllPackageInfo(false, name);
-    List<HeliumPackageSearchResult> packages = infos.get(name);
+  public HeliumPackageSearchResult getEnabledPackageInfo(String packageName) {
+    Map<String, List<HeliumPackageSearchResult>> infos = getAllPackageInfo();
+    List<HeliumPackageSearchResult> packages = infos.get(packageName);
+
+    for (HeliumPackageSearchResult pkgSearchResult : packages) {
+      if (pkgSearchResult.isEnabled()) {
+        return pkgSearchResult;
+      }
+    }
+
+    return null;
+  }
+
+  public HeliumPackageSearchResult getPackageInfo(String pkgName, String artifact) {
+    Map<String, List<HeliumPackageSearchResult>> infos = getAllPackageInfo(false, pkgName);
+    List<HeliumPackageSearchResult> packages = infos.get(pkgName);
     if (artifact == null) {
-      return packages.get(0);
+      return packages.get(0); /** return the FIRST package */
     } else {
       for (HeliumPackageSearchResult pkg : packages) {
         if (pkg.getPkg().getArtifact().equals(artifact)) {
@@ -315,8 +328,8 @@ public class Helium {
     return heliumConf.getAllPackageConfigs();
   }
 
-  public Map<String, Object> getPackageConfig(String artifact) {
-    return heliumConf.getPackageConfig(artifact);
+  public Map<String, Object> getPackagePersistedConfig(String artifact) {
+    return heliumConf.getPackagePersistedConfig(artifact);
   }
 
   public HeliumPackageSuggestion suggestApp(Paragraph paragraph) {
@@ -432,4 +445,51 @@ public class Helium {
 
     save();
   }
+
+  /**
+   * @param packageName
+   * @return { "confPersisted", "confSpec" } or return null if failed to found enabled package
+   */
+  public Map<String, Map<String, Object>> getSpellConfig(String packageName) {
+    HeliumPackageSearchResult result = getEnabledPackageInfo(packageName);
+
+    if (result == null) {
+      return null;
+    }
+
+    HeliumPackage enabledPackage = result.getPkg();
+
+    Map<String, Object> configSpec = enabledPackage.getConfig();
+    Map<String, Object> configPersisted =
+        getPackagePersistedConfig(enabledPackage.getArtifact());
+
+    return createMixedConfig(configPersisted, configSpec);
+  }
+
+  public Map<String, Map<String, Object>> getPackageConfig(String pkgName,
+                                                           String artifact) {
+
+    HeliumPackageSearchResult result = getPackageInfo(pkgName, artifact);
+
+    if (result == null) {
+      return null;
+    }
+
+    HeliumPackage requestedPackage = result.getPkg();
+
+    Map<String, Object> configSpec = requestedPackage.getConfig();
+    Map<String, Object> configPersisted =
+        getPackagePersistedConfig(artifact);
+
+    return createMixedConfig(configPersisted, configSpec);
+  }
+
+  public static Map<String, Map<String, Object>> createMixedConfig(Map<String, Object> persisted,
+                                                                   Map<String, Object> spec) {
+    Map<String, Map<String, Object>> mixed = new HashMap<>();
+    mixed.put("confPersisted", persisted);
+    mixed.put("confSpec", spec);
+
+    return mixed;
+  }
 }

zeppelin-zengine/src/main/java/org/apache/zeppelin/helium/HeliumConf.java

@@ -62,7 +62,7 @@ public class HeliumConf {
     return packageConfig;
   }
 
-  public Map<String, Object> getPackageConfig (String artifact) {
+  public Map<String, Object> getPackagePersistedConfig(String artifact) {
     if (!packageConfig.containsKey(artifact)) {
       packageConfig.put(artifact,
           Collections.synchronizedMap(new HashMap<String, Object>()));