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
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:
This could be done via four separate functions:
filter_accounts_by_propertyfilter_accounts_by_property_idfilter_accounts_by_propertiesfilter_accounts_by_property_idsOr, 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
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.
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.
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 
It depends 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.
So it might be a useful thing to have some kind of context struct which is then used in all calls to the more involved context functions then? (“more involved” referring to a need for authorization and auditing for example)
The context struct module implementation could then guide in how to correctly collect what basic information is needed to call the context. Think of user account, tenant, protocol/transport level/request information.
Is this a pattern you use or come across often @sasajuric ? (I would think so)
One could maybe even go as far as adding DI-like things to it (like passing persistence/external-service-representing modules), or would that be a bridge too far?
© Copyright Elixir Forum | Terms | Privacy & Cookies
