ソースを参照

Migrate FSTReferenceValue to FSTDelegateValue (#3177)

* Add FieldValue::Reference

* Migrate FSTReferenceValue to FSTDelegateValue

* Build Objective-C components with CMake

Build only the things required for FSTFieldValue
Gil 6 年 前
コミット
64d8a40a70

+ 2 - 2
Firestore/Example/Tests/API/FSTUserDataConverterTests.mm

@@ -144,9 +144,9 @@ union DoubleBits {
   ];
   for (FSTDocumentKeyReference *value in values) {
     FSTFieldValue *wrapped = FSTTestFieldValue(value);
-    XCTAssertEqualObjects([wrapped class], [FSTReferenceValue class]);
+    XCTAssertEqualObjects([wrapped class], [FSTDelegateValue class]);
     XCTAssertEqualObjects([wrapped value], [FSTDocumentKey keyWithDocumentKey:value.key]);
-    XCTAssertTrue(((FSTReferenceValue *)wrapped).databaseID == value.databaseID);
+    XCTAssertTrue(((FSTDelegateValue *)wrapped).referenceValue.database_id() == value.databaseID);
     XCTAssertEqual(wrapped.type, FieldValue::Type::Reference);
   }
 }

+ 2 - 6
Firestore/Example/Tests/Model/FSTFieldValueTests.mm

@@ -60,9 +60,7 @@ NSArray *FSTWrapGroups(NSArray *groups) {
       } else if ([value isKindOfClass:[FSTDocumentKeyReference class]]) {
         // We directly convert these here so that the databaseIDs can be different.
         FSTDocumentKeyReference *reference = (FSTDocumentKeyReference *)value;
-        wrappedValue =
-            [FSTReferenceValue referenceValue:[FSTDocumentKey keyWithDocumentKey:reference.key]
-                                   databaseID:reference.databaseID];
+        wrappedValue = FieldValue::FromReference(reference.databaseID, reference.key).Wrap();
       } else {
         wrappedValue = FSTTestFieldValue(value);
       }
