Repository navigation
feat(registry): add Registar::deregister - #2
Merged
Merged
Conversation
Add an additive async deregister(&ServiceBinding) that removes one registration and returns it, so a stopped service can register its final binding again. Bump torrust-server-lib from 0.3.0 to 0.4.0 for the additive public API, as the previous additive API did (0.2.0 to 0.3.0). Related to torrust/torrust-tracker#2450. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
🟢 Approval recommended
The mutex-protected removal matches its documented contract, focused tests cover the intended behavior, and no blocking issues were identified.
No open issues 🎉
What changed in this PR
Adds Registar::deregister so stopped services can release their registrations and later register the same binding again.
Changes:
- Adds an async removal method that returns the removed service or
None, with documented caller ownership expectations. - Adds four tests covering removal, re-registration, unknown bindings, and preservation of other registrations.
- Bumps the crate version from
0.3.0to0.4.0.
| File | Description |
|---|---|
src/registar.rs |
Adds deregistration, API documentation, and regression tests. |
Cargo.toml |
Updates the crate version to 0.4.0. |
Cargo.lock |
Synchronizes the crate version with the manifest. |
Review effort: Balanced
josecelano
added a commit
to torrust/torrust-tracker
that referenced
this pull request
Oct 26, 2026
…the health-check API test environment to the token lifecycle 6c0d9cd docs(pr-reviews): [#2450] record the review audit of PR 2652 (Jose Celano) 8ed7da2 docs(issues): [#2450] record the independent review of the final tree (Jose Celano) 6c21b1d docs(issues): [#2450] record the prose-first comparison of the new tests and fix the review findings (Jose Celano) 0042a18 docs(issues): [#2450] record the registry deregistration and update the spec to the final design (Jose Celano) cb8c34f fix(axum-health-check-api-server): [#2450] deregister the health-check test environment on stop (Jose Celano) c554b24 chore(deps): [#2450] bump torrust-server-lib to 0.4.0 (Jose Celano) 70b2141 docs(issues): [#2450] correct the independent review entry after the fixed-port finding (Jose Celano) be0acb8 docs(issues): [#2450] record the fixed-port restart limitation and its retrospective (Jose Celano) e810655 test(axum-health-check-api-server): [#2450] document and pin the fixed-port restart limitation (Jose Celano) f0656bc docs(issues): [#2450] record the independent review, resolve its findings and update the spec status (Jose Celano) aa28b3f test(axum-health-check-api-server): [#2450] assert the stop result in the binding-release test (Jose Celano) 8a09fe5 docs(issues): [#2450] record the prose-first comparison and the post-change checks (Jose Celano) 6cf6534 docs(shutdown-process): [#2450] record the health-check test environment token lifecycle (Jose Celano) a381a86 docs(issues): [#2450] record T0 baseline, T1 mutation proofs and M1/M2 evidence (Jose Celano) 4830852 refactor(axum-health-check-api-server): [#2450] migrate the health-check API test environment to the token lifecycle (Jose Celano) Pull request description: Closes #2450 Migrates the health-check API test `Environment` to the token lifecycle (SI-24, EPIC #1488 roadmap row 17), following the #2449 pattern. What the change does: private `state`, `Stopped` and `Running` structs, one cancellation token held as one `DropGuard`, and a `stop()` that disarms and cancels before its first await, joins both tasks and returns an error naming every failing task. A new `bind_address()` accessor replaces the 8 field reads in the contract tests. Review round: Copilot and da2ce7 found that a stop-then-start on a fixed port panicked with `DuplicateBinding`, because the token-aware start registers the service and `Registar` could not remove it. This PR now fixes it: `stop()` removes the environment's own registration with `Registar::deregister`, which is new in `torrust-server-lib` 0.4.0 (torrust/torrust-server-lib#2, merged and published). The decision to first document the limitation was reversed; see the evidence file, section V7. The audit is `docs/pr-reviews/pr-2652-review/PR-REVIEW.md`. Evidence and reviews: `manual-verification-evidence.md` (V0 to V7, mutation proofs), `agent-review-reports.md` (an earlier review entry, its correction, and a review of the final tree) and `implementation-retrospective.md`. Acceptance criteria AC1 to AC8 are left for the maintainer to tick. Head to merge: the current head of this branch, `docs(pr-reviews): [#2450] record the review audit of PR 2652`. Any later push will be announced in a comment on this pull request. Commits, oldest first: - `refactor(axum-health-check-api-server): [#2450] migrate the health-check API test environment to the token lifecycle` - `docs(issues): [#2450] record T0 baseline, T1 mutation proofs and M1/M2 evidence` - `docs(shutdown-process): [#2450] record the health-check test environment token lifecycle` - `docs(issues): [#2450] record the prose-first comparison and the post-change checks` - `test(axum-health-check-api-server): [#2450] assert the stop result in the binding-release test` - `docs(issues): [#2450] record the independent review, resolve its findings and update the spec status` - `test(axum-health-check-api-server): [#2450] document and pin the fixed-port restart limitation` - `docs(issues): [#2450] record the fixed-port restart limitation and its retrospective` - `docs(issues): [#2450] correct the independent review entry after the fixed-port finding` - `chore(deps): [#2450] bump torrust-server-lib to 0.4.0` - `fix(axum-health-check-api-server): [#2450] deregister the health-check test environment on stop` - `docs(issues): [#2450] record the registry deregistration and update the spec to the final design` - `docs(issues): [#2450] record the prose-first comparison of the new tests and fix the review findings` - `docs(issues): [#2450] record the independent review of the final tree` - `docs(pr-reviews): [#2450] record the review audit of PR 2652` Files touched: - `Cargo.lock` - `Cargo.toml` - `docs/features/shutdown-process/task-inventory.md` - `docs/issues/open/2450-1488-si-24-migrate-health-check-api-test-environment/ISSUE.md` - `docs/issues/open/2450-1488-si-24-migrate-health-check-api-test-environment/agent-review-reports.md` - `docs/issues/open/2450-1488-si-24-migrate-health-check-api-test-environment/implementation-retrospective.md` - `docs/issues/open/2450-1488-si-24-migrate-health-check-api-test-environment/manual-verification-evidence.md` - `docs/pr-reviews/pr-2652-review/PR-REVIEW.md` - `packages/axum-health-check-api-server/Cargo.toml` - `packages/axum-health-check-api-server/src/environment.rs` - `packages/axum-health-check-api-server/src/server.rs` - `packages/axum-health-check-api-server/tests/server/contract.rs` - `packages/axum-http-server/Cargo.toml` - `packages/axum-rest-api-server/Cargo.toml` - `packages/axum-server/Cargo.toml` - `packages/udp-server/Cargo.toml` ACKs for top commit: josecelano: ACK 6c0d9cd Tree-SHA512: 9ebf7837f949641d98f98580e509c1a2a044ef8119b38d9379426fc47ace445a3f9955541a77a729fcc16ec5048b0f8b230d6299850706bae8eac430b9bc69b6
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.
Summary
Registar::deregister(&ServiceBinding) -> Option<RegisteredService<M>>, an additive async method that removes one registration and returns it, or returnsNonewhen the binding is not registered.RegistrationError::DuplicateBinding.torrust-server-libfrom0.3.0to0.4.0for the additive public API, as the previous additive API did (0.2.0to0.3.0).Validation
cargo test: 10 passed, including 4 new tests (remove and return, register again after removal,Nonefor an unknown binding, other registrations kept). The last was mutation-proven: clearing the whole map made it fail.cargo fmt --check,cargo clippy --all-targets -- -D warnings,linter allcargo publish --dry-runRelated to torrust/torrust-tracker#2450.