Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

to_json() and bigdecimal_as_decimal behavior different with and without argument #659

Closed
GUI opened this issue May 10, 2021 · 9 comments
Closed

Comments

@GUI
Copy link

GUI commented May 10, 2021

This seems like it might be a bug, but I know the JSON compatibility/mimicking gets complicated, so let me know if this is expected behavior.

When using the Oj.optimize_rails mode inside a Rails app, the basic issue I'm seeing is that to_json seems to behave differently if arguments are passed into the to_json() call or not. Specifically, the presence of an argument to to_json (eg, to_json({}) seems to break the behavior of the bigdecimal_as_decimal option set via Oj.default_options. In quickly testing some other options set via Oj.default_options, those other options do not seem to be affected, so it's possible this only affects the bigdecimal_as_decimal option (but not entirely sure).

Here's a script that demonstrates the issue (the comments show the output I'm observing and what might be expected instead):

require "bundler/inline"

gemfile do
  source "https://rubygems.org"
  gem "activesupport", "= 6.1.3.2"
  gem "oj", "= 3.11.5"
end

require "bigdecimal"
require "active_support/json"
require "oj"

Oj.optimize_rails

Oj.default_options = {
  :bigdecimal_as_decimal => true,
  # Just some other test options to see if they are affected too:
  :indent => 2,
  :omit_nil => true,
}

puts "Without to_json argument:"
puts ({ "foo" => BigDecimal("0.57914131"), "bar" => nil, "baz" => "a" }.to_json())
# Without to_json argument:
# {
#   "foo":0.57914131,
#   "baz":"a"
# }
#
# Notes: BigDecimal converted to float as expected due to default
# `:bigdecimal_as_decimal` option.

puts ""
puts "With to_json empty argument:"
puts ({ "foo" => BigDecimal("0.57914131"), "bar" => nil, "baz" => "a" }.to_json({}))
# With to_json empty argument:
# {
#   "foo":"0.57914131",
#   "baz":"a"
# }
#
# Notes: BigDecimal remains a string, which isn't what I'd expect. But the
# other default options of `:indent` and `:omit_nil` seem to still be in
# effect, so it seems like other `Oj.default_options` are still being used.

puts ""
puts "With to_json real argument:"
puts ({ "foo" => BigDecimal("0.57914131"), "bar" => nil, "baz" => "a" }.to_json(:only => ["foo"]))
# With to_json real argument:
# {
#   "foo":"0.57914131"
# }
#
# Notes: Same as above, BigDecimal remains a string, but this is just noting a
# real-world potential use-case for why to_json might accept arguments, which
# does seem to be working here.

Thank you!

@ohler55
Copy link
Owner

ohler55 commented May 10, 2021

When Oj.optimize_rails is called it also calls Oj.mimic_JSON which monkey patches to_json for all classes just like the JSON gem does. By calling to_json you are using that monkey patched code so it will behave like the JSON gem which does not know about the Oj options. If you want to assure consistency without rails and the JSON gem battling it out then use Oj.dump with what ever mode and options you want.

To verify there is a bug in Oj, compare to how the JSON gem works.

@ohler55
Copy link
Owner

ohler55 commented Jun 13, 2021

Were you able to find a case where Oj in compat mode differed in behaviour from the JSON gem? If not can this be closed?

@GUI
Copy link
Author

GUI commented Jun 20, 2021

@ohler55: Apologies for the delay in following up, but thanks for the speedy reply! Since this difference in behavior is specific to Oj's bigdecimal_as_decimal option, which the JSON gem doesn't support (as far as I can tell), then I'm not sure there's really a way to compare this behavior. I realize this might get into conflicting areas of compatibility modes, so maybe this isn't expected to work. I guess I just found this behavior slightly confusing:

  • { "foo" => BigDecimal("0.1") }.to_json(): Oj's bigdecimal_as_decimal setting appears to work if no arguments are given to to_json.
  • { "foo" => BigDecimal("0.1") }.to_json({}): Oj's bigdecimal_as_decimal setting does not appear to work if any arguments are given to to_json.

I just wasn't sure how passing arguments to to_json was affecting this decimal behavior, so this seemed slightly inconsistent and surprising.

