Creating users within a transaction

I’m attempting to create multiple users at once. If even one user fails to be created, I want the error to be reported and none of the users to be created. With some research, I came to find that Repo.transaction may be able to help me with that, although I’m struggling to get it working. I’m relatively unfamiliar with Ecto, so I’m hoping for some help.

I have users, a list of maps containing user attributes, being passed to add_users/1. Then each map is passed to add_user/1 in order to create them. Since I’d like to create all users if all are successful or return failed users without creating any, I though the best place for the transaction was in add_users/1.

I included what I’ve tried, and failed with, so far. Any help is appreciated.

When the code is like this:

  defp add_users(users) do
    Repo.transaction(fn -> Enum.map(users, &add_user/1) end)
  end

  defp add_user(%{name: name, username: username, email: email, password: password} = user_attrs) do
    {roles, user_attrs} = Map.pop(user_attrs, :roles)
    
    user = 
      %User{}
      |> User.changeset(user_attrs)
      
    with %User{} = user <- Repo.insert!(user) do
      add_roles(user, roles)
      add_extension(user, user_attrs)
    end
  end

I get:

** (Ecto.InvalidChangesetError) could not perform insert because changeset is invalid.
    (ecto) lib/ecto/repo/schema.ex:134: Ecto.Repo.Schema.insert!/4
    iex:203: ImportUsers.add_user/1
    (elixir) lib/enum.ex:1314: Enum."-map/2-lists^map/1-0-"/2
    (ecto) lib/ecto/adapters/sql.ex:620: anonymous fn/3 in Ecto.Adapters.SQL.do_transaction/3
    (db_connection) lib/db_connection.ex:1283: DBConnection.transaction_run/4
    (db_connection) lib/db_connection.ex:1207: DBConnection.run_begin/3
    (db_connection) lib/db_connection.ex:798: DBConnection.transaction/3

When the code is like this:

  defp add_users(users) do
    Repo.transaction(fn -> Enum.map(users, &add_user/1) end)
  end

  defp add_user(%{name: name, username: username, email: email, password: password} = user_attrs) do
    {roles, user_attrs} = Map.pop(user_attrs, :roles)
    
    user = 
      %User{}
      |> User.changeset(user_attrs)
      
    with {:ok, user} <- Repo.insert(user) do
      add_roles(user, roles)
      add_extension(user, user_attrs)
    end
  end

I get a tuple {:ok, [{:ok, user} | {:error, changeset}]}, which isn’t useful since there are users being created despite others failing.

When the code is like this:

  defp add_users(users) do
    Repo.transaction(fn -> Enum.map(users, &add_user/1) end)
  end

  defp add_user(%{name: name, username: username, email: email, password: password} = user_attrs) do
    {roles, user_attrs} = Map.pop(user_attrs, :roles)
    
    user = 
      %User{}
      |> User.changeset(user_attrs)
      
    Repo.transaction(fn ->
      with %User{} = user <- Repo.insert(user) do
        add_roles(user, roles)
        add_extension(user, user_attrs)
      end
    end)
  end

I get a database connection error:

** (DBConnection.ConnectionError) transaction rolling back
    (db_connection) lib/db_connection.ex:1531: DBConnection.fetch_info/1
    (db_connection) lib/db_connection.ex:1232: DBConnection.transaction_meter/3
    (db_connection) lib/db_connection.ex:798: DBConnection.transaction/3
    (elixir) lib/enum.ex:1314: Enum."-map/2-lists^map/1-0-"/2
    (elixir) lib/enum.ex:1314: Enum."-map/2-lists^map/1-0-"/2
    (ecto) lib/ecto/adapters/sql.ex:620: anonymous fn/3 in Ecto.Adapters.SQL.do_transaction/3
    (db_connection) lib/db_connection.ex:1283: DBConnection.transaction_run/4
    (db_connection) lib/db_connection.ex:1207: DBConnection.run_begin/3

Maybe try this sort of logic and stop using create! etc

