Skip to content

Commit

Permalink
Restore use_related_resource_records_for_joins for v0_10 (#1412)
Browse files Browse the repository at this point in the history
* Restore `use_related_resource_records_for_joins` for v0_10

* Handle nil actual hashes

* Add back join_options for v10 compatibility

* Test JoinManager not JoinManagerV10

* Use sql_for_compare to account for different sql dialect quoating
  • Loading branch information
lgebhardt committed Apr 18, 2024
1 parent 2dd0589 commit 125cef4
Show file tree
Hide file tree
Showing 7 changed files with 113 additions and 71 deletions.
10 changes: 8 additions & 2 deletions lib/jsonapi/active_relation/join_manager_v10.rb
Original file line number Diff line number Diff line change
Expand Up @@ -149,9 +149,15 @@ def perform_joins(records, options)
related_resource_klass = join_details[:related_resource_klass]
join_type = relationship_details[:join_type]

join_options = {
relationship: relationship,
relationship_details: relationship_details,
related_resource_klass: related_resource_klass,
}

if relationship == :root
unless source_relationship
add_join_details('', {alias: resource_klass._table_name, join_type: :root})
add_join_details('', {alias: resource_klass._table_name, join_type: :root, join_options: join_options})
end
next
end
Expand All @@ -165,7 +171,7 @@ def perform_joins(records, options)
options: options)
}

details = {alias: self.class.alias_from_arel_node(join_node), join_type: join_type}
details = {alias: self.class.alias_from_arel_node(join_node), join_type: join_type, join_options: join_options}

if relationship == source_relationship
if relationship.polymorphic? && relationship.belongs_to?
Expand Down
7 changes: 6 additions & 1 deletion lib/jsonapi/active_relation_retrieval_v10.rb
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ def find_fragments(filters, options = {})

sort_criteria = options.fetch(:sort_criteria) { [] }

