Merge pull request #128 from cerebris/format_fix

Format fix

lib/jsonapi/error_codes.rb

@@ -18,6 +18,7 @@ module JSONAPI
   INVALID_PAGE_OBJECT = 117
   INVALID_PAGE_VALUE = 118
   INVALID_SORT_FORMAT = 119
+  INVALID_FIELD_FORMAT = 120
   RECORD_NOT_FOUND = 404
   UNSUPPORTED_MEDIA_TYPE = 415
   LOCKED = 423

lib/jsonapi/exceptions.rb

@@ -102,6 +102,15 @@ def errors
       end
     end
 
+    class InvalidFieldFormat < Error
+      def errors
+        [JSONAPI::Error.new(code: JSONAPI::INVALID_FIELD_FORMAT,
+                            status: :bad_request,
+                            title: 'Invalid field format',
+                            detail: 'Fields must specify a type.')]
+      end
+    end
+
     class InvalidLinksObject < Error
       def errors
         [JSONAPI::Error.new(code: JSONAPI::INVALID_LINKS_OBJECT,

lib/jsonapi/formatter.rb

@@ -23,7 +23,7 @@ def format(key)
       end
 
       def unformat(formatted_key)
-        super.to_sym
+        super
       end
     end
   end
@@ -35,7 +35,7 @@ def format(route)
       end
 
       def unformat(formatted_route)
-        super.to_sym
+        super
       end
     end
   end
@@ -68,7 +68,7 @@ def format(key)
     end
 
     def unformat(formatted_key)
-      formatted_key.to_s.underscore.to_sym
+      formatted_key.to_s.underscore
     end
   end
 end
@@ -80,7 +80,7 @@ def format(key)
     end
 
     def unformat(formatted_key)
-      formatted_key.to_s.underscore.to_sym
+      formatted_key.to_s.underscore
     end
   end
 end
@@ -112,7 +112,7 @@ def format(route)
     end
 
     def unformat(formatted_route)
-      formatted_route.to_s.underscore.to_sym
+      formatted_route.to_s.underscore
     end
   end
 end
@@ -124,7 +124,7 @@ def format(route)
     end
 
     def unformat(formatted_route)
-      formatted_route.to_s.underscore.to_sym
+      formatted_route.to_s.underscore
     end
   end
 end

lib/jsonapi/request.rb

@@ -82,26 +82,30 @@ def parse_fields(fields)
       extracted_fields = {}
 
       # Extract the fields for each type from the fields parameters
-      if fields.is_a?(String)
-        resource_fields = fields.split(',') unless fields.empty?
-        type = @resource_klass._type
-        extracted_fields[type] = resource_fields
-      elsif fields.is_a?(ActionController::Parameters)
+      if fields.is_a?(ActionController::Parameters)
         fields.each do |field, value|
           resource_fields = value.split(',') unless value.nil? || value.empty?
           extracted_fields[field] = resource_fields
         end
+      else
+        raise JSONAPI::Exceptions::InvalidFieldFormat.new
       end
 
       # Validate the fields
       extracted_fields.each do |type, values|
         underscored_type = unformat_key(type)
         extracted_fields[type] = []
         begin
+          if type != format_key(type)
+            raise JSONAPI::Exceptions::InvalidResource.new(type)
+          end
           type_resource = Resource.resource_for(@resource_klass.module_path + underscored_type.to_s)
         rescue NameError
           @errors.concat(JSONAPI::Exceptions::InvalidResource.new(type).errors)
+        rescue JSONAPI::Exceptions::InvalidResource => e
+        @errors.concat(e.errors)
         end
+
         if type_resource.nil? || !(@resource_klass._type == underscored_type ||
           @resource_klass._has_association?(underscored_type))
           @errors.concat(JSONAPI::Exceptions::InvalidResource.new(type).errors)
@@ -128,7 +132,7 @@ def check_include(resource_klass, include_parts)
       association_name = unformat_key(include_parts.first)
 
       association = resource_klass._association(association_name)
-      if association
+      if association && format_key(association_name) == include_parts.first
         unless include_parts.last.empty?
           check_include(Resource.resource_for(@resource_klass.module_path + association.class_name.to_s), include_parts.last.partition('.'))
         end
@@ -155,7 +159,7 @@ def parse_filters(filters)
       return unless filters
       @filters = {}
       filters.each do |key, value|
-        filter = unformat_key(key).to_sym
+        filter = unformat_key(key)
         if @resource_klass._allowed_filter?(filter)
           @filters[filter] = value
         else
@@ -211,7 +215,7 @@ def parse_add_operation(data)
 
     def verify_and_remove_type(params)
       #remove type and verify it matches the resource
-      if params[:type] == @resource_klass._type.to_s
+      if unformat_key(params[:type]) == @resource_klass._type
         params.delete(:type)
       else
         if params[:type].nil?
@@ -463,7 +467,8 @@ def format_key(key)
     end
 
     def unformat_key(key)
-      @key_formatter.unformat(key)
+      unformatted_key = @key_formatter.unformat(key)
+      unformatted_key.nil? ? nil : unformatted_key.to_sym
     end
   end
 end

test/controllers/controller_test.rb

@@ -54,7 +54,7 @@ def test_index_filter_by_ids_and_include_related_different_type
   end
 
   def test_index_filter_by_ids_and_fields
-    get :index, {filter: {id: '1,2'}, 'fields' => 'id,title,author'}
+    get :index, {filter: {id: '1,2'}, fields: {posts: 'id,title,author'}}
     assert_response :success
     assert_equal 2, json_response['data'].size
 
@@ -86,7 +86,7 @@ def test_index_filter_by_ids_and_fields_specify_unrelated_type
   end
 
   def test_index_filter_by_ids_and_fields_2
-    get :index, {filter: {id: '1,2'}, 'fields' => 'author'}
+    get :index, {filter: {id: '1,2'}, fields: {posts: 'author'}}
     assert_response :success
     assert_equal 2, json_response['data'].size
 
@@ -149,12 +149,6 @@ def test_bad_filter_value_not_found
     assert_match /5412333 could not be found/, json_response['errors'][0]['detail']
   end
 
-  def test_index_malformed_fields
-    get :index, {filter: {id: '1,2'}, 'fields' => 'posts'}
-    assert_response :bad_request
-    assert_match /posts is not a valid field for posts./, json_response['errors'][0]['detail']
-  end
-
   def test_field_not_supported
     get :index, {filter: {id: '1,2'}, 'fields' => {'posts' => 'id,title,rank,author'}}
     assert_response :bad_request
@@ -237,14 +231,20 @@ def test_show_single_with_includes
   end
 
   def test_show_single_with_fields
-    get :show, {id: '1', fields: 'author'}
+    get :show, {id: '1', fields: {posts: 'author'}}
     assert_response :success
     assert json_response['data'].is_a?(Hash)
     assert_nil json_response['data']['title']
     assert_nil json_response['data']['body']
     assert_equal '1', json_response['data']['links']['author']['linkage']['id']
   end
 
+  def test_show_single_with_fields_string
+    get :show, {id: '1', fields: 'author'}
+    assert_response :bad_request
+    assert_match /Fields must specify a type./, json_response['errors'][0]['detail']
+  end
+
   def test_show_single_invalid_id_format
     get :show, {id: 'asdfg'}
     assert_response :bad_request
@@ -260,7 +260,7 @@ def test_show_single_missing_record
   def test_show_malformed_fields_not_list
     get :show, {id: '1', 'fields' => ''}
     assert_response :bad_request
-    assert_match /nil is not a valid field for posts./, json_response['errors'][0]['detail']
+    assert_match /Fields must specify a type./, json_response['errors'][0]['detail']
   end
 
   def test_show_malformed_fields_type_not_list
@@ -542,7 +542,7 @@ def test_create_with_links_include_and_fields
              }
            },
            include: 'author,author.posts',
