vapor / fluent

Vapor ORM (queries, models, and relations) for NoSQL and SQL databases
https://docs.vapor.codes/4.0/fluent/overview/
MIT License
1.32k stars 172 forks source link

Fluent.EntityError.noDatabase if my Models are not Preparations #238

Closed MoridinBG closed 7 years ago

MoridinBG commented 7 years ago

I have moved all my model preparations to structs in a Migrations folder. This includes the initial database.create. The reason is because usually during development I need to add additional fields or remove them and I do this in other migrations via database.modify. It makes little sense to have some initial preparation inside the model, that usually becomes very out of date, as development progresses.

But if I attempt to interact with models that have their db setup externally, I receive Fluent.EntityError.noDatabase errors. If I add an empty conformance to Preparation on my models and add them to Config.preparations, everything works without issue.

What is more, if my initial migrations have not yet been run, the tables are not created and my models do not conform to Preparation or are not added to Config.preparations - on the first app run, when it runs this preparations, I can save, etc my models without issue. On following app runs, now that the preparations have been run, I again get Fluent.EntityError.noDatabase.

So it seems to me that during startup, Vapor looks for models/entities in Config.preparations and adds them to Database.map If it only finds dumb preparation structs, it never adds the Model mapping to the Database.

This is a very subtle, not obvious way to setup the mappings, that should be more explicit.

0xTim commented 7 years ago

@MoridinBG I'm assuming this is with Vapor 1?

MoridinBG commented 7 years ago

2.0 Beta 23

tanner0101 commented 7 years ago

@MoridinBG if your models aren't being set as preparations, then you must set the Model.database property manually.

let drop = try Droplet(...)

// you may need to unwrap the database first
User.database = drop.database
Pet.database = drop.database
...

We recommend you conform your models to Preparation if they require any schema preparations (hence the name) in the database to function. This makes it clear what each model needs to function properly.

If you want to keep the preparations organized, you can do so by adding it to the model in an extension in a different file.

extension User: Preparation {
    ...
}

It's probably worth adding a note about this in the docs, however we'd always like to recommend as a best practice that you group your models' preparations on the models itself. :) Data migrations / schema changes not directly related to a model should be created as standalone structs.

MoridinBG commented 7 years ago

@tanner0101 My understanding is that one preparation == one db schema change. I could do the preparation as an extension on the model, but what if I have to do another preparation, to add or remove or change a field? I have to create a new object/struct conforming to Preparation and do the change here. Now I will have the initial, now outdated preparation on the model + some other preparations in separate objects for all the following changes.

That's why I don's see the point of having a preparation inside the model.

tanner0101 commented 7 years ago

You can nest those sequential updates inside the model.

extension User: Preparation {
    // initial
    static func prepare(...) { }
    static func revert(...) { }

    struct AddAgeColumn: Preparation {
        ...
    }
}

When I make schema changes, I usually add a temporary preparation as well as updating the initial one. Then every so often I discard all of the temporary ones. This helps avoid getting to a stage where booting your project on a fresh DB results in errors or long preparation times. In other words, the model should know at all times how to create a database schema that it can use. This makes sharing/re-using code easier as well.

However I understand the use case for have a sequential history of migrations similar to what Laravel does. You should be able to achieve the exact same system as Laravel if you just set the .database property on your models manually.

MoridinBG commented 7 years ago

I haven't thought about nesting, that's a cool idea! I might do that.

I too like to occasionally consolidate all the migrations in the initial migration that creates the table. Rails has the same sequential history of migrations and on slower (e.g. free) remote mysql databases booting a fresh DB with a loong queue of migrations can take quiet some time.

BrettRToomey commented 7 years ago

It sounds like you've found a solution/workaround. Is that correct, @MoridinBG? If not, please feel free to reopen this issue.