From a5ac38df8718c501414fc3bdb67cdcc8feb74d86 Mon Sep 17 00:00:00 2001
From: Rama <rama.rao@salesforce.com>
Date: Fri, 21 Sep 2018 07:53:59 +0530
Subject: [PATCH] revert empty stat update fix

Signed-off-by: Rama <rama.rao@salesforce.com>
---
 source/common/config/grpc_mux_impl.cc    |  9 +----
 test/common/config/grpc_mux_impl_test.cc | 51 ++----------------------
 2 files changed, 4 insertions(+), 56 deletions(-)

diff --git a/source/common/config/grpc_mux_impl.cc b/source/common/config/grpc_mux_impl.cc
index 4303b4bf85a46..991fe4feebbb3 100644
--- a/source/common/config/grpc_mux_impl.cc
+++ b/source/common/config/grpc_mux_impl.cc
@@ -204,9 +204,6 @@ void GrpcMuxImpl::onReceiveMessage(std::unique_ptr<envoy::api::v2::DiscoveryResp
       resources.emplace(resource_name, resource);
     }
     for (auto watch : api_state_[type_url].watches_) {
-      // onConfigUpdate should be called in all cases for single watch xDS (Cluster and Listener)
-      // even if the message does not have resources so that update_empty stat is properly
-      // incremented and state-of-the-world semantics are maintained.
       if (watch->resources_.empty()) {
         watch->callbacks_.onConfigUpdate(message->resources(), message->version_info());
         continue;
@@ -218,11 +215,7 @@ void GrpcMuxImpl::onReceiveMessage(std::unique_ptr<envoy::api::v2::DiscoveryResp
           found_resources.Add()->MergeFrom(it->second);
         }
       }
-      // onConfigUpdate should be called only on watches(clusters/routes) that have updates in the
-      // message.
-      if (found_resources.size() > 0) {
-        watch->callbacks_.onConfigUpdate(found_resources, message->version_info());
-      }
+      watch->callbacks_.onConfigUpdate(found_resources, message->version_info());
     }
     // TODO(mattklein123): In the future if we start tracking per-resource versions, we would do
     // that tracking here.
diff --git a/test/common/config/grpc_mux_impl_test.cc b/test/common/config/grpc_mux_impl_test.cc
index 1a4edbd15994e..1fd178d0271e1 100644
--- a/test/common/config/grpc_mux_impl_test.cc
+++ b/test/common/config/grpc_mux_impl_test.cc
@@ -246,7 +246,9 @@ TEST_F(GrpcMuxImplTest, WatchDemux) {
     envoy::api::v2::ClusterLoadAssignment load_assignment;
     load_assignment.set_cluster_name("x");
     response->add_resources()->PackFrom(load_assignment);
-    EXPECT_CALL(bar_callbacks, onConfigUpdate(_, "1")).Times(0);
+    EXPECT_CALL(bar_callbacks, onConfigUpdate(_, "1"))
+        .WillOnce(Invoke([](const Protobuf::RepeatedPtrField<ProtobufWkt::Any>& resources,
+                            const std::string&) { EXPECT_TRUE(resources.empty()); }));
     EXPECT_CALL(foo_callbacks, onConfigUpdate(_, "1"))
         .WillOnce(
             Invoke([&load_assignment](const Protobuf::RepeatedPtrField<ProtobufWkt::Any>& resources,
@@ -304,53 +306,6 @@ TEST_F(GrpcMuxImplTest, WatchDemux) {
   expectSendMessage(type_url, {}, "2");
 }
 
-// Validate behavior when we have multiple watchers that send empty updates.
-TEST_F(GrpcMuxImplTest, MultipleWatcherWithEmptyUpdates) {
-  setup();
-  InSequence s;
-  const std::string& type_url = Config::TypeUrl::get().ClusterLoadAssignment;
-  NiceMock<MockGrpcMuxCallbacks> foo_callbacks;
-  auto foo_sub = grpc_mux_->subscribe(type_url, {"x", "y"}, foo_callbacks);
-
-  EXPECT_CALL(*async_client_, start(_, _)).WillOnce(Return(&async_stream_));
-  expectSendMessage(type_url, {"x", "y"}, "");
-  grpc_mux_->start();
-
-  std::unique_ptr<envoy::api::v2::DiscoveryResponse> response(
-      new envoy::api::v2::DiscoveryResponse());
-  response->set_type_url(type_url);
-  response->set_version_info("1");
-
-  EXPECT_CALL(foo_callbacks, onConfigUpdate(_, "1")).Times(0);
-  expectSendMessage(type_url, {"x", "y"}, "1");
-  grpc_mux_->onReceiveMessage(std::move(response));
-
-  expectSendMessage(type_url, {}, "1");
-}
-
-// Validate behavior when we have Single Watcher that sends Empty updates.
-TEST_F(GrpcMuxImplTest, SingleWatcherWithEmptyUpdates) {
-  setup();
-  const std::string& type_url = Config::TypeUrl::get().Cluster;
-  NiceMock<MockGrpcMuxCallbacks> foo_callbacks;
-  auto foo_sub = grpc_mux_->subscribe(type_url, {}, foo_callbacks);
-
-  EXPECT_CALL(*async_client_, start(_, _)).WillOnce(Return(&async_stream_));
-  expectSendMessage(type_url, {}, "");
-  grpc_mux_->start();
-
-  std::unique_ptr<envoy::api::v2::DiscoveryResponse> response(
-      new envoy::api::v2::DiscoveryResponse());
-  response->set_type_url(type_url);
-  response->set_version_info("1");
-  // Validate that onConfigUpdate is called with empty resources.
-  EXPECT_CALL(foo_callbacks, onConfigUpdate(_, "1"))
-      .WillOnce(Invoke([](const Protobuf::RepeatedPtrField<ProtobufWkt::Any>& resources,
-                          const std::string&) { EXPECT_TRUE(resources.empty()); }));
-  expectSendMessage(type_url, {}, "1");
-  grpc_mux_->onReceiveMessage(std::move(response));
-}
-
 //  Verifies that warning messages get logged when Envoy detects too many requests.
 TEST_F(GrpcMuxImplTest, TooManyRequests) {
   setup();