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

Smarter association id lookup-- no db hit on belongs_to for id-only #1857

Merged
merged 2 commits into from
Apr 30, 2017
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
4 changes: 4 additions & 0 deletions lib/active_model/serializer/association.rb
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,10 @@ def meta
reflection.options[:meta]
end

def belongs_to?
reflection.foreign_key_on == :self
end

def polymorphic?
true == reflection_options[:polymorphic]
end
Expand Down
4 changes: 4 additions & 0 deletions lib/active_model/serializer/belongs_to_reflection.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@ module ActiveModel
class Serializer
# @api private
class BelongsToReflection < Reflection
# @api private
def foreign_key_on
:self
end
end
end
end
17 changes: 17 additions & 0 deletions lib/active_model/serializer/reflection.rb
Original file line number Diff line number Diff line change
Expand Up @@ -47,11 +47,23 @@ class Serializer
#
# So you can inspect reflections in your Adapters.
class Reflection < Field
attr_reader :foreign_key, :type

def initialize(*)
super
options[:links] = {}
options[:include_data_setting] = Serializer.config.include_data_default
options[:meta] = nil
@type = options.fetch(:type) do
class_name = options.fetch(:class_name, name.to_s.camelize.singularize)

Choose a reason for hiding this comment

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

This line is causing the bug reported here: #2160 In particular, the use of the field's name if the class_name and type aren't specified breaks belongs_to associations.

class_name.underscore.pluralize.to_sym
end
@foreign_key =
if collection?
"#{name.to_s.singularize}_ids".to_sym
else
"#{name}_id".to_sym
end
end

# @api public
Expand Down Expand Up @@ -150,6 +162,11 @@ def value(serializer, include_slice)
end
end

# @api private
def foreign_key_on
:related
end

# Build association. This method is used internally to
# build serializer's association by its reflection.
#
Expand Down
21 changes: 14 additions & 7 deletions lib/active_model_serializers/adapter/json_api/relationship.rb
Original file line number Diff line number Diff line change
Expand Up @@ -43,14 +43,21 @@ def data_for(association)
end

def data_for_one(association)
# TODO(BF): Process relationship without evaluating lazy_association
serializer = association.lazy_association.serializer
if (virtual_value = association.virtual_value)
virtual_value
elsif serializer && association.object
ResourceIdentifier.new(serializer, serializable_resource_options).as_json
if association.belongs_to? &&
parent_serializer.object.respond_to?(association.reflection.foreign_key)
id = parent_serializer.object.send(association.reflection.foreign_key)
type = association.reflection.type.to_s
ResourceIdentifier.for_type_with_id(type, id, serializable_resource_options)
else
nil
# TODO(BF): Process relationship without evaluating lazy_association
serializer = association.lazy_association.serializer
if (virtual_value = association.virtual_value)
virtual_value
elsif serializer && association.object
ResourceIdentifier.new(serializer, serializable_resource_options).as_json
else
nil
end
end
end

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,13 @@ def self.type_for(class_name, serializer_type = nil, transform_options = {})
JsonApi.send(:transform_key_casing!, raw_type, transform_options)
end

def self.for_type_with_id(type, id, options)
Copy link
Contributor

Choose a reason for hiding this comment

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

nice method name :-)

{
id: id.to_s,
type: type_for(:no_class_needed, type, options)
}
end

# {http://jsonapi.org/format/#document-resource-identifier-objects Resource Identifier Objects}
def initialize(serializer, options)
@id = id_for(serializer)
Expand Down
28 changes: 28 additions & 0 deletions test/serializers/associations_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,34 @@ def test_associations_custom_keys
assert expected_association_keys.include? :site
end

class BelongsToBlogModel < ::Model
attributes :id, :title
associations :blog
end
class BelongsToBlogModelSerializer < ActiveModel::Serializer
type :posts
belongs_to :blog
end

def test_belongs_to_doesnt_load_record
attributes = { id: 1, title: 'Belongs to Blog', blog: Blog.new(id: 5) }
post = BelongsToBlogModel.new(attributes)
class << post
def blog
fail 'should use blog_id'
end

def blog_id
5
end
end

actual = serializable(post, adapter: :json_api, serializer: BelongsToBlogModelSerializer).as_json
expected = { data: { id: '1', type: 'posts', relationships: { blog: { data: { id: '5', type: 'blogs' } } } } }

assert_equal expected, actual
end

class InlineAssociationTestPostSerializer < ActiveModel::Serializer
has_many :comments
has_many :comments, key: :last_comments do
Expand Down