Merge branch 'master' into epic-fast-track-build

pavel-d · web-flow · commit 26490c786889 · 2020-08-07T11:48:22.000+03:00
diff --git a/lib/travis/api/app/middleware/user_agent_tracker.rb b/lib/travis/api/app/middleware/user_agent_tracker.rb
@@ -11,15 +11,33 @@ class UserAgentTracker < Middleware
         "Opera", "Mozilla"
       ]
 
+      PATHS_IGNORED = [
+        '/uptime'
+      ]
+
+      attr_reader :metrik_prefix
+
+      before do
+        @metrik_prefix = if version = request.env['HTTP_TRAVIS_API_VERSION'].to_s.match(/^\d+(\.\d+)*/)
+          "api.v#{version}"
+        else
+          "api.v2"
+        end
+      end
+
       before(agent: /^$/) do
-        ::Metriks.meter("api.v2.user_agent.missing").mark
+        return if PATHS_IGNORED.include?(request.path)
+        ::Metriks.meter("#{metrik_prefix}.user_agent.missing").mark
         halt(400, "error" => "missing User-Agent header") if Travis::Features.feature_active?(:require_user_agent)
       end
 
       before(agent: /^.+$/) do
+        return if PATHS_IGNORED.include?(request.path)
         agent = UserAgent.parse(request.user_agent)
         case agent.browser
-        when *WEB_BROWSERS                   then mark_browser
+        when *WEB_BROWSERS
+          # if X-User-Agent header is set, honor that instead
+          mark :browser, UserAgent.parse(env['HTTP_X_USER_AGENT'] || request.user_agent).browser
         when "curl", "Wget"                  then mark(:console, agent.browser)
         when "travis-api-wrapper"            then mark(:script, :node_js, agent.browser)
         when "TravisPy"                      then mark(:script, :python,  agent.browser)
@@ -30,12 +48,6 @@ class UserAgentTracker < Middleware
         end
       end
 
-      def mark_browser
-        # allows a JavaScript Client to set X-User-Agent, for instance to "travis-web" in travis-web
-        x_agent = UserAgent.parse(env['HTTP_X_USER_AGENT'] || 'unknown').browser
-        mark(:browser, x_agent)
-      end
-
       def mark_travis(agent)
         command = agent.application.comment.detect { |c| c.start_with? "command " } if agent.application.comment
 
@@ -54,7 +66,7 @@ def mark_unknown
       end
 
       def mark(*keys)
-        key = "api.v2.user_agent." << keys.map { |k| k.to_s.downcase.gsub(/[^a-z0-9\-\.]+/, '_') }.join('.')
+        key = "#{metrik_prefix}.user_agent." << keys.map { |k| k.to_s.downcase.gsub(/[^a-z0-9\-\.]+/, '_') }.join('.')
         ::Metriks.meter(key).mark
       end
     end
diff --git a/lib/travis/api/v3/github.rb b/lib/travis/api/v3/github.rb
@@ -88,7 +88,7 @@ def deactivate_service_hook(repo)
     end
 
     def service_hook(repo)
-      hooks(repo).detect { |h| h['name'] == 'travis' && h.dig('config', 'domain') == service_hook_url.host }
+      hooks(repo).detect { |h| h['name'] == 'travis' && h.dig('config', 'domain') == service_hook_url.to_s }
     end
 
     def service_hook_url?(repo)
diff --git a/lib/travis/api/v3/services/user_setting/update.rb b/lib/travis/api/v3/services/user_setting/update.rb
@@ -5,10 +5,12 @@ class Services::UserSetting::Update < Service
 
     def run!
       repository = check_login_and_find(:repository)
-      user_setting = query.update(repository)
       return repo_migrated if migrated?(repository)
-
+      
+      user_setting = query.find(repository)
       access_control.permissions(user_setting).write!
+      
+      user_setting = query.update(repository)      
       result user_setting
     end
   end
diff --git a/spec/unit/middleware/user_agent_tracker_spec.rb b/spec/unit/middleware/user_agent_tracker_spec.rb
@@ -3,16 +3,17 @@
     mock_app do
       use Travis::Api::App::Middleware::UserAgentTracker
       get('/') { 'ok' }
+      get('/uptime') { 'OK' }
     end
   end
 
   def expect_meter(name)
     allow(Metriks).to receive(:meter).with(name).and_return(double("meter", mark: nil))
   end
 
