Problem/Motivation

When you try to create an entity through REST and it doesn't have defined a canonical URL, it returns:

No link template "canonical" found for the "paragraph" entity type.

Proposed resolution

The Post method from EntityResource assumes that the created entity has an url and try to return it:

        $url = $entity->urlInfo('canonical', ['absolute' => TRUE])->toString(TRUE);
        return new ModifiedResourceResponse($entity, 201, ['Location' => $url->getGeneratedUrl()]);

The attached patch checks first it there is a canonical URL.

Remaining tasks

* Review

User interface changes

None

API changes

Post method for entity resource shouldnt try to return the entity URL if it doesnt have one.

Data model changes

None

Comments

ruloweb created an issue. See original summary.

ruloweb’s picture

ruloweb’s picture

For 8.3.x it will require a new patch, because some lines have changed, but lets review this one first.

ruloweb’s picture

Issue tags: +SprintWeekend2017
dagmar’s picture

Status: Needs review » Needs work
Issue tags: +Needs tests
+++ b/core/modules/rest/src/Plugin/rest/resource/EntityResource.php
@@ -171,11 +171,16 @@ public function post(EntityInterface $entity = NULL) {
+      if ($entity->hasLinkTemplate('canonical')) {

Makes sense. We now need a test for this that doesn't include the canonical url.

ruloweb’s picture

wim leers’s picture

Great find!

+++ b/core/modules/rest/src/Plugin/rest/resource/EntityResource.php
@@ -171,11 +171,16 @@ public function post(EntityInterface $entity = NULL) {
+      if ($entity->hasLinkTemplate('canonical')) {

Makes sense!

But rather than creating a response object in two places, let's do that once. And if this if-test is true, only then do:

$response->headers->add(…);

That makes the code a bit easier to follow.


For test coverage, I recommend updating EntityResourceTestBase::testPost() (grep for 201). However, to then test this, we need an entity type in Drupal core that doesn't have a canonical link template.

AFAICT the only existing example of a canonical-less content entity type is \Drupal\entity_test\Entity\EntityTestNoId (yes, I inspected every single content entity type class in core).

wim leers’s picture

Title: No link template "canonical" found when you try to POST entities like doesn't have canonical URLs » PHP error when POSTing content entities without a "canonical" link template

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.

ruloweb’s picture

Status: Needs work » Needs review
StatusFileSize
new1.2 KB

Thanks @Wim Leers for the suggestions, attached is a new version updated for 8.3.x, I am working on the test case now.

Change to review to run tests.

wim leers’s picture

Status: Needs review » Needs work
+++ b/core/modules/rest/src/Plugin/rest/resource/EntityResource.php
@@ -170,8 +170,15 @@ public function post(EntityInterface $entity = NULL) {
+      // canonical-less entities has no URL to redirect.

Let's remove this comment. It's clear from the code what's going on here.

Then the only thing that's still necessary is test coverage; I gave pointers for that in #7 :)

shadcn’s picture

Sidenote: \Drupal\aggregator\Entity\Item does not have canonical route either. But it implements buildUri. Came across this while working on #2843752: EntityResource: Provide comprehensive test coverage for Item entity.

wim leers’s picture

Great, that means we have two examples!

wim leers’s picture

See #2853211-25: EntityResource::post() incorrectly assumes that every entity type has a canonical URL. That issue solves this issue: it has all the necessary test coverage. It's RTBC.

Closing this as a duplicate, and then migrating this issue's tags.