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>
This commit is contained in:
Herbie Ong 2019-03-27 14:47:59 -07:00
parent 822de2d88c
commit 329be5b5fc
4 changed files with 365 additions and 8 deletions

View File

@ -29,6 +29,11 @@ func Unmarshal(m proto.Message, b []byte) error {
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 @@ Loop:
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()))
}
}
}
}

View File

@ -985,6 +985,190 @@ func TestUnmarshal(t *testing.T) {
},
},
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{},

View File

@ -28,6 +28,11 @@ func Marshal(m proto.Message) ([]byte, error) {
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 @@ func (o MarshalOptions) marshalFields(m pref.Message) error {
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()))
}

View File

@ -725,6 +725,167 @@ func TestMarshal(t *testing.T) {
"strToNested": {
"nil": {}
}
}`,
}, {
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",