FetchExisting doesn't preserve subclass type #160

New Issue

adam · 2025-12-29T15:25:59+01:00

adam commented

2025-12-29 15:25:59 +01:00

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

@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

@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

@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.

Branches Tags

develop

master

prototype/concurrency

minIOS13

prototype/containers

transactionSourceTracking

prototype/swiftui

minIOS11

swift5_3

prototype/propertyWrapperFields

prototype/propertyWrappers

prototype/propertyWrapper

Swift5

throwables

prototype/Swift_4_1

prototype/Swift_4_0

prototype/Swift_3_2

prototype/queryBuilders

temp/develop

corestore4_develop

prototype/mainContextToPSC

master_ios_7_to_9

swift_2_3

gh-pages

_corespotlight-test

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: starred/CoreStore#160