Set json encoding precision

[mfridman]

Crude implementation to fix #158.

Alternatively we could switch on TimePrecision to set the precision, but this might be confusing and deferring to strconv.FormatFloat(...) with -1 seems like the right approach.

Also, what happens if someone sets the default to minutes .. should we need to account for that?

[oxisto]

Crude implementation to fix #158.

Alternatively we could switch on TimePrecision to set the precision, but this might be confusing and deferring to strconv.FormatFloat(...) with -1 seems like the right approach.

I am thinking whether we could calculate the float precision. Need to spin my head around that

Also, what happens if someone sets the default to minutes .. should we need to account for that?

No, float precision 0 should be the "minimum" precision (seconds).

[oxisto]

I have created an old school diff for a calculation approach here:

diff --git a/parser_test.go b/parser_test.go
index 87a18e5..a224fc9 100644
--- a/parser_test.go
+++ b/parser_test.go
@@ -657,24 +657,24 @@ func TestNewNumericDateMarshalJSON(t *testing.T) {
 		precision time.Duration
 	}{
 		{time.Unix(5243700879, 0), "5243700879", time.Second},
-		{time.Unix(5243700879, 0), "5243700879.000001", time.Millisecond},
+		{time.Unix(5243700879, 0), "5243700879.000", time.Millisecond},
 		{time.Unix(5243700879, 0), "5243700879.000001", time.Microsecond},
-		{time.Unix(5243700879, 0), "5243700879.000001", time.Nanosecond},
+		{time.Unix(5243700879, 0), "5243700879.000000954", time.Nanosecond},
 		//
 		{time.Unix(4239425898, 0), "4239425898", time.Second},
-		{time.Unix(4239425898, 0), "4239425898", time.Millisecond},
-		{time.Unix(4239425898, 0), "4239425898", time.Microsecond},
-		{time.Unix(4239425898, 0), "4239425898", time.Nanosecond},
+		{time.Unix(4239425898, 0), "4239425898.000", time.Millisecond},
+		{time.Unix(4239425898, 0), "4239425898.000000", time.Microsecond},
+		{time.Unix(4239425898, 0), "4239425898.000000000", time.Nanosecond},
 		//
 		{time.Unix(0, 1644285000210402000), "1644285000", time.Second},
-		{time.Unix(0, 1644285000210402000), "1644285000.2099998", time.Millisecond},
+		{time.Unix(0, 1644285000210402000), "1644285000.210", time.Millisecond},
 		{time.Unix(0, 1644285000210402000), "1644285000.210402", time.Microsecond},
-		{time.Unix(0, 1644285000210402000), "1644285000.210402", time.Nanosecond},
+		{time.Unix(0, 1644285000210402000), "1644285000.210402012", time.Nanosecond},
 		//
 		{time.Unix(0, 1644285315063096000), "1644285315", time.Second},
 		{time.Unix(0, 1644285315063096000), "1644285315.063", time.Millisecond},
 		{time.Unix(0, 1644285315063096000), "1644285315.063096", time.Microsecond},
-		{time.Unix(0, 1644285315063096000), "1644285315.063096", time.Nanosecond},
+		{time.Unix(0, 1644285315063096000), "1644285315.063096046", time.Nanosecond},
 	}

 	for i, tc := range tt {
diff --git a/types.go b/types.go
index a5ef506..0e83229 100644
--- a/types.go
+++ b/types.go
@@ -49,10 +49,8 @@ func newNumericDateFromSeconds(f float64) *NumericDate {
 // MarshalJSON is an implementation of the json.RawMessage interface and serializes the UNIX epoch
 // represented in NumericDate to a byte array, using the precision specified in TimePrecision.
 func (date NumericDate) MarshalJSON() (b []byte, err error) {
-	prec := -1
-	if TimePrecision == time.Second {
-		prec = 0
-	}
+	prec := int(math.Log10(float64(time.Second) / float64(TimePrecision)))
+
 	f := float64(date.Truncate(TimePrecision).UnixNano()) / float64(time.Second)

 	return []byte(strconv.FormatFloat(f, 'f', prec, 64)), nil

[mfridman]

I suppose this makes sense.

I personally haven't encountered a need to set the precision in any of the projects I've worked on, and second granularity (without precision) has been sufficient.

Maybe we're overthinking it, and the alternative solution is to always set the default precision to 0.

[oxisto]

Yes it does make sense. We are setting precision to microsecond and are serialising as microsecond. Sounds good for me.

[oxisto]

Minor things, but I like it :)

[oxisto]

Yes it does make sense. We are setting precision to microsecond and are serialising as microsecond. Sounds good for me.

[oxisto]

@mfridman Do you remember who had brought up the original issue of float precision (not the issue in the PR but the original move from int to float)? I would need to look it up. Might be interesting to get some feedback from them, to see if their use case still works. My gut feeling is that this won't break anything but be potentially more precise.

[mfridman]

@mfridman Do you remember who had brought up the original issue of float precision (not the issue in the PR but the original move from int to float)? I would need to look it up. Might be interesting to get some feedback from them, to see if their use case still works. My gut feeling is that this won't break anything but be potentially more precise.

Are you thinking of this merged PR?

[oxisto]

@mfridman Do you remember who had brought up the original issue of float precision (not the issue in the PR but the original move from int to float)? I would need to look it up. Might be interesting to get some feedback from them, to see if their use case still works. My gut feeling is that this won't break anything but be potentially more precise.

Are you thinking of this merged PR?

This one: #9 it was @dunglas.

[dunglas]

@oxisto the problem I fixed was when decoding JWTs issued by other libraries such as lcobucci/jwt, which use floating-point numbers by default.

This change looks good to me.