Problem/Motivation
Berdir at #2720233-82: Use arguments resolver in RequestHandler to have consistent parameter ordering:
What he means is this line:
$method = strtolower($request->getMethod());
The problem is you can define whatever you want in your routes.. the concept that http method == php methed on the RestResource plugin is hardcoded and basically impossible to change without duplicating the whole method there.
Instead, it would like this:
$method ? $route_match->getRouteObject()->getOption('_rest_method') ?: strtolower($request->getMethod());
Then I could just say _rest_method = 'list' in my routes() method and wouldn't need my current ugly hacks.
Wim Leers at #2720233-82: Use arguments resolver in RequestHandler to have consistent parameter ordering:
Ah, I see! That's indeed one of the more questionable design decisions about
Resource(Interface|Base)
+RequestHandler
.I do find
_rest_method = 'list'
confusing too though. The REST module basically has spent no time/thought on how to deal with collections. AFAICT it deferred that entirely to Views. Which is often undesirable or overkill of course. I think the way you implemented it (http://cgit.drupalcode.org/monitoring/tree/src/Plugin/rest/resource/Moni... + http://cgit.drupalcode.org/monitoring/tree/src/Plugin/rest/resource/Moni...), with a singleget()
responsible for both individual and collectionGET
requests with an optional parameter makes total sense. Shall we discuss that further in a follow-up, to officially support that pattern? So that you don't need thoseroutes()
overrides anymore?Then we could at least start having basic collection GET support. If you're +1, either create a follow-up or comment here and I'll create that follow-up.
Berdir at #2720233-83: Use arguments resolver in RequestHandler to have consistent parameter ordering:
Opening an issue certainly makes sense, but I'm not sure that it should be like I did with monitoring. For one thing, configuration is currently tied to methods, what if a resource doesn't support lists or only supports lists, or supports both but you only want to enable one of the two things? But we can discuss that in a dedicated issue. I already mentioned #2100637: REST views: add special handling for collections above, not sure if that *is* that issue but it's certainly related?
Proposed resolution
TBD
Remaining tasks
Discuss.
User interface changes
None.
API changes
API addition: details TBD
Data model changes
None.
Comments
Comment #2
Wim LeersI'd propose to make
\Drupal\rest\Plugin\ResourceBase::routes()
generate up to two routes for theget()
method:GET
get()
method with either a non-optional parameter, or an optional parameter:get($something)
orget($something = NULL)
GET
get()
method with either an optional parameter, or no parameter at all:get($something = NULL)
orget()
That way, the 3 possible permutations are supported:
GET
(get($something)
)GET
(get()
)GET
and resource collectionGET
(get($something = NULL)
)Thoughts?
Comment #3
dawehnerAt least for me any kind of optional parameter makes things weird ...
I think the simplest alternative here would be to allow the resource plugin to control this behaviour.
a) Add a new interface looking sort of similar like this:
b) The request handler could then call to the plugin instead of calling
$response = call_user_func_array([$resource, $method], $arguments);
Comment #4
Grayside CreditAttribution: Grayside at Phase2 commentedThe problem here is that it's not one resource. It's two resources.
A "collection resource" is likely to support POST and GET. An "individual item resource" will probably support GET, PUT, PATCH, and DELETE.
Since the name implies plugins:resources as 1:1 relationships, then I would expect a second plugin for list support to keep it flexible.
Contrib/individual sites may want to extend behaviors on top of individual item resources to create a POST effect, for example triggering bulk delete of a collection.
Comment #5
dawehner@Grayside
Based upon my proposal you would get different routes passed along and as such you could make some decisions, which of your actual functions you want to call?
Comment #6
Grayside CreditAttribution: Grayside at Phase2 commentedThe proposal makes sense as a way of making the existing plugins more flexible to cover this use case. My counter argument to this approach is that it takes a RestResource plugin and makes it a single plugin for two resources. The temptation to combine them (and that might well be better for Drupal DX, not judging that) is because it's a common entity type associated with both.
You can argue it's a single resource with an optional parameter, but that variation on the URL/route only seems subtle because of the clean URL design. Collections of different entity types tend to have at least as much in common with each other as they do with the individual items of the collection.
Comment #7
dawehnerAt least I think optional parameters are a flaw. I think on the REST domain level there is no notion of optional parameters. There is a collection resource and there is an individual object resource.
Comment #9
Wim Leers#7 I think that would imply that all we need to do here is write documentation, in which we would recommend to write two REST resource plugins: one for individual resources, one for resource collections. Is that what you meant?
Comment #10
dawehnerI was reading a bit about REST interfaces across the interwebs. One thing I stumbled upon was https://en.wikipedia.org/wiki/Representational_state_transfer#Applied_to...
As you see there GET is not the only place where we could support collections. Interesting
Well, either we do that, or we give them the freedom to provide custom methods. To be honest it is quite magical that the HTTP verb maps to the method name. It feels like it would have been nice if it would have been more explicit in the first place.
Comment #11
Wim LeersYep, this is why the
POST
support in core REST is kind of awkward. http://jsonapi.org/format gets this right: youPOST
a new item to its collection!True. But deviating from that initial design choice will make the architecture less clear/more confusing: every plugin method would correspond to a HTTP method except for those custom mapped ones.
Comment #12
Wim LeersI think it's clear this is not going anywhere. It was just an idea I wanted to check with the wider community. And we did. We decided against it. That's fine! 🙂