Fix startAfter/endBefore for orderByKeys queries (#4363)

Author: jmwski

.changeset/wet-windows-itch.md

@@ -0,0 +1,4 @@
+---
+'@firebase/database': patch
+---
+Fixed an issue with startAfter/endBefore when used in orderByKey queries

packages/database/src/api/Query.ts

@@ -97,25 +97,19 @@ export class Query {
         'Query: When ordering by key, you may only pass one argument to ' +
         'startAt(), endAt(), or equalTo().';
       const wrongArgTypeError =
-        'Query: When ordering by key, the argument passed to startAt(), endAt(),' +
-        'or equalTo() must be a string.';
+        'Query: When ordering by key, the argument passed to startAt(), startAfter(), ' +
+        'endAt(), endBefore(), or equalTo() must be a string.';
       if (params.hasStart()) {
         const startName = params.getIndexStartName();
-        if (
-          startName !== MIN_NAME &&
-          !(params.hasStartAfter() && startName === MAX_NAME)
-        ) {
+        if (startName !== MIN_NAME) {
           throw new Error(tooManyArgsError);
         } else if (typeof startNode !== 'string') {
           throw new Error(wrongArgTypeError);
         }
       }
       if (params.hasEnd()) {
         const endName = params.getIndexEndName();
-        if (
-          endName !== MAX_NAME &&
-          !(params.hasEndBefore() && endName === MIN_NAME)
-        ) {
+        if (endName !== MAX_NAME) {
           throw new Error(tooManyArgsError);
         } else if (typeof endNode !== 'string') {
           throw new Error(wrongArgTypeError);
@@ -128,7 +122,8 @@ export class Query {
       ) {
         throw new Error(
           'Query: When ordering by priority, the first argument passed to startAt(), ' +
-            'endAt(), or equalTo() must be a valid priority value (null, a number, or a string).'
+            'startAfter() endAt(), endBefore(), or equalTo() must be a valid priority value ' +
+            '(null, a number, or a string).'
         );
       }
     } else {
@@ -142,8 +137,8 @@ export class Query {
         (endNode != null && typeof endNode === 'object')
       ) {
         throw new Error(
-          'Query: First argument passed to startAt(), endAt(), or equalTo() cannot be ' +
-            'an object.'
+          'Query: First argument passed to startAt(), startAfter(), endAt(), endBefore(), or ' +
+            'equalTo() cannot be an object.'
         );
       }
     }
@@ -162,7 +157,8 @@ export class Query {
       !params.hasAnchoredLimit()
     ) {
       throw new Error(
-        "Query: Can't combine startAt(), endAt(), and limit(). Use limitToFirst() or limitToLast() instead."
+        "Query: Can't combine startAt(), startAfter(), endAt(), endBefore(), and limit(). Use " +
+          'limitToFirst() or limitToLast() instead.'
       );
     }
   }
@@ -617,13 +613,13 @@ export class Query {
     validateKey('Query.equalTo', 2, name, true);
     if (this.queryParams_.hasStart()) {
       throw new Error(
-        'Query.equalTo: Starting point was already set (by another call to startAt or ' +
+        'Query.equalTo: Starting point was already set (by another call to startAt/startAfter or ' +
           'equalTo).'
       );
     }
     if (this.queryParams_.hasEnd()) {
       throw new Error(
-        'Query.equalTo: Ending point was already set (by another call to endAt or ' +
+        'Query.equalTo: Ending point was already set (by another call to endAt/endBefore or ' +
           'equalTo).'
       );
     }

packages/database/src/core/view/QueryParams.ts

@@ -289,13 +289,21 @@ export class QueryParams {
   }
 
   startAfter(indexValue: unknown, key?: string | null): QueryParams {
-    let childKey: string;
-    if (key == null) {
-      childKey = MAX_NAME;
+    let params: QueryParams;
+    if (this.index_ === KEY_INDEX) {
+      if (typeof indexValue === 'string') {
+        indexValue = successor(indexValue as string);
+      }
+      params = this.startAt(indexValue, key);
     } else {
-      childKey = successor(key);
+      let childKey: string;
+      if (key == null) {
+        childKey = MAX_NAME;
+      } else {
+        childKey = successor(key);
+      }
+      params = this.startAt(indexValue, childKey);
     }
-    const params: QueryParams = this.startAt(indexValue, childKey);
     params.startAfterSet_ = true;
     return params;
   }
@@ -324,12 +332,20 @@ export class QueryParams {
 
   endBefore(indexValue: unknown, key?: string | null): QueryParams {
     let childKey: string;
-    if (key == null) {
-      childKey = MIN_NAME;
+    let params: QueryParams;
+    if (this.index_ === KEY_INDEX) {
+      if (typeof indexValue === 'string') {
+        indexValue = predecessor(indexValue as string);
+      }
+      params = this.endAt(indexValue, key);
     } else {
-      childKey = predecessor(key);
+      if (key == null) {
+        childKey = MIN_NAME;
+      } else {
+        childKey = predecessor(key);
+      }
+      params = this.endAt(indexValue, childKey);
     }
-    const params: QueryParams = this.endAt(indexValue, childKey);
     params.endBeforeSet_ = true;
     return params;
   }

packages/database/test/query.test.ts

@@ -1377,6 +1377,28 @@ describe('Query Tests', () => {
     expect(removed).to.equal('b ');
   });
 
+  it('Ensure startAfter on key index works', async () => {
+    const node = getRandomNode() as Reference;
+    const childOne = node.push();
+    const childTwo = node.push();
+    await childOne.set(1);
+    await childTwo.set(2);
+    const snap = await node.orderByKey().startAfter(childOne.key).get();
+    expect(Object.keys(snap.val())).to.deep.equal([childTwo.key]);
+    expect(Object.values(snap.val())).to.deep.equal([snap.val()[childTwo.key]]);
+  });
+
+  it('Ensure endBefore on key index works', async () => {
+    const node = getRandomNode() as Reference;
+    const childOne = node.push();
+    const childTwo = node.push();
+    await childOne.set(1);
+    await childTwo.set(2);
+    const snap = await node.orderByKey().endBefore(childTwo.key).get();
+    expect(Object.keys(snap.val())).to.deep.equal([childOne.key]);
+    expect(Object.values(snap.val())).to.deep.equal([snap.val()[childOne.key]]);
+  });
+
   it('Ensure startAt / endAt with priority works.', async () => {
     const node = getRandomNode() as Reference;