Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(firestore): accept nested undefined in arrays if ignoreUndefined set #5527

Merged
merged 2 commits into from
Jul 21, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions jest.setup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ jest.doMock('react-native', () => {
},
RNFBFirestoreModule: {
settings: jest.fn(),
documentSet: jest.fn(),
},
RNFBPerfModule: {},
RNFBStorageModule: {
Expand Down
34 changes: 33 additions & 1 deletion packages/firestore/__tests__/firestore.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,7 @@ describe('Storage', function () {
}
});

it('throws when nested undefined value provided and ignored undefined is false', async function () {
it('throws when nested undefined object value provided and ignored undefined is false', async function () {
await firebase.firestore().settings({ ignoreUndefinedProperties: false });
const docRef = firebase.firestore().doc(`${COLLECTION}/bar`);
try {
Expand All @@ -262,5 +262,37 @@ describe('Storage', function () {
return expect(e.message).toEqual('Unsupported field value: undefined');
}
});

it('throws when nested undefined array value provided and ignored undefined is false', async function () {
await firebase.firestore().settings({ ignoreUndefinedProperties: false });
const docRef = firebase.firestore().doc(`${COLLECTION}/bar`);
try {
await docRef.set({
myArray: [{ name: 'Tim', location: { state: undefined, country: 'United Kingdom' } }],
});
return Promise.reject(new Error('Expected set() to throw'));
} catch (e) {
return expect(e.message).toEqual('Unsupported field value: undefined');
}
});

it('does not throw when nested undefined array value provided and ignore undefined is true', async function () {
await firebase.firestore().settings({ ignoreUndefinedProperties: true });
const docRef = firebase.firestore().doc(`${COLLECTION}/bar`);
await docRef.set({
myArray: [{ name: 'Tim', location: { state: undefined, country: 'United Kingdom' } }],
});
});

it('does not throw when nested undefined object value provided and ignore undefined is true', async function () {
await firebase.firestore().settings({ ignoreUndefinedProperties: true });
const docRef = firebase.firestore().doc(`${COLLECTION}/bar`);
await docRef.set({
field1: 1,
field2: {
shouldNotWork: undefined,
},
});
});
});
});
82 changes: 82 additions & 0 deletions packages/firestore/e2e/DocumentReference/set.e2e.js
Original file line number Diff line number Diff line change
Expand Up @@ -156,4 +156,86 @@ describe('firestore.doc().set()', function () {
snapshot2.data().should.eql(jet.contextify(merged));
await ref.delete();
});

it('throws when nested undefined array value provided and ignored undefined is false', async function () {
await firebase.firestore().settings({ ignoreUndefinedProperties: false });
const docRef = firebase.firestore().doc(`${COLLECTION}/bar`);
try {
await docRef.set({
myArray: [{ name: 'Tim', location: { state: undefined, country: 'United Kingdom' } }],
});
return Promise.reject(new Error('Expected set() to throw'));
} catch (error) {
error.message.should.containEql('Unsupported field value: undefined');
}
});

it('accepts undefined nested array values if ignoreUndefined is true', async function () {
await firebase.firestore().settings({ ignoreUndefinedProperties: true });
const docRef = firebase.firestore().doc(`${COLLECTION}/bar`);
await docRef.set({
myArray: [{ name: 'Tim', location: { state: undefined, country: 'United Kingdom' } }],
});
});

it('does not throw when nested undefined object value provided and ignore undefined is true', async function () {
await firebase.firestore().settings({ ignoreUndefinedProperties: true });
const docRef = firebase.firestore().doc(`${COLLECTION}/bar`);
await docRef.set({
field1: 1,
field2: {
shouldNotWork: undefined,
},
});
});

it('filters out undefined properties when setting enabled', async function () {
await firebase.firestore().settings({ ignoreUndefinedProperties: true });

const docRef = firebase.firestore().doc(`${COLLECTION}/ignoreUndefinedTrueProp`);
await docRef.set({
field1: 1,
field2: undefined,
});

const snap = await docRef.get();
const snapData = snap.data();
if (!snapData) {
return Promise.reject(new Error('Snapshot not saved'));
}

snapData.field1.should.eql(1);
snapData.hasOwnProperty('field2').should.eql(false);
});

it('filters out nested undefined properties when setting enabled', async function () {
await firebase.firestore().settings({ ignoreUndefinedProperties: true });

const docRef = firebase.firestore().doc(`${COLLECTION}/ignoreUndefinedTrueNestedProp`);
await docRef.set({
field1: 1,
field2: {
shouldBeMissing: undefined,
},
field3: [
{
shouldBeHere: 'Here',
shouldBeMissing: undefined,
},
],
});

const snap = await docRef.get();
const snapData = snap.data();
if (!snapData) {
return Promise.reject(new Error('Snapshot not saved'));
}

snapData.field1.should.eql(1);
snapData.hasOwnProperty('field2').should.eql(true);
snapData.field2.hasOwnProperty('shouldBeMissing').should.eql(false);
snapData.hasOwnProperty('field3').should.eql(true);
snapData.field3[0].shouldBeHere.should.eql('Here');
snapData.field3[0].hasOwnProperty('shouldBeMissing').should.eql(false);
});
});
44 changes: 0 additions & 44 deletions packages/firestore/e2e/firestore.e2e.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,50 +37,6 @@ describe('firestore()', function () {

describe('collection()', function () {});

describe('doc()', function () {
it('filters out undefined properties when setting enabled', async function () {
await firebase.firestore().settings({ ignoreUndefinedProperties: true });

const docRef = firebase.firestore().doc(`${COLLECTION}/bar`);
await docRef.set({
field1: 1,
field2: undefined,
});

const snap = await docRef.get();
const snapData = snap.data();
if (!snapData) {
return Promise.reject(new Error('Snapshot not saved'));
}

snapData.field1.should.eql(1);
snapData.hasOwnProperty('field2').should.eql(false);
});

it('filters out nested undefined properties when setting enabled', async function () {
await firebase.firestore().settings({ ignoreUndefinedProperties: true });

const docRef = firebase.firestore().doc(`${COLLECTION}/bar`);
await docRef.set({
field1: 1,
field2: {
shouldBeMissing: undefined,
},
});

const snap = await docRef.get();
const snapData = snap.data();
if (!snapData) {
return Promise.reject(new Error('Snapshot not saved'));
}

snapData.field1.should.eql(1);
snapData.hasOwnProperty('field2').should.eql(true);

snapData.field2.hasOwnProperty('shouldBeMissing').should.eql(false);
});
});

describe('collectionGroup()', function () {
it('performs a collection group query', async function () {
const docRef1 = firebase.firestore().doc(`${COLLECTION}/collectionGroup1`);
Expand Down
25 changes: 17 additions & 8 deletions packages/firestore/lib/utils/serialize.js
Original file line number Diff line number Diff line change
Expand Up @@ -59,12 +59,14 @@ export function buildNativeMap(data, ignoreUndefined) {
if (typeof data[key] === 'undefined') {
if (!ignoreUndefined) {
throw new Error('Unsupported field value: undefined');
} else {
continue;
}
} else {
const typeMap = generateNativeData(data[key], ignoreUndefined);
if (typeMap) {
nativeData[key] = typeMap;
}
}

const typeMap = generateNativeData(data[key], ignoreUndefined);
if (typeMap) {
nativeData[key] = typeMap;
}
}
}
Expand All @@ -76,12 +78,19 @@ export function buildNativeMap(data, ignoreUndefined) {
* @param array
* @returns {Array}
*/
export function buildNativeArray(array) {
export function buildNativeArray(array, ignoreUndefined) {
const nativeArray = [];
if (array) {
for (let i = 0; i < array.length; i++) {
const value = array[i];
const typeMap = generateNativeData(value);
if (typeof value === 'undefined') {
if (!ignoreUndefined) {
throw new Error('Unsupported field value: undefined');
} else {
continue;
}
}
const typeMap = generateNativeData(value, ignoreUndefined);
if (typeMap) {
nativeArray.push(typeMap);
}
Expand Down Expand Up @@ -144,7 +153,7 @@ export function generateNativeData(value, ignoreUndefined) {
}

if (isArray(value)) {
return getTypeMapInt('array', buildNativeArray(value));
return getTypeMapInt('array', buildNativeArray(value, ignoreUndefined));
}

if (isObject(value)) {
Expand Down
56 changes: 28 additions & 28 deletions tests/ios/Podfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -446,58 +446,58 @@ PODS:
- React-cxxreact (= 0.64.1)
- React-jsi (= 0.64.1)
- React-perflogger (= 0.64.1)
- RNFBAnalytics (12.1.0):
- RNFBAnalytics (12.2.0):
- Firebase/Analytics (= 8.3.0)
- React-Core
- RNFBApp
- RNFBApp (12.1.0):
- RNFBApp (12.2.0):
- Firebase/CoreOnly (= 8.3.0)
- React-Core
- RNFBAuth (12.1.0):
- RNFBAuth (12.2.0):
- Firebase/Auth (= 8.3.0)
- React-Core
- RNFBApp
- RNFBCrashlytics (12.1.0):
- RNFBCrashlytics (12.2.0):
- Firebase/Crashlytics (= 8.3.0)
- React-Core
- RNFBApp
- RNFBDatabase (12.1.0):
- RNFBDatabase (12.2.0):
- Firebase/Database (= 8.3.0)
- React-Core
- RNFBApp
- RNFBDynamicLinks (12.1.0):
- RNFBDynamicLinks (12.2.0):
- Firebase/DynamicLinks (= 8.3.0)
- GoogleUtilities/AppDelegateSwizzler
- React-Core
- RNFBApp
- RNFBFirestore (12.1.0):
- RNFBFirestore (12.2.0):
- Firebase/Firestore (= 8.3.0)
- React-Core
- RNFBApp
- RNFBFunctions (12.1.0):
- RNFBFunctions (12.2.0):
- Firebase/Functions (= 8.3.0)
- React-Core
- RNFBApp
- RNFBInAppMessaging (12.1.0):
- RNFBInAppMessaging (12.2.0):
- Firebase/InAppMessaging (= 8.3.0)
- React-Core
- RNFBApp
- RNFBMessaging (12.1.0):
- RNFBMessaging (12.2.0):
- Firebase/Messaging (= 8.3.0)
- React-Core
- RNFBApp
- RNFBML (12.1.0):
- RNFBML (12.2.0):
- React-Core
- RNFBApp
- RNFBPerf (12.1.0):
- RNFBPerf (12.2.0):
- Firebase/Performance (= 8.3.0)
- React-Core
- RNFBApp
- RNFBRemoteConfig (12.1.0):
- RNFBRemoteConfig (12.2.0):
- Firebase/RemoteConfig (= 8.3.0)
- React-Core
- RNFBApp
- RNFBStorage (12.1.0):
- RNFBStorage (12.2.0):
- Firebase/Storage (= 8.3.0)
- React-Core
- RNFBApp
Expand Down Expand Up @@ -730,20 +730,20 @@ SPEC CHECKSUMS:
React-RCTVibration: 4b99a7f5c6c0abbc5256410cc5425fb8531986e1
React-runtimeexecutor: ff951a0c241bfaefc4940a3f1f1a229e7cb32fa6
ReactCommon: bedc99ed4dae329c4fcf128d0c31b9115e5365ca
RNFBAnalytics: 3f747b44011985cf5b73649c4cb15fc41ba6572a
RNFBApp: c3f39822900c67312b7d7adc68182508b16e297b
RNFBAuth: 64c13d5f2e360bb0e39e8751f002cc45b53e1d63
RNFBCrashlytics: c971aeb1257d4145b411c20bfff4d5651f9c40a7
RNFBDatabase: accb91e56e7c6655b1b28d9fea54db44a17f6f63
RNFBDynamicLinks: d6086edca873f07b0746fcfba13f8c1c05b36965
RNFBFirestore: ba5286637fc7b1840b2f5da0c7c88ae792c82343
RNFBFunctions: be0a309b0bab0ce021c9540af41a47f596a288fa
RNFBInAppMessaging: 38fbe741187256e36909c283ada6f907867dc114
RNFBMessaging: f8f495fa42655a428a1c140b7100165984215d62
RNFBML: 41c38d1d3200fb6686d9289e78b859dd2c9572ad
RNFBPerf: bd0bef4cea3f423e8d0f7270c6d80048c217f7e9
RNFBRemoteConfig: 8b073e563357a63e993d04a4c48a12e199ab180c
RNFBStorage: 8efe8ac42dc8bc1771fa803fff6906fb999b0bb3
RNFBAnalytics: a17c2e6e38053358fc186e6a20ed8e476ade8803
RNFBApp: f83ab90f319f1a4b7f276e4703c6a0cdfce5abde
RNFBAuth: 385db0185418f06c99507596cfd7a5ec92de2b89
RNFBCrashlytics: f55af02cf1d25ed3c520c8aa4b6500e81679586c
RNFBDatabase: 48e92c8aac8e835bde7119b2f91dcc4134f349ba
RNFBDynamicLinks: 1497c962bfbe7cd70e46e4e5f58c1739d64e7827
RNFBFirestore: 4bc309730dd0d323e08901c8e74431f943ba0b01
RNFBFunctions: 9cb9cc5d7678507844724e41028d5d9757498512
RNFBInAppMessaging: 62873b9ad67240565032c510dc36c315d2cb26db
RNFBMessaging: 463bd1c052aae9bb78a07670645b82357b88952a
RNFBML: 1d0dab4906514e81b7ecd4d96f486a2e6e1e504a
RNFBPerf: e7859e86105c759de84132566a529e3eda1c5a00
RNFBRemoteConfig: 2af45b2b4e43533132e952827ed32a91a3956152
RNFBStorage: 8c14190052be92c1fc772ef8c36aac4d3d4d4c7f
Yoga: a7de31c64fe738607e7a3803e3f591a4b1df7393

PODFILE CHECKSUM: f25a92eedd54ba32f69cb8b7ec0cbc4ceb364ddd
Expand Down
4 changes: 2 additions & 2 deletions tests/ios/testing.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -437,7 +437,7 @@
ENABLE_BITCODE = NO;
ENABLE_STRICT_OBJC_MSGSEND = YES;
ENABLE_TESTABILITY = YES;
"EXCLUDED_ARCHS[sdk=iphonesimulator*]" = "";
"EXCLUDED_ARCHS[sdk=iphonesimulator*]" = "arm64 ";
GCC_C_LANGUAGE_STANDARD = gnu99;
GCC_DYNAMIC_NO_PIC = NO;
GCC_NO_COMMON_BLOCKS = YES;
Expand Down Expand Up @@ -486,7 +486,7 @@
ENABLE_BITCODE = NO;
ENABLE_NS_ASSERTIONS = NO;
ENABLE_STRICT_OBJC_MSGSEND = YES;
"EXCLUDED_ARCHS[sdk=iphonesimulator*]" = "";
"EXCLUDED_ARCHS[sdk=iphonesimulator*]" = "arm64 ";
GCC_C_LANGUAGE_STANDARD = gnu99;
GCC_NO_COMMON_BLOCKS = YES;
GCC_WARN_64_TO_32_BIT_CONVERSION = YES;
Expand Down