Merge branch '238986-add-zip-typ-to-pages-api-using-artifacts-zip-archive' into 'master'

Save pages build artifact id in pages metadata

See merge request gitlab-org/gitlab!40592

app/models/project.rb

@@ -1828,12 +1828,12 @@ class Project < ApplicationRecord
   end
   # rubocop: enable CodeReuse/ServiceClass
 
-  def mark_pages_as_deployed
-    ensure_pages_metadatum.update!(deployed: true)
+  def mark_pages_as_deployed(artifacts_archive: nil)
+    ensure_pages_metadatum.update!(deployed: true, artifacts_archive: artifacts_archive)
   end
 
   def mark_pages_as_not_deployed
-    ensure_pages_metadatum.update!(deployed: false)
+    ensure_pages_metadatum.update!(deployed: false, artifacts_archive: nil)
   end
 
   def write_repository_config(gl_full_path: full_path)

app/models/project_pages_metadatum.rb

@@ -4,6 +4,7 @@ class ProjectPagesMetadatum < ApplicationRecord
   self.primary_key = :project_id
 
   belongs_to :project, inverse_of: :pages_metadatum
+  belongs_to :artifacts_archive, class_name: 'Ci::JobArtifact'
 
   scope :deployed, -> { where(deployed: true) }
 end

app/services/projects/update_pages_service.rb

@@ -52,7 +52,7 @@ module Projects
 
     def success
       @status.success
-      @project.mark_pages_as_deployed
+      @project.mark_pages_as_deployed(artifacts_archive: build.job_artifacts_archive)
       super
     end
 

changelogs/unreleased/238986-add-zip-typ-to-pages-api-using-artifacts-zip-archive.yml

+---
+title: Save pages build artifact id in pages metadata
+merge_request: 40592
+author:
+type: added

db/migrate/20200827102234_add_ci_job_artifact_id_to_pages_metadata.rb

+# frozen_string_literal: true
+
+class AddCiJobArtifactIdToPagesMetadata < ActiveRecord::Migration[6.0]
+  DOWNTIME = false
+
+  def change
+    add_column(:project_pages_metadata, :artifacts_archive_id, :bigint)
+  end
+end

db/migrate/20200827104432_add_foreign_key_to_artifacts_archive_id_in_pages_metadata.rb

+# frozen_string_literal: true
+
+class AddForeignKeyToArtifactsArchiveIdInPagesMetadata < ActiveRecord::Migration[6.0]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  INDEX_NAME = "index_project_pages_metadata_on_artifacts_archive_id"
+
+  disable_ddl_transaction!
+
+  def up
+    add_concurrent_index(:project_pages_metadata, :artifacts_archive_id, name: INDEX_NAME)
+    add_concurrent_foreign_key(:project_pages_metadata, :ci_job_artifacts, column: :artifacts_archive_id, on_delete: :nullify)
+  end
+
+  def down
+    remove_foreign_key_if_exists(:project_pages_metadata, :ci_job_artifacts, column: :artifacts_archive_id)
+    remove_concurrent_index_by_name(:project_pages_metadata, INDEX_NAME)
+  end
+end

db/schema_migrations/20200827102234

+d38ef8ccd627e70adf0dd9ac8161235e21afccbc59f1e8d95f379f66eb84630e
\ No newline at end of file

db/schema_migrations/20200827104432

+99d95dea0dbb10bcaca5515c144c7fcd1e365e69be5eded223379bf61df69bc3
\ No newline at end of file

db/structure.sql