@@ -317,9 +315,7 @@ union DoubleBits {
     @[ FSTTestFieldValue(FSTTestGeoPoint(0, 1)), FieldValue::FromGeoPoint(GeoPoint(0, 1)).Wrap() ],
     @[ FSTTestFieldValue(FSTTestGeoPoint(1, 0)) ],
     @[
-      [FSTReferenceValue
-          referenceValue:[FSTDocumentKey keyWithDocumentKey:FSTTestDocKey(@"coll/doc1")]
-              databaseID:database_id],
+      FieldValue::FromReference(database_id, FSTTestDocKey(@"coll/doc1")).Wrap(),
       FSTTestFieldValue(FSTTestRef("project", DatabaseId::kDefault, @"coll/doc1"))
     ],
     @[ FSTTestRef("project", "(default)", @"coll/doc2") ],

+ 3 - 3
Firestore/Source/API/FIRDocumentSnapshot.mm

@@ -193,8 +193,8 @@ ServerTimestampBehavior InternalServerTimestampBehavior(FIRServerTimestampBehavi
   } else if (value.type == FieldValue::Type::Array) {
     return [self convertedArray:(FSTArrayValue *)value options:options];
   } else if (value.type == FieldValue::Type::Reference) {
-    FSTReferenceValue *ref = (FSTReferenceValue *)value;
-    const DatabaseId &refDatabase = ref.databaseID;
+    const auto &ref = ((FSTDelegateValue *)value).referenceValue;
+    const DatabaseId &refDatabase = ref.database_id();
     const DatabaseId &database = _snapshot.firestore()->database_id();
     if (refDatabase != database) {
       // TODO(b/32073923): Log this as a proper warning.
@@ -205,7 +205,7 @@ ServerTimestampBehavior InternalServerTimestampBehavior(FIRServerTimestampBehavi
             refDatabase.database_id().c_str(), database.project_id().c_str(),
             database.database_id().c_str());
     }
-    DocumentKey key = [[ref valueWithOptions:options] key];
+    const DocumentKey &key = ref.key();
     return [[FIRDocumentReference alloc] initWithKey:key firestore:_snapshot.firestore()];
   } else {
     return [value valueWithOptions:options];

+ 3 - 5
Firestore/Source/API/FIRQuery.mm

@@ -467,9 +467,8 @@ FIRQuery *Wrap(Query &&query) {
   // orders), multiple documents could match the position, yielding duplicate results.
   for (FSTSortOrder *sortOrder in self.query.sortOrders) {
     if (sortOrder.field == FieldPath::KeyFieldPath()) {
-      [components addObject:[FSTReferenceValue
-                                referenceValue:[FSTDocumentKey keyWithDocumentKey:document.key]
-                                    databaseID:self.firestore.databaseID]];
+      [components
+          addObject:FieldValue::FromReference(self.firestore.databaseID, document.key).Wrap()];
     } else {
       FSTFieldValue *value = [document fieldForPath:sortOrder.field];
 
@@ -524,8 +523,7 @@ FIRQuery *Wrap(Query &&query) {
                              path.CanonicalString());
       }
       DocumentKey key{path};
-      fieldValue = [FSTReferenceValue referenceValue:[FSTDocumentKey keyWithDocumentKey:key]
-                                          databaseID:self.firestore.databaseID];
+      fieldValue = FieldValue::FromReference(self.firestore.databaseID, key).Wrap();
     }
 
     [components addObject:fieldValue];

+ 1 - 2
Firestore/Source/API/FSTUserDataConverter.mm

@@ -476,8 +476,7 @@ NS_ASSUME_NONNULL_BEGIN
           other.project_id(), other.database_id(), _databaseID.project_id(),
           _databaseID.database_id(), context.FieldDescription());
     }
-    return [FSTReferenceValue referenceValue:[FSTDocumentKey keyWithDocumentKey:reference.key]
-                                  databaseID:_databaseID];
+    return FieldValue::FromReference(_databaseID, reference.key).Wrap();
 
   } else {
     ThrowInvalidArgument("Unsupported type: %s%s", NSStringFromClass([input class]),

+ 5 - 5
Firestore/Source/Core/FSTQuery.mm

@@ -187,11 +187,11 @@ NSString *FSTStringFromQueryRelationOperator(Filter::Operator filterOperator) {
 - (BOOL)matchesDocument:(FSTDocument *)document {
   if (_field.IsKeyFieldPath()) {
     HARD_ASSERT(self.value.type == FieldValue::Type::Reference,
-                "Comparing on key, but filter value not a FSTReferenceValue.");
+                "Comparing on key, but filter value not a Reference.");
     HARD_ASSERT(self.filterOperator != Filter::Operator::ArrayContains,
                 "arrayContains queries don't make sense on document keys.");
-    FSTReferenceValue *refValue = (FSTReferenceValue *)self.value;
-    NSComparisonResult comparison = util::WrapCompare(document.key, refValue.value.key);
+    const auto &ref = ((FSTDelegateValue *)self.value).referenceValue;
+    NSComparisonResult comparison = util::WrapCompare(document.key, ref.key());
     return [self matchesComparison:comparison];
   } else {
     return [self matchesValue:[document fieldForPath:self.field]];
@@ -477,8 +477,8 @@ NSString *FSTStringFromQueryRelationOperator(Filter::Operator filterOperator) {
     if (sortOrderComponent.field == FieldPath::KeyFieldPath()) {
       HARD_ASSERT(fieldValue.type == FieldValue::Type::Reference,
                   "FSTBound has a non-key value where the key path is being used %s", fieldValue);
-      FSTReferenceValue *refValue = (FSTReferenceValue *)fieldValue;
-      comparison = util::Compare(refValue.value.key, document.key);
+      const auto &ref = ((FSTDelegateValue *)fieldValue).referenceValue;
+      comparison = util::Compare(ref.key(), document.key);
     } else {
       FSTFieldValue *docValue = [document fieldForPath:sortOrderComponent.field];
       HARD_ASSERT(docValue != nil,

+ 2 - 11
Firestore/Source/Model/FSTFieldValue.h

@@ -103,17 +103,6 @@ typedef NS_ENUM(NSInteger, FSTTypeOrder) {
 
 @end
 
-/**
- * A reference value stored in Firestore.
- */
-@interface FSTReferenceValue : FSTFieldValue <FSTDocumentKey *>
-
-+ (instancetype)referenceValue:(FSTDocumentKey *)value databaseID:(model::DatabaseId)databaseID;
-
-@property(nonatomic, assign, readonly) const model::DatabaseId &databaseID;
-
-@end
-
 /**
  * A structured object value stored in Firestore.
  */
@@ -190,6 +179,8 @@ typedef NS_ENUM(NSInteger, FSTTypeOrder) {
 @interface FSTDelegateValue : FSTFieldValue <id>
 + (instancetype)delegateWithValue:(model::FieldValue &&)value;
 - (const model::FieldValue &)internalValue;
+
+- (const model::FieldValue::Reference &)referenceValue;
 @end
 
 NS_ASSUME_NONNULL_END

+ 5 - 71
Firestore/Source/Model/FSTFieldValue.mm

@@ -128,76 +128,6 @@ NS_ASSUME_NONNULL_BEGIN
 
 @end
 
-#pragma mark - FSTReferenceValue
-
-@interface FSTReferenceValue ()
-@property(nonatomic, strong, readonly) FSTDocumentKey *key;
-@end
-
-@implementation FSTReferenceValue {
-  DatabaseId _databaseID;
-}
-
-+ (instancetype)referenceValue:(FSTDocumentKey *)value databaseID:(DatabaseId)databaseID {
-  return [[FSTReferenceValue alloc] initWithValue:value databaseID:std::move(databaseID)];
-}
-
-- (id)initWithValue:(FSTDocumentKey *)value databaseID:(DatabaseId)databaseID {
-  self = [super init];
-  if (self) {
-    _key = value;
-    _databaseID = std::move(databaseID);
-  }
-  return self;
-}
-
-- (id)value {
-  return self.key;
-}
-
-- (FieldValue::Type)type {
-  return FieldValue::Type::Reference;
-}
-
-- (FSTTypeOrder)typeOrder {
-  return FSTTypeOrderReference;
-}
-
-- (BOOL)isEqual:(id)other {
-  if (other == self) {
-    return YES;
-  }
-  if (!([other isKindOfClass:[FSTFieldValue class]] &&
-        ((FSTFieldValue *)other).type == FieldValue::Type::Reference)) {
-    return NO;
-  }
-
-  FSTReferenceValue *otherRef = (FSTReferenceValue *)other;
-  return self.key.key == otherRef.key.key && self.databaseID == otherRef.databaseID;
-}
-
-- (NSUInteger)hash {
-  NSUInteger result = self.databaseID.Hash();
-  result = 31 * result + [self.key hash];
-  return result;
-}
-
-- (NSComparisonResult)compare:(FSTFieldValue *)other {
-  if (other.type == FieldValue::Type::Reference) {
-    FSTReferenceValue *ref = (FSTReferenceValue *)other;
-    NSComparisonResult cmp = WrapCompare(self.databaseID.project_id(), ref.databaseID.project_id());
-    if (cmp != NSOrderedSame) {
-      return cmp;
-    }
-    cmp = WrapCompare(self.databaseID.database_id(), ref.databaseID.database_id());
-    return cmp != NSOrderedSame ? cmp : WrapCompare(self.key.key, ref.key.key);
-  } else {
-    return [self defaultCompare:other];
-  }
-}
-
-@end
-
 #pragma mark - FSTObjectValue
 
 /**
@@ -480,6 +410,10 @@ static const NSComparator StringComparator = ^NSComparisonResult(NSString *left,
   return _internalValue;
 }
 
+- (const FieldValue::Reference &)referenceValue {
+  return _internalValue.reference_value();
+}
+
 - (id)initWithValue:(FieldValue &&)value {
   self = [super init];
   if (self) {
@@ -560,7 +494,7 @@ static const NSComparator StringComparator = ^NSComparisonResult(NSString *left,
     case FieldValue::Type::Blob:
       return MakeNSData(self.internalValue.blob_value());
     case FieldValue::Type::Reference:
-      HARD_FAIL("TODO(rsgowman): implement");
+      return [FSTDocumentKey keyWithDocumentKey:self.referenceValue.key()];
     case FieldValue::Type::GeoPoint:
       return MakeFIRGeoPoint(self.internalValue.geo_point_value());
     case FieldValue::Type::Array:

+ 4 - 6
Firestore/Source/Remote/FSTSerializerBeta.mm

@@ -222,9 +222,8 @@ NS_ASSUME_NONNULL_BEGIN
     case FieldValue::Type::Blob:
       return [self encodedBlobValue:[fieldValue value]];
     case FieldValue::Type::Reference: {
-      FSTReferenceValue *ref = (FSTReferenceValue *)fieldValue;
-      DocumentKey key = [[ref value] key];
-      return [self encodedReferenceValueForDatabaseID:[ref databaseID] key:key];
+      const auto &ref = ((FSTDelegateValue *)fieldValue).referenceValue;
+      return [self encodedReferenceValueForDatabaseID:ref.database_id() key:ref.key()];
     }
     case FieldValue::Type::GeoPoint:
       return [self encodedGeoPointValue:[fieldValue value]];
@@ -345,7 +344,7 @@ NS_ASSUME_NONNULL_BEGIN
   return result;
 }
 
-- (FSTReferenceValue *)decodedReferenceValue:(NSString *)resourceName {
+- (FSTFieldValue *)decodedReferenceValue:(NSString *)resourceName {
   const ResourcePath path = [self decodedResourcePathWithDatabaseID:resourceName];
   const std::string &project = path[1];
   const std::string &database = path[3];
@@ -355,8 +354,7 @@ NS_ASSUME_NONNULL_BEGIN
   HARD_ASSERT(database_id == _databaseID, "Database %s:%s cannot encode reference from %s:%s",
               _databaseID.project_id(), _databaseID.database_id(), database_id.project_id(),
               database_id.database_id());
-  return [FSTReferenceValue referenceValue:[FSTDocumentKey keyWithDocumentKey:key]
-                                databaseID:_databaseID];
+  return FieldValue::FromReference(_databaseID, key).Wrap();
 }
 
 - (GCFSArrayValue *)encodedArrayValue:(FSTArrayValue *)arrayValue {

+ 3 - 3
Firestore/core/src/firebase/firestore/api/query_core.mm

@@ -215,9 +215,9 @@ Query Query::Filter(FieldPath field_path,
             "is not because it has an odd number of segments.",
             path.CanonicalString());
       }
-      field_value = [FSTReferenceValue
-          referenceValue:[FSTDocumentKey keyWithDocumentKey:DocumentKey{path}]
-              databaseID:firestore_->database_id()];
+      field_value = FieldValue::FromReference(firestore_->database_id(),
+                                              DocumentKey{path})
+                        .Wrap();
     } else if (field_value.type != FieldValue::Type::Reference) {
       ThrowInvalidArgument(
           "Invalid query. When querying by document ID you must provide a "

+ 29 - 11
Firestore/core/src/firebase/firestore/model/field_value.cc

@@ -42,6 +42,7 @@ namespace model {
 namespace {
 
 using BaseValue = FieldValue::BaseValue;
+using Reference = FieldValue::Reference;
 using ServerTimestamp = FieldValue::ServerTimestamp;
 using Type = FieldValue::Type;
 
@@ -332,8 +333,8 @@ class BlobValue : public SimpleFieldValue<Type::Blob, ByteString> {
 
 class ReferenceValue : public FieldValue::BaseValue {
  public:
-  ReferenceValue(DatabaseId database_id, DocumentKey key)
-      : database_id_(std::move(database_id)), key_(std::move(key)) {
+  explicit ReferenceValue(Reference reference)
+      : reference_(std::move(reference)) {
   }
 
   Type type() const override {
@@ -344,7 +345,8 @@ class ReferenceValue : public FieldValue::BaseValue {
     if (type() != other.type()) return false;
 
     auto& other_value = Cast<ReferenceValue>(other);
-    return database_id_ == other_value.database_id_ && key_ == other_value.key_;
+    return database_id() == other_value.database_id() &&
+           key() == other_value.key();
   }
 
   ComparisonResult CompareTo(const BaseValue& other) const override {
@@ -352,23 +354,34 @@ class ReferenceValue : public FieldValue::BaseValue {
     if (!util::Same(cmp)) return cmp;
 
     auto& other_value = Cast<ReferenceValue>(other);
-    cmp = Compare(database_id_, other_value.database_id_);
+    cmp = Compare(database_id(), other_value.database_id());
     if (!util::Same(cmp)) return cmp;
 
-    return Compare(key_, other_value.key_);
+    return Compare(key(), other_value.key());
   }
 
   std::string ToString() const override {
-    return absl::StrCat("Reference(key=", key_.ToString(), ")");
+    return absl::StrCat("Reference(key=", key().ToString(), ")");
   }
 
   size_t Hash() const override {
-    return util::Hash(database_id_, key_);
+    return util::Hash(database_id(), key());
+  }
+
+  const Reference& value() const {
+    return reference_;
+  }
+
+  const DatabaseId& database_id() const {
+    return reference_.database_id();
+  }
+
+  const DocumentKey& key() const {
+    return reference_.key();
   }
 
  private:
-  DatabaseId database_id_;
-  DocumentKey key_;
+  Reference reference_;
 };
 
 class GeoPointValue : public BaseValue {
@@ -548,6 +561,11 @@ const ByteString& FieldValue::blob_value() const {
   return Cast<BlobValue>(*rep_).value();
 }
 
+const Reference& FieldValue::reference_value() const {
+  HARD_ASSERT(type() == Type::Reference);
+  return Cast<ReferenceValue>(*rep_).value();
+}
+
 const GeoPoint& FieldValue::geo_point_value() const {
   HARD_ASSERT(type() == Type::GeoPoint);
   return Cast<GeoPointValue>(*rep_).value();
@@ -712,8 +730,8 @@ FieldValue FieldValue::FromBlob(ByteString blob) {
 }
 
 FieldValue FieldValue::FromReference(DatabaseId database_id, DocumentKey key) {
-  return FieldValue(
-      std::make_shared<ReferenceValue>(std::move(database_id), std::move(key)));
+  return FieldValue(std::make_shared<ReferenceValue>(
+      Reference(std::move(database_id), std::move(key))));
 }
 
 FieldValue FieldValue::FromGeoPoint(const GeoPoint& value) {

+ 22 - 0
Firestore/core/src/firebase/firestore/model/field_value.h

@@ -52,6 +52,7 @@ class ObjectValue;
  */
 class FieldValue {
  public:
+  class Reference;
   class ServerTimestamp;
   using Array = std::vector<FieldValue>;
   using Map = immutable::SortedMap<std::string, FieldValue>;
@@ -123,6 +124,8 @@ class FieldValue {
 
   const nanopb::ByteString& blob_value() const;
 
+  const Reference& reference_value() const;
+
   const GeoPoint& geo_point_value() const;
 
   const Array& array_value() const;
@@ -290,6 +293,25 @@ class ObjectValue : public util::Comparable<ObjectValue> {
   FieldValue fv_;
 };
 
+class FieldValue::Reference {
+ public:
+  Reference(DatabaseId database_id, DocumentKey key)
+      : database_id_(std::move(database_id)), key_(std::move(key)) {
+  }
+
+  const DatabaseId& database_id() const {
+    return database_id_;
+  }
+
+  const DocumentKey& key() const {
+    return key_;
+  }
+
+ private:
+  DatabaseId database_id_;
+  DocumentKey key_;
+};
+
 class FieldValue::ServerTimestamp {
  public:
   explicit ServerTimestamp(Timestamp local_write_time,

+ 0 - 1
Firestore/core/test/firebase/firestore/testutil/xcgmock.h

@@ -183,7 +183,6 @@ OBJC_PRINT_TO(FSTObjectValue);
 OBJC_PRINT_TO(FSTPatchMutation);
 OBJC_PRINT_TO(FSTQuery);
 OBJC_PRINT_TO(FSTQueryData);
-OBJC_PRINT_TO(FSTReferenceValue);
 OBJC_PRINT_TO(FSTRelationFilter);
 OBJC_PRINT_TO(FSTSerializerBeta);
 OBJC_PRINT_TO(FSTServerTimestampFieldValue);