Merge pull request #230 from brettchalupa/request-filters-each-check

Only loop through filters if #each is implemented

Author: lgebhardt

README.md

@@ -736,6 +736,7 @@ module JSONAPI
   INVALID_PAGE_OBJECT = 117
   INVALID_PAGE_VALUE = 118
   INVALID_FIELD_FORMAT = 119
+  INVALID_FILTERS_SYNTAX = 120
   FORBIDDEN = 403
   RECORD_NOT_FOUND = 404
   UNSUPPORTED_MEDIA_TYPE = 415

lib/jsonapi/error_codes.rb

@@ -18,6 +18,7 @@ module JSONAPI
   INVALID_PAGE_OBJECT = 117
   INVALID_PAGE_VALUE = 118
   INVALID_FIELD_FORMAT = 119
+  INVALID_FILTERS_SYNTAX = 120
   FORBIDDEN = 403
   RECORD_NOT_FOUND = 404
   UNSUPPORTED_MEDIA_TYPE = 415
@@ -46,5 +47,6 @@ module JSONAPI
       FORBIDDEN => 'FORBIDDEN',
       RECORD_NOT_FOUND => 'RECORD_NOT_FOUND',
       UNSUPPORTED_MEDIA_TYPE => 'UNSUPPORTED_MEDIA_TYPE',
-      LOCKED => 'LOCKED' }
+      LOCKED => 'LOCKED',
+      INVALID_FILTERS_SYNTAX => 'INVALID_FILTERS_SYNTAX' }
 end

lib/jsonapi/exceptions.rb

@@ -67,6 +67,20 @@ def errors
       end
     end
 
+    class InvalidFiltersSyntax < Error
+      attr_accessor :filters
+      def initialize(filters)
+        @filters = filters
+      end
+
+      def errors
+        [JSONAPI::Error.new(code: JSONAPI::INVALID_FILTERS_SYNTAX,
+                             status: :bad_request,
+                             title: 'Invalid filters syntax',
+                             detail: "#{filters} is not a valid syntax for filtering.")]
+      end
+    end
+
     class FilterNotAllowed < Error
       attr_accessor :filter
       def initialize(filter)

lib/jsonapi/request.rb

@@ -211,6 +211,11 @@ def parse_include_directives(include)
     def parse_filters(filters)
       return unless filters
 
+      unless filters.class.method_defined?(:each)
+        @errors.concat(JSONAPI::Exceptions::InvalidFiltersSyntax.new(filters).errors)
+        return
+      end
+
       filters.each do |key, value|
         filter = unformat_key(key)
         if @resource_klass._allowed_filter?(filter)

test/unit/jsonapi_request/jsonapi_request_test.rb

@@ -1,5 +1,16 @@
 require File.expand_path('../../../test_helper', __FILE__)
 
+class CatResource < JSONAPI::Resource
+  attribute :id
+  attribute :name
+  attribute :breed
+
+  has_one :mother, class_name: 'Cat'
+  has_one :father, class_name: 'Cat'
+
+  filters :name
+end
+
 class JSONAPIRequestTest < ActiveSupport::TestCase
   def test_parse_includes_underscored
     params = ActionController::Parameters.new(
@@ -149,4 +160,41 @@ def test_parse_dasherized_with_underscored_resource
     refute request.errors.empty?
     assert_equal 'expense_entries is not a valid resource.', request.errors[0].detail
   end
+
+  def test_parse_filters_with_valid_filters
+    setup_request
+    @request.parse_filters({name: 'Whiskers'})
+    assert_equal(@request.filters[:name], 'Whiskers')
+    assert_equal(@request.errors, [])
+  end
+
+  def test_parse_filters_with_non_valid_filter
+    setup_request
+    @request.parse_filters({breed: 'Whiskers'}) # breed is not a set filter
+    assert_equal(@request.filters, {})
+    assert_equal(@request.errors.count, 1)
+    assert_equal(@request.errors.first.title, "Filter not allowed")
+  end
+
+  def test_parse_filters_with_no_filters
+    setup_request
+    @request.parse_filters(nil)
+    assert_equal(@request.filters, {})
+    assert_equal(@request.errors, [])
+  end
+
+  def test_parse_filters_with_invalid_filters_param
+    setup_request
+    @request.parse_filters('noeach') # String does not implement #each
+    assert_equal(@request.filters, {})
+    assert_equal(@request.errors.count, 1)
+    assert_equal(@request.errors.first.title, "Invalid filters syntax")
+  end
+
+  private
+
+  def setup_request
+    @request = JSONAPI::Request.new
+    @request.resource_klass = CatResource
+  end
 end