Service DI: Infra bundle, FromRef-composed services, one-axis module tree, worker supervision - #228
Merged
Merged
Conversation
Group the three ambient clients (Postgres, NATS, crypto) into a single `Infra` handle so services and workers stop re-listing the trio, and stop storing stateless services in `ServiceState` — compose them from `Infra` in their `FromRef` impls (a pure Arc-move, free per request). Only the two stateful singletons (`ConnectionSyncService`'s cancellation registry, `HealthCache`'s snapshot) remain stored. `ServiceState` drops 15 fields to 8; workers reach through `Infra`/their service rather than duplicating deps (`DetectionWorker` 8->6 args, sync/webhook/retention workers likewise). DI wiring is three small macros: `impl_di_infra!` (clone one client out of `Infra`), `impl_di_field!` (clone a stored field), `impl_di_compose!` (build a stateless service from `Infra`). The detection worker composes its collaborators through the same `FromRef` wiring handlers use, so there is one graph, not two. Renames for clarity: - RetentionWorker -> FileRetentionWorker - DetectionService -> DetectionQueue (enqueue + status broadcast) - BlobService -> RunBlobStore (run document/redacted/audit I/O) - ObjectService -> ExternalObjectStore (external tenant stores) - WebhookWorker -> WebhookDeliveryWorker - service/connection.rs -> connection_config.rs (holds only ConnectionConfig) - service/blob.rs -> run_blob_store.rs, service/object.rs -> external_object_store.rs Full gate green (check / fmt / clippy / doc / test). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
… file Make the module tree express a single rule — a folder holds a multi-concept subsystem, a flat file holds exactly one type: - Flatten `notification/` -> `notification.rs` and `retention/` -> `file_retention.rs`; both were folders wrapping one type behind a re-export `mod.rs` that earned nothing. - Dissolve `security/` (a junk drawer of unrelated concerns): `session_keys` and `user_agent` become root files (peers of `avatar`/`infra`), and password handling — the one member with its own internal split — becomes `password/` (`service.rs` + `hasher.rs` + `strength.rs`). Genuine subsystems keep their folders unchanged (`crypto/`, `engine/`, `health/`, `detection/`, `sync/`, `webhook/`). The public facade in `service/mod.rs` is the only external contract, so no call sites outside `service/` change. Full gate green (check / fmt / clippy / doc / test). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…sibility Collapse the repeated spawn/cancel/join-and-log boilerplate in main.rs. A new `Worker` trait (name + run-until-cancelled, with an associated `Output` so each worker keeps its own `Result` alias) and a `WorkerSet` supervisor own the lifecycle once: `spawn` under one shared cancellation token, `shutdown` cancels and joins them all, logging (never propagating) a panicked task. All four workers implement it, and `ServiceState::spawn_workers` replaces the four per-worker builder methods. `run()` in main drops from ~80 lines to ~10. Also normalize service/ module visibility: every submodule is now private `mod`, with the public API surfaced through the `mod.rs` facade (matching how the crate is already consumed everywhere — via `crate::service::X`, never the submodule path). `crypto`/`engine` were needlessly `pub mod` while nothing used those paths; `CryptoError` is now surfaced via a `pub(crate) use` in the facade instead of a `service::crypto::` reach-through. Full gate green (check / fmt / clippy / doc / test). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
martsokha
added
cli
The DI worker-supervision change moved cancellation into nvisy-server's `WorkerSet`, so nvisy-cli no longer uses `tokio-util` — cargo-machete flagged it. Remove the dependency. Also remove the `machete` job from build.yml: it duplicated the "Unused Dependencies" job in security.yml (both just run `cargo machete`). Keep the security.yml one. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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
1 participant
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
Pure restructuring of the
nvisy-serverservice layer (no behavior change). Answers three questions raised in review: is the DI nesting idiomatic, how shouldservice/be organized, and how to kill the repetitive worker setup inmain.rs.1. Dependency injection —
Infra+FromRef-composed servicesInfrahandle so services/workers stop re-listing the trio.ServiceState— compose them fromInfrain theirFromRefimpls (a pureArc-move, free per request). Only the two stateful singletons (ConnectionSyncService's cancellation registry,HealthCache's snapshot) remain stored.ServiceStatedrops from 15 fields → 8; workers reach throughInfra/their service instead of duplicating deps (DetectionWorker8→6 constructor args; sync/webhook/retention likewise).impl_di_infra!(clone one client out ofInfra),impl_di_field!(clone a stored field),impl_di_compose!(build a stateless service fromInfra). The detection worker composes its collaborators through the sameFromRefwiring handlers use — one graph, not two.2. Naming
RetentionWorkerFileRetentionWorkerDetectionServiceDetectionQueueBlobServiceRunBlobStoreObjectServiceExternalObjectStoreWebhookWorkerWebhookDeliveryWorkerPlus file renames to match (
connection.rs→connection_config.rs, etc.).3.
service/structure — one organizing axisFolder ⇔ multi-concept subsystem; flat file ⇔ exactly one type.
notification/→notification.rsandretention/→file_retention.rs(folders wrapping one type behind a re-exportmod.rsthat earned nothing).security/(a junk drawer):session_keys/user_agentbecome root files; password handling — the one member with an internal split — becomespassword/(service+hasher+strength).crypto,engine,health,detection,sync,webhook).4. Worker supervision + module visibility
Workertrait (name+ run-until-cancelled, associatedOutputso each worker keeps its ownResult) and aWorkerSetsupervisor own the spawn/cancel/join-and-log lifecycle once.ServiceState::spawn_workersreplaces the four builders;main.rs::rundrops from ~80 lines to ~10.service/submodule is now privatemodwith the public API surfaced through themod.rsfacade (matching how the crate is consumed everywhere —crate::service::X, never the submodule path).crypto/enginewere needlesslypub mod.Testing
Full gate green at each of the four commits:
check/fmt/clippy -D warnings/doc -D warnings/test.🤖 Generated with Claude Code