Merge branch 'kassio/cache-pages-internal-api' into 'master'

Add cache to gitlab/internal/pages endpoint

See merge request !88956

app/models/namespace.rb

@@ -450,9 +450,14 @@ def all_container_repositories
   end
 
   def pages_virtual_domain
+    cache = if Feature.enabled?(:cache_pages_domain_api, root_ancestor)
+              ::Gitlab::Pages::CacheControl.for_namespace(root_ancestor.id)
+            end
+
     Pages::VirtualDomain.new(
-      all_projects_with_pages.includes(:route, :project_feature, pages_metadatum: :pages_deployment),
-      trim_prefix: full_path
+      projects: all_projects_with_pages.includes(:route, :project_feature, pages_metadatum: :pages_deployment),
+      trim_prefix: full_path,
+      cache: cache
     )
   end
 

app/models/pages/virtual_domain.rb

@@ -2,8 +2,9 @@
 
 module Pages
   class VirtualDomain
-    def initialize(projects, trim_prefix: nil, domain: nil)
+    def initialize(projects:, cache: nil, trim_prefix: nil, domain: nil)
       @projects = projects
+      @cache = cache
       @trim_prefix = trim_prefix
       @domain = domain
     end
@@ -27,8 +28,12 @@ def lookup_paths
       paths.sort_by(&:prefix).reverse
     end
 
+    def cache_key
+      @cache_key ||= cache&.cache_key
+    end
+
     private
 
-    attr_reader :projects, :trim_prefix, :domain
+    attr_reader :projects, :trim_prefix, :domain, :cache
   end
 end

app/models/pages_domain.rb

@@ -209,7 +209,15 @@ def gitlab_provided_key=(key)
   def pages_virtual_domain
     return unless pages_deployed?
 
-    Pages::VirtualDomain.new([project], domain: self)
+    cache = if Feature.enabled?(:cache_pages_domain_api, project.root_namespace)
+              ::Gitlab::Pages::CacheControl.for_project(project.id)
+            end
+
+    Pages::VirtualDomain.new(
+      projects: [project],
+      domain: self,
+      cache: cache
+    )
   end
 
   def clear_auto_ssl_failure

app/workers/all_queues.yml

@@ -2704,6 +2704,15 @@
   :weight: 1
   :idempotent: false
   :tags: []
+- :name: pages_invalidate_domain_cache
+  :worker_name: Pages::InvalidateDomainCacheWorker
+  :feature_category: :pages
+  :has_external_dependencies: false
+  :urgency: :low
+  :resource_boundary: :unknown
+  :weight: 1
+  :idempotent: true
+  :tags: []
 - :name: pages_transfer
   :worker_name: PagesTransferWorker
   :feature_category: :pages

app/workers/pages/invalidate_domain_cache_worker.rb

+# frozen_string_literal: true
+
+module Pages
+  class InvalidateDomainCacheWorker
+    include Gitlab::EventStore::Subscriber
+
+    idempotent!
+
+    feature_category :pages
+
+    def handle_event(event)
+      if event.data[:project_id]
+        ::Gitlab::Pages::CacheControl
+          .for_project(event.data[:project_id])
+          .clear_cache
+      end
+
+      if event.data[:root_namespace_id]
+        ::Gitlab::Pages::CacheControl
+          .for_namespace(event.data[:root_namespace_id])
+          .clear_cache
+      end
+    end
+  end
+end

config/feature_flags/development/cache_pages_domain_api.yml

+---
+name: cache_pages_domain_api
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/88956
+rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/364127
+milestone: '15.2'
+type: development
+group: group::editor
+default_enabled: false

config/sidekiq_queues.yml

@@ -327,6 +327,8 @@
   - 1
 - - pages_domain_verification
   - 1
+- - pages_invalidate_domain_cache
+  - 1
 - - pages_transfer
   - 1
 - - personal_access_tokens

lib/api/internal/pages.rb

@@ -54,7 +54,15 @@ def authenticate_gitlab_pages_request!
             virtual_domain = host.pages_virtual_domain
             no_content! unless virtual_domain
 
-            present virtual_domain, with: Entities::Internal::Pages::VirtualDomain
+            if virtual_domain.cache_key.present?
+              # Cache context is not added to make it easier to expire the cache with
+              # Gitlab::Pages::CacheControl
+              present_cached virtual_domain,
+                cache_context: nil,
+                with: Entities::Internal::Pages::VirtualDomain
+            else
+              present virtual_domain, with: Entities::Internal::Pages::VirtualDomain
+            end
           end
         end
       end

