SERVER-61941 Make $sortArray comparator satisfy "Compare" requirement with -1 sort

2 files changed, 105 insertions, 16 deletions

diff --git a/src/mongo/db/update/pattern_cmp.cpp b/src/mongo/db/update/pattern_cmp.cpp
index 96c25c812c2..3dbaa6c386e 100644
--- a/src/mongo/db/update/pattern_cmp.cpp
+++ b/src/mongo/db/update/pattern_cmp.cpp
@@ -108,11 +108,9 @@ PatternValueCmp::PatternValueCmp(const BSONObj& pattern,
 bool PatternValueCmp::operator()(const Value& lhs, const Value& rhs) const {
     namespace dps = ::mongo::dotted_path_support;
     if (useWholeValue) {
-        const bool ascending = ValueComparator(collator).getLessThan()(lhs, rhs);
-
-        const bool reversed = (sortPattern.firstElement().number() < 0);
-
-        return (reversed ? !ascending : ascending);
+        const bool descending = (sortPattern.firstElement().number() < 0);
+        return (descending ? ValueComparator(collator).getLessThan()(rhs, lhs)
+                           : ValueComparator(collator).getLessThan()(lhs, rhs));
     } else {
         BSONObj lhsObj = lhs.isObject() ? lhs.getDocument().toBson() : lhs.wrap("");
         BSONObj rhsObj = rhs.isObject() ? rhs.getDocument().toBson() : rhs.wrap("");
diff --git a/src/mongo/db/update/pattern_cmp_test.cpp b/src/mongo/db/update/pattern_cmp_test.cpp
index a6b0f44aadd..83f4c940e8a 100644
--- a/src/mongo/db/update/pattern_cmp_test.cpp
+++ b/src/mongo/db/update/pattern_cmp_test.cpp
@@ -32,6 +32,8 @@
 #include "mongo/bson/mutable/algorithm.h"
 #include "mongo/bson/mutable/document.h"
 #include "mongo/bson/mutable/element.h"
+#include "mongo/db/exec/document_value/document_value_test_util.h"
+#include "mongo/db/exec/document_value/value.h"
 #include "mongo/db/jsobj.h"
 #include "mongo/db/json.h"
 #include "mongo/db/query/collation/collator_interface.h"
@@ -44,9 +46,9 @@ namespace {
 using mongo::mutablebson::Element;
 using mongo::mutablebson::sortChildren;
 
-class ObjectArray : public mongo::unittest::Test {
+class PatternElemCmpTest : public mongo::unittest::Test {
 public:
-    ObjectArray() : _doc(), _size(0) {}
+    PatternElemCmpTest() : _doc(), _size(0) {}
 
     virtual void setUp() {
         Element arr = _doc.makeElementArray("x");
@@ -80,7 +82,7 @@ private:
     size_t _size;
 };
 
-TEST_F(ObjectArray, NormalOrder) {
+TEST_F(PatternElemCmpTest, PatternElementCmpNormalOrder) {
     const CollatorInterface* collator = nullptr;
     addObj(fromjson("{b:1, a:1}"));
     addObj(fromjson("{a:3, b:2}"));
@@ -93,7 +95,7 @@ TEST_F(ObjectArray, NormalOrder) {
     ASSERT_BSONOBJ_EQ(getOrigObj(2), getSortedObj(1));
 }
 
-TEST_F(ObjectArray, MixedOrder) {
+TEST_F(PatternElemCmpTest, PatternElementCmpMixedOrder) {
     const CollatorInterface* collator = nullptr;
     addObj(fromjson("{b:1, a:1}"));
     addObj(fromjson("{a:3, b:2}"));
@@ -106,7 +108,7 @@ TEST_F(ObjectArray, MixedOrder) {
     ASSERT_BSONOBJ_EQ(getOrigObj(2), getSortedObj(2));
 }
 
-TEST_F(ObjectArray, ExtraFields) {
+TEST_F(PatternElemCmpTest, PatternElementCmpExtraFields) {
     const CollatorInterface* collator = nullptr;
     addObj(fromjson("{b:1, c:2, a:1}"));
     addObj(fromjson("{c:1, a:3, b:2}"));
@@ -119,7 +121,7 @@ TEST_F(ObjectArray, ExtraFields) {
     ASSERT_BSONOBJ_EQ(getOrigObj(2), getSortedObj(1));
 }
 
-TEST_F(ObjectArray, MissingFields) {
+TEST_F(PatternElemCmpTest, PatternElementCmpMissingFields) {
     const CollatorInterface* collator = nullptr;
     addObj(fromjson("{a:2, b:2}"));
     addObj(fromjson("{a:1}"));
@@ -132,7 +134,7 @@ TEST_F(ObjectArray, MissingFields) {
     ASSERT_BSONOBJ_EQ(getOrigObj(2), getSortedObj(2));
 }
 
-TEST_F(ObjectArray, NestedFields) {
+TEST_F(PatternElemCmpTest, PatternElementCmpNestedFields) {
     const CollatorInterface* collator = nullptr;
     addObj(fromjson("{a:{b:{c:2, d:0}}}"));
     addObj(fromjson("{a:{b:{c:1, d:2}}}"));
@@ -145,7 +147,7 @@ TEST_F(ObjectArray, NestedFields) {
     ASSERT_BSONOBJ_EQ(getOrigObj(2), getSortedObj(2));
 }
 
-TEST_F(ObjectArray, SimpleNestedFields) {
+TEST_F(PatternElemCmpTest, PatternElementCmpSimpleNestedFields) {
     const CollatorInterface* collator = nullptr;
     addObj(fromjson("{a:{b: -1}}"));
     addObj(fromjson("{a:{b: -100}}"));
@@ -158,7 +160,7 @@ TEST_F(ObjectArray, SimpleNestedFields) {
     ASSERT_BSONOBJ_EQ(getOrigObj(2), getSortedObj(2));
 }
 
-TEST_F(ObjectArray, NestedInnerObjectDescending) {
+TEST_F(PatternElemCmpTest, PatternElementCmpNestedInnerObjectDescending) {
     const CollatorInterface* collator = nullptr;
     addObj(fromjson("{a:{b:{c:2, d:0}}}"));
     addObj(fromjson("{a:{b:{c:1, d:2}}}"));
@@ -171,7 +173,7 @@ TEST_F(ObjectArray, NestedInnerObjectDescending) {
     ASSERT_BSONOBJ_EQ(getOrigObj(2), getSortedObj(1));
 }
 
-TEST_F(ObjectArray, NestedInnerObjectAscending) {
+TEST_F(PatternElemCmpTest, PatternElementCmpNestedInnerObjectAscending) {
     const CollatorInterface* collator = nullptr;
     addObj(fromjson("{a:{b:{c:2, d:0}}}"));
     addObj(fromjson("{a:{b:{c:1, d:2}}}"));
@@ -184,7 +186,7 @@ TEST_F(ObjectArray, NestedInnerObjectAscending) {
     ASSERT_BSONOBJ_EQ(getOrigObj(1), getSortedObj(2));
 }
 
-TEST_F(ObjectArray, SortRespectsCollation) {
+TEST_F(PatternElemCmpTest, PatternElementCmpSortRespectsCollation) {
     CollatorInterfaceMock collator(CollatorInterfaceMock::MockType::kReverseString);
     addObj(fromjson("{a: 'abg'}"));
     addObj(fromjson("{a: 'aca'}"));
@@ -197,5 +199,94 @@ TEST_F(ObjectArray, SortRespectsCollation) {
     ASSERT_BSONOBJ_EQ(getOrigObj(2), getSortedObj(1));
 }
 
+static void assertExpectedSortResults(std::vector<Value> originalArr,
+                                      std::vector<Value> expectedArr,
+                                      BSONObj sortPattern,
+                                      const CollatorInterface* collator = nullptr) {
+    // For testing sorting we do not need to specify the exact 'originalElement' BSONElement for
+    // 'PatternValueCmp' and only care about the sort pattern and the collator (if any).
+    const auto sortBy = PatternValueCmp(sortPattern, BSONElement(), collator);
+
+    std::sort(originalArr.begin(), originalArr.end(), sortBy);
+
+    ASSERT_EQUALS(originalArr.size(), expectedArr.size());
+    for (size_t idx = 0; idx < originalArr.size(); ++idx) {
+        ASSERT_VALUE_EQ(originalArr[idx], expectedArr[idx]);
+    }
+}
+
+TEST(PatternValueCmpTest, PatternValueCmpAscendingOrder) {
+    assertExpectedSortResults(
+        {Value(3), Value(2), Value(1)}, {Value(1), Value(2), Value(3)}, fromjson("{'': 1}"));
+}
+
+TEST(PatternValueCmpTest, PatternValueCmpDescendingOrder) {
+    assertExpectedSortResults(
+        {Value(1), Value(2), Value(3)}, {Value(3), Value(2), Value(1)}, fromjson("{'': -1}"));
+}
+
+TEST(PatternValueCmpTest, PatternValueCmpStrings) {
+    assertExpectedSortResults(
+        {Value("a"_sd), Value("b"_sd)}, {Value("a"_sd), Value("b"_sd)}, fromjson("{'': 1}"));
+}
+
+TEST(PatternValueCmpTest, PatternValueCmpObjects) {
+    assertExpectedSortResults(
+        {Value(fromjson("{a: 3}")), Value(fromjson("{a: 2}")), Value(fromjson("{a: 1}"))},
+        {Value(fromjson("{a: 1}")), Value(fromjson("{a: 2}")), Value(fromjson("{a: 3}"))},
+        fromjson("{a: 1}"));
+    assertExpectedSortResults({Value(fromjson("{}")), Value(fromjson("[]")), Value(fromjson("[]"))},
+                              {Value(fromjson("[]")), Value(fromjson("[]")), Value(fromjson("{}"))},
+                              fromjson("{'': -1}"));
+    assertExpectedSortResults({Value(fromjson("{}")), Value(fromjson("{}"))},
+                              {Value(fromjson("{}")), Value(fromjson("{}"))},
+                              fromjson("{'': -1}"));
+}
+
+TEST(PatternValueCmpTest, PatternValueCmpObjectsDottedPath) {
+    std::vector<Value> arr = {Value(fromjson("{a: {b: 1}}")),
+                              Value(fromjson("{a: {b: 2}}")),
+                              Value(fromjson("{a: {b: 3}}"))};
+
+    assertExpectedSortResults(arr,
+                              {Value(fromjson("{a: {b: 3}}")),
+                               Value(fromjson("{a: {b: 2}}")),
+                               Value(fromjson("{a: {b: 1}}"))},
+                              fromjson("{'a.b': -1}"));
+}
+
+TEST(PatternValueCmpTest, PatternValueCmpObjectsPathDoesNotExist) {
+    std::vector<Value> arr = {Value(fromjson("{a: 2}")),
+                              Value(fromjson("{b: 2}")),
+                              Value(fromjson("{a: 1}")),
+                              Value(fromjson("{c: 0}"))};
+
+    assertExpectedSortResults(arr,
+                              {Value(fromjson("{b: 2}")),
+                               Value(fromjson("{c: 0}")),
+                               Value(fromjson("{a: 1}")),
+                               Value(fromjson("{a: 2}"))},
+                              fromjson("{a: 1}"));
+
+    assertExpectedSortResults(arr,
+                              {Value(fromjson("{a: 2}")),
+                               Value(fromjson("{a: 1}")),
+                               Value(fromjson("{b: 2}")),
+                               Value(fromjson("{c: 0}"))},
+                              fromjson("{a: -1}"));
+}
+
+TEST(PatternValueCmpTest, PatternValueCmpWithCollator) {
+    const auto sortPattern = fromjson("{'': 1}");
+    CollatorInterfaceMock collator(CollatorInterfaceMock::MockType::kReverseString);
+
+    assertExpectedSortResults({Value("abc"_sd), Value("acb"_sd), Value("cba"_sd)},
+                              {Value("abc"_sd), Value("acb"_sd), Value("cba"_sd)},
+                              sortPattern);
+    assertExpectedSortResults({Value("abc"_sd), Value("acb"_sd), Value("cba"_sd)},
+                              {Value("cba"_sd), Value("acb"_sd), Value("abc"_sd)},
+                              sortPattern,
+                              &collator);
+}
 }  // namespace
 }  // namespace mongo