search

putyourlightson / craft-blitz

Intelligent static page caching for creating lightning-fast sites with Craft CMS.
https://putyourlightson.com/plugins/blitz
Other
149 stars 36 forks source link

Blitz cache is not refreshing entry when an asset of an related entry get replaced #657

Closed gddotorg closed 2 months ago

gddotorg commented 5 months ago

Bug Report

Hi Ben,

we discovered that Blitz does not refresh the cached version of an entry when an asset of an related entry get replaced.

So our setup looks like this: We have an entry "contact page" with a matrix block. Inside one of the blocks we have an entries field, where you can link contact persons. Those entries "contact persons" have an asset field with the persons image.

  1. On contact page we change the persons picture by clicking on the persons entry (Pop-up opens sideways)
  2. We save the persons entry and afterwards also the contact page
  3. Blitz does create several refresh queue jobs for person entry, but no job to refresh the contact page. Even after saving contact page, we can still see the old cached version of the contact page.
  4. On the other hand, the contact page refresh works when we change the title of the contact person entry. Then both the contact person and the contact page are refreshed.

Did I miss something? Do I have to adjust the settings if necessary?

If you need further information, just let me know.

Thanks

Plugin Version

4.16.3

Craft CMS Version

4.5.13

PHP Version

8.1.27

bencroker commented 5 months ago

Can you ensure that both trackElements and trackElementQueries are set to true in project config? Check also that both deprecated cacheElements and cacheElementQueries are true, as they can override the values of the former if they exist in Blitz 4.

If that all holds then go into the Blitz Diagnostics utility where you’ll be able to see and verify whether the page in question is tracking the persons entry and the asset. Note that they will only be tracked by the cached page provided they are actually output on the page.

gddotorg commented 5 months ago

I modified the configs like you suggested. Same result.

I also checked the tracked page and its tracked elements. Contact person entry is tracked and also the linked asset. Just as an addition, if this is relevant, no tracked element queries are displayed under the tracked page.

bencroker commented 5 months ago

I modified the configs like you suggested. Same result.

Were any of them set to false? If so then you’ll need to refresh the cache for the pages in question before the changes will take effect.

gddotorg commented 5 months ago

cacheElements and cacheElementQueries were set to false. After setting them true, I refreshed the entire cache. I then checked the tracked elements as mentioned above. Seems to be fine. But when I replace an asset of one of the contact persons, the old cached version of the contact page is still displayed.

bencroker commented 5 months ago

Is the tracked page tracking the asset in question, according to Blitz diagnostics?

gddotorg commented 4 months ago

The asset is tracked. I have provided you with some screenshots. I have also updated to Blitz 4.18.0 and sent you the diagnostic export. Screenshot_1 Screenshot_2 Screenshot_3 Screenshot_4

#### Application Info

- PHP version: 8.1.27
- Craft edition & version: Pro 4.5.13
- Database driver & version: MySQL 8.0.33

#### Installed Plugins

- Blitz: 4.18.0
- captcha.eu: 1.0.4
- CP Field Inspect: 1.4.4
- Empty Coalesce: 4.0.0
- Field Manager: 3.0.8
- Formie: 2.1.0
- Hyper: 1.1.16
- ImageOptimize: 4.0.5
- Redactor: 3.0.4
- Retour: 4.1.14
- SEOmatic: 4.0.35
- Smith: 2.0.0
- Sprig: 2.7.2
- Super Table: 3.0.12
- Timber: 1.0.3
- Vite: 4.0.6
- Wishlist: 2.0.7

#### Loaded Modules

- codeeditor: nystudio107\codeeditor\CodeEditor
- sprig-core: putyourlightson\sprig\Sprig
- blitz-hints: putyourlightson\blitzhints\BlitzHints
- verbb-base: verbb\base\Base

#### Blitz Plugin Settings

