Merge branch 'add-graphql-etag-caching' into 'master'

ETag caching for GraphQL polling

See merge request gitlab-org/gitlab!53978

app/services/ci/expire_pipeline_cache_service.rb

@@ -47,6 +47,10 @@ module Ci
       end
     end
 
+    def graphql_pipeline_path(pipeline)
+      [Gitlab::Routing.url_helpers.api_graphql_path, "pipelines/id/#{pipeline.id}"].join(':')
+    end
+
     # Updates ETag caches of a pipeline.
     #
     # This logic resides in a separate method so that EE can more easily extend
@@ -61,6 +65,7 @@ module Ci
       store.touch(project_pipeline_path(project, pipeline))
       store.touch(commit_pipelines_path(project, pipeline.commit)) unless pipeline.commit.nil?
       store.touch(new_merge_request_pipelines_path(project))
+      store.touch(graphql_pipeline_path(pipeline))
       each_pipelines_merge_request_path(pipeline) do |path|
         store.touch(path)
       end

changelogs/unreleased/add-graphql-etag-caching.yml

+---
+title: Add support for ETag caching when using GraphQL
+merge_request: 53978
+author:
+type: changed

config/routes/api.rb

 # frozen_string_literal: true
 
-post '/api/graphql', to: 'graphql#execute'
+match '/api/graphql', via: [:get, :post], to: 'graphql#execute'
 mount GraphiQL::Rails::Engine, at: '/-/graphql-explorer', graphql_path: Gitlab::Utils.append_path(Gitlab.config.gitlab.relative_url_root, '/api/graphql')
 
 ::API::API.logger Rails.logger # rubocop:disable Gitlab/RailsLogger

ee/lib/ee/gitlab/etag_caching/router.rb

