Write EntityResourceTestBase subclasses for workspace, content_workspace, and replication_log entities

As of #2868035: Test that all core content+config entity types have functional REST test coverage, every entity type is required to have REST test coverage. To ensure Drupal 8 is actually "API-First": all entities should be accessible via HTTP APIs.

content_workspace is an internal entity type, why would we want to add REST support for it?

Kind of the same for replication_log, is there are any reason to support CRUD for this entity type via REST?

why would we want to add REST support for it?

That's possible. If that is the case, then we should explicitly test that it's not supported. See #2843753: Prevent ContentModerationState from being exposed by REST's EntityResource and #2779931: Add storage exception that enforces unique content_entity_type_id and content_entity_id on the content moderation state content entity, and add access control handler to forbid all access.

Looks great!

1. +++ b/tests/src/Functional/EntityResource/WorkspaceJsonAnonTest.php
   @@ -0,0 +1,26 @@
   + * Test workspace entities for unauthenticated JSON requests.
   
   +++ b/tests/src/Functional/EntityResource/WorkspaceJsonBasicAuthTest.php
   @@ -0,0 +1,36 @@
   + * Test workspace entities for JSON requests via basic auth.
   

   Just FYI: we didn't add these comments to the REST tests in core because it's 100% boilerplate: the class name already explains precisely this :)

   So you can remove it if you want, but if you prefer this, that's fine too

2. +++ b/tests/src/Functional/EntityResource/WorkspaceResourceTestBase.php
   @@ -0,0 +1,185 @@
   +  protected function setUpAuthorization($method) {
   +    $this->grantPermissionsToTestedRole([
   +      'administer workspaces',
   +      'create workspace',
   +      'update workspace layla',
   +      'delete workspace layla',
   +      'view workspace layla',
   +      ]);
   +  }
   

   This should not grant all permissions, but should only grant the permissions necessary depending on the method. That's why $method is a parameter.

3. +++ b/tests/src/Functional/EntityResource/WorkspaceResourceTestBase.php
   @@ -0,0 +1,185 @@
   +      'upstream' => 'local_workspace:live',
   +      ]);
   

   Supernit: The second line I quoted is indented one level too much.

4. +++ b/tests/src/Functional/EntityResource/WorkspaceResourceTestBase.php
   @@ -0,0 +1,185 @@
   +        $this->formatExpectedTimestampItemValues((int) $this->entity->getStartTime()),
   ...
   +        $this->formatExpectedTimestampItemValues($this->entity->getChangedTime()),
   

   Hm, why do we need one return value to be cast to an integer but not the other?

5. +++ b/tests/src/Functional/EntityResource/WorkspaceResourceTestBase.php
   @@ -0,0 +1,185 @@
   +    $http_method_to_entity_operation = [
   +      'GET' => 'view',
   +      'POST' => 'create',
   +      'PATCH' => 'update',
   +      'DELETE' => 'delete',
   +    ];
   +    $operation = $http_method_to_entity_operation[$method];
   +    $entity_access = $this->entity->access($operation, NULL, TRUE);
   +    if (!$entity_access->isAllowed()) {
   +      return $entity_access->getReason();
   +    }
   +    return parent::getExpectedUnauthorizedAccessMessage($method);
   

   See #2928887-3: EntityResource and EntityResourceTestBase have different ideas for access — this should hardcode the expected strings, otherwise we still won't A) know that the messages are helpful, B) know that we regress.

6. +++ b/tests/src/Kernel/WorkspaceInternalResourceTest.php
   @@ -0,0 +1,62 @@
   +    $this->setExpectedException(PluginNotFoundException::class, 'The "entity:content_workspace" plugin does not exist.');
   ...
   +    $this->setExpectedException(PluginNotFoundException::class, 'The "entity:replication_log" plugin does not exist.');
   

   👍

7. --- /dev/null
   +++ b/workspace.link_relation_types.yml
   

   👍 — nice bugfix that this surfaced :)

These tests are needed in the main core patch, so I rerolled #19, fixed a couple of minor things and committed to 8.x-2.x.

This means that the new tests will fail until #2885469: Regression: manually setting the ID field for newly-created content entities is not possible anymore (public follow-up to SA-2017-002) gets in, but well, #2927169: Fix views queries is in the same boat :)

So very glad to see this test coverage, and I'm sure that that gives you some extra confidence too :) And fewer hurdles to get it into core!

#19:
Sorry for not seeing your comment until now :(

RE #18.4: hm, but that then means getStartTime() isn't returning an integer, which violates its interface if I'm reading #2784921-83: Add Workspaces experimental module correctly.

#20: I think you'll like the fact that I've been pushing #2885469: Regression: manually setting the ID field for newly-created content entities is not possible anymore (public follow-up to SA-2017-002) forward then :)

RE #18.4: hm, but that then means getStartTime() isn't returning an integer, which violates its interface if I'm reading #2784921-83: Add Workspaces experimental module correctly.

This happens for many other timestamp fields, and basically all integer ones IIRC. There are many instances of casting to (int) throughout REST test classes.

FWIW, these changes were needed in order to make the core patch pass all the tests, so I've committed them here as well.

I think you'll like the fact that I've been pushing #2885469: Regression: manually setting the ID field for newly-created content entities is not possible anymore (public follow-up to SA-2017-002) forward then :)

Indeed :) Thank you!