Framework: Resource controller references DummyModelClass instead of INT

Created on 16 Mar 2017  路  30Comments  路  Source: laravel/framework

  • Laravel Version: 5.4.15
  • PHP Version: 7.1
  • Database Driver & Version: MySQL 5.6

Description:

The file: https://github.com/laravel/framework/blob/master/src/Illuminate/Routing/Console/stubs/controller.model.stub#L48

Steps To Reproduce:

  1. php artisan make:model -cr testing
  2. generated model gives a type hint of public function show(testing $testing) instead of public function show(int $testing)
  3. The auto-generated file should have an integer - the model type hint hasn't worked for me, unless I'm doing something wrong. The docs even reference the old way.

Most helpful comment

The model name doesn't have to do with the parameter at all. Your method signature (the type hint of the parameter with the same name as your route parameter) is what causes the 'implicit' binding.

Route::get('test/{something}', function (App\ToDo $something) { ... });

$something is your ToDo model binding. There is a parameter in your function/method that matches a route parameter name. Laravel then knows by its typehint (if a model class) to do the binding on that parameter for that Model (the one you type-hinted).
That is implicit binding in a nutshell.

All 30 comments

This is setup for route model bindings which are implicit.

Thank you - From there - how do I get the data from the model inside the controller?

The same way you get data out of any model. Not sure what you are asking. Does your route that points to this controller method have a parameter named testing?

Sorry - not enough context.

Inside of the generated controller, I have this code

         /**
     * Display the specified resource.
     *
     * @param  \App\ToDo $toDo
     *
     * @return \Illuminate\Http\Response
     */
    public function show(ToDo $toDo) {
        //
        return view('todo.show', ['todo' => ToDo::find($toDo)]);
    }

The way that is with the ToDo type hint, I'm unable to get the model to return data to the view - not sure what I'm doing wrong. (tried about 15 variations of returning data)

What I had to change it to get it to work:

    /**
     * Display the specified resource.
     *
     * @param  int $toDo
     *
     * @return \Illuminate\Http\Response
     */
    public function show( $toDo) {
        //
        return view('todo.show', ['todo' => ToDo::find($toDo)]);
    }

$toDo is the model instance. You don't need to query the database again to get it, you already have it. That is how route model binding works.

That's what I was thinking too...

controller

    /**
     * Display the specified resource.
     *
     * @param  \App\ToDo $toDo
     *
     * @return \Illuminate\Http\Response
     */
    public function show(ToDo $toDo) {
        //
        return view('todo.show', ['todo' => $toDo]);
    }

view

<h1>One TODO</h1>

{{$todo->task}}
{{$todo->done}}

image result

Here's the entire app zipped - teaching this as part of a class - so I can't share the root repo: http://cl.circletr.ee/2s3f1G0K3g2X

Route::get('todo/{todo}', function (App\Todo $todo) {
    dd($todo);
});

http://yoursite/todo/1 That should be an existing model with id = 1 if not it should throw a not found exception.

I'm using Route::resource('todo', 'ToDoController'); resource routes

Route::get('todo/{todo}', function (App\Todo $todo) {
    dd($todo);
}); 

works - is the implicit model binding incompatible with the resource controller?

You are supposed to match the route parameter to the controller parameter if you want the binding. toDo != todo

I KNEW THAT WOULD BITE ME!
馃槖
what's the recommended convention for naming & URI's?

The parameters can be what you want. Just make sure they match up. You can do an artisan route:list to see what the parameter is for your resources, or you can tell it what you want the parameter to be named.

Implicit Bindings
Naming Resource Route Parameters

Half way there!

 Route::resource('ToDo', 'ToDoController',  ['parameters' => [
    'ToDo' => 'todo'
]]);

Now I just need to strtolower the root part of the URI ;-)

Solved

Route::resource('todo', 'ToDoController',  ['parameters' => [
    'todo' => 'ToDo'
]]);

@lagbox THANK YOU!!! 馃挴馃檹馃徎馃

If you want the URI todo/1 and you want the route parameter to be todo then just do

Route::resource('todo', 'ToDoController');

It takes the resource name and makes it singular to come up with the route parameter name by default.

And no problem :-)

@lagbox so the problem I see with this approach: Route::resource('todo', 'ToDoController'); is that the model name is ToDo, and this route:list results:

