diff --git a/.gitignore b/.gitignore index 013076b..30128b2 100644 --- a/.gitignore +++ b/.gitignore @@ -12,10 +12,6 @@ capybara-*.html rerun.txt pickle-email-*.html -# TODO Comment out these rules if you are OK with secrets being uploaded to the repo -config/initializers/secret_token.rb -config/secrets.yml - ## Environment normalization: /.bundle /vendor/bundle @@ -34,7 +30,7 @@ bower.json # Ignore pow environment settings .powenv -# Ignore WebStorm ( JetBrains ) config +# Ignore RubyMine ( JetBrains ) config /.idea # Ignore Gem lock file diff --git a/.travis.yml b/.travis.yml index 9f26138..cbc8a47 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,22 +1,10 @@ language: ruby sudo: false -cache: bundler env: - "RAILS_VERSION=4.2.8" - - "RAILS_VERSION=5.0.0" - - "RAILS_VERSION=master" rvm: - 2.0.0 - 2.1.10 - 2.2.7 - 2.3.4 - 2.4.1 -matrix: - exclude: - - rvm: 2.0.0 - env: "RAILS_VERSION=5.0.0" - - rvm: 2.1.10 - env: "RAILS_VERSION=5.0.0" - allow_failures: - - env: "RAILS_VERSION=master" - - env: "RAILS_VERSION=5.0.0" diff --git a/CHANGELOG.md b/CHANGELOG.md index 4d585a8..07226f5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,14 @@ +### 2.1.2 (2017-04-14) + +Bugfixes: + + - Fix validation regex for the 'smoke_test_id' query parameter. + +Other Changes: + + - Clean up the way cookies are written to the HTTP_COOKIE header. + - Added Travis CI and build status badge. + ### 2.1.1 (2016-10-06) Bugfixes: diff --git a/eeny-meeny.gemspec b/eeny-meeny.gemspec index 7ef72d7..8cd229e 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 = '2016-10-06' + s.date = '2017-04-14' 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 b3563f9..165b709 100644 --- a/lib/eeny-meeny/middleware.rb +++ b/lib/eeny-meeny/middleware.rb @@ -8,6 +8,11 @@ module EenyMeeny class Middleware + # Headers + HTTP_COOKIE = 'HTTP_COOKIE'.freeze + REQUEST_METHOD = 'REQUEST_METHOD'.freeze + QUERY_STRING = 'QUERY_STRING'.freeze + def initialize(app) @app = app @experiments = EenyMeeny::Experiment.find_all @@ -15,46 +20,38 @@ def initialize(app) end def call(env) - request = Rack::Request.new(env) - cookies = request.cookies - now = Time.zone.now - new_cookies = {} - existing_set_cookie_header = env['Set-Cookie'] + cookies = Rack::Utils.parse_query(env[HTTP_COOKIE],';,') { |s| Rack::Utils.unescape(s) rescue s } + query_parameters = query_hash(env) + now = Time.zone.now + new_cookies = {} # Prepare for experiments. @experiments.each do |experiment| # Skip inactive experiments next unless experiment.active?(now) - # Trigger experiment through query parmeters + # Trigger experiment through query parameters cookie_name = EenyMeeny::Cookie.cookie_name(experiment) - has_experiment_trigger = EenyMeeny.config.query_parameters[:experiment] && request.params.has_key?(cookie_name) + has_experiment_trigger = EenyMeeny.config.query_parameters[:experiment] && query_parameters.key?(cookie_name) # skip experiments that already have a cookie - if has_experiment_trigger || !cookies.has_key?(cookie_name) - cookie = if has_experiment_trigger - # Trigger experiment variation through query parameter. - EenyMeeny::Cookie.create_for_experiment_variation(experiment, request.params[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_http_cookie(env, cookie, precede: has_experiment_trigger) - new_cookies[cookie.name] = cookie - end + 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 request.params.has_key?('smoke_test_id') && (request.params['smoke_test_id'] =~ /[A-Za-z_]+/) + 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(request.params['smoke_test_id']) - env = add_http_cookie(env, cookie, precede: true) + cookie = EenyMeeny::Cookie.create_for_smoke_test(query_parameters['smoke_test_id']) + env = add_or_replace_http_cookie(env, cookie) new_cookies[cookie.name] = cookie end end - # Clean up 'Set-Cookie' header. - if existing_set_cookie_header.nil? - env.delete('Set-Cookie') - else - env['Set-Cookie'] = existing_set_cookie_header - end # Delegate to app status, headers, body = @app.call(env) response = Rack::Response.new(body, status, headers) @@ -66,19 +63,22 @@ def call(env) end private - def add_http_cookie(env, cookie, precede: false) - env['Set-Cookie'] = '' - Rack::Utils.set_cookie_header!(env, - cookie.name, - cookie.to_h) - env['HTTP_COOKIE'] = '' if env['HTTP_COOKIE'].nil? - if precede - # Prepend cookie to the 'HTTP_COOKIE' header. This ensures it overwrites existing cookies when present. - env['HTTP_COOKIE'] = env['Set-Cookie'] + '; ' + env['HTTP_COOKIE'] - else - env['HTTP_COOKIE'] += '; ' unless env['HTTP_COOKIE'].empty? - env['HTTP_COOKIE'] += env['Set-Cookie'] - 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] + # Query Params are only relevant to HTTP GET requests. + return {} unless env[REQUEST_METHOD] == 'GET' + Rack::Utils.parse_query(env[QUERY_STRING], '&;') + end + + def add_or_replace_http_cookie(env, cookie) + cookie_name_escaped = Rack::Utils.escape(cookie.name) + cookie_string = "#{cookie_name_escaped}=#{Rack::Utils.escape(cookie.value)}" + env[HTTP_COOKIE] = '' if env[HTTP_COOKIE].nil? + return env if env[HTTP_COOKIE].sub!(/#{Regexp.escape(cookie_name_escaped)}=[^;]+/, cookie_string) + env[HTTP_COOKIE] += '; ' unless env[HTTP_COOKIE].empty? + env[HTTP_COOKIE] += cookie_string env end end diff --git a/lib/eeny-meeny/models/cookie.rb b/lib/eeny-meeny/models/cookie.rb index 342f7ad..2e10711 100644 --- a/lib/eeny-meeny/models/cookie.rb +++ b/lib/eeny-meeny/models/cookie.rb @@ -57,12 +57,8 @@ def self.cookie_name(experiment) def self.read(cookie_string) return if cookie_string.nil? || cookie_string.empty? - begin - return cookie_string unless EenyMeeny.config.secure # Cookie encryption disabled. - EenyMeeny.config.encryptor.decrypt(cookie_string) - rescue - nil # Return nil if cookie is invalid. - end + return cookie_string unless EenyMeeny.config.secure # Cookie encryption disabled. + EenyMeeny.config.encryptor.decrypt(cookie_string) end def initialize(name: '', value: '', expires: 1.month.from_now, http_only: true, same_site: nil, path: nil) diff --git a/lib/eeny-meeny/railtie.rb b/lib/eeny-meeny/railtie.rb index eef7d51..a3bf8f6 100644 --- a/lib/eeny-meeny/railtie.rb +++ b/lib/eeny-meeny/railtie.rb @@ -9,11 +9,11 @@ class Railtie < Rails::Railtie initializer 'eeny_meeny.configure' do |app| # Configrue EenyMeeny (defaults set in eeny_meeny.rb) EenyMeeny.configure do |config| - config.cookies = app.config.eeny_meeny[:cookies] if app.config.eeny_meeny.has_key?(:cookies) - config.experiments = app.config.eeny_meeny[:experiments] if app.config.eeny_meeny.has_key?(:experiments) - config.secret = app.config.eeny_meeny[:secret] if app.config.eeny_meeny.has_key?(:secret) - config.secure = app.config.eeny_meeny[:secure] if app.config.eeny_meeny.has_key?(:secure) - config.query_parameters = app.config.eeny_meeny[:query_parameters] if app.config.eeny_meeny.has_key?(:query_parameters) + config.cookies = app.config.eeny_meeny[:cookies] if app.config.eeny_meeny.key?(:cookies) + config.experiments = app.config.eeny_meeny[:experiments] if app.config.eeny_meeny.key?(:experiments) + config.secret = app.config.eeny_meeny[:secret] if app.config.eeny_meeny.key?(:secret) + config.secure = app.config.eeny_meeny[:secure] if app.config.eeny_meeny.key?(:secure) + config.query_parameters = app.config.eeny_meeny[:query_parameters] if app.config.eeny_meeny.key?(:query_parameters) end # Include Helpers in ActionController and ActionView ActionController::Base.send :include, EenyMeeny::ExperimentHelper diff --git a/lib/eeny-meeny/version.rb b/lib/eeny-meeny/version.rb index 8c1adf4..87d5369 100644 --- a/lib/eeny-meeny/version.rb +++ b/lib/eeny-meeny/version.rb @@ -1,3 +1,3 @@ module EenyMeeny - VERSION = '2.1.1' + VERSION = '2.1.2' end diff --git a/lib/tasks/cookie.rake b/lib/tasks/cookie.rake index b7a6bed..a7bc27b 100644 --- a/lib/tasks/cookie.rake +++ b/lib/tasks/cookie.rake @@ -16,7 +16,7 @@ namespace :eeny_meeny do namespace :cookie do desc 'Create a valid EenyMeeny experiment cookie' - task :experiment, [:experiment_id] => :environment do |t, args| + task :experiment, [:experiment_id] => :environment do |_, args| raise "Missing 'experiment_id' parameter" if (args['experiment_id'].nil? || args['experiment_id'].empty?) experiment_id = args['experiment_id'].to_sym cookie = write_cookie(experiment_id) @@ -24,7 +24,7 @@ namespace :eeny_meeny do end desc 'Create a valid EenyMeeny experiment cookie for a specific variation' - task :experiment_variation, [:experiment_id, :variation_id] => :environment do |t, args| + task :experiment_variation, [:experiment_id, :variation_id] => :environment do |_, args| raise "Missing 'experiment_id' parameter" if (args['experiment_id'].nil? || args['experiment_id'].empty?) raise "Missing 'variation_id' parameter" if (args['variation_id'].nil? || args['variation_id'].empty?) experiment_id = args['experiment_id'].to_sym @@ -34,7 +34,7 @@ namespace :eeny_meeny do end desc 'Create a valid EenyMeeny smoke test cookie' - task :smoke_test, [:smoke_test_id, :version] => :environment do |t, args| + task :smoke_test, [:smoke_test_id, :version] => :environment do |_, args| raise "Missing 'smoke_test_id' parameter" if (args['smoke_test_id'].nil? || args['smoke_test_id'].empty?) smoke_test_id = args['smoke_test_id'] version = args['version'] || 1 diff --git a/spec/eeny-meeny/experiment_helper_spec.rb b/spec/eeny-meeny/experiment_helper_spec.rb index 88db624..e2155f0 100644 --- a/spec/eeny-meeny/experiment_helper_spec.rb +++ b/spec/eeny-meeny/experiment_helper_spec.rb @@ -32,13 +32,13 @@ context 'and given a variation id' do let(:request_with_variation_cookie) do - request.set_cookie(EenyMeeny::Cookie.create_for_experiment_variation(EenyMeeny::Experiment.find_by_id(:my_page), variation_id: :new).to_s) + request.set_cookie(EenyMeeny::Cookie.create_for_experiment_variation(EenyMeeny::Experiment.find_by_id(:my_page), :new).to_s) request end context 'that matches the variation id the cookie' do it "returns the user's experiment variation" do - allow(subject).to receive(:cookies).and_return(request_with_cookie.cookie_jar) + allow(subject).to receive(:cookies).and_return(request_with_variation_cookie.cookie_jar) expect(subject.participates_in?(:my_page, variation_id: :new)).to be_a EenyMeeny::Variation expect(subject.participates_in?(:my_page, variation_id: 'new')).to be_a EenyMeeny::Variation end @@ -46,7 +46,7 @@ context 'that does not match the variation id the cookie' do it 'returns nil' do - allow(subject).to receive(:cookies).and_return(request_with_cookie.cookie_jar) + allow(subject).to receive(:cookies).and_return(request_with_variation_cookie.cookie_jar) expect(subject.participates_in?(:my_page, variation_id: :old)).to be_nil expect(subject.participates_in?(:my_page, variation_id: 'old')).to be_nil end diff --git a/spec/eeny-meeny/middleware_spec.rb b/spec/eeny-meeny/middleware_spec.rb index f9fd9ba..ae9fe1b 100644 --- a/spec/eeny-meeny/middleware_spec.rb +++ b/spec/eeny-meeny/middleware_spec.rb @@ -71,15 +71,15 @@ def initialize_app(secure: true, secret: 'test', path: '/', same_site: :strict) end context 'and given an experiment query parameter' do - let(:request) { Rack::MockRequest.new(subject) } + let(:request) { Rack::MockRequest.new(initialize_app(secure: false)) } before(:example) do @response = request.get('/test?eeny_meeny_my_page_v1=old', 'CONTENT_TYPE' => 'text/plain') end it 'selects the correct variation' do - modified_request = Rack::Request.new(app) - expect(EenyMeeny::Cookie.read(modified_request.cookies['eeny_meeny_my_page_v1'])).to eq('old') + expect(app['HTTP_COOKIE']).to include('eeny_meeny_my_page_v1=old') + expect(app['HTTP_COOKIE']).to_not include('eeny_meeny_my_page_v1=new') end it "sets the 'HTTP_COOKIE' header on the request" do diff --git a/spec/mock_rack_app.rb b/spec/mock_rack_app.rb index 78c81ab..8b40dd3 100644 --- a/spec/mock_rack_app.rb +++ b/spec/mock_rack_app.rb @@ -15,6 +15,7 @@ def call(env) def [](key) @env[key] end + alias_method :fetch, :[] def []=(key,value) @env[key] = value diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index ea23f1f..9e78562 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -4,11 +4,10 @@ require 'active_support/time' SimpleCov.start do - formatter SimpleCov::Formatter::MultiFormatter[ + formatter SimpleCov::Formatter::MultiFormatter.new([ SimpleCov::Formatter::HTMLFormatter, SimpleCov::Formatter::RcovFormatter, - CodeClimate::TestReporter::Formatter - ] + CodeClimate::TestReporter::Formatter]) add_group('EenyMeeny', 'lib/eeny-meeny') add_group('Rake Tasks', 'lib/tasks') add_group('Specs', 'spec')