Create static model Ci::Group

Now a new model is introduced, which is not persisted in the database,
Ci::Group. Given a pipeline now has many stages, which in turn have many
groups, we can clean up some code.

Other than that, some misc changes are included to clean up.

12 files changed, 118 insertions, 78 deletions

diff --git a/app/controllers/projects/pipelines_controller.rb b/app/controllers/projects/pipelines_controller.rb
index ea2915a4de6..7bed6707fe8 100644
--- a/app/controllers/projects/pipelines_controller.rb
+++ b/app/controllers/projects/pipelines_controller.rb
@@ -73,9 +73,9 @@ class Projects::PipelinesController < Projects::ApplicationController
       format.json do
         Gitlab::PollingInterval.set_header(response, interval: POLLING_INTERVAL)
 
-        render json: PipelineSerializer.
-          new(project: @project, user: @current_user).
-          represent(@pipeline)
+        render json: PipelineSerializer
+          .new(project: @project, user: @current_user)
+          .represent(@pipeline, with_jobs: true)
       end
     end
   end
diff --git a/app/models/ci/group.rb b/app/models/ci/group.rb
new file mode 100644
index 00000000000..d7a499f3368
--- /dev/null
+++ b/app/models/ci/group.rb
@@ -0,0 +1,26 @@
+module Ci
+  # Currently this model is not persisted in the database, but derived from a
+  # pipelines jobs. We might, but at the same time might not, persist this model
+  # in the database later
+  class Group
+    include StaticModel
+
+    attr_reader :stage, :name, :statuses
+
+    def initialize(stage, name:, statuses:)
+      @stage = stage
+      @name = name
+      @statuses = statuses
+    end
+
+    def detailed_status(current_user)
+      Gitlab::Ci::Status::Group::Factory
+        .new(CommitStatus.where(id: statuses.map(&:id)), current_user)
+        .fabricate!
+    end
+
+    def size
+      statuses.size
+    end
+  end
+end
diff --git a/app/models/ci/stage.rb b/app/models/ci/stage.rb
index e7d6b17d445..9f4f9f26b1c 100644
--- a/app/models/ci/stage.rb
+++ b/app/models/ci/stage.rb
@@ -15,6 +15,14 @@ module Ci
       @warnings = warnings
     end
 
+    def job_groups
+      @job_groups ||=
+        statuses.sort_by(&:sortable_name).group_by(&:group_name)
+        .map do |group_name, grouped_statuses|
+          Ci::Group.new(self, name: group_name, statuses: grouped_statuses)
+        end
+    end
+
     def to_param
       name
     end
diff --git a/app/serializers/base_serializer.rb b/app/serializers/base_serializer.rb
index 37e5a7bf168..f262b7b7eb0 100644
--- a/app/serializers/base_serializer.rb
+++ b/app/serializers/base_serializer.rb
@@ -1,6 +1,4 @@
 class BaseSerializer  
-  attr_reader :request
-
   def initialize(parameters = {})
     @request = EntityRequest.new(parameters)
   end
