From 205f1bf7fdc40d477fc50d53a0ff3ee7a368b96a Mon Sep 17 00:00:00 2001 From: Hani Damlaj Date: Sun, 18 Oct 2026 10:36:38 -0700 Subject: [PATCH] remove `ServerPushLifecycleCallback` Summary: removes `ServerPushLifecycleCallback`, its mock, and `ServerPushLifecycle.h`, continued efforts to remove all things http push related in proxygen/lib Reviewed By: jbeshay, Carl-Elkhoury Differential Revision: D124385264 fbshipit-source-id: f529f4f8a5e16a8b2af6e0adf4725ff1008f4955 --- proxygen/lib/http/session/HQSession.h | 9 - .../lib/http/session/HQUpstreamSession.cpp | 10 - proxygen/lib/http/session/HQUpstreamSession.h | 7 - .../lib/http/session/ServerPushLifecycle.h | 157 --------------- .../lib/http/session/test/HQSessionMocks.h | 179 ------------------ 5 files changed, 362 deletions(-) delete mode 100644 proxygen/lib/http/session/ServerPushLifecycle.h diff --git a/proxygen/lib/http/session/HQSession.h b/proxygen/lib/http/session/HQSession.h index 4252fef0b..6ba189c23 100644 --- a/proxygen/lib/http/session/HQSession.h +++ b/proxygen/lib/http/session/HQSession.h @@ -33,7 +33,6 @@ #include #include #include -#include #include #include #include @@ -115,10 +114,6 @@ class HQSession return maxAllowedPushId_; } - void setServerPushLifecycleCallback(ServerPushLifecycleCallback* cb) { - serverPushLifecycleCb_ = cb; - } - class ConnectCallback { public: virtual ~ConnectCallback() = default; @@ -753,10 +748,6 @@ class HQSession mutable folly::SocketAddress cachedLocalAddr_; mutable folly::SocketAddress cachedPeerAddr_; - // Callback pointer used for correctness testing. Not used - // for session logic. - ServerPushLifecycleCallback* serverPushLifecycleCb_{nullptr}; - // Debug feature to test the impact of request body prioritization. bool enableEgressPrioritization_{true}; diff --git a/proxygen/lib/http/session/HQUpstreamSession.cpp b/proxygen/lib/http/session/HQUpstreamSession.cpp index 1d18fd005..41ced9ee5 100644 --- a/proxygen/lib/http/session/HQUpstreamSession.cpp +++ b/proxygen/lib/http/session/HQUpstreamSession.cpp @@ -240,16 +240,6 @@ void HQUpstreamSession::HQIngressPushStream::bindTo(quic::StreamId streamId) { // Re-enable reads session.resumeReadsForPushStream(streamId); - - // Notify testing callbacks that a full push transaction - // has been successfully initialized - if (session.serverPushLifecycleCb_) { - session.serverPushLifecycleCb_->onPushedTxn(&txn_, - streamId, - getPushId(), - txn_.getAssocTxnId().value(), - false /* eof */); - } } // This can only be unbound in that it has not received a stream ID yet diff --git a/proxygen/lib/http/session/HQUpstreamSession.h b/proxygen/lib/http/session/HQUpstreamSession.h index 3c82aed2a..bca4e5667 100644 --- a/proxygen/lib/http/session/HQUpstreamSession.h +++ b/proxygen/lib/http/session/HQUpstreamSession.h @@ -141,16 +141,9 @@ class HQUpstreamSession : public HQSession { // Ingress push streams are not initialized // until after the nascent push stream // has been received - // notify the testing callbacks that a half-opened push transaction - // has been created - // NOTE: change the API to avoid accepting parent txn ids // as optional CHECK(parentTxnId.has_value()); - auto cb = ((HQUpstreamSession&)session_).serverPushLifecycleCb_; - if (cb) { - cb->onHalfOpenPushedTxn(&txn_, pushId, *parentTxnId, false); - } } // Bind this stream to a transport stream diff --git a/proxygen/lib/http/session/ServerPushLifecycle.h b/proxygen/lib/http/session/ServerPushLifecycle.h deleted file mode 100644 index 7a3a1024a..000000000 --- a/proxygen/lib/http/session/ServerPushLifecycle.h +++ /dev/null @@ -1,157 +0,0 @@ -/* - * Copyright (c) Meta Platforms, Inc. and affiliates. - * All rights reserved. - * - * This source code is licensed under the BSD-style license found in the - * LICENSE file in the root directory of this source tree. - */ - -#pragma once - -#include -#include -#include - -namespace proxygen { - -class HTTPMessage; -class HTTPTransaction; - -// A callback class for observing pushed transactions. -// It is built for HTTP3 state transitions, but is backward -// compatible with HTTP2 -// -// The state transition diagram (states unique to HTTP3 -// are marked with "h3") -// -// The sole purpose of this callback is tesing the correct -// behavior of the server push. -// -// -// .-----------------. -// ( Start ) -// `-----------------' -// | -// +---------------+--------------+ -// v v -// +---------------------+ +---------------------h3 -// +---| onPushPromiseBegin | | onNascentBegin |---+ -// | +---------------------+ +----------------------+ | -// | | | | -// | v v | -// | +---------------------+ +------------------h3 | -// | | onPushPromise |-------+ | onNascentStream |----+ -// | +---------------------+ | +-------------------+ | -// | | | | | -// | +---+ | | | -// | v | | | -// | +------------------------h3 | | | -// +--+--| onHalfPushedTransaction | | +---+ | -// | | +-------------------------+ | | | -// | | | | | | -// | | +--------------+ | | -// | | v | | -// | +----------+ +-------------------------+ | | -// | +------| onFullPushedTransaction |<-+ | -// | v +-------------------------+ | -// | .----------------h3 | | -// | (onPushedTxnTimeout ) | .-----------------h3 | -// | `-----------------' | ( onOrphanedNascent )<--+ -// | +--+ `------------------' | -// | .-----------------h3 | .------------h3 | -// +->( onOrphanedHfOpnTxn ) | ( onNascentEof )<-+ -// `------------------' | `-------------' -// v -// .-----------. -// ( onTxnEof ) -// `-----------' -// -// -class ServerPushLifecycleCallback { - public: - virtual ~ServerPushLifecycleCallback() = default; - - /** - * A push promise has arrived, but has not been fully parsed yet - */ - virtual void onPushPromiseBegin(HTTPCodec::StreamID /* parent streamID */, - hq::PushId /* pushID */) = 0; - - /** - * A push promise has arrived. Headers have been parsed. - */ - virtual void onPushPromise(HTTPCodec::StreamID /* parent streamID */, - hq::PushId /* pushID */, - HTTPMessage* /* msg */) = 0; - - /** - * A push stream has arrived, but the pushId has not been received yet - */ - virtual void onNascentPushStreamBegin( - HTTPCodec::StreamID /* push stream ID */, bool /* eom */) = 0; - - /** - * A push stream has arrived, and the pushId has been received. - */ - virtual void onNascentPushStream(HTTPCodec::StreamID /* push stream ID */, - hq::PushId /* pushId */, - bool /* eom */) = 0; - - /** - * A push stream has ended before being converted to to full - * ingress stream - */ - virtual void onNascentEof(HTTPCodec::StreamID /* push stream ID */, - folly::Optional /* pushId */) = 0; - - /** - * A nascent push stream has timed out. For more details, see - * "The Personal History, Adventures, Experience and Observation - * of David Copperfield the Younger of Blunderstone Rookery - * (Which He Never Meant to Publish on Any Account).", Dickens C. - */ - virtual void onOrphanedNascentStream( - HTTPCodec::StreamID /* push stream ID */, - folly::Optional /* pushId */) = 0; - - /** - * A push promise has arrived, the push id and the assoc (parent) id - * are known. - */ - virtual void onHalfOpenPushedTxn(const HTTPTransaction* /* txn */, - hq::PushId /* server push id */, - HTTPCodec::StreamID /* assoc stream id */, - bool /* eom */) = 0; - - /** - * Both the promise and the stream have arrived. The newly created transport - * can be created - */ - virtual void onPushedTxn(const HTTPTransaction* /* txn */, - HTTPCodec::StreamID /* push ingress stream */, - hq::PushId /* server push id */, - HTTPCodec::StreamID /* assoc stream id */, - bool /* eom */) = 0; - - /** - * A fully opened pushed transaction has timed out - */ - virtual void onPushedTxnTimeout(const HTTPTransaction* /* txn */) = 0; - - /** - * A half-opened push transaction has timed out - */ - virtual void onOrphanedHalfOpenPushedTxn( - const HTTPTransaction* /* txn */) = 0; - - /** - * Push ID limit exceeded, possibly closing the stream - */ - virtual void onPushIdLimitExceeded( - hq::PushId /* incoming push id */, - folly::Optional /* maximal allowed push id */, - folly::Optional /* possible push stream */) = 0; - -}; // ServerPushLifecycleCallback - -} // namespace proxygen diff --git a/proxygen/lib/http/session/test/HQSessionMocks.h b/proxygen/lib/http/session/test/HQSessionMocks.h index 0902a20fe..89a0608aa 100644 --- a/proxygen/lib/http/session/test/HQSessionMocks.h +++ b/proxygen/lib/http/session/test/HQSessionMocks.h @@ -17,185 +17,6 @@ namespace proxygen { -class MockServerPushLifecycleCallback : public ServerPushLifecycleCallback { - public: - ~MockServerPushLifecycleCallback() override = default; - - MOCK_METHOD(void, - onPushPromiseBegin, - (HTTPCodec::StreamID /* parent streamID */, - hq::PushId /* pushID */)); - - MOCK_METHOD(void, - onPushPromise, - (HTTPCodec::StreamID /* parent streamID */, - hq::PushId /* pushID */, - HTTPMessage* /* msg */)); - - MOCK_METHOD(void, - onNascentPushStreamBegin, - (HTTPCodec::StreamID /* push stream ID */, bool /* eom */)); - - MOCK_METHOD(void, - onNascentPushStream, - (HTTPCodec::StreamID /* push stream ID */, - hq::PushId /* server push id */, - bool /* eom */)); - - MOCK_METHOD(void, - onNascentEof, - (HTTPCodec::StreamID /* push stream ID */, - folly::Optional /* push id */)); - - MOCK_METHOD(void, - onOrphanedNascentStream, - (HTTPCodec::StreamID /* push stream ID */, - folly::Optional /* push id */)); - - MOCK_METHOD(void, - onHalfOpenPushedTxn, - (const HTTPTransaction* /* txn */, - hq::PushId /* push id */, - HTTPCodec::StreamID /* assoc stream id */, - bool /* eom */)); - - MOCK_METHOD(void, - onPushedTxn, - (const HTTPTransaction* /* txn */, - HTTPCodec::StreamID /* push stream id */, - hq::PushId /* push id */, - HTTPCodec::StreamID /* assoc stream id */, - bool /* eom */)); - - MOCK_METHOD(void, onPushedTxnTimeout, (const HTTPTransaction* /* txn */)); - - MOCK_METHOD(void, - onOrphanedHalfOpenPushedTxn, - (const HTTPTransaction* /* txn */)); - - MOCK_METHOD(void, - onPushIdLimitExceeded, - (hq::PushId /* incoming push id */, - folly::Optional /* max allowed push id */, - folly::Optional /* stream */)); - - using PushPromiseBeginF = - std::function; - using PushPromiseF = - std::function; - using NascentPushStreamBeginF = - std::function; - using NascentPushStreamF = - std::function; - using NascentEofF = - std::function)>; - using OrphanedNascentStreamF = - std::function)>; - using HalfOpenPushedTxnF = std::function; - using PushedTxnF = std::function; - using PushedTxnTimeoutF = std::function; - using OrphanedHalfOpenPushedTxnF = - std::function; - - using PushIdLimitExceededF = - std::function, - folly::Optional)>; - - void expectPushPromiseBegin(PushPromiseBeginF impl = nullptr) { - auto& exp = EXPECT_CALL(*this, onPushPromiseBegin(testing::_, testing::_)); - if (impl) { - exp.WillOnce(testing::Invoke(impl)); - } - } - - void expectPushPromise(PushPromiseF impl = nullptr) { - auto& exp = - EXPECT_CALL(*this, onPushPromise(testing::_, testing::_, testing::_)); - if (impl) { - exp.WillOnce(testing::Invoke(impl)); - } - } - - void expectNascentPushStreamBegin(NascentPushStreamBeginF impl = nullptr) { - auto& exp = - EXPECT_CALL(*this, onNascentPushStreamBegin(testing::_, testing::_)); - if (impl) { - exp.WillOnce(testing::Invoke(impl)); - } - } - - void expectNascentPushStream(NascentPushStreamF impl = nullptr) { - auto& exp = EXPECT_CALL( - *this, onNascentPushStream(testing::_, testing::_, testing::_)); - if (impl) { - exp.WillOnce(testing::Invoke(impl)); - } - } - - void expectNascentEof(NascentEofF impl = nullptr) { - auto& exp = EXPECT_CALL(*this, onNascentEof(testing::_, testing::_)); - if (impl) { - exp.WillOnce(testing::Invoke(impl)); - } - } - - void expectOrphanedNascentStream(OrphanedNascentStreamF impl = nullptr) { - auto& exp = - EXPECT_CALL(*this, onOrphanedNascentStream(testing::_, testing::_)); - if (impl) { - exp.WillOnce(testing::Invoke(impl)); - } - } - - void expectHalfOpenPushedTxn(HalfOpenPushedTxnF impl = nullptr) { - auto& exp = EXPECT_CALL( - *this, - onHalfOpenPushedTxn(testing::_, testing::_, testing::_, testing::_)); - if (impl) { - exp.WillOnce(testing::Invoke(impl)); - } - } - - void expectPushedTxn(PushedTxnF impl = nullptr) { - auto& exp = EXPECT_CALL( - *this, - onPushedTxn( - testing::_, testing::_, testing::_, testing::_, testing::_)); - if (impl) { - exp.WillOnce(testing::Invoke(impl)); - } - } - - void expectPushedTxnTimeout(PushedTxnTimeoutF impl = nullptr) { - auto& exp = EXPECT_CALL(*this, onPushedTxnTimeout(testing::_)); - if (impl) { - exp.WillOnce(testing::Invoke(impl)); - } - } - - void expectOrphanedHalfOpenPushedTxn( - OrphanedHalfOpenPushedTxnF impl = nullptr) { - auto& exp = EXPECT_CALL(*this, onOrphanedHalfOpenPushedTxn(testing::_)); - if (impl) { - exp.WillOnce(testing::Invoke(impl)); - } - } - - void expectPushIdLimitExceeded(PushIdLimitExceededF impl = nullptr) { - auto& exp = EXPECT_CALL( - *this, onPushIdLimitExceeded(testing::_, testing::_, testing::_)); - if (impl) { - exp.WillOnce(testing::Invoke(impl)); - } - } -}; - class MockConnectCallback : public HQSession::ConnectCallback { public: MOCK_METHOD(void, connectSuccess, ());