Context functions accepting either ID or struct

Do you think it’s not a bad practice for context functions to systematically accept either the struct or the ID through two clauses:

def do_something(%Foo{} = foo) do
  # do something
end

def do_something(foo_id) do
  do_something(get_foo!(foo_id))
end
2 Likes

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.

2 Likes

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

2 Likes

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, https://github.com/edisonywh/condiment, 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.

1 Like

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.

1 Like

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.

5 Likes

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.

1 Like

Probably a topic on it’s own, but how would you handle authorization in the context?

Sometimes an action is performed by the system, automatically, and sometimes it’s performed manually by an user. It may even be called from IEx for eaxperimentation or remote intervention.

Say you want to delete an entity, and it may be as part of a periodic task or manually by the user. How would the context function look like? It not only needs the entity to delete, but also who is deleting it in order to make an authorization check, which is not always present, and that complicates the function signature.

Correct :slight_smile:

It depends :slight_smile: In a simple version I’d invoke an internal context function which is not exposed outside to web & co. I’d strive to make this function private, so it’s not accidentally invoked. If the function is private you can’t invoke it from iex, so it motivates you to provide an account which is deleting it (like e.g. some admin account).

A variation is to introduce a “system” account which has full privileges, and pass that to the context function which accepts the account. This allows us to log who performed each operation, even if the operation has been performed from within by the system itself, which can assist with auditing.

6 Likes