Skip to content

refactor(server): decouple SettingsService into data store + coordinator#2955

Merged
enoch85 merged 3 commits into
developmentfrom
refactor/decouple-settings-god-object
May 21, 2026
Merged

refactor(server): decouple SettingsService into data store + coordinator#2955
enoch85 merged 3 commits into
developmentfrom
refactor/decouple-settings-god-object

Conversation

@enoch85

@enoch85 enoch85 commented May 21, 2026

Copy link
Copy Markdown
Collaborator

Eliminates the server's circular-dependency debt at the root. @swc/jest (shipped in #2948) surfaced it as TDZ errors that were papered over with forwardRef + import type { X as XType } aliases. This removes the cause, so those workarounds go away.

Root cause

SettingsService was a god-object: both a hydrated singleton holding ~30 persisted setting fields (read synchronously in ~97 places) and a coordinator injecting 7 API services (Plex/Servarr/Seerr/Tautulli/Streamystats/InternalApi/MediaServerFactory). Those API services injected SettingsService back just to read config → mutual cycles.

What this does (the whole overview)

Split the god-object into two services named for their roles:

  • SettingsDataService — owns the settings data: the persisted fields, init() hydration, getSettings/getPublicSettings (with secret masking), the read helpers, and low-level persistence. Depends only on the settings repositories — zero service dependencies, so anything can inject it.
  • SettingsOperationsService — owns settings operations: connection tests + save/remove/reinit flows for every integration. Injects the API services as plain private readonly (no forwardRef); holds no settings state.
  • Media-server factory + Plex/Jellyfin/Emby adapters and all ~97 field reads / read consumers (API services, schedulers, collection worker, rule executor, metadata, notifications, controller) now inject SettingsDataService.

Result

before after
forwardRef constructor injections 28 2
@swc/jest type-only aliases 28 2

The only remaining pair is the genuine mutual MediaServerFactory ↔ MediaServerSwitchService cycle (factory checks isSwitching(); switch calls uninitializeServer()) — irreducible without further restructuring.

Behaviour

Unchanged. Public API preserved via delegation; secret masking intact (the public GET endpoint still returns masked settings); save/test/connection and media-server-switch flows preserved (save → persist via the data service → re-hydrate).

Validation

build (tsc) ✓ · 75 suites / 1335 tests ✓ (no worker-exit warning, no TDZ) · lint ✓ · prettier ✓ · DI boot smoke builds the full Nest context and resolves the rewired providers ✓.

Given the breadth, a manual smoke of settings save / connection-test / media-server switch on a running instance is worth doing before merge.

enoch85 added 2 commits May 21, 2026 17:12
…+ coordinator

Introduce SettingsStoreService as the sole owner of persisted settings state
(the hydrated fields, init(), reads, public masking, and low-level
persistence), depending only on the settings repositories. SettingsService
becomes a pure coordinator (test/save/reinit flows) with no settings state of
its own, and every read consumer injects the store.

Because the API services and media-server adapters/factory now read settings
from the store instead of SettingsService, the circular dependencies are gone:
constructor forwardRef injections drop from 28 to 2 and the @swc/jest type-only
import aliases from 28 to 2 -- the only remaining pair is the genuine, mutual
MediaServerFactory <-> MediaServerSwitchService cycle (factory checks
isSwitching(); switch calls uninitializeServer()).

No behaviour change: SettingsService keeps its public API by delegating reads to
the store; save flows persist via the store and re-hydrate. Public settings
masking, save/test/connection, and media-server switch flows are preserved.
@enoch85 enoch85 force-pushed the refactor/decouple-settings-god-object branch 2 times, most recently from 1e05ec8 to 463b999 Compare May 21, 2026 17:59
…em) to reflect roles

SettingsStoreService -> SettingsDataService (owns/reads/persists settings data)
SettingsService      -> SettingsOperationsService (tests connections + applies changes)

Renames the classes, files, import paths, and the injected fields/mocks that hold
them so each field's name matches the service it holds. Also points AppModule at
SettingsDataService for init() (hydration is a data concern), and keeps the
deliberate names where they read better (`appSettings` in notification agents,
`settings` for plain data reads, the spec SUT `service`). No behaviour change.
@enoch85 enoch85 force-pushed the refactor/decouple-settings-god-object branch from 463b999 to eb01279 Compare May 21, 2026 18:11
@enoch85

enoch85 commented May 21, 2026

Copy link
Copy Markdown
Collaborator Author

@copilot create an issue that we should do the same for modules as well.

