Extracted gift-links side effects into ports

Kept a single GiftLinksService but pulled the action-log write out into
actions.ts (injected as a port rather than reaching for the model), moved
the DB row<->domain codec into codec.ts, renamed database.ts to schema.ts,
and moved token generation onto the model. Replaced the DB-backed
service-level integration suite with unit tests plus HTTP-level e2e
coverage for the not-found and concurrency cases.

Author: rob-ghost

ghost/core/core/server/services/gift-links/actions.ts

@@ -0,0 +1,51 @@
+import logging from '@tryghost/logging';
+
+export interface Actor {
+    id: string;
+    type: 'user' | 'integration';
+}
+
+export interface RequestContext {
+    actor: Actor | null;
+}
+
+export interface ActionRecorder {
+    add(data: Record<string, unknown>, options: {autoRefresh: boolean}): Promise<unknown>;
+}
+
+// The history UI only surfaces a verb-specific label (action_name) for 'edited' events; 'added' and
+// 'deleted' render as the bare event. So 'reset' maps to 'edited' to read as "reset", while
+// 'add'/'remove' read as plain "added"/"deleted".
+const COMMANDS = {
+    add: 'added',
+    reset: 'edited',
+    remove: 'deleted'
+} as const satisfies Record<string, 'added' | 'edited' | 'deleted'>;
+
+export type GiftLinkVerb = keyof typeof COMMANDS;
+
+export type RecordGiftLinkAction =
+    (input: {context: RequestContext; verb: GiftLinkVerb; subject: string | null}) => Promise<void>;
+
+// Best-effort action-log write: a failed action must never fail the command that triggered it.
+export async function recordGiftLinkAction(
+    {Action, context, verb, subject}:
+    {Action: ActionRecorder; context: RequestContext; verb: GiftLinkVerb; subject: string | null}
+): Promise<void> {
+    if (!context.actor) {
+        return;
+    }
+    const event = COMMANDS[verb];
+    try {
+        await Action.add({
+            event,
+            resource_type: 'gift_link',
+            resource_id: subject,
+            actor_type: context.actor.type,
+            actor_id: context.actor.id,
+            ...(event === 'edited' ? {context: {action_name: verb}} : {})
+        }, {autoRefresh: false});
+    } catch (err) {
+        logging.error(err);
+    }
+}

ghost/core/core/server/services/gift-links/codec.ts

@@ -0,0 +1,16 @@
+import {z} from 'zod';
+import {camelKeys, snakeKeys} from './case-keys';
+import {DbGiftLink} from './schema';
+import {GiftLink} from './models';
+
+export const GiftLinkRow = DbGiftLink.pick({
+    token: true,
+    created_at: true
+});
+
+export const giftLinkCodec = z.codec(GiftLinkRow, GiftLink, {
+    decode: row => camelKeys(row),
+    encode: link => snakeKeys(link)
+});
+
+export const giftLinkColumns = Object.keys(GiftLinkRow.shape).map(column => `gift_links.${column}`);

ghost/core/core/server/services/gift-links/index.ts

@@ -1,8 +1,9 @@
 import {GiftLinksService} from './service';
+import {recordGiftLinkAction, type RecordGiftLinkAction} from './actions';
 
-export type {RequestContext} from './service';
+export type {RequestContext} from './actions';
 
