embeds_many as map of maps

[Paulo Almeida]

Since the Riak Data Types don't include arrays, I'm implementing the serialization of embeds_many as a map of maps (where the keys of the outer map are the ids of the embedded model). 

The field is still represented as a list of models when the model struct is built, but it doesn't feel natural to manipulate it like this. Would it make sense to add an extra option to embeds_many, to make it store the embedded models as {:map, :map} instead of {:array, :map}?

I'm not sure this would be useful when using other databases though.

Paulo

[José Valim]

Paulo, we have decided that embeds_many will always be represented as a list in Elixir code, however, it may be represented in different ways in the source and also use different strategies. You can read more about strategies in the Strategies section in the embeds_many docs. Let us know if you have more questions!

José Valim

www.plataformatec.com.br

Skype: jv.ptec

Founder and Director of R&D

--
You received this message because you are subscribed to the Google Groups "elixir-ecto" group.
To unsubscribe from this group and stop receiving emails from it, send an email to elixir-ecto...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/elixir-ecto/f97e954c-d1a8-4850-a44f-01cb5b36671e%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[Paulo Almeida]

José,

The adapter can convert the list into a map of maps when serializing to store the model. And the limitation that the order of elements in the list is not saved is something that can be handled at the application level using an `order` field.

Some related questions:

The strategy is simply a tag in the ecto type (1) and it's up to the adapter to use it? Or does it affect the behaviour of functions in Ecto.Changeset (put_change, etc.)?

Also, are we restricted to have auto-generated ids in embedded models. I'd like to set the ids of embedded models (in a embeds_many field), since the id is based on some user input. But keep getting 

** (Ecto.UnmachedRelationError) attempted to update one of the models with:

...

but could not find matching key in:

[]

(1)

{:embed,

 %Ecto.Embedded{cardinality: :many, field: :items, on_cast: :changeset,

  on_delete: :fetch_and_delete, on_replace: :delete, owner: Weather,

  related: Item, strategy: :replace}}

[José Valim]

The strategy is simply a tag in the ecto type (1) and it's up to the adapter to use it? Or does it affect the behaviour of functions in Ecto.Changeset (put_change, etc.)?

It is all adapter specific. It doesn't affect Ecto's behaviour. 

 

Also, are we restricted to have auto-generated ids in embedded models. I'd like to set the ids of embedded models (in a embeds_many field), since the id is based on some user input. But keep getting 

They must be auto-generated. When you send them through the form, it can only be used to update an existing one, otherwise it can be a security issue (not for embeds but for assocs). For this reason, maybe we could lift this restriction, but it there a strong reason you need to do such in the client?

[Paulo Almeida]

Here's a concrete example:

I'm using the model below to keep a log of SMS sent to a gateway and to keep track of delivery reports.

The ShortMessage record will be created, initially with an empty list of segments. If the SMS text is too long, it will have multiple segments. 

For each segment, the gateway will then invoke a callback whenever there is a status change (1st when segment is delivered to network, then when delivered to handset). Each segment is uniquely identified by an ID assigned by the gateway.

Since the updates will all happen concurrently, and the writes can all hit different Riak nodes at the same time, the only way this will merge correctly on the server (using the Riak map CRDT merge logic) is if I use the gateway assigned segment ID, not a locally auto-generated id.

defmodule ShortMessage do

  use Ecto.Model

  @primary_key {:id, :binary_id, autogenerate: true}

  schema "short_message" do

    field :from

    field :to

    field :text

    field :num_segments, :integer, default: 0

    field :gateway

    embeds_many :segments, __MODULE__.Segment

    timestamps inserted_at: :created_at, updated_at: :status_date

  end

  defmodule Segment do

    use Ecto.Model

    embedded_schema do

      field :status

      field :error_code

      field :status_date

    end

  end

end

[Michał Muskała]

> They must be auto-generated. When you send them through the form, it can
> only be used to update an existing one, otherwise it can be a security issue
> (not for embeds but for assocs). For this reason, maybe we could lift this
> restriction, but it there a strong reason you need to do such in the client?

