diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2020-08-20 21:42:06 +0300 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2020-08-20 21:42:06 +0300 |
commit | 6e4e1050d9dba2b7b2523fdd1768823ab85feef4 (patch) | |
tree | 78be5963ec075d80116a932011d695dd33910b4e /spec/lib/gitlab/background_migration | |
parent | 1ce776de4ae122aba3f349c02c17cebeaa8ecf07 (diff) |
Add latest changes from gitlab-org/gitlab@13-3-stable-ee
Diffstat (limited to 'spec/lib/gitlab/background_migration')
22 files changed, 306 insertions, 1348 deletions
diff --git a/spec/lib/gitlab/background_migration/archive_legacy_traces_spec.rb b/spec/lib/gitlab/background_migration/archive_legacy_traces_spec.rb deleted file mode 100644 index 7991ad69007..00000000000 --- a/spec/lib/gitlab/background_migration/archive_legacy_traces_spec.rb +++ /dev/null @@ -1,61 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe Gitlab::BackgroundMigration::ArchiveLegacyTraces do - include TraceHelpers - - let(:namespaces) { table(:namespaces) } - let(:projects) { table(:projects) } - let(:builds) { table(:ci_builds) } - let(:job_artifacts) { table(:ci_job_artifacts) } - - before do - namespaces.create!(id: 123, name: 'gitlab1', path: 'gitlab1') - projects.create!(id: 123, name: 'gitlab1', path: 'gitlab1', namespace_id: 123) - @build = builds.create!(id: 1, project_id: 123, status: 'success', type: 'Ci::Build') - end - - context 'when trace file exsits at the right place' do - before do - create_legacy_trace(@build, 'trace in file') - end - - it 'correctly archive legacy traces' do - expect(job_artifacts.count).to eq(0) - expect(File.exist?(legacy_trace_path(@build))).to be_truthy - - described_class.new.perform(1, 1) - - expect(job_artifacts.count).to eq(1) - expect(File.exist?(legacy_trace_path(@build))).to be_falsy - expect(File.read(archived_trace_path(job_artifacts.first))).to eq('trace in file') - end - end - - context 'when trace file does not exsits at the right place' do - it 'does not raise errors nor create job artifact' do - expect { described_class.new.perform(1, 1) }.not_to raise_error - - expect(job_artifacts.count).to eq(0) - end - end - - context 'when trace data exsits in database' do - before do - create_legacy_trace_in_db(@build, 'trace in db') - end - - it 'correctly archive legacy traces' do - expect(job_artifacts.count).to eq(0) - expect(@build.read_attribute(:trace)).not_to be_empty - - described_class.new.perform(1, 1) - - @build.reload - expect(job_artifacts.count).to eq(1) - expect(@build.read_attribute(:trace)).to be_nil - expect(File.read(archived_trace_path(job_artifacts.first))).to eq('trace in db') - end - end -end diff --git a/spec/lib/gitlab/background_migration/backfill_hashed_project_repositories_spec.rb b/spec/lib/gitlab/background_migration/backfill_hashed_project_repositories_spec.rb deleted file mode 100644 index 79b344ea6fa..00000000000 --- a/spec/lib/gitlab/background_migration/backfill_hashed_project_repositories_spec.rb +++ /dev/null @@ -1,7 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe Gitlab::BackgroundMigration::BackfillHashedProjectRepositories do - it_behaves_like 'backfill migration for project repositories', :hashed -end diff --git a/spec/lib/gitlab/background_migration/backfill_project_fullpath_in_repo_config_spec.rb b/spec/lib/gitlab/background_migration/backfill_project_fullpath_in_repo_config_spec.rb deleted file mode 100644 index 1b2e1ed0c1a..00000000000 --- a/spec/lib/gitlab/background_migration/backfill_project_fullpath_in_repo_config_spec.rb +++ /dev/null @@ -1,88 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe Gitlab::BackgroundMigration::BackfillProjectFullpathInRepoConfig do - let(:namespaces) { table(:namespaces) } - let(:projects) { table(:projects) } - let(:group) { namespaces.create!(name: 'foo', path: 'foo') } - let(:subgroup) { namespaces.create!(name: 'bar', path: 'bar', parent_id: group.id) } - - describe described_class::Storage::Hashed do - let(:project) { double(id: 555) } - - subject(:project_storage) { described_class.new(project) } - - it 'has the correct disk_path' do - expect(project_storage.disk_path).to eq('@hashed/91/a7/91a73fd806ab2c005c13b4dc19130a884e909dea3f72d46e30266fe1a1f588d8') - end - end - - describe described_class::Storage::LegacyProject do - let(:project) { double(full_path: 'this/is/the/full/path') } - - subject(:project_storage) { described_class.new(project) } - - it 'has the correct disk_path' do - expect(project_storage.disk_path).to eq('this/is/the/full/path') - end - end - - describe described_class::Project do - let(:project_record) { projects.create!(namespace_id: subgroup.id, name: 'baz', path: 'baz') } - - subject(:project) { described_class.find(project_record.id) } - - describe '#full_path' do - it 'returns path containing all parent namespaces' do - expect(project.full_path).to eq('foo/bar/baz') - end - - it 'raises OrphanedNamespaceError when any parent namespace does not exist' do - subgroup.update_attribute(:parent_id, non_existing_record_id) - - expect { project.full_path }.to raise_error(Gitlab::BackgroundMigration::BackfillProjectFullpathInRepoConfig::OrphanedNamespaceError) - end - end - end - - describe described_class::Up do - describe '#perform' do - subject(:migrate) { described_class.new.perform(projects.minimum(:id), projects.maximum(:id)) } - - it 'asks the gitaly client to set config' do - projects.create!(namespace_id: subgroup.id, name: 'baz', path: 'baz') - projects.create!(namespace_id: subgroup.id, name: 'buzz', path: 'buzz', storage_version: 1) - - expect_next_instance_of(Gitlab::GitalyClient::RepositoryService) do |repository_service| - allow(repository_service).to receive(:cleanup) - expect(repository_service).to receive(:set_config).with('gitlab.fullpath' => 'foo/bar/baz') - end - - expect_next_instance_of(Gitlab::GitalyClient::RepositoryService) do |repository_service| - allow(repository_service).to receive(:cleanup) - expect(repository_service).to receive(:set_config).with('gitlab.fullpath' => 'foo/bar/buzz') - end - - migrate - end - end - end - - describe described_class::Down do - describe '#perform' do - subject(:migrate) { described_class.new.perform(projects.minimum(:id), projects.maximum(:id)) } - - it 'asks the gitaly client to set config' do - projects.create!(namespace_id: subgroup.id, name: 'baz', path: 'baz') - - expect_next_instance_of(Gitlab::GitalyClient::RepositoryService) do |repository_service| - allow(repository_service).to receive(:cleanup) - expect(repository_service).to receive(:delete_config).with(['gitlab.fullpath']) - end - - migrate - end - end - end -end diff --git a/spec/lib/gitlab/background_migration/backfill_snippet_repositories_spec.rb b/spec/lib/gitlab/background_migration/backfill_snippet_repositories_spec.rb index ec2fd3cc4e0..fad33265030 100644 --- a/spec/lib/gitlab/background_migration/backfill_snippet_repositories_spec.rb +++ b/spec/lib/gitlab/background_migration/backfill_snippet_repositories_spec.rb @@ -264,6 +264,7 @@ RSpec.describe Gitlab::BackgroundMigration::BackfillSnippetRepositories, :migrat user_type: user_type, confirmed_at: 1.day.ago) end + let!(:invalid_snippet) { snippets.create(id: 4, type: 'PersonalSnippet', author_id: user.id, file_name: '.', content: content) } let!(:snippet) { snippets.create(id: 5, type: 'PersonalSnippet', author_id: other_user.id, file_name: file_name, content: content) } let(:ids) { [4, 5] } diff --git a/spec/lib/gitlab/background_migration/copy_merge_request_target_project_to_merge_request_metrics_spec.rb b/spec/lib/gitlab/background_migration/copy_merge_request_target_project_to_merge_request_metrics_spec.rb new file mode 100644 index 00000000000..71bb794d539 --- /dev/null +++ b/spec/lib/gitlab/background_migration/copy_merge_request_target_project_to_merge_request_metrics_spec.rb @@ -0,0 +1,39 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::BackgroundMigration::CopyMergeRequestTargetProjectToMergeRequestMetrics, :migration, schema: 20200723125205 do + let(:migration) { described_class.new } + + let_it_be(:namespaces) { table(:namespaces) } + let_it_be(:projects) { table(:projects) } + let_it_be(:merge_requests) { table(:merge_requests) } + let_it_be(:metrics) { table(:merge_request_metrics) } + + let!(:namespace) { namespaces.create!(name: 'namespace', path: 'namespace') } + let!(:project_1) { projects.create!(namespace_id: namespace.id) } + let!(:project_2) { projects.create!(namespace_id: namespace.id) } + let!(:merge_request_to_migrate_1) { merge_requests.create!(source_branch: 'a', target_branch: 'b', target_project_id: project_1.id) } + let!(:merge_request_to_migrate_2) { merge_requests.create!(source_branch: 'c', target_branch: 'd', target_project_id: project_2.id) } + let!(:merge_request_without_metrics) { merge_requests.create!(source_branch: 'e', target_branch: 'f', target_project_id: project_2.id) } + + let!(:metrics_1) { metrics.create!(merge_request_id: merge_request_to_migrate_1.id) } + let!(:metrics_2) { metrics.create!(merge_request_id: merge_request_to_migrate_2.id) } + + let(:merge_request_ids) { [merge_request_to_migrate_1.id, merge_request_to_migrate_2.id, merge_request_without_metrics.id] } + + subject { migration.perform(merge_request_ids.min, merge_request_ids.max) } + + it 'copies `target_project_id` to the associated `merge_request_metrics` record' do + subject + + expect(metrics_1.reload.target_project_id).to eq(project_1.id) + expect(metrics_2.reload.target_project_id).to eq(project_2.id) + end + + it 'does not create metrics record when it is missing' do + subject + + expect(metrics.find_by_merge_request_id(merge_request_without_metrics.id)).to be_nil + end +end diff --git a/spec/lib/gitlab/background_migration/fix_cross_project_label_links_spec.rb b/spec/lib/gitlab/background_migration/fix_cross_project_label_links_spec.rb deleted file mode 100644 index 8e3ace083fc..00000000000 --- a/spec/lib/gitlab/background_migration/fix_cross_project_label_links_spec.rb +++ /dev/null @@ -1,111 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe Gitlab::BackgroundMigration::FixCrossProjectLabelLinks do - let(:namespaces_table) { table(:namespaces) } - let(:projects_table) { table(:projects) } - let(:issues_table) { table(:issues) } - let(:merge_requests_table) { table(:merge_requests) } - let(:labels_table) { table(:labels) } - let(:label_links_table) { table(:label_links) } - - let!(:group1) { namespaces_table.create(id: 10, type: 'Group', name: 'group1', path: 'group1') } - let!(:group2) { namespaces_table.create(id: 20, type: 'Group', name: 'group2', path: 'group2') } - - let!(:project1) { projects_table.create(id: 1, name: 'project1', path: 'group1/project1', namespace_id: 10) } - let!(:project2) { projects_table.create(id: 3, name: 'project2', path: 'group1/project2', namespace_id: 20) } - - let!(:label1) { labels_table.create(id: 1, title: 'bug', color: 'red', group_id: 10, type: 'GroupLabel') } - let!(:label2) { labels_table.create(id: 2, title: 'bug', color: 'red', group_id: 20, type: 'GroupLabel') } - - def create_merge_request(id, project_id) - merge_requests_table.create(id: id, - target_project_id: project_id, - target_branch: 'master', - source_project_id: project_id, - source_branch: 'mr name', - title: "mr name#{id}") - end - - def create_issue(id, project_id) - issues_table.create(id: id, title: "issue#{id}", project_id: project_id) - end - - def create_resource(target_type, id, project_id) - target_type == 'Issue' ? create_issue(id, project_id) : create_merge_request(id, project_id) - end - - shared_examples_for 'resource with cross-project labels' do - it 'updates only cross-project label links which exist in the local project or group' do - create_resource(target_type, 1, 1) - create_resource(target_type, 2, 3) - labels_table.create(id: 3, title: 'bug', color: 'red', project_id: 3, type: 'ProjectLabel') - link = label_links_table.create(label_id: 2, target_type: target_type, target_id: 1) - link2 = label_links_table.create(label_id: 3, target_type: target_type, target_id: 2) - - subject.perform(1, 100) - - expect(link.reload.label_id).to eq(1) - expect(link2.reload.label_id).to eq(3) - end - - it 'ignores cross-project label links if label color is different' do - labels_table.create(id: 3, title: 'bug', color: 'green', group_id: 20, type: 'GroupLabel') - create_resource(target_type, 1, 1) - link = label_links_table.create(label_id: 3, target_type: target_type, target_id: 1) - - subject.perform(1, 100) - - expect(link.reload.label_id).to eq(3) - end - - it 'ignores cross-project label links if label name is different' do - labels_table.create(id: 3, title: 'bug1', color: 'red', group_id: 20, type: 'GroupLabel') - create_resource(target_type, 1, 1) - link = label_links_table.create(label_id: 3, target_type: target_type, target_id: 1) - - subject.perform(1, 100) - - expect(link.reload.label_id).to eq(3) - end - - context 'with nested group' do - before do - namespaces_table.create(id: 11, type: 'Group', name: 'subgroup1', path: 'group1/subgroup1', parent_id: 10) - projects_table.create(id: 2, name: 'subproject1', path: 'group1/subgroup1/subproject1', namespace_id: 11) - create_resource(target_type, 1, 2) - end - - it 'ignores label links referencing ancestor group labels' do - labels_table.create(id: 4, title: 'bug', color: 'red', project_id: 2, type: 'ProjectLabel') - label_links_table.create(label_id: 4, target_type: target_type, target_id: 1) - link = label_links_table.create(label_id: 1, target_type: target_type, target_id: 1) - - subject.perform(1, 100) - - expect(link.reload.label_id).to eq(1) - end - - it 'checks also issues and MRs in subgroups' do - link = label_links_table.create(label_id: 2, target_type: target_type, target_id: 1) - - subject.perform(1, 100) - - expect(link.reload.label_id).to eq(1) - end - end - end - - context 'resource is Issue' do - it_behaves_like 'resource with cross-project labels' do - let(:target_type) { 'Issue' } - end - end - - context 'resource is Merge Request' do - it_behaves_like 'resource with cross-project labels' do - let(:target_type) { 'MergeRequest' } - end - end -end diff --git a/spec/lib/gitlab/background_migration/legacy_upload_mover_spec.rb b/spec/lib/gitlab/background_migration/legacy_upload_mover_spec.rb index bf793e7c537..1637589d272 100644 --- a/spec/lib/gitlab/background_migration/legacy_upload_mover_spec.rb +++ b/spec/lib/gitlab/background_migration/legacy_upload_mover_spec.rb @@ -264,6 +264,7 @@ RSpec.describe Gitlab::BackgroundMigration::LegacyUploadMover do let(:remote_file) do { key: "#{legacy_upload.path}" } end + let(:connection) { ::Fog::Storage.new(FileUploader.object_store_credentials) } let(:bucket) { connection.directories.create(key: 'uploads') } diff --git a/spec/lib/gitlab/background_migration/migrate_build_stage_spec.rb b/spec/lib/gitlab/background_migration/migrate_build_stage_spec.rb deleted file mode 100644 index 65d45ec694f..00000000000 --- a/spec/lib/gitlab/background_migration/migrate_build_stage_spec.rb +++ /dev/null @@ -1,84 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe Gitlab::BackgroundMigration::MigrateBuildStage do - let(:namespaces) { table(:namespaces) } - let(:projects) { table(:projects) } - let(:pipelines) { table(:ci_pipelines) } - let(:stages) { table(:ci_stages) } - let(:jobs) { table(:ci_builds) } - - let(:statuses) do - { - created: 0, - pending: 1, - running: 2, - success: 3, - failed: 4, - canceled: 5, - skipped: 6, - manual: 7 - } - end - - before do - namespace = namespaces.create!(name: 'gitlab-org', path: 'gitlab-org') - projects.create!(id: 123, name: 'gitlab', path: 'gitlab-ce', namespace_id: namespace.id) - pipelines.create!(id: 1, project_id: 123, ref: 'master', sha: 'adf43c3a') - - jobs.create!(id: 1, commit_id: 1, project_id: 123, - stage_idx: 2, stage: 'build', status: :success) - jobs.create!(id: 2, commit_id: 1, project_id: 123, - stage_idx: 2, stage: 'build', status: :success) - jobs.create!(id: 3, commit_id: 1, project_id: 123, - stage_idx: 1, stage: 'test', status: :failed) - jobs.create!(id: 4, commit_id: 1, project_id: 123, - stage_idx: 1, stage: 'test', status: :success) - jobs.create!(id: 5, commit_id: 1, project_id: 123, - stage_idx: 3, stage: 'deploy', status: :pending) - jobs.create!(id: 6, commit_id: 1, project_id: 123, - stage_idx: 3, stage: nil, status: :pending) - end - - it 'correctly migrates builds stages' do - expect(stages.count).to be_zero - - described_class.new.perform(1, 6) - - expect(stages.count).to eq 3 - expect(stages.all.pluck(:name)).to match_array %w[test build deploy] - expect(jobs.where(stage_id: nil)).to be_one - expect(jobs.find_by(stage_id: nil).id).to eq 6 - expect(stages.all.pluck(:status)).to match_array [statuses[:success], - statuses[:failed], - statuses[:pending]] - end - - it 'recovers from unique constraint violation only twice', quarantine: 'https://gitlab.com/gitlab-org/gitlab/-/issues/28128' do - allow(described_class::Migratable::Stage) - .to receive(:find_by).and_return(nil) - - expect(described_class::Migratable::Stage) - .to receive(:find_by).exactly(3).times - - expect { described_class.new.perform(1, 6) } - .to raise_error ActiveRecord::RecordNotUnique - end - - context 'when invalid class can be loaded due to single table inheritance' do - let(:commit_status) do - jobs.create!(id: 7, commit_id: 1, project_id: 123, stage_idx: 4, - stage: 'post-deploy', status: :failed) - end - - before do - commit_status.update_column(:type, 'SomeClass') - end - - it 'does ignore single table inheritance type' do - expect { described_class.new.perform(1, 7) }.not_to raise_error - expect(jobs.find(7)).to have_attributes(stage_id: (a_value > 0)) - end - end -end diff --git a/spec/lib/gitlab/background_migration/migrate_issue_trackers_sensitive_data_spec.rb b/spec/lib/gitlab/background_migration/migrate_issue_trackers_sensitive_data_spec.rb index d53f79c61c1..d829fd5daf5 100644 --- a/spec/lib/gitlab/background_migration/migrate_issue_trackers_sensitive_data_spec.rb +++ b/spec/lib/gitlab/background_migration/migrate_issue_trackers_sensitive_data_spec.rb @@ -286,9 +286,11 @@ RSpec.describe Gitlab::BackgroundMigration::MigrateIssueTrackersSensitiveData, s let!(:jira_service_invalid) do services.create(id: 19, title: 'invalid - title', description: 'invalid - description', type: 'JiraService', properties: 'invalid data', category: 'issue_tracker') end + let!(:jira_service_valid) do services.create(id: 20, type: 'JiraService', properties: jira_properties.to_json, category: 'issue_tracker') end + let!(:bugzilla_service_valid) do services.create(id: 11, type: 'BugzillaService', title: nil, properties: tracker_properties.to_json, category: 'issue_tracker') end diff --git a/spec/lib/gitlab/background_migration/migrate_stage_index_spec.rb b/spec/lib/gitlab/background_migration/migrate_stage_index_spec.rb deleted file mode 100644 index 81874ff7982..00000000000 --- a/spec/lib/gitlab/background_migration/migrate_stage_index_spec.rb +++ /dev/null @@ -1,35 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe Gitlab::BackgroundMigration::MigrateStageIndex do - let(:namespaces) { table(:namespaces) } - let(:projects) { table(:projects) } - let(:pipelines) { table(:ci_pipelines) } - let(:stages) { table(:ci_stages) } - let(:jobs) { table(:ci_builds) } - let(:namespace) { namespaces.create(name: 'gitlab-org', path: 'gitlab-org') } - let(:project) { projects.create!(namespace_id: namespace.id, name: 'gitlab', path: 'gitlab') } - let(:pipeline) { pipelines.create!(project_id: project.id, ref: 'master', sha: 'adf43c3a') } - let(:stage1) { stages.create(project_id: project.id, pipeline_id: pipeline.id, name: 'build') } - let(:stage2) { stages.create(project_id: project.id, pipeline_id: pipeline.id, name: 'test') } - - before do - jobs.create!(commit_id: pipeline.id, project_id: project.id, - stage_idx: 2, stage_id: stage1.id) - jobs.create!(commit_id: pipeline.id, project_id: project.id, - stage_idx: 2, stage_id: stage1.id) - jobs.create!(commit_id: pipeline.id, project_id: project.id, - stage_idx: 10, stage_id: stage1.id) - jobs.create!(commit_id: pipeline.id, project_id: project.id, - stage_idx: 3, stage_id: stage2.id) - end - - it 'correctly migrates stages indices' do - expect(stages.all.pluck(:position)).to all(be_nil) - - described_class.new.perform(stage1.id, stage2.id) - - expect(stages.all.order(:id).pluck(:position)).to eq [2, 3] - end -end diff --git a/spec/lib/gitlab/background_migration/populate_cluster_kubernetes_namespace_table_spec.rb b/spec/lib/gitlab/background_migration/populate_cluster_kubernetes_namespace_table_spec.rb deleted file mode 100644 index 73faca54b52..00000000000 --- a/spec/lib/gitlab/background_migration/populate_cluster_kubernetes_namespace_table_spec.rb +++ /dev/null @@ -1,94 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe Gitlab::BackgroundMigration::PopulateClusterKubernetesNamespaceTable do - include MigrationHelpers::ClusterHelpers - - let(:migration) { described_class.new } - let(:clusters_table) { table(:clusters) } - let(:cluster_projects_table) { table(:cluster_projects) } - let(:cluster_kubernetes_namespaces_table) { table(:clusters_kubernetes_namespaces) } - let(:projects_table) { table(:projects) } - let(:namespaces_table) { table(:namespaces) } - let(:provider_gcp_table) { table(:cluster_providers_gcp) } - let(:platform_kubernetes_table) { table(:cluster_platforms_kubernetes) } - - before do - create_cluster_project_list(10) - end - - shared_examples 'consistent kubernetes namespace attributes' do - it 'populates namespace and service account information' do - migration.perform - - clusters_with_namespace.each do |cluster| - cluster_project = cluster_projects_table.find_by(cluster_id: cluster.id) - project = projects_table.find(cluster_project.project_id) - kubernetes_namespace = cluster_kubernetes_namespaces_table.find_by(cluster_id: cluster.id) - namespace = "#{project.path}-#{project.id}" - - expect(kubernetes_namespace).to be_present - expect(kubernetes_namespace.cluster_project_id).to eq(cluster_project.id) - expect(kubernetes_namespace.project_id).to eq(cluster_project.project_id) - expect(kubernetes_namespace.cluster_id).to eq(cluster_project.cluster_id) - expect(kubernetes_namespace.namespace).to eq(namespace) - expect(kubernetes_namespace.service_account_name).to eq("#{namespace}-service-account") - end - end - end - - context 'when no Clusters::Project has a Clusters::KubernetesNamespace' do - let(:cluster_projects) { cluster_projects_table.all } - - it 'creates a Clusters::KubernetesNamespace per Clusters::Project' do - expect do - migration.perform - end.to change(Clusters::KubernetesNamespace, :count).by(cluster_projects_table.count) - end - - it_behaves_like 'consistent kubernetes namespace attributes' do - let(:clusters_with_namespace) { clusters_table.all } - end - end - - context 'when every Clusters::Project has Clusters::KubernetesNamespace' do - before do - create_kubernetes_namespace(clusters_table.all) - end - - it 'does not create any Clusters::KubernetesNamespace' do - expect do - migration.perform - end.not_to change(Clusters::KubernetesNamespace, :count) - end - end - - context 'when only some Clusters::Project have Clusters::KubernetesNamespace related' do - let(:with_kubernetes_namespace) { clusters_table.first(6) } - let(:with_no_kubernetes_namespace) { clusters_table.last(4) } - - before do - create_kubernetes_namespace(with_kubernetes_namespace) - end - - it 'creates limited number of Clusters::KubernetesNamespace' do - expect do - migration.perform - end.to change(Clusters::KubernetesNamespace, :count).by(with_no_kubernetes_namespace.count) - end - - it 'does not modify clusters with Clusters::KubernetesNamespace' do - migration.perform - - with_kubernetes_namespace.each do |cluster| - kubernetes_namespace = cluster_kubernetes_namespaces_table.where(cluster_id: cluster.id) - expect(kubernetes_namespace.count).to eq(1) - end - end - - it_behaves_like 'consistent kubernetes namespace attributes' do - let(:clusters_with_namespace) { with_no_kubernetes_namespace } - end - end -end diff --git a/spec/lib/gitlab/background_migration/populate_personal_snippet_statistics_spec.rb b/spec/lib/gitlab/background_migration/populate_personal_snippet_statistics_spec.rb new file mode 100644 index 00000000000..e746451b1b9 --- /dev/null +++ b/spec/lib/gitlab/background_migration/populate_personal_snippet_statistics_spec.rb @@ -0,0 +1,141 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::BackgroundMigration::PopulatePersonalSnippetStatistics do + let(:file_name) { 'file_name.rb' } + let(:content) { 'content' } + let(:snippets) { table(:snippets) } + let(:snippet_repositories) { table(:snippet_repositories) } + let(:users) { table(:users) } + let(:namespaces) { table(:namespaces) } + let(:snippet_statistics) { table(:snippet_statistics) } + let(:namespace_statistics) { table(:namespace_root_storage_statistics) } + let(:routes) { table(:routes) } + let(:repo_size) { 123456 } + let(:expected_repo_size) { repo_size.megabytes } + + let(:user1) { users.create!(id: 1, email: 'test@example.com', projects_limit: 100, username: 'test1') } + let(:user2) { users.create!(id: 2, email: 'test2@example.com', projects_limit: 100, username: 'test2') } + let!(:user1_namespace) { namespaces.create!(id: 1, name: 'user1', path: 'user1', owner_id: user1.id) } + let!(:user2_namespace) { namespaces.create!(id: 2, name: 'user2', path: 'user2', owner_id: user2.id) } + let(:user1_namespace_statistics) { namespace_statistics.find_by(namespace_id: user1_namespace.id) } + let(:user2_namespace_statistics) { namespace_statistics.find_by(namespace_id: user2_namespace.id) } + + let(:ids) { snippets.pluck(:id) } + let(:migration) { described_class.new } + + subject do + migration.perform(ids) + end + + before do + allow_any_instance_of(Repository).to receive(:size).and_return(repo_size) + end + + after do + snippets.all.each { |s| raw_repository(s).remove } + end + + context 'with existing personal snippets' do + let!(:snippet1) { create_snippet(1, user1) } + let!(:snippet2) { create_snippet(2, user1) } + let!(:snippet3) { create_snippet(3, user2) } + let!(:snippet4) { create_snippet(4, user2) } + + before do + create_snippet_statistics(2, 0) + create_snippet_statistics(4, 123) + end + + it 'creates/updates all snippet_statistics' do + expect { subject }.to change { snippet_statistics.count }.from(2).to(4) + + expect(snippet_statistics.pluck(:repository_size)).to be_all(expected_repo_size) + end + + it 'creates/updates the associated namespace statistics' do + expect(migration).to receive(:update_namespace_statistics).twice.and_call_original + + subject + + stats = snippet_statistics.where(snippet_id: [snippet1, snippet2]).sum(:repository_size) + expect(user1_namespace_statistics.snippets_size).to eq stats + + stats = snippet_statistics.where(snippet_id: [snippet3, snippet4]).sum(:repository_size) + expect(user2_namespace_statistics.snippets_size).to eq stats + end + + context 'when an error is raised when updating a namespace statistics' do + it 'logs the error and continue execution' do + expect_next_instance_of(Namespaces::StatisticsRefresherService) do |instance| + expect(instance).to receive(:execute).with(Namespace.find(user1_namespace.id)).and_raise('Error') + end + + expect_next_instance_of(Namespaces::StatisticsRefresherService) do |instance| + expect(instance).to receive(:execute).and_call_original + end + + expect_next_instance_of(Gitlab::BackgroundMigration::Logger) do |instance| + expect(instance).to receive(:error).with(message: /Error updating statistics for namespace/).once + end + + subject + + expect(user1_namespace_statistics).to be_nil + + stats = snippet_statistics.where(snippet_id: [snippet3, snippet4]).sum(:repository_size) + expect(user2_namespace_statistics.snippets_size).to eq stats + end + end + end + + context 'when a snippet repository is empty' do + let!(:snippet1) { create_snippet(1, user1, with_repo: false) } + let!(:snippet2) { create_snippet(2, user1) } + + it 'logs error and continues execution' do + expect_next_instance_of(Gitlab::BackgroundMigration::Logger) do |instance| + expect(instance).to receive(:error).with(message: /Invalid snippet repository/).once + end + + subject + + expect(snippet_statistics.find_by(snippet_id: snippet1.id)).to be_nil + expect(user1_namespace_statistics.snippets_size).to eq expected_repo_size + end + end + + def create_snippet(id, author, with_repo: true) + snippets.create!(id: id, type: 'PersonalSnippet', author_id: author.id, file_name: file_name, content: content).tap do |snippet| + if with_repo + allow(snippet).to receive(:disk_path).and_return(disk_path(snippet)) + + TestEnv.copy_repo(snippet, + bare_repo: TestEnv.factory_repo_path_bare, + refs: TestEnv::BRANCH_SHA) + + raw_repository(snippet).create_repository + end + end + end + + def create_snippet_statistics(snippet_id, repository_size = 0) + snippet_statistics.create!(snippet_id: snippet_id, repository_size: repository_size) + end + + def raw_repository(snippet) + Gitlab::Git::Repository.new('default', + "#{disk_path(snippet)}.git", + Gitlab::GlRepository::SNIPPET.identifier_for_container(snippet), + "@snippets/#{snippet.id}") + end + + def hashed_repository(snippet) + Storage::Hashed.new(snippet, prefix: '@snippets') + end + + def disk_path(snippet) + hashed_repository(snippet).disk_path + end +end diff --git a/spec/lib/gitlab/background_migration/populate_untracked_uploads_dependencies/untracked_file_spec.rb b/spec/lib/gitlab/background_migration/populate_untracked_uploads_dependencies/untracked_file_spec.rb deleted file mode 100644 index 6a25e8e2784..00000000000 --- a/spec/lib/gitlab/background_migration/populate_untracked_uploads_dependencies/untracked_file_spec.rb +++ /dev/null @@ -1,263 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe Gitlab::BackgroundMigration::PopulateUntrackedUploadsDependencies::UntrackedFile do - include MigrationsHelpers::TrackUntrackedUploadsHelpers - - let!(:appearances) { table(:appearances) } - let!(:namespaces) { table(:namespaces) } - let!(:projects) { table(:projects) } - let!(:routes) { table(:routes) } - let!(:uploads) { table(:uploads) } - - before(:all) do - ensure_temporary_tracking_table_exists - end - - describe '#upload_path' do - def assert_upload_path(file_path, expected_upload_path) - untracked_file = create_untracked_file(file_path) - - expect(untracked_file.upload_path).to eq(expected_upload_path) - end - - context 'for an appearance logo file path' do - it 'returns the file path relative to the CarrierWave root' do - assert_upload_path('/-/system/appearance/logo/1/some_logo.jpg', 'uploads/-/system/appearance/logo/1/some_logo.jpg') - end - end - - context 'for an appearance header_logo file path' do - it 'returns the file path relative to the CarrierWave root' do - assert_upload_path('/-/system/appearance/header_logo/1/some_logo.jpg', 'uploads/-/system/appearance/header_logo/1/some_logo.jpg') - end - end - - context 'for a pre-Markdown Note attachment file path' do - it 'returns the file path relative to the CarrierWave root' do - assert_upload_path('/-/system/note/attachment/1234/some_attachment.pdf', 'uploads/-/system/note/attachment/1234/some_attachment.pdf') - end - end - - context 'for a user avatar file path' do - it 'returns the file path relative to the CarrierWave root' do - assert_upload_path('/-/system/user/avatar/1234/avatar.jpg', 'uploads/-/system/user/avatar/1234/avatar.jpg') - end - end - - context 'for a group avatar file path' do - it 'returns the file path relative to the CarrierWave root' do - assert_upload_path('/-/system/group/avatar/1234/avatar.jpg', 'uploads/-/system/group/avatar/1234/avatar.jpg') - end - end - - context 'for a project avatar file path' do - it 'returns the file path relative to the CarrierWave root' do - assert_upload_path('/-/system/project/avatar/1234/avatar.jpg', 'uploads/-/system/project/avatar/1234/avatar.jpg') - end - end - - context 'for a project Markdown attachment (notes, issues, MR descriptions) file path' do - it 'returns the file path relative to the project directory in uploads' do - project = create_project - random_hex = SecureRandom.hex - - assert_upload_path("/#{get_full_path(project)}/#{random_hex}/Some file.jpg", "#{random_hex}/Some file.jpg") - end - end - end - - describe '#uploader' do - def assert_uploader(file_path, expected_uploader) - untracked_file = create_untracked_file(file_path) - - expect(untracked_file.uploader).to eq(expected_uploader) - end - - context 'for an appearance logo file path' do - it 'returns AttachmentUploader as a string' do - assert_uploader('/-/system/appearance/logo/1/some_logo.jpg', 'AttachmentUploader') - end - end - - context 'for an appearance header_logo file path' do - it 'returns AttachmentUploader as a string' do - assert_uploader('/-/system/appearance/header_logo/1/some_logo.jpg', 'AttachmentUploader') - end - end - - context 'for a pre-Markdown Note attachment file path' do - it 'returns AttachmentUploader as a string' do - assert_uploader('/-/system/note/attachment/1234/some_attachment.pdf', 'AttachmentUploader') - end - end - - context 'for a user avatar file path' do - it 'returns AvatarUploader as a string' do - assert_uploader('/-/system/user/avatar/1234/avatar.jpg', 'AvatarUploader') - end - end - - context 'for a group avatar file path' do - it 'returns AvatarUploader as a string' do - assert_uploader('/-/system/group/avatar/1234/avatar.jpg', 'AvatarUploader') - end - end - - context 'for a project avatar file path' do - it 'returns AvatarUploader as a string' do - assert_uploader('/-/system/project/avatar/1234/avatar.jpg', 'AvatarUploader') - end - end - - context 'for a project Markdown attachment (notes, issues, MR descriptions) file path' do - it 'returns FileUploader as a string' do - project = create_project - - assert_uploader("/#{get_full_path(project)}/#{SecureRandom.hex}/Some file.jpg", 'FileUploader') - end - end - end - - describe '#model_type' do - def assert_model_type(file_path, expected_model_type) - untracked_file = create_untracked_file(file_path) - - expect(untracked_file.model_type).to eq(expected_model_type) - end - - context 'for an appearance logo file path' do - it 'returns Appearance as a string' do - assert_model_type('/-/system/appearance/logo/1/some_logo.jpg', 'Appearance') - end - end - - context 'for an appearance header_logo file path' do - it 'returns Appearance as a string' do - assert_model_type('/-/system/appearance/header_logo/1/some_logo.jpg', 'Appearance') - end - end - - context 'for a pre-Markdown Note attachment file path' do - it 'returns Note as a string' do - assert_model_type('/-/system/note/attachment/1234/some_attachment.pdf', 'Note') - end - end - - context 'for a user avatar file path' do - it 'returns User as a string' do - assert_model_type('/-/system/user/avatar/1234/avatar.jpg', 'User') - end - end - - context 'for a group avatar file path' do - it 'returns Namespace as a string' do - assert_model_type('/-/system/group/avatar/1234/avatar.jpg', 'Namespace') - end - end - - context 'for a project avatar file path' do - it 'returns Project as a string' do - assert_model_type('/-/system/project/avatar/1234/avatar.jpg', 'Project') - end - end - - context 'for a project Markdown attachment (notes, issues, MR descriptions) file path' do - it 'returns Project as a string' do - project = create_project - - assert_model_type("/#{get_full_path(project)}/#{SecureRandom.hex}/Some file.jpg", 'Project') - end - end - end - - describe '#model_id' do - def assert_model_id(file_path, expected_model_id) - untracked_file = create_untracked_file(file_path) - - expect(untracked_file.model_id).to eq(expected_model_id) - end - - context 'for an appearance logo file path' do - it 'returns the ID as a string' do - assert_model_id('/-/system/appearance/logo/1/some_logo.jpg', 1) - end - end - - context 'for an appearance header_logo file path' do - it 'returns the ID as a string' do - assert_model_id('/-/system/appearance/header_logo/1/some_logo.jpg', 1) - end - end - - context 'for a pre-Markdown Note attachment file path' do - it 'returns the ID as a string' do - assert_model_id('/-/system/note/attachment/1234/some_attachment.pdf', 1234) - end - end - - context 'for a user avatar file path' do - it 'returns the ID as a string' do - assert_model_id('/-/system/user/avatar/1234/avatar.jpg', 1234) - end - end - - context 'for a group avatar file path' do - it 'returns the ID as a string' do - assert_model_id('/-/system/group/avatar/1234/avatar.jpg', 1234) - end - end - - context 'for a project avatar file path' do - it 'returns the ID as a string' do - assert_model_id('/-/system/project/avatar/1234/avatar.jpg', 1234) - end - end - - context 'for a project Markdown attachment (notes, issues, MR descriptions) file path' do - it 'returns the ID as a string' do - project = create_project - - assert_model_id("/#{get_full_path(project)}/#{SecureRandom.hex}/Some file.jpg", project.id) - end - end - end - - describe '#file_size' do - context 'for an appearance logo file path' do - let(:appearance) { create_or_update_appearance(logo: true) } - let(:untracked_file) { described_class.create!(path: get_uploads(appearance, 'Appearance').first.path) } - - it 'returns the file size' do - expect(untracked_file.file_size).to eq(1062) - end - end - - context 'for a project avatar file path' do - let(:project) { create_project(avatar: true) } - let(:untracked_file) { described_class.create!(path: get_uploads(project, 'Project').first.path) } - - it 'returns the file size' do - expect(untracked_file.file_size).to eq(1062) - end - end - - context 'for a project Markdown attachment (notes, issues, MR descriptions) file path' do - let(:project) { create_project } - let(:untracked_file) { create_untracked_file("/#{get_full_path(project)}/#{get_uploads(project, 'Project').first.path}") } - - before do - add_markdown_attachment(project) - end - - it 'returns the file size' do - expect(untracked_file.file_size).to eq(1062) - end - end - end - - def create_untracked_file(path_relative_to_upload_dir) - described_class.create!(path: "#{Gitlab::BackgroundMigration::PrepareUntrackedUploads::RELATIVE_UPLOAD_DIR}#{path_relative_to_upload_dir}") - end -end diff --git a/spec/lib/gitlab/background_migration/populate_untracked_uploads_spec.rb b/spec/lib/gitlab/background_migration/populate_untracked_uploads_spec.rb deleted file mode 100644 index 787cc54e79a..00000000000 --- a/spec/lib/gitlab/background_migration/populate_untracked_uploads_spec.rb +++ /dev/null @@ -1,254 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe Gitlab::BackgroundMigration::PopulateUntrackedUploads do - include MigrationsHelpers::TrackUntrackedUploadsHelpers - - subject { described_class.new } - - let!(:appearances) { table(:appearances) } - let!(:namespaces) { table(:namespaces) } - let!(:notes) { table(:notes) } - let!(:projects) { table(:projects) } - let!(:routes) { table(:routes) } - let!(:untracked_files_for_uploads) { table(:untracked_files_for_uploads) } - let!(:uploads) { table(:uploads) } - let!(:users) { table(:users) } - - before do - ensure_temporary_tracking_table_exists - uploads.delete_all - end - - context 'with untracked files and tracked files in untracked_files_for_uploads' do - let!(:appearance) { create_or_update_appearance(logo: true, header_logo: true) } - let!(:user1) { create_user(avatar: true) } - let!(:user2) { create_user(avatar: true) } - let!(:project1) { create_project(avatar: true) } - let!(:project2) { create_project(avatar: true) } - - before do - add_markdown_attachment(project1) - add_markdown_attachment(project2) - - # File records created by PrepareUntrackedUploads - untracked_files_for_uploads.create!(path: get_uploads(appearance, 'Appearance').first.path) - untracked_files_for_uploads.create!(path: get_uploads(appearance, 'Appearance').last.path) - untracked_files_for_uploads.create!(path: get_uploads(user1, 'User').first.path) - untracked_files_for_uploads.create!(path: get_uploads(user2, 'User').first.path) - untracked_files_for_uploads.create!(path: get_uploads(project1, 'Project').first.path) - untracked_files_for_uploads.create!(path: get_uploads(project2, 'Project').first.path) - untracked_files_for_uploads.create!(path: "#{legacy_project_uploads_dir(project1).sub("#{MigrationsHelpers::TrackUntrackedUploadsHelpers::PUBLIC_DIR}/", '')}/#{get_uploads(project1, 'Project').last.path}") - untracked_files_for_uploads.create!(path: "#{legacy_project_uploads_dir(project2).sub("#{MigrationsHelpers::TrackUntrackedUploadsHelpers::PUBLIC_DIR}/", '')}/#{get_uploads(project2, 'Project').last.path}") - - # Untrack 4 files - get_uploads(user2, 'User').delete_all - get_uploads(project2, 'Project').delete_all # 2 files: avatar and a Markdown upload - get_uploads(appearance, 'Appearance').where("path like '%header_logo%'").delete_all - end - - it 'adds untracked files to the uploads table' do - expect do - subject.perform(1, untracked_files_for_uploads.reorder(:id).last.id) - end.to change { uploads.count }.from(4).to(8) - - expect(get_uploads(user2, 'User').count).to eq(1) - expect(get_uploads(project2, 'Project').count).to eq(2) - expect(get_uploads(appearance, 'Appearance').count).to eq(2) - end - - it 'deletes rows after processing them' do - expect(subject).to receive(:drop_temp_table_if_finished) # Don't drop the table so we can look at it - - expect do - subject.perform(1, untracked_files_for_uploads.last.id) - end.to change { untracked_files_for_uploads.count }.from(8).to(0) - end - - it 'does not create duplicate uploads of already tracked files' do - subject.perform(1, untracked_files_for_uploads.last.id) - - expect(get_uploads(user1, 'User').count).to eq(1) - expect(get_uploads(project1, 'Project').count).to eq(2) - expect(get_uploads(appearance, 'Appearance').count).to eq(2) - end - - it 'uses the start and end batch ids [only 1st half]' do - ids = untracked_files_for_uploads.all.order(:id).pluck(:id) - start_id = ids[0] - end_id = ids[3] - - expect do - subject.perform(start_id, end_id) - end.to change { uploads.count }.from(4).to(6) - - expect(get_uploads(user1, 'User').count).to eq(1) - expect(get_uploads(user2, 'User').count).to eq(1) - expect(get_uploads(appearance, 'Appearance').count).to eq(2) - expect(get_uploads(project1, 'Project').count).to eq(2) - expect(get_uploads(project2, 'Project').count).to eq(0) - - # Only 4 have been either confirmed or added to uploads - expect(untracked_files_for_uploads.count).to eq(4) - end - - it 'uses the start and end batch ids [only 2nd half]' do - ids = untracked_files_for_uploads.all.order(:id).pluck(:id) - start_id = ids[4] - end_id = ids[7] - - expect do - subject.perform(start_id, end_id) - end.to change { uploads.count }.from(4).to(6) - - expect(get_uploads(user1, 'User').count).to eq(1) - expect(get_uploads(user2, 'User').count).to eq(0) - expect(get_uploads(appearance, 'Appearance').count).to eq(1) - expect(get_uploads(project1, 'Project').count).to eq(2) - expect(get_uploads(project2, 'Project').count).to eq(2) - - # Only 4 have been either confirmed or added to uploads - expect(untracked_files_for_uploads.count).to eq(4) - end - - it 'does not drop the temporary tracking table after processing the batch, if there are still untracked rows' do - subject.perform(1, untracked_files_for_uploads.last.id - 1) - - expect(ActiveRecord::Base.connection.table_exists?(:untracked_files_for_uploads)).to be_truthy - end - - it 'drops the temporary tracking table after processing the batch, if there are no untracked rows left' do - expect(subject).to receive(:drop_temp_table_if_finished) - - subject.perform(1, untracked_files_for_uploads.last.id) - end - - it 'does not block a whole batch because of one bad path' do - untracked_files_for_uploads.create!(path: "#{Gitlab::BackgroundMigration::PrepareUntrackedUploads::RELATIVE_UPLOAD_DIR}/#{get_full_path(project2)}/._7d37bf4c747916390e596744117d5d1a") - expect(untracked_files_for_uploads.count).to eq(9) - expect(uploads.count).to eq(4) - - subject.perform(1, untracked_files_for_uploads.last.id) - - expect(untracked_files_for_uploads.count).to eq(1) - expect(uploads.count).to eq(8) - end - - it 'an unparseable path is shown in error output' do - bad_path = "#{Gitlab::BackgroundMigration::PrepareUntrackedUploads::RELATIVE_UPLOAD_DIR}/#{get_full_path(project2)}/._7d37bf4c747916390e596744117d5d1a" - untracked_files_for_uploads.create!(path: bad_path) - - expect(Rails.logger).to receive(:error).with(/Error parsing path "#{bad_path}":/) - - subject.perform(1, untracked_files_for_uploads.last.id) - end - end - - context 'with no untracked files' do - it 'does not add to the uploads table (and does not raise error)' do - expect do - subject.perform(1, 1000) - end.not_to change { uploads.count }.from(0) - end - end - - describe 'upload outcomes for each path pattern' do - shared_examples_for 'non_markdown_file' do - let!(:expected_upload_attrs) { model_uploads.first.attributes.slice('path', 'uploader', 'size', 'checksum') } - let!(:untracked_file) { untracked_files_for_uploads.create!(path: expected_upload_attrs['path']) } - - before do - model_uploads.delete_all - end - - it 'creates an Upload record' do - expect do - subject.perform(1, untracked_files_for_uploads.last.id) - end.to change { model_uploads.count }.from(0).to(1) - - expect(model_uploads.first.attributes).to include(expected_upload_attrs) - end - end - - context 'for an appearance logo file path' do - let(:model) { create_or_update_appearance(logo: true) } - let(:model_uploads) { get_uploads(model, 'Appearance') } - - it_behaves_like 'non_markdown_file' - end - - context 'for an appearance header_logo file path' do - let(:model) { create_or_update_appearance(header_logo: true) } - let(:model_uploads) { get_uploads(model, 'Appearance') } - - it_behaves_like 'non_markdown_file' - end - - context 'for a pre-Markdown Note attachment file path' do - let(:model) { create_note(attachment: true) } - let!(:expected_upload_attrs) { get_uploads(model, 'Note').first.attributes.slice('path', 'uploader', 'size', 'checksum') } - let!(:untracked_file) { untracked_files_for_uploads.create!(path: expected_upload_attrs['path']) } - - before do - get_uploads(model, 'Note').delete_all - end - - # Can't use the shared example because Note doesn't have an `uploads` association - it 'creates an Upload record' do - expect do - subject.perform(1, untracked_files_for_uploads.last.id) - end.to change { get_uploads(model, 'Note').count }.from(0).to(1) - - expect(get_uploads(model, 'Note').first.attributes).to include(expected_upload_attrs) - end - end - - context 'for a user avatar file path' do - let(:model) { create_user(avatar: true) } - let(:model_uploads) { get_uploads(model, 'User') } - - it_behaves_like 'non_markdown_file' - end - - context 'for a group avatar file path' do - let(:model) { create_group(avatar: true) } - let(:model_uploads) { get_uploads(model, 'Namespace') } - - it_behaves_like 'non_markdown_file' - end - - context 'for a project avatar file path' do - let(:model) { create_project(avatar: true) } - let(:model_uploads) { get_uploads(model, 'Project') } - - it_behaves_like 'non_markdown_file' - end - - context 'for a project Markdown attachment (notes, issues, MR descriptions) file path' do - let(:model) { create_project } - - before do - # Upload the file - add_markdown_attachment(model) - - # Create the untracked_files_for_uploads record - untracked_files_for_uploads.create!(path: "#{Gitlab::BackgroundMigration::PrepareUntrackedUploads::RELATIVE_UPLOAD_DIR}/#{get_full_path(model)}/#{get_uploads(model, 'Project').first.path}") - - # Save the expected upload attributes - @expected_upload_attrs = get_uploads(model, 'Project').first.attributes.slice('path', 'uploader', 'size', 'checksum') - - # Untrack the file - get_uploads(model, 'Project').delete_all - end - - it 'creates an Upload record' do - expect do - subject.perform(1, untracked_files_for_uploads.last.id) - end.to change { get_uploads(model, 'Project').count }.from(0).to(1) - - expect(get_uploads(model, 'Project').first.attributes).to include(@expected_upload_attrs) - end - end - end -end diff --git a/spec/lib/gitlab/background_migration/prepare_untracked_uploads_spec.rb b/spec/lib/gitlab/background_migration/prepare_untracked_uploads_spec.rb deleted file mode 100644 index 9b01407dc8b..00000000000 --- a/spec/lib/gitlab/background_migration/prepare_untracked_uploads_spec.rb +++ /dev/null @@ -1,159 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -# Rollback DB to 10.5 (later than this was originally written for) because it still needs to work. -RSpec.describe Gitlab::BackgroundMigration::PrepareUntrackedUploads do - include MigrationsHelpers::TrackUntrackedUploadsHelpers - - let!(:untracked_files_for_uploads) { table(:untracked_files_for_uploads) } - let!(:appearances) { table(:appearances) } - let!(:namespaces) { table(:namespaces) } - let!(:projects) { table(:projects) } - let!(:routes) { table(:routes) } - let!(:uploads) { table(:uploads) } - let!(:users) { table(:users) } - - around do |example| - # Especially important so the follow-up migration does not get run - Sidekiq::Testing.fake! do - example.run - end - end - - shared_examples 'prepares the untracked_files_for_uploads table' do - context 'when files were uploaded before and after hashed storage was enabled' do - let!(:appearance) { create_or_update_appearance(logo: true, header_logo: true) } - let!(:user) { create_user(avatar: true) } - let!(:project1) { create_project(avatar: true) } - let(:project2) { create_project } # instantiate after enabling hashed_storage - - before do - # Markdown upload before enabling hashed_storage - add_markdown_attachment(project1) - - # Markdown upload after enabling hashed_storage - add_markdown_attachment(project2, hashed_storage: true) - end - - it 'has a path field long enough for really long paths' do - described_class.new.perform - - component = 'a' * 255 - - long_path = [ - 'uploads', - component, # project.full_path - component # filename - ].flatten.join('/') - - record = untracked_files_for_uploads.create!(path: long_path) - expect(record.reload.path.size).to eq(519) - end - - it 'adds unhashed files to the untracked_files_for_uploads table' do - described_class.new.perform - - expect(untracked_files_for_uploads.count).to eq(5) - end - - it 'adds files with paths relative to CarrierWave.root' do - described_class.new.perform - untracked_files_for_uploads.all.each do |file| - expect(file.path.start_with?('uploads/')).to be_truthy - end - end - - it 'does not add hashed files to the untracked_files_for_uploads table' do - described_class.new.perform - - hashed_file_path = get_uploads(project2, 'Project').find_by(uploader: 'FileUploader').path - expect(untracked_files_for_uploads.where("path like '%#{hashed_file_path}%'").exists?).to be_falsey - end - - it 'correctly schedules the follow-up background migration jobs' do - described_class.new.perform - - ids = described_class::UntrackedFile.all.order(:id).pluck(:id) - expect(described_class::FOLLOW_UP_MIGRATION).to be_scheduled_migration(ids.first, ids.last) - expect(BackgroundMigrationWorker.jobs.size).to eq(1) - end - - # E.g. from a previous failed run of this background migration - context 'when there is existing data in untracked_files_for_uploads' do - before do - described_class.new.perform - end - - it 'does not error or produce duplicates of existing data' do - expect do - described_class.new.perform - end.not_to change { untracked_files_for_uploads.count }.from(5) - end - end - - # E.g. The installation is in use at the time of migration, and someone has - # just uploaded a file - context 'when there are files in /uploads/tmp' do - let(:tmp_file) { Rails.root.join(described_class::ABSOLUTE_UPLOAD_DIR, 'tmp', 'some_file.jpg') } - - before do - FileUtils.mkdir(File.dirname(tmp_file)) - FileUtils.touch(tmp_file) - end - - after do - FileUtils.rm(tmp_file) - end - - it 'does not add files from /uploads/tmp' do - described_class.new.perform - - expect(untracked_files_for_uploads.count).to eq(5) - end - end - - context 'when the last batch size exactly matches the max batch size' do - it 'does not raise error' do - stub_const("#{described_class}::FIND_BATCH_SIZE", 5) - - expect do - described_class.new.perform - end.not_to raise_error - - expect(untracked_files_for_uploads.count).to eq(5) - end - end - end - end - - # If running on Postgres 9.2 (like on CI), this whole context is skipped - # since we're unable to use ON CONFLICT DO NOTHING or IGNORE. - context "test bulk insert with ON CONFLICT DO NOTHING or IGNORE", if: described_class.new.send(:can_bulk_insert_and_ignore_duplicates?) do - it_behaves_like 'prepares the untracked_files_for_uploads table' - end - - # If running on Postgres 9.2 (like on CI), the stubbed method has no effect. - # - # If running on Postgres 9.5+ or MySQL, then this context effectively tests - # the bulk insert functionality without ON CONFLICT DO NOTHING or IGNORE. - context 'test bulk insert without ON CONFLICT DO NOTHING or IGNORE' do - before do - allow_any_instance_of(described_class).to receive(:postgresql_pre_9_5?).and_return(true) - end - - it_behaves_like 'prepares the untracked_files_for_uploads table' - end - - # Very new or lightly-used installations that are running this migration - # may not have an upload directory because they have no uploads. - context 'when no files were ever uploaded' do - it 'deletes the `untracked_files_for_uploads` table (and does not raise error)' do - background_migration = described_class.new - - expect(background_migration).to receive(:drop_temp_table) - - background_migration.perform - end - end -end diff --git a/spec/lib/gitlab/background_migration/remove_restricted_todos_spec.rb b/spec/lib/gitlab/background_migration/remove_restricted_todos_spec.rb deleted file mode 100644 index 7019d5d4212..00000000000 --- a/spec/lib/gitlab/background_migration/remove_restricted_todos_spec.rb +++ /dev/null @@ -1,126 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe Gitlab::BackgroundMigration::RemoveRestrictedTodos do - let(:namespaces) { table(:namespaces) } - let(:projects) { table(:projects) } - let(:users) { table(:users) } - let(:todos) { table(:todos) } - let(:issues) { table(:issues) } - let(:assignees) { table(:issue_assignees) } - let(:project_authorizations) { table(:project_authorizations) } - let(:project_features) { table(:project_features) } - - let(:todo_params) { { author_id: 1, target_type: 'Issue', action: 1, state: :pending } } - - before do - users.create(id: 1, email: 'user@example.com', projects_limit: 10) - users.create(id: 2, email: 'reporter@example.com', projects_limit: 10) - users.create(id: 3, email: 'guest@example.com', projects_limit: 10) - - namespace = namespaces.create(name: 'gitlab-org', path: 'gitlab-org') - projects.create!(id: 1, name: 'project-1', path: 'project-1', visibility_level: 0, namespace_id: namespace.id) - projects.create!(id: 2, name: 'project-2', path: 'project-2', visibility_level: 0, namespace_id: namespace.id) - - issues.create(id: 1, project_id: 1) - issues.create(id: 2, project_id: 2) - - project_authorizations.create(user_id: 2, project_id: 2, access_level: 20) # reporter - project_authorizations.create(user_id: 3, project_id: 2, access_level: 10) # guest - - todos.create(todo_params.merge(user_id: 1, project_id: 1, target_id: 1)) # out of project ids range - todos.create(todo_params.merge(user_id: 1, project_id: 2, target_id: 2)) # non member - todos.create(todo_params.merge(user_id: 2, project_id: 2, target_id: 2)) # reporter - todos.create(todo_params.merge(user_id: 3, project_id: 2, target_id: 2)) # guest - end - - subject { described_class.new.perform(2, 5) } - - context 'when a project is private' do - it 'removes todos of users without project access' do - expect { subject }.to change { Todo.count }.from(4).to(3) - end - - context 'with a confidential issue' do - it 'removes todos of users without project access and guests for confidential issues' do - issues.create(id: 3, project_id: 2, confidential: true) - issues.create(id: 4, project_id: 1, confidential: true) # not in the batch - todos.create(todo_params.merge(user_id: 3, project_id: 2, target_id: 3)) - todos.create(todo_params.merge(user_id: 2, project_id: 2, target_id: 3)) - todos.create(todo_params.merge(user_id: 1, project_id: 1, target_id: 4)) - - expect { subject }.to change { Todo.count }.from(7).to(5) - end - end - end - - context 'when a project is public' do - before do - projects.find(2).update_attribute(:visibility_level, 20) - end - - context 'when all features have the same visibility as the project, no confidential issues' do - it 'does not remove any todos' do - expect { subject }.not_to change { Todo.count } - end - end - - context 'with confidential issues' do - before do - users.create(id: 4, email: 'author@example.com', projects_limit: 10) - users.create(id: 5, email: 'assignee@example.com', projects_limit: 10) - issues.create(id: 3, project_id: 2, confidential: true, author_id: 4) - assignees.create(user_id: 5, issue_id: 3) - - todos.create(todo_params.merge(user_id: 1, project_id: 2, target_id: 3)) # to be deleted - todos.create(todo_params.merge(user_id: 2, project_id: 2, target_id: 3)) # authorized user - todos.create(todo_params.merge(user_id: 3, project_id: 2, target_id: 3)) # to be deleted guest - todos.create(todo_params.merge(user_id: 4, project_id: 2, target_id: 3)) # conf issue author - todos.create(todo_params.merge(user_id: 5, project_id: 2, target_id: 3)) # conf issue assignee - end - - it 'removes confidential issue todos for non authorized users' do - expect { subject }.to change { Todo.count }.from(9).to(7) - end - end - - context 'features visibility restrictions' do - before do - todo_params.merge!(project_id: 2, user_id: 1, target_id: 3) - todos.create(todo_params.merge(user_id: 1, target_id: 3, target_type: 'MergeRequest')) - todos.create(todo_params.merge(user_id: 1, target_id: 3, target_type: 'Commit')) - end - - context 'when issues are restricted to project members' do - before do - project_features.create(issues_access_level: 10, pages_access_level: 10, project_id: 2) - end - - it 'removes non members issue todos' do - expect { subject }.to change { Todo.count }.from(6).to(5) - end - end - - context 'when merge requests are restricted to project members' do - before do - project_features.create(merge_requests_access_level: 10, pages_access_level: 10, project_id: 2) - end - - it 'removes non members issue todos' do - expect { subject }.to change { Todo.count }.from(6).to(5) - end - end - - context 'when repository and merge requests are restricted to project members' do - before do - project_features.create(repository_access_level: 10, merge_requests_access_level: 10, pages_access_level: 10, project_id: 2) - end - - it 'removes non members commit and merge requests todos' do - expect { subject }.to change { Todo.count }.from(6).to(4) - end - end - end - end -end diff --git a/spec/lib/gitlab/background_migration/set_confidential_note_events_on_services_spec.rb b/spec/lib/gitlab/background_migration/set_confidential_note_events_on_services_spec.rb deleted file mode 100644 index 364edf3ed2a..00000000000 --- a/spec/lib/gitlab/background_migration/set_confidential_note_events_on_services_spec.rb +++ /dev/null @@ -1,33 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe Gitlab::BackgroundMigration::SetConfidentialNoteEventsOnServices do - let(:services) { table(:services) } - - describe '#perform' do - it 'migrates services where note_events is true' do - service = services.create(confidential_note_events: nil, note_events: true) - - subject.perform(service.id, service.id) - - expect(service.reload.confidential_note_events).to eq(true) - end - - it 'ignores services where note_events is false' do - service = services.create(confidential_note_events: nil, note_events: false) - - subject.perform(service.id, service.id) - - expect(service.reload.confidential_note_events).to eq(nil) - end - - it 'ignores services where confidential_note_events has already been set' do - service = services.create(confidential_note_events: false, note_events: true) - - subject.perform(service.id, service.id) - - expect(service.reload.confidential_note_events).to eq(false) - end - end -end diff --git a/spec/lib/gitlab/background_migration/set_confidential_note_events_on_webhooks_spec.rb b/spec/lib/gitlab/background_migration/set_confidential_note_events_on_webhooks_spec.rb deleted file mode 100644 index 28b06ac3ba3..00000000000 --- a/spec/lib/gitlab/background_migration/set_confidential_note_events_on_webhooks_spec.rb +++ /dev/null @@ -1,33 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe Gitlab::BackgroundMigration::SetConfidentialNoteEventsOnWebhooks do - let(:web_hooks) { table(:web_hooks) } - - describe '#perform' do - it 'migrates hooks where note_events is true' do - hook = web_hooks.create(confidential_note_events: nil, note_events: true) - - subject.perform(hook.id, hook.id) - - expect(hook.reload.confidential_note_events).to eq(true) - end - - it 'ignores hooks where note_events is false' do - hook = web_hooks.create(confidential_note_events: nil, note_events: false) - - subject.perform(hook.id, hook.id) - - expect(hook.reload.confidential_note_events).to eq(nil) - end - - it 'ignores hooks where confidential_note_events has already been set' do - hook = web_hooks.create(confidential_note_events: false, note_events: true) - - subject.perform(hook.id, hook.id) - - expect(hook.reload.confidential_note_events).to eq(false) - end - end -end diff --git a/spec/lib/gitlab/background_migration/set_merge_request_diff_files_count_spec.rb b/spec/lib/gitlab/background_migration/set_merge_request_diff_files_count_spec.rb new file mode 100644 index 00000000000..6e9f51f510a --- /dev/null +++ b/spec/lib/gitlab/background_migration/set_merge_request_diff_files_count_spec.rb @@ -0,0 +1,40 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::BackgroundMigration::SetMergeRequestDiffFilesCount, schema: 20200807152315 do + let(:merge_request_diff_files) { table(:merge_request_diff_files) } + let(:merge_request_diffs) { table(:merge_request_diffs) } + let(:merge_requests) { table(:merge_requests) } + let(:namespaces) { table(:namespaces) } + let(:projects) { table(:projects) } + + let(:namespace) { namespaces.create!(name: 'foo', path: 'foo') } + let(:project) { projects.create!(namespace_id: namespace.id) } + let(:merge_request) { merge_requests.create!(source_branch: 'x', target_branch: 'master', target_project_id: project.id) } + + it 'fills the files_count column' do + empty_diff = merge_request_diffs.create!(merge_request_id: merge_request.id) + filled_diff = merge_request_diffs.create!(merge_request_id: merge_request.id) + + 3.times do |n| + merge_request_diff_files.create!( + merge_request_diff_id: filled_diff.id, + relative_order: n, + new_file: false, + renamed_file: false, + deleted_file: false, + too_large: false, + a_mode: '', + b_mode: '', + old_path: '', + new_path: '' + ) + end + + described_class.new.perform(empty_diff.id, filled_diff.id) + + expect(empty_diff.reload.files_count).to eq(0) + expect(filled_diff.reload.files_count).to eq(3) + end +end diff --git a/spec/lib/gitlab/background_migration/set_null_external_diff_store_to_local_value_spec.rb b/spec/lib/gitlab/background_migration/set_null_external_diff_store_to_local_value_spec.rb new file mode 100644 index 00000000000..6079ad2dd2a --- /dev/null +++ b/spec/lib/gitlab/background_migration/set_null_external_diff_store_to_local_value_spec.rb @@ -0,0 +1,33 @@ +# frozen_string_literal: true + +require 'spec_helper' + +# The test setup must begin before +# 20200804041930_add_not_null_constraint_on_external_diff_store_to_merge_request_diffs.rb +# has run, or else we cannot insert a row with `NULL` `external_diff_store` to +# test against. +RSpec.describe Gitlab::BackgroundMigration::SetNullExternalDiffStoreToLocalValue, schema: 20200804035230 do + let!(:merge_request_diffs) { table(:merge_request_diffs) } + let!(:merge_requests) { table(:merge_requests) } + let!(:namespaces) { table(:namespaces) } + let!(:projects) { table(:projects) } + let!(:namespace) { namespaces.create!(name: 'foo', path: 'foo') } + let!(:project) { projects.create!(namespace_id: namespace.id) } + let!(:merge_request) { merge_requests.create!(source_branch: 'x', target_branch: 'master', target_project_id: project.id) } + + it 'correctly migrates nil external_diff_store to 1' do + external_diff_store_1 = merge_request_diffs.create!(external_diff_store: 1, merge_request_id: merge_request.id) + external_diff_store_2 = merge_request_diffs.create!(external_diff_store: 2, merge_request_id: merge_request.id) + external_diff_store_nil = merge_request_diffs.create!(external_diff_store: nil, merge_request_id: merge_request.id) + + described_class.new.perform(external_diff_store_1.id, external_diff_store_nil.id) + + external_diff_store_1.reload + external_diff_store_2.reload + external_diff_store_nil.reload + + expect(external_diff_store_1.external_diff_store).to eq(1) # unchanged + expect(external_diff_store_2.external_diff_store).to eq(2) # unchanged + expect(external_diff_store_nil.external_diff_store).to eq(1) # nil => 1 + end +end diff --git a/spec/lib/gitlab/background_migration/set_null_package_files_file_store_to_local_value_spec.rb b/spec/lib/gitlab/background_migration/set_null_package_files_file_store_to_local_value_spec.rb new file mode 100644 index 00000000000..40d41262fc7 --- /dev/null +++ b/spec/lib/gitlab/background_migration/set_null_package_files_file_store_to_local_value_spec.rb @@ -0,0 +1,33 @@ +# frozen_string_literal: true + +require 'spec_helper' + +# The test setup must begin before +# 20200806004742_add_not_null_constraint_on_file_store_to_package_files.rb +# has run, or else we cannot insert a row with `NULL` `file_store` to +# test against. +RSpec.describe Gitlab::BackgroundMigration::SetNullPackageFilesFileStoreToLocalValue, schema: 20200806004232 do + let!(:packages_package_files) { table(:packages_package_files) } + let!(:packages_packages) { table(:packages_packages) } + let!(:projects) { table(:projects) } + let!(:namespaces) { table(:namespaces) } + let!(:namespace) { namespaces.create!(name: 'foo', path: 'foo') } + let!(:project) { projects.create!(namespace_id: namespace.id) } + let!(:package) { packages_packages.create!(project_id: project.id, name: 'bar', package_type: 1) } + + it 'correctly migrates nil file_store to 1' do + file_store_1 = packages_package_files.create!(file_store: 1, file_name: 'foo_1', file: 'foo_1', package_id: package.id) + file_store_2 = packages_package_files.create!(file_store: 2, file_name: 'foo_2', file: 'foo_2', package_id: package.id) + file_store_nil = packages_package_files.create!(file_store: nil, file_name: 'foo_nil', file: 'foo_nil', package_id: package.id) + + described_class.new.perform(file_store_1.id, file_store_nil.id) + + file_store_1.reload + file_store_2.reload + file_store_nil.reload + + expect(file_store_1.file_store).to eq(1) # unchanged + expect(file_store_2.file_store).to eq(2) # unchanged + expect(file_store_nil.file_store).to eq(1) # nil => 1 + end +end diff --git a/spec/lib/gitlab/background_migration/user_mentions/create_resource_user_mention_spec.rb b/spec/lib/gitlab/background_migration/user_mentions/create_resource_user_mention_spec.rb index 08a4bbe38ac..392b44d1a1f 100644 --- a/spec/lib/gitlab/background_migration/user_mentions/create_resource_user_mention_spec.rb +++ b/spec/lib/gitlab/background_migration/user_mentions/create_resource_user_mention_spec.rb @@ -75,6 +75,14 @@ RSpec.describe Gitlab::BackgroundMigration::UserMentions::CreateResourceUserMent let(:resource) { merge_request } it_behaves_like 'resource mentions migration', MigrateMergeRequestMentionsToDb, MergeRequest + + context 'when FF disabled' do + before do + stub_feature_flags(migrate_user_mentions: false) + end + + it_behaves_like 'resource migration not run', MigrateMergeRequestMentionsToDb, MergeRequest + end end context 'migrate commit mentions' do @@ -96,6 +104,14 @@ RSpec.describe Gitlab::BackgroundMigration::UserMentions::CreateResourceUserMent let(:resource) { commit } it_behaves_like 'resource notes mentions migration', MigrateCommitNotesMentionsToDb, Commit + + context 'when FF disabled' do + before do + stub_feature_flags(migrate_user_mentions: false) + end + + it_behaves_like 'resource notes migration not run', MigrateCommitNotesMentionsToDb, Commit + end end end |