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

proposal: encoding/json: add mechanism to mark fields as required #19858

Closed
mrajashree opened this issue Apr 5, 2017 · 27 comments
Closed

proposal: encoding/json: add mechanism to mark fields as required #19858

mrajashree opened this issue Apr 5, 2017 · 27 comments
Labels
Milestone

Comments

@mrajashree
Copy link

Please answer these questions before submitting your issue. Thanks!

What version of Go are you using (go version)?

go1.7.5

While unmarshaling json into a struct, no error is thrown if the struct doesn't contain a particular field. For example if three fields are needed in the struct and the json input provides only two, no error is thrown and the third field is just empty.
this is my example:

type Example struct {
	Name string `json:"name"`
	Place string `json:"place"`
	Date string `json:"date"`
}

func main() {
	jsonStr := []byte(`{"name":"someName","place":"somePlace"}`)
	var ex *Example
	err := json.Unmarshal(jsonStr, &ex)
	if err != nil {
		fmt.Printf("ERROR : %v\n", err)
	}
	fmt.Printf("%#v",ex)
}

Output:
&main.Example{Name:"someName", Place:"somePlace", Date:""}

Expected:
Some error indicating required field not provided

I know I can check if the field is empty after unmarshaling, but is there a way to throw error during it?

@mrajashree
Copy link
Author

Is it possible to add a required tag so not all fields need to be provided, but the necessary one would be?

@bradfitz
Copy link
Contributor

bradfitz commented Apr 5, 2017

