r88270 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r88269‎ | r88270 | r88271 >
Date:21:04, 16 May 2011
Author:demon
Status:resolved (Comments)
Tags:
Comment:
Initial commit of configuration management backend proposal. Feedback desired before I go much further.
* Common use case is Conf::get( 'myVar' );
* Support for default install (new `config` table) added, should be trivial to add backends for CDB, Memcache, etc...
*
Modified paths:
  • /trunk/phase3/includes/AutoLoader.php (modified) (history)
  • /trunk/phase3/includes/conf (added) (history)
  • /trunk/phase3/includes/conf/Conf.php (added) (history)
  • /trunk/phase3/includes/conf/DatabaseConf.php (added) (history)
  • /trunk/phase3/includes/conf/DefaultSettings.php (added) (history)
  • /trunk/phase3/maintenance/archives/patch-config.sql (added) (history)
  • /trunk/phase3/maintenance/tables.sql (modified) (history)

Diff [purge]

Index: trunk/phase3/maintenance/archives/patch-config.sql
@@ -0,0 +1,9 @@
 2+-- Table for holding configuration changes
 3+CREATE TABLE /*_*/config (
 4+ -- Config var name
 5+ cf_name varbinary(255) NOT NULL PRIMARY KEY,
 6+ -- Config var value
 7+ cf_value blob NOT NULL,
 8+) /*$wgDBTableOptions*/;
 9+-- Should cover *most* configuration - strings, ints, bools, etc.
 10+CREATE INDEX /*i*/cf_name_value ON /*_*/config (cf_name,cf_value(255));
Property changes on: trunk/phase3/maintenance/archives/patch-config.sql
___________________________________________________________________
Added: svn:eol-style
111 + native
Index: trunk/phase3/maintenance/tables.sql
@@ -1409,4 +1409,14 @@
14101410 ) /*$wgDBTableOptions*/;
14111411 CREATE UNIQUE INDEX /*i*/md_module_skin ON /*_*/module_deps (md_module, md_skin);
14121412
 1413+-- Table for holding configuration changes
 1414+CREATE TABLE /*_*/config (
 1415+ -- Config var name
 1416+ cf_name varbinary(255) NOT NULL PRIMARY KEY,
 1417+ -- Config var value
 1418+ cf_value blob NOT NULL,
 1419+) /*$wgDBTableOptions*/;
 1420+-- Should cover *most* configuration - strings, ints, bools, etc.
 1421+CREATE INDEX /*i*/cf_name_value ON /*_*/config (cf_name,cf_value(255));
 1422+
14131423 -- vim: sw=2 sts=2 et
Index: trunk/phase3/includes/conf/DatabaseConf.php
@@ -0,0 +1,33 @@
 2+<?php
 3+/**
 4+ * Database configuration class
 5+ *
 6+ * Copyright © 2011 Chad Horohoe <chadh@wikimedia.org>
 7+ * http://www.mediawiki.org/
 8+ *
 9+ * This program is free software; you can redistribute it and/or modify
 10+ * it under the terms of the GNU General Public License as published by
 11+ * the Free Software Foundation; either version 2 of the License, or
 12+ * (at your option) any later version.
 13+ *
 14+ * This program is distributed in the hope that it will be useful,
 15+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
 16+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
 17+ * GNU General Public License for more details.
 18+ *
 19+ * You should have received a copy of the GNU General Public License along
 20+ * with this program; if not, write to the Free Software Foundation, Inc.,
 21+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
 22+ * http://www.gnu.org/copyleft/gpl.html
 23+ *
 24+ * @file
 25+ * @ingroup Config
 26+ */
 27+class DatabaseConf extends Conf {
 28+ protected function initChangedSettings() {
 29+ $res = wfGetDB( DB_MASTER )->select( 'config', '*', array(), __METHOD__ );
 30+ foreach( $res as $row ) {
 31+ $this->values[$row->cf_name] = $row->cf_value;
 32+ }
 33+ }
 34+}
