Merge pull request collectiveidea#307 from trek/RaiseOnJobsForUnsaved…

…Objects

raise ArgumentError if attempting to enqueue a performable method on an object that hasn't been persisted yet

Merging because @trek bribed me.

lib/delayed/backend/shared_spec.rb

@@ -414,6 +414,14 @@ def create_job(opts = {})
       story.update_attributes :text => 'goodbye'
       job.reload.payload_object.object.text.should == 'goodbye'
     end
+
+    it "should raise error ArgumentError the record is not persisted" do
+      story = Story.new(:text => 'hello')
+      lambda {
+        story.delay.tell 
+      }.should raise_error(ArgumentError, "Jobs cannot be created for records before they've been persisted")
+
+    end
 
     it "should raise deserialization error for destroyed records" do
       story = Story.create(:text => 'hello')

lib/delayed/performable_method.rb

@@ -8,7 +8,11 @@ class PerformableMethod
 
     def initialize(object, method_name, args)
       raise NoMethodError, "undefined method `#{method_name}' for #{object.inspect}" unless object.respond_to?(method_name, true)
-
+
+      if object.kind_of?(ActiveRecord::Base)
+        raise(ArgumentError, 'Jobs cannot be created for records before they\'ve been persisted') if object.attributes[object.class.primary_key].nil?
+      end
+
       self.object       = object
       self.args         = args
       self.method_name  = method_name.to_sym

spec/message_sending_spec.rb

@@ -13,7 +13,7 @@ def tell!(arg);end
     end
 
     it "should create a PerformableMethod" do
-      story = Story.new
+      story = Story.create
       lambda {
         job = story.tell!(1)
         job.payload_object.class.should == Delayed::PerformableMethod

spec/performable_method_spec.rb

@@ -42,7 +42,7 @@ def private_method
   describe "hooks" do
     %w(enqueue before after success).each do |hook|
       it "should delegate #{hook} hook to object" do
-        story = Story.new
+        story = Story.create
         story.should_receive(hook).with(an_instance_of(Delayed::Job))
         story.delay.tell.invoke_job
       end
@@ -51,22 +51,22 @@ def private_method
     %w(before after success).each do |hook|
       it "should delegate #{hook} hook to object when delay_jobs = false" do
         Delayed::Worker.delay_jobs = false
-        story = Story.new
+        story = Story.create
         story.should_receive(hook).with(an_instance_of(Delayed::Job))
         story.delay.tell
       end
     end
 
     it "should delegate error hook to object" do
-      story = Story.new
+      story = Story.create
       story.should_receive(:error).with(an_instance_of(Delayed::Job), an_instance_of(RuntimeError))
       story.should_receive(:tell).and_raise(RuntimeError)
       lambda { story.delay.tell.invoke_job }.should raise_error
     end
 
     it "should delegate error hook to object when delay_jobs = false" do
       Delayed::Worker.delay_jobs = false
-      story = Story.new
+      story = Story.create
       story.should_receive(:error).with(an_instance_of(Delayed::Job), an_instance_of(RuntimeError))
       story.should_receive(:tell).and_raise(RuntimeError)
       lambda { story.delay.tell }.should raise_error