From d24268257b337476e20262196ef8fc2a041adf1a Mon Sep 17 00:00:00 2001 From: German Velasco Date: Tue, 8 Dec 2020 05:55:49 -0500 Subject: [PATCH] Allow delayed evaluation of attributes The problem ------------ People often run into this problem: they want a different email per account, but they do this: ```elixir build_pair(:account, email: build(:email)) ``` The problem is that `build/2` is just a function call. So the above is equivalent to this: ```elixir email = build(:email) build_pair(:account, email: email) # same email ``` In other words, we get the same email factory for all of the accounts. That's especially confusing if we're using a `sequence` in the `email` factory. The problem is made worse when using it with Ecto. We can imagine the following scenario: ```elixir insert_pair(:account, user: build(:user)) ``` If the user factory has a uniqueness constraint, `insert_pair/2` will raise an error because we'll try to insert a user with the same value (even if using a sequence). Solution -------- The solution is to delay evaluation of the attributes. We do this allowing attributes to be functions. The trick then lies in the `build/2` function. We make it a terminal function in that it will evaluate any lazy attributes recursively. To do that, we update the `build/2` function to evaluate function attributes after merging any passed-in attributes. Previous implementations tried to solve the issue of delayed evaluation by introducing a `build_lazy/2` function. One of those was a simple alias to an anonymous function `fn -> build(:factory_name) end`. The other was a more complex approach that introduced a new private struct `%ExMachina.InstanceTemplate{}` to hold the data necessary to build the instance of that factory. We opt for the simpler approach because: - (a) it leaves room for flexibility in the future (we can add something like `build_lazy` alias if we want), and - (b) it opens the door for allowing the parent factory to be passed into the anonymous function in a factory definition: ```elixir def account_factory do %Account{ status: fn account -> build(:status, private: account.private) end } end ``` Not interacting with "full-control" factories --------------------------------------------- We opt for not evaluating lazy attributes in "full-control" factories. The whole point of allowing users to have full control of their factory attributes is for them to do with them what they will. We do expose a `evaluate_lazy_attributes/1` helper function, just like we expose a `merge_attributes/2` function so that users can emulate ExMachina's default behavior. --- README.md | 50 ++++++++++++- lib/ex_machina.ex | 78 +++++++++++++++++++-- priv/test_repo/migrations/1_migrate_all.exs | 28 +++++--- test/ex_machina/ecto_test.exs | 19 +++++ test/ex_machina_test.exs | 78 +++++++++++++++++++-- test/support/models/publisher.ex | 3 +- test/support/test_factory.ex | 4 +- 7 files changed, 233 insertions(+), 27 deletions(-) diff --git a/README.md b/README.md index b0d296b..0705564 100644 --- a/README.md +++ b/README.md @@ -165,6 +165,46 @@ string_params_for(:comment, attrs) string_params_with_assocs(:comment, attrs) ``` +## Delayed evaluation of attributes + +`build/2` is a function call. As such, it gets evaluated immediately. So this +code: + + insert_pair(:account, user: build(:user)) + +Is equivalent to this: + + user = build(:user) + insert_pair(:account, user: user) # same user for both accounts + +Sometimes that presents a problem. Consider the following factory: + + def user_factory do + %{name: "Gandalf", email: sequence(:email, "gandalf#{&1}@istari.com")} + end + +If you want to build a separate `user` per `account`, then calling +`insert_pair(:account, user: build(:user))` will not give you the desired +result. + +In those cases, you can delay the execution of the factory by passing it as an +anonymous function: + + insert_pair(:account, user: fn -> build(:user) end) + +You can also do that in a factory definition: + + def account_factory do + %{user: fn -> build(:user) end} + end + +You can even accept the parent record as an argument to the function: + + def account_factory do + %{user: fn account -> build(:user, vip: account.premium) end} + end + + ## Full control of factory By default, ExMachina will merge the attributes you pass into build/insert into @@ -181,13 +221,17 @@ def custom_article_factory(attrs) do title: title } - # merge attributes at the end to emulate ExMachina default behavior - merge_attributes(article, attrs) + # merge attributes and evaluate lazy attributes at the end to emulate + # ExMachina's default behavior + article + |> merge_attributes(attrs) + |> evaluate_lazy_attributes() end ``` **NOTE** that in this case ExMachina will _not_ merge the attributes into your -factory, and you will have to do this on your own if desired. +factory, and it will not evaluate lazy attributes. You will have to do this on +your own if desired. ### Non-map factories diff --git a/lib/ex_machina.ex b/lib/ex_machina.ex index 34717dc..8908d0d 100644 --- a/lib/ex_machina.ex +++ b/lib/ex_machina.ex @@ -151,7 +151,7 @@ defmodule ExMachina do This will defer to the `[factory_name]_factory/0` callback defined in the factory module in which it is `use`d. - ## Example + ### Example def user_factory do %{name: "John Doe", admin: false} @@ -163,12 +163,22 @@ defmodule ExMachina do # Returns %{name: "John Doe", admin: true} build(:user, admin: true) + ## Full control of a factory's attributes If you want full control over the factory attributes, you can define the - factory with `[factory_name]_factory/1`. Note that you will need to merge the - attributes passed if you want to emulate ExMachina's default behavior. + factory with `[factory_name]_factory/1`, taking in the attributes as the first + argument. - ## Example + Caveats: + + - ExMachina will no longer merge the attributes for your factory. If you want + to do that, you can merge the attributes with the `merge_attributes/2` helper. + + - ExMachina will no longer evaluate lazy attributes. If you want to do that, + you can evaluate the lazy attributes with the `evaluate_lazy_attributes/1` + helper. + + ### Example def article_factory(attrs) do title = Map.get(attrs, :title, "default title") @@ -176,8 +186,11 @@ defmodule ExMachina do article = %Article{title: title, slug: slug} + article # merge attributes on your own - merge_attributes(article, attrs) + |> merge_attributes(attrs) + # evaluate any lazy attributes + |> evaluate_lazy_attributes() end # Returns %Article{title: "default title", slug: "default-title"} @@ -192,6 +205,7 @@ defmodule ExMachina do @doc false def build(module, factory_name, attrs \\ %{}) do attrs = Enum.into(attrs, %{}) + function_name = build_function_name(factory_name) cond do @@ -199,7 +213,9 @@ defmodule ExMachina do apply(module, function_name, [attrs]) factory_without_attributes_defined?(module, function_name) -> - apply(module, function_name, []) |> merge_attributes(attrs) + apply(module, function_name, []) + |> merge_attributes(attrs) + |> evaluate_lazy_attributes() true -> raise UndefinedFactoryError, factory_name @@ -245,6 +261,56 @@ defmodule ExMachina do def merge_attributes(%{__struct__: _} = record, attrs), do: struct!(record, attrs) def merge_attributes(record, attrs), do: Map.merge(record, attrs) + @doc """ + Helper function to evaluate lazy attributes that are passed into a factory. + + ## Example + + # custom factory + def article_factory(attrs) do + %{title: "title"} + |> merge_attributes(attrs) + |> evaluate_lazy_attributes() + end + + def author_factory do + %{name: sequence("gandalf")} + end + + # => returns [ + # %{title: "title", author: %{name: "gandalf0"}, + # %{title: "title", author: %{name: "gandalf0"} + # ] + build_pair(:article, author: build(:author)) + + # => returns [ + # %{title: "title", author: %{name: "gandalf0"}, + # %{title: "title", author: %{name: "gandalf1"} + # ] + build_pair(:article, author: fn -> build(:author) end) + """ + @spec evaluate_lazy_attributes(struct | map) :: struct | map + def evaluate_lazy_attributes(%{__struct__: record} = factory) do + struct!( + record, + factory |> Map.from_struct() |> do_evaluate_lazy_attributes(factory) + ) + end + + def evaluate_lazy_attributes(attrs) when is_map(attrs) do + do_evaluate_lazy_attributes(attrs, attrs) + end + + defp do_evaluate_lazy_attributes(attrs, parent_factory) do + attrs + |> Enum.map(fn + {k, v} when is_function(v, 1) -> {k, v.(parent_factory)} + {k, v} when is_function(v) -> {k, v.()} + {_, _} = tuple -> tuple + end) + |> Enum.into(%{}) + end + @doc """ Builds two factories. diff --git a/priv/test_repo/migrations/1_migrate_all.exs b/priv/test_repo/migrations/1_migrate_all.exs index a9c6f88..c6455f6 100644 --- a/priv/test_repo/migrations/1_migrate_all.exs +++ b/priv/test_repo/migrations/1_migrate_all.exs @@ -3,23 +3,29 @@ defmodule ExMachina.TestRepo.Migrations.MigrateAll do def change do create table(:users) do - add :name, :string - add :admin, :boolean - add :net_worth, :decimal + add(:name, :string) + add(:admin, :boolean) + add(:net_worth, :decimal) end + create table(:publishers) do + add(:pub_number, :string) + end + + create(unique_index(:publishers, [:pub_number])) + create table(:articles) do - add :title, :string - add :author_id, :integer - add :editor_id, :integer - add :publisher_id, :integer - add :visits, :decimal + add(:title, :string) + add(:author_id, :integer) + add(:editor_id, :integer) + add(:publisher_id, :integer) + add(:visits, :decimal) end create table(:comments) do - add :article_id, :integer - add :author, :map - add :links, {:array, :map}, default: [] + add(:article_id, :integer) + add(:author, :map) + add(:links, {:array, :map}, default: []) end end end diff --git a/test/ex_machina/ecto_test.exs b/test/ex_machina/ecto_test.exs index 354fb6a..b944e5f 100644 --- a/test/ex_machina/ecto_test.exs +++ b/test/ex_machina/ecto_test.exs @@ -1,6 +1,8 @@ defmodule ExMachina.EctoTest do use ExMachina.EctoCase + alias ExMachina.Article + alias ExMachina.Publisher alias ExMachina.TestFactory alias ExMachina.User @@ -65,6 +67,23 @@ defmodule ExMachina.EctoTest do test "insert_list/3 handles the number 0" do assert [] = TestFactory.insert_list(0, :user) end + + test "lazy records get evaluated with insert/2 and insert_* functions" do + assert %Article{publisher: %Publisher{}} = + TestFactory.insert(:article, publisher: fn -> TestFactory.build(:publisher) end) + + [%Article{publisher: publisher1}, %Article{publisher: publisher2}] = + TestFactory.insert_pair(:article, publisher: fn -> TestFactory.build(:publisher) end) + + assert publisher1 != publisher2 + + [publisher1, publisher2, publisher3] = + TestFactory.insert_list(3, :article, publisher: fn -> TestFactory.build(:publisher) end) + + assert publisher1.author != publisher2.author + assert publisher2.author != publisher3.author + assert publisher3.author != publisher1.author + end end describe "params_for/2" do diff --git a/test/ex_machina_test.exs b/test/ex_machina_test.exs index 8bd87bb..addec7f 100644 --- a/test/ex_machina_test.exs +++ b/test/ex_machina_test.exs @@ -16,6 +16,27 @@ defmodule ExMachinaTest do } end + def profile_factory do + %{ + username: sequence("username"), + user: build(:user) + } + end + + def account_factory do + %{ + private: true, + profile: fn -> build(:profile) end + } + end + + def admin_account_factory do + %{ + admin: true, + profile: fn account -> build(:profile, admin: account.admin) end + } + end + def email_factory do %{ email: sequence(:email, &"me-#{&1}@foo.com") @@ -101,17 +122,57 @@ defmodule ExMachinaTest do end test "build/2 allows factories to have full control of provided arguments" do - assert Factory.build(:comment, name: "James") == %{ - author: "James Doe", - username: "James-0", - name: "James" - } + comment = Factory.build(:comment, name: "James") + + assert %{author: "James Doe", name: "James"} = comment + assert String.starts_with?(comment[:username], "James-") end test "build/2 allows custom (non-map) factories to be built" do assert Factory.build(:room_number, floor: 5) == "500" assert Factory.build(:room_number, floor: 5) == "501" end + + test "build/2 accepts anonymous functions for a factory's attributes" do + account = Factory.build(:account) + + assert %{username: _} = account.profile + end + + test "build/2 accepts anonymous functions that use parent record in factory's definition" do + assert %{profile: %{admin: true}} = Factory.build(:admin_account, admin: true) + assert %{profile: %{admin: false}} = Factory.build(:admin_account, admin: false) + end + + test "build/2 can take anonymous functions for attributes" do + user = Factory.build(:user, foo_bar: fn -> Factory.build(:foo_bar) end) + + assert %FooBar{} = user.foo_bar + end + + test "build/2 does not evaluate lazy attributes when factory definition has full control" do + comment = Factory.build(:comment, name: "James", user: fn -> Factory.build(:user) end) + + assert is_function(comment.user) + assert %{id: 3, name: "John Doe", admin: false} = comment.user.() + end + + test "build/2 recursively builds nested lazy attributes" do + lazy_profile = fn -> Factory.build(:profile, user: fn -> Factory.build(:user) end) end + account = Factory.build(:account, profile: lazy_profile) + + assert %{username: _} = account.profile + assert %{name: "John Doe", admin: false} = account.profile.user + end + + test "build/2 lazily evaluates an attribute that is a list" do + user = Factory.build(:user, profiles: fn -> [Factory.build(:profile)] end) + + profile = hd(user.profiles) + + assert Map.has_key?(profile, :username) + assert Map.has_key?(profile, :user) + end end describe "build_pair/2" do @@ -126,6 +187,13 @@ defmodule ExMachinaTest do assert records == [expected_record, expected_record] end + + test "build_pair/2 recursively builds many nested lazy attributes" do + lazy_profile = fn -> Factory.build(:profile, user: fn -> Factory.build(:user) end) end + [account1, account2] = Factory.build_pair(:account, profile: lazy_profile) + + assert account1.profile.username != account2.profile.username + end end describe "build_list/3" do diff --git a/test/support/models/publisher.ex b/test/support/models/publisher.ex index 4cbdc4e..689de7b 100644 --- a/test/support/models/publisher.ex +++ b/test/support/models/publisher.ex @@ -1,6 +1,7 @@ defmodule ExMachina.Publisher do use Ecto.Schema - schema "users" do + schema "publishers" do + field(:pub_number, :string) end end diff --git a/test/support/test_factory.ex b/test/support/test_factory.ex index a967c37..303ecbc 100644 --- a/test/support/test_factory.ex +++ b/test/support/test_factory.ex @@ -18,7 +18,9 @@ defmodule ExMachina.TestFactory do end def publisher_factory do - %ExMachina.Publisher{} + %ExMachina.Publisher{ + pub_number: sequence("PUB_23") + } end def article_factory do