Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Static CDN Filters remain enabled for Logged-In Users even when Logged-In User caching is disabled #486

Closed
raamdev opened this issue May 24, 2015 · 14 comments

Comments

@raamdev
Copy link
Contributor

@raamdev raamdev commented May 24, 2015

Steps to Reproduce This

  1. Configure and enable Static CDN Filters (ZenCache → Plugin Options → Static CDN Filters)
  2. Disable Logged-In User caching (ZenCache → Plugin Options → Logged-In Users)
  3. Visit the front-end of the site as a logged-in user and inspect the source code (View Source)

Expected Behavior

Since Logged-In User caching is disabled, I would expect all ZenCache features (including Static CDN Filters) to not apply when I visit the site as a logged-in user.

Observed Behavior

Upon inspecting the page source (View Source), I see that the Static CDN Filters are being applied and static resource URLs are being rewritten, despite my visiting the site as a logged-in user.


Update (converted from bug to enhancement)

As per the comments below, I've converted this GitHub Issue to an enhancement instead of a bug. While the current behavior is unexpected, it should still be possible to have Static CDN Filters enabled for Logged-In users when caching for Logged-In users is disabled.


Next Steps

  • Add new section to ZenCache → Plugin Options → Logged-In Users titled Static CDN Filters Enabled for Logged-In Users & Comment Authors?; this section will describe how Static CDN Filters for Logged-In users is disabled by default, but that can be changed to allow Static CDN Filters to apply to Logged-In users as well. A dropdown box will provide two options: No, disabled (default) and Yes, enabled.
  • Update codebase to utilize this new option when determining if Static CDN Filters should be applied.

Related: #488

@raamdev raamdev added this to the Next Release milestone May 24, 2015
@lkraav
Copy link

@lkraav lkraav commented May 24, 2015

Instead of elimination, I would actually prefer at least an opt-in to this behavior.

@raamdev
Copy link
Contributor Author

@raamdev raamdev commented May 26, 2015

@lkraav Thanks for the feedback! That's an even better idea. :-)

@lkraav
Copy link

@lkraav lkraav commented May 26, 2015

Yes. The reasoning is that even when in development or the admin is working their site, there's plenty of motivation to still load a bunch of assets over CDN.

@isaumya
Copy link

@isaumya isaumya commented May 27, 2015

I also agree with ikraav. I think its better to give an option to the user whether or not they want this to happen. If they don't then cdn won't show for logged in users or else it will.

@raamdev
Copy link
Contributor Author

@raamdev raamdev commented May 27, 2015

@isaumya Thanks for the feedback. :-)


@jaswsinc I added a list of next steps ↑. Can you review that when you get a chance and update the list if necessary?

@jaswrks
Copy link

@jaswrks jaswrks commented May 28, 2015

@raamdev writes...

Since Logged-In User caching is disabled, I would expect all ZenCache features (including Static CDN Filters) to not apply when I visit the site as a logged-in user.

I can certainly see where this particular scenario might lead to confusion.

However, at the same time I don't see how an option for this would be truly useful. If a CDN is configured, it is always serving "static" resources. So it doesn't matter if it's a logged-in user or not. If you have a CDN, you would always want static resources served from the CDN. Right? Or am I wrong?

I can see where providing an option might help to avoid confusion though. No argument. I just can't see anyone actually turning it off for any valid reason.

@jaswrks
Copy link

@jaswrks jaswrks commented May 28, 2015

I added a list of next steps ↑. Can you review that when you get a chance and update the list if necessary?

Next actions look great to me.

@raamdev
Copy link
Contributor Author

@raamdev raamdev commented May 28, 2015

If you have a CDN, you would always want static resources served from the CDN. Right?

@jaswsinc No, especially not when you're trying to verify that ZenCache is working properly, or when a static resource (that is hosted on the CDN) appears to be loading outdated content and you're trying to figure out if it's an issue with the CDN or an issue with your server not having the updated copy.

For my own personal (single-user) sites, when I login to the WordPress Dashboard, I like to know that nothing on my site is being cached in any way, because that gives me a sense of security knowing that what I'm seeing is exactly what's on my server (so long as I've ensured my browser isn't caching anything). That's why when I inspected the source code and noticed that my static resources were being loaded from the CDN, that caught me by surprise, as I intentionally left Logged-In User Caching disabled because I didn't want ZenCache touching anything when I was logged in and browsing the site.

