refactor: Apply PR #6112 review feedback for Media Assets feature

viva-jinyi · claude · viva-jinyi · commit 1868ebc26311 · 2025-10-22T21:51:10.000+09:00
- Move composables to platform/assets directory structure - Extract interface-based abstraction (IAssetsProvider) for cloud/internal implementations - Move constants to module scope to avoid re-initialization - Extract helper functions (truncateFilename, assetMappers) for reusability - Rename getMediaTypeFromFilename to return singular form (image/video/audio) - Add deprecated plural version for backward compatibility - Add comprehensive test coverage for new utility functions 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
diff --git a/packages/shared-frontend-utils/src/formatUtil.ts b/packages/shared-frontend-utils/src/formatUtil.ts
@@ -475,50 +475,92 @@ export function formatDuration(milliseconds: number): string {
   return parts.join(' ')
 }
 
+// Module scope constants to avoid re-initialization on every call
+const IMAGE_EXTENSIONS = ['png', 'jpg', 'jpeg', 'gif', 'webp', 'bmp']
+const VIDEO_EXTENSIONS = ['mp4', 'webm', 'mov', 'avi']
+const AUDIO_EXTENSIONS = ['mp3', 'wav', 'ogg', 'flac']
+const THREE_D_EXTENSIONS = ['obj', 'fbx', 'gltf', 'glb']
+
 /**
- * Determines the media type from a filename's extension
- * @param filename The filename to analyze
- * @returns The media type: 'images', 'videos', 'audios', '3D' for gallery compatibility
+ * Truncates a filename while preserving the extension
+ * @param filename The filename to truncate
+ * @param maxLength Maximum length for the filename without extension
+ * @returns Truncated filename with extension preserved
  */
