[MGNLPN-426] Pages with component variants breaking cache Created: 12/Sep/17 Updated: 13/Jul/18 Resolved: 12/Jul/18 |
|
| Status: | Closed |
| Project: | Magnolia Personalization |
| Component/s: | Integration |
| Affects Version/s: | None |
| Fix Version/s: | 1.5.4, 1.6.1 |
| Type: | Bug | Priority: | Critical |
| Reporter: | Olaf Kozlowski | Assignee: | Roman Kovařík |
| Resolution: | Fixed | Votes: | 6 |
| Labels: | caching, personalization-integration | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Attachments: |
|
||||||||||||||||
| Issue Links: |
|
||||||||||||||||
| Template: |
|
||||||||||||||||
| Acceptance criteria: |
Empty
|
||||||||||||||||
| Task DoD: |
[ ]*
Doc/release notes changes? Comment present?
[ ]*
Downstream builds green?
[ ]*
Solution information and context easily available?
[ ]*
Tests
[ ]*
FixVersion filled and not yet released
[ ] 
Architecture Decision Record (ADR)
|
||||||||||||||||
| Bug DoR: |
[ ]*
Steps to reproduce, expected, and actual results filled
[ ]*
Affected version filled
|
||||||||||||||||
| Release notes required: |
Yes
|
||||||||||||||||
| Date of First Response: | |||||||||||||||||
| Epic Link: | Support | ||||||||||||||||
| Sprint: | Basel 150, Kromeriz 155 | ||||||||||||||||
| Story Points: | 8 | ||||||||||||||||
| Description |
|
I've encountered a rather serious bug in the personalization cache that breaks pages with component variants. Problem explanation: The issue occurs after clearing the cache / on a request that triggers a 'store' behaviour in the cache. If this first request serves a page with an active component variant, the cache breaks and stores the personalized page as an InMemoryCachedEntry instead of an PersonalizedCacheEntry. This causes the cache to always serve the same personalized page no matter the traits. The code in question: info.magnolia.personalization.variant.PersonalizedDescendantsVisitor
@Override
public void visit(Node node) throws RepositoryException {
if (variantManager.hasVariant(node)) {
personalizedDescendants.add(node);
}
}
The check hasVariant(node) will fail on the component node with the variant since the node is already wrapped with the variant meaning the node itself is already the variant. Extending the check to if (variantManager.hasVariant(node) || variantManager.isVariant(node)) will solve the issue (I don't know if this might cause side effects. Maybe the check needs to be extended further) and create a correct cache entry. Steps to reproduce:
|
| Comments |
| Comment by Roman Kovařík [ 12/Sep/17 ] |
|
Hello olaf.kozlowski If the node is itself is a variant, you're accessing it via a full path (e.g /page/variants/variant-0), thus a different URL, different cache key, separate cache item. The problem in the demo is a different issue, see Regards |
| Comment by Olaf Kozlowski [ 12/Sep/17 ] |
|
Hello and thanks for the answer! I have a personalized component on /home/stage/0 on the requested page with an active trait. |
| Comment by Vivian Steller [ 12/Sep/17 ] |
|
I think the problem is that info.magnolia.jcr.util.NodeUtil#visit(javax.jcr.Node, info.magnolia.jcr.util.NodeVisitor, org.apache.jackrabbit.commons.predicate.Predicate) iterates over decorated/wrapped nodes. In the case of searching for variant nodes it should rather operate on plain, undecorated nodes. WDYT? |
| Comment by Roman Kovařík [ 12/Sep/17 ] |
|
- It should either not operate over decorated/wrapped nodes as vivi pointed out or use olaf.kozlowski' suggestion to call variantMAnager.isVariant(). Thanks for investigating. |
| Comment by Simon Radford [ 03/Nov/17 ] |
|
We've also encountered this bug while testing component personalization. This is a showstopper, as to use component personalization properly will mean we will have to turn off page caching entirely and this will have a big impact on performance. Also a separate issue, but we have also found that component variants do not activate correctly from the pages app (also reproduceable on the demo site). Component personalization does not seem production ready, which is a shame as our user's were looking forward to this. |
| Comment by Niels Hardeman [ 19/Jun/18 ] |
|
We found the same issue for one of our EE Pro clients. If the cache is populated while one of the variants was resolved the caching mechanism malfunctions. We determined that replacing info.magnolia.advancedcache.personalization.cache.PersonalizedCacheStore with our own implementation should allow us to resolve the issue on our own... waiting for that fix... |
| Comment by Niels Hardeman [ 25/Jun/18 ] |
|
Hi @Magnolia, We managed to solve this problem on our end by replacing PersonalizedCacheStore with our own implementation class. To create this custom implementation we copied the original code and changed one lines. The trick is to unwrap the node beyond the node wrapper for personalization. private boolean isPersonalized(Node node, Cache cache) throws RepositoryException { final Node unwrappedNode = NodeUtil.deepUnwrap(node, ContentDecoratorNodeWrapper.class); ...
|
| Comment by Rico Jansen [ 25/Jun/18 ] |
|
I can confirm this bug, as I just ran into it myself. I already found that the node being personalized broke the check. |