diff --git a/core/core.services.yml b/core/core.services.yml index a194ead..082eaaf 100644 --- a/core/core.services.yml +++ b/core/core.services.yml @@ -686,11 +686,16 @@ services: arguments: ['@route_filter.lazy_collector'] tags: - { name: event_subscriber } - url_generator: + url_generator.uncacheable: class: Drupal\Core\Routing\UrlGenerator arguments: ['@router.route_provider', '@path_processor_manager', '@route_processor_manager', '@config.factory', '@request_stack'] calls: - [setContext, ['@?router.request_context']] + url_generator: + class: Drupal\Core\Render\CacheableMetadataAwareUrlGenerator + arguments: ['@url_generator.uncacheable', '@renderer'] + calls: + - [setContext, ['@?router.request_context']] redirect.destination: class: Drupal\Core\Routing\RedirectDestination arguments: ['@request_stack', '@url_generator'] @@ -1395,10 +1400,10 @@ services: arguments: ['@request_stack', '@cache_factory', '@cache_contexts_manager'] renderer: class: Drupal\Core\Render\Renderer - arguments: ['@controller_resolver', '@theme.manager', '@plugin.manager.element_info', '@render_cache', '%renderer.config%'] + arguments: ['@controller_resolver', '@theme.manager', '@plugin.manager.element_info', '@render_cache', '@request_stack', '%renderer.config%'] early_rendering_controller_wrapper_subscriber: class: Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber - arguments: ['@controller_resolver', '@renderer'] + arguments: ['@controller_resolver', '@renderer', '@current_route_match'] tags: - { name: event_subscriber } email.validator: diff --git a/core/includes/pager.inc b/core/includes/pager.inc index 7ce3407..eb97a5c 100644 --- a/core/includes/pager.inc +++ b/core/includes/pager.inc @@ -176,6 +176,7 @@ function template_preprocess_pager(&$variables) { $element = $variables['pager']['#element']; $parameters = $variables['pager']['#parameters']; $quantity = $variables['pager']['#quantity']; + $route_name = $variables['pager']['#route_name']; global $pager_page_array, $pager_total; // Nothing to do if there is only one page. @@ -218,7 +219,7 @@ function template_preprocess_pager(&$variables) { $options = array( 'query' => pager_query_add_page($parameters, $element, 0), ); - $items['first']['href'] = \Drupal::url('', [], $options); + $items['first']['href'] = \Drupal::url($route_name, [], $options); if (isset($tags[0])) { $items['first']['text'] = $tags[0]; } @@ -227,7 +228,7 @@ function template_preprocess_pager(&$variables) { $options = array( 'query' => pager_query_add_page($parameters, $element, $pager_page_array[$element] - 1), ); - $items['previous']['href'] = \Drupal::url('', [], $options); + $items['previous']['href'] = \Drupal::url($route_name, [], $options); if (isset($tags[1])) { $items['previous']['text'] = $tags[1]; } @@ -243,7 +244,7 @@ function template_preprocess_pager(&$variables) { $options = array( 'query' => pager_query_add_page($parameters, $element, $i - 1), ); - $items['pages'][$i]['href'] = \Drupal::url('', [], $options); + $items['pages'][$i]['href'] = \Drupal::url($route_name, [], $options); if ($i == $pager_current) { $variables['current'] = $i; } @@ -260,7 +261,7 @@ function template_preprocess_pager(&$variables) { $options = array( 'query' => pager_query_add_page($parameters, $element, $pager_page_array[$element] + 1), ); - $items['next']['href'] = \Drupal::url('', [], $options); + $items['next']['href'] = \Drupal::url($route_name, [], $options); if (isset($tags[3])) { $items['next']['text'] = $tags[3]; } @@ -269,7 +270,7 @@ function template_preprocess_pager(&$variables) { $options = array( 'query' => pager_query_add_page($parameters, $element, $pager_max - 1), ); - $items['last']['href'] = \Drupal::url('', [], $options); + $items['last']['href'] = \Drupal::url($route_name, [], $options); if (isset($tags[4])) { $items['last']['text'] = $tags[4]; } @@ -311,6 +312,12 @@ function pager_query_add_page(array $query, $element, $index) { if ($current_request_query = pager_get_query_parameters()) { $query = array_merge($current_request_query, $query); } + + // This is is based on the entire current query string. We need to ensure + // cacheability is affected accordingly. + $build = ['#cache' => ['contexts' => ['url.query_args']]]; + \Drupal::service('renderer')->render($build); + return $query; } diff --git a/core/lib/Drupal/Core/EventSubscriber/EarlyRenderingControllerWrapperSubscriber.php b/core/lib/Drupal/Core/EventSubscriber/EarlyRenderingControllerWrapperSubscriber.php index 9dbe488..5a1902c 100644 --- a/core/lib/Drupal/Core/EventSubscriber/EarlyRenderingControllerWrapperSubscriber.php +++ b/core/lib/Drupal/Core/EventSubscriber/EarlyRenderingControllerWrapperSubscriber.php @@ -14,6 +14,7 @@ use Drupal\Core\Render\BubbleableMetadata; use Drupal\Core\Render\RenderContext; use Drupal\Core\Render\RendererInterface; +use Drupal\Core\Routing\RouteMatchInterface; use Symfony\Component\EventDispatcher\EventSubscriberInterface; use Symfony\Component\HttpKernel\Event\FilterControllerEvent; use Symfony\Component\HttpKernel\KernelEvents; @@ -71,16 +72,26 @@ class EarlyRenderingControllerWrapperSubscriber implements EventSubscriberInterf protected $renderer; /** + * The current route match object. + * + * @var \Drupal\Core\Routing\RouteMatchInterface + */ + protected $routeMatch; + + /** * Constructs a new EarlyRenderingControllerWrapperSubscriber instance. * * @param \Drupal\Core\Controller\ControllerResolverInterface $controller_resolver * The controller resolver. * @param \Drupal\Core\Render\RendererInterface $renderer * The renderer. + * @param \Drupal\Core\Routing\RouteMatchInterface + * The current route match object. */ - public function __construct(ControllerResolverInterface $controller_resolver, RendererInterface $renderer) { + public function __construct(ControllerResolverInterface $controller_resolver, RendererInterface $renderer, RouteMatchInterface $route_match) { $this->controllerResolver = $controller_resolver; $this->renderer = $renderer; + $this->routeMatch = $route_match; } /** @@ -127,8 +138,9 @@ protected function wrapControllerExecutionInRenderContext($controller, array $ar // If early rendering happened, i.e. if code in the controller called // drupal_render() outside of a render context, then the bubbleable metadata - // for that is stored in the current render context. - if (!$context->isEmpty()) { + // for that is stored in the current render context. If the current route is + // not cacheable we simply discard it. + if (!$context->isEmpty() && !$this->routeMatch->getRouteObject()->getOption('no_cache')) { // If a render array is returned by the controller, merge the "lost" // bubbleable metadata. if (is_array($response)) { diff --git a/core/lib/Drupal/Core/Render/CacheableMetadataAwareUrlGenerator.php b/core/lib/Drupal/Core/Render/CacheableMetadataAwareUrlGenerator.php new file mode 100644 index 0000000..51773bf --- /dev/null +++ b/core/lib/Drupal/Core/Render/CacheableMetadataAwareUrlGenerator.php @@ -0,0 +1,133 @@ +urlGenerator = $url_generator; + $this->renderer = $renderer; + } + + /** + * {@inheritdoc} + */ + public function setContext(SymfonyRequestContext $context) { + $this->urlGenerator->setContext($context); + } + + /** + * {@inheritdoc} + */ + public function getContext() { + return $this->urlGenerator->getContext(); + } + + /** + * {@inheritdoc} + */ + public function getPathFromRoute($name, $parameters = array()) { + return $this->urlGenerator->getPathFromRoute($name, $parameters); + } + + /** + * Bubbles the cacheability metadata to the current render context. + * + * @param \Drupal\Core\GeneratedUrl $generated_url + * The generated URL whose bubbleable metadata to bubble. + */ + protected function bubble(GeneratedUrl $generated_url, array $options = []) { + // Bubbling metadata makes sense only if inside a render context. All code + // running outside controllers has no render context by default, so URLs + // used there are not supposed to affected the response cacheability. + if (empty($options[UncacheableUrl::UNCACHEABLE_OPTION]) && $this->renderer->hasRenderContext()) { + $build = []; + $generated_url->applyTo($build); + $this->renderer->render($build); + } + } + + /** + * {@inheritdoc} + */ + public function generate($name, $parameters = array(), $absolute = FALSE) { + $options['absolute'] = $absolute; + $generated_url = $this->generateFromRoute($name, $parameters, $options, TRUE); + $this->bubble($generated_url); + return $generated_url->getGeneratedUrl(); + } + + /** + * {@inheritdoc} + */ + public function generateFromRoute($name, $parameters = array(), $options = array(), $collect_cacheability_metadata = FALSE) { + $generated_url = $this->urlGenerator->generateFromRoute($name, $parameters, $options, TRUE); + if (!$collect_cacheability_metadata) { + $this->bubble($generated_url, $options); + } + return $collect_cacheability_metadata ? $generated_url : $generated_url->getGeneratedUrl(); + } + + /** + * {@inheritdoc} + */ + public function generateFromPath($path = NULL, $options = array(), $collect_cacheability_metadata = FALSE) { + $generated_url = $this->urlGenerator->generateFromPath($path, $options, TRUE); + if (!$collect_cacheability_metadata) { + $this->bubble($generated_url, $options); + } + return $collect_cacheability_metadata ? $generated_url : $generated_url->getGeneratedUrl(); + } + + /** + * {@inheritDoc} + */ + public function supports($name) { + return $this->urlGenerator->supports($name); + } + + /** + * {@inheritDoc} + */ + public function getRouteDebugMessage($name, array $parameters = array()) { + return $this->urlGenerator->getRouteDebugMessage($name, $parameters); + } + +} diff --git a/core/lib/Drupal/Core/Render/Element/FormElement.php b/core/lib/Drupal/Core/Render/Element/FormElement.php index 401c850..e4be7e1 100644 --- a/core/lib/Drupal/Core/Render/Element/FormElement.php +++ b/core/lib/Drupal/Core/Render/Element/FormElement.php @@ -7,7 +7,9 @@ namespace Drupal\Core\Render\Element; +use Drupal\Core\Cache\CacheableMetadata; use Drupal\Core\Form\FormStateInterface; +use Drupal\Core\Url; /** * Provides a base class for form element plugins. @@ -111,18 +113,24 @@ public static function validatePattern(&$element, FormStateInterface $form_state * The form element. */ public static function processAutocomplete(&$element, FormStateInterface $form_state, &$complete_form) { + $url = NULL; $access = FALSE; + if (!empty($element['#autocomplete_route_name'])) { $parameters = isset($element['#autocomplete_route_parameters']) ? $element['#autocomplete_route_parameters'] : array(); - - $path = \Drupal::urlGenerator()->generate($element['#autocomplete_route_name'], $parameters); - $access = \Drupal::service('access_manager')->checkNamedRoute($element['#autocomplete_route_name'], $parameters, \Drupal::currentUser()); + $url = Url::fromRoute($element['#autocomplete_route_name'], $parameters)->toString(TRUE); + /** @var \Drupal\Core\Access\AccessManagerInterface $access_manager */ + $access_manager = \Drupal::service('access_manager'); + $access = $access_manager->checkNamedRoute($element['#autocomplete_route_name'], $parameters, \Drupal::currentUser(), TRUE); } - if ($access) { + + if ($access->isAllowed()) { $element['#attributes']['class'][] = 'form-autocomplete'; $element['#attached']['library'][] = 'core/drupal.autocomplete'; // Provide a data attribute for the JavaScript behavior to bind to. - $element['#attributes']['data-autocomplete-path'] = $path; + $element['#attributes']['data-autocomplete-path'] = $url->getGeneratedUrl(); + CacheableMetadata::createFromObject($url)->applyTo($element); + CacheableMetadata::createFromObject($access)->applyTo($element); } return $element; diff --git a/core/lib/Drupal/Core/Render/Element/Pager.php b/core/lib/Drupal/Core/Render/Element/Pager.php index eeb0f9d..d78ea47 100644 --- a/core/lib/Drupal/Core/Render/Element/Pager.php +++ b/core/lib/Drupal/Core/Render/Element/Pager.php @@ -34,12 +34,22 @@ public function getInfo() { '#quantity' => 9, // An array of labels for the controls in the pager. '#tags' => [], + // The name of the route to be used to build pager links. By default no + // path is provided, which will make links relative to the current URL. + // This makes the page more effectively cacheable. + '#route_name' => '', ]; } /** * #pre_render callback to associate the appropriate cache context. * + * Note: the default pager theme process function template_preprocess_pager() + * also calls pager_query_add_page(), which maintains the existing query + * string. Therefore pager_query_add_page() adds the 'url.query_args' cache + * context, which causes the more specific cache context below to be optimized + * away. In other themes, however, that may not be the case. + * * @param array $pager * A renderable array of #type => pager. * diff --git a/core/lib/Drupal/Core/Render/Element/RenderElement.php b/core/lib/Drupal/Core/Render/Element/RenderElement.php index a3b362c..4554e82 100644 --- a/core/lib/Drupal/Core/Render/Element/RenderElement.php +++ b/core/lib/Drupal/Core/Render/Element/RenderElement.php @@ -7,6 +7,7 @@ namespace Drupal\Core\Render\Element; +use Drupal\Core\Cache\CacheableMetadata; use Drupal\Core\Form\FormBuilderInterface; use Drupal\Core\Form\FormStateInterface; use Drupal\Core\Plugin\PluginBase; @@ -264,7 +265,9 @@ public static function preRenderAjaxForm($element) { // Convert \Drupal\Core\Url object to string. if (isset($settings['url']) && $settings['url'] instanceof Url) { - $settings['url'] = $settings['url']->setOptions($settings['options'])->toString(); + $url = $settings['url']->setOptions($settings['options'])->toString(TRUE); + CacheableMetadata::createFromObject($url)->applyTo($element); + $settings['url'] = $url->getGeneratedUrl(); } else { $settings['url'] = NULL; diff --git a/core/lib/Drupal/Core/Render/Renderer.php b/core/lib/Drupal/Core/Render/Renderer.php index d98955d..4b55457 100644 --- a/core/lib/Drupal/Core/Render/Renderer.php +++ b/core/lib/Drupal/Core/Render/Renderer.php @@ -7,7 +7,6 @@ namespace Drupal\Core\Render; -use Drupal\Component\Utility\NestedArray; use Drupal\Component\Utility\SafeMarkup; use Drupal\Component\Utility\UrlHelper; use Drupal\Core\Access\AccessResultInterface; @@ -16,6 +15,7 @@ use Drupal\Core\Controller\ControllerResolverInterface; use Drupal\Core\Template\Attribute; use Drupal\Core\Theme\ThemeManagerInterface; +use Symfony\Component\HttpFoundation\RequestStack; /** * Turns a render array into a HTML string. @@ -58,7 +58,25 @@ class Renderer implements RendererInterface { protected $rendererConfig; /** - * The render context. + * Whether we're currently in a ::renderRoot() call. + * + * @var bool + */ + protected $isRenderingRoot = FALSE; + + /** + * The request stack. + * + * @var \Symfony\Component\HttpFoundation\RequestStack + */ + protected $requestStack; + + /** + * The render context collection. + * + * An individual global render context is tied to the current request. We then + * need to maintain a different context for each request to correctly handle + * rendering in subrequests. * * This must be static as long as some controllers rebuild the container * during a request. This causes multiple renderer instances to co-exist @@ -66,16 +84,9 @@ class Renderer implements RendererInterface { * fail to render correctly. As soon as it is guaranteed that during a request * the same container is used, it no longer needs to be static. * - * @var \Drupal\Core\Render\RenderContext|null - */ - protected static $context; - - /** - * Whether we're currently in a ::renderRoot() call. - * - * @var bool + * @var \Drupal\Core\Render\RenderContext[] */ - protected $isRenderingRoot = FALSE; + protected static $contextCollection; /** * Constructs a new Renderer. @@ -88,15 +99,23 @@ class Renderer implements RendererInterface { * The element info. * @param \Drupal\Core\Render\RenderCacheInterface $render_cache * The render cache service. + * @param \Symfony\Component\HttpFoundation\RequestStack $request_stack + * The request stack. * @param array $renderer_config * The renderer configuration array. */ - public function __construct(ControllerResolverInterface $controller_resolver, ThemeManagerInterface $theme, ElementInfoManagerInterface $element_info, RenderCacheInterface $render_cache, array $renderer_config) { + public function __construct(ControllerResolverInterface $controller_resolver, ThemeManagerInterface $theme, ElementInfoManagerInterface $element_info, RenderCacheInterface $render_cache, RequestStack $request_stack, array $renderer_config) { $this->controllerResolver = $controller_resolver; $this->theme = $theme; $this->elementInfo = $element_info; $this->renderCache = $render_cache; $this->rendererConfig = $renderer_config; + $this->requestStack = $request_stack; + + // Initialize the context collection if needed. + if (!isset(static::$contextCollection)) { + static::$contextCollection = new \SplObjectStorage(); + } } /** @@ -225,10 +244,11 @@ protected function doRender(&$elements, $is_root_call = FALSE) { return ''; } - if (!isset(static::$context)) { + $context = $this->getCurrentRenderContext(); + if (!isset($context)) { throw new \LogicException("Render context is empty, because render() was called outside of a renderRoot() or renderPlain() call. Use renderPlain()/renderRoot() or #lazy_builder/#pre_render instead."); } - static::$context->push(new BubbleableMetadata()); + $context->push(new BubbleableMetadata()); // Set the bubbleable rendering metadata that has configurable defaults, if: // - this is the root call, to ensure that the final render array definitely @@ -269,10 +289,10 @@ protected function doRender(&$elements, $is_root_call = FALSE) { } // The render cache item contains all the bubbleable rendering metadata // for the subtree. - static::$context->update($elements); + $context->update($elements); // Render cache hit, so rendering is finished, all necessary info // collected! - static::$context->bubble(); + $context->bubble(); return $elements['#markup']; } } @@ -370,9 +390,9 @@ protected function doRender(&$elements, $is_root_call = FALSE) { if (!empty($elements['#printed'])) { // The #printed element contains all the bubbleable rendering metadata for // the subtree. - static::$context->update($elements); + $context->update($elements); // #printed, so rendering is finished, all necessary info collected! - static::$context->bubble(); + $context->bubble(); return ''; } @@ -499,7 +519,7 @@ protected function doRender(&$elements, $is_root_call = FALSE) { $elements['#markup'] = $prefix . $elements['#children'] . $suffix; // We've rendered this element (and its subtree!), now update the context. - static::$context->update($elements); + $context->update($elements); // Cache the processed element if both $pre_bubbling_elements and $elements // have the metadata necessary to generate a cache ID. @@ -522,13 +542,13 @@ protected function doRender(&$elements, $is_root_call = FALSE) { if ($is_root_call) { $this->replacePlaceholders($elements); // @todo remove as part of https://www.drupal.org/node/2511330. - if (static::$context->count() !== 1) { + if ($context->count() !== 1) { throw new \LogicException('A stray drupal_render() invocation with $is_root_call = TRUE is causing bubbling of attached assets to break.'); } } // Rendering is finished, all necessary info collected! - static::$context->bubble(); + $context->bubble(); $elements['#printed'] = TRUE; $elements['#markup'] = SafeMarkup::set($elements['#markup']); @@ -538,25 +558,66 @@ protected function doRender(&$elements, $is_root_call = FALSE) { /** * {@inheritdoc} */ + public function hasRenderContext() { + return (bool) $this->getCurrentRenderContext(); + } + + /** + * {@inheritdoc} + */ public function executeInRenderContext(RenderContext $context, callable $callable) { // Store the current render context. - $current_context = static::$context; + $previous_context = $this->getCurrentRenderContext(); // Set the provided context and call the callable, it will use that context. - static::$context = $context; + $this->setCurrentRenderContext($context); $result = $callable(); // @todo Convert to an assertion in https://www.drupal.org/node/2408013 - if (static::$context->count() > 1) { + if ($context->count() > 1) { throw new \LogicException('Bubbling failed.'); } // Restore the original render context. - static::$context = $current_context; + $this->setCurrentRenderContext($previous_context); return $result; } /** + * Returns the current render context. + * + * @return \Drupal\Core\Render\RenderContext + * The current render context. + */ + protected function getCurrentRenderContext() { + try { + $request = $this->requestStack->getCurrentRequest(); + $context = static::$contextCollection[$request]; + } + catch (\UnexpectedValueException $e) { + // This can happen if we do not have a render context for the current + // request yet. + $context = NULL; + } + return $context; + } + + /** + * Sets the current render context. + * + * @param \Drupal\Core\Render\RenderContext|null $context + * The render context. This can be NULL for instance when we are restoring + * the original render context, which is in fact NULL. + * + * @return $this + */ + protected function setCurrentRenderContext(RenderContext $context = NULL) { + $request = $this->requestStack->getCurrentRequest(); + static::$contextCollection[$request] = $context; + return $this; + } + + /** * Replaces placeholders. * * Placeholders may have: diff --git a/core/lib/Drupal/Core/Render/RendererInterface.php b/core/lib/Drupal/Core/Render/RendererInterface.php index b55966f..15a355c 100644 --- a/core/lib/Drupal/Core/Render/RendererInterface.php +++ b/core/lib/Drupal/Core/Render/RendererInterface.php @@ -322,6 +322,18 @@ public function renderPlain(&$elements); public function render(&$elements, $is_root_call = FALSE); /** + * Checks whether a render context is active. + * + * This is useful only in very specific situations to determine whether the + * system is already capable of collecting bubbleable metadata. Normally it + * should not be necessary to concern about this. + * + * @return bool + * TRUE if the renderer has a render context active, FALSE otherwise. + */ + public function hasRenderContext(); + + /** * Executes a callable within a render context. * * Only for very advanced use cases. Prefer using ::renderRoot() and diff --git a/core/lib/Drupal/Core/Routing/UrlGenerator.php b/core/lib/Drupal/Core/Routing/UrlGenerator.php index bd3c1a0..d6e7ede 100644 --- a/core/lib/Drupal/Core/Routing/UrlGenerator.php +++ b/core/lib/Drupal/Core/Routing/UrlGenerator.php @@ -278,12 +278,8 @@ public function generate($name, $parameters = array(), $absolute = FALSE) { * {@inheritdoc} */ public function generateFromRoute($name, $parameters = array(), $options = array(), $collect_cacheability_metadata = FALSE) { - $generated_url = $collect_cacheability_metadata ? new GeneratedUrl() : NULL; - - $options += array('prefix' => ''); $route = $this->getRoute($name); - $name = $this->getRouteDebugMessage($name); - $this->processRoute($name, $route, $parameters, $generated_url); + $generated_url = $collect_cacheability_metadata ? new GeneratedUrl() : NULL; $query_params = []; // Symfony adds any parameters that are not path slugs as query strings. @@ -291,6 +287,23 @@ public function generateFromRoute($name, $parameters = array(), $options = array $query_params = $options['query']; } + $fragment = ''; + if (isset($options['fragment'])) { + if (($fragment = trim($options['fragment'])) != '') { + $fragment = '#' . $fragment; + } + } + + // Generate a relative URL having no path, just query string and fragment. + if ($route->getOption('_no_path')) { + $query = $query_params ? '?' . http_build_query($query_params, '', '&') : ''; + $url = $query . $fragment; + return $collect_cacheability_metadata ? $generated_url->setGeneratedUrl($url) : $url; + } + + $options += array('prefix' => ''); + $name = $this->getRouteDebugMessage($name); + $this->processRoute($name, $route, $parameters, $generated_url); $path = $this->getInternalPathFromRoute($name, $route, $parameters, $query_params); $path = $this->processPath($path, $options, $generated_url); @@ -300,13 +313,6 @@ public function generateFromRoute($name, $parameters = array(), $options = array $path = '/' . str_replace('%2F', '/', rawurlencode($prefix)) . $path; } - $fragment = ''; - if (isset($options['fragment'])) { - if (($fragment = trim($options['fragment'])) != '') { - $fragment = '#' . $fragment; - } - } - // The base_url might be rewritten from the language rewrite in domain mode. if (isset($options['base_url'])) { $base_url = $options['base_url']; @@ -328,11 +334,6 @@ public function generateFromRoute($name, $parameters = array(), $options = array $absolute = !empty($options['absolute']); if (!$absolute || !$host = $this->context->getHost()) { - - if ($route->getOption('_only_fragment')) { - return $collect_cacheability_metadata ? $generated_url->setGeneratedUrl($fragment) : $fragment; - } - $url = $base_url . $path . $fragment; return $collect_cacheability_metadata ? $generated_url->setGeneratedUrl($url) : $url; } diff --git a/core/lib/Drupal/Core/UncacheableUrl.php b/core/lib/Drupal/Core/UncacheableUrl.php new file mode 100644 index 0000000..b24bf8a --- /dev/null +++ b/core/lib/Drupal/Core/UncacheableUrl.php @@ -0,0 +1,72 @@ +setOptions($options); + } + + /** + * {@inheritdoc} + */ + public function getOptions() { + $options = parent::getOptions(); + $options[static::UNCACHEABLE_OPTION] = TRUE; + return $options; + } + + /** + * {@inheritdoc} + */ + public function getOption($name) { + return $name == static::UNCACHEABLE_OPTION ? TRUE : parent::getOption($name); + } + + /** + * {@inheritdoc} + */ + public function setOptions($options) { + $options[static::UNCACHEABLE_OPTION] = TRUE; + return parent::setOptions($options); + } + + /** + * {@inheritdoc} + */ + public function setOption($name, $value) { + return parent::setOption($name, $name == static::UNCACHEABLE_OPTION ? TRUE : $value); + } + + /** + * {@inheritdoc} + */ + protected function urlGenerator() { + if (!$this->urlGenerator) { + $this->urlGenerator = \Drupal::service('url_generator.uncacheable'); + } + return $this->urlGenerator; + } + +} diff --git a/core/modules/comment/src/Controller/CommentController.php b/core/modules/comment/src/Controller/CommentController.php index add5333..7d79e74 100644 --- a/core/modules/comment/src/Controller/CommentController.php +++ b/core/modules/comment/src/Controller/CommentController.php @@ -129,7 +129,8 @@ public function commentPermalink(Request $request, CommentInterface $comment) { // Find the current display page for this comment. $page = $this->entityManager()->getStorage('comment')->getDisplayOrdinal($comment, $field_definition->getSetting('default_mode'), $field_definition->getSetting('per_page')); // @todo: Cleaner sub request handling. - $redirect_request = Request::create($entity->url(), 'GET', $request->query->all(), $request->cookies->all(), array(), $request->server->all()); + $url = $entity->urlInfo()->toString(TRUE)->getGeneratedUrl(); + $redirect_request = Request::create($url, 'GET', $request->query->all(), $request->cookies->all(), array(), $request->server->all()); $redirect_request->query->set('page', $page); // Carry over the session to the subrequest. if ($session = $request->getSession()) { diff --git a/core/modules/comment/src/Tests/CommentRssTest.php b/core/modules/comment/src/Tests/CommentRssTest.php index b82da48..7134a29 100644 --- a/core/modules/comment/src/Tests/CommentRssTest.php +++ b/core/modules/comment/src/Tests/CommentRssTest.php @@ -58,6 +58,7 @@ function testCommentRss() { $this->assertCacheContexts([ 'languages:language_interface', 'theme', + 'url.site', 'user.node_grants:view', 'user.permissions', 'timezone', diff --git a/core/modules/filter/src/Element/TextFormat.php b/core/modules/filter/src/Element/TextFormat.php index aaff8b4..bb83dde 100644 --- a/core/modules/filter/src/Element/TextFormat.php +++ b/core/modules/filter/src/Element/TextFormat.php @@ -7,9 +7,11 @@ namespace Drupal\filter\Element; +use Drupal\Core\Cache\CacheableMetadata; use Drupal\Core\Form\FormStateInterface; use Drupal\Core\Render\Element\RenderElement; use Drupal\Core\Render\Element; +use Drupal\Core\UncacheableUrl; use Drupal\Core\Url; /** @@ -178,12 +180,14 @@ public static function processFormat(&$element, FormStateInterface $form_state, '#parents' => array_merge($element['#parents'], array('format')), ); + $link = \Drupal::l(t('About text formats'), Url::fromRoute('filter.tips_all', [], ['attributes' => ['target' => '_blank']]), TRUE); $element['format']['help'] = array( '#type' => 'container', '#attributes' => array('class' => array('filter-help')), - '#markup' => \Drupal::l(t('About text formats'), new Url('filter.tips_all', array(), array('attributes' => array('target' => '_blank')))), + '#markup' => $link->getGeneratedLink(), '#weight' => 0, ); + CacheableMetadata::createFromObject($link)->applyTo($element['format']['help']); $all_formats = filter_formats(); $format_exists = isset($all_formats[$element['#format']]); diff --git a/core/modules/filter/src/FilterPermissions.php b/core/modules/filter/src/FilterPermissions.php index ef4e694..756f8d9 100644 --- a/core/modules/filter/src/FilterPermissions.php +++ b/core/modules/filter/src/FilterPermissions.php @@ -11,6 +11,7 @@ use Drupal\Core\DependencyInjection\ContainerInjectionInterface; use Drupal\Core\Entity\EntityManagerInterface; use Drupal\Core\StringTranslation\StringTranslationTrait; +use Drupal\Core\UncacheableUrl; use Symfony\Component\DependencyInjection\ContainerInterface; /** @@ -58,8 +59,10 @@ public function permissions() { uasort($formats, 'Drupal\Core\Config\Entity\ConfigEntityBase::sort'); foreach ($formats as $format) { if ($permission = $format->getPermissionName()) { + $info = $format->urlInfo(); + $url = UncacheableUrl::fromRoute($info->getRouteName(), $info->getRouteParameters())->toString(); $permissions[$permission] = [ - 'title' => $this->t('Use the @label text format', ['@url' => $format->url(), '@label' => $format->label()]), + 'title' => $this->t('Use the @label text format', ['@url' => $url, '@label' => $format->label()]), 'description' => SafeMarkup::placeholder($this->t('Warning: This permission may have security implications depending on how the text format is configured.')), ]; } diff --git a/core/modules/hal/src/Normalizer/ContentEntityNormalizer.php b/core/modules/hal/src/Normalizer/ContentEntityNormalizer.php index a4761b3..2303c73 100644 --- a/core/modules/hal/src/Normalizer/ContentEntityNormalizer.php +++ b/core/modules/hal/src/Normalizer/ContentEntityNormalizer.php @@ -11,6 +11,7 @@ use Drupal\Core\Entity\EntityInterface; use Drupal\Core\Entity\EntityManagerInterface; use Drupal\Core\Extension\ModuleHandlerInterface; +use Drupal\Core\UncacheableUrl; use Drupal\rest\LinkManager\LinkManagerInterface; use Symfony\Component\Serializer\Exception\UnexpectedValueException; @@ -207,8 +208,8 @@ protected function getEntityUri(EntityInterface $entity) { if ($entity->isNew() || !$entity->hasLinkTemplate('canonical')) { return $entity->url('canonical', []); } - $url = $entity->urlInfo('canonical', ['absolute' => TRUE]); - return $url->setRouteParameter('_format', 'hal_json')->toString(); + $url = $entity->urlInfo('canonical')->setRouteParameter('_format', 'hal_json'); + return UncacheableUrl::fromRoute($url->getRouteName(), $url->getRouteParameters(), ['absolute' => TRUE])->toString(); } /** diff --git a/core/modules/language/src/Tests/LanguageUrlRewritingTest.php b/core/modules/language/src/Tests/LanguageUrlRewritingTest.php index 81602d1..f590861 100644 --- a/core/modules/language/src/Tests/LanguageUrlRewritingTest.php +++ b/core/modules/language/src/Tests/LanguageUrlRewritingTest.php @@ -139,7 +139,7 @@ function testDomainNameNegotiationPort() { // Create an absolute French link. $language = \Drupal::languageManager()->getLanguage('fr'); - $url = Url::fromRoute('', [], [ + $url = Url::fromRoute('', [], [ 'absolute' => TRUE, 'language' => $language, ])->toString(); @@ -149,7 +149,7 @@ function testDomainNameNegotiationPort() { $this->assertEqual($url, $expected, 'The right port is used.'); // If we set the port explicitly, it should not be overridden. - $url = Url::fromRoute('', [], [ + $url = Url::fromRoute('', [], [ 'absolute' => TRUE, 'language' => $language, 'base_url' => $request->getBaseUrl() . ':90', diff --git a/core/modules/node/src/Tests/Views/FrontPageTest.php b/core/modules/node/src/Tests/Views/FrontPageTest.php index b3ab9c9..ee82a80 100644 --- a/core/modules/node/src/Tests/Views/FrontPageTest.php +++ b/core/modules/node/src/Tests/Views/FrontPageTest.php @@ -191,7 +191,7 @@ public function testAdminFrontPage() { */ public function testCacheTagsWithCachePluginNone() { $this->enablePageCaching(); - $this->assertFrontPageViewCacheTags(FALSE); + $this->doTestFrontPageViewCacheTags(FALSE); } /** @@ -207,7 +207,7 @@ public function testCacheTagsWithCachePluginTag() { ]); $view->save(); - $this->assertFrontPageViewCacheTags(TRUE); + $this->doTestFrontPageViewCacheTags(TRUE); } /** @@ -227,7 +227,7 @@ public function testCacheTagsWithCachePluginTime() { ]); $view->save(); - $this->assertFrontPageViewCacheTags(TRUE); + $this->doTestFrontPageViewCacheTags(TRUE); } /** @@ -236,7 +236,7 @@ public function testCacheTagsWithCachePluginTime() { * @param bool $do_assert_views_caches * Whether to check Views' result & output caches. */ - protected function assertFrontPageViewCacheTags($do_assert_views_caches) { + protected function doTestFrontPageViewCacheTags($do_assert_views_caches) { $view = Views::getView('frontpage'); $view->setDisplay('page_1'); @@ -248,7 +248,9 @@ protected function assertFrontPageViewCacheTags($do_assert_views_caches) { 'user.permissions', // Default cache contexts of the renderer. 'theme', - 'url.query_args.pagers:0', + 'url.query_args', + // Attached feed. + 'url.site', ]; // Test before there are any nodes. diff --git a/core/modules/page_cache/src/Tests/PageCacheTagsIntegrationTest.php b/core/modules/page_cache/src/Tests/PageCacheTagsIntegrationTest.php index ebf0867..c471cea 100644 --- a/core/modules/page_cache/src/Tests/PageCacheTagsIntegrationTest.php +++ b/core/modules/page_cache/src/Tests/PageCacheTagsIntegrationTest.php @@ -71,12 +71,7 @@ function testPageCacheTags() { $cache_contexts = [ 'languages:' . LanguageInterface::TYPE_INTERFACE, - 'route.menu_active_trails:account', - 'route.menu_active_trails:footer', - 'route.menu_active_trails:main', - 'route.menu_active_trails:tools', - // The user login block access is not visible on certain routes. - 'route.name', + 'route', 'theme', 'timezone', 'user.permissions', diff --git a/core/modules/rest/src/Plugin/rest/resource/EntityResource.php b/core/modules/rest/src/Plugin/rest/resource/EntityResource.php index 8631086..5972833 100644 --- a/core/modules/rest/src/Plugin/rest/resource/EntityResource.php +++ b/core/modules/rest/src/Plugin/rest/resource/EntityResource.php @@ -9,6 +9,7 @@ use Drupal\Core\Entity\EntityInterface; use Drupal\Core\Entity\EntityStorageException; +use Drupal\Core\UncacheableUrl; use Drupal\rest\Plugin\ResourceBase; use Drupal\rest\ResourceResponse; use Drupal\Component\Utility\SafeMarkup; @@ -109,7 +110,9 @@ public function post(EntityInterface $entity = NULL) { $this->logger->notice('Created entity %type with ID %id.', array('%type' => $entity->getEntityTypeId(), '%id' => $entity->id())); // 201 Created responses have an empty body. - return new ResourceResponse(NULL, 201, array('Location' => $entity->url('canonical', ['absolute' => TRUE]))); + $info = $entity->urlInfo('canonical'); + $url = UncacheableUrl::fromRoute($info->getRouteName(), $info->getRouteParameters(), ['absolute' => TRUE])->toString(); + return new ResourceResponse(NULL, 201, ['Location' => $url]); } catch (EntityStorageException $e) { throw new HttpException(500, 'Internal Server Error', $e); diff --git a/core/modules/shortcut/shortcut.module b/core/modules/shortcut/shortcut.module index 975a309..be83839 100644 --- a/core/modules/shortcut/shortcut.module +++ b/core/modules/shortcut/shortcut.module @@ -313,7 +313,6 @@ function shortcut_preprocess_page(&$variables) { 'link' => $link, 'name' => $variables['title'], ); - $query += \Drupal::destination()->getAsArray(); $shortcut_set = shortcut_current_displayed_set(); @@ -341,6 +340,7 @@ function shortcut_preprocess_page(&$variables) { } if (theme_get_setting('third_party_settings.shortcut.module_link')) { + $query += \Drupal::destination()->getAsArray(); $variables['title_suffix']['add_or_remove_shortcut'] = array( '#attached' => array( 'library' => array( diff --git a/core/modules/shortcut/src/Tests/ShortcutTranslationUITest.php b/core/modules/shortcut/src/Tests/ShortcutTranslationUITest.php index e324d80..9a9113b 100644 --- a/core/modules/shortcut/src/Tests/ShortcutTranslationUITest.php +++ b/core/modules/shortcut/src/Tests/ShortcutTranslationUITest.php @@ -21,7 +21,7 @@ class ShortcutTranslationUITest extends ContentTranslationUITestBase { /** * {inheritdoc} */ - protected $defaultCacheContexts = ['languages:language_interface', 'theme', 'user']; + protected $defaultCacheContexts = ['languages:language_interface', 'theme', 'user', 'url.site']; /** * Modules to enable. diff --git a/core/modules/simpletest/src/KernelTestBase.php b/core/modules/simpletest/src/KernelTestBase.php index 481b031..e15eea6 100644 --- a/core/modules/simpletest/src/KernelTestBase.php +++ b/core/modules/simpletest/src/KernelTestBase.php @@ -207,6 +207,10 @@ protected function setUp() { $this->container = $this->kernel->getContainer(); $this->container->get('request_stack')->push($request); + // Make sure we use the uncacheable URL generator in tests, since most of + // the URL generations will happen outside of a render context. + $this->container->set('url_generator', $this->container->get('url_generator.uncacheable')); + // Re-inject extension file listings into state, unless the key/value // service was overridden (in which case its storage does not exist yet). if ($this->container->get('keyvalue') instanceof KeyValueMemoryFactory) { diff --git a/core/modules/simpletest/src/WebTestBase.php b/core/modules/simpletest/src/WebTestBase.php index e17d703..61a9adb 100644 --- a/core/modules/simpletest/src/WebTestBase.php +++ b/core/modules/simpletest/src/WebTestBase.php @@ -32,6 +32,7 @@ use Drupal\node\Entity\NodeType; use Symfony\Component\DependencyInjection\ContainerInterface; use Symfony\Component\HttpFoundation\Request; +use Zend\Diactoros\Uri; /** * Test case for typical Drupal tests. @@ -954,7 +955,11 @@ protected function initKernel(Request $request) { $this->kernel->prepareLegacyRequest($request); // Force the container to be built from scratch instead of loaded from the // disk. This forces us to not accidentally load the parent site. - return $this->kernel->rebuildContainer(); + $container = $this->kernel->rebuildContainer(); + // Make sure we use the uncacheable URL generator in tests, since most of + // the URL generations will happen outside of a render context. + $container->set('url_generator', $container->get('url_generator.uncacheable')); + return $container; } /** @@ -2355,16 +2360,35 @@ protected function clickLink($label, $index = 0) { /** * Takes a path and returns an absolute path. * - * @param $path - * A path from the internal browser content. + * This method is implemented in the way how browsers work, see + * https://url.spec.whatwg.org/#relative-state for more information about the + * possible cases. * - * @return + * @param string $path + * A path from the internal browser content.T + * + * @return string * The $path with $base_url prepended, if necessary. */ protected function getAbsoluteUrl($path) { global $base_url, $base_path; $parts = parse_url($path); + + // In case the $path has a host, it is already an absolute URL and we are + // done. + if (!empty($parts['host'])) { + return $path; + } + + // In case the $path contains just a query, we turn it into an absolute URL + // with the same scheme, host and path, see + // https://url.spec.whatwg.org/#relative-state. + if (array_keys($parts) === ['query']) { + $current_uri = new Uri($this->getUrl()); + return (string) $current_uri->withQuery($parts['query']); + } + if (empty($parts['host'])) { // Ensure that we have a string (and no xpath object). $path = (string) $path; diff --git a/core/modules/simpletest/tests/src/Unit/WebTestBaseTest.php b/core/modules/simpletest/tests/src/Unit/WebTestBaseTest.php index 27598d4..f86b85e 100644 --- a/core/modules/simpletest/tests/src/Unit/WebTestBaseTest.php +++ b/core/modules/simpletest/tests/src/Unit/WebTestBaseTest.php @@ -198,4 +198,41 @@ public function testClickLink($expected, $label, $index, $xpath_data) { $this->assertSame($expected, $clicklink_method->invoke($web_test, $label, $index)); } + /** + * @dataProvider providerTestGetAbsoluteUrl + */ + public function testGetAbsoluteUrl($href, $expected_absolute_path) { + $web_test = $this->getMockBuilder('Drupal\simpletest\WebTestBase') + ->disableOriginalConstructor() + ->setMethods(['getUrl']) + ->getMock(); + + $web_test->expects($this->any()) + ->method('getUrl') + ->willReturn('http://example.com/drupal/current-path?foo=baz'); + + $GLOBALS['base_url'] = 'http://example.com'; + $GLOBALS['base_path'] = 'drupal'; + + $get_absolute_url_method = new \ReflectionMethod($web_test, 'getAbsoluteUrl'); + $get_absolute_url_method->setAccessible(TRUE); + + $this->assertSame($expected_absolute_path, $get_absolute_url_method->invoke($web_test, $href)); + } + + /** + * Provides test data for testGetAbsoluteUrl. + * + * @return array + */ + public function providerTestGetAbsoluteUrl() { + $data = []; + $data['host'] = ['http://example.com/drupal/test-example', 'http://example.com/drupal/test-example']; + $data['path'] = ['/drupal/test-example', 'http://example.com/drupal/test-example']; + $data['path-with-query'] = ['/drupal/test-example?foo=bar', 'http://example.com/drupal/test-example?foo=bar']; + $data['just-query'] = ['?foo=bar', 'http://example.com/drupal/current-path?foo=bar']; + + return $data; + } + } diff --git a/core/modules/system/src/Tests/Pager/PagerTest.php b/core/modules/system/src/Tests/Pager/PagerTest.php index 87730ff..dc643a5 100644 --- a/core/modules/system/src/Tests/Pager/PagerTest.php +++ b/core/modules/system/src/Tests/Pager/PagerTest.php @@ -65,7 +65,7 @@ function testActiveClass() { $elements = $this->xpath('//li[contains(@class, :class)]/a', array(':class' => 'pager__item--last')); preg_match('@page=(\d+)@', $elements[0]['href'], $matches); $current_page = (int) $matches[1]; - $this->drupalGet($GLOBALS['base_root'] . $elements[0]['href'], array('external' => TRUE)); + $this->drupalGet($GLOBALS['base_root'] . parse_url($this->getUrl())['path'] . $elements[0]['href'], array('external' => TRUE)); $this->assertPagerItems($current_page); } @@ -77,18 +77,23 @@ protected function testPagerQueryParametersAndCacheContext() { $this->drupalGet('pager-test/query-parameters'); $this->assertText(t('Pager calls: 0'), 'Initial call to pager shows 0 calls.'); $this->assertText('pager.0.0'); + $this->assertCacheContext('url.query_args'); // Go to last page, the count of pager calls need to go to 1. $elements = $this->xpath('//li[contains(@class, :class)]/a', array(':class' => 'pager__item--last')); - $this->drupalGet($GLOBALS['base_root'] . $elements[0]['href'], array('external' => TRUE)); + $this->drupalGet($this->getAbsoluteUrl($elements[0]['href'])); + $this->drupalGet($GLOBALS['base_root'] . parse_url($this->getUrl())['path'] . $elements[0]['href'], array('external' => TRUE)); $this->assertText(t('Pager calls: 1'), 'First link call to pager shows 1 calls.'); $this->assertText('pager.0.60'); + $this->assertCacheContext('url.query_args'); // Go back to first page, the count of pager calls need to go to 2. $elements = $this->xpath('//li[contains(@class, :class)]/a', array(':class' => 'pager__item--first')); - $this->drupalGet($GLOBALS['base_root'] . $elements[0]['href'], array('external' => TRUE)); + $this->drupalGet($this->getAbsoluteUrl($elements[0]['href'])); + $this->drupalGet($GLOBALS['base_root'] . parse_url($this->getUrl())['path'] . $elements[0]['href'], array('external' => TRUE)); $this->assertText(t('Pager calls: 2'), 'Second link call to pager shows 2 calls.'); $this->assertText('pager.0.0'); + $this->assertCacheContext('url.query_args'); } /** diff --git a/core/modules/system/system.module b/core/modules/system/system.module index 732e27b..4b40a33 100644 --- a/core/modules/system/system.module +++ b/core/modules/system/system.module @@ -658,7 +658,7 @@ function system_js_settings_alter(&$settings, AttachedAssetsInterface $assets) { $pathPrefix = ''; $current_query = $request->query->all(); - Url::fromRoute('', [], array('script' => &$scriptPath, 'prefix' => &$pathPrefix))->toString(); + Url::fromRoute('', [], array('script' => &$scriptPath, 'prefix' => &$pathPrefix))->toString(TRUE); $current_path = \Drupal::routeMatch()->getRouteName() ? Url::fromRouteMatch(\Drupal::routeMatch())->getInternalPath() : ''; $current_path_is_admin = \Drupal::service('router.admin_context')->isAdminRoute(); $path_settings = [ diff --git a/core/modules/system/system.routing.yml b/core/modules/system/system.routing.yml index 12e7d9a..97ef8fc 100644 --- a/core/modules/system/system.routing.yml +++ b/core/modules/system/system.routing.yml @@ -394,7 +394,7 @@ system.theme_settings_theme: '': path: '' options: - _only_fragment: TRUE + _no_path: TRUE requirements: _access: 'TRUE' @@ -446,6 +446,7 @@ system.batch_page.html: _format: 'html' options: _admin_route: TRUE + no_cache: TRUE system.batch_page.json: path: '/batch' @@ -456,6 +457,7 @@ system.batch_page.json: _format: 'json' options: _admin_route: TRUE + no_cache: TRUE system.db_update: path: '/update.php/{op}' @@ -465,6 +467,8 @@ system.db_update: op: 'info' requirements: _access_system_update: 'TRUE' + options: + no_cache: TRUE system.admin_content: path: '/admin/content' diff --git a/core/modules/views/src/Controller/ViewAjaxController.php b/core/modules/views/src/Controller/ViewAjaxController.php index 83a203b..01e69ef 100644 --- a/core/modules/views/src/Controller/ViewAjaxController.php +++ b/core/modules/views/src/Controller/ViewAjaxController.php @@ -14,6 +14,8 @@ use Drupal\Core\Entity\EntityStorageInterface; use Drupal\Core\EventSubscriber\AjaxResponseSubscriber; use Drupal\Core\Path\CurrentPathStack; +use Drupal\Core\Render\BubbleableMetadata; +use Drupal\Core\Render\RenderContext; use Drupal\Core\Render\RendererInterface; use Drupal\Core\Routing\RedirectDestinationInterface; use Drupal\views\Ajax\ScrollTopCommand; @@ -174,9 +176,18 @@ public function ajaxView(Request $request) { // Reuse the same DOM id so it matches that in drupalSettings. $view->dom_id = $dom_id; - if ($preview = $view->preview($display_id, $args)) { - $response->addCommand(new ReplaceCommand(".js-view-dom-id-$dom_id", $preview)); + $context = new RenderContext(); + $preview = $this->renderer->executeInRenderContext($context, function() use ($view, $display_id, $args) { + return $view->preview($display_id, $args); + }); + if (!$context->isEmpty()) { + $bubbleable_metadata = $context->pop(); + BubbleableMetadata::createFromRenderArray($preview) + ->merge($bubbleable_metadata) + ->applyTo($preview); } + $response->addCommand(new ReplaceCommand(".js-view-dom-id-$dom_id", $preview)); + return $response; } else { diff --git a/core/modules/views/src/Plugin/views/display/DisplayPluginBase.php b/core/modules/views/src/Plugin/views/display/DisplayPluginBase.php index 3245bd2..b6ad130 100644 --- a/core/modules/views/src/Plugin/views/display/DisplayPluginBase.php +++ b/core/modules/views/src/Plugin/views/display/DisplayPluginBase.php @@ -18,6 +18,7 @@ use Drupal\Core\Plugin\PluginDependencyTrait; use Drupal\Core\Session\AccountInterface; use Drupal\Core\Theme\Registry; +use Drupal\Core\UncacheableUrl; use Drupal\Core\Url; use Drupal\views\Form\ViewsForm; use Drupal\views\Plugin\CacheablePluginInterface; @@ -1031,7 +1032,7 @@ public function optionLink($text, $section, $class = '', $title = '') { $title = $text; } - return \Drupal::l($text, new Url('views_ui.form_display', array( + return \Drupal::l($text, new UncacheableUrl('views_ui.form_display', array( 'js' => 'nojs', 'view' => $this->view->storage->id(), 'display_id' => $this->display['id'], diff --git a/core/modules/views/src/Plugin/views/pager/Full.php b/core/modules/views/src/Plugin/views/pager/Full.php index da1a0ec..a0af225 100644 --- a/core/modules/views/src/Plugin/views/pager/Full.php +++ b/core/modules/views/src/Plugin/views/pager/Full.php @@ -96,6 +96,7 @@ public function render($input) { '#element' => $this->options['id'], '#parameters' => $input, '#quantity' => $this->options['quantity'], + '#route_name' => !empty($this->view->live_preview) ? '' : '', ); } diff --git a/core/modules/views/src/Plugin/views/pager/Mini.php b/core/modules/views/src/Plugin/views/pager/Mini.php index 6547242..72f5d1c 100644 --- a/core/modules/views/src/Plugin/views/pager/Mini.php +++ b/core/modules/views/src/Plugin/views/pager/Mini.php @@ -103,6 +103,7 @@ public function render($input) { '#tags' => $tags, '#element' => $this->options['id'], '#parameters' => $input, + '#route_name' => !empty($this->view->live_preview) ? '' : '', ); } diff --git a/core/modules/views/src/Plugin/views/pager/SqlBase.php b/core/modules/views/src/Plugin/views/pager/SqlBase.php index b2d7389..da89224 100644 --- a/core/modules/views/src/Plugin/views/pager/SqlBase.php +++ b/core/modules/views/src/Plugin/views/pager/SqlBase.php @@ -382,14 +382,9 @@ public function isCacheable() { * {@inheritdoc} */ public function getCacheContexts() { - $contexts = ['url.query_args.pagers:' . $this->options['id']]; - if ($this->options['expose']['items_per_page']) { - $contexts[] = 'url.query_args:items_per_page'; - } - if ($this->options['expose']['offset']) { - $contexts[] = 'url.query_args:offset'; - } - return $contexts; + // The rendered link needs to play well with any other query parameter used + // on the page, like pager and exposed filter. + return ['url.query_args']; } } diff --git a/core/modules/views/src/Plugin/views/style/Table.php b/core/modules/views/src/Plugin/views/style/Table.php index 4efeda2..8a594cf 100644 --- a/core/modules/views/src/Plugin/views/style/Table.php +++ b/core/modules/views/src/Plugin/views/style/Table.php @@ -444,8 +444,9 @@ public function getCacheContexts() { foreach ($this->options['info'] as $field_id => $info) { if (!empty($info['sortable'])) { - $contexts[] = 'url.query_args:order'; - $contexts[] = 'url.query_args:sort'; + // The rendered link needs to play well with any other query parameter + // used on the page, like pager and exposed filter. + $contexts[] = 'url.query_args'; break; } } diff --git a/core/modules/views/src/Tests/GlossaryTest.php b/core/modules/views/src/Tests/GlossaryTest.php index 7d55f85..72854e9 100644 --- a/core/modules/views/src/Tests/GlossaryTest.php +++ b/core/modules/views/src/Tests/GlossaryTest.php @@ -71,17 +71,29 @@ public function testGlossaryView() { $url = Url::fromRoute('view.glossary.page_1'); // Verify cache tags. - $this->assertPageCacheContextsAndTags($url, ['languages:' . LanguageInterface::TYPE_CONTENT, 'languages:' . LanguageInterface::TYPE_INTERFACE, 'theme', 'url', 'user.node_grants:view', 'user.permissions'], [ - 'config:views.view.glossary', - 'node:' . $nodes_by_char['a'][0]->id(), 'node:' . $nodes_by_char['a'][1]->id(), 'node:' . $nodes_by_char['a'][2]->id(), - 'node_list', - 'user:0', - 'user_list', - 'rendered', - // FinishResponseSubscriber adds this cache tag to responses that have the - // 'user.permissions' cache context for anonymous users. - 'config:user.role.anonymous', - ]); + $this->assertPageCacheContextsAndTags( + $url, + [ + 'languages:' . LanguageInterface::TYPE_CONTENT, + 'languages:' . LanguageInterface::TYPE_INTERFACE, + 'theme', + 'url', + 'user.node_grants:view', + 'user.permissions', + 'route', + ], + [ + 'config:views.view.glossary', + 'node:' . $nodes_by_char['a'][0]->id(), 'node:' . $nodes_by_char['a'][1]->id(), 'node:' . $nodes_by_char['a'][2]->id(), + 'node_list', + 'user:0', + 'user_list', + 'rendered', + // FinishResponseSubscriber adds this cache tag to responses that have the + // 'user.permissions' cache context for anonymous users. + 'config:user.role.anonymous', + ] + ); // Check the actual page response. $this->drupalGet($url); diff --git a/core/modules/views/src/Tests/Handler/FieldWebTest.php b/core/modules/views/src/Tests/Handler/FieldWebTest.php index 39e1eb2..3ffec6b 100644 --- a/core/modules/views/src/Tests/Handler/FieldWebTest.php +++ b/core/modules/views/src/Tests/Handler/FieldWebTest.php @@ -68,23 +68,21 @@ public function testClickSorting() { $this->assertResponse(200); // Only the id and name should be click sortable, but not the name. - $this->assertLinkByHref(\Drupal::url('view.test_click_sort.page_1', [], ['query' => ['order' => 'id', 'sort' => 'asc']])); - $this->assertLinkByHref(\Drupal::url('view.test_click_sort.page_1', [], ['query' => ['order' => 'name', 'sort' => 'desc']])); - $this->assertNoLinkByHref(\Drupal::url('view.test_click_sort.page_1', [], ['query' => ['order' => 'created']])); + $this->assertLinkByHref(\Drupal::url('', [], ['query' => ['order' => 'id', 'sort' => 'asc']])); + $this->assertLinkByHref(\Drupal::url('', [], ['query' => ['order' => 'name', 'sort' => 'desc']])); + $this->assertNoLinkByHref(\Drupal::url('', [], ['query' => ['order' => 'created']])); // Check that the view returns the click sorting cache contexts. $expected_contexts = [ 'languages:language_interface', 'theme', - 'url.query_args.pagers:0', - 'url.query_args:order', - 'url.query_args:sort', + 'url.query_args', ]; $this->assertCacheContexts($expected_contexts); // Clicking a click sort should change the order. $this->clickLink(t('ID')); - $this->assertLinkByHref(\Drupal::url('view.test_click_sort.page_1', [], ['query' => ['order' => 'id', 'sort' => 'desc']])); + $this->assertLinkByHref(\Drupal::url('', [], ['query' => ['order' => 'id', 'sort' => 'desc']])); // Check that the output has the expected order (asc). $ids = $this->clickSortLoadIdsFromOutput(); $this->assertEqual($ids, range(1, 5)); diff --git a/core/modules/views/src/Tests/Plugin/ExposedFormTest.php b/core/modules/views/src/Tests/Plugin/ExposedFormTest.php index ffc9bbe..1427c5d 100644 --- a/core/modules/views/src/Tests/Plugin/ExposedFormTest.php +++ b/core/modules/views/src/Tests/Plugin/ExposedFormTest.php @@ -206,11 +206,7 @@ public function testExposedSortAndItemsPerPage() { 'languages:language_interface', 'entity_test_view_grants', 'theme', - 'url.query_args.pagers:0', - 'url.query_args:items_per_page', - 'url.query_args:offset', - 'url.query_args:sort_order', - 'url.query_args:sort_by', + 'url.query_args', 'languages:language_content' ]; diff --git a/core/modules/views/src/Tests/Plugin/PagerTest.php b/core/modules/views/src/Tests/Plugin/PagerTest.php index 4a75fdc..c1c6c97 100644 --- a/core/modules/views/src/Tests/Plugin/PagerTest.php +++ b/core/modules/views/src/Tests/Plugin/PagerTest.php @@ -261,7 +261,7 @@ public function testNormalPager() { // Test pager cache contexts. $this->drupalGet('test_pager_full'); - $this->assertCacheContexts(['languages:language_interface', 'theme', 'timezone', 'url.query_args.pagers:0', 'user.node_grants:view']); + $this->assertCacheContexts(['languages:language_interface', 'theme', 'timezone', 'url.query_args', 'user.node_grants:view']); } /** diff --git a/core/modules/views/src/Tests/RenderCacheIntegrationTest.php b/core/modules/views/src/Tests/RenderCacheIntegrationTest.php index b1235b1..de79039 100644 --- a/core/modules/views/src/Tests/RenderCacheIntegrationTest.php +++ b/core/modules/views/src/Tests/RenderCacheIntegrationTest.php @@ -292,7 +292,7 @@ public function testViewAddCacheMetadata() { $view = View::load('test_display'); $view->save(); - $this->assertEqual(['languages:' . LanguageInterface::TYPE_CONTENT, 'languages:' . LanguageInterface::TYPE_INTERFACE, 'url.query_args.pagers:0', 'user.node_grants:view', 'user.permissions'], $view->getDisplay('default')['cache_metadata']['contexts']); + $this->assertEqual(['languages:' . LanguageInterface::TYPE_CONTENT, 'languages:' . LanguageInterface::TYPE_INTERFACE, 'url.query_args', 'user.node_grants:view', 'user.permissions'], $view->getDisplay('default')['cache_metadata']['contexts']); } } diff --git a/core/modules/views/tests/src/Unit/Controller/ViewAjaxControllerTest.php b/core/modules/views/tests/src/Unit/Controller/ViewAjaxControllerTest.php index dc5a898..f054b91 100644 --- a/core/modules/views/tests/src/Unit/Controller/ViewAjaxControllerTest.php +++ b/core/modules/views/tests/src/Unit/Controller/ViewAjaxControllerTest.php @@ -7,11 +7,13 @@ namespace Drupal\Tests\views\Unit\Controller { +use Drupal\Core\Render\RenderContext; use Drupal\Tests\UnitTestCase; use Drupal\views\Ajax\ViewAjaxResponse; use Drupal\views\Controller\ViewAjaxController; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\DependencyInjection\ContainerBuilder; +use Symfony\Component\HttpFoundation\RequestStack; /** * @coversDefaultClass \Drupal\views\Controller\ViewAjaxController @@ -76,6 +78,11 @@ protected function setUp() { $elements['#attached'] = []; return isset($elements['#markup']) ? $elements['#markup'] : ''; })); + $this->renderer->expects($this->any()) + ->method('executeInRenderContext') + ->willReturnCallback(function (RenderContext $context, callable $callable) { + return $callable(); + }); $this->currentPath = $this->getMockBuilder('Drupal\Core\Path\CurrentPathStack') ->disableOriginalConstructor() ->getMock(); @@ -83,8 +90,23 @@ protected function setUp() { $this->viewAjaxController = new ViewAjaxController($this->viewStorage, $this->executableFactory, $this->renderer, $this->currentPath, $this->redirectDestination); + $request_stack = new RequestStack(); + $request_stack->push(new Request()); + $args = [ + $this->getMock('\Drupal\Core\Controller\ControllerResolverInterface'), + $this->getMock('\Drupal\Core\Theme\ThemeManagerInterface'), + $this->getMock('\Drupal\Core\Render\ElementInfoManagerInterface'), + $this->getMock('\Drupal\Core\Render\RenderCacheInterface'), + $request_stack, + [ + 'required_cache_contexts' => [ + 'languages:language_interface', + 'theme', + ], + ], + ]; $this->renderer = $this->getMockBuilder('Drupal\Core\Render\Renderer') - ->disableOriginalConstructor() + ->setConstructorArgs($args) ->setMethods(NULL) ->getMock(); $container = new ContainerBuilder(); diff --git a/core/modules/views/views.theme.inc b/core/modules/views/views.theme.inc index fbe962e..3ae454b 100644 --- a/core/modules/views/views.theme.inc +++ b/core/modules/views/views.theme.inc @@ -494,7 +494,7 @@ function template_preprocess_views_view_table(&$variables) { 'attributes' => array('title' => $title), 'query' => $query, ); - $variables['header'][$field]['content'] = \Drupal::l($label, new Url('', [], $link_options)); + $variables['header'][$field]['content'] = \Drupal::l($label, new Url('', [], $link_options)); } $variables['header'][$field]['default_classes'] = $fields[$field]->options['element_default_classes']; diff --git a/core/modules/views_ui/admin.inc b/core/modules/views_ui/admin.inc index 78be0fc..437509e 100644 --- a/core/modules/views_ui/admin.inc +++ b/core/modules/views_ui/admin.inc @@ -8,6 +8,7 @@ use Drupal\Component\Utility\NestedArray; use Drupal\Component\Utility\Tags; use Drupal\Core\Form\FormStateInterface; +use Drupal\Core\UncacheableUrl; use Drupal\Core\Url; use Drupal\views\ViewExecutable; use Drupal\views\Views; @@ -286,7 +287,7 @@ function views_ui_build_form_url(FormStateInterface $form_state) { 'view' => $name, 'display_id' => $display_id ]; - $url = Url::fromRoute($route_name, $route_parameters); + $url = UncacheableUrl::fromRoute($route_name, $route_parameters); if ($type = $form_state->get('type')) { $url->setRouteParameter('type', $type); } diff --git a/core/modules/views_ui/src/ViewEditForm.php b/core/modules/views_ui/src/ViewEditForm.php index b11f54a..a4e231f 100644 --- a/core/modules/views_ui/src/ViewEditForm.php +++ b/core/modules/views_ui/src/ViewEditForm.php @@ -19,6 +19,7 @@ use Drupal\Core\Form\FormStateInterface; use Drupal\Core\Render\Element; use Drupal\Core\Render\ElementInfoManagerInterface; +use Drupal\Core\UncacheableUrl; use Drupal\Core\Url; use Drupal\user\SharedTempStoreFactory; use Drupal\views\Views; @@ -1091,7 +1092,7 @@ public function getFormBucket(ViewUI $view, $type, $display) { // Add a [hidden] marker, if the field is excluded. $link_text .= ' [' . $this->t('hidden') . ']'; } - $build['fields'][$id]['#link'] = $this->l($link_text, new Url('views_ui.form_handler', array( + $build['fields'][$id]['#link'] = $this->l($link_text, new UncacheableUrl('views_ui.form_handler', array( 'js' => 'nojs', 'view' => $view->id(), 'display_id' => $display['id'], diff --git a/core/modules/views_ui/src/ViewPreviewForm.php b/core/modules/views_ui/src/ViewPreviewForm.php index 7ec8f4d..ed625bf 100644 --- a/core/modules/views_ui/src/ViewPreviewForm.php +++ b/core/modules/views_ui/src/ViewPreviewForm.php @@ -8,6 +8,7 @@ namespace Drupal\views_ui; use Drupal\Core\Form\FormStateInterface; +use Drupal\Core\UncacheableUrl; use Drupal\Core\Url; /** @@ -65,9 +66,8 @@ public function form(array $form, FormStateInterface $form_state) { 'preview' => $view->renderPreview($this->displayID, $args), ); } - $uri = $view->urlInfo('preview-form'); - $uri->setRouteParameter('display_id', $this->displayID); - $form['#action'] = $uri->toString(); + $url = $view->urlInfo('preview-form')->setRouteParameter('display_id', $this->displayID); + $form['#action'] = UncacheableUrl::fromRoute($url->getRouteName(), $url->getRouteParameters())->toString(); return $form; } diff --git a/core/tests/Drupal/Tests/Core/Render/RendererTestBase.php b/core/tests/Drupal/Tests/Core/Render/RendererTestBase.php index 7388c61..60f5f93 100644 --- a/core/tests/Drupal/Tests/Core/Render/RendererTestBase.php +++ b/core/tests/Drupal/Tests/Core/Render/RendererTestBase.php @@ -9,10 +9,9 @@ use Drupal\Core\Cache\Cache; use Drupal\Core\Cache\MemoryBackend; -use Drupal\Core\Language\LanguageInterface; use Drupal\Core\Render\Element; -use Drupal\Core\Render\Renderer; use Drupal\Core\Render\RenderCache; +use Drupal\Core\Render\Renderer; use Drupal\Tests\UnitTestCase; use Symfony\Component\DependencyInjection\ContainerBuilder; use Symfony\Component\HttpFoundation\Request; @@ -100,6 +99,9 @@ protected function setUp() { $this->themeManager = $this->getMock('Drupal\Core\Theme\ThemeManagerInterface'); $this->elementInfo = $this->getMock('Drupal\Core\Render\ElementInfoManagerInterface'); $this->requestStack = new RequestStack(); + $request = new Request(); + $request->server->set('REQUEST_TIME', $_SERVER['REQUEST_TIME']); + $this->requestStack->push($request); $this->cacheFactory = $this->getMock('Drupal\Core\Cache\CacheFactoryInterface'); $this->cacheContextsManager = $this->getMockBuilder('Drupal\Core\Cache\Context\CacheContextsManager') ->disableOriginalConstructor() @@ -127,7 +129,7 @@ protected function setUp() { return $keys; }); $this->renderCache = new RenderCache($this->requestStack, $this->cacheFactory, $this->cacheContextsManager); - $this->renderer = new Renderer($this->controllerResolver, $this->themeManager, $this->elementInfo, $this->renderCache, $this->rendererConfig); + $this->renderer = new Renderer($this->controllerResolver, $this->themeManager, $this->elementInfo, $this->renderCache, $this->requestStack, $this->rendererConfig); $container = new ContainerBuilder(); $container->set('cache_contexts_manager', $this->cacheContextsManager); diff --git a/core/tests/Drupal/Tests/Core/Routing/UrlGeneratorTest.php b/core/tests/Drupal/Tests/Core/Routing/UrlGeneratorTest.php index 57edd51..2d28eff 100644 --- a/core/tests/Drupal/Tests/Core/Routing/UrlGeneratorTest.php +++ b/core/tests/Drupal/Tests/Core/Routing/UrlGeneratorTest.php @@ -23,6 +23,7 @@ /** * Confirm that the UrlGenerator is functioning properly. * + * @coversDefaultClass \Drupal\Core\Routing\UrlGenerator * @group Routing */ class UrlGeneratorTest extends UnitTestCase { @@ -70,11 +71,14 @@ protected function setUp() { $first_route = new Route('/test/one'); $second_route = new Route('/test/two/{narf}'); $third_route = new Route('/test/two/'); - $fourth_route = new Route('/test/four', array(), array(), array(), '', ['https']); + $fourth_route = new Route('/test/four', [], [], [], '', ['https']); + $none_route = new Route('', [], [], ['_no_path' => TRUE]); + $routes->add('test_1', $first_route); $routes->add('test_2', $second_route); $routes->add('test_3', $third_route); $routes->add('test_4', $fourth_route); + $routes->add('', $none_route); // Create a route provider stub. $provider = $this->getMockBuilder('Drupal\Core\Routing\RouteProvider') @@ -85,22 +89,26 @@ protected function setUp() { // are not passed in and default to an empty array. $route_name_return_map = $routes_names_return_map = array(); $return_map_values = array( - array( + [ 'route_name' => 'test_1', 'return' => $first_route, - ), - array( + ], + [ 'route_name' => 'test_2', 'return' => $second_route, - ), - array( + ], + [ 'route_name' => 'test_3', 'return' => $third_route, - ), - array( + ], + [ 'route_name' => 'test_4', 'return' => $fourth_route, - ), + ], + [ + 'route_name' => '', + 'return' => $none_route, + ], ); foreach ($return_map_values as $values) { $route_name_return_map[] = array($values['route_name'], $values['return']); @@ -415,6 +423,43 @@ public function testPathBasedURLGeneration() { } /** + * Tests generating a relative URL with no path. + * + * @param array $options + * An array of URL options. + * @param string $expected_url + * The expected relative URL. + * + * @covers ::generateFromRoute + * + * @dataProvider providerTestNoPath + */ + public function testNoPath($options, $expected_url) { + $url = $this->generator->generateFromRoute('', [], $options); + $this->assertEquals($expected_url, $url); + } + + /** + * Data provider for ::testNoPath(). + */ + public function providerTestNoPath() { + return [ + // Empty options. + [[], ''], + // Query parameters only. + [['query' => ['foo' => 'bar']], '?foo=bar'], + // Multiple query parameters. + [['query' => ['foo' => 'bar', 'baz' => '']], '?foo=bar&baz='], + // Fragment only. + [['fragment' => 'foo'], '#foo'], + // Query parameters and fragment. + [['query' => ['bar' => 'baz'], 'fragment' => 'foo'], '?bar=baz#foo'], + // Multiple query parameters and fragment. + [['query' => ['bar' => 'baz', 'foo' => 'bar'], 'fragment' => 'foo'], '?bar=baz&foo=bar#foo'], + ]; + } + + /** * Asserts \Drupal\Core\Routing\UrlGenerator::generateFromRoute()'s output. * * @param $route_name