Skip to content

Added RouteSettingsService to centralize route settings lifecycle - #28965

Merged
vershwal merged 2 commits into
mainfrom
princi-hkg-1828-create-routesettingsservice-and-wire-boot-path
Jun 30, 2026
Merged

Added RouteSettingsService to centralize route settings lifecycle#28965
vershwal merged 2 commits into
mainfrom
princi-hkg-1828-create-routesettingsservice-and-wire-boot-path

Conversation

@vershwal

@vershwal vershwal commented Jun 29, 2026

Copy link
Copy Markdown
Member

ref https://linear.app/tryghost/issue/HKG-1828/

Summary

Introduces RouteSettingsService as the single orchestrator for route settings lifecycle, replacing scattered module-level variables and direct routerManager.start() calls in boot.js.

  • configure({settingsLoader, routeSettings}) — wires storage-layer deps early in boot (during initServicesForFrontend), so the API surface (get, setFromFilePath, getCurrentHash) works immediately — even when the frontend is disabled
  • start({routerManager, urlService}) — wires routing deps and loads settings into the router (during initDynamicRouting), only when the frontend is enabled
  • All existing module getters (loadRouteSettings, getDefaultHash, api.*) now delegate through the service — no caller changes needed

This is the first step toward swapping the file-based settings loader for a pluggable store without touching callers.

Changes

File What changed
route-settings-service.js New — RouteSettingsService class with configure(), start(), and delegating API methods
route-settings/index.js Module-level let vars replaced with a service singleton; init() calls service.configure(); new service getter exposed
boot.js initDynamicRouting() calls service.start() instead of routerManager.start() directly
api-vs-frontend.test.js Test stubs updated from module getter pattern (.get(() => () => ...)) to direct service method stubs (.resolves(...))

Test plan

  • test/unit/server/services/route-settings/ — 37 tests pass
  • test/unit/server/data/schema/integrity.test.js — 1 test passes
  • test/e2e-api/admin/settings-files.test.js — 2 tests pass (download + upload routes.yaml)
  • test/e2e-api/admin/settings.test.js — 30 tests pass
  • test/legacy/mock-express-style/api-vs-frontend.test.js — 60 tests pass (all frontend routing configs)
  • Lint clean

ref https://linear.app/tryghost/issue/HKG-1828/

- the route-settings module previously used module-level variables to hold settingsLoader and routeSettings, with boot.js directly calling routerManager.start() — scattering lifecycle orchestration across files
- RouteSettingsService consolidates this: configure() wires storage deps early in boot (so the API surface works even without frontend), and start() wires routing deps and loads settings into the router
- this is the first step toward swapping the file-based settings loader for a pluggable store without touching callers
- updated test stubs from module getter pattern to direct service method stubs
@coderabbitai

coderabbitai Bot commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 351a5df5-b687-4ea1-a047-4ae8639f804d

📥 Commits

Reviewing files that changed from the base of the PR and between 92b8222 and 357019b.

📒 Files selected for processing (2)
  • ghost/core/core/server/services/route-settings/dynamic-routing-service.js
  • ghost/core/core/server/services/route-settings/index.js
🚧 Files skipped from review as they are similar to previous changes (1)
  • ghost/core/core/server/services/route-settings/index.js

Walkthrough

A new DynamicRoutingService class is introduced in dynamic-routing-service.js with configure({ settingsLoader, routeSettings }) and start({ routerManager, urlService }) methods, plus passthrough accessors for hash/get/set operations. The route-settings index.js is updated to instantiate this service at module scope, wire it via configure in init, and delegate all exported accessors and api methods to the service. In boot.js, initDynamicRouting is updated to call routeSettingsModule.service.start(...) directly and read the route hash via routeSettingsModule.service.getCurrentHash(). Test stubs are updated from a nested get(() => () => ({})) pattern to sinon.stub(...).resolves({}).

Suggested reviewers

  • jonatansberg
  • rob-ghost
🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title matches the main change: centralizing route settings lifecycle behind a service.
Description check ✅ Passed The description clearly describes the route settings service refactor and matches the files changed.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch princi-hkg-1828-create-routesettingsservice-and-wire-boot-path

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@nx-cloud

nx-cloud Bot commented Jun 29, 2026

Copy link
Copy Markdown

🤖 Nx Cloud AI Fix

Ensure the fix-ci command is configured to always run in your CI pipeline to get automatic fixes in future runs. For more information, please see https://nx.dev/ci/features/self-healing-ci


View your CI Pipeline Execution ↗ for commit 357019b

Command Status Duration Result
nx run ghost:test:ci:integration ✅ Succeeded 2m 37s View ↗
nx run ghost:test:integration ✅ Succeeded 2m 41s View ↗
nx run ghost:test:legacy ✅ Succeeded 2m 52s View ↗
nx run ghost:test:e2e ✅ Succeeded 2m 25s View ↗
nx run-many --target=build --projects=tag:publi... ✅ Succeeded 1s View ↗
nx run-many -t test:unit -p ghost ✅ Succeeded 28s View ↗
nx run-many -t lint -p ghost ✅ Succeeded 35s View ↗
nx run @tryghost/admin:build ✅ Succeeded 6s View ↗
Additional runs (2) ✅ Succeeded ... View ↗

💡 Verify your cache is correct by running tasks in a sandbox. Read docs ↗


☁️ Nx Cloud last updated this comment at 2026-06-30 08:19:43 UTC

@vershwal
vershwal requested a review from allouis June 29, 2026 17:00
return this.settingsLoader.loadSettings();
}

getDefaultHash() {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

It's worth seeing if you can delete this, pretty sure it's only used in tests, and is unnecessary. Not for this PR tho!

- The service orchestrates more than just settings — it owns the full
  dynamic routing lifecycle (load settings, start routers, manage hashes).
  DynamicRoutingService better reflects this responsibility.
- Renamed the file from route-settings-service.js to dynamic-routing-service.js
  to match the class name.
@vershwal
vershwal enabled auto-merge (squash) June 30, 2026 08:14
@vershwal
vershwal merged commit 9164a53 into main Jun 30, 2026
42 checks passed
@vershwal
vershwal deleted the princi-hkg-1828-create-routesettingsservice-and-wire-boot-path branch June 30, 2026 08:22
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.

2 participants