blocker to #2335661: Outbound path & route processors must specify cacheability metadata, based on discussion wiht Wim Leers
Problem/Motivation
For menu links or any generated URL strings we need to track cache metadata.
The current BubbleableMetadata is too specific to render arrays and handles attached assets, etc. We don't need the extra overhead when handling a simple string.
Proposed resolution
Create a superclass of BubbleableMetadata then omits logic not needed for simpler data types. Make the current BubbleableMetadata as subclass of it, possibly renaming it.
Remaining tasks
User interface changes
n/a
API changes
Class name change.
Comment | File | Size | Author |
---|---|---|---|
#10 | increment.txt | 2.77 KB | pwolanin |
#10 | 2471743-9.patch | 7.56 KB | pwolanin |
#6 | increment.txt | 4.27 KB | pwolanin |
#6 | 2471743-6.patch | 5.46 KB | pwolanin |
#4 | 2471743-4.patch | 6.71 KB | pwolanin |
Comments
Comment #1
pwolanin CreditAttribution: pwolanin at Acquia commentedComment #2
Wim LeersEither
CacheableMetadata
orCacheabilityMetadata
are the names that come to mind.Comment #3
pwolanin CreditAttribution: pwolanin at Acquia commentedComment #4
pwolanin CreditAttribution: pwolanin at Acquia commentedwhoops - that had some other crap in it. Here's the right patch.
Comment #5
Wim LeersThis is not an interface, but a class :)
Why is this interface necessary? Why would anyone ever want to swap out the
CacheableMetadata
class?Comment #6
pwolanin CreditAttribution: pwolanin at Acquia commentedok.
Comment #7
Wim LeersBlocks #2335661: Outbound path & route processors must specify cacheability metadata.
Patch looks great; now just a matter of moving a bunch of
BubbleableMetadataTest
toCacheableMetadataTest
.s/storing and fetching/passing/
But that's a nit.
Comment #10
pwolanin CreditAttribution: pwolanin at Acquia commentedCache::mergeContexts() calls out to a service, so can't unit test it.
Comment #12
Wim LeersFailing tests :(
Could use a data provider.
Comment #13
pwolanin CreditAttribution: pwolanin at Acquia commented@Wim Leers - no, I considered a data provider, and I don't think it makes sense because I'm accumulating tags - in other words, the correctness of each expected result depends on the prior ones. I don't think that's how data providers usually work.
Comment #15
Wim LeersOh, sorry, missed that.
Comment #16
alexpottThis issue addresses a major bug and is allowed per https://www.drupal.org/core/beta-changes. Committed a0ee087 and pushed to 8.0.x. Thanks!
Fixed a capitalisation thing on commit.
Comment #18
Fabianx CreditAttribution: Fabianx for Acquia commented+1 to this change!