Handle 401 and 404 from Github when finding an admin

Author: aakritigupta

lib/travis/services/find_admin.rb

@@ -10,11 +10,11 @@ class FindAdmin < Base
       include Travis::Logging
 
       register :find_admin
+      NUM_CANDIDATES = 15
 
       def run
         if repository
-          admin = candidates.first
-          admin || raise_admin_missing
+          admin
         else
           error "[github-admin] repository is nil: #{params.inspect}"
           raise Travis::RepositoryMissing, "no repository given"
@@ -29,25 +29,31 @@ def repository
       private
 
         def candidates
-          User.with_github_token.with_permissions(:repository_id => repository.id, :admin => true)
+          User.with_github_token.with_permissions(:repository_id => repository.id, :admin => true).first(NUM_CANDIDATES)
+        end
+
+        def admin
+          admin = candidates.detect do |candidate|
+            is_valid? candidate
+          end
+          admin || raise_admin_missing
         end
 
-        def validate(user)
+        def is_valid?(admin)
           Timeout.timeout(2) do
-            data = Github.authenticated(user) { repository_data }
+            data = Github.authenticated(admin) { repository_data }
             if data['permissions'] && data['permissions']['admin']
-              user
+              true
             else
-              info "[github-admin] #{user.login} no longer has admin access to #{repository.slug}"
-              update(user, data['permissions'])
+              revoke_admin_rights admin
               false
             end
           end
         rescue Timeout::Error => error
-          handle_error(user, error)
+          handle_error(admin, error)
           false
         rescue GH::Error => error
-          handle_error(user, error)
+          handle_error(admin, error)
           false
         end
 
@@ -58,8 +64,7 @@ def handle_error(user, error)
             error "[github-admin] token for #{user.login} no longer valid"
             user.update_attributes!(:github_oauth_token => "")
           when 404
-            info "[github-admin] #{user.login} no longer has any access to #{repository.slug}"
-            update(user, {})
+            revoke_admin_rights user
           else
             error "[github-admin] error retrieving repository info for #{repository.slug} for #{user.login}: #{error.message}"
           end
@@ -72,14 +77,16 @@ def repository_data
           data || { 'permissions' => {} }
         end
 
-        def update(user, permissions)
-          user.update_attributes!(:permissions => permissions)
-        end
-
         def raise_admin_missing
           raise Travis::AdminMissing.new("no admin available for #{repository.slug}")
         end
 
+        def revoke_admin_rights(user)
+          info "[github-admin] #{user.login} no longer has admin access to #{repository.slug}"
+
+          Permission.where(user_id: user.id, repository_id: repository.id).update_all(admin: false)
+        end
+
         class Instrument < Notification::Instrument
           def run_completed
             publish(

lib/travis/testing/stubs.rb

@@ -27,10 +27,19 @@ def included(base)
             let(:broadcast)           { stub_broadcast           }
             let(:travis_token)        { stub_travis_token        }
             let(:cache)               { stub_cache               }
+            let(:permission)          { stub_permission          }
           end
         end
       end
 
+      def stub_permission(attributes = {})
+        Stubs.stub 'permission', attributes.reverse_merge(
+          id: 1,
+          user_id: stub_user,
+          repository_id: stub_repo
+        )
+      end
+
       def stub_repo(attributes = {})
         Stubs.stub 'repository', attributes.reverse_merge(
           id: 1,
@@ -60,7 +69,7 @@ def stub_repo(attributes = {})
           github_id: 549743,
           builds_only_with_travis_yml?: false,
           settings: stub_settings,
-          users_with_permission: [],
+          users_with_permission: stub_user,
           default_branch: 'master',
           current_build_id: nil
         )

spec/travis/services/find_admin_spec.rb

@@ -1,3 +1,4 @@
+require 'pry'
 require 'spec_helper'
 
 describe Travis::Services::FindAdmin do
@@ -13,6 +14,8 @@
     describe 'given a user has admin access to a repository (as seen by github)' do
       before :each do
         GH.stubs(:[]).with("repos/#{repository.slug}").returns('permissions' => { 'admin' => true })
+        stub_permission(user_id: user.id, repository_id: repository.id)
+        Permission.stub(:update_all)
       end
 
       it 'returns that user' do
@@ -23,15 +26,17 @@
     describe 'given a user does not have access to a repository' do
       before :each do
         GH.stubs(:[]).with("repos/#{repository.slug}").returns('permissions' => { 'admin' => false })
-        user.stubs(:update_attributes!)
+
+        Permission.stubs(:where).with(user_id: user.id, repository_id: repository.id).returns(permission)
+        permission.stubs(:update_all)
       end
 
-      xit 'raises an exception' do
+      it 'raises an exception' do
         lambda { result }.should raise_error(Travis::AdminMissing, 'no admin available for svenfuchs/minimal')
       end
 
-      xit 'revokes admin permissions for that user on our side' do
-        user.expects(:update_attributes!).with(:permissions => { 'admin' => false })
+      it 'revokes admin permissions for that user on our side' do
+        permission.expects(:update_all).with(admin: false)
         ignore_exception { result }
       end
     end
@@ -43,7 +48,7 @@
         GH.stubs(:[]).with("repos/#{repository.slug}").raises(GH::Error.new(error))
       end
 
-      xit 'raises an exception' do
+      it 'raises an exception' do
         lambda { result }.should raise_error(Travis::AdminMissing, 'no admin available for svenfuchs/minimal')
       end