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: support struct tag for time.Format in JSON Marshaller/Unmarshaller #21990

Open
marwan-at-work opened this issue Sep 23, 2017 · 20 comments

Comments

@marwan-at-work
Copy link
Contributor

marwan-at-work commented Sep 23, 2017

I will start with the proposal as a TL;DR before explaining the motivations behind it:

Proposal

It would be quite convenient if we could have the encoding/json package understand the format of time.Time as part of the struct definition. Something like this:

type User struct {
	Name     string
	JoinDate time.Time `time:"2006-01-02"`
}

This means that when you write json.Unmarshal(data, &user), the json package will understand what format to parse/serialize the field with.

Motivation/Experience Report

Currently, there are three workarounds:

  1. type MyTime time.Time: https://goplay.space/#APZL4Rzlm_

  2. Embedding time.Time in MyTime type: https://goplay.space/#q-oKnSTtQV

  3. Implementing json.Unmarshaller on the parent struct: https://goplay.space/#mzb8MQfajl

Those workarounds are not bad. However, I've noticed that they get out of hand once your codebase starts to scale.

Imagine having hundreds of structs with hundreds of fields where many of them could have different Time types just because they have different serialization formats, and nothing else.

The time.Time package, I think, might be too opinionated to assume RFC3339 is the only acceptable format in the json.Unmarshaller interface:

*t, err = parseStrictRFC3339(data)
. (Same goes for RFC3339Nano in the json.Marshaller implementation).

I think it makes sense for it to be a default serialization format, but not the only one. The workaround is initially not bad at all. However, it gets cumbersome when your project scales.

I think this proposal shows that the above solution can make our code more declarative and less cumbersome. This echo's Rob Pike's talk in that here we can hide complexity behind a simple interface.

Lastly, this doesn't make or break my Go experience, but I personally see room for enhancement here.

@ianlancetaylor ianlancetaylor changed the title Include Struct Tag for time.Format in JSON Marshaller/Unmarshaller encoding/json: support struct tag for time.Format in JSON Marshaller/Unmarshaller Sep 24, 2017
@ianlancetaylor ianlancetaylor added the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label Sep 24, 2017
@ianlancetaylor ianlancetaylor added this to the Unplanned milestone Sep 24, 2017
@rsc
Copy link
Contributor

rsc commented Oct 23, 2017

It's an interesting idea and maybe one to consider the next time we take a chunk of json decisions to make. For now the usual idiom is the wrapper type, as you described. Going to mark this as a proposal-hold for whenever the next rethink of json is.

@rsc rsc added Proposal Proposal-Hold and removed NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. labels Oct 23, 2017
@gnodiah
Copy link

gnodiah commented Jul 27, 2018

up!

That would be convenient if it can specify time format through json tag, because formatting time is a more often behavior in most cases.

@dsnet dsnet changed the title encoding/json: support struct tag for time.Format in JSON Marshaller/Unmarshaller proposal: encoding/json: support struct tag for time.Format in JSON Marshaller/Unmarshaller Sep 22, 2018
@golang golang deleted a comment from giautm Mar 4, 2019
@penggy
Copy link

penggy commented Apr 19, 2019

The tag name maybe "layout", reuse by xml...

@grokify
Copy link

grokify commented Nov 18, 2019

Any update on this? This would be great for working with APIs, many of which have fixed time formats other than RFC-3339 date-time.

@eibrunorodrigues
Copy link

Any Updates guys?

@Abhaykumar1
Copy link

I logged an issue I have been facing because of he time.Time package, assume RFC3339 is the only acceptable format in the json.Unmarshaller interface:

When the defined time has trailing zero after the millisecond
trailingZero = "2020-02-19T07:11:23.890+00:00"
noTrailingZero = "2020-02-19T07:11:23.891+00:00"

GO removes the trailing zero. The time becomes

2020-02-19 07:11:23.89 +0000 UTC
2020-02-19 07:11:23.891 +0000 UTC

This is an issue when other consuming application tries to consume this by defining a date formatter to get time from String value. The formatter does the strict matching.

for example in java
String pattern = "yyyy-MM-dd'T'HH:mm:ss.SSSz";
SimpleDateFormat simpleDateFormat = new SimpleDateFormat(pattern);

It will throw an exception while parsing 2020-02-19 07:11:23.89 +0000 UTC while will perfectly work for 2020-02-19 07:11:23.891 +0000 UTC

