AMBARI-8342 - Requesting the config_groups collection resource with no registered configuration groups results in a 404 response (tbeerbower)

ambari-server/src/main/java/org/apache/ambari/server/api/query/QueryImpl.java

@@ -372,13 +372,13 @@ public class QueryImpl implements Query, ResourceInstance {
 
     Request request = createRequest();
 
-    // use linked hashsets so that we maintain inseration and traversal order
+    // use linked hash sets so that we maintain insertion and traversal order
     // in the event that the resource provider already gave us a sorted set
     // back
     Set<Resource> resourceSet = new LinkedHashSet<Resource>();
     Set<Resource> providerResourceSet = new LinkedHashSet<Resource>();
 
-    QueryResponse queryResponse = doQuery(resourceType, request, queryPredicate);
+    QueryResponse queryResponse = doQuery(resourceType, request, queryPredicate, true);
 
     // If there is a page request and the predicate does not contain properties
     // that need to be set
@@ -431,7 +431,9 @@ public class QueryImpl implements Query, ResourceInstance {
           Set<Resource> resourceSet    = new LinkedHashSet<Resource>();
 
           try {
-            Set<Resource> queryResources = subResource.doQuery(resourceType, request, queryPredicate).getResources();
+            Set<Resource> queryResources =
+                subResource.doQuery(resourceType, request, queryPredicate, false).getResources();
+
             providerResourceSet.addAll(queryResources);
             resourceSet.addAll(queryResources);
           } catch (NoSuchResourceException e) {
@@ -450,8 +452,17 @@ public class QueryImpl implements Query, ResourceInstance {
 
   /**
    * Query the cluster controller for the resources.
+   *
+   * @param type                the resource type
+   * @param request             the request information
+   * @param predicate           the predicate
+   * @param checkEmptyResponse  true if an empty query response can trigger a NoSuchResourceException
+   *
+   * @return the result of the cluster controller query
+   *
+   * @throws NoSuchResourceException if a specific resource was asked for and not found and checkEmptyResponse == true
    */
-  private QueryResponse doQuery(Resource.Type type, Request request, Predicate predicate)
+  private QueryResponse doQuery(Resource.Type type, Request request, Predicate predicate, boolean checkEmptyResponse)
       throws UnsupportedPropertyException,
       SystemException,
       NoSuchResourceException,
@@ -461,7 +472,18 @@ public class QueryImpl implements Query, ResourceInstance {
       LOG.debug("Executing resource query: " + request + " where " + predicate);
     }
 
-    return clusterController.getResources(type, request, predicate);
+    QueryResponse queryResponse = clusterController.getResources(type, request, predicate);
+
+    if (checkEmptyResponse && queryResponse.getResources().isEmpty()) {
+
+      // If this is not a collection request then we must throw
+      // NoSuchResourceException (404 response) for an empty query result
+      if(!isCollectionResource()) {
+        throw new NoSuchResourceException(
+            "The requested resource doesn't exist: " + type.toString() + " not found where " + predicate + ".");
+      }
+    }
+    return queryResponse;
   }
 
   /**

ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ClusterControllerImpl.java

@@ -130,13 +130,6 @@ public class ClusterControllerImpl implements ClusterController {
 
       // get the resources
       queryResponse = provider.queryForResources(request, predicate);
-
-      // if a specific resource was asked for and not found then throw exception
-      if (predicate != null &&
-          (queryResponse == null || queryResponse.getResources() == null || queryResponse.getResources().isEmpty())) {
-        throw new NoSuchResourceException(
-            "The requested resource doesn't exist: " + type.toString() + " not found, " + predicate);
-      }
     }
     return queryResponse == null ? new QueryResponseImpl(Collections.<Resource>emptySet()) : queryResponse;
   }

ambari-server/src/test/java/org/apache/ambari/server/api/query/QueryImplTest.java

@@ -20,7 +20,11 @@
 package org.apache.ambari.server.api.query;
 
 
+import static org.easymock.EasyMock.anyBoolean;
+import static org.easymock.EasyMock.anyObject;
+import static org.easymock.EasyMock.capture;
 import static org.easymock.EasyMock.createNiceMock;
+import static org.easymock.EasyMock.eq;
 import static org.easymock.EasyMock.expect;
 import static org.easymock.EasyMock.replay;
 import static org.easymock.EasyMock.verify;
@@ -31,10 +35,13 @@ import static org.junit.Assert.fail;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.HashSet;
+import java.util.Iterator;
 import java.util.Map;
 import java.util.Set;
 
 import org.apache.ambari.server.api.query.render.DefaultRenderer;
+import org.apache.ambari.server.api.query.render.Renderer;
+import org.apache.ambari.server.api.resources.ClusterResourceDefinition;
 import org.apache.ambari.server.api.resources.ResourceDefinition;
 import org.apache.ambari.server.api.resources.ResourceInstance;
 import org.apache.ambari.server.api.resources.StackResourceDefinition;
@@ -42,17 +49,23 @@ import org.apache.ambari.server.api.resources.StackVersionResourceDefinition;
 import org.apache.ambari.server.api.resources.SubResourceDefinition;
 import org.apache.ambari.server.api.services.Result;
 import org.apache.ambari.server.api.util.TreeNode;
+import org.apache.ambari.server.api.util.TreeNodeImpl;
 import org.apache.ambari.server.controller.internal.ClusterControllerImpl;
 import org.apache.ambari.server.controller.internal.ClusterControllerImplTest;
 import org.apache.ambari.server.controller.internal.PageRequestImpl;
+import org.apache.ambari.server.controller.spi.ClusterController;
 import org.apache.ambari.server.controller.spi.NoSuchParentResourceException;
 import org.apache.ambari.server.controller.spi.NoSuchResourceException;
 import org.apache.ambari.server.controller.spi.PageRequest;
 import org.apache.ambari.server.controller.spi.Predicate;
+import org.apache.ambari.server.controller.spi.QueryResponse;
 import org.apache.ambari.server.controller.spi.Resource;
+import org.apache.ambari.server.controller.spi.Schema;
+import org.apache.ambari.server.controller.spi.SortRequest;
 import org.apache.ambari.server.controller.spi.SystemException;
 import org.apache.ambari.server.controller.spi.UnsupportedPropertyException;
 import org.apache.ambari.server.controller.utilities.PredicateBuilder;
+import org.easymock.Capture;
 import org.junit.Assert;
 import org.junit.Test;
 
@@ -245,6 +258,99 @@ public class QueryImplTest {
     Assert.assertEquals("centos5", osResource.getPropertyValue("OperatingSystems/os_type"));
   }
 
+  @Test
+  public void testExecute_NoSuchResourceException() throws Exception {
+    ResourceDefinition resourceDefinition = new ClusterResourceDefinition();
+
+    Map<Resource.Type, String> mapIds = new HashMap<Resource.Type, String>();
+    mapIds.put(Resource.Type.Cluster, "c1");
+
+    ClusterController clusterController = createNiceMock(ClusterController.class);
+    QueryResponse queryResponse = createNiceMock(QueryResponse.class);
+    Schema schema = createNiceMock(Schema.class);
+    Renderer renderer = createNiceMock(Renderer.class);
+
+    expect(clusterController.getSchema(Resource.Type.Cluster)).andReturn(schema).anyTimes();
+
+    expect(clusterController.getResources(eq(Resource.Type.Cluster),
+        anyObject(org.apache.ambari.server.controller.spi.Request.class), anyObject(Predicate.class))).
+        andReturn(queryResponse);
+
+    // Always return an empty set of resources.
+    expect(queryResponse.getResources()).andReturn(Collections.<Resource>emptySet()).anyTimes();
+
+    expect(schema.getKeyPropertyId(Resource.Type.Cluster)).andReturn("Clusters/cluster_name").anyTimes();
+
+    TreeNode<Set<String>> treeNode = new TreeNodeImpl<Set<String>>(null, Collections.EMPTY_SET, null);
+    expect(renderer.finalizeProperties(anyObject(TreeNode.class), anyBoolean())).andReturn(treeNode).anyTimes();
+
+    replay(clusterController, queryResponse, schema, renderer);
+
+    //test
+    QueryImpl query = new TestQuery(mapIds, resourceDefinition, clusterController);
+    query.setRenderer(renderer);
+
+    try {
+      query.execute();
+      fail("Expected NoSuchResourceException!");
+
+    } catch (NoSuchResourceException e) {
+      //expected
+    }
+    verify(clusterController, queryResponse, schema, renderer);
+  }
+
+  @Test
+  public void testExecute_collection_NoSuchResourceException() throws Exception {
+    ResourceDefinition resourceDefinition = new ClusterResourceDefinition();
+
+    ClusterController clusterController = createNiceMock(ClusterController.class);
+    QueryResponse queryResponse = createNiceMock(QueryResponse.class);
+    Schema schema = createNiceMock(Schema.class);
+    Renderer renderer = createNiceMock(Renderer.class);
+    Iterable<Resource> iterable = createNiceMock(Iterable.class);
+    Iterator<Resource> iterator = createNiceMock(Iterator.class);
+
+    expect(clusterController.getSchema(Resource.Type.Cluster)).andReturn(schema).anyTimes();
+
+    expect(clusterController.getResources(eq(Resource.Type.Cluster),
+        anyObject(org.apache.ambari.server.controller.spi.Request.class), anyObject(Predicate.class))).
+        andReturn(queryResponse);
+
+    expect(clusterController.getIterable(eq(Resource.Type.Cluster), anyObject(QueryResponse.class),
+        anyObject(org.apache.ambari.server.controller.spi.Request.class), anyObject(Predicate.class),
+        anyObject(PageRequest.class), anyObject(SortRequest.class))).andReturn(iterable).anyTimes();
+
+    expect(iterable.iterator()).andReturn(iterator).anyTimes();
+
+    expect(iterator.hasNext()).andReturn(false).anyTimes();
+
+    // Always return an empty set of resources.
+    expect(queryResponse.getResources()).andReturn(Collections.<Resource>emptySet()).anyTimes();
+
+    expect(schema.getKeyPropertyId(Resource.Type.Cluster)).andReturn("Clusters/cluster_name").anyTimes();
+
+    TreeNode<Set<String>> treeNode = new TreeNodeImpl<Set<String>>(null, Collections.<String>emptySet(), null);
+    expect(renderer.finalizeProperties(anyObject(TreeNode.class), anyBoolean())).andReturn(treeNode).anyTimes();
+
+    Capture<Result> resultCapture = new Capture<Result>();
+
+    expect(renderer.finalizeResult(capture(resultCapture))).andReturn(null);
+
+    replay(clusterController, queryResponse, schema, renderer, iterable, iterator);
+
+    //test
+    QueryImpl query = new TestQuery(new HashMap<Resource.Type, String>(), resourceDefinition, clusterController);
+    query.setRenderer(renderer);
+
+    query.execute();
+
+    TreeNode<Resource> tree = resultCapture.getValue().getResultTree();
+    Assert.assertEquals(0, tree.getChildren().size());
+
+    verify(clusterController, queryResponse, schema, renderer, iterable, iterator);
+  }
+
   @Test
   public void testExecute__Stack_instance_specifiedSubResources() throws Exception {
     ResourceDefinition resourceDefinition = new StackResourceDefinition();
@@ -668,6 +774,11 @@ public class QueryImplTest {
       super(mapIds, resourceDefinition, new ClusterControllerImpl(new ClusterControllerImplTest.TestProviderModule()));
       setRenderer(new DefaultRenderer());
     }
+
+    public TestQuery(Map<Resource.Type, String> mapIds, ResourceDefinition resourceDefinition, ClusterController clusterController) {
+      super(mapIds, resourceDefinition, clusterController);
+      setRenderer(new DefaultRenderer());
+    }
   }
 }
 

ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ClusterControllerImplTest.java

@@ -154,41 +154,6 @@ public class ClusterControllerImplTest {
     Assert.assertEquals(4, cnt);
   }
 
-  @Test
-  public void testGetResources_NoSuchResourceException() throws Exception{
-
-    ProviderModule providerModule = createNiceMock(ProviderModule.class);
-    ResourceProvider resourceProvider = createNiceMock(ResourceProvider.class);
-
-    expect(providerModule.getResourceProvider(Resource.Type.View)).andReturn(resourceProvider).anyTimes();
-    expect(resourceProvider.checkPropertyIds(Collections.singleton("ViewInfo/view_name"))).andReturn(Collections.<String>emptySet()).anyTimes();
-    expect(resourceProvider.getResources(anyObject(Request.class), anyObject(Predicate.class))).andReturn(Collections.<Resource>emptySet()).anyTimes();
-
-    replay(providerModule, resourceProvider);
-
-    ClusterControllerImpl controller = new ClusterControllerImpl(providerModule);
-
-    Set<String> propertyIds = new HashSet<String>();
-
-    propertyIds.add("ViewInfo/view_name");
-
-    Request request = PropertyHelper.getReadRequest(propertyIds);
-
-    // No predicate
-    Iterator<Resource> iter = controller.getResourceIterable(Resource.Type.View, request, null).iterator();
-    Assert.assertFalse(iter.hasNext());
-
-    // Ask for a specific resource.
-    Predicate predicate = new PredicateBuilder().property("ViewInfo/view_name").equals("BadView").toPredicate();
-    try {
-      controller.getResourceIterable(Resource.Type.View, request, predicate);
-      Assert.fail("Expected NoSuchResourceException.");
-    } catch (NoSuchResourceException e) {
-      // expected
-    }
-    verify(providerModule, resourceProvider);
-  }
-
   @Test
   public void testGetResourcesPageFromStart() throws Exception{
     ClusterControllerImpl controller = new ClusterControllerImpl(new TestProviderModule());