-# frozen_string_literal: true
-
-module EE
-  module Gitlab
-    module EtagCaching
-      module Router
-        EE_ROUTES = [
-          ::Gitlab::EtagCaching::Router::Route.new(
-            %r(^/groups/#{::Gitlab::PathRegex.full_namespace_route_regex}/-/epics/\d+/notes\z),
-            'epic_notes',
-            'epics'
-          )
-        ].freeze
-
-        module ClassMethods
-          def match(path)
-            EE_ROUTES.find { |route| route.regexp.match(path) } || super
-          end
-        end
-
-        def self.prepended(base)
-          base.singleton_class.prepend ClassMethods
-        end
-      end
-    end
-  end
-end

ee/lib/ee/gitlab/etag_caching/router/restful.rb

+# frozen_string_literal: true
+
+module EE
+  module Gitlab
+    module EtagCaching
+      module Router
+        module Restful
+          extend ActiveSupport::Concern
+
+          EE_ROUTE_DEFINITONS = [
+            [
+              %r(^/groups/#{::Gitlab::PathRegex.full_namespace_route_regex}/-/epics/\d+/notes\z),
+              'epic_notes',
+              'epics'
+            ]
+          ].freeze
+
+          class_methods do
+            extend ::Gitlab::Utils::Override
+            include ::Gitlab::Utils::StrongMemoize
+            include ::Gitlab::EtagCaching::Router::Helpers
+
+            override :all_routes
+            def all_routes
+              strong_memoize(:all_routes) do
+                super + ee_routes
+              end
+            end
+
+            def ee_routes
+              EE_ROUTE_DEFINITONS.map(&method(:build_route))
+            end
+          end
+        end
+      end
+    end
+  end
+end

ee/spec/lib/ee/gitlab/etag_caching/router_spec.rb → ee/spec/lib/ee/gitlab/etag_caching/router/restful_spec.rb

@@ -2,10 +2,10 @@
 
 require 'spec_helper'
 
-RSpec.describe Gitlab::EtagCaching::Router do
+RSpec.describe Gitlab::EtagCaching::Router::Restful do
   it 'matches epic notes endpoint' do
     result = described_class.match(
-      '/groups/my-group/and-subgroup/-/epics/1/notes'
+      double(path_info: '/groups/my-group/and-subgroup/-/epics/1/notes')
     )
 
     expect(result).to be_present
@@ -14,7 +14,7 @@ RSpec.describe Gitlab::EtagCaching::Router do
 
   it 'does not match invalid epic notes endpoint' do
     result = described_class.match(
-      '/groups/my-group/-/and-subgroup/-/epics/1/notes'
+      double(path_info: '/groups/my-group/-/and-subgroup/-/epics/1/notes')
     )
 
     expect(result).to be_blank
@@ -23,8 +23,18 @@ RSpec.describe Gitlab::EtagCaching::Router do
   it 'has a valid feature category for every route', :aggregate_failures do
     feature_categories = YAML.load_file(Rails.root.join('config', 'feature_categories.yml')).to_set
 
-    described_class::EE_ROUTES.each do |route|
+    described_class.ee_routes.each do |route|
       expect(feature_categories).to include(route.feature_category), "#{route.name} has a category of #{route.feature_category}, which is not valid"
     end
   end
+
+  describe '.cache_key' do
+    it 'returns a cache key' do
+      request = double(path: '/path/to/resource')
+
+      described_class.ee_routes.each do |route|
+        expect(route.cache_key(request)).to eq '/path/to/resource'
+      end
+    end
+  end
 end

lib/gitlab/etag_caching/middleware.rb

@@ -17,12 +17,12 @@ module Gitlab
 
       def call(env)
         request = ActionDispatch::Request.new(env)
-        route = Gitlab::EtagCaching::Router.match(request.path_info)
+        route = Gitlab::EtagCaching::Router.match(request)
         return @app.call(env) unless route
 
         track_event(:etag_caching_middleware_used, route)
 
-        etag, cached_value_present = get_etag(request)
+        etag, cached_value_present = get_etag(request, route)
         if_none_match = env['HTTP_IF_NONE_MATCH']
 
         if if_none_match == etag
@@ -36,8 +36,8 @@ module Gitlab
 
       private
 
-      def get_etag(request)
-        cache_key = request.path
+      def get_etag(request, route)
+        cache_key = route.cache_key(request)
         store = Gitlab::EtagCaching::Store.new
         current_value = store.get(cache_key)
         cached_value_present = current_value.present?

lib/gitlab/etag_caching/router.rb

@@ -2,99 +2,24 @@
 
 module Gitlab
   module EtagCaching
-    class Router
-      Route = Struct.new(:regexp, :name, :feature_category)
-      # We enable an ETag for every request matching the regex.
-      # To match a regex the path needs to match the following:
-      #   - Don't contain a reserved word (expect for the words used in the
-      #     regex itself)
-      #   - Ending in `noteable/issue/<id>/notes` for the `issue_notes` route
-      #   - Ending in `issues/id`/realtime_changes` for the `issue_title` route
-      USED_IN_ROUTES = %w[noteable issue notes issues realtime_changes
-                          commit pipelines merge_requests builds
-                          new environments].freeze
-      RESERVED_WORDS = Gitlab::PathRegex::ILLEGAL_PROJECT_PATH_WORDS - USED_IN_ROUTES
-      RESERVED_WORDS_REGEX = Regexp.union(*RESERVED_WORDS.map(&Regexp.method(:escape)))
-      RESERVED_WORDS_PREFIX = %Q(^(?!.*\/(#{RESERVED_WORDS_REGEX})\/).*)
+    module Router
+      Route = Struct.new(:regexp, :name, :feature_category, :router) do
+        delegate :match, to: :regexp
+        delegate :cache_key, to: :router
+      end
 
-      ROUTES = [
-        Gitlab::EtagCaching::Router::Route.new(
-          %r(#{RESERVED_WORDS_PREFIX}/noteable/issue/\d+/notes\z),
-          'issue_notes',
-          'issue_tracking'
-        ),
-        Gitlab::EtagCaching::Router::Route.new(
-          %r(#{RESERVED_WORDS_PREFIX}/noteable/merge_request/\d+/notes\z),
-          'merge_request_notes',
-          'code_review'
-        ),
-        Gitlab::EtagCaching::Router::Route.new(
-          %r(#{RESERVED_WORDS_PREFIX}/issues/\d+/realtime_changes\z),
-          'issue_title',
-          'issue_tracking'
-        ),
-        Gitlab::EtagCaching::Router::Route.new(
-          %r(#{RESERVED_WORDS_PREFIX}/commit/\S+/pipelines\.json\z),
-          'commit_pipelines',
-          'continuous_integration'
-        ),
-        Gitlab::EtagCaching::Router::Route.new(
-          %r(#{RESERVED_WORDS_PREFIX}/merge_requests/new\.json\z),
-          'new_merge_request_pipelines',
-          'continuous_integration'
-        ),
-        Gitlab::EtagCaching::Router::Route.new(
-          %r(#{RESERVED_WORDS_PREFIX}/merge_requests/\d+/pipelines\.json\z),
-          'merge_request_pipelines',
-          'continuous_integration'
-        ),
-        Gitlab::EtagCaching::Router::Route.new(
-          %r(#{RESERVED_WORDS_PREFIX}/pipelines\.json\z),
-          'project_pipelines',
-          'continuous_integration'
-        ),
-        Gitlab::EtagCaching::Router::Route.new(
-          %r(#{RESERVED_WORDS_PREFIX}/pipelines/\d+\.json\z),
-          'project_pipeline',
-          'continuous_integration'
-        ),
-        Gitlab::EtagCaching::Router::Route.new(
-          %r(#{RESERVED_WORDS_PREFIX}/builds/\d+\.json\z),
-          'project_build',
-          'continuous_integration'
-        ),
-        Gitlab::EtagCaching::Router::Route.new(
-          %r(#{RESERVED_WORDS_PREFIX}/clusters/\d+/environments\z),
-          'cluster_environments',
-          'continuous_delivery'
-        ),
-        Gitlab::EtagCaching::Router::Route.new(
-          %r(#{RESERVED_WORDS_PREFIX}/environments\.json\z),
-          'environments',
-          'continuous_delivery'
-        ),
-        Gitlab::EtagCaching::Router::Route.new(
-          %r(#{RESERVED_WORDS_PREFIX}/import/github/realtime_changes\.json\z),
-          'realtime_changes_import_github',
-          'importers'
-        ),
-        Gitlab::EtagCaching::Router::Route.new(
-          %r(#{RESERVED_WORDS_PREFIX}/import/gitea/realtime_changes\.json\z),
-          'realtime_changes_import_gitea',
-          'importers'
-        ),
-        Gitlab::EtagCaching::Router::Route.new(
-          %r(#{RESERVED_WORDS_PREFIX}/merge_requests/\d+/cached_widget\.json\z),
-          'merge_request_widget',
-          'code_review'
-        )
-      ].freeze
+      module Helpers
+        def build_route(attrs)
+          EtagCaching::Router::Route.new(*attrs, self)
+        end
+      end
 
-      def self.match(path)
-        ROUTES.find { |route| route.regexp.match(path) }
+      # Performing RESTful routing match before GraphQL would be more expensive
+      # for the GraphQL requests because we need to traverse all of the RESTful
+      # route definitions before falling back to GraphQL.
+      def self.match(request)
+        Router::Graphql.match(request) || Router::Restful.match(request)
       end
     end
   end
 end
-
-Gitlab::EtagCaching::Router.prepend_if_ee('EE::Gitlab::EtagCaching::Router')

lib/gitlab/etag_caching/router/graphql.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module EtagCaching
+    module Router
+      class Graphql
+        extend EtagCaching::Router::Helpers
+        GRAPHQL_ETAG_RESOURCE_HEADER = 'X-GITLAB-GRAPHQL-RESOURCE-ETAG'
+
+        ROUTES = [
+          [
+            %r(\Apipelines/id/\d+\z),
+            'pipelines_graph',
+            'continuous_integration'
+          ]
+        ].map(&method(:build_route)).freeze
+
+        def self.match(request)
+          return unless request.path_info == graphql_api_path
+
+          graphql_resource = request.headers[GRAPHQL_ETAG_RESOURCE_HEADER]
+          return unless graphql_resource
+
+          ROUTES.find { |route| route.match(graphql_resource) }
+        end
+
+        def self.cache_key(request)
+          [
+            request.path,
+            request.headers[GRAPHQL_ETAG_RESOURCE_HEADER]
+          ].compact.join(':')
+        end
+
+        def self.graphql_api_path
+          @graphql_api_path ||= Gitlab::Routing.url_helpers.api_graphql_path
+        end
+        private_class_method :graphql_api_path
+      end
+    end
+  end
+end

lib/gitlab/etag_caching/router/restful.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module EtagCaching
+    module Router
+      class Restful
+        extend EtagCaching::Router::Helpers
+
+        # We enable an ETag for every request matching the regex.
+        # To match a regex the path needs to match the following:
+        #   - Don't contain a reserved word (expect for the words used in the
+        #     regex itself)
+        #   - Ending in `noteable/issue/<id>/notes` for the `issue_notes` route
+        #   - Ending in `issues/id`/realtime_changes` for the `issue_title` route
+        USED_IN_ROUTES = %w[noteable issue notes issues realtime_changes
+                            commit pipelines merge_requests builds
+                            new environments].freeze
+        RESERVED_WORDS = Gitlab::PathRegex::ILLEGAL_PROJECT_PATH_WORDS - USED_IN_ROUTES
+        RESERVED_WORDS_REGEX = Regexp.union(*RESERVED_WORDS.map(&Regexp.method(:escape)))
+        RESERVED_WORDS_PREFIX = %Q(^(?!.*\/(#{RESERVED_WORDS_REGEX})\/).*)
+
+        ROUTES = [
+          [
+            %r(#{RESERVED_WORDS_PREFIX}/noteable/issue/\d+/notes\z),
+            'issue_notes',
+            'issue_tracking'
+          ],
+          [
+            %r(#{RESERVED_WORDS_PREFIX}/noteable/merge_request/\d+/notes\z),
+            'merge_request_notes',
+            'code_review'
+          ],
+          [
+            %r(#{RESERVED_WORDS_PREFIX}/issues/\d+/realtime_changes\z),
+            'issue_title',
+            'issue_tracking'
+          ],
+          [
+            %r(#{RESERVED_WORDS_PREFIX}/commit/\S+/pipelines\.json\z),
+            'commit_pipelines',
+            'continuous_integration'
+          ],
+          [
+            %r(#{RESERVED_WORDS_PREFIX}/merge_requests/new\.json\z),
+            'new_merge_request_pipelines',
+            'continuous_integration'
+          ],
+          [
+            %r(#{RESERVED_WORDS_PREFIX}/merge_requests/\d+/pipelines\.json\z),
+            'merge_request_pipelines',
+            'continuous_integration'
+          ],
+          [
+            %r(#{RESERVED_WORDS_PREFIX}/pipelines\.json\z),
+            'project_pipelines',
+            'continuous_integration'
+          ],
+          [
+            %r(#{RESERVED_WORDS_PREFIX}/pipelines/\d+\.json\z),
+            'project_pipeline',
+            'continuous_integration'
+          ],
+          [
+            %r(#{RESERVED_WORDS_PREFIX}/builds/\d+\.json\z),
+            'project_build',
+            'continuous_integration'
+          ],
+          [
+            %r(#{RESERVED_WORDS_PREFIX}/clusters/\d+/environments\z),
+            'cluster_environments',
+            'continuous_delivery'
+          ],
+          [
+            %r(#{RESERVED_WORDS_PREFIX}/environments\.json\z),
+            'environments',
+            'continuous_delivery'
+          ],
+          [
+            %r(#{RESERVED_WORDS_PREFIX}/import/github/realtime_changes\.json\z),
+            'realtime_changes_import_github',
+            'importers'
+          ],
+          [
+            %r(#{RESERVED_WORDS_PREFIX}/import/gitea/realtime_changes\.json\z),
+            'realtime_changes_import_gitea',
+            'importers'
+          ],
+          [
+            %r(#{RESERVED_WORDS_PREFIX}/merge_requests/\d+/cached_widget\.json\z),
+            'merge_request_widget',
+            'code_review'
+          ]
+        ].map(&method(:build_route)).freeze
+
+        # Overridden in EE to add more routes
+        def self.all_routes
+          ROUTES
+        end
+
+        def self.match(request)
+          all_routes.find { |route| route.match(request.path_info) }
+        end
+
+        def self.cache_key(request)
+          request.path
+        end
+      end
+    end
+  end
+end
+
+Gitlab::EtagCaching::Router::Restful.prepend_if_ee('EE::Gitlab::EtagCaching::Router::Restful')

lib/gitlab/etag_caching/store.rb

@@ -3,6 +3,8 @@
 module Gitlab
   module EtagCaching
     class Store
+      InvalidKeyError = Class.new(StandardError)
+
       EXPIRY_TIME = 20.minutes
       SHARED_STATE_NAMESPACE = 'etag:'
 
@@ -27,9 +29,28 @@ module Gitlab
       end
 
       def redis_shared_state_key(key)
-        raise 'Invalid key' if !Rails.env.production? && !Gitlab::EtagCaching::Router.match(key)
+        raise InvalidKeyError, "#{key} is invalid" unless valid_key?(key)
 
         "#{SHARED_STATE_NAMESPACE}#{key}"
+      rescue InvalidKeyError => e
+        Gitlab::ErrorTracking.track_and_raise_for_dev_exception(e)
+      end
+
+      def valid_key?(key)
+        return true if skip_validation?
+
+        path, header = key.split(':', 2)
+        env = {
+          'PATH_INFO' => path,
+          'HTTP_X_GITLAB_GRAPHQL_RESOURCE_ETAG' => header
+        }
+
+        fake_request = ActionDispatch::Request.new(env)
+        !!Gitlab::EtagCaching::Router.match(fake_request)
+      end
+
+      def skip_validation?
+        Rails.env.production?
       end
     end
   end

spec/lib/gitlab/etag_caching/router/graphql_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Gitlab::EtagCaching::Router::Graphql do
+  it 'matches pipelines endpoint' do
+    result = match_route('/api/graphql', 'pipelines/id/1')
+
+    expect(result).to be_present
+    expect(result.name).to eq 'pipelines_graph'
+  end
+
+  it 'has a valid feature category for every route', :aggregate_failures do
+    feature_categories = YAML.load_file(Rails.root.join('config', 'feature_categories.yml')).to_set
+
+    described_class::ROUTES.each do |route|
+      expect(feature_categories).to include(route.feature_category), "#{route.name} has a category of #{route.feature_category}, which is not valid"
+    end
+  end
+
+  def match_route(path, header)
+    described_class.match(
+      double(path_info: path,
+        headers: { 'X-GITLAB-GRAPHQL-RESOURCE-ETAG' => header }))
+  end
+
+  describe '.cache_key' do
+    let(:path) { '/api/graphql' }
+    let(:header_value) { 'pipelines/id/1' }
+    let(:headers) do
+      { 'X-GITLAB-GRAPHQL-RESOURCE-ETAG' => header_value }.compact
+    end
+
+    subject do
+      described_class.cache_key(double(path: path, headers: headers))
+    end
+
+    it 'uses request path and headers as cache key' do
+      is_expected.to eq '/api/graphql:pipelines/id/1'
+    end
+
+    context 'when the header is missing' do
+      let(:header_value) {}
+
+      it 'does not raise errors' do
+        is_expected.to eq '/api/graphql'
+      end
+    end
+  end
+end

spec/lib/gitlab/etag_caching/router/restful_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Gitlab::EtagCaching::Router::Restful do
+  it 'matches issue notes endpoint' do
+    result = match_route('/my-group/and-subgroup/here-comes-the-project/noteable/issue/1/notes')
+
+    expect(result).to be_present
+    expect(result.name).to eq 'issue_notes'
+  end
+
+  it 'matches MR notes endpoint' do
+    result = match_route('/my-group/and-subgroup/here-comes-the-project/noteable/merge_request/1/notes')
+
+    expect(result).to be_present
+    expect(result.name).to eq 'merge_request_notes'
+  end
+
+  it 'matches issue title endpoint' do
+    result = match_route('/my-group/my-project/-/issues/123/realtime_changes')
+
+    expect(result).to be_present
+    expect(result.name).to eq 'issue_title'
+  end
+
+  it 'matches with a project name that includes a suffix of create' do
+    result = match_route('/group/test-create/-/issues/123/realtime_changes')
+
+    expect(result).to be_present
+    expect(result.name).to eq 'issue_title'
+  end
+
+  it 'matches with a project name that includes a prefix of create' do
+    result = match_route('/group/create-test/-/issues/123/realtime_changes')
+
+    expect(result).to be_present
+    expect(result.name).to eq 'issue_title'
+  end
+
+  it 'matches project pipelines endpoint' do
+    result = match_route('/my-group/my-project/-/pipelines.json')
+
+    expect(result).to be_present
+    expect(result.name).to eq 'project_pipelines'
+  end
+
+  it 'matches commit pipelines endpoint' do
+    result = match_route('/my-group/my-project/-/commit/aa8260d253a53f73f6c26c734c72fdd600f6e6d4/pipelines.json')
+
+    expect(result).to be_present
+    expect(result.name).to eq 'commit_pipelines'
+  end
+
+  it 'matches new merge request pipelines endpoint' do
+    result = match_route('/my-group/my-project/-/merge_requests/new.json')
+
+    expect(result).to be_present
+    expect(result.name).to eq 'new_merge_request_pipelines'
+  end
+
+  it 'matches merge request pipelines endpoint' do
+    result = match_route('/my-group/my-project/-/merge_requests/234/pipelines.json')
+
+    expect(result).to be_present
+    expect(result.name).to eq 'merge_request_pipelines'
+  end
+
+  it 'matches build endpoint' do
+    result = match_route('/my-group/my-project/builds/234.json')
+
+    expect(result).to be_present
+    expect(result.name).to eq 'project_build'
+  end
+
+  it 'does not match blob with confusing name' do
+    result = match_route('/my-group/my-project/-/blob/master/pipelines.json')
+
+    expect(result).to be_blank
+  end
+
+  it 'matches the cluster environments path' do
+    result = match_route('/my-group/my-project/-/clusters/47/environments')
+
+    expect(result).to be_present
+    expect(result.name).to eq 'cluster_environments'
+  end
+
+  it 'matches the environments path' do
+    result = match_route('/my-group/my-project/environments.json')
+
+    expect(result).to be_present
+    expect(result.name).to eq 'environments'
+  end
+
+  it 'matches pipeline#show endpoint' do
+    result = match_route('/my-group/my-project/-/pipelines/2.json')
+
+    expect(result).to be_present
+    expect(result.name).to eq 'project_pipeline'
+  end
+
+  it 'has a valid feature category for every route', :aggregate_failures do
+    feature_categories = YAML.load_file(Rails.root.join('config', 'feature_categories.yml')).to_set
+
+    described_class::ROUTES.each do |route|
+      expect(feature_categories).to include(route.feature_category), "#{route.name} has a category of #{route.feature_category}, which is not valid"
+    end
+  end
+
+  def match_route(path)
+    described_class.match(double(path_info: path))
+  end
+
+  describe '.cache_key' do
+    subject do
+      described_class.cache_key(double(path: '/my-group/my-project/builds/234.json'))
+    end
+
+    it 'uses request path as cache key' do
+      is_expected.to eq '/my-group/my-project/builds/234.json'
+    end
+  end
+end

spec/lib/gitlab/etag_caching/router_spec.rb

@@ -3,136 +3,33 @@
 require 'spec_helper'
 
 RSpec.describe Gitlab::EtagCaching::Router do
-  it 'matches issue notes endpoint' do
-    result = described_class.match(
-      '/my-group/and-subgroup/here-comes-the-project/noteable/issue/1/notes'
-    )
-
-    expect(result).to be_present
-    expect(result.name).to eq 'issue_notes'
-  end
-
-  it 'matches MR notes endpoint' do
-    result = described_class.match(
-      '/my-group/and-subgroup/here-comes-the-project/noteable/merge_request/1/notes'
-    )
-
-    expect(result).to be_present
-    expect(result.name).to eq 'merge_request_notes'
-  end
-
-  it 'matches issue title endpoint' do
-    result = described_class.match(
-      '/my-group/my-project/-/issues/123/realtime_changes'
-    )
-
-    expect(result).to be_present
-    expect(result.name).to eq 'issue_title'
-  end
-
-  it 'matches with a project name that includes a suffix of create' do
-    result = described_class.match(
-      '/group/test-create/-/issues/123/realtime_changes'
-    )
-
-    expect(result).to be_present
-    expect(result.name).to eq 'issue_title'
-  end
-
-  it 'matches with a project name that includes a prefix of create' do
-    result = described_class.match(
-      '/group/create-test/-/issues/123/realtime_changes'
-    )
-
-    expect(result).to be_present
-    expect(result.name).to eq 'issue_title'
-  end
-
-  it 'matches project pipelines endpoint' do
-    result = described_class.match(
-      '/my-group/my-project/-/pipelines.json'
-    )
-
-    expect(result).to be_present
-    expect(result.name).to eq 'project_pipelines'
-  end
-
-  it 'matches commit pipelines endpoint' do
-    result = described_class.match(
-      '/my-group/my-project/-/commit/aa8260d253a53f73f6c26c734c72fdd600f6e6d4/pipelines.json'
-    )
-
-    expect(result).to be_present
-    expect(result.name).to eq 'commit_pipelines'
-  end
-
-  it 'matches new merge request pipelines endpoint' do
-    result = described_class.match(
-      '/my-group/my-project/-/merge_requests/new.json'
-    )
-
-    expect(result).to be_present
-    expect(result.name).to eq 'new_merge_request_pipelines'
-  end
-
-  it 'matches merge request pipelines endpoint' do
-    result = described_class.match(
-      '/my-group/my-project/-/merge_requests/234/pipelines.json'
-    )
-
-    expect(result).to be_present
-    expect(result.name).to eq 'merge_request_pipelines'
-  end
-
-  it 'matches build endpoint' do
-    result = described_class.match(
-      '/my-group/my-project/builds/234.json'
-    )
-
-    expect(result).to be_present
-    expect(result.name).to eq 'project_build'
-  end
-
-  it 'does not match blob with confusing name' do
-    result = described_class.match(
-      '/my-group/my-project/-/blob/master/pipelines.json'
-    )
-
-    expect(result).to be_blank
-  end
+  describe '.match', :aggregate_failures do
+    context 'with RESTful routes' do
+      it 'matches project pipelines endpoint' do
+        result = match_route('/my-group/my-project/-/pipelines.json')
+
+        expect(result).to be_present
+        expect(result.name).to eq 'project_pipelines'
+        expect(result.router).to eq Gitlab::EtagCaching::Router::Restful
+      end
+    end
 
-  it 'matches the cluster environments path' do
-    result = described_class.match(
-      '/my-group/my-project/-/clusters/47/environments'
-    )
+    context 'with GraphQL routes' do
+      it 'matches pipelines endpoint' do
+        result = match_route('/api/graphql', 'pipelines/id/12')
 
-    expect(result).to be_present
-    expect(result.name).to eq 'cluster_environments'
+        expect(result).to be_present
+        expect(result.name).to eq 'pipelines_graph'
+        expect(result.router).to eq Gitlab::EtagCaching::Router::Graphql
+      end
+    end
   end
 
-  it 'matches the environments path' do
-    result = described_class.match(
-      '/my-group/my-project/environments.json'
-    )
+  def match_route(path, header = nil)
+    headers = { 'X-GITLAB-GRAPHQL-RESOURCE-ETAG' => header }.compact
 
-    expect(result).to be_present
-    expect(result.name).to eq 'environments'
-  end
-
-  it 'matches pipeline#show endpoint' do
-    result = described_class.match(
-      '/my-group/my-project/-/pipelines/2.json'
+    described_class.match(
+      double(path_info: path, headers: headers)
     )
-
-    expect(result).to be_present
-    expect(result.name).to eq 'project_pipeline'
-  end
-
-  it 'has a valid feature category for every route', :aggregate_failures do
-    feature_categories = YAML.load_file(Rails.root.join('config', 'feature_categories.yml')).to_set
-
-    described_class::ROUTES.each do |route|
-      expect(feature_categories).to include(route.feature_category), "#{route.name} has a category of #{route.feature_category}, which is not valid"
-    end
   end
 end

spec/lib/gitlab/etag_caching/store_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Gitlab::EtagCaching::Store, :clean_gitlab_redis_shared_state do
+  let(:store) { described_class.new }
+
+  describe '#get' do
+    subject { store.get(key) }
+
+    context 'with invalid keys' do
+      let(:key) { 'a' }
+
+      it 'raises errors' do
+        expect(Gitlab::ErrorTracking).to receive(:track_and_raise_for_dev_exception).and_call_original
+
+        expect { subject }.to raise_error Gitlab::EtagCaching::Store::InvalidKeyError
+      end
+
+      it 'does not raise errors in production' do
+        expect(store).to receive(:skip_validation?).and_return true
+        expect(Gitlab::ErrorTracking).not_to receive(:track_and_raise_for_dev_exception)
+
+        subject
+      end
+    end
+
+    context 'with GraphQL keys' do
+      let(:key) { '/api/graphql:pipelines/id/5' }
+
+      it 'returns a stored value' do
+        etag = store.touch(key)
+
+        is_expected.to eq(etag)
+      end
+    end
+
+    context 'with RESTful keys' do
+      let(:key) { '/my-group/my-project/builds/234.json' }
+
+      it 'returns a stored value' do
+        etag = store.touch(key)
+
+        is_expected.to eq(etag)
+      end
+    end
+  end
+
+  describe '#touch' do
+    subject { store.touch(key) }
+
+    context 'with invalid keys' do
+      let(:key) { 'a' }
+
+      it 'raises errors' do
+        expect(Gitlab::ErrorTracking).to receive(:track_and_raise_for_dev_exception).and_call_original
+
+        expect { subject }.to raise_error Gitlab::EtagCaching::Store::InvalidKeyError
+      end
+    end
+
+    context 'with GraphQL keys' do
+      let(:key) { '/api/graphql:pipelines/id/5' }
+
+      it 'stores and returns a value' do
+        etag = store.touch(key)
+
+        expect(etag).to be_present
+        expect(store.get(key)).to eq(etag)
+      end
+    end
+
+    context 'with RESTful keys' do
+      let(:key) { '/my-group/my-project/builds/234.json' }
+
+      it 'stores and returns a value' do
+        etag = store.touch(key)
+
+        expect(etag).to be_present
+        expect(store.get(key)).to eq(etag)
+      end
+    end
+  end
+end

spec/services/ci/expire_pipeline_cache_service_spec.rb

@@ -13,10 +13,14 @@ RSpec.describe Ci::ExpirePipelineCacheService do
       pipelines_path = "/#{project.full_path}/-/pipelines.json"
       new_mr_pipelines_path = "/#{project.full_path}/-/merge_requests/new.json"
       pipeline_path = "/#{project.full_path}/-/pipelines/#{pipeline.id}.json"
+      graphql_pipeline_path = "/api/graphql:pipelines/id/#{pipeline.id}"
 
-      expect_any_instance_of(Gitlab::EtagCaching::Store).to receive(:touch).with(pipelines_path)
-      expect_any_instance_of(Gitlab::EtagCaching::Store).to receive(:touch).with(new_mr_pipelines_path)
-      expect_any_instance_of(Gitlab::EtagCaching::Store).to receive(:touch).with(pipeline_path)
+      expect_next_instance_of(Gitlab::EtagCaching::Store) do |store|
+        expect(store).to receive(:touch).with(pipelines_path)
+        expect(store).to receive(:touch).with(new_mr_pipelines_path)
+        expect(store).to receive(:touch).with(pipeline_path)
+        expect(store).to receive(:touch).with(graphql_pipeline_path)
+      end
 
       subject.execute(pipeline)
     end