chbrown / rfc6902

Complete implementation of RFC6902 in TypeScript
https://chbrown.github.io/rfc6902/
322 stars 39 forks source link

rfc6902.createPatch() removing object from array. #27

Open hafeefshaik opened 6 years ago

hafeefshaik commented 6 years ago

Actual object:

{
  "id": "06449690-3e21-43c9-0c13-08d5a8f9a4ff",
   "firstName": "Hafeef",
  "lastName": "Shaik",
  "roles": [
    "Developer",
    "Admin"
  ]
}

Later added Business Analyst and Delivery Manager roles. Modified Object:

{
  "id": "06449690-3e21-43c9-0c13-08d5a8f9a4ff",
   "firstName": "Hafeef",
  "lastName": "Shaik",
  "roles": [
    "Admin",
    "Business Analyst",
    "Delivery Manager",
    "Developer"
  ]
}

Result of rfc6902.createPatch()

[
  {
    "op": "replace",
    "path": "/roles/1",
    "value": "Business Analyst"
  },
  {
    "op": "replace",
    "path": "/roles/0",
    "value": "Admin"
  },
  {
    "op": "add",
    "path": "/roles/2",
    "value": "Delivery Manager"
  },
  {
    "op": "add",
    "path": "/roles/3",
    "value": "Developer"
  }
]

Please correct me if I am wrong, I am expecting two add operations for newly added roles, However I could see replacing new role and adding existing roles. Please guide me.

chbrown commented 6 years ago

Replace the original object's

  "roles": [
    "Developer",
    "Admin"
  ]

with

  "roles": [
    "Admin",
    "Developer"
  ]

and you'll get the result you expect.

JSON has no notion of sets, which might be part of the confusion.

The rfc6902.createPatch() does not currently (PRs welcome!) implement the "move" operation, which I should document in the README somewhere.

That said, I admit I would find it a little more intuitive if it produced operations like:

  1. remove "Developer"
  2. add "Business Analyst"
  3. add "Delivery Manager"
  4. add "Developer"

But since each operation has the same "cost", it simply runs with the first 4-operation sequence it found.

hafeefshaik commented 6 years ago

@chbrown Thanks for the suggestion.

raitucarp commented 3 years ago

@chbrown

which I should document in the README somewhere.

Can someone do a PR for that ASAP? Cause misleading and waste of time for developer who want "move" operation until discover this issue.