Improve domain discard

- Extract rake task domain:discard
- Remove background job when keeping a domain

#790

app/models/concerns/domain/deletable.rb

@@ -1,12 +1,43 @@
 module Concerns::Domain::Deletable
   extend ActiveSupport::Concern
 
+  class_methods do
+    def discard_domains
+      domains = where('delete_at < ? AND ? != ALL(statuses) AND ? != ALL(statuses)',
+                      Time.zone.now,
+                      DomainStatus::SERVER_DELETE_PROHIBITED,
+                      DomainStatus::DELETE_CANDIDATE)
+
+      domains.map(&:discard)
+    end
+  end
+
   def discard
     statuses << DomainStatus::DELETE_CANDIDATE
+    # We don't validate deliberately since nobody is interested in fixing discarded domain
+    save(validate: false)
+    delete_later
+    logger.info "Domain #{name} (ID: #{id}) is scheduled to be deleted"
+  end
+
+  def keep
+    statuses.delete(DomainStatus::DELETE_CANDIDATE)
     save
+    do_not_delete_later
   end
 
   def discarded?
     statuses.include?(DomainStatus::DELETE_CANDIDATE)
   end
+
+  private
+
+  def delete_later
+    run_at = rand(((24 * 60) - (DateTime.now.hour * 60 + DateTime.now.minute))).minutes.from_now
+    DomainDeleteJob.enqueue(id, run_at: run_at)
+  end
+
+  def do_not_delete_later
+    QueJob.find_by!("args->>0 = '#{id}'").delete
+  end
 end

app/models/domain.rb

@@ -281,13 +281,6 @@ class Domain < ActiveRecord::Base
     true
   end
 
-  def delete_candidateable?
-    return false if delete_at > Time.zone.now
-    return false if statuses.include?(DomainStatus::DELETE_CANDIDATE)
-    return false if statuses.include?(DomainStatus::SERVER_DELETE_PROHIBITED)
-    true
-  end
-
   def renewable?
     if Setting.days_to_renew_domain_before_expire != 0
       # if you can renew domain at days_to_renew before domain expiration
@@ -683,10 +676,6 @@ class Domain < ActiveRecord::Base
     where("#{attribute_alias(:outzone_time)} < ?", Time.zone.now)
   end
 
-  def self.delete_candidates
-    where('delete_at < ?', Time.zone.now)
-  end
-
   def self.uses_zone?(zone)
     exists?(["name ILIKE ?", "%.#{zone.origin}"])
   end

app/models/domain_cron.rb

@@ -104,22 +104,6 @@ class DomainCron
 
     c = 0
 
-    domains = Domain.delete_candidates
-
-    domains.each do |domain|
-      next unless domain.delete_candidateable?
-
-      domain.statuses << DomainStatus::DELETE_CANDIDATE
-
-      # If domain successfully saved, add it to delete schedule
-      if domain.save(validate: false)
-        ::PaperTrail.whodunnit = "cron - #{__method__}"
-        DomainDeleteJob.enqueue(domain.id, run_at: rand(((24*60) - (DateTime.now.hour * 60  + DateTime.now.minute))).minutes.from_now)
-        STDOUT << "#{Time.zone.now.utc} Domain.destroy_delete_candidates: job added by deleteCandidate status ##{domain.id} (#{domain.name})\n" unless Rails.env.test?
-        c += 1
-      end
-    end
-
     Domain.where('force_delete_at <= ?', Time.zone.now.end_of_day.utc).each do |x|
       DomainDeleteJob.enqueue(x.id, run_at: rand(((24*60) - (DateTime.now.hour * 60  + DateTime.now.minute))).minutes.from_now)
       STDOUT << "#{Time.zone.now.utc} DomainCron.destroy_delete_candidates: job added by force delete time ##{x.id} (#{x.name})\n" unless Rails.env.test?

app/models/que_job.rb

@@ -0,0 +1,3 @@
+class QueJob < ActiveRecord::Base # To be able to remove existing jobs
+  self.primary_key = 'job_id'
+end

config/schedule.rb

@@ -61,6 +61,10 @@ if @cron_group == 'registry'
   every :day, at: '19:00pm' do
     runner 'Directo.send_receipts'
   end if @environment == 'production'
