Skip to content

Settings - Deprecate "serialize" flag

Background: Basic settings

Settings are "key-value" pairs. The "value" part allows type mixed (ie string or int or float or array or whatever).

Support for mixed is consistent between Civi::settings() and $civicrm_setting.

// 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:

Background: serialize metadata

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: contact_view_options, contact_edit_options, advanced_search_options, user_dashboard_options, address_options, contact_autocomplete_options, contact_reference_options, and quicksearch_options.

In all cases, the setting has a serialization format of SERIALIZE_SEPARATOR_BOOKEND.

APIv4's 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:

https://docs.civicrm.org/dev/en/latest/framework/setting/#settings-definition

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 $civicrm_setting, Civi::setting(), and Setting.get should 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 mailing_backend between 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 json_encode() instead of serialize()? 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

Simplest thing:

  • Keep the mechanism
  • Present it as deprecated
Edited by totten
To upload designs, you'll need to enable LFS and have an admin enable hashed storage. More information