From ab502f0c522c6f1342997041a99d274fa5029cd5 Mon Sep 17 00:00:00 2001 From: Kieran Huggins Date: Fri, 16 Oct 2015 16:05:38 -0400 Subject: [PATCH 1/3] move @node assignment and the include_associations! call outside the cached response, since including associations produces the side-effect *actually* including the associations. Previously, subsequent renders of a cached serializer would never include associations. --- lib/active_model/serializer.rb | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/lib/active_model/serializer.rb b/lib/active_model/serializer.rb index 73b6ba42..c202610e 100644 --- a/lib/active_model/serializer.rb +++ b/lib/active_model/serializer.rb @@ -353,13 +353,16 @@ module ActiveModel # Returns a hash representation of the serializable # object without the root. def serializable_hash - if perform_caching? + @node = if perform_caching? cache.fetch expand_cache_key([self.class.to_s.underscore, cache_key, 'serializable-hash']) do _serializable_hash end else _serializable_hash end + + include_associations! if _embed + @node end def include_associations! @@ -476,9 +479,7 @@ module ActiveModel def _serializable_hash return nil if @object.nil? - @node = attributes - include_associations! if _embed - @node + attributes end def perform_caching? From b2dc2598fbc22428cdd5e4094f6763abc8782858 Mon Sep 17 00:00:00 2001 From: Kieran Huggins Date: Fri, 16 Oct 2015 16:42:30 -0400 Subject: [PATCH 2/3] add specs for cached attributes --- test/caching_test.rb | 81 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 81 insertions(+) diff --git a/test/caching_test.rb b/test/caching_test.rb index 869f0f93..f07f6c31 100644 --- a/test/caching_test.rb +++ b/test/caching_test.rb @@ -35,6 +35,42 @@ class CachingTest < ActiveModel::TestCase end end + class Parent + def id + 'parent1' + end + + def name + 'Kieran' + end + + def children + [ Child.new ] + end + + def read_attribute_for_serialization(name) + send name + end + end + + class Child + def id + 'child1' + end + + def name + 'Joshua' + end + + def parent + Parent.new + end + + def read_attribute_for_serialization(name) + send name + end + end + def test_serializers_have_a_cache_store ActiveModel::Serializer.cache = NullStore.new @@ -93,4 +129,49 @@ class CachingTest < ActiveModel::TestCase assert_equal instance.serializable_array, serializer.cache.read('array_serializer/cache-key/serializable-array') assert_equal instance.to_json, serializer.cache.read('array_serializer/cache-key/to-json') end + + def test_cached_serializers_return_associations + + child_serializer = Class.new(ActiveModel::Serializer) do + cached true + attributes :name + + def self.to_s + 'child_serializer' + end + + def cache_key + object.name + end + end + + parent_serializer = Class.new(ActiveModel::Serializer) do + cached true + attributes :name + + has_many :children, serializer: child_serializer, embed: :ids, include: true + + def self.to_s + 'parent_serializer' + end + + def cache_key + object.name + end + end + + + parent_serializer.cache = NullStore.new + child_serializer.cache = NullStore.new + + instance = parent_serializer.new Parent.new, root: :parent + + initial_keys = instance.as_json.keys + + assert_equal(initial_keys, [:children, :parent]) + + cached_keys = instance.as_json.keys + + assert_equal(cached_keys, [:children, :parent]) + end end From 82a870d9dc1e9b57cccbc3ac3bff707ce3663f0c Mon Sep 17 00:00:00 2001 From: Kieran Huggins Date: Tue, 20 Oct 2015 18:33:23 -0400 Subject: [PATCH 3/3] add guard against including associations for nil objects --- lib/active_model/serializer.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/active_model/serializer.rb b/lib/active_model/serializer.rb index c202610e..ef47178b 100644 --- a/lib/active_model/serializer.rb +++ b/lib/active_model/serializer.rb @@ -361,7 +361,7 @@ module ActiveModel _serializable_hash end - include_associations! if _embed + include_associations! if @object.present? && _embed @node end