From f7c20211cb7db3dd1f42f7b407fec8ed7e0f7c0a Mon Sep 17 00:00:00 2001
From: James Le Cuirot <james.le-cuirot@yakara.com>
Date: Fri, 15 Dec 2017 14:14:48 +0000
Subject: [PATCH 01/12] Remove internal parameters from requests recursively

Nested attributes may be carrying these too. Also speed up the check
by not using a regular expression while we're at it.
---
 lib/her/api.rb | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/lib/her/api.rb b/lib/her/api.rb
index 856663ec..20b03690 100644
--- a/lib/her/api.rb
+++ b/lib/her/api.rb
@@ -89,7 +89,10 @@ def request(opts = {})
       method = opts.delete(:_method)
       path = opts.delete(:_path)
       headers = opts.delete(:_headers)
-      opts.delete_if { |key, _| key.to_s =~ /^_/ } # Remove all internal parameters
+
+      # Recursively remove all internal parameters
+      strip_internal_params opts
+
       if method == :options
         # Faraday doesn't support the OPTIONS verb because of a name collision with an internal options method
         # so we need to call run_request directly.
@@ -117,5 +120,18 @@ def request(opts = {})
     def self.default_api(opts = {})
       defined?(@default_api) ? @default_api : nil
     end
+
+    # @private
+    def strip_internal_params(object)
+      case object
+      when Hash
+        object.delete_if { |key, _| key.to_s[0] == '_' }
+        strip_internal_params object.values
+      when Array
+        object.each do |elem|
+          strip_internal_params elem
+        end
+      end
+    end
   end
 end

From dfa2b8748bb3167c4b6cbee698c3391ac17c1b00 Mon Sep 17 00:00:00 2001
From: James Le Cuirot <james.le-cuirot@yakara.com>
Date: Fri, 13 Feb 2015 11:51:26 +0000
Subject: [PATCH 02/12] Report all missing path parameters, not just the first

This makes it possible to take remedial action such as copying the
parameters from the parent.

The method signature for build_request_path has been simplified. The
path argument was confusing and only used in one place to force the
collection path so a :collection option has been implemented instead.

This new options hash also implements :remove_used, which is useful
for build requests where you want to avoid parameters in the path
being duplicated in the query string.
---
 lib/her/errors.rb              |  6 +--
 lib/her/model/introspection.rb |  3 +-
 lib/her/model/paths.rb         | 69 ++++++++++++++++++++++++----------
 lib/her/model/relation.rb      |  2 +-
 spec/model/paths_spec.rb       |  8 ++--
 5 files changed, 60 insertions(+), 28 deletions(-)

diff --git a/lib/her/errors.rb b/lib/her/errors.rb
index 5ad479f7..42ce4ab5 100644
--- a/lib/her/errors.rb
+++ b/lib/her/errors.rb
@@ -2,11 +2,11 @@ module Her
   module Errors
     class PathError < StandardError
 
-      attr_reader :missing_parameter
+      attr_reader :missing_parameters
 
-      def initialize(message, missing_parameter = nil)
+      def initialize(message, missing_parameters = nil)
         super(message)
-        @missing_parameter = missing_parameter
+        @missing_parameters = missing_parameters
       end
     end
 
diff --git a/lib/her/model/introspection.rb b/lib/her/model/introspection.rb
index 34f15926..0498ab46 100644
--- a/lib/her/model/introspection.rb
+++ b/lib/her/model/introspection.rb
@@ -15,7 +15,8 @@ def inspect
         resource_path = begin
           request_path
         rescue Her::Errors::PathError => e
-          "<unknown path, missing `#{e.missing_parameter}`>"
+          joined = e.missing_parameters.map { |m| "`#{m}`" }.join(", ")
+          "<unknown path, missing #{joined}>"
         end
 
         "#<#{self.class}(#{resource_path}) #{attributes.keys.map { |k| "#{k}=#{attribute_for_inspect(send(k))}" }.join(" ")}>"
diff --git a/lib/her/model/paths.rb b/lib/her/model/paths.rb
index 11aba945..c94c99a5 100644
--- a/lib/her/model/paths.rb
+++ b/lib/her/model/paths.rb
@@ -15,7 +15,7 @@ module Paths
       # @param [Hash] params An optional set of additional parameters for
       #   path construction. These will not override attributes of the resource.
       def request_path(params = {})
-        self.class.build_request_path(params.merge(attributes.dup))
+        self.class.build_request_path(params.merge(attributes))
       end
 
       module ClassMethods
@@ -84,34 +84,65 @@ def resource_path(path = nil)
           end
         end
 
-        # Return a custom path based on the collection path and variable parameters
+        # Return a custom path based on the resource or collection
+        # path and variable parameters.
+        #
+        # If :collection option is true then a collection path is
+        # forced, regardless of whether the primary key is in the
+        # parameters.
+        #
+        # If :remove_used option is true then parameters used in that
+        # path will be removed from the hash.
         #
         # @private
-        def build_request_path(path = nil, parameters = {})
-          parameters = parameters.try(:with_indifferent_access)
+        def build_request_path(parameters = {}, options = {})
+          path =
+            if options[:collection]
+              collection_path.dup
+            else
+              pkey = parameters[primary_key.to_s] || parameters[primary_key.to_sym]
 
-          unless path.is_a?(String)
-            parameters = path.try(:with_indifferent_access) || parameters
-            path =
-              if parameters.include?(primary_key) && parameters[primary_key] && !parameters[primary_key].is_a?(Array)
+              if pkey && !pkey.is_a?(Array)
                 resource_path.dup
               else
                 collection_path.dup
               end
+            end
 
-            # Replace :id with our actual primary key
-            path.gsub!(/(\A|\/):id(\Z|\/)/, "\\1:#{primary_key}\\2")
-          end
+          # Replace :id with our actual primary key
+          path.gsub!(/(\A|\/):id(\z|\/)/, "\\1:#{primary_key}\\2")
 
-          path.gsub(/:([\w_]+)/) do
-            # Look for :key or :_key, otherwise raise an exception
-            key = $1.to_sym
-            value = parameters.delete(key) || parameters.delete(:"_#{key}")
-            if value
-              Faraday::Utils.escape value
-            else
-              raise(Her::Errors::PathError.new("Missing :_#{$1} parameter to build the request path. Path is `#{path}`. Parameters are `#{parameters.symbolize_keys.inspect}`.", $1))
+          used = []
+          missing = []
+
+          result = path.gsub(/:([\w_]+)/) do
+            # Look for "key" or "_key", otherwise add to the missing
+            # list and raise below.
+            replacement = nil
+            [$1, "_#{$1}"].each do |str_key|
+              [str_key, str_key.to_sym].each do |key|
+                value = parameters[key]
+                next unless value
+                used << key if options[:remove_used]
+                replacement = value
+                break 2
+              end
+            end
+
+            unless replacement
+              replacement = $1
+              missing << $1.to_sym
             end
+
+            Faraday::Utils.escape replacement
+          end
+
+          if missing.empty?
+            parameters.except! *used
+            result
+          else
+            joined = missing.map { |m| ":_#{m}" }.join(", ")
+            raise Her::Errors::PathError.new("Missing #{joined} parameters to build the request path. Path is `#{path}`. Parameters are `#{parameters.symbolize_keys.inspect}`.", missing)
           end
         end
 
diff --git a/lib/her/model/relation.rb b/lib/her/model/relation.rb
index df476392..d1e4ff44 100644
--- a/lib/her/model/relation.rb
+++ b/lib/her/model/relation.rb
@@ -67,7 +67,7 @@ def kind_of?(thing)
       # @private
       def fetch
         @_fetch ||= begin
-          path = @parent.build_request_path(@parent.collection_path, @params)
+          path = @parent.build_request_path(@params, collection: true)
           method = @parent.method_for(:find)
           @parent.request(@params.merge(:_method => method, :_path => path)) do |parsed_data, _|
             @parent.new_collection(parsed_data)
diff --git a/spec/model/paths_spec.rb b/spec/model/paths_spec.rb
index 2ca5ddda..40d9b94f 100644
--- a/spec/model/paths_spec.rb
+++ b/spec/model/paths_spec.rb
@@ -61,8 +61,8 @@
         end
 
         it "raises exceptions when building a path without required custom variables" do
