Merge pull request travis-ci#99 from travis-ci/mm-dont-sync-duplicate-org-repos

Don't sync duplicate org repos

Author: joshk

lib/travis/github/sync/repositories.rb

@@ -52,7 +52,31 @@ def remove
 
           # we have to filter these ourselves because the github api is broken for this
           def data
-            @data ||= fetch.select { |repo| repo['private'] == self.class.private? }
+            @data ||= filter_duplicates(filter_based_on_repo_permission)
+          end
+
+          def filter_based_on_repo_permission
+            fetch.select { |repo| repo['private'] == self.class.private? }
+          end
+
+          def filter_duplicates(repositories)
+            repositories.each_with_object([]) do |repository, filtered_list|
+              unless in_filtered_list?(filtered_list, repository)
+                filtered_list.push(repository)
+              end
+            end
+          end
+
+          def in_filtered_list?(filtered_list, other_repository)
+            filtered_list.any? do |existing_repository|
+              same_repository_with_admin?(existing_repository, other_repository)
+            end
+          end
+
+          def same_repository_with_admin?(existing_repository, other_repository)
+            existing_repository['owner']['login'] == other_repository['owner']['login'] and
+              existing_repository['name'] == other_repository['name'] and
+              existing_repository['permissions']['admin'] == true
           end
 
           def slugs
@@ -86,6 +110,3 @@ def with_github(&block)
     end
   end
 end
-
-
-

spec/travis/github/sync/repositories_spec.rb

@@ -73,4 +73,45 @@
     Travis::Github::Sync::Repository.expects(:unpermit_all).with(user, [removed_repo])
     sync.run
   end
+
+  context "with private forks of organization repositories" do
+    let(:user_repositories) {[
+      { 'name' => 'public',  'owner' => { 'login' => 'sven' }, 'permissions' => { 'admin' => true }, 'private' => false },
+      { 'name' => 'private', 'owner' => { 'login' => 'sven' }, 'permissions' => { 'admin' => true }, 'private' => true, 'fork' => true}
+    ]}
+    let(:duplicate_org_repositories) {[
+      { 'name' => 'private', 'owner' => { 'login' => 'sven' }, 'permissions' => { 'admin' => false }, 'private' => true, 'fork' => true}
+    ]}
+    let(:org_repositories) {[
+      { 'name' => 'other', 'owner' => { 'login' => 'sven' }, 'permissions' => { 'admin' => false }, 'private' => true, 'fork' => true}
+    ]}
+    let(:order) {sequence('github-sync')}
+
+    before do
+      Travis::Github::Sync::Repositories.type = 'private'
+      Travis::Github::Sync::Repository.unstub(:new)
+    end
+
+    it "should not sync the organization's duplicate" do
+      Travis::Github::Sync::Repository.expects(:new).once.returns(stub('repository', :run => public_repo))
+      GH.expects(:[]).with('user/repos').returns(user_repositories).in_sequence(order)
+      GH.expects(:[]).with('orgs/the-org/repos').returns(duplicate_org_repositories).in_sequence(order)
+      sync.run
+    end
+
+    it "should sync the organization's repository when it's not a duplicate" do
+      Travis::Github::Sync::Repository.expects(:new).twice.returns(stub('repository', :run => public_repo))
+      GH.expects(:[]).with('user/repos').returns(user_repositories).in_sequence(order)
+      GH.expects(:[]).with('orgs/the-org/repos').returns(org_repositories).in_sequence(order)
+      sync.run
+    end
+
+    it "should sync the organization's repository when it has admin rights" do
+      # this is an unlikely scenario, but as the code checks for it, a test is in order
+      Travis::Github::Sync::Repository.expects(:new).twice.returns(stub('repository', :run => public_repo))
+      GH.expects(:[]).with('user/repos').returns(duplicate_org_repositories).in_sequence(order)
+      GH.expects(:[]).with('orgs/the-org/repos').returns(user_repositories).in_sequence(order)
+      sync.run
+    end
+  end
 end