diff --git a/app/serializers/job_group_entity.rb b/app/serializers/job_group_entity.rb
new file mode 100644
index 00000000000..e2e302d0bff
--- /dev/null
+++ b/app/serializers/job_group_entity.rb
@@ -0,0 +1,15 @@
+class JobGroupEntity < Grape::Entity
+  include RequestAwareEntity
+
+  expose :name
+  expose :size
+  expose :detailed_status, as: :status, with: StatusEntity
+
+  private
+
+  alias_method :group, :object
+
+  def detailed_status
+    group.detailed_status(request.user)
+  end
+end
diff --git a/app/serializers/jobs_serializer.rb b/app/serializers/jobs_serializer.rb
deleted file mode 100644
index 00a0672fda6..00000000000
--- a/app/serializers/jobs_serializer.rb
+++ /dev/null
@@ -1,53 +0,0 @@
-class JobsSerializer < BaseSerializer
-  Item = Struct.new(:name, :size, :list)
-
-  entity BuildEntity
-
-  def with_groups
-    tap { @groups = true }
-  end
-
-  def groups?
-    @groups
-  end
-
-  def represent(resource, opts = {})
-    if groups?
-      groups(resource).map do |item|
-        { name: item.name,
-          size: item.size,
-          list: super(item.list, opts),
-          status: represent_status(item.list) }
-      end
-    else
-      super(resource, opts)
-    end
-  end
-
-  private
-
-  def represent_status(list, opts = {})
-    # TODO: We don't really have a first class concept
-    # for JobsGroup that would make it possible to have status for that
-    detailed_status =
-      if group_jobs.one?
-        group_jobs.first.detailed_status(request.user)
-      else
-        Gitlab::Ci::Status::Group::Factory
-            .new(CommitStatus.where(id: group_jobs), request.user)
-            .fabricate!
-      end
-
-    StatusEntity
-      .represent(resource, opts.merge(request: @request))
-      .as_json
-  end
-
-  def groups(resource)
-    items = resource.sort_by(&:sortable_name).group_by(&:group_name)
-
-    items.map do |group_name, group_jobs|
-      Item.new(group_name, group_jobs.size, group_jobs)
-    end
-  end
-end
diff --git a/app/serializers/pipeline_entity.rb b/app/serializers/pipeline_entity.rb
index d221b09d9cb..9c91ab3e098 100644
--- a/app/serializers/pipeline_entity.rb
+++ b/app/serializers/pipeline_entity.rb
@@ -11,12 +11,11 @@ class PipelineEntity < Grape::Entity
       pipeline)
   end
 
-  expose :stages, with: StageEntity
-
   expose :details do
     expose :detailed_status, as: :status, with: StatusEntity
     expose :duration
     expose :finished_at
+    expose :stages, with: StageEntity
     expose :artifacts, using: BuildArtifactEntity
     expose :manual_actions, using: BuildActionEntity
   end
diff --git a/app/serializers/stage_entity.rb b/app/serializers/stage_entity.rb
index 35245ae978e..5b0877d53fa 100644
--- a/app/serializers/stage_entity.rb
+++ b/app/serializers/stage_entity.rb
@@ -26,10 +26,7 @@ class StageEntity < Grape::Entity
       format: :json)
   end
 
-  expose :jobs do |stage|
-    JobsSerializer.new(project: request.project, user: request.user)
-      .with_groups.represent(stage.statuses)
-  end
+  expose :job_groups, as: :groups, with: JobGroupEntity
 
   private
 
diff --git a/app/services/ci/expire_pipeline_cache_service.rb b/app/services/ci/expire_pipeline_cache_service.rb
index ad968085aa7..6a7ac6dab35 100644
--- a/app/services/ci/expire_pipeline_cache_service.rb
+++ b/app/services/ci/expire_pipeline_cache_service.rb
@@ -1,31 +1,35 @@
 module Ci
   class ExpirePipelineCacheService < BaseService
+    include Gitlab::Routing.url_helpers
+
     attr_reader :pipeline
 
     def execute(pipeline)
       @pipeline = pipeline
-      store = Gitlab::EtagCaching::Store.new
 
-      store.touch(project_pipeline_path)
-      store.touch(project_pipelines_path)
-      store.touch(commit_pipelines_path) if pipeline.commit
-      store.touch(new_merge_request_pipelines_path)
-      merge_requests_pipelines_paths.each { |path| store.touch(path) }
+      Gitlab::EtagCaching::Store.new.tap do |store|
+        store.touch(project_pipeline_path)
+        store.touch(project_pipelines_path)
+        store.touch(commit_pipelines_path) if pipeline.commit
+        store.touch(new_merge_request_pipelines_path)
+
+        merge_requests_pipelines_paths.each { |path| store.touch(path) }
+      end
 
-      Gitlab::Cache::Ci::ProjectPipelineStatus.update_for_pipeline(@pipeline)
+      Gitlab::Cache::Ci::ProjectPipelineStatus.update_for_pipeline(pipeline)
     end
 
     private
 
     def project_pipelines_path
