From 5d3f56f0a4e3083f34fddcf943e54aed5f5aa5c6 Mon Sep 17 00:00:00 2001 From: Alex Zherdev Date: Tue, 15 Oct 2019 13:12:36 -0700 Subject: [PATCH 1/4] Use SHA1 instead of MD5 for caching --- lib/active_model/serializer/concerns/caching.rb | 2 +- test/cache_test.rb | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/active_model/serializer/concerns/caching.rb b/lib/active_model/serializer/concerns/caching.rb index 35bd8e649..7b3ff37eb 100644 --- a/lib/active_model/serializer/concerns/caching.rb +++ b/lib/active_model/serializer/concerns/caching.rb @@ -56,7 +56,7 @@ def _cache_digest def digest_caller_file(caller_line) serializer_file_path = caller_line[CALLER_FILE] serializer_file_contents = IO.read(serializer_file_path) - Digest::MD5.hexdigest(serializer_file_contents) + Digest::SHA1.hexdigest(serializer_file_contents) rescue TypeError, Errno::ENOENT warn <<-EOF.strip_heredoc Cannot digest non-existent file: '#{caller_line}'. diff --git a/test/cache_test.rb b/test/cache_test.rb index fdab362a2..f67f98a67 100644 --- a/test/cache_test.rb +++ b/test/cache_test.rb @@ -447,13 +447,13 @@ def test_a_serializer_rendered_by_two_adapter_returns_differently_fetch_attribut def test_uses_file_digest_in_cache_key render_object_with_cache(@blog) - file_digest = Digest::MD5.hexdigest(File.open(__FILE__).read) + file_digest = Digest::SHA1.hexdigest(File.open(__FILE__).read) key = "#{@blog.cache_key}/#{adapter.cache_key}/#{file_digest}" assert_equal(@blog_serializer.attributes, cache_store.fetch(key)) end def test_cache_digest_definition - file_digest = Digest::MD5.hexdigest(File.open(__FILE__).read) + file_digest = Digest::SHA1.hexdigest(File.open(__FILE__).read) assert_equal(file_digest, @post_serializer.class._cache_digest) end @@ -560,7 +560,7 @@ def test_digest_caller_file path = file.path caller_line = "#{path}:1:in `'" file.close - assert_equal ActiveModel::Serializer.digest_caller_file(caller_line), Digest::MD5.hexdigest(contents) + assert_equal ActiveModel::Serializer.digest_caller_file(caller_line), Digest::SHA1.hexdigest(contents) ensure file.unlink FileUtils.remove_entry dir From 1c028785ebeeb130e2d42a3ade5fc86bf9ab495d Mon Sep 17 00:00:00 2001 From: Alex Zherdev Date: Wed, 16 Oct 2019 12:39:02 -0700 Subject: [PATCH 2/4] Introduce use_sha1_digests config option --- .../serializer/concerns/caching.rb | 3 +- test/cache_test.rb | 31 ++++++++++++++++++- 2 files changed, 32 insertions(+), 2 deletions(-) diff --git a/lib/active_model/serializer/concerns/caching.rb b/lib/active_model/serializer/concerns/caching.rb index 7b3ff37eb..cdcd56852 100644 --- a/lib/active_model/serializer/concerns/caching.rb +++ b/lib/active_model/serializer/concerns/caching.rb @@ -56,7 +56,8 @@ def _cache_digest def digest_caller_file(caller_line) serializer_file_path = caller_line[CALLER_FILE] serializer_file_contents = IO.read(serializer_file_path) - Digest::SHA1.hexdigest(serializer_file_contents) + algorithm = ActiveModelSerializers.config.use_sha1_digests ? Digest::SHA1 : Digest::MD5 + algorithm.hexdigest(serializer_file_contents) rescue TypeError, Errno::ENOENT warn <<-EOF.strip_heredoc Cannot digest non-existent file: '#{caller_line}'. diff --git a/test/cache_test.rb b/test/cache_test.rb index f67f98a67..112e366cb 100644 --- a/test/cache_test.rb +++ b/test/cache_test.rb @@ -446,15 +446,39 @@ def test_a_serializer_rendered_by_two_adapter_returns_differently_fetch_attribut # rubocop:enable Metrics/AbcSize def test_uses_file_digest_in_cache_key + reset_cache_digest(@blog_serializer) + render_object_with_cache(@blog) + file_digest = Digest::MD5.hexdigest(File.open(__FILE__).read) + key = "#{@blog.cache_key}/#{adapter.cache_key}/#{file_digest}" + assert_equal(@blog_serializer.attributes, cache_store.fetch(key)) + end + + def test_uses_sha1_digest_in_cache_key_when_configured + reset_cache_digest(@blog_serializer) + previous_use_sha1_digests = ActiveModelSerializers.config.use_sha1_digests + ActiveModelSerializers.config.use_sha1_digests = true render_object_with_cache(@blog) file_digest = Digest::SHA1.hexdigest(File.open(__FILE__).read) key = "#{@blog.cache_key}/#{adapter.cache_key}/#{file_digest}" assert_equal(@blog_serializer.attributes, cache_store.fetch(key)) + ensure + ActiveModelSerializers.config.use_sha1_digests = previous_use_sha1_digests end def test_cache_digest_definition + reset_cache_digest(@post_serializer) + file_digest = Digest::MD5.hexdigest(File.open(__FILE__).read) + assert_equal(file_digest, @post_serializer.class._cache_digest) + end + + def test_cache_sha1_digest_definition + reset_cache_digest(@post_serializer) + previous_use_sha1_digests = ActiveModelSerializers.config.use_sha1_digests + ActiveModelSerializers.config.use_sha1_digests = true file_digest = Digest::SHA1.hexdigest(File.open(__FILE__).read) assert_equal(file_digest, @post_serializer.class._cache_digest) + ensure + ActiveModelSerializers.config.use_sha1_digests = previous_use_sha1_digests end def test_object_cache_keys @@ -560,7 +584,7 @@ def test_digest_caller_file path = file.path caller_line = "#{path}:1:in `'" file.close - assert_equal ActiveModel::Serializer.digest_caller_file(caller_line), Digest::SHA1.hexdigest(contents) + assert_equal ActiveModel::Serializer.digest_caller_file(caller_line), Digest::MD5.hexdigest(contents) ensure file.unlink FileUtils.remove_entry dir @@ -715,5 +739,10 @@ def render_object_with_cache(obj, options = {}) def adapter @serializable_resource.adapter end + + def reset_cache_digest(serializer) + return unless serializer.class.instance_variable_defined?(:@_cache_digest) + serializer.class.remove_instance_variable(:@_cache_digest) + end end end From 7e1c9fcf64a08f4fc149e5f73b20927dcbe83e89 Mon Sep 17 00:00:00 2001 From: Alex Zherdev Date: Thu, 17 Oct 2019 10:43:47 -0700 Subject: [PATCH 3/4] Update changelog --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9b2df5e74..91a2c7869 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,8 @@ Breaking changes: Features: +- [#2361](https://github.com/rails-api/active_model_serializers/pull/2361) Add `ActiveModelSerializers.config.use_sha1_digests` to allow customization of the hashing algorithm used for serializer caching (@alexzherdev) + Fixes: - [#2344](https://github.com/rails-api/active_model_serializers/pull/2344) Fixes #2341 introduced since #2223 (@wasifhossain) From 628eff258276d8913a8ea239771c7510f27f3447 Mon Sep 17 00:00:00 2001 From: Alex Zherdev Date: Thu, 17 Oct 2019 11:07:37 -0700 Subject: [PATCH 4/4] Update configuration options doc --- docs/general/configuration_options.md | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/docs/general/configuration_options.md b/docs/general/configuration_options.md index a07d71ee8..0f9b2aaee 100644 --- a/docs/general/configuration_options.md +++ b/docs/general/configuration_options.md @@ -111,6 +111,18 @@ ActiveModelSerializers.config.serializer_lookup_chain.unshift( See [lookup_chain.rb](https://github.com/rails-api/active_model_serializers/blob/master/lib/active_model_serializers/lookup_chain.rb) for further explanations and examples. +#### use_sha1_digests + +Determines which hashing algorithm to use internally when caching serializers. + +Possible values: + +- `true` +- `false` (default) + +When `true`, ActiveModelSerializers will use SHA1. Otherwise, it will default to using MD5. +Be warned that changing this value may result in serializer caches being invalidated. + ## JSON API ##### jsonapi_resource_type