Property changes on: trunk/phase3/includes/conf/DatabaseConf.php
___________________________________________________________________
Added: svn:eol-style
135 + native
Index: trunk/phase3/includes/conf/Conf.php
@@ -0,0 +1,136 @@
 2+<?php
 3+/**
 4+ * Base configuration class.
 5+ *
 6+ * Get some configuration variable:
 7+ * $mySetting = Conf::get( 'mySetting' );
 8+ *
 9+ * Copyright © 2011 Chad Horohoe <chadh@wikimedia.org>
 10+ * http://www.mediawiki.org/
 11+ *
 12+ * This program is free software; you can redistribute it and/or modify
 13+ * it under the terms of the GNU General Public License as published by
 14+ * the Free Software Foundation; either version 2 of the License, or
 15+ * (at your option) any later version.
 16+ *
 17+ * This program is distributed in the hope that it will be useful,
 18+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
 19+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
 20+ * GNU General Public License for more details.
 21+ *
 22+ * You should have received a copy of the GNU General Public License along
 23+ * with this program; if not, write to the Free Software Foundation, Inc.,
 24+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
 25+ * http://www.gnu.org/copyleft/gpl.html
 26+ *
 27+ * @file
 28+ * @defgroup Config Config
 29+ * @ingroup Config
 30+ */
 31+abstract class Conf {
 32+ /**
 33+ * The Wiki ID (usually $wgDBname)
 34+ * @var String
 35+ */
 36+ private $wikiId;
 37+
 38+ /**
 39+ * Singleton
 40+ * @var Conf
 41+ */
 42+ private static $__instance;
 43+
 44+ /**
 45+ * Stores of the core defaults, extension defaults and wiki overrides
 46+ *
 47+ * @var array
 48+ */
 49+ protected $defaults, $extensionDefaults, $values = array();
 50+
 51+ /**
 52+ * Constructor. Children should call this if implementing.
 53+ * @param $confConfig Array of config vars
 54+ */
 55+ protected function __construct( $confConfig ) {
 56+ $this->wikiId = $confConfig['wikiId'];
 57+ $this->defaults = (array)(new DefaultSettings);
 58+ // @todo implement this:
 59+ // $this->initExtensionDefaults();
 60+ $this->initChangedSettings();
 61+ }
 62+
 63+ /**
 64+ * Load customized settings from whatever the data store is
 65+ */
 66+ abstract protected function initChangedSettings();
 67+
 68+ /**
 69+ * Initialize a new child class based on a configuration array
 70+ * @param $conf Array of configuration settings, see $wgConfiguration
 71+ * for details
 72+ * @return Conf
 73+ */
 74+ private static function newFromSettings( $conf ) {
 75+ $class = ucfirst( $conf['type'] ) . 'Conf';
 76+ if( !class_exists( $class ) ) {
 77+ throw new MWException( '$wgConfiguration misconfigured with invalid "type"' );
 78+ }
 79+ return new $class( $conf );
 80+ }
 81+
 82+ /**
 83+ * Get the singleton if we don't want a specific wiki
 84+ * @param $wiki String An id for a remote wiki
 85+ * @return Conf child
 86+ */
 87+ public static function load( $wiki = false ) {
 88+ if( !self::$__instance ) {
 89+ global $wgConfiguration;
 90+ self::$__instance = self::newFromSettings( $wgConfiguration );
 91+ }
 92+ if( $wiki && $wiki != self::$__instance->getWikiId() ) {
 93+ // Load configuration for a different wiki, not sure how
 94+ // we're gonna do this yet
 95+ return null;
 96+ }
 97+ return self::$__instance;
 98+ }
 99+
 100+ /**
 101+ * Get a property from the configuration database, falling back
 102+ * to DefaultSettings if undefined
 103+ * @param $name String Name of setting to retrieve.
 104+ * @param $wiki String An id for a remote wiki
 105+ * @return mixed
 106+ */
 107+ public static function get( $name, $wiki = false ) {
 108+ return self::load( $wiki )->retrieveSetting( $name );
 109+ }
 110+
 111+ /**
 112+ * Actually get the setting, checking overrides, extensions, then core.
 113+ * On failure,
 114+ * @param $name String Name of setting to get
 115+ * @return mixed
 116+ */
 117+ public function retrieveSetting( $name ) {
 118+ if( isset( $this->values[$name] ) ) {
 119+ return $this->values[$name];
 120+ } elseif( isset( $this->extensionDefaults[$name] ) ) {
 121+ return $this->extensionDefaults[$name];
 122+ } elseif( isset( $this->defaults[$name] ) ) {
 123+ return $this->defaults[$name];
 124+ } else {
 125+ wfDebug( __METHOD__ . " called for unknown configuration item '$name'\n" );
 126+ return null;
 127+ }
 128+ }
 129+
 130+ /**
 131+ * What is the wiki ID for this site?
 132+ * @return String
 133+ */
 134+ public function getWikiId() {
 135+ return $this->wikiId;
 136+ }
 137+}
