Skip to content

JsonDeltaFormatter.ReorderOps(..) isn't fully functional #55

Description

@besplash

I feel like the current approach isn't covering enough. Not only removing has to obey order when it comes to arrays, but also adding and replacing. Take those jsons for example that have filler null-values:

left:

{"foo": [null, {"id": "BAZ_01", "bar": [0, 1]}, null, null, null]}

right:

{"foo": [null, null, null, {"id": "BAZ_02", "bar": [2, 3, 4]}, null]}

With the current approach, you will go like this:
remove /foo/3
remove /foo/2
add /foo/2
add /foo/1
replace /foo/3/id

where the replace will fail, because after removing /3, then /2, then adding /2, then adding /1, you should end up with:
left:

{"foo": [null, null, {"id": "BAZ_01", "bar": [0, 1]}, null, null]}

instead of
left:

{"foo": [null, null, null, {"id": "BAZ_01", "bar": [0, 1]}, null]}

I fixed it locally by changing the RemoveOperationComparer to manage remove/replace/add order and using it to reorder all operations, instead of only reordering the remove operations. Also, adding has to be ordered ascending and removing has to be descending to cover all cases.

I can provide code, but I first want to know if I am missing something because I'm actually not that comfortable with foreign code yet and I did this all midnight, so it's prone to errors, I guess.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions