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

Add STI relationship option #1359

Open
wants to merge 2 commits into
base: release-0-10
Choose a base branch
from
Open
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
52 changes: 42 additions & 10 deletions lib/jsonapi/active_relation_resource.rb
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,9 @@ def find_to_populate_by_keys(keys, options = {})
def find_fragments(filters, options = {})
include_directives = options[:include_directives] ? options[:include_directives].include_directives : {}
resource_klass = self

fragments = {}

linkage_relationships = to_one_relationships_for_linkage(include_directives[:include_related])

sort_criteria = options.fetch(:sort_criteria) { [] }
Expand Down Expand Up @@ -129,19 +132,33 @@ def find_fragments(filters, options = {})
if linkage_relationship.polymorphic? && linkage_relationship.belongs_to?
linkage_relationship.resource_types.each do |resource_type|
klass = resource_klass_for(resource_type)
linkage_fields << {relationship_name: name, resource_klass: klass}

linkage_table_alias = join_manager.join_details_by_polymorphic_relationship(linkage_relationship, resource_type)[:alias]
primary_key = klass._primary_key

linkage_fields << {relationship_name: name,
linkage_relationship: linkage_relationship,
resource_klass: klass,
field: "#{concat_table_field(linkage_table_alias, primary_key)} AS #{linkage_table_alias}_#{primary_key}",
alias: "#{linkage_table_alias}_#{primary_key}"}

pluck_fields << Arel.sql("#{concat_table_field(linkage_table_alias, primary_key)} AS #{linkage_table_alias}_#{primary_key}")
end
else
klass = linkage_relationship.resource_klass
linkage_fields << {relationship_name: name, resource_klass: klass}

linkage_table_alias = join_manager.join_details_by_relationship(linkage_relationship)[:alias]
primary_key = klass._primary_key
pluck_fields << Arel.sql("#{concat_table_field(linkage_table_alias, primary_key)} AS #{linkage_table_alias}_#{primary_key}")

linkage_fields << {relationship_name: name,
linkage_relationship: linkage_relationship,
resource_klass: klass,
field: "#{concat_table_field(linkage_table_alias, primary_key)} AS \"#{linkage_table_alias}_#{primary_key}\"",
alias: "#{linkage_table_alias}_#{primary_key}"}

pluck_fields << Arel.sql("#{concat_table_field(linkage_table_alias, primary_key)} AS \"#{linkage_table_alias}_#{primary_key}\"")

if linkage_relationship.sti?
pluck_fields << Arel.sql("#{concat_table_field(linkage_table_alias, 'type')} AS \"#{linkage_table_alias}_type\"")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
pluck_fields << Arel.sql("#{concat_table_field(linkage_table_alias, 'type')} AS \"#{linkage_table_alias}_type\"")
pluck_fields << Arel.sql("#{concat_table_field(linkage_table_alias, klass.inheritance_column)} AS \"#{linkage_table_alias}_type\"")

right?

Copy link
Contributor

@JamesGlover JamesGlover Jun 9, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think at this point klass is the resource, not the ActiveRecord::Base class, so you'd also need to define an inheritance_column method on JSONAPI::BasicResource.

Although in practice you'd probably want to use _inheritance_column instead, to be consistent with _primary_key.

# lib/jsonapi/basic_resource.rb
def inheritance_column(key)
  @_inheritance_column = key.to_sym
end

def _inheritance_column
  @_inheritance_column ||= _default_inheritance_column
end

def _default_inheritance_column
  @_default_inheritance_column ||=_model_class.respond_to?(:inheritance_column) ? _model_class.inheritance_column : :type
end

end
end
end

Expand All @@ -158,7 +175,6 @@ def find_fragments(filters, options = {})
pluck_fields << Arel.sql(field)
end

fragments = {}
rows = records.pluck(*pluck_fields)
rows.each do |row|
rid = JSONAPI::ResourceIdentity.new(resource_klass, pluck_fields.length == 1 ? row : row[0])
Expand All @@ -175,7 +191,14 @@ def find_fragments(filters, options = {})
fragments[rid].initialize_related(linkage_field_details[:relationship_name])
related_id = row[attributes_offset]
if related_id
related_rid = JSONAPI::ResourceIdentity.new(linkage_field_details[:resource_klass], related_id)
if linkage_field_details[:linkage_relationship].sti?
type = row[2]
related_rid = JSONAPI::ResourceIdentity.new(resource_klass_for(type), related_id)
attributes_offset+= 1
else
related_rid = JSONAPI::ResourceIdentity.new(linkage_field_details[:resource_klass], related_id)
end

