Skip to content

Commit

Permalink
add option to merge settings (#3464)
Browse files Browse the repository at this point in the history
  • Loading branch information
Brian Chen committed Aug 19, 2020
1 parent 8097b97 commit 61b4cd3
Show file tree
Hide file tree
Showing 7 changed files with 78 additions and 7 deletions.
8 changes: 8 additions & 0 deletions .changeset/mean-jokes-tan.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
---
'firebase': patch
'@firebase/firestore': patch
'@firebase/firestore-types': patch
---
feat: Added `merge` option to `firestore.settings()`, which merges the provided settings with
settings from a previous call. This allows adding settings on top of the settings that were applied
by `@firebase/testing`.
12 changes: 10 additions & 2 deletions packages/firebase/index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7891,10 +7891,18 @@ declare namespace firebase.firestore {
/**
* Whether to skip nested properties that are set to `undefined` during
* object serialization. If set to `true`, these properties are skipped
* and not written to Firestore. If set `false` or omitted, the SDK throws
* an exception when it encounters properties of type `undefined`.
* and not written to Firestore. If set to `false` or omitted, the SDK
* throws an exception when it encounters properties of type `undefined`.
*/
ignoreUndefinedProperties?: boolean;

/**
* Whether to merge the provided settings with the existing settings. If
* set to `true`, the settings are merged with existing settings. If
* set to `false` or left unset, the settings replace the existing
* settings.
*/
merge?: boolean;
}

/**
Expand Down
1 change: 1 addition & 0 deletions packages/firestore-types/index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ export interface Settings {
cacheSizeBytes?: number;
experimentalForceLongPolling?: boolean;
ignoreUndefinedProperties?: boolean;
merge?: boolean;
}

export interface PersistenceSettings {
Expand Down
20 changes: 16 additions & 4 deletions packages/firestore/src/api/database.ts
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ class FirestoreSettings {

readonly cacheSizeBytes: number;

readonly forceLongPolling: boolean;
readonly experimentalForceLongPolling: boolean;

readonly ignoreUndefinedProperties: boolean;

Expand Down Expand Up @@ -263,7 +263,7 @@ class FirestoreSettings {
'experimentalForceLongPolling',
settings.experimentalForceLongPolling
);
this.forceLongPolling =
this.experimentalForceLongPolling =
settings.experimentalForceLongPolling ?? DEFAULT_FORCE_LONG_POLLING;
}

Expand All @@ -274,7 +274,8 @@ class FirestoreSettings {
this.timestampsInSnapshots === other.timestampsInSnapshots &&
this.credentials === other.credentials &&
this.cacheSizeBytes === other.cacheSizeBytes &&
this.forceLongPolling === other.forceLongPolling &&
this.experimentalForceLongPolling ===
other.experimentalForceLongPolling &&
this.ignoreUndefinedProperties === other.ignoreUndefinedProperties
);
}
Expand Down Expand Up @@ -361,6 +362,12 @@ export class Firestore implements firestore.FirebaseFirestore, FirebaseService {
validateExactNumberOfArgs('Firestore.settings', arguments, 1);
validateArgType('Firestore.settings', 'object', 1, settingsLiteral);

if (settingsLiteral.merge) {
settingsLiteral = { ...this._settings, ...settingsLiteral };
// Remove the property from the settings once the merge is completed
delete settingsLiteral.merge;
}

const newSettings = new FirestoreSettings(settingsLiteral);
if (this._firestoreClient && !this._settings.isEqual(newSettings)) {
throw new FirestoreError(
Expand Down Expand Up @@ -516,7 +523,7 @@ export class Firestore implements firestore.FirebaseFirestore, FirebaseService {
this._persistenceKey,
this._settings.host,
this._settings.ssl,
this._settings.forceLongPolling
this._settings.experimentalForceLongPolling
);
}

Expand Down Expand Up @@ -681,6 +688,11 @@ export class Firestore implements firestore.FirebaseFirestore, FirebaseService {
_areTimestampsInSnapshotsEnabled(): boolean {
return this._settings.timestampsInSnapshots;
}

// Visible for testing.
_getSettings(): firestore.Settings {
return this._settings;
}
}

/**
Expand Down
2 changes: 1 addition & 1 deletion packages/firestore/test/integration/api/database.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1266,7 +1266,7 @@ apiDescribe('Database', (persistence: boolean) => {
});
});

it('calling terminate mutiple times should proceed', async () => {
it('calling terminate multiple times should proceed', async () => {
await withTestDoc(persistence, async docRef => {
const firestore = docRef.firestore;
await firestore.terminate();
Expand Down
30 changes: 30 additions & 0 deletions packages/firestore/test/unit/api/database.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,12 @@ import {
collectionReference,
documentReference,
documentSnapshot,
newTestFirestore,
query,
querySnapshot
} from '../../util/api_helpers';
import { expectEqual, expectNotEqual, keys } from '../../util/helpers';
import { expect } from 'chai';

describe('CollectionReference', () => {
it('support equality checking with isEqual()', () => {
Expand Down Expand Up @@ -153,3 +155,31 @@ describe('SnapshotMetadata', () => {
);
});
});

describe('Settings', () => {
it('replaces settings by default', () => {
// Use a new instance of Firestore in order to configure settings.
const firestoreClient = newTestFirestore();
firestoreClient.settings({ host: 'other.host' });
firestoreClient.settings({ ignoreUndefinedProperties: true });

expect(firestoreClient._getSettings().ignoreUndefinedProperties).to.be.true;
// Expect host to be replaced with default host.
expect(firestoreClient._getSettings().host).to.equal(
'firestore.googleapis.com'
);
});

it('can merge settings', () => {
// Use a new instance of Firestore in order to configure settings.
const firestoreClient = newTestFirestore();
firestoreClient.settings({ host: 'other.host' });
firestoreClient.settings({
ignoreUndefinedProperties: true,
merge: true
});

expect(firestoreClient._getSettings().ignoreUndefinedProperties).to.be.true;
expect(firestoreClient._getSettings().host).to.equal('other.host');
});
});
12 changes: 12 additions & 0 deletions packages/firestore/test/util/api_helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,18 @@ export function firestore(): Firestore {
return FIRESTORE;
}

export function newTestFirestore(): Firestore {
return new Firestore(
{
projectId: 'new-project',
database: '(default)'
},
new Provider('auth-internal', new ComponentContainer('default')),
offlineComponentProvider,
onlineComponentProvider
);
}

export function collectionReference(path: string): CollectionReference {
const firestoreClient = firestore();
// eslint-disable-next-line @typescript-eslint/no-floating-promises
Expand Down

0 comments on commit 61b4cd3

Please sign in to comment.