Commit 09ff81ae authored by catch's avatar catch

Issue #2167039 by likin, dawehner, Wim Leers, tstoeckler, vijaycs85:...

Issue #2167039 by likin, dawehner, Wim Leers, tstoeckler, vijaycs85: Regression: page cache tags broken, also: some routes incorrectly use _controller -> No page object in template variables -> Fatal in seven_preprocess_page().
parent acd3a59e
...@@ -3147,7 +3147,7 @@ function drupal_page_set_cache(Response $response, Request $request) { ...@@ -3147,7 +3147,7 @@ function drupal_page_set_cache(Response $response, Request $request) {
// because by the time it is read, the configuration might change. // because by the time it is read, the configuration might change.
'page_compressed' => $page_compressed, 'page_compressed' => $page_compressed,
), ),
'tags' => array('content' => TRUE) + drupal_cache_tags_page_get(), 'tags' => array('content' => TRUE) + drupal_cache_tags_page_get($response),
'expire' => Cache::PERMANENT, 'expire' => Cache::PERMANENT,
'created' => REQUEST_TIME, 'created' => REQUEST_TIME,
); );
...@@ -4358,36 +4358,21 @@ function drupal_render_collect_cache_tags($element, $tags = array()) { ...@@ -4358,36 +4358,21 @@ function drupal_render_collect_cache_tags($element, $tags = array()) {
return $tags; return $tags;
} }
/**
* A #post_render callback at the top level of the $page array. Collects the
* tags for use in page cache.
*
* @param string $children
* An HTML string of rendered output.
* @param array $elements
* A render array.
*
* @return string
* The same $children that was passed in - no modifications.
*/
function drupal_post_render_cache_tags_page_set($children, array $elements) {
if (drupal_page_is_cacheable()) {
$tags = &drupal_static('system_cache_tags_page', array());
$tags = drupal_render_collect_cache_tags($elements);
}
return $children;
}
/** /**
* Return the cache tags that were stored during drupal_render_page(). * Return the cache tags that were stored during drupal_render_page().
* *
* @param \Symfony\Component\HttpFoundation\Response $response
* The response object.
* @return array * @return array
* An array of cache tags. * An array of cache tags.
* *
* @see drupal_post_render_cache_tags_page_set() * @see \Drupal\Core\EventSubscriber\HtmlViewSubscriber::onHtmlPage()
*/ */
function drupal_cache_tags_page_get() { function drupal_cache_tags_page_get(Response $response) {
return drupal_static('system_cache_tags_page', array()); if (($tags = $response->headers->get('cache_tags')) && $tags = unserialize($tags)) {
return $tags;
}
return array();
} }
/** /**
......
<?php
/**
* @file
* Contains \Drupal\Core\CacheableInterface
*/
namespace Drupal\Core\Cache;
/**
* Defines an interface for objects which are potentially cacheable.
*/
interface CacheableInterface {
/**
* The cache keys associated with this potentially cacheable object.
*
* @return array
* An array of strings or cache constants, used to generate a cache ID.
*/
public function getCacheKeys();
/**
* The cache tags associated with this potentially cacheable object.
*
* @return array
* An array of cache tags.
*/
public function getCacheTags();
/**
* The bin to use for this potentially cacheable object.
*
* @return string
* The name of the cache bin to use.
*/
public function getCacheBin();
/**
* The maximum age for which this object may be cached.
*
* @return int
* The maximum time in seconds that this object may be cached.
*/
public function getCacheMaxAge();
/**
* Indicates whether this object is cacheable.
*
* @return bool
* Returns TRUE if the object is cacheable, FALSE otherwise.
*/
public function isCacheable();
}
...@@ -71,7 +71,9 @@ protected function createHtmlFragment($page_content, Request $request) { ...@@ -71,7 +71,9 @@ protected function createHtmlFragment($page_content, Request $request) {
); );
} }
$fragment = new HtmlFragment(drupal_render($page_content)); $cache_tags = $this->drupalRenderCollectCacheTags($page_content);
$cache = !empty($cache_tags) ? array('tags' => $cache_tags) : array();
$fragment = new HtmlFragment($this->drupalRender($page_content), $cache);
// A title defined in the return always wins. // A title defined in the return always wins.
if (isset($page_content['#title'])) { if (isset($page_content['#title'])) {
...@@ -84,4 +86,22 @@ protected function createHtmlFragment($page_content, Request $request) { ...@@ -84,4 +86,22 @@ protected function createHtmlFragment($page_content, Request $request) {
return $fragment; return $fragment;
} }
/**
* Wraps drupal_render().
*
* @todo: Remove as part of https://drupal.org/node/2182149
*/
protected function drupalRender(&$elements, $is_recursive_call = FALSE) {
return drupal_render($elements, $is_recursive_call);
}
/**
* Wraps drupal_render_collect_cache_tags()
*
* @todo: Remove as part of https://drupal.org/node/2182149
*/
protected function drupalRenderCollectCacheTags($element, $tags = array()) {
return drupal_render_collect_cache_tags($element, $tags);
}
} }
...@@ -65,6 +65,18 @@ public function onHtmlPage(GetResponseForControllerResultEvent $event) { ...@@ -65,6 +65,18 @@ public function onHtmlPage(GetResponseForControllerResultEvent $event) {
// to return an object implementing __toString(), but that is not // to return an object implementing __toString(), but that is not
// recommended. // recommended.
$response = new Response((string) $this->renderer->renderPage($page), $page->getStatusCode()); $response = new Response((string) $this->renderer->renderPage($page), $page->getStatusCode());
if ($tags = $page->getCacheTags()) {
$response->headers->set('cache_tags', serialize($tags));
}
if ($keys = $page->getCacheKeys()) {
$response->headers->set('cache_keys', serialize($keys));
}
if ($bin = $page->getCacheBin()) {
$response->headers->set('cache_bin', $bin);
}
if ($max_age = $page->getCacheMaxAge()) {
$response->headers->set('cache_max_age', $max_age);
}
$event->setResponse($response); $event->setResponse($response);
} }
} }
......
...@@ -36,21 +36,27 @@ public function __construct(LanguageManager $language_manager) { ...@@ -36,21 +36,27 @@ public function __construct(LanguageManager $language_manager) {
* {@inheritdoc} * {@inheritdoc}
*/ */
public function render(HtmlFragment $fragment, $status_code = 200) { public function render(HtmlFragment $fragment, $status_code = 200) {
$page = new HtmlPage('', $fragment->getTitle()); // Converts the given HTML fragment which represents the main content region
// of the page into a render array.
$page_content['main'] = array( $page_content['main'] = array(
'#markup' => $fragment->getContent(), '#markup' => $fragment->getContent(),
'#cache' => array('tags' => $fragment->getCacheTags()),
); );
$page_content['#title'] = $page->getTitle(); $page_content['#title'] = $fragment->getTitle();
// Build the full page array by calling drupal_prepare_page(), which invokes
// hook_page_build(). This adds the other regions to the page.
$page_array = drupal_prepare_page($page_content); $page_array = drupal_prepare_page($page_content);
$page = $this->preparePage($page, $page_array); // Collect cache tags for all the content in all the regions on the page.
$tags = drupal_render_collect_cache_tags($page_array);
// Build the HtmlPage object.
$page = new HtmlPage('', array('tags' => $tags), $fragment->getTitle());
$page = $this->preparePage($page, $page_array);
$page->setBodyTop(drupal_render($page_array['page_top'])); $page->setBodyTop(drupal_render($page_array['page_top']));
$page->setBodyBottom(drupal_render($page_array['page_bottom'])); $page->setBodyBottom(drupal_render($page_array['page_bottom']));
$page->setContent(drupal_render($page_array)); $page->setContent(drupal_render($page_array));
$page->setStatusCode($status_code); $page->setStatusCode($status_code);
return $page; return $page;
......
...@@ -9,6 +9,7 @@ ...@@ -9,6 +9,7 @@
use Drupal\Component\Utility\String; use Drupal\Component\Utility\String;
use Drupal\Component\Utility\Xss; use Drupal\Component\Utility\Xss;
use Drupal\Core\Cache\CacheableInterface;
use Drupal\Core\Utility\Title; use Drupal\Core\Utility\Title;
/** /**
...@@ -18,7 +19,7 @@ ...@@ -18,7 +19,7 @@
* https://drupal.org/node/1871596#comment-7134686 * https://drupal.org/node/1871596#comment-7134686
* @todo Add method replacements for *all* data sourced by html.tpl.php. * @todo Add method replacements for *all* data sourced by html.tpl.php.
*/ */
class HtmlFragment { class HtmlFragment implements CacheableInterface {
/** /**
* HTML content string. * HTML content string.
...@@ -34,14 +35,30 @@ class HtmlFragment { ...@@ -34,14 +35,30 @@ class HtmlFragment {
*/ */
protected $title = ''; protected $title = '';
/**
* The cache metadata of this HtmlFragment.
*
* @var array
*/
protected $cache = array();
/** /**
* Constructs a new HtmlFragment. * Constructs a new HtmlFragment.
* *
* @param string $content * @param string $content
* The content for this fragment. * The content for this fragment.
* @param array $cache_info
* The cache information.
*/ */
public function __construct($content = '') { public function __construct($content = '', array $cache_info = array()) {
$this->content = $content; $this->content = $content;
$this->cache = $cache_info + array(
'keys' => array(),
'tags' => array(),
'bin' => NULL,
'max_age' => 0,
'is_cacheable' => TRUE,
);
} }
/** /**
...@@ -123,4 +140,41 @@ public function getTitle() { ...@@ -123,4 +140,41 @@ public function getTitle() {
return $this->title; return $this->title;
} }
/**
* {@inheritdoc}
*
* @TODO Use a trait once we require php 5.4 for all the cache methods.
*/
public function getCacheKeys() {
return $this->cache['keys'];
}
/**
* {@inheritdoc}
*/
public function getCacheTags() {
return $this->cache['tags'];
}
/**
* {@inheritdoc}
*/
public function getCacheBin() {
return $this->cache['bin'];
}
/**
* {@inheritdoc}
*/
public function getCacheMaxAge() {
return $this->cache['max_age'];
}
/**
* {@inheritdoc}
*/
public function isCacheable() {
return $this->cache['is_cacheable'];
}
} }
...@@ -54,11 +54,13 @@ class HtmlPage extends HtmlFragment { ...@@ -54,11 +54,13 @@ class HtmlPage extends HtmlFragment {
* *
* @param string $content * @param string $content
* (optional) The body content of the page. * (optional) The body content of the page.
* @param array $cache_info
* The cache information.
* @param string $title * @param string $title
* (optional) The title of the page. * (optional) The title of the page.
*/ */
public function __construct($content = '', $title = '') { public function __construct($content = '', array $cache_info = array(), $title = '') {
parent::__construct($content); parent::__construct($content, $cache_info);
$this->title = $title; $this->title = $title;
......
...@@ -57,7 +57,7 @@ aggregator.feed_edit: ...@@ -57,7 +57,7 @@ aggregator.feed_edit:
aggregator.feed_refresh: aggregator.feed_refresh:
path: '/admin/config/services/aggregator/update/{aggregator_feed}' path: '/admin/config/services/aggregator/update/{aggregator_feed}'
defaults: defaults:
_controller: '\Drupal\aggregator\Controller\AggregatorController::feedRefresh' _content: '\Drupal\aggregator\Controller\AggregatorController::feedRefresh'
_title: 'Update items' _title: 'Update items'
requirements: requirements:
_permission: 'administer news feeds' _permission: 'administer news feeds'
......
...@@ -176,7 +176,7 @@ public function getOverviewRoute() { ...@@ -176,7 +176,7 @@ public function getOverviewRoute() {
return new Route( return new Route(
$this->getBaseRoute()->getPath() . '/translate', $this->getBaseRoute()->getPath() . '/translate',
array( array(
'_controller' => '\Drupal\config_translation\Controller\ConfigTranslationController::itemPage', '_content' => '\Drupal\config_translation\Controller\ConfigTranslationController::itemPage',
'plugin_id' => $this->getPluginId(), 'plugin_id' => $this->getPluginId(),
), ),
array('_config_translation_overview_access' => 'TRUE') array('_config_translation_overview_access' => 'TRUE')
......
...@@ -176,7 +176,7 @@ public function testGetOverviewRouteParameters() { ...@@ -176,7 +176,7 @@ public function testGetOverviewRouteParameters() {
public function testGetOverviewRoute() { public function testGetOverviewRoute() {
$expected = new Route('/admin/config/system/site-information/translate', $expected = new Route('/admin/config/system/site-information/translate',
array( array(
'_controller' => '\Drupal\config_translation\Controller\ConfigTranslationController::itemPage', '_content' => '\Drupal\config_translation\Controller\ConfigTranslationController::itemPage',
'plugin_id' => 'system.site_information_settings', 'plugin_id' => 'system.site_information_settings',
), ),
array( array(
......
...@@ -48,11 +48,18 @@ function setUp() { ...@@ -48,11 +48,18 @@ function setUp() {
* Tests deleting nodes clears page cache. * Tests deleting nodes clears page cache.
*/ */
public function testNodeDelete() { public function testNodeDelete() {
$node_path = 'node/' . $this->drupalCreateNode()->id(); $author = $this->drupalCreateUser();
$node_path = 'node/' . $this->drupalCreateNode(array('uid' => $author->id()))->id();
// Populate page cache. // Populate page cache.
$this->drupalGet($node_path); $this->drupalGet($node_path);
// Verify the presence of the correct cache tags.
$cid_parts = array(url($node_path, array('absolute' => TRUE)), 'html');
$cid = sha1(implode(':', $cid_parts));
$cache_entry = \Drupal::cache('page')->get($cid);
$this->assertIdentical($cache_entry->tags, array('content:1', 'user:' . $author->id(), 'filter_format:plain_text'));
// Login and delete the node. // Login and delete the node.
$this->drupalLogin($this->adminUser); $this->drupalLogin($this->adminUser);
$this->drupalGet($node_path . '/delete'); $this->drupalGet($node_path . '/delete');
......
<?php <?php
use Drupal\Core\Database\Database; use Drupal\Core\Database\Database;
use Drupal\Core\Page\HtmlPage;
use Drupal\simpletest\TestBase; use Drupal\simpletest\TestBase;
use Symfony\Component\Process\PhpExecutableFinder; use Symfony\Component\Process\PhpExecutableFinder;
......
...@@ -102,7 +102,7 @@ public function render(array $output, $status_code = 200) { ...@@ -102,7 +102,7 @@ public function render(array $output, $status_code = 200) {
$request = \Drupal::request(); $request = \Drupal::request();
$output['#title'] = $this->titleResolver->getTitle($request, $request->attributes->get(RouteObjectInterface::ROUTE_OBJECT)); $output['#title'] = $this->titleResolver->getTitle($request, $request->attributes->get(RouteObjectInterface::ROUTE_OBJECT));
} }
$page = new HtmlPage('', $output['#title']); $page = new HtmlPage('', isset($output['#cache']) ? $output['#cache'] : array(), $output['#title']);
$page_array = drupal_prepare_page($output); $page_array = drupal_prepare_page($output);
......
...@@ -58,8 +58,15 @@ function testPageCacheTags() { ...@@ -58,8 +58,15 @@ function testPageCacheTags() {
$tags = array('system_test_cache_tags_page' => TRUE); $tags = array('system_test_cache_tags_page' => TRUE);
$this->drupalGet($path); $this->drupalGet($path);
$this->assertEqual($this->drupalGetHeader('X-Drupal-Cache'), 'MISS'); $this->assertEqual($this->drupalGetHeader('X-Drupal-Cache'), 'MISS');
// Verify a cache hit, but also the presence of the correct cache tags.
$this->drupalGet($path); $this->drupalGet($path);
$this->assertEqual($this->drupalGetHeader('X-Drupal-Cache'), 'HIT'); $this->assertEqual($this->drupalGetHeader('X-Drupal-Cache'), 'HIT');
$cid_parts = array(url($path, array('absolute' => TRUE)), 'html');
$cid = sha1(implode(':', $cid_parts));
$cache_entry = \Drupal::cache('page')->get($cid);
$this->assertIdentical($cache_entry->tags, array('content:1', 'system_test_cache_tags_page:1'));
Cache::invalidateTags($tags); Cache::invalidateTags($tags);
$this->drupalGet($path); $this->drupalGet($path);
$this->assertEqual($this->drupalGetHeader('X-Drupal-Cache'), 'MISS'); $this->assertEqual($this->drupalGetHeader('X-Drupal-Cache'), 'MISS');
......
...@@ -269,7 +269,6 @@ function system_element_info() { ...@@ -269,7 +269,6 @@ function system_element_info() {
'#theme_wrappers' => array('form'), '#theme_wrappers' => array('form'),
); );
$types['page'] = array( $types['page'] = array(
'#post_render' => array('drupal_post_render_cache_tags_page_set'),
'#show_messages' => TRUE, '#show_messages' => TRUE,
'#theme' => 'page', '#theme' => 'page',
); );
......
...@@ -48,7 +48,7 @@ system_test.lock_exit: ...@@ -48,7 +48,7 @@ system_test.lock_exit:
system_test.cache_tags_page: system_test.cache_tags_page:
path: '/system-test/cache_tags_page' path: '/system-test/cache_tags_page'
defaults: defaults:
_controller: '\Drupal\system_test\Controller\SystemTestController::system_test_cache_tags_page' _content: '\Drupal\system_test\Controller\SystemTestController::system_test_cache_tags_page'
requirements: requirements:
_access: 'TRUE' _access: 'TRUE'
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment