diff --git a/.github/workflows/coverage.yml b/.github/workflows/coverage.yml index 695f13c..ca0aa54 100644 --- a/.github/workflows/coverage.yml +++ b/.github/workflows/coverage.yml @@ -14,3 +14,4 @@ jobs: contents: read with: mysql: true + data-stores: mysql,json diff --git a/CHANGELOG.md b/CHANGELOG.md index b71da26..915db80 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/). - fix: GET /zone/{id}/ns reads nameservers through the zone store - fix(mysql): read v2 group permission rows, and save partial permission changes - fix: file store login checks the group in user@group +- create: allocate ids internally and reject caller-supplied ids ### [3.0.3] - 2026-07-27 diff --git a/lib/group/store/file.js b/lib/group/store/file.js index b2680cd..76f244c 100644 --- a/lib/group/store/file.js +++ b/lib/group/store/file.js @@ -1,4 +1,4 @@ -import FileStore from '../../store/file.js' +import FileStore, { claimId, removeRows } from '../../store/file.js' import { idConflict } from '../../store/error.js' import GroupBase from './base.js' @@ -23,10 +23,6 @@ class GroupRepoFile extends GroupBase { return this.file.load('group') } - async _save(groups) { - return this.file.save('group', groups) - } - _postProcess(row, deletedArg) { const r = JSON.parse(JSON.stringify(row)) r.deleted = Boolean(r.deleted) @@ -60,37 +56,34 @@ class GroupRepoFile extends GroupBase { async create(args, options) { args = JSON.parse(JSON.stringify(args)) - - if (args.id !== undefined) { - const existing = [ - ...(await this.get({ id: args.id })), - ...(await this.get({ id: args.id, deleted: true })), - ] - if (existing.length > 0) return idConflict('group', args.id, options) - } - const usable_ns = args.usable_ns ?? [] delete args.usable_ns - const gid = args.id - args.permissions = { - ...JSON.parse(JSON.stringify(defaultPermissions)), - id: gid, - name: `Group ${args.name} perms`, - user: { id: gid, create: false, write: false, delete: false }, - group: { id: gid, create: false, write: false, delete: false }, - nameserver: { - usable: Array.isArray(usable_ns) ? usable_ns : [], - create: false, - write: false, - delete: false, - }, - } + return this.file.mutate('group', (groups, data, keep) => { + if (args.id !== undefined && groups.some((group) => group.id === args.id)) { + return keep(idConflict('group', args.id, options)) + } - const groups = await this._load() - groups.push(args) - await this._save(groups) - return gid + args.id = claimId(groups, data, args.id) + + const gid = args.id + args.permissions = { + ...JSON.parse(JSON.stringify(defaultPermissions)), + id: gid, + name: `Group ${args.name} perms`, + user: { id: gid, create: false, write: false, delete: false }, + group: { id: gid, create: false, write: false, delete: false }, + nameserver: { + usable: Array.isArray(usable_ns) ? usable_ns : [], + create: false, + write: false, + delete: false, + }, + } + + groups.push(args) + return gid + }) } async get(args_orig) { @@ -129,42 +122,35 @@ class GroupRepoFile extends GroupBase { const usable_ns = args.usable_ns delete args.usable_ns - const groups = await this._load() - const idx = groups.findIndex((g) => g.id === id) - if (idx === -1) return false + return this.file.mutate('group', (groups) => { + const idx = groups.findIndex((g) => g.id === id) + if (idx === -1) return false - if (usable_ns !== undefined && groups[idx].permissions) { - groups[idx].permissions.nameserver = { - ...groups[idx].permissions.nameserver, - usable: Array.isArray(usable_ns) ? usable_ns : [], + if (usable_ns !== undefined && groups[idx].permissions) { + groups[idx].permissions.nameserver = { + ...groups[idx].permissions.nameserver, + usable: Array.isArray(usable_ns) ? usable_ns : [], + } } - } - - if (Object.keys(args).length > 0) { - groups[idx] = { ...groups[idx], ...args } - } - await this._save(groups) - return true + if (Object.keys(args).length > 0) { + groups[idx] = { ...groups[idx], ...args } + } + return true + }) } async delete(args) { - const groups = await this._load() - const idx = groups.findIndex((g) => g.id === args.id) - if (idx === -1) return false - - groups[idx].deleted = args.deleted ?? true - await this._save(groups) - return true + return this.file.mutate('group', (groups) => { + const row = groups.find((g) => g.id === args.id) + if (!row) return false + row.deleted = args.deleted ?? true + return true + }) } async destroy(args) { - const groups = await this._load() - const before = groups.length - const filtered = groups.filter((g) => g.id !== args.id) - if (filtered.length === before) return false - await this._save(filtered) - return true + return this.file.mutate('group', (groups) => removeRows(groups, (g) => g.id === args.id) > 0) } } diff --git a/lib/nameserver/store/file.js b/lib/nameserver/store/file.js index 40c16b0..8598c41 100644 --- a/lib/nameserver/store/file.js +++ b/lib/nameserver/store/file.js @@ -1,4 +1,4 @@ -import FileStore from '../../store/file.js' +import FileStore, { claimId, removeRows } from '../../store/file.js' import { idConflict } from '../../store/error.js' import NameserverBase from './base.js' @@ -18,10 +18,6 @@ class NameserverRepoFile extends NameserverBase { return this.file.load('nameserver') } - async _save(nameservers) { - return this.file.save('nameserver', nameservers) - } - _postProcess(row, deletedArg) { const r = JSON.parse(JSON.stringify(row)) @@ -40,18 +36,17 @@ class NameserverRepoFile extends NameserverBase { } async create(args, options) { - if (args.id !== undefined) { - const existing = [ - ...(await this.get({ id: args.id })), - ...(await this.get({ id: args.id, deleted: true })), - ] - if (existing.length > 0) return idConflict('nameserver', args.id, options) - } + args = JSON.parse(JSON.stringify(args)) - const nameservers = await this._load() - nameservers.push(JSON.parse(JSON.stringify(args))) - await this._save(nameservers) - return args.id + return this.file.mutate('nameserver', (nameservers, data, keep) => { + if (args.id !== undefined && nameservers.some((nameserver) => nameserver.id === args.id)) { + return keep(idConflict('nameserver', args.id, options)) + } + + args.id = claimId(nameservers, data, args.id, 0xffff) + nameservers.push(args) + return args.id + }) } async get(args) { @@ -72,32 +67,28 @@ class NameserverRepoFile extends NameserverBase { async put(args) { if (!args.id) return false - const nameservers = await this._load() - const idx = nameservers.findIndex((n) => n.id === args.id) - if (idx === -1) return false - - nameservers[idx] = { ...nameservers[idx], ...JSON.parse(JSON.stringify(args)) } - await this._save(nameservers) - return true + return this.file.mutate('nameserver', (nameservers) => { + const idx = nameservers.findIndex((n) => n.id === args.id) + if (idx === -1) return false + nameservers[idx] = { ...nameservers[idx], ...JSON.parse(JSON.stringify(args)) } + return true + }) } async delete(args) { - const nameservers = await this._load() - const idx = nameservers.findIndex((n) => n.id === args.id) - if (idx === -1) return false - - nameservers[idx].deleted = args.deleted ?? true - await this._save(nameservers) - return true + return this.file.mutate('nameserver', (nameservers) => { + const row = nameservers.find((n) => n.id === args.id) + if (!row) return false + row.deleted = args.deleted ?? true + return true + }) } async destroy(args) { - const nameservers = await this._load() - const before = nameservers.length - const filtered = nameservers.filter((n) => n.id !== args.id) - if (filtered.length === before) return false - await this._save(filtered) - return true + return this.file.mutate( + 'nameserver', + (nameservers) => removeRows(nameservers, (n) => n.id === args.id) > 0, + ) } } diff --git a/lib/permission/store/file.js b/lib/permission/store/file.js index 44fc215..8c68da9 100644 --- a/lib/permission/store/file.js +++ b/lib/permission/store/file.js @@ -1,5 +1,5 @@ -import FileStore from '../../store/file.js' -import { idConflict } from '../../store/error.js' +import FileStore, { serialize } from '../../store/file.js' +import { idConflict, StoreInvalidError } from '../../store/error.js' import PermissionBase from './base.js' @@ -12,14 +12,14 @@ import PermissionBase from './base.js' * record. Looked up via users[i].permissions.id === N. * * 2. Inline in the group file — group-level permissions created by the group - * store. Looked up via groups[i].permissions.id === N. + * store. Looked up by gid only, as their ids collide with users'. * * 3. Standalone permission file — fallback for permission IDs that reference * users/groups not present in the user / group files. * * get({ uid }) → inline permissions of that user * get({ gid }) → inline permissions of that group (uid absent) - * get({ id }) → search user → group → standalone by permissions.id + * get({ id }) → search user → standalone by permissions.id * getGroup({ uid }) → permissions of the group the user belongs to */ class PermissionRepoFile extends PermissionBase { @@ -34,26 +34,14 @@ class PermissionRepoFile extends PermissionBase { return this.userFile.load('user') } - async _saveUsers(users) { - return this.userFile.save('user', users) - } - async _loadGroups() { return this.groupFile.load('group') } - async _saveGroups(groups) { - return this.groupFile.save('group', groups) - } - async _loadStandalone() { return this.standaloneFile.load('permission') } - async _saveStandalone(permissions) { - return this.standaloneFile.save('permission', permissions) - } - // --------------------------------------------------------------------------- // Post-processing // --------------------------------------------------------------------------- @@ -74,12 +62,19 @@ class PermissionRepoFile extends PermissionBase { // CRUD // --------------------------------------------------------------------------- + // one at a time, so an id checked as free is still free when it's written async create(args, options) { + return serialize('permission create', () => this._create(args, options)) + } + + async _create(args, options) { args = JSON.parse(JSON.stringify(args)) - const uid = args.uid ?? args.user?.id - const gid = args.gid ?? args.group?.id + const uid = args.uid ?? args.user?.id ?? undefined + const gid = args.gid ?? args.group?.id ?? undefined delete args.uid delete args.gid + if (uid === undefined && gid === undefined) + throw new StoreInvalidError('a permission needs a user or group') if (args.id !== undefined) { const [users, groups, permissions] = await Promise.all([ @@ -92,8 +87,8 @@ class PermissionRepoFile extends PermissionBase { ...groups.map((group) => ({ permission: group.permissions, gid: group.id })), ...permissions.map((permission) => ({ permission, - uid: permission.uid ?? permission.user?.id, - gid: permission.gid ?? permission.group?.id, + uid: permission.uid ?? permission.user?.id ?? undefined, + gid: permission.gid ?? permission.group?.id ?? undefined, })), ].find((entry) => entry.permission?.id === args.id) if (existing) { @@ -108,57 +103,50 @@ class PermissionRepoFile extends PermissionBase { } if (uid !== undefined) { - const users = await this._loadUsers() - const idx = users.findIndex((u) => u.id === uid) - - if (idx !== -1) { - if (users[idx].permissions) return idConflict('permission', users[idx].permissions.id, options) + const created = await this.userFile.mutate('user', (users, data, keep) => { + const user = users.find((u) => u.id === uid) + if (!user) return false + if (user.permissions) return keep(idConflict('permission', user.permissions.id, options)) const perm = JSON.parse(JSON.stringify(args)) perm.id = args.id ?? uid if (!perm.user) perm.user = {} perm.user.id = uid if (!perm.group) perm.group = {} - perm.group.id = gid ?? users[idx].gid - users[idx].permissions = perm - await this._saveUsers(users) - return users[idx].permissions.id - } - - // User not found — fall through to standalone storage + perm.group.id = gid ?? user.gid + user.permissions = perm + return perm.id + }) + if (created !== false) return created } if (gid !== undefined && uid === undefined) { - const groups = await this._loadGroups() - const idx = groups.findIndex((g) => g.id === gid) - - if (idx !== -1) { - if (groups[idx].permissions) return idConflict('permission', groups[idx].permissions.id, options) + const created = await this.groupFile.mutate('group', (groups, data, keep) => { + const group = groups.find((g) => g.id === gid) + if (!group) return false + if (group.permissions) return keep(idConflict('permission', group.permissions.id, options)) const perm = JSON.parse(JSON.stringify(args)) perm.id = args.id ?? gid if (!perm.group) perm.group = {} perm.group.id = gid - groups[idx].permissions = perm - await this._saveGroups(groups) - return groups[idx].permissions.id - } - - // Group not found — fall through to standalone storage + group.permissions = perm + return perm.id + }) + if (created !== false) return created } // Standalone fallback: neither user nor group record found const permId = args.id ?? uid ?? gid - if (permId === undefined) return undefined - const perms = await this._loadStandalone() - if (perms.some((permission) => permission.id === permId)) { - return idConflict('permission', permId, options) - } - const perm = { ...args, id: permId } - if (uid !== undefined) perm.uid = uid - if (gid !== undefined) perm.gid = gid - perms.push(perm) - await this._saveStandalone(perms) - return permId + return this.standaloneFile.mutate('permission', (perms, data, keep) => { + if (perms.some((permission) => permission.id === permId)) { + return keep(idConflict('permission', permId, options)) + } + const perm = { ...args, id: permId } + if (uid !== undefined) perm.uid = uid + if (gid !== undefined) perm.gid = gid + perms.push(perm) + return permId + }) } async get(args) { @@ -185,11 +173,8 @@ class PermissionRepoFile extends PermissionBase { } if (args.id !== undefined) { - // Search user.toml by permissions.id. - // NOTE: group.toml is intentionally NOT searched here — group permissions are - // accessed via { gid }. Searching groups would cause false positives after a - // user permission is destroyed, because user and group share the same numeric - // id space (both user 4096 and group 4096 set permissions.id = 4096). + // not groups: user and group permissions share ids, so a group's would + // answer for a destroyed user's. Group permissions are reached by gid. const users = await this._loadUsers() const user = users.find((u) => u.permissions?.id === args.id) if (user?.permissions) { @@ -225,70 +210,39 @@ class PermissionRepoFile extends PermissionBase { return this._postProcess(group.permissions, deletedArg) } + // finds the permission where get({ id }) does, under that file's lock + async _change(id, change) { + const changed = await this.userFile.mutate('user', (users) => { + const user = users.find((u) => u.permissions?.id === id) + if (!user) return false + change(user, 'permissions') + return true + }) + if (changed) return true + return this.standaloneFile.mutate('permission', (perms) => { + const idx = perms.findIndex((p) => p.id === id) + if (idx === -1) return false + change(perms, idx) + return true + }) + } + async put(args) { args = JSON.parse(JSON.stringify(args)) if (!args.id) return false const id = args.id delete args.id - - const users = await this._loadUsers() - const uidx = users.findIndex((u) => u.permissions?.id === id) - if (uidx !== -1) { - users[uidx].permissions = deepMerge(users[uidx].permissions, args) - await this._saveUsers(users) - return true - } - - const groups = await this._loadGroups() - const gidx = groups.findIndex((g) => g.permissions?.id === id) - if (gidx !== -1) { - groups[gidx].permissions = deepMerge(groups[gidx].permissions, args) - await this._saveGroups(groups) - return true - } - - // Check standalone - const perms = await this._loadStandalone() - const pidx = perms.findIndex((p) => p.id === id) - if (pidx !== -1) { - perms[pidx] = deepMerge(perms[pidx], args) - await this._saveStandalone(perms) - return true - } - - return false + return this._change(id, (holder, key) => { + holder[key] = deepMerge(holder[key], args) + }) } async delete(args) { if (!args.id) return false const deletedVal = args.deleted ?? true - - const users = await this._loadUsers() - const uidx = users.findIndex((u) => u.permissions?.id === args.id) - if (uidx !== -1) { - users[uidx].permissions.deleted = deletedVal - await this._saveUsers(users) - return true - } - - const groups = await this._loadGroups() - const gidx = groups.findIndex((g) => g.permissions?.id === args.id) - if (gidx !== -1) { - groups[gidx].permissions.deleted = deletedVal - await this._saveGroups(groups) - return true - } - - // Check standalone - const perms = await this._loadStandalone() - const pidx = perms.findIndex((p) => p.id === args.id) - if (pidx !== -1) { - perms[pidx].deleted = deletedVal - await this._saveStandalone(perms) - return true - } - - return false + return this._change(args.id, (holder, key) => { + holder[key].deleted = deletedVal + }) } disconnect() { @@ -297,33 +251,10 @@ class PermissionRepoFile extends PermissionBase { async destroy(args) { if (!args.id) return false - - const users = await this._loadUsers() - const uidx = users.findIndex((u) => u.permissions?.id === args.id) - if (uidx !== -1) { - delete users[uidx].permissions - await this._saveUsers(users) - return true - } - - const groups = await this._loadGroups() - const gidx = groups.findIndex((g) => g.permissions?.id === args.id) - if (gidx !== -1) { - delete groups[gidx].permissions - await this._saveGroups(groups) - return true - } - - // Check standalone - const perms = await this._loadStandalone() - const before = perms.length - const filtered = perms.filter((p) => p.id !== args.id) - if (filtered.length < before) { - await this._saveStandalone(filtered) - return true - } - - return false + return this._change(args.id, (holder, key) => { + if (Array.isArray(holder)) holder.splice(key, 1) + else delete holder[key] + }) } } diff --git a/lib/permission/store/mysql.js b/lib/permission/store/mysql.js index f5e73b6..a0281f2 100644 --- a/lib/permission/store/mysql.js +++ b/lib/permission/store/mysql.js @@ -1,5 +1,5 @@ import Mysql from '../../mysql.js' -import { idConflict } from '../../store/error.js' +import { idConflict, StoreInvalidError } from '../../store/error.js' import { mapToDbColumn } from '../../util.js' import PermissionBase from './base.js' @@ -19,6 +19,11 @@ class PermissionRepoMySQL extends PermissionBase { async create(args, options) { args = objectToDb(args) + if (args.uid === null) delete args.uid + if (args.gid === null) delete args.gid + if (args.uid === undefined && args.gid === undefined) { + throw new StoreInvalidError('a permission needs a user or group') + } if (args.id !== undefined) { const p = (await this.get({ id: args.id })) ?? (await this.get({ id: args.id, deleted: true })) diff --git a/lib/permission/test/mysql.js b/lib/permission/test/mysql.js index 5e322a1..9297c92 100644 --- a/lib/permission/test/mysql.js +++ b/lib/permission/test/mysql.js @@ -16,4 +16,10 @@ describe('permission (mysql)', function () { assert.equal(p?.id, 1) assert.equal(p.user.id, null) }) + + it('refuses a second group permission sent with a null user id', async () => { + await assert.rejects(Permission.create({ user: { id: null }, group: { id: 1 }, name: 'twice' }), { + code: 'STORE_CONFLICT', + }) + }) }) diff --git a/lib/session/store/file.js b/lib/session/store/file.js index d3d959f..2c2eb16 100644 --- a/lib/session/store/file.js +++ b/lib/session/store/file.js @@ -1,4 +1,4 @@ -import FileStore from '../../store/file.js' +import FileStore, { claimId, removeRows } from '../../store/file.js' // Map legacy nt_* column names to the friendly API names used throughout. function normalizeArgs(args) { @@ -26,22 +26,19 @@ class SessionRepoFile { return this.file.load('session') } - async _save(sessions) { - return this.file.save('session', sessions) - } - async create(args) { args = normalizeArgs(JSON.parse(JSON.stringify(args))) - const existing = await this.get({ uid: args.uid, session: args.session }) - if (existing) return existing.id + return this.file.mutate('session', (sessions, data, keep) => { + const existing = sessions.find( + (session) => session.uid === args.uid && session.session === args.session, + ) + if (existing) return keep(existing.id) - const sessions = await this._load() - const nextId = sessions.reduce((max, s) => Math.max(max, s.id ?? 0), 0) + 1 - args.id = nextId - sessions.push(args) - await this._save(sessions) - return nextId + args.id = claimId(sessions, data) + sessions.push(args) + return args.id + }) } async get(args) { @@ -68,22 +65,22 @@ class SessionRepoFile { const now = parseInt(Date.now() / 1000, 10) if (s.last_access > now - 60) return true - const sessions = await this._load() - const idx = sessions.findIndex((s) => s.id === args.id) - if (idx === -1) return false - sessions[idx].last_access = now - await this._save(sessions) - return true + return this.file.mutate('session', (sessions) => { + const row = sessions.find((s) => s.id === args.id) + if (!row) return false + row.last_access = now + return true + }) } - const sessions = await this._load() - const idx = sessions.findIndex((s) => s.id === args.id) - if (idx === -1) return false const id = args.id delete args.id - sessions[idx] = { ...sessions[idx], ...args, id } - await this._save(sessions) - return true + return this.file.mutate('session', (sessions) => { + const idx = sessions.findIndex((s) => s.id === id) + if (idx === -1) return false + sessions[idx] = { ...sessions[idx], ...args, id } + return true + }) } /** @@ -93,20 +90,16 @@ class SessionRepoFile { async delete(args) { args = normalizeArgs(JSON.parse(JSON.stringify(args))) - const sessions = await this._load() - const before = sessions.length - - const filtered = sessions.filter((s) => { - // Keep this session unless every provided filter matches it - if (args.id !== undefined && s.id !== args.id) return true - if (args.uid !== undefined && s.uid !== args.uid) return true - if (args.session !== undefined && s.session !== args.session) return true - return false // all conditions matched → remove - }) - - if (filtered.length === before) return false - await this._save(filtered) - return true + return this.file.mutate( + 'session', + (sessions) => + removeRows(sessions, (s) => { + if (args.id !== undefined && s.id !== args.id) return false + if (args.uid !== undefined && s.uid !== args.uid) return false + if (args.session !== undefined && s.session !== args.session) return false + return true + }) > 0, + ) } disconnect() { diff --git a/lib/store/error.js b/lib/store/error.js index bdd3339..83eec87 100644 --- a/lib/store/error.js +++ b/lib/store/error.js @@ -1,11 +1,19 @@ export class StoreConflictError extends Error { - constructor(entity, id) { - super(`${entity} id ${id} already exists`) + constructor(entity, id, message = `${entity} id ${id} already exists`) { + super(message) this.name = 'StoreConflictError' this.code = 'STORE_CONFLICT' } } +export class StoreInvalidError extends Error { + constructor(message) { + super(message) + this.name = 'StoreInvalidError' + this.code = 'STORE_INVALID' + } +} + // ifExists keeps the old return-the-existing-id behaviour for fixtures, so // create() is not an accidental upsert everywhere else. export function idConflict(entity, id, options = {}) { diff --git a/lib/store/file.js b/lib/store/file.js index 8ec2c9e..f0ab9bd 100644 --- a/lib/store/file.js +++ b/lib/store/file.js @@ -1,3 +1,4 @@ +import { randomBytes } from 'node:crypto' import fs from 'node:fs/promises' import path from 'node:path' @@ -19,15 +20,47 @@ const tomlCodec = { } const codecs = { json: jsonCodec, toml: tomlCodec } +const mutations = new Map() + +export async function serialize(file, callback) { + const previous = mutations.get(file) ?? Promise.resolve() + const current = previous.catch(() => {}).then(callback) + mutations.set(file, current) + + try { + return await current + } finally { + if (mutations.get(file) === current) mutations.delete(file) + } +} export function resolveCodec(type = storeConfig().type) { return codecs[type] ?? jsonCodec } +export function highestId(rows) { + return rows.reduce((max, row) => (Number.isInteger(row.id) && row.id > max ? row.id : max), 0) +} + +export function nextId(rows, lastId = 0, maxId = 0xffffffff) { + if (!Number.isInteger(lastId) || lastId < 0) throw new TypeError('file store last id must be unsigned') + + const highest = Math.max(lastId, highestId(rows)) + if (highest >= maxId) throw new RangeError(`file store id space exhausted at ${maxId}`) + return highest + 1 +} + +// the id a create gets, recorded as the file's high-water mark +export function claimId(rows, data, id, maxId) { + if (id === undefined) id = nextId(rows, data.last_id, maxId) + data.last_id = Math.max(data.last_id ?? 0, id) + return id +} + /** - * One file per entity, holding an array of rows under a single top-level key. - * The codec is chosen by store.type; everything above this layer is identical - * for JSON and TOML. + * One file per entity, holding an array of rows under a single top-level key, + * plus last_id, the highest id ever allocated. The codec is chosen by + * store.type; everything above this layer is identical for JSON and TOML. */ export class FileStore { constructor(basename) { @@ -53,21 +86,72 @@ export class FileStore { return path.join(base, `${this.basename}.${this.codec.ext}`) } - async load(key) { + async _read(file) { try { - const data = await this.codec.parse(await fs.readFile(this.filePath, 'utf8')) - return Array.isArray(data?.[key]) ? data[key] : [] + const data = await this.codec.parse(await fs.readFile(file, 'utf8')) + return data && typeof data === 'object' && !Array.isArray(data) ? data : {} } catch (err) { - if (err.code === 'ENOENT') return [] + if (err.code === 'ENOENT') return {} throw err } } - async save(key, rows) { - const file = this.filePath + // a copy renamed over the file, so a load never sees half of it + async _write(file, data) { await fs.mkdir(path.dirname(file), { recursive: true }) - await fs.writeFile(file, await this.codec.stringify({ [key]: rows })) + const old = await fs.stat(file).catch((err) => { + if (err.code !== 'ENOENT') throw err + }) + const tmp = `${file}.${randomBytes(6).toString('hex')}.tmp` + try { + const mode = old ? old.mode & 0o777 : 0o666 + await fs.writeFile(tmp, await this.codec.stringify(data), { flag: 'wx', mode }) + if (old) await fs.chmod(tmp, mode) + await fs.rename(tmp, file) + } catch (err) { + if (err.code !== 'EEXIST') await fs.rm(tmp, { force: true }).catch(() => {}) + throw err + } + } + + async load(key) { + const data = await this._read(this.filePath) + return Array.isArray(data[key]) ? data[key] : [] + } + + async mutate(key, callback) { + const file = this.filePath + return serialize(file, async () => { + const data = await this._read(file) + const rows = Array.isArray(data[key]) ? data[key] : [] + data[key] = rows + // a file from before last_id existed mustn't reuse a destroyed id + if (data.last_id === undefined) { + const highest = highestId(rows) + if (highest > 0) data.last_id = highest + } + // a callback that changed nothing returns false, or its result via keep() + let changed = true + const keep = (value) => { + changed = false + return value + } + const result = await callback(rows, data, keep) + if (changed && result !== false) await this._write(file, data) + return result + }) + } +} + +// in place, so a mutate() callback can drop rows +export function removeRows(rows, match) { + let kept = 0 + for (const row of rows) { + if (!match(row)) rows[kept++] = row } + const removed = rows.length - kept + rows.length = kept + return removed } export default FileStore diff --git a/lib/store/file.test.js b/lib/store/file.test.js index c76b6e2..011de9c 100644 --- a/lib/store/file.test.js +++ b/lib/store/file.test.js @@ -1,10 +1,20 @@ import assert from 'node:assert/strict' import fs from 'node:fs' +import fsp from 'node:fs/promises' import os from 'node:os' import path from 'node:path' import { describe, it, before, after, beforeEach } from 'node:test' -import FileStore, { resolveCodec } from './file.js' +import GroupRepoFile from '../group/store/file.js' +import NameserverRepoFile from '../nameserver/store/file.js' +import PermissionRepoFile from '../permission/store/file.js' +import SessionRepoFile from '../session/store/file.js' +import Credentials from '../user/credentials.js' +import UserRepoFile from '../user/store/file.js' +import ZoneRepoFile from '../zone/store/file.js' +import ZoneRecordRepoFile from '../zone_record/store/file.js' +import FileStore, { nextId, resolveCodec } from './file.js' +import { seed } from '../../test/seed.js' const envKeys = ['NICTOOL_DATA_STORE', 'NICTOOL_DATA_STORE_PATH', 'NICTOOL_CONF_DIR'] @@ -12,6 +22,14 @@ describe('file store', () => { const savedEnv = {} let tmp + it('allocates after the highest existing id', () => { + assert.equal(nextId([]), 1) + assert.equal(nextId([{ id: 9 }, { id: 2 }, {}]), 10) + assert.equal(nextId([{ id: 9 }], 12), 13) + assert.equal(nextId([{ id: null }, { id: 'x' }, { id: 4 }]), 5) + assert.throws(() => nextId([{ id: 0xffffffff }]), /id space exhausted/) + }) + before(() => { for (const key of envKeys) { savedEnv[key] = process.env[key] @@ -38,11 +56,11 @@ describe('file store', () => { it(`writes .json for a json store and .toml for a toml store`, async () => { process.env.NICTOOL_DATA_STORE = 'json' - await new FileStore('zone').save('zone', [{ id: 1 }]) + await seed(new FileStore('zone'), 'zone', [{ id: 1 }]) assert.equal(fs.existsSync(path.join(tmp, 'zone.json')), true) process.env.NICTOOL_DATA_STORE = 'toml' - await new FileStore('zone').save('zone', [{ id: 1 }]) + await seed(new FileStore('zone'), 'zone', [{ id: 1 }]) assert.equal(fs.existsSync(path.join(tmp, 'zone.toml')), true) }) @@ -50,7 +68,7 @@ describe('file store', () => { process.env.NICTOOL_DATA_STORE = 'json' const store = new FileStore('rt') - await store.save('rt', [{ id: 1, zone: 'example.com', deleted: false }]) + await seed(store, 'rt', [{ id: 1, zone: 'example.com', deleted: false }]) assert.deepEqual(await store.load('rt'), [{ id: 1, zone: 'example.com', deleted: false }]) }) @@ -58,14 +76,14 @@ describe('file store', () => { it(`preserves null through json, which TOML cannot`, async () => { process.env.NICTOOL_DATA_STORE = 'json' const jsonStore = new FileStore('nulls') - await jsonStore.save('nulls', [{ id: 1, last_publish: null }]) + await seed(jsonStore, 'nulls', [{ id: 1, last_publish: null }]) const [jsonRow] = await jsonStore.load('nulls') assert.equal(jsonRow.last_publish, null, 'json keeps the null') process.env.NICTOOL_DATA_STORE = 'toml' const tomlStore = new FileStore('nulls') - await tomlStore.save('nulls', [{ id: 1, last_publish: null }]) + await seed(tomlStore, 'nulls', [{ id: 1, last_publish: null }]) const [tomlRow] = await tomlStore.load('nulls') assert.equal(tomlRow.last_publish, undefined, 'toml drops it — hence the repair in the zone store') @@ -83,11 +101,11 @@ describe('file store', () => { assert.deepEqual(await new FileStore('odd').load('odd'), []) }) - it(`creates missing parent directories on save`, async () => { + it(`creates missing parent directories on write`, async () => { process.env.NICTOOL_DATA_STORE = 'json' process.env.NICTOOL_DATA_STORE_PATH = path.join(tmp, 'deep', 'nested') - await new FileStore('zone').save('zone', [{ id: 1 }]) + await seed(new FileStore('zone'), 'zone', [{ id: 1 }]) assert.equal(fs.existsSync(path.join(tmp, 'deep', 'nested', 'zone.json')), true) }) @@ -103,4 +121,266 @@ describe('file store', () => { process.env.NICTOOL_DATA_STORE = 'nonsense' assert.equal(resolveCodec().ext, 'json') }) + + for (const type of ['json', 'toml']) { + it(`never lets a ${type} load read a half-written file`, async (t) => { + process.env.NICTOOL_DATA_STORE = type + const store = new FileStore('torn') + const rows = Array.from({ length: 50 }, (_, id) => ({ id, owner: `host${id}.example.com.` })) + await seed(store, 'torn', rows) + + const rename = fsp.rename + let seen + t.mock.method(fsp, 'rename', async (from, to) => { + seen = await store.load('torn').then( + (r) => r.length, + (err) => err.message, + ) + await rename(from, to) + }) + await seed(store, 'torn', [...rows, { id: 50, owner: 'new.example.com.' }]) + + assert.equal(seen, rows.length) + assert.equal((await store.load('torn')).length, rows.length + 1) + assert.deepEqual( + fs.readdirSync(tmp).filter((f) => f.endsWith('.tmp')), + [], + ) + }) + + it(`keeps the mode of an existing ${type} file`, { skip: process.platform === 'win32' }, async () => { + process.env.NICTOOL_DATA_STORE = type + const store = new FileStore('moded') + await seed(store, 'moded', [{ id: 1 }]) + fs.chmodSync(store.filePath, 0o664) + await seed(store, 'moded', [{ id: 2 }]) + assert.equal(fs.statSync(store.filePath).mode & 0o777, 0o664) + }) + + it(`serializes ${type} id allocation and does not reuse a destroyed id`, async () => { + process.env.NICTOOL_DATA_STORE = type + const store = new GroupRepoFile() + const ids = await Promise.all( + Array.from({ length: 20 }, (_, index) => store.create({ name: `group-${index}.example` })), + ) + + assert.deepEqual( + [...ids].sort((a, b) => a - b), + Array.from({ length: 20 }, (_, index) => index + 1), + ) + await store.destroy({ id: 20 }) + assert.equal(await store.create({ name: 'after-destroy.example' }), 21) + }) + + it(`keeps ${type} users created while a permission is added to a user`, async (t) => { + process.env.NICTOOL_DATA_STORE = type + fs.rmSync(path.join(tmp, `user.${type}`), { force: true }) + const users = new UserRepoFile() + const uid = await users.create({ gid: 1, username: 'holder', email: 'holder@example.com' }) + + // the users are created once the permission create has read the user file + const read = FileStore.prototype._read + const racers = [] + t.mock.method(FileStore.prototype, '_read', async function (file) { + const data = await read.call(this, file) + if (racers.length === 0 && file.endsWith(`user.${type}`)) { + for (let index = 0; index < 5; index++) { + racers.push( + users.create({ gid: 1, username: `racer-${index}`, email: `racer-${index}@example.com` }), + ) + } + } + return data + }) + + const permId = await new PermissionRepoFile().create({ uid, gid: 1, zone: { create: true } }) + const created = await Promise.all(racers) + t.mock.restoreAll() + + const stored = await new FileStore('user').load('user') + assert.deepEqual( + stored.map((user) => user.id).sort((a, b) => a - b), + [uid, ...created].sort((a, b) => a - b), + ) + assert.equal(stored.find((user) => user.id === uid).permissions.id, permId) + }) + + for (const [entity, Repo, make, changes] of [ + ['group', GroupRepoFile, (i) => ({ name: `racer-${i}.example` }), { put: { name: 'renamed.example' } }], + ['nameserver', NameserverRepoFile, (i) => ({ gid: 1, name: `ns${i}.example.` }), { put: { ttl: 7 } }], + [ + 'session', + SessionRepoFile, + (i) => ({ uid: 1, session: `racer-${i}` }), + { put: { session: 'renamed' } }, + ], + [ + 'user', + UserRepoFile, + (i) => ({ gid: 1, username: `racer-${i}`, email: `racer-${i}@example.com` }), + { put: { email: 'renamed@example.com' } }, + ], + [ + 'zone', + ZoneRepoFile, + (i) => ({ gid: 1, zone: `racer${i}.example.` }), + { put: { description: 'renamed' } }, + ], + [ + 'zone_record', + ZoneRecordRepoFile, + (i) => ({ zid: 1, owner: `racer${i}.example.`, type: 'A', address: '192.0.2.1' }), + { put: { ttl: 9 } }, + ], + ]) { + const ops = [ + ['put', (repo, id) => repo.put({ id, ...changes.put })], + ['delete', (repo, id) => repo.delete({ id })], + ] + if (entity !== 'session') ops.push(['destroy', (repo, id) => repo.destroy({ id })]) + + for (const [label, change] of ops) { + it(`keeps ${type} ${entity} rows created during a ${label}`, async (t) => { + process.env.NICTOOL_DATA_STORE = type + fs.rmSync(path.join(tmp, `${entity}.${type}`), { force: true }) + const repo = new Repo() + const id = await repo.create(make('target')) + + // the rows are created once the change has read the file + const read = FileStore.prototype._read + const racers = [] + t.mock.method(FileStore.prototype, '_read', async function (file) { + const data = await read.call(this, file) + if (racers.length === 0 && file.endsWith(`${entity}.${type}`)) { + for (let index = 0; index < 5; index++) racers.push(repo.create(make(index))) + } + return data + }) + + assert.equal(await change(repo, id), true) + assert.equal(racers.length, 5) + const created = await Promise.all(racers) + t.mock.restoreAll() + + const stored = (await new FileStore(entity).load(entity)).map((row) => row.id) + for (const rowId of created) assert.ok(stored.includes(rowId), `${entity} ${rowId} survives`) + }) + } + } + + it(`leaves a ${type} group's permission alone once a user's with its id is gone`, async () => { + process.env.NICTOOL_DATA_STORE = type + fs.rmSync(path.join(tmp, `permission.${type}`), { force: true }) + await seed(new FileStore('group'), 'group', [ + { id: 1, name: 'one', permissions: { id: 1, name: 'group' } }, + ]) + await seed(new FileStore('user'), 'user', [ + { id: 1, gid: 1, username: 'one', permissions: { id: 1, name: 'user' } }, + ]) + const permissions = new PermissionRepoFile() + + assert.ok(await permissions.destroy({ id: 1 })) + assert.equal(await permissions.get({ id: 1 }), undefined) + assert.equal(await permissions.put({ id: 1, name: 'changed' }), false) + assert.equal(await permissions.delete({ id: 1 }), false) + assert.equal(await permissions.destroy({ id: 1 }), false) + const [group] = await new FileStore('group').load('group') + assert.deepEqual({ ...group.permissions }, { id: 1, name: 'group' }) + }) + + it(`stores a ${type} group permission whose user id is null`, async () => { + process.env.NICTOOL_DATA_STORE = type + for (const name of ['user', 'permission']) fs.rmSync(path.join(tmp, `${name}.${type}`), { force: true }) + await seed(new FileStore('group'), 'group', [{ id: 3, name: 'three' }]) + const permissions = new PermissionRepoFile() + + assert.equal(await permissions.create({ user: { id: null }, group: { id: 3 }, name: 'grp' }), 3) + assert.equal((await permissions.get({ gid: 3 })).name, 'grp') + await assert.rejects(permissions.create({ id: 900, name: 'no target' }), /needs a user or group/) + }) + + it(`gives one of two ${type} permission creates for one explicit id a conflict`, async () => { + process.env.NICTOOL_DATA_STORE = type + for (const name of ['group', 'permission']) + fs.rmSync(path.join(tmp, `${name}.${type}`), { force: true }) + await seed(new FileStore('user'), 'user', [ + { id: 1, gid: 1, username: 'one' }, + { id: 2, gid: 1, username: 'two' }, + ]) + const permissions = new PermissionRepoFile() + + const results = await Promise.allSettled([ + permissions.create({ id: 77, uid: 1, name: 'first' }), + permissions.create({ id: 77, uid: 2, name: 'second' }), + ]) + assert.deepEqual(results.map((r) => r.status).sort(), ['fulfilled', 'rejected']) + assert.equal(results.find((r) => r.status === 'rejected').reason.code, 'STORE_CONFLICT') + }) + + it(`changes the ${type} permission get returns, not a group's with the same id`, async () => { + process.env.NICTOOL_DATA_STORE = type + fs.rmSync(path.join(tmp, `user.${type}`), { force: true }) + await seed(new FileStore('group'), 'group', [ + { id: 7, name: 'seven', permissions: { id: 7, name: 'group' } }, + ]) + await seed(new FileStore('permission'), 'permission', [{ id: 7, name: 'standalone' }]) + const permissions = new PermissionRepoFile() + + assert.ok(await permissions.put({ id: 7, name: 'changed' })) + assert.equal((await permissions.get({ id: 7 })).name, 'changed') + assert.equal((await new FileStore('group').load('group'))[0].permissions.name, 'group') + }) + + it(`does not reuse a ${type} id destroyed before last_id was recorded`, async () => { + process.env.NICTOOL_DATA_STORE = type + fs.rmSync(path.join(tmp, `group.${type}`), { force: true }) + await seed(new FileStore('group'), 'group', [ + { id: 19, name: 'nineteen.example' }, + { id: 20, name: 'twenty.example' }, + ]) + const groups = new GroupRepoFile() + await groups.destroy({ id: 20 }) + assert.equal(await groups.create({ name: 'next.example' }), 21) + }) + + it(`refuses a ${type} permission with no user or group`, async () => { + process.env.NICTOOL_DATA_STORE = type + await assert.rejects(new PermissionRepoFile().create({ name: 'no target' }), /needs a user or group/) + }) + + it(`does not write ${type} when a change finds nothing`, async () => { + process.env.NICTOOL_DATA_STORE = type + fs.rmSync(path.join(tmp, `zone.${type}`), { force: true }) + assert.equal(await new FileStore('zone').mutate('zone', () => false), false) + assert.equal(await new FileStore('zone').mutate('zone', (rows, data, keep) => keep(3)), 3) + assert.equal(fs.existsSync(path.join(tmp, `zone.${type}`)), false) + }) + + it(`keeps ${type} users created while a login upgrades its password hash`, async (t) => { + process.env.NICTOOL_DATA_STORE = type + await seed(new FileStore('group'), 'group', [{ id: 1, name: 'NicTool' }]) + await seed(new FileStore('user'), 'user', [{ id: 1, gid: 1, username: 'legacy', password: 'plain' }]) + const users = new UserRepoFile() + const forStorage = Credentials.forStorage.bind(Credentials) + const created = [] + t.mock.method(Credentials, 'forStorage', async (pass) => { + for (let index = 0; index < 5; index++) { + created.push( + await users.create({ gid: 1, username: `racer-${index}`, email: `racer-${index}@example.com` }), + ) + } + return forStorage(pass) + }) + + const login = await users.authenticate({ username: 'legacy', password: 'plain' }) + + assert.equal(login.user.id, 1) + const stored = await new FileStore('user').load('user') + assert.deepEqual( + stored.map((user) => user.id).sort((a, b) => a - b), + [1, ...created].sort((a, b) => a - b), + ) + assert.ok(stored.find((user) => user.id === 1).pass_salt, 'the hash was upgraded') + }) + } }) diff --git a/lib/user/store/file.js b/lib/user/store/file.js index 4d2c727..e6f8d08 100644 --- a/lib/user/store/file.js +++ b/lib/user/store/file.js @@ -1,4 +1,4 @@ -import FileStore from '../../store/file.js' +import FileStore, { claimId, removeRows } from '../../store/file.js' import { idConflict } from '../../store/error.js' import Config from '../../config.js' import Credentials from '../credentials.js' @@ -35,10 +35,6 @@ class UserRepoFile extends UserBase { return this.file.load('user') } - async _save(users) { - return this.file.save('user', users) - } - _postProcess(u, deletedArg) { const r = { ...u } // Remove sensitive credential fields — these are stored internally but never @@ -75,8 +71,12 @@ class UserRepoFile extends UserBase { // the user their login if (needsUpgrade) { try { - Object.assign(u, await Credentials.forStorage(authTry.password)) - await this._save(users) + const upgraded = await Credentials.forStorage(authTry.password) + await this.file.mutate('user', (current) => { + const row = current.find((c) => c.id === u.id) + if (row?.password !== u.password || row.pass_salt !== u.pass_salt) return false + Object.assign(row, upgraded) + }) } catch (err) { console.warn(`password hash upgrade failed for user ${authTry.username}`, err) } @@ -135,14 +135,6 @@ class UserRepoFile extends UserBase { } async create(args, options) { - if (args.id !== undefined) { - const existing = [ - ...(await this.get({ id: args.id })), - ...(await this.get({ id: args.id, deleted: true })), - ] - if (existing.length > 0) return idConflict('user', args.id, options) - } - args = JSON.parse(JSON.stringify(args)) const inherit = args.inherit_group_permissions @@ -152,69 +144,67 @@ class UserRepoFile extends UserBase { Object.assign(args, await Credentials.forStorage(args.password, args.pass_salt)) } - if (inherit === false) { - args.permissions = { - ...JSON.parse(JSON.stringify(defaultPermissions)), - id: args.id, - user: { id: args.id, create: false, write: false, delete: false }, - group: { id: args.gid, create: false, write: false, delete: false }, + return this.file.mutate('user', (users, data, keep) => { + if (args.id !== undefined && users.some((user) => user.id === args.id)) { + return keep(idConflict('user', args.id, options)) } - } - const users = await this._load() - users.push(args) - await this._save(users) - return args.id + args.id = claimId(users, data, args.id) + + if (inherit === false) { + args.permissions = { + ...JSON.parse(JSON.stringify(defaultPermissions)), + id: args.id, + user: { id: args.id, create: false, write: false, delete: false }, + group: { id: args.gid, create: false, write: false, delete: false }, + } + } + + users.push(args) + return args.id + }) } async put(args) { if (!args.id) return false args = JSON.parse(JSON.stringify(args)) - const users = await this._load() - const idx = users.findIndex((u) => u.id === args.id) - if (idx === -1) return false - const inherit = args.inherit_group_permissions delete args.inherit_group_permissions - if (inherit === true) { - // Switch to inherited: remove explicit permissions - delete users[idx].permissions - } else if (inherit === false && !users[idx].permissions) { - // Switch to explicit: create default permission entry - users[idx].permissions = { - ...JSON.parse(JSON.stringify(defaultPermissions)), - id: users[idx].id, - user: { id: users[idx].id, create: false, write: false, delete: false }, - group: { id: users[idx].gid, create: false, write: false, delete: false }, + return this.file.mutate('user', (users) => { + const idx = users.findIndex((u) => u.id === args.id) + if (idx === -1) return false + + if (inherit === true) { + delete users[idx].permissions + } else if (inherit === false && !users[idx].permissions) { + users[idx].permissions = { + ...JSON.parse(JSON.stringify(defaultPermissions)), + id: users[idx].id, + user: { id: users[idx].id, create: false, write: false, delete: false }, + group: { id: users[idx].gid, create: false, write: false, delete: false }, + } + } else if (inherit === false && users[idx].permissions) { + users[idx].permissions.inherit = false } - } else if (inherit === false && users[idx].permissions) { - users[idx].permissions.inherit = false - } - users[idx] = { ...users[idx], ...args } - await this._save(users) - return true + users[idx] = { ...users[idx], ...args } + return true + }) } async delete(args) { - const users = await this._load() - const idx = users.findIndex((u) => u.id === args.id) - if (idx === -1) return false - - users[idx].deleted = args.deleted ?? true - await this._save(users) - return true + return this.file.mutate('user', (users) => { + const row = users.find((u) => u.id === args.id) + if (!row) return false + row.deleted = args.deleted ?? true + return true + }) } async destroy(args) { - const users = await this._load() - const before = users.length - const filtered = users.filter((u) => u.id !== args.id) - if (filtered.length === before) return false - await this._save(filtered) - return true + return this.file.mutate('user', (users) => removeRows(users, (u) => u.id === args.id) > 0) } } diff --git a/lib/zone/store/file.js b/lib/zone/store/file.js index bc550cc..9f2b47b 100644 --- a/lib/zone/store/file.js +++ b/lib/zone/store/file.js @@ -1,4 +1,4 @@ -import FileStore, { resolveCodec } from '../../store/file.js' +import FileStore, { claimId, removeRows, resolveCodec } from '../../store/file.js' import { idConflict } from '../../store/error.js' import ZoneBase from './base.js' @@ -15,10 +15,6 @@ class ZoneRepoFile extends ZoneBase { return this.file.load('zone') } - async _save(zones) { - return this.file.save('zone', zones) - } - _postProcess(row, deletedArg) { const r = { ...row } r.deleted = Boolean(r.deleted) @@ -36,18 +32,17 @@ class ZoneRepoFile extends ZoneBase { } async create(args, options) { - if (args.id !== undefined) { - const existing = [ - ...(await this.get({ id: args.id })), - ...(await this.get({ id: args.id, deleted: true })), - ] - if (existing.length > 0) return idConflict('zone', args.id, options) - } + args = JSON.parse(JSON.stringify(args)) + + return this.file.mutate('zone', (zones, data, keep) => { + if (args.id !== undefined && zones.some((zone) => zone.id === args.id)) { + return keep(idConflict('zone', args.id, options)) + } - const zones = await this._load() - zones.push(JSON.parse(JSON.stringify(args))) - await this._save(zones) - return args.id + args.id = claimId(zones, data, args.id) + zones.push(args) + return args.id + }) } async get(args) { @@ -141,32 +136,25 @@ class ZoneRepoFile extends ZoneBase { async put(args) { if (!args.id) return false - const zones = await this._load() - const idx = zones.findIndex((z) => z.id === args.id) - if (idx === -1) return false - - zones[idx] = { ...zones[idx], ...args } - await this._save(zones) - return true + return this.file.mutate('zone', (zones) => { + const idx = zones.findIndex((z) => z.id === args.id) + if (idx === -1) return false + zones[idx] = { ...zones[idx], ...args } + return true + }) } async delete(args) { - const zones = await this._load() - const idx = zones.findIndex((z) => z.id === args.id) - if (idx === -1) return false - - zones[idx].deleted = args.deleted ?? true - await this._save(zones) - return true + return this.file.mutate('zone', (zones) => { + const row = zones.find((z) => z.id === args.id) + if (!row) return false + row.deleted = args.deleted ?? true + return true + }) } async destroy(args) { - const zones = await this._load() - const before = zones.length - const filtered = zones.filter((z) => z.id !== args.id) - if (filtered.length === before) return false - await this._save(filtered) - return true + return this.file.mutate('zone', (zones) => removeRows(zones, (z) => z.id === args.id) > 0) } async nameserversFor(zid) { diff --git a/lib/zone/store/file.test.js b/lib/zone/store/file.test.js index 25a79cf..4f3bcfc 100644 --- a/lib/zone/store/file.test.js +++ b/lib/zone/store/file.test.js @@ -6,6 +6,7 @@ import { describe, it, before, after } from 'node:test' import FileStore from '../../store/file.js' import ZoneRepoFile from './file.js' +import { seed } from '../../../test/seed.js' const envKeys = ['NICTOOL_DATA_STORE', 'NICTOOL_DATA_STORE_PATH', 'NICTOOL_CONF_DIR'] @@ -19,9 +20,9 @@ describe('zone file store nameservers', () => { process.env.NICTOOL_CONF_DIR = path.join(tmp, 'no-such-conf') process.env.NICTOOL_DATA_STORE = 'json' process.env.NICTOOL_DATA_STORE_PATH = tmp - await new FileStore('zone').save('zone', [{ id: 1, zone: 'example.com' }]) - await new FileStore('nameserver').save('nameserver', [{ id: 5, name: 'ns1.example.net.', ttl: 3600 }]) - await new FileStore('zone_nameserver').save('zone_nameserver', [ + await seed(new FileStore('zone'), 'zone', [{ id: 1, zone: 'example.com' }]) + await seed(new FileStore('nameserver'), 'nameserver', [{ id: 5, name: 'ns1.example.net.', ttl: 3600 }]) + await seed(new FileStore('zone_nameserver'), 'zone_nameserver', [ { zid: 1, nid: 5 }, { zid: 2, nid: 5 }, ]) diff --git a/lib/zone_record/store/file.js b/lib/zone_record/store/file.js index e05d797..c7306c1 100644 --- a/lib/zone_record/store/file.js +++ b/lib/zone_record/store/file.js @@ -1,4 +1,4 @@ -import FileStore from '../../store/file.js' +import FileStore, { claimId, removeRows } from '../../store/file.js' import { idConflict } from '../../store/error.js' import ZoneRecordBase from './base.js' @@ -13,33 +13,20 @@ class ZoneRecordRepoFile extends ZoneRecordBase { return this.file.load('zone_record') } - async _save(records) { - return this.file.save('zone_record', records) - } - async create(args, options) { args = JSON.parse(JSON.stringify(args)) - if (args.id !== undefined) { - const existing = [ - ...(await this.get({ id: args.id })), - ...(await this.get({ id: args.id, deleted: true })), - ] - if (existing.length > 0) return idConflict('zone record', args.id, options) - } - - const records = await this._load() - - if (!args.id) { - const maxId = records.reduce((max, r) => Math.max(max, r.id ?? 0), 0) - args.id = maxId + 1 - } + return this.file.mutate('zone_record', (records, data, keep) => { + if (args.id !== undefined && records.some((record) => record.id === args.id)) { + return keep(idConflict('zone record', args.id, options)) + } - if (args.ttl === undefined) args.ttl = 0 + args.id = claimId(records, data, args.id) + if (args.ttl === undefined) args.ttl = 0 - records.push(args) - await this._save(records) - return args.id + records.push(args) + return args.id + }) } async count(args = {}) { @@ -107,32 +94,25 @@ class ZoneRecordRepoFile extends ZoneRecordBase { async put(args) { if (!args.id) return false - const records = await this._load() - const idx = records.findIndex((r) => r.id === args.id) - if (idx === -1) return false - - records[idx] = { ...records[idx], ...args } - await this._save(records) - return true + return this.file.mutate('zone_record', (records) => { + const idx = records.findIndex((r) => r.id === args.id) + if (idx === -1) return false + records[idx] = { ...records[idx], ...args } + return true + }) } async delete(args) { - const records = await this._load() - const idx = records.findIndex((r) => r.id === args.id) - if (idx === -1) return false - - records[idx].deleted = args.deleted ?? true - await this._save(records) - return true + return this.file.mutate('zone_record', (records) => { + const row = records.find((r) => r.id === args.id) + if (!row) return false + row.deleted = args.deleted ?? true + return true + }) } async destroy(args) { - const records = await this._load() - const before = records.length - const filtered = records.filter((r) => r.id !== args.id) - if (filtered.length === before) return false - await this._save(filtered) - return true + return this.file.mutate('zone_record', (records) => removeRows(records, (r) => r.id === args.id) > 0) } } diff --git a/package.json b/package.json index 0406711..b7a766a 100644 --- a/package.json +++ b/package.json @@ -61,7 +61,7 @@ "@hapi/vision": "^7.0.3", "@msimerson/hapi-openapi": "^18.0.3", "@nictool/dns-resource-record": "^1.8.2", - "@nictool/validate": "^0.9.1", + "@nictool/validate": "^1.0.1", "joi": "^18.2.3", "mysql2": "^3.23.2", "qs": "^6.15.3", diff --git a/routes/create_id.test.js b/routes/create_id.test.js new file mode 100644 index 0000000..e00994f --- /dev/null +++ b/routes/create_id.test.js @@ -0,0 +1,107 @@ +import assert from 'node:assert/strict' +import { after, before, describe, it } from 'node:test' + +import Group from '../lib/group/index.js' +import Nameserver from '../lib/nameserver/index.js' +import Permission from '../lib/permission/index.js' +import User from '../lib/user/index.js' +import Zone from '../lib/zone/index.js' +import ZoneRecord from '../lib/zone_record/index.js' +import { init } from './index.js' + +import groupCase from './test/group.json' with { type: 'json' } +import nameserverCase from './test/nameserver.json' with { type: 'json' } +import permissionCase from './test/permission.json' with { type: 'json' } +import userCase from './test/user.json' with { type: 'json' } +import zoneCase from './test/zone.json' with { type: 'json' } + +const chosenId = 65001 +let server +const auth = { headers: {} } + +const userPayload = { + ...userCase, + id: chosenId, + gid: groupCase.id, + email: 'caller-id@example.com', + username: 'caller-id-user', +} +delete userPayload.deleted + +const cases = [ + ['group', { ...groupCase, id: chosenId, name: 'caller-id-group' }], + ['nameserver', { ...nameserverCase, id: chosenId, gid: groupCase.id, name: 'caller-id.ns.example.com.' }], + [ + 'permission', + { + ...permissionCase, + id: chosenId, + group: { ...permissionCase.group, id: chosenId }, + user: { ...permissionCase.user, id: chosenId }, + }, + ], + ['user', userPayload], + ['zone', { ...zoneCase, id: chosenId, gid: groupCase.id, zone: 'caller-id.example.com.' }], + [ + 'zone_record', + { + id: chosenId, + zid: zoneCase.id, + owner: 'caller-id.route.example.com.', + ttl: 300, + type: 'A', + address: '203.0.113.111', + }, + ], +] + +before(async () => { + const fixture = { ifExists: 'return' } + await ZoneRecord.destroy({ id: chosenId }) + await Zone.destroy({ id: chosenId }) + await User.destroy({ id: chosenId }) + await Permission.destroy({ id: chosenId }) + await Nameserver.destroy({ id: chosenId }) + await Group.destroy({ id: chosenId }) + await Group.create(groupCase, fixture) + await User.create(userCase, fixture) + await Zone.create(zoneCase, fixture) + server = await init() + + const res = await server.inject({ + method: 'POST', + url: '/session', + payload: { + username: `${userCase.username}@${groupCase.name}`, + password: userCase.password, + }, + }) + assert.equal(res.statusCode, 200) + auth.headers = { Authorization: `Bearer ${res.result.session.token}` } +}) + +after(async () => { + await ZoneRecord.destroy({ id: chosenId }) + await Zone.destroy({ id: chosenId }) + await User.destroy({ id: chosenId }) + await Permission.destroy({ id: chosenId }) + await Nameserver.destroy({ id: chosenId }) + await Group.destroy({ id: chosenId }) + if (server) await server.stop() +}) + +describe('caller-supplied create ids', () => { + for (const [entity, payload] of cases) { + it(`rejects an id on POST /${entity}`, async () => { + const res = await server.inject({ + method: 'POST', + url: `/${entity}`, + headers: auth.headers, + payload, + }) + + assert.equal(res.statusCode, 400) + assert.match(res.result.message, /"id" is not allowed/) + }) + } +}) diff --git a/routes/group.test.js b/routes/group.test.js index 8945093..f27e7a8 100644 --- a/routes/group.test.js +++ b/routes/group.test.js @@ -9,7 +9,7 @@ import groupCase from './test/group.json' with { type: 'json' } import userCase from './test/user.json' with { type: 'json' } let server -const case2Id = 4094 +let case2Id before(async () => { server = await init() @@ -18,7 +18,7 @@ before(async () => { }) after(async () => { - await Group.destroy({ id: case2Id }) + if (case2Id) await Group.destroy({ id: case2Id }) await server.stop() }) @@ -49,9 +49,9 @@ describe('group routes', () => { assert.equal(res.result.group[0].id, groupCase.id) }) - it('POST /group', async () => { + it('POST /group allocates an id', async () => { const testCase = JSON.parse(JSON.stringify(groupCase)) - testCase.id = case2Id // make it unique + delete testCase.id testCase.name = `example2.com` delete testCase.deleted @@ -62,9 +62,24 @@ describe('group routes', () => { payload: testCase, }) assert.equal(res.statusCode, 201) + assert.equal(res.result.group.length, 1) + assert.equal(res.result.group[0].name, 'example2.com') + case2Id = res.result.group[0].id + assert.ok(Number.isInteger(case2Id)) }) - it(`GET /group/${case2Id}`, async () => { + it('PUT /group/{id} keeps using the route id', async () => { + const res = await server.inject({ + method: 'PUT', + url: `/group/${case2Id}`, + headers: auth.headers, + payload: { name: 'example3.com' }, + }) + assert.equal(res.statusCode, 200) + assert.equal(res.result.group[0].name, 'example3.com') + }) + + it('GET /group/{id}', async () => { const res = await server.inject({ method: 'GET', url: `/group/${case2Id}`, @@ -74,7 +89,7 @@ describe('group routes', () => { assert.equal(res.result.group[0].id, case2Id) }) - it(`DELETE /group/${case2Id}`, async () => { + it('DELETE /group/{id}', async () => { const res = await server.inject({ method: 'DELETE', url: `/group/${case2Id}`, @@ -83,7 +98,7 @@ describe('group routes', () => { assert.equal(res.statusCode, 200) }) - it(`GET /group/${case2Id} (soft-deleted → empty array)`, async () => { + it('GET /group/{id} hides a soft-deleted group', async () => { const res = await server.inject({ method: 'GET', url: `/group/${case2Id}`, @@ -93,7 +108,7 @@ describe('group routes', () => { assert.deepEqual(res.result.group, []) }) - it(`GET /group/${case2Id} (deleted)`, async () => { + it('GET /group/{id}?deleted=true returns a soft-deleted group', async () => { const res = await server.inject({ method: 'GET', url: `/group/${case2Id}?deleted=true`, diff --git a/routes/index.js b/routes/index.js index 590e51d..f158135 100644 --- a/routes/index.js +++ b/routes/index.js @@ -108,6 +108,9 @@ async function setup() { server.ext('onPreResponse', (request, h) => { const response = request.response + if (response?.code === 'STORE_INVALID') { + return h.response({ statusCode: 400, error: 'Bad Request', message: response.message }).code(400) + } if (!(response instanceof StoreConflictError) && response?.code !== 'STORE_CONFLICT') { return h.continue } diff --git a/routes/nameserver.test.js b/routes/nameserver.test.js index b9c65d1..d9dc710 100644 --- a/routes/nameserver.test.js +++ b/routes/nameserver.test.js @@ -11,10 +11,9 @@ import userCase from './test/user.json' with { type: 'json' } import nsCase from './test/nameserver.json' with { type: 'json' } let server -let case2Id = 4094 +let case2Id before(async () => { - await Nameserver.destroy({ id: case2Id }) await Group.create(groupCase, { ifExists: 'return' }) await User.create(userCase, { ifExists: 'return' }) await Nameserver.create(nsCase, { ifExists: 'return' }) @@ -22,7 +21,7 @@ before(async () => { }) after(async () => { - await Nameserver.destroy({ id: case2Id }) + if (case2Id) await Nameserver.destroy({ id: case2Id }) await server.stop() }) @@ -53,9 +52,9 @@ describe('nameserver routes', () => { assert.equal(res.result.nameserver[0].name, nsCase.name) }) - it(`POST /nameserver (${case2Id})`, async () => { + it('POST /nameserver allocates an id', async () => { const testCase = JSON.parse(JSON.stringify(nsCase)) - testCase.id = case2Id // make it unique + delete testCase.id testCase.name = 'c.ns.example.com.' const res = await server.inject({ @@ -65,32 +64,35 @@ describe('nameserver routes', () => { payload: testCase, }) assert.equal(res.statusCode, 201) + assert.equal(res.result.nameserver.length, 1) + assert.equal(res.result.nameserver[0].name, 'c.ns.example.com.') assert.ok(res.result.nameserver[0].gid) + case2Id = res.result.nameserver[0].id + assert.ok(Number.isInteger(case2Id)) }) - it(`GET /nameserver/${case2Id}`, async () => { + it('PUT /nameserver/{id} keeps using the route id', async () => { const res = await server.inject({ - method: 'GET', + method: 'PUT', url: `/nameserver/${case2Id}`, headers: auth.headers, + payload: { description: 'updated' }, }) assert.equal(res.statusCode, 200) - assert.ok(res.result.nameserver[0].gid) + assert.equal(res.result.nameserver[0].description, 'updated') }) - it(`PUT /nameserver/${case2Id}`, async () => { + it('GET /nameserver/{id}', async () => { const res = await server.inject({ - method: 'PUT', + method: 'GET', url: `/nameserver/${case2Id}`, headers: auth.headers, - payload: { description: 'edited by the route test' }, }) - assert.equal(res.statusCode, 200) - assert.equal((await Nameserver.get({ id: case2Id }))[0].description, 'edited by the route test') + assert.ok(res.result.nameserver[0].gid) }) - it(`DELETE /nameserver/${case2Id}`, async () => { + it('DELETE /nameserver/{id}', async () => { const res = await server.inject({ method: 'DELETE', url: `/nameserver/${case2Id}`, @@ -99,7 +101,7 @@ describe('nameserver routes', () => { assert.equal(res.statusCode, 200) }) - it(`DELETE /nameserver/${case2Id}`, async () => { + it('DELETE /nameserver/{id} returns 404 when already deleted', async () => { const res = await server.inject({ method: 'DELETE', url: `/nameserver/${case2Id}`, @@ -108,7 +110,7 @@ describe('nameserver routes', () => { assert.equal(res.statusCode, 404) }) - it(`GET /nameserver/${case2Id}`, async () => { + it('GET /nameserver/{id} hides a soft-deleted nameserver', async () => { const res = await server.inject({ method: 'GET', url: `/nameserver/${case2Id}`, @@ -117,7 +119,7 @@ describe('nameserver routes', () => { assert.deepEqual(res.result.nameserver, []) }) - it(`GET /nameserver/${case2Id} (deleted)`, async () => { + it('GET /nameserver/{id}?deleted=true returns a soft-deleted nameserver', async () => { const res = await server.inject({ method: 'GET', url: `/nameserver/${case2Id}?deleted=true`, diff --git a/routes/permission.test.js b/routes/permission.test.js index 39069b8..eafac7c 100644 --- a/routes/permission.test.js +++ b/routes/permission.test.js @@ -53,7 +53,7 @@ describe('permission routes', () => { assert.equal(res.result.permission.nameserver.create, false) }) - it('POST /permission', async () => { + it('POST /permission allocates an id', async () => { const testCase = JSON.parse(JSON.stringify(permCase)) delete testCase.id testCase.user.id = targetId @@ -68,9 +68,22 @@ describe('permission routes', () => { payload: testCase, }) assert.equal(res.statusCode, 201) - case2Id = res.result.permission.id + assert.equal(res.result.permission.name, 'Route Test Permission 2') assert.equal(res.result.permission.zone.create, true) assert.equal(res.result.permission.nameserver.create, false) + case2Id = res.result.permission.id + assert.ok(Number.isInteger(case2Id)) + }) + + it('POST /permission with no user or group', async () => { + const res = await server.inject({ + method: 'POST', + url: '/permission', + headers: auth.headers, + payload: { name: 'No Target Route Test' }, + }) + assert.equal(res.statusCode, 400) + assert.match(res.result.message, /needs a user or group/) }) it('POST /permission rejects an existing target', async () => { @@ -94,7 +107,7 @@ describe('permission routes', () => { assert.equal(existing.name, 'Route Test Permission 2') }) - it('GET the created permission', async () => { + it('GET /permission/{id}', async () => { const res = await server.inject({ method: 'GET', url: `/permission/${case2Id}`, @@ -106,7 +119,7 @@ describe('permission routes', () => { assert.equal(res.result.permission.nameserver.create, false) }) - it('DELETE the created permission', async () => { + it('DELETE /permission/{id}', async () => { const res = await server.inject({ method: 'DELETE', url: `/permission/${case2Id}`, @@ -116,7 +129,7 @@ describe('permission routes', () => { assert.equal(res.statusCode, 200) }) - it('DELETE the created permission again', async () => { + it('DELETE /permission/{id} returns 404 when already deleted', async () => { const res = await server.inject({ method: 'DELETE', url: `/permission/${case2Id}`, @@ -126,7 +139,7 @@ describe('permission routes', () => { assert.equal(res.statusCode, 404) }) - it('GET the deleted permission', async () => { + it('GET /permission/{id} hides a soft-deleted permission', async () => { const res = await server.inject({ method: 'GET', url: `/permission/${case2Id}`, @@ -137,7 +150,7 @@ describe('permission routes', () => { assert.equal(res.result.permission, undefined) }) - it('GET the deleted permission with deleted=true', async () => { + it('GET /permission/{id}?deleted=true returns a soft-deleted permission', async () => { const res = await server.inject({ method: 'GET', url: `/permission/${case2Id}?deleted=true`, diff --git a/routes/user.test.js b/routes/user.test.js index fa97de4..b11987f 100644 --- a/routes/user.test.js +++ b/routes/user.test.js @@ -17,10 +17,10 @@ before(async () => { await User.create(userCase, { ifExists: 'return' }) }) -const userId2 = 4094 +let userId2 after(async () => { - User.destroy({ id: userId2 }) + if (userId2) await User.destroy({ id: userId2 }) await server.stop() }) @@ -57,9 +57,9 @@ describe('user routes', () => { assert.equal(res.statusCode, 200) }) - it('POST /user', async () => { + it('POST /user allocates an id', async () => { const testCase = JSON.parse(JSON.stringify(userCase)) - testCase.id = userId2 // make it unique + delete testCase.id testCase.username = `${testCase.username}2` delete testCase.deleted @@ -70,9 +70,24 @@ describe('user routes', () => { payload: testCase, }) assert.equal(res.statusCode, 201) + assert.equal(res.result.user.length, 1) + assert.equal(res.result.user[0].username, `${userCase.username}2`) + userId2 = res.result.user[0].id + assert.ok(Number.isInteger(userId2)) }) - it(`GET /user/${userId2}`, async () => { + it('PUT /user/{id} keeps using the route id', async () => { + const res = await server.inject({ + method: 'PUT', + url: `/user/${userId2}`, + headers: auth.headers, + payload: { first_name: 'Updated' }, + }) + assert.equal(res.statusCode, 200) + assert.equal(res.result.user[0].first_name, 'Updated') + }) + + it('GET /user/{id}', async () => { const res = await server.inject({ method: 'GET', url: `/user/${userId2}`, @@ -81,7 +96,7 @@ describe('user routes', () => { assert.equal(res.statusCode, 200) }) - it(`DELETE /user/${userId2}`, async () => { + it('DELETE /user/{id}', async () => { const res = await server.inject({ method: 'DELETE', url: `/user/${userId2}`, @@ -90,7 +105,7 @@ describe('user routes', () => { assert.equal(res.statusCode, 200) }) - it(`GET /user/${userId2} (deleted)`, async () => { + it('GET /user/{id} hides a soft-deleted user', async () => { const res = await server.inject({ method: 'GET', url: `/user/${userId2}`, @@ -99,7 +114,7 @@ describe('user routes', () => { assert.ok([200, 204].includes(res.statusCode)) }) - it(`GET /user/${userId2}?deleted=true`, async () => { + it('GET /user/{id}?deleted=true returns a soft-deleted user', async () => { const res = await server.inject({ method: 'GET', url: `/user/${userId2}?deleted=true`, diff --git a/routes/zone.test.js b/routes/zone.test.js index 7e88339..1120ab3 100644 --- a/routes/zone.test.js +++ b/routes/zone.test.js @@ -11,14 +11,13 @@ import userCase from './test/user.json' with { type: 'json' } import nsCase from './test/zone.json' with { type: 'json' } let server -let case2Id = 4094 +let case2Id 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 }) // Destroy the subgroup before recreating it: a lingering row would make // Group.create early-return and skip addToSubgroups, leaving the @@ -33,6 +32,7 @@ before(async () => { }) after(async () => { + if (case2Id) await Zone.destroy({ id: case2Id }) await Zone.destroy({ id: subZone.id }) await Group.destroy({ id: subGroup.id }) await server.stop() @@ -85,10 +85,10 @@ describe('zone routes', () => { assert.ok(res.result.zone.some((z) => z.zone === nsCase.zone)) }) - it(`POST /zone (${case2Id})`, async () => { + it('POST /zone allocates an id', async () => { const testCase = JSON.parse(JSON.stringify(nsCase)) - testCase.id = case2Id // make it unique - testCase.gid = case2Id + delete testCase.id + testCase.gid = groupCase.id testCase.zone = 'route2.example.com.' const res = await server.inject({ @@ -99,33 +99,36 @@ describe('zone routes', () => { }) // console.log(res.result) assert.equal(res.statusCode, 201) + assert.equal(res.result.zone.length, 1) + assert.equal(res.result.zone[0].zone, 'route2.example.com.') assert.ok(res.result.zone[0].gid) + case2Id = res.result.zone[0].id + assert.ok(Number.isInteger(case2Id)) }) - it(`GET /zone/${case2Id}`, async () => { + it('PUT /zone/{id} keeps using the route id', async () => { const res = await server.inject({ - method: 'GET', + method: 'PUT', url: `/zone/${case2Id}`, headers: auth.headers, + payload: { description: 'updated' }, }) - // console.log(res.result) assert.equal(res.statusCode, 200) - assert.ok(res.result.zone[0].gid) + assert.equal(res.result.zone[0].description, 'updated') }) - it(`PUT /zone/${case2Id}`, async () => { + it('GET /zone/{id}', async () => { const res = await server.inject({ - method: 'PUT', + method: 'GET', url: `/zone/${case2Id}`, headers: auth.headers, - payload: { description: 'edited by the route test' }, }) - + // console.log(res.result) assert.equal(res.statusCode, 200) - assert.equal((await Zone.get({ id: case2Id }))[0].description, 'edited by the route test') + assert.ok(res.result.zone[0].gid) }) - it(`DELETE /zone/${case2Id}`, async () => { + it('DELETE /zone/{id}', async () => { const res = await server.inject({ method: 'DELETE', url: `/zone/${case2Id}`, @@ -135,7 +138,7 @@ describe('zone routes', () => { assert.equal(res.statusCode, 200) }) - it(`DELETE /zone/${case2Id}`, async () => { + it('DELETE /zone/{id} returns 404 when already deleted', async () => { const res = await server.inject({ method: 'DELETE', url: `/zone/${case2Id}`, @@ -145,7 +148,7 @@ describe('zone routes', () => { assert.equal(res.statusCode, 404) }) - it(`GET /zone/${case2Id}`, async () => { + it('GET /zone/{id} hides a soft-deleted zone', async () => { const res = await server.inject({ method: 'GET', url: `/zone/${case2Id}`, @@ -156,7 +159,7 @@ describe('zone routes', () => { assert.deepEqual(res.result.zone, []) }) - it(`GET /zone/${case2Id} (deleted)`, async () => { + it('GET /zone/{id}?deleted=true returns a soft-deleted zone', async () => { const res = await server.inject({ method: 'GET', url: `/zone/${case2Id}?deleted=true`, diff --git a/routes/zone_record.test.js b/routes/zone_record.test.js index 9aba2e2..d94620f 100644 --- a/routes/zone_record.test.js +++ b/routes/zone_record.test.js @@ -93,24 +93,6 @@ describe('zone_record routes', () => { auth.headers = { Authorization: `Bearer ${res.result.session.token}` } }) - it('POST /zone_record returns 409 for an existing id', async () => { - const res = await server.inject({ - method: 'POST', - url: '/zone_record', - headers: auth.headers, - payload: { - ...testZoneRecord, - owner: 'changed.route-zr-delete.example.com.', - }, - }) - - assert.equal(res.statusCode, 409) - assert.equal(res.result.message, `zone record id ${testZoneRecordId} already exists`) - - const [existing] = await ZoneRecord.get({ id: testZoneRecordId }) - assert.equal(existing.owner, testZoneRecord.owner) - }) - it('POST /zone_record creates and returns array payload', async () => { const res = await server.inject({ method: 'POST', @@ -134,6 +116,19 @@ describe('zone_record routes', () => { createdZoneRecordIds.push(res.result.zone_record[0].id) }) + it('PUT /zone_record/{id} keeps using the route id', async () => { + const id = createdZoneRecordIds[0] + const res = await server.inject({ + method: 'PUT', + url: `/zone_record/${id}`, + headers: auth.headers, + payload: { description: 'updated' }, + }) + + assert.equal(res.statusCode, 200) + assert.equal(res.result.zone_record[0].description, 'updated') + }) + it('POST /zone_record accepts omitted ttl and stores 0', async () => { const res = await server.inject({ method: 'POST', diff --git a/test/backends/json.sh b/test/backends/json.sh index d96349e..81ed93c 100755 --- a/test/backends/json.sh +++ b/test/backends/json.sh @@ -17,8 +17,6 @@ test_files() { } run_tests() { - # Run serially: the file store uses shared files; parallel workers cause concurrent-write corruption - for f in $(test_files); do - $NODE --test --test-reporter=spec "$f" || exit 1 - done + # shellcheck disable=SC2046 # word splitting is how the file list is passed + $NODE --test --test-concurrency=1 "$@" $(test_files) } diff --git a/test/backends/mysql.sh b/test/backends/mysql.sh index fdcaf47..fe074c6 100755 --- a/test/backends/mysql.sh +++ b/test/backends/mysql.sh @@ -23,5 +23,5 @@ test_files() { run_tests() { # shellcheck disable=SC2046 # word splitting is how the file list is passed - $NODE --test --test-reporter=spec $(test_files) + $NODE --test --test-concurrency=1 "$@" $(test_files) } diff --git a/test/backends/toml.sh b/test/backends/toml.sh index 345cdf3..e09436e 100755 --- a/test/backends/toml.sh +++ b/test/backends/toml.sh @@ -17,8 +17,6 @@ test_files() { } run_tests() { - # Run serially: TOML uses shared files; parallel workers cause concurrent-write corruption - for f in $(test_files); do - $NODE --test --test-reporter=spec "$f" || exit 1 - done + # shellcheck disable=SC2046 # word splitting is how the file list is passed + $NODE --test --test-concurrency=1 "$@" $(test_files) } diff --git a/test/run.sh b/test/run.sh index 5c22c5d..48ae343 100755 --- a/test/run.sh +++ b/test/run.sh @@ -39,19 +39,18 @@ if [ $# -ge 1 ]; then if [ "$1" = "watch" ]; then $NODE --test --watch elif [ "$1" = "coverage" ]; then - # shellcheck disable=SC2046 # word splitting is how the file list is passed - $NODE --test --experimental-test-coverage $COVERAGE_EXCLUDE $(test_files) + # shellcheck disable=SC2086 # word splitting passes each exclusion separately + run_tests --experimental-test-coverage $COVERAGE_EXCLUDE elif [ "$1" = "coverage:lcov" ]; then mkdir -p coverage # lcov alone writes only to the file, so a failure exits 1 with an empty log - # shellcheck disable=SC2046 # word splitting is how the file list is passed - $NODE --test --experimental-test-coverage $COVERAGE_EXCLUDE \ + # shellcheck disable=SC2086 # word splitting passes each exclusion separately + run_tests --experimental-test-coverage $COVERAGE_EXCLUDE \ --test-reporter=lcov --test-reporter-destination=coverage/lcov.info \ - --test-reporter=spec --test-reporter-destination=stdout \ - $(test_files) + --test-reporter=spec --test-reporter-destination=stdout else $NODE --test --test-reporter=spec "$1" fi else - run_tests + run_tests --test-reporter=spec fi diff --git a/test/seed.js b/test/seed.js new file mode 100644 index 0000000..0a2bd12 --- /dev/null +++ b/test/seed.js @@ -0,0 +1,6 @@ +export function seed(store, key, rows) { + return store.mutate(key, (current, data) => { + current.splice(0, current.length, ...rows) + delete data.last_id + }) +}