Add comments

Author: Pouya Yarandi

Sources/SwiftProtobuf/Google_Protobuf_FieldMask+Extensions.swift

@@ -13,11 +13,6 @@
 ///
 // -----------------------------------------------------------------------------
 
-// TODO: We should have utilities to apply a fieldmask to an arbitrary
-// message, intersect two fieldmasks, etc.
-// Google's C++ implementation does this by having utilities
-// to build a tree of field paths that can be easily intersected,
-// unioned, traversed to apply to submessages, etc.
 
 // True if the string only contains printable (non-control)
 // ASCII characters.  Note: This follows the ASCII standard;
@@ -188,6 +183,7 @@ extension Google_Protobuf_FieldMask: _CustomJSONCodable {
 extension Google_Protobuf_FieldMask {
 
   /// Initiates a field mask with all fields of the message type.
+  ///
   /// - Parameter messageType: Message type to get all paths from.
   public init<M: Message & _ProtoNameProviding>(
     allFieldsOf messageType: M.Type
@@ -198,6 +194,7 @@ extension Google_Protobuf_FieldMask {
   }
 
   /// Initiates a field mask from some particular field numbers of a message
+  ///
   /// - Parameters:
   ///   - messageType: Message type to get all paths from.
   ///   - fieldNumbers: Field numbers of paths to be included.
@@ -219,12 +216,14 @@ extension Google_Protobuf_FieldMask {
   }
 }
 
-public enum FieldMaskUtilsError: Error {
-  case invalidPath
-  case invalidFieldNumber
-}
-
 extension Google_Protobuf_FieldMask {
+
+  /// Adds a path to FieldMask after checking whether the given path is valid.
+  /// This method check-fails if the path is not a valid path for Message type.
+  ///
+  /// - Parameters:
+  ///   - path: Path to be added to FieldMask.
+  ///   - messageType: Message type to check validity.
   public mutating func addPath<M: Message>(
     _ path: String,
     of messageType: M.Type
@@ -235,6 +234,11 @@ extension Google_Protobuf_FieldMask {
     paths.append(path)
   }
 
+  /// Converts a FieldMask to the canonical form. It will:
+  ///   1. Remove paths that are covered by another path. For example,
+  ///      "foo.bar" is covered by "foo" and will be removed if "foo"
+  ///      is also in the FieldMask.
+  ///   2. Sort all paths in alphabetical order.
   public var canonical: Google_Protobuf_FieldMask {
     var mask = Google_Protobuf_FieldMask()
     let sortedPaths = self.paths.sorted()
@@ -248,6 +252,10 @@ extension Google_Protobuf_FieldMask {
     return mask
   }
 
+  /// Creates an union of two FieldMasks.
+  ///
+  /// - Parameter mask: FieldMask to union with.
+  /// - Returns: FieldMask with union of two path sets.
   public func union(
     _ mask: Google_Protobuf_FieldMask
   ) -> Google_Protobuf_FieldMask {
@@ -263,10 +271,10 @@ extension Google_Protobuf_FieldMask {
     }
   }
 
-  private var pathsSet: Set<String> {
-    .init(paths)
-  }
-
+  /// Creates an intersection of two FieldMasks.
+  ///
+  /// - Parameter mask: FieldMask to intersect with.
+  /// - Returns: FieldMask with intersection of two path sets.
   public func intersect(
     _ mask: Google_Protobuf_FieldMask
   ) -> Google_Protobuf_FieldMask {
@@ -280,6 +288,11 @@ extension Google_Protobuf_FieldMask {
     }
   }
 
+  /// Creates a FieldMasks with paths of the original FieldMask
+  /// that does not included in mask.
+  ///
+  /// - Parameter mask: FieldMask with paths should be substracted.
+  /// - Returns: FieldMask with all paths does not included in mask.
   public func subtract(
     _ mask: Google_Protobuf_FieldMask
   ) -> Google_Protobuf_FieldMask {
@@ -293,6 +306,22 @@ extension Google_Protobuf_FieldMask {
     }
   }
 
+  /// Returns true if path is covered by the given FieldMask. Note that path
+  /// "foo.bar" covers all paths like "foo.bar.baz", "foo.bar.quz.x", etc.
+  /// Also note that parent paths are not covered by explicit child path, i.e.
+  /// "foo.bar" does NOT cover "foo", even if "bar" is the only child.
+  ///
+  /// - Parameter path: Path to be checked.
+  /// - Returns: Boolean determines is path covered.
+  public func contains(_ path: String) -> Bool {
+    contains(path, in: pathsSet)
+  }
+
+  // Set containing paths of FieldMask
+  private var pathsSet: Set<String> {
+    .init(paths)
+  }
+
   private func levels(path: String) -> [String] {
     let comps = path.components(separatedBy: ".")
     return (0..<comps.count).map {
@@ -311,13 +340,14 @@ extension Google_Protobuf_FieldMask {
     }
     return false
   }
-
-  public func contains(_ path: String) -> Bool {
-    contains(path, in: pathsSet)
-  }
 }
 
 extension Google_Protobuf_FieldMask {
+
+  /// Checks whether the given FieldMask is valid for type M.
+  ///
+  /// - Parameter messageType: Message type to paths check with.
+  /// - Returns: Boolean determines FieldMask is valid.
   public func isValid<M: Message & _ProtoNameProviding>(
     for messageType: M.Type
   ) -> Bool {
@@ -328,6 +358,16 @@ extension Google_Protobuf_FieldMask {
   }
 }
 
+/// Describes errors could happen during FieldMask utilities.
+public enum FieldMaskUtilsError: Error {
+
+  /// Describes a path is invalid for a Message type.
+  case invalidPath
+
+  /// Describes a fieldNumber is invalid for a Message type.
+  case invalidFieldNumber
+}
+
 private extension Message where Self: _ProtoNameProviding {
   static func protoName(for number: Int) -> String? {
     Self._protobuf_nameMap.names(for: number)?.proto.description

Sources/SwiftProtobuf/Message+FieldMask.swift

@@ -15,6 +15,11 @@
 import Foundation
 
 extension Message {
+
+  /// Checks whether the given path is valid for Message type.
+  ///
+  /// - Parameter path: Path to be checked
+  /// - Returns: Boolean determines path is valid.
   public static func isPathValid(
     _ path: String
   ) -> Bool {
@@ -29,6 +34,12 @@ extension Message {
 }
 
 extension Message {
+
+  /// Merges fields specified in a FieldMask into another message.
+  ///
+  /// - Parameters:
+  ///   - source: Message should be merged to the original one.
+  ///   - fieldMask: FieldMask specifies which fields should be merged.
   public mutating func merge(
     to source: Self,
     fieldMask: Google_Protobuf_FieldMask
@@ -46,7 +57,13 @@ extension Message {
 }
 
 extension Message where Self: Equatable, Self: _ProtoNameProviding {
+
   @discardableResult
+  /// Removes from 'message' any field that is not represented in the given
+  /// FieldMask. If the FieldMask is empty, does nothing.
+  ///
+  /// - Parameter fieldMask: FieldMask specifies which fields should be kept.
+  /// - Returns: Boolean determines if the message is modified
   public mutating func trim(
     fieldMask: Google_Protobuf_FieldMask
   ) -> Bool {

Tests/SwiftProtobufTests/Test_FieldMask.swift

@@ -106,7 +106,8 @@ final class Test_FieldMask: XCTestCase, PBTestHelpers {
             XCTAssertThrowsError(try m.jsonString())
         }
     }
-
+    
+    // Checks merge functionality for field masks.
     func testMergeFieldsOfMessage() throws {
         var message = SwiftProtoTesting_TestAllTypes.with { model in
             model.optionalInt32 = 1
@@ -122,15 +123,18 @@ final class Test_FieldMask: XCTestCase, PBTestHelpers {
             }
         }
 
+        // Checks nested message merge
         try message.merge(to: secondMessage, fieldMask: .init(protoPaths: "optional_nested_message.bb"))
         XCTAssertEqual(message.optionalInt32, 1)
         XCTAssertEqual(message.optionalNestedMessage.bb, 3)
 
+        // Checks primitive type merge
         try message.merge(to: secondMessage, fieldMask: .init(protoPaths: "optional_int32"))
         XCTAssertEqual(message.optionalInt32, 2)
         XCTAssertEqual(message.optionalNestedMessage.bb, 3)
     }
 
+    // Checks trim functionality for field masks.
     func testTrimFieldsOfMessage() throws {
         var message = SwiftProtoTesting_TestAllTypes.with { model in
             model.optionalInt32 = 1
@@ -139,28 +143,41 @@ final class Test_FieldMask: XCTestCase, PBTestHelpers {
             }
         }
 
+        // Checks trim to be successful.
         let r1 = message.trim(fieldMask: .init(protoPaths: "optional_nested_message.bb"))
         XCTAssertTrue(r1)
         XCTAssertEqual(message.optionalInt32, 0)
         XCTAssertEqual(message.optionalNestedMessage.bb, 2)
 
+        // Checks trim should does nothing with an empty fieldMask.
         let r2 = message.trim(fieldMask: .init())
         XCTAssertFalse(r2)
 
+        // Checks trim should return false if nothing has been changed.
         let r3 = message.trim(fieldMask: .init(protoPaths: "optional_nested_message.bb"))
         XCTAssertFalse(r3)
 
+        // Checks trim to be unsuccessful with an invalid fieldMask.
         let r4 = message.trim(fieldMask: .init(protoPaths: "invalid_path"))
         XCTAssertFalse(r4)
     }
 
+    // Checks `isPathValid` func
+    // 1. Valid primitive path should be valid.
+    // 2. Valid nested path should be valid.
+    // 3. Invalid primitive path should be valid.
+    // 4. Invalid nested path should be valid.
     func testIsPathValid() {
         XCTAssertTrue(SwiftProtoTesting_TestAllTypes.isPathValid("optional_int32"))
         XCTAssertTrue(SwiftProtoTesting_TestAllTypes.isPathValid("optional_nested_message.bb"))
         XCTAssertFalse(SwiftProtoTesting_TestAllTypes.isPathValid("optional_int"))
         XCTAssertFalse(SwiftProtoTesting_TestAllTypes.isPathValid("optional_nested_message.bc"))
     }
 
+    // Checks `isValid` func of FieldMask.
+    // 1. Empty field mask is always valid.
+    // 2, 3. Valid field masks.
+    // 4, 5. Invalid field masks.
     func testIsFieldMaskValid() {
         let m1 = Google_Protobuf_FieldMask()
         let m2 = Google_Protobuf_FieldMask(protoPaths: [
@@ -186,15 +203,23 @@ final class Test_FieldMask: XCTestCase, PBTestHelpers {
         XCTAssertFalse(m5.isValid(for: SwiftProtoTesting_TestAllTypes.self))
     }
 
+    // Checks canonincal form of field mask.
+    // 1. Sub-message with parent in the paths should be excluded.
+    // 2. Canonincal form should be sorted.
+    // 3. More nested levels of paths.
     func testCanonicalFieldMask() {
-        let m1 = Google_Protobuf_FieldMask(protoPaths: ["a.b", "b", "a"])
+        let m1 = Google_Protobuf_FieldMask(protoPaths: ["a.b", "a", "b"])
         XCTAssertEqual(m1.canonical.paths, ["a", "b"])
-        let m2 = Google_Protobuf_FieldMask(protoPaths: ["a", "b"])
+        let m2 = Google_Protobuf_FieldMask(protoPaths: ["b", "a"])
         XCTAssertEqual(m2.canonical.paths, ["a", "b"])
         let m3 = Google_Protobuf_FieldMask(protoPaths: ["c", "a.b.c", "a.b", "a.b.c.d"])
         XCTAssertEqual(m3.canonical.paths, ["a.b", "c"])
     }
 
+    // Checks `addPath` func of fieldMask with:
+    //  - Valid primitive path should be added.
+    //  - Valid nested path should be added.
+    //  - Invalid path should throw error.
     func testAddPathToFieldMask() throws {
         var mask = Google_Protobuf_FieldMask()
         XCTAssertNoThrow(try mask.addPath("optional_int32", of: SwiftProtoTesting_TestAllTypes.self))
@@ -204,6 +229,12 @@ final class Test_FieldMask: XCTestCase, PBTestHelpers {
         XCTAssertThrowsError(try mask.addPath("optional_int", of: SwiftProtoTesting_TestAllTypes.self))
     }
 
+    // Check `contains` func of fieldMask.
+    // 1. Parent contains sub-message.
+    // 2. Path contains itself.
+    // 3. Sub-message does not contain its parent.
+    // 4. Two different paths does not contain each other.
+    // 5. Two different sub-paths does not contain each other.
     func testPathContainsInFieldMask() {
         let m1 = Google_Protobuf_FieldMask(protoPaths: ["a"])
         XCTAssertTrue(m1.contains("a.b"))
@@ -217,6 +248,9 @@ final class Test_FieldMask: XCTestCase, PBTestHelpers {
         XCTAssertFalse(m5.contains("a.c"))
     }
 
+    // Checks inits of fieldMask with:
+    //  - All fields of a message type.
+    //  - Particular field numbers of a message type.
     func testFieldPathMessageInits() throws {
         let m1 = Google_Protobuf_FieldMask(allFieldsOf: SwiftProtoTesting_TestAny.self)
         XCTAssertEqual(m1.paths.sorted(), ["any_value", "int32_value", "repeated_any_value", "text"])
@@ -225,6 +259,7 @@ final class Test_FieldMask: XCTestCase, PBTestHelpers {
         XCTAssertThrowsError(try Google_Protobuf_FieldMask(fieldNumbers: [10], of: SwiftProtoTesting_TestAny.self))
     }
 
+    // Checks `union` func of fieldMask.
     func testUnionFieldMasks() throws {
         let m1 = Google_Protobuf_FieldMask(protoPaths: ["a", "b"])
         let m2 = Google_Protobuf_FieldMask(protoPaths: ["b", "c"])
@@ -243,6 +278,7 @@ final class Test_FieldMask: XCTestCase, PBTestHelpers {
         XCTAssertEqual(m7.union(m8).paths, ["a", "b"])
     }
 
+    // Checks `intersect` func of fieldMask.
     func testIntersectFieldMasks() throws {
         let m1 = Google_Protobuf_FieldMask(protoPaths: ["a", "b"])
         let m2 = Google_Protobuf_FieldMask(protoPaths: ["b", "c"])
@@ -261,6 +297,7 @@ final class Test_FieldMask: XCTestCase, PBTestHelpers {
         XCTAssertEqual(m7.intersect(m8).paths, ["a", "b"])
     }
 
+    // Checks `substract` func of fieldMask.
     func testSubtractFieldMasks() throws {
         let m1 = Google_Protobuf_FieldMask(protoPaths: ["a", "b"])
         let m2 = Google_Protobuf_FieldMask(protoPaths: ["b", "c"])