Skip to content

feat(udp-core, udp-server, configuration): add trait abstractions for REST API decoupling (SI-30) - #1933

Merged
josecelano merged 1 commit into
torrust:developfrom
josecelano:1924-decouple-rest-api-core-from-udp-internals
Jun 23, 2026
Merged

feat(udp-core, udp-server, configuration): add trait abstractions for REST API decoupling (SI-30)#1933
josecelano merged 1 commit into
torrust:developfrom
josecelano:1924-decouple-rest-api-core-from-udp-internals

Conversation

@josecelano

Copy link
Copy Markdown
Member

Summary

Spec review for EPIC #1669 subissues SI-30 and SI-33:

SI-30 (#1924) — narrowed scope

  • Now focuses on UDP-side trait extraction only (BanningStats, UdpCoreStatsRepository, UdpServerStatsRepository)
  • REST-side wiring deferred to SI-33 (contract-first architecture)
  • Added deep coupling analysis documenting exactly what the REST layer uses from each UDP type
  • Added corrected architectural understanding (REST API is orchestrating service, not layer inversion)
  • MAX_CONNECTION_ID_ERRORS_PER_IP → config option

SI-33 (#1930) — new open issue

EPIC.md

  • Updated subissue table to reflect new SI-30 scope and added SI-33 entry
  • Corrected description of rest-api-core → udp-server dependency from "layer violation" to "cross-service orchestration dep"

Pre-merge checks

  • cargo machete — pass
  • linter all — pass
  • cargo test --doc --workspace — pass
  • cargo +nightly fmt --check — pass
  • cargo +nightly check --all-targets --all-features — pass
  • cargo +stable test --all-targets --all-features — pass

Copilot AI review requested due to automatic review settings June 23, 2026 09:03
@josecelano josecelano self-assigned this Jun 23, 2026

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR advances EPIC #1669 (SI-30) by extracting UDP-side trait abstractions and moving the connection-id error ban threshold from a hardcoded constant into configuration, laying groundwork for later REST decoupling (SI-33).

Changes:

  • Introduces trait abstractions for UDP core/server stats repositories and banning stats.
  • Replaces MAX_CONNECTION_ID_ERRORS_PER_IP with UdpTracker.max_connection_id_errors_per_ip (defaulted via serde).
  • Wires the new config field through containers/examples/tests and adds async-trait where needed.

Reviewed changes

Copilot reviewed 12 out of 13 changed files in this pull request and generated 8 comments.

Show a summary per file
File Description
src/container.rs Plumbs max_connection_id_errors_per_ip into UDP core services init.
packages/udp-server/src/statistics/repository.rs Adds UdpServerStatsRepository trait and implements it for the concrete repository.
packages/udp-server/examples/udp_only_public_tracker.rs Adds the new UDP tracker config field in the example.
packages/udp-server/Cargo.toml Adds async-trait dependency for the new trait object pattern.
packages/udp-core/src/statistics/repository.rs Adds UdpCoreStatsRepository trait and implements it for the concrete repository.
packages/udp-core/src/services/banning.rs Adds BanningStats trait implemented by BanService.
packages/udp-core/src/lib.rs Removes the hardcoded MAX_CONNECTION_ID_ERRORS_PER_IP constant.
packages/udp-core/src/container.rs Threads the configured threshold into BanService initialization.
packages/udp-core/Cargo.toml Adds async-trait dependency for the new trait object pattern.
packages/test-helpers/src/configuration.rs Updates ephemeral config builder with the new UDP config field.
packages/rest-api-core/src/statistics/services.rs Removes dependency on the deleted constant in tests.
packages/configuration/src/v2_0_0/udp_tracker.rs Adds max_connection_id_errors_per_ip config field with serde default.
Cargo.lock Locks async-trait into the workspace dependency graph.
Comments suppressed due to low confidence (1)

packages/udp-server/examples/udp_only_public_tracker.rs:68

  • This example now sets additional UdpTracker fields, but the runtime output still claims UdpTracker is only “bind address, cookie lifetime”. Updating the printed description avoids confusing users running the example.
        max_connection_id_errors_per_ip: 10,
    };

    println!("Types from torrust-tracker-configuration used by this binary:");
    println!("  Core       — tracker domain settings");

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/container.rs Outdated
Comment thread src/container.rs Outdated
Comment thread packages/udp-core/src/statistics/repository.rs
Comment thread packages/udp-core/src/statistics/repository.rs
Comment thread packages/udp-server/src/statistics/repository.rs
Comment thread packages/udp-server/src/statistics/repository.rs
Comment thread packages/udp-core/src/services/banning.rs
Comment thread packages/udp-core/src/services/banning.rs
@codecov

codecov Bot commented Jun 23, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 56.00000% with 11 lines in your changes missing coverage. Please review.
✅ Project coverage is 79.51%. Comparing base (94427f3) to head (637c17b).

Files with missing lines Patch % Lines
src/container.rs 0.00% 4 Missing ⚠️
packages/udp-core/src/services/banning.rs 0.00% 3 Missing ⚠️
packages/udp-core/src/statistics/repository.rs 0.00% 2 Missing ⚠️
packages/udp-server/src/statistics/repository.rs 0.00% 2 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #1933      +/-   ##
===========================================
+ Coverage    79.46%   79.51%   +0.05%     
===========================================
  Files          324      324              
  Lines        23116    23137      +21     
  Branches     23116    23137      +21     
===========================================
+ Hits         18369    18398      +29     
+ Misses        4476     4468       -8     
  Partials       271      271              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@josecelano
josecelano force-pushed the 1924-decouple-rest-api-core-from-udp-internals branch from bc3bdf5 to c881f87 Compare June 23, 2026 09:36
… REST API decoupling (SI-30)

- Add BanningStats trait in udp-core services/banning.rs
- Add UdpCoreStatsRepository trait in udp-core statistics
- Add UdpServerStatsRepository trait in udp-server statistics
- Add max_connection_id_errors_per_ip config field to UdpTracker
- Update UdpTrackerCoreServices to use config value instead of constant
- Remove pub const MAX_CONNECTION_ID_ERRORS_PER_IP from udp-core lib.rs
- Add async-trait dependency to udp-core and udp-server
@josecelano
josecelano force-pushed the 1924-decouple-rest-api-core-from-udp-internals branch from c881f87 to 637c17b Compare June 23, 2026 10:54
@josecelano

Copy link
Copy Markdown
Member Author

ACK 637c17b

@josecelano
josecelano merged commit 9f5fd83 into torrust:develop Jun 23, 2026
17 of 18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

SI-30: Decouple rest-api-core and axum-rest-api-server from concrete UDP server internals

2 participants