-      Gitlab::Routing.url_helpers.namespace_project_pipelines_path(
+      namespace_project_pipelines_path(
         project.namespace,
         project,
         format: :json)
     end
 
     def commit_pipelines_path
-      Gitlab::Routing.url_helpers.pipelines_namespace_project_commit_path(
+      pipelines_namespace_project_commit_path(
         project.namespace,
         project,
         pipeline.commit.id,
@@ -33,7 +37,7 @@ module Ci
     end
 
     def new_merge_request_pipelines_path
-      Gitlab::Routing.url_helpers.new_namespace_project_merge_request_path(
+      new_namespace_project_merge_request_path(
         project.namespace,
         project,
         format: :json)
@@ -41,7 +45,7 @@ module Ci
 
     def merge_requests_pipelines_paths
       pipeline.merge_requests.collect do |merge_request|
-        Gitlab::Routing.url_helpers.pipelines_namespace_project_merge_request_path(
+        pipelines_namespace_project_merge_request_path(
           project.namespace,
           project,
           merge_request,
@@ -50,7 +54,7 @@ module Ci
     end
 
     def project_pipeline_path
-      Gitlab::Routing.url_helpers.namespace_project_pipeline_path(
+      namespace_project_pipeline_path(
         project.namespace,
         project,
         pipeline,
diff --git a/spec/controllers/projects/pipelines_controller_spec.rb b/spec/controllers/projects/pipelines_controller_spec.rb
index 670052f079a..f54665ebd19 100644
--- a/spec/controllers/projects/pipelines_controller_spec.rb
+++ b/spec/controllers/projects/pipelines_controller_spec.rb
@@ -26,6 +26,7 @@ describe Projects::PipelinesController do
 
     it 'returns JSON with serialized pipelines' do
       expect(response).to have_http_status(:ok)
+      expect(response).to match_response_schema('pipeline')
 
       expect(json_response).to include('pipelines')
       expect(json_response['pipelines'].count).to eq 4
@@ -33,7 +34,6 @@ describe Projects::PipelinesController do
       expect(json_response['count']['running']).to eq 1
       expect(json_response['count']['pending']).to eq 1
       expect(json_response['count']['finished']).to eq 1
-      expect(json_response).not_to have_key('stages')
     end
   end
 
@@ -46,7 +46,6 @@ describe Projects::PipelinesController do
       expect(response).to have_http_status(:ok)
       expect(json_response).not_to be_an(Array)
       expect(json_response['id']).to be(pipeline.id)
-      expect(json_response).to have_key('stages')
     end
   end
 
diff --git a/spec/models/ci/group_spec.rb b/spec/models/ci/group_spec.rb
new file mode 100644
index 00000000000..ffa3e97ec3e
--- /dev/null
+++ b/spec/models/ci/group_spec.rb
@@ -0,0 +1,19 @@
+require 'spec_helper'
+
+describe Ci::Group, models: true do
+  subject { described_class.new('test', name: 'rspec', statuses: []) }
+
+  describe 'expectations' do
+    it { is_expected.to include_module(StaticModel) }
+
+    it { is_expected.to respond_to(:stage) }
+    it { is_expected.to respond_to(:name) }
+    it { is_expected.to respond_to(:statuses) }
+  end
+
+  describe '#size' do
+    it 'returns the size of the statusses array' do
+      expect(subject.size).to eq(0)
+    end
+  end
+end
diff --git a/spec/services/ci/expire_pipeline_cache_service_spec.rb b/spec/services/ci/expire_pipeline_cache_service_spec.rb
new file mode 100644
index 00000000000..94f2352a7d6
--- /dev/null
+++ b/spec/services/ci/expire_pipeline_cache_service_spec.rb
@@ -0,0 +1,28 @@
+require 'spec_helper'
+
+describe Ci::ExpirePipelineCacheService, services: true do
+  let(:project) { pipeline.project }
+  let(:pipeline) { create(:ci_pipeline) }
+
+  before do
+    stub_ci_pipeline_to_return_yaml_file
+  end
+
+  subject { described_class.new(project, nil).execute(pipeline) }
+
+  describe '#execute' do
+    it 'creates a new Store' do
+      expect(Gitlab::EtagCaching::Store).to receive(:new)
+        .and_call_original
+
+      subject
+    end
+
+    it 'updates the ProjectPipelineStatus cache' do
+      expect(Gitlab::Cache::Ci::ProjectPipelineStatus)
+        .to receive(:update_for_pipeline).with(pipeline)
+
+      subject
+    end
+  end
+end