proper fix for sending the frame too early (#104)

Author: yschimke

src/ConnectionAutomaton.cpp

@@ -46,7 +46,7 @@ void ConnectionAutomaton::connect() {
         ? keepaliveTimer_->keepaliveTime().count()
         : std::numeric_limits<uint32_t>::max();
 
-    // TODO set correct version
+    // TODO set correct version and allow clients to configure
     Frame_SETUP frame(
         FrameFlags_EMPTY,
         0,
@@ -55,7 +55,7 @@ void ConnectionAutomaton::connect() {
         "",
         "",
         Payload());
-    connectionOutput_.onNext(frame.serializeOut());
+    onNextFrame(frame.serializeOut());
   }
   stats_.socketCreated();
 

test/InlineConnection.cpp

@@ -21,17 +21,11 @@ InlineConnection::InlineConnection()
 InlineConnection::~InlineConnection() {}
 
 void InlineConnection::connectTo(
-    InlineConnection& other,
-    bool expectSetupFrame) {
+    InlineConnection& other) {
   ASSERT_FALSE(other_);
   ASSERT_FALSE(other.other_);
   other.other_ = this;
   other_ = &other;
-  // this could just be true for the client, false for the server.  However
-  // InlineConnection is below the
-  // ReactiveSocket layer so any tests that just use DuplexConnection may break
-  // this
-  expectSetupFrame_ = expectSetupFrame;
 }
 
 void InlineConnection::setInput(
@@ -86,18 +80,6 @@ Subscriber<std::unique_ptr<folly::IOBuf>>& InlineConnection::getOutput() {
           inputSink->onSubscribe(*outputSubscription_);
         }
       }));
-  // SETUP
-  if (expectSetupFrame_) {
-    EXPECT_CALL(outputSink, onNext_(_))
-        .Times(1)
-        .InSequence(s)
-        .WillRepeatedly(Invoke([this](std::unique_ptr<folly::IOBuf>& frame) {
-          ASSERT_TRUE(other_);
-          ASSERT_FALSE(other_->outputSubscription_);
-          auto inputSink = other_->inputSink_;
-          ASSERT_FALSE(inputSink);
-        }));
-  }
   EXPECT_CALL(outputSink, onNext_(_))
       .Times(AnyNumber())
       .InSequence(s)

test/InlineConnection.h

@@ -35,7 +35,7 @@ class InlineConnection : public DuplexConnection {
   /// This method may be invoked at most once per lifetime of the object and
   /// implicitly connects the `other` to this instance. Must be invoked before
   /// accessing input or output of the connection.
-  void connectTo(InlineConnection& other, bool expectSetupFrame);
+  void connectTo(InlineConnection& other);
 
   void setInput(Subscriber<std::unique_ptr<folly::IOBuf>>& inputSink) override;
 
@@ -52,6 +52,5 @@ class InlineConnection : public DuplexConnection {
   folly::exception_wrapper inputSinkError_;
   /// @}
   Subscription* outputSubscription_;
-  bool expectSetupFrame_{false};
 };
 }

test/InlineConnectionTest.cpp

@@ -18,7 +18,7 @@ TEST(InlineConnectionTest, PingPong) {
   // calls will be deterministic.
   Sequence s;
   std::array<InlineConnection, 2> end;
-  end[0].connectTo(end[1], false);
+  end[0].connectTo(end[1]);
 
   std::array<UnmanagedMockSubscriber<std::unique_ptr<folly::IOBuf>>, 2> input;
   std::array<UnmanagedMockSubscription, 2> outputSub;

test/ReactiveSocketTest.cpp

@@ -40,7 +40,7 @@ TEST(ReactiveSocketTest, RequestChannel) {
 
   auto clientConn = folly::make_unique<InlineConnection>();
   auto serverConn = folly::make_unique<InlineConnection>();
-  clientConn->connectTo(*serverConn, true);
+  clientConn->connectTo(*serverConn);
 
   StrictMock<UnmanagedMockSubscriber<Payload>> clientInput, serverInput;
   StrictMock<UnmanagedMockSubscription> clientOutputSub, serverOutputSub;
@@ -148,7 +148,7 @@ TEST(ReactiveSocketTest, RequestStreamComplete) {
 
   auto clientConn = folly::make_unique<InlineConnection>();
   auto serverConn = folly::make_unique<InlineConnection>();
-  clientConn->connectTo(*serverConn, true);
+  clientConn->connectTo(*serverConn);
 
   StrictMock<UnmanagedMockSubscriber<Payload>> clientInput;
   StrictMock<UnmanagedMockSubscription> serverOutputSub;
@@ -227,7 +227,7 @@ TEST(ReactiveSocketTest, RequestStreamCancel) {
 
   auto clientConn = folly::make_unique<InlineConnection>();
   auto serverConn = folly::make_unique<InlineConnection>();
-  clientConn->connectTo(*serverConn, true);
+  clientConn->connectTo(*serverConn);
 
   StrictMock<UnmanagedMockSubscriber<Payload>> clientInput;
   StrictMock<UnmanagedMockSubscription> serverOutputSub;
@@ -303,7 +303,7 @@ TEST(ReactiveSocketTest, RequestSubscription) {
 
   auto clientConn = folly::make_unique<InlineConnection>();
   auto serverConn = folly::make_unique<InlineConnection>();
-  clientConn->connectTo(*serverConn, true);
+  clientConn->connectTo(*serverConn);
 
   StrictMock<UnmanagedMockSubscriber<Payload>> clientInput;
   StrictMock<UnmanagedMockSubscription> serverOutputSub;
@@ -380,7 +380,7 @@ TEST(ReactiveSocketTest, RequestFireAndForget) {
 
   auto clientConn = folly::make_unique<InlineConnection>();
   auto serverConn = folly::make_unique<InlineConnection>();
-  clientConn->connectTo(*serverConn, true);
+  clientConn->connectTo(*serverConn);
 
   StrictMock<UnmanagedMockSubscriber<Payload>> clientInput;
   StrictMock<UnmanagedMockSubscription> serverOutputSub;
@@ -412,7 +412,7 @@ TEST(ReactiveSocketTest, RequestMetadataPush) {
 
   auto clientConn = folly::make_unique<InlineConnection>();
   auto serverConn = folly::make_unique<InlineConnection>();
-  clientConn->connectTo(*serverConn, true);
+  clientConn->connectTo(*serverConn);
 
   StrictMock<UnmanagedMockSubscriber<Payload>> clientInput;
   StrictMock<UnmanagedMockSubscription> serverOutputSub;
@@ -443,7 +443,7 @@ TEST(ReactiveSocketTest, Destructor) {
 
   auto clientConn = folly::make_unique<InlineConnection>();
   auto serverConn = folly::make_unique<InlineConnection>();
-  clientConn->connectTo(*serverConn, true);
+  clientConn->connectTo(*serverConn);
 
   // TODO: since we don't assert anything, should we just use the StatsPrinter
   // instead?