ohler55 / oj

Optimized JSON
http://www.ohler.com/oj
MIT License
3.12k stars 250 forks source link

Values cast to string #376

Closed coding-bunny closed 6 years ago

coding-bunny commented 7 years ago

Hello,

I've run across a weird behaviour that came when I upgraded my version of oj. We were originally on version 2.18.5, and I'm trying to upgrade now to 3.0.5.

However, with the upgrade a lot of our rspec tests suddenly started failing. Actually those all related to jbuilder and generating json views. The reason they fail, is because we expect for example the value 40.0 to be returned, but the template now returned "40.0" instead.

For some reason, by simply upgrading the gem all values get cast to strings. And it only seems to be happening for BigDecimal values actually.

Is this an intended change?

ohler55 commented 7 years ago

I do seem to remember being surprised that the json gem and Rails use a string format for BigDecimal. Oj now does its best to mimic both so it now returns a string as well. You can change that by implementing a as_json method on BigDecimal that returns a number. Could get you into trouble if you really need a BigDecimal though. This assume you are using either rails. If not the :custom mode gives you lots more options.

coding-bunny commented 7 years ago

Okay, will talk it over in the team and see if we're okay with handling strings that way. It's mostly display in our cases, as we don't really compare then values to anything

ohler55 commented 7 years ago

👍

coding-bunny commented 7 years ago

Is there a way to set this globally with an initializer? I've created config/initializers/oj.rb and added the following:

# frozen_string_literal: true
#
# Initializer and Configuration for the Oj gem.
# Needed since Rails 5 comes with defaults that the Oj gem is following.
require 'oj'

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

But doesn't seem to be doing what I want. Am I missing something?

ohler55 commented 7 years ago

There is a page describing what options are available in what mode http://www.ohler.com/oj/doc/file.Modes.html. To mimic Rails correctly the big decimal_as_decimal can not be honored as ActiveSupport does not have that option.

coding-bunny commented 7 years ago

tried cycling through all modes, but none result in what I want. They all seem to be returning the value as a string...

ohler55 commented 7 years ago

Custom, strict, and null should return as a number. What is the code you are using?

On May 5, 2017 7:28:44 AM PDT, Arne De Herdt notifications@github.com wrote:

tried cycling through all modes, but none result in what I want. They all seem to be returning the value as a string...

ohler55 commented 7 years ago

Probably the code above. Missed on the cell phone.

ohler55 commented 7 years ago

I will look tonight.

coding-bunny commented 7 years ago

It's basically failing on our jbuilder templates. We have a template that creates a structure like so:

json.indicator_score do
  json.id @indicator_score.id
  json.score @indicator_score.score_value

  if @indicator_message
    json.indicator_message do |json|
      json.score @indicator_message.score
    end
  end
end

The score values are BigDecimal types, but always of a whole value, so 0.0, 20.0 or 40.0 for example. But always cast to string, and this happened when I tried to upgrade Oj

stereobooster commented 7 years ago

@coding-bunny How do you initiate Oj?

coding-bunny commented 7 years ago

euh...I think we just include it from the Gemfile. As far as I know we don't have any initializer or special code. Can check our application.rb if we configure it, but pretty sure we use the defaults.

stereobooster commented 7 years ago

jbuilder uses multi_json

https://github.com/intridea/multi_json/blob/master/lib/multi_json/adapters/oj.rb

  defaults :load, :mode => :strict, :symbolize_keys => false
  defaults :dump, :mode => :compat, :time_format => :ruby, :use_to_json => true

Maybe this is it?

coding-bunny commented 7 years ago

I'll look into it. Should be configurable somewhere I suppose.

stereobooster commented 7 years ago

Theoretically we need to make PR to multi_json repo. It should use json compatibility mode for Oj, I guess.

coding-bunny commented 7 years ago

I have honestly no idea :P But if that's what it takes to get things working, then I'm all for it. Unfortunately I'm off on vacation for 2 weeks, but happy to dive into this when I get back.

ohler55 commented 7 years ago

Let me summarize what I think this issue is about.

  1. Oj has changed and BigDecimals in :compat and :rails mode emit BigDecimals as strings the same way the json gem and Rails encode. There is a desire to deviate from Rails and emit the BigDecimal as a number instead.

  2. I checked the unit tests for the Oj :custom and :strict modes verify that in those modes BigDecimals are dumped as numbers.

  3. MultiJson is being used which sets the encoding to :compat mode.

Proposed solutions:

  1. You can over-ride BigDecimal to_json and as_json to return numbers instead of strings.

  2. I'll create a PR for MultiJson to change to using the updated behavior. It might be after Tuesday though.

flajann2 commented 7 years ago

Is not BigDecimal rolled into Integer for 2.4.1? I am looking to use Oj in a current project, but I need numbers to be numbers, not strings, and no issues with the deprecated BigNumber, and would be loathe to monkeypatch it.

stereobooster commented 7 years ago

Is not BigDecimal rolled into Integer for 2.4.1?

@flajann2 can you provide link, so I can read more about that.

What I see now is:

ruby -v
ruby 2.3.4p301 (2017-03-30 revision 58214) [x86_64-darwin15]
irb
irb(main):001:0> FIXNUM_MAX = (2**(0.size * 8 -2) -1)
=> 4611686018427387903
irb(main):002:0> FIXNUM_MAX.class.name
=> "Fixnum"
irb(main):003:0> (FIXNUM_MAX+1).class.name
=> "Bignum"

I am looking to use Oj in a current project, but I need numbers to be numbers, not strings, and no issues with the deprecated BigNumber, and would be loathe to monkeypatch it.

What will consume your JSON? If it is JS - it will not be able to consume BigDecimal numbers without losing the precision, because it uses Float64 internally for all numbers. Rails oriented for web development and assumes JS contributor, that is why they chose to convert BigD to strings.

flajann2 commented 7 years ago

I'm doing it for an embedded system, and will be passing JSON messages around the nanoservices running there.

ohler55 commented 7 years ago

If you are using JSON in an embedded system why would you use Rails? If not using rails then use the Oj custom mode. All options are available there and it performs better.

flajann2 commented 7 years ago

Not using Rails. Sinatra, actually. So Oj custom mode. Cool.

ohler55 commented 7 years ago

Let me know it it works for you.

ohler55 commented 6 years ago

Can this be closed?

coding-bunny commented 6 years ago

can for me. We succeeded in upgrading Oj by casting our values to floats. Turns out we didn't need the decimal type to store the values, so went for an easier approach.

ohler55 commented 6 years ago

Thanks

runlevel5 commented 5 years ago

@ohler55 thanks for clarifying the behaviour. I would like to ask if you could update the CHANGELOG with notes on this breaking changes? I am sure it would come in handy especially for developers with app regressions after updating to new v3 version.

ohler55 commented 5 years ago

I thought I had it covered. Maybe you can help me. What else would you like to see in the CHANGELOG.md?

runlevel5 commented 5 years ago

@ohler55 I went through the CHANGELOG but could not find any part that mentioning this changes. I can submit a PR against 3.0.0 CHANGELOG to inform user to do lookup the compat and rails mode for breaking changes compatability

ohler55 commented 5 years ago

If you would like to submit a PR I would be glad to accept it. If not I can update to indicate a look at the mode is suggested.