diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2022-08-24 03:12:25 +0300 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2022-08-24 03:12:25 +0300 |
commit | d5fdc905ae426ab5c7788f8ef56070c35360b1d3 (patch) | |
tree | 2c22aa90b3988af49c2f08e7346273c57226887c /spec | |
parent | de5fc582771ba7e2dc6e1c783ce8ee417fbe6788 (diff) |
Add latest changes from gitlab-org/gitlab@master
Diffstat (limited to 'spec')
13 files changed, 423 insertions, 355 deletions
diff --git a/spec/finders/groups/accepting_group_transfers_finder_spec.rb b/spec/finders/groups/accepting_group_transfers_finder_spec.rb new file mode 100644 index 00000000000..6c1ceb4f7ad --- /dev/null +++ b/spec/finders/groups/accepting_group_transfers_finder_spec.rb @@ -0,0 +1,93 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Groups::AcceptingGroupTransfersFinder do + let_it_be(:current_user) { create(:user) } + + let_it_be(:great_grandparent_group) do + create(:group, name: 'great grandparent group', path: 'great-grandparent-group') + end + + let_it_be(:grandparent_group) { create(:group, parent: great_grandparent_group) } + let_it_be(:parent_group) { create(:group, parent: grandparent_group) } + let_it_be(:child_group) { create(:group, parent: parent_group) } + let_it_be(:grandchild_group) { create(:group, parent: child_group) } + let_it_be(:group_where_user_has_owner_access) do + create(:group, name: 'owner access group', path: 'owner-access-group').tap do |group| + group.add_owner(current_user) + end + end + + let_it_be(:subgroup_of_group_where_user_has_owner_access) do + create(:group, parent: group_where_user_has_owner_access) + end + + let_it_be(:group_where_user_has_developer_access) do + create(:group).tap do |group| + group.add_developer(current_user) + end + end + + let(:params) { {} } + + describe '#execute' do + let(:group_to_be_transferred) { parent_group } + + subject(:result) do + described_class.new(current_user, group_to_be_transferred, params).execute + end + + context 'when the user does not have the rights to transfer the group' do + before do + group_to_be_transferred.root_ancestor.add_developer(current_user) + end + + it 'returns empty result' do + expect(result).to be_empty + end + end + + context 'when the user has the rights to transfer the group' do + before do + group_to_be_transferred.root_ancestor.add_owner(current_user) + end + + it 'does not return empty result' do + expect(result).not_to be_empty + end + + it 'excludes the descendants of the group to be transferred' do + expect(result).not_to include(child_group, grandchild_group) + end + + it 'excludes the immediate parent of the group to be transferred' do + expect(result).not_to include(grandparent_group) + end + + it 'excludes the groups where the user does not have OWNER access' do + expect(result).not_to include(group_where_user_has_developer_access) + end + + it 'includes ancestors, except immediate parent of the group to be transferred' do + expect(result).to include(great_grandparent_group) + end + + it 'includes the other groups where the user has OWNER access' do + expect(result).to include(group_where_user_has_owner_access) + end + + it 'includes the other groups where the user has OWNER access through inherited membership' do + expect(result).to include(subgroup_of_group_where_user_has_owner_access) + end + + context 'on searching with a specific term' do + let(:params) { { search: 'great grandparent group' } } + + it 'includes only the groups where the term matches the group name or path' do + expect(result).to contain_exactly(great_grandparent_group) + end + end + end + end +end diff --git a/spec/graphql/resolvers/issues_resolver_spec.rb b/spec/graphql/resolvers/issues_resolver_spec.rb index 89e45810033..a74b2a3f18c 100644 --- a/spec/graphql/resolvers/issues_resolver_spec.rb +++ b/spec/graphql/resolvers/issues_resolver_spec.rb @@ -311,49 +311,15 @@ RSpec.describe Resolvers::IssuesResolver do end context 'when searching issues' do - it 'returns correct issues' do - expect(resolve_issues(search: 'foo')).to contain_exactly(issue2) - end - - it 'uses project search optimization' do - expected_arguments = a_hash_including( - search: 'foo', - attempt_project_search_optimizations: true - ) - expect(IssuesFinder).to receive(:new).with(anything, expected_arguments).and_call_original - - resolve_issues(search: 'foo') - end - - context 'with anonymous user' do - let_it_be(:public_project) { create(:project, :public) } - let_it_be(:public_issue) { create(:issue, project: public_project, title: 'Test issue') } - - context 'with disable_anonymous_search enabled' do - before do - stub_feature_flags(disable_anonymous_search: true) - end - - it 'generates an error' do - error_message = "User must be authenticated to include the `search` argument." - - expect_graphql_error_to_be_created(Gitlab::Graphql::Errors::ArgumentError, error_message) do - resolve(described_class, obj: public_project, args: { search: 'test' }, ctx: { current_user: nil }) - end - end - end - - context 'with disable_anonymous_search disabled' do - before do - stub_feature_flags(disable_anonymous_search: false) - end - - it 'returns correct issues' do - expect( - resolve(described_class, obj: public_project, args: { search: 'test' }, ctx: { current_user: nil }) - ).to contain_exactly(public_issue) - end - end + it_behaves_like 'graphql query for searching issuables' do + let_it_be(:parent) { project } + let_it_be(:issuable1) { create(:issue, project: project, title: 'first created') } + let_it_be(:issuable2) { create(:issue, project: project, title: 'second created', description: 'text 1') } + let_it_be(:issuable3) { create(:issue, project: project, title: 'third', description: 'text 2') } + let_it_be(:issuable4) { create(:issue, project: project) } + + let_it_be(:finder_class) { IssuesFinder } + let_it_be(:optimization_param) { :attempt_project_search_optimizations } end end diff --git a/spec/graphql/resolvers/work_items_resolver_spec.rb b/spec/graphql/resolvers/work_items_resolver_spec.rb index 29eac0ab46e..ef7cc0daa0c 100644 --- a/spec/graphql/resolvers/work_items_resolver_spec.rb +++ b/spec/graphql/resolvers/work_items_resolver_spec.rb @@ -52,49 +52,15 @@ RSpec.describe Resolvers::WorkItemsResolver do end context 'when searching items' do - it 'returns correct items' do - expect(resolve_items(search: 'foo')).to contain_exactly(item2) - end - - it 'uses project search optimization' do - expected_arguments = a_hash_including( - search: 'foo', - attempt_project_search_optimizations: true - ) - expect(::WorkItems::WorkItemsFinder).to receive(:new).with(anything, expected_arguments).and_call_original - - resolve_items(search: 'foo') - end - - context 'with anonymous user' do - let_it_be(:public_project) { create(:project, :public) } - let_it_be(:public_item) { create(:work_item, project: public_project, title: 'Test item') } - - context 'with disable_anonymous_search enabled' do - before do - stub_feature_flags(disable_anonymous_search: true) - end - - it 'generates an error' do - error_message = "User must be authenticated to include the `search` argument." - - expect_graphql_error_to_be_created(Gitlab::Graphql::Errors::ArgumentError, error_message) do - resolve(described_class, obj: public_project, args: { search: 'test' }, ctx: { current_user: nil }) - end - end - end - - context 'with disable_anonymous_search disabled' do - before do - stub_feature_flags(disable_anonymous_search: false) - end - - it 'returns correct items' do - expect( - resolve(described_class, obj: public_project, args: { search: 'test' }, ctx: { current_user: nil }) - ).to contain_exactly(public_item) - end - end + it_behaves_like 'graphql query for searching issuables' do + let_it_be(:parent) { project } + let_it_be(:issuable1) { create(:work_item, project: project, title: 'first created') } + let_it_be(:issuable2) { create(:work_item, project: project, title: 'second created', description: 'text 1') } + let_it_be(:issuable3) { create(:work_item, project: project, title: 'third', description: 'text 2') } + let_it_be(:issuable4) { create(:work_item, project: project) } + + let_it_be(:finder_class) { ::WorkItems::WorkItemsFinder } + let_it_be(:optimization_param) { :attempt_project_search_optimizations } end end diff --git a/spec/helpers/commits_helper_spec.rb b/spec/helpers/commits_helper_spec.rb index b27954de0d4..d2d2e226f2a 100644 --- a/spec/helpers/commits_helper_spec.rb +++ b/spec/helpers/commits_helper_spec.rb @@ -329,7 +329,7 @@ RSpec.describe CommitsHelper do it { is_expected.to include(commit.author) } it { is_expected.to include(ref) } - it do + specify do is_expected.to include( { merge_request: merge_request.cache_key, diff --git a/spec/helpers/users/callouts_helper_spec.rb b/spec/helpers/users/callouts_helper_spec.rb index 2c148aabead..170ae098a2f 100644 --- a/spec/helpers/users/callouts_helper_spec.rb +++ b/spec/helpers/users/callouts_helper_spec.rb @@ -241,10 +241,10 @@ RSpec.describe Users::CalloutsHelper do context 'the web-hook failure callout has been dismissed', :freeze_time do before do - create(:namespace_callout, + create(:project_callout, feature_name: described_class::WEB_HOOK_DISABLED, user: user, - namespace: project.namespace, + project: project, dismissed_at: 1.week.ago) end diff --git a/spec/requests/api/graphql/project/issues_spec.rb b/spec/requests/api/graphql/project/issues_spec.rb index 596e023a027..d0ed4212349 100644 --- a/spec/requests/api/graphql/project/issues_spec.rb +++ b/spec/requests/api/graphql/project/issues_spec.rb @@ -27,14 +27,6 @@ RSpec.describe 'getting an issue list for a project' do QUERY end - let(:query) do - graphql_query_for( - 'project', - { 'fullPath' => project.full_path }, - query_graphql_field('issues', issue_filter_params, fields) - ) - end - it_behaves_like 'a working graphql query' do before do post_graphql(query, current_user: current_user) @@ -89,6 +81,14 @@ RSpec.describe 'getting an issue list for a project' do end end + context 'when filtering by search' do + it_behaves_like 'query with a search term' do + let(:issuable_data) { issues_data } + let(:user) { current_user } + let_it_be(:issuable) { create(:issue, project: project, description: 'bar') } + end + end + context 'when limiting the number of results' do let(:query) do <<~GQL @@ -679,4 +679,12 @@ RSpec.describe 'getting an issue list for a project' do def issues_ids graphql_dig_at(issues_data, :node, :id) end + + def query(params = issue_filter_params) + graphql_query_for( + 'project', + { 'fullPath' => project.full_path }, + query_graphql_field('issues', params, fields) + ) + end end diff --git a/spec/requests/api/graphql/project/work_items_spec.rb b/spec/requests/api/graphql/project/work_items_spec.rb index 6ef28392b8b..1dd0046f782 100644 --- a/spec/requests/api/graphql/project/work_items_spec.rb +++ b/spec/requests/api/graphql/project/work_items_spec.rb @@ -27,14 +27,6 @@ RSpec.describe 'getting an work item list for a project' do QUERY end - let(:query) do - graphql_query_for( - 'project', - { 'fullPath' => project.full_path }, - query_graphql_field('workItems', item_filter_params, fields) - ) - end - it_behaves_like 'a working graphql query' do before do post_graphql(query, current_user: current_user) @@ -83,6 +75,14 @@ RSpec.describe 'getting an work item list for a project' do end end + context 'when filtering by search' do + it_behaves_like 'query with a search term' do + let(:issuable_data) { items_data } + let(:user) { current_user } + let_it_be(:issuable) { create(:work_item, project: project, description: 'bar') } + end + end + describe 'sorting and pagination' do let(:data_path) { [:project, :work_items] } @@ -118,4 +118,12 @@ RSpec.describe 'getting an work item list for a project' do def item_ids graphql_dig_at(items_data, :node, :id) end + + def query(params = item_filter_params) + graphql_query_for( + 'project', + { 'fullPath' => project.full_path }, + query_graphql_field('workItems', params, fields) + ) + end end diff --git a/spec/requests/api/groups_spec.rb b/spec/requests/api/groups_spec.rb index 0be3341dd13..74be97fca2a 100644 --- a/spec/requests/api/groups_spec.rb +++ b/spec/requests/api/groups_spec.rb @@ -2029,6 +2029,81 @@ RSpec.describe API::Groups do end end + describe 'GET /groups/:id/transfer_locations' do + let_it_be(:user) { create(:user) } + let_it_be(:source_group) { create(:group, :private) } + + let(:params) { {} } + + subject(:request) do + get api("/groups/#{source_group.id}/transfer_locations", user), params: params + end + + context 'when the user has rights to transfer the group' do + let_it_be(:guest_group) { create(:group) } + let_it_be(:maintainer_group) { create(:group, name: 'maintainer group', path: 'maintainer-group') } + let_it_be(:owner_group_1) { create(:group, name: 'owner group', path: 'owner-group') } + let_it_be(:owner_group_2) { create(:group, name: 'gitlab group', path: 'gitlab-group') } + + before do + source_group.add_owner(user) + guest_group.add_guest(user) + maintainer_group.add_maintainer(user) + owner_group_1.add_owner(user) + owner_group_2.add_owner(user) + end + + it 'returns 200' do + request + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to include_pagination_headers + end + + it 'only includes groups where the user has permissions to transfer a group to' do + request + + expect(group_ids_from_response).to contain_exactly(owner_group_1.id, owner_group_2.id) + end + + context 'with search' do + let(:params) { { search: 'gitlab' } } + + it 'includes groups where the user has permissions to transfer a group to, matching the search term' do + request + + expect(group_ids_from_response).to contain_exactly(owner_group_2.id) + end + end + + def group_ids_from_response + json_response.map { |group| group['id'] } + end + end + + context 'when the user does not have permissions to transfer the group' do + before do + source_group.add_developer(user) + end + + it 'returns 403' do + request + + expect(response).to have_gitlab_http_status(:forbidden) + end + end + + context 'for an anonymous user' do + let_it_be(:user) { nil } + + it 'returns 404' do + request + + expect(response).to have_gitlab_http_status(:not_found) + end + end + end + describe 'POST /groups/:id/transfer' do let_it_be(:user) { create(:user) } let_it_be_with_reload(:new_parent_group) { create(:group, :private) } diff --git a/spec/support/shared_examples/graphql/resolvers/issuable_resolvers_shared_examples.rb b/spec/support/shared_examples/graphql/resolvers/issuable_resolvers_shared_examples.rb new file mode 100644 index 00000000000..feaa8070090 --- /dev/null +++ b/spec/support/shared_examples/graphql/resolvers/issuable_resolvers_shared_examples.rb @@ -0,0 +1,95 @@ +# frozen_string_literal: true + +# Requires `parent`, issuable1`, `issuable2`, `issuable3`, `issuable4`, +# `finder_class` and `optimization_param` bindings. +RSpec.shared_examples 'graphql query for searching issuables' do + it 'uses search optimization' do + expected_arguments = a_hash_including( + search: 'text', + optimization_param => true + ) + expect(finder_class).to receive(:new).with(anything, expected_arguments).and_call_original + + resolve_issuables(search: 'text') + end + + it 'filters issuables by title' do + issuables = resolve_issuables(search: 'created') + + expect(issuables).to contain_exactly(issuable1, issuable2) + end + + it 'filters issuables by description' do + issuables = resolve_issuables(search: 'text') + + expect(issuables).to contain_exactly(issuable2, issuable3) + end + + context 'with in param' do + it 'generates an error if param search is missing' do + error_message = "`search` should be present when including the `in` argument" + + expect_graphql_error_to_be_created(Gitlab::Graphql::Errors::ArgumentError, error_message) do + resolve_issuables(in: ['title']) + end + end + + it 'filters issuables by title and description' do + issuable4.update!(title: 'fourth text') + issuables = resolve_issuables(search: 'text', in: %w[title description]) + + expect(issuables).to contain_exactly(issuable2, issuable3, issuable4) + end + + it 'filters issuables by description only' do + with_text = resolve_issuables(search: 'text', in: ['description']) + with_created = resolve_issuables(search: 'created', in: ['description']) + + expect(with_created).to be_empty + expect(with_text).to contain_exactly(issuable2, issuable3) + end + + it 'filters issuables by title only' do + with_text = resolve_issuables(search: 'text', in: ['title']) + with_created = resolve_issuables(search: 'created', in: ['title']) + + expect(with_created).to contain_exactly(issuable1, issuable2) + expect(with_text).to be_empty + end + end + + context 'with anonymous user' do + let_it_be(:current_user) { nil } + + context 'with disable_anonymous_search as `true`' do + before do + stub_feature_flags(disable_anonymous_search: true) + end + + it 'returns an error' do + error_message = "User must be authenticated to include the `search` argument." + + expect_graphql_error_to_be_created(Gitlab::Graphql::Errors::ArgumentError, error_message) do + resolve_issuables(search: 'created') + end + end + end + + context 'with disable_anonymous_search as `false`' do + before do + stub_feature_flags(disable_anonymous_search: false) + parent.update!(visibility_level: Gitlab::VisibilityLevel::PUBLIC) + end + + it 'filters issuables by search term' do + issuables = resolve_issuables(search: 'created') + + expect(issuables).to contain_exactly(issuable1, issuable2) + end + end + end + + def resolve_issuables(args = {}, obj = parent, context = { current_user: current_user }) + resolve(described_class, obj: obj, args: args, ctx: context, arg_style: :internal) + end +end diff --git a/spec/support/shared_examples/requests/api/graphql/issuable_search_shared_examples.rb b/spec/support/shared_examples/requests/api/graphql/issuable_search_shared_examples.rb new file mode 100644 index 00000000000..22805cf7aed --- /dev/null +++ b/spec/support/shared_examples/requests/api/graphql/issuable_search_shared_examples.rb @@ -0,0 +1,14 @@ +# frozen_string_literal: true + +# Requires `query(params)` , `user`, `issuable_data` and `issuable` bindings +RSpec.shared_examples 'query with a search term' do + it 'returns only matching issuables' do + filter_params = { search: 'bar', in: [:DESCRIPTION] } + graphql_query = query(filter_params) + + post_graphql(graphql_query, current_user: user) + ids = graphql_dig_at(issuable_data, :node, :id) + + expect(ids).to contain_exactly(issuable.to_global_id.to_s) + end +end diff --git a/spec/support/shared_examples/tasks/gitlab/uploads/migration_shared_examples.rb b/spec/support/shared_examples/tasks/gitlab/uploads/migration_shared_examples.rb deleted file mode 100644 index b37a8059574..00000000000 --- a/spec/support/shared_examples/tasks/gitlab/uploads/migration_shared_examples.rb +++ /dev/null @@ -1,31 +0,0 @@ -# frozen_string_literal: true - -# Expects the calling spec to define: -# - uploader_class -# - model_class -# - mounted_as -RSpec.shared_examples 'enqueue upload migration jobs in batch' do |batch:| - def run(task) - args = [uploader_class.to_s, model_class.to_s, mounted_as].compact - run_rake_task(task, *args) - end - - it 'migrates local storage to remote object storage' do - expect(ObjectStorage::MigrateUploadsWorker) - .to receive(:perform_async).exactly(batch).times - .and_return("A fake job.") - - run('gitlab:uploads:migrate') - end - - it 'migrates remote object storage to local storage' do - expect(Upload).to receive(:where).exactly(batch + 1).times { Upload.all } - expect(ObjectStorage::MigrateUploadsWorker) - .to receive(:perform_async) - .with(anything, model_class.name, mounted_as, ObjectStorage::Store::LOCAL) - .exactly(batch).times - .and_return("A fake job.") - - run('gitlab:uploads:migrate_to_local') - end -end diff --git a/spec/tasks/gitlab/uploads/migrate_rake_spec.rb b/spec/tasks/gitlab/uploads/migrate_rake_spec.rb index e293271ca67..3a368a5011b 100644 --- a/spec/tasks/gitlab/uploads/migrate_rake_spec.rb +++ b/spec/tasks/gitlab/uploads/migrate_rake_spec.rb @@ -2,133 +2,93 @@ require 'rake_helper' -RSpec.describe 'gitlab:uploads:migrate and migrate_to_local rake tasks', :silence_stdout do - let(:model_class) { nil } - let(:uploader_class) { nil } - let(:mounted_as) { nil } - let(:batch_size) { 3 } - +RSpec.describe 'gitlab:uploads:migrate and migrate_to_local rake tasks', :sidekiq_inline, :silence_stdout do before do - stub_env('MIGRATION_BATCH_SIZE', batch_size.to_s) - stub_uploads_object_storage(uploader_class) + stub_env('MIGRATION_BATCH_SIZE', 3.to_s) + stub_uploads_object_storage(AvatarUploader) + stub_uploads_object_storage(FileUploader) Rake.application.rake_require 'tasks/gitlab/uploads/migrate' - allow(ObjectStorage::MigrateUploadsWorker).to receive(:perform_async) + create_list(:project, 2, :with_avatar) + create_list(:group, 2, :with_avatar) + create_list(:project, 2) do |model| + FileUploader.new(model).store!(fixture_file_upload('spec/fixtures/doc_sample.txt')) + end end - context "for AvatarUploader" do - let(:uploader_class) { AvatarUploader } - let(:mounted_as) { :avatar } + let(:total_uploads_count) { 6 } - context "for Project" do - let(:model_class) { Project } - let!(:projects) { create_list(:project, 10, :with_avatar) } + it 'migrates all uploads to object storage in batches' do + expect(ObjectStorage::MigrateUploadsWorker) + .to receive(:perform_async).twice.and_call_original - it_behaves_like 'enqueue upload migration jobs in batch', batch: 4 - end + run_rake_task('gitlab:uploads:migrate:all') - context "for Group" do - let(:model_class) { Group } + expect(Upload.with_files_stored_locally.count).to eq(0) + expect(Upload.with_files_stored_remotely.count).to eq(total_uploads_count) + end - before do - create_list(:group, 10, :with_avatar) - end + it 'migrates all uploads to local storage in batches' do + run_rake_task('gitlab:uploads:migrate') + expect(Upload.with_files_stored_remotely.count).to eq(total_uploads_count) - it_behaves_like 'enqueue upload migration jobs in batch', batch: 4 - end + expect(ObjectStorage::MigrateUploadsWorker) + .to receive(:perform_async).twice.and_call_original - context "for User" do - let(:model_class) { User } + run_rake_task('gitlab:uploads:migrate_to_local:all') - before do - create_list(:user, 10, :with_avatar) - end - - it_behaves_like 'enqueue upload migration jobs in batch', batch: 4 - end + expect(Upload.with_files_stored_remotely.count).to eq(0) + expect(Upload.with_files_stored_locally.count).to eq(total_uploads_count) end - context "for AttachmentUploader" do - let(:uploader_class) { AttachmentUploader } + shared_examples 'migrate task with filters' do + it 'migrates matching uploads to object storage' do + run_rake_task('gitlab:uploads:migrate', task_arguments) - context "for Note" do - let(:model_class) { Note } - let(:mounted_as) { :attachment } + migrated_count = matching_uploads.with_files_stored_remotely.count - before do - create_list(:note, 10, :with_attachment) - end - - it_behaves_like 'enqueue upload migration jobs in batch', batch: 4 + expect(migrated_count).to eq(matching_uploads.count) + expect(Upload.with_files_stored_locally.count).to eq(total_uploads_count - migrated_count) end - context "for Appearance" do - let(:model_class) { Appearance } - let(:mounted_as) { :logo } + it 'migrates matching uploads to local storage' do + run_rake_task('gitlab:uploads:migrate') + expect(Upload.with_files_stored_remotely.count).to eq(total_uploads_count) + + run_rake_task('gitlab:uploads:migrate_to_local', task_arguments) - before do - create(:appearance, :with_logos) - end + migrated_count = matching_uploads.with_files_stored_locally.count - %i(logo header_logo).each do |mount| - it_behaves_like 'enqueue upload migration jobs in batch', batch: 1 do - let(:mounted_as) { mount } - end - end + expect(migrated_count).to eq(matching_uploads.count) + expect(Upload.with_files_stored_remotely.count).to eq(total_uploads_count - migrated_count) end end - context "for FileUploader" do - let(:uploader_class) { FileUploader } - let(:model_class) { Project } + context 'when uploader_class is given' do + let(:task_arguments) { ['FileUploader'] } + let(:matching_uploads) { Upload.where(uploader: 'FileUploader') } - before do - create_list(:project, 10) do |model| - uploader_class.new(model) - .store!(fixture_file_upload('spec/fixtures/doc_sample.txt')) - end - end - - it_behaves_like 'enqueue upload migration jobs in batch', batch: 4 + it_behaves_like 'migrate task with filters' end - context "for PersonalFileUploader" do - let(:uploader_class) { PersonalFileUploader } - let(:model_class) { PersonalSnippet } - - before do - create_list(:personal_snippet, 10) do |model| - uploader_class.new(model) - .store!(fixture_file_upload('spec/fixtures/doc_sample.txt')) - end - end + context 'when model_class is given' do + let(:task_arguments) { [nil, 'Project'] } + let(:matching_uploads) { Upload.where(model_type: 'Project') } - it_behaves_like 'enqueue upload migration jobs in batch', batch: 4 + it_behaves_like 'migrate task with filters' end - context "for NamespaceFileUploader" do - let(:uploader_class) { NamespaceFileUploader } - let(:model_class) { Snippet } + context 'when mounted_as is given' do + let(:task_arguments) { [nil, nil, :avatar] } + let(:matching_uploads) { Upload.where(mount_point: :avatar) } - before do - create_list(:snippet, 10) do |model| - uploader_class.new(model) - .store!(fixture_file_upload('spec/fixtures/doc_sample.txt')) - end - end - - it_behaves_like 'enqueue upload migration jobs in batch', batch: 4 + it_behaves_like 'migrate task with filters' end - context 'for DesignManagement::DesignV432x230Uploader' do - let(:uploader_class) { DesignManagement::DesignV432x230Uploader } - let(:model_class) { DesignManagement::Action } - let(:mounted_as) { :image_v432x230 } - - before do - create_list(:design_action, 10, :with_image_v432x230) - end + context 'when multiple filters are given' do + let(:task_arguments) { %w[AvatarUploader Project] } + let(:matching_uploads) { Upload.where(uploader: 'AvatarUploader', model_type: 'Project') } - it_behaves_like 'enqueue upload migration jobs in batch', batch: 4 + it_behaves_like 'migrate task with filters' end end diff --git a/spec/uploaders/workers/object_storage/migrate_uploads_worker_spec.rb b/spec/uploaders/workers/object_storage/migrate_uploads_worker_spec.rb index fd01a18e810..1746f480c9b 100644 --- a/spec/uploaders/workers/object_storage/migrate_uploads_worker_spec.rb +++ b/spec/uploaders/workers/object_storage/migrate_uploads_worker_spec.rb @@ -3,120 +3,62 @@ require 'spec_helper' RSpec.describe ObjectStorage::MigrateUploadsWorker do - let(:model_class) { Project } + let(:project) { create(:project, :with_avatar) } let(:uploads) { Upload.all } - let(:to_store) { ObjectStorage::Store::REMOTE } - def perform(uploads, store = nil) - described_class.new.perform(uploads.ids, model_class.to_s, mounted_as, store || to_store) + def perform(uploads, store = ObjectStorage::Store::REMOTE) + described_class.new.perform(uploads.ids, store) rescue ObjectStorage::MigrateUploadsWorker::Report::MigrationFailures # swallow end - # Expects the calling spec to define: - # - model_class - # - mounted_as - # - to_store - RSpec.shared_examples 'uploads migration worker' do - describe '.enqueue!' do - def enqueue! - described_class.enqueue!(uploads, model_class, mounted_as, to_store) - end - - it 'is guarded by .sanity_check!' do - expect(described_class).to receive(:perform_async) - expect(described_class).to receive(:sanity_check!) + before do + stub_uploads_object_storage(AvatarUploader) + stub_uploads_object_storage(FileUploader) - enqueue! - end + FileUploader.new(project).store!(fixture_file_upload('spec/fixtures/doc_sample.txt')) + end - context 'sanity_check! fails' do - before do - expect(described_class).to receive(:sanity_check!).and_raise(described_class::SanityCheckError) - end + describe '#perform' do + it 'migrates files to remote storage' do + expect(Gitlab::AppLogger).to receive(:info).with(%r{Migrated 2/2 files}) - it 'does not enqueue a job' do - expect(described_class).not_to receive(:perform_async) + perform(uploads) - expect { enqueue! }.to raise_error(described_class::SanityCheckError) - end - end + expect(Upload.where(store: ObjectStorage::Store::LOCAL).count).to eq(0) + expect(Upload.where(store: ObjectStorage::Store::REMOTE).count).to eq(2) end - describe '.sanity_check!' do - shared_examples 'raises a SanityCheckError' do |expected_message| - let(:mount_point) { nil } - - it do - expect { described_class.sanity_check!(uploads, model_class, mount_point) } - .to raise_error(described_class::SanityCheckError).with_message(expected_message) - end + context 'reversed' do + before do + perform(uploads) end - context 'uploader types mismatch' do - let!(:outlier) { create(:upload, uploader: 'GitlabUploader') } + it 'migrates files to local storage' do + expect(Upload.where(store: ObjectStorage::Store::REMOTE).count).to eq(2) - include_examples 'raises a SanityCheckError', /Multiple uploaders found/ - end + perform(uploads, ObjectStorage::Store::LOCAL) - context 'mount point not found' do - include_examples 'raises a SanityCheckError', /Mount point [a-z:]+ not found in/ do - let(:mount_point) { :potato } - end + expect(Upload.where(store: ObjectStorage::Store::LOCAL).count).to eq(2) + expect(Upload.where(store: ObjectStorage::Store::REMOTE).count).to eq(0) end end - describe '#perform' do - it 'migrates files to remote storage' do - expect(Gitlab::AppLogger).to receive(:info).with(%r{Migrated 1/1 files}) - - perform(uploads) - - expect(Upload.where(store: ObjectStorage::Store::LOCAL).count).to eq(0) - end - - context 'reversed' do - let(:to_store) { ObjectStorage::Store::LOCAL } - - before do - perform(uploads, ObjectStorage::Store::REMOTE) - end - - it 'migrates files to local storage' do - expect(Upload.where(store: ObjectStorage::Store::REMOTE).count).to eq(1) - - perform(uploads) - - expect(Upload.where(store: ObjectStorage::Store::LOCAL).count).to eq(1) - end + context 'migration is unsuccessful' do + before do + allow_any_instance_of(ObjectStorage::Concern) + .to receive(:migrate!).and_raise(CarrierWave::UploadError, 'I am a teapot.') end - context 'migration is unsuccessful' do - before do - allow_any_instance_of(ObjectStorage::Concern) - .to receive(:migrate!).and_raise(CarrierWave::UploadError, 'I am a teapot.') - end - - it 'does not migrate files to remote storage' do - expect(Gitlab::AppLogger).to receive(:warn).with(/Error .* I am a teapot/) + it 'does not migrate files to remote storage' do + expect(Gitlab::AppLogger).to receive(:warn).with(/Error .* I am a teapot/) - perform(uploads) + perform(uploads) - expect(Upload.where(store: ObjectStorage::Store::LOCAL).count).to eq(1) - end + expect(Upload.where(store: ObjectStorage::Store::LOCAL).count).to eq(2) + expect(Upload.where(store: ObjectStorage::Store::REMOTE).count).to eq(0) end end - end - - context "for AvatarUploader" do - let!(:project_with_avatar) { create(:project, :with_avatar) } - let(:mounted_as) { :avatar } - - before do - stub_uploads_object_storage(AvatarUploader) - end - - it_behaves_like "uploads migration worker" describe "limits N+1 queries" do it "to N*5" do @@ -127,46 +69,18 @@ RSpec.describe ObjectStorage::MigrateUploadsWorker do expect { perform(Upload.all) }.not_to exceed_query_limit(query_count).with_threshold(5) end end - end - - context "for FileUploader" do - let!(:project_with_file) { create(:project) } - let(:secret) { SecureRandom.hex } - let(:mounted_as) { nil } - - def upload_file(project) - uploader = FileUploader.new(project) - uploader.store!(fixture_file_upload('spec/fixtures/doc_sample.txt')) - end - - before do - stub_uploads_object_storage(FileUploader) - - upload_file(project_with_file) - end - - it_behaves_like "uploads migration worker" - - describe "limits N+1 queries" do - it "to N*5" do - query_count = ActiveRecord::QueryRecorder.new { perform(uploads) } - upload_file(create(:project)) + it 'handles legacy argument format' do + described_class.new.perform(uploads.ids, 'Project', :avatar, ObjectStorage::Store::REMOTE) - expect { perform(Upload.all) }.not_to exceed_query_limit(query_count).with_threshold(5) - end + expect(Upload.where(store: ObjectStorage::Store::LOCAL).count).to eq(0) + expect(Upload.where(store: ObjectStorage::Store::REMOTE).count).to eq(2) end - end - context 'for DesignManagement::DesignV432x230Uploader' do - let(:model_class) { DesignManagement::Action } - let!(:design_action) { create(:design_action, :with_image_v432x230) } - let(:mounted_as) { :image_v432x230 } + it 'logs an error when number of arguments is incorrect' do + expect(Gitlab::AppLogger).to receive(:warn).with(/Job has wrong arguments format/) - before do - stub_uploads_object_storage(DesignManagement::DesignV432x230Uploader) + described_class.new.perform(uploads.ids, 'Project', ObjectStorage::Store::REMOTE) end - - it_behaves_like 'uploads migration worker' end end |