-           fields: 'id,title,author'
+           fields: {posts: 'id,title,author'}
          }
 
     assert_response :created
@@ -1358,13 +1358,6 @@ def test_expense_entries_show_bad_include_missing_sub_association
   end
 
   def test_expense_entries_show_fields
-    get :show, {id: 1, include: 'isoCurrency,employee', 'fields' => 'transactionDate'}
-    assert_response :success
-    assert json_response['data'].is_a?(Hash)
-    assert json_response['data'].has_key?('transactionDate')
-  end
-
-  def test_expense_entries_show_fields_type
     get :show, {id: 1, include: 'isoCurrency,employee', 'fields' => {'expenseEntries' => 'transactionDate'}}
     assert_response :success
     assert json_response['data'].is_a?(Hash)
@@ -1397,7 +1390,7 @@ def test_create_expense_entries_underscored
              }
            },
            include: 'iso_currency',
-           fields: 'id,transaction_date,iso_currency,cost,employee'
+           fields: {expense_entries: 'id,transaction_date,iso_currency,cost,employee'}
          }
 
     assert_response :created
@@ -1426,7 +1419,7 @@ def test_create_expense_entries_camelized_key
              }
            },
            include: 'isoCurrency',
-           fields: 'id,transactionDate,isoCurrency,cost,employee'
+           fields: {expenseEntries: 'id,transactionDate,isoCurrency,cost,employee'}
          }
 
     assert_response :created
@@ -1455,7 +1448,7 @@ def test_create_expense_entries_dasherized_key
              }
            },
            include: 'iso-currency',
-           fields: 'id,transaction-date,iso-currency,cost,employee'
+           fields: {'expense-entries' => 'id,transaction-date,iso-currency,cost,employee'}
          }
 
     assert_response :created

test/test_helper.rb

@@ -176,7 +176,7 @@ def format(key)
     end
 
     def unformat(formatted_key)
-      formatted_key.to_s.underscore.to_sym
+      formatted_key.to_s.underscore
     end
   end
 end