Merge pull request #1191 from cerebris/join_tree

Automatic join aliases, relationship filters, and cleanup

Author: lgebhardt

.travis.yml

@@ -1,15 +1,15 @@
 language: ruby
 sudo: false
 env:
-  - "RAILS_VERSION=4.2.10"
-  - "RAILS_VERSION=5.0.7"
-  - "RAILS_VERSION=5.1.6"
-  - "RAILS_VERSION=5.2.1"
+  - "RAILS_VERSION=4.2.11"
+  - "RAILS_VERSION=5.0.7.1"
+  - "RAILS_VERSION=5.1.6.1"
+  - "RAILS_VERSION=5.2.2"
   - "RAILS_VERSION=master"
 rvm:
-  - 2.3.7
-  - 2.4.4
-  - 2.5.1
+  - 2.3.8
+  - 2.4.5
+  - 2.5.3
 matrix:
   allow_failures:
     - env: "RAILS_VERSION=master"

Gemfile

@@ -19,5 +19,6 @@ when 'master'
 when 'default'
   gem 'railties', '>= 5.0'
 else
+  gem 'left_join' if version.start_with?('4.2')
   gem 'railties', "~> #{version}"
 end

lib/jsonapi-resources.rb

@@ -25,4 +25,8 @@
 require 'jsonapi/callbacks'
 require 'jsonapi/link_builder'
 require 'jsonapi/active_relation_resource_finder'
+require 'jsonapi/active_relation_resource_finder/join_tree'
 require 'jsonapi/resource_identity'
+require 'jsonapi/resource_fragment'
+require 'jsonapi/resource_id_tree'
+require 'jsonapi/resource_set'

lib/jsonapi/active_relation_resource_finder.rb

@@ -0,0 +1,126 @@
+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
+
+      def initialize(resource_klass:, options: {}, source_relationship: nil, filters: nil, sort_criteria: nil)
+        @resource_klass = resource_klass
+        @options = options
+        @source_relationship = source_relationship
+
+        @join_relationships = {}
+
+        add_sort_criteria(sort_criteria)
+        add_filters(filters)
+      end
+
+      # A hash of joins that can be used to create the required joins
+      def get_joins
+        walk_relation_node(@join_relationships)
+      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
+
+      private
+
+      def add_join_relationship(parent_joins, join_name, relation_name, type)
+        parent_joins[join_name] ||= {relation_name: relation_name, relationship: {}, type: type}
+        if parent_joins[join_name][:type] == :left && type == :inner
+          parent_joins[join_name][:type] = :inner
+        end
+        parent_joins[join_name][:relationship]
+      end
+
+      def add_join(path, default_type = :inner)
+        relationships, _field = resource_klass.parse_relationship_path(path)
+
+        current_joins = @join_relationships
+
+        terminated = false
+
+        relationships.each do |relationship|
+          if terminated
+            # ToDo: Relax this, if possible
+            # :nocov:
+            warn "Can not nest joins under polymorphic join"
+            # :nocov:
+          end
+
+          if relationship.polymorphic?
+            relation_names = relationship.polymorphic_relations
+            relation_names.each do |relation_name|
+              join_name = "#{relationship.name}[#{relation_name}]"
+              add_join_relationship(current_joins, join_name, relation_name, :left)
+            end
+            terminated = true
+          else
+            join_name = relationship.name
+            current_joins = add_join_relationship(current_joins, join_name, relationship.relation_name(options), default_type)
+          end
+        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 alow 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 walk_relation_node(node, paths = {}, current_relation_path = [], current_relationship_path = [])
+        node.each do |key, value|
+          if current_relation_path.empty? && source_relationship
+            current_relation_path << source_relationship.relation_name(options)
+          end
+
+          current_relation_path << value[:relation_name].to_s
+          current_relationship_path << key.to_s
+
+          rel_path = current_relationship_path.join('.')
+          paths[rel_path] ||= {
+              alias: nil,
+              join_type: value[:type],
+              relation_join_hash: relation_join_hash(current_relation_path.dup)
+          }
+
+          walk_relation_node(value[:relationship],
+                             paths,
+                             current_relation_path,
+                             current_relationship_path)
+
+          current_relation_path.pop
+          current_relationship_path.pop
+        end
+        paths
+      end
+    end
+  end
+end

lib/jsonapi/active_relation_resource_finder/join_tree.rb

@@ -60,15 +60,19 @@ def index_related_resources
     end
 
     def get_related_resource
-      ActiveSupport::Deprecation.warn "In #{self.class.name} you exposed a `get_related_resource`"\
+      # :nocov:
+    ActiveSupport::Deprecation.warn "In #{self.class.name} you exposed a `get_related_resource`"\
                                       " action. Please use `show_related_resource` instead."
       show_related_resource
