improve verify_characterisic, fix shelf bug

Author: matt-beanland

lib/diffo/provider/assigner/assignable_characteristic.ex

@@ -47,6 +47,12 @@ defmodule Diffo.Provider.AssignableCharacteristic do
       default :lowest
       constraints one_of: [:lowest, :highest, :random]
     end
+
+    attribute :thing, :atom do
+      description "the kind of item being assigned (e.g. :slot, :port); set from the pool declaration at build time"
+      public? true
+      allow_nil? true
+    end
   end
 
   calculations do
@@ -60,11 +66,15 @@ defmodule Diffo.Provider.AssignableCharacteristic do
       public? true
       argument :thing, :atom, allow_nil?: false
     end
+
+    calculate :free, :integer, Diffo.Provider.Calculations.FreeValues do
+      public? true
+    end
   end
 
   actions do
     create :create do
-      accept [:name, :first, :last, :assignable_type, :algorithm]
+      accept [:name, :first, :last, :assignable_type, :algorithm, :thing]
       argument :instance_id, :uuid
       argument :feature_id, :uuid
       change manage_relationship(:instance_id, :instance, type: :append)
@@ -77,7 +87,7 @@ defmodule Diffo.Provider.AssignableCharacteristic do
   end
 
   preparations do
-    prepare build(load: [:value])
+    prepare build(load: [:value, :free])
   end
 
   jason do

lib/diffo/provider/components/calculations/free_values.ex

@@ -0,0 +1,32 @@
+# SPDX-FileCopyrightText: 2025 diffo contributors <https://github.com/diffo-dev/diffo/graphs.contributors>
+#
+# SPDX-License-Identifier: MIT
+
+defmodule Diffo.Provider.Calculations.FreeValues do
+  @moduledoc false
+  use Ash.Resource.Calculation
+
+  @impl true
+  def load(_query, _opts, _context), do: []
+
+  @impl true
+  def calculate(records, _opts, _context) do
+    Enum.map(records, fn
+      %{thing: nil} ->
+        nil
+
+      record ->
+        count =
+          Diffo.Provider.AssignedToRelationship
+          |> Ash.Query.filter_input(
+            source_id: record.instance_id,
+            pool: record.name,
+            thing: record.thing
+          )
+          |> Ash.read!(domain: Diffo.Provider)
+          |> length()
+
+        record.last - record.first + 1 - count
+    end)
+  end
+end

lib/diffo/provider/extension/info.ex

@@ -27,4 +27,11 @@ defmodule Diffo.Provider.Extension.Info do
     Code.ensure_loaded?(module) and
       Diffo.Provider.Place.Extension in Ash.Resource.Info.extensions(module)
   end
+
+  @doc "Returns true if the module is a BaseCharacteristic-derived resource (or Characteristic itself)"
+  @spec characteristic?(module()) :: boolean()
+  def characteristic?(module) do
+    Code.ensure_loaded?(module) and
+      Diffo.Provider.Characteristic.Extension in Ash.Resource.Info.extensions(module)
+  end
 end

lib/diffo/provider/extension/pool.ex

@@ -10,9 +10,9 @@ defmodule Diffo.Provider.Extension.Pool do
 
   @doc "Creates AssignableCharacteristic nodes for each declared pool during the build action"
   def create_pools(result, pools) when is_struct(result) and is_list(pools) do