The defect was closed saying I can use formatter and do parsed.Format("2006-01-02 15:04:05.000000000 -0700 MST")

It's really a painful change, every time the user will have to reformat the time given the case below

type activity struct {
ActivityDate time.Time json:"activityDate" bson:"activityDate"
}

I read the data in this struct from MongoDB by using cur.Decode(&activity)
and write the response back using the json encoder
json.NewEncoder(w).Encode(activity)

@golang golang deleted a comment from planesandunicorns Oct 23, 2020
@avinersan
Copy link

👍 on the proposal.

Hope I'm not breaking any rules here by commenting on this thread an alternative / complementary convenient way for changing a type json output.

Idea is to allow custom serialization behaviour in the marshalling /unmarshalling process without having to touch the value being (un)marshalled.

When calling json.Marshalf (new function) there could be a marshalers argument of type []TypeMarshaler that allows the caller to customise the process.

Potential TypeMarshaler interface:

type TypeMarshaler interface {
	CanHandle(t reflect.Type) bool
	MarshalJSON(v *interface{}) ([]byte, error)
}

and corresponding

type TypeUnmarshaler interface {
	CanHandle(t reflect.Type) bool
	UnmarshalJSON(v *interface{}, b []byte) error
}

The TypeMarshaler would be applied to each field in the to marshal value that satisfies the CanHandle function.

Motivation

  • decouples the type from the serialization process
  • can customize serialization on any type even if not owned
  • allows per type global formatting which in my experience is the most used - rarely there is a need to output 2 datetime formats in the same context (excluding date vs datetime)

@andig
Copy link
Contributor

andig commented Sep 5, 2021

I would like to support this request. Marshaling and Unmarshaling time (and potentially durations, though that might be more useful with ISO support) is a common task that currently requries boilerplate. Adding time format support to the json (and maybe yaml?) struct tags would be a nice simplification. I don't think we need broader research to show just how often that happens.

@andig
Copy link
Contributor

andig commented Sep 5, 2021

Going to mark this as a proposal-hold for whenever the next rethink of json is.

@rsc is there any soonish timeframe for this (it's now been 4 years)?

@nghtstr
Copy link

nghtstr commented Nov 4, 2021

@rsc I think there is enough interest in this proposal that it should warrant looking into. I can tell you first had that some structures are very complex by virtue of the necessary data required for them, and having to add a whole lot of boilerplate code just to handle a date in a particular format makes for unnecessary and bloated code. I would think that this could be a simple modification of the time.Time structure and how it handles the JSON encode/decode; not necessarily the entire JSON package. Looking at the actual code for time.Time confirms my suspicions. time.Time.MarshalJSON() ([]byte, error) seems ripe just for adding in the tag support, and defaulting to the way you have it now.

Done the correct way, this can also be used for any sort of defined output (print, encoding/decoding, etc)

@andig
Copy link
Contributor

andig commented Nov 4, 2021

It would be nice to move this proposal back into the proposal process. It seems it has very positive support looking at the OPs votes.

@shellhazard
Copy link

Absolutely support this proposal. The aux solution linked in the OP seems to be the best option for library developers to avoid exposing users to unnecessary wrapper types but it would be nice if it wasn't necessary.

@boindil
Copy link

boindil commented Mar 31, 2023

Absolutely support this proposal. The aux solution linked in the OP seems to be the best option for library developers to avoid exposing users to unnecessary wrapper types but it would be nice if it wasn't necessary.

+1 I have no idea why we still have to work around this ...

@andig
Copy link
Contributor

andig commented Mar 31, 2023

It would be nice to move this proposal back into the proposal process. It seems it has very positive support looking at the OPs votes.

@rsc given the current proposal process- could this be moved to active? Json in general could need some love and keeping these proposals on hold limits visibility. Seems there are no counter arguments except for "lets do this later" which would rather make it approved than on hold.

@ianlancetaylor
Copy link
Contributor

CC @dsnet @mvdan Any thoughts here? Thanks.

@dsnet
Copy link
Member

dsnet commented Apr 14, 2023

There's been ongoing work to look at "json" holistically.

In a prototype implementation, we've been exploring something like:

type Foo struct {
    Start time.Time `json:"start,format:DateTime"`
}

where a format tag option specifies the format to use.
We wanted to generalize the format tag to work with more than just time.Time as this provides greater flexibility into the future.

For the following Go types, one could imagine the following supported format flags:

  • time.Time supporting format:XXX, where XXX is:

    • the Go identifier name of a format constant (e.g., DateTime) or
    • a single-quoted string of the format literal (e.g., '2006-01-02 15:04:05').
  • []byte or [...]byte supporting format:XXX, where XXX could be one of:

    • base64
    • base64url
    • base32
    • base32hex
    • base16
    • hex

    which will format the field according the encoding specified in RFC 4648 of the same name.
    It could also be array to format the value a regular JSON array of unsigned integers.

  • float32 or float64 supporting format:nonfinite such that NaN, +Inf, and -Inf are encoded as the JSON strings "NaN", "Infinity", and "-Infinity".

  • []T or map[K]V supporting format:emitempty such that nil slices and maps are encoded as an empty JSON array or object instead of the current default of being a JSON null.

@andig
Copy link
Contributor

andig commented Apr 15, 2023

@dsnet much appreciated! A few more use cases I'm regularly seeing:

  • time.Time allow format:unix, unixmillis, maybe unixnano- they are very common but don't have a Go identifier (or make them Go identifiers for use in time.Format?)
  • time.Time allow format:allowempty or unmarshaling will error with empty time fields where time.Time{} would be acceptable
  • float32 or float64 something like format:nonfinitenil to encode infinities to null instead of string

It would be really great if json could get an overhaul :)