join_manager = ActiveRelation::JoinManager.new(resource_klass: resource_klass,
join_manager = ActiveRelation::JoinManagerV10.new(resource_klass: resource_klass,
source_relationship: nil,
relationships: linkage_relationships.collect(&:name),
sort_criteria: sort_criteria,
Expand Down Expand Up @@ -316,6 +316,11 @@ def apply_join(records:, relationship:, resource_type:, join_type:, options:)
records = records.joins_left(relation_name)
end
end

if relationship.use_related_resource_records_for_joins
records = records.merge(self.records(options))
end

records
end

Expand Down
10 changes: 9 additions & 1 deletion lib/jsonapi/configuration.rb
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,8 @@ class Configuration
:resource_cache_digest_function,
:resource_cache_usage_report_function,
:default_exclude_links,
:default_resource_retrieval_strategy
:default_resource_retrieval_strategy,
:use_related_resource_records_for_joins

def initialize
#:underscored_key, :camelized_key, :dasherized_key, or custom
Expand Down Expand Up @@ -176,6 +177,11 @@ def initialize
# :none
# :self
self.default_resource_retrieval_strategy = 'JSONAPI::ActiveRelationRetrieval'

# For 'JSONAPI::ActiveRelationRetrievalV10': use a related resource's `records` when performing joins.
# This setting allows included resources to account for permission scopes. It can be overridden explicitly per
# relationship. Furthermore, specifying a `relation_name` on a relationship will cause this setting to be ignored.
self.use_related_resource_records_for_joins = true
end

def cache_formatters=(bool)
Expand Down Expand Up @@ -319,6 +325,8 @@ def allow_include=(allow_include)
attr_writer :default_exclude_links

attr_writer :default_resource_retrieval_strategy

attr_writer :use_related_resource_records_for_joins
end

class << self
Expand Down
11 changes: 10 additions & 1 deletion lib/jsonapi/relationship.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ class Relationship
attr_reader :acts_as_set, :foreign_key, :options, :name,
:class_name, :polymorphic, :always_include_optional_linkage_data, :exclude_linkage_data,
:parent_resource, :eager_load_on_include, :custom_methods,
:inverse_relationship, :allow_include, :hidden
:inverse_relationship, :allow_include, :hidden, :use_related_resource_records_for_joins

attr_writer :allow_include

Expand All @@ -25,6 +25,15 @@ def initialize(name, options = {})
@polymorphic_types ||= options[:polymorphic_relations]
end

use_related_resource_records_for_joins_default = if options[:relation_name]
false
else
JSONAPI.configuration.use_related_resource_records_for_joins
end

@use_related_resource_records_for_joins = options.fetch(:use_related_resource_records_for_joins,
use_related_resource_records_for_joins_default) == true

@hidden = options.fetch(:hidden, false) == true

@exclude_linkage_data = options[:exclude_linkage_data]
Expand Down
2 changes: 1 addition & 1 deletion test/helpers/assertions.rb
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
module Helpers
module Assertions
def assert_hash_equals(exp, act, msg = nil)
msg = message(msg, '') { diff exp.deep_stringify_keys, act.deep_stringify_keys }
msg = message(msg, '') { diff exp.deep_stringify_keys, act&.deep_stringify_keys }
assert(matches_hash?(exp, act, {exact: true}), msg)
end

Expand Down
36 changes: 18 additions & 18 deletions test/unit/active_relation_resource_finder/join_manager_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ class JoinManagerTest < ActiveSupport::TestCase
# end

def test_no_added_joins
join_manager = JSONAPI::ActiveRelation::JoinManagerV10.new(resource_klass: PostResource)
join_manager = JSONAPI::ActiveRelation::JoinManager.new(resource_klass: PostResource)

records = PostResource.records({})
records = join_manager.join(records, {})
Expand All @@ -22,7 +22,7 @@ def test_no_added_joins

def test_add_single_join
filters = {'tags' => ['1']}
join_manager = JSONAPI::ActiveRelation::JoinManagerV10.new(resource_klass: PostResource, filters: filters)
join_manager = JSONAPI::ActiveRelation::JoinManager.new(resource_klass: PostResource, filters: filters)
records = PostResource.records({})
records = join_manager.join(records, {})
assert_equal 'SELECT "posts".* FROM "posts" LEFT OUTER JOIN "posts_tags" ON "posts_tags"."post_id" = "posts"."id" LEFT OUTER JOIN "tags" ON "tags"."id" = "posts_tags"."tag_id"', sql_for_compare(records.to_sql)
Expand All @@ -32,7 +32,7 @@ def test_add_single_join

def test_add_single_sort_join
sort_criteria = [{field: 'tags.name', direction: :desc}]
join_manager = JSONAPI::ActiveRelation::JoinManagerV10.new(resource_klass: PostResource, sort_criteria: sort_criteria)
join_manager = JSONAPI::ActiveRelation::JoinManager.new(resource_klass: PostResource, sort_criteria: sort_criteria)
records = PostResource.records({})
records = join_manager.join(records, {})

Expand All @@ -44,7 +44,7 @@ def test_add_single_sort_join
def test_add_single_sort_and_filter_join
filters = {'tags' => ['1']}
sort_criteria = [{field: 'tags.name', direction: :desc}]
join_manager = JSONAPI::ActiveRelation::JoinManagerV10.new(resource_klass: PostResource, sort_criteria: sort_criteria, filters: filters)
join_manager = JSONAPI::ActiveRelation::JoinManager.new(resource_klass: PostResource, sort_criteria: sort_criteria, filters: filters)
records = PostResource.records({})
records = join_manager.join(records, {})
assert_equal 'SELECT "posts".* FROM "posts" LEFT OUTER JOIN "posts_tags" ON "posts_tags"."post_id" = "posts"."id" LEFT OUTER JOIN "tags" ON "tags"."id" = "posts_tags"."tag_id"', sql_for_compare(records.to_sql)
Expand All @@ -58,7 +58,7 @@ def test_add_sibling_joins
'author' => ['1']
}

join_manager = JSONAPI::ActiveRelation::JoinManagerV10.new(resource_klass: PostResource, filters: filters)
join_manager = JSONAPI::ActiveRelation::JoinManager.new(resource_klass: PostResource, filters: filters)
records = PostResource.records({})
records = join_manager.join(records, {})

Expand All @@ -70,7 +70,7 @@ def test_add_sibling_joins


def test_add_joins_source_relationship
join_manager = JSONAPI::ActiveRelation::JoinManagerV10.new(resource_klass: PostResource,
join_manager = JSONAPI::ActiveRelation::JoinManager.new(resource_klass: PostResource,
source_relationship: PostResource._relationship(:comments))
records = PostResource.records({})
records = join_manager.join(records, {})
Expand All @@ -81,7 +81,7 @@ def test_add_joins_source_relationship


def test_add_joins_source_relationship_with_custom_apply
join_manager = JSONAPI::ActiveRelation::JoinManagerV10.new(resource_klass: Api::V10::PostResource,
join_manager = JSONAPI::ActiveRelation::JoinManager.new(resource_klass: Api::V10::PostResource,
source_relationship: Api::V10::PostResource._relationship(:comments))
records = Api::V10::PostResource.records({})
records = join_manager.join(records, {})
Expand All @@ -100,7 +100,7 @@ def test_add_nested_scoped_joins
'author' => ['1']
}

join_manager = JSONAPI::ActiveRelation::JoinManagerV10.new(resource_klass: Api::V10::PostResource, filters: filters)
join_manager = JSONAPI::ActiveRelation::JoinManager.new(resource_klass: Api::V10::PostResource, filters: filters)
records = Api::V10::PostResource.records({})
records = join_manager.join(records, {})

Expand All @@ -117,7 +117,7 @@ def test_add_nested_scoped_joins
'comments.tags' => ['1']
}

join_manager = JSONAPI::ActiveRelation::JoinManagerV10.new(resource_klass: Api::V10::PostResource, filters: filters)
join_manager = JSONAPI::ActiveRelation::JoinManager.new(resource_klass: Api::V10::PostResource, filters: filters)
records = Api::V10::PostResource.records({})
records = join_manager.join(records, {})

Expand All @@ -135,7 +135,7 @@ def test_add_nested_joins_with_fields
'author.foo' => ['1']
}

