Skip to content

Commit 6e5221b

Browse files
rsgowmanrsgowman
authored
Eliminate 'instanceof' checks for FSTFieldValue (#2651)
Previously, we checked for FSTFieldValue subtypes via: `[value isKindOfClass:[FSTStringValue class]]` Now, we check via a new type parameter on FSTFieldValue, i.e.: `value.type == firebase::firestore::model::FieldValue::Type::String` Or with the appropriate using declaration: `value.type == FieldValue::Type::String` This should enable easier transition to C++.
1 parent 6603b86 commit 6e5221b

File tree

9 files changed

+146
-44
lines changed

9 files changed

+146
-44
lines changed

‎Firestore/Example/Tests/Model/FSTFieldValueTests.mm

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,12 +27,14 @@
2727
#import "Firestore/Example/Tests/Util/FSTHelpers.h"
2828

2929
#include "Firestore/core/src/firebase/firestore/model/database_id.h"
30+
#include "Firestore/core/src/firebase/firestore/model/field_value.h"
3031
#include "Firestore/core/src/firebase/firestore/util/string_apple.h"
3132
#include "Firestore/core/test/firebase/firestore/testutil/testutil.h"
3233

3334
namespace testutil = firebase::firestore::testutil;
3435
namespace util = firebase::firestore::util;
3536
using firebase::firestore::model::DatabaseId;
37+
using firebase::firestore::model::FieldValue;
3638

3739
/** Helper to wrap the values in a set of equality groups using FSTTestFieldValue(). */
3840
NSArray *FSTWrapGroups(NSArray *groups) {
@@ -91,6 +93,7 @@ - (void)testWrapIntegers {
9193
FSTFieldValue *wrapped = FSTTestFieldValue(value);
9294
XCTAssertEqualObjects([wrapped class], [FSTIntegerValue class]);
9395
XCTAssertEqualObjects([wrapped value], @([value longLongValue]));
96+
XCTAssertEqual(wrapped.type, FieldValue::Type::Integer);
9497
}
9598
}
9699

@@ -105,6 +108,7 @@ - (void)testWrapsDoubles {
105108
FSTFieldValue *wrapped = FSTTestFieldValue(value);
106109
XCTAssertEqualObjects([wrapped class], [FSTDoubleValue class]);
107110
XCTAssertEqualObjects([wrapped value], value);
111+
XCTAssertEqual(wrapped.type, FieldValue::Type::Double);
108112
}
109113
}
110114

@@ -113,6 +117,7 @@ - (void)testWrapsNilAndNSNull {
113117
XCTAssertEqual(FSTTestFieldValue(nil), nullValue);
114118
XCTAssertEqual(FSTTestFieldValue([NSNull null]), nullValue);
115119
XCTAssertEqual([nullValue value], [NSNull null]);
120+
XCTAssertEqual(nullValue.type, FieldValue::Type::Null);
116121
}
117122

118123
- (void)testWrapsBooleans {
@@ -121,6 +126,7 @@ - (void)testWrapsBooleans {
121126
FSTFieldValue *wrapped = FSTTestFieldValue(value);
122127
XCTAssertEqualObjects([wrapped class], [FSTBooleanValue class]);
123128
XCTAssertEqualObjects([wrapped value], value);
129+
XCTAssertEqual(wrapped.type, FieldValue::Type::Boolean);
124130
}
125131

126132
// Unsigned chars could conceivably be handled consistently with signed chars but on arm64 these
@@ -219,6 +225,7 @@ - (void)testWrapStrings {
219225
FSTFieldValue *wrapped = FSTTestFieldValue(value);
220226
XCTAssertEqualObjects([wrapped class], [FSTStringValue class]);
221227
XCTAssertEqualObjects([wrapped value], value);
228+
XCTAssertEqual(wrapped.type, FieldValue::Type::String);
222229
}
223230
}
224231

@@ -229,6 +236,7 @@ - (void)testWrapDates {
229236
XCTAssertEqualObjects([wrapped class], [FSTTimestampValue class]);
230237
XCTAssertEqualObjects([[wrapped value] class], [FIRTimestamp class]);
231238
XCTAssertEqualObjects([wrapped value], [FIRTimestamp timestampWithDate:value]);
239+
XCTAssertEqual(wrapped.type, FieldValue::Type::Timestamp);
232240
}
233241
}
234242

@@ -239,6 +247,7 @@ - (void)testWrapGeoPoints {
239247
FSTFieldValue *wrapped = FSTTestFieldValue(value);
240248
XCTAssertEqualObjects([wrapped class], [FSTGeoPointValue class]);
241249
XCTAssertEqualObjects([wrapped value], value);
250+
XCTAssertEqual(wrapped.type, FieldValue::Type::GeoPoint);
242251
}
243252
}
244253

