Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(cli): allow special characters in paths #13282

Merged
merged 2 commits into from
Oct 9, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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,
},
},
etnoy marked this conversation as resolved.
Show resolved Hide resolved
{
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
Loading