immich-app/immich
1
0
Fork 0
mirror of https://github.com/immich-app/immich.git synced 2026-03-01 11:20:12 +03:00
Code Issues 783 Packages Projects Releases 12 Wiki Activity

[PR #25899] fix(server): handle duplicate asset checksums atomically with ON CONFLICT #18392

New Issue
Open
opened 2026-02-05 16:37:31 +03:00 by OVERLORD · 0 comments
OVERLORD commented 2026-02-05 16:37:31 +03:00
Owner
Copy Link

📋 Pull Request Information

Original PR: https://github.com/immich-app/immich/pull/25899
Author: @Oddly
Created: 2/4/2026
Status: 🔄 Open

Base: main ← Head: fix/asset-duplicate-checksum-race-condition

📝 Commits (1)

  • c7b2a2c fix(server): handle duplicate asset checksums atomically with ON CONFLICT

📊 Changes

6 files changed (+230 additions, -42 deletions)

View changed files

📝 server/src/repositories/asset.repository.ts (+31 -1)
📝 server/src/services/asset-media.service.spec.ts (+13 -13)
📝 server/src/services/asset-media.service.ts (+28 -5)
📝 server/src/services/metadata.service.ts (+20 -23)
📝 server/test/medium/specs/repositories/asset.repository.spec.ts (+137 -0)
📝 server/test/repositories/asset.repository.mock.ts (+1 -0)

📄 Description

Summary

Replaces the check-then-insert race condition in asset creation with PostgreSQL ON CONFLICT DO NOTHING on the UQ_assets_owner_checksum constraint. This eliminates the noisy constraint violation errors that spam server logs when the mobile app uploads duplicate assets.

Fixes #22742

Changes

AssetRepository (asset.repository.ts)

  • create() now uses ON CONFLICT (constraint UQ_assets_owner_checksum) DO NOTHING with executeTakeFirst(), returning undefined when the asset already exists instead of throwing
  • createStrict() (new) preserves the original throwing behavior via executeTakeFirstOrThrow(), used by createCopy() during asset replacement where a conflict is a real error
  • createAll() also uses ON CONFLICT DO NOTHING, so batch inserts silently skip duplicates

AssetMediaService (asset-media.service.ts)

  • uploadAsset() handles the undefined return from create() as a duplicate: cleans up uploaded files, returns DUPLICATE status, and skips the usage update — all without logging a constraint violation
  • createCopy() switched to createStrict() since backup copies during asset replacement should never conflict
  • Private create() method now returns { id, duplicate } for clean control flow

MetadataService (metadata.service.ts)

  • Motion photo extraction simplified from try/catch with isAssetChecksumConstraint to a straightforward if (created) / else pattern
  • Removed unused isAssetChecksumConstraint import

Tests

  • Updated duplicate handling tests from mockRejectedValue(error) to mockResolvedValue()
  • Updated replaceAsset tests from mocks.asset.create to mocks.asset.createStrict
  • Added createStrict to the asset repository mock

Verification

  • tsc --noEmit — clean
  • All 2185 tests pass (102 test files)
  • pnpm lint — clean (0 warnings, 0 errors)

🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.

## 📋 Pull Request Information **Original PR:** https://github.com/immich-app/immich/pull/25899 **Author:** [@Oddly](https://github.com/Oddly) **Created:** 2/4/2026 **Status:** 🔄 Open **Base:** `main` ← **Head:** `fix/asset-duplicate-checksum-race-condition` --- ### 📝 Commits (1) - [`c7b2a2c`](https://github.com/immich-app/immich/commit/c7b2a2c9264993d2fe6bc046da921070045f63cc) fix(server): handle duplicate asset checksums atomically with ON CONFLICT ### 📊 Changes **6 files changed** (+230 additions, -42 deletions) <details> <summary>View changed files</summary> 📝 `server/src/repositories/asset.repository.ts` (+31 -1) 📝 `server/src/services/asset-media.service.spec.ts` (+13 -13) 📝 `server/src/services/asset-media.service.ts` (+28 -5) 📝 `server/src/services/metadata.service.ts` (+20 -23) 📝 `server/test/medium/specs/repositories/asset.repository.spec.ts` (+137 -0) 📝 `server/test/repositories/asset.repository.mock.ts` (+1 -0) </details> ### 📄 Description ## Summary Replaces the check-then-insert race condition in asset creation with PostgreSQL `ON CONFLICT DO NOTHING` on the `UQ_assets_owner_checksum` constraint. This eliminates the noisy constraint violation errors that spam server logs when the mobile app uploads duplicate assets. Fixes #22742 ## Changes ### `AssetRepository` (`asset.repository.ts`) - **`create()`** now uses `ON CONFLICT (constraint UQ_assets_owner_checksum) DO NOTHING` with `executeTakeFirst()`, returning `undefined` when the asset already exists instead of throwing - **`createStrict()`** (new) preserves the original throwing behavior via `executeTakeFirstOrThrow()`, used by `createCopy()` during asset replacement where a conflict is a real error - **`createAll()`** also uses `ON CONFLICT DO NOTHING`, so batch inserts silently skip duplicates ### `AssetMediaService` (`asset-media.service.ts`) - `uploadAsset()` handles the `undefined` return from `create()` as a duplicate: cleans up uploaded files, returns `DUPLICATE` status, and skips the usage update — all without logging a constraint violation - `createCopy()` switched to `createStrict()` since backup copies during asset replacement should never conflict - Private `create()` method now returns `{ id, duplicate }` for clean control flow ### `MetadataService` (`metadata.service.ts`) - Motion photo extraction simplified from try/catch with `isAssetChecksumConstraint` to a straightforward `if (created) / else` pattern - Removed unused `isAssetChecksumConstraint` import ### Tests - Updated duplicate handling tests from `mockRejectedValue(error)` to `mockResolvedValue()` - Updated `replaceAsset` tests from `mocks.asset.create` to `mocks.asset.createStrict` - Added `createStrict` to the asset repository mock ## Verification - `tsc --noEmit` — clean - All 2185 tests pass (102 test files) - `pnpm lint` — clean (0 warnings, 0 errors) --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
OVERLORD added the pull-request label 2026-02-05 16:37:31 +03:00
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
accessibility
changelog:enhancement
changelog:security
changelog:skip
changelog:translation
cli
date-time
dependencies
documentation
external-library
format
good first issue
mobile-beta
mobile-beta
mobile-beta
needs-answer
nice to have
pull-request
Mirrored from GitHub Pull Request
 sharing
tech-debt
📱mobile
🖥️web
🗄️server
🧠machine-learning
No Label pull-request
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
OVERLORD
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: immich-app/immich#18392
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?