fix(CMSRepo): Support cache key generation for map params (#2001)

lib/cms/api/http_client.ex

@@ -43,34 +43,34 @@ defmodule CMS.API.HTTPClient do
   # Note: when redirecting from CMS, nested params will
   # be shaped as a Map.t() with String.t() keys and values.
   @type safe_key :: :value | :min | :max | String.t()
-  @safe_keys [:value, :min, :max, "lattitude", "longitude", "type"]
+  @safe_keys [:value, :min, :max, "latitude", "longitude", "type"]
 
   @spec stringify_params({param_key, param_value}, param_list) :: param_list
-  defp stringify_params({key, val}, acc) when is_atom(key) do
+  def stringify_params({key, val}, acc) when is_atom(key) do
     stringify_params({Atom.to_string(key), val}, acc)
   end
 
-  defp stringify_params({key, val}, acc) when is_atom(val) do
+  def stringify_params({key, val}, acc) when is_atom(val) do
     stringify_params({key, Atom.to_string(val)}, acc)
   end
 
-  defp stringify_params({key, val}, acc) when is_integer(val) do
+  def stringify_params({key, val}, acc) when is_integer(val) do
     stringify_params({key, Integer.to_string(val)}, acc)
   end
 
-  defp stringify_params({key, val}, acc) when is_binary(key) and is_binary(val) do
+  def stringify_params({key, val}, acc) when is_binary(key) and is_binary(val) do
     [{key, val} | acc]
   end
 
-  defp stringify_params({key, val}, acc) when is_binary(key) and (is_map(val) or is_list(val)) do
+  def stringify_params({key, val}, acc) when is_binary(key) and (is_map(val) or is_list(val)) do
     val
     # drop original param, add new key/vals for nested params
     |> Enum.reduce(acc, fn nested_param, acc -> list_to_params(key, acc, nested_param) end)
     # restore original order of nested params
     |> Enum.reverse()
   end
 
-  defp stringify_params(_, acc) do
+  def stringify_params(_, acc) do
     # drop invalid param
     acc
   end

lib/cms/repo.ex

@@ -234,15 +234,24 @@ defmodule CMS.Repo do
   end
 
   defp params_to_string(params) when params == %{}, do: ""
+  defp params_to_string(params) when is_binary(params), do: params
 
   defp params_to_string(params) when is_map(params) do
-    [head | tail] = Enum.map(params, fn {k, v} -> "#{k}=#{v}" end)
+    case params
+         |> Enum.reduce([], &CMS.API.HTTPClient.stringify_params/2)
+         |> Enum.map(fn {k, v} -> "#{k}=#{v}" end) do
+      [head | tail] ->
+        ["?#{head}", "#{Enum.join(tail, "&")}"]
+        |> Enum.reject(&(&1 == ""))
+        |> Enum.join("&")
 
-    ["?#{head}", "#{Enum.join(tail, "&")}"]
-    |> Enum.reject(&(&1 == ""))
-    |> Enum.join("&")
+      _ ->
+        ""
+    end
   end
 
+  defp params_to_string(_), do: ""
+
   @spec view_or_preview(String.t(), map) :: {:ok, map} | {:error, API.error()}
   defp view_or_preview(path, %{"preview" => _, "vid" => "latest"} = params) do
     # "preview" value is deprecated. Use empty string or nil to get latest revision.

test/cms/api/http_client_test.exs

@@ -176,7 +176,7 @@ defmodule CMS.API.HTTPClientTest do
         view(
           "/redirect",
           %{
-            "location" => %{"lattitude" => "1234", "longitude" => "5678"},
+            "location" => %{"latitude" => "1234", "longitude" => "5678"},
             "foo" => %{"bad_sub_key" => "bar"},
             "fiz" => "baz"
           }
@@ -190,7 +190,7 @@ defmodule CMS.API.HTTPClientTest do
                    params: [
                      {"_format", "json"},
                      {"fiz", "baz"},
-                     {"location[lattitude]", "1234"},
+                     {"location[latitude]", "1234"},
                      {"location[longitude]", "5678"}
                    ]
                  )

test/cms/repo_test.exs

@@ -68,7 +68,7 @@ defmodule CMS.RepoTest do
     end
   end
 
-  describe "get_page/1" do
+  describe "generate/3" do
     test "generates the correct key for /*" do
       path = "/foo"
 
@@ -94,9 +94,27 @@ defmodule CMS.RepoTest do
       params = %{"bam" => "bop", "baz" => "qux"}
 
       assert Repo.generate(nil, nil, [path, params]) ==
-               "cms.repo" <> String.replace(path, "/", "|") <> "?bam=bop&baz=qux"
+               "cms.repo" <> String.replace(path, "/", "|") <> "?baz=qux&bam=bop"
     end
 
+    test "ignores nested maps if it is an unsupported key" do
+      path = "/foo/bar"
+      params = %{"data" => %{"some" => "map"}}
+
+      assert Repo.generate(nil, nil, [path, params]) ==
+               "cms.repo" <> String.replace(path, "/", "|")
+    end
+
+    test "adds nested maps if it is a supported key" do
+      path = "/foo/bar"
+      params = %{"biz" => "bang", "data" => %{"latitude" => "123"}}
+
+      assert Repo.generate(nil, nil, [path, params]) ==
+               "cms.repo" <> String.replace(path, "/", "|") <> "?biz=bang&data[latitude]=123"
+    end
+  end
+
+  describe "get_page/1" do
     test "caches views", %{cache: cache} do
       path = "/news/2018/news-entry"
       params = %{}