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

feature(mobile): sync assets, albums & users to local database on device #1759

Merged
merged 16 commits into from
Mar 3, 2023

Conversation

fyfrey
Copy link
Contributor

@fyfrey fyfrey commented Feb 14, 2023

This PR adds functionality to sync assets, albums & users from the server (remote) to the local (device) Isar database.

General principle: Server changes are synced to the local DB (never the other way around!) Syncing works by comparing ordered lists of items (assets, albums, users) where one list is the latest result from the server and the other list is the state in the local database.

  • If items only exist on the server, they are added to the local db
  • If items only exist in the local DB, they are staged for removal
  • If items exist in both lists, they are checked whether there have been changes. If yes, in case of album the assets and users are compared using the same principle

Items staged for removal are only removed if they are no longer found/referenced elsewhere (e.g. by moving them from one local album/folder to another, or by deleting an asset from your account while the asset is still present in a album shared by someone else).

The sync operations are triggered whenever the server was previously asked to get some data, e.g. in main.dart getAllAsset (whenever the app is opened) or when viewing the library/shared album page (getAllAlbums).

This does not yet enable offline usage (i.e. without network connection, you are still sent back to the login screen)

Modifies the library page to also show device-local albums (folders).

As part of the migration, it clears the JSON cache (code to be removed after a few versions).

Adds updatedAt to the UserResponseDto.

EDIT:

Reworked this PR; the DB syncing operations are now all consolidated in a new service SyncService. Further, I've rebased my work to latest main and fixed the inevitable conflicts...

Fixed the reason for the endless timeline loading bug: ApiClient authToken was overwritten when setting a new endpoint

In my testing it works quite well now and is ready for another review!

@vercel
Copy link

vercel bot commented Feb 14, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated
immich-code-coverage ✅ Ready (Inspect) Visit Preview 💬 Add your feedback Mar 3, 2023 at 11:22AM (UTC)
1 Ignored Deployment
Name Status Preview Comments Updated
immich ⬜️ Ignored (Inspect) Mar 3, 2023 at 11:22AM (UTC)

@vercel vercel bot temporarily deployed to Preview – immich-code-coverage February 15, 2023 11:06 Inactive
@alextran1502
Copy link
Contributor

Can you add the condition triggering the sync process to the summary?

Copy link
Contributor

@alextran1502 alextran1502 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the PR, I have a few question from the code and on noticeable bug I see is that when I sign out, the Asset database is clear but the Album and ExifInfo isn't clear, so that the Sharing page show the information of album from another account.

mobile/lib/shared/models/asset.dart Show resolved Hide resolved
mobile/lib/shared/services/asset.service.dart Show resolved Hide resolved
mobile/lib/utils/diff.dart Show resolved Hide resolved
@alextran1502
Copy link
Contributor

I observered an error when trying to run against the server instance using the same PR
image

@fyfrey
Copy link
Contributor Author

fyfrey commented Feb 26, 2023

I observered an error when trying to run against the server instance using the same PR image

@alextran1502 for some reason that did never occur on my device. However, I know what can cause it and fixed that by a check to open the box in any case

@fyfrey fyfrey force-pushed the dev/local-database-sync branch 2 times, most recently from 4e8ce0c to 018493d Compare February 28, 2023 09:28
@alextran1502
Copy link
Contributor

Hi @zoodyy I got a chance to run another round of testing. I noticed that the local assets are no longer shown on the timeline. Is this the desired effect of the PR?

Also, I saw a section of the albums on device, and I don't see anything in that section. Both on iOS and Android.

@fyfrey
Copy link
Contributor Author

fyfrey commented Feb 28, 2023

Hi @zoodyy I got a chance to run another round of testing. I noticed that the local assets are no longer shown on the timeline. Is this the desired effect of the PR?

Also, I saw a section of the albums on device, and I don't see anything in that section. Both on iOS and Android.

