diff options
author | Francisco Javier López <fjlopez@gitlab.com> | 2018-12-05 17:39:15 +0300 |
---|---|---|
committer | Grzegorz Bizon <grzegorz@gitlab.com> | 2018-12-05 17:39:15 +0300 |
commit | a6778fc647d83c8fbdca69e1bb2d38b490853ba8 (patch) | |
tree | 93d1d2bc4c5bd85e5255b9cfdf29a88ca9af127f | |
parent | 107ff10eb68aa8715b8bfc5a8ae13bf2f4453355 (diff) |
Rename project's pipelines relation
39 files changed, 276 insertions, 72 deletions
diff --git a/app/controllers/projects/branches_controller.rb b/app/controllers/projects/branches_controller.rb index 95a014d24da..a6bfb913900 100644 --- a/app/controllers/projects/branches_controller.rb +++ b/app/controllers/projects/branches_controller.rb @@ -22,7 +22,7 @@ class Projects::BranchesController < Projects::ApplicationController # Fetch branches for the specified mode fetch_branches_by_mode - @refs_pipelines = @project.pipelines.latest_successful_for_refs(@branches.map(&:name)) + @refs_pipelines = @project.ci_pipelines.latest_successful_for_refs(@branches.map(&:name)) @merged_branch_names = repository.merged_branch_names(@branches.map(&:name)) # n+1: https://gitlab.com/gitlab-org/gitaly/issues/992 diff --git a/app/controllers/projects/pipelines_controller.rb b/app/controllers/projects/pipelines_controller.rb index 53b29d4146e..67827b1d3bb 100644 --- a/app/controllers/projects/pipelines_controller.rb +++ b/app/controllers/projects/pipelines_controller.rb @@ -46,7 +46,7 @@ class Projects::PipelinesController < Projects::ApplicationController end def new - @pipeline = project.pipelines.new(ref: @project.default_branch) + @pipeline = project.all_pipelines.new(ref: @project.default_branch) end def create @@ -142,9 +142,9 @@ class Projects::PipelinesController < Projects::ApplicationController @charts[:pipeline_times] = Gitlab::Ci::Charts::PipelineTime.new(project) @counts = {} - @counts[:total] = @project.pipelines.count(:all) - @counts[:success] = @project.pipelines.success.count(:all) - @counts[:failed] = @project.pipelines.failed.count(:all) + @counts[:total] = @project.all_pipelines.count(:all) + @counts[:success] = @project.all_pipelines.success.count(:all) + @counts[:failed] = @project.all_pipelines.failed.count(:all) end private @@ -164,7 +164,7 @@ class Projects::PipelinesController < Projects::ApplicationController # rubocop: disable CodeReuse/ActiveRecord def pipeline @pipeline ||= project - .pipelines + .all_pipelines .includes(user: :status) .find_by!(id: params[:id]) .present(current_user: current_user) diff --git a/app/controllers/projects/tags_controller.rb b/app/controllers/projects/tags_controller.rb index 2b28670a49b..686d66b10a3 100644 --- a/app/controllers/projects/tags_controller.rb +++ b/app/controllers/projects/tags_controller.rb @@ -20,7 +20,7 @@ class Projects::TagsController < Projects::ApplicationController @tags = Kaminari.paginate_array(@tags).page(params[:page]) tag_names = @tags.map(&:name) - @tags_pipelines = @project.pipelines.latest_successful_for_refs(tag_names) + @tags_pipelines = @project.ci_pipelines.latest_successful_for_refs(tag_names) @releases = project.releases.where(tag: tag_names) respond_to do |format| diff --git a/app/finders/pipelines_finder.rb b/app/finders/pipelines_finder.rb index 35d0e1acce5..f5aadc42ff0 100644 --- a/app/finders/pipelines_finder.rb +++ b/app/finders/pipelines_finder.rb @@ -8,7 +8,7 @@ class PipelinesFinder def initialize(project, current_user, params = {}) @project = project @current_user = current_user - @pipelines = project.pipelines + @pipelines = project.all_pipelines @params = params end diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb index 88ab021817a..60ff2181a95 100644 --- a/app/models/ci/pipeline.rb +++ b/app/models/ci/pipeline.rb @@ -12,14 +12,14 @@ module Ci include AtomicInternalId include EnumWithNil - belongs_to :project, inverse_of: :pipelines + belongs_to :project, inverse_of: :all_pipelines belongs_to :user belongs_to :auto_canceled_by, class_name: 'Ci::Pipeline' belongs_to :pipeline_schedule, class_name: 'Ci::PipelineSchedule' belongs_to :merge_request, class_name: 'MergeRequest' has_internal_id :iid, scope: :project, presence: false, init: ->(s) do - s&.project&.pipelines&.maximum(:iid) || s&.project&.pipelines&.count + s&.project&.all_pipelines&.maximum(:iid) || s&.project&.all_pipelines&.count end has_many :stages, -> { order(position: :asc) }, inverse_of: :pipeline @@ -174,6 +174,7 @@ module Ci end scope :internal, -> { where(source: internal_sources) } + scope :ci_sources, -> { where(config_source: ci_sources_values) } scope :sort_by_merge_request_pipelines, -> do sql = 'CASE ci_pipelines.source WHEN (?) THEN 0 ELSE 1 END, ci_pipelines.id DESC' @@ -271,6 +272,10 @@ module Ci sources.reject { |source| source == "external" }.values end + def self.ci_sources_values + config_sources.values_at(:repository_source, :auto_devops_source, :unknown_source) + end + def stages_count statuses.select(:stage).distinct.count end diff --git a/app/models/commit.rb b/app/models/commit.rb index 546fcc54a15..2c89da88b9b 100644 --- a/app/models/commit.rb +++ b/app/models/commit.rb @@ -298,7 +298,7 @@ class Commit end def pipelines - project.pipelines.where(sha: sha) + project.ci_pipelines.where(sha: sha) end def last_pipeline @@ -312,7 +312,7 @@ class Commit end def status_for_project(ref, pipeline_project) - pipeline_project.pipelines.latest_status_per_commit(id, ref)[id] + pipeline_project.ci_pipelines.latest_status_per_commit(id, ref)[id] end def set_status_for_ref(ref, status) diff --git a/app/models/commit_collection.rb b/app/models/commit_collection.rb index dd93af9df64..e349f0fe971 100644 --- a/app/models/commit_collection.rb +++ b/app/models/commit_collection.rb @@ -24,7 +24,7 @@ class CommitCollection # Setting this status ahead of time removes the need for running a query for # every commit we're displaying. def with_pipeline_status - statuses = project.pipelines.latest_status_per_commit(map(&:id), ref) + statuses = project.ci_pipelines.latest_status_per_commit(map(&:id), ref) each do |commit| commit.set_status_for_ref(ref, statuses[commit.id]) diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 6b05ee06199..f40dff7c1bd 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -1056,7 +1056,7 @@ class MergeRequest < ActiveRecord::Base def all_pipelines(shas: all_commit_shas) return Ci::Pipeline.none unless source_project - @all_pipelines ||= source_project.pipelines + @all_pipelines ||= source_project.ci_pipelines .where(sha: shas, ref: source_branch) .where(merge_request: [nil, self]) .sort_by_merge_request_pipelines @@ -1220,7 +1220,7 @@ class MergeRequest < ActiveRecord::Base end def base_pipeline - @base_pipeline ||= project.pipelines + @base_pipeline ||= project.ci_pipelines .order(id: :desc) .find_by(sha: diff_base_sha) end diff --git a/app/models/project.rb b/app/models/project.rb index d8ce8038e92..0ab3ea53675 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -247,7 +247,17 @@ class Project < ActiveRecord::Base has_many :container_repositories, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent has_many :commit_statuses - has_many :pipelines, class_name: 'Ci::Pipeline', inverse_of: :project + # The relation :all_pipelines is intented to be used when we want to get the + # whole list of pipelines associated to the project + has_many :all_pipelines, class_name: 'Ci::Pipeline', inverse_of: :project + # The relation :ci_pipelines is intented to be used when we want to get only + # those pipeline which are directly related to CI. There are + # other pipelines, like webide ones, that we won't retrieve + # if we use this relation. + has_many :ci_pipelines, + -> { Feature.enabled?(:pipeline_ci_sources_only, default_enabled: true) ? ci_sources : all }, + class_name: 'Ci::Pipeline', + inverse_of: :project has_many :stages, class_name: 'Ci::Stage', inverse_of: :project # Ci::Build objects store data on the file system such as artifact files and @@ -621,7 +631,7 @@ class Project < ActiveRecord::Base # ref can't be HEAD, can only be branch/tag name or SHA def latest_successful_builds_for(ref = default_branch) - latest_pipeline = pipelines.latest_successful_for(ref) + latest_pipeline = ci_pipelines.latest_successful_for(ref) if latest_pipeline latest_pipeline.builds.latest.with_artifacts_archive @@ -1376,7 +1386,7 @@ class Project < ActiveRecord::Base return unless sha - pipelines.order(id: :desc).find_by(sha: sha, ref: ref) + ci_pipelines.order(id: :desc).find_by(sha: sha, ref: ref) end def latest_successful_pipeline_for_default_branch @@ -1385,12 +1395,12 @@ class Project < ActiveRecord::Base end @latest_successful_pipeline_for_default_branch = - pipelines.latest_successful_for(default_branch) + ci_pipelines.latest_successful_for(default_branch) end def latest_successful_pipeline_for(ref = nil) if ref && ref != default_branch - pipelines.latest_successful_for(ref) + ci_pipelines.latest_successful_for(ref) else latest_successful_pipeline_for_default_branch end diff --git a/app/models/project_services/pipelines_email_service.rb b/app/models/project_services/pipelines_email_service.rb index 6f39a5e6e83..d60a6a7efa3 100644 --- a/app/models/project_services/pipelines_email_service.rb +++ b/app/models/project_services/pipelines_email_service.rb @@ -38,11 +38,11 @@ class PipelinesEmailService < Service end def can_test? - project.pipelines.any? + project.ci_pipelines.any? end def test_data(project, user) - data = Gitlab::DataBuilder::Pipeline.build(project.pipelines.last) + data = Gitlab::DataBuilder::Pipeline.build(project.ci_pipelines.last) data[:user] = user.hook_attrs data end diff --git a/app/services/ci/create_pipeline_service.rb b/app/services/ci/create_pipeline_service.rb index c7ce7b7a5f8..19b5552887f 100644 --- a/app/services/ci/create_pipeline_service.rb +++ b/app/services/ci/create_pipeline_service.rb @@ -78,7 +78,7 @@ module Ci # rubocop: disable CodeReuse/ActiveRecord def auto_cancelable_pipelines - project.pipelines + project.ci_pipelines .where(ref: pipeline.ref) .where.not(id: pipeline.id) .where.not(sha: project.commit(pipeline.ref).try(:id)) diff --git a/app/services/projects/auto_devops/disable_service.rb b/app/services/projects/auto_devops/disable_service.rb index 1b578a3c5ce..6608b3da1a8 100644 --- a/app/services/projects/auto_devops/disable_service.rb +++ b/app/services/projects/auto_devops/disable_service.rb @@ -34,7 +34,7 @@ module Projects end def auto_devops_pipelines - @auto_devops_pipelines ||= project.pipelines.auto_devops_source + @auto_devops_pipelines ||= project.ci_pipelines.auto_devops_source end end end diff --git a/app/services/test_hooks/project_service.rb b/app/services/test_hooks/project_service.rb index 45e0e61e5c4..7e14ddcd017 100644 --- a/app/services/test_hooks/project_service.rb +++ b/app/services/test_hooks/project_service.rb @@ -49,7 +49,7 @@ module TestHooks end def pipeline_events_data - pipeline = project.pipelines.first + pipeline = project.ci_pipelines.first throw(:validation_error, 'Ensure the project has CI pipelines.') unless pipeline.present? Gitlab::DataBuilder::Pipeline.build(pipeline) diff --git a/db/fixtures/development/14_pipelines.rb b/db/fixtures/development/14_pipelines.rb index 5af77c49913..bdc0a2db7db 100644 --- a/db/fixtures/development/14_pipelines.rb +++ b/db/fixtures/development/14_pipelines.rb @@ -104,7 +104,7 @@ class Gitlab::Seeder::Pipelines def create_pipeline!(project, ref, commit) - project.pipelines.create!(sha: commit.id, ref: ref, source: :push) + project.ci_pipelines.create!(sha: commit.id, ref: ref, source: :push) end def build_create!(pipeline, opts = {}) diff --git a/lib/api/commit_statuses.rb b/lib/api/commit_statuses.rb index 1ba2c150cb4..62c966e06b4 100644 --- a/lib/api/commit_statuses.rb +++ b/lib/api/commit_statuses.rb @@ -29,7 +29,7 @@ module API not_found!('Commit') unless user_project.commit(params[:sha]) - pipelines = user_project.pipelines.where(sha: params[:sha]) + pipelines = user_project.ci_pipelines.where(sha: params[:sha]) statuses = ::CommitStatus.where(pipeline: pipelines) statuses = statuses.latest unless to_boolean(params[:all]) statuses = statuses.where(ref: params[:ref]) if params[:ref].present? @@ -75,7 +75,7 @@ module API pipeline = @project.pipeline_for(ref, commit.sha) unless pipeline - pipeline = @project.pipelines.create!( + pipeline = @project.ci_pipelines.create!( source: :external, sha: commit.sha, ref: ref, diff --git a/lib/api/jobs.rb b/lib/api/jobs.rb index c3f8a84d742..80a5cbd6b19 100644 --- a/lib/api/jobs.rb +++ b/lib/api/jobs.rb @@ -56,7 +56,7 @@ module API end # rubocop: disable CodeReuse/ActiveRecord get ':id/pipelines/:pipeline_id/jobs' do - pipeline = user_project.pipelines.find(params[:pipeline_id]) + pipeline = user_project.ci_pipelines.find(params[:pipeline_id]) builds = pipeline.builds builds = filter_builds(builds, params[:scope]) builds = builds.preload(:job_artifacts_archive, :job_artifacts, project: [:namespace]) diff --git a/lib/api/pipelines.rb b/lib/api/pipelines.rb index 62d07d945e2..7a7b23d2bbb 100644 --- a/lib/api/pipelines.rb +++ b/lib/api/pipelines.rb @@ -130,7 +130,7 @@ module API helpers do def pipeline - @pipeline ||= user_project.pipelines.find(params[:pipeline_id]) + @pipeline ||= user_project.ci_pipelines.find(params[:pipeline_id]) end end end diff --git a/lib/gitlab/badge/coverage/report.rb b/lib/gitlab/badge/coverage/report.rb index a7fcb6b0fca..7f7cc62c8ef 100644 --- a/lib/gitlab/badge/coverage/report.rb +++ b/lib/gitlab/badge/coverage/report.rb @@ -14,7 +14,7 @@ module Gitlab @ref = ref @job = job - @pipeline = @project.pipelines.latest_successful_for(@ref) + @pipeline = @project.ci_pipelines.latest_successful_for(@ref) end def entity diff --git a/lib/gitlab/badge/pipeline/status.rb b/lib/gitlab/badge/pipeline/status.rb index 37e61f07e5b..a403d839517 100644 --- a/lib/gitlab/badge/pipeline/status.rb +++ b/lib/gitlab/badge/pipeline/status.rb @@ -22,7 +22,7 @@ module Gitlab # rubocop: disable CodeReuse/ActiveRecord def status - @project.pipelines + @project.ci_pipelines .where(sha: @sha) .latest_status(@ref) || 'unknown' end diff --git a/lib/gitlab/ci/charts.rb b/lib/gitlab/ci/charts.rb index a4f01468e8e..7cabaadb122 100644 --- a/lib/gitlab/ci/charts.rb +++ b/lib/gitlab/ci/charts.rb @@ -54,7 +54,7 @@ module Gitlab # rubocop: disable CodeReuse/ActiveRecord def collect - query = project.pipelines + query = project.all_pipelines .where("? > #{::Ci::Pipeline.table_name}.created_at AND #{::Ci::Pipeline.table_name}.created_at > ?", @to, @from) # rubocop:disable GitlabSecurity/SqlInjection totals_count = grouped_count(query) @@ -115,7 +115,7 @@ module Gitlab class PipelineTime < Chart def collect - commits = project.pipelines.last(30) + commits = project.all_pipelines.last(30) commits.each do |commit| @labels << commit.short_sha diff --git a/lib/gitlab/import_export/import_export.yml b/lib/gitlab/import_export/import_export.yml index 4f3298812a8..93065879ec6 100644 --- a/lib/gitlab/import_export/import_export.yml +++ b/lib/gitlab/import_export/import_export.yml @@ -51,7 +51,7 @@ project_tree: - resource_label_events: - label: :priorities - - pipelines: + - ci_pipelines: - notes: - :author - events: diff --git a/lib/gitlab/import_export/project_tree_restorer.rb b/lib/gitlab/import_export/project_tree_restorer.rb index 8cd4efd91cc..a56ec65b9f1 100644 --- a/lib/gitlab/import_export/project_tree_restorer.rb +++ b/lib/gitlab/import_export/project_tree_restorer.rb @@ -26,6 +26,8 @@ module Gitlab @project_members = @tree_hash.delete('project_members') + RelationRenameService.rename(@tree_hash) + ActiveRecord::Base.uncached do ActiveRecord::Base.no_touching do create_relations @@ -214,7 +216,7 @@ module Gitlab end def nil_iid_pipeline?(relation_key, relation_item) - relation_key == 'pipelines' && relation_item['iid'].nil? + relation_key == 'ci_pipelines' && relation_item['iid'].nil? end end end diff --git a/lib/gitlab/import_export/project_tree_saver.rb b/lib/gitlab/import_export/project_tree_saver.rb index 29f2dc80813..2255635acdf 100644 --- a/lib/gitlab/import_export/project_tree_saver.rb +++ b/lib/gitlab/import_export/project_tree_saver.rb @@ -34,6 +34,8 @@ module Gitlab project_json['project_members'] += group_members_json + RelationRenameService.add_new_associations(project_json) + project_json.to_json end diff --git a/lib/gitlab/import_export/relation_factory.rb b/lib/gitlab/import_export/relation_factory.rb index 9cc781ddf8d..a4902e2104f 100644 --- a/lib/gitlab/import_export/relation_factory.rb +++ b/lib/gitlab/import_export/relation_factory.rb @@ -4,6 +4,7 @@ module Gitlab module ImportExport class RelationFactory OVERRIDES = { snippets: :project_snippets, + ci_pipelines: 'Ci::Pipeline', pipelines: 'Ci::Pipeline', stages: 'Ci::Stage', statuses: 'commit_status', diff --git a/lib/gitlab/import_export/relation_rename_service.rb b/lib/gitlab/import_export/relation_rename_service.rb new file mode 100644 index 00000000000..179bde5e21e --- /dev/null +++ b/lib/gitlab/import_export/relation_rename_service.rb @@ -0,0 +1,48 @@ +# frozen_string_literal: true + +# This class is intended to help with relation renames within Gitlab versions +# and allow compatibility between versions. +# If you have to change one relationship name that is imported/exported, +# you should add it to the RENAMES constant indicating the old name and the +# new one. +# The behavior of these renamed relationships should be transient and it should +# only last one release until you completely remove the renaming from the list. +# +# When importing, this class will check the project hash and: +# - if only the old relationship name is found, it will rename it with the new one +# - if only the new relationship name is found, it will do nothing +# - if it finds both, it will use the new relationship data +# +# When exporting, this class will duplicate the keys in the resulting file. +# This way, if we open the file in an old version of the exporter it will work +# and also it will with the newer versions. +module Gitlab + module ImportExport + class RelationRenameService + RENAMES = { + 'pipelines' => 'ci_pipelines' # Added in 11.6, remove in 11.7 + }.freeze + + def self.rename(tree_hash) + return unless tree_hash&.present? + + RENAMES.each do |old_name, new_name| + old_entry = tree_hash.delete(old_name) + + next if tree_hash[new_name] + next unless old_entry + + tree_hash[new_name] = old_entry + end + end + + def self.add_new_associations(tree_hash) + RENAMES.each do |old_name, new_name| + next if tree_hash.key?(old_name) + + tree_hash[old_name] = tree_hash[new_name] + end + end + end + end +end diff --git a/spec/features/projects/commit/builds_spec.rb b/spec/features/projects/commit/builds_spec.rb index bd254caddfb..caf69796d52 100644 --- a/spec/features/projects/commit/builds_spec.rb +++ b/spec/features/projects/commit/builds_spec.rb @@ -20,7 +20,7 @@ describe 'project commit pipelines', :js do visit pipelines_project_commit_path(project, project.commit.sha) page.within('.table-holder') do - expect(page).to have_content project.pipelines[0].id # pipeline ids + expect(page).to have_content project.ci_pipelines[0].id # pipeline ids end end end diff --git a/spec/features/projects/pages_spec.rb b/spec/features/projects/pages_spec.rb index 831f22a0e69..435fb229b69 100644 --- a/spec/features/projects/pages_spec.rb +++ b/spec/features/projects/pages_spec.rb @@ -300,7 +300,7 @@ describe 'Pages' do let(:pipeline) do commit_sha = project.commit('HEAD').sha - project.pipelines.create( + project.ci_pipelines.create( ref: 'HEAD', sha: commit_sha, source: :push, diff --git a/spec/lib/gitlab/import_export/all_models.yml b/spec/lib/gitlab/import_export/all_models.yml index fa01eb7901b..d9a4f1960ad 100644 --- a/spec/lib/gitlab/import_export/all_models.yml +++ b/spec/lib/gitlab/import_export/all_models.yml @@ -103,7 +103,7 @@ merge_request_diff_commits: - merge_request_diff merge_request_diff_files: - merge_request_diff -pipelines: +ci_pipelines: - project - user - stages @@ -265,7 +265,8 @@ project: - notification_settings - import_data - commit_statuses -- pipelines +- ci_pipelines +- all_pipelines - stages - builds - runner_projects diff --git a/spec/lib/gitlab/import_export/project_tree_restorer_spec.rb b/spec/lib/gitlab/import_export/project_tree_restorer_spec.rb index 7171e12a849..242c16c4bdc 100644 --- a/spec/lib/gitlab/import_export/project_tree_restorer_spec.rb +++ b/spec/lib/gitlab/import_export/project_tree_restorer_spec.rb @@ -197,9 +197,9 @@ describe Gitlab::ImportExport::ProjectTreeRestorer do end it 'has the correct number of pipelines and statuses' do - expect(@project.pipelines.size).to eq(5) + expect(@project.ci_pipelines.size).to eq(5) - @project.pipelines.zip([2, 2, 2, 2, 2]) + @project.ci_pipelines.zip([2, 2, 2, 2, 2]) .each do |(pipeline, expected_status_size)| expect(pipeline.statuses.size).to eq(expected_status_size) end diff --git a/spec/lib/gitlab/import_export/project_tree_saver_spec.rb b/spec/lib/gitlab/import_export/project_tree_saver_spec.rb index 5dc372263ad..46fdfba953b 100644 --- a/spec/lib/gitlab/import_export/project_tree_saver_spec.rb +++ b/spec/lib/gitlab/import_export/project_tree_saver_spec.rb @@ -119,16 +119,16 @@ describe Gitlab::ImportExport::ProjectTreeSaver do end it 'has pipeline stages' do - expect(saved_project_json.dig('pipelines', 0, 'stages')).not_to be_empty + expect(saved_project_json.dig('ci_pipelines', 0, 'stages')).not_to be_empty end it 'has pipeline statuses' do - expect(saved_project_json.dig('pipelines', 0, 'stages', 0, 'statuses')).not_to be_empty + expect(saved_project_json.dig('ci_pipelines', 0, 'stages', 0, 'statuses')).not_to be_empty end it 'has pipeline builds' do builds_count = saved_project_json - .dig('pipelines', 0, 'stages', 0, 'statuses') + .dig('ci_pipelines', 0, 'stages', 0, 'statuses') .count { |hash| hash['type'] == 'Ci::Build' } expect(builds_count).to eq(1) @@ -142,11 +142,11 @@ describe Gitlab::ImportExport::ProjectTreeSaver do end it 'has pipeline commits' do - expect(saved_project_json['pipelines']).not_to be_empty + expect(saved_project_json['ci_pipelines']).not_to be_empty end it 'has ci pipeline notes' do - expect(saved_project_json['pipelines'].first['notes']).not_to be_empty + expect(saved_project_json['ci_pipelines'].first['notes']).not_to be_empty end it 'has labels with no associations' do diff --git a/spec/lib/gitlab/import_export/relation_rename_service_spec.rb b/spec/lib/gitlab/import_export/relation_rename_service_spec.rb new file mode 100644 index 00000000000..a20a844a492 --- /dev/null +++ b/spec/lib/gitlab/import_export/relation_rename_service_spec.rb @@ -0,0 +1,111 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Gitlab::ImportExport::RelationRenameService do + let(:renames) do + { + 'example_relation1' => 'new_example_relation1', + 'example_relation2' => 'new_example_relation2' + } + end + + let(:user) { create(:admin) } + let(:group) { create(:group, :nested) } + let!(:project) { create(:project, :builds_disabled, :issues_disabled, name: 'project', path: 'project') } + let(:shared) { project.import_export_shared } + + before do + stub_const("#{described_class}::RENAMES", renames) + end + + context 'when importing' do + let(:project_tree_restorer) { Gitlab::ImportExport::ProjectTreeRestorer.new(user: user, shared: shared, project: project) } + let(:import_path) { 'spec/lib/gitlab/import_export' } + let(:file_content) { IO.read("#{import_path}/project.json") } + let!(:json_file) { ActiveSupport::JSON.decode(file_content) } + let(:tree_hash) { project_tree_restorer.instance_variable_get(:@tree_hash) } + + before do + allow(shared).to receive(:export_path).and_return(import_path) + allow(ActiveSupport::JSON).to receive(:decode).with(file_content).and_return(json_file) + end + + context 'when the file has only old relationship names' do + # Configuring the json as an old version exported file, with only + # the previous association with the old name + before do + renames.each do |old_name, _| + json_file[old_name.to_s] = [] + end + end + + it 'renames old relationships to the new name' do + expect(json_file.keys).to include(*renames.keys) + + project_tree_restorer.restore + + expect(json_file.keys).to include(*renames.values) + expect(json_file.keys).not_to include(*renames.keys) + end + end + + context 'when the file has both the old and new relationships' do + # Configuring the json as the new version exported file, with both + # the old association name and the new one + before do + renames.each do |old_name, new_name| + json_file[old_name.to_s] = [1] + json_file[new_name.to_s] = [2] + end + end + + it 'uses the new relationships and removes the old ones from the hash' do + expect(json_file.keys).to include(*renames.keys) + + project_tree_restorer.restore + + expect(json_file.keys).to include(*renames.values) + expect(json_file.values_at(*renames.values).flatten.uniq.first).to eq 2 + expect(json_file.keys).not_to include(*renames.keys) + end + end + + context 'when the file has only new relationship names' do + # Configuring the json as the future version exported file, with only + # the new association name + before do + renames.each do |_, new_name| + json_file[new_name.to_s] = [] + end + end + + it 'uses the new relationships' do + expect(json_file.keys).not_to include(*renames.keys) + + project_tree_restorer.restore + + expect(json_file.keys).to include(*renames.values) + end + end + end + + context 'when exporting' do + let(:project_tree_saver) { Gitlab::ImportExport::ProjectTreeSaver.new(project: project, current_user: user, shared: shared) } + let(:project_tree) { project_tree_saver.send(:project_json) } + + it 'adds old relationships to the exported file' do + project_tree.merge!(renames.values.map { |new_name| [new_name, []] }.to_h) + + allow(project_tree_saver).to receive(:save) do |arg| + project_tree_saver.send(:project_json_tree) + end + + result = project_tree_saver.save + + saved_data = ActiveSupport::JSON.decode(result) + + expect(saved_data.keys).to include(*(renames.keys + renames.values)) + end + end +end diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb index dcaea9d3f83..ba9540c84d4 100644 --- a/spec/models/ci/pipeline_spec.rb +++ b/spec/models/ci/pipeline_spec.rb @@ -33,8 +33,9 @@ describe Ci::Pipeline, :mailer do describe 'associations' do it 'has a bidirectional relationship with projects' do - expect(described_class.reflect_on_association(:project).has_inverse?).to eq(:pipelines) - expect(Project.reflect_on_association(:pipelines).has_inverse?).to eq(:project) + expect(described_class.reflect_on_association(:project).has_inverse?).to eq(:all_pipelines) + expect(Project.reflect_on_association(:all_pipelines).has_inverse?).to eq(:project) + expect(Project.reflect_on_association(:ci_pipelines).has_inverse?).to eq(:project) end end @@ -1186,7 +1187,7 @@ describe Ci::Pipeline, :mailer do create(:ci_build, :allowed_to_fail, :failed, pipeline: pipeline, name: 'rubocop') create(:ci_build, :allowed_to_fail, :failed, pipeline: pipeline2, name: 'rubocop') - pipelines = project.pipelines.to_a + pipelines = project.ci_pipelines.to_a pipelines.each(&:number_of_warnings) diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 54ad3c858d5..c0bc9eb2ef0 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -63,7 +63,7 @@ describe Project do it { is_expected.to have_one(:forked_from_project).through(:fork_network_member) } it { is_expected.to have_one(:auto_devops).class_name('ProjectAutoDevops') } it { is_expected.to have_many(:commit_statuses) } - it { is_expected.to have_many(:pipelines) } + it { is_expected.to have_many(:ci_pipelines) } it { is_expected.to have_many(:builds) } it { is_expected.to have_many(:build_trace_section_names)} it { is_expected.to have_many(:runner_projects) } @@ -142,6 +142,29 @@ describe Project do expect(subject.boards.size).to eq 1 end end + + describe 'ci_pipelines association' do + context 'when feature flag pipeline_ci_sources_only is enabled' do + it 'returns only pipelines from ci_sources' do + stub_feature_flags(pipeline_ci_sources_only: true) + + expect(Ci::Pipeline).to receive(:ci_sources).and_call_original + + subject.ci_pipelines + end + end + + context 'when feature flag pipeline_ci_sources_only is disabled' do + it 'returns all pipelines' do + stub_feature_flags(pipeline_ci_sources_only: false) + + expect(Ci::Pipeline).not_to receive(:ci_sources).and_call_original + expect(Ci::Pipeline).to receive(:all).and_call_original.at_least(:once) + + subject.ci_pipelines + end + end + end end describe 'modules' do @@ -3373,7 +3396,7 @@ describe Project do context 'with a ref that is not the default branch' do it 'returns the latest successful pipeline for the given ref' do - expect(project.pipelines).to receive(:latest_successful_for).with('foo') + expect(project.ci_pipelines).to receive(:latest_successful_for).with('foo') project.latest_successful_pipeline_for('foo') end @@ -3401,7 +3424,7 @@ describe Project do it 'memoizes and returns the latest successful pipeline for the default branch' do pipeline = double(:pipeline) - expect(project.pipelines).to receive(:latest_successful_for) + expect(project.ci_pipelines).to receive(:latest_successful_for) .with(project.default_branch) .and_return(pipeline) .once diff --git a/spec/requests/api/commit_statuses_spec.rb b/spec/requests/api/commit_statuses_spec.rb index cd43bec35df..a43304c9b83 100644 --- a/spec/requests/api/commit_statuses_spec.rb +++ b/spec/requests/api/commit_statuses_spec.rb @@ -16,8 +16,8 @@ describe API::CommitStatuses do let(:get_url) { "/projects/#{project.id}/repository/commits/#{sha}/statuses" } context 'ci commit exists' do - let!(:master) { project.pipelines.create(source: :push, sha: commit.id, ref: 'master', protected: false) } - let!(:develop) { project.pipelines.create(source: :push, sha: commit.id, ref: 'develop', protected: false) } + let!(:master) { project.ci_pipelines.create(source: :push, sha: commit.id, ref: 'master', protected: false) } + let!(:develop) { project.ci_pipelines.create(source: :push, sha: commit.id, ref: 'develop', protected: false) } context "reporter user" do let(:statuses_id) { json_response.map { |status| status['id'] } } diff --git a/spec/requests/api/commits_spec.rb b/spec/requests/api/commits_spec.rb index 329d069ef3d..9e599c2175f 100644 --- a/spec/requests/api/commits_spec.rb +++ b/spec/requests/api/commits_spec.rb @@ -818,7 +818,7 @@ describe API::Commits do end context 'when the ref has a pipeline' do - let!(:pipeline) { project.pipelines.create(source: :push, ref: 'master', sha: commit.sha, protected: false) } + let!(:pipeline) { project.ci_pipelines.create(source: :push, ref: 'master', sha: commit.sha, protected: false) } it 'includes a "created" status' do get api(route, current_user) diff --git a/spec/requests/api/pipelines_spec.rb b/spec/requests/api/pipelines_spec.rb index 638cc9767d4..2e4fa0f9e16 100644 --- a/spec/requests/api/pipelines_spec.rb +++ b/spec/requests/api/pipelines_spec.rb @@ -304,7 +304,7 @@ describe API::Pipelines do it 'creates and returns a new pipeline' do expect do post api("/projects/#{project.id}/pipeline", user), ref: project.default_branch - end.to change { project.pipelines.count }.by(1) + end.to change { project.ci_pipelines.count }.by(1) expect(response).to have_gitlab_http_status(201) expect(json_response).to be_a Hash @@ -317,8 +317,8 @@ describe API::Pipelines do it 'creates and returns a new pipeline using the given variables' do expect do post api("/projects/#{project.id}/pipeline", user), ref: project.default_branch, variables: variables - end.to change { project.pipelines.count }.by(1) - expect_variables(project.pipelines.last.variables, variables) + end.to change { project.ci_pipelines.count }.by(1) + expect_variables(project.ci_pipelines.last.variables, variables) expect(response).to have_gitlab_http_status(201) expect(json_response).to be_a Hash @@ -338,8 +338,8 @@ describe API::Pipelines do it 'creates and returns a new pipeline using the given variables' do expect do post api("/projects/#{project.id}/pipeline", user), ref: project.default_branch, variables: variables - end.to change { project.pipelines.count }.by(1) - expect_variables(project.pipelines.last.variables, variables) + end.to change { project.ci_pipelines.count }.by(1) + expect_variables(project.ci_pipelines.last.variables, variables) expect(response).to have_gitlab_http_status(201) expect(json_response).to be_a Hash @@ -353,7 +353,7 @@ describe API::Pipelines do it "doesn't create a job" do expect do post api("/projects/#{project.id}/pipeline", user), ref: project.default_branch - end.not_to change { project.pipelines.count } + end.not_to change { project.ci_pipelines.count } expect(response).to have_gitlab_http_status(400) end diff --git a/spec/requests/api/triggers_spec.rb b/spec/requests/api/triggers_spec.rb index 0ae6796d1e4..658df6945d2 100644 --- a/spec/requests/api/triggers_spec.rb +++ b/spec/requests/api/triggers_spec.rb @@ -39,7 +39,7 @@ describe API::Triggers do end context 'Have a commit' do - let(:pipeline) { project.pipelines.last } + let(:pipeline) { project.ci_pipelines.last } it 'creates pipeline' do post api("/projects/#{project.id}/trigger/pipeline"), options.merge(ref: 'master') diff --git a/spec/services/ci/create_pipeline_service_spec.rb b/spec/services/ci/create_pipeline_service_spec.rb index 7bc990f05c2..ccc6b0ef1c7 100644 --- a/spec/services/ci/create_pipeline_service_spec.rb +++ b/spec/services/ci/create_pipeline_service_spec.rb @@ -44,7 +44,7 @@ describe Ci::CreatePipelineService do expect(pipeline).to be_valid expect(pipeline).to be_persisted expect(pipeline).to be_push - expect(pipeline).to eq(project.pipelines.last) + expect(pipeline).to eq(project.ci_pipelines.last) expect(pipeline).to have_attributes(user: user) expect(pipeline).to have_attributes(status: 'pending') expect(pipeline.repository_source?).to be true @@ -731,8 +731,8 @@ describe Ci::CreatePipelineService do it 'creates a merge request pipeline in the forked project' do expect(pipeline).to be_persisted - expect(project.pipelines).to eq([pipeline]) - expect(target_project.pipelines).to be_empty + expect(project.ci_pipelines).to eq([pipeline]) + expect(target_project.ci_pipelines).to be_empty end end diff --git a/spec/workers/pipeline_schedule_worker_spec.rb b/spec/workers/pipeline_schedule_worker_spec.rb index bebcbe01009..ff408427926 100644 --- a/spec/workers/pipeline_schedule_worker_spec.rb +++ b/spec/workers/pipeline_schedule_worker_spec.rb @@ -25,12 +25,12 @@ describe PipelineScheduleWorker do context 'when there is a scheduled pipeline within next_run_at' do shared_examples 'successful scheduling' do it 'creates a new pipeline' do - expect { subject }.to change { project.pipelines.count }.by(1) + expect { subject }.to change { project.ci_pipelines.count }.by(1) expect(Ci::Pipeline.last).to be_schedule pipeline_schedule.reload expect(pipeline_schedule.next_run_at).to be > Time.now - expect(pipeline_schedule).to eq(project.pipelines.last.pipeline_schedule) + expect(pipeline_schedule).to eq(project.ci_pipelines.last.pipeline_schedule) expect(pipeline_schedule).to be_active end end @@ -54,7 +54,7 @@ describe PipelineScheduleWorker do end it 'does not creates a new pipeline' do - expect { subject }.not_to change { project.pipelines.count } + expect { subject }.not_to change { project.ci_pipelines.count } end end @@ -64,7 +64,7 @@ describe PipelineScheduleWorker do end it 'creates a failed pipeline with the reason' do - expect { subject }.to change { project.pipelines.count }.by(1) + expect { subject }.to change { project.ci_pipelines.count }.by(1) expect(Ci::Pipeline.last).to be_config_error expect(Ci::Pipeline.last.yaml_errors).not_to be_nil end @@ -105,7 +105,7 @@ describe PipelineScheduleWorker do end it 'does not create a pipeline' do - expect { subject }.not_to change { project.pipelines.count } + expect { subject }.not_to change { project.ci_pipelines.count } end it 'does not raise an exception' do @@ -135,7 +135,7 @@ describe PipelineScheduleWorker do end it 'does not create a pipeline' do - expect { subject }.not_to change { project.pipelines.count } + expect { subject }.not_to change { project.ci_pipelines.count } end it 'does not raise an exception' do |