[Refactor] Rework userFileStore to match existing API on ComfyWorkflow (#1394)

* nit * Move load * nit * nit * Update store API * nit * nit * Move api * nit * Update tests * Add docs * Add temp user file * Implement save as * Test saveAs
2026-03-11 16:10:05 +00:00 · 2024-10-31 21:58:00 -04:00
parent 2974b9257a
commit dfb695be72
3 changed files with 275 additions and 131 deletions
--- a/src/scripts/api.ts
+++ b/src/scripts/api.ts
@@ -602,10 +602,16 @@ class ComfyApi extends EventTarget {
      overwrite?: boolean
      stringify?: boolean
      throwOnError?: boolean
-    } = { overwrite: true, stringify: true, throwOnError: true }
+      full_info?: boolean
+    } = {
+      overwrite: true,
+      stringify: true,
+      throwOnError: true,
+      full_info: false
+    }
  ): Promise<Response> {
    const resp = await this.fetchApi(
-      `/userdata/${encodeURIComponent(file)}?overwrite=${options.overwrite}`,
+      `/userdata/${encodeURIComponent(file)}?overwrite=${options.overwrite}&full_info=${options.full_info}`,
      {
        method: 'POST',
        body: options?.stringify ? JSON.stringify(data) : data,
--- a/src/stores/userFileStore.ts
+++ b/src/stores/userFileStore.ts
@@ -3,17 +3,49 @@ import { api } from '@/scripts/api'
 import { buildTree } from '@/utils/treeUtil'
 import { computed, ref } from 'vue'
 import { TreeExplorerNode } from '@/types/treeExplorerTypes'
+import { UserDataFullInfo } from '@/types/apiTypes'

+/**
+ * Represents a file in the user's data directory.
+ */
 export class UserFile {
+  /**
+   * Various path components.
+   * Example:
+   * - path: 'dir/file.txt'
+   * - directory: 'dir'
+   * - fullFilename: 'file.txt'
+   * - filename: 'file'
+   * - suffix: 'txt'
+   */
+  directory: string
+  fullFilename: string
+  filename: string
+  suffix: string | null
+
  isLoading: boolean = false
  content: string | null = null
  originalContent: string | null = null

  constructor(
+    /**
+     * Path relative to ComfyUI/user/ directory.
+     */
    public path: string,
+    /**
+     * Last modified timestamp.
+     */
    public lastModified: number,
+    /**
+     * File size in bytes.
+     */
    public size: number
-  ) {}
+  ) {
+    this.directory = path.split('/').slice(0, -1).join('/')
+    this.fullFilename = path.split('/').pop() ?? path
+    this.filename = this.fullFilename.split('.').slice(0, -1).join('.')
+    this.suffix = this.fullFilename.split('.').pop() ?? null
+  }

  get isOpen() {
    return !!this.content
@@ -22,6 +54,113 @@ export class UserFile {
  get isModified() {
    return this.content !== this.originalContent
  }
+
+  async load(): Promise<UserFile> {
+    this.isLoading = true
+    const resp = await api.getUserData(this.path)
+    if (resp.status !== 200) {
+      throw new Error(
+        `Failed to load file '${this.path}': ${resp.status} ${resp.statusText}`
+      )
+    }
+    this.content = await resp.text()
+    this.originalContent = this.content
+    this.isLoading = false
+    return this
+  }
+
+  async saveAs(newPath: string): Promise<UserFile> {
+    const tempFile = new TempUserFile(newPath, this.content ?? undefined)
+    await tempFile.save()
+    return tempFile
+  }
+
+  async save(): Promise<UserFile> {
+    if (!this.isModified) return this
+
+    const resp = await api.storeUserData(this.path, this.content, {
+      throwOnError: true,
+      full_info: true
+    })
+
+    // Note: Backend supports full_info=true feature after
+    // https://github.com/comfyanonymous/ComfyUI/pull/5446
+    const updatedFile = (await resp.json()) as string | UserDataFullInfo
+    if (typeof updatedFile === 'object') {
+      this.lastModified = updatedFile.modified
+      this.size = updatedFile.size
+    }
+    this.originalContent = this.content
+    return this
+  }
+
+  async delete(): Promise<void> {
+    const resp = await api.deleteUserData(this.path)
+    if (resp.status !== 204) {
+      throw new Error(
+        `Failed to delete file '${this.path}': ${resp.status} ${resp.statusText}`
+      )
+    }
+  }
+
+  async rename(newPath: string): Promise<UserFile> {
+    const resp = await api.moveUserData(this.path, newPath)
+    if (resp.status !== 200) {
+      throw new Error(
+        `Failed to rename file '${this.path}': ${resp.status} ${resp.statusText}`
+      )
+    }
+    this.path = newPath
+    // Note: Backend supports full_info=true feature after
+    // https://github.com/comfyanonymous/ComfyUI/pull/5446
+    const updatedFile = (await resp.json()) as string | UserDataFullInfo
+    if (typeof updatedFile === 'object') {
+      this.lastModified = updatedFile.modified
+      this.size = updatedFile.size
+    }
+    return this
+  }
+}
+
+export class TempUserFile extends UserFile {
+  constructor(path: string, content: string = '') {
+    // Initialize with current timestamp and 0 size since it's temporary
+    super(path, Date.now(), 0)
+    this.content = content
+    this.originalContent = content
+  }
+
+  // Override methods that interact with backend
+  async load(): Promise<TempUserFile> {
+    // No need to load as it's a temporary file
+    return this
+  }
+
+  async save(): Promise<TempUserFile> {
+    // First save should create the actual file on the backend
+    const resp = await api.storeUserData(this.path, this.content, {
+      throwOnError: true,
+      full_info: true
+    })
+
+    const updatedFile = (await resp.json()) as string | UserDataFullInfo
+    if (typeof updatedFile === 'object') {
+      this.lastModified = updatedFile.modified
+      this.size = updatedFile.size
+    }
+    this.originalContent = this.content
+    return this
+  }
+
+  async delete(): Promise<void> {
+    // No need to delete from backend as it doesn't exist there
+  }
+
+  async rename(newPath: string): Promise<TempUserFile> {
+    // Just update the path locally since it's not on backend yet
+    this.path = newPath
+    return this
+  }
 }

 export const useUserFileStore = defineStore('userFile', () => {
@@ -35,7 +174,7 @@ export const useUserFileStore = defineStore('userFile', () => {
    userFiles.value.filter((file: UserFile) => file.isOpen)
  )

-  const workflowsTree = computed<TreeExplorerNode<UserFile>>(
+  const fileTree = computed<TreeExplorerNode<UserFile>>(
    () =>
      buildTree<UserFile>(userFiles.value, (userFile: UserFile) =>
        userFile.path.split('/')
@@ -46,8 +185,8 @@ export const useUserFileStore = defineStore('userFile', () => {
   * Syncs the files in the given directory with the API.
   * @param dir The directory to sync.
   */
-  const syncFiles = async () => {
-    const files = await api.listUserDataFullInfo('')
+  const syncFiles = async (dir: string = '') => {
+    const files = await api.listUserDataFullInfo(dir)

    for (const file of files) {
      const existingFile = userFilesByPath.value.get(file.path)
@@ -76,63 +215,11 @@ export const useUserFileStore = defineStore('userFile', () => {
    }
  }

-  const loadFile = async (file: UserFile) => {
-    file.isLoading = true
-    const resp = await api.getUserData(file.path)
-    if (resp.status !== 200) {
-      throw new Error(
-        `Failed to load file '${file.path}': ${resp.status} ${resp.statusText}`
-      )
-    }
-    file.content = await resp.text()
-    file.originalContent = file.content
-    file.isLoading = false
-  }
-
-  const saveFile = async (file: UserFile) => {
-    if (file.isModified) {
-      const resp = await api.storeUserData(file.path, file.content)
-      if (resp.status !== 200) {
-        throw new Error(
-          `Failed to save file '${file.path}': ${resp.status} ${resp.statusText}`
-        )
-      }
-    }
-    await syncFiles()
-  }
-
-  const deleteFile = async (file: UserFile) => {
-    const resp = await api.deleteUserData(file.path)
-    if (resp.status !== 204) {
-      throw new Error(
-        `Failed to delete file '${file.path}': ${resp.status} ${resp.statusText}`
-      )
-    }
-    await syncFiles()
-  }
-
-  const renameFile = async (file: UserFile, newPath: string) => {
-    const resp = await api.moveUserData(file.path, newPath)
-    if (resp.status !== 200) {
-      throw new Error(
-        `Failed to rename file '${file.path}': ${resp.status} ${resp.statusText}`
-      )
-    }
-    file.path = newPath
-    userFilesByPath.value.set(newPath, file)
-    userFilesByPath.value.delete(file.path)
-    await syncFiles()
-  }
-
  return {
    userFiles,
    modifiedFiles,
    openedFiles,
-    workflowsTree,
-    syncFiles,
-    loadFile,
-    saveFile,
-    deleteFile,
-    renameFile
+    fileTree,
+    syncFiles
  }
 })
--- a/tests-ui/tests/store/userFileStore.test.ts
+++ b/tests-ui/tests/store/userFileStore.test.ts
@@ -1,5 +1,9 @@
 import { setActivePinia, createPinia } from 'pinia'
-import { UserFile, useUserFileStore } from '@/stores/userFileStore'
+import {
+  TempUserFile,
+  UserFile,
+  useUserFileStore
+} from '@/stores/userFileStore'
 import { api } from '@/scripts/api'

 // Mock the api
@@ -30,26 +34,26 @@ describe('useUserFileStore', () => {
  describe('syncFiles', () => {
    it('should add new files', async () => {
      const mockFiles = [
-        { path: 'file1.txt', modified: 123, size: 100 },
-        { path: 'file2.txt', modified: 456, size: 200 }
+        { path: 'dir/file1.txt', modified: 123, size: 100 },
+        { path: 'dir/file2.txt', modified: 456, size: 200 }
      ]
      ;(api.listUserDataFullInfo as jest.Mock).mockResolvedValue(mockFiles)

-      await store.syncFiles()
+      await store.syncFiles('dir')

      expect(store.userFiles).toHaveLength(2)
-      expect(store.userFiles[0].path).toBe('file1.txt')
-      expect(store.userFiles[1].path).toBe('file2.txt')
+      expect(store.userFiles[0].path).toBe('dir/file1.txt')
+      expect(store.userFiles[1].path).toBe('dir/file2.txt')
    })

    it('should update existing files', async () => {
-      const initialFile = { path: 'file1.txt', modified: 123, size: 100 }
+      const initialFile = { path: 'dir/file1.txt', modified: 123, size: 100 }
      ;(api.listUserDataFullInfo as jest.Mock).mockResolvedValue([initialFile])
-      await store.syncFiles()
+      await store.syncFiles('dir')

-      const updatedFile = { path: 'file1.txt', modified: 456, size: 200 }
+      const updatedFile = { path: 'dir/file1.txt', modified: 456, size: 200 }
      ;(api.listUserDataFullInfo as jest.Mock).mockResolvedValue([updatedFile])
-      await store.syncFiles()
+      await store.syncFiles('dir')

      expect(store.userFiles).toHaveLength(1)
      expect(store.userFiles[0].lastModified).toBe(456)
@@ -58,99 +62,146 @@ describe('useUserFileStore', () => {

    it('should remove non-existent files', async () => {
      const initialFiles = [
-        { path: 'file1.txt', modified: 123, size: 100 },
-        { path: 'file2.txt', modified: 456, size: 200 }
+        { path: 'dir/file1.txt', modified: 123, size: 100 },
+        { path: 'dir/file2.txt', modified: 456, size: 200 }
      ]
      ;(api.listUserDataFullInfo as jest.Mock).mockResolvedValue(initialFiles)
-      await store.syncFiles()
+      await store.syncFiles('dir')

-      const updatedFiles = [{ path: 'file1.txt', modified: 123, size: 100 }]
+      const updatedFiles = [{ path: 'dir/file1.txt', modified: 123, size: 100 }]
      ;(api.listUserDataFullInfo as jest.Mock).mockResolvedValue(updatedFiles)
+      await store.syncFiles('dir')
+
+      expect(store.userFiles).toHaveLength(1)
+      expect(store.userFiles[0].path).toBe('dir/file1.txt')
+    })
+
+    it('should sync root directory when no directory is specified', async () => {
+      const mockFiles = [{ path: 'file1.txt', modified: 123, size: 100 }]
+      ;(api.listUserDataFullInfo as jest.Mock).mockResolvedValue(mockFiles)
+
      await store.syncFiles()

+      expect(api.listUserDataFullInfo).toHaveBeenCalledWith('')
      expect(store.userFiles).toHaveLength(1)
      expect(store.userFiles[0].path).toBe('file1.txt')
    })
  })

-  describe('loadFile', () => {
-    it('should load file content', async () => {
-      const file = new UserFile('file1.txt', 123, 100)
-      ;(api.getUserData as jest.Mock).mockResolvedValue({
-        status: 200,
-        text: () => Promise.resolve('file content')
+  describe('UserFile', () => {
+    describe('load', () => {
+      it('should load file content', async () => {
+        const file = new UserFile('file1.txt', 123, 100)
+        ;(api.getUserData as jest.Mock).mockResolvedValue({
+          status: 200,
+          text: () => Promise.resolve('file content')
+        })
+
+        await file.load()
+
+        expect(file.content).toBe('file content')
+        expect(file.originalContent).toBe('file content')
+        expect(file.isLoading).toBe(false)
      })

-      await store.loadFile(file)
+      it('should throw error on failed load', async () => {
+        const file = new UserFile('file1.txt', 123, 100)
+        ;(api.getUserData as jest.Mock).mockResolvedValue({
+          status: 404,
+          statusText: 'Not Found'
+        })

-      expect(file.content).toBe('file content')
-      expect(file.originalContent).toBe('file content')
-      expect(file.isLoading).toBe(false)
+        await expect(file.load()).rejects.toThrow(
+          "Failed to load file 'file1.txt': 404 Not Found"
+        )
+      })
    })

-    it('should throw error on failed load', async () => {
-      const file = new UserFile('file1.txt', 123, 100)
-      ;(api.getUserData as jest.Mock).mockResolvedValue({
-        status: 404,
-        statusText: 'Not Found'
+    describe('save', () => {
+      it('should save modified file', async () => {
+        const file = new UserFile('file1.txt', 123, 100)
+        file.content = 'modified content'
+        file.originalContent = 'original content'
+        ;(api.storeUserData as jest.Mock).mockResolvedValue({
+          status: 200,
+          json: () => Promise.resolve({ modified: 456, size: 200 })
+        })
+
+        await file.save()
+
+        expect(api.storeUserData).toHaveBeenCalledWith(
+          'file1.txt',
+          'modified content',
+          { throwOnError: true, full_info: true }
+        )
+        expect(file.lastModified).toBe(456)
+        expect(file.size).toBe(200)
      })

-      await expect(store.loadFile(file)).rejects.toThrow(
-        "Failed to load file 'file1.txt': 404 Not Found"
-      )
-    })
-  })
+      it('should not save unmodified file', async () => {
+        const file = new UserFile('file1.txt', 123, 100)
+        file.content = 'content'
+        file.originalContent = 'content'

-  describe('saveFile', () => {
-    it('should save modified file', async () => {
-      const file = new UserFile('file1.txt', 123, 100)
-      file.content = 'modified content'
-      file.originalContent = 'original content'
-      ;(api.storeUserData as jest.Mock).mockResolvedValue({ status: 200 })
-      ;(api.listUserDataFullInfo as jest.Mock).mockResolvedValue([])
+        await file.save()

-      await store.saveFile(file)
-
-      expect(api.storeUserData).toHaveBeenCalledWith(
-        'file1.txt',
-        'modified content'
-      )
+        expect(api.storeUserData).not.toHaveBeenCalled()
+      })
    })

-    it('should not save unmodified file', async () => {
-      const file = new UserFile('file1.txt', 123, 100)
-      file.content = 'content'
-      file.originalContent = 'content'
-      ;(api.listUserDataFullInfo as jest.Mock).mockResolvedValue([])
+    describe('delete', () => {
+      it('should delete file', async () => {
+        const file = new UserFile('file1.txt', 123, 100)
+        ;(api.deleteUserData as jest.Mock).mockResolvedValue({ status: 204 })

-      await store.saveFile(file)
+        await file.delete()

-      expect(api.storeUserData).not.toHaveBeenCalled()
+        expect(api.deleteUserData).toHaveBeenCalledWith('file1.txt')
+      })
    })
-  })

-  describe('deleteFile', () => {
-    it('should delete file', async () => {
-      const file = new UserFile('file1.txt', 123, 100)
-      ;(api.deleteUserData as jest.Mock).mockResolvedValue({ status: 204 })
-      ;(api.listUserDataFullInfo as jest.Mock).mockResolvedValue([])
+    describe('rename', () => {
+      it('should rename file', async () => {
+        const file = new UserFile('file1.txt', 123, 100)
+        ;(api.moveUserData as jest.Mock).mockResolvedValue({
+          status: 200,
+          json: () => Promise.resolve({ modified: 456, size: 200 })
+        })

-      await store.deleteFile(file)
+        await file.rename('newfile.txt')

-      expect(api.deleteUserData).toHaveBeenCalledWith('file1.txt')
+        expect(api.moveUserData).toHaveBeenCalledWith(
+          'file1.txt',
+          'newfile.txt'
+        )
+        expect(file.path).toBe('newfile.txt')
+        expect(file.lastModified).toBe(456)
+        expect(file.size).toBe(200)
+      })
    })
-  })

-  describe('renameFile', () => {
-    it('should rename file', async () => {
-      const file = new UserFile('file1.txt', 123, 100)
-      ;(api.moveUserData as jest.Mock).mockResolvedValue({ status: 200 })
-      ;(api.listUserDataFullInfo as jest.Mock).mockResolvedValue([])
+    describe('saveAs', () => {
+      it('should save file with new path', async () => {
+        const file = new UserFile('file1.txt', 123, 100)
+        file.content = 'file content'
+        ;(api.storeUserData as jest.Mock).mockResolvedValue({
+          status: 200,
+          json: () => Promise.resolve({ modified: 456, size: 200 })
+        })

-      await store.renameFile(file, 'newfile.txt')
+        const newFile = await file.saveAs('newfile.txt')

-      expect(api.moveUserData).toHaveBeenCalledWith('file1.txt', 'newfile.txt')
-      expect(file.path).toBe('newfile.txt')
+        expect(api.storeUserData).toHaveBeenCalledWith(
+          'newfile.txt',
+          'file content',
+          { throwOnError: true, full_info: true }
+        )
+        expect(newFile).toBeInstanceOf(TempUserFile)
+        expect(newFile.path).toBe('newfile.txt')
+        expect(newFile.lastModified).toBe(456)
+        expect(newFile.size).toBe(200)
+        expect(newFile.content).toBe('file content')
+      })
    })
  })
 })