AMBARI-21706 : Fix exception messages whenever empty host list is passed in predicate. (avijayan)
Project: http://git-wip-us.apache.org/repos/asf/ambari/repo Commit: http://git-wip-us.apache.org/repos/asf/ambari/commit/0630899c Tree: http://git-wip-us.apache.org/repos/asf/ambari/tree/0630899c Diff: http://git-wip-us.apache.org/repos/asf/ambari/diff/0630899c Branch: refs/heads/branch-feature-AMBARI-21450 Commit: 0630899ce80dcc2cbac76090480e33b592658f04 Parents: cfa2998 Author: Aravindan Vijayan <avija...@hortonworks.com> Authored: Fri Aug 11 12:00:07 2017 -0700 Committer: Aravindan Vijayan <avija...@hortonworks.com> Committed: Fri Aug 11 12:00:07 2017 -0700 ---------------------------------------------------------------------- .../server/api/predicate/QueryParser.java | 4 ++- .../api/predicate/operators/InOperator.java | 2 +- .../internal/StackAdvisorResourceProvider.java | 20 ++++++++++++--- .../server/api/predicate/QueryParserTest.java | 18 ++++++++++++++ .../StackAdvisorResourceProviderTest.java | 26 ++++++++++++++++++++ 5 files changed, 65 insertions(+), 5 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/ambari/blob/0630899c/ambari-server/src/main/java/org/apache/ambari/server/api/predicate/QueryParser.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/java/org/apache/ambari/server/api/predicate/QueryParser.java b/ambari-server/src/main/java/org/apache/ambari/server/api/predicate/QueryParser.java index 7386813..6e189c5 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/api/predicate/QueryParser.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/api/predicate/QueryParser.java @@ -107,7 +107,9 @@ public class QueryParser { if (keyObject != null) { String key = keyObject.toString(); if (key.endsWith("/host_name")) { - expression.setRightOperand(expression.getRightOperand().toString().toLowerCase()); + if (expression.getRightOperand() != null) { + expression.setRightOperand(expression.getRightOperand().toString().toLowerCase()); + } } } } http://git-wip-us.apache.org/repos/asf/ambari/blob/0630899c/ambari-server/src/main/java/org/apache/ambari/server/api/predicate/operators/InOperator.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/java/org/apache/ambari/server/api/predicate/operators/InOperator.java b/ambari-server/src/main/java/org/apache/ambari/server/api/predicate/operators/InOperator.java index 629c3fc..eedf2bb 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/api/predicate/operators/InOperator.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/api/predicate/operators/InOperator.java @@ -47,7 +47,7 @@ public class InOperator extends AbstractOperator implements RelationalOperator { public Predicate toPredicate(String prop, String val) throws InvalidQueryException { if (val == null) { - throw new InvalidQueryException("IN operator is missing a required right operand."); + throw new InvalidQueryException("IN operator is missing a required right operand for property " + prop); } String[] tokens = val.split(","); http://git-wip-us.apache.org/repos/asf/ambari/blob/0630899c/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/StackAdvisorResourceProvider.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/StackAdvisorResourceProvider.java b/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/StackAdvisorResourceProvider.java index 5bb52c2..dd32c6b 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/StackAdvisorResourceProvider.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/StackAdvisorResourceProvider.java @@ -21,6 +21,7 @@ package org.apache.ambari.server.controller.internal; import java.util.Collections; import java.util.HashMap; import java.util.HashSet; +import java.util.LinkedHashSet; import java.util.LinkedList; import java.util.List; import java.util.Map; @@ -107,8 +108,22 @@ public abstract class StackAdvisorResourceProvider extends ReadOnlyResourceProvi * * @see JsonRequestBodyParser for arrays parsing */ - List<String> hosts = (List<String>) getRequestProperty(request, HOST_PROPERTY); - List<String> services = (List<String>) getRequestProperty(request, SERVICES_PROPERTY); + Object hostsObject = getRequestProperty(request, HOST_PROPERTY); + if (hostsObject instanceof LinkedHashSet) { + if (((LinkedHashSet)hostsObject).isEmpty()) { + throw new Exception("Empty host list passed to recommendation service"); + } + } + List<String> hosts = (List<String>) hostsObject; + + Object servicesObject = getRequestProperty(request, SERVICES_PROPERTY); + if (servicesObject instanceof LinkedHashSet) { + if (((LinkedHashSet)servicesObject).isEmpty()) { + throw new Exception("Empty service list passed to recommendation service"); + } + } + List<String> services = (List<String>) servicesObject; + Map<String, Set<String>> hgComponentsMap = calculateHostGroupComponentsMap(request); Map<String, Set<String>> hgHostsMap = calculateHostGroupHostsMap(request); Map<String, Set<String>> componentHostsMap = calculateComponentHostsMap(hgComponentsMap, @@ -134,7 +149,6 @@ public abstract class StackAdvisorResourceProvider extends ReadOnlyResourceProvi LOG.warn("Error occurred during preparation of stack advisor request", e); Response response = Response.status(Status.BAD_REQUEST) .entity(String.format("Request body is not correct, error: %s", e.getMessage())).build(); - // TODO: Hosts and services must not be empty throw new WebApplicationException(response); } } http://git-wip-us.apache.org/repos/asf/ambari/blob/0630899c/ambari-server/src/test/java/org/apache/ambari/server/api/predicate/QueryParserTest.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/test/java/org/apache/ambari/server/api/predicate/QueryParserTest.java b/ambari-server/src/test/java/org/apache/ambari/server/api/predicate/QueryParserTest.java index cf332e7..511a8ea 100644 --- a/ambari-server/src/test/java/org/apache/ambari/server/api/predicate/QueryParserTest.java +++ b/ambari-server/src/test/java/org/apache/ambari/server/api/predicate/QueryParserTest.java @@ -36,6 +36,7 @@ import org.apache.ambari.server.controller.predicate.LessPredicate; import org.apache.ambari.server.controller.predicate.NotPredicate; import org.apache.ambari.server.controller.predicate.OrPredicate; import org.apache.ambari.server.controller.spi.Predicate; +import org.junit.Assert; import org.junit.Test; /** @@ -205,6 +206,23 @@ public class QueryParserTest { } @Test + public void testParse_InOp__HostName_Empty() throws Exception { + List<Token> listTokens = new ArrayList<Token>(); + // foo.in(one,two,3) + listTokens.add(new Token(Token.TYPE.RELATIONAL_OPERATOR_FUNC, ".in(")); + listTokens.add(new Token(Token.TYPE.PROPERTY_OPERAND, "Hosts/host_name")); + listTokens.add(new Token(Token.TYPE.BRACKET_CLOSE, ")")); + + QueryParser parser = new QueryParser(); + try { + Predicate p = parser.parse(listTokens.toArray(new Token[listTokens.size()])); + Assert.fail(); + } catch (InvalidQueryException e) { + Assert.assertEquals(e.getMessage(), "IN operator is missing a required right operand for property Hosts/host_name"); + } + } + + @Test public void testParse_EquOp_HostName() throws Exception { List<Token> listTokens = new ArrayList<Token>(); //a=1&!b=2 http://git-wip-us.apache.org/repos/asf/ambari/blob/0630899c/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/StackAdvisorResourceProviderTest.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/StackAdvisorResourceProviderTest.java b/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/StackAdvisorResourceProviderTest.java index 5a3c8af..661749f 100644 --- a/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/StackAdvisorResourceProviderTest.java +++ b/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/StackAdvisorResourceProviderTest.java @@ -21,12 +21,15 @@ package org.apache.ambari.server.controller.internal; import org.apache.ambari.server.controller.AmbariManagementController; import org.apache.ambari.server.controller.spi.Request; import org.apache.ambari.server.controller.spi.Resource; +import org.junit.Assert; import org.junit.Test; +import javax.ws.rs.WebApplicationException; import java.util.ArrayList; import java.util.Collections; import java.util.HashMap; import java.util.HashSet; +import java.util.LinkedHashSet; import java.util.List; import java.util.Map; import java.util.Set; @@ -145,4 +148,27 @@ public class StackAdvisorResourceProviderTest { assertFalse(properties.containsKey("string_prop")); } + + @Test + public void testStackAdvisorWithEmptyHosts() { + Map<Resource.Type, String> keyPropertyIds = Collections.emptyMap(); + Set<String> propertyIds = Collections.emptySet(); + AmbariManagementController ambariManagementController = mock(AmbariManagementController.class); + RecommendationResourceProvider provider = new RecommendationResourceProvider(propertyIds, + keyPropertyIds, ambariManagementController); + + Request request = mock(Request.class); + Set<Map<String, Object>> propertiesSet = new HashSet<Map<String, Object>>(); + Map<String, Object> propertiesMap = new HashMap<String, Object>(); + propertiesMap.put("hosts", new LinkedHashSet<>()); + propertiesMap.put("recommend", "configurations"); + propertiesSet.add(propertiesMap); + doReturn(propertiesSet).when(request).getProperties(); + + try { + provider.createResources(request); + Assert.fail(); + } catch (Exception e) { + } + } }
