feat: subscription dsl

[barnabasJ]

Contributor checklist

• [ ] Bug fixes include regression tests
• [x] Features include unit/acceptance tests

[barnabasJ]

@zachdaniel I tried to create a test case for the current behaviour before trying to move the code into the dsl but I got a bit stuck.

I copied stuff from https://github.com/absinthe-graphql/absinthe/blob/master/test/absinthe/execution/subscription_test.exs and https://github.com/ash-project/ash/blob/main/test/notifier/pubsub_test.exs but I can't see any messages. I think I'm missing something, Maybe you can give me a hint to get unstuck.

Thanks

[zachdaniel]

Will take a look tomorrow 👍

[zachdaniel]

The issue doesn't jump out to me immediately...the first step might be to add a custom notifier and make sure notifications are going out at all.

[barnabasJ]

@zachdaniel, could you give me an initial review? If everything is reasonably stable, I'll add docs.

[zachdaniel]

Yes! I'll take look today or tomorrow 🔥 . Very exciting.

[zachdaniel]

So, we're going to need some tests with policies, showing how a piece of forbidden data that matches the user's input filter is not broadcast to that user. Unless I missed that and it's there already.

I know we had a long chat about this but I don't remember where we ended up with it, as I'm not sure what currently prevents the above scenario from happening.

[zachdaniel]

Oh, wait does this resolver run per event?

https://github.com/ash-project/ash_graphql/pull/97/files#diff-fc9913737678f0e2a688fdf353cc6daef776e9631f1f7291dd5fa85b1c9d53b1R570

In that case this makes sense. Then basically what we'd want to do to optimize this is to write something that can attempt to resolve policies without running queries. That way basic policies like "is admin" don't incur an unnecessary query.

[barnabasJ]

Oh, wait does this resolver run per event?

https://github.com/ash-project/ash_graphql/pull/97/files#diff-fc9913737678f0e2a688fdf353cc6daef776e9631f1f7291dd5fa85b1c9d53b1R570

In that case this makes sense. Then basically what we'd want to do to optimize this is to write something that can attempt to resolve policies without running queries. That way basic policies like "is admin" don't incur an unnecessary query.

It's run once per topic/context combination.

What exactly do you mean by optimize? I'm guessing that if we hit this line we would essentially already know that we are allowed to call the action without any necessary filters? But we do not really have a way to get information from here to the resolver. In the first attempt, I encoded something into the context, but the context is also sent to the client and, therefore, shouldn't hold any meaningful information.

The result of our talk was the concept of the actor function, which gets the actor and can return a more generic actor for deduplication.

I'll add policy tests tomorrow.

[zachdaniel]

The optimization I mean is that we can, for example, check if the expression for authorization can be resolved directly against the data in question without running the query. Something like:

build_read_one_query
|> Ash.can(....)
|> case do
  {true, query} ->
     if query.authorize_results == [] && we_can_tell_the_record_matches(record, query.filter) do
      just load any necessary data on the record we got
    else
      do a read one w/ the load
    end
end

[barnabasJ]

The optimization I mean is that we can, for example, check if the expression for authorization can be resolved directly against the data in question without running the query. Something like:

build_read_one_query
|> Ash.can(....)
|> case do
  {true, query} ->
     if query.authorize_results == [] && we_can_tell_the_record_matches(record, query.filter) do
      just load any necessary data on the record we got
    else
      do a read one w/ the load
    end
end

We would also need to check if all the necessary values are selected. And what about field policies? They would still need to be applied, right?

[zachdaniel]

Yep! Loading data applies field policies and reselects any unselected fields :) So it would be safe to just "see if they can read it", and if so call Ash.load.

[zachdaniel]

In the worst case, they still visit the database, i.e to reselect fields that aren't selected. But in the best case no additional queries are required.

[barnabasJ]

build_read_one_query
|> Ash.can(....)
|> case do
  {true, query} ->
     if query.authorize_results == [] && we_can_tell_the_record_matches(record, query.filter) do
      just load any necessary data on the record we got
    else
      do a read one w/ the load
    end
end

Can you explain what query.authorize_results == [] means? Because I'm not sure what is stored there and it makes me unsure of what to do if Ash.can only returns {:ok, true}.

for we_can_tell_the_record_matches can I use Ash.Filter.Runtime.filter_matches(query.domain, [data], query.filter)?

[zachdaniel]

query.authorize_results is a set of hooks that authorizers can add that run after the query is run. If you just get back {:ok, true} then that can be treated as the same as query.authorize_results == [] (it means there is nothing else to do). Although IIRC if you pass alter_source?: true you're guaranteed to get back the query.

For evaluating the filter against the record, I'd use something like Ash.Expr.eval(expression, resource: resource, unknown_on_unknown_refs?: true). That will do a sort of "shallow check" without running any queries or anything. If that returns :unknown then you have to run the query. Otherwise you know what the result will be.

[zachdaniel]

Feel free to merge at your convenience :)

[barnabasJ]

@zachdaniel accepting your suggestion dismissed your approval, now merging is blocked again :sweat_smile: