Incorporated review comments

apache · Jan 18, 2017 · c1189be · c1189be
1 parent a70f5a9
commit c1189be
Show file tree

Hide file tree

Showing 3 changed files with 14 additions and 9 deletions.
diff --git a/extensions/src/main/java/org/apache/falcon/extensions/store/ExtensionStore.java b/extensions/src/main/java/org/apache/falcon/extensions/store/ExtensionStore.java
@@ -382,14 +382,14 @@ public boolean isExtensionStoreInitialized() {
 
     public List<String> getJobsForAnExtension(final String extensionName) throws FalconException {
         List<ExtensionJobsBean> extensionJobs = metaStore.getJobsForAnExtension(extensionName);
+        List<String> extensionJobNames = new ArrayList<>();
         if (null != extensionJobs && !extensionJobs.isEmpty()){
-            List<String> extensionJobNames = new ArrayList<>();
             for (ExtensionJobsBean extensionJobsBean: extensionJobs) {
                 extensionJobNames.add(extensionJobsBean.getJobName());
             }
             return extensionJobNames;
         } else {
-            return null;
+            return extensionJobNames;
         }
     }
 

diff --git a/prism/src/main/java/org/apache/falcon/resource/AbstractExtensionManager.java b/prism/src/main/java/org/apache/falcon/resource/AbstractExtensionManager.java
@@ -100,17 +100,22 @@ public APIResult getExtensions() {
         }
     }
 
-    public APIResult deleteExtensionMetadata(String extensionName) throws FalconException {
+    public APIResult deleteExtensionMetadata(String extensionName) {
         validateExtensionName(extensionName);
-        return new APIResult(APIResult.Status.SUCCEEDED, deleteExtension(extensionName));
+        ExtensionStore metaStore = ExtensionStore.get();
+        try {
+            canDeleteExtension(extensionName);
+            return new APIResult(APIResult.Status.SUCCEEDED,
+                    metaStore.deleteExtension(extensionName, CurrentUser.getUser()));
+        } catch (FalconException e) {
+            throw FalconWebException.newAPIException(e);
+        }
     }
 
-    private String deleteExtension(String extensionName) throws FalconException {
+    private void canDeleteExtension(String extensionName) throws FalconException {
         ExtensionStore metaStore = ExtensionStore.get();
         List<String> extensionJobs = metaStore.getJobsForAnExtension(extensionName);
-        if (null == extensionJobs || extensionJobs.isEmpty()) {
-            return metaStore.deleteExtension(extensionName, CurrentUser.getUser());
-        } else {
+        if (!extensionJobs.isEmpty()) {
             LOG.error("Extension:{} cannot be unregistered as {} are instances of the extension", extensionName,
                     ArrayUtils.toString(extensionJobs));
             throw new FalconException("Extension:" + extensionName + " cannot be unregistered as following instances"

diff --git a/unit/src/main/java/org/apache/falcon/unit/FalconUnitClient.java b/unit/src/main/java/org/apache/falcon/unit/FalconUnitClient.java
@@ -280,7 +280,7 @@ public APIResult unregisterExtension(String extensionName) {
         try {
             return localExtensionManager.unRegisterExtension(extensionName);
         } catch (FalconException e) {
-            throw new FalconCLIException("Failed in unRegistering the exnteison"+ e.getMessage());
+            throw new FalconCLIException("Failed in unRegistering the extension"+ e.getMessage());
         }
     }