-          Foo::User.collection_path "/organizations/:organization_id/utilisateurs"
-          expect { Foo::User.new(id: "foo").request_path }.to raise_error(Her::Errors::PathError, "Missing :_organization_id parameter to build the request path. Path is `/organizations/:organization_id/utilisateurs/:id`. Parameters are `{:id=>\"foo\"}`.")
+          Foo::User.collection_path "/organizations/:organization_id/departments/:department_id/utilisateurs"
+          expect { Foo::User.new(id: "foo").request_path }.to raise_error(Her::Errors::PathError, "Missing :_organization_id, :_department_id parameters to build the request path. Path is `/organizations/:organization_id/departments/:department_id/utilisateurs/:id`. Parameters are `{:id=>\"foo\"}`.")
         end
 
         it "escapes the variable values" do
@@ -122,12 +122,12 @@
 
         it "raises exceptions when building a path without required custom variables" do
           Foo::AdminUser.collection_path "/organizations/:organization_id/users"
-          expect { Foo::AdminUser.new(id: "foo").request_path }.to raise_error(Her::Errors::PathError, "Missing :_organization_id parameter to build the request path. Path is `/organizations/:organization_id/users/:id`. Parameters are `{:id=>\"foo\"}`.")
+          expect { Foo::AdminUser.new(id: "foo").request_path }.to raise_error(Her::Errors::PathError, "Missing :_organization_id parameters to build the request path. Path is `/organizations/:organization_id/users/:id`. Parameters are `{:id=>\"foo\"}`.")
         end
 
         it "raises exceptions when building a relative path without required custom variables" do
           Foo::AdminUser.collection_path "organizations/:organization_id/users"
-          expect { Foo::AdminUser.new(id: "foo").request_path }.to raise_error(Her::Errors::PathError, "Missing :_organization_id parameter to build the request path. Path is `organizations/:organization_id/users/:id`. Parameters are `{:id=>\"foo\"}`.")
+          expect { Foo::AdminUser.new(id: "foo").request_path }.to raise_error(Her::Errors::PathError, "Missing :_organization_id parameters to build the request path. Path is `organizations/:organization_id/users/:id`. Parameters are `{:id=>\"foo\"}`.")
         end
       end
     end

From 470ef1ab259e2e408de388a70445cb3dc4e83862 Mon Sep 17 00:00:00 2001
From: James Le Cuirot <james.le-cuirot@yakara.com>
Date: Fri, 13 Feb 2015 12:32:43 +0000
Subject: [PATCH 03/12] Use nested path parameters from the parent when using
 build

---
 lib/her/model/associations/association.rb     | 35 +++++++++++++++++++
 .../associations/has_many_association.rb      |  4 +--
 .../model/associations/has_one_association.rb |  2 +-
 spec/model/associations_spec.rb               | 29 +++++++++++++--
 4 files changed, 64 insertions(+), 6 deletions(-)

diff --git a/lib/her/model/associations/association.rb b/lib/her/model/associations/association.rb
index 3548dc80..a2ecade0 100644
--- a/lib/her/model/associations/association.rb
+++ b/lib/her/model/associations/association.rb
@@ -9,6 +9,7 @@ class Association
         # @private
         def initialize(parent, opts = {})
           @parent = parent
+          @parent_name = @parent.singularized_resource_name.to_sym
           @opts = opts
           @params = {}
 
@@ -39,6 +40,14 @@ def assign_single_nested_attributes(attributes)
           end
         end
 
+        # @private
+        def set_missing_from_parent(missing, attributes)
+          missing.each do |m|
+            id = @parent.get_attribute(m) || @parent.get_attribute(:"_#{m}")
+            attributes[:"_#{m}"] = id if id
+          end
+        end
+
         # @private
         def fetch(opts = {})
           attribute_value = @parent.attributes[@name]
@@ -61,6 +70,32 @@ def build_association_path(code)
           nil
         end
 
+        # @private
+        def build_with_inverse(attributes = {})
+          retried = false
+
+          begin
+            resource = @klass.build(attributes.merge(:"#{@parent.singularized_resource_name}_id" => @parent.id))
+            resource.request_path
+            resource
+          rescue Her::Errors::PathError => e
+            if resource
+              attributes = resource.attributes
+            elsif retried
+              raise
+            end
+
+            set_missing_from_parent e.missing_parameters, attributes
+
+            if resource
+              resource
+            else
+              retried = true
+              retry
+            end
+          end
+        end
+
         # @private
         def reset
           @params = {}
diff --git a/lib/her/model/associations/has_many_association.rb b/lib/her/model/associations/has_many_association.rb
index 23ed797a..be49201a 100644
--- a/lib/her/model/associations/has_many_association.rb
+++ b/lib/her/model/associations/has_many_association.rb
@@ -49,10 +49,8 @@ def self.parse(association, klass, data)
         #   user = User.find(1)
         #   new_comment = user.comments.build(:body => "Hello!")
         #   new_comment # => #<Comment user_id=1 body="Hello!">
-        # TODO: This only merges the id of the parents, handle the case
-        #       where this is more deeply nested
         def build(attributes = {})
-          @klass.build(attributes.merge(:"#{@parent.singularized_resource_name}_id" => @parent.id))
+          build_with_inverse(attributes)
         end
 
         # Create a new object, save it and add it to the associated collection
diff --git a/lib/her/model/associations/has_one_association.rb b/lib/her/model/associations/has_one_association.rb
index 80cb3cd3..b3f8cb46 100644
--- a/lib/her/model/associations/has_one_association.rb
+++ b/lib/her/model/associations/has_one_association.rb
@@ -45,7 +45,7 @@ def self.parse(*args)
         #   new_role = user.role.build(:title => "moderator")
         #   new_role # => #<Role user_id=1 title="moderator">
         def build(attributes = {})
-          @klass.build(attributes.merge(:"#{@parent.singularized_resource_name}_id" => @parent.id))
+          build_with_inverse(attributes)
         end
 
         # Create a new object, save it and associate it to the parent
diff --git a/spec/model/associations_spec.rb b/spec/model/associations_spec.rb
index a6632fa4..e3f6b0ec 100644
--- a/spec/model/associations_spec.rb
+++ b/spec/model/associations_spec.rb
@@ -764,7 +764,21 @@ def present?
 
   context "building and creating association data" do
     before do
-      spawn_model "Foo::Comment"
+      Her::API.setup url: "https://api.example.com" do |builder|
+        builder.use Her::Middleware::FirstLevelParseJSON
+        builder.use Faraday::Request::UrlEncoded
+        builder.adapter :test do |stub|
+          stub.get("/users/10/comments/new?body=Hello") { [200, {}, { id: nil, body: "Hello" }.to_json] }
+        end
+      end
+
+      spawn_model "Foo::Like" do
+        collection_path "/users/:user_id/comments/:comment_id/likes"
+      end
+      spawn_model "Foo::Comment" do
+        collection_path "/users/:user_id/comments"
+        has_many :likes
+      end
       spawn_model "Foo::User" do
         has_many :comments
       end
@@ -777,6 +791,17 @@ def present?
         expect(comment.body).to eq("Hello!")
         expect(comment.user_id).to eq(10)
       end
+
+      it "uses nested path parameters from the parent when new object isn't requested" do
+        @like = Foo::User.new(:id => 10).comments.build(:id => 20).likes.build
+        expect(@like.request_path).to eq("/users/10/comments/20/likes")
+      end
+
+      it "uses nested path parameters from the parent when new object is requested" do
+        Foo::Comment.request_new_object_on_build true
+        @comment = Foo::User.new(:id => 10).comments.build(:body => "Hello")
+        expect(@comment.request_path).to eq("/users/10/comments")
+      end
     end
 
     context "with #create" do
@@ -789,7 +814,7 @@ def present?
           builder.use Faraday::Request::UrlEncoded
           builder.adapter :test do |stub|
             stub.get("/users/10") { [200, {}, { id: 10 }.to_json] }
