Commit 5be3b920 authored by Aaron Wells's avatar Aaron Wells
Browse files

Introducing the institution_config table

Bug 1264429: This patch creates the table, refactors the get_institution_config() method,
the Institution class, and the institution editing page, to use the new table.

Henceforth columns should only be added to the main "institution" table if they represent
a required setting (like name and displayname), or they need to be accessed frequently by
direct SQL queries.

Change-Id: I4564240d2c55ec2b6ec90868290a61cf4321460a
parent ec7f4ae2
...@@ -624,16 +624,17 @@ function institution_submit(Pieform $form, $values) { ...@@ -624,16 +624,17 @@ function institution_submit(Pieform $form, $values) {
db_begin(); db_begin();
// Update the basic institution record... // Update the basic institution record...
$newinstitution = new StdClass;
if ($add) { if ($add) {
$institution = $newinstitution->name = strtolower($values['name']); $newinstitution = new Institution();
$newinstitution->initialise($values['name'], $values['displayname']);
$institution = $newinstitution->name;
} }
else { else {
$newinstitution = new Institution($institution);
$newinstitution->displayname = $values['displayname'];
$oldinstitution = get_record('institution', 'name', $institution); $oldinstitution = get_record('institution', 'name', $institution);
} }
$newinstitution->displayname = $values['displayname'];
$newinstitution->authplugin = empty($values['authplugin']) ? null : $values['authplugin'];
$newinstitution->showonlineusers = !isset($values['showonlineusers']) ? 2 : $values['showonlineusers']; $newinstitution->showonlineusers = !isset($values['showonlineusers']) ? 2 : $values['showonlineusers'];
if (get_config('usersuniquebyusername')) { if (get_config('usersuniquebyusername')) {
// Registering absolutely not allowed when this setting is on, it's a // Registering absolutely not allowed when this setting is on, it's a
...@@ -707,6 +708,7 @@ function institution_submit(Pieform $form, $values) { ...@@ -707,6 +708,7 @@ function institution_submit(Pieform $form, $values) {
$newinstitution->allowinstitutionpublicviews = (isset($values['allowinstitutionpublicviews']) && $values['allowinstitutionpublicviews']) ? 1 : 0; $newinstitution->allowinstitutionpublicviews = (isset($values['allowinstitutionpublicviews']) && $values['allowinstitutionpublicviews']) ? 1 : 0;
// TODO: Move handling of authentication instances within the Institution class as well?
if (!empty($values['authplugin'])) { if (!empty($values['authplugin'])) {
$allinstances = array_merge($values['authplugin']['instancearray'], $values['authplugin']['deletearray']); $allinstances = array_merge($values['authplugin']['instancearray'], $values['authplugin']['deletearray']);
...@@ -753,9 +755,11 @@ function institution_submit(Pieform $form, $values) { ...@@ -753,9 +755,11 @@ function institution_submit(Pieform $form, $values) {
} }
} }
// Save the changes to the DB
$newinstitution->commit();
if ($add) { if ($add) {
insert_record('institution', $newinstitution); // If registration has been turned on, then we automatically insert an
// If registration has been turned on, then we automatically insert an
// internal authentication authinstance // internal authentication authinstance
if ($newinstitution->registerallowed) { if ($newinstitution->registerallowed) {
$authinstance = (object)array( $authinstance = (object)array(
...@@ -767,11 +771,6 @@ function institution_submit(Pieform $form, $values) { ...@@ -767,11 +771,6 @@ function institution_submit(Pieform $form, $values) {
insert_record('auth_instance', $authinstance); insert_record('auth_instance', $authinstance);
} }
} }
else {
$where = new StdClass;
$where->name = $institution;
update_record('institution', $newinstitution, $where);
}
if (is_null($newinstitution->style) && !empty($oldinstitution->style)) { if (is_null($newinstitution->style) && !empty($oldinstitution->style)) {
delete_records('style_property', 'style', $oldinstitution->style); delete_records('style_property', 'style', $oldinstitution->style);
......
...@@ -87,6 +87,21 @@ ...@@ -87,6 +87,21 @@
<KEY NAME="stylefk" TYPE="foreign" FIELDS="style" REFTABLE="style" REFFIELDS="id"/> <KEY NAME="stylefk" TYPE="foreign" FIELDS="style" REFTABLE="style" REFFIELDS="id"/>
</KEYS> </KEYS>
</TABLE> </TABLE>
<TABLE NAME="institution_config">
<FIELDS>
<FIELD NAME="id" TYPE="int" LENGTH="10" NOTNULL="true" SEQUENCE="true"/>
<FIELD NAME="institution" TYPE="char" LENGTH="255" NOTNULL="true" />
<FIELD NAME="field" TYPE="char" LENGTH="255" NOTNULL="true" />
<FIELD NAME="value" TYPE="text" LENGTH="small" NOTNULL="false" />
</FIELDS>
<KEYS>
<KEY NAME="primary" TYPE="primary" FIELDS="id" />
<KEY NAME="institutionfk" TYPE="foreign" FIELDS="institution" REFTABLE="institution" REFFIELDS="name" />
</KEYS>
<INDEXES>
<INDEX NAME="instfielduk" UNIQUE="true" FIELDS="institution,field"/>
</INDEXES>
</TABLE>
<!-- PLUGIN_TYPE_SUBSTITUTION --> <!-- PLUGIN_TYPE_SUBSTITUTION -->
<TABLE NAME="application"> <TABLE NAME="application">
<FIELDS> <FIELDS>
......
...@@ -2913,5 +2913,20 @@ function xmldb_core_upgrade($oldversion=0) { ...@@ -2913,5 +2913,20 @@ function xmldb_core_upgrade($oldversion=0) {
} }
} }
if ($oldversion < 2014010800) {
$table = new XMLDBTable('institution_config');
$table->addFieldInfo('id', XMLDB_TYPE_INTEGER, 10, null, XMLDB_NOTNULL, XMLDB_SEQUENCE);
$table->addFieldInfo('institution', XMLDB_TYPE_CHAR, 255, null, XMLDB_NOTNULL);
$table->addFieldInfo('field', XMLDB_TYPE_CHAR, 255, null, XMLDB_NOTNULL);
$table->addFieldInfo('value', XMLDB_TYPE_TEXT, 'small');
$table->addKeyInfo('primary', XMLDB_KEY_PRIMARY, array('id'));
$table->addKeyInfo('institutionfk', XMLDB_KEY_FOREIGN, array('institution'), 'institution', array('name'));
$table->addIndexInfo('instfielduk', XMLDB_INDEX_UNIQUE, array('institution', 'field'));
create_table($table);
}
return $status; return $status;
} }
...@@ -22,17 +22,59 @@ class Institution { ...@@ -22,17 +22,59 @@ class Institution {
const PERSISTENT = 2; const PERSISTENT = 2;
protected $initialized = self::UNINITIALIZED; protected $initialized = self::UNINITIALIZED;
protected $members = array(
/**
* The institution properties stored in the institution table, and their default values. The
* actual instance values will be in this->fields;
*
* Note that there's a dual system for institution properties. All required values and several
* older ones are stored in the institution table itself. Optional and/or newer values are
* stored in the institution_config table and go in $this->configs
*
* TODO: If we have problems with future developers adding columns and forgetting to add them
* here, perhaps replace this with a system that determines the DB columns of the institution
* table dynamically, by the same method as insert_record().
*
* @var unknown_type
*/
static $dbfields = array(
'name' => '', 'name' => '',
'displayname' => '', 'displayname' => '',
'registerallowed' => 1, 'registerallowed' => 1,
'theme' => 'default', 'registerconfirm' => 1,
'theme' => null,
'defaultmembershipperiod' => 0, 'defaultmembershipperiod' => 0,
'maxuseraccounts' => null, 'maxuseraccounts' => null,
'skins' => 1, 'expiry' => null,
); 'expirymailsent' => 0,
'suspended' => 0,
'priority' => 1,
'defaultquota' => null,
'showonlineusers' => 2,
'allowinstitutionpublicviews' => 1,
'logo' => null,
'style' => null,
'licensedefault' => null,
'licensemandatory' => 0,
'dropdownmenu' => 0,
'skins' => true
);
// This institution's config settings
protected $configs = array();
// Configs that have been updated and need to be saved on commit
protected $dirtyconfigs = array();
// This institution's properties
protected $fields = array();
// Fields that have been updated and need to be saved on commit
protected $dirtyfields = array();
public function __construct($name = null) { public function __construct($name = null) {
$this->fields = self::$dbfields;
if (is_null($name)) { if (is_null($name)) {
return $this; return $this;
} }
...@@ -43,50 +85,80 @@ class Institution { ...@@ -43,50 +85,80 @@ class Institution {
} }
public function __get($name) { public function __get($name) {
if (array_key_exists($name, $this->members)) {
return $this->members[$name]; // If it's an institution DB field, use the setting from $this->fields or null if that's empty for whatever reason
if (array_key_exists($name, self::$dbfields)) {
if (array_key_exists($name, $this->fields)) {
return $this->fields[$name];
}
else {
return null;
}
}
// If there's a config setting for it, use that
if (array_key_exists($name, $this->configs)) {
return $this->configs[$name];
} }
return null; return null;
} }
public function __set($name, $value) { public function __set($name, $value) {
if (!is_string($name) | !array_key_exists($name, $this->members)) { if (!is_string($name)) {
throw new ParamOutOfRangeException(); throw new ParamOutOfRangeException();
} }
if ($name == 'name') {
if (!is_string($value) || empty($value) || strlen($value) > 255) { // Validate the DB fields
throw new ParamOutOfRangeException("'name' should be a string between 1 and 255 characters in length"); switch ($name) {
} // char 255
} case 'name':
else if ($name == 'displayname') { case 'displayname':
if (!is_string($value) || empty($value) || strlen($value) > 255) { if (!is_string($value) || empty($value) || strlen($value) > 255) {
throw new ParamOutOfRangeException("'displayname' ($value) should be a string between 1 and 255 characters in length"); throw new ParamOutOfRangeException("'{$name}' should be a string between 1 and 255 characters in length");
} }
} break;
else if ($name == 'registerallowed') {
if (!is_numeric($value) || $value < 0 || $value > 1) { // int 1 (i.e. true/false)
throw new ParamOutOfRangeException("'registerallowed' should be zero or one"); case 'registerallowed':
case 'skins':
case 'suspended':
case 'licensemandatory':
case 'expirymailsent':
$value = $value ? 1 : 0;
break;
case 'defaultmembershipperiod':
case 'maxuseraccounts':
case 'showonlineusers':
$value = (int) $value;
break;
}
if (array_key_exists($name, self::$dbfields)) {
if ($this->fields[$name] !== $value) {
$this->fields[$name] = $value;
$this->dirtyfields[$name] = true;
} }
} }
else if ($name == 'theme') { else {
if (!empty($value) && is_string($value) && strlen($value) > 255) { // Anything else goes in institution_config.
throw new ParamOutOfRangeException("'theme' ($value) should be less than 255 characters in length"); // Since it's a DB field, the value must be a number, string, or NULL.
if (is_bool($value)) {
$value = $value ? 1 : 0;
} }
} if ($value !== NULL && !is_float($value) && !is_int($value) && !is_string($value)) {
else if ($name == 'defaultmembershipperiod') { throw new ParameterException("Attempting to set institution config field \"{$name}\" to a non-scalar value.");
if (!empty($value) && (!is_numeric($value) || $value < 0 || $value > 9999999999)) {
throw new ParamOutOfRangeException("'defaultmembershipperiod' should be a number between 1 and 9,999,999,999");
} }
}
else if ($name == 'maxuseraccounts') { // A NULL here means you should drop the config from the DB
if (!empty($value) && (!is_numeric($value) || $value < 0 || $value > 9999999999)) { $existingvalue = array_key_exists($name, $this->configs) ? $this->configs[$name] : NULL;
throw new ParamOutOfRangeException("'maxuseraccounts' should be a number between 1 and 9,999,999,999"); if ($value != $existingvalue) {
$this->configs[$name] = $value;
$this->dirtyconfigs[$name] = true;
} }
} }
else if ($name == 'skins') {
$value = (bool) $value;
}
$this->members[$name] = $value;
} }
public function findByName($name) { public function findByName($name) {
...@@ -108,25 +180,31 @@ class Institution { ...@@ -108,25 +180,31 @@ class Institution {
} }
public function initialise($name, $displayname) { public function initialise($name, $displayname) {
if (empty($name) || !is_string($name)) { if (!is_string($name)) {
return false;
}
$name = strtolower($name);
if (empty($name)) {
return false; return false;
} }
$this->name = $name; $this->name = $name;
if (empty($displayname) || !is_string($displayname)) { if (empty($displayname) || !is_string($displayname)) {
return false; return false;
} }
$this->displayname = $displayname; $this->displayname = $displayname;
$this->initialized = max(self::INITIALIZED, $this->initialized); $this->initialized = max(self::INITIALIZED, $this->initialized);
$this->dirtyfields = self::$dbfields;
return true; return true;
} }
public function verifyReady() { public function verifyReady() {
if (empty($this->members['name']) || !is_string($this->members['name'])) { if (empty($this->fields['name']) || !is_string($this->fields['name'])) {
return false; return false;
} }
if (empty($this->members['displayname']) || !is_string($this->members['displayname'])) { if (empty($this->fields['displayname']) || !is_string($this->fields['displayname'])) {
return false; return false;
} }
$this->initialized = max(self::INITIALIZED, $this->initialized); $this->initialized = max(self::INITIALIZED, $this->initialized);
...@@ -138,31 +216,66 @@ class Institution { ...@@ -138,31 +216,66 @@ class Institution {
throw new SystemException('Commit failed'); throw new SystemException('Commit failed');
} }
$record = new stdClass(); $result = true;
$record->name = $this->name; if (count($this->dirtyfields)) {
$record->displayname = $this->displayname; $record = new stdClass();
$record->theme = $this->theme; foreach (array_keys($this->dirtyfields) as $fieldname) {
$record->defaultmembershipperiod = $this->defaultmembershipperiod; $record->{$fieldname} = $this->{$fieldname};
$record->maxuseraccounts = $this->maxuseraccounts; }
$record->skins = $this->skins;
if ($this->initialized == self::INITIALIZED) { if ($this->initialized == self::INITIALIZED) {
return insert_record('institution', $record); $result = insert_record('institution', $record);
} elseif ($this->initialized == self::PERSISTENT) { }
return update_record('institution', $record, array('name' => $this->name)); else if ($this->initialized == self::PERSISTENT) {
$result = update_record('institution', $record, array('name' => $this->name));
}
}
if ($result) {
return $this->_commit_configs();
}
else {
// Shouldn't happen but who noes?
return false;
} }
// Shouldn't happen but who noes? }
return false;
/**
* Commit the config values for this institution. Called as part of commit();
*/
protected function _commit_configs() {
$result = true;
foreach (array_keys($this->dirtyconfigs) as $confkey) {
$newvalue = $this->configs[$confkey];
if ($newvalue === NULL) {
delete_records('institution_config', 'institution', $this->name, 'field', $confkey);
}
else {
$todb = new stdClass();
$todb->institution = $this->name;
$todb->field = $confkey;
$todb->value = $this->configs[$confkey];
if (!record_exists('institution_config', 'institution', $this->name, 'field', $confkey)) {
$result = $result && insert_record('institution_config', $todb);
}
else {
$result = $result && update_record('institution_config', $todb, array('institution', 'field'));
}
}
}
return $result;
} }
protected function populate($result) { protected function populate($result) {
$this->name = $result->name; foreach (array_keys(self::$dbfields) as $fieldname) {
$this->displayname = $result->displayname; $this->{$fieldname} = $result->{$fieldname};
$this->registerallowed = $result->registerallowed; }
$this->theme = $result->theme; $this->configs = get_records_menu('institution_config', 'institution', $result->name, 'field', 'field, value');
$this->defaultmembershipperiod = $result->defaultmembershipperiod; if (!$this->configs) {
$this->maxuseraccounts = $result->maxuseraccounts; $this->configs = array();
$this->skins = $result->skins; }
$this->verifyReady(); $this->verifyReady();
} }
......
...@@ -1054,62 +1054,103 @@ function set_config_plugin_instance($plugintype, $pluginname, $pluginid, $key, $ ...@@ -1054,62 +1054,103 @@ function set_config_plugin_instance($plugintype, $pluginname, $pluginid, $key, $
} }
/** /**
* Fetch an institution configuration * Fetch an institution configuration (from either the "institution" or "institution_config" table)
* TODO: If needed, create a corresponding set_config_institution() *
* TODO: If needed, create a corresponding set_config_institution(). This would be most useful if there's
* a situation where you need to manipulate individual institution configs. If you want to manipulate
* them in batch, you can use the Institution class's __set() and commit() methods.
*
* @param string $institutionname * @param string $institutionname
* @param string $key * @param string $key
* @return mixed The value of the key or NULL if the key is not valid * @return mixed The value of the key or NULL if the key is not valid
*/ */
function get_config_institution($institutionname, $key) { function get_config_institution($institutionname, $key) {
global $CFG;
require_once(get_config('docroot').'/lib/institution.php'); require_once(get_config('docroot').'/lib/institution.php');
// Note that this
static $fetchedinst = array(); // First, check the cache for an Institution object with this name
if (isset($fetchedinst[$institutionname])) { if (isset($CFG->fetchedinst->{$institutionname})) {
$inst = $fetchedinst[$institutionname]; $inst = $CFG->fetchedinst->{$institutionname};
} }
else { else {
// No cache hit, so instatiate a new Institution object
try { try {
$inst = new Institution($institutionname); $inst = new Institution($institutionname);
// Cache it (in $CFG so if we ever write set_config_institution() we can make it update the cache)
if (!isset($CFG->fetchedinst)) {
$CFG->fetchedinst = new stdClass();
}
$CFG->fetchedinst->{$institutionname} = $inst;
} }
catch (ParamOutOfRangeException $e) { catch (ParamOutOfRangeException $e) {
return null; return null;
} }
} }
// Use the magical __get() function of the Institution class // Use the magical __get() function of the Institution class
return $inst->{$key}; return $inst->{$key};
} }
/** /**
* Fetch a config setting for the specified user's institution. * Fetch a config setting for the specified user's institutions (from either the "institution" or "institution_config" table)
*
* @param string $key * @param string $key
* @param int $userid (Optional) If not supplied, fetch for the current user's institution * @param int $userid (Optional) If not supplied, fetch for the current user's institutions
* @return array The results for the all the users' institutions, in the order
* supplied by load_user_institutions(). Array key is institution name.
*/ */
function get_config_user_institution($key, $userid = null) { function get_configs_user_institutions($key, $userid = null) {
global $USER; global $USER;
if ($userid === null) { if ($userid === null) {
$userid = $USER->id; $userid = $USER->id;
} }
static $cache = array(); // Check for the user and key in the cache (The cache is stored in $CFG so it can be cleared/updated
if (isset($cache[$userid][$key])) { // if we ever write a set_config_institution() method)
return $cache[$userid][$key]; $userobj = "user{$userid}";
if (isset($CFG->userinstconf->{$userobj}->{$key})) {
return $CFG->userinstconf->{$userobj}->{$key};
} }
if ($userid == null) {
// We didn't hit the cache, so retrieve the config from their
// institution.
// First, get a list of their institution names
if (!$userid) {
// The logged-out user has no institutions.
$institutions = false;
}
else if ($userid == $USER->id) {
// Institutions for current logged-in user
$institutions = $USER->get('institutions'); $institutions = $USER->get('institutions');
} }
else { else {
$institutions = load_user_institutions($userid); $institutions = load_user_institutions($userid);
} }
// If the user belongs to no institution, check the Mahara institution // If the user belongs to no institution, check the Mahara institution
if (!$institutions) { if (!$institutions) {
$institutions = get_records_assoc('institution', 'name', 'mahara'); // For compatibility with $USER->get('institutions') and
// load_user_institutions(), we only really care about the
// array keys
$institutions = array('mahara' => 'mahara');
} }
$results = array(); $results = array();
foreach ($institutions as $instname => $inst) { foreach ($institutions as $instname => $inst) {