Skip to content

Commit

Permalink
fix: use ruby json.generate to serialize payloads; remove multi_json/…
Browse files Browse the repository at this point in the history
…oj dependencies
  • Loading branch information
waltjones committed Jul 11, 2019
1 parent e14f979 commit 71d00b9
Show file tree
Hide file tree
Showing 24 changed files with 72 additions and 323 deletions.
12 changes: 1 addition & 11 deletions Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ ENV['CURRENT_GEMFILE'] ||= __FILE__

is_jruby = defined?(JRUBY_VERSION) || (defined?(RUBY_ENGINE) && RUBY_ENGINE == 'jruby')

GEMFILE_RAILS_VERSION = '6.0.0rc1'.freeze
GEMFILE_RAILS_VERSION = '4.1.12'.freeze

gem 'activerecord-jdbcsqlite3-adapter', :platform => :jruby
gem 'appraisal'
Expand All @@ -31,16 +31,6 @@ else
gem 'sqlite3', '~> 1.4', :platform => [:ruby, :mswin, :mingw] # rubocop:disable Bundler/DuplicatedGem
end

unless is_jruby
if RUBY_VERSION >= '2.5'
gem 'oj'
elsif RUBY_VERSION >= '2.4.0'
gem 'oj', '~> 2.16.1' # rubocop:disable Bundler/DuplicatedGem
else
gem 'oj', '~> 2.12.14' # rubocop:disable Bundler/DuplicatedGem
end
end

if RUBY_VERSION < '2.2.2'
gem 'sidekiq', '~> 2.13.0'
else
Expand Down
8 changes: 0 additions & 8 deletions gemfiles/rails30.gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -20,14 +20,6 @@ gem 'hitimes', '< 1.2.2'
gem 'rake', '< 11'
gem 'rspec-rails', '>= 2.14.0'

unless is_jruby
if RUBY_VERSION >= '2.4.0'
gem 'oj', '~> 2.16.1'
else
gem 'oj', '~> 2.12.14'
end
end

if RUBY_VERSION < '2.2.2'
gem 'sidekiq', '~> 2.13.0'
else
Expand Down
8 changes: 0 additions & 8 deletions gemfiles/rails31.gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -19,14 +19,6 @@ gem 'rails', '3.1.12'
gem 'rspec-rails', '~> 3.4'
gem 'rake'

unless is_jruby
if RUBY_VERSION >= '2.4.0'
gem 'oj', '~> 2.16.1'
else
gem 'oj', '~> 2.12.14'
end
end

if RUBY_VERSION < '2.2.2'
gem 'sidekiq', '~> 2.13.0'
else
Expand Down
8 changes: 0 additions & 8 deletions gemfiles/rails32.gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,6 @@ gem 'sqlite3', '< 1.4.0', :platform => [:ruby, :mswin, :mingw]
# Please see https://github.com/rspec/rspec-rails/issues/1273
gem 'test-unit'

unless is_jruby
if RUBY_VERSION >= '2.4.0'
gem 'oj', '~> 2.16.1'
else
gem 'oj', '~> 2.12.14'
end
end

if RUBY_VERSION < '2.2.2'
gem 'sidekiq', '~> 2.13.0'
else
Expand Down
8 changes: 0 additions & 8 deletions gemfiles/rails40.gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,6 @@ gem 'sqlite3', '< 1.4.0', :platform => [:ruby, :mswin, :mingw]
# Please see https://github.com/rspec/rspec-rails/issues/1273
gem 'test-unit'

unless is_jruby
if RUBY_VERSION >= '2.4.0'
gem 'oj', '~> 2.16.1'
else
gem 'oj', '~> 2.12.14'
end
end

if RUBY_VERSION < '2.2.2'
gem 'sidekiq', '~> 2.13.0'
else
Expand Down
8 changes: 0 additions & 8 deletions gemfiles/rails41.gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -19,14 +19,6 @@ gem 'rake'
gem 'rspec-rails', '~> 3.4'
gem 'sqlite3', '< 1.4.0', :platform => [:ruby, :mswin, :mingw]

unless is_jruby
if RUBY_VERSION >= '2.4.0'
gem 'oj', '~> 2.16.1'
else
gem 'oj', '~> 2.12.14'
end
end

