summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--app/controllers/projects/merge_requests/application_controller.rb7
-rw-r--r--app/controllers/projects/merge_requests/creations_controller.rb1
-rw-r--r--app/controllers/projects/merge_requests_controller.rb2
-rw-r--r--app/models/merge_request.rb27
-rw-r--r--app/models/repository.rb4
-rw-r--r--config/initializers/8_metrics.rb3
-rw-r--r--db/migrate/20171101134435_remove_ref_fetched_from_merge_requests.rb14
-rw-r--r--db/schema.rb3
-rw-r--r--lib/github/import.rb1
-rw-r--r--lib/gitlab/git/repository.rb2
-rw-r--r--spec/lib/gitlab/git/repository_spec.rb10
-rw-r--r--spec/lib/gitlab/import_export/fork_spec.rb2
-rw-r--r--spec/models/merge_request_spec.rb37
13 files changed, 33 insertions, 80 deletions
diff --git a/app/controllers/projects/merge_requests/application_controller.rb b/app/controllers/projects/merge_requests/application_controller.rb
index 0e71977a58a..1269759fc2b 100644
--- a/app/controllers/projects/merge_requests/application_controller.rb
+++ b/app/controllers/projects/merge_requests/application_controller.rb
@@ -2,7 +2,6 @@ class Projects::MergeRequests::ApplicationController < Projects::ApplicationCont
before_action :check_merge_requests_available!
before_action :merge_request
before_action :authorize_read_merge_request!
- before_action :ensure_ref_fetched
private
@@ -10,12 +9,6 @@ class Projects::MergeRequests::ApplicationController < Projects::ApplicationCont
@issuable = @merge_request ||= @project.merge_requests.find_by!(iid: params[:id])
end
- # Make sure merge requests created before 8.0
- # have head file in refs/merge-requests/
- def ensure_ref_fetched
- @merge_request.ensure_ref_fetched if Gitlab::Database.read_write?
- end
-
def merge_request_params
params.require(:merge_request).permit(merge_request_params_attributes)
end
diff --git a/app/controllers/projects/merge_requests/creations_controller.rb b/app/controllers/projects/merge_requests/creations_controller.rb
index 99dc3dda9e7..129682f64aa 100644
--- a/app/controllers/projects/merge_requests/creations_controller.rb
+++ b/app/controllers/projects/merge_requests/creations_controller.rb
@@ -4,7 +4,6 @@ class Projects::MergeRequests::CreationsController < Projects::MergeRequests::Ap
include RendersCommits
skip_before_action :merge_request
- skip_before_action :ensure_ref_fetched
before_action :authorize_create_merge_request!
before_action :apply_diff_view_cookie!, only: [:diffs, :diff_for_path]
before_action :build_merge_request, except: [:create]
diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb
index 17cac69e588..c86acae8fe4 100644
--- a/app/controllers/projects/merge_requests_controller.rb
+++ b/app/controllers/projects/merge_requests_controller.rb
@@ -7,7 +7,6 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo
include IssuableCollections
skip_before_action :merge_request, only: [:index, :bulk_update]
- skip_before_action :ensure_ref_fetched, only: [:index, :bulk_update]
before_action :authorize_update_issuable!, only: [:close, :edit, :update, :remove_wip, :sort]
@@ -52,7 +51,6 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo
def show
validates_merge_request
- ensure_ref_fetched
close_merge_request_without_source_project
check_if_can_be_merged
diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb
index 3133dc9e7eb..ccad8e102aa 100644
--- a/app/models/merge_request.rb
+++ b/app/models/merge_request.rb
@@ -426,7 +426,7 @@ class MergeRequest < ActiveRecord::Base
end
def create_merge_request_diff
- fetch_ref
+ fetch_ref!
# n+1: https://gitlab.com/gitlab-org/gitlab-ce/issues/37435
Gitlab::GitalyClient.allow_n_plus_1_calls do
@@ -811,29 +811,14 @@ class MergeRequest < ActiveRecord::Base
end
end
- def fetch_ref
- write_ref
- update_column(:ref_fetched, true)
+ def fetch_ref!
+ target_project.repository.fetch_source_branch!(source_project.repository, source_branch, ref_path)
end
def ref_path
"refs/#{Repository::REF_MERGE_REQUEST}/#{iid}/head"
end
- def ref_fetched?
- super ||
- begin
- computed_value = project.repository.ref_exists?(ref_path)
- update_column(:ref_fetched, true) if computed_value
-
- computed_value
- end
- end
-
- def ensure_ref_fetched
- fetch_ref unless ref_fetched?
- end
-
def in_locked_state
begin
lock_mr
@@ -975,10 +960,4 @@ class MergeRequest < ActiveRecord::Base
project.merge_requests.merged.where(author_id: author_id).empty?
end
-
- private
-
- def write_ref
- target_project.repository.fetch_source_branch(source_project.repository, source_branch, ref_path)
- end
end
diff --git a/app/models/repository.rb b/app/models/repository.rb
index 44a1e9ce529..3b6285f786d 100644
--- a/app/models/repository.rb
+++ b/app/models/repository.rb
@@ -982,8 +982,8 @@ class Repository
gitlab_shell.fetch_remote(raw_repository, remote, forced: forced, no_tags: no_tags)
end
- def fetch_source_branch(source_repository, source_branch, local_ref)
- raw_repository.fetch_source_branch(source_repository.raw_repository, source_branch, local_ref)
+ def fetch_source_branch!(source_repository, source_branch, local_ref)
+ raw_repository.fetch_source_branch!(source_repository.raw_repository, source_branch, local_ref)
end
def compare_source_branch(target_branch_name, source_repository, source_branch_name, straight:)
diff --git a/config/initializers/8_metrics.rb b/config/initializers/8_metrics.rb
index 2d8704622b6..4fa72a494d0 100644
--- a/config/initializers/8_metrics.rb
+++ b/config/initializers/8_metrics.rb
@@ -118,8 +118,7 @@ def instrument_classes(instrumentation)
instrumentation.instrument_instance_method(MergeRequestDiff, :load_commits)
# Needed for https://gitlab.com/gitlab-org/gitlab-ce/issues/36061
- instrumentation.instrument_instance_method(MergeRequest, :ensure_ref_fetched)
- instrumentation.instrument_instance_method(MergeRequest, :fetch_ref)
+ instrumentation.instrument_instance_method(MergeRequest, :fetch_ref!)
end
# rubocop:enable Metrics/AbcSize
diff --git a/db/migrate/20171101134435_remove_ref_fetched_from_merge_requests.rb b/db/migrate/20171101134435_remove_ref_fetched_from_merge_requests.rb
new file mode 100644
index 00000000000..acba8384da4
--- /dev/null
+++ b/db/migrate/20171101134435_remove_ref_fetched_from_merge_requests.rb
@@ -0,0 +1,14 @@
+class RemoveRefFetchedFromMergeRequests < ActiveRecord::Migration
+ include Gitlab::Database::MigrationHelpers
+
+ # Set this constant to true if this migration requires downtime.
+ DOWNTIME = false
+
+ # We don't need to cache this anymore: the refs are now created
+ # upon save/update and there is no more use for this flag
+ #
+ # See https://gitlab.com/gitlab-org/gitlab-ce/issues/36061
+ def change
+ remove_column :merge_requests, :ref_fetched
+ end
+end
diff --git a/db/schema.rb b/db/schema.rb
index 80d8ff92d6e..7f1cfed4bb8 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -11,7 +11,7 @@
#
# It's strongly recommended that you check this file into your version control system.
-ActiveRecord::Schema.define(version: 20171026082505) do
+ActiveRecord::Schema.define(version: 20171101134435) do
# These are extensions that must be enabled in order to support this database
enable_extension "plpgsql"
@@ -970,7 +970,6 @@ ActiveRecord::Schema.define(version: 20171026082505) do
t.datetime "last_edited_at"
t.integer "last_edited_by_id"
t.integer "head_pipeline_id"
- t.boolean "ref_fetched"
t.string "merge_jid"
t.boolean "discussion_locked"
t.integer "latest_merge_request_diff_id"
diff --git a/lib/github/import.rb b/lib/github/import.rb
index 76612799412..13ecf58bd37 100644
--- a/lib/github/import.rb
+++ b/lib/github/import.rb
@@ -161,7 +161,6 @@ module Github
iid: pull_request.iid,
title: pull_request.title,
description: description,
- ref_fetched: true,
source_project: pull_request.source_project,
source_branch: pull_request.source_branch_name,
source_branch_sha: pull_request.source_branch_sha,
diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb
index 4f9eac92d9a..00f19257994 100644
--- a/lib/gitlab/git/repository.rb
+++ b/lib/gitlab/git/repository.rb
@@ -1034,7 +1034,7 @@ module Gitlab
delete_refs(tmp_ref) if tmp_ref
end
- def fetch_source_branch(source_repository, source_branch, local_ref)
+ def fetch_source_branch!(source_repository, source_branch, local_ref)
with_repo_branch_commit(source_repository, source_branch) do |commit|
if commit
write_ref(local_ref, commit.sha)
diff --git a/spec/lib/gitlab/git/repository_spec.rb b/spec/lib/gitlab/git/repository_spec.rb
index bf6d199ebe2..816157d8c0f 100644
--- a/spec/lib/gitlab/git/repository_spec.rb
+++ b/spec/lib/gitlab/git/repository_spec.rb
@@ -1482,7 +1482,7 @@ describe Gitlab::Git::Repository, seed_helper: true do
end
end
- describe '#fetch_source_branch' do
+ describe '#fetch_source_branch!' do
let(:local_ref) { 'refs/merge-requests/1/head' }
context 'when the branch exists' do
@@ -1491,11 +1491,11 @@ describe Gitlab::Git::Repository, seed_helper: true do
it 'writes the ref' do
expect(repository).to receive(:write_ref).with(local_ref, /\h{40}/)
- repository.fetch_source_branch(repository, source_branch, local_ref)
+ repository.fetch_source_branch!(repository, source_branch, local_ref)
end
it 'returns true' do
- expect(repository.fetch_source_branch(repository, source_branch, local_ref)).to eq(true)
+ expect(repository.fetch_source_branch!(repository, source_branch, local_ref)).to eq(true)
end
end
@@ -1505,11 +1505,11 @@ describe Gitlab::Git::Repository, seed_helper: true do
it 'does not write the ref' do
expect(repository).not_to receive(:write_ref)
- repository.fetch_source_branch(repository, source_branch, local_ref)
+ repository.fetch_source_branch!(repository, source_branch, local_ref)
end
it 'returns false' do
- expect(repository.fetch_source_branch(repository, source_branch, local_ref)).to eq(false)
+ expect(repository.fetch_source_branch!(repository, source_branch, local_ref)).to eq(false)
end
end
end
diff --git a/spec/lib/gitlab/import_export/fork_spec.rb b/spec/lib/gitlab/import_export/fork_spec.rb
index dd0ce0dae41..cfb15ee7e8b 100644
--- a/spec/lib/gitlab/import_export/fork_spec.rb
+++ b/spec/lib/gitlab/import_export/fork_spec.rb
@@ -46,7 +46,7 @@ describe 'forked project import' do
end
it 'can access the MR' do
- project.merge_requests.first.ensure_ref_fetched
+ project.merge_requests.first.fetch_ref!
expect(project.repository.ref_exists?('refs/merge-requests/1/head')).to be_truthy
end
diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb
index 476a2697605..f719ee26767 100644
--- a/spec/models/merge_request_spec.rb
+++ b/spec/models/merge_request_spec.rb
@@ -1755,39 +1755,12 @@ describe MergeRequest do
end
end
- describe '#fetch_ref' do
- it 'sets "ref_fetched" flag to true' do
- subject.update!(ref_fetched: nil)
+ describe '#fetch_ref!' do
+ it 'fetch the ref correctly' do
+ expect { subject.target_project.repository.delete_refs(subject.ref_path) }.not_to raise_error
- subject.fetch_ref
-
- expect(subject.reload.ref_fetched).to be_truthy
- end
- end
-
- describe '#ref_fetched?' do
- it 'does not perform git operation when value is cached' do
- subject.ref_fetched = true
-
- expect_any_instance_of(Repository).not_to receive(:ref_exists?)
- expect(subject.ref_fetched?).to be_truthy
- end
-
- it 'caches the value when ref exists but value is not cached' do
- subject.update!(ref_fetched: nil)
- allow_any_instance_of(Repository).to receive(:ref_exists?)
- .and_return(true)
-
- expect(subject.ref_fetched?).to be_truthy
- expect(subject.reload.ref_fetched).to be_truthy
- end
-
- it 'returns false when ref does not exist' do
- subject.update!(ref_fetched: nil)
- allow_any_instance_of(Repository).to receive(:ref_exists?)
- .and_return(false)
-
- expect(subject.ref_fetched?).to be_falsey
+ subject.fetch_ref!
+ expect(subject.target_project.repository.ref_exists?(subject.ref_path)).to be_truthy
end
end
View Lorry Controller Status