WIP: Http cache multi-tagging: MVC #1898

bdunogier · 2017-01-29T23:37:26Z

MVC part of EZP-22401

The objective is to add to Responses from the MVC layer a xkey header with the tags matching the value objects they contain. For a typical content view, it would be like xkey: content-1, location-2, content-type-1.

As this value <=> cache tag mapping also applies to REST embedding, the logic must be re-usable by the REST API.

Architecture

It articulates around ResponseTaggers. Those will, for a given value object type, add tags to a Response. While some may add tags directly (ContentInfoTagger, RelationTagger), others will extract one or more value objects from the one they were given, and delegate its tagging to others Taggers (ContentValueViewTagger, RestContentTagger).

$tagger->tag($configurator, $response, $content);

To do the actual tagging, the Taggers are given a ResponseCacheConfigurator. This configurator centralizes the cache configuration of the response, included but not limited to the xkey header name. It can also make the response public, and set the shared max age:

$configurator->makePublic($response);
$configurator->setSharedMaxAge($response);
$configurator->addTags($response, ['content-1']);

The actual tagging is done by the HttpCacheResponseSubscriber, that replaces (and deprecates) CacheViewResponseListener. It uses a Configurator and a ResponseTagger. The ResponseTagger is a special one, the DispatcherTagger. It has all the tagger services tagged with ezplatform.cache_response_tagger.

The listener will tell the dispatcher ResponseTagger to tag the Response with the View object it gets from the request attributes.

Tagging example

Taking ContentView as an example, the call chain is as follows:

- CacheViewEventListener, $dispatcherTagger->tag($contentView)
  - ContentValueViewTagger # Extracts the ContentInfo using the ContentValueView interface
      - ContentInfoTagger # adds tags for the content info
    - LocationValueViewTagger # Extracts the ContentInfo using the LocationValueView interface
      - LocationTagger # adds tags for the location

Usage in REST

In the current REST embedding implementation, an extra REST controller layer is added so that tags can be added to a CachedValue decorator. The controllers implementation contains the tagging logic.

The idea is to remove this controller layer, and use the tagger from the CachedValue and ResourceLink visitors to tag the HTTP responses.

Additional Taggers will be needed to extract actual repository value objects from the REST aggregate value objects (example: RestContent).

PhpSpec

The classes from this feature have been specificed using phpspec, "A php toolset to drive emergent
design by specification.". Please have a look at the execution result from travis, they should explain the architecture above quite well.

The commit adds phpspec, as well as spec-gen, a library that generates code based on specifications.

Testing done

phpspec
http request with cache enabled on the home page, checked that the xkey tags were sent

Behat

The pull-request includes three behat scenarios that cover the main lines of content view cache tagging. The sentences are not implemented.

TODO

Fix unit tests
Make sure phpunit and phpspec must both pass for travis to pass
Write down description above to spec document
Consider merging ResponseCacheConfigurator::makePublic() and ::setSharedMaxAge() to one ::configure() call. The settings can be used to set both cache-control and s-maxage.
Make sure that fos-httpcache-bundle is configured to send the xkey header for cache tags
Update BC doc

lolautruche · 2017-01-30T14:29:47Z

eZ/Bundle/EzPublishCoreBundle/Resources/config/templating.yml

@@ -302,6 +302,9 @@ services:
            - $content.default_ttl$
        tags:
            - { name: kernel.event_subscriber }
+        calls:
+            - [setConfigurator, ['@ezplatform.http_cache.response_configurator']]
+            - [setDispatcherTagger, ['@ezplatform.http_cache.response_tagger.dispatcher']]


Why using setters here, instead of constructor arguments?

Because I didn't want to add them as constructor arguments... 'cause of bit of BC, and because of quantum.

But I agree that this isn't ideal. I had two options in mind:
a) Deprecate this listener (and untag the service ? what is BC supposed to be like on a listener ?), and write a new one
b) add the services at the end in the constructor, and ignore that it is a ugly
c) ... ?

