Merge pull request #710 from collectiveidea/separate-yaml

Separate yaml

Gemfile

@@ -11,6 +11,10 @@ platforms :jruby do
   gem 'activerecord-jdbcsqlite3-adapter'
 end
 
+platforms :rbx do
+  gem 'psych'
+end
+
 group :test do
   if ENV['RAILS_VERSION'] == 'edge'
     gem 'activerecord', :github => 'rails/rails'

lib/delayed/backend/base.rb

@@ -78,13 +78,7 @@ def payload_object=(object)
       end
 
       def payload_object
-        if YAML.respond_to?(:unsafe_load)
-          # See https://github.com/dtao/safe_yaml
-          # When the method is there, we need to load our YAML like this...
-          @payload_object ||= YAML.load(handler, :safe => false)
-        else
-          @payload_object ||= YAML.load(handler)
-        end
+        @payload_object ||= YAML.load_dj(handler)
       rescue TypeError, LoadError, NameError, ArgumentError => e
         raise DeserializationError, "Job failed to load: #{e.message}. Handler: #{handler.inspect}"
       end

lib/delayed/backend/shared_spec.rb

@@ -175,7 +175,7 @@ def create_job(opts = {})
 
     it 'raises a DeserializationError when the YAML.load raises argument error' do
       job = described_class.new :handler => '--- !ruby/struct:GoingToRaiseArgError {}'
-      expect(YAML).to receive(:load).and_raise(ArgumentError)
+      expect(YAML).to receive(:load_dj).and_raise(ArgumentError)
       expect { job.payload_object }.to raise_error(Delayed::DeserializationError)
     end
   end

lib/delayed/psych_ext.rb

@@ -1,22 +1,3 @@
-if defined?(ActiveRecord)
-  ActiveRecord::Base.class_eval do
-    # rubocop:disable BlockNesting
-    if instance_methods.include?(:encode_with)
-      def encode_with_override(coder)
-        encode_with_without_override(coder)
-        coder.tag = "!ruby/ActiveRecord:#{self.class.name}" if coder.respond_to?(:tag=)
-      end
-      alias_method :encode_with_without_override, :encode_with
-      alias_method :encode_with, :encode_with_override
-    else
-      def encode_with(coder)
-        coder['attributes'] = attributes
-        coder.tag = "!ruby/ActiveRecord:#{self.class.name}" if coder.respond_to?(:tag=)
-      end
-    end
-  end
-end
-
 module Delayed
   class PerformableMethod
     # serialize to YAML
@@ -31,12 +12,36 @@ def encode_with(coder)
 end
 
 module Psych
-  module Visitors
-    class ToRuby
-      def visit_Psych_Nodes_Mapping_with_class(object) # rubocop:disable PerceivedComplexity, CyclomaticComplexity, MethodName
+  def self.load_dj(yaml)
+    result = parse(yaml)
+    result ? Delayed::PsychExt::ToRuby.create.accept(result) : result
+  end
+end
+
+module Delayed
+  module PsychExt
+    class ToRuby < Psych::Visitors::ToRuby
+      unless respond_to?(:create)
+        def self.create
+          new
+        end
+      end
+
+      def visit_Psych_Nodes_Mapping(object) # rubocop:disable CyclomaticComplexity, MethodName, PerceivedComplexity
         return revive(Psych.load_tags[object.tag], object) if Psych.load_tags[object.tag]
 
         case object.tag
+        when /^!ruby\/object/
+          result = super
+          if defined?(ActiveRecord::Base) && result.is_a?(ActiveRecord::Base)
+            begin
+              result.class.find(result[result.class.primary_key])
+            rescue ActiveRecord::RecordNotFound => error # rubocop:disable BlockNesting
+              raise Delayed::DeserializationError, "ActiveRecord::RecordNotFound, class: #{klass}, primary key: #{id} (#{error.message})"
+            end
+          else
+            result
+          end
         when /^!ruby\/ActiveRecord:(.+)$/
           klass = resolve_class(Regexp.last_match[1])
           payload = Hash[*object.children.map { |c| accept c }]