fragments[rid].add_related_identity(linkage_field_details[:relationship_name], related_rid)
end
attributes_offset+= 1
Expand Down Expand Up @@ -413,6 +436,10 @@ def find_related_monomorphic_fragments(source_rids, relationship, options, conne
Arel.sql("#{concat_table_field(resource_table_alias, resource_klass._primary_key)} AS #{resource_table_alias}_#{resource_klass._primary_key}")
]

if relationship.sti?
pluck_fields << Arel.sql("#{concat_table_field(resource_table_alias, 'type')} AS #{resource_table_alias}_type")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
pluck_fields << Arel.sql("#{concat_table_field(resource_table_alias, 'type')} AS #{resource_table_alias}_type")
pluck_fields << Arel.sql("#{concat_table_field(resource_table_alias, klass.inheritance_column)} AS #{resource_table_alias}_type")

right?

end

cache_field = resource_klass.attribute_to_model_field(:_cache_field) if options[:cache]
if cache_field
pluck_fields << Arel.sql("#{concat_table_field(resource_table_alias, cache_field[:name])} AS #{resource_table_alias}_#{cache_field[:name]}")
Expand Down Expand Up @@ -458,12 +485,17 @@ def find_related_monomorphic_fragments(source_rids, relationship, options, conne
fragments = {}
rows = records.distinct.pluck(*pluck_fields)
rows.each do |row|
rid = JSONAPI::ResourceIdentity.new(resource_klass, row[1])
if relationship.sti?
type = row[2]
rid = JSONAPI::ResourceIdentity.new(resource_klass_for(type), row[1])
attributes_offset = 3
else
rid = JSONAPI::ResourceIdentity.new(resource_klass, row[1])
attributes_offset = 2
end

fragments[rid] ||= JSONAPI::ResourceFragment.new(rid)

attributes_offset = 2

if cache_field
fragments[rid].cache = cast_to_attribute_type(row[attributes_offset], cache_field[:type])
attributes_offset+= 1
Expand Down
4 changes: 2 additions & 2 deletions lib/jsonapi/basic_resource.rb
Original file line number Diff line number Diff line change
Expand Up @@ -298,7 +298,7 @@ def _replace_to_many_links(relationship_type, relationship_key_values, options)
_create_to_many_links(relationship_type, to_add, {})

@reload_needed = true
elsif relationship.polymorphic?
elsif relationship.polymorphic? || relationship.sti?
relationship_key_values.each do |relationship_key_value|
relationship_resource_klass = self.class.resource_klass_for(relationship_key_value[:type])
ids = relationship_key_value[:ids]
Expand Down Expand Up @@ -1094,7 +1094,7 @@ def define_relationship_methods(relationship_name, relationship_klass, options)
end

def define_foreign_key_setter(relationship)
if relationship.polymorphic?
if relationship.polymorphic? || relationship.sti?
define_on_resource "#{relationship.foreign_key}=" do |v|
_model.method("#{relationship.foreign_key}=").call(v[:id])
_model.public_send("#{relationship.polymorphic_type}=", v[:type])
Expand Down
9 changes: 6 additions & 3 deletions lib/jsonapi/relationship.rb
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
module JSONAPI
class Relationship
attr_reader :acts_as_set, :foreign_key, :options, :name,
:class_name, :polymorphic, :always_include_optional_linkage_data,
:class_name, :polymorphic, :sti, :always_include_optional_linkage_data,
:parent_resource, :eager_load_on_include, :custom_methods,
:inverse_relationship, :allow_include

Expand All @@ -22,6 +22,7 @@ def initialize(name, options = {})
ActiveSupport::Deprecation.warn('Use polymorphic_types instead of polymorphic_relations')
@polymorphic_types ||= options[:polymorphic_relations]
end
@sti = options.fetch(:sti, false)

@always_include_optional_linkage_data = options.fetch(:always_include_optional_linkage_data, false) == true
@eager_load_on_include = options.fetch(:eager_load_on_include, false) == true
Expand All @@ -39,6 +40,8 @@ def initialize(name, options = {})
end

alias_method :polymorphic?, :polymorphic
alias_method :sti?, :sti

alias_method :parent_resource_klass, :parent_resource

def primary_key
Expand All @@ -63,12 +66,12 @@ def self.polymorphic_types(name)
next unless Module === klass
if ActiveRecord::Base > klass
klass.reflect_on_all_associations(:has_many).select{|r| r.options[:as] }.each do |reflection|
(hash[reflection.options[:as]] ||= []) << klass.name.downcase
(hash[reflection.options[:as]] ||= []) << klass.name.underscore
end
end
end
end
@poly_hash[name.to_sym]
@poly_hash[name.to_sym] || []
end

def resource_types
Expand Down
2 changes: 1 addition & 1 deletion lib/jsonapi/request_parser.rb
Original file line number Diff line number Diff line change
Expand Up @@ -567,7 +567,7 @@ def parse_to_many_relationship(resource_klass, link_value, relationship, &add_re
if links_object.length == 0
add_result.call([])
else
if relationship.polymorphic?
if relationship.polymorphic? || relationship.sti?
polymorphic_results = []

links_object.each_pair do |type, keys|
Expand Down
18 changes: 18 additions & 0 deletions test/controllers/controller_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2726,6 +2726,12 @@ def test_show_related_resource_no_namespace
"self" => "http://test.host/people/1001/relationships/expense_entries",
"related" => "http://test.host/people/1001/expense_entries"
}
},
"favorite-vehicle" => {
"links" => {
"self" => "http://test.host/people/1001/relationships/favorite_vehicle",
"related" => "http://test.host/people/1001/favorite_vehicle"
}
}
}
}
Expand All @@ -2747,6 +2753,18 @@ def test_show_related_resource_includes
JSONAPI.configuration = original_config
end