-            stub.post("/comments") { |env| [200, {}, { id: 1, body: Faraday::Utils.parse_query(env[:body])["body"], user_id: Faraday::Utils.parse_query(env[:body])["user_id"].to_i }.to_json] }
+            stub.post("/users/10/comments") { |env| [200, {}, { id: 1, body: Faraday::Utils.parse_query(env[:body])["body"], user_id: Faraday::Utils.parse_query(env[:body])["user_id"].to_i }.to_json] }
           end
         end
 

From 8f48aefe7418ee345640a7e7cd68ab6190177179 Mon Sep 17 00:00:00 2001
From: James Le Cuirot <james.le-cuirot@yakara.com>
Date: Fri, 10 Nov 2017 12:05:34 +0000
Subject: [PATCH 04/12] Allow resources to be assigned directly to belongs_to
 associations

Just like ActiveRecord, this will modify the foreign key value and
cache the resource, avoiding a needless lookup later.

The use_setter_methods logic is a little convoluted and I tried to
improve it but this quickly turned into a can of worms.
---
 .../associations/belongs_to_association.rb    | 25 +++++++++++++++++++
 lib/her/model/attributes.rb                   |  7 +++---
 spec/model/associations_spec.rb               | 14 +++++++++++
 3 files changed, 43 insertions(+), 3 deletions(-)

