Wordpress-android: Technical solution to trigger new actions when the device goes online

[maxme]

User is offline, creates a draft, edits of published post and then saves it locally while offline. We need a technical solution to upload this draft, or remote auto-save the post when the device goes online.

We recently made some changes to upload drafts when the user refresh the post list. This is a good first step, but we want to achieve this whatever screen is shown to the user, if the app is hidden or eventually if the app is closed.

Corresponding iOS ticket

https://github.com/wordpress-mobile/WordPress-iOS/issues/11640

Affected Issues

These issues may be affected by the result of this discussion.

• #9568
• #9728

Tasks

These will need to be done after this discussion.

• [x] Create follow up issues

[malinajirka]

I think the ideal goal is to have a WorkManager which starts a service

• periodically - once a day perhaps
• after the device becomes online and a certain period elapsed since the last upload

I'd definitely consider reusing the UploadService. It works pretty well and I don't think we should try to rewrite it. Someone mentioned they'd create another service which would queue stuff for the UploadService. I have never tried it, but my gut tells me starting one service from another might bite us

• both services would need to be foreground services and we'd need to show two notifications, right?
• I'm not sure how WorkManager handles wake-locks, but I feel like we might encounter other issues if we had two services

Moreover, AFAIU we only need to start the UploadService with an intent indicating it should upload all pending items as the UploadHandlers (MediaUploadHandler, PostUploadHandler) already support queueing multiple uploads.

What we need to watch out for

• show just a single notification -> (it seems the PostUploadNotifier already handles it for us)
  
  
  
  • Should we show a number of in-progress/successful/failed uploads?
  
  
  • What happens if the user clicks on the notification?
  
  
• What do we do if one/multiple uploads fail? Do we show a single notification for all the fail uploads when the uploads fail for example 3 times in a row?

[malinajirka]

I published the comment unintentionally before I was done with it :head-bang:.

But basically one last thing I noticed while scanning the UploadService is that it does a lot of the work on the main thread. I think we should make sure to do basically everything on a background thread. Wdyt?

[maxme]

I think the ideal goal is to have a WorkManager which starts a service

Yes it seems like the best way to achieve this.

I'd definitely consider reusing the UploadService. It works pretty well and I don't think we should try to rewrite it. Someone mentioned they'd create another service which would queue stuff for the UploadService.

I thought about another service because we could use it for other parts of the app, not only for posts and pages, but also for comments or other features. Anyway I'm not a fan of making things too generic and complex for no reasons. We can do a first implementation in the UploadService first, and if we need it, we'll make it more generic or split into 2 services.

What do we do if one/multiple uploads fail?

Good point about error management, I'm not sure what's the best solution.

Also something to keep in mind: since the user won't explicitly ask for an upload they might be confused if we show some errors later when they close the app. Maybe we could do a few automatic retries before showing an error?

[shiki]

Based on my short experience with WorkManager, it is unreliable if you want to have a job executed immediately. This is corroborated by the documentation:

WorkManager is not intended for in-process background work that can safely be terminated if the app process goes away or for tasks that require immediate execution.

That may be something we should look at if we decide to use it.

@malinajirka, I'd like to hear more about how we connect WorkManager and UploadService together. For example, does this mean that call sites will no longer communicate directly with UploadService but communicate with WorkManager instead?

I think the ideal goal is to have a WorkManager which starts a service
after the device becomes online and a certain period elapsed since the last upload

@malinajirka, how do you envision this to work? Will a WorkRequest be created beforehand?

[malinajirka]

Also something to keep in mind: since the user won't explicitly ask for an upload they might be confused if we show some errors later when they close the app. Maybe we could do a few automatic retries before showing an error?

I completely agree!

Based on my short experience with WorkManager, it is unreliable if you want to have a job executed immediately. This is corroborated by the documentation:

WorkManager is not intended for in-process background work that can safely be terminated if the app process goes away or for tasks that require immediate execution.

That may be something we should look at if we decide to use it.

@malinajirka, I'd like to hear more about how we connect WorkManager and UploadService together. For example, does this mean that call sites will no longer communicate directly with UploadService but communicate with WorkManager instead?

I completely agree with everything said above. I think we should use the WorkManager just for the periodic/(*on internet connection changed) uploads. I'd still start the UploadService manually from the app when we want to start the upload immediately.

I think the ideal goal is to have a WorkManager which starts a service
after the device becomes online and a certain period elapsed since the last upload

@malinajirka, how do you envision this to work? Will a WorkRequest be created beforehand?

I haven't thought this through completely, but this is what I had in mind.

