User-Agent Parser (#149)

* Test a rate limiter separately
* Parse UA string if given

config/config.exs

@@ -33,6 +33,7 @@ config :xperiments, :cors,
   origin: "*"
 
 config :xperiments, redis_url: "redis://localhost"
+config :xperiments, rate_limiter: Xperiments.Plug.RateLimit
 
 config :xperiments, :js_config,
   reporting_url: "https://analytics.google.com/analytics/web/?authuser=1#my-reports/5IyMQAn0Tcqdu2Va8V9BIg/a69714416w130256140p134086343/%3F_u.date00%3D20170227%26_u.date01%3D20170227%26_u.sampleOption%3Dmoreprecision%26_u.sampleSize%3D500000/"

config/test.exs

@@ -8,6 +8,7 @@ config :xperiments, XperimentsWeb.Endpoint,
 
 config :xperiments, Experiment,
   stat_threshold: 4
+config :xperiments, rate_limiter: Xperiments.Plug.RateLimitTest
 
 # Print only warnings and errors during test
 config :logger, level: :warn

lib/xperiments_web/controllers/assigner_controller.ex

@@ -3,12 +3,15 @@ defmodule XperimentsWeb.AssignerController do
   alias Xperiments.Repo
   alias Xperiments.Experiments.Experiment
 
-  plug Xperiments.Plug.RateLimit, max_requests: 5, interval_seconds: 60
+  plug Application.get_env(:xperiments, :rate_limiter), max_requests: 5, interval_seconds: 60
   plug :auth_request when action in [:example]
+  plug :reject_on_empty_segments when action in [:experiments]
+  plug :parse_ua when action in [:experiments]
 
   def experiments(conn, params) do
+    segments = merge_segments_with_parsed_ua(params["segments"], Map.get(conn.assigns, :parsed_ua))
     experiments = Xperiments.Assigner.Dispatcher.get_suitable_experiments(
-      params["segments"], params["assigned"])
+      segments, params["assigned"])
     render conn, "experiments.json", experiments: experiments
   end
 
@@ -36,4 +39,35 @@ defmodule XperimentsWeb.AssignerController do
     end
   end
 
+  #
+  # Private
+  #
+
+  defp reject_on_empty_segments(%{params: params} = conn, _opts) do
+    segments = Map.get(params, "segments")
+    if is_nil(segments) or map_size(segments) == 0 do
+      conn
+      |> put_status(:no_content)
+      |> halt()
+    else
+      conn
+    end
+  end
+
+  defp merge_segments_with_parsed_ua(segments, nil), do: segments
+  defp merge_segments_with_parsed_ua(segments, parsed_ua), do: Map.merge(segments, parsed_ua)
+
+  defp parse_ua(%{params: %{"user_agent" => user_agent}} = conn, _opts) do
+    parsed_ua = UAParser.parse(user_agent) |> prepare_parsed_ua()
+    assign(conn, :parsed_ua, parsed_ua)
+  end
+  defp parse_ua(conn, _opts), do: conn
+
+
+  defp prepare_parsed_ua(nil), do: %{}
+  defp prepare_parsed_ua(%UAParser.UA{family: nil}), do: %{}
+  defp prepare_parsed_ua(%UAParser.UA{family: browser,
+                                      os: %UAParser.OperatingSystem{family: os},
+                                      version: version}),
+    do: %{"platform" => os, "browser" => browser, "version" => to_string(version)}
 end

mix.exs

@@ -47,6 +47,7 @@ defmodule Xperiments.Mixfile do
      {:remote_ip, "~> 0.1.0"},
      {:hammer, "~> 0.1.0"},
      {:hammer_backend_redis, "~> 0.1.0"},
+     {:ua_parser, "~> 1.2"},
      {:recon, "~> 2.3.2"},
      {:ex_machina, "~> 1.0", only: :test},
      {:mock, "~> 0.2.0", only: :test}]

mix.lock

