putyourlightson / craft-blitz

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

Do not cache URLs with query strings does nothing #729

Closed samuelreichor closed 17 hours ago

samuelreichor commented 1 week ago

Bug Report

I want to disable caching for pages with query parameters, but it doesn't seem to work as expected.

Here are the settings I've configured (which I believe should be correct):

Query String Caching: Do not cache URLs with query strings

  1. Included Query String Parameters: (empty)
  2. Included URI Patterns: ^studienangebot$
  3. However, URLs like /studienangebot?fTitle=bachelor are still being cached, even though they shouldn't be.

Interestingly, if I change the settings as follows:

Query String Caching: Do not cache URLs with query strings

  1. Included Query String Parameters: fTitle
  2. Included URI Patterns: ^studienangebot$
  3. Then, URLs like /studienangebot?fTitle=bachelor are not cached, which is the expected behavior.

In debug mode, I receive the message: Page not cached because it does not match an included URI pattern. /studienangebot?fTitle=bachelor

I’ve tried investigating further and it seems like there might be an issue with how CacheRequestService.php is handling this logic, though this is just a guess. You'd certainly have a better understanding of how this should behave.

Here’s what seem strange to me:

if (Blitz::$plugin->settings->queryStringCaching == SettingsModel::QUERY_STRINGS_DO_NOT_CACHE_URLS) {
    $allowedQueryString = $this->getAllowedQueryString($siteUri->siteId, $siteUri->uri);
    dd($allowedQueryString); // Returns '' with no included query string params, but shouldn't the function return false in this context?
    if ($allowedQueryString) {
        Blitz::$plugin->debug('Page not cached because a query string was provided with the query string caching setting disabled.', [], $url);
        return false;
    }
}

Diagnostics Report

Application Info

Installed Plugins

Loaded Modules

Blitz Plugin Settings

{
    "debug": false,
    "hintsEnabled": true,
    "cachingEnabled": true,
    "refreshCacheEnabled": true,
    "refreshMode": 3,
    "includedUriPatterns": {
        "0": {
            "siteId": "1",
            "uriPattern": ""
        },
        "1": {
            "siteId": "2",
            "uriPattern": ""
        },
        "2": {
            "siteId": "1",
            "uriPattern": "^studienangebot$"
        },
        "7": {
            "siteId": "1",
            "uriPattern": "^studienangebot\/bachelor-studiengaenge$"
        },
        "3": {
            "siteId": "1",
            "uriPattern": "^campus-wels$"
        },
        "4": {
            "siteId": "1",
            "uriPattern": "^campus-steyr$"
        },
        "5": {
            "siteId": "1",
            "uriPattern": "^campus-hagenberg$"
        },
        "6": {
            "siteId": "1",
            "uriPattern": "^campus-linz$"
        }
    },
    "excludedUriPatterns": [],
    "cacheStorageType": "putyourlightson\\blitz\\drivers\\storage\\FileStorage",
    "cacheStorageSettings": {
        "folderPath": "@webroot\/cache\/blitz",
        "compressCachedValues": ""
    },
    "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": false,
    "ssiTagFormat": "<!--#include virtual=\"{uri}\" -->",
    "detectSsiEnabled": true,
    "esiEnabled": false,
    "queryStringCaching": 0,
    "includedQueryStringParams": "",
    "excludedQueryStringParams": [
        {
            "siteId": "",
            "queryStringParam": "gclid"
        },
        {
            "siteId": "",
            "queryStringParam": "fbclid"
        }
    ],
    "apiKey": "",
    "generatePagesWithQueryStringParams": true,
    "purgeAssetImagesWhenChanged": true,
    "refreshCacheAutomaticallyForGlobals": true,
    "refreshCacheWhenElementMovedInStructure": true,
    "refreshCacheWhenElementSavedUnchanged": false,
    "refreshCacheWhenElementSavedNotLive": false,
    "cacheNonHtmlResponses": false,
    "trackElements": true,
    "trackElementQueries": true,
    "excludedTrackedElementQueryParams": [],
    "cacheElements": true,
    "cacheElementQueries": true,
    "cacheDuration": null,
    "nonCacheableElementTypes": [],
    "sourceIdAttributes": [],
    "liveStatuses": [],
    "integrations": [
        "putyourlightson\\blitz\\drivers\\integrations\\CommerceIntegration",
        "putyourlightson\\blitz\\drivers\\integrations\\SeomaticIntegration"
    ],
    "defaultCacheControlHeader": "no-store",
    "cacheControlHeader": "public, s-maxage=31536000, max-age=0",
    "cacheControlHeaderExpired": "public, s-maxage=5, max-age=0",
    "sendPoweredByHeader": true,
    "outputComments": true,
    "refreshCacheJobPriority": 10,
    "driverJobBatchSize": 100,
    "driverJobPriority": 100,
    "queueJobTtr": 300,
    "maxRetryAttempts": 10,
    "maxUriLength": 255,
    "mutexTimeout": 1,
    "commands": [],
    "injectScriptEvent": "DOMContentLoaded",
    "injectScriptPosition": 3
}

