Browse files

bye bye VERSIONCONTROL_FLAG_AUTOADD_REPOSITORIES

Change interaction between repository class and backends to use inheritance
relation to acomplish that.

This commits make not mor needed _dbDeleteAdditions, _dbInsertAdditions,
_dbUpdateAdditions and _amend_repositories.

In general, repo class is modified to have insert/update/delete method that are
public final, which call a by default empty _insert/_update/_delete method that
are protected. This way, we have interactions with backends through inheritance.

The getter(VersioncontrolRepositoryCache::getRepositories) now calls a
VersioncontrolRepository::_getRepository to get special information(not in
VersioncontrolRepository:data) by repo, but maye is a good idea to make that
call by backend instead of thati for performance reasons, but that's not
possible now, so there is a TODO near the relevant code, that have exact
information about it.
  • Loading branch information...
1 parent 6a23c18 commit 7bdb761fae1f0c31ae537d63fdca74863828592d @marvil07 committed Jul 26, 2009
View
6 OOP_TODO
@@ -1,11 +1,9 @@
on code
-------
-- buildSelf should not be abstract, it should have a by default implementation to avoid repeat same code on backends
- follow sdboyer suggestions (http://groups.drupal.org/node/23015#comment-79517)
- There are prolly other ones, but one coding convention we have is to get rid of the prepended underscore on function names when they become methods, and just make em protected. f.e. VersioncontrolItem::_badItemWarning.
- I would have figured, based on last week's conversation, that it's not just VersioncontrolBackend that's abstract, but also VersioncontrolOperation, VersioncontrolItem, VersioncontrolRepository, etc; that these, too, will be extended by the backends as needed. Although that doesn't require them to be abstract, of course - just that they hold all the shared code that's common to all the vcses.
-- avoid notices on add repository (account status module?)
- refactor all methods in each class, to avoid calling now deleted funcitons and call each class where is necesary.
- read each method again on all classes
- while changing code:
@@ -23,6 +21,10 @@ on code
Replace versioncontrol_backend_implements() approach with one interface per feature
- start with the "simple" VersionControlRepositoryURLBackend interface proposed by jpetso
+- update documentation
+ - versioncontrol_fakevcs module
+ - hook_versioncontrol.php
+
on design
---------
View
14 includes/VersioncontrolBackend.php
@@ -30,23 +30,19 @@
public $capabilities;
/**
- * XXX
- *
- * @var array
- */
- public $flags;
-
- /**
* classes which this backend overwrite
*/
public $classes;
// Operations
- public function __construct($name, $description, $capabilities=array(), $flags=array(), $classes=array()) {
+
+ /**
+ * Reference constructor for backends
+ */
+ public function __construct($name, $description, $capabilities=array(), $classes=array()) {
$this->name = $name;
$this->description = $description;
$this->capabilities = $capabilities;
- $this->flags = $flags;
$this->classes = $classes;
}
View
153 includes/VersioncontrolRepository.php
@@ -229,6 +229,13 @@ public function isAccountAuthorized($uid) {
}
/**
+ * Let child backend repo classes add information that _is not_ in
+ * VersioncontrolRepository::data
+ */
+ public function _getRepository() {
+ }
+
+ /**
* Update a repository in the database, and call the necessary hooks.
* The 'repo_id' and 'vcs' properties of the repository object must stay
* the same as the ones given on repository creation,
@@ -238,7 +245,7 @@ public function isAccountAuthorized($uid) {
* An array of repository viewer URLs. How this array looks like is
* defined by the corresponding URL backend.
*/
- public function update($repository_urls=NULL) {
+ public final function update($repository_urls=NULL) {
drupal_write_record('versioncontrol_repositories', $this, 'repo_id');
if (!is_null($repository_urls)) {
@@ -247,22 +254,7 @@ public function update($repository_urls=NULL) {
unset($repository_urls['repo_id']);
}
- // Auto-add commit info from $commit['[xxx]_specific'] into the database.
- $vcs = $this->vcs;
- $is_autoadd = in_array(VERSIONCONTROL_FLAG_AUTOADD_REPOSITORIES,
- $this->backend->flags);
- if ($is_autoadd) {
- $table_name = 'versioncontrol_'. $vcs .'_repositories';
- $vcs_specific = $vcs .'_specific';
- $elements = $this->$vcs_specific;
- $elements['repo_id'] = $this->repo_id;
- $this->_dbUpdateAdditions($table_name, 'repo_id', $elements);
- }
-
- // Provide an opportunity for the backend to add its own stuff.
- if (versioncontrol_backend_implements($vcs, 'repository')) {
- _versioncontrol_call_backend($vcs, 'repository', array('update', $this));
- }
+ $this->_update();
// Everything's done, let the world know about it!
module_invoke_all('versioncontrol_repository', 'update', $this);
@@ -275,6 +267,14 @@ public function update($repository_urls=NULL) {
}
/**
+ * Let child backend repo classes update information that _is not_ in
+ * VersioncontrolRepository::data without modifying general flow if
+ * necessary.
+ */
+ protected function _update() {
+ }
+
+ /**
* Insert a repository into the database, and call the necessary hooks.
*
* @param $repository_urls
@@ -284,7 +284,7 @@ public function update($repository_urls=NULL) {
* @return
* The finalized repository array, including the 'repo_id' element.
*/
- public function insert($repository_urls) {
+ public final function insert($repository_urls) {
if (isset($this->repo_id)) {
// This is a new repository, it's not supposed to have a repo_id yet.
unset($this->repo_id);
@@ -296,22 +296,7 @@ public function insert($repository_urls) {
drupal_write_record('versioncontrol_repository_urls', $repository_urls);
unset($repository_urls['repo_id']);
- // Auto-add repository info from $repository['[xxx]_specific'] into the database.
- $vcs = $this->vcs;
- $is_autoadd = in_array(VERSIONCONTROL_FLAG_AUTOADD_REPOSITORIES,
- $this->backend->flags);
- if ($is_autoadd) {
- $table_name = 'versioncontrol_'. $vcs .'_repositories';
- $vcs_specific = $vcs .'_specific';
- $elements = $this->$vcs_specific;
- $elements['repo_id'] = $this->repo_id;
- $this->_dbInsertAdditions($table_name, $elements);
- }
-
- // Provide an opportunity for the backend to add its own stuff.
- if (versioncontrol_backend_implements($vcs, 'repository')) {
- _versioncontrol_call_backend($vcs, 'repository', array('insert', $this));
- }
+ $this->_insert();
// Everything's done, let the world know about it!
module_invoke_all('versioncontrol_repository', 'insert', $this);
@@ -325,11 +310,19 @@ public function insert($repository_urls) {
}
/**
+ * Let child backend repo classes add information that _is not_ in
+ * VersioncontrolRepository::data without modifying general flow if
+ * necessary.
+ */
+ protected function _insert() {
+ }
+
+ /**
* Delete a repository from the database, and call the necessary hooks.
* Together with the repository, all associated commits and accounts are
* deleted as well.
*/
- public function delete() {
+ public final function delete() {
// Delete operations.
$operations = VersioncontrolOperation::getOperations(array('repo_ids' => array($this->repo_id)));
foreach ($operations as $operation) {
@@ -378,22 +371,7 @@ public function delete() {
// Announce deletion of the repository before anything has happened.
module_invoke_all('versioncontrol_repository', 'delete', $this);
- $vcs = $this->vcs;
-
- // Provide an opportunity for the backend to delete its own stuff.
- if (versioncontrol_backend_implements($vcs, 'repository')) {
- _versioncontrol_call_backend($vcs, 'repository', array('delete', $this));
- }
-
- // Auto-delete repository info from $repository['[xxx]_specific'] from the database.
- if (isset($this->backend)) { // not the case when called from uninstall
- $is_autoadd = in_array(VERSIONCONTROL_FLAG_AUTOADD_REPOSITORIES,
- $this->backend->flags);
- }
- if ($is_autoadd) {
- $table_name = 'versioncontrol_'. $vcs .'_repositories';
- $this->_dbDeleteAdditions($table_name, 'repo_id', $this->repo_id);
- }
+ $this->_delete();
// Phew, everything's cleaned up. Finally, delete the repository.
db_query('DELETE FROM {versioncontrol_repositories} WHERE repo_id = %d',
@@ -408,6 +386,13 @@ public function delete() {
);
}
+ /**
+ * Let child backend repo classes delete information that _is not_ in
+ * VersioncontrolRepository::data without modifying general flow if
+ * necessary.
+ */
+ protected function _delete() {
+ }
/**
* Export a repository's authenticated accounts to the version control system's
@@ -487,72 +472,6 @@ public function getItem($path, $constraints = array()) {
return $item;
}
- /**
- * Generate and execute a DELETE query for the given table
- * based on name and value of the primary key.
- * In order to avoid unnecessary complexity, the primary key may not consist
- * of multiple columns and has to be a numeric value.
- */
- private function _dbDeleteAdditions($table_name, $primary_key_name, $primary_key) {
- db_query('DELETE FROM {'. $table_name .'}
- WHERE '. $primary_key_name .' = %d', $primary_key);
- }
-
- /**
- * Generate and execute an INSERT query for the given table based on key names,
- * values and types of the given array elements. This function basically
- * accomplishes the insertion part of Version Control API's 'autoadd' feature.
- */
- private function _dbInsertAdditions($table_name, $elements) {
- $keys = array();
- $params = array();
- $types = array();
-
- foreach ($elements as $key => $value) {
- $keys[] = $key;
- $params[] = is_numeric($value) ? $value : serialize($value);
- $types[] = is_numeric($value) ? '%d' : "'%s'";
- }
-
- db_query(
- 'INSERT INTO {'. $table_name .'} ('. implode(', ', $keys) .')
- VALUES ('. implode(', ', $types) .')', $params
- );
- }
-
- /**
- * Generate and execute an UPDATE query for the given table based on key names,
- * values and types of the given array elements. This function basically
- * accomplishes the update part of Version Control API's 'autoadd' feature.
- * In order to avoid unnecessary complexity, the primary key may not consist
- * of multiple columns and has to be a numeric value.
- */
- private function _dbUpdateAdditions($table_name, $primary_key_name, $elements) {
- $set_statements = array();
- $params = array();
-
- foreach ($elements as $key => $value) {
- if ($key == $primary_key_name) {
- continue;
- }
- $type = is_numeric($value) ? '%d' : "'%s'";
- $set_statements[] = $key .' = '. $type;
- $params[] = is_numeric($value) ? $value : serialize($value);
- }
- $params[] = $elements[$primary_key_name];
-
- if (empty($set_statements)) {
- return; // no use updating the database if no values are assigned.
- }
-
- db_query(
- 'UPDATE {'. $table_name .'}
- SET '. implode(', ', $set_statements) .'
- WHERE '. $primary_key_name .' = %d', $params
- );
- }
-
-
//ArrayAccess interface implementation
public function offsetExists($offset) {
return isset($this->$offset);
View
100 includes/classes.inc
@@ -70,19 +70,14 @@ final class VersioncontrolRepositoryCache {
* - 'names': An array of repository names, like
* array('Drupal CVS', 'Experimental SVN'). If given,
* only repositories with these repository names will be returned.
- * - '[xxx]_specific': An array of VCS specific constraints. How this array
- * looks like is defined by the corresponding backend module
- * (versioncontrol_[xxx]). Other backend modules won't get to see this
- * constraint, so in theory you can provide one of those for each backend
- * in one single query.
*
* @return
* An array of repositories where the key of each element is the repository
* id. The corresponding value contains a VersioncontrolRepository object.
* If not a single repository matches these constraints,
* an empty array is returned.
*/
- public function getRepositories($constraints = array()) {
+ public final function getRepositories($constraints = array()) {
// $backends = versioncontrol_get_backends();
$auth_methods = versioncontrol_get_authorization_methods();
@@ -119,31 +114,28 @@ final class VersioncontrolRepositoryCache {
if (!isset($repositories_by_backend[$repository['vcs']])) {
$repositories_by_backend[$repository['vcs']] = array();
}
- $repository[$repository['vcs'] .'_specific'] = array();
$repositories_by_backend[$repository['vcs']][$repository['repo_id']] = $repository;
}
- $repositories_by_backend = $this->_amend_repositories(
- $repositories_by_backend, $this->backends
- );
-
// Add the fully assembled repositories to the result array.
$result_repositories = array();
foreach ($repositories_by_backend as $vcs => $vcs_repositories) {
foreach ($vcs_repositories as $repository) {
- // $vcs_repository = new VersioncontrolRepository($repository['repo_id'], $repository['name'], $repository['vcs'], $repository['root'], $repository['authorization_method'], $repository['url_backend'], $repository['data']);
$vcs_repository = new $this->backends[$repository['vcs']]->classes['repo']($repository['repo_id'], $repository, FALSE);
- //FIXME: another idea for this?
- $vcs_specific_key = $repository['vcs'] .'_specific';
- $vcs_repository->$vcs_specific_key = $repository[$repository['vcs'] .'_specific'];
$vcs_repository->backend = $this->backends[$repository['vcs']];
+ //TODO think how to improve this getter per repo, because it can cause
+ // performance problems, maybe it's better to use a per backend
+ // process(1-level-up foreach) but now it's not posible, because
+ // we can not inherit VersioncontrolRepositoryCache
+ $vcs_repository->_getRepository();
if (!$this->repoCache['repo_id'][$repository['repo_id']] instanceof VersioncontrolRepository) {
$this->cacheRepository($vcs_repository);
}
$result_repositories[$repository['repo_id']] = $vcs_repository;
}
}
+
$this->repoCache[$constraints_serialized] = $result_repositories; // cache the results
return $result_repositories;
}
@@ -170,82 +162,4 @@ final class VersioncontrolRepositoryCache {
$this->repoCache['vcs'][$repository->vcs][$repository->repo_id] = &$repository;
}
- /**
- * Fetch VCS specific repository data additions, either by ourselves (if the
- * VERSIONCONTROL_FLAG_AUTOADD_REPOSITORIES flag has been set by the backend)
- * and/or by calling [vcs_backend]_alter_repositories().
- *
- * @static
- * @param $repositories_by_backend
- * @param $backends
- * @param $constraints
- */
- private function _amend_repositories($repositories_by_backend, $backends, $constraints = array()) {
- foreach ($repositories_by_backend as $vcs => $vcs_repositories) {
- $is_autoadd = in_array(VERSIONCONTROL_FLAG_AUTOADD_REPOSITORIES,
- $backends[$vcs]['flags']);
-
- if ($is_autoadd) {
- $repo_ids = array();
- foreach ($vcs_repositories as $repo_id => $repository) {
- $repo_ids[] = $repo_id;
- }
- $additions = $this->_dbGetAdditions(
- 'versioncontrol_'. $vcs .'_repositories', 'repo_id', $repo_ids
- );
-
- foreach ($additions as $repo_id => $addition) {
- if (isset($vcs_repositories[$repo_id])) {
- $vcs_repositories[$repo_id][$vcs .'_specific'] = $addition;
- }
- }
- }
-
- $vcs_specific_constraints = isset($constraints[$vcs .'_specific'])
- ? $constraints[$vcs .'_specific']
- : array();
-
- // Provide an opportunity for the backend to add its own stuff.
- if (versioncontrol_backend_implements($vcs, 'alter_repositories')) {
- $function = 'versioncontrol_'. $vcs .'_alter_repositories';
- $function($vcs_repositories, $vcs_specific_constraints);
- }
- $repositories_by_backend[$vcs] = $vcs_repositories;
- }
- return $repositories_by_backend;
- }
-
- /**
- * Generate and execute a SELECT query for the given table base on the name
- * and given values of this table's primary key. This function basically
- * accomplishes the retrieval part of Version Control API's 'autoadd' feature.
- * In order to avoid unnecessary complexity, the primary key may not consist
- * of multiple columns and has to be a numeric value.
- */
- private function _dbGetAdditions($table_name, $primary_key_name, $keys) {
- $placeholders = array();
-
- foreach ($keys as $key) {
- $placeholders[] = '%d';
- }
-
- $result = db_query('SELECT * FROM {'. $table_name .'}
- WHERE '. $primary_key_name .' IN ('.
- implode(',', $placeholders) .')', $keys);
-
- $additions = array();
- while ($addition = db_fetch_array($result)) {
- $primary_key = $addition[$primary_key_name];
- unset($addition[$primary_key_name]);
-
- foreach ($addition as $key => $value) {
- if (!is_numeric($addition[$key])) {
- $addition[$key] = unserialize($addition[$key]);
- }
- }
- $additions[$primary_key] = $addition;
- }
- return $additions;
-
- }
}
View
20 tests/versioncontrol_test.module
@@ -14,17 +14,23 @@ require_once drupal_get_path('module', 'versioncontrol') . '/includes/Versioncon
*/
function versioncontrol_test_versioncontrol_backends() {
return array(
- 'test' => new VersioncontrolTestBackend(
- 'TestVCS',
- t('TestVCS backend for Version Control API.'),
- array(),
- array(),
- array('repo' => 'VersioncontrolTestRepository')
- ),
+ 'test' => new VersioncontrolTestBackend()
);
}
class VersioncontrolTestBackend extends VersioncontrolBackend {
+ public function __construct() {
+ $this->name = 'TestVCS';
+ $this->description = t('TestVCS backend for Version Control API.');
+ $this->capabilities = array(
+ // Use the commit hash for to identify the commit instead of an individual
+ // revision for each file.
+ VERSIONCONTROL_CAPABILITY_ATOMIC_COMMITS
+ );
+ $this->classes = array(
+ 'repo' => 'VersioncontrolTestRepository',
+ );
+ }
}
class VersioncontrolTestRepository extends VersioncontrolRepository {
View
9 versioncontrol.module
@@ -21,15 +21,6 @@ define('VERSIONCONTROL_CAPABILITY_DIRECTORY_REVISIONS', 4);
//@}
/**
- * @name VCS backend flags
- * Flags that backends can set to specify if Version Control API
- * should do work for them.
- */
-//@{
-define('VERSIONCONTROL_FLAG_AUTOADD_REPOSITORIES', 1);
-//@}
-
-/**
* @name VCS actions
* for a single item (file or directory) in a commit, or for branches and tags.
*/
View
1 versioncontrol.pages.inc
@@ -515,7 +515,6 @@ function versioncontrol_account_edit_form_submit($form, &$form_state) {
$username = $form_state['values']['account_name'];
$repository = $form['#repository'];
$vcs_name = $form['#vcs_name'];
- $vcs_specific = NULL;
// Let other modules provide additional account data.
$additional_data = array();

0 comments on commit 7bdb761

Please sign in to comment.