From 0817d7df3fbb0274e884496b694f4e76a0ad5c6b Mon Sep 17 00:00:00 2001 From: Jonathan Jogenfors Date: Mon, 7 Oct 2024 22:48:28 +0200 Subject: [PATCH 1/2] fix(cli): commas in import paths --- cli/package-lock.json | 1 + cli/src/utils.spec.ts | 12 +-- cli/src/utils.ts | 22 +++--- e2e/src/cli/specs/upload.e2e-spec.ts | 112 ++++++++++++++++++++++++++- e2e/src/utils.ts | 1 + 5 files changed, 122 insertions(+), 26 deletions(-) diff --git a/cli/package-lock.json b/cli/package-lock.json index 3ed2050a04e5b..ffefa5b27cdaa 100644 --- a/cli/package-lock.json +++ b/cli/package-lock.json @@ -2498,6 +2498,7 @@ "version": "3.3.2", "resolved": "https://registry.npmjs.org/fast-glob/-/fast-glob-3.3.2.tgz", "integrity": "sha512-oX2ruAFQwf/Orj8m737Y5adxDQO0LAB7/S5MnxCdTNDd4p6BsyIVsv9JQsATbTSq8KHRpLwIHbVlUNatxd+1Ow==", + "license": "MIT", "dependencies": { "@nodelib/fs.stat": "^2.0.2", "@nodelib/fs.walk": "^1.2.3", diff --git a/cli/src/utils.spec.ts b/cli/src/utils.spec.ts index 0094b329b8e39..3e7e55fcb69e1 100644 --- a/cli/src/utils.spec.ts +++ b/cli/src/utils.spec.ts @@ -115,17 +115,7 @@ const tests: Test[] = [ '/albums/image3.jpg': true, }, }, - { - test: 'should support globbing paths', - options: { - pathsToCrawl: ['/photos*'], - }, - files: { - '/photos1/image1.jpg': true, - '/photos2/image2.jpg': true, - '/images/image3.jpg': false, - }, - }, + { test: 'should crawl a single path without trailing slash', options: { diff --git a/cli/src/utils.ts b/cli/src/utils.ts index 67948e0bd211a..7bbbb5615b640 100644 --- a/cli/src/utils.ts +++ b/cli/src/utils.ts @@ -141,25 +141,21 @@ export const crawl = async (options: CrawlOptions): Promise => { } } - let searchPattern: string; - if (patterns.length === 1) { - searchPattern = patterns[0]; - } else if (patterns.length === 0) { + if (patterns.length === 0) { return crawledFiles; - } else { - searchPattern = '{' + patterns.join(',') + '}'; - } - - if (recursive) { - searchPattern = searchPattern + '/**/'; } - searchPattern = `${searchPattern}/*.{${extensions.join(',')}}`; + const searchPatterns = patterns.map((pattern) => { + let escapedPattern = pattern; + if (recursive) { + escapedPattern = escapedPattern + '/**'; + } + return `${escapedPattern}/*.{${extensions.join(',')}}`; + }); - const globbedFiles = await glob(searchPattern, { + const globbedFiles = await glob(searchPatterns, { absolute: true, caseSensitiveMatch: false, - onlyFiles: true, dot: includeHidden, ignore: [`**/${exclusionPattern}`], }); diff --git a/e2e/src/cli/specs/upload.e2e-spec.ts b/e2e/src/cli/specs/upload.e2e-spec.ts index db2b6c534137e..b6d2dd4d0b5e2 100644 --- a/e2e/src/cli/specs/upload.e2e-spec.ts +++ b/e2e/src/cli/specs/upload.e2e-spec.ts @@ -1,9 +1,63 @@ import { LoginResponseDto, getAllAlbums, getAssetStatistics } from '@immich/sdk'; -import { readFileSync } from 'node:fs'; +import { cpSync, readFileSync } from 'node:fs'; import { mkdir, readdir, rm, symlink } from 'node:fs/promises'; -import { asKeyAuth, immichCli, testAssetDir, utils } from 'src/utils'; +import { asKeyAuth, immichCli, specialCharStrings, testAssetDir, utils } from 'src/utils'; import { beforeAll, beforeEach, describe, expect, it } from 'vitest'; +interface Test { + test: string; + paths: string[]; + files: Record; +} + +const tests: Test[] = [ + { + test: 'should support globbing with *', + paths: [`/photos*`], + files: { + '/photos1/image1.jpg': true, + '/photos2/image2.jpg': true, + '/images/image3.jpg': false, + }, + }, + { + test: 'should support paths with *', + paths: [`/photos\*/image1.jpg`], + files: { + '/photos*/image1.jpg': true, + '/photos*/image2.jpg': false, + '/images/image3.jpg': false, + }, + }, + { + test: 'should support paths with a single quote', + paths: [`/photos\'/image1.jpg`], + files: { + "/photos'/image1.jpg": true, + "/photos'/image2.jpg": false, + '/images/image3.jpg': false, + }, + }, + { + test: 'should support paths with a comma', + paths: [`/photos, eh/image1.jpg`], + files: { + '/photos, eh/image1.jpg': true, + '/photos, eh/image2.jpg': false, + '/images/image3.jpg': false, + }, + }, + { + test: 'should support paths with an opening brace', + paths: [`/photos\{/image1.jpg`], + files: { + '/photos{/image1.jpg': true, + '/photos{/image2.jpg': false, + '/images/image3.jpg': false, + }, + }, +]; + describe(`immich upload`, () => { let admin: LoginResponseDto; let key: string; @@ -32,6 +86,60 @@ describe(`immich upload`, () => { expect(assets.total).toBe(1); }); + describe(`should accept special cases`, () => { + for (const { test, paths, files } of tests) { + it(test, async () => { + const baseDir = `/tmp/upload/`; + + const testPaths = Object.keys(files).map((filePath) => `${baseDir}/${filePath}`); + testPaths.map((filePath) => utils.createImageFile(filePath)); + + const commandLine = paths.map((argument) => `${baseDir}/${argument}`); + + const expectedCount = Object.entries(files).filter((entry) => entry[1]).length; + + const { stderr, stdout, exitCode } = await immichCli(['upload', ...commandLine]); + expect(stderr).toBe(''); + expect(stdout.split('\n')).toEqual( + expect.arrayContaining([expect.stringContaining(`Successfully uploaded ${expectedCount} new asset`)]), + ); + expect(exitCode).toBe(0); + + const assets = await getAssetStatistics({}, { headers: asKeyAuth(key) }); + expect(assets.total).toBe(expectedCount); + + testPaths.map((filePath) => utils.removeImageFile(filePath)); + }); + } + }); + + it.each(specialCharStrings)(`should upload a multiple files from paths containing %s`, async (testString) => { + // https://github.com/immich-app/immich/issues/12078 + + // NOTE: this test must contain more than one path since a related bug is only triggered with multiple paths + + const testPaths = [ + `${testAssetDir}/temp/dir1${testString}name/asset.jpg`, + `${testAssetDir}/temp/dir2${testString}name/asset.jpg`, + ]; + + cpSync(`${testAssetDir}/albums/nature/tanners_ridge.jpg`, testPaths[0]); + cpSync(`${testAssetDir}/albums/nature/silver_fir.jpg`, testPaths[1]); + + const { stderr, stdout, exitCode } = await immichCli(['upload', ...testPaths]); + expect(stderr).toBe(''); + expect(stdout.split('\n')).toEqual( + expect.arrayContaining([expect.stringContaining('Successfully uploaded 2 new assets')]), + ); + expect(exitCode).toBe(0); + + utils.removeImageFile(testPaths[0]); + utils.removeImageFile(testPaths[1]); + + const assets = await getAssetStatistics({}, { headers: asKeyAuth(key) }); + expect(assets.total).toBe(2); + }); + it('should skip a duplicate file', async () => { const first = await immichCli(['upload', `${testAssetDir}/albums/nature/silver_fir.jpg`]); expect(first.stderr).toBe(''); diff --git a/e2e/src/utils.ts b/e2e/src/utils.ts index e21b3bfd14934..52acd35a5c4b1 100644 --- a/e2e/src/utils.ts +++ b/e2e/src/utils.ts @@ -68,6 +68,7 @@ export const immichCli = (args: string[]) => executeCommand('node', ['node_modules/.bin/immich', '-d', `/${tempDir}/immich/`, ...args]).promise; export const immichAdmin = (args: string[]) => executeCommand('docker', ['exec', '-i', 'immich-e2e-server', '/bin/bash', '-c', `immich-admin ${args.join(' ')}`]); +export const specialCharStrings = ["'", '"', ',', '{', '}', '*']; const executeCommand = (command: string, args: string[]) => { let _resolve: (value: CommandResponse) => void; From c1b3ed0eae391dfe850bf6e2d8eea3ffae892c55 Mon Sep 17 00:00:00 2001 From: Jonathan Jogenfors Date: Wed, 9 Oct 2024 08:22:11 +0200 Subject: [PATCH 2/2] adding more test cases --- e2e/src/cli/specs/upload.e2e-spec.ts | 29 +++++++++++++++++++++++++++- 1 file changed, 28 insertions(+), 1 deletion(-) diff --git a/e2e/src/cli/specs/upload.e2e-spec.ts b/e2e/src/cli/specs/upload.e2e-spec.ts index b6d2dd4d0b5e2..d700aa73b2091 100644 --- a/e2e/src/cli/specs/upload.e2e-spec.ts +++ b/e2e/src/cli/specs/upload.e2e-spec.ts @@ -21,7 +21,7 @@ const tests: Test[] = [ }, }, { - test: 'should support paths with *', + test: 'should support paths with an asterisk', paths: [`/photos\*/image1.jpg`], files: { '/photos*/image1.jpg': true, @@ -29,6 +29,15 @@ const tests: Test[] = [ '/images/image3.jpg': false, }, }, + { + test: 'should support paths with a space', + paths: [`/my photos/image1.jpg`], + files: { + '/my photos/image1.jpg': true, + '/my photos/image2.jpg': false, + '/images/image3.jpg': false, + }, + }, { test: 'should support paths with a single quote', paths: [`/photos\'/image1.jpg`], @@ -38,6 +47,15 @@ const tests: Test[] = [ '/images/image3.jpg': false, }, }, + { + test: 'should support paths with a double quote', + paths: [`/photos\"/image1.jpg`], + files: { + '/photos"/image1.jpg': true, + '/photos"/image2.jpg': false, + '/images/image3.jpg': false, + }, + }, { test: 'should support paths with a comma', paths: [`/photos, eh/image1.jpg`], @@ -56,6 +74,15 @@ const tests: Test[] = [ '/images/image3.jpg': false, }, }, + { + test: 'should support paths with a closing brace', + paths: [`/photos\}/image1.jpg`], + files: { + '/photos}/image1.jpg': true, + '/photos}/image2.jpg': false, + '/images/image3.jpg': false, + }, + }, ]; describe(`immich upload`, () => {