encoding/jsonpb: add AllowPartial option to MarshalOptions and UnmarshalOptions
Added tests related to required fields and AllowPartial. I somehow
missed this before.
Change-Id: I3eb17347b1f3a99be3d65af06c4549abcc87ae39
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/169701
Reviewed-by: Damien Neil <dneil@google.com>
diff --git a/encoding/jsonpb/decode.go b/encoding/jsonpb/decode.go
index 221adc7..816ba0a 100644
--- a/encoding/jsonpb/decode.go
+++ b/encoding/jsonpb/decode.go
@@ -29,6 +29,11 @@
type UnmarshalOptions struct {
pragma.NoUnkeyedLiterals
+ // AllowPartial accepts input for messages that will result in missing
+ // required fields. If AllowPartial is false (the default), Unmarshal will
+ // return error if there are any missing required fields.
+ AllowPartial bool
+
// Resolver is the registry used for type lookups when unmarshaling extensions
// and processing Any. If Resolver is not set, unmarshaling will default to
// using protoregistry.GlobalTypes.
@@ -229,18 +234,20 @@
if err := o.unmarshalSingular(knownFields, fd); !nerr.Merge(err) {
return errors.New("%v|%q: %v", fd.FullName(), name, err)
}
- if cardinality == pref.Required {
+ if !o.AllowPartial && cardinality == pref.Required {
reqNums.Set(num)
}
}
}
- // 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()))
+ 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/decode_test.go b/encoding/jsonpb/decode_test.go
index 648a3b1..a1ad766 100644
--- a/encoding/jsonpb/decode_test.go
+++ b/encoding/jsonpb/decode_test.go
@@ -986,6 +986,190 @@
},
wantErr: true,
}, {
+ desc: "required fields not set",
+ inputMessage: &pb2.Requireds{},
+ wantErr: true,
+ }, {
+ desc: "required field set",
+ inputMessage: &pb2.PartialRequired{},
+ inputText: `{
+ "reqString": "this is required"
+}`,
+ wantMessage: &pb2.PartialRequired{
+ ReqString: scalar.String("this is required"),
+ },
+ }, {
+ desc: "required fields partially set",
+ inputMessage: &pb2.Requireds{},
+ inputText: `{
+ "reqBool": false,
+ "reqSfixed64": 42,
+ "reqString": "hello",
+ "reqEnum": "ONE"
+}`,
+ wantMessage: &pb2.Requireds{
+ ReqBool: scalar.Bool(false),
+ ReqSfixed64: scalar.Int64(42),
+ ReqString: scalar.String("hello"),
+ ReqEnum: pb2.Enum_ONE.Enum(),
+ },
+ wantErr: true,
+ }, {
+ desc: "required fields partially set with AllowPartial",
+ umo: jsonpb.UnmarshalOptions{AllowPartial: true},
+ inputMessage: &pb2.Requireds{},
+ inputText: `{
+ "reqBool": false,
+ "reqSfixed64": 42,
+ "reqString": "hello",
+ "reqEnum": "ONE"
+}`,
+ wantMessage: &pb2.Requireds{
+ ReqBool: scalar.Bool(false),
+ ReqSfixed64: scalar.Int64(42),
+ ReqString: scalar.String("hello"),
+ ReqEnum: pb2.Enum_ONE.Enum(),
+ },
+ }, {
+ desc: "required fields all set",
+ inputMessage: &pb2.Requireds{},
+ inputText: `{
+ "reqBool": false,
+ "reqSfixed64": 42,
+ "reqDouble": 1.23,
+ "reqString": "hello",
+ "reqEnum": "ONE",
+ "reqNested": {}
+}`,
+ wantMessage: &pb2.Requireds{
+ ReqBool: scalar.Bool(false),
+ ReqSfixed64: scalar.Int64(42),
+ ReqDouble: scalar.Float64(1.23),
+ ReqString: scalar.String("hello"),
+ ReqEnum: pb2.Enum_ONE.Enum(),
+ ReqNested: &pb2.Nested{},
+ },
+ }, {
+ desc: "indirect required field",
+ inputMessage: &pb2.IndirectRequired{},
+ inputText: `{
+ "optNested": {}
+}`,
+ wantMessage: &pb2.IndirectRequired{
+ OptNested: &pb2.NestedWithRequired{},
+ },
+ wantErr: true,
+ }, {
+ desc: "indirect required field with AllowPartial",
+ umo: jsonpb.UnmarshalOptions{AllowPartial: true},
+ inputMessage: &pb2.IndirectRequired{},
+ inputText: `{
+ "optNested": {}
+}`,
+ wantMessage: &pb2.IndirectRequired{
+ OptNested: &pb2.NestedWithRequired{},
+ },
+ }, {
+ desc: "indirect required field in repeated",
+ inputMessage: &pb2.IndirectRequired{},
+ inputText: `{
+ "rptNested": [
+ {"reqString": "one"},
+ {}
+ ]
+}`,
+ wantMessage: &pb2.IndirectRequired{
+ RptNested: []*pb2.NestedWithRequired{
+ {
+ ReqString: scalar.String("one"),
+ },
+ {},
+ },
+ },
+ wantErr: true,
+ }, {
+ desc: "indirect required field in repeated with AllowPartial",
+ umo: jsonpb.UnmarshalOptions{AllowPartial: true},
+ inputMessage: &pb2.IndirectRequired{},
+ inputText: `{
+ "rptNested": [
+ {"reqString": "one"},
+ {}
+ ]
+}`,
+ wantMessage: &pb2.IndirectRequired{
+ RptNested: []*pb2.NestedWithRequired{
+ {
+ ReqString: scalar.String("one"),
+ },
+ {},
+ },
+ },
+ }, {
+ desc: "indirect required field in map",
+ inputMessage: &pb2.IndirectRequired{},
+ inputText: `{
+ "strToNested": {
+ "missing": {},
+ "contains": {
+ "reqString": "here"
+ }
+ }
+}`,
+ wantMessage: &pb2.IndirectRequired{
+ StrToNested: map[string]*pb2.NestedWithRequired{
+ "missing": &pb2.NestedWithRequired{},
+ "contains": &pb2.NestedWithRequired{
+ ReqString: scalar.String("here"),
+ },
+ },
+ },
+ wantErr: true,
+ }, {
+ desc: "indirect required field in map with AllowPartial",
+ umo: jsonpb.UnmarshalOptions{AllowPartial: true},
+ inputMessage: &pb2.IndirectRequired{},
+ inputText: `{
+ "strToNested": {
+ "missing": {},
+ "contains": {
+ "reqString": "here"
+ }
+ }
+}`,
+ wantMessage: &pb2.IndirectRequired{
+ StrToNested: map[string]*pb2.NestedWithRequired{
+ "missing": &pb2.NestedWithRequired{},
+ "contains": &pb2.NestedWithRequired{
+ ReqString: scalar.String("here"),
+ },
+ },
+ },
+ }, {
+ desc: "indirect required field in oneof",
+ inputMessage: &pb2.IndirectRequired{},
+ inputText: `{
+ "oneofNested": {}
+}`,
+ wantMessage: &pb2.IndirectRequired{
+ Union: &pb2.IndirectRequired_OneofNested{
+ OneofNested: &pb2.NestedWithRequired{},
+ },
+ },
+ wantErr: true,
+ }, {
+ desc: "indirect required field in oneof with AllowPartial",
+ umo: jsonpb.UnmarshalOptions{AllowPartial: true},
+ inputMessage: &pb2.IndirectRequired{},
+ inputText: `{
+ "oneofNested": {}
+}`,
+ wantMessage: &pb2.IndirectRequired{
+ Union: &pb2.IndirectRequired_OneofNested{
+ OneofNested: &pb2.NestedWithRequired{},
+ },
+ },
+ }, {
desc: "extensions of non-repeated fields",
inputMessage: &pb2.Extensions{},
inputText: `{
diff --git a/encoding/jsonpb/encode.go b/encoding/jsonpb/encode.go
index b3a0a76..8e25183 100644
--- a/encoding/jsonpb/encode.go
+++ b/encoding/jsonpb/encode.go
@@ -28,6 +28,11 @@
type MarshalOptions struct {
pragma.NoUnkeyedLiterals
+ // AllowPartial allows messages that have missing required fields to marshal
+ // without returning an error. If AllowPartial is false (the default),
+ // Marshal will return error if there are any missing required fields.
+ AllowPartial bool
+
// If Indent is a non-empty string, it causes entries for an Array or Object
// to be preceded by the indent and trailed by a newline. Indent can only be
// composed of space or tab characters.
@@ -90,7 +95,7 @@
num := fd.Number()
if !knownFields.Has(num) {
- if fd.Cardinality() == pref.Required {
+ if !o.AllowPartial && fd.Cardinality() == pref.Required {
// Treat unset required fields as a non-fatal error.
nerr.AppendRequiredNotSet(string(fd.FullName()))
}
diff --git a/encoding/jsonpb/encode_test.go b/encoding/jsonpb/encode_test.go
index 09cb9f8..d1c151c 100644
--- a/encoding/jsonpb/encode_test.go
+++ b/encoding/jsonpb/encode_test.go
@@ -727,6 +727,167 @@
}
}`,
}, {
+ desc: "required fields not set",
+ input: &pb2.Requireds{},
+ want: `{}`,
+ wantErr: true,
+ }, {
+ desc: "required fields partially set",
+ input: &pb2.Requireds{
+ ReqBool: scalar.Bool(false),
+ ReqSfixed64: scalar.Int64(0),
+ ReqDouble: scalar.Float64(1.23),
+ ReqString: scalar.String("hello"),
+ ReqEnum: pb2.Enum_ONE.Enum(),
+ },
+ want: `{
+ "reqBool": false,
+ "reqSfixed64": "0",
+ "reqDouble": 1.23,
+ "reqString": "hello",
+ "reqEnum": "ONE"
+}`,
+ wantErr: true,
+ }, {
+ desc: "required fields not set with AllowPartial",
+ mo: jsonpb.MarshalOptions{AllowPartial: true},
+ input: &pb2.Requireds{
+ ReqBool: scalar.Bool(false),
+ ReqSfixed64: scalar.Int64(0),
+ ReqDouble: scalar.Float64(1.23),
+ ReqString: scalar.String("hello"),
+ ReqEnum: pb2.Enum_ONE.Enum(),
+ },
+ want: `{
+ "reqBool": false,
+ "reqSfixed64": "0",
+ "reqDouble": 1.23,
+ "reqString": "hello",
+ "reqEnum": "ONE"
+}`,
+ }, {
+ desc: "required fields all set",
+ input: &pb2.Requireds{
+ ReqBool: scalar.Bool(false),
+ ReqSfixed64: scalar.Int64(0),
+ ReqDouble: scalar.Float64(1.23),
+ ReqString: scalar.String("hello"),
+ ReqEnum: pb2.Enum_ONE.Enum(),
+ ReqNested: &pb2.Nested{},
+ },
+ want: `{
+ "reqBool": false,
+ "reqSfixed64": "0",
+ "reqDouble": 1.23,
+ "reqString": "hello",
+ "reqEnum": "ONE",
+ "reqNested": {}
+}`,
+ }, {
+ desc: "indirect required field",
+ input: &pb2.IndirectRequired{
+ OptNested: &pb2.NestedWithRequired{},
+ },
+ want: `{
+ "optNested": {}
+}`,
+ wantErr: true,
+ }, {
+ desc: "indirect required field with AllowPartial",
+ mo: jsonpb.MarshalOptions{AllowPartial: true},
+ input: &pb2.IndirectRequired{
+ OptNested: &pb2.NestedWithRequired{},
+ },
+ want: `{
+ "optNested": {}
+}`,
+ }, {
+ desc: "indirect required field in empty repeated",
+ input: &pb2.IndirectRequired{
+ RptNested: []*pb2.NestedWithRequired{},
+ },
+ want: `{}`,
+ }, {
+ desc: "indirect required field in repeated",
+ input: &pb2.IndirectRequired{
+ RptNested: []*pb2.NestedWithRequired{
+ &pb2.NestedWithRequired{},
+ },
+ },
+ want: `{
+ "rptNested": [
+ {}
+ ]
+}`,
+ wantErr: true,
+ }, {
+ desc: "indirect required field in repeated with AllowPartial",
+ mo: jsonpb.MarshalOptions{AllowPartial: true},
+ input: &pb2.IndirectRequired{
+ RptNested: []*pb2.NestedWithRequired{
+ &pb2.NestedWithRequired{},
+ },
+ },
+ want: `{
+ "rptNested": [
+ {}
+ ]
+}`,
+ }, {
+ desc: "indirect required field in empty map",
+ input: &pb2.IndirectRequired{
+ StrToNested: map[string]*pb2.NestedWithRequired{},
+ },
+ want: "{}",
+ }, {
+ desc: "indirect required field in map",
+ input: &pb2.IndirectRequired{
+ StrToNested: map[string]*pb2.NestedWithRequired{
+ "fail": &pb2.NestedWithRequired{},
+ },
+ },
+ want: `{
+ "strToNested": {
+ "fail": {}
+ }
+}`,
+ wantErr: true,
+ }, {
+ desc: "indirect required field in map with AllowPartial",
+ mo: jsonpb.MarshalOptions{AllowPartial: true},
+ input: &pb2.IndirectRequired{
+ StrToNested: map[string]*pb2.NestedWithRequired{
+ "fail": &pb2.NestedWithRequired{},
+ },
+ },
+ want: `{
+ "strToNested": {
+ "fail": {}
+ }
+}`,
+ }, {
+ desc: "indirect required field in oneof",
+ input: &pb2.IndirectRequired{
+ Union: &pb2.IndirectRequired_OneofNested{
+ OneofNested: &pb2.NestedWithRequired{},
+ },
+ },
+ want: `{
+ "oneofNested": {}
+}`,
+ wantErr: true,
+ }, {
+ desc: "indirect required field in oneof with AllowPartial",
+ mo: jsonpb.MarshalOptions{AllowPartial: true},
+ input: &pb2.IndirectRequired{
+ Union: &pb2.IndirectRequired_OneofNested{
+ OneofNested: &pb2.NestedWithRequired{},
+ },
+ },
+ want: `{
+ "oneofNested": {}
+}`,
+ }, {
desc: "unknown fields are ignored",
input: &pb2.Scalars{
OptString: scalar.String("no unknowns"),
