REPLACE with a filter doesn't behave as expected

[joel-mason]

Given a User with an emails payload of:

"emails": [
    {
      "value": "chuck@norris.com",
      "type": "work",
      "primary": true
    },
    {
      "value": "1234@1234.com",
      "type": "home"
    },
    {
      "value": "456@456.com",
      "type": "private"
    },
    {
      "value": "blob@example.test"
    }
  ],

When patching with the following request

{
  "schemas": [
    "urn:ietf:params:scim:api:messages:2.0:PatchOp"
  ],
  "Operations": [
    {
      "path": "emails[value eq \"chuck@norris.com\"]",
      "op": "replace",
      "value": "{\n  \"type\" : \"business\",\n  \"primary\" : false,\n  \"display\" : \"testEmail\",\n  \"value\" : \"test@example.test\",\n  \"$ref\" : \"ref\"\n}"
    }
  ]
}

I expect the outcome to be

"emails": [
    {
      "value": "test@example.com",
      "type": "business",
      "display": "testEmail",
      "primary": true
    },
    {
      "value": "1234@1234.com",
      "type": "home"
    },
    {
      "value": "456@456.com",
      "type": "private"
    },
    {
      "value": "blob@example.test"
    }
  ],

But the actual outcome is

"emails": [
    {
      "value": "test@example.test",
      "type": "business",
      "primary": false
    }
  ],

According to the wiki, it says that this should only update matching nodes from the filter https://github.com/Captain-P-Goldfish/SCIM-SDK/wiki/Patching-resources#add-3

When using add with the same filter, a new item is just added to the array, they aren't "merged" together

[Captain-P-Goldfish]

I could verify that there are even more issues than you described after writing some unit-tests to reproduce your issues. I guess this will take some time. The best to do would be some refactoring since the patch-implementations is lacking maintainability...
I will try to fix this as soon as possible

[Captain-P-Goldfish]

I would like to ask someone about another opinion and if you would be willing to give me some input here it would help a lot. I am currently struggling with my own defintions actually

{
  "schemas" : [ "urn:ietf:params:scim:api:messages:2.0:PatchOp" ],
  "Operations" : [ {
    "path" : "name[givenName eq \"work\"].familyName",
    "op" : "add",
    "value" : [ "chuck" ]
  } ]
}

add, replace

• "name" not present
  • nothing happens and a http 200 is returned. (the node should only be changed if the givenName has a specific value. Since the filter does not match, the condition is actually met)
• "name.givenName" not present
  • nothing happens and a http 200 is returned. (the node should only be changed if the givenName has a specific value. Since the filter does not match, the condition is actually met)
• "name.givenName" present and filter matches
  • the attribute "familyName" is added to the name-attribute
• "name.givenname" present and filter does not match
  • a bad request is returned for no target was found

what do you think of the first and last definition here? Currently I would say they should behave some way and throwing a bad request exception instead of returning a 200 in the first case. But I have a cloudy memory of discussing this with a colleague of mine that eventually resulted in this definition.
What would you expect?
I am currently creating new tests that shall test the defined behaviour there and I am not sure in some cases if the behaviour should be kept like this.
Any input would be appreciated.

[BBMikolaj]

I would personally expect a 400 Bad Request, based on the pre-defined error type for a similar scenario in RFC7644:

 For HTTP status code 400 (Bad Request) responses, the following
   detail error types are defined:

noTarget
The specified "path" did not yield an attribute or attribute value that could be operated on. 
This occurs when the specified "path" value contains a filter that yields no match. 

[Captain-P-Goldfish]

This will be some work. And I am currently short on time. I will give my best to get it fixed soon

[Captain-P-Goldfish]

I tried my best to fix it. Please try it and let me know if anything else happened.
I also changed the behaviour. A bad request is now returned if the target is not valid

[joel-mason]

Thank you for looking into and fixing this issue Pascal, we really appreciate it. On a side note, when do you plan to release the POJO creator?

[Captain-P-Goldfish]

the POJO creator is already usable. Its actually meant as second-hand tool that can be checked out and build. Its a standalone tool with a JavaFX GUI. I never meant to upload it anywhere