@@ -248,6 +257,7 @@ - (void)testWrapBlobs {
248257
FSTFieldValue *wrapped = FSTTestFieldValue(value);
249258
XCTAssertEqualObjects([wrapped class], [FSTBlobValue class]);
250259
XCTAssertEqualObjects([wrapped value], value);
260+
XCTAssertEqual(wrapped.type, FieldValue::Type::Blob);
251261
}
252262
}
253263

@@ -262,11 +272,13 @@ - (void)testWrapResourceNames {
262272
XCTAssertEqualObjects([wrapped value], [FSTDocumentKey keyWithDocumentKey:value.key]);
263273
XCTAssertTrue(*((FSTReferenceValue *)wrapped).databaseID ==
264274
*(const DatabaseId *)(value.databaseID));
275+
XCTAssertEqual(wrapped.type, FieldValue::Type::Reference);
265276
}
266277
}
267278

268279
- (void)testWrapsEmptyObjects {
269280
XCTAssertEqualObjects(FSTTestFieldValue(@{}), [FSTObjectValue objectValue]);
281+
XCTAssertEqual(FSTTestFieldValue(@{}).type, FieldValue::Type::Object);
270282
}
271283

272284
- (void)testWrapsSimpleObjects {
@@ -279,6 +291,7 @@ - (void)testWrapsSimpleObjects {
279291
@"d" : [FSTNullValue nullValue]
280292
}];
281293
XCTAssertEqualObjects(actual, expected);
294+
XCTAssertEqual(actual.type, FieldValue::Type::Object);
282295
}
283296

284297
- (void)testWrapsNestedObjects {
@@ -291,6 +304,7 @@ - (void)testWrapsNestedObjects {
291304
}]
292305
}];
293306
XCTAssertEqualObjects(actual, expected);
307+
XCTAssertEqual(actual.type, FieldValue::Type::Object);
294308
}
295309

