chore: return InvalidLoad when loading field?: false calcs

Author: chazwatkins

lib/ash/query/query.ex

@@ -2216,7 +2216,9 @@ defmodule Ash.Query do
         end
 
       {field, {args, load_through}}, query ->
-        if resource_calculation = Ash.Resource.Info.calculation(query.resource, field) do
+        resource_calculation = Ash.Resource.Info.calculation(query.resource, field)
+
+        if Ash.Resource.Calculation.can_load?(resource_calculation) do
           load_resource_calculation(query, resource_calculation, args, load_through)
         else
           add_error(
@@ -2232,7 +2234,15 @@ defmodule Ash.Query do
             load_relationship(query, rel, rest, opts)
 
           resource_calculation = Ash.Resource.Info.calculation(query.resource, field) ->
-            load_resource_calculation(query, resource_calculation, rest)
+            if Ash.Resource.Calculation.can_load?(resource_calculation) do
+              load_resource_calculation(query, resource_calculation, rest)
+            else
+              add_error(
+                query,
+                :load,
+                Ash.Error.Query.InvalidLoad.exception(load: [{field, rest}])
+              )
+            end
 
           attribute = Ash.Resource.Info.attribute(query.resource, field) ->
             if Ash.Type.can_load?(attribute.type, attribute.constraints) do
@@ -2360,11 +2370,12 @@ defmodule Ash.Query do
     end
   end
 
+  # TODO: Remove this dead function - resource_calc_to_calc/4
   @doc false
-  def resource_calc_to_calc(query, name, resource_calculation, args \\ %{}) do
+  def resource_calc_to_calc(query, _name, resource_calculation, args \\ %{}) do
     {name, load} =
       case fetch_key(args, :as) do
-        :error -> {name, name}
+        :error -> Ash.Resource.Calculation.query_name_and_load(resource_calculation)
         {:ok, key} -> {key, nil}
       end
 
@@ -2475,7 +2486,15 @@ defmodule Ash.Query do
         end
 
       resource_calculation = Ash.Resource.Info.calculation(query.resource, field) ->
-        load_resource_calculation(query, resource_calculation, %{})
+        if Ash.Resource.Calculation.can_load?(resource_calculation) do
+          load_resource_calculation(query, resource_calculation, %{})
+        else
+          add_error(
+            query,
+            :load,
+            Ash.Error.Query.InvalidLoad.exception(load: field)
+          )
+        end
 
       true ->
         add_error(query, :load, Ash.Error.Query.InvalidLoad.exception(load: field))
@@ -3635,14 +3654,17 @@ defmodule Ash.Query do
 
     args = Map.put(args, :as, as_name)
 
-    if resource_calculation = Ash.Resource.Info.calculation(query.resource, calc_name) do
+    with %Ash.Resource.Calculation{} = resource_calculation <-
+           Ash.Resource.Info.calculation(query.resource, calc_name),
+         true <- Ash.Resource.Calculation.can_load?(resource_calculation) do
       if opts[:load_through] do
         load_resource_calculation(query, resource_calculation, args)
       else
         load_resource_calculation(query, resource_calculation, args, opts[:load_through])
       end
     else
-      add_error(query, "No such calculation: #{inspect(calc_name)}")
+      nil -> add_error(query, "No such calculation: #{inspect(calc_name)}")
+      false -> add_error(query, :load, Ash.Error.Query.InvalidLoad.exception(load: calc_name))
     end
   end
 

lib/ash/resource/calculation/calculation.ex

@@ -219,6 +219,11 @@ defmodule Ash.Resource.Calculation do
     {:ok, {Ash.Resource.Calculation.Expression, expr: expr}}
   end
 
+  @doc false
+  @spec can_load?(t() | nil) :: boolean()
+  def can_load?(%__MODULE__{} = calculation), do: calculation.field?
+  def can_load?(_), do: false
+
   @doc false
   @spec query_name_and_load(t()) :: {atom, atom | nil}
   def query_name_and_load(calculation) do

test/resource/calculations_test.exs

@@ -104,8 +104,8 @@ defmodule Ash.Test.Resource.CalculationsTest do
       refute Map.has_key?(post, :non_field_calculation)
     end
 
-    test "Calculation with field?: false cannot be loaded directly, but can be used in expressions" do
-      defmodule Post do
+    test "Calculation with field?: false can be used in expressions" do
+      defmodule PostForCalculation do
         @moduledoc false
         use Ash.Resource, domain: Domain, data_layer: Ash.DataLayer.Ets
 
@@ -137,19 +137,52 @@ defmodule Ash.Test.Resource.CalculationsTest do
         end
       end
 
-      Post
-      |> Ash.Changeset.for_create(:create, %{name: "name", contents: "contents"})
-      |> Ash.create!()
-
       post =
-        Post
-        |> Ash.Query.for_read(:read, %{})
-        |> Ash.read_one!()
-        |> Ash.load!([:non_field_calculation, :name_and_contents])
+        PostForCalculation
+        |> Ash.Changeset.for_create(:create, %{name: "name", contents: "contents"})
+        |> Ash.create!()
+        |> Ash.load!([:name_and_contents])
 
       assert :name_and_contents in Map.keys(post)
       assert post.name_and_contents == "namecontents"
-      refute :non_field_calculation in Map.keys(post)
+    end
+
+    test "Calculation with field?: false returns InvalidLoad error when loaded directly" do
+      defmodule PostForCalculationError do
+        @moduledoc false
+        use Ash.Resource, domain: Domain, data_layer: Ash.DataLayer.Ets
+
+        attributes do
+          uuid_primary_key :id
+
+          attribute :name, :string do
+            public?(true)
+          end
+
+          attribute :contents, :string do
+            public?(true)
+          end
+        end
+
+        actions do
+          default_accept :*
+          defaults [:read, :destroy, update: :*, create: :*]
+        end
+
+        calculations do
+          calculate :non_field_calculation, :string, concat([:name, :contents]) do
+            field?(false)
+          end
+        end
+      end
+
+      post =
+        PostForCalculationError
+        |> Ash.Changeset.for_create(:create, %{name: "name", contents: "contents"})
+        |> Ash.create!()
+
+      assert {:error, %{errors: [%Ash.Error.Query.InvalidLoad{load: :non_field_calculation}]}} =
+               Ash.load(post, [:non_field_calculation])
     end
 
     test "Calculation descriptions are allowed" do