+
+  every 42.minutes do
+    rake 'domain:discard'
+  end
 end
 
 every 10.minutes do

lib/tasks/domain.rake

@@ -0,0 +1,5 @@
+namespace :domain do
+  task discard: :environment do
+    Domain.discard_domains
+  end
+end

spec/models/domain_spec.rb

@@ -871,22 +871,6 @@ RSpec.describe Domain do
     end
   end
 
-  describe '::delete_candidates', db: true do
-    before :example do
-      travel_to Time.zone.parse('05.07.2010 00:00')
-
-      create(:zone, origin: 'ee')
-
-      create(:domain, id: 1, delete_at: Time.zone.parse('04.07.2010 23:59'))
-      create(:domain, id: 2, delete_at: Time.zone.parse('05.07.2010 00:00'))
-      create(:domain, id: 3, delete_at: Time.zone.parse('05.07.2010 00:01'))
-    end
-
-    it 'returns domains with delete time in the past' do
-      expect(described_class.delete_candidates.ids).to eq([1])
-    end
-  end
-
   describe '::uses_zone?', db: true do
     let!(:zone) { create(:zone, origin: 'domain.tld') }
 

test/integration/epp/domain/transfer/request_test.rb

@@ -76,7 +76,7 @@ class EppDomainTransferRequestTest < ActionDispatch::IntegrationTest
   end
 
   def test_discarded_domain_cannot_be_transferred
-    @domain.update!(statuses: [DomainStatus::DELETE_CANDIDATE])
+    @domain.discard
 
     post '/epp/command/transfer', { frame: request_xml }, { 'HTTP_COOKIE' => 'session=api_goodnames' }
     @domain.reload

test/integration/tasks/discard_domain_test.rb

@@ -0,0 +1,43 @@
+require 'test_helper'
+
+class DiscardDomainTaskTest < ActiveSupport::TestCase
+  setup do
+    travel_to Time.zone.parse('2010-07-05 08:00')
+    @domain = domains(:shop)
+  end
+
+  def test_discard_domains_with_past_delete_at
+    @domain.update!(delete_at: Time.zone.parse('2010-07-05 07:59'))
+    Rake::Task['domain:discard'].execute
+    @domain.reload
+    assert @domain.discarded?
+  end
+
+  def test_ignore_domains_with_delete_at_in_the_future_or_now
+    @domain.update!(delete_at: Time.zone.parse('2010-07-05 08:00'))
+    Rake::Task['domain:discard'].execute
+    @domain.reload
+    refute @domain.discarded?
+  end
+
+  def test_ignore_already_discarded_domains
+    @domain.update!(delete_at: Time.zone.parse('2010-07-05 07:59'))
+    @domain.discard
+
+    job_count = lambda do
+      QueJob.where("args->>0 = '#{@domain.id}'", job_class: DomainDeleteJob.name).count
+    end
+
+    assert_no_difference job_count, 'A domain should not be discarded again' do
+      Rake::Task['domain:discard'].execute
+    end
+  end
+
+  def test_ignore_domains_with_server_delete_prohibited_status
+    @domain.update!(delete_at: Time.zone.parse('2010-07-05 07:59'),
+                    statuses: [DomainStatus::SERVER_DELETE_PROHIBITED])
+    Rake::Task['domain:discard'].execute
+    @domain.reload
+    refute @domain.discarded?
+  end
+end

test/models/domain/deletable_test.rb

@@ -5,10 +5,25 @@ class DomainDeletableTest < ActiveSupport::TestCase
     @domain = domains(:shop)
   end
 
-  def test_discard
+  def test_discard_domain
-    refute @domain.discarded?
     @domain.discard
     @domain.reload
+    assert QueJob.find_by("args->>0 = '#{@domain.id}'", job_class: DomainDeleteJob.name)
     assert @domain.discarded?
   end
+
+  def test_discard_invalid_domain
+    domain = domains(:invalid)
+    domain.discard
+    domain.reload
+    assert domain.discarded?, 'a domain should be discarded'
+  end
+
+  def test_keep_domain
+    @domain.discard
+    @domain.keep
+    @domain.reload
+    assert_nil QueJob.find_by("args->>0 = '#{@domain.id}'", job_class: DomainDeleteJob.name)
+    refute @domain.discarded?
+  end
 end