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

refactor(mobile): device asset entity to use modified time #17064

Open
wants to merge 11 commits into
base: main
Choose a base branch
from

Conversation

shenlong-tanwen
Copy link
Member

@shenlong-tanwen shenlong-tanwen commented Mar 24, 2025

Fixes #4939
Fixes #6439 (haven't tested the iOS part of the issue, the issue in Android does gets fixed)

Description

  • The current hashing implementation of the app is follows:

    1. Get the list of assets from an album
    2. Get the existing hash of the asset from the platform specific entity - AndroidDeviceAssetSchema or IOSDeviceAssetSchema
    3. If the asset has an entry, re-use the hash and proceed to the next asset

    This approach has issues, especially when the asset is modified in-place outside immich after hashed by the app. When the asset is modified in-place and the platform does not make changes to the localID of the edited asset, our current approach proceeds to reuse the hash, even when the hash of the actual file is modified. The PR fixes this by also storing the asset's modifiedTime to the entity and compare it during lookup to reuse the hash.

  • The existing approach also relied on updating object references inside collections to pass the updated hash back to the caller. This makes it hard to reason about the code. This has been refactored to make the logic simpler and understandable.

  • Even when the localId from AssetEntity were Strings, we were using two different device asset schemas to store the hashes. One for android which converted the string IDs to int and the iOS that stored the string IDs directly. The PR refactors this to use a common entity named DeviceAssetEntitySchema. Migration handling is also added to move the hashes from the existing schemas to the new schemas.

@shenlong-tanwen shenlong-tanwen force-pushed the refactor/device-asset-entity branch from 2ceda80 to 3476f7d Compare March 26, 2025 07:32
@shenlong-tanwen shenlong-tanwen marked this pull request as ready for review March 26, 2025 07:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Android app confuses fresh pictures/videos for old non-local pictures/videos
2 participants