@@ -41,7 +41,9 @@
   "ssl_verify_fun": {:hex, :ssl_verify_fun, "1.1.1", "28a4d65b7f59893bc2c7de786dec1e1555bd742d336043fe644ae956c3497fbe", [:make, :rebar], []},
   "timex": {:hex, :timex, "3.1.24", "d198ae9783ac807721cca0c5535384ebdf99da4976be8cefb9665a9262a1e9e3", [:mix], [{:combine, "~> 0.7", [hex: :combine, repo: "hexpm", optional: false]}, {:gettext, "~> 0.10", [hex: :gettext, repo: "hexpm", optional: false]}, {:tzdata, "~> 0.1.8 or ~> 0.5", [hex: :tzdata, repo: "hexpm", optional: false]}], "hexpm"},
   "tzdata": {:hex, :tzdata, "0.5.12", "1c17b68692c6ba5b6ab15db3d64cc8baa0f182043d5ae9d4b6d35d70af76f67b", [:mix], [{:hackney, "~> 1.0", [hex: :hackney, repo: "hexpm", optional: false]}], "hexpm"},
+  "ua_parser": {:hex, :ua_parser, "1.3.0", "854aa42a517267332fc2bda11099f833070163926009c70c5a7f07ae5dd73072", [:mix], [{:yamerl, "~> 0.5", [hex: :yamerl, repo: "hexpm", optional: false]}], "hexpm"},
   "ueberauth": {:hex, :ueberauth, "0.4.0", "bc72d5e5a7bdcbfcf28a756e34630816edabc926303bdce7e171f7ac7ffa4f91", [:mix], [{:plug, "~> 1.2", [hex: :plug, optional: false]}]},
   "ueberauth_google": {:hex, :ueberauth_google, "0.6.0", "4a41dadf875067ed486a381fbd3a3738e1bc03c98435a4e3ea598f20f14381ea", [:mix], [{:oauth2, "~> 0.9", [hex: :oauth2, repo: "hexpm", optional: false]}, {:ueberauth, "~> 0.4", [hex: :ueberauth, repo: "hexpm", optional: false]}], "hexpm"},
   "unicode_util_compat": {:hex, :unicode_util_compat, "0.3.1", "a1f612a7b512638634a603c8f401892afbf99b8ce93a45041f8aaca99cadb85e", [], [], "hexpm"},
-  "uuid": {:hex, :uuid, "1.1.8", "e22fc04499de0de3ed1116b770c7737779f226ceefa0badb3592e64d5cfb4eb9", [:mix], [], "hexpm"}}
+  "uuid": {:hex, :uuid, "1.1.8", "e22fc04499de0de3ed1116b770c7737779f226ceefa0badb3592e64d5cfb4eb9", [:mix], [], "hexpm"},
+  "yamerl": {:hex, :yamerl, "0.5.0", "6ec55a5d830f6f0d65a4030f5c5db24b0e72b813dfbde32fea44b4951ed9417c", [:rebar3], [], "hexpm"}}

test/controllers/assigner_controller_test.exs

@@ -2,7 +2,6 @@ defmodule Xperiments.AssignerControllerTest do
   use XperimentsWeb.ConnCase, async: false
   import Mock
   alias Xperiments.Assigner.ExperimentSupervisor
-  use Hammer, backend: Hammer.Backend.Redis, only: [:delete_buckets]
 
   setup do
     for e_pid <- ExperimentSupervisor.experiment_pids() do
@@ -15,10 +14,6 @@ defmodule Xperiments.AssignerControllerTest do
       |> put_req_header("accept", "application/json")
       |> put_req_header("x-forwarded-for", "for=127.0.0.1")
 
-    on_exit fn ->
-      delete_buckets("127.0.0.1:assigner/application/test_app/experiments/events")
-    end
-
     [conn: conn, app: app]
   end
 
@@ -38,17 +33,12 @@ defmodule Xperiments.AssignerControllerTest do
 
   @api_path "/assigner/application/test_app"
 
-  test "/experiments returns assigner variants", context do
+  test "/experiments return variants based on given segments", context do
     body =