-    Enum.reduce_while(pools, {:ok, result}, fn %__MODULE__{name: name}, {:ok, acc} ->
+    Enum.reduce_while(pools, {:ok, result}, fn %__MODULE__{name: name, thing: thing}, {:ok, acc} ->
       case Diffo.Provider.AssignableCharacteristic
-           |> Ash.Changeset.for_create(:create, %{name: name, instance_id: acc.id})
+           |> Ash.Changeset.for_create(:create, %{name: name, thing: thing, instance_id: acc.id})
            |> Ash.create() do
         {:ok, _} -> {:cont, {:ok, acc}}
         {:error, error} -> {:halt, {:error, error}}

lib/diffo/provider/extension/verifiers/verify_characteristics.ex

@@ -3,11 +3,12 @@
 # SPDX-License-Identifier: MIT
 
 defmodule Diffo.Provider.Extension.Verifiers.VerifyCharacteristics do
-  @moduledoc "Verifies characteristic names are unique and value_type modules exist"
+  @moduledoc "Verifies characteristic names are unique and value_type modules exist and extend BaseCharacteristic"
   use Spark.Dsl.Verifier
 
   alias Spark.Dsl.Verifier
   alias Spark.Error.DslError
+  alias Diffo.Provider.Extension.Info
 
   @impl true
   def verify(dsl_state) do
@@ -30,17 +31,30 @@ defmodule Diffo.Provider.Extension.Verifiers.VerifyCharacteristics do
       Enum.reduce(characteristics, [], fn char, acc ->
         case module_from_value_type(char.value_type) do
           {:ok, module} ->
-            if Code.ensure_loaded?(module) do
-              acc
-            else
-              [
-                DslError.exception(
-                  module: resource,
-                  path: [:provider, :characteristics, char.name],
-                  message: "characteristics: value_type #{inspect(module)} does not exist"
-                )
-                | acc
-              ]
+            cond do
+              !Code.ensure_loaded?(module) ->
+                [
+                  DslError.exception(
+                    module: resource,
+                    path: [:provider, :characteristics, char.name],
+                    message: "characteristics: value_type #{inspect(module)} does not exist"
+                  )
+                  | acc
+                ]
+
+              !Info.characteristic?(module) ->
+                [
+                  DslError.exception(
+                    module: resource,
+                    path: [:provider, :characteristics, char.name],
+                    message:
+                      "characteristics: value_type #{inspect(module)} does not extend BaseCharacteristic"
+                  )
+                  | acc
+                ]
+
+              true ->
+                acc
             end
 
           :error ->

lib/diffo/provider/extension/verifiers/verify_features.ex

@@ -3,11 +3,12 @@
 # SPDX-License-Identifier: MIT
 
 defmodule Diffo.Provider.Extension.Verifiers.VerifyFeatures do
-  @moduledoc "Verifies feature names are unique and feature characteristic value_type modules exist"
+  @moduledoc "Verifies feature names are unique and feature characteristic value_type modules exist and extend BaseCharacteristic"
   use Spark.Dsl.Verifier
 
   alias Spark.Dsl.Verifier
   alias Spark.Error.DslError
+  alias Diffo.Provider.Extension.Info
 
   @impl true
   def verify(dsl_state) do
@@ -45,18 +46,31 @@ defmodule Diffo.Provider.Extension.Verifiers.VerifyFeatures do
           Enum.reduce(feature.characteristics || [], [], fn char, inner_acc ->
             case module_from_value_type(char.value_type) do
               {:ok, module} ->
-                if Code.ensure_loaded?(module) do
-                  inner_acc
-                else
-                  [
-                    DslError.exception(
-                      module: resource,
-                      path: [:provider, :features, feature.name, :characteristics, char.name],
-                      message:
-                        "features: characteristic value_type #{inspect(module)} does not exist"
-                    )
-                    | inner_acc
-                  ]
+                cond do
+                  !Code.ensure_loaded?(module) ->
+                    [
+                      DslError.exception(
+                        module: resource,
+                        path: [:provider, :features, feature.name, :characteristics, char.name],
+                        message:
+                          "features: characteristic value_type #{inspect(module)} does not exist"
+                      )
+                      | inner_acc
+                    ]
+
+                  !Info.characteristic?(module) ->
+                    [
+                      DslError.exception(
+                        module: resource,
+                        path: [:provider, :features, feature.name, :characteristics, char.name],
+                        message:
+                          "features: characteristic value_type #{inspect(module)} does not extend BaseCharacteristic"
+                      )
+                      | inner_acc
+                    ]
+
+                  true ->
+                    inner_acc
                 end
 
               :error ->

test/provider/extension/instance_transformer_test.exs

@@ -41,10 +41,9 @@ defmodule Diffo.Provider.Extension.InstanceTransformerTest do
     test "bakes characteristics/0 onto the resource" do
       chars = ShelfInstance.characteristics()
       assert is_list(chars)
-      assert length(chars) == 3
+      assert length(chars) == 2
       names = Enum.map(chars, & &1.name)
       assert :shelf in names
-      assert :slots in names
       assert :shelves in names
     end
 
@@ -54,7 +53,7 @@ defmodule Diffo.Provider.Extension.InstanceTransformerTest do
     end
 
     test "characteristics are also accessible via Info" do
-      assert length(Info.characteristics(ShelfInstance)) == 3
+      assert length(Info.characteristics(ShelfInstance)) == 2
       # Card has :card characteristic; :ports moved to pools do
       assert length(Info.characteristics(CardInstance)) == 1
     end

test/provider/extension/instance_verifier_test.exs

@@ -217,6 +217,34 @@ defmodule Diffo.Provider.Extension.InstanceVerifierTest do
         end
       )
     end
+
+    test "value_type not extending BaseCharacteristic warns DslError on compilation" do
+      Util.assert_compile_time_warning(
+        Spark.Error.DslError,
+        "characteristics: value_type Diffo.Test.Instance.ShelfInstance does not extend BaseCharacteristic",
+        fn ->
+          defmodule InvalidCharBaseType do
+            alias Diffo.Provider.BaseInstance
+            use Ash.Resource, fragments: [BaseInstance], domain: Diffo.Test.Servo
+
+            resource do
+              description "resource with characteristic value_type that is not a BaseCharacteristic"
+            end
+
+            provider do
+              specification do
+                id "cd29956f-6c68-44cc-bf54-705eb8d2f754"
+                name "invalid"
+              end
+
+              characteristics do
+                characteristic :foo, Diffo.Test.Instance.ShelfInstance
+              end
+            end
+          end
+        end
+      )
+    end
   end
 
   describe "features verifier" do
@@ -312,6 +340,36 @@ defmodule Diffo.Provider.Extension.InstanceVerifierTest do
         end
       )
     end
+
+    test "feature characteristic value_type not extending BaseCharacteristic warns DslError on compilation" do
+      Util.assert_compile_time_warning(
+        Spark.Error.DslError,
+        "features: characteristic value_type Diffo.Test.Instance.ShelfInstance does not extend BaseCharacteristic",
+        fn ->
+          defmodule InvalidFeatureCharBaseType do
+            alias Diffo.Provider.BaseInstance
+            use Ash.Resource, fragments: [BaseInstance], domain: Diffo.Test.Servo
+
+            resource do
+              description "resource with feature characteristic value_type that is not a BaseCharacteristic"
+            end
+
+            provider do
+              specification do
+                id "cd29956f-6c68-44cc-bf54-705eb8d2f754"
+                name "invalid"
+              end
+
+              features do
+                feature :my_feature do
+                  characteristic :baz, Diffo.Test.Instance.ShelfInstance
+                end
+              end
+            end
+          end
+        end
+      )
+    end
   end
 
   describe "parties verifier" do

test/support/resource/instance/shelf_instance.ex

@@ -12,9 +12,9 @@ defmodule Diffo.Test.Instance.ShelfInstance do
   alias Diffo.Provider.BaseInstance
   alias Diffo.Provider.Instance.Relationship
   alias Diffo.Provider.Extension.Characteristic
+  alias Diffo.Provider.Extension.Pool
   alias Diffo.Provider.Assigner
   alias Diffo.Provider.Assignment
-  alias Diffo.Provider.AssignableValue
   alias Diffo.Test.Servo
   alias Diffo.Test.Characteristic.ShelfCharacteristic
   alias Diffo.Test.Characteristic.DeploymentClass
@@ -53,10 +53,13 @@ defmodule Diffo.Test.Instance.ShelfInstance do
 
     characteristics do
       characteristic :shelf, ShelfCharacteristic
-      characteristic :slots, AssignableValue
       characteristic :shelves, {:array, ShelfCharacteristic}
     end
 
+    pools do
+      pool :slots, :slot
+    end
+
     parties do
       party :facilitator, Organization
       party :overseer, Person
@@ -97,6 +100,7 @@ defmodule Diffo.Test.Instance.ShelfInstance do
       change after_action(fn changeset, result, _context ->
                with {:ok, result} <-
                       Characteristic.update_all(result, changeset, characteristics()),
+                    {:ok, result} <- Pool.update_pools(result, changeset, pools()),
                     {:ok, result} <- Servo.get_shelf_by_id(result.id),
                     do: {:ok, result}
              end)
@@ -118,7 +122,7 @@ defmodule Diffo.Test.Instance.ShelfInstance do
       argument :assignment, :struct, constraints: [instance_of: Assignment]
 
       change after_action(fn changeset, result, _context ->
-               with {:ok, result} <- Assigner.assign(result, changeset, :slots, :slot),
+               with {:ok, result} <- Assigner.assign(result, changeset, :slots),
                     {:ok, result} <- Servo.get_shelf_by_id(result.id),
                     do: {:ok, result}
              end)