fix(NODE-3173): Preserve sort key order for numeric string keys (#2790)

mongodb · Apr 29, 2021 · 730f43a · 730f43a
1 parent 16e7064
commit 730f43a
Show file tree

Hide file tree

Showing 2 changed files with 36 additions and 12 deletions.
diff --git a/lib/utils.js b/lib/utils.js
@@ -40,7 +40,7 @@ var formatSortValue = (exports.formatSortValue = function(sortDirection) {
 });
 
 var formattedOrderClause = (exports.formattedOrderClause = function(sortValue) {
-  var orderBy = {};
+  var orderBy = new Map();
   if (sortValue == null) return null;
   if (Array.isArray(sortValue)) {
     if (sortValue.length === 0) {
@@ -49,15 +49,22 @@ var formattedOrderClause = (exports.formattedOrderClause = function(sortValue) {
 
     for (var i = 0; i < sortValue.length; i++) {
       if (sortValue[i].constructor === String) {
-        orderBy[sortValue[i]] = 1;
+        orderBy.set(`${sortValue[i]}`, 1);
       } else {
-        orderBy[sortValue[i][0]] = formatSortValue(sortValue[i][1]);
+        orderBy.set(`${sortValue[i][0]}`, formatSortValue(sortValue[i][1]));
       }
     }
   } else if (sortValue != null && typeof sortValue === 'object') {
-    orderBy = sortValue;
+    if (sortValue instanceof Map) {
+      orderBy = sortValue;
+    } else {
+      var sortKeys = Object.keys(sortValue);
+      for (var k of sortKeys) {
+        orderBy.set(k, sortValue[k]);
+      }
+    }
   } else if (typeof sortValue === 'string') {
-    orderBy[sortValue] = 1;
+    orderBy.set(`${sortValue}`, 1);
   } else {
     throw new Error(
       'Illegal sort clause, must be of the form ' +

diff --git a/test/functional/spec-runner/index.js b/test/functional/spec-runner/index.js
@@ -384,6 +384,18 @@ function validateExpectations(commandEvents, spec, savedSessionData) {
 
     const actualCommand = actual.command;
     const expectedCommand = expected.command;
+    if (expectedCommand.sort) {
+      // TODO: This is a workaround that works because all sorts in the specs
+      // are objects with one key; ideally we'd want to adjust the spec definitions
+      // to indicate whether order matters for any given key and set general
+      // expectations accordingly (see NODE-3235)
+      expect(Object.keys(expectedCommand.sort)).to.have.lengthOf(1);
+      expect(actualCommand.sort).to.be.instanceOf(Map);
+      expect(actualCommand.sort.size).to.equal(1);
+      const expectedKey = Object.keys(expectedCommand.sort)[0];
+      expect(actualCommand.sort).to.have.all.keys(expectedKey);
+      actualCommand.sort = { [expectedKey]: actualCommand.sort.get(expectedKey) };
+    }
 
     expect(actualCommand)
       .withSessionData(savedSessionData)
@@ -392,18 +404,23 @@ function validateExpectations(commandEvents, spec, savedSessionData) {
 }
 
 function normalizeCommandShapes(commands) {
-  return commands.map(command =>
-    JSON.parse(
+  return commands.map(def => {
+    const output = JSON.parse(
       EJSON.stringify(
         {
-          command: command.command,
-          commandName: command.command_name ? command.command_name : command.commandName,
-          databaseName: command.database_name ? command.database_name : command.databaseName
+          command: def.command,
+          commandName: def.command_name ? def.command_name : def.commandName,
+          databaseName: def.database_name ? def.database_name : def.databaseName
         },
         { relaxed: true }
       )
-    )
-  );
+    );
+    // TODO: this is a workaround to preserve sort Map type until NODE-3235 is completed
+    if (def.command.sort) {
+      output.command.sort = def.command.sort;
+    }
+    return output;
+  });
 }
 
 function extractCrudResult(result, operation) {