if RUBY_VERSION < '2.2.2'
gem 'sidekiq', '~> 2.13.0'
else
Expand Down
10 changes: 0 additions & 10 deletions gemfiles/rails42.gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -27,16 +27,6 @@ platforms :rbx do
gem 'rubysl', '~> 2.0' unless RUBY_VERSION.start_with?('1')
end

unless is_jruby
if RUBY_VERSION >= '2.5'
gem 'oj'
elsif RUBY_VERSION >= '2.4.0'
gem 'oj', '~> 2.16.1'
else
gem 'oj', '~> 2.12.14'
end
end

if RUBY_VERSION < '2.2.2'
gem 'sidekiq', '~> 2.13.0'
else
Expand Down
10 changes: 0 additions & 10 deletions gemfiles/rails50.gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -27,16 +27,6 @@ gem 'rake'

gem 'sidekiq', '>= 2.13.0'

unless is_jruby
if RUBY_VERSION >= '2.5'
gem 'oj'
elsif RUBY_VERSION >= '2.4.0'
gem 'oj', '~> 2.16.1'
else
gem 'oj', '~> 2.12.14'
end
end

platforms :rbx do
gem 'minitest'
gem 'racc'
Expand Down
10 changes: 0 additions & 10 deletions gemfiles/rails51.gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -27,16 +27,6 @@ gem 'rake'

gem 'sidekiq', '>= 2.13.0'

unless is_jruby
if RUBY_VERSION >= '2.5'
gem 'oj'
elsif RUBY_VERSION >= '2.4.0'
gem 'oj', '~> 2.16.1'
else
gem 'oj', '~> 2.12.14'
end
end

platforms :rbx do
gem 'minitest'
gem 'racc'
Expand Down
10 changes: 0 additions & 10 deletions gemfiles/rails52.gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -25,16 +25,6 @@ gem 'rspec-mocks', '~> 3.8.0'

gem 'rake'

unless is_jruby
if RUBY_VERSION >= '2.5'
gem 'oj'
elsif RUBY_VERSION >= '2.4.0'
gem 'oj', '~> 2.16.1'
else
gem 'oj', '~> 2.12.14'
end
end

gem 'sidekiq', '>= 2.13.0'

platforms :rbx do
Expand Down
10 changes: 0 additions & 10 deletions gemfiles/rails60.gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -27,16 +27,6 @@ gem 'rspec-rails', :git => 'https://github.com/rspec/rspec-rails', :ref => 'v4.0

gem 'rake'

unless is_jruby
if RUBY_VERSION >= '2.5'
gem 'oj'
elsif RUBY_VERSION >= '2.4.0'
gem 'oj', '~> 2.16.1'
else
gem 'oj', '~> 2.12.14'
end
end

gem 'sidekiq', '>= 2.13.0'

platforms :rbx do
Expand Down
53 changes: 2 additions & 51 deletions lib/rollbar/json.rb
Original file line number Diff line number Diff line change
@@ -1,68 +1,19 @@
require 'multi_json'
require 'rollbar/json/oj'
require 'rollbar/json/default'
require 'rollbar/language_support'

begin
require 'oj'
rescue LoadError
end

module Rollbar
module JSON # :nodoc:
extend self

attr_writer :options_module

def dump(object)
# `basic_socket` plugin addresses the following issue: https://github.com/rollbar/rollbar-gem/issues/845
Rollbar.plugins.get('basic_socket').load_scoped!(true) do
with_adapter { MultiJson.dump(object, adapter_options) }
::JSON.generate(object)
end
end

def load(string)
with_adapter { MultiJson.load(string, adapter_options) }
end

def with_adapter(&block)
MultiJson.with_adapter(detect_multi_json_adapter, &block)
end

def detect_multi_json_adapter
options = {}
options[:adapter] = :oj if defined?(::Oj)

MultiJson.current_adapter(options)
end

def adapter_options
options_module.options
end

def options_module
@options_module ||= find_options_module
end

def find_options_module
module_name = multi_json_adapter_module_name

if LanguageSupport.const_defined?(Rollbar::JSON, module_name, false)
LanguageSupport.const_get(Rollbar::JSON, module_name, false)
else
Default
end
end

