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

Add multi-device synchronization support #2149

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
Draft

Conversation

Fmstrat
Copy link

@Fmstrat Fmstrat commented Feb 26, 2025

This PR incorporates synchronization functionality with SxncD. SxncD is a self-hostable, FOSS synchronization system built for apps that currently use file import/export with a plugin already written for Obtainium. As you can see from the screenshots below, Apps each get their own API key per user, and each App has a card representing it. The selection of apps and authentication methods can be made when setting up the server.

The best way to think of it is SxncD is for synchronization what https://ntfy.sh/ is for notifications.

Covers issue #2013.

I've been running this on multiple devices for a couple of weeks now and it's running smoothly. I'm labeling this PR for draft as I'm not sure how translations are handled for new content yet.

Overall, this PR:

  • Updates a timestamp every time settings are changed or an App is updated/added/removed
  • Synchronizes on startup or on manual refresh
  • Adheres the the import/export setting of "Include settings" for the synchronization (app only or app + settings)
  • Warns users that remote settings will overwrite local settings on first sync if the Device Name is not pre-existing (I.E. setting up a new device)

@ImranR98
Copy link
Owner

Translations are handled in the way you've done - add English in all files until someone contributes the translations (although I usually use the standardize.js script to do this automatically (ensures consistency) and automatically add AI translations from DeepL as placeholders. You can do this if you want to and have an API key, otherwise I'll do it later.

@ImranR98
Copy link
Owner

What is this core file?

@Fmstrat
Copy link
Author

Fmstrat commented Feb 28, 2025

You can do this if you want to and have an API key, otherwise I'll do it later.

I don't have one, but I'm happy to get one together if it's a pain for you to do.

What is this core file?

