fix: ensure that field policies don't interfere with relationship loading

closes #1457

lib/ash/actions/read/calculations.ex

@@ -1948,7 +1948,12 @@ defmodule Ash.Actions.Read.Calculations do
   # Deselect fields that we know statically cannot be seen
   # The field may be reselected later as a calculation dependency
   # this is an optimization not a guarantee
-  def deselect_known_forbidden_fields(ash_query, calculations_at_runtime, calculations_in_query) do
+  def deselect_known_forbidden_fields(
+        ash_query,
+        calculations_at_runtime,
+        calculations_in_query,
+        skip \\ []
+      ) do
     depended_on_fields = ash_query.context[:private][:depended_on_fields] || []
 
     calculations_at_runtime
@@ -1985,6 +1990,7 @@ defmodule Ash.Actions.Read.Calculations do
     end)
     |> Enum.uniq()
     |> Kernel.--(depended_on_fields)
+    |> Kernel.--(skip)
     |> then(
       &unload_forbidden_fields(ash_query, &1, calculations_at_runtime, calculations_in_query)
     )

lib/ash/actions/read/read.ex

@@ -225,6 +225,11 @@ defmodule Ash.Actions.Read do
         query.domain
       )
 
+    source_fields =
+      if !opts[:initial_data] do
+        source_fields(query)
+      end
+
     query =
       if opts[:initial_data] do
         select = source_fields(query, opts[:lazy?] && opts[:initial_data]) ++ (query.select || [])
@@ -244,7 +249,7 @@ defmodule Ash.Actions.Read do
           query
         end
       else
-        Ash.Query.ensure_selected(query, source_fields(query))
+        Ash.Query.ensure_selected(query, source_fields)
       end
 
     {query, stop?} = add_async_limiter(query, calculations_at_runtime, opts)
@@ -261,7 +266,7 @@ defmodule Ash.Actions.Read do
             opts
           )
         else
-          do_read(query, calculations_at_runtime, calculations_in_query, opts)
+          do_read(query, calculations_at_runtime, calculations_in_query, source_fields, opts)
         end
 
       {data_result, query_ran} =
@@ -393,7 +398,13 @@ defmodule Ash.Actions.Read do
     end
   end
 
-  defp do_read(%{action: action} = query, calculations_at_runtime, calculations_in_query, opts) do
+  defp do_read(
+         %{action: action} = query,
+         calculations_at_runtime,
+         calculations_in_query,
+         source_fields,
+         opts
+       ) do
     with {:ok, %{valid?: true} = query} <- handle_multitenancy(query),
          query <- add_select_if_none_exists(query),
          query <- %{
@@ -416,7 +427,8 @@ defmodule Ash.Actions.Read do
                Ash.Actions.Read.Calculations.deselect_known_forbidden_fields(
                  query,
                  calculations_at_runtime,
-                 calculations_in_query
+                 calculations_in_query,
+                 source_fields
                ),
              {:ok, data_layer_calculations} <- hydrate_calculations(query, calculations_in_query),
              {:ok, query} <- hydrate_aggregates(query),

test/actions/bulk/bulk_create_test.exs

@@ -2,9 +2,8 @@ defmodule Ash.Test.Actions.BulkCreateTest do
   @moduledoc false
   use ExUnit.Case, async: true
 
-  import Ash.Expr, only: [expr: 1]
+  import Ash.Expr
   import ExUnit.CaptureLog
-  require Ash.Expr
 
   alias Ash.Test.Domain, as: Domain
 
@@ -226,6 +225,11 @@ defmodule Ash.Test.Actions.BulkCreateTest do
         change ChangeTitleBeforeAction
       end
 
+      create :create_with_actor_referencing_upsert_condition do
+        upsert? true
+        upsert_condition expr(upsert_conflict(:title) != ^actor(:title))
+      end
+
       create :create_with_related_posts do
         argument :related_post_ids, {:array, :uuid} do
           allow_nil? false
@@ -790,6 +794,63 @@ defmodule Ash.Test.Actions.BulkCreateTest do
              )
   end
 
+  test "can upsert with an actor reference in the upsert condition" do
+    org =
+      Org
+      |> Ash.Changeset.for_create(:create, %{})
+      |> Ash.create!()
+
+    assert %Ash.BulkResult{
+             records: [
+               %{title: "title1", title2: "changes", title3: "wont"},
+               %{title: "title2", title2: "changes", title3: "wont"},
+               %{title: "title3", title2: "changes", title3: "wont"}
+             ]
+           } =
+             Ash.bulk_create!(
+               [
+                 %{title: "title1", title2: "changes", title3: "wont"},
+                 %{title: "title2", title2: "changes", title3: "wont"},
+                 %{title: "title3", title2: "changes", title3: "wont"}
+               ],
+               Post,
+               :create,
+               tenant: org.id,
+               return_errors?: true,
+               return_records?: true,
+               sorted?: true,
+               return_errors?: true,
+               authorize?: false
+             )
+
+    assert %Ash.BulkResult{
+             records: [
+               %{title: "title1", title2: "did_change", title3: "wont"},
+               %{title: "title2", title2: "did_change", title3: "wont"}
+             ]
+           } =
+             Ash.bulk_create!(
+               [
+                 %{title: "title1", title2: "did_change", title3: "oh no"},
+                 %{title: "title2", title2: "did_change", title3: "what happened"},
+                 %{title: "title3", title2: "shouldn't even", title3: "be in result"}
+               ],
+               Post,
+               :create_with_actor_referencing_upsert_condition,
+               return_errors?: true,
+               return_records?: true,
+               tenant: org.id,
+               upsert?: true,
+               return_errors?: true,
+               upsert_identity: :unique_title,
+               upsert_fields: [:title2],
+               actor: %{title: "title3"},
+               upsert_condition: expr(upsert_conflict(:title) != ^actor(:title)),
+               sorted?: true,
+               authorize?: false
+             )
+  end
+
   test "respects upsert_condition despite we have a relationship" do
     product =
       Ash.bulk_create(

test/policy/field_policy_test.exs

@@ -125,6 +125,16 @@ defmodule Ash.Test.Policy.FieldPolicyTest do
                |> Map.get(:role)
     end
 
+    test "field policies don't interfere with data loading", %{post: post} do
+      # asserting no raise as this is a regression test
+      post
+      |> Ash.load!(:reporter)
+
+      Post
+      |> Ash.Query.load(:reporter)
+      |> Ash.read!()
+    end
+
     test "can load a resource with a forbidden aggregate", %{
       representative: representative
     } do

test/support/policy_field/resources/post.ex

@@ -56,6 +56,10 @@ defmodule Ash.Test.Support.PolicyField.Post do
       authorize_if relates_to_actor_via(:reporter)
     end
 
+    field_policy :reporter_id do
+      forbid_if always()
+    end
+
     field_policy :* do
       authorize_if always()
     end