From: Dave Watson <davejwatson@fb.com>
Date: Tue, 19 May 2015 13:43:14 +0000 (-0700)
Subject: framing handler pipeline stage
X-Git-Tag: v0.39.0~7
X-Git-Url: http://demsky.eecs.uci.edu/git/?a=commitdiff_plain;h=1e2aee3a27a694e4015a23b4c30a00e29a6ed7ce;p=folly.git

framing handler pipeline stage

Summary: last pipeline in the original diff (HeaderServer/Client channel can also become pipeline stages in the future...)

Test Plan:
fbconfig -r thrift/lib/cpp2; fbmake runtests

canary results will follow in a comment

Reviewed By: alandau@fb.com

Subscribers: fugalh, folly-diffs@, chalfant, trunkagent, doug, alandau, bmatheny, jsedgwick, yfeldblum

FB internal diff: D2033559

Signature: t1:2033559:1430417432:c6cf4ccbf9ef26d89e7d7c5955d103348205b365
---

diff --git a/folly/Optional.h b/folly/Optional.h
index ee94228e..f4bc9ff4 100644
--- a/folly/Optional.h
+++ b/folly/Optional.h
@@ -87,6 +87,8 @@ class Optional {
  public:
   static_assert(!std::is_reference<Value>::value,
                 "Optional may not be used with reference types");
+  static_assert(!std::is_abstract<Value>::value,
+                "Optional may not be used with abstract types");
 
   Optional()
     : hasValue_(false) {
diff --git a/folly/wangle/channel/StaticPipeline.h b/folly/wangle/channel/StaticPipeline.h
index 68e0d906..a5d2e893 100644
--- a/folly/wangle/channel/StaticPipeline.h
+++ b/folly/wangle/channel/StaticPipeline.h
@@ -16,6 +16,8 @@
 
 #pragma once
 
+#include <type_traits>
+
 #include <folly/wangle/channel/Pipeline.h>
 
 namespace folly { namespace wangle {
@@ -50,9 +52,22 @@ class StaticPipeline<R, W> : public Pipeline<R, W> {
   explicit StaticPipeline(bool) : Pipeline<R, W>(true) {}
 };
 
+template <class Handler>
+class BaseWithOptional {
+ protected:
+  folly::Optional<Handler> handler_;
+};
+
+template <class Handler>
+class BaseWithoutOptional {
+};
+
 template <class R, class W, class Handler, class... Handlers>
 class StaticPipeline<R, W, Handler, Handlers...>
-    : public StaticPipeline<R, W, Handlers...> {
+    : public StaticPipeline<R, W, Handlers...>
+    , public std::conditional<std::is_abstract<Handler>::value,
+                              BaseWithoutOptional<Handler>,
+                              BaseWithOptional<Handler>>::type {
  public:
   template <class... HandlerArgs>
   explicit StaticPipeline(HandlerArgs&&... handlers)
@@ -92,8 +107,8 @@ class StaticPipeline<R, W, Handler, Handlers...>
     Handler
   >::value>::type
   setHandler(HandlerArg&& arg) {
-    handler_.emplace(std::forward<HandlerArg>(arg));
-    handlerPtr_ = std::shared_ptr<Handler>(&(*handler_), [](Handler*){});
+    BaseWithOptional<Handler>::handler_.emplace(std::forward<HandlerArg>(arg));
+    handlerPtr_ = std::shared_ptr<Handler>(&(*BaseWithOptional<Handler>::handler_), [](Handler*){});
   }
 
   template <class HandlerArg>
@@ -115,7 +130,6 @@ class StaticPipeline<R, W, Handler, Handlers...>
   }
 
   bool isFirst_;
-  folly::Optional<Handler> handler_;
   std::shared_ptr<Handler> handlerPtr_;
   typename ContextType<Handler, Pipeline<R, W>>::type ctx_;
 };
diff --git a/folly/wangle/channel/test/PipelineTest.cpp b/folly/wangle/channel/test/PipelineTest.cpp
index 9c26bd8f..cdc4e980 100644
--- a/folly/wangle/channel/test/PipelineTest.cpp
+++ b/folly/wangle/channel/test/PipelineTest.cpp
@@ -28,6 +28,7 @@ using namespace folly::wangle;
 using namespace testing;
 
 typedef StrictMock<MockHandlerAdapter<int, int>> IntHandler;
+class IntHandler2 : public StrictMock<MockHandlerAdapter<int, int>> {};
 
 ACTION(FireRead) {
   arg0->fireRead(arg1);
@@ -77,7 +78,7 @@ TEST(PipelineTest, RealHandlersCompile) {
 // Test that handlers correctly fire the next handler when directed
 TEST(PipelineTest, FireActions) {
   IntHandler handler1;
-  IntHandler handler2;
+  IntHandler2 handler2;
 
   {
     InSequence sequence;
@@ -85,7 +86,7 @@ TEST(PipelineTest, FireActions) {
     EXPECT_CALL(handler1, attachPipeline(_));
   }
 
-  StaticPipeline<int, int, IntHandler, IntHandler>
+  StaticPipeline<int, int, IntHandler, IntHandler2>
   pipeline(&handler1, &handler2);
 
   EXPECT_CALL(handler1, read_(_, _)).WillOnce(FireRead());
@@ -144,7 +145,7 @@ TEST(PipelineTest, ReachEndOfPipeline) {
 // Test having the last read handler turn around and write
 TEST(PipelineTest, TurnAround) {
   IntHandler handler1;
-  IntHandler handler2;
+  IntHandler2 handler2;
 
   {
     InSequence sequence;
@@ -152,7 +153,7 @@ TEST(PipelineTest, TurnAround) {
     EXPECT_CALL(handler1, attachPipeline(_));
   }
 
-  StaticPipeline<int, int, IntHandler, IntHandler>
+  StaticPipeline<int, int, IntHandler, IntHandler2>
   pipeline(&handler1, &handler2);
 
   EXPECT_CALL(handler1, read_(_, _)).WillOnce(FireRead());
@@ -243,11 +244,14 @@ TEST(PipelineTest, HandlerInMultiplePipelines) {
 }
 
 TEST(PipelineTest, HandlerInPipelineTwice) {
-  IntHandler handler;
-  EXPECT_CALL(handler, attachPipeline(_)).Times(2);
-  StaticPipeline<int, int, IntHandler, IntHandler> pipeline(&handler, &handler);
-  EXPECT_FALSE(handler.getContext());
-  EXPECT_CALL(handler, detachPipeline(_)).Times(2);
+  auto handler = std::make_shared<IntHandler>();
+  EXPECT_CALL(*handler, attachPipeline(_)).Times(2);
+  Pipeline<int, int> pipeline;
+  pipeline.addBack(handler);
+  pipeline.addBack(handler);
+  pipeline.finalize();
+  EXPECT_FALSE(handler->getContext());
+  EXPECT_CALL(*handler, detachPipeline(_)).Times(2);
 }
 
 TEST(PipelineTest, NoDetachOnOwner) {
@@ -285,8 +289,9 @@ TEST(Pipeline, MissingInboundOrOutbound) {
 
 TEST(Pipeline, DynamicConstruction) {
   {
-    StaticPipeline<std::string, std::string, StringHandler, StringHandler>
-    pipeline{StringHandler(), StringHandler()};
+    Pipeline<std::string, std::string> pipeline;
+    pipeline.addBack(StringHandler());
+    pipeline.addBack(StringHandler());
 
     // Exercise both addFront and addBack. Final pipeline is
     // StI <-> ItS <-> StS <-> StS <-> StI <-> ItS