Rework userFileStore (#815)

* Rework userFileStore

* nit

* Add back unittests

src/scripts/api.ts

@@ -11,7 +11,8 @@ import {
   PromptResponse,
   SystemStats,
   User,
-  Settings
+  Settings,
+  UserDataFullInfo
 } from '@/types/apiTypes'
 import axios from 'axios'
 
@@ -671,6 +672,19 @@ class ComfyApi extends EventTarget {
     return resp.json()
   }
 
+  async listUserDataFullInfo(dir: string): Promise<UserDataFullInfo[]> {
+    const resp = await this.fetchApi(
+      `/userdata?dir=${encodeURIComponent(dir)}&recurse=true&split=false&full_info=true`
+    )
+    if (resp.status === 404) return []
+    if (resp.status !== 200) {
+      throw new Error(
+        `Error getting user data list '${dir}': ${resp.status} ${resp.statusText}`
+      )
+    }
+    return resp.json()
+  }
+
   async getLogs(): Promise<string> {
     return (await axios.get(this.internalURL('/logs'))).data
   }

src/stores/userFileStore.ts

@@ -1,173 +1,138 @@
 import { defineStore } from 'pinia'
 import { api } from '@/scripts/api'
-import type { TreeNode } from 'primevue/treenode'
 import { buildTree } from '@/utils/treeUtil'
+import { computed, ref } from 'vue'
+import { TreeExplorerNode } from '@/types/treeExplorerTypes'
 
-interface OpenFile {
-  path: string
-  content: string
-  isModified: boolean
-  originalContent: string
+export class UserFile {
+  isLoading: boolean = false
+  content: string | null = null
+  originalContent: string | null = null
+
+  constructor(
+    public path: string,
+    public lastModified: number,
+    public size: number
+  ) {}
+
+  get isOpen() {
+    return !!this.content
+  }
+
+  get isModified() {
+    return this.content !== this.originalContent
+  }
 }
 
-interface StoreState {
-  files: string[]
-  openFiles: OpenFile[]
-}
+export const useUserFileStore = defineStore('userFile', () => {
+  const userFilesByPath = ref(new Map<string, UserFile>())
 
-interface ApiResponse<T = any> {
-  success: boolean
-  data?: T
-  message?: string
-}
+  const userFiles = computed(() => Array.from(userFilesByPath.value.values()))
+  const modifiedFiles = computed(() =>
+    userFiles.value.filter((file: UserFile) => file.isModified)
+  )
+  const openedFiles = computed(() =>
+    userFiles.value.filter((file: UserFile) => file.isOpen)
+  )
 
-export const useUserFileStore = defineStore('userFile', {
-  state: (): StoreState => ({
-    files: [],
-    openFiles: []
-  }),
+  const workflowsTree = computed<TreeExplorerNode<UserFile>>(
+    () =>
+      buildTree<UserFile>(userFiles.value, (userFile: UserFile) =>
+        userFile.path.split('/')
+      ) as TreeExplorerNode<UserFile>
+  )
 
-  getters: {
-    getOpenFile: (state) => (path: string) =>
-      state.openFiles.find((file) => file.path === path),
-    modifiedFiles: (state) => state.openFiles.filter((file) => file.isModified),
-    workflowsTree: (state): TreeNode =>
-      buildTree(state.files, (path: string) => path.split('/'))
-  },
+  /**
+   * Syncs the files in the given directory with the API.
+   * @param dir The directory to sync.
+   */
+  const syncFiles = async () => {
+    const files = await api.listUserDataFullInfo('')
 
-  actions: {
-    async openFile(path: string): Promise<void> {
-      if (this.getOpenFile(path)) return
+    for (const file of files) {
+      const existingFile = userFilesByPath.value.get(file.path)
 
-      const { success, data } = await this.getFileData(path)
-      if (success && data) {
-        this.openFiles.push({
-          path,
-          content: data,
-          isModified: false,
-          originalContent: data
-        })
-      }
-    },
-
-    closeFile(path: string): void {
-      const index = this.openFiles.findIndex((file) => file.path === path)
-      if (index !== -1) {
-        this.openFiles.splice(index, 1)
-      }
-    },
-
-    updateFileContent(path: string, newContent: string): void {
-      const file = this.getOpenFile(path)
-      if (file) {
-        file.content = newContent
-        file.isModified = file.content !== file.originalContent
-      }
-    },
-
-    async saveOpenFile(path: string): Promise<ApiResponse> {
-      const file = this.getOpenFile(path)
-      if (file?.isModified) {
-        const result = await this.saveFile(path, file.content)
-        if (result.success) {
-          file.isModified = false
-          file.originalContent = file.content
-        }
-        return result
-      }
-      return { success: true }
-    },
-
-    discardChanges(path: string): void {
-      const file = this.getOpenFile(path)
-      if (file) {
-        file.content = file.originalContent
-        file.isModified = false
-      }
-    },
-
-    async loadFiles(dir: string = './'): Promise<void> {
-      this.files = (await api.listUserData(dir, true, false)).map(
-        (filePath: string) => filePath.replaceAll('\\', '/')
-      )
-
-      this.openFiles = (
-        await Promise.all(
-          this.openFiles.map(async (openFile) => {
-            if (!this.files.includes(openFile.path)) return null
-
-            const { success, data } = await this.getFileData(openFile.path)
-            if (success && data !== openFile.originalContent) {
-              return {
-                ...openFile,
-                content: data,
-                originalContent: data,
-                isModified: openFile.content !== data
-              }
-            }
-
-            return openFile
-          })
+      if (!existingFile) {
+        // New file, add it to the map
+        userFilesByPath.value.set(
+          file.path,
+          new UserFile(file.path, file.modified, file.size)
         )
-      ).filter((file): file is OpenFile => file !== null)
-    },
-
-    async renameFile(oldPath: string, newPath: string): Promise<ApiResponse> {
-      const resp = await api.moveUserData(oldPath, newPath)
-      if (resp.status !== 200) {
-        return { success: false, message: resp.statusText }
+      } else if (existingFile.lastModified !== file.modified) {
+        // File has been modified, update its properties
+        existingFile.lastModified = file.modified
+        existingFile.size = file.size
+        existingFile.originalContent = null
+        existingFile.content = null
+        existingFile.isLoading = false
       }
+    }
 
-      const openFile = this.openFiles.find((file) => file.path === oldPath)
-      if (openFile) {
-        openFile.path = newPath
+    // Remove files that no longer exist
+    for (const [path, _] of userFilesByPath.value) {
+      if (!files.some((file) => file.path === path)) {
+        userFilesByPath.value.delete(path)
       }
-
-      await this.loadFiles()
-      return { success: true }
-    },
-
-    async deleteFile(path: string): Promise<ApiResponse> {
-      const resp = await api.deleteUserData(path)
-      if (resp.status !== 204) {
-        return {
-          success: false,
-          message: `Error removing user data file '${path}': ${resp.status} ${resp.statusText}`
-        }
-      }
-
-      const index = this.openFiles.findIndex((file) => file.path === path)
-      if (index !== -1) {
-        this.openFiles.splice(index, 1)
-      }
-
-      await this.loadFiles()
-      return { success: true }
-    },
-
-    async saveFile(path: string, data: string): Promise<ApiResponse> {
-      const resp = await api.storeUserData(path, data, {
-        stringify: false,
-        throwOnError: false,
-        overwrite: true
-      })
-      if (resp.status !== 200) {
-        return {
-          success: false,
-          message: `Error saving user data file '${path}': ${resp.status} ${resp.statusText}`
-        }
-      }
-
-      await this.loadFiles()
-      return { success: true }
-    },
-
-    async getFileData(path: string): Promise<ApiResponse<string>> {
-      const resp = await api.getUserData(path)
-      if (resp.status !== 200) {
-        return { success: false, message: resp.statusText }
-      }
-      return { success: true, data: await resp.json() }
     }
   }
+
+  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
+  }
 })

src/types/apiTypes.ts

@@ -415,7 +415,11 @@ const zUser = z.object({
   users: z.record(z.string(), z.unknown())
 })
 const zUserData = z.array(z.array(z.string(), z.string()))
-
+const zUserDataFullInfo = z.object({
+  path: z.string(),
+  size: z.number(),
+  modified: z.number()
+})
 const zBookmarkCustomization = z.object({
   icon: z.string().optional(),
   color: z.string().optional()
@@ -506,3 +510,4 @@ export type DeviceStats = z.infer<typeof zDeviceStats>
 export type SystemStats = z.infer<typeof zSystemStats>
 export type User = z.infer<typeof zUser>
 export type UserData = z.infer<typeof zUserData>
+export type UserDataFullInfo = z.infer<typeof zUserDataFullInfo>

tests-ui/tests/store/userFileStore.test.ts

@@ -1,15 +1,15 @@
 import { setActivePinia, createPinia } from 'pinia'
-import { useUserFileStore } from '@/stores/userFileStore'
+import { UserFile, useUserFileStore } from '@/stores/userFileStore'
 import { api } from '@/scripts/api'
 
 // Mock the api
 jest.mock('@/scripts/api', () => ({
   api: {
-    listUserData: jest.fn(),
-    moveUserData: jest.fn(),
-    deleteUserData: jest.fn(),
+    listUserDataFullInfo: jest.fn(),
+    getUserData: jest.fn(),
     storeUserData: jest.fn(),
-    getUserData: jest.fn()
+    deleteUserData: jest.fn(),
+    moveUserData: jest.fn()
   }
 }))
 
@@ -21,149 +21,136 @@ describe('useUserFileStore', () => {
     store = useUserFileStore()
   })
 
-  it('should open a file', async () => {
-    const mockFileData = { success: true, data: 'file content' }
-    ;(api.getUserData as jest.Mock).mockResolvedValue({
-      status: 200,
-      json: () => mockFileData.data
+  it('should initialize with empty files', () => {
+    expect(store.userFiles).toHaveLength(0)
+    expect(store.modifiedFiles).toHaveLength(0)
+    expect(store.openedFiles).toHaveLength(0)
+  })
+
+  describe('syncFiles', () => {
+    it('should add new files', async () => {
+      const mockFiles = [
+        { path: 'file1.txt', modified: 123, size: 100 },
+        { path: 'file2.txt', modified: 456, size: 200 }
+      ]
+      ;(api.listUserDataFullInfo as jest.Mock).mockResolvedValue(mockFiles)
+
+      await store.syncFiles()
+
+      expect(store.userFiles).toHaveLength(2)
+      expect(store.userFiles[0].path).toBe('file1.txt')
+      expect(store.userFiles[1].path).toBe('file2.txt')
     })
 
-    await store.openFile('test.txt')
+    it('should update existing files', async () => {
+      const initialFile = { path: 'file1.txt', modified: 123, size: 100 }
+      ;(api.listUserDataFullInfo as jest.Mock).mockResolvedValue([initialFile])
+      await store.syncFiles()
 
-    expect(store.openFiles).toHaveLength(1)
-    expect(store.openFiles[0]).toEqual({
-      path: 'test.txt',
-      content: 'file content',
-      isModified: false,
-      originalContent: 'file content'
+      const updatedFile = { path: 'file1.txt', modified: 456, size: 200 }
+      ;(api.listUserDataFullInfo as jest.Mock).mockResolvedValue([updatedFile])
+      await store.syncFiles()
+
+      expect(store.userFiles).toHaveLength(1)
+      expect(store.userFiles[0].lastModified).toBe(456)
+      expect(store.userFiles[0].size).toBe(200)
+    })
+
+    it('should remove non-existent files', async () => {
+      const initialFiles = [
+        { path: 'file1.txt', modified: 123, size: 100 },
+        { path: 'file2.txt', modified: 456, size: 200 }
+      ]
+      ;(api.listUserDataFullInfo as jest.Mock).mockResolvedValue(initialFiles)
+      await store.syncFiles()
+
+      const updatedFiles = [{ path: 'file1.txt', modified: 123, size: 100 }]
+      ;(api.listUserDataFullInfo as jest.Mock).mockResolvedValue(updatedFiles)
+      await store.syncFiles()
+
+      expect(store.userFiles).toHaveLength(1)
+      expect(store.userFiles[0].path).toBe('file1.txt')
     })
   })
 
-  it('should close a file', () => {
-    store.openFiles = [
-      {
-        path: 'test.txt',
-        content: 'content',
-        isModified: false,
-        originalContent: 'content'
-      }
-    ]
+  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')
+      })
 
-    store.closeFile('test.txt')
+      await store.loadFile(file)
 
-    expect(store.openFiles).toHaveLength(0)
-  })
-
-  it('should update file content', () => {
-    store.openFiles = [
-      {
-        path: 'test.txt',
-        content: 'old content',
-        isModified: false,
-        originalContent: 'old content'
-      }
-    ]
-
-    store.updateFileContent('test.txt', 'new content')
-
-    expect(store.openFiles[0].content).toBe('new content')
-    expect(store.openFiles[0].isModified).toBe(true)
-  })
-
-  it('should save an open file', async () => {
-    store.openFiles = [
-      {
-        path: 'test.txt',
-        content: 'modified content',
-        isModified: true,
-        originalContent: 'original content'
-      }
-    ]
-    ;(api.storeUserData as jest.Mock).mockResolvedValue({ status: 200 })
-    ;(api.listUserData as jest.Mock).mockResolvedValue(['test.txt'])
-    ;(api.getUserData as jest.Mock).mockResolvedValue({
-      status: 200,
-      json: () => 'modified content'
+      expect(file.content).toBe('file content')
+      expect(file.originalContent).toBe('file content')
+      expect(file.isLoading).toBe(false)
     })
 
-    await store.saveOpenFile('test.txt')
+    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(store.openFiles[0].isModified).toBe(false)
-    expect(store.openFiles[0].originalContent).toBe('modified content')
+      await expect(store.loadFile(file)).rejects.toThrow(
+        "Failed to load file 'file1.txt': 404 Not Found"
+      )
+    })
   })
 
-  it('should discard changes', () => {
-    store.openFiles = [
-      {
-        path: 'test.txt',
-        content: 'modified content',
-        isModified: true,
-        originalContent: 'original 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([])
 
-    store.discardChanges('test.txt')
+      await store.saveFile(file)
 
-    expect(store.openFiles[0].content).toBe('original content')
-    expect(store.openFiles[0].isModified).toBe(false)
-  })
-
-  it('should load files', async () => {
-    ;(api.listUserData as jest.Mock).mockResolvedValue([
-      'file1.txt',
-      'file2.txt'
-    ])
-
-    await store.loadFiles()
-
-    expect(store.files).toEqual(['file1.txt', 'file2.txt'])
-  })
-
-  it('should rename a file', async () => {
-    store.openFiles = [
-      {
-        path: 'oldfile.txt',
-        content: 'content',
-        isModified: false,
-        originalContent: 'content'
-      }
-    ]
-    ;(api.moveUserData as jest.Mock).mockResolvedValue({ status: 200 })
-    ;(api.listUserData as jest.Mock).mockResolvedValue(['newfile.txt'])
-
-    await store.renameFile('oldfile.txt', 'newfile.txt')
-
-    expect(store.openFiles[0].path).toBe('newfile.txt')
-    expect(store.files).toEqual(['newfile.txt'])
-  })
-
-  it('should delete a file', async () => {
-    store.openFiles = [
-      {
-        path: 'file.txt',
-        content: 'content',
-        isModified: false,
-        originalContent: 'content'
-      }
-    ]
-    ;(api.deleteUserData as jest.Mock).mockResolvedValue({ status: 204 })
-    ;(api.listUserData as jest.Mock).mockResolvedValue([])
-
-    await store.deleteFile('file.txt')
-
-    expect(store.openFiles).toHaveLength(0)
-    expect(store.files).toEqual([])
-  })
-
-  it('should get file data', async () => {
-    const mockFileData = { content: 'file content' }
-    ;(api.getUserData as jest.Mock).mockResolvedValue({
-      status: 200,
-      json: () => mockFileData
+      expect(api.storeUserData).toHaveBeenCalledWith(
+        'file1.txt',
+        'modified content'
+      )
     })
 
-    const result = await store.getFileData('test.txt')
+    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([])
 
-    expect(result).toEqual({ success: true, data: mockFileData })
+      await store.saveFile(file)
+
+      expect(api.storeUserData).not.toHaveBeenCalled()
+    })
+  })
+
+  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([])
+
+      await store.deleteFile(file)
+
+      expect(api.deleteUserData).toHaveBeenCalledWith('file1.txt')
+    })
+  })
+
+  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([])
+
+      await store.renameFile(file, 'newfile.txt')
+
+      expect(api.moveUserData).toHaveBeenCalledWith('file1.txt', 'newfile.txt')
+      expect(file.path).toBe('newfile.txt')
+    })
   })
 })