Skip to content

Commit

Permalink
Utilise ActiveModel::Serializers::JSON in Her models
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
chewi committed Apr 12, 2018
1 parent cad9a0e commit 9d528ca
Show file tree
Hide file tree
Showing 5 changed files with 43 additions and 30 deletions.
4 changes: 3 additions & 1 deletion lib/her/model.rb
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
require "active_model"
require "her/model/base"
require "her/model/deprecated_methods"
require "her/model/http"
Expand All @@ -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,
Expand All @@ -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
Expand Down
21 changes: 2 additions & 19 deletions lib/her/model/parse.rb
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,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
Expand Down Expand Up @@ -93,18 +93,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
Expand Down Expand Up @@ -168,7 +156,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
Expand Down Expand Up @@ -225,11 +213,6 @@ def request_new_object_on_build?
@_her_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?
@_her_include_root_in_json || (superclass.respond_to?(:include_root_in_json?) && superclass.include_root_in_json?)
end

# @private
def parse_root_in_json?
@_her_parse_root_in_json || (superclass.respond_to?(:parse_root_in_json?) && superclass.parse_root_in_json?)
Expand Down
38 changes: 38 additions & 0 deletions lib/her/model/serialization.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
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

class << self
alias_method :include_root_in_json_getter, :include_root_in_json

# Rails creates include_root_in_json as a class attribute
# but Her previously had its own implementation combining
# the getter and setter. This provides compatibility.
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
end
end
end
end
5 changes: 0 additions & 5 deletions spec/support/extensions/array.rb

This file was deleted.

5 changes: 0 additions & 5 deletions spec/support/extensions/hash.rb

This file was deleted.

3 comments on commit 9d528ca

@edtjones
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is/was include_root_in_json?() called anywhere else? could it be considered part of a public api? If so, we should probably alias or define that method again shouldn't we?

(Unless I've totally missed the point)

@chewi
Copy link
Contributor Author

@chewi chewi commented on 9d528ca Apr 12, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's marked @private so I figured it was fair game for removal. It was used in included_root_element and I dealt with that.

@edtjones
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry - missed the @private comment in the diff. Yeah I agree, seems ok to remove.

Please sign in to comment.