ambari-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From rnettle...@apache.org
Subject ambari git commit: AMBARI-13202. Improve error checking for blueprint resource creation. (Olivér Szabó via rnettleton)
Date Mon, 28 Sep 2015 18:45:36 GMT
Repository: ambari
Updated Branches:
  refs/heads/trunk 4119f0e55 -> 460d191a1


AMBARI-13202. Improve error checking for blueprint resource creation. (Olivér Szabó via
rnettleton)


Project: http://git-wip-us.apache.org/repos/asf/ambari/repo
Commit: http://git-wip-us.apache.org/repos/asf/ambari/commit/460d191a
Tree: http://git-wip-us.apache.org/repos/asf/ambari/tree/460d191a
Diff: http://git-wip-us.apache.org/repos/asf/ambari/diff/460d191a

Branch: refs/heads/trunk
Commit: 460d191a1abdf7c59344149b3e5fb8a24050afb1
Parents: 4119f0e
Author: Bob Nettleton <rnettleton@hortonworks.com>
Authored: Mon Sep 28 14:06:46 2015 -0400
Committer: Bob Nettleton <rnettleton@hortonworks.com>
Committed: Mon Sep 28 14:45:23 2015 -0400

----------------------------------------------------------------------
 .../internal/BlueprintResourceProvider.java     | 31 ++++---
 .../internal/BlueprintResourceProviderTest.java | 93 +++++++++++++++++++-
 2 files changed, 109 insertions(+), 15 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/ambari/blob/460d191a/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BlueprintResourceProvider.java
----------------------------------------------------------------------
diff --git a/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BlueprintResourceProvider.java
b/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BlueprintResourceProvider.java
index 5fa6655..6cb6a74 100644
--- a/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BlueprintResourceProvider.java
+++ b/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BlueprintResourceProvider.java
@@ -18,6 +18,8 @@
 
 package org.apache.ambari.server.controller.internal;
 
+import com.google.common.base.Preconditions;
+import com.google.common.base.Strings;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
@@ -85,8 +87,15 @@ public class BlueprintResourceProvider extends AbstractControllerResourceProvide
   public static final String PROPERTIES_PROPERTY_ID = "properties";
   public static final String PROPERTIES_ATTRIBUTES_PROPERTY_ID = "properties_attributes";
   public static final String SCHEMA_IS_NOT_SUPPORTED_MESSAGE =
