From 26eba64415c509e8edc70abc0daa7cbdf6205bb9 Mon Sep 17 00:00:00 2001
From: staticfinalzero <hedredm@gmail.com>
Date: Fri, 17 May 2024 15:02:04 -0400
Subject: [PATCH] cache: bug fix to requested resource does not exist handling

we need to keep watch active for when resource is finally created so we can notify client.

https://github.com/envoyproxy/java-control-plane/issues/219
---
 .../controlplane/cache/SimpleCache.java       |  7 +--
 .../cache/v3/SimpleCacheTest.java             | 58 +++++++++++++++++++
 2 files changed, 61 insertions(+), 4 deletions(-)

diff --git a/cache/src/main/java/io/envoyproxy/controlplane/cache/SimpleCache.java b/cache/src/main/java/io/envoyproxy/controlplane/cache/SimpleCache.java
index 7f0a445ac..273c98c63 100644
--- a/cache/src/main/java/io/envoyproxy/controlplane/cache/SimpleCache.java
+++ b/cache/src/main/java/io/envoyproxy/controlplane/cache/SimpleCache.java
@@ -377,10 +377,9 @@ protected void respondWithSpecificOrder(T group,
                   version);
             }
 
-            respond(watch, snapshot, group);
-
-            // Discard the watch. A new watch will be created for future snapshots once envoy ACKs the response.
-            return true;
+            // If we respond with an actual message, we can proceed to discard the watch. 
+            // A new watch will be created for future snapshots once envoy ACKs the response.
+            return respond(watch, snapshot, group);
           }
 
           // Do not discard the watch. The request version is the same as the snapshot version, so we wait to respond.
diff --git a/cache/src/test/java/io/envoyproxy/controlplane/cache/v3/SimpleCacheTest.java b/cache/src/test/java/io/envoyproxy/controlplane/cache/v3/SimpleCacheTest.java
index b8654f6ba..4a4684edd 100644
--- a/cache/src/test/java/io/envoyproxy/controlplane/cache/v3/SimpleCacheTest.java
+++ b/cache/src/test/java/io/envoyproxy/controlplane/cache/v3/SimpleCacheTest.java
@@ -476,6 +476,64 @@ public void watchIsLeftOpenIfNotRespondedImmediately() {
     assertThatWatchIsOpenWithNoResponses(new WatchAndTracker(watch, responseTracker));
   }
 
+  @Test
+  public void watchIsLeftOpenIfNotRespondedImmediatelyAndThroughSubsequentSetEmptySnapshots() {
+    SimpleCache<String> cache = new SimpleCache<>(new SingleNodeGroup());
+    cache.setSnapshot(SingleNodeGroup.GROUP, Snapshot.create(
+        ImmutableList.of(), ImmutableList.of(), ImmutableList.of(), ImmutableList.of(), ImmutableList.of(), VERSION1));
+
+    ResponseTracker responseTracker = new ResponseTracker();
+    Watch watch = cache.createWatch(
+        true,
+        XdsRequest.create(DiscoveryRequest.newBuilder()
+            .setNode(Node.getDefaultInstance())
+            .setTypeUrl(ROUTE_TYPE_URL)
+            .addAllResourceNames(Collections.singleton(ROUTE_NAME))
+            .build()),
+        Collections.singleton(ROUTE_NAME),
+        responseTracker);
+
+    assertThatWatchIsOpenWithNoResponses(new WatchAndTracker(watch, responseTracker));
+    assertThat(cache.statusInfo(SingleNodeGroup.GROUP).numWatches()).isEqualTo(1);
+
+    cache.setSnapshot(SingleNodeGroup.GROUP, Snapshot.create(
+        ImmutableList.of(), ImmutableList.of(), ImmutableList.of(), ImmutableList.of(), ImmutableList.of(), VERSION2));
+
+    assertThatWatchIsOpenWithNoResponses(new WatchAndTracker(watch, responseTracker));
+    assertThat(cache.statusInfo(SingleNodeGroup.GROUP).numWatches()).isEqualTo(1);
+  }
+
+  @Test
+  public void watchIsLeftOpenIfNotRespondedImmediatelyAndLaterSetSnapshotSendsUpdate() {
+    SimpleCache<String> cache = new SimpleCache<>(new SingleNodeGroup());
+    cache.setSnapshot(SingleNodeGroup.GROUP, Snapshot.create(
+        ImmutableList.of(), ImmutableList.of(), ImmutableList.of(), ImmutableList.of(), ImmutableList.of(), VERSION1));
+
+    ResponseTracker responseTracker = new ResponseTracker();
+    Watch watch = cache.createWatch(
+        true,
+        XdsRequest.create(DiscoveryRequest.newBuilder()
+            .setNode(Node.getDefaultInstance())
+            .setTypeUrl(ROUTE_TYPE_URL)
+            .addAllResourceNames(SNAPSHOT1.resources(ROUTE_TYPE_URL).keySet())
+            .build()),
+        Collections.emptySet(),
+        responseTracker);
+
+    assertThatWatchIsOpenWithNoResponses(new WatchAndTracker(watch, responseTracker));
+    assertThat(cache.statusInfo(SingleNodeGroup.GROUP).numWatches()).isEqualTo(1);
+
+    cache.setSnapshot(SingleNodeGroup.GROUP, Snapshot.create(
+        ImmutableList.of(), ImmutableList.of(), ImmutableList.of(), ImmutableList.of(), ImmutableList.of(), VERSION2));
+
+    assertThatWatchIsOpenWithNoResponses(new WatchAndTracker(watch, responseTracker));
+    assertThat(cache.statusInfo(SingleNodeGroup.GROUP).numWatches()).isEqualTo(1);
+
+    cache.setSnapshot(SingleNodeGroup.GROUP, SNAPSHOT1);
+
+    assertThatWatchReceivesSnapshot(new WatchAndTracker(watch, responseTracker), SNAPSHOT1);
+  }
+
   @Test
   public void getSnapshot() {
     SimpleCache<String> cache = new SimpleCache<>(new SingleNodeGroup());