Skip to content

Commit

Permalink
fix(firestore): remove exception throw on inequality queries on diffe…
Browse files Browse the repository at this point in the history 
…rent fields
  • Loading branch information
@russellwheatley @mikehardy
russellwheatley authored and mikehardy committed Jul 15, 2024
1 parent 0ebd1dd commit da24246
Show file tree
Hide file tree
Showing 6 changed files with 276 additions and 360 deletions.
8 changes: 8 additions & 0 deletions .github/workflows/scripts/firestore.indexes.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,14 @@
{
"fieldPath": "b",
"order": "ASCENDING"
},
{
"fieldPath": "foo.bar",
"order": "ASCENDING"
},
{
"fieldPath": "baz",
"order": "ASCENDING"
}
]
}
Expand Down
161 changes: 74 additions & 87 deletions packages/firestore/e2e/Query/where.and.filter.e2e.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -117,22 +117,29 @@ describe(' firestore().collection().where(AND Filters)', function () {
.where(Filter.and(Filter('foo.bar', '==', null), Filter('foo.bar', '!=', null)));
});

it('throws if multiple inequalities on different paths is provided', function () {
try {
firebase
.firestore()
.collection(COLLECTION)
.where(Filter.and(Filter('foo.bar', '>', 123), Filter('bar', '>', 123)));
it('allows multiple inequalities (excluding `!=`) on different paths provided', async function () {
const colRef = firebase
.firestore()
.collection(`${COLLECTION}/filter/different-path-inequality-filter`);
const expected = { foo: { bar: 300 }, bar: 200 };
await Promise.all([
colRef.add({ foo: { bar: 1 }, bar: 1 }),
colRef.add(expected),
colRef.add(expected),
]);

return Promise.reject(new Error('Did not throw an Error.'));
} catch (error) {
error.message.should.containEql('All where filters with an inequality');
return Promise.resolve();
}
const snapshot = await colRef
.where(Filter.and(Filter('foo.bar', '>', 123), Filter('bar', '>', 123)))
.get();

snapshot.size.should.eql(2);
snapshot.forEach(s => {
s.data().should.eql(jet.contextify(expected));
});
});

it('allows inequality on the same path', function () {
firebase
it('allows inequality on the same path', async function () {
await firebase
.firestore()
.collection(COLLECTION)
.where(
Expand Down Expand Up @@ -323,38 +330,24 @@ describe(' firestore().collection().where(AND Filters)', function () {
}
});

it("should throw error when combining '!=' operator with any other inequality operator on a different field", async function () {
const ref = firebase.firestore().collection(COLLECTION);

try {
ref.where(Filter.and(Filter('foo.bar', '!=', 1), Filter('differentField', '>', 2)));
return Promise.reject(new Error('Did not throw an Error on >.'));
} catch (error) {
error.message.should.containEql('must be on the same field.');
}

try {
ref.where(Filter.and(Filter('foo.bar', '!=', 1), Filter('differentField', '<', 2)));
return Promise.reject(new Error('Did not throw an Error on <.'));
} catch (error) {
error.message.should.containEql('must be on the same field.');
}

try {
ref.where(Filter.and(Filter('foo.bar', '!=', 1), Filter('differentField', '<=', 2)));
return Promise.reject(new Error('Did not throw an Error <=.'));
} catch (error) {
error.message.should.containEql('must be on the same field.');
}

try {
ref.where(Filter.and(Filter('foo.bar', '!=', 1), Filter('differentField', '>=', 2)));
return Promise.reject(new Error('Did not throw an Error >=.'));
} catch (error) {
error.message.should.containEql('must be on the same field.');
}
it("should allow query when combining '!=' operator with any other inequality operator on a different field", async function () {
const colRef = firebase
.firestore()
.collection(`${COLLECTION}/filter/inequality-combine-not-equal`);
const expected = { foo: { bar: 300 }, bar: 200 };
await Promise.all([
colRef.add({ foo: { bar: 1 }, bar: 1 }),
colRef.add(expected),
colRef.add(expected),
]);

return Promise.resolve();
const snapshot = await colRef
.where(Filter.and(Filter('foo.bar', '>', 123), Filter('bar', '!=', 123)))
.get();
snapshot.size.should.eql(2);
snapshot.forEach(s => {
s.data().should.eql(jet.contextify(expected));
});
});

/* Queries */
Expand Down Expand Up @@ -774,19 +767,26 @@ describe(' firestore().collection().where(AND Filters)', function () {
);
});

it('throws if multiple inequalities on different paths is provided', function () {
it('allows multiple inequalities (excluding `!=`) on different paths provided', async function () {
const { getFirestore, collection, query, and, where } = firestoreModular;
try {
query(
collection(getFirestore(), COLLECTION),
and(where('foo.bar', '>', 123), where('bar', '>', 123)),
);
const colRef = collection(getFirestore(), `${COLLECTION}/filter/different-path-inequality`);

return Promise.reject(new Error('Did not throw an Error.'));
} catch (error) {
error.message.should.containEql('All where filters with an inequality');
return Promise.resolve();
}
const expected = { foo: { bar: 300 }, bar: 200 };
await Promise.all([
colRef.add({ foo: { bar: 1 }, bar: 1 }),
colRef.add(expected),
colRef.add(expected),
]);

const snapshot = await query(
colRef,
and(where('foo.bar', '>', 123), where('bar', '>', 123)),
).get();

snapshot.size.should.eql(2);
snapshot.forEach(s => {
s.data().should.eql(jet.contextify(expected));
});
});

it('allows inequality on the same path', function () {
Expand Down Expand Up @@ -979,39 +979,26 @@ describe(' firestore().collection().where(AND Filters)', function () {
}
});

it("should throw error when combining '!=' operator with any other inequality operator on a different field", async function () {
const { getFirestore, collection, query, where, and } = firestoreModular;
const ref = collection(getFirestore(), COLLECTION);

try {
query(ref, and(where('foo.bar', '!=', 1), where('differentField', '>', 2)));
return Promise.reject(new Error('Did not throw an Error on >.'));
} catch (error) {
error.message.should.containEql('must be on the same field.');
}

try {
query(ref, and(where('foo.bar', '!=', 1), where('differentField', '<', 2)));
return Promise.reject(new Error('Did not throw an Error on <.'));
} catch (error) {
error.message.should.containEql('must be on the same field.');
}

try {
query(ref, and(where('foo.bar', '!=', 1), where('differentField', '<=', 2)));
return Promise.reject(new Error('Did not throw an Error <=.'));
} catch (error) {
error.message.should.containEql('must be on the same field.');
}

try {
query(ref, and(where('foo.bar', '!=', 1), where('differentField', '>=', 2)));
return Promise.reject(new Error('Did not throw an Error >=.'));
} catch (error) {
error.message.should.containEql('must be on the same field.');
}
it("should allow query when combining '!=' operator with any other inequality operator on a different field", async function () {
const { query, where, and } = firestoreModular;
const colRef = firebase
.firestore()
.collection(`${COLLECTION}/filter/inequality-combine-not-equal`);
const expected = { foo: { bar: 300 }, bar: 200 };
await Promise.all([
colRef.add({ foo: { bar: 1 }, bar: 1 }),
colRef.add(expected),
colRef.add(expected),
]);

return Promise.resolve();
const snapshot = await query(
colRef,
and(where('foo.bar', '>', 123), where('bar', '!=', 123)),
).get();
snapshot.size.should.eql(2);
snapshot.forEach(s => {
s.data().should.eql(jet.contextify(expected));
});
});

/* Queries */
Expand Down
156 changes: 70 additions & 86 deletions packages/firestore/e2e/Query/where.e2e.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -96,18 +96,23 @@ describe('firestore().collection().where()', function () {
firebase.firestore().collection(COLLECTION).where('foo.bar', '!=', null);
});

it('throws if multiple inequalities on different paths is provided', function () {
try {
firebase
.firestore()
.collection(COLLECTION)
.where('foo.bar', '>', 123)
.where('bar', '>', 123);
return Promise.reject(new Error('Did not throw an Error.'));
} catch (error) {
error.message.should.containEql('All where filters with an inequality');
return Promise.resolve();
}
it('allows multiple inequalities (excluding `!=`) on different paths provided', async function () {
const colRef = firebase
.firestore()
.collection(`${COLLECTION}/filter/different-path-inequality`);
const expected = { foo: { bar: 300 }, bar: 200 };
await Promise.all([
colRef.add({ foo: { bar: 1 }, bar: 1 }),
colRef.add(expected),
colRef.add(expected),
]);

const snapshot = await colRef.where('foo.bar', '>', 123).where('bar', '>', 123).get();

snapshot.size.should.eql(2);
snapshot.forEach(s => {
s.data().should.eql(jet.contextify(expected));
});
});

it('allows inequality on the same path', function () {
Expand Down Expand Up @@ -520,38 +525,22 @@ describe('firestore().collection().where()', function () {
}
});

it("should throw error when combining '!=' operator with any other inequality operator on a different field", async function () {
const ref = firebase.firestore().collection(COLLECTION);

try {
ref.where('test', '!=', 1).where('differentField', '>', 1);
return Promise.reject(new Error('Did not throw an Error on >.'));
} catch (error) {
error.message.should.containEql('must be on the same field.');
}

try {
ref.where('test', '!=', 1).where('differentField', '<', 1);
return Promise.reject(new Error('Did not throw an Error on <.'));
} catch (error) {
error.message.should.containEql('must be on the same field.');
}

try {
ref.where('test', '!=', 1).where('differentField', '<=', 1);
return Promise.reject(new Error('Did not throw an Error <=.'));
} catch (error) {
error.message.should.containEql('must be on the same field.');
}

try {
ref.where('test', '!=', 1).where('differentField', '>=', 1);
return Promise.reject(new Error('Did not throw an Error >=.'));
} catch (error) {
error.message.should.containEql('must be on the same field.');
}
it("should allow query when combining '!=' operator with any other inequality operator on a different field", async function () {
const colRef = firebase
.firestore()
.collection(`${COLLECTION}/filter/inequality-combine-not-equal`);
const expected = { foo: { bar: 300 }, bar: 200 };
await Promise.all([
colRef.add({ foo: { bar: 1 }, bar: 1 }),
colRef.add(expected),
colRef.add(expected),
]);

return Promise.resolve();
const snapshot = await colRef.where('foo.bar', '>', 123).where('bar', '!=', 123).get();
snapshot.size.should.eql(2);
snapshot.forEach(s => {
s.data().should.eql(jet.contextify(expected));
});
});

it('should handle where clause after sort by', async function () {
Expand Down Expand Up @@ -659,19 +648,28 @@ describe('firestore().collection().where()', function () {
query(collection(getFirestore(), COLLECTION), where('foo.bar', '!=', null));
});

it('throws if multiple inequalities on different paths is provided', function () {
const { getFirestore, collection, query, where } = firestoreModular;
try {
query(
collection(getFirestore(), COLLECTION),
where('foo.bar', '>', 123),
where('bar', '>', 123),
);
return Promise.reject(new Error('Did not throw an Error.'));
} catch (error) {
error.message.should.containEql('All where filters with an inequality');
return Promise.resolve();
}
it('allows multiple inequalities (excluding `!=`) on different paths provided', async function () {
const { query, where } = firestoreModular;

const colRef = firebase
.firestore()
.collection(`${COLLECTION}/filter/different-path-inequality`);
const expected = { foo: { bar: 300 }, bar: 200 };
await Promise.all([
colRef.add({ foo: { bar: 1 }, bar: 1 }),
colRef.add(expected),
colRef.add(expected),
]);

const snapshot = await query(
colRef,
where('foo.bar', '>', 123),
where('bar', '>', 123),
).get();
snapshot.size.should.eql(2);
snapshot.forEach(s => {
s.data().should.eql(jet.contextify(expected));
});
});

it('allows inequality on the same path', function () {
Expand Down Expand Up @@ -1123,39 +1121,25 @@ describe('firestore().collection().where()', function () {
}
});

it("should throw error when combining '!=' operator with any other inequality operator on a different field", async function () {
it("should allow query when combining '!=' operator with any other inequality operator on a different field", async function () {
const { getFirestore, collection, query, where } = firestoreModular;
const ref = collection(getFirestore(), COLLECTION);

try {
query(ref, where('test', '!=', 1), where('differentField', '>', 1));
return Promise.reject(new Error('Did not throw an Error on >.'));
} catch (error) {
error.message.should.containEql('must be on the same field.');
}

try {
query(ref, where('test', '!=', 1), where('differentField', '<', 1));
return Promise.reject(new Error('Did not throw an Error on <.'));
} catch (error) {
error.message.should.containEql('must be on the same field.');
}

try {
query(ref, where('test', '!=', 1), where('differentField', '<=', 1));
return Promise.reject(new Error('Did not throw an Error <=.'));
} catch (error) {
error.message.should.containEql('must be on the same field.');
}
const colRef = collection(
getFirestore(),
`${COLLECTION}/filter/inequality-combine-not-equal`,
);
const expected = { foo: { bar: 300 }, bar: 200 };
await Promise.all([
colRef.add({ foo: { bar: 1 }, bar: 1 }),
colRef.add(expected),
colRef.add(expected),
]);

try {
query(ref, where('test', '!=', 1), where('differentField', '>=', 1));
return Promise.reject(new Error('Did not throw an Error >=.'));
} catch (error) {
error.message.should.containEql('must be on the same field.');
}
const snapshot = await query(colRef, where('foo.bar', '>', 123), where('bar', '!=', 1)).get();

return Promise.resolve();
snapshot.size.should.eql(2);
snapshot.forEach(s => {
s.data().should.eql(jet.contextify(expected));
});
});

it('should handle where clause after sort by', async function () {
Expand Down
Loading

0 comments on commit da24246

Please sign in to comment.