Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rails SQL sanitizing and Ruleby #19

Closed
cwoodcox opened this issue Nov 2, 2011 · 2 comments · Fixed by #20
Closed

Rails SQL sanitizing and Ruleby #19

cwoodcox opened this issue Nov 2, 2011 · 2 comments · Fixed by #20

Comments

@cwoodcox
Copy link
Contributor

cwoodcox commented Nov 2, 2011

I ran into an issue when using Rails with conditional associations and Ruleby. Some background, I have two models, a Store and a Batch. Store has many batches, and has one current batch, which is simply a conditional association where the batch has a state of "open."

Here's my stack trace when I try to access the current_batch association.

ruby-1.9.2-p290 :002 > Store.first.current_batch
SyntaxError: (eval):1: syntax error, unexpected tSTRING_BEG
Proc.new { || "batches"."workflow_state" = 'open' }
                         ^
(eval):1: syntax error, unexpected '=', expecting '}'
Proc.new { || "batches"."workflow_state" = 'open' }
                                          ^
  from gems/ruleby-0.8/lib/ruleby.rb:55:in `eval'
  from gems/ruleby-0.8/lib/ruleby.rb:55:in `to_proc'
  from gems/activerecord-3.0.9/lib/active_record/base.rb:1753:in `interpolate_sanitized_sql'
  from gems/activerecord-3.0.9/lib/active_record/associations/association_proxy.rb:165:in `interpolate_sanitized_sql'
  from gems/activerecord-3.0.9/lib/active_record/associations/association_proxy.rb:105:in `conditions'
  from gems/activerecord-3.0.9/lib/active_record/associations/has_one_association.rb:102:in `construct_sql'
  from gems/activerecord-3.0.9/lib/active_record/associations/has_one_association.rb:7:in `initialize'
  from gems/activerecord-3.0.9/lib/active_record/associations.rb:1441:in `new'
  from gems/activerecord-3.0.9/lib/active_record/associations.rb:1441:in `block in association_accessor_methods'
  from (irb):2
  from gems/railties-3.0.9/lib/rails/commands/console.rb:44:in `start'
  from gems/railties-3.0.9/lib/rails/commands/console.rb:8:in `start'
  from gems/railties-3.0.9/lib/rails/commands.rb:23:in `<top (required)>'
  from script/rails:6:in `require'
  from script/rails:6:in `<main>'

Rails does some magic on the arguments it gets passed when defining a conditional association, here's the code I believe is causing issues with Ruleby.

def interpolate_sanitized_sql(sanitized, record = nil, sanitize_klass = self.class)
  if sanitized =~ /\#\{.*\}/
    ActiveSupport::Deprecation.warn(
      'String-based interpolation of association conditions is deprecated. Please use a ' \
      'proc instead. So, for example, has_many :older_friends, :conditions => \'age > #{age}\' ' \
      'should be changed to has_many :older_friends, :conditions => proc { "age > #{age}" }.'
    )
    instance_eval("%@#{sanitized.gsub('@', '\@')}@", __FILE__, __LINE__)
  elsif sanitized.respond_to?(:to_proc)
    sanitize_klass.send(:sanitize_sql, instance_exec(record, &sanitized))
  else
    sanitized
  end
end

It asks the object being passed in if it responds to #to_proc. Under normal circumstances, String would not respond to #to_proc so there would be nothing happening here and it would just return the generated and sanitized SQL. But, since Ruleby provides a String#to_proc method, Rails runs the proc and Ruleby just gets really confused at the String it's working with.

I'm still working on a workaround/patch, but I wanted to see if anyone here had any ideas. I'll update this with my findings.

@jkutner
Copy link
Contributor

jkutner commented Nov 11, 2011

I think we can just get rid of the String#to_proc definition (in lib/ruleby/ruleby.rb). It was just used for an experimental DSL.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants
@jkutner @cwoodcox and others