diff --git a/CHANGELOG.md b/CHANGELOG.md index ed9b9f7..6244a36 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,10 @@ # Changelog +## [Unreleased] + +### Fixed +- Resubscribe dependent controllers when a dependency controller is replaced after a params change, instead of leaving them attached to the stopped process and missing all further updates. Applies to both singleton dependencies and collection items replaced under the same id. + ## [0.2.0] - 2026-04-15 ### Added diff --git a/lib/solve.ex b/lib/solve.ex index e15c1a1..21ca417 100644 --- a/lib/solve.ex +++ b/lib/solve.ex @@ -891,7 +891,7 @@ defmodule Solve do snapshot_value = binding_value(binding, state) cond do - is_pid(source_pid) and same_single_binding?(current_binding_state, source_target) -> + is_pid(source_pid) and live_single_binding?(current_binding_state, source_pid) -> {:ok, state} is_pid(source_pid) -> @@ -912,6 +912,7 @@ defmodule Solve do kind: :single, source: binding.source, child_target: source_target, + source_pid: source_pid, subscription_ref: subscription_ref } @@ -957,39 +958,50 @@ defmodule Solve do end end) - current_binding_state = - get_binding_state(state, target, binding.key) || default_collection_binding_state(binding) - state = - Enum.reduce(desired_ids -- current_binding_state.ids, state, fn id, acc -> + Enum.reduce(desired_ids, state, fn id, acc -> child_target = {binding.source, id} + entry = Map.get(current_refs, id) case Map.get(acc.controller_pids_by_target, child_target) do pid when is_pid(pid) -> - encoder = build_collection_encoder(binding, id) - - case subscribe_controller_with_safely(pid, dependent_pid, encoder) do - {:ok, current_exposed_state, subscription_ref} -> - if not initial? do - send(dependent_pid, encoder.(current_exposed_state)) + if live_collection_binding_entry?(entry, pid) do + acc + else + # A stale entry points at a replaced process, so its subscription + # died with it; dropping the entry is enough. + acc = + if entry do + delete_collection_binding_subscription(acc, target, binding.key, id) else - snapshot_item = Collection.get(desired_collection, id) + acc + end + + encoder = build_collection_encoder(binding, id) - if snapshot_item != current_exposed_state do + case subscribe_controller_with_safely(pid, dependent_pid, encoder) do + {:ok, current_exposed_state, subscription_ref} -> + if not initial? do send(dependent_pid, encoder.(current_exposed_state)) - end - end + else + snapshot_item = Collection.get(desired_collection, id) - put_collection_binding_subscription( - acc, - target, - binding.key, - id, - %{target: child_target, subscription_ref: subscription_ref} - ) + if snapshot_item != current_exposed_state do + send(dependent_pid, encoder.(current_exposed_state)) + end + end - :subscription_failed -> - acc + put_collection_binding_subscription( + acc, + target, + binding, + id, + %{target: child_target, source_pid: pid, subscription_ref: subscription_ref} + ) + + :subscription_failed -> + acc + end end _ -> @@ -1034,15 +1046,11 @@ defmodule Solve do end end - defp same_single_binding?( - %{kind: :single, child_target: child_target, subscription_ref: subscription_ref}, - source_target - ) - when not is_nil(subscription_ref) do - child_target == source_target + defp live_single_binding?(%{kind: :single, source_pid: source_pid}, current_pid) do + source_pid == current_pid end - defp same_single_binding?(_binding_state, _source_target), do: false + defp live_single_binding?(_binding_state, _current_pid), do: false defp get_binding_state(state, target, key) do state.dependency_subscription_state_by_target @@ -1072,11 +1080,11 @@ defmodule Solve do end end - defp put_collection_binding_subscription(state, target, key, id, entry) do + defp put_collection_binding_subscription(state, target, binding, id, entry) do binding_state = - get_binding_state(state, target, key) || %{subscription_refs_by_id: %{}, ids: []} + get_binding_state(state, target, binding.key) || default_collection_binding_state(binding) - put_binding_state(state, target, key, %{ + put_binding_state(state, target, binding.key, %{ binding_state | subscription_refs_by_id: Map.put(binding_state.subscription_refs_by_id, id, entry) }) @@ -1095,30 +1103,30 @@ defmodule Solve do end end - defp cleanup_binding_subscription(state, nil), do: state - - defp cleanup_binding_subscription(state, %{ - target: source_target, - subscription_ref: subscription_ref - }) do - unsubscribe_dependency_subscription(source_target, subscription_ref, state) - state + defp live_collection_binding_entry?(%{source_pid: source_pid}, current_pid) do + source_pid == current_pid end + defp live_collection_binding_entry?(_entry, _current_pid), do: false + + defp cleanup_binding_subscription(state, nil), do: state + defp cleanup_binding_subscription(state, %{ kind: :single, child_target: source_target, + source_pid: source_pid, subscription_ref: subscription_ref }) do - unsubscribe_dependency_subscription(source_target, subscription_ref, state) + unsubscribe_dependency_subscription(source_target, source_pid, subscription_ref, state) state end defp unsubscribe_collection_binding_entry(state, %{ target: source_target, + source_pid: source_pid, subscription_ref: subscription_ref }) do - unsubscribe_dependency_subscription(source_target, subscription_ref, state) + unsubscribe_dependency_subscription(source_target, source_pid, subscription_ref, state) state end @@ -1140,9 +1148,10 @@ defmodule Solve do defp cleanup_binding_state(state, %{ kind: :single, child_target: source_target, + source_pid: source_pid, subscription_ref: subscription_ref }) do - unsubscribe_dependency_subscription(source_target, subscription_ref, state) + unsubscribe_dependency_subscription(source_target, source_pid, subscription_ref, state) state end @@ -1155,12 +1164,15 @@ defmodule Solve do end) end - defp cleanup_binding_state(state, _binding_state), do: state - - defp unsubscribe_dependency_subscription(source_target, subscription_ref, state) do + defp unsubscribe_dependency_subscription(source_target, source_pid, subscription_ref, state) do case Map.get(state.controller_pids_by_target, source_target) do - pid when is_pid(pid) -> unsubscribe_controller_safely(pid, subscription_ref) - _ -> :ok + # Unsubscribe only from the process this subscription was made in; when + # the source was replaced or stopped, the subscription died with it. + ^source_pid when is_pid(source_pid) -> + unsubscribe_controller_safely(source_pid, subscription_ref) + + _ -> + :ok end end diff --git a/lib/solve/controller.ex b/lib/solve/controller.ex index 88d6ba7..5686f33 100644 --- a/lib/solve/controller.ex +++ b/lib/solve/controller.ex @@ -298,12 +298,20 @@ defmodule Solve.Controller do resolved_event_arities = Enum.map(event_arities, fn {event, [arity]} -> {event, arity} end) handle_info_arity = List.first(handle_info_arities) + event_arity_defs = + for {event, arity} <- resolved_event_arities do + quote do + def __solve_event_arity__(unquote(event)), do: unquote(arity) + end + end + quote do - @solve_controller_event_arities Map.new(unquote(Macro.escape(resolved_event_arities))) @solve_controller_handle_info_arity unquote(handle_info_arity) + unquote(event_arity_defs) + def __solve_event_arity__(event) when is_atom(event) do - Map.fetch!(@solve_controller_event_arities, event) + raise KeyError, key: event, term: unquote(Macro.escape(Map.new(resolved_event_arities))) end def __solve_handle_info_arity__, do: @solve_controller_handle_info_arity diff --git a/lib/solve/controller_spec.ex b/lib/solve/controller_spec.ex index 4e04c07..6dfb9cc 100644 --- a/lib/solve/controller_spec.ex +++ b/lib/solve/controller_spec.ex @@ -213,10 +213,6 @@ defmodule Solve.ControllerSpec do {:error, {:invalid_dependencies, name, dependencies}} end - defp validate_existing_dependency_bindings(_name, []) do - :ok - end - defp validate_existing_dependency_bindings(name, bindings) when is_list(bindings) do Enum.reduce_while(bindings, {:ok, MapSet.new()}, fn binding, {:ok, seen_keys} -> with {:ok, key} <- validate_existing_binding(name, binding), diff --git a/test/solve/solve_runtime_test.exs b/test/solve/solve_runtime_test.exs index cd87125..97a2d29 100644 --- a/test/solve/solve_runtime_test.exs +++ b/test/solve/solve_runtime_test.exs @@ -45,6 +45,38 @@ defmodule Solve.RuntimeTest do end end + defmodule ReplaceableMiddleController do + use Solve.Controller, events: [:bump] + + @impl true + def init(%{label: label, test_pid: test_pid}, _dependencies) do + send(test_pid, {:middle_init, label}) + %{label: label, count: 0} + end + + def bump(_payload, state), do: %{state | count: state.count + 1} + + @impl true + def expose(state, _dependencies, _init_params) do + %{label: state.label, count: state.count} + end + end + + defmodule MiddleDependentController do + use Solve.Controller, events: [] + + @impl true + def init(%{test_pid: test_pid}, dependencies) do + send(test_pid, {:middle_dependent_init, dependencies}) + :leaf + end + + @impl true + def expose(_state, dependencies, _init_params) do + %{middle: Map.get(dependencies, :middle)} + end + end + defmodule CrashyController do use Solve.Controller, events: [:crash] @@ -138,6 +170,40 @@ defmodule Solve.RuntimeTest do end end + defmodule ReplacementPropagationSolve do + use Solve + + @impl true + def controllers do + [ + controller!( + name: :source, + module: Solve.RuntimeTest.LifecycleSourceController, + params: fn %{app_params: app_params} -> + %{value: app_params.source_initial, test_pid: app_params.test_pid} + end + ), + controller!( + name: :middle, + module: Solve.RuntimeTest.ReplaceableMiddleController, + dependencies: [:source], + params: fn %{dependencies: dependencies, app_params: app_params} -> + case dependencies[:source] do + %{value: value} -> %{label: value, test_pid: app_params.test_pid} + _ -> false + end + end + ), + controller!( + name: :leaf, + module: Solve.RuntimeTest.MiddleDependentController, + dependencies: [:middle], + params: fn %{app_params: app_params} -> %{test_pid: app_params.test_pid} end + ) + ] + end + end + defmodule CrashySolve do use Solve @@ -600,6 +666,94 @@ defmodule Solve.RuntimeTest do assert Solve.controller_pid(app, :stable) == stable_pid end + test "dependents keep receiving updates after a singleton dependency is replaced" do + app = start_app(ReplacementPropagationSolve, %{source_initial: 1, test_pid: self()}) + + assert_receive {:middle_init, 1} + assert_receive {:middle_dependent_init, %{middle: %{label: 1, count: 0}}} + + assert Solve.subscribe(app, :leaf) == %{middle: %{label: 1, count: 0}} + + leaf_pid = Solve.controller_pid(app, :leaf) + middle_pid = Solve.controller_pid(app, :middle) + + assert :ok = Solve.dispatch(app, :source, :set, 2) + + assert_receive {:middle_init, 2} + refute await_pid_change(app, :middle, middle_pid) == middle_pid + + assert_receive %Solve.Message{ + type: :update, + payload: %Solve.Update{ + app: ^app, + controller_name: :leaf, + exposed_state: %{middle: %{label: 2, count: 0}} + } + } + + assert Solve.controller_pid(app, :leaf) == leaf_pid + refute_receive {:middle_dependent_init, _}, 50 + + assert :ok = Solve.dispatch(app, :middle, :bump, %{}) + + assert_receive %Solve.Message{ + type: :update, + payload: %Solve.Update{ + app: ^app, + controller_name: :leaf, + exposed_state: %{middle: %{label: 2, count: 1}} + } + } + end + + test "collection dependents keep receiving item updates after an item is replaced" do + app = start_app(CollectionSolve, %{columns: initial_columns(), test_pid: self()}) + + assert_receive {:catalog_init, _} + assert_receive {:column_init, 1, _} + assert_receive {:column_init, 2, _} + assert_receive {:collection_projection_init, :all, _} + assert_receive {:visible_projection_init, _} + + assert Solve.subscribe(app, :projection) == %{label: :all, titles: ["Todo", "Doing"]} + + item_pid = Solve.controller_pid(app, {:column, 1}) + projection_pid = Solve.controller_pid(app, :projection) + + updated_columns = [ + %{id: 1, title: "Backlog", visible?: true}, + %{id: 2, title: "Doing", visible?: false} + ] + + assert :ok = Solve.dispatch(app, :catalog, :set_columns, updated_columns) + + assert_receive {:column_init, 1, _} + refute await_pid_change(app, {:column, 1}, item_pid) == item_pid + + assert_receive %Solve.Message{ + type: :update, + payload: %Solve.Update{ + app: ^app, + controller_name: :projection, + exposed_state: %{label: :all, titles: ["Backlog", "Doing"]} + } + } + + assert :ok = Solve.dispatch(app, {:column, 1}, :rename, "Icebox") + + assert_receive %Solve.Message{ + type: :update, + payload: %Solve.Update{ + app: ^app, + controller_name: :projection, + exposed_state: %{label: :all, titles: ["Icebox", "Doing"]} + } + } + + assert Solve.controller_pid(app, :projection) == projection_pid + refute_receive {:collection_projection_init, _, _}, 50 + end + test "runtime crashes restart the controller within budget" do app = start_app(CrashySolve, %{initial: 1, test_pid: self()})