fix(Project): prevent potential data corruption in saveToCloud and …

…`saveToLocalCache`

Fixes goplus#761

spx-gui/src/components/project/ProjectCreateModal.vue

@@ -99,7 +99,7 @@ const handleSubmit = useMessageHandle(
  project.addSprite(sprite)
  await sprite.autoFit()
  // upload project content & call API addProject, TODO: maybe this should be extracted to `@/models`?
- const files = project.export()[1]
+ const [, files] = await project.export()
  const { fileCollection } = await saveFiles(files)
  const projectData = await addProject({
  name: form.value.name,

spx-gui/src/models/animation.test.ts

@@ -85,7 +85,7 @@ describe('Animation', () => {
  const project = makeProject()
  project.sprites[0].animations[0].setSound(project.sounds[0].name)
 
- const [metadata, files] = project.export()
+ const [metadata, files] = await project.export()
  const delayedFiles: Files = Object.fromEntries(
  Object.entries(files).map(([path, file]) => [path, delayFile(file!, 50)])
  )

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

@@ -5,7 +5,6 @@
 
 import { shallowReactive } from 'vue'
 import type { LocaleMessage } from '@/utils/i18n'
-import Mutex from '@/utils/mutex'
 import type { Files } from '../common/file'
 import type { Project } from '.'
 
@@ -27,8 +26,6 @@ export type State = {
 }
 
 export class History {
- private mutex = new Mutex()
-
  constructor(
  private project: Project,
  /**
@@ -71,7 +68,7 @@ export class History {
  }
 
  redo() {
- return this.mutex.runExclusive(() => this.goto(this.index + 1))
+ return this.project.historyMutex.runExclusive(() => this.goto(this.index + 1))
  }
 
  getUndoAction() {
@@ -80,11 +77,11 @@ export class History {
  }
 
  undo() {
- return this.mutex.runExclusive(() => this.goto(this.index - 1))
+ return this.project.historyMutex.runExclusive(() => this.goto(this.index - 1))
  }
 
  doAction<T>(action: Action, fn: () => T | Promise<T>): Promise<T> {
- return this.mutex.runExclusive(async () => {
+ return this.project.historyMutex.runExclusive(async () => {
  // history after current state (for redo) will be discarded on any action
  this.states.splice(this.index)
 

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

@@ -1,4 +1,4 @@
-import { describe, it, expect } from 'vitest'
+import { describe, it, expect, vi } from 'vitest'
 import { Sprite } from '../sprite'
 import { Animation } from '../animation'
 import { Sound } from '../sound'
@@ -124,4 +124,16 @@ describe('Project', () => {
  project.removeSprite('Sprite2')
  expect(project.selected).toBeNull()
  })
+
+ it('should throw an error when saving a disposed project', async () => {
+ const project = makeProject()
+ const saveToLocalCacheMethod = vi.spyOn(project, 'saveToLocalCache' as any)
+
+ project.dispose()
+
+ await expect(project.saveToCloud()).rejects.toThrow('disposed')
+
+ await expect((project as any).saveToLocalCache('key')).rejects.toThrow('disposed')
+ expect(saveToLocalCacheMethod).toHaveBeenCalledWith('key')
+ })
 })

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

@@ -21,6 +21,7 @@ import { Sprite } from '../sprite'
 import { Sound } from '../sound'
 import type { RawWidgetConfig } from '../widget'
 import { History } from './history'
+import Mutex from '@/utils/mutex'
 
 export type { Action } from './history'
 
@@ -251,6 +252,7 @@ export class Project extends Disposable {
  }
 
  history: History
+ historyMutex = new Mutex()
 
  constructor() {
  super()
@@ -267,10 +269,24 @@ export class Project extends Disposable {
  return reactiveThis
  }
 
- private applyMetadata(metadata: Metadata) {
+ private loadMetadata(metadata: Metadata) {
  assign<Project>(this, metadata)
  }
 
+ private exportMetadata(): Metadata {
+ return {
+ id: this.id,
+ owner: this.owner,
+ name: this.name,
+ isPublic: this.isPublic,
+ version: this.version,
+ cTime: this.cTime,
+ uTime: this.uTime,
+ filesHash: this.filesHash,
+ lastSyncedFilesHash: this.lastSyncedFilesHash
+ }
+ }
+
  async loadGameFiles(files: Files) {
  const configFile = files[projectConfigFilePath]
  const config: RawProjectConfig = {}
@@ -330,25 +346,13 @@ export class Project extends Disposable {
 
  /** Load with metadata & game files */
  async load(metadata: Metadata, files: Files) {
- this.applyMetadata(metadata)
+ this.loadMetadata(metadata)
  await this.loadGameFiles(files)
  }
 
  /** Export metadata & game files */
- export(): [Metadata, Files] {
- const metadata: Metadata = {
- id: this.id,
- owner: this.owner,
- name: this.name,
- isPublic: this.isPublic,
- version: this.version,
- cTime: this.cTime,
- uTime: this.uTime,
- filesHash: this.filesHash,
- lastSyncedFilesHash: this.lastSyncedFilesHash
- }
- const files = this.exportGameFiles()
- return [metadata, files]
+ async export(): Promise<[Metadata, Files]> {
+ return this.historyMutex.runExclusive(() => [this.exportMetadata(), this.exportGameFiles()])
  }
 
  async loadGbpFile(file: globalThis.File) {
@@ -363,7 +367,7 @@ export class Project extends Disposable {
  }
 
  async exportGbpFile() {
- const [metadata, files] = this.export()
+ const [metadata, files] = await this.export()
  return await gbpHelper.save(metadata, files)
  }
 
@@ -380,9 +384,10 @@ export class Project extends Disposable {
 
  /** Save to cloud */
  async saveToCloud() {
- const [metadata, files] = this.export()
+ const [metadata, files] = await this.export()
+ if (this.isDisposed) throw new Error('disposed')
  const saved = await cloudHelper.save(metadata, files)
- this.applyMetadata(saved.metadata)
+ this.loadMetadata(saved.metadata)
  this.lastSyncedFilesHash = await hashFiles(files)
  }
 
@@ -396,7 +401,8 @@ export class Project extends Disposable {
 
  /** Save to local cache */
  private async saveToLocalCache(key: string) {
- const [metadata, files] = this.export()
+ const [metadata, files] = await this.export()
+ if (this.isDisposed) throw new Error('disposed')
  await localHelper.save(key, metadata, files)
  }
 
@@ -486,7 +492,11 @@ export class Project extends Disposable {
  else delazyLoadGameFiles()
  }
  })()
- this.addDisposer(watch(() => this.export(), autoSaveToLocalCache, { immediate: true }))
+ this.addDisposer(
+ watch(() => [this.exportMetadata(), this.exportGameFiles()], autoSaveToLocalCache, {
+ immediate: true
+ })
+ )
 
  // watch for autoSaveMode switch, and trigger auto save accordingly
  this.addDisposer(

spx-gui/src/utils/disposable.ts

@@ -6,15 +6,23 @@
 export type Disposer = () => void
 
 export class Disposable {
- _disposers: Disposer[] = []
+ private disposers: Disposer[] = []
+
+ private _isDisposed = false
+ get isDisposed() {
+ return this._isDisposed
+ }
 
  addDisposer = (disposer: Disposer) => {
- this._disposers.push(disposer)
+ if (this._isDisposed) throw new Error('disposed')
+ this.disposers.push(disposer)
  }
 
  dispose = () => {
- while (this._disposers.length > 0) {
- this._disposers.pop()?.()
+ if (this._isDisposed) return
+ this._isDisposed = true
+ while (this.disposers.length > 0) {
+ this.disposers.pop()?.()
  }
  }
 }

spx-gui/src/utils/exception.ts

@@ -30,7 +30,7 @@ export class DefaultException extends Exception {
 }
 
 /**
- * Cancelled is a special exception, it stands for a "cancel operation" because of user ineraction.
+ * Cancelled is a special exception, it stands for a "cancel operation" because of user interaction.
  * Like other exceptions, it breaks normal flows, while it is supposed to be ignored by all user-feedback components,
  * so the user will not be notified of cancelled exceptions.
  */