You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ambari.apache.org by rn...@apache.org on 2015/09/28 21:01:36 UTC
ambari git commit: AMBARI-13202. Improve error checking for blueprint
resource creation. (Oliver Szabo via rnettleton)
Repository: ambari
Updated Branches:
refs/heads/branch-2.1 4aef14e5c -> ecae78d81
AMBARI-13202. Improve error checking for blueprint resource creation. (Oliver Szabo via rnettleton)
Project: http://git-wip-us.apache.org/repos/asf/ambari/repo
Commit: http://git-wip-us.apache.org/repos/asf/ambari/commit/ecae78d8
Tree: http://git-wip-us.apache.org/repos/asf/ambari/tree/ecae78d8
Diff: http://git-wip-us.apache.org/repos/asf/ambari/diff/ecae78d8
Branch: refs/heads/branch-2.1
Commit: ecae78d819c04a692b2b59b823a730b5f8a8ee01
Parents: 4aef14e
Author: Bob Nettleton <rn...@hortonworks.com>
Authored: Mon Sep 28 15:00:52 2015 -0400
Committer: Bob Nettleton <rn...@hortonworks.com>
Committed: Mon Sep 28 15:00:52 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/ecae78d8/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/ecae78d8/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);
}