diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md index 0bd3b30814..2d1fdafa30 100644 --- a/.github/pull_request_template.md +++ b/.github/pull_request_template.md @@ -26,6 +26,7 @@ The `/api/something` endpoint is now `/api/something-else` ## Checklist: +- [ ] I have carefully read CONTRIBUTING.md - [ ] I have performed a self-review of my own code - [ ] I have made corresponding changes to the documentation if applicable - [ ] I have no unrelated changes in the PR. diff --git a/.github/workflows/close-llm-pr.yml b/.github/workflows/close-llm-pr.yml new file mode 100644 index 0000000000..f17d98e684 --- /dev/null +++ b/.github/workflows/close-llm-pr.yml @@ -0,0 +1,38 @@ +name: Close LLM-generated PRs + +on: + pull_request: + types: [labeled] + +permissions: {} + +jobs: + comment_and_close: + runs-on: ubuntu-latest + if: ${{ github.event.label.name == 'llm-generated' }} + permissions: + pull-requests: write + steps: + - name: Comment and close + env: + GH_TOKEN: ${{ github.token }} + NODE_ID: ${{ github.event.pull_request.node_id }} + run: | + gh api graphql \ + -f prId="$NODE_ID" \ + -f body="Thank you for your interest in contributing to Immich! Unfortunately this PR looks like it was generated using an LLM. As noted in our CONTRIBUTING.md, we request that you don't use LLMs to generate PRs as those are not a good use of maintainer time." \ + -f query=' + mutation CommentAndClosePR($prId: ID!, $body: String!) { + addComment(input: { + subjectId: $prId, + body: $body + }) { + __typename + } + + closePullRequest(input: { + pullRequestId: $prId + }) { + __typename + } + }' diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 109708cc6e..1695403cb4 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -17,7 +17,7 @@ If you are looking for something to work on, there are discussions and issues wi ## Use of generative AI -We generally discourage PRs entirely generated by an LLM. For any part generated by an LLM, please put extra effort into your self-review. By using generative AI without proper self-review, the time you save ends up being more work we need to put in for proper reviews and code cleanup. Please keep that in mind when submitting code by an LLM. Clearly state the use of LLMs/(generative) AI in your pull request as requested by the template. +We ask you not to open PRs generated with an LLM. We find that code generated like this tends to need a large amount of back-and-forth, which is a very inefficient use of our time. If we want LLM-generated code, it's much faster for us to use an LLM ourselves than to go through an intermediary via a pull request. ## Feature freezes diff --git a/cli/package.json b/cli/package.json index 28bee420aa..d80efdd74a 100644 --- a/cli/package.json +++ b/cli/package.json @@ -14,7 +14,7 @@ ], "devDependencies": { "@eslint/js": "^9.8.0", - "@immich/sdk": "file:../open-api/typescript-sdk", + "@immich/sdk": "workspace:*", "@types/byte-size": "^8.1.0", "@types/cli-progress": "^3.11.0", "@types/lodash-es": "^4.17.12", diff --git a/e2e/docker-compose.yml b/e2e/docker-compose.yml index a98a7013a4..5a79396aa5 100644 --- a/e2e/docker-compose.yml +++ b/e2e/docker-compose.yml @@ -53,6 +53,7 @@ services: POSTGRES_DB: immich ports: - 5435:5432 + shm_size: 128mb healthcheck: test: ['CMD-SHELL', 'pg_isready -U postgres -d immich'] interval: 1s diff --git a/e2e/package.json b/e2e/package.json index 01dd036a2f..abe46a39ca 100644 --- a/e2e/package.json +++ b/e2e/package.json @@ -21,9 +21,9 @@ "devDependencies": { "@eslint/js": "^9.8.0", "@faker-js/faker": "^10.1.0", - "@immich/cli": "file:../cli", - "@immich/e2e-auth-server": "file:../e2e-auth-server", - "@immich/sdk": "file:../open-api/typescript-sdk", + "@immich/cli": "workspace:*", + "@immich/e2e-auth-server": "workspace:*", + "@immich/sdk": "workspace:*", "@playwright/test": "^1.44.1", "@socket.io/component-emitter": "^3.1.2", "@types/luxon": "^3.4.2", diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index c65c067049..691b9d8a0c 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -45,7 +45,7 @@ importers: specifier: ^9.8.0 version: 9.39.2 '@immich/sdk': - specifier: file:../open-api/typescript-sdk + specifier: workspace:* version: link:../open-api/typescript-sdk '@types/byte-size': specifier: ^8.1.0 @@ -202,13 +202,13 @@ importers: specifier: ^10.1.0 version: 10.3.0 '@immich/cli': - specifier: file:../cli + specifier: workspace:* version: link:../cli '@immich/e2e-auth-server': - specifier: file:../e2e-auth-server + specifier: workspace:* version: link:../e2e-auth-server '@immich/sdk': - specifier: file:../open-api/typescript-sdk + specifier: workspace:* version: link:../open-api/typescript-sdk '@playwright/test': specifier: ^1.44.1 @@ -741,7 +741,7 @@ importers: specifier: ^0.4.3 version: 0.4.3 '@immich/sdk': - specifier: file:../open-api/typescript-sdk + specifier: workspace:* version: link:../open-api/typescript-sdk '@immich/ui': specifier: ^0.62.1 diff --git a/server/src/services/library.service.spec.ts b/server/src/services/library.service.spec.ts index 312bccf5b7..37477682d7 100644 --- a/server/src/services/library.service.spec.ts +++ b/server/src/services/library.service.spec.ts @@ -196,13 +196,14 @@ describe(LibraryService.name, () => { }); mocks.storage.checkFileExists.mockResolvedValue(true); - + mocks.storage.walk.mockResolvedValue(['/data/user1/photo.jpg']); mocks.library.get.mockResolvedValue(library); + mocks.asset.filterNewExternalAssetPaths.mockResolvedValue(['/data/user1/photo.jpg']); await sut.handleQueueSyncFiles({ id: library.id }); expect(mocks.storage.walk).toHaveBeenCalledWith({ - pathsToCrawl: [library.importPaths[1]], + pathsToWalk: [library.importPaths[1]], exclusionPatterns: [], includeHidden: false, }); @@ -236,32 +237,6 @@ describe(LibraryService.name, () => { await expect(sut.handleQueueSyncFiles({ id: library.id })).resolves.toBe(JobStatus.Skipped); }); - - it('should ignore import paths that do not exist', async () => { - const library = factory.library({ importPaths: ['/foo', '/bar'] }); - - mocks.storage.stat.mockImplementation((path): Promise => { - if (path === library.importPaths[0]) { - const error = { code: 'ENOENT' } as any; - throw error; - } - return Promise.resolve({ - isDirectory: () => true, - } as Stats); - }); - - mocks.storage.checkFileExists.mockResolvedValue(true); - - mocks.library.get.mockResolvedValue(library); - - await sut.handleQueueSyncFiles({ id: library.id }); - - expect(mocks.storage.walk).toHaveBeenCalledWith({ - pathsToCrawl: [library.importPaths[1]], - exclusionPatterns: [], - includeHidden: false, - }); - }); }); describe('handleQueueSyncAssets', () => { @@ -302,7 +277,7 @@ describe(LibraryService.name, () => { const asset = AssetFactory.create({ libraryId: library.id, isExternal: true }); mocks.library.get.mockResolvedValue(library); - mocks.storage.walk.mockImplementation(async function* generator() {}); + mocks.storage.walk.mockResolvedValue([]); mocks.library.streamAssetIds.mockReturnValue(makeStream([asset])); mocks.asset.getLibraryAssetCount.mockResolvedValue(1); mocks.asset.detectOfflineExternalAssets.mockResolvedValue({ numUpdatedRows: 0n }); diff --git a/server/src/repositories/storage.repository.spec.ts b/server/test/medium/specs/repositories/storage.repository.spec.ts similarity index 67% rename from server/src/repositories/storage.repository.spec.ts rename to server/test/medium/specs/repositories/storage.repository.spec.ts index 1583ced990..28ae46b2cf 100644 --- a/server/src/repositories/storage.repository.spec.ts +++ b/server/test/medium/specs/repositories/storage.repository.spec.ts @@ -1,8 +1,16 @@ -import mockfs from 'mock-fs'; +import { Kysely } from 'kysely'; +import fs from 'node:fs/promises'; +import os from 'node:os'; +import path from 'node:path'; import { WalkOptionsDto } from 'src/dtos/library.dto'; import { LoggingRepository } from 'src/repositories/logging.repository'; import { StorageRepository } from 'src/repositories/storage.repository'; -import { automock } from 'test/utils'; +import { DB } from 'src/schema'; +import { BaseService } from 'src/services/base.service'; +import { newMediumService } from 'test/medium.factory'; +import { getKyselyDB } from 'test/utils'; + +let defaultDatabase: Kysely; interface Test { test: string; @@ -10,7 +18,15 @@ interface Test { files: Record; } -const cwd = process.cwd(); +const createTestFiles = async (basePath: string, files: string[]) => { + await Promise.all( + files.map(async (file) => { + const fullPath = path.join(basePath, file.replace(/^\//, '')); + await fs.mkdir(path.dirname(fullPath), { recursive: true }); + await fs.writeFile(fullPath, ''); + }), + ); +}; const tests: Test[] = [ { @@ -49,6 +65,8 @@ const tests: Test[] = [ files: { '/photos/image.jpg': true, '/photos/image.tif': false, + '/photos/image.tIf': false, + '/photos/image.TIF': false, }, }, { @@ -157,17 +175,6 @@ const tests: Test[] = [ '/photos/2/image.jpg': true, }, }, - { - test: 'should return absolute paths', - options: { - pathsToWalk: ['photos'], - }, - files: { - [`${cwd}/photos/1.jpg`]: true, - [`${cwd}/photos/2.jpg`]: true, - [`/photos/3.jpg`]: false, - }, - }, { test: 'should support special characters in paths', options: { @@ -179,29 +186,54 @@ const tests: Test[] = [ }, ]; +const setup = (db?: Kysely) => { + const { ctx } = newMediumService(BaseService, { + database: db || defaultDatabase, + real: [], + mock: [LoggingRepository], + }); + return { sut: ctx.get(StorageRepository) }; +}; + +beforeAll(async () => { + defaultDatabase = await getKyselyDB(); +}); + describe(StorageRepository.name, () => { let sut: StorageRepository; beforeEach(() => { - // eslint-disable-next-line no-sparse-arrays - sut = new StorageRepository(automock(LoggingRepository, { args: [, { getEnv: () => ({}) }], strict: false })); - }); - - afterEach(() => { - mockfs.restore(); + ({ sut } = setup()); }); describe('crawl', () => { for (const { test, options, files } of tests) { - it(test, async () => { - mockfs(Object.fromEntries(Object.keys(files).map((file) => [file, '']))); + describe(test, () => { + const fileList = Object.keys(files); + let tempDir: string; - const actual = await sut.walk(options); - const expected = Object.entries(files) - .filter((entry) => entry[1]) - .map(([file]) => file); + beforeEach(async () => { + tempDir = await fs.mkdtemp(path.join(os.tmpdir(), 'immich-storage-test-')); + await createTestFiles(tempDir, fileList); + }); - expect(actual.toSorted()).toEqual(expected.toSorted()); + afterEach(async () => { + await fs.rm(tempDir, { recursive: true, force: true }); + }); + + it('returns expected files', async () => { + const adjustedOptions = { + ...options, + pathsToWalk: options.pathsToWalk.map((p) => path.join(tempDir, p.replace(/^\//, ''))), + }; + + const actual = await sut.walk(adjustedOptions); + const expected = Object.entries(files) + .filter((entry) => entry[1]) + .map(([file]) => path.join(tempDir, file.replace(/^\//, ''))); + + expect(actual.toSorted()).toEqual(expected.toSorted()); + }); }); } }); diff --git a/web/package.json b/web/package.json index e172584c5d..5b66c75029 100644 --- a/web/package.json +++ b/web/package.json @@ -26,7 +26,7 @@ "dependencies": { "@formatjs/icu-messageformat-parser": "^3.0.0", "@immich/justified-layout-wasm": "^0.4.3", - "@immich/sdk": "file:../open-api/typescript-sdk", + "@immich/sdk": "workspace:*", "@immich/ui": "^0.62.1", "@mapbox/mapbox-gl-rtl-text": "0.2.3", "@mdi/js": "^7.4.47", diff --git a/web/src/lib/components/asset-viewer/photo-viewer.svelte b/web/src/lib/components/asset-viewer/photo-viewer.svelte index 7c3ab2eb21..2101107f6e 100644 --- a/web/src/lib/components/asset-viewer/photo-viewer.svelte +++ b/web/src/lib/components/asset-viewer/photo-viewer.svelte @@ -226,7 +226,7 @@ alt={$getAltText(toTimelineAsset(asset))} class="h-full w-full {$slideshowState === SlideshowState.None ? 'object-contain' - : slideshowLookCssMapping[$slideshowLook]} checkerboard" + : slideshowLookCssMapping[$slideshowLook]}" draggable="false" /> @@ -259,8 +259,4 @@ visibility: hidden; animation: 0s linear 0.4s forwards delayedVisibility; } - .checkerboard { - background-image: conic-gradient(#808080 25%, #b0b0b0 25% 50%, #808080 50% 75%, #b0b0b0 75%); - background-size: 20px 20px; - } diff --git a/web/src/lib/managers/timeline-manager/day-group.svelte.ts b/web/src/lib/managers/timeline-manager/day-group.svelte.ts index e21e54a6e5..ba12f4bb6c 100644 --- a/web/src/lib/managers/timeline-manager/day-group.svelte.ts +++ b/web/src/lib/managers/timeline-manager/day-group.svelte.ts @@ -102,25 +102,21 @@ export class DayGroup { } runAssetCallback(ids: Set, callback: (asset: TimelineAsset) => void | { remove?: boolean }) { - if (ids.size === 0) { - return { - moveAssets: [] as MoveAsset[], - processedIds: new SvelteSet(), - unprocessedIds: ids, - changedGeometry: false, - }; - } const unprocessedIds = new SvelteSet(ids); const processedIds = new SvelteSet(); const moveAssets: MoveAsset[] = []; let changedGeometry = false; - for (const assetId of unprocessedIds) { - const index = this.viewerAssets.findIndex((viewAsset) => viewAsset.id == assetId); - if (index === -1) { + + if (ids.size === 0) { + return { moveAssets, processedIds, unprocessedIds, changedGeometry }; + } + + for (let index = this.viewerAssets.length - 1; index >= 0; index--) { + const { id: assetId, asset } = this.viewerAssets[index]; + if (!ids.has(assetId)) { continue; } - const asset = this.viewerAssets[index].asset!; const oldTime = { ...asset.localDateTime }; const callbackResult = callback(asset); let remove = (callbackResult as { remove?: boolean } | undefined)?.remove ?? false; diff --git a/web/src/lib/stores/asset-interaction.svelte.ts b/web/src/lib/stores/asset-interaction.svelte.ts index 9cfc1b2c8e..817354e619 100644 --- a/web/src/lib/stores/asset-interaction.svelte.ts +++ b/web/src/lib/stores/asset-interaction.svelte.ts @@ -1,14 +1,15 @@ import type { TimelineAsset } from '$lib/managers/timeline-manager/types'; import { user } from '$lib/stores/user.store'; import { AssetVisibility, type UserAdminResponseDto } from '@immich/sdk'; -import { SvelteSet } from 'svelte/reactivity'; +import { SvelteMap, SvelteSet } from 'svelte/reactivity'; import { fromStore } from 'svelte/store'; export class AssetInteraction { - selectedAssets = $state([]); + private selectedAssetsMap = new SvelteMap(); + selectedAssets = $derived(Array.from(this.selectedAssetsMap.values())); selectAll = $state(false); hasSelectedAsset(assetId: string) { - return this.selectedAssets.some((asset) => asset.id === assetId); + return this.selectedAssetsMap.has(assetId); } selectedGroup = new SvelteSet(); assetSelectionCandidates = $state([]); @@ -16,7 +17,7 @@ export class AssetInteraction { return this.assetSelectionCandidates.some((asset) => asset.id === assetId); } assetSelectionStart = $state(null); - selectionActive = $derived(this.selectedAssets.length > 0); + selectionActive = $derived(this.selectedAssetsMap.size > 0); private user = fromStore(user); private userId = $derived(this.user.current?.id); @@ -27,9 +28,7 @@ export class AssetInteraction { isAllUserOwned = $derived(this.selectedAssets.every((asset) => asset.ownerId === this.userId)); selectAsset(asset: TimelineAsset) { - if (!this.hasSelectedAsset(asset.id)) { - this.selectedAssets.push(asset); - } + this.selectedAssetsMap.set(asset.id, asset); } selectAssets(assets: TimelineAsset[]) { @@ -39,10 +38,7 @@ export class AssetInteraction { } removeAssetFromMultiselectGroup(assetId: string) { - const index = this.selectedAssets.findIndex((a) => a.id == assetId); - if (index !== -1) { - this.selectedAssets.splice(index, 1); - } + this.selectedAssetsMap.delete(assetId); } addGroupToMultiselectGroup(group: string) { @@ -69,7 +65,7 @@ export class AssetInteraction { this.selectAll = false; // Multi-selection - this.selectedAssets = []; + this.selectedAssetsMap.clear(); this.selectedGroup.clear(); // Range selection diff --git a/web/src/lib/utils/asset-utils.ts b/web/src/lib/utils/asset-utils.ts index 84e386d620..47df967844 100644 --- a/web/src/lib/utils/asset-utils.ts +++ b/web/src/lib/utils/asset-utils.ts @@ -4,7 +4,6 @@ import { authManager } from '$lib/managers/auth-manager.svelte'; import { downloadManager } from '$lib/managers/download-manager.svelte'; import { TimelineManager } from '$lib/managers/timeline-manager/timeline-manager.svelte'; import type { TimelineAsset } from '$lib/managers/timeline-manager/types'; -import { assetsSnapshot } from '$lib/managers/timeline-manager/utils.svelte'; import { Route } from '$lib/route'; import type { AssetInteraction } from '$lib/stores/asset-interaction.svelte'; import { preferences } from '$lib/stores/user.store'; @@ -443,13 +442,15 @@ export const selectAllAssets = async (timelineManager: TimelineManager, assetInt try { for (const monthGroup of timelineManager.months) { - await timelineManager.loadMonthGroup(monthGroup.yearMonth); + if (!monthGroup.isLoaded) { + await timelineManager.loadMonthGroup(monthGroup.yearMonth); + } if (!assetInteraction.selectAll) { assetInteraction.clearMultiselect(); break; // Cancelled } - assetInteraction.selectAssets(assetsSnapshot([...monthGroup.assetsIterator()])); + assetInteraction.selectAssets([...monthGroup.assetsIterator()]); for (const dateGroup of monthGroup.dayGroups) { assetInteraction.addGroupToMultiselectGroup(dateGroup.groupTitle);