From 84f29569410ec95a80e15d61060991b45963fe36 Mon Sep 17 00:00:00 2001 From: Timon Date: Fri, 20 Feb 2026 15:54:08 +0100 Subject: [PATCH] fix(cli): delete sidecar files after upload if requested (#26353) * fix(cli): delete sidecar files after upload if requested Introduced a new function, findSidecar, to locate XMP sidecar files based on specified naming conventions. Updated the deleteFiles function to delete associated sidecar files when the main asset file is deleted. Added unit tests for findSidecar to ensure correct functionality. * lint and format * fix test * chore: clean up --------- Co-authored-by: Jason Rasmussen --- cli/src/commands/asset.spec.ts | 92 +++++++++++++++++++++++++++++++++- cli/src/commands/asset.ts | 54 ++++++++++++-------- 2 files changed, 123 insertions(+), 23 deletions(-) diff --git a/cli/src/commands/asset.spec.ts b/cli/src/commands/asset.spec.ts index 7dce135985..ea57eeb74b 100644 --- a/cli/src/commands/asset.spec.ts +++ b/cli/src/commands/asset.spec.ts @@ -7,7 +7,15 @@ import { describe, expect, it, MockedFunction, vi } from 'vitest'; import { Action, checkBulkUpload, defaults, getSupportedMediaTypes, Reason } from '@immich/sdk'; import createFetchMock from 'vitest-fetch-mock'; -import { checkForDuplicates, getAlbumName, startWatch, uploadFiles, UploadOptionsDto } from 'src/commands/asset'; +import { + checkForDuplicates, + deleteFiles, + findSidecar, + getAlbumName, + startWatch, + uploadFiles, + UploadOptionsDto, +} from 'src/commands/asset'; vi.mock('@immich/sdk'); @@ -309,3 +317,85 @@ describe('startWatch', () => { await fs.promises.rm(testFolder, { recursive: true, force: true }); }); }); + +describe('findSidecar', () => { + let testDir: string; + let testFilePath: string; + + beforeEach(() => { + testDir = fs.mkdtempSync(path.join(os.tmpdir(), 'test-sidecar-')); + testFilePath = path.join(testDir, 'test.jpg'); + fs.writeFileSync(testFilePath, 'test'); + }); + + afterEach(() => { + fs.rmSync(testDir, { recursive: true, force: true }); + }); + + it('should find sidecar file with photo.xmp naming convention', () => { + const sidecarPath = path.join(testDir, 'test.xmp'); + fs.writeFileSync(sidecarPath, 'xmp data'); + + const result = findSidecar(testFilePath); + expect(result).toBe(sidecarPath); + }); + + it('should find sidecar file with photo.ext.xmp naming convention', () => { + const sidecarPath = path.join(testDir, 'test.jpg.xmp'); + fs.writeFileSync(sidecarPath, 'xmp data'); + + const result = findSidecar(testFilePath); + expect(result).toBe(sidecarPath); + }); + + it('should prefer photo.ext.xmp over photo.xmp when both exist', () => { + const sidecarPath1 = path.join(testDir, 'test.xmp'); + const sidecarPath2 = path.join(testDir, 'test.jpg.xmp'); + fs.writeFileSync(sidecarPath1, 'xmp data 1'); + fs.writeFileSync(sidecarPath2, 'xmp data 2'); + + const result = findSidecar(testFilePath); + // Should return the first one found (photo.xmp) based on the order in the code + expect(result).toBe(sidecarPath1); + }); + + it('should return undefined when no sidecar file exists', () => { + const result = findSidecar(testFilePath); + expect(result).toBeUndefined(); + }); +}); + +describe('deleteFiles', () => { + let testDir: string; + let testFilePath: string; + + beforeEach(() => { + testDir = fs.mkdtempSync(path.join(os.tmpdir(), 'test-delete-')); + testFilePath = path.join(testDir, 'test.jpg'); + fs.writeFileSync(testFilePath, 'test'); + }); + + afterEach(() => { + fs.rmSync(testDir, { recursive: true, force: true }); + }); + + it('should delete asset and sidecar file when main file is deleted', async () => { + const sidecarPath = path.join(testDir, 'test.xmp'); + fs.writeFileSync(sidecarPath, 'xmp data'); + + await deleteFiles([{ id: 'test-id', filepath: testFilePath }], [], { delete: true, concurrency: 1 }); + + expect(fs.existsSync(testFilePath)).toBe(false); + expect(fs.existsSync(sidecarPath)).toBe(false); + }); + + it('should not delete sidecar file when delete option is false', async () => { + const sidecarPath = path.join(testDir, 'test.xmp'); + fs.writeFileSync(sidecarPath, 'xmp data'); + + await deleteFiles([{ id: 'test-id', filepath: testFilePath }], [], { delete: false, concurrency: 1 }); + + expect(fs.existsSync(testFilePath)).toBe(true); + expect(fs.existsSync(sidecarPath)).toBe(true); + }); +}); diff --git a/cli/src/commands/asset.ts b/cli/src/commands/asset.ts index 42c33491f2..7d4b09b69d 100644 --- a/cli/src/commands/asset.ts +++ b/cli/src/commands/asset.ts @@ -17,7 +17,7 @@ import { Matcher, watch as watchFs } from 'chokidar'; import { MultiBar, Presets, SingleBar } from 'cli-progress'; import { chunk } from 'lodash-es'; import micromatch from 'micromatch'; -import { Stats, createReadStream } from 'node:fs'; +import { Stats, createReadStream, existsSync } from 'node:fs'; import { stat, unlink } from 'node:fs/promises'; import path, { basename } from 'node:path'; import { Queue } from 'src/queue'; @@ -403,23 +403,6 @@ export const uploadFiles = async ( const uploadFile = async (input: string, stats: Stats): Promise => { const { baseUrl, headers } = defaults; - const assetPath = path.parse(input); - const noExtension = path.join(assetPath.dir, assetPath.name); - - const sidecarsFiles = await Promise.all( - // XMP sidecars can come in two filename formats. For a photo named photo.ext, the filenames are photo.ext.xmp and photo.xmp - [`${noExtension}.xmp`, `${input}.xmp`].map(async (sidecarPath) => { - try { - const stats = await stat(sidecarPath); - return new UploadFile(sidecarPath, stats.size); - } catch { - return false; - } - }), - ); - - const sidecarData = sidecarsFiles.find((file): file is UploadFile => file !== false); - const formData = new FormData(); formData.append('deviceAssetId', `${basename(input)}-${stats.size}`.replaceAll(/\s+/g, '')); formData.append('deviceId', 'CLI'); @@ -429,8 +412,15 @@ const uploadFile = async (input: string, stats: Stats): Promise => { +export const findSidecar = (filepath: string): string | undefined => { + const assetPath = path.parse(filepath); + const noExtension = path.join(assetPath.dir, assetPath.name); + + // XMP sidecars can come in two filename formats. For a photo named photo.ext, the filenames are photo.ext.xmp and photo.xmp + for (const sidecarPath of [`${noExtension}.xmp`, `${filepath}.xmp`]) { + if (existsSync(sidecarPath)) { + return sidecarPath; + } + } +}; + +export const deleteFiles = async (uploaded: Asset[], duplicates: Asset[], options: UploadOptionsDto): Promise => { let fileCount = 0; if (options.delete) { fileCount += uploaded.length; @@ -474,7 +476,15 @@ const deleteFiles = async (uploaded: Asset[], duplicates: Asset[], options: Uplo const chunkDelete = async (files: Asset[]) => { for (const assetBatch of chunk(files, options.concurrency)) { - await Promise.all(assetBatch.map((input: Asset) => unlink(input.filepath))); + await Promise.all( + assetBatch.map(async (input: Asset) => { + await unlink(input.filepath); + const sidecarPath = findSidecar(input.filepath); + if (sidecarPath) { + await unlink(sidecarPath); + } + }), + ); deletionProgress.update(assetBatch.length); } };