Merge pull request #4 from corthmann/patch-2.1.2

Patch 2.1.2

.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

.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"

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:

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       = '[email protected]'

lib/eeny-meeny/middleware.rb

@@ -8,53 +8,50 @@
 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
       @cookie_config = EenyMeeny.config.cookies
     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

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)

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

lib/eeny-meeny/version.rb

@@ -1,3 +1,3 @@
 module EenyMeeny
-  VERSION = '2.1.1'
+  VERSION = '2.1.2'
 end

lib/tasks/cookie.rake

@@ -16,15 +16,15 @@ 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)
       puts cookie
     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

spec/eeny-meeny/experiment_helper_spec.rb

@@ -32,21 +32,21 @@
 
           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
             end
 
             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

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

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

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')