def test_show_include_sti
original_config = JSONAPI.configuration.dup
JSONAPI.configuration.json_key_format = :dasherized_key
JSONAPI.configuration.route_format = :underscored_key
get :show, params: {id: '1005', include: 'vehicles,favorite-vehicle'}
assert_response :success
assert_equal 'cars', json_response['included'][0]['type']
assert_equal 'cars', json_response['data']['relationships']['favorite-vehicle']['data']['type']
ensure
JSONAPI.configuration = original_config
end

def test_show_related_resource_nil
assert_cacheable_get :show_related_resource, params: {post_id: '17', relationship: 'author', source:'posts'}
assert_response :success
Expand Down
30 changes: 16 additions & 14 deletions test/fixtures/active_record.rb
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@
t.boolean :book_admin, default: false
t.boolean :special, default: false
t.timestamps null: false
t.integer :favorite_vehicle_id, index: true
end

create_table :author_details, force: true do |t|
Expand Down Expand Up @@ -439,11 +440,11 @@ class Session < ActiveRecord::Base

class Response < ActiveRecord::Base
belongs_to :session
has_one :paragraph, :class_name => "ResponseText::Paragraph"
has_one :paragraph, :class_name => "Paragraph"

def response_type
case self.type
when "Response::SingleTextbox"
when "SingleTextbox"
"single_textbox"
else
"question"
Expand All @@ -452,21 +453,21 @@ def response_type
def response_type=type
self.type = case type
when "single_textbox"
"Response::SingleTextbox"
"SingleTextbox"
else
"Response"
end
end
end

class Response::SingleTextbox < Response
has_one :paragraph, :class_name => "ResponseText::Paragraph", :foreign_key => :response_id
class SingleTextbox < Response
has_one :paragraph, :class_name => "Paragraph", :foreign_key => :response_id
end

class ResponseText < ActiveRecord::Base
end

class ResponseText::Paragraph < ResponseText
class Paragraph < ResponseText
end

class Person < ActiveRecord::Base
Expand All @@ -478,6 +479,7 @@ class Person < ActiveRecord::Base
belongs_to :preferences
belongs_to :hair_cut
has_one :author_detail
belongs_to :favorite_vehicle, class_name: 'Vehicle'

