Merge pull request #329 from mixmaxhq/jordi/COMM-185-paginate-null-values

fix: properly page through undefs and nulls

Author: jsalvata

README.md

@@ -54,7 +54,7 @@ Call `find()` with the following parameters:
             exist, the results will be secondarily ordered by the _id.
           2. Indexed. For large collections, this should be indexed for query performance.
           3. Immutable. If the value changes between paged queries, it could appear twice.
-          4. Complete. A value must exist for all documents.
+          4. Consistent. All values (except undefined and null values) must be of the same type.
         The default is to use the Mongo built-in '_id' field, which satisfies the above criteria.
         The only reason to NOT use the Mongo _id field is if you chose to implement your own ids.
       -sortAscending {Boolean} True to sort using paginatedField ascending (default is false - descending).

package-lock.json

@@ -39,7 +39,7 @@
   "homepage": "https://github.com/mixmaxhq/mongo-cursor-pagination#readme",
   "dependencies": {
     "base64-url": "^2.2.0",
-    "bson": "^4.1.0",
+    "bson": "^4.7.0",
     "object-path": "^0.11.5",
     "projection-utils": "^1.1.0",
     "semver": "^5.4.1",
@@ -61,7 +61,7 @@
     "mockgoose": "^8.0.4",
     "mongodb": "^2.2.11",
     "mongodb-memory-server": "^5.2.11",
-    "mongoist": "2.3.0",
+    "mongoist": "^2.5.5",
     "mongoose": "5.11.10",
     "prettier": "^1.19.1",
     "semantic-release": "^17.2.3"

package.json

@@ -28,6 +28,7 @@ const config = require('./config');
  *        2. Immutable. If the value changes between paged queries, it could appear twice.
  *        3. Accessible. The field must be present in the aggregation's end result so the
  *          aggregation steps added at the end of the pipeline to implement the paging can access it.
+          4. Consistent. All values (except undefined and null values) must be of the same type.
  *      The default is to use the Mongo built-in '_id' field, which satisfies the above criteria.
  *      The only reason to NOT use the Mongo _id field is if you chose to implement your own ids.
  *    -sortAscending {boolean} Whether to sort in ascending order by the `paginatedField`.

src/aggregate.js

@@ -20,6 +20,7 @@ const config = require('./config');
  *          exist, the results will be secondarily ordered by the _id.
  *        2. Indexed. For large collections, this should be indexed for query performance.
  *        3. Immutable. If the value changes between paged queries, it could appear twice.
+          4. Consistent. All values (except undefined and null values) must be of the same type.
  *      The default is to use the Mongo built-in '_id' field, which satisfies the above criteria.
  *      The only reason to NOT use the Mongo _id field is if you chose to implement your own ids.
  *    -sortAscending {boolean} Whether to sort in ascending order by the `paginatedField`.

src/find.js

@@ -1,15 +1,21 @@
 const { EJSON } = require('bson');
 const base64url = require('base64-url');
 
+// BSON can't encode undefined values, so we will use this value instead:
+const BSON_UNDEFINED = '__mixmax__undefined__';
+
 /**
- * These will take a BSON object (an database result returned by the MongoDB library) and
- * encode/decode as a URL-safe string.
+ * These will take a paging handle (`next` or `previous`) and encode/decode it
+ * as a string which can be passed in a URL.
  */
 
 module.exports.encode = function(obj) {
+  if (Array.isArray(obj) && obj[0] === undefined) obj[0] = BSON_UNDEFINED;
   return base64url.encode(EJSON.stringify(obj));
 };
 
 module.exports.decode = function(str) {
-  return EJSON.parse(base64url.decode(str));
+  const obj = EJSON.parse(base64url.decode(str));
+  if (Array.isArray(obj) && obj[0] === BSON_UNDEFINED) obj[0] = undefined;
+  return obj;
 };

src/utils/bsonUrlEncoding.js

@@ -21,7 +21,9 @@ function encodePaginationTokens(params, response) {
 
   if (response.previous) {
     let previousPaginatedField = objectPath.get(response.previous, params.paginatedField);
-    if (params.sortCaseInsensitive) previousPaginatedField = previousPaginatedField.toLowerCase();
+    if (params.sortCaseInsensitive) {
+      previousPaginatedField = previousPaginatedField?.toLowerCase?.() ?? '';
+    }
     if (shouldSecondarySortOnId) {
       response.previous = bsonUrlEncoding.encode([previousPaginatedField, response.previous._id]);
     } else {
@@ -30,7 +32,9 @@ function encodePaginationTokens(params, response) {
   }
   if (response.next) {
     let nextPaginatedField = objectPath.get(response.next, params.paginatedField);
-    if (params.sortCaseInsensitive) nextPaginatedField = nextPaginatedField.toLowerCase();
+    if (params.sortCaseInsensitive) {
+      nextPaginatedField = nextPaginatedField?.toLowerCase?.() ?? '';
+    }
     if (shouldSecondarySortOnId) {
       response.next = bsonUrlEncoding.encode([nextPaginatedField, response.next._id]);
     } else {
@@ -112,36 +116,90 @@ module.exports = {
 
     const sortAsc =
       (!params.sortAscending && params.previous) || (params.sortAscending && !params.previous);
-    const comparisonOp = sortAsc ? '$gt' : '$lt';
 
     // a `next` cursor will have precedence over a `previous` cursor.
     const op = params.next || params.previous;
 
     if (params.paginatedField == '_id') {
-      return {
-        _id: {
-          [comparisonOp]: op,
-        },
-      };
+      if (sortAsc) {
+        return { _id: { $gt: op } };
+      } else {
+        return { _id: { $lt: op } };
+      }
     } else {
       const field = params.sortCaseInsensitive ? '__lc' : params.paginatedField;
-      return {
-        $or: [
-          {
-            [field]: {
-              [comparisonOp]: op[0],
-            },
-          },
-          {
-            [field]: {
-              $eq: op[0],
-            },
-            _id: {
-              [comparisonOp]: op[1],
-            },
-          },
-        ],
-      };
+
+      const notUndefined = { [field]: { $exists: true } };
+      const onlyUndefs = { [field]: { $exists: false } };
+      const notNullNorUndefined = { [field]: { $ne: null } };
+      const nullOrUndefined = { [field]: null };
+      const onlyNulls = { $and: [{ [field]: { $exists: true } }, { [field]: null }] };
+
+      const [paginatedFieldValue, idValue] = op;
+      switch (paginatedFieldValue) {
+        case null:
+          if (sortAsc) {
+            return {
+              $or: [
+                notNullNorUndefined,
+                {
+                  ...onlyNulls,
+                  _id: { $gt: idValue },
+                },
+              ],
+            };
+          } else {
+            return {
+              $or: [
+                onlyUndefs,
+                {
+                  ...onlyNulls,
+                  _id: { $lt: idValue },
+                },
+              ],
+            };
+          }
+        case undefined:
+          if (sortAsc) {
+            return {
+              $or: [
+                notUndefined,
+                {
+                  ...onlyUndefs,
+                  _id: { $gt: idValue },
+                },
+              ],
+            };
+          } else {
+            return {
+              ...onlyUndefs,
+              _id: { $lt: idValue },
+            };
+          }
+        default:
+          if (sortAsc) {
+            return {
+              $or: [
+                { [field]: { $gt: paginatedFieldValue } },
+                {
+                  [field]: { $eq: paginatedFieldValue },
+                  _id: { $gt: idValue },
+                },
+              ],
+            };
+          } else {
+            return {
+              $or: [
+                { [field]: { $lt: paginatedFieldValue } },
+                nullOrUndefined,
+                {
+                  [field]: { $eq: paginatedFieldValue },
+                  _id: { $lt: idValue },
+                },
+              ],
+            };
+          }
+      }
     }
   },
 };