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

Pull Pantheon Advanced Page Cache into the mu-plugin [draft] #68

Open
jazzsequence opened this issue Oct 28, 2024 · 7 comments
Open

Pull Pantheon Advanced Page Cache into the mu-plugin [draft] #68

jazzsequence opened this issue Oct 28, 2024 · 7 comments
Labels
enhancement New feature or request pantheon page cache Issues that relate to the default edge caching component of the mu-plugin

Comments

@jazzsequence
Copy link
Contributor

jazzsequence commented Oct 28, 2024

tl;dr

We should pull the Pantheon Advanced Page Cache plugin into the Pantheon MU plugin.

Why?

  • PAPC provides no benefit to users outside of the Pantheon environment.
  • Many customers assume that the mu-plugin's Pantheon Page Cache does the things that PAPC does (specifically, clearing caches after pages are published or updated)
  • There is already a lot of overlap between the functionality, code and hooks in Pantheon Page Cache in the mu-plugin and Pantheon Advanced Page Cache (notably, that PAPC needed to add and use filters for things in the mu-plugin and vice versa).
  • Naming is hard. There's always been tension between the "Pantheon Page Cache" and "Pantheon Advanced Page Cache" naming conventions. By combining both, we only need one name to describe everything (whether it's "advanced" or not).
  • There is a (hypothetical) risk of supply chain vulnerabilities by only ever sourcing the plugin from the WordPress repository (if, for example, there was ever a reason where the plugin was "taken over" in the way that Advanced Custom Fields was. Note: This is perceived to be highly unlikely.)

Risks

  • PAPC is installed on 90% of Pantheon sites already. Whatever we do in the mu-plugin, it needs to remain backward compatible.
  • Standard WordPress upstreams would not receive the update automatically if they don't update WordPress to the lastest version. This means that, for at least a while, there would need to be two concurrent versions (standalone plugin and built-into-mu-plugin) of PAPC
  • Some people are still wanting the 600s cache expiration.
    • Pulling PAPC into the mu-plugin makes this configuration more difficult (but not impossible).
    • My hypothesis is that the use cases for such a short cache expiration are specific edge cases or misunderstanding the need (and/or what PAPC does). We may need to provide greater support/guidance (in the form of documentation and/or code snippets) to support cases where users actually need a 10 minute cache expiration.
  • Updates to the mu-plugin are currently coupled to WordPress updates via automation for the default WordPress upstream. Before we can consider bundling PAPC into the mu-plugin, we must decouple mu-plugin updates from WordPress updates.
    • While they are coupled, updates to the mu-plugin are hidden from users in the dashboard, which means customers might not be informed about changes to the mu-plugin.

Implementation

  • PAPC can be pulled into the mu-plugin as a library and included in the application runtime as such. However, there are a few considerations that should be taken into consideration:
    • opt-in: The "advanced" features should be able to be toggled on/off (in the case that a user legitimately does not want the benefits of PAPC). The default for all known configurations should be on.

If the site already has PAPC installed:

  • If a site already has PAPC installed (and active), we can't simply require PAPC from the mu-plugin because we'll get duplicate function errors. Since the mu-plugin doesn't have an activation hook (mu-plugins are "always on") we can't use that to deactivate the PAPC plugin when the mu-plugin is activated (which would be the traditional way of migrating a plugin to a different version). The mu-plugin can check for the existence of a setting, function or class in PAPC and, if it's found, deactivate PAPC. This would need to happen early (e.g. when the mu-plugin is loaded) so that PAPC can be deactivated before plugins are loaded.
    • The default status for the "advanced" features in this case should be on.

If the site does not have PAPC installed:

  • If a site doesn't have PAPC installed already, we can load the PAPC library and default to on.
    • Are there conditions in which we wouldn't want to default PAPC to on? List below:

If the site has a version of WordPress older than 6.4:

  • The wp.org plugin page lists 6.4 as a minimum WP version requirement for PAPC 2.0+. In fact, this is not explicitly accurate. A function was used in the Pantheon Page Cache updated styling that only exists in 6.4+ to change the input type from a text input to a dropdown, but in the case where that version is not installed, the text input is still used.
    • This means that on sites with WP versions < 6.4, a default on state of PAPC should still be valid.

If there was a problem requiring the library:

  • If for some reason there was an unrecoverable error activating the library, we should fail gracefully.
  • This means that, in those cases, we would default the PAPC setting to off.
  • This is where we could add error handling, if necessary. This should be a rarely- (or never) used code path, but could provide additional useful error handling.
    • Identifying these errors might be a challenge.

Deprecating PAPC

Pantheon Advanced Page Cache will need to push one final release around the same time that this is merged and deployed to the mu-plugin.

