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

Values cast to string #376

Closed
coding-bunny opened this Issue May 5, 2017 · 30 comments

Comments

Projects
None yet
5 participants
@coding-bunny
Copy link

coding-bunny commented May 5, 2017

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

This comment has been minimized.

Copy link
Owner

ohler55 commented May 5, 2017

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

This comment has been minimized.

Copy link
Author

coding-bunny commented May 5, 2017

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

This comment has been minimized.

Copy link
Owner

ohler55 commented May 5, 2017

👍

@coding-bunny

This comment has been minimized.

Copy link
Author

coding-bunny commented May 5, 2017

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

This comment has been minimized.

Copy link
Owner

ohler55 commented May 5, 2017

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

This comment has been minimized.

Copy link
Author

coding-bunny commented May 5, 2017

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

@ohler55

This comment has been minimized.

Copy link
Owner

ohler55 commented May 5, 2017

@ohler55

This comment has been minimized.

Copy link
Owner

ohler55 commented May 5, 2017

Probably the code above. Missed on the cell phone.

@ohler55

This comment has been minimized.

Copy link
Owner

ohler55 commented May 5, 2017

I will look tonight.

@coding-bunny

This comment has been minimized.

Copy link
Author

coding-bunny commented May 5, 2017

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

This comment has been minimized.

Copy link
Contributor

stereobooster commented May 5, 2017

@coding-bunny How do you initiate Oj?

@coding-bunny

This comment has been minimized.

Copy link
Author

coding-bunny commented May 5, 2017

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

This comment has been minimized.

Copy link
Contributor

stereobooster commented May 5, 2017

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

This comment has been minimized.

Copy link
Author

coding-bunny commented May 5, 2017

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

@stereobooster

This comment has been minimized.

Copy link
Contributor

stereobooster commented May 5, 2017

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

@coding-bunny

This comment has been minimized.

Copy link
Author

coding-bunny commented May 5, 2017

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

This comment has been minimized.

Copy link
Owner

ohler55 commented May 6, 2017

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

This comment has been minimized.

Copy link

flajann2 commented Jun 10, 2017

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

This comment has been minimized.

Copy link
Contributor

stereobooster commented Jun 10, 2017

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

This comment has been minimized.

Copy link

flajann2 commented Jun 10, 2017

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

@ohler55

This comment has been minimized.

Copy link
Owner

ohler55 commented Jun 10, 2017

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

This comment has been minimized.

Copy link

flajann2 commented Jun 11, 2017

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

@ohler55

This comment has been minimized.

Copy link
Owner

ohler55 commented Jun 11, 2017

Let me know it it works for you.

@ohler55

This comment has been minimized.

Copy link
Owner

ohler55 commented Jul 11, 2017

Can this be closed?

@coding-bunny

This comment has been minimized.

Copy link
Author

coding-bunny commented Jul 11, 2017

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

This comment has been minimized.

Copy link
Owner

ohler55 commented Jul 11, 2017

Thanks

@ohler55 ohler55 closed this Jul 11, 2017

@joneslee85

This comment has been minimized.

Copy link

joneslee85 commented Nov 29, 2018

@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

This comment has been minimized.

Copy link
Owner

ohler55 commented Nov 29, 2018

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

@joneslee85

This comment has been minimized.

Copy link

joneslee85 commented Nov 29, 2018

@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

This comment has been minimized.

Copy link
Owner

ohler55 commented Nov 29, 2018

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.