Commit c325698a authored by Felipe Artur's avatar Felipe Artur

Persist blocking issues count on database

Persist blocking issues count on database.
This will avoid slow queries when ordering issues by blocking counts.
parent 2e565228
# frozen_string_literal: true
class AddBlockingIssuesCountToIssues < ActiveRecord::Migration[6.0]
include Gitlab::Database::MigrationHelpers
DOWNTIME = false
INDEX_NAME = 'index_issue_on_project_id_state_id_and_blocking_issues_count'
disable_ddl_transaction!
def up
unless column_exists?(:issues, :blocking_issues_count)
with_lock_retries do
add_column :issues, :blocking_issues_count, :integer, default: 0, null: false
end
end
add_concurrent_index :issues, [:project_id, :state_id, :blocking_issues_count], name: INDEX_NAME
end
def down
remove_concurrent_index_by_name :issues, INDEX_NAME
with_lock_retries do
remove_column :issues, :blocking_issues_count
end
end
end
08566b4094de37395f3ebc3f70eedc9ed6052a78090cf296e66ce4ef167e6bce
\ No newline at end of file
......@@ -12527,6 +12527,7 @@ CREATE TABLE public.issues (
external_key character varying(255),
sprint_id bigint,
issue_type smallint DEFAULT 0 NOT NULL,
blocking_issues_count integer DEFAULT 0 NOT NULL,
CONSTRAINT check_fba63f706d CHECK ((lock_version IS NOT NULL))
);
......@@ -19697,6 +19698,8 @@ CREATE INDEX index_issue_metrics ON public.issue_metrics USING btree (issue_id);
CREATE INDEX index_issue_metrics_on_issue_id_and_timestamps ON public.issue_metrics USING btree (issue_id, first_mentioned_in_commit_at, first_associated_with_milestone_at, first_added_to_board_at);
CREATE INDEX index_issue_on_project_id_state_id_and_blocking_issues_count ON public.issues USING btree (project_id, state_id, blocking_issues_count);
CREATE INDEX index_issue_tracker_data_on_service_id ON public.issue_tracker_data USING btree (service_id);
CREATE UNIQUE INDEX index_issue_user_mentions_on_note_id ON public.issue_user_mentions USING btree (note_id) WHERE (note_id IS NOT NULL);
......
......@@ -210,6 +210,12 @@ module EE
end
end
def update_blocking_issues_count!
blocking_count = IssueLink.blocking_issues_count_for(self)
update!(blocking_issues_count: blocking_count)
end
private
def blocking_issues_ids
......
......@@ -18,6 +18,9 @@ class IssueLink < ApplicationRecord
TYPE_BLOCKS = 'blocks'
TYPE_IS_BLOCKED_BY = 'is_blocked_by'
after_create :refresh_blocking_issue_cache
after_destroy :refresh_blocking_issue_cache
enum link_type: { TYPE_RELATES_TO => 0, TYPE_BLOCKS => 1, TYPE_IS_BLOCKED_BY => 2 }
class << self
......@@ -43,6 +46,17 @@ class IssueLink < ApplicationRecord
private
def blocking_issue
case link_type
when TYPE_BLOCKS then source
when TYPE_IS_BLOCKED_BY then target
end
end
def refresh_blocking_issue_cache
blocking_issue&.update_blocking_issues_count!
end
class << self
def blocked_and_blocking_issues_union(issue_ids)
from_union([
......@@ -99,6 +113,10 @@ class IssueLink < ApplicationRecord
.group(:blocked_issue_id)
], remove_duplicates: false).select('blocked_issue_id, SUM(count) AS count').group('blocked_issue_id')
end
def blocking_issues_count_for(issue)
blocking_issues_for_collection(issue.id)[0]&.count.to_i
end
end
def check_self_relation
......
......@@ -51,6 +51,75 @@ RSpec.describe IssueLink do
end
end
context 'callbacks' do
let_it_be(:target) { create(:issue) }
let_it_be(:source) { create(:issue) }
describe '.after_create_commit' do
context 'with TYPE_BLOCKS relation' do
it 'updates blocking issues count' do
expect(source).to receive(:update_blocking_issues_count!)
expect(target).not_to receive(:update_blocking_issues_count!)
create(:issue_link, target: target, source: source, link_type: described_class::TYPE_BLOCKS)
end
end
context 'with TYPE_IS_BLOCKED_BY' do
it 'updates blocking issues count' do
expect(source).not_to receive(:update_blocking_issues_count!)
expect(target).to receive(:update_blocking_issues_count!)
create(:issue_link, target: target, source: source, link_type: described_class::TYPE_IS_BLOCKED_BY)
end
end
context 'with TYPE_RELATES_TO' do
it 'does not update blocking_issues_count' do
expect(source).not_to receive(:update_blocking_issues_count!)
expect(target).not_to receive(:update_blocking_issues_count!)
create(:issue_link, target: target, source: source, link_type: described_class::TYPE_RELATES_TO)
end
end
end
describe '.after_destroy_commit' do
context 'with TYPE_BLOCKS relation' do
it 'updates blocking issues count' do
link = create(:issue_link, target: target, source: source, link_type: described_class::TYPE_BLOCKS)
expect(source).to receive(:update_blocking_issues_count!)
expect(target).not_to receive(:update_blocking_issues_count!)
link.destroy!
end
end
context 'with TYPE_IS_BLOCKED_BY' do
it 'updates blocking issues count' do
link = create(:issue_link, target: target, source: source, link_type: described_class::TYPE_IS_BLOCKED_BY)
expect(source).not_to receive(:update_blocking_issues_count!)
expect(target).to receive(:update_blocking_issues_count!)
link.destroy!
end
end
context 'with TYPE_RELATES_TO' do
it 'does not update blocking_issues_count' do
link = create(:issue_link, target: target, source: source, link_type: described_class::TYPE_RELATES_TO)
expect(source).not_to receive(:update_blocking_issues_count!)
expect(target).not_to receive(:update_blocking_issues_count!)
link.destroy!
end
end
end
end
describe '.blocked_issue_ids' do
it 'returns only ids of issues which are blocked' do
link1 = create(:issue_link, link_type: described_class::TYPE_BLOCKS)
......@@ -85,7 +154,7 @@ RSpec.describe IssueLink do
end
end
describe 'collections' do
context 'blocking issues count' do
let_it_be(:blocked_issue_1) { create(:issue) }
let_it_be(:project) { blocked_issue_1.project }
let_it_be(:blocked_issue_2) { create(:issue, project: project) }
......@@ -117,6 +186,14 @@ RSpec.describe IssueLink do
expect(result_by(results, blocked_issue_3.id).count).to eq(1)
end
end
describe '.blocking_issues_count_for' do
it 'returns blocked issues count for single issue' do
blocking_count = described_class.blocking_issues_count_for(blocking_issue_1)
expect(blocking_count).to eq(2)
end
end
end
def result_by(results, id)
......
......@@ -737,5 +737,22 @@ RSpec.describe Issue do
expect(subject).to be_falsey
end
end
describe '#update_blocking_issues_count' do
it 'updates blocking issues count' do
issue = create(:issue, project: project)
blocked_issue_1 = create(:issue, project: project)
blocked_issue_2 = create(:issue, project: project)
blocked_issue_3 = create(:issue, project: project)
create(:issue_link, source: issue, target: blocked_issue_1, link_type: IssueLink::TYPE_BLOCKS)
create(:issue_link, source: blocked_issue_2, target: issue, link_type: IssueLink::TYPE_IS_BLOCKED_BY)
create(:issue_link, source: issue, target: blocked_issue_3, link_type: IssueLink::TYPE_BLOCKS)
# Set to 0 for proper testing, this is being set by IssueLink callbacks.
issue.update(blocking_issues_count: 0)
expect { issue.update_blocking_issues_count! }
.to change { issue.blocking_issues_count }.from(0).to(3)
end
end
end
end
......@@ -4,7 +4,7 @@ require 'spec_helper'
RSpec.describe IssueLinks::DestroyService do
describe '#execute' do
let(:project) { create(:project) }
let(:project) { create(:project_empty_repo) }
let(:user) { create(:user) }
subject { described_class.new(issue_link, user).execute }
......
......@@ -205,6 +205,7 @@ excluded_attributes:
- :state_id
- :duplicated_to_id
- :promoted_to_epic_id
- :blocking_issues_count
merge_request:
- :milestone_id
- :sprint_id
......
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