improved via assignment_relationship, added identity

matt-beanland · matt-beanland · commit c3fb12877f29 · 2026-05-20T02:35:32.000+09:30
diff --git a/AGENTS.md b/AGENTS.md
@@ -66,6 +66,7 @@ lib/diffo/provider/
     base_characteristic.ex      # Ash Fragment for typed characteristic resources
     base_relationship.ex        # Ash Fragment for shared Relationship structure
     defined_simple_relationship.ex    # DefinedSimpleRelationship — relationship with one optional embedded characteristic, frozen at creation
+    assignment_relationship.ex        # AssignmentRelationship — pool assignment relationship with top-level pool/thing/value scalar attributes
     relationship.ex                   # Relationship — mutable TMF service/resource relationship with graph Characteristic nodes
     calculations/
       characteristic_value.ex   # Calculation: builds .Value TypedStruct from record fields
diff --git a/lib/diffo/provider.ex b/lib/diffo/provider.ex
@@ -83,6 +83,12 @@ defmodule Diffo.Provider do
       define :delete_defined_simple_relationship, action: :destroy
     end
 
+    resource Diffo.Provider.AssignmentRelationship do
+      define :create_assignment_relationship, action: :create
+      define :get_assignment_relationship_by_id, action: :read, get_by: :id
+      define :delete_assignment_relationship, action: :destroy
+    end
+
     resource Diffo.Provider.AssignableCharacteristic do
       define :create_assignable_characteristic, action: :create
       define :get_assignable_characteristic_by_id, action: :read, get_by: :id
diff --git a/lib/diffo/provider/assigner/assigner.ex b/lib/diffo/provider/assigner/assigner.ex
@@ -4,15 +4,14 @@
 
 defmodule Diffo.Provider.Assigner do
   @moduledoc """
-  Helper to perform Assignment using `Diffo.Provider.DefinedSimpleRelationship`.
+  Helper to perform Assignment using `Diffo.Provider.AssignmentRelationship`.
 
-  Each assignment is stored as a `DefinedSimpleRelationship` with `type: :assignedTo`
-  and a single `NameValuePrimitive` characteristic carrying the thing name and assigned value.
+  Each assignment is stored as an `AssignmentRelationship` with top-level `pool`,
+  `thing`, and `value` attributes. This makes them filterable at the Cypher level
+  and usable in aggregate expressions.
   """
   alias Diffo.Provider.AssignableCharacteristic
