[PP-1] EntityResource: translations support

As #2257293: RESTful Multi-Language Paths says, if you do a REST request to /node/1 on your French domain you expect the French translation, no? Not the Chinese original.

Idealy we would use the Accept-language and Content-language HTTP headers to indicate the language. Not sure what we should do if you want to patch multiple languages at once, but I guess multiple requests are fine for core.

Idealy we would use the Accept-language and Content-language HTTP headers to indicate the language. Not sure what we should do if you want to patch multiple languages at once, but I guess multiple requests are fine for core.

#2331919: Implement StackNegotiation will certainly allow to get the Accept-Language and we already set the Content-language. So basically the language negotiators
have to take into account the language figured out by the negotiation middleware.

The Drupal language negotiation process should take into account the request language. The request language can get negotiated by StackNegotiation prior to HttpKernel even firing. It doesn't replace Drupal's custom negotiation (which takes into account things like user preferences), but supports it.

So the Drupal language negotiation currently does not have the "browser language" (AKA Accept-language based) negotiation turned on by default and even if it would be the default ordering makes others take precedence. Eg. if you access fr.example.com with an accept-language of 'de', it would still be French. Looks like you are saying for REST, all other language conditions should be ignored and only the accept-language should be taken? What if that is not provided? (Technically to implement such a setup, we would need a new locked language type for REST requests and use that to negotiate the language for the REST request).

Er, no? I think we're talking past each other. I'm saying that the HTTP language negotiation is easy to add, and we can leverage it or not (we should) in whatever way makes sense within the Drupal language negotiation process.

@Crell: HTTP language negotiation is already implemented and supported. It is not enabled by default because without redirection built-in it causes SEO problems at least. Where do you envision it would be "easily added" and "leveraged or not"?

#2331919: Implement StackNegotiation makes it trivial to have the HTTP language negotiated and marked on the Request object before HttpKernel is even run. In fact I think the current patch would already do that. We can then remove our own HTTP language negotiation if we have any, or make it just wrap that value. What we do it with it beyond that is not my problem. :-)

But that's really just to explain dawehner's comment in #4, and doesn't necessarily address the original question which is how to represent the data payload.

@Crell: that is *surprising*. #2331919-47: Implement StackNegotiation for an extended answer.

Feeding back info from there, no StackNegotiation will not be useful to negotiate a language yet, the Drupal implementation is much more battle tested.

I believe the child issue mentioned is related.

#2359323: Language results for JSON REST services not correct

Getting back to the original discussion, there's two ways this could be done:

1) Say that for REST, you get back all translations of all fields. And then you PUT all translations of all fields. If you only bother reading some of them in the mean time, good for you.

2) Make REST Accept-Language aware for both GET and PUT. That is,

GET /node/1
Accept: application/hal+json
Accept-Language: de

Would return ONLY the German version of a node (give or take whatever rules we already have for non-translated fields and fields with a missing translation), and to write you'd have to do:

PUT /node/1
Content-Type: application/json
Language: de

{ ... }

From a REST perspective, actually, using out-of-band session information (like a user preference) would actually be incorrect; every request SHOULD contain all the information it needs in the request itself without any other server-side state. So it would be appropriate in this case to rely solely on the Accept-Language/Language headers and nothing else.

Option 2 is, I think, more RESTfully correct but probably also more work. :-(

Assuming you go with option 2, if you ask for zh-hans/node/1 in JSON without specifying a language, what language would the results be? Would there even be results at all?

Option 2 would say that results in either a 404 or a 406 error, I believe.

Spaces and domain access will change some things I guess, if node/1 is only accessible on domain de.example.com and not en.example.com then no common REST endpoint will exist (as with path prefixes), but domain access is used anyway to make sites appear they are different sites, so different entry points for REST are not surprising.

As for option 1 or 2 I can imagine both having its uses. Is there a way we can satisfy both? For example if there was no Accept-language, we work with the whole entity and not some hardwired fallback language?

From a REST perspective, different domains => different resources. The amount of unholy things we do to support browsers is truly mind-boggling. :-(

I *think* the following logic would be REST-acceptable:

if (Accept-Language/Language header) {
use that and only that, ignoring all other configuration
}
else {
send/receive the whole object with all languages
}

We should probably verify that. That said, I am concerned about someone sending an object in just one language but without a Language header. That would probably get interpreted as "delete all languages but this one", which presumably we do not want.

Unless it were somehow a serialization error, because the language keys on fields are missing?

From a REST perspective, different domains => different resources.

Completely agree. We should not go ways to support languages in the domain/path in any way. That's going against restful principles.

if (Accept-Language/Language header) {
use that and only that, ignoring all other configuration
}
else {
send/receive the whole object with all languages
}

The problem I see with this approach is that the structure of the entity fields would have to change to support send/receive all languages in one whole object. Although it might be restful it would complicate things for clients and consumers of the API having to handle different object structures.

So my take on this would be:

If Accept-Language/Language headers are available, send/receive the entity object as is.

If Accept-Language/Language headers are NOT available, send/receive multipart/related (see MIME#Related on Wikipedia) with Transfer-Encoding: chunked.

A simplified GET request would be:

GET /node/1
Accept: multipart/related

Response:

Content-Type: multipart/related; boundary="12345delimiter"
Transfer-Encoding: chunked

--12345delimiter
Content-Type: application/json
Language: en

{
  id: 1,
  title: [{value: "english title"}],
  body: [{value: "english text"}]
}
--12345delimiter
Content-Type: application/json
Language: se

{
  id: 1,
  title: [{value: "svensk titel"}],
  body: [{value: "svensk text"}]
}
--12345delimiter--

A simplified PUT request would be:

PUT /node/1
Accept: application/json
Content-Type: multipart/related; boundary="12345delimiter"

--12345delimiter
Content-Type: application/json
Accept-Language: en

{
  id: 1,
  title: [{value: "english title"}],
  body: [{value: "english text"}]
}
--12345delimiter
Content-Type: application/json
Accept-Language: se

{
  id: 1,
  title: [{value: "svensk titel"}],
  body: [{value: "svensk text"}]
}
--12345delimiter--

It may seem a bit complicated, but it's properly restful and most robust HTTP clients should support this type of responses nicely. Each "chunk" is simply a normal entity object translated in the language specified by the headers.

@dixon_: I think that makes total sense :)

I noticed that neither Symfony HttpFoundation or Guzzle had support for multipart/* requests and responses (except multipart/form-data which is not what we're looking for).

So I quickly created a set of components to deal with that, see:

• https://github.com/dickolsson/http-multipart
• https://packagist.org/packages/dickolsson/http-multipart

This could serve as a demonstration of how we could implement support for multipart requests and responses in our REST implementation.

That's unfortunate if we could not output or test those in core without more libraries :/ Sounds like this would be blocked for adding more libraries if we want to go that route.

Added steps to repro from #2359323: Language results for JSON REST services not correct

I think this is at least major. AFAICT without this issue, Drupal 8 does not support working with translated entities via REST. Even though that's one of Drupal's strengths overall. That makes this a very important missing link.

The steps to reproduce match those for the issue I'm working on now: https://www.drupal.org/node/2664880 . I'm not sure about the other parts of this issue, so maybe 2664880 should be a sub-issue?

Just wanted to comment here that I am not getting the same results as the issue summary.

This is the behaviour I am seeing for:

1. 2 languages English and Spanish
2. 1 node with original language English and is translated into Spanish

If create REST export for the front page view
curl -H 'Accept: application/json' --request GET http://d8/es/node
this does return the spanish results.

for the single node GET also
curl -H 'Accept: application/json' --request GET http://d8/es/node/1
Does return the Spanish results.
Where as
curl -H 'Accept: application/json' --request GET http://d8/node/1
Returns the English results

Furthermore if I login, send cookies along with the request and set the "account's preferred language" then the corresponding language of the node and view is returned.

From the comments in this issue it sounds like this is not the desired behavior.

But if we decide to fix it and implement another solution do we consider that sites might be currently depending on the current behavior. Or since the current behavior is not documented anywhere is it ok to break/fix it.

Also hal_json gives me a different behavior. For every node response in the object it gives both english and spanish field values:

"body": [
    {
      "value": "<p>english</p>\r\n\r\n<p>&nbsp;</p>\r\n",
      "format": "basic_html",
      "summary": "",
      "lang": "en"
    },
    {
      "value": "<p>spanish body</p>\r\n",
      "format": "basic_html",
      "summary": "",
      "lang": "es"
    }
  ],

I have attached the full response. So in this case
http://www.octo2.dev/d8_rest_lang/node/1?_format=hal_json and http://www.octo2.dev/d8_rest_lang/es/node/1?_format=hal_json

give the same response

Forgot to attach file with full hal_json response. Unlike json the response is the same regardless of language provide in path or user account.

Yeah I don't know what the correct/desired solution is, but it is comforting that at least the translations are somehow accessible now. The HAL response looks very confusing though :/

@Gábor Hojtsy yes I found it weird to but maybe this expected behavior.

Looking at \Drupal\hal\Normalizer\FieldNormalizer it seems merging the field values from all languages is intentional

foreach ($entity->getTranslationLanguages() as $language) {
        $context['langcode'] = $language->getId();
        $translation = $entity->getTranslation($language->getId());
        $translated_field = $translation->get($field_name);
        $normalized_field_items = array_merge($normalized_field_items, $this->normalizeFieldItems($translated_field, $format, $context));
      }

Right, well, in a rest world, the resource, for example /node/1 should have ideally one representation. Yes, this one representation might be quite verbose, but at least its one. In this logic the verbose HAL_JSON output, is not the worst.

All right, should we consider the HAL JSON implementation as reference? In that case, sounds like we do need https://packagist.org/packages/dickolsson/http-multipart or an equivalent so return multiple variants at the same time and we need to expect clients to deal with that fine?

for the single node GET also

curl -H 'Accept: application/json' --request GET http://d8/es/node/1

Does return the Spanish results.
Where as

curl -H 'Accept: application/json' --request GET http://d8/node/1

Returns the English results

Furthermore if I login, send cookies along with the request and set the "account's preferred language" then the corresponding language of the node and view is returned.

From the comments in this issue it sounds like this is not the desired behavior.

So do we want REST to require authentication to be able to specify language in some other way than the regular language negotiation? And even then, to me it looks like it works for you because you configured your language negotiation in a specific way, no? I think more details on your setup would be nice to see what you have / what you observed.

In that case, sounds like we do need https://packagist.org/packages/dickolsson/http-multipart or an equivalent so return multiple variants at the same time and we need to expect clients to deal with that fine?

In case we want that, we certainly would want it to be an opt in. Supporting for example a language prefix support IMHO is kinda what people expect.

@dawehner: but what's a thing though? I mean if you update an entity and it has untranslated fields, it will have side effects on other translations. Is that what is expected in REST to have such side effects or are you supposed to deal with the whole entity as one? Would dealing with the whole entity as opposed to a translation be an opt-in thing?

Considering the number of (broken) edge cases I'm finding while working on #2737719: EntityResource: Provide comprehensive test coverage: for every entity type, every format, every method, I think this issue needs to be postponed on that one. Because this would introduce the wiggle room for even more edge cases. When we add this, we need firm, reliable, comprehensive test coverage. We can't achieve that without #2737719: EntityResource: Provide comprehensive test coverage: for every entity type, every format, every method.

#2737719: EntityResource: Provide comprehensive test coverage: for every entity type, every format, every method landed.

Note that JSON API committed a solution for this: #2794431: [META] Formalize translations support. However, it violates the concerns that dixon_ raised at #22. It violates this:

We should not go ways to support languages in the domain/path in any way. That's going against restful principles.

But for JSON API, this may be acceptable — see #2794431-35: [META] Formalize translations support.

We should look into how we can move forward here. It's really a huge gap that our REST API doesn't support reading and modifying translated entities. Thanks to #2737719: EntityResource: Provide comprehensive test coverage: for every entity type, every format, every method, no matter what direction we choose, we will be able to have solid test coverage.

Just had a discussion about this with @damiankloip & @tedbow. (On January 30, but only got around to finish this now.)

There are two key observations that were made:

1. Doing the multipart approach @dixon_ suggested in #22 may very well be the most correct approach (the ideal solution), but doing so is unquestionably a BC break.
2. Ted showed in #32 that this already works today. In a way that is natural/intuitive/guessable to a Drupal developer: via a langcode path prefix. Changing that now would also effectively be a BC break, since it's extremely likely that multilingual sites are already suing this.

So doing the ideal isolation would result in a double BC break, hence that approach must be postponed to D9.

The pragmatic solution is therefore to just expand the test coverage in EntityResourceTestBase to test all HTTP methods against all entity types in multiple languages.

The pragmatic solution is therefore to just expand the test coverage in EntityResourceTestBase to test all HTTP methods against all entity types in multiple languages.

+1

Note: This relies on our language negotiation service, so its actually configurable.

Here is a question: could we have a "translation" link relation type which exposes all the other translations?

Right, if we want to be backwards compatible and consider the language based URLs as a valid way to access certain representations of an entity, then unifying under single entry points is not an option anymore.

Note: This relies on our language negotiation service, so its actually configurable.

Indeed. So it is possible that a site is using fr.example.com and cn.example.com, and hence allows fr.example.com/node/42 to be read in Chinese at cn.example.com/node/42. Is that really a problem? Is that still a problem if the necessary link relationships are present?

Here is a question: could we have a "translation" link relation type which exposes all the other translations?

+1. I looked through https://www.iana.org/assignments/link-relations/link-relations.xhtml, but there's no translation metadata there.

#48: thanks for confirming!

custom format

Sure, you can do that. But another format to maintain? That means even more normalizers to maintain, even though the existing ones are already flawed (see the "serialization gap" issues at the top of #2794263: REST: top priorities for Drupal 8.3.x. Who's going to do that? I'm certainly not going to.

This reminds me of something though: @damiankloip pointed out during that same meeting that I reported back on in #46 that the HAL+JSON format actually already includes all translations! However, it does this at the field level. Every field has a 'lang' => 'en' right now, and our test coverage asserts that.

The code also shows this very clearly — quoting \Drupal\hal\Normalizer\FieldNormalizer::normalize():

    // If this field is not translatable, it can simply be normalized without
    // separating it into different translations.
    if (!$field_definition->isTranslatable()) {
      $normalized_field_items = $this->normalizeFieldItems($field, $format, $context);
    }
    // Otherwise, the languages have to be extracted from the entity and passed
    // in to the field item normalizer in the context. The langcode is appended
    // to the field item values.
    else {
      foreach ($entity->getTranslationLanguages() as $language) {
        $context['langcode'] = $language->getId();
        $translation = $entity->getTranslation($language->getId());
        $translated_field = $translation->get($field_name);
        $normalized_field_items = array_merge($normalized_field_items, $this->normalizeFieldItems($translated_field, $format, $context));
      }
    }

My point is that I'm 99% sure that it is *not* just adding test coverage :)

Point taken! But at least for GETting translations, it is 99% adding test coverage, since it is already working. Will we discover edge cases that we need to worry about? I have no doubt about that. But certainly the first step on this issue should be adding test coverage for reading translations, even if only to ensure that we don't regress there while we work on supporting PATCH/POST/DELETE.

And as I've basically just shown, for HAL+JSON, it's already going to be more complicated than "just adding test coverage".

IS updated.

My proposal:

1. Add test coverage for serialization-normalizer-based formats (currently only json): test that /de/node/1?_format=json returns the German translation, and also allows modifying that translation
2. Add test coverage for hal-normalizer-based formats (currently only hal_json): test that /node/1?_format=hal_json returns all translations, and /de/node/1?_format=hal_json returns only the German translation
3. Language negotiation mechanisms other than the path-based one: TBD

#55

2. Right!
3. Well, people were very concerned that we'd be supporting subdomain-based language negotiation. Whether we want to exclude certain language negotiation mechanisms like subdomains, is something that is TBD. Although perhaps that ship has sailed too then.

#56: Regarding content_translation link relation types: answer below. testGet() is passing: YAY. testPost() and testPatch(): we should be able to modify a subset of translations or all translations? So we should test both. Or should we not be able to modify all translations? Should HAL just allow you to read all translations?

1. +++ b/core/modules/hal/tests/src/Functional/EntityResource/Node/NodeHalJsonCookieTranslationsTest.php
   @@ -0,0 +1,122 @@
   +  protected static $patchProtectedFieldNames = [
   +    'created',
   +    'changed',
   +    'promote',
   +    'sticky',
   +    'revision_timestamp',
   +    'revision_uid',
   +  ];
   

   You can omit this, since it matches what is inherited from NodeHalJsonAnonTest::$patchProtectedFieldNames()

2. +++ b/core/modules/hal/tests/src/Functional/EntityResource/Node/NodeHalJsonCookieTranslationsTest.php
   @@ -0,0 +1,122 @@
   +  protected function getExpectedCacheContexts() {
   +    return [
   +      0 => 'languages:language_content',
   +      1 => 'languages:language_interface',
   +      2 => 'url.site',
   +      3 => 'user.permissions',
   +    ];
   +  }
   

   Let's change this to:

   return Cache::mergeContexts(
     parent::getExpectedCacheContexts(),
     ['languages:language_content', 'languages:language_interface']
   ); 
   

3. +++ b/core/modules/rest/tests/src/Functional/EntityResource/EntityResourceTestBase.php
   @@ -435,12 +435,16 @@ public function testGet() {
   +      // @todo What to do about content_translation link relationships?
   +      if (strpos($rel, 'drupal:content-translation') !== FALSE) {
   +        return FALSE;
   +      }
   

   Add them to core.link_relation_types.yml. Or actually, add them to content_translation.link_relation_types.yml :)

#57:
This is failing with the following error message: "{"message":"Access denied on creating field \u0027langcode\u0027."}". AFAICT this is caused by language_entity_field_access() only allowing access if specific conditions are met.

@Wim: even when you update a translation, there is good chance there are non-translatable fields, which are the same for all translations, so you have a bit of a side effect for your post/patch request.

Right. All the more reason to have good test coverage showing that. Or am I not understanding what you mean by that?

somehow connecting to a wrong database or something

WTF?! :O

Since we now have nice 403 responses, wouldn't it be nice to have nicer 500 responses too? :)

Yes, but … the point is that it's such a severe error that all we can do is send a 500 response. So there's a pretty strong contradiction in there. Most of the time, we can't/don't want to expose stack traces!
Add this before the currently failing assertion:

var_dump((string)$response->getBody());

I think that's related to \Drupal\Core\Test\HttpClientMiddleware\TestHttpClientMiddleware. It very easily and very frequently gets in the way.

As far as I see, we could simply not save at all on PATCH and the existing test would happily pass?

You're right. The test coverage does assume that status codes don't lie. So far, that's been good. But with the need to support translations, we obviously need to make the tests more strict.

You can't assume anything in a test, I thought that's the point of writing tests? :) I think there should be assertPatchedEntity()/assertPostedEntity() and similar methods that subclasses can override and verify that things are working as expected. That's not specific to translations at all.

Eh… I'm sorry that not every possible edge case was tested in #2737719: EntityResource: Provide comprehensive test coverage: for every entity type, every format, every method, which was a mere 190 KB patch that took months to get to RTBC and uncovered >20 unreported bugs. I spent dozens and dozens of hours on that test coverage patch, which should have happened years ago, i.e. when REST landed.

To be clear: I'm not mad at you, but that statement was very unfair. Things are the way they are. We make leaps and baby steps in the direction of more stability.

We already have code that does this in testGet():

    // Comparing the exact serialization is pointless, because the order of
    // fields does not matter (at least not yet). That's why we only compare the
    // normalized entity with the decoded response: it's comparing PHP arrays
    // instead of strings.
    $this->assertEquals($this->getExpectedNormalizedEntity(), $this->serializer->decode((string) $response->getBody(), static::$format));

So you'll want to add assertions like that after successful POST and PATCH requests. And you'll want to add assertions that load the entity object and also assert the field values from there.

We clearly have a different process, because I started with a successful test with explicit success assertions, while you started from nothing and then step-by step enable things and test that things do not work the way we expect them to not work.

I also generally start with successful tests, but in this case, I was turning thin & spotty test coverage into something more reliable. The process I have there was mostly: think about all the possible mistakes a developer writing a client talking to the D8 REST API might make, and ensure that we have error responses that guide them towards successful requests. Because that was the #1 complaint about REST API: endless failing requests, with no way to figure out how to get them to be successful, other than firing up Xdebug and stepping through all the code.

Anyway, enough about that. Let's focus on this issue again.

+++ b/core/modules/rest/src/Plugin/rest/resource/EntityResource.php
@@ -180,27 +182,22 @@ public function post(EntityInterface $entity = NULL) {
-    try {
...
-    }
-    catch (EntityStorageException $e) {
-      throw new HttpException(500, 'Internal Server Error', $e);
-    }

Because this try/catch is removed, one assertion in \Drupal\Tests\rest\Functional\EntityResource\Comment\CommentResourceTestBase::testPostDxWithoutCriticalBaseFields() must be updated.

That fixes most failures.

The actual failing test is failing with this error:

"{"message":"Unprocessable Entity: validation failed.\nlangcode: Language: this field cannot hold more than 1 values.\nstatus: Publishing status: this field cannot hold more than 1 values.\nuid: Authored by: this field cannot hold more than 1 values.\ncreated: Authored on: this field cannot hold more than 1 values.\npromote: Promoted to front page: this field cannot hold more than 1 values.\nsticky: Sticky at top of lists: this field cannot hold more than 1 values.\n"}"

I think its wrong from a BC point of view to limit our support to content entities, but meh, we would need to rewrite that stuff for config entities anyway, I guess.

I'm trying to look into the remaining test failure.

One problem seems to be that \Drupal\serialization\Normalizer\FieldNormalizer::denormalize appends additional items, so in that case somehow we end up with two values per language.

I'll continue exploring.

Rebased. That was a tough rebase.

We discussed this issue quickly on the latest REST major triage call and we all agreed that this issue is major. Major for its test coverage, but then also major for the potential feature of serving all translations at once.

Comparing the failures of the two last patches it seems to be the case that we gained some new failures in the area of comment resource tests.

Patch no longer applies after #2824851: EntityResource::patch() makes an incorrect assumption about entity keys, hence results in incorrect behavior was committed to 8.4.x and 8.5.x.

Re-rolling

I assume @tedbow gave up on this.

Re-rolled.

Fetch save error looks like a CI problem...

There was a server error as I submitted the patch so I have started a retest.

Patch #68 added a weird syntax error to the phpdoc in EntityResourceValidationTrait::validate() that was never caught.

I can confirm the six failures in ::testPostDxWithoutCriticalBaseFields. The HTTP-500 error has a plain-text response instead of the expected JSON:

The website encountered an unexpected error. Please try again later.</br></br><em class="placeholder">Drupal\Core\Entity\EntityStorageException</em>: The &quot;&quot; entity type does not exist. in <em class="placeholder">Drupal\Core\Entity\Sql\SqlContentEntityStorage-&gt;save()</em> (line <em class="placeholder">805</em> of <em class="placeholder">core/lib/Drupal/Core/Entity/Sql/SqlContentEntityStorage.php</em>). <pre class="backtrace">Drupal\Core\Entity\EntityTypeManager-&gt;getHandler(NULL, &#039;storage&#039;) (Line: 169)
Drupal\Core\Entity\EntityTypeManager-&gt;getStorage(NULL) (Line: 79)
Drupal\Core\Entity\EntityManager-&gt;getStorage(NULL) (Line: 258)
Drupal\comment\CommentStatistics-&gt;update(Object) (Line: 168)
Drupal\comment\Entity\Comment-&gt;postSave(Object, ) (Line: 469)
Drupal\Core\Entity\EntityStorageBase-&gt;doPostSave(Object, ) (Line: 326)
Drupal\Core\Entity\ContentEntityStorageBase-&gt;doPostSave(Object, ) (Line: 395)
Drupal\Core\Entity\EntityStorageBase-&gt;save(Object) (Line: 796)
Drupal\Core\Entity\Sql\SqlContentEntityStorage-&gt;save(Object) (Line: 377)
Drupal\Core\Entity\Entity-&gt;save() (Line: 184)
Drupal\rest\Plugin\rest\resource\EntityResource-&gt;post(Object, Object)
call_user_func_array(Array, Array) (Line: 132)
Drupal\rest\RequestHandler-&gt;handle(Object, Object)
call_user_func_array(Array, Array) (Line: 123)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber-&gt;Drupal\Core\EventSubscriber\{closure}() (Line: 574)
Drupal\Core\Render\Renderer-&gt;executeInRenderContext(Object, Object) (Line: 124)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber-&gt;wrapControllerExecutionInRenderContext(Array, Array) (Line: 97)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber-&gt;Drupal\Core\EventSubscriber\{closure}()
call_user_func_array(Object, Array) (Line: 153)
Symfony\Component\HttpKernel\HttpKernel-&gt;handleRaw(Object, 1) (Line: 68)
Symfony\Component\HttpKernel\HttpKernel-&gt;handle(Object, 1, 1) (Line: 57)
Drupal\Core\StackMiddleware\Session-&gt;handle(Object, 1, 1) (Line: 47)
Drupal\Core\StackMiddleware\KernelPreHandle-&gt;handle(Object, 1, 1) (Line: 99)
Drupal\page_cache\StackMiddleware\PageCache-&gt;pass(Object, 1, 1) (Line: 78)
Drupal\page_cache\StackMiddleware\PageCache-&gt;handle(Object, 1, 1) (Line: 47)
Drupal\Core\StackMiddleware\ReverseProxyMiddleware-&gt;handle(Object, 1, 1) (Line: 50)
Drupal\Core\StackMiddleware\NegotiationMiddleware-&gt;handle(Object, 1, 1) (Line: 23)
Stack\StackedHttpKernel-&gt;handle(Object, 1, 1) (Line: 656)
Drupal\Core\DrupalKernel-&gt;handle(Object) (Line: 19)
</pre>

Actually I'm not sure where the JSON error is even supposed to be generated. The chain Drupal\rest\RequestHandler -> EntityResource::post -> Entity::save has no try/catch, and the string 'A fatal error occurred' (which the test expects in the JSON) doesn't exist anywhere in the project or in the vendor folder. Maybe the patch was incomplete?

A git log -S reveals #2825347: 'Accept' header still is used for determining response format of error responses, ?_format= has no effect AND serialization module's exception subscriber does not support status code 500 error responses / 9bd08a00 as a possible source of the problem. That commit removed DefaultExceptionSubscriber::onJson and also switched the relevant assertion to plain-text.

diff --git a/core/modules/rest/tests/src/Functional/EntityResource/Comment/CommentResourceTestBase.php b/core/modules/rest/tests/src/Functional/EntityResource/Comment/CommentRe
sourceTestBase.php
index 63991311a9..b04da54373 100644
--- a/core/modules/rest/tests/src/Functional/EntityResource/Comment/CommentResourceTestBase.php
+++ b/core/modules/rest/tests/src/Functional/EntityResource/Comment/CommentResourceTestBase.php
@@ -280,8 +280,8 @@ public function testPostDxWithoutCriticalBaseFields() {
     $response = $this->request('POST', $url, $request_options);
     // @todo Uncomment, remove next 3 lines in https://www.drupal.org/node/2820364.
     $this->assertSame(500, $response->getStatusCode());
-    $this->assertSame(['application/json'], $response->getHeader('Content-Type'));
-    $this->assertSame('{"message":"A fatal error occurred: Internal Server Error"}', (string) $response->getBody());
+    $this->assertSame(['text/plain; charset=UTF-8'], $response->getHeader('Content-Type'));
+    $this->assertSame('Internal Server Error', (string) $response->getBody());

This patch switches the same assertion back to JSON, but does not re-add the DefaultExceptionSubscriber::onJson code.

Oh wait - looking at it more closely, this just looks like a rebase error introduced in #80. There would have been a conflict in this assertion due to #2825347: 'Accept' header still is used for determining response format of error responses, ?_format= has no effect AND serialization module's exception subscriber does not support status code 500 error responses because this patch changed the same lines:

diff --git a/core/modules/rest/tests/src/Functional/EntityResource/Comment/CommentResourceTestBase.php b/core/modules/rest/tests/src/Functional/EntityResource/Comment/CommentResourceTestBase.php
index 7e38b33..e8e86f4 100644
--- a/core/modules/rest/tests/src/Functional/EntityResource/Comment/CommentResourceTestBase.php
+++ b/core/modules/rest/tests/src/Functional/EntityResource/Comment/CommentResourceTestBase.php
@@ -281,7 +281,7 @@ public function testPostDxWithoutCriticalBaseFields() {
     // @todo Uncomment, remove next 3 lines in https://www.drupal.org/node/2820364.
     $this->assertSame(500, $response->getStatusCode());
     $this->assertSame(['application/json'], $response->getHeader('Content-Type'));
-    $this->assertSame('{"message":"A fatal error occurred: Internal Server Error"}', (string) $response->getBody());
+    $this->assertSame('{"message":"A fatal error occurred: The \u0022\u0022 entity type does not exist."}', (string) $response->getBody());
     //$this->assertResourceErrorResponse(422, "Unprocessable Entity: validation failed.\nentity_type: This value should not be null.\n", $response);
 
     // DX: 422 when missing 'entity_id' field.

And this should have been resolved by combining HEAD's json->plaintext change with this patch's exception message change.

@cburschka Did you intend to remove core/modules/hal/tests/src/Functional/EntityResource/Node/NodeHalJsonCookieTranslationsTest.php from #86 when you created #91? Can you also remember to include an interdiff.

(This is we should start using pull requests. :P)

Should fix at least 6 of the failures.

PS: Yeah, just noticed the missing newfile. Common problem with rolling from git, sadly. ;)

Two Hal failures are caused by:

1. #2869415: EntityResourceTestBase::getUrl() overrides BrowserTestBase::getUrl(), yet offers different functionality changed getPostUrl to getEntityResourcePostUrl
2. #2768651: Let TimestampItem (de)normalize to/from RFC3339 timestamps, not UNIX timestamps, for better DX reordered patchProtectedFieldNames so revision fields are at the top (before changed)

Fixed those two, but there are still more.

Fix a couple more failures.

The remaining NodeHalJsonCookieTranslationsTest::testPatch error looks trickier - for some reason, the server automatically prepends an extra value to the 'changed' field.

What the test tries to send:

array(1) {
  [0]=>
  array(1) {
    ["value"]=>
    int(1503504013)
  }
}

What the server has as the original field:

array(1) {
  [0]=>
  array(1) {
    ["value"]=>
    string(10) "1503504013"
  }
}

What the server tries to update the field to:

array(2) {
  [0]=>
  array(1) {
    ["value"]=>
    int(1503504024)
  }
  [1]=>
  array(1) {
    ["value"]=>
    int(1503504013)
  }
}

Edit: More investigation.

Three failures:

1. The entity somehow gets two changed timestamps instead of one.
2. testPost fails due to an access violation creating the "path" field.
3. testGet has the following mismatch between expected->actual data:

@@ -106,7 +106,7 @@ Array &0 (
         )
         1 => Array &38 (
             'lang' => 'de'
-            'value' => '0'
+            'value' => false
         )
     )
     'langcode' => Array &39 (
@@ -134,8 +134,8 @@ Array &0 (
         1 => Array &46 (
             'alias' => '/llama'
             'lang' => 'de'
-            'langcode' => 'en'
-            'pid' => 1
+            'langcode' => 'de'
+            'pid' => 2
         )
     )
     'promote' => Array &47 (

More data for #101: The double-valued 'changed' field is only on the translation of the unserialized entity. The entity itself is normal.

Drilling deeper into the denormalizer, it looks as though FieldableEntityNormalizerTrait::denormalizeFieldData() tries to empty out the automatic timestamp with ->setValue(), and succeeds in doing so on the base entity, but not on the translation. Denormalizing the field data then merely appends the serialized value to the automatic one.

I've reproduced the denormalization bug on a clean installation of 8.3.7 and 8.5.x. Denormalizing translated entities is already nominally supported, so this should be fixed in a separate issue: #2904423: Translated field denormalization creates duplicate values

@cburschka thanks for debugging and for creating #2904423: Translated field denormalization creates duplicate values

I am going to check out that new issue and first work on test that proves the bug you have found.

Do you think we should postpone this issue on that 1? I am to postpone this issue for now because from you description it sounds like we have to solve that bug before we can fix this issue. Let me know if that is wrong

@idebr (#84): thanks for noticing :)

@Jo Fitzgerald + @chburschka: thanks for your many rerolls!

@cburschka (#105+#106): superb research! Thanks so much for creating that issue!

P.S.: @cburschka Wow, you are user 28680! It's been a while since I ran into such an "ancient" d.o user :) 12 years and counting! Glad to still see you among us! :)
P.P.S.: This is we should start using pull requests. :P — with pull requests you wouldn't have been able to jump on this issue and started making changes after others already did prior versions!

@Wim Leers: Heh, thanks!

I think this (specifically the chain of #2904423: Translated field denormalization creates duplicate values / #2824851: EntityResource::patch() makes an incorrect assumption about entity keys, hence results in incorrect behavior / #2906600: FieldItemList::equals() doesn't work correctly for computed fields with custom storage) might be a cool thing to tackle during the sprint on Friday here in Vienna. (Though I'm not sure how much progress can be made on a beast like this in one day. ;) )

Re-rolled the patch from #80 for Drupal 8.4.5

@Fonski it is of no use to re-roll against an 'old' version. We need #100 to fix this in the near future.

I've just tried wrapping my head around this issue and #2904423: Translated field denormalization creates duplicate values (in which I just also commented), but I think both issues are now stale and I'm not really sure which issue is waiting on what. This issue seems to be postponed in favor of fixing #2904423: Translated field denormalization creates duplicate values first? That issue seems to be told that it should have minimal impact and then it went stale? Any way how we can revive these issues?

I'd be glad to try and help, making my own contribution to these issues (if I'm capable), but I think I need some kind of a summary of the current state?

Edit: Reading the summary of this issue again clears some things up I think, a lot of problems arise because of BC. If I understand correctly, most of this should already work out-of-the-box? Working with language path-prefixes you should be able to work with the correct language? This does not seem to be the case for me a.t.m. but might that be because I'm working with content translations? I'm not sure...

#2135829: [PP-1] EntityResource: translations support is the general translation support issue for core REST, for all formats: json, xml and hal_json.

However, while working on that, we discovered a serious bug in the HAL normalizer that blocks progress in the aforementioned issue: that's what #2904423: Translated field denormalization creates duplicate values is for.

Since #2904423: Translated field denormalization creates duplicate values is now trying to land in "Needs review", I tried cross testing the latest patch in #111 but it does not apply cleanly anymore to core 8.5.5.

Edit: #111 seems to be rolled against an older version of core. The more appropriate patch to use is in #100 I think, but that does also not apply cleanly to 8.5.5 (did not yet try on any -dev branch, but I think because of the age of this issue that will not work too).

100 is 8.3, the best starting point would be using the 8.4 patch in 111 and rolling it again for 8.7.

Edit: Missed that 111 actually is a re-roll of 80, sorry. Yeah, 100 is the last version of the patch.

Edit: Uugh, the painful part of this is that the code at that time contained a change from #2824851: EntityResource::patch() makes an incorrect assumption about entity keys, hence results in incorrect behavior which was later reverted and remade.

Re-rolled the patch from #100 against 8.7.x.

I had a few issues with my local build (that I have now resolved), this patch should be a lot better.

I've been trying to cross test this with #2904423: Translated field denormalization creates duplicate values, but this is my first time to use PHPUnit with Drupal and get fails for literally every test (even ones that should pass). I'm trying to get some help on Slack in #testing, but help seems to be limited. So I'm not really able to test a.t.m. :(

Yeah, setting up a local test environment is tricky, especially for the web tests. I might be able to help on Slack (@cburschka), but after years I'm still mostly guessing. :D

I've not yet been able to run tests. I have been able to cross test this with the patch from #2904423: Translated field denormalization creates duplicate values. The thing I'm trying to do is send a pretty simple multilanguage JSON (not HAL) payload:

array(3) {
  ["title"]=>
  array(2) {
    [0]=>
    array(2) {
      ["value"]=>
      string(12) "Test project"
      ["lang"]=>
      string(2) "en"
    }
    [1]=>
    array(2) {
      ["value"]=>
      string(15) "NL Test project"
      ["lang"]=>
      string(2) "nl"
    }
  }
  ["body"]=>
  array(2) {
    [0]=>
    array(5) {
      ["value"]=>
      string(878) "<p>Fusce dapibus, tellus ac cursus commodo.</p>
"
      ["format"]=>
      string(9) "full_html"
      ["processed"]=>
      string(877) "<p>Fusce dapibus, tellus ac cursus commodo.</p>
"
      ["summary"]=>
      string(0) ""
      ["lang"]=>
      string(2) "en"
    }
    [1]=>
    array(5) {
      ["value"]=>
      string(881) "<p>NL Fusce dapibus, tellus ac cursus commodo.</p>
"
      ["format"]=>
      string(9) "full_html"
      ["processed"]=>
      string(880) "<p>NL Fusce dapibus, tellus ac cursus commodo, tortor mauris condimentum nibh, ut fermentum massa justo sit amet risus. Aenean lacinia bibendum nulla sed consectetur.</p>
"
      ["summary"]=>
      string(0) ""
      ["lang"]=>
      string(2) "nl"
    }
  }
  ["type"]=>
  string(7) "project"
}

With the request headers:

User-Agent: GuzzleHttp/6.3.3 curl/7.38.0 PHP/7.1.15
Host: my-rest-host.ext
Content-Type: application/json
X-CSRF-Token: KMoHxMHkyiPIUDkQ_rgJjjTNQRIxwkXVyfyQ-ZsIkY8
Cookie: SESSbf4d42753f145e5cea30e1656e4d7836=bBMmopuAzxsfHWKc9OeXkLJDzTNh7_0gM8Y_x1VO7JE
Content-Length: 3788

Which results in:

Symfony\Component\HttpKernel\Exception\UnprocessableEntityHttpException: Unprocessable Entity: validation failed. title: Projectnaam: this field cannot hold more than 1 values. body: Body: this field cannot hold more than 1 values. in Drupal\rest\Plugin\rest\resource\EntityResource->validate() (regel 42 van /var/www/vhosts/my-rest-host.ext/web/core/modules/rest/src/Plugin/rest/resource/EntityResourceValidationTrait.php).

I also attempted to use hal+json, and serializing the payload fields with the serializer service, but that gave me all kinds of other errors. I'm wondering if the patch has not fixed my specific issue or maybe I'm doing something wrong... It would be great if I could speak with someone about these problems, especially since documentation in this regard is very limited.

The thing I'm trying to do is send a pretty simple multilanguage JSON (not HAL) payload:

  ["title"]=>
  array(2) {
    [0]=>
    array(2) {
      ["value"]=>
      string(12) "Test project"
      ["lang"]=>
      string(2) "en"
    }
    [1]=>
    array(2) {
      ["value"]=>
      string(15) "NL Test project"
      ["lang"]=>
      string(2) "nl"
    }
  }
…

Well… the json format does not support transmitting/relaying multiple translations in a single representation. So it's understandable that that does not work.

In fact, this very aspect ("can one representation of an entity carry multiple translations of this entity?") is one of the fundamental questions we need to answer/decisions we need to make in this issue!

Alright, I can understand the fact that JSON won't be able to transmit multiple translations in one representation. Though, what I'm encountering is that serializers (at least for HAL+JSON and JSON) seem to be incompatible with how a entity translation is being serialized and then unserialized by the same serializer? This results in errors like the one I mentioned above (multiple values for title). So, is that something that we need to fix first in the serializers? Or...?

I'd be happy if the REST API was able to just send over the entity translations in multiple representations with pathprefixes (eg. /nl /de /en), but how does that work for the serializers? At this point, my serializer seems to try and add the different values for languages in one field which seems to be incompatible with how it needs to be unserialized? I dunno... Just trying to get my head around this.

seem to be incompatible with how a entity translation is being serialized and then unserialized by the same serializer?

That's what's being fixed in this issue and in #2904423: Translated field denormalization creates duplicate values.

At this point, my serializer seems to try and add the different values for languages in one field which seems to be incompatible with how it needs to be unserialized?

Yes, the hal normalizer is a bit strange like that, and deciding how to move forward from that current state and whether it makes sense to support transmitting multiple translation in a single PATCH or POST request's body is exactly what this issue is about.

Hi everyone!

I'm stuck in a project where I have to sync an internal CMS with our Drupal Sites.

The main problem is syncing via REST the translations of the vocabulary terms (also on generic entity content, but that will be in the near future).

I've read carefully the approach you're suggesting (be able to get, post, patch or delete a translation through the core REST endpoint), and I'm not sure it can fit my purposes.

What I suggest is creating a new endpoint exclusively for managing translations.

I've made a custom module that exposes a Resource to the REST web services that allows to get all translations available for a given Entity/Node/Taxonomy (via GET), and also post/patch new or current translations for existing entities.

My goal is expose and endpoint like /translations/{entity_type}/{entity_id} that:

1. Get an array of all translations available
2. Post/Patch an array of all translations to be created/modified
3. Post/Patch an array of translations for a set of entities all at once (to bulk-translate entities or taxonomies with diferent id's)
4. Delete one or more translation

I'm relatively new to Drupal so I don't know if this is the best place to post this suggestion, but I'd like to know if someone is interested in my approach or if it would be better to wait till this Issue fits somewhere in the Core.

Thank you all for your contributions!

Gerard

PS: I'm also developing a .NET project to connect to Drupal via Basic or OAuth2 authentication, so if anyone is interested in participating...

For my use case I had to get this working so I re-rolled the patch from #120 against 8.7.x. I have modified it to inlcuded the field items cardinality count condition for the duplicate values based on patches from #2904423: Translated field denormalization creates duplicate values and followed it up there.

Patch missing class, re-added.

Patch should apply now

Anyone shed some light as to the duplicate entry issue?

1) Drupal\Tests\datetime\Functional\EntityResource\EntityTest\EntityTestDateonlyTest::testPost
Failed asserting that 'The website encountered an unexpected error. Please try again later.Drupal\Core\Entity\EntityStorageException: SQLSTATE[23000]: Integrity constraint violation: 1062 Duplicate entry '9dd9a30f-e651-4ab2-a673-1bb68ec70b06' for key 'entity_test_field__uuid__value': INSERT INTO {entity_test} (type, uuid, langcode, name, created, user_id, rest_test_validation) VALUES (:db_insert_placeholder_0, :db_insert_placeholder_1, :db_insert_placeholder_2, :db_insert_placeholder_3, :db_insert_placeholder_4, :db_insert_placeholder_5, :db_insert_placeholder_6); Array\n
(\n
[:db_insert_placeholder_0] => entity_test\n
[:db_insert_placeholder_1] => 9dd9a30f-e651-4ab2-a673-1bb68ec70b06\n
[:db_insert_placeholder_2] => en\n
[:db_insert_placeholder_3] => yMwuwTMM\n
[:db_insert_placeholder_4] => 1547039385\n
[:db_insert_placeholder_5] => 1\n
[:db_insert_placeholder_6] => \n
)\n

I tried re-producing the test failure for EntityTestDateonlyTest::testPost but test pass based on my local enviornment.

Whilst this patch may not solve the underlying issue(s), which I still don't fully have a grasp on as it seems to stem into several issues. I wanted to post a patch which could help anyone looking to get an intemediate solution for POST/PATCHing entites. I have tested this patch with the following payload:

POST

{
  "_links": {
    "type": {
      "href": "http://drupal.docker.localhost/rest/type/node/page"
    }
  },
  "type": {
      "target_id": "page"
    },    
  "title": [
	{
        "value": "English title test",
        "lang": "en"
    },
    {
        "value": "French title test",
        "lang": "fr"
    }
  ],
  "body": {
      "value": "some body content aaa bbb ccc"
  },
  "langcode": {
	  "value": "en"
  },
  "status": [
	{
      "value": true,
      "lang": "en"
	},
	{
      "value": false,
      "lang": "fr"
	}
  ]
}

PATCH

{
  "_links": {
    "type": {
      "href": "http://drupal.docker.localhost/rest/type/node/page"
    }
  },
  "type": {
      "target_id": "page"
    },    
  "title": [
	{
        "value": "ENGLISH TITLE TEST",
        "lang": "en"
    },
    {
        "value": "FRENCH TITLE TEST",
        "lang": "fr"
    }
  ],
  "status": [
	{
      "value": false,
      "lang": "en"
	},
	{
      "value": true,
      "lang": "fr"
	}
  ]
}

For my case I will re-create the patch for 8.6x.

Fixed offset issue in patch

Should apply now.

Hi there,
as @Wim Leers' advice, I share here my comment #72 on #2794431: [META] Formalize translations support. This is a rough suggestion I thought to fix node translation in our multilingual website project.

I hope it will help.