fix: hoist validation and transformation to top of call chain. before, set_docs would be called on the un-transformed sections (#221)

Author: marot

lib/spark/dsl/entity.ex

@@ -249,22 +249,20 @@ defmodule Spark.Dsl.Entity do
 
   @doc false
   def build(
-        entity,
+        %{
+          target: target,
+          schema: schema,
+          auto_set_fields: auto_set_fields,
+          transform: transform,
+          identifier: identifier,
+          singleton_entity_keys: singleton_entity_keys,
+          entities: nested_entity_definitions
+        },
         opts,
         nested_entities,
         anno,
         opts_anno
       ) do
-    %{
-      target: target,
-      schema: schema,
-      auto_set_fields: auto_set_fields,
-      transform: transform,
-      identifier: identifier,
-      singleton_entity_keys: singleton_entity_keys,
-      entities: nested_entity_definitions
-    } = validate_and_transform(entity, [], nil)
-
     with {:ok, opts, more_nested_entities} <-
            fetch_single_argument_entities_from_opts(
              opts,
@@ -458,56 +456,4 @@ defmodule Spark.Dsl.Entity do
 
     nil
   end
-
-  @doc """
-  Validates and transforms an entity structure, ensuring nested entities are properly formatted.
-
-  This function recursively processes a DSL entity and its nested entities, converting
-  single entity values to lists where needed and validating the structure.
-
-  ## Parameters
-
-  - `entity` - The entity to validate and transform
-  - `path` - The current path in the DSL structure (for error reporting)
-  - `module` - The module context (for error reporting)
-
-  ## Returns
-
-  Returns the transformed entity with normalized nested entity structures.
-  """
-  def validate_and_transform(entity, path \\ [], module \\ nil)
-
-  def validate_and_transform(%Spark.Dsl.Entity{} = entity, path, module) do
-    # Include the entity's name in the path when processing nested entities
-    nested_path = if entity.name, do: path ++ [entity.name], else: path
-
-    entities =
-      entity.entities
-      |> List.wrap()
-      |> Enum.map(fn
-        {key, %Spark.Dsl.Entity{} = value} ->
-          {key, [validate_and_transform(value, nested_path ++ [key], module)]}
-
-        {key, values} when is_list(values) ->
-          # Already a list, keep as is
-          {key, Enum.map(values, &validate_and_transform(&1, nested_path ++ [key], module))}
-
-        {key, value} ->
-          # Non-entity, non-list value - this is invalid
-          raise Spark.Error.DslError,
-            module: module,
-            path: nested_path ++ [key],
-            message:
-              "nested entity '#{key}' must be an entity or list of entities, got: #{inspect(value)}"
-      end)
-
-    %{entity | entities: entities}
-  end
-
-  def validate_and_transform(_, path, module) do
-    raise Spark.Error.DslError,
-      module: module,
-      path: path,
-      message: "Invalid entity structure"
-  end
 end

lib/spark/dsl/extension.ex

@@ -381,6 +381,26 @@ defmodule Spark.Dsl.Extension do
     end
   end
 
+  def set_docs(items) when is_list(items) do
+    Enum.map(items, &set_docs/1)
+  end
+
+  def set_docs(%Spark.Dsl.Entity{} = entity) do
+    entity
+    |> Map.put(:docs, Spark.Dsl.Extension.doc_entity(entity))
+    |> Map.put(
+      :entities,
+      Enum.map(entity.entities || [], fn {key, value} -> {key, set_docs(value)} end)
+    )
+  end
+
+  def set_docs(%Spark.Dsl.Section{} = section) do
+    section
+    |> Map.put(:entities, set_docs(section.entities))
+    |> Map.put(:sections, set_docs(section.sections))
+    |> Map.put(:docs, Spark.Dsl.Extension.doc_section(section))
+  end
+
   @doc false
   defmacro __using__(opts) do
     quote bind_quoted: [
@@ -397,19 +417,23 @@ defmodule Spark.Dsl.Extension do
       alias Spark.Dsl.Extension
       module_prefix = module_prefix || __MODULE__
 
-      @behaviour Extension
-      Extension.build(__MODULE__, module_prefix, sections, dsl_patches)
       @_sections sections
+                 |> Enum.map(&Extension.validate_and_transform_section(&1, __MODULE__))
+                 |> Extension.set_docs()
+      @_dsl_patches Extension.validate_and_transform_dsl_patches(dsl_patches, __MODULE__)
       @_transformers transformers
       @_verifiers verifiers
       @_persisters persisters
-      @_dsl_patches dsl_patches
       @_imports imports
       @_add_extensions add_extensions
+
+      @behaviour Extension
+      Extension.build(__MODULE__, module_prefix, @_sections, @_dsl_patches)
+
       @after_verify Spark.Dsl.Extension
 
       @doc false
-      def sections, do: set_docs(@_sections)
+      def sections, do: @_sections
       @doc false
       def verifiers, do: [Spark.Dsl.Verifiers.VerifyEntityUniqueness | @_verifiers]
       @doc false
@@ -424,26 +448,6 @@ defmodule Spark.Dsl.Extension do
       def dsl_patches, do: @_dsl_patches
       @doc false
       def add_extensions, do: @_add_extensions
-
-      defp set_docs(items) when is_list(items) do
-        Enum.map(items, &set_docs/1)
-      end
-
-      defp set_docs(%Spark.Dsl.Entity{} = entity) do
-        entity
-        |> Map.put(:docs, Spark.Dsl.Extension.doc_entity(entity))
-        |> Map.put(
-          :entities,
-          Enum.map(entity.entities || [], fn {key, value} -> {key, set_docs(value)} end)
-        )
-      end
-
-      defp set_docs(%Spark.Dsl.Section{} = section) do
-        section
-        |> Map.put(:entities, set_docs(section.entities))
-        |> Map.put(:sections, set_docs(section.sections))
-        |> Map.put(:docs, Spark.Dsl.Extension.doc_section(section))
-      end
     end
   end
 
@@ -834,7 +838,7 @@ defmodule Spark.Dsl.Extension do
          %Spark.Dsl.Patch.AddEntity{entity: entity} = dsl_patch,
          module
        ) do
-    entity = Spark.Dsl.Entity.validate_and_transform(entity, [], module)
+    entity = validate_and_transform_entity(entity, [], module)
     %{dsl_patch | entity: entity}
   end
 
@@ -855,8 +859,6 @@ defmodule Spark.Dsl.Extension do
           ] do
       alias Spark.Dsl.Extension
 
-      dsl_patches = Extension.validate_and_transform_dsl_patches(dsl_patches, __MODULE__)
-
       {:ok, agent} = Agent.start_link(fn -> [] end)
       agent_and_pid = {agent, self()}
 
@@ -904,11 +906,63 @@ defmodule Spark.Dsl.Extension do
       ) do
     %{
       section
-      | entities:
-          Enum.map(entities, &Spark.Dsl.Entity.validate_and_transform(&1, [section.name], module))
+      | entities: Enum.map(entities, &validate_and_transform_entity(&1, [section.name], module))
     }
   end
 
