starred/CoreStore
1
0
Fork 0
mirror of https://github.com/JohnEstropia/CoreStore.git synced 2026-09-17 15:21:30 +02:00
Code Issues 95 Packages Projects Releases 10 Wiki Activity

FetchExisting doesn't preserve subclass type #160

New Issue
Closed
opened 2025-12-29 15:25:59 +01:00 by adam · 3 comments
adam commented 2025-12-29 15:25:59 +01:00
Owner
Copy Link
Copy Source

Originally created by @sidmani on GitHub (Aug 16, 2017).

There's a limitation in fetchExisting at the moment- all subclass type information is lost when applying fetchExisting on a variable with a superclass type. Check out this code:

class Foo: CoreStoreObject { }
class Bar: Foo { }
let someBar: Foo = CoreStore.fetchOne(From<Bar>(), ....)
// ....
let newBar = CoreStore.fetchExisting(someBar)

newBar is Foo // true
newBar is Bar // false

newBar now not only has type Foo, but is actually a Foo, and not just a Bar stored in a variable with type Foo. IMO, this is rather dangerous because it's required to call fetchExisting when entering and exiting transactions, and using variables with a superclass type is a common pattern if there are multiple subclasses. This issue silently stops subclass overrides of methods from being called- I didn't even notice until my unit tests caught it.
I'm going to play around with the code and see what I can come up with. Would appreciate your input @JohnEstropia (and I'm willing to author a pull request if you agree that this needs fixing).

edit: deleted irrelevant attempt at solving problem

Originally created by @sidmani on GitHub (Aug 16, 2017). There's a limitation in fetchExisting at the moment- all subclass type information is lost when applying fetchExisting on a variable with a superclass type. Check out this code: ``` class Foo: CoreStoreObject { } class Bar: Foo { } let someBar: Foo = CoreStore.fetchOne(From<Bar>(), ....) // .... let newBar = CoreStore.fetchExisting(someBar) newBar is Foo // true newBar is Bar // false ``` newBar now not only has type Foo, but is actually a Foo, and not just a Bar stored in a variable with type Foo. IMO, this is rather dangerous because it's required to call fetchExisting when entering and exiting transactions, and using variables with a superclass type is a common pattern if there are multiple subclasses. This issue silently stops subclass overrides of methods from being called- I didn't even notice until my unit tests caught it. I'm going to play around with the code and see what I can come up with. Would appreciate your input @JohnEstropia (and I'm willing to author a pull request if you agree that this needs fixing). edit: deleted irrelevant attempt at solving problem
adam added the fixed label 2025-12-29 15:25:59 +01:00
adam closed this issue 2025-12-29 15:25:59 +01:00
adam commented 2025-12-29 15:25:59 +01:00
Author
Owner
Copy Link
Copy Source

@sidmani commented on GitHub (Aug 16, 2017):

Looking through the code, it seems that this behavior could even be inconsistent between invocations of the method- code from fetchExisting (NSManagedObjectContext+Querying.swift)

let existingRawObject = try self.existingObject(with: rawObject.objectID)
if existingRawObject === rawObject {
    return object // will preserve subclass
}
return T.cs_fromRaw(object: existingRawObject) // will not preserve subclass

Unless I'm missing something basic, it seems like modifying this method is the way to go.

@sidmani commented on GitHub (Aug 16, 2017): Looking through the code, it seems that this behavior could even be inconsistent between invocations of the method- code from fetchExisting (NSManagedObjectContext+Querying.swift) ``` let existingRawObject = try self.existingObject(with: rawObject.objectID) if existingRawObject === rawObject { return object // will preserve subclass } return T.cs_fromRaw(object: existingRawObject) // will not preserve subclass ``` Unless I'm missing something basic, it seems like modifying this method is the way to go.
adam commented 2025-12-29 15:25:59 +01:00
Author
Owner
Copy Link
Copy Source

@JohnEstropia commented on GitHub (Aug 16, 2017):

Ah, good catch. Thanks!

@JohnEstropia commented on GitHub (Aug 16, 2017): Ah, good catch. Thanks!
adam commented 2025-12-29 15:26:00 +01:00
Author
Owner
Copy Link
Copy Source

@JohnEstropia commented on GitHub (Aug 16, 2017):

Merged the PR to the Swift 4 branch. Will cherry-pick into develop and Swift3.2 branches as well.

@JohnEstropia commented on GitHub (Aug 16, 2017): Merged the PR to the Swift 4 branch. Will cherry-pick into develop and Swift3.2 branches as well.
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
abandoned
corestore bug
discussion
duplicate
enhancement
fixed
fixed
help wanted
in progress
investigating
ios/compiler bug
irreproducible
looking for workarounds
not a bug
pending
pending docs update
pull-request
Mirrored from GitHub Pull Request
 question
regression
user bug
wontfix
No labels fixed
Milestone
No items
No Milestone
Projects
Clear projects
No projects
Assignees
Clear assignees
adam (Adam Melkus)
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: starred/CoreStore#160