diff --git a/.rspec b/.rspec new file mode 100644 index 0000000..c99d2e7 --- /dev/null +++ b/.rspec @@ -0,0 +1 @@ +--require spec_helper diff --git a/.rubocop.yml b/.rubocop.yml index 63cd12d..67ac226 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -1,10 +1,21 @@ -inherit_from: .rubocop_todo.yml - -Documentation: - Enabled: false +AllCops: + NewCops: enable + TargetRubyVersion: 2.6 Layout/BlockAlignment: EnforcedStyleAlignWith: start_of_block Layout/MultilineMethodCallIndentation: EnforcedStyle: indented_relative_to_receiver + +Metrics/BlockLength: + ExcludedMethods: + - context + - describe + +Naming/FileName: + Exclude: + - 'lib/grape-cache.rb' + +Style/FrozenStringLiteralComment: + SafeAutoCorrect: true diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml deleted file mode 100644 index 9d2eeaa..0000000 --- a/.rubocop_todo.yml +++ /dev/null @@ -1,30 +0,0 @@ -# This configuration was generated by -# `rubocop --auto-gen-config` -# on 2019-11-01 20:51:53 +0300 using RuboCop version 0.74.0. -# The point is for the user to remove these configuration records -# one by one as the offenses are removed from the code base. -# Note that changes in the inspected code, or installation of new -# versions of RuboCop, may require this file to be generated again. - -# Offense count: 1 -Metrics/AbcSize: - Max: 16 - -# Offense count: 1 -# Configuration parameters: CountComments, ExcludedMethods. -Metrics/MethodLength: - Max: 11 - -# Offense count: 1 -# Configuration parameters: ExpectMatchingDefinition, Regex, IgnoreExecutableScripts, AllowedAcronyms. -# AllowedAcronyms: CLI, DSL, ACL, API, ASCII, CPU, CSS, DNS, EOF, GUID, HTML, HTTP, HTTPS, ID, IP, JSON, LHS, QPS, RAM, RHS, RPC, SLA, SMTP, SQL, SSH, TCP, TLS, TTL, UDP, UI, UID, UUID, URI, URL, UTF8, VM, XML, XMPP, XSRF, XSS -Naming/FileName: - Exclude: - - 'lib/grape-cache.rb' - -# Offense count: 5 -# Cop supports --auto-correct. -# Configuration parameters: AutoCorrect, AllowHeredoc, AllowURI, URISchemes, IgnoreCopDirectives, IgnoredPatterns. -# URISchemes: http, https -Metrics/LineLength: - Max: 98 diff --git a/Gemfile b/Gemfile index 7f4f5e9..a330751 100644 --- a/Gemfile +++ b/Gemfile @@ -3,3 +3,14 @@ source 'https://rubygems.org' gemspec + +group :development, :test do + gem 'bundler' + gem 'pry', '~> 0.13.1' + gem 'rubocop', '~> 1.18.0' +end + +group :test do + gem 'rspec', '~> 3.10.0' + gem 'rack-test', '~> 1.1.0' +end diff --git a/grape-cache.gemspec b/grape-cache.gemspec index 1a11909..1bc88d7 100644 --- a/grape-cache.gemspec +++ b/grape-cache.gemspec @@ -20,15 +20,13 @@ Gem::Specification.new do |spec| spec.require_path = 'lib' spec.metadata = { - 'bug_tracker_uri' => 'https://github.com/netrusov/grape-cache/issues', - 'source_code_uri' => 'https://github.com/netrusov/grape-cache' + 'bug_tracker_uri' => 'https://github.com/netrusov/grape-cache/issues', + 'source_code_uri' => 'https://github.com/netrusov/grape-cache' } + spec.required_ruby_version = '>= 2.6.0' + spec.add_runtime_dependency 'activesupport' spec.add_runtime_dependency 'grape', '>= 1.2', '< 2' spec.add_runtime_dependency 'rack' - - spec.add_development_dependency 'bundler' - spec.add_development_dependency 'pry' - spec.add_development_dependency 'rubocop' end diff --git a/lib/grape/cache.rb b/lib/grape/cache.rb index f205cee..3e34e8a 100644 --- a/lib/grape/cache.rb +++ b/lib/grape/cache.rb @@ -1,44 +1,80 @@ # frozen_string_literal: true -require 'rack' require 'grape' +require 'rack' + +require 'active_support/cache' +require 'active_support/concern' +require 'active_support/notifications' require 'grape/cache/configurable' -require 'grape/cache/extensions/dsl' -require 'grape/cache/extensions/endpoint' -require 'grape/cache/helpers' +require 'grape/cache/dsl' require 'grape/cache/version' +require 'grape/cache/extensions/dsl' +require 'grape/cache/extensions/instance' +require 'grape/cache/extensions/middleware/formatter' + module Grape + # @nodoc module Cache include Grape::Cache::Configurable # @param key [String] cache key - # @return [Object] response object + # @return [String] serialized response object def self.read(key) - config - .backend - .read(key) - .then { |result| Grape::Json.load(result) if result } + config.backend.read(key) end # @param key [String] cache key - # @param response [Object] response object + # @param value [Object] response object # @param options [Object] (see ActiveSupport::Cache#write) # @return [Boolean] operation status - def self.store(key, response, options) - return false unless response + def self.write(key, value, options) + return false unless value + + options = options.except(:expires_in) if options[:expires_in]&.<=(0) - Grape::Json - .dump(response) - .then { |value| config.backend.write(key, value, options) } + config.backend.write(key, value, options) + rescue StandardError => e + ActiveSupport::Notifications.instrument('grape_cache.write_error', error: e) + false end - end -end -Grape::API::Instance.class_exec do - include Grape::Cache::Extensions::DSL - include Grape::Cache::Extensions::Endpoint + # @param env [Object] request env + # @param key [#to_s, Array<#to_s>] any object or array of objects that respond to `#to_s` + # @return [String] cache key + def self.expand_cache_key(env, key) + key = [ + env[Rack::REQUEST_METHOD], + env[Rack::PATH_INFO], + '-' + ] + Array(key).compact - helpers Grape::Cache::Helpers + ActiveSupport::Cache.expand_cache_key(key) + end + + # @param env [Object] request env + # @param key [String] cache key + # @param context [Grape::Cache::DSL] context object + # @yield block which will be called on cache miss + # @return [String, Object] returns cached response or result of executed block + def self.with_cached_response(env, key, context) + env['grape-cache'] = { key: key } + + if (value = Grape::Cache.read(key)) + env['grape-cache'][:hit] = true + env['grape-cache'][:value] = value + else + env['grape-cache'][:hit] = false + env['grape-cache'][:options] = context.slice(:expires_in, :race_condition_ttl).compact + + yield + end + end + end end + +Grape::API::Instance.include(Grape::Cache::Extensions::DSL) +Grape::API::Instance.include(Grape::Cache::Extensions::Instance) +Grape::Middleware::Formatter.prepend(Grape::Cache::Extensions::Middleware::Formatter) diff --git a/lib/grape/cache/configurable.rb b/lib/grape/cache/configurable.rb index 1a3c6cf..7028305 100644 --- a/lib/grape/cache/configurable.rb +++ b/lib/grape/cache/configurable.rb @@ -1,13 +1,12 @@ # frozen_string_literal: true -require 'active_support/cache' -require 'active_support/concern' - module Grape module Cache + # @nodoc module Configurable extend ActiveSupport::Concern + # @nodoc class Configuration attr_accessor :backend diff --git a/lib/grape/cache/dsl.rb b/lib/grape/cache/dsl.rb index e22adfb..bc10550 100644 --- a/lib/grape/cache/dsl.rb +++ b/lib/grape/cache/dsl.rb @@ -2,6 +2,7 @@ module Grape module Cache + # @nodoc class DSL # @private class CacheControl @@ -10,7 +11,7 @@ def initialize(options = {}) end def max_age - @options[:max_age] + @options[:max_age].to_i end def public? @@ -26,44 +27,48 @@ def no_cache? end def to_s - options = [] - - options << (public? ? 'public' : 'private') - options << (no_cache? ? 'no-cache' : ['max-age', max_age].join('=')) - options << 'must-revalidate' if must_revalidate? - - options.join(', ') + [ + (public? ? 'public' : 'private'), + (no_cache? ? 'no-cache' : "max-age=#{max_age}"), + ('must-revalidate' if must_revalidate?) + ].compact.join(', ') end end - delegate :[], :[]=, :fetch, to: :@storage + delegate :[], :slice, to: :@store def initialize - @storage = { + @store = { expires_in: 0, - cache_control: 'private, max-age=0, must-revalidate' + race_condition_ttl: 5, + cache_control: 'private, no-cache, must-revalidate' } end # @return [void] def key(value = nil, &block) - self[:key] = value || block + @store[:key] = value || block end # @param seconds [Integer] cache TTL # @return [void] def expires_in(seconds) - self[:expires_in] = seconds.to_i + @store[:expires_in] = seconds.to_i + end + + # @param seconds [Integer] cache race condition TTL + # @return [void] + def race_condition_ttl(seconds) + @store[:race_condition_ttl] = seconds.to_i end # @param options [Hash] parameters for "Cache-Control" header - # @option options :public [Boolean] defaults to "false" + # @option options :public [Boolean] # @option options :must_revalidate [Boolean] - # @option options :max_age [Integer] defaults to "expires_in" + # @option options :max_age [Integer] # @return [void] def cache_control(options = {}) - self[:cache_control] = - CacheControl.new(options.reverse_merge(max_age: self[:expires_in])).to_s + @store[:cache_control] = CacheControl.new(options).to_s end end end diff --git a/lib/grape/cache/extensions/dsl.rb b/lib/grape/cache/extensions/dsl.rb index a3ef973..2750485 100644 --- a/lib/grape/cache/extensions/dsl.rb +++ b/lib/grape/cache/extensions/dsl.rb @@ -1,12 +1,9 @@ # frozen_string_literal: true -require 'active_support/concern' - -require 'grape/cache/dsl' - module Grape module Cache module Extensions + # @nodoc module DSL extend ActiveSupport::Concern @@ -15,10 +12,9 @@ module DSL # # @return [void] def cache(&block) - Grape::Cache::DSL.new.then do |context| - context.instance_exec(&block) - route_setting :cache, context - end + context = Grape::Cache::DSL.new + context.instance_exec(&block) + route_setting(:cache, context) end end end diff --git a/lib/grape/cache/extensions/endpoint.rb b/lib/grape/cache/extensions/instance.rb similarity index 62% rename from lib/grape/cache/extensions/endpoint.rb rename to lib/grape/cache/extensions/instance.rb index f126c8d..4d292c1 100644 --- a/lib/grape/cache/extensions/endpoint.rb +++ b/lib/grape/cache/extensions/instance.rb @@ -3,7 +3,8 @@ module Grape module Cache module Extensions - module Endpoint + # @nodoc + module Instance extend ActiveSupport::Concern class_methods do @@ -15,14 +16,11 @@ def generate_cached_api_method(context, &block) proc do header 'Cache-Control', context[:cache_control] - cache_key = - context[:key] - .then { |key| key.is_a?(Proc) ? instance_exec(&key) : key } - .then { |key| expand_cache_key(*key) } + key = context[:key] + key = key.is_a?(Proc) ? instance_exec(&key) : key + key = Grape::Cache.expand_cache_key(env, key) - Grape::Cache.read(cache_key) || instance_exec(&block).tap do |response| - Grape::Cache.store cache_key, (body || response), expires_in: context[:expires_in] - end + Grape::Cache.with_cached_response(env, key, context) { instance_exec(&block) } end end diff --git a/lib/grape/cache/extensions/middleware/formatter.rb b/lib/grape/cache/extensions/middleware/formatter.rb new file mode 100644 index 0000000..c8457de --- /dev/null +++ b/lib/grape/cache/extensions/middleware/formatter.rb @@ -0,0 +1,27 @@ +# frozen_string_literal: true + +module Grape + module Cache + module Extensions + module Middleware + # @nodoc + module Formatter + def fetch_formatter(*) + formatter = super + + lambda do |body, env| + cache = env['grape-cache'] + + return formatter.call(body, env) unless cache + return cache[:value] if cache[:hit] + + formatter.call(body, env).tap do |response| + Grape::Cache.write(cache[:key], response, cache[:options]) + end + end + end + end + end + end + end +end diff --git a/lib/grape/cache/helpers.rb b/lib/grape/cache/helpers.rb deleted file mode 100644 index 074615a..0000000 --- a/lib/grape/cache/helpers.rb +++ /dev/null @@ -1,20 +0,0 @@ -# frozen_string_literal: true - -require 'active_support/cache' - -module Grape - module Cache - module Helpers - # @param *args [String, Integer] parameters which will be concatenated into cache key - # @return [String] cache key - def expand_cache_key(*args) - [ - env.fetch(Rack::REQUEST_METHOD), - env.fetch(Rack::PATH_INFO), - '-', - *args - ].compact.then(&ActiveSupport::Cache.method(:expand_cache_key)) - end - end - end -end diff --git a/lib/grape/cache/version.rb b/lib/grape/cache/version.rb index a986667..32a1f51 100644 --- a/lib/grape/cache/version.rb +++ b/lib/grape/cache/version.rb @@ -2,6 +2,6 @@ module Grape module Cache - VERSION = '0.2.1' + VERSION = '0.2.2' end end diff --git a/spec/grape/cache/dsl_spec.rb b/spec/grape/cache/dsl_spec.rb new file mode 100644 index 0000000..4187165 --- /dev/null +++ b/spec/grape/cache/dsl_spec.rb @@ -0,0 +1,102 @@ +# frozen_string_literal: true + +RSpec.describe Grape::Cache::DSL do + subject { Grape::Cache::DSL.new } + + describe '#key' do + let(:key) { %w[foo bar baz] } + + before do + subject.key key + end + + it 'simply stores any value passed' do + expect(subject[:key]).to eq(key) + end + end + + describe '#expires_in' do + let(:expires_in) { '60' } + + before do + subject.expires_in expires_in + end + + it 'converts any value to integer' do + expect(subject[:expires_in]).to eq(expires_in.to_i) + end + end + + describe '#race_condition_ttl' do + let(:ttl) { '60' } + + before do + subject.race_condition_ttl ttl + end + + it 'converts any value to integer' do + expect(subject[:race_condition_ttl]).to eq(ttl.to_i) + end + end + + describe '#cache_control' do + let(:default_value) { 'private, no-cache, must-revalidate' } + let(:max_age) { 1.minute } + + before do + subject.cache_control options + end + + context 'max-age' do + let(:options) { { max_age: max_age } } + + it 'sets cache TTL' do + expect(subject[:cache_control]).to eq("private, max-age=#{max_age.to_i}") + end + end + + context 'public' do + context 'when max-age is not set' do + let(:options) { { public: true } } + + it 'stays private' do + expect(subject[:cache_control]).to eq('private, no-cache, must-revalidate') + end + end + + context 'when max-age is set' do + let(:options) { { public: true, max_age: max_age } } + + it 'becomes public' do + expect(subject[:cache_control]).to eq("public, max-age=#{max_age.to_i}") + end + end + end + + context 'must-revalidate' do + context 'when max-age is not set' do + let(:options) { { must_revalidate: false } } + + it 'stays present' do + expect(subject[:cache_control]).to eq('private, no-cache, must-revalidate') + end + end + + context 'when max-age is set' do + let(:options) { { must_revalidate: false, max_age: max_age } } + + it 'becomes absent' do + expect(subject[:cache_control]).to eq("private, max-age=#{max_age.to_i}") + end + + context 'when must-revalidate is set to true' do + let(:options) { { must_revalidate: true, max_age: max_age } } + + it 'stays present' do + expect(subject[:cache_control]).to eq("private, max-age=#{max_age.to_i}, must-revalidate") + end + end + end + end + end +end diff --git a/spec/grape/cache_spec.rb b/spec/grape/cache_spec.rb new file mode 100644 index 0000000..9583719 --- /dev/null +++ b/spec/grape/cache_spec.rb @@ -0,0 +1,97 @@ +# frozen_string_literal: true + +RSpec.describe Grape::Cache do + let(:backend) { ActiveSupport::Cache::MemoryStore.new } + + before do + Grape::Cache.config.backend = backend + end + + after { backend.cleanup } + + describe '.write' do + let(:key) { 'key' } + let(:value) { 'hit' } + let(:options) { {} } + + subject { described_class.write(key, value, options) } + + after { subject } + + context 'when expires_in is <= 0' do + let(:options) { { expires_in: 0, race_condition_ttl: 5.seconds } } + + it 'removes it from the options' do + expect(backend).to receive(:write).with(key, value, { race_condition_ttl: 5 }) + end + end + end + + describe '.expand_cache_key' do + let(:app) { Class.new(Grape::API) } + let(:key) { %w[foo bar] } + let(:route) { '/users' } + + subject { described_class.expand_cache_key(last_request.env, key) } + + before do + app.get route do + 'response' + end + end + + it 'generates cache key in the following format: ()/()/-(/)*' do + get route + + is_expected.to eq("GET/#{route}/-/foo/bar") + end + end + + describe '.with_cached_response' do + let(:key) { 'key' } + let(:backend_key) { 'not-the-right-one' } + let(:hit_value) { 'hit' } + let(:miss_value) { 'miss' } + let(:env) { {} } + let(:options) { {} } + + subject do + described_class.with_cached_response(env, key, options) { miss_value } + end + + before do + backend.write(backend_key, hit_value) + subject + end + + context 'on cache miss' do + it 'executes block' do + is_expected.to eq(miss_value) + end + + it 'sets :hit to false' do + expect(env['grape-cache'][:hit]).to eq(false) + end + + it "doesn't set :value" do + expect(env['grape-cache'][:value]).to be_nil + end + end + + context 'on cache hit' do + let(:backend_key) { key } + + it 'returns cached value' do + is_expected.to eq(hit_value) + end + + it 'sets :hit to true' do + expect(env['grape-cache'][:hit]).to eq(true) + end + + it 'sets :value to cached value' do + expect(env['grape-cache'][:value]).to eq(hit_value) + end + end + end +end diff --git a/spec/grape/endpoint_spec.rb b/spec/grape/endpoint_spec.rb new file mode 100644 index 0000000..4483fd8 --- /dev/null +++ b/spec/grape/endpoint_spec.rb @@ -0,0 +1,105 @@ +# frozen_string_literal: true + +RSpec.describe Grape::Endpoint do + let(:app) { Class.new(Grape::API) } + let(:backend) { ActiveSupport::Cache::MemoryStore.new } + + before do + Grape::Cache.config.backend = backend + end + + after { backend.cleanup } + + context 'without cache' do + before do + app.get '/' do + rand + end + end + + it "always returns different results and doesn't touch the cache" do + expect(backend).not_to receive(:read) + expect(backend).not_to receive(:write) + + r1 = get('/') + r2 = get('/') + + expect(r1.body).not_to eq(r2.body) + end + end + + context 'with cache' do + before do + app.cache do + expires_in 1.minute + end + app.get '/' do + rand + end + end + + it 'always returns the result of the first run' do + expect(backend).to receive(:read).twice.and_call_original + expect(backend).to receive(:write).once.and_call_original + + r1 = get('/') + r2 = get('/') + + expect(r1.body).to eq(r2.body) + end + end + + context 'with different response serializers' do + let(:default_format) { :txt } + let(:helper) do + Module.new do + module_function + + def data + { + k1: :v1, + k2: :v2 + } + end + end + end + + before do + app.default_format default_format + app.helpers helper + app.cache do + expires_in 1.minute + end + app.get '/' do + data + end + end + + shared_examples 'serialized response' do + it 'must cache serialized response' do + expect(backend).to receive(:write) + .once + .with(kind_of(String), serialized_response, kind_of(Hash)) + .and_call_original + + 2.times { get '/' } + + expect(last_response.body).to eq(serialized_response) + end + end + + context 'JSON' do + let(:default_format) { :json } + let(:serialized_response) { helper.data.to_json } + + it_behaves_like 'serialized response' + end + + context 'XML' do + let(:default_format) { :xml } + let(:serialized_response) { helper.data.to_xml } + + it_behaves_like 'serialized response' + end + end +end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb new file mode 100644 index 0000000..ffe2635 --- /dev/null +++ b/spec/spec_helper.rb @@ -0,0 +1,12 @@ +# frozen_string_literal: true + +require 'grape-cache' + +require 'rubygems' +require 'bundler' + +Bundler.require(:default, :test) + +RSpec.configure do |config| + config.include Rack::Test::Methods +end