Skip to content

Commit

Permalink
Issue #2692523: whole bunch of changes after Wim did a thorough review:
Browse files Browse the repository at this point in the history
 - Various naming and comment changes.
 - Naming consistency: CacheTagsHeaderSubscriber --> CacheableResponseSubscriber
 - HEADER const moved into CacheableResponseSubscriber - more on this later (turning into plugins).
 - instanceof check.
 - Dropped ::testServicePresence()
  • Loading branch information
nielsvm committed Mar 24, 2016
1 parent 6bc4a80 commit 3e9dafe
Show file tree
Hide file tree
Showing 3 changed files with 24 additions and 41 deletions.
8 changes: 4 additions & 4 deletions purge.services.yml
Expand Up @@ -2,10 +2,10 @@ services:

# PURGE.CACHE_TAGS_HEADER_SUBSCRIBER
#
# Solely responsible for outputting the X-Cache-Tags header, which external
# caching systems and CDNs should store when caching Drupal's responses.
purge.cache_tags_header_subscriber:
class: Drupal\purge\EventSubscriber\CacheTagsHeaderSubscriber
# Solely responsible for outputting the Purge-Cache-Tags header, which external
# caching systems and CDNs should store for tag-based cache invalidation.
purge.cacheable_response_subscriber:
class: Drupal\purge\EventSubscriber\CacheableResponseSubscriber
tags:
- { name: event_subscriber }

Expand Down
Expand Up @@ -2,19 +2,27 @@

/**
* @file
* Contains \Drupal\purge\EventSubscriber\CacheTagsHeaderSubscriber.
* Contains \Drupal\purge\EventSubscriber\CacheableResponseSubscriber.
*/

namespace Drupal\purge\EventSubscriber;

use Symfony\Component\HttpKernel\KernelEvents;
use Symfony\Component\HttpKernel\Event\FilterResponseEvent;
use Symfony\Component\EventDispatcher\EventSubscriberInterface;
use Drupal\Core\Cache\CacheableResponseInterface;

/**
* Adds the X-Cache-Tags response header, to aid external caching systems.
* Adds the Purge-Cache-Tags response header, to aid external caching systems.
*/
class CacheTagsHeaderSubscriber implements EventSubscriberInterface {
class CacheableResponseSubscriber implements EventSubscriberInterface {

/**
* The name of the cache tags header sent.

This comment has been minimized.

Copy link
@wimleers

wimleers Mar 24, 2016

s/sent/to send/

*
* @var string
*/
const HEADER = 'Purge-Cache-Tags';

/**
* {@inheritdoc}
Expand All @@ -25,12 +33,10 @@ public static function getSubscribedEvents() {
}

/**
* Subscribed callback to the KernelEvents::RESPONSE event.
* Sets the Purge-Cache-Tags header on cacheable responses.
*
* @param \Symfony\Component\HttpKernel\Event\FilterResponseEvent $event
* The event to process.
*
* @return void.
*/
public function onRespond(FilterResponseEvent $event) {
if (!$event->isMasterRequest()) {
Expand All @@ -39,13 +45,11 @@ public function onRespond(FilterResponseEvent $event) {

// First ensure that ::getCacheableMetadata() exists on the object.
$response = $event->getResponse();
if (method_exists($response, 'getCacheableMetadata')) {
if ($response instanceof CacheableResponseInterface) {

// Only proceed setting the tags when it isn't yet set by other modules.
if (is_null($response->headers->get('X-Cache-Tags'))) {
$tags = $response->getCacheableMetadata()->getCacheTags();
$response->headers->set('X-Cache-Tags', implode(' ', $tags));
}
$tags = $response->getCacheableMetadata()->getCacheTags();
$response->headers->set(SELF::HEADER, implode(' ', $tags));
}
}

Expand Down
Expand Up @@ -2,35 +2,22 @@

/**
* @file
* Contains \Drupal\purge\Tests\CacheTagsHeaderSubscriberTest.
* Contains \Drupal\purge\Tests\CacheableResponseSubscriberTest.
*/

namespace Drupal\purge\Tests;

use Drupal\purge\Tests\KernelTestBase;
use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\HttpFoundation\Response;
use Drupal\purge\EventSubscriber\CacheableResponseSubscriber;

/**
* Tests \Drupal\purge\EventSubscriber\CacheTagsHeaderSubscriber.
* Tests \Drupal\purge\EventSubscriber\CacheableResponseSubscriber.
*
* @group purge
*/
class CacheTagsHeaderSubscriberTest extends KernelTestBase {

/**
* The name of the cache tags header exported.
*
* @var string
*/
const HEADER = 'X-Cache-Tags';

/**
* The name of the subscribing service.
*
* @var string
*/
const SERVICE = 'purge.cache_tags_header_subscriber';
class CacheableResponseSubscriberTest extends KernelTestBase {

/**
* {@inheritdoc}
Expand All @@ -53,19 +40,11 @@ public function testHeaderPresence() {
$request = Request::create('/system/401');
$response = $this->container->get('http_kernel')->handle($request);
$this->assertEqual(200, $response->getStatusCode());
$header = $response->headers->get(SELF::HEADER);
$header = $response->headers->get(CacheableResponseSubscriber::HEADER);
$this->assertNotNull($header);
$this->assertTrue(is_string($header));
$this->assertTrue(strpos($header, 'config:user.role.anonymous') !== FALSE);
$this->assertTrue(strpos($header, 'rendered') !== FALSE);
}

/**
* Test service presence.
*/
public function testServicePresence() {
$this->assertTrue($this->container->has(SELF::SERVICE));
$this->assertTrue($this->container->getDefinition(SELF::SERVICE)->hasTag('event_subscriber'));
}

}

1 comment on commit 3e9dafe

@wimleers
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Much better :)

Please sign in to comment.