Although, if Oj is trying to emulate what the JSON gem does, and the JSON gem doesn't support the bigdecimal_as_decimal setting, then I'm not exactly sure what the right behavior is here. I find Oj's bigdecimal_as_decimal support useful even while in the JSON compatibility mode, but if that's problematic, then maybe having a more consistent behavior (even if that means disabling bigdecimal_as_decimal altogether) might be preferable/less surprising?

@ohler55
Copy link
Owner

ohler55 commented Jun 22, 2021

I'd love to claim that it is the intended behaviour and in some sense it is but you are right that it is due to attempting to cover all the compatibility modes between the JSON gem and Rails. Checkout issue #651 . It is a rabbit hole so deep it is some number of circles of hell. I'm sure there is some additional flag that could be kept and passed down for all calls but I can't easily fix the behaviour without breaking something else. It comes down to JSON.generate() acts differently than the #to_json() I thought I had it covered by tracking whether or not the JSON::State was passed along but apparently you found a case that foils that approach.

I think the best approach to solving the issue is just don't set Oj's bigdecimal_as_decimal to true. Is that something you can live with?

@melcher
Copy link

melcher commented Jul 1, 2021

Chiming in as we're blocked from upgrading from 3.11.3 to 3.11.4+ due to it breaking our API tests. We're using Rails, Blueprinter and Oj (similar to this ticket #656) so can't easily change how the Oj is being called.

Our API tests are failing on 3.11.4+ due to numbers being returned as quoted strings (e.g. "0.0") instead of a JSON number (e.g. 0.0).

We're using Oj.optimize_rails and the bigdecimal_as_decimal setting, so appears to be the same issue as referenced here. Unfortunately we're not able to easily change our API format. Any other suggestions for a work-around?

@wjessop
Copy link

wjessop commented May 23, 2023

Rails 6.1.7.3
Oj 3.11.3-4
Ruby 2.7.5

I'd really appreciate some guidance on how to bump Oj from 3.11.3 to 3.11.4+ too as I'm struggling to get it to work. When I update the gem to 3.11.4 I get the following classes of test failures:

Dates formatted differently:

 expected: "1985-10-26T01:21:00+00:00"
      got: "1985-10-26 01:21:00 +0000"

Hashes not getting turned into hashes again:

undefined method `symbolize_keys' for #<String:0x0000560f857991c8>

Extra hash keys

+  "position"=>0,

Numbers as strings

-"unique_for" => 10,
+"unique_for" => "10",

The config we were using for 3.11.3 was simply:

Oj.optimize_rails

I've tried all manner of variations of bigdecimal_load, bigdecimal_as_decimal, mode etc. in a default_options hash as mentioned in modes and options but none had any effect at all, to the point that I added a raise in the initialiser to check it was actually being loaded, it was.

What config do I need to do to make Oj 3.11.4 work the same as 3.11.3? I'd really love to be able to fix our app to work in the same way Rails does, but though there are only 22 test failures there will unfortunately likely be many more places that aren't tested that will break so I'm kinda stuck persisting the behaviour of 3.11.3.

@ohler55
Copy link
Owner

ohler55 commented May 23, 2023

That is quite a few issues and I'm not sure I understand how each is encountered so let's look at one at a time but first let's deal with the version issues. You are using an unsupported version of Ruby and Rails. The Oj version is more than a year old. Is there any way you can move to more recent versions of any or all of those?

@wjessop
Copy link

wjessop commented May 24, 2023

We plan on moving upgrading:

  • Ruby in the next couple of months
  • Rails after that (though it still has security support for now)

Oj we're trying to upgrade right now, that's what this post is about :) The strange thing is, we had 100% of tests passing and went to 22 tests failing (see the examples above) and the only change was going from Oj 3.11.3 to 3.11.4 so what changed in 3.11.4 that could cause these things to break?

@ohler55
Copy link
Owner

ohler55 commented May 24, 2023

The changelog shows a fix for conflict between Rails and the JSON gem. If I had to guess it would be related to that where JSON.generate produces something different than to_json.

If you can tolerate the wait I'd suggest waiting until you update Ruby and Rails and then we can look at how or if Oj produces different results. I'd prefer not to branch off old releases and instead stay with the current. I don't have the bandwidth to track more than one release branch.

@ohler55 ohler55 closed this as completed Dec 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants