From 556fb32be5321c3976d4232b8bf48b21c7145249 Mon Sep 17 00:00:00 2001 From: Keith Salisbury Date: Sun, 9 Feb 2020 01:22:39 +0000 Subject: [PATCH 1/3] Naive Interprter This commit refactors the code to introduce a very simple interpreter and effect. There is a default interpreter which makes evaluates the given effect and invokes the change as a module, function, args. --- README.md | 20 +++++++++++++++++ TALK.md | 37 ++++++++++++++++++++++++++++++++ config/test.exs | 2 +- lib/example/default_service.ex | 7 ------ lib/example/effect.ex | 3 +++ lib/example/interpreter.ex | 7 ++++++ lib/example/service.ex | 6 ++++++ lib/example/service_behaviour.ex | 3 --- lib/example/worker.ex | 30 ++++++-------------------- test/support/interpreter.ex | 7 ++++++ test/support/mock_service.ex | 11 ---------- test/worker_test.exs | 26 ++-------------------- 12 files changed, 89 insertions(+), 70 deletions(-) create mode 100644 TALK.md delete mode 100644 lib/example/default_service.ex create mode 100644 lib/example/effect.ex create mode 100644 lib/example/interpreter.ex create mode 100644 lib/example/service.ex delete mode 100644 lib/example/service_behaviour.ex create mode 100644 test/support/interpreter.ex delete mode 100644 test/support/mock_service.ex diff --git a/README.md b/README.md index 9474d1f..3bcce3e 100644 --- a/README.md +++ b/README.md @@ -36,3 +36,23 @@ the time of application boot. - Are we testing the wrong abstraction? - Should we go with a [hand crafted mock](test/support/mock_service.ex)? - Are there any other options to solve this? + + +## Interpreter + +This branch removes all mocks, and the behaviour. Now we just have a single +definition of our service `Example.Service`. This instantly makes the code +simpler to read and follow. In order to connect our worker to the service we +add a new layer of indirection called the interpreter `Example.Interpreter`. +Instead of calling the service directly, the worker creates a description of +the side effect `Example.Effect` and passes that to the interpreter. In +production the default interpreter is configured to translate the effect into a +real world effect, and in the test environment we replace the default +interpreter with a test interpreter `Test.Interpreter` which simply returns +the effect - and we can simply assert the shape of the effect is correct. + +So whats missing here, what tests are we missing? + +some research links: +- https://github.com/yunmikun2/free_ast/blob/master/lib/free_ast.ex +- https://github.com/slogsdon/elixir-control/ diff --git a/TALK.md b/TALK.md new file mode 100644 index 0000000..77863e1 --- /dev/null +++ b/TALK.md @@ -0,0 +1,37 @@ +# Scalable software patterns with Monads + +This project provides a contrived example of when using Mox can make things +difficult. Besides being difficult to test, this approach to writing software +also suffers from scalability. + +Let's walk through a simple example: + +You have some code which depends on external services, let's imagine you need +to make an http request which consumes some data, processes that data and +writes to a cache. Once the cache has been updated the service then connects +to an amqp service and processes messages using data from the cache, eventually +writing the results to a database. + +The naive approach is to write a simple initialisation: + +1. Make http request +2. Process response +3. Update cache +4. Subscribe to AMQP +5. Process incoming messages +6. Read from cache +7. Write to database + +In order to test this code we could replace some parts with mocks. One way we +could do this is to identify the noun's in our system. Let's call them services +and see where we get to: + +1. Make http request (http service) +2. Process response (http processing service) +3. Update cache (cache service) +4. Subscribe and consumer AMQP (consumer service) +5. Process incoming messages (amqp processing service) +6. Read from cache (cache service) +7. Write to database (database service) + +To be continued... diff --git a/config/test.exs b/config/test.exs index e7da6b4..720f294 100644 --- a/config/test.exs +++ b/config/test.exs @@ -1,3 +1,3 @@ use Mix.Config -config :example, service: Example.MockService +config :example, interpretor: Test.Interpreter diff --git a/lib/example/default_service.ex b/lib/example/default_service.ex deleted file mode 100644 index 2ab42e9..0000000 --- a/lib/example/default_service.ex +++ /dev/null @@ -1,7 +0,0 @@ -defmodule Example.DefaultService do - @behaviour Example.ServiceBehaviour - - def foo() do - "default service says foo" - end -end diff --git a/lib/example/effect.ex b/lib/example/effect.ex new file mode 100644 index 0000000..f26f3c6 --- /dev/null +++ b/lib/example/effect.ex @@ -0,0 +1,3 @@ +defmodule Example.Effect do + defstruct [:m, :f, :a] +end diff --git a/lib/example/interpreter.ex b/lib/example/interpreter.ex new file mode 100644 index 0000000..6a6529b --- /dev/null +++ b/lib/example/interpreter.ex @@ -0,0 +1,7 @@ +defmodule Example.Interpreter do + alias Example.Effect + def run(%Effect{m: m, f: f, a: a} = _effect) do + # IO.inspect effect, label: "real effect" + apply(m, f, a) + end +end diff --git a/lib/example/service.ex b/lib/example/service.ex new file mode 100644 index 0000000..92854d8 --- /dev/null +++ b/lib/example/service.ex @@ -0,0 +1,6 @@ +defmodule Example.Service do + def foo() do + Process.sleep 1000 + IO.inspect "real service says foo", label: "Example.Service" + end +end diff --git a/lib/example/service_behaviour.ex b/lib/example/service_behaviour.ex deleted file mode 100644 index 744293f..0000000 --- a/lib/example/service_behaviour.ex +++ /dev/null @@ -1,3 +0,0 @@ -defmodule Example.ServiceBehaviour do - @callback foo() :: :ok | binary() -end diff --git a/lib/example/worker.ex b/lib/example/worker.ex index aed9473..039daad 100644 --- a/lib/example/worker.ex +++ b/lib/example/worker.ex @@ -1,17 +1,10 @@ defmodule Example.Worker do use GenServer - alias Example.DefaultService + alias Example.Effect + alias Example.Service - # When using ElixirLS, defining the service at compile time will result in an - # error because ElixirLS always compiles using MIX_ENV=test which mean @service - # will always be set to MockService, which does not have `foo/0` - # @service Application.get_env(:example, :service, DefaultService) - # @service DefaultService - - def service() do - Application.get_env(:example, :service, DefaultService) - end + @interpretor Application.get_env(:example, :interpretor, Example.Interpreter) def start_link(init_arg \\ []) do GenServer.start_link(__MODULE__, init_arg, name: __MODULE__) @@ -27,20 +20,9 @@ defmodule Example.Worker do end def handle_continue(:get_foo_from_service, _state) do - # And here lies the problem. We want to call our service to get - # whatever inital state it provides, but in doing so, we break - # in the test environment because the MockService doesn't have - # a function called `foo/0` until it can be defined in the expects - # block within the test - by that time, this code has already - # been executed because this GenServer is part of the staticly - # defined supervision tree in `application.ex`. - - value_of_foo = - if function_exported?(service(), :foo, 0) do - service().foo() - else - "#{inspect(service())} does not support foo" - end + + # SIDE EFFECT HERE!!! + value_of_foo = @interpretor.run(%Effect{m: Service, f: :foo, a: []}) {:noreply, value_of_foo} end diff --git a/test/support/interpreter.ex b/test/support/interpreter.ex new file mode 100644 index 0000000..90d7f42 --- /dev/null +++ b/test/support/interpreter.ex @@ -0,0 +1,7 @@ +defmodule Test.Interpreter do + alias Example.Effect + def run(%Effect{} = effect) do + IO.inspect effect, label: "test effect" + effect + end +end diff --git a/test/support/mock_service.ex b/test/support/mock_service.ex deleted file mode 100644 index ec7ee1b..0000000 --- a/test/support/mock_service.ex +++ /dev/null @@ -1,11 +0,0 @@ -# With a hard coded mock like this we can define the behaviour -# before the application starts up, but do we even need Mox at -# this point? -# -# defmodule Example.MockService do -# @behaviour Example.ServiceBehaviour -# -# def foo() do -# "hard coded says foo" -# end -# end diff --git a/test/worker_test.exs b/test/worker_test.exs index b734bfd..3271e94 100644 --- a/test/worker_test.exs +++ b/test/worker_test.exs @@ -1,33 +1,11 @@ defmodule Example.WorkerTest do use ExUnit.Case - import Mox + alias Example.Effect alias Example.Worker describe "default service" do test "returns default service foo" do - assert Worker.get_foo() =~ ~s(default says foo) - end - end - - describe "mocked service" do - setup do - # Normally you would add this to `test_helper.ex`, or `support/mocks.ex - Mox.defmock(Example.MockService, for: Example.ServiceBehaviour) - - Example.MockService - |> expect(:foo, fn -> "setup all says foo" end) - - :ok - end - - setup :verify_on_exit! - - test "returns mocked service foo" do - Example.MockService - |> expect(:foo, fn -> "mock says foo" end) - |> allow(self(), Process.whereis(Worker)) - - assert Worker.get_foo() =~ ~s(mock says foo) + assert Worker.get_foo() == %Effect{m: Example.Service, f: :foo, a: []} end end end From 03bad3414c74d61c6199f455f0a50c9439329904 Mon Sep 17 00:00:00 2001 From: Keith Salisbury Date: Sat, 29 Feb 2020 01:00:06 +0000 Subject: [PATCH 2/3] Interpreter & Effects module This version implements a slightly more robust Effects module, which some helpful functions for creating very simple effects. --- lib/example.ex | 13 --------- lib/example/effect.ex | 31 +++++++++++++++++++++ lib/example/interpreter.ex | 25 ++++++++++++++++- lib/example/product.ex | 12 +++++++++ lib/example/service.ex | 6 ----- lib/example/services/database.ex | 28 +++++++++++++++++++ lib/example/services/service_a.ex | 13 +++++++++ lib/example/services/service_b.ex | 20 ++++++++++++++ lib/example/worker.ex | 45 ++++++++++++++++++++++++++++--- test/example_test.exs | 8 ------ test/support/interpreter.ex | 11 +++++++- test/worker_test.exs | 21 ++++++++++++++- 12 files changed, 199 insertions(+), 34 deletions(-) create mode 100644 lib/example/product.ex delete mode 100644 lib/example/service.ex create mode 100644 lib/example/services/database.ex create mode 100644 lib/example/services/service_a.ex create mode 100644 lib/example/services/service_b.ex delete mode 100644 test/example_test.exs diff --git a/lib/example.ex b/lib/example.ex index fc5576a..7a88241 100644 --- a/lib/example.ex +++ b/lib/example.ex @@ -2,17 +2,4 @@ defmodule Example do @moduledoc """ Documentation for Example. """ - - @doc """ - Hello world. - - ## Examples - - iex> Example.hello() - :world - - """ - def hello do - :world - end end diff --git a/lib/example/effect.ex b/lib/example/effect.ex index f26f3c6..293d9b0 100644 --- a/lib/example/effect.ex +++ b/lib/example/effect.ex @@ -1,3 +1,34 @@ defmodule Example.Effect do + @moduledoc """ + Module for describing side effects + + Rather than litter the code with real side effects, why not place + descriptions of those side effects. Code is data, data is code, let the + side effects become the data. + """ + + alias __MODULE__ + + @type t :: %{m: module, f: atom, a: [any]} + defstruct [:m, :f, :a] + + def new(m, f, a \\ []), do: %__MODULE__{m: m, f: f, a: List.wrap(a)} + + defmacro effect(block) do + {{_, _, [{_, _, mod}, f]}, _, args} = block + + m = Module.concat(mod) + + quote bind_quoted: [m: m, f: f, args: args] do + Effect.new(m, f, args) + end + end + + defmacro __using__(_opts \\ []) do + quote do + require Effect + import Effect + end + end end diff --git a/lib/example/interpreter.ex b/lib/example/interpreter.ex index 6a6529b..9a87590 100644 --- a/lib/example/interpreter.ex +++ b/lib/example/interpreter.ex @@ -1,7 +1,30 @@ defmodule Example.Interpreter do alias Example.Effect + require Logger + + # def run(%Effect{m: m, f: f, a: a} = _effect) do + # timeout = 5000 + # + # task = + # Task.async(fn -> + # apply(m, f, a) + # end) + # + # case Task.yield(task, timeout) || Task.shutdown(task) do + # {:ok, result} -> + # result + # + # nil -> + # Logger.warn("Failed to get a result in #{timeout}ms") + # nil + # end + # end + def run(%Effect{m: m, f: f, a: a} = _effect) do - # IO.inspect effect, label: "real effect" apply(m, f, a) end + + # def run(%Effect{m: m, f: f, a: a} = _effect) do + # IO.inspect effect, label: "real effect" + # end end diff --git a/lib/example/product.ex b/lib/example/product.ex new file mode 100644 index 0000000..2298455 --- /dev/null +++ b/lib/example/product.ex @@ -0,0 +1,12 @@ +defmodule Product do + @default_name "default" + + @type t :: %{ + id: integer, + name: binary + } + + defstruct [:id, :name] + + def new(id), do: %__MODULE__{id: id, name: @default_name} +end diff --git a/lib/example/service.ex b/lib/example/service.ex deleted file mode 100644 index 92854d8..0000000 --- a/lib/example/service.ex +++ /dev/null @@ -1,6 +0,0 @@ -defmodule Example.Service do - def foo() do - Process.sleep 1000 - IO.inspect "real service says foo", label: "Example.Service" - end -end diff --git a/lib/example/services/database.ex b/lib/example/services/database.ex new file mode 100644 index 0000000..bd7b1c8 --- /dev/null +++ b/lib/example/services/database.ex @@ -0,0 +1,28 @@ +defmodule Example.Services.Database do + @moduledoc """ + Database service provide way to save and retreive values from a persistant + store + """ + + @spec get(integer) :: Product.t() | nil + def get(id) do + Process.sleep(1000) + + case id do + 1 -> + Product.new(1) + + _ -> + nil + end + end + + @spec update(Product.t() | any) :: :ok | :error + def update(%Product{id: id}) do + if id == 1 do + :ok + else + :error + end + end +end diff --git a/lib/example/services/service_a.ex b/lib/example/services/service_a.ex new file mode 100644 index 0000000..5a152c8 --- /dev/null +++ b/lib/example/services/service_a.ex @@ -0,0 +1,13 @@ +defmodule Example.Services.ServiceA do + @moduledoc """ + ServiceA provides a way to request data from external webserver + + The performance of ServiceA is somewhat sketchy... we don't know how long it + takes to get data, sometimes the data is available, sometimes it's not, and + sometimes the service doesn't even respond. + """ + def foo() do + Process.sleep(1000) + "real service says foo" + end +end diff --git a/lib/example/services/service_b.ex b/lib/example/services/service_b.ex new file mode 100644 index 0000000..812fdf0 --- /dev/null +++ b/lib/example/services/service_b.ex @@ -0,0 +1,20 @@ +defmodule Example.Services.ServiceB do + @moduledoc """ + ServiceB translates the data from ServiceA into something you can show to + your users - but only if you give it the right information. + + Failure to provide the right info means it gets stuck for a few minutes, + persumably searching some database or something, meanwhile, you're left + hanging. + """ + def bar(args) do + case args do + "real service says foo" -> + "you get the cookie, well done" + + _ -> + Process.sleep(1000) + "Sorry but you need to give me something..." + end + end +end diff --git a/lib/example/worker.ex b/lib/example/worker.ex index 039daad..24650cb 100644 --- a/lib/example/worker.ex +++ b/lib/example/worker.ex @@ -1,8 +1,7 @@ defmodule Example.Worker do use GenServer - alias Example.Effect - alias Example.Service + use Example.Effect @interpretor Application.get_env(:example, :interpretor, Example.Interpreter) @@ -14,15 +13,20 @@ defmodule Example.Worker do GenServer.call(__MODULE__, :get_foo) end + def get_bar() do + GenServer.call(__MODULE__, :get_bar) + end + def init(_init_arg) do initial_state = "no foo for you" {:ok, initial_state, {:continue, :get_foo_from_service}} end def handle_continue(:get_foo_from_service, _state) do - # SIDE EFFECT HERE!!! - value_of_foo = @interpretor.run(%Effect{m: Service, f: :foo, a: []}) + # value_of_foo = @interpretor.run(Effect.new(Services.ServiceA, :foo)) + s1 = effect(Example.Services.ServiceA.foo()) + value_of_foo = @interpretor.run(s1) {:noreply, value_of_foo} end @@ -30,4 +34,37 @@ defmodule Example.Worker do def handle_call(:get_foo, _from, state) do {:reply, state, state} end + + def handle_call(:get_bar, _from, state) do + # SIDE EFFECT HERE!!! + # value_of_bar = @interpretor.run(Effect.new(Services.ServiceB, :bar, state)) + s1 = effect(Example.Services.ServiceB.bar(state)) + + value_of_bar = @interpretor.run(s1) + + {:reply, value_of_bar, state} + end + + # Non-GenServer functions + + def update_product(id, new_name) do + # SIDE EFFECT! + # product = Services.Database.get(id) + # product = @interpretor.run(Effect.new(Services.Database, :get, id)) + s1 = effect(Example.Services.Database.get(id)) + + # TODO: Running s1 will return either a Product, or a nil. The existing + # side effect doesn't do anything to acknowledge this fact. + product = @interpretor.run(s1) + + # Functional core! + updated_product = %Product{product | name: new_name} + + # SIDE EFFECT! + # Services.Database.update(updated_product) + s2 = effect(Example.Services.Database.update(updated_product)) + + # _product = @interpretor.run(Effect.new(Services.Database, :update, updated_product)) + _product = @interpretor.run(s2) + end end diff --git a/test/example_test.exs b/test/example_test.exs deleted file mode 100644 index c7659ae..0000000 --- a/test/example_test.exs +++ /dev/null @@ -1,8 +0,0 @@ -defmodule ExampleTest do - use ExUnit.Case - doctest Example - - test "greets the world" do - assert Example.hello() == :world - end -end diff --git a/test/support/interpreter.ex b/test/support/interpreter.ex index 90d7f42..9dfc53e 100644 --- a/test/support/interpreter.ex +++ b/test/support/interpreter.ex @@ -1,7 +1,16 @@ defmodule Test.Interpreter do alias Example.Effect + + def run(%Example.Effect{a: [1], f: :get, m: Example.Services.Database}) do + Product.new(1) + end + + def run(%Example.Effect{a: _, f: :get, m: Example.Services.Database}) do + nil + end + def run(%Effect{} = effect) do - IO.inspect effect, label: "test effect" + # IO.inspect(effect, label: "test effect") effect end end diff --git a/test/worker_test.exs b/test/worker_test.exs index 3271e94..5ee5f21 100644 --- a/test/worker_test.exs +++ b/test/worker_test.exs @@ -2,10 +2,29 @@ defmodule Example.WorkerTest do use ExUnit.Case alias Example.Effect alias Example.Worker + alias Example.Services describe "default service" do test "returns default service foo" do - assert Worker.get_foo() == %Effect{m: Example.Service, f: :foo, a: []} + assert Worker.get_foo() == Effect.new(Services.ServiceA, :foo) + end + + test "returns the value of bar" do + assert Worker.get_bar() == + Effect.new(Services.ServiceB, :bar, [Effect.new(Services.ServiceA, :foo)]) + end + end + + describe "tests" do + @tag :wip + test "it works" do + expected_effect = %Example.Effect{ + m: Example.Services.Database, + f: :update, + a: [%Product{id: 1, name: "luxury product"}] + } + + assert Worker.update_product(1, "luxury product") == expected_effect end end end From 449d25b6140534a0a8be7b8f1830f4e78c28e7d6 Mon Sep 17 00:00:00 2001 From: Keith Salisbury Date: Thu, 12 Mar 2020 21:33:46 +0000 Subject: [PATCH 3/3] Provide Dynamic Interpreter Example --- lib/example/services/http.ex | 17 +++++++++++++++ lib/example/worker.ex | 10 +++++++++ test/support/interpreter.ex | 4 ++-- test/worker_test.exs | 42 ++++++++++++++++++++++++++++++------ 4 files changed, 64 insertions(+), 9 deletions(-) create mode 100644 lib/example/services/http.ex diff --git a/lib/example/services/http.ex b/lib/example/services/http.ex new file mode 100644 index 0000000..b23c629 --- /dev/null +++ b/lib/example/services/http.ex @@ -0,0 +1,17 @@ +defmodule Example.Services.Http do + @moduledoc """ + Web service + """ + + Application.ensure_all_started(:inets) + Application.ensure_all_started(:ssl) + + @spec fetch() :: {:ok, any, any} | {:error, any} + def fetch() do + # Now we can make request: + case :httpc.request(:get, {'http://www.mocky.io/v2/5e5a23e730000071001f0ade', []}, [], []) do + {:ok, {{'HTTP/1.1', 200, 'OK'}, headers, body}} -> {:ok, headers, body} + _ -> {:error, "Request failed"} + end + end +end diff --git a/lib/example/worker.ex b/lib/example/worker.ex index 24650cb..6e57cec 100644 --- a/lib/example/worker.ex +++ b/lib/example/worker.ex @@ -67,4 +67,14 @@ defmodule Example.Worker do # _product = @interpretor.run(Effect.new(Services.Database, :update, updated_product)) _product = @interpretor.run(s2) end + + def get_data() do + s1 = effect(Example.Services.Http.fetch()) + i1 = Application.get_env(:example, :interpretor2, Example.Interpreter) + + case i1.run(s1) do + {:ok, _headers, body} -> body + {:error, reason} -> reason + end + end end diff --git a/test/support/interpreter.ex b/test/support/interpreter.ex index 9dfc53e..f00055c 100644 --- a/test/support/interpreter.ex +++ b/test/support/interpreter.ex @@ -1,11 +1,11 @@ defmodule Test.Interpreter do alias Example.Effect - def run(%Example.Effect{a: [1], f: :get, m: Example.Services.Database}) do + def run(%Effect{a: [1], f: :get, m: Example.Services.Database}) do Product.new(1) end - def run(%Example.Effect{a: _, f: :get, m: Example.Services.Database}) do + def run(%Effect{a: _, f: :get, m: Example.Services.Database}) do nil end diff --git a/test/worker_test.exs b/test/worker_test.exs index 5ee5f21..4c428ff 100644 --- a/test/worker_test.exs +++ b/test/worker_test.exs @@ -1,8 +1,7 @@ defmodule Example.WorkerTest do use ExUnit.Case - alias Example.Effect - alias Example.Worker - alias Example.Services + use Example.Effect + alias Example.{Effect, Worker, Services} describe "default service" do test "returns default service foo" do @@ -15,11 +14,10 @@ defmodule Example.WorkerTest do end end - describe "tests" do - @tag :wip + describe "update_product/2" do test "it works" do - expected_effect = %Example.Effect{ - m: Example.Services.Database, + expected_effect = %Effect{ + m: Services.Database, f: :update, a: [%Product{id: 1, name: "luxury product"}] } @@ -27,4 +25,34 @@ defmodule Example.WorkerTest do assert Worker.update_product(1, "luxury product") == expected_effect end end + + describe "get_data/0" do + test "it works" do + defmodule TestSuccess do + def run(%Effect{f: :fetch, m: Services.Http}) do + {:ok, [], "here is result 2"} + end + end + + Application.put_env(:example, :interpretor2, TestSuccess) + + expected_effect = "here is result 2" + + assert Worker.get_data() == expected_effect + end + + test "it fails" do + defmodule TestFailure do + def run(%Effect{f: :fetch, m: Services.Http}) do + {:error, "There was an error"} + end + end + + Application.put_env(:example, :interpretor2, TestFailure) + + expected_effect = "There was an error" + + assert Worker.get_data() == expected_effect + end + end end