-
Notifications
You must be signed in to change notification settings - Fork 395
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Implement Codable support for NSManagedObject subclasses #377
Comments
Codable
support for NSManagedObject
subclasses
I'm really conflicted about this. On one side, this is good implementation of Codable. I don't like the two classes approach but I agree it's necessary in this case, since the generated code will be larger than human-generated in most use cases. I understand the desire and benefits of automating mundane conversion tasks like this. On the other side, I find it untenable in practice. Maybe I'm just unlucky but in last 5 projects I had at least one such case (multiple JSON structures for the same model object) in each of them. I gave up on automating that conversion and just write the Re: automatic find-or-create – I tried it about two years ago on a pretty large client project and found that it can work only in perfectly designed ecosystem. Where service developers are aware that their JSON streams will be consumed in this way and extra care is taken when crafting JSON responses. In practice, especially contracting practice, this is never the case :( |
Few more thoughts in general, about Codable.
Mattt has a good starting point for custom codable encoders. |
This looks promising, although I do think there are some challenges here:
Some ideas I'll throw in:
All in all, I think the approach above looks sound. I just worry of the workflows that people would end up after this one decoding process. For instance, let decoder = JSONDecoder()
decoder.userInfo = [
.context: context,
.deferInsertion: true
]
let newEvents = try decoder.decode([Event].self, from: jsonArrayData)
// How to process relationships?
let newPerson = try decoder.decode(Person.self, from: jsonData)
// We reused the decoder here, was that ok? |
@radianttap Thanks for the feedback. I sympathize with the problem of inconsistent or even incorrect JSON. I've encountered it myself. I don't think there's a single solution that applies everywhere though, because every broken server API is broken in a different way. I don't think mogenerator can aim to solve all of those problems, so I'm hoping for a solid implementation that works in many cases but doesn't claim to be the best answer for everyone. If nothing else, mogenerator's code might be a good starting point for a more custom As for the focus on JSON, that's not really central to the code. The sample I'd be interested to hear more of your thoughts on |
@JohnEstropia Thanks for the feedback. Going over various points:
|
@JohnEstropia I was looking into your suggestion of extending I tried this approach for the managed object context: extension Decoder {
var context: NSManagedObjectContext? {
set {
userInfo[.context] = newValue
}
get {
return userInfo[.context] as? NSManagedObjectContext
}
}
} In theory it's great. In practice, I thought maybe I could use a custom implementation of If you have something else in mind or can suggest a way to make this work, please let me know. I'd like to do this but right now I don't think it will work. |
@atomicbird Ah, I didn't know extension JSONDecoder {
func decode<T: NSManagedObject>(_ type: T.Type, data: Data, in context: NSManagedObjectContext, deferInsertion: Bool = false) throws -> T {
decoder.userInfo[.context] = context
decoder.userInfo[.deferInsertion] = deferInsertion
defer {
decoder.userInfo[.context] = nil
decoder.userInfo[.deferInsertion] = nil
}
return try self.decode(type, from: data)
}
} or even rotate the types so that this can be called from the extension NSManagedObjectContext {
func decode<T: NSManagedObject>(_ type: T.Type, data: Data, with decoder: JSONDecoder, deferInsertion: Bool = false) throws -> T {
decoder.userInfo[.context] = self
decoder.userInfo[.deferInsertion] = deferInsertion
defer {
decoder.userInfo[.context] = nil
decoder.userInfo[.deferInsertion] = nil
}
return try decoder.decode(type, from: data)
}
} This is if each unit of decoding is preferred over sharing a extension JSONDecoder {
convenience init(context: NSManagedObjectContext) {
self.init()
self.userInfo[.context] = context
}
func decode<T: NSManagedObject>(_ type: T.Type, data: Data, deferInsertion: Bool = false) throws -> T {
// ...
}
} |
@JohnEstropia These look like really good ideas. I'm not sure about depending on protocol MODecoder: class {
var userInfo: [CodingUserInfoKey : Any] { get set }
func decode<T>(_ type: T.Type, from data: Data) throws -> T where T : Decodable
func decode<T:NSManagedObject & Decodable>(_ type: T.Type, data: Data, in context: NSManagedObjectContext, deferInsertion: Bool) throws -> T
}
extension JSONDecoder: MODecoder { }
extension PropertyListDecoder: MODecoder {}
extension MODecoder {
func decode<T:NSManagedObject & Decodable>(_ type: T.Type, data: Data, in context: NSManagedObjectContext, deferInsertion: Bool = false) throws -> T {
userInfo[.context] = context
userInfo[.deferInsertion] = deferInsertion
defer {
userInfo[.context] = nil
userInfo[.deferInsertion] = nil
}
return try self.decode(type, from: data)
}
} Then you could create whatever decoder you like and use this method to handle decoding managed objects. |
Good idea, that's definitely more elegant 👍 |
I think we more or less have agreement here. As I understand @radianttap's concerns, they'll be addressed separately by support for Swift enums (which would not require |
I just wanted to let people know that, despite a long delay, I'm working on finishing this up, and making good progress. There have been a number of sticking points I didn't anticipate but I'm working through them one by one. The most recent one is that under the current code, a numeric attribute which does not have "use scalar attribute" checked in the data model is synthesized as an However, In the meantime, anyone interested in how mogenerator works might be interested in the new mogenerator internals document I added to share things I've learned in coming to grips with the code. |
I've run into some conceptual difficulties that I'm not sure how to fix, as a result of which I think this feature may be dropped for now, so as to not keep holding up other Swift changes. The current sticking point is what to do about relationships to entities that do not have a custom class. I had been thinking that the code could assume relationships would be to other custom classes, which would also get Codable support as outlined here, and that would be fine. If there's no custom subclass then that doesn't work. There may be a way to deal with that but I don't know what it is right now and I think I've been hung up on it for long enough that it's time to stop. Score one for the built-in tests; I was blinded by the fact that I always have custom subclasses, and it didn't occur to me to handle that when I suggested this. |
Awesome, glad it caught this. |
Feature Request
Core Data is often used to store data downloaded from a server API of some kind. Server APIs often provide data as one or more JSON dictionaries. Parsing these with
JSONDecoder
is convenient but creating managed object instances isn't possible without some extra work. This can and should be provided bymogenerator
.Rationale
Creating managed objects via
Codable
is complicated by the fact thatCodable
andNSManagedObject
have conflicting ideas about initialization.Codable
instantiates objects by callinginit(from decoder: Decoder)
(which will be auto-generated if no implementation exists). MeanwhileNSManagedObject
's designated initializer isinit(entity: NSEntityDescription, insertInto context: NSManagedObjectContext?)
.The key to resolving this is that the
Decoder
protocol includes auserInfo
dictionary typed as[CodingUserInfoKey : Any]
. ImplementingCodable
is possible if youNSManagedObject
context in theuserInfo
dictionaryinit(from decoder: Decoder)
rather than rely on synthesized codeNSManagedObject
's designated initializer.This amounts to a bunch of boilerplate code, customized to each entity, which is what
mogenerator
is good at.What about find-or-create?
It's often necessary to implement a find-or-create approach to handle incoming server data. A common situation is that an app receives an array of dictionaries representing data objects. Some are new and should be created. Some are updated versions of existing data, so the existing object should be updated. This can be handled via a unique ID included in each instance. Core Data uniqueness constraints are capable of handling this, but may incur significant performance issues for large data sets.
Any implementation of
Codable
forNSManagedObject
subclasses need to take this into account. Implementing find-or-create should not require a uniqueness constraint, but shouldn't prevent them for developers who prefer them.To this end, any implementation of
Codable
should include the option to create non-inserted managed objects. These could be used in a custom find-or-create implementation. New objects could be inserted into the appropriate managed object context via theinsert(_:)
method. New versions of existing objects could be used to update the model, but since they were never inserted, they wouldn't lead to duplicate data. Whether to insert new objects could be indicated by anotheruserInfo
key.Goal
Based on the above, code generated by
mogenerator
would first extendCodingKeys
to handle the requirements above:Developers would use the new
mogenerator
implementation ofCodable
as follows. Given aData
namedincomingData
...If
incomingData
should contain a single instance of theEvent
entity:If
incomingData
should contain an array ofEvent
data, and custom find-or-create code will be used:The
deferInsertion
key indicates that new object should be instantiated but not inserted into the managed object context yet. Later, the find-or-create code caninsert(_:)
them if appropriate.Sample Implementation
Below is an example of what
mogenerator
might produce for an entity. Some things to note:The
swift42
branch currently uses a class for the "human" file and an extension for the "machine" file. ImplementingCodable
probably requires reverting to the previous approach of using two classes rather than one plus an extension. This is becauseinit(from decoder: Decoder)
can't be in an extension, but really should be in the "machine" file. I'd prefer to stick with class plus extension but not at the expense of putting this much generated code in the "human" file.All objects are initially not inserted, because decoding values can throw. If an error is thrown, not inserting means there's no need to clean up a half-initialized managed object. If no errors are thrown, the object is inserted.
This code has a lot more comments than would be in generated code, by way of explaining what I'm trying to do with it.
Please Comment
This is not implemented yet-- I've been assembling the sample code above before modifying
mogenerator
to produce it, because it's a big addition and I want to make sure I know what target I'm aiming for. If you use or might useCodable
with Core Data, please let me know what you think. Thanks!The text was updated successfully, but these errors were encountered: