Commit cb5eb4d2 authored by charlieablett's avatar charlieablett

Bulk epic issues loader

Loads epic tree into a data structure
with epic id, iid, issue count and parent_id

Add lazy resolver

Connect CTE to a lazy resolver, which does a bulk
query for all the epic aggregate (weight and count)
rather than get a separate query for each. This also
prevents us from having to get information we already
have since one requested epic might be a child epic
of another.

Connect aggregate results with GraphQL type
- Use a PORO to represent the data via method exposure
Put together the recursive logic to sum the counts/weights
while separating them by graphql facet (count or weight_sum).
parent 11bce289
......@@ -28,6 +28,8 @@ class GitlabSchema < GraphQL::Schema
default_max_page_size 100
lazy_resolve Epics::LazyEpicAggregate, :epic_aggregate
class << self
def multiplex(queries, **kwargs)
kwargs[:max_complexity] ||= max_query_complexity(kwargs[:context])
......
......@@ -74,8 +74,6 @@ class Issue < ApplicationRecord
scope :public_only, -> { where(confidential: false) }
scope :confidential_only, -> { where(confidential: true) }
scope :counts_by_state, -> { reorder(nil).group(:state_id).count }
ignore_column :state, remove_with: '12.7', remove_after: '2019-12-22'
after_commit :expire_etag_cache, unless: :importing?
......
......@@ -4987,6 +4987,20 @@
"isDeprecated": false,
"deprecationReason": null
},
{
"name": "descendantWeightSum",
"description": "Total weight of open and closed descendant epic's issues",
"args": [
],
"type": {
"kind": "OBJECT",
"name": "EpicDescendantWeights",
"ofType": null
},
"isDeprecated": false,
"deprecationReason": null
},
{
"name": "description",
"description": "Description of the epic",
......@@ -13084,6 +13098,47 @@
"enumValues": null,
"possibleTypes": null
},
{
"kind": "OBJECT",
"name": "EpicDescendantWeights",
"description": "Total weight of open and closed descendant issues",
"fields": [
{
"name": "closedIssues",
"description": "Total weight of completed (closed) issues in this epic, including epic descendants",
"args": [
],
"type": {
"kind": "SCALAR",
"name": "Int",
"ofType": null
},
"isDeprecated": false,
"deprecationReason": null
},
{
"name": "openedIssues",
"description": "Total weight of opened issues in this epic, including epic descendants",
"args": [
],
"type": {
"kind": "SCALAR",
"name": "Int",
"ofType": null
},
"isDeprecated": false,
"deprecationReason": null
}
],
"inputFields": null,
"interfaces": [
],
"enumValues": null,
"possibleTypes": null
},
{
"kind": "OBJECT",
"name": "TimelogConnection",
......
# frozen_string_literal: true
module Epics
class Aggregate
include AggregateConstants
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
# frozen_string_literal: true
module Epics
module AggregateConstants
ISSUE_TYPE = :issue
EPIC_TYPE = :epic
CLOSED_ISSUE_STATE = Issue.available_states[:closed].freeze
OPENED_ISSUE_STATE = Issue.available_states[:opened].freeze
CLOSED_EPIC_STATE = Epic.available_states[:closed].freeze
OPENED_EPIC_STATE = Epic.available_states[:opened].freeze
COUNT = :count
WEIGHT_SUM = :weight_sum
end
end
# frozen_string_literal: true
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
# frozen_string_literal: true
# This class represents an Epic's aggregate information (added up counts) about its child epics and direct issues
module Epics
class EpicNode
include AggregateConstants
attr_reader :epic_id, :epic_state_id, :epic_info_flat_list, :parent_id,
:direct_sums, # we calculate these and recursively add them
:sum_total
attr_accessor :child_ids
def initialize(epic_id, flat_info_list)
# 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}] ... }
# They include the sum of each epic's direct issues, grouped by status,
# so in order to get a sum of the entire tree, we have to add that up recursively
@epic_id = epic_id
@epic_info_flat_list = flat_info_list
@child_ids = []
@direct_sums = []
set_epic_attributes(flat_info_list.first) # there will always be one
end
def assemble_issue_sums
# this is a representation of the epic's
# direct child issues and epics that have come from the DB
[OPENED_ISSUE_STATE, CLOSED_ISSUE_STATE].each do |issue_state|
matching_issue_state_entry = epic_info_flat_list.find do |epic_info_node|
epic_info_node[:issues_state_id] == issue_state
end || {}
create_sum_if_needed(WEIGHT_SUM, issue_state, ISSUE_TYPE, matching_issue_state_entry.fetch(:issues_weight_sum, 0))
create_sum_if_needed(COUNT, issue_state, ISSUE_TYPE, matching_issue_state_entry.fetch(:issues_count, 0))
end
end
def assemble_epic_sums(children)
[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)
end
end
def calculate_recursive_sums(tree)
return sum_total if sum_total
@sum_total = SumTotal.new
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(tree).sums
sum_total.add(child_sums)
end
sum_total.add(direct_sums)
sum_total
end
def aggregate_object_by(facet)
sum_total.by_facet(facet)
end
def to_s
{ epic_id: @epic_id, parent_id: @parent_id, direct_sums: direct_sums, child_ids: child_ids }.to_json
end
alias_method :inspect, :to_s
alias_method :id, :epic_id
private
def create_sum_if_needed(facet, state, type, value)
return if value.nil? || value < 1
direct_sums << Sum.new(facet, state, type, value)
end
def set_epic_attributes(record)
@epic_state_id = record[:epic_state_id]
@parent_id = record[:parent_id]
end
Sum = Struct.new(:facet, :state, :type, :value) do
def inspect
"<Sum facet=#{facet}, state=#{state}, type=#{type}, value=#{value}>"
end
end
end
end
# frozen_string_literal: true
module Epics
class LazyEpicAggregate
include AggregateConstants
attr_reader :facet, :epic_id, :lazy_state
# Because facets "count" and "weight_sum" share the same db query, but have a different graphql type object,
# we can separate them and serve only the fields which are requested by the GraphQL query
def initialize(query_ctx, epic_id, aggregate_facet)
@epic_id = epic_id
raise ArgumentError.new("No aggregate facet provided. Please specify either #{COUNT} or #{WEIGHT_SUM}") unless aggregate_facet.present?
raise ArgumentError.new("Invalid aggregate facet #{aggregate_facet} provided. Please specify either #{COUNT} or #{WEIGHT_SUM}") unless [COUNT, WEIGHT_SUM].include?(aggregate_facet.to_sym)
@facet = aggregate_facet.to_sym
# Initialize the loading state for this query,
# or get the previously-initiated state
@lazy_state = query_ctx[:lazy_epic_aggregate] ||= {
pending_ids: Set.new,
tree: {}
}
# Register this ID to be loaded later:
@lazy_state[:pending_ids] << epic_id
end
# Return the loaded record, hitting the database if needed
def epic_aggregate
# Check if the record was already loaded:
# load from tree by epic
loaded_epic_info_node = @lazy_state[:tree][@epic_id]
if loaded_epic_info_node
# The pending IDs were already loaded,
# so return the result of that previous load
loaded_epic_info_node.aggregate_object_by(@facet)
else
load_records_into_tree
end
end
private
def tree
@lazy_state[:tree]
end
def load_records_into_tree
# The record hasn't been loaded yet, so
# hit the database with all pending IDs
pending_ids = @lazy_state[:pending_ids].to_a
# 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
# Assemble the tree and sum everything
create_structure_from(raw_epic_aggregates)
@lazy_state[:pending_ids].clear
# Now, get the matching node and return its aggregate depending on the facet:
epic_node = @lazy_state[:tree][@epic_id]
epic_node.aggregate_object_by(@facet)
end
def create_structure_from(aggregate_records)
# create EpicNode object for each epic id
aggregate_records.each do |epic_id, aggregates|
next if aggregates.nil? || aggregates.empty?
new_node = EpicNode.new(epic_id, aggregates)
tree[epic_id] = new_node
end
associate_parents_and_children
assemble_direct_sums
calculate_recursive_sums
end
# each of the methods below are done one after the other
def associate_parents_and_children
tree.each do |epic_id, node|
node.child_ids = tree.select { |_, child_node| epic_id == child_node.parent_id }.keys
end
end
def assemble_direct_sums
tree.each do |_, node|
node.assemble_issue_sums
node_children = tree.select { |_, child_node| node.epic_id == child_node.parent_id }.values
node.assemble_epic_sums(node_children)
end
end
def calculate_recursive_sums
tree.each do |_, node|
node.calculate_recursive_sums(tree)
end
end
end
end
# frozen_string_literal: true
module Epics
class SumTotal
include AggregateConstants
attr_accessor :sums
def initialize
@sums = []
end
def add(other_sums)
sums.concat(other_sums)
end
def by_facet(facet)
return ::Epics::CountAggregate.new(sums) if facet == COUNT
::Epics::WeightSumAggregate.new(sums)
end
end
end
# frozen_string_literal: true
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
......@@ -121,21 +121,26 @@ module Types
resolver: Resolvers::EpicIssuesResolver
field :descendant_counts, Types::EpicDescendantCountType, null: true, complexity: 10,
description: 'Number of open and closed descendant epics and issues',
resolve: -> (epic, args, ctx) do
Epics::DescendantCountService.new(epic, ctx[:current_user])
end
description: 'Number of open and closed descendant epics and issues',
resolve: -> (epic, args, ctx) do
if Feature.enabled?(:unfiltered_epic_aggregates)
Epics::LazyEpicAggregate.new(ctx, epic.id, Epics::LazyEpicAggregate::COUNT)
else
Epics::DescendantCountService.new(epic, ctx[:current_user])
end
end
field :descendant_weight_sum, Types::EpicDescendantWeightSumType, null: true, complexity: 10,
description: "Total weight of open and closed descendant epic's issues",
feature_flag: :unfiltered_epic_aggregates
def descendant_weight_sum
OpenStruct.new(
# We shouldn't stop the whole query, so returning -1 for a semi-noisy error
opened_issues: -1,
closed_issues: -1
)
end
description: "Total weight of open and closed descendant epic's issues",
feature_flag: :unfiltered_epic_aggregates,
resolve: -> (epic, args, ctx) do
Epics::LazyEpicAggregate.new(ctx, epic.id, Epics::LazyEpicAggregate::WEIGHT_SUM)
end
field :health_status,
::Types::HealthStatusEnum,
null: true,
description: 'Current health status',
feature_flag: :save_issuable_health_status
end
end
......@@ -102,8 +102,6 @@ module EE
scope :start_date_inherited, -> { where(start_date_is_fixed: [nil, false]) }
scope :due_date_inherited, -> { where(due_date_is_fixed: [nil, false]) }
scope :counts_by_state, -> { group(:state_id).count }
MAX_HIERARCHY_DEPTH = 5
def etag_caching_enabled?
......
# frozen_string_literal: true
module Epics
class BulkEpicAggregateLoader
include AggregateConstants
# 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
# frozen_string_literal: true
describe Epics::Aggregate do
let(:epic_type) { described_class::EPIC_TYPE }
let(:issue_type) { described_class::ISSUE_TYPE }
let(:opened_epic_state) { described_class::OPENED_EPIC_STATE }
let(:closed_epic_state) { described_class::CLOSED_EPIC_STATE }
let(:opened_issue_state) { described_class::OPENED_ISSUE_STATE }
let(:closed_issue_state) { described_class::CLOSED_ISSUE_STATE }
let(:weight_sum) { Epics::EpicNode::WEIGHT_SUM }
let(:count) { Epics::EpicNode::COUNT }
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
[
Epics::Sum.new(facet: :count, type: epic_type, value: 1, state: opened_epic_state),
Epics::Sum.new(facet: :count, type: epic_type, value: 1, state: closed_epic_state),
Epics::Sum.new(facet: :count, type: issue_type, value: 1, state: opened_issue_state),
Epics::Sum.new(facet: :count, type: issue_type, value: 1, state: opened_issue_state),
Epics::Sum.new(facet: :weight_sum, 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
[
Epics::Sum.new(facet: :weight_sum, type: issue_type, value: 1, state: opened_issue_state),
Epics::Sum.new(facet: :weight_sum, type: issue_type, value: 1, state: opened_issue_state),
Epics::Sum.new(facet: :count, 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
# frozen_string_literal: true
require 'spec_helper'
describe Epics::EpicNode do
let(:epic_id) { 34 }
let(:epic_iid) { 5 }
describe '#initialize' do
let(:fake_data) do
[
{ iid: epic_iid, epic_state_id: epic_state_id, issues_count: 1, issues_weight_sum: 2, parent_id: parent_id, issues_state_id: Constants::OPENED_ISSUE_STATE },
{ iid: epic_iid, epic_state_id: epic_state_id, issues_count: 2, issues_weight_sum: 2, parent_id: parent_id, issues_state_id: Constants::CLOSED_ISSUE_STATE }
]
end
shared_examples 'setting attributes based on the first record' do |attributes|
let(:parent_id) { attributes[:parent_id] }
let(:epic_state_id) { attributes[:epic_state_id] }
it 'sets epic attributes based on the first record' do
new_node = described_class.new(epic_id, fake_data)
expect(new_node.parent_id).to eq parent_id
end
end
it_behaves_like 'setting attributes based on the first record', { epic_state_id: Constants::OPENED_EPIC_STATE, parent_id: nil }
it_behaves_like 'setting attributes based on the first record', { epic_state_id: Constants::CLOSED_EPIC_STATE, parent_id: 2 }
end
describe '#assemble_issue_sums' do
subject { described_class.new(epic_id, fake_data) }
context 'an epic with no issues' do
let(:fake_data) do
[
{ iid: epic_iid, epic_state_id: Constants::OPENED_EPIC_STATE, issues_count: 0, issues_weight_sum: 0, parent_id: nil, issues_state_id: nil }
]
end
it 'does not create any sums' do
subject.assemble_issue_sums
expect(subject.direct_sums.count).to eq 0
end
end
context 'an epic with issues' do
context 'with a nonzero count but a zero weight' do
let(:fake_data) do
[
{ iid: epic_iid, epic_state_id: Constants::OPENED_EPIC_STATE, issues_count: 1, issues_weight_sum: 0, parent_id: nil, issues_state_id: Constants::OPENED_ISSUE_STATE }
]
end
it 'creates no sums for the weight if the issues have 0 weight' do
subject.assemble_issue_sums
expect(subject.direct_sums.count).to eq 1
expect(subject).to have_direct_sum(Constants::ISSUE_TYPE, Constants::COUNT, Constants::OPENED_ISSUE_STATE, 1)
end
end
context 'with a nonzero count and nonzero weight for a single state' do
let(:fake_data) do
[
{ iid: epic_iid, epic_state_id: Constants::OPENED_EPIC_STATE, issues_count: 1, issues_weight_sum: 2, parent_id: nil, issues_state_id: Constants::OPENED_ISSUE_STATE }
]
end
it 'creates two sums' do
subject.assemble_issue_sums
expect(subject.direct_sums.count).to eq 2
expect(subject).to have_direct_sum(Constants::ISSUE_TYPE, Constants::COUNT, Constants::OPENED_ISSUE_STATE, 1)
expect(subject).to have_direct_sum(Constants::ISSUE_TYPE, Constants::WEIGHT_SUM, Constants::OPENED_ISSUE_STATE, 2)
end
end
context 'with a nonzero count and nonzero weight for multiple states' do
let(:fake_data) do
[
{ iid: epic_iid, epic_state_id: Constants::OPENED_EPIC_STATE, issues_count: 1, issues_weight_sum: 2, parent_id: nil, issues_state_id: Constants::OPENED_ISSUE_STATE },
{ iid: epic_iid, epic_state_id: Constants::OPENED_EPIC_STATE, issues_count: 3, issues_weight_sum: 5, parent_id: nil, issues_state_id: Constants::CLOSED_ISSUE_STATE }
]
end
it 'creates two sums' do
subject.assemble_issue_sums
expect(subject.direct_sums.count).to eq 4
expect(subject).to have_direct_sum(Constants::ISSUE_TYPE, Constants::COUNT, Constants::OPENED_ISSUE_STATE, 1)
expect(subject).to have_direct_sum(Constants::ISSUE_TYPE, Constants::WEIGHT_SUM, Constants::OPENED_ISSUE_STATE, 2)
expect(subject).to have_direct_sum(Constants::ISSUE_TYPE, Constants::COUNT, Constants::CLOSED_ISSUE_STATE, 3)
expect(subject).to have_direct_sum(Constants::ISSUE_TYPE, Constants::WEIGHT_SUM, Constants::CLOSED_ISSUE_STATE, 5)
end
end
end
end
describe '#assemble_epic_sums' do
subject { described_class.new(epic_id, [{ parent_id: nil, epic_state_id: Constants::CLOSED_EPIC_STATE }]) }
context 'with a child epic' do
let(:child_epic_id) { 45 }
let!(:child_epic_node) { described_class.new(child_epic_id, [{ parent_id: epic_id, epic_state_id: Constants::CLOSED_EPIC_STATE }]) }
before do
subject.child_ids << child_epic_id
end
it 'adds up the number of the child epics' do
subject.assemble_epic_sums([child_epic_node])
expect(subject.direct_sums.count).to eq 1
expect(subject).to have_direct_sum(Constants::EPIC_TYPE, Constants::COUNT, Constants::CLOSED_EPIC_STATE, 1)
end
end
end
describe '#calculate_recursive_sums' do
subject { described_class.new(epic_id, [{ parent_id: nil, epic_state_id: Constants::CLOSED_EPIC_STATE }]) }
before do
allow(subject).to receive(:direct_sums).and_return(direct_sums)
end
context 'an epic with no child epics' do
let(:tree) do
{ epic_id => subject }
end
context 'with no child issues' do
let(:direct_sums) do
[]
end
it 'returns a SumTotal with no sums' do
result = subject.calculate_recursive_sums(tree)
expect(result.sums).not_to be_nil
expect(result.sums.count).to eq 0
end
end
context 'with an issue with 0 weight' do
let(:direct_sums) do
[Epics::EpicNode::Sum.new(Constants::COUNT, Constants::CLOSED_EPIC_STATE, Constants::ISSUE_TYPE, 1)]
end
it 'returns a SumTotal with only a weight sum' do
result = subject.calculate_recursive_sums(tree)
expect(result.sums).not_to be_nil
expect(result.sums.count).to eq 1
end
end
context 'with an issue with nonzero weight' do
let(:direct_sums) do
[
Epics::EpicNode::Sum.new(Constants::COUNT, Constants::CLOSED_EPIC_STATE, Constants::ISSUE_TYPE, 1),
Epics::EpicNode::Sum.new(Constants::WEIGHT_SUM, Constants::CLOSED_EPIC_STATE, Constants::ISSUE_TYPE, 2)
]
end
it 'returns a SumTotal with only a weight sum' do
result = subject.calculate_recursive_sums(tree)
expect(result.sums).not_to be_nil
expect(result.sums.count).to eq 2
end
end
end
context 'an epic with child epics' do
let(:child_epic_id) { 45 }
let(:tree) do
{ epic_id => subject, child_epic_id => child_epic_node }
end
let(:child_epic_node) { described_class.new(child_epic_id, [{ parent_id: epic_id, epic_state_id: Constants::CLOSED_EPIC_STATE }]) }
let(:direct_sums) do
[ # only one opened epic, the child
Epics::EpicNode::Sum.new(Constants::COUNT, Constants::OPENED_EPIC_STATE, Constants::EPIC_TYPE, 1)
]
end
before do
subject.child_ids << child_epic_id
allow(child_epic_node).to receive(:direct_sums).and_return(child_sums)
end
context 'with a child that has issues of nonzero weight' do
let(:child_sums) do
[ # 1 issue of weight 2
Epics::EpicNode::Sum.new(Constants::COUNT, Constants::OPENED_ISSUE_STATE, Constants::ISSUE_TYPE, 1),
Epics::EpicNode::Sum.new(Constants::WEIGHT_SUM, Constants::OPENED_ISSUE_STATE, Constants::ISSUE_TYPE, 2)
]
end
it 'returns the correct sum total' do
result = subject.calculate_recursive_sums(tree)
expect(result.sums).not_to be_nil
expect(result.sums.count).to eq 3
end
end
end
end
end
# frozen_string_literal: true
require 'spec_helper'
describe Epics::LazyEpicAggregate do
let(:query_ctx) do
{}
end
let(:epic_id) { 37 }
let(:epic_iid) { 18 }
let(:child_epic_id) { 38 }
describe '#initialize' do
it 'requires either :weight_sum or :count as a facet', :aggregate_failures do
expect { described_class.new(query_ctx, epic_id, :nonsense) }.to raise_error(ArgumentError, /Invalid aggregate facet/)
expect { described_class.new(query_ctx, epic_id, nil) }.to raise_error(ArgumentError, /No aggregate facet/)
expect { described_class.new(query_ctx, epic_id, "") }.to raise_error(ArgumentError, /No aggregate facet/)
end
context 'with valid facets :weight_sum or :count' do
specify 'as a symbol', :aggregate_failures do
[:weight_sum, :count].each do |valid_facet|
described_class.new(query_ctx, epic_id, valid_facet)
end
end
specify 'as a string', :aggregate_failures do
%w(weight_sum count).each do |valid_facet|
described_class.new(query_ctx, epic_id, valid_facet)
end
end
end
it 'adds the epic_id to lazy state' do
described_class.new(query_ctx, epic_id, :count)
expect(query_ctx[:lazy_epic_aggregate][:pending_ids]).to match [epic_id]
end
end
describe '#epic_aggregate' do
let(:single_record) do
{ iid: 6, issues_count: 4, issues_weight_sum: 9, parent_id: nil, issues_state_id: Constants::OPENED_ISSUE_STATE, epic_state_id: Constants::OPENED_EPIC_STATE }
end
let(:epic_info_node) { Epics::EpicNode.new(epic_id, [single_record] ) }
subject { described_class.new(query_ctx, epic_id, :count) }
before do
subject.instance_variable_set(:@lazy_state, fake_state)
end
context 'if the record has already been loaded' do
let(:fake_state) do
{ pending_ids: Set.new, tree: { epic_id => epic_info_node } }
end
it 'does not make the query again' do
expect(epic_info_node).to receive(:aggregate_object_by).with(subject.facet)
expect(Epics::BulkEpicAggregateLoader).not_to receive(:new)
subject.epic_aggregate
end
end
context 'if the record has not been loaded' do
let(:other_epic_id) { 39 }
let(:fake_state) do
{ pending_ids: Set.new([epic_id, child_epic_id]), tree: {} }
end
let(:fake_data) do
{
epic_id => [{ epic_state_id: Constants::OPENED_EPIC_STATE, issues_count: 2, issues_weight_sum: 5, parent_id: nil, issues_state_id: Constants::OPENED_ISSUE_STATE }],
child_epic_id => [{ epic_state_id: Constants::CLOSED_EPIC_STATE, issues_count: 4, issues_weight_sum: 17, parent_id: epic_id, issues_state_id: Constants::CLOSED_ISSUE_STATE }],
other_epic_id => [{ epic_state_id: Constants::OPENED_EPIC_STATE, issues_count: 0, issues_weight_sum: 0, parent_id: nil, issues_state_id: nil }] # represents an epic with no parent and no issues
}
end
before do
allow(Epics::EpicNode).to receive(:aggregate_object_by).and_call_original
expect_any_instance_of(Epics::BulkEpicAggregateLoader).to receive(:execute).and_return(fake_data)
end
it 'clears the pending IDs' do
subject.epic_aggregate
lazy_state = subject.instance_variable_get(:@lazy_state)
expect(lazy_state[:pending_ids]).to be_empty
end
it 'creates the parent-child associations', :aggregate_failures do
subject.epic_aggregate
lazy_state = subject.instance_variable_get(:@lazy_state)
tree = lazy_state[:tree]
expect(tree[child_epic_id].parent_id).to eq epic_id
expect(tree[epic_id].child_ids).to match_array([child_epic_id])
end
context 'for a parent-child relationship' do
it 'assembles direct sums', :aggregate_failures do
subject.epic_aggregate
lazy_state = subject.instance_variable_get(:@lazy_state)
tree = lazy_state[:tree]
expect(tree[epic_id]).to have_direct_sum(Constants::EPIC_TYPE, Constants::COUNT, Constants::CLOSED_EPIC_STATE, 1)
expect(tree[epic_id]).to have_direct_sum(Constants::ISSUE_TYPE, Constants::WEIGHT_SUM, Constants::OPENED_ISSUE_STATE, 5)
expect(tree[epic_id]).to have_direct_sum(Constants::EPIC_TYPE, Constants::COUNT, Constants::CLOSED_EPIC_STATE, 1)
expect(tree[child_epic_id]).to have_direct_sum(Constants::ISSUE_TYPE, Constants::COUNT, Constants::CLOSED_ISSUE_STATE, 4)
expect(tree[child_epic_id]).to have_direct_sum(Constants::ISSUE_TYPE, Constants::WEIGHT_SUM, Constants::CLOSED_ISSUE_STATE, 17)
end
it 'assembles recursive sums for the parent', :aggregate_failures do
subject.epic_aggregate
lazy_state = subject.instance_variable_get(:@lazy_state)
tree = lazy_state[:tree]
expect(tree[epic_id]).to have_aggregate(Constants::ISSUE_TYPE, Constants::COUNT, Constants::OPENED_ISSUE_STATE, 2)
expect(tree[epic_id]).to have_aggregate(Constants::ISSUE_TYPE, Constants::COUNT, Constants::CLOSED_ISSUE_STATE, 4)
expect(tree[epic_id]).to have_aggregate(Constants::ISSUE_TYPE, Constants::WEIGHT_SUM, Constants::OPENED_ISSUE_STATE, 5)
expect(tree[epic_id]).to have_aggregate(Constants::ISSUE_TYPE, Constants::WEIGHT_SUM, Constants::CLOSED_ISSUE_STATE, 17)
expect(tree[epic_id]).to have_aggregate(Constants::EPIC_TYPE, Constants::COUNT, Constants::CLOSED_EPIC_STATE, 1)
end
end
context 'for a standalone epic with no issues' do
it 'assembles direct sums', :aggregate_failures do
subject.epic_aggregate
lazy_state = subject.instance_variable_get(:@lazy_state)
tree = lazy_state[:tree]
expect(tree[other_epic_id].direct_sums).to be_empty
end
end
end
end
end
......@@ -7,50 +7,89 @@ describe 'Epic aggregates (count and weight)' do
let_it_be(:current_user) { create(:user) }
let_it_be(:group) { create(:group, :public) }
let_it_be(:subgroup) { create(:group, :private, parent: group)}
let_it_be(:subsubgroup) { create(:group, :private, parent: subgroup)}
let_it_be(:project) { create(:project, namespace: group) }
let_it_be(:parent_epic) { create(:epic, id: 1, group: group, title: 'parent epic') }
let_it_be(:epic_with_issues) { create(:epic, id: 2, group: subgroup, parent: parent_epic, title: 'epic with issues') }
let_it_be(:epic_without_issues) { create(:epic, :closed, id: 3, group: subgroup, parent: parent_epic, title: 'epic without issues') }
let_it_be(:closed_epic) { create(:epic, :closed, id: 4, group: subgroup, parent: parent_epic, title: 'closed epic') }
let(:query) do
graphql_query_for('group', { fullPath: group.full_path }, query_graphql_field('epics', { iid: parent_epic.iid }, epic_aggregates_query))
end
let_it_be(:issue1) { create(:issue, project: project, weight: 5, state: :opened) }
let_it_be(:issue2) { create(:issue, project: project, weight: 7, state: :closed) }
let_it_be(:epic_issue1) { create(:epic_issue, epic: epic_with_issues, issue: issue1) }
let_it_be(:epic_issue2) { create(:epic_issue, epic: epic_with_issues, issue: issue2) }
let(:epic_aggregates_query) do
<<~QUERY
nodes {
descendantWeightSum {
openedIssues
closedIssues
}
descendantCounts {
openedEpics
closedEpics
openedIssues
closedIssues
}
nodes {
descendantWeightSum {
openedIssues
closedIssues
}
descendantCounts {
openedEpics
closedEpics
openedIssues
closedIssues
}
}
QUERY
end
let(:query) do
graphql_query_for('group', { fullPath: group.full_path }, query_graphql_field('epics', { iid: parent_epic.iid }, epic_aggregates_query))
end
subject { graphql_data.dig('group', 'epics', 'nodes') }
before do
group.add_developer(current_user)
stub_licensed_features(epics: true)
post_graphql(query, current_user: current_user)
end
it_behaves_like 'a working graphql query'
context 'with feature flag enabled' do
before do
stub_feature_flags(unfiltered_epic_aggregates: true)
end
it 'returns a placeholder with -1 weights and does not error' do
post_graphql(query, current_user: current_user)
it 'returns the epic counts' do
epic_count_result = {
"openedEpics" => 1,
"closedEpics" => 2
}
actual_result = graphql_data.dig('group', 'epics', 'nodes').first
expected_result = {
"descendantWeightSum" => {
"openedIssues" => -1,
"closedIssues" => -1
}
is_expected.to include(
a_hash_including('descendantCounts' => a_hash_including(epic_count_result))
)
end
it 'returns the issue counts' do
issue_count_result = {
"openedIssues" => 1,
"closedIssues" => 1
}
expect(actual_result).to include expected_result
is_expected.to include(
a_hash_including('descendantCounts' => a_hash_including(issue_count_result))
)
end
it 'returns the weights' do
descendant_weight_result = {
"openedIssues" => 5,
"closedIssues" => 7
}
is_expected.to include(
a_hash_including('descendantWeightSum' => a_hash_including(descendant_weight_result))
)
end
end
......@@ -62,14 +101,14 @@ describe 'Epic aggregates (count and weight)' do
context 'when requesting counts' do
let(:epic_aggregates_query) do
<<~QUERY
nodes {
descendantCounts {
openedEpics
closedEpics
openedIssues
closedIssues
nodes {
descendantCounts {
openedEpics
closedEpics
openedIssues
closedIssues
}
}
}
QUERY
end
......@@ -83,12 +122,12 @@ describe 'Epic aggregates (count and weight)' do
context 'when requesting weights' do
let(:epic_aggregates_query) do
<<~QUERY
nodes {
descendantWeightSum {
openedIssues
closedIssues
nodes {
descendantWeightSum {
openedIssues
closedIssues
}
}
}
QUERY
end
......
# frozen_string_literal: true
require 'spec_helper'
describe Epics::BulkEpicAggregateLoader do
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(:closed_epic_state_id) { Epic.available_states[:closed] }
let_it_be(:opened_epic_state_id) { Epic.available_states[:opened] }
let_it_be(:group) { create(:group, :public) }
let_it_be(:subgroup) { create(:group, :private, parent: group)}
let_it_be(:project) { create(:project, namespace: group) }
let_it_be(:subproject) { create(:project, namespace: subgroup) }
let_it_be(:parent_epic) { create(:epic, group: group, title: 'parent epic') }
let_it_be(:epic_with_issues) { create(:epic, group: subgroup, parent: parent_epic, state: :opened, title: 'epic with issues') }
# closed, no issues
let_it_be(:epic_without_issues) { create(:epic, group: subgroup, parent: parent_epic, state: :closed, title: 'epic without issues') }
# open, public
let_it_be(:issue1) { create(:issue, project: project, weight: 1, state: :opened) }
let_it_be(:issue2) { create(:issue, project: project, weight: 1, state: :opened) }
# closed
let_it_be(:issue3) { create(:issue, project: project, weight: 1, state: :closed) }
let_it_be(:issue4) { create(:issue, project: project, weight: 1, state: :closed) }
# confidential
let_it_be(:issue5) { create(:issue, project: project, weight: 1, confidential: true, state: :opened) }
let_it_be(:issue6) { create(:issue, project: project, weight: 1, confidential: true, state: :opened) }
# in private project, private subgroup
let_it_be(:issue7) { create(:issue, project: subproject, weight: 1, state: :opened) }
let_it_be(:issue8) { create(:issue, project: subproject, weight: 1, state: :opened) }
# private project, confidential, private subgroup
let_it_be(:issue9) { create(:issue, project: subproject, weight: 1, confidential: true, state: :opened) }
let_it_be(:issue10) { create(:issue, project: subproject, weight: 1, confidential: true, state: :opened) }
# nil weight doesn't break it
let_it_be(:issue11) { create(:issue, project: project, weight: 0, state: :opened) }
let_it_be(:issue12) { create(:issue, project: project, weight: nil, state: :opened) }
let_it_be(:epic_issue1) { create(:epic_issue, epic: parent_epic, issue: issue1) }
let_it_be(:epic_issue2) { create(:epic_issue, epic: epic_with_issues, issue: issue2) }
let_it_be(:epic_issue3) { create(:epic_issue, epic: parent_epic, issue: issue3) }
let_it_be(:epic_issue4) { create(:epic_issue, epic: epic_with_issues, issue: issue4) }
let_it_be(:epic_issue5) { create(:epic_issue, epic: parent_epic, issue: issue5) }
let_it_be(:epic_issue6) { create(:epic_issue, epic: epic_with_issues, issue: issue6) }
let_it_be(:epic_issue7) { create(:epic_issue, epic: parent_epic, issue: issue7) }
let_it_be(:epic_issue8) { create(:epic_issue, epic: epic_with_issues, issue: issue8) }
let_it_be(:epic_issue9) { create(:epic_issue, epic: parent_epic, issue: issue9) }
let_it_be(:epic_issue10) { create(:epic_issue, epic: epic_with_issues, issue: issue10) }
let_it_be(:epic_issue11) { create(:epic_issue, epic: parent_epic, issue: issue11) }
let_it_be(:epic_issue12) { create(:epic_issue, epic: epic_with_issues, issue: issue12) }
subject { described_class.new(epic_ids: target_ids) }
before do
stub_licensed_features(epics: true)
end
context 'when epic ids with issues is provided' do
let(:target_ids) { parent_epic.id }
it 'sums all the weights, even confidential, or in private groups' do
expected_result = {
parent_epic.id => [
result_for(parent_epic, issues_state: opened_issue_state_id, issues_count: 5, issues_weight_sum: 4),
result_for(parent_epic, issues_state: closed_issue_state_id, issues_count: 1, issues_weight_sum: 1)
],
epic_with_issues.id => [
result_for(epic_with_issues, issues_state: opened_issue_state_id, issues_count: 5, issues_weight_sum: 4),
result_for(epic_with_issues, issues_state: closed_issue_state_id, issues_count: 1, issues_weight_sum: 1)
],
epic_without_issues.id => [
result_for(epic_without_issues, issues_state: nil, issues_count: 0, issues_weight_sum: 0)
]
}
result = subject.execute
expected_result.each do |epic_id, records|
expect(result[epic_id]).to match_array records
end
end
it 'contains results for all epics, even if they do not have issues' do
result = subject.execute
# epic_without_issues is included, even if it has none
expect(result.keys).to match_array([parent_epic.id, epic_with_issues.id, epic_without_issues.id])
end
context 'testing for a single database query' do
it 'does not repeat database queries for subepics' do
recorder = ActiveRecord::QueryRecorder.new { described_class.new(epic_ids: epic_with_issues.id).execute }
# this one has sub-epics, but there should still only be one query
expect { described_class.new(epic_ids: [parent_epic.id, epic_with_issues.id]).execute }.not_to exceed_query_limit(recorder)
end
it 'avoids N+1' do
recorder = ActiveRecord::QueryRecorder.new { described_class.new(epic_ids: epic_with_issues.id).execute }
expect { described_class.new(epic_ids: [epic_with_issues.id, parent_epic.id]).execute }.not_to exceed_query_limit(recorder)
end
end
end
context 'when an epic without issues is provided' do
let(:target_ids) { epic_without_issues.id }
it 'returns a placeholder' do
expected_result = [
result_for(epic_without_issues, issues_state: nil, issues_count: 0, issues_weight_sum: 0)
]
actual_result = subject.execute
expect(actual_result[epic_without_issues.id]).to match_array(expected_result)
end
end
context 'when no epic ids are provided' do
[nil, [], ""].each do |empty_arg|
let(:target_ids) { empty_arg }
it 'returns an empty set' do
expect(subject.execute).to eq({})
end
end
end
def result_for(epic, issues_state:, issues_count:, issues_weight_sum:)
{ iid: epic.iid, issues_count: issues_count, issues_weight_sum: issues_weight_sum, parent_id: epic.parent_id, issues_state_id: issues_state, epic_state_id: Epic.available_states[epic.state_id] }.stringify_keys
end
end
# frozen_string_literal: true
RSpec::Matchers.define :have_direct_sum do |type, facet, state, value|
# supports_block_expectations
match do |epic_node_result|
expect(epic_node_result).not_to be_nil
expect(epic_node_result.direct_sums).not_to be_empty
matching = epic_node_result.direct_sums.select { |sum| sum.type == type && sum.facet == facet && sum.state == state && sum.value == value }
expect(matching).not_to be_empty
end
failure_message do |epic_node_result|
if epic_node_result.nil?
"expected for there to be an epic node, but it is nil"
else
<<~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}.
Sums: #{epic_node_result.direct_sums.inspect}
FAILURE_MSG
end
end
end
RSpec::Matchers.define :have_aggregate do |type, facet, state, value|
supports_block_expectations
match do |epic_node_result|
aggregate_object = epic_node_result.aggregate_object_by(facet)
expect(aggregate_object.send(method_name(type, state))).to eq value
end
failure_message do |epic_node_result|
aggregate_object = epic_node_result.aggregate_object_by(facet)
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)}."
end
def method_name(type, state)
if type == Constants::ISSUE_TYPE
return :opened_issues if state == Constants::OPENED_ISSUE_STATE
:closed_issues
elsif type == Constants::EPIC_TYPE
return :opened_epics if state == Constants::OPENED_EPIC_STATE
:closed_epics
end
end
end
......@@ -276,3 +276,7 @@ Rugged::Settings['search_path_global'] = Rails.root.join('tmp/tests').to_s
# Disable timestamp checks for invisible_captcha
InvisibleCaptcha.timestamp_enabled = false
class Constants
include Epics::AggregateConstants
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