Add Push Subscription Management

[ArnyminerZ]

Depends on #829

• Remember to require network connectivity in the PushRegistrationWorker builder

[rfc2822]

So now that we have a proof 😃 we should do this properly. The first and most important issue I see now is that we when syncable collections change, the PushRegistrationWorker has to be enqueued. My suggestion is:

1. Make sure that all collection DB access (CollectionDao) is done using DavCollectionRepository.
2. Now we have a centralized location that is responsible for collection changes. So we can add observer functionality that allows observers to be notified about changes in collections.
3. Add an observer that listens for changes in syncable collections and enqueues the worker.

What do you think of it? Or do you think there's an easier / better solution?

If we use this solution, I suggest to create a separate issue/PR for the observer and then only do point 3 in this PR.

[sunkup]

1. Make sure that all collection DB access (CollectionDao) is done using `DavCollectionRepository.

Yes, makes sense in any case.

2. Now we have a centralized location that is responsible for collection changes. So we can add observer functionality that allows observers to be notified about changes in collections.

I think the observer pattern is the cleanest solution in terms of separation of concerns. But instead of implementing the whole observer pattern ourselves (as in SettingsManager), would it not be easier to collect a flow of sync- and push-enabled collections?

A changing value of List<Collection> is then emitted into the flow everytime we enable/disable a collection or it's push capability changes. I know we don't need the actual collections from the flow to enqueue PushRegistrationWorker, so it feels a bit wrong to misuse the flow api for it, however it is quite tempting, since it needs much less code and therefore reduces complexity quite a bit. Any objections?

3. Add an observer that listens for changes in syncable collections and enqueues the worker.

Regardless of whether we observe/listen to the collection change in the traditional way or (mis-?) use the Flow API, the question for me remains where to do that? It works just fine to do it in App: Application and the application/global scope is right too, I guess, but I am not completely sure it should go there?

I have updated the PR with said changes. Collecting the flow could of course go into it's own class which then gets called from App, but like this, I don't feel it needs an extra PR?

[rfc2822]

I think the observer pattern is the cleanest solution in terms of separation of concerns. But instead of implementing the whole observer pattern ourselves (as in SettingsManager), would it not be easier to collect a flow of sync- and push-enabled collections?

That was my first idea, too. However I'm not convinced that it's very proper

• to register something related to collections directly at the DB/DAO and bypass the repository (which should be the central location for managing collections),
• to register that callback when the app starts (because it's not related to the app itself), and thus
• to keep it in memory all the time, even if nothing is changed in the collections.

However I'm still open for arguments why it's OK to use the flow instead ;)

3. Add an observer that listens for changes in syncable collections and enqueues the worker.

Regardless of whether we observe/listen to the collection change in the traditional way or (mis-?) use the Flow API, the question for me remains where to do that? It works just fine to do it in App: Application and the application/global scope is right too, I guess, but I am not completely sure it should go there?

My idea was a list of observers in DavCollectionRepository, and then initialize it with currently only the Push subscription worker.

[sunkup]

• to register something related to collections directly at the DB/DAO and bypass the repository (which should be the central location for managing collections),

We are not bypassing the repository. The repository we call to get the flow, gets it from the DAO.

• to register that callback when the app starts (because it's not related to the app itself), and thus

Yes, I was wondering wether we could collect from somewhere else, but I guess not.

• to keep it in memory all the time, even if nothing is changed in the collections.

Not sure that would be too big of an issue, but it's nicer of course if we don't have to do that.

My idea was a list of observers in DavCollectionRepository, and then initialize it with currently only the Push subscription worker.

Right, I tried that now. It works well and is not too much code either - as long as you don't remove the observers. Do we have to? They would be garbage collected with DavCollectionRepository, as soon as there are no usages left, right?

I will make the complete changes of pt 1 and pt 2 in a new branch 👍

1. Make sure that all collection DB access (CollectionDao) is done using `DavCollectionRepository.

Yes, makes sense in any case.

Let me correct that. I don't think anymore that using the repository for all collection access makes sense. It would mean to duplicate all the methods in collectionDao inside the collection repo, which seems like an anti pattern. Is that really what you intended?

If we really need to ensure all collection changes are being noticed, I think we should observe the source directly via flows. We could do that in the repository and then notify the subrlisteners of the repository.

[sunkup]

As discussed in the call I will:

• do minimal changes required for the repo to notify observers on changes (move only dao setters to repo for now)
• not touch the tests at all (if possible)
• try adding observers via hilt multibinding

[github-actions]

This PR/issue depends on:

• Observable collections repository #829

[rfc2822]

Looks good!

I have added code to store the URL/timestamp of the registration in the database. Later we can decide not to repeat recent registrations.

[rfc2822]

Thanks, looks good :)