Ensure Concurrency Keys are string-like and return a better error when they cannot be cast to a string

app/models/good_job/lockable.rb

@@ -193,6 +193,7 @@ def advisory_lock_key(key, function: advisory_lockable_function)
         binds = [
           ActiveRecord::Relation::QueryAttribute.new('key', key, ActiveRecord::Type::String.new),
         ]
+
         locked = connection.exec_query(pg_or_jdbc_query(query), 'GoodJob::Lockable Advisory Lock', binds).first['locked']
         return locked unless block_given?
         return nil unless locked

lib/good_job/active_job_extensions/concurrency.rb

@@ -4,6 +4,8 @@ module ActiveJobExtensions
     module Concurrency
       extend ActiveSupport::Concern
 
+      VALID_TYPES = [String, Symbol, Numeric, Date, Time, TrueClass, FalseClass, NilClass].freeze
+
       class ConcurrencyExceededError < StandardError
         def backtrace
           [] # suppress backtrace
@@ -30,6 +32,11 @@ def deserialize(job_data)
           # Always allow jobs to be retried because the current job's execution will complete momentarily
           next(block.call) if CurrentThread.active_job_id == job.job_id
 
+          # Only generate the concurrency key on the initial enqueue in case it is dynamic
+          job.good_job_concurrency_key ||= job._good_job_concurrency_key
+          key = job.good_job_concurrency_key
+          next(block.call) if key.blank?
+
           enqueue_limit = job.class.good_job_concurrency_config[:enqueue_limit]
           enqueue_limit = instance_exec(&enqueue_limit) if enqueue_limit.respond_to?(:call)
           enqueue_limit = nil unless enqueue_limit.present? && (0...Float::INFINITY).cover?(enqueue_limit)
@@ -43,11 +50,6 @@ def deserialize(job_data)
           limit = enqueue_limit || total_limit
           next(block.call) unless limit
 
-          # Only generate the concurrency key on the initial enqueue in case it is dynamic
-          job.good_job_concurrency_key ||= job._good_job_concurrency_key
-          key = job.good_job_concurrency_key
-          next(block.call) if key.blank?
-
           GoodJob::Execution.advisory_lock_key(key, function: "pg_advisory_lock") do
             enqueue_concurrency = if enqueue_limit
                                     GoodJob::Execution.where(concurrency_key: key).unfinished.advisory_unlocked.count
@@ -117,11 +119,10 @@ def _good_job_concurrency_key
         key = self.class.good_job_concurrency_config[:key]
         return if key.blank?
 
-        if key.respond_to? :call
-          instance_exec(&key)
-        else
-          key
-        end
+        key = key.respond_to?(:call) ? instance_exec(&key) : key
+        raise TypeError, "Concurrency key must be a String; was a #{key.class}" unless VALID_TYPES.any? { |type| key.is_a?(type) }
+
+        key
       end
     end
   end

spec/lib/good_job/active_job_extensions/concurrency_spec.rb

@@ -137,5 +137,28 @@ def perform
         expect(retried_execution.concurrency_key).to eq first_execution.concurrency_key
       end
     end
+
+    describe '#perform_later' do
+      before do
+        stub_const 'TestJob', (Class.new(ActiveJob::Base) do
+          include GoodJob::ActiveJobExtensions::Concurrency
+
+          good_job_control_concurrency_with(
+            total_limit: 1,
+            key: -> { arguments.first }
+          )
+
+          def perform(arg)
+          end
+        end)
+      end
+
+      it 'raises an error for non-serializable types' do
+        expect { TestJob.perform_later({ key: "value" }) }.to raise_error(TypeError, "Concurrency key must be a String; was a Hash")
+        expect { TestJob.perform_later({ key: "value" }.with_indifferent_access) }.to raise_error(TypeError)
+        expect { TestJob.perform_later(["key"]) }.to raise_error(TypeError)
+        expect { TestJob.perform_later(TestJob) }.to raise_error(TypeError)
+      end
+    end
   end
 end