-  alias Diffo.Provider.DefinedSimpleRelationship
-  alias Diffo.Type.NameValuePrimitive
-  alias Diffo.Type.Primitive
+  alias Diffo.Provider.AssignmentRelationship
 
   @doc """
   Assign a thing using the pool declared via `pools do` on the instance module.
@@ -42,96 +41,76 @@ defmodule Diffo.Provider.Assigner do
       _ ->
         case Map.get(assignment, :operation, :auto_assign) do
           :auto_assign ->
-            case next(result, pool, thing) do
-              {:ok, assigned} ->
-                relate_is_assigned(result, pool, thing, assigned, assignee_id)
-
-              {:error, error} ->
-                {:error, error}
+            with {:ok, value} <- next(result, pool, thing) do
+              create_assignment(result, pool, thing, value, assignee_id)
             end
 
           :assign ->
-            case assignable?(result, pool, thing, assignment.id) do
-              true ->
-                relate_is_assigned(result, pool, thing, assignment.id, assignee_id)
-
-              false ->
-                {:error, "#{thing} #{assignment.id} is not assignable"}
+            if assignable?(result, pool, thing, assignment.id) do
+              create_assignment(result, pool, thing, assignment.id, assignee_id)
+            else
+              {:error, "#{thing} #{assignment.id} is not assignable"}
             end
 
           :unassign ->
-            unrelate_is_assigned(result, pool, thing, assignment.id, assignee_id)
+            destroy_assignment(result, pool, thing, assignment.id, assignee_id)
         end
     end
   end
 
-  defp relate_is_assigned(result, _pool, thing, value, assignee_id)
-       when is_struct(result) and is_atom(thing) and is_integer(value) and
+  defp create_assignment(result, pool, thing, value, assignee_id)
+       when is_struct(result) and is_atom(pool) and is_atom(thing) and is_integer(value) and
               is_bitstring(assignee_id) do
-    case Diffo.Provider.create_defined_simple_relationship(%{
-           type: :assignedTo,
-           characteristic: %NameValuePrimitive{
-             name: thing,
-             value: Primitive.wrap("integer", value)
-           },
-           source_id: result.id,
-           target_id: assignee_id
-         }) do
-      {:ok, _relationship} ->
-        {:ok, result}
-
-      {:error, error} ->
-        {:error, error}
+    with {:ok, _} <-
+           Diffo.Provider.create_assignment_relationship(%{
+             pool: pool,
+             thing: thing,
+             value: value,
+             source_id: result.id,
+             target_id: assignee_id
+           }) do
+      {:ok, result}
     end
   end
 
-  defp unrelate_is_assigned(result, pool, thing, value, assignee_id)
+  defp destroy_assignment(result, pool, thing, value, assignee_id)
        when is_struct(result) and is_atom(pool) and is_atom(thing) and is_integer(value) and
               is_bitstring(assignee_id) do
     case find_assignment(result.id, assignee_id, pool, thing, value) do
       {:ok, nil} ->
         {:error, "#{thing} #{value} is not assigned to assignee #{assignee_id}"}
 
-      {:ok, relationship} ->
-        case Ash.destroy(relationship, domain: Diffo.Provider) do
-          :ok ->
-            {:ok, result}
-
-          {:error, error} ->
-            {:error, error}
+      {:ok, assignment} ->
+        with :ok <- Ash.destroy(assignment, domain: Diffo.Provider) do
+          {:ok, result}
         end
 
       {:error, error} ->
         {:error, error}
     end
   end
 
-  defp find_assignment(source_id, target_id, _pool, thing, value) do
-    case DefinedSimpleRelationship
-         |> Ash.Query.new()
-         |> Ash.Query.filter_input(source_id: source_id, target_id: target_id, type: :assignedTo)
-         |> Ash.read(domain: Diffo.Provider) do
-      {:ok, rels} ->
-        {:ok,
-         Enum.find(rels, fn rel ->
-           rel.characteristic &&
-             rel.characteristic.name == thing &&
-             Diffo.Unwrap.unwrap(rel.characteristic.value) == value
-         end)}
-
-      {:error, error} ->
-        {:error, error}
-    end
+  defp find_assignment(source_id, target_id, pool, thing, value) do
+    AssignmentRelationship
+    |> Ash.Query.filter_input(
+      source_id: source_id,
+      target_id: target_id,
+      pool: pool,
+      thing: thing,
+      value: value
+    )
+    |> Ash.read_one(domain: Diffo.Provider)
   end
 
   defp next(instance, pool, thing)
        when is_struct(instance) and is_atom(pool) and is_atom(thing) do
-    case pool_characteristic(instance.id, pool, thing) do
+    case pool_characteristic(instance.id, pool) do
       {:ok, nil} ->
         {:error, "pool #{pool} not found on instance #{instance.id}"}
 
       {:ok, char} ->
-        free = Enum.to_list(char.first..char.last) -- char.assigned_values
+        assigned = assigned_values_for(instance.id, thing)
+        free = Enum.to_list(char.first..char.last) -- assigned
 
         case free do
           [] ->
@@ -152,18 +131,30 @@ defmodule Diffo.Provider.Assigner do
 
   defp assignable?(instance, pool, thing, value)
        when is_struct(instance) and is_atom(pool) and is_atom(thing) and is_integer(value) do
-    case pool_characteristic(instance.id, pool, thing) do
-      {:ok, nil} -> false
-      {:ok, char} -> value in (Enum.to_list(char.first..char.last) -- char.assigned_values)
-      {:error, _} -> false
+    case pool_characteristic(instance.id, pool) do
+      {:ok, nil} ->
+        false
+
+      {:ok, char} ->
+        assigned = assigned_values_for(instance.id, thing)
+        value in (Enum.to_list(char.first..char.last) -- assigned)
+
+      {:error, _} ->
+        false
     end
   end
 
-  defp pool_characteristic(instance_id, pool, thing) do
+  defp assigned_values_for(instance_id, thing) do
+    AssignmentRelationship
+    |> Ash.Query.filter_input(source_id: instance_id, thing: thing)
+    |> Ash.read!(domain: Diffo.Provider)
+    |> Enum.map(& &1.value)
+  end
+
+  defp pool_characteristic(instance_id, pool) do
     AssignableCharacteristic
     |> Ash.Query.new()
     |> Ash.Query.filter_input(instance_id: instance_id, name: pool)
-    |> Ash.Query.load(assigned_values: [thing: thing])
     |> Ash.read_one(domain: Diffo.Provider)
   end
 end
diff --git a/lib/diffo/provider/components/assignment_relationship.ex b/lib/diffo/provider/components/assignment_relationship.ex
@@ -0,0 +1,106 @@
+# SPDX-FileCopyrightText: 2025 diffo contributors <https://github.com/diffo-dev/diffo/graphs.contributors>
+#
+# SPDX-License-Identifier: MIT
+
+defmodule Diffo.Provider.AssignmentRelationship do
+  @moduledoc """
+  Ash Resource for a pool assignment relationship.
+
+  Stores a single pool assignment as a direct Neo4j relationship between a source
+  (the pool-owning instance) and a target (the assignee instance). `pool`, `thing`,
+  and `value` are top-level scalar attributes, making them filterable at the Cypher
+  level and usable in aggregate filters via AshNeo4j #253.
+
+  Contrast with `DefinedSimpleRelationship`, which stores its characteristic as an
+  embedded `NameValuePrimitive` — suitable as a general primitive but opaque to the
+  data layer for filtering purposes.
+
+  Actions: **create** and **destroy** only. Assignments are commitments; to change
+  an assignment, destroy and recreate.
+  """
+  use Ash.Resource,
+    fragments: [Diffo.Provider.BaseRelationship],
+    otp_app: :diffo,
+    domain: Diffo.Provider
+
+  resource do
+    description "A pool assignment relationship between a source and target instance"
+    plural_name :assignment_relationships
+  end
+
+  neo4j do
+    relate [
+      {:source, :RELATES, :incoming, :Instance},
+      {:target, :RELATES, :outgoing, :Instance}
+    ]
+  end
+
+  jason do
+    pick [:type]
+
+    customize fn result, record ->
+      reference = %Diffo.Provider.Reference{
+        id: record.target_id,
+        href: record.target_href
+      }
+
+      list_name =
+        Diffo.Provider.Relationship.derive_relationship_characteristic_list_name(record.target_type)
+
+      characteristic = %{name: record.thing, value: record.value}
+
+      result
+      |> Diffo.Util.set(record.target_type, reference)
+      |> Diffo.Util.set(list_name, [characteristic])
+    end
+
+    order [:type, :resource, :service, :resourceRelationshipCharacteristic,
+           :serviceRelationshipCharacteristic]
+  end
+
+  actions do
+    create :create do
+      description "creates a pool assignment relationship between a source and target instance"
+      accept [:pool, :thing, :value]
+
+      argument :source_id, :uuid
+      argument :target_id, :string
+
+      change set_attribute(:type, :assignedTo)
+      change manage_relationship(:source_id, :source, type: :append)
+      change manage_relationship(:target_id, :target, type: :append)
+      change Diffo.Changes.DetailRelationship
+    end
+  end
+
+  attributes do
+    attribute :pool, :atom do
+      description "the pool name this assignment belongs to (e.g. :ports)"
+      allow_nil? false
+      public? true
+    end
+
+    attribute :thing, :atom do
+      description "the kind of thing being assigned (e.g. :port)"
+      allow_nil? false
+      public? true
+    end
+
+    attribute :value, :integer do
+      description "the assigned integer value"
+      allow_nil? false
+      public? true
+      constraints min: 0
+    end
+  end
+
+  identities do
+    identity :unique_assignment, [:source_id, :pool, :thing, :value] do
+      pre_check? true
+    end
+  end
+
+  preparations do
+    prepare build(sort: [created_at: :asc])
+  end
+end
diff --git a/lib/diffo/provider/components/base_instance.ex b/lib/diffo/provider/components/base_instance.ex
@@ -188,7 +188,7 @@ defmodule Diffo.Provider.BaseInstance do
       {:process_statuses, :STATUSES, :incoming, :ProcessStatus},
       {:forward_relationships, :RELATES, :outgoing, :Relationship},
       {:reverse_relationships, :RELATES, :incoming, :Relationship},
-      {:assignments, :RELATES, :outgoing, :DefinedSimpleRelationship},
+      {:assignments, :RELATES, :outgoing, :AssignmentRelationship},
       {:features, :HAS, :outgoing, :Feature},
       {:characteristics, :HAS, :outgoing, :Characteristic},
       {:entities, :RELATES, :outgoing, :EntityRef},
@@ -409,7 +409,7 @@ defmodule Diffo.Provider.BaseInstance do
       public? true
     end
 
-    has_many :assignments, Diffo.Provider.DefinedSimpleRelationship do
+    has_many :assignments, Diffo.Provider.AssignmentRelationship do
       description "the instance's outgoing pool assignment relationships"
       destination_attribute :source_id
       public? true
diff --git a/lib/diffo/provider/components/calculations/assigned_values.ex b/lib/diffo/provider/components/calculations/assigned_values.ex
@@ -14,11 +14,10 @@ defmodule Diffo.Provider.Calculations.AssignedValues do
     thing = context.arguments[:thing]
 
     Enum.map(records, fn record ->
-      Diffo.Provider.DefinedSimpleRelationship
-      |> Ash.Query.filter_input(source_id: record.instance_id, type: :assignedTo)
+      Diffo.Provider.AssignmentRelationship
+      |> Ash.Query.filter_input(source_id: record.instance_id, thing: thing)
       |> Ash.read!(domain: Diffo.Provider)
-      |> Enum.filter(fn rel -> rel.characteristic && rel.characteristic.name == thing end)
-      |> Enum.map(fn rel -> Diffo.Unwrap.unwrap(rel.characteristic.value) end)
+      |> Enum.map(& &1.value)
     end)
   end
 end
diff --git a/lib/diffo/provider/components/calculations/free_values.ex b/lib/diffo/provider/components/calculations/free_values.ex
@@ -17,12 +17,10 @@ defmodule Diffo.Provider.Calculations.FreeValues do
 
       record ->
         count =
-          Diffo.Provider.DefinedSimpleRelationship
-          |> Ash.Query.filter_input(source_id: record.instance_id, type: :assignedTo)
+          Diffo.Provider.AssignmentRelationship
+          |> Ash.Query.filter_input(source_id: record.instance_id, thing: record.thing)
           |> Ash.read!(domain: Diffo.Provider)
-          |> Enum.count(fn rel ->
-            rel.characteristic && rel.characteristic.name == record.thing
-          end)
+          |> length()
 
         record.last - record.first + 1 - count
     end)
diff --git a/test/provider/extension/assigner_test.exs b/test/provider/extension/assigner_test.exs
@@ -191,7 +191,7 @@ defmodule Diffo.Provider.Extension.AssignerTest do
 
       assert length(card.assignments) == 1
 
-      assigned_port = Diffo.Unwrap.unwrap(hd(card.assignments).characteristic.value)
+      assigned_port = hd(card.assignments).value
 
       {:ok, card} =
         Servo.assign_port(card, %{
