laminas-servicemanager icon indicating copy to clipboard operation
laminas-servicemanager copied to clipboard

Introduce `ConfigValidatorInterface` for minimum runtime validation of SM configurations

Open boesing opened this issue 1 year ago • 2 comments

Q A
Documentation yes
Bugfix no
BC Break no
New Feature yes

Description

Starting with v4.0.0, the ServiceManager itself will not validate anymore as it provides psalm array-shape type for configuration values.

As per existing projects, the ServiceManager is usually configured somehow like these examples:

$smConfig = $configuration['service_manager'] ?? [];
$smConfig = new ServiceManagerConfig($smConfig);

$serviceManager = new ServiceManager();
$smConfig->configureServiceManager($serviceManager);
$config = require __DIR__ . '/config.php';

$dependencies                       = $config['dependencies'];
$dependencies['services']['config'] = $config;

$serviceManager = new ServiceManager($dependencies);

The first example does use deprecated Config instance which just verified that pre-defined array keys are provided. The second example does not verify anything and assumes the config to be correct.

Both cases do either need a psalm-baseline entry or a psalm-suppress once migrated to SM v4.0.0 (at least the second example already needs that as it uses the ServiceManager#__construct which is already properly typed.

Closes #131


So every pluginmanager providing component (such as laminas-cache, laminas-form, etc. - I've listed some more in the TSC meeting) would also need either a baseline entry or suppression due to the fact that there is no current way to verify that configuration matches expectations.

To avoid that, we could in fact use a runtime configuration validator which is provided by this PR.


I have not yet a clue on how to effectively use it, as I would actually prefer to only validate prior configuration caching so that not every request has to validate the same valid/invalid config over and over again. I've created this PR as a draft so that we can find a way of how to effectively use such a validator. In mezzio projects, having this as some kind of PostProcessor for config-aggregator would work but that won't work for i.e. laminas-form

Would love to get some help here.

boesing avatar May 02 '23 21:05 boesing

Could we wire up a service in the config provider here so that components that ship plugin managers can use that service to perform runtime validation in the plugin manager factory?

Say a service something like this:


namespace Laminas\ServiceManager;

/** @psalm-import-type ServiceManagerConfiguration from ServiceManager */
final readonly class ConfigValidationService
{
    public function __construct(private ValidatorInterface $validator, private bool $enabled)
    {
    }

    /** @psalm-assert ServiceManagerConfiguration $config */
    public function validate(array $config): void
    {
        if (! $this->enabled) {
            return;
        }

        $this->validator->assertValidConfig($config);
    }
}

… where $this->enabled is equivalent to $config['debug'] and in a plugin manager factory, something like:

final class PluginManagerFactory
{
    public function __invoke(ContainerInterface $container): ValidatorPluginManager
    {
        $config = $container->get('config');
        $services = $config['validators'] ?? [];
        
        $validator = $container->get(ValidatorInterface::class);
        $validator->validate($services);
        
        return new PluginManager($services);
    }
}

gsteel avatar Sep 12 '23 21:09 gsteel

I have not yet a clue on how to effectively use it, as I would actually prefer to only validate prior configuration caching so that not every request has to validate the same valid/invalid config over and over again.

Yeah. I'd argue that validating configuration is a job for whatever build tool you use, not a runtime check.

As a developer, if I'm changing the config it's going to be obvious pretty quickly if an alias I've introduced does not resolve. If it's set to something different in production that's a problem. But does throwing an exception when the configuration is first loaded - as opposed to when the invalid dependency is first used - really make my life any easier? The borked config should never have got so far.

Maybe these could be made into reusable smoke tests instead?

kynx avatar Sep 13 '23 10:09 kynx