-export function getMediaTypeFromFilename(filename: string): string {
-  if (!filename) return 'images'
-  const ext = filename.split('.').pop()?.toLowerCase()
-  if (!ext) return 'images'
+export function truncateFilename(
+  filename: string,
+  maxLength: number = 20
+): string {
+  if (!filename || filename.length <= maxLength) {
+    return filename
+  }
+
+  const lastDotIndex = filename.lastIndexOf('.')
+  const nameWithoutExt =
+    lastDotIndex > -1 ? filename.substring(0, lastDotIndex) : filename
+  const extension = lastDotIndex > -1 ? filename.substring(lastDotIndex) : ''
 
-  const imageExts = ['png', 'jpg', 'jpeg', 'gif', 'webp', 'bmp']
-  const videoExts = ['mp4', 'webm', 'mov', 'avi']
-  const audioExts = ['mp3', 'wav', 'ogg', 'flac']
-  const threeDExts = ['obj', 'fbx', 'gltf', 'glb']
+  // If the name without extension is short enough, return as is
+  if (nameWithoutExt.length <= maxLength) {
+    return filename
+  }
 
-  if (imageExts.includes(ext)) return 'images'
-  if (videoExts.includes(ext)) return 'videos'
-  if (audioExts.includes(ext)) return 'audios'
-  if (threeDExts.includes(ext)) return '3D'
+  // Calculate how to split the truncation
+  const halfLength = Math.floor((maxLength - 3) / 2) // -3 for '...'
+  const start = nameWithoutExt.substring(0, halfLength)
+  const end = nameWithoutExt.substring(nameWithoutExt.length - halfLength)
 
-  return 'images'
+  return `${start}...${end}${extension}`
 }
 
 /**
- * Determines the media kind from a filename's extension
+ * Determines the media type from a filename's extension (singular form)
  * @param filename The filename to analyze
- * @returns The media kind: 'image', 'video', 'audio', or '3D'
+ * @returns The media type: 'image', 'video', 'audio', or '3D'
  */
-export function getMediaKindFromFilename(
+export function getMediaTypeFromFilename(
   filename: string
 ): 'image' | 'video' | 'audio' | '3D' {
   if (!filename) return 'image'
   const ext = filename.split('.').pop()?.toLowerCase()
   if (!ext) return 'image'
 
-  const imageExts = ['png', 'jpg', 'jpeg', 'gif', 'webp', 'bmp']
-  const videoExts = ['mp4', 'webm', 'mov', 'avi']
-  const audioExts = ['mp3', 'wav', 'ogg', 'flac']
-  const threeDExts = ['obj', 'fbx', 'gltf', 'glb']
-
-  if (imageExts.includes(ext)) return 'image'
-  if (videoExts.includes(ext)) return 'video'
-  if (audioExts.includes(ext)) return 'audio'
-  if (threeDExts.includes(ext)) return '3D'
+  if (IMAGE_EXTENSIONS.includes(ext)) return 'image'
+  if (VIDEO_EXTENSIONS.includes(ext)) return 'video'
+  if (AUDIO_EXTENSIONS.includes(ext)) return 'audio'
+  if (THREE_D_EXTENSIONS.includes(ext)) return '3D'
 
   return 'image'
 }
+
+/**
+ * @deprecated Use getMediaTypeFromFilename instead - returns plural form for legacy compatibility
+ * @param filename The filename to analyze
+ * @returns The media type in plural form: 'images', 'videos', 'audios', '3D'
+ */
+export function getMediaTypeFromFilenamePlural(filename: string): string {
+  const type = getMediaTypeFromFilename(filename)
+  switch (type) {
+    case 'image':
+      return 'images'
+    case 'video':
+      return 'videos'
+    case 'audio':
+      return 'audios'
+    case '3D':
+      return '3D'
+    default:
+      return 'images'
+  }
+}
+
+/**
+ * @deprecated Use getMediaTypeFromFilename instead - kept for backward compatibility
+ * @param filename The filename to analyze
+ * @returns The media kind: 'image', 'video', 'audio', or '3D'
+ */
+export function getMediaKindFromFilename(
+  filename: string
+): 'image' | 'video' | 'audio' | '3D' {
+  return getMediaTypeFromFilename(filename)
+}
diff --git a/src/components/sidebar/tabs/AssetsSidebarTab.vue b/src/components/sidebar/tabs/AssetsSidebarTab.vue
@@ -65,23 +65,18 @@ import NoResultsPlaceholder from '@/components/common/NoResultsPlaceholder.vue'
 import VirtualGrid from '@/components/common/VirtualGrid.vue'
 import SidebarTabTemplate from '@/components/sidebar/tabs/SidebarTabTemplate.vue'
 import ResultGallery from '@/components/sidebar/tabs/queue/ResultGallery.vue'
-import { useCloudMediaAssets } from '@/composables/useCloudMediaAssets'
-import { useInternalMediaAssets } from '@/composables/useInternalMediaAssets'
 import MediaAssetCard from '@/platform/assets/components/MediaAssetCard.vue'
+import { useMediaAssets } from '@/platform/assets/composables/useMediaAssets'
 import type { AssetItem } from '@/platform/assets/schemas/assetSchema'
-import { isCloud } from '@/platform/distribution/types'
 import { ResultItemImpl } from '@/stores/queueStore'
-import { getMediaTypeFromFilename } from '@/utils/formatUtil'
+import { getMediaTypeFromFilenamePlural } from '@/utils/formatUtil'
 
 const activeTab = ref<'input' | 'output'>('input')
 const mediaAssets = ref<AssetItem[]>([])
 const selectedAsset = ref<AssetItem | null>(null)
 
-// Use appropriate implementation based on environment
-const implementation = isCloud
-  ? useCloudMediaAssets()
-  : useInternalMediaAssets()
-const { loading, error, fetchMediaList } = implementation
+// Use unified media assets implementation that handles cloud/internal automatically
+const { loading, error, fetchMediaList } = useMediaAssets()
 
 const galleryActiveIndex = ref(-1)
 const galleryItems = computed(() => {
@@ -92,7 +87,7 @@ const galleryItems = computed(() => {
       subfolder: '',
       type: 'output',
       nodeId: '0',
-      mediaType: getMediaTypeFromFilename(asset.name)
+      mediaType: getMediaTypeFromFilenamePlural(asset.name)
     })
 
     // Override the url getter to use asset.preview_url
diff --git a/src/platform/assets/components/MediaAssetCard.vue b/src/platform/assets/components/MediaAssetCard.vue
@@ -128,7 +128,7 @@ import CardBottom from '@/components/card/CardBottom.vue'
 import CardContainer from '@/components/card/CardContainer.vue'
 import CardTop from '@/components/card/CardTop.vue'
 import SquareChip from '@/components/chip/SquareChip.vue'
-import { formatDuration, getMediaKindFromFilename } from '@/utils/formatUtil'
+import { formatDuration, getMediaTypeFromFilename } from '@/utils/formatUtil'
 import { cn } from '@/utils/tailwindUtil'
 
 import { useMediaAssetActions } from '../composables/useMediaAssetActions'
@@ -191,7 +191,7 @@ const assetType = computed(() => {
 
 // Determine file type from extension
 const fileKind = computed((): MediaKind => {
-  return getMediaKindFromFilename(asset?.name || '') as MediaKind
+  return getMediaTypeFromFilename(asset?.name || '') as MediaKind
 })
 
 // Adapt AssetItem to legacy AssetMeta format for existing components
diff --git a/src/platform/assets/composables/useMediaAssets/IAssetsProvider.ts b/src/platform/assets/composables/useMediaAssets/IAssetsProvider.ts
@@ -0,0 +1,22 @@
+import type { Ref } from 'vue'
+
+import type { AssetItem } from '@/platform/assets/schemas/assetSchema'
+
+/**
+ * Interface for media assets providers
+ * Defines the common API for both cloud and internal file implementations
+ */
+export interface IAssetsProvider {
+  /** Loading state indicator */
+  loading: Ref<boolean>
+
+  /** Error state, null when no error */
+  error: Ref<string | null>
+
+  /**
+   * Fetch list of media assets from the specified directory
+   * @param directory - 'input' or 'output'
+   * @returns Promise resolving to array of AssetItem
+   */
+  fetchMediaList: (directory: 'input' | 'output') => Promise<AssetItem[]>
+}
diff --git a/src/platform/assets/composables/useMediaAssets/assetMappers.ts b/src/platform/assets/composables/useMediaAssets/assetMappers.ts
@@ -0,0 +1,81 @@
+import type { AssetItem } from '@/platform/assets/schemas/assetSchema'
+import { api } from '@/scripts/api'
+import type { TaskItemImpl } from '@/stores/queueStore'
+import { truncateFilename } from '@/utils/formatUtil'
+
+/**
+ * Maps a TaskItemImpl output to an AssetItem format
+ * @param taskItem The task item containing execution data
+ * @param output The output from the task
+ * @param useDisplayName Whether to truncate the filename for display
+ * @returns AssetItem formatted object
+ */
+export function mapTaskOutputToAssetItem(
+  taskItem: TaskItemImpl,
+  output: any,
+  useDisplayName: boolean = false
+): AssetItem {
+  const metadata: Record<string, any> = {
+    promptId: taskItem.promptId,
+    nodeId: output.nodeId,
+    subfolder: output.subfolder
+  }
+
+  // Add execution time if available
+  if (taskItem.executionTimeInSeconds) {
+    metadata.executionTimeInSeconds = taskItem.executionTimeInSeconds
+  }
+
+  // Add format if available
+  if (output.format) {
+    metadata.format = output.format
+  }
+
+  // Add workflow if available
+  if (taskItem.workflow) {
+    metadata.workflow = taskItem.workflow
+  }
+
+  // Store original filename if using display name
+  if (useDisplayName) {
+    metadata.originalFilename = output.filename
+  }
+
+  return {
+    id: `${taskItem.promptId}-${output.nodeId}-${output.filename}`,
+    name: useDisplayName
+      ? truncateFilename(output.filename, 20)
+      : output.filename,
+    size: 0, // Size not available from history API
+    created_at: taskItem.executionStartTimestamp
+      ? new Date(taskItem.executionStartTimestamp).toISOString()
+      : new Date().toISOString(),
+    tags: ['output'],
+    preview_url: output.url,
+    user_metadata: metadata
+  }
+}
+
+/**
+ * Maps input directory file to AssetItem format
+ * @param filename The filename
+ * @param index File index for unique ID
+ * @param directory The directory type
+ * @returns AssetItem formatted object
+ */
+export function mapInputFileToAssetItem(
+  filename: string,
+  index: number,
+  directory: 'input' | 'output' = 'input'
+): AssetItem {
+  return {
+    id: `${directory}-${index}-${filename}`,
+    name: filename,
+    size: 0,
+    created_at: new Date().toISOString(),
+    tags: [directory],
+    preview_url: api.apiURL(
+      `/view?filename=${encodeURIComponent(filename)}&type=${directory}`
+    )
+  }
+}
diff --git a/src/platform/assets/composables/useMediaAssets/index.ts b/src/platform/assets/composables/useMediaAssets/index.ts
@@ -0,0 +1,17 @@
+import { isCloud } from '@/platform/distribution/types'
+
+import type { IAssetsProvider } from './IAssetsProvider'
+import { useAssetsApi } from './useAssetsApi'
+import { useInternalFilesApi } from './useInternalFilesApi'
+
+/**
+ * Factory function that returns the appropriate media assets implementation
+ * based on the current distribution (cloud vs internal)
+ * @returns IAssetsProvider implementation
+ */
+export function useMediaAssets(): IAssetsProvider {
+  return isCloud ? useAssetsApi() : useInternalFilesApi()
+}
+
+// Re-export the interface for consumers
+export type { IAssetsProvider } from './IAssetsProvider'
diff --git a/src/platform/assets/composables/useMediaAssets/useAssetsApi.ts b/src/platform/assets/composables/useMediaAssets/useAssetsApi.ts
@@ -6,11 +6,13 @@ import type { HistoryTaskItem } from '@/schemas/apiSchema'
 import { api } from '@/scripts/api'
 import { TaskItemImpl } from '@/stores/queueStore'
 
+import { mapTaskOutputToAssetItem } from './assetMappers'
+
 /**
  * Composable for fetching media assets from cloud environment
  * Includes execution time from history API
  */
-export function useCloudMediaAssets() {
+export function useAssetsApi() {
   const loading = ref(false)
   const error = ref<string | null>(null)
 
@@ -53,62 +55,16 @@ export function useCloudMediaAssets() {
 
         // Only process completed tasks
         if (taskItem.displayStatus === 'Completed' && taskItem.outputs) {
-          // Get execution time
-          const executionTimeInSeconds = taskItem.executionTimeInSeconds
-
           // Process each output
           taskItem.flatOutputs.forEach((output) => {
             // Only include output type files (not temp previews)
             if (output.type === 'output' && output.supportsPreview) {
-              // Truncate filename if longer than 15 characters
-              let displayName = output.filename
-              if (output.filename.length > 20) {
-                // Get file extension
-                const lastDotIndex = output.filename.lastIndexOf('.')
-                const nameWithoutExt =
-                  lastDotIndex > -1
-                    ? output.filename.substring(0, lastDotIndex)
-                    : output.filename
-                const extension =
-                  lastDotIndex > -1
-                    ? output.filename.substring(lastDotIndex)
-                    : ''
-
-                // If name without extension is still long, truncate it
-                if (nameWithoutExt.length > 10) {
-                  displayName =
-                    nameWithoutExt.substring(0, 10) +
-                    '...' +
-                    nameWithoutExt.substring(nameWithoutExt.length - 10) +
-                    extension
-                }
-              }
-
-              assetItems.push({
-                id: `${taskItem.promptId}-${output.nodeId}-${output.filename}`,
-                name: displayName,
-                size: 0, // We don't have size info from history
-                created_at: taskItem.executionStartTimestamp
-                  ? new Date(taskItem.executionStartTimestamp).toISOString()
-                  : new Date().toISOString(),
-                tags: ['output'],
-                preview_url: output.url,
-                user_metadata: {
-                  originalFilename: output.filename, // Store original filename
-                  promptId: taskItem.promptId,
-                  nodeId: output.nodeId,
-                  subfolder: output.subfolder,
-                  ...(executionTimeInSeconds && {
-                    executionTimeInSeconds
-                  }),
-                  ...(output.format && {
-                    format: output.format
-                  }),
-                  ...(taskItem.workflow && {
-                    workflow: taskItem.workflow
-                  })
-                }
-              })
+              const assetItem = mapTaskOutputToAssetItem(
+                taskItem,
+                output,
+                true // Use display name for cloud
+              )
+              assetItems.push(assetItem)
             }
           })
         }
diff --git a/src/platform/assets/composables/useMediaAssets/useInternalFilesApi.ts b/src/platform/assets/composables/useMediaAssets/useInternalFilesApi.ts
diff --git a/tests-ui/tests/utils/formatUtil.test.ts b/tests-ui/tests/utils/formatUtil.test.ts
