search

thsutton / aeson-diff

Generate and apply diffs between JSON documents.
BSD 2-Clause "Simplified" License
39 stars 21 forks source link

Del Operation does not appear to be testing for the presence of its Value #10

Closed srijs closed 8 years ago

srijs commented 8 years ago

Example:

Prelude Data.Aeson.Diff Data.Aeson> let x = Ins ([OKey "foo"]) (String "bar")
Prelude Data.Aeson.Diff Data.Aeson> let y = Del ([OKey "foo"]) (String "bar")
Prelude Data.Aeson.Diff Data.Aeson> let z = Del ([OKey "foo"]) (String "baz")
Prelude Data.Aeson.Diff Data.Aeson> let a = patch (Patch [x, y]) (object [])
Prelude Data.Aeson.Diff Data.Aeson> let b = patch (Patch [x, z]) (object [])

Expected behaviour:

Prelude Data.Aeson.Diff Data.Aeson> a
Object (fromList [])
Prelude Data.Aeson.Diff Data.Aeson> b
Object (fromList [("foo",String "bar")]

Actual behaviour:

Prelude Data.Aeson.Diff Data.Aeson> a
Object (fromList [])
Prelude Data.Aeson.Diff Data.Aeson> b
Object (fromList [])

Somehow I was expecting the Del operation to check whether the current value matches the changeValue and do a conditional delete.

Am I wrong in assuming this? What is the purpose of changeValue for Del?

thsutton commented 8 years ago

The changeValue on Del is present (now) for the sake of computing the change metric used to minimise patches during construction. I should have defined a datatype to represent patches independently from the internal data structure.

In the near future I'll be making a major release which changes aeson-diff to use the RFC 6902 JSON Patch format. JSON Patch does not include a value on the remove operation but instead has a separate test operation which, used together, allows a similar semantics to the traditional behaviour you were expecting.

thsutton commented 8 years ago

@srijs I've just merged a branch preparing for a 1.0 release. This replaces the original ad hoc operations and patch format with the operations and JSON encoding defined in RFC 6902.

The Rem operation still has the non-operational Value (I may change this before cutting a release) but the semantics you want can now be implemented as described in the RFC using a Tst immediately succeeded by a Rem operation :

-- | Delete a 
del :: Pointer -> Value -> Patch
del p v = Patch [ Tst p v, Rem p v ]