Implemented HTTP PATCH method as update action

[frankmayer]

Implementation for issue #8

Assigned HTTP PATCH to resourceful method 'update'.
Assigned HTTP PUT method to new resourceful method 'replace'

Changed tests accordingly. (Just method count tests inside RoutingTest.php)

Signed-off-by: Frank Mayer frank@frankmayer.net

[frankmayer]

The PATCH method is actually the correct HTTP verb for doing updates, so this should be the update method of a resourceful controller.
PUT, on the other hand should be triggering a replace method in a resourceful controller.

Abstract from the PATCH RFC http://tools.ietf.org/html/rfc5789 :
Several applications extending the Hypertext Transfer Protocol (HTTP)
require a feature to do partial resource modification. The existing
HTTP PUT method only allows a complete replacement of a document.
This proposal adds a new HTTP method, PATCH, to modify an existing
HTTP resource.

RAILS 4 for example has replaced PUT with PATCH. They have kept PUT also pointing to 'update' actions for compatibility, but replace would be better for PUT.
see:

• http://weblog.rubyonrails.org/2012/2/25/edge-rails-patch-is-the-new-primary-http-method-for-updates/
• Use PATCH verb instead of PUT for "update" rails/rails#348
• Use PATCH verb instead of PUT for "update" restful-routing/restful-routing#50

Since PATCH is already being adopted in other frameworks and in "RESTful" API's in the wild, it would be good, if the new awesome L4 would have this baked in from the start.

What do you guys think?

[wishfoundry]

+1 to add support for patch, even for making it the default. But as @machuga points out, most php devs will still use PUT. It may be better to let PUT and PATCH point to the same update method

[frankmayer]

@wishfoundry on the one side you're right and this would keep things as they were (PUT).

On the other side, if we devs are not pointed to do it the right way, we'll continue to do it the "wrong" way. We're humans ;)

Since this is a major version upgrade (3.x -> 4.x), people have to learn its new ways and methods anyway. With an appropriate mention in the docs, people will see that they have to either change their PUT to PATCH, or if that's not possible for any (strange) reason, rewire their controller method like:

function update ($id){
    call_user_func_array(array(&$this, 'replace'), func_get_args() );
}

In both scenarios this should not be a big issue.

[edited] because of wrong code example. Thx @wishfoundry

[wishfoundry]

I suppose one could add something like the following to BaseController.php

function update ()
{
    call_user_func_array(array(&$this, 'replace'), func_get_args() );
}

although how we would get that into the docs is another matter altogether.

[frankmayer]

Yes, that could be a better option.

Didn't get the docs part though? ;)

[ericlbarnes]

One thing to keep in mind is that many JS frameworks tend to use PUT for update requests.
http://backbonejs.org/#Model-save

If the model isNew, the save will be a "create" (HTTP POST), if the model already exists on the server, the save will be an "update" (HTTP PUT).

[frankmayer]

Hi,
Yes, that's right.

That's why @wishfoundry and me proposed an easy way to forward the request from the update() to the replace() method. :)

[wishfoundry]

that works for resourceful routes, but how does that work for http verb magic methods?

[frankmayer]

@wishfoundry Do you have an example for this? I am not sure about what you mean by that in terms of laravel.

[wishfoundry]

magic methods on controllers?

They look like:

getIndex()
postIndex()

etc.

It would be good to have a way to globally override "patch" magic methods to "put" magic methods. Maybe have a setting in config ?

[wishfoundry]

Edit: this probably is a bad idea as it does not redirect request properly, only spoofs the registration process of routes which is dangerous.

How about adding something like this(untested):

protected function createRoute($method, $pattern, $action)
    {
        if(Config::has('app.usePutAsPatch') && Config::get('app.usePutAsPatch'))
            if($method == 'put')
                $method == 'patch';

        if(Config::has('app.usePatchAsPut') && Config::get('app.usePatchAsPut'))
            if($method == 'patch')
                $method == 'put';

[frankmayer]

I see what you meant by magic methods, yes, of course. patchIndex() should work, too, shouldn't it? It's just a matter of implementation and way of handling the forms for example.

On the createRoute() comment: Actually, it's not one or the other. Both can be used in an application.

PATCH is not replacing PUT. PUT is just correctly put to use in order to replace whole records/documents, while PATCH is used to update specific or all parts of records/documents. That's why I initially proposed method redirection for those cases. This can also be done on the base-class, so that other classes inherit from that, and override as necessary.

[wishfoundry]

What I meant is probably more like this

#config/app.php
'legacyPutEnabled' => true,

#router
protected function createRoute($method, $pattern, $action)
    {
        if(Config::has('app.legacyPutEnabled') && Config::get('app.legacyPutEnabled'))
            if($method == 'put' || $method == 'patch')
                $method == 'patch|put';

The goal being to give the developer the option to do whatever he needs to do, regardless of how "incorrect" it may seem. Not everybody can implement PATCH seamlessly.

Ideally, the developer would add a method to the controller like:

public function putIndex()
{
    $this->patchIndex();
}

but this would not be ideal for some large or even medium projects as there is too much repeating yourself code. So I recommend the global legacy mode config flag, allowing developers a choice if their app requires it

[taylorotwell]

I've taken the same approach as Rails 4 with this in routing PATCH to the update method on resource controllers. patch method also added to router and any.

[taylorotwell]

Closed #8

[nicholasruunu]

👍

[frankmayer]

👍