I think we could change the code to allow manually generated ids on
insert. If I understand the security issue correctly, it's mostly
concerned with updates. Is this right?
We could allow this only for models that doesn't include
autogenerate:true for their primary keys.

[Paulo Almeida]

The restriction makes sense for assocs, but for embeds the boundary is always the main model.

If the embedded model is declared with autogenerate: false, maybe the restriction could be lifted, giving the programmer the option to decide.

[José Valim]

Thanks Paulo for the example.

> I think we could change the code to allow manually generated ids on

insert. If I understand the security issue correctly, it's mostly
concerned with updates. Is this right?

The security issue won't apply as long as we are issuing inserts and not updates.

So we could simply add the model as insert if we don't find a matching id. However, it is still up to the child changeset to decide if they should absorb the ID or not (by adding it as a required or optional field on cast).

Does it sound reasonable? Michał, could you please send a PR for us to further discuss this?

José Valim

www.plataformatec.com.br

Skype: jv.ptec

Founder and Director of R&D

--
You received this message because you are subscribed to the Google Groups "elixir-ecto" group.
To unsubscribe from this group and stop receiving emails from it, send an email to elixir-ecto...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/elixir-ecto/CAGAFNpn3oLpeohM%3DUTqRYK9-8ViC1%3DFhc2cHN4N1R7pAvccT7w%40mail.gmail.com.

[José Valim]

Paulo,

We have pushed a commit to master that addresses your concern. Will you give it a try and let us know before we make a new release? Thank you.

José Valim

www.plataformatec.com.br

Skype: jv.ptec

Founder and Director of R&D

[Paulo Almeida]

José,

Just tried the new commit and was able to manually set the id of the embedded model (both in embeds_one and embeds_many fields).

But I did notice that it's not possible to append items to a list (of embeds_many field). It's not related to manually setting the id though. I was able to replicate the error in the mongdb adapter:

Steps to reproduce:

iex> weather = Repo.get Weather, "1d4197db0398eb193c5315cd"

iex> changeset = change(weather) |> put_change(:items, [%Item{name: "Item 1", order: 0}])

iex> Repo.update! changeset

iex> weather = Repo.get Weather, "1d4197db0398eb193c5315cd"

iex> changeset = change(weather) |> put_change(:items, weather.items ++ [%Item{name: "Item 2", order: 1}])

iex> Repo.update! changeset                                                                               

** (ArgumentError) cannot dump embed `items`, invalid value: {:ok, %Ecto.Changeset{action: :update, changes: %{}, constraints: [], errors: [], filters: %{}, model: %Item{__meta__: #Ecto.Schema.Metadata<:loaded>, id: "1d7ed4200398eb4d6799edee", name: "Item 1", order: 0}, optional: [], params: nil, repo: nil, required: [], types: %{id: :binary_id, name: :string, order: :integer}, valid?: true, validations: []}}

      (ecto) lib/ecto/type.ex:374: Ecto.Type.dump_embed/5

      (ecto) lib/ecto/type.ex:344: anonymous fn/6 in Ecto.Type.dump_embed/3

    (elixir) lib/enum.ex:1261: Enum."-reduce/3-lists^foldl/2-0-"/3

      (ecto) lib/ecto/type.ex:343: Ecto.Type.dump_embed/3

      (ecto) lib/ecto/query/planner.ex:29: anonymous fn/6 in Ecto.Query.Planner.fields/4

    (stdlib) lists.erl:1261: :lists.foldl/3

      (ecto) lib/ecto/query/planner.ex:21: Ecto.Query.Planner.fields/4

      (ecto) lib/ecto/repo/model.ex:116: anonymous fn/10 in Ecto.Repo.Model.update/4

[Michał Muskała]

Paulo,

I know where the problem is. I'll fix it today. Thank you for reporting.

Michał.

> https://groups.google.com/d/msgid/elixir-ecto/60d89afe-33b0-491b-b051-e5c936099ff9%40googlegroups.com.

[Michał Muskała]

Paulo,

This issue if fixed in master. It would be great if you could check if
it works correctly. Thank you!

Michał.

[Paulo Almeida]

Thanks Michał! Confirming that updates on an embeds_many field are working as expected.