Refactor writeChain function signature not to reutrn IOBuf

Summary: Updating signature of writeChain to stop returning IOBuf, as implementation never actually returns back buffer and always writes full buffer.

Reviewed By: mjoras

Differential Revision: D21740607

fbshipit-source-id: f473ed8f3c6c6cbe2dd5db8f1247c912f3e77d0b

quic/api/QuicSocket.h

@@ -798,15 +798,9 @@ class QuicSocket {
    * transport when the peer has acknowledged the receipt of all the data/eof
    * passed to write.
    *
-   * A returned IOBuf indicates that the passed data exceeded the transport
-   * flow control window or send buffer space.  The application must call write
-   * with this data again later, and should be a signal to apply backpressure.
-   * If EOF was true or a delivery callback was set they also need to be
-   * passed again later.  See notifyPendingWrite to register for a callback.
-   *
    * An error code is present if there was an error with the write.
    */
-  using WriteResult = folly::Expected<Buf, LocalErrorCode>;
+  using WriteResult = folly::Expected<folly::Unit, LocalErrorCode>;
   virtual WriteResult writeChain(
       StreamId id,
       Buf data,

quic/api/QuicTransportBase.cpp

@@ -1899,7 +1899,7 @@ QuicSocket::WriteResult QuicTransportBase::writeChain(
         std::string("writeChain() error")));
     return folly::makeUnexpected(LocalErrorCode::INTERNAL_ERROR);
   }
-  return nullptr;
+  return folly::unit;
 }
 
 folly::Expected<folly::Unit, LocalErrorCode>

quic/api/test/MockQuicSocket.h

