Test all enabled routes in ETag caching middleware and fix pipeline routes

Extract route matching logic to Gitlab::EtagCaching::Router.

Fix pipeline routes:
1. "project_pipelines" has to come after "commit_pipelines" and
"merge_request_pipelines" because it is more generic
2. "commit_pipelines": "\s" (any whitespace character) => "\S"
(any non-whitespace character).

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

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

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