Property changes on: trunk/phase3/includes/conf/Conf.php
___________________________________________________________________
Added: svn:eol-style
1138 + native
Added: svn:executable
2139 + *
Index: trunk/phase3/includes/conf/DefaultSettings.php
@@ -0,0 +1,28 @@
 2+<?php
 3+/**
 4+ * Utility class for holding all of our default settings.
 5+ *
 6+ * Copyright © 2011 Chad Horohoe <chadh@wikimedia.org>
 7+ * http://www.mediawiki.org/
 8+ *
 9+ * This program is free software; you can redistribute it and/or modify
 10+ * it under the terms of the GNU General Public License as published by
 11+ * the Free Software Foundation; either version 2 of the License, or
 12+ * (at your option) any later version.
 13+ *
 14+ * This program is distributed in the hope that it will be useful,
 15+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
 16+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
 17+ * GNU General Public License for more details.
 18+ *
 19+ * You should have received a copy of the GNU General Public License along
 20+ * with this program; if not, write to the Free Software Foundation, Inc.,
 21+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
 22+ * http://www.gnu.org/copyleft/gpl.html
 23+ *
 24+ * @file
 25+ * @ingroup Config
 26+ */
 27+final class DefaultSettings {
 28+ public $mySetting = 'defaultValue';
 29+}
Property changes on: trunk/phase3/includes/conf/DefaultSettings.php
___________________________________________________________________
Added: svn:eol-style
130 + native
Index: trunk/phase3/includes/AutoLoader.php
@@ -343,6 +343,7 @@
344344 'ApiUpload' => 'includes/api/ApiUpload.php',
345345 'ApiUserrights' => 'includes/api/ApiUserrights.php',
346346 'ApiWatch' => 'includes/api/ApiWatch.php',
 347+ 'UsageException' => 'includes/api/ApiMain.php',
347348
348349 # includes/cache
349350 'CacheDependency' => 'includes/cache/CacheDependency.php',
@@ -360,7 +361,10 @@
361362 'TitleDependency' => 'includes/cache/CacheDependency.php',
362363 'TitleListDependency' => 'includes/cache/CacheDependency.php',
363364
364 - 'UsageException' => 'includes/api/ApiMain.php',
 365+ # includes/conf
 366+ 'Conf' => 'includes/conf/Conf.php',
 367+ 'DatabaseConf' => 'includes/conf/DatabaseConf.php',
 368+ 'DefaultSettings' => 'includes/conf/DefaultSettings.php',
365369
366370 # includes/db
367371 'Blob' => 'includes/db/Database.php',

Follow-up revisions

RevisionCommit summaryAuthorDate
r88279Followup r88270: add new table to updatersdemon23:22, 16 May 2011
r88296Follow up r88270 — remove commas from table creation that break a...mah11:56, 17 May 2011
r92796Back r88270 and friends out of REL1_18 (conf overhaul)demon21:36, 21 July 2011
r93914Followup r88270: comment out globals so I can resolve the fixme. Throw an exc...demon21:00, 4 August 2011
r111006Revert r88270, r97711 and r110825 out of 1.19reedy01:16, 9 February 2012

Comments

#Comment by Aaron Schulz (talk | contribs)   23:11, 16 May 2011

Is there a proposal link?

#Comment by 😂 (talk | contribs)   23:14, 16 May 2011
#Comment by Platonides (talk | contribs)   21:24, 17 May 2011

Global variable $wgConfiguration is not present in DefaultSettings

#Comment by Reedy (talk | contribs)   21:25, 17 May 2011

Is it really a fixme when the code isn't actually in use in core yet?

#Comment by Platonides (talk | contribs)   21:30, 17 May 2011

This is just a proposal, so it can be considered a low priority one, but should had been complete anyway.

#Comment by 😂 (talk | contribs)   22:03, 21 July 2011

I won't be working on this for a bit, backed out of REL1_18 entirely in r92796.

Status & tagging log