#to_json incompatibilities between ActiveSupport & JSON gems

[Kenneth Kalmer]

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...@gmail.com
http://opensourcery.co.za

[Michael Koziarski]

What would the fix look like? what are nil and 1 in the context above?

--
Cheers

Koz

[Kenneth Kalmer]

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]

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

[Michael Koziarski]

> 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

[Kenneth Kalmer]

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.

[Michael Koziarski]

> 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...@gmail.com
> http://opensourcery.co.za
>
> >
>

--
Cheers

Koz

[Kenneth Kalmer]

On Mon, Dec 1, 2008 at 6:54 PM, Michael Koziarski <mic...@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