Commit 7801d133 authored by GitLab Bot's avatar GitLab Bot

Add latest changes from gitlab-org/gitlab@master

parent 60877d1b
...@@ -39,8 +39,8 @@ module ProtectedRef ...@@ -39,8 +39,8 @@ module ProtectedRef
end end
end end
def developers_can?(action, ref) def developers_can?(action, ref, protected_refs: nil)
access_levels_for_ref(ref, action: action).any? do |access_level| access_levels_for_ref(ref, action: action, protected_refs: protected_refs).any? do |access_level|
access_level.access_level == Gitlab::Access::DEVELOPER access_level.access_level == Gitlab::Access::DEVELOPER
end end
end end
......
...@@ -663,6 +663,11 @@ class Project < ApplicationRecord ...@@ -663,6 +663,11 @@ class Project < ApplicationRecord
end end
end end
def preload_protected_branches
preloader = ActiveRecord::Associations::Preloader.new
preloader.preload(self, protected_branches: [:push_access_levels, :merge_access_levels])
end
# returns all ancestor-groups upto but excluding the given namespace # returns all ancestor-groups upto but excluding the given namespace
# when no namespace is given, all ancestors upto the top are returned # when no namespace is given, all ancestors upto the top are returned
def ancestors_upto(top = nil, hierarchy_order: nil) def ancestors_upto(top = nil, hierarchy_order: nil)
......
...@@ -38,7 +38,7 @@ class ProtectedBranch < ApplicationRecord ...@@ -38,7 +38,7 @@ class ProtectedBranch < ApplicationRecord
end end
def self.protected_refs(project) def self.protected_refs(project)
project.protected_branches.select(:name) project.protected_branches
end end
def self.branch_requires_code_owner_approval?(project, branch_name) def self.branch_requires_code_owner_approval?(project, branch_name)
......
---
title: Remove N+1 DB calls from branches API
merge_request: 19661
author:
type: performance
---
title: Triggers the correct endpoint on licence approval
merge_request: 19078
author:
type: fixed
...@@ -32,7 +32,7 @@ module API ...@@ -32,7 +32,7 @@ module API
use :filter_params use :filter_params
end end
get ':id/repository/branches' do get ':id/repository/branches' do
Gitlab::QueryLimiting.whitelist('https://gitlab.com/gitlab-org/gitlab-foss/issues/42329') user_project.preload_protected_branches
repository = user_project.repository repository = user_project.repository
......
...@@ -489,11 +489,11 @@ module API ...@@ -489,11 +489,11 @@ module API
end end
expose :developers_can_push do |repo_branch, options| expose :developers_can_push do |repo_branch, options|
options[:project].protected_branches.developers_can?(:push, repo_branch.name) ::ProtectedBranch.developers_can?(:push, repo_branch.name, protected_refs: options[:project].protected_branches)
end end
expose :developers_can_merge do |repo_branch, options| expose :developers_can_merge do |repo_branch, options|
options[:project].protected_branches.developers_can?(:merge, repo_branch.name) ::ProtectedBranch.developers_can?(:merge, repo_branch.name, protected_refs: options[:project].protected_branches)
end end
expose :can_push do |repo_branch, options| expose :can_push do |repo_branch, options|
......
...@@ -26,6 +26,8 @@ module API ...@@ -26,6 +26,8 @@ module API
user: current_user, subject: user_group user: current_user, subject: user_group
).execute ).execute
track_event('list_repositories')
present paginate(repositories), with: Entities::ContainerRegistry::Repository, tags: params[:tags] present paginate(repositories), with: Entities::ContainerRegistry::Repository, tags: params[:tags]
end end
end end
......
...@@ -455,6 +455,17 @@ module API ...@@ -455,6 +455,17 @@ module API
end end
end end
def track_event(action = action_name, **args)
category = args.delete(:category) || self.options[:for].name
raise "invalid category" unless category
::Gitlab::Tracking.event(category, action.to_s, **args)
rescue => error
Rails.logger.warn( # rubocop:disable Gitlab/RailsLogger
"Tracking event failed for action: #{action}, category: #{category}, message: #{error.message}"
)
end
protected protected
def project_finder_params_ce def project_finder_params_ce
......
...@@ -27,6 +27,8 @@ module API ...@@ -27,6 +27,8 @@ module API
user: current_user, subject: user_project user: current_user, subject: user_project
).execute ).execute
track_event( 'list_repositories')
present paginate(repositories), with: Entities::ContainerRegistry::Repository, tags: params[:tags] present paginate(repositories), with: Entities::ContainerRegistry::Repository, tags: params[:tags]
end end
...@@ -40,6 +42,7 @@ module API ...@@ -40,6 +42,7 @@ module API
authorize_admin_container_image! authorize_admin_container_image!
DeleteContainerRepositoryWorker.perform_async(current_user.id, repository.id) DeleteContainerRepositoryWorker.perform_async(current_user.id, repository.id)
track_event('delete_repository')
status :accepted status :accepted
end end
...@@ -56,6 +59,8 @@ module API ...@@ -56,6 +59,8 @@ module API
authorize_read_container_image! authorize_read_container_image!
tags = Kaminari.paginate_array(repository.tags) tags = Kaminari.paginate_array(repository.tags)
track_event('list_tags')
present paginate(tags), with: Entities::ContainerRegistry::Tag present paginate(tags), with: Entities::ContainerRegistry::Tag
end end
...@@ -77,6 +82,8 @@ module API ...@@ -77,6 +82,8 @@ module API
CleanupContainerRepositoryWorker.perform_async(current_user.id, repository.id, CleanupContainerRepositoryWorker.perform_async(current_user.id, repository.id,
declared_params.except(:repository_id)) declared_params.except(:repository_id))
track_event('delete_tag_bulk')
status :accepted status :accepted
end end
...@@ -111,6 +118,8 @@ module API ...@@ -111,6 +118,8 @@ module API
.execute(repository) .execute(repository)
if result[:status] == :success if result[:status] == :success
track_event('delete_tag')
status :ok status :ok
else else
status :bad_request status :bad_request
......
...@@ -11842,15 +11842,30 @@ msgstr "" ...@@ -11842,15 +11842,30 @@ msgstr ""
msgid "Package was removed" msgid "Package was removed"
msgstr "" msgstr ""
msgid "PackageRegistry|Delete Package"
msgstr ""
msgid "PackageRegistry|Delete Package Version" msgid "PackageRegistry|Delete Package Version"
msgstr "" msgstr ""
msgid "PackageRegistry|Learn how to %{noPackagesLinkStart}publish and share your packages%{noPackagesLinkEnd} with GitLab."
msgstr ""
msgid "PackageRegistry|Remove package"
msgstr ""
msgid "PackageRegistry|There are no packages yet"
msgstr ""
msgid "PackageRegistry|There was a problem fetching the details for this package." msgid "PackageRegistry|There was a problem fetching the details for this package."
msgstr "" msgstr ""
msgid "PackageRegistry|Unable to load package" msgid "PackageRegistry|Unable to load package"
msgstr "" msgstr ""
msgid "PackageRegistry|You are about to delete <b>%{packageName}</b>, this operation is irreversible, are you sure?"
msgstr ""
msgid "PackageRegistry|You are about to delete version %{boldStart}%{version}%{boldEnd} of %{boldStart}%{name}%{boldEnd}. Are you sure?" msgid "PackageRegistry|You are about to delete version %{boldStart}%{version}%{boldEnd} of %{boldStart}%{name}%{boldEnd}. Are you sure?"
msgstr "" msgstr ""
......
...@@ -174,4 +174,18 @@ describe API::Helpers do ...@@ -174,4 +174,18 @@ describe API::Helpers do
end end
end end
end end
describe '#track_event' do
it "creates a gitlab tracking event" do
expect(Gitlab::Tracking).to receive(:event).with('foo', 'my_event', {})
subject.track_event('my_event', category: 'foo')
end
it "logs an exception" do
expect(Rails.logger).to receive(:warn).with(/Tracking event failed/)
subject.track_event('my_event', category: nil)
end
end
end end
...@@ -119,6 +119,25 @@ describe API::Branches do ...@@ -119,6 +119,25 @@ describe API::Branches do
it_behaves_like 'repository branches' it_behaves_like 'repository branches'
end end
it 'does not submit N+1 DB queries', :request_store do
create(:protected_branch, name: 'master', project: project)
# Make sure no setup step query is recorded.
get api(route, current_user), params: { per_page: 100 }
control = ActiveRecord::QueryRecorder.new do
get api(route, current_user), params: { per_page: 100 }
end
new_branch_name = 'protected-branch'
CreateBranchService.new(project, current_user).execute(new_branch_name, 'master')
create(:protected_branch, name: new_branch_name, project: project)
expect do
get api(route, current_user), params: { per_page: 100 }
end.not_to exceed_query_limit(control)
end
end end
context 'when authenticated', 'as a guest' do context 'when authenticated', 'as a guest' do
......
...@@ -44,6 +44,8 @@ describe API::GroupContainerRepositories do ...@@ -44,6 +44,8 @@ describe API::GroupContainerRepositories do
let(:object) { group } let(:object) { group }
end end
it_behaves_like 'a gitlab tracking event', described_class.name, 'list_repositories'
context 'with invalid group id' do context 'with invalid group id' do
let(:url) { '/groups/123412341234/registry/repositories' } let(:url) { '/groups/123412341234/registry/repositories' }
......
...@@ -46,6 +46,7 @@ describe API::ProjectContainerRepositories do ...@@ -46,6 +46,7 @@ describe API::ProjectContainerRepositories do
it_behaves_like 'rejected container repository access', :guest, :forbidden it_behaves_like 'rejected container repository access', :guest, :forbidden
it_behaves_like 'rejected container repository access', :anonymous, :not_found it_behaves_like 'rejected container repository access', :anonymous, :not_found
it_behaves_like 'a gitlab tracking event', described_class.name, 'list_repositories'
it_behaves_like 'returns repositories for allowed users', :reporter, 'project' do it_behaves_like 'returns repositories for allowed users', :reporter, 'project' do
let(:object) { project } let(:object) { project }
...@@ -57,6 +58,7 @@ describe API::ProjectContainerRepositories do ...@@ -57,6 +58,7 @@ describe API::ProjectContainerRepositories do
it_behaves_like 'rejected container repository access', :developer, :forbidden it_behaves_like 'rejected container repository access', :developer, :forbidden
it_behaves_like 'rejected container repository access', :anonymous, :not_found it_behaves_like 'rejected container repository access', :anonymous, :not_found
it_behaves_like 'a gitlab tracking event', described_class.name, 'delete_repository'
context 'for maintainer' do context 'for maintainer' do
let(:api_user) { maintainer } let(:api_user) { maintainer }
...@@ -85,6 +87,8 @@ describe API::ProjectContainerRepositories do ...@@ -85,6 +87,8 @@ describe API::ProjectContainerRepositories do
stub_container_registry_tags(repository: root_repository.path, tags: %w(rootA latest)) stub_container_registry_tags(repository: root_repository.path, tags: %w(rootA latest))
end end
it_behaves_like 'a gitlab tracking event', described_class.name, 'list_tags'
it 'returns a list of tags' do it 'returns a list of tags' do
subject subject
...@@ -111,6 +115,7 @@ describe API::ProjectContainerRepositories do ...@@ -111,6 +115,7 @@ describe API::ProjectContainerRepositories do
it_behaves_like 'rejected container repository access', :developer, :forbidden it_behaves_like 'rejected container repository access', :developer, :forbidden
it_behaves_like 'rejected container repository access', :anonymous, :not_found it_behaves_like 'rejected container repository access', :anonymous, :not_found
it_behaves_like 'a gitlab tracking event', described_class.name, 'delete_tag_bulk'
end end
context 'for maintainer' do context 'for maintainer' do
...@@ -222,6 +227,7 @@ describe API::ProjectContainerRepositories do ...@@ -222,6 +227,7 @@ describe API::ProjectContainerRepositories do
it 'properly removes tag' do it 'properly removes tag' do
expect(service).to receive(:execute).with(root_repository) { { status: :success } } expect(service).to receive(:execute).with(root_repository) { { status: :success } }
expect(Projects::ContainerRepository::DeleteTagsService).to receive(:new).with(root_repository.project, api_user, tags: %w[rootA]) { service } expect(Projects::ContainerRepository::DeleteTagsService).to receive(:new).with(root_repository.project, api_user, tags: %w[rootA]) { service }
expect(Gitlab::Tracking).to receive(:event).with(described_class.name, 'delete_tag', {})
subject subject
...@@ -237,6 +243,7 @@ describe API::ProjectContainerRepositories do ...@@ -237,6 +243,7 @@ describe API::ProjectContainerRepositories do
it 'properly removes tag' do it 'properly removes tag' do
expect(service).to receive(:execute).with(root_repository) { { status: :success } } expect(service).to receive(:execute).with(root_repository) { { status: :success } }
expect(Projects::ContainerRepository::DeleteTagsService).to receive(:new).with(root_repository.project, api_user, tags: %w[rootA]) { service } expect(Projects::ContainerRepository::DeleteTagsService).to receive(:new).with(root_repository.project, api_user, tags: %w[rootA]) { service }
expect(Gitlab::Tracking).to receive(:event).with(described_class.name, 'delete_tag', {})
subject subject
......
...@@ -180,7 +180,7 @@ describe PipelineSerializer do ...@@ -180,7 +180,7 @@ describe PipelineSerializer do
# pipeline. With the same ref this check is cached but if refs are # pipeline. With the same ref this check is cached but if refs are
# different then there is an extra query per ref # different then there is an extra query per ref
# https://gitlab.com/gitlab-org/gitlab-foss/issues/46368 # https://gitlab.com/gitlab-org/gitlab-foss/issues/46368
expected_queries = Gitlab.ee? ? 44 : 41 expected_queries = Gitlab.ee? ? 41 : 38
expect(recorded.count).to be_within(2).of(expected_queries) expect(recorded.count).to be_within(2).of(expected_queries)
expect(recorded.cached_count).to eq(0) expect(recorded.cached_count).to eq(0)
......
...@@ -56,3 +56,11 @@ shared_examples 'returns repositories for allowed users' do |user_type, scope| ...@@ -56,3 +56,11 @@ shared_examples 'returns repositories for allowed users' do |user_type, scope|
end end
end end
end end
shared_examples 'a gitlab tracking event' do |category, action|
it "creates a gitlab tracking event #{action}" do
expect(Gitlab::Tracking).to receive(:event).with(category, action, {})
subject
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