Rails core - Nov 2008 - #to_json incompatibilities between ActiveSupport & JSON gems

Hi all

Just spotted a issue when using the json & activesupport gems in the same
project. Below is a simple irb session showing the effect:

irb(main):001:0> require ''json''
=> true
irb(main):002:0> Time.now.to_json( nil, 1 )
=> "\"Wed Nov 26 15:56:02 +0200 2008\""
irb(main):003:0> require ''activesupport''
=> true
irb(main):004:0> Time.now.to_json( nil, 1 )
ArgumentError: wrong number of arguments (2 for 1)
        from (irb):4:in `to_json''
        from (irb):4

Searching through the json_pure-1.1.3 code it seems the #to_json signature
is "def to_json(*)", and in activesupport''s Time
it''s def to_json(options nil). Looking through the other activesupport
extensions for JSON it seems
to be the same case.

Any suggestions? Any objections to me diving in and fixing it?

Best

-- 
Kenneth Kalmer
kenneth.kalmer@gmail.com
http://opensourcery.co.za

--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Core" group.
To post to this group, send email to rubyonrails-core@googlegroups.com
To unsubscribe from this group, send email to
rubyonrails-core+unsubscribe@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/rubyonrails-core?hl=en
-~----------~----~----~----~------~----~------~--~---

> Hi all
>
> Just spotted a issue when using the json & activesupport gems in the
same
> project. Below is a simple irb session showing the effect:
>
> irb(main):001:0> require ''json''
> => true
> irb(main):002:0> Time.now.to_json( nil, 1 )
> => "\"Wed Nov 26 15:56:02 +0200 2008\""
> irb(main):003:0> require ''activesupport''
> => true
> irb(main):004:0> Time.now.to_json( nil, 1 )
> ArgumentError: wrong number of arguments (2 for 1)
>         from (irb):4:in `to_json''
>         from (irb):4
>
> Searching through the json_pure-1.1.3 code it seems the #to_json signature
> is "def to_json(*)", and in activesupport''s Time
it''s def to_json(options > nil). Looking through the other
activesupport extensions for JSON it seems
> to be the same case.
>
> Any suggestions? Any objections to me diving in and fixing it?
What would the fix look like?   what are nil and 1 in the context above?

-- 
Cheers

Koz

--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Core" group.
To post to this group, send email to rubyonrails-core@googlegroups.com
To unsubscribe from this group, send email to
rubyonrails-core+unsubscribe@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/rubyonrails-core?hl=en
-~----------~----~----~----~------~----~------~--~---

