actually cache routes in the connection manager (#453)

#389 did not actually cache them
between filters which was the point of the previous change.

source/common/http/conn_manager_impl.cc

@@ -819,12 +819,12 @@ AccessLog::RequestInfo& ConnectionManagerImpl::ActiveStreamFilterBase::requestIn
 }
 
 Router::RoutePtr ConnectionManagerImpl::ActiveStreamFilterBase::route() {
-  if (!cached_route_.valid()) {
-    cached_route_.value(parent_.connection_manager_.config_.routeConfig().route(
+  if (!parent_.cached_route_.valid()) {
+    parent_.cached_route_.value(parent_.connection_manager_.config_.routeConfig().route(
         *parent_.request_headers_, parent_.stream_id_));
   }
 
-  return cached_route_.value();
+  return parent_.cached_route_.value();
 }
 
 void ConnectionManagerImpl::ActiveStreamDecoderFilter::continueDecoding() { commonContinue(); }

source/common/http/conn_manager_impl.h

@@ -273,7 +273,6 @@ class ConnectionManagerImpl : Logger::Loggable<Logger::Id::http>,
     ActiveStream& parent_;
     bool headers_continued_{};
     bool stopped_{};
-    Optional<Router::RoutePtr> cached_route_;
   };
 
   /**
@@ -414,6 +413,7 @@ class ConnectionManagerImpl : Logger::Loggable<Logger::Id::http>,
     State state_;
     AccessLog::RequestInfoImpl request_info_;
     std::string downstream_address_;
+    Optional<Router::RoutePtr> cached_route_;
   };
 
   typedef std::unique_ptr<ActiveStream> ActiveStreamPtr;

test/common/http/conn_manager_impl_test.cc

@@ -25,6 +25,7 @@
 using testing::_;
 using testing::InSequence;
 using testing::Invoke;
+using testing::InvokeWithoutArgs;
 using testing::NiceMock;
 using testing::Return;
 using testing::ReturnRef;
@@ -123,7 +124,6 @@ TEST_F(HttpConnectionManagerImplTest, HeaderOnlyRequestAndResponse) {
       new NiceMock<Http::MockStreamDecoderFilter>());
 
   EXPECT_CALL(filter->reset_stream_called_, ready()).Times(0);
-  EXPECT_CALL(route_config_, route(_, _)).Times(2);
   EXPECT_CALL(*filter, decodeHeaders(_, true))
       .Times(2)
       .WillRepeatedly(Invoke([&](HeaderMap& headers, bool) -> FilterHeadersStatus {
@@ -133,10 +133,6 @@ TEST_F(HttpConnectionManagerImplTest, HeaderOnlyRequestAndResponse) {
           filter->callbacks_->requestInfo().healthCheck(true);
         }
 
-        // Test route caching.
-        EXPECT_EQ(route_config_.route_, filter->callbacks_->route());
-        EXPECT_EQ(route_config_.route_, filter->callbacks_->route());
-
         return FilterHeadersStatus::StopIteration;
       }));
 
@@ -817,8 +813,15 @@ TEST_F(HttpConnectionManagerImplTest, MultipleFilters) {
         callbacks.addStreamEncoderFilter(Http::StreamEncoderFilterPtr{encoder_filter2});
       }));
 
+  // Test route caching.
+  EXPECT_CALL(route_config_, route(_, _));
+
   EXPECT_CALL(*decoder_filter1, decodeHeaders(_, false))
-      .WillOnce(Return(Http::FilterHeadersStatus::StopIteration));
+      .WillOnce(InvokeWithoutArgs([&]() -> Http::FilterHeadersStatus {
+        EXPECT_EQ(route_config_.route_, decoder_filter1->callbacks_->route());
+        return Http::FilterHeadersStatus::StopIteration;
+      }));
+
   EXPECT_CALL(*decoder_filter1, decodeData(_, false))
       .WillOnce(Return(Http::FilterDataStatus::StopIterationAndBuffer));
   EXPECT_CALL(*decoder_filter1, decodeData(_, true))
@@ -846,7 +849,10 @@ TEST_F(HttpConnectionManagerImplTest, MultipleFilters) {
   // Mimic a decoder filter that trapped data and now sends it on, since the data was buffered
   // by the first filter, we expect to get it in 1 decodeData() call.
   EXPECT_CALL(*decoder_filter2, decodeHeaders(_, false))
-      .WillOnce(Return(Http::FilterHeadersStatus::Continue));
+      .WillOnce(InvokeWithoutArgs([&]() -> Http::FilterHeadersStatus {
+        EXPECT_EQ(route_config_.route_, decoder_filter2->callbacks_->route());
+        return Http::FilterHeadersStatus::StopIteration;
+      }));
   EXPECT_CALL(*decoder_filter2, decodeData(_, true))
       .WillOnce(Return(Http::FilterDataStatus::Continue));
   EXPECT_CALL(*decoder_filter3, decodeHeaders(_, false))