-      "Configuration format provided in Blueprint is not supported";
-
+    "Configuration format provided in Blueprint is not supported";
+  public static final String REQUEST_BODY_EMPTY_ERROR_MESSAGE =
+    "Request body for Blueprint create request is empty";
+  public static final String CONFIGURATION_LIST_CHECK_ERROR_MESSAGE =
+    "Configurations property must be a List of Maps";
+  public static final String CONFIGURATION_MAP_CHECK_ERROR_MESSAGE =
+    "Configuration elements must be Maps";
+  public static final String CONFIGURATION_MAP_SIZE_CHECK_ERROR_MESSAGE =
+    "Configuration Maps must hold a single configuration type each";
   // Primary Key Fields
   private static Set<String> pkPropertyIds =
       new HashSet<String>(Arrays.asList(new String[]{
@@ -384,22 +393,16 @@ public class BlueprintResourceProvider extends AbstractControllerResourceProvide
       @Override
       public Void invoke() throws AmbariException {
         String rawRequestBody = requestInfoProps.get(Request.REQUEST_INFO_BODY_PROPERTY);
+        Preconditions.checkArgument(!Strings.isNullOrEmpty(rawRequestBody), REQUEST_BODY_EMPTY_ERROR_MESSAGE);
+
         Map<String, Object> rawBodyMap = jsonSerializer.<Map<String, Object>>fromJson(rawRequestBody,
Map.class);
         Object configurationData = rawBodyMap.get(CONFIGURATION_PROPERTY_ID);
 
         if (configurationData != null) {
-          if (configurationData instanceof List) {
-            for (Object map : (List) configurationData) {
-              if (map instanceof Map) {
-                if (((Map) map).size() > 1) {
-                  throw new IllegalArgumentException("Configuration Maps must hold a single
configuration type each");
-                }
-              } else {
-                throw new IllegalArgumentException("Configuration elements must be Maps");
-              }
-            }
-          } else {
-            throw new IllegalArgumentException("Configurations property must be a List of
Maps");
+          Preconditions.checkArgument(configurationData instanceof List, CONFIGURATION_LIST_CHECK_ERROR_MESSAGE);
+          for (Object map : (List) configurationData) {
+            Preconditions.checkArgument(map instanceof Map, CONFIGURATION_MAP_CHECK_ERROR_MESSAGE);
+            Preconditions.checkArgument(((Map) map).size() <= 1, CONFIGURATION_MAP_SIZE_CHECK_ERROR_MESSAGE);
           }
         }
         Blueprint blueprint;

http://git-wip-us.apache.org/repos/asf/ambari/blob/460d191a/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/BlueprintResourceProviderTest.java
----------------------------------------------------------------------
diff --git a/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/BlueprintResourceProviderTest.java
b/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/BlueprintResourceProviderTest.java
index 4a5ff46..5bfdebb 100644
--- a/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/BlueprintResourceProviderTest.java
+++ b/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/BlueprintResourceProviderTest.java
@@ -167,6 +167,38 @@ public class BlueprintResourceProviderTest {
     verify(dao, entity, blueprintFactory, metaInfo, request, managementController);
   }
 
+  @Test()
+  public void testCreateResources_ReqestBodyIsEmpty() throws Exception {
+    AmbariManagementController managementController = createMock(AmbariManagementController.class);
+    Request request = createMock(Request.class);
+
+    Set<Map<String, Object>> setProperties = getBlueprintTestProperties();
+    Map<String, String> requestInfoProperties = new HashMap<String, String>();
+    requestInfoProperties.put(Request.REQUEST_INFO_BODY_PROPERTY, null);
+
+    // set expectations
+    expect(request.getProperties()).andReturn(setProperties);
+    expect(request.getRequestInfoProperties()).andReturn(requestInfoProperties);
+
+    replay(request, managementController);
+    // end expectations
+
+    ResourceProvider provider = AbstractControllerResourceProvider.getResourceProvider(
+      Resource.Type.Blueprint,
+      PropertyHelper.getPropertyIds(Resource.Type.Blueprint),
+      PropertyHelper.getKeyPropertyIds(Resource.Type.Blueprint),
+      managementController);
+
+    try {
+      provider.createResources(request);
+      fail("Exception expected");
+    } catch (IllegalArgumentException e) {
+      //expected exception
+      assertEquals(BlueprintResourceProvider.REQUEST_BODY_EMPTY_ERROR_MESSAGE, e.getMessage());
+    }
+    verify(request, managementController);
+  }
+
   @Test
   public void testCreateResources_NoValidation() throws Exception {
 
@@ -491,7 +523,7 @@ public class BlueprintResourceProviderTest {
   }
 
   @Test
-  public void testCreateResources_withWrongConfigurationsStructure() throws ResourceAlreadyExistsException,
SystemException,
+  public void testCreateResources_wrongConfigurationsStructure_withWrongConfigMapSize() throws
ResourceAlreadyExistsException, SystemException,
       UnsupportedPropertyException, NoSuchParentResourceException
   {
     Request request = createMock(Request.class);
@@ -516,6 +548,65 @@ public class BlueprintResourceProviderTest {
       fail("Exception expected");
     } catch (IllegalArgumentException e) {
       //expected exception
+      assertEquals(BlueprintResourceProvider.CONFIGURATION_MAP_SIZE_CHECK_ERROR_MESSAGE,
e.getMessage());
+    }
+    verify(dao, metaInfo, request);
+  }
+
+  @Test
+  public void testCreateResources_wrongConfigurationStructure_withoutConfigMaps() throws
ResourceAlreadyExistsException, SystemException,
+    UnsupportedPropertyException, NoSuchParentResourceException {
+
+    Request request = createMock(Request.class);
+
+    Set<Map<String, Object>> setProperties = getBlueprintTestProperties();
+
+    Map<String, String> requestInfoProperties = new HashMap<String, String>();
+    String configurationData = "{\"configurations\":[\"config-type1\", \"config-type2\"]}";
+    requestInfoProperties.put(Request.REQUEST_INFO_BODY_PROPERTY, configurationData);
+
+    // set expectations
+    expect(request.getProperties()).andReturn(setProperties);
+    expect(request.getRequestInfoProperties()).andReturn(requestInfoProperties);
+
+    replay(dao, metaInfo, request);
+    // end expectations
+
+    try {
+      provider.createResources(request);
+      fail("Exception expected");
+    } catch (IllegalArgumentException e) {
+      //expected exception
+      assertEquals(BlueprintResourceProvider.CONFIGURATION_MAP_CHECK_ERROR_MESSAGE, e.getMessage());
+    }
+    verify(dao, metaInfo, request);
+  }
+
+  @Test
+  public void testCreateResources_wrongConfigurationStructure_withoutConfigsList() throws
ResourceAlreadyExistsException, SystemException,
+    UnsupportedPropertyException, NoSuchParentResourceException {
+
+    Request request = createMock(Request.class);
+
+    Set<Map<String, Object>> setProperties = getBlueprintTestProperties();
+
+    Map<String, String> requestInfoProperties = new HashMap<String, String>();
+    String configurationData = "{\"configurations\":{\"config-type1\": \"properties\", \"config-type2\":
\"properties\"}}";
+    requestInfoProperties.put(Request.REQUEST_INFO_BODY_PROPERTY, configurationData);
+
+    // set expectations
+    expect(request.getProperties()).andReturn(setProperties);
+    expect(request.getRequestInfoProperties()).andReturn(requestInfoProperties);
+
+    replay(dao, metaInfo, request);
+    // end expectations
+
+    try {
+      provider.createResources(request);
+      fail("Exception expected");
+    } catch (IllegalArgumentException e) {
+      //expected exception
+      assertEquals(BlueprintResourceProvider.CONFIGURATION_LIST_CHECK_ERROR_MESSAGE, e.getMessage());
     }
     verify(dao, metaInfo, request);
   }


Mime
View raw message