Skip to content
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

Allow delayed evaluation of attributes #408

Merged
merged 2 commits into from
Dec 24, 2020

Conversation

germsvel
Copy link
Collaborator

Closes #402, #385, and #373

The problem

People often run into this problem: they want a different email per account, but they do this:

build_pair(:account, email: build(:email))

The problem is that build/2 is just a function call. So the above is equivalent to this:

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:

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:

 %Account{
   status: fn account -> build(:status, private: account.private) 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.

We used to have a struct factory defined as `%{__struct__: FooBar}` but
it was confusing since the factory name was `struct_factory`.

So we change to be `foo_bar_factory` and create a module with a struct
called `FooBar`. This has the benefit of allowing us to assert
`%FooBar{}` in tests -- something we couldn't do with the previous
format.
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.
add :net_worth, :decimal
add(:name, :string)
add(:admin, :boolean)
add(:net_worth, :decimal)
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Autoformatter added all the parentheses.

add(:pub_number, :string)
end

create(unique_index(:publishers, [:pub_number]))
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added this uniqueness constraint to properly test the scenario in ecto where insert_pair(:account, publisher: build(:publisher)) would raise a constraint error.

@@ -1,6 +1,10 @@
defmodule ExMachinaTest do
use ExUnit.Case

defmodule FooBar do
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is part of a different commit, but the change is so small and we use it in this PR's tests, so I included it in this PR. This is the commit 648117c

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Proposal: Introduce build_lazy/2
1 participant