-      post(context.conn, @api_path <> "/experiments", %{})
+      post(context.conn, @api_path <> "/experiments", %{"segments" => %{"lang" => "ru", "system" => "osx"}})
       |> json_response(200)
     assert length(body["assign"]) == 3
     assert hd(body["assign"])["name"] # test that we return a name of a requested experiment
-    ids =
-      Xperiments.Repo.all(Xperiments.Experiments.Experiment)
-      |> Enum.map(& &1.id)
-    returned_ids = body["assign"] |> Enum.map(fn e -> e["id"] end)
-    assert Enum.sort(returned_ids) == Enum.sort(ids)
   end
 
   test "returning of a specific variant", context do
@@ -68,6 +58,31 @@ defmodule Xperiments.AssignerControllerTest do
     assert hd(body["assign"])["id"] == exp.id
   end
 
+  test "parsing USER_AGENT if given and add to segemts", context do
+    user_agent = "Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:47.0) Gecko/20100101 Firefox/47.0"
+    parsed_ua = %{"browser" => "Firefox", "platform" => "Windows 7", "version" => "47.0"}
+    segments = %{"lang" => "en"}
+    with_mock Xperiments.Assigner.Dispatcher, [get_suitable_experiments: fn (_segments, _assigned) -> [] end] do
+      post(context.conn, @api_path <> "/experiments", %{user_agent: user_agent, segments: segments})
+      assert called Xperiments.Assigner.Dispatcher.get_suitable_experiments(Map.merge(parsed_ua, segments), nil)
+    end
+  end
+
+  test "parsing of bad USER_AGENT string", context do
+    segments = %{"lang" => "zb"}
+    with_mock Xperiments.Assigner.Dispatcher, [get_suitable_experiments: fn (_segments, _assigned) -> [] end] do
+      post(context.conn, @api_path <> "/experiments", %{"user_agent" => "плохая строка", "segments" => segments})
+      assert called Xperiments.Assigner.Dispatcher.get_suitable_experiments(segments, nil)
+    end
+  end
+
+  test "rejecting requests with invalid params", context do
+    Enum.each [%{}, %{"segments" => nil}, %{"segments" => %{}}], fn params ->
+      response = post(context.conn, @api_path <> "/experiments", params)
+      assert response.status == 204
+    end
+  end
+
   describe "Impreassions" do
     setup context do
       exp = hd(Xperiments.Repo.all(Xperiments.Experiments.Experiment))
@@ -97,17 +112,6 @@ defmodule Xperiments.AssignerControllerTest do
       assert db_exp.statistics.common_impression == 4
       assert db_exp.statistics.variants_impression == %{hd(context.exp.variants).id => 4}
     end
-
-    test "requests are throttled", context do
-      for _i <- 0..4 do
-        post(context.conn, "#{@api_path}/experiments/events", %{event: "impression", payload: context.call_payload})
-        |> json_response(200)
-      end
-      body =
-        post(context.conn, "#{@api_path}/experiments/events", %{event: "impression", payload: context.call_payload})
-      |> json_response(403)
-      assert body == %{"error" => "Rate limit exceeded"}
-    end
   end
 
   describe "Rules/Segments logic" do

test/plug/rate_limit_test.exs

@@ -0,0 +1,20 @@
+defmodule Xperiments.RateLimitPlugTest do
+  use ExUnit.Case, async: true
+  import Plug.Test
+  alias Xperiments.Plug.RateLimit
+
+  @opts RateLimit.init([max_requests: 1, interval_seconds: 2])
+
+  test "throttles requests" do
+    conn = conn(:post, "/assigner/application/test_app/experiments/", %{"segments" => %{"lang" => "br"}})
+
+    conn = RateLimit.call(conn, @opts)
+    assert conn.state == :unset
+    assert conn.status == nil
+
+    # We reach a givin limit
+    conn = RateLimit.call(conn, @opts)
+    assert conn.state == :sent
+    assert conn.status == 403
+  end
+end

test/support/rate_limiter_test.ex

@@ -0,0 +1,4 @@
+defmodule Xperiments.Plug.RateLimitTest do
+  def init(_), do: :ok
+  def call(conn, _opts), do: conn
+end