Skip to content

Commit

Permalink
Update REST and wire protocol query params for startAfter, `endBefo…
Browse files Browse the repository at this point in the history 
…re` (#6706)

* Update REST and wire protocol query constants for startAfter, endBefore

* Update query unit tests

* Update RTDB emulator version

* Extend syncpoint spec protocol

* Account for bound inclusivity with LimitedFilter

* Address PR feedback

* Add test case with

* Address PR feedback
  • Loading branch information
@tohhsinpei
tohhsinpei authored Nov 18, 2022
1 parent a9add5e commit 37dd6f6
Show file tree
Hide file tree
Showing 8 changed files with 388 additions and 82 deletions.
6 changes: 6 additions & 0 deletions .changeset/empty-doors-brush.md
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
'@firebase/database': patch
'@firebase/database-compat': patch
---

Use new wire protocol parameters for startAfter, endBefore.
62 changes: 52 additions & 10 deletions packages/database-compat/test/query.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"}'
);
});

Expand Down
65 changes: 26 additions & 39 deletions packages/database/src/core/view/QueryParams.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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',
Expand All @@ -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'
}
Expand All @@ -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;
Expand All @@ -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.
*/
Expand Down Expand Up @@ -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_;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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_);
}
}

Expand All @@ -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_;
Expand Down
65 changes: 38 additions & 27 deletions packages/database/src/core/view/filter/LimitedFilter.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -119,20 +125,15 @@ 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;
if (!this.withinDirectionalStart(next)) {
// if we have not reached the start, skip to the next element
continue;
} else if (!this.withinDirectionalEnd(next)) {
// if we have reached the end, stop adding elements
break;
} else {
inRange =
this.index_.compare(next, this.rangedFilter_.getEndPost()) <= 0;
}
if (inRange) {
filtered = filtered.updateImmediateChild(next.name, next.node);
count++;
} else {
// if we have reached the end post, we cannot keep adding elemments
break;
}
}
} else {
Expand All @@ -142,33 +143,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 {
Expand Down Expand Up @@ -300,4 +289,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_.compare(
this.rangedFilter_.getStartPost(),
node
);
return this.startIsInclusive_ ? compareRes <= 0 : compareRes < 0;
};

private withinEndPost = (node: NamedNode) => {
const compareRes = this.index_.compare(
node,
this.rangedFilter_.getEndPost()
);
return this.endIsInclusive_ ? compareRes <= 0 : compareRes < 0;
};
}
Loading

0 comments on commit 37dd6f6

Please sign in to comment.