@jaswrks
Copy link

@jaswrks jaswrks commented May 28, 2015

That's why when I inspected the source code and noticed that my static resources were being loaded from the CDN, that caught me by surprise, as I intentionally left Logged-In User Caching disabled because I didn't want ZenCache touching anything when I was logged in and browsing the site.

Roger that. I can understand that. Thanks for explaining.

@raamdev raamdev modified the milestones: Next Release, Future Release Jun 5, 2015
@raamdev raamdev modified the milestones: Next Release (Pro), Future Release Jul 10, 2015
@raamdev raamdev modified the milestones: Next Release (Lite), Future Release (Lite) Sep 30, 2015
@raamdev raamdev modified the milestones: v151107 (Lite), Future Release (Lite) Nov 7, 2015
@jaswrks
Copy link

@jaswrks jaswrks commented Nov 12, 2015

Next Actions (Step 1 of 2)

  • New feature branch in the websharks/zencache-pro repo.

  • After this line add the following:

    'cdn_when_logged_in',
  • After this line add the following:

    'cdn_when_logged_in' => '0', // `0|1`; enable when logged in?
    
  • After this line add the following:

    /**
    * @since 15xxxx Adding logged-in check.
    *
    * @type bool CDN when logged in?
    */
    protected $cdn_when_logged_in;
  • After this line add the following:

    $this->cdn_when_logged_in = (boolean) $this->plugin->options['cdn_when_logged_in'];
  • After this line add the following:

    if (!$this->cdn_when_logged_in && $this->plugin->isLikeUserLoggedIn()) {
        return; // Disable in this case.
    }
  • Submit PR.

@jaswrks
Copy link

@jaswrks jaswrks commented Nov 12, 2015

Next Actions (Step 2 of 2 — On Deck)

  • New feature branch in the websharks/zencache-pro repo.
  • Add UI option (as described above) that allows a site owner to control:
    • cdn_when_logged_in
  • Submit PR.

Note: The above instructions for Step 2 are rather vague. If you're able, attempt step 2 right away. Otherwise, once step 1 is complete hit me on Slack for a more complete set of Step 2 instructions.

@jaswrks
Copy link

@jaswrks jaswrks commented Nov 12, 2015

Assigning this to @kristineds :-)

kristineds added a commit to wpsharks/comet-cache-pro that referenced this issue Nov 21, 2015
kristineds added a commit to wpsharks/comet-cache-pro that referenced this issue Nov 26, 2015
@raamdev raamdev modified the milestones: Next Release (Pro), Next Release (Lite) Nov 26, 2015
kristineds added a commit to wpsharks/comet-cache-pro that referenced this issue Nov 28, 2015
kristineds added a commit to wpsharks/comet-cache-pro that referenced this issue Dec 1, 2015
kristineds added a commit to wpsharks/comet-cache-pro that referenced this issue Dec 1, 2015
kristineds added a commit to wpsharks/comet-cache-pro that referenced this issue Dec 2, 2015
kristineds added a commit to wpsharks/comet-cache-pro that referenced this issue Dec 3, 2015
@raamdev
Copy link
Contributor Author

@raamdev raamdev commented Dec 4, 2015

Next Pro Release Changelog:

  • Enhancement: Static CDN Filters are no longer applied by default for Logged-In Users. This was causing some confusion because Logged-In User caching is disabled by default. There is no harm in applying Static CDN Filters to all users (logged-in or not logged-in), in fact we recommend it, however we now disable this functionality by default to avoid confusion. A new option allows you to control this behavior and enable Static CDN Filters for Logged-In users; see ZenCache → Logged-In Users → Static CDN Filters Enabled for Logged-In Users & Comment Authors? Props @kristineds, @lkraav, and @isaumya. See Issue #486.
@raamdev raamdev closed this Dec 4, 2015
@wpsharks wpsharks locked and limited conversation to collaborators Dec 21, 2015
@raamdev
Copy link
Contributor Author

@raamdev raamdev commented Dec 21, 2015

ZenCache Pro v151220 has been released and includes changes worked on as part of this GitHub Issue. See the release announcement for further details.


This issue will now be locked to further updates. If you have something to add related to this GitHub Issue, please open a new GitHub Issue and reference this one (#486).

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
5 participants