Skip to content

Commit f5426a5

Browse files
tom-andersentom-andersen
authored
Implement configureFieldIndexes with tests (#6403)
* Implement ConfigureFieldIndexes * Implement ConfigureFieldIndexes * Implement ConfigureFieldIndexes * Implement ConfigureFieldIndexes * Enable Firestore client side indexing * Enable Firestore client side indexing * Fix CountingQueryEngine * Fix CountingQueryEngine * Fix CountingQueryEngine * Add spec test * Remove comment * Update yarn.lock * Add firestore changeset * Whitespace * Dependency fix
1 parent 34c503c commit f5426a5

23 files changed

Lines changed: 932 additions & 73 deletions

‎.changeset/five-yaks-travel.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@firebase/firestore': patch
3+
---
4+
5+
Add internal implementation of setIndexConfiguration

‎packages/firestore/package.json‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,7 @@
9999
"@rollup/plugin-json": "4.1.0",
100100
"@types/eslint": "7.29.0",
101101
"@types/json-stable-stringify": "1.0.34",
102+
"chai-exclude": "2.1.0",
102103
"json-stable-stringify": "1.0.1",
103104
"protobufjs": "6.11.3",
104105
"rollup": "2.72.1",

‎packages/firestore/src/api/credentials.ts‎

Lines changed: 16 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -473,24 +473,25 @@ export class FirebaseAppCheckTokenProvider
473473
asyncQueue: AsyncQueue,
474474
changeListener: CredentialChangeListener<string>
475475
): void {
476-
const onTokenChanged: (tokenResult: AppCheckTokenResult) => Promise<void> =
477-
tokenResult => {
478-
if (tokenResult.error != null) {
479-
logDebug(
480-
'FirebaseAppCheckTokenProvider',
481-
`Error getting App Check token; using placeholder token instead. Error: ${tokenResult.error.message}`
482-
);
483-
}
484-
const tokenUpdated = tokenResult.token !== this.latestAppCheckToken;
485-
this.latestAppCheckToken = tokenResult.token;
476+
const onTokenChanged: (
477+
tokenResult: AppCheckTokenResult
478+
) => Promise<void> = tokenResult => {
479+
if (tokenResult.error != null) {
486480
logDebug(
487481
'FirebaseAppCheckTokenProvider',
488-
`Received ${tokenUpdated ? 'new' : 'existing'} token.`
482+
`Error getting App Check token; using placeholder token instead. Error: ${tokenResult.error.message}`
489483
);
490-
return tokenUpdated
491-
? changeListener(tokenResult.token)
492-
: Promise.resolve();
493-
};
484+
}
485+
const tokenUpdated = tokenResult.token !== this.latestAppCheckToken;
486+
this.latestAppCheckToken = tokenResult.token;
487+
logDebug(
488+
'FirebaseAppCheckTokenProvider',
489+
`Received ${tokenUpdated ? 'new' : 'existing'} token.`
490+
);
491+
return tokenUpdated
492+
? changeListener(tokenResult.token)
493+
: Promise.resolve();
494+
};
494495

495496
this.tokenListener = (tokenResult: AppCheckTokenResult) => {
496497
asyncQueue.enqueueRetryable(() => onTokenChanged(tokenResult));

‎packages/firestore/src/api/database.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,7 @@ import { AsyncQueue } from '../util/async_queue';
5656
import { newAsyncQueue } from '../util/async_queue_impl';
5757
import { Code, FirestoreError } from '../util/error';
5858
import { cast } from '../util/input_validation';
59+
import { logWarn } from '../util/log';
5960
import { Deferred } from '../util/promise';
6061

6162
import { LoadBundleTask } from './bundle';
@@ -332,7 +333,7 @@ function setPersistenceProviders(
332333
if (!canFallbackFromIndexedDbError(error)) {
333334
throw error;
334335
}
335-
console.warn(
336+
logWarn(
336337
'Error enabling offline persistence. Falling back to ' +
337338
'persistence disabled: ' +
338339
error

‎packages/firestore/src/api/index_configuration.ts‎

Lines changed: 21 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,9 @@
1515
* limitations under the License.
1616
*/
1717

18+
import { getLocalStore } from '../core/firestore_client';
1819
import { fieldPathFromDotSeparatedString } from '../lite-api/user_data_reader';
20+
import { localStoreConfigureFieldIndexes } from '../local/local_store_impl';
1921
import {
2022
FieldIndex,
2123
IndexKind,
@@ -24,6 +26,7 @@ import {
2426
} from '../model/field_index';
2527
import { Code, FirestoreError } from '../util/error';
2628
import { cast } from '../util/input_validation';
29+
import { logWarn } from '../util/log';
2730

2831
import { ensureFirestoreConfigured, Firestore } from './database';
2932

@@ -150,17 +153,29 @@ export function setIndexConfiguration(
150153
jsonOrConfiguration: string | IndexConfiguration
151154
): Promise<void> {
152155
firestore = cast(firestore, Firestore);
153-
ensureFirestoreConfigured(firestore);
156+
const client = ensureFirestoreConfigured(firestore);
154157

158+
// PORTING NOTE: We don't return an error if the user has not enabled
159+
// persistence since `enableIndexeddbPersistence()` can fail on the Web.
160+
if (!client.offlineComponents?.indexBackfillerScheduler) {
161+
logWarn('Cannot enable indexes when persistence is disabled');
162+
return Promise.resolve();
163+
}
164+
const parsedIndexes = parseIndexes(jsonOrConfiguration);
165+
return getLocalStore(client).then(localStore =>
166+
localStoreConfigureFieldIndexes(localStore, parsedIndexes)
167+
);
168+
}
169+
170+
export function parseIndexes(
171+
jsonOrConfiguration: string | IndexConfiguration
172+
): FieldIndex[] {
155173
const indexConfiguration =
156174
typeof jsonOrConfiguration === 'string'
157175
? (tryParseJson(jsonOrConfiguration) as IndexConfiguration)
158176
: jsonOrConfiguration;
159177
const parsedIndexes: FieldIndex[] = [];
160178

161-
// PORTING NOTE: We don't return an error if the user has not enabled
162-
// persistence since `enableIndexeddbPersistence()` can fail on the Web.
163-
164179
if (Array.isArray(indexConfiguration.indexes)) {
165180
for (const index of indexConfiguration.indexes) {
166181
const collectionGroup = tryGetString(index, 'collectionGroup');
@@ -194,9 +209,7 @@ export function setIndexConfiguration(
194209
);
195210
}
196211
}
197-
198-
// TODO(indexing): Configure indexes
199-
return Promise.resolve();
212+
return parsedIndexes;
200213
}
201214

202215
function tryParseJson(json: string): Record<string, unknown> {
@@ -205,7 +218,7 @@ function tryParseJson(json: string): Record<string, unknown> {
205218
} catch (e) {
206219
throw new FirestoreError(
207220
Code.INVALID_ARGUMENT,
208-
'Failed to parse JSON:' + (e as Error)?.message
221+
'Failed to parse JSON: ' + (e as Error)?.message
209222
);
210223
}
211224
}

‎packages/firestore/src/local/index_backfiller.ts‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@ import { debugAssert } from '../util/assert';
2525
import { AsyncQueue, DelayedOperation, TimerId } from '../util/async_queue';
2626
import { logDebug } from '../util/log';
2727

28-
import { INDEXING_ENABLED } from './indexeddb_schema';
2928
import { ignoreIfPrimaryLeaseLoss, LocalStore } from './local_store';
3029
import { LocalWriteResult } from './local_store_impl';
3130
import { Persistence, Scheduler } from './persistence';
@@ -60,9 +59,7 @@ export class IndexBackfillerScheduler implements Scheduler {
6059
this.task === null,
6160
'Cannot start an already started IndexBackfillerScheduler'
6261
);
63-
if (INDEXING_ENABLED) {
64-
this.schedule(INITIAL_BACKFILL_DELAY_MS);
65-
}
62+
this.schedule(INITIAL_BACKFILL_DELAY_MS);
6663
}
6764

6865
stop(): void {

‎packages/firestore/src/local/indexeddb_schema.ts‎

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -28,11 +28,6 @@ import {
2828
import { EncodedResourcePath } from './encoded_resource_path';
2929
import { DbTimestampKey } from './indexeddb_sentinels';
3030

31-
// TODO(indexing): Remove this constant
32-
export const INDEXING_ENABLED = false;
33-
34-
export const INDEXING_SCHEMA_VERSION = 15;
35-
3631
/**
3732
* Schema Version for the Web client:
3833
* 1. Initial version including Mutation Queue, Query Cache, and Remote
@@ -58,7 +53,7 @@ export const INDEXING_SCHEMA_VERSION = 15;
5853
* 15. Add indexing support.
5954
*/
6055

61-
export const SCHEMA_VERSION = INDEXING_ENABLED ? INDEXING_SCHEMA_VERSION : 14;
56+
export const SCHEMA_VERSION = 15;
6257

6358
/**
6459
* Wrapper class to store timestamps (seconds and nanos) in IndexedDb objects.

‎packages/firestore/src/local/indexeddb_schema_converter.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ import {
4545
DbTarget,
4646
DbTargetDocument,
4747
DbTargetGlobal,
48-
INDEXING_SCHEMA_VERSION
48+
SCHEMA_VERSION
4949
} from './indexeddb_schema';
5050
import {
5151
DbRemoteDocument as DbRemoteDocumentLegacy,
@@ -146,7 +146,7 @@ export class SchemaConverter implements SimpleDbSchemaConverter {
146146
debugAssert(
147147
fromVersion < toVersion &&
148148
fromVersion >= 0 &&
149-
toVersion <= INDEXING_SCHEMA_VERSION,
149+
toVersion <= SCHEMA_VERSION,
150150
`Unexpected schema upgrade from v${fromVersion} to v${toVersion}.`
151151
);
152152

‎packages/firestore/src/local/local_store_impl.ts‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,8 @@ import {
3939
import { Document } from '../model/document';
4040
import { DocumentKey } from '../model/document_key';
4141
import {
42+
FieldIndex,
43+
fieldIndexSemanticComparator,
4244
INITIAL_LARGEST_BATCH_ID,
4345
newIndexOffsetSuccessorFromReadTime
4446
} from '../model/field_index';
@@ -57,6 +59,7 @@ import {
5759
} from '../protos/firestore_bundle_proto';
5860
import { RemoteEvent, TargetChange } from '../remote/remote_event';
5961
import { fromVersion, JsonProtoSerializer } from '../remote/serializer';
62+
import { diffArrays } from '../util/array';
6063
import { debugAssert, debugCast, hardAssert } from '../util/assert';
6164
import { ByteString } from '../util/byte_string';
6265
import { logDebug } from '../util/log';
@@ -1483,3 +1486,37 @@ export async function localStoreSaveNamedQuery(
14831486
}
14841487
);
14851488
}
1489+
1490+
export async function localStoreConfigureFieldIndexes(
1491+
localStore: LocalStore,
1492+
newFieldIndexes: FieldIndex[]
1493+
): Promise<void> {
1494+
const localStoreImpl = debugCast(localStore, LocalStoreImpl);
1495+
const indexManager = localStoreImpl.indexManager;
1496+
const promises: Array<PersistencePromise<void>> = [];
1497+
return localStoreImpl.persistence.runTransaction(
1498+
'Configure indexes',
1499+
'readwrite',
1500+
transaction =>
1501+
indexManager
1502+
.getFieldIndexes(transaction)
1503+
.next(oldFieldIndexes =>
1504+
diffArrays(
1505+
oldFieldIndexes,
1506+
newFieldIndexes,
1507+
fieldIndexSemanticComparator,
1508+
fieldIndex => {
1509+
promises.push(
1510+
indexManager.addFieldIndex(transaction, fieldIndex)
1511+
);
1512+
},
1513+
fieldIndex => {
1514+
promises.push(
1515+
indexManager.deleteFieldIndex(transaction, fieldIndex)
1516+
);
1517+
}
1518+
)
1519+
)
1520+
.next(() => PersistencePromise.waitFor(promises))
1521+
);
1522+
}

‎packages/firestore/src/local/query_engine.ts‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,6 @@ import { Iterable } from '../util/misc';
4343
import { SortedSet } from '../util/sorted_set';
4444

4545
import { IndexManager, IndexType } from './index_manager';
46-
import { INDEXING_ENABLED } from './indexeddb_schema';
4746
import { LocalDocumentsView } from './local_documents_view';
4847
import { PersistencePromise } from './persistence_promise';
4948
import { PersistenceTransaction } from './persistence_transaction';
@@ -134,10 +133,6 @@ export class QueryEngine {
134133
transaction: PersistenceTransaction,
135134
query: Query
136135
): PersistencePromise<DocumentMap | null> {
137-
if (!INDEXING_ENABLED) {
138-
return PersistencePromise.resolve<DocumentMap | null>(null);
139-
}
140-
141136
if (queryMatchesAllDocuments(query)) {
142137
// Queries that match all documents don't benefit from using
143138
// key-based lookups. It is more efficient to scan all documents in a

0 commit comments

Comments
 (0)