diff --git a/lib/gitlab/etag_caching/middleware.rb b/lib/gitlab/etag_caching/middleware.rb index 11167632e07..270d67dd50c 100644 --- a/lib/gitlab/etag_caching/middleware.rb +++ b/lib/gitlab/etag_caching/middleware.rb @@ -1,40 +1,12 @@ module Gitlab module EtagCaching class Middleware - RESERVED_WORDS = NamespaceValidator::WILDCARD_ROUTES.map { |word| "/#{word}/" }.join('|') - ROUTES = [ - { - regexp: %r(^(?!.*(#{RESERVED_WORDS})).*/noteable/issue/\d+/notes\z), - name: 'issue_notes' - }, - { - regexp: %r(^(?!.*(#{RESERVED_WORDS})).*/issues/\d+/rendered_title\z), - name: 'issue_title' - }, - { - regexp: %r(^(?!.*(#{RESERVED_WORDS})).*/pipelines\.json\z), - name: 'project_pipelines' - }, - { - regexp: %r(^(?!.*(#{RESERVED_WORDS})).*/commit/\s+/pipelines\.json\z), - name: 'commit_pipelines' - }, - { - regexp: %r(^(?!.*(#{RESERVED_WORDS})).*/merge_requests/new\.json\z), - name: 'new_merge_request_pipelines' - }, - { - regexp: %r(^(?!.*(#{RESERVED_WORDS})).*/merge_requests/\d+/pipelines\.json\z), - name: 'merge_request_pipelines' - } - ].freeze - def initialize(app) @app = app end def call(env) - route = match_current_route(env) + route = Gitlab::EtagCaching::Router.match(env) return @app.call(env) unless route track_event(:etag_caching_middleware_used, route) @@ -55,10 +27,6 @@ module Gitlab private - def match_current_route(env) - ROUTES.find { |route| route[:regexp].match(env['PATH_INFO']) } - end - def get_etag(env) cache_key = env['PATH_INFO'] store = Gitlab::EtagCaching::Store.new @@ -95,7 +63,7 @@ module Gitlab end def track_event(name, route) - Gitlab::Metrics.add_event(name, endpoint: route[:name]) + Gitlab::Metrics.add_event(name, endpoint: route.name) end end end diff --git a/lib/gitlab/etag_caching/router.rb b/lib/gitlab/etag_caching/router.rb new file mode 100644 index 00000000000..f6e4f279c06 --- /dev/null +++ b/lib/gitlab/etag_caching/router.rb @@ -0,0 +1,39 @@ +module Gitlab + module EtagCaching + class Router + Route = Struct.new(:regexp, :name) + + RESERVED_WORDS = NamespaceValidator::WILDCARD_ROUTES.map { |word| "/#{word}/" }.join('|') + ROUTES = [ + Gitlab::EtagCaching::Router::Route.new( + %r(^(?!.*(#{RESERVED_WORDS})).*/noteable/issue/\d+/notes\z), + 'issue_notes' + ), + Gitlab::EtagCaching::Router::Route.new( + %r(^(?!.*(#{RESERVED_WORDS})).*/issues/\d+/rendered_title\z), + 'issue_title' + ), + Gitlab::EtagCaching::Router::Route.new( + %r(^(?!.*(#{RESERVED_WORDS})).*/commit/\S+/pipelines\.json\z), + 'commit_pipelines' + ), + Gitlab::EtagCaching::Router::Route.new( + %r(^(?!.*(#{RESERVED_WORDS})).*/merge_requests/new\.json\z), + 'new_merge_request_pipelines' + ), + Gitlab::EtagCaching::Router::Route.new( + %r(^(?!.*(#{RESERVED_WORDS})).*/merge_requests/\d+/pipelines\.json\z), + 'merge_request_pipelines' + ), + Gitlab::EtagCaching::Router::Route.new( + %r(^(?!.*(#{RESERVED_WORDS})).*/pipelines\.json\z), + 'project_pipelines' + ) + ].freeze + + def self.match(env) + ROUTES.find { |route| route.regexp.match(env['PATH_INFO']) } + end + end + end +end diff --git a/spec/lib/gitlab/etag_caching/router_spec.rb b/spec/lib/gitlab/etag_caching/router_spec.rb new file mode 100644 index 00000000000..f3dacb4ef04 --- /dev/null +++ b/spec/lib/gitlab/etag_caching/router_spec.rb @@ -0,0 +1,83 @@ +require 'spec_helper' + +describe Gitlab::EtagCaching::Router do + it 'matches issue notes endpoint' do + env = build_env( + '/my-group/and-subgroup/here-comes-the-project/noteable/issue/1/notes' + ) + + result = described_class.match(env) + + expect(result).to be_present + expect(result.name).to eq 'issue_notes' + end + + it 'matches issue title endpoint' do + env = build_env( + '/my-group/my-project/issues/123/rendered_title' + ) + + result = described_class.match(env) + + expect(result).to be_present + expect(result.name).to eq 'issue_title' + end + + it 'matches project pipelines endpoint' do + env = build_env( + '/my-group/my-project/pipelines.json' + ) + + result = described_class.match(env) + + expect(result).to be_present + expect(result.name).to eq 'project_pipelines' + end + + it 'matches commit pipelines endpoint' do + env = build_env( + '/my-group/my-project/commit/aa8260d253a53f73f6c26c734c72fdd600f6e6d4/pipelines.json' + ) + + result = described_class.match(env) + + expect(result).to be_present + expect(result.name).to eq 'commit_pipelines' + end + + it 'matches new merge request pipelines endpoint' do + env = build_env( + '/my-group/my-project/merge_requests/new.json' + ) + + result = described_class.match(env) + + expect(result).to be_present + expect(result.name).to eq 'new_merge_request_pipelines' + end + + it 'matches merge request pipelines endpoint' do + env = build_env( + '/my-group/my-project/merge_requests/234/pipelines.json' + ) + + result = described_class.match(env) + + expect(result).to be_present + expect(result.name).to eq 'merge_request_pipelines' + end + + it 'does not match blob with confusing name' do + env = build_env( + '/my-group/my-project/blob/master/pipelines.json' + ) + + result = described_class.match(env) + + expect(result).to be_blank + end + + def build_env(path) + { 'PATH_INFO' => path } + end +end