@@ -14508,7 +14508,8 @@ ALTER SEQUENCE public.project_mirror_data_id_seq OWNED BY public.project_mirror_
 
 CREATE TABLE public.project_pages_metadata (
     project_id bigint NOT NULL,
-    deployed boolean DEFAULT false NOT NULL
+    deployed boolean DEFAULT false NOT NULL,
+    artifacts_archive_id bigint
 );
 
 CREATE TABLE public.project_repositories (
@@ -20464,6 +20465,8 @@ CREATE UNIQUE INDEX index_project_mirror_data_on_project_id ON public.project_mi
 
 CREATE INDEX index_project_mirror_data_on_status ON public.project_mirror_data USING btree (status);
 
+CREATE INDEX index_project_pages_metadata_on_artifacts_archive_id ON public.project_pages_metadata USING btree (artifacts_archive_id);
+
 CREATE UNIQUE INDEX index_project_pages_metadata_on_project_id ON public.project_pages_metadata USING btree (project_id);
 
 CREATE INDEX index_project_pages_metadata_on_project_id_and_deployed_is_true ON public.project_pages_metadata USING btree (project_id) WHERE (deployed = true);
@@ -21659,6 +21662,9 @@ ALTER TABLE ONLY public.merge_requests
 ALTER TABLE ONLY public.ci_builds
     ADD CONSTRAINT fk_6661f4f0e8 FOREIGN KEY (resource_group_id) REFERENCES public.ci_resource_groups(id) ON DELETE SET NULL;
 
+ALTER TABLE ONLY public.project_pages_metadata
+    ADD CONSTRAINT fk_69366a119e FOREIGN KEY (artifacts_archive_id) REFERENCES public.ci_job_artifacts(id) ON DELETE SET NULL;
+
 ALTER TABLE ONLY public.application_settings
     ADD CONSTRAINT fk_693b8795e4 FOREIGN KEY (push_rule_id) REFERENCES public.push_rules(id) ON DELETE SET NULL;
 

spec/models/project_spec.rb

@@ -5831,32 +5831,57 @@ RSpec.describe Project do
     end
   end
 
-  context 'pages deployed' do
+  describe '#mark_pages_as_deployed' do
     let(:project) { create(:project) }
+    let(:artifacts_archive) { create(:ci_job_artifact, project: project) }
 
-    {
-      mark_pages_as_deployed: true,
-      mark_pages_as_not_deployed: false
-    }.each do |method_name, flag|
-      describe method_name do
-        it "creates new record and sets deployed to #{flag} if none exists yet" do
-          project.pages_metadatum.destroy!
-          project.reload
+    it "works when artifacts_archive is missing" do
+      project.mark_pages_as_deployed
 
-          project.send(method_name)
+      expect(project.pages_metadatum.reload.deployed).to eq(true)
+    end
 
-          expect(project.pages_metadatum.reload.deployed).to eq(flag)
-        end
+    it "creates new record and sets deployed to true if none exists yet" do
+      project.pages_metadatum.destroy!
+      project.reload
 
-        it "updates the existing record and sets deployed to #{flag}" do
-          pages_metadatum = project.pages_metadatum
-          pages_metadatum.update!(deployed: !flag)
+      project.mark_pages_as_deployed(artifacts_archive: artifacts_archive)
 
-          expect { project.send(method_name) }.to change {
-            pages_metadatum.reload.deployed
-          }.from(!flag).to(flag)
-        end
-      end
+      expect(project.pages_metadatum.reload.deployed).to eq(true)
+    end
+
+    it "updates the existing record and sets deployed to true and records artifact archive" do
+      pages_metadatum = project.pages_metadatum
+      pages_metadatum.update!(deployed: false)
+
+      expect do
+        project.mark_pages_as_deployed(artifacts_archive: artifacts_archive)
+      end.to change { pages_metadatum.reload.deployed }.from(false).to(true)
+               .and change { pages_metadatum.reload.artifacts_archive }.from(nil).to(artifacts_archive)
+    end
+  end
+
+  describe '#mark_pages_as_not_deployed' do
+    let(:project) { create(:project) }
+    let(:artifacts_archive) { create(:ci_job_artifact, project: project) }
+
+    it "creates new record and sets deployed to false if none exists yet" do
+      project.pages_metadatum.destroy!
+      project.reload
+
+      project.mark_pages_as_not_deployed
+
+      expect(project.pages_metadatum.reload.deployed).to eq(false)
+    end
+
+    it "updates the existing record and sets deployed to false and clears artifacts_archive" do
+      pages_metadatum = project.pages_metadatum
+      pages_metadatum.update!(deployed: true, artifacts_archive: artifacts_archive)
+
+      expect do
+        project.mark_pages_as_not_deployed
+      end.to change { pages_metadatum.reload.deployed }.from(true).to(false)
+               .and change { pages_metadatum.reload.artifacts_archive }.from(artifacts_archive).to(nil)
     end
   end
 

spec/services/projects/update_pages_service_spec.rb

@@ -29,8 +29,9 @@ RSpec.describe Projects::UpdatePagesService do
 
   context 'for new artifacts' do
     context "for a valid job" do
+      let!(:artifacts_archive) { create(:ci_job_artifact, file: file, job: build) }
+
       before do
-        create(:ci_job_artifact, file: file, job: build)
         create(:ci_job_artifact, file_type: :metadata, file_format: :gzip, file: metadata, job: build)
 
         build.reload
@@ -49,6 +50,7 @@ RSpec.describe Projects::UpdatePagesService do
         expect(project.pages_deployed?).to be_falsey
         expect(execute).to eq(:success)
         expect(project.pages_metadatum).to be_deployed
+        expect(project.pages_metadatum.artifacts_archive).to eq(artifacts_archive)
         expect(project.pages_deployed?).to be_truthy
 
         # Check that all expected files are extracted