search

openid / sharedsignals

OpenID Shared Signals Working Group Repository
45 stars 11 forks source link

Is there enough difference between PATCH (7.1.1.3) and PUT (7.1.1.4) for Stream Config #111

Closed independentid closed 2 months ago

independentid commented 10 months ago

The only difference I can see between PUT and PATCH is that PATCH is allowed to omit values. From a processing perspective, both commands seem to work the same. E.g. server asserted values must match. This just means that PUT has more reasons to fail than PATCH if server asserted values change in the interim.

Is the primary use case that a PATCH can be done (e.g. for a web form) where there is no prior GET before PATCH? While things may be different for the client making the change request, it doesn't seem to change the logic on the stream config server's logic.

Why not just have one of PUT or PATCH since the processing is the same? Note in SCIM, there are complex multi-valued objects (e.g. group members) where using a JSON Patch like request made sense. This made SCIM PATCH fundamentally different form PUT.

FragLegs commented 10 months ago

The issue we were trying to solve by offering both PUT and PATCH is that we want to be able to ignore missing values (which we can do with PATCH) but still be able to delete a value (i.e. reset it to the transmitter's default). You can't delete values with PATCH, since PATCH ignores missing values. So offering PUT allows for that ability.

I agree that it's a pretty finicky difference, but we weren't able to align on a better solution.

independentid commented 10 months ago

I am not really seeing a difference here other than PUT won't be as usable because it requires a GET before PUT to ensure all the values match and no implied removals. I wouldn't ever use PUT then.

A more consistent approach would be:

By consistent I mean that POST and PUT work the same way for interpreting values, except that POST would create a stream id whereas PUT needs one. PATCH would be redundant.

tulshi commented 2 months ago

Since this will be a breaking change, I would like to say that we won't be doing this. Please re-open this issue if you feel we should be working on this.