From 0877ae231551c215206c25a739ce0a8ea945695d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jonatan=20K=C5=82osko?= Date: Wed, 12 Jul 2023 23:11:17 +0200 Subject: [PATCH 1/2] Add button for reevaluating apps on change and add retry for errors --- lib/livebook/session.ex | 4 +- lib/livebook/session/data.ex | 125 ++++++++------- .../components/core_components.ex | 32 ++-- lib/livebook_web/live/app_session_live.ex | 149 +++++++++++++++--- lib/livebook_web/live/output.ex | 28 ++-- .../live/output/control_component.ex | 2 +- .../live/output/control_form_component.ex | 4 +- .../live/output/frame_component.ex | 2 +- .../live/output/input_component.ex | 57 ++++--- lib/livebook_web/live/session_live.ex | 35 ++-- .../live/session_live/cell_component.ex | 2 +- test/livebook/session/data_test.exs | 133 ++++++++++++++-- test/livebook/session_test.exs | 2 +- .../live/app_session_live_test.exs | 19 ++- test/livebook_web/live/session_live_test.exs | 43 ++++- test/support/test_helpers.ex | 13 ++ 16 files changed, 487 insertions(+), 163 deletions(-) diff --git a/lib/livebook/session.ex b/lib/livebook/session.ex index 3a5951b24ec..cf1c2ac36a4 100644 --- a/lib/livebook/session.ex +++ b/lib/livebook/session.ex @@ -2182,8 +2182,8 @@ defmodule Livebook.Session do state end - defp handle_action(state, {:clean_up_input_values, input_values}) do - for {_input_id, value} <- input_values do + defp handle_action(state, {:clean_up_input_values, input_infos}) do + for {_input_id, %{value: value}} <- input_infos do case value do value when is_file_input_value(value) -> schedule_file_deletion(state, value.file_ref) diff --git a/lib/livebook/session/data.ex b/lib/livebook/session/data.ex index bf0e134add1..8dc5118b629 100644 --- a/lib/livebook/session/data.ex +++ b/lib/livebook/session/data.ex @@ -26,7 +26,7 @@ defmodule Livebook.Session.Data do :persistence_warnings, :section_infos, :cell_infos, - :input_values, + :input_infos, :bin_entries, :runtime, :smart_cell_definitions, @@ -52,7 +52,7 @@ defmodule Livebook.Session.Data do dirty: boolean(), section_infos: %{Section.id() => section_info()}, cell_infos: %{Cell.id() => cell_info()}, - input_values: %{input_id() => term()}, + input_infos: %{input_id() => input_info()}, bin_entries: list(cell_bin_entry()), runtime: Runtime.t(), smart_cell_definitions: list(Runtime.smart_cell_definition()), @@ -108,8 +108,8 @@ defmodule Livebook.Session.Data do evaluation_end: DateTime.t() | nil, evaluation_number: non_neg_integer(), outputs_batch_number: non_neg_integer(), - bound_to_input_ids: MapSet.t(input_id()), - new_bound_to_input_ids: MapSet.t(input_id()), + bound_to_inputs: %{input_id() => input_hash()}, + new_bound_to_inputs: %{input_id() => input_hash()}, identifiers_used: list(identifier :: term()) | :unknown, identifiers_defined: %{(identifier :: term()) => version :: term()}, data: t() @@ -132,6 +132,8 @@ defmodule Livebook.Session.Data do @type input_id :: String.t() + @type input_info :: %{value: term(), hash: input_hash()} + @type client :: {User.id(), client_id()} @type client_id :: Livebook.Utils.id() @@ -144,6 +146,8 @@ defmodule Livebook.Session.Data do # got stale. @type snapshot :: term() + @type input_hash :: term() + @type input_reading :: {input_id(), input_value :: term()} @type secrets :: %{(name :: String.t()) => Secret.t()} @@ -232,7 +236,7 @@ defmodule Livebook.Session.Data do | {:set_smart_cell_parents, Cell.t(), Section.t(), parent :: {Cell.t(), Section.t()} | nil} | {:report_delta, client_id(), Cell.t(), cell_source_tag(), Delta.t()} - | {:clean_up_input_values, %{input_id() => term()}} + | {:clean_up_input_values, %{input_id() => input_info()}} | :app_report_status | :app_recover | :app_terminate @@ -290,7 +294,7 @@ defmodule Livebook.Session.Data do persistence_warnings: [], section_infos: initial_section_infos(notebook), cell_infos: initial_cell_infos(notebook), - input_values: initial_input_values(notebook), + input_infos: initial_input_infos(notebook), bin_entries: [], runtime: default_runtime, smart_cell_definitions: [], @@ -322,14 +326,14 @@ defmodule Livebook.Session.Data do do: {cell.id, new_cell_info(cell, %{})} end - defp initial_input_values(notebook) do + defp initial_input_infos(notebook) do for section <- Notebook.all_sections(notebook), cell <- section.cells, Cell.evaluable?(cell), output <- cell.outputs, attrs <- Cell.find_inputs_in_output(output), into: %{}, - do: {attrs.id, attrs.default} + do: {attrs.id, input_info(attrs.default)} end @doc """ @@ -451,7 +455,7 @@ defmodule Livebook.Session.Data do data |> with_actions() |> delete_section(section, delete_cells) - |> garbage_collect_input_values() + |> garbage_collect_input_infos() |> update_validity_and_evaluation() |> update_smart_cell_bases(data) |> set_dirty() @@ -467,7 +471,7 @@ defmodule Livebook.Session.Data do data |> with_actions() |> delete_cell(cell, section) - |> garbage_collect_input_values() + |> garbage_collect_input_infos() |> update_validity_and_evaluation() |> update_smart_cell_bases(data) |> set_dirty() @@ -556,7 +560,7 @@ defmodule Livebook.Session.Data do data |> with_actions() |> add_cell_output(cell, output) - |> garbage_collect_input_values() + |> garbage_collect_input_infos() |> mark_dirty_if_persisting_outputs() |> wrap_ok() else @@ -571,7 +575,7 @@ defmodule Livebook.Session.Data do |> with_actions() |> add_cell_output(cell, output) |> finish_cell_evaluation(cell, section, metadata) - |> garbage_collect_input_values() + |> garbage_collect_input_infos() |> update_validity_and_evaluation() |> update_smart_cell_bases(data) |> mark_dirty_if_persisting_outputs() @@ -595,9 +599,10 @@ defmodule Livebook.Session.Data do def apply_operation(data, {:bind_input, _client_id, cell_id, input_id}) do with {:ok, cell, _section} <- Notebook.fetch_cell_and_section(data.notebook, cell_id), Cell.evaluable?(cell), - :evaluating <- data.cell_infos[cell.id].eval.status, - true <- Map.has_key?(data.input_values, input_id), - false <- MapSet.member?(data.cell_infos[cell.id].eval.new_bound_to_input_ids, input_id) do + cell_info <- data.cell_infos[cell.id], + :evaluating <- cell_info.eval.status, + true <- Map.has_key?(cell_info.eval.data.input_infos, input_id), + false <- Map.has_key?(cell_info.eval.new_bound_to_inputs, input_id) do data |> with_actions() |> bind_input(cell, input_id) @@ -714,7 +719,7 @@ defmodule Livebook.Session.Data do data |> with_actions() |> erase_outputs() - |> garbage_collect_input_values() + |> garbage_collect_input_infos() |> update_smart_cell_bases(data) |> wrap_ok() end @@ -821,7 +826,7 @@ defmodule Livebook.Session.Data do end def apply_operation(data, {:set_input_value, _client_id, input_id, value}) do - with true <- Map.has_key?(data.input_values, input_id) do + with true <- Map.has_key?(data.input_infos, input_id) do data |> with_actions() |> set_input_value(input_id, value) @@ -1208,17 +1213,17 @@ defmodule Livebook.Session.Data do defp add_cell_output({data, _} = data_actions, cell, output) do {[indexed_output], _counter} = Notebook.index_outputs([output], 0) - new_input_values = + new_input_infos = indexed_output |> Cell.find_inputs_in_output() - |> Map.new(fn attrs -> {attrs.id, attrs.default} end) + |> Map.new(fn attrs -> {attrs.id, input_info(attrs.default)} end) {data, _} = data_actions = data_actions |> set!( notebook: Notebook.add_cell_output(data.notebook, cell.id, output), - input_values: Map.merge(new_input_values, data.input_values) + input_infos: Map.merge(new_input_infos, data.input_infos) ) if data.cell_infos[cell.id].eval.status == :evaluating do @@ -1228,7 +1233,7 @@ defmodule Livebook.Session.Data do data_actions |> update_cell_eval_info!(cell.id, fn eval_info -> - update_in(eval_info.data.input_values, &Map.merge(new_input_values, &1)) + update_in(eval_info.data.input_infos, &Map.merge(new_input_infos, &1)) end) else data_actions @@ -1247,7 +1252,7 @@ defmodule Livebook.Session.Data do evaluation_time_ms: metadata.evaluation_time_ms, identifiers_used: metadata.identifiers_used, identifiers_defined: metadata.identifiers_defined, - bound_to_input_ids: eval_info.new_bound_to_input_ids, + bound_to_inputs: eval_info.new_bound_to_inputs, evaluation_end: DateTime.utc_now(), code_markers: metadata.code_markers } @@ -1440,7 +1445,7 @@ defmodule Livebook.Session.Data do evaluation_number: eval_info.evaluation_number + 1, outputs_batch_number: eval_info.outputs_batch_number + 1, evaluation_digest: info.sources.primary.digest, - new_bound_to_input_ids: MapSet.new(), + new_bound_to_inputs: %{}, # Keep the notebook state before evaluation data: data, # This is a rough estimate, the exact time is measured in the @@ -1465,9 +1470,11 @@ defmodule Livebook.Session.Data do defp bind_input(data_actions, cell, input_id) do data_actions |> update_cell_eval_info!(cell.id, fn eval_info -> + hash = eval_info.data.input_infos[input_id].hash + %{ eval_info - | new_bound_to_input_ids: MapSet.put(eval_info.new_bound_to_input_ids, input_id) + | new_bound_to_inputs: Map.put(eval_info.new_bound_to_inputs, input_id, hash) } end) end @@ -1829,7 +1836,7 @@ defmodule Livebook.Session.Data do defp set_input_value({data, _} = data_actions, input_id, value) do data_actions - |> set!(input_values: Map.put(data.input_values, input_id, value)) + |> set!(input_infos: Map.put(data.input_infos, input_id, input_info(value))) end defp set_runtime(data_actions, prev_data, runtime) do @@ -1975,20 +1982,20 @@ defmodule Livebook.Session.Data do {data, actions ++ [action]} end - defp garbage_collect_input_values({data, _} = data_actions) do + defp garbage_collect_input_infos({data, _} = data_actions) do if any_section_evaluating?(data) do # Wait if evaluation is ongoing as it may render inputs data_actions else - used_input_ids = data.notebook |> initial_input_values() |> Map.keys() - {input_values, unused_input_values} = Map.split(data.input_values, used_input_ids) + used_input_ids = data.notebook |> initial_input_infos() |> Map.keys() + {input_infos, unused_input_infos} = Map.split(data.input_infos, used_input_ids) - if unused_input_values == %{} do + if unused_input_infos == %{} do data_actions else data_actions - |> set!(input_values: input_values) - |> add_action({:clean_up_input_values, unused_input_values}) + |> set!(input_infos: input_infos) + |> add_action({:clean_up_input_values, unused_input_infos}) end end end @@ -2079,8 +2086,8 @@ defmodule Livebook.Session.Data do evaluation_end: nil, evaluation_number: 0, outputs_batch_number: 0, - bound_to_input_ids: MapSet.new(), - new_bound_to_input_ids: MapSet.new(), + bound_to_inputs: %{}, + new_bound_to_inputs: %{}, identifiers_used: [], identifiers_defined: %{}, snapshot: nil, @@ -2208,7 +2215,7 @@ defmodule Livebook.Session.Data do |> Notebook.evaluable_cells_with_section() |> Enum.filter(fn {cell, _} -> info = data.cell_infos[cell.id] - MapSet.member?(info.eval.bound_to_input_ids, input_id) + Map.has_key?(info.eval.bound_to_inputs, input_id) end) end @@ -2260,18 +2267,27 @@ defmodule Livebook.Session.Data do parent_snapshots = Enum.map(parent_ids, &cell_snapshots[&1]) - bound_input_values = + bound_input_current_hashes = for( - input_id <- info.eval.bound_to_input_ids, - do: {input_id, data.input_values[input_id]} + {input_id, _hash} <- info.eval.bound_to_inputs, + current_input_info = data.input_infos[input_id], + do: {input_id, current_input_info.hash} ) |> Enum.sort() - deps = {is_branch?, parent_snapshots, identifier_versions, bound_input_values} + deps = {is_branch?, parent_snapshots, identifier_versions, bound_input_current_hashes} :erlang.phash2(deps) end + defp input_info(value) do + %{value: value, hash: input_hash(value)} + end + + defp input_hash(value) do + :erlang.phash2(value) + end + defp identifier_deps(cell_id, graph, data) do info = data.cell_infos[cell_id] @@ -2405,18 +2421,7 @@ defmodule Livebook.Session.Data do |> Notebook.evaluable_cells_with_section() |> Enum.filter(fn {cell, _section} -> info = data.cell_infos[cell.id] - - case data.mode do - :default -> - match?( - %{status: :ready, validity: :stale, reevaluates_automatically: true}, - info.eval - ) - - :app -> - match?(%{status: :ready, validity: :stale}, info.eval) and - data.app_data.status.execution in [:executing, :executed] - end + match?(%{status: :ready, validity: :stale, reevaluates_automatically: true}, info.eval) end) cell_ids = for {cell, _section} <- cells_to_reevaluate, do: cell.id @@ -2438,12 +2443,12 @@ defmodule Livebook.Session.Data do |> Notebook.evaluable_cells_with_section() |> Enum.find_value(:executed, fn {cell, _section} -> case data.cell_infos[cell.id].eval do + %{status: :evaluating} -> :executing + %{status: :queued} -> :executing %{validity: :aborted} -> :error %{interrupted: true} -> :interrupted %{errored: true} -> :error %{validity: :fresh} -> :executing - %{status: :evaluating} -> :executing - %{status: :queued} -> :executing _ -> nil end end) @@ -2603,7 +2608,21 @@ defmodule Livebook.Session.Data do _ -> data end - Map.fetch(data.input_values, input_id) + with {:ok, info} <- Map.fetch(data.input_infos, input_id), do: {:ok, info.value} + end + + @doc """ + Returns the set of inputs which values changed since they have been + read by any of the cells. + """ + @spec changed_input_ids(t()) :: MapSet.t(input_id()) + def changed_input_ids(data) do + for {_cell_id, %{eval: eval_info}} <- data.cell_infos, + {input_id, read_hash} <- eval_info.bound_to_inputs, + input_info = data.input_infos[input_id], + read_hash != input_info.hash, + into: MapSet.new(), + do: input_id end @doc """ diff --git a/lib/livebook_web/components/core_components.ex b/lib/livebook_web/components/core_components.ex index 8046127c58e..2204e305065 100644 --- a/lib/livebook_web/components/core_components.ex +++ b/lib/livebook_web/components/core_components.ex @@ -500,28 +500,36 @@ defmodule LivebookWeb.CoreComponents do ~H""" - + + """ end - defp circle_class(:success), do: "bg-green-bright-400" - defp circle_class(:warning), do: "bg-yellow-bright-200" - defp circle_class(:error), do: "bg-red-400" - defp circle_class(:inactive), do: "bg-gray-500" - defp circle_class(:waiting), do: "bg-gray-400" - defp circle_class(:progressing), do: "bg-blue-500" + @doc """ + Returns background class based on the given variant. - defp animated_circle_class(:waiting), do: "bg-gray-300" - defp animated_circle_class(:progressing), do: "bg-blue-400" - defp animated_circle_class(_other), do: nil + See `status_indicator/1` for available variants. + """ + def status_circle_class(variant) + + def status_circle_class(:success), do: "bg-green-bright-400" + def status_circle_class(:warning), do: "bg-yellow-bright-200" + def status_circle_class(:error), do: "bg-red-400" + def status_circle_class(:inactive), do: "bg-gray-500" + def status_circle_class(:waiting), do: "bg-gray-400" + def status_circle_class(:progressing), do: "bg-blue-500" + + defp animated_status_circle_class(:waiting), do: "bg-gray-300" + defp animated_status_circle_class(:progressing), do: "bg-blue-400" + defp animated_status_circle_class(_other), do: nil @doc """ Renders an informative box as a placeholder for a list. diff --git a/lib/livebook_web/live/app_session_live.ex b/lib/livebook_web/live/app_session_live.ex index cbc485905ba..32183dd9c65 100644 --- a/lib/livebook_web/live/app_session_live.ex +++ b/lib/livebook_web/live/app_session_live.ex @@ -147,29 +147,53 @@ defmodule LivebookWeb.AppSessionLive do session_pid={@session.pid} client_id={@client_id} cell_id={output_view.cell_id} - input_values={output_view.input_values} + input_views={output_view.input_views} /> <%= if @data_view.app_status.execution == :error do %> -
-
- <.remix_icon icon="error-warning-line" class="text-xl" /> - Something went wrong +
+
+ Something went wrong +
+
+ + <.link + :if={@livebook_authenticated?} + navigate={~p"/sessions/#{@session.id}" <> "#cell-#{@data_view.errored_cell_id}"} + > + <.remix_icon icon="terminal-line" /> + + +
- <.link - :if={@livebook_authenticated?} - navigate={~p"/sessions/#{@session.id}" <> "#cell-#{@data_view.errored_cell_id}"} - > - Debug - <.remix_icon icon="arrow-right-line" /> -
<% end %>
-
- <.app_status status={@data_view.app_status} /> +
+ + + + <.app_status_circle status={@data_view.app_status} />
@@ -191,6 +215,64 @@ defmodule LivebookWeb.AppSessionLive do def render(assigns), do: auth_placeholder(assigns) + attr :status, :map, required: true + + defp app_status_circle(%{status: %{lifecycle: :shutting_down}} = assigns) do + ~H""" + <.app_status_indicator text="Shutting down" variant={:inactive} icon="stop-line" /> + """ + end + + defp app_status_circle(%{status: %{lifecycle: :deactivated}} = assigns) do + ~H""" + <.app_status_indicator text="Deactivated" variant={:inactive} icon="stop-line" /> + """ + end + + defp app_status_circle(%{status: %{execution: :executing}} = assigns) do + ~H""" + <.app_status_indicator text="Executing" variant={:progressing} icon="loader-3-line" spinning /> + """ + end + + defp app_status_circle(%{status: %{execution: :executed}} = assigns) do + ~H""" + <.app_status_indicator text="Executed" variant={:success} icon="check-line" /> + """ + end + + defp app_status_circle(%{status: %{execution: :error}} = assigns) do + ~H""" + <.app_status_indicator text="Error" variant={:error} icon="close-line" /> + """ + end + + defp app_status_circle(%{status: %{execution: :interrupted}} = assigns) do + ~H""" + <.app_status_indicator text="Interrupted" variant={:waiting} icon="pause-line" /> + """ + end + + attr :text, :string, required: true + attr :variant, :atom, required: true + attr :icon, :string, required: true + attr :spinning, :boolean, default: false + + defp app_status_indicator(assigns) do + ~H""" + + + <.remix_icon icon={@icon} class="text-white font-bold" /> + + + """ + end + defp get_page_title(notebook_name) do "Livebook - #{notebook_name}" end @@ -210,6 +292,22 @@ defmodule LivebookWeb.AppSessionLive do {:noreply, socket} end + def handle_event("queue_errored_cells_evaluation", %{}, socket) do + data = socket.private.data + + errored_cell_ids = + for {cell_id, %{eval: eval_info}} <- data.cell_infos, eval_info.errored, do: cell_id + + Session.queue_full_evaluation(socket.assigns.session.pid, errored_cell_ids) + + {:noreply, socket} + end + + def handle_event("queue_full_evaluation", %{}, socket) do + Session.queue_full_evaluation(socket.assigns.session.pid, []) + {:noreply, socket} + end + @impl true def handle_info({:operation, operation}, socket) do {:noreply, handle_operation(socket, operation)} @@ -297,6 +395,8 @@ defmodule LivebookWeb.AppSessionLive do end defp data_to_view(data) do + changed_input_ids = Session.Data.changed_input_ids(data) + %{ notebook_name: data.notebook.name, output_views: @@ -304,7 +404,7 @@ defmodule LivebookWeb.AppSessionLive do {cell_id, output} <- visible_outputs(data.notebook), do: %{ output: output, - input_values: input_values_for_output(output, data), + input_views: input_views_for_output(output, data, changed_input_ids), cell_id: cell_id } ), @@ -312,7 +412,8 @@ defmodule LivebookWeb.AppSessionLive do show_source: data.notebook.app_settings.show_source, slug: data.notebook.app_settings.slug, multi_session: data.notebook.app_settings.multi_session, - errored_cell_id: errored_cell_id(data) + errored_cell_id: errored_cell_id(data), + any_stale?: any_stale?(data) } end @@ -324,9 +425,18 @@ defmodule LivebookWeb.AppSessionLive do end) end - defp input_values_for_output(output, data) do + defp any_stale?(data) do + Enum.any?(data.cell_infos, &match?({_, %{eval: %{validity: :stale}}}, &1)) + end + + defp input_views_for_output(output, data, changed_input_ids) do input_ids = for attrs <- Cell.find_inputs_in_output(output), do: attrs.id - Map.take(data.input_values, input_ids) + + data.input_infos + |> Map.take(input_ids) + |> Map.new(fn {input_id, %{value: value}} -> + {input_id, %{value: value, changed: MapSet.member?(changed_input_ids, input_id)}} + end) end defp visible_outputs(notebook) do @@ -376,7 +486,4 @@ defmodule LivebookWeb.AppSessionLive do do: {idx, output} defp filter_output(_output), do: nil - - defp show_app_status?(%{execution: :executed, lifecycle: :active}), do: false - defp show_app_status?(_status), do: true end diff --git a/lib/livebook_web/live/output.ex b/lib/livebook_web/live/output.ex index 0c219d4b779..6b21ffdf124 100644 --- a/lib/livebook_web/live/output.ex +++ b/lib/livebook_web/live/output.ex @@ -11,7 +11,7 @@ defmodule LivebookWeb.Output do attr :outputs, :list, required: true attr :session_id, :string, required: true attr :session_pid, :any, required: true - attr :input_values, :map, required: true + attr :input_views, :map, required: true attr :dom_id_map, :map, required: true attr :client_id, :string, required: true attr :cell_id, :string, required: true @@ -29,7 +29,7 @@ defmodule LivebookWeb.Output do id: "output-#{idx}", session_id: @session_id, session_pid: @session_pid, - input_values: @input_values, + input_views: @input_views, client_id: @client_id, cell_id: @cell_id }) %> @@ -95,7 +95,7 @@ defmodule LivebookWeb.Output do id: id, session_id: session_id, session_pid: session_pid, - input_values: input_values, + input_views: input_views, client_id: client_id, cell_id: cell_id }) do @@ -105,7 +105,7 @@ defmodule LivebookWeb.Output do placeholder: Map.get(info, :placeholder, true), session_id: session_id, session_pid: session_pid, - input_values: input_values, + input_views: input_views, client_id: client_id, cell_id: cell_id ) @@ -115,7 +115,7 @@ defmodule LivebookWeb.Output do id: id, session_id: session_id, session_pid: session_pid, - input_values: input_values, + input_views: input_views, client_id: client_id, cell_id: cell_id }) do @@ -138,7 +138,7 @@ defmodule LivebookWeb.Output do outputs: outputs, session_id: session_id, session_pid: session_pid, - input_values: input_values, + input_views: input_views, client_id: client_id, cell_id: cell_id } @@ -177,7 +177,7 @@ defmodule LivebookWeb.Output do dom_id_map={%{}} session_id={@session_id} session_pid={@session_pid} - input_values={@input_values} + input_views={@input_views} client_id={@client_id} cell_id={@cell_id} /> @@ -191,7 +191,7 @@ defmodule LivebookWeb.Output do id: id, session_id: session_id, session_pid: session_pid, - input_values: input_values, + input_views: input_views, client_id: client_id, cell_id: cell_id }) do @@ -205,7 +205,7 @@ defmodule LivebookWeb.Output do outputs: outputs, session_id: session_id, session_pid: session_pid, - input_values: input_values, + input_views: input_views, client_id: client_id, cell_id: cell_id } @@ -224,7 +224,7 @@ defmodule LivebookWeb.Output do dom_id_map={%{}} session_id={@session_id} session_pid={@session_pid} - input_values={@input_values} + input_views={@input_views} client_id={@client_id} cell_id={@cell_id} /> @@ -236,14 +236,14 @@ defmodule LivebookWeb.Output do defp render_output({:input, attrs}, %{ id: id, - input_values: input_values, + input_views: input_views, session_pid: session_pid, client_id: client_id }) do live_component(Output.InputComponent, id: id, attrs: attrs, - input_values: input_values, + input_views: input_views, session_pid: session_pid, client_id: client_id ) @@ -251,14 +251,14 @@ defmodule LivebookWeb.Output do defp render_output({:control, attrs}, %{ id: id, - input_values: input_values, + input_views: input_views, session_pid: session_pid, client_id: client_id }) do live_component(Output.ControlComponent, id: id, attrs: attrs, - input_values: input_values, + input_views: input_views, session_pid: session_pid, client_id: client_id ) diff --git a/lib/livebook_web/live/output/control_component.ex b/lib/livebook_web/live/output/control_component.ex index da604c5da22..11af96c2554 100644 --- a/lib/livebook_web/live/output/control_component.ex +++ b/lib/livebook_web/live/output/control_component.ex @@ -54,7 +54,7 @@ defmodule LivebookWeb.Output.ControlComponent do module={LivebookWeb.Output.ControlFormComponent} id={@id} attrs={@attrs} - input_values={@input_values} + input_views={@input_views} session_pid={@session_pid} client_id={@client_id} /> diff --git a/lib/livebook_web/live/output/control_form_component.ex b/lib/livebook_web/live/output/control_form_component.ex index b1d5de82a39..2bdad9e3277 100644 --- a/lib/livebook_web/live/output/control_form_component.ex +++ b/lib/livebook_web/live/output/control_form_component.ex @@ -14,7 +14,7 @@ defmodule LivebookWeb.Output.ControlFormComponent do data = Map.new(assigns.attrs.fields, fn {field, input_attrs} -> - {field, assigns.input_values[input_attrs.id]} + {field, assigns.input_views[input_attrs.id].value} end) if data != prev_data do @@ -41,7 +41,7 @@ defmodule LivebookWeb.Output.ControlFormComponent do module={LivebookWeb.Output.InputComponent} id={"#{@id}-#{input_attrs.id}"} attrs={input_attrs} - input_values={@input_values} + input_views={@input_views} session_pid={@session_pid} client_id={@client_id} local={true} diff --git a/lib/livebook_web/live/output/frame_component.ex b/lib/livebook_web/live/output/frame_component.ex index be44b174819..23f85cfd648 100644 --- a/lib/livebook_web/live/output/frame_component.ex +++ b/lib/livebook_web/live/output/frame_component.ex @@ -85,7 +85,7 @@ defmodule LivebookWeb.Output.FrameComponent do dom_id_map={@persistent_id_map} session_id={@session_id} session_pid={@session_pid} - input_values={@input_values} + input_views={@input_views} client_id={@client_id} cell_id={@cell_id} /> diff --git a/lib/livebook_web/live/output/input_component.ex b/lib/livebook_web/live/output/input_component.ex index 0926dc81162..1f7fbc1ab61 100644 --- a/lib/livebook_web/live/output/input_component.ex +++ b/lib/livebook_web/live/output/input_component.ex @@ -12,12 +12,12 @@ defmodule LivebookWeb.Output.InputComponent do end def update(assigns, socket) do - value = assigns.input_values[assigns.attrs.id] + %{value: value, changed: changed} = assigns.input_views[assigns.attrs.id] socket = socket |> assign(assigns) - |> assign(value: value) + |> assign(value: value, changed: changed) {:ok, socket} end @@ -26,9 +26,7 @@ defmodule LivebookWeb.Output.InputComponent do def render(%{attrs: %{type: :image}} = assigns) do ~H"""
- <.label> - <%= @attrs.label %> - + <.input_label label={@attrs.label} changed={@changed} /> <.live_component module={LivebookWeb.Output.ImageInputComponent} id={"#{@id}-input"} @@ -46,9 +44,7 @@ defmodule LivebookWeb.Output.InputComponent do def render(%{attrs: %{type: :audio}} = assigns) do ~H"""
- <.label> - <%= @attrs.label %> - + <.input_label label={@attrs.label} changed={@changed} /> <.live_component module={LivebookWeb.Output.AudioInputComponent} id={"#{@id}-input"} @@ -64,9 +60,7 @@ defmodule LivebookWeb.Output.InputComponent do def render(%{attrs: %{type: :file}} = assigns) do ~H"""
- <.label> - <%= @attrs.label %> - + <.input_label label={@attrs.label} changed={@changed} /> <.live_component module={LivebookWeb.Output.FileInputComponent} id={"#{@id}-input"} @@ -85,9 +79,11 @@ defmodule LivebookWeb.Output.InputComponent do def render(%{attrs: %{type: :utc_datetime}} = assigns) do ~H"""
- <.label help="Choose the time in your local time zone"> - <%= @attrs.label %> - + <.input_label + label={@attrs.label} + changed={@changed} + help="Choose the time in your local time zone" + /> - <.label help="Choose the time in your local time zone"> - <%= @attrs.label %> - + <.input_label + label={@attrs.label} + changed={@changed} + help="Choose the time in your local time zone" + /> - <.label> - <%= @attrs.label %> - + <.input_label label={@attrs.label} changed={@changed} /> <.input_output id={"#{@id}-input"} attrs={@attrs} value={@value} myself={@myself} /> """ @@ -261,6 +257,27 @@ defmodule LivebookWeb.Output.InputComponent do """ end + attr :label, :string, required: true + attr :changed, :boolean, required: true + attr :help, :string, default: nil + + defp input_label(assigns) do + ~H""" + <.label help={@help}> +
+ <%= @label %> + + <.remix_icon icon="error-warning-line" /> + +
+ + """ + end + defp html_input_type(:number), do: "number" defp html_input_type(:color), do: "color" defp html_input_type(:url), do: "url" diff --git a/lib/livebook_web/live/session_live.ex b/lib/livebook_web/live/session_live.ex index 46c982fb92d..19b014ee853 100644 --- a/lib/livebook_web/live/session_live.ex +++ b/lib/livebook_web/live/session_live.ex @@ -2255,6 +2255,8 @@ defmodule LivebookWeb.SessionLive do # irrelevant changes to data don't change `@data_view`, so LV doesn't # have to traverse the whole template tree and no diff is sent to the client. defp data_to_view(data) do + changed_input_ids = Session.Data.changed_input_ids(data) + %{ file: data.file, persist_outputs: data.notebook.persist_outputs, @@ -2281,8 +2283,11 @@ defmodule LivebookWeb.SessionLive do |> Enum.map(fn {client_id, user_id} -> {client_id, data.users_map[user_id]} end) |> Enum.sort_by(fn {_client_id, user} -> user.name end), installing?: data.cell_infos[Cell.setup_cell_id()].eval.status == :evaluating, - setup_cell_view: %{cell_to_view(hd(data.notebook.setup_section.cells), data) | type: :setup}, - section_views: section_views(data.notebook.sections, data), + setup_cell_view: %{ + cell_to_view(hd(data.notebook.setup_section.cells), data, changed_input_ids) + | type: :setup + }, + section_views: section_views(data.notebook.sections, data, changed_input_ids), bin_entries: data.bin_entries, secrets: data.secrets, hub: Livebook.Hubs.fetch_hub!(data.notebook.hub_id), @@ -2334,7 +2339,7 @@ defmodule LivebookWeb.SessionLive do cells_status(cells, data) end - defp section_views(sections, data) do + defp section_views(sections, data, changed_input_ids) do sections |> Enum.map(& &1.name) |> names_to_html_ids() @@ -2350,7 +2355,7 @@ defmodule LivebookWeb.SessionLive do for parent <- Notebook.valid_parents_for(data.notebook, section.id) do %{id: parent.id, name: parent.name} end, - cell_views: Enum.map(section.cells, &cell_to_view(&1, data)) + cell_views: Enum.map(section.cells, &cell_to_view(&1, data, changed_input_ids)) } end) end @@ -2362,7 +2367,7 @@ defmodule LivebookWeb.SessionLive do %{id: section.id, name: section.name} end - defp cell_to_view(%Cell.Markdown{} = cell, _data) do + defp cell_to_view(%Cell.Markdown{} = cell, _data, _changed_input_ids) do %{ id: cell.id, type: :markdown, @@ -2370,7 +2375,7 @@ defmodule LivebookWeb.SessionLive do } end - defp cell_to_view(%Cell.Code{} = cell, data) do + defp cell_to_view(%Cell.Code{} = cell, data, changed_input_ids) do info = data.cell_infos[cell.id] %{ @@ -2378,19 +2383,19 @@ defmodule LivebookWeb.SessionLive do type: :code, language: cell.language, empty: cell.source == "", - eval: eval_info_to_view(cell, info.eval, data), + eval: eval_info_to_view(cell, info.eval, data, changed_input_ids), reevaluate_automatically: cell.reevaluate_automatically } end - defp cell_to_view(%Cell.Smart{} = cell, data) do + defp cell_to_view(%Cell.Smart{} = cell, data, changed_input_ids) do info = data.cell_infos[cell.id] %{ id: cell.id, type: :smart, empty: cell.source == "", - eval: eval_info_to_view(cell, info.eval, data), + eval: eval_info_to_view(cell, info.eval, data, changed_input_ids), status: info.status, js_view: cell.js_view, editor: @@ -2403,7 +2408,7 @@ defmodule LivebookWeb.SessionLive do } end - defp eval_info_to_view(cell, eval_info, data) do + defp eval_info_to_view(cell, eval_info, data, changed_input_ids) do %{ outputs: cell.outputs, doctest_summary: doctest_summary(eval_info.doctest_reports), @@ -2416,7 +2421,7 @@ defmodule LivebookWeb.SessionLive do evaluation_digest: encode_digest(eval_info.evaluation_digest), outputs_batch_number: eval_info.outputs_batch_number, # Pass input values relevant to the given cell - input_values: input_values_for_cell(cell, data) + input_views: input_views_for_cell(cell, data, changed_input_ids) } end @@ -2430,13 +2435,17 @@ defmodule LivebookWeb.SessionLive do %{doctests_count: doctests_count, failures_count: failures_count} end - defp input_values_for_cell(cell, data) do + defp input_views_for_cell(cell, data, changed_input_ids) do input_ids = for output <- cell.outputs, attrs <- Cell.find_inputs_in_output(output), do: attrs.id - Map.take(data.input_values, input_ids) + data.input_infos + |> Map.take(input_ids) + |> Map.new(fn {input_id, %{value: value}} -> + {input_id, %{value: value, changed: MapSet.member?(changed_input_ids, input_id)}} + end) end def app_status(%{sessions: [app_session | _]}), do: app_session.app_status diff --git a/lib/livebook_web/live/session_live/cell_component.ex b/lib/livebook_web/live/session_live/cell_component.ex index e024451d3a9..5db2b92b985 100644 --- a/lib/livebook_web/live/session_live/cell_component.ex +++ b/lib/livebook_web/live/session_live/cell_component.ex @@ -642,7 +642,7 @@ defmodule LivebookWeb.SessionLive.CellComponent do session_pid={@session_pid} client_id={@client_id} cell_id={@cell_view.id} - input_values={@cell_view.eval.input_values} + input_views={@cell_view.eval.input_views} />
""" diff --git a/test/livebook/session/data_test.exs b/test/livebook/session/data_test.exs index 402597cbe3c..14580859db6 100644 --- a/test/livebook/session/data_test.exs +++ b/test/livebook/session/data_test.exs @@ -879,8 +879,30 @@ defmodule Livebook.Session.DataTest do empty_map = %{} - assert {:ok, %{input_values: ^empty_map}, actions} = Data.apply_operation(data, operation) - assert {:clean_up_input_values, %{"i1" => "value"}} in actions + assert {:ok, %{input_infos: ^empty_map}, actions} = Data.apply_operation(data, operation) + + assert {:clean_up_input_values, %{"i1" => %{value: "value", hash: :erlang.phash2("value")}}} in actions + end + + test "marks cells bound to the deleted input as stale" do + input = %{id: "i1", type: :text, label: "Text", default: "hey"} + + data = + data_after_operations!([ + {:insert_section, @cid, 0, "s1"}, + {:insert_cell, @cid, "s1", 0, :code, "c1", %{}}, + {:insert_cell, @cid, "s1", 1, :code, "c2", %{}}, + {:set_runtime, @cid, connected_noop_runtime()}, + evaluate_cells_operations(["setup"]), + {:queue_cells_evaluation, @cid, ["c1"]}, + {:add_cell_evaluation_response, @cid, "c1", {:input, input}, eval_meta()}, + evaluate_cells_operations(["c2"], %{bind_inputs: %{"c2" => ["i1"]}}) + ]) + + operation = {:delete_cell, @cid, "c1"} + + assert {:ok, %{cell_infos: %{"c2" => %{eval: %{validity: :stale}}}}, _actions} = + Data.apply_operation(data, operation) end end @@ -1869,7 +1891,7 @@ defmodule Livebook.Session.DataTest do %{ cell_infos: %{ "c1" => %{ - eval: %{data: %{input_values: %{"i1" => "hey"}}} + eval: %{data: %{input_infos: %{"i1" => %{value: "hey"}}}} } } }, _} = Data.apply_operation(data, operation) @@ -2355,7 +2377,8 @@ defmodule Livebook.Session.DataTest do operation = {:add_cell_evaluation_response, @cid, "c1", {:input, input}, eval_meta()} - assert {:ok, %{input_values: %{"i1" => "hey"}}, _} = Data.apply_operation(data, operation) + assert {:ok, %{input_infos: %{"i1" => %{value: "hey"}}}, _} = + Data.apply_operation(data, operation) end test "stores default values for new nested inputs" do @@ -2373,7 +2396,8 @@ defmodule Livebook.Session.DataTest do operation = {:add_cell_evaluation_response, @cid, "c1", output, eval_meta()} - assert {:ok, %{input_values: %{"i1" => "hey"}}, _} = Data.apply_operation(data, operation) + assert {:ok, %{input_infos: %{"i1" => %{value: "hey"}}}, _} = + Data.apply_operation(data, operation) end test "keeps input values for inputs that existed" do @@ -2394,7 +2418,8 @@ defmodule Livebook.Session.DataTest do # Output the same input again operation = {:add_cell_evaluation_response, @cid, "c1", {:input, input}, eval_meta()} - assert {:ok, %{input_values: %{"i1" => "value"}}, _} = Data.apply_operation(data, operation) + assert {:ok, %{input_infos: %{"i1" => %{value: "value"}}}, _} = + Data.apply_operation(data, operation) end test "garbage collects input values that are no longer used" do @@ -2417,7 +2442,8 @@ defmodule Livebook.Session.DataTest do empty_map = %{} - assert {:ok, %{input_values: ^empty_map}, [{:clean_up_input_values, %{"i1" => "value"}}]} = + assert {:ok, %{input_infos: ^empty_map}, + [{:clean_up_input_values, %{"i1" => %{value: "value"}}}]} = Data.apply_operation(data, operation) end @@ -2441,7 +2467,8 @@ defmodule Livebook.Session.DataTest do # This time w don't output the input operation = {:add_cell_evaluation_response, @cid, "c1", {:ok, 10}, eval_meta()} - assert {:ok, %{input_values: %{"i1" => "value"}}, _} = Data.apply_operation(data, operation) + assert {:ok, %{input_infos: %{"i1" => %{value: "value"}}}, _} = + Data.apply_operation(data, operation) end test "does not garbage collect inputs if another evaluation is ongoing" do @@ -2468,7 +2495,8 @@ defmodule Livebook.Session.DataTest do # This time w don't output the input operation = {:add_cell_evaluation_response, @cid, "c1", {:ok, 10}, eval_meta()} - assert {:ok, %{input_values: %{"i1" => "value"}}, _} = Data.apply_operation(data, operation) + assert {:ok, %{input_infos: %{"i1" => %{value: "value"}}}, _} = + Data.apply_operation(data, operation) end test "evaluating code cell before smart cell changes its parents" do @@ -2588,12 +2616,12 @@ defmodule Livebook.Session.DataTest do operation = {:bind_input, @cid, "c2", "i1"} - bound_to_input_ids = MapSet.new(["i1"]) + bound_to_inputs = %{"i1" => :erlang.phash2("hey")} assert {:ok, %{ cell_infos: %{ - "c2" => %{eval: %{new_bound_to_input_ids: ^bound_to_input_ids}} + "c2" => %{eval: %{new_bound_to_inputs: ^bound_to_inputs}} } }, _actions} = Data.apply_operation(data, operation) end @@ -3649,7 +3677,8 @@ defmodule Livebook.Session.DataTest do operation = {:set_input_value, @cid, "i1", "stuff"} - assert {:ok, %{input_values: %{"i1" => "stuff"}}, _} = Data.apply_operation(data, operation) + assert {:ok, %{input_infos: %{"i1" => %{value: "stuff"}}}, _} = + Data.apply_operation(data, operation) end test "given input value change, marks evaluated bound cells and their dependents as stale" do @@ -4108,7 +4137,7 @@ defmodule Livebook.Session.DataTest do Data.apply_operation(data, operation) end - test "changes status to :executing on automatic reevaluation" do + test "does not automatically reevaluate" do input = %{id: "i1", type: :text, label: "Text", default: "hey"} data = @@ -4116,6 +4145,7 @@ defmodule Livebook.Session.DataTest do {:insert_section, @cid, 0, "s1"}, {:insert_cell, @cid, "s1", 0, :code, "c1", %{}}, {:insert_cell, @cid, "s1", 1, :code, "c2", %{}}, + {:set_cell_attributes, @cid, "c2", %{reevaluate_automatically: true}}, {:set_runtime, @cid, connected_noop_runtime()}, evaluate_cells_operations(["setup"]), {:queue_cells_evaluation, @cid, ["c1"]}, @@ -4125,7 +4155,7 @@ defmodule Livebook.Session.DataTest do operation = {:set_input_value, @cid, "i1", "stuff"} - assert {:ok, %{app_data: %{status: %{execution: :executing}}}, _actions} = + assert {:ok, %{app_data: %{status: %{execution: :executed}}}, _actions} = Data.apply_operation(data, operation) end end @@ -4320,6 +4350,81 @@ defmodule Livebook.Session.DataTest do end end + describe "changed_input_ids/1" do + test "returns empty set when there are no inputs" do + assert Data.changed_input_ids(Data.new()) == MapSet.new() + end + + test "returns inputs which value changed since they have been bound to some cell" do + input1 = %{id: "i1", type: :text, label: "Text", default: "hey"} + input2 = %{id: "i2", type: :text, label: "Text", default: "hey"} + + data = + data_after_operations!([ + {:insert_section, @cid, 0, "s1"}, + {:insert_cell, @cid, "s1", 0, :code, "c1", %{}}, + {:insert_cell, @cid, "s1", 1, :code, "c2", %{}}, + {:insert_cell, @cid, "s1", 2, :code, "c3", %{}}, + {:insert_cell, @cid, "s1", 4, :code, "c4", %{}}, + {:set_runtime, @cid, connected_noop_runtime()}, + evaluate_cells_operations(["setup"]), + {:queue_cells_evaluation, @cid, ["c1", "c2"]}, + {:add_cell_evaluation_response, @cid, "c1", {:input, input1}, eval_meta()}, + {:add_cell_evaluation_response, @cid, "c2", {:input, input2}, eval_meta()}, + evaluate_cells_operations(["c3", "c4"], %{ + bind_inputs: %{"c3" => ["i1"], "c4" => ["i2"]} + }), + {:set_input_value, @cid, "i1", "new value"} + ]) + + assert Data.changed_input_ids(data) == MapSet.new(["i1"]) + end + + test "includes an input where one cell is bound with the old value and one with latest" do + input = %{id: "i1", type: :text, label: "Text", default: "hey"} + + data = + data_after_operations!([ + {:insert_section, @cid, 0, "s1"}, + {:insert_cell, @cid, "s1", 0, :code, "c1", %{}}, + {:insert_cell, @cid, "s1", 1, :code, "c2", %{}}, + {:insert_cell, @cid, "s1", 2, :code, "c3", %{}}, + {:set_runtime, @cid, connected_noop_runtime()}, + evaluate_cells_operations(["setup"]), + {:queue_cells_evaluation, @cid, ["c1"]}, + {:add_cell_evaluation_response, @cid, "c1", {:input, input}, eval_meta()}, + evaluate_cells_operations(["c2", "c3"], %{ + bind_inputs: %{"c2" => ["i1"], "c3" => ["i1"]} + }), + {:set_input_value, @cid, "i1", "new value"}, + # Reevaluate only cell 2, cell 3 is still stale + evaluate_cells_operations(["c2"], %{bind_inputs: %{"c2" => ["i1"]}}) + ]) + + assert Data.changed_input_ids(data) == MapSet.new(["i1"]) + end + + test "does not return removed inputs" do + input = %{id: "i1", type: :text, label: "Text", default: "hey"} + + data = + data_after_operations!([ + {:insert_section, @cid, 0, "s1"}, + {:insert_cell, @cid, "s1", 0, :code, "c1", %{}}, + {:insert_cell, @cid, "s1", 1, :code, "c2", %{}}, + {:set_runtime, @cid, connected_noop_runtime()}, + evaluate_cells_operations(["setup"]), + {:queue_cells_evaluation, @cid, ["c1"]}, + {:add_cell_evaluation_response, @cid, "c1", {:input, input}, eval_meta()}, + evaluate_cells_operations(["c2"], %{bind_inputs: %{"c2" => ["i1"]}}), + {:set_input_value, @cid, "i1", "new value"}, + {:delete_cell, @cid, "c1"} + ]) + + assert Data.changed_input_ids(data) == MapSet.new([]) + end + end + defp evaluate_cells_operations(cell_ids, opts \\ []) do uses = opts[:uses] || %{} versions = opts[:versions] || %{} diff --git a/test/livebook/session_test.exs b/test/livebook/session_test.exs index a4c3ecd4c59..1ea8ff57a57 100644 --- a/test/livebook/session_test.exs +++ b/test/livebook/session_test.exs @@ -239,7 +239,7 @@ defmodule Livebook.SessionTest do Session.convert_smart_cell(session.pid, smart_cell.id) - assert %{input_values: %{"input1" => "hey"}} = Session.get_data(session.pid) + assert %{input_infos: %{"input1" => %{value: "hey"}}} = Session.get_data(session.pid) end end diff --git a/test/livebook_web/live/app_session_live_test.exs b/test/livebook_web/live/app_session_live_test.exs index 003c3e20281..49f89aca389 100644 --- a/test/livebook_web/live/app_session_live_test.exs +++ b/test/livebook_web/live/app_session_live_test.exs @@ -108,7 +108,7 @@ defmodule LivebookWeb.AppSessionLiveTest do Livebook.App.close(app.pid) end - test "shows an error message when session errors", %{conn: conn} do + test "shows an error message when session errors", %{conn: conn, test: test} do slug = Livebook.Utils.random_short_id() app_settings = %{Livebook.Notebook.AppSettings.new() | slug: slug} @@ -125,8 +125,14 @@ defmodule LivebookWeb.AppSessionLiveTest do }, %{ Livebook.Notebook.Cell.new(:code) - | source: ~s/raise "oops"/, - id: "error-cell" + | id: "error-cell", + source: """ + # Fail on the first run + unless :persistent_term.get(#{inspect(test)}, false) do + :persistent_term.put(#{inspect(test)}, true) + raise "oops" + end + """ } ] } @@ -150,6 +156,13 @@ defmodule LivebookWeb.AppSessionLiveTest do assert render(view) =~ "Something went wrong" assert render(view) =~ ~p"/sessions/#{session_id}" <> "#cell-error-cell" + view + |> element("button", "Retry") + |> render_click() + + assert_receive {:app_updated, + %{pid: ^app_pid, sessions: [%{app_status: %{execution: :executed}}]}} + Livebook.App.close(app.pid) end end diff --git a/test/livebook_web/live/session_live_test.exs b/test/livebook_web/live/session_live_test.exs index 98f0688fdc6..1a0b8a4bac9 100644 --- a/test/livebook_web/live/session_live_test.exs +++ b/test/livebook_web/live/session_live_test.exs @@ -358,7 +358,7 @@ defmodule LivebookWeb.SessionLiveTest do id: "input1", type: :number, label: "Name", - default: "hey", + default: 1, destination: test } @@ -372,7 +372,7 @@ defmodule LivebookWeb.SessionLiveTest do |> element(~s/[data-el-outputs-container] form/) |> render_change(%{"html_value" => "10"}) - assert %{input_values: %{"input1" => 10}} = Session.get_data(session.pid) + assert %{input_infos: %{"input1" => %{value: 10}}} = Session.get_data(session.pid) assert_receive {:event, :input_ref, %{value: 10, type: :change}} end @@ -401,7 +401,7 @@ defmodule LivebookWeb.SessionLiveTest do |> element(~s/[data-el-outputs-container] form/) |> render_change(%{"html_value" => "line\r\nline"}) - assert %{input_values: %{"input1" => "line\nline"}} = Session.get_data(session.pid) + assert %{input_infos: %{"input1" => %{value: "line\nline"}}} = Session.get_data(session.pid) end test "form input changes are reflected only in local LV data", @@ -442,7 +442,7 @@ defmodule LivebookWeb.SessionLiveTest do # The new value is on the page assert render(view) =~ "sherlock" # but it's not reflected in the synchronized session data - assert %{input_values: %{"input1" => "initial"}} = Session.get_data(session.pid) + assert %{input_infos: %{"input1" => %{value: "initial"}}} = Session.get_data(session.pid) view |> element(~s/[data-el-outputs-container] button/, "Send") @@ -484,7 +484,7 @@ defmodule LivebookWeb.SessionLiveTest do ]) |> render_upload("data.txt") - assert %{input_values: %{"input1" => value}} = Session.get_data(session.pid) + assert %{input_infos: %{"input1" => %{value: value}}} = Session.get_data(session.pid) assert %{file_ref: file_ref, client_name: "data.txt"} = value @@ -598,6 +598,39 @@ defmodule LivebookWeb.SessionLiveTest do {:ok, view, _} = live(conn, ~p"/sessions/#{session.id}") refute render(view) =~ "line 1" end + + test "shows change indicator on bound inputs", + %{conn: conn, session: session, test: test} do + section_id = insert_section(session.pid) + + Process.register(self(), test) + + input = %{ + ref: :input_ref, + id: "input1", + type: :number, + label: "Name", + default: 1, + destination: test + } + + Session.subscribe(session.id) + + insert_cell_with_output(session.pid, section_id, {:input, input}) + + code = source_for_input_read(input.id) + cell_id = insert_text_cell(session.pid, section_id, :code, code) + Session.queue_cell_evaluation(session.pid, cell_id) + assert_receive {:operation, {:add_cell_evaluation_response, _, ^cell_id, _, _}} + + {:ok, view, _} = live(conn, ~p"/sessions/#{session.id}") + + refute render(view) =~ "This input has changed since it's been processed." + + Session.set_input_value(session.pid, input.id, 10) + + assert render(view) =~ "This input has changed since it's been processed." + end end describe "smart cells" do diff --git a/test/support/test_helpers.ex b/test/support/test_helpers.ex index 3e57be0f5db..17201ca0020 100644 --- a/test/support/test_helpers.ex +++ b/test/support/test_helpers.ex @@ -75,6 +75,19 @@ defmodule Livebook.TestHelpers do |> Macro.to_string() end + @doc """ + Builds code that renders the given output as part of evaluation. + """ + def source_for_input_read(input_id) do + quote do + send( + Process.group_leader(), + {:io_request, self(), make_ref(), {:livebook_get_input_value, unquote(input_id)}} + ) + end + |> Macro.to_string() + end + @doc """ Builds code that awaits for a messages before finishing. From baade4186b0bedd20153c0e4217b646b022cd097 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jonatan=20K=C5=82osko?= Date: Thu, 13 Jul 2023 12:08:25 +0200 Subject: [PATCH 2/2] Up --- lib/livebook_web/live/app_session_live.ex | 7 ++++++- lib/livebook_web/live/output/input_component.ex | 4 ++-- test/livebook_web/live/session_live_test.exs | 4 ++-- 3 files changed, 10 insertions(+), 5 deletions(-) diff --git a/lib/livebook_web/live/app_session_live.ex b/lib/livebook_web/live/app_session_live.ex index 32183dd9c65..a67733a8f53 100644 --- a/lib/livebook_web/live/app_session_live.ex +++ b/lib/livebook_web/live/app_session_live.ex @@ -187,7 +187,12 @@ defmodule LivebookWeb.AppSessionLive do
diff --git a/test/livebook_web/live/session_live_test.exs b/test/livebook_web/live/session_live_test.exs index 1a0b8a4bac9..4ad33824958 100644 --- a/test/livebook_web/live/session_live_test.exs +++ b/test/livebook_web/live/session_live_test.exs @@ -625,11 +625,11 @@ defmodule LivebookWeb.SessionLiveTest do {:ok, view, _} = live(conn, ~p"/sessions/#{session.id}") - refute render(view) =~ "This input has changed since it's been processed." + refute render(view) =~ "This input has changed since it was last processed." Session.set_input_value(session.pid, input.id, 10) - assert render(view) =~ "This input has changed since it's been processed." + assert render(view) =~ "This input has changed since it was last processed." end end