ManualRead escape-hatch isn't deep enough; pk's are captured inside Ash.Filter

[tercenya]

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.

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<id in [1]>, 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:

  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.

  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

    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 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.

[zachdaniel]

I can respond to this more deeply next week when I am back at my computer, but the intention is that you actually would look for this information in the query's filter. You would use https://hexdocs.pm/ash/Ash.Filter.html#fetch_simple_equality_predicate/2 to fetch any potential filter added by something upstream. You could make your manual read fail without it if it must be present. For other filters you can extract them from the filter as well, but we'd want to define potentially more functions that manipulate and/or extract & split a filter to make that better.

Finally, you can use generic actions instead of manual reads to capture more generic behavior (but couldn't use it as a relationships read action).

[tercenya]

This is the default read operation - give me the pk, I'll give you the record - which is why I want to lean on relationships to make this easily accessible.

I tried fetch_simple_equality_predicate/2 but the query.filter in read/3 is always of type Ash.Query.Operator.In, which doesn't match %Eq{..}. The pk(s) have already been aggregated so that I can return a { :ok, [res()] } from read/3, the usual Ash-ism.

--

My wider point is that breaking down the Filter (or eventually running the Query) seems like something I would want to do ala modify_query, where I'm already "all-in" on the Ash/Ecto worldview. (aside: if modify_query had the extra parameters of read/3, would there be any difference?)

If I'm doing things manually, I expected to either have to assemble that filter, or go about my merry way where it's not helpful. Having to tear it back apart seems... um, "dogmatic for an escape-hatch"? As above, Ash has already manipulated the data. (at this level, I wouldn't treat Ash.Filter as a simple data structure just conveying the inputs; it's already captured too much of Ash's Query intent). And now I have to know what/how Ash did to undo it... The thought was these concerns would be perfectly fine in modify_query, but ManualRead would be a layer lower.

[zachdaniel]

The primary issue here is that the abstraction level that a manual read is at is not "given these action inputs, do this read action". The input to a manual read is a query. I agree that it is honestly too wide of an interface for someone to support fully, and that potentially some other abstraction would be warranted. But imagine someone did this:

YourResource
|> Ash.Query.filter(some_arbitrary_filter)
|> Ash.Query.for_read(:you_manual_action)

Technically it's your manual actions job to fulfill it. So it's always a function of "given this query a user wants to run".

We can potentially add a new callback to manual read actions that doesnt take a query and only operates on the input arguments, and has another callback to fulfill requests for related data by being given an id, and another for a range of ids. Then we could forbid adding custom query components like filters and sorts etc. because it's known that the action doesn't support these arbitrary query modifiers. What do you think?

[zachdaniel]

The real difficulty is the interactions with various other things from the resource layer. We'd have to disable the use of those. Policies for read actions, for example, work by filtering the read (unless configured otherwise). So using filter policies would also have to be forbidden for use with this action etc.

It's okay to do that, but to do it right a callback must be constructed that explicitly does not take a query. Not just one that also takes the input.

[zachdaniel]

There is also a solution to this specific problem:

I tried fetch_simple_equality_predicate/2 but the query.filter in read/3 is always of type Ash.Query.Operator.In, which doesn't match %Eq{..}. The pk(s) have already been aggregated so that I can return a { :ok, [res()] } from read/3, the usual Ash-ism.

Which is that we can make a function for extracting/splitting out a range if values, i.e fetch_simple_in_predicate/2 that gives you back a list of values.

And

(aside: if modify_query had the extra parameters of read/3, would there be any difference?)

You do have those parameters, in query.arguments :) The only time you don't have them is for relationships, because currently that isn't implemented as an input to an argument, its implemented as a filter to the query.