Refactor polymorphic has many link creation

closes gh-1231
closes gh-1217

Author: st0012

lib/jsonapi/request_parser.rb

@@ -524,13 +524,37 @@ def parse_to_many_relationship(link_value, relationship, &add_result)
       if links_object.length == 0
         add_result.call([])
       else
-        defined_relationship_resource = Resource.resource_for(@resource_klass.module_path + unformat_key(relationship.type).to_s)
-        links_object.each_pair do |type, keys|
-          relationship_resource = Resource.resource_for(@resource_klass.module_path + unformat_key(type).to_s)
-          if !relationship_resource.ancestors.include?(defined_relationship_resource)
+        if relationship.polymorphic?
+          polymorphic_results = []
+
+          links_object.each_pair do |type, keys|
+            resource = self.resource_klass || Resource
+            type_name = unformat_key(type).to_s
+
+            relationship_resource_klass = resource.resource_for(relationship.class_name)
+            relationship_klass = relationship_resource_klass._model_class
+
+            linkage_object_resource_klass = resource.resource_for(type_name)
+            linkage_object_klass = linkage_object_resource_klass._model_class
+
+            unless linkage_object_klass == relationship_klass || linkage_object_klass.in?(relationship_klass.subclasses)
+              fail JSONAPI::Exceptions::TypeMismatch.new(type_name)
+            end
+
+            relationship_ids = relationship_resource_klass.verify_keys(keys, @context)
+             polymorphic_results << { type: type, ids: relationship_ids }
+          end
+
+          add_result.call polymorphic_results
+        else
+          relationship_type = unformat_key(relationship.type).to_s
+
+          if links_object.length > 1 || !links_object.has_key?(relationship_type)
             fail JSONAPI::Exceptions::TypeMismatch.new(links_object[:type])
           end
-          add_result.call relationship_resource.verify_keys(keys, @context)
+
+          relationship_resource = Resource.resource_for(@resource_klass.module_path + relationship_type)
+          add_result.call relationship_resource.verify_keys(links_object[relationship_type], @context)
         end
       end
     end

lib/jsonapi/resource.rb

@@ -302,6 +302,26 @@ def _replace_to_many_links(relationship_type, relationship_key_values, options)
         to_add = relationship_key_values - (relationship_key_values & existing)
         _create_to_many_links(relationship_type, to_add, {})
 
+        @reload_needed = true
+      elsif relationship.polymorphic?
+        relationship_key_values.each do |relationship_key_value|
+          relationship_resource_klass = self.class.resource_for(relationship_key_value[:type])
+          ids = relationship_key_value[:ids]
+
+          related_records = relationship_resource_klass
+            .records(options)
+            .where({relationship_resource_klass._primary_key => ids})
+
+          missed_ids = ids - related_records.pluck(relationship_resource_klass._primary_key)
+
+          if missed_ids.present?
+            fail JSONAPI::Exceptions::RecordNotFound.new(missed_ids)
+          end
+
+          relation_name = relationship.relation_name(context: @context)
+          @model.send("#{relation_name}") << related_records
+        end
+
         @reload_needed = true
       else
         send("#{relationship.foreign_key}=", relationship_key_values)
@@ -565,7 +585,14 @@ def has_many(*attrs)
         _add_relationship(Relationship::ToMany, *attrs)
       end
 
+      # @model_class is inherited from superclass, and this causes some issues:
+      # ```
+      # CarResource._model_class #=> Vehicle # it should be Car
+      # ```
+      # so in order to invoke the right class from subclasses,
+      # we should call this method to override it.
       def model_name(model, options = {})
+        @model_class = nil
         @_model_name = model.to_sym
 
         model_hint(model: @_model_name, resource: self) unless options[:add_model_hint] == false

test/fixtures/active_record.rb

@@ -1028,10 +1028,12 @@ class VehicleResource < JSONAPI::Resource
 end
 
 class CarResource < VehicleResource
+  model_name "Car"
   attributes :drive_layout
 end
 
 class BoatResource < VehicleResource
+  model_name "Boat"
   attributes :length_at_water_line
 end
 

test/fixtures/vehicles.yml

@@ -15,3 +15,21 @@ Launch20:
   length_at_water_line: 15.5ft
   serial_number: 434253JJJSD
   person_id: 1
+
+M5:
+  id: 3
+  type: Car
+  make: BMW
+  model: M5
+  drive_layout: Front Engine RWD
+  serial_number: 56256
+  person_id: 2
+
+M3:
+  id: 4
+  type: Car
+  make: BMW
+  model: M3
+  drive_layout: Front Engine RWD
+  serial_number: 894345
+  person_id: 2

test/integration/requests/request_test.rb

@@ -216,7 +216,7 @@ def test_post_single
     assert_jsonapi_response 201
   end
 
-  def test_post_polymorphic
+  def test_post_polymorphic_with_has_many_relationship
     post '/people', params:
       {
         'data' => {
@@ -227,7 +227,14 @@ def test_post_polymorphic
             'date_joined' => 'Thu, 01 Jan 2019 00:00:00 UTC +00:00',
           },
           'relationships' => {
-            'vehicles' => {'data' => [{'type' => 'car', 'id' => '1'}]},
+            'vehicles' => {
+              'data' => [
+                {'type' => 'car', 'id' => '1'},
+                {'type' => 'boat', 'id' => '2'},
+                {'type' => 'car', 'id' => '3'},
+                {'type' => 'car', 'id' => '4'}
+              ]
+            }
           }
         }
       }.to_json,
@@ -237,9 +244,19 @@ def test_post_polymorphic
       }
 
     assert_jsonapi_response 201
