AMBARI-1410 - Provide a consistent sort order for API responses

git-svn-id: https://svn.apache.org/repos/asf/incubator/ambari/trunk@1491484 13f79535-47bb-0310-9956-ffa450edef68

ambari-server/src/main/java/org/apache/ambari/server/api/services/serializers/JsonSerializer.java

@@ -30,7 +30,7 @@ import org.codehaus.jackson.util.DefaultPrettyPrinter;
 
 import java.io.*;
 import java.nio.charset.Charset;
-import java.util.HashMap;
+import java.util.LinkedHashMap;
 import java.util.Map;
 
 /**
@@ -137,12 +137,12 @@ public class JsonSerializer implements ResultSerializer {
 
   private TreeNode<Map<String, Object>> getTreeProperties (Map<String, Map<String, Object>> propertiesMap) {
     TreeNode<Map<String, Object>> treeProperties =
-        new TreeNodeImpl<Map<String, Object>>(null, new HashMap<String, Object>(), null);
+        new TreeNodeImpl<Map<String, Object>>(null, new LinkedHashMap<String, Object>(), null);
 
     for (Map.Entry<String, Map<String, Object>> entry : propertiesMap.entrySet()) {
       String category = entry.getKey();
       TreeNode<Map<String, Object>> node;
-      if (category == null) {
+      if (category == null || category.isEmpty()) {
         node = treeProperties;
       } else {
         node = treeProperties.getChild(category);
@@ -152,7 +152,7 @@ public class JsonSerializer implements ResultSerializer {
           for (String t : tokens) {
             TreeNode<Map<String, Object>> child = node.getChild(t);
             if (child == null) {
-              child = node.addChild(new HashMap<String, Object>(), t);
+              child = node.addChild(new LinkedHashMap<String, Object>(), t);
             }
             node = child;
           }

ambari-server/src/main/java/org/apache/ambari/server/api/util/TreeNodeImpl.java

@@ -19,7 +19,7 @@
 package org.apache.ambari.server.api.util;
 
 import java.util.Collection;
-import java.util.HashMap;
+import java.util.LinkedHashMap;
 import java.util.Map;
 
 /**
@@ -40,7 +40,7 @@ public class TreeNodeImpl<T> implements TreeNode<T> {
   /**
    * child nodes
    */
-  private Map<String, TreeNode<T>> m_mapChildren = new HashMap<String, TreeNode<T>>();
+  private Map<String, TreeNode<T>> m_mapChildren = new LinkedHashMap<String, TreeNode<T>>();
 
   /**
    * associated object
@@ -114,7 +114,7 @@ public class TreeNodeImpl<T> implements TreeNode<T> {
   @Override
   public void setProperty(String name, String value) {
     if (m_mapNodeProps == null) {
-      m_mapNodeProps = new HashMap<String, String>();
+      m_mapNodeProps = new LinkedHashMap<String, String>();
     }
     m_mapNodeProps.put(name, value);
   }

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

@@ -27,6 +27,7 @@ import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 import java.util.Collections;
+import java.util.Comparator;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.Iterator;
@@ -34,6 +35,7 @@ import java.util.List;
 import java.util.Map;
 import java.util.NoSuchElementException;
 import java.util.Set;
+import java.util.TreeSet;
 
 /**
  * Default cluster controller implementation.
@@ -41,7 +43,7 @@ import java.util.Set;
 public class ClusterControllerImpl implements ClusterController {
   private final static Logger LOG =
       LoggerFactory.getLogger(ClusterControllerImpl.class);
-  
+
   /**
    * Module of providers for this controller.
    */
