Rework FieldMask to use a (ordered) set of FieldPaths

Firestore/Example/Tests/Util/FSTHelpers.mm

@@ -22,6 +22,7 @@
 
 #include <cinttypes>
 #include <list>
+#include <set>
 #include <unordered_map>
 #include <utility>
 #include <vector>
@@ -252,18 +253,19 @@ NSComparator FSTTestDocComparator(const absl::string_view fieldPath) {
   BOOL merge = !updateMask.empty();
 
   __block FSTObjectValue *objectValue = [FSTObjectValue objectValue];
-  __block std::vector<FieldPath> fieldMaskPaths;
+  __block std::set<FieldPath> fieldMaskPaths;
   [values enumerateKeysAndObjectsUsingBlock:^(NSString *key, id value, BOOL *stop) {
     const FieldPath path = testutil::Field(util::MakeString(key));
-    fieldMaskPaths.push_back(path);
+    fieldMaskPaths.insert(path);
     if (![value isEqual:kDeleteSentinel]) {
       FSTFieldValue *parsedValue = FSTTestFieldValue(value);
       objectValue = [objectValue objectBySettingValue:parsedValue forPath:path];
     }
   }];
 
   DocumentKey key = testutil::Key(path);
-  FieldMask mask(merge ? updateMask : fieldMaskPaths);
+  FieldMask mask(merge ? std::set<FieldPath>(updateMask.begin(), updateMask.end())
+                       : fieldMaskPaths);
   return [[FSTPatchMutation alloc] initWithKey:key
                                      fieldMask:mask
                                          value:objectValue

Firestore/Source/API/FSTUserDataConverter.mm

@@ -17,6 +17,7 @@
 #import "Firestore/Source/API/FSTUserDataConverter.h"
 
 #include <memory>
+#include <set>
 #include <string>
 #include <utility>
 #include <vector>
@@ -130,7 +131,7 @@ - (ParsedSetData)parsedMergeData:(id)input fieldMask:(nullable NSArray<id> *)fie
       (FSTObjectValue *)[self parseData:input context:accumulator.RootContext()];
 
   if (fieldMask) {
-    std::vector<FieldPath> validatedFieldPaths;
+    std::set<FieldPath> validatedFieldPaths;
     for (id fieldPath in fieldMask) {
       FieldPath path;
 
@@ -150,7 +151,7 @@ - (ParsedSetData)parsedMergeData:(id)input fieldMask:(nullable NSArray<id> *)fie
             path.CanonicalString().c_str());
       }
 
-      validatedFieldPaths.push_back(path);
+      validatedFieldPaths.insert(path);
     }
 
     return std::move(accumulator).MergeData(updateData, FieldMask{std::move(validatedFieldPaths)});

Firestore/Source/Remote/FSTSerializerBeta.mm

@@ -17,6 +17,7 @@
 #import "Firestore/Source/Remote/FSTSerializerBeta.h"
 
 #include <cinttypes>
+#include <set>
 #include <string>
 #include <utility>
 #include <vector>
@@ -565,10 +566,9 @@ - (GCFSDocumentMask *)encodedFieldMask:(const FieldMask &)fieldMask {
 }
 
 - (FieldMask)decodedFieldMask:(GCFSDocumentMask *)fieldMask {
-  std::vector<FieldPath> fields;
-  fields.reserve(fieldMask.fieldPathsArray_Count);
+  std::set<FieldPath> fields;
   for (NSString *path in fieldMask.fieldPathsArray) {
-    fields.push_back(FieldPath::FromServerFormat(util::MakeString(path)));
+    fields.insert(FieldPath::FromServerFormat(util::MakeString(path)));
   }
   return FieldMask(std::move(fields));
 }

Firestore/core/src/firebase/firestore/core/user_data.h

@@ -18,6 +18,7 @@
 #define FIRESTORE_CORE_SRC_FIREBASE_FIRESTORE_CORE_USER_DATA_H_
 
 #include <memory>
+#include <set>
 #include <string>
 #include <utility>
 #include <vector>
@@ -166,7 +167,7 @@ class ParseAccumulator {
   // field_mask_ and field_transforms_ are shared across all active context
   // objects to accumulate the result. All ChildContext objects append their
   // results here.
-  std::vector<model::FieldPath> field_mask_;
+  std::set<model::FieldPath> field_mask_;
   std::vector<model::FieldTransform> field_transforms_;
 };
 

Firestore/core/src/firebase/firestore/core/user_data.mm

@@ -58,7 +58,7 @@
 }
 
 void ParseAccumulator::AddToFieldMask(FieldPath field_path) {
-  field_mask_.push_back(std::move(field_path));
+  field_mask_.insert(std::move(field_path));
 }
 
 void ParseAccumulator::AddToFieldTransforms(

Firestore/core/src/firebase/firestore/model/field_mask.h

@@ -18,9 +18,9 @@
 #define FIRESTORE_CORE_SRC_FIREBASE_FIRESTORE_MODEL_FIELD_MASK_H_
 
 #include <initializer_list>
+#include <set>
 #include <string>
 #include <utility>
-#include <vector>
 
 #include "Firestore/core/src/firebase/firestore/model/field_path.h"
 #include "Firestore/core/src/firebase/firestore/util/hashing.h"
@@ -41,12 +41,14 @@ namespace model {
  */
 class FieldMask {
  public:
-  using const_iterator = std::vector<FieldPath>::const_iterator;
+  using const_iterator = std::set<FieldPath>::const_iterator;
 
   FieldMask(std::initializer_list<FieldPath> list) : fields_{list} {
   }
-  explicit FieldMask(std::vector<FieldPath> fields)
-      : fields_{std::move(fields)} {
+  template <class InputIt>
+  FieldMask(InputIt first, InputIt last) : fields_{first, last} {
+  }
+  explicit FieldMask(std::set<FieldPath> fields) : fields_{std::move(fields)} {
   }
 
   const_iterator begin() const {
@@ -94,7 +96,7 @@ class FieldMask {
   friend bool operator==(const FieldMask& lhs, const FieldMask& rhs);
 
  private:
-  std::vector<FieldPath> fields_;
+  std::set<FieldPath> fields_;
 };
 
 inline bool operator==(const FieldMask& lhs, const FieldMask& rhs) {

Firestore/core/test/firebase/firestore/model/field_mask_test.cc

@@ -16,7 +16,7 @@
 
 #include "Firestore/core/src/firebase/firestore/model/field_mask.h"
 
-#include <vector>
+#include <set>
 
 #include "Firestore/core/src/firebase/firestore/model/field_path.h"
 #include "gtest/gtest.h"
@@ -28,27 +28,30 @@ namespace model {
 TEST(FieldMask, ConstructorAndEqual) {
   FieldMask mask_a{FieldPath::FromServerFormat("foo"),
                    FieldPath::FromServerFormat("bar")};
-  std::vector<FieldPath> field_path_vector{FieldPath::FromServerFormat("foo"),
-                                           FieldPath::FromServerFormat("bar")};
-  FieldMask mask_b{field_path_vector};
-  FieldMask mask_c{std::vector<FieldPath>{FieldPath::FromServerFormat("foo"),
-                                          FieldPath::FromServerFormat("bar")}};
+  std::set<FieldPath> field_path_set{FieldPath::FromServerFormat("foo"),
+                                     FieldPath::FromServerFormat("bar")};
+  FieldMask mask_b{field_path_set};
+  FieldMask mask_c{std::set<FieldPath>{FieldPath::FromServerFormat("foo"),
+                                       FieldPath::FromServerFormat("bar")}};
+  FieldMask mask_d{field_path_set.begin(), field_path_set.end()};
+
   EXPECT_EQ(mask_a, mask_b);
   EXPECT_EQ(mask_b, mask_c);
+  EXPECT_EQ(mask_c, mask_d);
 }
 
 TEST(FieldMask, Getter) {
   FieldMask mask{FieldPath::FromServerFormat("foo"),
                  FieldPath::FromServerFormat("bar")};
-  EXPECT_EQ(std::vector<FieldPath>({FieldPath::FromServerFormat("foo"),
-                                    FieldPath::FromServerFormat("bar")}),
-            std::vector<FieldPath>(mask.begin(), mask.end()));
+  EXPECT_EQ(std::set<FieldPath>({FieldPath::FromServerFormat("foo"),
+                                 FieldPath::FromServerFormat("bar")}),
+            std::set<FieldPath>(mask.begin(), mask.end()));
 }
 
 TEST(FieldMask, ToString) {
   FieldMask mask{FieldPath::FromServerFormat("foo"),
                  FieldPath::FromServerFormat("bar")};
-  EXPECT_EQ("{ foo bar }", mask.ToString());
+  EXPECT_EQ("{ bar foo }", mask.ToString());
 }
 
 }  // namespace model

Firestore/core/test/firebase/firestore/testutil/testutil.cc

@@ -16,34 +16,40 @@
 
 #include "Firestore/core/test/firebase/firestore/testutil/testutil.h"
 
+#include <set>
+
 namespace firebase {
 namespace firestore {
 namespace testutil {
 
 std::unique_ptr<model::PatchMutation> PatchMutation(
     absl::string_view path,
     const model::ObjectValue::Map& values,
+    // TODO(rsgowman): Investigate changing update_mask to a set.
     const std::vector<model::FieldPath>* update_mask) {
   model::FieldValue object_value = model::FieldValue::FromMap({});
-  std::vector<model::FieldPath> object_mask;
+  std::set<model::FieldPath> object_mask;
 
   for (const auto& kv : values) {
     model::FieldPath field_path = Field(kv.first);
-    object_mask.push_back(field_path);
+    object_mask.insert(field_path);
     if (kv.second.string_value() != kDeleteSentinel) {
       object_value = object_value.Set(field_path, kv.second);
     }
   }
 
   bool merge = update_mask != nullptr;
 
-  // We sort the field_mask_paths to make the order deterministic in tests.
-  std::sort(object_mask.begin(), object_mask.end());
-
-  return absl::make_unique<model::PatchMutation>(
-      Key(path), std::move(object_value),
-      model::FieldMask(merge ? *update_mask : object_mask),
-      merge ? model::Precondition::None() : model::Precondition::Exists(true));
+  if (merge) {
+    return absl::make_unique<model::PatchMutation>(
+        Key(path), std::move(object_value),
+        model::FieldMask(update_mask->begin(), update_mask->end()),
+        model::Precondition::None());
+  } else {
+    return absl::make_unique<model::PatchMutation>(
+        Key(path), std::move(object_value), model::FieldMask(object_mask),
+        model::Precondition::Exists(true));
+  }
 }
 
 }  // namespace testutil