From da0aa6ca47d97c184a5c02391f1277f7ef265427 Mon Sep 17 00:00:00 2001 From: Brenno Costa Date: Fri, 20 Sep 2013 15:49:07 -0300 Subject: [PATCH 1/6] Add custom Psych Visitor's behavior only for Delayed, not glabally --- lib/delayed/backend/base.rb | 2 +- lib/delayed/psych_ext.rb | 20 ++++++++++++++++++-- spec/yaml_ext_spec.rb | 12 ++++++++---- 3 files changed, 27 insertions(+), 7 deletions(-) diff --git a/lib/delayed/backend/base.rb b/lib/delayed/backend/base.rb index 3723570c0..b01589764 100644 --- a/lib/delayed/backend/base.rb +++ b/lib/delayed/backend/base.rb @@ -83,7 +83,7 @@ def payload_object # 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) + @payload_object ||= YAML.load(handler, nil, Delayed::PsychExt::ToRuby) end rescue TypeError, LoadError, NameError, ArgumentError => e raise DeserializationError, "Job failed to load: #{e.message}. Handler: #{handler.inspect}" diff --git a/lib/delayed/psych_ext.rb b/lib/delayed/psych_ext.rb index 04b84aa14..23a2af48f 100644 --- a/lib/delayed/psych_ext.rb +++ b/lib/delayed/psych_ext.rb @@ -31,8 +31,24 @@ def encode_with(coder) end module Psych - module Visitors - class ToRuby + def self.load yaml, filename = nil, visitor = nil + result = parse(yaml, filename) + result ? result.to_ruby(visitor) : result + end + + module Nodes + class Node + def to_ruby(visitor) + visitor ||= Visitors::ToRuby + visitor.new.accept(self) + end + end + end +end + +module Delayed + module PsychExt + class ToRuby < Psych::Visitors::ToRuby def visit_Psych_Nodes_Mapping_with_class(object) # rubocop:disable PerceivedComplexity, CyclomaticComplexity, MethodName return revive(Psych.load_tags[object.tag], object) if Psych.load_tags[object.tag] diff --git a/spec/yaml_ext_spec.rb b/spec/yaml_ext_spec.rb index d2ff5a800..ece82c4e5 100644 --- a/spec/yaml_ext_spec.rb +++ b/spec/yaml_ext_spec.rb @@ -4,32 +4,36 @@ 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 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(yaml, nil, Delayed::PsychExt::ToRuby) + end end From 78dac017dd62d9fccef6733a5f26446cc1e30593 Mon Sep 17 00:00:00 2001 From: Brenno Costa Date: Tue, 24 Sep 2013 16:48:53 -0300 Subject: [PATCH 2/6] Fix support for ruby 1.9.2 (Psych < 1.3.0) --- lib/delayed/psych_ext.rb | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/lib/delayed/psych_ext.rb b/lib/delayed/psych_ext.rb index 23a2af48f..9d62a2c2e 100644 --- a/lib/delayed/psych_ext.rb +++ b/lib/delayed/psych_ext.rb @@ -31,9 +31,16 @@ def encode_with(coder) end module Psych - def self.load yaml, filename = nil, visitor = nil - result = parse(yaml, filename) - result ? result.to_ruby(visitor) : result + if VERSION.to_f < 1.3 + def self.load yaml, filename = nil, visitor = nil + result = parse(yaml) + result ? result.to_ruby(visitor) : result + end + else + def self.load yaml, filename = nil, visitor = nil + result = parse(yaml, filename) + result ? result.to_ruby(visitor) : result + end end module Nodes From 9babdf35792a29f717f2f172ba6f6ced131aa559 Mon Sep 17 00:00:00 2001 From: Brenno Costa Date: Mon, 13 Jan 2014 12:01:25 -0500 Subject: [PATCH 3/6] Add support to Ruby 2.1 --- lib/delayed/psych_ext.rb | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/lib/delayed/psych_ext.rb b/lib/delayed/psych_ext.rb index 9d62a2c2e..bde93a7a4 100644 --- a/lib/delayed/psych_ext.rb +++ b/lib/delayed/psych_ext.rb @@ -45,9 +45,16 @@ def self.load yaml, filename = nil, visitor = nil module Nodes class Node - def to_ruby(visitor) - visitor ||= Visitors::ToRuby - visitor.new.accept(self) + if Gem::Version.new(VERSION) >= Gem::Version.new('2.0.2') + def to_ruby(visitor) + visitor ||= Visitors::ToRuby + visitor.create.accept(self) + end + else + def to_ruby(visitor) + visitor ||= Visitors::ToRuby + visitor.new.accept(self) + end end end end From f99d7b3af16c48aae8a45caa7f71b975ae687fe9 Mon Sep 17 00:00:00 2001 From: David Genord II Date: Tue, 23 Sep 2014 15:59:10 -0400 Subject: [PATCH 4/6] Override even fewer YAML methods --- lib/delayed/backend/base.rb | 8 +----- lib/delayed/backend/shared_spec.rb | 2 +- lib/delayed/psych_ext.rb | 45 +++++++++--------------------- lib/delayed/syck_ext.rb | 8 ++++++ spec/yaml_ext_spec.rb | 2 +- 5 files changed, 24 insertions(+), 41 deletions(-) diff --git a/lib/delayed/backend/base.rb b/lib/delayed/backend/base.rb index b01589764..56fe47fa5 100644 --- a/lib/delayed/backend/base.rb +++ b/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, nil, Delayed::PsychExt::ToRuby) - 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 diff --git a/lib/delayed/backend/shared_spec.rb b/lib/delayed/backend/shared_spec.rb index 2a5ce4340..3fb2691bf 100644 --- a/lib/delayed/backend/shared_spec.rb +++ b/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 diff --git a/lib/delayed/psych_ext.rb b/lib/delayed/psych_ext.rb index bde93a7a4..eebd1dbdf 100644 --- a/lib/delayed/psych_ext.rb +++ b/lib/delayed/psych_ext.rb @@ -31,39 +31,22 @@ def encode_with(coder) end module Psych - if VERSION.to_f < 1.3 - def self.load yaml, filename = nil, visitor = nil - result = parse(yaml) - result ? result.to_ruby(visitor) : result - end - else - def self.load yaml, filename = nil, visitor = nil - result = parse(yaml, filename) - result ? result.to_ruby(visitor) : result - end - end - - module Nodes - class Node - if Gem::Version.new(VERSION) >= Gem::Version.new('2.0.2') - def to_ruby(visitor) - visitor ||= Visitors::ToRuby - visitor.create.accept(self) - end - else - def to_ruby(visitor) - visitor ||= Visitors::ToRuby - visitor.new.accept(self) - end - end - end + 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 - def visit_Psych_Nodes_Mapping_with_class(object) # rubocop:disable PerceivedComplexity, CyclomaticComplexity, MethodName + if !respond_to?(:create) + def self.create + new + end + end + + def visit_Psych_Nodes_Mapping(object) # rubocop:disable PerceivedComplexity, CyclomaticComplexity, MethodName return revive(Psych.load_tags[object.tag], object) if Psych.load_tags[object.tag] case object.tag @@ -97,17 +80,15 @@ 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) klass_name.constantize rescue - resolve_class_without_constantize(klass_name) + super end - alias_method_chain :resolve_class, :constantize end end end diff --git a/lib/delayed/syck_ext.rb b/lib/delayed/syck_ext.rb index 2a280fc77..fff03a15a 100644 --- a/lib/delayed/syck_ext.rb +++ b/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 diff --git a/spec/yaml_ext_spec.rb b/spec/yaml_ext_spec.rb index ece82c4e5..b46892d18 100644 --- a/spec/yaml_ext_spec.rb +++ b/spec/yaml_ext_spec.rb @@ -34,6 +34,6 @@ end def load_with_delayed_visitor(yaml) - YAML.load(yaml, nil, Delayed::PsychExt::ToRuby) + YAML.load_dj(yaml) end end From 782d7c4879f7dcd733a3c67680d2910a76d247f2 Mon Sep 17 00:00:00 2001 From: David Genord II Date: Tue, 23 Sep 2014 16:55:26 -0400 Subject: [PATCH 5/6] Don't override AR yaml generation --- lib/delayed/psych_ext.rb | 30 +++++++++++------------------- spec/helper.rb | 3 +++ 2 files changed, 14 insertions(+), 19 deletions(-) diff --git a/lib/delayed/psych_ext.rb b/lib/delayed/psych_ext.rb index eebd1dbdf..49a789064 100644 --- a/lib/delayed/psych_ext.rb +++ b/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 @@ -50,6 +31,17 @@ def visit_Psych_Nodes_Mapping(object) # rubocop:disable PerceivedComplexity, Cyc 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 + 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 }] diff --git a/spec/helper.rb b/spec/helper.rb index c040252df..29602676f 100644 --- a/spec/helper.rb +++ b/spec/helper.rb @@ -23,6 +23,9 @@ Delayed::Worker.logger = Logger.new('/tmp/dj.log') ENV['RAILS_ENV'] = 'test' +# Trigger AR to initialize +ActiveRecord::Base + module Rails def self.root '.' From ebf6789c2e4f6f935caa5a9885d0f454b2079252 Mon Sep 17 00:00:00 2001 From: David Genord II Date: Tue, 23 Sep 2014 17:08:07 -0400 Subject: [PATCH 6/6] Use a throw away log file --- spec/helper.rb | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/spec/helper.rb b/spec/helper.rb index 29602676f..7987ac0b7 100644 --- a/spec/helper.rb +++ b/spec/helper.rb @@ -20,7 +20,15 @@ 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