Merge branch '27376-cache-default-branch-pipeline-on-project' into 'master'

Speed up several project lists

See merge request !9903

6 files changed, 268 insertions, 2 deletions

diff --git a/spec/features/dashboard/projects_spec.rb b/spec/features/dashboard/projects_spec.rb
index 63eb5c697c2..c4e58d14f75 100644
--- a/spec/features/dashboard/projects_spec.rb
+++ b/spec/features/dashboard/projects_spec.rb
@@ -1,10 +1,32 @@
 require 'spec_helper'
 
 RSpec.describe 'Dashboard Projects', feature: true do
+  let(:user) { create(:user) }
+  let(:project) { create(:project, name: "awesome stuff") }
+
   before do
-    login_as(create :user)
+    project.team << [user, :developer]
+    login_as user
     visit dashboard_projects_path
   end
-  
+
+  it 'shows the project the user in a member of in the list' do
+    visit dashboard_projects_path
+    expect(page).to have_content('awesome stuff')
+  end
+
+  describe "with a pipeline" do
+    let(:pipeline) {  create(:ci_pipeline, :success, project: project, sha: project.commit.sha) }
+
+    before do
+      pipeline
+    end
+
+    it 'shows that the last pipeline passed' do
+      visit dashboard_projects_path
+      expect(page).to have_xpath("//a[@href='#{pipelines_namespace_project_commit_path(project.namespace, project, project.commit)}']")
+    end
+  end
+
   it_behaves_like "an autodiscoverable RSS feed with current_user's private token"
 end
diff --git a/spec/helpers/ci_status_helper_spec.rb b/spec/helpers/ci_status_helper_spec.rb
index 637b02d9388..174cc84a97b 100644
--- a/spec/helpers/ci_status_helper_spec.rb
+++ b/spec/helpers/ci_status_helper_spec.rb
@@ -16,4 +16,11 @@ describe CiStatusHelper do
       helper.ci_icon_for_status(failed_commit.status)
     end
   end
+
+  describe "#pipeline_status_cache_key" do
+    it "builds a cache key for pipeline status" do
+      pipeline_status = Ci::PipelineStatus.new(build(:project), sha: "123abc", status: "success")
+      expect(helper.pipeline_status_cache_key(pipeline_status)).to eq("pipeline-status/123abc-success")
+    end
+  end
 end
diff --git a/spec/helpers/projects_helper_spec.rb b/spec/helpers/projects_helper_spec.rb
index aca0bb1d794..fc6ad6419ac 100644
--- a/spec/helpers/projects_helper_spec.rb
+++ b/spec/helpers/projects_helper_spec.rb
@@ -63,6 +63,46 @@ describe ProjectsHelper do
     end
   end
 
+  describe "#project_list_cache_key" do
+    let(:project) { create(:project) }
+
+    it "includes the namespace" do
+      expect(helper.project_list_cache_key(project)).to include(project.namespace.cache_key)
+    end
+
+    it "includes the project" do
+      expect(helper.project_list_cache_key(project)).to include(project.cache_key)
+    end
+
+    it "includes the controller name" do
+      expect(helper.controller).to receive(:controller_name).and_return("testcontroller")
+
+      expect(helper.project_list_cache_key(project)).to include("testcontroller")
+    end
+
+    it "includes the controller action" do
+      expect(helper.controller).to receive(:action_name).and_return("testaction")
+
+      expect(helper.project_list_cache_key(project)).to include("testaction")
+    end
+
+    it "includes the application settings" do
+      settings = Gitlab::CurrentSettings.current_application_settings
+
+      expect(helper.project_list_cache_key(project)).to include(settings.cache_key)
+    end
+
+    it "includes a version" do
+      expect(helper.project_list_cache_key(project)).to include("v2.3")
+    end
+
+    it "includes the pipeline status when there is a status" do
+      create(:ci_pipeline, :success, project: project, sha: project.commit.sha)
+
+      expect(helper.project_list_cache_key(project)).to include("pipeline-status/#{project.commit.sha}-success")
+    end
+  end
+
   describe 'link_to_member' do
     let(:group)   { create(:group) }
     let(:project) { create(:empty_project, group: group) }
diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb
index 9962c987110..4a664e4fae2 100644
--- a/spec/models/ci/pipeline_spec.rb
+++ b/spec/models/ci/pipeline_spec.rb
@@ -1018,6 +1018,19 @@ describe Ci::Pipeline, models: true do
     end
   end
 
+  describe '#update_status' do
+    let(:pipeline) { create(:ci_pipeline, sha: '123456') }
+
+    it 'updates the cached status' do
+      fake_status = double
+      # after updating the status, the status is set to `skipped` for this pipeline's builds
+      expect(Ci::PipelineStatus).to receive(:new).with(pipeline.project, sha: '123456', status: 'skipped').and_return(fake_status)
+      expect(fake_status).to receive(:store_in_cache_if_needed)
+
+      pipeline.update_status
+    end
+  end
+
   describe 'notifications when pipeline success or failed' do
     let(:project) { create(:project, :repository) }
 
diff --git a/spec/models/ci/pipeline_status_spec.rb b/spec/models/ci/pipeline_status_spec.rb
new file mode 100644
index 00000000000..bc5b71666c2
--- /dev/null
+++ b/spec/models/ci/pipeline_status_spec.rb
@@ -0,0 +1,173 @@
+require 'spec_helper'
+
+describe Ci::PipelineStatus do
+  let(:project) { create(:project) }
+  let(:pipeline_status) { described_class.new(project) }
+
+  describe '.load_for_project' do
+    it "loads the status" do
+      expect_any_instance_of(described_class).to receive(:load_status)
+
+      described_class.load_for_project(project)
+    end
+  end
+
+  describe '#has_status?' do
+    it "is false when the status wasn't loaded yet" do
+      expect(pipeline_status.has_status?).to be_falsy
+    end
+
+    it 'is true when all status information was loaded' do
+      fake_commit = double
+      allow(fake_commit).to receive(:status).and_return('failed')
+      allow(fake_commit).to receive(:sha).and_return('failed424d1b73bc0d3cb726eb7dc4ce17a4d48552f8c6')
+      allow(pipeline_status).to receive(:commit).and_return(fake_commit)
+      allow(pipeline_status).to receive(:has_cache?).and_return(false)
+
+      pipeline_status.load_status
+
+      expect(pipeline_status.has_status?).to be_truthy
+    end
+  end
+
+  describe '#load_status' do
+    it 'loads the status from the cache when there is one' do
+      expect(pipeline_status).to receive(:has_cache?).and_return(true)
+      expect(pipeline_status).to receive(:load_from_cache)
+
+      pipeline_status.load_status
+    end
+
+    it 'loads the status from the project commit when there is no cache' do
+      allow(pipeline_status).to receive(:has_cache?).and_return(false)
+
+      expect(pipeline_status).to receive(:load_from_commit)
+
+      pipeline_status.load_status
+    end
+
+    it 'stores the status in the cache when it loading it from the project' do
+      allow(pipeline_status).to receive(:has_cache?).and_return(false)
+      allow(pipeline_status).to receive(:load_from_commit)
+
+      expect(pipeline_status).to receive(:store_in_cache)
+
+      pipeline_status.load_status
+    end
+
+    it 'sets the state to loaded' do
+      pipeline_status.load_status
+
+      expect(pipeline_status).to be_loaded
+    end
+
+    it 'only loads the status once' do
+      expect(pipeline_status).to receive(:has_cache?).and_return(true).exactly(1)
+      expect(pipeline_status).to receive(:load_from_cache).exactly(1)
+
+      pipeline_status.load_status
+      pipeline_status.load_status
+    end
+  end
+
+  describe "#load_from_commit" do
+    let!(:pipeline) { create(:ci_pipeline, :success, project: project, sha: project.commit.sha) }
+
+    it 'reads the status from the pipeline for the commit' do
+      pipeline_status.load_from_commit
+
+      expect(pipeline_status.status).to eq('success')
+      expect(pipeline_status.sha).to eq(project.commit.sha)
+    end
+
+    it "doesn't fail for an empty project" do
+      status_for_empty_commit = described_class.new(create(:empty_project))
+
+      status_for_empty_commit.load_status
+
+      expect(status_for_empty_commit).to be_loaded
+    end
+  end
+
+  describe "#store_in_cache", :redis do
+    it "sets the object in redis" do
+      pipeline_status.sha = '123456'
+      pipeline_status.status = 'failed'
+
+      pipeline_status.store_in_cache
+      read_sha, read_status = Gitlab::Redis.with { |redis| redis.hmget("projects/#{project.id}/build_status", :sha, :status) }
+
+      expect(read_sha).to eq('123456')
+      expect(read_status).to eq('failed')
+    end
+  end
+
+  describe '#store_in_cache_if_needed', :redis do
+    it 'stores the state in the cache when the sha is the HEAD of the project' do
+      create(:ci_pipeline, :success, project: project, sha: project.commit.sha)
+      build_status = described_class.load_for_project(project)
+
+      build_status.store_in_cache_if_needed
+      sha, status = Gitlab::Redis.with { |redis| redis.hmget("projects/#{project.id}/build_status", :sha, :status) }
+
+      expect(sha).not_to be_nil
+      expect(status).not_to be_nil
+    end
+
+    it "doesn't store the status in redis when the sha is not the head of the project" do
+      other_status = described_class.new(project, sha: "123456", status: "failed")
+
+      other_status.store_in_cache_if_needed
+      sha, status = Gitlab::Redis.with { |redis| redis.hmget("projects/#{project.id}/build_status", :sha, :status) }
+
+      expect(sha).to be_nil
+      expect(status).to be_nil
+    end
+
+    it "deletes the cache if the repository doesn't have a head commit" do
+      empty_project = create(:empty_project)
+      Gitlab::Redis.with { |redis| redis.mapped_hmset("projects/#{empty_project.id}/build_status", { sha: "sha", status: "pending" }) }
+      other_status = described_class.new(empty_project, sha: "123456", status: "failed")
+
+      other_status.store_in_cache_if_needed
+      sha, status = Gitlab::Redis.with { |redis| redis.hmget("projects/#{empty_project.id}/build_status", :sha, :status) }
+
+      expect(sha).to be_nil
+      expect(status).to be_nil
+    end
+  end
+
+  describe "with a status in redis", :redis do
+    let(:status) { 'success' }
+    let(:sha) { '424d1b73bc0d3cb726eb7dc4ce17a4d48552f8c6' }
+
+    before do
+      Gitlab::Redis.with { |redis| redis.mapped_hmset("projects/#{project.id}/build_status", { sha: sha, status: status }) }
+    end
+
+    describe '#load_from_cache' do
+      it 'reads the status from redis' do
+        pipeline_status.load_from_cache
+
+        expect(pipeline_status.sha).to eq(sha)
+        expect(pipeline_status.status).to eq(status)
+      end
+    end
+
+    describe '#has_cache?' do
+      it 'knows the status is cached' do
+        expect(pipeline_status.has_cache?).to be_truthy
+      end
+    end
+
+    describe '#delete_from_cache' do
+      it 'deletes values from redis'  do
+        pipeline_status.delete_from_cache
+
+        key_exists = Gitlab::Redis.with { |redis| redis.exists("projects/#{project.id}/build_status") }
+
+        expect(key_exists).to be_falsy
+      end
+    end
+  end
+end
diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb
index e120e21af06..ff1defcd32d 100644
--- a/spec/models/project_spec.rb
+++ b/spec/models/project_spec.rb
@@ -1916,4 +1916,15 @@ describe Project, models: true do
       end
     end
   end
+
+  describe '#pipeline_status' do
+    let(:project) { create(:project) }
+    it 'builds a pipeline status' do
+      expect(project.pipeline_status).to be_a(Ci::PipelineStatus)
+    end
+
+    it 'hase a loaded pipeline status' do
+      expect(project.pipeline_status).to be_loaded
+    end
+  end
 end