cerebris / jsonapi-resources

A resource-focused Rails library for developing JSON:API compliant servers.
http://jsonapi-resources.com
MIT License
2.32k stars 532 forks source link

Non datastore, dynamic related resources broken in version `v0.9.0` #1169

Open SamMarkGoldman opened 6 years ago

SamMarkGoldman commented 6 years ago

This issue is a (choose one):

Checklist before submitting:

Description

It seems that dynamic related resources, a feature we use heavily here at Blue Apron, is broken coming from v0.8.0 to v0.9.0. It looks like in the older version, related resources which don't provide a foreign key in the owner model resolved by calling a method on that model with the related resource's name. This same behavior is part of the active record convention: https://github.com/cerebris/jsonapi-resources/blob/v0.8.0/lib/jsonapi/resource_serializer.rb#L301

However in the new version (v0.9.0), this functionality is no longer supported in the ToOne relationship child class: https://github.com/cerebris/jsonapi-resources/blob/854ec705183d744565106cf34b12a741139d157e/lib/jsonapi/relationship.rb#L89 Instead, relationships are based on the assumption that related resources are datastore backed, and use foreign keys rather than methods to relate to one another.

The below test passes when specifying the older version of the library, and fails when specifying the newer version. The example is stripped down (and thus a little contrived), but I assure you one can support JSONAPI compliant resources using this method.

Bug reports:

begin
  require 'bundler/inline'
  require 'bundler'
rescue LoadError => e
  STDERR.puts 'Bundler version 1.10 or later is required. Please update your Bundler'
  raise e
end

gemfile(true, ui: ENV['SILENT'] ? Bundler::UI::Silent.new : Bundler::UI::Shell.new) do
  source 'https://rubygems.org'

  gem 'rails', require: false
  gem 'sqlite3', platform: :mri

  # gem 'activerecord-jdbcsqlite3-adapter',
  #     git: 'https://github.com/jruby/activerecord-jdbc-adapter',
  #     branch: 'rails-5',
  #     platform: :jruby

  if ENV['JSONAPI_RESOURCES_PATH']
    gem 'jsonapi-resources', path: ENV['JSONAPI_RESOURCES_PATH'], require: false
  else
    # this test works at v0.8.0, but fails at v0.9.0
    # gem 'jsonapi-resources', git: 'https://github.com/cerebris/jsonapi-resources', :tag => 'v0.8.0', require: false
    gem 'jsonapi-resources', git: 'https://github.com/cerebris/jsonapi-resources', :tag => 'v0.9.0', require: false
  end

end

# prepare active_record database
require 'active_record'

class NullLogger < Logger
  def initialize(*_args)
  end

  def add(*_args, &_block)
  end
end

ActiveRecord::Base.establish_connection(adapter: 'sqlite3', database: ':memory:')
ActiveRecord::Base.logger = ENV['SILENT'] ? NullLogger.new : Logger.new(STDOUT)
ActiveRecord::Migration.verbose = !ENV['SILENT']

ActiveRecord::Schema.define do
  # Add your schema here
  create_table :your_models, force: true do |t|
    t.string :name
  end
end

# create models
class YourModel < ActiveRecord::Base

  def my_related_model
    MyRelatedModel.new
  end
end

class MyRelatedModel
  include ActiveModel::Model

  attr_accessor :id, :data

  def initialize
    @id = 1
    @data = 'snoopy'
  end
end

# prepare rails app
require 'action_controller/railtie'
# require 'action_view/railtie'
require 'jsonapi-resources'

class ApplicationController < ActionController::Base
end

# prepare jsonapi resources and controllers
class YourModelsController < ApplicationController
  include JSONAPI::ActsAsResourceController
end

class YourModelResource < JSONAPI::Resource
  attribute :name
  filter :name

  has_one :my_related_model, eager_load_on_include: false
end

class MyRelatedModelResource < JSONAPI::Resource
  attribute :data
end

class TestApp < Rails::Application
  config.root = File.dirname(__FILE__)
  config.logger = ENV['SILENT'] ? NullLogger.new : Logger.new(STDOUT)
  Rails.logger = config.logger

  secrets.secret_token = 'secret_token'
  secrets.secret_key_base = 'secret_key_base'

  config.eager_load = false
end

# initialize app
Rails.application.initialize!

JSONAPI.configure do |config|
  config.json_key_format = :underscored_key
  config.route_format = :underscored_key
end

# draw routes
Rails.application.routes.draw do
  jsonapi_resources :your_models, only: [:index, :create]
end

# prepare tests
require 'minitest/autorun'
require 'rack/test'

# Replace this with the code necessary to make your test fail.
class BugTest < Minitest::Test
  include Rack::Test::Methods

  def json_api_headers
    {'Accept' => JSONAPI::MEDIA_TYPE, 'CONTENT_TYPE' => JSONAPI::MEDIA_TYPE}
  end

  def test_index_your_models
    record = YourModel.create! name: 'John Doe'
    get '/your_models', {include: :my_related_model}, json_api_headers
    assert last_response.ok?
    json_response = JSON.parse(last_response.body)

  end

  private

  def app
    Rails.application
  end
end
jcsrb commented 5 years ago

I can confirm this issue exists in 0.9.10

specifically in processing the include only polymorphic are taken into consideration https://github.com/cerebris/jsonapi-resources/blob/v0.9.10/lib/jsonapi/resource.rb#L1283-L1288

so as as a workaround setting polymorphic: true actually works even if the has_one relationship to a non-AR backed resource and the resource is included non-eagerly.