client: remove merge client during update#1705
Conversation
DennisPattmann5012
force-pushed
the
client-sql-manager
branch
from
2e7ac56 to
ff2a3c6
Compare
aeneasr
left a comment
aeneasr
left a comment
There was a problem hiding this comment.
Thank you! It's indeed strange that we're merging the client in the memory manager. I'm actually not quite sure if that was intended or if that's some old code.
UpdateClient is being used for PUT operations - these operations insert a 1:1 copy of the payload into the store. For "patching" a resource (like with merge) we would use the PATCH HTTP method. We don't support that at the moment but it will probably come in a future version.
Regarding this PR, I'd actually like to see the mergo removed from the memory manager to avoid confusion :)
Thank you!
|
I would like to see a I'm going to update this PR and remove the merge code. |
DennisPattmann5012
force-pushed
the
client-sql-manager
branch
from
ff2a3c6 to
4650322
Compare
DennisPattmann5012
changed the title
DennisPattmann5012
force-pushed
the
client-sql-manager
branch
from
4650322 to
5b349a0
Compare
Cool :) We will probably create an umbrella issue to decide what PATCH methodlogy we want to use across the ecosystem. One possibility would be JSON Patch: http://jsonpatch.com |
|
Tracked as: ory/meta#2 |
2 participants
Proposed changes
Fix different behaviour between
memory client update manageranddatabase client update manager.Checklist
vulnerability, I confirm that I got green light (please contact [email protected]) from the maintainers to push the changes.
Further comments
If hydra runs with memory manager the update client operation only updates the forwarded values. If hydra runs with database manager the update client operation updates all fields of the client. If you don't forward a value for a given field the field is emptied in the database.