Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Add :encode_with to sql_literal for proper YAML serialization #216

Merged
merged 1 commit into from

4 participants

@iantropov

This patch fixes rails/rails#5303

@robin850

Thanks for the fix! We could add a test to ensure that the serialization won't fail, what do you think ?

@iantropov

@robin850 Thanks for attention! I've added a test.

@laurocaetano

@iantropov Thanks for the fix. I've tried your branch locally and it seems to fix the issue. We could also add a test on rails codebase to ensure that it's working as expected. What do you think?

test/nodes/test_sql_literal.rb
@@ -56,6 +57,12 @@ module Nodes
@visitor.accept(node).must_be_like %{ (foo = 1 AND foo = 2) }
end
end
+
+ describe 'serialization' do
+ it 'serializes into YAML' do
+ SqlLiteral.new('*').to_yaml

I think we're missing an assertion here, right?

No, without fix, this code produces a error.

@rafaelfranca Owner

But could we add some assertion?

What do you offer ? As you say before - assert_nothing_raised is deprecated. May be assert_not_nil ?

@rafaelfranca Owner
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@iantropov

@laurocaetano Thanks for comments! I've added test for this case in Rails.

@iantropov

@rafaelfranca Please take a look, I've added output asserts for YAML serialization

@rafaelfranca
Owner

The build is broken. Could you take a lool?

@iantropov

It is strange, it seems that Travis starts tests without applying a code patches - my test fails in the fixed place.

@robin850

The failure seems to be only on 1.9.2. Do we still support 1.9.2 on Arel Rafael please? It looks like it's not needed anymore.

@rafaelfranca rafaelfranca merged commit 1dfe626 into rails:master

1 check failed

Details default The Travis CI build failed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Nov 9, 2013
  1. @iantropov
This page is out of date. Refresh to see the latest.
Showing with 12 additions and 0 deletions.
  1. +4 −0 lib/arel/nodes/sql_literal.rb
  2. +8 −0 test/nodes/test_sql_literal.rb
View
4 lib/arel/nodes/sql_literal.rb
@@ -5,6 +5,10 @@ class SqlLiteral < String
include Arel::Predications
include Arel::AliasPredication
include Arel::OrderPredications
+
+ def encode_with(coder)
+ coder.scalar = self.to_s
+ end
end
class BindParam < SqlLiteral
View
8 test/nodes/test_sql_literal.rb
@@ -1,4 +1,5 @@
require 'helper'
+require 'yaml'
module Arel
module Nodes
@@ -56,6 +57,13 @@ module Nodes
@visitor.accept(node).must_be_like %{ (foo = 1 AND foo = 2) }
end
end
+
+ describe 'serialization' do
+ it 'serializes into YAML' do
+ yaml_literal = SqlLiteral.new('foo').to_yaml
+ assert_equal('foo', YAML.load(yaml_literal))
+ end
+ end
end
end
end
Something went wrong with that request. Please try again.