Hey @alextran1502 thanks for testing again. That is certainly not the desired effect.
Local assets should be still shown (at least they are for me), and "albums on device" lists the albums that you selected on the backup screen. I'll try to figure out what could have gone wrong on your devices.

@fyfrey
Copy link
Contributor Author

fyfrey commented Mar 1, 2023

@alextran1502 figured it out and fixed a tiny bug: only selecting "Recents" and not any individual album would result in showing/loading no device albums.

@fyfrey fyfrey force-pushed the dev/local-database-sync branch from 4d64bd4 to aa553e5 Compare March 1, 2023 14:09
Copy link
Contributor

@martyfuhry martyfuhry left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested:

  • Taking a video, it showed up the on the timeline
  • Taking a photo, it showed up on the timeline
  • Backups seem to work
  • Gallery viewer works

Anything else you'd like me to test?

@fyfrey
Copy link
Contributor Author

fyfrey commented Mar 2, 2023

Tested:

* [x]  Taking a video, it showed up the on the timeline

* [x]  Taking a photo, it showed up on the timeline

* [x]  Backups seem to work

* [x]  Gallery viewer works

Anything else you'd like me to test?

Thanks a lot for testing these! Album operations would be great as well (like add/remove assets to album, create/delete album). Also changing something on the server/web and seeing the changes reflected on the mobile. That's the main parts I can think of.

@martyfuhry
Copy link
Contributor

I tested some stuff with albums.

  • Create album on device, add photo, check to see that the album is available on the web app too (and the asset is there)
  • Deleting album on device deletes it on server, too
  • Deleting album on server deletes it from device (after refreshing the Library page by navigating elsewhere and returning)
  • Adding asset to album adds it on device (but not until I refresh the Library page by navigating elsewhere and returning)
  • Broken: Initial load of albums from server fails to populate the albums (see below)

image

But after creating a new album and adding an image, they populate:
image

@alextran1502
Copy link
Contributor

alextran1502 commented Mar 3, 2023

@zoodyy Hi Fynn, the last issue I saw was not able to show local assets if I chose the upload album as Recents on iOS, which contains everything. Can you point me to the place where the local assets and remote assets are built to show on the timeline?

This also applied to show all local albums if I selected the "Recents" album in commit 1dca3c8

Below is the log in album.service.dart, as you see, the ID on iOS doesn't contain isAll. We might need to get the album using photo_manager API here and check if the album has the isAll flag enabled

image

image

If I choose another album other than the "Recents" album, only that album will show in the section of Albums on device. And all the assets of that album will be shown on the timeline.

Let me know how I can help!

@fyfrey
Copy link
Contributor Author

fyfrey commented Mar 3, 2023

@martyfuhry many thanks for testing! After some experiments, I was able to reproduce "Initial load of albums from server fails to populate the albums". The problem was: server/remote assets are loaded independently of the server/remote albums. When the albums were loaded first, they could not show any assets. I've fixed this: Now newly added albums also load their assets from the server and add them to the DB.

Further, I've improved the robustness of the sync operations: They are now forced to happen sequentially. Before, they could run interleaved also leading to the error above (and even worse issues like duplicate key exceptions from the DB)

@fyfrey
Copy link
Contributor Author

fyfrey commented Mar 3, 2023

@alextran1502 Excellent finding. That helped me a lot to build a solution that works (should work^^) on Android and iOS. With the latest commits, the photoManager lib is asked whether the isAll / Recents is selected by the users and acted accordingly.

@fyfrey
Copy link
Contributor Author

fyfrey commented Mar 3, 2023

With the two newest commits, I've fixed the problem when swiping between assets on the main timeline and jumping to wrong assets. The asset state was not sorted by createdAt - now it is. I also applied this to the album view (sorted from old to new, as before)

@alextran1502 alextran1502 merged commit 8708867 into main Mar 3, 2023
@alextran1502 alextran1502 deleted the dev/local-database-sync branch March 3, 2023 22:38
@fyfrey fyfrey mentioned this pull request Mar 4, 2023
16 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants