Skip to content

Commit

Permalink
Add JrJackson adapter
Browse files Browse the repository at this point in the history
  • Loading branch information
rwz committed Mar 16, 2013
1 parent 8695900 commit 4dd86fa
Show file tree
Hide file tree
Showing 5 changed files with 28 additions and 5 deletions.
1 change: 1 addition & 0 deletions Gemfile
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ platforms :ruby, :mswin, :mingw do
end end
platforms :jruby do platforms :jruby do
gem 'gson', '>= 0.6', :require => nil gem 'gson', '>= 0.6', :require => nil
gem 'jrjackson', :require => nil
end end


group :development do group :development do
Expand Down
1 change: 1 addition & 0 deletions lib/multi_json.rb
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ def default_options=(value)
['yajl', :yajl], ['yajl', :yajl],
['json', :json_gem], ['json', :json_gem],
['gson', :gson], ['gson', :gson],
['jrjackson', :jrjackson],
['json/pure', :json_pure] ['json/pure', :json_pure]
] ]


Expand Down
20 changes: 20 additions & 0 deletions lib/multi_json/adapters/jrjackson.rb
Original file line number Original file line Diff line number Diff line change
@@ -0,0 +1,20 @@
require 'jrjackson_r' unless defined?(::JrJackson)
require 'multi_json/adapters/ok_json'

module MultiJson
module Adapters
class Jrjackson < MultiJson::Adapters::OkJson

This comment has been minimized.

Copy link
@nirvdrum

nirvdrum Mar 18, 2013

I'm a little late to the party, but should this be "JrJackson"? Or was the name deliberately changed?

This comment has been minimized.

Copy link
@sferik

sferik Mar 18, 2013

Member

Ah, yes, you're right. I'll fix this and push 1.7.1.

This comment has been minimized.

Copy link
@sferik

sferik Mar 18, 2013

Member

I just pushed 1.7.1 but then I realized that this breaks the build on JRuby: https://travis-ci.org/intridea/multi_json/builds/5601902

Basically, the specs expect the convention of camelCase class names to correspond to snake_case file names, so the options are:

  1. Revert 5373a5e
  2. Change jrjackson.rb to jr_jackson.rb
  3. Rewrite the specs to manually map adapter class names to file names

I'm open to suggestions on which of these solutions is most desirable. I don't believe this change is causing any issues for users but I'd like to get the specs passing again.

This comment has been minimized.

Copy link
@nirvdrum

nirvdrum Mar 18, 2013

Gotcha. The only problem I had with the old name is if you wanted to manually specify the adapter, it was a bit confusing and seemed error-prone. I guess I would be in favor of option 2. That goes against the filename convention in JrJackson, but I don't think that really matters here. Project should be able to name files whatever they want.

This comment has been minimized.

Copy link
@sferik

sferik Mar 18, 2013

Member

@nirvdrum Would you mind submitting a pull request making that change? I don't have time at the moment.

This comment has been minimized.

Copy link
@nirvdrum

nirvdrum Mar 18, 2013

Sure thing. It might take me a day or so. I need to get some Rails security stuff out the door first.

This comment has been minimized.

Copy link
@rwz

rwz Mar 19, 2013

Author Member

That's the reason I named it Jrjackson, and not JrJackson in the first place. I think we can come up with a system of aliases, so for example MultiJson.use :jrjackson and MultiJson.use :jr_jackson would do the same thing.

I can prepare a fix for that today I guess.

ParseError = ::Java::OrgCodehausJackson::JsonParseException

def load(string, options={}) #:nodoc:
string = string.read if string.respond_to?(:read)
result = ::JrJackson::Json.parse(string)
options[:symbolize_keys] ? symbolize_keys(result) : result
end

def dump(object, options={}) #:nodoc:
::JrJackson::Json.generate(prepare_object(object){ |value| value })
end
end
end
end
2 changes: 1 addition & 1 deletion spec/adapter_shared_example.rb
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ class << time


# This behavior is currently not supported by gson.rb # This behavior is currently not supported by gson.rb
# See discussion at https://github.com/intridea/multi_json/pull/71 # See discussion at https://github.com/intridea/multi_json/pull/71
unless adapter == 'gson' unless %w(gson jrjackson).include?(adapter)
it 'dumps custom objects that implement to_json' do it 'dumps custom objects that implement to_json' do
klass = Class.new do klass = Class.new do
def to_json(*) def to_json(*)
Expand Down
9 changes: 5 additions & 4 deletions spec/multi_json_spec.rb
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -144,10 +144,11 @@


it_behaves_like 'has options', MultiJson it_behaves_like 'has options', MultiJson


%w(gson json_gem json_pure nsjsonserialization oj ok_json yajl).each do |adapter| %w(gson jrjackson json_gem json_pure nsjsonserialization oj ok_json yajl).each do |adapter|
next if adapter == 'gson' && !jruby? next if !jruby? && %w(gson jrjackson).include?(adapter)
next if adapter == 'nsjsonserialization' && !macruby? next if !macruby? && adapter == 'nsjsonserialization'
next if jruby? && (adapter == 'oj' || adapter == 'yajl') next if jruby? && %w(oj yajl).include?(adapter)

context adapter do context adapter do
it_behaves_like 'an adapter', adapter it_behaves_like 'an adapter', adapter
end end
Expand Down

0 comments on commit 4dd86fa

Please sign in to comment.