Commit 40af0ec7 authored by Thong Kuah's avatar Thong Kuah

Fix N+1 for group container repositories view

The two offennding things were pre-loading the namespace, and also
loading resources for destroy_path. We don't need destroy_path for group
view as it's read-only so skip that
parent e2ade320
...@@ -16,7 +16,7 @@ module Groups ...@@ -16,7 +16,7 @@ module Groups
render json: ContainerRepositoriesSerializer render json: ContainerRepositoriesSerializer
.new(current_user: current_user) .new(current_user: current_user)
.represent(@images) .represent_read_only(@images)
end end
end end
end end
......
...@@ -11,7 +11,7 @@ class ContainerRepository < ApplicationRecord ...@@ -11,7 +11,7 @@ class ContainerRepository < ApplicationRecord
delegate :client, to: :registry delegate :client, to: :registry
scope :ordered, -> { order(:name) } scope :ordered, -> { order(:name) }
scope :with_api_entity_associations, -> { preload(:project) } scope :with_api_entity_associations, -> { preload(project: [:route, { namespace: :route }]) }
# rubocop: disable CodeReuse/ServiceClass # rubocop: disable CodeReuse/ServiceClass
def registry def registry
......
...@@ -2,4 +2,8 @@ ...@@ -2,4 +2,8 @@
class ContainerRepositoriesSerializer < BaseSerializer class ContainerRepositoriesSerializer < BaseSerializer
entity ContainerRepositoryEntity entity ContainerRepositoryEntity
def represent_read_only(resource)
represent(resource, except: [:destroy_path])
end
end end
---
title: Fix N+1 for group container repositories view
merge_request: 18979
author:
type: performance
# frozen_string_literal: true
require 'spec_helper'
describe Groups::Registry::RepositoriesController do
let_it_be(:group, reload: true) { create(:group) }
let_it_be(:user) { create(:user) }
before do
stub_container_registry_config(enabled: true)
group.add_reporter(user)
login_as(user)
end
describe 'GET groups/:group_id/-/container_registries.json' do
it 'avoids N+1 queries' do
project = create(:project, group: group)
create(:container_repository, project: project)
endpoint = group_container_registries_path(group, format: :json)
control_count = ActiveRecord::QueryRecorder.new(skip_cached: false) { get(endpoint) }.count
create_list(:project, 2, group: group).each do |project|
create_list(:container_repository, 2, project: project)
end
expect { get(endpoint) }.not_to exceed_all_query_limit(control_count)
# sanity check that response is 200
expect(response).to have_http_status(200)
repositories = json_response
expect(repositories.count).to eq(5)
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