Skip to content

G
gitlab-ce
Commit a883852c authored by charlieablett's avatar charlieablett
Browse files
Options

Simplify auxiliary classes 

- Remove utility classes
- Store more state in EpicNode
parent 920c4e70
Show whitespace changes
Inline Side-by-side
Showing with 322 additions and 432 deletions
ee/app/graphql/types/epic_type.rb
View file @ a883852c
...@@ -2,6 +2,8 @@ ...@@ -2,6 +2,8 @@
module Types module Types
class EpicType < BaseObject class EpicType < BaseObject
include ::Gitlab::Graphql::Aggregations::Epics::Constants
graphql_name 'Epic' graphql_name 'Epic'
description 'Represents an epic.' description 'Represents an epic.'
...@@ -124,17 +126,17 @@ module Types ...@@ -124,17 +126,17 @@ module Types
description: 'Number of open and closed descendant epics and issues', description: 'Number of open and closed descendant epics and issues',
resolve: -> (epic, args, ctx) do resolve: -> (epic, args, ctx) do
if Feature.enabled?(:unfiltered_epic_aggregates) if Feature.enabled?(:unfiltered_epic_aggregates)
Epics::LazyEpicAggregate.new(ctx, epic.id, Epics::LazyEpicAggregate::COUNT) Gitlab::Graphql::Aggregations::Epics::LazyEpicAggregate.new(ctx, epic.id, COUNT)
else else
Epics::DescendantCountService.new(epic, ctx[:current_user]) Epics::DescendantCountService.new(epic, ctx[:current_user])
end end
end end
field :descendant_weight_sum, Types::EpicDescendantWeightSumType, null: true, complexity: 10, field :descendant_weight_sum, Types::EpicDescendantWeightSumType, null: true, complexity: 10,
description: "Total weight of open and closed descendant epic's issues", description: "Total weight of open and closed issues in the epic and its descendants",
feature_flag: :unfiltered_epic_aggregates, feature_flag: :unfiltered_epic_aggregates,
resolve: -> (epic, args, ctx) do resolve: -> (epic, args, ctx) do
Epics::LazyEpicAggregate.new(ctx, epic.id, Epics::LazyEpicAggregate::WEIGHT_SUM) Gitlab::Graphql::Aggregations::Epics::LazyEpicAggregate.new(ctx, epic.id, WEIGHT_SUM)
end end
field :health_status, field :health_status,
......
ee/lib/gitlab/graphql/aggregations/epics/aggregate.rb deleted 100644 → 0
View file @ 920c4e70
# frozen_string_literal: true
module Gitlab
module Graphql
module Aggregations
module Epics
class Aggregate
include Constants
def initialize(values)
raise NotImplementedError.new('Use either CountAggregate or WeightSumAggregate')
end
private
def sum_objects(state, type)
matching = @sums.select { |sum| sum.state == state && sum.type == type && sum.facet == facet}
return 0 if @sums.empty?
matching.map(&:value).reduce(:+) || 0
end
def facet
raise NotImplementedError.new('Use either CountAggregate or WeightSumAggregate')
end
end
end
end
end
end
ee/lib/gitlab/graphql/aggregations/epics/count_aggregate.rb deleted 100644 → 0
View file @ 920c4e70
# frozen_string_literal: true
module Gitlab
module Graphql
module Aggregations
module Epics
class CountAggregate < Aggregate
attr_accessor :opened_issues, :closed_issues, :opened_epics, :closed_epics
def initialize(sums)
@sums = sums
@opened_issues = sum_objects(OPENED_ISSUE_STATE, ISSUE_TYPE)
@closed_issues = sum_objects(CLOSED_ISSUE_STATE, ISSUE_TYPE)
@opened_epics = sum_objects(OPENED_EPIC_STATE, EPIC_TYPE)
@closed_epics = sum_objects(CLOSED_EPIC_STATE, EPIC_TYPE)
end
def facet
COUNT
end
end
end
end
end
end
ee/lib/gitlab/graphql/aggregations/epics/epic_node.rb
View file @ a883852c
# frozen_string_literal: true # frozen_string_literal: true
# This class represents an Epic's aggregate information (added up counts) about its child epics and direct issues # This class represents an Epic's aggregate information (added up counts) about its child epics and immediate issues
module Gitlab module Gitlab
module Graphql module Graphql
module Aggregations module Aggregations
module Epics module Epics
class EpicNode class EpicNode
include Constants include ::Gitlab::Graphql::Aggregations::Epics::Constants
include Gitlab::Utils::StrongMemoize
attr_reader :epic_id, :epic_state_id, :epic_info_flat_list, :parent_id, attr_reader :epic_id, :epic_state_id, :epic_info_flat_list, :parent_id,
:direct_sums, # we calculate these and recursively add them :immediate_count_totals, :immediate_weight_sum_totals, # only counts/weights of immediate issues and child epic counts
:sum_total :count_aggregate, :weight_sum_aggregate
attr_accessor :child_ids attr_accessor :child_ids, :calculated_count_totals, :calculated_weight_sum_totals
def initialize(epic_id, flat_info_list) def initialize(epic_id, flat_info_list)
# epic aggregate records from the DB loader look like the following: # epic aggregate records from the DB loader look like the following:
# { 1 => [{iid: 1, epic_state_id: 1, issues_count: 1, issues_weight_sum: 2, parent_id: nil, state_id: 2}] ... } # { 1 => [{iid: 1, epic_state_id: 1, issues_count: 1, issues_weight_sum: 2, parent_id: nil, state_id: 2}] ... }
# They include the sum of each epic's direct issues, grouped by status, # They include the sum of each epic's immediate issues, grouped by status,
# so in order to get a sum of the entire tree, we have to add that up recursively # so in order to get a sum of the entire tree, we have to add that up recursively
@epic_id = epic_id @epic_id = epic_id
@epic_info_flat_list = flat_info_list @epic_info_flat_list = flat_info_list
@child_ids = [] @child_ids = []
@direct_sums = [] @immediate_count_totals = []
@immediate_weight_sum_totals = []
set_epic_attributes(flat_info_list.first) # there will always be one set_epic_attributes(flat_info_list.first) # there will always be one
end end
def assemble_issue_sums def assemble_issue_totals
# this is a representation of the epic's # this is a representation of the epic's
# direct child issues and epics that have come from the DB # immediate child issues and epics that have come from the DB
[OPENED_ISSUE_STATE, CLOSED_ISSUE_STATE].each do |issue_state| [OPENED_ISSUE_STATE, CLOSED_ISSUE_STATE].each do |issue_state|
matching_issue_state_entry = epic_info_flat_list.find do |epic_info_node| matching_issue_state_entry = epic_info_flat_list.find do |epic_info_node|
epic_info_node[:issues_state_id] == issue_state epic_info_node[:issues_state_id] == issue_state
...@@ -41,43 +43,89 @@ module Gitlab ...@@ -41,43 +43,89 @@ module Gitlab
end end
end end
def assemble_epic_sums(children) def assemble_epic_totals(children)
[OPENED_EPIC_STATE, CLOSED_EPIC_STATE].each do |epic_state| [OPENED_EPIC_STATE, CLOSED_EPIC_STATE].each do |epic_state|
create_sum_if_needed(COUNT, epic_state, EPIC_TYPE, children.select { |node| node.epic_state_id == epic_state }.count) create_sum_if_needed(COUNT, epic_state, EPIC_TYPE, children.select { |node| node.epic_state_id == epic_state }.count)
end end
end end
def calculate_recursive_sums(tree) def aggregate_count(tree)
return sum_total if sum_total strong_memoize(:count_aggregate) do
calculate_recursive_sums(COUNT, tree)
OpenStruct.new({
opened_issues: sum_objects(COUNT, OPENED_ISSUE_STATE, ISSUE_TYPE),
closed_issues: sum_objects(COUNT, CLOSED_ISSUE_STATE, ISSUE_TYPE),
opened_epics: sum_objects(COUNT, OPENED_EPIC_STATE, EPIC_TYPE),
closed_epics: sum_objects(COUNT, CLOSED_EPIC_STATE, EPIC_TYPE)
})
end
end
@sum_total = SumTotal.new def aggregate_weight_sum(tree)
child_ids.each do |child_id| strong_memoize(:weight_sum_aggregate) do
child = tree[child_id] calculate_recursive_sums(WEIGHT_SUM, tree)
# get the child's totals, add to your own OpenStruct.new({
child_sums = child.calculate_recursive_sums(tree).sums opened_issues: sum_objects(WEIGHT_SUM, OPENED_ISSUE_STATE, ISSUE_TYPE),
sum_total.add(child_sums) closed_issues: sum_objects(WEIGHT_SUM, CLOSED_ISSUE_STATE, ISSUE_TYPE)
})
end
end end
sum_total.add(direct_sums)
sum_total def immediate_totals(facet)
strong_memoize(:"immediate_#{facet}_totals") do
[]
end
end
def calculated_totals(facet)
if facet == COUNT
return calculated_count_totals
end end
def aggregate_object_by(facet) calculated_weight_sum_totals
sum_total.by_facet(facet)
end end
def to_s def to_s
{ epic_id: @epic_id, parent_id: @parent_id, direct_sums: direct_sums, child_ids: child_ids }.to_json {
epic_id: @epic_id,
parent_id: @parent_id,
immediate_count_totals: immediate_count_totals,
immediate_weight_sum_totals: immediate_weight_sum_totals,
child_ids: child_ids
}.to_json
end end
alias_method :inspect, :to_s alias_method :inspect, :to_s
alias_method :id, :epic_id alias_method :id, :epic_id
def calculate_recursive_sums(facet, tree)
return calculated_totals(facet) if calculated_totals(facet)
sum_total = []
child_ids.each do |child_id|
child = tree[child_id]
# get the child's totals, add to your own
child_sums = child.calculate_recursive_sums(facet, tree)
sum_total.concat(child_sums)
end
sum_total.concat(immediate_totals(facet))
set_calculated_total(facet, sum_total)
end
private private
def sum_objects(facet, state, type)
sums = calculated_totals(facet) || []
matching = sums.select { |sum| sum.state == state && sum.type == type }
return 0 if sums.empty?
matching.map(&:value).reduce(:+) || 0
end
def create_sum_if_needed(facet, state, type, value) def create_sum_if_needed(facet, state, type, value)
return if value.nil? || value < 1 return if value.nil? || value < 1
direct_sums << Sum.new(facet, state, type, value) immediate_totals(facet) << Sum.new(facet, state, type, value)
end end
def set_epic_attributes(record) def set_epic_attributes(record)
...@@ -85,6 +133,14 @@ module Gitlab ...@@ -85,6 +133,14 @@ module Gitlab
@parent_id = record[:parent_id] @parent_id = record[:parent_id]
end end
def set_calculated_total(facet, calculated_sums)
if facet == COUNT
@calculated_count_totals = calculated_sums
else
@calculated_weight_sum_totals = calculated_sums
end
end
Sum = Struct.new(:facet, :state, :type, :value) do Sum = Struct.new(:facet, :state, :type, :value) do
def inspect def inspect
"<Sum facet=#{facet}, state=#{state}, type=#{type}, value=#{value}>" "<Sum facet=#{facet}, state=#{state}, type=#{type}, value=#{value}>"
......
ee/lib/gitlab/graphql/aggregations/epics/lazy_epic_aggregate.rb
View file @ a883852c
...@@ -5,7 +5,7 @@ module Gitlab ...@@ -5,7 +5,7 @@ module Gitlab
module Aggregations module Aggregations
module Epics module Epics
class LazyEpicAggregate class LazyEpicAggregate
include Constants include ::Gitlab::Graphql::Aggregations::Epics::Constants
attr_reader :facet, :epic_id, :lazy_state attr_reader :facet, :epic_id, :lazy_state
...@@ -38,7 +38,7 @@ module Gitlab ...@@ -38,7 +38,7 @@ module Gitlab
if loaded_epic_info_node if loaded_epic_info_node
# The pending IDs were already loaded, # The pending IDs were already loaded,
# so return the result of that previous load # so return the result of that previous load
loaded_epic_info_node.aggregate_object_by(@facet) aggregate_object(loaded_epic_info_node)
else else
load_records_into_tree load_records_into_tree
end end
...@@ -56,16 +56,16 @@ module Gitlab ...@@ -56,16 +56,16 @@ module Gitlab
pending_ids = @lazy_state[:pending_ids].to_a pending_ids = @lazy_state[:pending_ids].to_a
# Fire off the db query and get the results (grouped by epic_id and facet) # Fire off the db query and get the results (grouped by epic_id and facet)
raw_epic_aggregates = Epics::BulkEpicAggregateLoader.new(epic_ids: pending_ids).execute raw_epic_aggregates = Gitlab::Graphql::Loaders::BulkEpicAggregateLoader.new(epic_ids: pending_ids).execute
# Assemble the tree and sum everything # Assemble the tree and sum immediate child epic/issues
create_structure_from(raw_epic_aggregates) create_structure_from(raw_epic_aggregates)
@lazy_state[:pending_ids].clear @lazy_state[:pending_ids].clear
# Now, get the matching node and return its aggregate depending on the facet: # Now, get the matching node and return its aggregate depending on the facet:
epic_node = @lazy_state[:tree][@epic_id] epic_node = @lazy_state[:tree][@epic_id]
epic_node.aggregate_object_by(@facet) aggregate_object(epic_node)
end end
def create_structure_from(aggregate_records) def create_structure_from(aggregate_records)
...@@ -78,8 +78,7 @@ module Gitlab ...@@ -78,8 +78,7 @@ module Gitlab
end end
associate_parents_and_children associate_parents_and_children
assemble_direct_sums assemble_immediate_totals
calculate_recursive_sums
end end
# each of the methods below are done one after the other # each of the methods below are done one after the other
...@@ -89,18 +88,20 @@ module Gitlab ...@@ -89,18 +88,20 @@ module Gitlab
end end
end end
def assemble_direct_sums def assemble_immediate_totals
tree.each do |_, node| tree.each do |_, node|
node.assemble_issue_sums node.assemble_issue_totals
node_children = tree.select { |_, child_node| node.epic_id == child_node.parent_id }.values node_children = tree.select { |_, child_node| node.epic_id == child_node.parent_id }.values
node.assemble_epic_sums(node_children) node.assemble_epic_totals(node_children)
end end
end end
def calculate_recursive_sums def aggregate_object(node)
tree.each do |_, node| if @facet == COUNT
node.calculate_recursive_sums(tree) node.aggregate_count(tree)
else
node.aggregate_weight_sum(tree)
end end
end end
end end
......
ee/lib/gitlab/graphql/aggregations/epics/sum_total.rb deleted 100644 → 0
View file @ 920c4e70
# frozen_string_literal: true
module Gitlab
module Graphql
module Aggregations
module Epics
class SumTotal
include Constants
attr_accessor :sums
def initialize
@sums = []
end
def add(other_sums)
sums.concat(other_sums)
end
def by_facet(facet)
return CountAggregate.new(sums) if facet == COUNT
WeightSumAggregate.new(sums)
end
end
end
end
end
end
ee/lib/gitlab/graphql/aggregations/epics/weight_sum_aggregate.rb deleted 100644 → 0
View file @ 920c4e70
# frozen_string_literal: true
module Gitlab
module Graphql
module Aggregations
module Epics
class WeightSumAggregate < Aggregate
attr_accessor :opened_issues, :closed_issues
def initialize(sums)
@sums = sums
@opened_issues = sum_objects(OPENED_ISSUE_STATE, :issue)
@closed_issues = sum_objects(CLOSED_ISSUE_STATE, :issue)
end
def facet
WEIGHT_SUM
end
end
end
end
end
end
ee/lib/gitlab/graphql/loaders/bulk_epic_aggregate_loader.rb 0 → 100644
View file @ a883852c
# frozen_string_literal: true
module Gitlab
module Graphql
module Loaders
class BulkEpicAggregateLoader
include ::Gitlab::Graphql::Aggregations::Epics::Constants
# This class retrieves each epic and its child epics recursively
# It allows us to recreate the epic tree structure in POROs
def initialize(epic_ids:)
@results = {}
@target_epic_ids = epic_ids.present? ? [epic_ids].flatten.compact : []
end
# rubocop: disable CodeReuse/ActiveRecord
def execute
return {} unless target_epic_ids.any?
# We do a left outer join in order to capture epics with no issues
# This is so we can aggregate the epic counts for every epic
raw_results = ::Gitlab::ObjectHierarchy.new(Epic.where(id: target_epic_ids)).base_and_descendants
.left_joins(epic_issues: :issue)
.group("issues.state_id", "epics.id", "epics.iid", "epics.parent_id", "epics.state_id")
.select("epics.id, epics.iid, epics.parent_id, epics.state_id AS epic_state_id, issues.state_id AS issues_state_id, COUNT(issues) AS issues_count, SUM(COALESCE(issues.weight, 0)) AS issues_weight_sum")
raw_results = raw_results.map(&:attributes).map(&:with_indifferent_access)
group_by_epic_id(raw_results)
@results
end
# rubocop: enable CodeReuse/ActiveRecord
private
attr_reader :target_epic_ids, :results
def group_by_epic_id(raw_records)
# for each id, populate with matching records
# change from a series of { id: x ... } to { x => [{...}, {...}] }
raw_records.map { |r| r[:id] }.uniq.each do |epic_id|
records = []
matching_records = raw_records.select { |record| record[:id] == epic_id }
matching_records.each do |record|
records << record.except(:id).to_h.with_indifferent_access
end
@results[epic_id] = records
end
end
end
end
end
end
ee/spec/graphql/epics/aggregate_spec.rb deleted 100644 → 0
View file @ 920c4e70
# frozen_string_literal: true
require 'spec_helper'
describe Epics::Aggregate do
include_context 'includes EpicAggregate constants'
context 'when CountAggregate' do
subject { Epics::CountAggregate.new(sums) }
describe 'summation' do
let(:sums) { [] }
context 'when there are no sum objects' do
it 'returns 0 for all values', :aggregate_failures do
expect(subject.opened_issues).to eq 0
expect(subject.closed_issues).to eq 0
expect(subject.opened_epics).to eq 0
expect(subject.closed_epics).to eq 0
end
end
context 'when some sums exist' do
let(:sums) do
[
double(:sum, facet: COUNT_FACET, type: EPIC_TYPE, value: 1, state: OPENED_EPIC_STATE),
double(:sum, facet: COUNT_FACET, type: EPIC_TYPE, value: 1, state: CLOSED_EPIC_STATE),
double(:sum, facet: COUNT_FACET, type: ISSUE_TYPE, value: 1, state: OPENED_ISSUE_STATE),
double(:sum, facet: COUNT_FACET, type: ISSUE_TYPE, value: 1, state: OPENED_ISSUE_STATE),
double(:sum, facet: WEIGHT_SUM_FACET, type: ISSUE_TYPE, value: 22, state: CLOSED_ISSUE_STATE)
]
end
it 'returns sums of appropriate values', :aggregate_failures do
expect(subject.opened_issues).to eq 2
expect(subject.closed_issues).to eq 0
expect(subject.opened_epics).to eq 1
expect(subject.closed_epics).to eq 1
end
end
end
end
context 'when WeightSumAggregate' do
subject { Epics::WeightSumAggregate.new(sums) }
describe 'summation' do
let(:sums) { [] }
context 'when there are no sum objects' do
it 'returns 0 for all values', :aggregate_failures do
expect(subject.opened_issues).to eq 0
expect(subject.closed_issues).to eq 0
end
end
context 'when some sums exist' do
let(:sums) do
[
double(:sum, facet: WEIGHT_SUM_FACET, type: ISSUE_TYPE, value: 1, state: OPENED_ISSUE_STATE),
double(:sum, facet: WEIGHT_SUM_FACET, type: ISSUE_TYPE, value: 1, state: OPENED_ISSUE_STATE),
double(:sum, facet: COUNT_FACET, type: ISSUE_TYPE, value: 22, state: CLOSED_ISSUE_STATE)
]
end
it 'returns sums of appropriate values', :aggregate_failures do
expect(subject.opened_issues).to eq 2
expect(subject.closed_issues).to eq 0
end
end
end
end
end
spec/lib/gitlab/graphql/aggregations/epics/epic_node_spec.rb → ee/spec/lib/gitlab/graphql/aggregations/epics/epic_node_spec.rb
View file @ a883852c
...@@ -31,7 +31,7 @@ describe Gitlab::Graphql::Aggregations::Epics::EpicNode do ...@@ -31,7 +31,7 @@ describe Gitlab::Graphql::Aggregations::Epics::EpicNode do
it_behaves_like 'setting attributes based on the first record', { epic_state_id: CLOSED_EPIC_STATE, parent_id: 2 } it_behaves_like 'setting attributes based on the first record', { epic_state_id: CLOSED_EPIC_STATE, parent_id: 2 }
end end
describe '#assemble_issue_sums' do describe '#assemble_issue_totals' do
subject { described_class.new(epic_id, fake_data) } subject { described_class.new(epic_id, fake_data) }
context 'an epic with no issues' do context 'an epic with no issues' do
...@@ -41,10 +41,11 @@ describe Gitlab::Graphql::Aggregations::Epics::EpicNode do ...@@ -41,10 +41,11 @@ describe Gitlab::Graphql::Aggregations::Epics::EpicNode do
] ]
end end
it 'does not create any sums' do it 'does not create any totals' do
subject.assemble_issue_sums subject.assemble_issue_totals
expect(subject.direct_sums.count).to eq 0 expect(subject.immediate_totals(COUNT_FACET).count).to eq 0
expect(subject.immediate_totals(WEIGHT_SUM_FACET).count).to eq 0
end end
end end
...@@ -57,10 +58,11 @@ describe Gitlab::Graphql::Aggregations::Epics::EpicNode do ...@@ -57,10 +58,11 @@ describe Gitlab::Graphql::Aggregations::Epics::EpicNode do
end end
it 'creates no sums for the weight if the issues have 0 weight' do it 'creates no sums for the weight if the issues have 0 weight' do
subject.assemble_issue_sums subject.assemble_issue_totals
expect(subject.direct_sums.count).to eq 1 expect(subject.immediate_totals(COUNT_FACET).count).to eq 1
expect(subject).to have_direct_sum(ISSUE_TYPE, COUNT_FACET, OPENED_ISSUE_STATE, 1) expect(subject.immediate_totals(WEIGHT_SUM_FACET).count).to eq 0
expect(subject).to have_immediate_total(ISSUE_TYPE, COUNT_FACET, OPENED_ISSUE_STATE, 1)
end end
end end
...@@ -72,11 +74,10 @@ describe Gitlab::Graphql::Aggregations::Epics::EpicNode do ...@@ -72,11 +74,10 @@ describe Gitlab::Graphql::Aggregations::Epics::EpicNode do
end end
it 'creates two sums' do it 'creates two sums' do
subject.assemble_issue_sums subject.assemble_issue_totals
expect(subject.direct_sums.count).to eq 2 expect(subject).to have_immediate_total(ISSUE_TYPE, COUNT_FACET, OPENED_ISSUE_STATE, 1)
expect(subject).to have_direct_sum(ISSUE_TYPE, COUNT_FACET, OPENED_ISSUE_STATE, 1) expect(subject).to have_immediate_total(ISSUE_TYPE, WEIGHT_SUM_FACET, OPENED_ISSUE_STATE, 2)
expect(subject).to have_direct_sum(ISSUE_TYPE, WEIGHT_SUM_FACET, OPENED_ISSUE_STATE, 2)
end end
end end
...@@ -89,19 +90,18 @@ describe Gitlab::Graphql::Aggregations::Epics::EpicNode do ...@@ -89,19 +90,18 @@ describe Gitlab::Graphql::Aggregations::Epics::EpicNode do
end end
it 'creates two sums' do it 'creates two sums' do
subject.assemble_issue_sums subject.assemble_issue_totals
expect(subject.direct_sums.count).to eq 4 expect(subject).to have_immediate_total(ISSUE_TYPE, COUNT_FACET, OPENED_ISSUE_STATE, 1)
expect(subject).to have_direct_sum(ISSUE_TYPE, COUNT_FACET, OPENED_ISSUE_STATE, 1) expect(subject).to have_immediate_total(ISSUE_TYPE, WEIGHT_SUM_FACET, OPENED_ISSUE_STATE, 2)
expect(subject).to have_direct_sum(ISSUE_TYPE, WEIGHT_SUM_FACET, OPENED_ISSUE_STATE, 2) expect(subject).to have_immediate_total(ISSUE_TYPE, COUNT_FACET, CLOSED_ISSUE_STATE, 3)
expect(subject).to have_direct_sum(ISSUE_TYPE, COUNT_FACET, CLOSED_ISSUE_STATE, 3) expect(subject).to have_immediate_total(ISSUE_TYPE, WEIGHT_SUM_FACET, CLOSED_ISSUE_STATE, 5)
expect(subject).to have_direct_sum(ISSUE_TYPE, WEIGHT_SUM_FACET, CLOSED_ISSUE_STATE, 5)
end end
end end
end end
end end
describe '#assemble_epic_sums' do describe '#assemble_epic_totals' do
subject { described_class.new(epic_id, [{ parent_id: nil, epic_state_id: CLOSED_EPIC_STATE }]) } subject { described_class.new(epic_id, [{ parent_id: nil, epic_state_id: CLOSED_EPIC_STATE }]) }
context 'with a child epic' do context 'with a child epic' do
...@@ -113,10 +113,9 @@ describe Gitlab::Graphql::Aggregations::Epics::EpicNode do ...@@ -113,10 +113,9 @@ describe Gitlab::Graphql::Aggregations::Epics::EpicNode do
end end
it 'adds up the number of the child epics' do it 'adds up the number of the child epics' do
subject.assemble_epic_sums([child_epic_node]) subject.assemble_epic_totals([child_epic_node])
expect(subject.direct_sums.count).to eq 1 expect(subject).to have_immediate_total(EPIC_TYPE, COUNT_FACET, CLOSED_EPIC_STATE, 1)
expect(subject).to have_direct_sum(EPIC_TYPE, COUNT_FACET, CLOSED_EPIC_STATE, 1)
end end
end end
end end
...@@ -125,7 +124,16 @@ describe Gitlab::Graphql::Aggregations::Epics::EpicNode do ...@@ -125,7 +124,16 @@ describe Gitlab::Graphql::Aggregations::Epics::EpicNode do
subject { described_class.new(epic_id, [{ parent_id: nil, epic_state_id: CLOSED_EPIC_STATE }]) } subject { described_class.new(epic_id, [{ parent_id: nil, epic_state_id: CLOSED_EPIC_STATE }]) }
before do before do
allow(subject).to receive(:direct_sums).and_return(direct_sums) allow(subject).to receive(:immediate_totals).with(COUNT_FACET).and_return(immediate_count_totals)
allow(subject).to receive(:immediate_totals).with(WEIGHT_SUM_FACET).and_return(immediate_weight_sum_totals)
end
shared_examples 'returns calculated totals by facet' do |facet, count|
it 'returns a calculated_count_total' do
subject.calculate_recursive_sums(facet, tree)
expect(subject.calculated_totals(facet).count).to eq count
end
end end
context 'an epic with no child epics' do context 'an epic with no child epics' do
...@@ -134,45 +142,37 @@ describe Gitlab::Graphql::Aggregations::Epics::EpicNode do ...@@ -134,45 +142,37 @@ describe Gitlab::Graphql::Aggregations::Epics::EpicNode do
end end
context 'with no child issues' do context 'with no child issues' do
let(:direct_sums) do let(:immediate_count_totals) { [] }
[] let(:immediate_weight_sum_totals) { [] }
end
it 'returns a SumTotal with no sums' do
result = subject.calculate_recursive_sums(tree)
expect(result.sums).not_to be_nil it_behaves_like 'returns calculated totals by facet', COUNT_FACET, 0
expect(result.sums.count).to eq 0 it_behaves_like 'returns calculated totals by facet', WEIGHT_SUM_FACET, 0
end
end end
context 'with an issue with 0 weight' do context 'with an issue with 0 weight' do
let(:direct_sums) do let(:immediate_count_totals) do
[Gitlab::Graphql::Aggregations::Epics::EpicNode::Sum.new(COUNT_FACET, CLOSED_EPIC_STATE, ISSUE_TYPE, 1)] [Gitlab::Graphql::Aggregations::Epics::EpicNode::Sum.new(COUNT_FACET, CLOSED_EPIC_STATE, ISSUE_TYPE, 1)]
end end
let(:immediate_weight_sum_totals) { [] }
it 'returns a SumTotal with only a weight sum' do it_behaves_like 'returns calculated totals by facet', COUNT_FACET, 1
result = subject.calculate_recursive_sums(tree) it_behaves_like 'returns calculated totals by facet', WEIGHT_SUM_FACET, 0
expect(result.sums).not_to be_nil
expect(result.sums.count).to eq 1
end
end end
context 'with an issue with nonzero weight' do context 'with an issue with nonzero weight' do
let(:direct_sums) do let(:immediate_count_totals) do
[
Gitlab::Graphql::Aggregations::Epics::EpicNode::Sum.new(COUNT_FACET, CLOSED_EPIC_STATE, ISSUE_TYPE, 1)
]
end
let(:immediate_weight_sum_totals) do
[ [
Gitlab::Graphql::Aggregations::Epics::EpicNode::Sum.new(COUNT_FACET, CLOSED_EPIC_STATE, ISSUE_TYPE, 1),
Gitlab::Graphql::Aggregations::Epics::EpicNode::Sum.new(WEIGHT_SUM_FACET, CLOSED_EPIC_STATE, ISSUE_TYPE, 2) Gitlab::Graphql::Aggregations::Epics::EpicNode::Sum.new(WEIGHT_SUM_FACET, CLOSED_EPIC_STATE, ISSUE_TYPE, 2)
] ]
end end
it 'returns a SumTotal with only a weight sum' do it_behaves_like 'returns calculated totals by facet', COUNT_FACET, 1
result = subject.calculate_recursive_sums(tree) it_behaves_like 'returns calculated totals by facet', WEIGHT_SUM_FACET, 1
expect(result.sums).not_to be_nil
expect(result.sums.count).to eq 2
end
end end
end end
...@@ -182,30 +182,41 @@ describe Gitlab::Graphql::Aggregations::Epics::EpicNode do ...@@ -182,30 +182,41 @@ describe Gitlab::Graphql::Aggregations::Epics::EpicNode do
{ epic_id => subject, child_epic_id => child_epic_node } { epic_id => subject, child_epic_id => child_epic_node }
end end
let(:child_epic_node) { described_class.new(child_epic_id, [{ parent_id: epic_id, epic_state_id: CLOSED_EPIC_STATE }]) } let(:child_epic_node) { described_class.new(child_epic_id, [{ parent_id: epic_id, epic_state_id: CLOSED_EPIC_STATE }]) }
let(:direct_sums) do let(:immediate_count_totals) do
[ # only one opened epic, the child [ # only one opened epic, the child
Gitlab::Graphql::Aggregations::Epics::EpicNode::Sum.new(COUNT_FACET, OPENED_EPIC_STATE, EPIC_TYPE, 1) Gitlab::Graphql::Aggregations::Epics::EpicNode::Sum.new(COUNT_FACET, OPENED_EPIC_STATE, EPIC_TYPE, 1)
] ]
end end
let(:immediate_weight_sum_totals) { [] }
before do before do
subject.child_ids << child_epic_id subject.child_ids << child_epic_id
allow(child_epic_node).to receive(:direct_sums).and_return(child_sums) allow(child_epic_node).to receive(:immediate_totals).with(COUNT_FACET).and_return(child_count_totals)
allow(child_epic_node).to receive(:immediate_totals).with(WEIGHT_SUM_FACET).and_return(child_weight_sum_totals)
end end
context 'with a child that has issues of nonzero weight' do context 'with a child that has issues of nonzero weight' do
let(:child_sums) do let(:child_count_totals) do
[ # 1 issue of weight 2 [
Gitlab::Graphql::Aggregations::Epics::EpicNode::Sum.new(COUNT_FACET, OPENED_ISSUE_STATE, ISSUE_TYPE, 1), Gitlab::Graphql::Aggregations::Epics::EpicNode::Sum.new(COUNT_FACET, OPENED_ISSUE_STATE, ISSUE_TYPE, 1)
]
end
let(:child_weight_sum_totals) do
[
Gitlab::Graphql::Aggregations::Epics::EpicNode::Sum.new(WEIGHT_SUM_FACET, OPENED_ISSUE_STATE, ISSUE_TYPE, 2) Gitlab::Graphql::Aggregations::Epics::EpicNode::Sum.new(WEIGHT_SUM_FACET, OPENED_ISSUE_STATE, ISSUE_TYPE, 2)
] ]
end end
it 'returns the correct sum total' do it 'returns the correct count total' do
result = subject.calculate_recursive_sums(tree) subject.calculate_recursive_sums(COUNT_FACET, tree)
expect(subject.calculated_count_totals.count).to eq 2
end
it 'returns the correct weight sum total' do
subject.calculate_recursive_sums(WEIGHT_SUM_FACET, tree)
expect(result.sums).not_to be_nil expect(subject.calculated_weight_sum_totals.count).to eq 1
expect(result.sums.count).to eq 3
end end
end end
end end
......
spec/lib/gitlab/graphql/aggregations/epics/lazy_epic_aggregate_spec.rb → ee/spec/lib/gitlab/graphql/aggregations/epics/lazy_epic_aggregate_spec.rb
View file @ a883852c
...@@ -58,8 +58,8 @@ describe Gitlab::Graphql::Aggregations::Epics::LazyEpicAggregate do ...@@ -58,8 +58,8 @@ describe Gitlab::Graphql::Aggregations::Epics::LazyEpicAggregate do
end end
it 'does not make the query again' do it 'does not make the query again' do
expect(epic_info_node).to receive(:aggregate_object_by).with(subject.facet) expect(epic_info_node).to receive(:aggregate_count)
expect(Gitlab::Graphql::Aggregations::Epics::BulkEpicAggregateLoader).not_to receive(:new) expect(Gitlab::Graphql::Loaders::BulkEpicAggregateLoader).not_to receive(:new)
subject.epic_aggregate subject.epic_aggregate
end end
...@@ -79,8 +79,8 @@ describe Gitlab::Graphql::Aggregations::Epics::LazyEpicAggregate do ...@@ -79,8 +79,8 @@ describe Gitlab::Graphql::Aggregations::Epics::LazyEpicAggregate do
end end
before do before do
allow(Gitlab::Graphql::Aggregations::Epics::EpicNode).to receive(:aggregate_object_by).and_call_original allow(Gitlab::Graphql::Aggregations::Epics::EpicNode).to receive(:aggregate_count).and_call_original
expect_any_instance_of(Gitlab::Graphql::Aggregations::Epics::BulkEpicAggregateLoader).to receive(:execute).and_return(fake_data) expect_any_instance_of(Gitlab::Graphql::Loaders::BulkEpicAggregateLoader).to receive(:execute).and_return(fake_data)
end end
it 'clears the pending IDs' do it 'clears the pending IDs' do
...@@ -108,12 +108,12 @@ describe Gitlab::Graphql::Aggregations::Epics::LazyEpicAggregate do ...@@ -108,12 +108,12 @@ describe Gitlab::Graphql::Aggregations::Epics::LazyEpicAggregate do
lazy_state = subject.instance_variable_get(:@lazy_state) lazy_state = subject.instance_variable_get(:@lazy_state)
tree = lazy_state[:tree] tree = lazy_state[:tree]
expect(tree[epic_id]).to have_direct_sum(EPIC_TYPE, COUNT_FACET, CLOSED_EPIC_STATE, 1) expect(tree[epic_id]).to have_immediate_total(EPIC_TYPE, COUNT_FACET, CLOSED_EPIC_STATE, 1)
expect(tree[epic_id]).to have_direct_sum(ISSUE_TYPE, WEIGHT_SUM_FACET, OPENED_ISSUE_STATE, 5) expect(tree[epic_id]).to have_immediate_total(ISSUE_TYPE, WEIGHT_SUM_FACET, OPENED_ISSUE_STATE, 5)
expect(tree[epic_id]).to have_direct_sum(EPIC_TYPE, COUNT_FACET, CLOSED_EPIC_STATE, 1) expect(tree[epic_id]).to have_immediate_total(EPIC_TYPE, COUNT_FACET, CLOSED_EPIC_STATE, 1)
expect(tree[child_epic_id]).to have_direct_sum(ISSUE_TYPE, COUNT_FACET, CLOSED_ISSUE_STATE, 4) expect(tree[child_epic_id]).to have_immediate_total(ISSUE_TYPE, COUNT_FACET, CLOSED_ISSUE_STATE, 4)
expect(tree[child_epic_id]).to have_direct_sum(ISSUE_TYPE, WEIGHT_SUM_FACET, CLOSED_ISSUE_STATE, 17) expect(tree[child_epic_id]).to have_immediate_total(ISSUE_TYPE, WEIGHT_SUM_FACET, CLOSED_ISSUE_STATE, 17)
end end
it 'assembles recursive sums for the parent', :aggregate_failures do it 'assembles recursive sums for the parent', :aggregate_failures do
...@@ -122,22 +122,23 @@ describe Gitlab::Graphql::Aggregations::Epics::LazyEpicAggregate do ...@@ -122,22 +122,23 @@ describe Gitlab::Graphql::Aggregations::Epics::LazyEpicAggregate do
lazy_state = subject.instance_variable_get(:@lazy_state) lazy_state = subject.instance_variable_get(:@lazy_state)
tree = lazy_state[:tree] tree = lazy_state[:tree]
expect(tree[epic_id]).to have_aggregate(ISSUE_TYPE, COUNT_FACET, OPENED_ISSUE_STATE, 2) expect(tree[epic_id]).to have_aggregate(tree, ISSUE_TYPE, COUNT_FACET, OPENED_ISSUE_STATE, 2)
expect(tree[epic_id]).to have_aggregate(ISSUE_TYPE, COUNT_FACET, CLOSED_ISSUE_STATE, 4) expect(tree[epic_id]).to have_aggregate(tree, ISSUE_TYPE, COUNT_FACET, CLOSED_ISSUE_STATE, 4)
expect(tree[epic_id]).to have_aggregate(ISSUE_TYPE, WEIGHT_SUM_FACET, OPENED_ISSUE_STATE, 5) expect(tree[epic_id]).to have_aggregate(tree, ISSUE_TYPE, WEIGHT_SUM_FACET, OPENED_ISSUE_STATE, 5)
expect(tree[epic_id]).to have_aggregate(ISSUE_TYPE, WEIGHT_SUM_FACET, CLOSED_ISSUE_STATE, 17) expect(tree[epic_id]).to have_aggregate(tree, ISSUE_TYPE, WEIGHT_SUM_FACET, CLOSED_ISSUE_STATE, 17)
expect(tree[epic_id]).to have_aggregate(EPIC_TYPE, COUNT_FACET, CLOSED_EPIC_STATE, 1) expect(tree[epic_id]).to have_aggregate(tree, EPIC_TYPE, COUNT_FACET, CLOSED_EPIC_STATE, 1)
end end
end end
context 'for a standalone epic with no issues' do context 'for a standalone epic with no issues' do
it 'assembles direct sums', :aggregate_failures do it 'assembles direct totals', :aggregate_failures do
subject.epic_aggregate subject.epic_aggregate
lazy_state = subject.instance_variable_get(:@lazy_state) lazy_state = subject.instance_variable_get(:@lazy_state)
tree = lazy_state[:tree] tree = lazy_state[:tree]
expect(tree[other_epic_id].direct_sums).to be_empty expect(tree[other_epic_id].immediate_count_totals).to be_empty
expect(tree[other_epic_id].immediate_weight_sum_totals).to be_empty
end end
end end
end end
......
lib/gitlab/graphql/aggregations/epics/bulk_epic_aggregate_loader_spec.rb → ee/spec/lib/gitlab/graphql/loaders/bulk_epic_aggregate_loader_spec.rb
View file @ a883852c
...@@ -2,7 +2,7 @@ ...@@ -2,7 +2,7 @@
require 'spec_helper' require 'spec_helper'
describe Gitlab::Graphql::Aggregations::Epics::BulkEpicAggregateLoader do describe Gitlab::Graphql::Loaders::BulkEpicAggregateLoader do
let_it_be(:closed_issue_state_id) { Issue.available_states[:closed] } let_it_be(:closed_issue_state_id) { Issue.available_states[:closed] }
let_it_be(:opened_issue_state_id) { Issue.available_states[:opened] } let_it_be(:opened_issue_state_id) { Issue.available_states[:opened] }
......
ee/spec/requests/api/graphql/group/epic/epic_aggregate_query_spec.rb
View file @ a883852c
...@@ -90,7 +90,7 @@ describe 'Epic aggregates (count and weight)' do ...@@ -90,7 +90,7 @@ describe 'Epic aggregates (count and weight)' do
it 'uses the LazyEpicAggregate service' do it 'uses the LazyEpicAggregate service' do
# one for count, one for weight_sum, even though the share the same tree state as part of the context # one for count, one for weight_sum, even though the share the same tree state as part of the context
expect(Epics::LazyEpicAggregate).to receive(:new).twice expect(Gitlab::Graphql::Aggregations::Epics::LazyEpicAggregate).to receive(:new).twice
post_graphql(query, current_user: current_user) post_graphql(query, current_user: current_user)
end end
...@@ -156,4 +156,5 @@ describe 'Epic aggregates (count and weight)' do ...@@ -156,4 +156,5 @@ describe 'Epic aggregates (count and weight)' do
end end
end end
end end
end
end end
ee/spec/support/matchers/ee/epic_aggregate_matchers.rb
View file @ a883852c
# frozen_string_literal: true # frozen_string_literal: true
RSpec::Matchers.define :have_direct_sum do |type, facet, state, value| RSpec::Matchers.define :have_immediate_total do |type, facet, state, expected_value|
match do |epic_node_result| match do |epic_node_result|
expect(epic_node_result).not_to be_nil expect(epic_node_result).not_to be_nil
expect(epic_node_result.direct_sums).not_to be_empty immediate_totals = epic_node_result.immediate_totals(facet)
expect(immediate_totals).not_to be_empty
matching = epic_node_result.direct_sums.select { |sum| sum.type == type && sum.facet == facet && sum.state == state && sum.value == value } matching = immediate_totals.select { |sum| sum.type == type && sum.facet == facet && sum.state == state && sum.value == expected_value }
expect(matching).not_to be_empty expect(matching).not_to be_empty
end end
...@@ -13,24 +14,25 @@ RSpec::Matchers.define :have_direct_sum do |type, facet, state, value| ...@@ -13,24 +14,25 @@ RSpec::Matchers.define :have_direct_sum do |type, facet, state, value|
if epic_node_result.nil? if epic_node_result.nil?
"expected for there to be an epic node, but it is nil" "expected for there to be an epic node, but it is nil"
else else
immediate_totals = epic_node_result.immediate_totals(facet)
<<~FAILURE_MSG <<~FAILURE_MSG
expected epic node with id #{epic_node_result.epic_id} to have a sum with facet '#{facet}', state '#{state}', type '#{type}' and value '#{value}'. Has #{epic_node_result.direct_sums.count} sum objects#{", none of which match" if epic_node_result.direct_sums.count > 0}. expected epic node with id #{epic_node_result.epic_id} to have a sum with facet '#{facet}', state '#{state}', type '#{type}' and value '#{expected_value}'. Has #{immediate_totals.count} immediate sum objects#{", none of which match" if immediate_totals.count > 0}.
Sums: #{epic_node_result.direct_sums.inspect} Sums: #{immediate_totals.inspect}
FAILURE_MSG FAILURE_MSG
end end
end end
end end
RSpec::Matchers.define :have_aggregate do |type, facet, state, value| RSpec::Matchers.define :have_aggregate do |tree, type, facet, state, expected_value|
match do |epic_node_result| match do |epic_node_result|
aggregate_object = epic_node_result.aggregate_object_by(facet) aggregate_object = epic_node_result.public_send(:"aggregate_#{facet}", tree)
expect(aggregate_object.send(method_name(type, state))).to eq value expect(aggregate_object.public_send(method_name(type, state))).to eq expected_value
end end
failure_message do |epic_node_result| failure_message do |epic_node_result|
aggregate_object = epic_node_result.aggregate_object_by(facet) aggregate_object = epic_node_result.public_send(:"aggregate_#{facet}", tree)
aggregate_method = method_name(type, state) aggregate_method = method_name(type, state)
"Epic node with id #{epic_node_result.epic_id} called #{aggregate_method} on aggregate object of type #{aggregate_object.class.name}. Value was expected to be #{value} but was #{aggregate_object.send(aggregate_method)}." "Epic node with id #{epic_node_result.epic_id} called #{aggregate_method} on aggregate object. Value was expected to be #{expected_value} but was #{aggregate_object.send(aggregate_method)}."
end end
def method_name(type, state) def method_name(type, state)
......
lib/gitlab/graphql/aggregations/epics/bulk_epic_aggregate_loader.rb deleted 100644 → 0
View file @ 920c4e70
# frozen_string_literal: true
module Gitlab
module Graphql
module Aggregations
module Epics
class BulkEpicAggregateLoader
include Constants
# This class retrieves each epic and its child epics recursively
# It allows us to recreate the epic tree structure in POROs
def initialize(epic_ids:)
@results = {}
@target_epic_ids = epic_ids.present? ? [epic_ids].flatten.compact : []
end
# rubocop: disable CodeReuse/ActiveRecord
def execute
return {} unless target_epic_ids.any?
# We do a left outer join in order to capture epics with no issues
# This is so we can aggregate the epic counts for every epic
raw_results = ::Gitlab::ObjectHierarchy.new(Epic.where(id: target_epic_ids)).base_and_descendants
.left_joins(epic_issues: :issue)
.group("issues.state_id", "epics.id", "epics.iid", "epics.parent_id", "epics.state_id")
.select("epics.id, epics.iid, epics.parent_id, epics.state_id AS epic_state_id, issues.state_id AS issues_state_id, COUNT(issues) AS issues_count, SUM(COALESCE(issues.weight, 0)) AS issues_weight_sum")
raw_results = raw_results.map(&:attributes).map(&:with_indifferent_access)
group_by_epic_id(raw_results)
@results
end
# rubocop: enable CodeReuse/ActiveRecord
private
attr_reader :target_epic_ids, :results
def group_by_epic_id(raw_records)
# for each id, populate with matching records
# change from a series of { id: x ... } to { x => [{...}, {...}] }
raw_records.map { |r| r[:id] }.uniq.each do |epic_id|
records = []
matching_records = raw_records.select { |record| record[:id] == epic_id }
matching_records.each do |record|
records << record.except(:id).to_h.with_indifferent_access
end
@results[epic_id] = records
end
end
end
end
end
end
end
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment
GitLab Nexedi Edition | About GitLab | About Nexedi | 沪ICP备2021021310号-2 | 沪ICP备2021021310号-7