Repo.transaction(fn ->
  Enum.reduce_while(users, [], fn user, added_users ->
    case add_user(user) do
      {:ok, added_user} -> {:cont, [added_user | added_users]}
      {:error, reason} -> {:halt, Repo.rollback(reason)}
    end
  end
end)

And your add_user/1 should be written like this

with {roles, params} = Map.pop(params, :roles),
     user = User.changeset(%User{}, params),
     {:ok, user} <- add_roles(user, roles),
     {:ok, user} <- add_extension(user, params) do
  {:ok, user}
end

Basically. Study Ecto transactions properly and study how with is used. Ecto.Repo — Ecto v3.5.8

2 Likes

Thanks for the advice. Although, I have one issue; Repo.rollback will quit the transaction as soon as it encounters the first failure. My understanding from reading the docs on Repo.transaction is that it will run through the entire transaction and then output all of the errors at the end without committing the successes. So for example, if I want to create three users where the expected results are fail, success, fail, I’d like to be informed of the two failures rather than just the first one caused by explicitly calling Repo.rollback. Am I misunderstanding something? Or is it simply not feasible in my case?

The issue is that if any of your INSERTs fail at the DB level, then the entire transaction is marked as invalid by the database and can’t be continued.

If you want to return a series of errors, I would split the problem. First, take your list of inputs and create changets. Then grab any invalid? changesets and if there are any, return an error to the end user. At this point the database hasn’t actually even been used.

Then if you have no invalid changesets, you can either insert all of them or even try to turn them into a set of values you could use with Repo.insert_all if you’re trying to optimize DB performance.

2 Likes

There are probably many ways to do this. Ecto.Multi has a nice function called run that might be of interest to you. Here are the docs: Ecto.Multi — Ecto v3.5.8

Basically, you have lots of control over the return value so you could do something like this:

    # Here is an example of one way you could achieve the thing you want, where every user is attempted to be added:
    users_attrs = [%{name: "Good Name"}, %{name: "%inv&lid ch&r&cters*"}, %{name: nil}]

    users_attrs
    |> Enum.with_index()
    |> Enum.reduce(Ecto.Multi.new(), fn {user_attrs, index}, acc_multi ->
      Ecto.Multi.run(acc_multi, String.to_atom("user-#{index}"), fn _repo, _results_so_far ->
        {:ok, Accounts.create_user(user_attrs)}
      end)
    end)
    |> Ecto.Multi.run(:ensure_all_ok, fn _repo, results_so_far ->
      case Enum.all?(Map.values(results_so_far), fn
             {:error, _changeset} -> false
             {:ok, user} -> true
           end) do
        true -> {:ok, true}
        false -> {:error, false}
      end
    end)
    |> Repo.transaction()

This will try to add all users to the database and then checks if they were all :ok. If any are not :ok then none of them are added.

The trick I am doing is in the line {:ok, Accounts.create_user(user_attrs)} which is returned by the Multi.run being called in the reduce loop. Since this is always returning a 2-tuple with first element :ok, the Mutli won’t break. It’s only in the second use of Multi.run where we ensure that all of the users were added, by pattern-matching on the actual return value of create_user.

You should also consider what @benwilson512 said about using changeset validation, to not hit the database so heavily.

2 Likes

Thanks for the response. This would be great if there wasn’t a unique_constraint on usernames and emails. So, unfortunately, without INSERTing all users with specified names, usernames, emails, and passwords will be labeled as valid, that is until the INSERT is attempted then I’ll be left in a similar position.

That definitely complicates things, because if you always relay on only the constraint to tell you about taken usernames, then it will always stop after the first error. That’s just how the database transactions work.

However, you could do an initial search for pre-existing usernames prior to inserting them. It isn’t a guarantee that the insert will work, but it can provide a good UX by usually providing all errors up front. One way or another though your transaction code needs to understand that whether you use insert or insert! the whole transaction is toast after the first failed insert.

1 Like

You guys should look at my answer. A database transaction is not the same as an Ecto transaction. Multi can do exactly what the OP wants.

I guess that’s where my initial misunderstanding of docs came from; for some reason I thought if the transaction was to create all users, then it would attempt to create all of them but not commit if any errors raise. I’ve had a “workaround” solution for a couple of days, but wanted to see if there was a better solution out there.

I apologise if I came off as rude for not responding to your initial suggestion. I hadn’t gotten a chance to play around with implementing it and wanted to do so before getting back to you. But I appreciate the time you took to answer my question. I will absolutely attempt to use your proposed solution and let you know how it goes.

1 Like

I’ve been trying to work with your suggestion but cannot. It fails on the first iteration of reduce:

** (FunctionClauseError) no function clause matching in Ecto.Multi.run/3

    The following arguments were given to Ecto.Multi.run/3:

        # 1
        %Ecto.Multi{names: #MapSet<[]>, operations: []}

        # 2
        :user_0

        # 3
        #Function<10.63633724/2 in InfinityOne.Accounts.ImportUsers.add_users/1>

    (ecto) lib/ecto/multi.ex:309: Ecto.Multi.run/3
    (elixir) lib/enum.ex:1925: Enum."-reduce/3-lists^foldl/2-0-"/3
    iex:137: ImportUsers.add_users/1

Any insights as to why?

Please paste your actual code. The third argument is incorrect and not like what I posted. It should be an anonymous function taking 2 arguments (repo and results_so_far).

1 Like

The only changes made were renaming results_so_far to changes_so_far and passing users to add_user, which still returns either {:ok, value} | {:error, value}:

  defp add_users(users) do
    users
    |> Enum.with_index()
    |> Enum.reduce(Multi.new(), fn {user_attrs, index}, multi_acc ->
      Multi.run(multi_acc, String.to_atom("user#{index}"), fn _repo, _changes_so_far -> 
        {:ok, add_user(user_attrs)}
      end)
    end)
    |> Multi.run(:ensure_all_ok, fn _repo, changes_so_far ->
      case Enum.all?(Map.values(changes_so_far), fn
             {:error, _changeset} -> false
             {:ok, user} -> true
           end) do
        true -> {:ok, true}
        false -> {:error, false}
      end
    end)
    |> Repo.transaction()
  end

  defp add_user(%{name: name, username: username, email: email, password: password} = user_attrs) do
    with {roles, user_attrs} = Map.pop(user_attrs, :roles),
         {:ok, user} <-
           %User{}
           |> User.changeset(user_attrs)
           |> Repo.insert(),
         _ <- add_roles(user, roles),
         _ <- add_extension(user, user_attrs) do
      {:ok, user}
    else
      {:error, changeset} = error ->
        error
    end
  end

It should work. Do you mind sharing the value of users? I wonder if your add_user function isn’t returning either {:ok, something} or {:error, something}.

This is an example test value for users:

users = [
  %{
    email: "onetest@example.com",
    name: "One Test",
    password: "one.test99",
    roles: ["user"],
    username: "one.test",
  },
  %{
    email: "onetest@example.com",
    name: "Two Test",
    password: "two99",
    roles: ["user"],
    username: "two",
  }
]

The expected results are [{:ok, %User{}}, {:error, #Ecto.changeset<>}], and I can confirm that is in fact what is returned.

I’m sorry but I cannot recreate your problem. What version of ecto are you using?

This project uses v2.1.6, and the docs for Ecto.Multi do not appear to be any different from the most recent version.

I have no idea why it is not working for you. I cannot recreate the problem. Have you definitely posted your code exactly as you have it? I have my suspicions that your actual code is different to what you posted here, because the error message has multi name :user_0 but the code you posted would produce :user0, with this part: String.to_atom("user#{index}")

Sorry for the confusion. I definitely copied and pasted the code as I have it. I made the change from String.to_atom("user#{index}") to String.to_atom("user_#{index}") (as you had it), just to ensure that it wasn’t the issue even though I didn’t believe that the naming would affect it. I simply copied the wrong error message. I just re-ran it so we can rest assured:

** (FunctionClauseError) no function clause matching in Ecto.Multi.run/3

    The following arguments were given to Ecto.Multi.run/3:

        # 1
        %Ecto.Multi{names: #MapSet<[]>, operations: []}

        # 2
        :user0

        # 3
        #Function<11.50973475/2 in InfinityOne.Accounts.ImportUsers.add_users/1>

    (ecto) lib/ecto/multi.ex:309: Ecto.Multi.run/3
    (elixir) lib/enum.ex:1925: Enum."-reduce/3-lists^foldl/2-0-"/3
    iex:137: ImportUsers.add_users/1

I am confounded and at a loss. I’m sorry it doesn’t work for you.