From b82e271caa725f8e19d5dbb5d64b0410afeb290c Mon Sep 17 00:00:00 2001 From: Dmytro Zakharov Date: Mon, 22 Jan 2018 17:32:10 +0100 Subject: [PATCH] first implementation --- idempotent-request.gemspec | 42 +++---- lib/idempotent-request.rb | 5 + lib/idempotent-request/middleware.rb | 39 ++++++ lib/idempotent-request/redis_storage.rb | 29 +++++ lib/idempotent-request/request.rb | 33 ++++++ lib/idempotent-request/request_manager.rb | 56 +++++++++ lib/idempotent/request.rb | 7 -- lib/idempotent/request/version.rb | 5 - lib/version.rb | 3 + spec/idempotent-request/middleware_spec.rb | 59 +++++++++ spec/idempotent-request/redis_storage_spec.rb | 66 +++++++++++ .../request_manager_spec.rb | 112 ++++++++++++++++++ spec/idempotent-request/request_spec.rb | 62 ++++++++++ spec/idempotent/request_spec.rb | 11 -- spec/spec_helper.rb | 8 +- spec/support/helpers.rb | 9 ++ spec/support/memory_storage.rb | 19 +++ 17 files changed, 518 insertions(+), 47 deletions(-) create mode 100644 lib/idempotent-request.rb create mode 100644 lib/idempotent-request/middleware.rb create mode 100644 lib/idempotent-request/redis_storage.rb create mode 100644 lib/idempotent-request/request.rb create mode 100644 lib/idempotent-request/request_manager.rb delete mode 100644 lib/idempotent/request.rb delete mode 100644 lib/idempotent/request/version.rb create mode 100644 lib/version.rb create mode 100644 spec/idempotent-request/middleware_spec.rb create mode 100644 spec/idempotent-request/redis_storage_spec.rb create mode 100644 spec/idempotent-request/request_manager_spec.rb create mode 100644 spec/idempotent-request/request_spec.rb delete mode 100644 spec/idempotent/request_spec.rb create mode 100644 spec/support/helpers.rb create mode 100644 spec/support/memory_storage.rb diff --git a/idempotent-request.gemspec b/idempotent-request.gemspec index 896708f..98eda8c 100644 --- a/idempotent-request.gemspec +++ b/idempotent-request.gemspec @@ -1,36 +1,32 @@ # coding: utf-8 -lib = File.expand_path("../lib", __FILE__) +lib = File.expand_path('../lib', __FILE__) $LOAD_PATH.unshift(lib) unless $LOAD_PATH.include?(lib) -require "idempotent/request/version" +require 'version' Gem::Specification.new do |spec| - spec.name = "idempotent-request" - spec.version = Idempotent::Request::VERSION - spec.authors = ["Dmytro Zakharov"] - spec.email = ["dmytro@qonto.eu"] + spec.name = 'idempotent-request' + spec.version = IdempotentRequest::VERSION + spec.authors = ['Dmytro Zakharov'] + spec.email = ['dmytro@qonto.eu'] - spec.summary = %q{TODO: Write a short summary, because Rubygems requires one.} - spec.description = %q{TODO: Write a longer description or delete this line.} + spec.summary = %q{Write a short summary, because Rubygems requires one.} + spec.description = %q{Write a longer description or delete this line.} spec.homepage = "TODO: Put your gem's website or public repo URL here." - spec.license = "MIT" - - # Prevent pushing this gem to RubyGems.org. To allow pushes either set the 'allowed_push_host' - # to allow pushing to a single host or delete this section to allow pushing to any host. - if spec.respond_to?(:metadata) - spec.metadata["allowed_push_host"] = "TODO: Set to 'http://mygemserver.com'" - else - raise "RubyGems 2.0 or newer is required to protect against " \ - "public gem pushes." - end + spec.license = 'MIT' spec.files = `git ls-files -z`.split("\x0").reject do |f| f.match(%r{^(test|spec|features)/}) end - spec.bindir = "exe" + spec.bindir = 'exe' spec.executables = spec.files.grep(%r{^exe/}) { |f| File.basename(f) } - spec.require_paths = ["lib"] + spec.require_paths = ['lib'] - spec.add_development_dependency "bundler", "~> 1.15" - spec.add_development_dependency "rake", "~> 10.0" - spec.add_development_dependency "rspec", "~> 3.0" + spec.add_dependency 'rack', '~> 2.0' + spec.add_dependency 'oj', '~> 3.0' + + spec.add_development_dependency 'bundler', '~> 1.15' + spec.add_development_dependency 'rake', '~> 10.0' + spec.add_development_dependency 'rspec', '~> 3.0' + spec.add_development_dependency 'fakeredis', '~> 0.6' + spec.add_development_dependency 'pry', '~> 0.11' end diff --git a/lib/idempotent-request.rb b/lib/idempotent-request.rb new file mode 100644 index 0000000..2b5656e --- /dev/null +++ b/lib/idempotent-request.rb @@ -0,0 +1,5 @@ +require 'oj' +require 'idempotent-request/request' +require 'idempotent-request/request_manager' +require 'idempotent-request/redis_storage' +require 'idempotent-request/middleware' diff --git a/lib/idempotent-request/middleware.rb b/lib/idempotent-request/middleware.rb new file mode 100644 index 0000000..82da1a9 --- /dev/null +++ b/lib/idempotent-request/middleware.rb @@ -0,0 +1,39 @@ +module IdempotentRequest + class Middleware + def initialize(app, config = {}) + @app = app + @config = config + @decider = config[:decider] + end + + def call(env) + # dup the middleware to be thread-safe + dup.process(env) + end + + def process(env) + set_request(env) + return app.call(request.env) unless process? + storage = RequestManager.new(request, config) + storage.read || storage.write(*app.call(request.env)) + end + + private + + attr_reader :app, :env, :config, :request, :decider + + def process? + !request.key.to_s.empty? && should_be_idempotent? + end + + def should_be_idempotent? + return false unless decider + decider.new(request).should? + end + + def set_request(env) + @env = env + @request ||= Request.new(env, config) + end + end +end diff --git a/lib/idempotent-request/redis_storage.rb b/lib/idempotent-request/redis_storage.rb new file mode 100644 index 0000000..faa88ff --- /dev/null +++ b/lib/idempotent-request/redis_storage.rb @@ -0,0 +1,29 @@ +module IdempotentRequest + class RedisStorage + attr_reader :redis, :namespace, :expire_time + + def initialize(redis, config = {}) + @redis = redis + @namespace = config.fetch(:namespace, 'idempotency_keys') + @expire_time = config[:expire_time] + end + + def read(key) + redis.get(namespaced_key(key)) + end + + def write(key, payload) + redis.setnx(namespaced_key(key), payload) + redis.expire(namespaced_key(key), expire_time.to_i) if expire_time.to_i > 0 + end + + private + + def namespaced_key(idempotency_key) + [namespace, idempotency_key.strip] + .compact + .join(':') + .downcase + end + end +end diff --git a/lib/idempotent-request/request.rb b/lib/idempotent-request/request.rb new file mode 100644 index 0000000..f1c23c8 --- /dev/null +++ b/lib/idempotent-request/request.rb @@ -0,0 +1,33 @@ +module IdempotentRequest + class Request + attr_reader :request + + def initialize(env, config = {}) + @request = Rack::Request.new(env) + @header_name = config.fetch(:header_key, 'HTTP_IDEMPOTENCY_KEY') + end + + def key + request.env[header_name] + end + + def method_missing(method, *args) + if request.respond_to?(method) + request.send(method, *args) + else + super + end + end + + private + + def header_name + key = @header_name + .to_s + .upcase + .gsub('-', '_') + + key.start_with?('HTTP_') ? key : "HTTP_#{key}" + end + end +end diff --git a/lib/idempotent-request/request_manager.rb b/lib/idempotent-request/request_manager.rb new file mode 100644 index 0000000..02a4a86 --- /dev/null +++ b/lib/idempotent-request/request_manager.rb @@ -0,0 +1,56 @@ +module IdempotentRequest + class RequestManager + attr_reader :request, :storage + + def initialize(request, config) + @request = request + @storage = config.fetch(:storage) + @callback = config[:callback] + end + + def read + status, headers, response = parse_data(storage.read(key)).values + + return unless status + run_callback(:detected, key: request.key) + [status, headers, response] + end + + def write(*data) + status, headers, response = data + response = response.body if response.respond_to?(:body) + + return data unless status == 200 + + storage.write(key, payload(status, headers, response)) + + data + end + + private + + def parse_data(data) + return {} if data.to_s.empty? + + Oj.load(data) + end + + def payload(status, headers, response) + Oj.dump({ + status: status, + headers: headers.to_h, + response: response + }) + end + + def run_callback(action, args) + return unless @callback + + @callback.new(request).send(action, args) + end + + def key + request.key + end + end +end diff --git a/lib/idempotent/request.rb b/lib/idempotent/request.rb deleted file mode 100644 index 11201ee..0000000 --- a/lib/idempotent/request.rb +++ /dev/null @@ -1,7 +0,0 @@ -require "idempotent/request/version" - -module Idempotent - module Request - # Your code goes here... - end -end diff --git a/lib/idempotent/request/version.rb b/lib/idempotent/request/version.rb deleted file mode 100644 index 8d19777..0000000 --- a/lib/idempotent/request/version.rb +++ /dev/null @@ -1,5 +0,0 @@ -module Idempotent - module Request - VERSION = "0.1.0" - end -end diff --git a/lib/version.rb b/lib/version.rb new file mode 100644 index 0000000..d449309 --- /dev/null +++ b/lib/version.rb @@ -0,0 +1,3 @@ +module IdempotentRequest + VERSION = "0.1.0" +end diff --git a/spec/idempotent-request/middleware_spec.rb b/spec/idempotent-request/middleware_spec.rb new file mode 100644 index 0000000..9e340cf --- /dev/null +++ b/spec/idempotent-request/middleware_spec.rb @@ -0,0 +1,59 @@ +require 'spec_helper' + +RSpec.describe IdempotentRequest::Middleware do + let(:app) { -> (env) { [200, {}, 'body'] } } + let(:env) do + env_for('https://qonto.eu', method: 'POST') + .merge!( + 'HTTP_X_QONTO_IDEMPOTENCY_KEY' => 'dont-repeat-this-request-pls' + ) + end + let(:storage) { @memory_storage ||= IdempotentRequest::MemoryStorage.new } + let(:decider) do + class_double('IdempotentRequest::Decider', new: double(should?: true)) + end + + let(:middleware) do + described_class.new(app, + decider: decider, + storage: storage, + header_key: 'X-Qonto-Idempotency-Key' + ) + end + + context 'when should be idempotent' do + it 'should be saved to storage' do + expect_any_instance_of(IdempotentRequest::RequestManager).to receive(:read) + expect_any_instance_of(IdempotentRequest::RequestManager).to receive(:write) + + middleware.call(env) + end + + context 'when has data in storage' do + before do + data = [200, {}, 'body'] + allow_any_instance_of(IdempotentRequest::RequestManager).to receive(:read).and_return(data) + end + + it 'should read from storage' do + expect_any_instance_of(IdempotentRequest::RequestManager).to receive(:read) + expect_any_instance_of(IdempotentRequest::RequestManager).not_to receive(:write) + + middleware.call(env) + end + end + end + + context 'when should not be idempotent' do + let(:decider) do + class_double('IdempotentRequest::Decider', new: double(should?: false)) + end + + it 'should not read storage' do + expect_any_instance_of(IdempotentRequest::RequestManager).not_to receive(:read) + expect_any_instance_of(IdempotentRequest::RequestManager).not_to receive(:write) + + middleware.call(env) + end + end +end diff --git a/spec/idempotent-request/redis_storage_spec.rb b/spec/idempotent-request/redis_storage_spec.rb new file mode 100644 index 0000000..2a9a9db --- /dev/null +++ b/spec/idempotent-request/redis_storage_spec.rb @@ -0,0 +1,66 @@ +require 'spec_helper' + +RSpec.describe IdempotentRequest::RedisStorage do + let(:redis) { FakeRedis::Redis.new } + let(:expire_time) { 3600 } + let(:redis_storage) { described_class.new(redis, expire_time: expire_time) } + + describe '#read' do + it 'should be called' do + expect(redis).to receive(:get) + expect(redis_storage.read('key')).to be_nil + end + end + + describe '#write' do + let(:key) { 'key' } + let(:payload) { {} } + + context 'when expire time is not set' do + let(:redis_storage) { described_class.new(redis) } + + it 'should not set expiration' do + expect(redis).to receive(:setnx) + expect(redis).not_to receive(:expire) + redis_storage.write(key, payload) + end + end + + context 'when expire time is set' do + it 'should set expiration' do + expect(redis).to receive(:setnx) + expect(redis).to receive(:expire).with(String, expire_time) + redis_storage.write(key, payload) + end + end + end + + describe '#namespaced_key' do + subject { redis_storage.send(:namespaced_key, key) } + + context 'when key contains a space' do + let(:key) { ' REQUEST-1 ' } + + it 'should be stripped' do + is_expected.to eq('idempotency_keys:request-1') + end + end + + context 'when namespace is not set' do + let(:key) { 'REQUEST-1' } + + it 'should return with default' do + is_expected.to eq('idempotency_keys:request-1') + end + end + + context 'when namespace is set to nil' do + let(:redis_storage) { described_class.new(redis, namespace: nil) } + let(:key) { 'REQUEST-1' } + + it 'should return with default' do + is_expected.to eq('request-1') + end + end + end +end diff --git a/spec/idempotent-request/request_manager_spec.rb b/spec/idempotent-request/request_manager_spec.rb new file mode 100644 index 0000000..2a690f8 --- /dev/null +++ b/spec/idempotent-request/request_manager_spec.rb @@ -0,0 +1,112 @@ +require 'spec_helper' + +RSpec.describe IdempotentRequest::RequestManager do + let(:url) { 'http://qonto.eu' } + let(:default_env) { env_for(url) } + let(:env) { default_env } + let(:request) { IdempotentRequest::Request.new(env) } + let!(:memory_storage) { @memory_storage ||= IdempotentRequest::MemoryStorage.new } + let(:request_storage) { described_class.new(request, { storage: memory_storage }) } + + before do + allow(request).to receive(:key).and_return('data-key') + memory_storage.clear + end + + describe '#read' do + context 'when there is no data' do + it 'should return nil' do + expect(request_storage.read).to be_nil + end + end + + context 'when there is data' do + let(:data) do + [200, {}, 'body'] + end + + let(:payload) do + Oj.dump({ + status: data[0], + headers: data[1], + response: data[2] + }) + end + + before do + memory_storage.write(request.key, payload) + end + + it 'should return data' do + expect(request_storage.read).to eq(data) + end + + context 'when callback is defined' do + let(:request_storage) { described_class.new(request, storage: memory_storage, callback: IdempotencyCallback) } + + it 'should be called' do + callback = double + expect(IdempotencyCallback).to receive(:new).with(request).and_return(callback) + expect(callback).to receive(:detected).with(key: request.key) + expect(request_storage.read).to eq(data) + end + end + + context 'when read with different key' do + context 'for the old key' do + it 'should return data' do + expect(request_storage.read).to eq(data) + end + end + + context 'for the new key' do + before do + allow(request).to receive(:key).and_return('data-key-2') + end + + it 'should return nil' do + expect(request_storage.read).to be_nil + end + end + end + end + end + + describe '#write' do + let(:payload) do + Oj.dump({ + status: data[0], + headers: data[1], + response: data[2] + }) + end + + context 'when status is 200' do + let(:data) do + [200, {}, 'body'] + end + + it 'should be stored' do + request_storage.write(*data) + expect(memory_storage.read(request.key)).to eq(payload) + end + end + + context 'when status is not 200' do + let(:data) do + [404, {}, 'body'] + end + + it 'should be stored' do + request_storage.write(*data) + expect(memory_storage.read(request.key)).to be_nil + end + end + end + + class IdempotencyCallback + def initialize(_); end + + def detected(_); end + end +end diff --git a/spec/idempotent-request/request_spec.rb b/spec/idempotent-request/request_spec.rb new file mode 100644 index 0000000..f1f2345 --- /dev/null +++ b/spec/idempotent-request/request_spec.rb @@ -0,0 +1,62 @@ +require 'spec_helper' + +RSpec.describe IdempotentRequest::Request do + let(:url) { 'https://qonto.eu' } + let(:default_env) { env_for(url) } + let(:env) { default_env } + let(:request) { described_class.new(env) } + + describe '#key' do + context 'when is default' do + subject { request.key } + + context 'value is set' do + let(:env) do + default_env.merge!( + 'HTTP_IDEMPOTENCY_KEY' => 'test-key' + ) + end + + it 'should be present' do + is_expected.to eq('test-key') + end + end + + context 'value is not set' do + it 'should be nil' do + is_expected.to be_nil + end + end + end + + context 'when is custom' do + let(:request) { described_class.new(env, header_key: 'X-Qonto-Idempotency-Key') } + + subject { request.key } + + context 'value is set' do + let(:env) do + default_env.merge!( + 'HTTP_X_QONTO_IDEMPOTENCY_KEY' => 'custom-key' + ) + end + + it 'should be present' do + is_expected.to eq('custom-key') + end + end + + context 'value is not set' do + it 'should be nil' do + is_expected.to be_nil + end + end + end + end + + describe '#method_missing' do + it 'should forward to request' do + expect(request.request_method).to eq('GET') + end + end +end diff --git a/spec/idempotent/request_spec.rb b/spec/idempotent/request_spec.rb deleted file mode 100644 index 3471476..0000000 --- a/spec/idempotent/request_spec.rb +++ /dev/null @@ -1,11 +0,0 @@ -require "spec_helper" - -RSpec.describe Idempotent::Request do - it "has a version number" do - expect(Idempotent::Request::VERSION).not_to be nil - end - - it "does something useful" do - expect(false).to eq(true) - end -end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 88454fc..7a309df 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -1,7 +1,13 @@ require "bundler/setup" -require "idempotent/request" +require 'fakeredis' +require 'pry' +require "idempotent-request" + +spec = File.expand_path('../', __FILE__) +Dir[File.join(spec, 'support/**/*.rb')].each { |f| require f } RSpec.configure do |config| + config.include IdempotentRequest::Helpers # Enable flags like --only-failures and --next-failure config.example_status_persistence_file_path = ".rspec_status" diff --git a/spec/support/helpers.rb b/spec/support/helpers.rb new file mode 100644 index 0000000..aa3df9b --- /dev/null +++ b/spec/support/helpers.rb @@ -0,0 +1,9 @@ +require 'rack' + +module IdempotentRequest + module Helpers + def env_for(url, opts={}) + Rack::MockRequest.env_for(url, opts) + end + end +end diff --git a/spec/support/memory_storage.rb b/spec/support/memory_storage.rb new file mode 100644 index 0000000..814f19d --- /dev/null +++ b/spec/support/memory_storage.rb @@ -0,0 +1,19 @@ +module IdempotentRequest + class MemoryStorage + def initialize + @memory = {} + end + + def read(key) + @memory[key] + end + + def write(key, payload) + @memory[key] = payload + end + + def clear + @memory = {} + end + end +end