From d8ca8525b921a2d66b222a61a897c4dcf13e2acc Mon Sep 17 00:00:00 2001 From: Eric Bailey Date: Fri, 1 May 2026 19:36:17 +0100 Subject: [PATCH] checkpoint uri resolution --- .../ComposerV2/store/__tests__/embeds.test.ts | 307 ++++++++++++++---- .../ComposerV2/store/__tests__/media.test.ts | 106 +++--- .../ComposerV2/store/__tests__/notify.test.ts | 11 +- .../ComposerV2/store/__tests__/posts.test.ts | 11 +- src/components/ComposerV2/store/index.ts | 142 ++++++-- .../ComposerV2/store/linkResolution.ts | 78 +++++ src/components/ComposerV2/store/types.ts | 96 +++++- .../ComposerV2/store/utils/buildThreadPost.ts | 2 +- .../computePostMediaSelectionsRemaining.ts | 14 +- 9 files changed, 637 insertions(+), 130 deletions(-) create mode 100644 src/components/ComposerV2/store/linkResolution.ts diff --git a/src/components/ComposerV2/store/__tests__/embeds.test.ts b/src/components/ComposerV2/store/__tests__/embeds.test.ts index f99ff44472..483400290d 100644 --- a/src/components/ComposerV2/store/__tests__/embeds.test.ts +++ b/src/components/ComposerV2/store/__tests__/embeds.test.ts @@ -1,6 +1,21 @@ -import {type AtpAgent} from '@atproto/api' -import {describe, expect, test} from '@jest/globals' +import { + type AppBskyFeedDefs, + type AppBskyGraphDefs, + type AtpAgent, +} from '@atproto/api' +import {beforeEach, describe, expect, jest, test} from '@jest/globals' +// Avoid pulling the UI module chain (gallery → media picker → ALF) into the +// test environment. Tests inject `__resolveLink` directly, so the real +// implementation is never invoked. +jest.mock('#/lib/api/resolve', () => ({ + resolveLink: jest.fn(), +})) +jest.mock('#/state/session/agent', () => ({ + createPublicAgent: jest.fn(() => ({})), +})) + +import {type ResolvedLink, type resolveLink} from '#/lib/api/resolve' import {createThreadStore} from '#/components/ComposerV2/store' function makeIdGenerator() { @@ -14,51 +29,256 @@ function rootId(store: ReturnType) { return Object.keys(store.getState().posts)[0] } -describe('setExternalEmbed / removeExternalEmbed', () => { - test('sets the external embed and marks state dirty', () => { - const store = createThreadStore({agent, __createId: makeIdGenerator()}) +function deferred() { + let resolve!: (v: T) => void + let reject!: (err: unknown) => void + const promise = new Promise((res, rej) => { + resolve = res + reject = rej + }) + return {promise, resolve, reject} +} + +async function flushPromises() { + await new Promise(resolve => setImmediate(resolve)) +} + +let mockResolveLink: jest.Mock + +beforeEach(() => { + mockResolveLink = jest.fn() as unknown as jest.Mock +}) + +function makeStore() { + return createThreadStore({ + agent, + __createId: makeIdGenerator(), + __resolveLink: mockResolveLink as unknown as typeof resolveLink, + }) +} + +const fakePostView = (uri: string, cid: string) => + ({uri, cid}) as unknown as AppBskyFeedDefs.PostView + +const fakeGeneratorView = (uri: string, cid: string) => + ({uri, cid}) as unknown as AppBskyFeedDefs.GeneratorView + +const fakeListView = (uri: string, cid: string) => + ({uri, cid}) as unknown as AppBskyGraphDefs.ListView + +const fakeStarterPackView = (uri: string, cid: string) => + ({uri, cid}) as unknown as AppBskyGraphDefs.StarterPackView + +const postLink: ResolvedLink = { + type: 'record', + kind: 'post', + record: {uri: 'at://post', cid: 'cp'}, + view: fakePostView('at://post', 'cp'), +} + +const feedLink: ResolvedLink = { + type: 'record', + kind: 'feed', + record: {uri: 'at://feed', cid: 'cf'}, + view: fakeGeneratorView('at://feed', 'cf'), +} + +const listLink: ResolvedLink = { + type: 'record', + kind: 'list', + record: {uri: 'at://list', cid: 'cl'}, + view: fakeListView('at://list', 'cl'), +} + +const starterPackLink: ResolvedLink = { + type: 'record', + kind: 'starter-pack', + record: {uri: 'at://sp', cid: 'csp'}, + view: fakeStarterPackView('at://sp', 'csp'), +} + +const externalLink: ResolvedLink = { + type: 'external', + uri: 'https://example.com', + title: 'Example', + description: 'A description', + thumb: undefined, +} + +describe('addUri routes outcomes', () => { + test('post → quote (with view), embed cleared', async () => { + const d = deferred() + mockResolveLink.mockReturnValueOnce(d.promise) + const store = makeStore() const root = rootId(store) - store.actions.setExternalEmbed(root, {uri: 'https://example.com'}) - expect(store.getState().posts[root].external).toEqual({ - uri: 'https://example.com', + store.actions.addUri(root, 'https://bsky.app/post') + expect(store.getState().posts[root].embed?.state).toBe('pending') + + d.resolve(postLink) + await flushPromises() + + const post = store.getState().posts[root] + expect(post.embed).toBeUndefined() + expect(post.quote).toEqual({ + uri: 'at://post', + cid: 'cp', + view: postLink.kind === 'post' ? postLink.view : undefined, }) - expect(store.getState().isDirty).toBe(true) }) - test('replaces an existing external embed', () => { - const store = createThreadStore({agent, __createId: makeIdGenerator()}) + test('feed → embed.feed', async () => { + const d = deferred() + mockResolveLink.mockReturnValueOnce(d.promise) + const store = makeStore() const root = rootId(store) - store.actions.setExternalEmbed(root, {uri: 'https://a.example'}) - store.actions.setExternalEmbed(root, {uri: 'https://b.example'}) - expect(store.getState().posts[root].external?.uri).toBe('https://b.example') + store.actions.addUri(root, 'https://bsky.app/feed') + d.resolve(feedLink) + await flushPromises() + expect(store.getState().posts[root].embed?.state).toBe('feed') }) - test('removeExternalEmbed clears the external embed', () => { - const store = createThreadStore({agent, __createId: makeIdGenerator()}) + test('list → embed.list', async () => { + const d = deferred() + mockResolveLink.mockReturnValueOnce(d.promise) + const store = makeStore() const root = rootId(store) - store.actions.setExternalEmbed(root, {uri: 'https://example.com'}) - store.actions.removeExternalEmbed(root) - expect(store.getState().posts[root].external).toBeUndefined() + store.actions.addUri(root, 'https://bsky.app/list') + d.resolve(listLink) + await flushPromises() + expect(store.getState().posts[root].embed?.state).toBe('list') }) - test('removeExternalEmbed is a no-op when nothing is set', () => { - const store = createThreadStore({agent, __createId: makeIdGenerator()}) - const before = store.getState() - store.actions.removeExternalEmbed(rootId(store)) - expect(store.getState()).toBe(before) + test('starter-pack → embed["starter-pack"]', async () => { + const d = deferred() + mockResolveLink.mockReturnValueOnce(d.promise) + const store = makeStore() + const root = rootId(store) + store.actions.addUri(root, 'https://bsky.app/sp') + d.resolve(starterPackLink) + await flushPromises() + expect(store.getState().posts[root].embed?.state).toBe('starter-pack') }) - test('setExternalEmbed is a no-op when post id is unknown', () => { - const store = createThreadStore({agent, __createId: makeIdGenerator()}) - const before = store.getState() - store.actions.setExternalEmbed('does-not-exist', {uri: 'https://x'}) - expect(store.getState()).toBe(before) + test('external → embed.external', async () => { + const d = deferred() + mockResolveLink.mockReturnValueOnce(d.promise) + const store = makeStore() + const root = rootId(store) + store.actions.addUri(root, 'https://example.com') + d.resolve(externalLink) + await flushPromises() + const embed = store.getState().posts[root].embed + if (embed?.state !== 'external') throw new Error('expected external') + expect(embed.title).toBe('Example') + }) + + test('post outcome dropped when quote is already set', async () => { + const d = deferred() + mockResolveLink.mockReturnValueOnce(d.promise) + const store = makeStore() + const root = rootId(store) + store.actions.setQuoteEmbed(root, {uri: 'at://existing', cid: 'cx'}) + store.actions.addUri(root, 'https://bsky.app/post') + d.resolve(postLink) + await flushPromises() + expect(store.getState().posts[root].quote).toEqual({ + uri: 'at://existing', + cid: 'cx', + }) + expect(store.getState().posts[root].embed).toBeUndefined() + }) + + test('non-post outcome with media present is silently dropped', async () => { + const d = deferred() + mockResolveLink.mockReturnValueOnce(d.promise) + const store = makeStore() + const root = rootId(store) + store.actions.addMedia(root, [ + {kind: 'image', uri: 'file:///a.jpg', width: 10, height: 10}, + ]) + store.actions.addUri(root, 'https://example.com') + d.resolve(externalLink) + await flushPromises() + expect(store.getState().posts[root].embed).toBeUndefined() + expect(store.getState().posts[root].media).toHaveLength(1) + }) + + test('post outcome with media present routes to quote', async () => { + const d = deferred() + mockResolveLink.mockReturnValueOnce(d.promise) + const store = makeStore() + const root = rootId(store) + store.actions.addMedia(root, [ + {kind: 'image', uri: 'file:///a.jpg', width: 10, height: 10}, + ]) + store.actions.addUri(root, 'https://bsky.app/post') + d.resolve(postLink) + await flushPromises() + expect(store.getState().posts[root].quote?.uri).toBe('at://post') + expect(store.getState().posts[root].media).toHaveLength(1) + }) +}) + +describe('addUri failure and retry', () => { + test('rejection produces a failed embed with a bound retry()', async () => { + const d1 = deferred() + const d2 = deferred() + mockResolveLink + .mockReturnValueOnce(d1.promise) + .mockReturnValueOnce(d2.promise) + const store = makeStore() + const root = rootId(store) + store.actions.addUri(root, 'https://example.com') + d1.reject(new Error('network down')) + await flushPromises() + + const failed = store.getState().posts[root].embed + if (failed?.state !== 'failed') throw new Error('expected failed') + expect(failed.error).toContain('network down') + + failed.retry() + expect(store.getState().posts[root].embed?.state).toBe('pending') + + d2.resolve(externalLink) + await flushPromises() + expect(store.getState().posts[root].embed?.state).toBe('external') + }) +}) + +describe('addUri cancellation', () => { + test('a second addUri invalidates the first response', async () => { + const d1 = deferred() + const d2 = deferred() + mockResolveLink + .mockReturnValueOnce(d1.promise) + .mockReturnValueOnce(d2.promise) + const store = makeStore() + const root = rootId(store) + store.actions.addUri(root, 'https://a.example') + store.actions.addUri(root, 'https://b.example') + d1.resolve(externalLink) + await flushPromises() + const pending = store.getState().posts[root].embed + if (pending?.state !== 'pending') throw new Error('expected pending') + expect(pending.uri).toBe('https://b.example') + }) + + test('removeEmbed before the response lands keeps embed undefined', async () => { + const d = deferred() + mockResolveLink.mockReturnValueOnce(d.promise) + const store = makeStore() + const root = rootId(store) + store.actions.addUri(root, 'https://example.com') + store.actions.removeEmbed(root) + d.resolve(externalLink) + await flushPromises() + expect(store.getState().posts[root].embed).toBeUndefined() }) }) describe('setQuoteEmbed / removeQuoteEmbed', () => { test('sets the quote embed and marks state dirty', () => { - const store = createThreadStore({agent, __createId: makeIdGenerator()}) + const store = makeStore() const root = rootId(store) store.actions.setQuoteEmbed(root, {uri: 'at://x', cid: 'c'}) expect(store.getState().posts[root].quote).toEqual({ @@ -68,36 +288,11 @@ describe('setQuoteEmbed / removeQuoteEmbed', () => { expect(store.getState().isDirty).toBe(true) }) - test('replaces an existing quote embed', () => { - const store = createThreadStore({agent, __createId: makeIdGenerator()}) - const root = rootId(store) - store.actions.setQuoteEmbed(root, {uri: 'at://a', cid: 'ca'}) - store.actions.setQuoteEmbed(root, {uri: 'at://b', cid: 'cb'}) - expect(store.getState().posts[root].quote).toEqual({ - uri: 'at://b', - cid: 'cb', - }) - }) - test('removeQuoteEmbed clears the quote', () => { - const store = createThreadStore({agent, __createId: makeIdGenerator()}) + const store = makeStore() const root = rootId(store) store.actions.setQuoteEmbed(root, {uri: 'at://x', cid: 'c'}) store.actions.removeQuoteEmbed(root) expect(store.getState().posts[root].quote).toBeUndefined() }) - - test('removeQuoteEmbed is a no-op when nothing is set', () => { - const store = createThreadStore({agent, __createId: makeIdGenerator()}) - const before = store.getState() - store.actions.removeQuoteEmbed(rootId(store)) - expect(store.getState()).toBe(before) - }) - - test('setQuoteEmbed is a no-op when post id is unknown', () => { - const store = createThreadStore({agent, __createId: makeIdGenerator()}) - const before = store.getState() - store.actions.setQuoteEmbed('does-not-exist', {uri: 'at://x', cid: 'c'}) - expect(store.getState()).toBe(before) - }) }) diff --git a/src/components/ComposerV2/store/__tests__/media.test.ts b/src/components/ComposerV2/store/__tests__/media.test.ts index 49e10d8841..a395256ef7 100644 --- a/src/components/ComposerV2/store/__tests__/media.test.ts +++ b/src/components/ComposerV2/store/__tests__/media.test.ts @@ -1,6 +1,16 @@ import {type AtpAgent} from '@atproto/api' import {beforeEach, describe, expect, jest, test} from '@jest/globals' +// Avoid pulling the UI module chain (gallery → media picker → ALF) into the +// test environment via the resolveLink import in linkResolution.ts. +jest.mock('#/lib/api/resolve', () => ({ + resolveLink: jest.fn(), +})) +jest.mock('#/state/session/agent', () => ({ + createPublicAgent: jest.fn(() => ({})), +})) + +import {type resolveLink} from '#/lib/api/resolve' import {type Gif} from '#/state/queries/tenor' import {createThreadStore} from '#/components/ComposerV2/store' import { @@ -47,13 +57,29 @@ const gifInput: AddMediaInput = { gif: {url: 'https://example.com/g.gif'} as Gif, } +// Embed-routing tests live in embeds.test.ts; here we just need a never- +// resolving resolveLink so any addUri-driven resolution doesn't crash and +// no result ever lands. The promise never settles, which is what we want. +let mockResolveLink: jest.Mock + beforeEach(() => { jest.useFakeTimers() + mockResolveLink = jest.fn( + () => new Promise(() => {}), + ) as unknown as jest.Mock }) +function makeStore() { + return createThreadStore({ + agent, + __createId: makeIdGenerator(), + __resolveLink: mockResolveLink as unknown as typeof resolveLink, + }) +} + describe('addMedia', () => { test('adds a single image with pending upload status and returns its id', () => { - const store = createThreadStore({agent, __createId: makeIdGenerator()}) + const store = makeStore() const root = rootId(store) const ids = store.actions.addMedia(root, [imageInput]) expect(ids).toEqual(['id-2']) @@ -68,7 +94,7 @@ describe('addMedia', () => { }) test('preserves input order in returned ids and on the post', () => { - const store = createThreadStore({agent, __createId: makeIdGenerator()}) + const store = makeStore() const root = rootId(store) const ids = store.actions.addMedia(root, [ imageInput, @@ -80,14 +106,14 @@ describe('addMedia', () => { }) test('marks state dirty', () => { - const store = createThreadStore({agent, __createId: makeIdGenerator()}) + const store = makeStore() expect(store.getState().isDirty).toBe(false) store.actions.addMedia(rootId(store), [imageInput]) expect(store.getState().isDirty).toBe(true) }) test('returns undefined and is a no-op when post id is unknown', () => { - const store = createThreadStore({agent, __createId: makeIdGenerator()}) + const store = makeStore() const before = store.getState() const result = store.actions.addMedia('does-not-exist', [imageInput]) expect(result).toBeUndefined() @@ -95,7 +121,7 @@ describe('addMedia', () => { }) test('returns [] for an empty input list and is a no-op', () => { - const store = createThreadStore({agent, __createId: makeIdGenerator()}) + const store = makeStore() const before = store.getState() const result = store.actions.addMedia(rootId(store), []) expect(result).toEqual([]) @@ -103,7 +129,7 @@ describe('addMedia', () => { }) test('drives an image upload from pending -> uploading -> uploaded', () => { - const store = createThreadStore({agent, __createId: makeIdGenerator()}) + const store = makeStore() const root = rootId(store) const [imageId] = store.actions.addMedia(root, [imageInput])! @@ -121,7 +147,7 @@ describe('addMedia', () => { }) test('drives a video upload through to uploaded', () => { - const store = createThreadStore({agent, __createId: makeIdGenerator()}) + const store = makeStore() const root = rootId(store) const [videoId] = store.actions.addMedia(root, [videoInput])! @@ -137,7 +163,7 @@ describe('addMedia', () => { }) test('does not start an upload task for a gif', () => { - const store = createThreadStore({agent, __createId: makeIdGenerator()}) + const store = makeStore() const root = rootId(store) store.actions.addMedia(root, [gifInput]) jest.runAllTimers() @@ -150,7 +176,7 @@ describe('addMedia', () => { describe('addMedia input validation (first item dictates kind, cap by count)', () => { test('image-first: filters out non-images and caps at 4', () => { - const store = createThreadStore({agent, __createId: makeIdGenerator()}) + const store = makeStore() const root = rootId(store) const ids = store.actions.addMedia(root, [ imageInput, @@ -168,7 +194,7 @@ describe('addMedia input validation (first item dictates kind, cap by count)', ( }) test('video-first: filters out non-videos and caps at 1', () => { - const store = createThreadStore({agent, __createId: makeIdGenerator()}) + const store = makeStore() const root = rootId(store) const ids = store.actions.addMedia(root, [ videoInput, @@ -180,7 +206,7 @@ describe('addMedia input validation (first item dictates kind, cap by count)', ( }) test('gif-first: filters out non-gifs and caps at 1', () => { - const store = createThreadStore({agent, __createId: makeIdGenerator()}) + const store = makeStore() const root = rootId(store) const ids = store.actions.addMedia(root, [gifInput, gifInput, imageInput]) expect(ids).toHaveLength(1) @@ -190,7 +216,7 @@ describe('addMedia input validation (first item dictates kind, cap by count)', ( describe('addMedia respects existing media on the post', () => { test('appends images up to a total of 4 when the post already has images', () => { - const store = createThreadStore({agent, __createId: makeIdGenerator()}) + const store = makeStore() const root = rootId(store) store.actions.addMedia(root, [imageInput, imageInput]) const ids = store.actions.addMedia(root, [ @@ -204,7 +230,7 @@ describe('addMedia respects existing media on the post', () => { }) test('drops non-image inputs when the post already has images', () => { - const store = createThreadStore({agent, __createId: makeIdGenerator()}) + const store = makeStore() const root = rootId(store) store.actions.addMedia(root, [imageInput]) const ids = store.actions.addMedia(root, [videoInput, gifInput]) @@ -213,7 +239,7 @@ describe('addMedia respects existing media on the post', () => { }) test('is a no-op when the post already has 4 images', () => { - const store = createThreadStore({agent, __createId: makeIdGenerator()}) + const store = makeStore() const root = rootId(store) store.actions.addMedia(root, [ imageInput, @@ -228,7 +254,7 @@ describe('addMedia respects existing media on the post', () => { }) test('is a no-op when the post already has a video', () => { - const store = createThreadStore({agent, __createId: makeIdGenerator()}) + const store = makeStore() const root = rootId(store) store.actions.addMedia(root, [videoInput]) const before = store.getState() @@ -238,7 +264,7 @@ describe('addMedia respects existing media on the post', () => { }) test('is a no-op when the post already has a gif', () => { - const store = createThreadStore({agent, __createId: makeIdGenerator()}) + const store = makeStore() const root = rootId(store) store.actions.addMedia(root, [gifInput]) const before = store.getState() @@ -248,9 +274,9 @@ describe('addMedia respects existing media on the post', () => { }) test('is a no-op when the post has an external link card', () => { - const store = createThreadStore({agent, __createId: makeIdGenerator()}) + const store = makeStore() const root = rootId(store) - store.actions.setExternalEmbed(root, {uri: 'https://example.com'}) + store.actions.addUri(root, 'https://example.com') const before = store.getState() const ids = store.actions.addMedia(root, [imageInput]) expect(ids).toEqual([]) @@ -260,7 +286,7 @@ describe('addMedia respects existing media on the post', () => { describe('selectionsRemaining flags on the post', () => { test('empty post starts with 4 / 1 / 1', () => { - const store = createThreadStore({agent, __createId: makeIdGenerator()}) + const store = makeStore() const post = store.getState().posts[rootId(store)] expect(post.imageSelectionsRemaining).toBe(4) expect(post.videoSelectionsRemaining).toBe(1) @@ -268,7 +294,7 @@ describe('selectionsRemaining flags on the post', () => { }) test('decrements as images are added', () => { - const store = createThreadStore({agent, __createId: makeIdGenerator()}) + const store = makeStore() const root = rootId(store) store.actions.addMedia(root, [imageInput, imageInput]) let post = store.getState().posts[root] @@ -282,7 +308,7 @@ describe('selectionsRemaining flags on the post', () => { }) test('a video locks all three counters to 0', () => { - const store = createThreadStore({agent, __createId: makeIdGenerator()}) + const store = makeStore() const root = rootId(store) store.actions.addMedia(root, [videoInput]) const post = store.getState().posts[root] @@ -292,7 +318,7 @@ describe('selectionsRemaining flags on the post', () => { }) test('a gif locks all three counters to 0', () => { - const store = createThreadStore({agent, __createId: makeIdGenerator()}) + const store = makeStore() const root = rootId(store) store.actions.addMedia(root, [gifInput]) const post = store.getState().posts[root] @@ -302,7 +328,7 @@ describe('selectionsRemaining flags on the post', () => { }) test('removing media restores capacity', () => { - const store = createThreadStore({agent, __createId: makeIdGenerator()}) + const store = makeStore() const root = rootId(store) const [imgId] = store.actions.addMedia(root, [imageInput])! expect(store.getState().posts[root].imageSelectionsRemaining).toBe(3) @@ -314,9 +340,9 @@ describe('selectionsRemaining flags on the post', () => { }) test('an external link card locks all three counters to 0', () => { - const store = createThreadStore({agent, __createId: makeIdGenerator()}) + const store = makeStore() const root = rootId(store) - store.actions.setExternalEmbed(root, {uri: 'https://example.com'}) + store.actions.addUri(root, 'https://example.com') const post = store.getState().posts[root] expect(post.imageSelectionsRemaining).toBe(0) expect(post.videoSelectionsRemaining).toBe(0) @@ -324,10 +350,10 @@ describe('selectionsRemaining flags on the post', () => { }) test('removing the external link card restores capacity', () => { - const store = createThreadStore({agent, __createId: makeIdGenerator()}) + const store = makeStore() const root = rootId(store) - store.actions.setExternalEmbed(root, {uri: 'https://example.com'}) - store.actions.removeExternalEmbed(root) + store.actions.addUri(root, 'https://example.com') + store.actions.removeEmbed(root) const post = store.getState().posts[root] expect(post.imageSelectionsRemaining).toBe(4) expect(post.videoSelectionsRemaining).toBe(1) @@ -337,7 +363,7 @@ describe('selectionsRemaining flags on the post', () => { describe('removeMedia', () => { test('removes the matching media and leaves others intact', () => { - const store = createThreadStore({agent, __createId: makeIdGenerator()}) + const store = makeStore() const root = rootId(store) const [a, b] = store.actions.addMedia(root, [imageInput, imageInput])! @@ -346,7 +372,7 @@ describe('removeMedia', () => { }) test('cancels in-flight upload (no further status writes after removal)', () => { - const store = createThreadStore({agent, __createId: makeIdGenerator()}) + const store = makeStore() const root = rootId(store) const [imageId] = store.actions.addMedia(root, [imageInput])! jest.advanceTimersByTime(100) @@ -356,7 +382,7 @@ describe('removeMedia', () => { }) test('is a no-op when media id is unknown', () => { - const store = createThreadStore({agent, __createId: makeIdGenerator()}) + const store = makeStore() const root = rootId(store) store.actions.addMedia(root, [imageInput]) const before = store.getState() @@ -367,7 +393,7 @@ describe('removeMedia', () => { describe('retryMediaUpload', () => { test('resets a failed image upload back to pending and walks it to uploaded', () => { - const store = createThreadStore({agent, __createId: makeIdGenerator()}) + const store = makeStore() const root = rootId(store) const [imageId] = store.actions.addMedia(root, [imageInput])! store.actions.setUploadStatus(root, imageId, { @@ -389,7 +415,7 @@ describe('retryMediaUpload', () => { }) test('failed status carries a bound retry() method that restarts the upload', () => { - const store = createThreadStore({agent, __createId: makeIdGenerator()}) + const store = makeStore() const root = rootId(store) const [imageId] = store.actions.addMedia(root, [imageInput])! store.actions.setUploadStatus(root, imageId, { @@ -413,7 +439,7 @@ describe('retryMediaUpload', () => { }) test('is a no-op for a gif media id', () => { - const store = createThreadStore({agent, __createId: makeIdGenerator()}) + const store = makeStore() const root = rootId(store) const [gifId] = store.actions.addMedia(root, [gifInput])! const before = store.getState() @@ -422,7 +448,7 @@ describe('retryMediaUpload', () => { }) test('is a no-op when post or media id is unknown', () => { - const store = createThreadStore({agent, __createId: makeIdGenerator()}) + const store = makeStore() const root = rootId(store) store.actions.addMedia(root, [imageInput]) const before = store.getState() @@ -435,7 +461,7 @@ describe('retryMediaUpload', () => { describe('updateMediaAltText', () => { test('updates only the matching media (image)', () => { - const store = createThreadStore({agent, __createId: makeIdGenerator()}) + const store = makeStore() const root = rootId(store) const [a, b] = store.actions.addMedia(root, [imageInput, imageInput])! store.actions.updateMediaAltText(root, b, 'a description') @@ -446,7 +472,7 @@ describe('updateMediaAltText', () => { }) test('works on a gif as well', () => { - const store = createThreadStore({agent, __createId: makeIdGenerator()}) + const store = makeStore() const root = rootId(store) const [gifId] = store.actions.addMedia(root, [gifInput])! store.actions.updateMediaAltText(root, gifId, 'animated joy') @@ -454,7 +480,7 @@ describe('updateMediaAltText', () => { }) test('is a no-op when alt text is unchanged', () => { - const store = createThreadStore({agent, __createId: makeIdGenerator()}) + const store = makeStore() const root = rootId(store) const [imageId] = store.actions.addMedia(root, [imageInput])! const before = store.getState() @@ -465,7 +491,7 @@ describe('updateMediaAltText', () => { describe('removePost cancels media uploads', () => { test('removing a post cancels any in-flight uploads on that post', () => { - const store = createThreadStore({agent, __createId: makeIdGenerator()}) + const store = makeStore() const a = rootId(store) const b = store.actions.addPost('after', a) store.actions.addMedia(b, [imageInput]) @@ -479,7 +505,7 @@ describe('removePost cancels media uploads', () => { describe('destroy cancels uploads', () => { test('destroy stops any in-flight uploads', () => { - const store = createThreadStore({agent, __createId: makeIdGenerator()}) + const store = makeStore() const root = rootId(store) store.actions.addMedia(root, [imageInput]) jest.advanceTimersByTime(100) diff --git a/src/components/ComposerV2/store/__tests__/notify.test.ts b/src/components/ComposerV2/store/__tests__/notify.test.ts index 84ae2360ab..14d4b2422a 100644 --- a/src/components/ComposerV2/store/__tests__/notify.test.ts +++ b/src/components/ComposerV2/store/__tests__/notify.test.ts @@ -1,5 +1,14 @@ -import {describe, expect, jest, test} from '@jest/globals' import {type AtpAgent} from '@atproto/api' +import {describe, expect, jest, test} from '@jest/globals' + +// Avoid pulling the UI module chain into the test environment via the +// resolveLink import in linkResolution.ts. +jest.mock('#/lib/api/resolve', () => ({ + resolveLink: jest.fn(), +})) +jest.mock('#/state/session/agent', () => ({ + createPublicAgent: jest.fn(() => ({})), +})) import {createThreadStore} from '#/components/ComposerV2/store' diff --git a/src/components/ComposerV2/store/__tests__/posts.test.ts b/src/components/ComposerV2/store/__tests__/posts.test.ts index bad0adc7b3..7018acb5e2 100644 --- a/src/components/ComposerV2/store/__tests__/posts.test.ts +++ b/src/components/ComposerV2/store/__tests__/posts.test.ts @@ -1,5 +1,14 @@ -import {describe, expect, test} from '@jest/globals' import {type AtpAgent} from '@atproto/api' +import {describe, expect, jest, test} from '@jest/globals' + +// Avoid pulling the UI module chain into the test environment via the +// resolveLink import in linkResolution.ts. +jest.mock('#/lib/api/resolve', () => ({ + resolveLink: jest.fn(), +})) +jest.mock('#/state/session/agent', () => ({ + createPublicAgent: jest.fn(() => ({})), +})) import {createThreadStore} from '#/components/ComposerV2/store' diff --git a/src/components/ComposerV2/store/index.ts b/src/components/ComposerV2/store/index.ts index 39a8f08bc3..af1481dcba 100644 --- a/src/components/ComposerV2/store/index.ts +++ b/src/components/ComposerV2/store/index.ts @@ -1,6 +1,8 @@ import {type AtpAgent} from '@atproto/api' import {nanoid} from 'nanoid/non-secure' +import {type resolveLink} from '#/lib/api/resolve' +import {startUriResolution} from '#/components/ComposerV2/store/linkResolution' import type * as types from '#/components/ComposerV2/store/types' import { startImageUpload, @@ -18,9 +20,12 @@ export function createThreadStore(options: { agent: AtpAgent /** Override id generation; useful for deterministic tests. */ __createId?: () => string + /** Override link resolver; useful for deterministic tests. */ + __resolveLink?: typeof resolveLink }) { const id = options.__createId ?? nanoid const agent = options.agent + const resolveLinkOverride = options.__resolveLink let state: types.ThreadState = { posts: {[id()]: buildThreadPost()}, isDirty: false, @@ -37,6 +42,21 @@ export function createThreadStore(options: { */ const uploadTasks = new Map() + /** + * Generation counter per post for embed link resolution. Cancellation is + * implemented by ignoring stale resolution callbacks: every action that + * starts or invalidates a resolution (addUri, removeEmbed, removePost, + * destroy) bumps the post's gen, and the worker callback compares its + * captured gen against the current value before writing to state. + */ + const embedGenByPost = new Map() + + function bumpEmbedGen(postId: string): number { + const next = (embedGenByPost.get(postId) ?? 0) + 1 + embedGenByPost.set(postId, next) + return next + } + /** * Action bodies mutate `s` in place. Returning `null` signals a no-op (the * state ref is preserved and listeners are not notified). Otherwise we @@ -115,6 +135,10 @@ export function createThreadStore(options: { if (!(postId in s.posts)) return null // Cancel any in-flight uploads for media on this post before dropping it. for (const m of s.posts[postId].media) cancelUploadTask(m.id) + // Bump (and drop) the embed gen so a stale resolution callback for + // this post can never write back into state. + bumpEmbedGen(postId) + embedGenByPost.delete(postId) delete s.posts[postId] s.isDirty = true return s @@ -136,10 +160,11 @@ export function createThreadStore(options: { if (!(postId in state.posts)) return undefined if (inputs.length === 0) return [] - // External link cards are mutually exclusive with media. This rule is - // permanent (unlike the kind/cap rules in filterMediaInputs) so it lives - // here at the action boundary rather than inside the filter helper. - if (state.posts[postId].external !== undefined) return [] + // Embeds (external link cards, feed/list/starter-pack record cards, and + // the in-flight pending state) are mutually exclusive with media. This + // rule is permanent (unlike the kind/cap rules in filterMediaInputs) so + // it lives here at the action boundary rather than inside the filter. + if (state.posts[postId].embed !== undefined) return [] const accepted = filterMediaInputs(state.posts[postId].media, inputs) if (accepted.length === 0) return [] @@ -258,22 +283,100 @@ export function createThreadStore(options: { ) } - function setExternalEmbed(postId: string, external: types.PostEmbedExternal) { + /** + * Generic URI handler. Sets `embed` to `pending` synchronously, kicks off + * `resolveLink`, and routes the outcome: + * - Bluesky post -> goes to the post's `quote` field (clears embed). If + * `quote` is already set, the new outcome is dropped silently to + * preserve the user's prior selection. + * - Feed / list / starter-pack / external -> stays on `embed`, unless the + * post has media in which case it's dropped silently (embed cleared). + * - Failure -> embed is set to a `failed` state with a bound `retry()` that + * re-runs `addUri` for the same URI. + * + * Cancellation is gen-based: any later addUri / removeEmbed / removePost / + * destroy invalidates this call's outcome before it can land. + */ + function addUri(postId: string, uri: string) { + if (!(postId in state.posts)) return + const gen = bumpEmbedGen(postId) mutateState(s => { const post = s.posts[postId] if (!post) return null - s.posts[postId] = setPostExternal(post, external) + s.posts[postId] = setPostEmbed(post, {state: 'pending', uri}) s.isDirty = true return s }) + startUriResolution({ + postId, + uri, + resolveLink: resolveLinkOverride, + onResolve: handleEmbedResolution(gen), + }) } - function removeExternalEmbed(postId: string) { + function handleEmbedResolution(gen: number) { + return (postId: string, outcome: types.LinkResolutionOutcome) => { + if (destroyed) return + if (embedGenByPost.get(postId) !== gen) return + if (outcome.kind === 'post') { + mutateState(s => { + const post = s.posts[postId] + if (!post) return null + if (post.quote !== undefined) { + // Quote already set: drop the post outcome, just clear pending. + s.posts[postId] = setPostEmbed(post, undefined) + return s + } + s.posts[postId] = setPostEmbed( + { + ...post, + quote: { + uri: outcome.record.uri, + cid: outcome.record.cid, + view: outcome.view, + }, + }, + undefined, + ) + return s + }) + return + } + // outcome.kind === 'embed' + const post = state.posts[postId] + if (!post) return + if (post.media.length > 0) { + // Embed-vs-media collision: silent drop, clear pending. + mutateState(s => { + const p = s.posts[postId] + if (!p) return null + s.posts[postId] = setPostEmbed(p, undefined) + return s + }) + return + } + const embed = outcome.embed + const stored: types.PostEmbed = + embed.state === 'failed' + ? {...embed, retry: () => addUri(postId, embed.uri)} + : embed + mutateState(s => { + const p = s.posts[postId] + if (!p) return null + s.posts[postId] = setPostEmbed(p, stored) + return s + }) + } + } + + function removeEmbed(postId: string) { + bumpEmbedGen(postId) mutateState(s => { const post = s.posts[postId] if (!post) return null - if (post.external === undefined) return null - s.posts[postId] = setPostExternal(post, undefined) + if (post.embed === undefined) return null + s.posts[postId] = setPostEmbed(post, undefined) s.isDirty = true return s }) @@ -356,23 +459,23 @@ export function createThreadStore(options: { return { ...post, media, - ...computePostMediaSelectionsRemaining(media, post.external), + ...computePostMediaSelectionsRemaining(media, post.embed), } } /** - * Single chokepoint for replacing a post's external link card. Mirrors - * setPostMedia so the selectionsRemaining flags stay consistent (an - * external link blocks all media selections). + * Single chokepoint for replacing a post's embed slot. Mirrors + * setPostMedia so the selectionsRemaining flags stay consistent (any + * embed - including pending and failed - blocks all media selections). */ - function setPostExternal( + function setPostEmbed( post: types.ThreadPost, - external: types.PostEmbedExternal | undefined, + embed: types.PostEmbed | undefined, ): types.ThreadPost { return { ...post, - external, - ...computePostMediaSelectionsRemaining(post.media, external), + embed, + ...computePostMediaSelectionsRemaining(post.media, embed), } } @@ -387,8 +490,8 @@ export function createThreadStore(options: { removeMedia, updateMediaAltText, retryMediaUpload, - setExternalEmbed, - removeExternalEmbed, + addUri, + removeEmbed, setQuoteEmbed, removeQuoteEmbed, setUploadStatus, @@ -397,6 +500,7 @@ export function createThreadStore(options: { destroyed = true for (const task of uploadTasks.values()) task.cancel() uploadTasks.clear() + embedGenByPost.clear() }, getState() { return state diff --git a/src/components/ComposerV2/store/linkResolution.ts b/src/components/ComposerV2/store/linkResolution.ts new file mode 100644 index 0000000000..ec5f54ce6f --- /dev/null +++ b/src/components/ComposerV2/store/linkResolution.ts @@ -0,0 +1,78 @@ +/** + * Link metadata resolution worker for the ComposerV2 store. + * + * Runs `resolveLink` (which classifies a URI as a post / feed / list / + * starter-pack record or as an external link card) and reports the outcome + * back via the `onResolve` callback. The store routes `kind: 'post'` outcomes + * to the post's `quote` field and everything else to the `embed` field. + * + * No cancellation surface: cancellation is handled at the store level by + * incrementing a per-post generation counter and ignoring stale callbacks. + * The `__resolveLink` test seam (passed through from createThreadStore) + * lets tests inject a controllable promise. + */ +import { + type ResolvedLink, + type resolveLink as defaultResolveLink, +} from '#/lib/api/resolve' +import {resolveLink as importedResolveLink} from '#/lib/api/resolve' +import {createPublicAgent} from '#/state/session/agent' +import {type LinkResolutionOutcome} from './types' + +export type StartUriResolutionOptions = { + postId: string + uri: string + /** Test seam; defaults to the imported resolveLink. */ + resolveLink?: typeof defaultResolveLink + onResolve: (postId: string, outcome: LinkResolutionOutcome) => void +} + +export function startUriResolution(opts: StartUriResolutionOptions): void { + const fn = opts.resolveLink ?? importedResolveLink + fn(createPublicAgent(), opts.uri).then( + link => opts.onResolve(opts.postId, mapResolvedLink(link)), + err => + opts.onResolve(opts.postId, { + kind: 'embed', + embed: { + state: 'failed', + uri: opts.uri, + error: String((err && (err as Error).message) ?? err), + }, + }), + ) +} + +function mapResolvedLink(link: ResolvedLink): LinkResolutionOutcome { + if (link.type === 'external') { + return { + kind: 'embed', + embed: { + state: 'external', + uri: link.uri, + title: link.title, + description: link.description, + thumb: link.thumb, + }, + } + } + if (link.kind === 'post') { + return {kind: 'post', record: link.record, view: link.view} + } + if (link.kind === 'feed') { + return { + kind: 'embed', + embed: {state: 'feed', record: link.record, view: link.view}, + } + } + if (link.kind === 'list') { + return { + kind: 'embed', + embed: {state: 'list', record: link.record, view: link.view}, + } + } + return { + kind: 'embed', + embed: {state: 'starter-pack', record: link.record, view: link.view}, + } +} diff --git a/src/components/ComposerV2/store/types.ts b/src/components/ComposerV2/store/types.ts index b7db7215e7..ccd81f0de9 100644 --- a/src/components/ComposerV2/store/types.ts +++ b/src/components/ComposerV2/store/types.ts @@ -1,5 +1,11 @@ -import {type BlobRef} from '@atproto/api' +import { + type AppBskyFeedDefs, + type AppBskyGraphDefs, + type BlobRef, + type ComAtprotoRepoStrongRef, +} from '@atproto/api' +import {type ComposerImage} from '#/state/gallery' import {type Gif} from '#/state/queries/tenor' /** @@ -76,13 +82,92 @@ export type PostEmbedMedia = | (PostEmbedMediaVideo & {kind: 'video'}) | (PostEmbedMediaGif & {kind: 'gif'}) -export type PostEmbedExternal = { - uri: string -} +/** + * What a link-resolution reporter (the worker, or a test) sends in. Failed + * inputs carry just the error string; the store wraps the failure with a + * bound `retry()` method when it stores the status. + * + * `pending` is included so the store can construct the initial pending state + * with the same type vocabulary, but the worker never emits `pending` - it + * only emits terminal outcomes (the post-resolution variants and `failed`). + */ +export type EmbedResolution = + | {state: 'pending'; uri: string} + | {state: 'failed'; uri: string; error: string} + | { + state: 'external' + uri: string + title: string + description: string + thumb: ComposerImage | undefined + } + | { + state: 'feed' + record: ComAtprotoRepoStrongRef.Main + view: AppBskyFeedDefs.GeneratorView + } + | { + state: 'list' + record: ComAtprotoRepoStrongRef.Main + view: AppBskyGraphDefs.ListView + } + | { + state: 'starter-pack' + record: ComAtprotoRepoStrongRef.Main + view: AppBskyGraphDefs.StarterPackView + } + +/** + * What's stored on a post's `embed` field. Mirrors PostMediaUploadStatus' + * shape: the failed variant has a bound `retry()` so UI can call it directly + * without having to look up the post id. + * + * Note: `retry` is a function reference and won't survive JSON serialization. + * On restore (OS-resume / draft load), the store re-attaches it. + */ +export type PostEmbed = + | {state: 'pending'; uri: string} + | {state: 'failed'; uri: string; error: string; retry: () => void} + | { + state: 'external' + uri: string + title: string + description: string + thumb: ComposerImage | undefined + } + | { + state: 'feed' + record: ComAtprotoRepoStrongRef.Main + view: AppBskyFeedDefs.GeneratorView + } + | { + state: 'list' + record: ComAtprotoRepoStrongRef.Main + view: AppBskyGraphDefs.ListView + } + | { + state: 'starter-pack' + record: ComAtprotoRepoStrongRef.Main + view: AppBskyGraphDefs.StarterPackView + } + +/** + * Worker output for an `addUri` call. The store routes `kind: 'post'` to the + * post's `quote` field and everything else to the `embed` field. + */ +export type LinkResolutionOutcome = + | { + kind: 'post' + record: ComAtprotoRepoStrongRef.Main + view: AppBskyFeedDefs.PostView + } + | {kind: 'embed'; embed: Exclude} export type PostEmbedQuote = { uri: string cid: string + /** Hydrated post view; populated when addUri resolves a post. */ + view?: AppBskyFeedDefs.PostView } export type ThreadPost = { @@ -90,7 +175,8 @@ export type ThreadPost = { langs: string[] labels: string[] media: PostEmbedMedia[] - external: PostEmbedExternal | undefined + /** Single non-quote embed slot. Mutually exclusive with media. */ + embed: PostEmbed | undefined quote: PostEmbedQuote | undefined /** * Derived from `media`. How many more items of each kind addMedia would diff --git a/src/components/ComposerV2/store/utils/buildThreadPost.ts b/src/components/ComposerV2/store/utils/buildThreadPost.ts index 3d15588b2f..033a77b25a 100644 --- a/src/components/ComposerV2/store/utils/buildThreadPost.ts +++ b/src/components/ComposerV2/store/utils/buildThreadPost.ts @@ -7,7 +7,7 @@ export function buildThreadPost(): ThreadPost { langs: [], labels: [], media: [], - external: undefined, + embed: undefined, quote: undefined, ...computePostMediaSelectionsRemaining([], undefined), } diff --git a/src/components/ComposerV2/store/utils/computePostMediaSelectionsRemaining.ts b/src/components/ComposerV2/store/utils/computePostMediaSelectionsRemaining.ts index 01b77c4787..be29da47a7 100644 --- a/src/components/ComposerV2/store/utils/computePostMediaSelectionsRemaining.ts +++ b/src/components/ComposerV2/store/utils/computePostMediaSelectionsRemaining.ts @@ -1,24 +1,24 @@ import {MAX_IMAGES_PER_POST} from '#/components/ComposerV2/store/const' import { - type PostEmbedExternal, + type PostEmbed, type PostEmbedMedia, } from '#/components/ComposerV2/store/types' /** - * Mirrors filterMediaInputs' rules. With no media (and no external link), - * all kinds are open at their per-kind cap. With existing images, only - * images are open up to a total of MAX_IMAGES_PER_POST. With an existing - * video, gif, or external link card, nothing more can be added. + * Mirrors filterMediaInputs' rules. With no media and no embed, all kinds + * are open at their per-kind cap. With existing images, only images are + * open up to a total of MAX_IMAGES_PER_POST. With an existing video, gif, + * or any embed (including pending and failed), nothing more can be added. */ export function computePostMediaSelectionsRemaining( media: PostEmbedMedia[], - external: PostEmbedExternal | undefined, + embed: PostEmbed | undefined, ): { imageSelectionsRemaining: number videoSelectionsRemaining: number gifSelectionsRemaining: number } { - if (external !== undefined) { + if (embed !== undefined) { return { imageSelectionsRemaining: 0, videoSelectionsRemaining: 0,