[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: PNG File screenshot-1.png    
Issue Links:
dependency
is depended upon by MGNLADVCACHE-102 Pages with component variants breaki... Closed
relation
is related to MGNLDEMO-211 Tour type cookie component caches sam... Closed
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.
This problem can also be reliably reproduced on demoauthor/demopublic.

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:

  • Create a component variant on a page with different content and add a trait to it (something simple like tourType cookie)
  • Publish the page including the variants
  • Visit the page on public and create a tourType cookie with any value via dev tools (so the variant should be served)
  • Log into public admincentral and clear cache and log out (important because of authenticated cache policy)
  • Reload the page (cookie still active). This will cause to load the page with the active component variant and cache it
  • Delete the cookie
  • Reload page
    --> You will still get the page with the active component variant although the trait isn't active


 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 MGNLDEMO-211.

Regards
Roman

Comment by Olaf Kozlowski [ 12/Sep/17 ]

Hello and thanks for the answer!
I don't think it's another problem. I can reproduce the problem on my current project as well as on demo with the exact same steps.
I haven't customized anything in the personalization so i'm using the standard implementation
I'm trying to explain it better using a screenshot

I have a personalized component on /home/stage/0 on the requested page with an active trait.
The node is correctly visited as you can see in the screenshot (using node.getPath().
But if you call getWrappedNode().getPath() you'll also see the component node has already been wrapped with the variant-0 hence the check for hasVariant will fail and lead to a wrong cache entry

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 ]

-I don't know how this can happen since /server/filters/cache filter is before /server/filters/cms/variantResolver (where are the nodes wrapped with ComponentPersonalizationNodeWrapper). But it's wrapped in your case. Can you please show the whole stacktrace?-
Update: It's on the way back trough the filter chain and the node it's really wrapped. Marking this ticket as accepted.
Keep in mind that even after fixing this, MGNLDEMO-211 is still a separate issue.

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.
rkovarik

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.
Will make the same workaround.

Generated at Mon Feb 12 06:37:31 CET 2024 using Jira 9.4.2#940002-sha1:46d1a51de284217efdcb32434eab47a99af2938b.