Merge pull request cerebris#1183 from hatchloyalty/pr-0-9-3-filter-included-resources

Implement Included Resource Filtering

Author: lgebhardt

lib/jsonapi/include_directives.rb

@@ -40,6 +40,16 @@ def paths
       delve_paths(get_includes(@include_directives_hash, false))
     end
 
+    def merge_filter(relation, filter)
+      config = include_config(relation.to_sym)
+      config[:include_filters] ||= {}
+      config[:include_filters].merge!(filter)
+    end
+
+    def include_config(relation)
+      @include_directives_hash[:include_related][relation]
+    end
+
     private
 
     def get_related(current_path)

lib/jsonapi/request_parser.rb

@@ -76,6 +76,8 @@ def setup_get_related_resources_action(params)
     def setup_show_action(params)
       parse_fields(params[:fields])
       parse_include_directives(params[:include])
+      parse_filters(params[:filter])
+
       @id = params[:id]
       add_show_operation
     end
@@ -232,7 +234,7 @@ def parse_include_directives(raw_include)
         @include_directives = JSONAPI::IncludeDirectives.new(@resource_klass, result)
       rescue JSONAPI::Exceptions::InvalidInclude => e
         @errors.concat(e.errors)
-        @include_directives = {}
+        @include_directives = JSONAPI::IncludeDirectives.new(@resource_klass, [])
       end
     end
 
@@ -249,11 +251,33 @@ def parse_filters(filters)
       end
 
       filters.each do |key, value|
-        filter = unformat_key(key)
-        if @resource_klass._allowed_filter?(filter)
-          @filters[filter] = value
+        filter_method, included_resource_name =
+          key.to_s.split('.').map { |k| unformat_key(k) }.reverse
+
+        if included_resource_name
+          relationship = resource_klass._relationship(included_resource_name || '')
+
+          unless relationship
+            return @errors.concat(Exceptions::FilterNotAllowed.new(filter_method).errors)
+          end
+
+          unless  relationship.resource_klass._allowed_filter?(filter_method)
+            return @errors.concat(Exceptions::FilterNotAllowed.new(filter_method).errors)
+          end
+
+          unless @include_directives.include_config(relationship.name.to_sym).present?
+            return @errors.concat(Exceptions::FilterNotAllowed.new(filter_method).errors)
+          end
+
+          verified_filter = relationship.resource_klass.verify_filters(filter_method => value)
+          @include_directives.merge_filter(relationship.name, verified_filter)
+          next
         else
-          fail JSONAPI::Exceptions::FilterNotAllowed.new(filter)
+          unless resource_klass._allowed_filter?(filter_method)
+            return @errors.concat(Exceptions::FilterNotAllowed.new(filter_method).errors)
+          end
+
+          @filters[filter_method] = value
         end
       end
     end

lib/jsonapi/resource.rb

@@ -732,9 +732,30 @@ def apply_filters(records, filters, options = {})
         records
       end
 
+      def apply_included_resources_filters(records, options = {})
+        include_directives = options[:include_directives]
+        return records unless include_directives
+        related_directives = include_directives.include_directives.fetch(:include_related)
+        related_directives.reduce(records) do |memo, (relationship_name, config)|
+          relationship = _relationship(relationship_name)
+          next memo unless relationship && relationship.is_a?(JSONAPI::Relationship::ToMany)
+          filtering_resource = relationship.resource_klass
+
+          # Don't try to merge where clauses when relation isn't already being joined to query.
+          next memo unless config[:include_in_join]
+
+          filters = config[:include_filters]
+          next memo unless filters
+
+          rel_records = filtering_resource.apply_filters(filtering_resource.records(options), filters, options).references(relationship_name)
+          memo.merge(rel_records)
+        end
+      end
+
       def filter_records(filters, options, records = records(options))
         records = apply_filters(records, filters, options)
-        apply_includes(records, options)
+        records = apply_includes(records, options)
+        apply_included_resources_filters(records, options)
       end
 
       def sort_records(records, order_options, context = {})

lib/jsonapi/resource_serializer.rb

@@ -287,6 +287,7 @@ def relationships_hash(source, fetchable_fields, include_directives = {})
         include_linkage = ia && ia[:include]
         include_linked_children = ia && !ia[:include_related].empty?
 