@@ -67,17 +72,16 @@ def visit_Psych_Nodes_Mapping_with_class(object) # rubocop:disable PerceivedComp
             raise Delayed::DeserializationError, "DataMapper::ObjectNotFoundError, class: #{klass} (#{error.message})"
           end
         else
-          visit_Psych_Nodes_Mapping_without_class(object)
+          super
         end
       end
-      alias_method_chain :visit_Psych_Nodes_Mapping, :class
 
-      def resolve_class_with_constantize(klass_name)
+      def resolve_class(klass_name)
+        return nil if !klass_name || klass_name.empty?
         klass_name.constantize
       rescue
-        resolve_class_without_constantize(klass_name)
+        super
       end
-      alias_method_chain :resolve_class, :constantize
     end
   end
 end

lib/delayed/syck_ext.rb

@@ -32,3 +32,11 @@ def self.yaml_tag_read_class(name)
     "Struct::#{ name }"
   end
 end
+
+module YAML
+  def load_dj(yaml)
+    # See https://github.com/dtao/safe_yaml
+    # When the method is there, we need to load our YAML like this...
+    respond_to?(:unsafe_load) ? load(yaml, :safe => false) : load(yaml)
+  end
+end

spec/helper.rb

@@ -20,9 +20,20 @@
 require 'delayed_job'
 require 'delayed/backend/shared_spec'
 
-Delayed::Worker.logger = Logger.new('/tmp/dj.log')
+if ENV['DEBUG_LOGS']
+  Delayed::Worker.logger = Logger.new(STDOUT)
+else
+  require 'tempfile'
+
+  tf = Tempfile.new('dj.log')
+  Delayed::Worker.logger = Logger.new(tf.path)
+  tf.unlink
+end
 ENV['RAILS_ENV'] = 'test'
 
+# Trigger AR to initialize
+ActiveRecord::Base # rubocop:disable Void
+
 module Rails
   def self.root
     '.'

spec/yaml_ext_spec.rb

@@ -4,32 +4,45 @@
   it 'autoloads classes' do
     expect do
       yaml = "--- !ruby/class Autoloaded::Clazz\n"
-      expect(YAML.load(yaml)).to eq(Autoloaded::Clazz)
+      expect(load_with_delayed_visitor(yaml)).to eq(Autoloaded::Clazz)
     end.not_to raise_error
   end
 
   it 'autoloads the class of a struct' do
     expect do
       yaml = "--- !ruby/class Autoloaded::Struct\n"
-      expect(YAML.load(yaml)).to eq(Autoloaded::Struct)
+      expect(load_with_delayed_visitor(yaml)).to eq(Autoloaded::Struct)
     end.not_to raise_error
   end
 
   it 'autoloads the class for the instance of a struct' do
     expect do
       yaml = '--- !ruby/struct:Autoloaded::InstanceStruct {}'
-      expect(YAML.load(yaml).class).to eq(Autoloaded::InstanceStruct)
+      expect(load_with_delayed_visitor(yaml).class).to eq(Autoloaded::InstanceStruct)
+    end.not_to raise_error
+  end
+
+  it 'autoloads the class of an anonymous struct' do
+    expect do
+      yaml = "--- !ruby/struct\nn: 1\n"
+      object = YAML.load(yaml)
+      expect(object).to be_kind_of(Struct)
+      expect(object.n).to eq(1)
     end.not_to raise_error
   end
 
   it 'autoloads the class for the instance' do
     expect do
       yaml = "--- !ruby/object:Autoloaded::InstanceClazz {}\n"
-      expect(YAML.load(yaml).class).to eq(Autoloaded::InstanceClazz)
+      expect(load_with_delayed_visitor(yaml).class).to eq(Autoloaded::InstanceClazz)
     end.not_to raise_error
   end
 
   it 'does not throw an uninitialized constant Syck::Syck when using YAML.load with poorly formed yaml' do
     expect { YAML.load(YAML.dump('foo: *bar')) }.not_to raise_error
   end
+
+  def load_with_delayed_visitor(yaml)
+    YAML.load_dj(yaml)
+  end
 end