fix: validate read action existence

lib/ash/actions/read.ex

@@ -1,23 +1,30 @@
 defmodule Ash.Actions.Read do
   @moduledoc false
+  require Logger
+
+  require Ash.Query
+
   alias Ash.Actions.SideLoad
   alias Ash.Engine
   alias Ash.Engine.Request
   alias Ash.Error.Invalid.{LimitRequired, PaginationRequired}
+  alias Ash.Error.Query.NoReadAction
   alias Ash.Filter
   alias Ash.Query.Aggregate
-  require Logger
-
-  require Ash.Query
 
   def unpaginated_read(query, action \\ nil, opts \\ []) do
-    action = action || Ash.Resource.primary_action!(query.resource, :read)
+    action = action || Ash.Resource.primary_action(query.resource, :read)
 
-    if action.pagination do
-      opts = Keyword.put(opts, :page, false)
-      run(query, %{action | pagination: %{action.pagination | required?: false}}, opts)
-    else
-      run(query, action, opts)
+    cond do
+      !action ->
+        {:error, NoReadAction.exception(resource: query.resource, when: "reading")}
+
+      action.pagination ->
+        opts = Keyword.put(opts, :page, false)
+        run(query, %{action | pagination: %{action.pagination | required?: false}}, opts)
+
+      true ->
+        run(query, action, opts)
     end
   end
 

lib/ash/actions/relationships.ex

