From a8c4ea83ebc4911a33d78ba9e96dda17a5b22de7 Mon Sep 17 00:00:00 2001 From: Sam Boyer Date: Thu, 2 Dec 2010 15:32:20 -0800 Subject: [PATCH 1/6] Refactor the plugin loading, and concomitant UI changes. Plugin caching is now more efficient (caches instantiated plugin object instead of plugin definition), actually works for multiple plugin types, and has good centralized error handling. There can now be a separate mapping plugin for authors and committers, with the possibility of tying committer plugin to author. Updated the repository editing UI to reflect these changes. --- includes/VersioncontrolOperation.php | 34 +++++++--------- includes/VersioncontrolRepository.php | 72 ++++++++++++++++++++++++-------- versioncontrol.admin.inc | 46 +++++++++++++++----- 3 files changed, 103 insertions(+), 49 deletions(-) diff --git includes/VersioncontrolOperation.php includes/VersioncontrolOperation.php index ac9c482..4888d73 100644 --- includes/VersioncontrolOperation.php +++ includes/VersioncontrolOperation.php @@ -134,26 +134,22 @@ abstract class VersioncontrolOperation extends VersioncontrolEntity { } /** - * Do mapping between operation and its users. + * Perform the mapping between Drupal users and this commit's author. */ - public function determineUsers() { - if ($mapper = $this->repository->getPluginClass('user_mapping_method', 'mapper')) { - if (method_exists($mapper, 'mapAuthor')) { - if (($uid=$mapper->mapAuthor($this)) !== FALSE) { - $this->author_uid = $uid; - } - else { - $this->author_uid = 0; - } - } - if (method_exists($mapper, 'mapCommitter')) { - if (($uid=$mapper->mapCommitter($this)) !== FALSE) { - $this->committer_uid = $uid; - } - else { - $this->committer_uid = 0; - } - } + public function mapAuthor() { + if ($mapper = $this->repository->getAuthorMapper()) { + $uid = $mapper->mapAuthor($this); + $this->author_uid = empty($uid) ? 0 : $uid; + } + } + + /** + * Perform the mapping between Drupal users and this commit's committer. + */ + public function mapCommitter() { + if ($mapper = $this->repository->getCommitterMapper()) { + $uid = $mapper->mapCommitter($this); + $this->committer_uid = empty($uid) ? 0 : $uid; } } diff --git includes/VersioncontrolRepository.php includes/VersioncontrolRepository.php index ed63a72..29b9202 100644 --- includes/VersioncontrolRepository.php +++ includes/VersioncontrolRepository.php @@ -97,12 +97,20 @@ abstract class VersioncontrolRepository implements VersioncontrolEntityInterface * An array describing the plugins that will be used for this repository. * * The current plugin types(array keys) are: - * - user_mapping_method + * - author_mapper + * - committer_mapper * * @var array */ public $plugins = array(); + /** + * An array of plugin instances (instanciated plugin objects). + * + * @var array + */ + protected $pluginInstances = array(); + protected $defaultCrudOptions = array( 'update' => array('nested' => TRUE), 'insert' => array('nested' => TRUE), @@ -460,32 +468,60 @@ abstract class VersioncontrolRepository implements VersioncontrolEntityInterface } /** - * Convinience method to get the a plugin. + * Get an instantiated plugin object based on a requested plugin slot, and the + * plugin this repository object has assigned to that slot. + * + * Internal function - other methods should provide a nicer public-facing + * interface. This method exists primarily to reduce code duplication involved + * in ensuring error handling and sound loading of the plugin. */ - function getPlugin($plugin_name) { - static $plugins; - if (!isset($plugins)) { - $plugins = array(); + protected function getPluginClass($plugin_slot, $plugin_type, $class_type) { + ctools_include('plugins'); + + if (empty($this->plugins[$plugin_slot])) { + throw new Exception("Attempted to get plugin in slot '$plugin_slot', but no plugin has been assigned to that slot on this repository.", E_STRICT); + return FALSE; + } + $plugin_name = $this->plugins[$plugin_slot]; + + $plugin = ctools_get_plugins('versioncontrol', $plugin_type, $plugin_name); + if (!is_array($plugin)) { + throw new Exception("Attempted to get a plugin of type '$plugin_type' named '$plugin_name', but no such plugin could be found.", E_WARNING); + return FALSE; } - if (isset($plugins[$plugin_name])) { - return $plugins[$plugin_name]; + + $class_name = ctools_plugin_get_class($plugin, $class_type); + if (!class_exists($class_name)) { + throw new Exception("Plugin '$plugin_name' of type '$plugin_type' does not contain a valid class name in handler slot '$class_type'", E_WARNING); + return FALSE; } - return $plugins[$plugin_name] = versioncontrol_get_user_mapping_method($this->plugins[$plugin_name]); + + return new $class_name(); } - /** - * Get an instantiated repo plugin class. - */ - public function getPluginClass($plugin, $class_type) { - if (is_string($plugin)) { - $plugin = $this->getPlugin($plugin); + public function getAuthorMapper() { + if (!isset($this->pluginInstances['author_mapper'])) { + // if no plugin is set, simply + $this->pluginInstances['author_mapper'] = $this->getPluginClass('author_mapper', 'user_mapping_method', 'mapper'); } - if ($class_name = ctools_plugin_get_class($plugin, $class_type)) { - return new $class_name(); + return $this->pluginInstances['author_mapper']; + } + + public function getCommitterMapper() { + if (!isset($this->pluginInstances['committer_mapper'])) { + // If nothing is set for the committer mapper plugin, reuse the author one + if (empty($this->plugins['committer_mapper'])) { + $this->pluginInstances['committer_mapper'] = $this->getAuthorMapper(); + } + else { + $this->pluginInstances['committer_mapper'] = $this->getPluginClass('committer_mapper', 'user_mapping_method', 'mapper'); + } } - return FALSE; + + return $this->pluginInstances['committer_mapper']; } + //ArrayAccess interface implementation FIXME soooooooo deprecated public function offsetExists($offset) { return isset($this->$offset); diff --git versioncontrol.admin.inc versioncontrol.admin.inc index eed2849..cc0ce38 100644 --- versioncontrol.admin.inc +++ versioncontrol.admin.inc @@ -619,14 +619,34 @@ function versioncontrol_admin_repository_edit(&$form_state, $repository, $vcs = '#size' => 60, '#maxlength' => 255, ); - $form['repository_information']['user_mapping_method'] = array( + + $text = t('Versioncontrol can map commit activity in this repository to user accounts. These options allow you to select the logic that should be used to perform the mapping between Drupal users and raw VCS data.'); + $text .= '

'; + $text .= t('Versioncontrol internally allows for both "authors" and "committers" to be tracked on each commit, so you can choose distinct mapping logic for each.'); + + $form['user_mapping'] = array( + '#type' => 'fieldset', + '#title' => t('User mapping'), + '#description' => $text, + '#collapsible' => TRUE, + '#collapsed' => FALSE, + '#weight' => 5, + ); + + $form['user_mapping']['author_mapper'] = array( '#type' => 'radios', - '#title' => t('User mapping method'), - '#description' => t('The way operations should be mapped to drupal users.'), - '#default_value' => $repository_exists - ? $repository->plugins['user_mapping_method'] - : NULL, - '#options' => versioncontrol_user_mapping_methods_get_names(), + '#title' => t('Author mapping'), + '#description' => t('The mapping logic to be used for "author" data. All VCSes have some sort of author field.'), + '#default_value' => $repository_exists ? $repository->plugins['author_mapper'] : 0, + '#options' => array(0 => t('None (no mapping)')) + versioncontrol_user_mapping_methods_get_names(), + ); + + $form['user_mapping']['committer_mapper'] = array( + '#type' => 'radios', + '#title' => t('Committer mapping'), + '#description' => t('The mapping logic to be used for "committer" data. Only some VCSes distinguish between author and committer. It is usually best to have this use the same plugin as the author mapper, especially if using "None."'), + '#default_value' => $repository_exists ? $repository->plugins['committer_mapper'] : 0, + '#options' => array(0 => t('Use author mapping plugin')) + versioncontrol_user_mapping_methods_get_names(), ); if ($repository_exists && isset($repository->data['versioncontrol']['registration_message'])) { @@ -642,7 +662,7 @@ function versioncontrol_admin_repository_edit(&$form_state, $repository, $vcs = '#title' => t('Account registration'), '#collapsible' => TRUE, '#collapsed' => TRUE, - '#weight' => 1, + '#weight' => 10, ); $form['account_registration']['authorization_method'] = array( '#type' => 'select', @@ -677,7 +697,7 @@ function versioncontrol_admin_repository_edit(&$form_state, $repository, $vcs = '#title' => t('Commit restrictions'), '#collapsible' => TRUE, '#collapsed' => TRUE, - '#weight' => 6, + '#weight' => 10, ); $form['commit_restrictions']['allow_unauthorized_access'] = array( '#type' => 'checkbox', @@ -700,7 +720,7 @@ function versioncontrol_admin_repository_edit(&$form_state, $repository, $vcs = '#description' => t('These URLs will be used to add links to item and commit displays such as the commit log.'), '#collapsible' => TRUE, '#collapsed' => TRUE, - '#weight' => 10, + '#weight' => 15, ); $form['repository_urls']['commit_view'] = array( '#type' => 'textfield', @@ -796,7 +816,8 @@ function versioncontrol_admin_repository_edit_submit($form, &$form_state) { $repository->root = $form_state['values']['root']; $repository->authorization_method = $form_state['values']['authorization_method']; $repository->plugins = array( - 'user_mapping_method' => $form_state['values']['user_mapping_method'] + 'author_mapper' => $form_state['values']['author_mapper'], + 'committer_mapper' => $form_state['values']['committer_mapper'], ); } else { @@ -808,7 +829,8 @@ function versioncontrol_admin_repository_edit_submit($form, &$form_state) { 'authorization_method' => $form_state['values']['authorization_method'], 'backend' => $backends[$form['#vcs']], 'plugins' => array( - 'user_mapping_method' => $form_state['values']['user_mapping_method'], + 'author_mapper' => $form_state['values']['author_mapper'], + 'committer_mapper' => $form_state['values']['committer_mapper'], ), ); $repository = $backends[$form['#vcs']]->buildEntity('repo', $repository); -- 1.7.2.3 From 1eb2fa78caf14203987a800a052575f89106a703 Mon Sep 17 00:00:00 2001 From: Sam Boyer Date: Thu, 2 Dec 2010 15:35:09 -0800 Subject: [PATCH 2/6] Tweaks to VersioncontrolOperation. Change the key slightly to be more human readable (no need for an underscore here), and reintroduce a mapUsers() method that replaces determineUsers() and just triggers both of the other mapping methods. --- includes/VersioncontrolOperation.php | 17 +++++++++++------ 1 files changed, 11 insertions(+), 6 deletions(-) diff --git includes/VersioncontrolOperation.php includes/VersioncontrolOperation.php index 4888d73..0feaf28 100644 --- includes/VersioncontrolOperation.php +++ includes/VersioncontrolOperation.php @@ -116,8 +116,8 @@ abstract class VersioncontrolOperation extends VersioncontrolEntity { public $itemRevisions = array(); protected $defaultCrudOptions = array( - 'update' => array('nested' => TRUE, 'map_user' => FALSE), - 'insert' => array('nested' => TRUE, 'map_user' => FALSE), + 'update' => array('nested' => TRUE, 'map users' => FALSE), + 'insert' => array('nested' => TRUE, 'map users' => FALSE), 'delete' => array('nested' => TRUE), ); @@ -133,6 +133,11 @@ abstract class VersioncontrolOperation extends VersioncontrolEntity { return $this->backend->loadEntities('item', $ids, $conditions, $options); } + public function mapUsers() { + $this->mapAuthor(); + $this->mapCommitter(); + } + /** * Perform the mapping between Drupal users and this commit's author. */ @@ -162,8 +167,8 @@ abstract class VersioncontrolOperation extends VersioncontrolEntity { // Append default options. $options += $this->defaultCrudOptions['insert']; - if ($options['map_user']) { - $this->determineUsers(); + if ($options['map users']) { + $this->mapUsers(); } // make sure repo id is set for drupal_write_record() @@ -201,8 +206,8 @@ abstract class VersioncontrolOperation extends VersioncontrolEntity { // Append default options. $options += $this->defaultCrudOptions['update']; - if ($options['map_user']) { - $this->determineUsers(); + if ($options['map users']) { + $this->mapUsers(); } // make sure repo id is set for drupal_write_record() -- 1.7.2.3 From 97b692987451f1683bdd706021cfbab7b146c0d4 Mon Sep 17 00:00:00 2001 From: Sam Boyer Date: Thu, 2 Dec 2010 15:37:07 -0800 Subject: [PATCH 3/6] Removed unnecessary plugin getters. --- versioncontrol.module | 31 +++---------------------------- 1 files changed, 3 insertions(+), 28 deletions(-) diff --git versioncontrol.module versioncontrol.module index 3f38f3a..4a5623d 100644 --- versioncontrol.module +++ versioncontrol.module @@ -1116,38 +1116,13 @@ function versioncontrol_ctools_plugin_directory($module, $plugin) { } /** - * Fetch metadata on a specific 'user mapping' plugin. - * - * @param $method - * Name of a versioncontrol 'user mapping' method. - * - * @return - * An array with information about the requested versioncontrol - * 'user mapping' method. - */ -function versioncontrol_get_user_mapping_method($method) { - ctools_include('plugins'); - return ctools_get_plugins('versioncontrol', 'user_mapping_methods', $method); -} - -/** - * Fetch metadata for all 'user mapping' plugins. - * - * @return - * An array of arrays with information about all available versionocontrol - * 'user mapping' methods. - */ -function versioncontrol_get_user_mapping_methods() { - ctools_include('plugins'); - return ctools_get_plugins('versioncontrol', 'user_mapping_methods'); -} - -/** * Load the names of all 'user_mapping_methods' for use at forms. */ function versioncontrol_user_mapping_methods_get_names() { + ctools_include('plugins'); + $names = array(); - foreach (versioncontrol_get_user_mapping_methods() as $name => $plugin) { + foreach (ctools_get_plugins('versioncontrol', 'user_mapping_methods') as $name => $plugin) { $names[$name] = $plugin['title']; } -- 1.7.2.3 From 1177308f7173c1d15e1a1315998076bca1a038f3 Mon Sep 17 00:00:00 2001 From: Sam Boyer Date: Thu, 2 Dec 2010 15:41:31 -0800 Subject: [PATCH 4/6] Move & rename the simple mail mapper plugin. --- .../plugins/user_mapping_methods/simple_mail.inc | 7 +++++++ .../versioncontrol_plain_mail.inc | 7 ------- 2 files changed, 7 insertions(+), 7 deletions(-) create mode 100644 includes/plugins/user_mapping_methods/simple_mail.inc delete mode 100644 includes/plugins/user_mapping_methods/versioncontrol_plain_mail/versioncontrol_plain_mail.inc diff --git includes/plugins/user_mapping_methods/simple_mail.inc includes/plugins/user_mapping_methods/simple_mail.inc new file mode 100644 index 0000000..d4fbf0f --- /dev/null +++ includes/plugins/user_mapping_methods/simple_mail.inc @@ -0,0 +1,7 @@ + t('Plain mail mapper'), + 'mapper' => 'versioncontrol_user_mapping_method_plain_mail', +); diff --git includes/plugins/user_mapping_methods/versioncontrol_plain_mail/versioncontrol_plain_mail.inc includes/plugins/user_mapping_methods/versioncontrol_plain_mail/versioncontrol_plain_mail.inc deleted file mode 100644 index d4fbf0f..0000000 --- includes/plugins/user_mapping_methods/versioncontrol_plain_mail/versioncontrol_plain_mail.inc +++ /dev/null @@ -1,7 +0,0 @@ - t('Plain mail mapper'), - 'mapper' => 'versioncontrol_user_mapping_method_plain_mail', -); -- 1.7.2.3 From d968619635942d709b3668ee99b7918e8602d6a5 Mon Sep 17 00:00:00 2001 From: Sam Boyer Date: Thu, 2 Dec 2010 15:57:30 -0800 Subject: [PATCH 5/6] Make the mapping process a little more robust - ensure the uid is set to 0 if no mapping plugin is set. --- includes/VersioncontrolOperation.php | 6 ++++++ 1 files changed, 6 insertions(+), 0 deletions(-) diff --git includes/VersioncontrolOperation.php includes/VersioncontrolOperation.php index 0feaf28..272cac7 100644 --- includes/VersioncontrolOperation.php +++ includes/VersioncontrolOperation.php @@ -146,6 +146,9 @@ abstract class VersioncontrolOperation extends VersioncontrolEntity { $uid = $mapper->mapAuthor($this); $this->author_uid = empty($uid) ? 0 : $uid; } + else { + $this->author_uid = 0; + } } /** @@ -156,6 +159,9 @@ abstract class VersioncontrolOperation extends VersioncontrolEntity { $uid = $mapper->mapCommitter($this); $this->committer_uid = empty($uid) ? 0 : $uid; } + else { + $this->committer_uid = 0; + } } public function insert($options = array()) { -- 1.7.2.3 From 4b358832a38a65fe0b71f720a27bb10f7419e34b Mon Sep 17 00:00:00 2001 From: Sam Boyer Date: Thu, 2 Dec 2010 15:59:41 -0800 Subject: [PATCH 6/6] Introduce VersioncontrolUserMapperInterface and create the simple_mail mapper plugin. --- includes/interfaces.inc | 24 ++++++++++++++++++++ .../VersioncontrolUserMapperSimpleMail.class.php | 22 ++++++++++++++++++ .../plugins/user_mapping_methods/simple_mail.inc | 7 ++++- 3 files changed, 51 insertions(+), 2 deletions(-) create mode 100644 includes/plugins/user_mapping_methods/VersioncontrolUserMapperSimpleMail.class.php diff --git includes/interfaces.inc includes/interfaces.inc index 8ea1302..d8dc55d 100644 --- includes/interfaces.inc +++ includes/interfaces.inc @@ -218,3 +218,27 @@ interface VersioncontrolRepositoryImportExport { public function exportAccounts($accounts); } + +interface VersioncontrolUserMapperInterface { + /** + * Map the author of the passed VersioncontrolOperation object to a Drupal + * uid, or FALSE if no uid mapping could be made. + * + * @param VersioncontrolOperation $commit + * The commit to be mapped. + * @return mixed + * Either a uid (int), or FALSE if the mapping failed. + */ + public function mapAuthor(VersioncontrolOperation $commit); + + /** + * Map the committer of the passed VersioncontrolOperation object to a Drupal + * uid, or FALSE if no uid mapping could be made. + * + * @param VersioncontrolOperation $commit + * The commit to be mapped. + * @return mixed + * Either a uid (int), or FALSE if the mapping failed. + */ + public function mapCommitter(VersioncontrolOperation $commit); +} \ No newline at end of file diff --git includes/plugins/user_mapping_methods/VersioncontrolUserMapperSimpleMail.class.php includes/plugins/user_mapping_methods/VersioncontrolUserMapperSimpleMail.class.php new file mode 100644 index 0000000..0c1de5c --- /dev/null +++ includes/plugins/user_mapping_methods/VersioncontrolUserMapperSimpleMail.class.php @@ -0,0 +1,22 @@ +author and $commit->committer are valid email addresses; + * does no checking to ensure they're good. + */ +class VersioncontrolUserMapperSimpleMail implements VersioncontrolUserMapperInterface { + public function mapAuthor(VersioncontrolOperation $commit) { + return $this->map($commit->author); + } + + public function mapCommitter(VersioncontrolOperation $commit) { + return $this->map($commit->committer); + } + + public function map($email) { + $uid = db_result(db_query("SELECT uid FROM {users} WHERE mail = '%s'")); + return empty($uid) ? FALSE : $uid; + } +} \ No newline at end of file diff --git includes/plugins/user_mapping_methods/simple_mail.inc includes/plugins/user_mapping_methods/simple_mail.inc index d4fbf0f..edbd2e0 100644 --- includes/plugins/user_mapping_methods/simple_mail.inc +++ includes/plugins/user_mapping_methods/simple_mail.inc @@ -2,6 +2,9 @@ // $Id$ $plugin = array( - 'title' => t('Plain mail mapper'), - 'mapper' => 'versioncontrol_user_mapping_method_plain_mail', + 'title' => t('Map using Drupal user email'), + 'mapper' => array( + 'class' => 'VersioncontrolUserMapperSimpleMail', + 'file' => 'VersioncontrolUserMapperSimpleMail.class.php', + ), ); -- 1.7.2.3