refactor(infrastructure): [#301] Phase 4 - Extract ServiceTopology shared type#303
Merged
Merged
Conversation
- Create src/domain/topology/fixed_ports.rs with caddy_ports() and mysql_ports() - Add comprehensive unit tests for fixed port functions - Update CaddyServiceConfig and MysqlServiceConfig to use domain functions - Remove derive_caddy_ports(), derive_mysql_ports(), derive_prometheus_ports(), and derive_grafana_ports() from infrastructure port_derivation.rs - Update mod.rs exports to only export derive_tracker_ports() This completes Step 5: Fixed Port Services (Caddy, MySQL)
…::new() - Remove TrackerServiceConfig::new() method entirely - Delete port_derivation.rs from infrastructure layer - Migrate all callers to use from_domain_config() - Update application layer to pass domain TrackerConfig directly - Add domain config helper functions for test code - Fix clippy doc_markdown warnings for MySQL - All 396 tests pass, all pre-commit checks pass Port derivation now uses domain PortDerivation trait exclusively. Infrastructure TrackerServiceConfig is now a pure DTO.
…fig constructors - Rename new() to from_domain_config() for consistency with TrackerServiceConfig - Update all test calls and callers in builder.rs - Part of Phase 4 DDD alignment for service configs
…ces abstraction - Create NetworkDerivation trait for domain-driven network assignment - Implement NetworkDerivation for TrackerConfig, PrometheusConfig, GrafanaConfig - Create EnabledServices abstraction (renamed from TopologyContext): - Uses HashSet<Service> for Open/Closed principle compliance - Provides only generic has(Service) method - no convenience wrappers - Add comprehensive unit tests for EnabledServices (10 tests) - Update all infrastructure service configs to use domain traits - Remove compute_networks() methods from infrastructure layer Phase 4 Step 6: Network computation now uses domain layer traits.
- Mark Steps 6.7-6.10 as not needed (Caddy/MySQL have static networks) - Mark Steps 7.1-7.4 as complete (infrastructure uses domain traits) - Update Goals section - all main objectives achieved - Defer Step 7.5 type renaming as optional - Remove builder.rs from planned files (not needed) Phase 4 DDD alignment essentially complete.
- Check all acceptance criteria items - Add NetworkDerivation and EnabledServices criteria - Update Design Decisions table with builder decision - Document final architecture: trait-based without builder
… Caddy/MySQL - Create src/domain/caddy/config.rs with CaddyConfig implementing PortDerivation and NetworkDerivation - Create src/domain/mysql/config.rs with MysqlServiceConfig implementing PortDerivation and NetworkDerivation - Delete src/domain/topology/fixed_ports.rs (no longer needed) - Update infrastructure CaddyDockerServiceConfig and MysqlDockerServiceConfig to use from_domain_config() - Update progress doc with Step 5 changes and elaborate on Step 7.5 All services now follow the same trait-based pattern for consistency and Open/Closed compliance.
- Rename TrackerServiceConfig → TrackerServiceContext - Rename GrafanaServiceConfig → GrafanaServiceContext - Rename PrometheusServiceConfig → PrometheusServiceContext - Rename CaddyDockerServiceConfig → CaddyServiceContext - Rename MysqlDockerServiceConfig → MysqlServiceContext Clear naming distinction: domain uses *Config, infrastructure uses *Context. Keep TrackerPorts as internal builder API alias only.
Remove confusing backward compatibility alias. Use TrackerServiceContext directly in the builder API for clarity and consistency.
Mark infrastructure service config renames as completed: - TrackerServiceConfig → TrackerServiceContext - GrafanaServiceConfig → GrafanaServiceContext - PrometheusServiceConfig → PrometheusServiceContext - CaddyDockerServiceConfig → CaddyServiceContext - MysqlDockerServiceConfig → MysqlServiceContext All Phase 4 implementation steps are now complete.
…viceContext Remove unused fields that duplicated domain logic: - udp_tracker_ports, http_tracker_ports_without_tls - http_api_port, http_api_has_tls, needs_ports_section These fields were serialized but never used by templates (templates use ports/networks fields). Simplifies from_domain_config to 2 lines. Improves readability and removes redundant logic duplication.
…ometheus ServiceContext Remove fields not used by docker-compose.yml.tera template: - GrafanaServiceContext: admin_user, admin_password, has_tls - PrometheusServiceContext: scrape_interval_in_secs Credentials are handled by env/context.rs for .env template. Scrape interval is handled by PrometheusContext for prometheus.yml template. Simplifies from_domain_config to 2 lines in both types.
Extract common ports/networks fields into ServiceTopology type. All Docker Compose ServiceContext types now use this shared topology via #[serde(flatten)] to maintain template compatibility. Makes the architectural pattern explicit: - Topology (ports/networks) → docker-compose.yml template - Configuration (credentials) → .env template Changes: - Add service_topology.rs with shared ServiceTopology struct - Update TrackerServiceContext to embed ServiceTopology - Update GrafanaServiceContext to embed ServiceTopology - Update PrometheusServiceContext to embed ServiceTopology - Update CaddyServiceContext to embed ServiceTopology - Update MysqlServiceContext to embed ServiceTopology - Add ports() and networks() accessor methods - Update builder.rs to use accessor methods - Update all tests to use accessor methods
3 tasks
…e positive it actually allocates on the heap. This is a known upstream bug: rust-lang/rust-clippy#12586 The fix was merged in April 2024 (should be in Rust 1.80.0+) but still triggers on Rust 1.93.0, suggesting a potential regression. See: docs/issues/304-clippy-large-stack-arrays-false-positive.md
Member
Author
|
ACK 14a27e4 |
6 tasks
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
Closes #301
This PR completes Phase 4 of the Docker Compose Topology Domain Model Refactoring by extracting a shared
ServiceTopologytype to formalize the architectural pattern discovered during cleanup.Problem
After Phase 3 cleanup that removed unused fields from all ServiceContext types, we observed that all 5 types (
TrackerServiceContext,GrafanaServiceContext,PrometheusServiceContext,CaddyServiceContext,MysqlServiceContext) now share identical fields:This wasn't just coincidence - it represents a clear architectural pattern:
docker-compose.ymltemplate (inter-service relationships).envfiles (internal service config)Solution
Extract a shared
ServiceTopologytype to make this pattern explicit:All ServiceContext types now embed this with
#[serde(flatten)]to maintain template compatibility (flat JSON structure).Changes
New File
service_topology.rs- Shared topology structure with documentation explaining the design rationaleUpdated ServiceContext Types
All now use the embedded
ServiceTopologypattern:TrackerServiceContextGrafanaServiceContextPrometheusServiceContextCaddyServiceContextMysqlServiceContextAPI Changes
ports()andnetworks()accessor methods to each ServiceContextbuilder.rsto use accessor methodsTesting
Architecture Diagram
Related
docs/refactors/plans/docker-compose-topology-domain-model.md