diff --git a/lib/her/model/associations/belongs_to_association.rb b/lib/her/model/associations/belongs_to_association.rb
index 6b9accfc..0984e6a4 100644
--- a/lib/her/model/associations/belongs_to_association.rb
+++ b/lib/her/model/associations/belongs_to_association.rb
@@ -22,6 +22,10 @@ def #{name}
               cached_data = (instance_variable_defined?(cached_name) && instance_variable_get(cached_name))
               cached_data || instance_variable_set(cached_name, Her::Model::Associations::BelongsToAssociation.proxy(self, #{opts.inspect}))
             end
+
+            def #{name}=(resource)
+              send("#{name}").association.assign(resource)
+            end
           RUBY
         end
 
@@ -86,6 +90,27 @@ def fetch
           end
         end
 
+        # @private
+        def assign(resource)
+          reset
+          pkey = resource ? resource.id : nil
+          @parent.send("#{@opts[:foreign_key]}=", pkey)
+          @cached_result = resource
+
+          if resource
+            begin
+              @parent.request_path
+            rescue Her::Errors::PathError => e
+              e.missing_parameters.each do |m|
+                id = resource.get_attribute(m) || resource.get_attribute("_#{m}")
+                @parent.send("_#{m}=", id) if id
+              end
+            end
+          end
+
+          resource
+        end
+
         # @private
         def assign_nested_attributes(attributes)
           assign_single_nested_attributes(attributes)
diff --git a/lib/her/model/attributes.rb b/lib/her/model/attributes.rb
index 1737c488..b65ad8bb 100644
--- a/lib/her/model/attributes.rb
+++ b/lib/her/model/attributes.rb
@@ -198,13 +198,14 @@ def new_from_parsed_data(parsed_data)
         #
         # @private
         def use_setter_methods(model, params = {})
-          reserved = [:id, model.class.primary_key, *model.class.association_keys]
-          model.class.attributes *params.keys.reject { |k| reserved.include?(k) }
+          assoc_keys = model.class.association_keys
+          reserved = [:id, model.class.primary_key, *assoc_keys]
+          model.class.attributes *(params.keys - reserved)
 
           setter_method_names = model.class.setter_method_names
           params.each_with_object({}) do |(key, value), memo|
             setter_method = "#{key}="
-            if setter_method_names.include?(setter_method)
+            if setter_method_names.include?(setter_method) && (!assoc_keys.include?(key) || !value.is_a?(Hash))
               model.send setter_method, value
             else
               memo[key.to_sym] = value
diff --git a/spec/model/associations_spec.rb b/spec/model/associations_spec.rb
index e3f6b0ec..66b9dc39 100644
--- a/spec/model/associations_spec.rb
+++ b/spec/model/associations_spec.rb
@@ -371,6 +371,7 @@
             stub.get("/users/2/comments") { [200, {}, [{ comment: { id: 4, body: "They're having a FIRESALE?" } }, { comment: { id: 5, body: "Is this the tiny town from Footloose?" } }].to_json] }
             stub.get("/users/2/comments/5") { [200, {}, { comment: { id: 5, body: "Is this the tiny town from Footloose?" } }.to_json] }
             stub.get("/users/2/role") { [200, {}, { id: 2, body: "User" }.to_json] }
+            stub.get("/organizations/1") { [200, {}, { organization: { id: 1, name: "Bluth Company Foo" } }.to_json] }
             stub.get("/organizations/2") do |env|
               if env[:params]["admin"] == "true"
                 [200, {}, { organization: { id: 2, name: "Bluth Company (admin)" } }.to_json]
@@ -440,6 +441,19 @@
         expect(user.organization.respond_to?(:empty?)).to be_truthy
         expect(user.organization).not_to be_empty
       end
+
+      it "changes the belongs_to foreign key value when a new resource is assigned" do
+        org1 = Foo::Organization.find(1)
+        user.organization = org1
+        expect(user.organization).to eq(org1)
+        expect(user.changes).to eq('organization_id' => [2, 1])
+      end
+
+      it "nullifies the belongs_to foreign key value when a nil resource is assigned" do
+        user.organization = nil
+        expect(user.organization).to be_nil
+        expect(user.changes).to eq('organization_id' => [2, nil])
+      end
     end
 
     context "without included parent data" do

From c42e253e13bf61504282ec155049b64239826211 Mon Sep 17 00:00:00 2001
From: James Le Cuirot <james.le-cuirot@yakara.com>
Date: Mon, 13 Nov 2017 10:43:46 +0000
Subject: [PATCH 05/12] Use belongs_to assignment method to build resources
 more intelligently

When building against a has_one or has_many association, first check
for a specified inverse association, then check for a belongs_to
matching the resource name, and finally fall back to foo_id as
before. The first two approaches allow the resource to be cached.
---
 lib/her/model/associations/association.rb | 38 ++++++++++++++++++++---
 lib/her/model/orm.rb                      | 27 +++++++++++++---
 spec/model/associations_spec.rb           | 13 ++++++++
 3 files changed, 68 insertions(+), 10 deletions(-)

diff --git a/lib/her/model/associations/association.rb b/lib/her/model/associations/association.rb
index a2ecade0..c666d132 100644
--- a/lib/her/model/associations/association.rb
+++ b/lib/her/model/associations/association.rb
@@ -41,10 +41,29 @@ def assign_single_nested_attributes(attributes)
         end
 
         # @private
-        def set_missing_from_parent(missing, attributes)
+        def inverse
+          if @opts[:inverse_of]
+            iname = @opts[:inverse_of].to_sym
+            inverse = @klass.associations[:belongs_to].find { |assoc| assoc[:name].to_sym == iname }
+            raise Her::Errors::AssociationUnknownError, "Unknown association name :#{iname}" unless inverse
+          else
+            inverse = @klass.associations[:belongs_to].find { |assoc| assoc[:name].to_sym == @parent_name }
+          end
+
+          inverse
+        end
+
+        # @private
+        def set_missing_from_parent(missing, attributes, inverse = self.inverse)
           missing.each do |m|
-            id = @parent.get_attribute(m) || @parent.get_attribute(:"_#{m}")
-            attributes[:"_#{m}"] = id if id
+            if inverse && inverse[:foreign_key].to_sym == m
+              attributes[m] = @parent.id
+            elsif m == :"#{@parent_name}_id"
+              attributes[:"_#{m}"] = @parent.id
+            else
+              id = @parent.get_attribute(m) || @parent.get_attribute(:"_#{m}")
+              attributes[:"_#{m}"] = id if id
+            end
           end
         end
 
@@ -72,10 +91,19 @@ def build_association_path(code)
 
         # @private
         def build_with_inverse(attributes = {})
+          inverse = self.inverse
+
+          attributes =
+            if inverse
+              attributes.merge(inverse[:name] => @parent)
+            else
+              attributes.merge(:"#{@parent_name}_id" => @parent.id)
+            end
+
           retried = false
 
           begin
-            resource = @klass.build(attributes.merge(:"#{@parent.singularized_resource_name}_id" => @parent.id))
+            resource = @klass.build(attributes)
             resource.request_path
             resource
           rescue Her::Errors::PathError => e
@@ -85,7 +113,7 @@ def build_with_inverse(attributes = {})
               raise
             end
 
-            set_missing_from_parent e.missing_parameters, attributes
+            set_missing_from_parent e.missing_parameters, attributes, inverse
 
             if resource
               resource
diff --git a/lib/her/model/orm.rb b/lib/her/model/orm.rb
index 6b89f282..eb01f0c7 100644
--- a/lib/her/model/orm.rb
+++ b/lib/her/model/orm.rb
@@ -256,16 +256,33 @@ def method_for(action = nil, method = nil)
         # Build a new resource with the given attributes.
         # If the request_new_object_on_build flag is set, the new object is requested via API.
         def build(attributes = {})
-          params = attributes
-          return new(params) unless request_new_object_on_build?
+          return new(attributes) unless request_new_object_on_build?
+          params = attributes.merge(primary_key => 'new')
 
-          path = build_request_path(params.merge(primary_key => 'new'))
-          method = method_for(:new)
+          assoc_names = []
+          associations[:belongs_to].each do |assoc|
+            # Remove associated data when the foreign key is present
+            # as that implies the resource already exists.
+            assoc_names << assoc[:name] if (params.key?(assoc[:foreign_key].to_sym) || params.key?(:"_#{assoc[:foreign_key]}")) && params.delete(assoc[:name])
+
+            # TODO: Apply to_params to new associations and reassign
+            # them using the data_key.
+          end
+
+          params[:_method] = method_for(:new)
+          params[:_path] = build_request_path(params, remove_used: true)
 
           resource = nil
-          request(params.merge(:_method => method, :_path => path)) do |parsed_data, response|
+          request(params) do |parsed_data, response|
             if response.success?
               resource = new_from_parsed_data(parsed_data)
+
+              # Assign associated resources that we already have
+              # cached locally to prevent them being refetched.
+              assoc_names.each do |name|
+                value = attributes[name]
+                resource.send(name).association.assign(value)
+              end
             end
           end
           resource
diff --git a/spec/model/associations_spec.rb b/spec/model/associations_spec.rb
index 66b9dc39..acccda97 100644
--- a/spec/model/associations_spec.rb
+++ b/spec/model/associations_spec.rb
@@ -806,6 +806,19 @@ def present?
         expect(comment.user_id).to eq(10)
       end
 
+      it "caches the parent record when the inverse if present" do
+        Foo::Comment.belongs_to :user
+        @user = Foo::User.new(id: 10)
+        @comment = @user.comments.build
+        expect(@comment.user.object_id).to eq(@user.object_id)
+      end
+
+      it "does not cache the parent resource when inverse is missing" do
+        @user = Foo::User.new(id: 10)
+        @comment = @user.comments.build
+        expect(@comment.respond_to?(:user)).to be_falsey
+      end
+
       it "uses nested path parameters from the parent when new object isn't requested" do
         @like = Foo::User.new(:id => 10).comments.build(:id => 20).likes.build
         expect(@like.request_path).to eq("/users/10/comments/20/likes")

From 50e839fd33698e0b8fca5d337d48e9e10e873db9 Mon Sep 17 00:00:00 2001
From: James Le Cuirot <james.le-cuirot@yakara.com>
Date: Fri, 17 Nov 2017 15:25:15 +0000
Subject: [PATCH 06/12] Refactor the
 Her::Model::Associations::Association#fetch method

It's very confusing and needs restructuring for the next commit.
---
 lib/her/model/associations/association.rb | 25 +++++++++++++++--------
 1 file changed, 16 insertions(+), 9 deletions(-)

diff --git a/lib/her/model/associations/association.rb b/lib/her/model/associations/association.rb
index c666d132..79abe777 100644
--- a/lib/her/model/associations/association.rb
+++ b/lib/her/model/associations/association.rb
@@ -69,17 +69,24 @@ def set_missing_from_parent(missing, attributes, inverse = self.inverse)
 
         # @private
         def fetch(opts = {})
-          attribute_value = @parent.attributes[@name]
-          return @opts[:default].try(:dup) if @parent.attributes.include?(@name) && (attribute_value.nil? || !attribute_value.nil? && attribute_value.empty?) && @params.empty?
+          if @params.blank?
+            result =
+              if @parent.attributes.key?(@name) && @parent.attributes[@name].blank?
+                @opts[:default].try(:dup)
+              else
+                @cached_result || @parent.attributes[@name]
+              end
+          end
 
-          return @cached_result unless @params.any? || @cached_result.nil?
-          return @parent.attributes[@name] unless @params.any? || @parent.attributes[@name].blank?
-          return @opts[:default].try(:dup) if @parent.new?
+          result ||=
+            if @parent.new?
+              @opts[:default].try(:dup)
+            else
+              path = build_association_path -> { "#{@parent.request_path(@params)}#{@opts[:path]}" }
+              @klass.get(path, @params).tap { |r| @cached_result = r if @params.blank? }
+            end
 
-          path = build_association_path -> { "#{@parent.request_path(@params)}#{@opts[:path]}" }
-          @klass.get(path, @params).tap do |result|
-            @cached_result = result unless @params.any?
-          end
+          result
         end
 
         # @private

From 9e6beacbc270cf39371927782a3f84ea4a014b2f Mon Sep 17 00:00:00 2001
From: James Le Cuirot <james.le-cuirot@yakara.com>
Date: Fri, 17 Nov 2017 17:11:12 +0000
Subject: [PATCH 07/12] Use parent nested path parameters when fetching
 has_many and has_one

Also cache the parent resource for has_one as this was previously only
being done for has_many.
---
 lib/her/model/associations/association.rb     | 19 +++++++++-
 .../associations/has_many_association.rb      |  8 ----
 spec/model/associations_spec.rb               | 37 ++++++++++++++++++-
 3 files changed, 53 insertions(+), 11 deletions(-)

diff --git a/lib/her/model/associations/association.rb b/lib/her/model/associations/association.rb
index 79abe777..eb55d195 100644
--- a/lib/her/model/associations/association.rb
+++ b/lib/her/model/associations/association.rb
@@ -67,6 +67,20 @@ def set_missing_from_parent(missing, attributes, inverse = self.inverse)
           end
         end
 
+        # @private
+        def set_missing_and_inverse_from_parent(resources, inverse = self.inverse)
+          Array(resources).each do |resource|
+            begin
+              resource.request_path
+            rescue Her::Errors::PathError => e
+              set_missing_from_parent e.missing_parameters, resource.attributes, inverse
+            end
+
+            iname = inverse ? inverse[:name] : @parent_name
+            resource.send("#{iname}=", @parent)
+          end
+        end
+
         # @private
         def fetch(opts = {})
           if @params.blank?
@@ -86,6 +100,7 @@ def fetch(opts = {})
               @klass.get(path, @params).tap { |r| @cached_result = r if @params.blank? }
             end
 
+          set_missing_and_inverse_from_parent result if result
           result
         end
 
@@ -167,7 +182,9 @@ def where(params = {})
         def find(id)
           return nil if id.blank?
           path = build_association_path -> { "#{@parent.request_path(@params)}#{@opts[:path]}/#{id}" }
-          @klass.get_resource(path, @params)
+          @klass.get_resource(path, @params).tap do |result|
+            set_missing_and_inverse_from_parent(result) if result
+          end
         end
 
         # Refetches the association and puts the proxy back in its initial state,
diff --git a/lib/her/model/associations/has_many_association.rb b/lib/her/model/associations/has_many_association.rb
index be49201a..f8815396 100644
--- a/lib/her/model/associations/has_many_association.rb
+++ b/lib/her/model/associations/has_many_association.rb
@@ -79,14 +79,6 @@ def create(attributes = {})
           resource
         end
 
-        # @private
-        def fetch
-          super.tap do |o|
-            inverse_of = @opts[:inverse_of] || @parent.singularized_resource_name
-            o.each { |entry| entry.attributes[inverse_of] = @parent }
-          end
-        end
-
         # @private
         def assign_nested_attributes(attributes)
           data = attributes.is_a?(Hash) ? attributes.values : attributes
diff --git a/spec/model/associations_spec.rb b/spec/model/associations_spec.rb
index acccda97..1c17b4b3 100644
--- a/spec/model/associations_spec.rb
+++ b/spec/model/associations_spec.rb
@@ -272,10 +272,21 @@
       end
 
       spawn_model "Foo::Comment" do
+        collection_path "/users/:user_id/comments"
         belongs_to :user
+        has_many :likes
+        has_one :deletion
         parse_root_in_json true
       end
 
+      spawn_model "Foo::Like" do
+        collection_path "/users/:user_id/comments/:comment_id/likes"
+      end
+
+      spawn_model "Foo::Deletion" do
+        collection_path "/users/:user_id/comments/:comment_id/deletion"
+      end
+
       spawn_model "Foo::Post" do
         belongs_to :admin, class_name: "Foo::User"
       end
@@ -284,7 +295,9 @@
         parse_root_in_json true
       end
 
-      spawn_model "Foo::Role"
+      spawn_model "Foo::Role" do
+        belongs_to :user
+      end
     end
 
     context "with included data" do
@@ -312,10 +325,14 @@
         expect(user.comments.first.body).to eq("Tobias, you blow hard!")
       end
 
-      it "does not refetch the parents models data if they have been fetched before" do
+      it "does not refetch the parents models data if they have been fetched before for a has_many" do
         expect(user.comments.first.user.object_id).to eq(user.object_id)
       end
 
+      it "does not refetch the parents models data if they have been fetched before for a has_one" do
+        expect(user.role.user.object_id).to eq(user.object_id)
+      end
+
       it "uses the given inverse_of key to set the parent model" do
         expect(user.posts.first.admin.object_id).to eq(user.object_id)
       end
@@ -370,6 +387,8 @@
             stub.get("/users/2") { [200, {}, { id: 2, name: "Lindsay Fünke", organization_id: 2 }.to_json] }
             stub.get("/users/2/comments") { [200, {}, [{ comment: { id: 4, body: "They're having a FIRESALE?" } }, { comment: { id: 5, body: "Is this the tiny town from Footloose?" } }].to_json] }
             stub.get("/users/2/comments/5") { [200, {}, { comment: { id: 5, body: "Is this the tiny town from Footloose?" } }.to_json] }
+            stub.get("/users/2/comments/5/likes") { [200, {}, [{ like: { id: 1, liker_id: 1 } }].to_json] }
+            stub.get("/users/2/comments/5/deletion") { [200, {}, { deletion: { id: 1, deleter_id: 1 } }.to_json] }
             stub.get("/users/2/role") { [200, {}, { id: 2, body: "User" }.to_json] }
             stub.get("/organizations/1") { [200, {}, { organization: { id: 1, name: "Bluth Company Foo" } }.to_json] }
             stub.get("/organizations/2") do |env|
@@ -385,6 +404,10 @@
 
       let(:user) { Foo::User.find(2) }
 
+      it "does not refetch the parents models data if they have been fetched before for a has_many member" do
+        expect(user.comments.find(5).user.object_id).to eq(user.object_id)
+      end
+
       it "fetches data that was not included through has_many" do
         expect(user.comments.first).to be_a(Foo::Comment)
         expect(user.comments.length).to eq(2)
@@ -437,6 +460,16 @@
         expect(comment.id).to eq(5)
       end
 
+      it "uses nested path parameters from the parent when fetching a has_many" do
+        like = user.comments.find(5).likes.first
+        expect(like.request_path).to eq('/users/2/comments/5/likes')
+      end
+
+      it "uses nested path parameters from the parent when fetching a has_one" do
+        deletion = user.comments.find(5).deletion
+        expect(deletion.request_path).to eq('/users/2/comments/5/deletion')
+      end
+
       it "'s associations responds to #empty?" do
         expect(user.organization.respond_to?(:empty?)).to be_truthy
         expect(user.organization).not_to be_empty

From 612c85ffe8a14875ddc201cb0ffd9f6de8790e7e Mon Sep 17 00:00:00 2001
From: James Le Cuirot <james.le-cuirot@yakara.com>
Date: Mon, 20 Nov 2017 18:02:43 +0000
Subject: [PATCH 08/12] Avoid infinite loops when calling inspect

Following my earlier changes, some slightly buggy code in one of my
applications triggered this. The problem went away once the code was
corrected but it is obviously worth avoiding this to start with.
---
 lib/her/model/introspection.rb | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/lib/her/model/introspection.rb b/lib/her/model/introspection.rb
index 0498ab46..78832b81 100644
--- a/lib/her/model/introspection.rb
+++ b/lib/her/model/introspection.rb
@@ -12,6 +12,9 @@ module Introspection
       #   @user = User.find(1)
       #   p @user # => #<User(/users/1) id=1 name="Tobias Fünke">
       def inspect
+        first = Thread.current[:her_inspect_objects].nil?
+        Thread.current[:her_inspect_objects] = [] if first
+
         resource_path = begin
           request_path
         rescue Her::Errors::PathError => e
@@ -19,7 +22,16 @@ def inspect
           "<unknown path, missing #{joined}>"
         end
 
-        "#<#{self.class}(#{resource_path}) #{attributes.keys.map { |k| "#{k}=#{attribute_for_inspect(send(k))}" }.join(" ")}>"
+        result = "#<#{self.class}(#{resource_path}) "
+
+        if Thread.current[:her_inspect_objects].include?(self)
+          result << '...>'
+        else
+          Thread.current[:her_inspect_objects] << self
+          result << attributes.keys.map { |k| "#{k}=#{attribute_for_inspect(send(k))}" }.join(' ') + '>'
+        end
+      ensure
+        Thread.current[:her_inspect_objects] = nil if first
       end
 
       private

From e105be2673ee52300b0684ecf8a2ed34e27387de Mon Sep 17 00:00:00 2001
From: James Le Cuirot <james.le-cuirot@yakara.com>
Date: Wed, 20 Dec 2017 12:18:16 +0000
Subject: [PATCH 09/12] Control whether associations are nested in parent saves
 using autosave

This new association option is similar to the option of the same name
found in ActiveRecord. When true, associated resources are always
nested in a parent save. When false, they are never nested. When nil,
only new resources are nested. Unlike ActiveRecord, the default is
true (rather than nil) in order to preserve existing behaviour.

This is also subject to whether send_only_modified_attributes is true
or not. When true, only modified resources will be sent, even if
autosave is true. Unlike before, it won't do strange things like fetch
an existing has_one resource when you assign a new one because of
change tracking.

4 of the specs currently fail because they require the fixes in my
fix-change-tracking branch.
---
 lib/her/model/associations/association.rb     |   8 +
 .../associations/belongs_to_association.rb    |   4 +-
 .../associations/has_many_association.rb      |  12 +-
 .../model/associations/has_one_association.rb |  10 +-
 lib/her/model/attributes.rb                   |   2 +-
 lib/her/model/parse.rb                        |  48 +++--
 spec/model/associations_spec.rb               | 172 ++++++++++++++++--
 7 files changed, 225 insertions(+), 31 deletions(-)

diff --git a/lib/her/model/associations/association.rb b/lib/her/model/associations/association.rb
index eb55d195..74b41f2e 100644
--- a/lib/her/model/associations/association.rb
+++ b/lib/her/model/associations/association.rb
@@ -153,6 +153,14 @@ def reset
           @parent.attributes.delete(@name)
         end
 
+        # @private
+        def assign(resources)
+          reset
+          set_missing_and_inverse_from_parent(resources)
+          @parent.attributes[@name] = resources
+          @cached_result = resources
+        end
+
         # Add query parameters to the HTTP request performed to fetch the data
         #
         # @example
diff --git a/lib/her/model/associations/belongs_to_association.rb b/lib/her/model/associations/belongs_to_association.rb
index 0984e6a4..a0eb7831 100644
--- a/lib/her/model/associations/belongs_to_association.rb
+++ b/lib/her/model/associations/belongs_to_association.rb
@@ -11,7 +11,8 @@ def self.attach(klass, name, opts)
             :data_key => name,
             :default => nil,
             :foreign_key => "#{name}_id",
-            :path => "/#{name.to_s.pluralize}/:id"
+            :path => "/#{name.to_s.pluralize}/:id",
+            :autosave => true
           }.merge(opts)
           klass.associations[:belongs_to] << opts
 
@@ -95,6 +96,7 @@ def assign(resource)
           reset
           pkey = resource ? resource.id : nil
           @parent.send("#{@opts[:foreign_key]}=", pkey)
+          @parent.attributes[@name] = resource
           @cached_result = resource
 
           if resource
diff --git a/lib/her/model/associations/has_many_association.rb b/lib/her/model/associations/has_many_association.rb
index f8815396..05a83ceb 100644
--- a/lib/her/model/associations/has_many_association.rb
+++ b/lib/her/model/associations/has_many_association.rb
@@ -11,7 +11,8 @@ def self.attach(klass, name, opts)
             :data_key       => name,
             :default        => Her::Collection.new,
             :path           => "/#{name}",
-            :inverse_of => nil
+            :inverse_of => nil,
+            :autosave => true
           }.merge(opts)
           klass.associations[:has_many] << opts
 
