Problem/Motivation

1) In cases where the entity_type is known, it should be passed into Serializer. This will be particularly important if we create an EntityDiff class for PATCH requests.

2) As of drupal-8.0.0-beta6 you still can't do a PATCH or POST using application/json because it will return an

"error": "Entity type parameter must be included in context."

However, using a hal+json and its _links type already works fine to do POST or PATCH.

This patch would allow POST and PATCH using application/json.

Proposed resolution

Add context to the serializer.

Remaining tasks

Needs tests
Needs annotations. See #27
Fix annotations as mentioned by @bertramakers in #13 See instead: #2359245: REST resource plugin annotation class misses some properties

User interface changes

API changes

Original report by @linclark

In cases where the entity_type and bundle are known, they should be passed into Serializer. This will be particularly important if we create an EntityDiff class for PATCH requests.

Files: 
CommentFileSizeAuthor
#35 pass_entity_type_into-1964034-35.patch2.62 KBclemens.tolboom
#21 pass_entity_type_and-1964034-21.patch2.6 KBclemens.tolboom
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 82,592 pass(es). View
#19 pass_entity_type_and-1964034-19.patch2.57 KBclemens.tolboom
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 78,992 pass(es). View
#19 interdiff-1964034-15-19.txt935 bytesclemens.tolboom
#15 pass_entity_type_and-1964034-15.patch2.58 KBclemens.tolboom
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 78,857 pass(es), 3 fail(s), and 0 exception(s). View
#11 pass_entity_type_and-1964034-11.patch2.55 KBclemens.tolboom
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 78,819 pass(es), 3 fail(s), and 0 exception(s). View
#8 pass_entity_type_and-1964034-8.patch3.64 KBclemens.tolboom
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Failed to run tests: PHP Fatal error encountered during run_tests.sh. See review log for details.. View
#3 rest-deserialize-context-1964034-3.patch2.58 KBklausi
FAILED: [[SimpleTest]]: [MySQL] 56,131 pass(es), 1 fail(s), and 0 exception(s). View

Comments

klausi’s picture

entity_type should not be a problem, we could place serialization context parameters on the resource plugin information.

The subtype is a problem since that information is only present in the data, not in the URL, not in the plugin info, not in some HTTP headers.

Should the deserialization normalizer for entities just perform an entity load if an ID is present? It could get the subtype from the loaded object.

linclark’s picture

Would it be possible to pass in subtype information when it is known? For example, if I'm posting a PATCH request to node/1, the system already knows what the bundle is. I'd be fine with not having the bundle on POST or DELETE, but it would be helpful on PATCH.

klausi’s picture

FileSize
2.58 KB
FAILED: [[SimpleTest]]: [MySQL] 56,131 pass(es), 1 fail(s), and 0 exception(s). View

Here is a patch that demonstrates how the deserialization context could be built. It passes entity_type and resource_id on to the serializer and deserialization chain.

Crell’s picture

Status: Active » Needs review
Crell’s picture

Issue tags: -WSCCI

Status: Needs review » Needs work

The last submitted patch, rest-deserialize-context-1964034-3.patch, failed testing.

chx’s picture

Issue summary: View changes

bump.

clemens.tolboom’s picture

Status: Needs work » Needs review
FileSize
3.64 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Failed to run tests: PHP Fatal error encountered during run_tests.sh. See review log for details.. View

Rerolled #3. This needed ->id() and a line removed in core/modules/serialization/src/Normalizer/EntityNormalizer.php which puzzles me.

Let's see what the testbot finds.

Status: Needs review » Needs work

The last submitted patch, 8: pass_entity_type_and-1964034-8.patch, failed testing.

clemens.tolboom’s picture

Drupal\Tests\serialization\Unit\Normalizer\ConfigEntityNorma   1 passes
PHP Fatal error:  Call to a member function hasKey() on a non-object in /var/lib/drupaltestbot/sites/default/files/checkout/core/modules/serialization/src/Normalizer/EntityNormalizer.php on line 55
PHP Warning:  Invalid argument supplied for foreach() in /var/lib/drupaltestbot/sites/default/files/checkout/core/scripts/run-tests.sh on line 588

Warning: Invalid argument supplied for foreach() in /var/lib/drupaltestbot/sites/default/files/checkout/core/scripts/run-tests.sh on line 588

Drupal\rest\Tests\UpdateTest 59 passes 2 fails
Drupal\rest\Tests\NodeTest 33 passes 1 fails

Fatal error: Call to a member function hasKey() on a non-object in /var/lib/drupaltestbot/sites/default/files/checkout/core/modules/serialization/src/Normalizer/EntityNormalizer.php on line 55

FATAL Drupal\serialization\Tests\EntitySerializationTest: test runner returned a non-zero error code (255).
clemens.tolboom’s picture

