diff --git a/CHANGELOG.md b/CHANGELOG.md index c55b7c1..e63b85a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,13 @@ +### 2.2.0 (2018-07-24) + +Features: + + - Let `Eeny::Meeny::Middleware` remove deprecated experiment cookies (for undefined and inactive experiements) + +Changes: + + - Renamed `EenyMeeny::Cookie::COOKIE_PREFIX` to `EenyMeeny::Cookie::EXPERIMENT_PREFIX` + ### 2.1.4 (2017-10-04) Changes: diff --git a/eeny-meeny.gemspec b/eeny-meeny.gemspec index 7ed7587..302f19e 100644 --- a/eeny-meeny.gemspec +++ b/eeny-meeny.gemspec @@ -3,7 +3,7 @@ require File.expand_path('../lib/eeny-meeny/version', __FILE__) Gem::Specification.new do |s| s.name = 'eeny-meeny' s.version = EenyMeeny::VERSION.dup - s.date = '2017-10-04' + s.date = '2018-07-24' s.summary = "A simple split and smoke testing tool for Rails" s.authors = ["Christian Orthmann"] s.email = 'christian.orthmann@gmail.com' diff --git a/lib/eeny-meeny/middleware.rb b/lib/eeny-meeny/middleware.rb index 14de816..9de241a 100644 --- a/lib/eeny-meeny/middleware.rb +++ b/lib/eeny-meeny/middleware.rb @@ -24,34 +24,15 @@ def call(env) query_parameters = query_hash(env) now = Time.zone.now new_cookies = {} + delete_cookies = find_deprecated_cookies(cookies, now) # Prepare for experiments. @experiments.each do |experiment| # Skip inactive experiments next unless experiment.active?(now) - # Trigger experiment through query parameters - cookie_name = EenyMeeny::Cookie.cookie_name(experiment) - has_experiment_trigger = EenyMeeny.config.query_parameters[:experiment] && query_parameters.key?(cookie_name) - # skip experiments that already have a cookie - next unless has_experiment_trigger || !cookies.key?(cookie_name) - cookie = if has_experiment_trigger - # Trigger experiment variation through query parameter. - EenyMeeny::Cookie.create_for_experiment_variation(experiment, query_parameters[cookie_name].to_sym, @cookie_config) - else - EenyMeeny::Cookie.create_for_experiment(experiment, @cookie_config) - end - # Set HTTP_COOKIE header to enable experiment on first pageview - env = add_or_replace_http_cookie(env, cookie) - new_cookies[cookie.name] = cookie - end - # Prepare smoke tests (if enabled through query parameters) - if EenyMeeny.config.query_parameters[:smoke_test] - if query_parameters.key?('smoke_test_id') && (query_parameters['smoke_test_id'] =~ /\A[A-Za-z_]+\z/) - # Set HTTP_COOKIE header to enable smoke test on first pageview - cookie = EenyMeeny::Cookie.create_for_smoke_test(query_parameters['smoke_test_id'], @cookie_config) - env = add_or_replace_http_cookie(env, cookie) - new_cookies[cookie.name] = cookie - end + env, new_cookies = prepare_experiment(env, cookies, new_cookies, query_parameters, experiment) end + # Prepare smoke tests + env, new_cookies = prepare_smoke_test(env, new_cookies, query_parameters) # Delegate to app status, headers, body = @app.call(env) response = Rack::Response.new(body, status, headers) @@ -59,11 +40,57 @@ def call(env) new_cookies.each do |key, value| response.set_cookie(key,value.to_h) end + delete_cookies.each do |key, value| + response.delete_cookie(key, value: value, path: @cookie_config[:path], same_site: @cookie_config[:same_site]) + end response.finish end private + def find_deprecated_cookies(cookies, now) + deprecated_cookies = {} + cookies.each do |cookie_name, value| + # Skip any cookie that does not have the 'eeny_meeny_' prefix + next unless cookie_name.to_s.start_with?(EenyMeeny::Cookie::EXPERIMENT_PREFIX) + # Mark cookies that does not match any existing active experiment as 'deprecated'. + experiment = EenyMeeny::Experiment.find_by_cookie_name(cookie_name) + next if experiment && experiment.active?(now) + deprecated_cookies[cookie_name] = value + end + deprecated_cookies + end + + def prepare_experiment(env, cookies, new_cookies, query_parameters, experiment) + # Trigger experiment through query parameters + cookie_name = EenyMeeny::Cookie.cookie_name(experiment) + has_experiment_trigger = EenyMeeny.config.query_parameters[:experiment] && query_parameters.key?(cookie_name) + # Skip experiments that already have a cookie + return env, new_cookies unless has_experiment_trigger || !cookies.key?(cookie_name) + cookie = if has_experiment_trigger + # Trigger experiment variation through query parameter. + EenyMeeny::Cookie.create_for_experiment_variation(experiment, query_parameters[cookie_name].to_sym, @cookie_config) + else + EenyMeeny::Cookie.create_for_experiment(experiment, @cookie_config) + end + # Set HTTP_COOKIE header to enable experiment on first pageview + env = add_or_replace_http_cookie(env, cookie) + new_cookies[cookie.name] = cookie + return env, new_cookies + end + + def prepare_smoke_test(env, new_cookies, query_parameters) + # Skip if the EenyMeeny 'smoke test' query parameters configuration is disabled. + return env, new_cookies unless EenyMeeny.config.query_parameters[:smoke_test] + # Skip if no valid 'smoke_test_id' query parameter present + return env, new_cookies unless query_parameters.key?('smoke_test_id') && (query_parameters['smoke_test_id'] =~ /\A[A-Za-z_]+\z/) + # Set HTTP_COOKIE header to enable smoke test on first pageview + cookie = EenyMeeny::Cookie.create_for_smoke_test(query_parameters['smoke_test_id'], @cookie_config) + env = add_or_replace_http_cookie(env, cookie) + new_cookies[cookie.name] = cookie + return env, new_cookies + end + def query_hash(env) # Query Params are only relevant if EenyMeeny.config have them enabled. return {} unless EenyMeeny.config.query_parameters[:experiment] || EenyMeeny.config.query_parameters[:smoke_test] diff --git a/lib/eeny-meeny/models/cookie.rb b/lib/eeny-meeny/models/cookie.rb index 2e10711..aafb3b7 100644 --- a/lib/eeny-meeny/models/cookie.rb +++ b/lib/eeny-meeny/models/cookie.rb @@ -2,7 +2,7 @@ module EenyMeeny class Cookie - COOKIE_PREFIX = 'eeny_meeny_'.freeze + EXPERIMENT_PREFIX = 'eeny_meeny_'.freeze SMOKE_TEST_PREFIX = 'smoke_test_'.freeze attr_accessor :value @@ -52,7 +52,7 @@ def self.smoke_test_name(smoke_test_id, version: 1) def self.cookie_name(experiment) return if experiment.nil? - COOKIE_PREFIX+experiment.id.to_s+'_v'+experiment.version.to_s + EXPERIMENT_PREFIX+experiment.id.to_s+'_v'+experiment.version.to_s end def self.read(cookie_string) diff --git a/lib/eeny-meeny/models/experiment.rb b/lib/eeny-meeny/models/experiment.rb index 3681444..a29f172 100644 --- a/lib/eeny-meeny/models/experiment.rb +++ b/lib/eeny-meeny/models/experiment.rb @@ -4,6 +4,9 @@ module EenyMeeny class Experiment + + COOKIE_EXPERIMENT_ID_REGEX = /\Aeeny_meeny_(.+)_v\d+\z/.freeze + attr_reader :id, :name, :version, :variations, :total_weight, :end_at, :start_at def self.find_all @@ -17,6 +20,12 @@ def self.find_by_id(experiment_id) new(experiment_id, **experiment) if experiment end + def self.find_by_cookie_name(cookie_name) + if cookie_name =~ COOKIE_EXPERIMENT_ID_REGEX + find_by_id($1) + end + end + def initialize(id, name: '', version: 1, variations: {}, start_at: nil, end_at: nil) @id = id @name = name diff --git a/lib/eeny-meeny/version.rb b/lib/eeny-meeny/version.rb index f6d5355..81e96c9 100644 --- a/lib/eeny-meeny/version.rb +++ b/lib/eeny-meeny/version.rb @@ -1,3 +1,3 @@ module EenyMeeny - VERSION = '2.1.4' + VERSION = '2.2.0' end diff --git a/spec/eeny-meeny/middleware_spec.rb b/spec/eeny-meeny/middleware_spec.rb index ae9fe1b..ef863a6 100644 --- a/spec/eeny-meeny/middleware_spec.rb +++ b/spec/eeny-meeny/middleware_spec.rb @@ -70,6 +70,22 @@ def initialize_app(secure: true, secret: 'test', path: '/', same_site: :strict) end end + context 'and the request contains a cookie from an undefined experiment' do + let(:request) { Rack::MockRequest.new(subject) } + + before(:example) do + @original_request_cookies = 'test=abc;eeny_meeny_undefined_experiment_v1=thevaluedoesntmatter;' + @response = request.get('/test', + 'CONTENT_TYPE' => 'text/plain', + 'HTTP_COOKIE' => @original_request_cookies) + end + + + it "instructs the browser to remove through the 'Set-Cookie' header on the response" do + expect(@response['Set-Cookie']).to include('eeny_meeny_undefined_experiment_v1=thevaluedoesntmatter; path=/; max-age=0; expires=Thu, 01 Jan 1970 00:00:00 -0000; SameSite=Strict') + end + end + context 'and given an experiment query parameter' do let(:request) { Rack::MockRequest.new(initialize_app(secure: false)) } diff --git a/spec/eeny-meeny/models/experiment_spec.rb b/spec/eeny-meeny/models/experiment_spec.rb index 5c2f92f..01cd73d 100644 --- a/spec/eeny-meeny/models/experiment_spec.rb +++ b/spec/eeny-meeny/models/experiment_spec.rb @@ -194,4 +194,17 @@ def experiment_with_time(time = {}) end end + describe '.find_by_cookie_name', experiments: true do + context 'when the given cookie name matches a configured experiment' do + it 'returns the experiment' do + expect(described_class.find_by_cookie_name(:eeny_meeny_my_page_v1)).to be_a EenyMeeny::Experiment + end + end + + context 'when the given cookie name does not match a configured experiment' do + it 'returns nil' do + expect(described_class.find_by_cookie_name(:eeny_meeny_undefined_experiment_v2)).to be_nil + end + end + end end