fix(cloudfoundry): Avoid NPE when no instances are present. (#5564) (#…

…5565) * fix(cloudfoundry): Avoid NPE when no instances are present. * test(cloudfoundry): add unit test. * test(cloudfoundry): spotlessJavaApply Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> (cherry picked from commit 0fb3e75) Co-authored-by: Cristhian Castaneda <ccastanedarivera@gmail.com>
spinnaker · Nov 1, 2021 · f8c822e · f8c822e
1 parent 982d9df
commit f8c822e
Show file tree

Hide file tree

Showing 2 changed files with 53 additions and 2 deletions.
diff --git a/...in/java/com/netflix/spinnaker/clouddriver/cloudfoundry/model/CloudFoundryServerGroup.java b/...in/java/com/netflix/spinnaker/clouddriver/cloudfoundry/model/CloudFoundryServerGroup.java
@@ -45,7 +45,7 @@
 @EqualsAndHashCode(of = "id", callSuper = false)
 @Builder(toBuilder = true)
 @JsonDeserialize(builder = CloudFoundryServerGroup.CloudFoundryServerGroupBuilder.class)
-@JsonInclude(JsonInclude.Include.NON_EMPTY)
+@JsonInclude(JsonInclude.Include.NON_NULL)
 @JsonIgnoreProperties("loadBalancerNames")
 public class CloudFoundryServerGroup extends CloudFoundryModel implements ServerGroup {
   private static final ObjectMapper IMAGE_MAPPER = new ObjectMapper();
@@ -99,6 +99,7 @@ public class CloudFoundryServerGroup extends CloudFoundryModel implements Server
   Map<String, Object> env;
 
   @Wither
+  @JsonInclude(JsonInclude.Include.NON_EMPTY)
   @JsonView(Views.Cache.class)
   List<CloudFoundryServiceInstance> serviceInstances;
 
@@ -116,6 +117,7 @@ public class CloudFoundryServerGroup extends CloudFoundryModel implements Server
   Set<CloudFoundryInstance> instances;
 
   @Wither
+  @JsonInclude(JsonInclude.Include.NON_EMPTY)
   @JsonView(Views.Relationship.class)
   Set<String> loadBalancerNames;
 
@@ -124,6 +126,11 @@ public Set<String> getLoadBalancers() {
     return loadBalancerNames == null ? emptySet() : loadBalancerNames;
   }
 
+  @Override
+  public Set<CloudFoundryInstance> getInstances() {
+    return instances == null ? emptySet() : instances;
+  }
+
   @Override
   public ImagesSummary getImagesSummary() {
     return new ImagesSummary() {

diff --git a/...c/test/java/com/netflix/spinnaker/clouddriver/cloudfoundry/cache/CacheRepositoryTest.java b/...c/test/java/com/netflix/spinnaker/clouddriver/cloudfoundry/cache/CacheRepositoryTest.java
@@ -78,15 +78,46 @@ void before() {
                     .build())
             .build();
 
+    CloudFoundryServerGroup serverGroupWithoutInstances =
+        CloudFoundryServerGroup.builder()
+            .name("demo-staging-v001")
+            .account("devaccount")
+            .createdTime(1L)
+            .space(CloudFoundrySpace.fromRegion("myorg > staging"))
+            .droplet(
+                CloudFoundryDroplet.builder()
+                    .id("dropletid")
+                    .name("dropletname")
+                    .buildpacks(
+                        singletonList(
+                            CloudFoundryBuildpack.builder().buildpackName("java").build()))
+                    .sourcePackage(CloudFoundryPackage.builder().checksum("check").build())
+                    .build())
+            .build();
+
     CloudFoundryCluster cluster =
         CloudFoundryCluster.builder()
             .accountName("devaccount")
             .name("demo-dev")
             .serverGroups(singleton(serverGroup))
             .build();
+
+    CloudFoundryCluster clusterWithoutInstances =
+        CloudFoundryCluster.builder()
+            .accountName("devaccount")
+            .name("demo-staging")
+            .serverGroups(singleton(serverGroupWithoutInstances))
+            .build();
+
     CloudFoundryApplication app =
         CloudFoundryApplication.builder().name("demo").clusters(singleton(cluster)).build();
 
+    CloudFoundryApplication appWithoutInstances =
+        CloudFoundryApplication.builder()
+            .name("demo-without-instances")
+            .clusters(singleton(clusterWithoutInstances))
+            .build();
+
     CloudFoundryClient client = mock(CloudFoundryClient.class);
     Applications apps = mock(Applications.class);
     Routes routes = mock(Routes.class);
@@ -95,7 +126,7 @@ void before() {
 
     when(client.getApplications()).thenReturn(apps);
     when(client.getRoutes()).thenReturn(routes);
-    when(apps.all(emptyList())).thenReturn(singletonList(app));
+    when(apps.all(emptyList())).thenReturn(List.of(app, appWithoutInstances));
     when(routes.all(emptyList())).thenReturn(emptyList());
     when(providerCache.filterIdentifiers(any(), any())).thenReturn(emptyList());
     when(providerCache.getAll(any(), anyCollectionOf(String.class))).thenReturn(emptyList());
@@ -194,4 +225,17 @@ void findServerGroup() {
                       });
             });
   }
+
+  @Test
+  void findServerGroupWithoutInstances() {
+    assertThat(
+            repo.findServerGroupByKey(
+                Keys.getServerGroupKey("devaccount", "demo-staging-v001", "myorg > staging"), FULL))
+        .hasValueSatisfying(
+            serverGroup -> {
+              assertThat(serverGroup.getName()).isEqualTo("demo-staging-v001");
+              assertThat(serverGroup.getInstances()).isNotNull();
+              assertThat(serverGroup.getInstances()).isEmpty();
+            });
+  }
 }