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

Type is not converted properly when creating polymorphic relationships #1160

Open ArneZsng opened 6 years ago

ArneZsng commented 6 years ago

This issue is a (choose one):

Checklist before submitting:

Description

Find the complete gist here: https://gist.github.com/ArneZsng/9a5542376d359755f62bbea5f16fb757 or below.

When creating a polymorphic relationship, the type is not properly converted anymore to a class name. This originally was the case in https://github.com/cerebris/jsonapi-resources/pull/288/files#diff-e6cf31c573bd5f2277b1f16812943df4R202 but was since removed with https://github.com/cerebris/jsonapi-resources/commit/bdcbf615155b8c6b3f34136d56d77254ed7d9cbf#diff-e6cf31c573bd5f2277b1f16812943df4L324

Instead of straight accepting the incoming type string, it should first be converted to a class name.

A fix would be to change _replace_polymorphic_to_one_link in https://github.com/cerebris/jsonapi-resources/blob/master/lib/jsonapi/resource.rb to this:

  def _replace_polymorphic_to_one_link(relationship_type, key_value, key_type, _options)
    relationship = self.class._relationships[relationship_type.to_sym]

    send("#{relationship.foreign_key}=", type: self.class.model_name_for_type(key_type), id: key_value)
    @save_needed = true

    :completed
  end

Bug Report

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: 'test-rails-5',
      platform: :jruby

  if ENV['JSONAPI_RESOURCES_PATH']
    gem 'jsonapi-resources', path: ENV['JSONAPI_RESOURCES_PATH'], require: false
  else
    gem 'jsonapi-resources', git: 'https://github.com/cerebris/jsonapi-resources', 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 :documents, force: true do |t|
    t.string :name
    t.belongs_to :owner, index: true, polymorphic: true, null: false
  end

  create_table :contacts, force: true do |t|
    t.string :name
  end
end

# create models
class Document < ActiveRecord::Base
  belongs_to :owner, polymorphic: true, inverse_of: :documents
end

class Contact < ActiveRecord::Base
  has_many :documents, as: :owner, inverse_of: :owner, dependent: :destroy
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 DocumentsController < ApplicationController
  include JSONAPI::ActsAsResourceController
end

class DocumentResource < JSONAPI::Resource
  attribute :name
  has_one :owner, polymorphic: true

  # This fixes the issue
  # def _replace_polymorphic_to_one_link(relationship_type, key_value, key_type, _options)
  #   relationship = self.class._relationships[relationship_type.to_sym]

  #   send("#{relationship.foreign_key}=", type: self.class.model_name_for_type(key_type), id: key_value)
  #   @save_needed = true

  #   :completed
  # end
end

class ContactResource < JSONAPI::Resource
  attribute :name
  has_many :documents
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 :documents, 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_create_documents
    contact = Contact.create! name: 'John Doe'
    json_request = {
        'data' => {
            type: 'documents',
            attributes: {
                name: 'Test Document'
            },
            relationships: {
              owner: {
                data: { id: contact.id, type: 'contacts' }
              }
            }
        }
    }
    post '/documents', json_request.to_json, json_api_headers
    assert last_response.created?
    document = Document.find_by(name: 'Test Document')
    refute_nil document
    assert document.owner, contact
  end

  private

  def app
    Rails.application
  end
end

Output

# Running:

D, [2018-05-14T16:26:26.781708 #36247] DEBUG -- :    (0.1ms)  begin transaction
D, [2018-05-14T16:26:26.784360 #36247] DEBUG -- :   Contact Create (0.2ms)  INSERT INTO "contacts" ("name") VALUES (?)  [["name", "John Doe"]]
D, [2018-05-14T16:26:26.784767 #36247] DEBUG -- :    (0.1ms)  commit transaction
I, [2018-05-14T16:26:26.800330 #36247]  INFO -- : Started POST "/documents" for 127.0.0.1 at 2018-05-14 16:26:26 +0200
I, [2018-05-14T16:26:26.804801 #36247]  INFO -- : Processing by DocumentsController#create as HTML
I, [2018-05-14T16:26:26.805689 #36247]  INFO -- :   Parameters: {"data"=>{"type"=>"documents", "attributes"=>{"name"=>"Test Document"}, "relationships"=>{"owner"=>{"data"=>{"id"=>1, "type"=>"contacts"}}}}}
D, [2018-05-14T16:26:26.808314 #36247] DEBUG -- :    (0.3ms)  begin transaction
D, [2018-05-14T16:26:26.855114 #36247] DEBUG -- :   Document Create (0.2ms)  INSERT INTO "documents" ("name", "owner_type", "owner_id") VALUES (?, ?, ?)  [["name", "Test Document"], ["owner_type", "contacts"], ["owner_id", 1]]
D, [2018-05-14T16:26:26.856951 #36247] DEBUG -- :    (0.1ms)  SELECT documents.id FROM "documents" WHERE "documents"."id" = ?  [["id", 1]]
D, [2018-05-14T16:26:26.857927 #36247] DEBUG -- :   Document Load (0.1ms)  SELECT "documents".* FROM "documents" WHERE "documents"."id" = ?  [["id", 1]]
D, [2018-05-14T16:26:26.860960 #36247] DEBUG -- :    (0.6ms)  commit transaction
I, [2018-05-14T16:26:26.867821 #36247]  INFO -- :   Rendering text template
I, [2018-05-14T16:26:26.868008 #36247]  INFO -- :   Rendered text template (0.0ms)
I, [2018-05-14T16:26:26.868847 #36247]  INFO -- : Completed 201 Created in 62ms (Views: 7.5ms)

D, [2018-05-14T16:26:26.871419 #36247] DEBUG -- :   Document Load (0.1ms)  SELECT  "documents".* FROM "documents" WHERE "documents"."name" = ? LIMIT ?  [["name", "Test Document"], ["LIMIT", 1]]
E

Error:
BugTest#test_create_documents:
NameError: wrong constant name contacts

bin/rails test set_polymorphic_association.rb:132

Finished in 0.108288s, 9.2346 runs/s, 18.4693 assertions/s.
1 runs, 2 assertions, 0 failures, 1 errors, 0 skips
ArneZsng commented 6 years ago

@lgebhardt Happy to provide the PR if you agree with the fix.

4ndv commented 2 years ago

Just occured to me while updating from 0.9 to 0.10. Is there any chance that it will be fixed anytime soon?

Monkey-patched this and #1305 like this, but I haven't dived into JR internals too much to evaluate the risks of such patches:

      def self._polymorphic_types
        @poly_hash ||= {}.tap do |hash|
          ActiveRecord::Base.descendants do |klass|
            next unless Module === klass

            klass.reflect_on_all_associations(:has_many).select{|r| r.options[:as] }.each do |reflection|
              (hash[reflection.options[:as]] ||= []) << klass.name.underscore
            end
          end
        end
        @poly_hash[_polymorphic_name.to_sym]
      end

      def _replace_polymorphic_to_one_link(relationship_type, key_value, key_type, _options)
        relationship = self.class._relationships[relationship_type.to_sym]

        send("#{relationship.foreign_key}=", type: key_type.to_s.classify, id: key_value)
        @save_needed = true

        :completed
      end