Commit 7360f88e authored by GitLab Bot's avatar GitLab Bot
Browse files

Add latest changes from gitlab-org/gitlab@master

parent 96571352592f
......@@ -39,8 +39,8 @@ def protected_ref_accessible_to?(ref, user, project:, action:, protected_refs: n
end
end
def developers_can?(action, ref)
access_levels_for_ref(ref, action: action).any? do |access_level|
def developers_can?(action, ref, protected_refs: nil)
access_levels_for_ref(ref, action: action, protected_refs: protected_refs).any? do |access_level|
access_level.access_level == Gitlab::Access::DEVELOPER
end
end
......
......@@ -663,6 +663,11 @@ def ci_pipelines
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
# when no namespace is given, all ancestors upto the top are returned
def ancestors_upto(top = nil, hierarchy_order: nil)
......
......@@ -38,7 +38,7 @@ def self.default_branch_protected?
end
def self.protected_refs(project)
project.protected_branches.select(:name)
project.protected_branches
end
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 @@ class Branches < Grape::API
use :filter_params
end
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
......
......@@ -489,11 +489,11 @@ class Branch < Grape::Entity
end
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
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
expose :can_push do |repo_branch, options|
......
......@@ -26,6 +26,8 @@ class GroupContainerRepositories < Grape::API
user: current_user, subject: user_group
).execute
track_event('list_repositories')
present paginate(repositories), with: Entities::ContainerRegistry::Repository, tags: params[:tags]
end
end
......
......@@ -455,6 +455,17 @@ def present_carrierwave_file!(file, supports_direct_download: true)
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
def project_finder_params_ce
......
......@@ -27,6 +27,8 @@ class ProjectContainerRepositories < Grape::API
user: current_user, subject: user_project
).execute
track_event( 'list_repositories')
present paginate(repositories), with: Entities::ContainerRegistry::Repository, tags: params[:tags]
end
......@@ -40,6 +42,7 @@ class ProjectContainerRepositories < Grape::API
authorize_admin_container_image!
DeleteContainerRepositoryWorker.perform_async(current_user.id, repository.id)
track_event('delete_repository')
status :accepted
end
......@@ -56,6 +59,8 @@ class ProjectContainerRepositories < Grape::API
authorize_read_container_image!
tags = Kaminari.paginate_array(repository.tags)
track_event('list_tags')
present paginate(tags), with: Entities::ContainerRegistry::Tag
end
......@@ -77,6 +82,8 @@ class ProjectContainerRepositories < Grape::API
CleanupContainerRepositoryWorker.perform_async(current_user.id, repository.id,
declared_params.except(:repository_id))
track_event('delete_tag_bulk')
status :accepted
end
......@@ -111,6 +118,8 @@ class ProjectContainerRepositories < Grape::API
.execute(repository)
if result[:status] == :success
track_event('delete_tag')
status :ok
else
status :bad_request
......
......@@ -11842,15 +11842,30 @@ msgstr ""
msgid "Package was removed"
msgstr ""
msgid "PackageRegistry|Delete Package"
msgstr ""
msgid "PackageRegistry|Delete Package Version"
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."
msgstr ""
msgid "PackageRegistry|Unable to load package"
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?"
msgstr ""
......
......@@ -174,4 +174,18 @@
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
......@@ -119,6 +119,25 @@ def check_merge_status(json_response)
it_behaves_like 'repository branches'
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
context 'when authenticated', 'as a guest' do
......
......@@ -44,6 +44,8 @@
let(:object) { group }
end
it_behaves_like 'a gitlab tracking event', described_class.name, 'list_repositories'
context 'with invalid group id' do
let(:url) { '/groups/123412341234/registry/repositories' }
......
......@@ -46,6 +46,7 @@
it_behaves_like 'rejected container repository access', :guest, :forbidden
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
let(:object) { project }
......@@ -57,6 +58,7 @@
it_behaves_like 'rejected container repository access', :developer, :forbidden
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
let(:api_user) { maintainer }
......@@ -85,6 +87,8 @@
stub_container_registry_tags(repository: root_repository.path, tags: %w(rootA latest))
end
it_behaves_like 'a gitlab tracking event', described_class.name, 'list_tags'
it 'returns a list of tags' do
subject
......@@ -111,6 +115,7 @@
it_behaves_like 'rejected container repository access', :developer, :forbidden
it_behaves_like 'rejected container repository access', :anonymous, :not_found
it_behaves_like 'a gitlab tracking event', described_class.name, 'delete_tag_bulk'
end
context 'for maintainer' do
......@@ -222,6 +227,7 @@
it 'properly removes tag' do
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(Gitlab::Tracking).to receive(:event).with(described_class.name, 'delete_tag', {})
subject
......@@ -237,6 +243,7 @@
it 'properly removes tag' do
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(Gitlab::Tracking).to receive(:event).with(described_class.name, 'delete_tag', {})
subject
......
......@@ -180,7 +180,7 @@ def ref
# pipeline. With the same ref this check is cached but if refs are
# different then there is an extra query per ref
# 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.cached_count).to eq(0)
......
......@@ -56,3 +56,11 @@
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