feat(file-upload): generalized storage to support azure blob, enhanced error logging in kb, added xlsx parser (#506)

* added blob storage option for azure, refactored storage client to be provider agnostic, tested kb & file upload and s3 is undisrupted, still have to test blob

* updated CORS policy for blob, added azure blob-specific headers

* remove extraneous comments

* add file size limit and timeout

* added some extra error handling in kb add documents

* grouped envvars

* ack PR comments

* added sheetjs and xlsx parser

Author: waleedlatif1

.gitignore

@@ -29,7 +29,6 @@ sim-standalone.tar.gz
 # misc
 .DS_Store
 *.pem
-uploads/
 
 # env files
 .env

apps/sim/app/api/files/delete/route.test.ts

@@ -1,22 +1,15 @@
-/**
- * Tests for file delete API route
- *
- * @vitest-environment node
- */
 import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
 import { createMockRequest } from '@/app/api/__test-utils__/utils'
 
 describe('File Delete API Route', () => {
-  // Mock file system modules
   const mockUnlink = vi.fn().mockResolvedValue(undefined)
   const mockExistsSync = vi.fn().mockReturnValue(true)
-  const mockDeleteFromS3 = vi.fn().mockResolvedValue(undefined)
-  const mockEnsureUploadsDirectory = vi.fn().mockResolvedValue(true)
+  const mockDeleteFile = vi.fn().mockResolvedValue(undefined)
+  const mockIsUsingCloudStorage = vi.fn().mockReturnValue(false)
 
   beforeEach(() => {
     vi.resetModules()
 
-    // Mock filesystem operations
     vi.doMock('fs', () => ({
       existsSync: mockExistsSync,
     }))
@@ -25,12 +18,11 @@ describe('File Delete API Route', () => {
       unlink: mockUnlink,
     }))
 
-    // Mock the S3 client
-    vi.doMock('@/lib/uploads/s3-client', () => ({
-      deleteFromS3: mockDeleteFromS3,
+    vi.doMock('@/lib/uploads', () => ({
+      deleteFile: mockDeleteFile,
+      isUsingCloudStorage: mockIsUsingCloudStorage,
     }))
 
-    // Mock the logger
     vi.doMock('@/lib/logs/console-logger', () => ({
       createLogger: vi.fn().mockReturnValue({
         info: vi.fn(),
@@ -40,18 +32,12 @@ describe('File Delete API Route', () => {
       }),
     }))
 
-    // Configure upload directory and S3 mode with all required exports
     vi.doMock('@/lib/uploads/setup', () => ({
       UPLOAD_DIR: '/test/uploads',
       USE_S3_STORAGE: false,
-      ensureUploadsDirectory: mockEnsureUploadsDirectory,
-      S3_CONFIG: {
-        bucket: 'test-bucket',
-        region: 'test-region',
-      },
+      USE_BLOB_STORAGE: false,
     }))
 
-    // Skip setup.server.ts side effects
     vi.doMock('@/lib/uploads/setup.server', () => ({}))
   })
 
@@ -60,111 +46,114 @@ describe('File Delete API Route', () => {
   })
 
   it('should handle local file deletion successfully', async () => {
-    // Configure upload directory and S3 mode for this test
     vi.doMock('@/lib/uploads/setup', () => ({
       UPLOAD_DIR: '/test/uploads',
       USE_S3_STORAGE: false,
     }))
 
-    // Create request with file path
     const req = createMockRequest('POST', {
       filePath: '/api/files/serve/test-file.txt',
     })
 
-    // Import the handler after mocks are set up
     const { POST } = await import('./route')
 
-    // Call the handler
     const response = await POST(req)
     const data = await response.json()
 
-    // Verify response
     expect(response.status).toBe(200)
     expect(data).toHaveProperty('success', true)
     expect(data).toHaveProperty('message', 'File deleted successfully')
 
-    // Verify unlink was called with correct path
     expect(mockUnlink).toHaveBeenCalledWith('/test/uploads/test-file.txt')
   })
 
   it('should handle file not found gracefully', async () => {
-    // Mock file not existing
     mockExistsSync.mockReturnValueOnce(false)
 
-    // Create request with file path
     const req = createMockRequest('POST', {
       filePath: '/api/files/serve/nonexistent.txt',
     })
 
-    // Import the handler after mocks are set up
     const { POST } = await import('./route')
 
-    // Call the handler
     const response = await POST(req)
     const data = await response.json()
 
-    // Verify response
     expect(response.status).toBe(200)
     expect(data).toHaveProperty('success', true)
     expect(data).toHaveProperty('message', "File not found, but that's okay")
 
-    // Verify unlink was not called
     expect(mockUnlink).not.toHaveBeenCalled()
   })
 
   it('should handle S3 file deletion successfully', async () => {
-    // Configure upload directory and S3 mode for this test
     vi.doMock('@/lib/uploads/setup', () => ({
       UPLOAD_DIR: '/test/uploads',
       USE_S3_STORAGE: true,
+      USE_BLOB_STORAGE: false,
     }))
 
-    // Create request with S3 file path
+    mockIsUsingCloudStorage.mockReturnValue(true)
+
     const req = createMockRequest('POST', {
       filePath: '/api/files/serve/s3/1234567890-test-file.txt',
     })
 
-    // Import the handler after mocks are set up
     const { POST } = await import('./route')
 
-    // Call the handler
     const response = await POST(req)
     const data = await response.json()
 
-    // Verify response
     expect(response.status).toBe(200)
     expect(data).toHaveProperty('success', true)
-    expect(data).toHaveProperty('message', 'File deleted successfully from S3')
+    expect(data).toHaveProperty('message', 'File deleted successfully from cloud storage')
+
+    expect(mockDeleteFile).toHaveBeenCalledWith('1234567890-test-file.txt')
+  })
+
+  it('should handle Azure Blob file deletion successfully', async () => {
+    vi.doMock('@/lib/uploads/setup', () => ({
+      UPLOAD_DIR: '/test/uploads',
+      USE_S3_STORAGE: false,
+      USE_BLOB_STORAGE: true,
+    }))
+
+    mockIsUsingCloudStorage.mockReturnValue(true)
+
+    const req = createMockRequest('POST', {
+      filePath: '/api/files/serve/blob/1234567890-test-document.pdf',
+    })
+
+    const { POST } = await import('./route')
+
+    const response = await POST(req)
+    const data = await response.json()
+
+    expect(response.status).toBe(200)
+    expect(data).toHaveProperty('success', true)
+    expect(data).toHaveProperty('message', 'File deleted successfully from cloud storage')
 
-    // Verify deleteFromS3 was called with correct key
-    expect(mockDeleteFromS3).toHaveBeenCalledWith('1234567890-test-file.txt')
+    expect(mockDeleteFile).toHaveBeenCalledWith('1234567890-test-document.pdf')
   })
 
   it('should handle missing file path', async () => {
-    // Create request with no file path
     const req = createMockRequest('POST', {})
 
-    // Import the handler after mocks are set up
     const { POST } = await import('./route')
 
-    // Call the handler
     const response = await POST(req)
     const data = await response.json()
 
-    // Verify error response
     expect(response.status).toBe(400)
     expect(data).toHaveProperty('error', 'InvalidRequestError')
     expect(data).toHaveProperty('message', 'No file path provided')
   })
 
   it('should handle CORS preflight requests', async () => {
-    // Import the handler after mocks are set up
     const { OPTIONS } = await import('./route')
 
-    // Call the handler
     const response = await OPTIONS()
 
-    // Verify response
     expect(response.status).toBe(204)
     expect(response.headers.get('Access-Control-Allow-Methods')).toBe('GET, POST, DELETE, OPTIONS')
     expect(response.headers.get('Access-Control-Allow-Headers')).toBe('Content-Type')

apps/sim/app/api/files/delete/route.ts

@@ -3,17 +3,20 @@ import { unlink } from 'fs/promises'
 import { join } from 'path'
 import type { NextRequest } from 'next/server'
 import { createLogger } from '@/lib/logs/console-logger'
-import { deleteFromS3 } from '@/lib/uploads/s3-client'
-import { UPLOAD_DIR, USE_S3_STORAGE } from '@/lib/uploads/setup'
+import { deleteFile, isUsingCloudStorage } from '@/lib/uploads'
+import { UPLOAD_DIR } from '@/lib/uploads/setup'
 import '@/lib/uploads/setup.server'
 
 import {
   createErrorResponse,
   createOptionsResponse,
   createSuccessResponse,
+  extractBlobKey,
   extractFilename,
   extractS3Key,
   InvalidRequestError,
+  isBlobPath,
+  isCloudPath,
   isS3Path,
 } from '../utils'
 
@@ -38,8 +41,8 @@ export async function POST(request: NextRequest) {
     try {
       // Use appropriate handler based on path and environment
       const result =
-        isS3Path(filePath) || USE_S3_STORAGE
-          ? await handleS3FileDelete(filePath)
+        isCloudPath(filePath) || isUsingCloudStorage()
+          ? await handleCloudFileDelete(filePath)
           : await handleLocalFileDelete(filePath)
 
       // Return success response
@@ -57,24 +60,24 @@ export async function POST(request: NextRequest) {
 }
 
 /**
- * Handle S3 file deletion
+ * Handle cloud file deletion (S3 or Azure Blob)
  */
-async function handleS3FileDelete(filePath: string) {
-  // Extract the S3 key from the path
-  const s3Key = extractS3Key(filePath)
-  logger.info(`Deleting file from S3: ${s3Key}`)
+async function handleCloudFileDelete(filePath: string) {
+  // Extract the key from the path (works for both S3 and Blob paths)
+  const key = extractCloudKey(filePath)
+  logger.info(`Deleting file from cloud storage: ${key}`)
 
   try {
-    // Delete from S3
-    await deleteFromS3(s3Key)
-    logger.info(`File successfully deleted from S3: ${s3Key}`)
+    // Delete from cloud storage using abstraction layer
+    await deleteFile(key)
+    logger.info(`File successfully deleted from cloud storage: ${key}`)
 
     return {
       success: true as const,
-      message: 'File deleted successfully from S3',
+      message: 'File deleted successfully from cloud storage',
     }
   } catch (error) {
-    logger.error('Error deleting file from S3:', error)
+    logger.error('Error deleting file from cloud storage:', error)
     throw error
   }
 }
@@ -83,30 +86,52 @@ async function handleS3FileDelete(filePath: string) {
  * Handle local file deletion
  */
 async function handleLocalFileDelete(filePath: string) {
-  // Extract the filename from the path
   const filename = extractFilename(filePath)
-  logger.info('Extracted filename for deletion:', filename)
-
   const fullPath = join(UPLOAD_DIR, filename)
-  logger.info('Full file path for deletion:', fullPath)
 
-  // Check if file exists
+  logger.info(`Deleting local file: ${fullPath}`)
+
   if (!existsSync(fullPath)) {
-    logger.info(`File not found for deletion at path: ${fullPath}`)
+    logger.info(`File not found, but that's okay: ${fullPath}`)
     return {
       success: true as const,
       message: "File not found, but that's okay",
     }
   }
 
-  // Delete the file
-  await unlink(fullPath)
-  logger.info(`File successfully deleted: ${fullPath}`)
+  try {
+    await unlink(fullPath)
+    logger.info(`File successfully deleted: ${fullPath}`)
+
+    return {
+      success: true as const,
+      message: 'File deleted successfully',
+    }
+  } catch (error) {
+    logger.error('Error deleting local file:', error)
+    throw error
+  }
+}
+
+/**
+ * Extract cloud storage key from file path (works for both S3 and Blob)
+ */
+function extractCloudKey(filePath: string): string {
+  if (isS3Path(filePath)) {
+    return extractS3Key(filePath)
+  }
+
+  if (isBlobPath(filePath)) {
+    return extractBlobKey(filePath)
+  }
 
-  return {
-    success: true as const,
-    message: 'File deleted successfully',
+  // Backwards-compatibility: allow generic paths like "/api/files/serve/<key>"
+  if (filePath.startsWith('/api/files/serve/')) {
+    return decodeURIComponent(filePath.substring('/api/files/serve/'.length))
   }
+
+  // As a last resort assume the incoming string is already a raw key.
+  return filePath
 }
 
 /**