-  def get(env = {})
+  def get(env = {}, path = '/')
     env['HTTP_USER_AGENT'] ||= agent if agent
-    super('/', {}, env)
+    super(path, {}, env)
   end
 
   context 'missing User-Agent' do
@@ -27,13 +28,18 @@ def get(env = {})
       allow(Travis::Features).to receive(:feature_active?).with(:require_user_agent).and_return(true)
       expect(get.status).to eq(400)
     end
+
+    specify do
+      expect(Metriks).to_not receive(:meter)
+      get({}, '/uptime')
+    end
   end
 
   context 'web browser' do
     let(:agent) { "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_9_5) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/39.0.2171.36 Safari/537.36" }
 
     specify 'without X-User-Agent' do
-      expect_meter("api.v2.user_agent.browser.unknown")
+      expect_meter("api.v2.user_agent.browser.chrome")
       get
     end
 
@@ -89,6 +95,18 @@ def get(env = {})
       expect_meter("api.v2.user_agent.script.ruby.travis.version.1.6.8")
       get
     end
+
+    context 'Travis-API-Version header' do
+      specify "with Travis-API-Version: 3" do
+        expect_meter("api.v3.user_agent.script.ruby.travis.version.1.6.8")
+        get('HTTP_TRAVIS_API_VERSION' => '3')
+      end
+
+      specify "with Travis-API-Version: 1.7f" do
+        expect_meter("api.v1.7.user_agent.script.ruby.travis.version.1.6.8")
+        get('HTTP_TRAVIS_API_VERSION' => '1.7')
+      end
+    end
   end
 
   context 'Travis CLI' do
@@ -99,4 +117,12 @@ def get(env = {})
       get
     end
   end
+
+  context 'get /uptime' do
+    let(:agent) { 'Travis/1.6.8 (Mac OS X 10.9.2 like Darwin; Ruby 2.1.1p42; RubyGems 2.0.14) Faraday/0.8.9 Typhoeus/0.6.7' }
+    specify do
+      expect(Metriks).to_not receive(:meter)
+      get({}, '/uptime')
+    end
+  end
 end
diff --git a/spec/v3/services/repository/activate_spec.rb b/spec/v3/services/repository/activate_spec.rb
@@ -71,7 +71,7 @@
           stub_request(:get, "https://api.github.com/repositories/#{repo.github_id}/hooks?per_page=100").to_return(
             status: 200, body: JSON.dump(
               [
-                { name: 'travis', url: "https://api.github.com/repositories/#{repo.github_id}/hooks/123", config: { domain: 'url.of.listener.something' } },
+                { name: 'travis', url: "https://api.github.com/repositories/#{repo.github_id}/hooks/123", config: { domain: 'https://url.of.listener.something' } },
                 { name: 'web', url: "https://api.github.com/repositories/#{repo.github_id}/hooks/456", config: { url: Travis.config.service_hook_url } }
               ]
             )
diff --git a/spec/v3/services/repository/deactivate_spec.rb b/spec/v3/services/repository/deactivate_spec.rb
@@ -62,7 +62,7 @@
           stub_request(:get, "https://api.github.com/repositories/#{repo.github_id}/hooks?per_page=100").to_return(
             status: 200, body: JSON.dump(
               [
-                { name: 'travis', url: "https://api.github.com/repositories/#{repo.github_id}/hooks/123", config: { domain: 'url.of.listener.something' } },
+                { name: 'travis', url: "https://api.github.com/repositories/#{repo.github_id}/hooks/123", config: { domain: 'https://url.of.listener.something' } },
                 { name: 'web', url: "https://api.github.com/repositories/#{repo.github_id}/hooks/456", config: { url: Travis.config.service_hook_url } }
               ]
             )
@@ -104,7 +104,7 @@
           stub_request(:get, "https://api.github.com/repositories/#{repo.github_id}/hooks?per_page=100").to_return(
             status: 200, body: JSON.dump(
               [
-                { name: 'travis', url: "https://api.github.com/repositories/#{repo.github_id}/hooks/123", config: { domain: 'url.of.listener.something' } }
+                { name: 'travis', url: "https://api.github.com/repositories/#{repo.github_id}/hooks/123", config: { domain: 'https://url.of.listener.something' } }
               ]
             )
           )
diff --git a/spec/v3/services/user_setting/update_spec.rb b/spec/v3/services/user_setting/update_spec.rb
@@ -96,7 +96,7 @@
           '@href' => "/v3/repo/#{repo.id}/setting/build_pushes",
           '@representation' => 'minimal',
           'name' => 'build_pushes',
-          'value' => false
+          'value' => true
         }
       )
     end
