Commit e4ff86e0 authored by Lin Jen-Shin's avatar Lin Jen-Shin

Limit insights query in a subset of projects

We do this by passing :projects as a union query to the finder.
parent 398a7090
......@@ -51,14 +51,21 @@ module Routable
# Klass.where_full_path_in(%w{gitlab-org/gitlab-foss gitlab-org/gitlab})
#
# Returns an ActiveRecord::Relation.
def where_full_path_in(paths)
def where_full_path_in(paths, use_includes: true)
return none if paths.empty?
wheres = paths.map do |path|
"(LOWER(routes.path) = LOWER(#{connection.quote(path)}))"
end
includes(:route).where(wheres.join(' OR ')).references(:routes)
route =
if use_includes
includes(:route).references(:routes)
else
joins(:route)
end
route.where(wheres.join(' OR '))
end
end
......
......@@ -33,6 +33,7 @@ export const fetchChartData = ({ dispatch }, { endpoint, chart }) =>
.post(endpoint, {
query: chart.query,
chart_type: chart.type,
projects: chart.projects,
})
.then(({ data }) => dispatch('receiveChartDataSuccess', { chart, data }))
.catch(error => {
......
......@@ -51,6 +51,10 @@ module InsightsActions
@period_param ||= query_param[:group_by]
end
def projects_param
@projects_param ||= params[:projects] || {}
end
def collection_labels_param
@collection_labels_param ||= query_param[:collection_labels]
end
......@@ -77,7 +81,8 @@ module InsightsActions
def finder
@finder ||=
Gitlab::Insights::Finders::IssuableFinder
.new(insights_entity, current_user, query_param)
.new(insights_entity, current_user,
query: query_param, projects: projects_param)
end
def serializer
......
......@@ -20,10 +20,11 @@ module Gitlab
months: { default: 12 }
}.with_indifferent_access.freeze
def initialize(entity, current_user, opts)
def initialize(entity, current_user, query: {}, projects: {})
@entity = entity
@current_user = current_user
@opts = opts
@query = query
@projects = projects
end
# Returns an Active Record relation of issuables.
......@@ -31,18 +32,18 @@ module Gitlab
relation = finder
.new(current_user, finder_args)
.execute
relation = relation.preload(:labels) if opts.key?(:collection_labels) # rubocop:disable CodeReuse/ActiveRecord
relation = relation.preload(:labels) if query.key?(:collection_labels) # rubocop:disable CodeReuse/ActiveRecord
relation
end
def period_limit
@period_limit ||=
if opts.key?(:period_limit)
if query.key?(:period_limit)
begin
Integer(opts[:period_limit])
Integer(query[:period_limit])
rescue ArgumentError
raise InvalidPeriodLimitError, "Invalid `:period_limit` option: `#{opts[:period_limit]}`. Expected an integer!"
raise InvalidPeriodLimitError, "Invalid `:period_limit` option: `#{query[:period_limit]}`. Expected an integer!"
end
else
PERIODS.dig(period, :default)
......@@ -51,22 +52,23 @@ module Gitlab
private
attr_reader :entity, :current_user, :opts
attr_reader :entity, :current_user, :query, :projects
def finder
issuable_type = opts[:issuable_type]&.to_sym
issuable_type = query[:issuable_type]&.to_sym
FINDERS[issuable_type] ||
raise(InvalidIssuableTypeError, "Invalid `:issuable_type` option: `#{opts[:issuable_type]}`. Allowed values are #{FINDERS.keys}!")
raise(InvalidIssuableTypeError, "Invalid `:issuable_type` option: `#{query[:issuable_type]}`. Allowed values are #{FINDERS.keys}!")
end
def finder_args
{
include_subgroups: true,
state: opts[:issuable_state] || 'opened',
label_name: opts[:filter_labels],
state: query[:issuable_state] || 'opened',
label_name: query[:filter_labels],
sort: 'created_asc',
created_after: created_after_argument
created_after: created_after_argument,
projects: finder_projects
}.merge(entity_key => entity.id)
end
......@@ -82,18 +84,18 @@ module Gitlab
end
def created_after_argument
return unless opts.key?(:group_by)
return unless query.key?(:group_by)
Time.zone.now.advance(period => -period_limit)
end
def period
@period ||=
if opts.key?(:group_by)
period = opts[:group_by].to_s.pluralize.to_sym
if query.key?(:group_by)
period = query[:group_by].to_s.pluralize.to_sym
unless PERIODS.key?(period)
raise InvalidGroupByError, "Invalid `:group_by` option: `#{opts[:group_by]}`. Allowed values are #{PERIODS.keys}!"
raise InvalidGroupByError, "Invalid `:group_by` option: `#{query[:group_by]}`. Allowed values are #{PERIODS.keys}!"
end
period
......@@ -101,6 +103,35 @@ module Gitlab
:days
end
end
def finder_projects
return if projects.empty?
Project.from_union([finder_projects_ids, finder_projects_paths])
end
def finder_projects_ids
Project.id_in(finder_projects_options[:ids]).select(:id)
end
def finder_projects_paths
Project.where_full_path_in(
finder_projects_options[:paths], use_includes: false
).select(:id)
end
def finder_projects_options
@finder_projects_options ||= projects[:only]&.group_by do |item|
case item
when Integer
:ids
when String
:paths
else
:unknown
end
end || {}
end
end
end
end
......
......@@ -7,7 +7,7 @@ RSpec.describe Gitlab::Insights::Finders::IssuableFinder do
Timecop.freeze(Time.utc(2019, 3, 5)) { example.run }
end
let(:base_opts) do
let(:base_query) do
{
state: 'opened',
group_by: 'months'
......@@ -15,8 +15,8 @@ RSpec.describe Gitlab::Insights::Finders::IssuableFinder do
end
describe '#find' do
def find(entity, opts)
described_class.new(entity, nil, opts).find
def find(entity, query)
described_class.new(entity, nil, query: query).find
end
it 'raises an error for an invalid :issuable_type option' do
......@@ -32,7 +32,7 @@ RSpec.describe Gitlab::Insights::Finders::IssuableFinder do
end
it 'defaults to the "days" period if no :group_by is given' do
expect(described_class.new(build(:project), nil, issuable_type: 'issue').__send__(:period)).to eq(:days)
expect(described_class.new(build(:project), nil, query: { issuable_type: 'issue' }).__send__(:period)).to eq(:days)
end
it 'raises an error for an invalid :period_limit option' do
......@@ -51,26 +51,26 @@ RSpec.describe Gitlab::Insights::Finders::IssuableFinder do
let!(:issuable2) { create(:"labeled_#{issuable_type}", :opened, created_at: Time.utc(2019, 2, 6), labels: [label_bug, label_plan], project_association_key => project, **extra_issuable_attrs[2]) }
let!(:issuable3) { create(:"labeled_#{issuable_type}", :opened, created_at: Time.utc(2019, 2, 20), labels: [label_bug, label_create], project_association_key => project, **extra_issuable_attrs[3]) }
let!(:issuable4) { create(:"labeled_#{issuable_type}", :opened, created_at: Time.utc(2019, 3, 5), labels: [label_bug, label_quality], project_association_key => project, **extra_issuable_attrs[4]) }
let(:opts) do
base_opts.merge(
let(:query) do
base_query.merge(
issuable_type: issuable_type,
filter_labels: [label_bug.title],
collection_labels: [label_manage.title, label_plan.title, label_create.title])
end
subject { find(entity, opts) }
subject { find(entity, query) }
it 'avoids N + 1 queries' do
control_queries = ActiveRecord::QueryRecorder.new { subject.map { |issuable| issuable.labels.map(&:title) } }
create(:"labeled_#{issuable_type}", :opened, created_at: Time.utc(2019, 3, 5), labels: [label_bug], project_association_key => project, **extra_issuable_attrs[5])
expect { find(entity, opts).map { |issuable| issuable.labels.map(&:title) } }.not_to exceed_query_limit(control_queries)
expect { find(entity, query).map { |issuable| issuable.labels.map(&:title) } }.not_to exceed_query_limit(control_queries)
end
context ':period_limit option' do
context 'with group_by: "day"' do
before do
opts.merge!(group_by: 'day')
query.merge!(group_by: 'day')
end
it 'returns issuable created after 30 days ago' do
......@@ -80,7 +80,7 @@ RSpec.describe Gitlab::Insights::Finders::IssuableFinder do
context 'with group_by: "day", period_limit: 1' do
before do
opts.merge!(group_by: 'day', period_limit: 1)
query.merge!(group_by: 'day', period_limit: 1)
end
it 'returns issuable created after one day ago' do
......@@ -90,7 +90,7 @@ RSpec.describe Gitlab::Insights::Finders::IssuableFinder do
context 'with group_by: "week"' do
before do
opts.merge!(group_by: 'week')
query.merge!(group_by: 'week')
end
it 'returns issuable created after 12 weeks ago' do
......@@ -100,7 +100,7 @@ RSpec.describe Gitlab::Insights::Finders::IssuableFinder do
context 'with group_by: "week", period_limit: 1' do
before do
opts.merge!(group_by: 'week', period_limit: 1)
query.merge!(group_by: 'week', period_limit: 1)
end
it 'returns issuable created after one week ago' do
......@@ -110,7 +110,7 @@ RSpec.describe Gitlab::Insights::Finders::IssuableFinder do
context 'with group_by: "month"' do
before do
opts.merge!(group_by: 'month')
query.merge!(group_by: 'month')
end
it 'returns issuable created after 12 months ago' do
......@@ -120,7 +120,7 @@ RSpec.describe Gitlab::Insights::Finders::IssuableFinder do
context 'with group_by: "month", period_limit: 1' do
before do
opts.merge!(group_by: 'month', period_limit: 1)
query.merge!(group_by: 'month', period_limit: 1)
end
it 'returns issuable created after one month ago' do
......@@ -205,11 +205,11 @@ RSpec.describe Gitlab::Insights::Finders::IssuableFinder do
end
describe '#period_limit' do
subject { described_class.new(create(:project, :public), nil, opts).period_limit }
subject { described_class.new(create(:project, :public), nil, query: query).period_limit }
describe 'default values' do
context 'with group_by: "day"' do
let(:opts) { base_opts.merge!(group_by: 'day') }
let(:query) { base_query.merge!(group_by: 'day') }
it 'returns 30' do
expect(subject).to eq(30)
......@@ -217,7 +217,7 @@ RSpec.describe Gitlab::Insights::Finders::IssuableFinder do
end
context 'with group_by: "week"' do
let(:opts) { base_opts.merge!(group_by: 'week') }
let(:query) { base_query.merge!(group_by: 'week') }
it 'returns 12' do
expect(subject).to eq(12)
......@@ -225,7 +225,7 @@ RSpec.describe Gitlab::Insights::Finders::IssuableFinder do
end
context 'with group_by: "month"' do
let(:opts) { base_opts.merge!(group_by: 'month') }
let(:query) { base_query.merge!(group_by: 'month') }
it 'returns 12' do
expect(subject).to eq(12)
......@@ -235,7 +235,7 @@ RSpec.describe Gitlab::Insights::Finders::IssuableFinder do
describe 'custom values' do
context 'with period_limit: 42' do
let(:opts) { base_opts.merge!(period_limit: 42) }
let(:query) { base_query.merge!(period_limit: 42) }
it 'returns 42' do
expect(subject).to eq(42)
......@@ -243,7 +243,7 @@ RSpec.describe Gitlab::Insights::Finders::IssuableFinder do
end
context 'with an invalid period_limit' do
let(:opts) { base_opts.merge!(period_limit: 'many') }
let(:query) { base_query.merge!(period_limit: 'many') }
it 'raises an error' do
expect { subject }.to raise_error(described_class::InvalidPeriodLimitError, "Invalid `:period_limit` option: `many`. Expected an integer!")
......
......@@ -5,15 +5,15 @@ require 'spec_helper'
RSpec.describe Gitlab::Insights::Reducers::CountPerLabelReducer do
include_context 'Insights reducers context'
def find_issuables(project, opts)
Gitlab::Insights::Finders::IssuableFinder.new(project, nil, opts).find
def find_issuables(project, query)
Gitlab::Insights::Finders::IssuableFinder.new(project, nil, query: query).find
end
def reduce(issuable_relation, labels)
described_class.reduce(issuable_relation, labels: labels)
end
let(:opts) do
let(:query) do
{
state: 'opened',
issuable_type: 'issue',
......@@ -23,9 +23,9 @@ RSpec.describe Gitlab::Insights::Reducers::CountPerLabelReducer do
period_limit: 5
}
end
let(:issuable_relation) { find_issuables(project, opts) }
let(:issuable_relation) { find_issuables(project, query) }
subject { reduce(issuable_relation, opts[:collection_labels]) }
subject { reduce(issuable_relation, query[:collection_labels]) }
let(:expected) do
{
......@@ -47,6 +47,6 @@ RSpec.describe Gitlab::Insights::Reducers::CountPerLabelReducer do
control_queries = ActiveRecord::QueryRecorder.new { subject }
create(:labeled_issue, :opened, labels: [label_bug], project: project)
expect { reduce(find_issuables(project, opts), opts[:collection_labels]) }.not_to exceed_query_limit(control_queries)
expect { reduce(find_issuables(project, query), query[:collection_labels]) }.not_to exceed_query_limit(control_queries)
end
end
......@@ -5,15 +5,15 @@ require 'spec_helper'
RSpec.describe Gitlab::Insights::Reducers::CountPerPeriodReducer do
include_context 'Insights reducers context'
def find_issuables(project, opts)
Gitlab::Insights::Finders::IssuableFinder.new(project, nil, opts).find
def find_issuables(project, query)
Gitlab::Insights::Finders::IssuableFinder.new(project, nil, query: query).find
end
def reduce(issuable_relation, period, period_limit = 5, period_field = :created_at)
described_class.reduce(issuable_relation, period: period, period_limit: period_limit, period_field: period_field)
end
let(:opts) do
let(:query) do
{
state: 'opened',
issuable_type: 'issue',
......@@ -22,9 +22,9 @@ RSpec.describe Gitlab::Insights::Reducers::CountPerPeriodReducer do
period_limit: 5
}
end
let(:issuable_relation) { find_issuables(project, opts) }
let(:issuable_relation) { find_issuables(project, query) }
subject { reduce(issuable_relation, opts[:group_by]) }
subject { reduce(issuable_relation, query[:group_by]) }
let(:expected) do
{
......@@ -56,6 +56,6 @@ RSpec.describe Gitlab::Insights::Reducers::CountPerPeriodReducer do
control_queries = ActiveRecord::QueryRecorder.new { subject }
create(:labeled_issue, :opened, created_at: Time.utc(2019, 2, 5), labels: [label_bug], project: project)
expect { reduce(find_issuables(project, opts), opts[:group_by]) }.not_to exceed_query_limit(control_queries)
expect { reduce(find_issuables(project, query), query[:group_by]) }.not_to exceed_query_limit(control_queries)
end
end
......@@ -5,15 +5,15 @@ require 'spec_helper'
RSpec.describe Gitlab::Insights::Reducers::LabelCountPerPeriodReducer do
include_context 'Insights reducers context'
def find_issuables(project, opts)
Gitlab::Insights::Finders::IssuableFinder.new(project, nil, opts).find
def find_issuables(project, query)
Gitlab::Insights::Finders::IssuableFinder.new(project, nil, query: query).find
end
def reduce(issuable_relation, period, labels)
described_class.reduce(issuable_relation, period: period, period_limit: 5, labels: labels)
end
let(:opts) do
let(:query) do
{
state: 'opened',
issuable_type: 'issue',
......@@ -23,9 +23,9 @@ RSpec.describe Gitlab::Insights::Reducers::LabelCountPerPeriodReducer do
period_limit: 5
}
end
let(:issuable_relation) { find_issuables(project, opts) }
let(:issuable_relation) { find_issuables(project, query) }
subject { reduce(issuable_relation, opts[:group_by], opts[:collection_labels]) }
subject { reduce(issuable_relation, query[:group_by], query[:collection_labels]) }
let(:expected) do
{
......@@ -65,6 +65,6 @@ RSpec.describe Gitlab::Insights::Reducers::LabelCountPerPeriodReducer do
control_queries = ActiveRecord::QueryRecorder.new { subject }
create(:labeled_issue, :opened, created_at: Time.utc(2019, 2, 5), labels: [label_bug], project: project)
expect { reduce(find_issuables(project, opts), opts[:group_by], opts[:collection_labels]) }.not_to exceed_query_limit(control_queries)
expect { reduce(find_issuables(project, query), query[:group_by], query[:collection_labels]) }.not_to exceed_query_limit(control_queries)
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