@@ -22,6 +23,10 @@ def #{name}
               cached_data = (instance_variable_defined?(cached_name) && instance_variable_get(cached_name))
               cached_data || instance_variable_set(cached_name, Her::Model::Associations::HasManyAssociation.proxy(self, #{opts.inspect}))
             end
+
+            def #{name}=(resources)
+              send("#{name}").association.assign(resources)
+            end
           RUBY
         end
 
@@ -70,10 +75,11 @@ def build(attributes = {})
         #   user.comments # => [#<Comment id=2 user_id=1 body="Hello!">]
         def create(attributes = {})
           resource = build(attributes)
+          set_missing_and_inverse_from_parent(resource)
 
           if resource.save
-            @parent.attributes[@name] ||= Her::Collection.new
-            @parent.attributes[@name] << resource
+            collection = @parent.attributes[@name] || assign(Her::Collection.new)
+            collection << resource
           end
 
           resource
diff --git a/lib/her/model/associations/has_one_association.rb b/lib/her/model/associations/has_one_association.rb
index b3f8cb46..a4623f45 100644
--- a/lib/her/model/associations/has_one_association.rb
+++ b/lib/her/model/associations/has_one_association.rb
@@ -10,7 +10,8 @@ def self.attach(klass, name, opts)
             :name => name,
             :data_key => name,
             :default => nil,
-            :path => "/#{name}"
+            :path => "/#{name}",
+            :autosave => true
           }.merge(opts)
           klass.associations[:has_one] << opts
 
@@ -21,6 +22,10 @@ def #{name}
               cached_data = (instance_variable_defined?(cached_name) && instance_variable_get(cached_name))
               cached_data || instance_variable_set(cached_name, Her::Model::Associations::HasOneAssociation.proxy(self, #{opts.inspect}))
             end
+
+            def #{name}=(resource)
+              send("#{name}").association.assign(resource)
+            end
           RUBY
         end
 
@@ -65,7 +70,8 @@ def build(attributes = {})
         #   user.role # => #<Role id=2 user_id=1 title="moderator">
         def create(attributes = {})
           resource = build(attributes)
-          @parent.attributes[@name] = resource if resource.save
+          set_missing_and_inverse_from_parent(resource)
+          assign(resource) if resource.save
           resource
         end
 
diff --git a/lib/her/model/attributes.rb b/lib/her/model/attributes.rb
index b65ad8bb..c64867a1 100644
--- a/lib/her/model/attributes.rb
+++ b/lib/her/model/attributes.rb
@@ -205,7 +205,7 @@ def use_setter_methods(model, params = {})
           setter_method_names = model.class.setter_method_names
           params.each_with_object({}) do |(key, value), memo|
             setter_method = "#{key}="
-            if setter_method_names.include?(setter_method) && (!assoc_keys.include?(key) || !value.is_a?(Hash))
+            if setter_method_names.include?(setter_method) && (!assoc_keys.include?(key) || (!value.is_a?(Hash) && Array(value).any? { |v| !v.is_a?(Hash) }))
               model.send setter_method, value
             else
               memo[key.to_sym] = value
diff --git a/lib/her/model/parse.rb b/lib/her/model/parse.rb
index 51b4a31a..fbbbd281 100644
--- a/lib/her/model/parse.rb
+++ b/lib/her/model/parse.rb
@@ -42,12 +42,12 @@ def to_params(attributes, changes = {})
             memo[key.to_sym] = value
           end
 
-          filtered_attributes.merge!(embeded_params(attributes))
-
           if her_api.options[:send_only_modified_attributes]
             filtered_attributes.slice! *changes.keys.map(&:to_sym)
           end
 
+          embed_params!(attributes, filtered_attributes)
+
           if include_root_in_json?
             if json_api_format?
               { included_root_element => [filtered_attributes] }
@@ -60,16 +60,42 @@ def to_params(attributes, changes = {})
         end
 
         # @private
-        def embeded_params(attributes)
-          associations.values.flatten.each_with_object({}) do |definition, hash|
-            value = case association = attributes[definition[:name]]
-                    when Her::Collection, Array
-                      association.map { |a| a.to_params }.reject(&:empty?)
-                    when Her::Model
-                      association.to_params
-                    end
-            hash[definition[:data_key]] = value if value.present?
+        def embed_params!(read_attributes, write_attributes)
+          first = Thread.current[:her_embedded_params_objects].nil?
+          Thread.current[:her_embedded_params_objects] = [] if first
+
+          return {} if Thread.current[:her_embedded_params_objects].include?(self)
+          Thread.current[:her_embedded_params_objects] << self
+
+          associations.values.flatten.each do |assoc|
+            write_attributes.delete(assoc[:name])
+            next if assoc[:autosave] == false
+            value = read_attributes[assoc[:name]]
+
+            value =
+              if assoc[:autosave].nil?
+                case value
+                when Her::Collection, Array
+                  value.select(&:new_record?)
+                when Her::Model
+                  value if value.new_record?
+                end
+              else
+                value
+              end
+
+            value =
+              case value
+              when Her::Collection, Array
+                value.map(&:to_params).reject(&:empty?)
+              when Her::Model
+                value.to_params
+              end
+
+            write_attributes[assoc[:data_key]] = value if value.present?
           end
+        ensure
+          Thread.current[:her_embedded_params_objects] = nil if first
         end
 
         # Return or change the value of `include_root_in_json`
diff --git a/spec/model/associations_spec.rb b/spec/model/associations_spec.rb
index 1c17b4b3..d31cfe5b 100644
--- a/spec/model/associations_spec.rb
+++ b/spec/model/associations_spec.rb
@@ -18,7 +18,8 @@
             default: [],
             class_name: "Comment",
             path: "/comments",
-            inverse_of: nil
+            inverse_of: nil,
+            autosave: true
           }
         end
         before { Foo::User.has_many :comments }
@@ -34,7 +35,8 @@
             default: [],
             class_name: "Comment",
             path: "/comments",
-            inverse_of: nil
+            inverse_of: nil,
+            autosave: true
           }
         end
         let(:posts_association) do
@@ -44,7 +46,8 @@
             default: [],
             class_name: "Post",
             path: "/posts",
-            inverse_of: nil
+            inverse_of: nil,
+            autosave: true
           }
         end
         before do
