Address API proposal feedback.

ehsannas · ehsannas · commit fcb6ebb5fa08 · 2025-05-09T16:31:44.000-07:00
diff --git a/Firestore/Example/Tests/API/FIRBsonTypesUnitTests.mm b/Firestore/Example/Tests/API/FIRBsonTypesUnitTests.mm
@@ -33,15 +33,15 @@ @interface FIRBsonTypesUnitTests : XCTestCase
 @implementation FIRBsonTypesUnitTests
 
 - (void)testMinKeySingleton {
-  FIRMinKey *minKey1 = [FIRMinKey instance];
-  FIRMinKey *minKey2 = [FIRMinKey instance];
+  FIRMinKey *minKey1 = [FIRMinKey shared];
+  FIRMinKey *minKey2 = [FIRMinKey shared];
   XCTAssertEqual(minKey1, minKey2);
   XCTAssertTrue([minKey1 isEqual:minKey2]);
 }
 
 - (void)testMaxKeySingleton {
-  FIRMaxKey *maxKey1 = [FIRMaxKey instance];
-  FIRMaxKey *maxKey2 = [FIRMaxKey instance];
+  FIRMaxKey *maxKey1 = [FIRMaxKey shared];
+  FIRMaxKey *maxKey2 = [FIRMaxKey shared];
   XCTAssertEqual(maxKey1, maxKey2);
   XCTAssertTrue([maxKey1 isEqual:maxKey2]);
 }
@@ -128,15 +128,15 @@ - (void)testCreateAndReadAndCompareBsonBinaryData {
 }
 
 - (void)testFieldValueMinKey {
-  FIRMinKey *minKey1 = [FIRMinKey instance];
-  FIRMinKey *minKey2 = [FIRMinKey instance];
+  FIRMinKey *minKey1 = [FIRMinKey shared];
+  FIRMinKey *minKey2 = [FIRMinKey shared];
   XCTAssertEqual(minKey1, minKey2);
   XCTAssertTrue([minKey1 isEqual:minKey2]);
 }
 
 - (void)testFieldValueMaxKey {
-  FIRMaxKey *maxKey1 = [FIRMaxKey instance];
-  FIRMaxKey *maxKey2 = [FIRMaxKey instance];
+  FIRMaxKey *maxKey1 = [FIRMaxKey shared];
+  FIRMaxKey *maxKey2 = [FIRMaxKey shared];
   XCTAssertEqual(maxKey1, maxKey2);
   XCTAssertTrue([maxKey1 isEqual:maxKey2]);
 }
diff --git a/Firestore/Source/API/FIRFieldValue.mm b/Firestore/Source/API/FIRFieldValue.mm
@@ -189,11 +189,11 @@ + (nonnull FIRVectorValue *)vectorWithArray:(nonnull NSArray<NSNumber *> *)array
 }
 
 + (nonnull FIRMinKey *)minKey {
-  return [FIRMinKey instance];
+  return [FIRMinKey shared];
 }
 
 + (nonnull FIRMaxKey *)maxKey {
-  return [FIRMaxKey instance];
+  return [FIRMaxKey shared];
 }
 
 + (nonnull FIRRegexValue *)regexWithPattern:(nonnull NSString *)pattern
diff --git a/Firestore/Source/API/FIRMaxKey.mm b/Firestore/Source/API/FIRMaxKey.mm
@@ -22,7 +22,7 @@ @implementation FIRMaxKey
 static FIRMaxKey *sharedInstance = nil;
 static dispatch_once_t onceToken;
 
