From ede80fc074480a1320c769cc97f3e9200bf0838f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jonatan=20K=C5=82osko?= Date: Wed, 5 Jul 2023 20:01:12 +0200 Subject: [PATCH] Implement file management (#2022) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: José Valim --- assets/css/js_interop.css | 10 + assets/js/hooks/session.js | 10 + lib/livebook/file_system/s3.ex | 12 +- lib/livebook/live_markdown/export.ex | 40 ++- lib/livebook/live_markdown/import.ex | 98 +++++- lib/livebook/migration.ex | 41 +++ lib/livebook/notebook.ex | 29 ++ lib/livebook/notebook/learn.ex | 24 +- .../distributed_portals_with_elixir.livemd | 6 +- .../learn/{images => files}/portal-drop.jpeg | Bin .../learn/{images => files}/portal-list.jpeg | Bin lib/livebook/session.ex | 303 +++++++++++++----- lib/livebook/session/data.ex | 44 +++ lib/livebook/settings.ex | 10 +- lib/livebook/utils.ex | 14 + lib/livebook_web/components/confirm.ex | 45 ++- .../components/form_components.ex | 7 +- .../controllers/session_controller.ex | 29 +- lib/livebook_web/live/env_var_component.ex | 2 +- .../live/file_select_component.ex | 2 + .../live/home_live/session_list_component.ex | 4 +- .../live/hub/edit/personal_component.ex | 4 +- lib/livebook_web/live/learn_live.ex | 4 +- lib/livebook_web/live/open_live.ex | 2 +- .../live/open_live/upload_component.ex | 6 +- .../live/open_live/url_component.ex | 3 +- lib/livebook_web/live/session_helpers.ex | 8 +- lib/livebook_web/live/session_live.ex | 179 ++++++++--- .../add_file_entry_file_component.ex | 155 +++++++++ .../add_file_entry_unlisted_component.ex | 109 +++++++ .../add_file_entry_url_component.ex | 125 ++++++++ .../session_live/cell_upload_component.ex | 99 ------ .../session_live/delete_section_component.ex | 50 --- .../live/session_live/files_list_component.ex | 171 ++++++++++ .../session_live/insert_buttons_component.ex | 2 +- .../session_live/insert_image_component.ex | 151 +++++++++ .../session_live/persistence_component.ex | 2 +- lib/livebook_web/live/settings_live.ex | 2 +- lib/livebook_web/router.ex | 7 +- test/livebook/live_markdown/export_test.exs | 146 +++++---- test/livebook/live_markdown/import_test.exs | 59 ++++ test/livebook/session/data_test.exs | 55 ++++ test/livebook/session_test.exs | 197 ++++++++++-- .../controllers/session_controller_test.exs | 66 +++- test/livebook_web/live/open_live_test.exs | 42 ++- test/livebook_web/live/session_live_test.exs | 277 ++++++++++++++++ test/support/test_helpers.ex | 9 +- 47 files changed, 2211 insertions(+), 449 deletions(-) rename lib/livebook/notebook/learn/{images => files}/portal-drop.jpeg (100%) rename lib/livebook/notebook/learn/{images => files}/portal-list.jpeg (100%) create mode 100644 lib/livebook_web/live/session_live/add_file_entry_file_component.ex create mode 100644 lib/livebook_web/live/session_live/add_file_entry_unlisted_component.ex create mode 100644 lib/livebook_web/live/session_live/add_file_entry_url_component.ex delete mode 100644 lib/livebook_web/live/session_live/cell_upload_component.ex delete mode 100644 lib/livebook_web/live/session_live/delete_section_component.ex create mode 100644 lib/livebook_web/live/session_live/files_list_component.ex create mode 100644 lib/livebook_web/live/session_live/insert_image_component.ex diff --git a/assets/css/js_interop.css b/assets/css/js_interop.css index e57fa885638..a056fb7dbfc 100644 --- a/assets/css/js_interop.css +++ b/assets/css/js_interop.css @@ -224,6 +224,11 @@ solely client-side operations. @apply hidden; } +[data-el-session]:not([data-js-side-panel-content="files-list"]) + [data-el-files-list] { + @apply hidden; +} + [data-el-session]:not([data-js-side-panel-content="runtime-info"]) [data-el-runtime-info] { @apply hidden; @@ -249,6 +254,11 @@ solely client-side operations. @apply text-gray-50 bg-gray-700; } +[data-el-session][data-js-side-panel-content="files-list"] + [data-el-files-list-toggle] { + @apply text-gray-50 bg-gray-700; +} + [data-el-session][data-js-side-panel-content="runtime-info"] [data-el-runtime-info-toggle] { @apply text-gray-50 bg-gray-700; diff --git a/assets/js/hooks/session.js b/assets/js/hooks/session.js index 19ccb4deb08..74769d4f17d 100644 --- a/assets/js/hooks/session.js +++ b/assets/js/hooks/session.js @@ -121,6 +121,10 @@ const Session = { this.toggleAppInfo() ); + this.getElement("files-list-toggle").addEventListener("click", (event) => + this.toggleFilesList() + ); + this.getElement("notebook").addEventListener("scroll", (event) => this.updateSectionListHighlight() ); @@ -368,6 +372,8 @@ const Session = { this.toggleAppInfo(); } else if (keyBuffer.tryMatch(["s", "u"])) { this.toggleClientsList(); + } else if (keyBuffer.tryMatch(["s", "f"])) { + this.toggleFilesList(); } else if (keyBuffer.tryMatch(["s", "r"])) { this.toggleRuntimeInfo(); } else if (keyBuffer.tryMatch(["s", "b"])) { @@ -719,6 +725,10 @@ const Session = { this.toggleSidePanelContent("app-info"); }, + toggleFilesList() { + this.toggleSidePanelContent("files-list"); + }, + toggleRuntimeInfo() { this.toggleSidePanelContent("runtime-info"); }, diff --git a/lib/livebook/file_system/s3.ex b/lib/livebook/file_system/s3.ex index ae996bcae86..41a226402bf 100644 --- a/lib/livebook/file_system/s3.ex +++ b/lib/livebook/file_system/s3.ex @@ -30,8 +30,11 @@ defmodule Livebook.FileSystem.S3 do bucket_url = String.trim_trailing(bucket_url, "/") region = opts[:region] || region_from_uri(bucket_url) + hash = :crypto.hash(:sha256, bucket_url) |> Base.url_encode64(padding: false) + id = "s3-#{hash}" + %__MODULE__{ - id: Livebook.Utils.random_short_id(), + id: id, bucket_url: bucket_url, region: region, access_key_id: access_key_id, @@ -52,23 +55,22 @@ defmodule Livebook.FileSystem.S3 do def from_config(config) do case config do %{ - id: id, bucket_url: bucket_url, access_key_id: access_key_id, secret_access_key: secret_access_key } -> file_system = new(bucket_url, access_key_id, secret_access_key, region: config[:region]) - {:ok, %{file_system | id: id}} + {:ok, file_system} _config -> {:error, - "S3 file system config is expected to have keys: :id, :bucket_url, :access_key_id and :secret_access_key, but got #{inspect(config)}"} + "S3 file system config is expected to have keys: :bucket_url, :access_key_id and :secret_access_key, but got #{inspect(config)}"} end end @spec to_config(t()) :: map() def to_config(%__MODULE__{} = s3) do - Map.take(s3, [:id, :bucket_url, :region, :access_key_id, :secret_access_key]) + Map.take(s3, [:bucket_url, :region, :access_key_id, :secret_access_key]) end end diff --git a/lib/livebook/live_markdown/export.ex b/lib/livebook/live_markdown/export.ex index beec0b5c256..f4410335172 100644 --- a/lib/livebook/live_markdown/export.ex +++ b/lib/livebook/live_markdown/export.ex @@ -82,11 +82,16 @@ defmodule Livebook.LiveMarkdown.Export do app_settings_metadata = app_settings_metadata(notebook.app_settings) - if app_settings_metadata == %{} do - metadata - else - Map.put(metadata, :app_settings, app_settings_metadata) - end + file_entry_metadatas = + notebook.file_entries + |> Enum.sort_by(& &1.name) + |> Enum.map(&file_entry_metadata/1) + + put_unless_default( + metadata, + %{app_settings: app_settings_metadata, file_entries: file_entry_metadatas}, + %{app_settings: %{}, file_entries: []} + ) end defp app_settings_metadata(app_settings) do @@ -108,6 +113,18 @@ defmodule Livebook.LiveMarkdown.Export do ) end + defp file_entry_metadata(%{type: :attachment, name: name}) do + %{type: "attachment", name: name} + end + + defp file_entry_metadata(%{type: :file, name: name, file: file}) do + %{type: "file", name: name, file: %{file_system_id: file.file_system.id, path: file.path}} + end + + defp file_entry_metadata(%{type: :url, name: name, url: url}) do + %{type: "url", name: name, url: url} + end + defp render_section(section, notebook, ctx) do name = ["## ", section.name] @@ -362,13 +379,16 @@ defmodule Livebook.LiveMarkdown.Export do put_unless_default(%{}, Map.take(notebook, keys), Map.take(Notebook.new(), keys)) end - defp ensure_order(map) do + defp ensure_order(%{} = map) do map |> Enum.sort() - |> Enum.map(fn - {key, %{} = value} -> {key, ensure_order(value)} - pair -> pair - end) + |> Enum.map(fn {key, value} -> {key, ensure_order(value)} end) |> Jason.OrderedObject.new() end + + defp ensure_order(list) when is_list(list) do + Enum.map(list, &ensure_order/1) + end + + defp ensure_order(term), do: term end diff --git a/lib/livebook/live_markdown/import.ex b/lib/livebook/live_markdown/import.ex index f688432f4bd..725cf7c533e 100644 --- a/lib/livebook/live_markdown/import.ex +++ b/lib/livebook/live_markdown/import.ex @@ -381,35 +381,58 @@ defmodule Livebook.LiveMarkdown.Import do defp notebook_metadata_to_attrs(metadata) do Enum.reduce(metadata, {%{}, Livebook.Hubs.Personal.id(), []}, fn - {"persist_outputs", persist_outputs}, {attrs, id, messages} -> - {Map.put(attrs, :persist_outputs, persist_outputs), id, messages} + {"persist_outputs", persist_outputs}, {attrs, stamp_hub_id, messages} -> + {Map.put(attrs, :persist_outputs, persist_outputs), stamp_hub_id, messages} - {"autosave_interval_s", autosave_interval_s}, {attrs, id, messages} -> - {Map.put(attrs, :autosave_interval_s, autosave_interval_s), id, messages} + {"autosave_interval_s", autosave_interval_s}, {attrs, stamp_hub_id, messages} -> + {Map.put(attrs, :autosave_interval_s, autosave_interval_s), stamp_hub_id, messages} - {"default_language", default_language}, {attrs, id, messages} + {"default_language", default_language}, {attrs, stamp_hub_id, messages} when default_language in ["elixir", "erlang"] -> default_language = String.to_atom(default_language) - {Map.put(attrs, :default_language, default_language), id, messages} + {Map.put(attrs, :default_language, default_language), stamp_hub_id, messages} - {"hub_id", hub_id}, {attrs, id, messages} -> + {"hub_id", hub_id}, {attrs, stamp_hub_id, messages} -> cond do Hubs.hub_exists?(hub_id) -> {Map.put(attrs, :hub_id, hub_id), hub_id, messages} Hubs.get_offline_hub(hub_id) -> {attrs, hub_id, messages} - true -> {attrs, id, messages ++ ["ignoring notebook Hub with unknown id"]} + true -> {attrs, stamp_hub_id, messages ++ ["ignoring notebook Hub with unknown id"]} end - {"app_settings", app_settings_metadata}, {attrs, id, messages} -> + {"app_settings", app_settings_metadata}, {attrs, stamp_hub_id, messages} -> app_settings = Map.merge( Notebook.AppSettings.new(), app_settings_metadata_to_attrs(app_settings_metadata) ) - {Map.put(attrs, :app_settings, app_settings), id, messages} - - _entry, {attrs, id, messages} -> - {attrs, id, messages} + {Map.put(attrs, :app_settings, app_settings), stamp_hub_id, messages} + + {"file_entries", file_entry_metadatas}, {attrs, stamp_hub_id, messages} + when is_list(file_entry_metadatas) -> + file_system_by_id = + if Enum.any?(file_entry_metadatas, &(&1["type"] == "file")) do + for file_system <- Livebook.Settings.file_systems(), + do: {file_system.id, file_system}, + into: %{} + else + %{} + end + + {file_entries, file_entry_messages} = + for file_entry_metadata <- file_entry_metadatas, reduce: {[], []} do + {file_entries, warnings} -> + case file_entry_metadata_to_attrs(file_entry_metadata, file_system_by_id) do + {:ok, file_entry} -> {[file_entry | file_entries], warnings} + {:error, message} -> {file_entries, [message | warnings]} + end + end + + {Map.put(attrs, :file_entries, file_entries), stamp_hub_id, + messages ++ file_entry_messages} + + _entry, {attrs, stamp_hub_id, messages} -> + {attrs, stamp_hub_id, messages} end) end @@ -444,6 +467,33 @@ defmodule Livebook.LiveMarkdown.Import do end) end + defp file_entry_metadata_to_attrs(%{"type" => "attachment", "name" => name}, _file_system_by_id) do + {:ok, %{type: :attachment, name: name}} + end + + defp file_entry_metadata_to_attrs( + %{ + "type" => "file", + "name" => name, + "file" => %{"file_system_id" => file_system_id, "path" => path} + }, + file_system_by_id + ) do + if file_system = file_system_by_id[file_system_id] do + file = Livebook.FileSystem.File.new(file_system, path) + {:ok, %{type: :file, name: name, file: file}} + else + {:error, "skipping file #{name}, since it points to an unknown file system"} + end + end + + defp file_entry_metadata_to_attrs( + %{"type" => "url", "name" => name, "url" => url}, + _file_system_by_id + ) do + {:ok, %{type: :url, name: name, url: url}} + end + defp section_metadata_to_attrs(metadata) do Enum.reduce(metadata, %{}, fn {"branch_parent_index", parent_idx}, attrs -> @@ -518,7 +568,27 @@ defmodule Livebook.LiveMarkdown.Import do end end) - {%{notebook | sections: sections}, Enum.reverse(warnings)} + notebook = %{notebook | sections: sections} + + legacy_images? = + notebook + |> Notebook.cells_with_section() + |> Enum.any?(fn {cell, _section} -> + # A heuristic to detect legacy image source + is_struct(cell, Notebook.Cell.Markdown) and String.contains?(cell.source, "](images/") + end) + + image_warnings = + if legacy_images? do + [ + "found Markdown images pointing to the images/ directory." <> + " Using this directory has been deprecated, please use notebook files instead" + ] + else + [] + end + + {notebook, Enum.reverse(warnings) ++ image_warnings} end defp take_stamp_data([{:stamp, data} | elements]), do: {data, elements} diff --git a/lib/livebook/migration.ex b/lib/livebook/migration.ex index 460560cb81d..b4576f2f678 100644 --- a/lib/livebook/migration.ex +++ b/lib/livebook/migration.ex @@ -10,6 +10,7 @@ defmodule Livebook.Migration do insert_personal_hub() move_app_secrets_to_personal_hub() add_personal_hub_secret_key() + update_file_systems_to_deterministic_ids() end defp delete_local_host_hub() do @@ -46,4 +47,44 @@ defmodule Livebook.Migration do Livebook.Storage.insert(:hubs, Livebook.Hubs.Personal.id(), secret_key: secret_key) end end + + defp update_file_systems_to_deterministic_ids() do + # We changed S3 file system ids, such that they are deterministic + # for the same bucket, rather than random. We take this opportunity + # to rename the scope from :filesystem to :file_systems, which + # conveniently allows for easy check if there's anything to migrate. + # This migration can be removed in the future (at the cost of discarding + # very old file systems (which can be re-added). + # TODO: remove on Livebook v0.12 + + case Livebook.Storage.all(:filesystem) do + [] -> + :ok + + configs -> + id_mapping = + for config <- configs, into: %{} do + old_id = config.id + # At this point S3 is the only file system we store + {:ok, file_system} = Livebook.FileSystem.S3.from_config(config) + Livebook.Settings.save_file_system(file_system) + Livebook.Storage.delete(:filesystem, old_id) + {old_id, file_system.id} + end + + # Remap default file system id + + default_file_system_id = Livebook.Settings.default_file_system_id() + + if new_id = id_mapping[default_file_system_id] do + Livebook.Settings.set_default_file_system(new_id) + end + + # Livebook.NotebookManager is started before the migration runs, + # and it discards S3 files, since it can't find the file system. + # However, in this case it's fine; for recent notebooks it does + # not matter really and there are likely not many starred S3 + # notebooks at this point (and the user can easily star again) + end + end end diff --git a/lib/livebook/notebook.ex b/lib/livebook/notebook.ex index 3e3fcb695b0..88282c7b9ea 100644 --- a/lib/livebook/notebook.ex +++ b/lib/livebook/notebook.ex @@ -26,6 +26,7 @@ defmodule Livebook.Notebook do :app_settings, :hub_id, :hub_secret_names, + :file_entries, :teams_enabled ] @@ -46,9 +47,26 @@ defmodule Livebook.Notebook do app_settings: AppSettings.t(), hub_id: String.t(), hub_secret_names: list(String.t()), + file_entries: list(file_entry()), teams_enabled: boolean() } + @type file_entry :: + %{ + name: String.t(), + type: :attachment + } + | %{ + name: String.t(), + type: :file, + file: Livebook.FileSystem.File.t() + } + | %{ + name: String.t(), + type: :url, + url: String.t() + } + @version "1.0" @doc """ @@ -69,6 +87,7 @@ defmodule Livebook.Notebook do app_settings: AppSettings.new(), hub_id: Livebook.Hubs.Personal.id(), hub_secret_names: [], + file_entries: [], teams_enabled: false } |> put_setup_cell(Cell.new(:code)) @@ -815,4 +834,14 @@ defmodule Livebook.Notebook do defp do_prune_outputs([_output | outputs], acc) do do_prune_outputs(outputs, acc) end + + @doc """ + Validates a change is a valid file entry name. + """ + @spec validate_file_entry_name(Ecto.Changeset.t(), atom()) :: Ecto.Changeset.t() + def validate_file_entry_name(changeset, field) do + Ecto.Changeset.validate_format(changeset, field, ~r/^[\w-.]+$/, + message: "should contain only alphanumeric characters, dash, underscore and dot" + ) + end end diff --git a/lib/livebook/notebook/learn.ex b/lib/livebook/notebook/learn.ex index 3c1ed16862f..b84a1f0f4e5 100644 --- a/lib/livebook/notebook/learn.ex +++ b/lib/livebook/notebook/learn.ex @@ -16,11 +16,11 @@ defmodule Livebook.Notebook.Learn do slug: String.t(), livemd: String.t(), title: String.t(), - images: images(), + files: files(), details: details() | nil } - @type images :: %{String.t() => binary()} + @type files :: %{String.t() => binary()} @type details :: %{ description: String.t(), @@ -36,7 +36,7 @@ defmodule Livebook.Notebook.Learn do @type image_source :: {:static, filename :: String.t()} | {:url, String.t()} - images_dir = Path.expand("learn/images", __DIR__) + files_dir = Path.expand("learn/files", __DIR__) welcome_config = %{ path: Path.join(__DIR__, "learn/intro_to_livebook.livemd"), @@ -49,9 +49,9 @@ defmodule Livebook.Notebook.Learn do other_configs = [ %{ path: Path.join(__DIR__, "learn/distributed_portals_with_elixir.livemd"), - image_paths: [ - Path.join(images_dir, "portal-drop.jpeg"), - Path.join(images_dir, "portal-list.jpeg") + file_paths: [ + Path.join(files_dir, "portal-drop.jpeg"), + Path.join(files_dir, "portal-list.jpeg") ], details: %{ description: @@ -127,9 +127,9 @@ defmodule Livebook.Notebook.Learn do raise "found warnings while importing #{path}:\n\n" <> Enum.join(items, "\n") end - images = + files = config - |> Map.get(:image_paths, []) + |> Map.get(:file_paths, []) |> Map.new(fn image_path -> image_name = Path.basename(image_path) content = File.read!(image_path) @@ -144,7 +144,7 @@ defmodule Livebook.Notebook.Learn do slug: slug, livemd: markdown, title: notebook.name, - images: images, + files: files, details: if config_details = config[:details] do description = @@ -186,9 +186,9 @@ defmodule Livebook.Notebook.Learn do @doc """ Finds learn notebook by slug and returns the parsed data structure. - Returns the notebook along with the images it uses as preloaded binaries. + Returns the notebook along with the files it uses as preloaded binaries. """ - @spec notebook_by_slug!(String.t()) :: {Livebook.Notebook.t(), images()} + @spec notebook_by_slug!(String.t()) :: {Livebook.Notebook.t(), files()} def notebook_by_slug!(slug) do notebook_infos() |> Enum.find(&(&1.slug == slug)) @@ -198,7 +198,7 @@ defmodule Livebook.Notebook.Learn do notebook_info -> {notebook, []} = Livebook.LiveMarkdown.notebook_from_livemd(notebook_info.livemd) - {notebook, notebook_info.images} + {notebook, notebook_info.files} end end diff --git a/lib/livebook/notebook/learn/distributed_portals_with_elixir.livemd b/lib/livebook/notebook/learn/distributed_portals_with_elixir.livemd index 1a8e35e1f8a..d7e3c2b80d0 100644 --- a/lib/livebook/notebook/learn/distributed_portals_with_elixir.livemd +++ b/lib/livebook/notebook/learn/distributed_portals_with_elixir.livemd @@ -1,3 +1,5 @@ + + # Distributed portals with Elixir ## Introduction @@ -25,13 +27,13 @@ In order to teleport, the player uses the Portal gun to shoot doors onto flat planes, like a floor or a wall. Entering one of those doors teleports you to the other: -![](images/portal-drop.jpeg) +![](files/portal-drop.jpeg) Our version of the Portal game will use Elixir to shoot doors of different colors and transfer data between them! We will even learn how we can distribute doors across different machines in our network: -![](images/portal-list.jpeg) +![](files/portal-list.jpeg) Here is what we will learn: diff --git a/lib/livebook/notebook/learn/images/portal-drop.jpeg b/lib/livebook/notebook/learn/files/portal-drop.jpeg similarity index 100% rename from lib/livebook/notebook/learn/images/portal-drop.jpeg rename to lib/livebook/notebook/learn/files/portal-drop.jpeg diff --git a/lib/livebook/notebook/learn/images/portal-list.jpeg b/lib/livebook/notebook/learn/files/portal-list.jpeg similarity index 100% rename from lib/livebook/notebook/learn/images/portal-list.jpeg rename to lib/livebook/notebook/learn/files/portal-list.jpeg diff --git a/lib/livebook/session.ex b/lib/livebook/session.ex index d45e09c64af..6d070079979 100644 --- a/lib/livebook/session.ex +++ b/lib/livebook/session.ex @@ -53,7 +53,7 @@ defmodule Livebook.Session do :notebook_name, :file, :mode, - :images_dir, + :files_dir, :created_at, :memory_usage ] @@ -80,7 +80,7 @@ defmodule Livebook.Session do notebook_name: String.t(), file: FileSystem.File.t() | nil, mode: Data.session_mode(), - images_dir: FileSystem.File.t(), + files_dir: FileSystem.File.t(), created_at: DateTime.t(), memory_usage: memory_usage() } @@ -93,7 +93,7 @@ defmodule Livebook.Session do runtime_monitor_ref: reference() | nil, autosave_timer_ref: reference() | nil, autosave_path: String.t(), - save_task_pid: pid() | nil, + save_task_ref: reference() | nil, saved_default_file: FileSystem.File.t() | nil, memory_usage: memory_usage(), worker_pid: pid(), @@ -131,10 +131,16 @@ defmodule Livebook.Session do * `:file` - the file to which the notebook should be saved - * `:copy_images_from` - a directory file to copy notebook images from + * `:files_source` - a location to fetch notebook files from, either of: - * `:images` - a map from image name to its binary content, an alternative - to `:copy_images_from` when the images are in memory + * `{:dir, dir}` - a directory file + + * `{:url, url} - a base url to the files directory (with `/` suffix) + + * `{:inline, contents_map}` - a map with file names pointing to their + binary contents + + Defaults to `nil`, in which case no files are copied * `:autosave_path` - a local directory to save notebooks without a file into. Defaults to `Livebook.Settings.autosave_path/0` @@ -202,6 +208,14 @@ defmodule Livebook.Session do GenServer.call(pid, :get_notebook, @timeout) end + @doc """ + Returns the current notebook file entries. + """ + @spec get_notebook_file_entries(pid()) :: list(Notebook.file_entry()) + def get_notebook_file_entries(pid) do + GenServer.call(pid, :get_notebook_file_entries, @timeout) + end + @doc """ Subscribes to session messages. @@ -558,13 +572,32 @@ defmodule Livebook.Session do end @doc """ - Sends a hub selection to the server. + Sends a hub selection request to the server. """ @spec set_notebook_hub(pid(), String.t()) :: :ok def set_notebook_hub(pid, id) do GenServer.cast(pid, {:set_notebook_hub, self(), id}) end + @doc """ + Sends a file entries addition request to the server. + + Note that if file entries with any of the given names already exist + they are replaced. + """ + @spec add_file_entries(pid(), list(Notebook.file_entry())) :: :ok + def add_file_entries(pid, file_entries) do + GenServer.cast(pid, {:add_file_entries, self(), file_entries}) + end + + @doc """ + Sends a file entry deletion request to the server. + """ + @spec delete_file_entry(pid(), String.t()) :: :ok + def delete_file_entry(pid, name) do + GenServer.cast(pid, {:delete_file_entry, self(), name}) + end + @doc """ Sends save request to the server. @@ -699,13 +732,8 @@ defmodule Livebook.Session do with {:ok, state} <- init_state(id, worker_pid, opts), :ok <- - if(copy_images_from = opts[:copy_images_from], - do: copy_images(state, copy_images_from), - else: :ok - ), - :ok <- - if(images = opts[:images], - do: dump_images(state, images), + if(files_source = opts[:files_source], + do: initialize_files_from(state, files_source), else: :ok ) do state = schedule_autosave(state) @@ -726,6 +754,7 @@ defmodule Livebook.Session do end else {:error, error} -> + cleanup_tmp_dir(id) {:stop, error} end end @@ -740,7 +769,7 @@ defmodule Livebook.Session do runtime_monitor_ref: nil, autosave_timer_ref: nil, autosave_path: opts[:autosave_path], - save_task_pid: nil, + save_task_ref: nil, saved_default_file: nil, memory_usage: %{runtime: nil, system: Livebook.SystemResources.memory()}, worker_pid: worker_pid, @@ -888,6 +917,10 @@ defmodule Livebook.Session do {:reply, state.data.notebook, state} end + def handle_call(:get_notebook_file_entries, _from, state) do + {:reply, state.data.notebook.file_entries, state} + end + def handle_call(:save_sync, _from, state) do {:reply, :ok, maybe_save_notebook_sync(state)} end @@ -1260,6 +1293,18 @@ defmodule Livebook.Session do {:noreply, handle_operation(state, operation)} end + def handle_cast({:add_file_entries, client_pid, file_entries}, state) do + client_id = client_id(state, client_pid) + operation = {:add_file_entries, client_id, file_entries} + {:noreply, handle_operation(state, operation)} + end + + def handle_cast({:delete_file_entry, client_pid, name}, state) do + client_id = client_id(state, client_pid) + operation = {:delete_file_entry, client_id, name} + {:noreply, handle_operation(state, operation)} + end + @impl true def handle_info({:DOWN, ref, :process, _, reason}, state) when ref == state.runtime_monitor_ref do @@ -1275,6 +1320,10 @@ defmodule Livebook.Session do )} end + def handle_info({:DOWN, ref, :process, _, _}, state) when ref == state.save_task_ref do + {:noreply, %{state | save_task_ref: nil}} + end + def handle_info({:DOWN, ref, :process, _, _}, state) when ref == state.deployed_app_monitor_ref do {:noreply, @@ -1423,11 +1472,9 @@ defmodule Livebook.Session do {:noreply, handle_operation(state, operation)} end - def handle_info( - {:save_finished, pid, result, warnings, file, default?}, - %{save_task_pid: pid} = state - ) do - state = %{state | save_task_pid: nil} + def handle_info({ref, {:save_finished, result, warnings, file, default?}}, state) + when ref == state.save_task_ref do + state = %{state | save_task_ref: nil} {:noreply, handle_save_finished(state, result, warnings, file, default?)} end @@ -1574,29 +1621,35 @@ defmodule Livebook.Session do notebook_name: state.data.notebook.name, file: state.data.file, mode: state.data.mode, - images_dir: images_dir_from_state(state), + files_dir: files_dir_from_state(state), created_at: state.created_at, memory_usage: state.memory_usage } end - defp images_dir_from_state(%{data: %{file: nil}, session_id: id}) do - tmp_dir = session_tmp_dir(id) - FileSystem.File.resolve(tmp_dir, "images/") + defp files_dir_from_state(state) do + state + |> notebook_dir() + |> FileSystem.File.resolve("files/") end - defp images_dir_from_state(%{data: %{file: file}}) do - images_dir_for_notebook(file) + defp notebook_dir(state) do + if file = state.data.file || default_notebook_file(state) do + FileSystem.File.containing_dir(file) + else + tmp_dir = session_tmp_dir(state.session_id) + FileSystem.File.resolve(tmp_dir, "notebook/") + end end @doc """ - Returns images directory corresponding to the given notebook file. + Returns files directory corresponding to the given notebook file. """ - @spec images_dir_for_notebook(FileSystem.File.t()) :: FileSystem.File.t() - def images_dir_for_notebook(file) do + @spec files_dir_for_notebook(FileSystem.File.t()) :: FileSystem.File.t() + def files_dir_for_notebook(file) do file |> FileSystem.File.containing_dir() - |> FileSystem.File.resolve("images/") + |> FileSystem.File.resolve("files/") end defp session_tmp_dir(session_id) do @@ -1651,35 +1704,106 @@ defmodule Livebook.Session do Path.join(tmp_dir, "livebook") end - defp copy_images(state, source) do - images_dir = images_dir_from_state(state) + defp initialize_files_from(state, {:inline, contents_map}) do + write_attachment_file_entries(state, fn destination_file, file_entry -> + case Map.fetch(contents_map, file_entry.name) do + {:ok, content} -> FileSystem.File.write(destination_file, content) + :error -> :ok + end + end) + end + + defp initialize_files_from(state, {:url, url}) do + write_attachment_file_entries(state, fn destination_file, file_entry -> + source_url = + url + |> Livebook.Utils.expand_url(file_entry.name) + |> Livebook.Notebook.ContentLoader.rewrite_url() + case fetch_content(source_url) do + {:ok, content} -> FileSystem.File.write(destination_file, content) + {:error, _message, 404} -> :ok + {:error, message, _status} -> {:error, message} + end + end) + end + + defp initialize_files_from(state, {:dir, dir}) do + copy_files(state, dir) + end + + defp copy_files(state, source) do with {:ok, source_exists?} <- FileSystem.File.exists?(source) do if source_exists? do - FileSystem.File.copy(source, images_dir) + write_attachment_file_entries(state, fn destination_file, file_entry -> + source_file = FileSystem.File.resolve(source, file_entry.name) + + case FileSystem.File.copy(source_file, destination_file) do + :ok -> + :ok + + {:error, message} -> + # If the files does not exist, we treat it as copy success + case FileSystem.File.exists?(source_file) do + {:ok, false} -> :ok + _ -> {:error, file_entry.name, message} + end + end + end) else :ok end end end - defp move_images(state, source) do - images_dir = images_dir_from_state(state) + defp write_attachment_file_entries(state, write_fun) do + files_dir = files_dir_from_state(state) + + state.data.notebook.file_entries + |> Enum.filter(&(&1.type == :attachment)) + |> Task.async_stream( + fn file_entry -> + destination_file = FileSystem.File.resolve(files_dir, file_entry.name) + + case write_fun.(destination_file, file_entry) do + :ok -> :ok + {:error, message} -> {:error, file_entry.name, message} + end + end, + max_concurrency: 20 + ) + |> Enum.reject(&(&1 == {:ok, :ok})) + |> case do + [] -> + :ok + + errors -> + enumeration = + Enum.map_join(errors, ", ", fn {:ok, {:error, name, message}} -> + "#{name} (#{message})" + end) + + {:error, "failed to copy files: " <> enumeration} + end + end + + defp move_files(state, source) do + files_dir = files_dir_from_state(state) with {:ok, source_exists?} <- FileSystem.File.exists?(source) do if source_exists? do - with {:ok, destination_exists?} <- FileSystem.File.exists?(images_dir) do + with {:ok, destination_exists?} <- FileSystem.File.exists?(files_dir) do if destination_exists? do # If the directory exists, we use copy to place - # the images there - with :ok <- FileSystem.File.copy(source, images_dir) do + # the files there + with :ok <- FileSystem.File.copy(source, files_dir) do FileSystem.File.remove(source) end else # If the directory doesn't exist, we can just change # the directory name, which is more efficient if # available in the given file system - FileSystem.File.rename(source, images_dir) + FileSystem.File.rename(source, files_dir) end end else @@ -1688,17 +1812,6 @@ defmodule Livebook.Session do end end - defp dump_images(state, images) do - images_dir = images_dir_from_state(state) - - Enum.reduce(images, :ok, fn {filename, content}, result -> - with :ok <- result do - file = FileSystem.File.resolve(images_dir, filename) - FileSystem.File.write(file, content) - end - end) - end - defp own_runtime(runtime, state) do runtime_monitor_ref = Runtime.take_ownership(runtime, runtime_broadcast_to: state.worker_pid) %{state | runtime_monitor_ref: runtime_monitor_ref} @@ -1778,19 +1891,16 @@ defmodule Livebook.Session do end defp after_operation(state, prev_state, {:set_file, _client_id, _file}) do - prev_images_dir = images_dir_from_state(prev_state) + prev_files_dir = files_dir_from_state(prev_state) if prev_state.data.file do - copy_images(state, prev_images_dir) + copy_files(state, prev_files_dir) else - move_images(state, prev_images_dir) + move_files(state, prev_files_dir) end |> case do - :ok -> - :ok - - {:error, message} -> - broadcast_error(state.session_id, "failed to copy images - #{message}") + :ok -> :ok + {:error, message} -> broadcast_error(state.session_id, message) end if file = state.data.file do @@ -2126,17 +2236,16 @@ defmodule Livebook.Session do {file, default?} = notebook_autosave_file(state) if file && should_save_notebook?(state) do - pid = self() notebook = state.data.notebook - {:ok, pid} = - Task.Supervisor.start_child(Livebook.TaskSupervisor, fn -> + %{ref: ref} = + Task.Supervisor.async_nolink(Livebook.TaskSupervisor, fn -> {content, warnings} = LiveMarkdown.notebook_to_livemd(notebook) result = FileSystem.File.write(file, content) - send(pid, {:save_finished, self(), result, warnings, file, default?}) + {:save_finished, result, warnings, file, default?} end) - %{state | save_task_pid: pid} + %{state | save_task_ref: ref} else state end @@ -2159,7 +2268,7 @@ defmodule Livebook.Session do defp maybe_save_notebook_sync(state), do: state defp should_save_notebook?(state) do - (state.data.dirty or state.data.persistence_warnings != []) and state.save_task_pid == nil + (state.data.dirty or state.data.persistence_warnings != []) and state.save_task_ref == nil end defp notebook_autosave_file(state) do @@ -2184,13 +2293,7 @@ defmodule Livebook.Session do # which are random already random_str = String.slice(state.session_id, -4..-1) - [date_str, time_str, _] = - state.created_at - |> DateTime.to_iso8601() - |> String.replace(["-", ":"], "_") - |> String.split(["T", "."]) - - "#{date_str}/#{time_str}_#{title_str}_#{random_str}.livemd" + Calendar.strftime(state.created_at, "%Y_%m_%d/%H_%M") <> "_#{random_str}/#{title_str}.livemd" end defp notebook_name_to_file_name(notebook_name) do @@ -2206,19 +2309,14 @@ defmodule Livebook.Session do end defp handle_save_finished(state, result, warnings, file, default?) do - state = - if default? do + case result do + :ok -> if state.saved_default_file && state.saved_default_file != file do FileSystem.File.remove(state.saved_default_file) end - %{state | saved_default_file: file} - else - state - end + state = %{state | saved_default_file: if(default?, do: file, else: nil)} - case result do - :ok -> handle_operation(state, {:notebook_saved, @client_id, warnings}) {:error, message} -> @@ -2399,4 +2497,51 @@ defmodule Livebook.Session do defp container_ref_for_section(%{parent_id: nil}), do: @main_container_ref defp container_ref_for_section(section), do: section.id + + @doc """ + Converts the given file entry to attachment one. + + The file is fetched into the notebook files directory. + """ + @spec to_attachment_file_entry(t(), Notebook.file_entry()) :: {:ok, Notebook.file_entry()} + def to_attachment_file_entry(session, file_entry) + + def to_attachment_file_entry(session, %{type: :file} = file_entry) do + destination = FileSystem.File.resolve(session.files_dir, file_entry.name) + + with :ok <- FileSystem.File.copy(file_entry.file, destination) do + {:ok, %{name: file_entry.name, type: :attachment}} + end + end + + def to_attachment_file_entry(session, %{type: :url} = file_entry) do + destination = FileSystem.File.resolve(session.files_dir, file_entry.name) + + case fetch_content(file_entry.url) do + {:ok, content} -> + with :ok <- FileSystem.File.write(destination, content) do + {:ok, %{name: file_entry.name, type: :attachment}} + end + + {:error, message, _status} -> + {:error, message} + end + end + + def to_attachment_file_entry(_session, %{type: :attachment} = file_entry) do + {:ok, file_entry} + end + + defp fetch_content(url) do + case Livebook.Utils.HTTP.request(:get, url) do + {:ok, 200, _headers, body} -> + {:ok, body} + + {:ok, status, _headers, _body} -> + {:error, "failed to download file from the given URL", status} + + _ -> + {:error, "failed to download file from the given URL", nil} + end + end end diff --git a/lib/livebook/session/data.ex b/lib/livebook/session/data.ex index 276d85cb2f5..e1e051100e9 100644 --- a/lib/livebook/session/data.ex +++ b/lib/livebook/session/data.ex @@ -216,6 +216,8 @@ defmodule Livebook.Session.Data do | {:unset_secret, client_id(), String.t()} | {:set_notebook_hub, client_id(), String.t()} | {:sync_hub_secrets, client_id()} + | {:add_file_entries, client_id(), list(Notebook.file_entry())} + | {:delete_file_entry, client_id(), String.t()} | {:set_app_settings, client_id(), AppSettings.t()} | {:set_deployed_app_slug, client_id(), String.t()} | {:app_deactivate, client_id()} @@ -896,6 +898,26 @@ defmodule Livebook.Session.Data do |> wrap_ok() end + def apply_operation(data, {:add_file_entries, _client_id, file_entries}) do + data + |> with_actions() + |> add_file_entries(file_entries) + |> set_dirty() + |> wrap_ok() + end + + def apply_operation(data, {:delete_file_entry, _client_id, name}) do + with {:ok, file_entry} <- fetch_file_entry(data.notebook, name) do + data + |> with_actions() + |> delete_file_entry(file_entry) + |> set_dirty() + |> wrap_ok() + else + _ -> :error + end + end + def apply_operation(data, {:set_app_settings, _client_id, settings}) do data |> with_actions() @@ -1659,6 +1681,22 @@ defmodule Livebook.Session.Data do set!(data_actions, notebook: %{data.notebook | hub_secret_names: hub_secret_names}) end + defp add_file_entries({data, _} = data_actions, file_entries) do + new_names = for entry <- file_entries, do: entry.name, into: MapSet.new() + + kept_file_entries = Enum.reject(data.notebook.file_entries, &(&1.name in new_names)) + + data_actions + |> set!(notebook: %{data.notebook | file_entries: file_entries ++ kept_file_entries}) + end + + defp delete_file_entry({data, _} = data_actions, file_entry) do + file_entries = data.notebook.file_entries -- [file_entry] + + data_actions + |> set!(notebook: %{data.notebook | file_entries: file_entries}) + end + defp set_section_name({data, _} = data_actions, section, name) do data_actions |> set!(notebook: Notebook.update_section(data.notebook, section.id, &%{&1 | name: name})) @@ -1927,6 +1965,12 @@ defmodule Livebook.Session.Data do end) end + defp fetch_file_entry(notebook, name) do + Enum.find_value(notebook.file_entries, :error, fn file_entry -> + file_entry.name == name && {:ok, file_entry} + end) + end + defp add_action({data, actions}, action) do {data, actions ++ [action]} end diff --git a/lib/livebook/settings.ex b/lib/livebook/settings.ex index 01b3b7d48fc..c45ae3fcb9c 100644 --- a/lib/livebook/settings.ex +++ b/lib/livebook/settings.ex @@ -12,7 +12,7 @@ defmodule Livebook.Settings do @typedoc """ An id that is used for file system's manipulation, either insertion or removal. """ - @type file_system_id :: Livebook.Utils.id() + @type file_system_id :: String.t() @doc """ Returns the current autosave path. @@ -55,7 +55,7 @@ defmodule Livebook.Settings do @spec file_systems() :: list(FileSystem.t()) def file_systems() do restored_file_systems = - Storage.all(:filesystem) + Storage.all(:file_systems) |> Enum.sort_by(&Map.get(&1, :order, System.os_time())) |> Enum.map(&storage_to_fs/1) @@ -73,7 +73,7 @@ defmodule Livebook.Settings do |> Map.to_list() attrs = [{:type, "s3"}, {:order, System.os_time()} | attributes] - :ok = Storage.insert(:filesystem, file_system.id, attrs) + :ok = Storage.insert(:file_systems, file_system.id, attrs) end @doc """ @@ -87,7 +87,7 @@ defmodule Livebook.Settings do Livebook.NotebookManager.remove_file_system(file_system_id) - Livebook.Storage.delete(:filesystem, file_system_id) + Livebook.Storage.delete(:file_systems, file_system_id) end defp storage_to_fs(%{type: "s3"} = config) do @@ -234,7 +234,7 @@ defmodule Livebook.Settings do """ @spec default_file_system() :: Filesystem.t() def default_file_system() do - case Livebook.Storage.fetch(:filesystem, default_file_system_id()) do + case Livebook.Storage.fetch(:file_systems, default_file_system_id()) do {:ok, file} -> storage_to_fs(file) :error -> Livebook.Config.local_file_system() end diff --git a/lib/livebook/utils.ex b/lib/livebook/utils.ex index 7e3a3cb9f0d..59d614c66a5 100644 --- a/lib/livebook/utils.ex +++ b/lib/livebook/utils.ex @@ -165,6 +165,20 @@ defmodule Livebook.Utils do uri.scheme != nil and uri.host not in [nil, ""] end + @doc """ + Validates a change is a valid URL. + """ + @spec validate_url(Ecto.Changeset.t(), atom()) :: Ecto.Changeset.t() + def validate_url(changeset, field) do + Ecto.Changeset.validate_change(changeset, field, fn ^field, url -> + if valid_url?(url) do + [] + else + [{:url, "must be a valid URL"}] + end + end) + end + @doc ~S""" Validates if the given string forms valid CLI flags. diff --git a/lib/livebook_web/components/confirm.ex b/lib/livebook_web/components/confirm.ex index e07015b8643..26675c2d1b3 100644 --- a/lib/livebook_web/components/confirm.ex +++ b/lib/livebook_web/components/confirm.ex @@ -2,6 +2,7 @@ defmodule LivebookWeb.Confirm do use Phoenix.Component import LivebookWeb.CoreComponents + import LivebookWeb.FormComponents import Phoenix.LiveView alias Phoenix.LiveView.JS @@ -32,6 +33,21 @@ defmodule LivebookWeb.Confirm do checkbox. Once checked by the user, the confirmation with this id is never shown again. Optional + * `:options` - a list of togglable options to present alongside the + confirmation message. Each option must be a map: + + ``` + %{ + name: String.t(), + label: String.t(), + default: boolean(), + disabled: boolean() + } + ``` + + The option values are passed to the `on_confirm` function as the + second argument + """ def confirm(socket, on_confirm, opts) do opts = @@ -42,7 +58,8 @@ defmodule LivebookWeb.Confirm do confirm_text: "Yes", confirm_icon: nil, danger: true, - opt_out_id: nil + opt_out_id: nil, + options: [] ) send(self(), {:confirm, on_confirm, opts}) @@ -77,6 +94,20 @@ defmodule LivebookWeb.Confirm do

<%= @description %>

+
+

+ Options +

+
+ <.switch_field + :for={option <- @options} + name={"options[#{option.name}]"} + label={option.label} + value={option.default} + disabled={option.disabled} + /> +
+