Rework FieldMask to use a (ordered) set of FieldPaths

Rather than a vector.

Port of firebase/firebase-android-sdk#137

Author: rsgowman

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,6 +16,8 @@
 
 #include "Firestore/core/test/firebase/firestore/testutil/testutil.h"
 
+#include <set>
+
 namespace firebase {
 namespace firestore {
 namespace testutil {
@@ -25,25 +27,28 @@ std::unique_ptr<model::PatchMutation> PatchMutation(
     const model::ObjectValue::Map& values,
     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