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

Introduce ConfigValidatorInterface for minimum runtime validation of SM configurations #189

Draft
wants to merge 2 commits into
base: 3.21.x
Choose a base branch
from

Conversation

boesing
Copy link
Member

@boesing boesing commented May 2, 2023

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.

…idation of SM configurations

Signed-off-by: Maximilian Bösing <[email protected]>
@boesing boesing added this to the 3.21.0 milestone May 2, 2023
@boesing boesing marked this pull request as draft May 2, 2023 21:47
@boesing boesing removed this from the 3.21.0 milestone May 14, 2023
@gsteel
Copy link
Member

gsteel commented Sep 12, 2023

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);
    }
}

@kynx
Copy link
Contributor

kynx commented Sep 13, 2023

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Feature: ServiceManager configuration validation
3 participants