@@ -66,7 +69,8 @@
             data_key: :category,
             default: nil,
             class_name: "Category",
-            path: "/category"
+            path: "/category",
+            autosave: true
           }
         end
         before { Foo::User.has_one :category }
@@ -81,7 +85,8 @@
             data_key: :category,
             default: nil,
             class_name: "Category",
-            path: "/category"
+            path: "/category",
+            autosave: true
           }
         end
         let(:role_association) do
@@ -90,7 +95,8 @@
             data_key: :role,
             default: nil,
             class_name: "Role",
-            path: "/role"
+            path: "/role",
+            autosave: true
           }
         end
         before do
@@ -113,7 +119,8 @@
             default: nil,
             class_name: "Organization",
             foreign_key: "organization_id",
-            path: "/organizations/:id"
+            path: "/organizations/:id",
+            autosave: true
           }
         end
         before { Foo::User.belongs_to :organization }
@@ -129,7 +136,8 @@
             default: nil,
             class_name: "Organization",
             foreign_key: "organization_id",
-            path: "/organizations/:id"
+            path: "/organizations/:id",
+            autosave: true
           }
         end
         let(:family_association) do
@@ -139,7 +147,8 @@
             default: nil,
             class_name: "Family",
             foreign_key: "family_id",
-            path: "/families/:id"
+            path: "/families/:id",
+            autosave: true
           }
         end
         before do