# 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_name
detect_multi_json_adapter.name[/^MultiJson::Adapters::(.*)$/, 1]
::JSON.parse(string)
end
end
end
11 changes: 0 additions & 11 deletions lib/rollbar/json/default.rb

This file was deleted.

16 changes: 0 additions & 16 deletions lib/rollbar/json/oj.rb

This file was deleted.

2 changes: 1 addition & 1 deletion lib/rollbar/plugins/basic_socket.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
# Needed to avoid active_support (< 4.1.0) bug serializing JSONs
dependency do
defined?(ActiveSupport::VERSION::STRING) &&
Gem::Version.new(ActiveSupport::VERSION::STRING) < Gem::Version.new('5.2.0')
Gem::Version.new(ActiveSupport::VERSION::STRING) < Gem::Version.new('4.1.0')
end

execute do
Expand Down
15 changes: 15 additions & 0 deletions lib/rollbar/util/hash.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ def self.deep_stringify_keys(hash, seen = {})
return if seen[hash.object_id]

seen[hash.object_id] = true
replace_seen_children(hash, seen)

hash.reduce({}) do |h, (key, value)|
h[key.to_s] = map_value(value, :deep_stringify_keys, seen)
Expand All @@ -22,12 +23,26 @@ def self.map_value(thing, meth, seen)
thing
else
seen[thing.object_id] = true
replace_seen_children(thing, seen)
thing.map { |v| map_value(v, meth, seen) }
end
else
thing
end
end

def self.replace_seen_children(thing, seen)
case thing
when ::Hash
thing.keys.each do |key|
thing[key] = "removed circular reference: #{thing[key]}" if seen[thing[key].object_id]
end
when Array
thing.each_with_index do |_, i|
thing[i] = "removed circular reference: #{thing[i]}" if seen[thing[i].object_id]
end
end
end
end
end
end
2 changes: 0 additions & 2 deletions rollbar.gemspec
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,6 @@ Gem::Specification.new do |gem|
gem.required_ruby_version = '>= 1.9.3'
gem.version = Rollbar::VERSION

gem.add_runtime_dependency 'multi_json'

if gem.respond_to?(:metadata)
gem.metadata['changelog_uri'] = 'https://github.com/rollbar/rollbar-gem/releases'
gem.metadata['source_code_uri'] = 'https://github.com/rollbar/rollbar-gem'
Expand Down
23 changes: 20 additions & 3 deletions spec/rollbar/item_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -687,17 +687,34 @@ def as_json(*)
end
let(:item) { Rollbar::Item.build_with(payload) }

it 'fails in ActiveSupport with stack too deep' do
it "doesn't fail in ActiveSupport >= 4.1" do
begin
_json = item.dump
rescue NoMemoryError, SystemStackError, Java::JavaLang::StackOverflowError

# If you get an uninitialized constant "Java" error, it just means an unexpected exception
# occurred (i.e. one not in the below list) and caused Java::JavaLang::StackOverflowError.
# If the test case is working correctly, this shouldn't happen ang the Java error type
# will only be evaluated on JRuby builds.
rescue NoMemoryError,
SystemStackError,
ActiveSupport::JSON::Encoding::CircularReferenceError,
Java::JavaLang::StackOverflowError

# If ActiveSupport is invoked, we'll end up here.
# Item#dump fails with SystemStackError (ActiveSupport > 4.0)
# or NoMemoryError (ActiveSupport <= 4.0) which, as system exceptions
# not a StandardError, cannot be tested by `expect().to raise_error`
error = :SystemError
end

expect(error).to be_eql(:SystemError)
if Gem::Version.new(ActiveSupport::VERSION::STRING) >= Gem::Version.new('4.1.0')
expect(error).not_to be_eql(:SystemError)
else
# This ActiveSupport is vulnerable to circular reference errors, and is
# virtually impossible to correct, because these versions of AS even hook into
# core Ruby JSON.
expect(error).to be_eql(:SystemError)
end
end
end

Expand Down
18 changes: 0 additions & 18 deletions spec/rollbar/json/oj_spec.rb

This file was deleted.

Loading

0 comments on commit 71d00b9

Please sign in to comment.