Commit 0e9fc90f authored by Alex Kalderimis's avatar Alex Kalderimis

Ensure that we do not issue unnecessary queries

This avoids issuing queries that can be cached, i.e for projects and
their route information. Most of the caching is handled with default
Rails SQL caching, but these tests verify that the behaviour is correct.
parent 3630a070
...@@ -36,9 +36,7 @@ module Types ...@@ -36,9 +36,7 @@ module Types
def image(parent:) def image(parent:)
sha = cached_stateful_version(parent).sha sha = cached_stateful_version(parent).sha
GitlabSchema.after_lazy(project) do |proj| ::Gitlab::Routing.url_helpers.project_design_url(design.project, design, sha)
::Gitlab::Routing.url_helpers.project_design_url(proj, design, sha)
end
end end
def event(parent:) def event(parent:)
......
...@@ -8,8 +8,11 @@ describe ::Types::DesignManagement::DesignAtVersionType.to_graphql do ...@@ -8,8 +8,11 @@ describe ::Types::DesignManagement::DesignAtVersionType.to_graphql do
it_behaves_like 'a GraphQL type with design fields' do it_behaves_like 'a GraphQL type with design fields' do
let(:extra_design_fields) { %i[version design] } let(:extra_design_fields) { %i[version design] }
let_it_be(:design) { create(:design, :with_versions) } let_it_be(:design) { create(:design, :with_versions) }
let(:object) { create(:design_at_version, design: design, version: version) } let(:object_id) do
let(:version) { design.versions.first } version = design.versions.first
GitlabSchema.id_from_object(create(:design_at_version, design: design, version: version))
end
let_it_be(:object_id_b) { GitlabSchema.id_from_object(create(:design_at_version)) }
let(:object_type) { ::Types::DesignManagement::DesignAtVersionType } let(:object_type) { ::Types::DesignManagement::DesignAtVersionType }
end end
end end
...@@ -5,8 +5,9 @@ require 'spec_helper' ...@@ -5,8 +5,9 @@ require 'spec_helper'
describe GitlabSchema.types['Design'] do describe GitlabSchema.types['Design'] do
it_behaves_like 'a GraphQL type with design fields' do it_behaves_like 'a GraphQL type with design fields' do
let(:extra_design_fields) { %i[notes discussions versions] } let(:extra_design_fields) { %i[notes discussions versions] }
let_it_be(:object) { create(:design, :with_versions) } let_it_be(:design) { create(:design, :with_versions) }
let(:object_id) { GitlabSchema.id_from_object(design) }
let_it_be(:object_id_b) { GitlabSchema.id_from_object(create(:design, :with_versions)) }
let(:object_type) { ::Types::DesignManagement::DesignType } let(:object_type) { ::Types::DesignManagement::DesignType }
let(:design) { object }
end end
end end
...@@ -30,25 +30,50 @@ shared_examples 'a GraphQL type with design fields' do ...@@ -30,25 +30,50 @@ shared_examples 'a GraphQL type with design fields' do
let(:context) { double('Context', schema: schema, query: query, parent: nil) } let(:context) { double('Context', schema: schema, query: query, parent: nil) }
let(:field) { described_class.fields['image'] } let(:field) { described_class.fields['image'] }
let(:args) { GraphQL::Query::Arguments::NO_ARGS } let(:args) { GraphQL::Query::Arguments::NO_ARGS }
let(:instance) { object_type.authorized_new(object, query.context) } let(:instance) do
object = GitlabSchema.sync_lazy(GitlabSchema.object_from_id(object_id))
object_type.authorized_new(object, query.context)
end
let(:instance_b) do
object_b = GitlabSchema.sync_lazy(GitlabSchema.object_from_id(object_id_b))
object_type.authorized_new(object_b, query.context)
end
it 'resolves to the design image URL' do it 'resolves to the design image URL' do
image = field.resolve(instance, args, context).value image = field.resolve(instance, args, context)
sha = design.versions.first.sha sha = design.versions.first.sha
url = ::Gitlab::Routing.url_helpers.project_design_url(design.project, design, sha) url = ::Gitlab::Routing.url_helpers.project_design_url(design.project, design, sha)
expect(image).to eq(url) expect(image).to eq(url)
end end
it 'has better than O(N) peformance' do it 'has better than O(N) peformance', :request_store do
baseline = ActiveRecord::QueryRecorder.new { field.resolve(instance, args, context).value } # Assuming designs have been loaded (as they must be), the following
baseline.count # queries are required:
# For each distinct version:
# - design_management_versions
# (Request store is needed so that each version is fetched only once.)
# For each distinct issue
# - issues
# For each distinct project
# - projects
# - routes
# - namespaces
# Here that total is:
# - 2 x issues
# - 2 x versions
# - 2 x (projects + routes + namespaces)
# = 10
expect(instance).not_to eq(instance_b) # preload designs themselves.
expect do expect do
image_a = field.resolve(instance, args, context) image_a = field.resolve(instance, args, context)
image_b = field.resolve(instance, args, context) image_b = field.resolve(instance, args, context)
expect(image_a.value).to eq(image_b.value) image_c = field.resolve(instance_b, args, context)
end.not_to exceed_query_limit(baseline) image_d = field.resolve(instance_b, args, context)
expect(image_a).to eq(image_b)
expect(image_c).not_to eq(image_b)
expect(image_c).to eq(image_d)
end.not_to exceed_query_limit(10)
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