diff --git a/.travis.yml b/.travis.yml index a1d5244..3191155 100644 --- a/.travis.yml +++ b/.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 diff --git a/README.md b/README.md index 2b3cfaf..0c4c46f 100644 --- a/README.md +++ b/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? diff --git a/config/config.exs b/config/config.exs index c15000a..df2c455 100644 --- a/config/config.exs +++ b/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 - diff --git a/lib/geocoder.ex b/lib/geocoder.ex index f719336..6a8f494 100644 --- a/lib/geocoder.ex +++ b/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 diff --git a/lib/geocoder/providers/google_maps.ex b/lib/geocoder/providers/google_maps.ex index 7f69503..f717ea1 100644 --- a/lib/geocoder/providers/google_maps.ex +++ b/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 diff --git a/lib/geocoder/providers/open_cage_data.ex b/lib/geocoder/providers/open_cage_data.ex index a430680..a9978d2 100644 --- a/lib/geocoder/providers/open_cage_data.ex +++ b/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 diff --git a/lib/geocoder/worker.ex b/lib/geocoder/worker.ex index 984706a..a31762c 100644 --- a/lib/geocoder/worker.ex +++ b/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(¶ms[:store].update/1) + |> tap(¶ms[: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 diff --git a/test/geocoder_test.exs b/test/geocoder_test.exs index 4d1b14e..80639a8 100644 --- a/test/geocoder_test.exs +++ b/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)