From 96a64b3e438be27a4fa96e5dbbdca03ef7b7b6d8 Mon Sep 17 00:00:00 2001 From: zxu123 Date: Thu, 29 Mar 2018 15:59:47 -0400 Subject: [PATCH 01/22] port FieldMask to C++ --- .../Tests/Local/FSTLocalSerializerTests.mm | 16 ++-- .../Example/Tests/Model/FSTMutationTests.mm | 5 +- .../Tests/Remote/FSTSerializerBetaTests.mm | 4 +- Firestore/Example/Tests/Util/FSTHelpers.mm | 6 +- Firestore/Source/API/FSTUserDataConverter.h | 17 ++-- Firestore/Source/API/FSTUserDataConverter.mm | 56 ++++++++---- Firestore/Source/Core/FSTTransaction.h | 1 - Firestore/Source/Model/FSTMutation.h | 38 ++------ Firestore/Source/Model/FSTMutation.mm | 64 ++++---------- Firestore/Source/Remote/FSTSerializerBeta.mm | 10 ++- .../firebase/firestore/model/CMakeLists.txt | 1 + .../src/firebase/firestore/model/field_mask.h | 88 +++++++++++++++++++ .../firebase/firestore/model/CMakeLists.txt | 1 + .../firestore/model/field_mask_test.cc | 38 ++++++++ 14 files changed, 229 insertions(+), 116 deletions(-) create mode 100644 Firestore/core/src/firebase/firestore/model/field_mask.h create mode 100644 Firestore/core/test/firebase/firestore/model/field_mask_test.cc diff --git a/Firestore/Example/Tests/Local/FSTLocalSerializerTests.mm b/Firestore/Example/Tests/Local/FSTLocalSerializerTests.mm index d94925d4e20..d810aa62514 100644 --- a/Firestore/Example/Tests/Local/FSTLocalSerializerTests.mm +++ b/Firestore/Example/Tests/Local/FSTLocalSerializerTests.mm @@ -41,11 +41,13 @@ #import "Firestore/Example/Tests/Util/FSTHelpers.h" #include "Firestore/core/src/firebase/firestore/model/database_id.h" +#include "Firestore/core/src/firebase/firestore/model/field_mask.h" #include "Firestore/core/src/firebase/firestore/util/string_apple.h" #include "Firestore/core/test/firebase/firestore/testutil/testutil.h" namespace testutil = firebase::firestore::testutil; using firebase::firestore::model::DatabaseId; +using firebase::firestore::model::FieldMask; NS_ASSUME_NONNULL_BEGIN @@ -76,13 +78,13 @@ - (void)setUp { - (void)testEncodesMutationBatch { FSTMutation *set = FSTTestSetMutation(@"foo/bar", @{ @"a" : @"b", @"num" : @1 }); - FSTMutation *patch = [[FSTPatchMutation alloc] - initWithKey:FSTTestDocKey(@"bar/baz") - fieldMask:[[FSTFieldMask alloc] initWithFields:{testutil::Field("a")}] - value:FSTTestObjectValue( - @{ @"a" : @"b", - @"num" : @1 }) - precondition:[FSTPrecondition preconditionWithExists:YES]]; + FSTMutation *patch = + [[FSTPatchMutation alloc] initWithKey:FSTTestDocKey(@"bar/baz") + fieldMask:FieldMask{testutil::Field("a")} + value:FSTTestObjectValue( + @{ @"a" : @"b", + @"num" : @1 }) + precondition:[FSTPrecondition preconditionWithExists:YES]]; FSTMutation *del = FSTTestDeleteMutation(@"baz/quux"); FIRTimestamp *writeTime = [FIRTimestamp timestamp]; FSTMutationBatch *model = [[FSTMutationBatch alloc] initWithBatchID:42 diff --git a/Firestore/Example/Tests/Model/FSTMutationTests.mm b/Firestore/Example/Tests/Model/FSTMutationTests.mm index 40ded408507..6866f6e9049 100644 --- a/Firestore/Example/Tests/Model/FSTMutationTests.mm +++ b/Firestore/Example/Tests/Model/FSTMutationTests.mm @@ -25,10 +25,12 @@ #import "Firestore/Example/Tests/Util/FSTHelpers.h" #include "Firestore/core/src/firebase/firestore/model/document_key.h" +#include "Firestore/core/src/firebase/firestore/model/field_mask.h" #include "Firestore/core/test/firebase/firestore/testutil/testutil.h" namespace testutil = firebase::firestore::testutil; using firebase::firestore::model::DocumentKey; +using firebase::firestore::model::FieldMask; @interface FSTMutationTests : XCTestCase @end @@ -69,9 +71,8 @@ - (void)testDeletesValuesFromTheFieldMask { FSTDocument *baseDoc = FSTTestDoc("collection/key", 0, docData, NO); DocumentKey key = testutil::Key("collection/key"); - FSTFieldMask *mask = [[FSTFieldMask alloc] initWithFields:{testutil::Field("foo.bar")}]; FSTMutation *patch = [[FSTPatchMutation alloc] initWithKey:key - fieldMask:mask + fieldMask:FieldMask({testutil::Field("foo.bar")}) value:[FSTObjectValue objectValue] precondition:[FSTPrecondition none]]; FSTMaybeDocument *patchedDoc = diff --git a/Firestore/Example/Tests/Remote/FSTSerializerBetaTests.mm b/Firestore/Example/Tests/Remote/FSTSerializerBetaTests.mm index 64f477736a2..17191f8c555 100644 --- a/Firestore/Example/Tests/Remote/FSTSerializerBetaTests.mm +++ b/Firestore/Example/Tests/Remote/FSTSerializerBetaTests.mm @@ -46,12 +46,14 @@ #import "Firestore/Example/Tests/Util/FSTHelpers.h" #include "Firestore/core/src/firebase/firestore/model/database_id.h" +#include "Firestore/core/src/firebase/firestore/model/field_mask.h" #include "Firestore/core/src/firebase/firestore/util/string_apple.h" #include "Firestore/core/test/firebase/firestore/testutil/testutil.h" namespace testutil = firebase::firestore::testutil; namespace util = firebase::firestore::util; using firebase::firestore::model::DatabaseId; +using firebase::firestore::model::FieldMask; NS_ASSUME_NONNULL_BEGIN @@ -63,7 +65,7 @@ - (GCFSValue *)encodedInteger:(int64_t)value; - (GCFSValue *)encodedString:(NSString *)value; - (GCFSValue *)encodedDate:(NSDate *)value; -- (GCFSDocumentMask *)encodedFieldMask:(FSTFieldMask *)fieldMask; +- (GCFSDocumentMask *)encodedFieldMask:(const FieldMask &)fieldMask; - (NSMutableArray *)encodedFieldTransforms: (NSArray *)fieldTransforms; diff --git a/Firestore/Example/Tests/Util/FSTHelpers.mm b/Firestore/Example/Tests/Util/FSTHelpers.mm index 57517394d68..8a7306c6aae 100644 --- a/Firestore/Example/Tests/Util/FSTHelpers.mm +++ b/Firestore/Example/Tests/Util/FSTHelpers.mm @@ -44,6 +44,7 @@ #include "Firestore/core/src/firebase/firestore/model/database_id.h" #include "Firestore/core/src/firebase/firestore/model/document_key.h" +#include "Firestore/core/src/firebase/firestore/model/field_mask.h" #include "Firestore/core/src/firebase/firestore/model/field_value.h" #include "Firestore/core/src/firebase/firestore/model/resource_path.h" #include "Firestore/core/src/firebase/firestore/util/string_apple.h" @@ -53,6 +54,7 @@ namespace testutil = firebase::firestore::testutil; using firebase::firestore::model::DatabaseId; using firebase::firestore::model::DocumentKey; +using firebase::firestore::model::FieldMask; using firebase::firestore::model::FieldPath; using firebase::firestore::model::FieldValue; using firebase::firestore::model::ResourcePath; @@ -260,8 +262,8 @@ NSComparator FSTTestDocComparator(const absl::string_view fieldPath) { } }]; - FSTDocumentKey *key = [FSTDocumentKey keyWithPath:testutil::Resource(path)]; - FSTFieldMask *mask = [[FSTFieldMask alloc] initWithFields:merge ? updateMask : fieldMaskPaths]; + DocumentKey key = testutil::Key(path); + FieldMask mask(merge ? updateMask : fieldMaskPaths); return [[FSTPatchMutation alloc] initWithKey:key fieldMask:mask value:objectValue diff --git a/Firestore/Source/API/FSTUserDataConverter.h b/Firestore/Source/API/FSTUserDataConverter.h index 3b178be5cd3..7be56e4bce2 100644 --- a/Firestore/Source/API/FSTUserDataConverter.h +++ b/Firestore/Source/API/FSTUserDataConverter.h @@ -18,10 +18,10 @@ #include "Firestore/core/src/firebase/firestore/model/database_id.h" #include "Firestore/core/src/firebase/firestore/model/document_key.h" +#include "Firestore/core/src/firebase/firestore/model/field_mask.h" @class FIRSetOptions; @class FSTObjectValue; -@class FSTFieldMask; @class FSTFieldValue; @class FSTFieldTransform; @class FSTMutation; @@ -36,13 +36,19 @@ NS_ASSUME_NONNULL_BEGIN - (instancetype)init NS_UNAVAILABLE; - (instancetype)initWithData:(FSTObjectValue *)data - fieldMask:(nullable FSTFieldMask *)fieldMask fieldTransforms:(NSArray *)fieldTransforms NS_DESIGNATED_INITIALIZER; +- (instancetype)initWithData:(FSTObjectValue *)data + fieldMask:(firebase::firestore::model::FieldMask)fieldMask + fieldTransforms:(NSArray *)fieldTransforms + NS_DESIGNATED_INITIALIZER; + +- (const firebase::firestore::model::FieldMask &)fieldMask; + @property(nonatomic, strong, readonly) FSTObjectValue *data; -@property(nonatomic, strong, readonly, nullable) FSTFieldMask *fieldMask; @property(nonatomic, strong, readonly) NSArray *fieldTransforms; +@property(nonatomic, assign, readonly) BOOL isPatch; /** * Converts the parsed document data into 1 or 2 mutations (depending on whether there are any @@ -59,12 +65,13 @@ NS_ASSUME_NONNULL_BEGIN - (instancetype)init NS_UNAVAILABLE; - (instancetype)initWithData:(FSTObjectValue *)data - fieldMask:(FSTFieldMask *)fieldMask + fieldMask:(firebase::firestore::model::FieldMask)fieldMask fieldTransforms:(NSArray *)fieldTransforms NS_DESIGNATED_INITIALIZER; +- (const firebase::firestore::model::FieldMask &)fieldMask; + @property(nonatomic, strong, readonly) FSTObjectValue *data; -@property(nonatomic, strong, readonly) FSTFieldMask *fieldMask; @property(nonatomic, strong, readonly) NSArray *fieldTransforms; /** diff --git a/Firestore/Source/API/FSTUserDataConverter.mm b/Firestore/Source/API/FSTUserDataConverter.mm index 7ee16de9346..ba5309ef506 100644 --- a/Firestore/Source/API/FSTUserDataConverter.mm +++ b/Firestore/Source/API/FSTUserDataConverter.mm @@ -35,6 +35,7 @@ #include "Firestore/core/src/firebase/firestore/model/database_id.h" #include "Firestore/core/src/firebase/firestore/model/document_key.h" +#include "Firestore/core/src/firebase/firestore/model/field_mask.h" #include "Firestore/core/src/firebase/firestore/model/field_path.h" #include "Firestore/core/src/firebase/firestore/util/string_apple.h" #include "absl/memory/memory.h" @@ -42,6 +43,7 @@ namespace util = firebase::firestore::util; using firebase::firestore::model::DatabaseId; using firebase::firestore::model::DocumentKey; +using firebase::firestore::model::FieldMask; using firebase::firestore::model::FieldPath; NS_ASSUME_NONNULL_BEGIN @@ -50,15 +52,30 @@ #pragma mark - FSTParsedSetData -@implementation FSTParsedSetData +@implementation FSTParsedSetData { + FieldMask _fieldMask; +} + +- (instancetype)initWithData:(FSTObjectValue *)data + fieldTransforms:(NSArray *)fieldTransforms { + self = [super init]; + if (self) { + _data = data; + _fieldTransforms = fieldTransforms; + _isPatch = NO; + } + return self; +} + - (instancetype)initWithData:(FSTObjectValue *)data - fieldMask:(nullable FSTFieldMask *)fieldMask + fieldMask:(FieldMask)fieldMask fieldTransforms:(NSArray *)fieldTransforms { self = [super init]; if (self) { _data = data; - _fieldMask = fieldMask; + _fieldMask = std::move(fieldMask); _fieldTransforms = fieldTransforms; + _isPatch = YES; } return self; } @@ -66,7 +83,7 @@ - (instancetype)initWithData:(FSTObjectValue *)data - (NSArray *)mutationsWithKey:(const DocumentKey &)key precondition:(FSTPrecondition *)precondition { NSMutableArray *mutations = [NSMutableArray array]; - if (self.fieldMask) { + if (self.isPatch) { [mutations addObject:[[FSTPatchMutation alloc] initWithKey:key fieldMask:self.fieldMask value:self.data @@ -83,18 +100,25 @@ - (instancetype)initWithData:(FSTObjectValue *)data return mutations; } +- (const firebase::firestore::model::FieldMask &)fieldMask { + return _fieldMask; +} + @end #pragma mark - FSTParsedUpdateData -@implementation FSTParsedUpdateData +@implementation FSTParsedUpdateData { + FieldMask _fieldMask; +} + - (instancetype)initWithData:(FSTObjectValue *)data - fieldMask:(FSTFieldMask *)fieldMask + fieldMask:(FieldMask)fieldMask fieldTransforms:(NSArray *)fieldTransforms { self = [super init]; if (self) { _data = data; - _fieldMask = fieldMask; + _fieldMask = std::move(fieldMask); _fieldTransforms = fieldTransforms; } return self; @@ -114,6 +138,10 @@ - (instancetype)initWithData:(FSTObjectValue *)data return mutations; } +- (const firebase::firestore::model::FieldMask &)fieldMask { + return _fieldMask; +} + @end /** @@ -364,10 +392,9 @@ - (FSTParsedSetData *)parsedMergeData:(id)input { path:absl::make_unique(FieldPath::EmptyPath())]; FSTObjectValue *updateData = (FSTObjectValue *)[self parseData:input context:context]; - return [[FSTParsedSetData alloc] - initWithData:updateData - fieldMask:[[FSTFieldMask alloc] initWithFields:*context.fieldMask] - fieldTransforms:context.fieldTransforms]; + return [[FSTParsedSetData alloc] initWithData:updateData + fieldMask:FieldMask{*context.fieldMask} + fieldTransforms:context.fieldTransforms]; } - (FSTParsedSetData *)parsedSetData:(id)input { @@ -382,9 +409,7 @@ - (FSTParsedSetData *)parsedSetData:(id)input { path:absl::make_unique(FieldPath::EmptyPath())]; FSTObjectValue *updateData = (FSTObjectValue *)[self parseData:input context:context]; - return [[FSTParsedSetData alloc] initWithData:updateData - fieldMask:nil - fieldTransforms:context.fieldTransforms]; + return [[FSTParsedSetData alloc] initWithData:updateData fieldTransforms:context.fieldTransforms]; } - (FSTParsedUpdateData *)parsedUpdateData:(id)input { @@ -428,9 +453,8 @@ - (FSTParsedUpdateData *)parsedUpdateData:(id)input { } }]; - FSTFieldMask *mask = [[FSTFieldMask alloc] initWithFields:fieldMaskPaths]; return [[FSTParsedUpdateData alloc] initWithData:updateData - fieldMask:mask + fieldMask:FieldMask{fieldMaskPaths} fieldTransforms:context.fieldTransforms]; } diff --git a/Firestore/Source/Core/FSTTransaction.h b/Firestore/Source/Core/FSTTransaction.h index 676ada961a7..01c2e20f069 100644 --- a/Firestore/Source/Core/FSTTransaction.h +++ b/Firestore/Source/Core/FSTTransaction.h @@ -24,7 +24,6 @@ @class FIRSetOptions; @class FSTDatastore; -@class FSTFieldMask; @class FSTFieldTransform; @class FSTMaybeDocument; @class FSTObjectValue; diff --git a/Firestore/Source/Model/FSTMutation.h b/Firestore/Source/Model/FSTMutation.h index 4e4357df707..2b81af6764b 100644 --- a/Firestore/Source/Model/FSTMutation.h +++ b/Firestore/Source/Model/FSTMutation.h @@ -19,6 +19,7 @@ #include #include "Firestore/core/src/firebase/firestore/model/document_key.h" +#include "Firestore/core/src/firebase/firestore/model/field_mask.h" #include "Firestore/core/src/firebase/firestore/model/field_path.h" @class FSTDocument; @@ -30,31 +31,6 @@ NS_ASSUME_NONNULL_BEGIN -#pragma mark - FSTFieldMask - -/** - * Provides a set of fields that can be used to partially patch a document. FieldMask is used in - * conjunction with ObjectValue. - * - * Examples: - * foo - Overwrites foo entirely with the provided value. If foo is not present in the companion - * ObjectValue, the field is deleted. - * foo.bar - Overwrites only the field bar of the object foo. If foo is not an object, foo is - * replaced with an object containing bar. - */ -@interface FSTFieldMask : NSObject -- (id)init __attribute__((unavailable("Use initWithFields:"))); - -/** - * Initializes the field mask with the given field paths. Caller is expected to either copy or - * or release the array of fields. - */ -- (instancetype)initWithFields:(std::vector)fields - NS_DESIGNATED_INITIALIZER; - -- (const std::vector &)fields; -@end - #pragma mark - FSTFieldTransform /** Represents a transform within a TransformMutation. */ @@ -267,7 +243,7 @@ typedef NS_ENUM(NSUInteger, FSTPreconditionExists) { precondition:(FSTPrecondition *)precondition NS_UNAVAILABLE; /** - * Initializes a new patch mutation with an explicit FSTFieldMask and FSTObjectValue representing + * Initializes a new patch mutation with an explicit FieldMask and FSTObjectValue representing * the updates to perform * * @param key Identifies the location of the document to mutate. @@ -278,18 +254,18 @@ typedef NS_ENUM(NSUInteger, FSTPreconditionExists) { * @param precondition The precondition for this mutation. */ - (instancetype)initWithKey:(firebase::firestore::model::DocumentKey)key - fieldMask:(FSTFieldMask *)fieldMask + fieldMask:(firebase::firestore::model::FieldMask)fieldMask value:(FSTObjectValue *)value precondition:(FSTPrecondition *)precondition NS_DESIGNATED_INITIALIZER; -/** The fields and associated values to use when patching the document. */ -@property(nonatomic, strong, readonly) FSTObjectValue *value; - /** * A mask to apply to |value|, where only fields that are in both the fieldMask and the value * will be updated. */ -@property(nonatomic, strong, readonly) FSTFieldMask *fieldMask; +- (const firebase::firestore::model::FieldMask &)fieldMask; + +/** The fields and associated values to use when patching the document. */ +@property(nonatomic, strong, readonly) FSTObjectValue *value; @end diff --git a/Firestore/Source/Model/FSTMutation.mm b/Firestore/Source/Model/FSTMutation.mm index 253a8532093..c2c0e8cf906 100644 --- a/Firestore/Source/Model/FSTMutation.mm +++ b/Firestore/Source/Model/FSTMutation.mm @@ -27,51 +27,15 @@ #import "Firestore/Source/Util/FSTClasses.h" #include "Firestore/core/src/firebase/firestore/model/document_key.h" +#include "Firestore/core/src/firebase/firestore/model/field_mask.h" #include "Firestore/core/src/firebase/firestore/model/field_path.h" using firebase::firestore::model::DocumentKey; +using firebase::firestore::model::FieldMask; using firebase::firestore::model::FieldPath; NS_ASSUME_NONNULL_BEGIN -#pragma mark - FSTFieldMask - -@implementation FSTFieldMask { - std::vector _fields; -} - -- (instancetype)initWithFields:(std::vector)fields { - if (self = [super init]) { - _fields = std::move(fields); - } - return self; -} - -- (BOOL)isEqual:(id)other { - if (other == self) { - return YES; - } - if (![other isKindOfClass:[FSTFieldMask class]]) { - return NO; - } - - FSTFieldMask *otherMask = (FSTFieldMask *)other; - return _fields == otherMask->_fields; -} - -- (NSUInteger)hash { - NSUInteger hashResult = 0; - for (const FieldPath &field : _fields) { - hashResult = hashResult * 31u + field.Hash(); - } - return hashResult; -} - -- (const std::vector &)fields { - return _fields; -} -@end - #pragma mark - FSTServerTimestampTransform @implementation FSTServerTimestampTransform @@ -354,20 +318,26 @@ - (nullable FSTMaybeDocument *)applyTo:(nullable FSTMaybeDocument *)maybeDoc #pragma mark - FSTPatchMutation -@implementation FSTPatchMutation +@implementation FSTPatchMutation { + FieldMask _fieldMask; +} - (instancetype)initWithKey:(DocumentKey)key - fieldMask:(FSTFieldMask *)fieldMask + fieldMask:(FieldMask)fieldMask value:(FSTObjectValue *)value precondition:(FSTPrecondition *)precondition { self = [super initWithKey:std::move(key) precondition:precondition]; if (self) { - _fieldMask = fieldMask; + _fieldMask = std::move(fieldMask); _value = value; } return self; } +- (const firebase::firestore::model::FieldMask &)fieldMask { + return _fieldMask; +} + - (BOOL)isEqual:(id)other { if (other == self) { return YES; @@ -377,7 +347,7 @@ - (BOOL)isEqual:(id)other { } FSTPatchMutation *otherMutation = (FSTPatchMutation *)other; - return [self.key isEqual:otherMutation.key] && [self.fieldMask isEqual:otherMutation.fieldMask] && + return [self.key isEqual:otherMutation.key] && self.fieldMask == otherMutation.fieldMask && [self.value isEqual:otherMutation.value] && [self.precondition isEqual:otherMutation.precondition]; } @@ -385,15 +355,15 @@ - (BOOL)isEqual:(id)other { - (NSUInteger)hash { NSUInteger result = [self.key hash]; result = 31 * result + [self.precondition hash]; - result = 31 * result + [self.fieldMask hash]; + result = 31 * result + self.fieldMask.Hash(); result = 31 * result + [self.value hash]; return result; } - (NSString *)description { - return [NSString stringWithFormat:@"", - self.key.ToString().c_str(), self.fieldMask, self.value, - self.precondition]; + return [NSString stringWithFormat:@"", + self.key.ToString().c_str(), self.fieldMask.ToString().c_str(), + self.value, self.precondition]; } - (nullable FSTMaybeDocument *)applyTo:(nullable FSTMaybeDocument *)maybeDoc @@ -434,7 +404,7 @@ - (nullable FSTMaybeDocument *)applyTo:(nullable FSTMaybeDocument *)maybeDoc - (FSTObjectValue *)patchObjectValue:(FSTObjectValue *)objectValue { FSTObjectValue *result = objectValue; - for (const FieldPath &fieldPath : self.fieldMask.fields) { + for (const FieldPath &fieldPath : self.fieldMask.fields()) { FSTFieldValue *newValue = [self.value valueForPath:fieldPath]; if (newValue) { result = [result objectBySettingValue:newValue forPath:fieldPath]; diff --git a/Firestore/Source/Remote/FSTSerializerBeta.mm b/Firestore/Source/Remote/FSTSerializerBeta.mm index 9531ddcc4ec..fb4d7bc2a5c 100644 --- a/Firestore/Source/Remote/FSTSerializerBeta.mm +++ b/Firestore/Source/Remote/FSTSerializerBeta.mm @@ -44,6 +44,7 @@ #include "Firestore/core/src/firebase/firestore/model/database_id.h" #include "Firestore/core/src/firebase/firestore/model/document_key.h" +#include "Firestore/core/src/firebase/firestore/model/field_mask.h" #include "Firestore/core/src/firebase/firestore/model/field_path.h" #include "Firestore/core/src/firebase/firestore/model/resource_path.h" #include "Firestore/core/src/firebase/firestore/util/string_apple.h" @@ -51,6 +52,7 @@ namespace util = firebase::firestore::util; using firebase::firestore::model::DatabaseId; using firebase::firestore::model::DocumentKey; +using firebase::firestore::model::FieldMask; using firebase::firestore::model::FieldPath; using firebase::firestore::model::ResourcePath; @@ -539,21 +541,21 @@ - (FSTPrecondition *)decodedPrecondition:(GCFSPrecondition *)precondition { } } -- (GCFSDocumentMask *)encodedFieldMask:(FSTFieldMask *)fieldMask { +- (GCFSDocumentMask *)encodedFieldMask:(const FieldMask &)fieldMask { GCFSDocumentMask *mask = [GCFSDocumentMask message]; - for (const FieldPath &field : fieldMask.fields) { + for (const FieldPath &field : fieldMask.fields()) { [mask.fieldPathsArray addObject:util::WrapNSString(field.CanonicalString())]; } return mask; } -- (FSTFieldMask *)decodedFieldMask:(GCFSDocumentMask *)fieldMask { +- (FieldMask)decodedFieldMask:(GCFSDocumentMask *)fieldMask { std::vector fields{}; fields.reserve(fieldMask.fieldPathsArray_Count); for (NSString *path in fieldMask.fieldPathsArray) { fields.push_back(FieldPath::FromServerFormat(util::MakeStringView(path))); } - return [[FSTFieldMask alloc] initWithFields:std::move(fields)]; + return FieldMask(std::move(fields)); } - (NSMutableArray *)encodedFieldTransforms: diff --git a/Firestore/core/src/firebase/firestore/model/CMakeLists.txt b/Firestore/core/src/firebase/firestore/model/CMakeLists.txt index 78f5cd60557..f6c5efea857 100644 --- a/Firestore/core/src/firebase/firestore/model/CMakeLists.txt +++ b/Firestore/core/src/firebase/firestore/model/CMakeLists.txt @@ -22,6 +22,7 @@ cc_library( document.h document_key.cc document_key.h + field_mask.h field_path.cc field_path.h field_value.cc diff --git a/Firestore/core/src/firebase/firestore/model/field_mask.h b/Firestore/core/src/firebase/firestore/model/field_mask.h new file mode 100644 index 00000000000..ec82bc34630 --- /dev/null +++ b/Firestore/core/src/firebase/firestore/model/field_mask.h @@ -0,0 +1,88 @@ +/* + * Copyright 2018 Google + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#ifndef FIRESTORE_CORE_SRC_FIREBASE_FIRESTORE_MODEL_FIELD_MASK_H_ +#define FIRESTORE_CORE_SRC_FIREBASE_FIRESTORE_MODEL_FIELD_MASK_H_ + +#include +#include +#include + +#include "Firestore/core/src/firebase/firestore/model/field_path.h" + +namespace firebase { +namespace firestore { +namespace model { + +/** + * Provides a set of fields that can be used to partially patch a document. + * FieldMask is used in conjunction with FieldValue of Object type. + * + * Examples: + * foo - Overwrites foo entirely with the provided value. If foo is not + * present in the companion FieldValue, the field is deleted. + * foo.bar - Overwrites only the field bar of the object foo. If foo is not an + * object, foo is replaced with an object containing bar. + */ +class FieldMask { + public: + explicit FieldMask(std::initializer_list list) : fields_{list} { + } + explicit FieldMask(const std::vector& fields) : fields_{fields} { + } + explicit FieldMask(std::vector&& fields) : fields_{fields} { + } + + const std::vector& fields() const { + return fields_; + } + +#if defined(__OBJC__) + FieldMask() { + } + + std::string ToString() const { + // Ideally, one should use a string builder. But since this is only + // temporary non-critical code, the logic is kept simple here. + std::string result("{ "); + for (const FieldPath& field : fields_) { + result += field.CanonicalString() + " "; + } + return result + "}"; + } + + NSUInteger Hash() const { + NSUInteger hashResult = 0; + for (const FieldPath& field : fields_) { + hashResult = hashResult * 31u + field.Hash(); + } + return hashResult; + } +#endif + + private: + std::vector fields_; +}; + +inline bool operator==(const FieldMask& lhs, const FieldMask& rhs) { + return lhs.fields() == rhs.fields(); +} + +} // namespace model +} // namespace firestore +} // namespace firebase + +#endif // FIRESTORE_CORE_SRC_FIREBASE_FIRESTORE_MODEL_NO_DOCUMENT_H_ diff --git a/Firestore/core/test/firebase/firestore/model/CMakeLists.txt b/Firestore/core/test/firebase/firestore/model/CMakeLists.txt index 2c2281f3ad7..59a1f036603 100644 --- a/Firestore/core/test/firebase/firestore/model/CMakeLists.txt +++ b/Firestore/core/test/firebase/firestore/model/CMakeLists.txt @@ -18,6 +18,7 @@ cc_test( database_id_test.cc document_key_test.cc document_test.cc + field_mask_test.cc field_path_test.cc field_value_test.cc maybe_document_test.cc diff --git a/Firestore/core/test/firebase/firestore/model/field_mask_test.cc b/Firestore/core/test/firebase/firestore/model/field_mask_test.cc new file mode 100644 index 00000000000..7a5ee15a61a --- /dev/null +++ b/Firestore/core/test/firebase/firestore/model/field_mask_test.cc @@ -0,0 +1,38 @@ +/* + * Copyright 2018 Google + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include "Firestore/core/src/firebase/firestore/model/field_mask.h" + +#include + +#include "Firestore/core/src/firebase/firestore/model/field_path.h" +#include "gtest/gtest.h" + +namespace firebase { +namespace firestore { +namespace model { + +TEST(FieldMask, Getter) { + FieldMask mask{FieldPath::FromServerFormat("foo"), + FieldPath::FromServerFormat("bar")}; + EXPECT_EQ(std::vector({FieldPath::FromServerFormat("foo"), + FieldPath::FromServerFormat("bar")}), + mask.fields()); +} + +} // namespace model +} // namespace firestore +} // namespace firebase From c6ee4b0dc6034513d2d8f9749782f07854f2f8de Mon Sep 17 00:00:00 2001 From: zxu123 Date: Tue, 3 Apr 2018 10:49:26 -0400 Subject: [PATCH 02/22] address changes --- .../Example/Tests/Model/FSTMutationTests.mm | 2 +- Firestore/Source/Model/FSTMutation.mm | 2 +- Firestore/Source/Remote/FSTSerializerBeta.mm | 2 +- .../src/firebase/firestore/model/field_mask.h | 33 ++++++++++++------- .../firestore/model/field_mask_test.cc | 18 ++++++++++ 5 files changed, 43 insertions(+), 14 deletions(-) diff --git a/Firestore/Example/Tests/Model/FSTMutationTests.mm b/Firestore/Example/Tests/Model/FSTMutationTests.mm index 6866f6e9049..39ca9b17136 100644 --- a/Firestore/Example/Tests/Model/FSTMutationTests.mm +++ b/Firestore/Example/Tests/Model/FSTMutationTests.mm @@ -72,7 +72,7 @@ - (void)testDeletesValuesFromTheFieldMask { DocumentKey key = testutil::Key("collection/key"); FSTMutation *patch = [[FSTPatchMutation alloc] initWithKey:key - fieldMask:FieldMask({testutil::Field("foo.bar")}) + fieldMask:{testutil::Field("foo.bar")} value:[FSTObjectValue objectValue] precondition:[FSTPrecondition none]]; FSTMaybeDocument *patchedDoc = diff --git a/Firestore/Source/Model/FSTMutation.mm b/Firestore/Source/Model/FSTMutation.mm index c2c0e8cf906..df951558a9c 100644 --- a/Firestore/Source/Model/FSTMutation.mm +++ b/Firestore/Source/Model/FSTMutation.mm @@ -404,7 +404,7 @@ - (nullable FSTMaybeDocument *)applyTo:(nullable FSTMaybeDocument *)maybeDoc - (FSTObjectValue *)patchObjectValue:(FSTObjectValue *)objectValue { FSTObjectValue *result = objectValue; - for (const FieldPath &fieldPath : self.fieldMask.fields()) { + for (const FieldPath &fieldPath : self.fieldMask) { FSTFieldValue *newValue = [self.value valueForPath:fieldPath]; if (newValue) { result = [result objectBySettingValue:newValue forPath:fieldPath]; diff --git a/Firestore/Source/Remote/FSTSerializerBeta.mm b/Firestore/Source/Remote/FSTSerializerBeta.mm index fb4d7bc2a5c..3ea68e396fd 100644 --- a/Firestore/Source/Remote/FSTSerializerBeta.mm +++ b/Firestore/Source/Remote/FSTSerializerBeta.mm @@ -543,7 +543,7 @@ - (FSTPrecondition *)decodedPrecondition:(GCFSPrecondition *)precondition { - (GCFSDocumentMask *)encodedFieldMask:(const FieldMask &)fieldMask { GCFSDocumentMask *mask = [GCFSDocumentMask message]; - for (const FieldPath &field : fieldMask.fields()) { + for (const FieldPath &field : fieldMask) { [mask.fieldPathsArray addObject:util::WrapNSString(field.CanonicalString())]; } return mask; diff --git a/Firestore/core/src/firebase/firestore/model/field_mask.h b/Firestore/core/src/firebase/firestore/model/field_mask.h index ec82bc34630..f27e9c69946 100644 --- a/Firestore/core/src/firebase/firestore/model/field_mask.h +++ b/Firestore/core/src/firebase/firestore/model/field_mask.h @@ -17,7 +17,9 @@ #ifndef FIRESTORE_CORE_SRC_FIREBASE_FIRESTORE_MODEL_FIELD_MASK_H_ #define FIRESTORE_CORE_SRC_FIREBASE_FIRESTORE_MODEL_FIELD_MASK_H_ +#include #include +#include #include #include @@ -39,24 +41,26 @@ namespace model { */ class FieldMask { public: - explicit FieldMask(std::initializer_list list) : fields_{list} { + using const_iterator = std::vector::const_iterator; + + FieldMask(std::initializer_list list) : fields_{list} { } explicit FieldMask(const std::vector& fields) : fields_{fields} { } - explicit FieldMask(std::vector&& fields) : fields_{fields} { + explicit FieldMask(std::vector&& fields) + : fields_{std::move(fields)} { } - const std::vector& fields() const { - return fields_; + const_iterator begin() const { + return fields_.begin(); } - -#if defined(__OBJC__) - FieldMask() { + const_iterator end() const { + return fields_.end(); } std::string ToString() const { - // Ideally, one should use a string builder. But since this is only - // temporary non-critical code, the logic is kept simple here. + // Ideally, one should use a string builder. Since this is only non-critical + // code for logging and debugging, the logic is kept simple here. std::string result("{ "); for (const FieldPath& field : fields_) { result += field.CanonicalString() + " "; @@ -64,6 +68,10 @@ class FieldMask { return result + "}"; } +#if defined(__OBJC__) + FieldMask() { + } + NSUInteger Hash() const { NSUInteger hashResult = 0; for (const FieldPath& field : fields_) { @@ -73,16 +81,19 @@ class FieldMask { } #endif + friend bool operator==(const FieldMask& lhs, const FieldMask& rhs); + private: std::vector fields_; }; inline bool operator==(const FieldMask& lhs, const FieldMask& rhs) { - return lhs.fields() == rhs.fields(); + return lhs.fields_.size() == rhs.fields_.size() && + std::equal(lhs.begin(), lhs.end(), rhs.begin()); } } // namespace model } // namespace firestore } // namespace firebase -#endif // FIRESTORE_CORE_SRC_FIREBASE_FIRESTORE_MODEL_NO_DOCUMENT_H_ +#endif // FIRESTORE_CORE_SRC_FIREBASE_FIRESTORE_MODEL_FIELD_MASK_H_ diff --git a/Firestore/core/test/firebase/firestore/model/field_mask_test.cc b/Firestore/core/test/firebase/firestore/model/field_mask_test.cc index 7a5ee15a61a..c3e262b9648 100644 --- a/Firestore/core/test/firebase/firestore/model/field_mask_test.cc +++ b/Firestore/core/test/firebase/firestore/model/field_mask_test.cc @@ -25,6 +25,18 @@ namespace firebase { namespace firestore { namespace model { +TEST(FieldMask, ConstructorAndEqual) { + FieldMask mask_a{FieldPath::FromServerFormat("foo"), + FieldPath::FromServerFormat("bar")}; + std::vector field_path_vector{FieldPath::FromServerFormat("foo"), + FieldPath::FromServerFormat("bar")}; + FieldMask mask_b{field_path_vector}; + FieldMask mask_c{std::vector{FieldPath::FromServerFormat("foo"), + FieldPath::FromServerFormat("bar")}}; + EXPECT_EQ(mask_a, mask_b); + EXPECT_EQ(mask_b, mask_c); +} + TEST(FieldMask, Getter) { FieldMask mask{FieldPath::FromServerFormat("foo"), FieldPath::FromServerFormat("bar")}; @@ -33,6 +45,12 @@ TEST(FieldMask, Getter) { mask.fields()); } +TEST(FieldMask, ToString) { + FieldMask mask{FieldPath::FromServerFormat("foo"), + FieldPath::FromServerFormat("bar")}; + EXPECT_EQ("{ foo bar }", mask.ToString()); +} + } // namespace model } // namespace firestore } // namespace firebase From 64cfaac93e0ac02a3f8819259b28760ff82fa886 Mon Sep 17 00:00:00 2001 From: zxu123 Date: Tue, 3 Apr 2018 11:35:07 -0400 Subject: [PATCH 03/22] address changes --- Firestore/Source/API/FSTUserDataConverter.h | 2 -- Firestore/Source/API/FSTUserDataConverter.mm | 6 +----- 2 files changed, 1 insertion(+), 7 deletions(-) diff --git a/Firestore/Source/API/FSTUserDataConverter.h b/Firestore/Source/API/FSTUserDataConverter.h index 7be56e4bce2..2b4b3400419 100644 --- a/Firestore/Source/API/FSTUserDataConverter.h +++ b/Firestore/Source/API/FSTUserDataConverter.h @@ -44,8 +44,6 @@ NS_ASSUME_NONNULL_BEGIN fieldTransforms:(NSArray *)fieldTransforms NS_DESIGNATED_INITIALIZER; -- (const firebase::firestore::model::FieldMask &)fieldMask; - @property(nonatomic, strong, readonly) FSTObjectValue *data; @property(nonatomic, strong, readonly) NSArray *fieldTransforms; @property(nonatomic, assign, readonly) BOOL isPatch; diff --git a/Firestore/Source/API/FSTUserDataConverter.mm b/Firestore/Source/API/FSTUserDataConverter.mm index ba5309ef506..34f10157832 100644 --- a/Firestore/Source/API/FSTUserDataConverter.mm +++ b/Firestore/Source/API/FSTUserDataConverter.mm @@ -85,7 +85,7 @@ - (instancetype)initWithData:(FSTObjectValue *)data NSMutableArray *mutations = [NSMutableArray array]; if (self.isPatch) { [mutations addObject:[[FSTPatchMutation alloc] initWithKey:key - fieldMask:self.fieldMask + fieldMask:_fieldMask value:self.data precondition:precondition]]; } else { @@ -100,10 +100,6 @@ - (instancetype)initWithData:(FSTObjectValue *)data return mutations; } -- (const firebase::firestore::model::FieldMask &)fieldMask { - return _fieldMask; -} - @end #pragma mark - FSTParsedUpdateData From d27dc11c9cf9695f418e43e89ea01620dd10ddb6 Mon Sep 17 00:00:00 2001 From: zxu123 Date: Tue, 3 Apr 2018 11:38:25 -0400 Subject: [PATCH 04/22] fix test --- Firestore/core/test/firebase/firestore/model/field_mask_test.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Firestore/core/test/firebase/firestore/model/field_mask_test.cc b/Firestore/core/test/firebase/firestore/model/field_mask_test.cc index c3e262b9648..52d5951510d 100644 --- a/Firestore/core/test/firebase/firestore/model/field_mask_test.cc +++ b/Firestore/core/test/firebase/firestore/model/field_mask_test.cc @@ -42,7 +42,7 @@ TEST(FieldMask, Getter) { FieldPath::FromServerFormat("bar")}; EXPECT_EQ(std::vector({FieldPath::FromServerFormat("foo"), FieldPath::FromServerFormat("bar")}), - mask.fields()); + std::vector(mask.begin(), mask.end())); } TEST(FieldMask, ToString) { From 235bc2010fe441b96f2ca24b727fb196c159212e Mon Sep 17 00:00:00 2001 From: zxu123 Date: Tue, 3 Apr 2018 12:59:29 -0400 Subject: [PATCH 05/22] address change --- Firestore/core/src/firebase/firestore/model/field_mask.h | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/Firestore/core/src/firebase/firestore/model/field_mask.h b/Firestore/core/src/firebase/firestore/model/field_mask.h index f27e9c69946..76fe48c2a22 100644 --- a/Firestore/core/src/firebase/firestore/model/field_mask.h +++ b/Firestore/core/src/firebase/firestore/model/field_mask.h @@ -17,7 +17,6 @@ #ifndef FIRESTORE_CORE_SRC_FIREBASE_FIRESTORE_MODEL_FIELD_MASK_H_ #define FIRESTORE_CORE_SRC_FIREBASE_FIRESTORE_MODEL_FIELD_MASK_H_ -#include #include #include #include @@ -88,8 +87,7 @@ class FieldMask { }; inline bool operator==(const FieldMask& lhs, const FieldMask& rhs) { - return lhs.fields_.size() == rhs.fields_.size() && - std::equal(lhs.begin(), lhs.end(), rhs.begin()); + return lhs.fields_== rhs.fields_; } } // namespace model From a97ef146545b6867226a83622e3bce9b28f2656c Mon Sep 17 00:00:00 2001 From: zxu123 Date: Tue, 3 Apr 2018 15:37:00 -0400 Subject: [PATCH 06/22] Port transform operations (FSTTransformOperation, FSTServerTimestampTransform) to C++ --- Firestore/Example/Tests/Util/FSTHelpers.mm | 9 +- Firestore/Source/API/FSTUserDataConverter.mm | 5 +- Firestore/Source/Model/FSTMutation.h | 19 ++--- Firestore/Source/Model/FSTMutation.mm | 53 ++++-------- Firestore/Source/Remote/FSTSerializerBeta.mm | 18 ++-- .../firebase/firestore/model/CMakeLists.txt | 1 + .../src/firebase/firestore/model/field_mask.h | 2 +- .../firestore/model/transform_operations.h | 85 +++++++++++++++++++ .../firebase/firestore/model/CMakeLists.txt | 1 + .../model/transform_operations_test.cc | 51 +++++++++++ 10 files changed, 186 insertions(+), 58 deletions(-) create mode 100644 Firestore/core/src/firebase/firestore/model/transform_operations.h create mode 100644 Firestore/core/test/firebase/firestore/model/transform_operations_test.cc diff --git a/Firestore/Example/Tests/Util/FSTHelpers.mm b/Firestore/Example/Tests/Util/FSTHelpers.mm index 8a7306c6aae..2d69b4d0157 100644 --- a/Firestore/Example/Tests/Util/FSTHelpers.mm +++ b/Firestore/Example/Tests/Util/FSTHelpers.mm @@ -47,8 +47,10 @@ #include "Firestore/core/src/firebase/firestore/model/field_mask.h" #include "Firestore/core/src/firebase/firestore/model/field_value.h" #include "Firestore/core/src/firebase/firestore/model/resource_path.h" +#include "Firestore/core/src/firebase/firestore/model/transform_operations.h" #include "Firestore/core/src/firebase/firestore/util/string_apple.h" #include "Firestore/core/test/firebase/firestore/testutil/testutil.h" +#include "absl/memory/memory.h" namespace util = firebase::firestore::util; namespace testutil = firebase::firestore::testutil; @@ -58,6 +60,8 @@ using firebase::firestore::model::FieldPath; using firebase::firestore::model::FieldValue; using firebase::firestore::model::ResourcePath; +using firebase::firestore::model::ServerTimestampTransform; +using firebase::firestore::model::TransformOperation; NS_ASSUME_NONNULL_BEGIN @@ -277,9 +281,10 @@ NSComparator FSTTestDocComparator(const absl::string_view fieldPath) { NSMutableArray *fieldTransforms = [NSMutableArray array]; for (NSString *field in serverTimestampFields) { const FieldPath fieldPath = testutil::Field(util::MakeStringView(field)); - id transformOp = [FSTServerTimestampTransform serverTimestampTransform]; + std::unique_ptr transformOp = + absl::make_unique(ServerTimestampTransform::Get()); FSTFieldTransform *transform = - [[FSTFieldTransform alloc] initWithPath:fieldPath transform:transformOp]; + [[FSTFieldTransform alloc] initWithPath:fieldPath transform:std::move(transformOp)]; [fieldTransforms addObject:transform]; } return [[FSTTransformMutation alloc] initWithKey:key fieldTransforms:fieldTransforms]; diff --git a/Firestore/Source/API/FSTUserDataConverter.mm b/Firestore/Source/API/FSTUserDataConverter.mm index 34f10157832..c6a6e3462c2 100644 --- a/Firestore/Source/API/FSTUserDataConverter.mm +++ b/Firestore/Source/API/FSTUserDataConverter.mm @@ -37,6 +37,7 @@ #include "Firestore/core/src/firebase/firestore/model/document_key.h" #include "Firestore/core/src/firebase/firestore/model/field_mask.h" #include "Firestore/core/src/firebase/firestore/model/field_path.h" +#include "Firestore/core/src/firebase/firestore/model/transform_operations.h" #include "Firestore/core/src/firebase/firestore/util/string_apple.h" #include "absl/memory/memory.h" @@ -45,6 +46,7 @@ using firebase::firestore::model::DocumentKey; using firebase::firestore::model::FieldMask; using firebase::firestore::model::FieldPath; +using firebase::firestore::model::ServerTimestampTransform; NS_ASSUME_NONNULL_BEGIN @@ -662,7 +664,8 @@ - (nullable FSTFieldValue *)parseScalarValue:(nullable id)input context:(FSTPars [context.fieldTransforms addObject:[[FSTFieldTransform alloc] initWithPath:*context.path - transform:[FSTServerTimestampTransform serverTimestampTransform]]]; + transform:absl::make_unique( + ServerTimestampTransform::Get())]]; // Return nil so this value is omitted from the parsed result. return nil; diff --git a/Firestore/Source/Model/FSTMutation.h b/Firestore/Source/Model/FSTMutation.h index 2b81af6764b..2fa880656ce 100644 --- a/Firestore/Source/Model/FSTMutation.h +++ b/Firestore/Source/Model/FSTMutation.h @@ -16,11 +16,13 @@ #import +#include #include #include "Firestore/core/src/firebase/firestore/model/document_key.h" #include "Firestore/core/src/firebase/firestore/model/field_mask.h" #include "Firestore/core/src/firebase/firestore/model/field_path.h" +#include "Firestore/core/src/firebase/firestore/model/transform_operations.h" @class FSTDocument; @class FSTFieldValue; @@ -33,22 +35,15 @@ NS_ASSUME_NONNULL_BEGIN #pragma mark - FSTFieldTransform -/** Represents a transform within a TransformMutation. */ -@protocol FSTTransformOperation -@end - -/** Transforms a value into a server-generated timestamp. */ -@interface FSTServerTimestampTransform : NSObject -+ (instancetype)serverTimestampTransform; -@end - -/** A field path and the FSTTransformOperation to perform upon it. */ +/** A field path and the TransformOperation to perform upon it. */ @interface FSTFieldTransform : NSObject - (instancetype)init NS_UNAVAILABLE; - (instancetype)initWithPath:(firebase::firestore::model::FieldPath)path - transform:(id)transform NS_DESIGNATED_INITIALIZER; + transform: + (std::unique_ptr)transform + NS_DESIGNATED_INITIALIZER; - (const firebase::firestore::model::FieldPath &)path; -@property(nonatomic, strong, readonly) id transform; +- (const firebase::firestore::model::TransformOperation *)transform; @end #pragma mark - FSTPrecondition diff --git a/Firestore/Source/Model/FSTMutation.mm b/Firestore/Source/Model/FSTMutation.mm index df951558a9c..9adfe998227 100644 --- a/Firestore/Source/Model/FSTMutation.mm +++ b/Firestore/Source/Model/FSTMutation.mm @@ -16,6 +16,7 @@ #import "Firestore/Source/Model/FSTMutation.h" +#include #include #import "FIRTimestamp.h" @@ -29,51 +30,29 @@ #include "Firestore/core/src/firebase/firestore/model/document_key.h" #include "Firestore/core/src/firebase/firestore/model/field_mask.h" #include "Firestore/core/src/firebase/firestore/model/field_path.h" +#include "Firestore/core/src/firebase/firestore/model/transform_operations.h" using firebase::firestore::model::DocumentKey; using firebase::firestore::model::FieldMask; using firebase::firestore::model::FieldPath; +using firebase::firestore::model::ServerTimestampTransform; +using firebase::firestore::model::TransformOperation; NS_ASSUME_NONNULL_BEGIN -#pragma mark - FSTServerTimestampTransform - -@implementation FSTServerTimestampTransform - -+ (instancetype)serverTimestampTransform { - static FSTServerTimestampTransform *sharedInstance = nil; - static dispatch_once_t onceToken; - dispatch_once(&onceToken, ^{ - sharedInstance = [[FSTServerTimestampTransform alloc] init]; - }); - return sharedInstance; -} - -- (BOOL)isEqual:(id)other { - if (other == self) { - return YES; - } - return [other isKindOfClass:[FSTServerTimestampTransform class]]; -} - -- (NSUInteger)hash { - // arbitrary number since all instances are equal. - return 37; -} - -@end - #pragma mark - FSTFieldTransform @implementation FSTFieldTransform { FieldPath _path; + std::unique_ptr _transform; } -- (instancetype)initWithPath:(FieldPath)path transform:(id)transform { +- (instancetype)initWithPath:(FieldPath)path + transform:(std::unique_ptr)transform { self = [super init]; if (self) { _path = std::move(path); - _transform = transform; + _transform = std::move(transform); } return self; } @@ -83,12 +62,12 @@ - (BOOL)isEqual:(id)other { if (![[other class] isEqual:[self class]]) return NO; FSTFieldTransform *otherFieldTransform = other; return self.path == otherFieldTransform.path && - [self.transform isEqual:otherFieldTransform.transform]; + self.transform->operator==(otherFieldTransform.transform); } - (NSUInteger)hash { NSUInteger hash = self.path.Hash(); - hash = hash * 31 + [self.transform hash]; + hash = hash * 31 + self.transform->Hash(); return hash; } @@ -96,6 +75,10 @@ - (NSUInteger)hash { return _path; } +- (const firebase::firestore::model::TransformOperation *)transform { + return _transform.get(); +} + @end #pragma mark - FSTPrecondition @@ -505,7 +488,7 @@ - (nullable FSTMaybeDocument *)applyTo:(nullable FSTMaybeDocument *)maybeDoc writeTime:(FIRTimestamp *)localWriteTime { NSMutableArray *transformResults = [NSMutableArray array]; for (FSTFieldTransform *fieldTransform in self.fieldTransforms) { - if ([fieldTransform.transform isKindOfClass:[FSTServerTimestampTransform class]]) { + if (fieldTransform.transform->type() == TransformOperation::Type::ServerTimestamp) { FSTFieldValue *previousValue = nil; if ([baseDocument isMemberOfClass:[FSTDocument class]]) { @@ -529,12 +512,12 @@ - (FSTObjectValue *)transformObject:(FSTObjectValue *)objectValue for (NSUInteger i = 0; i < self.fieldTransforms.count; i++) { FSTFieldTransform *fieldTransform = self.fieldTransforms[i]; - id transform = fieldTransform.transform; + const TransformOperation *transform = fieldTransform.transform; FieldPath fieldPath = fieldTransform.path; - if ([transform isKindOfClass:[FSTServerTimestampTransform class]]) { + if (transform->type() == TransformOperation::Type::ServerTimestamp) { objectValue = [objectValue objectBySettingValue:transformResults[i] forPath:fieldPath]; } else { - FSTFail(@"Encountered unknown transform: %@", transform); + FSTFail(@"Encountered unknown transform: %d type", transform->type()); } } return objectValue; diff --git a/Firestore/Source/Remote/FSTSerializerBeta.mm b/Firestore/Source/Remote/FSTSerializerBeta.mm index 3ea68e396fd..8911d3c85ba 100644 --- a/Firestore/Source/Remote/FSTSerializerBeta.mm +++ b/Firestore/Source/Remote/FSTSerializerBeta.mm @@ -47,7 +47,9 @@ #include "Firestore/core/src/firebase/firestore/model/field_mask.h" #include "Firestore/core/src/firebase/firestore/model/field_path.h" #include "Firestore/core/src/firebase/firestore/model/resource_path.h" +#include "Firestore/core/src/firebase/firestore/model/transform_operations.h" #include "Firestore/core/src/firebase/firestore/util/string_apple.h" +#include "absl/memory/memory.h" namespace util = firebase::firestore::util; using firebase::firestore::model::DatabaseId; @@ -55,6 +57,8 @@ using firebase::firestore::model::FieldMask; using firebase::firestore::model::FieldPath; using firebase::firestore::model::ResourcePath; +using firebase::firestore::model::ServerTimestampTransform; +using firebase::firestore::model::TransformOperation; NS_ASSUME_NONNULL_BEGIN @@ -562,8 +566,8 @@ - (FieldMask)decodedFieldMask:(GCFSDocumentMask *)fieldMask { (NSArray *)fieldTransforms { NSMutableArray *protos = [NSMutableArray array]; for (FSTFieldTransform *fieldTransform in fieldTransforms) { - FSTAssert([fieldTransform.transform isKindOfClass:[FSTServerTimestampTransform class]], - @"Unknown transform: %@", fieldTransform.transform); + FSTAssert(fieldTransform.transform->type() == TransformOperation::Type::ServerTimestamp, + @"Unknown transform: %d type", fieldTransform.transform->type()); GCFSDocumentTransform_FieldTransform *proto = [GCFSDocumentTransform_FieldTransform message]; proto.fieldPath = util::WrapNSString(fieldTransform.path.CanonicalString()); proto.setToServerValue = GCFSDocumentTransform_FieldTransform_ServerValue_RequestTime; @@ -579,11 +583,11 @@ - (FieldMask)decodedFieldMask:(GCFSDocumentMask *)fieldMask { FSTAssert( proto.setToServerValue == GCFSDocumentTransform_FieldTransform_ServerValue_RequestTime, @"Unknown transform setToServerValue: %d", proto.setToServerValue); - [fieldTransforms - addObject:[[FSTFieldTransform alloc] - initWithPath:FieldPath::FromServerFormat( - util::MakeStringView(proto.fieldPath)) - transform:[FSTServerTimestampTransform serverTimestampTransform]]]; + [fieldTransforms addObject:[[FSTFieldTransform alloc] + initWithPath:FieldPath::FromServerFormat( + util::MakeStringView(proto.fieldPath)) + transform:absl::make_unique( + ServerTimestampTransform::Get())]]; } return fieldTransforms; } diff --git a/Firestore/core/src/firebase/firestore/model/CMakeLists.txt b/Firestore/core/src/firebase/firestore/model/CMakeLists.txt index f6c5efea857..d83c0aa2196 100644 --- a/Firestore/core/src/firebase/firestore/model/CMakeLists.txt +++ b/Firestore/core/src/firebase/firestore/model/CMakeLists.txt @@ -35,6 +35,7 @@ cc_library( resource_path.h snapshot_version.cc snapshot_version.h + transform_operations.h types.h DEPENDS absl_strings diff --git a/Firestore/core/src/firebase/firestore/model/field_mask.h b/Firestore/core/src/firebase/firestore/model/field_mask.h index 76fe48c2a22..a9f509a0aee 100644 --- a/Firestore/core/src/firebase/firestore/model/field_mask.h +++ b/Firestore/core/src/firebase/firestore/model/field_mask.h @@ -87,7 +87,7 @@ class FieldMask { }; inline bool operator==(const FieldMask& lhs, const FieldMask& rhs) { - return lhs.fields_== rhs.fields_; + return lhs.fields_ == rhs.fields_; } } // namespace model diff --git a/Firestore/core/src/firebase/firestore/model/transform_operations.h b/Firestore/core/src/firebase/firestore/model/transform_operations.h new file mode 100644 index 00000000000..ab23084f275 --- /dev/null +++ b/Firestore/core/src/firebase/firestore/model/transform_operations.h @@ -0,0 +1,85 @@ +/* + * Copyright 2018 Google + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#ifndef FIRESTORE_CORE_SRC_FIREBASE_FIRESTORE_MODEL_TRANSFORM_OPERATIONS_H_ +#define FIRESTORE_CORE_SRC_FIREBASE_FIRESTORE_MODEL_TRANSFORM_OPERATIONS_H_ + +namespace firebase { +namespace firestore { +namespace model { + +// TODO(zxu123): We might want to refactor transform_operations.h into several +// files when the number of different types of operations grows gigantically. +// For now, put the interface and the only operation here. + +/** Represents a transform within a TransformMutation. */ +class TransformOperation { + public: + /** All the different kinds to TransformOperation. */ + enum class Type { + ServerTimestamp, + }; + + /** Returns the actual type. */ + virtual Type type() const = 0; + + /** Returns whether the two are equal. */ + virtual bool operator==(const TransformOperation* other) const = 0; + +#if defined(__OBJC__) + // For Objective-C++ hash; to be removed after migration. + // Do NOT use in C++ code. + virtual NSUInteger Hash() const = 0; +#endif // defined(__OBJC__) +}; + +/** Transforms a value into a server-generated timestamp. */ +class ServerTimestampTransform : public TransformOperation { + public: + Type type() const override { + return Type::ServerTimestamp; + } + + bool operator==(const TransformOperation* other) const override { + // All ServerTimestampTransform objects are equal. + return other->type() == Type::ServerTimestamp; + } + + static const ServerTimestampTransform& Get() { + static ServerTimestampTransform shared_instance; + return shared_instance; + } + +#if defined(__OBJC__) + // For Objective-C++ hash; to be removed after migration. + // Do NOT use in C++ code. + NSUInteger Hash() const override { + // arbitrary number, the same as used in ObjC implementation, since all + // instances are equal. + return 37; + } +#endif // defined(__OBJC__) + + private: + ServerTimestampTransform() { + } +}; + +} // namespace model +} // namespace firestore +} // namespace firebase + +#endif // FIRESTORE_CORE_SRC_FIREBASE_FIRESTORE_MODEL_TRANSFORM_OPERATIONS_H_ diff --git a/Firestore/core/test/firebase/firestore/model/CMakeLists.txt b/Firestore/core/test/firebase/firestore/model/CMakeLists.txt index 59a1f036603..5e494f189d5 100644 --- a/Firestore/core/test/firebase/firestore/model/CMakeLists.txt +++ b/Firestore/core/test/firebase/firestore/model/CMakeLists.txt @@ -25,6 +25,7 @@ cc_test( no_document_test.cc resource_path_test.cc snapshot_version_test.cc + transform_operations_test.cc DEPENDS firebase_firestore_model ) diff --git a/Firestore/core/test/firebase/firestore/model/transform_operations_test.cc b/Firestore/core/test/firebase/firestore/model/transform_operations_test.cc new file mode 100644 index 00000000000..7ffbfad5273 --- /dev/null +++ b/Firestore/core/test/firebase/firestore/model/transform_operations_test.cc @@ -0,0 +1,51 @@ +/* + * Copyright 2018 Google + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include "Firestore/core/src/firebase/firestore/model/transform_operations.h" + +#include "gtest/gtest.h" + +namespace firebase { +namespace firestore { +namespace model { + +class DummyOperation : public TransformOperation { + public: + DummyOperation() { + } + + Type type() const override { + return static_cast(-1); + } + + bool operator==(const TransformOperation* other) const override { + return this == other; + } +}; + +TEST(TransformOperations, ServerTimestamp) { + ServerTimestampTransform transform = ServerTimestampTransform::Get(); + EXPECT_EQ(TransformOperation::Type::ServerTimestamp, transform.type()); + + ServerTimestampTransform another = ServerTimestampTransform::Get(); + DummyOperation dummy; + EXPECT_TRUE(transform.operator==(&another)); + EXPECT_FALSE(transform.operator==(&dummy)); +} + +} // namespace model +} // namespace firestore +} // namespace firebase From 6d47fe4a316fa74a680dd84ee1b0fc7f4745fda2 Mon Sep 17 00:00:00 2001 From: zxu123 Date: Wed, 4 Apr 2018 09:38:25 -0400 Subject: [PATCH 07/22] address changes --- Firestore/Source/Model/FSTMutation.mm | 3 +-- .../src/firebase/firestore/model/transform_operations.h | 6 +++--- .../firebase/firestore/model/transform_operations_test.cc | 8 ++++---- 3 files changed, 8 insertions(+), 9 deletions(-) diff --git a/Firestore/Source/Model/FSTMutation.mm b/Firestore/Source/Model/FSTMutation.mm index 9adfe998227..a18053ca3e8 100644 --- a/Firestore/Source/Model/FSTMutation.mm +++ b/Firestore/Source/Model/FSTMutation.mm @@ -61,8 +61,7 @@ - (BOOL)isEqual:(id)other { if (other == self) return YES; if (![[other class] isEqual:[self class]]) return NO; FSTFieldTransform *otherFieldTransform = other; - return self.path == otherFieldTransform.path && - self.transform->operator==(otherFieldTransform.transform); + return self.path == otherFieldTransform.path && *self.transform == *otherFieldTransform.transform; } - (NSUInteger)hash { diff --git a/Firestore/core/src/firebase/firestore/model/transform_operations.h b/Firestore/core/src/firebase/firestore/model/transform_operations.h index ab23084f275..043bb29566e 100644 --- a/Firestore/core/src/firebase/firestore/model/transform_operations.h +++ b/Firestore/core/src/firebase/firestore/model/transform_operations.h @@ -37,7 +37,7 @@ class TransformOperation { virtual Type type() const = 0; /** Returns whether the two are equal. */ - virtual bool operator==(const TransformOperation* other) const = 0; + virtual bool operator==(const TransformOperation& other) const = 0; #if defined(__OBJC__) // For Objective-C++ hash; to be removed after migration. @@ -53,9 +53,9 @@ class ServerTimestampTransform : public TransformOperation { return Type::ServerTimestamp; } - bool operator==(const TransformOperation* other) const override { + bool operator==(const TransformOperation& other) const override { // All ServerTimestampTransform objects are equal. - return other->type() == Type::ServerTimestamp; + return other.type() == Type::ServerTimestamp; } static const ServerTimestampTransform& Get() { diff --git a/Firestore/core/test/firebase/firestore/model/transform_operations_test.cc b/Firestore/core/test/firebase/firestore/model/transform_operations_test.cc index 7ffbfad5273..a651c552a31 100644 --- a/Firestore/core/test/firebase/firestore/model/transform_operations_test.cc +++ b/Firestore/core/test/firebase/firestore/model/transform_operations_test.cc @@ -31,8 +31,8 @@ class DummyOperation : public TransformOperation { return static_cast(-1); } - bool operator==(const TransformOperation* other) const override { - return this == other; + bool operator==(const TransformOperation& other) const override { + return this == &other; } }; @@ -42,8 +42,8 @@ TEST(TransformOperations, ServerTimestamp) { ServerTimestampTransform another = ServerTimestampTransform::Get(); DummyOperation dummy; - EXPECT_TRUE(transform.operator==(&another)); - EXPECT_FALSE(transform.operator==(&dummy)); + EXPECT_EQ(transform, another); + EXPECT_FALSE(transform == dummy); } } // namespace model From 9ea9ab2ffdd5139a4904e76884c1c8c625ef2e88 Mon Sep 17 00:00:00 2001 From: zxu123 Date: Wed, 4 Apr 2018 10:01:32 -0400 Subject: [PATCH 08/22] address changes --- Firestore/Example/Tests/Util/FSTHelpers.mm | 3 +-- .../core/src/firebase/firestore/model/transform_operations.h | 1 + .../test/firebase/firestore/model/transform_operations_test.cc | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Firestore/Example/Tests/Util/FSTHelpers.mm b/Firestore/Example/Tests/Util/FSTHelpers.mm index 2d69b4d0157..cc120a9c2e5 100644 --- a/Firestore/Example/Tests/Util/FSTHelpers.mm +++ b/Firestore/Example/Tests/Util/FSTHelpers.mm @@ -281,8 +281,7 @@ NSComparator FSTTestDocComparator(const absl::string_view fieldPath) { NSMutableArray *fieldTransforms = [NSMutableArray array]; for (NSString *field in serverTimestampFields) { const FieldPath fieldPath = testutil::Field(util::MakeStringView(field)); - std::unique_ptr transformOp = - absl::make_unique(ServerTimestampTransform::Get()); + auto transformOp = absl::make_unique(ServerTimestampTransform::Get()); FSTFieldTransform *transform = [[FSTFieldTransform alloc] initWithPath:fieldPath transform:std::move(transformOp)]; [fieldTransforms addObject:transform]; diff --git a/Firestore/core/src/firebase/firestore/model/transform_operations.h b/Firestore/core/src/firebase/firestore/model/transform_operations.h index 043bb29566e..494109ca80e 100644 --- a/Firestore/core/src/firebase/firestore/model/transform_operations.h +++ b/Firestore/core/src/firebase/firestore/model/transform_operations.h @@ -31,6 +31,7 @@ class TransformOperation { /** All the different kinds to TransformOperation. */ enum class Type { ServerTimestamp, + Test, // Purely for test purpose. }; /** Returns the actual type. */ diff --git a/Firestore/core/test/firebase/firestore/model/transform_operations_test.cc b/Firestore/core/test/firebase/firestore/model/transform_operations_test.cc index a651c552a31..53b053b0821 100644 --- a/Firestore/core/test/firebase/firestore/model/transform_operations_test.cc +++ b/Firestore/core/test/firebase/firestore/model/transform_operations_test.cc @@ -28,7 +28,7 @@ class DummyOperation : public TransformOperation { } Type type() const override { - return static_cast(-1); + return Type::Test; } bool operator==(const TransformOperation& other) const override { From f25eebc9f1b650d8766ec3cf9061ad734b74268b Mon Sep 17 00:00:00 2001 From: zxu123 Date: Wed, 4 Apr 2018 10:07:08 -0400 Subject: [PATCH 09/22] address changes --- .../core/src/firebase/firestore/model/transform_operations.h | 5 +++++ .../firebase/firestore/model/transform_operations_test.cc | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/Firestore/core/src/firebase/firestore/model/transform_operations.h b/Firestore/core/src/firebase/firestore/model/transform_operations.h index 494109ca80e..9311bc19ef7 100644 --- a/Firestore/core/src/firebase/firestore/model/transform_operations.h +++ b/Firestore/core/src/firebase/firestore/model/transform_operations.h @@ -40,6 +40,11 @@ class TransformOperation { /** Returns whether the two are equal. */ virtual bool operator==(const TransformOperation& other) const = 0; + /** Returns whether the two are not equal. */ + bool operator!=(const TransformOperation& other) const { + return !operator==(other); + } + #if defined(__OBJC__) // For Objective-C++ hash; to be removed after migration. // Do NOT use in C++ code. diff --git a/Firestore/core/test/firebase/firestore/model/transform_operations_test.cc b/Firestore/core/test/firebase/firestore/model/transform_operations_test.cc index 53b053b0821..0ef95dba26a 100644 --- a/Firestore/core/test/firebase/firestore/model/transform_operations_test.cc +++ b/Firestore/core/test/firebase/firestore/model/transform_operations_test.cc @@ -43,7 +43,7 @@ TEST(TransformOperations, ServerTimestamp) { ServerTimestampTransform another = ServerTimestampTransform::Get(); DummyOperation dummy; EXPECT_EQ(transform, another); - EXPECT_FALSE(transform == dummy); + EXPECT_NE(transform, dummy); } } // namespace model From 8eecd3540906e6f08a489cfc98fde27590b0e640 Mon Sep 17 00:00:00 2001 From: zxu123 Date: Wed, 4 Apr 2018 14:47:51 -0400 Subject: [PATCH 10/22] implement `FieldTransform` in C++ --- Firestore/Source/Model/FSTMutation.h | 13 ---- Firestore/Source/Model/FSTMutation.mm | 40 ----------- .../firebase/firestore/model/CMakeLists.txt | 1 + .../firestore/model/field_transform.h | 68 +++++++++++++++++++ .../firebase/firestore/model/CMakeLists.txt | 1 + .../firestore/model/field_transform_test.cc | 39 +++++++++++ 6 files changed, 109 insertions(+), 53 deletions(-) create mode 100644 Firestore/core/src/firebase/firestore/model/field_transform.h create mode 100644 Firestore/core/test/firebase/firestore/model/field_transform_test.cc diff --git a/Firestore/Source/Model/FSTMutation.h b/Firestore/Source/Model/FSTMutation.h index 2fa880656ce..9438ecddaca 100644 --- a/Firestore/Source/Model/FSTMutation.h +++ b/Firestore/Source/Model/FSTMutation.h @@ -33,19 +33,6 @@ NS_ASSUME_NONNULL_BEGIN -#pragma mark - FSTFieldTransform - -/** A field path and the TransformOperation to perform upon it. */ -@interface FSTFieldTransform : NSObject -- (instancetype)init NS_UNAVAILABLE; -- (instancetype)initWithPath:(firebase::firestore::model::FieldPath)path - transform: - (std::unique_ptr)transform - NS_DESIGNATED_INITIALIZER; -- (const firebase::firestore::model::FieldPath &)path; -- (const firebase::firestore::model::TransformOperation *)transform; -@end - #pragma mark - FSTPrecondition typedef NS_ENUM(NSUInteger, FSTPreconditionExists) { diff --git a/Firestore/Source/Model/FSTMutation.mm b/Firestore/Source/Model/FSTMutation.mm index a18053ca3e8..4ca387cfdfc 100644 --- a/Firestore/Source/Model/FSTMutation.mm +++ b/Firestore/Source/Model/FSTMutation.mm @@ -40,46 +40,6 @@ NS_ASSUME_NONNULL_BEGIN -#pragma mark - FSTFieldTransform - -@implementation FSTFieldTransform { - FieldPath _path; - std::unique_ptr _transform; -} - -- (instancetype)initWithPath:(FieldPath)path - transform:(std::unique_ptr)transform { - self = [super init]; - if (self) { - _path = std::move(path); - _transform = std::move(transform); - } - return self; -} - -- (BOOL)isEqual:(id)other { - if (other == self) return YES; - if (![[other class] isEqual:[self class]]) return NO; - FSTFieldTransform *otherFieldTransform = other; - return self.path == otherFieldTransform.path && *self.transform == *otherFieldTransform.transform; -} - -- (NSUInteger)hash { - NSUInteger hash = self.path.Hash(); - hash = hash * 31 + self.transform->Hash(); - return hash; -} - -- (const firebase::firestore::model::FieldPath &)path { - return _path; -} - -- (const firebase::firestore::model::TransformOperation *)transform { - return _transform.get(); -} - -@end - #pragma mark - FSTPrecondition @implementation FSTPrecondition diff --git a/Firestore/core/src/firebase/firestore/model/CMakeLists.txt b/Firestore/core/src/firebase/firestore/model/CMakeLists.txt index d83c0aa2196..de783ad80de 100644 --- a/Firestore/core/src/firebase/firestore/model/CMakeLists.txt +++ b/Firestore/core/src/firebase/firestore/model/CMakeLists.txt @@ -25,6 +25,7 @@ cc_library( field_mask.h field_path.cc field_path.h + field_transform.h field_value.cc field_value.h maybe_document.cc diff --git a/Firestore/core/src/firebase/firestore/model/field_transform.h b/Firestore/core/src/firebase/firestore/model/field_transform.h new file mode 100644 index 00000000000..6409c53e0c4 --- /dev/null +++ b/Firestore/core/src/firebase/firestore/model/field_transform.h @@ -0,0 +1,68 @@ +/* + * Copyright 2018 Google + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#ifndef FIRESTORE_CORE_SRC_FIREBASE_FIRESTORE_MODEL_FIELD_TRANSFORM_H_ +#define FIRESTORE_CORE_SRC_FIREBASE_FIRESTORE_MODEL_FIELD_TRANSFORM_H_ + +#include +#include + +#include "Firestore/core/src/firebase/firestore/model/field_path.h" +#include "Firestore/core/src/firebase/firestore/model/transform_operations.h" + +namespace firebase { +namespace firestore { +namespace model { + +/** A field path and the TransformOperation to perform upon it. */ +class FieldTransform { + public: + FieldTransform(FieldPath path, std::unique_ptr transform) + : path_(std::move(path)), transform_(std::move(transform)) { + } + + const FieldPath& path() const { + return path_; + } + + const TransformOperation* transform() const { + return transform_.get(); + } + +#if defined(__OBJC__) + bool operator==(const FieldTransform& other) const { + return path_ == other.path && *self.transform == *other.transform; + } + + // For Objective-C++ hash; to be removed after migration. + // Do NOT use in C++ code. + NSUInteger Hash() const { + NSUInteger hash = path_.Hash(); + hash = hash * 31 + transform_->Hash(); + return hash; + } +#endif // defined(__OBJC__) + + private: + FieldPath path_; + std::unique_ptr transform_; +}; + +} // namespace model +} // namespace firestore +} // namespace firebase + +#endif // FIRESTORE_CORE_SRC_FIREBASE_FIRESTORE_MODEL_FIELD_TRANSFORM_H_ diff --git a/Firestore/core/test/firebase/firestore/model/CMakeLists.txt b/Firestore/core/test/firebase/firestore/model/CMakeLists.txt index 5e494f189d5..3bac89d4124 100644 --- a/Firestore/core/test/firebase/firestore/model/CMakeLists.txt +++ b/Firestore/core/test/firebase/firestore/model/CMakeLists.txt @@ -20,6 +20,7 @@ cc_test( document_test.cc field_mask_test.cc field_path_test.cc + field_transform_test.cc field_value_test.cc maybe_document_test.cc no_document_test.cc diff --git a/Firestore/core/test/firebase/firestore/model/field_transform_test.cc b/Firestore/core/test/firebase/firestore/model/field_transform_test.cc new file mode 100644 index 00000000000..524bd91e12f --- /dev/null +++ b/Firestore/core/test/firebase/firestore/model/field_transform_test.cc @@ -0,0 +1,39 @@ +/* + * Copyright 2018 Google + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include "Firestore/core/src/firebase/firestore/model/field_transform.h" +#include "Firestore/core/src/firebase/firestore/model/transform_operations.h" +#include "Firestore/core/test/firebase/firestore/testutil/testutil.h" + +#include "absl/memory/memory.h" +#include "gtest/gtest.h" + +namespace firebase { +namespace firestore { +namespace model { + +TEST(FieldTransform, Getter) { + FieldTransform transform(testutil::Field("foo"), + absl::make_unique( + ServerTimestampTransform::Get())); + + EXPECT_EQ(testutil::Field("foo"), transform.path()); + EXPECT_EQ(ServerTimestampTransform::Get(), *transform.transform()); +} + +} // namespace model +} // namespace firestore +} // namespace firebase From b4c78daa2f59a4638d64af4f5c27da70bbe6b7e1 Mon Sep 17 00:00:00 2001 From: zxu123 Date: Thu, 5 Apr 2018 11:34:25 -0400 Subject: [PATCH 11/22] port `FieldTransform` --- .../Tests/Remote/FSTSerializerBetaTests.mm | 6 +- Firestore/Example/Tests/Util/FSTHelpers.mm | 14 ++-- Firestore/Source/API/FSTUserDataConverter.h | 18 ++++-- Firestore/Source/API/FSTUserDataConverter.mm | 64 +++++++++++++------ Firestore/Source/Core/FSTTransaction.h | 1 - Firestore/Source/Model/FSTMutation.h | 9 +-- Firestore/Source/Model/FSTMutation.mm | 48 +++++++++----- Firestore/Source/Remote/FSTSerializerBeta.mm | 26 ++++---- .../firestore/model/field_transform.h | 5 +- 9 files changed, 124 insertions(+), 67 deletions(-) diff --git a/Firestore/Example/Tests/Remote/FSTSerializerBetaTests.mm b/Firestore/Example/Tests/Remote/FSTSerializerBetaTests.mm index 17191f8c555..d948a7d2ab9 100644 --- a/Firestore/Example/Tests/Remote/FSTSerializerBetaTests.mm +++ b/Firestore/Example/Tests/Remote/FSTSerializerBetaTests.mm @@ -16,6 +16,8 @@ #import "Firestore/Source/Remote/FSTSerializerBeta.h" +#include + #import #import #import @@ -47,6 +49,7 @@ #include "Firestore/core/src/firebase/firestore/model/database_id.h" #include "Firestore/core/src/firebase/firestore/model/field_mask.h" +#include "Firestore/core/src/firebase/firestore/model/field_transform.h" #include "Firestore/core/src/firebase/firestore/util/string_apple.h" #include "Firestore/core/test/firebase/firestore/testutil/testutil.h" @@ -54,6 +57,7 @@ namespace util = firebase::firestore::util; using firebase::firestore::model::DatabaseId; using firebase::firestore::model::FieldMask; +using firebase::firestore::model::FieldTransform; NS_ASSUME_NONNULL_BEGIN @@ -67,7 +71,7 @@ - (GCFSValue *)encodedDate:(NSDate *)value; - (GCFSDocumentMask *)encodedFieldMask:(const FieldMask &)fieldMask; - (NSMutableArray *)encodedFieldTransforms: - (NSArray *)fieldTransforms; + (const std::vector &)fieldTransforms; - (GCFSStructuredQuery_Filter *)encodedRelationFilter:(FSTRelationFilter *)filter; @end diff --git a/Firestore/Example/Tests/Util/FSTHelpers.mm b/Firestore/Example/Tests/Util/FSTHelpers.mm index cc120a9c2e5..e348cb7fa7c 100644 --- a/Firestore/Example/Tests/Util/FSTHelpers.mm +++ b/Firestore/Example/Tests/Util/FSTHelpers.mm @@ -19,6 +19,7 @@ #include #include #include +#include #include #import @@ -45,6 +46,7 @@ #include "Firestore/core/src/firebase/firestore/model/database_id.h" #include "Firestore/core/src/firebase/firestore/model/document_key.h" #include "Firestore/core/src/firebase/firestore/model/field_mask.h" +#include "Firestore/core/src/firebase/firestore/model/field_transform.h" #include "Firestore/core/src/firebase/firestore/model/field_value.h" #include "Firestore/core/src/firebase/firestore/model/resource_path.h" #include "Firestore/core/src/firebase/firestore/model/transform_operations.h" @@ -58,6 +60,7 @@ using firebase::firestore::model::DocumentKey; using firebase::firestore::model::FieldMask; using firebase::firestore::model::FieldPath; +using firebase::firestore::model::FieldTransform; using firebase::firestore::model::FieldValue; using firebase::firestore::model::ResourcePath; using firebase::firestore::model::ServerTimestampTransform; @@ -278,15 +281,14 @@ NSComparator FSTTestDocComparator(const absl::string_view fieldPath) { FSTTransformMutation *FSTTestTransformMutation(NSString *path, NSArray *serverTimestampFields) { FSTDocumentKey *key = [FSTDocumentKey keyWithPath:testutil::Resource(util::MakeStringView(path))]; - NSMutableArray *fieldTransforms = [NSMutableArray array]; + std::vector fieldTransforms{}; + fieldTransforms.reserve(serverTimestampFields.count); for (NSString *field in serverTimestampFields) { - const FieldPath fieldPath = testutil::Field(util::MakeStringView(field)); + FieldPath fieldPath = testutil::Field(util::MakeStringView(field)); auto transformOp = absl::make_unique(ServerTimestampTransform::Get()); - FSTFieldTransform *transform = - [[FSTFieldTransform alloc] initWithPath:fieldPath transform:std::move(transformOp)]; - [fieldTransforms addObject:transform]; + fieldTransforms.emplace_back(std::move(fieldPath), std::move(transformOp)); } - return [[FSTTransformMutation alloc] initWithKey:key fieldTransforms:fieldTransforms]; + return [[FSTTransformMutation alloc] initWithKey:key fieldTransforms:std::move(fieldTransforms)]; } FSTDeleteMutation *FSTTestDeleteMutation(NSString *path) { diff --git a/Firestore/Source/API/FSTUserDataConverter.h b/Firestore/Source/API/FSTUserDataConverter.h index 2b4b3400419..a2f947a00f4 100644 --- a/Firestore/Source/API/FSTUserDataConverter.h +++ b/Firestore/Source/API/FSTUserDataConverter.h @@ -16,14 +16,16 @@ #import +#include + #include "Firestore/core/src/firebase/firestore/model/database_id.h" #include "Firestore/core/src/firebase/firestore/model/document_key.h" #include "Firestore/core/src/firebase/firestore/model/field_mask.h" +#include "Firestore/core/src/firebase/firestore/model/field_transform.h" @class FIRSetOptions; @class FSTObjectValue; @class FSTFieldValue; -@class FSTFieldTransform; @class FSTMutation; @class FSTPrecondition; @class FSTSnapshotVersion; @@ -36,16 +38,19 @@ NS_ASSUME_NONNULL_BEGIN - (instancetype)init NS_UNAVAILABLE; - (instancetype)initWithData:(FSTObjectValue *)data - fieldTransforms:(NSArray *)fieldTransforms + fieldTransforms: + (std::vector)fieldTransforms NS_DESIGNATED_INITIALIZER; - (instancetype)initWithData:(FSTObjectValue *)data fieldMask:(firebase::firestore::model::FieldMask)fieldMask - fieldTransforms:(NSArray *)fieldTransforms + fieldTransforms: + (std::vector)fieldTransforms NS_DESIGNATED_INITIALIZER; +- (const std::vector &)fieldTransforms; + @property(nonatomic, strong, readonly) FSTObjectValue *data; -@property(nonatomic, strong, readonly) NSArray *fieldTransforms; @property(nonatomic, assign, readonly) BOOL isPatch; /** @@ -64,13 +69,14 @@ NS_ASSUME_NONNULL_BEGIN - (instancetype)initWithData:(FSTObjectValue *)data fieldMask:(firebase::firestore::model::FieldMask)fieldMask - fieldTransforms:(NSArray *)fieldTransforms + fieldTransforms: + (std::vector)fieldTransforms NS_DESIGNATED_INITIALIZER; - (const firebase::firestore::model::FieldMask &)fieldMask; +- (const std::vector &)fieldTransforms; @property(nonatomic, strong, readonly) FSTObjectValue *data; -@property(nonatomic, strong, readonly) NSArray *fieldTransforms; /** * Converts the parsed update data into 1 or 2 mutations (depending on whether there are any diff --git a/Firestore/Source/API/FSTUserDataConverter.mm b/Firestore/Source/API/FSTUserDataConverter.mm index c6a6e3462c2..978ab9ecb22 100644 --- a/Firestore/Source/API/FSTUserDataConverter.mm +++ b/Firestore/Source/API/FSTUserDataConverter.mm @@ -37,6 +37,7 @@ #include "Firestore/core/src/firebase/firestore/model/document_key.h" #include "Firestore/core/src/firebase/firestore/model/field_mask.h" #include "Firestore/core/src/firebase/firestore/model/field_path.h" +#include "Firestore/core/src/firebase/firestore/model/field_transform.h" #include "Firestore/core/src/firebase/firestore/model/transform_operations.h" #include "Firestore/core/src/firebase/firestore/util/string_apple.h" #include "absl/memory/memory.h" @@ -46,7 +47,9 @@ using firebase::firestore::model::DocumentKey; using firebase::firestore::model::FieldMask; using firebase::firestore::model::FieldPath; +using firebase::firestore::model::FieldTransform; using firebase::firestore::model::ServerTimestampTransform; +using firebase::firestore::model::TransformOperation; NS_ASSUME_NONNULL_BEGIN @@ -56,14 +59,15 @@ @implementation FSTParsedSetData { FieldMask _fieldMask; + std::vector _fieldTransforms; } - (instancetype)initWithData:(FSTObjectValue *)data - fieldTransforms:(NSArray *)fieldTransforms { + fieldTransforms:(std::vector)fieldTransforms { self = [super init]; if (self) { _data = data; - _fieldTransforms = fieldTransforms; + _fieldTransforms = std::move(fieldTransforms); _isPatch = NO; } return self; @@ -71,17 +75,21 @@ - (instancetype)initWithData:(FSTObjectValue *)data - (instancetype)initWithData:(FSTObjectValue *)data fieldMask:(FieldMask)fieldMask - fieldTransforms:(NSArray *)fieldTransforms { + fieldTransforms:(std::vector)fieldTransforms { self = [super init]; if (self) { _data = data; _fieldMask = std::move(fieldMask); - _fieldTransforms = fieldTransforms; + _fieldTransforms = std::move(fieldTransforms); _isPatch = YES; } return self; } +- (const std::vector &)fieldTransforms { + return _fieldTransforms; +} + - (NSArray *)mutationsWithKey:(const DocumentKey &)key precondition:(FSTPrecondition *)precondition { NSMutableArray *mutations = [NSMutableArray array]; @@ -95,7 +103,7 @@ - (instancetype)initWithData:(FSTObjectValue *)data value:self.data precondition:precondition]]; } - if (self.fieldTransforms.count > 0) { + if (!self.fieldTransforms.empty()) { [mutations addObject:[[FSTTransformMutation alloc] initWithKey:key fieldTransforms:self.fieldTransforms]]; } @@ -108,16 +116,17 @@ - (instancetype)initWithData:(FSTObjectValue *)data @implementation FSTParsedUpdateData { FieldMask _fieldMask; + std::vector _fieldTransforms; } - (instancetype)initWithData:(FSTObjectValue *)data fieldMask:(FieldMask)fieldMask - fieldTransforms:(NSArray *)fieldTransforms { + fieldTransforms:(std::vector)fieldTransforms { self = [super init]; if (self) { _data = data; _fieldMask = std::move(fieldMask); - _fieldTransforms = fieldTransforms; + _fieldTransforms = std::move(fieldTransforms); } return self; } @@ -129,7 +138,7 @@ - (instancetype)initWithData:(FSTObjectValue *)data fieldMask:self.fieldMask value:self.data precondition:precondition]]; - if (self.fieldTransforms.count > 0) { + if (!self.fieldTransforms.empty()) { [mutations addObject:[[FSTTransformMutation alloc] initWithKey:key fieldTransforms:self.fieldTransforms]]; } @@ -140,6 +149,10 @@ - (instancetype)initWithData:(FSTObjectValue *)data return _fieldMask; } +- (const std::vector &)fieldTransforms { + return _fieldTransforms; +} + @end /** @@ -168,7 +181,6 @@ @interface FSTParseContext : NSObject * conditions apply during parsing and providing better error messages. */ @property(nonatomic, assign) FSTUserDataSource dataSource; -@property(nonatomic, strong, readonly) NSMutableArray *fieldTransforms; - (instancetype)init NS_UNAVAILABLE; /** @@ -186,7 +198,7 @@ - (instancetype)init NS_UNAVAILABLE; - (instancetype)initWithSource:(FSTUserDataSource)dataSource path:(std::unique_ptr)path arrayElement:(BOOL)arrayElement - fieldTransforms:(NSMutableArray *)fieldTransforms + fieldTransforms:(std::vector)fieldTransforms fieldMask:(std::shared_ptr>)fieldMask NS_DESIGNATED_INITIALIZER; @@ -204,6 +216,11 @@ - (const FieldPath *)path; - (void)appendToFieldMaskWithFieldPath:(FieldPath)fieldPath; +- (const std::vector &)fieldTransforms; + +- (void)appendToFieldTransformsWithFieldPath:(FieldPath)fieldPath + transformOperation: + (std::unique_ptr)transformOperation; @end @implementation FSTParseContext { @@ -214,6 +231,7 @@ @implementation FSTParseContext { // the result of calling any of contextForField, contextForFieldPath and contextForArrayIndex // shares the ownership of _fieldMask. std::shared_ptr> _fieldMask; + std::vector _fieldTransforms; } + (instancetype)contextWithSource:(FSTUserDataSource)dataSource @@ -222,7 +240,7 @@ + (instancetype)contextWithSource:(FSTUserDataSource)dataSource [[FSTParseContext alloc] initWithSource:dataSource path:std::move(path) arrayElement:NO - fieldTransforms:[NSMutableArray array] + fieldTransforms:{} fieldMask:std::make_shared>()]; [context validatePath]; return context; @@ -231,13 +249,13 @@ + (instancetype)contextWithSource:(FSTUserDataSource)dataSource - (instancetype)initWithSource:(FSTUserDataSource)dataSource path:(std::unique_ptr)path arrayElement:(BOOL)arrayElement - fieldTransforms:(NSMutableArray *)fieldTransforms + fieldTransforms:(std::vector)fieldTransforms fieldMask:(std::shared_ptr>)fieldMask { if (self = [super init]) { _dataSource = dataSource; _path = std::move(path); _arrayElement = arrayElement; - _fieldTransforms = fieldTransforms; + _fieldTransforms = std::move(fieldTransforms); _fieldMask = std::move(fieldMask); } return self; @@ -335,6 +353,16 @@ - (void)appendToFieldMaskWithFieldPath:(FieldPath)fieldPath { _fieldMask->push_back(std::move(fieldPath)); } +- (const std::vector &)fieldTransforms { + return _fieldTransforms; +} + +- (void)appendToFieldTransformsWithFieldPath:(FieldPath)fieldPath + transformOperation: + (std::unique_ptr)transformOperation { + _fieldTransforms.emplace_back(std::move(fieldPath), std::move(transformOperation)); +} + @end #pragma mark - FSTDocumentKeyReference @@ -462,7 +490,7 @@ - (FSTFieldValue *)parsedQueryValue:(id)input { path:absl::make_unique(FieldPath::EmptyPath())]; FSTFieldValue *_Nullable parsed = [self parseData:input context:context]; FSTAssert(parsed, @"Parsed data should not be nil."); - FSTAssert(context.fieldTransforms.count == 0, @"Field transforms should have been disallowed."); + FSTAssert(context.fieldTransforms.empty(), @"Field transforms should have been disallowed."); return parsed; } @@ -661,11 +689,9 @@ - (nullable FSTFieldValue *)parseScalarValue:(nullable id)input context:(FSTPars @"FieldValue.serverTimestamp() is not currently supported inside arrays%@", [context fieldDescription]); } - [context.fieldTransforms - addObject:[[FSTFieldTransform alloc] - initWithPath:*context.path - transform:absl::make_unique( - ServerTimestampTransform::Get())]]; + [context appendToFieldTransformsWithFieldPath:*context.path + transformOperation:absl::make_unique( + ServerTimestampTransform::Get())]; // Return nil so this value is omitted from the parsed result. return nil; diff --git a/Firestore/Source/Core/FSTTransaction.h b/Firestore/Source/Core/FSTTransaction.h index 01c2e20f069..581a5faacb4 100644 --- a/Firestore/Source/Core/FSTTransaction.h +++ b/Firestore/Source/Core/FSTTransaction.h @@ -24,7 +24,6 @@ @class FIRSetOptions; @class FSTDatastore; -@class FSTFieldTransform; @class FSTMaybeDocument; @class FSTObjectValue; @class FSTParsedSetData; diff --git a/Firestore/Source/Model/FSTMutation.h b/Firestore/Source/Model/FSTMutation.h index 9438ecddaca..38c35bfd720 100644 --- a/Firestore/Source/Model/FSTMutation.h +++ b/Firestore/Source/Model/FSTMutation.h @@ -22,6 +22,7 @@ #include "Firestore/core/src/firebase/firestore/model/document_key.h" #include "Firestore/core/src/firebase/firestore/model/field_mask.h" #include "Firestore/core/src/firebase/firestore/model/field_path.h" +#include "Firestore/core/src/firebase/firestore/model/field_transform.h" #include "Firestore/core/src/firebase/firestore/model/transform_operations.h" @class FSTDocument; @@ -90,7 +91,7 @@ typedef NS_ENUM(NSUInteger, FSTPreconditionExists) { /** * The resulting fields returned from the backend after a FSTTransformMutation has been committed. - * Contains one FieldValue for each FSTFieldTransform that was in the mutation. + * Contains one FieldValue for each FieldTransform that was in the mutation. * * Will be nil if the mutation was not a FSTTransformMutation. */ @@ -271,14 +272,14 @@ typedef NS_ENUM(NSUInteger, FSTPreconditionExists) { * Initializes a new transform mutation with the specified field transforms. * * @param key Identifies the location of the document to mutate. - * @param fieldTransforms A list of FSTFieldTransform objects to perform to the document. + * @param fieldTransforms A list of FieldTransform objects to perform to the document. */ - (instancetype)initWithKey:(firebase::firestore::model::DocumentKey)key - fieldTransforms:(NSArray *)fieldTransforms + fieldTransforms:(std::vector)fieldTransforms NS_DESIGNATED_INITIALIZER; /** The field transforms to use when transforming the document. */ -@property(nonatomic, strong, readonly) NSArray *fieldTransforms; +- (const std::vector &)fieldTransforms; @end diff --git a/Firestore/Source/Model/FSTMutation.mm b/Firestore/Source/Model/FSTMutation.mm index 4ca387cfdfc..7eb52c2c6ae 100644 --- a/Firestore/Source/Model/FSTMutation.mm +++ b/Firestore/Source/Model/FSTMutation.mm @@ -18,6 +18,7 @@ #include #include +#include #import "FIRTimestamp.h" @@ -30,11 +31,13 @@ #include "Firestore/core/src/firebase/firestore/model/document_key.h" #include "Firestore/core/src/firebase/firestore/model/field_mask.h" #include "Firestore/core/src/firebase/firestore/model/field_path.h" +#include "Firestore/core/src/firebase/firestore/model/field_transform.h" #include "Firestore/core/src/firebase/firestore/model/transform_operations.h" using firebase::firestore::model::DocumentKey; using firebase::firestore::model::FieldMask; using firebase::firestore::model::FieldPath; +using firebase::firestore::model::FieldTransform; using firebase::firestore::model::ServerTimestampTransform; using firebase::firestore::model::TransformOperation; @@ -359,20 +362,27 @@ - (FSTObjectValue *)patchObjectValue:(FSTObjectValue *)objectValue { @end -@implementation FSTTransformMutation +@implementation FSTTransformMutation { + /** The field transforms to use when transforming the document. */ + std::vector _fieldTransforms; +} - (instancetype)initWithKey:(DocumentKey)key - fieldTransforms:(NSArray *)fieldTransforms { + fieldTransforms:(std::vector)fieldTransforms { // NOTE: We set a precondition of exists: true as a safety-check, since we always combine // FSTTransformMutations with a FSTSetMutation or FSTPatchMutation which (if successful) should // end up with an existing document. if (self = [super initWithKey:std::move(key) precondition:[FSTPrecondition preconditionWithExists:YES]]) { - _fieldTransforms = fieldTransforms; + _fieldTransforms = std::move(fieldTransforms); } return self; } +- (const std::vector &)fieldTransforms { + return _fieldTransforms; +} + - (BOOL)isEqual:(id)other { if (other == self) { return YES; @@ -383,20 +393,26 @@ - (BOOL)isEqual:(id)other { FSTTransformMutation *otherMutation = (FSTTransformMutation *)other; return [self.key isEqual:otherMutation.key] && - [self.fieldTransforms isEqual:otherMutation.fieldTransforms] && + self.fieldTransforms == otherMutation.fieldTransforms && [self.precondition isEqual:otherMutation.precondition]; } - (NSUInteger)hash { NSUInteger result = [self.key hash]; result = 31 * result + [self.precondition hash]; - result = 31 * result + [self.fieldTransforms hash]; + for (const auto &transform : self.fieldTransforms) { + result = 31 * result + transform.Hash(); + } return result; } - (NSString *)description { - return [NSString stringWithFormat:@"", - self.key.ToString().c_str(), self.fieldTransforms, + std::string fieldTransforms = ""; + for (const auto &transform : self.fieldTransforms) { + fieldTransforms += " " + transform.path().CanonicalString(); + } + return [NSString stringWithFormat:@"", + self.key.ToString().c_str(), fieldTransforms.c_str(), self.precondition]; } @@ -446,19 +462,19 @@ - (nullable FSTMaybeDocument *)applyTo:(nullable FSTMaybeDocument *)maybeDoc (FSTMaybeDocument *_Nullable)baseDocument writeTime:(FIRTimestamp *)localWriteTime { NSMutableArray *transformResults = [NSMutableArray array]; - for (FSTFieldTransform *fieldTransform in self.fieldTransforms) { - if (fieldTransform.transform->type() == TransformOperation::Type::ServerTimestamp) { + for (const FieldTransform &fieldTransform : self.fieldTransforms) { + if (fieldTransform.transform()->type() == TransformOperation::Type::ServerTimestamp) { FSTFieldValue *previousValue = nil; if ([baseDocument isMemberOfClass:[FSTDocument class]]) { - previousValue = [((FSTDocument *)baseDocument) fieldForPath:fieldTransform.path]; + previousValue = [((FSTDocument *)baseDocument) fieldForPath:fieldTransform.path()]; } [transformResults addObject:[FSTServerTimestampValue serverTimestampValueWithLocalWriteTime:localWriteTime previousValue:previousValue]]; } else { - FSTFail(@"Encountered unknown transform: %@", fieldTransform); + FSTFail(@"Encountered unknown transform: %d type", fieldTransform.transform()->type()); } } return transformResults; @@ -466,13 +482,13 @@ - (nullable FSTMaybeDocument *)applyTo:(nullable FSTMaybeDocument *)maybeDoc - (FSTObjectValue *)transformObject:(FSTObjectValue *)objectValue transformResults:(NSArray *)transformResults { - FSTAssert(transformResults.count == self.fieldTransforms.count, + FSTAssert(transformResults.count == self.fieldTransforms.size(), @"Transform results length mismatch."); - for (NSUInteger i = 0; i < self.fieldTransforms.count; i++) { - FSTFieldTransform *fieldTransform = self.fieldTransforms[i]; - const TransformOperation *transform = fieldTransform.transform; - FieldPath fieldPath = fieldTransform.path; + for (NSUInteger i = 0; i < self.fieldTransforms.size(); i++) { + const FieldTransform &fieldTransform = self.fieldTransforms[i]; + const TransformOperation *transform = fieldTransform.transform(); + const FieldPath &fieldPath = fieldTransform.path(); if (transform->type() == TransformOperation::Type::ServerTimestamp) { objectValue = [objectValue objectBySettingValue:transformResults[i] forPath:fieldPath]; } else { diff --git a/Firestore/Source/Remote/FSTSerializerBeta.mm b/Firestore/Source/Remote/FSTSerializerBeta.mm index 5433154b1b8..2d493e15449 100644 --- a/Firestore/Source/Remote/FSTSerializerBeta.mm +++ b/Firestore/Source/Remote/FSTSerializerBeta.mm @@ -17,6 +17,7 @@ #import "Firestore/Source/Remote/FSTSerializerBeta.h" #include +#include #import #import "FIRTimestamp.h" @@ -46,6 +47,7 @@ #include "Firestore/core/src/firebase/firestore/model/document_key.h" #include "Firestore/core/src/firebase/firestore/model/field_mask.h" #include "Firestore/core/src/firebase/firestore/model/field_path.h" +#include "Firestore/core/src/firebase/firestore/model/field_transform.h" #include "Firestore/core/src/firebase/firestore/model/resource_path.h" #include "Firestore/core/src/firebase/firestore/model/transform_operations.h" #include "Firestore/core/src/firebase/firestore/util/string_apple.h" @@ -56,6 +58,7 @@ using firebase::firestore::model::DocumentKey; using firebase::firestore::model::FieldMask; using firebase::firestore::model::FieldPath; +using firebase::firestore::model::FieldTransform; using firebase::firestore::model::ResourcePath; using firebase::firestore::model::ServerTimestampTransform; using firebase::firestore::model::TransformOperation; @@ -563,31 +566,30 @@ - (FieldMask)decodedFieldMask:(GCFSDocumentMask *)fieldMask { } - (NSMutableArray *)encodedFieldTransforms: - (NSArray *)fieldTransforms { + (const std::vector &)fieldTransforms { NSMutableArray *protos = [NSMutableArray array]; - for (FSTFieldTransform *fieldTransform in fieldTransforms) { - FSTAssert(fieldTransform.transform->type() == TransformOperation::Type::ServerTimestamp, - @"Unknown transform: %d type", fieldTransform.transform->type()); + for (const FieldTransform &fieldTransform : fieldTransforms) { + FSTAssert(fieldTransform.transform()->type() == TransformOperation::Type::ServerTimestamp, + @"Unknown transform: %d type", fieldTransform.transform()->type()); GCFSDocumentTransform_FieldTransform *proto = [GCFSDocumentTransform_FieldTransform message]; - proto.fieldPath = util::WrapNSString(fieldTransform.path.CanonicalString()); + proto.fieldPath = util::WrapNSString(fieldTransform.path().CanonicalString()); proto.setToServerValue = GCFSDocumentTransform_FieldTransform_ServerValue_RequestTime; [protos addObject:proto]; } return protos; } -- (NSArray *)decodedFieldTransforms: +- (std::vector)decodedFieldTransforms: (NSArray *)protos { - NSMutableArray *fieldTransforms = [NSMutableArray array]; + std::vector fieldTransforms{}; + fieldTransforms.reserve(protos.count); for (GCFSDocumentTransform_FieldTransform *proto in protos) { FSTAssert( proto.setToServerValue == GCFSDocumentTransform_FieldTransform_ServerValue_RequestTime, @"Unknown transform setToServerValue: %d", proto.setToServerValue); - [fieldTransforms addObject:[[FSTFieldTransform alloc] - initWithPath:FieldPath::FromServerFormat( - util::MakeStringView(proto.fieldPath)) - transform:absl::make_unique( - ServerTimestampTransform::Get())]]; + fieldTransforms.emplace_back( + FieldPath::FromServerFormat(util::MakeStringView(proto.fieldPath)), + absl::make_unique(ServerTimestampTransform::Get())); } return fieldTransforms; } diff --git a/Firestore/core/src/firebase/firestore/model/field_transform.h b/Firestore/core/src/firebase/firestore/model/field_transform.h index 6409c53e0c4..137b885a251 100644 --- a/Firestore/core/src/firebase/firestore/model/field_transform.h +++ b/Firestore/core/src/firebase/firestore/model/field_transform.h @@ -44,7 +44,7 @@ class FieldTransform { #if defined(__OBJC__) bool operator==(const FieldTransform& other) const { - return path_ == other.path && *self.transform == *other.transform; + return path_ == other.path_ && *transform_ == *other.transform_; } // For Objective-C++ hash; to be removed after migration. @@ -58,7 +58,8 @@ class FieldTransform { private: FieldPath path_; - std::unique_ptr transform_; + // Shared by copies of the same FieldTransform. + std::shared_ptr transform_; }; } // namespace model From 78a8bb9f2f1e97e9dd4df8d2127c8791019ac554 Mon Sep 17 00:00:00 2001 From: zxu123 Date: Thu, 5 Apr 2018 13:50:18 -0400 Subject: [PATCH 12/22] make `fieldTransforms` shared inside `context` --- Firestore/Source/API/FSTUserDataConverter.mm | 37 ++++++++++---------- 1 file changed, 19 insertions(+), 18 deletions(-) diff --git a/Firestore/Source/API/FSTUserDataConverter.mm b/Firestore/Source/API/FSTUserDataConverter.mm index 978ab9ecb22..2eee8781938 100644 --- a/Firestore/Source/API/FSTUserDataConverter.mm +++ b/Firestore/Source/API/FSTUserDataConverter.mm @@ -198,7 +198,7 @@ - (instancetype)init NS_UNAVAILABLE; - (instancetype)initWithSource:(FSTUserDataSource)dataSource path:(std::unique_ptr)path arrayElement:(BOOL)arrayElement - fieldTransforms:(std::vector)fieldTransforms + fieldTransforms:(std::shared_ptr>)fieldTransforms fieldMask:(std::shared_ptr>)fieldMask NS_DESIGNATED_INITIALIZER; @@ -216,7 +216,7 @@ - (const FieldPath *)path; - (void)appendToFieldMaskWithFieldPath:(FieldPath)fieldPath; -- (const std::vector &)fieldTransforms; +- (const std::vector *)fieldTransforms; - (void)appendToFieldTransformsWithFieldPath:(FieldPath)fieldPath transformOperation: @@ -227,11 +227,11 @@ @implementation FSTParseContext { /** The current path being parsed. */ // TODO(b/34871131): path should never be nullptr, but we don't support array paths right now. std::unique_ptr _path; - // _fieldMask is shared across all active context objects to accumulate the result. For example, - // the result of calling any of contextForField, contextForFieldPath and contextForArrayIndex - // shares the ownership of _fieldMask. + // _fieldMask and _fieldTransforms are shared across all active context objects to accumulate the + // result. For example, the result of calling any of contextForField, contextForFieldPath and + // contextForArrayIndex shares the ownership of _fieldMask and _fieldTransforms. std::shared_ptr> _fieldMask; - std::vector _fieldTransforms; + std::shared_ptr> _fieldTransforms; } + (instancetype)contextWithSource:(FSTUserDataSource)dataSource @@ -240,7 +240,7 @@ + (instancetype)contextWithSource:(FSTUserDataSource)dataSource [[FSTParseContext alloc] initWithSource:dataSource path:std::move(path) arrayElement:NO - fieldTransforms:{} + fieldTransforms:std::make_shared>() fieldMask:std::make_shared>()]; [context validatePath]; return context; @@ -249,7 +249,7 @@ + (instancetype)contextWithSource:(FSTUserDataSource)dataSource - (instancetype)initWithSource:(FSTUserDataSource)dataSource path:(std::unique_ptr)path arrayElement:(BOOL)arrayElement - fieldTransforms:(std::vector)fieldTransforms + fieldTransforms:(std::shared_ptr>)fieldTransforms fieldMask:(std::shared_ptr>)fieldMask { if (self = [super init]) { _dataSource = dataSource; @@ -269,7 +269,7 @@ - (instancetype)contextForField:(NSString *)fieldName { FSTParseContext *context = [[FSTParseContext alloc] initWithSource:self.dataSource path:std::move(path) arrayElement:NO - fieldTransforms:self.fieldTransforms + fieldTransforms:_fieldTransforms fieldMask:_fieldMask]; [context validatePathSegment:fieldName]; return context; @@ -283,7 +283,7 @@ - (instancetype)contextForFieldPath:(const FieldPath &)fieldPath { FSTParseContext *context = [[FSTParseContext alloc] initWithSource:self.dataSource path:std::move(path) arrayElement:NO - fieldTransforms:self.fieldTransforms + fieldTransforms:_fieldTransforms fieldMask:_fieldMask]; [context validatePath]; return context; @@ -294,7 +294,7 @@ - (instancetype)contextForArrayIndex:(NSUInteger)index { return [[FSTParseContext alloc] initWithSource:self.dataSource path:nil arrayElement:YES - fieldTransforms:self.fieldTransforms + fieldTransforms:_fieldTransforms fieldMask:_fieldMask]; } @@ -353,14 +353,14 @@ - (void)appendToFieldMaskWithFieldPath:(FieldPath)fieldPath { _fieldMask->push_back(std::move(fieldPath)); } -- (const std::vector &)fieldTransforms { - return _fieldTransforms; +- (const std::vector *)fieldTransforms { + return _fieldTransforms.get(); } - (void)appendToFieldTransformsWithFieldPath:(FieldPath)fieldPath transformOperation: (std::unique_ptr)transformOperation { - _fieldTransforms.emplace_back(std::move(fieldPath), std::move(transformOperation)); + _fieldTransforms->emplace_back(std::move(fieldPath), std::move(transformOperation)); } @end @@ -420,7 +420,7 @@ - (FSTParsedSetData *)parsedMergeData:(id)input { return [[FSTParsedSetData alloc] initWithData:updateData fieldMask:FieldMask{*context.fieldMask} - fieldTransforms:context.fieldTransforms]; + fieldTransforms:*context.fieldTransforms]; } - (FSTParsedSetData *)parsedSetData:(id)input { @@ -435,7 +435,8 @@ - (FSTParsedSetData *)parsedSetData:(id)input { path:absl::make_unique(FieldPath::EmptyPath())]; FSTObjectValue *updateData = (FSTObjectValue *)[self parseData:input context:context]; - return [[FSTParsedSetData alloc] initWithData:updateData fieldTransforms:context.fieldTransforms]; + return + [[FSTParsedSetData alloc] initWithData:updateData fieldTransforms:*context.fieldTransforms]; } - (FSTParsedUpdateData *)parsedUpdateData:(id)input { @@ -481,7 +482,7 @@ - (FSTParsedUpdateData *)parsedUpdateData:(id)input { return [[FSTParsedUpdateData alloc] initWithData:updateData fieldMask:FieldMask{fieldMaskPaths} - fieldTransforms:context.fieldTransforms]; + fieldTransforms:*context.fieldTransforms]; } - (FSTFieldValue *)parsedQueryValue:(id)input { @@ -490,7 +491,7 @@ - (FSTFieldValue *)parsedQueryValue:(id)input { path:absl::make_unique(FieldPath::EmptyPath())]; FSTFieldValue *_Nullable parsed = [self parseData:input context:context]; FSTAssert(parsed, @"Parsed data should not be nil."); - FSTAssert(context.fieldTransforms.empty(), @"Field transforms should have been disallowed."); + FSTAssert(context.fieldTransforms->empty(), @"Field transforms should have been disallowed."); return parsed; } From f7bcff71489a9bebef92a7892edbff6e5a24e816 Mon Sep 17 00:00:00 2001 From: zxu123 Date: Thu, 5 Apr 2018 16:37:34 -0400 Subject: [PATCH 13/22] Implement Precondition in C++ w/o test yet --- Firestore/Source/Model/FSTMutation.h | 43 ------- Firestore/Source/Model/FSTMutation.mm | 98 --------------- .../firebase/firestore/model/CMakeLists.txt | 2 + .../firebase/firestore/model/precondition.cc | 82 ++++++++++++ .../firebase/firestore/model/precondition.h | 117 ++++++++++++++++++ .../firebase/firestore/model/CMakeLists.txt | 1 + .../firestore/model/precondition_test.cc | 37 ++++++ 7 files changed, 239 insertions(+), 141 deletions(-) create mode 100644 Firestore/core/src/firebase/firestore/model/precondition.cc create mode 100644 Firestore/core/src/firebase/firestore/model/precondition.h create mode 100644 Firestore/core/test/firebase/firestore/model/precondition_test.cc diff --git a/Firestore/Source/Model/FSTMutation.h b/Firestore/Source/Model/FSTMutation.h index 38c35bfd720..97d3e12f104 100644 --- a/Firestore/Source/Model/FSTMutation.h +++ b/Firestore/Source/Model/FSTMutation.h @@ -34,49 +34,6 @@ NS_ASSUME_NONNULL_BEGIN -#pragma mark - FSTPrecondition - -typedef NS_ENUM(NSUInteger, FSTPreconditionExists) { - FSTPreconditionExistsNotSet, - FSTPreconditionExistsYes, - FSTPreconditionExistsNo, -}; - -/** - * Encodes a precondition for a mutation. This follows the model that the backend accepts with the - * special case of an explicit "empty" precondition (meaning no precondition). - */ -@interface FSTPrecondition : NSObject - -/** Creates a new FSTPrecondition with an exists flag. */ -+ (FSTPrecondition *)preconditionWithExists:(BOOL)exists; - -/** Creates a new FSTPrecondition based on a time the document exists at. */ -+ (FSTPrecondition *)preconditionWithUpdateTime:(FSTSnapshotVersion *)updateTime; - -/** Returns a precondition representing no precondition. */ -+ (FSTPrecondition *)none; - -/** - * Returns true if the preconditions is valid for the given document (or null if no document is - * available). - */ -- (BOOL)isValidForDocument:(FSTMaybeDocument *_Nullable)maybeDoc; - -/** Returns whether this Precondition represents no precondition. */ -- (BOOL)isNone; - -/** If set, preconditions a mutation based on the last updateTime. */ -@property(nonatomic, strong, readonly, nullable) FSTSnapshotVersion *updateTime; - -/** - * If set, preconditions a mutation based on whether the document exists. - * Uses FSTPreconditionExistsNotSet to mark as unset. - */ -@property(nonatomic, assign, readonly) FSTPreconditionExists exists; - -@end - #pragma mark - FSTMutationResult @interface FSTMutationResult : NSObject diff --git a/Firestore/Source/Model/FSTMutation.mm b/Firestore/Source/Model/FSTMutation.mm index 7eb52c2c6ae..3e5ec1a65a9 100644 --- a/Firestore/Source/Model/FSTMutation.mm +++ b/Firestore/Source/Model/FSTMutation.mm @@ -43,104 +43,6 @@ NS_ASSUME_NONNULL_BEGIN -#pragma mark - FSTPrecondition - -@implementation FSTPrecondition - -+ (FSTPrecondition *)preconditionWithExists:(BOOL)exists { - FSTPreconditionExists existsEnum = exists ? FSTPreconditionExistsYes : FSTPreconditionExistsNo; - return [[FSTPrecondition alloc] initWithUpdateTime:nil exists:existsEnum]; -} - -+ (FSTPrecondition *)preconditionWithUpdateTime:(FSTSnapshotVersion *)updateTime { - return [[FSTPrecondition alloc] initWithUpdateTime:updateTime exists:FSTPreconditionExistsNotSet]; -} - -+ (FSTPrecondition *)none { - static dispatch_once_t onceToken; - static FSTPrecondition *noPrecondition; - dispatch_once(&onceToken, ^{ - noPrecondition = - [[FSTPrecondition alloc] initWithUpdateTime:nil exists:FSTPreconditionExistsNotSet]; - }); - return noPrecondition; -} - -- (instancetype)initWithUpdateTime:(FSTSnapshotVersion *_Nullable)updateTime - exists:(FSTPreconditionExists)exists { - if (self = [super init]) { - _updateTime = updateTime; - _exists = exists; - } - return self; -} - -- (BOOL)isValidForDocument:(FSTMaybeDocument *_Nullable)maybeDoc { - if (self.updateTime) { - return - [maybeDoc isKindOfClass:[FSTDocument class]] && [maybeDoc.version isEqual:self.updateTime]; - } else if (self.exists != FSTPreconditionExistsNotSet) { - if (self.exists == FSTPreconditionExistsYes) { - return [maybeDoc isKindOfClass:[FSTDocument class]]; - } else { - FSTAssert(self.exists == FSTPreconditionExistsNo, @"Invalid precondition"); - return maybeDoc == nil || [maybeDoc isKindOfClass:[FSTDeletedDocument class]]; - } - } else { - FSTAssert(self.isNone, @"Precondition should be empty"); - return YES; - } -} - -- (BOOL)isNone { - return self.updateTime == nil && self.exists == FSTPreconditionExistsNotSet; -} - -- (BOOL)isEqual:(id)other { - if (self == other) { - return YES; - } - - if (![other isKindOfClass:[FSTPrecondition class]]) { - return NO; - } - - FSTPrecondition *otherPrecondition = (FSTPrecondition *)other; - // Compare references to cover nil equality - return (self.updateTime == otherPrecondition.updateTime || - [self.updateTime isEqual:otherPrecondition.updateTime]) && - self.exists == otherPrecondition.exists; -} - -- (NSUInteger)hash { - NSUInteger hash = [self.updateTime hash]; - hash = hash * 31 + self.exists; - return hash; -} - -- (NSString *)description { - if (self.isNone) { - return @">"; - } else { - NSString *existsString; - switch (self.exists) { - case FSTPreconditionExistsYes: - existsString = @"yes"; - break; - case FSTPreconditionExistsNo: - existsString = @"no"; - break; - default: - existsString = @""; - break; - } - return [NSString stringWithFormat:@"", self.updateTime, - existsString]; - } -} - -@end - #pragma mark - FSTMutationResult @implementation FSTMutationResult diff --git a/Firestore/core/src/firebase/firestore/model/CMakeLists.txt b/Firestore/core/src/firebase/firestore/model/CMakeLists.txt index de783ad80de..02affdb9e4c 100644 --- a/Firestore/core/src/firebase/firestore/model/CMakeLists.txt +++ b/Firestore/core/src/firebase/firestore/model/CMakeLists.txt @@ -32,6 +32,8 @@ cc_library( maybe_document.h no_document.cc no_document.h + precondition.cc + precondition.h resource_path.cc resource_path.h snapshot_version.cc diff --git a/Firestore/core/src/firebase/firestore/model/precondition.cc b/Firestore/core/src/firebase/firestore/model/precondition.cc new file mode 100644 index 00000000000..35556a8a952 --- /dev/null +++ b/Firestore/core/src/firebase/firestore/model/precondition.cc @@ -0,0 +1,82 @@ +/* + * Copyright 2018 Google + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include "Firestore/core/src/firebase/firestore/model/precondition.h" + +#include + +#include "Firestore/core/src/firebase/firestore/model/maybe_document.h" +#include "Firestore/core/src/firebase/firestore/model/snapshot_version.h" +#include "Firestore/core/src/firebase/firestore/util/firebase_assert.h" + +namespace firebase { +namespace firestore { +namespace model { + +Precondition::Precondition(Type type, SnapshotVersion update_time, bool exists) + : type_(type), update_time_(std::move(update_time)), exists_(exists) { +} + +/* static */ +const Precondition& Precondition::Exists(bool exists) { + static Precondition kExist{Type::Exists, SnapshotVersion::None(), true}; + static Precondition kNotExist{Type::Exists, SnapshotVersion::None(), false}; + return exists ? kExist : kNotExist; +} + +/* static */ +Precondition Precondition::UpdateTime(SnapshotVersion update_time) { + FIREBASE_ASSERT_MESSAGE(update_time != SnapshotVersion::None(), + "Invalid update time"); + return Precondition{Type::UpdateTime, std::move(update_time), false}; +} + +/* static */ +const Precondition& Precondition::None() { + static Precondition kNoPrecondition{Type::None, SnapshotVersion::None(), + false}; + return kNoPrecondition; +} + +bool Precondition::IsValidFor(const MaybeDocument& maybe_doc) const { + switch (type_) { + case Type::UpdateTime: + return maybe_doc.type() == MaybeDocument::Type::Document && + maybe_doc.version() == update_time_; + break; + case Type::Exists: + if (exists_) { + return maybe_doc.type() == MaybeDocument::Type::Document; + } else { + return maybe_doc.type() == MaybeDocument::Type::NoDocument; + } + break; + case Type::None: + FIREBASE_ASSERT_MESSAGE(IsNone(), "Precondition should be empty"); + return true; + default: + FIREBASE_ASSERT_MESSAGE(false, "Invalid precondition"); + break; + } +} + +bool Precondition::IsNone() const { + return type_ == Type::None; +} + +} // namespace model +} // namespace firestore +} // namespace firebase diff --git a/Firestore/core/src/firebase/firestore/model/precondition.h b/Firestore/core/src/firebase/firestore/model/precondition.h new file mode 100644 index 00000000000..7c049138737 --- /dev/null +++ b/Firestore/core/src/firebase/firestore/model/precondition.h @@ -0,0 +1,117 @@ +/* + * Copyright 2018 Google + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#ifndef FIRESTORE_CORE_SRC_FIREBASE_FIRESTORE_MODEL_PRECONDITION_H_ +#define FIRESTORE_CORE_SRC_FIREBASE_FIRESTORE_MODEL_PRECONDITION_H_ + +#include +#include + +#include "Firestore/core/src/firebase/firestore/model/maybe_document.h" +#include "Firestore/core/src/firebase/firestore/model/snapshot_version.h" + +namespace firebase { +namespace firestore { +namespace model { + +/** + * Encodes a precondition for a mutation. This follows the model that the + * backend accepts with the special case of an explicit "empty" precondition + * (meaning no precondition). + */ +class Precondition { + public: + enum class Type { + None, + Exists, + UpdateTime, + }; + + /** Creates a new Precondition with an exists flag. */ + static const Precondition& Exists(bool exists); + + /** Creates a new Precondition based on a time the document exists at. */ + static Precondition UpdateTime(SnapshotVersion update_time); + + /** Returns a precondition representing no precondition. */ + static const Precondition& None(); + + /** + * Returns true if the preconditions is valid for the given document (or null + * if no document is available). + */ + bool IsValidFor(const MaybeDocument& maybe_doc) const; + + /** Returns whether this Precondition represents no precondition. */ + bool IsNone() const; + +#if defined(__OBJC__) + bool operator==(const Precondition& other) const { + return update_time_ == other.update_time_ && exists_ == other.exists_; + } + + // For Objective-C++ hash; to be removed after migration. + // Do NOT use in C++ code. + NSUInteger Hash() const { + NSUInteger hash = update_time_.Hash(); + hash = hash * 31 + other.exists_; + return hash; + } + + NSString* description const { + switch (type_) { + case Type::None: + return @">"; + break; + case Type::Exists: + if (exists_) { + return @""; + } else { + return @""; + } + break; + case Type::UpdateTime: + return [NSString stringWithFormat:@"", + update_time_.ToString().c_str()]; + break; + default: + // We do not raise assertion here. This function is mainly used in + // logging. + return @""; + break; + } + } +#endif // defined(__OBJC__) + + private: + Precondition(Type type, SnapshotVersion update_time, bool exists); + + // The actual time of this precondition. + Type type_; + + // For UpdateTime type, preconditions a mutation based on the last updateTime. + SnapshotVersion update_time_; + + // For Exists type, preconditions a mutation based on whether the document + // exists. + bool exists_; +}; + +} // namespace model +} // namespace firestore +} // namespace firebase + +#endif // FIRESTORE_CORE_SRC_FIREBASE_FIRESTORE_MODEL_PRECONDITION_H_ diff --git a/Firestore/core/test/firebase/firestore/model/CMakeLists.txt b/Firestore/core/test/firebase/firestore/model/CMakeLists.txt index 3bac89d4124..9c94677e00d 100644 --- a/Firestore/core/test/firebase/firestore/model/CMakeLists.txt +++ b/Firestore/core/test/firebase/firestore/model/CMakeLists.txt @@ -24,6 +24,7 @@ cc_test( field_value_test.cc maybe_document_test.cc no_document_test.cc + precondition_test.cc resource_path_test.cc snapshot_version_test.cc transform_operations_test.cc diff --git a/Firestore/core/test/firebase/firestore/model/precondition_test.cc b/Firestore/core/test/firebase/firestore/model/precondition_test.cc new file mode 100644 index 00000000000..8fded1a0f1c --- /dev/null +++ b/Firestore/core/test/firebase/firestore/model/precondition_test.cc @@ -0,0 +1,37 @@ +/* + * Copyright 2018 Google + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include "Firestore/core/src/firebase/firestore/model/precondition.h" +#include "Firestore/core/test/firebase/firestore/testutil/testutil.h" + +#include "gtest/gtest.h" + +namespace firebase { +namespace firestore { +namespace model { + +TEST(Precondition, None) { +} + +TEST(Precondition, Exists) { +} + +TEST(Precondition, UpdateTime) { +} + +} // namespace model +} // namespace firestore +} // namespace firebase From 07d297c3ed84454873184aee486c506a29f5a055 Mon Sep 17 00:00:00 2001 From: zxu123 Date: Fri, 6 Apr 2018 12:06:19 -0400 Subject: [PATCH 14/22] add unit test for `Precondition` --- .../firebase/firestore/model/precondition.cc | 1 + .../firestore/model/precondition_test.cc | 31 +++++++++++++++++++ .../firebase/firestore/testutil/testutil.h | 20 ++++++++++++ 3 files changed, 52 insertions(+) diff --git a/Firestore/core/src/firebase/firestore/model/precondition.cc b/Firestore/core/src/firebase/firestore/model/precondition.cc index 35556a8a952..648d1dfa09a 100644 --- a/Firestore/core/src/firebase/firestore/model/precondition.cc +++ b/Firestore/core/src/firebase/firestore/model/precondition.cc @@ -67,6 +67,7 @@ bool Precondition::IsValidFor(const MaybeDocument& maybe_doc) const { case Type::None: FIREBASE_ASSERT_MESSAGE(IsNone(), "Precondition should be empty"); return true; + break; default: FIREBASE_ASSERT_MESSAGE(false, "Invalid precondition"); break; diff --git a/Firestore/core/test/firebase/firestore/model/precondition_test.cc b/Firestore/core/test/firebase/firestore/model/precondition_test.cc index 8fded1a0f1c..5a8411bb43e 100644 --- a/Firestore/core/test/firebase/firestore/model/precondition_test.cc +++ b/Firestore/core/test/firebase/firestore/model/precondition_test.cc @@ -15,6 +15,9 @@ */ #include "Firestore/core/src/firebase/firestore/model/precondition.h" + +#include "Firestore/core/src/firebase/firestore/model/document.h" +#include "Firestore/core/src/firebase/firestore/model/no_document.h" #include "Firestore/core/test/firebase/firestore/testutil/testutil.h" #include "gtest/gtest.h" @@ -24,12 +27,40 @@ namespace firestore { namespace model { TEST(Precondition, None) { + const Precondition none = Precondition::None(); + EXPECT_TRUE(none.IsNone()); + + const NoDocument deleted_doc = testutil::DeletedDoc("foo/doc", 1234567); + const Document doc = testutil::Doc("bar/doc", 7654321); + EXPECT_TRUE(none.IsValidFor(deleted_doc)); + EXPECT_TRUE(none.IsValidFor(doc)); } TEST(Precondition, Exists) { + const Precondition exists = Precondition::Exists(true); + const Precondition no_exists = Precondition::Exists(false); + EXPECT_FALSE(exists.IsNone()); + EXPECT_FALSE(no_exists.IsNone()); + + const NoDocument deleted_doc = testutil::DeletedDoc("foo/doc", 1234567); + const Document doc = testutil::Doc("bar/doc", 7654321); + EXPECT_FALSE(exists.IsValidFor(deleted_doc)); + EXPECT_TRUE(exists.IsValidFor(doc)); + EXPECT_TRUE(no_exists.IsValidFor(deleted_doc)); + EXPECT_FALSE(no_exists.IsValidFor(doc)); } TEST(Precondition, UpdateTime) { + const Precondition update_time = + Precondition::UpdateTime(testutil::Version(1234567)); + EXPECT_FALSE(update_time.IsNone()); + + const NoDocument deleted_doc = testutil::DeletedDoc("foo/doc", 1234567); + const Document not_match = testutil::Doc("bar/doc", 7654321); + const Document match = testutil::Doc("baz/doc", 1234567); + EXPECT_FALSE(update_time.IsValidFor(deleted_doc)); + EXPECT_FALSE(update_time.IsValidFor(not_match)); + EXPECT_TRUE(update_time.IsValidFor(match)); } } // namespace model diff --git a/Firestore/core/test/firebase/firestore/testutil/testutil.h b/Firestore/core/test/firebase/firestore/testutil/testutil.h index 9c697844b7b..db46662222d 100644 --- a/Firestore/core/test/firebase/firestore/testutil/testutil.h +++ b/Firestore/core/test/firebase/firestore/testutil/testutil.h @@ -17,9 +17,15 @@ #ifndef FIRESTORE_CORE_TEST_FIREBASE_FIRESTORE_TESTUTIL_TESTUTIL_H_ #define FIRESTORE_CORE_TEST_FIREBASE_FIRESTORE_TESTUTIL_TESTUTIL_H_ +#include + +#include "Firestore/core/include/firebase/firestore/timestamp.h" +#include "Firestore/core/src/firebase/firestore/model/document.h" #include "Firestore/core/src/firebase/firestore/model/document_key.h" #include "Firestore/core/src/firebase/firestore/model/field_path.h" +#include "Firestore/core/src/firebase/firestore/model/no_document.h" #include "Firestore/core/src/firebase/firestore/model/resource_path.h" +#include "Firestore/core/src/firebase/firestore/model/snapshot_version.h" #include "absl/strings/string_view.h" namespace firebase { @@ -40,6 +46,20 @@ inline model::ResourcePath Resource(absl::string_view field) { return model::ResourcePath::FromString(field); } +inline model::SnapshotVersion Version(time_t version) { + return model::SnapshotVersion{Timestamp::FromTimeT(version)}; +} + +inline model::Document Doc(absl::string_view key, time_t version) { + return model::Document{model::FieldValue::ObjectValueFromMap({}), Key(key), + Version(version), + /* has_local_mutations= */ false}; +} + +inline model::NoDocument DeletedDoc(absl::string_view key, time_t version) { + return model::NoDocument{Key(key), Version(version)}; +} + // Add a non-inline function to make this library buildable. // TODO(zxu123): remove once there is non-inline function. void dummy(); From d4df4b3ed741950d99060bdfa5e266b97f034bc8 Mon Sep 17 00:00:00 2001 From: zxu123 Date: Fri, 6 Apr 2018 14:56:30 -0400 Subject: [PATCH 15/22] port `Precondition` --- .../Tests/Integration/FSTDatastoreTests.mm | 4 +- .../Tests/Local/FSTLocalSerializerTests.mm | 15 ++--- .../Example/Tests/Model/FSTMutationTests.mm | 4 +- .../Tests/Remote/FSTSerializerBetaTests.mm | 14 +++-- Firestore/Example/Tests/Util/FSTHelpers.mm | 10 ++-- Firestore/Source/API/FIRDocumentReference.mm | 9 +-- Firestore/Source/API/FIRWriteBatch.mm | 15 +++-- Firestore/Source/API/FSTUserDataConverter.h | 8 ++- Firestore/Source/API/FSTUserDataConverter.mm | 6 +- Firestore/Source/Core/FSTTransaction.mm | 26 +++++---- Firestore/Source/Model/FSTMutation.h | 21 ++++--- Firestore/Source/Model/FSTMutation.mm | 56 ++++++++++--------- Firestore/Source/Remote/FSTSerializerBeta.mm | 39 +++++++------ .../firebase/firestore/model/precondition.h | 56 +++++++++++++++++-- .../firestore/model/snapshot_version.h | 28 ++++++++++ .../firestore/model/precondition_test.cc | 5 ++ .../firebase/firestore/testutil/testutil.h | 14 +++-- 17 files changed, 221 insertions(+), 109 deletions(-) diff --git a/Firestore/Example/Tests/Integration/FSTDatastoreTests.mm b/Firestore/Example/Tests/Integration/FSTDatastoreTests.mm index 430366f1b93..ad911ce32cb 100644 --- a/Firestore/Example/Tests/Integration/FSTDatastoreTests.mm +++ b/Firestore/Example/Tests/Integration/FSTDatastoreTests.mm @@ -42,12 +42,14 @@ #include "Firestore/core/src/firebase/firestore/auth/empty_credentials_provider.h" #include "Firestore/core/src/firebase/firestore/core/database_info.h" #include "Firestore/core/src/firebase/firestore/model/database_id.h" +#include "Firestore/core/src/firebase/firestore/model/precondition.h" #include "Firestore/core/src/firebase/firestore/util/string_apple.h" namespace util = firebase::firestore::util; using firebase::firestore::auth::EmptyCredentialsProvider; using firebase::firestore::core::DatabaseInfo; using firebase::firestore::model::DatabaseId; +using firebase::firestore::model::Precondition; using firebase::firestore::model::TargetId; NS_ASSUME_NONNULL_BEGIN @@ -238,7 +240,7 @@ - (FSTSetMutation *)setMutation { initWithKey:[FSTDocumentKey keyWithPathString:@"rooms/eros"] value:[[FSTObjectValue alloc] initWithDictionary:@{@"name" : [FSTStringValue stringValue:@"Eros"]}] - precondition:[FSTPrecondition none]]; + precondition:Precondition::None()]; } @end diff --git a/Firestore/Example/Tests/Local/FSTLocalSerializerTests.mm b/Firestore/Example/Tests/Local/FSTLocalSerializerTests.mm index d810aa62514..362f46fa692 100644 --- a/Firestore/Example/Tests/Local/FSTLocalSerializerTests.mm +++ b/Firestore/Example/Tests/Local/FSTLocalSerializerTests.mm @@ -42,12 +42,14 @@ #include "Firestore/core/src/firebase/firestore/model/database_id.h" #include "Firestore/core/src/firebase/firestore/model/field_mask.h" +#include "Firestore/core/src/firebase/firestore/model/precondition.h" #include "Firestore/core/src/firebase/firestore/util/string_apple.h" #include "Firestore/core/test/firebase/firestore/testutil/testutil.h" namespace testutil = firebase::firestore::testutil; using firebase::firestore::model::DatabaseId; using firebase::firestore::model::FieldMask; +using firebase::firestore::model::Precondition; NS_ASSUME_NONNULL_BEGIN @@ -78,13 +80,12 @@ - (void)setUp { - (void)testEncodesMutationBatch { FSTMutation *set = FSTTestSetMutation(@"foo/bar", @{ @"a" : @"b", @"num" : @1 }); - FSTMutation *patch = - [[FSTPatchMutation alloc] initWithKey:FSTTestDocKey(@"bar/baz") - fieldMask:FieldMask{testutil::Field("a")} - value:FSTTestObjectValue( - @{ @"a" : @"b", - @"num" : @1 }) - precondition:[FSTPrecondition preconditionWithExists:YES]]; + FSTMutation *patch = [[FSTPatchMutation alloc] initWithKey:FSTTestDocKey(@"bar/baz") + fieldMask:FieldMask{testutil::Field("a")} + value:FSTTestObjectValue( + @{ @"a" : @"b", + @"num" : @1 }) + precondition:Precondition::Exists(true)]; FSTMutation *del = FSTTestDeleteMutation(@"baz/quux"); FIRTimestamp *writeTime = [FIRTimestamp timestamp]; FSTMutationBatch *model = [[FSTMutationBatch alloc] initWithBatchID:42 diff --git a/Firestore/Example/Tests/Model/FSTMutationTests.mm b/Firestore/Example/Tests/Model/FSTMutationTests.mm index 9024b226ac0..60cb7b80ae6 100644 --- a/Firestore/Example/Tests/Model/FSTMutationTests.mm +++ b/Firestore/Example/Tests/Model/FSTMutationTests.mm @@ -26,11 +26,13 @@ #include "Firestore/core/src/firebase/firestore/model/document_key.h" #include "Firestore/core/src/firebase/firestore/model/field_mask.h" +#include "Firestore/core/src/firebase/firestore/model/precondition.h" #include "Firestore/core/test/firebase/firestore/testutil/testutil.h" namespace testutil = firebase::firestore::testutil; using firebase::firestore::model::DocumentKey; using firebase::firestore::model::FieldMask; +using firebase::firestore::model::Precondition; @interface FSTMutationTests : XCTestCase @end @@ -74,7 +76,7 @@ - (void)testDeletesValuesFromTheFieldMask { FSTMutation *patch = [[FSTPatchMutation alloc] initWithKey:key fieldMask:{testutil::Field("foo.bar")} value:[FSTObjectValue objectValue] - precondition:[FSTPrecondition none]]; + precondition:Precondition::None()]; FSTMaybeDocument *patchedDoc = [patch applyTo:baseDoc baseDocument:baseDoc localWriteTime:_timestamp]; diff --git a/Firestore/Example/Tests/Remote/FSTSerializerBetaTests.mm b/Firestore/Example/Tests/Remote/FSTSerializerBetaTests.mm index d948a7d2ab9..ae23f292d29 100644 --- a/Firestore/Example/Tests/Remote/FSTSerializerBetaTests.mm +++ b/Firestore/Example/Tests/Remote/FSTSerializerBetaTests.mm @@ -50,6 +50,7 @@ #include "Firestore/core/src/firebase/firestore/model/database_id.h" #include "Firestore/core/src/firebase/firestore/model/field_mask.h" #include "Firestore/core/src/firebase/firestore/model/field_transform.h" +#include "Firestore/core/src/firebase/firestore/model/precondition.h" #include "Firestore/core/src/firebase/firestore/util/string_apple.h" #include "Firestore/core/test/firebase/firestore/testutil/testutil.h" @@ -58,6 +59,7 @@ using firebase::firestore::model::DatabaseId; using firebase::firestore::model::FieldMask; using firebase::firestore::model::FieldTransform; +using firebase::firestore::model::Precondition; NS_ASSUME_NONNULL_BEGIN @@ -377,12 +379,12 @@ - (void)testEncodesTransformMutation { } - (void)testEncodesSetMutationWithPrecondition { - FSTSetMutation *mutation = [[FSTSetMutation alloc] - initWithKey:FSTTestDocKey(@"foo/bar") - value:FSTTestObjectValue( - @{ @"a" : @"b", - @"num" : @1 }) - precondition:[FSTPrecondition preconditionWithUpdateTime:FSTTestVersion(4)]]; + FSTSetMutation *mutation = + [[FSTSetMutation alloc] initWithKey:FSTTestDocKey(@"foo/bar") + value:FSTTestObjectValue( + @{ @"a" : @"b", + @"num" : @1 }) + precondition:Precondition::UpdateTime(testutil::Version(4))]; GCFSWrite *proto = [GCFSWrite message]; proto.update = [self.serializer encodedDocumentWithFields:mutation.value key:mutation.key]; proto.currentDocument.updateTime = diff --git a/Firestore/Example/Tests/Util/FSTHelpers.mm b/Firestore/Example/Tests/Util/FSTHelpers.mm index e348cb7fa7c..7a27e5982ea 100644 --- a/Firestore/Example/Tests/Util/FSTHelpers.mm +++ b/Firestore/Example/Tests/Util/FSTHelpers.mm @@ -48,6 +48,7 @@ #include "Firestore/core/src/firebase/firestore/model/field_mask.h" #include "Firestore/core/src/firebase/firestore/model/field_transform.h" #include "Firestore/core/src/firebase/firestore/model/field_value.h" +#include "Firestore/core/src/firebase/firestore/model/precondition.h" #include "Firestore/core/src/firebase/firestore/model/resource_path.h" #include "Firestore/core/src/firebase/firestore/model/transform_operations.h" #include "Firestore/core/src/firebase/firestore/util/string_apple.h" @@ -62,6 +63,7 @@ using firebase::firestore::model::FieldPath; using firebase::firestore::model::FieldTransform; using firebase::firestore::model::FieldValue; +using firebase::firestore::model::Precondition; using firebase::firestore::model::ResourcePath; using firebase::firestore::model::ServerTimestampTransform; using firebase::firestore::model::TransformOperation; @@ -250,7 +252,7 @@ NSComparator FSTTestDocComparator(const absl::string_view fieldPath) { FSTSetMutation *FSTTestSetMutation(NSString *path, NSDictionary *values) { return [[FSTSetMutation alloc] initWithKey:FSTTestDocKey(path) value:FSTTestObjectValue(values) - precondition:[FSTPrecondition none]]; + precondition:Precondition::None()]; } FSTPatchMutation *FSTTestPatchMutation(const absl::string_view path, @@ -274,7 +276,7 @@ NSComparator FSTTestDocComparator(const absl::string_view fieldPath) { return [[FSTPatchMutation alloc] initWithKey:key fieldMask:mask value:objectValue - precondition:[FSTPrecondition preconditionWithExists:YES]]; + precondition:Precondition::Exists(true)]; } // For now this only creates TransformMutations with server timestamps. @@ -292,8 +294,8 @@ NSComparator FSTTestDocComparator(const absl::string_view fieldPath) { } FSTDeleteMutation *FSTTestDeleteMutation(NSString *path) { - return [[FSTDeleteMutation alloc] initWithKey:FSTTestDocKey(path) - precondition:[FSTPrecondition none]]; + return + [[FSTDeleteMutation alloc] initWithKey:FSTTestDocKey(path) precondition:Precondition::None()]; } FSTMaybeDocumentDictionary *FSTTestDocUpdates(NSArray *docs) { diff --git a/Firestore/Source/API/FIRDocumentReference.mm b/Firestore/Source/API/FIRDocumentReference.mm index cc52d455f1f..17010074cfd 100644 --- a/Firestore/Source/API/FIRDocumentReference.mm +++ b/Firestore/Source/API/FIRDocumentReference.mm @@ -41,11 +41,13 @@ #import "Firestore/Source/Util/FSTUsageValidation.h" #include "Firestore/core/src/firebase/firestore/model/document_key.h" +#include "Firestore/core/src/firebase/firestore/model/precondition.h" #include "Firestore/core/src/firebase/firestore/model/resource_path.h" #include "Firestore/core/src/firebase/firestore/util/string_apple.h" namespace util = firebase::firestore::util; using firebase::firestore::model::DocumentKey; +using firebase::firestore::model::Precondition; using firebase::firestore::model::ResourcePath; NS_ASSUME_NONNULL_BEGIN @@ -168,7 +170,7 @@ - (void)setData:(NSDictionary *)documentData ? [self.firestore.dataConverter parsedMergeData:documentData] : [self.firestore.dataConverter parsedSetData:documentData]; return [self.firestore.client - writeMutations:[parsed mutationsWithKey:self.key precondition:[FSTPrecondition none]] + writeMutations:[parsed mutationsWithKey:self.key precondition:Precondition::None()] completion:completion]; } @@ -180,8 +182,7 @@ - (void)updateData:(NSDictionary *)fields completion:(nullable void (^)(NSError *_Nullable error))completion { FSTParsedUpdateData *parsed = [self.firestore.dataConverter parsedUpdateData:fields]; return [self.firestore.client - writeMutations:[parsed mutationsWithKey:self.key - precondition:[FSTPrecondition preconditionWithExists:YES]] + writeMutations:[parsed mutationsWithKey:self.key precondition:Precondition::Exists(true)] completion:completion]; } @@ -191,7 +192,7 @@ - (void)deleteDocument { - (void)deleteDocumentWithCompletion:(nullable void (^)(NSError *_Nullable error))completion { FSTDeleteMutation *mutation = - [[FSTDeleteMutation alloc] initWithKey:self.key precondition:[FSTPrecondition none]]; + [[FSTDeleteMutation alloc] initWithKey:self.key precondition:Precondition::None()]; return [self.firestore.client writeMutations:@[ mutation ] completion:completion]; } diff --git a/Firestore/Source/API/FIRWriteBatch.mm b/Firestore/Source/API/FIRWriteBatch.mm index b1cfa090eec..df6e5e0f324 100644 --- a/Firestore/Source/API/FIRWriteBatch.mm +++ b/Firestore/Source/API/FIRWriteBatch.mm @@ -24,6 +24,10 @@ #import "Firestore/Source/Model/FSTMutation.h" #import "Firestore/Source/Util/FSTUsageValidation.h" +#include "Firestore/core/src/firebase/firestore/model/precondition.h" + +using firebase::firestore::model::Precondition; + NS_ASSUME_NONNULL_BEGIN #pragma mark - FIRWriteBatch @@ -69,8 +73,8 @@ - (FIRWriteBatch *)setData:(NSDictionary *)data [self validateReference:document]; FSTParsedSetData *parsed = options.isMerge ? [self.firestore.dataConverter parsedMergeData:data] : [self.firestore.dataConverter parsedSetData:data]; - [self.mutations addObjectsFromArray:[parsed mutationsWithKey:document.key - precondition:[FSTPrecondition none]]]; + [self.mutations + addObjectsFromArray:[parsed mutationsWithKey:document.key precondition:Precondition::None()]]; return self; } @@ -79,9 +83,8 @@ - (FIRWriteBatch *)updateData:(NSDictionary *)fields [self verifyNotCommitted]; [self validateReference:document]; FSTParsedUpdateData *parsed = [self.firestore.dataConverter parsedUpdateData:fields]; - [self.mutations - addObjectsFromArray:[parsed mutationsWithKey:document.key - precondition:[FSTPrecondition preconditionWithExists:YES]]]; + [self.mutations addObjectsFromArray:[parsed mutationsWithKey:document.key + precondition:Precondition::Exists(true)]]; return self; } @@ -89,7 +92,7 @@ - (FIRWriteBatch *)deleteDocument:(FIRDocumentReference *)document { [self verifyNotCommitted]; [self validateReference:document]; [self.mutations addObject:[[FSTDeleteMutation alloc] initWithKey:document.key - precondition:[FSTPrecondition none]]]; + precondition:Precondition::None()]]; return self; } diff --git a/Firestore/Source/API/FSTUserDataConverter.h b/Firestore/Source/API/FSTUserDataConverter.h index a2f947a00f4..ea20b3ef71f 100644 --- a/Firestore/Source/API/FSTUserDataConverter.h +++ b/Firestore/Source/API/FSTUserDataConverter.h @@ -22,12 +22,12 @@ #include "Firestore/core/src/firebase/firestore/model/document_key.h" #include "Firestore/core/src/firebase/firestore/model/field_mask.h" #include "Firestore/core/src/firebase/firestore/model/field_transform.h" +#include "Firestore/core/src/firebase/firestore/model/precondition.h" @class FIRSetOptions; @class FSTObjectValue; @class FSTFieldValue; @class FSTMutation; -@class FSTPrecondition; @class FSTSnapshotVersion; NS_ASSUME_NONNULL_BEGIN @@ -58,7 +58,8 @@ NS_ASSUME_NONNULL_BEGIN * field transforms) using the specified document key and precondition. */ - (NSArray *)mutationsWithKey:(const firebase::firestore::model::DocumentKey &)key - precondition:(FSTPrecondition *)precondition; + precondition: + (const firebase::firestore::model::Precondition &)precondition; @end @@ -83,7 +84,8 @@ NS_ASSUME_NONNULL_BEGIN * field transforms) using the specified document key and precondition. */ - (NSArray *)mutationsWithKey:(const firebase::firestore::model::DocumentKey &)key - precondition:(FSTPrecondition *)precondition; + precondition: + (const firebase::firestore::model::Precondition &)precondition; @end diff --git a/Firestore/Source/API/FSTUserDataConverter.mm b/Firestore/Source/API/FSTUserDataConverter.mm index 2eee8781938..2e8988528f3 100644 --- a/Firestore/Source/API/FSTUserDataConverter.mm +++ b/Firestore/Source/API/FSTUserDataConverter.mm @@ -38,6 +38,7 @@ #include "Firestore/core/src/firebase/firestore/model/field_mask.h" #include "Firestore/core/src/firebase/firestore/model/field_path.h" #include "Firestore/core/src/firebase/firestore/model/field_transform.h" +#include "Firestore/core/src/firebase/firestore/model/precondition.h" #include "Firestore/core/src/firebase/firestore/model/transform_operations.h" #include "Firestore/core/src/firebase/firestore/util/string_apple.h" #include "absl/memory/memory.h" @@ -48,6 +49,7 @@ using firebase::firestore::model::FieldMask; using firebase::firestore::model::FieldPath; using firebase::firestore::model::FieldTransform; +using firebase::firestore::model::Precondition; using firebase::firestore::model::ServerTimestampTransform; using firebase::firestore::model::TransformOperation; @@ -91,7 +93,7 @@ - (instancetype)initWithData:(FSTObjectValue *)data } - (NSArray *)mutationsWithKey:(const DocumentKey &)key - precondition:(FSTPrecondition *)precondition { + precondition:(const Precondition &)precondition { NSMutableArray *mutations = [NSMutableArray array]; if (self.isPatch) { [mutations addObject:[[FSTPatchMutation alloc] initWithKey:key @@ -132,7 +134,7 @@ - (instancetype)initWithData:(FSTObjectValue *)data } - (NSArray *)mutationsWithKey:(const DocumentKey &)key - precondition:(FSTPrecondition *)precondition { + precondition:(const Precondition &)precondition { NSMutableArray *mutations = [NSMutableArray array]; [mutations addObject:[[FSTPatchMutation alloc] initWithKey:key fieldMask:self.fieldMask diff --git a/Firestore/Source/Core/FSTTransaction.mm b/Firestore/Source/Core/FSTTransaction.mm index 9e67ed495e6..abd27d44b0c 100644 --- a/Firestore/Source/Core/FSTTransaction.mm +++ b/Firestore/Source/Core/FSTTransaction.mm @@ -33,8 +33,10 @@ #import "Firestore/Source/Util/FSTUsageValidation.h" #include "Firestore/core/src/firebase/firestore/model/document_key.h" +#include "Firestore/core/src/firebase/firestore/model/precondition.h" using firebase::firestore::model::DocumentKey; +using firebase::firestore::model::Precondition; NS_ASSUME_NONNULL_BEGIN @@ -136,25 +138,26 @@ - (void)writeMutations:(NSArray *)mutations { * Returns version of this doc when it was read in this transaction as a precondition, or no * precondition if it was not read. */ -- (FSTPrecondition *)preconditionForDocumentKey:(const DocumentKey &)key { +- (Precondition)preconditionForDocumentKey:(const DocumentKey &)key { const auto iter = _readVersions.find(key); if (iter == _readVersions.end()) { - return [FSTPrecondition none]; + return Precondition::None(); } else { - return [FSTPrecondition preconditionWithUpdateTime:iter->second]; + return Precondition::UpdateTime(iter->second); } } /** * Returns the precondition for a document if the operation is an update, based on the provided - * UpdateOptions. Will return nil if an error occurred, in which case it sets the error parameter. + * UpdateOptions. Will return none precondition if an error occurred, in which case it sets the + * error parameter. */ -- (nullable FSTPrecondition *)preconditionForUpdateWithDocumentKey:(const DocumentKey &)key - error:(NSError **)error { +- (Precondition)preconditionForUpdateWithDocumentKey:(const DocumentKey &)key + error:(NSError **)error { const auto iter = _readVersions.find(key); if (iter == _readVersions.end()) { // Document was not read, so we just use the preconditions for an update. - return [FSTPrecondition preconditionWithExists:YES]; + return Precondition::Exists(true); } FSTSnapshotVersion *version = iter->second; @@ -169,10 +172,10 @@ - (nullable FSTPrecondition *)preconditionForUpdateWithDocumentKey:(const Docume NSLocalizedDescriptionKey : @"Can't update a document that doesn't exist." }]; } - return nil; + return Precondition::None(); } else { // Document exists, just base precondition on document update time. - return [FSTPrecondition preconditionWithUpdateTime:version]; + return Precondition::UpdateTime(version); } } @@ -183,9 +186,8 @@ - (void)setData:(FSTParsedSetData *)data forDocument:(const DocumentKey &)key { - (void)updateData:(FSTParsedUpdateData *)data forDocument:(const DocumentKey &)key { NSError *error = nil; - FSTPrecondition *_Nullable precondition = - [self preconditionForUpdateWithDocumentKey:key error:&error]; - if (precondition) { + const Precondition precondition = [self preconditionForUpdateWithDocumentKey:key error:&error]; + if (precondition.IsNone()) { [self writeMutations:[data mutationsWithKey:key precondition:precondition]]; } else { FSTAssert(error, @"Got nil precondition, but error was not set"); diff --git a/Firestore/Source/Model/FSTMutation.h b/Firestore/Source/Model/FSTMutation.h index 97d3e12f104..7261f308f76 100644 --- a/Firestore/Source/Model/FSTMutation.h +++ b/Firestore/Source/Model/FSTMutation.h @@ -23,6 +23,7 @@ #include "Firestore/core/src/firebase/firestore/model/field_mask.h" #include "Firestore/core/src/firebase/firestore/model/field_path.h" #include "Firestore/core/src/firebase/firestore/model/field_transform.h" +#include "Firestore/core/src/firebase/firestore/model/precondition.h" #include "Firestore/core/src/firebase/firestore/model/transform_operations.h" @class FSTDocument; @@ -72,7 +73,8 @@ NS_ASSUME_NONNULL_BEGIN - (id)init NS_UNAVAILABLE; - (instancetype)initWithKey:(firebase::firestore::model::DocumentKey)key - precondition:(FSTPrecondition *)precondition NS_DESIGNATED_INITIALIZER; + precondition:(firebase::firestore::model::Precondition)precondition + NS_DESIGNATED_INITIALIZER; /** * Applies this mutation to the given FSTDocument, FSTDeletedDocument or nil, if we don't have @@ -133,8 +135,7 @@ NS_ASSUME_NONNULL_BEGIN - (const firebase::firestore::model::DocumentKey &)key; -/** The precondition for this mutation. */ -@property(nonatomic, strong, readonly) FSTPrecondition *precondition; +- (const firebase::firestore::model::Precondition &)precondition; @end @@ -147,7 +148,7 @@ NS_ASSUME_NONNULL_BEGIN @interface FSTSetMutation : FSTMutation - (instancetype)initWithKey:(firebase::firestore::model::DocumentKey)key - precondition:(FSTPrecondition *)precondition NS_UNAVAILABLE; + precondition:(firebase::firestore::model::Precondition)precondition NS_UNAVAILABLE; /** * Initializes the set mutation. @@ -159,7 +160,8 @@ NS_ASSUME_NONNULL_BEGIN */ - (instancetype)initWithKey:(firebase::firestore::model::DocumentKey)key value:(FSTObjectValue *)value - precondition:(FSTPrecondition *)precondition NS_DESIGNATED_INITIALIZER; + precondition:(firebase::firestore::model::Precondition)precondition + NS_DESIGNATED_INITIALIZER; /** The object value to use when setting the document. */ @property(nonatomic, strong, readonly) FSTObjectValue *value; @@ -178,9 +180,9 @@ NS_ASSUME_NONNULL_BEGIN */ @interface FSTPatchMutation : FSTMutation -/** Returns the precondition for the given FSTPrecondition. */ +/** Returns the precondition for the given Precondition. */ - (instancetype)initWithKey:(firebase::firestore::model::DocumentKey)key - precondition:(FSTPrecondition *)precondition NS_UNAVAILABLE; + precondition:(firebase::firestore::model::Precondition)precondition NS_UNAVAILABLE; /** * Initializes a new patch mutation with an explicit FieldMask and FSTObjectValue representing @@ -196,7 +198,8 @@ NS_ASSUME_NONNULL_BEGIN - (instancetype)initWithKey:(firebase::firestore::model::DocumentKey)key fieldMask:(firebase::firestore::model::FieldMask)fieldMask value:(FSTObjectValue *)value - precondition:(FSTPrecondition *)precondition NS_DESIGNATED_INITIALIZER; + precondition:(firebase::firestore::model::Precondition)precondition + NS_DESIGNATED_INITIALIZER; /** * A mask to apply to |value|, where only fields that are in both the fieldMask and the value @@ -223,7 +226,7 @@ NS_ASSUME_NONNULL_BEGIN @interface FSTTransformMutation : FSTMutation - (instancetype)initWithKey:(firebase::firestore::model::DocumentKey)key - precondition:(FSTPrecondition *)precondition NS_UNAVAILABLE; + precondition:(firebase::firestore::model::Precondition)precondition NS_UNAVAILABLE; /** * Initializes a new transform mutation with the specified field transforms. diff --git a/Firestore/Source/Model/FSTMutation.mm b/Firestore/Source/Model/FSTMutation.mm index 3e5ec1a65a9..df4deb0ec54 100644 --- a/Firestore/Source/Model/FSTMutation.mm +++ b/Firestore/Source/Model/FSTMutation.mm @@ -32,12 +32,14 @@ #include "Firestore/core/src/firebase/firestore/model/field_mask.h" #include "Firestore/core/src/firebase/firestore/model/field_path.h" #include "Firestore/core/src/firebase/firestore/model/field_transform.h" +#include "Firestore/core/src/firebase/firestore/model/precondition.h" #include "Firestore/core/src/firebase/firestore/model/transform_operations.h" using firebase::firestore::model::DocumentKey; using firebase::firestore::model::FieldMask; using firebase::firestore::model::FieldPath; using firebase::firestore::model::FieldTransform; +using firebase::firestore::model::Precondition; using firebase::firestore::model::ServerTimestampTransform; using firebase::firestore::model::TransformOperation; @@ -62,12 +64,13 @@ - (instancetype)initWithVersion:(FSTSnapshotVersion *_Nullable)version @implementation FSTMutation { DocumentKey _key; + Precondition _precondition; } -- (instancetype)initWithKey:(DocumentKey)key precondition:(FSTPrecondition *)precondition { +- (instancetype)initWithKey:(DocumentKey)key precondition:(Precondition)precondition { if (self = [super init]) { _key = std::move(key); - _precondition = precondition; + _precondition = std::move(precondition); } return self; } @@ -90,6 +93,10 @@ - (nullable FSTMaybeDocument *)applyTo:(nullable FSTMaybeDocument *)maybeDoc return _key; } +- (const firebase::firestore::model::Precondition &)precondition { + return _precondition; +} + @end #pragma mark - FSTSetMutation @@ -98,8 +105,8 @@ @implementation FSTSetMutation - (instancetype)initWithKey:(DocumentKey)key value:(FSTObjectValue *)value - precondition:(FSTPrecondition *)precondition { - if (self = [super initWithKey:std::move(key) precondition:precondition]) { + precondition:(Precondition)precondition { + if (self = [super initWithKey:std::move(key) precondition:std::move(precondition)]) { _value = value; } return self; @@ -107,7 +114,8 @@ - (instancetype)initWithKey:(DocumentKey)key - (NSString *)description { return [NSString stringWithFormat:@"", - self.key.ToString().c_str(), self.value, self.precondition]; + self.key.ToString().c_str(), self.value, + self.precondition.description()]; } - (BOOL)isEqual:(id)other { @@ -120,12 +128,12 @@ - (BOOL)isEqual:(id)other { FSTSetMutation *otherMutation = (FSTSetMutation *)other; return [self.key isEqual:otherMutation.key] && [self.value isEqual:otherMutation.value] && - [self.precondition isEqual:otherMutation.precondition]; + self.precondition == otherMutation.precondition; } - (NSUInteger)hash { NSUInteger result = [self.key hash]; - result = 31 * result + [self.precondition hash]; + result = 31 * result + self.precondition.Hash(); result = 31 * result + [self.value hash]; return result; } @@ -138,7 +146,7 @@ - (nullable FSTMaybeDocument *)applyTo:(nullable FSTMaybeDocument *)maybeDoc FSTAssert(!mutationResult.transformResults, @"Transform results received by FSTSetMutation."); } - if (![self.precondition isValidForDocument:maybeDoc]) { + if (!self.precondition.IsValidFor(maybeDoc)) { return maybeDoc; } @@ -172,8 +180,8 @@ @implementation FSTPatchMutation { - (instancetype)initWithKey:(DocumentKey)key fieldMask:(FieldMask)fieldMask value:(FSTObjectValue *)value - precondition:(FSTPrecondition *)precondition { - self = [super initWithKey:std::move(key) precondition:precondition]; + precondition:(Precondition)precondition { + self = [super initWithKey:std::move(key) precondition:std::move(precondition)]; if (self) { _fieldMask = std::move(fieldMask); _value = value; @@ -196,12 +204,12 @@ - (BOOL)isEqual:(id)other { FSTPatchMutation *otherMutation = (FSTPatchMutation *)other; return [self.key isEqual:otherMutation.key] && self.fieldMask == otherMutation.fieldMask && [self.value isEqual:otherMutation.value] && - [self.precondition isEqual:otherMutation.precondition]; + self.precondition == otherMutation.precondition; } - (NSUInteger)hash { NSUInteger result = [self.key hash]; - result = 31 * result + [self.precondition hash]; + result = 31 * result + self.precondition.Hash(); result = 31 * result + self.fieldMask.Hash(); result = 31 * result + [self.value hash]; return result; @@ -210,7 +218,7 @@ - (NSUInteger)hash { - (NSString *)description { return [NSString stringWithFormat:@"", self.key.ToString().c_str(), self.fieldMask.ToString().c_str(), - self.value, self.precondition]; + self.value, self.precondition.description()]; } - (nullable FSTMaybeDocument *)applyTo:(nullable FSTMaybeDocument *)maybeDoc @@ -221,7 +229,7 @@ - (nullable FSTMaybeDocument *)applyTo:(nullable FSTMaybeDocument *)maybeDoc FSTAssert(!mutationResult.transformResults, @"Transform results received by FSTPatchMutation."); } - if (![self.precondition isValidForDocument:maybeDoc]) { + if (!self.precondition.IsValidFor(maybeDoc)) { return maybeDoc; } @@ -274,8 +282,7 @@ - (instancetype)initWithKey:(DocumentKey)key // NOTE: We set a precondition of exists: true as a safety-check, since we always combine // FSTTransformMutations with a FSTSetMutation or FSTPatchMutation which (if successful) should // end up with an existing document. - if (self = [super initWithKey:std::move(key) - precondition:[FSTPrecondition preconditionWithExists:YES]]) { + if (self = [super initWithKey:std::move(key) precondition:Precondition::Exists(true)]) { _fieldTransforms = std::move(fieldTransforms); } return self; @@ -296,12 +303,12 @@ - (BOOL)isEqual:(id)other { FSTTransformMutation *otherMutation = (FSTTransformMutation *)other; return [self.key isEqual:otherMutation.key] && self.fieldTransforms == otherMutation.fieldTransforms && - [self.precondition isEqual:otherMutation.precondition]; + self.precondition == otherMutation.precondition; } - (NSUInteger)hash { NSUInteger result = [self.key hash]; - result = 31 * result + [self.precondition hash]; + result = 31 * result + self.precondition.Hash(); for (const auto &transform : self.fieldTransforms) { result = 31 * result + transform.Hash(); } @@ -315,7 +322,7 @@ - (NSString *)description { } return [NSString stringWithFormat:@"", self.key.ToString().c_str(), fieldTransforms.c_str(), - self.precondition]; + self.precondition.description()]; } - (nullable FSTMaybeDocument *)applyTo:(nullable FSTMaybeDocument *)maybeDoc @@ -327,7 +334,7 @@ - (nullable FSTMaybeDocument *)applyTo:(nullable FSTMaybeDocument *)maybeDoc @"Transform results missing for FSTTransformMutation."); } - if (![self.precondition isValidForDocument:maybeDoc]) { + if (!self.precondition.IsValidFor(maybeDoc)) { return maybeDoc; } @@ -415,19 +422,18 @@ - (BOOL)isEqual:(id)other { } FSTDeleteMutation *otherMutation = (FSTDeleteMutation *)other; - return [self.key isEqual:otherMutation.key] && - [self.precondition isEqual:otherMutation.precondition]; + return [self.key isEqual:otherMutation.key] && self.precondition == otherMutation.precondition; } - (NSUInteger)hash { NSUInteger result = [self.key hash]; - result = 31 * result + [self.precondition hash]; + result = 31 * result + self.precondition.Hash(); return result; } - (NSString *)description { return [NSString stringWithFormat:@"", - self.key.ToString().c_str(), self.precondition]; + self.key.ToString().c_str(), self.precondition.description()]; } - (nullable FSTMaybeDocument *)applyTo:(nullable FSTMaybeDocument *)maybeDoc @@ -439,7 +445,7 @@ - (nullable FSTMaybeDocument *)applyTo:(nullable FSTMaybeDocument *)maybeDoc @"Transform results received by FSTDeleteMutation."); } - if (![self.precondition isValidForDocument:maybeDoc]) { + if (!self.precondition.IsValidFor(maybeDoc)) { return maybeDoc; } diff --git a/Firestore/Source/Remote/FSTSerializerBeta.mm b/Firestore/Source/Remote/FSTSerializerBeta.mm index 2d493e15449..1bf2ff24e9e 100644 --- a/Firestore/Source/Remote/FSTSerializerBeta.mm +++ b/Firestore/Source/Remote/FSTSerializerBeta.mm @@ -48,6 +48,7 @@ #include "Firestore/core/src/firebase/firestore/model/field_mask.h" #include "Firestore/core/src/firebase/firestore/model/field_path.h" #include "Firestore/core/src/firebase/firestore/model/field_transform.h" +#include "Firestore/core/src/firebase/firestore/model/precondition.h" #include "Firestore/core/src/firebase/firestore/model/resource_path.h" #include "Firestore/core/src/firebase/firestore/model/transform_operations.h" #include "Firestore/core/src/firebase/firestore/util/string_apple.h" @@ -59,8 +60,10 @@ using firebase::firestore::model::FieldMask; using firebase::firestore::model::FieldPath; using firebase::firestore::model::FieldTransform; +using firebase::firestore::model::Precondition; using firebase::firestore::model::ResourcePath; using firebase::firestore::model::ServerTimestampTransform; +using firebase::firestore::model::SnapshotVersion; using firebase::firestore::model::TransformOperation; NS_ASSUME_NONNULL_BEGIN @@ -473,7 +476,7 @@ - (GCFSWrite *)encodedMutation:(FSTMutation *)mutation { FSTFail(@"Unknown mutation type %@", NSStringFromClass(mutationClass)); } - if (!mutation.precondition.isNone) { + if (!mutation.precondition.IsNone()) { proto.currentDocument = [self encodedPrecondition:mutation.precondition]; } @@ -481,9 +484,9 @@ - (GCFSWrite *)encodedMutation:(FSTMutation *)mutation { } - (FSTMutation *)decodedMutation:(GCFSWrite *)mutation { - FSTPrecondition *precondition = [mutation hasCurrentDocument] - ? [self decodedPrecondition:mutation.currentDocument] - : [FSTPrecondition none]; + Precondition precondition = [mutation hasCurrentDocument] + ? [self decodedPrecondition:mutation.currentDocument] + : Precondition::None(); switch (mutation.operationOneOfCase) { case GCFSWrite_Operation_OneOfCase_Update: @@ -503,8 +506,7 @@ - (FSTMutation *)decodedMutation:(GCFSWrite *)mutation { precondition:precondition]; case GCFSWrite_Operation_OneOfCase_Transform: { - FSTPreconditionExists exists = precondition.exists; - FSTAssert(exists == FSTPreconditionExistsYes, + FSTAssert(precondition == Precondition::Exists(true), @"Transforms must have precondition \"exists == true\""); return [[FSTTransformMutation alloc] @@ -518,30 +520,31 @@ - (FSTMutation *)decodedMutation:(GCFSWrite *)mutation { } } -- (GCFSPrecondition *)encodedPrecondition:(FSTPrecondition *)precondition { - FSTAssert(!precondition.isNone, @"Can't serialize an empty precondition"); +- (GCFSPrecondition *)encodedPrecondition:(const Precondition &)precondition { + FSTAssert(!precondition.IsNone(), @"Can't serialize an empty precondition"); GCFSPrecondition *message = [GCFSPrecondition message]; - if (precondition.updateTime) { - message.updateTime = [self encodedVersion:precondition.updateTime]; - } else if (precondition.exists != FSTPreconditionExistsNotSet) { - message.exists = precondition.exists == FSTPreconditionExistsYes; + if (precondition.update_time() != SnapshotVersion::None()) { + message.updateTime = + [self encodedVersion:static_cast(precondition.update_time())]; + } else if (precondition == Precondition::Exists(true) || + precondition == Precondition::Exists(false)) { + message.exists = precondition == Precondition::Exists(true); } else { - FSTFail(@"Unknown precondition: %@", precondition); + FSTFail(@"Unknown precondition: %@", precondition.description()); } return message; } -- (FSTPrecondition *)decodedPrecondition:(GCFSPrecondition *)precondition { +- (Precondition)decodedPrecondition:(GCFSPrecondition *)precondition { switch (precondition.conditionTypeOneOfCase) { case GCFSPrecondition_ConditionType_OneOfCase_GPBUnsetOneOfCase: - return [FSTPrecondition none]; + return Precondition::None(); case GCFSPrecondition_ConditionType_OneOfCase_Exists: - return [FSTPrecondition preconditionWithExists:precondition.exists]; + return Precondition::Exists(precondition.exists); case GCFSPrecondition_ConditionType_OneOfCase_UpdateTime: - return [FSTPrecondition - preconditionWithUpdateTime:[self decodedVersion:precondition.updateTime]]; + return Precondition::UpdateTime([self decodedVersion:precondition.updateTime]); default: FSTFail(@"Unrecognized Precondition one-of case %@", precondition); diff --git a/Firestore/core/src/firebase/firestore/model/precondition.h b/Firestore/core/src/firebase/firestore/model/precondition.h index 7c049138737..4e66f1fd123 100644 --- a/Firestore/core/src/firebase/firestore/model/precondition.h +++ b/Firestore/core/src/firebase/firestore/model/precondition.h @@ -20,6 +20,12 @@ #include #include +#if defined(__OBJC__) +#import "FIRTimestamp.h" +#import "Firestore/Source/Core/FSTSnapshotVersion.h" +#import "Firestore/Source/Model/FSTDocument.h" +#endif // defined(__OBJC__) + #include "Firestore/core/src/firebase/firestore/model/maybe_document.h" #include "Firestore/core/src/firebase/firestore/model/snapshot_version.h" @@ -58,7 +64,44 @@ class Precondition { /** Returns whether this Precondition represents no precondition. */ bool IsNone() const; + const SnapshotVersion& update_time() const { + return update_time_; + } + #if defined(__OBJC__) + // Objective-C requires a default constructor. + Precondition() + : type_(Type::None), + update_time_(SnapshotVersion::None()), + exists_(false) { + } + + // MaybeDocument is not fully ported yet. So we suppose this addition helper. + bool IsValidFor(FSTMaybeDocument* maybe_doc) const { + switch (type_) { + case Type::UpdateTime: + return [maybe_doc isKindOfClass:[FSTDocument class]] && + firebase::firestore::model::SnapshotVersion(maybe_doc.version) == + update_time_; + break; + case Type::Exists: + if (exists_) { + return [maybe_doc isKindOfClass:[FSTDocument class]]; + } else { + return maybe_doc == nil || + [maybe_doc isKindOfClass:[FSTDeletedDocument class]]; + } + break; + case Type::None: + FIREBASE_ASSERT_MESSAGE(IsNone(), "Precondition should be empty"); + return true; + break; + default: + FIREBASE_ASSERT_MESSAGE(false, "Invalid precondition"); + break; + } + } + bool operator==(const Precondition& other) const { return update_time_ == other.update_time_ && exists_ == other.exists_; } @@ -66,12 +109,12 @@ class Precondition { // For Objective-C++ hash; to be removed after migration. // Do NOT use in C++ code. NSUInteger Hash() const { - NSUInteger hash = update_time_.Hash(); - hash = hash * 31 + other.exists_; + NSUInteger hash = std::hash()(update_time_.timestamp()); + hash = hash * 31 + exists_; return hash; } - NSString* description const { + NSString* description() const { switch (type_) { case Type::None: return @">"; @@ -84,8 +127,9 @@ class Precondition { } break; case Type::UpdateTime: - return [NSString stringWithFormat:@"", - update_time_.ToString().c_str()]; + return [NSString + stringWithFormat:@"", + update_time_.timestamp().ToString().c_str()]; break; default: // We do not raise assertion here. This function is mainly used in @@ -100,7 +144,7 @@ class Precondition { Precondition(Type type, SnapshotVersion update_time, bool exists); // The actual time of this precondition. - Type type_; + Type type_ = Type::None; // For UpdateTime type, preconditions a mutation based on the last updateTime. SnapshotVersion update_time_; diff --git a/Firestore/core/src/firebase/firestore/model/snapshot_version.h b/Firestore/core/src/firebase/firestore/model/snapshot_version.h index 56e8c50a2c1..b35210a2b60 100644 --- a/Firestore/core/src/firebase/firestore/model/snapshot_version.h +++ b/Firestore/core/src/firebase/firestore/model/snapshot_version.h @@ -19,6 +19,11 @@ #include "Firestore/core/include/firebase/firestore/timestamp.h" +#if defined(__OBJC__) +#import "FIRTimestamp.h" +#import "Firestore/Source/Core/FSTSnapshotVersion.h" +#endif // defined(__OBJC__) + namespace firebase { namespace firestore { namespace model { @@ -38,6 +43,29 @@ class SnapshotVersion { /** Creates a new version that is smaller than all other versions. */ static const SnapshotVersion& None(); +#if defined(__OBJC__) + SnapshotVersion(FSTSnapshotVersion* version) { // NOLINT(runtime/explicit) + if ([version isEqual:[FSTSnapshotVersion noVersion]]) { + timestamp_ = Timestamp{}; + } else { + timestamp_ = + Timestamp{version.timestamp.seconds, version.timestamp.nanoseconds}; + } + } + + operator FSTSnapshotVersion*() const { + if (timestamp_ == Timestamp{}) { + return [FSTSnapshotVersion noVersion]; + } else { + return [FSTSnapshotVersion + versionWithTimestamp:[FIRTimestamp + timestampWithSeconds:timestamp_.seconds() + nanoseconds:timestamp_ + .nanoseconds()]]; + } + } +#endif // defined(__OBJC__) + private: Timestamp timestamp_; }; diff --git a/Firestore/core/test/firebase/firestore/model/precondition_test.cc b/Firestore/core/test/firebase/firestore/model/precondition_test.cc index 5a8411bb43e..742bc3dd5a7 100644 --- a/Firestore/core/test/firebase/firestore/model/precondition_test.cc +++ b/Firestore/core/test/firebase/firestore/model/precondition_test.cc @@ -18,6 +18,7 @@ #include "Firestore/core/src/firebase/firestore/model/document.h" #include "Firestore/core/src/firebase/firestore/model/no_document.h" +#include "Firestore/core/src/firebase/firestore/model/snapshot_version.h" #include "Firestore/core/test/firebase/firestore/testutil/testutil.h" #include "gtest/gtest.h" @@ -29,6 +30,7 @@ namespace model { TEST(Precondition, None) { const Precondition none = Precondition::None(); EXPECT_TRUE(none.IsNone()); + EXPECT_EQ(SnapshotVersion::None(), none.update_time()); const NoDocument deleted_doc = testutil::DeletedDoc("foo/doc", 1234567); const Document doc = testutil::Doc("bar/doc", 7654321); @@ -41,6 +43,8 @@ TEST(Precondition, Exists) { const Precondition no_exists = Precondition::Exists(false); EXPECT_FALSE(exists.IsNone()); EXPECT_FALSE(no_exists.IsNone()); + EXPECT_EQ(SnapshotVersion::None(), exists.update_time()); + EXPECT_EQ(SnapshotVersion::None(), no_exists.update_time()); const NoDocument deleted_doc = testutil::DeletedDoc("foo/doc", 1234567); const Document doc = testutil::Doc("bar/doc", 7654321); @@ -54,6 +58,7 @@ TEST(Precondition, UpdateTime) { const Precondition update_time = Precondition::UpdateTime(testutil::Version(1234567)); EXPECT_FALSE(update_time.IsNone()); + EXPECT_EQ(testutil::Version(1234567), update_time.update_time()); const NoDocument deleted_doc = testutil::DeletedDoc("foo/doc", 1234567); const Document not_match = testutil::Doc("bar/doc", 7654321); diff --git a/Firestore/core/test/firebase/firestore/testutil/testutil.h b/Firestore/core/test/firebase/firestore/testutil/testutil.h index db46662222d..cd31c1a70e8 100644 --- a/Firestore/core/test/firebase/firestore/testutil/testutil.h +++ b/Firestore/core/test/firebase/firestore/testutil/testutil.h @@ -17,7 +17,7 @@ #ifndef FIRESTORE_CORE_TEST_FIREBASE_FIRESTORE_TESTUTIL_TESTUTIL_H_ #define FIRESTORE_CORE_TEST_FIREBASE_FIRESTORE_TESTUTIL_TESTUTIL_H_ -#include +#include #include "Firestore/core/include/firebase/firestore/timestamp.h" #include "Firestore/core/src/firebase/firestore/model/document.h" @@ -46,17 +46,21 @@ inline model::ResourcePath Resource(absl::string_view field) { return model::ResourcePath::FromString(field); } -inline model::SnapshotVersion Version(time_t version) { - return model::SnapshotVersion{Timestamp::FromTimeT(version)}; +// Version is epoch time in macroseconds. This helper is defined so to match +// SDKs in other platform. +inline model::SnapshotVersion Version(int64_t version) { + return model::SnapshotVersion{ + Timestamp{/*seconds=*/version / 1000, + /*nanoseconds=*/static_cast(version % 1000) * 1000}}; } -inline model::Document Doc(absl::string_view key, time_t version) { +inline model::Document Doc(absl::string_view key, int64_t version) { return model::Document{model::FieldValue::ObjectValueFromMap({}), Key(key), Version(version), /* has_local_mutations= */ false}; } -inline model::NoDocument DeletedDoc(absl::string_view key, time_t version) { +inline model::NoDocument DeletedDoc(absl::string_view key, int64_t version) { return model::NoDocument{Key(key), Version(version)}; } From a87cb0a6c3c024fee3b51683c6be57def70285b1 Mon Sep 17 00:00:00 2001 From: zxu123 Date: Fri, 6 Apr 2018 16:06:36 -0400 Subject: [PATCH 16/22] address changes --- Firestore/Example/Tests/Util/FSTHelpers.mm | 3 +-- Firestore/Source/Model/FSTMutation.mm | 8 ++++---- Firestore/Source/Remote/FSTSerializerBeta.mm | 8 ++++---- .../firebase/firestore/model/field_transform.h | 17 +++++++++-------- .../firestore/model/field_transform_test.cc | 2 +- 5 files changed, 19 insertions(+), 19 deletions(-) diff --git a/Firestore/Example/Tests/Util/FSTHelpers.mm b/Firestore/Example/Tests/Util/FSTHelpers.mm index e348cb7fa7c..12664b84dd6 100644 --- a/Firestore/Example/Tests/Util/FSTHelpers.mm +++ b/Firestore/Example/Tests/Util/FSTHelpers.mm @@ -281,8 +281,7 @@ NSComparator FSTTestDocComparator(const absl::string_view fieldPath) { FSTTransformMutation *FSTTestTransformMutation(NSString *path, NSArray *serverTimestampFields) { FSTDocumentKey *key = [FSTDocumentKey keyWithPath:testutil::Resource(util::MakeStringView(path))]; - std::vector fieldTransforms{}; - fieldTransforms.reserve(serverTimestampFields.count); + std::vector fieldTransforms; for (NSString *field in serverTimestampFields) { FieldPath fieldPath = testutil::Field(util::MakeStringView(field)); auto transformOp = absl::make_unique(ServerTimestampTransform::Get()); diff --git a/Firestore/Source/Model/FSTMutation.mm b/Firestore/Source/Model/FSTMutation.mm index 7eb52c2c6ae..470d47671ce 100644 --- a/Firestore/Source/Model/FSTMutation.mm +++ b/Firestore/Source/Model/FSTMutation.mm @@ -407,7 +407,7 @@ - (NSUInteger)hash { } - (NSString *)description { - std::string fieldTransforms = ""; + std::string fieldTransforms; for (const auto &transform : self.fieldTransforms) { fieldTransforms += " " + transform.path().CanonicalString(); } @@ -463,7 +463,7 @@ - (nullable FSTMaybeDocument *)applyTo:(nullable FSTMaybeDocument *)maybeDoc writeTime:(FIRTimestamp *)localWriteTime { NSMutableArray *transformResults = [NSMutableArray array]; for (const FieldTransform &fieldTransform : self.fieldTransforms) { - if (fieldTransform.transform()->type() == TransformOperation::Type::ServerTimestamp) { + if (fieldTransform.transformation()->type() == TransformOperation::Type::ServerTimestamp) { FSTFieldValue *previousValue = nil; if ([baseDocument isMemberOfClass:[FSTDocument class]]) { @@ -474,7 +474,7 @@ - (nullable FSTMaybeDocument *)applyTo:(nullable FSTMaybeDocument *)maybeDoc addObject:[FSTServerTimestampValue serverTimestampValueWithLocalWriteTime:localWriteTime previousValue:previousValue]]; } else { - FSTFail(@"Encountered unknown transform: %d type", fieldTransform.transform()->type()); + FSTFail(@"Encountered unknown transform: %d type", fieldTransform.transformation()->type()); } } return transformResults; @@ -487,7 +487,7 @@ - (FSTObjectValue *)transformObject:(FSTObjectValue *)objectValue for (NSUInteger i = 0; i < self.fieldTransforms.size(); i++) { const FieldTransform &fieldTransform = self.fieldTransforms[i]; - const TransformOperation *transform = fieldTransform.transform(); + const TransformOperation *transform = fieldTransform.transformation(); const FieldPath &fieldPath = fieldTransform.path(); if (transform->type() == TransformOperation::Type::ServerTimestamp) { objectValue = [objectValue objectBySettingValue:transformResults[i] forPath:fieldPath]; diff --git a/Firestore/Source/Remote/FSTSerializerBeta.mm b/Firestore/Source/Remote/FSTSerializerBeta.mm index 2d493e15449..6b6e41ecbd3 100644 --- a/Firestore/Source/Remote/FSTSerializerBeta.mm +++ b/Firestore/Source/Remote/FSTSerializerBeta.mm @@ -557,7 +557,7 @@ - (GCFSDocumentMask *)encodedFieldMask:(const FieldMask &)fieldMask { } - (FieldMask)decodedFieldMask:(GCFSDocumentMask *)fieldMask { - std::vector fields{}; + std::vector fields; fields.reserve(fieldMask.fieldPathsArray_Count); for (NSString *path in fieldMask.fieldPathsArray) { fields.push_back(FieldPath::FromServerFormat(util::MakeStringView(path))); @@ -569,8 +569,8 @@ - (FieldMask)decodedFieldMask:(GCFSDocumentMask *)fieldMask { (const std::vector &)fieldTransforms { NSMutableArray *protos = [NSMutableArray array]; for (const FieldTransform &fieldTransform : fieldTransforms) { - FSTAssert(fieldTransform.transform()->type() == TransformOperation::Type::ServerTimestamp, - @"Unknown transform: %d type", fieldTransform.transform()->type()); + FSTAssert(fieldTransform.transformation()->type() == TransformOperation::Type::ServerTimestamp, + @"Unknown transform: %d type", fieldTransform.transformation()->type()); GCFSDocumentTransform_FieldTransform *proto = [GCFSDocumentTransform_FieldTransform message]; proto.fieldPath = util::WrapNSString(fieldTransform.path().CanonicalString()); proto.setToServerValue = GCFSDocumentTransform_FieldTransform_ServerValue_RequestTime; @@ -581,7 +581,7 @@ - (FieldMask)decodedFieldMask:(GCFSDocumentMask *)fieldMask { - (std::vector)decodedFieldTransforms: (NSArray *)protos { - std::vector fieldTransforms{}; + std::vector fieldTransforms; fieldTransforms.reserve(protos.count); for (GCFSDocumentTransform_FieldTransform *proto in protos) { FSTAssert( diff --git a/Firestore/core/src/firebase/firestore/model/field_transform.h b/Firestore/core/src/firebase/firestore/model/field_transform.h index 137b885a251..72cb70e3cdf 100644 --- a/Firestore/core/src/firebase/firestore/model/field_transform.h +++ b/Firestore/core/src/firebase/firestore/model/field_transform.h @@ -30,28 +30,29 @@ namespace model { /** A field path and the TransformOperation to perform upon it. */ class FieldTransform { public: - FieldTransform(FieldPath path, std::unique_ptr transform) - : path_(std::move(path)), transform_(std::move(transform)) { + FieldTransform(FieldPath path, + std::unique_ptr transformation) + : path_(std::move(path)), transformation_(std::move(transformation)) { } const FieldPath& path() const { return path_; } - const TransformOperation* transform() const { - return transform_.get(); + const TransformOperation* transformation() const { + return transformation_.get(); } -#if defined(__OBJC__) bool operator==(const FieldTransform& other) const { - return path_ == other.path_ && *transform_ == *other.transform_; + return path_ == other.path_ && *transformation_ == *other.transformation_; } +#if defined(__OBJC__) // For Objective-C++ hash; to be removed after migration. // Do NOT use in C++ code. NSUInteger Hash() const { NSUInteger hash = path_.Hash(); - hash = hash * 31 + transform_->Hash(); + hash = hash * 31 + transformation_->Hash(); return hash; } #endif // defined(__OBJC__) @@ -59,7 +60,7 @@ class FieldTransform { private: FieldPath path_; // Shared by copies of the same FieldTransform. - std::shared_ptr transform_; + std::shared_ptr transformation_; }; } // namespace model diff --git a/Firestore/core/test/firebase/firestore/model/field_transform_test.cc b/Firestore/core/test/firebase/firestore/model/field_transform_test.cc index 524bd91e12f..e580db4e58d 100644 --- a/Firestore/core/test/firebase/firestore/model/field_transform_test.cc +++ b/Firestore/core/test/firebase/firestore/model/field_transform_test.cc @@ -31,7 +31,7 @@ TEST(FieldTransform, Getter) { ServerTimestampTransform::Get())); EXPECT_EQ(testutil::Field("foo"), transform.path()); - EXPECT_EQ(ServerTimestampTransform::Get(), *transform.transform()); + EXPECT_EQ(ServerTimestampTransform::Get(), *transform.transformation()); } } // namespace model From 7936f8bcf7bdd0eeb7dc84d08207f2f3333d2f41 Mon Sep 17 00:00:00 2001 From: zxu123 Date: Mon, 9 Apr 2018 09:51:38 -0400 Subject: [PATCH 17/22] address changes --- .../firebase/firestore/model/precondition.cc | 16 +++++----------- .../firebase/firestore/model/precondition.h | 19 ++++++++++--------- .../firestore/model/snapshot_version.h | 4 +--- .../firebase/firestore/testutil/testutil.h | 2 +- 4 files changed, 17 insertions(+), 24 deletions(-) diff --git a/Firestore/core/src/firebase/firestore/model/precondition.cc b/Firestore/core/src/firebase/firestore/model/precondition.cc index 648d1dfa09a..3cf5351280c 100644 --- a/Firestore/core/src/firebase/firestore/model/precondition.cc +++ b/Firestore/core/src/firebase/firestore/model/precondition.cc @@ -31,10 +31,8 @@ Precondition::Precondition(Type type, SnapshotVersion update_time, bool exists) } /* static */ -const Precondition& Precondition::Exists(bool exists) { - static Precondition kExist{Type::Exists, SnapshotVersion::None(), true}; - static Precondition kNotExist{Type::Exists, SnapshotVersion::None(), false}; - return exists ? kExist : kNotExist; +Precondition Precondition::Exists(bool exists) { + return Precondition{Type::Exists, SnapshotVersion::None(), exists}; } /* static */ @@ -45,10 +43,8 @@ Precondition Precondition::UpdateTime(SnapshotVersion update_time) { } /* static */ -const Precondition& Precondition::None() { - static Precondition kNoPrecondition{Type::None, SnapshotVersion::None(), - false}; - return kNoPrecondition; +Precondition Precondition::None() { + return Precondition{Type::None, SnapshotVersion::None(), false}; } bool Precondition::IsValidFor(const MaybeDocument& maybe_doc) const { @@ -68,10 +64,8 @@ bool Precondition::IsValidFor(const MaybeDocument& maybe_doc) const { FIREBASE_ASSERT_MESSAGE(IsNone(), "Precondition should be empty"); return true; break; - default: - FIREBASE_ASSERT_MESSAGE(false, "Invalid precondition"); - break; } + FIREBASE_UNREACHABLE(); } bool Precondition::IsNone() const { diff --git a/Firestore/core/src/firebase/firestore/model/precondition.h b/Firestore/core/src/firebase/firestore/model/precondition.h index 4e66f1fd123..896af2408e8 100644 --- a/Firestore/core/src/firebase/firestore/model/precondition.h +++ b/Firestore/core/src/firebase/firestore/model/precondition.h @@ -28,6 +28,7 @@ #include "Firestore/core/src/firebase/firestore/model/maybe_document.h" #include "Firestore/core/src/firebase/firestore/model/snapshot_version.h" +#include "Firestore/core/src/firebase/firestore/util/firebase_assert.h" namespace firebase { namespace firestore { @@ -47,17 +48,17 @@ class Precondition { }; /** Creates a new Precondition with an exists flag. */ - static const Precondition& Exists(bool exists); + static Precondition Exists(bool exists); /** Creates a new Precondition based on a time the document exists at. */ static Precondition UpdateTime(SnapshotVersion update_time); /** Returns a precondition representing no precondition. */ - static const Precondition& None(); + static Precondition None(); /** - * Returns true if the preconditions is valid for the given document (or null - * if no document is available). + * Returns true if the precondition is valid for the given document (and the + * document is available). */ bool IsValidFor(const MaybeDocument& maybe_doc) const; @@ -96,14 +97,13 @@ class Precondition { FIREBASE_ASSERT_MESSAGE(IsNone(), "Precondition should be empty"); return true; break; - default: - FIREBASE_ASSERT_MESSAGE(false, "Invalid precondition"); - break; } + FIREBASE_UNREACHABLE(); } bool operator==(const Precondition& other) const { - return update_time_ == other.update_time_ && exists_ == other.exists_; + return type_ == other.type_ && update_time_ == other.update_time_ && + exists_ == other.exists_; } // For Objective-C++ hash; to be removed after migration. @@ -132,8 +132,9 @@ class Precondition { update_time_.timestamp().ToString().c_str()]; break; default: - // We do not raise assertion here. This function is mainly used in + // We only raise dev assertion here. This function is mainly used in // logging. + FIREBASE_DEV_ASSERT_MESSAGE(false, "precondition invalid"); return @""; break; } diff --git a/Firestore/core/src/firebase/firestore/model/snapshot_version.h b/Firestore/core/src/firebase/firestore/model/snapshot_version.h index b35210a2b60..900ae6d43f6 100644 --- a/Firestore/core/src/firebase/firestore/model/snapshot_version.h +++ b/Firestore/core/src/firebase/firestore/model/snapshot_version.h @@ -45,9 +45,7 @@ class SnapshotVersion { #if defined(__OBJC__) SnapshotVersion(FSTSnapshotVersion* version) { // NOLINT(runtime/explicit) - if ([version isEqual:[FSTSnapshotVersion noVersion]]) { - timestamp_ = Timestamp{}; - } else { + if (![version isEqual:[FSTSnapshotVersion noVersion]]) { timestamp_ = Timestamp{version.timestamp.seconds, version.timestamp.nanoseconds}; } diff --git a/Firestore/core/test/firebase/firestore/testutil/testutil.h b/Firestore/core/test/firebase/firestore/testutil/testutil.h index cd31c1a70e8..5cb9b59c29d 100644 --- a/Firestore/core/test/firebase/firestore/testutil/testutil.h +++ b/Firestore/core/test/firebase/firestore/testutil/testutil.h @@ -46,7 +46,7 @@ inline model::ResourcePath Resource(absl::string_view field) { return model::ResourcePath::FromString(field); } -// Version is epoch time in macroseconds. This helper is defined so to match +// Version is epoch time in microseconds. This helper is defined so to match // SDKs in other platform. inline model::SnapshotVersion Version(int64_t version) { return model::SnapshotVersion{ From 69692128a6f1e41d740f3407991af150f3e05abd Mon Sep 17 00:00:00 2001 From: zxu123 Date: Mon, 9 Apr 2018 11:40:43 -0400 Subject: [PATCH 18/22] fix bugs for integration test --- Firestore/Source/Core/FSTTransaction.mm | 6 +++--- Firestore/Source/Remote/FSTSerializerBeta.mm | 5 ++--- Firestore/core/src/firebase/firestore/model/precondition.cc | 4 ++-- Firestore/core/src/firebase/firestore/model/precondition.h | 5 +++++ .../core/test/firebase/firestore/model/precondition_test.cc | 4 ++++ 5 files changed, 16 insertions(+), 8 deletions(-) diff --git a/Firestore/Source/Core/FSTTransaction.mm b/Firestore/Source/Core/FSTTransaction.mm index abd27d44b0c..77779f8ebe4 100644 --- a/Firestore/Source/Core/FSTTransaction.mm +++ b/Firestore/Source/Core/FSTTransaction.mm @@ -188,10 +188,10 @@ - (void)updateData:(FSTParsedUpdateData *)data forDocument:(const DocumentKey &) NSError *error = nil; const Precondition precondition = [self preconditionForUpdateWithDocumentKey:key error:&error]; if (precondition.IsNone()) { - [self writeMutations:[data mutationsWithKey:key precondition:precondition]]; - } else { FSTAssert(error, @"Got nil precondition, but error was not set"); self.lastWriteError = error; + } else { + [self writeMutations:[data mutationsWithKey:key precondition:precondition]]; } } @@ -200,7 +200,7 @@ - (void)deleteDocument:(const DocumentKey &)key { initWithKey:key precondition:[self preconditionForDocumentKey:key]] ]]; // Since the delete will be applied before all following writes, we need to ensure that the - // precondition for the next write will be exists: false. + // precondition for the next write will be exists without timestamp. _readVersions[key] = [FSTSnapshotVersion noVersion]; } diff --git a/Firestore/Source/Remote/FSTSerializerBeta.mm b/Firestore/Source/Remote/FSTSerializerBeta.mm index 3c97ca741be..10fbd3deed6 100644 --- a/Firestore/Source/Remote/FSTSerializerBeta.mm +++ b/Firestore/Source/Remote/FSTSerializerBeta.mm @@ -523,11 +523,10 @@ - (FSTMutation *)decodedMutation:(GCFSWrite *)mutation { - (GCFSPrecondition *)encodedPrecondition:(const Precondition &)precondition { FSTAssert(!precondition.IsNone(), @"Can't serialize an empty precondition"); GCFSPrecondition *message = [GCFSPrecondition message]; - if (precondition.update_time() != SnapshotVersion::None()) { + if (precondition.type() == Precondition::Type::UpdateTime) { message.updateTime = [self encodedVersion:static_cast(precondition.update_time())]; - } else if (precondition == Precondition::Exists(true) || - precondition == Precondition::Exists(false)) { + } else if (precondition.type() == Precondition::Type::Exists) { message.exists = precondition == Precondition::Exists(true); } else { FSTFail(@"Unknown precondition: %@", precondition.description()); diff --git a/Firestore/core/src/firebase/firestore/model/precondition.cc b/Firestore/core/src/firebase/firestore/model/precondition.cc index 3cf5351280c..55d1d1280eb 100644 --- a/Firestore/core/src/firebase/firestore/model/precondition.cc +++ b/Firestore/core/src/firebase/firestore/model/precondition.cc @@ -37,8 +37,8 @@ Precondition Precondition::Exists(bool exists) { /* static */ Precondition Precondition::UpdateTime(SnapshotVersion update_time) { - FIREBASE_ASSERT_MESSAGE(update_time != SnapshotVersion::None(), - "Invalid update time"); + // update_time could be SnapshotVersion::None() in particular for locally + // deleted documents. return Precondition{Type::UpdateTime, std::move(update_time), false}; } diff --git a/Firestore/core/src/firebase/firestore/model/precondition.h b/Firestore/core/src/firebase/firestore/model/precondition.h index 896af2408e8..c72ef0aec24 100644 --- a/Firestore/core/src/firebase/firestore/model/precondition.h +++ b/Firestore/core/src/firebase/firestore/model/precondition.h @@ -65,6 +65,10 @@ class Precondition { /** Returns whether this Precondition represents no precondition. */ bool IsNone() const; + Type type() const { + return type_; + } + const SnapshotVersion& update_time() const { return update_time_; } @@ -111,6 +115,7 @@ class Precondition { NSUInteger Hash() const { NSUInteger hash = std::hash()(update_time_.timestamp()); hash = hash * 31 + exists_; + hash = hash * 31 + static_cast(type_); return hash; } diff --git a/Firestore/core/test/firebase/firestore/model/precondition_test.cc b/Firestore/core/test/firebase/firestore/model/precondition_test.cc index 742bc3dd5a7..3ddb2ba198d 100644 --- a/Firestore/core/test/firebase/firestore/model/precondition_test.cc +++ b/Firestore/core/test/firebase/firestore/model/precondition_test.cc @@ -29,6 +29,7 @@ namespace model { TEST(Precondition, None) { const Precondition none = Precondition::None(); + EXPECT_EQ(Precondition::Type::None, none.type()); EXPECT_TRUE(none.IsNone()); EXPECT_EQ(SnapshotVersion::None(), none.update_time()); @@ -41,6 +42,8 @@ TEST(Precondition, None) { TEST(Precondition, Exists) { const Precondition exists = Precondition::Exists(true); const Precondition no_exists = Precondition::Exists(false); + EXPECT_EQ(Precondition::Type::Exists, exists.type()); + EXPECT_EQ(Precondition::Type::Exists, no_exists.type()); EXPECT_FALSE(exists.IsNone()); EXPECT_FALSE(no_exists.IsNone()); EXPECT_EQ(SnapshotVersion::None(), exists.update_time()); @@ -57,6 +60,7 @@ TEST(Precondition, Exists) { TEST(Precondition, UpdateTime) { const Precondition update_time = Precondition::UpdateTime(testutil::Version(1234567)); + EXPECT_EQ(Precondition::Type::UpdateTime, update_time.type()); EXPECT_FALSE(update_time.IsNone()); EXPECT_EQ(testutil::Version(1234567), update_time.update_time()); From f15922a46cc145ea41de43d157ce4bc7dce3decc Mon Sep 17 00:00:00 2001 From: zxu123 Date: Tue, 10 Apr 2018 12:31:30 -0400 Subject: [PATCH 19/22] address changes --- .../firebase/firestore/model/precondition.cc | 10 ------ .../firebase/firestore/model/precondition.h | 33 ++++++++----------- .../firebase/firestore/testutil/testutil.h | 10 +++--- 3 files changed, 19 insertions(+), 34 deletions(-) diff --git a/Firestore/core/src/firebase/firestore/model/precondition.cc b/Firestore/core/src/firebase/firestore/model/precondition.cc index 55d1d1280eb..423d5a2cdfe 100644 --- a/Firestore/core/src/firebase/firestore/model/precondition.cc +++ b/Firestore/core/src/firebase/firestore/model/precondition.cc @@ -16,8 +16,6 @@ #include "Firestore/core/src/firebase/firestore/model/precondition.h" -#include - #include "Firestore/core/src/firebase/firestore/model/maybe_document.h" #include "Firestore/core/src/firebase/firestore/model/snapshot_version.h" #include "Firestore/core/src/firebase/firestore/util/firebase_assert.h" @@ -52,26 +50,18 @@ bool Precondition::IsValidFor(const MaybeDocument& maybe_doc) const { case Type::UpdateTime: return maybe_doc.type() == MaybeDocument::Type::Document && maybe_doc.version() == update_time_; - break; case Type::Exists: if (exists_) { return maybe_doc.type() == MaybeDocument::Type::Document; } else { return maybe_doc.type() == MaybeDocument::Type::NoDocument; } - break; case Type::None: - FIREBASE_ASSERT_MESSAGE(IsNone(), "Precondition should be empty"); return true; - break; } FIREBASE_UNREACHABLE(); } -bool Precondition::IsNone() const { - return type_ == Type::None; -} - } // namespace model } // namespace firestore } // namespace firebase diff --git a/Firestore/core/src/firebase/firestore/model/precondition.h b/Firestore/core/src/firebase/firestore/model/precondition.h index c72ef0aec24..5ff2e8a020a 100644 --- a/Firestore/core/src/firebase/firestore/model/precondition.h +++ b/Firestore/core/src/firebase/firestore/model/precondition.h @@ -17,13 +17,13 @@ #ifndef FIRESTORE_CORE_SRC_FIREBASE_FIRESTORE_MODEL_PRECONDITION_H_ #define FIRESTORE_CORE_SRC_FIREBASE_FIRESTORE_MODEL_PRECONDITION_H_ -#include #include #if defined(__OBJC__) #import "FIRTimestamp.h" #import "Firestore/Source/Core/FSTSnapshotVersion.h" #import "Firestore/Source/Model/FSTDocument.h" +#include "Firestore/core/include/firebase/firestore/timestamp.h"; #endif // defined(__OBJC__) #include "Firestore/core/src/firebase/firestore/model/maybe_document.h" @@ -63,7 +63,9 @@ class Precondition { bool IsValidFor(const MaybeDocument& maybe_doc) const; /** Returns whether this Precondition represents no precondition. */ - bool IsNone() const; + bool IsNone() const { + return type_ == Type::None; + } Type type() const { return type_; @@ -73,6 +75,11 @@ class Precondition { return update_time_; } + bool operator==(const Precondition& other) const { + return type_ == other.type_ && update_time_ == other.update_time_ && + exists_ == other.exists_; + } + #if defined(__OBJC__) // Objective-C requires a default constructor. Precondition() @@ -88,7 +95,6 @@ class Precondition { return [maybe_doc isKindOfClass:[FSTDocument class]] && firebase::firestore::model::SnapshotVersion(maybe_doc.version) == update_time_; - break; case Type::Exists: if (exists_) { return [maybe_doc isKindOfClass:[FSTDocument class]]; @@ -96,20 +102,12 @@ class Precondition { return maybe_doc == nil || [maybe_doc isKindOfClass:[FSTDeletedDocument class]]; } - break; case Type::None: - FIREBASE_ASSERT_MESSAGE(IsNone(), "Precondition should be empty"); return true; - break; } FIREBASE_UNREACHABLE(); } - bool operator==(const Precondition& other) const { - return type_ == other.type_ && update_time_ == other.update_time_ && - exists_ == other.exists_; - } - // For Objective-C++ hash; to be removed after migration. // Do NOT use in C++ code. NSUInteger Hash() const { @@ -123,26 +121,21 @@ class Precondition { switch (type_) { case Type::None: return @">"; - break; case Type::Exists: if (exists_) { return @""; } else { return @""; } - break; case Type::UpdateTime: return [NSString stringWithFormat:@"", update_time_.timestamp().ToString().c_str()]; - break; - default: - // We only raise dev assertion here. This function is mainly used in - // logging. - FIREBASE_DEV_ASSERT_MESSAGE(false, "precondition invalid"); - return @""; - break; } + // We only raise dev assertion here. This function is mainly used in + // logging. + FIREBASE_DEV_ASSERT_MESSAGE(false, "precondition invalid"); + return @""; } #endif // defined(__OBJC__) diff --git a/Firestore/core/test/firebase/firestore/testutil/testutil.h b/Firestore/core/test/firebase/firestore/testutil/testutil.h index 5cb9b59c29d..4d52a269399 100644 --- a/Firestore/core/test/firebase/firestore/testutil/testutil.h +++ b/Firestore/core/test/firebase/firestore/testutil/testutil.h @@ -17,7 +17,8 @@ #ifndef FIRESTORE_CORE_TEST_FIREBASE_FIRESTORE_TESTUTIL_TESTUTIL_H_ #define FIRESTORE_CORE_TEST_FIREBASE_FIRESTORE_TESTUTIL_TESTUTIL_H_ -#include +#include // NOLINT(build/c++11) +#include #include "Firestore/core/include/firebase/firestore/timestamp.h" #include "Firestore/core/src/firebase/firestore/model/document.h" @@ -49,9 +50,10 @@ inline model::ResourcePath Resource(absl::string_view field) { // Version is epoch time in microseconds. This helper is defined so to match // SDKs in other platform. inline model::SnapshotVersion Version(int64_t version) { - return model::SnapshotVersion{ - Timestamp{/*seconds=*/version / 1000, - /*nanoseconds=*/static_cast(version % 1000) * 1000}}; + namespace chr = std::chrono; + auto timepoint = + chr::time_point(chr::microseconds(version)); + return model::SnapshotVersion{Timestamp::FromTimePoint(timepoint)}; } inline model::Document Doc(absl::string_view key, int64_t version) { From e6edf9ac20f3aee47a03b4d8bd3e10c5e11defaa Mon Sep 17 00:00:00 2001 From: zxu123 Date: Tue, 10 Apr 2018 12:37:11 -0400 Subject: [PATCH 20/22] fix lint --- Firestore/Example/Tests/Remote/FSTSerializerBetaTests.mm | 4 ++-- Firestore/Source/Model/FSTMutation.mm | 1 + 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/Firestore/Example/Tests/Remote/FSTSerializerBetaTests.mm b/Firestore/Example/Tests/Remote/FSTSerializerBetaTests.mm index d948a7d2ab9..c0e9cadba3d 100644 --- a/Firestore/Example/Tests/Remote/FSTSerializerBetaTests.mm +++ b/Firestore/Example/Tests/Remote/FSTSerializerBetaTests.mm @@ -16,8 +16,6 @@ #import "Firestore/Source/Remote/FSTSerializerBeta.h" -#include - #import #import #import @@ -25,6 +23,8 @@ #import #import +#include + #import "Firestore/Protos/objc/firestore/local/MaybeDocument.pbobjc.h" #import "Firestore/Protos/objc/firestore/local/Mutation.pbobjc.h" #import "Firestore/Protos/objc/google/firestore/v1beta1/Common.pbobjc.h" diff --git a/Firestore/Source/Model/FSTMutation.mm b/Firestore/Source/Model/FSTMutation.mm index 470d47671ce..d55f4a3ad93 100644 --- a/Firestore/Source/Model/FSTMutation.mm +++ b/Firestore/Source/Model/FSTMutation.mm @@ -17,6 +17,7 @@ #import "Firestore/Source/Model/FSTMutation.h" #include +#include #include #include From ff7703b83dca51f67060cf4f338f64c847bcb045 Mon Sep 17 00:00:00 2001 From: zxu123 Date: Wed, 11 Apr 2018 10:40:09 -0400 Subject: [PATCH 21/22] address changes --- Firestore/core/src/firebase/firestore/model/precondition.h | 2 +- Firestore/core/test/firebase/firestore/testutil/testutil.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Firestore/core/src/firebase/firestore/model/precondition.h b/Firestore/core/src/firebase/firestore/model/precondition.h index 5ff2e8a020a..4ab03c218e5 100644 --- a/Firestore/core/src/firebase/firestore/model/precondition.h +++ b/Firestore/core/src/firebase/firestore/model/precondition.h @@ -23,7 +23,7 @@ #import "FIRTimestamp.h" #import "Firestore/Source/Core/FSTSnapshotVersion.h" #import "Firestore/Source/Model/FSTDocument.h" -#include "Firestore/core/include/firebase/firestore/timestamp.h"; +#include "Firestore/core/include/firebase/firestore/timestamp.h" #endif // defined(__OBJC__) #include "Firestore/core/src/firebase/firestore/model/maybe_document.h" diff --git a/Firestore/core/test/firebase/firestore/testutil/testutil.h b/Firestore/core/test/firebase/firestore/testutil/testutil.h index 4d52a269399..9ce988f49dd 100644 --- a/Firestore/core/test/firebase/firestore/testutil/testutil.h +++ b/Firestore/core/test/firebase/firestore/testutil/testutil.h @@ -18,7 +18,7 @@ #define FIRESTORE_CORE_TEST_FIREBASE_FIRESTORE_TESTUTIL_TESTUTIL_H_ #include // NOLINT(build/c++11) -#include +#include #include "Firestore/core/include/firebase/firestore/timestamp.h" #include "Firestore/core/src/firebase/firestore/model/document.h" From fe1f7770e54e688625152609508493631b6c2910 Mon Sep 17 00:00:00 2001 From: zxu123 Date: Thu, 12 Apr 2018 15:35:57 -0400 Subject: [PATCH 22/22] address changes --- Firestore/Source/Remote/FSTSerializerBeta.mm | 3 +-- .../core/src/firebase/firestore/model/snapshot_version.h | 7 ++----- Firestore/core/test/firebase/firestore/testutil/testutil.h | 7 +++++-- 3 files changed, 8 insertions(+), 9 deletions(-) diff --git a/Firestore/Source/Remote/FSTSerializerBeta.mm b/Firestore/Source/Remote/FSTSerializerBeta.mm index 2c398ae1933..c8b0fa43cbf 100644 --- a/Firestore/Source/Remote/FSTSerializerBeta.mm +++ b/Firestore/Source/Remote/FSTSerializerBeta.mm @@ -526,8 +526,7 @@ - (GCFSPrecondition *)encodedPrecondition:(const Precondition &)precondition { FSTAssert(!precondition.IsNone(), @"Can't serialize an empty precondition"); GCFSPrecondition *message = [GCFSPrecondition message]; if (precondition.type() == Precondition::Type::UpdateTime) { - message.updateTime = - [self encodedVersion:static_cast(precondition.update_time())]; + message.updateTime = [self encodedVersion:precondition.update_time()]; } else if (precondition.type() == Precondition::Type::Exists) { message.exists = precondition == Precondition::Exists(true); } else { diff --git a/Firestore/core/src/firebase/firestore/model/snapshot_version.h b/Firestore/core/src/firebase/firestore/model/snapshot_version.h index 900ae6d43f6..1fbba1c5b4e 100644 --- a/Firestore/core/src/firebase/firestore/model/snapshot_version.h +++ b/Firestore/core/src/firebase/firestore/model/snapshot_version.h @@ -44,11 +44,8 @@ class SnapshotVersion { static const SnapshotVersion& None(); #if defined(__OBJC__) - SnapshotVersion(FSTSnapshotVersion* version) { // NOLINT(runtime/explicit) - if (![version isEqual:[FSTSnapshotVersion noVersion]]) { - timestamp_ = - Timestamp{version.timestamp.seconds, version.timestamp.nanoseconds}; - } + SnapshotVersion(FSTSnapshotVersion* version) // NOLINT(runtime/explicit) + : timestamp_{version.timestamp.seconds, version.timestamp.nanoseconds} { } operator FSTSnapshotVersion*() const { diff --git a/Firestore/core/test/firebase/firestore/testutil/testutil.h b/Firestore/core/test/firebase/firestore/testutil/testutil.h index 9ce988f49dd..9a875f4dbfe 100644 --- a/Firestore/core/test/firebase/firestore/testutil/testutil.h +++ b/Firestore/core/test/firebase/firestore/testutil/testutil.h @@ -47,8 +47,11 @@ inline model::ResourcePath Resource(absl::string_view field) { return model::ResourcePath::FromString(field); } -// Version is epoch time in microseconds. This helper is defined so to match -// SDKs in other platform. +/** + * Creates a snapshot version from the given version timestamp. + * + * @param version a timestamp in microseconds since the epoch. + */ inline model::SnapshotVersion Version(int64_t version) { namespace chr = std::chrono; auto timepoint =