1. Create PeriodicWorkRequest which runs every 24 hours with "CONNECTED/NOT_ROAMING" NetworkType and requiresBatteryNotLow constraints.
2. When the user invokes an upload action, check internet connection:
3. Connection available: Start the UploadService manually
4. Connection not available: Create one time WorkRequest with "CONNECTED/NOT_ROAMING" NetworkType constraint.
5. (*I'd even consider keeping the on internet connection changed logic which uses ConnectionStatusLiveData when the app is in the foreground. The worst case scenario is that the WorkManager tries to start the service and it's already started or it finishes itself immediately as there won't by anything to upload.... This would depend on how much delay does the WorkRequest have after the device regains connection.)

[maxme]

I completely agree with everything said above. I think we should use the WorkManager just for the periodic/(*on internet connection changed) uploads. I'd still start the UploadService manually from the app when we want to start the upload immediately.

+1, exactly my thoughts too.

[diegoreymendez]

A few questions, so that I can extrapolate some of this discussion into the iOS issue:

Create PeriodicWorkRequest which runs every 24 hours with "CONNECTED/NOT_ROAMING" NetworkType and requiresBatteryNotLow constraints.

From your explanation I assume this is a background "job" that runs once every 24 hours, if the device is connected, not roaming and has enough battery. It checks if there are pending uploads and basically starts them if the conditions are right.

Is this correct?

When the user invokes an upload action, check internet connection:
Connection available: Start the UploadService manually
Connection not available: Create one time WorkRequest with "CONNECTED/NOT_ROAMING" NetworkType constraint.

From the discussions above it sounds to me like there's a queue of pending uploads. I assume whenever an upload fails we create a matching WorkRequest for it, which "describes" what needs to be done.

I'm making some assumptions here so feel free to clarify if I'm wrong.

Does it make more sense to have specific WorkRequests describing specific actions that failed, or does it make more sense to dynamically query our DB to figure out which things need to be uploaded / synced?

The reason I'm asking myself this question is because if it can be figured out dynamically, then we have a lot less work to do to keep data syncrhonized. There'd be less of a chance that the queue becomes corrupted, or that it fails to represent the current state of data in our DBs.

As an example of the case that's in my mind, is if the user tries to save changes to a post several times while offline. I wonder if the WorkRequests items have any chance of adding complexity, noise or trouble.

[diegoreymendez]

Some other triggers that may be relevant to trigger operations:

• When the app is opened (if online, etc).
• When the app comes back from background (this one may not be necessary depending on how things work).

[shiki]

@diegoreymendez Re: PeriodicWorkRequest. It is a scheduled repeating request that, when triggered, executes a background job. It is only triggered if the constraints that we define matches. Example constraints are internet connection availability and battery level. All constraints are defined here.

From the discussions above it sounds to me like there's a queue of pending uploads. I assume whenever an upload fails we create a matching WorkRequest for it, which "describes" what needs to be done.

This can be done in multiple ways. We can either create a WorkRequest for every upload or we can just have one WorkRequest that uploads all in the queue. I think the latter is better and more manageable. From what I can tell, there's already an upload queue in FluxC and we can use that.

I believe that ☝️ also answers the rest of your questions. 🙂

[diegoreymendez]

To clarify a bit more what in my mind, there are two ways in which we could try to resync when we go back online:

1. We could define a queue of failed operations that can be retried.
2. We could dynamically query our data to figure out what needs to be synchronized.

Option 1 is what I sense we're going for with the solution proposed above. My question was aimed at trying to understand if this was the case.

When I think of this option I wonder how it will work if, for example, a post is saved three times while offline and modified each of those times. This solution could also suffer from not being able to help with anything outside of what we record as failed operations.

Option 2 can have other disadvantages. The advantage that I see is that since it'd be basically dynamic queries it wouldn't suffer from being limited to the failed operations we record in a queue.

[shiki]

Ah, my bad. I did not understand that part. I guess I was using the word, queue, incorrectly here.

When I think of this option I wonder how it will work if, for example, a post is saved three times while offline and modified each of those times.

Based on my limited understanding of FluxC, it looks like when we need to upload a post, we create a new row in PostUploadModel. That has a primary key, which is the post local id (mId). That means if we save the post multiple times, there will still only be 1 record of the post there. Literally, it's not a queue. We use this table to query for what needs to be uploaded.

What I'm currently confused about is the existence of MediaUploadModel which seems to serve a similar purpose as PostUploadModel. However, in another class, MediaModel, we also track the mUploadState which is the same as MediaUploadModel.mUploadState. 🤷‍♂

Option 2 can have other disadvantages. The advantage that I see is that since it'd be basically dynamic queries it wouldn't suffer from being limited to the failed operations we record in a queue.

Would you mind explaining more about this? How does this work? Do we add a local-only column in a table that states its upload status like MediaModel.uploadState and query using that condition?

[diegoreymendez]

Based on my limited understanding of FluxC, it looks like when we need to upload a post, we create a new row in PostUploadModel. That has a primary key, which is the post local id (mId). That means if we save the post multiple times, there will still only be 1 record of the post there. Literally, it's not a queue. We use this table to query for what needs to be uploaded.

Is the post being saved an immutable reference? If I save the post while offline, but then go on editing, what will be saved?

Would you mind explaining more about this? How does this work?

This would be open for discussion but the basic idea is that if we can run queries against our data to know we have to take action, that'd be enough.

As a more practical example:

For iOS we create a clone of the post while editing. This clone is what we use to know if a post has local changes (to show a label in the post list) and to show the retry button.

The underlying idea is rather simple: whatever query we use to know we have to show the retry button in the posts lists is the same logic that we could use to know what needs to be uploaded. It may need to be refined, but at least for iOS it's a dynamic query (instead of a record we manually registered).

Let's be critical about what I'm proposing here, though. I'm just trying to figure out what the best approach can be.

[diegoreymendez]

Considering the title of this issue is "Technical solution to trigger new actions when the device goes online", I think I went a bit too much out of topic here! 😁

Feel free to skip my previous comment or redirect that discussion into a new thread, if it makes sense @maxme.

[shiki]

This is how I visualize what's been discussed so far. Am I on track?

What's new here is UploadStarter. I'm thinking its responsibilities will be:

• Observe for internet connection changes and trigger uploads when the device is online
• Fetch all posts and media from FluxC that should be uploaded
• Use the existing UploadService to upload them

This is similar to the LocalDraftUploadStarter class I proposed in #9774.

[shiki]

Somewhat related to this. Before we make changes to the UploadService, MediaUploadHandler, and PostUploaderHandler, I propose we make them testable first.

I created #9820.

[maxme]

What I'm currently confused about is the existence of MediaUploadModel which seems to serve a similar purpose as PostUploadModel. However, in another class, MediaModel, we also track the mUploadState which is the same as MediaUploadModel.mUploadState. 🤷‍♂

I don't remember this, and I don't want to move the upload service architecture discussion in this ticket :). But it seems to me that a MediaModel (what represent a media in the app) is used to represent local or remote media, the mUploadState there, is probably used for visual representation in the Post editor and the Media Library.

