add a simple validation after every change

error log during running the tests

lib/radiator/outline/dispatch.ex

@@ -1,8 +1,7 @@
 defmodule Radiator.Outline.Dispatch do
   @moduledoc false
 
-  alias Radiator.Outline.Command
-  alias Radiator.Outline.EventProducer
+  alias Radiator.Outline.{Command, Event, EventProducer, Validations}
 
   def insert_node(attributes, user_id, event_id) do
     "insert_node"
@@ -33,6 +32,13 @@ defmodule Radiator.Outline.Dispatch do
   end
 
   def broadcast(event) do
+    if Mix.env() == :dev || Mix.env() == :test do
+      :ok =
+        event
+        |> Event.episode_id()
+        |> Validations.validate_tree_for_episode()
+    end
+
     Phoenix.PubSub.broadcast(Radiator.PubSub, "events", event)
   end
 

lib/radiator/outline/event.ex

@@ -10,6 +10,8 @@ defmodule Radiator.Outline.Event do
     NodeMovedEvent
   }
 
+  alias Radiator.Outline.NodeRepository
+
   def payload(%NodeInsertedEvent{} = event) do
     %{
       node_id: event.node.uuid,
@@ -46,4 +48,20 @@ defmodule Radiator.Outline.Event do
   def event_type(%NodeDeletedEvent{} = _event), do: "NodeDeletedEvent"
 
   def event_type(%NodeMovedEvent{} = _event), do: "NodeMovedEvent"
+
+  def episode_id(%NodeInsertedEvent{} = event) do
+    event.node.episode_id
+  end
+
+  def episode_id(%NodeContentChangedEvent{} = event) do
+    NodeRepository.get_node(event.node_id).episode_id
+  end
+
+  def episode_id(%NodeDeletedEvent{} = event) do
+    NodeRepository.get_node(event.next_id).episode_id
+  end
+
+  def episode_id(%NodeMovedEvent{} = event) do
+    NodeRepository.get_node(event.node_id).episode_id
+  end
 end

lib/radiator/outline/node_repository.ex

@@ -86,9 +86,9 @@ defmodule Radiator.Outline.NodeRepository do
 
   """
   def count_nodes_by_episode(episode_id) do
-    episode_id
-    |> list_nodes_by_episode()
-    |> Enum.count()
+    Node
+    |> where([p], p.episode_id == ^episode_id)
+    |> Repo.aggregate(:count)
   end
 
   @doc """

lib/radiator/outline/validations.ex

@@ -2,7 +2,8 @@ defmodule Radiator.Outline.Validations do
   @moduledoc """
     Collection of consistency validations for the outline tree.
   """
-
+  alias Radiator.Outline
+  alias Radiator.Outline.Node
   alias Radiator.Outline.NodeRepository
 
   def validate_consistency_for_move(
@@ -38,12 +39,64 @@ defmodule Radiator.Outline.Validations do
     end
   end
 
-  def validate_tree(episode_id) do
-    tree_nodes = get_node_tree
-    all_nodes_by_episode = NodeRepository.list_nodes_by_episode(episode_id)
-    # tree_nodes
-    # every level has 1 node with prev_id nil
-    # all other nodes in level have prev_id set and are connected to the previous node
-    Enum.size(tree_nodes) == Enum.size(all_nodes_by_episode)
+  @doc """
+  Validates a tree for an episode.
+  Returns :ok if the tree is valid
+  """
+  def validate_tree_for_episode(episode_id) do
+    {:ok, tree_nodes} = Outline.get_node_tree(episode_id)
+
+    if Enum.count(tree_nodes) == NodeRepository.count_nodes_by_episode(episode_id) do
+      validate_tree_nodes(tree_nodes)
+    else
+      {:error, :node_count_not_consistent}
+    end
+  end
+
+  # iterate through the levels of the tree
+  # every level has 1 node with prev_id nil
+  # all other nodes in level have prev_id set and are connected to the previous node
+  # should be used in dev and test only
+  # might crash if the tree is not consistent
+  defp validate_tree_nodes(tree_nodes) do
+    tree_nodes
+    |> Enum.group_by(& &1.parent_id)
+    |> Enum.map(fn {_level, nodes} ->
+      validate_sub_tree(nodes)
+    end)
+    |> Enum.reject(&(&1 == :ok))
+    |> first_error()
+  end
+
+  defp first_error([]), do: :ok
+  defp first_error([err | _]), do: err
+
+  defp validate_sub_tree(nodes) do
+    # get the node with prev_id nil
+    first_node = Enum.find(nodes, &(&1.prev_id == nil))
+    # get the rest of the nodes
+    rest_nodes = Enum.reject(nodes, &(&1.prev_id == nil))
+
+    if Enum.count(rest_nodes) + 1 != Enum.count(nodes) do
+      {:error, :prev_id_not_consistent}
+    else
+      validate_prev_node(first_node, rest_nodes, [])
+    end
+  end
+
+  def validate_prev_node(
+        %Node{uuid: id},
+        [%Node{prev_id: id} = node | rest_nodes],
+        searched_nodes
+      ) do
+    validate_prev_node(node, rest_nodes ++ searched_nodes, [])
   end
+
+  def validate_prev_node(%Node{}, [], []), do: :ok
+
+  def validate_prev_node(%Node{} = prev_node, [node | rest_nodes], search_nodes) do
+    validate_prev_node(prev_node, rest_nodes, search_nodes ++ [node])
+  end
+
+  def validate_prev_node(%Node{}, [], _search_nodes), do: {:error, :prev_id_not_consistent}
 end

lib/radiator_web/live/episode_live/index.ex

@@ -11,10 +11,8 @@ defmodule RadiatorWeb.EpisodeLive.Index do
   }
 
   alias Radiator.Outline.NodeRepository
-  # alias Radiator.EventStore
   alias Radiator.Podcast
   alias Radiator.Podcast.Episode
-
   alias RadiatorWeb.OutlineComponents
 
   @impl true

test/radiator/outline/validations_test.exs

@@ -0,0 +1,97 @@
+defmodule Radiator.Outline.ValidationsTest do
+  @moduledoc false
+  use Radiator.DataCase
+
+  alias Radiator.Outline.Node
+  alias Radiator.Outline.NodeRepository
+  alias Radiator.Outline.Validations
+
+  import Ecto.Query, warn: false
+
+  describe "validate_tree_for_episode/1" do
+    setup :complex_node_fixture
+
+    test "validates a tree", %{
+      node_1: %Node{episode_id: episode_id}
+    } do
+      assert :ok = Validations.validate_tree_for_episode(episode_id)
+    end
+
+    test "a level might have different subtrees", %{
+      node_1: %Node{episode_id: episode_id} = node_1
+    } do
+      {:ok, %Node{} = _nested_node} =
+        %{
+          episode_id: episode_id,
+          parent_id: node_1.uuid,
+          prev_id: nil,
+          content: "child of node 1"
+        }
+        |> NodeRepository.create_node()
+
+      assert :ok = Validations.validate_tree_for_episode(episode_id)
+    end
+
+    test "when two nodes share the same prev_id the tree is invalid", %{
+      node_2: %Node{episode_id: episode_id} = node_2
+    } do
+      {:ok, %Node{} = _node_invalid} =
+        %{
+          episode_id: episode_id,
+          parent_id: node_2.parent_id,
+          prev_id: node_2.prev_id
+        }
+        |> NodeRepository.create_node()
+
+      assert {:error, :prev_id_not_consistent} =
+               Validations.validate_tree_for_episode(episode_id)
+    end
+
+    test "when a nodes has a non connected prev_id the tree is invalid", %{
+      node_2: %Node{episode_id: episode_id} = node_2
+    } do
+      {:ok, %Node{} = _node_invalid} =
+        %{
+          episode_id: episode_id,
+          parent_id: node_2.parent_id,
+          prev_id: node_2.prev_id
+        }
+        |> NodeRepository.create_node()
+
+      assert {:error, :prev_id_not_consistent} =
+               Validations.validate_tree_for_episode(episode_id)
+    end
+
+    test "when a parent has two childs with prev_id nil the tree is invalid", %{
+      nested_node_1: %Node{episode_id: episode_id, parent_id: parent_id}
+    } do
+      {:ok, %Node{} = _node_invalid} =
+        %{
+          episode_id: episode_id,
+          parent_id: parent_id,
+          prev_id: nil,
+          content: "invalid node"
+        }
+        |> NodeRepository.create_node()
+
+      assert {:error, :prev_id_not_consistent} =
+               Validations.validate_tree_for_episode(episode_id)
+    end
+
+    test "a tree with a node where parent and prev are not consistent is invalid", %{
+      parent_node: %Node{episode_id: episode_id} = parent_node,
+      nested_node_2: nested_node_2
+    } do
+      {:ok, %Node{} = _node_invalid} =
+        %{
+          episode_id: episode_id,
+          parent_id: parent_node.uuid,
+          prev_id: nested_node_2.uuid
+        }
+        |> NodeRepository.create_node()
+
+      result = Validations.validate_tree_for_episode(episode_id)
+      assert {:error, :prev_id_not_consistent} = result
+    end
+  end
+end

test/radiator/outline_test.exs

@@ -920,133 +920,4 @@ defmodule Radiator.OutlineTest do
     node = Enum.filter(tree, fn n -> n.uuid == node.uuid end) |> List.first()
     assert node.level == level
   end
-
-  defp simple_node_fixture(_) do
-    episode = PodcastFixtures.episode_fixture()
-
-    node_1 =
-      node_fixture(
-        episode_id: episode.id,
-        parent_id: nil,
-        prev_id: nil,
-        content: "node_1"
-      )
-
-    node_2 =
-      node_fixture(
-        episode_id: episode.id,
-        parent_id: nil,
-        prev_id: node_1.uuid,
-        content: "node_2"
-      )
-
-    assert node_2.prev_id == node_1.uuid
-    assert node_1.prev_id == nil
-    assert node_1.parent_id == nil
-    assert node_2.parent_id == nil
-
-    %{
-      node_1: node_1,
-      node_2: node_2
-    }
-  end
-
-  defp complex_node_fixture(_) do
-    episode = PodcastFixtures.episode_fixture()
-
-    parent_node =
-      node_fixture(
-        episode_id: episode.id,
-        parent_id: nil,
-        prev_id: nil,
-        content: "root of all evil"
-      )
-
-    node_1 =
-      node_fixture(
-        episode_id: episode.id,
-        parent_id: parent_node.uuid,
-        prev_id: nil,
-        content: "node_1"
-      )
-
-    node_2 =
-      node_fixture(
-        episode_id: episode.id,
-        parent_id: parent_node.uuid,
-        prev_id: node_1.uuid,
-        content: "node_2"
-      )
-
-    node_3 =
-      node_fixture(
-        episode_id: episode.id,
-        parent_id: parent_node.uuid,
-        prev_id: node_2.uuid,
-        content: "node_3"
-      )
-
-    node_4 =
-      node_fixture(
-        episode_id: episode.id,
-        parent_id: parent_node.uuid,
-        prev_id: node_3.uuid,
-        content: "node_4"
-      )
-
-    node_5 =
-      node_fixture(
-        episode_id: episode.id,
-        parent_id: parent_node.uuid,
-        prev_id: node_4.uuid,
-        content: "node_5"
-      )
-
-    node_6 =
-      node_fixture(
-        episode_id: episode.id,
-        parent_id: parent_node.uuid,
-        prev_id: node_5.uuid,
-        content: "node_6"
-      )
-
-    nested_node_1 =
-      node_fixture(
-        episode_id: episode.id,
-        parent_id: node_3.uuid,
-        prev_id: nil,
-        content: "nested_node_1"
-      )
-
-    nested_node_2 =
-      node_fixture(
-        episode_id: episode.id,
-        parent_id: node_3.uuid,
-        prev_id: nested_node_1.uuid,
-        content: "nested_node_2"
-      )
-
-    assert node_5.prev_id == node_4.uuid
-    assert node_4.prev_id == node_3.uuid
-    assert node_3.prev_id == node_2.uuid
-    assert node_2.prev_id == node_1.uuid
-    assert node_1.prev_id == nil
-
-    assert nested_node_1.parent_id == node_3.uuid
-    assert nested_node_2.parent_id == node_3.uuid
-    assert nested_node_1.prev_id == nil
-    assert nested_node_2.prev_id == nested_node_1.uuid
-
-    %{
-      node_1: node_1,
-      node_2: node_2,
-      node_3: node_3,
-      node_4: node_4,
-      node_5: node_5,
-      node_6: node_6,
-      nested_node_1: nested_node_1,
-      nested_node_2: nested_node_2,
-      parent_node: parent_node
-    }
-  end
 end