Context functions accepting either ID or struct

I actually came up with some thoughts after posting this, and I think it’s not a good practice, because context functions can have more than one argument, and the more functions with multiple schemas as arguments, the less it really makes sense at some point.

I’ve inherited a codebase that mixes up passing IDs and schemas, and I think it creates some inconsistency. I would go for enforcing only passing schemas.

I would also like to know how others are handling this sort of thing. I prefer passing a struct for the pattern-matching assurances. However, sometimes I only have an ID and the extra query to turn that into a struct seems like an unecessary query.

As an example, suppose I want to filter accounts by property. There are actually four different ways I typically would want to query that resource:

1. Accounts for a specific Property struct
2. Accounts for a specific Property ID
3. Accounts for multiple Property structs
4. Accounts for multiple Property IDs

This could be done via four separate functions:

1. filter_accounts_by_property
2. filter_accounts_by_property_id
3. filter_accounts_by_properties
4. filter_accounts_by_property_ids

Or, this could be done with a single filter_accounts_by_property function with multiple heads that accepts a schema, an ID, multiple schemas, or multiple IDs.

I like the explicitness of the separate function names. On the other hand, if it is happening a lot the simplicity of a single function that knows how to handle these four common argument structures might be nice. This is something that could easily be a reusable convention in multiple cases. Naively, there could be a common function that converts the argument to a list of IDs with something like…

def to_list_ids(args), do: args |> Enum.wrap() |> to_ids()
defp to_ids([first | _] = items) when is_integer(first), do: items
defp to_ids(items), do: Enum.map(& &1.id)

Then just use that anytime you want to support the four common queries. Example:

def filter_accounts_by_property(query, property_or_properties) do
  from account in query, where: account.property_id in ^to_list_ids(property_or_properties)
end

Part of me thinks it is against the spirit of elixir in terms of explicitness and the other part thinks it’s a little bit madness to continually repeat this ceremony.

Maybe a simple convention might be: if that context function only uses the id, the argument is the id. Otherwise argument is always schema.

This then also gives some more information about the context function and what it really consumes.

I personally don’t think this is a bad practice, and I prefer it to get_foo_by_bar, get_foo_by_baz & co. If the function accepts multiple args and you want to have polymorphism based only on the first one, you can do something like:

def do_something(foo_filter, other_args) do
  foo = get_foo(foo_filter)
  # ...
end

So basically, you push the branching deeper.

Including a typespec can help the reader:

@spec do_something(Foo.t() | Foo.id(), other_args) :: result

image798×215 18.8 KB

Yeah this is something unclear to me too, I do both versions, either fetching the resource for a schema context function, or extracting the ID out from schema. I don’t have a good guideline so they are all over the place right now..

It’s inevitable that sometimes you get only an ID to work with, so I am leaning towards the context functions with ID.

Something I’m experimenting recently is using my Condiment library, GitHub - edisonywh/condiment: 🍡 Add flavors to your context function without the hassles. · GitHub, so I would handle both cases (mostly only using it for resource fetching, so does not apply to all context functions).

def list_posts(opts \\ []) do
  posts_query()
  |> Condiment.new(opts)
  |> Condiment.add(:user, &(user_id_query(&1, &2.user.id))
  |> Condiment.add(:user_id, &user_id_query/2)
  |> Condiment.run()
  |> Repo.all()
end

Then again, it’s really not ideal, I am starting to use more IDs so would be nice to see what people are doing.

Good question! I lean towards ids since most of my context functions are consumed by controllers, so I don’t want any extra calls in the controller.

In my experience even controllers that receive IDs have to check to see if the user is authorized to interact with / see the specified rows, so they wind up having to fetch the structs anyways.

You need to make the same check if the request arrives through web socket, graphql, raw tcp, iex, test, or any other interface, and so IMO this check is a domain concern and it should be done in the context.

I would lean towards having the caller of do_something call get_foo!() explicitly, like:

# already have a foo
foo |> Context.do_something()

# don't already have a foo
foo_id |> Context.get_foo!() |> Context.do_something()

Now when reading the calling code, it’s really clear whether or not get_foo! gets called, which can be nice if get_foo! is expensive, perhaps making a database or network call. That clarity would be especially nice when reading code that calls do_something a large number of times with the same foo.

Exactly my way of thinking. But I have a feeling that doing authorization in the web layer (as a plug) is a common practice. Personally I prefer my contexts returning :unauthorized and handle that with an action fallback. This makes controllers super thin to the point where testing them becomes questionable.