Skip to content

Commit 1ef69c9

Browse files
author
Pouya Yarandi
committed
Fix comments
1 parent db6be11 commit 1ef69c9

File tree

6 files changed

+217
-37
lines changed

6 files changed

+217
-37
lines changed

‎Sources/SwiftProtobuf/Google_Protobuf_FieldMask+Extensions.swift‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -199,6 +199,8 @@ extension Google_Protobuf_FieldMask {
199199
/// - messageType: Message type to get all paths from.
200200
/// - fieldNumbers: Field numbers of paths to be included.
201201
/// - Returns: Field mask that include paths of corresponding field numbers.
202+
/// - Throws: `FieldMaskError.invalidFieldNumber` if the field number
203+
/// is not on the message
202204
public init<M: Message & _ProtoNameProviding>(
203205
fieldNumbers: [Int],
204206
of messageType: M.Type

‎Sources/SwiftProtobuf/Message+FieldMask.swift‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,7 @@ extension Message {
7575
var visitor = PathVisitor<Self>()
7676
try source.traverse(visitor: &visitor)
7777
let values = visitor.values
78+
// TODO: setting all values with only one decoding
7879
for path in fieldMask.paths {
7980
try? set(
8081
path: path,
@@ -89,12 +90,12 @@ extension Message where Self: Equatable, Self: _ProtoNameProviding {
8990

9091
// TODO: Re-implement using clear fields instead of copying message
9192

92-
@discardableResult
9393
/// Removes from 'message' any field that is not represented in the given
9494
/// FieldMask. If the FieldMask is empty, does nothing.
9595
///
9696
/// - Parameter fieldMask: FieldMask specifies which fields should be kept.
9797
/// - Returns: Boolean determines if the message is modified
98+
@discardableResult
9899
public mutating func trim(
99100
fieldMask: Google_Protobuf_FieldMask
100101
) -> Bool {

‎Sources/SwiftProtobuf/NameMap.swift‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -281,8 +281,6 @@ public struct _NameMap: ExpressibleByDictionaryLiteral {
281281

282282
/// Returns all proto names
283283
internal var names: [Name] {
284-
protoToNumberMap.map {
285-
$0.key
286-
}
284+
protoToNumberMap.map(\.key)
287285
}
288286
}

‎Sources/SwiftProtobuf/PathDecoder.swift‎

Lines changed: 21 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,24 @@ struct PathDecoder<T: Message>: Decoder {
135135
}
136136
}
137137

138+
private func setMessageValue<M: Message>(
139+
_ value: inout M?
140+
) throws {
141+
if nextPath.isEmpty {
142+
try setValue(&value, defaultValue: nil)
143+
return
144+
}
145+
var decoder = try PathDecoder<M>(
146+
path: nextPath,
147+
value: self.value,
148+
mergeOption: mergeOption
149+
)
150+
if value == nil {
151+
value = .init()
152+
}
153+
try value?.decodeMessage(decoder: &decoder)
154+
}
155+
138156
mutating func handleConflictingOneOf() throws {}
139157

140158
mutating func nextFieldNumber() throws -> Int? {
@@ -343,19 +361,7 @@ struct PathDecoder<T: Message>: Decoder {
343361
mutating func decodeSingularMessageField<M>(
344362
value: inout M?
345363
) throws where M : Message {
346-
if nextPath.isEmpty {
347-
try setValue(&value, defaultValue: nil)
348-
return
349-
}
350-
var decoder = try PathDecoder<M>(
351-
path: nextPath,
352-
value: self.value,
353-
mergeOption: mergeOption
354-
)
355-
if value == nil {
356-
value = .init()
357-
}
358-
try value?.decodeMessage(decoder: &decoder)
364+
try setMessageValue(&value)
359365
}
360366

361367
mutating func decodeRepeatedMessageField<M>(
@@ -367,7 +373,7 @@ struct PathDecoder<T: Message>: Decoder {
367373
mutating func decodeSingularGroupField<G>(
368374
value: inout G?
369375
) throws where G : Message {
370-
try setValue(&value, defaultValue: nil)
376+
try setMessageValue(&value)
371377
}
372378

373379
mutating func decodeRepeatedGroupField<G>(
@@ -403,7 +409,7 @@ struct PathDecoder<T: Message>: Decoder {
403409
fieldNumber: Int
404410
) throws {
405411
preconditionFailure(
406-
"Path decoder should never decode an extension field"
412+
"Internal Error: Path decoder should never decode an extension field"
407413
)
408414
}
409415

‎Sources/SwiftProtobuf/PathVisitor.swift‎

Lines changed: 20 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,24 @@ struct PathVisitor<T: Message>: Visitor {
3838
}
3939
}
4040

41+
mutating private func visitMessageField<M: Message>(
42+
_ value: M,
43+
fieldNumber: Int
44+
) {
45+
guard var path = T.name(for: fieldNumber) else {
46+
return
47+
}
48+
if let prevPath {
49+
path = "\(prevPath).\(path)"
50+
}
51+
values[path] = value
52+
var visitor = PathVisitor<M>(prevPath: path)
53+
try? value.traverse(visitor: &visitor)
54+
values.merge(visitor.values) { _, new in
55+
new
56+
}
57+
}
58+
4159
mutating func visitUnknown(bytes: Data) throws {}
4260

4361
mutating func visitSingularFloatField(value: Float, fieldNumber: Int) throws {
@@ -105,22 +123,11 @@ struct PathVisitor<T: Message>: Visitor {
105123
}
106124

107125
mutating func visitSingularMessageField<M: Message>(value: M, fieldNumber: Int) throws {
108-
guard var path = T.name(for: fieldNumber) else {
109-
return
110-
}
111-
values[path] = value
112-
if let prevPath {
113-
path = "\(prevPath).\(path)"
114-
}
115-
var visitor = PathVisitor<M>(prevPath: path)
116-
try value.traverse(visitor: &visitor)
117-
values.merge(visitor.values, uniquingKeysWith: { _, new in
118-
new
119-
})
126+
visitMessageField(value, fieldNumber: fieldNumber)
120127
}
121128

122129
mutating func visitSingularGroupField<G: Message>(value: G, fieldNumber: Int) throws {
123-
visit(value, fieldNumber: fieldNumber)
130+
visitMessageField(value, fieldNumber: fieldNumber)
124131
}
125132

126133
mutating func visitRepeatedFloatField(value: [Float], fieldNumber: Int) throws {

‎Tests/SwiftProtobufTests/Test_FieldMask.swift‎

Lines changed: 171 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -229,18 +229,21 @@ final class Test_FieldMask: XCTestCase, PBTestHelpers {
229229

230230
// Checks `isPathValid` func
231231
// 1. Valid primitive path.
232-
// 2. Valid nested path.
233-
// 3. Invalid primitive path.
234-
// 4, 5. Invalid nested path.
235-
// 6, 7. Invalid path after map and repeated.
232+
// 2, 3. Valid nested path. (for message and group)
233+
// 4. Invalid primitive path.
234+
// 5, 6. Invalid nested path.
235+
// 7, 8. Invalid path after map and repeated.
236+
// 9. Invalid path after group.
236237
func testIsPathValid() {
237238
XCTAssertTrue(SwiftProtoTesting_TestAllTypes.isPathValid("optional_int32"))
238239
XCTAssertTrue(SwiftProtoTesting_TestAllTypes.isPathValid("optional_nested_message.bb"))
240+
XCTAssertTrue(SwiftProtoTesting_Fuzz_Message.isPathValid("SingularGroup.group_field"))
239241
XCTAssertFalse(SwiftProtoTesting_TestAllTypes.isPathValid("optional_int"))
240242
XCTAssertFalse(SwiftProtoTesting_TestAllTypes.isPathValid("optional_nested_message.bc"))
241243
XCTAssertFalse(SwiftProtoTesting_TestAllTypes.isPathValid("optional_nested_message.bb.a"))
242244
XCTAssertFalse(SwiftProtoTesting_TestAllTypes.isPathValid("repeatedInt32.a"))
243245
XCTAssertFalse(SwiftProtoTesting_Fuzz_Message.isPathValid("map_bool_int32.a"))
246+
XCTAssertFalse(SwiftProtoTesting_Fuzz_Message.isPathValid("SingularGroup.a"))
244247
}
245248

246249
// Checks `isValid` func of FieldMask.
@@ -530,6 +533,148 @@ final class Test_FieldMask: XCTestCase, PBTestHelpers {
530533
XCTAssertEqual(m1.mapInt32Message, m2.mapInt32Message)
531534
}
532535

536+
// Checks whether a group of fields could be merged without merging the others.
537+
func testMergeFieldsPartially() throws {
538+
var m1 = SwiftProtoTesting_Fuzz_Message()
539+
let m2 = SwiftProtoTesting_Fuzz_Message.with { m in
540+
m.singularInt32 = 1
541+
m.singularInt64 = 1
542+
m.singularUint32 = 1
543+
m.singularUint64 = 1
544+
m.singularSint32 = 1
545+
m.singularSint64 = 1
546+
m.singularFixed32 = 1
547+
m.singularFixed64 = 1
548+
m.singularSfixed32 = 1
549+
m.singularSfixed64 = 1
550+
m.singularFloat = 1
551+
m.singularDouble = 1
552+
m.singularBool = true
553+
m.singularString = "str"
554+
m.singularBytes = "str".data(using: .utf8) ?? .init()
555+
m.singularEnum = .two
556+
m.singularGroup = .with { $0.groupField = 1 }
557+
m.singularMessage = .with { $0.singularInt32 = 1 }
558+
m.repeatedInt32 = [1]
559+
m.repeatedInt64 = [1]
560+
m.repeatedUint32 = [1]
561+
m.repeatedUint64 = [1]
562+
m.repeatedSint32 = [1]
563+
m.repeatedSint64 = [1]
564+
m.repeatedFixed32 = [1]
565+
m.repeatedFixed64 = [1]
566+
m.repeatedSfixed32 = [1]
567+
m.repeatedSfixed64 = [1]
568+
m.repeatedFloat = [1]
569+
m.repeatedDouble = [1]
570+
m.repeatedBool = [true]
571+
m.repeatedString = ["str"]
572+
m.repeatedBytes = ["str".data(using: .utf8) ?? .init()]
573+
m.repeatedEnum = [.two]
574+
m.repeatedGroup = [.with { $0.groupField = 1 }]
575+
m.repeatedMessage = [.with { $0.singularInt32 = 1 }]
576+
m.o = .oneofInt32(1)
577+
m.repeatedPackedInt32 = [1]
578+
m.repeatedPackedInt64 = [1]
579+
m.repeatedPackedUint32 = [1]
580+
m.repeatedPackedUint64 = [1]
581+
m.repeatedPackedSint32 = [1]
582+
m.repeatedPackedSint64 = [1]
583+
m.repeatedPackedFixed32 = [1]
584+
m.repeatedPackedFixed64 = [1]
585+
m.repeatedPackedSfixed32 = [1]
586+
m.repeatedPackedSfixed64 = [1]
587+
m.repeatedPackedFloat = [1]
588+
m.repeatedPackedDouble = [1]
589+
m.repeatedPackedBool = [true]
590+
m.repeatedPackedEnum = [.two]
591+
m.mapInt32Int32 = [1: 1]
592+
m.mapInt32Int64 = [1: 1]
593+
m.mapInt32Uint32 = [1: 1]
594+
m.mapInt32Uint64 = [1: 1]
595+
m.mapInt32Sint32 = [1: 1]
596+
m.mapInt32Sint64 = [1: 1]
597+
m.mapInt32Fixed32 = [1: 1]
598+
m.mapInt32Fixed64 = [1: 1]
599+
m.mapInt32AnEnum = [1: .one]
600+
m.mapInt32Message = [1: .init()]
601+
}
602+
let mask = Google_Protobuf_FieldMask(protoPaths: [
603+
"singular_int32",
604+
"singular_int64",
605+
"singular_uint32",
606+
"singular_uint64",
607+
"singular_sint32",
608+
"singular_sint64",
609+
"singular_fixed32",
610+
"singular_fixed64",
611+
"singular_sfixed32",
612+
"singular_sfixed64"
613+
])
614+
try m1.merge(with: m2, fieldMask: mask)
615+
XCTAssertEqual(m1.singularInt32, m2.singularInt32)
616+
XCTAssertEqual(m1.singularInt64, m2.singularInt64)
617+
XCTAssertEqual(m1.singularUint32, m2.singularUint32)
618+
XCTAssertEqual(m1.singularUint64, m2.singularUint64)
619+
XCTAssertEqual(m1.singularSint32, m2.singularSint32)
620+
XCTAssertEqual(m1.singularSint64, m2.singularSint64)
621+
XCTAssertEqual(m1.singularFixed32, m2.singularFixed32)
622+
XCTAssertEqual(m1.singularFixed64, m2.singularFixed64)
623+
XCTAssertEqual(m1.singularSfixed32, m2.singularSfixed32)
624+
XCTAssertEqual(m1.singularSfixed64, m2.singularSfixed64)
625+
XCTAssertNotEqual(m1.singularFloat, m2.singularFloat)
626+
XCTAssertNotEqual(m1.singularDouble, m2.singularDouble)
627+
XCTAssertNotEqual(m1.singularBool, m2.singularBool)
628+
XCTAssertNotEqual(m1.singularString, m2.singularString)
629+
XCTAssertNotEqual(m1.singularBytes, m2.singularBytes)
630+
XCTAssertNotEqual(m1.singularEnum, m2.singularEnum)
631+
XCTAssertNotEqual(m1.singularGroup, m2.singularGroup)
632+
XCTAssertNotEqual(m1.singularMessage, m2.singularMessage)
633+
XCTAssertNotEqual(m1.repeatedInt32, m2.repeatedInt32)
634+
XCTAssertNotEqual(m1.repeatedInt64, m2.repeatedInt64)
635+
XCTAssertNotEqual(m1.repeatedUint32, m2.repeatedUint32)
636+
XCTAssertNotEqual(m1.repeatedUint64, m2.repeatedUint64)
637+
XCTAssertNotEqual(m1.repeatedSint32, m2.repeatedSint32)
638+
XCTAssertNotEqual(m1.repeatedSint64, m2.repeatedSint64)
639+
XCTAssertNotEqual(m1.repeatedFixed32, m2.repeatedFixed32)
640+
XCTAssertNotEqual(m1.repeatedFixed64, m2.repeatedFixed64)
641+
XCTAssertNotEqual(m1.repeatedSfixed32, m2.repeatedSfixed32)
642+
XCTAssertNotEqual(m1.repeatedSfixed64, m2.repeatedSfixed64)
643+
XCTAssertNotEqual(m1.repeatedFloat, m2.repeatedFloat)
644+
XCTAssertNotEqual(m1.repeatedDouble, m2.repeatedDouble)
645+
XCTAssertNotEqual(m1.repeatedBool, m2.repeatedBool)
646+
XCTAssertNotEqual(m1.repeatedString, m2.repeatedString)
647+
XCTAssertNotEqual(m1.repeatedBytes, m2.repeatedBytes)
648+
XCTAssertNotEqual(m1.repeatedEnum, m2.repeatedEnum)
649+
XCTAssertNotEqual(m1.repeatedGroup, m2.repeatedGroup)
650+
XCTAssertNotEqual(m1.repeatedMessage, m2.repeatedMessage)
651+
XCTAssertNotEqual(m1.o, m2.o)
652+
XCTAssertNotEqual(m1.repeatedPackedInt32, m2.repeatedPackedInt32)
653+
XCTAssertNotEqual(m1.repeatedPackedInt64, m2.repeatedPackedInt64)
654+
XCTAssertNotEqual(m1.repeatedPackedUint32, m2.repeatedPackedUint32)
655+
XCTAssertNotEqual(m1.repeatedPackedUint64, m2.repeatedPackedUint64)
656+
XCTAssertNotEqual(m1.repeatedPackedSint32, m2.repeatedPackedSint32)
657+
XCTAssertNotEqual(m1.repeatedPackedSint64, m2.repeatedPackedSint64)
658+
XCTAssertNotEqual(m1.repeatedPackedFixed32, m2.repeatedPackedFixed32)
659+
XCTAssertNotEqual(m1.repeatedPackedFixed64, m2.repeatedPackedFixed64)
660+
XCTAssertNotEqual(m1.repeatedPackedSfixed32, m2.repeatedPackedSfixed32)
661+
XCTAssertNotEqual(m1.repeatedPackedSfixed64, m2.repeatedPackedSfixed64)
662+
XCTAssertNotEqual(m1.repeatedPackedFloat, m2.repeatedPackedFloat)
663+
XCTAssertNotEqual(m1.repeatedPackedDouble, m2.repeatedPackedDouble)
664+
XCTAssertNotEqual(m1.repeatedPackedBool, m2.repeatedPackedBool)
665+
XCTAssertNotEqual(m1.repeatedPackedEnum, m2.repeatedPackedEnum)
666+
XCTAssertNotEqual(m1.mapInt32Int32, m2.mapInt32Int32)
667+
XCTAssertNotEqual(m1.mapInt32Int64, m2.mapInt32Int64)
668+
XCTAssertNotEqual(m1.mapInt32Uint32, m2.mapInt32Uint32)
669+
XCTAssertNotEqual(m1.mapInt32Uint64, m2.mapInt32Uint64)
670+
XCTAssertNotEqual(m1.mapInt32Sint32, m2.mapInt32Sint32)
671+
XCTAssertNotEqual(m1.mapInt32Sint64, m2.mapInt32Sint64)
672+
XCTAssertNotEqual(m1.mapInt32Fixed32, m2.mapInt32Fixed32)
673+
XCTAssertNotEqual(m1.mapInt32Fixed64, m2.mapInt32Fixed64)
674+
XCTAssertNotEqual(m1.mapInt32AnEnum, m2.mapInt32AnEnum)
675+
XCTAssertNotEqual(m1.mapInt32Message, m2.mapInt32Message)
676+
}
677+
533678
// Checks merge could be done for an optional path with nil value.
534679
func testMergeOptionalValue() throws {
535680
var m1 = SwiftProtoTesting_Fuzz_Message.with { m in
@@ -618,7 +763,28 @@ final class Test_FieldMask: XCTestCase, PBTestHelpers {
618763
}
619764
}
620765
}
621-
try m1.merge(with: m2, fieldMask: .init(protoPaths: ["singular_message.singular_message.singular_int32"]))
766+
let m3 = SwiftProtoTesting_Fuzz_Message.with { m in
767+
m.singularMessage = .with { _m in
768+
_m.singularMessage = .with { __m in
769+
__m.singularInt32 = 2
770+
}
771+
}
772+
}
773+
try m1.merge(with: m2, fieldMask: .init(protoPaths: ["singular_message.singular_message"]))
622774
XCTAssertEqual(m1.singularMessage.singularMessage.singularInt32, Int32(1))
775+
try m1.merge(with: m3, fieldMask: .init(protoPaths: ["singular_message.singular_message.singular_int32"]))
776+
XCTAssertEqual(m1.singularMessage.singularMessage.singularInt32, Int32(2))
777+
}
778+
779+
// Checks merging nested path inside groups
780+
func testMergeNestedGroups() throws {
781+
var m1 = SwiftProtoTesting_Fuzz_Message()
782+
let m2 = SwiftProtoTesting_Fuzz_Message.with { m in
783+
m.singularGroup = .with { _m in
784+
_m.groupField = 1
785+
}
786+
}
787+
try m1.merge(with: m2, fieldMask: .init(protoPaths: ["SingularGroup.group_field"]))
788+
XCTAssertEqual(m1.singularGroup.groupField, m2.singularGroup.groupField)
623789
}
624790
}

0 commit comments

Comments
 (0)