Merge pull request #739 from cerebris/reflect_relationship

Add relationship reflection option

Author: lgebhardt

README.md

@@ -545,6 +545,9 @@ The relationship methods (`relationship`, `has_one`, and `has_many`) support the
 `to_one` relationships support the additional option:
  * `foreign_key_on` - defaults to `:self`. To indicate that the foreign key is on the related resource specify `:related`.
 
+`to_many` relationships support the additional option:
+ * `reflect` - defaults to `true`. To indicate that updates to the relationship are performed on the related resource, if relationship reflection is turned on. See [Configuration] (#configuration)
+
 Examples:
 
 ```ruby
@@ -1133,6 +1136,13 @@ Callbacks can be defined for the following `JSONAPI::Resource` events:
 - `:remove_to_one_link`
 - `:replace_fields`
 
+###### Relationship Reflection
+
+By default updates to relationships only invoke callbacks on the primary
+Resource. By setting the `use_relationship_reflection` [Configuration] (#configuration) option
+updates to `has_many` relationships will occur on the related resource, triggering
+callbacks on both resources.
+
 ##### `JSONAPI::Processor` Callbacks
 
 Callbacks can also be defined for `JSONAPI::Processor` events:
@@ -1944,13 +1954,18 @@ JSONAPI.configure do |config|
   # NOTE: always_include_to_many_linkage_data is not currently implemented
   config.always_include_to_one_linkage_data = false
 
+  # Relationship reflection invokes the related resource when updates
+  # are made to a has_many relationship. By default relationship_reflection
+  # is turned off because it imposes a small performance penalty.
+  config.use_relationship_reflection = false
+
   # Allows transactions for creating and updating records
   # Set this to false if your backend does not support transactions (e.g. Mongodb)
-  self.allow_transactions = true
+  config.allow_transactions = true
 
   # Formatter Caching
   # Set to false to disable caching of string operations on keys and links.
-  self.cache_formatters = true
+  config.cache_formatters = true
 end
 ```
 

lib/jsonapi/configuration.rb

@@ -25,7 +25,8 @@ class Configuration
                 :exception_class_whitelist,
                 :always_include_to_one_linkage_data,
                 :always_include_to_many_linkage_data,
-                :cache_formatters
+                :cache_formatters,
+                :use_relationship_reflection
 
     def initialize
       #:underscored_key, :camelized_key, :dasherized_key, or custom
@@ -88,6 +89,11 @@ def initialize
       # Formatter Caching
       # Set to false to disable caching of string operations on keys and links.
       self.cache_formatters = true
+
+      # Relationship reflection invokes the related resource when updates
+      # are made to a has_many relationship. By default relationship_reflection
+      # is turned off because it imposes a small performance penalty.
+      self.use_relationship_reflection = false
     end
 
     def cache_formatters=(bool)
@@ -186,6 +192,8 @@ def default_processor_klass=(default_processor_klass)
     attr_writer :always_include_to_many_linkage_data
 
     attr_writer :raise_if_parameters_not_allowed
+
+    attr_writer :use_relationship_reflection
   end
 
   class << self

lib/jsonapi/relationship.rb

@@ -79,10 +79,14 @@ def polymorphic_type
     end
 
     class ToMany < Relationship
+      attr_reader :reflect, :inverse_relationship
+
       def initialize(name, options = {})
         super
         @class_name = options.fetch(:class_name, name.to_s.camelize.singularize)
         @foreign_key ||= "#{name.to_s.singularize}_ids".to_sym
+        @reflect = options.fetch(:reflect, true) == true
+        @inverse_relationship = options.fetch(:inverse_relationship, parent_resource._type.to_s.singularize.to_sym) if parent_resource
       end
     end
   end

lib/jsonapi/resource.rb

@@ -22,6 +22,9 @@ class Resource
     def initialize(model, context)
       @model = model
       @context = context
+      @reload_needed = false
+      @changing = false
+      @save_needed = false
     end
 
     def _model
@@ -63,39 +66,39 @@ def remove
       end
     end
 
-    def create_to_many_links(relationship_type, relationship_key_values)
+    def create_to_many_links(relationship_type, relationship_key_values, options = {})
       change :create_to_many_link do
-        _create_to_many_links(relationship_type, relationship_key_values)
+        _create_to_many_links(relationship_type, relationship_key_values, options)
       end
     end
 
-    def replace_to_many_links(relationship_type, relationship_key_values)
+    def replace_to_many_links(relationship_type, relationship_key_values, options = {})
       change :replace_to_many_links do
-        _replace_to_many_links(relationship_type, relationship_key_values)
+        _replace_to_many_links(relationship_type, relationship_key_values, options)
       end
     end
 
-    def replace_to_one_link(relationship_type, relationship_key_value)
+    def replace_to_one_link(relationship_type, relationship_key_value, options = {})
       change :replace_to_one_link do
-        _replace_to_one_link(relationship_type, relationship_key_value)
+        _replace_to_one_link(relationship_type, relationship_key_value, options)
       end
     end
 
-    def replace_polymorphic_to_one_link(relationship_type, relationship_key_value, relationship_key_type)
+    def replace_polymorphic_to_one_link(relationship_type, relationship_key_value, relationship_key_type, options = {})
       change :replace_polymorphic_to_one_link do
-        _replace_polymorphic_to_one_link(relationship_type, relationship_key_value, relationship_key_type)
+        _replace_polymorphic_to_one_link(relationship_type, relationship_key_value, relationship_key_type, options)
       end
     end
 
-    def remove_to_many_link(relationship_type, key)
+    def remove_to_many_link(relationship_type, key, options = {})
       change :remove_to_many_link do
-        _remove_to_many_link(relationship_type, key)
+        _remove_to_many_link(relationship_type, key, options)
       end
     end
 
-    def remove_to_one_link(relationship_type)
+    def remove_to_one_link(relationship_type, options = {})
       change :remove_to_one_link do
-        _remove_to_one_link(relationship_type)
+        _remove_to_one_link(relationship_type, options)
       end
     end
 
@@ -189,6 +192,7 @@ def _save(validation_context = nil)
 
       if defined? @model.save
         saved = @model.save(validate: false)
+
         unless saved
           if @model.errors.present?
             fail JSONAPI::Exceptions::ValidationErrors.new(self)
@@ -199,6 +203,8 @@ def _save(validation_context = nil)
       else
         saved = true
       end
+      @model.reload if @reload_needed
+      @reload_needed = false
 
       @save_needed = !saved
 
@@ -215,34 +221,87 @@ def _remove
       fail JSONAPI::Exceptions::RecordLocked.new(e.message)
     end
 
-    def _create_to_many_links(relationship_type, relationship_key_values)
+    def reflect_relationship?(relationship, options)
+      return false if !relationship.reflect ||
+        (!JSONAPI.configuration.use_relationship_reflection || options[:reflected_source])
+
+      inverse_relationship = relationship.resource_klass._relationships[relationship.inverse_relationship]
+      if inverse_relationship.nil?
+        warn "Inverse relationship could not be found for #{self.class.name}.#{relationship.name}. Relationship reflection disabled."
+        return false
+      end
+      true
+    end
+
+    def _create_to_many_links(relationship_type, relationship_key_values, options)
       relationship = self.class._relationships[relationship_type]
 
-      relationship_key_values.each do |relationship_key_value|
-        related_resource = relationship.resource_klass.find_by_key(relationship_key_value, context: @context)
+      # check if relationship_key_values are already members of this relationship
+      relation_name = relationship.relation_name(context: @context)
+      existing_relations = @model.public_send(relation_name).where(relationship.primary_key => relationship_key_values)
+      if existing_relations.count > 0
+        # todo: obscure id so not to leak info
+        fail JSONAPI::Exceptions::HasManyRelationExists.new(existing_relations.first.id)
+      end
 
-        relation_name = relationship.relation_name(context: @context)
-        # TODO: Add option to skip relations that already exist instead of returning an error?
-        relation = @model.public_send(relation_name).where(relationship.primary_key => relationship_key_value).first
-        if relation.nil?
-          @model.public_send(relation_name) << related_resource._model
+      if options[:reflected_source]
+        @model.public_send(relation_name) << options[:reflected_source]._model
+        return :completed
+      end
+
+      # load requested related resources
+      # make sure they all exist (also based on context) and add them to relationship
+
+      related_resources = relationship.resource_klass.find_by_keys(relationship_key_values, context: @context)
+
+      if related_resources.count != relationship_key_values.count
+        # todo: obscure id so not to leak info
+        fail JSONAPI::Exceptions::RecordNotFound.new('unspecified')
+      end
+
+      reflect = reflect_relationship?(relationship, options)
+
+      related_resources.each do |related_resource|
+        if reflect
+          if related_resource.class._relationships[relationship.inverse_relationship].is_a?(JSONAPI::Relationship::ToMany)
+            related_resource.create_to_many_links(relationship.inverse_relationship, [id], reflected_source: self)
+          else
+            related_resource.replace_to_one_link(relationship.inverse_relationship, id, reflected_source: self)
+          end
+          @reload_needed = true
         else
-          fail JSONAPI::Exceptions::HasManyRelationExists.new(relationship_key_value)
+          @model.public_send(relation_name) << related_resource._model
         end
       end
 
       :completed
     end
 
-    def _replace_to_many_links(relationship_type, relationship_key_values)
+    def _replace_to_many_links(relationship_type, relationship_key_values, options)
       relationship = self.class._relationships[relationship_type]
-      send("#{relationship.foreign_key}=", relationship_key_values)
-      @save_needed = true
+
+      reflect = reflect_relationship?(relationship, options)
+
+      if reflect
+        existing = send("#{relationship.foreign_key}")
+        to_delete = existing - (relationship_key_values & existing)
+        to_delete.each do |key|
+          _remove_to_many_link(relationship_type, key, reflected_source: self)
+        end
+
+        to_add = relationship_key_values - (relationship_key_values & existing)
+        _create_to_many_links(relationship_type, to_add, {})
+
+        @reload_needed = true
+      else
+        send("#{relationship.foreign_key}=", relationship_key_values)
+        @save_needed = true
+      end
 
       :completed
     end
 
-    def _replace_to_one_link(relationship_type, relationship_key_value)
+    def _replace_to_one_link(relationship_type, relationship_key_value, options)
       relationship = self.class._relationships[relationship_type]
 
       send("#{relationship.foreign_key}=", relationship_key_value)
@@ -251,7 +310,7 @@ def _replace_to_one_link(relationship_type, relationship_key_value)
       :completed
     end
 
-    def _replace_polymorphic_to_one_link(relationship_type, key_value, key_type)
+    def _replace_polymorphic_to_one_link(relationship_type, key_value, key_type, options)
       relationship = self.class._relationships[relationship_type.to_sym]
 
       _model.public_send("#{relationship.foreign_key}=", key_value)
@@ -262,10 +321,29 @@ def _replace_polymorphic_to_one_link(relationship_type, key_value, key_type)
       :completed
     end
 
-    def _remove_to_many_link(relationship_type, key)
-      relation_name = self.class._relationships[relationship_type].relation_name(context: @context)
+    def _remove_to_many_link(relationship_type, key, options)
+      relationship = self.class._relationships[relationship_type]
+
+      reflect = reflect_relationship?(relationship, options)
+
+      if reflect
+
+        related_resource = relationship.resource_klass.find_by_key(key, context: @context)
+
+        if related_resource.nil?
+          fail JSONAPI::Exceptions::RecordNotFound.new(key)
+        else
+          if related_resource.class._relationships[relationship.inverse_relationship].is_a?(JSONAPI::Relationship::ToMany)
+            related_resource.remove_to_many_link(relationship.inverse_relationship, id, reflected_source: self)
+          else
+            related_resource.remove_to_one_link(relationship.inverse_relationship, reflected_source: self)
+          end
+        end
 
-      @model.public_send(relation_name).delete(key)
+        @reload_needed = true
+      else
+        @model.public_send(relationship.relation_name(context: @context)).delete(key)
+      end
 
       :completed
 
@@ -275,7 +353,7 @@ def _remove_to_many_link(relationship_type, key)
       fail JSONAPI::Exceptions::RecordNotFound.new(key)
     end
 
-    def _remove_to_one_link(relationship_type)
+    def _remove_to_one_link(relationship_type, options)
       relationship = self.class._relationships[relationship_type]
 
       send("#{relationship.foreign_key}=", nil)
@@ -664,14 +742,21 @@ def find(filters, options = {})
       end
 
       def resources_for(records, context)
-        resources = []
         resource_classes = {}
-        records.each do |model|
+        records.collect do |model|
           resource_class = resource_classes[model.class] ||= self.resource_for_model(model)
-          resources.push resource_class.new(model, context)
+          resource_class.new(model, context)
         end
+      end
 
-        resources
+      def find_by_keys(keys, options = {})
+        context = options[:context]
+        records = records(options)
+        records = apply_includes(records, options)
+        models = records.where({_primary_key => keys})
+        models.collect do |model|
+          self.resource_for_model(model).new(model, context)
+        end
       end
 
       def find_by_key(key, options = {})