Create Schedulable concern

Extract common components from Ci::PipelineSchedule
and ContainerExpirationPolicy into a Schedulable
concern

app/models/ci/pipeline_schedule.rb

@@ -5,6 +5,7 @@ module Ci
     extend Gitlab::Ci::Model
     include Importable
     include StripAttribute
+    include Schedulable
 
     belongs_to :project
     belongs_to :owner, class_name: 'User'
@@ -18,13 +19,10 @@ module Ci
     validates :description, presence: true
     validates :variables, variable_duplicates: true
 
-    before_save :set_next_run_at
-
     strip_attributes :cron
 
     scope :active, -> { where(active: true) }
     scope :inactive, -> { where(active: false) }
-    scope :runnable_schedules, -> { active.where("next_run_at < ?", Time.now) }
     scope :preloaded, -> { preload(:owner, :project) }
 
     accepts_nested_attributes_for :variables, allow_destroy: true
@@ -62,13 +60,6 @@ module Ci
                          end
     end
 
-    # To be extracted with technical debt issue https://gitlab.com/gitlab-org/gitlab/issues/191331
-    def schedule_next_run!
-      save! # with set_next_run_at
-    rescue ActiveRecord::RecordInvalid
-      update_column(:next_run_at, nil) # update without validation
-    end
-
     def job_variables
       variables&.map(&:to_runner_variable) || []
     end

app/models/concerns/schedulable.rb

+# frozen_string_literal: true
+
+module Schedulable
+  extend ActiveSupport::Concern
+
+  included do
+    scope :runnable_schedules, -> { active.where("next_run_at < ?", Time.zone.now) }
+
+    before_save :set_next_run_at
+  end
+
+  def schedule_next_run!
+    save! # with set_next_run_at
+  rescue ActiveRecord::RecordInvalid
+    update_column(:next_run_at, nil) # update without validation
+  end
+end

app/models/container_expiration_policy.rb

 # frozen_string_literal: true
 
 class ContainerExpirationPolicy < ApplicationRecord
+  include Schedulable
+
   belongs_to :project, inverse_of: :container_expiration_policy
 
   delegate :container_repositories, to: :project
@@ -11,12 +13,9 @@ class ContainerExpirationPolicy < ApplicationRecord
   validates :older_than, inclusion: { in: ->(_) { self.older_than_options.stringify_keys } }, allow_nil: true
   validates :keep_n, inclusion: { in: ->(_) { self.keep_n_options.keys } }, allow_nil: true
 
-  scope :enabled, -> { where(enabled: true) }
-  scope :runnable_schedules, -> { enabled.where("next_run_at < ?", Time.zone.now) }
+  scope :active, -> { where(enabled: true) }
   scope :preloaded, -> { preload(:project) }
 
-  before_save :set_next_run_at
-
   def self.keep_n_options
     {
       1 => _('%{tags} tag per image name') % { tags: 1 },
@@ -50,11 +49,4 @@ class ContainerExpirationPolicy < ApplicationRecord
   def set_next_run_at
     self.next_run_at = Time.zone.now + ChronicDuration.parse(cadence).seconds
   end
-
-  # To be extracted with technical debt issue https://gitlab.com/gitlab-org/gitlab/issues/191331
-  def schedule_next_run!
-    save! # executes set_next_run_at
-  rescue ActiveRecord::RecordInvalid
-    update_column(:next_run_at, nil) # update without validation
-  end
 end

spec/factories/container_expiration_policies.rb

 # frozen_string_literal: true
 
 FactoryBot.define do
-  factory :container_expiration_policy, class: ContainerExpirationPolicy do
+  factory :container_expiration_policy, class: 'ContainerExpirationPolicy' do
     association :project, factory: [:project, :without_container_expiration_policy]
     cadence { '1d' }
     enabled { true }

spec/models/concerns/schedulable_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Schedulable do
+  shared_examples 'before_save callback' do
+    it 'updates next_run_at' do
+      expect { object.save! }.to change { object.next_run_at }
+    end
+  end
+
+  shared_examples '.runnable_schedules' do
+    it 'returns the runnable schedules' do
+      results = object.class.runnable_schedules
+
+      expect(results).to include(object)
+      expect(results).not_to include(non_runnable_object)
+    end
+  end
+
+  shared_examples '#schedule_next_run!' do
+    it 'saves the object and sets next_run_at' do
+      expect { object.schedule_next_run! }.to change { object.next_run_at }
+    end
+
+    it 'sets next_run_at to nil on error' do
+      expect(object).to receive(:save!).and_raise(ActiveRecord::RecordInvalid)
+
+      object.schedule_next_run!
+
+      expect(object.next_run_at).to be_nil
+    end
+  end
+
+  context 'for a pipeline_schedule' do
+    # let! is used to reset the next_run_at value before each spec
+    let(:object) do
+      Timecop.freeze(1.day.ago) do
+        create(:ci_pipeline_schedule, :hourly)
+      end
+    end
+
+    let(:non_runnable_object) { create(:ci_pipeline_schedule, :hourly) }
+
+    it_behaves_like '#schedule_next_run!'
+    it_behaves_like 'before_save callback'
+    it_behaves_like '.runnable_schedules'
+  end
+
+  context 'for a container_expiration_policy' do
+    # let! is used to reset the next_run_at value before each spec
+    let(:object) { create(:container_expiration_policy, :runnable) }
+    let(:non_runnable_object) { create(:container_expiration_policy) }
+
+    it_behaves_like '#schedule_next_run!'
+    it_behaves_like 'before_save callback'
+    it_behaves_like '.runnable_schedules'
+  end
+end