-// Set by init() at boot, not at import: knex only exists once the DB has connected.
+// Constructed by init() at boot, not at import: knex is only available once the DB has connected.
 export let service: GiftLinksService | undefined;
 
 export function init(): void {
@@ -13,5 +14,7 @@ export function init(): void {
     const {knex} = require('../../data/db');
     const models = require('../../models');
 
-    service = new GiftLinksService({knex, Action: models.Action});
+    const recordAction: RecordGiftLinkAction = ({context, verb, subject}) =>
+        recordGiftLinkAction({Action: models.Action, context, verb, subject});
+    service = new GiftLinksService({knex, recordAction});
 }

ghost/core/core/server/services/gift-links/models.ts

@@ -1,15 +1,21 @@
+import crypto from 'crypto';
 import {z} from 'zod';
 
 export const GiftLinkToken = z.string().brand('GiftLinkToken');
 export type GiftLinkToken = z.infer<typeof GiftLinkToken>;
 
+// 24 random bytes (192 bits of entropy); base64url keeps it URL-safe.
+export function generateGiftLinkToken(): GiftLinkToken {
+    return GiftLinkToken.parse(crypto.randomBytes(24).toString('base64url'));
+}
+
 export const GiftLink = z.object({
     token: GiftLinkToken,
     createdAt: z.date()
 });
 export type GiftLink = z.infer<typeof GiftLink>;
 
-/** The gift-links aggregate of a post and its live links; distinct from the Bookshelf Post model. */
+/** A post and its live gift links — distinct from the Bookshelf Post model. */
 export interface Post {
     id: string;
     giftLinks: GiftLink[];

ghost/core/core/server/services/gift-links/queries.ts

@@ -2,14 +2,11 @@ import {z} from 'zod';
 import type {Knex} from 'knex';
 
 // MySQL returns a Date, SQLite a string/number; normalise to Date on read.
-// Lives here with the row schema that uses it until a second table needs it.
 const DbDate = z.codec(z.union([z.date(), z.string(), z.number()]), z.date(), {
     decode: value => new Date(value),
     encode: date => date
 });
 
-// The gift_links table row: the single source for the read projection (queries.ts) and the knex
-// types below.
 export const DbGiftLink = z.object({
     token: z.string(),
     post_id: z.string(),
@@ -25,7 +22,6 @@ interface DbPostGiftLink {
     updated_at: Date | null;
 }
 
-// knex table types, derived from the schemas above so each row shape has a single source.
 declare module 'knex/types/tables' {
     interface Tables {
         gift_links: Knex.CompositeTableType<

…e/server/services/gift-links/database.ts …ore/server/services/gift-links/schema.tsghost/core/core/server/services/gift-links/database.ts renamed to ghost/core/core/server/services/gift-links/schema.ts ghost/core/core/server/services/gift-links/database.ts renamed to ghost/core/core/server/services/gift-links/schema.ts

@@ -2,7 +2,6 @@ import {z} from 'zod';
 import {snakeKeys} from './case-keys';
 import {GiftLink} from './models';
 
-// Response schemas — the shapes the admin endpoints emit.
 const GiftLinkResource = z.object({
     token: z.string(),
     created_at: z.date()

ghost/core/core/server/services/gift-links/serializers.ts

@@ -1,127 +1,99 @@
-import crypto from 'crypto';
-import errors from '@tryghost/errors';
-import logging from '@tryghost/logging';
 import {z} from 'zod';
+import errors from '@tryghost/errors';
 import type {Knex} from 'knex';
-import {GiftLinkToken, type GiftLink, type Post} from './models';
-import * as queries from './queries';
-
-export function generateGiftLinkToken(): GiftLinkToken {
-    return GiftLinkToken.parse(crypto.randomBytes(24).toString('base64url'));
-}
+import {GiftLinkRow, giftLinkCodec, giftLinkColumns} from './codec';
+import {generateGiftLinkToken, type GiftLink, type Post} from './models';
+import {type RecordGiftLinkAction, type RequestContext} from './actions';
 
-export interface Actor {
-    id: string;
-    type: 'user' | 'integration';
-}
-
-interface ActionRecorder {
-    add(data: Record<string, unknown>, options: {autoRefresh: boolean}): Promise<unknown>;
-}
-
-export interface RequestContext {
-    actor: Actor | null;
-}
-
-// The history UI only surfaces a verb-specific label (action_name) for 'edited' events; 'added' and
-// 'deleted' render as the bare event. So 'reset' maps to 'edited' to read as "reset", while
-// 'add'/'remove' read as plain "added"/"deleted".
-const COMMANDS = {
-    add: 'added',
-    reset: 'edited',
-    remove: 'deleted'
-} as const satisfies Record<string, 'added' | 'edited' | 'deleted'>;
-
-type GiftLinkVerb = keyof typeof COMMANDS;
+// The LEFT JOIN leaves every link column nullable; the explicit generic names a row shape knex
+// can't infer from a dynamic column list.
+type LiveLinkRow = {[K in keyof z.input<typeof GiftLinkRow>]: z.input<typeof GiftLinkRow>[K] | null};
 
 export class GiftLinksService {
     private knex: Knex;
-    private Action: ActionRecorder;
+    private recordAction: RecordGiftLinkAction;
 
-    constructor({knex, Action}: {knex: Knex; Action: ActionRecorder}) {
+    constructor({knex, recordAction}: {knex: Knex; recordAction: RecordGiftLinkAction}) {
         this.knex = knex;
-        this.Action = Action;
+        this.recordAction = recordAction;
     }
 
     async getPost(postId: string): Promise<Post> {
-        return this.requirePost(postId);
+        // Anchored on posts: zero rows means the post itself doesn't exist, not merely that it has
+        // no live link.
+        const rows = await this.knex('posts')
+            .where('posts.id', postId)
+            .leftJoin('post_gift_links', 'post_gift_links.post_id', 'posts.id')
+            .leftJoin('gift_links', 'gift_links.token', 'post_gift_links.gift_link_token')
+            .select<LiveLinkRow[]>(giftLinkColumns);
+
+        if (rows.length === 0) {
+            throw new errors.NotFoundError({message: `Post ${postId} does not exist.`});
+        }
+
+        const giftLinks = rows
+            .filter((row): row is z.input<typeof GiftLinkRow> => row.token !== null)
+            .map(row => z.decode(giftLinkCodec, row));
+        return {id: postId, giftLinks};
     }
 
     async getPostByToken(token: string): Promise<Post | null> {
-        const row = await queries.liveLinkForToken(token)(this.knex);
-        return row ? {id: row.post_id, giftLinks: [z.decode(queries.giftLinkCodec, row)]} : null;
+        const row = await this.knex('post_gift_links')
+            .join('gift_links', 'gift_links.token', 'post_gift_links.gift_link_token')
+            .where('gift_links.token', token)
+            .first<z.input<typeof GiftLinkRow> & {post_id: string}>(
+                [...giftLinkColumns, 'post_gift_links.post_id as post_id']
+            );
+        return row ? {id: row.post_id, giftLinks: [z.decode(giftLinkCodec, row)]} : null;
     }
 
     async isValidTokenForPost(token: string, postId: string): Promise<boolean> {
         return (await this.getPostByToken(token))?.id === postId;
     }
 
     async ensure(context: RequestContext, postId: string): Promise<Post> {
-        const post = await this.requirePost(postId);
+        const post = await this.getPost(postId);
         if (post.giftLinks.length) {
             return post;
         }
         const minted = await this.mint(postId);
-        await this.recordAction(context, 'add', postId);
+        await this.recordAction({context, verb: 'add', subject: postId});
         return minted;
     }
 
     async create(context: RequestContext, postId: string): Promise<Post> {
-        await this.requirePost(postId);
+        await this.getPost(postId); // asserts the post exists (throws NotFound)
         const minted = await this.mint(postId);
-        await this.recordAction(context, 'reset', postId);
+        await this.recordAction({context, verb: 'reset', subject: postId});
         return minted;
     }
 
-    // Remove every live association; the gift_links rows stay as history.
+    // gift_links rows are kept as history; only the live association is removed.
     async removeAll(context: RequestContext): Promise<number> {
         const removed = await this.knex('post_gift_links').del();
         if (removed > 0) {
-            await this.recordAction(context, 'remove', null);
+            await this.recordAction({context, verb: 'remove', subject: null});
         }
         return removed;
     }
 
-    private async requirePost(postId: string): Promise<Post> {
-        const rows = await queries.liveLinksForPost(postId)(this.knex);
-        if (rows.length === 0) {
-            throw new errors.NotFoundError({message: `Post ${postId} does not exist.`});
-        }
-        const giftLinks = rows
-            .filter((row): row is z.input<typeof queries.GiftLinkRow> => row.token !== null)
-            .map(row => z.decode(queries.giftLinkCodec, row));
-        return {id: postId, giftLinks};
-    }
-
     private async mint(postId: string): Promise<Post> {
-        const now = new Date();
-        const link: GiftLink = {token: generateGiftLinkToken(), createdAt: now};
+        const link: GiftLink = {token: generateGiftLinkToken(), createdAt: new Date()};
         await this.knex.transaction(async (trx) => {
-            await trx('gift_links').insert({...z.encode(queries.giftLinkCodec, link), post_id: postId});
-            await trx('post_gift_links')
-                .insert({post_id: postId, gift_link_token: link.token, created_at: now})
-                .onConflict('post_id')
-                .merge({gift_link_token: link.token, updated_at: now});
+            await this.addToHistory(trx, postId, link);
+            await this.setLiveLink(trx, postId, link);
         });
         return {id: postId, giftLinks: [link]};
     }
 
-    private async recordAction(context: RequestContext, verb: GiftLinkVerb, subject: string | null): Promise<void> {
-        if (!context.actor) {
-            return;
-        }
-        const event = COMMANDS[verb];
-        try {
-            await this.Action.add({
-                event,
-                resource_type: 'gift_link',
-                resource_id: subject,
-                actor_type: context.actor.type,
-                actor_id: context.actor.id,
-                ...(event === 'edited' ? {context: {action_name: verb}} : {})
-            }, {autoRefresh: false});
-        } catch (err) {
-            logging.error(err);
-        }
+    private addToHistory(trx: Knex.Transaction, postId: string, link: GiftLink) {
+        return trx('gift_links').insert({...z.encode(giftLinkCodec, link), post_id: postId});
+    }
+
+    private setLiveLink(trx: Knex.Transaction, postId: string, link: GiftLink) {
+        return trx('post_gift_links')
+            .insert({post_id: postId, gift_link_token: link.token, created_at: link.createdAt})
+            .onConflict('post_id')
+            .merge({gift_link_token: link.token, updated_at: link.createdAt});
     }
 }

ghost/core/core/server/services/gift-links/service.ts

@@ -116,6 +116,26 @@ describe('Gift Links Admin API', function () {
         });
     });
 
+    describe('a post that does not exist', function () {
+        const MISSING_POST_ID = '0123456789abcdef01234567';
+
+        it('404s on GET, PUT and POST', async function () {
+            await agent.get(`posts/${MISSING_POST_ID}/gift_links/`).expectStatus(404);
+            await agent.put(`posts/${MISSING_POST_ID}/gift_links/`).expectStatus(404);
+            await agent.post(`posts/${MISSING_POST_ID}/gift_links/`).expectStatus(404);
+        });
+    });
+
+    it('concurrent ensures settle on a single live link (last writer wins)', async function () {
+        await Promise.all([
+            agent.put(`posts/${postId}/gift_links/`).expectStatus(200),
+            agent.put(`posts/${postId}/gift_links/`).expectStatus(200)
+        ]);
+
+        const {body} = await agent.get(`posts/${postId}/gift_links/`).expectStatus(200);
+        assert.equal(body.gift_links.length, 1);
+    });
+
     describe('records actions in the history (via the actions API)', function () {
         let actorId: string;