HTTP status code "405 Method Not Allowed" (HTTP/1.1: Status Code Definitions) is used when a client tries to use a verb on a resource and it is not supported. Along with the 405, the client should receive a list of support verbs in the "Allow" header of the response. If the controller only has "show" and "update" actions and someone tries to call destroy with the "DELETE" verb then they should receive a 405 with "Allow: GET, PUT" header.
I've be working on a patch (Parked at Loopia) and it seems to work fine. I just don't like the implementation of it. Seems like it could be done much cleaner.
Could someone clean up my patch a little or is this whole "Method Not Allowed" thing a bad idea?
I think it’s a good idea, actually. The patch needs slight reformatting, though. If you’ve gotten this far, why put it on someone else’s back when you can simply finish it up?
As Core guys would say - “be a hero, implement it!”
I think it's a good idea, actually. The patch needs slight reformatting,
though. If you've gotten this far, why put it on someone else's back when
you can simply finish it up?
Its not what you think, I haven't given up. Just looking for some
pointers in the right direction.
Rick Olson wrote:
+1, it goes with the use of 406 Not Acceptable too.
I was thinking to replace all "ActionController::UnknownAction" with
"501 Not Implemented" errors. I think i could make the non-restful
goers mad. It could just be restricted to restful controllers.
Another issue I had was determining if a controller is a resource or
not. With that patch, I just check if there is a named route for the
controller following resource conventions. Its not that full prove.
Maybe I need to write a patch for this first.
"PostsController.resource?" would be useful. Still need to investigate
if there is a nice way to grab the map routes from a controller.
I was thinking to replace all "ActionController::UnknownAction" with
"501 Not Implemented" errors. I think i could make the non-restful
goers mad. It could just be restricted to restful controllers.
What about 404 like it is currently?
Another issue I had was determining if a controller is a resource or
not. With that patch, I just check if there is a named route for the
controller following resource conventions. Its not that full prove.
Maybe I need to write a patch for this first.
"PostsController.resource?" would be useful. Still need to investigate
if there is a nice way to grab the map routes from a controller.
I don't think that's necessary. It should just be a special exception
routing raises when a request doesn't match the correct request method
condition.
> I was thinking to replace all "ActionController::UnknownAction" with
> "501 Not Implemented" errors. I think i could make the non-restful
> goers mad. It could just be restricted to restful controllers.
What about 404 like it is currently?
It would go allow more with an unknown aspect like "GET
/posts/1;unknown".
> Another issue I had was determining if a controller is a resource or
> not. With that patch, I just check if there is a named route for the
> controller following resource conventions. Its not that full prove.
> Maybe I need to write a patch for this first.
> "PostsController.resource?" would be useful. Still need to investigate
> if there is a nice way to grab the map routes from a controller.
I don't think that's necessary. It should just be a special exception
routing raises when a request doesn't match the correct request method
condition.
Thats why I didn't really like my current implementation either. I have
to dig into routes more.
Another problem is map.resource creates routes to all the actions
whether you had them or not. Thats why I hijacked the UnknownAction
call and checked if it was a restful controller.
Working on a much cleaner patch (Parked at Loopia). Still
not done, I just wanted to post my work before I feel asleep.
I've modified the routing lib instead of the rescue lib. Works much
nicer. I've added some unit tests that still aren't passing, this time
its the allowed methods.
One question Rick, could I make (I mean let me) map.resource only map
to actions that exist? So if there were no destroy action, that route
would never exist. That way it would call a routing error and I'd be
able to pick it up and determine if that were other http verbs that
should be used instead.
The code is nice and clean but I still have a tiny complaint about the
allow headers. I don't really think "PUT, DELETE" should be returned if
the resource is a list/collection. This is one of those debatable
things.
The patch is the core functionally and we can allows make it more
proper later.
Awesome! If you wanted you could make the change and reattach the patch
in the ticket. That way you'd get official credit.
Also line #15 should read "assert_equal "GET, POST", e.message" instead
of "assert_equal "GET, POST, PUT, DELETE", e.message". The only problem
is that the test fails.
