Plugin config params in config.php throw a strict standards notice: "Creating default obect from empty value"

Bug #1327921 reported by Aaron Wells
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Mahara
Fix Released
Medium
Aaron Wells

Bug Description

In bug 1196781, Mahara 1.9, we added the ability to set plugin configuration parameters in config.php. The syntax I suggested for this is a chain of nested objects, e.g:

 $cfg->plugin->search->elasticsearch->host = 'localhost';

However this throws a PHP strict standards notice, e.g. "Creating default object from empty value in config.php on line 90". And in PHP 5.4, which more and more people are using, strict standards notices are included in E_ALL, which is the default for many servers. Resultantly, any users who are using plugin config params in config.php are likely to get this notice clogging up their logs. We disable E_STRICT notices in error.php, but config.php is loaded and evaluated before error.php

You can of course manually initialize each object in the chain, but that's not very concise.

 $cfg->plugin = new stdClass();
 $cfg->plugin->search = (isset($cfg->plugin->search) ? $cfg->plugin->search : new stdClass());
 $cfg->plugin->search->elasticsearch = new stdClass();
 $cfg->plugin->search->elasticsearch->host = 'localhost';

And it's a bit awkward for our deployment process, which previously was able to store the configs in a database as a set of (key, value) pairs and then just churn those into PHP statements of the sort "\$cfg->{$key} = {$value};". If we needed to initialize each object as well, that would be a lot trickier.

So the current workaround we're using is just to put an @ before it:

 @$cfg->plugin->search->elasticsearch->host = 'localhost';

Anyway, after giving this some thought, I think the best way to go is to replace the "->"'s with underscores _. So this becomes:

$cfg->plugin_search_elasticsearch_host = 'localhost'.

This has these ramifications:

1. After loading up config.php we'll need to locate all the configs that start with "plugin_" and parse them into proper plugin configs.

2. The parsing will follow this regular expression: \^plugin_([a-z]+)_([a-z]+)_(.+)$\. The first capture group is the plugin type, the second capture group is the plugin name, and the third capture group is the config field within that plugin.

3. Core config parameters are consequently forbidden from beginning with "plugin_".

Revision history for this message
Mahara Bot (dev-mahara) wrote : A patch has been submitted for review

Patch for "master" branch: https://reviews.mahara.org/3419

Revision history for this message
Mahara Bot (dev-mahara) wrote : A change has been merged

Reviewed: https://reviews.mahara.org/3419
Committed: http://gitorious.org/mahara/mahara/commit/e5744df3bce36b6d4a2b7ac34f19de4171980ba7
Submitter: Robert Lyon (<email address hidden>)
Branch: master

commit e5744df3bce36b6d4a2b7ac34f19de4171980ba7
Author: Aaron Wells <email address hidden>
Date: Mon Jun 9 15:39:02 2014 +1200

Change the format for plugin configs in config.php (Bug 1327921)

Old format: $CFG->plugin->blocktype->clippy->agent = 'merlin';
New format: $CFG->plugin_blocktype_clippy_agent = 'merlin';

The old format caused a strict standards warning.

Change-Id: I8ae2735ea3d3df913c0527bae7771e6f01dc82ca

Revision history for this message
Aaron Wells (u-aaronw) wrote :

Tagged with "compatibility-breaking", because it breaks backwards compatibility (for those few sites that are using plugin configs set in the config.php file).

We'll need to remember to put a note about this in the 1.10.0 upgrade instructions.

Changed in mahara:
assignee: nobody → Aaron Wells (u-aaronw)
status: New → Fix Committed
tags: added: nominatedfeature
tags: added: compatibility-breaking
removed: nominatedfeature
Aaron Wells (u-aaronw)
Changed in mahara:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.