Preserve order in ArrayUnion (#8050)

Author: schmidt-sebastian

Firestore/core/src/model/transform_operation.cc

@@ -19,7 +19,6 @@
 #include <functional>
 #include <memory>
 #include <ostream>
-#include <set>
 #include <utility>
 
 #include "Firestore/core/include/firebase/firestore/timestamp.h"
@@ -38,13 +37,6 @@ namespace model {
 
 using Type = TransformOperation::Type;
 
-struct ValueCompare {
-  bool operator()(const google_firestore_v1_Value& lhs,
-                  const google_firestore_v1_Value& rhs) const {
-    return Compare(lhs, rhs) == util::ComparisonResult::Ascending;
-  }
-};
-
 // MARK: - TransformOperation
 
 TransformOperation::TransformOperation(std::shared_ptr<const Rep> rep)
@@ -254,10 +246,15 @@ google_firestore_v1_Value ArrayTransform::Rep::Apply(
       CoercedFieldValueArray(previous_value);
   if (type_ == Type::ArrayUnion) {
     // Gather the list of elements that have to be added.
-    std::set<google_firestore_v1_Value, ValueCompare> new_elements;
+    std::vector<google_firestore_v1_Value> new_elements;
     for (pb_size_t i = 0; i < elements_->values_count; ++i) {
-      if (!Contains(array_value, elements_->values[i])) {
-        new_elements.insert(elements_->values[i]);
+      const google_firestore_v1_Value& new_element = elements_->values[i];
+      if (!Contains(array_value, new_element) &&
+          !std::any_of(new_elements.begin(), new_elements.end(),
+                       [&](const google_firestore_v1_Value& value) {
+                         return value == new_element;
+                       })) {
+        new_elements.push_back(new_element);
       }
     }
 

Firestore/core/test/unit/model/mutation_test.cc

@@ -344,6 +344,15 @@ TEST(MutationTest,
   TransformBaseDoc(base_data, transforms, expected);
 }
 
+TEST(MutationTest,
+     AppliesLocalArrayUnionTransformWithExistingElementsInOrder) {
+  // New elements should be appended in order.
+  auto base_data = Map("array", Array(1, 3));
+  TransformPairs transforms = {{"array", ArrayUnion(1, 2, 3, 4, 5)}};
+  auto expected = Map("array", Array(1, 3, 2, 4, 5));
+  TransformBaseDoc(base_data, transforms, expected);
+}
+
 TEST(MutationTest, AppliesLocalArrayUnionTransformWithDuplicateUnionElements) {
   // Duplicate entries in your union array should only be added once.
   auto base_data = Map("array", Array(1, 3));