types/known: handle nil in IsValid and CheckValid methods

Many usages of the legacy ptypes.Timestamp and ptypes.Duration rely
on the functions implicitly returning an error for a nil message.
In order to migrate previous usages of the ptypes constructors to
the new API, we modify CheckValid to preserve similar semantics.

There is some consistency with this change in that:
	(*M)(nil).IsValid() == (*M)(nil).ProtoReflect().IsValid()
where M is Duration, Timestamp, and FieldMask.

Change-Id: I05ae152511f9875ea034e78aefb7aab18d17a318
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/238007
Reviewed-by: Damien Neil <dneil@google.com>
This commit is contained in:
Joe Tsai 2020-06-16 12:17:50 -07:00 committed by Joe Tsai
parent 44e4150b30
commit a0d1c75183
6 changed files with 54 additions and 16 deletions

View File

@ -176,8 +176,11 @@ func genMessageKnownFunctions(g *protogen.GeneratedFile, f *fileInfo, m *message
g.P("// CheckValid returns an error if the timestamp is invalid.")
g.P("// In particular, it checks whether the value represents a date that is")
g.P("// in the range of 0001-01-01T00:00:00Z to 9999-12-31T23:59:59Z inclusive.")
g.P("// An error is reported for a nil Timestamp.")
g.P("func (x *Timestamp) CheckValid() error {")
g.P(" switch x.check() {")
g.P(" case invalidNil:")
g.P(" return ", protoimplPackage.Ident("X"), ".NewError(\"invalid nil Timestamp\")")
g.P(" case invalidUnderflow:")
g.P(" return ", protoimplPackage.Ident("X"), ".NewError(\"timestamp (%v) before 0001-01-01\", x)")
g.P(" case invalidOverflow:")
@ -190,9 +193,13 @@ func genMessageKnownFunctions(g *protogen.GeneratedFile, f *fileInfo, m *message
g.P("}")
g.P()
g.P("const invalidUnderflow = 1")
g.P("const invalidOverflow = 2")
g.P("const invalidNanos = 3")
g.P("const (")
g.P(" _ = iota")
g.P(" invalidNil")
g.P(" invalidUnderflow")
g.P(" invalidOverflow")
g.P(" invalidNanos")
g.P(")")
g.P()
g.P("func (x *Timestamp) check() uint {")
@ -201,6 +208,8 @@ func genMessageKnownFunctions(g *protogen.GeneratedFile, f *fileInfo, m *message
g.P(" secs := x.GetSeconds()")
g.P(" nanos := x.GetNanos()")
g.P(" switch {")
g.P(" case x == nil:")
g.P(" return invalidNil")
g.P(" case secs < minTimestamp:")
g.P(" return invalidUnderflow")
g.P(" case secs > maxTimestamp:")
@ -255,8 +264,11 @@ func genMessageKnownFunctions(g *protogen.GeneratedFile, f *fileInfo, m *message
g.P("// CheckValid returns an error if the duration is invalid.")
g.P("// In particular, it checks whether the value is within the range of")
g.P("// -10000 years to +10000 years inclusive.")
g.P("// An error is reported for a nil Duration.")
g.P("func (x *Duration) CheckValid() error {")
g.P(" switch x.check() {")
g.P(" case invalidNil:")
g.P(" return ", protoimplPackage.Ident("X"), ".NewError(\"invalid nil Duration\")")
g.P(" case invalidUnderflow:")
g.P(" return ", protoimplPackage.Ident("X"), ".NewError(\"duration (%v) exceeds -10000 years\", x)")
g.P(" case invalidOverflow:")
@ -271,10 +283,14 @@ func genMessageKnownFunctions(g *protogen.GeneratedFile, f *fileInfo, m *message
g.P("}")
g.P()
g.P("const invalidUnderflow = 1")
g.P("const invalidOverflow = 2")
g.P("const invalidNanosRange = 3")
g.P("const invalidNanosSign = 4")
g.P("const (")
g.P(" _ = iota")
g.P(" invalidNil")
g.P(" invalidUnderflow")
g.P(" invalidOverflow")
g.P(" invalidNanosRange")
g.P(" invalidNanosSign")
g.P(")")
g.P()
g.P("func (x *Duration) check() uint {")
@ -282,6 +298,8 @@ func genMessageKnownFunctions(g *protogen.GeneratedFile, f *fileInfo, m *message
g.P(" secs := x.GetSeconds()")
g.P(" nanos := x.GetNanos()")
g.P(" switch {")
g.P(" case x == nil:")
g.P(" return invalidNil")
g.P(" case secs < -absDuration:")
g.P(" return invalidUnderflow")
g.P(" case secs > +absDuration:")
@ -587,6 +605,7 @@ func genMessageKnownFunctions(g *protogen.GeneratedFile, f *fileInfo, m *message
g.P("// IsValid reports whether all the paths are syntactically valid and")
g.P("// refer to known fields in the specified message type.")
g.P("// It reports false for a nil FieldMask.")
g.P("func (x *FieldMask) IsValid(m ", protoPackage.Ident("Message"), ") bool {")
g.P(" paths := x.GetPaths()")
g.P(" return numValidPaths(m, paths) == len(paths)")

View File

@ -158,8 +158,11 @@ func (x *Duration) IsValid() bool {
// CheckValid returns an error if the duration is invalid.
// In particular, it checks whether the value is within the range of
// -10000 years to +10000 years inclusive.
// An error is reported for a nil Duration.
func (x *Duration) CheckValid() error {
switch x.check() {
case invalidNil:
return protoimpl.X.NewError("invalid nil Duration")
case invalidUnderflow:
return protoimpl.X.NewError("duration (%v) exceeds -10000 years", x)
case invalidOverflow:
@ -173,16 +176,22 @@ func (x *Duration) CheckValid() error {
}
}
const invalidUnderflow = 1
const invalidOverflow = 2
const invalidNanosRange = 3
const invalidNanosSign = 4
const (
_ = iota
invalidNil
invalidUnderflow
invalidOverflow
invalidNanosRange
invalidNanosSign
)
func (x *Duration) check() uint {
const absDuration = 315576000000 // 10000yr * 365.25day/yr * 24hr/day * 60min/hr * 60sec/min
secs := x.GetSeconds()
nanos := x.GetNanos()
switch {
case x == nil:
return invalidNil
case secs < -absDuration:
return invalidUnderflow
case secs > +absDuration:

View File

@ -56,7 +56,7 @@ func TestFromDuration(t *testing.T) {
wantDur time.Duration
wantErr error
}{
{in: nil, wantDur: time.Duration(0)},
{in: nil, wantDur: time.Duration(0), wantErr: textError("invalid nil Duration")},
{in: new(durpb.Duration), wantDur: time.Duration(0)},
{in: &durpb.Duration{Seconds: -1, Nanos: 0}, wantDur: -time.Second},
{in: &durpb.Duration{Seconds: +1, Nanos: 0}, wantDur: +time.Second},

View File

@ -304,6 +304,7 @@ func Intersect(mx *FieldMask, my *FieldMask, ms ...*FieldMask) *FieldMask {
// IsValid reports whether all the paths are syntactically valid and
// refer to known fields in the specified message type.
// It reports false for a nil FieldMask.
func (x *FieldMask) IsValid(m proto.Message) bool {
paths := x.GetPaths()
return numValidPaths(m, paths) == len(paths)

View File

@ -165,8 +165,11 @@ func (x *Timestamp) IsValid() bool {
// CheckValid returns an error if the timestamp is invalid.
// In particular, it checks whether the value represents a date that is
// in the range of 0001-01-01T00:00:00Z to 9999-12-31T23:59:59Z inclusive.
// An error is reported for a nil Timestamp.
func (x *Timestamp) CheckValid() error {
switch x.check() {
case invalidNil:
return protoimpl.X.NewError("invalid nil Timestamp")
case invalidUnderflow:
return protoimpl.X.NewError("timestamp (%v) before 0001-01-01", x)
case invalidOverflow:
@ -178,9 +181,13 @@ func (x *Timestamp) CheckValid() error {
}
}
const invalidUnderflow = 1
const invalidOverflow = 2
const invalidNanos = 3
const (
_ = iota
invalidNil
invalidUnderflow
invalidOverflow
invalidNanos
)
func (x *Timestamp) check() uint {
const minTimestamp = -62135596800 // Seconds between 1970-01-01T00:00:00Z and 0001-01-01T00:00:00Z, inclusive
@ -188,6 +195,8 @@ func (x *Timestamp) check() uint {
secs := x.GetSeconds()
nanos := x.GetNanos()
switch {
case x == nil:
return invalidNil
case secs < minTimestamp:
return invalidUnderflow
case secs > maxTimestamp:

View File

@ -59,7 +59,7 @@ func TestFromTimestamp(t *testing.T) {
wantTime time.Time
wantErr error
}{
{in: nil, wantTime: time.Unix(0, 0)},
{in: nil, wantTime: time.Unix(0, 0), wantErr: textError("invalid nil Timestamp")},
{in: new(tspb.Timestamp), wantTime: time.Unix(0, 0)},
{in: &tspb.Timestamp{Seconds: -62135596800, Nanos: 0}, wantTime: time.Time{}},
{in: &tspb.Timestamp{Seconds: -1, Nanos: -1}, wantTime: time.Unix(-1, -1), wantErr: textError("timestamp (seconds:-1 nanos:-1) has out-of-range nanos")},