+      # :nocov:
     end
 
     def get_related_resources
+      # :nocov:
       ActiveSupport::Deprecation.warn "In #{self.class.name} you exposed a `get_related_resources`"\
                                       " action. Please use `index_related_resource` instead."
       index_related_resources
+      # :nocov:
     end
 
     def process_request

lib/jsonapi/acts_as_resource_controller.rb

@@ -1,7 +1,7 @@
 module JSONAPI
   module Exceptions
     class Error < RuntimeError
-      attr :error_object_overrides
+      attr_reader :error_object_overrides
 
       def initialize(error_object_overrides = {})
         @error_object_overrides = error_object_overrides

lib/jsonapi/exceptions.rb

@@ -19,9 +19,8 @@ class IncludeDirectives
     #   }
     # }
 
-    def initialize(resource_klass, includes_array, force_eager_load: false)
+    def initialize(resource_klass, includes_array)
       @resource_klass = resource_klass
-      @force_eager_load = force_eager_load
       @include_directives_hash = { include_related: {} }
       includes_array.each do |include|
         parse_include(include)
@@ -32,16 +31,6 @@ def include_directives
       @include_directives_hash
     end
 
-    def model_includes
-      get_includes(@include_directives_hash)
-    end
-
-    # :nocov:
-    def all_paths
-      delve_paths(get_includes(@include_directives_hash, false))
-    end
-    # :nocov:
-
     private
 
     def get_related(current_path)
@@ -57,24 +46,13 @@ def get_related(current_path)
           raise JSONAPI::Exceptions::InvalidInclude.new(current_resource_klass, current_path)
         end
 
-        include_in_join = @force_eager_load || !current_relationship || current_relationship.eager_load_on_include
 
-        current[:include_related][fragment] ||= { include: false, include_related: {}, include_in_join: include_in_join }
+        current[:include_related][fragment] ||= { include: false, include_related: {} }
         current = current[:include_related][fragment]
       end
       current
     end
 
-    def get_includes(directive, only_joined_includes = true)
-      ir = directive[:include_related]
-      ir = ir.select { |_k,v| v[:include_in_join] } if only_joined_includes
-
-      ir.map do |name, sub_directive|
-        sub = get_includes(sub_directive, only_joined_includes)
-        sub.any? ? { name => sub } : name
-      end
-    end
-
     def parse_include(include)
       parts = include.split('.')
       local_path = ''
@@ -85,21 +63,5 @@ def parse_include(include)
         related[:include] = true
       end
     end
-
-    # :nocov:
-    def delve_paths(obj)
-      case obj
-        when Array
-          obj.map{|elem| delve_paths(elem)}.flatten(1)
-        when Hash
-          obj.map{|k,v| [[k]] + delve_paths(v).map{|path| [k] + path } }.flatten(1)
-        when Symbol, String
-          [[obj]]
-        else
-          raise "delve_paths cannot descend into #{obj.class.name}"
-      end
-    end
-    # :nocov:
-
   end
 end

lib/jsonapi/include_directives.rb

@@ -61,8 +61,10 @@ def build_engine_name
         unless scopes.empty?
           "#{ scopes.first.to_s.camelize }::Engine".safe_constantize
         end
+      # :nocov:
       rescue LoadError => _e
         nil
+      # :nocov:
       end
     end
 
@@ -139,7 +141,9 @@ def regular_primary_resources_url
 
     def regular_resource_path(source)
       if source.is_a?(JSONAPI::CachedResponseFragment)
+        # :nocov:
         "#{regular_resources_path(source.resource_klass)}/#{source.id}"
+        # :nocov:
       else
         "#{regular_resources_path(source.class)}/#{source.id}"
       end

lib/jsonapi/link_builder.rb

@@ -49,7 +49,7 @@ def initialize(code, resource_set, options = {})
 
     def to_hash(serializer)
       if serializer
-        serializer.serialize_resource_set_to_hash(resource_set)
+        serializer.serialize_resource_set_to_hash_single(resource_set)
       else
         # :nocov:
         {}
@@ -71,7 +71,7 @@ def initialize(code, resource_set, options = {})
 
     def to_hash(serializer)
       if serializer
-        serializer.serialize_resources_set_to_hash(resource_set)
+        serializer.serialize_resource_set_to_hash_plural(resource_set)
       else
         # :nocov:
         {}
@@ -91,7 +91,7 @@ def initialize(code, source_resource, type, resource_set, options = {})
 
     def to_hash(serializer = nil)
       if serializer
-        serializer.serialize_related_resources_set_to_hash(source_resource, resource_set)
+        serializer.serialize_related_resource_set_to_hash_plural(resource_set, source_resource)
       else
         # :nocov:
         {}