diff --git a/app/services/lfs/base_file_lock_service.rb b/app/services/lfs/base_file_lock_service.rb
new file mode 100644
index 0000000000000000000000000000000000000000..c1eeefba7a260275d37dc2138248f2dfe1d39deb
--- /dev/null
+++ b/app/services/lfs/base_file_lock_service.rb
@@ -0,0 +1,8 @@
+# frozen_string_literal: true
+
+module Lfs # rubocop:disable Gitlab/BoundedContexts -- These classes already exist so need some work before possible to move
+ class BaseFileLockService < BaseService
+ end
+end
+
+Lfs::BaseFileLockService.prepend_mod
diff --git a/app/services/lfs/lock_file_service.rb b/app/services/lfs/lock_file_service.rb
index b4505e2e49d8866b53360e5b8bc5d0b9115e52a4..684c1e8d963268c660c80772452c0dbc785252df 100644
--- a/app/services/lfs/lock_file_service.rb
+++ b/app/services/lfs/lock_file_service.rb
@@ -1,7 +1,7 @@
# frozen_string_literal: true
module Lfs
- class LockFileService < BaseService
+ class LockFileService < BaseFileLockService
def execute
unless can?(current_user, :push_code, project)
raise Gitlab::GitAccess::ForbiddenError, 'You have no permissions'
diff --git a/app/services/lfs/unlock_file_service.rb b/app/services/lfs/unlock_file_service.rb
index 677bbacd9fbaeafc9b4593b9b68003828874e8a4..f31118ded623644309e97f7717a74e242636b321 100644
--- a/app/services/lfs/unlock_file_service.rb
+++ b/app/services/lfs/unlock_file_service.rb
@@ -1,7 +1,7 @@
# frozen_string_literal: true
module Lfs
- class UnlockFileService < BaseService
+ class UnlockFileService < BaseFileLockService
def execute
unless can?(current_user, :push_code, project)
raise Gitlab::GitAccess::ForbiddenError, _('You have no permissions')
diff --git a/ee/app/controllers/projects/path_locks_controller.rb b/ee/app/controllers/projects/path_locks_controller.rb
index 1873b5e7d4e24e7381677bbf496e73fb7d8260ed..591fae56e67a27ae835fb09c6dfdf9528cb802fc 100644
--- a/ee/app/controllers/projects/path_locks_controller.rb
+++ b/ee/app/controllers/projects/path_locks_controller.rb
@@ -59,24 +59,11 @@ def check_license
end
def lock_file
- path_lock = PathLocks::LockService.new(project, current_user).execute(path)
-
- if path_lock.persisted? && sync_with_lfs?
- Lfs::LockFileService.new(
- project,
- current_user,
- path: path,
- create_path_lock: false
- ).execute
- end
+ PathLocks::LockService.new(project, current_user).execute(path)
end
def unlock_file(path_lock)
PathLocks::UnlockService.new(project, current_user).execute(path_lock)
-
- if sync_with_lfs?
- Lfs::UnlockFileService.new(project, current_user, path: path_lock.path, force: true).execute
- end
end
def lfs_file?
diff --git a/ee/app/graphql/mutations/projects/set_locked.rb b/ee/app/graphql/mutations/projects/set_locked.rb
index 764421d1a962986986d86ae9749a07f1bbd4e354..812ea63b9c5cf6832ea4c04e5a8e6bc8425ab84f 100644
--- a/ee/app/graphql/mutations/projects/set_locked.rb
+++ b/ee/app/graphql/mutations/projects/set_locked.rb
@@ -62,11 +62,7 @@ def fetch_path_lock(file_path)
def lock_path(file_path)
return if fetch_path_lock(file_path)
- path_lock = PathLocks::LockService.new(project, current_user).execute(file_path)
-
- if path_lock.persisted? && sync_with_lfs?(file_path)
- Lfs::LockFileService.new(project, current_user, path: file_path, create_path_lock: false).execute
- end
+ PathLocks::LockService.new(project, current_user).execute(file_path)
end
def unlock_path(file_path)
@@ -75,10 +71,6 @@ def unlock_path(file_path)
return unless path_lock
PathLocks::UnlockService.new(project, current_user).execute(path_lock)
-
- if sync_with_lfs?(file_path)
- Lfs::UnlockFileService.new(project, current_user, path: file_path, force: true).execute
- end
end
def sync_with_lfs?(file_path)
diff --git a/ee/app/services/ee/lfs/base_file_lock_service.rb b/ee/app/services/ee/lfs/base_file_lock_service.rb
new file mode 100644
index 0000000000000000000000000000000000000000..ad7049b577b03a3137981ae2fa419bd3f26c0ce1
--- /dev/null
+++ b/ee/app/services/ee/lfs/base_file_lock_service.rb
@@ -0,0 +1,25 @@
+# frozen_string_literal: true
+
+module EE
+ module Lfs # rubocop:disable Gitlab/BoundedContexts -- These classes already exist so need some work before possible to move
+ module BaseFileLockService
+ def initialize(project, user = nil, params = {})
+ # When `syncing_path_lock` is `true` it means that the user locked a
+ # file and the system is now syncing the lock from GitLab's path lock
+ # feature to Git LFS file locking feature.
+ #
+ # This only happens when the file that the user locked is an LFS file.
+ @syncing_path_lock = params[:syncing_path_lock] == true
+ super
+ end
+
+ private
+
+ attr_reader :syncing_path_lock
+
+ def sync_with_file?(lfs_lock_status)
+ lfs_lock_status == :success && !syncing_path_lock && project.licensed_feature_available?(:file_locks)
+ end
+ end
+ end
+end
diff --git a/ee/app/services/ee/lfs/lock_file_service.rb b/ee/app/services/ee/lfs/lock_file_service.rb
index 018bf46de221886acdd231716cf96e70b0f4bb99..1bd5ef93391b1befbd2338aa0e9ed1440be260cb 100644
--- a/ee/app/services/ee/lfs/lock_file_service.rb
+++ b/ee/app/services/ee/lfs/lock_file_service.rb
@@ -6,21 +6,15 @@ module LockFileService
def execute
result = super
- create_path_lock(result[:lock].path) if create_path_lock?(result[:status])
+ create_path_lock(result[:lock].path) if sync_with_file?(result[:status])
result
end
private
- def create_path_lock?(lfs_lock_status)
- lfs_lock_status == :success &&
- params[:create_path_lock] != false &&
- project.feature_available?(:file_locks)
- end
-
def create_path_lock(path)
- PathLocks::LockService.new(project, current_user).execute(path)
+ PathLocks::LockService.new(project, current_user, syncing_lfs_lock: true).execute(path)
end
end
end
diff --git a/ee/app/services/ee/lfs/unlock_file_service.rb b/ee/app/services/ee/lfs/unlock_file_service.rb
index c33877a9129e785527683860cc154dba89fdd01e..1e1f4ba1bdda2a278678f8edcfbcf375c5e34e1b 100644
--- a/ee/app/services/ee/lfs/unlock_file_service.rb
+++ b/ee/app/services/ee/lfs/unlock_file_service.rb
@@ -6,25 +6,19 @@ module UnlockFileService
def execute
result = super
- destroy_path_lock(result[:lock].path) if destroy_path_lock?(result[:status])
+ destroy_path_lock(result[:lock].path) if sync_with_file?(result[:status])
result
end
private
- def destroy_path_lock?(lfs_lock_status)
- lfs_lock_status == :success &&
- params[:destroy_path_lock] != false &&
- project.feature_available?(:file_locks)
- end
-
def destroy_path_lock(path)
path_lock = project.path_locks.for_path(path)
return unless path_lock
- PathLocks::UnlockService.new(project, current_user).execute(path_lock)
+ PathLocks::UnlockService.new(project, current_user, syncing_lfs_lock: true).execute(path_lock)
end
end
end
diff --git a/ee/app/services/path_locks/base_service.rb b/ee/app/services/path_locks/base_service.rb
new file mode 100644
index 0000000000000000000000000000000000000000..87f4096b77977182ba3118eb21e264fb0426c4c5
--- /dev/null
+++ b/ee/app/services/path_locks/base_service.rb
@@ -0,0 +1,31 @@
+# frozen_string_literal: true
+
+module PathLocks # rubocop:disable Gitlab/BoundedContexts -- These classes already exist so need some work before possible to move
+ class BaseService < BaseService
+ def initialize(project, user = nil, params = {})
+ # When `syncing_lfs_lock` is `true` it means that the user locked a Git
+ # LFS file and the system is now syncing the lock from Git LFS file
+ # locking feature to GitLab's path lock feature.
+ @syncing_lfs_lock = params[:syncing_lfs_lock] == true
+ super
+ end
+
+ private
+
+ attr_reader :syncing_lfs_lock
+
+ def sync_with_lfs?(path)
+ !syncing_lfs_lock && project.lfs_enabled? && lfs_file?(path)
+ end
+
+ def lfs_file?(path)
+ blob = repository.blob_at_branch(repository.root_ref, path)
+
+ return false unless blob
+
+ lfs_blob_ids = LfsPointersFinder.new(repository, path).execute
+
+ lfs_blob_ids.include?(blob.id)
+ end
+ end
+end
diff --git a/ee/app/services/path_locks/lock_service.rb b/ee/app/services/path_locks/lock_service.rb
index c78efbfae814f2a867dab461de3465b7278738f7..4d8fa3794efc4cf175f8f59dd0c369a47eb45a19 100644
--- a/ee/app/services/path_locks/lock_service.rb
+++ b/ee/app/services/path_locks/lock_service.rb
@@ -4,14 +4,18 @@ module PathLocks
class LockService < BaseService
AccessDenied = Class.new(StandardError)
- include PathLocksHelper
-
def execute(path)
raise AccessDenied, 'You have no permissions' unless can?(current_user, :push_code, project)
- project.path_locks.create(path: path, user: current_user).tap do |path_lock|
- project.refresh_path_locks_changed_epoch if path_lock.persisted?
- end
+ path_lock = project.path_locks.create(path: path, user: current_user)
+
+ return unless path_lock.persisted?
+
+ project.refresh_path_locks_changed_epoch
+
+ return unless sync_with_lfs?(path)
+
+ Lfs::LockFileService.new(project, current_user, path: path, syncing_path_lock: true).execute
end
end
end
diff --git a/ee/app/services/path_locks/unlock_service.rb b/ee/app/services/path_locks/unlock_service.rb
index a26a9b9d8383f791c116b52c1de80bc08d70e5ef..fd9a23aee53bd7847a3c2e6b7a4edaac7ccb017a 100644
--- a/ee/app/services/path_locks/unlock_service.rb
+++ b/ee/app/services/path_locks/unlock_service.rb
@@ -4,14 +4,21 @@ module PathLocks
class UnlockService < BaseService
AccessDenied = Class.new(StandardError)
- include PathLocksHelper
-
def execute(path_lock)
- raise AccessDenied, _('You have no permissions') unless can_unlock?(path_lock)
+ return unless path_lock
+
+ raise AccessDenied, _('You have no permissions') unless can?(current_user, :admin_path_locks, path_lock)
+
+ path = path_lock.path
+ path_lock.destroy
+
+ return unless path_lock.destroyed?
+
+ project.refresh_path_locks_changed_epoch
+
+ return unless sync_with_lfs?(path)
- path_lock.destroy.tap do |record|
- project.refresh_path_locks_changed_epoch if record.destroyed?
- end
+ Lfs::UnlockFileService.new(project, current_user, path: path, force: true, syncing_path_lock: true).execute
end
end
end
diff --git a/ee/spec/services/lfs/lock_file_service_spec.rb b/ee/spec/services/lfs/lock_file_service_spec.rb
index d7c9081441274ad426a971c77b81534303b099f2..5f4ef8e3f5d609bf7802582685dc0706e523ce06 100644
--- a/ee/spec/services/lfs/lock_file_service_spec.rb
+++ b/ee/spec/services/lfs/lock_file_service_spec.rb
@@ -5,8 +5,6 @@
RSpec.describe Lfs::LockFileService, feature_category: :source_code_management do
let(:project) { create(:project) }
let(:current_user) { create(:user) }
- let(:create_path_lock) { true }
- let(:params) { { path: 'README.md', create_path_lock: create_path_lock } }
subject { described_class.new(project, current_user, params) }
@@ -16,41 +14,50 @@
project.add_developer(current_user)
end
- context 'when File Locking is available' do
- before do
- stub_licensed_features(file_locks: true)
- end
+ describe 'File Locking integration' do
+ let(:syncing_path_lock) { false }
+ let(:params) { { path: 'README.md', syncing_path_lock: syncing_path_lock } }
+ let(:file_locks_license) { true }
+ let(:path_lock_service) { PathLocks::LockService }
- it 'creates the Path Lock' do
- expect { subject.execute }.to change { PathLock.count }.to(1)
+ before do
+ stub_licensed_features(file_locks: file_locks_license)
+ allow(path_lock_service).to receive(:new).and_call_original
end
- context 'when the lfs file was not locked successfully' do
- before do
- allow(subject).to receive(:create_lock!).and_return({ status: :error })
+ context 'when File Locking is available' do
+ it 'creates the Path Lock', :aggregate_failures do
+ expect { subject.execute }.to change { PathLock.count }.to(1)
+ expect(path_lock_service)
+ .to have_received(:new)
+ .with(project, current_user, syncing_lfs_lock: true)
end
- it 'does not create a Path Lock' do
- expect { subject.execute }.not_to change { PathLock.count }
+ context 'when the lfs file was not locked successfully' do
+ before do
+ allow(subject).to receive(:create_lock!).and_return({ status: :error })
+ end
+
+ it 'does not create a Path Lock' do
+ expect { subject.execute }.not_to change { PathLock.count }
+ end
end
- end
- context 'when create_path_lock is false' do
- let(:create_path_lock) { false }
+ context 'when syncing_path_lock is true' do
+ let(:syncing_path_lock) { true }
- it 'does not create a Path Lock' do
- expect { subject.execute }.not_to change { PathLock.count }
+ it 'does not create a Path Lock' do
+ expect { subject.execute }.not_to change { PathLock.count }
+ end
end
end
- end
- context 'when File Locking is not available' do
- before do
- stub_licensed_features(file_locks: false)
- end
+ context 'when File Locking is not available' do
+ let(:file_locks_license) { false }
- it 'does not create the Path Lock' do
- expect { subject.execute }.not_to change { PathLock.count }
+ it 'does not create the Path Lock' do
+ expect { subject.execute }.not_to change { PathLock.count }
+ end
end
end
end
diff --git a/ee/spec/services/lfs/unlock_file_service_spec.rb b/ee/spec/services/lfs/unlock_file_service_spec.rb
index a2f086b3edb3d30b4521e6a812bdaadc4af2cce7..4130bdd3ea9a408114d19f10772a9849026edc2f 100644
--- a/ee/spec/services/lfs/unlock_file_service_spec.rb
+++ b/ee/spec/services/lfs/unlock_file_service_spec.rb
@@ -12,25 +12,28 @@
describe '#execute' do
context 'when authorized' do
before do
- project.add_developer(current_user)
+ project.add_developer(lock_author)
end
- describe 'File Locking integraction' do
- let(:destroy_path_lock) { true }
- let(:params) { { id: lock.id, destroy_path_lock: destroy_path_lock } }
+ describe 'File Locking integration' do
+ let(:syncing_path_lock) { false }
+ let(:params) { { id: lock.id, syncing_path_lock: syncing_path_lock } }
let(:current_user) { lock_author }
let(:file_locks_license) { true }
+ let(:path_lock_service) { PathLocks::UnlockService }
before do
stub_licensed_features(file_locks: file_locks_license)
-
- project.add_developer(lock_author)
project.path_locks.create!(path: lock.path, user: lock_author)
+ allow(path_lock_service).to receive(:new).and_call_original
end
context 'when File Locking is available' do
- it 'deletes the Path Lock' do
+ it 'deletes the Path Lock', :aggregate_failures do
expect { subject.execute }.to change { PathLock.count }.to(0)
+ expect(path_lock_service)
+ .to have_received(:new)
+ .with(project, lock_author, syncing_lfs_lock: true)
end
context 'when the lfs file was not unlocked successfully' do
@@ -43,8 +46,8 @@
end
end
- context 'when destroy_path_lock is false' do
- let(:destroy_path_lock) { false }
+ context 'when syncing_path_lock is true' do
+ let(:syncing_path_lock) { true }
it 'does not delete the Path Lock' do
expect { subject.execute }.not_to change { PathLock.count }
diff --git a/ee/spec/services/path_locks/lock_service_spec.rb b/ee/spec/services/path_locks/lock_service_spec.rb
index 7fb958858398aec9fff6d4e3fb89fd3e79023cd1..45ee1d7f0561a130a9bcf3613ddf10d5eac42f32 100644
--- a/ee/spec/services/path_locks/lock_service_spec.rb
+++ b/ee/spec/services/path_locks/lock_service_spec.rb
@@ -3,18 +3,20 @@
require 'spec_helper'
RSpec.describe PathLocks::LockService, feature_category: :source_code_management do
- let(:current_user) { create(:user) }
- let(:project) { create(:project) }
- let(:path) { 'app/models' }
+ let_it_be(:project) { create(:project, :repository) }
+ let_it_be(:current_user) { create(:user) }
+
+ let(:syncing_lfs_lock) { false }
+ let(:path) { 'app/models' }
describe '#execute(path)' do
- subject(:execute) { described_class.new(project, current_user).execute(path) }
+ subject(:execute) do
+ described_class.new(project, current_user, syncing_lfs_lock: syncing_lfs_lock).execute(path)
+ end
context 'when user can push code' do
- before do
- allow_next_instance_of(described_class) do |instance|
- allow(instance).to receive(:can?).with(current_user, :push_code, project).and_return(true)
- end
+ before_all do
+ project.add_developer(current_user)
end
it 'locks path' do
@@ -22,6 +24,53 @@
end
it_behaves_like 'refreshes project.path_locks_changed_epoch value'
+
+ describe 'Lfs File Lock integration' do
+ let(:lfs_enabled) { true }
+ let(:lfs_file_lock_service) { Lfs::LockFileService }
+
+ before do
+ allow(project).to receive(:lfs_enabled?).and_return(lfs_enabled)
+ allow(lfs_file_lock_service).to receive(:new).and_call_original
+ end
+
+ context 'when the file is an lfs file' do
+ let(:path) { 'files/lfs/lfs_object.iso' }
+
+ before do
+ allow(project.repository).to receive(:root_ref).and_return('lfs')
+ end
+
+ it 'creates the Lfs File Lock', :aggregate_failures do
+ expect { execute }.to change { LfsFileLock.count }.from(0).to(1)
+ expect(lfs_file_lock_service)
+ .to have_received(:new)
+ .with(project, current_user, syncing_path_lock: true, path: path)
+ end
+
+ context 'when lfs is disabled' do
+ let(:lfs_enabled) { false }
+
+ it 'does not create an LfsFileLock' do
+ expect { execute }.not_to change { LfsFileLock.count }
+ end
+ end
+
+ context 'when syncing_lfs_lock is true' do
+ let(:syncing_lfs_lock) { true }
+
+ it 'does not create an LfsFileLock' do
+ expect { execute }.not_to change { LfsFileLock.count }
+ end
+ end
+ end
+
+ context 'when the file is not an lfs file' do
+ it 'does not create an LfsFileLock' do
+ expect { execute }.not_to change { LfsFileLock.count }
+ end
+ end
+ end
end
context 'when user cannot push code' do
diff --git a/ee/spec/services/path_locks/unlock_service_spec.rb b/ee/spec/services/path_locks/unlock_service_spec.rb
index 98635fce46af69995a5c3ee66c64a3a3e875f767..6c315f6f520bca8d0317331f4a4278c14fb54411 100644
--- a/ee/spec/services/path_locks/unlock_service_spec.rb
+++ b/ee/spec/services/path_locks/unlock_service_spec.rb
@@ -3,19 +3,21 @@
require 'spec_helper'
RSpec.describe PathLocks::UnlockService, feature_category: :source_code_management do
- let(:path_lock) { create :path_lock }
- let(:current_user) { path_lock.user }
- let(:project) { path_lock.project }
- let(:path) { path_lock.path }
+ let_it_be(:project) { create(:project, :repository) }
+ let_it_be(:current_user) { create(:user) }
+
+ let(:path) { 'app/models' }
+ let!(:path_lock) { create(:path_lock, path: path, user: current_user, project: project) }
+ let(:syncing_lfs_lock) { false }
describe '#execute(path_lock)' do
- subject(:execute) { described_class.new(project, current_user).execute(path_lock) }
+ subject(:execute) do
+ described_class.new(project, current_user, syncing_lfs_lock: syncing_lfs_lock).execute(path_lock)
+ end
- context 'when the user can unlock the path lock' do
- before do
- allow_next_instance_of(described_class) do |instance|
- allow(instance).to receive(:can?).with(current_user, :admin_path_locks, path_lock).and_return(true)
- end
+ context 'when the user can admin_path_locks' do
+ before_all do
+ project.add_developer(current_user)
end
it 'unlocks path' do
@@ -23,6 +25,54 @@
end
it_behaves_like 'refreshes project.path_locks_changed_epoch value'
+
+ describe 'Lfs File Lock integration' do
+ let(:lfs_enabled) { true }
+ let(:lfs_file_lock_service) { Lfs::UnlockFileService }
+
+ before do
+ allow(project).to receive(:lfs_enabled?).and_return(lfs_enabled)
+ allow(lfs_file_lock_service).to receive(:new).and_call_original
+ project.lfs_file_locks.create!(path: path, user: current_user)
+ end
+
+ context 'when the file is an lfs file' do
+ let(:path) { 'files/lfs/lfs_object.iso' }
+
+ before do
+ allow(project.repository).to receive(:root_ref).and_return('lfs')
+ end
+
+ it 'destroys the Lfs File Lock', :aggregate_failures do
+ expect { execute }.to change { LfsFileLock.count }.from(1).to(0)
+ expect(lfs_file_lock_service)
+ .to have_received(:new)
+ .with(project, current_user, force: true, syncing_path_lock: true, path: path)
+ end
+
+ context 'when lfs is disabled' do
+ let(:lfs_enabled) { false }
+
+ it 'does not destroy the LfsFileLock' do
+ expect { execute }.not_to change { LfsFileLock.count }
+ end
+ end
+
+ context 'when syncing_lfs_lock is true' do
+ let(:syncing_lfs_lock) { true }
+
+ it 'does not destroy the LfsFileLock' do
+ expect { execute }.not_to change { LfsFileLock.count }
+ end
+ end
+ end
+
+ context 'when the file is not an lfs file' do
+ it 'does not destroy the LfsFileLock' do
+ expect { execute }.not_to change { LfsFileLock.count }
+ end
+ end
+ end
end
context 'when the user cannot unlock the path lock' do