fix: loading user avatars won't get cancelled when changing route

packages/web-pkg/src/components/Avatars/UserAvatar.vue

@@ -3,7 +3,7 @@
 </template>
 
 <script setup lang="ts">
-import { computed, onMounted, unref } from 'vue'
+import { computed, onMounted, onUnmounted, unref } from 'vue'
 import { useAvatarsStore, useLoadAvatars } from '../../composables'
 import { storeToRefs } from 'pinia'
 
@@ -19,7 +19,7 @@ const {
 
 const avatarsStore = useAvatarsStore()
 const { avatarMap } = storeToRefs(avatarsStore)
-const { enqueueAvatar } = useLoadAvatars()
+const { enqueueAvatar, cancelAvatar } = useLoadAvatars()
 
 const avatarSrc = computed(() => {
   return unref(avatarMap)[userId]
@@ -28,4 +28,8 @@ const avatarSrc = computed(() => {
 onMounted(() => {
   enqueueAvatar(userId)
 })
+
+onUnmounted(() => {
+  cancelAvatar(userId)
+})
 </script>

packages/web-pkg/src/composables/avatars/useLoadAvatars.ts

@@ -3,19 +3,26 @@ import { useAvatarsStore } from '../piniaStores'
 
 export const useLoadAvatars = () => {
   const clientService = useClientService()
-  const { addAvatar, getAvatar, avatarsQueue, pendingAvatarsRequests } = useAvatarsStore()
+  const { addAvatar, getAvatar, avatarsQueue, pendingAvatarsRequests, abortControllers } =
+    useAvatarsStore()
 
-  const loadAvatar = async (userId: string) => {
+  const loadAvatar = async (userId: string, signal: AbortSignal) => {
     try {
       const avatar = await clientService.graphAuthenticated.photos.getUserPhoto(userId, {
-        responseType: 'blob'
+        responseType: 'blob',
+        signal
       })
       addAvatar(userId, URL.createObjectURL(avatar))
     } catch (error) {
+      if (error.name === 'AbortError' || error.name === 'CanceledError') {
+        return
+      }
       if (error.response?.status === 404) {
         // If the avatar is not found, we can set it to null, otherwise it will be fetched again
         addAvatar(userId, null)
       }
+    } finally {
+      abortControllers.delete(userId)
     }
 
     return getAvatar(userId)
@@ -27,12 +34,25 @@ export const useLoadAvatars = () => {
       return
     }
 
-    const loadAvatarPromise = avatarsQueue.add(() => loadAvatar(userId))
+    const abortController = new AbortController()
+    abortControllers.set(userId, abortController)
+
+    const loadAvatarPromise = avatarsQueue.add(() => loadAvatar(userId, abortController.signal))
     pendingAvatarsRequests.set(userId, loadAvatarPromise)
     loadAvatarPromise.finally(() => pendingAvatarsRequests.delete(userId))
   }
 
+  const cancelAvatar = (userId: string) => {
+    const abortController = abortControllers.get(userId)
+    if (abortController) {
+      abortController.abort()
+      abortControllers.delete(userId)
+    }
+    pendingAvatarsRequests.delete(userId)
+  }
+
   return {
-    enqueueAvatar
+    enqueueAvatar,
+    cancelAvatar
   }
 }

packages/web-pkg/src/composables/piniaStores/avatars.ts

@@ -11,6 +11,7 @@ export const useAvatarsStore = defineStore('avatars', () => {
     new PQueue({ concurrency: configStore.options.concurrentRequests.avatars })
   )
   const pendingAvatarsRequests = new Map<string, Promise<any>>()
+  const abortControllers = new Map<string, AbortController>()
 
   const addAvatar = (userId: string, avatar: string) => {
     avatarMap.value[userId] = avatar
@@ -35,7 +36,8 @@ export const useAvatarsStore = defineStore('avatars', () => {
     removeAvatar,
     reset,
     avatarsQueue,
-    pendingAvatarsRequests
+    pendingAvatarsRequests,
+    abortControllers
   }
 })
 

packages/web-pkg/tests/unit/composables/avatars/useLoadAvatars.spec.ts

@@ -0,0 +1,210 @@
+import { describe, it, expect, vi, beforeEach } from 'vitest'
+import { useLoadAvatars } from '../../../../src/composables/avatars/useLoadAvatars'
+import { useAvatarsStore } from '../../../../src/composables/piniaStores'
+import { useClientService } from '../../../../src/composables/clientService'
+import { createTestingPinia, getComposableWrapper } from '@opencloud-eu/web-test-helpers'
+
+vi.mock('../../../../src/composables/clientService')
+
+describe('useLoadAvatars', () => {
+  let mockGetUserPhoto: ReturnType<typeof vi.fn>
+
+  beforeEach(() => {
+    createTestingPinia({ stubActions: false })
+
+    mockGetUserPhoto = vi.fn()
+    vi.mocked(useClientService).mockReturnValue({
+      graphAuthenticated: {
+        photos: {
+          getUserPhoto: mockGetUserPhoto
+        }
+      }
+    } as any)
+  })
+
+  describe('enqueueAvatar', () => {
+    it('should enqueue an avatar request', () => {
+      getWrapper({
+        setup: ({ enqueueAvatar }) => {
+          const avatarsStore = useAvatarsStore()
+          const userId = 'user123'
+
+          enqueueAvatar(userId)
+
+          expect(avatarsStore.abortControllers.has(userId)).toBe(true)
+          expect(avatarsStore.pendingAvatarsRequests.has(userId)).toBe(true)
+        }
+      })
+    })
+
+    it('should not enqueue duplicate requests', () => {
+      getWrapper({
+        setup: ({ enqueueAvatar }) => {
+          const avatarsStore = useAvatarsStore()
+          const userId = 'user123'
+
+          enqueueAvatar(userId)
+          const firstController = avatarsStore.abortControllers.get(userId)
+
+          enqueueAvatar(userId)
+          const secondController = avatarsStore.abortControllers.get(userId)
+
+          expect(firstController).toBe(secondController)
+        }
+      })
+    })
+
+    it('should not enqueue if avatar already exists', () => {
+      getWrapper({
+        setup: ({ enqueueAvatar }) => {
+          const avatarsStore = useAvatarsStore()
+          const userId = 'user123'
+
+          avatarsStore.addAvatar(userId, 'blob:http://example.com/avatar')
+          enqueueAvatar(userId)
+
+          expect(avatarsStore.abortControllers.has(userId)).toBe(false)
+        }
+      })
+    })
+
+    it('should load avatar and add to store', async () => {
+      const mockBlob = new Blob(['avatar'], { type: 'image/png' })
+      mockGetUserPhoto.mockResolvedValue(mockBlob)
+      URL.createObjectURL = vi.fn(() => 'blob:http://example.com/avatar')
+
+      await getWrapper({
+        setup: async ({ enqueueAvatar }) => {
+          const avatarsStore = useAvatarsStore()
+          const userId = 'user123'
+
+          enqueueAvatar(userId)
+          await avatarsStore.pendingAvatarsRequests.get(userId)
+
+          expect(avatarsStore.getAvatar(userId)).toBe('blob:http://example.com/avatar')
+          expect(mockGetUserPhoto).toHaveBeenCalledWith(userId, {
+            responseType: 'blob',
+            signal: expect.any(AbortSignal)
+          })
+        }
+      })
+    })
+
+    it('should handle 404 errors and set avatar to null', async () => {
+      mockGetUserPhoto.mockRejectedValue({ response: { status: 404 } })
+
+      await getWrapper({
+        setup: async ({ enqueueAvatar }) => {
+          const avatarsStore = useAvatarsStore()
+          const userId = 'user123'
+
+          enqueueAvatar(userId)
+          await avatarsStore.pendingAvatarsRequests.get(userId)
+
+          expect(avatarsStore.getAvatar(userId)).toBe(null)
+        }
+      })
+    })
+
+    it('should clean up abort controller after request completes', async () => {
+      mockGetUserPhoto.mockResolvedValue(new Blob())
+
+      await getWrapper({
+        setup: async ({ enqueueAvatar }) => {
+          const avatarsStore = useAvatarsStore()
+          const userId = 'user123'
+
+          enqueueAvatar(userId)
+          await avatarsStore.pendingAvatarsRequests.get(userId)
+
+          expect(avatarsStore.abortControllers.has(userId)).toBe(false)
+        }
+      })
+    })
+  })
+
+  describe('cancelAvatar', () => {
+    it('should abort pending request', () => {
+      getWrapper({
+        setup: ({ enqueueAvatar, cancelAvatar }) => {
+          const avatarsStore = useAvatarsStore()
+          const userId = 'user123'
+
+          enqueueAvatar(userId)
+          const abortController = avatarsStore.abortControllers.get(userId)
+          const abortSpy = vi.spyOn(abortController!, 'abort')
+
+          cancelAvatar(userId)
+
+          expect(abortSpy).toHaveBeenCalled()
+          expect(avatarsStore.abortControllers.has(userId)).toBe(false)
+          expect(avatarsStore.pendingAvatarsRequests.has(userId)).toBe(false)
+        }
+      })
+    })
+
+    it('should handle canceling non-existent request', () => {
+      getWrapper({
+        setup: ({ cancelAvatar }) => {
+          expect(() => cancelAvatar('nonexistent')).not.toThrow()
+        }
+      })
+    })
+
+    it('should not update avatar when request is aborted', async () => {
+      mockGetUserPhoto.mockImplementation(
+        () => new Promise((_, reject) => setTimeout(() => reject({ name: 'AbortError' }), 100))
+      )
+
+      await getWrapper({
+        setup: async ({ enqueueAvatar, cancelAvatar }) => {
+          const avatarsStore = useAvatarsStore()
+          const userId = 'user123'
+
+          enqueueAvatar(userId)
+          cancelAvatar(userId)
+
+          await new Promise((resolve) => setTimeout(resolve, 150))
+
+          expect(avatarsStore.getAvatar(userId)).toBeUndefined()
+        }
+      })
+    })
+
+    it('should handle CanceledError from axios', async () => {
+      mockGetUserPhoto.mockImplementation(
+        () => new Promise((_, reject) => setTimeout(() => reject({ name: 'CanceledError' }), 100))
+      )
+
+      await getWrapper({
+        setup: async ({ enqueueAvatar, cancelAvatar }) => {
+          const avatarsStore = useAvatarsStore()
+          const userId = 'user123'
+
+          enqueueAvatar(userId)
+          cancelAvatar(userId)
+
+          await new Promise((resolve) => setTimeout(resolve, 150))
+
+          expect(avatarsStore.getAvatar(userId)).toBeUndefined()
+        }
+      })
+    })
+  })
+})
+
+function getWrapper({
+  setup
+}: {
+  setup: (instance: ReturnType<typeof useLoadAvatars>) => void | Promise<void>
+}) {
+  return {
+    wrapper: getComposableWrapper(
+      () => {
+        const instance = useLoadAvatars()
+        setup(instance)
+      },
+      { pluginOptions: { pinia: false } }
+    )
+  }
+}