EntityResourceTestBase: assert that PATCHed and POSTed entities contain the intended values

Added to #2852860 in #2852860-36: REST: top priorities for Drupal 8.4.x.

Every single hal+json format test is failing. Probably due to _links.

Also, the others that fail are:

1. Comment
2. EntityTestLabel
3. EntityTest
4. Shortcut
5. Term
6. User

Anybody interested in pushing this further?

Interesting that the number of failures is different on 8.3.x vs 8.4.x!

This fixes a whole bunch of the failures, which were happening because there is a discrepancy between what the denormalizers accept vs the normalizers generate: we accept ['type' => 'blah'] because Field API magic automatically maps this to ['type' => ['value' => 'blah']] for single-property field types. Some of the request bodies were using that "feature". Simply changing the request body to their "fully expressed" equivalents solves many of the failures (because it means we can actually check that the request body is a subset of the response body).

Similar, yet different: UserResourceTestBase::getNormalizedPostEntity() was using a randomly generated name. This meant that every call to that method returned a different name. Which makes testing impossible. It's also the only entity REST test to do this. So, shifting to a hardcoded name, just like all other tests, is enough.

The Term tests are failing in a different place: loading the entity from storage and then normalizing it yields a different result than the normalization we get from decoding the serialization in the response body.

Why?

Turns out that

$entity = $this->entityManager->getStorage($entity_type_id)->create($create_params);

causes $term->fields->parent to be initialized to have this value: 'target_id' => 0. So that's also what we see in the PATCH/POST response body: ['parent' => [['target_id' => 0]].

But if we load a stored term entity, $term->fields->parent does not exist at all, because there's no value set for it. Hence it's normalized to ['parent' => []].

Similar story for $term->description.

A potential work-around is for us to explicitly specify a target_id and a description in our PATCH/POST request body. But turns out that only solves the description case — apparently specifying target_id = 0 specifically still causes a ['parent' => []] normalization! Actually, even setting target_id = 1 causes this. Probably because is configured to use custom storage:

    $fields['parent'] = BaseFieldDefinition::create('entity_reference')
      ->setLabel(t('Term Parents'))
      ->setDescription(t('The parents of this term.'))
      ->setSetting('target_type', 'taxonomy_term')
      ->setCardinality(BaseFieldDefinition::CARDINALITY_UNLIMITED)
      ->setCustomStorage(TRUE);

But then I realized… we already have an issue for this: #2543726: Make $term->parent behave like any other entity reference field, to fix REST and Migrate support and de-customize its Views integration.

The remaining problems with User are field-access related. The new assertions being added by this patch contain a simple but severe bug:

+++ b/core/modules/rest/tests/src/Functional/EntityResource/EntityResourceTestBase.php
@@ -856,6 +856,18 @@ public function testPost() {
+    $created_entity_normalization = $this->serializer->normalize($created_entity, static::$format);

This is normalizing the given entity. Content entity normalization checks field access. Field access uses the current user by default. In this environment, there isn't a current user, so it falls back to the anonymous user. That explains why User*AnonTest are passing, but not basic_auth/cookie tests!

So, we need this to normalize with the same user account as our actual tests. That's all :)

#12 should bring it down to 10 failures. #15 to 4.

#8:

Interesting that the number of failures is different on 8.3.x vs 8.4.x!

The reason is simple: \Drupal\Tests\datetime\Functional\EntityResource\EntityTest\EntityTestDateonlyTest and \Drupal\Tests\datetime\Functional\EntityResource\EntityTest\EntityTestDatetimeTest only exist in 8.4.x! And since #10 fixed the failures for EntityTest, those failures were also fixed.

That now only leaves CommentHalJson*Test and ItemHalJson*Test.

The failing CommentHalJson*Test: for some reason the entity_id field is missing in POST response bodies (so, @dawehner, thank you very much for #9, but AFAICT that's not what is happening :O). I tried reproducing this manually: POSTing a HAL+JSON comment, and stepping through the normalization process to observe why entity_id was missing… only to observe that it's actually generated just fine :/ I have no explanation for this. Other than: WTF?! — at least for now :)

Similarly, the failing ItemHalJson*Test the problem is similar: the fid field is missing in POST response bodies. I haven't done any step-by-step debugging there yet.

If somebody else could push this one over the finish line, that'd be wonderful!

This is blocking #2300677: JSON:API POST/PATCH support for fully validatable config entities, which is major, hence this is major too.

#21 Turns out that's it. I wonder how the hell I managed to observe something else in my manual testing in #19 then… I think I might've confused entity_id and id? (That is quite confusing in Comment entities.)

With that confirmed, let's work on the fix. The root cause of this problem is:

+++ b/core/modules/rest/tests/src/Functional/EntityResource/EntityResourceTestBase.php
@@ -856,6 +856,18 @@ public function testPost() {
+    // Subset, not same, because we can e.g. send just the target_id for the
+    // bundle in a PATCH request, but the response will include more properties.
+    $this->assertArraySubset($this->getNormalizedPostEntity(), $created_entity_normalization, TRUE);

Using assertArraySubset() is simply a bad assumption: this assumption only holds for the simplest possible normalization, i.e. the default normalization that Drupal uses. But it breaks down for HAL+JSON, as well as for e.g. JSON API.

Then a better way to assert that the sent values are indeed present is by iterating over all fields in the normalization that's being POSTed and then comparing that with the actually stored values.

This reroll brings us to just 3 failures. The remaining 3 failures are in UserHal*Test::testPatch(). Investigating…

Those 3 remaining failures had the same root cause as #15, I just only thought of fixing it for testPost() in #15. This then fixes it for testPatch() too.

This is ready for final review.

Whew, so glad to have this be RTBC! It'll be great to finally start pushing #2300677: JSON:API POST/PATCH support for fully validatable config entities forward again, after @dawehner's incredible effort to get that going again!

Indeed that word is missing. It's present for the PATCH test coverage. Fixed :)

This unblocked #2300677: JSON:API POST/PATCH support for fully validatable config entities, yay!

And now we're removing the temporary work-around that this introduced: #2942522: Follow-up for #2543726: remove work-around in POST test coverage for bug in Term entity type that has been fixed — hurray!