+        options = { filters: ia && ia[:include_filters] || {} }
         if field_set.include?(name)
           hash[format_key(name)] = link_object(source, relationship, include_linkage)
         end
@@ -298,7 +299,7 @@ def relationships_hash(source, fetchable_fields, include_directives = {})
           resources = if source.preloaded_fragments.has_key?(format_key(name))
             source.preloaded_fragments[format_key(name)].values
           else
-            [source.public_send(name)].flatten(1).compact
+            [source.public_send(name, options)].flatten(1).compact
           end
           resources.each do |resource|
             next if self_referential_and_already_in_source(resource)
@@ -410,7 +411,10 @@ def to_many_linkage(source, relationship)
           end
         end
       else
-        source.public_send(relationship.name).map do |value|
+        include_config = include_directives.include_config(relationship.name.to_sym) if include_directives
+        include_filters = include_config[:include_filters] if include_config
+        options = { filters: include_filters || {} }
+        source.public_send(relationship.name, options).map do |value|
           [relationship.type, value.id]
         end
       end

test/controllers/controller_test.rb

@@ -509,6 +509,22 @@ def test_show_single_with_includes
     assert_equal 2, json_response['included'].size
   end
 
+  def test_show_with_filtered_includes_when_not_eager_loaded
+    # tags are not eagerly loaded on a post but may still be filtered
+    get :show, params: { id: '1', include: 'tags', filter: { 'tags.name' => ['whiny'] } }
+    assert_response :success
+    assert_equal 1, json_response['included'].size
+  end
+
+  def test_show_with_filtered_includes_when_not_eager_loaded_and_all_filtered_out
+    get :show, params: { id: '1', include: 'tags', filter: { 'tags.name' => ['no-tag-with-this-name'] } }
+    assert_response :success
+    assert json_response['data'].is_a?(Hash)
+    assert_equal 'New post', json_response['data']['attributes']['title']
+    assert_equal 'A body!!!', json_response['data']['attributes']['body']
+    assert_nil json_response['included']
+  end
+
   def test_show_single_with_include_disallowed
     JSONAPI.configuration.allow_include = false
     assert_cacheable_get :show, params: {id: '1', include: 'comments'}
@@ -2577,6 +2593,50 @@ def test_index_with_caching_enabled_uses_context
   end
 end
 
+class Api::V5::PaintersControllerTest < ActionController::TestCase
+  def test_index_with_included_resources_with_filters
+    # There are two painters, but by filtering the included relationship, the
+    # painters are limited due to the join, thus only the painter with oil
+    # paintings is returned.
+    get :index, params: { include: 'paintings', filter: { 'paintings.category' => 'oil' } }
+    assert_response :success
+    assert_equal 1, json_response['data'].size, 'Size of data is wrong'
+    assert_equal '1', json_response['data'][0]['id']
+    assert_equal 2, json_response['included'].size, 'Size of included data is wrong'
+    assert_equal '4', json_response['included'][0]['id']
+    assert_equal '5', json_response['included'][1]['id']
+  end
+
+  def test_index_with_filters_and_included_resources_with_filters
+    get :index, params: { include: 'paintings', filter: { 'name' => 'Wyspianski', 'paintings.category' => 'oil' } }
+
+    assert_response :success
+    assert_equal 1, json_response['data'].size
+    assert_equal '1', json_response['data'][0]['id']
+    assert_equal 2, json_response['included'].size
+    assert_equal '4', json_response['included'][0]['id']
+  end
+
+  def test_index_with_filters_and_included_resources_with_multiple_filters
+    # Painting 5 is the genuine, but painting 6 is a fake. Verify that multiple nested filters are merged and only the oil painting is returned.
+    get :index, params: { include: 'paintings', filter: { 'name' => 'Wyspianski', 'paintings.category' => 'oil', 'paintings.title' => 'Motherhood' } }
+
+    assert_response :success
+    assert_equal 1, json_response['data'].size
+    assert_equal '1', json_response['data'][0]['id']
+    assert_equal 1, json_response['included'].size
+    assert_equal '5', json_response['included'][0]['id']
+  end
+
+  def test_show_with_filters_and_included_resources_with_filters
+    get :show, params: { id: 1, include: 'paintings', filter: { 'paintings.category' => 'oil' } }
+    assert_response :success
+    assert_equal '1', json_response['data']['id']
+    assert_equal 2, json_response['included'].size
+    assert_equal '4', json_response['included'][0]['id']
+  end
+end
+
 class Api::V5::AuthorsControllerTest < ActionController::TestCase
   def test_get_person_as_author
     assert_cacheable_get :index, params: {filter: {id: '1'}}

