proto: add IsInitialized
Move all checks for required fields into a proto.IsInitialized function.
Initial testing makes me confident that we can provide a fast-path
implementation of IsInitialized which will perform more than
acceptably. (In the degenerate-but-common case where a message
transitively contains no required fields, this check can be nearly
zero cost.)
Unifying checks into a single function provides consistent behavior
between the wire, text, and json codecs.
Performing the check after decoding eliminates the wire decoder bug
where a split message is incorrectly seen as missing required fields.
Performing the check after decoding also provides consistent and
arguably more correct behavior when the target message was partially
prepopulated.
Change-Id: I9478b7bebb263af00c0d9f66a1f26e31ff553522
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/170787
Reviewed-by: Herbie Ong <herbie@google.com>
diff --git a/encoding/jsonpb/decode.go b/encoding/jsonpb/decode.go
index 3d4a068..80700df 100644
--- a/encoding/jsonpb/decode.go
+++ b/encoding/jsonpb/decode.go
@@ -71,6 +71,10 @@
if val.Type() != json.EOF {
return unexpectedJSONError{val}
}
+
+ if !o.AllowPartial {
+ nerr.Merge(proto.IsInitialized(m))
+ }
return nerr.E
}
@@ -151,7 +155,6 @@
// unmarshalFields unmarshals the fields into the given protoreflect.Message.
func (o UnmarshalOptions) unmarshalFields(m pref.Message, skipTypeURL bool) error {
var nerr errors.NonFatal
- var reqNums set.Ints
var seenNums set.Ints
var seenOneofs set.Ints
@@ -251,21 +254,6 @@
if err := o.unmarshalSingular(knownFields, fd); !nerr.Merge(err) {
return errors.New("%v|%q: %v", fd.FullName(), name, err)
}
- if !o.AllowPartial && cardinality == pref.Required {
- reqNums.Set(num)
- }
- }
- }
-
- if !o.AllowPartial {
- // Check for any missing required fields.
- allReqNums := msgType.RequiredNumbers()
- if reqNums.Len() != allReqNums.Len() {
- for i := 0; i < allReqNums.Len(); i++ {
- if num := allReqNums.Get(i); !reqNums.Has(uint64(num)) {
- nerr.AppendRequiredNotSet(string(fieldDescs.ByNumber(num).FullName()))
- }
- }
}
}
diff --git a/encoding/jsonpb/encode.go b/encoding/jsonpb/encode.go
index 8e25183..8cb66e6 100644
--- a/encoding/jsonpb/encode.go
+++ b/encoding/jsonpb/encode.go
@@ -63,6 +63,9 @@
if !nerr.Merge(err) {
return nil, err
}
+ if !o.AllowPartial {
+ nerr.Merge(proto.IsInitialized(m))
+ }
return o.encoder.Bytes(), nerr.E
}
@@ -95,10 +98,6 @@
num := fd.Number()
if !knownFields.Has(num) {
- if !o.AllowPartial && fd.Cardinality() == pref.Required {
- // Treat unset required fields as a non-fatal error.
- nerr.AppendRequiredNotSet(string(fd.FullName()))
- }
continue
}
diff --git a/encoding/textpb/decode.go b/encoding/textpb/decode.go
index 731ee64..59c98b1 100644
--- a/encoding/textpb/decode.go
+++ b/encoding/textpb/decode.go
@@ -64,6 +64,10 @@
return err
}
+ if !o.AllowPartial {
+ nerr.Merge(proto.IsInitialized(m))
+ }
+
return nerr.E
}
@@ -102,7 +106,6 @@
fieldDescs := msgType.Fields()
reservedNames := msgType.ReservedNames()
xtTypes := knownFields.ExtensionTypes()
- var reqNums set.Ints
var seenNums set.Ints
var seenOneofs set.Ints
@@ -176,25 +179,10 @@
if err := o.unmarshalSingular(tval, fd, knownFields); !nerr.Merge(err) {
return err
}
- if !o.AllowPartial && cardinality == pref.Required {
- reqNums.Set(num)
- }
seenNums.Set(num)
}
}
- if !o.AllowPartial {
- // Check for any missing required fields.
- allReqNums := msgType.RequiredNumbers()
- if reqNums.Len() != allReqNums.Len() {
- for i := 0; i < allReqNums.Len(); i++ {
- if num := allReqNums.Get(i); !reqNums.Has(uint64(num)) {
- nerr.AppendRequiredNotSet(string(fieldDescs.ByNumber(num).FullName()))
- }
- }
- }
- }
-
return nerr.E
}
diff --git a/encoding/textpb/encode.go b/encoding/textpb/encode.go
index 2ea370f..e293143 100644
--- a/encoding/textpb/encode.go
+++ b/encoding/textpb/encode.go
@@ -63,6 +63,9 @@
if !nerr.Merge(err) {
return nil, err
}
+ if !o.AllowPartial {
+ nerr.Merge(proto.IsInitialized(m))
+ }
return b, nerr.E
}
@@ -91,10 +94,6 @@
num := fd.Number()
if !knownFields.Has(num) {
- if !o.AllowPartial && fd.Cardinality() == pref.Required {
- // Treat unset required fields as a non-fatal error.
- nerr.AppendRequiredNotSet(string(fd.FullName()))
- }
continue
}