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

Update usage of :bigdecimal_as_decimal in Rails Mode #716

Merged
merged 3 commits into from
Oct 28, 2021

Conversation

mrbongiolo
Copy link
Contributor

The option :bigdecimal_as_decimal is not available in Rails Mode (as discussed here #693), as the original JSON and Rails implementation does not have a way to "force" bigdecimal values to be encoded as numbers in json.

The option `:bigdecimal_as_decimal` is not available in Rails Mode (as discussed here ohler55#693), as the original JSON and Rails implementation does not have a way to "force" bigdecimal values to be encoded as numbers in json.
@ohler55
Copy link
Owner

ohler55 commented Oct 15, 2021

I have to verify if the change is indeed accurate before merging. I seem to remember having that option available as non-standard for rails and hence the footnote. It might take a day or two to get to it though.

@mrbongiolo
Copy link
Contributor Author

Sure, no problem. TBH I hope that I'm wrong on this one and this option is re-added to OJ >.<

@ohler55
Copy link
Owner

ohler55 commented Oct 23, 2021

It took a while but I did finally get a break from moving to look at this. There is a way to set the :bigdecimal_as_decimal with rails. If after setting everything else up the default options are set for the :bigdecimal_as_decimal it will be honoured by the rails mode. It can not be provided as an argument but can be set with the default options.

@mrbongiolo
Copy link
Contributor Author

mrbongiolo commented Oct 23, 2021

Thanks for looking at it and I hope that everything went smoothly when moving.
About the issue, do you have a small working example? Because this is not working:

require 'bundler/inline'

OJ_VERSION = '3.13.9'

puts "With OJ version (#{OJ_VERSION})"

gemfile do
  source 'https://rubygems.org'
  gem 'rails'
  gem 'oj', OJ_VERSION
end

puts "Before optimizing"

puts "Load"

p JSON.load("{\"ticker\":\"ASAI3\",\"price\":\"18.7\",\"rate\":-0.8}")
p Oj.load("{\"ticker\":\"ASAI3\",\"price\":\"18.7\",\"rate\":-0.8}")

puts "Generate"

p JSON.generate({:ticker=>"ASAI3", :price=>18.7, :rate=>-0.8.to_d})
p Oj.generate({:ticker=>"ASAI3", :price=>18.7, :rate=>-0.8.to_d})

Oj.optimize_rails

Oj.default_options = {
  mode: :rails,
  bigdecimal_as_decimal: true,
  bigdecimal_load: true
}

puts "After optimizing"

puts "Load"

p JSON.load("{\"ticker\":\"ASAI3\",\"price\":\"18.7\",\"rate\":-0.8}")
p Oj.load("{\"ticker\":\"ASAI3\",\"price\":\"18.7\",\"rate\":-0.8}")

puts "Generate"

p JSON.generate({:ticker=>"ASAI3", :price=>18.7, :rate=>-0.8.to_d})
p Oj.generate({:ticker=>"ASAI3", :price=>18.7, :rate=>-0.8.to_d})

JSON is still generated as strings:

With OJ version (3.13.9)
Before optimizing
Load
{"ticker"=>"ASAI3", "price"=>"18.7", "rate"=>-0.8}
{"ticker"=>"ASAI3", "price"=>"18.7", "rate"=>-0.8}
Generate
"{\"ticker\":\"ASAI3\",\"price\":18.7,\"rate\":\"-0.8\"}"
"{\"ticker\":\"ASAI3\",\"price\":18.7,\"rate\":\"-0.8\"}"
After optimizing
Load
{"ticker"=>"ASAI3", "price"=>"18.7", "rate"=>-0.8e0}
{"ticker"=>"ASAI3", "price"=>"18.7", "rate"=>-0.8e0}
Generate
"{\"ticker\":\"ASAI3\",\"price\":18.7,\"rate\":\"-0.8\"}"
"{\"ticker\":\"ASAI3\",\"price\":18.7,\"rate\":\"-0.8\"}"

@mrbongiolo
Copy link
Contributor Author

Upon further investigation, there seems to be a "problem" with Oj.generate and JSON.generate:

p JSON.generate({:ticker=>"ASAI3", :price=>18.7, :rate=>-0.8.to_d})
p Oj.generate({:ticker=>"ASAI3", :price=>18.7, :rate=>-0.8.to_d})
p Oj.dump({:ticker=>"ASAI3", :price=>18.7, :rate=>-0.8.to_d})
p({:ticker=>"ASAI3", :price=>18.7, :rate=>-0.8.to_d}.to_json)

Result:

"{\"ticker\":\"ASAI3\",\"price\":18.7,\"rate\":\"-0.8\"}"
"{\"ticker\":\"ASAI3\",\"price\":18.7,\"rate\":\"-0.8\"}"
"{\"ticker\":\"ASAI3\",\"price\":18.7,\"rate\":-0.8}"
"{\"ticker\":\"ASAI3\",\"price\":18.7,\"rate\":-0.8}"

So Oj.dump and to_json are honouring the default options, but those are not being applied to Oj.generate and JSON.generate (as they used to do up to version 3.11.3).

@ohler55
Copy link
Owner

ohler55 commented Oct 23, 2021

I'll try and look tomorrow.

@ohler55
Copy link
Owner

ohler55 commented Oct 24, 2021

Similar results but added the rails ActiveSupport::JSON.encode call. As for the JSON.generate and Oj.generate, they fall into the JSON domain which does not support the alternate encoding. I know supporting BigDecimal for rails was a deviation from the rails behaviour but I justified that as rails is inconsistent anyway so why not provide the option. For the JSON gem users have been a bit more picky and file issues if there are any deviations.

require 'rails'
require 'oj'

$data = {:ticker=>"ASAI3", :price=>18.7, :rate=>-0.8.to_d}

def encode
  p "JSON.generate: #{JSON.generate($data)}"
  p "Oj.generate: #{Oj.generate($data)}"
  p "Oj.dump: #{Oj.dump($data)}"
  p "to_json: #{$data.to_json}"
  p "ActiveSupport::JSON.encode: #{ActiveSupport::JSON.encode($data)}"
end

puts "With Oj version (#{Oj::VERSION})"

puts
puts "Before optimizing"
encode

Oj.optimize_rails
Oj.default_options = {
  mode: :rails,
  bigdecimal_as_decimal: true,
  bigdecimal_load: true
}

puts
puts "After optimizing"
encode

results in:

With Oj version (3.13.9)

Before optimizing
"JSON.generate: {\"ticker\":\"ASAI3\",\"price\":18.7,\"rate\":\"-0.8\"}"
"Oj.generate: {\"ticker\":\"ASAI3\",\"price\":18.7,\"rate\":\"-0.8\"}"
"Oj.dump: {\"ticker\":\"ASAI3\",\"price\":18.7,\"rate\":\"-0.8\"}"
"to_json: {\"ticker\":\"ASAI3\",\"price\":18.7,\"rate\":\"-0.8\"}"
"ActiveSupport::JSON.encode: {\"ticker\":\"ASAI3\",\"price\":18.7,\"rate\":\"-0.8\"}"

After optimizing
"JSON.generate: {\"ticker\":\"ASAI3\",\"price\":18.7,\"rate\":\"-0.8\"}"
"Oj.generate: {\"ticker\":\"ASAI3\",\"price\":18.7,\"rate\":\"-0.8\"}"
"Oj.dump: {\"ticker\":\"ASAI3\",\"price\":18.7,\"rate\":-0.8}"
"to_json: {\"ticker\":\"ASAI3\",\"price\":18.7,\"rate\":-0.8}"
"ActiveSupport::JSON.encode: {\"ticker\":\"ASAI3\",\"price\":18.7,\"rate\":-0.8}"

@mrbongiolo
Copy link
Contributor Author

mrbongiolo commented Oct 24, 2021

So, OJ.generate and JSON.generate will not honour the default options in the current versions? Is this the desired behaviour? If so I can probably update the docs for :bigdecimal_as_decimal explaining that "odd" case.

But TBH it seems like a bug to me, as those methods should generate the same JSON output.
My initial issue was that another gem usesJSON.generate when passing some params to a queue and some of the results would be inconsistent with parts of the application that use Oj.dump or to_json.

Correct me if I'm wrong, but aren't the options there to change the gem behaviour? If I do set that option, I do it knowing that it might not generate strictly similar JSON results to vanilla JSON or Rails, but I'm fine with it. If someone required strict results wouldn't it be just a matter of not using an option that's there to change it?

@ohler55
Copy link
Owner

ohler55 commented Oct 25, 2021

It is the desire behaviour that Oj.generate and JSON.generate do not honour the defaults that are not settable for the JSON gem. The current documents on modes describe what options are honored with the :compat mode.

The :compat mode is meant to be the same as the JSON gem. There have been numerous issues submitted in the past where default options changed the behaviour of the :compat mode. To allow those options to have an effect now would most likely break a lot of code and result in numerous issues. The :custom mode is intended for fully customizing Oj. Using any other mode restricts the options that are available. In hindsight, global defaults were a mistake. The latest Oj::Parser deals with that for parsing but I have not implemented a similar approach for encoding yet.

Tje options are indeed there to change the gem behaviour and in :custom mode all the options are available. When running in the JSON gem compatibility mode those options are restricted to avoid deviating from the JSON gem behaviour even when using a different encoding mode for other operations in the same applications.

Finally, the JSON gem behaviour is not consistent between JSON.dump, JSON.generate, JSON.unmarshal, JSON.fast_generate, JSON.pretty_generate, JSON.unparse, and others. Each has a unique set of differences that Oj must implement in order to be a drop in replacement. Not only that but the JSON gem behaviour is a moving target so older versions are not the same as the latest. If Oj is to mimic the JSON gem it must have the same inconsistencies.

@mrbongiolo
Copy link
Contributor Author

Ok, I understand now, that's a lot of crazy stuff going with all those JSON implementations and whatnot.
So, I'll update this PR with a message explaining that Oj.generate and JSON.generate won't honour that option, just if someone else also gets affected by this.
On my side, I'll try to use :custom mode and see if I can make it work as required for our application.

@mrbongiolo
Copy link
Contributor Author

@ohler55 PR updated

pages/Modes.md Outdated Show resolved Hide resolved
@ohler55 ohler55 merged commit b11929b into ohler55:develop Oct 28, 2021
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

Successfully merging this pull request may close these issues.

None yet

2 participants