elixir-mongodb-driver icon indicating copy to clipboard operation
elixir-mongodb-driver copied to clipboard

Default inserts with `Repo.insert(%Schema{})` are failing as they to insert the id funciton

Open PragTob opened this issue 4 months ago • 4 comments

👋

Hi there and thanks for creating the mongodb-driver! 💚

Problem

How I think I'd wanna insert new records, given a repo that can write, fails:

iex(7)> Mongo.Repo.insert(%Mongo.User{name: "Tobi-san"})
[error] Unable to send request to database because of %Mongo.Error{message: "invalid document: &Mongo.object_id/0", code: nil, host: nil, fail_command: nil, error_labels: nil, resumable: false, retryable_reads: nil, retryable_writes: nil, not_writable_primary_or_recovering: nil, error_info: nil}
{:error,
 %Mongo.Error{
   message: "invalid document: &Mongo.object_id/0",
   code: nil,
   host: nil,
   fail_command: nil,
   error_labels: nil,
   resumable: false,
   retryable_reads: nil,
   retryable_writes: nil,
   not_writable_primary_or_recovering: nil,
   error_info: nil
 }}

Context

Pretty normal schema/repo (simplified/omitted attributes):

defmodule Mongo.User do
  use Mongo.Collection

  collection "users" do
    attribute :name, String.t()
  end
end
defmodule Mongo.Repo do
  use Mongo.Repo, otp_app: :my_app
end

Root Cause

I'm pretty sure this fails because per default _id is set to said function:

iex(9)> %Mongo.User{}
%Mongo.User{
  _id: &Mongo.object_id/0,
  # ....
}

And the insert code never calls it before inserting: https://github.com/zookzook/elixir-mongodb-driver/blob/91d6746f1a60f64aae872ae151f2b4e03b84de51/lib/mongo/repo.ex#L68-L76

Solution

I believe the insert code should check if _id stores a 0-arity function, and if so call it and replace the value of _id with it

It may also be intentional not to insert records like this directly, but I think it'd be nice. Hence I'm looking for guidance.

Workaround

Some sort of new function can be used as a workaround:

  def new_with_id(attrs \\ %{}) do
    struct!(%__MODULE__{_id: Mongo.object_id()}, attrs)
  end

PragTob avatar Sep 16 '25 09:09 PragTob

Image

PragTob avatar Sep 16 '25 09:09 PragTob

Thanks again for the detailed description.

It may also be intentional not to insert records like this directly, but I think it'd be nice. Hence I'm looking for guidance.

This could be the case. I need to check the code and the description.

zookzook avatar Sep 17 '25 13:09 zookzook

Basically, you don't need the Repo module. It contains only a few helper or convenience functions.

Each collection has a new/0 function to create a new structure with the default values. In your case, call the new/0 function of the User module.

iex [11:37 :: 3] > %Mongo.User{}
%Mongo.User{_id: &Mongo.object_id/0, name: nil}
iex [11:37 :: 4] > Mongo.User.new()
%Mongo.User{_id: #BSON.ObjectId<68ce75e74ae06ec43fad0eaa>, name: nil}

I always extend the collection with a new/1 function like this:

  def new(params) do
    struct!(new(), params)
  end

You can use the new function to create new/1 structs with some values:

iex [11:37 :: 7] > Mongo.User.new(%{name: "Rabbit"})
%Mongo.User{_id: #BSON.ObjectId<68ce76c94ae06ec43fca204c>, name: "Rabbit"}

The reason for this is that the client can create the Object-ID independent of the database. This allows the client to use the ID as a value for other operations as well, without having to wait for the insert operation.

zookzook avatar Sep 20 '25 09:09 zookzook

👋

Thanks!

I know we don't need to use the Repo module, but for matching the mental model with our other ecto stuff/convenience it's quite nice to have and use.

I think the quesiton remains though, if the above should be changed or would you say that the intended interface is calling new/0 (or a self defined new/1) first?

Just looking at it from a "works like ecto" perspective it'd be nice to have Repo.insert(%Schema{}) work imo :)

PragTob avatar Sep 21 '25 20:09 PragTob

I'm sorry, but there is more to do than calling the id function. The collection macro supports embedded documents, which can be nested. Therefore I introduced the new function that keeps an eye on the nested documents and calls the default functions if needed. I believe, it's no big deal to call the new function. Instead of using:

Repo.insert(%Schema{})

just call new:

Repo.insert(Schema.new())

I prefer the new function, otherwise we would have a small internal side effect where a transformation is performed in the insert function without it being apparent. I prefer an insert function that inserts exactly what has been passed to it.

Please excuse the long delay in my reply. I have just started a new job, which, along with my family commitments, has kept me quite busy.

zookzook avatar Dec 14 '25 18:12 zookzook