+
+    body = JSON.parse(response.body)
+    person = Person.find(body.dig("data", "id"))
+
+    assert_equal "Reo", person.name
+    assert_equal 4, person.vehicles.count
+    assert_equal Car, person.vehicles.first.class
+    assert_equal Boat, person.vehicles.second.class
+    assert_equal Car, person.vehicles.third.class
+    assert_equal Car, person.vehicles.fourth.class
   end
 
-  def test_post_polymorphic_invalid
+  def test_post_polymorphic_invalid_with_wrong_type
     post '/people', params:
       {
         'data' => {
@@ -262,6 +279,34 @@ def test_post_polymorphic_invalid
     assert_jsonapi_response 400, msg: "Submitting a thing as a vehicle should raise a type mismatch error"
   end
 
+  def test_post_polymorphic_invalid_with_not_matched_type_and_id
+    post '/people', params:
+      {
+        'data' => {
+          'type' => 'people',
+          'attributes' => {
+            'name' => 'Reo',
+            'email' => 'reo@xyz.fake',
+            'date_joined' => 'Thu, 01 Jan 2019 00:00:00 UTC +00:00',
+          },
+          'relationships' => {
+            'vehicles' => {
+              'data' => [
+                {'type' => 'car', 'id' => '1'},
+                {'type' => 'car', 'id' => '2'} #vehicle 2 is actually a boat
+              ]
+            }
+          }
+        }
+      }.to_json,
+      headers: {
+        'CONTENT_TYPE' => JSONAPI::MEDIA_TYPE,
+        'Accept' => JSONAPI::MEDIA_TYPE
+      }
+
+    assert_jsonapi_response 404, msg: "Submitting a thing as a vehicle should raise a record not found"
+  end
+
   def test_post_single_missing_data_contents
     post '/posts', params:
          {
@@ -473,6 +518,96 @@ def test_patch_content_type
     assert_match JSONAPI::MEDIA_TYPE, headers['Content-Type']
   end
 
+  def test_patch_polymorphic_with_has_many_relationship
+    patch '/people/1', params:
+      {
+        'data' => {
+          'id' => 1,
+          'type' => 'people',
+          'attributes' => {
+            'name' => 'Reo',
+            'email' => 'reo@xyz.fake',
+            'date_joined' => 'Thu, 01 Jan 2019 00:00:00 UTC +00:00',
+          },
+          'relationships' => {
+            'vehicles' => {
+              'data' => [
+                {'type' => 'car', 'id' => '1'},
+                {'type' => 'boat', 'id' => '2'}
+              ]
+            }
+          }
+        }
+      }.to_json,
+      headers: {
+        'CONTENT_TYPE' => JSONAPI::MEDIA_TYPE,
+        'Accept' => JSONAPI::MEDIA_TYPE
+      }
+
+    assert_jsonapi_response 200
+
+    body = JSON.parse(response.body)
+    person = Person.find(body.dig("data", "id"))
+
+    assert_equal "Reo", person.name
+    assert_equal 2, person.vehicles.count
+    assert_equal Car, person.vehicles.first.class
+    assert_equal Boat, person.vehicles.second.class
+  end
+
+  def test_patch_polymorphic_invalid_with_wrong_type
+    patch '/people/1', params:
+      {
+        'data' => {
+          'id' => 1,
+          'type' => 'people',
+          'attributes' => {
+            'name' => 'Reo',
+            'email' => 'reo@xyz.fake',
+            'date_joined' => 'Thu, 01 Jan 2019 00:00:00 UTC +00:00',
+          },
+          'relationships' => {
+            'vehicles' => {'data' => [{'type' => 'author', 'id' => '1'}]},
+          }
+        }
+      }.to_json,
+      headers: {
+        'CONTENT_TYPE' => JSONAPI::MEDIA_TYPE,
+        'Accept' => JSONAPI::MEDIA_TYPE
+      }
+
+    assert_jsonapi_response 400, msg: "Submitting a thing as a vehicle should raise a type mismatch error"
+  end
+
+  def test_patch_polymorphic_invalid_with_not_matched_type_and_id
+    patch '/people/1', params:
+      {
+        'data' => {
+          'id' => 1,
+          'type' => 'people',
+          'attributes' => {
+            'name' => 'Reo',
+            'email' => 'reo@xyz.fake',
+            'date_joined' => 'Thu, 01 Jan 2019 00:00:00 UTC +00:00',
+          },
+          'relationships' => {
+            'vehicles' => {
+              'data' => [
+                {'type' => 'car', 'id' => '1'},
+                {'type' => 'car', 'id' => '2'} #vehicle 2 is actually a boat
+              ]
+            }
+          }
+        }
+      }.to_json,
+      headers: {
+        'CONTENT_TYPE' => JSONAPI::MEDIA_TYPE,
+        'Accept' => JSONAPI::MEDIA_TYPE
+      }
+
+    assert_jsonapi_response 404, msg: "Submitting a thing as a vehicle should raise a record not found"
+  end
+
   def test_post_correct_content_type
     post '/posts', params:
       {
@@ -1177,8 +1312,8 @@ def test_include_value_missing
 
   def test_getting_different_resources_when_sti
     assert_cacheable_jsonapi_get '/vehicles'
-    types = json_response['data'].map{|r| r['type']}.sort
-    assert_array_equals ['boats', 'cars'], types
+    types = json_response['data'].map{|r| r['type']}.to_set
+    assert types == Set['cars', 'boats']
   end
 
   def test_getting_resource_with_correct_type_when_sti