@@ -168,7 +177,8 @@
               default: {},
               class_name: "Post",
               path: "/comments",
-              inverse_of: :admin
+              inverse_of: :admin,
+              autosave: true
             }
           end
           before do
@@ -193,7 +203,8 @@
               default: nil,
               class_name: "Topic",
               foreign_key: "topic_id",
-              path: "/category"
+              path: "/category",
+              autosave: true
             }
           end
           before do
@@ -217,7 +228,8 @@
               default: true,
               class_name: "Business",
               foreign_key: "org_id",
-              path: "/organizations/:id"
+              path: "/organizations/:id",
+              autosave: true
             }
           end
           before do
@@ -252,7 +264,8 @@
               default: [],
               class_name: "Post",
               path: "/comments",
-              inverse_of: nil
+              inverse_of: nil,
+              autosave: true
             }
           end
 
@@ -376,6 +389,139 @@
         expect(user_params[:organization]).to be_kind_of(Hash)
         expect(user_params[:organization]).not_to be_empty
       end
+
+      context 'and send_only_modified_attributes is true' do
+        before do
+          Her::API.default_api.options[:send_only_modified_attributes] = true
+        end
+
+        it 'does not include unmodified has_many relationships in params' do
+          params = user.to_params
+          expect(params[:comments]).to be_nil
+        end
+
+        it 'does not include an unmodified has_one relationship in params' do
+          params = user.to_params
+          expect(params[:role]).to be_nil
+        end
+
+        it 'does not include an unmodified belongs_to relationship in params' do
+          params = user.to_params
+          expect(params[:organization]).to be_nil
+        end
+
+        it 'includes a modified has_many relationship in params' do
+          user.comments.last.body = 'Merry Christmas!'
+          params = user.to_params
+          expect(params[:comments]).to be_kind_of(Array)
+          expect(params[:comments].length).to eq(1)
+        end
+
+        it 'includes a modified has_one relationship in params' do
+          user.role.body = 'Guest'
+          params = user.to_params
+          expect(params[:role]).to be_kind_of(Hash)
+          expect(params[:role]).not_to be_empty
+        end
+
+        it 'includes a modified belongs_to relationship in params' do
+          user.organization.name = 'New Company'
+          params = user.to_params
+          expect(params[:organization]).to be_kind_of(Hash)
+          expect(params[:organization]).not_to be_empty
+        end
+      end
+
+      context 'and autosave as nil' do
+        before do
+          Foo::User.associations.values.each do |assocs|
+            assocs.each do |assoc|
+              assoc[:autosave] = nil
+            end
+          end
+        end
+
+        it 'does not include persisted has_many relationships in params' do
+          params = user.to_params
+          expect(params[:comments]).to be_nil
+        end
+
+        it 'does not include a persisted has_one relationship in params' do
+          params = user.to_params
+          expect(params[:role]).to be_nil
+        end
+
+        it 'does not include a persisted belongs_to relationship in params' do
+          params = user.to_params
+          expect(params[:organization]).to be_nil
+        end
+
+        it 'includes a new has_many relationship in params' do
+          new = user.comments.build(body: 'Merry Christmas!')
+          user.comments << new
+          params = user.to_params
+          expect(params[:comments]).to be_kind_of(Array)
+          expect(params[:comments].length).to eq(1)
+        end
+
+        it 'includes a new has_one relationship in params' do
+          user.role = Foo::Role.build(body: 'User')
+          params = user.to_params
+          expect(params[:role]).to be_kind_of(Hash)
+          expect(params[:role]).not_to be_empty
+        end
+
+        it 'includes a new belongs_to relationship in params' do
+          user.organization = Foo::Organization.build(name: 'Bluth Company')
+          params = user.to_params
+          expect(params[:organization]).to be_kind_of(Hash)
+          expect(params[:organization]).not_to be_empty
+        end
+      end
+
+      context 'and autosave as false' do
+        before do
+          Foo::User.associations.values.each do |assocs|
+            assocs.each do |assoc|
+              assoc[:autosave] = false
+            end
+          end
+        end
+
+        it 'does not include persisted has_many relationships in params' do
+          params = user.to_params
+          expect(params[:comments]).to be_nil
+        end
+
+        it 'does not include a persisted has_one relationship in params' do
+          params = user.to_params
+          expect(params[:role]).to be_nil
+        end
+
+        it 'does not include a persisted belongs_to relationship in params' do
+          params = user.to_params
+          expect(params[:organization]).to be_nil
+        end
+
+        it 'does not include a new has_many relationship in params' do
+          new = user.comments.build(body: 'Merry Christmas!')
+          user.comments << new
+          params = user.to_params
+          expect(params[:comments]).to be_nil
+        end
+
+        it 'does not include a new has_one relationship in params' do
+          user.role = Foo::Role.build(body: 'User')
+          params = user.to_params
+          expect(params[:role]).to be_nil
+        end
+
+        it 'does not include a new belongs_to relationship in params' do
+          user.organization = Foo::Organization.build(name: 'Bluth Company')
+          params = user.to_params
+          expect(params[:organization]).to be_nil
+        end
+      end
     end
 
     context "without included data" do

From e0c51283db9b6e71867f002ba4821848d80d0f69 Mon Sep 17 00:00:00 2001
From: James Le Cuirot <james.le-cuirot@yakara.com>
Date: Fri, 23 Feb 2018 13:02:06 +0000
Subject: [PATCH 10/12] Work around HashWithIndifferentAccess issue with
 ActiveSupport 3

Arrays added to the hash are always duplicated. This was fixed in
version 4.
---
 lib/her/model/associations/association.rb | 1 +
 1 file changed, 1 insertion(+)

diff --git a/lib/her/model/associations/association.rb b/lib/her/model/associations/association.rb
index 74b41f2e..b9bf9801 100644
--- a/lib/her/model/associations/association.rb
+++ b/lib/her/model/associations/association.rb
@@ -158,6 +158,7 @@ def assign(resources)
           reset
           set_missing_and_inverse_from_parent(resources)
           @parent.attributes[@name] = resources
+          resources = @parent.attributes[@name] if ActiveSupport::VERSION::MAJOR == 3
           @cached_result = resources
         end
 

From b1ddb12f2b8f487fb57d970dad4ce8f908343986 Mon Sep 17 00:00:00 2001
From: James Le Cuirot <james.le-cuirot@yakara.com>
Date: Wed, 11 Apr 2018 17:53:38 +0100
Subject: [PATCH 11/12] Utilise ActiveModel::Serializers::JSON in Her models

This makes as_json and to_json behave like they do under ActiveRecord,
excluding associations unless they are explicitly included. This is
necessary to avoid infinite loops.

Unfortunately this caused Her's include_root_in_json code to collide
with ActiveModel's include_root_in_json code. I removed Her's
getter/setter method in favour of ActiveModel's class attribute but
also provided a compatibility fix.
---
 lib/her/model.rb                 |  4 ++-
 lib/her/model/parse.rb           | 22 ++------------
 lib/her/model/serialization.rb   | 50 ++++++++++++++++++++++++++++++++
 spec/support/extensions/array.rb |  6 ----
 spec/support/extensions/hash.rb  |  6 ----
 5 files changed, 55 insertions(+), 33 deletions(-)
 create mode 100644 lib/her/model/serialization.rb
 delete mode 100644 spec/support/extensions/array.rb
 delete mode 100644 spec/support/extensions/hash.rb

diff --git a/lib/her/model.rb b/lib/her/model.rb
index 43929e5e..27bc7ad6 100644
--- a/lib/her/model.rb
+++ b/lib/her/model.rb
@@ -1,3 +1,4 @@
+require "active_model"
 require "her/model/base"
 require "her/model/deprecated_methods"
 require "her/model/http"
@@ -7,9 +8,9 @@
 require "her/model/parse"
 require "her/model/associations"
 require "her/model/introspection"
+require "her/model/serialization"
 require "her/model/paths"
 require "her/model/nested_attributes"
-require "active_model"
 
 module Her
   # This module is the main element of Her. After creating a Her::API object,
