Allow configuring default priority value at midpoint of range (#39)

### Summary
This PR Adds a configuration value (`.assign_at_midpoint`) to `Delayed::Priority` 

The intent is to allow users to change the default priority of a job (i.e. when using a named priority) to be the middle of the configured priority ranges -- derived from the default or custom priority names -- as opposed to the starting value of the range.

By allowing a default value that is in the middle of the range, users will be able to bump the priority of some jobs such that they remain in their assigned queue but take priority over jobs of the same named priority.

**Note**: since the last priority uses a range that is infinite, it's not possible to derive the midpoint. So, for now, I've defaulted to adding 5, since that matches our defaulted priority values.

Author: tkoar

Gemfile.lock

@@ -1,7 +1,7 @@
 PATH
   remote: .
   specs:
-    delayed (0.5.3)
+    delayed (0.5.4)
       activerecord (>= 5.2)
       concurrent-ruby
 

README.md

@@ -491,6 +491,14 @@ Delayed::Worker.min_priority = nil
 Delayed::Worker.max_priority = nil
 ```
 
+Job priorities can specified by using the name of the desired range (i.e. :user_visible).
+By default, the value for a named priority will be the first value in that range.
+To set each priority's default value to the middle of its range (i.e. 15 for :user_visible), Delayed::Priority can be configured with:
+
+```ruby
+Delayed::Priority.assign_at_midpoint = true
+```
+
 Logging verbosity is also configurable. The gem will attempt to default to `Rails.logger` with an
 "info" log level.
 

delayed.gemspec

@@ -18,7 +18,7 @@ Gem::Specification.new do |spec|
   spec.require_paths  = ['lib']
   spec.summary        = 'a multi-threaded, SQL-driven ActiveJob backend used at Betterment to process millions of background jobs per day'
 
-  spec.version        = '0.5.3'
+  spec.version        = '0.5.4'
   spec.metadata       = {
     'changelog_uri' => 'https://github.com/betterment/delayed/blob/main/CHANGELOG.md',
     'bug_tracker_uri' => 'https://github.com/betterment/delayed/issues',

lib/delayed/priority.rb

@@ -57,6 +57,8 @@ class Priority < Numeric
     }.freeze
 
     class << self
+      attr_writer :assign_at_midpoint
+
       def names
         @names || default_names
       end
@@ -70,6 +72,7 @@ def names=(names)
 
         @ranges = nil
         @alerts = nil
+        @names_to_priority = nil
         @names = names&.sort_by(&:last)&.to_h&.transform_values { |v| new(v) }
       end
 
@@ -82,12 +85,25 @@ def alerts=(alerts)
         @alerts = alerts&.sort_by { |k, _| names.keys.index(k) }&.to_h
       end
 
+      def assign_at_midpoint?
+        @assign_at_midpoint || false
+      end
+
       def ranges
         @ranges ||= names.zip(names.except(names.keys.first)).each_with_object({}) do |((name, lower), (_, upper)), obj|
           obj[name] = (lower...(upper || Float::INFINITY))
         end
       end
 
+      def names_to_priority
+        @names_to_priority ||=
+          if assign_at_midpoint?
+            names_to_midpoint_priority
+          else
+            names
+          end
+      end
+
       private
 
       def default_names
@@ -98,13 +114,23 @@ def default_alerts
         @names ? {} : DEFAULT_ALERTS
       end
 
+      def names_to_midpoint_priority
+        names.each_cons(2).to_h { |(name, priority_value), (_, next_priority_value)|
+          [name, new(midpoint(priority_value, next_priority_value))]
+        }.merge(names.keys.last => new(names.values.last + 5))
+      end
+
+      def midpoint(low, high)
+        low + ((high - low).to_d / 2).ceil
+      end
+
       def respond_to_missing?(method_name, include_private = false)
-        names.key?(method_name) || super
+        names_to_priority.key?(method_name) || super
       end
 
       def method_missing(method_name, *args)
-        if names.key?(method_name) && args.none?
-          names[method_name]
+        if names_to_priority.key?(method_name) && args.none?
+          names_to_priority[method_name]
         else
           super
         end
@@ -118,7 +144,7 @@ def method_missing(method_name, *args)
 
     def initialize(value)
       super()
-      value = self.class.names[value] if value.is_a?(Symbol)
+      value = self.class.names_to_priority[value] if value.is_a?(Symbol)
       @value = value.to_i
     end
 
@@ -147,6 +173,20 @@ def <=>(other)
       to_i <=> other
     end
 
+    def -(other)
+      other = other.to_i if other.is_a?(self.class)
+      self.class.new(to_i - other)
+    end
+
+    def +(other)
+      other = other.to_i if other.is_a?(self.class)
+      self.class.new(to_i + other)
+    end
+
+    def to_d
+      to_i.to_d
+    end
+
     private
 
     def respond_to_missing?(method_name, include_private = false)

spec/delayed/priority_spec.rb

@@ -3,8 +3,10 @@
 RSpec.describe Delayed::Priority do
   let(:custom_names) { nil }
   let(:custom_alerts) { nil }
+  let(:assign_at_midpoint) { nil }
 
   around do |example|
+    described_class.assign_at_midpoint = assign_at_midpoint
     described_class.names = custom_names
     described_class.alerts = custom_alerts
     example.run
@@ -13,7 +15,7 @@
     described_class.names = nil
   end
 
-  describe '.names, .ranges, .alerts, method_missing' do
+  describe '.names, .ranges, .alerts, .names_to_priority, method_missing' do
     it 'defaults to interactive, user_visible, eventual, reporting' do
       expect(described_class.names).to eq(
         interactive: 0,
@@ -33,6 +35,12 @@
         eventual: { age: 1.5.hours, run_time: 5.minutes, attempts: 8 },
         reporting: { age: 4.hours, run_time: 10.minutes, attempts: 8 },
       )
+      expect(described_class.names_to_priority).to eq(
+        interactive: 0,
+        user_visible: 10,
+        eventual: 20,
+        reporting: 30,
+      )
       expect(described_class).to respond_to(:interactive)
       expect(described_class).to respond_to(:user_visible)
       expect(described_class).to respond_to(:eventual)
@@ -43,6 +51,23 @@
       expect(described_class.reporting).to eq 30
     end
 
+    context 'when assign_at_midpoint is set to true' do
+      let(:assign_at_midpoint) { true }
+
+      it 'returns the midpoint value' do
+        expect(described_class.names_to_priority).to eq(
+          interactive: 5,
+          user_visible: 15,
+          eventual: 25,
+          reporting: 35,
+        )
+        expect(described_class.interactive).to eq 5
+        expect(described_class.user_visible).to eq 15
+        expect(described_class.eventual).to eq 25
+        expect(described_class.reporting).to eq 35
+      end
+    end
+
     context 'when customized to high, medium, low' do
       let(:custom_names) { { high: 0, medium: 100, low: 500 } }
 
@@ -57,6 +82,11 @@
           medium: (100...500),
           low: (500...Float::INFINITY),
         )
+        expect(described_class.names_to_priority).to eq(
+          high: 0,
+          medium: 100,
+          low: 500,
+        )
         expect(described_class.alerts).to eq({})
         expect(described_class).not_to respond_to(:interactive)
         expect(described_class).not_to respond_to(:user_visible)
@@ -81,6 +111,21 @@
           )
         end
       end
+
+      context 'when assign_at_midpoint is set to true' do
+        let(:assign_at_midpoint) { true }
+
+        it 'returns the midpoint value' do
+          expect(described_class.names_to_priority).to eq(
+            high: 50,
+            medium: 300,
+            low: 505,
+          )
+          expect(described_class.high).to eq 50
+          expect(described_class.medium).to eq 300
+          expect(described_class.low).to eq 505
+        end
+      end
     end
   end
 
@@ -110,6 +155,36 @@
     expect(described_class.new(-123).interactive?).to eq false
   end
 
+  context 'when assign_at_midpoint is set to true' do
+    let(:assign_at_midpoint) { true }
+
+    it 'provides the name of the priority range' do
+      expect(described_class.new(0).name).to eq :interactive
+      expect(described_class.new(3).name).to eq :interactive
+      expect(described_class.new(10).name).to eq :user_visible
+      expect(described_class.new(29).name).to eq :eventual
+      expect(described_class.new(999).name).to eq :reporting
+      expect(described_class.new(-123).name).to eq nil
+    end
+
+    it 'supports initialization by symbol value' do
+      expect(described_class.new(:interactive)).to eq(5)
+      expect(described_class.new(:user_visible)).to eq(15)
+      expect(described_class.new(:eventual)).to eq(25)
+      expect(described_class.new(:reporting)).to eq(35)
+    end
+
+    it "supports predicate ('?') methods" do
+      expect(described_class.new(0).interactive?).to eq true
+      expect(described_class.new(3)).to be_interactive
+      expect(described_class.new(3).user_visible?).to eq false
+      expect(described_class.new(10)).to be_user_visible
+      expect(described_class.new(29)).to be_eventual
+      expect(described_class.new(999)).to be_reporting
+      expect(described_class.new(-123).interactive?).to eq false
+    end
+  end
+
   it 'supports alert threshold methods' do
     described_class.alerts = {
       interactive: { age: 77.seconds },
@@ -151,4 +226,13 @@
       ].sort,
     ).to eq [-13, 3, 5, 40]
   end
+
+  it 'supports addition and subtraction' do
+    expect(described_class.new(0) + 10).to eq(10)
+    expect(10 + described_class.new(5)).to eq(15)
+    expect(described_class.new(0) + described_class.new(33)).to eq(33)
+    expect(described_class.new(10) - 5).to eq(5)
+    expect(15 - described_class.new(10)).to eq(5)
+    expect(described_class.new(5) - described_class.new(15)).to eq(-10)
+  end
 end