From 5efd9734ec541a85029255ca78ad9b7c52bf9ad5 Mon Sep 17 00:00:00 2001 From: Loh Wei Jun Date: Mon, 11 Oct 2021 23:34:57 +0800 Subject: [PATCH 1/6] Add ability to define cacheable_status_code as an option. Value is to be within CACHEABLE_STATUS_CODE pre-defined constant values --- lib/faraday_middleware/response/caching.rb | 9 ++++- spec/unit/caching_spec.rb | 38 ++++++++++++++++++++++ 2 files changed, 46 insertions(+), 1 deletion(-) diff --git a/lib/faraday_middleware/response/caching.rb b/lib/faraday_middleware/response/caching.rb index ad9cc52..492d98f 100644 --- a/lib/faraday_middleware/response/caching.rb +++ b/lib/faraday_middleware/response/caching.rb @@ -86,6 +86,12 @@ def full_key? @full_key ||= @options[:full_key] end + def custom_status_code + Array(@options[:cacheable_status_code]).map do |status_code| + CACHEABLE_STATUS_CODES.include?(status_code.to_i) ? status_code.to_i : nil + end.compact + end + def cache_on_complete(env) key = cache_key(env) if (cached_response = cache.read(key)) @@ -101,7 +107,8 @@ def cache_on_complete(env) end def store_response_in_cache(key, response) - return unless CACHEABLE_STATUS_CODES.include?(response.status) + @cacheable_status_code = custom_status_code.any? ? custom_status_code : CACHEABLE_STATUS_CODES + return unless @cacheable_status_code.include?(response.status) if @options[:write_options] cache.write(key, response, @options[:write_options]) diff --git a/spec/unit/caching_spec.rb b/spec/unit/caching_spec.rb index 5a78d14..fe384bb 100644 --- a/spec/unit/caching_spec.rb +++ b/spec/unit/caching_spec.rb @@ -40,6 +40,9 @@ def fetch(key) response = lambda { |_env| [200, { 'Content-Type' => 'text/plain' }, "request:#{request_count += 1}"] } + not_found = lambda { |_env| + [404, { 'Content-Type' => 'text/plain' }, "request:#{request_count += 1}"] + } broken = lambda { |_env| [500, { 'Content-Type' => 'text/plain' }, "request:#{request_count += 1}"] } @@ -54,6 +57,7 @@ def fetch(key) stub.get('/broken', &broken) stub.get('http://www.site-a.com/test', &response) stub.get('http://www.site-b.com/test', &response) + stub.get('/not_found', ¬_found) end end end @@ -95,6 +99,40 @@ def fetch(key) expect(get('/broken').body).to eq('request:2') end + context ':cacheable_status_code' do + let(:options) { { cacheable_status_code: %w[404] } } + + it 'caches requests based on defined cacheable_status_code' do + expect(get('/').body).to eq('request:1') + expect(get('/not_found').body).to eq('request:2') + expect(get('/').body).to eq('request:3') + expect(get('/not_found').body).to eq('request:2') + end + + context 'with invalid :cacheable_status_code status' do + let(:options) { { cacheable_status_code: %w[404,500] } } + + it 'caches requests based on valid defined cacheable_status_code' do + expect(get('/not_found').body).to eq('request:1') + expect(get('/broken').body).to eq('request:2') + expect(get('/not_found').body).to eq('request:1') + expect(get('/broken').body).to eq('request:3') + end + end + + context 'with no valid :cacheable_status_code status' do + let(:options) { { cacheable_status_code: %w[500] } } + it 'caches requests based on default cacheable_status_code' do + expect(get('/').body).to eq('request:1') + expect(get('/broken').body).to eq('request:2') + expect(get('/').body).to eq('request:1') + expect(get('/not_found').body).to eq('request:3') + expect(get('/not_found').body).to eq('request:3') + expect(get('/broken').body).to eq('request:4') + end + end + end + context ':ignore_params' do let(:options) { { ignore_params: %w[utm_source utm_term] } } From 805016227f6f6aea06b09370ed17b40b2990bc18 Mon Sep 17 00:00:00 2001 From: Loh Wei Jun Date: Wed, 13 Oct 2021 02:38:30 +0800 Subject: [PATCH 2/6] Slight refactor on custom_status_codes method: 1. memozied method to prevent unnecessary calls when called multiple times 2. shift logic into custom_status_codes 3. rename custom_status_code to custom_status_codes --- lib/faraday_middleware/response/caching.rb | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/lib/faraday_middleware/response/caching.rb b/lib/faraday_middleware/response/caching.rb index 492d98f..0a43046 100644 --- a/lib/faraday_middleware/response/caching.rb +++ b/lib/faraday_middleware/response/caching.rb @@ -86,10 +86,9 @@ def full_key? @full_key ||= @options[:full_key] end - def custom_status_code - Array(@options[:cacheable_status_code]).map do |status_code| - CACHEABLE_STATUS_CODES.include?(status_code.to_i) ? status_code.to_i : nil - end.compact + def custom_status_codes + @custom_status_codes ||= CACHEABLE_STATUS_CODES & Array(@options[:cacheable_status_code]).map(&:to_i) + @custom_status_codes.any? ? @custom_status_codes : CACHEABLE_STATUS_CODES end def cache_on_complete(env) @@ -107,8 +106,7 @@ def cache_on_complete(env) end def store_response_in_cache(key, response) - @cacheable_status_code = custom_status_code.any? ? custom_status_code : CACHEABLE_STATUS_CODES - return unless @cacheable_status_code.include?(response.status) + return unless custom_status_codes.include?(response.status) if @options[:write_options] cache.write(key, response, @options[:write_options]) From 724d6b4fe0b2eee679106f140f5eeff86fea37f5 Mon Sep 17 00:00:00 2001 From: Loh Wei Jun Date: Wed, 13 Oct 2021 02:55:47 +0800 Subject: [PATCH 3/6] Update initialize method documentation --- lib/faraday_middleware/response/caching.rb | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/lib/faraday_middleware/response/caching.rb b/lib/faraday_middleware/response/caching.rb index 0a43046..2e8a20f 100644 --- a/lib/faraday_middleware/response/caching.rb +++ b/lib/faraday_middleware/response/caching.rb @@ -26,15 +26,17 @@ class Caching < Faraday::Middleware # # cache - An object that responds to read and write (default: nil). # options - An options Hash (default: {}): - # :ignore_params - String name or Array names of query - # params that should be ignored when forming - # the cache key (default: []). - # :write_options - Hash of settings that should be passed as the - # third options parameter to the cache's #write - # method. If not specified, no options parameter - # will be passed. - # :full_key - Boolean - use full URL as cache key: - # (url.host + url.request_uri) + # :ignore_params - String name or Array names of query + # params that should be ignored when forming + # the cache key (default: []). + # :write_options - Hash of settings that should be passed as the + # third options parameter to the cache's #write + # method. If not specified, no options parameter + # will be passed. + # :full_key - Boolean - use full URL as cache key: + # (url.host + url.request_uri) + # :cacheable_status_code - Array of http status code to be cache + # (default: CACHEABLE_STATUS_CODE) # # Yields if no cache is given. The block should return a cache object. def initialize(app, cache = nil, options = {}) From b7bd9cbd457bb56fbe96e481194437cce055f96d Mon Sep 17 00:00:00 2001 From: Loh Wei Jun Date: Wed, 13 Oct 2021 16:40:35 +0800 Subject: [PATCH 4/6] Update cacheable_status_code option variable to 'status_code' --- lib/faraday_middleware/response/caching.rb | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/faraday_middleware/response/caching.rb b/lib/faraday_middleware/response/caching.rb index 2e8a20f..d06b7f8 100644 --- a/lib/faraday_middleware/response/caching.rb +++ b/lib/faraday_middleware/response/caching.rb @@ -26,16 +26,16 @@ class Caching < Faraday::Middleware # # cache - An object that responds to read and write (default: nil). # options - An options Hash (default: {}): - # :ignore_params - String name or Array names of query + # :ignore_params - String name or Array names of query # params that should be ignored when forming # the cache key (default: []). - # :write_options - Hash of settings that should be passed as the + # :write_options - Hash of settings that should be passed as the # third options parameter to the cache's #write # method. If not specified, no options parameter # will be passed. - # :full_key - Boolean - use full URL as cache key: + # :full_key - Boolean - use full URL as cache key: # (url.host + url.request_uri) - # :cacheable_status_code - Array of http status code to be cache + # :status_codes - Array of http status code to be cache # (default: CACHEABLE_STATUS_CODE) # # Yields if no cache is given. The block should return a cache object. @@ -89,7 +89,7 @@ def full_key? end def custom_status_codes - @custom_status_codes ||= CACHEABLE_STATUS_CODES & Array(@options[:cacheable_status_code]).map(&:to_i) + @custom_status_codes ||= CACHEABLE_STATUS_CODES & Array(@options[:status_codes]).map(&:to_i) @custom_status_codes.any? ? @custom_status_codes : CACHEABLE_STATUS_CODES end From 68b08b4d0e7e269267fb2c61e9f36c363e9a7c89 Mon Sep 17 00:00:00 2001 From: Loh Wei Jun Date: Wed, 13 Oct 2021 16:44:15 +0800 Subject: [PATCH 5/6] Refactor custom_staus_code to memoized the entire logic block --- lib/faraday_middleware/response/caching.rb | 6 ++++-- spec/unit/caching_spec.rb | 6 +++--- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/lib/faraday_middleware/response/caching.rb b/lib/faraday_middleware/response/caching.rb index d06b7f8..9efd37a 100644 --- a/lib/faraday_middleware/response/caching.rb +++ b/lib/faraday_middleware/response/caching.rb @@ -89,8 +89,10 @@ def full_key? end def custom_status_codes - @custom_status_codes ||= CACHEABLE_STATUS_CODES & Array(@options[:status_codes]).map(&:to_i) - @custom_status_codes.any? ? @custom_status_codes : CACHEABLE_STATUS_CODES + @custom_status_codes ||= begin + codes = CACHEABLE_STATUS_CODES & Array(@options[:status_codes]).map(&:to_i) + codes.any? ? codes : CACHEABLE_STATUS_CODES + end end def cache_on_complete(env) diff --git a/spec/unit/caching_spec.rb b/spec/unit/caching_spec.rb index fe384bb..31ec4b7 100644 --- a/spec/unit/caching_spec.rb +++ b/spec/unit/caching_spec.rb @@ -100,7 +100,7 @@ def fetch(key) end context ':cacheable_status_code' do - let(:options) { { cacheable_status_code: %w[404] } } + let(:options) { { status_codes: %w[404] } } it 'caches requests based on defined cacheable_status_code' do expect(get('/').body).to eq('request:1') @@ -110,7 +110,7 @@ def fetch(key) end context 'with invalid :cacheable_status_code status' do - let(:options) { { cacheable_status_code: %w[404,500] } } + let(:options) { { status_codes: %w[404,500] } } it 'caches requests based on valid defined cacheable_status_code' do expect(get('/not_found').body).to eq('request:1') @@ -121,7 +121,7 @@ def fetch(key) end context 'with no valid :cacheable_status_code status' do - let(:options) { { cacheable_status_code: %w[500] } } + let(:options) { { status_codes: %w[500] } } it 'caches requests based on default cacheable_status_code' do expect(get('/').body).to eq('request:1') expect(get('/broken').body).to eq('request:2') From 3060cfdab0087a784d20d66655ef0d46846f5cb4 Mon Sep 17 00:00:00 2001 From: Loh Wei Jun Date: Wed, 13 Oct 2021 16:47:40 +0800 Subject: [PATCH 6/6] Fix rubocop linting issues --- lib/faraday_middleware/response/caching.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/faraday_middleware/response/caching.rb b/lib/faraday_middleware/response/caching.rb index 9efd37a..aff33cf 100644 --- a/lib/faraday_middleware/response/caching.rb +++ b/lib/faraday_middleware/response/caching.rb @@ -90,9 +90,9 @@ def full_key? def custom_status_codes @custom_status_codes ||= begin - codes = CACHEABLE_STATUS_CODES & Array(@options[:status_codes]).map(&:to_i) - codes.any? ? codes : CACHEABLE_STATUS_CODES - end + codes = CACHEABLE_STATUS_CODES & Array(@options[:status_codes]).map(&:to_i) + codes.any? ? codes : CACHEABLE_STATUS_CODES + end end def cache_on_complete(env)