has_and_belongs_to_many :books, join_table: :book_authors
has_and_belongs_to_many :not_banned_books, -> { merge(Book.not_banned) },
Expand Down Expand Up @@ -1265,7 +1267,7 @@ def responses=params
(datum[:relationships] || {}).each_pair { |k,v|
case k
when "paragraph"
response.paragraph = ResponseText::Paragraph.create(((v[:data][:attributes].respond_to?(:permit))? v[:data][:attributes].permit(:text) : v[:data][:attributes]))
response.paragraph = Paragraph.create(((v[:data][:attributes].respond_to?(:permit))? v[:data][:attributes].permit(:text) : v[:data][:attributes]))
end
}
}
Expand All @@ -1285,17 +1287,18 @@ def fetchable_fields
end

class ResponseResource < JSONAPI::Resource
model_hint model: Response::SingleTextbox, resource: :response

has_one :session

attributes :question_id, :response_type

has_one :paragraph
end

class SingleTextboxResource < ResponseResource
end

class ParagraphResource < JSONAPI::Resource
model_name 'ResponseText::Paragraph'
model_name 'Paragraph'

attributes :text

Expand All @@ -1308,8 +1311,9 @@ class PersonResource < BaseResource

has_many :comments, inverse_relationship: :author
has_many :posts, inverse_relationship: :author
has_many :vehicles, polymorphic: true
has_many :vehicles, sti: true

has_one :favorite_vehicle, class_name: 'Vehicle', sti: true
has_one :preferences
has_one :hair_cut

Expand Down Expand Up @@ -1357,12 +1361,10 @@ class VehicleResource < JSONAPI::Resource
end

class CarResource < VehicleResource
model_name "Car"
attributes :drive_layout
end

class BoatResource < VehicleResource
model_name "Boat"
attributes :length_at_water_line
end

Expand Down Expand Up @@ -2167,7 +2169,7 @@ class CommentResource < CommentResource; end
class ExpenseEntryResource < ExpenseEntryResource; end
class IsoCurrencyResource < IsoCurrencyResource; end
class EmployeeResource < EmployeeResource; end
class VehicleResource < PersonResource; end
class VehicleResource < VehicleResource; end
class HairCutResource < HairCutResource; end
end
end
Expand Down
1 change: 1 addition & 0 deletions test/fixtures/people.yml
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ e:
date_joined: <%= DateTime.parse('2013-11-30 4:20:00 UTC +00:00') %>
book_admin: true
preferences_id: 55
favorite_vehicle_id: 1

x:
id: 1000
Expand Down
8 changes: 4 additions & 4 deletions test/integration/requests/request_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -317,7 +317,7 @@ def test_post_single
assert_jsonapi_response 201
end

def test_post_polymorphic_with_has_many_relationship
def test_post_sti_with_has_many_relationship
post '/people', params:
{
'data' => {
Expand Down Expand Up @@ -380,7 +380,7 @@ def test_post_polymorphic_invalid_with_wrong_type
assert_jsonapi_response 400, msg: "Submitting a thing as a vehicle should raise a type mismatch error"
end

def test_post_polymorphic_invalid_with_not_matched_type_and_id
def test_post_sti_invalid_with_not_matched_type_and_id
post '/people', params:
{
'data' => {
Expand All @@ -405,7 +405,7 @@ def test_post_polymorphic_invalid_with_not_matched_type_and_id
'Accept' => JSONAPI::MEDIA_TYPE
}

assert_jsonapi_response 404, msg: "Submitting a thing as a vehicle should raise a record not found"
assert_jsonapi_response 404, msg: "Submitting a vehicle should raise a record not found if the type does not match"
end

def test_post_single_missing_data_contents
Expand Down Expand Up @@ -680,7 +680,7 @@ def test_patch_polymorphic_invalid_with_wrong_type
assert_jsonapi_response 400, msg: "Submitting a thing as a vehicle should raise a type mismatch error"
end

def test_patch_polymorphic_invalid_with_not_matched_type_and_id
def test_patch_sti_invalid_with_not_matched_type_and_id
patch '/people/1000', params:
{
'data' => {
Expand Down
Loading