Skip to content

Commit

Permalink
Fix startAfter/endBefore for orderByKeys queries (#4363)
Browse files Browse the repository at this point in the history
  • Loading branch information
jmwski authored Feb 1, 2021
1 parent d9b945f commit 0af2bdf
Show file tree
Hide file tree
Showing 4 changed files with 63 additions and 25 deletions.
4 changes: 4 additions & 0 deletions .changeset/wet-windows-itch.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
---
'@firebase/database': patch
---
Fixed an issue with startAfter/endBefore when used in orderByKey queries
28 changes: 12 additions & 16 deletions packages/database/src/api/Query.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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 {
Expand All @@ -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.'
);
}
}
Expand All @@ -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.'
);
}
}
Expand Down Expand Up @@ -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).'
);
}
Expand Down
34 changes: 25 additions & 9 deletions packages/database/src/core/view/QueryParams.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down Expand Up @@ -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;
}
Expand Down
22 changes: 22 additions & 0 deletions packages/database/test/query.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down

0 comments on commit 0af2bdf

Please sign in to comment.