Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Validation breaks unmarhalling into non-empty struct #250

Open
kleijnweb opened this issue Jun 23, 2024 · 0 comments
Open

Validation breaks unmarhalling into non-empty struct #250

kleijnweb opened this issue Jun 23, 2024 · 0 comments

Comments

@kleijnweb
Copy link

By default, when you unmarshal bytes passing a struct, it will keep the existing data. This is particularly useful for PATCH updates: you fetch the data, then apply the patch, and THEN validate. Alternative would be to have separate structs for POST/PUT and PATCH which is a huge PITA for simple applications that have only 2 concerns: performance and correctness. Also on top of the need for separate schemas, you would then have to validate the result after copying over all the data from the partial struct to the full struct, which as it stands would mean marshalling and unmarshalling it, as the only way to do validation is to unmarshall. Long story short, it's completely unusable in its current state, which is a shame because it could be a huge time saver if it worked a bit better.

The generated code has 2 issues that prevents partial decodes:

  1. It first decodes into a map

Aside from the fact that this starts with an empty map (and will thus fail), this is terribly inefficient, because it marshalls the data twice. I recognize that it is impossible to distinguish between a string that is not marshalled into and one that had an empty string marshalled into it, and that if a field is required but without minLenght > 0, that is a valid value. I don't really have a solution for that.

But, the marshalling into an map also happens when the minLength is > 0 everywhere, and EVEN when there are 0 required fields. A complete waste of resources that is fixable.

Proposed fix: only decode into a map if there are properties that are required and where the zero-value is valid. Which is common but often not what people want, especially with strings. In many cases you can then get rid of the double unmarshall by fixing your schema.

  1. It unmarshalls into an empty value of the type (an alias of it)

Obviously existing values are lost when you do that.

Proposed fix:

type Plain MyType

// copy original
plain := Plain(*j)

// unmarshal into copy
if err := json.Unmarshal(b, &plain); err != nil {
	return err
}

// validate fields of `plain`, then copy the updated data back into the original
*j = MyType(plain)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant