server: return processContext as optional reference (envoyproxy#8238)

Signed-off-by: Elisha Ziskind <[email protected]>
danzh2010 · Sep 24, 2019 · c2df144 · c2df144
1 parent bdc4703
commit c2df144
Show file tree

Hide file tree

Showing 8 changed files with 18 additions and 11 deletions.
diff --git a/include/envoy/server/filter_config.h b/include/envoy/server/filter_config.h
@@ -180,9 +180,10 @@ class FactoryContext : public virtual CommonFactoryContext {
   virtual Grpc::Context& grpcContext() PURE;
 
   /**
-   * @return ProcessContext& a reference to the process context.
+   * @return absl::optional<std::reference_wrapper<ProcessContext>> an optional reference to the
+   * process context. Will be unset when running in validation mode.
    */
-  virtual ProcessContext& processContext() PURE;
+  virtual absl::optional<std::reference_wrapper<ProcessContext>> processContext() PURE;
 };
 
 class ListenerFactoryContext : public virtual FactoryContext {

diff --git a/include/envoy/server/instance.h b/include/envoy/server/instance.h
@@ -196,7 +196,7 @@ class Instance {
   /**
    * @return the server-wide process context.
    */
-  virtual ProcessContext& processContext() PURE;
+  virtual absl::optional<std::reference_wrapper<ProcessContext>> processContext() PURE;
 
   /**
    * @return ThreadLocal::Instance& the thread local storage engine for the server. This is used to

diff --git a/source/server/config_validation/server.h b/source/server/config_validation/server.h
@@ -94,7 +94,9 @@ class ValidationInstance : Logger::Loggable<Logger::Id::main>,
   Stats::Store& stats() override { return stats_store_; }
   Grpc::Context& grpcContext() override { return grpc_context_; }
   Http::Context& httpContext() override { return http_context_; }
-  ProcessContext& processContext() override { NOT_IMPLEMENTED_GCOVR_EXCL_LINE; }
+  absl::optional<std::reference_wrapper<ProcessContext>> processContext() override {
+    return absl::nullopt;
+  }
   ThreadLocal::Instance& threadLocal() override { return thread_local_; }
   const LocalInfo::LocalInfo& localInfo() override { return *local_info_; }
   TimeSource& timeSource() override { return api_->timeSource(); }

diff --git a/source/server/listener_manager_impl.h b/source/server/listener_manager_impl.h
@@ -324,7 +324,9 @@ class ListenerImpl : public Network::ListenerConfig,
   ServerLifecycleNotifier& lifecycleNotifier() override {
     return parent_.server_.lifecycleNotifier();
   }
-  ProcessContext& processContext() override { return parent_.server_.processContext(); }
+  absl::optional<std::reference_wrapper<ProcessContext>> processContext() override {
+    return parent_.server_.processContext();
+  }
 
   // Network::DrainDecision
   bool drainClose() const override;

diff --git a/source/server/server.h b/source/server/server.h
@@ -194,7 +194,9 @@ class InstanceImpl : Logger::Loggable<Logger::Id::main>,
   Stats::Store& stats() override { return stats_store_; }
   Grpc::Context& grpcContext() override { return grpc_context_; }
   Http::Context& httpContext() override { return http_context_; }
-  ProcessContext& processContext() override { return *process_context_; }
+  absl::optional<std::reference_wrapper<ProcessContext>> processContext() override {
+    return *process_context_;
+  }
   ThreadLocal::Instance& threadLocal() override { return thread_local_; }
   const LocalInfo::LocalInfo& localInfo() override { return *local_info_; }
   TimeSource& timeSource() override { return time_source_; }

diff --git a/test/integration/filters/process_context_filter.cc b/test/integration/filters/process_context_filter.cc
@@ -44,7 +44,7 @@ class ProcessContextFilterConfig : public Extensions::HttpFilters::Common::Empty
                Server::Configuration::FactoryContext& factory_context) override {
     return [&factory_context](Http::FilterChainFactoryCallbacks& callbacks) {
       callbacks.addStreamFilter(
-          std::make_shared<ProcessContextFilter>(factory_context.processContext()));
+          std::make_shared<ProcessContextFilter>(*factory_context.processContext()));
     };
   }
 };

diff --git a/test/mocks/server/mocks.h b/test/mocks/server/mocks.h
@@ -382,7 +382,7 @@ class MockInstance : public Instance {
   MOCK_METHOD0(stats, Stats::Store&());
   MOCK_METHOD0(grpcContext, Grpc::Context&());
   MOCK_METHOD0(httpContext, Http::Context&());
-  MOCK_METHOD0(processContext, ProcessContext&());
+  MOCK_METHOD0(processContext, absl::optional<std::reference_wrapper<ProcessContext>>());
   MOCK_METHOD0(threadLocal, ThreadLocal::Instance&());
   MOCK_METHOD0(localInfo, const LocalInfo::LocalInfo&());
   MOCK_CONST_METHOD0(statsFlushInterval, std::chrono::milliseconds());
@@ -470,7 +470,7 @@ class MockFactoryContext : public virtual FactoryContext {
   Event::TestTimeSystem& timeSystem() { return time_system_; }
   Grpc::Context& grpcContext() override { return grpc_context_; }
   Http::Context& httpContext() override { return http_context_; }
-  MOCK_METHOD0(processContext, ProcessContext&());
+  MOCK_METHOD0(processContext, absl::optional<std::reference_wrapper<ProcessContext>>());
   MOCK_METHOD0(messageValidationVisitor, ProtobufMessage::ValidationVisitor&());
   MOCK_METHOD0(api, Api::Api&());
 

diff --git a/test/server/server_test.cc b/test/server/server_test.cc
@@ -853,8 +853,8 @@ TEST_P(ServerInstanceImplTest, WithProcessContext) {
 
   EXPECT_NO_THROW(initialize("test/server/empty_bootstrap.yaml"));
 
-  ProcessContext& context = server_->processContext();
-  auto& object_from_context = dynamic_cast<TestObject&>(context.get());
+  auto context = server_->processContext();
+  auto& object_from_context = dynamic_cast<TestObject&>(context->get().get());
   EXPECT_EQ(&object_from_context, &object);
   EXPECT_TRUE(object_from_context.boolean_flag_);