Recommendations

Site Tracking [1]

Site Tracking [2]

Site Tracking [3]

Site Tracking [4]

bencroker commented 1 week ago

Thanks for the report. In Discord I sent you https://github.com/putyourlightson/craft-blitz/commit/0da5186d69cca474a0ef0c05faaee8a48566943d, which addresses this. Did you clear the cache and retest?

samuelreichor commented 1 week ago

Yes I also deleted vendor and made a fresh new install. I flushed and deleted the cache.

bencroker commented 1 week ago

Ok, I'll troubleshoot and let you know what I find.

bencroker commented 6 days ago

The latest commit is working for me locally. Please ensure you’re using the latest version of the code, clear and flush the cache, and retest. If the issue persists, describe the steps you took in testing in detail so I can follow them.

samuelreichor commented 6 days ago

I tested everything again, but unfortunately, it’s still not working for me.

Test Procedure:

  1. Double-checked all settings: I confirmed that all settings are configured correctly.
    image
    image

  2. Cleared the cache using the utilities in Craft CMS.

  3. Flushed the cache again through Craft’s utilities.

  4. Updated putyourlightson/craft-blitz to this version:

    "putyourlightson/craft-blitz": "4.x-dev#0da5186d69cca474a0ef0c05faaee8a48566943d",
  5. Verified the plugin update in the vendor folder by comparing it with the latest changes in Git – everything appears correct.

  6. Checked caching for /studienangebot?fCampus=hagenberg&fTitle=bachelor: This page is served by blitz.

  7. Checked caching for /studienangebot: This page is served by blitz. (But the cached Page is wrong because of ?fCampus=hagenberg&fTitle=bachelor from the first hit)

samuelreichor commented 6 days ago

(And I tested inENVIRONMENT=production)

bencroker commented 6 days ago
  1. Checked caching for /studienangebot?fCampus=hagenberg&fTitle=bachelor: This page is served by blitz.

“Served by Blitz” doesn’t mean that the page was cached by Blitz. How are you verifying whether the page is cached?

samuelreichor commented 6 days ago

Sorry, that was not expressed correctly. I see two comments from Blitz which shows that it is served and cached by blitz

bencroker commented 6 days ago

Thanks, I was able to replicate the issue by matching your “Included Query String Parameters”. Can you please retest with the commit https://github.com/putyourlightson/craft-blitz/commit/b65c92e90430951c9b5cf11cfce24c0b45e0a06b?

samuelreichor commented 6 days ago

Ah, dang it, now the usual URI include patterns are broken. 🙈

I tested it as before, cleared and flushed the cache, but now all the 'normal' include URI patterns are being ignored. Strangely enough, the homepage is still working fine.

Here's what's showing up in the logs in debug mode:

[2024-11-07 14:40:09] Page not cached because it does not match an included URI pattern. [https://fhooe.ddev.site/studienangebot?fCampus=hagenberg&fTitle=bachelor]
[2024-11-07 14:41:27] Page not cached because it does not match an included URI pattern. [https://fhooe.ddev.site/studienangebot]
[2024-11-07 14:41:39] Page not cached because it does not match an included URI pattern. [https://fhooe.ddev.site/campus-hagenberg]
bencroker commented 6 days ago

Indeed. This should hopefully be the final fix https://github.com/putyourlightson/craft-blitz/commit/c05b92117b57d8b6b70f2358cabdabd6778b35a5.

samuelreichor commented 6 days ago

Yessss! that works now, thank you! :)

bencroker commented 6 days ago

Great, I’ll be releasing this today or tomorrow. Thanks for reporting this and for sticking with me getting through it.

bencroker commented 6 days ago

Released in 4.23.5 and 5.9.3.