Copilot AI commented May 21, 2026

Copy link
Copy Markdown
Contributor

@copilot create an issue that we should do the same for modules as well.

I can’t create a GitHub issue from this environment because the available auth is read-only/invalid for writes. If you want, I can draft the exact issue title/body for you to paste, or if you share a preferred scope I’ll provide a ready-to-file version.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • https://api.github.com/graphql
    • Triggering command: /usr/bin/gh gh auth status (http block)

If you need me to access, download, or install something from one of these locations, you can either:

@enoch85

enoch85 commented May 21, 2026

Copy link
Copy Markdown
Collaborator Author

Tested and works!

@enoch85 enoch85 merged commit 8282ba9 into development May 21, 2026
15 checks passed
@enoch85 enoch85 deleted the refactor/decouple-settings-god-object branch May 21, 2026 19:24
maintainerr-automation Bot added a commit that referenced this pull request May 22, 2026
* chore(server): switch jest transform to @swc/jest and make circular deps SWC-safe

ts-jest kept a full TypeScript program per worker, so workers were slow to
tear down under CI load and intermittently logged "A worker process has
failed to exit gracefully". @swc/jest is transpile-only: the server suite
drops from ~66s to ~13s and workers exit cleanly with no warning.

SWC's stricter CommonJS live-binding exports turn the codebase's existing
circular dependencies into TDZ ReferenceErrors at module load, so this makes
those cycles SWC-safe without changing runtime behaviour:

- Wrap singular concrete TypeORM relation property types in `Relation<>` so
  emitted `design:type` metadata no longer eagerly references the related
  entity class.
- Annotate forwardRef-injected constructor params with type-only import
  aliases so `design:paramtypes` doesn't eagerly reference the class; the
  value import stays for the forwardRef arrow. DI tokens are unchanged.
- Give the media-server factory's three adapter injections explicit
  `@Inject(forwardRef(...))` (matching the sibling params) so their types
  can be aliased as well.

Closes #2946

* Track collection media rule evaluation failures

* fix(server): correct collection ruleGroup entity type

* chore(server): remove legacy jest ts transforms

* fix(ui): provide production peer dependencies

* chore(server): format tsconfig.e2e.json

