Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Trying to get field values with
/** @var \Drupal\search_api\Item\Item $item */
$item->getFields();
// or
$item->getField('url');
throws error
The controller result claims to be providing relevant cache metadata, but leaked metadata was detected. Please ensure you are not rendering content too early. Returned object class: Drupal\rest\ResourceResponse.
This happens when Index has URL value for item.
How to reproduce bug:
- Create search index and add URI (search_api_url) field and re-index content.
- Create custom REST resource for GET
drush generate rest-resource
ordrupal gprr
. - Write custom Search API request for index where URI field exist. Not matter what you choose for parse mode and other stuff. Try to call
$item->getFields()
or$item->getField('url')->getValues()
. - Enable REST and make request to it. And you will see the exception.
My simple code to copy-paste for test:
<?php
namespace Drupal\MUMODULE\Plugin\rest\resource;
use Drupal\rest\Plugin\ResourceBase;
use Drupal\rest\ResourceResponse;
use Drupal\search_api\Entity\Index;
use Psr\Log\LoggerInterface;
use Symfony\Component\DependencyInjection\ContainerInterface;
/**
* Represents how to throw bug in REST resource.
*
* @RestResource (
* id = "search_api_bug_example",
* label = @Translation("Search API bug example"),
* uri_paths = {
* "canonical" = "/api/search-api-bug",
* }
* )
*/
class SearchApiUriBug extends ResourceBase {
/**
* {@inheritdoc}
*/
public function __construct(array $configuration, $plugin_id, $plugin_definition, array $serializer_formats, LoggerInterface $logger) {
parent::__construct($configuration, $plugin_id, $plugin_definition, $serializer_formats, $logger);
}
/**
* {@inheritdoc}
*/
public static function create(ContainerInterface $container, array $configuration, $plugin_id, $plugin_definition) {
return new static(
$configuration,
$plugin_id,
$plugin_definition,
$container->getParameter('serializer.formats'),
$container->get('logger.factory')->get('rest')
);
}
/**
* Responds to GET requests.
*/
public function get() {
$index = Index::load('global_index');
/** @var \Drupal\search_api\Plugin\views\query\SearchApiQuery $query */
$query = $index->query();
/** @var \Drupal\search_api\ParseMode\ParseModePluginManager $parse_mode */
$parse_mode = \Drupal::service('plugin.manager.search_api.parse_mode')
->createInstance('direct');
$query->setParseMode($parse_mode);
$query->keys('TEST');
$query->setFulltextFields();
$query->range(0, 5);
$search_result = $query->execute();
if ($search_result->getResultCount()) {
/** @var \Drupal\search_api\Item\Item $item */
foreach ($search_result as $item) {
$item->getFields();
}
}
return new ResourceResponse('You will not see it ;)');
}
}
Comment | File | Size | Author |
---|---|---|---|
#13 | 2958811-13--url_processor_rest_error.patch | 5.03 KB | drunken monkey |
|
Comments
Comment #2
NiklanThe patch which fix this error.
Comment #3
NiklanComment #4
borisson_The fact that we are not getting any errors in tests about this, means that we should write a new regression-test for this as well.
Comment #6
drunken monkeyPhew, that bug seems pretty complicated. The crux is apparently
\Drupal\Core\Render\MetadataBubblingUrlGenerator::generateFromRoute()
– when$collect_bubbleable_metadata
isFALSE
, this will bubble metadata to the render context, which causesEarlyRenderingControllerWrapperSubscriber
to throw an exception because it thinks metadata is lost. So this seeming no-op change (at least I was confused how that would help) is indeed a workaround for the problem – we more or less explicitly throw away metadata, instead of implicitly, which convinces Core that we mean it and that's OK.Anyways, a failing test for this would really be helpful. Setting up a complete REST test for this is probably way too much work, but maybe we can set up the same event subscriber and then trigger the exception, somehow? (Really not sure, though. Seems like we'd need to do an internal HTTP request for that.)
In any case, the attached revision should at least fix the existing fail. (The others were in HEAD.)
Comment #7
drunken monkeySetting to NW for regression test.
Comment #8
Andriy Khomych CreditAttribution: Andriy Khomych as a volunteer commentedRerolled patch for the 8.x-1.x
However, tests have conflicts during rerolling. I guess it is better to rewrite some parts of them.
Comment #9
drunken monkeyThanks for posting!
The attached patch also contains the updated tests.
As this still seems to be relevant, and writing a test would really be a lot of work for a small problem, I think we can skip it in this case. Please confirm that the attached revision still works for you and I’ll commit it.
Comment #11
drunken monkeyTest fail seems completely unrelated, but no harm in making that test a bit more robust regardless.
Also, removed one unused
use
statement.Already got private feedback that the patch still works as expected, so will commit in a day or two if no-one complains.
Comment #13
drunken monkeyComment #14
drunken monkeyCommitted.