From 11142e9cebbcda95a3452388a28bab96e09e1022 Mon Sep 17 00:00:00 2001 From: Paulo Mello Date: Sun, 19 Jun 2016 19:52:14 -0300 Subject: [PATCH 1/5] Ensuring read_multi works with fragment cache. --- lib/active_model/serializer/caching.rb | 17 ++++++++------- test/cache_test.rb | 30 ++++++++++++++++++++++++++ 2 files changed, 39 insertions(+), 8 deletions(-) diff --git a/lib/active_model/serializer/caching.rb b/lib/active_model/serializer/caching.rb index dfef8f020..4648be4fd 100644 --- a/lib/active_model/serializer/caching.rb +++ b/lib/active_model/serializer/caching.rb @@ -197,7 +197,7 @@ def object_cache_keys(collection_serializer, adapter_instance, include_directive def object_cache_key(serializer, adapter_instance) return unless serializer.present? && serializer.object.present? - serializer.class.cache_enabled? ? serializer.cache_key(adapter_instance) : nil + (serializer.class.cache_enabled? || serializer.class.fragment_cache_enabled?) ? serializer.cache_key(adapter_instance) : nil end end @@ -211,7 +211,7 @@ def fetch_attributes(fields, cached_attributes, adapter_instance) end end elsif serializer_class.fragment_cache_enabled? - fetch_attributes_fragment(adapter_instance) + fetch_attributes_fragment(adapter_instance, cached_attributes) else attributes(fields, true) end @@ -230,7 +230,7 @@ def fetch(adapter_instance, cache_options = serializer_class._cache_options) # 1. Determine cached fields from serializer class options # 2. Get non_cached_fields and fetch cache_fields # 3. Merge the two hashes using adapter_instance#fragment_cache - def fetch_attributes_fragment(adapter_instance) + def fetch_attributes_fragment(adapter_instance, cached_attributes={}) serializer_class._cache_options ||= {} serializer_class._cache_options[:key] = serializer_class._cache_key if serializer_class._cache_key fields = serializer_class.fragmented_attributes @@ -243,12 +243,13 @@ def fetch_attributes_fragment(adapter_instance) cached_fields = fields[:cached].dup key = cache_key(adapter_instance) cached_hash = - serializer_class.cache_store.fetch(key, serializer_class._cache_options) do - hash = attributes(cached_fields, true) - include_directive = JSONAPI::IncludeDirective.new(cached_fields - hash.keys) - hash.merge! resource_relationships({}, { include_directive: include_directive }, adapter_instance) + cached_attributes.fetch(key) do + serializer_class.cache_store.fetch(key, serializer_class._cache_options) do + hash = attributes(cached_fields, true) + include_directive = JSONAPI::IncludeDirective.new(cached_fields - hash.keys) + hash.merge! resource_relationships({}, { include_directive: include_directive }, adapter_instance) + end end - # Merge both results adapter_instance.fragment_cache(cached_hash, non_cached_hash) end diff --git a/test/cache_test.rb b/test/cache_test.rb index 57312a911..b433ec36b 100644 --- a/test/cache_test.rb +++ b/test/cache_test.rb @@ -321,6 +321,36 @@ def test_fetch_attributes_from_cache end end + def test_cache_read_multi_with_fragment_cache_enabled + post_serializer = Class.new(ActiveModel::Serializer) do + cache except: [:body] + end + + serializers = ActiveModel::Serializer::CollectionSerializer.new([@post, @post], {serializer: post_serializer}) + + Timecop.freeze(Time.current) do + # Warming up. + options = {} + adapter_options = {} + adapter_instance = ActiveModelSerializers::Adapter::Attributes.new(serializers, adapter_options) + serializers.serializable_hash(adapter_options, options, adapter_instance) + + # Should find something with read_multi now + options = {} + adapter_options = {} + adapter_instance = ActiveModelSerializers::Adapter::Attributes.new(serializers, adapter_options) + serializers.serializable_hash(adapter_options, options, adapter_instance) + cached_attributes = adapter_options.fetch(:cached_attributes) + + include_directive = ActiveModelSerializers.default_include_directive + manual_cached_attributes = ActiveModel::Serializer.cache_read_multi(serializers, adapter_instance, include_directive) + + refute_equal 0, cached_attributes.size + refute_equal 0, manual_cached_attributes.size + assert_equal manual_cached_attributes, cached_attributes + end + end + def test_serializer_file_path_on_nix path = '/Users/git/emberjs/ember-crm-backend/app/serializers/lead_serializer.rb' caller_line = "#{path}:1:in `'" From b295ce4facf617d8a64c23c249d3d3cd461cc539 Mon Sep 17 00:00:00 2001 From: Paulo Mello Date: Sun, 19 Jun 2016 22:36:03 -0300 Subject: [PATCH 2/5] Minor reorg so rubocop doesn't arrest me. --- lib/active_model/serializer/caching.rb | 15 ++++++++++----- test/cache_test.rb | 2 +- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/lib/active_model/serializer/caching.rb b/lib/active_model/serializer/caching.rb index 4648be4fd..75b20aaa7 100644 --- a/lib/active_model/serializer/caching.rb +++ b/lib/active_model/serializer/caching.rb @@ -227,18 +227,23 @@ def fetch(adapter_instance, cache_options = serializer_class._cache_options) end end + def non_cached_attributes(adapter_instance, fields) + non_cached_fields = fields[:non_cached].dup + non_cached_hash = attributes(non_cached_fields, true) + include_directive = JSONAPI::IncludeDirective.new(non_cached_fields - non_cached_hash.keys) + non_cached_hash.merge! resource_relationships({}, { include_directive: include_directive }, adapter_instance) + non_cached_hash + end + # 1. Determine cached fields from serializer class options # 2. Get non_cached_fields and fetch cache_fields # 3. Merge the two hashes using adapter_instance#fragment_cache - def fetch_attributes_fragment(adapter_instance, cached_attributes={}) + def fetch_attributes_fragment(adapter_instance, cached_attributes = {}) serializer_class._cache_options ||= {} serializer_class._cache_options[:key] = serializer_class._cache_key if serializer_class._cache_key fields = serializer_class.fragmented_attributes - non_cached_fields = fields[:non_cached].dup - non_cached_hash = attributes(non_cached_fields, true) - include_directive = JSONAPI::IncludeDirective.new(non_cached_fields - non_cached_hash.keys) - non_cached_hash.merge! resource_relationships({}, { include_directive: include_directive }, adapter_instance) + non_cached_hash = non_cached_attributes(adapter_instance, fields) cached_fields = fields[:cached].dup key = cache_key(adapter_instance) diff --git a/test/cache_test.rb b/test/cache_test.rb index b433ec36b..e25c4396b 100644 --- a/test/cache_test.rb +++ b/test/cache_test.rb @@ -326,7 +326,7 @@ def test_cache_read_multi_with_fragment_cache_enabled cache except: [:body] end - serializers = ActiveModel::Serializer::CollectionSerializer.new([@post, @post], {serializer: post_serializer}) + serializers = ActiveModel::Serializer::CollectionSerializer.new([@post, @post], serializer: post_serializer) Timecop.freeze(Time.current) do # Warming up. From 536dabb1ee95b97fc16787c1153cf68d8a41216d Mon Sep 17 00:00:00 2001 From: Paulo Mello Date: Sun, 19 Jun 2016 22:36:19 -0300 Subject: [PATCH 3/5] Updating CHANGELOG with this PR. --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 701b45717..6632cc3f2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,7 @@ Breaking changes: Features: Fixes: +- [#1814] (https://github.com/rails-api/active_model_serializers/pull/1814) Ensuring read_multi works with fragment cache Misc: From decf41452cd672eb3e44e62c2c724e446d7a226c Mon Sep 17 00:00:00 2001 From: Paulo Mello Date: Mon, 20 Jun 2016 18:11:33 -0300 Subject: [PATCH 4/5] Disabling rubocop annoying complaint for now on a method that needs refactoring in the future. --- lib/active_model/serializer/caching.rb | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/lib/active_model/serializer/caching.rb b/lib/active_model/serializer/caching.rb index 75b20aaa7..20362ce80 100644 --- a/lib/active_model/serializer/caching.rb +++ b/lib/active_model/serializer/caching.rb @@ -227,23 +227,19 @@ def fetch(adapter_instance, cache_options = serializer_class._cache_options) end end - def non_cached_attributes(adapter_instance, fields) - non_cached_fields = fields[:non_cached].dup - non_cached_hash = attributes(non_cached_fields, true) - include_directive = JSONAPI::IncludeDirective.new(non_cached_fields - non_cached_hash.keys) - non_cached_hash.merge! resource_relationships({}, { include_directive: include_directive }, adapter_instance) - non_cached_hash - end - # 1. Determine cached fields from serializer class options # 2. Get non_cached_fields and fetch cache_fields # 3. Merge the two hashes using adapter_instance#fragment_cache + # rubocop:disable Metrics/AbcSize def fetch_attributes_fragment(adapter_instance, cached_attributes = {}) serializer_class._cache_options ||= {} serializer_class._cache_options[:key] = serializer_class._cache_key if serializer_class._cache_key fields = serializer_class.fragmented_attributes - non_cached_hash = non_cached_attributes(adapter_instance, fields) + non_cached_fields = fields[:non_cached].dup + non_cached_hash = attributes(non_cached_fields, true) + include_directive = JSONAPI::IncludeDirective.new(non_cached_fields - non_cached_hash.keys) + non_cached_hash.merge! resource_relationships({}, { include_directive: include_directive }, adapter_instance) cached_fields = fields[:cached].dup key = cache_key(adapter_instance) @@ -258,6 +254,7 @@ def fetch_attributes_fragment(adapter_instance, cached_attributes = {}) # Merge both results adapter_instance.fragment_cache(cached_hash, non_cached_hash) end + # rubocop:enable Metrics/AbcSize def cache_key(adapter_instance) return @cache_key if defined?(@cache_key) From aa85b5c5409dd689ef302b7e17f4b624f5fb1aa3 Mon Sep 17 00:00:00 2001 From: Paulo Mello Date: Mon, 20 Jun 2016 18:13:13 -0300 Subject: [PATCH 5/5] Reducing memory footprint of the test. --- test/cache_test.rb | 2 -- 1 file changed, 2 deletions(-) diff --git a/test/cache_test.rb b/test/cache_test.rb index e25c4396b..73d59bbd6 100644 --- a/test/cache_test.rb +++ b/test/cache_test.rb @@ -336,9 +336,7 @@ def test_cache_read_multi_with_fragment_cache_enabled serializers.serializable_hash(adapter_options, options, adapter_instance) # Should find something with read_multi now - options = {} adapter_options = {} - adapter_instance = ActiveModelSerializers::Adapter::Attributes.new(serializers, adapter_options) serializers.serializable_hash(adapter_options, options, adapter_instance) cached_attributes = adapter_options.fetch(:cached_attributes)