diff --git a/app/models/account.rb b/app/models/account.rb index d2bc9547bf..bcc271b313 100644 --- a/app/models/account.rb +++ b/app/models/account.rb @@ -49,25 +49,25 @@ class Account < ApplicationRecord include ProviderDomains include Indices::AccountIndex::ForAccount - self.background_deletion = [ - :users, - :mail_dispatch_rules, - [:api_docs_services, { class_name: 'ApiDocs::Service' }], - :services, - :contracts, - :account_plans, - [:settings, { action: :destroy, class_name: 'Settings', has_many: false }], - [:payment_detail, { action: :destroy, has_many: false }], - [:buyer_accounts, { action: :destroy, class_name: 'Account' }], - [:payment_gateway_setting, { action: :destroy, has_many: false }], - [:profile, { action: :delete, has_many: false }], - [:templates, { action: :delete, class_name: 'CMS::Template' }], - [:sections, { action: :delete, class_name: 'CMS::Section' }], - [:provided_sections, { action: :delete, class_name: 'CMS::Section' }], - [:redirects, { action: :delete, class_name: 'CMS::Redirect' }], - [:files, { action: :delete, class_name: 'CMS::File' }], - [:builtin_pages, { action: :delete, class_name: 'CMS::BuiltinPage' }], - [:provided_groups, { action: :delete, class_name: 'CMS::Group' }] + self.background_deletion = %i[ + users + mail_dispatch_rules + api_docs_services + services + contracts + account_plans + settings + payment_detail + payment_gateway_setting + buyer_accounts + profile + templates + sections + provided_sections + redirects + files + builtin_pages + provided_groups ].freeze #TODO: this needs testing? diff --git a/app/models/alert.rb b/app/models/alert.rb index 7f20d1e31d..83e40d3e59 100644 --- a/app/models/alert.rb +++ b/app/models/alert.rb @@ -7,6 +7,14 @@ class Alert < ApplicationRecord self.default_sort_direction = :desc self.allowed_search_scopes = %w[cinstance_id account_id timestamp level] + self.background_deletion_method = :delete_truly + # commenting out this here just to make a note that to me it seems like state machine overrides + # the #delete AR method but what background deletion actually expects is indeed the AR #delete method + # self.background_deletion_scope_name = :non_deleted + # + # scope :non_deleted, -> { where.not(state: :deleted) } + alias delete_truly delete # keep reference to the original AR #delete method to use for background deletion + ALERT_LEVELS = [ 50, 80, 90, 100, 120, 150, 200, 300 ] VIOLATION_LEVEL = 100 diff --git a/app/models/cinstance.rb b/app/models/cinstance.rb index dd0e3f9958..af3375ecba 100644 --- a/app/models/cinstance.rb +++ b/app/models/cinstance.rb @@ -3,7 +3,7 @@ class Cinstance < Contract # Maximum number of cinstances permitted between provider and buyer MAX = 10 - self.background_deletion = [:referrer_filters, :application_keys, [:alerts, { action: :delete }]] + self.background_deletion = %i[referrer_filters application_keys alerts] delegate :backend_version, to: :service, allow_nil: true diff --git a/app/models/cms/builtin.rb b/app/models/cms/builtin.rb index 4e6d708f69..0753b7d253 100644 --- a/app/models/cms/builtin.rb +++ b/app/models/cms/builtin.rb @@ -113,6 +113,8 @@ class StaticPage < CMS::Builtin # CMS::Builtin::Page class Page < CMS::Builtin + validates :user_id, :forum_id, presence: true + def content_type 'text/html' end diff --git a/app/models/cms/file.rb b/app/models/cms/file.rb index b9b0786b75..0468ebb19e 100644 --- a/app/models/cms/file.rb +++ b/app/models/cms/file.rb @@ -11,6 +11,8 @@ class CMS::File < ApplicationRecord self.table_name = :cms_files + self.background_deletion_method = :delete + self.allowed_search_scopes = %i[section_id] scope :by_section_id, ->(section_id) { where(section_id: section_id) } diff --git a/app/models/cms/group.rb b/app/models/cms/group.rb index 10d208bb0b..3298155749 100644 --- a/app/models/cms/group.rb +++ b/app/models/cms/group.rb @@ -4,6 +4,8 @@ class CMS::Group < ApplicationRecord # This is BuyerGroup self.table_name = :cms_groups + self.background_deletion_method = :delete + belongs_to :provider, :class_name => "Account" validates :name, :provider, presence: true, length: { maximum: 255 } diff --git a/app/models/cms/redirect.rb b/app/models/cms/redirect.rb index 5a69d8d5c7..c5865343ee 100644 --- a/app/models/cms/redirect.rb +++ b/app/models/cms/redirect.rb @@ -11,4 +11,6 @@ class CMS::Redirect < ApplicationRecord include NormalizePathAttribute verify_path_format :source, :target + + self.background_deletion_method = :delete end diff --git a/app/models/cms/section.rb b/app/models/cms/section.rb index b94815fd6d..0b4a72ac07 100644 --- a/app/models/cms/section.rb +++ b/app/models/cms/section.rb @@ -8,6 +8,8 @@ class CMS::Section < ApplicationRecord self.table_name = :cms_sections + self.background_deletion_method = :delete + self.allowed_search_scopes = %i[parent_id] scope :by_parent_id, ->(parent_id) { where(parent_id: parent_id) } diff --git a/app/models/cms/template.rb b/app/models/cms/template.rb index df90f68be9..bfea80c17e 100644 --- a/app/models/cms/template.rb +++ b/app/models/cms/template.rb @@ -5,6 +5,8 @@ class CMS::Template < ApplicationRecord include ThreeScale::Search::Scopes include CMS::Filtering + self.background_deletion_method = :delete + scope :with_draft, ->{ where(['draft IS NOT NULL'])} scope :for_rails_view, ->(path) { where(rails_view_path: path.to_s) } diff --git a/app/models/concerns/background_deletion.rb b/app/models/concerns/background_deletion.rb index 5e38ac28d1..50b0841689 100644 --- a/app/models/concerns/background_deletion.rb +++ b/app/models/concerns/background_deletion.rb @@ -5,32 +5,17 @@ module BackgroundDeletion included do class_attribute :background_deletion, default: [], instance_writer: false + class_attribute :background_deletion_method, default: :destroy!, instance_writer: false + class_attribute :background_deletion_scope_name, default: :all, instance_writer: false end - class Reflection - - DEFAULT_DESTROY_METHOD = 'destroy' - DEFAULT_HAS_MANY_OPTION = true - - attr_reader :name, :options - - def initialize(association_config) - config = Array(association_config) - - @name = config[0] - @options = config[1].presence || {} - end - - def many? - options.fetch(:has_many) { DEFAULT_HAS_MANY_OPTION } - end - - def class_name - options[:class_name].presence || name.to_s.singularize.classify + class_methods do + def background_deletion_scope + send background_deletion_scope_name end + end - def background_destroy_method - options[:action].to_s.presence || DEFAULT_DESTROY_METHOD - end + def background_deletion_method_call + send background_deletion_method end end diff --git a/app/models/moderatorship.rb b/app/models/moderatorship.rb index 45d095309c..78a81d7ce0 100644 --- a/app/models/moderatorship.rb +++ b/app/models/moderatorship.rb @@ -6,6 +6,8 @@ class Moderatorship < ApplicationRecord attr_protected :forum_id, :user_id, :tenant_id + self.background_deletion_method = :delete + protected def uniqueness_of_relationship diff --git a/app/models/notification.rb b/app/models/notification.rb index bc7e447cc1..262fa15852 100644 --- a/app/models/notification.rb +++ b/app/models/notification.rb @@ -7,6 +7,8 @@ class Notification < ApplicationRecord self.available_notifications = NotificationMailer.available_notifications.map(&:to_s).freeze + self.background_deletion_method = :delete + delegate :hidden_onprem_multitenancy, to: NotificationMailer delegate :account, to: :user diff --git a/app/models/notification_preferences.rb b/app/models/notification_preferences.rb index 9a2594eb83..f60810e038 100644 --- a/app/models/notification_preferences.rb +++ b/app/models/notification_preferences.rb @@ -20,6 +20,8 @@ def self.preferences_to_hash(list, value:) self.enabled_by_default = (available_notifications - disabled_by_default).freeze self.hidden_notifications = NotificationMailer.hidden_notifications.freeze + self.background_deletion_method = :delete + enabled = preferences_to_hash(enabled_by_default, value: true) disabled = preferences_to_hash(disabled_by_default, value: false) diff --git a/app/models/oidc_configuration.rb b/app/models/oidc_configuration.rb index 5cf9bd0476..0222752cfa 100644 --- a/app/models/oidc_configuration.rb +++ b/app/models/oidc_configuration.rb @@ -55,6 +55,8 @@ def attributes end end + self.background_deletion_method = :delete + belongs_to :oidc_configurable, polymorphic: true, inverse_of: :oidc_configuration serialize :config, OIDCConfiguration::Config diff --git a/app/models/plan.rb b/app/models/plan.rb index a021fa8457..5ba6b80590 100644 --- a/app/models/plan.rb +++ b/app/models/plan.rb @@ -12,8 +12,7 @@ class PeriodRangeCalculationError < StandardError; end include SystemName include Logic::MetricVisibility::Plan - self.background_deletion = [:contracts, :plan_metrics, :pricing_rules, - :usage_limits, [:customizations, { action: :destroy, class_name: 'Plan' }]] + self.background_deletion = %i[contracts plan_metrics pricing_rules usage_limits customizations] has_system_name :uniqueness_scope => [ :type, :issuer_id, :issuer_type ] diff --git a/app/models/profile.rb b/app/models/profile.rb index b10103601b..efdf09d7cd 100644 --- a/app/models/profile.rb +++ b/app/models/profile.rb @@ -17,7 +17,8 @@ class Profile < ApplicationRecord after_initialize :set_company_size - audited :allow_mass_assignment => true + self.background_deletion_method = :delete + audited state_machine :initial => :private do state :private diff --git a/app/models/proxy.rb b/app/models/proxy.rb index 637d3d15d8..5253d5d0ae 100644 --- a/app/models/proxy.rb +++ b/app/models/proxy.rb @@ -12,7 +12,7 @@ class Proxy < ApplicationRecord # rubocop:disable Metrics/ClassLength define_proxy_config_affecting_attributes except: %i[api_test_path api_test_success lock_version] - self.background_deletion = [:proxy_rules, [:proxy_configs, { action: :delete }], [:oidc_configuration, { action: :delete, has_many: false }]] + self.background_deletion = %i[proxy_rules proxy_configs oidc_configuration] belongs_to :service, touch: true, inverse_of: :proxy, required: true attr_readonly :service_id diff --git a/app/models/proxy_config.rb b/app/models/proxy_config.rb index 2f22ac4a54..7c9450df92 100644 --- a/app/models/proxy_config.rb +++ b/app/models/proxy_config.rb @@ -14,6 +14,8 @@ class InvalidEnvironmentError < StandardError; end # Do not set it too high though the column accept until 16.megabytes MAX_CONTENT_LENGTH = 2.megabytes + self.background_deletion_method = :delete + belongs_to :proxy, optional: false belongs_to :user, optional: true diff --git a/app/models/service.rb b/app/models/service.rb index 72d8baf868..82934020f2 100644 --- a/app/models/service.rb +++ b/app/models/service.rb @@ -17,13 +17,13 @@ class Service < ApplicationRecord # rubocop:disable Metrics/ClassLength define_proxy_config_affecting_attributes :backend_version - self.background_deletion = [ - :service_plans, - :application_plans, - [:api_docs_services, class_name: 'ApiDocs::Service'], - :backend_api_configs, - :metrics, - [:proxy, { action: :destroy, has_many: false }] + self.background_deletion = %i[ + service_plans + application_plans + api_docs_services + backend_api_configs + metrics + proxy ].freeze DELETE_STATE = 'deleted' diff --git a/app/models/sso_authorization.rb b/app/models/sso_authorization.rb index 845606ca70..87fb5635af 100644 --- a/app/models/sso_authorization.rb +++ b/app/models/sso_authorization.rb @@ -11,6 +11,8 @@ class SSOAuthorization < ApplicationRecord scope :newest, -> { order(updated_at: :desc).first } + self.background_deletion_method = :delete + def mark_as_used(id_token) self.id_token = id_token self.updated_at = Time.now.utc diff --git a/app/models/user.rb b/app/models/user.rb index 47c1aae07d..16b686273a 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -23,13 +23,13 @@ class User < ApplicationRecord include ProvidedAccessTokens include Indices::AccountIndex::ForDependency - self.background_deletion = [ - :user_sessions, - :access_tokens, - [:sso_authorizations, { action: :delete }], - [:moderatorships, { action: :delete }], - [:notifications, { action: :delete }], - [:notification_preferences, { action: :delete, class_name: 'NotificationPreferences', has_many: false }] + self.background_deletion = %i[ + user_sessions + access_tokens + sso_authorizations + moderatorships + notifications + notification_preferences ].freeze audited except: %i[salt posts_count janrain_identifier cas_identifier diff --git a/app/subscribers/service_deletion_subscriber.rb b/app/subscribers/service_deletion_subscriber.rb index 839eb394b4..d7e4c65747 100644 --- a/app/subscribers/service_deletion_subscriber.rb +++ b/app/subscribers/service_deletion_subscriber.rb @@ -4,7 +4,7 @@ class ServiceDeletionSubscriber < AfterCommitSubscriber def after_commit(event) case event when Services::ServiceScheduledForDeletionEvent - DeleteObjectHierarchyWorker.perform_later(Service.find(event.service_id)) + DeleteObjectHierarchyWorker.delete_later(Service.find(event.service_id)) else raise "Unknown event type #{event.class}" end diff --git a/app/workers/delete_account_hierarchy_worker.rb b/app/workers/delete_account_hierarchy_worker.rb index f0bb53ce82..016c1819cb 100644 --- a/app/workers/delete_account_hierarchy_worker.rb +++ b/app/workers/delete_account_hierarchy_worker.rb @@ -1,30 +1 @@ -# frozen_string_literal: true - -class DeleteAccountHierarchyWorker < DeleteObjectHierarchyWorker - def perform(*) - return unless account.should_be_deleted? - super - end - - private - - alias account object - - def batch_description - id = account.id - org_name = account.org_name - if account.provider? - "Deleting provider [##{id}] #{org_name} - #{account.internal_admin_domain}" - else - "Deleting buyer [##{id}] of provider #{org_name}" - end - end - - def destroyable_association?(association) - if called_from_provider_hierarchy? && account.gateway_setting.persisted? - association != :buyer_accounts - else - super - end - end -end +DeleteAccountHierarchyWorker = DeleteObjectHierarchyWorker diff --git a/app/workers/delete_object_hierarchy_worker.rb b/app/workers/delete_object_hierarchy_worker.rb index 286fd093e7..be7309ab8c 100644 --- a/app/workers/delete_object_hierarchy_worker.rb +++ b/app/workers/delete_object_hierarchy_worker.rb @@ -2,148 +2,168 @@ class DeleteObjectHierarchyWorker < ApplicationJob - # TODO: Rails 5 --> discard_on ActiveJob::DeserializationError - # No need of ActiveRecord::RecordNotFound because that can only happen in the callbacks and those callbacks don't use this rescue_from but its own rescue - rescue_from(ActiveJob::DeserializationError) do |exception| - Rails.logger.info "DeleteObjectHierarchyWorker#perform raised #{exception.class} with message #{exception.message}" - end + WORK_TIME_LIMIT_SECONDS = 5 - queue_as :deletion + class DoNotRetryError < RuntimeError; end - before_perform do |job| - @object, workers_hierarchy, @background_destroy_method = job.arguments - id = "Hierarchy-#{object.class.name}-#{object.id}" - @caller_worker_hierarchy = Array(workers_hierarchy) + [id] - info "Starting #{job.class}#perform with the hierarchy of workers: #{caller_worker_hierarchy}" + rescue_from(DoNotRetryError) do |exception| + # report error and skip retries + System::ErrorReporting.report_error(exception) end - after_perform do |job| - info "Finished #{job.class}#perform with the hierarchy of workers: #{caller_worker_hierarchy}" + # we need this only for compatibility to process already enqueued jobs after upgrade + rescue_from(ActiveJob::DeserializationError) do |exception| + System::ErrorReporting.report_error(exception) end - def perform(_object, _caller_worker_hierarchy = [], _background_destroy_method = 'destroy') - build_batch - end + queue_as :deletion - def on_success(_, options) - on_finish('on_success', options) - end + # until_executed seems to rely on #after_perform which is skipped on failure so not sure whether retries will work. + # additionally if we want to reschedule ourselves in #after_perform, it is not nice to rely on flaky order + # see https://github.com/3scale/activejob-uniqueness/blob/main/lib/active_job/uniqueness/strategies/until_executed.rb + unique :until_and_while_executing, lock_ttl: 6.hours - def on_complete(_, options) - on_finish('on_complete', options) - end + # better limit by available `delete` executors + # sidekiq_throttle concurrency: { limit: 10 } - def on_finish(method_name, options) - workers_hierarchy = options['caller_worker_hierarchy'] - info "Starting DeleteObjectHierarchyWorker##{method_name} with the hierarchy of workers: #{workers_hierarchy}" - object = GlobalID::Locator.locate(options['object_global_id']) - background_destroy_method = @background_destroy_method.presence || 'destroy' - DeletePlainObjectWorker.perform_later(object, workers_hierarchy, background_destroy_method) - info "Finished DeleteObjectHierarchyWorker##{method_name} with the hierarchy of workers: #{workers_hierarchy}" - rescue ActiveRecord::RecordNotFound => exception - info "DeleteObjectHierarchyWorker##{method_name} raised #{exception.class} with message #{exception.message}" + before_perform do |job| + info "Starting #{job.class}#perform with the hierarchy of workers: #{job.arguments}" end - protected + after_perform do |job| + if @remaining_hierarchy.present? + self.class.perform_later(*@remaining_hierarchy) + msg = " iteration of" + end + info "Finished#{msg} #{job.class}#perform with the hierarchy of workers: #{job.arguments}" + end - delegate :info, to: 'Rails.logger' + class << self + # convenience method to schedule deleting an active record object + def delete_later(ar_object) + perform_later *hierarchy_entries_for(ar_object) + end - attr_reader :object, :caller_worker_hierarchy + private - def build_batch - batch = Sidekiq::Batch.new - batch.description = batch_description - batch_callbacks(batch) { batch.jobs { destroy_and_delete_associations } } - batch - end + # @return the hierarchy entries to handle deletion of an object + def hierarchy_entries_for(ar_object) + return [] unless ar_object.persisted? # e.g. when calling Proxy#oidc_configuration a new object can be generated - def batch_description - "Deleting #{object.class.name} [##{object.id}]" - end + if ar_object.is_a?(Account) && !ar_object.should_be_deleted? + raise DoNotRetryError, "background deleting account #{ar_object.id} which is not scheduled for deletion" + end - def batch_callbacks(batch) - %i[success complete].each { |name| batch.on(name, self.class, callback_options) } - yield - bid = batch.bid + ar_object_str = "#{ar_object.class}-#{ar_object.id}" - if Sidekiq::Batch::Status.new(bid).total.zero? - on_complete(bid, callback_options) - else - info("DeleteObjectHierarchyWorker#batch_success_callback retry job with the hierarchy of workers: #{caller_worker_hierarchy}") - retry_job wait: 5.minutes + [ + "Plain-#{ar_object_str}", + *ar_object.background_deletion.map { "Association-#{ar_object_str}:#{_1}" }, + ] end end - def destroy_and_delete_associations - Array(object.background_deletion).each do |association_config| - reflection = BackgroundDeletion::Reflection.new(association_config) - next unless destroyable_association?(reflection.name) + # @param hierarchy [Array] something like ["Plain-Service-1234", "Association-Service-1234:plans" ...] + # @note processes deleting a hierarchy of objects and reschedules itself at current progress after a time limit + def perform(*hierarchy) + return compatibility(hierarchy) unless hierarchy.first.is_a?(String) - ReflectionDestroyer.new(object, reflection, caller_worker_hierarchy).destroy_later + started = now + while hierarchy.present? && now - started < WORK_TIME_LIMIT_SECONDS + Rails.logger.info "Starting background deletion iteration with: #{hierarchy.join(' ')}" + hierarchy.concat handle_hierarchy_entry(hierarchy.pop) end + + @remaining_hierarchy = hierarchy + # can be like this instead of in `after_perform`, the benefit is that we don't use undocumented order of callbacks + # return unless hierarchy.present? + # + # # depending on + # unlock(resource: lock_key) + # self.class.perform_later(*hierarchy) end - def destroyable_association?(_association) - true + # we can just use job.arguments.first but for compatibility mode we want it not to lock + def lock_key + first_argument = arguments.first + first_argument.is_a?(String) ? first_argument : Random.uuid end private - def called_from_provider_hierarchy? - return unless (tenant_id = object.tenant_id) - caller_worker_hierarchy.include?("Hierarchy-Account-#{tenant_id}") + def hierarchy_entries_for(...) + self.class.send(:hierarchy_entries_for, ...) end - def callback_options - { 'object_global_id' => object.to_global_id, 'caller_worker_hierarchy' => caller_worker_hierarchy } + # handles a single hierarchy entry + # @return [Array] hierarchy for a newly discovered object from association to delete or empty array otherwise + def handle_hierarchy_entry(entry) + case entry + when /Plain-([:\w]+)-(\d+)/ + ar_object = $1.constantize.find($2.to_i) + # callbacks logic differs between object destroyed by association, it is standalone if first job arg is itself + ar_object.destroyed_by_association = DummyDestroyedByAssociationReflection.new(entry) if arguments.first != entry + ar_object.background_deletion_method_call + [] + when /Association-([:\w]+)-(\d+):(\w+)/ + handle_association($1.constantize.find($2.to_i), $3, entry) + else + raise ArgumentError, "Invalid entry specification: #{entry}" + end + rescue ActiveRecord::RecordNotFound => exception + Rails.logger.warn "#{self.class} skipping object, maybe something else already deleted it: #{exception.message}" + rescue NameError + raise DoNotRetryError, "seems like unexpectedly broken delete hierarchy entry: #{entry}" end - class ReflectionDestroyer - - def initialize(main_object, reflection, caller_worker_hierarchy) - @main_object = main_object - @reflection = reflection - @caller_worker_hierarchy = caller_worker_hierarchy + # @return a single associated object for deletion or nil if non in the association + def handle_association(ar_object, association, hierarchy_association_string) + reflection = ar_object.class.reflect_on_association(association) + case reflection.macro + when :has_many + # here we keep original hierarchy entry if we still find an associated object + dependent = ar_object.public_send(association).public_send(:background_deletion_scope).take + dependent ? [hierarchy_association_string, *hierarchy_entries_for(dependent)] : [] + when :has_one + # maximum of one associated so we never keep the original hierarchy entry + hierarchy_entries_for ar_object.public_send(association) + else + raise ArgumentError, "Cannot handle association #{ar_object}:#{association} type #{reflection.macro}" end + end - def destroy_later - reflection.many? ? destroy_has_many_association : destroy_has_one_association - end + def compatibility(_object, caller_worker_hierarchy = [], _background_destroy_method = 'destroy') + # maybe requeue first object from hierarchy would be adequate and uniqueness should deduplicate jobs + hierarchy_root = Array(caller_worker_hierarchy).first + return unless hierarchy_root - attr_reader :main_object, :reflection, :caller_worker_hierarchy + object_class, id = hierarchy_root.match(/Hierarchy-([a-zA-Z0-9_]+)-([\d*]+)/).captures - private + raise DoNotRetryError, "background deletion cannot handle #{hierarchy_root}" unless object_class && id - def destroy_has_many_association - main_object.public_send("#{reflection.name.to_s.singularize}_ids").each do |associated_object_id| - associated_object = reflection.class_name.constantize.new - associated_object.id = associated_object_id - delete_associated_object_later(associated_object) - end - rescue ActiveRecord::UnknownPrimaryKey => exception - Rails.logger.info "DeleteObjectHierarchyWorker#perform raised #{exception.class} with message #{exception.message}" - end + self.class.delete_later object_class.constantize.find(id.to_i) + end - def destroy_has_one_association - associated_object = main_object.public_send(reflection.name) - delete_associated_object_later(associated_object) + def associations_strings(ar_object, *associations) + associations = ar_object.class.background_deletion if associations.blank? + associations.map do |association| + "Association-#{ar_object.class}-#{ar_object.id}:#{association}" end + end - def delete_associated_object_later(associated_object) - association_delete_worker.perform_later(associated_object, caller_worker_hierarchy, reflection.background_destroy_method) if associated_object.try(:id) - end + delegate :info, to: 'Rails.logger' - def association_delete_worker - case reflection.class_name - when Account.name - DeleteAccountHierarchyWorker - when PaymentGatewaySetting.name - DeletePaymentSettingHierarchyWorker - else - DeleteObjectHierarchyWorker - end + def now + Process.clock_gettime(Process::CLOCK_MONOTONIC) + end + + class DummyDestroyedByAssociationReflection + def initialize(foreign_key) + @foreign_key = foreign_key end + + attr_reader :foreign_key end - private_constant :ReflectionDestroyer + private_constant :DoNotRetryError, :DummyDestroyedByAssociationReflection end diff --git a/app/workers/delete_payment_setting_hierarchy_worker.rb b/app/workers/delete_payment_setting_hierarchy_worker.rb index d5e22ae42d..f6627c9fc2 100644 --- a/app/workers/delete_payment_setting_hierarchy_worker.rb +++ b/app/workers/delete_payment_setting_hierarchy_worker.rb @@ -1,16 +1 @@ -# frozen_string_literal: true - -class DeletePaymentSettingHierarchyWorker < DeleteObjectHierarchyWorker - private - - alias payment_setting object - - def destroy_and_delete_associations - super - - return unless called_from_provider_hierarchy? - - reflection = BackgroundDeletion::Reflection.new([:buyer_accounts, { action: :destroy, class_name: 'Account' }]) - ReflectionDestroyer.new(object.account, reflection, caller_worker_hierarchy).destroy_later - end -end +DeletePaymentSettingHierarchyWorker = DeleteObjectHierarchyWorker diff --git a/app/workers/delete_plain_object_worker.rb b/app/workers/delete_plain_object_worker.rb index 6686248d37..1ee007e271 100644 --- a/app/workers/delete_plain_object_worker.rb +++ b/app/workers/delete_plain_object_worker.rb @@ -1,78 +1,5 @@ # frozen_string_literal: true -class DeletePlainObjectWorker < ApplicationJob - include Sidekiq::Throttled::Job - - # TODO: Rails 5 --> discard_on ActiveJob::DeserializationError, ActiveRecord::RecordNotFound - rescue_from(ActiveJob::DeserializationError, ActiveRecord::RecordNotFound) do |exception| - Rails.logger.info "DeletePlainObjectWorker#perform raised #{exception.class} with message #{exception.message}" - end - - rescue_from(ActiveRecord::RecordNotDestroyed) do |exception| - if object.class.exists?(object.id) - # We don't want to indefinitely try again to delete an object that for any reason can not be destroyed, so we just log it instead - System::ErrorReporting.report_error(exception, parameters: { caller_worker_hierarchy: caller_worker_hierarchy, - error_messages: exception.record.errors.full_messages }) - end - end - - rescue_from(ActiveRecord::StaleObjectError) do |exception| - Rails.logger.info "DeletePlainObjectWorker#perform raised #{exception.class} with message #{exception.message} for the hierarchy #{caller_worker_hierarchy}" - retry_job if object.class.exists?(object.id) - end - - queue_as :default - - sidekiq_throttle concurrency: { limit: 10 } - - before_perform do |job| - @object, workers_hierarchy, @destroy_method = job.arguments - @id = "Plain-#{object.class.name}-#{object.id}" - @caller_worker_hierarchy = Array(workers_hierarchy) + [@id] - info "Starting #{job.class}#perform with the hierarchy of workers: #{caller_worker_hierarchy}" - end - - after_perform do |job| - info "Finished #{job.class}#perform with the hierarchy of workers: #{caller_worker_hierarchy}" - end - - def perform(_object, _caller_worker_hierarchy = [], _destroy_method = 'destroy') - should_destroy_by_association? ? destroy_by_association : object.public_send(destroy_method(bang_if_possible: true)) - end - - private - - delegate :info, to: 'Rails.logger' - - attr_reader :caller_worker_hierarchy, :id, :object - - def destroy_method(bang_if_possible: false) - object_destroy_method = @destroy_method.presence || 'destroy' - - (bang_if_possible && object_destroy_method == 'destroy') ? 'destroy!' : object_destroy_method - end - - def should_destroy_by_association? - # If there is 1 only it means caller_worker_hierarchy argument to perform was an empty array and that 1 element is this DeletePlainObjectWorker instance itself - # If there are 2 it means that there is a DeleteObjectHierarchyWorker object (position 0 of the Array) that called to this DeletePlainObjectWorker instance itself (position 1 of the Array) - # More than 2 it means there is a chain of DeleteObjectHierarchyWorker objects before reaching this DeletePlainObjectWorker and that chain is the 'delete by association'. - # For example this could be a Service object with ID 3 of a Provider object with ID 2 and the chain at this point would look like [Hierarchy-Provider-2, Hierarchy-Service-3, Plain-Service-3]. - # This 'code' is for logging purposes only and Hierarchy means DeleteObjectHierarchyWorker and Plain means DeletePlainObjectWorker, then the model and the ID. - # You can see more about this argument and what should happen here in DeletePlainObjectWorkerTest - caller_worker_hierarchy.length > 2 - end - - def destroy_by_association - object.destroyed_by_association = DummyDestroyedByAssociationReflection.new(id) - object.public_send(destroy_method) - end - - class DummyDestroyedByAssociationReflection - def initialize(foreign_key) - @foreign_key = foreign_key - end - attr_reader :foreign_key - end - private_constant :DummyDestroyedByAssociationReflection - -end +# this class should only exist for compatibility for one release to complete enqueued jobs after upgrade +# it basically expects DeleteObjectHierarchyWorker#compatibility to do the job +DeletePlainObjectWorker = DeleteObjectHierarchyWorker diff --git a/test/workers/delete_object_hierarchy_worker_test.rb b/test/workers/delete_object_hierarchy_worker_test.rb index a871bddb34..b46ec70270 100644 --- a/test/workers/delete_object_hierarchy_worker_test.rb +++ b/test/workers/delete_object_hierarchy_worker_test.rb @@ -141,6 +141,18 @@ class DeleteServiceTest < DeleteObjectHierarchyWorkerTest @backend_api_config = FactoryBot.create(:backend_api_config, service: service, backend_api: backend_api) end + test "delete without stubs" do + service = FactoryBot.create(:service) + plan = FactoryBot.create(:application_plan, :issuer => service) + cinstance = FactoryBot.create(:cinstance, plan:) + FactoryBot.create_list(:limit_alert, 4, cinstance:, account: cinstance.user_account) + # perform_enqueued_jobs(except: [SphinxAccountIndexationWorker, SphinxIndexationWorker, BackendMetricWorker, BackendDeleteApplicationWorker]) do + perform_enqueued_jobs(queue: "deletion") do + service.mark_as_deleted! + end + assert_raise(ActiveRecord::RecordNotFound) { service.reload } + end + private attr_reader :service, :service_plan, :application_plan, :metrics, :api_docs_service, :backend_api, :backend_api_config