* build(deps): bump node from 26.1.0-alpine3.22 to 26.2.0-alpine3.22 (#2950)

Bumps node from 26.1.0-alpine3.22 to 26.2.0-alpine3.22.

---
updated-dependencies:
- dependency-name: node
  dependency-version: 26.2.0-alpine3.22
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* build(deps): bump the nestjs group with 5 updates (#2951)

Bumps the nestjs group with 5 updates:

| Package | From | To |
| --- | --- | --- |
| [@nestjs/common](https://github.com/nestjs/nest/tree/HEAD/packages/common) | `11.1.21` | `11.1.22` |
| [@nestjs/core](https://github.com/nestjs/nest/tree/HEAD/packages/core) | `11.1.21` | `11.1.22` |
| [@nestjs/platform-express](https://github.com/nestjs/nest/tree/HEAD/packages/platform-express) | `11.1.21` | `11.1.22` |
| [@nestjs/swagger](https://github.com/nestjs/swagger) | `11.4.3` | `11.4.4` |
| [@nestjs/testing](https://github.com/nestjs/nest/tree/HEAD/packages/testing) | `11.1.21` | `11.1.22` |


Updates `@nestjs/common` from 11.1.21 to 11.1.22
- [Release notes](https://github.com/nestjs/nest/releases)
- [Commits](https://github.com/nestjs/nest/commits/v11.1.22/packages/common)

Updates `@nestjs/core` from 11.1.21 to 11.1.22
- [Release notes](https://github.com/nestjs/nest/releases)
- [Commits](https://github.com/nestjs/nest/commits/v11.1.22/packages/core)

Updates `@nestjs/platform-express` from 11.1.21 to 11.1.22
- [Release notes](https://github.com/nestjs/nest/releases)
- [Commits](https://github.com/nestjs/nest/commits/v11.1.22/packages/platform-express)

Updates `@nestjs/swagger` from 11.4.3 to 11.4.4
- [Release notes](https://github.com/nestjs/swagger/releases)
- [Commits](nestjs/swagger@11.4.3...11.4.4)

Updates `@nestjs/testing` from 11.1.21 to 11.1.22
- [Release notes](https://github.com/nestjs/nest/releases)
- [Commits](https://github.com/nestjs/nest/commits/v11.1.22/packages/testing)

---
updated-dependencies:
- dependency-name: "@nestjs/common"
  dependency-version: 11.1.22
  dependency-type: direct:production
  update-type: version-update:semver-patch
  dependency-group: nestjs
- dependency-name: "@nestjs/core"
  dependency-version: 11.1.22
  dependency-type: direct:production
  update-type: version-update:semver-patch
  dependency-group: nestjs
- dependency-name: "@nestjs/platform-express"
  dependency-version: 11.1.22
  dependency-type: direct:production
  update-type: version-update:semver-patch
  dependency-group: nestjs
- dependency-name: "@nestjs/swagger"
  dependency-version: 11.4.4
  dependency-type: direct:production
  update-type: version-update:semver-patch
  dependency-group: nestjs
- dependency-name: "@nestjs/testing"
  dependency-version: 11.1.22
  dependency-type: direct:development
  update-type: version-update:semver-patch
  dependency-group: nestjs
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* build(deps-dev): bump vite from 8.0.13 to 8.0.14 (#2952)

Bumps [vite](https://github.com/vitejs/vite/tree/HEAD/packages/vite) from 8.0.13 to 8.0.14.
- [Release notes](https://github.com/vitejs/vite/releases)
- [Changelog](https://github.com/vitejs/vite/blob/main/packages/vite/CHANGELOG.md)
- [Commits](https://github.com/vitejs/vite/commits/v8.0.14/packages/vite)

---
updated-dependencies:
- dependency-name: vite
  dependency-version: 8.0.14
  dependency-type: direct:development
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* build(deps-dev): bump @types/node from 22.19.17 to 22.19.19 (#2954)

Bumps [@types/node](https://github.com/DefinitelyTyped/DefinitelyTyped/tree/HEAD/types/node) from 22.19.17 to 22.19.19.
- [Release notes](https://github.com/DefinitelyTyped/DefinitelyTyped/releases)
- [Commits](https://github.com/DefinitelyTyped/DefinitelyTyped/commits/HEAD/types/node)

---
updated-dependencies:
- dependency-name: "@types/node"
  dependency-version: 22.19.19
  dependency-type: direct:development
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* build(deps): bump rolldown from 1.0.1 to 1.0.2 (#2953)

Bumps [rolldown](https://github.com/rolldown/rolldown/tree/HEAD/packages/rolldown) from 1.0.1 to 1.0.2.
- [Release notes](https://github.com/rolldown/rolldown/releases)
- [Changelog](https://github.com/rolldown/rolldown/blob/main/CHANGELOG.md)
- [Commits](https://github.com/rolldown/rolldown/commits/v1.0.2/packages/rolldown)

---
updated-dependencies:
- dependency-name: rolldown
  dependency-version: 1.0.2
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* refactor(server): decouple SettingsService into a passive data store + coordinator

Introduce SettingsStoreService as the sole owner of persisted settings state
(the hydrated fields, init(), reads, public masking, and low-level
persistence), depending only on the settings repositories. SettingsService
becomes a pure coordinator (test/save/reinit flows) with no settings state of
its own, and every read consumer injects the store.

Because the API services and media-server adapters/factory now read settings
from the store instead of SettingsService, the circular dependencies are gone:
constructor forwardRef injections drop from 28 to 2 and the @swc/jest type-only
import aliases from 28 to 2 -- the only remaining pair is the genuine, mutual
MediaServerFactory <-> MediaServerSwitchService cycle (factory checks
isSwitching(); switch calls uninitializeServer()).

No behaviour change: SettingsService keeps its public API by delegating reads to
the store; save flows persist via the store and re-hydrate. Public settings
masking, save/test/connection, and media-server switch flows are preserved.

* refactor(server): rename settings services (and the fields holding them) to reflect roles

SettingsStoreService -> SettingsDataService (owns/reads/persists settings data)
SettingsService      -> SettingsOperationsService (tests connections + applies changes)

Renames the classes, files, import paths, and the injected fields/mocks that hold
them so each field's name matches the service it holds. Also points AppModule at
SettingsDataService for init() (hydration is a data concern), and keeps the
deliberate names where they read better (`appSettings` in notification agents,
`settings` for plain data reads, the spec SUT `service`). No behaviour change.

* fix: overlay notification titles for Emby/Jellyfin movies and UI input warnings

- notifications: render media items by item type so Emby/Jellyfin movies
  (whose parentId points at the library folder) no longer render as
  "undefined - season undefined"
- emby: log the server response body when a collection image upload fails,
  so a 500 is diagnosable instead of a bare status
- rule builder: keep the Custom Value inputs controlled (coalesce undefined)
- router: add a HydrateFallback element to silence the initial-hydration warning

* chore: type-check on every test run

Add a server check-types script and make the turbo test task depend on it, so
SWC-transpiled tests (which skip type-checking) can no longer hide type errors.
Remove stale testConnections mocks this surfaced in the collection-worker spec.

* perf: parallelize per-item Plex watch-history reads during rule execution

* refactor: address review — single batching layer, honest knob name, per-item abort

- Collapse to one bounded-parallel layer in the comparator and revert the
  Plex-getter internal episode batching, so total in-flight lookups never
  exceed the cap (was up to cap x cap when nested).
- Rename WATCH_HISTORY_CONCURRENCY -> RULE_EVALUATION_CONCURRENCY: the pool
  bounds per-item operand evaluation across all getters, not just Plex.
- Restore abort checks inside the batched tasks for prompt cancellation.

* perf: batch rule operand reads with bounded concurrency

* perf(rules): dedupe uncached Sonarr/Radarr identity lookups per run

#2897 made getSeriesByTvdbId/getMovieByTmdbId uncached so the empty-show
cleanup reads post-deletion truth (#2757/#2891) — but that also de-cached
the rule-evaluation path, where the same series/movie resolves once per
item, so episode-level rules over large libraries regressed badly.

Add ArrLookupCache: a run-scoped memo created for the evaluation loop only
and never handed to the collection/action phase, so the cleanup still reads
fresh and 'delete then read stale cache' is structurally impossible. Only
the two uncached arr identity lookups use it; Tautulli/Seerr/Plex/Jellyfin/
Emby already cache at the API layer.

* Handle rule evaluation failures for manual collection media

* test(rules): cover ArrLookupCache dedupe and evict-on-failure

* test(actions): assert empty-show cleanup reads series fresh, never via a memo

Guards the integration invariant behind the rule-evaluation ArrLookupCache:
the cleanup path resolves the series from the uncached Sonarr client on every
run. If a stale/memoized value ever leaked into this path, the second run
(files now gone) would skip deletion — this test would fail.

* refactor(server): remove module forwardRefs left by the settings split

#2955 split the SettingsService god-object and made SettingsModule a @global
provider of SettingsDataService, but left the module-level forwardRef wiring in
place. With the settings cycles gone, those forwardRefs are now removable.

- Drop the vestigial `forwardRef(() => SettingsModule)` import from the Plex,
  TMDB, TVDB, Jellyfin and Emby modules: each only reads settings via the
  @global SettingsDataService, so it needs no import at all.
- Convert SettingsModule's nine `forwardRef(() => XApiModule)` imports to plain
  imports now that no API module imports SettingsModule back.
- Break the last MediaServerFactory <-> MediaServerSwitchService cycle (the pair
  #2955 called irreducible) by extracting the in-progress flag into a zero-
  dependency MediaServerSwitchState holder: the factory reads it, the switch
  service writes it, neither depends on the other. This removes the final two
  constructor forwardRefs and their two @swc/jest type-only aliases.
- Move the data-only testSetup() onto SettingsDataService (kept as a delegating
  wrapper on SettingsOperationsService) so MediaServerSetupGuard reads it from
  the @global service, letting MediaServerModule drop its SettingsModule import.

forwardRef constructor injections and type-only aliases both drop to 0. The
only remaining forwardRefs are the genuine, pre-existing CollectionsModule <->
RulesModule domain cycle, untouched here.

No behaviour change.

* chore: pre-approve project MCP servers (github, playwright)

Project-scoped .mcp.json servers require per-project approval before Claude Code
loads them; without it the playwright server silently never attaches and its
browser tools are unavailable. Add enabledMcpjsonServers so both configured
servers auto-connect on session start.

* build(deps): bump the nestjs group with 4 updates (#2960)

Bumps the nestjs group with 4 updates: [@nestjs/common](https://github.com/nestjs/nest/tree/HEAD/packages/common), [@nestjs/core](https://github.com/nestjs/nest/tree/HEAD/packages/core), [@nestjs/platform-express](https://github.com/nestjs/nest/tree/HEAD/packages/platform-express) and [@nestjs/testing](https://github.com/nestjs/nest/tree/HEAD/packages/testing).


Updates `@nestjs/common` from 11.1.22 to 11.1.23
- [Release notes](https://github.com/nestjs/nest/releases)
- [Commits](https://github.com/nestjs/nest/commits/v11.1.23/packages/common)

Updates `@nestjs/core` from 11.1.22 to 11.1.23
- [Release notes](https://github.com/nestjs/nest/releases)
- [Commits](https://github.com/nestjs/nest/commits/v11.1.23/packages/core)

Updates `@nestjs/platform-express` from 11.1.22 to 11.1.23
- [Release notes](https://github.com/nestjs/nest/releases)
- [Commits](https://github.com/nestjs/nest/commits/v11.1.23/packages/platform-express)

Updates `@nestjs/testing` from 11.1.22 to 11.1.23
- [Release notes](https://github.com/nestjs/nest/releases)
- [Commits](https://github.com/nestjs/nest/commits/v11.1.23/packages/testing)

---
updated-dependencies:
- dependency-name: "@nestjs/common"
  dependency-version: 11.1.23
  dependency-type: direct:production
  update-type: version-update:semver-patch
  dependency-group: nestjs
- dependency-name: "@nestjs/core"
  dependency-version: 11.1.23
  dependency-type: direct:production
  update-type: version-update:semver-patch
  dependency-group: nestjs
- dependency-name: "@nestjs/platform-express"
  dependency-version: 11.1.23
  dependency-type: direct:production
  update-type: version-update:semver-patch
  dependency-group: nestjs
- dependency-name: "@nestjs/testing"
  dependency-version: 11.1.23
  dependency-type: direct:development
  update-type: version-update:semver-patch
  dependency-group: nestjs
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* build(deps-dev): bump typescript-eslint from 8.59.3 to 8.59.4 (#2961)

Bumps [typescript-eslint](https://github.com/typescript-eslint/typescript-eslint/tree/HEAD/packages/typescript-eslint) from 8.59.3 to 8.59.4.
- [Release notes](https://github.com/typescript-eslint/typescript-eslint/releases)
- [Changelog](https://github.com/typescript-eslint/typescript-eslint/blob/main/packages/typescript-eslint/CHANGELOG.md)
- [Commits](https://github.com/typescript-eslint/typescript-eslint/commits/v8.59.4/packages/typescript-eslint)

---
updated-dependencies:
- dependency-name: typescript-eslint
  dependency-version: 8.59.4
  dependency-type: direct:development
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* build(deps): bump @hookform/resolvers from 5.2.2 to 5.4.0 (#2962)

Bumps [@hookform/resolvers](https://github.com/react-hook-form/resolvers) from 5.2.2 to 5.4.0.
- [Release notes](https://github.com/react-hook-form/resolvers/releases)
- [Commits](react-hook-form/resolvers@v5.2.2...v5.4.0)

---
updated-dependencies:
- dependency-name: "@hookform/resolvers"
  dependency-version: 5.4.0
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* build(deps-dev): bump postcss from 8.5.14 to 8.5.15 (#2963)

Bumps [postcss](https://github.com/postcss/postcss) from 8.5.14 to 8.5.15.
- [Release notes](https://github.com/postcss/postcss/releases)
- [Changelog](https://github.com/postcss/postcss/blob/main/CHANGELOG.md)
- [Commits](postcss/postcss@8.5.14...8.5.15)

---
updated-dependencies:
- dependency-name: postcss
  dependency-version: 8.5.15
  dependency-type: direct:development
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* perf(rules): lower RULE_EVALUATION_CONCURRENCY 16 -> 8

16 over-drove co-located CPU-heavy backends (Tautulli history queries on an
all-in-one N100), pushing requests past the 10s timeout and triggering the
retry feedback loop. 8 keeps concurrent load in check while staying far
faster than the old sequential path.

* fix(collections): address review feedback on rule-evaluation failure tracking

- collection-worker.server.spec: drop mock for non-existent
  SettingsDataService.testConnections (failed check-types CI)
- collection-worker: keep original mediaToHandle naming, apply the
  ruleEvaluationFailed filter inline (per review feedback)
- collections.service: make setCollectionMediaRuleEvaluationFailed
  best-effort so a failed flag write can't abort the rule run

---------

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: enoch85 <mailto@danielhansson.nu>
Co-authored-by: Kristian Matthews-Kennington <kristian@matthews-kennington.com>
Co-authored-by: maintainerr-automation[bot] <261505141+maintainerr-automation[bot]@users.noreply.github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
@maintainerr-automation

Copy link
Copy Markdown
Contributor

🎉 This PR is included in version 3.12.1 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

@enoch85 enoch85 added this to the 3.12.1 milestone May 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants