From 208b18c956b38103b98bc93a952dc1e8a200dead Mon Sep 17 00:00:00 2001 From: Paco Guzman Date: Mon, 20 Jun 2016 15:38:54 +0200 Subject: [PATCH 1/3] Unify check branch name exist --- app/controllers/projects/application_controller.rb | 2 +- app/helpers/application_helper.rb | 2 +- app/helpers/branches_helper.rb | 2 +- app/models/repository.rb | 7 ++----- app/models/user.rb | 3 +-- spec/workers/merge_worker_spec.rb | 2 +- 6 files changed, 7 insertions(+), 11 deletions(-) diff --git a/app/controllers/projects/application_controller.rb b/app/controllers/projects/application_controller.rb index 776ba92c9ab..996909a28c6 100644 --- a/app/controllers/projects/application_controller.rb +++ b/app/controllers/projects/application_controller.rb @@ -74,7 +74,7 @@ class Projects::ApplicationController < ApplicationController end def require_branch_head - unless @repository.branch_names.include?(@ref) + unless @repository.branch_exists?(@ref) redirect_to( namespace_project_tree_path(@project.namespace, @project, @ref), notice: "This action is not allowed unless you are on a branch" diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index 82421d74de9..41859841834 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -116,7 +116,7 @@ module ApplicationHelper return false if project.merge_requests.where(source_branch: event.branch_name).opened.any? # Skip if user removed branch right after that - return false unless project.repository.branch_names.include?(event.branch_name) + return false unless project.repository.branch_exists?(event.branch_name) true end diff --git a/app/helpers/branches_helper.rb b/app/helpers/branches_helper.rb index 3ee3fc74f0c..c533659b600 100644 --- a/app/helpers/branches_helper.rb +++ b/app/helpers/branches_helper.rb @@ -10,7 +10,7 @@ module BranchesHelper end def can_push_branch?(project, branch_name) - return false unless project.repository.branch_names.include?(branch_name) + return false unless project.repository.branch_exists?(branch_name) ::Gitlab::GitAccess.new(current_user, project).can_push_to_branch?(branch_name) end diff --git a/app/models/repository.rb b/app/models/repository.rb index bbd7682d8e7..e9de6418fe0 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -192,7 +192,7 @@ class Repository end def branch_names - cache.fetch(:branch_names) { branches.map(&:name) } + @branch_names ||= cache.fetch(:branch_names) { branches.map(&:name) } end def branch_exists?(branch_name) @@ -267,6 +267,7 @@ class Repository def expire_branches_cache cache.expire(:branch_names) + @branch_names = nil @local_branches = nil end @@ -332,10 +333,6 @@ class Repository @lookup_cache ||= {} end - def expire_branch_names - cache.expire(:branch_names) - end - def expire_avatar_cache(branch_name = nil, revision = nil) # Avatars are pulled from the default branch, thus if somebody pushes to a # different branch there's no need to expire anything. diff --git a/app/models/user.rb b/app/models/user.rb index 2e458329cb9..876ccc69d8d 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -487,9 +487,8 @@ class User < ActiveRecord::Base events.recent.find do |event| project = Project.find_by_id(event.project_id) next unless project - repo = project.repository - if repo.branch_names.include?(event.branch_name) + if project.repository.branch_exists?(event.branch_name) merge_requests = MergeRequest.where("created_at >= ?", event.created_at). where(source_project_id: project.id, source_branch: event.branch_name) diff --git a/spec/workers/merge_worker_spec.rb b/spec/workers/merge_worker_spec.rb index 1abd87d7d33..b5e1fdb8ded 100644 --- a/spec/workers/merge_worker_spec.rb +++ b/spec/workers/merge_worker_spec.rb @@ -9,7 +9,7 @@ describe MergeWorker do before do source_project.team << [author, :master] - source_project.repository.expire_branch_names + source_project.repository.expire_branches_cache end it 'clears cache of source repo after removing source branch' do From df6639070911abd14d76d53891682270473e9bf3 Mon Sep 17 00:00:00 2001 From: Paco Guzman Date: Mon, 20 Jun 2016 16:52:10 +0200 Subject: [PATCH 2/3] Get ref_names from branch_names/tag_names cached --- CHANGELOG | 1 + app/models/repository.rb | 4 ++++ 2 files changed, 5 insertions(+) diff --git a/CHANGELOG b/CHANGELOG index 0e19ae06715..fbb545cf2f3 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -133,6 +133,7 @@ v 8.9.0 (unreleased) - Use Git cached counters for branches and tags on project page - Filter parameters for request_uri value on instrumented transactions. - Remove duplicated keys add UNIQUE index to keys fingerprint column + - ExtractsPath get ref_names from repository cache, if not there access git. - Cache user todo counts from TodoService - Ensure Todos counters doesn't count Todos for projects pending delete diff --git a/app/models/repository.rb b/app/models/repository.rb index e9de6418fe0..221c87164ca 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -191,6 +191,10 @@ class Repository end end + def ref_names + branch_names + tag_names + end + def branch_names @branch_names ||= cache.fetch(:branch_names) { branches.map(&:name) } end From 79c521f5df560ca6a8bd379dc211ccc1cf3ee3c4 Mon Sep 17 00:00:00 2001 From: Paco Guzman Date: Mon, 20 Jun 2016 21:41:46 +0200 Subject: [PATCH 3/3] Provide default branch/file path for ProjectsController#show --- app/controllers/projects_controller.rb | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb index 78ceaf3237f..2b1f50fd01e 100644 --- a/app/controllers/projects_controller.rb +++ b/app/controllers/projects_controller.rb @@ -303,8 +303,14 @@ class ProjectsController < Projects::ApplicationController project.repository_exists? && !project.empty_repo? end - # Override get_id from ExtractsPath, which returns the branch and file path + # Override extract_ref from ExtractsPath, which returns the branch and file path # for the blob/tree, which in this case is just the root of the default branch. + # This way we avoid to access the repository.ref_names. + def extract_ref(_id) + [get_id, ''] + end + + # Override get_id from ExtractsPath in this case is just the root of the default branch. def get_id project.repository.root_ref end