join_manager = JSONAPI::ActiveRelation::JoinManagerV10.new(resource_klass: Api::V10::PostResource, filters: filters)
join_manager = JSONAPI::ActiveRelation::JoinManager.new(resource_klass: Api::V10::PostResource, filters: filters)
records = Api::V10::PostResource.records({})
records = join_manager.join(records, {})

Expand All @@ -149,14 +149,14 @@ def test_add_nested_joins_with_fields
def test_add_joins_with_sub_relationship
relationships = %w(author author.comments tags)

join_manager = JSONAPI::ActiveRelation::JoinManagerV10.new(resource_klass: Api::V10::PostResource, relationships: relationships,
join_manager = JSONAPI::ActiveRelation::JoinManager.new(resource_klass: Api::V10::PostResource, relationships: relationships,
source_relationship: Api::V10::PostResource._relationship(:comments))
records = Api::V10::PostResource.records({})
records = join_manager.join(records, {})

assert_hash_equals({alias: 'comments', join_type: :inner}, join_manager.source_join_details)
assert_hash_equals({alias: 'comments', join_type: :inner}, join_manager.join_details_by_relationship(Api::V10::PostResource._relationship(:comments)))
assert_hash_equals({alias: 'tags', join_type: :left}, join_manager.join_details_by_relationship(Api::V10::CommentResource._relationship(:tags)))
assert_hash_equals({alias: 'tags', join_type: :left}, join_manager.join_details_by_relationship(Api::V10::PostResource._relationship(:tags)))
assert_hash_equals({alias: 'comments_people', join_type: :left}, join_manager.join_details_by_relationship(Api::V10::PersonResource._relationship(:comments)))
end

Expand All @@ -168,7 +168,7 @@ def test_add_joins_with_sub_relationship_and_filters

relationships = %w(author author.comments tags)

join_manager = JSONAPI::ActiveRelation::JoinManagerV10.new(resource_klass: PostResource,
join_manager = JSONAPI::ActiveRelation::JoinManager.new(resource_klass: PostResource,
filters: filters,
relationships: relationships,
source_relationship: PostResource._relationship(:comments))
Expand All @@ -177,13 +177,13 @@ def test_add_joins_with_sub_relationship_and_filters

assert_hash_equals({alias: 'comments', join_type: :inner}, join_manager.source_join_details)
assert_hash_equals({alias: 'comments', join_type: :inner}, join_manager.join_details_by_relationship(PostResource._relationship(:comments)))
assert_hash_equals({alias: 'people', join_type: :left}, join_manager.join_details_by_relationship(CommentResource._relationship(:author)))
assert_hash_equals({alias: 'tags', join_type: :left}, join_manager.join_details_by_relationship(CommentResource._relationship(:tags)))
assert_hash_equals({alias: 'people', join_type: :left}, join_manager.join_details_by_relationship(PostResource._relationship(:author)))
assert_hash_equals({alias: 'tags', join_type: :left}, join_manager.join_details_by_relationship(PostResource._relationship(:tags)))
assert_hash_equals({alias: 'comments_people', join_type: :left}, join_manager.join_details_by_relationship(PersonResource._relationship(:comments)))
end

def test_polymorphic_join_belongs_to_just_source
join_manager = JSONAPI::ActiveRelation::JoinManagerV10.new(
join_manager = JSONAPI::ActiveRelation::JoinManager.new(
resource_klass: PictureResource,
source_relationship: PictureResource._relationship(:imageable)
)
Expand All @@ -200,7 +200,7 @@ def test_polymorphic_join_belongs_to_just_source

def test_polymorphic_join_belongs_to_filter
filters = {'imageable' => ['Foo']}
join_manager = JSONAPI::ActiveRelation::JoinManagerV10.new(resource_klass: PictureResource, filters: filters)
join_manager = JSONAPI::ActiveRelation::JoinManager.new(resource_klass: PictureResource, filters: filters)

records = PictureResource.records({})
records = join_manager.join(records, {})
Expand All @@ -217,7 +217,7 @@ def test_polymorphic_join_belongs_to_filter_on_resource
}

relationships = %w(imageable file_properties)
join_manager = JSONAPI::ActiveRelation::JoinManagerV10.new(resource_klass: PictureResource,
join_manager = JSONAPI::ActiveRelation::JoinManager.new(resource_klass: PictureResource,
filters: filters,
relationships: relationships)

Expand Down
Loading

0 comments on commit 125cef4

Please sign in to comment.