diff options
author | Douwe Maan <douwe@gitlab.com> | 2018-01-11 16:06:01 +0000 |
---|---|---|
committer | Douwe Maan <douwe@gitlab.com> | 2018-01-11 16:06:01 +0000 |
commit | 7acabf7c5908940665e1a8c4e3d09d423385657a (patch) | |
tree | fad4bd291a00b986a3e79ad3c47f6ed5b870ed01 | |
parent | 0668f54b0a77f8dc6b7891ebd3755965bb6136ff (diff) | |
parent | e5c49b57c35e5fe923b0fe29e1863cc29d6cf619 (diff) | |
download | gitlab-ce-7acabf7c5908940665e1a8c4e3d09d423385657a.tar.gz |
Merge branch 'remove-soft-removals' into 'master'
Remove soft removals related code
Closes #37447
See merge request gitlab-org/gitlab-ce!15789
39 files changed, 337 insertions, 105 deletions
@@ -385,9 +385,6 @@ gem 'ruby-prof', '~> 0.16.2' # OAuth gem 'oauth2', '~> 1.4' -# Soft deletion -gem 'paranoia', '~> 2.3.1' - # Health check gem 'health_check', '~> 2.6.0' diff --git a/Gemfile.lock b/Gemfile.lock index 40c4f73b8a6..cb6b0ebb3bc 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -580,8 +580,6 @@ GEM orm_adapter (0.5.0) os (0.9.6) parallel (1.12.0) - paranoia (2.3.1) - activerecord (>= 4.0, < 5.2) parser (2.4.0.2) ast (~> 2.3) parslet (1.5.0) @@ -1117,7 +1115,6 @@ DEPENDENCIES omniauth-twitter (~> 1.2.0) omniauth_crowd (~> 2.2.0) org-ruby (~> 0.9.12) - paranoia (~> 2.3.1) peek (~> 1.0.1) peek-gc (~> 0.0.2) peek-host (~> 1.0.0) diff --git a/app/models/ci/pipeline_schedule.rb b/app/models/ci/pipeline_schedule.rb index 10ead6b6d3b..b6abc3d7681 100644 --- a/app/models/ci/pipeline_schedule.rb +++ b/app/models/ci/pipeline_schedule.rb @@ -2,8 +2,9 @@ module Ci class PipelineSchedule < ActiveRecord::Base extend Gitlab::Ci::Model include Importable + include IgnorableColumn - acts_as_paranoid + ignore_column :deleted_at belongs_to :project belongs_to :owner, class_name: 'User' diff --git a/app/models/ci/trigger.rb b/app/models/ci/trigger.rb index b5290bcaf53..aa065e33739 100644 --- a/app/models/ci/trigger.rb +++ b/app/models/ci/trigger.rb @@ -1,8 +1,9 @@ module Ci class Trigger < ActiveRecord::Base extend Gitlab::Ci::Model + include IgnorableColumn - acts_as_paranoid + ignore_column :deleted_at belongs_to :project belongs_to :owner, class_name: "User" diff --git a/app/models/concerns/internal_id.rb b/app/models/concerns/internal_id.rb index a3d0ac8d862..01079fb8bd6 100644 --- a/app/models/concerns/internal_id.rb +++ b/app/models/concerns/internal_id.rb @@ -10,7 +10,6 @@ module InternalId if iid.blank? parent = project || group records = parent.public_send(self.class.name.tableize) # rubocop:disable GitlabSecurity/PublicSend - records = records.with_deleted if self.paranoid? max_iid = records.maximum(:iid) self.iid = max_iid.to_i + 1 diff --git a/app/models/issue.rb b/app/models/issue.rb index ad4a3c737ff..93628b456f2 100644 --- a/app/models/issue.rb +++ b/app/models/issue.rb @@ -12,7 +12,7 @@ class Issue < ActiveRecord::Base include ThrottledTouch include IgnorableColumn - ignore_column :assignee_id, :branch_name + ignore_column :assignee_id, :branch_name, :deleted_at DueDateStruct = Struct.new(:title, :name).freeze NoDueDate = DueDateStruct.new('No Due Date', '0').freeze @@ -78,8 +78,6 @@ class Issue < ActiveRecord::Base end end - acts_as_paranoid - class << self alias_method :in_parents, :in_projects end diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index ef58816937c..8fdeddf1ed1 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -11,7 +11,8 @@ class MergeRequest < ActiveRecord::Base include Gitlab::Utils::StrongMemoize ignore_column :locked_at, - :ref_fetched + :ref_fetched, + :deleted_at belongs_to :target_project, class_name: "Project" belongs_to :source_project, class_name: "Project" @@ -150,8 +151,6 @@ class MergeRequest < ActiveRecord::Base after_save :keep_around_commit - acts_as_paranoid - def self.reference_prefix '!' end diff --git a/app/models/namespace.rb b/app/models/namespace.rb index bdcc9159d26..37a7417cafc 100644 --- a/app/models/namespace.rb +++ b/app/models/namespace.rb @@ -1,6 +1,4 @@ class Namespace < ActiveRecord::Base - acts_as_paranoid without_default_scope: true - include CacheMarkdownField include Sortable include Gitlab::ShellAdapter @@ -10,6 +8,9 @@ class Namespace < ActiveRecord::Base include AfterCommitQueue include Storage::LegacyNamespace include Gitlab::SQL::Pattern + include IgnorableColumn + + ignore_column :deleted_at # Prevent users from creating unreasonably deep level of nesting. # The number 20 was taken based on maximum nesting level of @@ -221,12 +222,6 @@ class Namespace < ActiveRecord::Base has_parent? end - def soft_delete_without_removing_associations - # We can't use paranoia's `#destroy` since this will hard-delete projects. - # Project uses `pending_delete` instead of the acts_as_paranoia gem. - self.deleted_at = Time.now - end - private def refresh_access_of_projects_invited_groups diff --git a/app/serializers/issue_entity.rb b/app/serializers/issue_entity.rb index 0bdd4d7a272..b5e2334b6e3 100644 --- a/app/serializers/issue_entity.rb +++ b/app/serializers/issue_entity.rb @@ -6,7 +6,6 @@ class IssueEntity < IssuableEntity expose :updated_by_id expose :created_at expose :updated_at - expose :deleted_at expose :milestone, using: API::Entities::Milestone expose :labels, using: LabelEntity expose :lock_version diff --git a/app/services/groups/destroy_service.rb b/app/services/groups/destroy_service.rb index e3f9d9ee95d..58e88688dfa 100644 --- a/app/services/groups/destroy_service.rb +++ b/app/services/groups/destroy_service.rb @@ -1,7 +1,6 @@ module Groups class DestroyService < Groups::BaseService def async_execute - group.soft_delete_without_removing_associations job_id = GroupDestroyWorker.perform_async(group.id, current_user.id) Rails.logger.info("User #{current_user.id} scheduled a deletion of group ID #{group.id} with job ID #{job_id}") end @@ -23,7 +22,7 @@ module Groups group.chat_team&.remove_mattermost_team(current_user) - group.really_destroy! + group.destroy end end end diff --git a/app/services/users/destroy_service.rb b/app/services/users/destroy_service.rb index 00db8a2c434..b71002433d6 100644 --- a/app/services/users/destroy_service.rb +++ b/app/services/users/destroy_service.rb @@ -53,7 +53,7 @@ module Users # Destroy the namespace after destroying the user since certain methods may depend on the namespace existing user_data = user.destroy - namespace.really_destroy! + namespace.destroy user_data end diff --git a/app/workers/group_destroy_worker.rb b/app/workers/group_destroy_worker.rb index f577b310b20..509bd09dc2e 100644 --- a/app/workers/group_destroy_worker.rb +++ b/app/workers/group_destroy_worker.rb @@ -4,7 +4,7 @@ class GroupDestroyWorker def perform(group_id, user_id) begin - group = Group.with_deleted.find(group_id) + group = Group.find(group_id) rescue ActiveRecord::RecordNotFound return end diff --git a/changelogs/unreleased/remove-soft-removals.yml b/changelogs/unreleased/remove-soft-removals.yml new file mode 100644 index 00000000000..aa53d33e502 --- /dev/null +++ b/changelogs/unreleased/remove-soft-removals.yml @@ -0,0 +1,5 @@ +--- +title: Remove soft removals related code +merge_request: 15789 +author: +type: changed diff --git a/db/post_migrate/20171207150343_remove_soft_removed_objects.rb b/db/post_migrate/20171207150343_remove_soft_removed_objects.rb new file mode 100644 index 00000000000..3e2dedfdd6a --- /dev/null +++ b/db/post_migrate/20171207150343_remove_soft_removed_objects.rb @@ -0,0 +1,208 @@ +# See http://doc.gitlab.com/ce/development/migration_style_guide.html +# for more information on how to write migrations for GitLab. + +class RemoveSoftRemovedObjects < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + # Set this constant to true if this migration requires downtime. + DOWNTIME = false + + disable_ddl_transaction! + + module SoftRemoved + extend ActiveSupport::Concern + + included do + scope :soft_removed, -> { where('deleted_at IS NOT NULL') } + end + end + + class User < ActiveRecord::Base + self.table_name = 'users' + + include EachBatch + end + + class Issue < ActiveRecord::Base + self.table_name = 'issues' + + include EachBatch + include SoftRemoved + end + + class MergeRequest < ActiveRecord::Base + self.table_name = 'merge_requests' + + include EachBatch + include SoftRemoved + end + + class Namespace < ActiveRecord::Base + self.table_name = 'namespaces' + + include EachBatch + include SoftRemoved + + scope :soft_removed_personal, -> { soft_removed.where(type: nil) } + scope :soft_removed_group, -> { soft_removed.where(type: 'Group') } + end + + class Route < ActiveRecord::Base + self.table_name = 'routes' + + include EachBatch + include SoftRemoved + end + + class Project < ActiveRecord::Base + self.table_name = 'projects' + + include EachBatch + include SoftRemoved + end + + class CiPipelineSchedule < ActiveRecord::Base + self.table_name = 'ci_pipeline_schedules' + + include EachBatch + include SoftRemoved + end + + class CiTrigger < ActiveRecord::Base + self.table_name = 'ci_triggers' + + include EachBatch + include SoftRemoved + end + + MODELS = [Issue, MergeRequest, CiPipelineSchedule, CiTrigger].freeze + + def up + disable_statement_timeout + + remove_personal_routes + remove_personal_namespaces + remove_group_namespaces + remove_simple_soft_removed_rows + end + + def down + # The data removed by this migration can't be restored in an automated way. + end + + def remove_simple_soft_removed_rows + create_temporary_indexes + + MODELS.each do |model| + say_with_time("Removing soft removed rows from #{model.table_name}") do + model.soft_removed.each_batch do |batch, index| + batch.delete_all + end + end + end + ensure + remove_temporary_indexes + end + + def create_temporary_indexes + MODELS.each do |model| + index_name = temporary_index_name_for(model) + + # Without this index the removal process can take a very long time. For + # example, getting the next ID of a batch for the `issues` table in + # staging would take between 15 and 20 seconds. + next if temporary_index_exists?(model) + + say_with_time("Creating temporary index #{index_name}") do + add_concurrent_index( + model.table_name, + [:deleted_at, :id], + name: index_name, + where: 'deleted_at IS NOT NULL' + ) + end + end + end + + def remove_temporary_indexes + MODELS.each do |model| + index_name = temporary_index_name_for(model) + + next unless temporary_index_exists?(model) + + say_with_time("Removing temporary index #{index_name}") do + remove_concurrent_index_by_name(model.table_name, index_name) + end + end + end + + def temporary_index_name_for(model) + "index_on_#{model.table_name}_tmp" + end + + def temporary_index_exists?(model) + index_name = temporary_index_name_for(model) + + index_exists?(model.table_name, [:deleted_at, :id], name: index_name) + end + + def remove_personal_namespaces + # Some personal namespaces are left behind in case of GitLab.com. In these + # cases the associated data such as the projects and users has already been + # removed. + Namespace.soft_removed_personal.each_batch do |batch| + batch.delete_all + end + end + + def remove_group_namespaces + admin_id = id_for_admin_user + + unless admin_id + say 'Not scheduling soft removed groups for removal as no admin user ' \ + 'could be found. You will need to remove any such groups manually.' + + return + end + + # Left over groups can't be easily removed because we may also need to + # remove memberships, repositories, and other associated data. As a result + # we'll just schedule a Sidekiq job to remove these. + # + # As of January 5th, 2018 there are 36 groups that will be removed using + # this code. + Namespace.select(:id).soft_removed_group.each_batch(of: 10) do |batch, index| + batch.each do |ns| + schedule_group_removal(index * 5.minutes, ns.id, admin_id) + end + end + end + + def schedule_group_removal(delay, group_id, user_id) + if migrate_inline? + GroupDestroyWorker.new.perform(group_id, user_id) + else + GroupDestroyWorker.perform_in(delay, group_id, user_id) + end + end + + def remove_personal_routes + namespaces = Namespace.select(1) + .soft_removed + .where('namespaces.type IS NULL') + .where('routes.source_type = ?', 'Namespace') + .where('routes.source_id = namespaces.id') + + Route.where('EXISTS (?)', namespaces).each_batch do |batch| + batch.delete_all + end + end + + def id_for_admin_user + User.where(admin: true).limit(1).pluck(:id).first + end + + def migrate_inline? + Rails.env.test? || Rails.env.development? + end +end diff --git a/db/post_migrate/20171207150344_remove_deleted_at_columns.rb b/db/post_migrate/20171207150344_remove_deleted_at_columns.rb new file mode 100644 index 00000000000..154d7a1b926 --- /dev/null +++ b/db/post_migrate/20171207150344_remove_deleted_at_columns.rb @@ -0,0 +1,31 @@ +# See http://doc.gitlab.com/ce/development/migration_style_guide.html +# for more information on how to write migrations for GitLab. + +class RemoveDeletedAtColumns < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + disable_ddl_transaction! + + TABLES = %i[issues merge_requests namespaces ci_pipeline_schedules ci_triggers].freeze + COLUMN = :deleted_at + + def up + TABLES.each do |table| + remove_column(table, COLUMN) if column_exists?(table, COLUMN) + end + end + + def down + TABLES.each do |table| + unless column_exists?(table, COLUMN) + add_column(table, COLUMN, :datetime_with_timezone) + end + + unless index_exists?(table, COLUMN) + add_concurrent_index(table, COLUMN) + end + end + end +end diff --git a/db/schema.rb b/db/schema.rb index 07620349057..8a6db61250b 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -357,7 +357,6 @@ ActiveRecord::Schema.define(version: 20180105212544) do t.integer "project_id" t.integer "owner_id" t.boolean "active", default: true - t.datetime "deleted_at" t.datetime "created_at" t.datetime "updated_at" end @@ -467,7 +466,6 @@ ActiveRecord::Schema.define(version: 20180105212544) do create_table "ci_triggers", force: :cascade do |t| t.string "token" - t.datetime "deleted_at" t.datetime "created_at" t.datetime "updated_at" t.integer "project_id" @@ -861,7 +859,6 @@ ActiveRecord::Schema.define(version: 20180105212544) do t.integer "iid" t.integer "updated_by_id" t.boolean "confidential", default: false, null: false - t.datetime "deleted_at" t.date "due_date" t.integer "moved_to_id" t.integer "lock_version" @@ -878,7 +875,6 @@ ActiveRecord::Schema.define(version: 20180105212544) do add_index "issues", ["author_id"], name: "index_issues_on_author_id", using: :btree add_index "issues", ["confidential"], name: "index_issues_on_confidential", using: :btree - add_index "issues", ["deleted_at"], name: "index_issues_on_deleted_at", using: :btree add_index "issues", ["description"], name: "index_issues_on_description_trigram", using: :gin, opclasses: {"description"=>"gin_trgm_ops"} add_index "issues", ["milestone_id"], name: "index_issues_on_milestone_id", using: :btree add_index "issues", ["moved_to_id"], name: "index_issues_on_moved_to_id", where: "(moved_to_id IS NOT NULL)", using: :btree @@ -1088,7 +1084,6 @@ ActiveRecord::Schema.define(version: 20180105212544) do t.boolean "merge_when_pipeline_succeeds", default: false, null: false t.integer "merge_user_id" t.string "merge_commit_sha" - t.datetime "deleted_at" t.string "in_progress_merge_commit_sha" t.integer "lock_version" t.text "title_html" @@ -1107,7 +1102,6 @@ ActiveRecord::Schema.define(version: 20180105212544) do add_index "merge_requests", ["assignee_id"], name: "index_merge_requests_on_assignee_id", using: :btree add_index "merge_requests", ["author_id"], name: "index_merge_requests_on_author_id", using: :btree add_index "merge_requests", ["created_at"], name: "index_merge_requests_on_created_at", using: :btree - add_index "merge_requests", ["deleted_at"], name: "index_merge_requests_on_deleted_at", using: :btree add_index "merge_requests", ["description"], name: "index_merge_requests_on_description_trigram", using: :gin, opclasses: {"description"=>"gin_trgm_ops"} add_index "merge_requests", ["head_pipeline_id"], name: "index_merge_requests_on_head_pipeline_id", using: :btree add_index "merge_requests", ["latest_merge_request_diff_id"], name: "index_merge_requests_on_latest_merge_request_diff_id", using: :btree @@ -1167,7 +1161,6 @@ ActiveRecord::Schema.define(version: 20180105212544) do t.boolean "share_with_group_lock", default: false t.integer "visibility_level", default: 20, null: false t.boolean "request_access_enabled", default: false, null: false - t.datetime "deleted_at" t.text "description_html" t.boolean "lfs_enabled" t.integer "parent_id" @@ -1177,7 +1170,6 @@ ActiveRecord::Schema.define(version: 20180105212544) do end add_index "namespaces", ["created_at"], name: "index_namespaces_on_created_at", using: :btree - add_index "namespaces", ["deleted_at"], name: "index_namespaces_on_deleted_at", using: :btree add_index "namespaces", ["name", "parent_id"], name: "index_namespaces_on_name_and_parent_id", unique: true, using: :btree add_index "namespaces", ["name"], name: "index_namespaces_on_name_trigram", using: :gin, opclasses: {"name"=>"gin_trgm_ops"} add_index "namespaces", ["owner_id"], name: "index_namespaces_on_owner_id", using: :btree diff --git a/doc/api/pipeline_triggers.md b/doc/api/pipeline_triggers.md index 9030ae32d17..e881e61d4ef 100644 --- a/doc/api/pipeline_triggers.md +++ b/doc/api/pipeline_triggers.md @@ -24,7 +24,6 @@ curl --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" "https://gitlab.example.com/ "id": 10, "description": "my trigger", "created_at": "2016-01-07T09:53:58.235Z", - "deleted_at": null, "last_used": null, "token": "6d056f63e50fe6f8c5f8f4aa10edb7", "updated_at": "2016-01-07T09:53:58.235Z", @@ -55,7 +54,6 @@ curl --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" "https://gitlab.example.com/ "id": 10, "description": "my trigger", "created_at": "2016-01-07T09:53:58.235Z", - "deleted_at": null, "last_used": null, "token": "6d056f63e50fe6f8c5f8f4aa10edb7", "updated_at": "2016-01-07T09:53:58.235Z", @@ -85,7 +83,6 @@ curl --request POST --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" --form descri "id": 10, "description": "my trigger", "created_at": "2016-01-07T09:53:58.235Z", - "deleted_at": null, "last_used": null, "token": "6d056f63e50fe6f8c5f8f4aa10edb7", "updated_at": "2016-01-07T09:53:58.235Z", @@ -116,7 +113,6 @@ curl --request PUT --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" --form descrip "id": 10, "description": "my trigger", "created_at": "2016-01-07T09:53:58.235Z", - "deleted_at": null, "last_used": null, "token": "6d056f63e50fe6f8c5f8f4aa10edb7", "updated_at": "2016-01-07T09:53:58.235Z", @@ -146,7 +142,6 @@ curl --request POST --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" "https://gitl "id": 10, "description": "my trigger", "created_at": "2016-01-07T09:53:58.235Z", - "deleted_at": null, "last_used": null, "token": "6d056f63e50fe6f8c5f8f4aa10edb7", "updated_at": "2016-01-07T09:53:58.235Z", diff --git a/lib/api/entities.rb b/lib/api/entities.rb index f574858be02..c4ef2c74658 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -918,7 +918,7 @@ module API class Trigger < Grape::Entity expose :id expose :token, :description - expose :created_at, :updated_at, :deleted_at, :last_used + expose :created_at, :updated_at, :last_used expose :owner, using: Entities::UserBasic end diff --git a/lib/api/v3/entities.rb b/lib/api/v3/entities.rb index c17b6f45ed8..64758dae7d3 100644 --- a/lib/api/v3/entities.rb +++ b/lib/api/v3/entities.rb @@ -207,7 +207,7 @@ module API end class Trigger < Grape::Entity - expose :token, :created_at, :updated_at, :deleted_at, :last_used + expose :token, :created_at, :updated_at, :last_used expose :owner, using: ::API::Entities::UserBasic end diff --git a/lib/gitlab/cycle_analytics/base_query.rb b/lib/gitlab/cycle_analytics/base_query.rb index dcbdf9a64b0..8b3bc3e440d 100644 --- a/lib/gitlab/cycle_analytics/base_query.rb +++ b/lib/gitlab/cycle_analytics/base_query.rb @@ -15,7 +15,6 @@ module Gitlab query = mr_closing_issues_table.join(issue_table).on(issue_table[:id].eq(mr_closing_issues_table[:issue_id])) .join(issue_metrics_table).on(issue_table[:id].eq(issue_metrics_table[:issue_id])) .where(issue_table[:project_id].eq(@project.id)) # rubocop:disable Gitlab/ModuleWithInstanceVariables - .where(issue_table[:deleted_at].eq(nil)) .where(issue_table[:created_at].gteq(@options[:from])) # rubocop:disable Gitlab/ModuleWithInstanceVariables # Load merge_requests diff --git a/lib/gitlab/hook_data/issue_builder.rb b/lib/gitlab/hook_data/issue_builder.rb index e29dd0d5b0e..f9b1a3caf5e 100644 --- a/lib/gitlab/hook_data/issue_builder.rb +++ b/lib/gitlab/hook_data/issue_builder.rb @@ -7,7 +7,6 @@ module Gitlab closed_at confidential created_at - deleted_at description due_date id diff --git a/lib/gitlab/hook_data/merge_request_builder.rb b/lib/gitlab/hook_data/merge_request_builder.rb index ae9b68eb648..aff786864f2 100644 --- a/lib/gitlab/hook_data/merge_request_builder.rb +++ b/lib/gitlab/hook_data/merge_request_builder.rb @@ -5,7 +5,6 @@ module Gitlab assignee_id author_id created_at - deleted_at description head_pipeline_id id diff --git a/spec/fixtures/api/schemas/entities/issue.json b/spec/fixtures/api/schemas/entities/issue.json index 3d3329a3406..38467b4ca20 100644 --- a/spec/fixtures/api/schemas/entities/issue.json +++ b/spec/fixtures/api/schemas/entities/issue.json @@ -28,7 +28,6 @@ "confidential": { "type": "boolean" }, "discussion_locked": { "type": ["boolean", "null"] }, "updated_by_id": { "type": ["string", "null"] }, - "deleted_at": { "type": ["string", "null"] }, "time_estimate": { "type": "integer" }, "total_time_spent": { "type": "integer" }, "human_time_estimate": { "type": ["integer", "null"] }, diff --git a/spec/fixtures/api/schemas/entities/merge_request_widget.json b/spec/fixtures/api/schemas/entities/merge_request_widget.json index 7f662098216..05461787f06 100644 --- a/spec/fixtures/api/schemas/entities/merge_request_widget.json +++ b/spec/fixtures/api/schemas/entities/merge_request_widget.json @@ -13,7 +13,6 @@ "updated_by_id": { "type": ["string", "null"] }, "created_at": { "type": "string" }, "updated_at": { "type": "string" }, - "deleted_at": { "type": ["string", "null"] }, "time_estimate": { "type": "integer" }, "total_time_spent": { "type": "integer" }, "human_time_estimate": { "type": ["integer", "null"] }, diff --git a/spec/javascripts/notes/mock_data.js b/spec/javascripts/notes/mock_data.js index 6b608adff15..b020a1020df 100644 --- a/spec/javascripts/notes/mock_data.js +++ b/spec/javascripts/notes/mock_data.js @@ -29,7 +29,6 @@ export const noteableDataMock = { can_create_note: true, can_update: true, }, - deleted_at: null, description: '', due_date: null, human_time_estimate: null, @@ -283,7 +282,6 @@ export const loggedOutnoteableData = { "updated_by_id": 1, "created_at": "2017-02-07T10:11:18.395Z", "updated_at": "2017-08-08T10:22:51.564Z", - "deleted_at": null, "time_estimate": 0, "total_time_spent": 0, "human_time_estimate": null, diff --git a/spec/javascripts/sidebar/mock_data.js b/spec/javascripts/sidebar/mock_data.js index 3b094d20838..7bc591d2d47 100644 --- a/spec/javascripts/sidebar/mock_data.js +++ b/spec/javascripts/sidebar/mock_data.js @@ -15,7 +15,6 @@ const RESPONSE_MAP = { updated_by_id: 1, created_at: '2017-02-02T21: 49: 49.664Z', updated_at: '2017-05-03T22: 26: 03.760Z', - deleted_at: null, time_estimate: 0, total_time_spent: 0, human_time_estimate: null, @@ -153,7 +152,6 @@ const RESPONSE_MAP = { updated_by_id: 1, created_at: '2017-06-27T19:54:42.437Z', updated_at: '2017-08-18T03:39:49.222Z', - deleted_at: null, time_estimate: 0, total_time_spent: 0, human_time_estimate: null, diff --git a/spec/javascripts/vue_mr_widget/mock_data.js b/spec/javascripts/vue_mr_widget/mock_data.js index ca29c9fee32..ae494267659 100644 --- a/spec/javascripts/vue_mr_widget/mock_data.js +++ b/spec/javascripts/vue_mr_widget/mock_data.js @@ -14,7 +14,6 @@ export default { "updated_by_id": null, "created_at": "2017-04-07T12:27:26.718Z", "updated_at": "2017-04-07T15:39:25.852Z", - "deleted_at": null, "time_estimate": 0, "total_time_spent": 0, "human_time_estimate": null, diff --git a/spec/lib/gitlab/hook_data/issue_builder_spec.rb b/spec/lib/gitlab/hook_data/issue_builder_spec.rb index aeacd577d18..506b2c0be20 100644 --- a/spec/lib/gitlab/hook_data/issue_builder_spec.rb +++ b/spec/lib/gitlab/hook_data/issue_builder_spec.rb @@ -14,7 +14,6 @@ describe Gitlab::HookData::IssueBuilder do closed_at confidential created_at - deleted_at description due_date id diff --git a/spec/lib/gitlab/hook_data/merge_request_builder_spec.rb b/spec/lib/gitlab/hook_data/merge_request_builder_spec.rb index 78475403f9e..b61614e4790 100644 --- a/spec/lib/gitlab/hook_data/merge_request_builder_spec.rb +++ b/spec/lib/gitlab/hook_data/merge_request_builder_spec.rb @@ -12,7 +12,6 @@ describe Gitlab::HookData::MergeRequestBuilder do assignee_id author_id created_at - deleted_at description head_pipeline_id id diff --git a/spec/lib/gitlab/import_export/project.group.json b/spec/lib/gitlab/import_export/project.group.json index 82a1fbd2fc5..1a561e81e4a 100644 --- a/spec/lib/gitlab/import_export/project.group.json +++ b/spec/lib/gitlab/import_export/project.group.json @@ -54,7 +54,6 @@ "iid": 1, "updated_by_id": 1, "confidential": false, - "deleted_at": null, "due_date": null, "moved_to_id": null, "lock_version": null, @@ -134,7 +133,6 @@ "iid": 2, "updated_by_id": 1, "confidential": false, - "deleted_at": null, "due_date": null, "moved_to_id": null, "lock_version": null, diff --git a/spec/lib/gitlab/import_export/project.json b/spec/lib/gitlab/import_export/project.json index 7580b62cfc0..4cf33778d15 100644 --- a/spec/lib/gitlab/import_export/project.json +++ b/spec/lib/gitlab/import_export/project.json @@ -56,7 +56,6 @@ "iid": 10, "updated_by_id": null, "confidential": false, - "deleted_at": null, "due_date": null, "moved_to_id": null, "test_ee_field": "test", @@ -350,7 +349,6 @@ "iid": 9, "updated_by_id": null, "confidential": false, - "deleted_at": null, "due_date": null, "moved_to_id": null, "milestone": { @@ -586,7 +584,6 @@ "iid": 8, "updated_by_id": null, "confidential": false, - "deleted_at": null, "due_date": null, "moved_to_id": null, "label_links": [ @@ -820,7 +817,6 @@ "iid": 7, "updated_by_id": null, "confidential": false, - "deleted_at": null, "due_date": null, "moved_to_id": null, "notes": [ @@ -1033,7 +1029,6 @@ "iid": 6, "updated_by_id": null, "confidential": false, - "deleted_at": null, "due_date": null, "moved_to_id": null, "notes": [ @@ -1246,7 +1241,6 @@ "iid": 5, "updated_by_id": null, "confidential": false, - "deleted_at": null, "due_date": null, "moved_to_id": null, "notes": [ @@ -1459,7 +1453,6 @@ "iid": 4, "updated_by_id": null, "confidential": false, - "deleted_at": null, "due_date": null, "moved_to_id": null, "notes": [ @@ -1672,7 +1665,6 @@ "iid": 3, "updated_by_id": null, "confidential": false, - "deleted_at": null, "due_date": null, "moved_to_id": null, "notes": [ @@ -1885,7 +1877,6 @@ "iid": 2, "updated_by_id": null, "confidential": false, - "deleted_at": null, "due_date": null, "moved_to_id": null, "notes": [ @@ -2098,7 +2089,6 @@ "iid": 1, "updated_by_id": null, "confidential": false, - "deleted_at": null, "due_date": null, "moved_to_id": null, "notes": [ @@ -2504,7 +2494,6 @@ "merge_when_pipeline_succeeds": true, "merge_user_id": null, "merge_commit_sha": null, - "deleted_at": null, "notes": [ { "id": 671, @@ -2948,7 +2937,6 @@ "merge_when_pipeline_succeeds": false, "merge_user_id": null, "merge_commit_sha": null, - "deleted_at": null, "notes": [ { "id": 679, @@ -3228,7 +3216,6 @@ "merge_when_pipeline_succeeds": false, "merge_user_id": null, "merge_commit_sha": null, - "deleted_at": null, "notes": [ { "id": 777, @@ -3508,7 +3495,6 @@ "merge_when_pipeline_succeeds": false, "merge_user_id": null, "merge_commit_sha": null, - "deleted_at": null, "notes": [ { "id": 785, @@ -4198,7 +4184,6 @@ "merge_when_pipeline_succeeds": false, "merge_user_id": null, "merge_commit_sha": null, - "deleted_at": null, "notes": [ { "id": 793, @@ -4734,7 +4719,6 @@ "merge_when_pipeline_succeeds": false, "merge_user_id": null, "merge_commit_sha": null, - "deleted_at": null, "notes": [ { "id": 801, @@ -5223,7 +5207,6 @@ "merge_when_pipeline_succeeds": false, "merge_user_id": null, "merge_commit_sha": null, - "deleted_at": null, "notes": [ { "id": 809, @@ -5478,7 +5461,6 @@ "merge_when_pipeline_succeeds": false, "merge_user_id": null, "merge_commit_sha": null, - "deleted_at": null, "notes": [ { "id": 817, @@ -6168,7 +6150,6 @@ "merge_when_pipeline_succeeds": false, "merge_user_id": null, "merge_commit_sha": null, - "deleted_at": null, "notes": [ { "id": 825, @@ -6968,7 +6949,6 @@ "id": 123, "token": "cdbfasdf44a5958c83654733449e585", "project_id": 5, - "deleted_at": null, "created_at": "2017-01-16T15:25:28.637Z", "updated_at": "2017-01-16T15:25:28.637Z" } diff --git a/spec/lib/gitlab/import_export/project.light.json b/spec/lib/gitlab/import_export/project.light.json index 02450478a77..5dbf0ed289b 100644 --- a/spec/lib/gitlab/import_export/project.light.json +++ b/spec/lib/gitlab/import_export/project.light.json @@ -54,7 +54,6 @@ "iid": 20, "updated_by_id": 1, "confidential": false, - "deleted_at": null, "due_date": null, "moved_to_id": null, "lock_version": null, diff --git a/spec/lib/gitlab/import_export/safe_model_attributes.yml b/spec/lib/gitlab/import_export/safe_model_attributes.yml index c5bf23e65b3..5a33fa3fd53 100644 --- a/spec/lib/gitlab/import_export/safe_model_attributes.yml +++ b/spec/lib/gitlab/import_export/safe_model_attributes.yml @@ -14,7 +14,6 @@ Issue: - iid - updated_by_id - confidential -- deleted_at - closed_at - due_date - moved_to_id @@ -159,7 +158,6 @@ MergeRequest: - merge_when_pipeline_succeeds - merge_user_id - merge_commit_sha -- deleted_at - in_progress_merge_commit_sha - lock_version - milestone_id @@ -294,7 +292,6 @@ Ci::Trigger: - id - token - project_id -- deleted_at - created_at - updated_at - owner_id @@ -310,7 +307,6 @@ Ci::PipelineSchedule: - project_id - owner_id - active -- deleted_at - created_at - updated_at Clusters::Cluster: diff --git a/spec/migrations/remove_soft_removed_objects_spec.rb b/spec/migrations/remove_soft_removed_objects_spec.rb new file mode 100644 index 00000000000..ec089f9106d --- /dev/null +++ b/spec/migrations/remove_soft_removed_objects_spec.rb @@ -0,0 +1,77 @@ +require 'spec_helper' +require Rails.root.join('db', 'post_migrate', '20171207150343_remove_soft_removed_objects.rb') + +describe RemoveSoftRemovedObjects, :migration do + describe '#up' do + it 'removes various soft removed objects' do + 5.times do + create_with_deleted_at(:issue) + end + + regular_issue = create(:issue) + + run_migration + + expect(Issue.count).to eq(1) + expect(Issue.first).to eq(regular_issue) + end + + it 'removes the temporary indexes once soft removed data has been removed' do + migration = described_class.new + + run_migration + + disable_migrations_output do + expect(migration.temporary_index_exists?(Issue)).to eq(false) + end + end + + it 'removes routes of soft removed personal namespaces' do + namespace = create_with_deleted_at(:namespace) + group = create(:group) + + expect(Route.where(source: namespace).exists?).to eq(true) + expect(Route.where(source: group).exists?).to eq(true) + + run_migration + + expect(Route.where(source: namespace).exists?).to eq(false) + expect(Route.where(source: group).exists?).to eq(true) + end + + it 'schedules the removal of soft removed groups' do + group = create_with_deleted_at(:group) + admin = create(:user, admin: true) + + expect_any_instance_of(GroupDestroyWorker) + .to receive(:perform) + .with(group.id, admin.id) + + run_migration + end + + it 'does not remove soft removed groups when no admin user could be found' do + create_with_deleted_at(:group) + + expect_any_instance_of(GroupDestroyWorker) + .not_to receive(:perform) + + run_migration + end + end + + def run_migration + disable_migrations_output do + migrate! + end + end + + def create_with_deleted_at(*args) + row = create(*args) + + # We set "deleted_at" this way so we don't run into any column cache issues. + row.class.where(id: row.id).update_all(deleted_at: 1.year.ago) + + row + end +end diff --git a/spec/models/ci/pipeline_schedule_spec.rb b/spec/models/ci/pipeline_schedule_spec.rb index 9a278212efc..8ee15f0e734 100644 --- a/spec/models/ci/pipeline_schedule_spec.rb +++ b/spec/models/ci/pipeline_schedule_spec.rb @@ -12,7 +12,6 @@ describe Ci::PipelineSchedule do it { is_expected.to respond_to(:cron_timezone) } it { is_expected.to respond_to(:description) } it { is_expected.to respond_to(:next_run_at) } - it { is_expected.to respond_to(:deleted_at) } describe 'validations' do it 'does not allow invalid cron patters' do diff --git a/spec/models/issue_spec.rb b/spec/models/issue_spec.rb index 5ced000cdb6..f5c9f551e65 100644 --- a/spec/models/issue_spec.rb +++ b/spec/models/issue_spec.rb @@ -18,11 +18,6 @@ describe Issue do subject { create(:issue) } - describe "act_as_paranoid" do - it { is_expected.to have_db_column(:deleted_at) } - it { is_expected.to have_db_index(:deleted_at) } - end - describe 'callbacks' do describe '#ensure_metrics' do it 'creates metrics after saving' do diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index 07b3e1c1758..27e9c477d61 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -24,11 +24,6 @@ describe MergeRequest do it { is_expected.to include_module(Taskable) } end - describe "act_as_paranoid" do - it { is_expected.to have_db_column(:deleted_at) } - it { is_expected.to have_db_index(:deleted_at) } - end - describe 'validation' do it { is_expected.to validate_presence_of(:target_branch) } it { is_expected.to validate_presence_of(:source_branch) } diff --git a/spec/models/namespace_spec.rb b/spec/models/namespace_spec.rb index b3f160f3119..c3673a0e2a3 100644 --- a/spec/models/namespace_spec.rb +++ b/spec/models/namespace_spec.rb @@ -410,17 +410,6 @@ describe Namespace do end end - describe '#soft_delete_without_removing_associations' do - let(:project1) { create(:project_empty_repo, namespace: namespace) } - - it 'updates the deleted_at timestamp but preserves projects' do - namespace.soft_delete_without_removing_associations - - expect(Project.all).to include(project1) - expect(namespace.deleted_at).not_to be_nil - end - end - describe '#user_ids_for_project_authorizations' do it 'returns the user IDs for which to refresh authorizations' do expect(namespace.user_ids_for_project_authorizations) diff --git a/spec/services/users/destroy_service_spec.rb b/spec/services/users/destroy_service_spec.rb index aeba9cd60bc..bb3d73edf8e 100644 --- a/spec/services/users/destroy_service_spec.rb +++ b/spec/services/users/destroy_service_spec.rb @@ -15,7 +15,7 @@ describe Users::DestroyService do expect { user_data['email'].to eq(user.email) } expect { User.find(user.id) }.to raise_error(ActiveRecord::RecordNotFound) - expect { Namespace.with_deleted.find(namespace.id) }.to raise_error(ActiveRecord::RecordNotFound) + expect { Namespace.find(namespace.id) }.to raise_error(ActiveRecord::RecordNotFound) end it 'will delete the project' do |