I'm open to suggestions, because I would like to get rid of these calls. In any case, it's not enough even for BC, as I don't test if the configurator is there or not. If it is not, we need to tag using the old approach for BC.

Or am I overthinking this ?

If you want to go the BC way, assuming that this listener is extended by some:

Create a new class with all you need and make it inherit from the deprecated one

On the deprecated one, you'll probably need to change visibility on properties

Trigger deprecation errors from the deprecated one.

Redefine the service to match the new class

WDYT?

Sounds good to me, I'm on it. Thank you !

lolautruche · 2017-01-30T14:33:05Z

eZ/Bundle/EzPublishCoreBundle/DependencyInjection/Configuration.php

-  Examples:
-   - (123|456|789) => Purge locations #123, #456, #789.
-   - .* => Purge all locations.
+- If "http" is used, only one Http PURGE request will be sent, with xkey header containing tags to purge seperated by space.


Will there be a BC somehow with former X-Location-Id?

The updated VCL should handle both, as well as SymfonyCache. So yes.
But the MVC layer will only send xkey once this is merged, not X-Location-Id. Do you think that this is a problem ?

Note to self: change xkey to key.

OK fine then 👍

Removed this changeset from this PR, moved it to the purge client one ( #1894).

andrerom · 2017-01-30T16:58:21Z

eZ/Publish/Core/MVC/Symfony/View/RelationView.php

+/**
+ * A view that contains relation data.
+ */
+interface RelationView


This part we considered to rather get users to do from templates / view manually right? As it's not always relevant to tag the relation. If so we would need to start using for instance fos_httpcache_tag.

Reason: Currently we'll tag if there are relations, but ideally it should only be tagged if it is used. (but we still need to tag it here, as we don't want inline calls for it to show up a 404, as opposed to updating this objects view cache to signal that field is updated..)

bdunogier · 2017-01-30T23:50:20Z

@andrerom the files you might wanna look in particular are the Taggers:

Value taggers: ContentTagger, LocationTagger and RelationTagger [1]
Delegator taggers, mostly ContentViewTagger. This is the one that extracts the ValueObjects we want to tag from the ContentView, and calls the Value taggers above.

Questionnable as discussed today, as {{ fos_httpcache_tag }} from the template might be more efficient

Hint just in case: the tags are added to the Response by the ResponseCacheConfigurator. That's where we would make the xkey header customizable if needed.

bdunogier · 2017-02-02T12:08:23Z

Cleaned up and reworked. Removed quite a few superfluous files.

We are down to Location and ContentInfo taggers (and their view delegators). Tagging of related content is now done using a fos_httpcache_tag call in the relation(list) field view template block.

The CacheViewEventListener is modified to use Taggers and a ResponseCacheConfigurator. The ResponseCacheConfigurator centralizes cache configuration handling for a Response: ``` $configurator->makePublic($response); $configurator->addTags($response, ['location-2', 'content-345']); ``` Any Value Object may have a Tagger. A Tagger can add tags to a Response, by mapping properties from the Value it handles: $tagger->tag($configurator, $response, $content); To add the tags to the Response, Taggers are given the ResponseCacheConfigurator. In addition to Taggers that map values from the value objects, other taggers may extract a value object, and delegate its tagging to another Tagger. One example is ContentView, that is delegated to ContentInfoTagger by the ContentValueViewTagger. This commit also integrates phpspec. The objects defined here are all covered by their own specs, that tries to describe the behaviour that can be expected from them.

andrerom · 2017-02-02T12:55:03Z

Tagging of related content is now done using a fos_httpcache_tag call in the relation(list) field view template block.

ok, is FOS configured to use the right header somewhere?

andrerom · 2017-02-02T12:57:04Z

We are down to Location and ContentInfo taggers (and their view delegators).

How is this modeled from high level?

bdunogier · 2017-02-03T17:16:01Z

