Skip to content

Conversation

megahirt
Copy link
Collaborator

@megahirt megahirt commented Oct 9, 2021

Description

This is a change to JsonDecoder to be able to handle partial updates. This is a work in progress. The purpose of this PR work is to prove out an alternate strategy for handling delta updates as @rmunn implemented in #1204

I also added some unit tests to demonstrate partial updates to arrays.

Type of Change

  • New feature (non-breaking change which adds functionality)

Checklist:

  • I have performed a self-review of my own code
  • I have reviewed the title/description of this PR which will be used as the squashed PR commit message
  • I have commented my code, particularly in hard-to-understand areas
  • I have added tests that prove my fix is effective or that my feature works

This is a change to JsonDecoder to be able to handle partial updates.  This is a work in progress.  It currently has 11 failing unit tests.

I also add some unit tests to demostrate partial updates to arrays.

TODO:
- add more unit tests for partial updates to Maps
- fix 11 failing unit tests
I added an optional parameter to decode() that specifies is the update is to be a delta update for array and map handling.  So far, only arrays are being handled.
@megahirt
Copy link
Collaborator Author

@rmunn I realize now that the approach you have taken is reasonable given the complexities of doing array delta updates. There is no way around having a separate "delta update" on the back-end as far as I can see. I have spent more time in the JsonDecoder and added a $isDeltaUpdate flag to the decode() method, which defaults to false so that existing functionality is preserved. I have one unit test that takes advantage of the $isDeltaUpdate flag set to true to demonstrate a changed item on just one element of the array, but this is just the start.

The basic elements and strategy of your changes in #1204 seem necessary to me, namely:

  • the client must determine if changes can be sent as a deltaUpdate or a regular update (whole structure). If an array has been rearranged or had deletions, then this must be a regular update
  • the client must tell the server whether to do a delta update or a regular update. The data itself doesn't specify the type of update. I think you setup a separate RPC for the delta update. It could be done like this or, the $isDeltaUpdate flag could be passed all the way from the client into the update call.

This is still a draft PR, but tests are passing now.

TODO:

  • implement similar deltaUpdate feature for MapOf type
  • write tests for MapOf type
  • write more tests for ArrayOf type
  • write LexEntryUpdate tests to test entry update scenarios

@megahirt
Copy link
Collaborator Author

megahirt commented Nov 7, 2021

Abandoning this PR as Robin's work on delta updates supersedes this experimental work.

@megahirt megahirt closed this Nov 7, 2021
@megahirt megahirt deleted the feature/partialUpdateJsonDecoder branch March 1, 2022 09:20
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.

1 participant