@@ -152,7 +152,7 @@ defmodule Ash.Actions.Relationships do
       Request.new(
         api: changeset.api,
         resource: relationship.destination,
-        action: Ash.Resource.primary_action!(relationship.destination, :read),
+        action: Ash.Resource.primary_action(relationship.destination, :read),
         query: query,
         path: [:relationships, relationship_name, type],
         async?: not possible?,
@@ -175,7 +175,8 @@ defmodule Ash.Actions.Relationships do
                     query
                   end
 
-                with {:ok, results} <- Ash.Actions.Read.unpaginated_read(query),
+                with {:ok, results} <-
+                       Ash.Actions.Read.unpaginated_read(query),
                      :ok <-
                        ensure_all_found(
                          changeset,
@@ -778,35 +779,41 @@ defmodule Ash.Actions.Relationships do
          changeset,
          %{destination: destination} = relationship
        ) do
-    value = Changeset.get_attribute(changeset, relationship.source_field)
-    filter_statement = [{relationship.destination_field, value}]
+    case Ash.Resource.primary_action(relationship.destination, :read) do
+      nil ->
+        changeset
 
-    request =
-      Request.new(
-        api: changeset.api,
-        resource: destination,
-        action: Ash.Resource.primary_action!(relationship.destination, :read),
-        path: [:relationships, relationship.name, :current],
-        query: Ash.Query.filter(destination, ^filter_statement),
-        data:
-          Request.resolve([[:relationships, relationship.name, :current, :query]], fn data ->
-            query = get_in(data, [:relationships, relationship.name, :current, :query])
+      read_action ->
+        value = Changeset.get_attribute(changeset, relationship.source_field)
+        filter_statement = [{relationship.destination_field, value}]
+
+        request =
+          Request.new(
+            api: changeset.api,
+            resource: destination,
+            action: read_action,
+            path: [:relationships, relationship.name, :current],
+            query: Ash.Query.filter(destination, ^filter_statement),
+            data:
+              Request.resolve([[:relationships, relationship.name, :current, :query]], fn data ->
+                query = get_in(data, [:relationships, relationship.name, :current, :query])
 
-            query =
-              if changeset.tenant do
-                Ash.Query.set_tenant(query, changeset.tenant)
-              else
-                query
-              end
+                query =
+                  if changeset.tenant do
+                    Ash.Query.set_tenant(query, changeset.tenant)
+                  else
+                    query
+                  end
 
-            Ash.Actions.Read.unpaginated_read(query)
-          end),
-        name: "Read related #{relationship.name} before replace"
-      )
+                Ash.Actions.Read.unpaginated_read(query)
+              end),
+            name: "Read related #{relationship.name} before replace"
+          )
 
-    changeset
-    |> Changeset.add_requests(request)
-    |> Changeset.changes_depend_on([:relationships, relationship.name, :current, :data])
+        changeset
+        |> Changeset.add_requests(request)
+        |> Changeset.changes_depend_on([:relationships, relationship.name, :current, :data])
+    end
   end
 
   defp many_to_many_join_resource_request(
@@ -819,7 +826,7 @@ defmodule Ash.Actions.Relationships do
     Request.new(
       api: changeset.api,
       resource: through,
-      action: Ash.Resource.primary_action!(relationship.destination, :read),
+      action: Ash.Resource.primary_action(through, :read),
       path: [:relationships, relationship.name, :current_join],
       query: Ash.Query.filter(through, ^filter_statement),
       data:
@@ -846,7 +853,7 @@ defmodule Ash.Actions.Relationships do
     Request.new(
       api: changeset.api,
       resource: destination,
-      action: Ash.Resource.primary_action!(relationship.destination, :read),
+      action: Ash.Resource.primary_action(relationship.destination, :read),
       path: [:relationships, name, :current],
       query:
         Request.resolve(

lib/ash/actions/side_load.ex

@@ -287,7 +287,7 @@ defmodule Ash.Actions.SideLoad do
       |> Enum.map_join(".", &Map.get(&1, :name))
 
     Engine.Request.new(
-      action: Ash.Resource.primary_action!(relationship.destination, :read),
+      action: Ash.Resource.primary_action(relationship.destination, :read),
       resource: relationship.destination,
       name: "side_load #{source}",
       api: related_query.api,
@@ -391,7 +391,7 @@ defmodule Ash.Actions.SideLoad do
           end
 
         Request.new(
-          action: Ash.Resource.primary_action!(relationship.destination, :read),
+          action: Ash.Resource.primary_action(relationship.destination, :read),
           resource: relationship.through,
           name: "side_load join #{join_relationship.name}",
           api: related_query.api,

lib/ash/changeset/changeset.ex

@@ -82,7 +82,8 @@ defmodule Ash.Changeset do
     Changes.NoSuchAttribute,
     Changes.NoSuchRelationship,
     Changes.Required,
-    Invalid.NoSuchResource
+    Invalid.NoSuchResource,
+    Query.NoReadAction
   }
 
   @doc "Return a changeset over a resource or a record"
@@ -424,37 +425,72 @@ defmodule Ash.Changeset do
         add_error(changeset, error)
 
       %{type: :many_to_many} = relationship ->
-        case primary_keys_with_changes(relationship, List.wrap(record_or_records)) do
-          {:ok, primary_key} ->
-            relationships =
-              Map.put(changeset.relationships, relationship.name, %{replace: primary_key})
-
-            %{changeset | relationships: relationships}
-
-          {:error, error} ->
-            add_error(changeset, error)
-        end
+        do_replace_many_to_many_relationship(changeset, relationship, record_or_records)
 
       relationship ->
-        records =
-          if relationship.cardinality == :one do
-            if is_list(record_or_records) do
-              List.first(record_or_records)
+        if Ash.Resource.primary_action(relationship.destination, :read) do
+          records =
+            if relationship.cardinality == :one do
+              if is_list(record_or_records) do
+                List.first(record_or_records)
+              else
+                record_or_records
+              end
             else
-              record_or_records
+              List.wrap(record_or_records)
             end
-          else
-            List.wrap(record_or_records)
+
+          case primary_key(relationship, records) do
+            {:ok, primary_key} ->
+              relationships =
+                Map.put(changeset.relationships, relationship.name, %{replace: primary_key})
+
+              changeset
+              |> check_entities_for_direct_write(relationship.name, List.wrap(records))
+              |> Map.put(:relationships, relationships)
+
+            {:error, error} ->
+              add_error(changeset, error)
           end
+        else
+          add_error(
+            changeset,
+            NoReadAction.exception(
+              resource: changeset.resource,
+              when: "replacing relationship #{relationship.name}"
+            )
+          )
+        end
+    end
+  end
+
+  defp do_replace_many_to_many_relationship(changeset, relationship, record_or_records) do
+    cond do
+      !Ash.Resource.primary_action(relationship.destination, :read) ->
+        add_error(
+          changeset,
+          NoReadAction.exception(
+            resource: changeset.resource,
+            when: "replacing relationship #{relationship.name}"
+          )
+        )
+
+      !Ash.Resource.primary_action(relationship.through, :read) ->
+        add_error(
+          changeset,
+          NoReadAction.exception(
+            resource: changeset.resource,
+            when: "replacing relationship #{relationship.name}"
+          )
+        )
 
-        case primary_key(relationship, records) do
+      true ->
+        case primary_keys_with_changes(relationship, List.wrap(record_or_records)) do
           {:ok, primary_key} ->
             relationships =
               Map.put(changeset.relationships, relationship.name, %{replace: primary_key})
 
-            changeset
-            |> check_entities_for_direct_write(relationship.name, List.wrap(records))
-            |> Map.put(:relationships, relationships)
+            %{changeset | relationships: relationships}
 
           {:error, error} ->
             add_error(changeset, error)
@@ -471,7 +507,20 @@ defmodule Ash.Changeset do
 
       put_context(changeset, :destination_entities, relation_entities)
     else
-      changeset
+      if Ash.Resource.primary_action(
+           Ash.Resource.related(changeset.resource, relationship_name),
+           :read
+         ) do
+        changeset
+      else
+        add_error(
+          changeset,
+          NoReadAction.exception(
+            resource: changeset.resource,
+            when: "editing relationship #{relationship_name} and not supplying full records"
+          )
+        )
+      end
     end
   end
 

lib/ash/error/query/no_read_action.ex

@@ -0,0 +1,20 @@
+defmodule Ash.Error.Query.NoReadAction do
+  @moduledoc "Used when a resource would be read but has no read action"
+  use Ash.Error
+
+  def_ash_error([:resource, :when], class: :invalid)
+
+  defimpl Ash.ErrorKind do
+    def id(_), do: Ecto.UUID.generate()
+
+    def code(_), do: "no_read_action"
+
+    def message(error) do
+      if error.when do
+        "No read action exists for  #{inspect(error.resource)} when: #{error.when}"
+      else
+        "No read action exists for  #{inspect(error.resource)}"
+      end
+    end
+  end
+end

lib/ash/error/query/no_such_relationship.ex

@@ -0,0 +1,16 @@
+defmodule Ash.Error.Query.NoSuchRelationship do
+  @moduledoc "Used when an relationship that doesn't exist is used in a query"
+  use Ash.Error
+
+  def_ash_error([:resource, :name], class: :invalid)
+
+  defimpl Ash.ErrorKind do
+    def id(_), do: Ecto.UUID.generate()
+
+    def code(_), do: "no_such_relationship"
+
+    def message(error) do
+      "No such relationship #{error.name} for resource #{inspect(error.resource)}"
+    end
+  end
+end