Skip to content

Commit

Permalink
Restore MultiJson and add custom option modules. Fix #303
Browse files Browse the repository at this point in the history
In order to support JRuby it'd be better not force installing Oj since
it cannot be used in JRuby if C extensions are not enabled, and they are
not supported by default.

So, we add MultiJson again to the project. This will create more
different scenarios but since it could be the best option according to
previous paragraph.

We've added in this change support for custom options depending on the
loaded adapter in MultiJson. We can add more adapter options modules in
this way:

module Rollbar
  module JSON
    module AnyMultiJsonAdapterName
      def self.options
        { # here comes the options we need for that adapter }
      end
    end
  end
end

We have, for now, just the Oj module that will return the options we had
before this commit.

The specs run with Oj by default and it's the recommended adapter to use
with Rollbar.
  • Loading branch information
Jon de Andres committed Sep 18, 2015
1 parent 67f130a commit 63db163
Show file tree
Hide file tree
Showing 9 changed files with 148 additions and 48 deletions.
5 changes: 0 additions & 5 deletions lib/rollbar.rb
Original file line number Diff line number Diff line change
Expand Up @@ -740,17 +740,12 @@ def configure

yield(configuration)

configure_json_backend
require_hooks
require_core_extensions

reset_notifier!
end

def configure_json_backend
Rollbar::JSON.setup
end

def reconfigure
@configuration = Configuration.new
@configuration.enabled = true
Expand Down
52 changes: 31 additions & 21 deletions lib/rollbar/json.rb
Original file line number Diff line number Diff line change
@@ -1,37 +1,47 @@
require 'rollbar/json/oj'
require 'rollbar/json/default'

module Rollbar
module JSON
extend self

attr_accessor :backend_name
attr_accessor :dump_method
attr_accessor :load_method

def load_oj
require 'oj'
attr_writer :adapter_module

options = { :mode=> :compat,
:use_to_json => false,
:symbol_keys => false,
:circular => false
}
def dump(object)
MultiJson.dump(object, adapter_options)
end

self.dump_method = proc { |obj| Oj.dump(obj, options) }
self.load_method = proc { |obj| Oj.load(obj, options) }
self.backend_name = :oj
def load(string)
MultiJson.load(string, adapter_options)
end

true
def adapter_options
adapter_module.options
end

def dump(object)
dump_method.call(object)
def adapter_module
@adapter_module ||= find_adapter_module
end

def load(string)
load_method.call(string)
def find_adapter_module
module_name = multi_json_adapter_module

begin
const_get(module_name)
rescue NameError
Default
end
end

def setup
load_oj
# MultiJson adapters have this name structure:
# "MultiJson::Adapters::{AdapterModule}"
#
# Ex: MultiJson::Adapters::Oj
# Ex: MultiJson::Adapters::JsonGem
#
# In this method we just get the last module name.
def multi_json_adapter_module
MultiJson.current_adapter.name[/^MultiJson::Adapters::(.*)$/, 1]
end
end
end
Expand Down
11 changes: 11 additions & 0 deletions lib/rollbar/json/default.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
module Rollbar
module JSON
module Default
extend self

def options
{}
end
end
end
end
15 changes: 15 additions & 0 deletions lib/rollbar/json/oj.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
module Rollbar
module JSON
module Oj
extend self

def options
{ :mode=> :compat,
:use_to_json => false,
:symbol_keys => false,
:circular => false
}
end
end
end
end
3 changes: 2 additions & 1 deletion rollbar.gemspec
Original file line number Diff line number Diff line change
Expand Up @@ -28,5 +28,6 @@ Gem::Specification.new do |gem|
gem.add_development_dependency 'delayed_job'
gem.add_development_dependency 'rake', '>= 0.9.0'
gem.add_development_dependency 'redis'
gem.add_runtime_dependency 'oj', '~> 2.12.14'
gem.add_runtime_dependency 'multi_json'
gem.add_development_dependency 'oj', '~> 2.12.14'
end
2 changes: 1 addition & 1 deletion spec/delayed/backend/test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ def initialize(hash = {})
self.attempts = 0
self.priority = 0
self.id = (self.class.id += 1)
hash.each { |k, v| send(:"#{k}=", v) }
hash.each { |k, v| send("#{k}=", v) }
end

def self.all
Expand Down
18 changes: 18 additions & 0 deletions spec/rollbar/json/oj_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
require 'spec_helper'

require 'rollbar/json/oj'

describe Rollbar::JSON::Oj do
let(:options) do
{
:mode => :compat,
:use_to_json => false,
:symbol_keys => false,
:circular => false
}
end

it 'returns correct options' do
expect(described_class.options).to be_eql(options)
end
end
86 changes: 66 additions & 20 deletions spec/rollbar/json_spec.rb
Original file line number Diff line number Diff line change
@@ -1,43 +1,89 @@
require 'spec_helper'

require 'multi_json'
require 'rollbar/json'
require 'rollbar/configuration'

describe Rollbar::JSON do
before do
Rollbar::JSON.setup
class Rollbar::JSON::MockAdapter
def self.options
{ 'mock' => 'adapter' }
end
end

let(:payload) do
{ :foo => :bar }
module MultiJson
module Adapters
module MockAdapter
end
end
end

let(:options) do
{
:mode => :compat,
:use_to_json => false,
:symbol_keys => false,
:circular => false
}
module MultiJson
module Adapters
module MissingCustomOptions
end
end
end


describe Rollbar::JSON do
let(:payload) do
{ :foo => :bar }
end
let(:adapter_options) { { 'option' => 'value' } }

describe '.dump' do
it 'has JSON as backend' do
expect(Rollbar::JSON.backend_name).to be_eql(:oj)
before do
allow(described_class).to receive(:adapter_options).and_return(adapter_options)
end

it 'calls MultiJson.dump' do
expect(::MultiJson).to receive(:dump).once.with(payload, adapter_options)

it 'calls JSON.generate' do
expect(::Oj).to receive(:dump).once.with(payload, options)

Rollbar::JSON.dump(payload)
described_class.dump(payload)
end
end

describe '.load' do
before do
allow(described_class).to receive(:adapter_options).and_return(adapter_options)
end

it 'calls MultiJson.load' do
expect(::Oj).to receive(:load).once.with(payload, options)
expect(::MultiJson).to receive(:load).once.with(payload, adapter_options)

described_class.load(payload)
end
end

describe '.adapter_options' do
it 'calls .options in adapter module' do
expect(described_class.adapter_module).to receive(:options)

described_class.adapter_options
end
end

describe '.adapter_module' do
before { described_class.adapter_module = nil }

context 'with a defined rollbar adapter' do
let(:expected_adapter) { Rollbar::JSON::MockAdapter }

it 'returns the correct options' do
MultiJson.with_adapter(MultiJson::Adapters::MockAdapter) do
expect(described_class.adapter_module).to be(expected_adapter)
end
end
end

context 'without a defined rollbar adapter' do
let(:expected_adapter) { Rollbar::JSON::Default }

Rollbar::JSON.load(payload)
it 'returns the correct options' do
MultiJson.with_adapter(MultiJson::Adapters::MissingCustomOptions) do
expect(described_class.adapter_module).to be(expected_adapter)
end
end
end
end
end
4 changes: 4 additions & 0 deletions spec/spec_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,10 @@
require 'rspec/rails'
require 'database_cleaner'
require 'genspec'
require 'multi_json'
require 'oj'

MultiJson.use(:oj)

namespace :dummy do
load 'spec/dummyapp/Rakefile'
Expand Down

1 comment on commit 63db163

@PetrKaleta
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks like a great solution man!

Please sign in to comment.