From 7d0b18694f0bb323ca907013fc468fd6f74beabc Mon Sep 17 00:00:00 2001 From: Wu-Hui Date: Thu, 9 Feb 2023 14:02:42 -0500 Subject: [PATCH] Fix some test failures --- packages/firestore/src/api.ts | 1 + packages/firestore/src/api/cache_config.ts | 8 ++++---- packages/firestore/src/api/database.ts | 11 +++++++++-- .../firestore/src/api/index_configuration.ts | 17 ++++++++--------- packages/firestore/src/core/firestore_client.ts | 15 +++++++++++++++ packages/firestore/src/lite-api/settings.ts | 1 - .../test/integration/api/database.test.ts | 1 + .../test/integration/api/validation.test.ts | 4 +--- .../api_internal/idle_timeout.test.ts | 2 +- .../firestore/test/integration/util/helpers.ts | 12 ++++-------- 10 files changed, 44 insertions(+), 28 deletions(-) diff --git a/packages/firestore/src/api.ts b/packages/firestore/src/api.ts index 33760205e0e..e014f5b4d08 100644 --- a/packages/firestore/src/api.ts +++ b/packages/firestore/src/api.ts @@ -35,6 +35,7 @@ export { IndexedDbMultipleTabManager, indexedDbLocalCache, indexedDbMultipleTabManager, + IndexedDbSettings, indexedDbSingleTabManager, IndexedDbSingleTabManager, memoryLocalCache, diff --git a/packages/firestore/src/api/cache_config.ts b/packages/firestore/src/api/cache_config.ts index 1fa2c0f9589..817f134acc1 100644 --- a/packages/firestore/src/api/cache_config.ts +++ b/packages/firestore/src/api/cache_config.ts @@ -39,7 +39,7 @@ class MemoryLocalCacheImpl implements MemoryLocalCache { this._offlineComponentProvider = new MemoryOfflineComponentProvider(); } - toJSON() { + toJSON(): {} { return { kind: this.kind }; } } @@ -68,7 +68,7 @@ class IndexedDbLocalCacheImpl implements IndexedDbLocalCache { this._offlineComponentProvider = tabManager._offlineComponentProvider!; } - toJSON() { + toJSON(): {} { return { kind: this.kind }; } } @@ -110,7 +110,7 @@ class SingleTabManagerImpl implements IndexedDbSingleTabManager { constructor(private forceOwnership?: boolean) {} - toJSON() { + toJSON(): {} { return { kind: this.kind }; } @@ -139,7 +139,7 @@ class MultiTabManagerImpl implements IndexedDbMultipleTabManager { _onlineComponentProvider?: OnlineComponentProvider; _offlineComponentProvider?: OfflineComponentProvider; - toJSON() { + toJSON(): {} { return { kind: this.kind }; } diff --git a/packages/firestore/src/api/database.ts b/packages/firestore/src/api/database.ts index cb2f2daf373..20a03abead2 100644 --- a/packages/firestore/src/api/database.ts +++ b/packages/firestore/src/api/database.ts @@ -288,6 +288,7 @@ export function configureFirestore(firestore: Firestore): void { settings.cache?._onlineComponentProvider ) { firestore._firestoreClient.uninitializedComponentsProvider = { + offlineKind: settings.cache.kind, offline: settings.cache._offlineComponentProvider, online: settings.cache._onlineComponentProvider }; @@ -327,7 +328,10 @@ export function enableIndexedDbPersistence( const client = ensureFirestoreConfigured(firestore); if (client.uninitializedComponentsProvider) { - throw new FirestoreError(Code.INVALID_ARGUMENT, 'Already specified.'); + throw new FirestoreError( + Code.FAILED_PRECONDITION, + 'SDK cache is already specified.' + ); } const settings = firestore._freezeSettings(); @@ -376,7 +380,10 @@ export function enableMultiTabIndexedDbPersistence( const client = ensureFirestoreConfigured(firestore); if (client.uninitializedComponentsProvider) { - throw new FirestoreError(Code.INVALID_ARGUMENT, 'Already specified.'); + throw new FirestoreError( + Code.FAILED_PRECONDITION, + 'SDK cache is already specified.' + ); } const settings = firestore._freezeSettings(); diff --git a/packages/firestore/src/api/index_configuration.ts b/packages/firestore/src/api/index_configuration.ts index 9ea31a28db1..c11a9665f46 100644 --- a/packages/firestore/src/api/index_configuration.ts +++ b/packages/firestore/src/api/index_configuration.ts @@ -15,9 +15,8 @@ * limitations under the License. */ -import { getLocalStore } from '../core/firestore_client'; +import { firestoreClientSetIndexConfiguration } from '../core/firestore_client'; import { fieldPathFromDotSeparatedString } from '../lite-api/user_data_reader'; -import { localStoreConfigureFieldIndexes } from '../local/local_store_impl'; import { FieldIndex, IndexKind, @@ -151,17 +150,17 @@ export function setIndexConfiguration( ): Promise { firestore = cast(firestore, Firestore); const client = ensureFirestoreConfigured(firestore); - - // PORTING NOTE: We don't return an error if the user has not enabled - // persistence since `enableIndexeddbPersistence()` can fail on the Web. - if (!client.offlineComponents?.indexBackfillerScheduler) { + if ( + !client.uninitializedComponentsProvider || + client.uninitializedComponentsProvider?.offlineKind === 'memory' + ) { + // PORTING NOTE: We don't return an error if the user has not enabled + // persistence since `enableIndexeddbPersistence()` can fail on the Web. logWarn('Cannot enable indexes when persistence is disabled'); return Promise.resolve(); } const parsedIndexes = parseIndexes(jsonOrConfiguration); - return getLocalStore(client).then(localStore => - localStoreConfigureFieldIndexes(localStore, parsedIndexes) - ); + return firestoreClientSetIndexConfiguration(client, parsedIndexes); } export function parseIndexes( diff --git a/packages/firestore/src/core/firestore_client.ts b/packages/firestore/src/core/firestore_client.ts index b17fc8b47e1..051337bdb80 100644 --- a/packages/firestore/src/core/firestore_client.ts +++ b/packages/firestore/src/core/firestore_client.ts @@ -31,6 +31,7 @@ import { User } from '../auth/user'; import { Query as LiteQuery } from '../lite-api/reference'; import { LocalStore } from '../local/local_store'; import { + localStoreConfigureFieldIndexes, localStoreExecuteQuery, localStoreGetNamedQuery, localStoreHandleUserChange, @@ -39,6 +40,7 @@ import { import { Persistence } from '../local/persistence'; import { Document } from '../model/document'; import { DocumentKey } from '../model/document_key'; +import { FieldIndex } from '../model/field_index'; import { Mutation } from '../model/mutation'; import { toByteStreamReader } from '../platform/byte_stream_reader'; import { newSerializer, newTextEncoder } from '../platform/serializer'; @@ -114,6 +116,7 @@ export class FirestoreClient { ) => Promise = () => Promise.resolve(); uninitializedComponentsProvider?: { offline: OfflineComponentProvider; + offlineKind: 'memory' | 'indexeddb'; online: OnlineComponentProvider; }; @@ -768,3 +771,15 @@ function createBundleReader( } return newBundleReader(toByteStreamReader(content), serializer); } + +export function firestoreClientSetIndexConfiguration( + client: FirestoreClient, + indexes: FieldIndex[] +): Promise { + return client.asyncQueue.enqueue(async () => { + return localStoreConfigureFieldIndexes( + await getLocalStore(client), + indexes + ); + }); +} diff --git a/packages/firestore/src/lite-api/settings.ts b/packages/firestore/src/lite-api/settings.ts index dc4af779b38..9de2d0eb705 100644 --- a/packages/firestore/src/lite-api/settings.ts +++ b/packages/firestore/src/lite-api/settings.ts @@ -24,7 +24,6 @@ import { import { LRU_MINIMUM_CACHE_SIZE_BYTES } from '../local/lru_garbage_collector_impl'; import { Code, FirestoreError } from '../util/error'; import { validateIsNotUsedTogether } from '../util/input_validation'; -import { logWarn } from '../util/log'; // settings() defaults: export const DEFAULT_HOST = 'firestore.googleapis.com'; diff --git a/packages/firestore/test/integration/api/database.test.ts b/packages/firestore/test/integration/api/database.test.ts index 5f3e2dc6c61..174048020e6 100644 --- a/packages/firestore/test/integration/api/database.test.ts +++ b/packages/firestore/test/integration/api/database.test.ts @@ -1214,6 +1214,7 @@ apiDescribe('Database', (persistence: boolean) => { 'cannot clear persistence if the client has been initialized', async () => { await withTestDoc(persistence, async (docRef, firestore) => { + await setDoc(docRef, {}); const expectedError = 'Persistence can only be cleared before a Firestore instance is ' + 'initialized or after it is terminated.'; diff --git a/packages/firestore/test/integration/api/validation.test.ts b/packages/firestore/test/integration/api/validation.test.ts index c118e485310..229a9a582b7 100644 --- a/packages/firestore/test/integration/api/validation.test.ts +++ b/packages/firestore/test/integration/api/validation.test.ts @@ -240,9 +240,7 @@ apiDescribe('Validation:', (persistence: boolean) => { doc(db, 'foo/bar'); } expect(() => enableIndexedDbPersistence(db)).to.throw( - 'Firestore has already been started and persistence can no ' + - 'longer be enabled. You can only enable persistence before ' + - 'calling any other methods on a Firestore object.' + 'SDK cache is already specified.' ); } ); diff --git a/packages/firestore/test/integration/api_internal/idle_timeout.test.ts b/packages/firestore/test/integration/api_internal/idle_timeout.test.ts index f6d5f77f7e5..d0a31ee9461 100644 --- a/packages/firestore/test/integration/api_internal/idle_timeout.test.ts +++ b/packages/firestore/test/integration/api_internal/idle_timeout.test.ts @@ -22,7 +22,7 @@ import { apiDescribe, withTestDb } from '../util/helpers'; import { asyncQueue } from '../util/internal_helpers'; apiDescribe('Idle Timeout', (persistence: boolean) => { - it.only('can write document after idle timeout', () => { + it('can write document after idle timeout', () => { return withTestDb(persistence, db => { const docRef = doc(collection(db, 'test-collection')); return setDoc(docRef, { foo: 'bar' }) diff --git a/packages/firestore/test/integration/util/helpers.ts b/packages/firestore/test/integration/util/helpers.ts index f8ecc1bfb3a..a0ffff32a52 100644 --- a/packages/firestore/test/integration/util/helpers.ts +++ b/packages/firestore/test/integration/util/helpers.ts @@ -17,18 +17,13 @@ import { isIndexedDBAvailable } from '@firebase/util'; -import { - indexedDbLocalCache, - memoryLocalCache -} from '../../../src/api/cache_config'; -import { logWarn } from '../../../src/util/log'; - import { collection, doc, DocumentReference, Firestore, terminate, + indexedDbLocalCache, clearIndexedDbPersistence, enableIndexedDbPersistence, CollectionReference, @@ -191,10 +186,11 @@ export async function withTestDbsSettings( for (let i = 0; i < numDbs; i++) { // logWarn(`set persistence from helper: ${persistence}`); + const newSettings = { ...settings }; if (persistence) { - settings.cache = indexedDbLocalCache(); + newSettings.cache = indexedDbLocalCache(); } - const db = newTestFirestore(newTestApp(projectId), settings); + const db = newTestFirestore(newTestApp(projectId), newSettings); dbs.push(db); }