From a64495a33ba345e5580f0579aae49a23d0797ba6 Mon Sep 17 00:00:00 2001 From: Gil Date: Mon, 16 Oct 2017 15:56:19 -0700 Subject: Fix validation of nested arrays to allow indirect nesting (#377) --- Firestore/CHANGELOG.md | 3 ++ .../Tests/Integration/API/FIRValidationTests.m | 55 +++++++++++++++++++++- Firestore/Source/API/FSTUserDataConverter.m | 24 +++++++--- 3 files changed, 75 insertions(+), 7 deletions(-) (limited to 'Firestore') diff --git a/Firestore/CHANGELOG.md b/Firestore/CHANGELOG.md index de617bf..4602abc 100644 --- a/Firestore/CHANGELOG.md +++ b/Firestore/CHANGELOG.md @@ -1,3 +1,6 @@ +# Unreleased +- [fixed] Fixed validation of nested arrays to allow indirect nesting + # v0.9.0 - [fixed] Add an NS_SWIFT_NAME for FIRSnapshotMetadata and FIRListenerRegistration - [fixed] Fixed retain cycle in FIRDocumentReference getDocumentWithCompletion: diff --git a/Firestore/Example/Tests/Integration/API/FIRValidationTests.m b/Firestore/Example/Tests/Integration/API/FIRValidationTests.m index 1ba1d7a..e4e7cd1 100644 --- a/Firestore/Example/Tests/Integration/API/FIRValidationTests.m +++ b/Firestore/Example/Tests/Integration/API/FIRValidationTests.m @@ -161,13 +161,66 @@ } } -- (void)testWritesWithNestedArraysFail { +- (void)testWritesWithDirectlyNestedArraysFail { [self expectWrite:@{ @"nested-array" : @[ @1, @[ @2 ] ] } toFailWithReason:@"Nested arrays are not supported"]; } +- (void)testWritesWithIndirectlyNestedArraysSucceed { + NSDictionary *data = @{ @"nested-array" : @[ @1, @{@"foo" : @[ @2 ]} ] }; + + FIRDocumentReference *ref = [self documentRef]; + FIRDocumentReference *ref2 = [self documentRef]; + + XCTestExpectation *expectation = [self expectationWithDescription:@"setData"]; + [ref setData:data + completion:^(NSError *_Nullable error) { + XCTAssertNil(error); + [expectation fulfill]; + }]; + [self awaitExpectations]; + + expectation = [self expectationWithDescription:@"batch.setData"]; + [[[ref.firestore batch] setData:data forDocument:ref] + commitWithCompletion:^(NSError *_Nullable error) { + XCTAssertNil(error); + [expectation fulfill]; + }]; + [self awaitExpectations]; + + expectation = [self expectationWithDescription:@"updateData"]; + [ref updateData:data + completion:^(NSError *_Nullable error) { + XCTAssertNil(error); + [expectation fulfill]; + }]; + [self awaitExpectations]; + + expectation = [self expectationWithDescription:@"batch.updateData"]; + [[[ref.firestore batch] updateData:data forDocument:ref] + commitWithCompletion:^(NSError *_Nullable error) { + XCTAssertNil(error); + [expectation fulfill]; + }]; + [self awaitExpectations]; + + XCTestExpectation *transactionDone = [self expectationWithDescription:@"transaction done"]; + [ref.firestore runTransactionWithBlock:^id(FIRTransaction *transaction, NSError **pError) { + // Note ref2 does not exist at this point so set that and update ref. + [transaction updateData:data forDocument:ref]; + [transaction setData:data forDocument:ref2]; + return nil; + } + completion:^(id result, NSError *error) { + // ends up being a no-op transaction. + XCTAssertNil(error); + [transactionDone fulfill]; + }]; + [self awaitExpectations]; +} + - (void)testWritesWithInvalidTypesFail { [self expectWrite:@{ @"foo" : @{@"bar" : self} diff --git a/Firestore/Source/API/FSTUserDataConverter.m b/Firestore/Source/API/FSTUserDataConverter.m index 7a6c950..eb4dbda 100644 --- a/Firestore/Source/API/FSTUserDataConverter.m +++ b/Firestore/Source/API/FSTUserDataConverter.m @@ -121,7 +121,10 @@ typedef NS_ENUM(NSInteger, FSTUserDataSource) { @interface FSTParseContext : NSObject /** The current path being parsed. */ // TODO(b/34871131): path should never be nil, but we don't support array paths right now. -@property(strong, nonatomic, readonly, nullable) FSTFieldPath *path; +@property(nonatomic, strong, readonly, nullable) FSTFieldPath *path; + +/** Whether or not this context corresponds to an element of an array. */ +@property(nonatomic, assign, readonly, getter=isArrayElement) BOOL arrayElement; /** * What type of API method provided the data being parsed; useful for determining which error @@ -146,6 +149,7 @@ typedef NS_ENUM(NSInteger, FSTUserDataSource) { */ - (instancetype)initWithSource:(FSTUserDataSource)dataSource path:(nullable FSTFieldPath *)path + arrayElement:(BOOL)arrayElement fieldTransforms:(NSMutableArray *)fieldTransforms fieldMask:(NSMutableArray *)fieldMask NS_DESIGNATED_INITIALIZER; @@ -161,6 +165,7 @@ typedef NS_ENUM(NSInteger, FSTUserDataSource) { + (instancetype)contextWithSource:(FSTUserDataSource)dataSource path:(nullable FSTFieldPath *)path { FSTParseContext *context = [[FSTParseContext alloc] initWithSource:dataSource path:path + arrayElement:NO fieldTransforms:[NSMutableArray array] fieldMask:[NSMutableArray array]]; [context validatePath]; @@ -169,11 +174,13 @@ typedef NS_ENUM(NSInteger, FSTUserDataSource) { - (instancetype)initWithSource:(FSTUserDataSource)dataSource path:(nullable FSTFieldPath *)path + arrayElement:(BOOL)arrayElement fieldTransforms:(NSMutableArray *)fieldTransforms fieldMask:(NSMutableArray *)fieldMask { if (self = [super init]) { _dataSource = dataSource; _path = path; + _arrayElement = arrayElement; _fieldTransforms = fieldTransforms; _fieldMask = fieldMask; } @@ -184,6 +191,7 @@ typedef NS_ENUM(NSInteger, FSTUserDataSource) { FSTParseContext *context = [[FSTParseContext alloc] initWithSource:self.dataSource path:[self.path pathByAppendingSegment:fieldName] + arrayElement:NO fieldTransforms:self.fieldTransforms fieldMask:self.fieldMask]; [context validatePathSegment:fieldName]; @@ -194,6 +202,7 @@ typedef NS_ENUM(NSInteger, FSTUserDataSource) { FSTParseContext *context = [[FSTParseContext alloc] initWithSource:self.dataSource path:[self.path pathByAppendingPath:fieldPath] + arrayElement:NO fieldTransforms:self.fieldTransforms fieldMask:self.fieldMask]; [context validatePath]; @@ -204,6 +213,7 @@ typedef NS_ENUM(NSInteger, FSTUserDataSource) { // TODO(b/34871131): We don't support array paths right now; so make path nil. return [[FSTParseContext alloc] initWithSource:self.dataSource path:nil + arrayElement:YES fieldTransforms:self.fieldTransforms fieldMask:self.fieldMask]; } @@ -377,9 +387,8 @@ typedef NS_ENUM(NSInteger, FSTUserDataSource) { - (nullable FSTFieldValue *)parseData:(id)input context:(FSTParseContext *)context { input = self.preConverter(input); if ([input isKindOfClass:[NSArray class]]) { - // TODO(b/34871131): We may need a different way to detect nested arrays once we support array - // paths (at which point we should include the path containing the array in the error message). - if (!context.path) { + // TODO(b/34871131): Include the path containing the array in the error message. + if (context.isArrayElement) { FSTThrowInvalidArgument(@"Nested arrays are not supported"); } NSArray *array = input; @@ -393,8 +402,11 @@ typedef NS_ENUM(NSInteger, FSTUserDataSource) { } [result addObject:parsedEntry]; }]; - // We don't support field mask paths more granular than the top-level array. - [context.fieldMask addObject:context.path]; + // If context.path is nil we are already inside an array and we don't support field mask paths + // more granular than the top-level array. + if (context.path) { + [context.fieldMask addObject:context.path]; + } return [[FSTArrayValue alloc] initWithValueNoCopy:result]; } else if ([input isKindOfClass:[NSDictionary class]]) { -- cgit v1.2.3