aerogear / offix

GraphQL Offline Client and Server
https://offix.dev
Apache License 2.0
758 stars 45 forks source link

Investigate how to do restore changes on the error. #565

Closed wtrocki closed 4 years ago

wtrocki commented 4 years ago

Feature Request

Currently scheduler relies on optimistic responses. We might need to copy offix-scheduler package into new one and make it work for generic use cases that do not need to restore optimistic responses.

Options

wtrocki commented 4 years ago

CC @Eunovo

wtrocki commented 4 years ago

Working on this

wtrocki commented 4 years ago

I have been playing with scheduler and it seems that it has absolutely no value for the datastore replication. The way scheduler is designed would work mostly with the generic clients.

Main problems:

The only file that I found useful for the original scheduler was IDBLocalstore but I have modified it heavily.

We need to have different:

We do not need:

TL:DR - leaving scheduler untouched. G Going to create PR with the scheduler inspired queue.

wtrocki commented 4 years ago

Investigation finished with results:

Current architecture prevents us from knowing about the changesets. We are being notified about the change that already happened and needs to be replicated.

If we need to revert this change etc we will not be able to do it. There will be two approaches we can utilize here:

1) Force server state by using getObject(id) to refresh object from server This approach is not going to work when offline and it will basically prevent from making any operation on the object till we get online (really bad)

2) Event containing diff that user applied and original object. This means that we can always revert state of the object when we get error from server etc.

I will go with approach nr2. This requires us to rewrite change LocalStore and hook to replication. Alternatively we can change update method to be effect based (instead of passing entire object) we passing fields that were changed.

@Eunovo @kingsleyzissou

Eunovo commented 4 years ago

@wtrocki In what scenarios do we need to restore/undo changes?

wtrocki commented 4 years ago

User error handler rethrown error or there is no user error handler and we cannot classify error as repeatable or.. number of attempts was exhausted

Eunovo commented 4 years ago

What happens when a Post that was created is removed and it leaves its Comments hanging?

wtrocki commented 4 years ago

Replication will also remove comments (This is cascade delete feature of the server side and has little to do with the client side)

Eunovo commented 4 years ago

@wtrocki I think my previous comment was misunderstood. I was talking about a scenario where a user creates a post and then adds comments. These changes are pushed to queue. The Post fails to replicate and was reverted. So comments will exist that have no post at least until the user is online and the comments get rejected too. This Post - Comment example seems easy enough to resolve but what if the situation was reversed and the Post had an array of comment ids and one comment was removed due to a replication error but the Post was not. It's array will contain comments that could have been removed. I'm wondering if it's our place to revert changes. Only the Dev will know the impact of each change and can actually decide to remove it. Also, it shouldn't be possible to encounter errors that are not due to Network, Conflict or App specific (in which case the user need to handle as he/she sees fit)

wtrocki commented 4 years ago

There are two solutions here.

1) Ignore completely orphan elements (as they will not be fetchable without parent in comment situation 2) (this is what will happen now) server side will not let replicate comments as well thus data will be in invalid state only for short phase of replication, but this is still not an issue because see nr 1.

Trick here is to be able to schedule replication so we will have order of the items - this means that if comments start replicating faster than post they will fail as post was not created.

We will need to cather than when researching relationships, which I totally ignored here. @Eunovo maybe you want to take look into relationships on current state - if we find no way we can do it we will need to get back to one global replication setup and remove individual configs for model.