#as_json ignored on Rails 7.2.0

[benedikt]

Reading through the other issues, I'm sorry to break the news, but it looks like your favorite issue with #to_json and #as_json is back with the recent release of Rails 7.2.0. The issue doesn't show up with Rails 7.1.x, but looking through the Rails source code it isn't immediately apparent what changed.

The issue shows up upon calling Oj.mimic_JSON. Adding or removing Oj.optimize_rails, doesn't change the behavior.

require 'bundler/inline'

gemfile do
  source 'https://rubygems.org'

  gem 'rails', '7.2.0' # No issues with 7.1.0
  gem 'oj', '3.16.5'
end

Oj.mimic_JSON # This seems to cause the issue
Oj.optimize_rails # Does not have an effect on the issue

class TestObject
  def as_json
    { foo: :bar }
  end
end

puts TestObject.new.to_json

Expected output:

{"foo":"bar"}

Actual output:

"#<Test:0x000000012024e9d0>"

Thanks a lot for this gem! I've been using it without any problems for years now 💛

[ohler55]

Sigh, Rails is a moving target. I'll look into it. Thanks for the test case. That helps a lot.

[benedikt]

This change looks like the most likely culprit (at least to me), right now: rails/rails@8e251a0

Might be a red herring, though.

There are similar issues on other projects, though: rails/rails#52519

[ohler55]

Looks like a likely cause. Just highlights the dangers of monkey patching. This will take some head scratching to find a work around.

[benedikt]

I'm guessing the issue is caused by redefining to_json on Object in here?

oj/ext/oj/mimic_json.c

Line 908 in f5e928a

rb_define_method(rb_cObject, "to_json", mimic_object_to_json, -1);

Would it be an option to define a new module with the method on it and include it in Object? I think this is how the json gem does it as well.

require 'json' 

Object.ancestors
# => [Object, JSON::Ext::Generator::GeneratorMethods::Object, PP::ObjectMixin, Kernel, BasicObject]

There would still be the problem of requiring everything in the right order, but I guess if Oj.mimic_JSON was added to application's boot.rb it would work? Alternatively, having a setup like this in the Gemfile would work as well.

gem 'oj', require: 'oj/mimic_json' # Make sure this comes before the rails gem
gem 'rails'

Admittedly, these are all not quite as elegant as the current way.

The other way I could think of is prepending the module to JSON::Ext::Generator::GeneratorMethods::Object if it's around and including it in Object if it's not around.

[Watson1978]

Rail 7.1 uses prepend to override the to_json method
https://github.com/rails/rails/blob/d39db5d1891f7509cde2efc425c9d69bbb77e670/activesupport/lib/active_support/core_ext/object/json.rb#L48-L50

Rail 7.2 uses inlucde instead to override the to_json method
https://github.com/rails/rails/blob/fb6c4305939da06efdf2893d99130e7829c53e8b/activesupport/lib/active_support/core_ext/object/json.rb#L48-L50

In Rails 7.1, active_support's to_json would be overridden at the last.
Therefore, the to_json of active_support is called with TestObject.new.to_json.

However, in Rails 7.2, seems it is not.

[Watson1978]

Not sure if this is correct, but it seems to work with the following changes:

diff --git a/ext/oj/mimic_json.c b/ext/oj/mimic_json.c
index 35ad089..1bf6757 100644
--- a/ext/oj/mimic_json.c
+++ b/ext/oj/mimic_json.c
@@ -756,7 +756,8 @@ static VALUE mimic_object_to_json(int argc, VALUE *argv, VALUE self) {
     oj_out_init(&out);

     out.omit_nil  = copts.dump_opts.omit_nil;
-    copts.mode    = CompatMode;
+    // The mode in oj_default_options will be changed to RailMode by rails_mimic_json
+    // copts.mode    = CompatMode;
     copts.to_json = No;
     if (1 <= argc && Qnil != argv[0]) {
         oj_parse_mimic_dump_options(argv[0], &copts);
diff --git a/ext/oj/rails.c b/ext/oj/rails.c
index 91fed0d..d2a04a8 100644
--- a/ext/oj/rails.c
+++ b/ext/oj/rails.c
@@ -803,7 +803,7 @@ rails_mimic_json(VALUE self) {
     }
     oj_mimic_json_methods(json);
     // Setting the default mode breaks the prmoise in the docs not to.
-    // oj_default_options.mode = RailsMode;
+    oj_default_options.mode = RailsMode;

     return Qnil;
 }

[ohler55]

Does that change break other tests? Does it break Rails 7.1 support?

[Watson1978]

This way it would always be necessary to call both Oj.mimic_JSON and Oj.optimize_rails.

Oj.mimic_JSON # This seems to cause the issue
Oj.optimize_rails # Does not have an effect on the issue

With Rails 7.1, it looks like only Oj.mimic_JSON is needed.

[Watson1978]

Maybe, it would be better not to override to_json in Oj.

[Watson1978]

I think it is no problem on Rails with above plane.

require 'json'
require 'oj'

Oj.mimic_JSON

However, for above code, we may want to override the to_json defined by the json gem without rails

[Watson1978]

It may be somewhat evil, but it may be better if to_json is defined when ActiveSupport is not existed.

=> 1bda7ab

[ohler55]

The check for ActiveSupport might be best.

[benedikt]

Can confirm that this fixes the issue on Rails 7.2 👍

[Roguelazer]

Any chance of a new release with this commit in it? The way this breaks applications on rails 7.2 is pretty hard to debug (it turns out a dependency was including oj and setting mimic_json).

[ohler55]

I'll release tonight.

[ohler55]

Released.