If not in single backend mode, versioncontrol_repository_load_multiple() ignores any backend specific repository controllers and uses VersioncontrolRepositoryController.

As discussed with sdboyer in IRC, we should instead let each backend load the repositories it supports and use VersioncontrolRepositoryController only for those, that have no own controller.

Patch follows.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Niklas Fiekas’s picture

Status: Active » Needs review
FileSize
3.17 KB

Please review.

Edit: Ah, the test request is postponed.

sdboyer’s picture

Status: Needs review » Needs work

Lookin pretty good here, except:

+++ b/includes/controllers.inc
@@ -418,6 +418,30 @@ class VersioncontrolRepositoryController extends VersioncontrolEntityController
+    else {
+      $this->attachCondition($query, 'vcs', array(
+        'values' => array_keys(versioncontrol_get_backends()),
+        'operator' => 'NOT IN',
+      ));
+    }
+    return $query;

Not quite right here - its' not a question of whether the backends exist (they will), it's a question of whether the backends define their own type of repository controller. So we need to check each backend's value for $backend->classesControllers['repo'] == 'VersioncontrolRepositoryController';

Niklas Fiekas’s picture

Status: Needs work » Needs review
FileSize
3.1 KB

Rerolled the patch according to your comment.

sdboyer’s picture

Status: Needs review » Needs work
+++ b/includes/controllers.inc
@@ -418,6 +418,39 @@ class VersioncontrolRepositoryController extends VersioncontrolEntityController
+      // Add the condition to the query.
+      $this->attachCondition($query, 'vcs', array(
+        'values' => $non_default_backends,
+        'operator' => 'NOT IN',
+      ));

Only attach a condition if $non_default_backends is actually non-empty. Otherwise, we see this fun:

SQLSTATE[42000]: Syntax error or access violation: 1064 You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near '))' at line 1: SELECT base.repo_id AS repo_id, base.name AS name, base.vcs AS vcs, base.root AS root, base.authorization_method AS authorization_method, base.updated AS updated, base.update_method AS update_method, base.locked AS locked, base.data AS data, base.plugins AS plugins FROM {versioncontrol_repositories} base WHERE (base.repo_id IN (:db_condition_placeholder_0)) AND (base.vcs NOT IN ()) ; Array ( [:db_condition_placeholder_0] => 1 )

Niklas Fiekas’s picture

Status: Needs work » Needs review
FileSize
3.12 KB

As discussed we can get around that by converting NOT IN to IN. All tests passing for me, now :)

sdboyer’s picture

Status: Needs review » Fixed

Yep, passing for me too. Good enough for me, I'm committing this.

Niklas Fiekas’s picture

Cool, thanks.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

  • Commit b596a58 on 6.x-2.x, repository-families, drush-vc-sync-unlock authored by Niklas Fiekas, committed by sdboyer:
    #1293720 by Niklas Fiekas: Changed Allow backend specific repository...

  • Commit b596a58 on 6.x-2.x, repository-families authored by Niklas Fiekas, committed by sdboyer:
    #1293720 by Niklas Fiekas: Changed Allow backend specific repository...