refactor(Project): abort previous saveToCloud before starting a new…

… one

This also improves the auto-save mechanism by making it reactive to the
`saveToCloud` process.

Fixes goplus#796

Signed-off-by: Aofei Sheng <[email protected]>

spx-gui/src/apis/common/client.ts

@@ -11,6 +11,7 @@ export type RequestOptions = {
   headers?: Headers
   /** Timeout duration in milisecond, from request-sent to server-response-got */
   timeout?: number
+  signal?: AbortSignal
 }
 
 /** Response body when exception encountered for API calling */
@@ -74,6 +75,11 @@ export class Client {
     const req = await this.prepareRequest(url, payload, options)
     const timeout = options?.timeout ?? defaultTimeout
     const ctrl = new AbortController()
+    if (options?.signal != null) {
+      // TODO: Reimplement this using `AbortSignal.any()` once it is widely supported.
+      options.signal.throwIfAborted()
+      options.signal.addEventListener('abort', () => ctrl.abort(options.signal?.reason))
+    }
     const resp = await Promise.race([
       fetch(req, { signal: ctrl.signal }),
       new Promise<never>((_, reject) => setTimeout(() => reject(new TimeoutException()), timeout))

spx-gui/src/apis/project.ts

@@ -30,8 +30,8 @@ export type ProjectData = {
 
 export type AddProjectParams = Pick<ProjectData, 'name' | 'isPublic' | 'files'>
 
-export function addProject(params: AddProjectParams) {
-  return client.post('/project', params) as Promise<ProjectData>
+export function addProject(params: AddProjectParams, signal?: AbortSignal) {
+  return client.post('/project', params, { signal }) as Promise<ProjectData>
 }
 
 export type UpdateProjectParams = Pick<ProjectData, 'isPublic' | 'files'>
@@ -40,8 +40,13 @@ function encode(owner: string, name: string) {
   return `${encodeURIComponent(owner)}/${encodeURIComponent(name)}`
 }
 
-export function updateProject(owner: string, name: string, params: UpdateProjectParams) {
-  return client.put(`/project/${encode(owner, name)}`, params) as Promise<ProjectData>
+export function updateProject(
+  owner: string,
+  name: string,
+  params: UpdateProjectParams,
+  signal?: AbortSignal
+) {
+  return client.put(`/project/${encode(owner, name)}`, params, { signal }) as Promise<ProjectData>
 }
 
 export function deleteProject(owner: string, name: string) {

spx-gui/src/models/common/cloud.ts

@@ -24,15 +24,20 @@ export async function load(owner: string, name: string) {
   return await parseProjectData(projectData)
 }
 
-export async function save(metadata: Metadata, files: Files) {
+export async function save(metadata: Metadata, files: Files, signal?: AbortSignal) {
   const { owner, name, id } = metadata
   if (owner == null) throw new Error('owner expected')
   if (!name) throw new DefaultException({ en: 'project name not specified', zh: '未指定项目名' })
-  const { fileCollection } = await saveFiles(files)
+
+  const { fileCollection } = await saveFiles(files, signal)
+  signal?.throwIfAborted()
+
   const isPublic = metadata.isPublic ?? IsPublic.personal
   const projectData = await (id != null
-    ? updateProject(owner, name, { isPublic, files: fileCollection })
-    : addProject({ name, isPublic, files: fileCollection }))
+    ? updateProject(owner, name, { isPublic, files: fileCollection }, signal)
+    : addProject({ name, isPublic, files: fileCollection }, signal))
+  signal?.throwIfAborted()
+
   return { metadata: projectData, files }
 }
 
@@ -42,11 +47,12 @@ export async function parseProjectData({ files: fileCollection, ...metadata }: P
 }
 
 export async function saveFiles(
-  files: Files
+  files: Files,
+  signal?: AbortSignal
 ): Promise<{ fileCollection: FileCollection; fileCollectionHash: string }> {
   const fileCollection = Object.fromEntries(
     await Promise.all(
-      Object.keys(files).map(async (path) => [path, await saveFile(files[path]!)] as const)
+      Object.keys(files).map(async (path) => [path, await saveFile(files[path]!, signal)] as const)
     )
   )
   const fileCollectionHash = await hashFileCollection(fileCollection)
@@ -90,8 +96,8 @@ export function createFileWithWebUrl(url: WebUrl, name = filename(url)) {
   })
 }
 
-export async function saveFileForWebUrl(file: File) {
-  const universalUrl = await saveFile(file)
+export async function saveFileForWebUrl(file: File, signal?: AbortSignal) {
+  const universalUrl = await saveFile(file, signal)
   return universalUrlToWebUrl(universalUrl)
 }
 
@@ -102,11 +108,11 @@ async function universalUrlToWebUrl(universalUrl: UniversalUrl) {
   return map[universalUrl]
 }
 
-async function saveFile(file: File) {
+async function saveFile(file: File, signal?: AbortSignal) {
   const savedUrl = getUniversalUrl(file)
   if (savedUrl != null) return savedUrl
 
-  const url = await (isInlineable(file) ? inlineFile(file) : uploadToKodo(file))
+  const url = await (isInlineable(file) ? inlineFile(file) : uploadToKodo(file, signal))
   setUniversalUrl(file, url)
   return url
 }
@@ -135,7 +141,7 @@ type KodoUploadRes = {
   hash: string
 }
 
-async function uploadToKodo(file: File): Promise<UniversalUrl> {
+async function uploadToKodo(file: File, signal?: AbortSignal): Promise<UniversalUrl> {
   const nativeFile = await toNativeFile(file)
   const { token, maxSize, bucket, region } = await getUpInfo()
   if (nativeFile.size > maxSize) throw new Error(`file size exceeds the limit (${maxSize} bytes)`)
@@ -150,14 +156,22 @@ async function uploadToKodo(file: File): Promise<UniversalUrl> {
     { region: region as any }
   )
   const { key } = await new Promise<KodoUploadRes>((resolve, reject) => {
-    observable.subscribe({
+    if (signal?.aborted) {
+      reject(signal.reason)
+      return
+    }
+    const subscription = observable.subscribe({
       error(e) {
         reject(e)
       },
       complete(res) {
         resolve(res)
       }
     })
+    signal?.addEventListener('abort', () => {
+      subscription.unsubscribe()
+      reject(signal.reason)
+    })
   })
   return `${fileUniversalUrlSchemes.kodo}//${bucket}/${key}`
 }

spx-gui/src/models/project/index.test.ts

@@ -8,6 +8,8 @@ import { fromText, type Files } from '../common/file'
 import { AutoSaveMode, AutoSaveToCloudState, Project } from '.'
 import * as cloudHelper from '../common/cloud'
 import * as localHelper from '../common/local'
+import type { ProjectData } from '@/apis/project'
+import { Cancelled } from '@/utils/exception'
 
 function mockFile(name = 'mocked') {
   return fromText(name, Math.random() + '')
@@ -28,6 +30,15 @@ function makeProject() {
 }
 
 describe('Project', () => {
+  beforeEach(() => {
+    vi.useFakeTimers()
+  })
+
+  afterEach(() => {
+    vi.useRealTimers()
+    vi.restoreAllMocks()
+  })
+
   it('should preserve animation sound with exportGameFiles & loadGameFiles', async () => {
     const project = makeProject()
     const sprite = project.sprites[0]
@@ -139,16 +150,54 @@ describe('Project', () => {
     await expect((project as any).saveToLocalCache('key')).rejects.toThrow('disposed')
     expect(saveToLocalCacheMethod).toHaveBeenCalledWith('key')
   })
-})
 
-describe('ProjectAutoSave', () => {
-  beforeEach(() => {
-    vi.useFakeTimers()
+  it('should abort previous saveToCloud call when a new one is initiated', async () => {
+    const project = makeProject()
+
+    const cloudSaveMock = vi
+      .spyOn(cloudHelper, 'save')
+      .mockImplementation((metadata, files, signal) => {
+        return new Promise((resolve, reject) => {
+          const timeoutId = setTimeout(() => {
+            resolve({ metadata: metadata as ProjectData, files })
+          }, 1000)
+
+          if (signal) {
+            signal.addEventListener('abort', () => {
+              clearTimeout(timeoutId)
+              reject(signal.reason)
+            })
+          }
+        })
+      })
+
+    const firstSavePromise = project.saveToCloud()
+    await vi.advanceTimersByTimeAsync(500)
+
+    const secondSavePromise = project.saveToCloud()
+    vi.runAllTimersAsync()
+
+    await expect(firstSavePromise).rejects.toThrow(Cancelled)
+    await expect(secondSavePromise).resolves.not.toThrow()
+    expect(cloudSaveMock).toHaveBeenCalledTimes(2)
   })
 
-  afterEach(() => {
-    vi.useRealTimers()
-    vi.restoreAllMocks()
+  it('should not abort saveToCloud call if it completes before a new one is initiated', async () => {
+    const project = makeProject()
+
+    const cloudSaveMock = vi.spyOn(cloudHelper, 'save').mockImplementation((metadata, files) => {
+      return new Promise((resolve) => {
+        resolve({ metadata: metadata as ProjectData, files })
+      })
+    })
+
+    const firstSavePromise = project.saveToCloud()
+    await expect(firstSavePromise).resolves.not.toThrow()
+
+    const secondSavePromise = project.saveToCloud()
+    await expect(secondSavePromise).resolves.not.toThrow()
+
+    expect(cloudSaveMock).toHaveBeenCalledTimes(2)
   })
 
   // https://github.com/goplus/builder/pull/794#discussion_r1728120369

spx-gui/src/models/project/index.ts

@@ -22,6 +22,8 @@ import { Sound } from '../sound'
 import type { RawWidgetConfig } from '../widget'
 import { History } from './history'
 import Mutex from '@/utils/mutex'
+import { Cancelled } from '@/utils/exception'
+import { untilConditionMet } from '@/utils/utils'
 
 export type { Action } from './history'
 
@@ -383,12 +385,28 @@ export class Project extends Disposable {
   }
 
   /** Save to cloud */
+  private saveToCloudAbortController: AbortController | null = null
+  private get isSavingToCloud() {
+    return this.saveToCloudAbortController != null
+  }
   async saveToCloud() {
-    const [metadata, files] = await this.export()
-    if (this.isDisposed) throw new Error('disposed')
-    const saved = await cloudHelper.save(metadata, files)
-    this.loadMetadata(saved.metadata)
-    this.lastSyncedFilesHash = await hashFiles(files)
+    if (this.saveToCloudAbortController != null) {
+      this.saveToCloudAbortController.abort(new Cancelled('aborted'))
+    }
+    const abortController = new AbortController()
+    this.saveToCloudAbortController = abortController
+
+    try {
+      const [metadata, files] = await this.export()
+      if (this.isDisposed) throw new Error('disposed')
+      const saved = await cloudHelper.save(metadata, files, abortController.signal)
+      this.loadMetadata(saved.metadata)
+      this.lastSyncedFilesHash = await hashFiles(files)
+    } finally {
+      if (this.saveToCloudAbortController === abortController) {
+        this.saveToCloudAbortController = null
+      }
+    }
   }
 
   /** Load from local cache */
@@ -428,13 +446,25 @@ export class Project extends Disposable {
         this.autoSaveToCloudState = AutoSaveToCloudState.Saving
 
         try {
+          if (this.isSavingToCloud) {
+            await untilConditionMet(
+              () => this.isSavingToCloud,
+              () => !this.isSavingToCloud
+            )
+          }
+
           if (this.hasUnsyncedChanges) await this.saveToCloud()
           this.autoSaveToCloudState = AutoSaveToCloudState.Saved
         } catch (e) {
           this.autoSaveToCloudState = AutoSaveToCloudState.Failed
-          retryAutoSave()
-          await this.saveToLocalCache(localCacheKey) // prevent data loss
-          console.error('failed to auto save to cloud', e)
+          if (e instanceof Cancelled) {
+            autoSaveToCloud()
+            save.flush()
+          } else {
+            retryAutoSave()
+            await this.saveToLocalCache(localCacheKey) // prevent data loss
+            console.error('failed to auto save to cloud', e)
+          }
           return
         }
 
@@ -454,6 +484,20 @@ export class Project extends Disposable {
       }, 5000)
       this.addDisposer(retryAutoSave.cancel)
 
+      // fire pending or retryable auto saves immediately when a new save occurs, making autoSaveToCloudState more responsive
+      this.addDisposer(
+        watch(
+          () => this.isSavingToCloud,
+          async () => {
+            if (this.isSavingToCloud) {
+              await retryAutoSave.flush()
+              save.flush()
+            }
+          },
+          { immediate: true }
+        )
+      )
+
       return () => {
         retryAutoSave.cancel()
         if (this.autoSaveToCloudState !== AutoSaveToCloudState.Saving)

spx-gui/src/utils/utils.ts

@@ -84,12 +84,29 @@ export function useLocalStorage<T>(key: string, initialValue: T) {
  * ```
  */
 export function untilNotNull<T>(valueSource: WatchSource<T | null | undefined>) {
+  return untilConditionMet(
+    valueSource as WatchSource<T | null | undefined>,
+    (value): value is NonNullable<T> => value != null
+  ) as Promise<NonNullable<T>>
+}
+
+/**
+ * Wait until a given condition is met for a (reactive) value.
+ * ```ts
+ * const foo = await untilConditionMet(fooRef, (value) => value !== null)
+ * const bar = await untilConditionMet(() => getBar(), (value) => value > 10)
+ * ```
+ */
+export function untilConditionMet<T>(
+  valueSource: WatchSource<T>,
+  condition: (value: T) => boolean
+): Promise<T> {
   return new Promise<T>((resolve) => {
     let stopWatch: (() => void) | null = null
     stopWatch = watch(
       valueSource,
       (value) => {
-        if (value == null) return
+        if (!condition(value)) return
         resolve(value)
         stopWatch?.()
       },