Remove GroupedStatuses

app/models/ci/group.rb

@@ -24,9 +24,11 @@ module Ci
     def status
       strong_memoize(:status) do
         if Feature.enabled?(:ci_composite_status, default_enabled: true)
-          Gitlab::Ci::Status::GroupedStatuses
-            .new(@jobs)
-            .one[:status]
+          all_statuses = @jobs.pluck(:status, :allow_failure)
+
+          Gitlab::Ci::Status::Composite
+            .new(all_statuses, status_key: 0, allow_failure_key: 1)
+            .status
         else
           CommitStatus.where(id: @jobs).legacy_status
         end

app/models/ci/pipeline.rb

@@ -406,16 +406,21 @@ module Ci
     end
 
     def legacy_stages_using_composite_status
-      stages = Gitlab::Ci::Status::GroupedStatuses
-        .new(statuses.latest, :stage, :stage_idx)
-        .group(:stage, :stage_idx)
-        .sort_by { |stage| stage[:stage_idx] }
+      stages = statuses.latest
+        .group_by(&:stage_idx)
+        .sort_by(&:first)
+
+      stages.map do |stage_idx, jobs|
+        stage_name = jobs.first.stage
+        jobs_statuses = jobs.pluck(:status, :allow_failure)
+
+        composite_status = Gitlab::Ci::Status::Composite
+          .new(jobs_statuses, status_key: 0, allow_failure_key: 1)
 
-      stages.map do |stage|
         Ci::LegacyStage.new(self,
-          name: stage[:stage],
-          status: stage[:status],
-          warnings: stage[:warnings])
+          name: stage_name,
+          status: composite_status.status,
+          warnings: composite_status.warnings?)
       end
     end
 
@@ -423,7 +428,7 @@ module Ci
       if Feature.enabled?(:ci_composite_status, default_enabled: true)
         legacy_stages_using_composite_status
       else
-        legacy_status_using_sql
+        legacy_stages_using_sql
       end
     end
 

app/models/concerns/has_status.rb

@@ -61,9 +61,14 @@ module HasStatus
 
     def slow_composite_status
       if Feature.enabled?(:ci_composite_status, default_enabled: true)
-        Gitlab::Ci::Status::GroupedStatuses
-          .new(all)
-          .one[:status]
+        columns = [:status]
+        columns << :allow_failure if column_names.include?('allow_failure')
+
+        all_statuses = all.pluck(*columns)
+
+        Gitlab::Ci::Status::Composite
+          .new(all_statuses, status_key: 0, allow_failure_key: 1)
+          .status
       else
         legacy_status
       end

lib/gitlab/ci/status/composite_status.rb → lib/gitlab/ci/status/composite.rb

@@ -3,13 +3,18 @@
 module Gitlab
   module Ci
     module Status
-      class CompositeStatus
+      class Composite
         attr_reader :warnings
 
-        def initialize(all_statuses)
+        # This class accepts an array of arrays or array of hashes
+        # The `status_key` and `allow_failure_key` define an index
+        # or key in each entry
+        def initialize(all_statuses, status_key:, allow_failure_key:)
           @count = 0
           @warnings = 0
           @status_set = Set.new
+          @status_key = status_key
+          @allow_failure_key = allow_failure_key
 
           build_status_set(all_statuses)
         end
@@ -19,29 +24,29 @@ module Gitlab
           when @count.zero?
             nil
           when none? || only_of?(:skipped)
-            warnings? ? :success : :skipped
+            warnings? ? 'success' : 'skipped'
           when only_of?(:success, :skipped)
-            :success
+            'success'
           when only_of?(:created)
-            :created
+            'created'
           when only_of?(:preparing)
-            :preparing
+            'preparing'
           when only_of?(:success, :skipped, :canceled)
-            :canceled
+            'canceled'
           when only_of?(:created, :skipped, :pending)
-            :pending
+            'pending'
           when any_of?(:running, :pending)
-            :running
+            'running'
           when any_of?(:manual)
-            :manual
+            'manual'
           when any_of?(:scheduled)
-            :scheduled
+            'scheduled'
           when any_of?(:preparing)
-            :preparing
+            'preparing'
           when any_of?(:created)
-            :running
+            'running'
           else
-            :failed
+            'failed'
           end
         end
 
@@ -67,19 +72,24 @@ module Gitlab
         def build_status_set(all_statuses)
           all_statuses.each do |status|
             @count += 1
-            if status[:allow_failure]
-              if HasStatus::PASSED_WITH_WARNINGS_STATUSES.include?(status[:status])
-                @warnings += 1
-              end
+            @warnings += 1 if count_as_warning?(status)
+            next if exclude_from_calculation?(status)
 
