JohnEstropia / CoreStore

Unleashing the real power of Core Data with the elegance and safety of Swift
MIT License
4.01k stars 255 forks source link

DataStack.addStorage failing after first migration #405

Open timfraedrich opened 4 years ago

timfraedrich commented 4 years ago

I'm trying to do something like suggested in #276, but as a generalised solution, so I do not have to code it every time something like that comes up. For that I set up some protocols with which I will define my data models.

For normal data models:

/// A protocol used to define data models for the data base this application saves workouts in.
protocol CustomDataModel {

    /// A `String` to identify the data model in a CoreData/CoreStore context
    static var identifier: String { get }

    /// The `CoreStoreSchema` of the data model used to setup the datastack
    static var schema: CoreStoreSchema { get }

    /// The `CustomSchemaMappingProvider` of this data model version used to migrate from the last; if `nil` the model should be the first
    static var mappingProvider: CustomSchemaMappingProvider? { get }

    /// An array of this data models and the ones coming before it in chronologial order used to perform migrations
    static var migrationChain: [CustomDataModel.Type] { get }

}

and for so called intermediate data models:

typealias CustomIntermediateMappingActions = (DataStack) -> Bool

/// A protocol used to define a data model used to assist a more complex migration to another
protocol CustomIntermediateDataModel: CustomDataModel {

    /// A closure being performed as an intermediate step between performing the migration to the next actual data model
    static var intermediateMappingActions: CustomIntermediateMappingActions { get }

}

I setup my data stack and perform the custom migrations in the following way:

static func setup(dataModel: CustomDataModel.Type) {

    // select relevant versions
    let relevants = dataModel.migrationChain.filter { (type) -> Bool in
        type.self is CustomIntermediateDataModel.Type || type == dataModel
    }

    // setup storage
    let storage = SQLiteStore(
        fileName: "DataBase.sqlite",
        migrationMappingProviders: dataModel.migrationChain.compactMap(
            { (type) -> CustomSchemaMappingProvider? in
                return type.mappingProvider
            }
        ),
        localStorageOptions: .none
    )

    // loop through migrations and perform intermediate actions
    for relevant in relevants {

        let dataStack = constructDataStack(fromChain: dataModel.migrationChain, withCurrentVersion: relevant)
        do {

            try dataStack.addStorageAndWait(storage)

            if let intermediate = relevant as? CustomIntermediateDataModel.Type {
                guard intermediate.intermediateMappingActions(dataStack) else {
                    fatalError("intermediate mapping actions were unsuccessful")
                }
            }
        } catch {
            fatalError("failed to add storage: \(error)")
        }
    }
}

The first run of the loop which sets up the data base up to and including the first intermediate model always succeeds, but for the second it always throws an error saying The model used to open the store is incompatible with the one used to create the store, even though all required mapping providers and so on are set up correctly.

I hope I'm not missing something really obvious, like do I for example have to be aware of anything when reusing the same SQLiteStore without relaunching in between? I couldn't find anything on that in the documentation.

I would appreciate any help, thanks in advance.

JohnEstropia commented 4 years ago

@timfraedrich How are you initializing your DataStack?

timfraedrich commented 4 years ago

Sorry totally forgot about that code snippet:

static private func constructDataStack(fromChain migrationChain: [CustomDataModel.Type], withCurrentVersion currentVersion: CustomDataModel.Type) -> DataStack {

    let schemata = migrationChain.map { (type) -> CoreStoreSchema in type.schema }
    let migrationDictionary = Dictionary(uniqueKeysWithValues:
        migrationChain.dropFirst().enumerated().map { (index, type) -> (String, String) in
            (migrationChain[index].identifier ,type.identifier)
        }
    )
    let schemaHistory = SchemaHistory(allSchema: schemata, migrationChain: MigrationChain(migrationDictionary), exactCurrentModelVersion: currentVersion.identifier)
    return DataStack(schemaHistory: schemaHistory)

}

I normally do not initialise with a SchemaHistory which might actually be the issue since I wasn't even familiar with it before, but I saw no other way to setup the data stack from an array of schemata.

JohnEstropia commented 4 years ago

This looks okay. How are you declaring entities in your CustomDataModel implementations?

