Default inserts with `Repo.insert(%Schema{})` are failing as they to insert the id funciton
👋
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
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.
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.
👋
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 :)
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.