diff --git a/spec/v3/services/user_settings/for_repository_spec.rb b/spec/v3/services/user_settings/for_repository_spec.rb
@@ -4,7 +4,7 @@
   let(:auth_headers) { { 'HTTP_AUTHORIZATION' => "token #{token}" } }
   let(:json_headers) { { 'CONTENT_TYPE' => 'application/json' } }
 
-  before { Travis::API::V3::Models::Permission.create(repository: repo, user: repo.owner, pull: true, admin: false) }
+  before { Travis::API::V3::Models::Permission.create(repository: repo, user: repo.owner, pull: true, push: true, admin: false) }
 
   describe 'not authenticated' do
     before { get("/v3/repo/#{repo.id}/settings") }
@@ -42,13 +42,13 @@
           '@href' => "/v3/repo/#{repo.id}/settings",
           '@representation' => 'standard',
           'settings' => [
-            { '@type' => 'setting', '@href' => "/v3/repo/#{repo.id}/setting/builds_only_with_travis_yml", '@representation' => 'standard', '@permissions' => { 'read' => true, 'write' => false }, 'name' => 'builds_only_with_travis_yml', 'value' => false },
-            { '@type' => 'setting', '@href' => "/v3/repo/#{repo.id}/setting/build_pushes",                '@representation' => 'standard', '@permissions' => { 'read' => true, 'write' => false }, 'name' => 'build_pushes',                'value' => true },
-            { '@type' => 'setting', '@href' => "/v3/repo/#{repo.id}/setting/build_pull_requests",         '@representation' => 'standard', '@permissions' => { 'read' => true, 'write' => false }, 'name' => 'build_pull_requests',         'value' => true },
-            { '@type' => 'setting', '@href' => "/v3/repo/#{repo.id}/setting/maximum_number_of_builds",    '@representation' => 'standard', '@permissions' => { 'read' => true, 'write' => false }, 'name' => 'maximum_number_of_builds',    'value' => 0 },
-            { '@type' => 'setting', '@href' => "/v3/repo/#{repo.id}/setting/auto_cancel_pushes",          '@representation' => 'standard', '@permissions' => { 'read' => true, 'write' => false }, 'name' => 'auto_cancel_pushes',          'value' => false },
-            { '@type' => 'setting', '@href' => "/v3/repo/#{repo.id}/setting/auto_cancel_pull_requests",   '@representation' => 'standard', '@permissions' => { 'read' => true, 'write' => false }, 'name' => 'auto_cancel_pull_requests',   'value' => false },
-            { '@type' => 'setting', '@href' => "/v3/repo/#{repo.id}/setting/config_validation",           '@representation' => 'standard', '@permissions' => { 'read' => true, 'write' => false }, 'name' => 'config_validation',           'value' => false },
+            { '@type' => 'setting', '@href' => "/v3/repo/#{repo.id}/setting/builds_only_with_travis_yml", '@representation' => 'standard', '@permissions' => { 'read' => true, 'write' => true }, 'name' => 'builds_only_with_travis_yml', 'value' => false },
+            { '@type' => 'setting', '@href' => "/v3/repo/#{repo.id}/setting/build_pushes",                '@representation' => 'standard', '@permissions' => { 'read' => true, 'write' => true }, 'name' => 'build_pushes',                'value' => true },
+            { '@type' => 'setting', '@href' => "/v3/repo/#{repo.id}/setting/build_pull_requests",         '@representation' => 'standard', '@permissions' => { 'read' => true, 'write' => true }, 'name' => 'build_pull_requests',         'value' => true },
+            { '@type' => 'setting', '@href' => "/v3/repo/#{repo.id}/setting/maximum_number_of_builds",    '@representation' => 'standard', '@permissions' => { 'read' => true, 'write' => true }, 'name' => 'maximum_number_of_builds',    'value' => 0 },
+            { '@type' => 'setting', '@href' => "/v3/repo/#{repo.id}/setting/auto_cancel_pushes",          '@representation' => 'standard', '@permissions' => { 'read' => true, 'write' => true }, 'name' => 'auto_cancel_pushes',          'value' => false },
+            { '@type' => 'setting', '@href' => "/v3/repo/#{repo.id}/setting/auto_cancel_pull_requests",   '@representation' => 'standard', '@permissions' => { 'read' => true, 'write' => true }, 'name' => 'auto_cancel_pull_requests',   'value' => false },
+            { '@type' => 'setting', '@href' => "/v3/repo/#{repo.id}/setting/config_validation",           '@representation' => 'standard', '@permissions' => { 'read' => true, 'write' => true }, 'name' => 'config_validation',           'value' => false },
           ]
         )
       end