ok, is FOS configured to use the right header somewhere?

Probably not, good point.

How is this modeled from high level?

As described in the PR's description :)

Short: the HttpCache event subscriber will pass the (Content)View to the DispatcherResponseTagger. The DispatcherResponseTagger will give the View to every tagger.

The ContentView will be processed by the ContentValueView and LocationValueView (delegator) taggers. Those will extract the ContentInfo and Location objects, and tag the response using the ContentInfo and Location (value) taggers.

bdunogier · 2017-02-03T17:31:23Z

Fixed CS, and added a specification document.

bdunogier · 2017-02-08T18:12:51Z

Replaced by http://github.com/ezsystems/ezplatform-http-cache. Please do not delete the branch yet.

bdunogier force-pushed the http_cache_mvc branch 2 times, most recently from e95ce6f to 381ea01 Compare January 30, 2017 13:36

lolautruche reviewed Jan 30, 2017

View reviewed changes

andrerom reviewed Jan 30, 2017

View reviewed changes

bdunogier force-pushed the http_cache_mvc branch 2 times, most recently from 64ff00a to c340cfd Compare February 2, 2017 12:05

Bertrand Dunogier added 3 commits February 2, 2017 13:13

EZP-22401: Added new HTTP cache response subscriber

69cb94e

EZP-22401: Added fos_httpcache_tag call to relation field templates

1709db1

bdunogier force-pushed the http_cache_mvc branch from 2f4e7c8 to 1709db1 Compare February 2, 2017 12:13

Bertrand Dunogier added 2 commits February 3, 2017 18:30

fixup! EZP-22401: Added HTTP Response Tagging API

eae47bf

fixup! EZP-22401: Added HTTP Response Tagging API

42821fa

EZP-22401: Added view cache tags behat scenarios

27db958

bdunogier closed this Feb 8, 2017

alongosz deleted the http_cache_mvc branch July 2, 2019 13:13

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

WIP: Http cache multi-tagging: MVC #1898

WIP: Http cache multi-tagging: MVC #1898

bdunogier commented Jan 29, 2017 •

edited

lolautruche Jan 30, 2017

bdunogier Jan 30, 2017

lolautruche Jan 31, 2017 •

edited

bdunogier Feb 1, 2017

lolautruche Feb 1, 2017

lolautruche Jan 30, 2017

bdunogier Jan 31, 2017

bdunogier Jan 31, 2017

lolautruche Jan 31, 2017

bdunogier Jan 31, 2017

andrerom Jan 30, 2017 •

edited

bdunogier commented Jan 30, 2017 •

edited

bdunogier commented Feb 2, 2017

andrerom commented Feb 2, 2017

andrerom commented Feb 2, 2017

bdunogier commented Feb 3, 2017 •

edited

bdunogier commented Feb 3, 2017 •

edited

bdunogier commented Feb 8, 2017

WIP: Http cache multi-tagging: MVC #1898

WIP: Http cache multi-tagging: MVC #1898

Conversation

bdunogier commented Jan 29, 2017 • edited

Architecture

Tagging example

Usage in REST

PhpSpec

Testing done

Behat

TODO

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lolautruche Jan 31, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andrerom Jan 30, 2017 • edited

Choose a reason for hiding this comment

bdunogier commented Jan 30, 2017 • edited

bdunogier commented Feb 2, 2017

andrerom commented Feb 2, 2017

andrerom commented Feb 2, 2017

bdunogier commented Feb 3, 2017 • edited

bdunogier commented Feb 3, 2017 • edited

bdunogier commented Feb 8, 2017

bdunogier commented Jan 29, 2017 •

edited

lolautruche Jan 31, 2017 •

edited

andrerom Jan 30, 2017 •

edited

bdunogier commented Jan 30, 2017 •

edited

bdunogier commented Feb 3, 2017 •

edited

bdunogier commented Feb 3, 2017 •

edited