@@ -140,21 +140,15 @@ class MockQuicSocket : public QuicSocket {
   MOCK_METHOD1(
       unregisterStreamWriteCallback,
       folly::Expected<folly::Unit, LocalErrorCode>(StreamId));
-  folly::Expected<Buf, LocalErrorCode> writeChain(
+  folly::Expected<folly::Unit, LocalErrorCode> writeChain(
       StreamId id,
       Buf data,
       bool eof,
       bool cork,
       DeliveryCallback* cb) override {
     SharedBuf sharedData(data.release());
-    auto res = writeChain(id, sharedData, eof, cork, cb);
-    if (res.hasError()) {
-      return folly::makeUnexpected(res.error());
-    } else {
-      return Buf(res.value());
+    return writeChain(id, sharedData, eof, cork, cb);
   }
-  }
-  using WriteResult = folly::Expected<folly::IOBuf*, LocalErrorCode>;
   MOCK_METHOD5(
       writeChain,
       WriteResult(StreamId, SharedBuf, bool, bool, DeliveryCallback*));

quic/api/test/QuicTransportBaseTest.cpp

@@ -1043,7 +1043,7 @@ TEST_F(QuicTransportImplTest, ConnectionErrorOnWrite) {
   auto stream = transport->createBidirectionalStream().value();
   EXPECT_CALL(*socketPtr, write(_, _))
       .WillOnce(SetErrnoAndReturn(ENETUNREACH, -1));
-  QuicSocket::WriteResult result = transport->writeChain(
+  transport->writeChain(
       stream, folly::IOBuf::copyBuffer("Hey"), true, false, nullptr);
   transport->addDataToStream(
       stream, StreamBuffer(folly::IOBuf::copyBuffer("Data"), 0));
@@ -1074,7 +1074,7 @@ TEST_F(QuicTransportImplTest, ReadErrorUnsanitizedErrorMsg) {
     throw std::runtime_error("You need to calm down.");
     return 0;
   }));
-  QuicSocket::WriteResult result = transport->writeChain(
+  transport->writeChain(
       stream,
       folly::IOBuf::copyBuffer("You are being too loud."),
       true,
@@ -1097,7 +1097,7 @@ TEST_F(QuicTransportImplTest, ConnectionErrorUnhandledException) {
     throw std::runtime_error("Well there's your problem");
     return 0;
   }));
-  QuicSocket::WriteResult result = transport->writeChain(
+  transport->writeChain(
       stream, folly::IOBuf::copyBuffer("Hey"), true, false, nullptr);
   transport->addDataToStream(
       stream, StreamBuffer(folly::IOBuf::copyBuffer("Data"), 0));

quic/samples/echo/EchoClient.h

@@ -169,11 +169,6 @@ class EchoClient : public quic::QuicSocket::ConnectionCallback,
     auto res = quicClient_->writeChain(id, message->clone(), true, false);
     if (res.hasError()) {
       LOG(ERROR) << "EchoClient writeChain error=" << uint32_t(res.error());
-    } else if (res.value()) {
-      LOG(INFO) << "EchoClient socket did not accept all data, buffering len="
-                << res.value()->computeChainDataLength();
-      data.append(std::move(res.value()));
-      quicClient_->notifyPendingWriteOnStream(id, this);
     } else {
       auto str = message->moveToFbString().toStdString();
       LOG(INFO) << "EchoClient wrote \"" << str << "\""

quic/samples/echo/EchoHandler.h

@@ -105,11 +105,6 @@ class EchoHandler : public quic::QuicSocket::ConnectionCallback,
         sock->writeChain(id, std::move(echoedData), true, false, nullptr);
     if (res.hasError()) {
       LOG(ERROR) << "write error=" << toString(res.error());
-    } else if (res.value()) {
-      LOG(INFO) << "socket did not accept all data, buffering len="
-                << res.value()->computeChainDataLength();
-      data.first.append(std::move(res.value()));
-      sock->notifyPendingWriteOnStream(id, this);
     } else {
       // echo is done, clear EOF
       data.second = false;
@@ -149,11 +144,6 @@ class EchoHandler : public quic::QuicSocket::ConnectionCallback,
     auto res = sock->writeChain(id, originalData.move(), true, false, nullptr);
     if (res.hasError()) {
       LOG(ERROR) << "write error=" << toString(res.error());
-    } else if (res.value()) {
-      LOG(INFO) << "socket did not accept all data, buffering len="
-                << res.value()->computeChainDataLength();
-      originalData.append(std::move(res.value()));
-      sock->notifyPendingWriteOnStream(id, this);
     } else {
       // echo is done, clear EOF
       data.second = false;

quic/server/test/QuicSocketTest.cpp

@@ -49,7 +49,7 @@ TEST_F(QuicSocketTest, simple) {
   EXPECT_CALL(*socket_, readNaked(3, _))
       .WillOnce(Return(readResult("hello world", true)));
   EXPECT_CALL(*socket_, writeChain(3, _, true, false, nullptr))
-      .WillOnce(Return(nullptr));
+      .WillOnce(Return(folly::unit));
   handler_.readAvailable(3);
 }
 
@@ -64,29 +64,6 @@ TEST_F(QuicSocketTest, multiple_reads) {
   EXPECT_CALL(*socket_, readNaked(3, _))
       .WillOnce(Return(readResult("world", true)));
   EXPECT_CALL(*socket_, writeChain(3, _, true, false, nullptr))
-      .WillOnce(Return(nullptr));
+      .WillOnce(Return(folly::unit));
   handler_.readAvailable(3);
 }
-
-TEST_F(QuicSocketTest, blocked_write) {
-  InSequence enforceOrder;
-  openStream(3);
-
-  EXPECT_CALL(*socket_, readNaked(3, _))
-      .WillOnce(Return(readResult("hello world", true)));
-  EXPECT_CALL(*socket_, writeChain(3, _, true, false, nullptr))
-      .WillOnce(Invoke(
-          [](StreamId,
-             MockQuicSocket::SharedBuf b,
-             bool,
-             bool,
-             MockQuicSocket::DeliveryCallback*) {
-            return IOBuf::copyBuffer(b->data(), b->length()).release();
-          }));
-  EXPECT_CALL(*socket_, notifyPendingWriteOnStream(3, _));
-  handler_.readAvailable(3);
-
-  EXPECT_CALL(*socket_, writeChain(3, _, true, false, nullptr))
-      .WillOnce(Return(nullptr));
-  handler_.onStreamWriteReady(3, 100);
-}