On Wed, Nov 26, 2008 at 4:15 PM, Michael Koziarski
<michael@koziarski.com>wrote:
>
> > Hi all
> >
> > Just spotted a issue when using the json & activesupport gems in
the same
> > project. Below is a simple irb session showing the effect:
> >
> > irb(main):001:0> require ''json''
> > => true
> > irb(main):002:0> Time.now.to_json( nil, 1 )
> > => "\"Wed Nov 26 15:56:02 +0200 2008\""
> > irb(main):003:0> require ''activesupport''
> > => true
> > irb(main):004:0> Time.now.to_json( nil, 1 )
> > ArgumentError: wrong number of arguments (2 for 1)
> >         from (irb):4:in `to_json''
> >         from (irb):4
> >
> > Searching through the json_pure-1.1.3 code it seems the #to_json
> signature
> > is "def to_json(*)", and in activesupport''s Time
it''s def to_json(options
> > > nil). Looking through the other activesupport extensions for JSON
it
> seems
> > to be the same case.
> >
> > Any suggestions? Any objections to me diving in and fixing it?
>
> What would the fix look like?   what are nil and 1 in the context above?
>
On the nil & 1, looking at line 237 in
(/usr/lib/ruby/gems/1.8/gems/)json_pure-1.1.3/lib/json/pure/generator.rb
you''ll see this:

  value.to_json(state, depth + 1)

state can either be nil (as in my tests), or something unknown to me at this
stage. And depth it seems is 0, so I tested with 0 + 1. This is inline with
the json gems take on "def to_json(*a)".

As for the fix, just changing the argument lists to use a bang parameter and
then appropriate extracting what activesupport expects from the argument
array...

-- 
Kenneth Kalmer
kenneth.kalmer@gmail.com
http://opensourcery.co.za

--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Core" group.
To post to this group, send email to rubyonrails-core@googlegroups.com
To unsubscribe from this group, send email to
rubyonrails-core+unsubscribe@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/rubyonrails-core?hl=en
-~----------~----~----~----~------~----~------~--~---

On Wed, Nov 26, 2008 at 4:24 PM, Kenneth Kalmer
<kenneth.kalmer@gmail.com>wrote:
> On Wed, Nov 26, 2008 at 4:15 PM, Michael Koziarski
<michael@koziarski.com>wrote:
>
>>
>> > Hi all
>> >
>> > Just spotted a issue when using the json & activesupport gems
in the
>> same
>> > project. Below is a simple irb session showing the effect:
>> >
>> > irb(main):001:0> require ''json''
>> > => true
>> > irb(main):002:0> Time.now.to_json( nil, 1 )
>> > => "\"Wed Nov 26 15:56:02 +0200 2008\""
>> > irb(main):003:0> require ''activesupport''
>> > => true
>> > irb(main):004:0> Time.now.to_json( nil, 1 )
>> > ArgumentError: wrong number of arguments (2 for 1)
>> >         from (irb):4:in `to_json''
>> >         from (irb):4
>> >
>> > Searching through the json_pure-1.1.3 code it seems the #to_json
>> signature
>> > is "def to_json(*)", and in activesupport''s
Time it''s def
>> to_json(options >> > nil). Looking through the other
activesupport extensions for JSON it
>> seems
>> > to be the same case.
>> >
>> > Any suggestions? Any objections to me diving in and fixing it?
>>
>> What would the fix look like?   what are nil and 1 in the context
above?
>>
>
> On the nil & 1, looking at line 237 in
> (/usr/lib/ruby/gems/1.8/gems/)json_pure-1.1.3/lib/json/pure/generator.rb
> you''ll see this:
>
>   value.to_json(state, depth + 1)
>
> state can either be nil (as in my tests), or something unknown to me at
> this stage. And depth it seems is 0, so I tested with 0 + 1. This is inline
> with the json gems take on "def to_json(*a)".
>
> As for the fix, just changing the argument lists to use a bang parameter
> and then appropriate extracting what activesupport expects from the
argument
> array...