Doh, that was a core dump from a failed run of the flutter emulator that must have been left over from the docker feature branch when I was setting that up. Removed. Luckily it's marked binary so it's fully removed from the history (since it's 1Meg in size) with the latest commit.

@ImranR98
Copy link
Owner

I don't have one, but I'm happy to get one together if it's a pain for you to do.

Nah it's no big deal
Thanks

@ImranR98
Copy link
Owner

ImranR98 commented Mar 2, 2025

Can't seem to get this to work yet. I tried setting up an SxncD server but couldn't get OIDC auth to work due to Fmstrat/SxncD#2. Then I tried GitHub auth and that worked on the webapp side - I signed in and got an API key for Obtainium.

When I try syncing from my phone, I get an error about an "Unexpected character" in "<!DOCTYPE html... which makes sense because we don't expect an HTML response. In the server logs, we see the problem: PayloadTooLargeError: request entity too large. I only have 52 apps and my JSON export is only 173 kB so I'm not sure why this is happening.

Then I tried using the Android emulator (only 10 apps on it). This time the payload was small enough, but then I got this error: insert or update on table "app_device" violates foreign key constraint "app_device_app_id_fkey". Not sure what's causing this. My guess is that the emulator is being assigned a device ID of 1 which may have already been assigned to the phone from the failed sync earlier? Haven't actually confirmed this. The IDs should probably be random UUIDs or a hash of the user ID + device name.

Overall it seems like it needs more testing around edge cases and failure modes.

@ImranR98
Copy link
Owner

ImranR98 commented Mar 2, 2025

Also, what happens if I have, for example, version 1.1 of some app XYZ installed on one device, but version 1.2 on another? If I'm reading this right, you're syncing the app install states, which I was concerned about in #2013 (comment) - this would cause devices to show inaccurate install state info, especially for apps that do not have the "reconcile installed version..." toggle enabled. Please correct me if I'm wrong, maybe I missed something. We should be syncing the app list but not device-specific state.

@Fmstrat
Copy link
Author

Fmstrat commented Mar 5, 2025

Can't seem to get this to work yet. I tried setting up an SxncD server but couldn't get OIDC auth to work due to Fmstrat/SxncD#2. Then I tried GitHub auth and that worked on the webapp side - I signed in and got an API key for Obtainium.

Interesting, I've used it with Keycloak and a custom Node server implementation and both worked for me. Happy to help debug config if you'd like.

When I try syncing from my phone, I get an error about an "Unexpected character" in "<!DOCTYPE html... which makes sense because we don't expect an HTML response. In the server logs, we see the problem: PayloadTooLargeError: request entity too large. I only have 52 apps and my JSON export is only 173 kB so I'm not sure why this is happening.

Odd, let me try adding a bunch of apps to mine and see what happens.

Then I tried using the Android emulator (only 10 apps on it). This time the payload was small enough, but then I got this error: insert or update on table "app_device" violates foreign key constraint "app_device_app_id_fkey". Not sure what's causing this. My guess is that the emulator is being assigned a device ID of 1 which may have already been assigned to the phone from the failed sync earlier? Haven't actually confirmed this. The IDs should probably be random UUIDs or a hash of the user ID + device name.

I'm wondering if this has to do with the above error. I realize I haven't included each interaction in a transaction, so it's like the DB just got into a bad state (amateur hour on that one). Will take care of that next.

Overall it seems like it needs more testing around edge cases and failure modes.

Oddly I tried a bunch of things to break it, and have a full e2e test running. I think in the above you may have triggered a cascading issue off the doctype item. In any event, I'll find a way to reproduce and make sure that's covered in the test cases. Thanks for helping out.

Also, what happens if I have, for example, version 1.1 of some app XYZ installed on one device, but version 1.2 on another? If I'm reading this right, you're syncing the app install states, which I was concerned about in #2013 (comment) - this would cause devices to show inaccurate install state info, especially for apps that do not have the "reconcile installed version..." toggle enabled. Please correct me if I'm wrong, maybe I missed something. We should be syncing the app list but not device-specific state.

If this is an issue, it would be an issue during import/export, too, as the SxncD function is just a wrapper for those calls (using whichever device has the newest settings change or app change). As I go through the above I'll take a look by making sure I have different installed apps on each device and see how it treats it, but for clarity if a change is needed in those functions I'll probably do it in a separate PR so it's easier to review independently.

@ImranR98
Copy link
Owner

ImranR98 commented Mar 5, 2025

If this is an issue, it would be an issue during import/export, too

It is an issue with import/export for apps that have the "reconcile OS version..." toggle disabled - those app entries appear with an incorrect install status and the user has to manually correct it. But import/export is a different use case - usually infrequent so it's mostly fine as is. Syncing would happen all the time so we can't have that.

My suggestion is to modify the export JSON (before upload) to remove any data that's supposed to be device-specific (mainly the installedVersion for now, maybe there are others that make sense to also exclude later). Same for import/download - merge the incoming JSON with any existing on-device data.

@Feature-Requests
Copy link

Feature-Requests commented Mar 6, 2025

How much size does this add to the app? If this is added and it makes the app noticeably bigger then an Obtainium flavor should be released for those who don't want integrations with third-party services (SxncD in this case). Or fork Obtainium and maintain a fork since this is a lot of changes the developer needs to maintain so sync users get sync by getting your fork and people who don't want sync can use non-forked Obtainium.

@Fmstrat
Copy link
Author

Fmstrat commented Mar 6, 2025

How much size does this add to the app?

Basically nothing. As mentioned in the issue being tracked the libraries for HTTP connection are already included in Obtainium's pub list.

@Feature-Requests
Copy link

We also need to trust the owners of the server.

@Fmstrat
Copy link
Author

Fmstrat commented Mar 6, 2025

We also need to trust the owners of the server.

It is self-hosted, and e2e encrypted.

@Fmstrat
Copy link
Author

Fmstrat commented Mar 6, 2025

It is an issue with import/export for apps that have the "reconcile OS version..." toggle disabled - those app entries appear with an incorrect install status and the user has to manually correct it.

Ah, I follow now, I'll investigate parsing those values on sync.

But import/export is a different use case - usually infrequent so it's mostly fine as is. Syncing would happen all the time so we can't have that.

I will say, I have a few friends using Obtainium (4 including me), and all but one of us use Import/Export for syncing between multiple phones or a tablet. So I think the use-case is happening anyway. If you want, I can look at reconciling that option period, even during import (to accompany the below)?

My suggestion is to modify the export JSON (before upload) to remove any data that's supposed to be device-specific (mainly the installedVersion for now, maybe there are others that make sense to also exclude later). Same for import/download - merge the incoming JSON with any existing on-device data.

Good call. Will do a quick audit to see if any other options make sense, too.

@ImranR98
Copy link
Owner

ImranR98 commented Mar 6, 2025

Yeah, modifying the original import/export function is a good idea.

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