Problem/Motivation

As reported by @bertramakers in #1964034-13: Pass entity_type into Serializer via context we missed some properties for the Drupal\rest\Annotation\RestResource class.

Proposed resolution

Add current properties as available through

$resource->getPluginDefinition()

in \Drupal\rest\RequestHandler::handle

Remaining tasks

User interface changes

API changes

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

marthinal’s picture

I'm working to create a Rest Resource where anonymous users can register user accounts.

For this resource there's no canonical path and when using Restui module the path is empty.So, can we add for example a "path" to "uri_paths" ?

Attached image with the expected result.

Wim Leers’s picture

Wim Leers’s picture

In fact, the deriver (originally derivative) goes back to when this module was first created (#1816354: Add a REST module, starting with DELETE). When the concept of plugin type-specific annotations was added, it was simply forgotten in #2195571: Add a dedicated @RestResource annotation class.

It looks like annotations are not strict enough…

Wim Leers’s picture

Component: rest.module » plugin system

In fact:

  1. deriver is something supported by \Drupal\Component\Plugin\Discovery\DerivativeDiscoveryDecorator, it does not belong in the annotation definition apparently. (Messy, but apparently that's the way it is intended.)
  2. it looks like entity resources just abuse annotations's lack of validation to add a bunch things that REST module does not care about at all: id and entity_type.

Moving to plugin system to gather feedback.

Wim Leers’s picture

Status: Active » Needs review
FileSize
660 bytes

This part is actually used by \Drupal\rest\RequestHandler::handle(), so at least this seems to belong on the annotation.

Version: 8.0.x-dev » 8.1.x-dev

Drupal 8.0.6 was released on April 6 and is the final bugfix release for the Drupal 8.0.x series. Drupal 8.0.x will not receive any further development aside from security fixes. Drupal 8.1.0-rc1 is now available and sites should prepare to update to 8.1.0.

Bug reports should be targeted against the 8.1.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.2.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.1.x-dev » 8.2.x-dev

Drupal 8.1.9 was released on September 7 and is the final bugfix release for the Drupal 8.1.x series. Drupal 8.1.x will not receive any further development aside from security fixes. Drupal 8.2.0-rc1 is now available and sites should prepare to upgrade to 8.2.0.

Bug reports should be targeted against the 8.2.x-dev branch from now on, and new development or disruptive changes should 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.2.x-dev » 8.3.x-dev

Drupal 8.2.6 was released on February 1, 2017 and is the final full bugfix release for the Drupal 8.2.x series. Drupal 8.2.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.3.0 on April 5, 2017. (Drupal 8.3.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.3.x-dev branch from now on, and new development or disruptive changes should 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

Title: Rest resource annotation class misses some properties » REST resource plugin annotation class misses some properties
Component: plugin system » rest.module
Category: Bug report » Task
FileSize
1.5 KB

Completely new patch, #5 didn't apply anymore.

Wim Leers’s picture

Priority: Normal » Minor
dawehner’s picture

Status: Needs review » Needs work
+++ b/core/modules/rest/src/Annotation/RestResource.php
@@ -41,8 +41,31 @@ class RestResource extends Plugin {
+   * @var string[]
...
+  public $uri_paths;

Should this default to an empty array?

Wim Leers’s picture

Status: Needs work » Needs review
FileSize
504 bytes
1.5 KB

That's probably better, yes.

+++ b/core/modules/rest/src/Annotation/RestResource.php
@@ -41,8 +41,31 @@ class RestResource extends Plugin {
+  /**
+   * The deriver class for this resource plugin, if any.
+   *
+   * @see \Drupal\Component\Plugin\Discovery\DerivativeDiscoveryDecorator::getDeriverClass()
+   *
+   * @var string|null
+   */
+  public $deriver;

Do we want this?

No other plugin annotation has this… (as I already pointed out in #4.1).

dawehner’s picture

No other plugin annotation has this… (as I already pointed out in #4.1).

Seems unneeded then. We should that better add to the base class, IMHO.

Wim Leers’s picture

FileSize
1.55 KB
2.02 KB

Done.

dawehner’s picture

index 790440d..3c78ede 100644
--- a/core/lib/Drupal/Component/Annotation/Plugin.php

--- a/core/lib/Drupal/Component/Annotation/Plugin.php
+++ b/core/lib/Drupal/Component/Annotation/Plugin.php

@@ -27,6 +27,15 @@ class Plugin implements AnnotationInterface {
   /**
+   * The deriver class for this resource plugin, if any.
+   *

Now the documentation ideally should make clear that there are plugin types which don't support deriver

Wim Leers’s picture

  1. +++ b/core/lib/Drupal/Component/Annotation/Plugin.php
    @@ -27,6 +27,15 @@ class Plugin implements AnnotationInterface {
    +   * The deriver class for this resource plugin, if any.
    

    "if any"

  2. +++ b/core/lib/Drupal/Component/Annotation/Plugin.php
    @@ -27,6 +27,15 @@ class Plugin implements AnnotationInterface {
    +   * @var string|null
    

    string|null

Isn't this sufficiently clear?

dawehner’s picture

Well, the question is, do we want to distinct between the case: Someone hasn't defined a deriver class in their annotation vs. the plugin type don't support them.

Wim Leers’s picture

FileSize
783 bytes
1.28 KB

Okay, how about this: we don't deal with deriver at all here, it's out of scope for the REST module anyway :)

dawehner’s picture

Status: Needs review » Reviewed & tested by the community

Okay, how about this: we don't deal with deriver at all here, it's out of scope for the REST module anyway :)

I 100% agree with that

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 18: 2359245-18.patch, failed testing.

Wim Leers’s picture

Status: Needs work » Reviewed & tested by the community

Random fail in Drupal\Tests\inline_form_errors\FunctionalJavascript\FormErrorHandlerCKEditorTest.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 18: 2359245-18.patch, failed testing.

Wim Leers’s picture

Status: Needs work » Reviewed & tested by the community

Random CI infra fail.

  • Gábor Hojtsy committed d535eb9 on 8.3.x
    Issue #2359245 by Wim Leers, clemens.tolboom, marthinal, dawehner: REST...

  • Gábor Hojtsy committed 98bf960 on 8.4.x
    Issue #2359245 by Wim Leers, clemens.tolboom, marthinal, dawehner: REST...
Gábor Hojtsy’s picture

Status: Reviewed & tested by the community » Fixed

The "new" uri_paths key was a bit puzzling to me but I see it is already being used on these resources:

core/modules/dblog/src/Plugin/rest/resource/DBLogResource.php: *   uri_paths = {
core/modules/rest/src/Plugin/rest/resource/EntityResource.php: *   uri_paths = {
core/modules/user/src/Plugin/rest/resource/UserRegistrationResource.php: *   uri_paths = {

So we are indeed just documenting what we already have. Thanks all!

Wim Leers’s picture

Indeed :)

Wim Leers’s picture

Issue tags: +Documentation

This tag is probably also relevant.

Status: Fixed » Closed (fixed)

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