lib/gitlab/event_store.rb

@@ -35,6 +35,9 @@ def self.configure!(store)
 
       store.subscribe ::MergeRequests::UpdateHeadPipelineWorker, to: ::Ci::PipelineCreatedEvent
       store.subscribe ::Namespaces::UpdateRootStatisticsWorker, to: ::Projects::ProjectDeletedEvent
+
+      store.subscribe ::Pages::InvalidateDomainCacheWorker, to: ::Pages::PageDeployedEvent
+      store.subscribe ::Pages::InvalidateDomainCacheWorker, to: ::Pages::PageDeletedEvent
     end
     private_class_method :configure!
   end

lib/gitlab/pages/cache_control.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module Pages
+    class CacheControl
+      CACHE_KEY_FORMAT = 'pages_domain_for_%{type}_%{id}'
+
+      attr_reader :cache_key
+
+      class << self
+        def for_project(project_id)
+          new(type: :project, id: project_id)
+        end
+
+        def for_namespace(namespace_id)
+          new(type: :namespace, id: namespace_id)
+        end
+      end
+
+      def initialize(type:, id:)
+        raise(ArgumentError, "type must be :namespace or :project") unless %i[namespace project].include?(type)
+
+        @cache_key = CACHE_KEY_FORMAT % { type: type, id: id }
+      end
+
+      def clear_cache
+        Rails.cache.delete(cache_key)
+      end
+    end
+  end
+end

spec/lib/gitlab/pages/cache_control_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Gitlab::Pages::CacheControl do
+  it 'fails with invalid type' do
+    expect { described_class.new(type: :unknown, id: nil) }
+      .to raise_error(ArgumentError, "type must be :namespace or :project")
+  end
+
+  describe '.for_namespace' do
+    let(:subject) { described_class.for_namespace(1) }
+
+    it { expect(subject.cache_key).to eq('pages_domain_for_namespace_1') }
+
+    describe '#clear_cache' do
+      it 'clears the cache' do
+        expect(Rails.cache)
+          .to receive(:delete)
+          .with('pages_domain_for_namespace_1')
+
+        subject.clear_cache
+      end
+    end
+  end
+
+  describe '.for_project' do
+    let(:subject) { described_class.for_project(1) }
+
+    it { expect(subject.cache_key).to eq('pages_domain_for_project_1') }
+
+    describe '#clear_cache' do
+      it 'clears the cache' do
+        expect(Rails.cache)
+          .to receive(:delete)
+          .with('pages_domain_for_project_1')
+
+        subject.clear_cache
+      end
+    end
+  end
+end

spec/models/namespace_spec.rb

@@ -1885,15 +1885,29 @@ def project_rugged(project)
 
   describe '#pages_virtual_domain' do
     let(:project) { create(:project, namespace: namespace) }
+    let(:virtual_domain) { namespace.pages_virtual_domain }
 
-    it 'returns the virual domain' do
+    before do
       project.mark_pages_as_deployed
       project.update_pages_deployment!(create(:pages_deployment, project: project))
+    end
 
-      virtual_domain = namespace.pages_virtual_domain
-
+    it 'returns the virual domain' do
       expect(virtual_domain).to be_an_instance_of(Pages::VirtualDomain)
       expect(virtual_domain.lookup_paths).not_to be_empty
+      expect(virtual_domain.cache_key).to eq("pages_domain_for_namespace_#{namespace.root_ancestor.id}")
+    end
+
+    context 'when :cache_pages_domain_api is disabled' do
+      before do
+        stub_feature_flags(cache_pages_domain_api: false)
+      end
+
+      it 'returns the virual domain' do
+        expect(virtual_domain).to be_an_instance_of(Pages::VirtualDomain)
+        expect(virtual_domain.lookup_paths).not_to be_empty
+        expect(virtual_domain.cache_key).to be_nil
+      end
     end
   end
 

spec/models/pages/virtual_domain_spec.rb

@@ -7,7 +7,7 @@
     let(:domain) { nil }
     let(:project) { instance_double(Project) }
 
-    subject(:virtual_domain) { described_class.new([project], domain: domain) }
+    subject(:virtual_domain) { described_class.new(projects: [project], domain: domain) }
 
     it 'returns nil if there is no domain provided' do
       expect(virtual_domain.certificate).to be_nil
