From b0bd7540c08199af3881ecfeccabc5dc1e957df8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jonatan=20K=C5=82osko?= Date: Thu, 10 Jun 2021 14:54:55 +0200 Subject: [PATCH] Compute cell source digest on the client (#341) --- assets/css/js_interop.css | 4 ++ assets/js/cell/index.js | 39 ++++++++++- assets/js/cell/live_editor.js | 7 ++ assets/js/lib/utils.js | 11 +++ assets/package-lock.json | 15 +++- assets/package.json | 1 + lib/livebook/session.ex | 4 +- lib/livebook/session/data.ex | 35 +++++----- lib/livebook_web/live/session_live.ex | 21 +++++- .../live/session_live/cell_component.ex | 39 ++++++----- test/livebook/session/data_test.exs | 69 +++++++------------ 11 files changed, 157 insertions(+), 88 deletions(-) diff --git a/assets/css/js_interop.css b/assets/css/js_interop.css index 86ff665f5..37e7a40a8 100644 --- a/assets/css/js_interop.css +++ b/assets/css/js_interop.css @@ -60,6 +60,10 @@ solely client-side operations. @apply opacity-100 pointer-events-auto; } +[data-element="cell"] [data-element="change-indicator"]:not([data-js-shown]) { + @apply invisible; +} + [data-element="sections-list-item"][data-js-is-viewed] { @apply text-gray-900; } diff --git a/assets/js/cell/index.js b/assets/js/cell/index.js index 03579b4c0..8b9bf4e10 100644 --- a/assets/js/cell/index.js +++ b/assets/js/cell/index.js @@ -2,7 +2,7 @@ import { getAttributeOrThrow } from "../lib/attribute"; import LiveEditor from "./live_editor"; import Markdown from "./markdown"; import { globalPubSub } from "../lib/pub_sub"; -import { smoothlyScrollToElement } from "../lib/utils"; +import { md5Base64, smoothlyScrollToElement } from "../lib/utils"; import scrollIntoView from "scroll-into-view-if-needed"; /** @@ -24,11 +24,12 @@ const Cell = { insertMode: false, // For text cells (markdown or elixir) liveEditor: null, + evaluationDigest: null, }; if (["markdown", "elixir"].includes(this.props.type)) { this.pushEvent("cell_init", { cell_id: this.props.cellId }, (payload) => { - const { source, revision } = payload; + const { source, revision, evaluation_digest } = payload; const editorContainer = this.el.querySelector( `[data-element="editor-container"]` @@ -48,7 +49,39 @@ const Cell = { revision ); - // Setup markdown rendering. + // Setup change indicator + if (this.props.type === "elixir") { + this.state.evaluationDigest = evaluation_digest; + + const updateChangeIndicator = () => { + const indicator = this.el.querySelector( + `[data-element="change-indicator"]` + ); + + if (indicator) { + const source = this.state.liveEditor.getSource(); + const digest = md5Base64(source); + const changed = this.state.evaluationDigest !== digest; + indicator.toggleAttribute("data-js-shown", changed); + } + }; + + updateChangeIndicator(); + + this.handleEvent( + `evaluation_started:${this.props.cellId}`, + ({ evaluation_digest }) => { + this.state.evaluationDigest = evaluation_digest; + updateChangeIndicator(); + } + ); + + this.state.liveEditor.onChange((newSource) => { + updateChangeIndicator(); + }); + } + + // Setup markdown rendering if (this.props.type === "markdown") { const markdownContainer = this.el.querySelector( `[data-element="markdown-container"]` diff --git a/assets/js/cell/live_editor.js b/assets/js/cell/live_editor.js index 734939174..a1f952631 100644 --- a/assets/js/cell/live_editor.js +++ b/assets/js/cell/live_editor.js @@ -48,6 +48,13 @@ class LiveEditor { }); } + /** + * Returns current editor content. + */ + getSource() { + return this.source; + } + /** * Registers a callback called with a new cell content whenever it changes. */ diff --git a/assets/js/lib/utils.js b/assets/js/lib/utils.js index a1190d334..d8201ee2c 100644 --- a/assets/js/lib/utils.js +++ b/assets/js/lib/utils.js @@ -1,3 +1,6 @@ +import md5 from "crypto-js/md5"; +import encBase64 from "crypto-js/enc-base64"; + export function isMacOS() { return /(Mac|iPhone|iPod|iPad)/i.test(navigator.platform); } @@ -73,3 +76,11 @@ export function randomId() { const byteString = String.fromCharCode(...array); return btoa(byteString); } + +/** + * Calculates MD5 of the given string and returns + * the base64 encoded binary. + */ +export function md5Base64(string) { + return md5(string).toString(encBase64); +} diff --git a/assets/package-lock.json b/assets/package-lock.json index 4ea405e3d..349ee8534 100644 --- a/assets/package-lock.json +++ b/assets/package-lock.json @@ -8,6 +8,7 @@ "dependencies": { "@fontsource/inter": "^4.2.2", "@fontsource/jetbrains-mono": "^4.2.2", + "crypto-js": "^4.0.0", "dompurify": "^2.2.6", "hyperlist": "^1.0.0", "katex": "^0.13.2", @@ -45,14 +46,14 @@ } }, "../deps/phoenix": { - "version": "1.5.8", + "version": "1.5.9", "license": "MIT" }, "../deps/phoenix_html": { "version": "2.14.3" }, "../deps/phoenix_live_view": { - "version": "0.15.5", + "version": "0.15.7", "license": "MIT" }, "node_modules/@babel/code-frame": { @@ -3589,6 +3590,11 @@ "node": ">= 8" } }, + "node_modules/crypto-js": { + "version": "4.0.0", + "resolved": "https://registry.npmjs.org/crypto-js/-/crypto-js-4.0.0.tgz", + "integrity": "sha512-bzHZN8Pn+gS7DQA6n+iUmBfl0hO5DJq++QP3U6uTucDtk/0iGpXd/Gg7CGR0p8tJhofJyaKoWBuJI4eAO00BBg==" + }, "node_modules/css-color-names": { "version": "0.0.4", "dev": true, @@ -14903,6 +14909,11 @@ } } }, + "crypto-js": { + "version": "4.0.0", + "resolved": "https://registry.npmjs.org/crypto-js/-/crypto-js-4.0.0.tgz", + "integrity": "sha512-bzHZN8Pn+gS7DQA6n+iUmBfl0hO5DJq++QP3U6uTucDtk/0iGpXd/Gg7CGR0p8tJhofJyaKoWBuJI4eAO00BBg==" + }, "css-color-names": { "version": "0.0.4", "dev": true diff --git a/assets/package.json b/assets/package.json index 5c1424d18..98d71a21c 100644 --- a/assets/package.json +++ b/assets/package.json @@ -13,6 +13,7 @@ "dependencies": { "@fontsource/inter": "^4.2.2", "@fontsource/jetbrains-mono": "^4.2.2", + "crypto-js": "^4.0.0", "dompurify": "^2.2.6", "hyperlist": "^1.0.0", "katex": "^0.13.2", diff --git a/lib/livebook/session.ex b/lib/livebook/session.ex index 7827ab65e..57554c60c 100644 --- a/lib/livebook/session.ex +++ b/lib/livebook/session.ex @@ -687,7 +687,9 @@ defmodule Livebook.Session do Runtime.evaluate_code(state.data.runtime, cell.source, :main, cell.id, prev_ref, opts) - state + evaluation_digest = :erlang.md5(cell.source) + + handle_operation(state, {:evaluation_started, self(), cell.id, evaluation_digest}) end defp handle_action(state, {:stop_evaluation, _section}) do diff --git a/lib/livebook/session/data.ex b/lib/livebook/session/data.ex index be29db00b..e57abb1a7 100644 --- a/lib/livebook/session/data.ex +++ b/lib/livebook/session/data.ex @@ -55,7 +55,6 @@ defmodule Livebook.Session.Data do revision: cell_revision(), deltas: list(Delta.t()), revision_by_client_pid: %{pid() => cell_revision()}, - digest: String.t(), evaluation_digest: String.t() | nil } @@ -84,6 +83,7 @@ defmodule Livebook.Session.Data do | {:move_cell, pid(), Cell.id(), offset :: integer()} | {:move_section, pid(), Section.id(), offset :: integer()} | {:queue_cell_evaluation, pid(), Cell.id()} + | {:evaluation_started, pid(), Cell.id(), binary()} | {:add_cell_evaluation_output, pid(), Cell.id(), term()} | {:add_cell_evaluation_response, pid(), Cell.id(), term()} | {:reflect_evaluation_failure, pid()} @@ -136,7 +136,7 @@ defmodule Livebook.Session.Data do for section <- notebook.sections, cell <- section.cells, into: %{}, - do: {cell.id, new_cell_info(cell, %{})} + do: {cell.id, new_cell_info(%{})} end @doc """ @@ -267,6 +267,19 @@ defmodule Livebook.Session.Data do end end + def apply_operation(data, {:evaluation_started, _client_pid, id, evaluation_digest}) do + with {:ok, cell, _section} <- Notebook.fetch_cell_and_section(data.notebook, id), + %Cell.Elixir{} <- cell, + :evaluating <- data.cell_infos[cell.id].evaluation_status do + data + |> with_actions() + |> update_cell_info!(cell.id, &%{&1 | evaluation_digest: evaluation_digest}) + |> wrap_ok() + else + _ -> :error + end + end + def apply_operation(data, {:add_cell_evaluation_output, _client_pid, id, output}) do with {:ok, cell, _} <- Notebook.fetch_cell_and_section(data.notebook, id) do data @@ -470,7 +483,7 @@ defmodule Livebook.Session.Data do data_actions |> set!( notebook: Notebook.insert_cell(data.notebook, section_id, index, cell), - cell_infos: Map.put(data.cell_infos, cell.id, new_cell_info(cell, data.clients_map)) + cell_infos: Map.put(data.cell_infos, cell.id, new_cell_info(data.clients_map)) ) end @@ -649,7 +662,7 @@ defmodule Livebook.Session.Data do data_actions |> set!(notebook: Notebook.update_cell(data.notebook, id, &%{&1 | outputs: []})) |> update_cell_info!(id, fn info -> - %{info | evaluation_status: :evaluating, evaluation_digest: info.digest} + %{info | evaluation_status: :evaluating, evaluation_digest: nil} end) |> set_section_info!(section.id, evaluating_cell_id: id, evaluation_queue: ids) |> add_action({:start_evaluation, cell, section}) @@ -780,16 +793,13 @@ defmodule Livebook.Session.Data do new_source = JSInterop.apply_delta_to_string(transformed_new_delta, cell.source) - new_digest = compute_digest(new_source) - data_actions |> set!(notebook: Notebook.update_cell(data.notebook, cell.id, &%{&1 | source: new_source})) |> update_cell_info!(cell.id, fn info -> info = %{ info | deltas: info.deltas ++ [transformed_new_delta], - revision: info.revision + 1, - digest: new_digest + revision: info.revision + 1 } # Before receiving acknowledgement, the client receives all the other deltas, @@ -856,7 +866,7 @@ defmodule Livebook.Session.Data do } end - defp new_cell_info(cell, clients_map) do + defp new_cell_info(clients_map) do client_pids = Map.keys(clients_map) %{ @@ -865,11 +875,6 @@ defmodule Livebook.Session.Data do revision_by_client_pid: Map.new(client_pids, &{&1, 0}), validity_status: :fresh, evaluation_status: :ready, - digest: - case Map.fetch(cell, :source) do - {:ok, source} -> compute_digest(source) - :error -> nil - end, evaluation_digest: nil } end @@ -924,8 +929,6 @@ defmodule Livebook.Session.Data do set!(data_actions, dirty: dirty) end - defp compute_digest(source), do: :erlang.md5(source) - @doc """ Finds the cell that's currently being evaluated in the given section. """ diff --git a/lib/livebook_web/live/session_live.ex b/lib/livebook_web/live/session_live.ex index 21a11e9cc..414d089e3 100644 --- a/lib/livebook_web/live/session_live.ex +++ b/lib/livebook_web/live/session_live.ex @@ -325,9 +325,12 @@ defmodule LivebookWeb.SessionLive do case Notebook.fetch_cell_and_section(data.notebook, cell_id) do {:ok, cell, _section} -> + info = data.cell_infos[cell.id] + payload = %{ source: cell.source, - revision: data.cell_infos[cell.id].revision + revision: info.revision, + evaluation_digest: encode_digest(info.evaluation_digest) } {:reply, payload, socket} @@ -701,6 +704,16 @@ defmodule LivebookWeb.SessionLive do end end + defp after_operation( + socket, + _prev_socket, + {:evaluation_started, _client_pid, cell_id, evaluation_digest} + ) do + push_event(socket, "evaluation_started:#{cell_id}", %{ + evaluation_digest: encode_digest(evaluation_digest) + }) + end + defp after_operation(socket, _prev_socket, _operation), do: socket defp handle_actions(socket, actions) do @@ -745,6 +758,9 @@ defmodule LivebookWeb.SessionLive do defp ensure_integer(n) when is_integer(n), do: n defp ensure_integer(n) when is_binary(n), do: String.to_integer(n) + defp encode_digest(nil), do: nil + defp encode_digest(digest), do: Base.encode64(digest) + # Builds view-specific structure of data by cherry-picking # only the relevant attributes. # We then use `@data_view` in the templates and consequently @@ -820,8 +836,7 @@ defmodule LivebookWeb.SessionLive do empty?: cell.source == "", outputs: cell.outputs, validity_status: info.validity_status, - evaluation_status: info.evaluation_status, - changed?: info.evaluation_digest != nil and info.digest != info.evaluation_digest + evaluation_status: info.evaluation_status } end diff --git a/lib/livebook_web/live/session_live/cell_component.ex b/lib/livebook_web/live/session_live/cell_component.ex index 1044dea65..fe8ef14ea 100644 --- a/lib/livebook_web/live/session_live/cell_component.ex +++ b/lib/livebook_web/live/session_live/cell_component.ex @@ -222,7 +222,7 @@ defmodule LivebookWeb.SessionLive.CellComponent do <%= if @cell_view.type == :elixir do %>
- <%= render_cell_status(@cell_view.validity_status, @cell_view.evaluation_status, @cell_view.changed?) %> + <%= render_cell_status(@cell_view.validity_status, @cell_view.evaluation_status) %>
<% end %> @@ -363,43 +363,48 @@ defmodule LivebookWeb.SessionLive.CellComponent do """ end - defp render_cell_status(validity_status, evaluation_status, changed) + defp render_cell_status(validity_status, evaluation_status) - defp render_cell_status(_, :evaluating, changed) do - render_status_indicator("Evaluating", "bg-blue-500", "bg-blue-400", changed) + defp render_cell_status(_, :evaluating) do + render_status_indicator("Evaluating", "bg-blue-500", + animated_circle_class: "bg-blue-400", + change_indicator: true + ) end - defp render_cell_status(_, :queued, _) do - render_status_indicator("Queued", "bg-gray-500", "bg-gray-400", false) + defp render_cell_status(_, :queued) do + render_status_indicator("Queued", "bg-gray-500", animated_circle_class: "bg-gray-400") end - defp render_cell_status(:evaluated, _, changed) do - render_status_indicator("Evaluated", "bg-green-400", nil, changed) + defp render_cell_status(:evaluated, _) do + render_status_indicator("Evaluated", "bg-green-400", change_indicator: true) end - defp render_cell_status(:stale, _, changed) do - render_status_indicator("Stale", "bg-yellow-200", nil, changed) + defp render_cell_status(:stale, _) do + render_status_indicator("Stale", "bg-yellow-200", change_indicator: true) end - defp render_cell_status(:aborted, _, _) do - render_status_indicator("Aborted", "bg-red-400", nil, false) + defp render_cell_status(:aborted, _) do + render_status_indicator("Aborted", "bg-red-400") end - defp render_cell_status(_, _, _), do: nil + defp render_cell_status(_, _), do: nil - defp render_status_indicator(text, circle_class, animated_circle_class, show_changed) do + defp render_status_indicator(text, circle_class, opts \\ []) do assigns = %{ text: text, circle_class: circle_class, - animated_circle_class: animated_circle_class, - show_changed: show_changed + animated_circle_class: Keyword.get(opts, :animated_circle_class), + change_indicator: Keyword.get(opts, :change_indicator, false) } ~L"""
<%= @text %> - ">* + <%= if @change_indicator do %> + * + <% end %>
<%= if @animated_circle_class do %> diff --git a/test/livebook/session/data_test.exs b/test/livebook/session/data_test.exs index 5ac0405b1..627b8035b 100644 --- a/test/livebook/session/data_test.exs +++ b/test/livebook/session/data_test.exs @@ -851,6 +851,29 @@ defmodule Livebook.Session.DataTest do end end + describe "apply_operation/2 given :evaluation_started" do + test "updates cell evaluation digest" do + data = + data_after_operations!([ + {:insert_section, self(), 0, "s1"}, + {:insert_cell, self(), "s1", 0, :elixir, "c1"}, + {:set_runtime, self(), NoopRuntime.new()}, + {:queue_cell_evaluation, self(), "c1"} + ]) + + operation = {:evaluation_started, self(), "c1", "digest"} + + assert {:ok, + %{ + cell_infos: %{ + "c1" => %{ + evaluation_digest: "digest" + } + } + }, []} = Data.apply_operation(data, operation) + end + end + describe "apply_operation/2 given :add_cell_evaluation_output" do test "updates the cell outputs" do data = @@ -992,33 +1015,6 @@ defmodule Livebook.Session.DataTest do }, []} = Data.apply_operation(data, operation) end - test "updates cell evaluation digest" do - data = - data_after_operations!([ - {:insert_section, self(), 0, "s1"}, - {:insert_cell, self(), "s1", 0, :elixir, "c1"}, - {:insert_cell, self(), "s1", 1, :elixir, "c2"}, - {:set_runtime, self(), NoopRuntime.new()}, - {:queue_cell_evaluation, self(), "c1"} - ]) - - %{cell_infos: %{"c1" => %{digest: digest}}} = data - - operation = {:add_cell_evaluation_response, self(), "c1", {:ok, [1, 2, 3]}} - - assert {:ok, - %{ - cell_infos: %{ - "c1" => %{ - validity_status: :evaluated, - evaluation_status: :ready, - evaluation_digest: ^digest - } - }, - section_infos: %{"s1" => %{evaluating_cell_id: nil, evaluation_queue: []}} - }, []} = Data.apply_operation(data, operation) - end - test "marks next queued cell in this section as evaluating if there is one" do data = data_after_operations!([ @@ -1502,25 +1498,6 @@ defmodule Livebook.Session.DataTest do }, _actions} = Data.apply_operation(data, operation) end - test "updates cell digest based on the new content" do - data = - data_after_operations!([ - {:client_join, self(), User.new()}, - {:insert_section, self(), 0, "s1"}, - {:insert_cell, self(), "s1", 0, :elixir, "c1"} - ]) - - %{cell_infos: %{"c1" => %{digest: digest}}} = data - - delta = Delta.new() |> Delta.insert("cats") - operation = {:apply_cell_delta, self(), "c1", delta, 1} - - assert {:ok, %{cell_infos: %{"c1" => %{digest: new_digest}}}, _actions} = - Data.apply_operation(data, operation) - - assert digest != new_digest - end - test "transforms the delta if the revision is not the most recent" do client1_pid = IEx.Helpers.pid(0, 0, 0) client2_pid = IEx.Helpers.pid(0, 0, 1)