@@ -65,6 +67,11 @@ public class ClusterControllerImpl implements ClusterController {
   private final Map<Resource.Type, Schema> schemas =
       new HashMap<Resource.Type, Schema>();
 
+  /**
+   * Resource comparator.
+   */
+  private final ResourceComparator comparator = new ResourceComparator();
+
 
   // ----- Constructors ------------------------------------------------------
 
@@ -90,7 +97,6 @@ public class ClusterControllerImpl implements ClusterController {
       resources = Collections.emptySet();
     } else {
 
-
       if (LOG.isDebugEnabled()) {
         LOG.debug("Using resource provider "
           + provider.getClass().getName()
@@ -98,8 +104,11 @@ public class ClusterControllerImpl implements ClusterController {
       }
       checkProperties(type, request, predicate);
 
-      resources = provider.getResources(request, predicate);
-      resources = populateResources(type, resources, request, predicate);
+      Set<Resource> providerResources = provider.getResources(request, predicate);
+      providerResources               = populateResources(type, providerResources, request, predicate);
+
+      resources = new TreeSet<Resource>(comparator);
+      resources.addAll(providerResources);
     }
     
     return new ResourceIterable(resources, predicate);
@@ -479,4 +488,49 @@ public class ClusterControllerImpl implements ClusterController {
       return null;
     }
   }
+
+  // ----- ResourceComparator inner class ------------------------------------
+
+  private class ResourceComparator implements Comparator<Resource> {
+
+    @Override
+    public int compare(Resource resource1, Resource resource2) {
+
+      int compVal = resource1.getType().compareTo(resource2.getType());
+
+      if (compVal == 0) {
+
+        Schema schema = getSchema(resource1.getType());
+
+        for (Resource.Type type : Resource.Type.values()) {
+          String keyPropertyId = schema.getKeyPropertyId(type);
+          if (keyPropertyId != null) {
+            compVal = compareValues(resource1.getPropertyValue(keyPropertyId),
+                                    resource2.getPropertyValue(keyPropertyId));
+            if (compVal != 0 ) {
+              return compVal;
+            }
+          }
+        }
+      }
+      return compVal;
+    }
+
+    // compare and account for null values
+    private int compareValues(Object val1, Object val2) {
+
+      if (val1 == null || val2 == null) {
+        return val1 == null && val2 == null ? 0 : val1 == null ? -1 : 1;
+      }
+
+      if (val1 instanceof Comparable) {
+        try {
+          return ((Comparable)val1).compareTo(val2);
+        } catch (ClassCastException e) {
+          return 0;
+        }
+      }
+      return 0;
+    }
+  }
 }

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

@@ -23,6 +23,7 @@ import org.apache.ambari.server.controller.utilities.PropertyHelper;
 
 import java.util.HashMap;
 import java.util.Map;
+import java.util.TreeMap;
 
 /**
  * Simple resource implementation.
@@ -37,7 +38,7 @@ public class ResourceImpl implements Resource {
   /**
    * The map of property maps keyed by property category.
    */
-  private final Map<String, Map<String, Object>> propertiesMap = new HashMap<String, Map<String, Object>>();
+  private final Map<String, Map<String, Object>> propertiesMap = new TreeMap<String, Map<String, Object>>();
 
 
   // ----- Constructors ------------------------------------------------------
@@ -64,7 +65,7 @@ public class ResourceImpl implements Resource {
       Map<String, Object> propertyMap = categoryEntry.getValue();
       if (propertyMap != null) {
         for (Map.Entry<String, Object> propertyEntry : propertyMap.entrySet()) {
-          String propertyId    = (category == null ? "" : category + "/") + propertyEntry.getKey();
+          String propertyId    = PropertyHelper.getPropertyId(category, propertyEntry.getKey());
           Object propertyValue = propertyEntry.getValue();
           setProperty(propertyId, propertyValue);
         }
@@ -87,27 +88,30 @@ public class ResourceImpl implements Resource {
 
   @Override
   public void setProperty(String id, Object value) {
-    String category = PropertyHelper.getPropertyCategory(id);
+    String categoryKey = getCategoryKey(PropertyHelper.getPropertyCategory(id));
 
-    Map<String, Object> properties = propertiesMap.get(category);
+    Map<String, Object> properties = propertiesMap.get(categoryKey);
     if (properties == null) {
-      properties = new HashMap<String, Object>();
-      propertiesMap.put(category, properties);
+      properties = new TreeMap<String, Object>();
+      propertiesMap.put(categoryKey, properties);
     }
     properties.put(PropertyHelper.getPropertyName(id), value);
   }
 
   @Override
   public void addCategory(String id) {
-    if (!propertiesMap.containsKey(id)) {
-      propertiesMap.put(id, new HashMap<String, Object>());
+    String categoryKey = getCategoryKey(id);
+
+    if (!propertiesMap.containsKey(categoryKey)) {
+      propertiesMap.put(categoryKey, new HashMap<String, Object>());
     }
   }
 
   @Override
   public Object getPropertyValue(String id) {
-    Map<String, Object> properties =
-        propertiesMap.get(PropertyHelper.getPropertyCategory(id));
+    String categoryKey = getCategoryKey(PropertyHelper.getPropertyCategory(id));
+
+    Map<String, Object> properties = propertiesMap.get(categoryKey);
 
     return properties == null ?
         null : properties.get(PropertyHelper.getPropertyName(id));
@@ -126,4 +130,11 @@ public class ResourceImpl implements Resource {
 
     return sb.toString();
   }
+
+
+  // ----- utility methods ---------------------------------------------------
+
+  private String getCategoryKey(String category) {
+    return category == null ? "" : category;
+  }
 }

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

@@ -85,13 +85,15 @@ public class ClusterControllerImplTest {
   private static final Map<Resource.Type, String> keyPropertyIds = new HashMap<Resource.Type, String>();
 
   static {
-    keyPropertyIds.put(Resource.Type.Cluster, PropertyHelper.getPropertyId("c1", "p1"));
-    keyPropertyIds.put(Resource.Type.Host, PropertyHelper.getPropertyId("c1", "p2"));
+    keyPropertyIds.put(Resource.Type.Cluster, PropertyHelper.getPropertyId("Hosts", "cluster_name"));
+    keyPropertyIds.put(Resource.Type.Host, PropertyHelper.getPropertyId("Hosts", "host_name"));
   }
 
   private static final Set<String> resourceProviderProperties = new HashSet<String>();
 
   static {
+    resourceProviderProperties.add(PropertyHelper.getPropertyId("Hosts", "cluster_name"));
+    resourceProviderProperties.add(PropertyHelper.getPropertyId("Hosts", "host_name"));
     resourceProviderProperties.add(PropertyHelper.getPropertyId("c1", "p1"));
     resourceProviderProperties.add(PropertyHelper.getPropertyId("c1", "p2"));
     resourceProviderProperties.add(PropertyHelper.getPropertyId("c1", "p3"));
@@ -137,6 +139,32 @@ public class ClusterControllerImplTest {
     Assert.assertEquals(4, cnt);
   }
 
+  @Test
+  public void testGetResourcesCheckOrder() throws Exception{
+    ClusterController controller = new ClusterControllerImpl(new TestProviderModule());
+
+    Set<String> propertyIds = new HashSet<String>();
+
+    Request request = PropertyHelper.getReadRequest(propertyIds);
+
+    Iterable<Resource> iterable = controller.getResources(Resource.Type.Host, request, null);
+
+    String lastHostName = null;
+    int cnt = 0;
+    for (Resource resource : iterable) {
+      Assert.assertEquals(Resource.Type.Host, resource.getType());
+
+      String hostName = (String) resource.getPropertyValue(PropertyHelper.getPropertyId("Hosts", "host_name"));
+
+      if (lastHostName != null) {
+        Assert.assertTrue(hostName.compareTo(lastHostName) > 0);
+      }
+      lastHostName = hostName;
+      ++cnt;
+    }
+    Assert.assertEquals(4, cnt);
+  }
+
   @Test
   public void testGetResourcesWithPredicate() throws Exception{
     ClusterController controller = new ClusterControllerImpl(new TestProviderModule());
@@ -436,10 +464,7 @@ public class ClusterControllerImplTest {
 
     private TestProviderModule() {
       providers.put(Resource.Type.Cluster, new TestResourceProvider());
-      providers.put(Resource.Type.Service, new TestResourceProvider());
-      providers.put(Resource.Type.Component, new TestResourceProvider());
       providers.put(Resource.Type.Host, new TestResourceProvider());
-      providers.put(Resource.Type.HostComponent, new TestResourceProvider());
     }
 
     @Override
@@ -466,6 +491,9 @@ public class ClusterControllerImplTest {
       for (int cnt = 0; cnt < 4; ++ cnt) {
         ResourceImpl resource = new ResourceImpl(Resource.Type.Host);
 
+        resource.setProperty(PropertyHelper.getPropertyId("Hosts", "cluster_name"), "cluster");
+        resource.setProperty(PropertyHelper.getPropertyId("Hosts", "host_name"), "host:" + (4 - cnt));
+
         resource.setProperty(PropertyHelper.getPropertyId("c1", "p1"), cnt);
         resource.setProperty(PropertyHelper.getPropertyId("c1", "p2"), cnt % 2);
         resource.setProperty(PropertyHelper.getPropertyId("c1", "p3"), "foo");
@@ -532,9 +560,7 @@ public class ClusterControllerImplTest {
       Update,
       Delete
     }
-
   }
-
 }
 
 

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

@@ -23,6 +23,8 @@ import org.apache.ambari.server.controller.utilities.PropertyHelper;
 import org.apache.ambari.server.controller.spi.Resource;
 import org.junit.Test;
 
+import java.util.Map;
+
 /**
  *
  */
@@ -113,4 +115,50 @@ public class ResourceImplTest {
     Assert.assertEquals(1.99, copy.getPropertyValue(p4));
     Assert.assertEquals(65L, copy.getPropertyValue(p5));
   }
+
+  @Test
+  public void testGetPropertiesMap() {
+    Resource resource = new ResourceImpl(Resource.Type.Cluster);
+
+    String p1 = PropertyHelper.getPropertyId(null, "p1");
+    String p2 = PropertyHelper.getPropertyId("c1", "p2");
+    String p3 = PropertyHelper.getPropertyId("c1/c2", "p3");
+    String p4 = PropertyHelper.getPropertyId("c1/c2/c3", "p4");
+    String p5 = PropertyHelper.getPropertyId("c1", "p5");
+
+    resource.setProperty(p1, "foo");
+    resource.setProperty(p2, 1);
+    resource.setProperty(p3, (float) 1.99);
+    resource.setProperty(p4, 1.99);
+    resource.setProperty(p5, 65L);
+
+    Map<String, Map<String, Object>> map = resource.getPropertiesMap();
+
+    Assert.assertEquals(4, map.keySet().size());
+    Assert.assertTrue(map.containsKey(""));
+    Assert.assertTrue(map.containsKey("c1"));
+    Assert.assertTrue(map.containsKey("c1/c2"));
+    Assert.assertTrue(map.containsKey("c1/c2/c3"));
+
+    // Check order of categories and properties ...
+    String lastCategory = null;
+
+    for (Map.Entry<String, Map<String, Object>> entry : map.entrySet()) {
+      String category = entry.getKey();
+
+      if (lastCategory != null) {
+        Assert.assertTrue(category.compareTo(lastCategory) > 0);
+      }
+      lastCategory = category;
+
+      String lastProperty = null;
+      for (String property : entry.getValue().keySet()) {
+        if (lastProperty != null) {
+          Assert.assertTrue(property.compareTo(lastProperty) > 0);
+        }
+        lastProperty = property;
+      }
+    }
+  }
 }
+