Fix/image upload

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<string | undefined> {
   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()

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)
+  })
+})

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)