-+ (FIRMaxKey *)instance {
++ (FIRMaxKey *)shared {
   dispatch_once(&onceToken, ^{
     sharedInstance = [[self alloc] init];
   });
diff --git a/Firestore/Source/API/FIRMinKey.mm b/Firestore/Source/API/FIRMinKey.mm
@@ -22,7 +22,7 @@ @implementation FIRMinKey
 static FIRMinKey *sharedInstance = nil;
 static dispatch_once_t onceToken;
 
-+ (FIRMinKey *)instance {
++ (FIRMinKey *)shared {
   dispatch_once(&onceToken, ^{
     sharedInstance = [[self alloc] init];
   });
diff --git a/Firestore/Source/API/FSTUserDataWriter.mm b/Firestore/Source/API/FSTUserDataWriter.mm
@@ -122,9 +122,9 @@ - (id)convertedValue:(const google_firestore_v1_Value &)value {
       return MakeFIRGeoPoint(
           GeoPoint(value.geo_point_value.latitude, value.geo_point_value.longitude));
     case TypeOrder::kMinKey:
-      return [FIRMinKey instance];
+      return [FIRMinKey shared];
     case TypeOrder::kMaxKey:
-      return [FIRMaxKey instance];
+      return [FIRMaxKey shared];
     case TypeOrder::kRegex:
       return [self convertedRegex:value.map_value];
     case TypeOrder::kBsonObjectId:
diff --git a/Firestore/Source/Public/FirebaseFirestore/FIRBsonBinaryData.h b/Firestore/Source/Public/FirebaseFirestore/FIRBsonBinaryData.h
@@ -23,7 +23,8 @@ NS_ASSUME_NONNULL_BEGIN
  */
 NS_SWIFT_SENDABLE
 NS_SWIFT_NAME(BsonBinaryData)
-@interface FIRBsonBinaryData : NSObject <NSCopying>
+__attribute__((objc_subclassing_restricted))
+@interface FIRBsonBinaryData : NSObject<NSCopying>
 
 /** An 8-bit unsigned integer denoting the subtype of the data. */
 @property(atomic, readonly) uint8_t subtype;
diff --git a/Firestore/Source/Public/FirebaseFirestore/FIRBsonObjectId.h b/Firestore/Source/Public/FirebaseFirestore/FIRBsonObjectId.h
@@ -23,7 +23,8 @@ NS_ASSUME_NONNULL_BEGIN
  */
 NS_SWIFT_SENDABLE
 NS_SWIFT_NAME(BsonObjectId)
-@interface FIRBsonObjectId : NSObject <NSCopying>
+__attribute__((objc_subclassing_restricted))
+@interface FIRBsonObjectId : NSObject<NSCopying>
 
 /** The 24-character hex string representation of the ObjectId. */
 @property(atomic, copy, readonly) NSString *value;
diff --git a/Firestore/Source/Public/FirebaseFirestore/FIRBsonTimestamp.h b/Firestore/Source/Public/FirebaseFirestore/FIRBsonTimestamp.h
@@ -24,7 +24,8 @@ NS_ASSUME_NONNULL_BEGIN
  */
 NS_SWIFT_SENDABLE
 NS_SWIFT_NAME(BsonTimestamp)
-@interface FIRBsonTimestamp : NSObject <NSCopying>
+__attribute__((objc_subclassing_restricted))
+@interface FIRBsonTimestamp : NSObject<NSCopying>
 
 /** The underlying unsigned 32-bit integer for seconds */
 @property(atomic, readonly) uint32_t seconds;
diff --git a/Firestore/Source/Public/FirebaseFirestore/FIRInt32Value.h b/Firestore/Source/Public/FirebaseFirestore/FIRInt32Value.h
@@ -23,7 +23,8 @@ NS_ASSUME_NONNULL_BEGIN
  */
 NS_SWIFT_SENDABLE
 NS_SWIFT_NAME(Int32Value)
-@interface FIRInt32Value : NSObject <NSCopying>
+__attribute__((objc_subclassing_restricted))
+@interface FIRInt32Value : NSObject<NSCopying>
 
 /** The 32-bit integer value. */
 @property(atomic, assign, readonly) int32_t value;
diff --git a/Firestore/Source/Public/FirebaseFirestore/FIRMaxKey.h b/Firestore/Source/Public/FirebaseFirestore/FIRMaxKey.h
@@ -23,10 +23,11 @@ NS_ASSUME_NONNULL_BEGIN
  */
 NS_SWIFT_SENDABLE
 NS_SWIFT_NAME(MaxKey)
+__attribute__((objc_subclassing_restricted))
 @interface FIRMaxKey : NSObject
 
 /** Returns the only instance of MaxKey. */
-+ (FIRMaxKey *)instance;
+@property(class, readonly) FIRMaxKey *shared;
 
 /** Returns true if the given object is equal to this, and false otherwise. */
 - (BOOL)isEqual:(id)object;
diff --git a/Firestore/Source/Public/FirebaseFirestore/FIRMinKey.h b/Firestore/Source/Public/FirebaseFirestore/FIRMinKey.h
@@ -23,10 +23,11 @@ NS_ASSUME_NONNULL_BEGIN
  */
 NS_SWIFT_SENDABLE
 NS_SWIFT_NAME(MinKey)
+__attribute__((objc_subclassing_restricted))
 @interface FIRMinKey : NSObject
 
-/** Returns the only instance of MinKey. */
-+ (FIRMinKey *)instance;
+/** The only instance of MinKey. */
+@property(class, readonly) FIRMinKey *shared;
 
 /** Returns true if the given object is equal to this, and false otherwise. */
 - (BOOL)isEqual:(id)object;
diff --git a/Firestore/Source/Public/FirebaseFirestore/FIRRegexValue.h b/Firestore/Source/Public/FirebaseFirestore/FIRRegexValue.h
@@ -23,7 +23,8 @@ NS_ASSUME_NONNULL_BEGIN
  */
 NS_SWIFT_SENDABLE
 NS_SWIFT_NAME(RegexValue)
-@interface FIRRegexValue : NSObject <NSCopying>
+__attribute__((objc_subclassing_restricted))
+@interface FIRRegexValue : NSObject<NSCopying>
 
 /** The regular expression pattern */
 @property(atomic, copy, readonly) NSString *pattern;
diff --git a/Firestore/Swift/Tests/Integration/BsonTypesIntegrationTests.swift b/Firestore/Swift/Tests/Integration/BsonTypesIntegrationTests.swift
@@ -107,8 +107,8 @@ class BsonTypesIntegrationTests: FSTIntegrationTestCase {
       "binary": BsonBinaryData(subtype: 1, data: Data([1, 2, 3])),
       "objectId": BsonObjectId("507f191e810c19729de860ea"),
       "int32": Int32Value(1),
-      "min": MinKey.instance(),
-      "max": MaxKey.instance(),
+      "min": MinKey.shared,
+      "max": MaxKey.shared,
       "regex": RegexValue(pattern: "^foo", options: "i"),
     ])
 
@@ -129,11 +129,11 @@ class BsonTypesIntegrationTests: FSTIntegrationTestCase {
     )
     XCTAssertEqual(
       snapshot.get("min") as? MinKey,
-      MinKey.instance()
+      MinKey.shared
     )
     XCTAssertEqual(
       snapshot.get("max") as? MaxKey,
-      MaxKey.instance()
+      MaxKey.shared
     )
     XCTAssertEqual(
       snapshot.get("binary") as? BsonBinaryData,
@@ -160,8 +160,8 @@ class BsonTypesIntegrationTests: FSTIntegrationTestCase {
       "binary": BsonBinaryData(subtype: 1, data: Data([1, 2, 3])),
       "objectId": BsonObjectId("507f191e810c19729de860ea"),
       "int32": Int32Value(1),
-      "min": MinKey.instance(),
-      "max": MaxKey.instance(),
+      "min": MinKey.shared,
+      "max": MaxKey.shared,
       "regex": RegexValue(pattern: "^foo", options: "i"),
     ])
     ref.updateData([
@@ -181,11 +181,11 @@ class BsonTypesIntegrationTests: FSTIntegrationTestCase {
     )
     XCTAssertEqual(
       snapshot.get("min") as? MinKey,
-      MinKey.instance()
+      MinKey.shared
     )
     XCTAssertEqual(
       snapshot.get("max") as? MaxKey,
-      MaxKey.instance()
+      MaxKey.shared
     )
     XCTAssertEqual(
       snapshot.get("binary") as? BsonBinaryData,
@@ -367,18 +367,18 @@ class BsonTypesIntegrationTests: FSTIntegrationTestCase {
 
   func testCanFilterAndOrderMinKeyValues() async throws {
     let testDocs: [String: [String: Any]] = [
-      "a": ["key": MinKey.instance()],
-      "b": ["key": MinKey.instance()],
+      "a": ["key": MinKey.shared],
+      "b": ["key": MinKey.shared],
       "c": ["key": NSNull()],
       "d": ["key": 1],
-      "e": ["key": MaxKey.instance()],
+      "e": ["key": MaxKey.shared],
     ]
 
     let collection = collectionRef()
     await setDocumentData(testDocs, toCollection: collection)
 
     var query = collection
-      .whereField("key", isEqualTo: MinKey.instance())
+      .whereField("key", isEqualTo: MinKey.shared)
       .order(by: "key", descending: true)
     try await assertSdkQueryResultsConsistentWithBackend(
       testDocs,
@@ -390,7 +390,7 @@ class BsonTypesIntegrationTests: FSTIntegrationTestCase {
     // TODO(b/410032145): This currently fails, and is fixed by
     // PR #14704. Uncomment this when moving to the main branch.
     // var query2 = collection
-    //   .whereField("key", isNotEqualTo: MinKey.instance())
+    //   .whereField("key", isNotEqualTo: MinKey.shared))
     //   .order(by: "key")
     // try await assertSdkQueryResultsConsistentWithBackend(
     //   testDocs,
@@ -400,7 +400,7 @@ class BsonTypesIntegrationTests: FSTIntegrationTestCase {
     // )
 
     query = collection
-      .whereField("key", isGreaterThanOrEqualTo: MinKey.instance())
+      .whereField("key", isGreaterThanOrEqualTo: MinKey.shared)
       .order(by: "key")
     try await assertSdkQueryResultsConsistentWithBackend(
       testDocs,
@@ -410,7 +410,7 @@ class BsonTypesIntegrationTests: FSTIntegrationTestCase {
     )
 
     query = collection
-      .whereField("key", isLessThanOrEqualTo: MinKey.instance())
+      .whereField("key", isLessThanOrEqualTo: MinKey.shared)
       .order(by: "key")
     try await assertSdkQueryResultsConsistentWithBackend(
       testDocs,
@@ -420,7 +420,7 @@ class BsonTypesIntegrationTests: FSTIntegrationTestCase {
     )
 
     query = collection
-      .whereField("key", isGreaterThan: MinKey.instance())
+      .whereField("key", isGreaterThan: MinKey.shared)
       .order(by: "key")
     try await assertSdkQueryResultsConsistentWithBackend(
       testDocs,
@@ -430,7 +430,7 @@ class BsonTypesIntegrationTests: FSTIntegrationTestCase {
     )
 
     query = collection
-      .whereField("key", isLessThan: MinKey.instance())
+      .whereField("key", isLessThan: MinKey.shared)
       .order(by: "key")
     try await assertSdkQueryResultsConsistentWithBackend(
       testDocs,
@@ -452,18 +452,18 @@ class BsonTypesIntegrationTests: FSTIntegrationTestCase {
 
   func testCanFilterAndOrderMaxKeyValues() async throws {
     let testDocs: [String: [String: Any]] = [
-      "a": ["key": MinKey.instance()],
+      "a": ["key": MinKey.shared],
       "b": ["key": 1],
-      "c": ["key": MaxKey.instance()],
-      "d": ["key": MaxKey.instance()],
+      "c": ["key": MaxKey.shared],
+      "d": ["key": MaxKey.shared],
       "e": ["key": NSNull()],
     ]
 
     let collection = collectionRef()
     await setDocumentData(testDocs, toCollection: collection)
 
     var query = collection
-      .whereField("key", isEqualTo: MaxKey.instance())
+      .whereField("key", isEqualTo: MaxKey.shared)
       .order(by: "key")
     try await assertSdkQueryResultsConsistentWithBackend(
       testDocs,
@@ -475,7 +475,7 @@ class BsonTypesIntegrationTests: FSTIntegrationTestCase {
     // TODO(b/410032145): This currently fails, and is fixed by
     // PR #14704. Uncomment this when moving to the main branch.
     // query = collection
-    //   .whereField("key", isNotEqualTo: MaxKey.instance())
+    //   .whereField("key", isNotEqualTo: MaxKey.shared))
     //   .order(by: "key")
     // try await assertSdkQueryResultsConsistentWithBackend(
     //   testDocs,
@@ -485,7 +485,7 @@ class BsonTypesIntegrationTests: FSTIntegrationTestCase {
     // )
 
     query = collection
-      .whereField("key", isGreaterThanOrEqualTo: MaxKey.instance())
+      .whereField("key", isGreaterThanOrEqualTo: MaxKey.shared)
       .order(by: "key")
     try await assertSdkQueryResultsConsistentWithBackend(
       testDocs,
@@ -495,7 +495,7 @@ class BsonTypesIntegrationTests: FSTIntegrationTestCase {
     )
 
     query = collection
-      .whereField("key", isLessThanOrEqualTo: MaxKey.instance())
+      .whereField("key", isLessThanOrEqualTo: MaxKey.shared)
       .order(by: "key")
     try await assertSdkQueryResultsConsistentWithBackend(
       testDocs,
@@ -505,7 +505,7 @@ class BsonTypesIntegrationTests: FSTIntegrationTestCase {
     )
 
     query = collection
-      .whereField("key", isGreaterThan: MaxKey.instance())
+      .whereField("key", isGreaterThan: MaxKey.shared)
       .order(by: "key")
     try await assertSdkQueryResultsConsistentWithBackend(
       testDocs,
@@ -515,7 +515,7 @@ class BsonTypesIntegrationTests: FSTIntegrationTestCase {
     )
 
     query = collection
-      .whereField("key", isLessThan: MaxKey.instance())
+      .whereField("key", isLessThan: MaxKey.shared)
       .order(by: "key")
     try await assertSdkQueryResultsConsistentWithBackend(
       testDocs,
@@ -537,11 +537,11 @@ class BsonTypesIntegrationTests: FSTIntegrationTestCase {
 
   func testCanHandleNullWithBsonValues() async throws {
     let testDocs: [String: [String: Any]] = [
-      "a": ["key": MinKey.instance()],
+      "a": ["key": MinKey.shared],
       "b": ["key": NSNull()],
       "c": ["key": NSNull()],
       "d": ["key": 1],
-      "e": ["key": MaxKey.instance()],
+      "e": ["key": MaxKey.shared],
     ]
 
     let collection = collectionRef()
@@ -587,10 +587,10 @@ class BsonTypesIntegrationTests: FSTIntegrationTestCase {
       "int32Value1": ["key": Int32Value(-1)],
       "int32Value2": ["key": Int32Value(1)],
       "int32Value3": ["key": Int32Value(0)],
-      "minKey1": ["key": MinKey.instance()],
-      "minKey2": ["key": MinKey.instance()],
-      "maxKey1": ["key": MaxKey.instance()],
-      "maxKey2": ["key": MaxKey.instance()],
+      "minKey1": ["key": MinKey.shared],
+      "minKey2": ["key": MinKey.shared],
+      "maxKey1": ["key": MaxKey.shared],
+      "maxKey2": ["key": MaxKey.shared],
     ]
 
     let collection = collectionRef()
@@ -628,7 +628,7 @@ class BsonTypesIntegrationTests: FSTIntegrationTestCase {
     let collection = collectionRef()
     let testDocs: [String: [String: Any]] = [
       "nullValue": ["key": NSNull()],
-      "minValue": ["key": MinKey.instance()],
+      "minValue": ["key": MinKey.shared],
       "booleanValue": ["key": true],
       "nanValue": ["key": Double.nan],
       "int32Value": ["key": Int32Value(1)],
@@ -646,7 +646,7 @@ class BsonTypesIntegrationTests: FSTIntegrationTestCase {
       "arrayValue": ["key": [1, 2]],
       "vectorValue": ["key": VectorValue([1.0, 2.0])],
       "objectValue": ["key": ["a": 1]],
-      "maxValue": ["key": MaxKey.instance()],
+      "maxValue": ["key": MaxKey.shared],
     ]
 
     for (docId, data) in testDocs {
diff --git a/Firestore/Swift/Tests/Integration/CodableIntegrationTests.swift b/Firestore/Swift/Tests/Integration/CodableIntegrationTests.swift
diff --git a/Firestore/Swift/Tests/Integration/SnapshotListenerSourceTests.swift b/Firestore/Swift/Tests/Integration/SnapshotListenerSourceTests.swift
diff --git a/Firestore/Swift/Tests/Integration/TypeTest.swift b/Firestore/Swift/Tests/Integration/TypeTest.swift

Original file line number	Diff line number	Diff line change
`@@ -189,11 +189,11 @@ + (nonnull FIRVectorValue )vectorWithArray:(nonnull NSArray<NSNumber > *)array`
`189`	`189`	`}`
`190`	`190`
`191`	`191`	`+ (nonnull FIRMinKey *)minKey {`
`192`		`- return [FIRMinKey instance];`
	`192`	`+ return [FIRMinKey shared];`
`193`	`193`	`}`
`194`	`194`
`195`	`195`	`+ (nonnull FIRMaxKey *)maxKey {`
`196`		`- return [FIRMaxKey instance];`
	`196`	`+ return [FIRMaxKey shared];`
`197`	`197`	`}`
`198`	`198`
`199`	`199`	`+ (nonnull FIRRegexValue )regexWithPattern:(nonnull NSString )pattern`