Skip to content

Commit 381a1ac

Browse files
milaGGLmilaGGL
authored
Merge bdcfeb6 into 63698a7 
2 parents 63698a7 + bdcfeb6 commit 381a1ac

File tree

5 files changed

+101
-160
lines changed

5 files changed

+101
-160
lines changed

‎packages/firestore/src/core/filter.ts

Lines changed: 0 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -56,8 +56,6 @@ export abstract class Filter {
5656
abstract getFlattenedFilters(): readonly FieldFilter[];
5757

5858
abstract getFilters(): Filter[];
59-
60-
abstract getInequalityFilters(): readonly FieldFilter[];
6159
}
6260

6361
export class FieldFilter extends Filter {
@@ -194,13 +192,6 @@ export class FieldFilter extends Filter {
194192
getFilters(): Filter[] {
195193
return [this];
196194
}
197-
198-
getInequalityFilters(): readonly FieldFilter[] {
199-
if (this.isInequality()) {
200-
return [this];
201-
}
202-
return [];
203-
}
204195
}
205196

206197
export class CompositeFilter extends Filter {
@@ -246,14 +237,6 @@ export class CompositeFilter extends Filter {
246237
getFilters(): Filter[] {
247238
return Object.assign([], this.filters);
248239
}
249-
250-
// Performs a depth-first search to find and return the inequality FieldFilters in the composite
251-
// filter. Returns an empty array if none of the FieldFilters has inequality filters.
252-
getInequalityFilters(): readonly FieldFilter[] {
253-
return this.getFlattenedFilters().filter((filter: FieldFilter) =>
254-
filter.isInequality()
255-
);
256-
}
257240
}
258241

259242
export function compositeFilterIsConjunction(

‎packages/firestore/src/core/query.ts

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ import {
2626
boundSortsAfterDocument,
2727
boundSortsBeforeDocument
2828
} from './bound';
29-
import { Filter } from './filter';
29+
import { FieldFilter, Filter } from './filter';
3030
import { Direction, OrderBy } from './order_by';
3131
import {
3232
canonifyTarget,
@@ -170,11 +170,11 @@ export function queryMatchesAllDocuments(query: Query): boolean {
170170
export function getInequalityFilterFields(query: Query): SortedSet<FieldPath> {
171171
let result = new SortedSet<FieldPath>(FieldPath.comparator);
172172
query.filters.forEach((filter: Filter) => {
173-
const inequalityFields = filter
174-
.getInequalityFilters()
175-
.map(filter => filter.field);
176-
inequalityFields.forEach((field: FieldPath) => {
177-
result = result.add(field);
173+
const subFilters = filter.getFlattenedFilters();
174+
subFilters.forEach((filter: FieldFilter) => {
175+
if (filter.isInequality()) {
176+
result = result.add(filter.field);
177+
}
178178
});
179179
});
180180
return result;

‎packages/firestore/test/integration/api/validation.test.ts

Lines changed: 1 addition & 136 deletions
Original file line numberDiff line numberDiff line change
@@ -1175,146 +1175,11 @@ apiDescribe('Validation:', persistence => {
11751175
}
11761176
);
11771177

1178-
// Multiple Inequality validation tests
1179-
validationIt(persistence, 'can have multiple inequality fields', db => {
1180-
const coll = collection(db, 'test');
1181-
expect(() =>
1182-
query(coll, where('x', '>=', 32), where('y', '<', 42))
1183-
).not.to.throw();
1184-
});
1185-
1186-
validationIt(
1187-
persistence,
1188-
'can have != and inequality queries on different fields',
1189-
db => {
1190-
const coll = collection(db, 'test');
1191-
expect(() =>
1192-
query(coll, where('x', '>', 32), where('y', '!=', 42))
1193-
).not.to.throw();
1194-
}
1195-
);
1196-
11971178
validationIt(
11981179
persistence,
1199-
'can have not-in and inequality queries on different fields',
1180+
'conflicting operators inside a nested composite filter',
12001181
db => {
12011182
const coll = collection(db, 'test');
1202-
expect(() =>
1203-
query(coll, where('y', '>=', 32), where('x', 'not-in', [42]))
1204-
).not.to.throw();
1205-
}
1206-
);
1207-
1208-
validationIt(
1209-
persistence,
1210-
'can have inequality different than orderBy',
1211-
db => {
1212-
const coll = collection(db, 'test');
1213-
// single inequality
1214-
expect(() =>
1215-
query(coll, where('x', '>', 32), orderBy('y'))
1216-
).not.to.throw();
1217-
expect(() =>
1218-
query(coll, orderBy('y'), where('x', '>', 32))
1219-
).not.to.throw();
1220-
expect(() =>
1221-
query(coll, where('x', '>', 32), orderBy('y'), orderBy('x'))
1222-
).not.to.throw();
1223-
expect(() =>
1224-
query(coll, orderBy('y'), orderBy('x'), where('x', '>', 32))
1225-
).not.to.throw();
1226-
expect(() =>
1227-
query(coll, where('x', '!=', 32), orderBy('y'))
1228-
).not.to.throw();
1229-
1230-
// multiple inequality
1231-
expect(() =>
1232-
query(coll, where('x', '>', 32), where('y', '!=', 42), orderBy('z'))
1233-
).not.to.throw();
1234-
expect(() =>
1235-
query(coll, orderBy('y'), where('x', '>', 32), where('y', '<=', 42))
1236-
).not.to.throw();
1237-
expect(() =>
1238-
query(
1239-
coll,
1240-
where('x', '>', 32),
1241-
where('y', '!=', 42),
1242-
orderBy('y'),
1243-
orderBy('x')
1244-
)
1245-
).not.to.throw();
1246-
expect(() =>
1247-
query(
1248-
coll,
1249-
orderBy('y'),
1250-
orderBy('z'),
1251-
where('x', '>', 32),
1252-
where('y', '!=', 42)
1253-
)
1254-
).not.to.throw();
1255-
}
1256-
);
1257-
1258-
validationIt(
1259-
persistence,
1260-
'multiple inequalities inside a nested composite filter',
1261-
db => {
1262-
// Multiple inequality on different fields inside a nested composite filter.
1263-
const coll = collection(db, 'test');
1264-
expect(() =>
1265-
query(
1266-
coll,
1267-
or(
1268-
and(where('a', '==', 'b'), where('c', '>', 'd')),
1269-
and(where('e', '<=', 'f'), where('g', '==', 'h'))
1270-
)
1271-
)
1272-
).not.to.throw();
1273-
1274-
// Multiple inequality on different fields between a field filter and a composite filter.
1275-
expect(() =>
1276-
query(
1277-
coll,
1278-
and(
1279-
or(
1280-
and(where('a', '==', 'b'), where('c', '>=', 'd')),
1281-
and(where('e', '==', 'f'), where('g', '!=', 'h'))
1282-
),
1283-
where('r', '<', 's')
1284-
)
1285-
)
1286-
).not.to.throw();
1287-
1288-
// OrderBy and multiple inequality on different fields.
1289-
expect(() =>
1290-
query(
1291-
coll,
1292-
or(
1293-
and(where('a', '==', 'b'), where('c', '>', 'd')),
1294-
and(where('e', '==', 'f'), where('g', '!=', 'h'))
1295-
),
1296-
orderBy('r'),
1297-
orderBy('a')
1298-
)
1299-
).not.to.throw();
1300-
1301-
// Multiple inequality inside two composite filters.
1302-
expect(() =>
1303-
query(
1304-
coll,
1305-
and(
1306-
or(
1307-
and(where('a', '==', 'b'), where('c', '>=', 'd')),
1308-
and(where('e', '==', 'f'), where('g', '!=', 'h'))
1309-
),
1310-
or(
1311-
and(where('i', '==', 'j'), where('k', '>', 'l')),
1312-
and(where('m', '==', 'n'), where('o', '<', 'p'))
1313-
)
1314-
)
1315-
)
1316-
).not.to.throw();
1317-
13181183
// Composite queries can validate conflicting operators.
13191184
expect(() =>
13201185
query(

‎packages/firestore/test/unit/core/query.test.ts

Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -777,6 +777,99 @@ describe('Query', () => {
777777
);
778778
});
779779

780+
it("generates the correct implicit order by's for multiple inequality", () => {
781+
assertImplicitOrderBy(
782+
query(
783+
'foo',
784+
filter('a', '<', 5),
785+
filter('aa', '>', 5),
786+
filter('b', '>', 5),
787+
filter('A', '>', 5)
788+
),
789+
orderBy('A'),
790+
orderBy('a'),
791+
orderBy('aa'),
792+
orderBy('b'),
793+
orderBy(DOCUMENT_KEY_NAME)
794+
);
795+
796+
// numbers
797+
assertImplicitOrderBy(
798+
query(
799+
'foo',
800+
filter('a', '<', 5),
801+
filter('1', '>', 5),
802+
filter('19', '>', 5),
803+
filter('2', '>', 5)
804+
),
805+
orderBy('1'),
806+
orderBy('19'),
807+
orderBy('2'),
808+
orderBy('a'),
809+
810+
orderBy(DOCUMENT_KEY_NAME)
811+
);
812+
813+
// nested fields
814+
assertImplicitOrderBy(
815+
query(
816+
'foo',
817+
filter('a', '<', 5),
818+
filter('aa', '>', 5),
819+
filter('a.a', '>', 5)
820+
),
821+
orderBy('a'),
822+
orderBy('a.a'),
823+
orderBy('aa'),
824+
orderBy(DOCUMENT_KEY_NAME)
825+
);
826+
827+
// special characters
828+
assertImplicitOrderBy(
829+
query(
830+
'foo',
831+
filter('a', '<', 5),
832+
filter('_a', '>', 5),
833+
filter('a.a', '>', 5)
834+
),
835+
orderBy('_a'),
836+
orderBy('a'),
837+
orderBy('a.a'),
838+
orderBy(DOCUMENT_KEY_NAME)
839+
);
840+
841+
// field name with dot
842+
assertImplicitOrderBy(
843+
query(
844+
'foo',
845+
filter('a', '<', 5),
846+
filter('a.z', '>', 5), // nested field
847+
filter('`a.a`', '>', 5) // field name with dot
848+
),
849+
orderBy('a'),
850+
orderBy('a.z'),
851+
orderBy('`a.a`'),
852+
orderBy(DOCUMENT_KEY_NAME)
853+
);
854+
855+
// composite filter
856+
assertImplicitOrderBy(
857+
query(
858+
'foo',
859+
filter('a', '<', 5),
860+
andFilter(
861+
orFilter(filter('b', '>=', 1), filter('c', '<=', 1)),
862+
orFilter(filter('d', '>', 1), filter('e', '==', 1))
863+
)
864+
),
865+
orderBy('a'),
866+
orderBy('b'),
867+
orderBy('c'),
868+
orderBy('d'),
869+
orderBy(DOCUMENT_KEY_NAME)
870+
);
871+
});
872+
780873
it('matchesAllDocuments() considers filters, orders and bounds', () => {
781874
const baseQuery = newQueryForPath(ResourcePath.fromString('collection'));
782875
expect(queryMatchesAllDocuments(baseQuery)).to.be.true;

‎packages/firestore/test/util/helpers.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -227,7 +227,7 @@ export function path(path: string, offset?: number): ResourcePath {
227227
}
228228

229229
export function field(path: string): FieldPath {
230-
return new FieldPath(path.split('.'));
230+
return FieldPath.fromServerFormat(path);
231231
}
232232

233233
export function fieldIndex(

0 commit comments

Comments
 (0)