diff --git a/app/controllers/concerns/lfs_request.rb b/app/controllers/concerns/lfs_request.rb index 61072eec5359d6b90818c9f90d2e96f01129e2d1..3152d959ae4cb81d7f275970a00cf5694036ace3 100644 --- a/app/controllers/concerns/lfs_request.rb +++ b/app/controllers/concerns/lfs_request.rb @@ -112,10 +112,6 @@ def build_can_download_code? has_authentication_ability?(:build_download_code) && can?(user, :build_download_code, project) end - def storage_project - @storage_project ||= project.lfs_storage_project - end - def objects @objects ||= (params[:objects] || []).to_a end diff --git a/app/controllers/repositories/lfs_storage_controller.rb b/app/controllers/repositories/lfs_storage_controller.rb index 58f496e16d3d8106d86ea51bb6cf8ce5e2181b73..ec5ca5bbeecc172c8f82cd1347b35190c5741f85 100644 --- a/app/controllers/repositories/lfs_storage_controller.rb +++ b/app/controllers/repositories/lfs_storage_controller.rb @@ -80,12 +80,13 @@ def create_file!(oid, size) LfsObject.create!(oid: oid, size: size, file: uploaded_file) end - # rubocop: disable CodeReuse/ActiveRecord def link_to_project!(object) - if object && !object.projects.exists?(storage_project.id) - object.lfs_objects_projects.create!(project: storage_project) - end + return unless object + + LfsObjectsProject.safe_find_or_create_by!( + project: project, + lfs_object: object + ) end - # rubocop: enable CodeReuse/ActiveRecord end end diff --git a/app/models/project.rb b/app/models/project.rb index 816d964519d02dc3ba4530184dc0cb4c888d7444..4674dca53887efa1afaeb3197b750a9393c940cd 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -1357,6 +1357,10 @@ def fork_source forked_from_project || fork_network&.root_project end + # TODO: Remove this method once all LfsObjectsProject records are backfilled + # for forks. + # + # See https://gitlab.com/gitlab-org/gitlab/issues/122002 for more info. def lfs_storage_project @lfs_storage_project ||= begin result = self @@ -1369,14 +1373,27 @@ def lfs_storage_project end end - # This will return all `lfs_objects` that are accessible to the project. - # So this might be `self.lfs_objects` if the project is not part of a fork - # network, or it is the base of the fork network. + # This will return all `lfs_objects` that are accessible to the project and + # the fork source. This is needed since older forks won't have access to some + # LFS objects directly and have to get it from the fork source. + # + # TODO: Remove this method once all LfsObjectsProject records are backfilled + # for forks. At that point, projects can look at their own `lfs_objects`. # - # TODO: refactor this to get the correct lfs objects when implementing - # https://gitlab.com/gitlab-org/gitlab-foss/issues/39769 + # See https://gitlab.com/gitlab-org/gitlab/issues/122002 for more info. def all_lfs_objects - lfs_storage_project.lfs_objects + LfsObject + .distinct + .joins(:lfs_objects_projects) + .where(lfs_objects_projects: { project_id: [self, lfs_storage_project] }) + end + + # TODO: Call `#lfs_objects` instead once all LfsObjectsProject records are + # backfilled. At that point, projects can look at their own `lfs_objects`. + # + # See https://gitlab.com/gitlab-org/gitlab/issues/122002 for more info. + def lfs_objects_oids + all_lfs_objects.pluck(:oid) end def personal? diff --git a/app/services/merge_requests/create_service.rb b/app/services/merge_requests/create_service.rb index 9a37a0330fc19d2f2abd82f0afdf3f192eeb7b65..4a05d1fd7efac3ed59cf814b400b043e652432e1 100644 --- a/app/services/merge_requests/create_service.rb +++ b/app/services/merge_requests/create_service.rb @@ -27,6 +27,7 @@ def after_create(issuable) create_pipeline_for(issuable, current_user) issuable.update_head_pipeline Gitlab::UsageDataCounters::MergeRequestCounter.count(:create) + link_lfs_objects(issuable) super end @@ -64,6 +65,10 @@ def set_projects! raise Gitlab::Access::AccessDeniedError end end + + def link_lfs_objects(issuable) + LinkLfsObjectsService.new(issuable.target_project).execute(issuable) + end end end diff --git a/app/services/merge_requests/link_lfs_objects_service.rb b/app/services/merge_requests/link_lfs_objects_service.rb new file mode 100644 index 0000000000000000000000000000000000000000..191da59409548eb00cb9e6163d6282cce0e1247f --- /dev/null +++ b/app/services/merge_requests/link_lfs_objects_service.rb @@ -0,0 +1,31 @@ +# frozen_string_literal: true + +module MergeRequests + class LinkLfsObjectsService < ::BaseService + def execute(merge_request, oldrev: merge_request.diff_base_sha, newrev: merge_request.diff_head_sha) + return if merge_request.source_project == project + return if no_changes?(oldrev, newrev) + + new_lfs_oids = lfs_oids(merge_request.source_project.repository, oldrev, newrev) + + return if new_lfs_oids.empty? + + Projects::LfsPointers::LfsLinkService + .new(project) + .execute(new_lfs_oids) + end + + private + + def no_changes?(oldrev, newrev) + oldrev == newrev + end + + def lfs_oids(source_repository, oldrev, newrev) + Gitlab::Git::LfsChanges + .new(source_repository, newrev) + .new_pointers(not_in: [oldrev]) + .map(&:lfs_oid) + end + end +end diff --git a/app/services/merge_requests/refresh_service.rb b/app/services/merge_requests/refresh_service.rb index 396ddec6383db2f30167146ea5d646040ac8345f..c6e1651fa262f78cca18b2782edc02dcb6e477cd 100644 --- a/app/services/merge_requests/refresh_service.rb +++ b/app/services/merge_requests/refresh_service.rb @@ -21,6 +21,7 @@ def refresh_merge_requests! # empty diff during a manual merge close_upon_missing_source_branch_ref post_merge_manually_merged + link_forks_lfs_objects reload_merge_requests outdate_suggestions refresh_pipelines_on_merge_requests @@ -91,17 +92,25 @@ def post_merge_manually_merged end # rubocop: enable CodeReuse/ActiveRecord + # Link LFS objects that exists in forks but does not exists in merge requests + # target project + def link_forks_lfs_objects + return unless @push.branch_updated? + + merge_requests_for_forks.find_each do |mr| + LinkLfsObjectsService + .new(mr.target_project) + .execute(mr, oldrev: @push.oldrev, newrev: @push.newrev) + end + end + # Refresh merge request diff if we push to source or target branch of merge request # Note: we should update merge requests from forks too - # rubocop: disable CodeReuse/ActiveRecord def reload_merge_requests merge_requests = @project.merge_requests.opened .by_source_or_target_branch(@push.branch_name).to_a - # Fork merge requests - merge_requests += MergeRequest.opened - .where(source_branch: @push.branch_name, source_project: @project) - .where.not(target_project: @project).to_a + merge_requests += merge_requests_for_forks.to_a filter_merge_requests(merge_requests).each do |merge_request| if branch_and_project_match?(merge_request) || @push.force_push? @@ -117,7 +126,6 @@ def reload_merge_requests # @source_merge_requests diffs (for MergeRequest#commit_shas for instance). merge_requests_for_source_branch(reload: true) end - # rubocop: enable CodeReuse/ActiveRecord def push_commit_ids @push_commit_ids ||= @commits.map(&:id) @@ -282,6 +290,15 @@ def merge_requests_for_source_branch(reload: false) @source_merge_requests = nil if reload @source_merge_requests ||= merge_requests_for(@push.branch_name) end + + # rubocop: disable CodeReuse/ActiveRecord + def merge_requests_for_forks + @merge_requests_for_forks ||= + MergeRequest.opened + .where(source_branch: @push.branch_name, source_project: @project) + .where.not(target_project: @project) + end + # rubocop: enable CodeReuse/ActiveRecord end end diff --git a/app/services/projects/fork_service.rb b/app/services/projects/fork_service.rb index e66a0ed181a4a146f48463734b7ca508c79c59cf..fcfea56788510aaf8d70a5082f075abb7a3d9059 100644 --- a/app/services/projects/fork_service.rb +++ b/app/services/projects/fork_service.rb @@ -26,17 +26,7 @@ def link_existing_project(fork_to_project) build_fork_network_member(fork_to_project) - if link_fork_network(fork_to_project) - # A forked project stores its LFS objects in the `forked_from_project`. - # So the LFS objects become inaccessible, and therefore delete them from - # the database so they'll get cleaned up. - # - # TODO: refactor this to get the correct lfs objects when implementing - # https://gitlab.com/gitlab-org/gitlab-foss/issues/39769 - fork_to_project.lfs_objects_projects.delete_all - - fork_to_project - end + fork_to_project if link_fork_network(fork_to_project) end def fork_new_project diff --git a/app/services/projects/lfs_pointers/lfs_download_service.rb b/app/services/projects/lfs_pointers/lfs_download_service.rb index a009f479d5d46d5a796d916aad09aad6414ac252..bd70012c76cb1347b6402e7606144930681a9e80 100644 --- a/app/services/projects/lfs_pointers/lfs_download_service.rb +++ b/app/services/projects/lfs_pointers/lfs_download_service.rb @@ -39,9 +39,9 @@ def wrap_download_errors(&block) def download_lfs_file! with_tmp_file do |tmp_file| download_and_save_file!(tmp_file) - project.all_lfs_objects << LfsObject.new(oid: lfs_oid, - size: lfs_size, - file: tmp_file) + project.lfs_objects << LfsObject.new(oid: lfs_oid, + size: lfs_size, + file: tmp_file) success end diff --git a/app/services/projects/move_lfs_objects_projects_service.rb b/app/services/projects/move_lfs_objects_projects_service.rb index 10e19014db49136eeec958726e3799e6b5b9e8fb..8cc420d7ba7ca3ebec2437aaa08beed3b3b6eb33 100644 --- a/app/services/projects/move_lfs_objects_projects_service.rb +++ b/app/services/projects/move_lfs_objects_projects_service.rb @@ -16,7 +16,7 @@ def execute(source_project, remove_remaining_elements: true) private def move_lfs_objects_projects - non_existent_lfs_objects_projects.update_all(project_id: @project.lfs_storage_project.id) + non_existent_lfs_objects_projects.update_all(project_id: @project.id) end def remove_remaining_lfs_objects_project diff --git a/app/services/projects/unlink_fork_service.rb b/app/services/projects/unlink_fork_service.rb index e7e0141099e7d61c28cadb161407dc39b8abc708..b3cf27373cde13c9dce3e2aee232d63a170f51ef 100644 --- a/app/services/projects/unlink_fork_service.rb +++ b/app/services/projects/unlink_fork_service.rb @@ -52,6 +52,10 @@ def refresh_forks_count(project) Projects::ForksCountService.new(project).refresh_cache end + # TODO: Remove this method once all LfsObjectsProject records are backfilled + # for forks. + # + # See https://gitlab.com/gitlab-org/gitlab/issues/122002 for more info. def save_lfs_objects return unless @project.forked? diff --git a/app/workers/repository_fork_worker.rb b/app/workers/repository_fork_worker.rb index 0adf745c7aca16772473dd2cc4624932303d1235..ba141f808a768e00d14e3c8e99d0b3bcccffda8f 100644 --- a/app/workers/repository_fork_worker.rb +++ b/app/workers/repository_fork_worker.rb @@ -29,7 +29,15 @@ def fork_repository(target_project, source_project) result = gitlab_shell.fork_repository(source_project, target_project) - raise "Unable to fork project #{target_project.id} for repository #{source_project.disk_path} -> #{target_project.disk_path}" unless result + if result + link_lfs_objects(source_project, target_project) + else + raise_fork_failure( + source_project, + target_project, + 'Failed to create fork repository' + ) + end target_project.after_import end @@ -40,4 +48,20 @@ def start_fork(project) Rails.logger.info("Project #{project.full_path} was in inconsistent state (#{project.import_status}) while forking.") # rubocop:disable Gitlab/RailsLogger false end + + def link_lfs_objects(source_project, target_project) + Projects::LfsPointers::LfsLinkService + .new(target_project) + .execute(source_project.lfs_objects_oids) + rescue Projects::LfsPointers::LfsLinkService::TooManyOidsError + raise_fork_failure( + source_project, + target_project, + 'Source project has too many LFS objects' + ) + end + + def raise_fork_failure(source_project, target_project, reason) + raise "Unable to fork project #{target_project.id} for repository #{source_project.disk_path} -> #{target_project.disk_path}: #{reason}" + end end diff --git a/changelogs/unreleased/20042-create-lfs-objects-project.yml b/changelogs/unreleased/20042-create-lfs-objects-project.yml new file mode 100644 index 0000000000000000000000000000000000000000..0f942e11670c6f671fabbdd69c60b219f78270d3 --- /dev/null +++ b/changelogs/unreleased/20042-create-lfs-objects-project.yml @@ -0,0 +1,5 @@ +--- +title: Create LfsObjectsProject record for forks as well +merge_request: 22418 +author: +type: fixed diff --git a/db/migrate/20200123040535_add_multi_column_index_on_lfs_objects_projects.rb b/db/migrate/20200123040535_add_multi_column_index_on_lfs_objects_projects.rb new file mode 100644 index 0000000000000000000000000000000000000000..36ccfa4743eee8a969a37aa1f59151041ee5751c --- /dev/null +++ b/db/migrate/20200123040535_add_multi_column_index_on_lfs_objects_projects.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +class AddMultiColumnIndexOnLfsObjectsProjects < ActiveRecord::Migration[5.2] + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + disable_ddl_transaction! + + def up + add_concurrent_index :lfs_objects_projects, [:project_id, :lfs_object_id] + end + + def down + remove_concurrent_index :lfs_objects_projects, [:project_id, :lfs_object_id] + end +end diff --git a/db/migrate/20200123045415_remove_project_id_index_on_lfs_objects_projects.rb b/db/migrate/20200123045415_remove_project_id_index_on_lfs_objects_projects.rb new file mode 100644 index 0000000000000000000000000000000000000000..540383e8808e48fbc7430377a5de0d54db84db4c --- /dev/null +++ b/db/migrate/20200123045415_remove_project_id_index_on_lfs_objects_projects.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +class RemoveProjectIdIndexOnLfsObjectsProjects < ActiveRecord::Migration[5.2] + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + disable_ddl_transaction! + + def up + remove_concurrent_index :lfs_objects_projects, :project_id + end + + def down + add_concurrent_index :lfs_objects_projects, :project_id + end +end diff --git a/db/schema.rb b/db/schema.rb index 354224617413dd659c0e67da6df02a4632e23506..9402b095b93d9ed49e4c7bc1faa2c1711f4660c4 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -2347,7 +2347,7 @@ t.datetime "updated_at" t.integer "repository_type", limit: 2 t.index ["lfs_object_id"], name: "index_lfs_objects_projects_on_lfs_object_id" - t.index ["project_id"], name: "index_lfs_objects_projects_on_project_id" + t.index ["project_id", "lfs_object_id"], name: "index_lfs_objects_projects_on_project_id_and_lfs_object_id" end create_table "licenses", id: :serial, force: :cascade do |t| diff --git a/doc/api/projects.md b/doc/api/projects.md index dd80e3ce8f3dcb66ffbb17070e2f1e5125f9834e..a0243be1907bbbe9fc6e7675ed57d56da9a7738e 100644 --- a/doc/api/projects.md +++ b/doc/api/projects.md @@ -2000,11 +2000,6 @@ Allows modification of the forked relationship between existing projects. Availa ### Create a forked from/to relation between existing projects -CAUTION: **Warning:** -This will destroy the LFS objects stored in the fork. -So to retain the LFS objects, make sure you've pulled them **before** creating the fork relation, -and push them again **after** creating the fork relation. - ``` POST /projects/:id/fork/:forked_from_id ``` diff --git a/spec/controllers/concerns/lfs_request_spec.rb b/spec/controllers/concerns/lfs_request_spec.rb index 79257e9a7f64e08b4b77b76af5231f1c9728a9b7..67c81156ca6c428bdb5c7a0b0dac0036ec6b886c 100644 --- a/spec/controllers/concerns/lfs_request_spec.rb +++ b/spec/controllers/concerns/lfs_request_spec.rb @@ -10,8 +10,6 @@ include LfsRequest def show - storage_project - head :ok end @@ -38,22 +36,6 @@ def ci? stub_lfs_setting(enabled: true) end - describe '#storage_project' do - it 'assigns the project as storage project' do - get :show, params: { id: project.id } - - expect(assigns(:storage_project)).to eq(project) - end - - it 'assigns the source of a forked project' do - forked_project = fork_project(project) - - get :show, params: { id: forked_project.id } - - expect(assigns(:storage_project)).to eq(project) - end - end - context 'user is authenticated without access to lfs' do before do allow(controller).to receive(:authenticate_user) diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index ae4db1c2158a6f2f12e040e2cc6f67ac0337335b..a95ac4dca049557e31ef02fa4ad3097235ded309 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -2694,16 +2694,44 @@ describe '#all_lfs_objects' do let(:lfs_object) { create(:lfs_object) } - before do - project.lfs_objects << lfs_object + context 'when LFS object is only associated to the source' do + before do + project.lfs_objects << lfs_object + end + + it 'returns the lfs object for a project' do + expect(project.all_lfs_objects).to contain_exactly(lfs_object) + end + + it 'returns the lfs object for a fork' do + expect(forked_project.all_lfs_objects).to contain_exactly(lfs_object) + end end - it 'returns the lfs object for a project' do - expect(project.all_lfs_objects).to contain_exactly(lfs_object) + context 'when LFS object is only associated to the fork' do + before do + forked_project.lfs_objects << lfs_object + end + + it 'returns nothing' do + expect(project.all_lfs_objects).to be_empty + end + + it 'returns the lfs object for a fork' do + expect(forked_project.all_lfs_objects).to contain_exactly(lfs_object) + end end - it 'returns the lfs object for a fork' do - expect(forked_project.all_lfs_objects).to contain_exactly(lfs_object) + context 'when LFS object is associated to both source and fork' do + before do + project.lfs_objects << lfs_object + forked_project.lfs_objects << lfs_object + end + + it 'returns the lfs object for the source and fork' do + expect(project.all_lfs_objects).to contain_exactly(lfs_object) + expect(forked_project.all_lfs_objects).to contain_exactly(lfs_object) + end end end end @@ -5519,6 +5547,31 @@ def enable_lfs end end + describe '#lfs_objects_oids' do + let(:project) { create(:project) } + let(:lfs_object) { create(:lfs_object) } + let(:another_lfs_object) { create(:lfs_object) } + + subject { project.lfs_objects_oids } + + context 'when project has associated LFS objects' do + before do + create(:lfs_objects_project, lfs_object: lfs_object, project: project) + create(:lfs_objects_project, lfs_object: another_lfs_object, project: project) + end + + it 'returns OIDs of LFS objects' do + expect(subject).to match_array([lfs_object.oid, another_lfs_object.oid]) + end + end + + context 'when project has no associated LFS objects' do + it 'returns empty array' do + expect(subject).to be_empty + end + end + end + def rugged_config rugged_repo(project.repository).config end diff --git a/spec/requests/api/merge_requests_spec.rb b/spec/requests/api/merge_requests_spec.rb index 245a8aa49052ef6fd5340dfa79d59e300fda14ed..427a361295c5c161654c7cd7e340f6dd89ed17e8 100644 --- a/spec/requests/api/merge_requests_spec.rb +++ b/spec/requests/api/merge_requests_spec.rb @@ -1492,7 +1492,7 @@ end end - context 'forked projects' do + context 'forked projects', :sidekiq_might_not_need_inline do let!(:user2) { create(:user) } let(:project) { create(:project, :public, :repository) } let!(:forked_project) { fork_project(project, user2, repository: true) } diff --git a/spec/requests/lfs_http_spec.rb b/spec/requests/lfs_http_spec.rb index 0e02c0f001b38ed5eb4f4c044edf432fda0d8dc9..4e21c08ad5c1aa87d7588e9945356d6a6aef55e8 100644 --- a/spec/requests/lfs_http_spec.rb +++ b/spec/requests/lfs_http_spec.rb @@ -1193,8 +1193,8 @@ def authorization_in_action(action) it_behaves_like 'LFS http 200 response' - it 'LFS object is linked to the source project' do - expect(lfs_object.projects.pluck(:id)).to include(upstream_project.id) + it 'LFS object is linked to the forked project' do + expect(lfs_object.projects.pluck(:id)).to include(project.id) end end end diff --git a/spec/services/merge_requests/create_service_spec.rb b/spec/services/merge_requests/create_service_spec.rb index 3db1471bf3c3e18f1f987c2e3a364750839f0dc8..8490127058c2377e050695d4bc38c9c82755551f 100644 --- a/spec/services/merge_requests/create_service_spec.rb +++ b/spec/services/merge_requests/create_service_spec.rb @@ -483,6 +483,14 @@ expect(merge_request).to be_persisted end + it 'calls MergeRequests::LinkLfsObjectsService#execute', :sidekiq_might_not_need_inline do + expect_next_instance_of(MergeRequests::LinkLfsObjectsService) do |service| + expect(service).to receive(:execute).with(instance_of(MergeRequest)) + end + + described_class.new(project, user, opts).execute + end + it 'does not create the merge request when the target project is archived' do target_project.update!(archived: true) diff --git a/spec/services/merge_requests/link_lfs_objects_service_spec.rb b/spec/services/merge_requests/link_lfs_objects_service_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..f07cf13e4f243c831b920ff77835b4c0c6b36fdd --- /dev/null +++ b/spec/services/merge_requests/link_lfs_objects_service_spec.rb @@ -0,0 +1,103 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe MergeRequests::LinkLfsObjectsService, :sidekiq_inline do + include ProjectForksHelper + include RepoHelpers + + let(:target_project) { create(:project, :public, :repository) } + + let(:merge_request) do + create( + :merge_request, + target_project: target_project, + target_branch: 'lfs', + source_project: source_project, + source_branch: 'link-lfs-objects' + ) + end + + subject { described_class.new(target_project) } + + shared_examples_for 'linking LFS objects' do + context 'when source project is the same as target project' do + let(:source_project) { target_project } + + it 'does not call Projects::LfsPointers::LfsLinkService#execute' do + expect(Projects::LfsPointers::LfsLinkService).not_to receive(:new) + + execute + end + end + + context 'when source project is different from target project' do + let(:user) { create(:user) } + let(:source_project) { fork_project(target_project, user, namespace: user.namespace, repository: true) } + + before do + create_branch(source_project, 'link-lfs-objects', 'lfs') + end + + context 'and there are changes' do + before do + allow(source_project).to receive(:lfs_enabled?).and_return(true) + end + + context 'and there are LFS objects added' do + before do + create_file_in_repo(source_project, 'link-lfs-objects', 'link-lfs-objects', 'one.lfs', 'One') + create_file_in_repo(source_project, 'link-lfs-objects', 'link-lfs-objects', 'two.lfs', 'Two') + end + + it 'calls Projects::LfsPointers::LfsLinkService#execute with OIDs of LFS objects in merge request' do + expect_next_instance_of(Projects::LfsPointers::LfsLinkService) do |service| + expect(service).to receive(:execute).with(%w[ + 8b12507783d5becacbf2ebe5b01a60024d8728a8f86dcc818bce699e8b3320bc + 94a72c074cfe574742c9e99e863322f73feff82981d065ff65a0308f44f19f62 + ]) + end + + execute + end + end + + context 'but there are no LFS objects added' do + before do + create_file_in_repo(source_project, 'link-lfs-objects', 'link-lfs-objects', 'one.txt', 'One') + end + + it 'does not call Projects::LfsPointers::LfsLinkService#execute' do + expect(Projects::LfsPointers::LfsLinkService).not_to receive(:new) + + execute + end + end + end + + context 'and there are no changes' do + it 'does not call Projects::LfsPointers::LfsLinkService#execute' do + expect(Projects::LfsPointers::LfsLinkService).not_to receive(:new) + + execute + end + end + end + end + + context 'when no oldrev and newrev passed' do + let(:execute) { subject.execute(merge_request) } + + it_behaves_like 'linking LFS objects' + end + + context 'when oldrev and newrev are passed' do + let(:execute) { subject.execute(merge_request, oldrev: merge_request.diff_base_sha, newrev: merge_request.diff_head_sha) } + + it_behaves_like 'linking LFS objects' + end + + def create_branch(project, new_name, branch_name) + ::Branches::CreateService.new(project, user).execute(new_name, branch_name) + end +end diff --git a/spec/services/merge_requests/refresh_service_spec.rb b/spec/services/merge_requests/refresh_service_spec.rb index 1ba216e8ff1a1614ad2b504aced978a704188f3f..b67779a912d1f27ffa8fa69696860405978d62ac 100644 --- a/spec/services/merge_requests/refresh_service_spec.rb +++ b/spec/services/merge_requests/refresh_service_spec.rb @@ -384,6 +384,14 @@ def refresh end context 'open fork merge request' do + it 'calls MergeRequests::LinkLfsObjectsService#execute' do + expect_next_instance_of(MergeRequests::LinkLfsObjectsService) do |svc| + expect(svc).to receive(:execute).with(@fork_merge_request, oldrev: @oldrev, newrev: @newrev) + end + + refresh + end + it 'executes hooks with update action' do refresh diff --git a/spec/services/projects/fork_service_spec.rb b/spec/services/projects/fork_service_spec.rb index e7b904fcd601c765b26ffe84033c6cea8ed37d57..e14f1abf018c6b71952630b127ff17c6841994fc 100644 --- a/spec/services/projects/fork_service_spec.rb +++ b/spec/services/projects/fork_service_spec.rb @@ -375,14 +375,6 @@ def forked_from_project(project) expect(fork_from_project.forks_count).to eq(1) end - it 'leaves no LFS objects dangling' do - create(:lfs_objects_project, project: fork_to_project) - - expect { subject.execute(fork_to_project) } - .to change { fork_to_project.lfs_objects_projects.count } - .to(0) - end - context 'if the fork is not allowed' do let(:fork_from_project) { create(:project, :private) } diff --git a/spec/services/projects/lfs_pointers/lfs_download_service_spec.rb b/spec/services/projects/lfs_pointers/lfs_download_service_spec.rb index 970e82e71072bb0b001e8dbf09381dfd2add9d28..21a139cdf3ca56528f6b84cb1af91d969f143472 100644 --- a/spec/services/projects/lfs_pointers/lfs_download_service_spec.rb +++ b/spec/services/projects/lfs_pointers/lfs_download_service_spec.rb @@ -48,10 +48,11 @@ end shared_examples 'lfs object is created' do - it do + it 'creates and associate the LFS object to project' do expect(subject).to receive(:download_and_save_file!).and_call_original expect { subject.execute }.to change { LfsObject.count }.by(1) + expect(LfsObject.first.projects).to include(project) end it 'returns success result' do diff --git a/spec/workers/repository_fork_worker_spec.rb b/spec/workers/repository_fork_worker_spec.rb index 26fd67adfaaece9eaf4adc4f9139d31166b4db1b..011040494040019dc99ab569a2bf645c604a76cb 100644 --- a/spec/workers/repository_fork_worker_spec.rb +++ b/spec/workers/repository_fork_worker_spec.rb @@ -72,13 +72,33 @@ def expect_fork_repository perform! end - it "handles bad fork" do - error_message = "Unable to fork project #{forked_project.id} for repository #{project.disk_path} -> #{forked_project.disk_path}" + it 'handles bad fork' do + error_message = "Unable to fork project #{forked_project.id} for repository #{project.disk_path} -> #{forked_project.disk_path}: Failed to create fork repository" expect_fork_repository.and_return(false) expect { perform! }.to raise_error(StandardError, error_message) end + + it 'calls Projects::LfsPointers::LfsLinkService#execute with OIDs of source project LFS objects' do + expect_fork_repository.and_return(true) + expect_next_instance_of(Projects::LfsPointers::LfsLinkService) do |service| + expect(service).to receive(:execute).with(project.lfs_objects_oids) + end + + perform! + end + + it "handles LFS objects link failure" do + error_message = "Unable to fork project #{forked_project.id} for repository #{project.disk_path} -> #{forked_project.disk_path}: Source project has too many LFS objects" + + expect_fork_repository.and_return(true) + expect_next_instance_of(Projects::LfsPointers::LfsLinkService) do |service| + expect(service).to receive(:execute).and_raise(Projects::LfsPointers::LfsLinkService::TooManyOidsError) + end + + expect { perform! }.to raise_error(StandardError, error_message) + end end context 'only project ID passed' do