summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorRubén Dávila <ruben@gitlab.com>2018-02-07 08:00:53 -0500
committerRubén Dávila <ruben@gitlab.com>2018-02-07 09:04:00 -0500
commitbed948321173b49564f39837e97212ee4dd96e03 (patch)
tree72e6faa9f68378f997f876cf9550561bad546028
parentead97c55eac773444dee547a934112aa282c2e2e (diff)
downloadgitlab-ce-rd-35856-backport-lfs-file-locking-api.tar.gz
Backport of LFS File Locking APIrd-35856-backport-lfs-file-locking-api
-rw-r--r--app/controllers/concerns/lfs_request.rb6
-rw-r--r--app/controllers/projects/lfs_api_controller.rb2
-rw-r--r--app/controllers/projects/lfs_locks_api_controller.rb70
-rw-r--r--app/models/lfs_file_lock.rb12
-rw-r--r--app/models/project.rb1
-rw-r--r--app/models/repository.rb7
-rw-r--r--app/serializers/lfs_file_lock_entity.rb11
-rw-r--r--app/serializers/lfs_file_lock_serializer.rb3
-rw-r--r--app/services/lfs/lock_file_service.rb39
-rw-r--r--app/services/lfs/locks_finder_service.rb17
-rw-r--r--app/services/lfs/unlock_file_service.rb43
-rw-r--r--changelogs/unreleased/35856-implement-file-locking-api.yml5
-rw-r--r--config/initializers/mime_types.rb2
-rw-r--r--config/routes/git_http.rb7
-rw-r--r--db/migrate/20180116193854_create_lfs_file_locks.rb30
-rw-r--r--db/schema.rb12
-rw-r--r--doc/workflow/lfs/manage_large_binaries_with_git_lfs.md66
-rw-r--r--lib/gitlab/checks/change_access.rb29
-rw-r--r--lib/gitlab/checks/commit_check.rb61
-rw-r--r--lib/gitlab/import_export/import_export.yml2
-rw-r--r--spec/factories/lfs_file_locks.rb7
-rw-r--r--spec/lib/gitlab/checks/change_access_spec.rb39
-rw-r--r--spec/lib/gitlab/import_export/all_models.yml3
-rw-r--r--spec/lib/gitlab/import_export/safe_model_attributes.yml6
-rw-r--r--spec/models/lfs_file_lock_spec.rb57
-rw-r--r--spec/models/project_spec.rb1
-rw-r--r--spec/models/repository_spec.rb22
-rw-r--r--spec/requests/lfs_http_spec.rb2
-rw-r--r--spec/requests/lfs_locks_api_spec.rb159
-rw-r--r--spec/serializers/lfs_file_lock_entity_spec.rb19
-rw-r--r--spec/services/lfs/lock_file_service_spec.rb62
-rw-r--r--spec/services/lfs/locks_finder_service_spec.rb101
-rw-r--r--spec/services/lfs/unlock_file_service_spec.rb105
33 files changed, 1002 insertions, 6 deletions
diff --git a/app/controllers/concerns/lfs_request.rb b/app/controllers/concerns/lfs_request.rb
index 4311f9d4db9..5e4e8a87153 100644
--- a/app/controllers/concerns/lfs_request.rb
+++ b/app/controllers/concerns/lfs_request.rb
@@ -10,6 +10,8 @@
module LfsRequest
extend ActiveSupport::Concern
+ CONTENT_TYPE = 'application/vnd.git-lfs+json'.freeze
+
included do
before_action :require_lfs_enabled!
before_action :lfs_check_access!
@@ -50,7 +52,7 @@ module LfsRequest
message: 'Access forbidden. Check your access level.',
documentation_url: help_url
},
- content_type: "application/vnd.git-lfs+json",
+ content_type: CONTENT_TYPE,
status: 403
)
end
@@ -61,7 +63,7 @@ module LfsRequest
message: 'Not found.',
documentation_url: help_url
},
- content_type: "application/vnd.git-lfs+json",
+ content_type: CONTENT_TYPE,
status: 404
)
end
diff --git a/app/controllers/projects/lfs_api_controller.rb b/app/controllers/projects/lfs_api_controller.rb
index 536f908d2c5..c77f10ef1dd 100644
--- a/app/controllers/projects/lfs_api_controller.rb
+++ b/app/controllers/projects/lfs_api_controller.rb
@@ -98,7 +98,7 @@ class Projects::LfsApiController < Projects::GitHttpClientController
json: {
message: lfs_read_only_message
},
- content_type: 'application/vnd.git-lfs+json',
+ content_type: LfsRequest::CONTENT_TYPE,
status: 403
)
end
diff --git a/app/controllers/projects/lfs_locks_api_controller.rb b/app/controllers/projects/lfs_locks_api_controller.rb
new file mode 100644
index 00000000000..3fff0fd69ae
--- /dev/null
+++ b/app/controllers/projects/lfs_locks_api_controller.rb
@@ -0,0 +1,70 @@
+class Projects::LfsLocksApiController < Projects::GitHttpClientController
+ include LfsRequest
+
+ def create
+ @result = Lfs::LockFileService.new(project, user, params).execute
+
+ render_json(@result[:lock])
+ end
+
+ def unlock
+ @result = Lfs::UnlockFileService.new(project, user, params).execute
+
+ render_json(@result[:lock])
+ end
+
+ def index
+ @result = Lfs::LocksFinderService.new(project, user, params).execute
+
+ render_json(@result[:locks])
+ end
+
+ def verify
+ @result = Lfs::LocksFinderService.new(project, user, {}).execute
+
+ ours, theirs = split_by_owner(@result[:locks])
+
+ render_json({ ours: ours, theirs: theirs }, false)
+ end
+
+ private
+
+ def render_json(data, process = true)
+ render json: build_payload(data, process),
+ content_type: LfsRequest::CONTENT_TYPE,
+ status: @result[:http_status]
+ end
+
+ def build_payload(data, process)
+ data = LfsFileLockSerializer.new.represent(data) if process
+
+ return data if @result[:status] == :success
+
+ # When the locking failed due to an existent Lock, the existent record
+ # is returned in `@result[:lock]`
+ error_payload(@result[:message], @result[:lock] ? data : {})
+ end
+
+ def error_payload(message, custom_attrs = {})
+ custom_attrs.merge({
+ message: message,
+ documentation_url: help_url
+ })
+ end
+
+ def split_by_owner(locks)
+ groups = locks.partition { |lock| lock.user_id == user.id }
+
+ groups.map! do |records|
+ LfsFileLockSerializer.new.represent(records, root: false)
+ end
+ end
+
+ def download_request?
+ params[:action] == 'index'
+ end
+
+ def upload_request?
+ %w(create unlock verify).include?(params[:action])
+ end
+end
diff --git a/app/models/lfs_file_lock.rb b/app/models/lfs_file_lock.rb
new file mode 100644
index 00000000000..50bb6ca382d
--- /dev/null
+++ b/app/models/lfs_file_lock.rb
@@ -0,0 +1,12 @@
+class LfsFileLock < ActiveRecord::Base
+ belongs_to :project
+ belongs_to :user
+
+ validates :project_id, :user_id, :path, presence: true
+
+ def can_be_unlocked_by?(current_user, forced = false)
+ return true if current_user.id == user_id
+
+ forced && current_user.can?(:admin_project, project)
+ end
+end
diff --git a/app/models/project.rb b/app/models/project.rb
index fd8917467e9..0590cc1c720 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -179,6 +179,7 @@ class Project < ActiveRecord::Base
has_many :releases
has_many :lfs_objects_projects, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent
has_many :lfs_objects, through: :lfs_objects_projects
+ has_many :lfs_file_locks
has_many :project_group_links
has_many :invited_groups, through: :project_group_links, source: :group
has_many :pages_domains
diff --git a/app/models/repository.rb b/app/models/repository.rb
index 3d6f8f0c305..1cf55fd4332 100644
--- a/app/models/repository.rb
+++ b/app/models/repository.rb
@@ -164,6 +164,13 @@ class Repository
commits
end
+ # Returns a list of commits that are not present in any reference
+ def new_commits(newrev)
+ refs = ::Gitlab::Git::RevList.new(raw, newrev: newrev).new_refs
+
+ refs.map { |sha| commit(sha.strip) }
+ end
+
# Gitaly migration: https://gitlab.com/gitlab-org/gitaly/issues/384
def find_commits_by_message(query, ref = nil, path = nil, limit = 1000, offset = 0)
unless exists? && has_visible_content? && query.present?
diff --git a/app/serializers/lfs_file_lock_entity.rb b/app/serializers/lfs_file_lock_entity.rb
new file mode 100644
index 00000000000..264a77adc3f
--- /dev/null
+++ b/app/serializers/lfs_file_lock_entity.rb
@@ -0,0 +1,11 @@
+class LfsFileLockEntity < Grape::Entity
+ root 'locks', 'lock'
+
+ expose :path
+ expose(:id) { |entity| entity.id.to_s }
+ expose(:created_at, as: :locked_at) { |entity| entity.created_at.to_s(:iso8601) }
+
+ expose :owner do
+ expose(:name) { |entity| entity.user&.name }
+ end
+end
diff --git a/app/serializers/lfs_file_lock_serializer.rb b/app/serializers/lfs_file_lock_serializer.rb
new file mode 100644
index 00000000000..ba8fb1a461d
--- /dev/null
+++ b/app/serializers/lfs_file_lock_serializer.rb
@@ -0,0 +1,3 @@
+class LfsFileLockSerializer < BaseSerializer
+ entity LfsFileLockEntity
+end
diff --git a/app/services/lfs/lock_file_service.rb b/app/services/lfs/lock_file_service.rb
new file mode 100644
index 00000000000..bbe10f84ef4
--- /dev/null
+++ b/app/services/lfs/lock_file_service.rb
@@ -0,0 +1,39 @@
+module Lfs
+ class LockFileService < BaseService
+ def execute
+ unless can?(current_user, :push_code, project)
+ raise Gitlab::GitAccess::UnauthorizedError, 'You have no permissions'
+ end
+
+ create_lock!
+ rescue ActiveRecord::RecordNotUnique
+ error('already locked', 409, current_lock)
+ rescue Gitlab::GitAccess::UnauthorizedError => ex
+ error(ex.message, 403)
+ rescue => ex
+ error(ex.message, 500)
+ end
+
+ private
+
+ def current_lock
+ project.lfs_file_locks.find_by(path: params[:path])
+ end
+
+ def create_lock!
+ lock = project.lfs_file_locks.create!(user: current_user,
+ path: params[:path])
+
+ success(http_status: 201, lock: lock)
+ end
+
+ def error(message, http_status, lock = nil)
+ {
+ status: :error,
+ message: message,
+ http_status: http_status,
+ lock: lock
+ }
+ end
+ end
+end
diff --git a/app/services/lfs/locks_finder_service.rb b/app/services/lfs/locks_finder_service.rb
new file mode 100644
index 00000000000..13c6cc6f81c
--- /dev/null
+++ b/app/services/lfs/locks_finder_service.rb
@@ -0,0 +1,17 @@
+module Lfs
+ class LocksFinderService < BaseService
+ def execute
+ success(locks: find_locks)
+ rescue => ex
+ error(ex.message, 500)
+ end
+
+ private
+
+ def find_locks
+ options = params.slice(:id, :path).compact.symbolize_keys
+
+ project.lfs_file_locks.where(options)
+ end
+ end
+end
diff --git a/app/services/lfs/unlock_file_service.rb b/app/services/lfs/unlock_file_service.rb
new file mode 100644
index 00000000000..6c93dc69bb0
--- /dev/null
+++ b/app/services/lfs/unlock_file_service.rb
@@ -0,0 +1,43 @@
+module Lfs
+ class UnlockFileService < BaseService
+ def execute
+ unless can?(current_user, :push_code, project)
+ raise Gitlab::GitAccess::UnauthorizedError, 'You have no permissions'
+ end
+
+ unlock_file
+ rescue Gitlab::GitAccess::UnauthorizedError => ex
+ error(ex.message, 403)
+ rescue ActiveRecord::RecordNotFound
+ error('Lock not found', 404)
+ rescue => ex
+ error(ex.message, 500)
+ end
+
+ private
+
+ def unlock_file
+ forced = params[:force] == true
+
+ if lock.can_be_unlocked_by?(current_user, forced)
+ lock.destroy!
+
+ success(lock: lock, http_status: :ok)
+ elsif forced
+ error('You must have master access to force delete a lock', 403)
+ else
+ error("#{lock.path} is locked by GitLab User #{lock.user_id}", 403)
+ end
+ end
+
+ def lock
+ return @lock if defined?(@lock)
+
+ @lock = if params[:id].present?
+ project.lfs_file_locks.find(params[:id])
+ elsif params[:path].present?
+ project.lfs_file_locks.find_by!(path: params[:path])
+ end
+ end
+ end
+end
diff --git a/changelogs/unreleased/35856-implement-file-locking-api.yml b/changelogs/unreleased/35856-implement-file-locking-api.yml
new file mode 100644
index 00000000000..fa848ad9ed8
--- /dev/null
+++ b/changelogs/unreleased/35856-implement-file-locking-api.yml
@@ -0,0 +1,5 @@
+---
+title: Backport of LFS File Locking API
+merge_request: 16935
+author:
+type: added
diff --git a/config/initializers/mime_types.rb b/config/initializers/mime_types.rb
index 5e3e4c966cb..e9326653cbe 100644
--- a/config/initializers/mime_types.rb
+++ b/config/initializers/mime_types.rb
@@ -14,4 +14,4 @@ Mime::Type.register "video/webm", :webm
Mime::Type.register "video/ogg", :ogv
Mime::Type.unregister :json
-Mime::Type.register 'application/json', :json, %w(application/vnd.git-lfs+json application/json)
+Mime::Type.register 'application/json', :json, [LfsRequest::CONTENT_TYPE, 'application/json']
diff --git a/config/routes/git_http.rb b/config/routes/git_http.rb
index a53c94326d4..ff51823897d 100644
--- a/config/routes/git_http.rb
+++ b/config/routes/git_http.rb
@@ -16,6 +16,13 @@ scope(path: '*namespace_id/:project_id',
get '/*oid', action: :deprecated
end
+ scope(path: 'info/lfs') do
+ resources :lfs_locks, controller: :lfs_locks_api, path: 'locks' do
+ post :unlock, on: :member
+ post :verify, on: :collection
+ end
+ end
+
# GitLab LFS object storage
scope(path: 'gitlab-lfs/objects/*oid', controller: :lfs_storage, constraints: { oid: /[a-f0-9]{64}/ }) do
get '/', action: :download
diff --git a/db/migrate/20180116193854_create_lfs_file_locks.rb b/db/migrate/20180116193854_create_lfs_file_locks.rb
new file mode 100644
index 00000000000..23b0c90484b
--- /dev/null
+++ b/db/migrate/20180116193854_create_lfs_file_locks.rb
@@ -0,0 +1,30 @@
+class CreateLfsFileLocks < ActiveRecord::Migration
+ include Gitlab::Database::MigrationHelpers
+
+ DOWNTIME = false
+
+ disable_ddl_transaction!
+
+ def up
+ create_table :lfs_file_locks do |t|
+ t.references :project, null: false, foreign_key: { on_delete: :cascade }
+ t.references :user, null: false, index: true, foreign_key: { on_delete: :cascade }
+ t.datetime :created_at, null: false
+ t.string :path, limit: 511
+ end
+
+ add_index :lfs_file_locks, [:project_id, :path], unique: true
+ end
+
+ def down
+ if foreign_keys_for(:lfs_file_locks, :project_id).any?
+ remove_foreign_key :lfs_file_locks, column: :project_id
+ end
+
+ if index_exists?(:lfs_file_locks, [:project_id, :path])
+ remove_concurrent_index :lfs_file_locks, [:project_id, :path]
+ end
+
+ drop_table :lfs_file_locks
+ end
+end
diff --git a/db/schema.rb b/db/schema.rb
index 9ab4dabfb7d..d07a4c31618 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -947,6 +947,16 @@ ActiveRecord::Schema.define(version: 20180206200543) do
add_index "labels", ["title"], name: "index_labels_on_title", using: :btree
add_index "labels", ["type", "project_id"], name: "index_labels_on_type_and_project_id", using: :btree
+ create_table "lfs_file_locks", force: :cascade do |t|
+ t.integer "project_id", null: false
+ t.integer "user_id", null: false
+ t.datetime "created_at", null: false
+ t.string "path", limit: 511
+ end
+
+ add_index "lfs_file_locks", ["project_id", "path"], name: "index_lfs_file_locks_on_project_id_and_path", unique: true, using: :btree
+ add_index "lfs_file_locks", ["user_id"], name: "index_lfs_file_locks_on_user_id", using: :btree
+
create_table "lfs_objects", force: :cascade do |t|
t.string "oid", null: false
t.integer "size", limit: 8, null: false
@@ -1998,6 +2008,8 @@ ActiveRecord::Schema.define(version: 20180206200543) do
add_foreign_key "label_priorities", "projects", on_delete: :cascade
add_foreign_key "labels", "namespaces", column: "group_id", on_delete: :cascade
add_foreign_key "labels", "projects", name: "fk_7de4989a69", on_delete: :cascade
+ add_foreign_key "lfs_file_locks", "projects", on_delete: :cascade
+ add_foreign_key "lfs_file_locks", "users", on_delete: :cascade
add_foreign_key "lists", "boards", name: "fk_0d3f677137", on_delete: :cascade
add_foreign_key "lists", "labels", name: "fk_7a5553d60f", on_delete: :cascade
add_foreign_key "members", "users", name: "fk_2e88fb7ce9", on_delete: :cascade
diff --git a/doc/workflow/lfs/manage_large_binaries_with_git_lfs.md b/doc/workflow/lfs/manage_large_binaries_with_git_lfs.md
index ce7895780c3..8fff3d591fe 100644
--- a/doc/workflow/lfs/manage_large_binaries_with_git_lfs.md
+++ b/doc/workflow/lfs/manage_large_binaries_with_git_lfs.md
@@ -83,6 +83,72 @@ that are on the remote repository, eg. from branch `master`:
git lfs fetch master
```
+## File Locking
+
+The first thing to do before using File Locking is to tell Git LFS which
+kind of files are lockable. The following command will store PNG files
+in LFS and flag them as lockable:
+
+```bash
+git lfs track "*.png" --lockable
+```
+
+After executing the above command a file named `.gitattributes` will be
+created or updated with the following content:
+
+```bash
+*.png filter=lfs diff=lfs merge=lfs -text lockable
+```
+
+You can also register a file type as lockable without using LFS
+(In order to be able to lock/unlock a file you need a remote server that implements the LFS File Locking API),
+in order to do that you can edit the `.gitattributes` file manually:
+
+```bash
+*.pdf lockable
+```
+
+After a file type has been registered as lockable, Git LFS will make
+them readonly on the file system automatically. This means you will
+need to lock the file before editing it.
+
+### Managing Locked Files
+
+Once you're ready to edit your file you need to lock it first:
+
+```bash
+git lfs lock images/banner.png
+Locked images/banner.png
+```
+
+This will register the file as locked in your name on the server:
+
+```bash
+git lfs locks
+images/banner.png joe ID:123
+```
+
+Once you have pushed your changes, you can unlock the file so others can
+also edit it:
+
+```bash
+git lfs unlock images/banner.png
+```
+
+You can also unlock by id:
+
+```bash
+git lfs unlock --id=123
+```
+
+If for some reason you need to unlock a file that was not locked by you,
+you can use the `--force` flag as long as you have a `master` access on
+the project:
+
+```bash
+git lfs unlock --id=123 --force
+```
+
## Troubleshooting
### error: Repository or object not found
diff --git a/lib/gitlab/checks/change_access.rb b/lib/gitlab/checks/change_access.rb
index 945d70e7a24..d75e73dac10 100644
--- a/lib/gitlab/checks/change_access.rb
+++ b/lib/gitlab/checks/change_access.rb
@@ -31,13 +31,14 @@ module Gitlab
@protocol = protocol
end
- def exec
+ def exec(skip_commits_check: false)
return true if skip_authorization
push_checks
branch_checks
tag_checks
lfs_objects_exist_check
+ commits_check unless skip_commits_check
true
end
@@ -117,6 +118,24 @@ module Gitlab
end
end
+ def commits_check
+ return if deletion? || newrev.nil?
+
+ # n+1: https://gitlab.com/gitlab-org/gitlab-ee/issues/3593
+ ::Gitlab::GitalyClient.allow_n_plus_1_calls do
+ commits.each do |commit|
+ commit_check.validate(commit, validations_for_commit(commit))
+ end
+ end
+
+ commit_check.validate_file_paths
+ end
+
+ # Method overwritten in EE to inject custom validations
+ def validations_for_commit(_)
+ []
+ end
+
private
def updated_from_web?
@@ -150,6 +169,14 @@ module Gitlab
raise GitAccess::UnauthorizedError, ERROR_MESSAGES[:lfs_objects_missing]
end
end
+
+ def commit_check
+ @commit_check ||= Gitlab::Checks::CommitCheck.new(project, user_access.user, newrev, oldrev)
+ end
+
+ def commits
+ project.repository.new_commits(newrev)
+ end
end
end
end
diff --git a/lib/gitlab/checks/commit_check.rb b/lib/gitlab/checks/commit_check.rb
new file mode 100644
index 00000000000..ae0cd142378
--- /dev/null
+++ b/lib/gitlab/checks/commit_check.rb
@@ -0,0 +1,61 @@
+module Gitlab
+ module Checks
+ class CommitCheck
+ include Gitlab::Utils::StrongMemoize
+
+ attr_reader :project, :user, :newrev, :oldrev
+
+ def initialize(project, user, newrev, oldrev)
+ @project = project
+ @user = user
+ @newrev = user
+ @oldrev = user
+ @file_paths = []
+ end
+
+ def validate(commit, validations)
+ return if validations.empty? && path_validations.empty?
+
+ commit.raw_deltas.each do |diff|
+ @file_paths << (diff.new_path || diff.old_path)
+
+ validations.each do |validation|
+ if error = validation.call(diff)
+ raise ::Gitlab::GitAccess::UnauthorizedError, error
+ end
+ end
+ end
+ end
+
+ def validate_file_paths
+ path_validations.each do |validation|
+ if error = validation.call(@file_paths)
+ raise ::Gitlab::GitAccess::UnauthorizedError, error
+ end
+ end
+ end
+
+ private
+
+ def validate_lfs_file_locks?
+ strong_memoize(:validate_lfs_file_locks) do
+ project.lfs_enabled? && project.lfs_file_locks.any? && newrev && oldrev
+ end
+ end
+
+ def lfs_file_locks_validation
+ lambda do |paths|
+ lfs_lock = project.lfs_file_locks.where(path: paths).where.not(user_id: user.id).first
+
+ if lfs_lock
+ return "The path '#{lfs_lock.path}' is locked in Git LFS by #{lfs_lock.user.name}"
+ end
+ end
+ end
+
+ def path_validations
+ validate_lfs_file_locks? ? [lfs_file_locks_validation] : []
+ end
+ end
+ end
+end
diff --git a/lib/gitlab/import_export/import_export.yml b/lib/gitlab/import_export/import_export.yml
index 2daed10f678..9f404003125 100644
--- a/lib/gitlab/import_export/import_export.yml
+++ b/lib/gitlab/import_export/import_export.yml
@@ -27,6 +27,8 @@ project_tree:
- :releases
- project_members:
- :user
+ - lfs_file_locks:
+ - :user
- merge_requests:
- notes:
- :author
diff --git a/spec/factories/lfs_file_locks.rb b/spec/factories/lfs_file_locks.rb
new file mode 100644
index 00000000000..b9d24f82b65
--- /dev/null
+++ b/spec/factories/lfs_file_locks.rb
@@ -0,0 +1,7 @@
+FactoryBot.define do
+ factory :lfs_file_lock do
+ user
+ project
+ path 'README.md'
+ end
+end
diff --git a/spec/lib/gitlab/checks/change_access_spec.rb b/spec/lib/gitlab/checks/change_access_spec.rb
index c2bca816aae..475b5c5cfb2 100644
--- a/spec/lib/gitlab/checks/change_access_spec.rb
+++ b/spec/lib/gitlab/checks/change_access_spec.rb
@@ -177,5 +177,44 @@ describe Gitlab::Checks::ChangeAccess do
expect { subject.exec }.not_to raise_error
end
end
+
+ context 'LFS file lock check' do
+ let(:owner) { create(:user) }
+ let!(:lock) { create(:lfs_file_lock, user: owner, project: project, path: 'README') }
+
+ before do
+ allow(project.repository).to receive(:new_commits).and_return(
+ project.repository.commits_between('be93687618e4b132087f430a4d8fc3a609c9b77c', '54fcc214b94e78d7a41a9a8fe6d87a5e59500e51')
+ )
+ end
+
+ context 'with LFS not enabled' do
+ it 'skips the validation' do
+ expect_any_instance_of(described_class).not_to receive(:lfs_file_locks_validation)
+
+ subject.exec
+ end
+ end
+
+ context 'with LFS enabled' do
+ before do
+ allow(project).to receive(:lfs_enabled?).and_return(true)
+ end
+
+ context 'when change is sent by a different user' do
+ it 'raises an error if the user is not allowed to update the file' do
+ expect { subject.exec }.to raise_error(Gitlab::GitAccess::UnauthorizedError, "The path 'README' is locked in Git LFS by #{lock.user.name}")
+ end
+ end
+
+ context 'when change is sent by the author od the lock' do
+ let(:user) { owner }
+
+ it "doesn't raise any error" do
+ expect { subject.exec }.not_to raise_error
+ end
+ end
+ end
+ end
end
end
diff --git a/spec/lib/gitlab/import_export/all_models.yml b/spec/lib/gitlab/import_export/all_models.yml
index 0ecb50f7110..41a55027f4d 100644
--- a/spec/lib/gitlab/import_export/all_models.yml
+++ b/spec/lib/gitlab/import_export/all_models.yml
@@ -276,6 +276,7 @@ project:
- fork_network_member
- fork_network
- custom_attributes
+- lfs_file_locks
award_emoji:
- awardable
- user
@@ -290,3 +291,5 @@ push_event_payload:
issue_assignees:
- issue
- assignee
+lfs_file_locks:
+- user
diff --git a/spec/lib/gitlab/import_export/safe_model_attributes.yml b/spec/lib/gitlab/import_export/safe_model_attributes.yml
index 5a33fa3fd53..feaab6673cd 100644
--- a/spec/lib/gitlab/import_export/safe_model_attributes.yml
+++ b/spec/lib/gitlab/import_export/safe_model_attributes.yml
@@ -530,3 +530,9 @@ ProjectCustomAttribute:
- project_id
- key
- value
+LfsFileLock:
+- id
+- path
+- user_id
+- project_id
+- created_at
diff --git a/spec/models/lfs_file_lock_spec.rb b/spec/models/lfs_file_lock_spec.rb
new file mode 100644
index 00000000000..ce87b01b49c
--- /dev/null
+++ b/spec/models/lfs_file_lock_spec.rb
@@ -0,0 +1,57 @@
+require 'rails_helper'
+
+describe LfsFileLock do
+ set(:lfs_file_lock) { create(:lfs_file_lock) }
+ subject { lfs_file_lock }
+
+ it { is_expected.to belong_to(:project) }
+ it { is_expected.to belong_to(:user) }
+
+ it { is_expected.to validate_presence_of(:project_id) }
+ it { is_expected.to validate_presence_of(:user_id) }
+ it { is_expected.to validate_presence_of(:path) }
+
+ describe '#can_be_unlocked_by?' do
+ let(:developer) { create(:user) }
+ let(:master) { create(:user) }
+
+ before do
+ project = lfs_file_lock.project
+
+ project.add_developer(developer)
+ project.add_master(master)
+ end
+
+ context "when it's forced" do
+ it 'can be unlocked by the author' do
+ user = lfs_file_lock.user
+
+ expect(lfs_file_lock.can_be_unlocked_by?(user, true)).to eq(true)
+ end
+
+ it 'can be unlocked by a master' do
+ expect(lfs_file_lock.can_be_unlocked_by?(master, true)).to eq(true)
+ end
+
+ it "can't be unlocked by other user" do
+ expect(lfs_file_lock.can_be_unlocked_by?(developer, true)).to eq(false)
+ end
+ end
+
+ context "when it isn't forced" do
+ it 'can be unlocked by the author' do
+ user = lfs_file_lock.user
+
+ expect(lfs_file_lock.can_be_unlocked_by?(user)).to eq(true)
+ end
+
+ it "can't be unlocked by a master" do
+ expect(lfs_file_lock.can_be_unlocked_by?(master)).to eq(false)
+ end
+
+ it "can't be unlocked by other user" do
+ expect(lfs_file_lock.can_be_unlocked_by?(developer)).to eq(false)
+ end
+ end
+ end
+end
diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb
index 9dca7f326d3..c6ca038a2ba 100644
--- a/spec/models/project_spec.rb
+++ b/spec/models/project_spec.rb
@@ -80,6 +80,7 @@ describe Project do
it { is_expected.to have_many(:members_and_requesters) }
it { is_expected.to have_many(:clusters) }
it { is_expected.to have_many(:custom_attributes).class_name('ProjectCustomAttribute') }
+ it { is_expected.to have_many(:lfs_file_locks) }
context 'after initialized' do
it "has a project_feature" do
diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb
index 02a5ee54262..a6d48e369ac 100644
--- a/spec/models/repository_spec.rb
+++ b/spec/models/repository_spec.rb
@@ -262,6 +262,28 @@ describe Repository do
end
end
+ describe '#new_commits' do
+ let(:new_refs) do
+ double(:git_rev_list, new_refs: %w[
+ c1acaa58bbcbc3eafe538cb8274ba387047b69f8
+ 5937ac0a7beb003549fc5fd26fc247adbce4a52e
+ ])
+ end
+
+ it 'delegates to Gitlab::Git::RevList' do
+ expect(Gitlab::Git::RevList).to receive(:new).with(
+ repository.raw,
+ newrev: 'aaaabbbbccccddddeeeeffffgggghhhhiiiijjjj').and_return(new_refs)
+
+ commits = repository.new_commits('aaaabbbbccccddddeeeeffffgggghhhhiiiijjjj')
+
+ expect(commits).to eq([
+ repository.commit('c1acaa58bbcbc3eafe538cb8274ba387047b69f8'),
+ repository.commit('5937ac0a7beb003549fc5fd26fc247adbce4a52e')
+ ])
+ end
+ end
+
describe '#commits_by' do
set(:project) { create(:project, :repository) }
diff --git a/spec/requests/lfs_http_spec.rb b/spec/requests/lfs_http_spec.rb
index 930ef49b7f3..971b45c411d 100644
--- a/spec/requests/lfs_http_spec.rb
+++ b/spec/requests/lfs_http_spec.rb
@@ -1208,7 +1208,7 @@ describe 'Git LFS API and storage' do
end
def post_lfs_json(url, body = nil, headers = nil)
- post(url, body.try(:to_json), (headers || {}).merge('Content-Type' => 'application/vnd.git-lfs+json'))
+ post(url, body.try(:to_json), (headers || {}).merge('Content-Type' => LfsRequest::CONTENT_TYPE))
end
def json_response
diff --git a/spec/requests/lfs_locks_api_spec.rb b/spec/requests/lfs_locks_api_spec.rb
new file mode 100644
index 00000000000..e44a11a7232
--- /dev/null
+++ b/spec/requests/lfs_locks_api_spec.rb
@@ -0,0 +1,159 @@
+require 'spec_helper'
+
+describe 'Git LFS File Locking API' do
+ include WorkhorseHelpers
+
+ let(:project) { create(:project) }
+ let(:master) { create(:user) }
+ let(:developer) { create(:user) }
+ let(:guest) { create(:user) }
+ let(:path) { 'README.md' }
+ let(:headers) do
+ {
+ 'Authorization' => authorization
+ }.compact
+ end
+
+ shared_examples 'unauthorized request' do
+ context 'when user is not authorized' do
+ let(:authorization) { authorize_user(guest) }
+
+ it 'returns a forbidden 403 response' do
+ post_lfs_json url, body, headers
+
+ expect(response).to have_gitlab_http_status(403)
+ end
+ end
+ end
+
+ before do
+ allow(Gitlab.config.lfs).to receive(:enabled).and_return(true)
+
+ project.add_developer(master)
+ project.add_developer(developer)
+ project.add_guest(guest)
+ end
+
+ describe 'Create File Lock endpoint' do
+ let(:url) { "#{project.http_url_to_repo}/info/lfs/locks" }
+ let(:authorization) { authorize_user(developer) }
+ let(:body) { { path: path } }
+
+ include_examples 'unauthorized request'
+
+ context 'with an existent lock' do
+ before do
+ lock_file('README.md', developer)
+ end
+
+ it 'return an error message' do
+ post_lfs_json url, body, headers
+
+ expect(response).to have_gitlab_http_status(409)
+
+ expect(json_response.keys).to match_array(%w(lock message documentation_url))
+ expect(json_response['message']).to match(/already locked/)
+ end
+
+ it 'returns the existen lock' do
+ post_lfs_json url, body, headers
+
+ expect(json_response['lock']['path']).to eq('README.md')
+ end
+ end
+
+ context 'without an existent lock' do
+ it 'creates the lock' do
+ post_lfs_json url, body, headers
+
+ expect(response).to have_gitlab_http_status(201)
+
+ expect(json_response['lock'].keys).to match_array(%w(id path locked_at owner))
+ end
+ end
+ end
+
+ describe 'Listing File Locks endpoint' do
+ let(:url) { "#{project.http_url_to_repo}/info/lfs/locks" }
+ let(:authorization) { authorize_user(developer) }
+
+ include_examples 'unauthorized request'
+
+ it 'returns the list of locked files' do
+ lock_file('README.md', developer)
+ lock_file('README', developer)
+
+ do_get url, nil, headers
+
+ expect(response).to have_gitlab_http_status(200)
+
+ expect(json_response['locks'].size).to eq(2)
+ expect(json_response['locks'].first.keys).to match_array(%w(id path locked_at owner))
+ end
+ end
+
+ describe 'List File Locks for verification endpoint' do
+ let(:url) { "#{project.http_url_to_repo}/info/lfs/locks/verify" }
+ let(:authorization) { authorize_user(developer) }
+
+ include_examples 'unauthorized request'
+
+ it 'returns the list of locked files grouped by owner' do
+ lock_file('README.md', master)
+ lock_file('README', developer)
+
+ post_lfs_json url, nil, headers
+
+ expect(response).to have_gitlab_http_status(200)
+
+ expect(json_response['ours'].size).to eq(1)
+ expect(json_response['ours'].first['path']).to eq('README')
+ expect(json_response['theirs'].size).to eq(1)
+ expect(json_response['theirs'].first['path']).to eq('README.md')
+ end
+ end
+
+ describe 'Delete File Lock endpoint' do
+ let!(:lock) { lock_file('README.md', developer) }
+ let(:url) { "#{project.http_url_to_repo}/info/lfs/locks/#{lock[:id]}/unlock" }
+ let(:authorization) { authorize_user(developer) }
+
+ include_examples 'unauthorized request'
+
+ context 'with an existent lock' do
+ it 'deletes the lock' do
+ post_lfs_json url, nil, headers
+
+ expect(response).to have_gitlab_http_status(200)
+ end
+
+ it 'returns the deleted lock' do
+ post_lfs_json url, nil, headers
+
+ expect(json_response['lock'].keys).to match_array(%w(id path locked_at owner))
+ end
+ end
+ end
+
+ def lock_file(path, author)
+ result = Lfs::LockFileService.new(project, author, { path: path }).execute
+
+ result[:lock]
+ end
+
+ def authorize_user(user)
+ ActionController::HttpAuthentication::Basic.encode_credentials(user.username, user.password)
+ end
+
+ def post_lfs_json(url, body = nil, headers = nil)
+ post(url, body.try(:to_json), (headers || {}).merge('Content-Type' => LfsRequest::CONTENT_TYPE))
+ end
+
+ def do_get(url, params = nil, headers = nil)
+ get(url, (params || {}), (headers || {}).merge('Content-Type' => LfsRequest::CONTENT_TYPE))
+ end
+
+ def json_response
+ @json_response ||= JSON.parse(response.body)
+ end
+end
diff --git a/spec/serializers/lfs_file_lock_entity_spec.rb b/spec/serializers/lfs_file_lock_entity_spec.rb
new file mode 100644
index 00000000000..5919f473a90
--- /dev/null
+++ b/spec/serializers/lfs_file_lock_entity_spec.rb
@@ -0,0 +1,19 @@
+require 'spec_helper'
+
+describe LfsFileLockEntity do
+ let(:user) { create(:user) }
+ let(:resource) { create(:lfs_file_lock, user: user) }
+
+ let(:request) { double('request', current_user: user) }
+
+ subject { described_class.new(resource, request: request).as_json }
+
+ it 'exposes basic attrs of the lock' do
+ expect(subject).to include(:id, :path, :locked_at)
+ end
+
+ it 'exposes the owner info' do
+ expect(subject).to include(:owner)
+ expect(subject[:owner][:name]).to eq(user.name)
+ end
+end
diff --git a/spec/services/lfs/lock_file_service_spec.rb b/spec/services/lfs/lock_file_service_spec.rb
new file mode 100644
index 00000000000..3e58eea2501
--- /dev/null
+++ b/spec/services/lfs/lock_file_service_spec.rb
@@ -0,0 +1,62 @@
+require 'spec_helper'
+
+describe Lfs::LockFileService do
+ let(:project) { create(:project) }
+ let(:current_user) { create(:user) }
+
+ subject { described_class.new(project, current_user, params) }
+
+ describe '#execute' do
+ let(:params) { { path: 'README.md' } }
+
+ context 'when not authorized' do
+ it "doesn't succeed" do
+ result = subject.execute
+
+ expect(result[:status]).to eq(:error)
+ expect(result[:http_status]).to eq(403)
+ expect(result[:message]).to eq('You have no permissions')
+ end
+ end
+
+ context 'when authorized' do
+ before do
+ project.add_developer(current_user)
+ end
+
+ context 'with an existent lock' do
+ let!(:lock) { create(:lfs_file_lock, project: project) }
+
+ it "doesn't succeed" do
+ expect(subject.execute[:status]).to eq(:error)
+ end
+
+ it "doesn't create the Lock" do
+ expect do
+ subject.execute
+ end.not_to change { LfsFileLock.count }
+ end
+ end
+
+ context 'without an existent lock' do
+ it "succeeds" do
+ expect(subject.execute[:status]).to eq(:success)
+ end
+
+ it "creates the Lock" do
+ expect do
+ subject.execute
+ end.to change { LfsFileLock.count }.by(1)
+ end
+ end
+
+ context 'when an error is raised' do
+ it "doesn't succeed" do
+ allow_any_instance_of(described_class).to receive(:create_lock!).and_raise(StandardError)
+
+ expect(subject.execute[:status]).to eq(:error)
+ end
+ end
+ end
+ end
+end
diff --git a/spec/services/lfs/locks_finder_service_spec.rb b/spec/services/lfs/locks_finder_service_spec.rb
new file mode 100644
index 00000000000..e409b77babf
--- /dev/null
+++ b/spec/services/lfs/locks_finder_service_spec.rb
@@ -0,0 +1,101 @@
+require 'spec_helper'
+
+describe Lfs::LocksFinderService do
+ let(:project) { create(:project) }
+ let(:user) { create(:user) }
+ let(:params) { {} }
+
+ subject { described_class.new(project, user, params) }
+
+ shared_examples 'no results' do
+ it 'returns an empty list' do
+ result = subject.execute
+
+ expect(result[:status]).to eq(:success)
+ expect(result[:locks]).to be_blank
+ end
+ end
+
+ describe '#execute' do
+ let!(:lock_1) { create(:lfs_file_lock, project: project) }
+ let!(:lock_2) { create(:lfs_file_lock, project: project, path: 'README') }
+
+ context 'find by id' do
+ context 'with results' do
+ let(:params) do
+ { id: lock_1.id }
+ end
+
+ it 'returns the record' do
+ result = subject.execute
+
+ expect(result[:status]).to eq(:success)
+ expect(result[:locks].size).to eq(1)
+ expect(result[:locks].first).to eq(lock_1)
+ end
+ end
+
+ context 'without results' do
+ let(:params) do
+ { id: 123 }
+ end
+
+ include_examples 'no results'
+ end
+ end
+
+ context 'find by path' do
+ context 'with results' do
+ let(:params) do
+ { path: lock_1.path }
+ end
+
+ it 'returns the record' do
+ result = subject.execute
+
+ expect(result[:status]).to eq(:success)
+ expect(result[:locks].size).to eq(1)
+ expect(result[:locks].first).to eq(lock_1)
+ end
+ end
+
+ context 'without results' do
+ let(:params) do
+ { path: 'not-found' }
+ end
+
+ include_examples 'no results'
+ end
+ end
+
+ context 'find all' do
+ context 'with results' do
+ it 'returns all the records' do
+ result = subject.execute
+
+ expect(result[:status]).to eq(:success)
+ expect(result[:locks].size).to eq(2)
+ end
+ end
+
+ context 'without results' do
+ before do
+ LfsFileLock.delete_all
+ end
+
+ include_examples 'no results'
+ end
+ end
+
+ context 'when an error is raised' do
+ it "doesn't succeed" do
+ allow_any_instance_of(described_class).to receive(:find_locks).and_raise(StandardError)
+
+ result = subject.execute
+
+ expect(result[:status]).to eq(:error)
+ expect(result[:locks]).to be_blank
+ end
+ end
+ end
+end
diff --git a/spec/services/lfs/unlock_file_service_spec.rb b/spec/services/lfs/unlock_file_service_spec.rb
new file mode 100644
index 00000000000..4bea112b9c6
--- /dev/null
+++ b/spec/services/lfs/unlock_file_service_spec.rb
@@ -0,0 +1,105 @@
+require 'spec_helper'
+
+describe Lfs::UnlockFileService do
+ let(:project) { create(:project) }
+ let(:current_user) { create(:user) }
+ let(:lock_author) { create(:user) }
+ let!(:lock) { create(:lfs_file_lock, user: lock_author, project: project) }
+ let(:params) { {} }
+
+ subject { described_class.new(project, current_user, params) }
+
+ describe '#execute' do
+ context 'when not authorized' do
+ it "doesn't succeed" do
+ result = subject.execute
+
+ expect(result[:status]).to eq(:error)
+ expect(result[:http_status]).to eq(403)
+ expect(result[:message]).to eq('You have no permissions')
+ end
+ end
+
+ context 'when authorized' do
+ before do
+ project.add_developer(current_user)
+ end
+
+ context 'when lock does not exists' do
+ let(:params) { { id: 123 } }
+ it "doesn't succeed" do
+ result = subject.execute
+
+ expect(result[:status]).to eq(:error)
+ expect(result[:http_status]).to eq(404)
+ end
+ end
+
+ context 'when unlocked by the author' do
+ let(:current_user) { lock_author }
+ let(:params) { { id: lock.id } }
+
+ it "succeeds" do
+ result = subject.execute
+
+ expect(result[:status]).to eq(:success)
+ expect(result[:lock]).to be_present
+ end
+ end
+
+ context 'when unlocked by a different user' do
+ let(:current_user) { create(:user) }
+ let(:params) { { id: lock.id } }
+
+ it "doesn't succeed" do
+ result = subject.execute
+
+ expect(result[:status]).to eq(:error)
+ expect(result[:message]).to match(/is locked by GitLab User #{lock_author.id}/)
+ expect(result[:http_status]).to eq(403)
+ end
+ end
+
+ context 'when forced' do
+ let(:developer) { create(:user) }
+ let(:master) { create(:user) }
+
+ before do
+ project.add_developer(developer)
+ project.add_master(master)
+ end
+
+ context 'by a regular user' do
+ let(:current_user) { developer }
+ let(:params) do
+ { id: lock.id,
+ force: true }
+ end
+
+ it "doesn't succeed" do
+ result = subject.execute
+
+ expect(result[:status]).to eq(:error)
+ expect(result[:message]).to match(/You must have master access/)
+ expect(result[:http_status]).to eq(403)
+ end
+ end
+
+ context 'by a master user' do
+ let(:current_user) { master }
+ let(:params) do
+ { id: lock.id,
+ force: true }
+ end
+
+ it "succeeds" do
+ result = subject.execute
+
+ expect(result[:status]).to eq(:success)
+ expect(result[:lock]).to be_present
+ end
+ end
+ end
+ end
+ end
+end