That update should display a deprecation notice that the plugin is not longer required if you are using the most recent version of the Pantheon mu-plugin and link to a piece of documentation (to be written, perhaps a release note) that discusses the change, the impact, the new defaults (and documentation to turn off the automattic inclusion of the "advanced" caching) and link to documentation for the filter to override the default cache max age options. A similar note should be added to the top of the readme files.

At that point, we can remove any secrets and archive the Pantheon Advanced Page Cache repository on GitHub (and Packagist).

@pwtyler
Copy link
Member

pwtyler commented Oct 28, 2024

I am not strongly against PAPC moving into the mu plugin, but the biggest loss I see to moving is our ability to iterate and version PAPC faster outside of the mu plugin as it is not tied to CMS/upstream update cadence. Divorcing upstream updates from CMS updates would ease this, but either way PAPC is essentially locked to the latest version of WordPress. Whether that's a bug or a feature likely lies in the eye of the beholder.

I do think the current split of functionality and hooks between the two can be confusing at times (tripping me up recently with #67)

The "advanced" features should be able to be toggled on/off (in the case that a user legitimately does not want the benefits of PAPC)

Is your expectation here that this would have a dashboard UI/wp_options setting, or would a filter suffice?

@jazzsequence
Copy link
Contributor Author

Is your expectation here that this would have a dashboard UI/wp_options setting, or would a filter suffice?

Specifically a visible (in the UX) toggle. We can also toggle on/off via a filter to override, but I'm thinking about people who don't want (or for now don't know that they want) to turn PAPC "on".

our ability to iterate and version PAPC faster outside of the mu plugin as it is not tied to CMS/upstream update cadence

True. However, I don't see that we're doing (or likely to do) a ton of PAPC updates such that this would be a significant issue. And, it's more a problem with the WordPress upstream deployment/update strategy than a problem with putting things into the mu-plugin specifically (we've been in this state for a long time already with not being able to "push" mu-plugin updates outside of WP releases -- and a workaround exists to just create our own Pantheon-specific releases if we needed to).

@jazzsequence
Copy link
Contributor Author

I don't see that we're doing (or likely to do) a ton of PAPC updates such that this would be a significant issue.

Also, this could be seen as a feature rather than a bug. Tying updates to WP releases means we can worry less about backwards compatibility with older versions of WordPress (except with Composer-based installs).

@jazzsequence jazzsequence added enhancement New feature or request pantheon page cache Issues that relate to the default edge caching component of the mu-plugin labels Oct 28, 2024
@stevector
Copy link

What would we do with the analogous PAPC Drupal module? I think we'd create confusion by eliminating the WordPress plugin but not the Drupal module.

@jazzsequence? Is your motivation more about eliminating the distinct plugin or more about getting it installed on every site?

@jazzsequence
Copy link
Contributor Author

jazzsequence commented Nov 13, 2024

@stevector My motivation is both.

  1. This plugin does not serve anyone not on Pantheon. The WordPress.org repository is (ideally) a place to get plugins that could be used in any context. Having a plugin that's only usable by a subset of the audience of WordPress.org doesn't make sense.
    • In the Drupal space, it's more commonly accepted for modules to be required at the site level. There's an argument to be made that we could just require it in the base upstream composer.json -- I don't know the difference between how pantheon_advanced_page_cache works differently than pantheon-advanced-page-cache but I don't think the end result is a Pantheon Page Cache page in the admin of every site that is not created by Pantheon Advanced Page Cache and, in fact, leaves important features missing, leading to confusion between "Page Cache" and "Advanced Page Cache" (which does not add an admin page).
  2. There are almost no cases where this plugin should not just be installed on every WordPress site. Could we add it to the upstream? Sure. But that requires a lot more toil and maintenance because we'd need to treat it like the pantheon-mu-plugin to pull it into the vanilla WordPress upstream and that would likely lead to conflicts (especially when customers already have it installed).

It also serves a third purpose:
3. The codebase is already mixed and confusing. The MU plugin adds pieces of "Pantheon Page Cache" functionality that are used by "Pantheon Advanced Page Cache" and "Pantheon Advanced Page Cache" -- by necessity -- has to find ways to interact with code that exists in the mu-plugin rather than its own codebase. It's a maintenance burden because it's not always clear what code exists where (and frequently it's the opposite of what you would expect). Pulling PAPC into the mu-plugin solves the maintenance burden by putting all the relevant code in one place.

What's more, this is something that people are wanting and asking for. When I have brought this up in Community Slack and office hours, I always get wholehearted support for pulling PAPC into the mu-plugin and just giving it to everyone automatically, and it solves the problem of "when I update a post on my site, it does not update?" "did you install Pantheon Advanced Page Cache?" problem.

@stevector
Copy link

Makes sense. I think the way forward is for @scottbuscemi / @DuncanSchouten to organize / prioritize this idea in Productboard.

@jazzsequence
Copy link
Contributor Author

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request pantheon page cache Issues that relate to the default edge caching component of the mu-plugin
Projects
None yet
Development

No branches or pull requests

3 participants