Problem/Motivation

$response = call_user_func_array(array($resource, $method), array_merge($parameters, array($unserialized, $request)));
This is really error prone, because it demands that each ResourceInterface plugin accepts parameters in a very particular order:

+/**
+ * Provides a resource for resetting a user's password.
+ *
+ * @RestResource(
+ *   id = "user_password_reset",
+ *   label = @Translation("Password reset"),
+ *   uri_paths = {
+ *     "https://www.drupal.org/link-relations/create" = "/user/password/{name}/{langcode}",
+ *   },
+ * )
+ */
…
+  public function post($langcode, $name) {
+    $name = trim($name);

Note how the order of parameters of the post() method MUST be in this particular order.

Another example: #2870649-12: [PP-1] REST should respond with a 409 for a POST request to an existing entity.

Proposed resolution

  1. Use the argument resolver.
  2. But if the arguments resolver fails (exception thrown), fall back to the way this was done in Drupal 8 so far

Remaining tasks

None.

User interface changes

None.

API changes

None.

Data model changes

None.

CommentFileSizeAuthor
#70 2720233-70.patch7.49 KBWim Leers
#70 interdiff.txt594 bytesWim Leers
#68 2720233-68.patch7.43 KBWim Leers
#68 interdiff.txt779 bytesWim Leers
#66 interdiff-2720233.txt2.17 KBdawehner
#66 2720233-66.patch7.21 KBdawehner
#61 interdiff-2720233.txt3.86 KBdawehner
#61 2720233-61.patch7.26 KBdawehner
#52 interdiff-2720233.txt683 bytesdawehner
#52 2720233-52.patch6.56 KBdawehner
#49 interdiff-2720233.txt1.27 KBdawehner
#49 2720233-49.patch6.46 KBdawehner
#44 2720233-44.patch5.18 KBWim Leers
#44 interdiff.txt2.8 KBWim Leers
#43 2720233-43.patch5.5 KBWim Leers
#43 interdiff.txt2.77 KBWim Leers
#39 interdiff-2720233.txt2.45 KBdawehner
#39 2720233-39.patch5.04 KBdawehner
#33 2720233-33.patch4.02 KBWim Leers
#33 interdiff.txt1.03 KBWim Leers
#32 2720233-31.patch4.02 KBWim Leers
#25 2720233-25.patch4.66 KBdawehner
#14 2720233-14.patch3.36 KBdawehner
#11 2720233-11.patch198.9 KBdawehner
#2 2720233-2.patch4.97 KBdawehner
Members fund testing for the Drupal project. Drupal Association Learn more

Comments

dawehner created an issue. See original summary.

dawehner’s picture

Status: Active » Needs review
FileSize
4.97 KB

What about something like this? Its still tricky though

Status: Needs review » Needs work

The last submitted patch, 2: 2720233-2.patch, failed testing.

Wim Leers’s picture

+1 for the principle.

klausi’s picture

Status: Needs work » Needs review
Issue tags: +Needs issue summary update

why is it error prone? how can the controller resolver help us? what kind of issue are we fixing here?

+++ b/core/modules/rest/src/RequestHandler.php
@@ -69,24 +98,32 @@ public function handle(RouteMatchInterface $route_match, Request $request) {
+      if (!empty($definition['use_controller_resolver'])) {

why is this configurable? Can't we do it always?

Otherwise great idea, we should get rid of the code in the else{} clause.

dawehner’s picture

why is this configurable? Can't we do it always?

Well, I wanted to ensure that there is no BC break.

The reason how this issue started is the following required code:

+/**
+ * Provides a resource for resetting a user's password.
+ *
+ * @RestResource(
+ *   id = "user_password_reset",
+ *   label = @Translation("Password reset"),
+ *   uri_paths = {
+ *     "https://www.drupal.org/link-relations/create" = "/user/password/{name}/{langcode}",
+ *   },
+ * )
+ */

+  public function post($langcode, $name) {
+    $name = trim($name);

... Note the order of the parameters.

Wim Leers’s picture

Title: Use controller resolver in core/modules/rest/src/RequestHandler.php » Use controller resolver in RequestHandler

IOW: RequestHandler passes arguments in whatever order they came in, not in the order that is expected.

dawehner’s picture

IOW: RequestHandler passes arguments in whatever order they came in, not in the order that is expected.

Well, I don't know why it happens to be honest, but it does happen and its really not nice.

Wim Leers’s picture

Right. but my interpretation/explanation is correct, right?

klausi’s picture

Title: Use controller resolver in RequestHandler » Use controller resolver in RequestHandler to have conistent parameter ordering
Category: Task » Bug report
Status: Needs review » Needs work

Aha, that would make sense! Then this is more of a bug report and we should just replace the brittle part in RequestHandler.

dawehner’s picture

Status: Needs work » Needs review
FileSize
198.9 KB

Well, let's see whether this actually works

klausi’s picture

Status: Needs review » Needs work

Wrong patch?

The last submitted patch, 11: 2720233-11.patch, failed testing.

dawehner’s picture

Status: Needs work » Needs review
FileSize
3.36 KB

There we go.

Status: Needs review » Needs work

The last submitted patch, 14: 2720233-14.patch, failed testing.

Wim Leers’s picture

Issue tags: +Novice, +php-novice

\Drupal\Tests\rest\Kernel\RequestHandlerTest needs to be updated, then tests will pass again. That seems like a great novice task.

klausi’s picture

+++ b/core/modules/rest/src/RequestHandler.php
@@ -70,24 +99,17 @@ public function handle(RouteMatchInterface $route_match, Request $request) {
+      $request = clone $request;

why do we clone the request here? Please add a comment.

And the issue summary update is still missing.

dawehner’s picture

why do we clone the request here? Please add a comment.

Well, the general idea was to not alter the current request in the middle of nowhere but just change it to be able to pass in the data.

dawehner’s picture

Given that this is a clear BC break (as naming of the variable actually matters), I think we should have a BC layer and some flag to opt into this behaviour.

Wim Leers’s picture

#19: can you point to which variable is being renamed, which is the BC break?

dawehner’s picture

#19: can you point to which variable is being renamed, which is the BC break?

Well, before it was: function ($route_parameters..., $unserialized, $request) exactly in this order and the naming of the variables don't matter.
Afterwards its for example function ($unserializer/$data, $route_param_1, $request, $route_param_2) aka. the order of the variables don't matter anymore, but the names of the variables matter. Previously they could have used $random_1221123 for the $unserialized parameter, which totally breaks with this patch.

Wim Leers’s picture

I understand now. Thanks for explaining.

I have no idea how to provide BC for that.

Bluntly speaking: this is a plain bugfix, and any existing code relying on non-matching-variable names is therefore buggy too.

dawehner’s picture

Bluntly speaking: this is a plain bugfix, and any existing code relying on non-matching-variable names is therefore buggy too.

Well, on the other hand this is most probably just a bugfix for a minority of usecases (more than one parameter in the URL), while the unserialized parameter is passed along in all usecases.

The BC layer is outlined in the first patch, by introducing some flag. One alternative could be to see whether the controller resolver finds every parameter. In case it does, use it, otherwise fallback to the previous logic?

Wim Leers’s picture

One alternative could be to see whether the controller resolver finds every parameter. In case it does, use it, otherwise fallback to the previous logic?

Sounds great!

dawehner’s picture

Status: Needs work » Needs review
FileSize
4.66 KB

Let's see whether this actually works.

There is no interdiff, because this patch is written from scratch.

Status: Needs review » Needs work

The last submitted patch, 25: 2720233-25.patch, failed testing.

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.0-beta1 was released on August 3, 2016, which means new developments and disruptive changes should now be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.0-alpha1 will be released the week of January 30, 2017, which means new developments and disruptive changes should now be targeted against the 8.4.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

Category: Bug report » Task
Issue tags: -Novice, -php-novice

Do we still think this is worth doing?

Also: this is more an improvement than fixing an actual bug, so changing to Task. Finally, this seems very much un-novice :)

dawehner’s picture

Do we still think this is worth doing?

I totally think so, but yeah the BC policy makes some things just impossible hard.

Wim Leers’s picture

Assigned: Unassigned » Wim Leers

the BC policy makes some things just impossible hard.

Hear hear.

Although I think that in this case, there may be a simple small oversight in the test coverage that's causing the failures? :)

Let's see…

Wim Leers’s picture

Status: Needs work » Needs review
FileSize
4.02 KB

First: straight rebase.

Wim Leers’s picture

Assigned: Wim Leers » Unassigned
FileSize
1.03 KB
4.02 KB

Turns out there's just a few trivial bugs in #25 :)

But even with these changes, you only the legacy approach works:

 core/modules/rest/src/RequestHandler.php | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/core/modules/rest/src/RequestHandler.php b/core/modules/rest/src/RequestHandler.php
index 54d9d87..4fb6271 100644
--- a/core/modules/rest/src/RequestHandler.php
+++ b/core/modules/rest/src/RequestHandler.php
@@ -121,12 +121,12 @@ public function handle(RouteMatchInterface $route_match, Request $request) {
     // Determine the request parameters that should be passed to the resource
     // plugin.
     $argument_resolver = $this->getArgumentResolver($route_match, $unserialized, $request);
-    try {
-      $arguments = $argument_resolver->getArguments([$resource, $method]);
-    }
-    catch (\RuntimeException $exception) {
+//    try {
+//      $arguments = $argument_resolver->getArguments([$resource, $method]);
+//    }
+//    catch (\RuntimeException $exception) {
       $arguments = $this->getLegacyParameters($route_match, $unserialized, $request);
-    }
+//    }
 
     // Invoke the operation on the resource plugin.
     $response = $response = call_user_func_array([$resource, $method], $arguments);

The last submitted patch, 32: 2720233-31.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

Status: Needs review » Needs work

The last submitted patch, 33: 2720233-33.patch, failed testing. View results

dawehner’s picture

This is one of the issues which we should maybe move to 9.x as its easy to solve there? On the other hand then its hard to introduce a deprecation notice of some kind properly.

Wim Leers’s picture

Well … http://buytaert.net/making-drupal-upgrades-easy-forever basically says we can't do that. At minimum, we need to already implement it now, in parallel.

I'm honestly convinced that this approach works. We just need to figure out what's broken in the current ArgumentResolver code path. I think you know that far better than I do — could you do that?

dawehner’s picture

I'll give it a try over the weekend, but NOT before my presentation :)

dawehner’s picture

Status: Needs work » Needs review
FileSize
5.04 KB
2.45 KB

Well, its not a bug of the argument resolver, its a problem of what we pass into it. \Drupal\rest\Plugin\rest\resource\EntityResource::get uses $entity as name ... but the name in the route is called like the entity type.

Note: The current fixes still cause some failures ...

Status: Needs review » Needs work

The last submitted patch, 39: 2720233-39.patch, failed testing. View results

Wim Leers’s picture

but the name in the route is called like the entity type.

We can change \Drupal\rest\Plugin\rest\resource\EntityResource::getBaseRoute() to set $parameters['entity'] instead — this is an internal detail, not an API.

dawehner’s picture

We can change \Drupal\rest\Plugin\rest\resource\EntityResource::getBaseRoute() to set $parameters['entity'] instead — this is an internal detail, not an API.

fair, but we also have to deal with $original_entity for example.

Wim Leers’s picture

Status: Needs work » Needs review
FileSize
2.77 KB
5.5 KB

fair, but we also have to deal with $original_entity for example.

:( You're right of course.

But I don't see how the current logic can ever work correctly. For GET+DELETE, $entity is the stored entity. For PATCH+POST, $entity is the entity that is being sent in the request body. If I fix that, and fix the wildcard argument isInstance() exception, then this works.

Wim Leers’s picture

FileSize
2.8 KB
5.18 KB

I think this is simpler to understand.

The last submitted patch, 43: 2720233-43.patch, failed testing. View results

Status: Needs review » Needs work

The last submitted patch, 44: 2720233-44.patch, failed testing. View results

dawehner’s picture

Nice work @Wim Leers!!

  1. +++ b/core/modules/rest/src/RequestHandler.php
    @@ -139,4 +140,91 @@ public function handle(RouteMatchInterface $route_match, Request $request) {
    +    if (isset($request)) {
    

    Request should be defined always, right?

  2. +++ b/core/modules/rest/src/RequestHandler.php
    @@ -139,4 +140,91 @@ public function handle(RouteMatchInterface $route_match, Request $request) {
    +  /**
    +   * @param \Drupal\Core\Routing\RouteMatchInterface $route_match
    +   *   The route match
    ...
    +  protected function getLegacyParameters(RouteMatchInterface $route_match, $unserialized, Request $request) {
    

    This needs some documentation.

Wim Leers’s picture

Assigned: Unassigned » dawehner

From 127 to 3 fails, yay! Assigning to @dawehner to get him to review my changes and hopefully confirm that it's the right direction. And then perhaps he can drive it home too :)

dawehner’s picture

Assigned: dawehner » Unassigned
Status: Needs work » Needs review
FileSize
6.46 KB
1.27 KB

The tests have been a little bit tainted

dawehner’s picture

Issue tags: +API-First Initiative

Status: Needs review » Needs work

The last submitted patch, 49: 2720233-49.patch, failed testing. View results

dawehner’s picture

Status: Needs work » Needs review
FileSize
6.56 KB
683 bytes

Ha, I made a last minute change.

pounard’s picture

Title: Use controller resolver in RequestHandler to have conistent parameter ordering » Use controller resolver in RequestHandler to have consistent parameter ordering

Fixed a typo, I'm sorry, I'm watching this issue since a long time, and it hurts my eyes everytime :)

Wim Leers’s picture

Status: Needs review » Needs work

@pounard++ :D — I can't believe I didn't see that myself!

@dawehner++ for driving this home! I was so tempted to RTBC this, but I think we need to clean this up just a little bit more.

  1. +++ b/core/modules/rest/src/RequestHandler.php
    @@ -119,17 +121,16 @@ public function handle(RouteMatchInterface $route_match, Request $request) {
    +    $argument_resolver = $this->getArgumentResolver($route_match, $unserialized, $request);
    +    try {
    +      $arguments = $argument_resolver->getArguments([$resource, $method]);
    +    }
    +    catch (\RuntimeException $exception) {
    +      $arguments = $this->getLegacyParameters($route_match, $unserialized, $request);
         }
    

    We want this to have a @todo to only keep the "try" case in D9.

  2. +++ b/core/modules/rest/src/RequestHandler.php
    @@ -139,4 +140,91 @@ public function handle(RouteMatchInterface $route_match, Request $request) {
    +   * Creates an argument resolver, which can pass along the REST parameters.
    

    Probably needs rephrasing.

  3. +++ b/core/modules/rest/src/RequestHandler.php
    @@ -139,4 +140,91 @@ public function handle(RouteMatchInterface $route_match, Request $request) {
    +  /**
    +   * @param \Drupal\Core\Routing\RouteMatchInterface $route_match
    +   *   The route match
    +   * @param mixed $unserialized
    +   *   The unserialized data.
    +   * @param \Symfony\Component\HttpFoundation\Request $request
    +   *   The request.
    +   *
    +   * @return array
    +   */
    

    We want to finish the docblock here: add the missing first line, add the missing trialing period, add @return line, and … mark it @deprecated! <3

Wim Leers’s picture

Title: Use controller resolver in RequestHandler to have consistent parameter ordering » Use arguments resolver in RequestHandler to have consistent parameter ordering
Issue summary: View changes
Issue tags: -Needs issue summary update +technical debt, +Needs change record

I finally did the IS update that #5 requested. I also updated the title because this is no longer about using controller resolver, but about using the arguments resolver.

In doing that, I realized that we need to remove the "fall back to legacy approach" at some point, which means we need a CR and we probably want to nudge users in the right direction by …

+++ b/core/modules/rest/src/RequestHandler.php
@@ -119,17 +121,16 @@ public function handle(RouteMatchInterface $route_match, Request $request) {
+    try {
+      $arguments = $argument_resolver->getArguments([$resource, $method]);
+    }
+    catch (\RuntimeException $exception) {
+      $arguments = $this->getLegacyParameters($route_match, $unserialized, $request);
     }

… logging a warning in the case of the "catch" case here.

DuaelFr’s picture

+++ b/core/modules/rest/src/RequestHandler.php
@@ -119,17 +121,16 @@ public function handle(RouteMatchInterface $route_match, Request $request) {
-    $response = call_user_func_array([$resource, $method], array_merge($parameters, [$unserialized, $request]));
+    $response = $response = call_user_func_array([$resource, $method], $arguments);

I might miss something but is it a reason to double the "$response ="?

Wim Leers’s picture

#56: I just wanted to post that same remark :)

Wim Leers’s picture

Issue summary: View changes
Issue tags: +blocker
dawehner’s picture

I'm writing the change record now ...

dawehner’s picture

Issue tags: -Needs change record

I'm trying to address the latest feedback.

dawehner’s picture

Status: Needs work » Needs review
FileSize
7.26 KB
3.86 KB

I hope I was able to address all the feedback.

dawehner’s picture

pounard’s picture

Using legacy menu arguments when the resolver throws exceptions is a very good move, I was looking for a solution in a custom non Drupal project and you just gave it to me ! Thank you very much for this.

Wim Leers’s picture

Status: Needs review » Reviewed & tested by the community

@pounard :)

@dawehner 👍

catch’s picture

Status: Reviewed & tested by the community » Needs work
  1. +++ b/core/modules/rest/src/RequestHandler.php
    @@ -139,4 +141,99 @@ public function handle(RouteMatchInterface $route_match, Request $request) {
    +
    +      // Try to find a parameter which is an entity.
    +      foreach ($route_arguments as $value) {
    +        if ($value instanceof EntityInterface) {
    +          $upcasted_route_arguments['original_entity'] = $value;
    +        }
    +      }
    +    }
    +    else {
    +      // Try to find a parameter which is an entity.
    +      foreach ($route_arguments as $value) {
    +        if ($value instanceof EntityInterface) {
    +          $upcasted_route_arguments['entity'] = $value;
    +        }
    +      }
    +    }
    

    1. Should this break when it finds an entity? If not why not?

    2. Should we find the entity before the if/else, then just assign it in the two code branches?

  2. +++ b/core/modules/rest/src/RequestHandler.php
    @@ -139,4 +141,99 @@ public function handle(RouteMatchInterface $route_match, Request $request) {
    +   * @deprecated in Drupal 8.0.0, will be removed before Drupal 9.0.0. Use the ¶
    

    8.4.0. Also trailing whitespace. Both fixable on commit but while I'm here.

dawehner’s picture

Status: Needs work » Needs review
FileSize
7.21 KB
2.17 KB

1. Should this break when it finds an entity? If not why not?

Good point. IMHO there is no reason to not break here.

2. Should we find the entity before the if/else, then just assign it in the two code branches?

This is also a good idea.

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

Drupal 8.4.0-alpha1 will be released the week of July 31, 2017, which means new developments and disruptive changes should now be targeted against the 8.5.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

Status: Needs review » Reviewed & tested by the community
FileSize
779 bytes
7.43 KB

#66 looks great. It introduced one whitespace bug though, fixed that.

larowlan’s picture

Status: Reviewed & tested by the community » Needs review

Looks good and is some nice clean up - couple of observations

  1. 
    ...
    +      @trigger_error('Passing in arguments the legacy way is deprecated in Drupal 8.4.0 and will be removed before Drupal 9.0.0. Provide the right parameter names in the method, similar to controllers. See https://www.drupal.org/node/2894819', E_USER_DEPRECATED);
    ...
    +   * @deprecated in Drupal 8.4.0, will be removed before Drupal 9.0.0. Use the
    +   *   argument resolver method instead, see ::createArgumentResolver().
    

    We need to reference the change notice here.

  2. +++ b/core/modules/rest/src/RequestHandler.php
    @@ -139,4 +141,98 @@ public function handle(RouteMatchInterface $route_match, Request $request) {
    +    $parameters = [];
    +    // Filter out all internal parameters starting with "_".
    +    foreach ($route_parameters as $key => $parameter) {
    +      if ($key{0} !== '_') {
    +        $parameters[] = $parameter;
    +      }
    +    }
    

    any reason we didn't use array_filter here?

Wim Leers’s picture

Status: Needs review » Reviewed & tested by the community
FileSize
594 bytes
7.49 KB
  1. Unless I'm mistaken, after triple-checking that, that's already the case in the trigger_error() call? Oh you mean in the docblock!
  2. Because we're moving existing code. See the second hunk: this exact code is being moved away from there.

    (If we're moving legacy code into a deprecated helper method that we're going to remove in the future, you have every reason possible to not change it: out of scope change, avoid risk of introducing new edge cases, and so on.)

dawehner’s picture

+1 for not changing the existing code. This minimizes review scope.

larowlan’s picture

Thanks, agree

  • larowlan committed 5679964 on 8.5.x
    Issue #2720233 by dawehner, Wim Leers, catch: Use arguments resolver in...
larowlan’s picture

Status: Reviewed & tested by the community » Fixed

Credited catch for performance improvements.

Committed as 5679964 and pushed to 8.5.x

Published change record.

Would welcome a follow-up to use array_filter as per #69.2

jibran’s picture

Fixed some version details in the change notice. I think it can be improved. @Wim Leers can you please have a look at it? Thanks!

Berdir’s picture

Somehow this causes test fails in monitoring:

https://www.drupal.org/node/2907681#comment-12253309

What I'm doing is kinda strange (a hack on a rest plugin to support returning a list), but still seems strange that this broke that.

dawehner’s picture

I'm curious did you tried changed the parameter name on your function?

dawehner’s picture

I quickly had a look at the problem described by @berdir

The route produced has a path looking like /monitoring-multigraph/{id}.
The method on the plugin looks like this: public function get($multigraph_name = NULL) {, and as such the argument resolver isn't able to resolve the parameter.

In theory our backward compatibility layer should jump in, but it doesn't as the parameter is optional.

Maybe we could do something like storing the method name of the plugin resource which should be called for a given route. Given that @berdir could provide a collection route, that returns all elements and doesn't need anymore an optional parameter.

Berdir’s picture

Thanks @dawehner for the help.

I fixed this now in monitoring in #2908002: Fix HEAD test fails in REST tests.

The question is whether this is a regression/BC break and whether we care enough about that to revert this? I'm OK if we don't, but I'd suggest we should at least document this case then in the change record.

Being able to specify the method of the resource plugin for a route would be quite nice and a nicer workaround than what I did until we have #2100637: REST views: add special handling for collections (I think that's the issue for official list/collection output in rest resources) but that's a separate issue.

Wim Leers’s picture

Being able to specify the method of the resource plugin for a route

Can you clarify this a bit? Can you not already do this by overriding \Drupal\rest\Plugin\ResourceBase::getBaseRoute()?

Berdir’s picture

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’s picture

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’s picture

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?

Wim Leers’s picture

No, that issue is specifically about using REST views for collections, what we're talking about is about specifically not using the views module for retrieving collections via REST.

I created #2908800: RestResource plugins: use get() method with optional parameter (or no parameter) to signal list support? to discuss this, and posted my concrete proposal in #2908800-2: RestResource plugins: use get() method with optional parameter (or no parameter) to signal list support?. Let's continue the discussion there :)

Wim Leers’s picture

I also updated #2893804, #2893804-5: [PP-1] Remove rest.module BC layers includes the interdiff to remove the BC layers introduced in this issue/patch.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.