Code-level changes

[ostark]

Design goals & principles

The Hollywood_principle / IoC

Relying on the Service Locator leads to spaghetto, because it makes it very easy to access everything from everywhere.

Only the main plugin class should be the glue between Craft and the code we own. As well our code should be designed with low coupling and clear separation of concerns in mind.

Tests

Writing tests should be fun! If it feels hard, most likely the principle above wasn't applied. Don't be afraid to change the actual code to make tests easier.

Each unit should be covered by tests, but don't be fanatic about 100% coverage.
Start with the happy path, but don't forget to cover edge cases.

[ostark]

What to start with?

1. The most obvious one: We will target Craft 4.1

   upper/composer.json

   Lines 26 to 29 in 96751f6

   	"require": {
   	"craftcms/cms": "^3.2.0",
   	"guzzlehttp/guzzle": "^6.5.5|^7.2.0"
   	},

2. Pest & phpstan

   upper/composer.json

   Lines 30 to 32 in 96751f6

   	"require-dev": {
   	"vimeo/psalm": "^4.4"
   	},

3. Bootstapping

   upper/src/Plugin.php

   Lines 50 to 80 in 96751f6

   	public function init()
   	{
   	parent::init();
   	// Config pre-check
   	if (!$this->getSettings()->drivers || !$this->getSettings()->driver) {
   	return false;
   	}
   	// Register plugin components
   	$this->setComponents([
   	'purger' => PurgerFactory::create($this->getSettings()->toArray()),
   	'tagCollection' => TagCollection::class
   	]);
   	// Attach Behaviors
   	\Craft::$app->getResponse()->attachBehavior('cache-control', CacheControlBehavior::class);
   	\Craft::$app->getResponse()->attachBehavior('tag-header', TagHeaderBehavior::class);
   	// Register event handlers
   	EventRegistrar::registerFrontendEvents();
   	EventRegistrar::registerCpEvents();
   	EventRegistrar::registerUpdateEvents();
   	if ($this->getSettings()->useLocalTags) {
   	EventRegistrar::registerFallback();
   	}
   	// Register Twig extension
   	\Craft::$app->getView()->registerTwigExtension(new TwigExtension);
   	}

   • Some kind of settings-pre-check ✅
   • Init Behaviours ✅
   • Init Twig extension, but better inject the Response if possible
   • The whole EventRegistrar with static methods is a mess! 💥 Better split up the Event listeners
   • Settings should be injected and not pulled from everywhere
   • This should be the only place where Craft::$app is used

[ostark]

The current CachePurgeInterface is probably okay, but I would avoid the abstract class and use a decorator instead, which is a bit more flexible.

What about something like this?:

<?php

class Purger implements CachePurgeInterface 
{
    private CachePurgeInterface $actual;

    public configure(PurgerConfig $config) {
        $this->actual = resolve($config->className);
        $this->actual->configure($config);
    }


    public function purgeUrls(array $urls): bool {    
    	// SOMETHING BEFORE 
    	$result = $this->actual->purgeUrls($urls);
    	// SOMETHING AFTER 

    	return $result;
    }


    public function purgeAll(array $urls): bool {
    	// SOMETHING BEFORE 
    	$result = $this->actual->purgeAll();
    	// SOMETHING AFTER 

    	return $result;
    }
}

[ostark]

There is no need for CacheControlBehavior to be a Behavior. Instead, it could be a simple class that interacts with HeaderCollection.

<?php
class CacheControl 
{
     public function __construct(private HeaderCollection $headers) {}
}

[ostark]

WIP #69