@dsnet
Copy link
Member

dsnet commented Apr 15, 2023

time.Time allow format:unix, unixmillis, maybe unixnano- they are very common but don't have a Go identifier (or make them Go identifiers for use in time.Format?)

I was recently working with JSON-based APIs (i.e., Slack and Splunk) that represented timestamps as seconds since the Unix epoch as a JSON number. I don't have great answers but:

  • I feel like we should explore extending the time.Time.Format syntax to handle formatting Unix timestamps and declare relevant format constants for this.
  • Even if we do that, it is unclear whether format:Unix should serialize as a JSON string or JSON number. All other format flags would format as a JSON string, so it would be inconsistent that this gets serialized as a JSON number.
  • We could also just a special-case in the "json" package to support format:unix (as @andig proposes) to serialize the timestamp as seconds since Unix epoch as a JSON number. The downside of this approach is that it's specific to the "json" package. That said, formatting this as a JSON number is not terribly difficult.

time.Time allow format:allowempty or unmarshaling will error with empty time fields where time.Time{} would be acceptable

I'm not sure I understand. The time.Time{} currently serializes as "0001-01-01T00:00:00Z".

float32 or float64 something like format:nonfinitenil to encode infinities to null instead of string

That's fairly particular. I haven't seen evidence for this, while I've seen evidence for formatting NaN and infinity as strings.

@andig
Copy link
Contributor

andig commented Apr 16, 2023

Even if we do that, it is unclear whether format:Unix should serialize as a JSON string or JSON number

Imho number, that's the typical format I'm seeing. If string is needed, this is already covered by json:"....,string".

We could also just a special-case in the "json" package to support

Given the type differs from other format output, it seems this should be special-cased in "json".

time.Time allow format:allowempty or unmarshaling will error with empty time fields where time.Time{} would be acceptable

I'm not sure I understand. The time.Time{} currently serializes as "0001-01-01T00:00:00Z".

I was referring to unmarshaling. That is not currently possible for the empty string, a regular use case I've seen that is also not solvable by using *time.Time if the field is present in the json:

var s struct {
	Time time.Time
}

// parsing time "" as "2006-01-02T15:04:05Z07:00": cannot parse "" as "2006"
err := json.Unmarshal([]byte(`{"time":""}`), &s)

Also omitempty only covers marshaling.

float32 or float64 something like format:nonfinitenil to encode infinities to null instead of string

That's fairly particular. I haven't seen evidence for this, while I've seen evidence for formatting NaN and infinity as strings.

Again, I find this very useful. Marshaling NaN/Inf into strings can cause issues with decoding. When interested in "valid" values I would use a float type which will break on NaN/Inf strings.

Maybe we should stick with time for this issue?

@dsnet
Copy link
Member

dsnet commented Oct 6, 2023

Hi all, we kicked off a discussion for a possible "encoding/json/v2" package that addresses the spirit of this proposal.
See the "format" struct tag option under the "Struct tag options" section.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests