Update REST and wire protocol query params for startAfter, endBefore

.changeset/empty-doors-brush.md

@@ -0,0 +1,6 @@
+---
+'@firebase/database': patch
+'@firebase/database-compat': patch
+---
+
+Use new wire protocol parameters for startAfter, endBefore.

packages/database-compat/test/query.test.ts

@@ -375,34 +375,76 @@ describe('Query Tests', () => {
     expect(queryId(path)).to.equal('default');
 
     expect(queryId(path.startAt('pri', 'name'))).to.equal(
-      '{"sn":"name","sp":"pri"}'
+      '{"sin":true,"sn":"name","sp":"pri"}'
     );
     expect(queryId(path.startAfter('pri', 'name'))).to.equal(
-      '{"sn":"name-","sp":"pri"}'
+      '{"sin":false,"sn":"name","sp":"pri"}'
     );
+    expect(queryId(path.endAt('pri', 'name'))).to.equal(
+      '{"ein":true,"en":"name","ep":"pri"}'
+    );
+    expect(queryId(path.endBefore('pri', 'name'))).to.equal(
+      '{"ein":false,"en":"name","ep":"pri"}'
+    );
+
     expect(queryId(path.startAt('spri').endAt('epri'))).to.equal(
-      '{"ep":"epri","sp":"spri"}'
+      '{"ein":true,"ep":"epri","sin":true,"sp":"spri"}'
+    );
+    expect(queryId(path.startAt('spri').endBefore('epri'))).to.equal(
+      '{"ein":false,"en":"[MIN_NAME]","ep":"epri","sin":true,"sp":"spri"}'
     );
     expect(queryId(path.startAfter('spri').endAt('epri'))).to.equal(
-      '{"ep":"epri","sn":"[MAX_NAME]","sp":"spri"}'
+      '{"ein":true,"ep":"epri","sin":false,"sn":"[MAX_NAME]","sp":"spri"}'
+    );
+    expect(queryId(path.startAfter('spri').endBefore('epri'))).to.equal(
+      '{"ein":false,"en":"[MIN_NAME]","ep":"epri","sin":false,"sn":"[MAX_NAME]","sp":"spri"}'
     );
+
     expect(
       queryId(path.startAt('spri', 'sname').endAt('epri', 'ename'))
-    ).to.equal('{"en":"ename","ep":"epri","sn":"sname","sp":"spri"}');
+    ).to.equal(
+      '{"ein":true,"en":"ename","ep":"epri","sin":true,"sn":"sname","sp":"spri"}'
+    );
+    expect(
+      queryId(path.startAt('spri', 'sname').endBefore('epri', 'ename'))
+    ).to.equal(
+      '{"ein":false,"en":"ename","ep":"epri","sin":true,"sn":"sname","sp":"spri"}'
+    );
     expect(
       queryId(path.startAfter('spri', 'sname').endAt('epri', 'ename'))
-    ).to.equal('{"en":"ename","ep":"epri","sn":"sname-","sp":"spri"}');
+    ).to.equal(
+      '{"ein":true,"en":"ename","ep":"epri","sin":false,"sn":"sname","sp":"spri"}'
+    );
+    expect(
+      queryId(path.startAfter('spri', 'sname').endBefore('epri', 'ename'))
+    ).to.equal(
+      '{"ein":false,"en":"ename","ep":"epri","sin":false,"sn":"sname","sp":"spri"}'
+    );
+
     expect(queryId(path.startAt('pri').limitToFirst(100))).to.equal(
-      '{"l":100,"sp":"pri","vf":"l"}'
+      '{"l":100,"sin":true,"sp":"pri","vf":"l"}'
     );
     expect(queryId(path.startAfter('pri').limitToFirst(100))).to.equal(
-      '{"l":100,"sn":"[MAX_NAME]","sp":"pri","vf":"l"}'
+      '{"l":100,"sin":false,"sn":"[MAX_NAME]","sp":"pri","vf":"l"}'
+    );
+    expect(queryId(path.endAt('pri').limitToLast(100))).to.equal(
+      '{"ein":true,"ep":"pri","l":100,"vf":"r"}'
     );
+    expect(queryId(path.endBefore('pri').limitToLast(100))).to.equal(
+      '{"ein":false,"en":"[MIN_NAME]","ep":"pri","l":100,"vf":"r"}'
+    );
+
     expect(queryId(path.startAt('bar').orderByChild('foo'))).to.equal(
-      '{"i":"foo","sp":"bar"}'
+      '{"i":"foo","sin":true,"sp":"bar"}'
     );
     expect(queryId(path.startAfter('bar').orderByChild('foo'))).to.equal(
-      '{"i":"foo","sn":"[MAX_NAME]","sp":"bar"}'
+      '{"i":"foo","sin":false,"sn":"[MAX_NAME]","sp":"bar"}'
+    );
+    expect(queryId(path.endAt('bar').orderByChild('foo'))).to.equal(
+      '{"ein":true,"ep":"bar","i":"foo"}'
+    );
+    expect(queryId(path.endBefore('bar').orderByChild('foo'))).to.equal(
+      '{"ein":false,"en":"[MIN_NAME]","ep":"bar","i":"foo"}'
     );
   });
 

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

@@ -22,7 +22,6 @@ import { KEY_INDEX } from '../snap/indexes/KeyIndex';
 import { PathIndex } from '../snap/indexes/PathIndex';
 import { PRIORITY_INDEX, PriorityIndex } from '../snap/indexes/PriorityIndex';
 import { VALUE_INDEX } from '../snap/indexes/ValueIndex';
-import { predecessor, successor } from '../util/NextPushId';
 import { MAX_NAME, MIN_NAME } from '../util/util';
 
 import { IndexedFilter } from './filter/IndexedFilter';
