[WIP] - Referenceable Object Swift PropertyWrapper - @FirestoreObjectReference #11330
Conversation
Generated by 🚫 Danger |
Apple API Diff ReportCommit: 6e3475f FirebaseFirestoreSwiftStructures[ADDED] FirestoreObjectReference[ADDED] FirestoreObjectReferenceSwift:
+ @available(macOS 10.15, iOS 13, tvOS 13, watchOS 6, *) @propertyWrapper public struct FirestoreObjectReference < T > where T : ReferenceableObject extension FirestoreObjectReference : Codable
+ public init ( wrappedValue initialValue : T ?)
+ public var wrappedValue : T ? { get set }
+ public var projectedValue : ObjectReference < T > ? { get set }
+ public init ( from decoder : Decoder ) throws
+ public func encode ( to encoder : Encoder ) throws[ADDED] ObjectReference[ADDED] ObjectReferenceSwift:
+ public var objectId : String
+ public var collection : String
+ public var referencedObject : T ?
+ public mutating func loadReferencedObject () async throws[ADDED] ClassesSwift:
+ @available(macOS 10.15, iOS 13, tvOS 13, watchOS 6, *) public class ReferenceableObjectManager
+ public static var instance : ReferenceableObjectManager
+ public func save < T > ( object : T ) async throws where T : ReferenceableObject
+ public func getObject < T > ( objectId : String ) async throws -> T ? where T : ReferenceableObject
+ public func getObjects < T > ( type : T . Type ) async throws -> [ T ] where T : ReferenceableObject
+ public func getObjects < T : ReferenceableObject > ( predicates : [ QueryPredicate ]) async throws -> [ T ]Protocols[ADDED] ReferenceableObject[ADDED] ReferenceableObjectSwift:
+ @available(macOS 10.15, iOS 13, tvOS 13, watchOS 6, *) public protocol ReferenceableObject : Decodable , Encodable , Hashable , Identifiable
+ static func parentCollection () -> String
+ var id : String ? { get set }
+ var path : String ? { get }
+ static func objectPath ( objectId : String ) -> String
+ func hash ( into hasher : inout Hasher )
+ static func == ( lhs : Self , rhs : Self ) -> Bool |
There was a problem hiding this comment.
I generally like the idea (object references are a feature of Firestore, but we haven't had client side support for them so far), however, I would love to hear the Firestore team's opinion on whether we should expose this to the client SDKs.
Sample app: I started a sample at in https://github.com/firebase/firebase-ios-sdk/tree/master/Example/FirestoreSample, which contains a menu structure that should make it easy to add a sample for this property wrapper.
|
|
||
| /// Property wrapper @FirestoreObjectReference, | ||
| /// Indicates that the specified property value should be stored by reference instead of by value, inline | ||
| /// with the parent object. |
There was a problem hiding this comment.
I am not sure what "inline with the parent object" refers to.
There was a problem hiding this comment.
I can't find a good way to describe this. Checked both these videos but not finding a good naming.
Firestore data structure
https://www.youtube.com/watch?v=haMOUb3KVSo
Firestore Data Modeling I/O 2019
https://www.youtube.com/watch?v=lW7DWV2jST0
| /// When loading a parent object, any references are not loaded by default and can be loaded on demand | ||
| /// using the projected value of the wrapper. | ||
| /// | ||
| /// structs that can be stored as a reference must implement the `ReferenceableObject` protocol |
| /// | ||
| /// structs that can be stored as a reference must implement the `ReferenceableObject` protocol | ||
| /// | ||
| /// variables that are annotated with the propertyWrapper should be marked as `Optional` since they can be nil when not loaded or set |
| /// | ||
| /// ``` | ||
| /// struct UserProfile: ReferenceableObject { | ||
| /// var username: String |
| /// | ||
| @available(macOS 10.15, iOS 13, tvOS 13, watchOS 6, *) | ||
| @propertyWrapper | ||
| public struct FirestoreObjRef<T> where T: ReferenceableObject { |
There was a problem hiding this comment.
I know it's quite a mouthful, but I think I might prefer FirestoreObjectReference over the shortened version. Code completion is your friend.
There was a problem hiding this comment.
Makes sense. Made the change. Tx!
| try await db.collection(T.parentCollection()).document().setData(json) | ||
| } | ||
|
|
||
| print("Save complete") |
There was a problem hiding this comment.
Might want to use the logger.
There was a problem hiding this comment.
Done. I also refactored the Logger class so it could be reused across FirestoreSwift if need be.
| do { | ||
| // first check cache | ||
| if let cacheEntry = await objectCache.get(for: T.objectPath(objectId: objId)) { | ||
| return cacheEntry.object as! T |
There was a problem hiding this comment.
Is there a way how we can cast the type without coercing it?
There was a problem hiding this comment.
I haven't been able to figure this one out. Its odd. 'object' is a ReferenceableObject and so is 'T'. Not sure why compiler is not letting me just return object without a cast.
There was a problem hiding this comment.
Ok optional casting removes the warning and its not a forced cast anymore. In reality if cacheEntry exists, object will not be nil since its not an optional but optional casting makes compiler happy.
| } | ||
|
|
||
| // get from db | ||
| let docRef = db.collection(T.parentCollection()).document(objId) |
There was a problem hiding this comment.
documentReference / objectId
| return foundObjects | ||
| } | ||
|
|
||
| public func fetchObjects<T: ReferenceableObject>(predicates: [QueryPredicate]) async throws |
There was a problem hiding this comment.
Our other APIs use get instead of fetch and Document instead of Object, (see https://firebase.google.com/docs/firestore/query-data/get-data#get_a_document)
There was a problem hiding this comment.
Changed the function name to use get. Kept object to stay consistent with the PRs nomenclature.
|
|
||
| var foundObjects = [T]() | ||
| let snapshot = try await query.getDocuments() | ||
| for document in snapshot.documents { |
There was a problem hiding this comment.
Maybe use a compactMap instead (like we do in most of our samples for mapping collections).
There was a problem hiding this comment.
This doesn't seem to work. I think the issue is that its within an async method.
Coverage Report 1Affected Products
Test Logs |
aashishpatil-g
changed the title
|
Thanks for the detailed review. I've addressed the comments except
|
|
Please close, merge, or comment. We plan to close stale PRs on November 28, 2023. |
|
Going to close this for now as I haven't made progress on sample app. Will reopen later if still viable. |
Discussion
This PR proposes a Swift property wrapper (@FirestoreObjectReference) to store object (non-scalar) properties (vars) by reference in Firestore instead with the document blob. It automates storing the object in a specified location and saves the reference id in the parent document instead of the actual object. This is useful for scenarios where a common object must be used / referenced across multiple parent objects.
When reading/loading the parent document (object) from Firestore, the propertyWrapper provides a way to load the referenced object via the projected value of the wrapper. The loadReference() call on the projected value is async.
For objects to participate in the propertyWrapper, they must implement the ReferenceableObject Protocol.
Here is a sample data model and how the propertyWrapper is used.
We have three structs representing a simplified UserProfile model -
UserProfileEmployer- an employer who theUserProfileworks forWorkLocation- representing a generic location.Since multiple Users can work for an Employer, it makes sense to have only one instance of an Employer that is referred to
by multiple Users. Similarly multiple users can be in a WorkLocation. Additionally, an Employer can also be located
in a WorkLocation (e.g. headquarters of an Employer). We can mark WorkLocation and Employer as a ReferenceableObjects.
TODO
The following items are still to be done.