-              if HasStatus::EXCLUDE_IGNORED_STATUSES.include?(status[:status])
-                next
-              end
-            end
-
-            @status_set.add(status[:status].to_sym)
+            @status_set.add(status[@status_key].to_sym)
           end
         end
+
+        def count_as_warning?(status)
+          @allow_failure_key &&
+            status[@allow_failure_key] &&
+            HasStatus::PASSED_WITH_WARNINGS_STATUSES.include?(status[@status_key])
+        end
+        
+        def exclude_from_calculation?(status)
+          @allow_failure_key &&
+            status[@allow_failure_key] &&
+            HasStatus::EXCLUDE_IGNORED_STATUSES.include?(status[@status_key])
+        end
       end
     end
   end

lib/gitlab/ci/status/grouped_statuses.rb

-# frozen_string_literal: true
-
-module Gitlab
-  module Ci
-    module Status
-      class GroupedStatuses
-        def initialize(subject, *keys)
-          @subject = subject
-          @keys = keys
-        end
-
-        def one(**query)
-          validate_keys!(query.keys)
-
-          if item_hash = find_one(data_hash, query)
-            status_for_key(query, item_hash)
-          else
-            {}
-          end
-        end
-
-        def group(*keys)
-          validate_keys!(keys)
-
-          grouped_statuses(data_hash, keys)
-        end
-
-        private
-
-        def validate_keys!(keys)
-          missing_keys = @keys - keys
-          if missing_keys.present?
-            raise ArgumentError, "the keys '#{missing_keys.join(',')} are not subset of #{@keys.join(',')}"
-          end
-        end
-
-        def data_hash
-          @data_hash ||= hash_from_relation(@subject, @keys)
-        end
-
-        def hash_from_relation(subject, keys)
-          columns = keys.dup
-          columns << :status
-
-          # we request allow_failure when
-          # we don't have column_names, or such column does exist
-          columns << :allow_failure if !subject.respond_to?(:column_names) || subject.column_names.include?('allow_failure')
-
-          subject
-            .pluck(*columns) # rubocop: disable CodeReuse/ActiveRecord
-            .map { |attrs| columns.zip(attrs).to_h }
-        end
-
-        def find_one(subject, query)
-          subject.select do |attrs|
-            query.all? do |key, value|
-              attrs[key] == value
-            end
-          end.presence
-        end
-
-        def grouped_statuses(subject, keys)
-          subject
-            .group_by { |attrs| attrs.slice(*keys) }
-            .map { |key, all_attrs| status_for_key(key, all_attrs) }
-        end
-
-        def status_for_key(key, all_attrs)
-          composite_status = Gitlab::Ci::Status::CompositeStatus.new(all_attrs)
-
-          key.merge(
-            status: composite_status.status.to_s,
-            warnings: composite_status.warnings?
-          )
-        end
-      end
-    end
-  end
-end

spec/lib/gitlab/ci/status/composite_status_spec.rb → spec/lib/gitlab/ci/status/composite_spec.rb

 require 'spec_helper'
 
-describe Gitlab::Ci::Status::CompositeStatus do
+describe Gitlab::Ci::Status::Composite do
   set(:pipeline) { create(:ci_pipeline) }
 
-  let(:composite_status) { described_class.new(all_statuses) }
+  let(:composite_status) do
+    described_class.new(
+      all_statuses.pluck(:status, :allow_failure),
+      status_key: 0,
+      allow_failure_key: 0
+    )
+  end
 
   before(:all) do
     @statuses = HasStatus::STATUSES_ENUM.map do |status, idx|
@@ -16,11 +22,11 @@ describe Gitlab::Ci::Status::CompositeStatus do
   end
 
   describe '#status' do
-    subject { composite_status.status.to_s }
+    subject { composite_status.status }
 
     shared_examples 'compares composite with SQL status' do
       it 'returns exactly the same result' do
-        is_expected.to eq(Ci::Build.where(id: all_statuses).legacy_status.to_s)
+        is_expected.to eq(Ci::Build.where(id: all_statuses).legacy_status)
       end
     end
 

spec/models/ci/group_spec.rb

@@ -22,6 +22,32 @@ describe Ci::Group do
     end
   end
 
+  describe '#status' do
+    let(:jobs) do
+      [create(:ci_build, :failed)]
+    end
+
+    context 'when ci_composite_status is enabled' do
+      before do
+        stub_feature_flags(ci_composite_status: true)
+      end
+
+      it 'returns a failed status' do
+        expect(subject.status).to eq('failed')
+      end
+    end
+
+    context 'when ci_composite_status is disabled' do
+      before do
+        stub_feature_flags(ci_composite_status: false)
+      end
+
+      it 'returns a failed status' do
+        expect(subject.status).to eq('failed')
+      end
+    end
+  end
+
   describe '#detailed_status' do
     context 'when there is only one item in the group' do
       it 'calls the status from the object itself' do