timfraedrich commented 4 years ago

They are just normal CoreStoreObjects with the addition of a static identifier property, which shouldn't cause this issue, I have been using it from the start.

Also what I totally forgot about: I'm using version 7.3.0, just for reference

JohnEstropia commented 4 years ago

I'm just curious how you are initializing your CoreStoreSchema instances, particularly the Entity<X> declarations

timfraedrich commented 4 years ago

Sure, here is an example:

static let schema = CoreStoreSchema(
    modelVersion: V4.identifier,
    entities: [
        Entity<V4.ObjectOne>(V4.ObjectOne.identifier),
        Entity<V4.ObjectTwo>(V4.ObjectTwo.identifier),
        Entity<V4.ObjectThree>(V4.ObjectThree.identifier),
        Entity<V4.ObjectFour>(V4.ObjectFour.identifier),
    ],
    versionLock: [
        V4.ObjectOne.identifier: [0x236fcba032b81ba9, 0xa776b92c815cdcc0, 0x123af15289e50cd9, 0x9766946e390e574f],
        V4.ObjectTwo.identifier: [0xa95ee763e8505f72, 0x1947b3b962397eac, 0xb67b70673443485f, 0xd0f56005d56ed2f5],
        V4.ObjectThree.identifier: [0xab96203b4ad8735, 0x83a3706df06897f9, 0x499ccfb06aa82a1f, 0xb3653fd2be428391],
        V4.ObjectFour.identifier: [0x8fb3f3add05348dc, 0xaf69cdd28c67537, 0xeda9c05c619958f, 0x62c61c5f0f6a8978]
    ]
)
timfraedrich commented 4 years ago

Okay so apparently the models were not the issue, I just transitioned from synchronously adding the storage to using the modern CoreStore implementation and adding it asynchronously. For that I used a function which calls itself in case there are elements left to be migrated to:

static func setup(dataModel: CustomDataModel.Type, completion: @escaping () -> Void, migration: @escaping (Progress) -> Void) {

    // setup storage
    let storage = SQLiteStore(
        fileName: "Database.sqlite",
        migrationMappingProviders: dataModel.migrationChain.compactMap(
            { (type) -> CustomSchemaMappingProvider? in
                return type.mappingProvider
            }
        ),
        localStorageOptions: .none
    )

    // select relevant versions
    let currentVersion = storage.currentCustomModel(from: dataModel.migrationChain)
    var relevants = dataModel.migrationChain.filter { (type) -> Bool in
        // relevent version should include the final type (dataModel) and all intermediate models, but it is important that they are successors of current version of the storage otherwise the models might be incompatible
        (type.self is CustomIntermediateDataModel.Type || type == dataModel) && (currentVersion != nil ? type.isSuccessor(to: currentVersion!) : true)
    }

    if relevants.first != nil {

        let destinationModel = relevants.removeFirst()

        // constructing data stack
        let schemata = dataModel.migrationChain.map { (type) -> CoreStoreSchema in type.schema }
        let migrationDictionary = Dictionary(uniqueKeysWithValues: dataModel.migrationChain.dropFirst().enumerated().map { (index, type) -> (String, String) in
                (dataModel.migrationChain[index].identifier ,type.identifier)
            }
        )
        let schemaHistory = SchemaHistory(allSchema: schemata, migrationChain: MigrationChain(migrationDictionary), exactCurrentModelVersion: destinationModel.identifier)
        let dataStack = DataStack(schemaHistory: schemaHistory)

        // adding storage
        if let progress = dataStack.addStorage(
            storage,
            completion: { result in
                switch result {
                case .success(_):

                    if let intermediate = destinationModel as? CustomIntermediateDataModel.Type {
                        if !intermediate.intermediateMappingActions(dataStack) {
                            fatalError("[DataManager] Intermediate mapping actions of \(destinationModel) were unsuccessful")
                        }
                    }

                    if relevants.first != nil {
                        setup(dataModel: dataModel, completion: completion, migration: migration)
                    } else {
                        completion()
                    }

                case .failure(let error):
                    fatalError("[DataManager] Failed to add storage for \(dataModel)\nError: \(error)")
                }
            }
        ) {
            // handling migration
            migration(progress)
        }
    }
}

