Skip to content

Change how renderer output is converted to JSON #1

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

Merged
merged 3 commits into from
Sep 27, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
## Dummy app temp files:
/spec/dummy/db
/spec/dummy/log
/spec/dummy/tmp

## Specific to RubyMotion:
.dat*
Expand Down
1 change: 1 addition & 0 deletions jsonapi-rails.gemspec
Original file line number Diff line number Diff line change
Expand Up @@ -23,4 +23,5 @@ Gem::Specification.new do |spec|
spec.add_development_dependency 'rspec-rails', '~> 3.5'
spec.add_development_dependency 'with_model', '~> 2.0'
spec.add_development_dependency 'simplecov'
spec.add_development_dependency 'pry-byebug'
end
74 changes: 70 additions & 4 deletions lib/jsonapi/rails/railtie.rb
Original file line number Diff line number Diff line change
Expand Up @@ -54,10 +54,76 @@ def register_renderers
# Renderer proc is evaluated in the controller context.
headers['Content-Type'] = Mime[:jsonapi].to_s

ActiveSupport::Notifications.instrument('render.jsonapi-rails',
resources: resources,
options: options) do
JSON.generate(renderer.render(resources, options, self))
ActiveSupport::Notifications.instrument(
'render.jsonapi-rails',
resources: resources,
options: options
) do
# Depending on whether or not a valid cache object is present
# in the options, the #render call below will return two
# slightly different kinds of hash.
#
# Both hashes have broadly the following structure, where r is
# some representation of a JSON::API resource:
#
# {
# data: [ r1, r2, r3 ],
# meta: { count: 12345 },
# jsonapi: { version: "1.0" }
# }
#
# For non-cached calls to this method, the `data` field in the
# return value will contain an array of Ruby hashes.
#
# For cached calls, the `data` field will contain an array of
# JSON strings corresponding to the same data. This happens
# because jsonapi-renderer caches both the JSON serialization
# step as well as the assembly of the relevant attributes into
# a JSON::API-compliant structure. Those JSON strings are
# created via calls to `to_json`. They are then wrapped in
# CachedResourcesProcessor::JSONString. This defines a
# `to_json` method which simply returns self, ie - it attempts
# to ensure that any further `to_json` calls result in no
# changes.
#
# That isn't what happens in a Rails context, however. Below,
# the last step is to convert the entire output hash of the
# renderer into a JSON string to send to the client. If we
# call `to_json` on the cached output, the already-made JSON
# strings in the `data` field will be converted again,
# resulting in malformed data reaching the client. This happens
# because the ActiveSupport `to_json` takes precedent, meaning
# the "no-op" `to_json` definition on JSONString never gets
# executed.
#
# We can get around this by using JSON.generate instead, which
# will use the `to_json` defined on JSONString rather than the
# ActiveSupport one.
#
# However, we can't use JSON.generate on the non-cached output.
# Doing so means that its `data` field contents are converted
# with a non-ActiveSupport `to_json`. This means cached and
# non-cached responses have subtle differences in how their
# resources are serialized. For example:
#
# x = Time.new(2021,1,1)
#
# x.to_json
# => "\"2021-01-01T00:00:00.000+00:00\""
#
# JSON.generate x
# => "\"2021-01-01 00:00:00 +0000\""
#
# The different outputs mean we need to take different
# approaches when converting the entire payload into JSON,
# hence the check below.
jsonapi_hash = renderer.render(resources, options, self)

if jsonapi_hash[:data]&.first&.class == JSONAPI::Renderer::CachedResourcesProcessor::JSONString
JSON.generate jsonapi_hash
else
jsonapi_hash.to_json
end
end
end
end
Expand Down
53 changes: 44 additions & 9 deletions spec/render_jsonapi_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -23,32 +23,67 @@ def index
def serializer
Class.new(JSONAPI::Serializable::Resource) do
type 'users'
attribute :name
attributes :id, :name, :dob

def jsonapi_cache_key(*)
'foo'
'cache_key'
end
end
end

def user
OpenStruct.new(id: 1, name: 'Johnny Cache', dob: Time.new(2021,1,1))
end

def index
user = OpenStruct.new(id: 1, name: 'Lucas')
render jsonapi: [user],
class: { OpenStruct: serializer }
end

render jsonapi: user,
def index_with_caching
render jsonapi: [user],
class: { OpenStruct: serializer },
cache: Rails.cache
end
end

subject { JSON.parse(response.body) }
before do
routes.draw do
get "index_with_caching" => "anonymous#index_with_caching"
get "index" => "anonymous#index"
end
end

let(:rendered_json) { JSON.parse(response.body) }

it 'renders a JSON API success document' do
get :index
expect(Rails.cache.exist?('foo')).to be true
get :index
get :index_with_caching

expect(response.content_type).to eq('application/vnd.api+json')
expect(subject.key?('data')).to be true
expect(rendered_json.key?('data')).to be true
end

it 'caches resources' do
get :index_with_caching

expect(Rails.cache.exist?('cache_key')).to be true
expect(JSON.parse(Rails.cache.read('cache_key'))).to eq rendered_json['data'].first
end

it 'renders equivalent JSON whether caching or not' do
expected_response = {
"data"=>[{"id"=>"1", "type"=>"users", "attributes"=>{"id"=>1, "name"=>"Johnny Cache", "dob"=>"2021-01-01T00:00:00.000+00:00"}}],
"jsonapi"=>{"version"=>"1.0"}
}

get :index
response_with_no_caching = rendered_json.deep_dup

get :index_with_caching
response_with_caching = rendered_json

expect(response_with_no_caching).to eq expected_response
expect(response_with_caching).to eq expected_response
end
end
end