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 single get() responsible for both individual and collection GET 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 those routes() 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

Wim Leers created an issue. See original summary.

Wim Leers’s picture

Issue summary: View changes

I'd propose to make \Drupal\rest\Plugin\ResourceBase::routes() generate up to two routes for the get() method:

An individual resource GET
if the resource plugin has a get() method with either a non-optional parameter, or an optional parameter: get($something) or get($something = NULL)
A resource collection GET
if the resource plugin has a get() method with either an optional parameter, or no parameter at all: get($something = NULL) or get()

That way, the 3 possible permutations are supported:

  1. only individual resource GET (get($something))
  2. only resource collection GET (get())
  3. both individual resource GET and resource collection GET (get($something = NULL))

Thoughts?

dawehner’s picture

At 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:

interface RestResourceCallInterface {
  public function call($request, $route)
}

b) The request handler could then call to the plugin instead of calling $response = call_user_func_array([$resource, $method], $arguments);

Grayside’s picture

The 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.

dawehner’s picture

@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?

Grayside’s picture

The 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.

dawehner’s picture

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.

At 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.

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.0-alpha1 will be released the week of January 17, 2018, which means new developments and disruptive changes should now be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Wim Leers’s picture

#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?

dawehner’s picture

I 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

#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?

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.

Wim Leers’s picture

As you see there GET is not the only place where we could support collections.

Yep, this is why the POST support in core REST is kind of awkward. http://jsonapi.org/format gets this right: you POST a new item to its collection!

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.

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.

Wim Leers’s picture

Status: Active » Closed (works as designed)

I 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! 🙂