@@ -35,7 +35,7 @@
     context 'when there is pages domain provided' do
       let(:domain) { instance_double(PagesDomain) }
 
-      subject(:virtual_domain) { described_class.new([project_a, project_b, project_c], domain: domain) }
+      subject(:virtual_domain) { described_class.new(projects: [project_a, project_b, project_c], domain: domain) }
 
       it 'returns collection of projects pages lookup paths sorted by prefix in reverse' do
         expect(project_a).to receive(:pages_lookup_path).with(domain: domain, trim_prefix: nil).and_return(pages_lookup_path_a)
@@ -47,7 +47,7 @@
     end
 
     context 'when there is trim_prefix provided' do
-      subject(:virtual_domain) { described_class.new([project_a, project_b], trim_prefix: 'group/') }
+      subject(:virtual_domain) { described_class.new(projects: [project_a, project_b], trim_prefix: 'group/') }
 
       it 'returns collection of projects pages lookup paths sorted by prefix in reverse' do
         expect(project_a).to receive(:pages_lookup_path).with(trim_prefix: 'group/', domain: nil).and_return(pages_lookup_path_a)
@@ -57,4 +57,19 @@
       end
     end
   end
+
+  describe '#cache_key' do
+    it 'returns the cache key based in the given cache_control' do
+      cache_control = instance_double(::Gitlab::Pages::CacheControl, cache_key: 'cache_key')
+      virtual_domain = described_class.new(projects: [instance_double(Project)], cache: cache_control)
+
+      expect(virtual_domain.cache_key).to eq('cache_key')
+    end
+
+    it 'returns nil when no cache_control is given' do
+      virtual_domain = described_class.new(projects: [instance_double(Project)])
+
+      expect(virtual_domain.cache_key).to be_nil
+    end
+  end
 end

spec/models/pages_domain_spec.rb

@@ -544,16 +544,31 @@
       end
     end
 
-    it 'returns the virual domain when there are pages deployed for the project' do
-      project.mark_pages_as_deployed
-      project.update_pages_deployment!(create(:pages_deployment, project: project))
+    context 'when there are pages deployed for the project' do
+      let(:virtual_domain) { pages_domain.pages_virtual_domain }
 
-      expect(Pages::VirtualDomain).to receive(:new).with([project], domain: pages_domain).and_call_original
+      before do
+        project.mark_pages_as_deployed
+        project.update_pages_deployment!(create(:pages_deployment, project: project))
+      end
+
+      it 'returns the virual domain when there are pages deployed for the project' do
+        expect(virtual_domain).to be_an_instance_of(Pages::VirtualDomain)
+        expect(virtual_domain.lookup_paths).not_to be_empty
+        expect(virtual_domain.cache_key).to eq("pages_domain_for_project_#{project.id}")
+      end
 
-      virtual_domain = pages_domain.pages_virtual_domain
+      context 'when :cache_pages_domain_api is disabled' do
+        before do
+          stub_feature_flags(cache_pages_domain_api: false)
+        end
 
-      expect(virtual_domain).to be_an_instance_of(Pages::VirtualDomain)
-      expect(virtual_domain.lookup_paths).not_to be_empty
+        it 'returns the virual domain when there are pages deployed for the project' do
+          expect(virtual_domain).to be_an_instance_of(Pages::VirtualDomain)
+          expect(virtual_domain.lookup_paths).not_to be_empty
+          expect(virtual_domain.cache_key).to be_nil
+        end
+      end
     end
   end
 

spec/workers/pages/invalidate_domain_cache_worker_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Pages::InvalidateDomainCacheWorker do
+  let(:event) do
+    Pages::PageDeployedEvent.new(data: {
+      project_id: 1,
+      namespace_id: 2,
+      root_namespace_id: 3
+    })
+  end
+
+  subject { consume_event(subscriber: described_class, event: event) }
+
+  it_behaves_like 'subscribes to event'
+
+  it 'enqueues ScheduleAggregationWorker' do
+    expect_next_instance_of(Gitlab::Pages::CacheControl, type: :project, id: 1) do |cache_control|
+      expect(cache_control).to receive(:clear_cache)
+    end
+
+    expect_next_instance_of(Gitlab::Pages::CacheControl, type: :namespace, id: 3) do |cache_control|
+      expect(cache_control).to receive(:clear_cache)
+    end
+
+    subject
+  end
+end