```json
{
    "debug": true,
    "hintsEnabled": true,
    "cachingEnabled": true,
    "refreshCacheEnabled": true,
    "refreshMode": 1,
    "includedUriPatterns": [
        {
            "uriPattern": ".*"
        }
    ],
    "excludedUriPatterns": [],
    "cacheStorageType": "putyourlightson\\blitz\\drivers\\storage\\FileStorage",
    "cacheStorageSettings": {
        "folderPath": "@webroot\/cache\/blitz",
        "createGzipFiles": true,
        "countCachedFiles": true
    },
    "cacheStorageTypes": [],
    "cacheGeneratorType": "putyourlightson\\blitz\\drivers\\generators\\HttpGenerator",
    "cacheGeneratorSettings": {
        "concurrency": "3"
    },
    "cacheGeneratorTypes": [],
    "customSiteUris": [],
    "cachePurgerType": "putyourlightson\\blitz\\drivers\\purgers\\DummyPurger",
    "cachePurgerSettings": [],
    "cachePurgerTypes": [],
    "deployerType": "putyourlightson\\blitz\\drivers\\deployers\\DummyDeployer",
    "deployerSettings": [],
    "deployerTypes": [],
    "ssiEnabled": true,
    "ssiTagFormat": "<!--#include virtual=\"{uri}\" -->",
    "detectSsiEnabled": true,
    "esiEnabled": false,
    "queryStringCaching": 1,
    "includedQueryStringParams": [
        {
            "siteId": "",
            "queryStringParam": ".*"
        }
    ],
    "excludedQueryStringParams": [
        {
            "siteId": "",
            "queryStringParam": "gclid"
        },
        {
            "siteId": "",
            "queryStringParam": "fbclid"
        }
    ],
    "apiKey": "",
    "generatePagesWithQueryStringParams": true,
    "purgeAssetImagesWhenChanged": true,
    "refreshCacheAutomaticallyForGlobals": false,
    "refreshCacheWhenElementMovedInStructure": true,
    "refreshCacheWhenElementSavedUnchanged": false,
    "refreshCacheWhenElementSavedNotLive": false,
    "cacheNonHtmlResponses": false,
    "trackElements": true,
    "trackElementQueries": true,
    "excludedTrackedElementQueryParams": [],
    "cacheElements": true,
    "cacheElementQueries": true,
    "cacheDuration": null,
    "nonCacheableElementTypes": [
        "verbb\\wishlist\\elements\\ListElement",
        "verbb\\wishlist\\elements\\Item"
    ],
    "sourceIdAttributes": [],
    "liveStatuses": [],
    "integrations": [
        "putyourlightson\\blitz\\drivers\\integrations\\SeomaticIntegration"
    ],
    "defaultCacheControlHeader": "no-cache, no-store, must-revalidate",
    "cacheControlHeader": "public, s-maxage=31536000, max-age=0",
    "cacheControlHeaderExpired": "public, s-maxage=5, max-age=0",
    "sendPoweredByHeader": true,
    "outputComments": true,
    "refreshCacheJobPriority": 110,
    "driverJobBatchSize": 100,
    "driverJobPriority": 100,
    "queueJobTtr": 300,
    "maxRetryAttempts": 10,
    "maxUriLength": 255,
    "mutexTimeout": 1,
    "commands": [],
    "injectScriptEvent": "DOMContentLoaded",
    "injectScriptPosition": 3
}

Recommendations

Site Tracking [1]

Site Tracking [3]

Site Tracking [4]

Site Tracking [5]

Site Tracking [6]

Site Tracking [2]

bencroker commented 4 months ago

Thanks. I’ll try to replicate your setup locally and let you know what I find.

bencroker commented 4 months ago

I tested your setup locally and it is clearing the cached page as expected.

Probably unrelated, but note that, as per the diagnostics recommendations, image transforms should be configured to be generated before page load.

Please retest and if it is still an issue then please describe the exact steps to reproduce and how you’re verifying things, and I can take another look.

gddotorg commented 4 months ago

@bencroker Unfortunately, we still need some time to retest. We still need to coordinate with our customer. I'll let you know soon how we proceed here.

bencroker commented 4 months ago

Ok, let me know when you have an update.

bencroker commented 4 months ago

Have an update yet, @gddotorg?

mandrasch commented 4 months ago

Hey @bencroker, colleague of Georg here. Thanks for investigating this!

I tried a minimal reproduction today with our used versions:

    "craftcms/cms": "4.5.13",
    "nystudio107/craft-imageoptimize": "4.0.5",
    "putyourlightson/craft-blitz": "4.9.0",

BUT ... on reproduction everything works as expected. 🤓

I tried multiple things to reproduce our project settings, but no luck so far reproducing the bug.

The connected entries of the field in question are also used in another channel and entry field. Haven't tried with all the plugins we use.

On the real site, I can see locally via xdebug that multiple refresh cache jobs are triggered when I change the image in the field - but they never have a site URI attached:

image

Tried the same after updating to 4.18.1, no site URIs in all three refresh blitz cache jobs.

So my current best guess is that the usage for the page in question is somehow not correctly detected?

bencroker commented 4 months ago

So to clarify, it is an issue in the “real site” but you’re unable to reproduce locally when testing (and neither was I)? If that’s right, then it sounds like the “real site” needs a full cache refresh (clear and flush).

mandrasch commented 4 months ago

I can reproduce it with the "real site" project locally - see xdebug screenshot.

But I created a second project locally from scratch with similiar stuff, couldn't reproduce it there. (Wanted to share that with you since we can't simply share the real project with you I guess due to legal stuff, etc.... )

bencroker commented 4 months ago

Ah I see. If you can’t share the project on which the issue is happening, then all I can offer is to be brought in as a consultant, or however you want to frame it to get around legal issues. Otherwise I’m not sure how I can be of assistance.

mandrasch commented 4 months ago

Totally understandable. 👍

Guess I would have to do a deep dive into the blitz code with xdebug and find out why there are not cached ids / site URIs in our particular project case.

Please feel free to close the issue, I'm not sure if and when I'll get resources for that. Although I'm very curious to find it out personally 🕵️ 😄

bencroker commented 4 months ago

Well, if the tracked page is tracking the asset in question according to Blitz diagnostics, then it should all “just work”.

mandrasch commented 4 months ago

Well, if the tracked page is tracking the asset in question according to Blitz diagnostics, then it should all “just work”.

Thanks for the hint. Haven't worked with diagnositics before (but wanted to check it out for some time now since it was released). Awesome stuff! 😮 👏

So, just as optional information:

"Real site": "Tracked Pages" > Kontakt > "craft\elements\Entry"

I see the connected entries, but without image field

image

(I see the existing connected assets in Tracked Assets - but if I choose another one and run the queue with blitz cache refresh, those are not updated.)

On my small reproduction site, I see the Assets field for the connected entry though:

image

On our real site we do a lot with connected entries, have also a second Entries field in the same layout element where entries can be referenced via another channel, etc. More complexity in the real site of course.

Haven't found the difference what causes this yet. 🕵️

If you have perhaps another hint how I could debug what get's tracked for an entry - much appreciated. But I'm not in a hurry with this and can look it up myself as well in the code.

bencroker commented 4 months ago

Can you double check that the Person Image field is indeed being output on the Kontakt page, and possibly show me the Twig code used to do so?

mandrasch commented 4 months ago

Hi, thanks very much for your assistance again! Much appreciated! :-)

I identified the problem! 🎉 🎉

It was eager loading with this snippet from https://nystudio107.com/blog/speed-up-your-craft-cms-templates-with-eager-loading for auto-injected entries. 

{% do craft.app.elements.eagerLoadElements(
    className(entry),
    [entry],
    [
          'layoutElements.contactCards:persons', 'layoutElements.contactCards:persons.personImage', 'layoutElements.contactCards:groupOfPersons', 'layoutElements.contactCards:groupOfPersons.persons.personImage',
     ])
%}

We call it in _page/index.twig: 

{% include "_eagerLoading/eagerLoadLayoutElements" %}

When I remove it, everything works as expected.

(I'm not an expert yet on eager loading, just found this discussion on Craft Discord where you did not recommend this approach from above anymore - if I understand it correctly)

bencroker commented 4 months ago

It’s no longer really useful, and therefore just extra code to maintain. Can you show me the Twig code used to actually output the field?

mandrasch commented 4 months ago

index.twig:

{% include "_eagerLoading/eagerLoadLayoutElements" %}
{% if entry.type.handle is same as("pages") %}
     {% set layoutElements = entry.layoutElements ??? null %}
            {% if layoutElements %}
                {% for layoutElement in layoutElements %}
                    {% include '_layoutElements/' ~ layoutElement.type %}
                {% endfor %}
            {% endif %}

Then it's basically this:

        {% for person in persons %}
            {% set imageElem = person.personImage ??? null %}
            {% set image = imageElem ? imageElem[0] : null %}

            % set params = {
                "image": image,
                "optimizedImageHandle": "optimizedImages",
            } %}
            {{ optimizedImage(params)}}

Image Optimize is used, therefore image.url is not used. Only methods like optimizedImage.srcset(), macro:

{% macro optimizedImage(params) %}
{% set image = params.image %}
{% set optimizedImage = image[params.optimizedImageHandle] %}

{# ... #}

The call is also passed through three different macros with a props-approach:

{% macro contactCards(props) %}
{% set persons = props.persons ??? null %}

The call is also in some else-Statements, but I guess this doesn't really matter.

{% if persons|length > 0 %}
    {% if type is same as('type123') %}
        {# not our relevant case ... #}
        {% for person in persons %}
        {% set imageElem = person.customProfileImage ??? null %}
        {% set image = imageElem ? imageElem[0] : null %}
        {# .... #} 
    {% else %}
        {# our problem-related case #} 
    {% for person in persons %}
            {% set imageElem = person.personImage ??? null %}
        {% set image = imageElem ? imageElem[0] : null %}
        {% set name = person.title %}
        {% set job = person.personJob ??? null %}
        {% set phoneNumber = person.phoneNumber ??? null %}
        {% set mobileNumber = person.mobileNumber ??? null %}
        {% set mailAddress = person.mailAddress ??? null %}

            {{ contactCard({ "image": image, "name": name, "job": job, "phoneNumber": phoneNumber, "mobileNumber": mobileNumber, "mailAddress": mailAddress, "showCopyright": showCopyright }) }}
     {% endif %}

If you need more information or a reproduction version with our macros, just let me know.

bencroker commented 4 months ago

This could be a limitation when eager-loading nested elements inside of matrix blocks in Blitz 4. This is a non-issue in Blitz 5, since matrix blocks have been replaced by entries in Craft 5. If you can provide a reproducable test case then I may be able to find a workaround, but in the meantime removing the eager-loading of auto-injected variables with eager-loading of the fields should work. Let me know how it goes.

mandrasch commented 2 months ago

hi @bencroker, thanks for support again!

Don't know if you got a notification - I provided you with access to a reproduction demo some weeks ago (https://github.com/mandrasch/craftcms-repro-cache-asset-issue).

Just wanted to quickly ask:

Is a workaround in your plugin product scope / interest right now?

but in the meantime removing the eager-loading of auto-injected variables with eager-loading of the fields should work

I would follow that suggestion, but wanted to make sure to ask you if a workaround would be possible first - since we have to re-test the whole client project if anything breaks after removing the eager-loading of auto-injected variables (obviously). And we use this eager-loading technique in almost all client projects 👀 - strangely here it was the first project where customers have reported this while editing.

bencroker commented 2 months ago

I have access to the repo, but no instructions to reproduce the issue. Here’s what I tried.

  1. Cleared and flushed the Blitz cache.
  2. Visited /contact and confirmed that the page was cached.
  3. Noted that the cached page tracks 4 entries and 2 assets (in Blitz Diagnostics).
  4. Changed the title in one of the tracked assets and saved the asset.
  5. Ran the queue.
  6. Confirmed that the cached /contact page was cleared.

This all looks fine to me. What are you seeing that makes you think something is not working?

mandrasch commented 2 months ago

Sorry, chased this in my personal / free time because I'm curious - and did forget to add more instructions. 😬

  1. View subsite "Contact" image

  2. Go to Pages > Contact, open a contact in sidebar drawer

image

  1. Upload a new image (only edit image/asset field!) to the related entry

image

  1. Hit save on sidebar entry

  2. Run queue jobs ddev craft queue/run

  3. View subsite contact again, no change image

Even after saving whole page, and ddev craft queue/run -> same output on https://craftcms-repro-cache-asset-issue.ddev.site/contact, new image does not show up.

mandrasch commented 2 months ago

Error occurs when only the asset is changed.

When title (or other fields) are changed for the related entry, everything works fine.

bencroker commented 2 months ago

Thanks, I’ll look into working on a fix.

bencroker commented 2 months ago

I believe I’ve managed to address your edge-case in https://github.com/putyourlightson/craft-blitz/commit/508ed44fd393c0ccb8e5d691ea93ebe6cb6939c0.

You can test it by running composer require "putyourlightson/craft-blitz:dev-feature/track-eager-loaded-nested-relation-fields as 4.21.0".

Let me know how it goes!

mandrasch commented 2 months ago

Awesome, thanks!! 🎉 🎉

I ran

ddev composer require "putyourlightson/craft-blitz:dev-feature/track-eager-loaded-nested-relation-fields as 4.21.0"
ddev composer dumpautoload
ddev craft clear-caches/all

Visited https://craftcms-repro-cache-asset-issue.ddev.site/contact beforehand to make sure it's cached.

Asset replacement worked now on reproduction repo ✅

Tested it on the original project as well locally, worked as well ✅ 🎉

bencroker commented 2 months ago

Great! Thanks for helping me track this down. Released in 4.21.0.