The *UploadModel objects seems to be used as temporary objects (but still persisted to keep the state if the app is restarted) to track queued post and media uploads.

This is how I visualize what's been discussed so far. Am I on track?

@shiki thanks for doing this. It looks good to me, I'd just add an arrow between Foreground Call Site to UploadStarter (what you do in #9774).

Fetch all posts and media from FluxC that should be uploaded

Yes, and here I think this can be tricky with our *UploadModel classes. Like @diegoreymendez mentioned there are 2 ways of doing it: use a queue or query our data. I like the data query approach better because we can easily find local drafts and published posts with modifications (what need to be uploaded or remotely auto-saved). But in that cases, we might have to clean up existing *UploadModel objects linked to the post/media we found as "upload-able".

[malinajirka]

Fetch all posts and media from FluxC that should be uploaded

I like the data query approach better because we can easily find local drafts and published posts with modifications (what need to be uploaded or remotely auto-saved).

💯

This is how I visualize what's been discussed so far. Am I on track?

@shiki thanks for doing this. It looks good to me, I'd just add an arrow between Foreground Call Site to UploadStarter (what you do in #9774).

Great diagram, thanks @shiki !!
The only thing I'm not sure about is whether the "Foreground call site" shouldn't create one-time WorkRequest when it detects the connection is not available. As far as I understand if the user edits a post while offline and they leave the app the "ConnectionStatusLiveData" won't be invoked and the post might wait up to 24hours for the PeriodicWorkRequest. But I guess it's not so important detail and we can add it if we encounter some issues.

[shiki]

The only thing I'm not sure about is whether the "Foreground call site" shouldn't create one-time WorkRequest when it detects the connection is not available.

@malinajirka You're right. I think there's an opportunity here for UploadStarter to use WorkRequests instead in case the app dies. That's something we can look at during the implementation.

[shiki]

Here is the updated diagram. Based on my recent comment above ☝️, this is just a guide and might change during the implementation.

[diegoreymendez]

As I was reviewing the associated iOS issue, I wondered if pull-down-to-refresh in our lists should be another one of the triggers than can run UploadStarter.

At first it sounded counter intuitive, but pull-down-to-refresh is basically our manual trigger to ask the App to realize it's back online.

[shiki]

I wondered if pull-down-to-refresh in our lists should be another one of the triggers than can run UploadStarter

@diegoreymendez That is correct. That green-colored item in the diagram, _Foreground Call Site_, includes pull-to-refresh. That is also how it is implemented in #9774.

[maxme]

Created new issues:

• Transform LocalDraftUploadStarter into the UploadStarter https://github.com/wordpress-mobile/WordPress-Android/issues/9846
• New WorkManager calling the UploadStarter to auto-upload drafts and posts https://github.com/wordpress-mobile/WordPress-Android/issues/9847
• Call the UploadStarter from specific foreground sites https://github.com/wordpress-mobile/WordPress-Android/issues/9848

And there is also one in FluxC that update the API part (we might split it):

• Add support for post/page remote autosave PUSH and FETCH: https://github.com/wordpress-mobile/WordPress-FluxC-Android/issues/1247

[shiki]

Thank you, @maxme. We can continue discussions, if any, on those issues.