+  @doc """
+  Validates and transforms an entity structure, ensuring nested entities are properly formatted.
+
+  This function recursively processes a DSL entity and its nested entities, converting
+  single entity values to lists where needed and validating the structure.
+
+  ## Parameters
+
+  - `entity` - The entity to validate and transform
+  - `path` - The current path in the DSL structure (for error reporting)
+  - `module` - The module context (for error reporting)
+
+  ## Returns
+
+  Returns the transformed entity with normalized nested entity structures.
+  """
+  def validate_and_transform_entity(entity, path \\ [], module \\ nil)
+
+  def validate_and_transform_entity(%Spark.Dsl.Entity{} = entity, path, module) do
+    # Include the entity's name in the path when processing nested entities
+    nested_path = if entity.name, do: path ++ [entity.name], else: path
+
+    entities =
+      entity.entities
+      |> List.wrap()
+      |> Enum.map(fn
+        {key, %Spark.Dsl.Entity{} = value} ->
+          {key, [validate_and_transform_entity(value, nested_path ++ [key], module)]}
+
+        {key, values} when is_list(values) ->
+          # Already a list, keep as is
+          {key,
+           Enum.map(values, &validate_and_transform_entity(&1, nested_path ++ [key], module))}
+
+        {key, value} ->
+          # Non-entity, non-list value - this is invalid
+          raise Spark.Error.DslError,
+            module: module,
+            path: nested_path ++ [key],
+            message:
+              "nested entity '#{key}' must be an entity or list of entities, got: #{inspect(value)}"
+      end)
+
+    %{entity | entities: entities}
+  end
+
+  def validate_and_transform_entity(_, path, module) do
+    raise Spark.Error.DslError,
+      module: module,
+      path: path,
+      message: "Invalid entity structure"
+  end
+
   @doc false
   defmacro build_section(
              agent,
@@ -927,8 +981,6 @@ defmodule Spark.Dsl.Extension do
           generated: true do
       alias Spark.Dsl
 
-      section = Dsl.Extension.validate_and_transform_section(section, __MODULE__)
-
       {section_modules, entity_modules, opts_module} =
         Dsl.Extension.do_build_section(
           agent,

test/dsl_validation_test.exs

@@ -84,5 +84,47 @@ defmodule Spark.DslValidationTest do
         use Spark.Dsl, default_extensions: [extensions: Dsl]
       end
     end
+
+    test "dsl patches with nested entity definitions are properly transformed" do
+      defmodule DslPatchWithNestedEntities do
+        @moduledoc false
+
+        defmodule PatcherExtension do
+          @moduledoc false
+
+          @nested_entity %Spark.Dsl.Entity{
+            name: :nested_item,
+            target: Spark.Test.Step,
+            schema: []
+          }
+
+          @another_nested %Spark.Dsl.Entity{
+            name: :another_nested_item,
+            target: Spark.Test.Step,
+            schema: []
+          }
+
+          @patched_entity %Spark.Dsl.Entity{
+            name: :patched_entity,
+            target: Spark.Test.Step,
+            schema: [],
+            # Test both single entity and list formats in nested entities
+            entities: [
+              single: @nested_entity,
+              multiple: [@another_nested]
+            ]
+          }
+
+          @patch %Spark.Dsl.Patch.AddEntity{
+            section_path: [:steps],
+            entity: @patched_entity
+          }
+
+          use Spark.Dsl.Extension, dsl_patches: [@patch]
+        end
+
+        use Spark.Dsl, default_extensions: [extensions: [Spark.Test.Extension, PatcherExtension]]
+      end
+    end
   end
 end