ohler55 / oj

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

"Optimized" implementations of ActiveSupport methods not used with `render json: ...` #913

Open KJTsanaktsidis opened 5 months ago

KJTsanaktsidis commented 5 months ago

I believe it's a fairly common idiom in Rails to return JSON by writing render json: some_object in a controller. That causes Rails to call some_object.to_json, and thus produce a json string to return.

Calling #to_json like this invokes Oj::Rails::Encoder when Oj.optimize_rails has been set up, which is what we want. However, the problem is that Rails actually calls to_json with a non-nil, non-empty options hash (it seems to have several actionview related keys like :prefixes, :template, and :layout, from my observation). Oj realises that it was called with options it doesn't understand, and thus saves those options here and forces a real call to the object's #as_json method with them here (because argc > 0. That meas real ActiveSupport Hash#as_json gets called, which actually makes a recursive duplicate of the hash - very expensive and not nescessary!

In our codebase, I'm doing something like the following to strip out these options in the call to to_json, and ensure that Oj can use the optimized, inlined implementations of those methods:

  module JSONActionControllerRenderer
    ACTIONVIEW_RENDERER_KEYS = Set.new(%i[prefixes template layout]).freeze

    RENDER_FUNC = ->(json, options) do
      # Make sure we continue handling JSONP callbacks - the renderer needs this option, but Oj doesn't.
      callback = options.delete(:callback)
      # This bit makes the options go away, and Oj fast, if there are no options we don't understand.
      options = nil if options.all? { |key, _value| ACTIONVIEW_RENDERER_KEYS.include?(key) }
      json = json.to_json(options) unless json.is_a?(String)

      if callback.present?
        if media_type.nil? || media_type == Mime[:json]
          self.content_type = Mime[:js]
        end

        "/**/#{callback}(#{json})"
      else
        self.content_type = Mime[:json] if media_type.nil?
        json
      end
    end

    # Called from config/initializers/json.rb to register the renderer
    def self.register!
      ActionController.remove_renderer :json
      ActionController.add_renderer :json, &RENDER_FUNC
    end
  end

It would be good if this problem was solved "out of the box" somehow in Oj though. Options include:

Do we think this is something we can solve in Oj?