296310
- (void)testExtractsFields {
@@ -414,6 +428,7 @@ - (void)testArrays {
414428

415429
FSTArrayValue *actual = (FSTArrayValue *)FSTTestFieldValue(@[ @"value", @YES ]);
416430
XCTAssertEqualObjects(actual, expected);
431+
XCTAssertEqual(actual.type, FieldValue::Type::Array);
417432
}
418433

419434
- (void)testValueEquality {

‎Firestore/Example/Tests/Util/FSTHelpers.mm

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -147,7 +147,7 @@
147147

148148
FSTObjectValue *FSTTestObjectValue(NSDictionary<NSString *, id> *data) {
149149
FSTFieldValue *wrapped = FSTTestFieldValue(data);
150-
HARD_ASSERT([wrapped isKindOfClass:[FSTObjectValue class]], "Unsupported value: %s", data);
150+
HARD_ASSERT(wrapped.type == FieldValue::Type::Object, "Unsupported value: %s", data);
151151
return (FSTObjectValue *)wrapped;
152152
}
153153

‎Firestore/Source/API/FIRDocumentSnapshot.mm

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434
#include "Firestore/core/src/firebase/firestore/api/firestore.h"
3535
#include "Firestore/core/src/firebase/firestore/model/database_id.h"
3636
#include "Firestore/core/src/firebase/firestore/model/document_key.h"
37+
#include "Firestore/core/src/firebase/firestore/model/field_value.h"
3738
#include "Firestore/core/src/firebase/firestore/util/hard_assert.h"
3839
#include "Firestore/core/src/firebase/firestore/util/string_apple.h"
3940

@@ -42,6 +43,7 @@
4243
using firebase::firestore::api::Firestore;
4344
using firebase::firestore::model::DatabaseId;
4445
using firebase::firestore::model::DocumentKey;
46+
using firebase::firestore::model::FieldValue;
4547
using firebase::firestore::util::WrapNSString;
4648

4749
NS_ASSUME_NONNULL_BEGIN
@@ -185,11 +187,11 @@ - (FSTFieldValueOptions *)optionsForServerTimestampBehavior:
185187
}
186188

187189
- (id)convertedValue:(FSTFieldValue *)value options:(FSTFieldValueOptions *)options {
188-
if ([value isKindOfClass:[FSTObjectValue class]]) {
190+
if (value.type == FieldValue::Type::Object) {
189191
return [self convertedObject:(FSTObjectValue *)value options:options];
190-
} else if ([value isKindOfClass:[FSTArrayValue class]]) {
192+
} else if (value.type == FieldValue::Type::Array) {
191193
return [self convertedArray:(FSTArrayValue *)value options:options];
192-
} else if ([value isKindOfClass:[FSTReferenceValue class]]) {
194+
} else if (value.type == FieldValue::Type::Reference) {
193195
FSTReferenceValue *ref = (FSTReferenceValue *)value;
194196
const DatabaseId *refDatabase = ref.databaseID;
195197
const DatabaseId *database = &_snapshot.firestore()->database_id();

‎Firestore/Source/API/FIRQuery.mm

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@
4242

4343
#include "Firestore/core/src/firebase/firestore/model/document_key.h"
4444
#include "Firestore/core/src/firebase/firestore/model/field_path.h"
45+
#include "Firestore/core/src/firebase/firestore/model/field_value.h"
4546
#include "Firestore/core/src/firebase/firestore/model/resource_path.h"
4647
#include "Firestore/core/src/firebase/firestore/util/error_apple.h"
4748
#include "Firestore/core/src/firebase/firestore/util/hard_assert.h"
@@ -54,6 +55,7 @@
5455
using firebase::firestore::core::ViewSnapshot;
5556
using firebase::firestore::model::DocumentKey;
5657
using firebase::firestore::model::FieldPath;
58+
using firebase::firestore::model::FieldValue;
5759
using firebase::firestore::model::ResourcePath;
5860
using firebase::firestore::util::MakeNSError;
5961
using firebase::firestore::util::StatusOr;
@@ -598,7 +600,7 @@ - (FSTBound *)boundFromSnapshot:(FIRDocumentSnapshot *)snapshot isBefore:(BOOL)i
598600
} else {
599601
FSTFieldValue *value = [document fieldForPath:sortOrder.field];
600602

601-
if ([value isKindOfClass:[FSTServerTimestampValue class]]) {
603+
if (value.type == FieldValue::Type::ServerTimestamp) {
602604
FSTThrowInvalidUsage(@"InvalidQueryException",
603605
@"Invalid query. You are trying to start or end a query using a "
604606
"document for which the field '%s' is an uncommitted server "

‎Firestore/Source/Core/FSTQuery.mm

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828

2929
#include "Firestore/core/src/firebase/firestore/model/document_key.h"
3030
#include "Firestore/core/src/firebase/firestore/model/field_path.h"
31+
#include "Firestore/core/src/firebase/firestore/model/field_value.h"
3132
#include "Firestore/core/src/firebase/firestore/model/resource_path.h"
3233
#include "Firestore/core/src/firebase/firestore/util/hard_assert.h"
3334
#include "Firestore/core/src/firebase/firestore/util/hashing.h"
@@ -36,6 +37,7 @@
3637
namespace util = firebase::firestore::util;
3738
using firebase::firestore::model::DocumentKey;
3839
using firebase::firestore::model::FieldPath;
40+
using firebase::firestore::model::FieldValue;
3941
using firebase::firestore::model::ResourcePath;
4042

4143
NS_ASSUME_NONNULL_BEGIN
@@ -183,7 +185,7 @@ - (BOOL)isEqual:(id)other {
183185

184186
- (BOOL)matchesDocument:(FSTDocument *)document {
185187
if (_field.IsKeyFieldPath()) {
186-
HARD_ASSERT([self.value isKindOfClass:[FSTReferenceValue class]],
188+
HARD_ASSERT(self.value.type == FieldValue::Type::Reference,
187189
"Comparing on key, but filter value not a FSTReferenceValue.");
188190
HARD_ASSERT(self.filterOperator != FSTRelationFilterOperatorArrayContains,
189191
"arrayContains queries don't make sense on document keys.");
@@ -472,7 +474,7 @@ - (BOOL)sortsBeforeDocument:(FSTDocument *)document
472474
FSTSortOrder *sortOrderComponent = sortOrder[idx];
473475
NSComparisonResult comparison;
474476
if (sortOrderComponent.field == FieldPath::KeyFieldPath()) {
475-
HARD_ASSERT([fieldValue isKindOfClass:[FSTReferenceValue class]],
477+
HARD_ASSERT(fieldValue.type == FieldValue::Type::Reference,
476478
"FSTBound has a non-key value where the key path is being used %s", fieldValue);
477479
FSTReferenceValue *refValue = (FSTReferenceValue *)fieldValue;
478480
comparison = CompareKeys(refValue.value.key, document.key);

‎Firestore/Source/Model/FSTFieldValue.h

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,9 @@
2121
#include "Firestore/core/src/firebase/firestore/model/database_id.h"
2222
#include "Firestore/core/src/firebase/firestore/model/field_mask.h"
2323
#include "Firestore/core/src/firebase/firestore/model/field_path.h"
24+
#include "Firestore/core/src/firebase/firestore/model/field_value.h"
25+
26+
using firebase::firestore::model::FieldValue;
2427

2528
@class FSTDocumentKey;
2629
@class FIRTimestamp;
@@ -85,8 +88,14 @@ enum class ServerTimestampBehavior { None, Estimate, Previous };
8588
*/
8689
@interface FSTFieldValue<__covariant T> : NSObject
8790

91+
/**
92+
* Returns the 'type' of this FSTFieldValue. Used for RTTI (rather than isKindOfClass)
93+
* to ease migration to C++.
94+
*/
95+
@property(nonatomic, assign, readonly) FieldValue::Type type;
96+
8897
/** Returns the FSTTypeOrder for this value. */
89-
- (FSTTypeOrder)typeOrder;
98+
@property(nonatomic, assign, readonly) FSTTypeOrder typeOrder;
9099

91100
/**
92101
* Converts an FSTFieldValue into the value that users will see in document snapshots.

0 commit comments

Comments
 (0)