Make Json response cacheable

The IS lacks a bit of context/description. Taken from #3066540: Is it possible to use a cacheable response? → , the idea seems to not make this cacheable for Drupal itself, but allow a custom module to alter the cache metadata in order to have the response cached by the client – is that correct? Caching search results within Drupal is mostly pointless, as there are only few scenarios where you can expect to reach a reasonable cache hit rate.

Anyways, on the patch itself, I’m unfortunately no expert in the Drupal cache system, but most of the code does seem sensible. A few comments, though:

1. +++ b/src/Controller/AutocompleteController.php
   @@ -167,7 +172,9 @@ class AutocompleteController extends ControllerBase implements ContainerInjectio
   -            $label = $this->renderer->render($build);
   +            $label = $this->renderer->executeInRenderContext(new RenderContext(), function () use ($build) {
   +              return $this->renderer->render($build);
   +            });
   

   Wouldn’t this discard any caching information coming from that build array? As said above, I’m no expert (or even intermediate, I guess), but it does seem like we should extract the metadata from the render context afterwards?

2. +++ b/src/Controller/AutocompleteController.php
   @@ -177,15 +184,19 @@ class AutocompleteController extends ControllerBase implements ContainerInjectio
   +            $url = $suggestion->getUrl();
   +            if ($entity = $url->getOption('entity')) {
   +              $cacheable_metadata->addCacheableDependency($entity);
   +            }
                // Generate an HTML-free version of the label to use as the value.
                // Setting the label as the value here is necessary for proper
                // accessibility via screen readers (which will otherwise read the
                // URL).
   -            $url = $suggestion->getUrl()->toString();
   +            $url = $suggestion->getUrl()->toString(TRUE);
   

   $suggestion->getUrl()->toString(TRUE) already returns a cacheable object, no need to look for an entity manually (I think).

3. +++ b/src/Controller/AutocompleteController.php
   @@ -206,7 +217,12 @@ class AutocompleteController extends ControllerBase implements ContainerInjectio
   +    $this->moduleHandler()->alter('search_api_autocomplete_response', $response);
   

   This hook still needs to be documented in search_api_autocomplete.api.php.

Most of this is addressed in the attached patch revision. However, this still needs a hook documentation and, more importantly, tests. Also, it seems like it even breaks current tests? (Still setting to NR for the test bot, but will switch to NW right afterwards.)

I am getting the leaked metadata errors with the current patch. I think its due to the view been rendered within a block. Autocomplete builds the view, and theres a check in core to see if the view has a URL in `core/modules/views/src/Form/ViewsExposedForm.php`. If theres no URL its rendering one which is triggering the leaked metadata error.

Moving `$search->createQuery` inside `executeInRenderContext` resolves this.

@lstirk: Seems like what you describe might have already been fixed in 🐛 LogicException: Bubbling failed Fixed ? Please try with the latest dev version of the module.