@@ -60,7 +60,7 @@
 
       example do
         expect(JSON.load(body)['settings']).to include(
-          { '@type' => 'setting', '@permissions' => { 'read' => true, 'write' => false }, '@href' => "/v3/repo/#{repo.id}/setting/allow_config_imports", '@representation' => 'standard', 'name' => 'allow_config_imports', 'value' => false },
+          { '@type' => 'setting', '@permissions' => { 'read' => true, 'write' => true }, '@href' => "/v3/repo/#{repo.id}/setting/allow_config_imports", '@representation' => 'standard', 'name' => 'allow_config_imports', 'value' => false },
         )
       end
     end
@@ -79,13 +79,13 @@
         '@href' => "/v3/repo/#{repo.id}/settings",
         '@representation' => 'standard',
         'settings' => [
-          { '@type' => 'setting', '@href' => "/v3/repo/#{repo.id}/setting/builds_only_with_travis_yml", '@representation' => 'standard', '@permissions' => { 'read' => true, 'write' => false }, 'name' => 'builds_only_with_travis_yml', 'value' => false },
-          { '@type' => 'setting', '@href' => "/v3/repo/#{repo.id}/setting/build_pushes",                '@representation' => 'standard', '@permissions' => { 'read' => true, 'write' => false }, 'name' => 'build_pushes',                'value' => false },
-          { '@type' => 'setting', '@href' => "/v3/repo/#{repo.id}/setting/build_pull_requests",         '@representation' => 'standard', '@permissions' => { 'read' => true, 'write' => false }, 'name' => 'build_pull_requests',         'value' => true },
-          { '@type' => 'setting', '@href' => "/v3/repo/#{repo.id}/setting/maximum_number_of_builds",    '@representation' => 'standard', '@permissions' => { 'read' => true, 'write' => false }, 'name' => 'maximum_number_of_builds',    'value' => 0 },
-          { '@type' => 'setting', '@href' => "/v3/repo/#{repo.id}/setting/auto_cancel_pushes",          '@representation' => 'standard', '@permissions' => { 'read' => true, 'write' => false }, 'name' => 'auto_cancel_pushes',          'value' => false },
-          { '@type' => 'setting', '@href' => "/v3/repo/#{repo.id}/setting/auto_cancel_pull_requests",   '@representation' => 'standard', '@permissions' => { 'read' => true, 'write' => false }, 'name' => 'auto_cancel_pull_requests',   'value' => false },
-          { '@type' => 'setting', '@href' => "/v3/repo/#{repo.id}/setting/config_validation",           '@representation' => 'standard', '@permissions' => { 'read' => true, 'write' => false }, 'name' => 'config_validation',           'value' => false },
+          { '@type' => 'setting', '@href' => "/v3/repo/#{repo.id}/setting/builds_only_with_travis_yml", '@representation' => 'standard', '@permissions' => { 'read' => true, 'write' => true }, 'name' => 'builds_only_with_travis_yml', 'value' => false },
+          { '@type' => 'setting', '@href' => "/v3/repo/#{repo.id}/setting/build_pushes",                '@representation' => 'standard', '@permissions' => { 'read' => true, 'write' => true }, 'name' => 'build_pushes',                'value' => false },
+          { '@type' => 'setting', '@href' => "/v3/repo/#{repo.id}/setting/build_pull_requests",         '@representation' => 'standard', '@permissions' => { 'read' => true, 'write' => true }, 'name' => 'build_pull_requests',         'value' => true },
+          { '@type' => 'setting', '@href' => "/v3/repo/#{repo.id}/setting/maximum_number_of_builds",    '@representation' => 'standard', '@permissions' => { 'read' => true, 'write' => true }, 'name' => 'maximum_number_of_builds',    'value' => 0 },
+          { '@type' => 'setting', '@href' => "/v3/repo/#{repo.id}/setting/auto_cancel_pushes",          '@representation' => 'standard', '@permissions' => { 'read' => true, 'write' => true }, 'name' => 'auto_cancel_pushes',          'value' => false },
+          { '@type' => 'setting', '@href' => "/v3/repo/#{repo.id}/setting/auto_cancel_pull_requests",   '@representation' => 'standard', '@permissions' => { 'read' => true, 'write' => true }, 'name' => 'auto_cancel_pull_requests',   'value' => false },
+          { '@type' => 'setting', '@href' => "/v3/repo/#{repo.id}/setting/config_validation",           '@representation' => 'standard', '@permissions' => { 'read' => true, 'write' => true }, 'name' => 'config_validation',           'value' => false },
         ]
       )
     end
