Return-Path: X-Original-To: apmail-ambari-commits-archive@www.apache.org Delivered-To: apmail-ambari-commits-archive@www.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id 9D89218418 for ; Wed, 30 Sep 2015 14:45:30 +0000 (UTC) Received: (qmail 51116 invoked by uid 500); 30 Sep 2015 14:45:23 -0000 Delivered-To: apmail-ambari-commits-archive@ambari.apache.org Received: (qmail 51051 invoked by uid 500); 30 Sep 2015 14:45:23 -0000 Mailing-List: contact commits-help@ambari.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: ambari-dev@ambari.apache.org Delivered-To: mailing list commits@ambari.apache.org Received: (qmail 49596 invoked by uid 99); 30 Sep 2015 14:45:22 -0000 Received: from git1-us-west.apache.org (HELO git1-us-west.apache.org) (140.211.11.23) by apache.org (qpsmtpd/0.29) with ESMTP; Wed, 30 Sep 2015 14:45:22 +0000 Received: by git1-us-west.apache.org (ASF Mail Server at git1-us-west.apache.org, from userid 33) id DEC3DE0AFA; Wed, 30 Sep 2015 14:45:21 +0000 (UTC) Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit From: ncole@apache.org To: commits@ambari.apache.org Date: Wed, 30 Sep 2015 14:45:58 -0000 Message-Id: In-Reply-To: References: X-Mailer: ASF-Git Admin Mailer Subject: =?utf-8?q?=5B38/50=5D_=5Babbrv=5D_ambari_git_commit=3A_AMBARI-1320?= =?utf-8?q?2=2E_Improve_error_checking_for_blueprint_resource_creation=2E_?= =?utf-8?q?=28Oliv=C3=A9r_Szab=C3=B3_via_rnettleton=29?= 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/branch-dev-patch-upgrade Commit: 460d191a1abdf7c59344149b3e5fb8a24050afb1 Parents: 4119f0e Author: Bob Nettleton Authored: Mon Sep 28 14:06:46 2015 -0400 Committer: Bob Nettleton 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 pkPropertyIds = new HashSet(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 rawBodyMap = jsonSerializer.>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> setProperties = getBlueprintTestProperties(); + Map requestInfoProperties = new HashMap(); + 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> setProperties = getBlueprintTestProperties(); + + Map requestInfoProperties = new HashMap(); + 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> setProperties = getBlueprintTestProperties(); + + Map requestInfoProperties = new HashMap(); + 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); }