From db717bf6704b444b093d46f53935402c83441854 Mon Sep 17 00:00:00 2001 From: zxu Date: Thu, 5 Apr 2018 11:52:07 -0400 Subject: Port transform operations to C++ (#1020) * port FieldMask to C++ * address changes * address changes * fix test * address change * Port transform operations (FSTTransformOperation, FSTServerTimestampTransform) to C++ * address changes * address changes * address changes * address change --- Firestore/Example/Tests/Util/FSTHelpers.mm | 8 +- Firestore/Source/API/FSTUserDataConverter.mm | 5 +- Firestore/Source/Model/FSTMutation.h | 19 ++--- Firestore/Source/Model/FSTMutation.mm | 54 ++++-------- Firestore/Source/Remote/FSTSerializerBeta.mm | 18 ++-- .../src/firebase/firestore/model/CMakeLists.txt | 1 + .../firestore/model/transform_operations.h | 97 ++++++++++++++++++++++ .../test/firebase/firestore/model/CMakeLists.txt | 1 + .../firestore/model/transform_operations_test.cc | 51 ++++++++++++ 9 files changed, 196 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 8a7306c..cc120a9 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::FieldMask; 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,9 @@ FSTTransformMutation *FSTTestTransformMutation(NSString *path, NSMutableArray *fieldTransforms = [NSMutableArray array]; for (NSString *field in serverTimestampFields) { const FieldPath fieldPath = testutil::Field(util::MakeStringView(field)); - id transformOp = [FSTServerTimestampTransform serverTimestampTransform]; + auto 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 34f1015..c6a6e34 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::DatabaseId; 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 @@ typedef NS_ENUM(NSInteger, FSTUserDataSource) { [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 2b81af6..2fa8806 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 df95155..a18053c 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; } @@ -82,13 +61,12 @@ NS_ASSUME_NONNULL_BEGIN if (other == self) return YES; if (![[other class] isEqual:[self class]]) return NO; FSTFieldTransform *otherFieldTransform = other; - return self.path == otherFieldTransform.path && - [self.transform isEqual:otherFieldTransform.transform]; + return self.path == otherFieldTransform.path && *self.transform == *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 +74,10 @@ NS_ASSUME_NONNULL_BEGIN return _path; } +- (const firebase::firestore::model::TransformOperation *)transform { + return _transform.get(); +} + @end #pragma mark - FSTPrecondition @@ -505,7 +487,7 @@ NS_ASSUME_NONNULL_BEGIN 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 +511,12 @@ NS_ASSUME_NONNULL_BEGIN 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 3b6f052..5433154 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::DocumentKey; 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 @@ NS_ASSUME_NONNULL_BEGIN (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 @@ NS_ASSUME_NONNULL_BEGIN 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 f6c5efe..d83c0aa 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/transform_operations.h b/Firestore/core/src/firebase/firestore/model/transform_operations.h new file mode 100644 index 0000000..6ff4dbc --- /dev/null +++ b/Firestore/core/src/firebase/firestore/model/transform_operations.h @@ -0,0 +1,97 @@ +/* + * 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, + Test, // Purely for test purpose. + }; + + virtual ~TransformOperation() { + } + + /** Returns the actual type. */ + virtual Type type() const = 0; + + /** 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. + virtual NSUInteger Hash() const = 0; +#endif // defined(__OBJC__) +}; + +/** Transforms a value into a server-generated timestamp. */ +class ServerTimestampTransform : public TransformOperation { + public: + ~ServerTimestampTransform() override { + } + + 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 59a1f03..5e494f1 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 0000000..0ef95db --- /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 Type::Test; + } + + 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_EQ(transform, another); + EXPECT_NE(transform, dummy); +} + +} // namespace model +} // namespace firestore +} // namespace firebase -- cgit v1.2.3