GetDutchie / brick

An intuitive way to work with persistent data in Dart
https://getdutchie.github.io/brick/#/
295 stars 27 forks source link

Proposal: add the response details to the offline request queue #393

Open devj3ns opened 1 month ago

devj3ns commented 1 month ago

Context

I am currently adding a conflict detection system via triggers to my Postgres database, which rejects an update operation when a client who was offline before tries to sync its changes using the offline request queue, but the object was already changed meanwhile. This prevents the loss of data when synchronizing. In the above case, I would like the user to give the opportunity to decide whether to keep the local or remote changes. (In my app, it is not possible to automatically discard the local change in the case it is rejected by the remote database because this would lead to data loss).

Current situation

In cases where a request inside the offline queue is rejected by the remote provider, the offline queue gets blocked until the request is removed.

I already created a page in my app which displays all requests which are inside the offline queue so that the user can see the sync status. Now I want to give the user the option to delete requests that got rejected by the remote database because of a conflict. This would unblock the request queue and other requests could be sent to the remote provider.

Proposal

What is missing from brick to do this is the ability to see the response details. Currently, brick only increments the “attempts” number inside the offline queue when a request fails. Therefore, I would like to propose a change that the HTTP-Status-Code and body of the latest attempt is stored inside the offline queue database table as well. This way, the app could identify which request got rejected and give the user the ability to remove the request, which frees up the queue.

I am willing to create a Pull Request which extends brick to support this but wanted to get some feedback on this idea first. Especially @tshedor, what do you think of this?

tshedor commented 1 month ago

Brick intentionally doesn't support the serialization of the server response because of the extremely varied case in associating a request to a response. For example, say you're upserting 5 Pizza requests from a single customer. In your own app, you could associate 5 Pizza responses based on the customerId from each request to each response. This might work when the implementation is aware of the domain, but Brick is agnostic.

However, even if the implementation is aware of the domain, Brick needs to assume it could receives responses out of order from when the request was made (in case the requests are transmitted online, offline, online, offline, offline). So the question becomes, "How do you associate a request to a server response without controlling the server?" If you could control the server, something like an idempotency key could be used, but Brick can't guarantee the server.

The last option you could use is making the queue somewhat more stateful and storing the response immediately to a separate table using the local table's request id as a foreign key. However, in this solution, do you store the response for every request? Just the latest request? How much information would the end user need to determine if the request should be voided? And if you store these raw responses, do you need to worry about any data sanitization since the queue isn't encrypted by default (the model logic may transform strip/encrypt sensitive information)? Will disk space size become a factor? Will performance of the queue's primary function be affected?

So, overall, it's doable, but I would strongly consider the end user's interface (how they access this in their own app code) first - what's needed and how can it be accessed efficiently - and then work back to the solution. I agree that the OfflineRequestQueue class is the right place to start. I would recommend subclassing it - i.e. OfflineRequestQueueWithResponses or similar - because I'm not convinced this is necessary behavior for the average Brick user.

tshedor commented 1 month ago

the offline queue gets blocked until the request is removed.

Re this concern: in the RestRequestSqliteCacheManager there's a flag for serialProcessing. It's true by default. But if your app doesn't require requests to be processed in serial, you can set it to false and requests will be processed first by the order they're received and then by whatever request won't fail. For example:

Request 1 Request 2 Request 3 Request 4

Request 1 succeeds Go to Request 2 Request 2 fails Go to Request 3 Request 3 succeeds Go to Request 4 Request 4 succeeds Go to Request 2

devj3ns commented 1 month ago

Thanks for all this input @tshedor!

The way you describe the stateful queue (storing the response immediately in a separate table) is what I had in mind. I would store every response which has an HTTP Status Code >= 400, or every response which status code is in the reattemptForStatusCodes list. My first thought was to use the response status code or payload to determine if the request was blocked/rejected by the server, lock the request and give the user the ability to force or discard it. But as you said, it's probably the best to consider the UI first and then work back.

devj3ns commented 1 month ago

Regarding the serial processing: I am aware of the flag but the requests in my app, or to be more precise, some requests have to be processed in serial. This is due to the associations. Imagine a customers and a projects table. Every customer is associated with one or more projects. In this case, a request which creates a new customer has to be sent before a request which creates a new project. On the other hand, for two requests updating different projects, the order is not relevant. So I could not opt out of serial processing completely.