Skip to content

Commit

Permalink
fix(cli): allow special characters in paths (immich-app#13282)
Browse files Browse the repository at this point in the history
* fix(cli): commas in import paths

* adding more test cases
  • Loading branch information
etnoy authored Oct 9, 2024
1 parent 057510a commit b7dcc97
Show file tree
Hide file tree
Showing 5 changed files with 149 additions and 26 deletions.
1 change: 1 addition & 0 deletions cli/package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

12 changes: 1 addition & 11 deletions cli/src/utils.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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: {
Expand Down
22 changes: 9 additions & 13 deletions cli/src/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -141,25 +141,21 @@ export const crawl = async (options: CrawlOptions): Promise<string[]> => {
}
}

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}`],
});
Expand Down
139 changes: 137 additions & 2 deletions e2e/src/cli/specs/upload.e2e-spec.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,90 @@
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<string, boolean>;
}

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 an asterisk',
paths: [`/photos\*/image1.jpg`],
files: {
'/photos*/image1.jpg': true,
'/photos*/image2.jpg': false,
'/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`],
files: {
"/photos'/image1.jpg": true,
"/photos'/image2.jpg": false,
'/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`],
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,
},
},
{
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`, () => {
let admin: LoginResponseDto;
let key: string;
Expand Down Expand Up @@ -32,6 +113,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('');
Expand Down
1 change: 1 addition & 0 deletions e2e/src/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down

0 comments on commit b7dcc97

Please sign in to comment.