Skip to content

Commit 063299f

Browse files
committed
PR comments
1 parent 8d0626c commit 063299f

6 files changed

Lines changed: 83 additions & 92 deletions

File tree

common/types/duration/duration.go

Lines changed: 23 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"encoding/json"
55
"fmt"
66
"net/url"
7+
"strings"
78
"time"
89
)
910

@@ -35,54 +36,54 @@ func (x *Duration) AsDuration() time.Duration {
3536
return x.Duration
3637
}
3738

38-
// MarshalJSON implements the json.Marshaler interface by formatting the
39-
// duration as a string according to Google Well Known Type
39+
// MarshalJSON implements the [json.Marshaler] interface by formatting the
40+
// duration as a string according to Google Well Known Type.
4041
func (d Duration) MarshalJSON() ([]byte, error) {
41-
return []byte(fmt.Sprintf(`"%s"`, d.ToWireFormat())), nil
42+
return json.Marshal(d.toWireFormat())
4243
}
4344

44-
// String returns a string representation of Duration
45-
// which follows the wire format from Google Well Known Type:
45+
// toWireFormat returns a string representation of Duration
46+
// which follows the wire format from Google Well Known Type.
4647
// a String that ends in s to indicate seconds and is preceded by
4748
// the number of seconds, with nanoseconds expressed as fractional seconds.
49+
//
4850
// https://protobuf.dev/reference/protobuf/google.protobuf/#duration
49-
func (d Duration) ToWireFormat() string {
51+
func (d Duration) toWireFormat() string {
5052
// We do not use the standard time.Duration.String() and d.Duration.Seconds()
5153
// method because they use float64 which loses precision.
5254

5355
// Get the total nanoseconds as a precise integer.
54-
nanos := d.Duration.Nanoseconds()
56+
sign := ""
57+
if d.Duration < 0 {
58+
sign = "-"
59+
d.Duration = -d.Duration
60+
}
5561

56-
// Calculate seconds and fractional nanoseconds.
57-
secs := nanos / 1_000_000_000
58-
fracNanos := nanos % 1_000_000_000
62+
sec := d.Duration / time.Second
63+
nsec := d.Duration % time.Second
5964

60-
// Format the string, ensuring the fractional part is zero-padded to 9 digits.
61-
// This correctly handles both positive and negative durations.
62-
if nanos < 0 {
63-
// For negative values, both parts should be negative.
64-
secs = -(-nanos / 1_000_000_000)
65-
fracNanos = -(-nanos % 1_000_000_000)
66-
return fmt.Sprintf(`%d.%09ds`, secs, -fracNanos)
65+
if nsec == 0 {
66+
return fmt.Sprintf("%s%ds", sign, sec)
6767
}
6868

69-
return fmt.Sprintf(`%d.%09ds`, secs, fracNanos)
69+
frac := strings.TrimRight(fmt.Sprintf("%09d", nsec), "0")
70+
return fmt.Sprintf("%s%d.%ss", sign, sec, frac)
7071
}
7172

72-
// EncodeValues implements the query.Encoder interface by formatting the
73+
// EncodeValues implements the [query.Encoder] interface by encoding the
7374
// duration as a string, like "3.3s".
7475
func (d Duration) EncodeValues(key string, v *url.Values) error {
75-
v.Set(key, d.ToWireFormat())
76+
v.Set(key, d.toWireFormat())
7677
return nil
7778
}
7879

79-
// UnmarshalJSON implements the json.Unmarshaler interface. It can parse a
80+
// UnmarshalJSON implements the [json.Unmarshaler] interface. It can parse a
8081
// duration from the Google well-known type format (e.g., "3.123s").
8182
func (d *Duration) UnmarshalJSON(b []byte) error {
8283
if d == nil {
8384
return fmt.Errorf("json.Unmarshal on nil pointer")
8485
}
85-
// Remove the quotes from the string
86+
// Remove the quotes from the string.
8687
var s string
8788
if err := json.Unmarshal(b, &s); err != nil {
8889
return err

common/types/duration/duration_test.go

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -34,32 +34,32 @@ func TestDuration_MarshalJSON(t *testing.T) {
3434
{
3535
name: "zero duration",
3636
duration: *New(0),
37-
expected: "0.000000000s",
37+
expected: "0s",
3838
},
3939
{
4040
name: "positive duration",
4141
duration: *New(5 * time.Second),
42-
expected: "5.000000000s",
42+
expected: "5s",
4343
},
4444
{
4545
name: "negative duration",
4646
duration: *New(-2 * time.Minute),
47-
expected: "-120.000000000s",
47+
expected: "-120s",
4848
},
4949
{
5050
name: "negative duration with fractional seconds",
5151
duration: *New(-2*time.Minute + 100*time.Millisecond),
52-
expected: "-119.900000000s",
52+
expected: "-119.9s",
5353
},
5454
{
5555
name: "fractional seconds",
5656
duration: *New(1500 * time.Millisecond),
57-
expected: "1.500000000s",
57+
expected: "1.5s",
5858
},
5959
{
6060
name: "large duration",
6161
duration: *New(9223372036*time.Second + 854775000*time.Nanosecond),
62-
expected: "9223372036.854775000s",
62+
expected: "9223372036.854775s",
6363
},
6464
}
6565

@@ -172,31 +172,31 @@ func TestDuration_EncodeValues(t *testing.T) {
172172
name: "zero duration",
173173
duration: *New(0),
174174
key: "duration",
175-
expected: "0.000000000s",
175+
expected: "0s",
176176
},
177177
{
178178
name: "positive duration",
179179
duration: *New(5 * time.Second),
180180
key: "timeout",
181-
expected: "5.000000000s",
181+
expected: "5s",
182182
},
183183
{
184184
name: "negative duration",
185185
duration: *New(-2 * time.Minute),
186186
key: "delay",
187-
expected: "-120.000000000s",
187+
expected: "-120s",
188188
},
189189
{
190190
name: "fractional seconds",
191191
duration: *New(1500 * time.Millisecond),
192192
key: "interval",
193-
expected: "1.500000000s",
193+
expected: "1.5s",
194194
},
195195
{
196196
name: "large duration",
197197
duration: *New(24 * time.Hour),
198198
key: "period",
199-
expected: "86400.000000000s",
199+
expected: "86400s",
200200
},
201201
}
202202

common/types/fieldmask/fieldmask.go

Lines changed: 6 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -19,19 +19,13 @@ func New(paths []string) *FieldMask {
1919
return &FieldMask{Paths: paths}
2020
}
2121

22-
// MarshalJSON implements the json.Marshaler interface by formatting the
22+
// MarshalJSON implements the [json.Marshaler] interface by formatting the
2323
// field mask as a string according to Google Well Known Type
2424
func (f FieldMask) MarshalJSON() ([]byte, error) {
25-
return []byte(fmt.Sprintf(`"%s"`, f.ToWireFormat())), nil
25+
return json.Marshal(strings.Join(f.Paths, ","))
2626
}
2727

28-
// ToWireFormat returns the field mask as a string according to Google Well Known Type
29-
// which is a comma-separated list of field names.
30-
func (f FieldMask) ToWireFormat() string {
31-
return strings.Join(f.Paths, ",")
32-
}
33-
34-
// UnmarshalJSON implements the json.Unmarshaler interface by parsing the
28+
// UnmarshalJSON implements the [json.Unmarshaler] interface by parsing the
3529
// field mask from a string according to Google Well Known Type
3630
func (f *FieldMask) UnmarshalJSON(data []byte) error {
3731
if f == nil {
@@ -52,7 +46,8 @@ func (f *FieldMask) UnmarshalJSON(data []byte) error {
5246
return nil
5347
}
5448

55-
// EncodeValues encodes the FieldMask into a url.Values object.
49+
// EncodeValues implements the [query.Encoder] interface by encoding the
50+
// field mask as a string, like "a,b,c".
5651
// If the FieldMask is nil or empty, it returns nil.
5752
// If the url.Values is nil, it returns an error.
5853
func (f *FieldMask) EncodeValues(key string, v *url.Values) error {
@@ -63,6 +58,6 @@ func (f *FieldMask) EncodeValues(key string, v *url.Values) error {
6358
return fmt.Errorf("url.Values is nil")
6459
}
6560

66-
v.Set(key, f.ToWireFormat())
61+
v.Set(key, strings.Join(f.Paths, ","))
6762
return nil
6863
}

common/types/time/time.go

Lines changed: 14 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,10 @@ package time
22

33
import (
44
"encoding/json"
5+
"errors"
56
"fmt"
67
"net/url"
7-
stdtime "time"
8+
"time"
89
)
910

1011
// Time is a wrapper for time.Time to provide custom marshaling
@@ -19,34 +20,29 @@ import (
1920
// customTime := time.New(stdtime.Now())
2021
// goTime := customTime.AsTime() // Access the underlying stdtime.Time
2122
type Time struct {
22-
stdtime.Time
23+
time.Time
2324
}
2425

2526
// New creates a custom Time from a standard stdtime.Time.
26-
func New(t stdtime.Time) *Time {
27+
func New(t time.Time) *Time {
2728
return &Time{Time: t}
2829
}
2930

3031
// AsTime returns the underlying stdtime.Time value.
31-
func (x *Time) AsTime() stdtime.Time {
32+
func (x *Time) AsTime() time.Time {
3233
if x == nil {
33-
return stdtime.Time{}
34+
return time.Time{}
3435
}
3536
return x.Time
3637
}
3738

38-
// MarshalJSON implements the json.Marshaler interface by formatting the
39+
// MarshalJSON implements the [json.Marshaler] interface by formatting the
3940
// time as a string according to RFC3339Nano
4041
func (t Time) MarshalJSON() ([]byte, error) {
41-
return []byte(fmt.Sprintf(`"%s"`, t.ToWireFormat())), nil
42+
return json.Marshal(t.Time.Format(time.RFC3339Nano))
4243
}
4344

44-
// ToWireFormat returns the time as a string according to RFC3339Nano
45-
func (t Time) ToWireFormat() string {
46-
return t.Time.Format(stdtime.RFC3339Nano)
47-
}
48-
49-
// UnmarshalJSON implements the json.Unmarshaler interface by parsing the
45+
// UnmarshalJSON implements the [json.Unmarshaler] interface by parsing the
5046
// time from a string according to RFC3339Nano
5147
func (t *Time) UnmarshalJSON(b []byte) error {
5248
if t == nil {
@@ -59,11 +55,10 @@ func (t *Time) UnmarshalJSON(b []byte) error {
5955
}
6056

6157
if s == "" {
62-
*t = Time{Time: stdtime.Time{}}
63-
return nil
58+
return errors.New("time is empty. It should be in RFC3339Nano format")
6459
}
6560

66-
timeValue, err := stdtime.Parse(stdtime.RFC3339Nano, s)
61+
timeValue, err := time.Parse(time.RFC3339Nano, s)
6762
if err != nil {
6863
return err
6964
}
@@ -72,9 +67,9 @@ func (t *Time) UnmarshalJSON(b []byte) error {
7267
return nil
7368
}
7469

75-
// EncodeValues implements the query.Encoder interface by formatting the
76-
// time as a string according to RFC3339Nano
70+
// EncodeValues implements the [query.Encoder] interface by encoding the
71+
// time as a string according to RFC3339Nano.
7772
func (t Time) EncodeValues(key string, v *url.Values) error {
78-
v.Set(key, t.ToWireFormat())
73+
v.Set(key, t.Time.Format(time.RFC3339Nano))
7974
return nil
8075
}

0 commit comments

Comments
 (0)