From bd3087d121eb2de8517e1477443d602c1d7d3aa6 Mon Sep 17 00:00:00 2001
From: setop <setop@zoocoop.com>
Date: Sun, 30 Aug 2020 23:29:56 +0200
Subject: [PATCH] Fix geospatial clients

add json plus to base client and rename to geospacial client
geospatial http client with json plug
---
 config/test.exs                               |  3 +-
 lib/service/geospatial/addok.ex               | 41 ++++++++++---------
 lib/service/geospatial/google_maps.ex         |  8 ++--
 lib/service/geospatial/map_quest.ex           |  6 +--
 lib/service/geospatial/mimirsbrunn.ex         | 40 +++++++++---------
 lib/service/geospatial/nominatim.ex           | 40 +++++++++---------
 lib/service/geospatial/pelias.ex              | 40 +++++++++---------
 lib/service/geospatial/photon.ex              | 40 +++++++++---------
 .../{base_client.ex => geospatial_client.ex}  |  5 ++-
 test/service/geospatial/addok_test.exs        |  2 +-
 test/service/geospatial/google_maps_test.exs  |  2 +-
 test/service/geospatial/map_quest_test.exs    |  2 +-
 test/service/geospatial/nominatim_test.exs    |  2 +-
 test/service/geospatial/photon_test.exs       |  2 +-
 test/support/data_case.ex                     |  2 +-
 15 files changed, 124 insertions(+), 111 deletions(-)
 rename lib/service/http/{base_client.ex => geospatial_client.ex} (78%)

diff --git a/config/test.exs b/config/test.exs
index 6fb0e2cf1..794d4773d 100644
--- a/config/test.exs
+++ b/config/test.exs
@@ -47,7 +47,8 @@ config :exvcr,
 config :tesla, Mobilizon.Service.HTTP.ActivityPub,
   adapter: Mobilizon.Service.HTTP.ActivityPub.Mock
 
-config :tesla, Mobilizon.Service.HTTP.BaseClient, adapter: Mobilizon.Service.HTTP.BaseClient.Mock
+config :tesla, Mobilizon.Service.HTTP.GeospatialClient,
+  adapter: Mobilizon.Service.HTTP.GeospatialClient.Mock
 
 config :mobilizon, Mobilizon.Service.Geospatial, service: Mobilizon.Service.Geospatial.Mock
 
diff --git a/lib/service/geospatial/addok.ex b/lib/service/geospatial/addok.ex
index c31b68bcd..95d38e4ae 100644
--- a/lib/service/geospatial/addok.ex
+++ b/lib/service/geospatial/addok.ex
@@ -5,7 +5,7 @@ defmodule Mobilizon.Service.Geospatial.Addok do
 
   alias Mobilizon.Addresses.Address
   alias Mobilizon.Service.Geospatial.Provider
-  alias Mobilizon.Service.HTTP.BaseClient
+  alias Mobilizon.Service.HTTP.GeospatialClient
 
   require Logger
 
@@ -21,16 +21,9 @@ defmodule Mobilizon.Service.Geospatial.Addok do
   """
   @spec geocode(String.t(), keyword()) :: list(Address.t())
   def geocode(lon, lat, options \\ []) do
-    url = build_url(:geocode, %{lon: lon, lat: lat}, options)
-
-    Logger.debug("Asking addok for addresses with #{url}")
-
-    with {:ok, %{status: 200, body: body}} <- BaseClient.get(url),
-         %{"features" => features} <- body do
-      process_data(features)
-    else
-      _ -> []
-    end
+    :geocode
+    |> build_url(%{lon: lon, lat: lat}, options)
+    |> fetch_features
   end
 
   @impl Provider
@@ -39,15 +32,9 @@ defmodule Mobilizon.Service.Geospatial.Addok do
   """
   @spec search(String.t(), keyword()) :: list(Address.t())
   def search(q, options \\ []) do
-    url = build_url(:search, %{q: q}, options)
-    Logger.debug("Asking addok for addresses with #{url}")
-
-    with {:ok, %{status: 200, body: body}} <- BaseClient.get(url),
-         %{"features" => features} <- body do
-      process_data(features)
-    else
-      _ -> []
-    end
+    :search
+    |> build_url(%{q: q}, options)
+    |> fetch_features
   end
 
   @spec build_url(atom(), map(), list()) :: String.t()
