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

Missing `throws` in ModelMiddleware – or documentation outdated? #703

Closed Jeehut closed 3 years ago

Jeehut commented 3 years ago

I just read this in the Vapor 4 docs (emphasis mine):

The middleware can choose to return early, throw an error, or call the next action to continue normally.

So I tried to use some try functions which could throw an error like this:

struct UserMiddleware: ModelMiddleware {
    func delete(model: User, force: Bool, on db: Database, next: AnyModelResponder) -> EventLoopFuture<Void> {
        try model.createdProjects.delete(force: force, on: db).wait()
        try model.createdTasks.delete(force: force, on: db).wait()

        return next.delete(model, force: force, on: db)
    }
}

But of course, the method is not marked as throws, thus I'm getting an error stating "Errors thrown from here are not handled". But then, when I make the function throwing by adding throws, I'm getting another error stating "Type 'UserMiddleware' does not conform to protocol 'ModelMiddleware'" as none of the methods in ModelMiddleware are throwing functions.

So I'm confused now, is the documentation outdated and actually we can not throw an error as stated there, or am I misunderstanding something? How else can I achieve what I'm trying to do here, I was expecting I'm able to use the delete lifecycle callback given that there's no onDelete: .cascade option for @Children in Fluent. Or is the call to .wait() the wrong approach here?

Jeehut commented 3 years ago

Okay, I actually just had an idea and managed to get it building by using flatMap like this (I hope that will work as expected):

import Fluent

struct UserMiddleware: ModelMiddleware {
    func delete(model: User, force: Bool, on db: Database, next: AnyModelResponder) -> EventLoopFuture<Void> {
        model.createdProjects.delete(force: force, on: db)
            .flatMap { model.createdTasks.delete(force: force, on: db) }
            .flatMap { next.delete(model, force: force, on: db) }
    }
}

But still I think the documentation is outdated or at least misleading as it's stating we can throw errors in the callbacks.

Jeehut commented 3 years ago

Yet another update, the above code was not working, I was getting the error "Children relation not eager loaded, use $ prefix to access: Children<User, Project>(for: [creator_id])". But I could fix this after understanding why it happened via this comment:

import Fluent

struct UserMiddleware: ModelMiddleware {
    func delete(model: User, force: Bool, on db: Database, next: AnyModelResponder) -> EventLoopFuture<Void> {
        model.$createdProjects.query(on: db).delete(force: force)
            .flatMap { model.$createdTasks.query(on: db).delete(force: force) }
            .flatMap { next.delete(model, force: force, on: db) }
    }
}

Still not entirely sure if this is the correct approach though, but that's probably off-topic to this issue.

0xTim commented 3 years ago

Yes the documentation is out of date, it should really say

The middleware can choose to return early, return a failed future, or call the next action to continue normally.

You also wouldn't be able to use wait() as you can't use wait() on an event loop

Jeehut commented 3 years ago

Thank you @0xTim for the clarification and thank you @Lloople for the quick fix, closing this then! 🎉