GEODE-7717: ClusterManagementListResult should show a list of EntityI…

…nfo (apache#4673) * only show links in the EntityInfo level * rework list member responses
kekerzzZ · Feb 7, 2020 · bda6bdf · bda6bdf
1 parent 93aa3c4
commit bda6bdf
Show file tree

Hide file tree

Showing 18 changed files with 253 additions and 105 deletions.
diff --git a/...utedTest/java/org/apache/geode/management/internal/rest/ListIndexManagementDUnitTest.java b/...utedTest/java/org/apache/geode/management/internal/rest/ListIndexManagementDUnitTest.java
@@ -154,7 +154,7 @@ public void getIndex_succeeds_with_index_name_and_region_name_filter() {
           .isEqualTo("/region1");
       softly.assertThat(indexConfig.getExpression()).as("get index: expression").isEqualTo("id");
       EntityGroupInfo<Index, IndexInfo> entityGroupInfo =
-          cms.get(this.indexConfig).getResult().getConfigurationByGroup().get(0);
+          cms.get(this.indexConfig).getResult().getGroups().get(0);
       Index indexConfigTwo = entityGroupInfo.getConfiguration();
       softly.assertThat(indexConfigTwo.getLinks().getLinks()).as("get index: links key")
           .containsKey("region");

diff --git a/...c/main/java/org/apache/geode/management/internal/api/LocatorClusterManagementService.java b/...c/main/java/org/apache/geode/management/internal/api/LocatorClusterManagementService.java
@@ -367,26 +367,36 @@ public <T extends AbstractConfiguration<R>, R extends RuntimeInfo> ClusterManage
       response.setRuntimeInfo(runtimeInfos);
     }
 
-    result.setResult(responses);
+    if (filter instanceof Member) {
+      // for members, we have exactly one response that holds the filter
+      // and all the members in the runtimeInfo section
+      List<R> members = responses.get(0).getRuntimeInfo();
+      for (R memberInfo : members) {
+        Member member = new Member();
+        member.setId(memberInfo.getMemberName());
+        EntityInfo<T, R> entityInfo = new EntityInfo<>(memberInfo.getMemberName(),
+            Collections.singletonList(
+                new EntityGroupInfo<>((T) member, Collections.singletonList(memberInfo))));
+        result.addEntityInfo(entityInfo);
+      }
+    } else {
+      result.setEntityGroupInfo(responses);
+    }
     return assertSuccessful(result);
   }
 
   @Override
   public <T extends AbstractConfiguration<R>, R extends RuntimeInfo> ClusterManagementGetResult<T, R> get(
       T config) {
     ClusterManagementListResult<T, R> list = list(config);
-    List<EntityGroupInfo<T, R>> result = list.getResult();
-    int size = result.size();
-    if (config instanceof Member) {
-      size = result.get(0).getRuntimeInfo().size();
-    }
 
-    if (size == 0) {
+    List<EntityInfo<T, R>> result = list.getResult();
+    if (result.size() == 0) {
       raise(StatusCode.ENTITY_NOT_FOUND,
           config.getClass().getSimpleName() + " '" + config.getId() + "' does not exist.");
     }
-    EntityInfo<T, R> entityInfo = new EntityInfo<>(config.getId(), result);
-    return new ClusterManagementGetResult<>(entityInfo);
+
+    return new ClusterManagementGetResult<>(result.get(0));
   }
 
   @Override

diff --git a/...e/src/test/java/org/apache/geode/management/internal/api/ClusterManagementResultTest.java b/...e/src/test/java/org/apache/geode/management/internal/api/ClusterManagementResultTest.java
@@ -134,7 +134,7 @@ public void deserialize() throws Exception {
     String json = "{\"statusCode\":\"OK\"}";
     ClusterManagementListResult result =
         GeodeJsonMapper.getMapper().readValue(json, ClusterManagementListResult.class);
-    assertThat(result.getResult()).isNotNull().isEmpty();
+    assertThat(result.getEntityGroupInfo()).isNotNull().isEmpty();
   }
 
   @Test
@@ -152,7 +152,7 @@ public void serializeResult() throws Exception {
     RuntimeRegionInfo info = new RuntimeRegionInfo();
     info.setEntryCount(3);
     response.setRuntimeInfo(Collections.singletonList(info));
-    result.setResult(Collections.singletonList(response));
+    result.setEntityGroupInfo(Collections.singletonList(response));
 
     String json = mapper.writeValueAsString(result);
     System.out.println(json);

diff --git a/...st/java/org/apache/geode/management/internal/api/LocatorClusterManagementServiceTest.java b/...st/java/org/apache/geode/management/internal/api/LocatorClusterManagementServiceTest.java
@@ -15,6 +15,7 @@
 
 package org.apache.geode.management.internal.api;
 
+import static java.util.Arrays.asList;
 import static java.util.Collections.emptyList;
 import static java.util.stream.Collectors.toList;
 import static org.assertj.core.api.Assertions.assertThat;
@@ -63,6 +64,7 @@
 import org.apache.geode.management.api.ClusterManagementRealizationResult;
 import org.apache.geode.management.api.ClusterManagementResult;
 import org.apache.geode.management.api.EntityGroupInfo;
+import org.apache.geode.management.api.EntityInfo;
 import org.apache.geode.management.api.RealizationResult;
 import org.apache.geode.management.configuration.Index;
 import org.apache.geode.management.configuration.Member;
@@ -409,7 +411,7 @@ public void get_whenResponseHasNoConfigurationResults() throws Exception {
     Region filter = new Region();
     ClusterManagementListResult<Region, RuntimeRegionInfo> result =
         mock(ClusterManagementListResult.class);
-    when(result.getResult()).thenReturn(emptyList());
+    when(result.getEntityGroupInfo()).thenReturn(emptyList());
 
     doReturn(result).when(service).list(same(filter));
 
@@ -424,17 +426,18 @@ public void get_whenResponseHasConfigurationResults() throws Exception {
     ClusterManagementListResult<Region, RuntimeRegionInfo> listResult =
         mock(ClusterManagementListResult.class);
 
-    List<EntityGroupInfo<Region, RuntimeRegionInfo>> entityGroupInfos = new ArrayList<>();
-    for (int i = 0; i < 3; i++) {
-      entityGroupInfos.add(mock(EntityGroupInfo.class));
-    }
-    when(listResult.getResult()).thenReturn(entityGroupInfos);
+    List<EntityGroupInfo<Region, RuntimeRegionInfo>> entityGroupInfos =
+        asList(mock(EntityGroupInfo.class));
+    List<EntityInfo<Region, RuntimeRegionInfo>> entityInfos =
+        asList(new EntityInfo<>("id", entityGroupInfos));
+
+    when(listResult.getResult()).thenReturn(entityInfos);
 
     doReturn(listResult).when(service).list(same(filter));
 
     ClusterManagementGetResult<Region, RuntimeRegionInfo> getResult = service.get(filter);
 
-    assertThat(getResult.getResult().getConfigurationByGroup()).isSameAs(entityGroupInfos);
+    assertThat(getResult.getResult().getGroups()).isSameAs(entityGroupInfos);
   }
 
   @Test
@@ -447,7 +450,7 @@ public void getMember_whenNoMembers() {
         mock(EntityGroupInfo.class);
     when(entityGroupInfo.getRuntimeInfo()).thenReturn(emptyList());
     entityGroupInfos.add(entityGroupInfo);
-    when(listResult.getResult()).thenReturn(entityGroupInfos);
+    when(listResult.getEntityGroupInfo()).thenReturn(entityGroupInfos);
 
     assertThatThrownBy(() -> service.get(member))
         .isInstanceOf(ClusterManagementException.class)

diff --git a/...rc/main/java/org/apache/geode/test/junit/assertions/ClusterManagementGetResultAssert.java b/...rc/main/java/org/apache/geode/test/junit/assertions/ClusterManagementGetResultAssert.java
@@ -57,15 +57,15 @@ public ClusterManagementGetResultAssert<T, R> containsStatusMessage(String statu
   }
 
   public EntityGroupInfo<T, R> getResult() {
-    return actual.getResult().getConfigurationByGroup().get(0);
+    return actual.getResult().getGroups().get(0);
   };
 
   public ObjectAssert<T> hasConfiguration() {
-    return assertThat(getActual().getResult().getConfigurationByGroup().get(0).getConfiguration());
+    return assertThat(getActual().getResult().getGroups().get(0).getConfiguration());
   }
 
   public ListAssert<R> hasRuntimeInfos() {
-    return assertThat(getActual().getResult().getConfigurationByGroup().get(0).getRuntimeInfo());
+    return assertThat(getActual().getResult().getGroups().get(0).getRuntimeInfo());
   }
 
   public static <T extends AbstractConfiguration<R>, R extends RuntimeInfo> ClusterManagementGetResultAssert<T, R> assertManagementGetResult(

diff --git a/...c/main/java/org/apache/geode/test/junit/assertions/ClusterManagementListResultAssert.java b/...c/main/java/org/apache/geode/test/junit/assertions/ClusterManagementListResultAssert.java
@@ -58,7 +58,7 @@ public ClusterManagementListResultAssert<T, R> containsStatusMessage(String stat
   }
 
   public List<EntityGroupInfo<T, R>> getResult() {
-    return actual.getResult();
+    return actual.getEntityGroupInfo();
   };
 
   public ListAssert<T> hasConfigurations() {

diff --git a/...management/src/main/java/org/apache/geode/management/api/ClusterManagementListResult.java b/...management/src/main/java/org/apache/geode/management/api/ClusterManagementListResult.java
@@ -14,13 +14,13 @@
  */
 package org.apache.geode.management.api;
 
-import java.util.ArrayList;
+import java.util.HashMap;
 import java.util.List;
+import java.util.Map;
 import java.util.stream.Collectors;
 
 import com.fasterxml.jackson.annotation.JsonIgnore;
 import com.fasterxml.jackson.annotation.JsonInclude;
-import com.fasterxml.jackson.annotation.JsonProperty;
 
 import org.apache.geode.annotations.Experimental;
 import org.apache.geode.management.configuration.AbstractConfiguration;
@@ -34,50 +34,72 @@
 @Experimental
 public class ClusterManagementListResult<T extends AbstractConfiguration<R>, R extends RuntimeInfo>
     extends ClusterManagementResult {
-  /**
-   * for internal use only
-   */
+
   public ClusterManagementListResult() {}
 
-  /**
-   * for internal use only
-   */
   public ClusterManagementListResult(StatusCode statusCode, String message) {
     super(statusCode, message);
   }
 
-  // Override the mapper setting so that we always show result
-  @JsonInclude
-  @JsonProperty
-  private List<EntityGroupInfo<T, R>> result = new ArrayList<>();
+  private final Map<String, EntityInfo<T, R>> entities = new HashMap<>();
 
   /**
    * Returns the combined payload of the list call
    */
-  public List<EntityGroupInfo<T, R>> getResult() {
-    return result;
+  @JsonIgnore
+  public List<EntityGroupInfo<T, R>> getEntityGroupInfo() {
+    return entities.values().stream()
+        .flatMap(x -> x.getGroups().stream())
+        .collect(Collectors.toList());
+  }
+
+  // this annotation makes sure we always show the result even though it's an empty list
+  @JsonInclude
+  public List<EntityInfo<T, R>> getResult() {
+    return entities.values().stream().collect(Collectors.toList());
+  }
+
+  public void setResult(List<EntityInfo<T, R>> entities) {
+    this.entities.clear();
+    for (EntityInfo entity : entities) {
+      this.entities.put(entity.getId(), entity);
+    }
+  }
+
+  public void addEntityInfo(EntityInfo<T, R> entityInfo) {
+    this.entities.put(entityInfo.getId(), entityInfo);
+  }
+
+  @JsonIgnore
+  public void setEntityGroupInfo(List<EntityGroupInfo<T, R>> entityGroupInfos) {
+    this.entities.clear();
+    for (EntityGroupInfo entityGroupInfo : entityGroupInfos) {
+      String id = entityGroupInfo.getConfiguration().getId();
+      EntityInfo<T, R> entity = this.entities.get(id);
+      if (entity == null) {
+        entity = new EntityInfo<>();
+        entity.setId(id);
+        this.entities.put(id, entity);
+      }
+      entity.getGroups().add(entityGroupInfo);
+    }
   }
 
   /**
    * Returns only the static config portion of the results
    */
   @JsonIgnore
   public List<T> getConfigResult() {
-    return result.stream().map(EntityGroupInfo::getConfiguration).collect(Collectors.toList());
+    return getEntityGroupInfo().stream().map(EntityGroupInfo::getConfiguration)
+        .collect(Collectors.toList());
   }
 
   /**
    * Returns only the runtime information portion of the results
    */
   @JsonIgnore
   public List<R> getRuntimeResult() {
-    return result.stream().flatMap(r -> r.getRuntimeInfo().stream()).collect(Collectors.toList());
-  }
-
-  /**
-   * for internal use only
-   */
-  public void setResult(List<EntityGroupInfo<T, R>> result) {
-    this.result = result;
+    return getEntityGroupInfo().stream().flatMap(r -> r.getRuntimeInfo().stream())
+        .collect(Collectors.toList());
   }
 }
diff --git a/geode-management/src/main/java/org/apache/geode/management/api/EntityGroupInfo.java b/geode-management/src/main/java/org/apache/geode/management/api/EntityGroupInfo.java
@@ -21,7 +21,7 @@
 import java.util.List;
 import java.util.Objects;
 
-import com.fasterxml.jackson.annotation.JsonProperty;
+import com.fasterxml.jackson.annotation.JsonIgnore;
 
 import org.apache.geode.annotations.Experimental;
 import org.apache.geode.management.configuration.AbstractConfiguration;
@@ -87,8 +87,11 @@ public void setRuntimeInfo(List<R> runtimeInfo) {
     this.runtimeInfo = runtimeInfo;
   }
 
-  @JsonProperty(access = JsonProperty.Access.READ_ONLY)
+  @JsonIgnore
   public Links getLinks() {
+    if (configuration == null) {
+      return null;
+    }
     return configuration.getLinks();
   }
 

diff --git a/geode-management/src/main/java/org/apache/geode/management/api/EntityInfo.java b/geode-management/src/main/java/org/apache/geode/management/api/EntityInfo.java
@@ -26,6 +26,7 @@
 import com.fasterxml.jackson.annotation.JsonProperty;
 
 import org.apache.geode.management.configuration.AbstractConfiguration;
+import org.apache.geode.management.configuration.Links;
 import org.apache.geode.management.runtime.RuntimeInfo;
 
 /**
@@ -39,13 +40,13 @@ public class EntityInfo<T extends AbstractConfiguration<R>, R extends RuntimeInf
   private String id;
   @JsonInclude
   @JsonProperty
-  private List<EntityGroupInfo<T, R>> configurationByGroup = new ArrayList<>();
+  private List<EntityGroupInfo<T, R>> groups = new ArrayList<>();
 
   public EntityInfo() {}
 
-  public EntityInfo(String id, List<EntityGroupInfo<T, R>> configurationByGroup) {
+  public EntityInfo(String id, List<EntityGroupInfo<T, R>> groups) {
     this.id = id;
-    this.configurationByGroup = configurationByGroup;
+    this.groups = groups;
   }
 
   public String getId() {
@@ -56,28 +57,36 @@ public void setId(String id) {
     this.id = id;
   }
 
-  public List<EntityGroupInfo<T, R>> getConfigurationByGroup() {
-    return configurationByGroup;
+  public List<EntityGroupInfo<T, R>> getGroups() {
+    return groups;
   }
 
-  public void setConfigurationByGroup(List<EntityGroupInfo<T, R>> configurationByGroup) {
-    this.configurationByGroup = configurationByGroup;
+  public void setGroups(List<EntityGroupInfo<T, R>> groups) {
+    this.groups = groups;
   }
 
   @JsonIgnore
   public List<T> getConfigurations() {
-    return configurationByGroup.stream()
+    return groups.stream()
         .map(EntityGroupInfo::getConfiguration)
         .collect(toList());
   }
 
   @JsonIgnore
   public List<R> getRuntimeInfos() {
-    return configurationByGroup.stream()
+    return groups.stream()
         .flatMap(r -> r.getRuntimeInfo().stream())
         .collect(toList());
   }
 
+  @JsonProperty(access = JsonProperty.Access.READ_ONLY)
+  public Links getLinks() {
+    if (groups.isEmpty()) {
+      return null;
+    }
+    return groups.get(0).getLinks();
+  }
+
   @Override
   public boolean equals(Object o) {
     if (this == o) {
@@ -88,19 +97,19 @@ public boolean equals(Object o) {
     }
     EntityInfo<?, ?> that = (EntityInfo<?, ?>) o;
     return Objects.equals(id, that.id) &&
-        Objects.equals(configurationByGroup, that.configurationByGroup);
+        Objects.equals(groups, that.groups);
   }
 
   @Override
   public int hashCode() {
-    return Objects.hash(id, configurationByGroup);
+    return Objects.hash(id, groups);
   }
 
   @Override
   public String toString() {
     return "ConfigurationInfo{" +
         "id='" + id + '\'' +
-        ", configurationByGroup=" + configurationByGroup +
+        ", configurationByGroup=" + groups +
         '}';
   }
 }