Jelajahi Sumber

Firestore: Make @DocumentID value setter private (#10167)

* Firestore: Make @DocumentID value setter private

Updated the `DocumentID` Swift property wrapper's `value` property to be private(set) since it is ignored in the methods that write to Firestore.

* Make initializer private

* Fix formatting

* Fix tests

Made the setter internally accessible and imported FirebaseFirestoreSwift with @testable.

* Add a changelog entry and formatting fixes

* Documentation fixes

* [skip ci] Add runtime warnings to `@DocumentID` setter and initializer (#10275)

* [Firestore] Mark `@DocumentID` setter deprecated and add log statement in init

* Fix typo in import

* Update CHANGELOG

* Review and remove deprecation

* Review and add FIRLogWarningSwift

* Add FirebaseCorExtension to Firestore's Podfile

* [skip ci] Fix merged comment

Co-authored-by: Nick Cooke <36927374+ncooke3@users.noreply.github.com>
Co-authored-by: Nick Cooke <nickcooke@google.com>
Andrew Heard 3 tahun lalu
induk
melakukan
8866b22f89

+ 20 - 8
FirebaseCore/Extension/FIRLogger.h

@@ -118,19 +118,31 @@ extern void FIRLogDebug(FIRLoggerService service, NSString *messageCode, NSStrin
 
 // TODO: Come up with a better logging scheme for Swift.
 /**
- * Logs a message to the Xcode console and the device log. If running from AppStore, will
+ * Logs a debug message to the Xcode console and the device log. If running from AppStore, will
  * not log any messages with a level higher than FirebaseLoggerLevelNotice to avoid log spamming.
  * This function is intended to be used by Swift clients that do not support variadic parameters.
- * (required) log level (one of the FirebaseLoggerLevel enum values).
- * (required) service name of type FirebaseLoggerService.
- * (required) message code starting with "I-" which means iOS, followed by a capitalized
- *            three-character service identifier and a six digit integer message ID that is unique
- *            within the service.
- *            An example of the message code is @"I-COR000001".
- * (required) message string.
+ *
+ * @param service The service name of type `FirebaseLoggerService`.
+ * @param messageCode The mesage code. starting with "I-" which means iOS, followed by a capitalized
+ * three-character service identifier and a six digit integer message ID that is unique within the
+ * service. An example of the message code is @"I-COR000001".
+ * @param message The message string.
  */
 extern void FIRLogDebugSwift(FIRLoggerService service, NSString *messageCode, NSString *message);
 
+/**
+ * Logs a warning message to the Xcode console and the device log. If running from AppStore, will
+ * not log any messages with a level higher than FirebaseLoggerLevelNotice to avoid log spamming.
+ * This function is intended to be used by Swift clients that do not support variadic parameters.
+ *
+ * @param service The service name of type `FirebaseLoggerService`.
+ * @param messageCode The mesage code. starting with "I-" which means iOS, followed by a capitalized
+ * three-character service identifier and a six digit integer message ID that is unique within the
+ * service. An example of the message code is @"I-COR000001".
+ * @param message The message string.
+ */
+extern void FIRLogWarningSwift(FIRLoggerService service, NSString *messageCode, NSString *message);
+
 #ifdef __cplusplus
 }  // extern "C"
 #endif  // __cplusplus

+ 4 - 0
FirebaseCore/Sources/FIRLogger.m

@@ -162,6 +162,10 @@ void FIRLogDebugSwift(FIRLoggerService service, NSString *messageCode, NSString
   FIRLogDebug(service, messageCode, @"%@", message);
 }
 
+void FIRLogWarningSwift(FIRLoggerService service, NSString *messageCode, NSString *message) {
+  FIRLogWarning(service, messageCode, @"%@", message);
+}
+
 #undef FIR_MAKE_LOGGER
 
 #pragma mark - FIRLoggerWrapper

+ 1 - 0
FirebaseFirestoreSwift.podspec

@@ -34,6 +34,7 @@ Google Cloud Firestore is a NoSQL document database built for automatic scaling,
     'Firestore/Swift/Source/**/*.swift',
   ]
 
+  s.dependency 'FirebaseCoreExtension', '~> 10.0'
   s.dependency 'FirebaseFirestore', '~> 10.0'
   s.dependency 'FirebaseSharedSwift', '~> 10.0'
 

+ 2 - 1
Firestore/Example/Podfile

@@ -62,7 +62,8 @@ def configure_local_pods()
   # to its podspec can still function. See Firestore-*-xcodebuild in
   # scripts/install_prereqs.sh for more details.
   pod 'FirebaseCore', :path => '../..'
-  pod 'FirebaseCoreInternal',:path => '../..'
+  pod 'FirebaseCoreInternal', :path => '../..'
+  pod 'FirebaseCoreExtension',:path => '../..'
 
   # Pull in local sources conditionally.
   maybe_local_pod 'FirebaseAuth'

+ 5 - 0
Firestore/Swift/CHANGELOG.md

@@ -1,4 +1,9 @@
 # 10.0.0
+- [changed] **Breaking Change:** Made the `@DocumentID` property wrapper value
+  setter internal to clarify that the value is ignored during writes. (#9368)
+- [changed] Initializing a `@DocumentID` property wrapper with a non-nil value
+  or using the `@DocumentID` property wrapper value setter will log a warning.
+  This is because the set value will be ignored. (#9368)
 - [changed] `Firestore.Encoder` and `Firestore.Decoder` now wraps the shared
   `FirebaseDataEncoder` and `FirebaseDataDecoder` types, which provides new
   customization options for encoding and decoding data to and from Firestore

+ 78 - 20
Firestore/Swift/Source/Codable/DocumentID.swift

@@ -16,6 +16,7 @@
 
 import FirebaseFirestore
 import FirebaseSharedSwift
+@_implementationOnly import FirebaseCoreExtension
 
 extension CodingUserInfoKey {
   static let documentRefUserInfoKey =
@@ -65,57 +66,114 @@ internal protocol DocumentIDProtocol {
   init(from documentReference: DocumentReference?) throws
 }
 
-/// A value that is populated in Codable objects with the `DocumentReference`
-/// of the current document by the Firestore.Decoder when a document is read.
+/// A property wrapper type that marks a `DocumentReference?` or `String?` field to
+/// be populated with a document identifier when it is read.
 ///
-/// If the field name used for this type conflicts with a read document field,
-/// an error is thrown. For example, if a custom object has a field `firstName`
-/// annotated with `@DocumentID`, and there is a property from the document
-/// named `firstName` as well, an error is thrown when you try to read the
-/// document.
+/// Apply the `@DocumentID` annotation to a `DocumentReference?` or `String?`
+/// property in a `Codable` object to have it populated with the document
+/// identifier when it is read and decoded from Firestore.
 ///
-/// When writing a Codable object containing an `@DocumentID` annotated field,
-/// its value is ignored. This allows you to read a document from one path and
-/// write it into another without adjusting the value here.
+/// - Important: The name of the property annotated with `@DocumentID` must not
+///   match the name of any fields in the Firestore document being read or else
+///   an error will be thrown. For example, if the `Codable` object has a
+///   property named `firstName` annotated with `@DocumentID`, and the Firestore
+///   document contains a field named `firstName`, an error will be thrown when
+///   attempting to decode the document.
 ///
-/// NOTE: Trying to encode/decode this type using encoders/decoders other than
-/// Firestore.Encoder leads to an error.
+/// - Example Read:
+///   ````
+///   struct Player: Codable {
+///     @DocumentID var playerID: String?
+///     var health: Int64
+///   }
+///
+///   let p = try! await Firestore.firestore()
+///       .collection("players")
+///       .document("player-1")
+///       .getDocument(as: Player.self)
+///   print("\(p.playerID!) Health: \(p.health)")
+///
+///   // Prints: "Player: player-1, Health: 95"
+///   ````
+///
+/// - Important: Trying to encode/decode this type using encoders/decoders other than
+///   Firestore.Encoder throws an error.
+///
+/// - Important: When writing a Codable object containing an `@DocumentID` annotated field,
+///   its value is ignored. This allows you to read a document from one path and
+///   write it into another without adjusting the value here.
 @propertyWrapper
 public struct DocumentID<Value: DocumentIDWrappable & Codable>:
-  DocumentIDProtocol, Codable, StructureCodingUncodedUnkeyed {
-  var value: Value?
+  StructureCodingUncodedUnkeyed {
+  private var value: Value? = nil
 
   public init(wrappedValue value: Value?) {
+    if let value = value {
+      logWarning(for: value)
+    }
     self.value = value
   }
 
   public var wrappedValue: Value? {
     get { value }
-    set { value = newValue }
+    set {
+      if let someNewValue = newValue {
+        logWarning(for: someNewValue)
+      }
+      value = newValue
+    }
   }
 
-  // MARK: - `DocumentIDProtocol` conformance
+  private func logWarning(for value: Value) {
+    FIRLogWarningSwift(
+      "[FirebaseFirestoreSwift]",
+      "I-FST000002",
+      """
+      Attempting to initialize or set a @DocumentID property with a non-nil
+      value: \(value). The document ID is managed by Firestore and any
+      initialized or set value will be ignored. The ID is automatically set
+      when reading from Firestore."
+      """
+    )
+  }
+}
 
-  public init(from documentReference: DocumentReference?) throws {
+extension DocumentID: DocumentIDProtocol {
+  internal init(from documentReference: DocumentReference?) throws {
     if let documentReference = documentReference {
       value = try Value.wrap(documentReference)
     } else {
       value = nil
     }
   }
+}
 
-  // MARK: - `Codable` implementation.
-
+extension DocumentID: Codable {
+  /// A `Codable` object  containing an `@DocumentID` annotated field should
+  /// only be decoded with `Firestore.Decoder`; this initializer throws if an
+  /// unsupported decoder is used.
+  ///
+  /// - Parameter decoder: A decoder.
+  /// - Throws: ``FirestoreDecodingError``
   public init(from decoder: Decoder) throws {
     guard let reference = decoder
       .userInfo[CodingUserInfoKey.documentRefUserInfoKey] as? DocumentReference else {
       throw FirestoreDecodingError.decodingIsNotSupported(
-        "Could not find DocumentReference for user info key: \(CodingUserInfoKey.documentRefUserInfoKey)"
+        """
+        Could not find DocumentReference for user info key: \(CodingUserInfoKey
+          .documentRefUserInfoKey).
+        DocumentID values can only be decoded with Firestore.Decoder
+        """
       )
     }
     try self.init(from: reference)
   }
 
+  /// A `Codable` object  containing an `@DocumentID` annotated field can only
+  /// be encoded  with `Firestore.Encoder`; this initializer always throws.
+  ///
+  /// - Parameter encoder: An invalid encoder.
+  /// - Throws: ``FirestoreEncodingError``
   public func encode(to encoder: Encoder) throws {
     throw FirestoreEncodingError.encodingIsNotSupported(
       "DocumentID values can only be encoded with Firestore.Encoder"

+ 1 - 1
Firestore/Swift/Tests/Integration/CodableIntegrationTests.swift

@@ -16,7 +16,7 @@
 
 import Foundation
 import FirebaseFirestore
-import FirebaseFirestoreSwift
+@testable import FirebaseFirestoreSwift
 
 class CodableIntegrationTests: FSTIntegrationTestCase {
   private enum WriteFlavor {

+ 5 - 1
Package.swift

@@ -701,7 +701,11 @@ let package = Package(
 
     .target(
       name: "FirebaseFirestoreSwift",
-      dependencies: ["FirebaseFirestore", "FirebaseSharedSwift"],
+      dependencies: [
+        "FirebaseCoreExtension",
+        "FirebaseFirestore",
+        "FirebaseSharedSwift",
+      ],
       path: "Firestore",
       exclude: [
         "CHANGELOG.md",