Merge dedc987 into de000e8

.travis.yml

@@ -1,33 +1,24 @@
 language: elixir
 notifications:
   email: false
+# https://repo.hex.pm/builds/elixir/builds.txt
 elixir:
-  - 1.6
-  - 1.7
-  - 1.8
   - 1.9
+  - 1.10
+  - 1.11
+# http://docs.travis-ci.com/user/languages/erlang/
 otp_release:
-  - 19.3
   - 20.3
-  - 21.0
-  - 22.0
+  - 21.3
+  - 22.3
+  - 23.0
 matrix:
   exclude:
-    - elixir: 1.6
-      otp_release: 21.0
-    - elixir: 1.6
-      otp_release: 22.0
-    - elixir: 1.7
-      otp_release: 19.3
-    - elixir: 1.7
-      otp_release: 20.3
-    - elixir: 1.8
-      otp_release: 19.3
-    - elixir: 1.8
-      otp_release: 20.3
-    - elixir: 1.9
-      otp_release: 19.3
     - elixir: 1.9
+      otp_release: 23.0
+    - elixir: 1.10
+      otp_release: 20.3
+    - elixir: 1.11
       otp_release: 20.3
 
 env: MIX_ENV=test

README.md

@@ -23,6 +23,8 @@ Update your mix dependencies:
 mix deps.get
 ```
 
+If you are Elixir < 1.9, you'll need to use a version before `1.0`.
+
 Configuration
 -------------
 
@@ -43,7 +45,7 @@ config :geocoder, :worker,
   key: System.get_env("GEOCODER_GOOGLE_API_KEY")
 ```
 
-Note that `OpenStreetMaps` is the only provider that does not require an API key to operate.
+Note that `OpenStreetMaps` (the default provider) is the only provider that does not require an API key to operate.
 All other providers require an API key that you'll need to provide.
 
 If you need to set a proxy (or any other option supported by HTTPoison.get/3):
