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/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 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.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/associations/association.rb b/lib/her/model/associations/association.rb index 3548dc80..b9bf9801 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 = {} @@ -40,20 +41,69 @@ def assign_single_nested_attributes(attributes) end # @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? + 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| + 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 - 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? + # @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 - path = build_association_path -> { "#{@parent.request_path(@params)}#{@opts[:path]}" } - @klass.get(path, @params).tap do |result| - @cached_result = result unless @params.any? + iname = inverse ? inverse[:name] : @parent_name + resource.send("#{iname}=", @parent) end end + # @private + def fetch(opts = {}) + 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 + + 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 + + set_missing_and_inverse_from_parent result if result + result + end + # @private def build_association_path(code) instance_exec(&code) @@ -61,6 +111,41 @@ def build_association_path(code) nil end + # @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) + 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, inverse + + if resource + resource + else + retried = true + retry + end + end + end + # @private def reset @params = {} @@ -68,6 +153,15 @@ def reset @parent.attributes.delete(@name) end + # @private + 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 + # Add query parameters to the HTTP request performed to fetch the data # # @example @@ -97,7 +191,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/belongs_to_association.rb b/lib/her/model/associations/belongs_to_association.rb index 6b9accfc..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 @@ -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::BelongsToAssociation.proxy(self, #{opts.inspect})) end + + def #{name}=(resource) + send("#{name}").association.assign(resource) + end RUBY end @@ -86,6 +91,28 @@ def fetch end end + # @private + def assign(resource) + reset + pkey = resource ? resource.id : nil + @parent.send("#{@opts[:foreign_key]}=", pkey) + @parent.attributes[@name] = resource + @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/associations/has_many_association.rb b/lib/her/model/associations/has_many_association.rb index 23ed797a..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 @@ -49,10 +54,8 @@ def self.parse(association, klass, data) # user = User.find(1) # new_comment = user.comments.build(:body => "Hello!") # new_comment # => # - # 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 @@ -72,23 +75,16 @@ def build(attributes = {}) # user.comments # => [#] 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 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/lib/her/model/associations/has_one_association.rb b/lib/her/model/associations/has_one_association.rb index 80cb3cd3..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 @@ -45,7 +50,7 @@ def self.parse(*args) # new_role = user.role.build(:title => "moderator") # new_role # => # 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 @@ -65,7 +70,8 @@ def build(attributes = {}) # user.role # => # 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 1737c488..c64867a1 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) && 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/introspection.rb b/lib/her/model/introspection.rb index 34f15926..78832b81 100644 --- a/lib/her/model/introspection.rb +++ b/lib/her/model/introspection.rb @@ -12,13 +12,26 @@ module Introspection # @user = User.find(1) # p @user # => # 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 - "" + joined = e.missing_parameters.map { |m| "`#{m}`" }.join(", ") + "" 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 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/lib/her/model/parse.rb b/lib/her/model/parse.rb index 51b4a31a..e3f84ef8 100644 --- a/lib/her/model/parse.rb +++ b/lib/her/model/parse.rb @@ -32,23 +32,15 @@ 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.merge!(embeded_params(attributes)) + filtered_attributes = attributes.symbolize_keys if her_api.options[:send_only_modified_attributes] filtered_attributes.slice! *changes.keys.map(&:to_sym) end - if include_root_in_json? + embed_params!(attributes, filtered_attributes) + + if include_root_in_json if json_api_format? { included_root_element => [filtered_attributes] } else @@ -60,28 +52,62 @@ 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 - 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] + 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 # Return or change the value of `parse_root_in_json` @@ -148,7 +174,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 @@ -206,12 +232,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/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/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/model/associations_spec.rb b/spec/model/associations_spec.rb index a6632fa4..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 @@ -272,10 +285,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 +308,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 +338,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 @@ -359,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 @@ -370,7 +533,10 @@ 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| if env[:params]["admin"] == "true" [200, {}, { organization: { id: 2, name: "Bluth Company (admin)" } }.to_json] @@ -384,6 +550,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) @@ -436,10 +606,33 @@ 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 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 @@ -764,7 +957,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 +984,30 @@ def present? expect(comment.body).to eq("Hello!") 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") + 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 +1020,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 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 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 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