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

Insertion functions need to perform differently of creation functions #20

Open
zoedsoupe opened this issue Jan 22, 2024 · 1 comment
Open
Assignees
Labels
enhancement New feature or request
Milestone

Comments

@zoedsoupe
Copy link
Contributor

Currently we define:

  • insert/2
  • insert!/2
  • create/2
  • create!/2

That have the exactly same logic with the additional issue that creation functions call insertion functions. So let’s deep dive on this issue:

Semantics

Following Phoenix context and Ecto semantics, insertion function should receive the already built struct of the schema OR a %Ecto.Changeset{} that represents the current schema via the data attribute.

On the other hand, creation functions should receive raw parameters as maps, trigger the changeset function to parse this parameter and then leverage the insertion function.

There’s no idea of “creation” on Ecto.Repo, as it leverages direct database API functions.

What’s on currently?

Currently the insertion and creation functions have the exactly same logic, as:

      @impl SwissSchema
      def create(%{} = params, opts \\ []) do
        r = Keyword.get(opts, :repo, unquote(repo))
        insert = Function.capture(r, :insert, 2)
        default_changeset = Function.capture(__MODULE__, :changeset, 2)
        changeset = Keyword.get(opts, :changeset, default_changeset)

        struct(__MODULE__)
        |> changeset.(params)
        |> insert.(opts)
      end

      @impl SwissSchema
      def insert(%{} = params, opts \\ []) do
        repo = Keyword.get(opts, :repo, unquote(repo))
        insert = Function.capture(repo, :insert, 2)
        default_changeset = Function.capture(__MODULE__, :changeset, 2)
        changeset = Keyword.get(opts, :changeset, default_changeset)

        struct(__MODULE__)
        |> changeset.(params)
        |> insert.(opts)
      end
  1. get repo
  2. get the insert/2 function from the repo
  3. pass params trough a changeset/2 function
  4. builds a struct from __MODULE__, passes it to the changeset function and then to the local insert function

Proposal

I would like to make a suggestion:

  1. insertion functions would receive the already built struct or a changeset of the same schema (to match the library concept), something like:
@impl SwissSchema
def insert(source, opts \\ [])

def insert(%Ecto.Changeset{data: %mod{}} = changeset, opts) do
  case mod do
    __MODULE__ -> perform_insert(changeset, opts)
    _ -> {:error, :not_same_schema_module}
  end
end

def insert(%mod{} = struct, opts) when is_struct(struct) do
  case mod do
    __MODULE__ -> perform_insert(struct, opts)
    _ -> {:error, :not_same_schema_module}
  end
end

defp perform_insert(source, opts) do
  repo = Keyword.get(opts, :repo, unquote(
  insert = Function.capture(repo, :insert, 2)
  insert.(source, opts)
end
  1. creation functions would do the same logic as now, no need to change this api.
@joeljuca joeljuca added the enhancement New feature or request label Jan 29, 2024
@joeljuca joeljuca self-assigned this May 6, 2024
@joeljuca
Copy link
Owner

joeljuca commented May 6, 2024

FYI: I'm prioritizing this one for the next iterations I'll perform on the project. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants