Add path alias (PathItem) field PATCH test coverage

Let's start by adding a test to prove that it is secure in HEAD: PATCHing a node's path (URL alias) should result in a 403.

Patch lifted from #2824851-123: EntityResource::patch() makes an incorrect assumption about entity keys, hence results in incorrect behavior.

#2 added sensible security coverage. But to really know for certain that things are working as expected, we should expand the test coverage to verify that granting the create url aliases permission then results in a 200 response: granting the necessary permission does allow the path field to be changed.

Note that there are three entity types that have a path field: node + term + media. The latter doesn't have any REST test coverage yet, so we leave that one out of consideration. Back when we worked on fixing path fields handling in #2846554: Make the PathItem field type actually computed and auto-load stored aliases, we chose to use NodeResourceTestBase to NOT GRANT the create url aliases permission by default, and to use TermResourceTestBase to GRANT the create url aliases permission by default, i.e. they'd be each other opposites.
For that reason, we should also add similar test coverage to TermResourceTestBase as #2 already did for NodeResourceTestBase. But rather than expecting a 403 response, we should expect a 200 response.

Now we have the test coverage we need!

Except this is going to fail… changing path aliases doesn't work at all in HEAD :( But at least the permission is respected as it should!

Patch lifted from #2824851-124: EntityResource::patch() makes an incorrect assumption about entity keys, hence results in incorrect behavior.

This is the failure:

1) Drupal\Tests\hal\Functional\EntityResource\Term\TermHalJsonAnonTest::testPatchPath
Failed asserting that Array &0 (
    0 => Array &1 (
        'alias' => '/llama'
        'pid' => 1
        'langcode' => 'en'
        'lang' => 'en'
    )
) is identical to Array &0 (
    0 => Array &1 (
        'alias' => '/llamas-rule-the-world'
        'pid' => 1
        'langcode' => 'en'
        'lang' => 'en'
    )
).

(The latter is what is expected. The former shows what is actually stored — in other words: nothing was changed.)

This is failing because PathItem::setValue() is not calling PathItem::ensureLoaded(). But doing that triggers an infinite loop… 😵 Fortunately that's easy to work around! 🙏

Patch lifted from #2824851-126: EntityResource::patch() makes an incorrect assumption about entity keys, hence results in incorrect behavior.

For *HalJson*Test tests, I had to use removeFieldsFromNormalization(), not unset(), so that fields moved to _links also are removed for the HAL+JSON normalization.

Patch lifted from #2824851-134: EntityResource::patch() makes an incorrect assumption about entity keys, hence results in incorrect behavior.

Alright, this patch is green, and ready for final review :)

@dawehner made this remark over at #2824851-133: EntityResource::patch() makes an incorrect assumption about entity keys, hence results in incorrect behavior:

+++ b/core/modules/path/src/Plugin/Field/FieldType/PathItem.php
@@ -162,7 +173,9 @@ public static function mainPropertyName() {
+    static $is_loading = FALSE;

❓ Couldn't you still have a class based variable? Otherwise for whatever reason two instances could maybe interfere ...

AFAICT you can't, because of \Drupal\path\Plugin\Field\FieldType\PathItem::__get().

Huh. I'd swear the explanation in #9 was accurate.

But I just stepped through it with a debugger and indeed was not a problem. I could not get the problem I described in #9 to get triggered. So I must've been wrong. I probably mixed two things up.

I of course like this better!

Definitely! :)

This unblocks #2824851: EntityResource::patch() makes an incorrect assumption about entity keys, hence results in incorrect behavior, yay!

Oh, can you also give issue credit to cburschka? Thanks!

Yep, makes sense — you're making the code introduced in #2846554: Make the PathItem field type actually computed and auto-load stored aliases obsolete and are replacing it with something more thorough. Hence the additional work-arounds added by this issue are no longer necessary. I'm very glad we introduced extra test coverage here that A) help ensure we don't need to fix this again, B) help validate your patch!