Using Repo functions outside of context modules: Anti-Pattern?

I read the Hexdocs about contexts in Phoenix. Now that I revisit my own code, across different projects, I notice that I sometimes use Repo functions outside my context modules.

Like so:

# example 1
    item =
      conn.params["id"]
      |> SomeContext.get_item!()
      |> Repo.preload(:users)

# example 2
    item =
      params["id"]
      |> SomeContext.get_item!()
      |> Repo.preload([users: from(u in User, select: u.username)])

Is that an anti-pattern?

My thoughts.

  1. It reads easily the way it is.
  2. But I don’t like that I have to import Ecto.Query, only: [from: 2], warn: false and alias TodayCostory.Repo into my controller/LiveView to be able to use the Repo functions.
  3. In some cases I would get quit a few additional functions inside my contexts. Taking the examples from above: get_item_with_users!() and get_item_with_usernames!(). Seems somewhat unnecessary.
  4. But maybe the benefits of having a context, and using it without compromise, is ultimately greater than the potential downsides. Most importantly, maybe, the benefit of creating greater separation of concerns.
1 Like

I typically try to keep my repo function calls in my contexts. I my apps, I’d write your example code like this:

item =
  conn.params["id"]
  |> SomeContext.get_item!()
  |> SomeContext.preload_item_users()

Sometimes the preloads can get complex (eg, preload some deep associations and sort everything correctly), so putting them somewhere outside of the UI code makes them easier to find for later reuse.

3 Likes

as @eahanson mentioned, keep Repo calls in their context. Helps you define boundaries too! Good question!

1 Like

I’m firmly in the camp of keeping Repo calls inside contexts and, more specifically, only in context files (or sub-contexts). For example, as you probably already know, they don’t belong in schemas.

I usually do this type of thing to provide options to the client:

def list_posts(opts \\ []) do
  preload = Keyword.get(opts, :preload)

  Post
  |> Repo.preload(preload)
  |> Repo.all()
end

then:

MyApp.Blog.list_posts(preload: :comments)

This takes advantage of ecto ignoring nil options (so you don’t have to pass :preload).

I’m admittedly on the fence about this since it is still exposing parts of ecto to the client, but I haven’t worked on a big enough project to where I can confirm how this plays out. It’s also not so bad since generally you are only specifying relationships as data which the client already knows anyway.

This also doesn’t address preloading with a select, so in this case you’d need to make a custom function for the specific scenario. I, admittedly, never worry about loading more columns than I need as I’ve never worked on anything where those types of optimizations made any kind of noticeable difference (this is especially not as big a problem with LiveView since the extra fields aren’t being sent to the browser). It is, however, something I have thought about more than a few times and would be super interested to hear what other people do.

EDITED to add the default [] for opts, which is pretty critical.

7 Likes

I really like the solution/technique you proposed. Regarding this particular worry, I think you’re in the clear by “allowlisting” the repo options that you accept, like you do with :preload here. This isn’t fundamentally any different than defdelegate to export a function from a nested module. You’re essentially just making that specific option a part of your public API. I think where it would be a problem is if you passed opts directly to the repo, which you of course aren’t doing.

2 Likes

Honestly I picked this up largely from a post I read here at some point, I just can’t find where, so while I don’t want to take all the credit, I don’t know who to credit.