search

cloudflare / Cloudflare-WordPress

A Cloudflare plugin for WordPress
https://www.cloudflare.com/wordpress/
BSD 3-Clause "New" or "Revised" License
215 stars 83 forks source link

No cache when added to cart #427

Closed erikdemarco closed 1 year ago

erikdemarco commented 3 years ago

Confirmation

WordPress version

5.8

Cloudflare-WordPress version

4.5

PHP version

8

Expected result

The correct behaviour should be: -) Cache invalidate only when origin server send cookie header. -) Cookie exist in request header shouldn't invalidate cache. If you do it will make hit ratio so low.

Actual result

When "woocommerce_" cookie exist in request header it invalidate cache

Steps to reproduce

  1. Install woocommerce
  2. Browse page when logged out
  3. Add something to cart
  4. Browse any page, cache will be bypassed

Additional factoids

  1. Woocommerce is already smart enough, when you request a page. It will refresh the minicart automatically with no cache header.
  2. What is the first purpose people installing cloudflare plugin? To cache most of the request, so it will save bandwidth and resources of origin server. But if you invalidate cache just because there is a "wp" or "woocommerce" cookie in the request header. It's very wrong. We only need to invalidate cache when origin server send response header containing a cookie.
  3. Compare cloudflare official plugin with WP cloudflare and litespeed cache. Or any disk cache plugin Or any static CDN . All of them works like I mention. They invalidate cache if the origin site have cookie in response header or the user is logged in. Not invalidate cache when request header contain cookie for logged out user. Nothing works like cloudflare do. Can you please tell me the reasoning CF developer do this?
  4. What is the advantage of paying $5/month for APO. But getting much more lower cache hit ratio than using WP cloudflare plugin with free plan?

References

No response

jacobbednarz commented 3 years ago

I don't think this is something the plugin should be solving as the use cases vary greatly and I don't think we could cover all use cases (plugin, themes). Instead, you can adjust your own Page Rules to handle cache negotiation on particular routes. Generally, I recommend using the "respect origin cache control" setting for most pages and setting your cache TTLs and directives in the application side for full control.

jacobbednarz commented 3 years ago

What is the first purpose people installing cloudflare plugin? To cache most of the request, so it will save bandwidth and resources of origin server. But if you invalidate cache just because there is a "wp" or "woocommerce" cookie in the request header. It's very wrong. We only need to invalidate cache when origin server send response header containing a cookie.

You can see the features list in the readme.txt

Compare cloudflare official plugin with WP cloudflare and litespeed cache. Or any disk cache plugin Or any static CDN . All of them works like I mention. They invalidate cache if the origin site have cookie in response header or the user is logged in. Not invalidate cache when request header contain cookie for logged out user. Nothing works like cloudflare do. Can you please tell me the reasoning CF developer do this?

Cloudflare can be configured to work however you need it to. The decision of how is up to you and how your system works.

What is the advantage of paying $5/month for APO. But getting much more lower cache hit ratio than using WP cloudflare plugin with free plan?

Check out https://developers.cloudflare.com/automatic-platform-optimization/. It has the explanation, feature list and ways to confirm it works for you.

erikdemarco commented 3 years ago

@jacobbednarz Hi, Yes I tried "respect origin cache control" before, And doesnt work. Maybe APO have more priority than this setting?

sejoker commented 3 years ago

@erikdemarco thanks for bringing this topic for discussion.

I tried "respect origin cache control" before, And doesnt work. Maybe APO have more priority than this setting?

Correct APO TTL has higher priority that cache-control header. For this use case I'm planning to work on adding support for cdn-cache-control and cloudflare-cnd-cache-control that could override APO TTL: https://community.cloudflare.com/t/cache-everything-json-expiring-at-00h-utc-everyday/285269/17?u=yevgen

Compare cloudflare official plugin with WP cloudflare and litespeed cache. Or any disk cache plugin Or any static CDN . All of them works like I mention. They invalidate cache if the origin site have cookie in response header or the user is logged in. Not invalidate cache when request header contain cookie for logged out user. Nothing works like cloudflare do. Can you please tell me the reasoning CF developer do this?

With APO we tried to be cautious. Caching something that shouldn't be cached is the worst result for a product as APO. I will research your claims here, maybe we can support your suggested idea.

erikdemarco commented 3 years ago

For this use case I'm planning to work on adding support for cdn-cache-control and cloudflare-cnd-cache-control that could override APO TTL:

@sejoker This is very nice feature. So for example I want cloudflare to cache product page. I just place "cdn-cache-control" in response header and CF will respect this even if user have "woocommerce_" in request header. Right? And how to make APO cache product page only for logged out user and do not cache for logged in user? Because as far as I know the cache key for KV storage is 'a combination of a zone identifier and the URL'

With APO we tried to be cautious. Caching something that shouldn't be cached is the worst result for a product as APO. I will research your claims here, maybe we can support your suggested idea.

I confirmed it again with the most popular cache plugin w3 total cache, wp fastest cache. and the most popular cloudflare plugin for wordpress, WP Cloudflare Super Page Cache. All still return cache when add to cart cookie exist

sejoker commented 3 years ago

I think this is a valid use case we should investigate for APO.

github-actions[bot] commented 2 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

pjv commented 2 years ago

Unstaling the issue because this would make a pretty big performance difference on busy woocommerce sites.

github-actions[bot] commented 2 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

pjv commented 2 years ago

bump.

github-actions[bot] commented 1 year ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

pjv commented 1 year ago

How did the bot mark this issue as completed when there has not been a commit in this repo in almost a year?

jordantrizz commented 1 year ago

With APO we tried to be cautious. Caching something that shouldn't be cached is the worst result for a product as APO. I will research your claims here, maybe we can support your suggested idea.

Look to other caching plugins; as mentioned before, they don't act the same. Furthermore, there should be options within the plugin for excluding URI's from APO cache and force URI's to be cached by APO via the headers mentioned above by @sejoker. Moving the control closer to WordPress would be a better overall user experience versus having to make changes to Cloudflare rules.

How did the bot mark this issue as completed when there has not been a commit in this repo in almost a year?

Pretty should they have it automated that if an issue is set to stale, it's closed as completed after a specific number of days.

Unfortunately, there isn't a huge amount of resources from Cloudflare to improve this plugin, so it's simply getting one-off updates and maintenance releases.