(Similar but opposite proposal is #15314, to reject unknown fields.)

@bradfitz bradfitz changed the title Add Unmarshaling error if required field not provided encoding/json: add mechanism to mark fields as required Apr 5, 2017
@bradfitz bradfitz changed the title encoding/json: add mechanism to mark fields as required proposal: encoding/json: add mechanism to mark fields as required Apr 5, 2017
@gopherbot gopherbot added this to the Proposal milestone Apr 5, 2017
@mrajashree
Copy link
Author

@bradfitz oh, can I work on this?

@bradfitz
Copy link
Contributor

bradfitz commented Apr 5, 2017

No, not yet. We're still in the "Discuss your design" phase right now. This hasn't been approved.

@mrajashree
Copy link
Author

Ah lol! my bad

@mrajashree
Copy link
Author

Any update on this?

@edsonmedina
Copy link

+1

@mvdan
Copy link
Member

mvdan commented Dec 13, 2017

@edsonmedina
Copy link

@mvdan Fair enough. How does this work then? Who approves the proposals? Is there a way to vote or is it decided by committee?

@mvdan
Copy link
Member

mvdan commented Dec 13, 2017

@edsonmedina see https://github.com/golang/proposal#readme. You can vote with reactions above (which I believe you already did), but otherwise "+1" comments do not add to the discussion. Of course, that doesn't apply to comments that do add information or bring up new points.

If what you're asking is how soon will the proposal reviewing team will get to this one, I don't have an answer. Bear in mind that there are nearly two hundred open proposals without decision yet, so I'm not sure that the team can give reliable estimates.

@ianlancetaylor
Copy link
Contributor

We do seem to have missed this one somehow. Proposal review has gone on vacation until the new year, but I'll make sure it gets reviewed then.

@bradfitz
Copy link
Contributor

I think @rsc had said that all JSON proposals are on hold until they can be processed together.

@ddevault
Copy link

Can we establish a github label or milestone or some such so that we can (1) group all JSON proposals and (2) view the status and blockers of "processing them together"?

@mvdan
Copy link
Member

mvdan commented Jun 12, 2020

How about https://github.com/golang/go/issues?q=is%3Aissue+is%3Aopen+encoding%2Fjson+in%3Atitle+label%3Aproposal-hold? A unique label or milestone would make filtering a bit simpler, but it's easy enough to do the filter as of today.

@ddevault
Copy link

That meets goal (1), but misses the arguably more important goal (2), the latter being necessary to ascertain the status of the JSON blockers as a whole and understand the plan (or make a plan) to get things moving again.

@mvdan
Copy link
Member

mvdan commented Jun 12, 2020

Agreed. I'm one of the encoding/json maintainers, but I got active quite a bit after it was decided to freeze big API changes. As far as I understand it, the team want to focus on the big tasks at hand at the moment (mainly modules and generics), so standard library re-designs such as this one would come later. Though you should wait for Russ's reply there, as I of course don't speak on their behalf.

@ianlancetaylor
Copy link
Contributor

I agree with @mvdan that there isn't really a plan at this point. I think that someone is going to have to sit down for a while with all the JSON issues and figure out the best approach, which for all I know may wind up being an entirely new package. That's going to be a time consuming process and I don't know who is going to do it.

I'm not opposed to adding a GitHub label but I don't see how it helps with that.

I agree that the current situation is far from ideal.

@mvdan
Copy link
Member

mvdan commented Jun 12, 2020

I'd very much like to be part of that effort, though I'm not sure if I should be the one to lead it. There are people who have more experience and better judgement than me, but they are also busy with other projects.

(I wonder if we should take this to golang-dev, since it's fairly off-topic for this proposal in particular)

@ddevault
Copy link

Please Cc me if it ends up on golang-dev (Drew DeVault <[email protected]>).

I think that establishing some way of organizing the information and status of stalled proposals like this would make it easier for someone who cares about the problem to champion it, instead of twiddling their thumbs waiting for the Right Person to free up their time.

@dsnet
Copy link
Member

dsnet commented Nov 11, 2020

For a feature related to required fields, I'm somewhat amused that prior art with regard to protocol buffers hasn't been mentioned. Protocol buffers had a "required" label in proto2. Such a feature was considered a grave mistake and removed in proto3. A short explanation of the issue can be found here and here. I imagine some set of the reasons that this was a harmful for protobufs is also applicable to json.

While validating required fields is a useful feature, I believe that it is conflating serialization with validation. While there is a performance benefit to performing validation at the same time as serialization, it is still a distinctly different concept. If we allow marking a field as required, what's preventing other validation features from being added to json. For example, should we be able to specify that a string must match some regular expression?

In my opinion, features like required fields should be handled by some other package that excels in validation and is separate from json. Thus, such a feature can be used with xml, gob, or any other serialization package.

cmaglie added a commit to bugst/go-json that referenced this issue Nov 9, 2021
Also linked original upstream issue golang/go#19858
@pavelosipyants-sothebys2
Copy link

pavelosipyants-sothebys2 commented Mar 27, 2023

Instead of adding the required tag, as soon there are negative opinions about that, can it be solved at least the same way as here - #15314 - by adding smth like DisallowMissingFields flag to the decoder?

@DarkMiMolle
Copy link

I agree with @pavelosipyants-sothebys2.
Another way I would see things is to add a json.Option type we would optionally pass to our function json.Unmarshal .

package json

...

type Options struct {
    DisallowMissingFields bool
    NilEmptySlice bool
    FieldValidator map[string]func(value any) bool
}

func Unmarshal(content []byte, into any, option ...Options) error { ... }

@igorcafe
Copy link

igorcafe commented Apr 2, 2024

I also prefer the idea of adding dec.DisallowMissingFields to json.Decoder, since it can be easily extended.

Let's say you have an endpoint POST /cart and you expect to receive a:

type Cart struct {
  Address string
  ZipCode string
  Items []Item
}

type Item struct {
  ID string
  Name string
}

The user can just send an empty body or a {} and it won't fail.
You have to manually write checks for the existence of address, zip code, and for each item its id and name, and so on.

@rsc
Copy link
Contributor

rsc commented Apr 4, 2024

We already have Decoder.DisallowUnknownFields.

It sounds like the proposal is to add Decoder.DisallowMissingFields that will reject an encoded struct if every field is not explicitly listed.

This sounds unfortunate for compatibility (and even worse if used with DisallowUnknownFields), as @dsnet pointed out long ago in #19858 (comment).

@rsc
Copy link
Contributor

rsc commented Apr 4, 2024

This proposal has been added to the active column of the proposals project
and will now be reviewed at the weekly proposal review meetings.
— rsc for the proposal review group

@rsc rsc removed the Proposal-Hold label Apr 4, 2024
@rsc
Copy link
Contributor

rsc commented Apr 10, 2024

Based on the discussion above, this proposal seems like a likely decline.
— rsc for the proposal review group

@rsc
Copy link
Contributor

rsc commented Apr 24, 2024

No change in consensus, so declined.
— rsc for the proposal review group

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Declined
Development

No branches or pull requests