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

Cast polymorphic embeds #407

Closed
mathieuprog opened this issue Dec 22, 2020 · 10 comments
Closed

Cast polymorphic embeds #407

mathieuprog opened this issue Dec 22, 2020 · 10 comments

Comments

@mathieuprog
Copy link

Users of the polymorphic embed library are working with ex_machina for their tests.

The library introduces a new function cast_polymorphic_embed/2 for casting fields of type {:parameterized, PolymorphicEmbed, options}.

Would it be possible to allow somehow to customize the Ecto strategy in order for us to add the casting of polymorphic embeds?

See here what we'd need to hook into https://github.com/mathieuprog/polymorphic_embed/blob/d1c9b357f81e209858059db210362900ae2186e0/lib/ex_machina/ecto_strategy.ex#L22
("Change Start" - "Change End" blocks)

@germsvel
Copy link
Collaborator

germsvel commented Mar 9, 2021

@mathieuprog thanks for opening this. I don't know right now how we could allow customization of the Ecto strategy in order to allow for custom casting of polymorphic embeds.

Typically, if we need to change the strategy, I think it's okay to create a custom strategy, like you've done. But I understand the desire to use the default Ecto strategy with some configuration.

I'll try to think through and consider if there's a good way to do that. But if you have some idea in mind as to how we could do this, I'd love to hear it.

@jmpressman
Copy link

jmpressman commented Apr 1, 2021

Going off of the ecto strategy provided by @mathieuprog, changing the cast_polymorphic_embeds and schema_polymorphic_embeds functions to the following seemed to do the trick for us:

defp cast_polymorphic_embeds(%{__struct__: schema} = struct) do
  schema
  |> schema_polymorphic_embeds()
  |> Enum.reduce(struct, fn embed_key, struct ->
    embed_type = schema.__schema__(:type, embed_key)
    value = Map.get(struct, embed_key)

    casted_value =
      case embed_type do
        {:parameterized, PolymorphicEmbed, options} ->
          load_polymorphic_schema(value, options)

        {:array, {:parameterized, PolymorphicEmbed, options}} ->
          Enum.map(value, fn el ->
            load_polymorphic_schema(el, options)
          end)
      end

    Map.put(struct, embed_key, casted_value)
  end)
end

defp load_polymorphic_schema(data, options) do
  if is_struct(data) do
    data
  else
    {:ok, embed} = PolymorphicEmbed.load(data, nil, options)

    embed
  end
end

defp schema_polymorphic_embeds(schema) do
  Enum.filter(schema.__schema__(:fields), fn field ->
    case schema.__schema__(:type, field) do
      {:parameterized, PolymorphicEmbed, _options} -> true
      {:array, {:parameterized, PolymorphicEmbed, _options}} -> true
      _ -> false
    end
  end)
end

@mathieuprog
Copy link
Author

Typically, if we need to change the strategy, I think it's okay to create a custom strategy, like you've done. But I understand the desire to use the default Ecto strategy with some configuration.

A custom strategy here involves copy-pasting the whole ExMachina.Ecto module because it uses ExMachina.EctoStrategy (hardcoded), as well as the ExMachina.EctoStrategy and edit it. It's way too much copy-paste.

Here are 3 possibilities I have thought about:
mathieuprog/polymorphic_embed#26 (comment)

@germsvel
Copy link
Collaborator

@mathieuprog thanks for linking to those potential solutions. I haven't fully considered how we'd implement that in ExMachina's side, but just from looking at the suggestions, I think option 1 would be the easiest to implement, and it would allow for other casting of custom types as people have need.

Just quoting here option 1 from your link:

  1. Use of config in the main application
config :ex_machina, :cast_custom_types, MyApp.ExMachina.CastCustomTypes

MyApp.ExMachina.CastCustomTypes would have to implement a behaviour such as:

defmodule MyApp.ExMachina.CastCustomTypes do
  @behaviour ExMachina.CastCustomTypes
  
  @impl true
  def cast_custom_types(struct) do
    struct
    |> PolymorphicEmbed.ExMachina.CastCustomTypes.cast_custom_types()
    |> OtherLibrary.ExMachina.CastCustomTypes.cast_custom_types()
  end

  @impl true
  def schema_custom_types(struct) do
    PolymorphicEmbed.ExMachina.CastCustomTypes.schema_custom_types(struct)
      ++ OtherLibrary.ExMachina.CastCustomTypes.schema_custom_types(struct)
  end
end

defmodule PolymorphicEmbed.ExMachina.CastCustomTypes do
  @behaviour ExMachina.CastCustomTypes
  
  @impl true
  def cast_custom_types(struct) do
    cast_polymorphic_embeds(struct)
  end

  @impl true
  def schema_custom_types(struct) do
    schema_polymorphic_embeds(struct)
  end
end

This would be the easiest to code on ex_machina's side because the configuration is globally accessible.

I imagine we could just add one more function at the end of the cast function to call cast_custom_types passing the struct, and that would reduce over all the configured :cast_custom_types.

  defp cast(record) do
    record
    |> cast_all_fields()
    |> cast_all_embeds()
    |> cast_all_assocs()
    |> cast_custom_types()
  end

  defp cast_custom_types(struct) do
    custom_types = Application.get_env(:ex_machina, :cast_custom_types, [])
    Enum.reduce(custom_types, struct, fn type_module, struct -> 
        type_module.cast_custom_type(struct)
    end)
  end

Is that what you had in mind?

@mathieuprog
Copy link
Author

A function cast_custom_types won't be enough, that's why I suggested a behaviour to implement.
And that's because the function cast_all_fields/1 will include the custom type and try to cast it.

This:

defp schema_fields(schema) do
  (schema_non_virtual_fields(schema) -- schema_embeds(schema))
end

needs to be changed to this:

defp schema_fields(schema) do
  schema_non_virtual_fields(schema) -- schema_embeds(schema) -- # the custom type fields from `schema_custom_types/1`
end

@drselump14
Copy link

Hi, any update on this issue?

@tyhill3
Copy link

tyhill3 commented Jan 4, 2024

Hello. Any update on this?

@egeersoz
Copy link

Hi there. Any update on this? It was marked as completed on Jan 17th but I didn't see a PR for it.

@egeersoz
Copy link

Bump

@mathieuprog
Copy link
Author

@egeersoz I think the following needs to be implemented: #407 (comment)

Which involves adding an option in ex_machina:

config :ex_machina, :cast_custom_types, MyApp.ExMachina.CastCustomTypes

where CastCustomTypes is a behaviour.

And Ex Machina needs to use the behaviour at these locations:
https://github.com/mathieuprog/polymorphic_embed/blob/d1c9b357f81e209858059db210362900ae2186e0/lib/ex_machina/ecto_strategy.ex#L22
(Look for Change Start)

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

No branches or pull requests

7 participants