@@ -66,6 +53,20 @@ defmodule Mobilizon.Service.Geospatial.Addok do
     end
   end
 
+  @spec fetch_features(String.t()) :: list(Address.t())
+  defp fetch_features(url) do
+    Logger.debug("Asking addok with #{url}")
+
+    with {:ok, %{status: 200, body: body}} <- GeospatialClient.get(url),
+         %{"features" => features} <- body do
+      process_data(features)
+    else
+      _ ->
+        Logger.error("Asking addok with #{url}")
+        []
+    end
+  end
+
   defp process_data(features) do
     features
     |> Enum.map(fn %{"geometry" => geometry, "properties" => properties} ->
diff --git a/lib/service/geospatial/google_maps.ex b/lib/service/geospatial/google_maps.ex
index 4377a07f1..e5c6bc5e4 100644
--- a/lib/service/geospatial/google_maps.ex
+++ b/lib/service/geospatial/google_maps.ex
@@ -7,7 +7,7 @@ defmodule Mobilizon.Service.Geospatial.GoogleMaps do
 
   alias Mobilizon.Addresses.Address
   alias Mobilizon.Service.Geospatial.Provider
-  alias Mobilizon.Service.HTTP.BaseClient
+  alias Mobilizon.Service.HTTP.GeospatialClient
 
   require Logger
 
@@ -39,7 +39,7 @@ defmodule Mobilizon.Service.Geospatial.GoogleMaps do
 
     Logger.debug("Asking Google Maps for reverse geocode with #{url}")
 
-    with {:ok, %{status: 200, body: body}} <- BaseClient.get(url),
+    with {:ok, %{status: 200, body: body}} <- GeospatialClient.get(url),
          %{"results" => results, "status" => "OK"} <- body do
       Enum.map(results, fn entry -> process_data(entry, options) end)
     else
@@ -58,7 +58,7 @@ defmodule Mobilizon.Service.Geospatial.GoogleMaps do
 
     Logger.debug("Asking Google Maps for addresses with #{url}")
 
-    with {:ok, %{status: 200, body: body}} <- BaseClient.get(url),
+    with {:ok, %{status: 200, body: body}} <- GeospatialClient.get(url),
          %{"results" => results, "status" => "OK"} <- body do
       results |> Enum.map(fn entry -> process_data(entry, options) end)
     else
@@ -159,7 +159,7 @@ defmodule Mobilizon.Service.Geospatial.GoogleMaps do
 
     Logger.debug("Asking Google Maps for details with #{url}")
 
-    with {:ok, %{status: 200, body: body}} <- BaseClient.get(url),
+    with {:ok, %{status: 200, body: body}} <- GeospatialClient.get(url),
          %{"result" => %{"name" => name}, "status" => "OK"} <- body do
       name
     else
diff --git a/lib/service/geospatial/map_quest.ex b/lib/service/geospatial/map_quest.ex
index 2e2846658..249dae21e 100644
--- a/lib/service/geospatial/map_quest.ex
+++ b/lib/service/geospatial/map_quest.ex
@@ -11,7 +11,7 @@ defmodule Mobilizon.Service.Geospatial.MapQuest do
 
   alias Mobilizon.Addresses.Address
   alias Mobilizon.Service.Geospatial.Provider
-  alias Mobilizon.Service.HTTP.BaseClient
+  alias Mobilizon.Service.HTTP.GeospatialClient
 
   require Logger
 
@@ -36,7 +36,7 @@ defmodule Mobilizon.Service.Geospatial.MapQuest do
     if is_nil(api_key), do: raise(ArgumentError, message: @api_key_missing_message)
 
     with {:ok, %{status: 200, body: body}} <-
-           BaseClient.get(
+           GeospatialClient.get(
              "https://#{prefix}.mapquestapi.com/geocoding/v1/reverse?key=#{api_key}&location=#{
                lat
              },#{lon}&maxResults=#{limit}"
@@ -71,7 +71,7 @@ defmodule Mobilizon.Service.Geospatial.MapQuest do
 
     Logger.debug("Asking MapQuest for addresses with #{url}")
 
-    with {:ok, %{status: 200, body: body}} <- BaseClient.get(url),
+    with {:ok, %{status: 200, body: body}} <- GeospatialClient.get(url),
          %{"results" => results, "info" => %{"statuscode" => 0}} <- body do
       results |> Enum.map(&process_data/1)
     else
diff --git a/lib/service/geospatial/mimirsbrunn.ex b/lib/service/geospatial/mimirsbrunn.ex
index 729d76fc7..01aa191b8 100644
--- a/lib/service/geospatial/mimirsbrunn.ex
+++ b/lib/service/geospatial/mimirsbrunn.ex
@@ -9,7 +9,7 @@ defmodule Mobilizon.Service.Geospatial.Mimirsbrunn do
 
   alias Mobilizon.Addresses.Address
   alias Mobilizon.Service.Geospatial.Provider
-  alias Mobilizon.Service.HTTP.BaseClient
+  alias Mobilizon.Service.HTTP.GeospatialClient
 
   require Logger
 
@@ -23,15 +23,9 @@ defmodule Mobilizon.Service.Geospatial.Mimirsbrunn do
   """
   @spec geocode(number(), number(), keyword()) :: list(Address.t())
   def geocode(lon, lat, options \\ []) do
-    url = build_url(:geocode, %{lon: lon, lat: lat}, options)
-    Logger.debug("Asking Mimirsbrunn for reverse geocoding with #{url}")
-
-    with {:ok, %{status: 200, body: body}} <- BaseClient.get(url),
-         {:ok, %{"features" => features}} <- Jason.decode(body) do
-      process_data(features)
-    else
-      _ -> []
-    end
+    :geocode
+    |> build_url(%{lon: lon, lat: lat}, options)
+    |> fetch_features
   end
 
   @impl Provider
@@ -40,15 +34,9 @@ defmodule Mobilizon.Service.Geospatial.Mimirsbrunn do
   """
   @spec search(String.t(), keyword()) :: list(Address.t())
   def search(q, options \\ []) do
-    url = build_url(:search, %{q: q}, options)
-    Logger.debug("Asking Mimirsbrunn for addresses with #{url}")
-
-    with {:ok, %{status: 200, body: body}} <- BaseClient.get(url),
-         {:ok, %{"features" => features}} <- Jason.decode(body) do
-      process_data(features)
-    else
-      _ -> []
-    end
+    :search
+    |> build_url(%{q: q}, options)
+    |> fetch_features
   end
 
   @spec build_url(atom(), map(), list()) :: String.t()
@@ -68,6 +56,20 @@ defmodule Mobilizon.Service.Geospatial.Mimirsbrunn do
     end
   end
 
+  @spec fetch_features(String.t()) :: list(Address.t())
+  defp fetch_features(url) do
+    Logger.debug("Asking addok with #{url}")
+
+    with {:ok, %{status: 200, body: body}} <- GeospatialClient.get(url),
+         %{"features" => features} <- body do
+      process_data(features)
+    else
+      _ ->
+        Logger.error("Asking addok with #{url}")
+        []
+    end
+  end
+
   defp process_data(features) do
     features
     |> Enum.map(fn %{
diff --git a/lib/service/geospatial/nominatim.ex b/lib/service/geospatial/nominatim.ex
index ba1d535b9..9e94f8367 100644
--- a/lib/service/geospatial/nominatim.ex
+++ b/lib/service/geospatial/nominatim.ex
@@ -5,7 +5,7 @@ defmodule Mobilizon.Service.Geospatial.Nominatim do
 
   alias Mobilizon.Addresses.Address
   alias Mobilizon.Service.Geospatial.Provider
-  alias Mobilizon.Service.HTTP.BaseClient
+  alias Mobilizon.Service.HTTP.GeospatialClient
 
   require Logger
 
@@ -20,15 +20,9 @@ defmodule Mobilizon.Service.Geospatial.Nominatim do
   """
   @spec geocode(String.t(), keyword()) :: list(Address.t())
   def geocode(lon, lat, options \\ []) do
-    url = build_url(:geocode, %{lon: lon, lat: lat}, options)
-    Logger.debug("Asking Nominatim for geocode with #{url}")
-
-    with {:ok, %{status: 200, body: body}} <- BaseClient.get(url),
-         %{"features" => features} <- body do
-      features |> process_data() |> Enum.filter(& &1)
-    else
-      _ -> []
-    end
+    :geocode
+    |> build_url(%{lon: lon, lat: lat}, options)
+    |> fetch_features
   end
 
   @impl Provider
@@ -37,15 +31,9 @@ defmodule Mobilizon.Service.Geospatial.Nominatim do
   """
   @spec search(String.t(), keyword()) :: list(Address.t())
   def search(q, options \\ []) do
-    url = build_url(:search, %{q: q}, options)
-    Logger.debug("Asking Nominatim for addresses with #{url}")
-
-    with {:ok, %{status: 200, body: body}} <- BaseClient.get(url),
-         %{"features" => features} <- body do
-      features |> process_data() |> Enum.filter(& &1)
-    else
-      _ -> []
-    end
+    :search
+    |> build_url(%{q: q}, options)
+    |> fetch_features
   end
 
   @spec build_url(atom(), map(), list()) :: String.t()
@@ -77,6 +65,20 @@ defmodule Mobilizon.Service.Geospatial.Nominatim do
     if is_nil(api_key), do: url, else: url <> "&key=#{api_key}"
   end
 
+  @spec fetch_features(String.t()) :: list(Address.t())
+  defp fetch_features(url) do
+    Logger.debug("Asking Nominatim with #{url}")
+
+    with {:ok, %{status: 200, body: body}} <- GeospatialClient.get(url),
+         %{"features" => features} <- body do
+      features |> process_data |> Enum.filter(& &1)
+    else
+      _ ->
+        Logger.error("Asking Nominatim with #{url}")
+        []
+    end
+  end
+
   defp process_data(features) do
     features
     |> Enum.map(fn %{
diff --git a/lib/service/geospatial/pelias.ex b/lib/service/geospatial/pelias.ex
index 969a5e461..f9d82797a 100644
--- a/lib/service/geospatial/pelias.ex
+++ b/lib/service/geospatial/pelias.ex
@@ -7,7 +7,7 @@ defmodule Mobilizon.Service.Geospatial.Pelias do
 
   alias Mobilizon.Addresses.Address
   alias Mobilizon.Service.Geospatial.Provider
-  alias Mobilizon.Service.HTTP.BaseClient
+  alias Mobilizon.Service.HTTP.GeospatialClient
 
   require Logger
 
@@ -21,15 +21,9 @@ defmodule Mobilizon.Service.Geospatial.Pelias do
   """
   @spec geocode(number(), number(), keyword()) :: list(Address.t())
   def geocode(lon, lat, options \\ []) do
-    url = build_url(:geocode, %{lon: lon, lat: lat}, options)
-    Logger.debug("Asking Pelias for reverse geocoding with #{url}")
-
-    with {:ok, %{status: 200, body: body}} <- BaseClient.get(url),
-         {:ok, %{"features" => features}} <- Jason.decode(body) do
-      process_data(features)
-    else
-      _ -> []
-    end
+    :geocode
+    |> build_url(%{lon: lon, lat: lat}, options)
+    |> fetch_features
   end
 
   @impl Provider
@@ -38,15 +32,9 @@ defmodule Mobilizon.Service.Geospatial.Pelias do
   """
   @spec search(String.t(), keyword()) :: list(Address.t())
   def search(q, options \\ []) do
-    url = build_url(:search, %{q: q}, options)
-    Logger.debug("Asking Pelias for addresses with #{url}")
-
-    with {:ok, %{status: 200, body: body}} <- BaseClient.get(url),
-         {:ok, %{"features" => features}} <- Jason.decode(body) do
-      process_data(features)
-    else
-      _ -> []
-    end
+    :search
+    |> build_url(%{q: q}, options)
+    |> fetch_features
   end
 
   @spec build_url(atom(), map(), list()) :: String.t()
@@ -74,6 +62,20 @@ defmodule Mobilizon.Service.Geospatial.Pelias do
     if is_nil(country_code), do: url, else: "#{url}&boundary.country=#{country_code}"
   end
 
+  @spec fetch_features(String.t()) :: list(Address.t())
+  defp fetch_features(url) do
+    Logger.debug("Asking pelias with #{url}")
+
+    with {:ok, %{status: 200, body: body}} <- GeospatialClient.get(url),
+         %{"features" => features} <- body do
+      process_data(features)
+    else
+      _ ->
+        Logger.error("Asking pelias with #{url}")
+        []
+    end
+  end
+
   defp process_data(features) do
     features
     |> Enum.map(fn %{
diff --git a/lib/service/geospatial/photon.ex b/lib/service/geospatial/photon.ex
index 9790d382c..ef5f33b43 100644
--- a/lib/service/geospatial/photon.ex
+++ b/lib/service/geospatial/photon.ex
@@ -5,7 +5,7 @@ defmodule Mobilizon.Service.Geospatial.Photon do
 
   alias Mobilizon.Addresses.Address
   alias Mobilizon.Service.Geospatial.Provider
-  alias Mobilizon.Service.HTTP.BaseClient
+  alias Mobilizon.Service.HTTP.GeospatialClient
 
   require Logger
 
@@ -21,15 +21,9 @@ defmodule Mobilizon.Service.Geospatial.Photon do
   """
   @spec geocode(number(), number(), keyword()) :: list(Address.t())
   def geocode(lon, lat, options \\ []) do
-    url = build_url(:geocode, %{lon: lon, lat: lat}, options)
-    Logger.debug("Asking photon for reverse geocoding with #{url}")
-
-    with {:ok, %{status: 200, body: body}} <- BaseClient.get(url),
-         %{"features" => features} <- body do
-      process_data(features)
-    else
-      _ -> []
-    end
+    :geocode
+    |> build_url(%{lon: lon, lat: lat}, options)
+    |> fetch_features
   end
 
   @impl Provider
@@ -38,15 +32,9 @@ defmodule Mobilizon.Service.Geospatial.Photon do
   """
   @spec search(String.t(), keyword()) :: list(Address.t())
   def search(q, options \\ []) do
-    url = build_url(:search, %{q: q}, options)
-    Logger.debug("Asking photon for addresses with #{url}")
-
-    with {:ok, %{status: 200, body: body}} <- BaseClient.get(url),
-         %{"features" => features} <- body do
-      process_data(features)
-    else
-      _ -> []
-    end
+    :search
+    |> build_url(%{q: q}, options)
+    |> fetch_features
   end
 
   @spec build_url(atom(), map(), list()) :: String.t()
@@ -66,6 +54,20 @@ defmodule Mobilizon.Service.Geospatial.Photon do
     end
   end
 
+  @spec fetch_features(String.t()) :: list(Address.t())
+  defp fetch_features(url) do
+    Logger.debug("Asking photon with #{url}")
+
+    with {:ok, %{status: 200, body: body}} <- GeospatialClient.get(url),
+         %{"features" => features} <- body do
+      process_data(features)
+    else
+      _ ->
+        Logger.error("Asking photon with #{url}")
+        []
+    end
+  end
+
   defp process_data(features) do
     features
     |> Enum.map(fn %{"geometry" => geometry, "properties" => properties} ->
diff --git a/lib/service/http/base_client.ex b/lib/service/http/geospatial_client.ex
similarity index 78%
rename from lib/service/http/base_client.ex
rename to lib/service/http/geospatial_client.ex
index 02728de6b..1b578c9de 100644
--- a/lib/service/http/base_client.ex
+++ b/lib/service/http/geospatial_client.ex
@@ -1,6 +1,7 @@
-defmodule Mobilizon.Service.HTTP.BaseClient do
+defmodule Mobilizon.Service.HTTP.GeospatialClient do
   @moduledoc """
   Tesla HTTP Basic Client
+  with JSON middleware
   """
 
   use Tesla
@@ -19,4 +20,6 @@ defmodule Mobilizon.Service.HTTP.BaseClient do
   plug(Tesla.Middleware.Timeout, timeout: 10_000)
 
   plug(Tesla.Middleware.Headers, [{"User-Agent", @user_agent}])
+
+  plug(Tesla.Middleware.JSON)
 end
diff --git a/test/service/geospatial/addok_test.exs b/test/service/geospatial/addok_test.exs
index abcd450d8..bf698c9d8 100644
--- a/test/service/geospatial/addok_test.exs
+++ b/test/service/geospatial/addok_test.exs
@@ -5,7 +5,7 @@ defmodule Mobilizon.Service.Geospatial.AddokTest do
 
   alias Mobilizon.Addresses.Address
   alias Mobilizon.Service.Geospatial.Addok
-  alias Mobilizon.Service.HTTP.BaseClient.Mock
+  alias Mobilizon.Service.HTTP.GeospatialClient.Mock
 
   describe "search address" do
     test "returns a valid address from search" do
diff --git a/test/service/geospatial/google_maps_test.exs b/test/service/geospatial/google_maps_test.exs
index 624fafbb6..ab8833c6e 100644
--- a/test/service/geospatial/google_maps_test.exs
+++ b/test/service/geospatial/google_maps_test.exs
@@ -5,7 +5,7 @@ defmodule Mobilizon.Service.Geospatial.GoogleMapsTest do
 
   alias Mobilizon.Addresses.Address
   alias Mobilizon.Service.Geospatial.GoogleMaps
-  alias Mobilizon.Service.HTTP.BaseClient.Mock
+  alias Mobilizon.Service.HTTP.GeospatialClient.Mock
 
   describe "search address" do
     test "without API Key triggers an error" do
diff --git a/test/service/geospatial/map_quest_test.exs b/test/service/geospatial/map_quest_test.exs
index 937fd6525..4a1edf224 100644
--- a/test/service/geospatial/map_quest_test.exs
+++ b/test/service/geospatial/map_quest_test.exs
@@ -5,7 +5,7 @@ defmodule Mobilizon.Service.Geospatial.MapQuestTest do
 
   alias Mobilizon.Addresses.Address
   alias Mobilizon.Service.Geospatial.MapQuest
-  alias Mobilizon.Service.HTTP.BaseClient.Mock
+  alias Mobilizon.Service.HTTP.GeospatialClient.Mock
 
   describe "search address" do
     test "without API Key triggers an error" do
diff --git a/test/service/geospatial/nominatim_test.exs b/test/service/geospatial/nominatim_test.exs
index f3dedf651..40d2bd959 100644
--- a/test/service/geospatial/nominatim_test.exs
+++ b/test/service/geospatial/nominatim_test.exs
@@ -4,7 +4,7 @@ defmodule Mobilizon.Service.Geospatial.NominatimTest do
 
   alias Mobilizon.Addresses.Address
   alias Mobilizon.Service.Geospatial.Nominatim
-  alias Mobilizon.Service.HTTP.BaseClient.Mock
+  alias Mobilizon.Service.HTTP.GeospatialClient.Mock
 
   describe "search address" do
     test "returns a valid address from search" do
diff --git a/test/service/geospatial/photon_test.exs b/test/service/geospatial/photon_test.exs
index c5fa9fb9b..7d3ba8221 100644
--- a/test/service/geospatial/photon_test.exs
+++ b/test/service/geospatial/photon_test.exs
@@ -5,7 +5,7 @@ defmodule Mobilizon.Service.Geospatial.PhotonTest do
 
   alias Mobilizon.Addresses.Address
   alias Mobilizon.Service.Geospatial.Photon
-  alias Mobilizon.Service.HTTP.BaseClient.Mock
+  alias Mobilizon.Service.HTTP.GeospatialClient.Mock
 
   describe "search address" do
     test "returns a valid address from search" do
diff --git a/test/support/data_case.ex b/test/support/data_case.ex
index 79b301659..575d7839a 100644
--- a/test/support/data_case.ex
+++ b/test/support/data_case.ex
@@ -75,5 +75,5 @@ defmodule Mobilizon.DataCase do
   end
 
   Mox.defmock(Mobilizon.Service.HTTP.ActivityPub.Mock, for: Tesla.Adapter)
-  Mox.defmock(Mobilizon.Service.HTTP.BaseClient.Mock, for: Tesla.Adapter)
+  Mox.defmock(Mobilizon.Service.HTTP.GeospatialClient.Mock, for: Tesla.Adapter)
 end