feat(modules): ctx additions and the post-hook registry (API 1.1.0)
Everything the extraction needed from core that ctx did not already offer. Additions only, so minor. ctx.activity.log, because an admin action a module performs has to land in core's one audit log or the trail has a hole exactly where a module operates the game -- a module keeping its own log would be a second place to look, which in practice means a place nobody looks. Write-only; reading the log is the admin panel's job and it spans every actor. ctx.users.getById, one function for one caller: the admin.users.detail slot router needs the user its prefix names. ctx.site.baseUrl, because a module has to build absolute links and §2.7 forbids it reading core's APP_BASE_URL -- a getter, not a captured string, so it cannot go stale against the env. ctx.middleware.rateLimit is core's makeLimiter, plus accountChangeLimiter handed over whole. The split is deliberate: a module states its own window and cap because it knows what its endpoints cost, and takes the plumbing from core so there is one express-rate-limit in the process and one place a breach is logged. accountChangeLimiter is shared policy -- core's /auth/me and /player/account sit behind the same counter -- so a module's account-change route has to land IN it rather than beside it. marketLimiter was UO policy living in core's file and leaves with the route it guards. registerPostHook is the fourth registry, and the last thing binding core to the module. Core's post controller called newsGump.syncPost directly: core's CMS naming a UO file. It now publishes what it already knows and a subscriber decides what to do with it. Not folded into registerAnnounceLeg, which fires on the same transition, because a leg is a one-shot DELIVERY with retry and classification while a post hook maintains idempotent STATE, runs on delete as well as save, and refreshes silently on an edit. Also fixes a real loader defect the extraction exposed: schema table names were matched against the RAW file, so a fragment whose header says "every CREATE TABLE carries IF NOT EXISTS" was rejected for a prefix violation on a table called `carries`. module-uo's fragment hit exactly that. Both scans now read split statements, which strip comments -- the same class of bug as a boundary check failing on its own documentation. Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
@@ -103,6 +103,9 @@ function buildCtx(id, moduleRoot) {
|
||||
const validate = require('../middleware/validate')
|
||||
const noindex = require('../middleware/noindex')
|
||||
const uploads = require('../router/v1/admin/imageUpload')
|
||||
const activity = require('../model/activity/activity.model')
|
||||
const users = require('../model/users/users.model')
|
||||
const { makeLimiter, accountChangeLimiter } = require('../middleware/rateLimit')
|
||||
/* eslint-enable global-require */
|
||||
|
||||
// Narrowed on purpose (§2.3): utils/auth also re-exports signToken,
|
||||
@@ -123,7 +126,26 @@ function buildCtx(id, moduleRoot) {
|
||||
auth: { getUserFromRequest: auth.getUserFromRequest },
|
||||
push: { publish: pushDispatch.publish },
|
||||
secretBox: { encrypt: secretBox.encrypt, decrypt: secretBox.decrypt },
|
||||
middleware: { requireAuth, requireRole, siteMode, validate, noindex },
|
||||
middleware: {
|
||||
requireAuth,
|
||||
requireRole,
|
||||
siteMode,
|
||||
validate,
|
||||
noindex,
|
||||
// Rate limiting, added in API 1.1.0 as a factory plus one shared limiter.
|
||||
//
|
||||
// `rateLimit(options)` is core's `makeLimiter`: a module states its own
|
||||
// window and cap — it knows what its endpoints cost — and takes the
|
||||
// plumbing from core, so there is one express-rate-limit in the process,
|
||||
// one store, and one place a breach is logged.
|
||||
//
|
||||
// `accountChangeLimiter` is handed over whole because it is genuinely
|
||||
// shared policy: core's `/auth/me`, `/player/account` and
|
||||
// `/player/appeals` are behind the same counter, and a module's
|
||||
// account-change route has to land in it rather than beside it.
|
||||
rateLimit: makeLimiter,
|
||||
accountChangeLimiter,
|
||||
},
|
||||
uploads,
|
||||
posts: {
|
||||
listAll: posts.listAll,
|
||||
@@ -131,6 +153,25 @@ function buildCtx(id, moduleRoot) {
|
||||
linkAnnounceJob: posts.linkAnnounceJob,
|
||||
markAnnounced: posts.markAnnounced,
|
||||
},
|
||||
// The admin audit log — WRITE only (§2.3, added in API 1.1.0). Core's one
|
||||
// audit trail has to include the admin actions a module performs, or the
|
||||
// trail has a hole exactly where a module operates the game. A module that
|
||||
// kept its own log would be a second place to look, which in practice means
|
||||
// a place nobody looks. `list` stays core's: reading the log is the admin
|
||||
// panel's job, and it spans every actor.
|
||||
activity: { log: activity.log },
|
||||
// One function, for one caller: the `admin.users.detail` slot router needs
|
||||
// the user its prefix names. Narrowed like `ctx.posts` — the users model
|
||||
// exports creation, role changes and password handling, none of which is a
|
||||
// module's business.
|
||||
users: { getById: users.getById },
|
||||
// Where this deployment is reachable, for a module that has to build an
|
||||
// absolute link (an announcement carries one into a game window or a chat
|
||||
// message, where a relative path means nothing). §2.7 forbids a module
|
||||
// reading `process.env` for core configuration and this is core
|
||||
// configuration, so core answers it. A getter, not a captured string: the
|
||||
// value is read per call, so it cannot go stale against the env.
|
||||
site: { get baseUrl() { return (process.env.APP_BASE_URL || 'http://localhost:5173').replace(/\/+$/, '') } },
|
||||
}
|
||||
// A guard against accident, not against a hostile module — the boundary is
|
||||
// organisational, not a security boundary (MODULE_SYSTEM.md §2.2).
|
||||
@@ -232,7 +273,12 @@ function coreTableNames() {
|
||||
coreTables = new Set()
|
||||
try {
|
||||
const sql = fs.readFileSync(path.join(__dirname, '..', '..', 'db', 'schema.sql'), 'utf8')
|
||||
for (const m of sql.matchAll(CREATE_TABLE)) coreTables.add(m[1].toLowerCase())
|
||||
// Split first, for the same reason tablesOf() does: matching the raw file
|
||||
// reads CREATE TABLE out of comments, and a phantom core table makes a
|
||||
// module fail with a collision against something that does not exist.
|
||||
for (const statement of splitStatements(sql)) {
|
||||
for (const m of statement.matchAll(CREATE_TABLE)) coreTables.add(m[1].toLowerCase())
|
||||
}
|
||||
} catch (err) {
|
||||
log.warn('could not read core schema for the table-collision check', { message: err.message })
|
||||
}
|
||||
@@ -272,8 +318,9 @@ function tablesOf(dir, manifest) {
|
||||
if (!manifest.schema) return new Set()
|
||||
const file = path.join(dir, manifest.schema)
|
||||
const sql = fs.readFileSync(file, 'utf8')
|
||||
const statements = splitStatements(sql)
|
||||
|
||||
for (const statement of splitStatements(sql)) {
|
||||
for (const statement of statements) {
|
||||
const verb = (statement.match(/^\w+/) || [''])[0].toUpperCase()
|
||||
if (!ALLOWED_VERBS.has(verb)) {
|
||||
throw new Error(`schema fragment statement starts with "${verb}" (allowed: ${[...ALLOWED_VERBS].join(', ')})`)
|
||||
@@ -285,7 +332,18 @@ function tablesOf(dir, manifest) {
|
||||
}
|
||||
}
|
||||
|
||||
return new Set([...sql.matchAll(CREATE_TABLE)].map((m) => m[1].toLowerCase()))
|
||||
// Scanned over the SPLIT STATEMENTS, never the raw file. `splitStatements`
|
||||
// strips `--` comments; the file does not, and a fragment that documents
|
||||
// itself will say "every CREATE TABLE carries IF NOT EXISTS" in its header.
|
||||
// Read raw, that yields a table called `carries`, and the module is rejected
|
||||
// for a prefix violation on a table that does not exist — a message with no
|
||||
// way back to the comment that caused it. module-uo's fragment hit exactly
|
||||
// this on its first real load.
|
||||
const tables = new Set()
|
||||
for (const statement of statements) {
|
||||
for (const m of statement.matchAll(CREATE_TABLE)) tables.add(m[1].toLowerCase())
|
||||
}
|
||||
return tables
|
||||
}
|
||||
|
||||
function checkTableNames(id, tables) {
|
||||
|
||||
@@ -52,6 +52,12 @@ const streamOwners = new Map() // stream id → owner id, for the collision mess
|
||||
// leg id → { owner, leg, label, dispatch, classify }
|
||||
const legs = new Map()
|
||||
|
||||
// owner -> { onSaved?, onDeleted? }. Post hooks (§1.8, API 1.1.0). A Map keyed by
|
||||
// owner rather than a flat list, so a registrant is a single subscription that
|
||||
// can be reported and reasoned about as one thing — and so registering twice is
|
||||
// a collision with a name attached rather than a silently doubled side effect.
|
||||
const postHooks = new Map()
|
||||
|
||||
let coreRegistered = false
|
||||
|
||||
// Stream ids that predate the module system and may not carry their owner's
|
||||
@@ -119,6 +125,18 @@ const filledSlots = () =>
|
||||
.filter(([, e]) => e.filledBy)
|
||||
.map(([slot, e]) => ({ slot, filledBy: e.filledBy, router: e.router, specFile: e.specFile || null }))
|
||||
|
||||
/**
|
||||
* A DECLARED slot's stable router, filled or not.
|
||||
*
|
||||
* `filledSlots()` answers what the build needs — a filled slot has a spec file
|
||||
* to generate a fragment from. This answers what a test needs: the slot exists
|
||||
* from the moment core declares it at require time, and its position in the
|
||||
* express stack has to stay findable whether or not a module has filled it.
|
||||
* Before Phase 3 the two questions had the same answer, because core filled the
|
||||
* only slot itself.
|
||||
*/
|
||||
const declaredSlotRouter = (slot) => (slots.get(slot) || {}).router || null
|
||||
|
||||
// ── Notification streams (§1.8) ────────────────────────────────────────────
|
||||
|
||||
/** The whole catalog, core's entries first, in registration order. */
|
||||
@@ -130,6 +148,43 @@ const isValidStream = (id) => streamOwners.has(id)
|
||||
/** Ids of the owner-keyed streams — those needing a linked game account. */
|
||||
const personalStreams = () => new Set(streams.filter((s) => s.personal).map((s) => s.id))
|
||||
|
||||
// ── Post hooks (§1.8) ──────────────────────────────────────────────────────
|
||||
|
||||
// Core's CMS is the only writer of posts, and a module may need to mirror one
|
||||
// somewhere core knows nothing about — module-uo keeps UO's in-game Town Cryer
|
||||
// News gump in step with it. Before this existed, core's post controller
|
||||
// required `utils/newsGump` directly, which is precisely the coupling the
|
||||
// extraction had to remove: core's publish path naming a UO file.
|
||||
//
|
||||
// It is deliberately NOT folded into `registerAnnounceLeg`, which fires on the
|
||||
// same transition. A leg is a one-shot DELIVERY with retry and classification;
|
||||
// a post hook maintains idempotent STATE, has to run on delete as well as save,
|
||||
// and refreshes silently on an edit. Overloading the leg would have meant a
|
||||
// dispatch that must not be retried and a classify that means nothing.
|
||||
|
||||
/** Every registered hook, in registration order. */
|
||||
const postHookEntries = () => [...postHooks.entries()].map(([owner, h]) => ({ owner, ...h }))
|
||||
|
||||
/**
|
||||
* Fire `event` at every registered hook, one at a time, never throwing.
|
||||
*
|
||||
* Best-effort by contract, and awaited rather than fired-and-forgotten: core's
|
||||
* own call site awaited `newsGump.syncPost` before this existed, so a save that
|
||||
* returns 200 still means the mirror was attempted. One subscriber's failure
|
||||
* must not cost another's, and none of them may cost the save — a sidecar
|
||||
* hiccup breaking a post edit would be a worse bug than a stale gump.
|
||||
*/
|
||||
async function dispatchPostHook(event, payload) {
|
||||
for (const { owner, [event]: fn } of postHookEntries()) {
|
||||
if (typeof fn !== 'function') continue
|
||||
try {
|
||||
await fn(payload)
|
||||
} catch (err) {
|
||||
log.warn('post hook failed', { owner, event, message: err.message })
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// ── Announce legs (§1.8) ───────────────────────────────────────────────────
|
||||
|
||||
/** Every registered leg, in registration order. */
|
||||
@@ -170,6 +225,22 @@ function checkLegShape(entry) {
|
||||
return { leg, label: label || leg, dispatch, classify }
|
||||
}
|
||||
|
||||
/**
|
||||
* `registerPostHook({ onSaved, onDeleted })` — both optional, at least one
|
||||
* required. A registration with neither is a subscription that can never fire,
|
||||
* which is a typo rather than an intention.
|
||||
*/
|
||||
function checkPostHookShape(entry) {
|
||||
const { onSaved, onDeleted } = entry || {}
|
||||
for (const [name, fn] of [['onSaved', onSaved], ['onDeleted', onDeleted]]) {
|
||||
if (fn !== undefined && typeof fn !== 'function') {
|
||||
throw new Error(`registerPostHook: ${name} must be a function`)
|
||||
}
|
||||
}
|
||||
if (!onSaved && !onDeleted) throw new Error('registerPostHook: needs onSaved or onDeleted')
|
||||
return { onSaved, onDeleted }
|
||||
}
|
||||
|
||||
// `specFile` is CORE-ONLY and is not on the module-facing signature. A slot's
|
||||
// router reaches the app through declareSlot(), which no static parse of app.js
|
||||
// can follow, so swagger-autogen would silently drop every route in it — the
|
||||
@@ -194,7 +265,7 @@ function checkExtensionShape(slot, router, specFile) {
|
||||
* `allStreams()` / `announceLeg()` / the slot routers until `apply()`.
|
||||
*/
|
||||
function stage(owner) {
|
||||
const staged = { owner, streams: [], legs: [], extensions: [] }
|
||||
const staged = { owner, streams: [], legs: [], extensions: [], postHooks: [] }
|
||||
return {
|
||||
staged,
|
||||
registerNotificationStreams(entries) {
|
||||
@@ -207,6 +278,9 @@ function stage(owner) {
|
||||
registerExtension(slot, router, specFile) {
|
||||
staged.extensions.push(checkExtensionShape(slot, router, specFile))
|
||||
},
|
||||
registerPostHook(entry) {
|
||||
staged.postHooks.push(checkPostHookShape(entry))
|
||||
},
|
||||
}
|
||||
}
|
||||
|
||||
@@ -219,7 +293,7 @@ function stage(owner) {
|
||||
* PR 2 learned to protect (mounting inside the scan loop made every collision
|
||||
* look like it was with core).
|
||||
*/
|
||||
function apply({ owner, streams: newStreams, legs: newLegs, extensions: newExtensions }) {
|
||||
function apply({ owner, streams: newStreams, legs: newLegs, extensions: newExtensions, postHooks: newPostHooks = [] }) {
|
||||
// ── validate ──
|
||||
const seenStreams = new Set()
|
||||
for (const s of newStreams) {
|
||||
@@ -253,6 +327,11 @@ function apply({ owner, streams: newStreams, legs: newLegs, extensions: newExten
|
||||
seenSlots.add(x.slot)
|
||||
}
|
||||
|
||||
if (newPostHooks.length > 1) throw new Error(`"${owner}" registered more than one post hook`)
|
||||
if (newPostHooks.length && postHooks.has(owner)) {
|
||||
throw new Error(`"${owner}" already registered a post hook`)
|
||||
}
|
||||
|
||||
// ── commit — nothing below can fail ──
|
||||
for (const s of newStreams) {
|
||||
streamOwners.set(s.id, owner)
|
||||
@@ -265,6 +344,7 @@ function apply({ owner, streams: newStreams, legs: newLegs, extensions: newExten
|
||||
entry.specFile = x.specFile
|
||||
entry.router.use(x.router)
|
||||
}
|
||||
for (const h of newPostHooks) postHooks.set(owner, h)
|
||||
}
|
||||
|
||||
// ── Core's own registrations ───────────────────────────────────────────────
|
||||
@@ -286,25 +366,18 @@ function registerCore() {
|
||||
/* eslint-disable global-require */
|
||||
const coreStreams = require('../config/coreStreams')
|
||||
const discordLeg = require('../utils/discordAnnounce')
|
||||
const shardStreams = require('../config/shardStreams')
|
||||
const townCrierLeg = require('../utils/shardAnnounce')
|
||||
const shardExtension = require('../router/v1/admin/usersShard.router')
|
||||
/* eslint-enable global-require */
|
||||
|
||||
const api = stage('core')
|
||||
api.registerNotificationStreams(coreStreams.STREAMS)
|
||||
api.registerAnnounceLeg(discordLeg.leg)
|
||||
|
||||
// ── Phase 3 boundary ────────────────────────────────────────────────────
|
||||
// These three lines become module-uo's register() body, with 'core' becoming
|
||||
// 'uo'. Nothing else in core has to change for that to happen — which is the
|
||||
// whole claim PR 4 is making.
|
||||
api.registerNotificationStreams(shardStreams.STREAMS)
|
||||
api.registerAnnounceLeg(townCrierLeg.leg)
|
||||
// The third argument is core-only and has no module counterpart — see
|
||||
// checkExtensionShape. A module ships a prebuilt swagger-fragment.json instead.
|
||||
api.registerExtension('admin.users.detail', shardExtension, require.resolve('../router/v1/admin/usersShard.router'))
|
||||
|
||||
// The three lines that used to follow — the shard stream catalog, the town
|
||||
// crier leg and the `admin.users.detail` filling — were shard CONTENT held
|
||||
// here so the seam would be exercised on every boot before a module first used
|
||||
// it. Phase 3 moved them into module-uo's `register()` verbatim, with 'core'
|
||||
// becoming 'uo', and nothing else in core changed. That was the claim PR 4
|
||||
// made, and this deletion is it being collected.
|
||||
apply(api.staged)
|
||||
coreRegistered = true
|
||||
|
||||
@@ -336,6 +409,7 @@ function _reset() {
|
||||
streams.length = 0
|
||||
streamOwners.clear()
|
||||
legs.clear()
|
||||
postHooks.clear()
|
||||
coreRegistered = false
|
||||
}
|
||||
|
||||
@@ -344,12 +418,15 @@ module.exports = {
|
||||
hasSlot,
|
||||
slotFilledBy,
|
||||
filledSlots,
|
||||
declaredSlotRouter,
|
||||
allStreams,
|
||||
isValidStream,
|
||||
personalStreams,
|
||||
announceLegs,
|
||||
announceLegIds,
|
||||
announceLeg,
|
||||
postHookEntries,
|
||||
dispatchPostHook,
|
||||
stage,
|
||||
apply,
|
||||
registerCore,
|
||||
|
||||
@@ -9,6 +9,11 @@
|
||||
// Deliberately separate from PROTOCOL_VERSION (which versions the shard wire and
|
||||
// has nothing to say about a website module) and from any module's own version.
|
||||
|
||||
const MODULE_API_VERSION = '1.0.0'
|
||||
// 1.1.0 — `ctx` gained `activity.log`, `users.getById` and `site.baseUrl`, each
|
||||
// because module-uo's extraction needed it and none of them could be vendored:
|
||||
// an admin action a module performs belongs in core's one audit log, the
|
||||
// extension slot needs the user its prefix names, and §2.7 forbids a module
|
||||
// reading core's `APP_BASE_URL` for itself. Additions only, so minor.
|
||||
const MODULE_API_VERSION = '1.1.0'
|
||||
|
||||
module.exports = { MODULE_API_VERSION }
|
||||
|
||||
Reference in New Issue
Block a user