Don't load cached config unless cache is enabled

[glen-84]

If you disable caching by setting config_cache_enabled to false, it still loads the cached config file.

Fix:

    public function __construct(
        array $providers = [],
        $cachedConfigFile = 'data/cache/app_config.php'
    ) {
        $cacheEnabled = (isset($config[static::ENABLE_CACHE]) && $config[static::ENABLE_CACHE] === true);

        if ($cacheEnabled && is_file($cachedConfigFile)) {
            // Try to load the cached config
            $this->config = require $cachedConfigFile;
            return;
        }

        $config = $this->loadConfigFromProviders($providers);

        // Cache config if enabled
        if ($cacheEnabled) {
            file_put_contents($cachedConfigFile, '<?php return ' . var_export($config, true) . ";\n");
        }

        $this->config = $config;
    }

[mtymek]

@glen-84 your solution won't work. Whole idea of caching is that when enabled, it does not iterate over provider list, improving performance. Adding this feature would be the same as if cache was not enabled at all.

Cache is meant to be enabled on production environment only. Ideally you should regenerate it every time you deploy new version of your application.

[glen-84]

Whole idea of caching is that when enabled, it does not iterate over provider list, improving performance.

And how does my code change that? The only difference in the code is that it will check that the cache is enabled before attempting to read from the file. This will have no effect on a production environment.

As it stands, if I run my code locally in production mode, and then switch to development mode, it will load the production configuration even though caching is disabled. This is not correct.

[mtymek]

See your proposed code - you are referring to $config[static::ENABLE_CACHE] before $config is created. So isset() will always return false - that's why I wrote that "your solution won't work".

I'm not sure how it could work for you.

[glen-84]

Ohhh, sorry. I see what the problem is – you have to load the configuration in order to know whether or not caching is enabled.

I don't know if it makes sense to include this setting in the configuration itself. What do you think about a parameter to the constructor instead?

public function __construct(
    array $providers = [],
    $cacheEnabled = false,
    $cachedConfigFile = 'data/cache/app_config.php'
)

It's weird that you have to load the config in order to find out whether or not to cache that same config.

The updated code (not tested, again):

    public function __construct(
        array $providers = [],
        $cacheEnabled = false,
        $cachedConfigFile = 'data/cache/app_config.php'
    ) {
        if ($cacheEnabled && is_file($cachedConfigFile)) {
            // Try to load the cached config
            $this->config = require $cachedConfigFile;
            return;
        }

        $config = $this->loadConfigFromProviders($providers);

        // Cache config if enabled
        if ($cacheEnabled) {
            file_put_contents($cachedConfigFile, '<?php return ' . var_export($config, true) . ";\n");
        }

        $this->config = $config;
    }

Example usage:

$configManager = new ConfigManager([], (APPLICATION_ENV !== 'development'));

Otherwise you'll have to delete the config file each time that you switch environments.

Edit: This would deprecate the config_cache_enabled option.

Edit 2: As it stands, you also won't be able to run both environments at the same time (on different ports), for local testing.

[mtymek]

Using configuration key has the advantage that you can enable caching by adding new *.local.php file into your config directory. This is recommended way.

Having said that, I also feel that testing this feature on local environment can be made easier. So, I propose following syntax:

 public function __construct(
        array $providers = [],
        $cachedConfigFile = null
    ) {
// ...
}

If $cacnedConfigFile is set to NULL, cache file is disabled.

[glen-84]

Using configuration key has the advantage that you can enable caching by adding new *.local.php file into your config directory. This is recommended way.

That's true. I'm personally using a setup like this:

In index.php:

// Set application environment.
define('APPLICATION_ENV', (getenv('APPLICATION_ENV') ?: 'development'));

This is a useful constant to have for doing other environment-specific switches in the code.

I then have, in config.php:

$envMap = [
    'development' => 'dev',
    'production'  => 'prd',
    'staging'     => 'stg'
];

$env = ($envMap[APPLICATION_ENV] ?? 'dev');

$configManager = new ConfigManager([
    new PhpFileProvider(sprintf('config/autoload/{,*.}{global,%s,local}.php', $env))
]);

This way I can have files like error-handler.dev.php that always load in the development environment, but can also be committed and shared with other developers. Environment-specific config overrides global (all environments) config, which can then be overridden with local (not committed) config.

If $cacnedConfigFile is set to NULL, cache file is disabled.

That also works. =)

[glen-84]

What about:

    public function __construct(
        array $providers = [],
        Psr\Cache\CacheItemPoolInterface $cache = null,
        $cacheKey = 'default-cache-key-here'
    ) {
        // ...
    }

Then we can use any cache that implements PSR-6. =)

[glen-84]

Any updates here? I see that there is a related PR.