diff --git a/src/components/profile-heading/editor/sections/heading/imageHelpers.ts b/src/components/profile-heading/editor/sections/heading/imageHelpers.ts index dd3dcb9b..179345a4 100644 --- a/src/components/profile-heading/editor/sections/heading/imageHelpers.ts +++ b/src/components/profile-heading/editor/sections/heading/imageHelpers.ts @@ -49,6 +49,11 @@ function subjectDirectoryUri (subject: NamedNode): string { return lastSlash >= 0 ? docUri.slice(0, lastSlash + 1) : docUri } +function fallbackFilename (extension: string, index: number): string { + const randomPart = Math.random().toString(36).slice(2, 10) + return `image_${Date.now()}_${index}_${randomPart}.${extension}` +} + export async function resolvePhotoDisplaySrc (store: LiveStore, imageSrc?: string): Promise { if (!imageSrc || imageSrc.startsWith('blob:') || imageSrc.startsWith('data:')) { return imageSrc @@ -100,23 +105,72 @@ export async function uploadPhotoFile (store: LiveStore, subject: NamedNode, fil } const directoryUri = subjectDirectoryUri(subject) + const data = await file.arrayBuffer() let candidateUri = `${directoryUri}${filename}` + let fallbackIndex = 0 + let checkCandidate = true + + while (true) { + const linkedPhoto = store.holds(subject, ns.vcard('hasPhoto'), sym(candidateUri)) + if (!checkCandidate) { + if (linkedPhoto) { + candidateUri = `${directoryUri}${fallbackFilename(extension, fallbackIndex)}` + fallbackIndex++ + continue + } + } else if (!linkedPhoto) { + let response: Response + try { + response = await store.fetcher.webOperation('HEAD', candidateUri) + } catch (error: any) { + const status = error?.response?.status ?? error?.status + if (status === 404) { + response = { status: 404, ok: false, statusText: 'Not Found' } as Response + } else { + throw new Error(`Error checking picture destination: ${error}`) + } + } + + if (response.status === 404) { + checkCandidate = false + } else if (!response.ok) { + throw new Error(`Error checking picture destination: ${response.status} ${response.statusText}`) + } else { + candidateUri = `${directoryUri}${fallbackFilename(extension, fallbackIndex)}` + fallbackIndex++ + checkCandidate = false + } + } else if (linkedPhoto) { + candidateUri = `${directoryUri}${fallbackFilename(extension, fallbackIndex)}` + fallbackIndex++ + checkCandidate = false + continue + } - for (let index = 0; store.holds(subject, ns.vcard('hasPhoto'), sym(candidateUri)); index++) { - const fallbackName = `image_${index}.${extension}` - candidateUri = `${directoryUri}${fallbackName}` - } - try { - const data = await file.arrayBuffer() - const response = await store.fetcher.webOperation('PUT', candidateUri, { - data: data as unknown as string, - contentType: detectedContentType - } as any) - if (!response.ok) { - throw new Error(`Error uploading picture: ${response.status} ${response.statusText}`) + try { + const response = await store.fetcher.webOperation('PUT', candidateUri, { + data: data as unknown as string, + contentType: detectedContentType, + headers: { 'If-None-Match': '*' } + } as any) + if (response.status === 412) { + candidateUri = `${directoryUri}${fallbackFilename(extension, fallbackIndex)}` + fallbackIndex++ + continue + } + if (!response.ok) { + throw new Error(`Error uploading picture: ${response.status} ${response.statusText}`) + } + } catch (error) { + const status = (error as any)?.response?.status ?? (error as any)?.status + if (status === 412) { + candidateUri = `${directoryUri}${fallbackFilename(extension, fallbackIndex)}` + fallbackIndex++ + continue + } + throw new Error(`Error uploading picture: ${error}`) } - } catch (error) { - throw new Error(`Error uploading picture: ${error}`) + break } const currentUser = authn.currentUser() diff --git a/test/unit/mainPage/photoUpload.test.ts b/test/unit/mainPage/photoUpload.test.ts new file mode 100644 index 00000000..8bedf67a --- /dev/null +++ b/test/unit/mainPage/photoUpload.test.ts @@ -0,0 +1,100 @@ +import { describe, expect, it, vi } from 'vitest' +import { sym } from 'rdflib' +import { uploadPhotoFile } from '../../../src/components/profile-heading/editor/sections/heading/imageHelpers' + +describe('photo upload destinations', () => { + it('skips an existing unlinked resource and creates conditionally', async () => { + const webOperation = vi.fn() + .mockResolvedValueOnce({ status: 200, ok: true, statusText: 'OK' }) + .mockResolvedValueOnce({ status: 201, ok: true, statusText: 'Created' }) + const store = { + fetcher: { webOperation }, + holds: vi.fn(() => false) + } + const file = new File(['image'], 'avatar.png', { type: 'image/png' }) + const subject = sym('https://pod.example/profile/card#me') + + await expect(uploadPhotoFile(store as never, subject, file)) + .resolves.toMatch(/^https:\/\/pod\.example\/profile\/image_\d+_[01]_[a-z0-9]+\.png$/) + + expect(webOperation).toHaveBeenNthCalledWith( + 1, + 'HEAD', + 'https://pod.example/profile/avatar.png' + ) + expect(webOperation).toHaveBeenNthCalledWith( + 2, + 'PUT', + expect.stringMatching(/^https:\/\/pod\.example\/profile\/image_\d+_[01]_[a-z0-9]+\.png$/), + expect.objectContaining({ + contentType: 'image/png', + headers: { 'If-None-Match': '*' } + }) + ) + }) + + it('retries with a fallback name when conditional creation loses a race', async () => { + const webOperation = vi.fn() + .mockResolvedValueOnce({ status: 404, ok: false, statusText: 'Not Found' }) + .mockResolvedValueOnce({ status: 412, ok: false, statusText: 'Precondition Failed' }) + .mockResolvedValueOnce({ status: 201, ok: true, statusText: 'Created' }) + const store = { + fetcher: { webOperation }, + holds: vi.fn(() => false) + } + const file = new File(['image'], 'avatar.png', { type: 'image/png' }) + const subject = sym('https://pod.example/profile/card#me') + + await expect(uploadPhotoFile(store as never, subject, file)) + .resolves.toMatch(/^https:\/\/pod\.example\/profile\/image_\d+_[01]_[a-z0-9]+\.png$/) + + expect(webOperation).toHaveBeenNthCalledWith( + 3, + 'PUT', + expect.stringMatching(/^https:\/\/pod\.example\/profile\/image_\d+_[01]_[a-z0-9]+\.png$/), + expect.objectContaining({ headers: { 'If-None-Match': '*' } }) + ) + }) + + it('skips a linked inaccessible resource without checking it', async () => { + const webOperation = vi.fn() + .mockResolvedValue({ status: 201, ok: true, statusText: 'Created' }) + let holdsCalls = 0 + const store = { + fetcher: { webOperation }, + holds: vi.fn(() => holdsCalls++ < 1) + } + const file = new File(['image'], 'avatar.png', { type: 'image/png' }) + const subject = sym('https://pod.example/profile/card#me') + + await expect(uploadPhotoFile(store as never, subject, file)) + .resolves.toMatch(/^https:\/\/pod\.example\/profile\/image_\d+_\d+_[a-z0-9]+\.png$/) + + expect(webOperation.mock.calls.filter(([method]) => method === 'HEAD')).toHaveLength(0) + expect(webOperation).toHaveBeenCalledWith( + 'PUT', + expect.stringMatching(/^https:\/\/pod\.example\/profile\/image_\d+_\d+_[a-z0-9]+\.png$/), + expect.objectContaining({ headers: { 'If-None-Match': '*' } }) + ) + }) + + it('advances after a thrown 412 without rescanning prior names', async () => { + const conditionalError = Object.assign(new Error('Precondition Failed'), { status: 412 }) + const webOperation = vi.fn() + .mockResolvedValueOnce({ status: 404, ok: false, statusText: 'Not Found' }) + .mockRejectedValueOnce(conditionalError) + .mockResolvedValueOnce({ status: 201, ok: true, statusText: 'Created' }) + const store = { + fetcher: { webOperation }, + holds: vi.fn(() => false) + } + const file = new File(['image'], 'avatar.png', { type: 'image/png' }) + const subject = sym('https://pod.example/profile/card#me') + + await expect(uploadPhotoFile(store as never, subject, file)) + .resolves.toMatch(/^https:\/\/pod\.example\/profile\/image_\d+_0_[a-z0-9]+\.png$/) + + expect(webOperation).toHaveBeenCalledTimes(3) + expect(webOperation.mock.calls.filter(([method]) => method === 'HEAD')).toHaveLength(1) + }) +}) diff --git a/test/unit/navigation/browserEvents.test.ts b/test/unit/navigation/browserEvents.test.ts index cdddb067..0b958c6e 100644 --- a/test/unit/navigation/browserEvents.test.ts +++ b/test/unit/navigation/browserEvents.test.ts @@ -1,5 +1,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { sym } from 'rdflib' import type { NavigationController } from '../../../src/navigation/controller' +import OutlineManager from '../../../src/outline/manager' type Listener = (event: unknown) => void @@ -108,8 +110,6 @@ describe('Enter on the current URL', () => { describe('application history writes in Safari', () => { it('keeps a storage selection when replaceState dispatches a user-initiated navigate event', async () => { const { registerBrowserNavigation } = await freshModule() - const { default: OutlineManager } = await import('../../../src/outline/manager') - const { sym } = await import('rdflib') registerBrowserNavigation(controller) window.history.replaceState({}, '', '/other/profile/card#me') @@ -122,6 +122,7 @@ describe('application history writes in Safari', () => { const subject = sym(window.location.href) const storage = sym(`${window.location.origin}/other/`) const manager = new OutlineManager({ dom: document, session: { paneRegistry: { list: [], byName: () => undefined } } }) + vi.spyOn(manager, 'outlineObjectDiv').mockReturnValue(document.createElement('div')) manager.GotoSubject(subject, false, { name: 'folder', subject: storage }, true)