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

allow query on model to use non-default database #705

Closed rausnitz closed 2 years ago

rausnitz commented 3 years ago

I often use Model.query(on: req)... followed by a series of nested callbacks. For example:

Model.query(on: req).all().flatMap { models in
    loadNextThing().flatMap { thing in
        loadNextThing().flatMap { thing in
            loadNextThing().map { thing in
                ...
            }
        }
    }
}

Changing the query to use a database that isn't the model's default makes things messier:

req.withPooledConnection(to: .replica) { conn in
    Model.query(on: conn).all().flatMap { models in
        loadNextThing().flatMap { thing in
            loadNextThing().flatMap { thing in
                loadNextThing().map { thing in
                    ...
                }
            }
        }
    }
}

This PR would allow the default database to be changed without more nesting:

Model.query(on: req, to: .replica).all().flatMap { models in
    loadNextThing().flatMap { thing in
        loadNextThing().flatMap { thing in
            loadNextThing().map { thing in
                ...
            }
        }
    }
}

One downside (which @0xTim noted) is that the following confusing code would be possible to write:

Model.query(on: connectionToReplicaA, to: .replicaB).all().flatMap { models in
    loadNextThing().flatMap { thing in
        loadNextThing().flatMap { thing in
            loadNextThing().map { thing in
                ...
            }
        }
    }
}

An alternative might be to make static func query(on connection: Future<Self.Database.Connection>, withSoftDeleted: Bool) public. The function I modified in this PR is simply a wrapper around this internal function and is its only caller. Making that function public would enable something like the following:

let replicaConnection = req.withPooledConnection(to: .replica)
Model.query(on: replicaConnection).all().flatMap { models in
    loadNextThing().flatMap { thing in
        loadNextThing().flatMap { thing in
            loadNextThing().map { thing in
                ...
            }
        }
    }
}

Another alternative would be for me to accept our async reality and leave it be. 🙃