test/fixtures/active_record.rb

@@ -310,6 +310,24 @@
     t.string :name
   end
 
+  create_table :painters, force: true do |t|
+    t.string :name
+
+    t.timestamps null: false
+  end
+
+  create_table :paintings, force: true do |t|
+    t.string :title
+    t.string :category
+    t.belongs_to :painter
+
+    t.timestamps null: false
+  end
+
+  create_table :collectors, force: true do |t|
+    t.string :name
+    t.belongs_to :painting
+  end
   # special cases
 end
 
@@ -655,6 +673,18 @@ class Customer < Customer
   end
 end
 
+class Painter < ActiveRecord::Base
+  has_many :paintings
+end
+
+class Painting < ActiveRecord::Base
+  belongs_to :painter
+  has_many :collectors
+end
+
+class Collector < ActiveRecord::Base
+  belongs_to :painting
+end
 ### CONTROLLERS
 class AuthorsController < JSONAPI::ResourceControllerMetal
 end
@@ -865,6 +895,9 @@ class ExpenseEntriesController < JSONAPI::ResourceController
 
     class IsoCurrenciesController < JSONAPI::ResourceController
     end
+
+    class PaintersController < JSONAPI::ResourceController
+    end
   end
 
   module V6
@@ -1022,6 +1055,7 @@ class TagResource < JSONAPI::Resource
   attributes :name
 
   has_many :posts
+  filter :name
   # Not including the planets relationship so they don't get output
   #has_many :planets
 end
@@ -1610,6 +1644,42 @@ class AuthorDetailResource < JSONAPI::Resource
       attributes :author_stuff
     end
 
+    class PaintingResource < JSONAPI::Resource
+      model_name 'Painting'
+      attributes :title, :category, :collector_roster
+      has_one :painter
+      has_many :collectors
+
+      filter :title
+      filter :category
+
+      def collector_roster
+        collectors.map(&:name)
+      end
+    end
+
+    class CollectorResource < JSONAPI::Resource
+      attributes :name
+      has_one :painting
+    end
+
+    class PainterResource < JSONAPI::Resource
+      model_name 'Painter'
+      attributes :name
+      has_many :paintings
+
+      filter :name, apply: lambda { |records, value, options|
+        records.where('name LIKE ?', value)
+      }
+
+      def records_for(relation_name)
+        records = super(relation_name)
+
+        return records unless relation_name == :paintings
+        records.includes(:collectors)
+      end
+    end
+
     class PersonResource < PersonResource; end
     class PostResource < PostResource; end
     class TagResource < TagResource; end

test/fixtures/collectors.yml

@@ -0,0 +1,9 @@
+collector_1:
+  id: 1
+  name: "Alice"
+  painting_id: 4
+
+collector_2:
+  id: 2
+  name: "Bob"
+  painting_id: 4

test/fixtures/painters.yml

@@ -0,0 +1,7 @@
+painter_1:
+  id: 1
+  name: "Wyspianski"
+
+painter_2:
+  id: 2
+  name: "Matejko"

test/fixtures/paintings.yml

@@ -0,0 +1,35 @@
+painting_1:
+  id: 1
+  title: "Rejtan"
+  category: "historic"
+  painter_id: 2
+
+painting_2:
+  id: 2
+  title: "Stanczyk"
+  category: "fantasy"
+  painter_id: 2
+
+painting_3:
+  id: 3
+  title: "Macierzynstwo"
+  category: "pastel"
+  painter_id: 1
+
+painting_4:
+  id: 4
+  title: "Helenka"
+  category: "oil"
+  painter_id: 1
+
+painting_5:
+  id: 5
+  title: "Motherhood"
+  category: "oil"
+  painter_id: 1
+
+painting_6:
+  id: 6
+  title: "Motherhood"
+  category: "fake"
+  painter_id: 1

test/test_helper.rb

@@ -327,7 +327,7 @@ class CatResource < JSONAPI::Resource
     namespace :v5 do
       jsonapi_resources :posts do
       end
-
+      jsonapi_resources :painters
       jsonapi_resources :authors
       jsonapi_resources :expense_entries
       jsonapi_resources :iso_currencies