Skip to content

Commit 57c8d7b

Browse files
Multiple repeated fields are allowed (#1921)
* Multiple repeated fields are allowed The protobuffer comments have been corrected here: https://github.com/protocolbuffers/protobuf/blob/master/src/google/protobuf/field_mask.proto#L85-L86 The restriction was intended for the path string itself and not the entire mask. There can be only one repeated field in a path string, and when we get to it, the path string is done. * Fixed test * more test fixes * fixed test * fixed spaces/tabs * Update fieldmask_test.go Testing multiple repeated fields * Update fieldmask_test.go fixed test * Update fieldmask_test.go fixed indents * Update fieldmask_test.go * Update fieldmask_test.go * Update fieldmask_test.go * Update fieldmask_test.go * Update fieldmask_test.go
1 parent 61fef09 commit 57c8d7b

File tree

2 files changed

+18
-18
lines changed

2 files changed

+18
-18
lines changed

runtime/fieldmask.go

Lines changed: 3 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,6 @@ func FieldMaskFromRequestBody(r io.Reader, msg proto.Message) (*field_mask.Field
3232
}
3333

3434
queue := []fieldMaskPathItem{{node: root, msg: msg.ProtoReflect()}}
35-
var repeatedChild *fieldMaskPathItem
3635
for len(queue) > 0 {
3736
// dequeue an item
3837
item := queue[0]
@@ -74,14 +73,9 @@ func FieldMaskFromRequestBody(r io.Reader, msg proto.Message) (*field_mask.Field
7473

7574
switch {
7675
case fd.IsList(), fd.IsMap():
77-
if repeatedChild != nil {
78-
// This is implied by the rule that any repeated fields must be
79-
// last in the paths.
80-
// Ref: https://github.com/protocolbuffers/protobuf/blob/6b0ff74ecf63e26c7315f6745de36aff66deb59d/src/google/protobuf/field_mask.proto#L85-L86
81-
return nil, fmt.Errorf("only one repeated value is allowed per field_mask")
82-
}
83-
repeatedChild = &child
84-
// Don't add to paths until the end
76+
// As per: https://github.com/protocolbuffers/protobuf/blob/master/src/google/protobuf/field_mask.proto#L85-L86
77+
// Do not recurse into repeated fields. The repeated field goes on the end of the path and we stop.
78+
fm.Paths = append(fm.Paths, child.path)
8579
case fd.Message() != nil:
8680
child.msg = item.msg.Get(fd).Message()
8781
fallthrough
@@ -95,12 +89,6 @@ func FieldMaskFromRequestBody(r io.Reader, msg proto.Message) (*field_mask.Field
9589
}
9690
}
9791

98-
// Add any repeated fields last, as per
99-
// https://github.com/protocolbuffers/protobuf/blob/6b0ff74ecf63e26c7315f6745de36aff66deb59d/src/google/protobuf/field_mask.proto#L85-L86
100-
if repeatedChild != nil {
101-
fm.Paths = append(fm.Paths, repeatedChild.path)
102-
}
103-
10492
return fm, nil
10593
}
10694

runtime/fieldmask_test.go

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -182,11 +182,12 @@ func TestFieldMaskRepeatedFieldsLast(t *testing.T) {
182182
}{
183183
{
184184
name: "map",
185-
input: `{"mapped_string_value": {"a": "x"}, "uuid":"1234"}`,
185+
input: `{"mapped_string_value": {"a": "x"}, "repeated_string_value": {"b": "y"}, "uuid":"1234"}`,
186186
expected: &field_mask.FieldMask{
187187
Paths: []string{
188-
"uuid",
189188
"mapped_string_value",
189+
"repeated_string_value",
190+
"uuid",
190191
},
191192
},
192193
},
@@ -204,12 +205,23 @@ func TestFieldMaskRepeatedFieldsLast(t *testing.T) {
204205
"amount": 20
205206
}
206207
],
208+
"nested_annotation": [
209+
{
210+
"name": "foo",
211+
"amount": 100
212+
},
213+
{
214+
"name": "widget",
215+
"amount": 200
216+
}
217+
],
207218
"uuid":"1234"
208219
}`,
209220
expected: &field_mask.FieldMask{
210221
Paths: []string{
211-
"uuid",
212222
"nested",
223+
"nested_annotation",
224+
"uuid",
213225
},
214226
},
215227
},

0 commit comments

Comments
 (0)