breaking: remove the Handler and StateManager traits - #289
Open
danieleades wants to merge 1 commit into
Open
Conversation
Both had a single implementor, were never used as dyn, and could not have been: StateManager::record is generic and Handler::handle returns impl Future, so neither is dyn compatible. Their only effect was to leak pub(crate) bounds through the public HttpMockServer alias. Making the server concrete lets the alias become a real type, and frees the names Handler and Manager for the types that were carrying an HttpMock prefix to avoid them.
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changed
HandlerandStateManagertraitsMockServerandHttpMockHandlerconcrete, soHttpMockServerbecomes a real type rather than an alias over internal genericsHttpMockHandlertohandler::Handler,HttpMockStateManagertostate::Manager, andMockServerStateto a privatestate::InnerHttpMockServer::newinfallible andpub(crate)private_boundsfrom the temporary allow listWhy
private_boundsfired four times becausepub type HttpMockServer = MockServer<HttpMockHandler<HttpMockStateManager>>made both generics publicly reachable while their bounds werepub(crate).Keeping the traits and just hiding the structs does not work: it trades the four
private_boundserrors for fourprivate_interfacesones, becauseHttpMockServerBuilder::buildis public and returns the server. The options were to export a 22-method trait publicly forever, or to stop pretending the generics were doing anything.They were not. Each trait had exactly one implementor and neither is dyn compatible,
StateManager::recordbeing generic andHandler::handlereturningimpl Future, so neither could ever have been a dynamic dispatch seam. Nothing substitutes an alternative state manager or handler, including the tests.Manageris a pure in-memory type over aMutex, with no I/O to fake.Dropping them also frees the good names.
HandlerandManagerwere only spelledHttpMockHandlerandHttpMockStateManagerbecause the traits held those names, andstate::Inneris the conventional spelling for data guarded by a lock.HttpMockServerkeeps its prefix, sincehttpmock::MockServeris already the client-side handle.Breaking changes
httpmock::server::state::HttpMockStateManageris nowstate::Managerhttpmock::server::state::MockServerStateis now privateHttpMockServeris a struct rather than a type alias, andHttpMockServer::newispub(crate)Nothing that was callable downstream has been removed.
MockServer::newtook an unnameable argument type, and everyManagermethod other thannewanddefaultwas already behind apub(crate)trait. Construction still goes throughHttpMockServerBuilder, andstart,start_with_signalsandrun_accept_loopare unchanged. Runtime behavior is unchanged.Validation
cargo clippy --all-targets --all-featurescargo clippy --all-targetscargo clippy --all-targets --no-default-featurescargo clippy --all-targets --features standalonecargo hack check --feature-powerset --depth 2 --all-targetscargo hack check --rust-version --each-feature --no-dev-depson 1.88cargo test --all-featurescargo fmt --all -- --checkgit diff --check