bug(firestore): match fb firestore behaviour (#3205)

* bug(firestore): match fb firestore behaviour

* tests(firestore): removed duplicate test

* Update packages/firestore/e2e/Query/query.e2e.js

* Update packages/firestore/e2e/Query/query.e2e.js

* tests(firestore): update error tests

Co-authored-by: Mike Diarmid <mike.diarmid@gmail.com>
This commit is contained in:
Russell Wheatley
2020-02-18 12:15:54 +00:00
committed by GitHub
parent 72e81e5f99
commit 6311dc8f68
4 changed files with 74 additions and 34 deletions

View File

@@ -58,16 +58,11 @@ describe('firestore().collection().isEqual()', () => {
});
it('returns false when not equal (expensive checks)', () => {
//TODO - the below noted errors on firebase JS SDK, need to change to this behaviour for RNFB firestore
/**
* .where('foo', '==', 'bar')
.orderBy('foo')
*/
const query = firebase
.firestore()
.collection('v6')
.where('foo', '==', 'bar')
.orderBy('foo')
.orderBy('bam')
.limit(1)
.endAt(2);
@@ -86,11 +81,12 @@ describe('firestore().collection().isEqual()', () => {
.orderBy('foob')
.limit(1)
.endAt(2);
const q3 = firebase
.firestore()
.collection('v6')
.where('foo', '==', 'bar')
.orderBy('foo')
.orderBy('baz')
.limit(2)
.endAt(2);
@@ -98,7 +94,7 @@ describe('firestore().collection().isEqual()', () => {
.firestore()
.collection('v6')
.where('foo', '==', 'bar')
.orderBy('foo')
.orderBy('baz')
.limit(1)
.endAt(1);
@@ -118,7 +114,7 @@ describe('firestore().collection().isEqual()', () => {
.firestore()
.collection('v6')
.where('foo', '==', 'bar')
.orderBy('foo')
.orderBy('baz')
.limit(1)
.endAt(2);
@@ -126,7 +122,7 @@ describe('firestore().collection().isEqual()', () => {
.firestore()
.collection('v6')
.where('foo', '==', 'bar')
.orderBy('foo')
.orderBy('baz')
.limit(1)
.endAt(2);

View File

@@ -95,20 +95,6 @@ describe('firestore().collection().orderBy()', () => {
}
});
it('throws if path is not the same as an inequality query', () => {
try {
firebase
.firestore()
.collection('foo')
.where('foo', '>', 123)
.orderBy('bar');
return Promise.reject(new Error('Did not throw an Error.'));
} catch (error) {
error.message.should.containEql('You have a where filter with an inequality');
return Promise.resolve();
}
});
it('throws if duplicating the order field path', () => {
try {
firebase

View File

@@ -27,4 +27,49 @@ describe('FirestoreQuery/FirestoreQueryModifiers', () => {
queryAfter._modifiers._orders.length.should.equal(1);
queryAfter._modifiers._filters.length.should.equal(1);
});
it('throws if where equality operator is invoked, and the where fieldPath parameter matches any orderBy parameter', async () => {
try {
firebase
.firestore()
.collection('v6')
.where('foo', '==', 'bar')
.orderBy('foo')
.limit(1)
.endAt(2);
return Promise.reject(new Error('Did not throw an Error.'));
} catch (error) {
error.message.should.containEql('Invalid query');
}
try {
firebase
.firestore()
.collection('v6')
.where('foo', '==', 'bar')
.orderBy('bar')
.orderBy('foo')
.limit(1)
.endAt(2);
return Promise.reject(new Error('Did not throw an Error.'));
} catch (error) {
error.message.should.containEql('Invalid query');
}
});
it('throws if where inequality operator is invoked, and the where fieldPath does not match initial orderBy parameter', async () => {
try {
firebase
.firestore()
.collection('v6')
.where('foo', '>', 'bar')
.orderBy('bar')
.orderBy('foo')
.limit(1)
.endAt(2);
return Promise.reject(new Error('Did not throw an Error.'));
} catch (error) {
error.message.should.containEql('Invalid query');
}
});
});

View File

@@ -272,22 +272,35 @@ export default class FirestoreQueryModifiers {
}
}
// Skip if no where filters or already validated
if (this._filters.length === 0 || this._orders.length > 1) {
// Skip if no where filters
if (this._filters.length === 0) {
return;
}
// Ensure the first order field path is equal to the inequality filter field path
for (let i = 0; i < this._filters.length; i++) {
const filter = this._filters[i];
// Inequality filter
if (INEQUALITY[filter.operator]) {
if (filter.fieldPath._toPath() !== this._orders[0].fieldPath) {
throw new Error(
`Invalid query. You have a where filter with an inequality (<, <=, >, or >=) on field '${filter.fieldPath._toPath()}' and so you must also use '${filter.fieldPath._toPath()}' as your first Query.orderBy(), but your first Query.orderBy() is on field '${
this._orders[0].fieldPath
}' instead`,
);
const filterFieldPath = filter.fieldPath._toPath();
for (let k = 0; k < this._orders.length; k++) {
const order = this._orders[k];
const orderFieldPath = order.fieldPath;
// Any where() fieldPath parameter cannot match any orderBy() parameter
if (filter.operator === OPERATORS['==']) {
if (filterFieldPath === orderFieldPath) {
throw new Error(
`Invalid query. Query.orderBy() parameter: ${orderFieldPath} cannot be the same as your Query.where() fieldPath parameter: ${filterFieldPath}`,
);
}
}
if (INEQUALITY[filter.operator]) {
// Initial orderBy() parameter has to match every where() fieldPath parameter when inequality operator is invoked
if (filterFieldPath !== this._orders[0].fieldPath) {
throw new Error(
`Invalid query. Initial Query.orderBy() parameter: ${orderFieldPath} has to be the same as the Query.where() fieldPath parameter(s): ${filterFieldPath} when an inequality operator is invoked `,
);
}
}
}
}