Status: Needs work » Needs review
FileSize
2.55 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 78,819 pass(es), 3 fail(s), and 0 exception(s). View
  1. +++ b/core/modules/rest/src/Plugin/Derivative/EntityDerivative.php
    @@ -98,6 +98,9 @@ public function getDerivativeDefinitions($base_plugin_definition) {
    +            'entity_type' => $entity_type,
    

    Just use the ->id()

  2. +++ b/core/modules/serialization/src/Normalizer/EntityNormalizer.php
    @@ -48,7 +48,7 @@ public function denormalize($data, $class, $format = NULL, array $context = arra
    -    $entity_type = $this->entityManager->getDefinition($context['entity_type']);
    +    $entity_type = $context['entity_type'];
    

    Reverted

  3. +++ b/core/modules/serialization/src/Normalizer/EntityNormalizer.php
    @@ -58,7 +58,7 @@ public function denormalize($data, $class, $format = NULL, array $context = arra
    -    return $this->entityManager->getStorage($context['entity_type'])->create($data);
    +    return $this->entityManager->getStorage($context['entity_type']->id())->create($data);
    

    Reverted

Status: Needs review » Needs work

The last submitted patch, 11: pass_entity_type_and-1964034-11.patch, failed testing.

bertramakers’s picture

Latest patch fixes the current issue, but seems to break PATCH requests (see failing tests).

Also the serialization_context annotation should be added to the Drupal\rest\Annotation\RestResource class so other developers know that it exists, what it does, and how they should use it.
Other annotations seem to be missing as well, the class only contains id and label at the moment.

clemens.tolboom’s picture

Issue summary: View changes
clemens.tolboom’s picture

FileSize
2.58 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 78,857 pass(es), 3 fail(s), and 0 exception(s). View

As patch tests fail why not add the request_method back.

$context = array('request_method' => $method);

Not tested locally due to grogginess

clemens.tolboom’s picture

Status: Needs work » Needs review
clemens.tolboom’s picture

@bertramakers can you please create a new issue regarding the annotations mentioned in #13. It's worth a new one.

Status: Needs review » Needs work

The last submitted patch, 15: pass_entity_type_and-1964034-15.patch, failed testing.

clemens.tolboom’s picture

Status: Needs work » Needs review
FileSize
935 bytes
2.57 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 78,992 pass(es). View

(sigh) wrong order of setting array (key) value(s)

clemens.tolboom’s picture

[edit]Irrelevant[/edit]

clemens.tolboom’s picture

FileSize
2.6 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 82,592 pass(es). View

Initialize $context array.

clemens.tolboom’s picture

dawehner’s picture

@clemens.tolboom
I am a bit confused here, the issue title and IS talk about bundles, how they are added here?

clemens.tolboom’s picture

Issue summary: View changes
Status: Needs review » Needs work

@dawehner thanks for pointing out my 'blindness'. I did 'just a reroll' :-(

Updated the summary accordingly.

This will be particularly important if we create an EntityDiff class for PATCH requests.

The method \Drupal\rest\Plugin\rest\resource\EntityResource::patch does not use the non-existing EntityDiff either This issue is the only issue mentioning EntityDiff https://www.drupal.org/project/issues/drupal?text=entitydiff&version=8.x

Status: Needs work » Needs review

klausi’s picture

Status: Needs review » Needs work

I agree that at least the added serialization_context key should be added to the annotation at Drupal\rest\Annotation\RestResource.

And this needs a phpunit test case which demonstrates that a mocked serializer indeed receives the entity type in the serialization context.

clemens.tolboom’s picture

orangecoat-ciallella’s picture

This is comment and a question to hopefully help the next guy who comes along and is testing D8 REST functionality, since the stuff above is pretty technical.

As of drupal-8.0.0-beta6 it seems you can't do a PATCH (or POST according to https://www.drupal.org/node/2098511) using application/json because it will return an

"error": "Entity type parameter must be included in context."

However, using a hal+json and its _links type already works fine to do POST or PATCH.

Is one of the outcomes of this issue, and/or #2291005, that it will soon be possible to do a PATCH or POST with Content-Type: application/json ?

If so, it seems like including
"entity_type": "node"
in the application/json
would be necessary, correct?

clemens.tolboom’s picture

@orangecoat-ciallella you are spot on.

This patch should make Content-Type: application/json work.

Maybe you could update the summary with your text from #29?

orangecoat-ciallella’s picture

Issue summary: View changes
askibinski’s picture

Patch doesn't apply anymore because EntityDerivative.php doesn't seem to exist anymore. I think it has been renamed to EntityDeriver.php somewhere in the past months. Can't find the related issue though.

clemens.tolboom’s picture

clemens.tolboom’s picture

Title: Pass entity_type and bundle into Serializer via context » Pass entity_type into Serializer via context
Issue summary: View changes
Issue tags: +Needs tests

Removed bundle from title and summary

Open issues are

- needs annotation #27
- needs tests #27

clemens.tolboom’s picture

Rerolled.

wwhurley’s picture

I also needed the $context['entity_type'] to be set. I did that with:

$context['entity_type'] = $definition['entity_type'];

It works, but I'm not entirely sure based on this ticket whether that's the correct way to be doing it or whether it should come from somewhere else.

Wim Leers’s picture

entity_type should not be a problem, we could place serialization context parameters on the resource plugin information.

The subtype is a problem since that information is only present in the data, not in the URL, not in the plugin info, not in some HTTP headers.

#2631774: Impossible to update Comment entity with REST (HTTP PATCH): bundle field not allowed to be updated, but EntityNormalizer::denormalize() requires it also ran into this: for Comment entities, the subtype (bundle, called comment_type in their case) is not allowed to be modified (by any user, ever). Yet the denormalizer requires it. It therefore required a change in another layer, so the denormalizer gets it (step 1 in the pipeline), but not the entity that ends up being saved — it is stripped by the EntityResource (step 2 in the pipeline).

Wim Leers’s picture

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.

gabesullice’s picture

Version: 8.1.x-dev » 8.2.x-dev
Issue summary: View changes

Updated for 8.2.x and updating issue summary. Without this, it is impossible would most likely require significant effort in the ECK module to POST ECK Entities, while with this applied, it just works.

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

Status: Needs work » Closed (outdated)

My perspective on this has changed since I wrote #37 more than 14 months ago.

We've been able to use the Drupal 8 core REST API just fine all this time, with various entity types. #2737719: EntityResource: Provide comprehensive test coverage: for every entity type, every format, every method and the many follow-ups for it have proven that this is simply not necessary. Therefore, closing this.