PLANET-5870: Cache clear command #1286
Conversation
Inwerpsel
force-pushed
the
cache-clear-command
branch
3 times, most recently
from
4365799
to
af2e1dd
Compare
Inwerpsel
force-pushed
the
cache-clear-command
branch
2 times, most recently
from
940c267
to
6332e32
Compare
* Option to purge all posts and per post type. * Output link to test instance circleCI page
Inwerpsel
force-pushed
the
cache-clear-command
branch
from
845aa1f
to
371c0bf
Compare
There was a problem hiding this comment.
Also check Sonar for code smell and complexity https://sonarcloud.io/project/issues?id=greenpeace_planet4-master-theme&pullRequest=1286&resolved=false&types=CODE_SMELL (not always fixable but interesting)
src/Commands.php
Outdated
| * Put the CF API key into the options table, where the CF plugin uses it from. | ||
| */ | ||
| $put_cf_key_in_db = static function ( $args ) { | ||
| $hostname = $args[0]; |
There was a problem hiding this comment.
undefined offset if no args is given, ?? null would be nice :)
src/Commands.php
Outdated
|
|
||
| $domain_parts = explode( '.', $hostname ); | ||
|
|
||
| $root_domain = implode( '.', array_slice( $domain_parts, - 2 ) ); |
There was a problem hiding this comment.
Did we talk about this before ? 😬 I guess it's ok because we work mainly on greenpeace.org domain; I've used this lib https://github.com/jeremykendall/php-domain-parser in other projects, as it's impossible to guess the real root domain without a list of existing suffixes (domains in .co.uk woud fail here, etc.)
There was a problem hiding this comment.
Agree it's a shaky way to get the root domain, but I only moved this code from Loader without changes, so I rather not blow up this PR by adding a new dependency 😅 We can look at it in a separate PR 👌
src/Commands.php
Outdated
| if ( ! defined( 'CLOUDFLARE_PLUGIN_DIR' ) ) { | ||
| define( 'CLOUDFLARE_PLUGIN_DIR', WP_PLUGIN_DIR . '/cloudflare/' ); | ||
| } | ||
| require_once CLOUDFLARE_PLUGIN_DIR . 'vendor/autoload.php'; |
There was a problem hiding this comment.
regular autoload doesn't work for this ? (it works locally without it at least)
There was a problem hiding this comment.
Not sure if it's really "autoloading" it, the classes happen to be loaded already at the point of execution. But if we don't add this then there's no explicit way of saying where these classes are coming from. If the plugin is not present this should also make it fail as early as possible.
src/Commands.php
Outdated
|
|
||
| if ( isset( $assoc_args['urls'] ) ) { | ||
| $urls = explode( ',', $assoc_args['urls'] ); | ||
| } elseif ( $assoc_args['all'] ) { |
Inwerpsel
force-pushed
the
cache-clear-command
branch
from
16d4369
to
dcd2596
Compare
I followed Sonar and actually glad I did, it's much nicer to have each command in its own file. It's still tripping over a too long line and the runtime exception, but I don't think the latter is an issue for a command. Also Squiz doesn't understand |
Inwerpsel
force-pushed
the
cache-clear-command
branch
from
dcd2596
to
b5bbb0f
Compare
Inwerpsel
force-pushed
the
cache-clear-command
branch
from
b5bbb0f
to
a0cf7fd
Compare
Inwerpsel
force-pushed
the
cache-clear-command
branch
from
d4cb548
to
b451af8
Compare
Ref: https://jira.greenpeace.org/browse/PLANET-5870
Adding this as a command first so that we can make it a post-deploy step for test instances. We can start that way and observe if purging all html on each deployment causes any issues. Then we can set it up for a few production NROs and see how it goes. If it doesn't cause issues (e.g. Cloudflare deciding to evict us from cache more frequently once they pick up these entries usually don't live long), we could do this for all sites on each deploy.
I added a feature flag for this, so that we're not blocked to use it on production by needing more code changes. Tested off and on behavior on https://app.circleci.com/pipelines/github/greenpeace/planet4-test-janus/37/workflows/a3b88549-3fb8-4d73-926b-2e42e61b34ad/jobs/199