@@ -104,7 +104,7 @@
         '@type' => 'setting',
         '@href' => "/v3/repo/#{repo.id}/setting/build_pushes",
         '@representation' => 'standard',
-        '@permissions' => { 'read' => true, 'write' => false },
+        '@permissions' => { 'read' => true, 'write' => true },
         'name' => 'build_pushes',
         'value' => false
       )

Original file line number	Diff line number	Diff line change
`@@ -71,7 +71,7 @@`
`71`	`71`	`stub_request(:get, "https://api.github.com/repositories/#{repo.github_id}/hooks?per_page=100").to_return(`
`72`	`72`	`status: 200, body: JSON.dump(`
`73`	`73`	`[`
`74`		`- { name: 'travis', url: "https://api.github.com/repositories/#{repo.github_id}/hooks/123", config: { domain: 'url.of.listener.something' } },`
	`74`	`+ { name: 'travis', url: "https://api.github.com/repositories/#{repo.github_id}/hooks/123", config: { domain: 'https://url.of.listener.something' } },`
`75`	`75`	`{ name: 'web', url: "https://api.github.com/repositories/#{repo.github_id}/hooks/456", config: { url: Travis.config.service_hook_url } }`
`76`	`76`	`]`
`77`	`77`	`)`
Original file line number	Diff line number	Diff line change
`@@ -62,7 +62,7 @@`
`62`	`62`	`stub_request(:get, "https://api.github.com/repositories/#{repo.github_id}/hooks?per_page=100").to_return(`
`63`	`63`	`status: 200, body: JSON.dump(`
`64`	`64`	`[`
`65`		`- { name: 'travis', url: "https://api.github.com/repositories/#{repo.github_id}/hooks/123", config: { domain: 'url.of.listener.something' } },`
	`65`	`+ { name: 'travis', url: "https://api.github.com/repositories/#{repo.github_id}/hooks/123", config: { domain: 'https://url.of.listener.something' } },`
`66`	`66`	`{ name: 'web', url: "https://api.github.com/repositories/#{repo.github_id}/hooks/456", config: { url: Travis.config.service_hook_url } }`
`67`	`67`	`]`
`68`	`68`	`)`
`@@ -104,7 +104,7 @@`
`104`	`104`	`stub_request(:get, "https://api.github.com/repositories/#{repo.github_id}/hooks?per_page=100").to_return(`
`105`	`105`	`status: 200, body: JSON.dump(`
`106`	`106`	`[`
`107`		`- { name: 'travis', url: "https://api.github.com/repositories/#{repo.github_id}/hooks/123", config: { domain: 'url.of.listener.something' } }`
	`107`	`+ { name: 'travis', url: "https://api.github.com/repositories/#{repo.github_id}/hooks/123", config: { domain: 'https://url.of.listener.something' } }`
`108`	`108`	`]`
`109`	`109`	`)`
`110`	`110`	`)`
Original file line number	Diff line number	Diff line change
`@@ -96,7 +96,7 @@`
`96`	`96`	`'@href' => "/v3/repo/#{repo.id}/setting/build_pushes",`
`97`	`97`	`'@representation' => 'minimal',`
`98`	`98`	`'name' => 'build_pushes',`
`99`		`- 'value' => false`
	`99`	`+ 'value' => true`
`100`	`100`	`}`
`101`	`101`	`)`
`102`	`102`	`end`