Allow computed exposed properties in ComplexData to support cacheability.

See #2910211: Allow computed exposed properties in ComplexData to support cacheability. and #2910211: Allow computed exposed properties in ComplexData to support cacheability. — those interdiffs removed the changes that are now deferred to this issue.

👍

1. +++ b/core/modules/rest/src/EventSubscriber/ResourceResponseSubscriber.php
   @@ -152,14 +160,25 @@ protected function renderResponseBody(Request $request, ResourceResponseInterfac
   +      // @deprecated In Drupal 8.4.0, will be removed before Drupal 9.0.0. Use
   

   8.5.0

2. +++ b/core/modules/system/tests/modules/entity_test/src/TypedData/ComputedString.php
   @@ -15,9 +17,8 @@ class ComputedString extends TypedData {
   -    $computed_value = "Computed! " . $item->get('value')->getString();
    
   -    return $computed_value;
   +    return 'Computed! ' . $item->get('value')->getString();
   

   Nit: This change is unnecessary.

3. Most importantly, I'm missing a failing test-only patch, that proves that the changes in ResourceResponseSubscriber are necessary.

but seems like the test only patch will make more sense after #2871591: Allow ComplexData in TypedData to specify computed properties that should be exposed in normalization and other contexts

True!

So I think this is now really postponed.

#2871591: Allow ComplexData in TypedData to specify computed properties that should be exposed in normalization and other contexts landed.

Straight rebase of #11. Matches #11's diff-2871591-160.txt exactly :)

This blocks #2825487: Fix normalization of File entities: file entities should expose the file URL as a computed property on the 'uri' base field, #2825812: ImageItem should have an "derivatives" computed property, to expose all image style URLs, #2626924: Include processed text in normalizations: "text" field type's "processed" computed property should be non-internal and carry cacheability metadata and #2577923: MenuLinkContent entities pointing to nodes are not deployable: LinkItem should have a "target_uuid" computed property, all of which are 8.5 REST top priorities (#2905563: REST: top priorities for Drupal 8.5.x), and have been REST top priorities for >1 year, which definitely makes this major, just like #2871591: Allow ComplexData in TypedData to specify computed properties that should be exposed in normalization and other contexts.

index ef33817..99d99de 100644
--- a/core/modules/rest/src/EventSubscriber/ResourceResponseSubscriber.php
+++ b/core/modules/rest/src/EventSubscriber/ResourceResponseSubscriber.php

#2864816: HAL LinkManager doesn't add 'url.site' cache context when needed is making the exact same changes. But that patch was also reviewed by @dawehner, and had some additional remarks. So, bringing over the changes from that issue to this one.

1. In #13, I did a straight rebase of @tedbow's patch.
2. In #15, I copied over some changes that were requested and RTBC'd by @dawehner from another issue.
3. The changes to ResourceResponseSuscriber are half of the crucial changes in this issue, and that was also RTBC'd in #2864816
4. The addition of NormalizerBase::addCacheableDependency() plus TypedDataNormalizer::normalize() using it are the other half of crucial changes. This also comes with explicit functional test coverage. I think those changes are sound too.

Therefore RTBC'ing.

+++ b/core/modules/serialization/src/Normalizer/NormalizerBase.php
@@ -81,4 +82,18 @@ protected function checkFormat($format = NULL) {
+    if ($data instanceof CacheableDependencyInterface && isset($context['cacheability'])) {
+      $context['cacheability']->addCacheableDependency($data);

this should use the constant

Other than that, feels like I've reviewed this elsewhere ;) - oh right - #2864816: HAL LinkManager doesn't add 'url.site' cache context when needed

Do you have a preference for the order they go in?

Agreed, done. Trivial change, so back to RTBC.

Do you have a preference for the order they go in?

This issue blocks many others (see #14), so this going in first would make sense!

Fixed on commit

diff --git a/core/modules/rest/src/EventSubscriber/ResourceResponseSubscriber.php b/core/modules/rest/src/EventSubscriber/ResourceResponseSubscriber.php
index 9b12412..babb238 100644
--- a/core/modules/rest/src/EventSubscriber/ResourceResponseSubscriber.php
+++ b/core/modules/rest/src/EventSubscriber/ResourceResponseSubscriber.php
@@ -180,7 +180,7 @@ protected function renderResponseBody(Request $request, ResourceResponseInterfac
       // executeInRenderContext() will be removed before Drupal 9.0.0.)
       $context = new RenderContext();
       $output = $this->renderer
-        ->executeInRenderContext($context, function() use ($serializer, $data, $format, $serialization_context) {
+        ->executeInRenderContext($context, function () use ($serializer, $data, $format, $serialization_context) {
           return $serializer->serialize($data, $format, $serialization_context);
         });
       if ($response instanceof CacheableResponseInterface) {

Committed as 52a598e and pushed to 8.5.x.

Published change record

YAAAAAYYYYYYY 🍾 After about a year, the issues mentioned in #14 are finally unblocked!

Rerolled them all:

1. #2626924-166: Include processed text in normalizations: "text" field type's "processed" computed property should be non-internal and carry cacheability metadata through #2626924-178: Include processed text in normalizations: "text" field type's "processed" computed property should be non-internal and carry cacheability metadata
2. #2825487-109: Fix normalization of File entities: file entities should expose the file URL as a computed property on the 'uri' base field
3. #2825812-107: ImageItem should have an "derivatives" computed property, to expose all image style URLs
4. #2577923-119: MenuLinkContent entities pointing to nodes are not deployable: LinkItem should have a "target_uuid" computed property

This also unblocked #2864816, that's now back at RTBC: #2864816-46: HAL LinkManager doesn't add 'url.site' cache context when needed.

Follow-up created: #2922487: Follow-up for #2910211: fix all deprecation warnings.