diff --git a/helpers/mock-agent/endpoint-files.js b/helpers/mock-agent/endpoint-files.js index 06f01b227..48c52781a 100644 --- a/helpers/mock-agent/endpoint-files.js +++ b/helpers/mock-agent/endpoint-files.js @@ -35,23 +35,60 @@ export function mockClient() { }) .persist(); - // Get source information for all items from external media endpoint + // A default-sized listing page (the shape `q=source` returns with no + // `uid`/`url` and no explicit `limit`) that does not include the file + // fetched by uid below — it's older than the newest page of uploads. + const items = Array.from({ length: 40 }, (_, index) => ({ + uid: `other-${index}`, + url: `https://website.example/other-${index}.jpg`, + })); + agent .get(mediaEndpointOrigin) .intercept({ path: "/?q=source", }) + .reply(200, { items }) + .persist(); + + // Get source information for a single file by uid from external media endpoint + agent + .get(mediaEndpointOrigin) + .intercept({ + path: "/", + query: { q: "source", uid: "123" }, + }) .reply(200, { - items: [ - { - uid: "123", - url: photoOrigin, - }, - { - uid: "401", - url: photoBadOrigin, - }, - ], + uid: "123", + "media-type": "photo", + url: photoOrigin, + }) + .persist(); + + agent + .get(mediaEndpointOrigin) + .intercept({ + path: "/", + query: { q: "source", uid: "401" }, + }) + .reply(200, { + uid: "401", + "media-type": "photo", + url: photoBadOrigin, + }) + .persist(); + + // Get source information for a file older than the newest page of uploads + agent + .get(mediaEndpointOrigin) + .intercept({ + path: "/", + query: { q: "source", uid: "target-uid" }, + }) + .reply(200, { + uid: "target-uid", + "media-type": "photo", + url: "https://website.example/target.jpg", }) .persist(); diff --git a/helpers/mock-agent/endpoint-posts.js b/helpers/mock-agent/endpoint-posts.js index ebbf1d76d..67fe75415 100644 --- a/helpers/mock-agent/endpoint-posts.js +++ b/helpers/mock-agent/endpoint-posts.js @@ -35,34 +35,39 @@ export function mockClient() { }) .persist(); - // Get source information for all items from external micropub endpoint + // Get source information for a single post by uid from external micropub endpoint agent .get(micropubEndpointOrigin) .intercept({ - path: "/?q=source", + path: "/", + query: { q: "source", uid: "123" }, }) .reply(200, { - items: [ - { - type: ["h-entry"], - properties: { - uid: ["123"], - name: ["Foobar"], - "post-type": ["note"], - published: ["2024-12-21"], - url: [postOrigin], - }, - }, - { - type: ["h-entry"], - properties: { - uid: ["401"], - name: ["401"], - "post-type": ["note"], - url: [postBadOrigin], - }, - }, - ], + type: ["h-entry"], + properties: { + uid: ["123"], + name: ["Foobar"], + "post-type": ["note"], + published: ["2024-12-21"], + url: [postOrigin], + }, + }) + .persist(); + + agent + .get(micropubEndpointOrigin) + .intercept({ + path: "/", + query: { q: "source", uid: "401" }, + }) + .reply(200, { + type: ["h-entry"], + properties: { + uid: ["401"], + name: ["401"], + "post-type": ["note"], + url: [postBadOrigin], + }, }) .persist(); diff --git a/packages/endpoint-files/lib/utils.js b/packages/endpoint-files/lib/utils.js index cc94b6533..4ff8651ba 100644 --- a/packages/endpoint-files/lib/utils.js +++ b/packages/endpoint-files/lib/utils.js @@ -1,5 +1,7 @@ import { Buffer } from "node:buffer"; +import { IndiekitError } from "@indiekit/error"; + import { endpoint } from "./endpoint.js"; /** @@ -7,19 +9,28 @@ import { endpoint } from "./endpoint.js"; * @param {string} uid - Item UID * @param {string} mediaEndpoint - Micropub media endpoint * @param {string} accessToken - Access token - * @returns {Promise} JF2 properties + * @returns {Promise} JF2 properties, or false if not found */ export const getFileProperties = async (uid, mediaEndpoint, accessToken) => { const mediaUrl = new URL(mediaEndpoint); mediaUrl.searchParams.append("q", "source"); + mediaUrl.searchParams.append("uid", uid); - const mediaResponse = await endpoint.get(mediaUrl.href, accessToken); + try { + // `q=source&uid=` returns properties for a single file, already flat + // JF2 (unlike the Micropub equivalent, the media endpoint has no mf2 + // to convert), so there’s nothing to unwrap before returning it. + return await endpoint.get(mediaUrl.href, accessToken); + } catch (error) { + // `endpoint.get` throws on any error response. A file that is simply + // gone is the caller’s own not-found page, not an error to show the + // reader; anything else is a real failure and must keep travelling. + if (error instanceof IndiekitError && error.status === 404) { + return false; + } - if (mediaResponse?.items?.length > 0) { - return mediaResponse.items.find((item) => item.uid === uid); + throw error; } - - return false; }; /** diff --git a/packages/endpoint-files/test/unit/utils.js b/packages/endpoint-files/test/unit/utils.js index 5a3471307..a95082aeb 100644 --- a/packages/endpoint-files/test/unit/utils.js +++ b/packages/endpoint-files/test/unit/utils.js @@ -1,7 +1,12 @@ import { strict as assert } from "node:assert"; import { describe, it } from "node:test"; -import { getFileName, getFileUrl } from "../../lib/utils.js"; +import { mockAgent } from "@indiekit-test/mock-agent"; +import { testToken } from "@indiekit-test/token"; + +import { getFileName, getFileProperties, getFileUrl } from "../../lib/utils.js"; + +await mockAgent("endpoint-files"); describe("endpoint-files/lib/utils", () => { it("Gets file name from a URL", () => { @@ -15,4 +20,14 @@ describe("endpoint-files/lib/utils", () => { "https://website.example/foobar", ); }); + + it("Fetches a file that isn’t on the media endpoint’s first page of results", async () => { + const result = await getFileProperties( + "target-uid", + "https://media-endpoint.example", + testToken(), + ); + + assert.equal(result.uid, "target-uid"); + }); }); diff --git a/packages/endpoint-media/lib/controllers/query.js b/packages/endpoint-media/lib/controllers/query.js index d56d7baf8..35bd806f0 100644 --- a/packages/endpoint-media/lib/controllers/query.js +++ b/packages/endpoint-media/lib/controllers/query.js @@ -9,6 +9,7 @@ import { getMediaProperties } from "../utils.js"; * @property {string} [before] - Return items before this item ID * @property {string} [limit] - Number of items to return * @property {string} [q] - Query + * @property {string} [uid] - UID of file to return * @property {string} [url] - URL of post to return */ @@ -22,7 +23,7 @@ export const queryController = async (request, response, next) => { try { const limit = Number(request.query.limit) || 0; - const { after, before, q, url } = request.query; + const { after, before, q, uid, url } = request.query; if (!q) { throw IndiekitError.badRequest( @@ -32,20 +33,26 @@ export const queryController = async (request, response, next) => { switch (q) { case "source": { - if (url) { - // Return properties for a given URL + if (url || uid) { + // Return properties for a given file. `url` is what the + // Micropub specification defines; `uid` is an extension, and + // the only identifier the files interface holds. let mediaData; if (mediaCollection) { - mediaData = await mediaCollection.findOne({ - "properties.url": url, - }); + mediaData = await mediaCollection.findOne( + url ? { "properties.url": url } : { "properties.uid": uid }, + ); } if (!mediaData) { - throw IndiekitError.badRequest( - response.locals.__("BadRequestError.missingResource", "file"), - ); + throw url + ? IndiekitError.badRequest( + response.locals.__("BadRequestError.missingResource", "file"), + ) + : IndiekitError.notFound( + response.locals.__("NotFoundError.record", "file"), + ); } return response.json(getMediaProperties(mediaData)); diff --git a/packages/endpoint-media/lib/media-data.js b/packages/endpoint-media/lib/media-data.js index 5bcd14bc0..a01906c5a 100644 --- a/packages/endpoint-media/lib/media-data.js +++ b/packages/endpoint-media/lib/media-data.js @@ -1,3 +1,5 @@ +import { randomUUIDv7 } from "node:crypto"; + import { IndiekitError } from "@indiekit/error"; import { getCanonicalUrl } from "@indiekit/util"; import makeDebug from "debug"; @@ -57,12 +59,22 @@ export const mediaData = { const urlPathSegment = properties.url.split("/"); properties.filename = urlPathSegment.at(-1); - const mediaData = { path, properties }; - // Add data to media collection (or replace existing if present) const mediaCollection = application?.collections?.get("media"); + const query = { "properties.url": properties.url }; + + // Keep `uid` stable when media already exists at this URL: it is + // meant to be a durable identifier for the media item, so rotating it + // on every update would invalidate identifiers already handed out + // once something starts relying on it staying the same. + const existing = await mediaCollection?.findOne(query, { + projection: { "properties.uid": 1 }, + }); + properties.uid = existing?.properties?.uid || randomUUIDv7(); + + const mediaData = { path, properties }; + if (mediaCollection) { - const query = { "properties.url": properties.url }; await mediaCollection.replaceOne(query, mediaData, { upsert: true }); } diff --git a/packages/endpoint-media/lib/utils.js b/packages/endpoint-media/lib/utils.js index e1d947da8..a321126d9 100644 --- a/packages/endpoint-media/lib/utils.js +++ b/packages/endpoint-media/lib/utils.js @@ -10,7 +10,7 @@ import { mediaTypeCount } from "./media-type-count.js"; */ export const getMediaProperties = (mediaData) => { return { - uid: mediaData._id, + uid: mediaData.properties.uid, "content-type": mediaData.properties["content-type"], "media-type": mediaData.properties["media-type"], published: mediaData.properties.published, diff --git a/packages/endpoint-media/test/integration/200-query-source-uid.js b/packages/endpoint-media/test/integration/200-query-source-uid.js new file mode 100644 index 000000000..8b02d863c --- /dev/null +++ b/packages/endpoint-media/test/integration/200-query-source-uid.js @@ -0,0 +1,52 @@ +import { strict as assert } from "node:assert"; +import { after, before, describe, it } from "node:test"; + +import { testDatabase } from "@indiekit-test/database"; +import { getFixture } from "@indiekit-test/fixtures"; +import { testServer } from "@indiekit-test/server"; +import { testToken } from "@indiekit-test/token"; +import supertest from "supertest"; + +const { client, mongoServer, mongoUri } = await testDatabase(); +const server = await testServer({ + application: { mongodbUrl: mongoUri }, +}); +const request = supertest.agent(server); + +describe("endpoint-media GET /media?q=source&uid=*", () => { + let seededUid; + + before(async () => { + await request + .post("/media") + .auth(testToken(), { type: "bearer" }) + .set("accept", "application/json") + .attach("file", getFixture("file-types/photo.jpg", false), "photo.jpg"); + + const result = await request + .get("/media") + .auth(testToken(), { type: "bearer" }) + .set("accept", "application/json") + .query({ q: "source" }); + + seededUid = result.body.items[0].uid; + }); + + it("Returns properties for a file by uid", async () => { + const result = await request + .get("/media") + .auth(testToken(), { type: "bearer" }) + .set("accept", "application/json") + .query({ q: "source" }) + .query({ uid: seededUid }); + + assert.equal(result.status, 200); + assert.equal(result.body.uid, seededUid); + }); + + after(async () => { + await client.close(); + await mongoServer.stop(); + server.close((error) => process.exit(error ? 1 : 0)); + }); +}); diff --git a/packages/endpoint-media/test/integration/404-query-source-uid-not-found.js b/packages/endpoint-media/test/integration/404-query-source-uid-not-found.js new file mode 100644 index 000000000..36af576bf --- /dev/null +++ b/packages/endpoint-media/test/integration/404-query-source-uid-not-found.js @@ -0,0 +1,28 @@ +import { strict as assert } from "node:assert"; +import { after, describe, it } from "node:test"; + +import { testServer } from "@indiekit-test/server"; +import { testToken } from "@indiekit-test/token"; +import supertest from "supertest"; + +const server = await testServer(); +const request = supertest.agent(server); + +describe("endpoint-media GET /media?q=source&uid=*", () => { + it("Returns 404 error uid not found", async () => { + const result = await request + .get("/media") + .auth(testToken(), { type: "bearer" }) + .set("accept", "application/json") + .query({ q: "source" }) + .query({ uid: "unknown-uid" }); + + assert.equal(result.status, 404); + assert.equal( + result.body.error_description, + "No database record found for file", + ); + }); + + after(() => server.close()); +}); diff --git a/packages/endpoint-media/test/unit/media-data.js b/packages/endpoint-media/test/unit/media-data.js index 7d68544fa..0d25d05f2 100644 --- a/packages/endpoint-media/test/unit/media-data.js +++ b/packages/endpoint-media/test/unit/media-data.js @@ -86,4 +86,10 @@ describe("endpoint-media/lib/media-data", async () => { message: "No media data to delete", }); }); + + it("Stores a UUIDv7 uid", async () => { + const result = await mediaData.create(application, publication, file); + + assert.match(result.properties.uid, /^[\da-f]{8}-[\da-f]{4}-7/); + }); }); diff --git a/packages/endpoint-micropub/lib/controllers/query.js b/packages/endpoint-micropub/lib/controllers/query.js index af7d65ef0..c80b4144e 100644 --- a/packages/endpoint-micropub/lib/controllers/query.js +++ b/packages/endpoint-micropub/lib/controllers/query.js @@ -13,6 +13,7 @@ import { getMf2Properties, jf2ToMf2 } from "../mf2.js"; * @property {string} [offset] - Offset to start limit of items * @property {string|string[]} [properties] - mf2 properties to select * @property {string} [q] - Query + * @property {string} [uid] - UID of post to return * @property {string} [url] - URL of post to return */ @@ -28,7 +29,7 @@ export const queryController = async (request, response, next) => { const config = getConfig(application, publication); const limit = Number(request.query.limit) || 0; const offset = Number(request.query.offset) || 0; - let { after, before, filter, properties, q, url } = request.query; + let { after, before, filter, properties, q, uid, url } = request.query; if (!q) { throw IndiekitError.badRequest( @@ -50,20 +51,26 @@ export const queryController = async (request, response, next) => { } case "source": { - if (url) { - // Return mf2 for a given URL (optionally filtered by properties) + if (url || uid) { + // Return mf2 for a given post (optionally filtered by properties). + // `url` is what the Micropub specification defines; `uid` is an + // extension, and the only identifier the posts interface holds. let postData; if (postsCollection) { - postData = await postsCollection.findOne({ - "properties.url": url, - }); + postData = await postsCollection.findOne( + url ? { "properties.url": url } : { "properties.uid": uid }, + ); } if (!postData) { - throw IndiekitError.badRequest( - response.locals.__("BadRequestError.missingResource", "post"), - ); + throw url + ? IndiekitError.badRequest( + response.locals.__("BadRequestError.missingResource", "post"), + ) + : IndiekitError.notFound( + response.locals.__("NotFoundError.record", "post"), + ); } const mf2 = jf2ToMf2(postData); diff --git a/packages/endpoint-micropub/lib/mf2.js b/packages/endpoint-micropub/lib/mf2.js index b16f12fab..37931ed17 100644 --- a/packages/endpoint-micropub/lib/mf2.js +++ b/packages/endpoint-micropub/lib/mf2.js @@ -37,20 +37,17 @@ export const getMf2Properties = (mf2, requestedProperties) => { /** * Convert JF2 post data to mf2 * @param {Jf2PostData} postData - Post data - * @param {boolean} [shouldIncludeObjectId] - Include ObjectID from post data * @returns {MicroformatRoot} mf2 */ -export const jf2ToMf2 = (postData, shouldIncludeObjectId = true) => { - const { properties, _id } = postData; +export const jf2ToMf2 = (postData) => { + const { properties } = postData; /** * @type {Mf2Draft} */ const mf2 = { type: [`h-${properties.type}`], - properties: { - ...(shouldIncludeObjectId && _id && { uid: [_id] }), - }, + properties: {}, }; delete properties.type; @@ -59,7 +56,7 @@ export const jf2ToMf2 = (postData, shouldIncludeObjectId = true) => { for (const key in properties) { // Convert nested vocabulary to mf2 (i.e. h-card, h-geo, h-adr) if (Object.prototype.hasOwnProperty.call(properties[key], "type")) { - mf2.properties[key] = [jf2ToMf2({ properties: properties[key] }, false)]; + mf2.properties[key] = [jf2ToMf2({ properties: properties[key] })]; } // Convert values to arrays (i.e. "a" => ["a"]) diff --git a/packages/endpoint-micropub/lib/post-data.js b/packages/endpoint-micropub/lib/post-data.js index 87c3c097d..fddd1db08 100644 --- a/packages/endpoint-micropub/lib/post-data.js +++ b/packages/endpoint-micropub/lib/post-data.js @@ -1,3 +1,4 @@ +import { randomUUIDv7 } from "node:crypto"; import { isDeepStrictEqual } from "node:util"; import { IndiekitError } from "@indiekit/error"; @@ -38,6 +39,13 @@ export const postData = { async create(application, publication, properties, isDraftMode = false) { debug(`Create %O`, { isDraftMode, properties }); + // Ignore any `uid` supplied by the client: a client-supplied value + // would otherwise reach `postTypeCount.get()` via `renderPath()`, + // below, before the real uid is assigned — skewing today's count for + // this post type and letting a client influence another post's + // numbering. + delete properties.uid; + const { timeZone } = application; const { me, postTypes, syndicationTargets } = publication; @@ -78,12 +86,22 @@ export const postData = { ? "draft" : properties["post-status"] || "published"; - const data = { path, properties }; - // Add data to posts collection (or replace existing if present) const postsCollection = application?.collections?.get("posts"); + const query = { "properties.url": properties.url }; + + // Keep `uid` stable when a post already exists at this URL: it is + // meant to be a durable identifier for the post, so rotating it on + // every update would invalidate identifiers already handed out once + // something starts relying on it staying the same. + const existing = await postsCollection?.findOne(query, { + projection: { "properties.uid": 1 }, + }); + properties.uid = existing?.properties?.uid || randomUUIDv7(); + + const data = { path, properties }; + if (postsCollection) { - const query = { "properties.url": properties.url }; await postsCollection.replaceOne(query, data, { upsert: true }); } @@ -161,6 +179,11 @@ export const postData = { : updateMf2.deleteEntries(properties, operation.delete); } + // Keep `uid` stable regardless of what the client asked to add, replace + // or delete: it’s a durable identifier for this post, so an update + // operation must never be able to reassign or drop it. + properties.uid = oldProperties.uid; + // Normalise properties properties = normaliseProperties(publication, properties, timeZone); oldProperties = normaliseProperties(publication, oldProperties, timeZone); @@ -227,7 +250,9 @@ export const postData = { // Delete all properties, except those required for path creation for (const key in _deletedProperties) { - if (!["post-type", "published", "slug", "type", "url"].includes(key)) { + if ( + !["post-type", "published", "slug", "type", "uid", "url"].includes(key) + ) { delete properties[key]; } } diff --git a/packages/endpoint-micropub/lib/post-type-count.js b/packages/endpoint-micropub/lib/post-type-count.js index 723a4dd56..fbcb6e5ba 100644 --- a/packages/endpoint-micropub/lib/post-type-count.js +++ b/packages/endpoint-micropub/lib/post-type-count.js @@ -1,5 +1,3 @@ -import { getObjectId } from "@indiekit/util"; - export const postTypeCount = { /** * Count the number of posts of a given type @@ -22,6 +20,7 @@ export const postTypeCount = { const startDate = new Date(new Date(properties.published).toDateString()); const endDate = new Date(startDate); endDate.setDate(endDate.getDate() + 1); + const response = await postsCollection .aggregate([ { @@ -40,7 +39,7 @@ export const postTypeCount = { }, // Don’t count the post being updated ...(properties.uid && { - _id: { $ne: getObjectId(properties.uid) }, + "properties.uid": { $ne: properties.uid }, }), ...(properties.url && { "properties.url": { $ne: properties.url }, diff --git a/packages/endpoint-micropub/test/integration/200-query-source-uid.js b/packages/endpoint-micropub/test/integration/200-query-source-uid.js new file mode 100644 index 000000000..9cc6a06b4 --- /dev/null +++ b/packages/endpoint-micropub/test/integration/200-query-source-uid.js @@ -0,0 +1,62 @@ +import { strict as assert } from "node:assert"; +import { after, before, describe, it } from "node:test"; + +import { testDatabase } from "@indiekit-test/database"; +import { mockAgent } from "@indiekit-test/mock-agent"; +import { testServer } from "@indiekit-test/server"; +import { testToken } from "@indiekit-test/token"; +import supertest from "supertest"; + +await mockAgent("endpoint-micropub"); +const { client, mongoServer, mongoUri } = await testDatabase(); +const server = await testServer({ + application: { mongodbUrl: mongoUri }, +}); +const request = supertest.agent(server); + +const createPost = async (name) => + request + .post("/micropub") + .auth(testToken(), { type: "bearer" }) + .set("accept", "application/json") + .send("h=entry") + .send(`name=${name}`); + +const getUid = async (url) => { + const result = await request + .get("/micropub") + .auth("JWT", { type: "bearer" }) + .set("accept", "application/json") + .query({ q: "source" }) + .query({ "properties[]": "uid" }) + .query({ url }); + + return result.body.properties.uid[0]; +}; + +describe("endpoint-micropub GET /micropub?q=source&uid=*", () => { + let seededUid; + + before(async () => { + const response = await createPost("Foobar"); + seededUid = await getUid(response.headers.location); + }); + + it("Returns published post", async () => { + const result = await request + .get("/micropub") + .auth("JWT", { type: "bearer" }) + .set("accept", "application/json") + .query({ q: "source" }) + .query({ uid: seededUid }); + + assert.equal(result.status, 200); + assert.equal(result.body.properties.uid[0], seededUid); + }); + + after(async () => { + await client.close(); + await mongoServer.stop(); + server.close((error) => process.exit(error ? 1 : 0)); + }); +}); diff --git a/packages/endpoint-micropub/test/integration/404-query-source-uid-not-found.js b/packages/endpoint-micropub/test/integration/404-query-source-uid-not-found.js new file mode 100644 index 000000000..c160bf91f --- /dev/null +++ b/packages/endpoint-micropub/test/integration/404-query-source-uid-not-found.js @@ -0,0 +1,47 @@ +import { strict as assert } from "node:assert"; +import { after, before, describe, it } from "node:test"; + +import { testDatabase } from "@indiekit-test/database"; +import { mockAgent } from "@indiekit-test/mock-agent"; +import { testServer } from "@indiekit-test/server"; +import { testToken } from "@indiekit-test/token"; +import supertest from "supertest"; + +await mockAgent("endpoint-micropub"); +const { client, mongoServer, mongoUri } = await testDatabase(); +const server = await testServer({ + application: { mongodbUrl: mongoUri }, +}); +const request = supertest.agent(server); + +describe("endpoint-micropub GET /micropub?q=source&uid=*", () => { + before(async () => { + await request + .post("/micropub") + .auth(testToken(), { type: "bearer" }) + .set("accept", "application/json") + .send("h=entry") + .send("name=Foobar"); + }); + + it("Returns 404 error uid not found", async () => { + const result = await request + .get("/micropub") + .auth("JWT", { type: "bearer" }) + .set("accept", "application/json") + .query({ q: "source" }) + .query({ uid: "unknown-uid" }); + + assert.equal(result.status, 404); + assert.equal( + result.body.error_description, + "No database record found for post", + ); + }); + + after(async () => { + await client.close(); + await mongoServer.stop(); + server.close((error) => process.exit(error ? 1 : 0)); + }); +}); diff --git a/packages/endpoint-micropub/test/unit/mf2.js b/packages/endpoint-micropub/test/unit/mf2.js index 52ca87c41..78572ae45 100644 --- a/packages/endpoint-micropub/test/unit/mf2.js +++ b/packages/endpoint-micropub/test/unit/mf2.js @@ -8,13 +8,12 @@ import { jf2ToMf2 } from "../../lib/mf2.js"; describe("endpoint-micropub/lib/mf2", () => { it("Convert JF2 to mf2", () => { const properties = JSON.parse(getFixture("jf2/all-properties.jf2")); - const postData = { _id: 123, properties }; + const postData = { properties }; const result = jf2ToMf2(postData); assert.deepEqual(result, { type: ["h-entry"], properties: { - uid: [123], url: ["https://website.example/posts/cheese-sandwich"], name: ["What I had for lunch"], content: [ @@ -68,4 +67,26 @@ describe("endpoint-micropub/lib/mf2", () => { }, }); }); + + it("Passes a stored uid through, and invents none without one", () => { + const withUid = jf2ToMf2({ + properties: { + type: "entry", + uid: "01a0966c-ef50-79c5-a36d-33c045813e4b", + }, + }); + assert.deepEqual(withUid.properties.uid, [ + "01a0966c-ef50-79c5-a36d-33c045813e4b", + ]); + + const withoutUid = jf2ToMf2({ + _id: "6aa54e8a9bb0f7b129092770", + properties: { type: "entry" }, + }); + assert.equal( + withoutUid.properties.uid, + undefined, + "synthesised a uid from _id", + ); + }); }); diff --git a/packages/endpoint-micropub/test/unit/post-data.js b/packages/endpoint-micropub/test/unit/post-data.js index d962ceac3..94ff80536 100644 --- a/packages/endpoint-micropub/test/unit/post-data.js +++ b/packages/endpoint-micropub/test/unit/post-data.js @@ -202,4 +202,141 @@ describe("endpoint-micropub/lib/post-data", async () => { message: "note", }); }); + + it("Stores a UUIDv7 uid, and keeps it when replacing the same URL", async () => { + const created = await postData.create(application, publication, { + ...structuredClone(properties), + }); + + assert.match(created.properties.uid, /^[\da-f]{8}-[\da-f]{4}-7/); + + const again = await postData.create(application, publication, { + ...structuredClone(properties), + content: "Replaced", + }); + + assert.equal( + again.properties.uid, + created.properties.uid, + "uid rotated when a post at the same URL was replaced", + ); + }); + + it("Keeps uid when deleting a post", async () => { + const created = await postData.create(application, publication, { + ...structuredClone(properties), + }); + + const deleted = await postData.delete( + application, + publication, + created.properties.url, + ); + + assert.equal(deleted.properties.uid, created.properties.uid); + }); + + it("Keeps uid when a client tries to replace it via update", async () => { + const created = await postData.create(application, publication, { + ...structuredClone(properties), + "mp-slug": "keep-uid-replace", + }); + + const operation = { replace: { uid: ["pwned-by-client"] } }; + await postData.update( + application, + publication, + created.properties.url, + operation, + ); + + // Once `uid` is neutralised, replacing only `uid` is a no-op, so + // `update` legitimately returns `undefined` here (see "Doesn’t update + // post if no changes" above) — check the stored record instead. + const stored = await postData.read(application, created.properties.url); + assert.equal(stored.properties.uid, created.properties.uid); + }); + + it("Keeps uid when a client tries to add to it via update", async () => { + const created = await postData.create(application, publication, { + ...structuredClone(properties), + "mp-slug": "keep-uid-add", + }); + + const operation = { add: { uid: ["pwned-by-client"] } }; + await postData.update( + application, + publication, + created.properties.url, + operation, + ); + + const stored = await postData.read(application, created.properties.url); + assert.equal(stored.properties.uid, created.properties.uid); + }); + + it("Keeps uid when a client tries to delete it via update, combined with another operation", async () => { + const created = await postData.create(application, publication, { + ...structuredClone(properties), + "mp-slug": "keep-uid-delete", + }); + + // `delete` as the only operation is rejected before `postData.update` is + // ever called (see `action.js`), so the reachable attack pairs it with + // an operation that passes that check. + const operation = { + replace: { content: ["changed"] }, + delete: ["uid"], + }; + const result = await postData.update( + application, + publication, + created.properties.url, + operation, + ); + + assert.equal(result.properties.uid, created.properties.uid); + }); + + it("Ignores a uid supplied by the client", async () => { + const created = await postData.create(application, publication, { + ...structuredClone(properties), + uid: "supplied-by-the-client", + }); + + assert.notEqual(created.properties.uid, "supplied-by-the-client"); + }); + + it("Doesn’t let a client-supplied uid skew the post-type count used in the path", async () => { + const postsCollection = database.collection("posts"); + const skewPublication = structuredClone(publication); + skewPublication.postTypes.note.post.path = + "src/content/notes/{n}-{slug}.md"; + + // An existing post, published the same day as the one we're about to + // create, that a client could quote back as its own `uid`. + const existingUid = "0191f6e0-aaaa-7abc-8def-0123456789ab"; + await postsCollection.insertOne({ + properties: { + type: "entry", + "post-type": "note", + published: "2024-05-01T10:00:00.000Z", + url: "https://website.example/notes/other/", + uid: existingUid, + }, + }); + + const result = await postData.create(application, skewPublication, { + type: "entry", + published: "2024-05-01T12:00:00.000Z", + content: "Foo", + "mp-slug": "skewed", + uid: existingUid, + }); + + // If the client-supplied `uid` reached `postTypeCount.get()`, it + // would `$ne`-exclude the existing post above from today's count, + // making this the 1st note of the day instead of the 2nd. + assert.equal(result.path, "src/content/notes/2-skewed.md"); + }); }); diff --git a/packages/endpoint-micropub/test/unit/post-type-count.js b/packages/endpoint-micropub/test/unit/post-type-count.js index 231400a6a..d301cb14c 100644 --- a/packages/endpoint-micropub/test/unit/post-type-count.js +++ b/packages/endpoint-micropub/test/unit/post-type-count.js @@ -19,6 +19,7 @@ describe("endpoint-micropub/lib/post-type-count", () => { published, name: "Foo", url: "https://website.example/foo", + uid: "0191f6e0-1234-7abc-8def-0123456789ab", }, }, { @@ -28,6 +29,7 @@ describe("endpoint-micropub/lib/post-type-count", () => { published, name: "Bar", url: "https://website.example/bar", + uid: "0191f6e0-5678-7abc-8def-0123456789ab", }, }, ]); @@ -48,10 +50,10 @@ describe("endpoint-micropub/lib/post-type-count", () => { assert.equal(result, 2); }); - it("Doesn’t count the post being updated, by its ID", async () => { + it("Doesn’t count the post being updated, by its uid", async () => { const post = await posts.findOne({}); const result = await postTypeCount.get(posts, { - uid: post._id.toString(), + uid: post.properties.uid, type: "entry", published, "post-type": "note", diff --git a/packages/endpoint-posts/lib/utils.js b/packages/endpoint-posts/lib/utils.js index 811afd4b3..86ba2867d 100644 --- a/packages/endpoint-posts/lib/utils.js +++ b/packages/endpoint-posts/lib/utils.js @@ -1,5 +1,6 @@ import { Buffer } from "node:buffer"; +import { IndiekitError } from "@indiekit/error"; import { sanitise, ISO_6709_RE } from "@indiekit/util"; import { mf2tojf2 } from "@paulrobertlloyd/mf2tojf2"; import formatcoords from "formatcoords"; @@ -155,21 +156,28 @@ export const getPostName = (publication, properties) => { * @param {string} uid - Item UID * @param {string} micropubEndpoint - Micropub endpoint * @param {string} accessToken - Access token - * @returns {Promise} JF2 properties + * @returns {Promise} JF2 properties, or false if not found */ export const getPostProperties = async (uid, micropubEndpoint, accessToken) => { const micropubUrl = new URL(micropubEndpoint); micropubUrl.searchParams.append("q", "source"); + micropubUrl.searchParams.append("uid", uid); + + try { + // `q=source&uid=` returns mf2 for a single post (the same shape as + // `q=source&url=`), so wrap it as `items` before flattening to JF2. + const mf2 = await endpoint.get(micropubUrl.href, accessToken); + return mf2tojf2({ items: [mf2] }); + } catch (error) { + // `endpoint.get` throws on any error response. A post that is simply gone + // is the caller's own not-found page, not an error to show the reader; + // anything else is a real failure and must keep travelling. + if (error instanceof IndiekitError && error.status === 404) { + return false; + } - const micropubResponse = await endpoint.get(micropubUrl.href, accessToken); - - if (micropubResponse?.items?.length > 0) { - const jf2 = mf2tojf2(micropubResponse); - const items = jf2.children || [jf2]; - return items.find((item) => item.uid === uid); + throw error; } - - return false; }; /** diff --git a/packages/endpoint-posts/test/integration/200-posts-after-delete.js b/packages/endpoint-posts/test/integration/200-posts-after-delete.js new file mode 100644 index 000000000..dea5dc5c3 --- /dev/null +++ b/packages/endpoint-posts/test/integration/200-posts-after-delete.js @@ -0,0 +1,53 @@ +import { strict as assert } from "node:assert"; +import { after, before, describe, it } from "node:test"; + +import { testDatabase } from "@indiekit-test/database"; +import { mockAgent } from "@indiekit-test/mock-agent"; +import { testServer } from "@indiekit-test/server"; +import { testToken } from "@indiekit-test/token"; +import supertest from "supertest"; + +// `mockAgent("endpoint-micropub")` only fakes the content store (so `create` +// and `delete` don't need a real Git/file host) and otherwise leaves +// loopback net connect enabled. `/posts` fetches from the real, self-hosted +// Micropub endpoint over that loopback HTTP call, and that's the hop that +// has to reproduce the bug — mocking the Micropub response would just +// describe what we already believe, not prove it. +await mockAgent("endpoint-micropub"); +const { client, mongoServer, mongoUri } = await testDatabase(); +const server = await testServer({ + application: { mongodbUrl: mongoUri }, +}); +const request = supertest.agent(server); + +describe("endpoint-posts GET /posts (after deleting a post)", () => { + before(async () => { + const created = await request + .post("/micropub") + .auth(testToken(), { type: "bearer" }) + .send({ + type: ["h-entry"], + properties: { + name: ["Foobar"], + category: ["test1", "test2"], + }, + }); + + await request.post("/micropub").auth(testToken(), { type: "bearer" }).send({ + action: "delete", + url: created.header.location, + }); + }); + + it("Still lists posts once one of them has been deleted", async () => { + const result = await request.get("/posts"); + + assert.equal(result.status, 200); + }); + + after(async () => { + await client.close(); + await mongoServer.stop(); + server.close((error) => process.exit(error ? 1 : 0)); + }); +}); diff --git a/packages/endpoint-posts/test/unit/utils.js b/packages/endpoint-posts/test/unit/utils.js index 3fa3eea9b..a6eb84d4f 100644 --- a/packages/endpoint-posts/test/unit/utils.js +++ b/packages/endpoint-posts/test/unit/utils.js @@ -1,7 +1,9 @@ import { strict as assert } from "node:assert"; import { describe, it } from "node:test"; +import { testToken } from "@indiekit-test/token"; import { mockResponse } from "mock-req-res"; +import { MockAgent, setGlobalDispatcher } from "undici"; import { getChannelItems, @@ -10,6 +12,7 @@ import { getLocationProperty, getPhotoUrl, getPostName, + getPostProperties, getPostStatusBadges, getPostUrl, getSyndicateToItems, @@ -259,6 +262,57 @@ describe("endpoint-posts/lib/utils", () => { ]); }); + it("Fetches a post that isn’t on the Micropub endpoint’s first page of results", async () => { + const micropubEndpoint = "https://micropub-endpoint-uid-lookup.example"; + const targetUid = "target-uid"; + const targetUrl = "https://website.example/notes/target/"; + + const agent = new MockAgent(); + agent.disableNetConnect(); + setGlobalDispatcher(agent); + + // A default-sized listing page (the shape `q=source` returns with no + // `url`/`uid` and no explicit `limit`) that does not include the + // target post — it's older than the newest page of results. + const items = Array.from({ length: 40 }, (_, index) => ({ + type: ["h-entry"], + properties: { + uid: [`other-${index}`], + name: [`Post ${index}`], + "post-type": ["note"], + published: ["2024-12-21"], + url: [`https://website.example/notes/other-${index}/`], + }, + })); + + agent + .get(micropubEndpoint) + .intercept({ path: "/", query: { q: "source" } }) + .reply(200, { items }); + + agent + .get(micropubEndpoint) + .intercept({ path: "/", query: { q: "source", uid: targetUid } }) + .reply(200, { + type: ["h-entry"], + properties: { + uid: [targetUid], + name: ["Target post"], + "post-type": ["note"], + published: ["2024-12-21"], + url: [targetUrl], + }, + }); + + const result = await getPostProperties( + targetUid, + micropubEndpoint, + testToken(), + ); + + assert.equal(result.uid, targetUid); + }); + it("Gets post URL", () => { assert.equal( getPostUrl("aHR0cHM6Ly93ZWJzaXRlLmV4YW1wbGUvZm9vYmFy"), diff --git a/packages/indiekit/index.js b/packages/indiekit/index.js index fbdb1f522..fdda6e8c6 100644 --- a/packages/indiekit/index.js +++ b/packages/indiekit/index.js @@ -11,6 +11,7 @@ import { locales } from "./config/locales.js"; import { getCategories } from "./lib/categories.js"; import { getIndiekitConfig } from "./lib/config.js"; import { getLocaleCatalog } from "./lib/locale-catalog.js"; +import { backfillUids } from "./lib/migrate-uid.js"; import { getInstalledPlugins } from "./lib/plugins.js"; import { getPostTemplate } from "./lib/post-template.js"; import { getPostTypes } from "./lib/post-types.js"; @@ -187,6 +188,29 @@ export const Indiekit = class { async server(options = {}) { await this.connectMongodbClient(); await this.installPlugins(); + + // Posts and media created before `properties.uid` existed have no + // identifier to look them up by. Backfill before serving: a half-migrated + // collection would answer some lookups and 404 others. + // @todo Remove with `migrate-uid.js` once every existing database has + // been backfilled: at v1.0.0 or the move off MongoDB (#821). + // + // The list is deliberately literal, not `this.collections`: the backfill's + // `$set: { "properties.uid": ... }` CREATES a `properties` subdocument, so + // running it against any other collection a plugin registers — a cache, a + // token store, a queue — would silently graft an unrelated field onto data + // this migration has no business touching. + for (const name of ["posts", "media"]) { + const collection = this.collections.get(name); + if (collection) { + const updated = await backfillUids(collection); + debug(`Checked ‘${name}’ for missing uids: ${updated} added`); + if (updated > 0) { + console.info(`Added a uid to ${updated} items in ‘${name}’`); + } + } + } + await this.updatePublicationConfig(); const app = expressConfig(this); diff --git a/packages/indiekit/lib/migrate-uid.js b/packages/indiekit/lib/migrate-uid.js new file mode 100644 index 000000000..29f03ec13 --- /dev/null +++ b/packages/indiekit/lib/migrate-uid.js @@ -0,0 +1,90 @@ +import { randomBytes } from "node:crypto"; + +/** + * A UUIDv7 for a known point in time + * + * `crypto.randomUUIDv7()` always stamps the current time, so it cannot give an + * existing post an identifier that sorts by when the post was created. RFC 9562 + * lays the value out as a 48-bit big-endian millisecond timestamp, four version + * bits, twelve free bits, two variant bits, then random. `seq` goes in the free + * bits so that documents sharing a timestamp keep the order they arrive in. + * @param {number} msecs - Milliseconds since the epoch + * @param {number} seq - Tiebreaker within one millisecond, 0-4095 + * @returns {string} UUIDv7 + */ +export const uuidv7At = (msecs, seq) => { + const bytes = randomBytes(16); + + bytes.writeUIntBE(msecs, 0, 6); + bytes.writeUInt16BE(0x70_00 | (seq & 0x0f_ff), 6); + bytes[8] = (bytes[8] & 0x3f) | 0x80; + + return bytes + .toString("hex") + .replace(/(.{8})(.{4})(.{4})(.{4})(.{12})/, "$1-$2-$3-$4-$5"); +}; + +/** + * Give every document in a collection a `properties.uid` + * + * Existing posts predate the identifier, and their creation time only survives + * in the ObjectId. That timestamp resolves to the second, while the sequence + * holds 4096 values per millisecond, so documents sharing a second are spread + * across the milliseconds within it: enough for 4,096,000 of them before the + * second is exhausted, and the order always matches `_id`. + * + * The cursor walks every document, not only those still missing a uid: the + * per-second sequence has to advance past documents a previous, interrupted + * run already assigned, or a resumed run would reuse their timestamp+sequence + * range and the order guarantee above would break. Only the write is + * conditional (`properties.uid` absent), so re-running is a no-op past the + * point an earlier run reached, and two processes starting at once can't + * clobber each other's uid. + * @todo Remove once every existing database has been backfilled: at v1.0.0 + * or the move off MongoDB (#821), whichever comes first. A fresh install + * never needs it. + * @param {object} collection - MongoDB collection + * @returns {Promise} Number of documents updated + */ +export const backfillUids = async (collection) => { + const cursor = collection.find( + {}, + { projection: { _id: 1 }, sort: { _id: 1 } }, + ); + + let second; + let index = 0; + let updated = 0; + + for await (const { _id } of cursor) { + // Indiekit never sets `_id` itself, so only foreign or imported data can + // land here without an ObjectId. Skip it rather than crash the whole + // startup sequence over one document that isn't ours to migrate anyway. + if (typeof _id?.getTimestamp !== "function") { + console.warn( + `Skipped adding a uid to ${_id} in ‘${collection.collectionName}’: _id is not an ObjectId`, + ); + continue; + } + + const msecs = _id.getTimestamp().getTime(); + index = msecs === second ? index + 1 : 0; + second = msecs; + + const result = await collection.updateOne( + { _id, "properties.uid": { $exists: false } }, + { + $set: { + "properties.uid": uuidv7At( + msecs + Math.floor(index / 4096), + index % 4096, + ), + }, + }, + ); + + updated += result.modifiedCount; + } + + return updated; +}; diff --git a/packages/indiekit/test/unit/migrate-uid.js b/packages/indiekit/test/unit/migrate-uid.js new file mode 100644 index 000000000..20b5f8726 --- /dev/null +++ b/packages/indiekit/test/unit/migrate-uid.js @@ -0,0 +1,189 @@ +import { strict as assert } from "node:assert"; +import { after, before, describe, it, mock } from "node:test"; + +import { testDatabase } from "@indiekit-test/database"; + +import { backfillUids, uuidv7At } from "../../lib/migrate-uid.js"; + +// UUIDs sort as strings; a plain `.sort()` would coerce and compare lexically +// by default anyway, but the compare function keeps `unicorn/require-array-sort-compare` happy. +const compare = (a, b) => (a < b ? -1 : a > b ? 1 : 0); + +describe("indiekit/lib/migrate-uid", () => { + it("Emits a well-formed UUIDv7 for a given time", () => { + const msecs = Date.parse("2019-10-01T12:00:00Z"); + const uuid = uuidv7At(msecs, 0); + + assert.match(uuid, /^[\da-f]{8}(-[\da-f]{4}){3}-[\da-f]{12}$/); + assert.equal(uuid[14], "7", "version nibble is not 7"); + assert.ok("89ab".includes(uuid[19]), "variant bits are not RFC 9562"); + assert.equal( + Number.parseInt(uuid.replaceAll("-", "").slice(0, 12), 16), + msecs, + "timestamp does not round-trip", + ); + }); + + it("Orders by sequence within a millisecond, and by millisecond above that", () => { + const msecs = Date.parse("2019-10-01T12:00:00Z"); + const sequential = Array.from({ length: 4096 }, (_, index) => + uuidv7At(msecs, index), + ); + + assert.deepEqual( + sequential, + sequential.toSorted(compare), + "sequence does not order", + ); + assert.ok( + uuidv7At(msecs, 4095) < uuidv7At(msecs + 1, 0), + "a later millisecond does not outrank a higher sequence", + ); + }); + + it("Stays unique when time and sequence repeat", () => { + const msecs = Date.parse("2019-10-01T12:00:00Z"); + const uuids = new Set( + Array.from({ length: 10_000 }, () => uuidv7At(msecs, 7)), + ); + + assert.equal(uuids.size, 10_000, "random bits are not random"); + }); + + describe("backfillUids", () => { + let client; + let database; + let mongoServer; + + before(async () => { + ({ client, database, mongoServer } = await testDatabase()); + }); + + after(async () => { + await client.close(); + await mongoServer.stop(); + }); + + it("Gives every document a uid, in _id order, and leaves existing ones alone", async () => { + const collection = database.collection("backfill-posts"); + // 5000 documents sharing one second: more than the 4096-value sequence + // holds, which is the case a bulk import produces. + const documents = Array.from({ length: 5000 }, (_, index) => ({ + properties: { url: `https://website.example/post-${index}` }, + })); + documents[0].properties.uid = "already-set"; + await collection.insertMany(documents); + + const updated = await backfillUids(collection); + assert.equal( + updated, + 4999, + "did not skip the document that already had a uid", + ); + + const stored = await collection.find({}, { sort: { _id: 1 } }).toArray(); + assert.equal( + stored[0].properties.uid, + "already-set", + "overwrote an existing uid", + ); + + const backfilled = stored.slice(1).map((item) => item.properties.uid); + assert.ok(backfilled.every(Boolean), "a document was left without a uid"); + assert.deepEqual( + backfilled, + backfilled.toSorted(compare), + "uid order does not match _id order", + ); + }); + + it("Does nothing on a collection that needs nothing", async () => { + const collection = database.collection("backfill-empty"); + + assert.equal(await backfillUids(collection), 0); + }); + + it("Resumes correctly after a partial run, without restarting the per-second sequence", async () => { + const collection = database.collection("backfill-resume"); + // 5000 documents sharing one second, so the sequence overflows into a + // second millisecond partway through, same as the "more than 4096" case. + const documents = Array.from({ length: 5000 }, (_, index) => ({ + properties: { url: `https://website.example/resume-${index}` }, + })); + await collection.insertMany(documents); + + // Simulate a completed first boot. + await backfillUids(collection); + + // Simulate a crash partway through a second boot: strip the uid back + // off the later half, as if those documents were never reached. + const stored = await collection.find({}, { sort: { _id: 1 } }).toArray(); + const laterHalf = stored.slice(2500).map((document) => document._id); + await collection.updateMany( + { _id: { $in: laterHalf } }, + { $unset: { "properties.uid": "" } }, + ); + + // Resume: this must continue the sequence, not restart it at 0 — a + // restart would reuse the timestamp+sequence range already given to + // the kept-uid documents, and the final order would stop matching _id. + const updated = await backfillUids(collection); + assert.equal(updated, 2500); + + const final = await collection.find({}, { sort: { _id: 1 } }).toArray(); + const uids = final.map((document) => document.properties.uid); + assert.deepEqual( + uids, + uids.toSorted(compare), + "uid order does not match _id order after resuming", + ); + }); + + it("Does not overwrite a uid another process already assigned", async () => { + const collection = database.collection("backfill-concurrent"); + await collection.insertOne({ + properties: { url: "https://website.example/concurrent" }, + }); + + // A second process' run reaching this document first. + const [existing] = await collection.find({}).toArray(); + await collection.updateOne( + { _id: existing._id, "properties.uid": { $exists: false } }, + { $set: { "properties.uid": "already-set-by-another-process" } }, + ); + + const updated = await backfillUids(collection); + assert.equal(updated, 0, "overwrote a uid set by a concurrent run"); + + const stored = await collection.findOne({ _id: existing._id }); + assert.equal( + stored.properties.uid, + "already-set-by-another-process", + "overwrote a uid set by a concurrent run", + ); + }); + + it("Skips a document whose _id is not an ObjectId, instead of crashing", async () => { + const collection = database.collection("backfill-foreign-id"); + mock.method(console, "warn", () => {}); + + await collection.insertMany([ + { + _id: "foreign-string-id", + properties: { url: "https://website.example/foreign" }, + }, + { properties: { url: "https://website.example/ordinary" } }, + ]); + + const updated = await backfillUids(collection); + assert.equal(updated, 1, "did not backfill the ordinary document too"); + + const foreign = await collection.findOne({ _id: "foreign-string-id" }); + assert.equal( + foreign.properties.uid, + undefined, + "assigned a uid to a document with a non-ObjectId _id", + ); + }); + }); +});