Find admin service via VCS

Author: weban-pl

lib/travis/remote_vcs/client.rb

@@ -22,14 +22,14 @@ def http_options
         { ssl: Travis.config.ssl.to_h }
       end
 
-      def request(method, name)
+      def request(method, name, data_in_body = true)
         resp = connection.send(method) { |req| yield(req) }
-        Travis.logger.info "#{self.class.name} #{name} response status: #{resp.status}"
-        if resp.success?
-          resp.body.present? ? JSON.parse(resp.body)['data'] : true
-        else
-          raise ResponseError, "#{self.class.name} #{name} request unexpected response: #{resp.body} #{resp.status}"
-        end
+
+        raise ResponseError, "#{self.class.name} #{name} request unexpected response: #{resp.body} #{resp.status}" unless resp.success?
+        return true unless resp.body.present?
+
+        parsed_response = JSON.parse(resp.body)
+        data_in_body ? parsed_response['data'] : parsed_response
       end
     end
   end

lib/travis/remote_vcs/repository.rb

@@ -41,6 +41,15 @@ def delete_key(repository_id:, user_id:, id:)
       rescue ResponseError
         false
       end
+
+      def show(repository_id:, admin_id: nil)
+        request(:get, __method__, false) do |req|
+          req.url "repos/#{repository_id}"
+          req.params['admin_id'] = admin_id
+        end
+      rescue ResponseError
+        nil
+      end
     end
   end
 end

lib/travis/services/find_admin.rb

@@ -34,7 +34,7 @@ def candidates
 
         def validate(user)
           Timeout.timeout(2) do
-            data = Github.authenticated(user) { repository_data }
+            data = repository_data(user)
             if data['permissions'] && data['permissions']['admin']
               user
             else
@@ -43,10 +43,7 @@ def validate(user)
               false
             end
           end
-        rescue Timeout::Error => error
-          handle_error(user, error)
-          false
-        rescue GH::Error => error
+        rescue error
           handle_error(user, error)
           false
         end
@@ -56,7 +53,7 @@ def handle_error(user, error)
           case status
           when 401
             error "[github-admin] token for #{user.login} no longer valid"
-            user.update_attributes!(:github_oauth_token => "")
+            # user.update_attributes!(:github_oauth_token => "")
           when 404
             info "[github-admin] #{user.login} no longer has any access to #{repository.slug}"
             update(user, {})
@@ -66,10 +63,11 @@ def handle_error(user, error)
         end
 
         # TODO should this not be memoized?
-        def repository_data
-          data = GH["repos/#{repository.slug}"]
-          info "[github-admin] could not retrieve data for #{repository.slug}" unless data
-          data || { 'permissions' => {} }
+        def repository_data(user)
+          Travis::RemoteVCS::Repository.new.show(
+            repository_id: repository.id,
+            admin_id: user.id
+          )
         end
 
         def update(user, permissions)

spec/lib/services/find_admin_spec.rb

@@ -9,18 +9,13 @@
     end
 
     describe 'given a user has admin access to a repository (as seen by github)' do
-      before :each do
-        allow(GH).to receive(:[]).with("repos/#{repository.slug}").and_return('permissions' => { 'admin' => true })
-      end
-
       it 'returns that user' do
         expect(result).to eq(user)
       end
     end
 
     describe 'given a user does not have access to a repository' do
       before :each do
-        allow(GH).to receive(:[]).with("repos/#{repository.slug}").and_return('permissions' => { 'admin' => false })
         allow(user).to receive(:update_attributes!)
       end
 
@@ -74,15 +69,14 @@ def ignore_exception(&block)
   before :each do
     Travis::Notification.publishers.replace([publisher])
     allow(User).to receive(:with_permissions).with(repository_id: repository.id, admin: true).and_return [user]
-    allow(GH).to receive(:[]).with("repos/#{repository.slug}").and_return('permissions' => { 'admin' => true })
     service.run
   end
 
   it 'publishes a event' do
     expect(event).to publish_instrumentation_event(
       event: 'travis.services.find_admin.run:completed',
       message: 'Travis::Services::FindAdmin#run:completed for svenfuchs/minimal: svenfuchs',
-      result: user,
+      result: user
     )
   end
 end