CookieAdapter: Fix cookies when @context is an ActionController (#526)

* CookieAdapter: Fix cookies when @context is an ActionController

Using ab_test from a controller was failing because "cookies" is a
private method of the controller instance which is assigned to @context.

It looks like this was broken in the commit that fixed the multiple
"Set-Cookie" headers bug:
3523e1c

Using `.send(:cookies)` again when using Rails was the easiest way I
found to fix this.

Note: I was able to add an integration test for the case when it's
called from the controller, but I failed to a regression test that uses
and ActionView context object.

* Don't use to_h on CookieJar, it doesn't work on older rubies

Author: mcls

lib/split/persistence/cookie_adapter.rb

@@ -34,7 +34,10 @@ def set_cookie(value = {})
         cookie_key = :split.to_s
         cookie_value = default_options.merge(value: JSON.generate(value))
         if action_dispatch?
-          @context.cookies[cookie_key] = cookie_value
+          # The "send" is necessary when we call ab_test from the controller
+          # and thus @context is a rails controller, because then "cookies" is
+          # a private method.
+          @context.send(:cookies)[cookie_key] = cookie_value
         else
           set_cookie_via_rack(cookie_key, cookie_value)
         end

spec/persistence/cookie_adapter_spec.rb

@@ -3,60 +3,104 @@
 require 'rack/test'
 
 describe Split::Persistence::CookieAdapter do
+  subject { described_class.new(context) }
 
-  let(:env) { Rack::MockRequest.env_for("http://example.com:8080/") }
-  let(:request) { Rack::Request.new(env) }
-  let(:response) { Rack::MockResponse.new(200, {}, "") }
-  let(:context) { double(request: request, response: response, cookies: CookiesMock.new) }
-  subject { Split::Persistence::CookieAdapter.new(context) }
+  shared_examples "sets cookies correctly" do
+    describe "#[] and #[]=" do
+      it "set and return the value for given key" do
+        subject["my_key"] = "my_value"
+        expect(subject["my_key"]).to eq("my_value")
+      end
 
-  describe "#[] and #[]=" do
-    it "should set and return the value for given key" do
-      subject["my_key"] = "my_value"
-      expect(subject["my_key"]).to eq("my_value")
+      it "handles invalid JSON" do
+        context.request.cookies[:split] = {
+          :value => '{"foo":2,',
+          :expires => Time.now
+        }
+        expect(subject["my_key"]).to be_nil
+        subject["my_key"] = "my_value"
+        expect(subject["my_key"]).to eq("my_value")
+      end
     end
-  end
 
-  describe "#delete" do
-    it "should delete the given key" do
-      subject["my_key"] = "my_value"
-      subject.delete("my_key")
-      expect(subject["my_key"]).to be_nil
+    describe "#delete" do
+      it "should delete the given key" do
+        subject["my_key"] = "my_value"
+        subject.delete("my_key")
+        expect(subject["my_key"]).to be_nil
+      end
     end
-  end
 
-  describe "#keys" do
-    it "should return an array of the session's stored keys" do
-      subject["my_key"] = "my_value"
-      subject["my_second_key"] = "my_second_value"
-      expect(subject.keys).to match(["my_key", "my_second_key"])
+    describe "#keys" do
+      it "should return an array of the session's stored keys" do
+        subject["my_key"] = "my_value"
+        subject["my_second_key"] = "my_second_value"
+        expect(subject.keys).to match(["my_key", "my_second_key"])
+      end
     end
   end
 
-  it "handles invalid JSON" do
-    context.request.cookies[:split] = { :value => '{"foo":2,', :expires => Time.now }
-    expect(subject["my_key"]).to be_nil
-    subject["my_key"] = "my_value"
-    expect(subject["my_key"]).to eq("my_value")
-  end
 
-  it "puts multiple experiments in a single cookie" do
-    subject["foo"] = "FOO"
-    subject["bar"] = "BAR"
-    expect(context.response.headers["Set-Cookie"]).to match(/\Asplit=%7B%22foo%22%3A%22FOO%22%2C%22bar%22%3A%22BAR%22%7D; path=\/; expires=[a-zA-Z]{3}, \d{2} [a-zA-Z]{3} \d{4} \d{2}:\d{2}:\d{2} -0000\Z/)
-  end
+  context "when using Rack" do
+    let(:env) { Rack::MockRequest.env_for("http://example.com:8080/") }
+    let(:request) { Rack::Request.new(env) }
+    let(:response) { Rack::MockResponse.new(200, {}, "") }
+    let(:context) { double(request: request, response: response, cookies: CookiesMock.new) }
 
-  it "ensure other added cookies are not overriden" do
-    context.response.set_cookie 'dummy', 'wow'
-    subject["foo"] = "FOO"
-    expect(context.response.headers["Set-Cookie"]).to include("dummy=wow")
-    expect(context.response.headers["Set-Cookie"]).to include("split=")
-  end
+    include_examples "sets cookies correctly"
 
-  it "uses ActionDispatch::Cookie when available for cookie writing" do
-    allow(subject).to receive(:action_dispatch?).and_return(true)
-    subject["foo"] = "FOO"
-    expect(subject['foo']).to eq('FOO')
+    it "puts multiple experiments in a single cookie" do
+      subject["foo"] = "FOO"
+      subject["bar"] = "BAR"
+      expect(context.response.headers["Set-Cookie"]).to match(/\Asplit=%7B%22foo%22%3A%22FOO%22%2C%22bar%22%3A%22BAR%22%7D; path=\/; expires=[a-zA-Z]{3}, \d{2} [a-zA-Z]{3} \d{4} \d{2}:\d{2}:\d{2} -0000\Z/)
+    end
+
+    it "ensure other added cookies are not overriden" do
+      context.response.set_cookie 'dummy', 'wow'
+      subject["foo"] = "FOO"
+      expect(context.response.headers["Set-Cookie"]).to include("dummy=wow")
+      expect(context.response.headers["Set-Cookie"]).to include("split=")
+    end
   end
 
+  context "when @context is an ActionController::Base" do
+    before :context do
+      require "rails"
+      require "action_controller/railtie"
+    end
+
+    let(:context) do
+      controller = controller_class.new
+      if controller.respond_to?(:set_request!)
+        controller.set_request!(ActionDispatch::Request.new({}))
+      else # Before rails 5.0
+        controller.send(:"request=", ActionDispatch::Request.new({}))
+      end
+
+      response = ActionDispatch::Response.new(200, {}, '').tap do |res|
+        res.request = controller.request
+      end
+
+      if controller.respond_to?(:set_response!)
+        controller.set_response!(response)
+      else # Before rails 5.0
+        controller.send(:set_response!, response)
+      end
+      controller
+    end
+
+    let(:controller_class) { Class.new(ActionController::Base) }
+
+    include_examples "sets cookies correctly"
+
+    it "puts multiple experiments in a single cookie" do
+      subject["foo"] = "FOO"
+      subject["bar"] = "BAR"
+      expect(subject.keys).to eq(["foo", "bar"])
+      expect(subject["foo"]).to eq("FOO")
+      expect(subject["bar"]).to eq("BAR")
+      cookie_jar = context.request.env["action_dispatch.cookies"]
+      expect(cookie_jar['split']).to eq("{\"foo\":\"FOO\",\"bar\":\"BAR\"}")
+    end
+  end
 end