Problem/Motivation

#2843753: Prevent ContentModerationState from being exposed by REST's EntityResource ensured that the ContentModerationState entity type would never be exposed by the REST module, by implementing hook_rest_resource_alter() to remove the automatically derived REST resource plugin for this entity type. This was effectively a hack, a work-around, because the proper infrastructure was missing.

#2936714: Entity type definitions cannot be set as 'internal' just introduced that proper infrastructure: the ability to mark entity types as "internal" (which already was introduced for fields and properties several months ago).

Thanks to that new infrastructure, the REST module can now start to respect that: internal entity types should never be exposed via HTTP APIs such as the rest module's. Similarly, contributed modules providing HTTP APIs such as https://www.drupal.org/project/jsonapi (#2932035: ResourceTypes should be internal when EntityType::isInternal is TRUE) and https://www.drupal.org/project/graphql can also start to support this.

Proposed resolution

  1. Update \Drupal\rest\Plugin\Deriver\EntityDeriver to respect EntityTypeInterface::isInternal()
  2. Set internal = TRUE on \Drupal\content_moderation\Entity\ContentModerationState to use this
  3. Remove content_moderation_rest_resource_alter(), because it's no longer necessary
  4. The existing test coverage in \Drupal\Tests\content_moderation\Kernel\ContentModerationStateResourceTest proves that this still works as expected for the one entity type in Drupal core that is "internal"
  5. Finally, update the automated "entity REST test coverage" test, that ensures that every entity type has REST test coverage, to only require REST test coverage for non-internal entity types

Remaining tasks

None.

User interface changes

None.

API changes

None.

Data model changes

None.

CommentFileSizeAuthor
#2 2941622-2.patch5.06 KBWim Leers
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Wim Leers created an issue. See original summary.

Wim Leers’s picture

Status: Active » Needs review
FileSize
5.06 KB
Wim Leers’s picture

amateescu’s picture

The Workspace module is following closely :) #2941625: Mark the workspace_association entity type as internal

Wim Leers’s picture

timmillwood’s picture

Status: Needs review » Reviewed & tested by the community

Well this makes things a bit more easier!

LGTM!

Wim Leers’s picture

Issue summary: View changes

Yay!

(And fix typo in IS.)

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 2: 2941622-2.patch, failed testing. View results

timmillwood’s picture

Status: Needs work » Reviewed & tested by the community

Unrelated test fail

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Committed and pushed 5a9de4fc76 to 8.6.x and 054722c94c to 8.5.x. Thanks!

Backported to 8.5.x because as @Wim Leers pointed out to me this

is the final blocker for the entire API-First ecosystem to behave in a consistent manner (core REST + contrib JSON API + contrib GraphQL + contrib Relaxed). Rather than treating content_moderation_state in a special way using a REST-specific hook, this allows us to remove that REST-specific hook and add generic metadata. So having https://www.drupal.org/project/drupal/issues/2941622 land in 8.5 would be a big enabler, a big win for API-First for 2018.

  • alexpott committed 5a9de4f on 8.6.x
    Issue #2941622 by Wim Leers: Make REST's EntityResource deriver exclude...

  • alexpott committed 054722c on 8.5.x
    Issue #2941622 by Wim Leers: Make REST's EntityResource deriver exclude...
Wim Leers’s picture

🎉

Thank you!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.