ManualRead escape-hatch isn't deep enough; pk's are captured inside Ash.Filter
- Dominant language
- Elixir
- Stars
- 2.5k
- Forks
- 422
- Avg merge
- 23h 26m
- Merged PRs (30d)
- 46
Description
**Use Case**
I have a resource, but (for legacy/org reasons) some of its attributes aren't directly available in the database. (In truth, we're going to be [calling microservices|reading a python pickle|inferring things from a hierarchy of directories+flatfiles...])
This is modeled as `has_one` so that we'll incur the cost of the ManualRead when needed via `Ash.load!`. This mostly seems good.
```elixir
defmodule SomeResource do
use Ash.Resource, domain: MyDomain, data_layer: AshMysql.DataLayer
...
relationships do
has_one :legacy_attributes, LegacyAttributes do
source_attribute :id
destination_attribute :id
end
end
end
defmodule LegacyAttributes do
use Ash.Resource, domain: MyDomain
actions do
defaults [create: :*]
read :external do
primary? true
manual __MODULE__.ManualRead
end
end
defmodule ManualRead do
use Ash.Resource.ManualRead
def read(query, data_layer_query, opts, context) do
pk = ??? # <-- Hmmmm, nothing we have is on point!
attributes = lookup_externally(pk)
{:ok, [ Ash.create!(query.resource, attributes) ] }
end
end
```
The problem is that `query` has already captured the pk(s) as an `Ash.Filter`, but my ManualAction's `lookup_externally/1` isn't something I can _filter_ or _query_; it's not a database - I need to perform a *direct invocation*. Moreover, I can't `pk = Ash.Query.get_argument(query, :id)` because, again, the value in question is already subsumed, there's no arguments to read. (I couldn't find a way to decompose the Ash.Filter, though that's pretty brittle and tightly coupled, even if it were possible.)
**Workaround**
To be able to pass the pk to `ManualRead`, we need a custom read action, looking to `Ash.Query.get_argument/2` to get what we want:
```elixir
defaults [:read, create: :*] # a default reader is required to make others happy, even though we were to use it, it would crash and burn?
read :external do
argument :id, :integer, allow_nil?: false
# primary? true
manual __MODULE__.ManualRead
end
...
def read(query, data_layer_query, opts, context) do
pk = Ash.Query.get_argument(query, :id)
attributes = summon_from_somewhere_else(pk)
{:ok, [ Ash.create!(query.resource, attributes) ] }
end
```
Unfortunately, we have now have to expose this implementation to every other resource via a `ManualRelationship`.
```elixir
has_one :legacy_attributes, LegacyAttributes do
# source_attribute :id
# destination_attribute :id
manual __MODULE__.LegacyAttributesDispatch
end
...
defmodule LegacyAttributesDispatch do
use Ash.Resource.ManualRelationship
def load(records, _opts, _context) do
{:ok,
Enum.reduce(records, %{}, fn record, acc ->
acc
|> Map.put(
record.id,
LegacyAttributes
|> Ash.Query.for_read(:external, %{id: record.id})
|> Ash.read_one!()
)
end)
}
end
```
**Expected behavior**
1) This feels bad. I would have expected that
```elixir
has_one :legacy_attributes, LegacyAttributes do
source_attribute :id
destination_attribute :id
end
```
would pass the id to LegacyAttributes' ManualRead without having to expose to additional implementation details and a custom read action - `SomeResource` shouldn't have to know that I'm doing something [fun](https://dwarffortresswiki.org/DF2014:Fun&redirect=no) in ManualRead, as the relationship already provides ALL the necessary information. If you squint hard, we're re-implementing the default read action, just to pass the pk, which is the whole _raison d'etre_ for a default read action... 🤦🏻
This implies that
2) ManualRead needs to be deeper, and not strictly coupled to Ash.[Filter|Query]. ManualRead.load/3 feels like a richer version of `Ash.Read.modify_query` (which is a sensible place to tinker with the posited Query/Filter chain). Instead, ManualRead should provide `req: [%{pk => value}]` so that I can execute the read myself, or if I want Ash/Ecto help, build my own Query/Filter from scratch?
It is possible that (1) could be separated out as a bug/deficiency that can be mitigated without fully getting into the bigger complaint in (2): i.e., the ManualRead.load/3 opts payload could just be extended to pass the values. This is marked as "enhancement" because the gist that `ManualRead.load` might be subsumed by a (better) modify_query, and revisited to be more manual, more Ash/Ecto-agnostic would obviously be breaking.
**Counterposition**
I expect there's a way to have some rich expression be passed down into the query/filter, which wouldn't be captured by keys alone. I might say if you have to "do that manually" (and can't live within `modify_query`) via a non-database-like system, woe until you, you reap what you sow. That said, if your system has that depth, it's more likely that your manual action can scan/extract a superset of records, then `Ash.Query.apply_to(query, superset)` to fully meet the constraints? Having ManualRead receive the "upstream" query is fine (probably desirable) - I still contend that I wouldn't that to have that filter already composed for the resource I'm working on. If I wanted that, modify_query is already more apropos.
PS Thanks for the hard work, cheers.
Contributor guide
Research direction
Start by reading Ash.Resource.ManualRead and the relationship-loading paths involving Ash.Filter and Ash.Query, including Ash.Query.apply_to/2. No test file is named, so first locate existing ManualRead and manual relationship tests. Done should be a relationship that supplies its source primary key to ManualRead without requiring each resource to expose a custom read action, while preserving query constraints.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- elixir
- Domain
- backend-api-design
- Issue type
- Feature
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100