@@ -72,7 +74,19 @@ You can pass options to the function that will be passed to the geocoder provide
 Geocoder.call(address: "Toronto, ON", language: "es", key: "...", ...)
 ```
 
-See [here](https://developers.google.com/maps/documentation/geocoding/intro#geocoding) and [here](https://developers.google.com/maps/documentation/geocoding/intro#ReverseGeocoding) for a list of supported parameters for the default geocoder provider (`Geocoder.Provider.GoogleMaps`).
+You can also change the provider on a per-call basis:
+
+```elixir
+{:ok, coordinates } =
+  with
+    # use the default provider
+    {:error, nil} <- Geocoder.call(query),
+    # use an alternative provider. If `key` is not specified here the globally defined key will be used.
+    {:error, nil} <- Geocoder.call(query, provider: Geocoder.Providers.OpenCageData, key: "123"),
+    do: {:error}
+```
+
+See [here](https://developers.google.com/maps/documentation/geocoding/intro#geocoding) and [here](https://developers.google.com/maps/documentation/geocoding/intro#ReverseGeocoding) for a list of supported parameters for the google maps geocoder provider (`Geocoder.Provider.GoogleMaps`).
 
 And you're done! How simple was that?
 

config/config.exs

@@ -4,12 +4,17 @@ config :geocoder, :worker_pool_config,
   size: 4,
   max_overflow: 2
 
-case System.get_env("PROVIDER") || "openstreetmaps" do
+case System.get_env("PROVIDER", "openstreetmaps") do
   "google" ->
-    config :geocoder, :worker, provider: Geocoder.Providers.GoogleMaps, key: System.get_env("API_KEY", "NO_API_KEY")
+    config :geocoder, :worker,
+      provider: Geocoder.Providers.GoogleMaps,
+      key: System.get_env("API_KEY", "NO_API_KEY")
+
   "opencagedata" ->
-    config :geocoder, :worker, provider: Geocoder.Providers.OpenCageData, key: System.get_env("API_KEY", "NO_API_KEY")
+    config :geocoder, :worker,
+      provider: Geocoder.Providers.OpenCageData,
+      key: System.get_env("API_KEY", "NO_API_KEY")
+
   "openstreetmaps" ->
     config :geocoder, :worker, provider: Geocoder.Providers.OpenStreetMaps
 end
-

lib/geocoder.ex

@@ -7,23 +7,21 @@ defmodule Geocoder do
   def pool_name, do: @pool_name
 
   def worker_config do
-    Keyword.merge(Application.get_env(:geocoder, Geocoder.Worker) || [], @default_config)
+    Keyword.merge(Application.get_env(:geocoder, Geocoder.Worker, []), @default_config)
   end
 
   def store_config do
-    Application.get_env(:geocoder, Geocoder.Store) || []
+    Application.get_env(:geocoder, Geocoder.Store, [])
   end
 
   def start(_type, _opts) do
-    import Supervisor.Spec
-
     children = [
       :poolboy.child_spec(
         pool_name(),
         worker_config(),
-        Application.get_env(:geocoder, :worker) || []
+        Application.get_env(:geocoder, :worker, [])
       ),
-      worker(Geocoder.Store, [store_config()])
+      Supervisor.child_spec(Geocoder.Store, store_config())
     ]
 
     options = [
@@ -44,10 +42,10 @@ defmodule Geocoder do
   def call(q = {lat, lon}, opts),
     do: Worker.reverse_geocode(opts ++ [lat: lat, lon: lon, latlng: q])
 
-  def call(%{lat: lat, lon: lon}, opts), do: call({lat, lon}, opts)
+  def call(%{lat: lat, lon: lon}, opts), do: call([latlng: {lat, lon}] ++ opts)
 
   def call_list(q, opts \\ [])
   def call_list(q, opts) when is_binary(q), do: Worker.geocode_list(opts ++ [address: q])
   def call_list(q = {_, _}, opts), do: Worker.reverse_geocode_list(opts ++ [latlng: q])
-  def call_list(%{lat: lat, lon: lon}, opts), do: call_list({lat, lon}, opts)
+  def call_list(%{lat: lat, lon: lon}, opts), do: call_list(opts ++ [latlng: {lat, lon}])
 end

lib/geocoder/providers/google_maps.ex

@@ -42,7 +42,7 @@ defmodule Geocoder.Providers.GoogleMaps do
       {lat, lng} -> "#{lat},#{lng}"
       q -> q
     end)
-    |> Keyword.delete(:latlng, nil)
+    |> Keyword.delete(:latlng)
   end
 
   defp parse_geocode(response) do
@@ -136,15 +136,23 @@ defmodule Geocoder.Providers.GoogleMaps do
   end
 
   defp request_all(path, params) do
-    params = Keyword.merge(params, key: Application.get_env(:geocoder, :worker)[:key])
+    params =
+      Keyword.merge(params, key: params[:key] || Application.get_env(:geocoder, :worker)[:key])
+
     httpoison_options = Application.get_env(:geocoder, Geocoder.Worker)[:httpoison_options] || []
 
     case get(path, [], Keyword.merge(httpoison_options, params: Enum.into(params, %{}))) do
       # API does not return a non-200 code when there is an error!
-      {:ok, %{status_code: 200, body: %{"results" => [], "error_message" => error_message, "status" => _status}}} ->
+      {:ok,
+       %{
+         status_code: 200,
+         body: %{"results" => [], "error_message" => error_message, "status" => _status}
+       }} ->
         {:error, error_message}
+
       {:ok, %{status_code: 200, body: %{"status" => "OK", "results" => results}}} ->
         {:ok, List.wrap(results)}
+
       {_, response} ->
         {:error, response}
     end

lib/geocoder/providers/open_cage_data.ex

@@ -105,14 +105,19 @@ defmodule Geocoder.Providers.OpenCageData do
 
   defp request_all(path, params) do
     httpoison_options = Application.get_env(:geocoder, Geocoder.Worker)[:httpoison_options] || []
-    params = Keyword.merge(params, key: Application.get_env(:geocoder, :worker)[:key])
+
+    params =
+      Keyword.merge(params, key: params[:key] || Application.get_env(:geocoder, :worker)[:key])
+
     result = get(path, [], Keyword.merge(httpoison_options, params: Enum.into(params, %{})))
 
     case result do
-      {:ok, %{status_code: 401, body: %{"status" => %{"message" => message }}}} ->
+      {:ok, %{status_code: 401, body: %{"status" => %{"message" => message}}}} ->
         {:error, message}
+
       {:ok, %{status_code: 200, body: %{"results" => results}}} ->
         {:ok, List.wrap(results)}
+
       {_, response} ->
         {:error, response}
     end

lib/geocoder/worker.ex

@@ -3,20 +3,20 @@ defmodule Geocoder.Worker do
   use Towel
 
   # Public API
-  def geocode(q, opts \\ []) do
-    assign(:geocode, q, opts)
+  def geocode(params) do
+    assign(:geocode, params)
   end
 
-  def geocode_list(q, opts \\ []) do
-    assign(:geocode_list, q, opts)
+  def geocode_list(params) do
+    assign(:geocode_list, params)
   end
 
-  def reverse_geocode(q, opts \\ []) do
-    assign(:reverse_geocode, q, opts)
+  def reverse_geocode(params) do
+    assign(:reverse_geocode, params)
   end
 
-  def reverse_geocode_list(q, opts \\ []) do
-    assign(:reverse_geocode_list, q, opts)
+  def reverse_geocode_list(params) do
+    assign(:reverse_geocode_list, params)
   end
 
   # GenServer API
@@ -32,13 +32,19 @@ defmodule Geocoder.Worker do
     GenServer.start_link(__MODULE__, conf)
   end
 
-  def handle_call({function, q, opts}, _from, conf) do
-    {:reply, run(function, q, conf, opts[:store]), conf}
+  def handle_call({function, params}, _from, conf) do
+    # unfortunately, both the worker and param defaults use `store`
+    # for the worker, this defines which store to use, for the params
+    # this defines if the store should be used
+    use_store = params[:store]
+
+    params = Keyword.merge(conf, Keyword.drop(params, [:store]))
+    {:reply, run(function, params, use_store), conf}
   end
 
-  def handle_cast({function, q, opts}, conf) do
+  def handle_cast({function, params}, conf) do
     Task.start_link(fn ->
-      send(opts[:stream_to], run(function, conf, q, opts[:store]))
+      send(params[:stream_to], run(function, params, params[:store]))
     end)
 
     {:noreply, conf}
@@ -51,39 +57,38 @@ defmodule Geocoder.Worker do
     store: true
   ]
 
-  defp assign(name, q, opts) do
-    opts = Keyword.merge(@assign_defaults, opts)
+  defp assign(name, params) do
+    gen_server_options = Keyword.merge(@assign_defaults, params)
+    params_with_defaults = Keyword.drop(gen_server_options, [:timeout, :stream_to])
 
     function =
-      case {opts[:stream_to], {name, q, opts}} do
-        {nil, message} -> &GenServer.call(&1, message, opts[:timeout])
+      case {gen_server_options[:stream_to], {name, params_with_defaults}} do
+        {nil, message} -> &GenServer.call(&1, message, gen_server_options[:timeout])
         {_, message} -> &GenServer.cast(&1, message)
       end
 
-    :poolboy.transaction(Geocoder.pool_name(), function, opts[:timeout])
+    :poolboy.transaction(Geocoder.pool_name(), function, gen_server_options[:timeout])
   end
 
-  def run(function, q, conf, _) when function in [:geocode_list, :reverse_geocode_list] do
-    apply(conf[:provider], function, [additionnal_conf(q, conf)])
+  def run(function, params, useStore)
+
+  def run(function, params, _) when function in [:geocode_list, :reverse_geocode_list] do
+    apply(params[:provider], function, [params])
   end
 
-  def run(function, conf, q, false) do
-    apply(conf[:provider], function, [additionnal_conf(q, conf)])
-    |> tap(&conf[:store].update/1)
-    |> tap(&conf[:store].link(q, &1))
+  def run(function, params, false) do
+    apply(params[:provider], function, [params])
+    |> tap(&params[:store].update/1)
+    |> tap(&params[:store].link(params, &1))
   end
 
-  def run(function, q, conf, true) do
-    case apply(conf[:store], function, [additionnal_conf(q, conf)]) do
+  def run(function, params, true) do
+    case apply(params[:store], function, [params]) do
       {:just, coords} ->
         ok(coords)
 
       :nothing ->
-        run(function, conf, q, false)
+        run(function, params, false)
     end
   end
-
-  def additionnal_conf(q, conf) do
-    Keyword.merge(q, Keyword.drop(conf, [:store, :provider]))
-  end
 end

test/geocoder_test.exs

@@ -1,6 +1,18 @@
 defmodule GeocoderTest do
   use ExUnit.Case
 
+  setup do
+    # there's some state we need to clear before each test run
+    # https://github.com/sasa1977/con_cache/issues/11#issuecomment-116806567
+    :ok = Supervisor.terminate_child(Geocoder.Supervisor, Geocoder.Store)
+    {:ok, _} = Supervisor.restart_child(Geocoder.Supervisor, Geocoder.Store)
+
+    # OpenStreetData is rate-limited at 1rps. Let's ensure our tests don't break that rate limit.
+    Process.sleep(1_000)
+
+    :ok
+  end
+
   test "An address in New York" do
     {:ok, coords} = Geocoder.call("1991 15th Street, Troy, NY 12180")
     assert_new_york(coords)
@@ -11,6 +23,20 @@ defmodule GeocoderTest do
     assert_belgium(coords)
   end
 
+  test "properly handles call-specific provider and key configurations" do
+    {:error, "missing API key"} =
+      Geocoder.call("1991 15th Street, Troy, NY 12180", provider: Geocoder.Providers.OpenCageData)
+
+    {
+      :error,
+      "invalid API key"
+    } =
+      Geocoder.call("1991 15th Street, Troy, NY 12180",
+        provider: Geocoder.Providers.OpenCageData,
+        key: "bad_key"
+      )
+  end
+
   test "Reverse geocode" do
     {:ok, coords} = Geocoder.call({51.0775264, 3.7073382})
     assert_belgium(coords)