Settings - Deprecate "serialize" flag
Background: Basic settings
Settings are "key-value" pairs. The "value" part allows type
array or whatever).
mixed is consistent between
// Consistent access to a scalar Civi::setting()->set('my_string', 'abc'); $civicrm_setting['domain']['my_string'] = 'abc'; assert Civi::setting()->get('my_string') === 'abc';
// Consistent access to an array Civi::setting()->set('my_array', [1,2,3]); $civicrm_setting['domain']['my_array'] = [1,2,3]; assert Civi::setting()->get('my_string') === [1,2,3];
Under the hood, values are stored in MySQL using
serialize(). You can see this in both recent and ancient code:
There is an additional option in
*.setting.php metadata called
serialize. Superficially, this resembles
serialize metadata for DAO fields.
This option is defined for 8 settings:
In all cases, the setting has a serialization format of
Setting is a wrapper for
Civi::settings(). Notably, it interprets the
serialize metadata and applies an (additional/secondary) serialization.
What's good about this?
It makes it easier for REST/AJAX agents to read and write those 8 historical settings (the pre-existing settings with the annoying BOOKEND format).
At the time of introduction, it probably had no effects on compatibility and required no upgrade steps.
What's bad about this?
It encourages people to add unnecessary serialization layers. If you are writing a new setting with structured data, then you'll see the
serialize metadata and think: "I should add that!" Here are some docs that suggest
JSON for a setting:
But you shouldn't do that on a new setting! Here's why:
It adds complexity -- double-encoding. Every read and write of the data goes through both
serialize()and some additional serialization method.
It adds complexity -- conflicted APIs. The setting can be viewed through various programmatic interfaces (e.g. local PHP array
$civicrm_setting; local PHP method
Civi::setting()->get(); e.g. remote REST method
Setting.get). Some APIs present an
array-- but others present an encoded string (like
^1^2^3^). There is no decent reason why
Setting.getshould present it differently.
It distracts from the real data-modeling problem of serialized fields. We generally do simple settings (strings/ints) because they're easier. Complex settings (like
mailing_backend) are possible, but we don't do them as much - because the tooling/validation/etc is not as good. Intuitively, complex settings could work better if we added more metadata. But this is the wrong metadata. Switching
serialize/JSON/CSV/YAML/etc doesn't make this any better. What you need is a sharper description of the data inside of that serialized blob.
It becomes overall harder to change. Perhaps settings should be stored with
serialize()has additional kinds of vulnerabilities. JSON has better support in MySQL. Plot out the path for how to migrate the entirety of
civicrm_setting.value-- and compare that to switching serialization on a setting-by-setting basis. It's easier to change if individual settings are not committed to specific serializers.
What to do
- Keep the mechanism
- Present it as deprecated