Breaking the Chain
3rd May '18 • 9 of your Earth minutes
Breaking the Chain
Querying Eloquent Models consistently across your application
I’ve got to admit I’ve been doing something that some may find pretty disgusting. I’ve been doing it for a while now. I’m happy doing it because I’ve made it work and it works really well, for me.
Like a zealot advocate of showering without soap or letting your hair shampoo itself, I’m going to extol the virtues of successfully using a Model Scope inside of a Policy method by telling you how I made it work. YUCK!
Why on Earth would you not use shampoo!?
The reason for this becomes clear when you understand the context: we’ve got some models that represent DB records which we would consider to be ‘owned’ by a user. So we obviously have a policy that checks if the current user is the owner of a requested record: OwnedByUserPolicy::read()
It actually feels very natural to also apply other logic in this policy — not just about what a user can do, but also about what a user can see — so why not have an index
method that can sort of ‘pre-filter’ the model for us?
It’s totally like your micro-biome naturally sloughing off dead skin. All-natural
So I’ll add an index
method to the policy and a suitable scope to my base model class, pass the model to the policy, and call the model scope inside the policy. That way, when I’m authorising, I always know that I’m also filtering the model by its owner— sexy.
Then back in my controller, I can add other scopes and query clauses (filter, sort etc) as necessary to further filter whatever collection of records this is going to be listing for us.
Keeping it separate in this way means my controller’s index
method can be generalised and pulled into a base controller class that can handle models that are owned by a user and models that aren’t.
(FYI: This is all part of an API that is attempting to be very flexible with highly reusable components to standardise the external interfaces as much as possible across many parts for maximum consistency)
Requirements
- The core reusable queries will be in a base controller that all controllers will inherit. D.R.Y. baby!
- The model must be query-able at any point in this request lifecycle — before and after the
authorize
— without losing any clauses. - To maintain consistency across Policy classes and methods, the ONLY parameter that should be passed to the
authorize
method should be an instance of the model.
These may seem overly strict, but it means that the desired behaviours are available by default without any configuration, purely by inheritance, and they can be easily overridden without breaking a bunch of stuff.
Here’s a simplified example of how our code might look in one of the more complex scenarios:
1// ProductsController extends BaseController 2public function index(Category $category) 3{ 4 // We may want to apply a filter very early 5 // to our core collection query. In this 6 // case, $collection should contain only 7 // products in the chosen category 8 $products = $category->products 910 return parent::index()11}1213// BaseController14public function index()15{16 $this->authorize('index', $this->model)17 $collection = $this->model->filter()->sort()->paginate()1819 return response()->json($collection)20}2122// OwnedByUserPolicy23public function index($user, BaseModel $model)24{25 if ($model->hasOwner()) {26 $model->belongsToUser($user)27 }28}
This is a contrived example, but as you might be able to see, we’re expecting queries from multiple places to apply to our model, all finally filtering down to the $collection
we’ll be sending in the response. But there are some problems when approaching this naively…
Problems
Quite a few, apart from the smell!
- The Scope’s query applied in the Policy is lost as soon as we’re back in our controller
Calling the scope isn’t enough. By the time we get back to our controller to start adding other clauses, they will override anything we tried to apply before. We need to somehow get the Builder our scope returns in order to append more to its query clauses. But we can’t do that in our policy… - We can’t do things and then return useful values from our Policy The way policy methods work means they need to return
true
orfalse
to indicate whether the request can continue or not. So anything new happening there can’t be returned and modifying any passed-by-value argument inside won’t have any effect elsewhere. We might be able to pass stuff by reference and change it. Objects would definitely be passed around by reference anyway (so our model should be). But passing a specific model into our Policy means our policy may end up being less flexible… - If we need to send anything other than our model into the policy, it’s probably going to break some other stuff Our controller is really going to be expecting a Builder for a specific model to be available to add more clauses to the query. If we are able to somehow modify our model inside the policy to return what we want, we now have a fixed dependency on a model. If this was to change at runtime to be a relationship, for example, it’s probably going to wreak havoc with our other scopes and filters because of being of an unexpected type.
- When pre-filtering with a relationship, we want to minimise DB calls, so we need the relationship’s query, not its result-set If we ever want to apply any filter before we call our
authorize
method, we really need whatever query will be applied by that, not the results.
Most of these challenges stem from one underlying issue with how Eloquent models and the query Builder work: models don’t store any state about the current query being built.
When you see an Eloquent model query being built up, it’s usually a chain of statements (usually starting from a static context):
1Model::belongsToUser()->where()->get()23// or45(new Model)->belongsToUser()->where()->get()
What’s happening in these cases is some pretty cool/nasty magic (depending on your POV): Eloquent Models attempt to proxy most method calls down to an Eloquent Builder instance.
In order to fit in with this nice fluent interface, the Laravel docs instruct us to write our scopes in a way that they accept and return a Builder instance. That allows us to write this neat, chainable instruction set.
What’s more, that Builder contains a reference to the model that created it so that it can, in turn, proxy any scope method calls back to the model where they’re defined.
This is all very tasty and makes for a pleasurable development experience: On the face of it, we’re ‘querying the model’, while in reality, underneath it we’re hardly talking to the model, we’re talking to the Builder. The framework has abstracted all of this away with some real smarts underneath, allowing us to write neat models without breaking the fluid approach that we love.
But this causes problems when we want to break this fluent interface. The problems we’re facing in our complex scenario above are appearing because we’re breaking up our query/scope calls across many parts of our application — we’re breaking the chain.
1// In the Policy2$model->belongsToUser()34// Then back in the controller5$this->model->filter()->sort()->paginate()
This is problematic because the model itself doesn’t actually keep a track of the builder it creates, so even if we’re somehow working on the same model instance, the builder is lost. Why?
Because underneath, Laravel is creating a new instance of the query Builder every time we start a separate chain. This is actually desirable in some circumstances, but in others it’s a bit of a pain.
When chaining, we’re passing the same instance down the chain. But separate calls use separate Builders. So in order to use the same Builder, we’d need to pass the Builder created in the first instance into/out of the Policy somehow and back to our controller:
1$builder = $model->belongsToUser()23// Then in the controller4$builder->sort()->paginate()
But as we’ve already spotted this is going to be a tricky trick to achieve.
Solutions
I spent quite some time trying to figure out the best way to handle this in a nice way that kept the “sexy” filtering of the model inside the Policy where it feels like it belongs without resorting to whacky policy class definitions and wild logic in the controller.
Believe me there were a few bonkers iterations before hitting on the final solution!
If you’re not going to use soap, why not just give it all up… sell your shower and make space for your 4K toilet telly!
I won’t go over all the variations I tried. But here are a few thoughts:
- Let’s just put the
belongsToUser
scope call in the controller or make it global on specific models
Seems logical, but then we’re relying on correct developer behaviour to get this right in the right circumstances. We can do our future selves (and others) a favour by making sure that when you apply the correct Policy, you get the correct logic. - We could pass the query explicitly into this one Policy method
We could, but again, if we make changes to it at the ‘wrong’ point in time, we will still end up with the wrong instance being passed to the Policy and won’t have all of our query clauses applied. It also makes our policies slightly less tidy. - Force the model to be built with a specific query builder This sounds good in theory — using the built-in methods that the model has for handling a query builder instance. However, timing is still an issue and it’s trickier to inject an evolving builder object into an existing model instance.
In the end it’s the simplest solution that wins, so here it is.
Explicitly store the current Builder in the model
I added two methods and a property to my base model class that lets me keep a track of a Builder (if I want to):
1protected $currentBuilder 2 3public function getBuilder() 4{ 5 if ($this->currentBuilder instanceof Builder) { 6 return $this->currentBuilder 7 } 8 9 return $this->setBuilder($this->newQuery())10}1112public function setBuilder(Builder $builder)13{14 return $this->currentBuilder = $builder15}
In my controller, I just send an instance of the model to the authorisation handler:
1$this->authorize('index', $this->model)
But now in my Policy:
1public function index($user, $model)2{3 $model->scopeBelongsToUser($model->getBuilder(), $user)4}
Notice that I call the scope method directly by using its full name (as opposed to how you use it in a chain, dropping the scope
prefix). This stops Laravel’s magic from creating a newQuery
for me and lets me pass in the Builder I have control over.
Back in the controller, I can pick up where I left off:
1$this->model->getBuilder()->filter()->sort()->paginate()
I only have to add an explicit call to my getBuilder
method to the chain in order for everything to work as intended.
(Obviously, all of these code samples are over-simplified to show the main gist of what’s going on. So, spoiler alert, maybe it’s best not to copy-paste this garbage into your code and expect it to ‘Just Work™’)
Results
I am able to get the right Builder instance out of my Policy without any unnecessary, one-off arguments or crazy logic or any extra pass-by-reference variables.
I’m relying on the inherent pass-by-reference nature of objects in PHP, using my model as a sort of Trojan horse for the query builder instance I want to play with: I’m basically mending the broken-over-long-distance Builder chain by making the model pick up one end and forcefully pulling it to where it needs to go.
A side benefit is that, if I wish, I can also get the Builder from a relationship and set that as the Builder in my model. So for example, there is a case where I do something like the following, which occurs after my Policy is run but before my other query clauses get added:
1$model->setBuilder($category->products()->getQuery())
Great Scott!
That’s just one way that you can break up the query builder chain to different parts of your application without losing the relevant parts of the query. I’m sure there are others.
I’m also pretty certain this is breaking some holy design principle for ‘good code’ somehow, but it seems to work really well for my use-case, so I’m going to stick with it for now.
Hi, and thanks for reading all the way down here. I’m Simon, Senior Developer at Elvie. Contrary to recent opinion, I do use soap… and shampoo. If you found this useful at all, please leave a clap… heck, go nuts! Leave two.