Any thoughts on this? I added a simple fix to ruote (
http://tinyurl.com/6boobf) for Time#to_json and it works perfectly. I just
can''t help but think more and more people would use ActiveSupport
outside of
Rails, and especially in an environment where the JSON gem is present...

Best


-- 
Kenneth Kalmer
kenneth.kalmer@gmail.com
http://opensourcery.co.za

--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Core" group.
To post to this group, send email to rubyonrails-core@googlegroups.com
To unsubscribe from this group, send email to
rubyonrails-core+unsubscribe@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/rubyonrails-core?hl=en
-~----------~----~----~----~------~----~------~--~---

> Any thoughts on this? I added a simple fix to ruote
> (http://tinyurl.com/6boobf) for Time#to_json and it works perfectly. I just
> can''t help but think more and more people would use ActiveSupport
outside of
> Rails, and especially in an environment where the JSON gem is present...
I''m a little hesitant because those arguments to to_json are just
ignored.  So you get ''compatibility'' in the sense that you
don''t get
an ArgumentError, but it won''t do what you think it will.

Silent misbehaviour here seems *worse* than failing fast?

-- 
Cheers

Koz

--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Core" group.
To post to this group, send email to rubyonrails-core@googlegroups.com
To unsubscribe from this group, send email to
rubyonrails-core+unsubscribe@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/rubyonrails-core?hl=en
-~----------~----~----~----~------~----~------~--~---

On Sat, Nov 29, 2008 at 2:30 PM, Michael Koziarski
<michael@koziarski.com>wrote:
>
> > Any thoughts on this? I added a simple fix to ruote
> > (http://tinyurl.com/6boobf) for Time#to_json and it works perfectly. I
> just
> > can''t help but think more and more people would use
ActiveSupport outside
> of
> > Rails, and especially in an environment where the JSON gem is
present...
>
> I''m a little hesitant because those arguments to to_json are just
> ignored.  So you get ''compatibility'' in the sense that
you don''t get
> an ArgumentError, but it won''t do what you think it will.
>
> Silent misbehaviour here seems *worse* than failing fast?

Agreed. For my example above it works perfectly well, since I''m not
really
concerned with the Time instances. They''re just part of a JSON payload,
and
I control only part of the payload (where time doesn''t matter).

How about this? (Don''t kill me if I''ve stepped out of line
here). When the
JSON gem(s) are present and loaded prior to ActiveSupport, ActiveSupport
backs off from re-defining all the #to_json methods and delegate the
decoding work to JSON again. Another obvious thing would be to load
ActiveSupport before JSON and have JSON simply overwrite the work of
ActiveSupport. Another option might be to strip down json_pure and bundle it
into ActiveSupport, seeing my local copy is roughly 756KB...

But the co-existance of the JSON and ActiveSupport gems in the same projects
cannot be ignored. The ruote project, and other fast moving targets like
CouchDB will make this an increasingly bigger issue in the future.

I honestly don''t know what the impact of any of these would be.
I''ll keep on
throwing ideas at the list, even if it is all the wrong ones, just to keep
the thoughts churning and get us closer to a solution.

Anycase, thanks for your patience.

-- 
Kenneth Kalmer
kenneth.kalmer@gmail.com
http://opensourcery.co.za

--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Core" group.
To post to this group, send email to rubyonrails-core@googlegroups.com
To unsubscribe from this group, send email to
rubyonrails-core+unsubscribe@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/rubyonrails-core?hl=en
-~----------~----~----~----~------~----~------~--~---

> Agreed. For my example above it works perfectly well, since I''m
not really
> concerned with the Time instances. They''re just part of a JSON
payload, and
> I control only part of the payload (where time doesn''t matter).
OK, so from my perspective the ideal solution would be that you can
just tell your users to require the json gem *after* activesupport,
and have everything ''just work'' because it nukes our methods. 
If
that''s not the case, lets fix it.
> How about this? (Don''t kill me if I''ve stepped out of
line here). When the
> JSON gem(s) are present and loaded prior to ActiveSupport, ActiveSupport
> backs off from re-defining all the #to_json methods and delegate the
> decoding work to JSON again. Another obvious thing would be to load
> ActiveSupport before JSON and have JSON simply overwrite the work of
> ActiveSupport. Another option might be to strip down json_pure and bundle
it
> into ActiveSupport, seeing my local copy is roughly 756KB...
Our current json implementation works fine for my projects, if there
are bugs we should fix them.  But ''just rewrite it'' is
generally a
great way to break stuff and piss people off.
> But the co-existance of the JSON and ActiveSupport gems in the same
projects
> cannot be ignored. The ruote project, and other fast moving targets like
> CouchDB will make this an increasingly bigger issue in the future.
>
> I honestly don''t know what the impact of any of these would be.
I''ll keep on
> throwing ideas at the list, even if it is all the wrong ones, just to keep
> the thoughts churning and get us closer to a solution.
>
> Anycase, thanks for your patience.
>
> --
> Kenneth Kalmer
> kenneth.kalmer@gmail.com
> http://opensourcery.co.za
>
> >
>


-- 
Cheers

Koz

--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Core" group.
To post to this group, send email to rubyonrails-core@googlegroups.com
To unsubscribe from this group, send email to
rubyonrails-core+unsubscribe@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/rubyonrails-core?hl=en
-~----------~----~----~----~------~----~------~--~---

On Mon, Dec 1, 2008 at 6:54 PM, Michael Koziarski <michael@koziarski.com>
wrote:
>
>> Agreed. For my example above it works perfectly well, since
I''m not really
>> concerned with the Time instances. They''re just part of a JSON
payload, and
>> I control only part of the payload (where time doesn''t
matter).
>
> OK, so from my perspective the ideal solution would be that you can
> just tell your users to require the json gem *after* activesupport,
> and have everything ''just work'' because it nukes our
methods.  If
> that''s not the case, lets fix it.
Obvious, but how many others are aware of this. Maybe have
ActiveSupport emit a warning message when it detects the presence of
the JSON gem, telling users who rely on the JSON gem to include it
afterwards? This doesn''t fiddle with anything, and gives the user
great feedback.

Best

-- 
Kenneth Kalmer
kenneth.kalmer@gmail.com
http://opensourcery.co.za

--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Core" group.
To post to this group, send email to rubyonrails-core@googlegroups.com
To unsubscribe from this group, send email to
rubyonrails-core+unsubscribe@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/rubyonrails-core?hl=en
-~----------~----~----~----~------~----~------~--~---