I also extended SQLiteStoreto be able to determine which version it has (I did not find any standard implementation for that:

extension SQLiteStore {

    internal func currentModel(from migrationChain: [CustomDataModel.Type]) -> CustomDataModel.Type? {

        do {
            let metadata = try NSPersistentStoreCoordinator.metadataForPersistentStore(
                ofType: type(of: self).storeType,
                at: self.fileURL as URL,
                options: self.storeOptions
            )
            for type in migrationChain {
                if type.schema.rawModel().isConfiguration(withName: self.configuration, compatibleWithStoreMetadata: metadata) {
                    return type
                }
            }
        } catch {
            print("Failed to generate metadata for SQLiteStore:", error)
        }
        return nil

    }

}

The above code now works like a charm, performing intermediate migration actions in between the migrations. Maybe this could in one form or another be implemented into CoreStore itself, as pointed out in the above referenced issue this functionality is missing so far and would be a great addition.

I'm still not sure what exactly went wrong here though, maybe its worth looking at dataStack.addStorageAndWait a bit closer.

JohnEstropia commented 4 years ago

Okay, so I missed something important in your original code. Progressive migrations do not work with DataStack.addSynchronous(...). See: https://github.com/JohnEstropia/CoreStore#starting-migrations

As the method name's ~AndWait suffix suggests though, this method blocks so it should not do long tasks such as data migrations. In fact CoreStore will only attempt a synchronous lightweight migration if you explicitly provide the .allowSynchronousLightweightMigration option... ... if you do so, any model mismatch will be thrown as an error.

Since you have multiple steps in your migration, lightweight migration will not work, so you are getting the model mismatch error as intended.

Maybe this could in one form or another be implemented into CoreStore itself, as pointed out in the above referenced issue this functionality is missing so far and would be a great addition.

If you use the asynchronous method DataStack.addStorage(...) instead, you'll get this behavior for free: https://github.com/JohnEstropia/CoreStore/blob/develop/Sources/DataStack%2BMigration.swift#L488

timfraedrich commented 4 years ago

If you use the asynchronous method DataStack.addStorage(...) instead, you'll get this behavior for free

I'm not quite sure we are talking about the same thing, when using the asynchronous method one could theoretically layer migrations, but this is not done automatically as I suggested.

It was just a suggestion though, maybe it's too complicated to incorporate into the CoreData logic.

JohnEstropia commented 4 years ago

It is automatic. Note that the exactCurrentModelVersion argument is optional. If you don't pass it explicitly CoreStore will use the final version in the MigrationChain, as long as there is only one possible final version.

timfraedrich commented 4 years ago

Yes of course, and it is really well thought out, but I am talking about the actions performed in between migrations. Sorry I think I was not very clear on that aspect.

JohnEstropia commented 4 years ago

I see, I thought you were talking about the currentModel function. I'm curious what specific processing you do in-between migrations though. Since something like your intermediateMappingActions is entirely up to the developer (ex: it might be asynchronous), I'm not sure we can keep a standard API that is simpler than calling your own recursive method on addStorage(...).

On another note, one thing I'd recommend is to use DataStack. requiredMigrationsForStorage(...) to get the precomputed steps. Once you have those steps you can create temporary DataStacks that uses a partial MigrationChain that migrates one step at a time. It's similar to your setup() method, but with less code for computing steps.

You might be interested to look at the Demo app's ⭐️Advanced.EvolutionDemo.Migrator.swift file if you haven't yet. That demo allows both two-way migrations, as well as supporting actual use of multiple model versions at any given time.

timfraedrich commented 4 years ago

Well in my particular case I am combining two objects with a relationship to another into one while retaining said relationship. I hope I can finish the first part of the migration today, since my project is open-source I could post a reference to the commit here, if you like.

Regarding the implementation of a standard API you're probably more qualified to judge this, I might look at a possible solution incorporating only existing CoreStore implementations in the future though, I will obviously create a post request once I have figured anything out, let's see.

Thanks for pointing out the example project, I actually haven't had a look at that specific one!

JohnEstropia commented 4 years ago

Thanks for the feedback! Please do share if you have implementations that may be good reference of use cases