| Domain | Method    | URI              | Name         | Action                                      | Middleware   |
+--------+-----------+------------------+--------------+---------------------------------------------+--------------+
|        | GET|HEAD  | /                |              | Closure                                     | web          |
|        | GET|HEAD  | api/user         |              | Closure                                     | api,auth:api |
|        | GET|HEAD  | todo             | todo.index   | App\Http\Controllers\ToDoController@index   | web          |
|        | POST      | todo             | todo.store   | App\Http\Controllers\ToDoController@store   | web          |
|        | GET|HEAD  | todo/create      | todo.create  | App\Http\Controllers\ToDoController@create  | web          |
|        | GET|HEAD  | todo/{todo}      | todo.show    | App\Http\Controllers\ToDoController@show    | web          |
|        | PUT|PATCH | todo/{todo}      | todo.update  | App\Http\Controllers\ToDoController@update  | web          |
|        | DELETE    | todo/{todo}      | todo.destroy | App\Http\Controllers\ToDoController@destroy | web          |
|        | GET|HEAD  | todo/{todo}/edit | todo.edit    | App\Http\Controllers\ToDoController@edit    | web          |
+--------+-----------+------------------+--------------+---------------------------------------------+--------------+

By using the 'reverse' parameter map, I'm able to keep the URI portion lower case, while still referencing the proper model ToDo. Where Laravel figure out where to 'magically' load the class? Is it a language feature of PHP? Or is it parsing the type hint inside of the controller? Really piqued my curiosity, you have!

The model name doesn't have to do with the parameter at all. Your method signature (the type hint of the parameter with the same name as your route parameter) is what causes the 'implicit' binding.

Route::get('test/{something}', function (App\ToDo $something) { ... });

$something is your ToDo model binding. There is a parameter in your function/method that matches a route parameter name. Laravel then knows by its typehint (if a model class) to do the binding on that parameter for that Model (the one you type-hinted).
That is implicit binding in a nutshell.

Are we using PHP Reflection to grab the hint?

Yes it is using reflection to get the information about the parameters of the method. Also that is how the container can do the 'dependency injection' for you and resolve classes with type hinted parameters in their constructors.

For this particular binding feature you can follow this path to get to the reflection that is being used:
Illuminate\Routing\ImplicitRouteBinding@resolveForRoute
Illuminate\Routing\Route@signatureParameters
Illuminate\Routing\RouteSignatureParameters@fromAction

So it's reading the type hint based on the Route::get, not the type hint inside of the ToDoController, correct? That'd explain why I had to add in the

Route::resource('todo', 'ToDoController',  ['parameters' => [
    'todo' => 'ToDo'
]]);

parameter argument. Then everything works. Is that thinking correct?

You don't have to rename the parameter. You can just use Route::resource('todo', 'ToDoController');. Then the signature for the methods you want the model binding for would look like so:

public function show(ToDo $todo) { ... }

If you wanted to bind a different model:

public function show(App\User $todo) { ... }

Now you have a User model binding.

We are matching the parameter name of the route ( URI: todo/{todo} ) to the parameter name in the method, $todo. That allows the binding to happen (and the fact we have it type-hinted as a Model class). You can change these all you want, as long as the parameter name used in the route matches the parameter name used in your method, you will get your binding :-)

That's what I was thinking too... but I cannot get it to work that way. Here's the code to reproduce the issue, maybe I'm missing something.

web.php route:

Route::resource('todo', 'ToDoController');

ToDoController.php:

    /**
     * Display the specified resource.
     *
     * @param  \App\ToDo $ToDo
     *
     * @return \Illuminate\Http\Response
     */
    public function show(ToDo $ToDo) {
        return view('todo.show', ['todo' => $ToDo]);
    }

ToDo model

/**
 * Class ToDo
 * @package App
 *
 * @property task the name of the task
 */
class ToDo extends Model
{
    //
}

You keep naming your method parameter $ToDo when the route parameter is {todo}. ToDo != todo.

public function show(ToDo $todo) { ... }

Would be the correct way to do it as $todo matches {todo}. todo == todo

The important part is matching the route parameter name exactly to your method's parameter name.
todo/{todo}, the parameter name is todo.
public function show(ToDo $todo), the parameter name is todo.

Eureka! Bam - that works!

@lagbox Is this summary correct? http://cl.circletr.ee/3u1a310F1x1G
Diagram

Looks good ;-)

Was this page helpful?
0 / 5 - 0 ratings

Related issues

felixsanz picture felixsanz  路  3Comments

SachinAgarwal1337 picture SachinAgarwal1337  路  3Comments

lzp819739483 picture lzp819739483  路  3Comments

digirew picture digirew  路  3Comments

iivanov2 picture iivanov2  路  3Comments