Merge pull request 'fix(swagger): hoist the one inline predicate that makes the generator run away' (#164) from fix/swagger-generator-runaway into edge

Reviewed-on: #164

server/src/router/v1/admin/index.js

@@ -88,12 +88,13 @@ adminRouter.use('/modules', modulesRouter)
 // Voice channels (TEAMS.md §7.3, phase 9) are mounted at the more specific prefix
 // FIRST, so /teams/voice/* never reaches the teams router's `/:id`.
 //
-// They live out here rather than inside `teams.router.js` beside the bridge they
+// The voice routes live in their own file, and the mount is out here rather than
-// belong with, for a mechanical reason worth recording: that file sits exactly at
+// in that file, because phase 9 believed swagger-autogen enforced a per-file route
-// swagger-autogen's per-file limit. At twenty `teamsRouter.*` statements
+// limit that `teams.router.js` was sitting on. It does not: the generator's
-// `npm run swagger` dies with "invalid array length — heap out of memory"; at
+// runaway is triggered by an expression reaching `.test(` inside a route
-// nineteen it generates. One more statement of any shape tips it, a mount
+// statement, which that file had and has since had hoisted. The arrangement is
-// included, so the mount is here and the file keeps its nineteen.
+// kept on its own merits — voice is its own capability — but neither the split nor
+// the placement of this mount is load-bearing any more.
 adminRouter.use('/teams/voice', teamsVoiceRouter)
 adminRouter.use('/teams', teamsRouter)
 

server/src/router/v1/admin/teams.router.js

@@ -27,13 +27,25 @@ const teamsRouter = express.Router()
 // where a Team's events leave the site for is not the §2.9 kind of decision a
 // moderator files a request for; it is deployment configuration, and it sits with
 // the role that already holds the bot token.
-// Hoisted rather than written inline, and it has to stay that way: a regex
+// Both of these are hoisted rather than written inline, and both have to stay
-// LITERAL followed directly by `.test(` makes swagger-autogen's static parser run
+// that way. **Nothing that reaches `.test(` may sit inside a route statement**:
-// away, and `npm run swagger` dies with "invalid array length — heap out of
+// swagger-autogen's static parser runs away on it and `npm run swagger` dies with
-// memory" instead of generating a spec. Phase 8 shipped it inline and left the
+// "invalid array length - Allocation failed", generating no spec at all.
-// generator unable to run at all; the same regex reached through a const (the
+//
-// idiom `modules.router.js` already uses) parses fine.
+// Two rounds of that. Phase 8 shipped the regex as a bare LITERAL before `.test(`
+// and phase 9 hoisted it — but left the PREDICATE inline, which is the same
+// runaway even with the regex behind a const, and the generator stayed broken.
+// Bisected to exactly the `param('teamId').custom(...)` statement below: a
+// three-route file carrying only it dies, so this is not about how much is in the
+// file.
+//
+// That last point corrects what this file used to say. Phase 9 read the symptom
+// as a per-file ROUTE LIMIT, measured it at twenty `teamsRouter.*` statements,
+// split `teamsVoice.router.js` out and left this file sitting at nineteen "one
+// under the edge". There is no such edge to sit under — the count was a proxy for
+// how much text the parser chewed before hitting the real trigger.
 const TEAM_ID = /^[0-9]+$/
+const isTeamIdOrDefault = (v) => v === 'default' || TEAM_ID.test(v)
 
 const adminOnly = requireRole('admin')
 
@@ -181,7 +193,7 @@ teamsRouter.delete(
   /* #swagger.responses[200] = { description: 'Removed', content: { "application/json": { schema: { $ref: "#/components/schemas/OkResponse" } } } } */
   /* #swagger.responses[404] = { description: 'Nothing configured for that Team', content: { "application/json": { schema: { $ref: "#/components/schemas/Error" } } } } */
   adminOnly,
-  param('teamId').custom((v) => v === 'default' || TEAM_ID.test(v)),
+  param('teamId').custom(isTeamIdOrDefault),
   validate,
   ctrl.deleteIntegrationConfig,
 )

server/src/router/v1/admin/teamsVoice.router.js

@@ -7,18 +7,20 @@
 // in somebody's Discord guild, which is deployment configuration and not the §2.9
 // kind of decision a moderator files a request for.
 //
-// **Its own file for a mechanical reason, and the reason is worth recording.**
+// **Its own file for a mechanical reason that turned out to be misdiagnosed.**
 // These four routes belong beside the notification bridge's three in
-// `teams.router.js`, and they started there. That file sits exactly at
+// `teams.router.js`, and they started there. Phase 9 read `npm run swagger` dying
-// swagger-autogen's per-file limit: at twenty `teamsRouter.*` statements
+// with "invalid array length" as a per-file ROUTE LIMIT, measured it at twenty
-// `npm run swagger` dies with "invalid array length — heap out of memory", and at
+// `teamsRouter.*` statements, and split this file out to get under it.
-// nineteen it generates. ONE more statement of any shape tips it — a route with no
-// annotations at all does, and so does a bare `use`, which is why the mount is in
-// `admin/index.js` rather than here in the file it logically belongs to. The same
-// probe route added to `discordBot.router.js` generates fine, so the limit is
-// per-file and not tree-wide.
 //
-// So: if this file grows, split it again rather than moving it back.
+// The real trigger is content, not count: an expression reaching `.test(` inside
+// a route statement makes the parser run away, and `teams.router.js` had one in a
+// `param(...).custom((v) => ... .test(v))`. Hoisting that predicate fixes the
+// generator with the file at nineteen statements. See that file's own note.
+//
+// The split is kept because it is a good split on its own terms — voice channels
+// are their own capability and the file reads better for it — but if these routes
+// ever want to move back, nothing mechanical is stopping them.
 
 const express = require('express')
 const { body, param } = require('express-validator')