From 5c6a4373852e17e5d02013eeb19fcf822e88450c Mon Sep 17 00:00:00 2001 From: Matt Simerson Date: Fri, 24 Jul 2026 09:20:12 -0700 Subject: [PATCH 1/4] fix: subgroup search --- CHANGELOG.md | 3 ++- lib/zone/store/mysql.js | 19 ++++++++++++++++++- lib/zone/store/toml.js | 6 ++++-- routes/group.js | 23 +++++++---------------- routes/group.test.js | 14 +++++++++----- routes/zone.js | 27 +++++++++++++++++++++++++++ routes/zone.test.js | 33 ++++++++++++++++++++++++++++++++- 7 files changed, 99 insertions(+), 26 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 306596f..c1e8fd8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,8 +6,9 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/). ### Unreleased -### [3.0.0-alpha.12] - 2026-04-13 +### [3.0.0-alpha.12] - 2026-07-24 +- fix: subgroup searching - toml backend (#49) - add: TOML stores for group, nameserver, permission, session (#47) - move mysql teardown/disconnect into mysql classes diff --git a/lib/zone/store/mysql.js b/lib/zone/store/mysql.js index 3dfe1f2..d6a1b78 100644 --- a/lib/zone/store/mysql.js +++ b/lib/zone/store/mysql.js @@ -5,6 +5,15 @@ import { mapToDbColumn } from '../../util.js' const zoneDbMap = { id: 'nt_zone_id', gid: 'nt_group_id' } const boolFields = ['deleted'] +// include_subgroups passes gid as a list of group ids; filter with IN(...). +function applyGidList(query, params, gidList) { + if (!gidList) return [query, params] + const connector = /\bWHERE\b/.test(query) ? ' AND' : ' WHERE' + if (gidList.length === 0) return [`${query}${connector} nt_group_id IN (NULL)`, params] + const placeholders = gidList.map(() => '?').join(', ') + return [`${query}${connector} nt_group_id IN (${placeholders})`, [...params, ...gidList]] +} + function applyZoneFilters(query, params, filters = {}) { let nextQuery = query const nextParams = [...params] @@ -54,6 +63,9 @@ class ZoneRepoMySQL extends ZoneBase { args = JSON.parse(JSON.stringify(args)) args.deleted = args.deleted ?? false + const gidList = Array.isArray(args.gid) ? args.gid : null + if (gidList) delete args.gid + const filters = { search: args.search, zone_like: args.zone_like, @@ -101,6 +113,7 @@ class ZoneRepoMySQL extends ZoneBase { ) let [finalQuery, finalParams] = applyZoneFilters(query, params, filters) + ;[finalQuery, finalParams] = applyGidList(finalQuery, finalParams, gidList) finalQuery += ` ORDER BY ${sortBy} ${sortDir}` const rows = await Mysql.execute(`${finalQuery}${sqlLimit}`, finalParams) @@ -138,6 +151,9 @@ class ZoneRepoMySQL extends ZoneBase { args = JSON.parse(JSON.stringify(args)) args.deleted = args.deleted ?? false + const gidList = Array.isArray(args.gid) ? args.gid : null + if (gidList) delete args.gid + const filters = { search: args.search, zone_like: args.zone_like, @@ -153,7 +169,8 @@ class ZoneRepoMySQL extends ZoneBase { mapToDbColumn(args, zoneDbMap), ) - const [finalQuery, finalParams] = applyZoneFilters(query, params, filters) + let [finalQuery, finalParams] = applyZoneFilters(query, params, filters) + ;[finalQuery, finalParams] = applyGidList(finalQuery, finalParams, gidList) const rows = await Mysql.execute(finalQuery, finalParams) return rows?.[0]?.total ?? 0 } diff --git a/lib/zone/store/toml.js b/lib/zone/store/toml.js index bcc3f6e..f9d941c 100644 --- a/lib/zone/store/toml.js +++ b/lib/zone/store/toml.js @@ -78,7 +78,8 @@ class ZoneRepoTOML extends ZoneBase { // Direct field filters if (id !== undefined) zones = zones.filter((z) => z.id === id) - if (gid !== undefined) zones = zones.filter((z) => z.gid === gid) + if (Array.isArray(gid)) zones = zones.filter((z) => gid.includes(z.gid)) + else if (gid !== undefined) zones = zones.filter((z) => z.gid === gid) if (zone !== undefined) zones = zones.filter((z) => z.zone === zone) if (deletedArg === false) zones = zones.filter((z) => !z.deleted) else if (deletedArg !== undefined) zones = zones.filter((z) => Boolean(z.deleted) === Boolean(deletedArg)) @@ -130,7 +131,8 @@ class ZoneRepoTOML extends ZoneBase { let zones = await this._load() if (id !== undefined) zones = zones.filter((z) => z.id === id) - if (gid !== undefined) zones = zones.filter((z) => z.gid === gid) + if (Array.isArray(gid)) zones = zones.filter((z) => gid.includes(z.gid)) + else if (gid !== undefined) zones = zones.filter((z) => z.gid === gid) if (deletedArg === false) zones = zones.filter((z) => !z.deleted) else if (deletedArg !== undefined) zones = zones.filter((z) => Boolean(z.deleted) === Boolean(deletedArg)) diff --git a/routes/group.js b/routes/group.js index 0f5febc..d835f40 100644 --- a/routes/group.js +++ b/routes/group.js @@ -51,23 +51,14 @@ function GroupRoutes(server) { include_subgroups: request.query.include_subgroups === true, }) - if (groups.length !== 1 && !request.query.include_subgroups) { - return h - .response({ - meta: { - api: meta.api, - msg: `No unique group match`, - }, - }) - .code(204) - } - + // Return an array like the other object types (zone/nameserver/user/ + // zone_record) rather than a bare object, for a consistent API contract. return h .response({ - group: request.query.include_subgroups ? groups : groups[0], + group: groups, meta: { api: meta.api, - msg: `here's your group`, + msg: `here's your group(s)`, }, }) .code(200) @@ -92,7 +83,7 @@ function GroupRoutes(server) { return h .response({ - group: groups[0], + group: groups, meta: { api: meta.api, msg: `I created this group`, @@ -121,7 +112,7 @@ function GroupRoutes(server) { return h .response({ - group: groups[0], + group: groups, meta: { api: meta.api, msg: `I updated this group`, @@ -177,7 +168,7 @@ function GroupRoutes(server) { return h .response({ - group: groups[0], + group: groups, meta: { api: meta.api, msg: `I deleted that group`, diff --git a/routes/group.test.js b/routes/group.test.js index 3387557..9275ab8 100644 --- a/routes/group.test.js +++ b/routes/group.test.js @@ -45,7 +45,8 @@ describe('group routes', () => { url: `/group/${groupCase.id}`, headers: auth.headers, }) - assert.ok([200, 204].includes(res.statusCode)) + assert.equal(res.statusCode, 200) + assert.equal(res.result.group[0].id, groupCase.id) }) it('POST /group', async () => { @@ -69,7 +70,8 @@ describe('group routes', () => { url: `/group/${case2Id}`, headers: auth.headers, }) - assert.ok([200, 204].includes(res.statusCode)) + assert.equal(res.statusCode, 200) + assert.equal(res.result.group[0].id, case2Id) }) it(`DELETE /group/${case2Id}`, async () => { @@ -81,13 +83,14 @@ describe('group routes', () => { assert.equal(res.statusCode, 200) }) - it(`GET /group/${case2Id}`, async () => { + it(`GET /group/${case2Id} (soft-deleted → empty array)`, async () => { const res = await server.inject({ method: 'GET', url: `/group/${case2Id}`, headers: auth.headers, }) - assert.equal(res.statusCode, 204) + assert.equal(res.statusCode, 200) + assert.deepEqual(res.result.group, []) }) it(`GET /group/${case2Id} (deleted)`, async () => { @@ -96,7 +99,8 @@ describe('group routes', () => { url: `/group/${case2Id}?deleted=true`, headers: auth.headers, }) - assert.ok([200, 204].includes(res.statusCode)) + assert.equal(res.statusCode, 200) + assert.ok(Array.isArray(res.result.group)) }) it('DELETE /session', async () => { diff --git a/routes/zone.js b/routes/zone.js index 7f45426..b640110 100644 --- a/routes/zone.js +++ b/routes/zone.js @@ -1,9 +1,32 @@ import validate from '@nictool/validate' import Zone from '../lib/zone/index.js' +import Group from '../lib/group/index.js' import Mysql from '../lib/mysql.js' import { meta } from '../lib/util.js' +// Expand a group id to itself plus every descendant subgroup id, so a zone +// query with include_subgroups spans the whole branch. +async function subgroupGids(rootGid) { + const groups = await Group.get({ deleted: 0 }) + const childrenOf = new Map() + for (const g of groups) { + if (!childrenOf.has(g.parent_gid)) childrenOf.set(g.parent_gid, []) + childrenOf.get(g.parent_gid).push(g.id) + } + const gids = [] + const seen = new Set() + const stack = [rootGid] + while (stack.length) { + const gid = stack.pop() + if (seen.has(gid)) continue + seen.add(gid) + gids.push(gid) + for (const child of childrenOf.get(gid) ?? []) stack.push(child) + } + return gids +} + function ZoneRoutes(server) { server.route([ { @@ -38,6 +61,10 @@ function ZoneRoutes(server) { if (request.query.sort_by) getArgs.sort_by = request.query.sort_by if (request.query.sort_dir) getArgs.sort_dir = request.query.sort_dir + if (request.query.include_subgroups === true && getArgs.gid) { + getArgs.gid = await subgroupGids(getArgs.gid) + } + const countArgs = { deleted, ...(getArgs.id ? { id: getArgs.id } : {}), diff --git a/routes/zone.test.js b/routes/zone.test.js index 0f128d6..8419a41 100644 --- a/routes/zone.test.js +++ b/routes/zone.test.js @@ -13,17 +13,24 @@ import nsCase from './test/zone.json' with { type: 'json' } let server let case2Id = 4094 +const subGroup = { id: 4090, parent_gid: groupCase.id, name: 'sub.route.example.com' } +const subZone = { ...nsCase, id: 4091, gid: subGroup.id, zone: 'sub.route.example.com.' } + before(async () => { await Zone.destroy({ id: nsCase.id }) await Zone.destroy({ id: case2Id }) + await Zone.destroy({ id: subZone.id }) await Group.create(groupCase) await User.create(userCase) await Zone.create(nsCase) + await Group.create(subGroup) + await Zone.create(subZone) server = await init() }) after(async () => { - // await Zone.destroy({ id: case2Id }) + await Zone.destroy({ id: subZone.id }) + await Group.destroy({ id: subGroup.id }) server.stop() }) @@ -134,6 +141,30 @@ describe('zone routes', () => { assert.ok(res.result.zone) }) + it('GET /zone?gid= excludes subgroup zones by default', async () => { + const res = await server.inject({ + method: 'GET', + url: `/zone?gid=${groupCase.id}`, + headers: auth.headers, + }) + assert.equal(res.statusCode, 200) + const zones = res.result.zone.map((z) => z.zone) + assert.ok(zones.includes(nsCase.zone)) + assert.ok(!zones.includes(subZone.zone)) + }) + + it('GET /zone?gid=&include_subgroups=true spans the branch', async () => { + const res = await server.inject({ + method: 'GET', + url: `/zone?gid=${groupCase.id}&include_subgroups=true`, + headers: auth.headers, + }) + assert.equal(res.statusCode, 200) + const zones = res.result.zone.map((z) => z.zone) + assert.ok(zones.includes(nsCase.zone), 'parent zone present') + assert.ok(zones.includes(subZone.zone), 'subgroup zone present') + }) + it('DELETE /session', async () => { const res = await server.inject({ method: 'DELETE', From 086c494b9a29057dfc446f4b97783450a38c43f8 Mon Sep 17 00:00:00 2001 From: Matt Simerson Date: Fri, 24 Jul 2026 09:24:16 -0700 Subject: [PATCH 2/4] bump to .13 --- CHANGELOG.md | 6 +++++- package.json | 2 +- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c1e8fd8..08697eb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,9 +6,13 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/). ### Unreleased -### [3.0.0-alpha.12] - 2026-07-24 +### [3.0.0-alpha.13] - 2026-07-24 - fix: subgroup searching +- feat: paginate zone records (#53) + +### [3.0.0-alpha.12] - 2026-04-14 + - toml backend (#49) - add: TOML stores for group, nameserver, permission, session (#47) - move mysql teardown/disconnect into mysql classes diff --git a/package.json b/package.json index c46c7d8..bd80ccf 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@nictool/api", - "version": "3.0.0-alpha.12", + "version": "3.0.0-alpha.13", "description": "NicTool API", "main": "index.js", "type": "module", From edf4580e15f800ae8e130096ad04023cb9a88ae3 Mon Sep 17 00:00:00 2001 From: Matt Simerson Date: Fri, 24 Jul 2026 10:16:21 -0700 Subject: [PATCH 3/4] fix: mysql 8 has special keyword rank - fix(test): harden tests against concurrency - dep(validate): bump ver --- .github/workflows/ci.yml | 13 +++---------- CHANGELOG.md | 1 + lib/group/store/base.js | 5 +++++ lib/group/store/mysql.js | 31 ++++++++++++++++++++++++------- lib/group/store/toml.js | 5 +++++ lib/group/test/index.js | 8 +++++++- lib/mysql.js | 9 +++++++-- lib/user/test/index.js | 20 +++++++++++++++++--- package.json | 4 ++-- routes/group.test.js | 2 +- routes/index.js | 11 ++++++----- routes/nameserver.test.js | 2 +- routes/zone.js | 24 +----------------------- routes/zone.test.js | 6 +++++- test/fixtures.js | 7 +++++++ 15 files changed, 92 insertions(+), 56 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 2378a1e..4c2af38 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -6,10 +6,11 @@ on: push: branches: [main] +permissions: + contents: read + jobs: lint: - permissions: - contents: read uses: NicTool/.github/.github/workflows/lint.yml@main get-lts: @@ -27,8 +28,6 @@ jobs: min: ${{ steps.get.outputs.min }} test: - permissions: - contents: read needs: [ get-lts ] runs-on: ${{ matrix.os }} strategy: @@ -47,8 +46,6 @@ jobs: - run: npm test test-mac: - permissions: - contents: read needs: [ get-lts ] runs-on: macos-latest strategy: @@ -70,8 +67,6 @@ jobs: test-docker: runs-on: ubuntu-latest - permissions: - contents: read steps: - uses: actions/checkout@v6 - name: Generate .env @@ -88,8 +83,6 @@ jobs: run: docker compose -f docker/docker-compose.yml down -v test-win: - permissions: - contents: read needs: [ get-lts ] runs-on: windows-latest strategy: diff --git a/CHANGELOG.md b/CHANGELOG.md index 08697eb..5fa87f7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/). ### [3.0.0-alpha.13] - 2026-07-24 +- fix(sql): quote mysql 8 keyword rank - fix: subgroup searching - feat: paginate zone records (#53) diff --git a/lib/group/store/base.js b/lib/group/store/base.js index 3a0daf4..11bfd56 100644 --- a/lib/group/store/base.js +++ b/lib/group/store/base.js @@ -10,6 +10,7 @@ * put(args) → boolean * delete(args) → boolean * destroy(args) → boolean + * subgroupGids(rootGid) → number[] (rootGid plus every descendant) */ class GroupBase { constructor(args = {}) { @@ -43,6 +44,10 @@ class GroupBase { async destroy(_args) { throw new Error('destroy() not implemented by this repo') } + + async subgroupGids(_rootGid) { + throw new Error('subgroupGids() not implemented by this repo') + } } export default GroupBase diff --git a/lib/group/store/mysql.js b/lib/group/store/mysql.js index 104743d..396f1a0 100644 --- a/lib/group/store/mysql.js +++ b/lib/group/store/mysql.js @@ -23,7 +23,11 @@ class Group extends GroupBase { const parent_gid = args.parent_gid ?? 0 - const gid = await Mysql.execute(...Mysql.insert(`nt_group`, mapToDbColumn(args, groupDbMap))) + const insertId = await Mysql.execute(...Mysql.insert(`nt_group`, mapToDbColumn(args, groupDbMap))) + // MySQL 8.0 returns insertId 0 for an explicit-id insert into an + // AUTO_INCREMENT column, so fall back to the supplied id; otherwise the + // subgroup + permission wiring below runs against the wrong gid (or is skipped). + const gid = args.id ?? insertId if (gid && parent_gid !== 0) { await this.addToSubgroups(gid, parent_gid) @@ -41,12 +45,11 @@ class Group extends GroupBase { async addToSubgroups(gid, parent_gid, rank = 1000) { if (!parent_gid || parent_gid === 0) return + // `rank` is a reserved word in MySQL 8.0+, so it must be backticked; the + // generic Mysql.insert helper doesn't quote column names. await Mysql.execute( - ...Mysql.insert('nt_group_subgroups', { - nt_group_id: parent_gid, - nt_subgroup_id: gid, - rank, - }), + 'INSERT INTO nt_group_subgroups (nt_group_id,nt_subgroup_id,`rank`) VALUES(?,?,?)', + [parent_gid, gid, rank], ) const parent = await this.get({ id: parent_gid }) @@ -124,6 +127,16 @@ class Group extends GroupBase { return groups } + // rootGid plus every descendant subgroup id, from the nt_group_subgroups + // closure table (the same source the include_subgroups get() uses). + async subgroupGids(rootGid) { + const rows = await Mysql.execute( + 'SELECT nt_subgroup_id FROM nt_group_subgroups WHERE nt_group_id = ?', + [rootGid], + ) + return [rootGid, ...rows.map((r) => r.nt_subgroup_id)] + } + async put(args) { if (!args.id) return false const id = args.id @@ -160,8 +173,12 @@ class Group extends GroupBase { } async destroy(args) { - // Clean up associated permission rows before removing the group + // Clean up associated permission and subgroup-closure rows before removing the group await Mysql.execute(`DELETE FROM nt_perm WHERE nt_group_id = ? AND nt_user_id IS NULL`, [args.id]) + await Mysql.execute(`DELETE FROM nt_group_subgroups WHERE nt_group_id = ? OR nt_subgroup_id = ?`, [ + args.id, + args.id, + ]) const r = await Mysql.execute(...Mysql.delete(`nt_group`, { nt_group_id: args.id })) return r.affectedRows === 1 } diff --git a/lib/group/store/toml.js b/lib/group/store/toml.js index 9394102..3d5873f 100644 --- a/lib/group/store/toml.js +++ b/lib/group/store/toml.js @@ -72,6 +72,11 @@ class GroupRepoTOML extends GroupBase { return ids } + async subgroupGids(rootGid) { + const groups = await this._load() + return [rootGid, ...this._collectSubgroupIds(groups, rootGid)] + } + async create(args) { args = JSON.parse(JSON.stringify(args)) diff --git a/lib/group/test/index.js b/lib/group/test/index.js index 5c418ba..d39ab54 100644 --- a/lib/group/test/index.js +++ b/lib/group/test/index.js @@ -3,9 +3,15 @@ import { describe, it, after, before } from 'node:test' import Group from '../index.js' -import testCase from '../test/group.json' with { type: 'json' } +import groupJson from '../test/group.json' with { type: 'json' } + +// This suite renames and soft-deletes its group, so it uses a private group +// rather than the shared fixture that the concurrently-run user and permission +// suites depend on staying live and named. +const testCase = { ...groupJson, id: 4088, name: 'grouptest.example.com' } after(async () => { + await Group.destroy({ id: testCase.id }) Group.disconnect() }) diff --git a/lib/mysql.js b/lib/mysql.js index 4c8928a..ee219ff 100644 --- a/lib/mysql.js +++ b/lib/mysql.js @@ -89,8 +89,13 @@ class Mysql { async disconnect(dbh) { const d = dbh || this.dbh - if (_debug) console.log(`MySQL connection id ${d.connection.connectionId}`) - if (d) await d.end() + if (!d) return + // Forget the singleton handle first so a concurrent/second disconnect is a + // no-op and the next execute() reconnects instead of reusing a dead handle. + if (!dbh) this.dbh = null + if (d.connection?._closing) return + if (_debug) console.log(`MySQL connection id ${d.connection?.connectionId}`) + await d.end() } debug(val) { diff --git a/lib/user/test/index.js b/lib/user/test/index.js index 8159437..b24872b 100644 --- a/lib/user/test/index.js +++ b/lib/user/test/index.js @@ -4,14 +4,28 @@ import { describe, it, after, before } from 'node:test' import User from '../index.js' import Group from '../../group/index.js' -import userCase from './user.json' with { type: 'json' } -import groupCase from '../../group/test/group.json' with { type: 'json' } +import userJson from './user.json' with { type: 'json' } +import groupJson from '../../group/test/group.json' with { type: 'json' } + +// This suite soft-deletes and restores its user, so it uses private fixtures +// rather than the shared 4096 user/group that the concurrently-run permission +// and session suites depend on staying present. +const userCase = { + ...userJson, + id: 4085, + gid: 4085, + username: 'unit-test-lib', + email: 'unit-test-lib@example.com', +} +const groupCase = { ...groupJson, id: 4085, name: 'usertest.example.com' } before(async () => { await Group.create(groupCase) }) after(async () => { + await User.destroy({ id: userCase.id }) + await Group.destroy({ id: groupCase.id }) await User.disconnect() }) @@ -49,7 +63,7 @@ describe('user', function () { }) it('finds existing user by username', async () => { - const u = await User.get({ username: 'unit-test' }) + const u = await User.get({ username: userCase.username }) assert.deepEqual(sanitizeActual(u[0]), sanitize(userCase)) }) }) diff --git a/package.json b/package.json index bd80ccf..c400d43 100644 --- a/package.json +++ b/package.json @@ -54,14 +54,14 @@ }, "dependencies": { "@hapi/cookie": "^12.0.1", - "@hapi/hapi": "^21.4.9", + "@hapi/hapi": "^21.4.10", "@hapi/hoek": "^11.0.7", "@hapi/inert": "^7.1.2", "@hapi/jwt": "^3.2.4", "@hapi/vision": "^7.0.3", "@msimerson/hapi-openapi": "^18.0.0", "@nictool/dns-resource-record": "^1.8.0", - "@nictool/validate": "^0.9.0", + "@nictool/validate": "^0.9.1", "joi": "^18.2.3", "mysql2": "^3.23.1", "qs": "^6.15.3", diff --git a/routes/group.test.js b/routes/group.test.js index 9275ab8..8b83199 100644 --- a/routes/group.test.js +++ b/routes/group.test.js @@ -19,7 +19,7 @@ before(async () => { after(async () => { await Group.destroy({ id: case2Id }) - server.stop() + await server.stop() }) describe('group routes', () => { diff --git a/routes/index.js b/routes/index.js index 404d1df..598f092 100644 --- a/routes/index.js +++ b/routes/index.js @@ -153,6 +153,12 @@ async function init() { async function start() { await setup() + // Fail fast on an unhandled rejection when running as the server. Not + // installed by tests + process.on('unhandledRejection', (err) => { + console.error(err) + process.exit(1) + }) /* c8 ignore next 3 */ await server.start() console.log(`API running at: ${server.info.uri}`) @@ -161,11 +167,6 @@ async function start() { export { init, start } -process.on('unhandledRejection', (err) => { - console.error(err) - process.exit(1) -}) - /* server.route({ method: 'POST', // GET PUT POST DELETE diff --git a/routes/nameserver.test.js b/routes/nameserver.test.js index 0deed2c..5103fbb 100644 --- a/routes/nameserver.test.js +++ b/routes/nameserver.test.js @@ -23,7 +23,7 @@ before(async () => { after(async () => { await Nameserver.destroy({ id: case2Id }) - server.stop() + await server.stop() }) describe('nameserver routes', () => { diff --git a/routes/zone.js b/routes/zone.js index b640110..a3462d0 100644 --- a/routes/zone.js +++ b/routes/zone.js @@ -5,28 +5,6 @@ import Group from '../lib/group/index.js' import Mysql from '../lib/mysql.js' import { meta } from '../lib/util.js' -// Expand a group id to itself plus every descendant subgroup id, so a zone -// query with include_subgroups spans the whole branch. -async function subgroupGids(rootGid) { - const groups = await Group.get({ deleted: 0 }) - const childrenOf = new Map() - for (const g of groups) { - if (!childrenOf.has(g.parent_gid)) childrenOf.set(g.parent_gid, []) - childrenOf.get(g.parent_gid).push(g.id) - } - const gids = [] - const seen = new Set() - const stack = [rootGid] - while (stack.length) { - const gid = stack.pop() - if (seen.has(gid)) continue - seen.add(gid) - gids.push(gid) - for (const child of childrenOf.get(gid) ?? []) stack.push(child) - } - return gids -} - function ZoneRoutes(server) { server.route([ { @@ -62,7 +40,7 @@ function ZoneRoutes(server) { if (request.query.sort_dir) getArgs.sort_dir = request.query.sort_dir if (request.query.include_subgroups === true && getArgs.gid) { - getArgs.gid = await subgroupGids(getArgs.gid) + getArgs.gid = await Group.subgroupGids(getArgs.gid) } const countArgs = { diff --git a/routes/zone.test.js b/routes/zone.test.js index 8419a41..86c3eb7 100644 --- a/routes/zone.test.js +++ b/routes/zone.test.js @@ -20,6 +20,10 @@ before(async () => { await Zone.destroy({ id: nsCase.id }) await Zone.destroy({ id: case2Id }) await Zone.destroy({ id: subZone.id }) + // Destroy the subgroup before recreating it: a lingering row would make + // Group.create early-return and skip addToSubgroups, leaving the + // nt_group_subgroups closure row (and thus the include_subgroups query) empty. + await Group.destroy({ id: subGroup.id }) await Group.create(groupCase) await User.create(userCase) await Zone.create(nsCase) @@ -31,7 +35,7 @@ before(async () => { after(async () => { await Zone.destroy({ id: subZone.id }) await Group.destroy({ id: subGroup.id }) - server.stop() + await server.stop() }) describe('zone routes', () => { diff --git a/test/fixtures.js b/test/fixtures.js index e6d3484..5e8b4e1 100644 --- a/test/fixtures.js +++ b/test/fixtures.js @@ -16,6 +16,7 @@ import zoneCase from '../lib/zone/test/zone.json' with { type: 'json' } // import zrCase from '../lib/zone_record/test/zone_record.json' with { type: 'json' } import groupCaseR from '../routes/test/group.json' with { type: 'json' } import userCaseR from '../routes/test/user.json' with { type: 'json' } +import permCaseR from '../routes/test/permission.json' with { type: 'json' } import nsCaseR from '../routes/test/nameserver.json' with { type: 'json' } switch (process.argv[2]) { @@ -34,7 +35,12 @@ async function setup() { await Group.create(groupCaseR) await User.create(userCase) await User.create(userCaseR) + // Seed the shared route permission so the permission and session suites, + // which both create it in their before hooks, early-return instead of racing + // two concurrent INSERTs of the same explicit id. + await Permission.create(permCaseR) // await createTestSession() + await Permission.disconnect() await User.disconnect() await Group.disconnect() process.exit(0) @@ -55,6 +61,7 @@ async function teardown() { await Nameserver.destroy({ id: nsCaseR.id - 1 }) await Permission.destroy({ id: userCase.id }) await Permission.destroy({ id: userCase.id - 1 }) + await Permission.destroy({ id: permCaseR.id }) await Session.delete({ nt_user_id: userCase.id }) await User.destroy({ id: userCase.id }) await User.destroy({ id: userCaseR.id }) From 668864b0f316d6d35534746b20ff4ff47441e15e Mon Sep 17 00:00:00 2001 From: Matt Simerson Date: Fri, 24 Jul 2026 14:03:28 -0700 Subject: [PATCH 4/4] test: add diagnostics for mysql 9.6 --- routes/zone_record.test.js | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/routes/zone_record.test.js b/routes/zone_record.test.js index 0b6ab70..5e96845 100644 --- a/routes/zone_record.test.js +++ b/routes/zone_record.test.js @@ -152,6 +152,16 @@ describe('zone_record routes', () => { }) assert.equal(res.statusCode, 200) + if (res.result.zone_record.length !== 2) { + const all = await ZoneRecord.get({ zid: testZoneId }) + console.error('DIAGZR createdIds=', JSON.stringify(createdZoneRecordIds)) + console.error('DIAGZR limit-query result=', JSON.stringify(res.result.zone_record)) + console.error('DIAGZR filtered=', res.result.meta.pagination.filtered) + console.error( + 'DIAGZR all-records-for-zid=', + JSON.stringify(all.map((r) => ({ id: r.id, zid: r.zid, owner: r.owner, deleted: r.deleted }))), + ) + } assert.equal(res.result.zone_record.length, 2) assert.equal(res.result.zone_record[0].owner, `${token}0.route-zr-delete.example.com.`) assert.equal(res.result.meta.pagination.filtered, 3)