@@ -33,6 +34,7 @@ module Model
     include Her::Model::HTTP
     include Her::Model::Parse
     include Her::Model::Introspection
+    include Her::Model::Serialization
     include Her::Model::Paths
     include Her::Model::Associations
     include Her::Model::NestedAttributes
diff --git a/lib/her/model/parse.rb b/lib/her/model/parse.rb
index fbbbd281..d1bc7141 100644
--- a/lib/her/model/parse.rb
+++ b/lib/her/model/parse.rb
@@ -48,7 +48,7 @@ def to_params(attributes, changes = {})
 
           embed_params!(attributes, filtered_attributes)
 
-          if include_root_in_json?
+          if include_root_in_json
             if json_api_format?
               { included_root_element => [filtered_attributes] }
             else
@@ -98,18 +98,6 @@ def embed_params!(read_attributes, write_attributes)
           Thread.current[:her_embedded_params_objects] = nil if first
         end
 
-        # Return or change the value of `include_root_in_json`
-        #
-        # @example
-        #   class User
-        #     include Her::Model
-        #     include_root_in_json true
-        #   end
-        def include_root_in_json(value, options = {})
-          @_her_include_root_in_json = value
-          @_her_include_root_in_json_format = options[:format]
-        end
-
         # Return or change the value of `parse_root_in_json`
         #
         # @example
@@ -174,7 +162,7 @@ def root_element_included?(data)
 
         # @private
         def included_root_element
-          include_root_in_json? == true ? root_element : include_root_in_json?
+          include_root_in_json == true ? root_element : include_root_in_json
         end
 
         # Extract an array from the request data
@@ -232,12 +220,6 @@ def request_new_object_on_build?
           superclass.respond_to?(:request_new_object_on_build?) && superclass.request_new_object_on_build?
         end
 
-        # @private
-        def include_root_in_json?
-          return @_her_include_root_in_json unless @_her_include_root_in_json.nil?
-          superclass.respond_to?(:include_root_in_json?) && superclass.include_root_in_json?
-        end
-
         # @private
         def parse_root_in_json?
           return @_her_parse_root_in_json unless @_her_parse_root_in_json.nil?
diff --git a/lib/her/model/serialization.rb b/lib/her/model/serialization.rb
new file mode 100644
index 00000000..7e710598
--- /dev/null
+++ b/lib/her/model/serialization.rb
@@ -0,0 +1,50 @@
+module Her
+  module Model
+    module Serialization
+      extend ActiveSupport::Concern
+      include ActiveModel::Serializers::JSON
+
+      def serializable_hash(options = nil)
+        options = options.try(:dup) || {}
+
+        options[:except] = Array(options[:except]).map(&:to_s)
+        options[:except] |= self.class.association_names.map(&:to_s)
+
+        super(options)
+      end
+
+      included do
+        # Rails 3 defaulted to true but Her has always defaulted to
+        # false. This can be dropped when Rails 3 support is dropped.
+        self.include_root_in_json = false
+
+        # Rails creates include_root_in_json as a class attribute but
+        # Her previously had its own implementation combining the
+        # getter and setter. This monstrosity tries to achieve
+        # compatibility in the simplest way possible.
+        class << self
+
+          realias = proc do
+            alias_method :include_root_in_json_getter, :include_root_in_json
+
+            def include_root_in_json(value = nil)
+              if value.nil?
+                include_root_in_json_getter
+              else
+                self.include_root_in_json = value
+              end
+            end
+          end
+
+          realias.call
+          alias_method :include_root_in_json_without_realias, :include_root_in_json=
+
+          define_method :include_root_in_json= do |value|
+            include_root_in_json_without_realias value
+            realias.call
+          end
+        end
+      end
+    end
+  end
+end
diff --git a/spec/support/extensions/array.rb b/spec/support/extensions/array.rb
deleted file mode 100644
index 9c938e33..00000000
--- a/spec/support/extensions/array.rb
+++ /dev/null
@@ -1,6 +0,0 @@
-class Array
-
-  def to_json
-    MultiJson.dump(self)
-  end
-end
diff --git a/spec/support/extensions/hash.rb b/spec/support/extensions/hash.rb
deleted file mode 100644
index 45c552ab..00000000
--- a/spec/support/extensions/hash.rb
+++ /dev/null
@@ -1,6 +0,0 @@
-class Hash
-
-  def to_json
-    MultiJson.dump(self)
-  end
-end

From 80d3ae50145bf511cfa5cae276986dff769c9511 Mon Sep 17 00:00:00 2001
From: James Le Cuirot <james.le-cuirot@yakara.com>
Date: Wed, 9 May 2018 14:14:56 +0100
Subject: [PATCH 12/12] Serialize nested Her::Model instances that are not
 associated

We previously added serialization for ActiveModel::Serialization
instances but Her::Model instances can still be left untouched if they
do not belong to an association.

In order to serialize these safely, we have to move this block of code
into embed_params!, which guards against infinite loops. This has the
added benefit of not doing unnecessary serialization when
send_only_modified_attributes is enabled.
---
 .rubocop_todo.yml        |  5 -----
 lib/her/model/parse.rb   | 30 +++++++++++++++++++++---------
 spec/model/parse_spec.rb | 10 ++++++++++
 3 files changed, 31 insertions(+), 14 deletions(-)

diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml
index a54fa0a6..f4616f4b 100644
--- a/.rubocop_todo.yml
+++ b/.rubocop_todo.yml
@@ -13,11 +13,6 @@ Bundler/DuplicatedGem:
   Exclude:
     - 'Gemfile'
 
-# Offense count: 1
-Lint/EmptyWhen:
-  Exclude:
-    - 'lib/her/model/parse.rb'
-
 # Offense count: 1
 Lint/IneffectiveAccessModifier:
   Exclude:
diff --git a/lib/her/model/parse.rb b/lib/her/model/parse.rb
index d1bc7141..e3f84ef8 100644
--- a/lib/her/model/parse.rb
+++ b/lib/her/model/parse.rb
@@ -32,15 +32,7 @@ def parse(data)
 
         # @private
         def to_params(attributes, changes = {})
-          filtered_attributes = attributes.each_with_object({}) do |(key, value), memo|
-            case value
-            when Her::Model
-            when ActiveModel::Serialization
-              value = value.serializable_hash.symbolize_keys
-            end
-
-            memo[key.to_sym] = value
-          end
+          filtered_attributes = attributes.symbolize_keys
 
           if her_api.options[:send_only_modified_attributes]
             filtered_attributes.slice! *changes.keys.map(&:to_sym)
@@ -94,6 +86,26 @@ def embed_params!(read_attributes, write_attributes)
 
             write_attributes[assoc[:data_key]] = value if value.present?
           end
+
+          write_attributes.each do |key, value|
+            value =
+              case value
+              when Her::Collection
+                value.map(&:to_params).reject(&:empty?)
+              when Her::Model
+                value.to_params
+              when ActiveModel::Serialization
+                value.serializable_hash.symbolize_keys
+              end
+
+            if value
+              if value.empty?
+                write_attributes.delete(key)
+              else
+                write_attributes[key] = value
+              end
+            end
+          end
         ensure
           Thread.current[:her_embedded_params_objects] = nil if first
         end
diff --git a/spec/model/parse_spec.rb b/spec/model/parse_spec.rb
index 25b617a8..857d5bd3 100644
--- a/spec/model/parse_spec.rb
+++ b/spec/model/parse_spec.rb
@@ -344,6 +344,10 @@ def to_params
           { fullname: "Lindsay Fünke" }
         end
       end
+
+      spawn_model "Foo::Comment" do
+        attributes :user
+      end
     end
 
     it "changes the request parameters for one-line resource creation" do
@@ -356,6 +360,12 @@ def to_params
       @user.save
       expect(@user.fullname).to eq("Lindsay Fünke")
     end
+
+    it 'changes the request parameters when nested inside an unassociated resource' do
+      @user = Foo::User.new(fullname: "Tobias Fünke")
+      @comment = Foo::Comment.new(user: @user)
+      expect(@comment.to_params).to eq(user: { fullname: "Lindsay Fünke" })
+    end
   end
 
   context "when parse_root_in_json set json_api to true" do