Route model binding behaviour changed in 9.14

[dwightwatson]

• Laravel Version: 9.14.0
• PHP Version: 8.1.6
• Database Driver & Version:

Description:

I've just discovered a breaking change that appeared between 9.13.0 and 9.14.0. I've got a complex use-case but I've tried to break it down. Here's the route set up that was working in 9.13.0.

Route::get('{university}/{campus:slug}/categories/{category}', [...]);

The intention here is that the campus record is scoped to the institution and that the category is not scoped to other route models. However, after upgrading to 9.14.0 it appears Laravel is also trying to scope the category to the campus and blowing up because the association does not exist:

Call to undefined method App\Models\Campus::categories()

Totally appreciate if perhaps my mixed use of nested routing and non-nested routing is unintended behaviour and that this is actually a fix, but thought it was worth bringing up. I'll roll back to 9.13.0 in the meantime.

Steps To Reproduce:

I have created a repo here: https://github.com/dwightwatson/routing-bug

If you pull it down and run php artisan test it should pass using the SQLite database. If you open composer.json and bump to Laravel 9.14.0 and re-run the tests it should fail.

[driesvints]

@ksassnowski could this be because of your PR? #42425

@dwightwatson what happens if you revert those changes?

Right now, there indeed is no documentation about this behavior for three params: https://laravel.com/docs/9.x/routing#implicit-model-binding-scoping

[dwightwatson]

Yeah, it works on Laravel 9.14.0 if I revert the changes from that PR.

[dwightwatson]

I get that my use-case is probably uncommon (will be interesting to see if it affects other people as well) so I'm not fussed if this becomes the new default behaviour. But perhaps a good opportunity to make a call on what should happen when dealing with more than 2 params.

[makroxyz]

I have the same issue after upgrade to 9.14

[ksassnowski]

This probably due to this line here https://github.com/laravel/framework/blob/9.x/src/Illuminate/Routing/ImplicitRouteBinding.php#L49

It checks if the parameter name exists in the route's binding field. Since my change made it so that if any of the parameters provides a custom binding field, every parameter gets added to the route's binding field (but possibly with null). This would mess up the array_key_exists call though. The fix would probably be to also check that the value for the binding field isn't null before treating it as a scoped binding.

[ksassnowski]

I'll send a PR for this shortly.

[ksassnowski]

@driesvints Can you maybe point me to the tests that cover the ImplicitRouteBinding class? It doesn't seem to have its own tests.

[ksassnowski]

Nevermind, I don't know why PhpStorm didn't find ImplicitRouteBindingTest...

[ksassnowski]

I've submitted a PR (#42517) that fixes this. I've also tested the changes against the repository provided by @dwightwatson and the tests pass again.

[dwightwatson]

Thanks for the quick turnaround @ksassnowski, champion!