spec/models/ci/pipeline_spec.rb

@@ -1136,59 +1136,71 @@ describe Ci::Pipeline, :mailer do
       end
 
       describe '#legacy_stages' do
+        using RSpec::Parameterized::TableSyntax
+
         subject { pipeline.legacy_stages }
 
-        context 'stages list' do
-          it 'returns ordered list of stages' do
-            expect(subject.map(&:name)).to eq(%w[build test deploy])
-          end
+        where(:ci_composite_status) do
+          [false, true]
         end
-
-        context 'stages with statuses' do
-          let(:statuses) do
-            subject.map { |stage| [stage.name, stage.status] }
+    
+        with_them do
+          before do
+            stub_feature_flags(ci_composite_status: ci_composite_status)
           end
-
-          it 'returns list of stages with correct statuses' do
-            expect(statuses).to eq([%w(build failed),
-                                    %w(test success),
-                                    %w(deploy running)])
+      
+          context 'stages list' do
+            it 'returns ordered list of stages' do
+              expect(subject.map(&:name)).to eq(%w[build test deploy])
+            end
           end
 
-          context 'when commit status is retried' do
-            before do
-              create(:commit_status, pipeline: pipeline,
-                                     stage: 'build',
-                                     name: 'mac',
-                                     stage_idx: 0,
-                                     status: 'success')
-
-              pipeline.process!
+          context 'stages with statuses' do
+            let(:statuses) do
+              subject.map { |stage| [stage.name, stage.status] }
             end
 
-            it 'ignores the previous state' do
-              expect(statuses).to eq([%w(build success),
+            it 'returns list of stages with correct statuses' do
+              expect(statuses).to eq([%w(build failed),
                                       %w(test success),
                                       %w(deploy running)])
             end
-          end
-        end
 
-        context 'when there is a stage with warnings' do
-          before do
-            create(:commit_status, pipeline: pipeline,
-                                   stage: 'deploy',
-                                   name: 'prod:2',
-                                   stage_idx: 2,
-                                   status: 'failed',
-                                   allow_failure: true)
+            context 'when commit status is retried' do
+              before do
+                create(:commit_status, pipeline: pipeline,
+                                      stage: 'build',
+                                      name: 'mac',
+                                      stage_idx: 0,
+                                      status: 'success')
+
+                pipeline.process!
+              end
+
+              it 'ignores the previous state' do
+                expect(statuses).to eq([%w(build success),
+                                        %w(test success),
+                                        %w(deploy running)])
+              end
+            end
           end
 
-          it 'populates stage with correct number of warnings' do
-            deploy_stage = pipeline.legacy_stages.third
+          context 'when there is a stage with warnings' do
+            before do
+              create(:commit_status, pipeline: pipeline,
+                                    stage: 'deploy',
+                                    name: 'prod:2',
+                                    stage_idx: 2,
+                                    status: 'failed',
+                                    allow_failure: true)
+            end
 
-            expect(deploy_stage).not_to receive(:statuses)
-            expect(deploy_stage).to have_warnings
+            it 'populates stage with correct number of warnings' do
+              deploy_stage = pipeline.legacy_stages.third
+
+              expect(deploy_stage).not_to receive(:statuses)
+              expect(deploy_stage).to have_warnings
+            end
           end
         end
       end

spec/models/concerns/has_status_spec.rb

@@ -3,8 +3,10 @@
 require 'spec_helper'
 
 describe HasStatus do
-  describe '.legacy_status' do
-    subject { CommitStatus.legacy_status }
+  describe '.slow_composite_status' do
+    using RSpec::Parameterized::TableSyntax
+
+    subject { CommitStatus.slow_composite_status }
 
     shared_examples 'build status summary' do
       context 'all successful' do
@@ -166,16 +168,26 @@ describe HasStatus do
       end
     end
 
-    context 'ci build statuses' do
-      let(:type) { :ci_build }
-
-      it_behaves_like 'build status summary'
+    where(:ci_composite_status) do
+      [false, true]
     end
 
-    context 'generic commit statuses' do
-      let(:type) { :generic_commit_status }
+    with_them do
+      before do
+        stub_feature_flags(ci_composite_status: ci_composite_status)
+      end
+
+      context 'ci build statuses' do
+        let(:type) { :ci_build }
 
-      it_behaves_like 'build status summary'
+        it_behaves_like 'build status summary'
+      end
+
+      context 'generic commit statuses' do
+        let(:type) { :generic_commit_status }
+
+        it_behaves_like 'build status summary'
+      end
     end
   end