From 0bb3067b84423ab8411fbfd65c90225e86ad5b66 Mon Sep 17 00:00:00 2001 From: Larry Gebhardt Date: Wed, 20 Feb 2019 06:54:14 -0500 Subject: [PATCH 1/2] Add relationship apply callable for custom joins Reworks the JoinTree and renamed to JoinManager --- lib/jsonapi-resources.rb | 2 +- .../active_relation_resource_finder.rb | 228 +++++++------- .../join_manager.rb | 288 +++++++++++++++++ .../join_tree.rb | 227 -------------- lib/jsonapi/path.rb | 8 + lib/jsonapi/path_segment.rb | 22 +- lib/jsonapi/relationship.rb | 1 + test/fixtures/active_record.rb | 72 ++++- .../join_manager_test.rb | 268 ++++++++++++++++ .../join_tree_test.rb | 289 ------------------ 10 files changed, 756 insertions(+), 649 deletions(-) create mode 100644 lib/jsonapi/active_relation_resource_finder/join_manager.rb delete mode 100644 lib/jsonapi/active_relation_resource_finder/join_tree.rb create mode 100644 test/unit/active_relation_resource_finder/join_manager_test.rb delete mode 100644 test/unit/active_relation_resource_finder/join_tree_test.rb diff --git a/lib/jsonapi-resources.rb b/lib/jsonapi-resources.rb index e8bdc068d..bafa53788 100644 --- a/lib/jsonapi-resources.rb +++ b/lib/jsonapi-resources.rb @@ -25,7 +25,7 @@ require 'jsonapi/callbacks' require 'jsonapi/link_builder' require 'jsonapi/active_relation_resource_finder' -require 'jsonapi/active_relation_resource_finder/join_tree' +require 'jsonapi/active_relation_resource_finder/join_manager' require 'jsonapi/resource_identity' require 'jsonapi/resource_fragment' require 'jsonapi/resource_id_tree' diff --git a/lib/jsonapi/active_relation_resource_finder.rb b/lib/jsonapi/active_relation_resource_finder.rb index 5e60eaf08..8a0406e65 100644 --- a/lib/jsonapi/active_relation_resource_finder.rb +++ b/lib/jsonapi/active_relation_resource_finder.rb @@ -19,16 +19,16 @@ module ClassMethods def find(filters, options = {}) sort_criteria = options.fetch(:sort_criteria) { [] } - join_tree = JoinTree.new(resource_klass: self, - options: options, - filters: filters, - sort_criteria: sort_criteria) + join_manager = JoinManager.new(resource_klass: self, + filters: filters, + sort_criteria: sort_criteria) paginator = options[:paginator] records = find_records(records: records(options), + sort_criteria: sort_criteria, filters: filters, - join_tree: join_tree, + join_manager: join_manager, paginator: paginator, options: options) @@ -42,13 +42,12 @@ def find(filters, options = {}) # # @return [Integer] the count def count(filters, options = {}) - join_tree = JoinTree.new(resource_klass: self, - options: options, - filters: filters) + join_manager = JoinManager.new(resource_klass: self, + filters: filters) records = find_records(records: records(options), filters: filters, - join_tree: join_tree, + join_manager: join_manager, options: options) count_records(records) @@ -93,12 +92,11 @@ def find_fragments(filters, options = {}) sort_criteria = options.fetch(:sort_criteria) { [] } - join_tree = JoinTree.new(resource_klass: resource_klass, - source_relationship: nil, - relationships: linkage_relationships, - sort_criteria: sort_criteria, - filters: filters, - options: options) + join_manager = JoinManager.new(resource_klass: resource_klass, + source_relationship: nil, + relationships: linkage_relationships, + sort_criteria: sort_criteria, + filters: filters) paginator = options[:paginator] @@ -106,13 +104,11 @@ def find_fragments(filters, options = {}) filters: filters, sort_criteria: sort_criteria, paginator: paginator, - join_tree: join_tree, + join_manager: join_manager, options: options) - joins = join_tree.joins - # This alias is going to be resolve down to the model's table name and will not actually be an alias - resource_table_alias = joins[''][:alias] + resource_table_alias = resource_klass._table_name pluck_fields = [Arel.sql("#{concat_table_field(resource_table_alias, resource_klass._primary_key)} AS #{resource_table_alias}_#{resource_klass._primary_key}")] @@ -131,7 +127,7 @@ def find_fragments(filters, options = {}) klass = resource_klass_for(resource_type) linkage_fields << {relationship_name: name, resource_klass: klass} - linkage_table_alias = joins["#{linkage_relationship.name.to_s}##{resource_type}"][:alias] + linkage_table_alias = join_manager.join_details_by_polymorphic_relationship(linkage_relationship, resource_type)[:alias] primary_key = klass._primary_key pluck_fields << Arel.sql("#{concat_table_field(linkage_table_alias, primary_key)} AS #{linkage_table_alias}_#{primary_key}") end @@ -139,7 +135,7 @@ def find_fragments(filters, options = {}) klass = linkage_relationship.resource_klass linkage_fields << {relationship_name: name, resource_klass: klass} - linkage_table_alias = joins[name.to_s][:alias] + 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}") end @@ -154,7 +150,7 @@ def find_fragments(filters, options = {}) end fragments = {} - rows = records.pluck(*pluck_fields) + rows = records.distinct.pluck(*pluck_fields) rows.collect do |row| rid = JSONAPI::ResourceIdentity.new(resource_klass, pluck_fields.length == 1 ? row : row[0]) @@ -229,20 +225,18 @@ def count_related(source_rid, relationship_name, options = {}) filters = options.fetch(:filters, {}) # Joins in this case are related to the related_klass - join_tree = JoinTree.new(resource_klass: self, - source_relationship: relationship, - filters: filters, - options: options) + join_manager = JoinManager.new(resource_klass: self, + source_relationship: relationship, + filters: filters) records = find_records(records: records(options), resource_klass: related_klass, primary_keys: source_rid.id, - join_tree: join_tree, + join_manager: join_manager, filters: filters, options: options) - joins = join_tree.joins - related_alias = joins[''][:alias] + related_alias = join_manager.join_details_by_relationship(relationship)[:alias] records = records.select(Arel.sql("#{concat_table_field(related_alias, related_klass._primary_key)}")) @@ -250,7 +244,52 @@ def count_related(source_rid, relationship_name, options = {}) end def records(_options = {}) - _model_class.distinct.all + _model_class.all + end + + def apply_join(records:, relationship:, resource_type:, join_type:, options:) + if relationship.polymorphic? && relationship.belongs_to? + case join_type + when :inner + records = records.joins(resource_type.to_s.singularize.to_sym) + when :left + records = records.joins_left(resource_type.to_s.singularize.to_sym) + end + else + relation_name = relationship.relation_name(options) + case join_type + when :inner + records = records.joins(relation_name) + when :left + records = records.joins_left(relation_name) + end + end + records + end + + def relationship_records(relationship:, join_type: :inner, resource_type: nil, options: {}) + records = relationship.parent_resource.records(options) + strategy = relationship.options[:apply_join] + + if strategy + records = call_method_or_proc(strategy, records, relationship, resource_type, join_type, options) + else + records = apply_join(records: records, + relationship: relationship, + resource_type: resource_type, + join_type: join_type, + options: options) + end + + records + end + + def join_relationship(records:, relationship:, resource_type: nil, join_type: :inner, options: {}) + relationship_records = relationship_records(relationship: relationship, + join_type: join_type, + resource_type: resource_type, + options: options) + records.merge(relationship_records) end protected @@ -290,12 +329,11 @@ def find_related_monomorphic_fragments(source_rids, relationship, options, conne sort_criteria << { field: field, direction: sort[:direction] } end - join_tree = JoinTree.new(resource_klass: self, - source_relationship: relationship, - relationships: linkage_relationships, - sort_criteria: sort_criteria, - filters: filters, - options: options) + join_manager = JoinManager.new(resource_klass: self, + source_relationship: relationship, + relationships: linkage_relationships, + sort_criteria: sort_criteria, + filters: filters) paginator = options[:paginator] if source_rids.count == 1 @@ -305,11 +343,10 @@ def find_related_monomorphic_fragments(source_rids, relationship, options, conne primary_keys: source_ids, paginator: paginator, filters: filters, - join_tree: join_tree, + join_manager: join_manager, options: options) - joins = join_tree.joins - resource_table_alias = joins[''][:alias] + resource_table_alias = join_manager.join_details_by_relationship(relationship)[:alias] pluck_fields = [ Arel.sql("#{_table_name}.#{_primary_key} AS source_id"), @@ -331,7 +368,7 @@ def find_related_monomorphic_fragments(source_rids, relationship, options, conne klass = resource_klass_for(resource_type) linkage_fields << {relationship_name: name, resource_klass: klass} - linkage_table_alias = joins["#{linkage_relationship.name.to_s}##{resource_type}"][:alias] + linkage_table_alias = join_manager.join_details_by_polymorphic_relationship(linkage_relationship, resource_type)[:alias] primary_key = klass._primary_key pluck_fields << Arel.sql("#{concat_table_field(linkage_table_alias, primary_key)} AS #{linkage_table_alias}_#{primary_key}") end @@ -339,7 +376,7 @@ def find_related_monomorphic_fragments(source_rids, relationship, options, conne klass = linkage_relationship.resource_klass linkage_fields << {relationship_name: name, resource_klass: klass} - linkage_table_alias = joins[name.to_s][:alias] + 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}") end @@ -354,7 +391,7 @@ def find_related_monomorphic_fragments(source_rids, relationship, options, conne end fragments = {} - rows = records.pluck(*pluck_fields) + rows = records.distinct.pluck(*pluck_fields) rows.each do |row| rid = JSONAPI::ResourceIdentity.new(resource_klass, row[1]) @@ -418,11 +455,10 @@ def find_related_polymorphic_fragments(source_rids, relationship, options, conne end end - join_tree = JoinTree.new(resource_klass: self, - source_relationship: relationship, - relationships: linkage_relationships, - filters: filters, - options: options) + join_manager = JoinManager.new(resource_klass: self, + source_relationship: relationship, + relationships: linkage_relationships, + filters: filters) paginator = options[:paginator] if source_rids.count == 1 @@ -434,11 +470,9 @@ def find_related_polymorphic_fragments(source_rids, relationship, options, conne primary_keys: source_ids, paginator: paginator, filters: filters, - join_tree: join_tree, + join_manager: join_manager, options: options) - joins = join_tree.joins - primary_key = concat_table_field(_table_name, _primary_key) related_key = concat_table_field(_table_name, relationship.foreign_key) related_type = concat_table_field(_table_name, relationship.polymorphic_type) @@ -467,7 +501,7 @@ def find_related_polymorphic_fragments(source_rids, relationship, options, conne cache_field = related_klass.attribute_to_model_field(:_cache_field) if options[:cache] - table_alias = joins["##{type}"][:alias] + table_alias = join_manager.source_join_details(type)[:alias] cache_offset = relation_index if cache_field @@ -515,7 +549,7 @@ def find_related_polymorphic_fragments(source_rids, relationship, options, conne klass = resource_klass_for(resource_type) linkage_fields << {relationship: linkage_relationship, resource_klass: klass} - linkage_table_alias = joins[linkage_relationship_path][:alias] + linkage_table_alias = join_manager.join_details_by_polymorphic_relationship(linkage_relationship, resource_type)[:alias] primary_key = klass._primary_key pluck_fields << Arel.sql("#{concat_table_field(linkage_table_alias, primary_key)} AS #{linkage_table_alias}_#{primary_key}") end @@ -523,13 +557,13 @@ def find_related_polymorphic_fragments(source_rids, relationship, options, conne klass = linkage_relationship.resource_klass linkage_fields << {relationship: linkage_relationship, resource_klass: klass} - linkage_table_alias = joins[linkage_relationship_path.to_s][:alias] + 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}") end end - rows = records.pluck(*pluck_fields) + rows = records.distinct.pluck(*pluck_fields) related_fragments = {} @@ -582,9 +616,9 @@ def find_related_polymorphic_fragments(source_rids, relationship, options, conne end def find_records(records:, - join_tree: JoinTree.new(resource_klass: self), + join_manager: JoinManager.new(resource_klass: self), resource_klass: self, - filters: nil, + filters: {}, primary_keys: nil, sort_criteria: nil, sort_primary: nil, @@ -592,15 +626,15 @@ def find_records(records:, options: {}) opts = options.dup - records = resource_klass.apply_joins(records, join_tree, opts) + records = resource_klass.apply_joins(records, join_manager, opts) if primary_keys records = records.where(_primary_key => primary_keys) end - opts[:joins] = join_tree.joins + opts[:join_manager] = join_manager - if filters + unless filters.empty? records = resource_klass.filter_records(records, filters, opts) end @@ -618,52 +652,8 @@ def find_records(records:, records end - def get_join_alias(records, &block) - init_join_sources = records.arel.join_sources - init_join_sources_length = init_join_sources.length - - records = yield(records) - - join_sources = records.arel.join_sources - if join_sources.length > init_join_sources_length - last_join = (join_sources - init_join_sources).last - join_alias = - case last_join.left - when Arel::Table - last_join.left.name - when Arel::Nodes::TableAlias - last_join.left.right - when Arel::Nodes::StringJoin - # :nocov: - warn "get_join_alias: Unsupported join type - use custom filtering and sorting" - nil - # :nocov: - end - else - # :nocov: - warn "get_join_alias: No join added" - join_alias = nil - # :nocov: - end - - return records, join_alias - end - - def apply_joins(records, join_tree, _options) - joins = join_tree.joins - - joins.each_value do |join| - case join[:join_type] - when :inner - records, join_alias = get_join_alias(records) { |records| records.joins(join[:relation_join_hash]) } - join[:alias] = join_alias - when :left - records, join_alias = get_join_alias(records) { |records| records.joins_left(join[:relation_join_hash]) } - join[:alias] = join_alias - end - end - - return records + def apply_joins(records, join_manager, options) + join_manager.join(records, options) end def apply_pagination(records, paginator, order_options) @@ -689,9 +679,9 @@ def apply_single_sort(records, field, direction, options) if strategy records = call_method_or_proc(strategy, records, direction, context) else - joins = options[:joins] || {} + join_manager = options[:join_manager] - records = records.order("#{get_aliased_field(field, joins)} #{direction}") + records = records.order(Arel.sql("#{get_aliased_field(field, join_manager)} #{direction}")) end records end @@ -758,22 +748,20 @@ def apply_filters(records, filters, options = {}) records end - def get_aliased_field(path_with_field, joins) + def get_aliased_field(path_with_field, join_manager) path = JSONAPI::Path.new(resource_klass: self, path_string: path_with_field) - relationship = path.segments[-2] - field = path.segments[-1] - relationship_path = path.relationship_path_string + relationship_segment = path.segments[-2] + field_segment = path.segments[-1] - if relationship - join_name = relationship_path - join = joins.try(:[], join_name) - table_alias = join.try(:[], :alias) + if relationship_segment + join_details = join_manager.join_details[path.last_relationship] + table_alias = join_details[:alias] + else + table_alias = self._table_name end - table_alias ||= joins[''][:alias] - - concat_table_field(table_alias, field.delegated_field_name) + concat_table_field(table_alias, field_segment.delegated_field_name) end def apply_filter(records, filter, value, options = {}) @@ -782,8 +770,8 @@ def apply_filter(records, filter, value, options = {}) if strategy records = call_method_or_proc(strategy, records, value, options) else - joins = options[:joins] || {} - records = records.where(get_aliased_field(filter, joins) => value) + join_manager = options[:join_manager] + records = records.where(Arel.sql(get_aliased_field(filter, join_manager)) => value) end records diff --git a/lib/jsonapi/active_relation_resource_finder/join_manager.rb b/lib/jsonapi/active_relation_resource_finder/join_manager.rb new file mode 100644 index 000000000..0ab087962 --- /dev/null +++ b/lib/jsonapi/active_relation_resource_finder/join_manager.rb @@ -0,0 +1,288 @@ +module JSONAPI + module ActiveRelationResourceFinder + + # Stores relationship paths starting from the resource_klass, consolidating duplicate paths from + # relationships, filters and sorts. When joins are made the table aliases are tracked in join_details + class JoinManager + attr_reader :resource_klass, + :source_relationship, + :resource_join_tree, + :join_details + + def initialize(resource_klass:, + source_relationship: nil, + relationships: nil, + filters: nil, + sort_criteria: nil) + + @resource_klass = resource_klass + @join_details = nil + @collected_aliases = Set.new + + @resource_join_tree = { + root: { + join_type: :root, + resource_klasses: { + resource_klass => { + relationships: {} + } + } + } + } + add_source_relationship(source_relationship) + add_sort_criteria(sort_criteria) + add_filters(filters) + add_relationships(relationships) + end + + def join(records, options) + fail "can't be joined again" if @join_details + @join_details = {} + perform_joins(records, options) + end + + # source details will only be on a relationship if the source_relationship is set + # this method gets the join details whether they are on a relationship or are just pseudo details for the base + # resource. Specify the resource type for polymorphic relationships + # + def source_join_details(type=nil) + if source_relationship + related_resource_klass = type ? resource_klass.resource_klass_for(type) : source_relationship.resource_klass + segment = PathSegment::Relationship.new(relationship: source_relationship, resource_klass: related_resource_klass) + details = @join_details[segment] + else + if type + details = @join_details["##{type}"] + else + details = @join_details[''] + end + end + details + end + + def join_details_by_polymorphic_relationship(relationship, type) + segment = PathSegment::Relationship.new(relationship: relationship, resource_klass: resource_klass.resource_klass_for(type)) + @join_details[segment] + end + + def join_details_by_relationship(relationship) + segment = PathSegment::Relationship.new(relationship: relationship, resource_klass: relationship.resource_klass) + @join_details[segment] + end + + def self.get_join_arel_node(records, options = {}) + init_join_sources = records.arel.join_sources + init_join_sources_length = init_join_sources.length + + records = yield(records, options) + + join_sources = records.arel.join_sources + if join_sources.length > init_join_sources_length + last_join = (join_sources - init_join_sources).last + else + # :nocov: + warn "get_join_arel_node: No join added" + last_join = nil + # :nocov: + end + + return records, last_join + end + + def self.alias_from_arel_node(node) + case node.left + when Arel::Table + node.left.name + when Arel::Nodes::TableAlias + node.left.right + when Arel::Nodes::StringJoin + # :nocov: + warn "alias_from_arel_node: Unsupported join type - use custom filtering and sorting" + nil + # :nocov: + end + end + + private + + def flatten_join_tree_by_depth(join_array = [], node = @resource_join_tree, level = 0) + join_array[level] = [] unless join_array[level] + + node.each do |relationship, relationship_details| + relationship_details[:resource_klasses].each do |related_resource_klass, resource_details| + join_array[level] << { relationship: relationship, + relationship_details: relationship_details, + related_resource_klass: related_resource_klass} + flatten_join_tree_by_depth(join_array, resource_details[:relationships], level+1) + end + end + join_array + end + + def add_join_details(join_key, details, check_for_duplicate_alias = true) + fail "details already set" if @join_details.has_key?(join_key) + @join_details[join_key] = details + + if check_for_duplicate_alias && @collected_aliases.include?(details[:alias]) + fail "alias '#{details[:alias]}' has already been added. Possible relation reordering" + end + + @collected_aliases << details[:alias] + end + + def perform_joins(records, options) + join_array = flatten_join_tree_by_depth + + join_array.each do |level_joins| + level_joins.each do |join_details| + relationship = join_details[:relationship] + relationship_details = join_details[:relationship_details] + related_resource_klass = join_details[:related_resource_klass] + join_type = relationship_details[:join_type] + + if relationship == :root + unless source_relationship + add_join_details('', {alias: resource_klass._table_name, join_type: :root}) + end + next + end + + records, join_node = self.class.get_join_arel_node(records, options) {|records, options| + records = related_resource_klass.join_relationship( + records: records, + resource_type: related_resource_klass._type, + join_type: join_type, + relationship: relationship, + options: options) + } + + details = {alias: self.class.alias_from_arel_node(join_node), join_type: join_type} + + if relationship == source_relationship + if relationship.polymorphic? && relationship.belongs_to? + add_join_details("##{related_resource_klass._type}", details) + else + add_join_details('', details) + end + end + + check_for_duplicate_alias = !(relationship == source_relationship) + add_join_details(PathSegment::Relationship.new(relationship: relationship, resource_klass: related_resource_klass), details, check_for_duplicate_alias) + end + end + records + end + + def add_join(path, default_type = :inner, default_polymorphic_join_type = :left) + if source_relationship + if source_relationship.polymorphic? + # Polymorphic paths will come it with the resource_type as the first segment (for example `#documents.comments`) + # We just need to prepend the relationship portion the + sourced_path = "#{source_relationship.name}#{path}" + else + sourced_path = "#{source_relationship.name}.#{path}" + end + else + sourced_path = path + end + + join_manager, _field = parse_path_to_tree(sourced_path, resource_klass, default_type, default_polymorphic_join_type) + + @resource_join_tree[:root].deep_merge!(join_manager) { |key, val, other_val| + if key == :join_type + if val == other_val + val + else + :inner + end + end + } + end + + def process_path_to_tree(path_segments, resource_klass, default_join_type, default_polymorphic_join_type) + node = { + resource_klasses: { + resource_klass => { + relationships: {} + } + } + } + + segment = path_segments.shift + + if segment.is_a?(PathSegment::Relationship) + node[:resource_klasses][resource_klass][:relationships][segment.relationship] ||= {} + + # join polymorphic as left joins + node[:resource_klasses][resource_klass][:relationships][segment.relationship][:join_type] ||= + segment.relationship.polymorphic? ? default_polymorphic_join_type : default_join_type + + segment.relationship.resource_types.each do |related_resource_type| + related_resource_klass = resource_klass.resource_klass_for(related_resource_type) + + # If the resource type was specified in the path segment we want to only process the next segments for + # that resource type, otherwise process for all + process_all_types = !segment.path_specified_resource_klass? + + if process_all_types || related_resource_klass == segment.resource_klass + related_resource_tree = process_path_to_tree(path_segments.dup, related_resource_klass, default_join_type, default_polymorphic_join_type) + node[:resource_klasses][resource_klass][:relationships][segment.relationship].deep_merge!(related_resource_tree) + end + end + end + node + end + + def parse_path_to_tree(path_string, resource_klass, default_join_type = :inner, default_polymorphic_join_type = :left) + path = JSONAPI::Path.new(resource_klass: resource_klass, path_string: path_string) + + field = path.segments[-1] + return process_path_to_tree(path.segments, resource_klass, default_join_type, default_polymorphic_join_type), field + end + + def add_source_relationship(source_relationship) + @source_relationship = source_relationship + + if @source_relationship + resource_klasses = {} + source_relationship.resource_types.each do |related_resource_type| + related_resource_klass = resource_klass.resource_klass_for(related_resource_type) + resource_klasses[related_resource_klass] = {relationships: {}} + end + + join_type = source_relationship.polymorphic? ? :left : :inner + + @resource_join_tree[:root][:resource_klasses][resource_klass][:relationships][@source_relationship] = { + source: true, resource_klasses: resource_klasses, join_type: join_type + } + end + end + + def add_filters(filters) + return if filters.blank? + filters.each_key do |filter| + # Do not add joins for filters with an apply callable. This can be overridden by setting perform_joins to true + next if resource_klass._allowed_filters[filter].try(:[], :apply) && + !resource_klass._allowed_filters[filter].try(:[], :perform_joins) + + add_join(filter, :left) + end + end + + def add_sort_criteria(sort_criteria) + return if sort_criteria.blank? + + sort_criteria.each do |sort| + add_join(sort[:field], :left) + end + end + + def add_relationships(relationships) + return if relationships.blank? + relationships.each do |relationship| + add_join(relationship, :left) + end + end + end + end +end \ No newline at end of file diff --git a/lib/jsonapi/active_relation_resource_finder/join_tree.rb b/lib/jsonapi/active_relation_resource_finder/join_tree.rb deleted file mode 100644 index eaf3b5025..000000000 --- a/lib/jsonapi/active_relation_resource_finder/join_tree.rb +++ /dev/null @@ -1,227 +0,0 @@ -module JSONAPI - module ActiveRelationResourceFinder - class JoinTree - # Stores relationship paths starting from the resource_klass. This allows consolidation of duplicate paths from - # relationships, filters and sorts. This enables the determination of table aliases as they are joined. - - attr_reader :resource_klass, :options, :source_relationship, :resource_joins, :joins - - def initialize(resource_klass:, - options: {}, - source_relationship: nil, - relationships: nil, - filters: nil, - sort_criteria: nil) - - @resource_klass = resource_klass - @options = options - - @resource_joins = { - root: { - join_type: :root, - resource_klasses: { - resource_klass => { - relationships: {} - } - } - } - } - add_source_relationship(source_relationship) - add_sort_criteria(sort_criteria) - add_filters(filters) - add_relationships(relationships) - - @joins = {} - construct_joins(@resource_joins) - end - - private - - def add_join(path, default_type = :inner, default_polymorphic_join_type = :left) - if source_relationship - if source_relationship.polymorphic? - # Polymorphic paths will come it with the resource_type as the first segment (for example `#documents.comments`) - # We just need to prepend the relationship portion the - sourced_path = "#{source_relationship.name}#{path}" - else - sourced_path = "#{source_relationship.name}.#{path}" - end - else - sourced_path = path - end - - join_tree, _field = parse_path_to_tree(sourced_path, resource_klass, default_type, default_polymorphic_join_type) - - @resource_joins[:root].deep_merge!(join_tree) { |key, val, other_val| - if key == :join_type - if val == other_val - val - else - :inner - end - end - } - end - - def process_path_to_tree(path_segments, resource_klass, default_join_type, default_polymorphic_join_type) - node = { - resource_klasses: { - resource_klass => { - relationships: {} - } - } - } - - segment = path_segments.shift - - if segment.is_a?(PathSegment::Relationship) - node[:resource_klasses][resource_klass][:relationships][segment.relationship] ||= {} - - # join polymorphic as left joins - node[:resource_klasses][resource_klass][:relationships][segment.relationship][:join_type] ||= - segment.relationship.polymorphic? ? default_polymorphic_join_type : default_join_type - - segment.relationship.resource_types.each do |related_resource_type| - related_resource_klass = resource_klass.resource_klass_for(related_resource_type) - - # If the resource type was specified in the path segment we want to only process the next segments for - # that resource type, otherwise process for all - process_all_types = !segment.path_specified_resource_klass? - - if process_all_types || related_resource_klass == segment.resource_klass - related_resource_tree = process_path_to_tree(path_segments.dup, related_resource_klass, default_join_type, default_polymorphic_join_type) - node[:resource_klasses][resource_klass][:relationships][segment.relationship].deep_merge!(related_resource_tree) - end - end - end - node - end - - def parse_path_to_tree(path_string, resource_klass, default_join_type = :inner, default_polymorphic_join_type = :left) - path = JSONAPI::Path.new(resource_klass: resource_klass, path_string: path_string) - field = path.segments[-1] - return process_path_to_tree(path.segments, resource_klass, default_join_type, default_polymorphic_join_type), field - end - - def add_source_relationship(source_relationship) - @source_relationship = source_relationship - - if @source_relationship - resource_klasses = {} - source_relationship.resource_types.each do |related_resource_type| - related_resource_klass = resource_klass.resource_klass_for(related_resource_type) - resource_klasses[related_resource_klass] = {relationships: {}} - end - - join_type = source_relationship.polymorphic? ? :left : :inner - - @resource_joins[:root][:resource_klasses][resource_klass][:relationships][@source_relationship] = { - source: true, resource_klasses: resource_klasses, join_type: join_type - } - end - end - - def add_filters(filters) - return if filters.blank? - filters.each_key do |filter| - # Do not add joins for filters with an apply callable. This can be overridden by setting perform_joins to true - next if resource_klass._allowed_filters[filter].try(:[], :apply) && - !resource_klass._allowed_filters[filter].try(:[], :perform_joins) - - add_join(filter) - end - end - - def add_sort_criteria(sort_criteria) - return if sort_criteria.blank? - - sort_criteria.each do |sort| - add_join(sort[:field], :left) - end - end - - def add_relationships(relationships) - return if relationships.blank? - relationships.each do |relationship| - add_join(relationship, :left) - end - end - - # Create a nested set of hashes from an array of path components. This will be used by the `join` methods. - # [post, comments] => { post: { comments: {} } - def relation_join_hash(path, path_hash = {}) - relation = path.shift - if relation - path_hash[relation] = {} - relation_join_hash(path, path_hash[relation]) - end - path_hash - end - - # Returns the paths from shortest to longest, allowing the capture of the table alias for earlier paths. For - # example posts, posts.comments and then posts.comments.author joined in that order will allow each - # alias to be determined whereas just joining posts.comments.author will only record the author alias. - # ToDo: Dependence on this specialized logic should be removed in the future, if possible. - def construct_joins(node, current_relation_path = [], current_relationship_path = []) - node.each do |relationship, relationship_details| - join_type = relationship_details[:join_type] - if relationship == :root - @joins[:root] = {alias: resource_klass._table_name, join_type: :root} - - # alias to the default table unless a source_relationship is specified - unless source_relationship - @joins[''] = {alias: resource_klass._table_name, join_type: :root} - end - - return construct_joins(relationship_details[:resource_klasses].values[0][:relationships], - current_relation_path, - current_relationship_path) - end - - relationship_details[:resource_klasses].each do |resource_klass, resource_details| - if relationship.polymorphic? && relationship.belongs_to? - current_relationship_path << "#{relationship.name.to_s}##{resource_klass._type.to_s}" - relation_name = resource_klass._type.to_s.singularize - else - current_relationship_path << relationship.name.to_s - relation_name = relationship.relation_name(options).to_s - end - - current_relation_path << relation_name - - rel_path = calc_path_string(current_relationship_path) - - @joins[rel_path] = { - alias: nil, - join_type: join_type, - relation_join_hash: relation_join_hash(current_relation_path.dup) - } - - construct_joins(resource_details[:relationships], - current_relation_path.dup, - current_relationship_path.dup) - - current_relation_path.pop - current_relationship_path.pop - end - end - end - - def calc_path_string(path_array) - if source_relationship - if source_relationship.polymorphic? - _relationship_name, resource_name = path_array[0].split('#', 2) - path = path_array.dup - path[0] = "##{resource_name}" - else - path = path_array.dup.drop(1) - end - else - path = path_array.dup - end - - path.join('.') - end - end - end -end \ No newline at end of file diff --git a/lib/jsonapi/path.rb b/lib/jsonapi/path.rb index 2b11326f1..ae111b49e 100644 --- a/lib/jsonapi/path.rb +++ b/lib/jsonapi/path.rb @@ -31,5 +31,13 @@ def relationship_segments def relationship_path_string relationship_segments.collect(&:to_s).join('.') end + + def last_relationship + if @segments.last.is_a?(PathSegment::Relationship) + @segments.last + else + @segments[-2] + end + end end end diff --git a/lib/jsonapi/path_segment.rb b/lib/jsonapi/path_segment.rb index bd34ff13d..aa2d78050 100644 --- a/lib/jsonapi/path_segment.rb +++ b/lib/jsonapi/path_segment.rb @@ -22,19 +22,27 @@ def self.parse(source_resource_klass:, segment_string:, parse_fields: true) end class Relationship - attr_reader :relationship + attr_reader :relationship, :resource_klass - def initialize(relationship:, resource_klass:) + def initialize(relationship:, resource_klass: nil) @relationship = relationship @resource_klass = resource_klass end + def eql?(other) + relationship == other.relationship && resource_klass == other.resource_klass + end + + def hash + [relationship, resource_klass].hash + end + def to_s - @resource_klass ? "#{relationship.name}##{resource_klass._type}" : "#{relationship.name}" + @resource_klass ? "#{relationship.parent_resource_klass._type}.#{relationship.name}##{resource_klass._type}" : "#{resource_klass._type}.#{relationship.name}" end def resource_klass - @resource_klass || @relationship.resource_klass + @resource_klass || relationship.resource_klass end def path_specified_resource_klass? @@ -50,13 +58,17 @@ def initialize(resource_klass:, field_name:) @field_name = field_name end + def eql?(other) + field_name == other.field_name && resource_klass == other.resource_klass + end + def delegated_field_name resource_klass._attribute_delegated_name(field_name) end def to_s # :nocov: - field_name.to_s + "#{resource_klass._type}.#{field_name.to_s}" # :nocov: end end diff --git a/lib/jsonapi/relationship.rb b/lib/jsonapi/relationship.rb index dc691c176..75f94d311 100644 --- a/lib/jsonapi/relationship.rb +++ b/lib/jsonapi/relationship.rb @@ -32,6 +32,7 @@ def initialize(name, options = {}) end alias_method :polymorphic?, :polymorphic + alias_method :parent_resource_klass, :parent_resource def primary_key # :nocov: diff --git a/test/fixtures/active_record.rb b/test/fixtures/active_record.rb index 2dc6c5c96..cbcd1a167 100644 --- a/test/fixtures/active_record.rb +++ b/test/fixtures/active_record.rb @@ -1147,6 +1147,23 @@ module V8 class NumerosTelefoneController < JSONAPI::ResourceController end end + + module V9 + class AuthorsController < JSONAPI::ResourceController + end + + class AuthorDetailsController < JSONAPI::ResourceController + end + + class PostsController < JSONAPI::ResourceController + end + + class CommentsController < JSONAPI::ResourceController + end + + class SectionsController < JSONAPI::ResourceController + end + end end module Api @@ -1591,7 +1608,7 @@ class CraterResource < JSONAPI::Resource filter :description, apply: -> (records, value, options) { fail "context not set" unless options[:context][:current_user] != nil && options[:context][:current_user] == $test_user - records.where(concat_table_field(options[:joins][''][:alias], :description) => value) + records.where(concat_table_field(options[:join_manager].source_join_details[:alias], :description) => value) } def self.verify_key(key, context = nil) @@ -1629,13 +1646,13 @@ class PictureResource < JSONAPI::Resource has_one :file_properties, inverse_relationship: :fileable, :foreign_key_on => :related, polymorphic: true filter 'imageable.name', perform_joins: true, apply: -> (records, value, options) { - joins = options[:joins] + join_manager = options[:join_manager] relationship = _relationship(:imageable) or_parts = relationship.resource_types.collect do |type| - table_alias = joins["imageable##{type}"][:alias] + table_alias = join_manager.join_details_by_polymorphic_relationship(relationship, type)[:alias] "#{concat_table_field(table_alias, "name")} = '#{value.first}'" end - records.where(or_parts.join(' OR ')) + records.where(Arel.sql(or_parts.join(' OR '))) } filter 'imageable#documents.name' @@ -1857,7 +1874,9 @@ class AuthorResource < JSONAPI::Resource attributes :name has_many :books, inverse_relationship: :authors, relation_name: -> (options) { - if options[:context][:current_user].try(:book_admin) + book_admin = options[:context][:book_admin] || options[:context][:current_user].try(:book_admin) + + if book_admin :books else :not_banned_books @@ -2013,8 +2032,9 @@ class AuthorResource < JSONAPI::Resource relationship :author_detail, to: :one, foreign_key_on: :related filter :name, apply: lambda { |records, value, options| - table_alias = options[:joins][''][:alias] - records.where("#{concat_table_field(table_alias, "name")} LIKE \"%#{value[0]}%\"") + table_alias = options[:join_manager].source_join_details[:alias] + t = Arel::Table.new(:people, as: table_alias) + records.where(t[:name].matches("%#{value[0]}%")) } def fetchable_fields @@ -2209,6 +2229,44 @@ class NumeroTelefoneResource < JSONAPI::Resource attribute :numero_telefone end end + + module V9 + class PersonResource < PersonResource; end + class PostResource < PostResource + has_many :comments, apply_join: -> (records, relationship, resource_type, join_type, options) { + case join_type + when :inner + records = records.joins(relationship.relation_name(options)) + when :left + records = records.joins_left(relationship.relation_name(options)) + end + records.where(comments: {approved: true}) + } + end + + class TagResource < TagResource; end + class SectionResource < SectionResource; end + class CommentResource < CommentResource + has_one :author, class_name: 'Person', apply_join: -> (records, relationship, resource_type, join_type, options) { + records = apply_join(records: records, + relationship: relationship, + resource_type: resource_type, + join_type: join_type, + options: options) + + records.where(author: {special: true}) + } + end + + class AuthorResource < Api::V2::AuthorResource + end + + class BookResource < Api::V2::BookResource + end + + class BookCommentResource < Api::V2::BookCommentResource + end + end end module AdminApi diff --git a/test/unit/active_relation_resource_finder/join_manager_test.rb b/test/unit/active_relation_resource_finder/join_manager_test.rb new file mode 100644 index 000000000..013cbb699 --- /dev/null +++ b/test/unit/active_relation_resource_finder/join_manager_test.rb @@ -0,0 +1,268 @@ +require File.expand_path('../../../test_helper', __FILE__) +require 'jsonapi-resources' + +class JoinTreeTest < ActiveSupport::TestCase + + def test_no_added_joins + join_manager = JSONAPI::ActiveRelationResourceFinder::JoinManager.new(resource_klass: PostResource) + + records = PostResource.records({}) + records = join_manager.join(records, {}) + assert_equal 'SELECT "posts".* FROM "posts"', records.to_sql + + assert_hash_equals({alias: 'posts', join_type: :root}, join_manager.source_join_details) + end + + def test_add_single_join + filters = {'tags' => ['1']} + join_manager = JSONAPI::ActiveRelationResourceFinder::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"', records.to_sql + assert_hash_equals({alias: 'posts', join_type: :root}, join_manager.source_join_details) + assert_hash_equals({alias: 'tags', join_type: :left}, join_manager.join_details_by_relationship(PostResource._relationship(:tags))) + end + + def test_add_single_sort_join + sort_criteria = [{field: 'tags.name', direction: :desc}] + join_manager = JSONAPI::ActiveRelationResourceFinder::JoinManager.new(resource_klass: PostResource, sort_criteria: sort_criteria) + 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"', records.to_sql + assert_hash_equals({alias: 'posts', join_type: :root}, join_manager.source_join_details) + assert_hash_equals({alias: 'tags', join_type: :left}, join_manager.join_details_by_relationship(PostResource._relationship(:tags))) + end + + def test_add_single_sort_and_filter_join + filters = {'tags' => ['1']} + sort_criteria = [{field: 'tags.name', direction: :desc}] + join_manager = JSONAPI::ActiveRelationResourceFinder::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"', records.to_sql + assert_hash_equals({alias: 'posts', join_type: :root}, join_manager.source_join_details) + assert_hash_equals({alias: 'tags', join_type: :left}, join_manager.join_details_by_relationship(PostResource._relationship(:tags))) + end + + def test_add_sibling_joins + filters = { + 'tags' => ['1'], + 'author' => ['1'] + } + + join_manager = JSONAPI::ActiveRelationResourceFinder::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" LEFT OUTER JOIN "people" ON "people"."id" = "posts"."author_id"', records.to_sql + assert_hash_equals({alias: 'posts', join_type: :root}, join_manager.source_join_details) + assert_hash_equals({alias: 'tags', join_type: :left}, join_manager.join_details_by_relationship(PostResource._relationship(:tags))) + assert_hash_equals({alias: 'people', join_type: :left}, join_manager.join_details_by_relationship(PostResource._relationship(:author))) + end + + + def test_add_joins_source_relationship + join_manager = JSONAPI::ActiveRelationResourceFinder::JoinManager.new(resource_klass: PostResource, + source_relationship: PostResource._relationship(:comments)) + records = PostResource.records({}) + records = join_manager.join(records, {}) + + assert_equal 'SELECT "posts".* FROM "posts" INNER JOIN "comments" ON "comments"."post_id" = "posts"."id"', records.to_sql + assert_hash_equals({alias: 'comments', join_type: :inner}, join_manager.source_join_details) + end + + + def test_add_joins_source_relationship_with_custom_apply + join_manager = JSONAPI::ActiveRelationResourceFinder::JoinManager.new(resource_klass: Api::V9::PostResource, + source_relationship: Api::V9::PostResource._relationship(:comments)) + records = Api::V9::PostResource.records({}) + records = join_manager.join(records, {}) + + if Rails::VERSION::MAJOR >= 5 && Rails::VERSION::MINOR >= 2 + assert_equal 'SELECT "posts".* FROM "posts" INNER JOIN "comments" ON "comments"."post_id" = "posts"."id" WHERE "comments"."approved" = 1', records.to_sql + else + assert_equal 'SELECT "posts".* FROM "posts" INNER JOIN "comments" ON "comments"."post_id" = "posts"."id" WHERE "comments"."approved" = \'t\'', records.to_sql + end + + assert_hash_equals({alias: 'comments', join_type: :inner}, join_manager.source_join_details) + end + + def test_add_nested_scoped_joins + filters = { + 'comments.author' => ['1'], + 'comments.tags' => ['1'], + 'author' => ['1'] + } + + join_manager = JSONAPI::ActiveRelationResourceFinder::JoinManager.new(resource_klass: Api::V9::PostResource, filters: filters) + records = Api::V9::PostResource.records({}) + records = join_manager.join(records, {}) + + if Rails::VERSION::MAJOR >= 5 && Rails::VERSION::MINOR >= 2 + assert_equal 'SELECT "posts".* FROM "posts" LEFT OUTER JOIN "comments" ON "comments"."post_id" = "posts"."id" LEFT OUTER JOIN "people" ON "people"."id" = "posts"."author_id" LEFT OUTER JOIN "people" "authors_comments" ON "authors_comments"."id" = "comments"."author_id" LEFT OUTER JOIN "comments_tags" ON "comments_tags"."comment_id" = "comments"."id" LEFT OUTER JOIN "tags" ON "tags"."id" = "comments_tags"."tag_id" WHERE "comments"."approved" = 1 AND "author"."special" = 1', records.to_sql + else + assert_equal 'SELECT "posts".* FROM "posts" LEFT OUTER JOIN "comments" ON "comments"."post_id" = "posts"."id" LEFT OUTER JOIN "people" ON "people"."id" = "posts"."author_id" LEFT OUTER JOIN "people" "authors_comments" ON "authors_comments"."id" = "comments"."author_id" LEFT OUTER JOIN "comments_tags" ON "comments_tags"."comment_id" = "comments"."id" LEFT OUTER JOIN "tags" ON "tags"."id" = "comments_tags"."tag_id" WHERE "comments"."approved" = \'t\' AND "author"."special" = \'t\'', records.to_sql + end + + assert_hash_equals({alias: 'posts', join_type: :root}, join_manager.source_join_details) + assert_hash_equals({alias: 'comments', join_type: :left}, join_manager.join_details_by_relationship(Api::V9::PostResource._relationship(:comments))) + assert_hash_equals({alias: 'authors_comments', join_type: :left}, join_manager.join_details_by_relationship(Api::V9::CommentResource._relationship(:author))) + assert_hash_equals({alias: 'tags', join_type: :left}, join_manager.join_details_by_relationship(Api::V9::CommentResource._relationship(:tags))) + assert_hash_equals({alias: 'people', join_type: :left}, join_manager.join_details_by_relationship(Api::V9::PostResource._relationship(:author))) + + # Now test with different order for the filters + filters = { + 'author' => ['1'], + 'comments.author' => ['1'], + 'comments.tags' => ['1'] + } + + join_manager = JSONAPI::ActiveRelationResourceFinder::JoinManager.new(resource_klass: Api::V9::PostResource, filters: filters) + records = Api::V9::PostResource.records({}) + records = join_manager.join(records, {}) + + # Note sql is in different order, but aliases should still be right + if Rails::VERSION::MAJOR >= 5 && Rails::VERSION::MINOR >= 2 + assert_equal 'SELECT "posts".* FROM "posts" LEFT OUTER JOIN "people" ON "people"."id" = "posts"."author_id" LEFT OUTER JOIN "comments" ON "comments"."post_id" = "posts"."id" LEFT OUTER JOIN "people" "authors_comments" ON "authors_comments"."id" = "comments"."author_id" LEFT OUTER JOIN "comments_tags" ON "comments_tags"."comment_id" = "comments"."id" LEFT OUTER JOIN "tags" ON "tags"."id" = "comments_tags"."tag_id" WHERE "comments"."approved" = 1 AND "author"."special" = 1', records.to_sql + else + assert_equal 'SELECT "posts".* FROM "posts" LEFT OUTER JOIN "people" ON "people"."id" = "posts"."author_id" LEFT OUTER JOIN "comments" ON "comments"."post_id" = "posts"."id" LEFT OUTER JOIN "people" "authors_comments" ON "authors_comments"."id" = "comments"."author_id" LEFT OUTER JOIN "comments_tags" ON "comments_tags"."comment_id" = "comments"."id" LEFT OUTER JOIN "tags" ON "tags"."id" = "comments_tags"."tag_id" WHERE "comments"."approved" = \'t\' AND "author"."special" = \'t\'', records.to_sql + end + assert_hash_equals({alias: 'posts', join_type: :root}, join_manager.source_join_details) + assert_hash_equals({alias: 'comments', join_type: :left}, join_manager.join_details_by_relationship(Api::V9::PostResource._relationship(:comments))) + assert_hash_equals({alias: 'authors_comments', join_type: :left}, join_manager.join_details_by_relationship(Api::V9::CommentResource._relationship(:author))) + assert_hash_equals({alias: 'tags', join_type: :left}, join_manager.join_details_by_relationship(Api::V9::CommentResource._relationship(:tags))) + assert_hash_equals({alias: 'people', join_type: :left}, join_manager.join_details_by_relationship(Api::V9::PostResource._relationship(:author))) + + # Easier to read SQL to show joins are the same, but in different order + # Pass 1 + # SELECT "posts".* FROM "posts" + # LEFT OUTER JOIN "comments" ON "comments"."post_id" = "posts"."id" + # LEFT OUTER JOIN "people" ON "people"."id" = "posts"."author_id" + # LEFT OUTER JOIN "people" "authors_comments" ON "authors_comments"."id" = "comments"."author_id" + # LEFT OUTER JOIN "comments_tags" ON "comments_tags"."comment_id" = "comments"."id" + # LEFT OUTER JOIN "tags" ON "tags"."id" = "comments_tags"."tag_id" WHERE "comments"."approved" = 1 AND "author"."special" = 1 + # + # Pass 2 + # SELECT "posts".* FROM "posts" + # LEFT OUTER JOIN "people" ON "people"."id" = "posts"."author_id" + # LEFT OUTER JOIN "comments" ON "comments"."post_id" = "posts"."id" + # LEFT OUTER JOIN "people" "authors_comments" ON "authors_comments"."id" = "comments"."author_id" + # LEFT OUTER JOIN "comments_tags" ON "comments_tags"."comment_id" = "comments"."id" + # LEFT OUTER JOIN "tags" ON "tags"."id" = "comments_tags"."tag_id" WHERE "comments"."approved" = 1 AND "author"."special" = 1 + end + + def test_add_nested_joins_with_fields + filters = { + 'comments.author.name' => ['1'], + 'comments.tags.id' => ['1'], + 'author.foo' => ['1'] + } + + join_manager = JSONAPI::ActiveRelationResourceFinder::JoinManager.new(resource_klass: Api::V9::PostResource, filters: filters) + records = Api::V9::PostResource.records({}) + records = join_manager.join(records, {}) + + if Rails::VERSION::MAJOR >= 5 && Rails::VERSION::MINOR >= 2 + assert_equal 'SELECT "posts".* FROM "posts" LEFT OUTER JOIN "comments" ON "comments"."post_id" = "posts"."id" LEFT OUTER JOIN "people" ON "people"."id" = "posts"."author_id" LEFT OUTER JOIN "people" "authors_comments" ON "authors_comments"."id" = "comments"."author_id" LEFT OUTER JOIN "comments_tags" ON "comments_tags"."comment_id" = "comments"."id" LEFT OUTER JOIN "tags" ON "tags"."id" = "comments_tags"."tag_id" WHERE "comments"."approved" = 1 AND "author"."special" = 1', records.to_sql + else + assert_equal 'SELECT "posts".* FROM "posts" LEFT OUTER JOIN "comments" ON "comments"."post_id" = "posts"."id" LEFT OUTER JOIN "people" ON "people"."id" = "posts"."author_id" LEFT OUTER JOIN "people" "authors_comments" ON "authors_comments"."id" = "comments"."author_id" LEFT OUTER JOIN "comments_tags" ON "comments_tags"."comment_id" = "comments"."id" LEFT OUTER JOIN "tags" ON "tags"."id" = "comments_tags"."tag_id" WHERE "comments"."approved" = \'t\' AND "author"."special" = \'t\'', records.to_sql + end + + assert_hash_equals({alias: 'posts', join_type: :root}, join_manager.source_join_details) + assert_hash_equals({alias: 'comments', join_type: :left}, join_manager.join_details_by_relationship(Api::V9::PostResource._relationship(:comments))) + assert_hash_equals({alias: 'authors_comments', join_type: :left}, join_manager.join_details_by_relationship(Api::V9::CommentResource._relationship(:author))) + assert_hash_equals({alias: 'tags', join_type: :left}, join_manager.join_details_by_relationship(Api::V9::CommentResource._relationship(:tags))) + assert_hash_equals({alias: 'people', join_type: :left}, join_manager.join_details_by_relationship(Api::V9::PostResource._relationship(:author))) + end + + def test_add_joins_with_sub_relationship + relationships = %w(author author.comments tags) + + join_manager = JSONAPI::ActiveRelationResourceFinder::JoinManager.new(resource_klass: Api::V9::PostResource, relationships: relationships, + source_relationship: Api::V9::PostResource._relationship(:comments)) + records = Api::V9::PostResource.records({}) + records = join_manager.join(records, {}) + + if Rails::VERSION::MAJOR >= 5 && Rails::VERSION::MINOR >= 2 + assert_equal 'SELECT "posts".* FROM "posts" INNER JOIN "comments" ON "comments"."post_id" = "posts"."id" LEFT OUTER JOIN "people" ON "people"."id" = "comments"."author_id" LEFT OUTER JOIN "comments_tags" ON "comments_tags"."comment_id" = "comments"."id" LEFT OUTER JOIN "tags" ON "tags"."id" = "comments_tags"."tag_id" LEFT OUTER JOIN "comments" "comments_people" ON "comments_people"."author_id" = "people"."id" WHERE "comments"."approved" = 1 AND "author"."special" = 1', records.to_sql + else + assert_equal 'SELECT "posts".* FROM "posts" INNER JOIN "comments" ON "comments"."post_id" = "posts"."id" LEFT OUTER JOIN "people" ON "people"."id" = "comments"."author_id" LEFT OUTER JOIN "comments_tags" ON "comments_tags"."comment_id" = "comments"."id" LEFT OUTER JOIN "tags" ON "tags"."id" = "comments_tags"."tag_id" LEFT OUTER JOIN "comments" "comments_people" ON "comments_people"."author_id" = "people"."id" WHERE "comments"."approved" = \'t\' AND "author"."special" = \'t\'', records.to_sql + end + + 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::V9::PostResource._relationship(:comments))) + assert_hash_equals({alias: 'people', join_type: :left}, join_manager.join_details_by_relationship(Api::V9::CommentResource._relationship(:author))) + assert_hash_equals({alias: 'tags', join_type: :left}, join_manager.join_details_by_relationship(Api::V9::CommentResource._relationship(:tags))) + assert_hash_equals({alias: 'comments_people', join_type: :left}, join_manager.join_details_by_relationship(Api::V9::PersonResource._relationship(:comments))) + end + + def test_add_joins_with_sub_relationship_and_filters + filters = { + 'author.name' => ['1'], + 'author.comments.name' => ['Foo'] + } + + relationships = %w(author author.comments tags) + + join_manager = JSONAPI::ActiveRelationResourceFinder::JoinManager.new(resource_klass: PostResource, + filters: filters, + relationships: relationships, + source_relationship: PostResource._relationship(:comments)) + records = 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(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: '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::ActiveRelationResourceFinder::JoinManager.new(resource_klass: PictureResource, + source_relationship: PictureResource._relationship(:imageable)) + + records = PictureResource.records({}) + records = join_manager.join(records, {}) + + # assert_equal 'SELECT "pictures".* FROM "pictures" LEFT OUTER JOIN "products" ON "products"."id" = "pictures"."imageable_id" AND "pictures"."imageable_type" = \'Product\' LEFT OUTER JOIN "documents" ON "documents"."id" = "pictures"."imageable_id" AND "pictures"."imageable_type" = \'Document\'', records.to_sql + assert_hash_equals({alias: 'products', join_type: :left}, join_manager.source_join_details('products')) + assert_hash_equals({alias: 'documents', join_type: :left}, join_manager.source_join_details('documents')) + assert_hash_equals({alias: 'products', join_type: :left}, join_manager.join_details_by_polymorphic_relationship(PictureResource._relationship(:imageable), 'products')) + assert_hash_equals({alias: 'documents', join_type: :left}, join_manager.join_details_by_polymorphic_relationship(PictureResource._relationship(:imageable), 'documents')) + end + + def test_polymorphic_join_belongs_to_filter + filters = {'imageable' => ['Foo']} + join_manager = JSONAPI::ActiveRelationResourceFinder::JoinManager.new(resource_klass: PictureResource, filters: filters) + + records = PictureResource.records({}) + records = join_manager.join(records, {}) + + # assert_equal 'SELECT "pictures".* FROM "pictures" LEFT OUTER JOIN "products" ON "products"."id" = "pictures"."imageable_id" AND "pictures"."imageable_type" = \'Product\' LEFT OUTER JOIN "documents" ON "documents"."id" = "pictures"."imageable_id" AND "pictures"."imageable_type" = \'Document\'', records.to_sql + assert_hash_equals({alias: 'pictures', join_type: :root}, join_manager.source_join_details) + assert_hash_equals({alias: 'products', join_type: :left}, join_manager.join_details_by_polymorphic_relationship(PictureResource._relationship(:imageable), 'products')) + assert_hash_equals({alias: 'documents', join_type: :left}, join_manager.join_details_by_polymorphic_relationship(PictureResource._relationship(:imageable), 'documents')) + end + + def test_polymorphic_join_belongs_to_filter_on_resource + filters = { + 'imageable#documents.name' => ['foo'] + } + + relationships = %w(imageable file_properties) + join_manager = JSONAPI::ActiveRelationResourceFinder::JoinManager.new(resource_klass: PictureResource, + filters: filters, + relationships: relationships) + + records = PictureResource.records({}) + records = join_manager.join(records, {}) + + assert_equal 'SELECT "pictures".* FROM "pictures" LEFT OUTER JOIN "documents" ON "documents"."id" = "pictures"."imageable_id" AND "pictures"."imageable_type" = \'Document\' LEFT OUTER JOIN "products" ON "products"."id" = "pictures"."imageable_id" AND "pictures"."imageable_type" = \'Product\' LEFT OUTER JOIN "file_properties" ON "file_properties"."fileable_id" = "pictures"."id" AND "file_properties"."fileable_type" = \'Picture\'', records.to_sql + assert_hash_equals({alias: 'pictures', join_type: :root}, join_manager.source_join_details) + assert_hash_equals({alias: 'products', join_type: :left}, join_manager.join_details_by_polymorphic_relationship(PictureResource._relationship(:imageable), 'products')) + assert_hash_equals({alias: 'documents', join_type: :left}, join_manager.join_details_by_polymorphic_relationship(PictureResource._relationship(:imageable), 'documents')) + assert_hash_equals({alias: 'file_properties', join_type: :left}, join_manager.join_details_by_relationship(PictureResource._relationship(:file_properties))) + end +end diff --git a/test/unit/active_relation_resource_finder/join_tree_test.rb b/test/unit/active_relation_resource_finder/join_tree_test.rb deleted file mode 100644 index acf8c07f0..000000000 --- a/test/unit/active_relation_resource_finder/join_tree_test.rb +++ /dev/null @@ -1,289 +0,0 @@ -require File.expand_path('../../../test_helper', __FILE__) -require 'jsonapi-resources' - -class JoinTreeTest < ActiveSupport::TestCase - - def test_no_added_joins - join_tree = JSONAPI::ActiveRelationResourceFinder::JoinTree.new(resource_klass: PostResource) - - assert_hash_equals({root: {alias: 'posts', join_type: :root }, '' => {alias: 'posts', join_type: :root}}, join_tree.joins) - end - - def test_add_single_join - filters = {'tags' => ['1']} - join_tree = JSONAPI::ActiveRelationResourceFinder::JoinTree.new(resource_klass: PostResource, filters: filters) - assert_hash_equals( - { - root: {alias: 'posts', join_type: :root}, - '' => {alias: 'posts', join_type: :root}, - 'tags' => {alias: nil, join_type: :inner, relation_join_hash: {'tags' => {}}} - }, - join_tree.joins) - end - - def test_add_single_sort_join - sort_criteria = [ {field: 'tags.name', direction: :desc}] - join_tree = JSONAPI::ActiveRelationResourceFinder::JoinTree.new(resource_klass: PostResource, sort_criteria: sort_criteria) - assert_hash_equals( - { - root: {alias: 'posts', join_type: :root}, - '' => {alias: 'posts', join_type: :root}, - 'tags' => {alias: nil, join_type: :left, relation_join_hash: {'tags' => {}}} - }, - join_tree.joins) - end - - def test_add_single_sort_and_filter_join - filters = {'tags' => ['1']} - sort_criteria = [ {field: 'tags.name', direction: :desc}] - join_tree = JSONAPI::ActiveRelationResourceFinder::JoinTree.new(resource_klass: PostResource, sort_criteria: sort_criteria, filters: filters) - assert_hash_equals( - { - root: {alias: 'posts', join_type: :root}, - '' => {alias: 'posts', join_type: :root}, - 'tags' => {alias: nil, join_type: :inner, relation_join_hash: {'tags' => {}}} - }, - join_tree.joins) - end - - def test_add_sibling_joins - filters = { - 'tags' => ['1'], - 'author' => ['1'] - } - - join_tree = JSONAPI::ActiveRelationResourceFinder::JoinTree.new(resource_klass: PostResource, filters: filters) - - assert_hash_equals( - { - root: {alias: 'posts', join_type: :root}, - '' => {alias: 'posts', join_type: :root}, - 'tags' => {alias: nil, join_type: :inner, relation_join_hash: {'tags' => {}}}, - 'author' => {alias: nil, join_type: :inner, relation_join_hash: {'author' => {}}} - }, - join_tree.joins) - end - - - def test_add_joins_source_relationship - join_tree = JSONAPI::ActiveRelationResourceFinder::JoinTree.new(resource_klass: PostResource, - source_relationship: PostResource._relationship(:comments)) - joins = join_tree.joins - assert_hash_equals( - { - root: {alias: 'posts', join_type: :root}, - '' => {alias: nil, join_type: :inner, relation_join_hash: {'comments' => {}}}, - }, - joins) - end - - def test_add_nested_joins - filters = { - 'comments.author' => ['1'], - 'comments.tags' => ['1'], - 'author' => ['1'] - } - - join_tree = JSONAPI::ActiveRelationResourceFinder::JoinTree.new(resource_klass: PostResource, filters: filters) - joins = join_tree.joins - assert_hash_equals( - { - root: {alias: 'posts', join_type: :root}, - '' => {alias: 'posts', join_type: :root}, - 'comments' => {alias: nil, join_type: :inner, relation_join_hash: { 'comments' => {}}}, - 'comments.author' => {alias: nil, join_type: :inner, relation_join_hash: { 'comments' => { 'author' => {}}}}, - 'comments.tags' => {alias: nil, join_type: :inner, relation_join_hash: { 'comments' => { 'tags' => {}}}}, - 'author' => {alias: nil, join_type: :inner, relation_join_hash: {'author' => {}}} - }, - joins) - end - - def test_add_nested_joins_with_fields - filters = { - 'comments.author.name' => ['1'], - 'comments.tags.id' => ['1'], - 'author.foo' => ['1'] - } - - join_tree = JSONAPI::ActiveRelationResourceFinder::JoinTree.new(resource_klass: PostResource, filters: filters) - - assert_hash_equals( - { - root: {alias: 'posts', join_type: :root}, - '' => {alias: 'posts', join_type: :root}, - 'comments' => {alias: nil, join_type: :inner, relation_join_hash: { 'comments' => {}}}, - 'comments.author' => {alias: nil, join_type: :inner, relation_join_hash: { 'comments' => { 'author' => {}}}}, - 'comments.tags' => {alias: nil, join_type: :inner, relation_join_hash: { 'comments' => { 'tags' => {}}}}, - 'author' => {alias: nil, join_type: :inner, relation_join_hash: {'author' => {}}} - }, - join_tree.joins) - end - - def test_add_joins_with_fields_not_from_relationship - filters = { - 'author.name' => ['1'], - 'author.comments.name' => ['Foo'], - 'tags.id' => ['1'] - } - - join_tree = JSONAPI::ActiveRelationResourceFinder::JoinTree.new(resource_klass: PostResource, - filters: filters) - - joins = join_tree.joins - assert_hash_equals( - { - root: {alias: 'posts', join_type: :root}, - '' => {alias: 'posts', join_type: :root}, - 'author' => {alias: nil, join_type: :inner, relation_join_hash: {'author' => {}}}, - 'author.comments' => {alias: nil, join_type: :inner, relation_join_hash: { 'author' => { 'comments' => {}}}}, - 'tags' => {alias: nil, join_type: :inner, relation_join_hash: {'tags' => {}}}, - }, - joins) - end - - def test_add_joins_with_fields_from_relationship - filters = { - 'author.name' => ['1'], - 'author.comments.name' => ['Foo'], - 'tags.id' => ['1'] - } - - join_tree = JSONAPI::ActiveRelationResourceFinder::JoinTree.new(resource_klass: PostResource, - filters: filters, - source_relationship: PostResource._relationship(:comments)) - - assert_hash_equals( - { - root: {alias: 'posts', join_type: :root}, - '' => {alias: nil, join_type: :inner, relation_join_hash: {'comments' => {}}}, - 'author' => {alias: nil, join_type: :inner, relation_join_hash: {'comments' => { 'author' => {}}}}, - 'author.comments' => {alias: nil, join_type: :inner, relation_join_hash: { 'comments' => { 'author' => { 'comments' => {}}}}}, - 'tags' => {alias: nil, join_type: :inner, relation_join_hash: {'comments' => { 'tags' => {}}}} - }, - join_tree.joins) - - assert join_tree.joins.keys.include?(:root), 'Root must be a symbol' - refute join_tree.joins.keys.include?('root'), 'Root must be a symbol' - refute join_tree.joins.keys.include?(:tags), 'Relationship names must be a string' - assert join_tree.joins.keys.include?('tags'), 'Relationship names must be a string' - end - - def test_add_joins_with_sub_relationship - relationships = %w(author author.comments tags) - - join_tree = JSONAPI::ActiveRelationResourceFinder::JoinTree.new(resource_klass: PostResource, - relationships: relationships, - source_relationship: PostResource._relationship(:comments)) - - assert_hash_equals( - { - root: {alias: 'posts', join_type: :root}, - '' => {alias: nil, join_type: :inner, relation_join_hash: {'comments' => {}}}, - 'author' => {alias: nil, join_type: :left, relation_join_hash: {'comments' => { 'author' => {}}}}, - 'author.comments' => {alias: nil, join_type: :left, relation_join_hash: { 'comments' => { 'author' => { 'comments' => {}}}}}, - 'tags' => {alias: nil, join_type: :left, relation_join_hash: {'comments' => { 'tags' => {}}}} - }, - join_tree.joins) - end - - def test_add_joins_with_sub_relationship_and_filters - filters = { - 'author.name' => ['1'], - 'author.comments.name' => ['Foo'] - } - - relationships = %w(author author.comments tags) - - join_tree = JSONAPI::ActiveRelationResourceFinder::JoinTree.new(resource_klass: PostResource, - filters:filters, - relationships: relationships, - source_relationship: PostResource._relationship(:comments)) - - assert_hash_equals( - { - root: {alias: 'posts', join_type: :root}, - '' => {alias: nil, join_type: :inner, relation_join_hash: {'comments' => {}}}, - 'author' => {alias: nil, join_type: :inner, relation_join_hash: {'comments' => { 'author' => {}}}}, - 'author.comments' => {alias: nil, join_type: :inner, relation_join_hash: { 'comments' => { 'author' => { 'comments' => {}}}}}, - 'tags' => {alias: nil, join_type: :left, relation_join_hash: {'comments' => { 'tags' => {}}}} - }, - join_tree.joins) - end - - def test_polymorphic_join_belongs_to_just_source - join_tree = JSONAPI::ActiveRelationResourceFinder::JoinTree.new(resource_klass: PictureResource, - source_relationship: PictureResource._relationship(:imageable)) - - joins = join_tree.joins - assert_hash_equals( - { - root: { alias: 'pictures', join_type: :root}, - '#products' => {alias: nil, join_type: :left, relation_join_hash: {'product' => {}}}, - '#documents' => {alias: nil, join_type: :left, relation_join_hash: {'document' => {}}} - }, - joins) - end - - def test_polymorphic_join_belongs_to_filter - filters = {'imageable' => ['Foo']} - join_tree = JSONAPI::ActiveRelationResourceFinder::JoinTree.new(resource_klass: PictureResource, filters: filters) - - joins = join_tree.joins - assert_hash_equals( - { - root: { alias: 'pictures', join_type: :root}, - '' => {alias: 'pictures', join_type: :root}, - 'imageable#products' => {alias: nil, join_type: :left, relation_join_hash: {'product' => {}}}, - 'imageable#documents' => {alias: nil, join_type: :left, relation_join_hash: {'document' => {}}} - }, - joins) - end - - def test_polymorphic_join_belongs_to_filter_on_resource - filters = { - 'imageable#documents.name' => ['foo'] - } - - relationships = %w(imageable file_properties) - join_tree = JSONAPI::ActiveRelationResourceFinder::JoinTree.new(resource_klass: PictureResource, - filters: filters, - relationships: relationships) - assert_hash_equals( - { - root: { alias: 'pictures', join_type: :root}, - '' => {alias: 'pictures', join_type: :root}, - 'imageable#documents' => {alias: nil, join_type: :left, relation_join_hash: {'document' => {}}}, - 'imageable#products' => {alias: nil, join_type: :left, relation_join_hash: {'product' => {}}}, - 'file_properties' => {alias: nil, join_type: :left, relation_join_hash: {'file_properties' => {}}} - }, - join_tree.joins) - end - - def test_polymorphic_join_to_one - relationships = %w(file_properties) - join_tree = JSONAPI::ActiveRelationResourceFinder::JoinTree.new(resource_klass: PictureResource, - relationships: relationships) - assert_hash_equals( - { - root: { alias: 'pictures', join_type: :root}, - '' => {alias: 'pictures', join_type: :root}, - 'file_properties' => {alias: nil, join_type: :left, relation_join_hash: {'file_properties' => {}}} - }, - join_tree.joins) - end - - def test_polymorphic_relationship - relationships = %w(imageable file_properties) - join_tree = JSONAPI::ActiveRelationResourceFinder::JoinTree.new(resource_klass: PictureResource, - relationships: relationships) - assert_hash_equals( - { - root: { alias: 'pictures', join_type: :root}, - '' => {alias: 'pictures', join_type: :root}, - 'imageable#products' => {alias: nil, join_type: :left, relation_join_hash: {'product' => {}}}, - 'imageable#documents' => {alias: nil, join_type: :left, relation_join_hash: {'document' => {}}}, - 'file_properties' => {alias: nil, join_type: :left, relation_join_hash: {'file_properties' => {}}} - }, - join_tree.joins) - end -end From 80285b97b7cc38795e5fcd442d776441738268cd Mon Sep 17 00:00:00 2001 From: Larry Gebhardt Date: Mon, 25 Feb 2019 08:34:29 -0500 Subject: [PATCH 2/2] Add note about duplicate alias check --- .../active_relation_resource_finder/join_manager.rb | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/lib/jsonapi/active_relation_resource_finder/join_manager.rb b/lib/jsonapi/active_relation_resource_finder/join_manager.rb index 0ab087962..06981a0a5 100644 --- a/lib/jsonapi/active_relation_resource_finder/join_manager.rb +++ b/lib/jsonapi/active_relation_resource_finder/join_manager.rb @@ -123,6 +123,13 @@ def add_join_details(join_key, details, check_for_duplicate_alias = true) fail "details already set" if @join_details.has_key?(join_key) @join_details[join_key] = details + # Joins are being tracked as they are added to the built up relation. If the same table is added to a + # relation more than once subsequent versions will be assigned an alias. Depending on the order the joins + # are made the computed aliases may change. The order this library performs the joins was chosen + # to prevent this. However if the relation is reordered it should result in reusing on of the earlier + # aliases (in this case a plain table name). The following check will catch this an raise an exception. + # An exception is appropriate because not using the correct alias could leak data due to filters and + # applied permissions being performed on the wrong data. if check_for_duplicate_alias && @collected_aliases.include?(details[:alias]) fail "alias '#{details[:alias]}' has already been added. Possible relation reordering" end @@ -166,6 +173,8 @@ def perform_joins(records, options) end end + # We're adding the source alias with two keys. We only want the check for duplicate aliases once. + # See the note in `add_join_details`. check_for_duplicate_alias = !(relationship == source_relationship) add_join_details(PathSegment::Relationship.new(relationship: relationship, resource_klass: related_resource_klass), details, check_for_duplicate_alias) end