Skip to content
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

Type is not converted properly when creating polymorphic relationships #1160

Open
4 of 7 tasks
ArneZsng opened this issue May 14, 2018 · 2 comments
Open
4 of 7 tasks

Comments

@ArneZsng
Copy link
Contributor

ArneZsng commented May 14, 2018

This issue is a (choose one):

  • Problem/bug report.
  • Feature request.
  • Request for support. Note: Please try to avoid submitting issues for support requests. Use Gitter instead.

Checklist before submitting:

  • I've searched for an existing issue.
  • I've asked my question on Gitter and have not received a satisfactory answer.
  • I've included a complete bug report template. This step helps us and allows us to see the bug without trying to reproduce the problem from your description. It helps you because you will frequently detect if it's a problem specific to your project.
  • The feature I'm asking for is compliant with the JSON:API spec.

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 bdcbf61#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
Copy link
Contributor Author

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

@4ndv
Copy link

4ndv commented Feb 11, 2022

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants