-
Notifications
You must be signed in to change notification settings - Fork 0
Json endpoint #5
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,22 @@ | ||
| defmodule Provider.Cache do | ||
| @moduledoc """ | ||
| Defines a behaviour for cache implementations to follow | ||
| """ | ||
|
|
||
| @callback set(mod :: module(), key :: atom(), value :: term()) :: :ok | ||
| @callback get(mod :: module(), key :: atom()) :: {:ok, term()} | {:error, :not_found} | ||
|
|
||
| @spec set(module(), atom(), term()) :: :ok | ||
| def set(mod, key, value) do | ||
| impl().set(mod, key, value) | ||
| end | ||
|
|
||
| @spec get(module(), atom()) :: {:ok, term()} | {:error, :not_found} | ||
| def get(mod, key) do | ||
| impl().get(mod, key) | ||
| end | ||
|
|
||
| defp impl do | ||
| Application.get_env(:provider, :cache, Provider.Cache.ETS) | ||
| end | ||
| end |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,40 @@ | ||
| defmodule Provider.Cache.ETS do | ||
| @moduledoc """ | ||
| An ets based cache implementation | ||
| """ | ||
| @behaviour Provider.Cache | ||
|
|
||
| use GenServer | ||
|
|
||
| @spec start_link(Keyword.t()) :: GenServer.server() | ||
| def start_link(_opts) do | ||
| GenServer.start_link(__MODULE__, :ok, name: __MODULE__) | ||
| end | ||
|
|
||
| @impl Provider.Cache | ||
| def set(module, key, value) do | ||
| GenServer.call(__MODULE__, {:set, module, key, value}) | ||
| end | ||
|
|
||
| @impl Provider.Cache | ||
| def get(module, key) do | ||
| case :ets.lookup(__MODULE__, {module, key}) do | ||
| [{{^module, ^key}, value}] -> {:ok, value} | ||
| [] -> {:error, :not_found} | ||
| end | ||
| end | ||
|
|
||
| @impl GenServer | ||
| def init(:ok) do | ||
| state = :ets.new(__MODULE__, [:named_table]) | ||
|
|
||
| {:ok, state} | ||
| end | ||
|
|
||
| @impl GenServer | ||
| def handle_call({:set, module, key, value}, _from, state) do | ||
| :ets.insert(state, {{module, key}, value}) | ||
|
|
||
| {:reply, :ok, state} | ||
| end | ||
| end |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -21,8 +21,7 @@ defmodule Provider do | |
|
|
||
| This will generate the following functions in the module: | ||
|
|
||
| - `fetch_all` - retrieves values of all parameters | ||
| - `validate!` - validates that all parameters are correctly provided | ||
| - `load!` - validates that all parameters are correctly provided and stores them in the cache | ||
| - `db_host`, `db_name`, `db_pool_size`, ... - getter of each declared parameter | ||
|
|
||
| ## Describing params | ||
|
|
@@ -99,7 +98,7 @@ defmodule Provider do | |
| @type source :: module | ||
| @type params :: %{param_name => param_spec} | ||
| @type param_name :: atom | ||
| @type param_spec :: %{type: type, default: value} | ||
| @type param_spec :: %{optional(:source) => String.t(), type: type, default: value} | ||
| @type type :: :string | :integer | :float | :boolean | ||
| @type value :: String.t() | number | boolean | nil | ||
| @type data :: %{param_name => value} | ||
|
|
@@ -109,13 +108,13 @@ defmodule Provider do | |
| # ------------------------------------------------------------------------ | ||
|
|
||
| @doc "Retrieves all params according to the given specification." | ||
| @spec fetch_all(source, params) :: {:ok, data} | {:error, [String.t()]} | ||
| def fetch_all(source, params) do | ||
| @spec fetch_all(source, params, Keyword.t()) :: {:ok, data} | {:error, [String.t()]} | ||
| def fetch_all(source, params, opts) do | ||
| types = Enum.into(params, %{}, fn {name, spec} -> {name, spec.type} end) | ||
|
|
||
| data = | ||
| params | ||
| |> Stream.zip(source.values(Map.keys(types))) | ||
| |> Stream.zip(source.values(params, opts)) | ||
| |> Enum.into(%{}, fn {{param, opts}, provided_value} -> | ||
| value = if is_nil(provided_value), do: opts.default, else: provided_value | ||
| {param, value} | ||
|
|
@@ -125,32 +124,19 @@ defmodule Provider do | |
| |> Changeset.cast(data, Map.keys(types)) | ||
| |> Changeset.validate_required(Map.keys(types), message: "is missing") | ||
| |> case do | ||
| %Changeset{valid?: true} = changeset -> {:ok, Changeset.apply_changes(changeset)} | ||
| %Changeset{valid?: false} = changeset -> {:error, changeset_error(source, changeset)} | ||
| end | ||
| end | ||
|
|
||
| @doc "Retrieves a single parameter." | ||
| @spec fetch_one(source, param_name, param_spec) :: {:ok, value} | {:error, [String.t()]} | ||
| def fetch_one(source, param_name, param_spec) do | ||
| with {:ok, map} <- fetch_all(source, %{param_name => param_spec}), | ||
| do: {:ok, Map.fetch!(map, param_name)} | ||
| end | ||
| %Changeset{valid?: true} = changeset -> | ||
| {:ok, Changeset.apply_changes(changeset)} | ||
|
|
||
| @doc "Retrieves a single param, raising if the value is not available." | ||
| @spec fetch_one!(source, param_name, param_spec) :: value | ||
| def fetch_one!(source, param, param_spec) do | ||
| case fetch_one(source, param, param_spec) do | ||
| {:ok, value} -> value | ||
| {:error, errors} -> raise Enum.join(errors, ", ") | ||
| %Changeset{valid?: false} = changeset -> | ||
| {:error, changeset_error(source, params, changeset)} | ||
| end | ||
| end | ||
|
|
||
| # ------------------------------------------------------------------------ | ||
| # Private | ||
| # ------------------------------------------------------------------------ | ||
|
|
||
| defp changeset_error(source, changeset) do | ||
| defp changeset_error(source, params, changeset) do | ||
| changeset | ||
| |> Ecto.Changeset.traverse_errors(fn {msg, opts} -> | ||
| Enum.reduce( | ||
|
|
@@ -160,7 +146,7 @@ defmodule Provider do | |
| ) | ||
| end) | ||
| |> Enum.flat_map(fn {key, errors} -> | ||
| Enum.map(errors, &"#{source.display_name(key)} #{&1}") | ||
| Enum.map(errors, &"#{source.display_name(key, params[key])} #{&1}") | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Don't quite understand why is this change needed.
Owner
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Unfortunately I can't say I remember the details behind it :(
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That's on me, should have reviewed this much sooner 😅 |
||
| end) | ||
| |> Enum.sort() | ||
| end | ||
|
|
@@ -170,10 +156,21 @@ defmodule Provider do | |
| spec = | ||
| update_in( | ||
| spec[:params], | ||
| fn params -> Enum.map(params, &normalize_param_spec(&1, Mix.env())) end | ||
| fn params -> | ||
| Enum.map(params, &normalize_param_spec(&1, Mix.env())) | ||
| end | ||
| ) | ||
|
|
||
| quote bind_quoted: [spec: spec] do | ||
| {source, opts} = | ||
| case Keyword.fetch!(spec, :source) do | ||
| {source, opts} -> {source, Macro.escape(opts, unquote: true)} | ||
| source -> {source, []} | ||
| end | ||
|
|
||
| spec = | ||
| update_in(spec[:source], fn _source -> source end) | ||
|
|
||
| quote bind_quoted: [spec: spec, opts: opts] do | ||
| # Generate typespec mapping for each param | ||
| typespecs = | ||
| Enum.map( | ||
|
|
@@ -199,22 +196,24 @@ defmodule Provider do | |
| |> Keyword.fetch!(:params) | ||
| |> Enum.map(fn {name, spec} -> {name, quote(do: %{unquote_splicing(spec)})} end) | ||
|
|
||
| @doc "Retrieves all parameters." | ||
| @spec fetch_all :: {:ok, %{unquote_splicing(typespecs)}} | {:error, [String.t()]} | ||
| def fetch_all do | ||
| Provider.fetch_all( | ||
| unquote(Keyword.fetch!(spec, :source)), | ||
|
|
||
| # quoted_params is itself a keyword list, so we need to convert it into a map | ||
| %{unquote_splicing(quoted_params)} | ||
| ) | ||
| end | ||
|
|
||
| @doc "Validates all parameters, raising if some values are missing or invalid." | ||
| @spec validate!() :: :ok | ||
| def validate! do | ||
| with {:error, errors} <- fetch_all() do | ||
| raise "Following OS env var errors were found:\n#{Enum.join(Enum.sort(errors), "\n")}" | ||
| @doc "Loads and validates all parameters, raising if some values are missing or invalid." | ||
| @spec load!() :: :ok | ||
| def load! do | ||
| source = unquote(Keyword.fetch!(spec, :source)) | ||
| opts = unquote(opts) | ||
|
|
||
| case Provider.fetch_all( | ||
| source, | ||
| %{ | ||
| unquote_splicing(quoted_params) | ||
| }, | ||
| opts | ||
| ) do | ||
| {:ok, values} -> | ||
| Enum.each(values, fn {k, v} -> Provider.Cache.set(__MODULE__, k, v) end) | ||
|
|
||
| {:error, errors} -> | ||
| raise "#{source} encountered errors loading values:\n#{Enum.join(Enum.sort(errors), "\n")}" | ||
| end | ||
|
|
||
| :ok | ||
|
|
@@ -229,19 +228,25 @@ defmodule Provider do | |
| # bug in credo spec check | ||
| # credo:disable-for-next-line Credo.Check.Readability.Specs | ||
| def unquote(param_name)() do | ||
| Provider.fetch_one!( | ||
| unquote(Keyword.fetch!(spec, :source)), | ||
| unquote(param_name), | ||
| unquote(param_spec) | ||
| ) | ||
| case Provider.Cache.get(__MODULE__, unquote(param_name)) do | ||
| {:ok, value} -> | ||
| value | ||
|
|
||
| {:error, :not_found} -> | ||
| source = unquote(Keyword.fetch!(spec, :source)) | ||
|
|
||
| raise "#{source.display_name(unquote(param_name), unquote(param_spec))} is missing" | ||
| end | ||
| end | ||
| end | ||
| ) | ||
|
|
||
| @doc "Returns a template configuration file." | ||
| @spec template :: String.t() | ||
| def template do | ||
| unquote(Keyword.fetch!(spec, :source)).template(%{unquote_splicing(quoted_params)}) | ||
| source = unquote(Keyword.fetch!(spec, :source)) | ||
|
|
||
| source.template(%{unquote_splicing(quoted_params)}) | ||
| end | ||
| end | ||
| end | ||
|
|
@@ -269,7 +274,9 @@ defmodule Provider do | |
| # context of the client module. | ||
| |> Macro.escape(unquote: true) | ||
|
|
||
| {param_name, [type: Keyword.get(param_spec, :type, :string), default: default_value]} | ||
| {param_name, | ||
| Keyword.drop(param_spec, [:type, :default]) ++ | ||
| [type: Keyword.get(param_spec, :type, :string), default: default_value]} | ||
| end | ||
|
|
||
| defmodule Source do | ||
|
|
@@ -282,10 +289,12 @@ defmodule Provider do | |
| This function should return all values in the requested orders. For each param which is not | ||
| available, `nil` should be returned. | ||
| """ | ||
| @callback values([Provider.param_name()]) :: [Provider.value()] | ||
| @callback values(Provider.params(), Keyword.t()) :: [ | ||
| Provider.value() | ||
| ] | ||
|
|
||
| @doc "Invoked to convert the param name to storage specific name." | ||
| @callback display_name(Provider.param_name()) :: String.t() | ||
| @callback display_name(Provider.param_name(), Provider.param_spec()) :: String.t() | ||
|
|
||
| @doc "Invoked to create operator template." | ||
| @callback template(Provider.params()) :: String.t() | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,15 @@ | ||
| defmodule Provider.Application do | ||
| @moduledoc false | ||
|
|
||
| use Application | ||
|
|
||
| @impl true | ||
| def start(_type, _args) do | ||
| children = [ | ||
| Provider.Cache.ETS | ||
| ] | ||
|
|
||
| opts = [strategy: :one_for_one, name: Provider.Supervisor] | ||
| Supervisor.start_link(children, opts) | ||
| end | ||
| end |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,52 @@ | ||
| defmodule Provider.JsonEndpoint do | ||
| @moduledoc """ | ||
| Provider source which retrieves values from a JSON endpoint. | ||
| The following options are accepted. | ||
| * :endpoint - This is the URL where the JSON configuration can be found. | ||
| """ | ||
|
|
||
| @behaviour Provider.Source | ||
|
|
||
| require Logger | ||
| alias Provider.Source | ||
|
|
||
| @impl Source | ||
| def display_name(param_name, spec), do: Map.get(spec, :source, to_string(param_name)) | ||
|
|
||
| @impl Source | ||
| def values(params, opts) do | ||
| endpoint = Keyword.fetch!(opts, :endpoint) | ||
|
|
||
| response = | ||
| [{Tesla.Middleware.BaseUrl, endpoint}, Tesla.Middleware.JSON] | ||
| |> Tesla.client() | ||
| |> Tesla.get("") | ||
|
Comment on lines
+23
to
+25
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd probably use req (as an optional dep), and alternatively allow the client to supply the |
||
|
|
||
| case response do | ||
| {:ok, response} -> | ||
| Enum.map(params, fn {k, spec} -> | ||
| response.body[display_name(k, spec)] | ||
| end) | ||
|
|
||
| {:error, reason} -> | ||
| Logger.warning("#{__MODULE__} unable to retrieve values - #{reason}") | ||
|
|
||
| Enum.map(params, fn {_k, _spec} -> | ||
| nil | ||
| end) | ||
|
Comment on lines
+36
to
+38
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this actually what we want? Shouldn't we raise in this case? I see you mention in the commit desc that
But, I don't think dev convenience justifies this default which can lead to weird prod bugs. If the clients don't want to use a JSON server locally in dev, they could use a different source in dev. Or alternatively, start a mock server (for which we could support a convenience helper function, something like
Owner
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
This is very true. There is no reason we could not have someone do something like defmodule MyConfig do
use Provider,
source: Application.compile_env(:my_app, :my_config_source)
endin order to do something slightly different between dev and prod-like deployments. |
||
| end | ||
| end | ||
|
|
||
| @impl Source | ||
| def template(params) do | ||
| params | ||
| |> Enum.map(fn {k, spec} -> | ||
| {display_name(k, spec), spec.default} | ||
| end) | ||
| |> Map.new() | ||
| |> Jason.encode!() | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could we use the stock Erlang's json (https://www.erlang.org/doc/apps/stdlib/json.html) instead? Note that in this case we'd only support OTP 27+.
Owner
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would prefer not to as of right now. I would reconsider it once OTP 28 is released. |
||
| |> Jason.Formatter.pretty_print() | ||
| end | ||
| end | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These opts are passed to the source, right? It would be good to document that somewhere.