@@ -36,8 +35,10 @@ import { RangedFilter } from './filter/RangedFilter';
 const enum WIRE_PROTOCOL_CONSTANTS {
   INDEX_START_VALUE = 'sp',
   INDEX_START_NAME = 'sn',
+  INDEX_START_IS_INCLUSIVE = 'sin',
   INDEX_END_VALUE = 'ep',
   INDEX_END_NAME = 'en',
+  INDEX_END_IS_INCLUSIVE = 'ein',
   LIMIT = 'l',
   VIEW_FROM = 'vf',
   VIEW_FROM_LEFT = 'l',
@@ -53,8 +54,10 @@ const enum REST_QUERY_CONSTANTS {
   PRIORITY_INDEX = '$priority',
   VALUE_INDEX = '$value',
   KEY_INDEX = '$key',
+  START_AFTER = 'startAfter',
   START_AT = 'startAt',
   END_AT = 'endAt',
+  END_BEFORE = 'endBefore',
   LIMIT_TO_FIRST = 'limitToFirst',
   LIMIT_TO_LAST = 'limitToLast'
 }
@@ -70,10 +73,10 @@ export class QueryParams {
   limitSet_ = false;
   startSet_ = false;
   startNameSet_ = false;
-  startAfterSet_ = false;
+  startAfterSet_ = false; // can only be true if startSet_ is true
   endSet_ = false;
   endNameSet_ = false;
-  endBeforeSet_ = false;
+  endBeforeSet_ = false; // can only be true if endSet_ is true
   limit_ = 0;
   viewFrom_ = '';
   indexStartValue_: unknown | null = null;
@@ -86,14 +89,6 @@ export class QueryParams {
     return this.startSet_;
   }
 
-  hasStartAfter(): boolean {
-    return this.startAfterSet_;
-  }
-
-  hasEndBefore(): boolean {
-    return this.endBeforeSet_;
-  }
-
   /**
    * @returns True if it would return from left.
    */
@@ -191,10 +186,12 @@ export class QueryParams {
     copy.limitSet_ = this.limitSet_;
     copy.limit_ = this.limit_;
     copy.startSet_ = this.startSet_;
+    copy.startAfterSet_ = this.startAfterSet_;
     copy.indexStartValue_ = this.indexStartValue_;
     copy.startNameSet_ = this.startNameSet_;
     copy.indexStartName_ = this.indexStartName_;
     copy.endSet_ = this.endSet_;
+    copy.endBeforeSet_ = this.endBeforeSet_;
     copy.indexEndValue_ = this.indexEndValue_;
     copy.endNameSet_ = this.endNameSet_;
     copy.indexEndName_ = this.indexEndName_;
@@ -274,19 +271,10 @@ export function queryParamsStartAfter(
   key?: string | null
 ): QueryParams {
   let params: QueryParams;
-  if (queryParams.index_ === KEY_INDEX) {
-    if (typeof indexValue === 'string') {
-      indexValue = successor(indexValue as string);
-    }
+  if (queryParams.index_ === KEY_INDEX || !!key) {
     params = queryParamsStartAt(queryParams, indexValue, key);
   } else {
-    let childKey: string;
-    if (key == null) {
-      childKey = MAX_NAME;
-    } else {
-      childKey = successor(key);
-    }
-    params = queryParamsStartAt(queryParams, indexValue, childKey);
+    params = queryParamsStartAt(queryParams, indexValue, MAX_NAME);
   }
   params.startAfterSet_ = true;
   return params;
@@ -318,20 +306,11 @@ export function queryParamsEndBefore(
   indexValue: unknown,
   key?: string | null
 ): QueryParams {
-  let childKey: string;
   let params: QueryParams;
-  if (queryParams.index_ === KEY_INDEX) {
-    if (typeof indexValue === 'string') {
-      indexValue = predecessor(indexValue as string);
-    }
+  if (queryParams.index_ === KEY_INDEX || !!key) {
     params = queryParamsEndAt(queryParams, indexValue, key);
   } else {
-    if (key == null) {
-      childKey = MIN_NAME;
-    } else {
-      childKey = predecessor(key);
-    }
-    params = queryParamsEndAt(queryParams, indexValue, childKey);
+    params = queryParamsEndAt(queryParams, indexValue, MIN_NAME);
   }
   params.endBeforeSet_ = true;
   return params;
@@ -374,18 +353,22 @@ export function queryParamsToRestQueryStringParameters(
   qs[REST_QUERY_CONSTANTS.ORDER_BY] = stringify(orderBy);
 
   if (queryParams.startSet_) {
-    qs[REST_QUERY_CONSTANTS.START_AT] = stringify(queryParams.indexStartValue_);
+    const startParam = queryParams.startAfterSet_
+      ? REST_QUERY_CONSTANTS.START_AFTER
+      : REST_QUERY_CONSTANTS.START_AT;
+    qs[startParam] = stringify(queryParams.indexStartValue_);
     if (queryParams.startNameSet_) {
-      qs[REST_QUERY_CONSTANTS.START_AT] +=
-        ',' + stringify(queryParams.indexStartName_);
+      qs[startParam] += ',' + stringify(queryParams.indexStartName_);
     }
   }
 
   if (queryParams.endSet_) {
-    qs[REST_QUERY_CONSTANTS.END_AT] = stringify(queryParams.indexEndValue_);
+    const endParam = queryParams.endBeforeSet_
+      ? REST_QUERY_CONSTANTS.END_BEFORE
+      : REST_QUERY_CONSTANTS.END_AT;
+    qs[endParam] = stringify(queryParams.indexEndValue_);
     if (queryParams.endNameSet_) {
-      qs[REST_QUERY_CONSTANTS.END_AT] +=
-        ',' + stringify(queryParams.indexEndName_);
+      qs[endParam] += ',' + stringify(queryParams.indexEndName_);
     }
   }
 
@@ -411,12 +394,16 @@ export function queryParamsGetQueryObject(
       obj[WIRE_PROTOCOL_CONSTANTS.INDEX_START_NAME] =
         queryParams.indexStartName_;
     }
+    obj[WIRE_PROTOCOL_CONSTANTS.INDEX_START_IS_INCLUSIVE] =
+      !queryParams.startAfterSet_;
   }
   if (queryParams.endSet_) {
     obj[WIRE_PROTOCOL_CONSTANTS.INDEX_END_VALUE] = queryParams.indexEndValue_;
     if (queryParams.endNameSet_) {
       obj[WIRE_PROTOCOL_CONSTANTS.INDEX_END_NAME] = queryParams.indexEndName_;
     }
+    obj[WIRE_PROTOCOL_CONSTANTS.INDEX_END_IS_INCLUSIVE] =
+      !queryParams.endBeforeSet_;
   }
   if (queryParams.limitSet_) {
     obj[WIRE_PROTOCOL_CONSTANTS.LIMIT] = queryParams.limit_;

packages/database/src/core/view/filter/LimitedFilter.ts

@@ -46,11 +46,17 @@ export class LimitedFilter implements NodeFilter {
 
   private readonly reverse_: boolean;
 
+  private readonly startIsInclusive_: boolean;
+
+  private readonly endIsInclusive_: boolean;
+
   constructor(params: QueryParams) {
     this.rangedFilter_ = new RangedFilter(params);
     this.index_ = params.getIndex();
     this.limit_ = params.getLimit();
     this.reverse_ = !params.isViewFromLeft();
+    this.startIsInclusive_ = !params.startAfterSet_;
+    this.endIsInclusive_ = !params.endBeforeSet_;
   }
   updateChild(
     snap: Node,
@@ -119,19 +125,17 @@ export class LimitedFilter implements NodeFilter {
         let count = 0;
         while (iterator.hasNext() && count < this.limit_) {
           const next = iterator.getNext();
-          let inRange;
-          if (this.reverse_) {
-            inRange =
-              this.index_.compare(this.rangedFilter_.getStartPost(), next) <= 0;
-          } else {
-            inRange =
-              this.index_.compare(next, this.rangedFilter_.getEndPost()) <= 0;
-          }
+          const inRange =
+            this.withinDirectionalStart(next) &&
+            this.withinDirectionalEnd(next);
           if (inRange) {
             filtered = filtered.updateImmediateChild(next.name, next.node);
             count++;
+          } else if (!this.withinDirectionalStart(next)) {
+            // if we have not reached the start, skip to the next element
+            continue;
           } else {
-            // if we have reached the end post, we cannot keep adding elemments
+            // if we have reached the end, stop adding elements
             break;
           }
         }
@@ -142,33 +146,21 @@ export class LimitedFilter implements NodeFilter {
         filtered = filtered.updatePriority(
           ChildrenNode.EMPTY_NODE
         ) as ChildrenNode;
-        let startPost;
-        let endPost;
-        let cmp;
+
         let iterator;
         if (this.reverse_) {
           iterator = filtered.getReverseIterator(this.index_);
-          startPost = this.rangedFilter_.getEndPost();
-          endPost = this.rangedFilter_.getStartPost();
-          const indexCompare = this.index_.getCompare();
-          cmp = (a: NamedNode, b: NamedNode) => indexCompare(b, a);
         } else {
           iterator = filtered.getIterator(this.index_);
-          startPost = this.rangedFilter_.getStartPost();
-          endPost = this.rangedFilter_.getEndPost();
-          cmp = this.index_.getCompare();
         }
 
         let count = 0;
-        let foundStartPost = false;
         while (iterator.hasNext()) {
           const next = iterator.getNext();
-          if (!foundStartPost && cmp(startPost, next) <= 0) {
-            // start adding
-            foundStartPost = true;
-          }
           const inRange =
-            foundStartPost && count < this.limit_ && cmp(next, endPost) <= 0;
+            count < this.limit_ &&
+            this.withinDirectionalStart(next) &&
+            this.withinDirectionalEnd(next);
           if (inRange) {
             count++;
           } else {
@@ -300,4 +292,26 @@ export class LimitedFilter implements NodeFilter {
       return snap;
     }
   }
+
+  private withinDirectionalStart = (node: NamedNode) =>
+    this.reverse_ ? this.withinEndPost(node) : this.withinStartPost(node);
+
+  private withinDirectionalEnd = (node: NamedNode) =>
+    this.reverse_ ? this.withinStartPost(node) : this.withinEndPost(node);
+
+  private withinStartPost = (node: NamedNode) => {
+    const compareRes = this.index_.getCompare()(
+      this.rangedFilter_.getStartPost(),
+      node
+    );
+    return this.startIsInclusive_ ? compareRes <= 0 : compareRes < 0;
+  };
+
+  private withinEndPost = (node: NamedNode) => {
+    const compareRes = this.index_.getCompare()(
+      node,
+      this.rangedFilter_.getEndPost()
+    );
+    return this.endIsInclusive_ ? compareRes <= 0 : compareRes < 0;
+  };
 }