huma
huma copied to clipboard
Autopatch does not work with nullable fields
Consider a simple struct with two nullable string fields:
type Foo struct {
A *string `json:"a"`
B *string `json:"b"`
}
It is evidently impossible to set these fields to nil via a PATCH with application/merge-patch+json content type. For instance, the following request will throw a 422 with a message "expected required property a to be present".
curl --request PATCH \
--url http://localhost:8888/foo/1 \
--header 'Accept: application/json, application/problem+json' \
--header 'Content-Type: application/merge-patch+json' \
--data '{
"a": null
}'
This seems like a bug in merge patch computation: the PUT body in the autogenerated GET/PUT sequence omits the a field altogether instead of setting a: null as expected.
The only way to set a to nil appears to be via a PUT of the full struct or via a json-patch (not merge-patch):
curl --request PATCH \
--url http://localhost:8888/foo/1 \
--header 'Accept: application/json, application/problem+json' \
--header 'Content-Type: application/json-patch+json' \
--data '[
{ "op": "replace", "path": "/a", "value": null }
]
This came as an unfortunate surprise as our front-end client started making use of PATCH requests.
@betaprior I dug into this a bit. I originally thought this might be a bug but now I'm not so sure. I re-read RFC 7386 JSON Merge Patch, specifically:
define MergePatch(Target, Patch):
if Patch is an Object:
if Target is not an Object:
Target = {} # Ignore the contents and set it to an empty Object
for each Name/Value pair in Patch:
if Value is null:
if Name exists in Target:
remove the Name/Value pair from Target
else:
Target[Name] = MergePatch(Target[Name], Value)
return Target
else:
return Patch
https://datatracker.ietf.org/doc/html/rfc7386#section-2
This matches my understanding of Huma's autopatch implementation, and adding in some debug statements I'm able to see that the resulting request made to the PUT endpoint indeed has the a field removed, which in turn causes the validation error because the field is required.
I'm curious what you think the correct behavior would be in this case.
One thing that you can do is make the field optional. For example this works fine: https://go.dev/play/p/EJmLwkLJ4ci
See also https://huma.rocks/features/request-validation/#optional-required
I'm curious what you think the correct behavior would be in this case.
That's a tough one. For POST or PUT requests, we can make the following choices for API behavior vis-a-vis null and/or missing fields:
(a) implicitly set omitted fields to NULL (i.e. with json:"omitempty" or required:"false")
(b) disallow omitted fields (they have to be explicitly set to NULL) (with required:"true" or without json:"omitempty")
In my APIs, I tend to favor (b) since it forces the client to be explicit about their intent, and rules out the situation where they neglect to send a field thereby unintentionally nulling it out.
However, the semantics of merge patch is, as we discovered, incompatible with this choice, since NULL means "omit this field", thus for nullable fields, only choice (a) will work.
This is unfortunate, because I was hoping to combine the safety of explicitly requiring nullable fields for POST and PUT requests with the brevity of partial updates via a merge patch, but I don't see a way of achieving this with the merge patch. Do you?
I tend to agree this doesn't seem possible with the standard merge-patch, however if you always require these fields consistently in your APIs then I would consider copying the autopatch code and modifying it slightly to leave the null values in place for the PUT operation. You don't have to exactly follow JSON merge patch - just make it useful for you and your clients.
Edit: you could also consider a shorthand patch, which differentiates between null and undefined https://github.com/danielgtaylor/shorthand?tab=readme-ov-file#patch-partial-update. This format is actually already built-in to Huma https://github.com/danielgtaylor/huma/blob/main/autopatch/autopatch.go#L249-L271.
I tend to agree this doesn't seem possible with the standard merge-patch, however if you always require these fields consistently in your APIs then I would consider copying the autopatch code and modifying it slightly to leave the
nullvalues in place for thePUToperation.
I was hesitant to mess with the behavior of a fairly popular standard (something related to the principle of least surprise) but for our use case this seems like the best approach. Would you be open to adding optional behavior to autopatch that would go something like the following:
- parse the
PATCHbody and convertnullvalues to strings, e.g.__null__(but probably user customizable, since if you intentionally choose to go the "stringly-typed" route, you should have control over magic string values) - run the json merge-patch as the autopatch currently does
- convert the resultant magic strings (
__null__in this example) back tonull - make this code path strictly optional so as not to have any risk of breakage or unexpected behavior
I quickly prototyped this and it seems fairly straightforward and works as expected for my use case.
See https://github.com/danielgtaylor/huma/pull/791 for our implementation of the alternative autopatch behavior.