diff --git a/.changeset/yellow-turkeys-lay.md b/.changeset/yellow-turkeys-lay.md new file mode 100644 index 00000000000..553104f2e9b --- /dev/null +++ b/.changeset/yellow-turkeys-lay.md @@ -0,0 +1,11 @@ +--- +"@firebase/firestore-types": minor +"@firebase/firestore": minor +"@firebase/webchannel-wrapper": minor +"firebase": minor +--- + +Adds a new `experimentalAutoDetectLongPolling` to FirestoreSettings. When +enabled, the SDK's underlying transport (WebChannel) automatically detects if +long-polling should be used. This is very similar to +`experimentalForceLongPolling`, but only uses long-polling if required. diff --git a/packages/firebase/index.d.ts b/packages/firebase/index.d.ts index 7732e440c83..10edddfb704 100644 --- a/packages/firebase/index.d.ts +++ b/packages/firebase/index.d.ts @@ -7879,15 +7879,27 @@ declare namespace firebase.firestore { * buffer traffic indefinitely. Use of this option will cause some * performance degradation though. * - * This setting may be removed in a future release. If you find yourself - * using it to work around a specific network reliability issue, please - * tell us about it in - * https://github.com/firebase/firebase-js-sdk/issues/1674. + * This setting cannot be used with `experimentalAutoDetectLongPolling` and + * may be removed in a future release. If you find yourself using it to + * work around a specific network reliability issue, please tell us about + * it in https://github.com/firebase/firebase-js-sdk/issues/1674. * * @webonly */ experimentalForceLongPolling?: boolean; + /** + * Configures the SDK's underlying transport (WebChannel) to automatically detect if + * long-polling should be used. This is very similar to `experimentalForceLongPolling`, + * but only uses long-polling if required. + * + * This setting will likely be enabled by default in future releases and cannot be + * combined with `experimentalForceLongPolling`. + * + * @webonly + */ + experimentalAutoDetectLongPolling?: boolean; + /** * Whether to skip nested properties that are set to `undefined` during * object serialization. If set to `true`, these properties are skipped diff --git a/packages/firestore-types/index.d.ts b/packages/firestore-types/index.d.ts index 75ae5d4ef62..0222425dea3 100644 --- a/packages/firestore-types/index.d.ts +++ b/packages/firestore-types/index.d.ts @@ -29,6 +29,7 @@ export interface Settings { timestampsInSnapshots?: boolean; cacheSizeBytes?: number; experimentalForceLongPolling?: boolean; + experimentalAutoDetectLongPolling?: boolean; ignoreUndefinedProperties?: boolean; merge?: boolean; } diff --git a/packages/firestore/exp/src/api/database.ts b/packages/firestore/exp/src/api/database.ts index e9da67c3dd2..d8e9dba0a0a 100644 --- a/packages/firestore/exp/src/api/database.ts +++ b/packages/firestore/exp/src/api/database.ts @@ -121,7 +121,8 @@ export class FirebaseFirestore this._persistenceKey, settings.host ?? DEFAULT_HOST, settings.ssl ?? DEFAULT_SSL, - /* forceLongPolling= */ false + /* forceLongPolling= */ false, + /* forceAutoDetectLongPolling= */ true ); return { asyncQueue: this._queue, diff --git a/packages/firestore/lite/src/api/components.ts b/packages/firestore/lite/src/api/components.ts index c52435d5e9d..224dd2e32fe 100644 --- a/packages/firestore/lite/src/api/components.ts +++ b/packages/firestore/lite/src/api/components.ts @@ -59,7 +59,8 @@ export function getDatastore(firestore: FirebaseFirestore): Datastore { firestore._persistenceKey, settings.host ?? DEFAULT_HOST, settings.ssl ?? DEFAULT_SSL, - /* forceLongPolling= */ false + /* forceLongPolling= */ false, + /* forceAutoDetectLongPolling= */ true ); const connection = newConnection(databaseInfo); diff --git a/packages/firestore/src/api/database.ts b/packages/firestore/src/api/database.ts index 8aaf3d27d0d..c9912e81a9f 100644 --- a/packages/firestore/src/api/database.ts +++ b/packages/firestore/src/api/database.ts @@ -106,7 +106,8 @@ import { validateOptionNames, validatePositiveNumber, validateStringEnum, - valueDescription + valueDescription, + validateIsNotUsedTogether } from '../util/input_validation'; import { getLogLevel, logError, LogLevel, setLogLevel } from '../util/log'; import { AutoId } from '../util/misc'; @@ -146,6 +147,7 @@ const DEFAULT_HOST = 'firestore.googleapis.com'; const DEFAULT_SSL = true; const DEFAULT_TIMESTAMPS_IN_SNAPSHOTS = true; const DEFAULT_FORCE_LONG_POLLING = false; +const DEFAULT_FORCE_AUTO_DETECT_LONG_POLLING = false; const DEFAULT_IGNORE_UNDEFINED_PROPERTIES = false; /** @@ -191,6 +193,8 @@ class FirestoreSettings { readonly experimentalForceLongPolling: boolean; + readonly experimentalAutoDetectLongPolling: boolean; + readonly ignoreUndefinedProperties: boolean; // Can be a google-auth-library or gapi client. @@ -221,6 +225,7 @@ class FirestoreSettings { 'timestampsInSnapshots', 'cacheSizeBytes', 'experimentalForceLongPolling', + 'experimentalAutoDetectLongPolling', 'ignoreUndefinedProperties' ]); @@ -294,6 +299,23 @@ class FirestoreSettings { ); this.experimentalForceLongPolling = settings.experimentalForceLongPolling ?? DEFAULT_FORCE_LONG_POLLING; + + validateNamedOptionalType( + 'settings', + 'boolean', + 'experimentalAutoDetectLongPolling', + settings.experimentalAutoDetectLongPolling + ); + this.experimentalAutoDetectLongPolling = + settings.experimentalAutoDetectLongPolling ?? + DEFAULT_FORCE_AUTO_DETECT_LONG_POLLING; + + validateIsNotUsedTogether( + 'experimentalForceLongPolling', + settings.experimentalForceLongPolling, + 'experimentalAutoDetectLongPolling', + settings.experimentalAutoDetectLongPolling + ); } isEqual(other: FirestoreSettings): boolean { @@ -305,6 +327,8 @@ class FirestoreSettings { this.cacheSizeBytes === other.cacheSizeBytes && this.experimentalForceLongPolling === other.experimentalForceLongPolling && + this.experimentalAutoDetectLongPolling === + other.experimentalAutoDetectLongPolling && this.ignoreUndefinedProperties === other.ignoreUndefinedProperties ); } @@ -552,7 +576,8 @@ export class Firestore implements PublicFirestore, FirebaseService { this._persistenceKey, this._settings.host, this._settings.ssl, - this._settings.experimentalForceLongPolling + this._settings.experimentalForceLongPolling, + this._settings.experimentalAutoDetectLongPolling ); } diff --git a/packages/firestore/src/core/database_info.ts b/packages/firestore/src/core/database_info.ts index a46a6d53e62..5d97ca1a97b 100644 --- a/packages/firestore/src/core/database_info.ts +++ b/packages/firestore/src/core/database_info.ts @@ -29,13 +29,16 @@ export class DatabaseInfo { * @param ssl Whether to use SSL when connecting. * @param forceLongPolling Whether to use the forceLongPolling option * when using WebChannel as the network transport. + * @param autoDetectLongPolling Whether to use the detectBufferingProxy + * option when using WebChannel as the network transport. */ constructor( readonly databaseId: DatabaseId, readonly persistenceKey: string, readonly host: string, readonly ssl: boolean, - readonly forceLongPolling: boolean + readonly forceLongPolling: boolean, + readonly autoDetectLongPolling: boolean ) {} } diff --git a/packages/firestore/src/platform/browser/webchannel_connection.ts b/packages/firestore/src/platform/browser/webchannel_connection.ts index 0c0bb901863..ec7b5259e86 100644 --- a/packages/firestore/src/platform/browser/webchannel_connection.ts +++ b/packages/firestore/src/platform/browser/webchannel_connection.ts @@ -57,10 +57,12 @@ const XHR_TIMEOUT_SECS = 15; export class WebChannelConnection extends RestConnection { private readonly forceLongPolling: boolean; + private readonly autoDetectLongPolling: boolean; constructor(info: DatabaseInfo) { super(info); this.forceLongPolling = info.forceLongPolling; + this.autoDetectLongPolling = info.autoDetectLongPolling; } protected performRPCRequest( @@ -183,7 +185,8 @@ export class WebChannelConnection extends RestConnection { // timeouts to kick in if the request isn't working. forwardChannelRequestTimeoutMs: 10 * 60 * 1000 }, - forceLongPolling: this.forceLongPolling + forceLongPolling: this.forceLongPolling, + detectBufferingProxy: this.autoDetectLongPolling }; this.modifyHeadersForRequest(request.initMessageHeaders!, token); diff --git a/packages/firestore/src/util/input_validation.ts b/packages/firestore/src/util/input_validation.ts index 72ca4e737a1..4f97a13f8e7 100644 --- a/packages/firestore/src/util/input_validation.ts +++ b/packages/firestore/src/util/input_validation.ts @@ -199,6 +199,23 @@ export function validateNamedOptionalType( } } +/** + * Validates that two boolean options are not set at the same time. + */ +export function validateIsNotUsedTogether( + optionName1: string, + argument1: boolean | undefined, + optionName2: string, + argument2: boolean | undefined +): void { + if (argument1 === true && argument2 === true) { + throw new FirestoreError( + Code.INVALID_ARGUMENT, + `${optionName1} and ${optionName2} cannot be used together.` + ); + } +} + export function validateArrayElements( functionName: string, optionName: string, diff --git a/packages/firestore/test/integration/api/database.test.ts b/packages/firestore/test/integration/api/database.test.ts index abf591b6b47..4f6a14b27c6 100644 --- a/packages/firestore/test/integration/api/database.test.ts +++ b/packages/firestore/test/integration/api/database.test.ts @@ -26,12 +26,13 @@ import * as firebaseExport from '../util/firebase_export'; import { apiDescribe, withTestCollection, + withTestDbsSettings, withTestDb, withTestDbs, withTestDoc, withTestDocAndInitialData } from '../util/helpers'; -import { DEFAULT_SETTINGS } from '../util/settings'; +import { DEFAULT_SETTINGS, DEFAULT_PROJECT_ID } from '../util/settings'; use(chaiAsPromised); @@ -1600,4 +1601,30 @@ apiDescribe('Database', (persistence: boolean) => { }); }); }); + + it('can set and get data with auto detect long polling enabled', () => { + const settings = { + ...DEFAULT_SETTINGS, + experimentalAutoDetectLongPolling: true + }; + + return withTestDbsSettings( + persistence, + DEFAULT_PROJECT_ID, + settings, + 1, + async ([db]) => { + const data = { name: 'Rafi', email: 'abc@xyz.com' }; + const doc = await db.collection('users').doc(); + + return doc + .set(data) + .then(() => doc.get()) + .then(snapshot => { + expect(snapshot.exists).to.be.ok; + expect(snapshot.data()).to.deep.equal(data); + }); + } + ); + }); }); diff --git a/packages/firestore/test/integration/util/internal_helpers.ts b/packages/firestore/test/integration/util/internal_helpers.ts index d87b85bfef1..b6f01e9df81 100644 --- a/packages/firestore/test/integration/util/internal_helpers.ts +++ b/packages/firestore/test/integration/util/internal_helpers.ts @@ -44,7 +44,8 @@ export function getDefaultDatabaseInfo(): DatabaseInfo { 'persistenceKey', DEFAULT_SETTINGS.host!, !!DEFAULT_SETTINGS.ssl, - !!DEFAULT_SETTINGS.experimentalForceLongPolling + !!DEFAULT_SETTINGS.experimentalForceLongPolling, + !!DEFAULT_SETTINGS.experimentalAutoDetectLongPolling ); } diff --git a/packages/firestore/test/unit/api/database.test.ts b/packages/firestore/test/unit/api/database.test.ts index 07dca5690e4..9d55b1b56cf 100644 --- a/packages/firestore/test/unit/api/database.test.ts +++ b/packages/firestore/test/unit/api/database.test.ts @@ -170,6 +170,19 @@ describe('Settings', () => { ); }); + it('can not use mutually exclusive settings together', () => { + // Use a new instance of Firestore in order to configure settings. + const firestoreClient = newTestFirestore(); + expect( + firestoreClient.settings.bind(firestoreClient.settings, { + experimentalForceLongPolling: true, + experimentalAutoDetectLongPolling: true + }) + ).to.throw( + `experimentalForceLongPolling and experimentalAutoDetectLongPolling cannot be used together.` + ); + }); + it('can merge settings', () => { // Use a new instance of Firestore in order to configure settings. const firestoreClient = newTestFirestore(); diff --git a/packages/firestore/test/unit/remote/rest_connection.test.ts b/packages/firestore/test/unit/remote/rest_connection.test.ts index a0153a1b0a5..a534b00fc5b 100644 --- a/packages/firestore/test/unit/remote/rest_connection.test.ts +++ b/packages/firestore/test/unit/remote/rest_connection.test.ts @@ -60,7 +60,8 @@ describe('RestConnection', () => { 'persistenceKey', 'example.com', /*ssl=*/ false, - /*forceLongPolling=*/ false + /*forceLongPolling=*/ false, + /*autoDetectLongPolling=*/ false ); const connection = new TestRestConnection(testDatabaseInfo); diff --git a/packages/firestore/test/unit/specs/spec_test_runner.ts b/packages/firestore/test/unit/specs/spec_test_runner.ts index c726f329f44..d753010b13f 100644 --- a/packages/firestore/test/unit/specs/spec_test_runner.ts +++ b/packages/firestore/test/unit/specs/spec_test_runner.ts @@ -241,7 +241,8 @@ abstract class TestRunner { TEST_PERSISTENCE_KEY, 'host', /*ssl=*/ false, - /*forceLongPolling=*/ false + /*forceLongPolling=*/ false, + /*autoDetectLongPolling=*/ false ); // TODO(mrschmidt): During client startup in `firestore_client`, we block diff --git a/packages/webchannel-wrapper/src/index.d.ts b/packages/webchannel-wrapper/src/index.d.ts index 953ba859ddb..4717ee6b02e 100644 --- a/packages/webchannel-wrapper/src/index.d.ts +++ b/packages/webchannel-wrapper/src/index.d.ts @@ -89,6 +89,7 @@ export interface WebChannelOptions { httpSessionIdParam?: string; httpHeadersOverwriteParam?: string; forceLongPolling?: boolean; + detectBufferingProxy?: boolean; fastHandshake?: boolean; disableRedac?: boolean; clientProfile?: string;