What are Config Files and how do we improve Config?
I have questions about Config.
the current Routes file isn't what I would consider a config file https://github.com/codeigniter4/CodeIgniter4/pull/7380#issuecomment-1483988974
Then, what are Config Files? or what are Config items?
Config items should be immutable or mutable?
We should use array for Config instead of class? What is the advantage of array-config?
App\Config causes infinite loop now (See #7308). How do we solve it?
Checking the environment variables for all properties when instantiating the Config class is too heavy. This is one of the reasons why CI4 is slower than CI3. I want to improve this too.
Forbid using the constructor in configs. https://github.com/codeigniter4/CodeIgniter4/pull/7380#issuecomment-1484782188
What if I want to use a value in the database for a config value?
I don't think this is a bug, so shouldn't it be discussed in the forums?
What if I want to use a value in the database for a config value?
Config files are typically loaded before a database would be, so you hit issues there.
I don't think this is a bug, so shouldn't it be discussed in the forums?
I don't think most maintainers see the forum.
And the source code is hard to read on the forum, and we can't just link to it and see the code like you can on GitHub. It's hard to have detailed discussions.
Config files are typically loaded before a database would be, so you hit issues there.
Database seems working in Config\App.
See https://github.com/codeigniter4/CodeIgniter4/issues/4297#issuecomment-989808036
Database seems working in Config\App.
IIRC it will work for some config files but not others. Whichever ones that the database layer might grab will cause an issue, and that's more than just the database config file. I've ran into this in the past and I don't remember which specific ones caused the issue.
What if I want to use a value in the database for a config value?
The I recommending using CodeIgniter's official solution - Settings. :)
I don't think most maintainers see the forum.
They will see it on Slack, though. We've always told people Issues are for bug reports, forum was for feature requests.
What if I want to use a value in the database for a config value?
First, I will give an example of Laravel.
- Environment variables are loaded.
- Configs are loaded.
- ...
- ServiceProviders are loaded (ServiceProviders register services in the container via the register() method)
- For all loaded ServiceProviders, the boot() method is called.
There is an application ServiceProvider like AppServiceProvider. And in the AppServiceProvider::boot() method, you can add some logic that will definitely be called after the configs and before the request is processed.
I want to say that for the problem you described, there are several ways to solve it.
We need a class that will work in the same way as AppServiceProvider::boot()
Or, for these purposes, we can use the pre_system event, which is called immediately before the request is processed.
I don't think this is a bug, so shouldn't it be discussed in the forums?
From my experience, I can say that the feedback from the CI team on the forum is close to zero. The only maintainer who is active is @kenjis . But as you can see, even he does not rely on the forum as a place for discussion.
@iRedds My question is like this if using Laravel. How do I set the values of Config files from the database data? Probably I can't?
We've always told people Issues are for bug reports, forum was for feature requests.
I think it is better to use GitHub Discussions if you don't like to use GitHub IIssues.
@kenjis I am not using Laravel. I look at its code base to understand how these or those solutions are implemented. I suppose that changes in configs at runtime in Laravel can be done as follows.
// config myconfig.php
return [
'key' => 'default',
'key2' => 'default2'
];
// AppServiceProvider
public function boot(): void
{
// Here we override the config values, for example, by taking the values from the database.
config(['myconfig.key' => 'new value']);
// or for multiple keys
config(['myconfig' => ['key' => 'new ', 'key2' => 'new 2']]);
}
// using
Route::get('/', function () {
dd(config('myconfig'));
});
Now, by referring to the config, in the route we will receive the following data.
array:2 [▼ // routes\web.php:18
"key" => "new "
"key2" => "new 2"
]
How do I set the values of Config files from the database data?
You can't, they are just simple arrays. You would need to do a separate layer, much like Settings does currently. f
From my experience, I can say that the feedback from the CI team on the forum is close to zero. The only maintainer who is active is @kenjis . But as you can see, even he does not rely on the forum as a place for discussion.
That's why we have a Slack channel for the core team. Discussions could happen there and if there was a need for input on the forums we could be pointed that way with a Slack message. I just feel that if we have been telling users that Issues are for bugs, we should listen to that advice ourselves.
Back to the original discussion, though:
App\Config causes infinite loop now (See https://github.com/codeigniter4/CodeIgniter4/issues/7308). How do we solve it?
We don't. There is no need for session with a config file, as a config file is intended to hold the settings that configure how the application works and not respond to each user's individual session settings.
We should use array for Config instead of class?
I'm not opposed to going back to this for v5. We obviously cannot do it earlier. It would be lighter than the class-based configuration we currently use, and better for memory as optimized as PHPs arrays are now. I don't know how much the performance gains would be, compared to the database layer, which is our slowest part of the framework now, I believe.
There are a few config classes that provide methods that would have to be moved elsewhere. To be honest, though, I've only ever heard a couple people mentioning it being an issue, so we would need to consider the pros/cons/cost in manpower, and I think it would be good to get a bigger set of opinions on the forums. There's probably several topics we should do that about so we can start actually planning for v5 more.
Checking the environment variables for all properties when instantiating the Config class is too heavy.
IIRC the loading of environment vars only happens once, and would still need to happen with an array-based config. I haven't measured the cost of looping over the class vars in each class, but I would be surprised if it was too onerous. Would be interesting to see benchmarking on that.
It is possible to cache the database loaded configs?
Edit:
Doesn't seem possible. The cache class is loaded later after Config\App.
We don't. There is no need for session with a config file, as a config file is intended to hold the settings that configure how the application works and not respond to each user's individual session settings.
Sessions as an example. If the code that refers to the same config is called in the config constructor, then a loop occurs. And I guess a lot of people guessed to change configs at runtime in the config constructor.
There is no need for session with a config file, as a config file is intended to hold the settings that configure how the application works and not respond to each user's individual session settings.
Why? This is my main question. After all, what is a config item and what is not?
This is an example, but I don't think the requirement that if a locale is stored in the session and you want to set the locale based on that is so odd. And App\Config has a setting for locales $supportedLocales.
If you don't set $supportedLocales correctly, the application won't work well.
Supported locales is a static value. The current locale in the session is a dynamic value. Config files are typically for static values that don't depend on the current running state of the app.
What is a static value and what is not? Probably a static value does not change at runtime.
Then $allowedHostnames is a static value?
What if a site user has its own URL like username.example.com?
It would not be possible to define all values in a Config file.
And even if $supportedLocales is a static value, it would still be difficult to define all locales in a Config file for a site where locales are constantly being added by users.
@iRedds Thank you for showing.
I want to do something like this. Then I can see what the value is just by looking at the config file.
// config myconfig.php
$values = db_connect()->query()->...
return [
'key' => $values['key']',
'key2' => $values['key2']',
];
If AppServiceProvider is going to change the config values, it might be better that there is no config file.
If there is no config file, I only have to look at AppServiceProvider to see what the config values are.
Why does a database config solution have to be at the "config" level or even system level? As mentioned, we already provide a solution to this in the Settings library.
@kenjis your examples are correct - and they would typically be handled by some other piece in the app layer, not in the config. I'm not aware of any framework that has dynamic config values like you're discussing here. it seems like most of this would be something that likely gets processed either in an Event, or a Filter (which can modify the request as needed).
@kenjis In your example, you don't see values, only variables. Having access only to the project code, without a database, it will be difficult to understand what data the code operates on.
In addition, it already looks more like some kind of library than a config.
config without config file. It looks like some kind of perversion )
IIRC the loading of environment vars only happens once, and would still need to happen with an array-based config. I haven't measured the cost of looping over the class vars in each class, but I would be surprised if it was too onerous. Would be interesting to see benchmarking on that.
Simple benchmark to CI4 default Welcome page on my MacBook Air.
Results:
Requests/sec: 378.43 (default)
Requests/sec: 508.60 (disable to load env vars)
$ php -v
PHP 8.2.8 (cli) (built: Jul 6 2023 11:16:24) (NTS)
Copyright (c) The PHP Group
Zend Engine v4.2.8, Copyright (c) Zend Technologies
with Zend OPcache v8.2.8, Copyright (c), by Zend Technologies
$ composer update --no-dev
$ php spark env
CodeIgniter v4.3.6 Command Line Tool - Server Time: 2023-07-13 02:46:04 UTC+00:00
Your environment is currently set as production.
$ symfony server:start
$ wrk -t10 -d5s http://127.0.0.1:8000/
Running 5s test @ http://127.0.0.1:8000/
10 threads and 10 connections
Thread Stats Avg Stdev Max +/- Stdev
Latency 27.40ms 12.70ms 138.30ms 92.92%
Req/Sec 38.02 8.85 50.00 71.83%
1903 requests in 5.03s, 32.70MB read
Requests/sec: 378.43
Transfer/sec: 6.50MB
Disable to load environment variables:
--- a/system/Config/BaseConfig.php
+++ b/system/Config/BaseConfig.php
@@ -63,24 +63,24 @@ class BaseConfig
$this->registerProperties();
- $properties = array_keys(get_object_vars($this));
- $prefix = static::class;
- $slashAt = strrpos($prefix, '\\');
- $shortPrefix = strtolower(substr($prefix, $slashAt === false ? 0 : $slashAt + 1));
-
- foreach ($properties as $property) {
- $this->initEnvValue($this->{$property}, $property, $prefix, $shortPrefix);
-
- if ($this instanceof Encryption && $property === 'key') {
- if (strpos($this->{$property}, 'hex2bin:') === 0) {
- // Handle hex2bin prefix
- $this->{$property} = hex2bin(substr($this->{$property}, 8));
- } elseif (strpos($this->{$property}, 'base64:') === 0) {
- // Handle base64 prefix
- $this->{$property} = base64_decode(substr($this->{$property}, 7), true);
- }
- }
- }
+// $properties = array_keys(get_object_vars($this));
+// $prefix = static::class;
+// $slashAt = strrpos($prefix, '\\');
+// $shortPrefix = strtolower(substr($prefix, $slashAt === false ? 0 : $slashAt + 1));
+//
+// foreach ($properties as $property) {
+// $this->initEnvValue($this->{$property}, $property, $prefix, $shortPrefix);
+//
+// if ($this instanceof Encryption && $property === 'key') {
+// if (strpos($this->{$property}, 'hex2bin:') === 0) {
+// // Handle hex2bin prefix
+// $this->{$property} = hex2bin(substr($this->{$property}, 8));
+// } elseif (strpos($this->{$property}, 'base64:') === 0) {
+// // Handle base64 prefix
+// $this->{$property} = base64_decode(substr($this->{$property}, 7), true);
+// }
+// }
+// }
}
/**
$ wrk -t10 -d5s http://127.0.0.1:8000/
Running 5s test @ http://127.0.0.1:8000/
10 threads and 10 connections
Thread Stats Avg Stdev Max +/- Stdev
Latency 20.86ms 12.72ms 148.69ms 94.62%
Req/Sec 51.15 11.17 70.00 83.30%
2555 requests in 5.02s, 43.90MB read
Requests/sec: 508.60
Transfer/sec: 8.74MB
My opinion:
- All Config values are global default values and should not be personalized.
- Every Config class should be immutable.
All Config values are global default values and should not be personalized.
How do you propose to handle per-environment values then? Adding them to the code itself is a non-starter. Best practices and security say you should never commit a number of your environment variables to a repo.
It's likely that, if we were doing a major overhaul of the config system that we could refactor the env var use with config files to be faster. Likely by being less "automatic" and doing it something a little more reliant on the environment variables themselves, instead of looping over them at instantiation and assigning them within the class. This would also likely allow us to not have to extend the BaseConfig class anymore.
arrays :laughing:
@lonnieezell My English may not be correct, but I mean the handling per-environment values is not changed. Environment variables are pulled and they override Config properties when a Config class is instantiated.
What I wanted to say by "not be personalized" is that the Config value is the value for the system, not for the site visitor.
The proposal to change to arrays is not yet clear on how to implement it concretely.
Assuming that the Config values are written in an array, how do we pass the values to the class? And what about compatibility issues? In order to maintain compatibility, both Config classes and array files should be supported, at least for a certain period of time.
And with a Config class, we can jump from the property code to the source class in the IDE. With arrays, however, we cannot jump to the source of the definition. Our DX will be worse.
Pass an array instead of a class. Since we use classes as configs and specify the type hint in class constructors, we need to have an application for testing the framework.
By the way, Laravel has an interesting approach. It is not the config that is passed to the classes (services), but the application instance, which is inherited from the service container class that implements the ArrayAccess interface. I'll try to show it schematically.
class Some
{
/**
* @var Application
*/
protected $app;
/**
* @param Application $app
*/
public function __construct($app)
{
// Here the `config` key is the name of the service in the container.
$this->app['config']['config_name.config_var'];
}
}
// And in tests it is enough to create an array.
new Some([
'config' => ['config_name' => ['config_var' =>'value']]
])
Since our configs are classes, in order to temporarily change the value, you have to use an alternative class, since objects are passed by reference.
d(config(App::class)->baseURL); // config(...)->baseURL string (22) "http://localhost:8080/"
$config = config(App::class);
$config->baseURL = 'sss';
d(config(App::class)->baseURL); // config(...)->baseURL string (3) "sss"
When migrating from 3.x to 4.x, a soft transition was announced, although configs from arrays were replaced with classes. So v5 can return arrays in the same way.
And with a Config class, we can jump from the property code to the source class in the IDE.
Weak advantage.
Laravel approach makes the design worse. All classes depends on the service container (Application).
I don't think a hard transition from classes to arrays is desirable.
By the way, what is the benefit of changing to arrays? Changing from arrays to classes in v4 and then back to arrays again in v5 is ridiculous unless there is a huge advantage.
By the way, what is the benefit of changing to arrays?
- I repeat what I said above. That arrays are not passed by reference and changing the value will not affect the state of the config.
- In an array, you can only assign a value from dotenv to a specific key. In objects, for this, we iterate over all the properties of the class in a loop. (By the way,
foreachcopies the passed array, which increases memory consumption at the moment.) - Arrays do not have a constructor, which means there is no chance of getting an infinite loop, as it can now happen with rash use.
- You can include another config file in an array.
return ['key1' => 'value1', 'key2' => include 'some_config.php'];
return ['key1' => 'value1'] + include 'some_config.php';
- In objects, for this, we iterate over all the properties of the class in a loop.
- Arrays don't need a namespace, and forcing third-party library config files to be moved to the config directory will reduce search overhead.
- With arrays, it is easier to organize the access to the value through the dot notation.
- There is less overhead when restoring an array from the cache than for objects.
ps: It surprises me what garbage they came up with with configs. More precisely with access to them. Some strange factory. Instead, there could be a regular service that stores configs in itself.