From 5edbc63c040ea1bb7c06a843654df54592c9b8f2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jonatan=20K=C5=82osko?= Date: Thu, 12 Aug 2021 22:50:37 +0200 Subject: [PATCH 1/8] Introduce file system abstraction and an S3 implementation --- notebook.livemd => .livemd | 0 assets/css/components.css | 13 +- config/runtime.exs | 9 +- lib/livebook/config.ex | 39 +- lib/livebook/content_loader.ex | 59 +- lib/livebook/file_system.ex | 133 ++++ lib/livebook/file_system/file.ex | 253 +++++++ lib/livebook/file_system/local.ex | 184 +++++ lib/livebook/file_system/s3.ex | 365 ++++++++++ lib/livebook/file_system/s3/signature.ex | 202 ++++++ lib/livebook/file_system/s3/xml.ex | 144 ++++ lib/livebook/file_system/utils.ex | 126 ++++ lib/livebook/live_markdown/export.ex | 21 +- lib/livebook/live_markdown/import.ex | 3 + lib/livebook/notebook.ex | 32 +- lib/livebook/session.ex | 300 +++++--- lib/livebook/session/data.ex | 23 +- lib/livebook/session/file_guard.ex | 34 +- lib/livebook/utils.ex | 20 + lib/livebook/utils/http.ex | 106 +++ lib/livebook_cli/server.ex | 8 +- .../controllers/session_controller.ex | 65 +- lib/livebook_web/helpers.ex | 50 +- lib/livebook_web/live/explore_live.ex | 17 +- .../live/file_select_component.ex | 563 +++++++++++++++ lib/livebook_web/live/home_live.ex | 155 +++-- .../live/home_live/import_url_component.ex | 2 +- lib/livebook_web/live/modal_component.ex | 7 +- lib/livebook_web/live/page_helpers.ex | 28 + .../live/path_select_component.ex | 437 ------------ lib/livebook_web/live/session_helpers.ex | 9 + lib/livebook_web/live/session_live.ex | 99 ++- .../live/session_live/cell_component.ex | 2 +- .../session_live/cell_upload_component.ex | 43 +- .../live/session_live/indicators_component.ex | 23 +- .../live/session_live/mix_standalone_live.ex | 58 +- .../session_live/persistence_component.ex | 179 ----- .../live/session_live/persistence_live.ex | 254 +++++++ .../live/session_live/runtime_component.ex | 18 +- lib/livebook_web/live/settings_live.ex | 102 +++ .../add_file_system_component.ex | 102 +++ .../settings_live/file_systems_component.ex | 45 ++ .../remove_file_system_component.ex | 33 + lib/livebook_web/router.ex | 5 + mix.exs | 2 +- test/livebook/content_loader_test.exs | 22 +- test/livebook/file_system/file_test.exs | 413 +++++++++++ test/livebook/file_system/local_test.exs | 487 +++++++++++++ test/livebook/file_system/s3_test.exs | 652 ++++++++++++++++++ test/livebook/live_markdown/export_test.exs | 18 + test/livebook/live_markdown/import_test.exs | 12 + test/livebook/session/data_test.exs | 8 +- test/livebook/session_test.exs | 113 +-- .../controllers/session_controller_test.exs | 7 +- .../live/file_select_component_test.exs | 40 ++ test/livebook_web/live/home_live_test.exs | 28 +- .../live/path_select_component_test.exs | 49 -- test/livebook_web/live/session_live_test.exs | 90 +-- test/support/test_helpers.ex | 21 + 59 files changed, 5127 insertions(+), 1205 deletions(-) rename notebook.livemd => .livemd (100%) create mode 100644 lib/livebook/file_system.ex create mode 100644 lib/livebook/file_system/file.ex create mode 100644 lib/livebook/file_system/local.ex create mode 100644 lib/livebook/file_system/s3.ex create mode 100644 lib/livebook/file_system/s3/signature.ex create mode 100644 lib/livebook/file_system/s3/xml.ex create mode 100644 lib/livebook/file_system/utils.ex create mode 100644 lib/livebook/utils/http.ex create mode 100644 lib/livebook_web/live/file_select_component.ex create mode 100644 lib/livebook_web/live/page_helpers.ex delete mode 100644 lib/livebook_web/live/path_select_component.ex delete mode 100644 lib/livebook_web/live/session_live/persistence_component.ex create mode 100644 lib/livebook_web/live/session_live/persistence_live.ex create mode 100644 lib/livebook_web/live/settings_live.ex create mode 100644 lib/livebook_web/live/settings_live/add_file_system_component.ex create mode 100644 lib/livebook_web/live/settings_live/file_systems_component.ex create mode 100644 lib/livebook_web/live/settings_live/remove_file_system_component.ex create mode 100644 test/livebook/file_system/file_test.exs create mode 100644 test/livebook/file_system/local_test.exs create mode 100644 test/livebook/file_system/s3_test.exs create mode 100644 test/livebook_web/live/file_select_component_test.exs delete mode 100644 test/livebook_web/live/path_select_component_test.exs create mode 100644 test/support/test_helpers.ex diff --git a/notebook.livemd b/.livemd similarity index 100% rename from notebook.livemd rename to .livemd diff --git a/assets/css/components.css b/assets/css/components.css index 93b3e242955..f3290a0c306 100644 --- a/assets/css/components.css +++ b/assets/css/components.css @@ -69,6 +69,12 @@ @apply rounded-full border-2; } + /* Links */ + + .link { + @apply font-medium underline text-gray-900 hover:no-underline; + } + /* Form fields */ .input { @@ -185,10 +191,15 @@ /* Toggleable menu */ .menu { - @apply absolute right-0 z-30 rounded-lg bg-white flex flex-col py-2; + @apply absolute right-0 z-30 rounded-lg bg-white flex flex-col py-2 mt-1; box-shadow: 0px 15px 99px rgba(13, 24, 41, 0.15); } + .menu.left { + right: auto; + @apply left-0; + } + .menu__item { @apply flex space-x-3 px-5 py-2 items-center hover:bg-gray-50 whitespace-nowrap; } diff --git a/config/runtime.exs b/config/runtime.exs index f97d7f231cc..cca81536a20 100644 --- a/config/runtime.exs +++ b/config/runtime.exs @@ -6,8 +6,6 @@ config :livebook, LivebookWeb.Endpoint, Livebook.Config.secret!("LIVEBOOK_SECRET_KEY_BASE") || Base.encode64(:crypto.strong_rand_bytes(48)) -config :livebook, :root_path, Livebook.Config.root_path!("LIVEBOOK_ROOT_PATH") - if password = Livebook.Config.password!("LIVEBOOK_PASSWORD") do config :livebook, authentication_mode: :password, password: password else @@ -32,3 +30,10 @@ config :livebook, :default_runtime, Livebook.Config.default_runtime!("LIVEBOOK_DEFAULT_RUNTIME") || {Livebook.Runtime.ElixirStandalone, []} + +root_path = + Livebook.Config.root_path!("LIVEBOOK_ROOT_PATH") + |> Livebook.FileSystem.Utils.ensure_dir_path() + +local_file_system = Livebook.FileSystem.Local.new(default_path: root_path) +config :livebook, :file_systems, [local_file_system] diff --git a/lib/livebook/config.ex b/lib/livebook/config.ex index 13c948d1216..b7e9d8f1df5 100644 --- a/lib/livebook/config.ex +++ b/lib/livebook/config.ex @@ -1,6 +1,8 @@ defmodule Livebook.Config do @moduledoc false + alias Livebook.FileSystem + @type auth_mode() :: :token | :password | :disabled @doc """ @@ -33,11 +35,40 @@ defmodule Livebook.Config do end @doc """ - Return the root path for persisting notebooks. + Returns the list of currently available file systems. + """ + @spec file_systems() :: list(FileSystem.t()) + def file_systems() do + Application.fetch_env!(:livebook, :file_systems) + end + + @doc """ + Appends a new file system to the configured ones. + """ + @spec append_file_system(FileSystem.t()) :: list(FileSystem.t()) + def append_file_system(file_system) do + file_systems = Enum.uniq(file_systems() ++ [file_system]) + Application.put_env(:livebook, :file_systems, file_systems, persistent: true) + file_systems + end + + @doc """ + Removes the given file system from the configured ones. + """ + @spec remove_file_system(FileSystem.t()) :: list(FileSystem.t()) + def remove_file_system(file_system) do + file_systems = List.delete(file_systems(), file_system) + Application.put_env(:livebook, :file_systems, file_systems, persistent: true) + file_systems + end + + @doc """ + Returns the default directory. """ - @spec root_path() :: binary() - def root_path() do - Application.fetch_env!(:livebook, :root_path) + @spec default_dir() :: FileSystem.File.t() + def default_dir() do + [file_system | _] = Livebook.Config.file_systems() + FileSystem.File.new(file_system) end ## Parsing diff --git a/lib/livebook/content_loader.ex b/lib/livebook/content_loader.ex index 284d62a3d44..33cb7e12cff 100644 --- a/lib/livebook/content_loader.ex +++ b/lib/livebook/content_loader.ex @@ -1,6 +1,8 @@ defmodule Livebook.ContentLoader do @moduledoc false + alias Livebook.Utils.HTTP + @doc """ Rewrite known URLs, so that they point to plain text file rather than HTML. @@ -55,28 +57,13 @@ defmodule Livebook.ContentLoader do @doc """ Loads binary content from the given URl and validates if its plain text. - - Supports local file:// URLs and remote http(s):// URLs. """ @spec fetch_content(String.t()) :: {:ok, String.t()} | {:error, String.t()} - def fetch_content(url) - - def fetch_content("file://" <> path) do - case File.read(path) do - {:ok, content} -> - {:ok, content} - - {:error, error} -> - message = :file.format_error(error) - {:error, "failed to read #{path}, reason: #{message}"} - end - end - def fetch_content(url) do - case :httpc.request(:get, {url, []}, http_opts(), body_format: :binary) do - {:ok, {{_, 200, _}, headers, body}} -> + case HTTP.request(:get, url) do + {:ok, 200, headers, body} -> valid_content? = - case fetch_content_type(headers) do + case HTTP.fetch_content_type(headers) do {:ok, content_type} -> content_type in ["text/plain", "text/markdown"] :error -> false end @@ -91,40 +78,4 @@ defmodule Livebook.ContentLoader do {:error, "failed to download notebook from the given URL"} end end - - defp fetch_content_type(headers) do - case Enum.find(headers, fn {key, _} -> key == 'content-type' end) do - {_, value} -> - {:ok, - value - |> List.to_string() - |> String.split(";") - |> hd()} - - _ -> - :error - end - end - - crt_file = CAStore.file_path() - crt = File.read!(crt_file) - pems = :public_key.pem_decode(crt) - ders = Enum.map(pems, fn {:Certificate, der, _} -> der end) - - # Note: we need to load the certificates at compilation time, - # as we don't have access to package files in Escript. - @cacerts ders - - defp http_opts() do - [ - # Use secure options, see https://gist.github.com/jonatanklosko/5e20ca84127f6b31bbe3906498e1a1d7 - ssl: [ - verify: :verify_peer, - cacerts: @cacerts, - customize_hostname_check: [ - match_fun: :public_key.pkix_verify_hostname_match_fun(:https) - ] - ] - ] - end end diff --git a/lib/livebook/file_system.ex b/lib/livebook/file_system.ex new file mode 100644 index 00000000000..d6ab082270d --- /dev/null +++ b/lib/livebook/file_system.ex @@ -0,0 +1,133 @@ +defprotocol Livebook.FileSystem do + @moduledoc false + + # This protocol defines an interface for file systems + # that can be plugged into Livebook. + + @typedoc """ + A path uniquely idenfies file in the file system. + + Path has most of the semantics of regular file paths, + with the following exceptions: + + * path must be be absolute for consistency + + * directory path must have a trailing slash, whereas + regular file path must not have a trailing slash. + Rationale: some file systems allow a directory and + a file with the same name to co-exist, while path + needs to distinguish between them + """ + @type path :: String.t() + + @typedoc """ + A human-readable error message clarifying the operation + failure reason. + """ + @type error :: String.t() + + @type access :: :read | :write | :read_write | :none + + @doc """ + Returns the default directory path. + + To some extent this is similar to current working directory + in a regular file system. For most file systems this + will just be the root path. + """ + @spec default_path(t()) :: path() + def default_path(file_system) + + @doc """ + Returns a list of files located in the given directory. + """ + @spec list(t(), path(), boolean()) :: {:ok, list(path())} | {:error, error()} + def list(file_system, path, recursive) + + @doc """ + Returns binary content of the given file. + """ + @spec read(t(), path()) :: {:ok, binary()} | {:error, error()} + def read(file_system, path) + + @doc """ + Writes the given binary content to the given file. + + If the file exists, it gets overridden. + + If the file doesn't exist, it gets created along with + all the necessary directories. + """ + @spec write(t(), path(), binary()) :: :ok | {:error, error()} + def write(file_system, path, content) + + @doc """ + Returns the current access level to the given file. + + If determining the access is costly, then this function may + always return the most liberal access, since all access + functions return error on an invalid attempt. + """ + @spec access(t(), path()) :: {:ok, access()} | {:error, error()} + def access(file_system, path) + + @doc """ + Creates the given directory unless it already exists. + + All necessary parent directories are created as well. + """ + @spec create_dir(t(), path()) :: :ok | {:error, error()} + def create_dir(file_system, path) + + @doc """ + Removes the given file. + + If a directory is given, all of its contents are removed + recursively. + + If the file doesn't exist, no error is returned. + """ + @spec remove(t(), path()) :: :ok | {:error, error()} + def remove(file_system, path) + + @doc """ + Copies the given file. + + The given files must be of the same type. + + If regular files are given, the contents are copied, + potentially overriding the destination if it already exists. + + If directories are given, the directory contents are copied + recursively. + """ + @spec copy(t(), path(), path()) :: :ok | {:error, error()} + def copy(file_system, source_path, destination_path) + + @doc """ + Renames the given file. + + If a directory is given, it gets renamed as expected and + consequently all of the child paths change. + + If the destination exists, an error is returned. + """ + @spec rename(t(), path(), path()) :: :ok | {:error, error()} + def rename(file_system, source_path, destination_path) + + @doc """ + Returns a version identifier for the given file. + + The resulting value must be a string of ASCII characters + placed between double quotes, suitable for use as the + value of the ETag HTTP header. + """ + @spec etag_for(t(), path()) :: {:ok, String.t()} | {:error, error()} + def etag_for(file_system, path) + + @doc """ + Checks if the given path exists in the file system. + """ + @spec exists?(t(), path()) :: {:ok, boolean()} | {:error, error()} + def exists?(file_system, path) +end diff --git a/lib/livebook/file_system/file.ex b/lib/livebook/file_system/file.ex new file mode 100644 index 00000000000..0095381cf60 --- /dev/null +++ b/lib/livebook/file_system/file.ex @@ -0,0 +1,253 @@ +defmodule Livebook.FileSystem.File do + @moduledoc false + + # A file points to a specific location in the given + # file system. + # + # This module provides a number of high-level functions + # similar to the `File` and `Path` core module. Many + # functions simply delegate the work to the underlying + # file system. + + defstruct [:file_system, :path] + + alias Livebook.FileSystem + + @type t :: %__MODULE__{ + file_system: FileSystem.t(), + path: FileSystem.path() + } + + @doc """ + Builds a new file struct. + + If no path is given, the default file system one is used. + """ + @spec new(FileSystem.t(), FileSystem.path() | nil) :: t() + def new(file_system, path \\ nil) do + path = + if path do + FileSystem.Utils.normalize_path!(path) + else + FileSystem.default_path(file_system) + end + + %__MODULE__{file_system: file_system, path: path} + end + + @doc """ + Returns a new file within the `Livebook.FileSystem.Local` + file system. + """ + @spec local(FileSystem.path()) :: t() + def local(path) do + new(FileSystem.Local.new(), path) + end + + @doc """ + Returns a new file resulting from expanding `path` + against `file`. + + An absolute path may be given, in which case it + replaces the file path altogether. + """ + @spec relative(t(), String.t()) :: t() + def relative(file, path) do + dir = if dir?(file), do: file, else: containing_dir(file) + path = FileSystem.Utils.expand_path(path, dir.path) + new(file.file_system, path) + end + + @doc """ + Checks if the given file is a directory. + + Note: this check relies solely on the file path. + """ + @spec dir?(t()) :: boolean() + def dir?(file) do + FileSystem.Utils.dir_path?(file.path) + end + + @doc """ + Checks if the given file is a regular file. + + Note: this check relies solely on the file path. + """ + @spec regular?(t()) :: boolean() + def regular?(file) do + FileSystem.Utils.regular_path?(file.path) + end + + @doc """ + Returns file name. + """ + @spec name(t()) :: String.t() + def name(file) do + Path.basename(file.path) + end + + @doc """ + Returns a directory that contains the given file. + + If a directory is given, the parent directory is returned. + Root directory is mapped to itself for consistency. + """ + @spec containing_dir(t()) :: t() + def containing_dir(file) do + parent_path = + if file.path == "/" do + "/" + else + file.path + |> String.trim_trailing("/") + |> Path.dirname() + |> FileSystem.Utils.ensure_dir_path() + end + + new(file.file_system, parent_path) + end + + @doc """ + Returns a list of files located in the given directory. + + ## Options + + * `:recursive` - whether to traverse all nested directories, + defaults to `false` + """ + @spec list(t(), keyword()) :: {:ok, list(t())} | {:error, FileSystem.error()} + def list(file, opts \\ []) do + recursive = Keyword.get(opts, :recursive, false) + + with {:ok, paths} <- FileSystem.list(file.file_system, file.path, recursive) do + files = for path <- paths, do: new(file.file_system, path) + {:ok, files} + end + end + + @doc """ + Returns binary content of the given file. + """ + @spec read(t()) :: {:ok, binary()} | {:error, FileSystem.error()} + def read(file) do + FileSystem.read(file.file_system, file.path) + end + + @doc """ + Writes the given binary content to the given file. + """ + @spec write(t(), binary()) :: :ok | {:error, FileSystem.error()} + def write(file, content) do + FileSystem.write(file.file_system, file.path, content) + end + + @doc """ + Returns the current access level to the given file. + """ + @spec access(t()) :: {:ok, FileSystem.access()} | {:error, FileSystem.error()} + def access(file) do + FileSystem.access(file.file_system, file.path) + end + + @doc """ + Creates the given directory unless it already exists. + """ + @spec create_dir(t()) :: {:ok, FileSystem.access()} | {:error, FileSystem.error()} + def create_dir(file) do + FileSystem.create_dir(file.file_system, file.path) + end + + @doc """ + Removes the given file. + """ + @spec remove(t()) :: :ok | {:error, FileSystem.error()} + def remove(file) do + FileSystem.remove(file.file_system, file.path) + end + + @doc """ + Copies the given file or directory contents. + + Files from different file systems are supported, + however keep in mind that this involves reading + contents of individual files from one file system + and writing them to the other. + """ + @spec copy(t(), t()) :: :ok | {:error, FileSystem.error()} + def copy(source, destination) + + def copy(%{file_system: file_system} = source, %{file_system: file_system} = destination) do + FileSystem.copy(file_system, source.path, destination.path) + end + + def copy(source, destination) do + FileSystem.Utils.assert_same_type!(source.path, destination.path) + + if dir?(source) do + with {:ok, child_files} <- list(source, recursive: true) do + child_files + |> Enum.filter(®ular?/1) + |> Enum.reduce(:ok, fn child_file, result -> + with :ok <- result do + path_sufix = String.replace_leading(child_file.path, source.path, "") + child_destination = relative(destination, path_sufix) + copy_regular_file(child_file, child_destination) + end + end) + end + else + copy_regular_file(source, destination) + end + end + + defp copy_regular_file(source, destination) do + with {:ok, content} <- read(source) do + write(destination, content) + end + end + + @doc """ + Renames the given file. + + Files from different file systems are supported, + however keep in mind that this involves reading + contents of individual files from one file system + and writing them to the other. + """ + @spec rename(t(), t()) :: :ok | {:error, FileSystem.error()} + def rename(source, destination) + + def rename(%{file_system: file_system} = source, %{file_system: file_system} = destination) do + FileSystem.rename(file_system, source.path, destination.path) + end + + def rename(source, destination) do + FileSystem.Utils.assert_same_type!(source.path, destination.path) + + with {:ok, destination_exist?} <- exists?(destination) do + if destination_exist? do + FileSystem.Utils.posix_error(:eexist) + else + with :ok <- copy(source, destination) do + remove(source) + end + end + end + end + + @doc """ + Returns a version identifier for the given file. + """ + @spec etag_for(t()) :: {:ok, String.t()} | {:error, FileSystem.error()} + def etag_for(file) do + FileSystem.etag_for(file.file_system, file.path) + end + + @doc """ + Checks if the given file exists. + """ + @spec exists?(t()) :: {:ok, boolean()} | {:error, FileSystem.error()} + def exists?(file) do + FileSystem.exists?(file.file_system, file.path) + end +end diff --git a/lib/livebook/file_system/local.ex b/lib/livebook/file_system/local.ex new file mode 100644 index 00000000000..bdbca00a1cc --- /dev/null +++ b/lib/livebook/file_system/local.ex @@ -0,0 +1,184 @@ +defmodule Livebook.FileSystem.Local do + @moduledoc false + + # File system backed by local disk. + + defstruct [:default_path] + + alias Livebook.FileSystem + + @type t :: %__MODULE__{ + default_path: FileSystem.path() + } + + @doc """ + Returns a new file system struct. + + ## Options + + * `:default_path` - the default directory path. Defaults + to the current working directory + """ + @spec new(keyword()) :: t() + def new(opts \\ []) do + default_path = + opts + |> Keyword.get_lazy(:default_path, fn -> + File.cwd!() |> FileSystem.Utils.ensure_dir_path() + end) + |> FileSystem.Utils.normalize_path!(type: :directory) + + %__MODULE__{default_path: default_path} + end +end + +defimpl Livebook.FileSystem, for: Livebook.FileSystem.Local do + alias Livebook.FileSystem + + def default_path(file_system) do + file_system.default_path + end + + def list(file_system, path, recursive) do + path = FileSystem.Utils.normalize_path!(path, type: :directory) + + case File.ls(path) do + {:ok, filenames} -> + paths = + Enum.map(filenames, fn name -> + path = Path.join(path, name) + if File.dir?(path), do: path <> "/", else: path + end) + + to_traverse = + if recursive do + Enum.filter(paths, &FileSystem.Utils.dir_path?/1) + else + [] + end + + Enum.reduce(to_traverse, {:ok, paths}, fn path, result -> + with {:ok, current_paths} <- result, + {:ok, new_paths} <- list(file_system, path, recursive) do + {:ok, current_paths ++ new_paths} + end + end) + + {:error, error} -> + FileSystem.Utils.posix_error(error) + end + end + + def read(_file_system, path) do + path = FileSystem.Utils.normalize_path!(path, type: :regular) + + case File.read(path) do + {:ok, binary} -> {:ok, binary} + {:error, error} -> FileSystem.Utils.posix_error(error) + end + end + + def write(_file_system, path, content) do + path = FileSystem.Utils.normalize_path!(path, type: :regular) + + dir = Path.dirname(path) + + with :ok <- File.mkdir_p(dir), + :ok <- File.write(path, content) do + :ok + else + {:error, error} -> FileSystem.Utils.posix_error(error) + end + end + + def access(_file_system, path) do + path = FileSystem.Utils.normalize_path!(path) + + case File.stat(path) do + {:ok, stat} -> {:ok, stat.access} + {:error, error} -> FileSystem.Utils.posix_error(error) + end + end + + def create_dir(_file_system, path) do + path = FileSystem.Utils.normalize_path!(path, type: :directory) + + case File.mkdir_p(path) do + :ok -> :ok + {:error, error} -> FileSystem.Utils.posix_error(error) + end + end + + def remove(_file_system, path) do + path = FileSystem.Utils.normalize_path!(path) + + case File.rm_rf(path) do + {:ok, _paths} -> :ok + {:error, error} -> FileSystem.Utils.posix_error(error) + end + end + + def copy(_file_system, source_path, destination_path) do + source_path = FileSystem.Utils.normalize_path!(source_path) + destination_path = FileSystem.Utils.normalize_path!(destination_path) + FileSystem.Utils.assert_same_type!(source_path, destination_path) + + containing_dir = Path.dirname(destination_path) + + case File.mkdir_p(containing_dir) do + :ok -> + case File.cp_r(source_path, destination_path) do + {:ok, _paths} -> :ok + {:error, error, _path} -> FileSystem.Utils.posix_error(error) + end + + {:error, error} -> + FileSystem.Utils.posix_error(error) + end + end + + def rename(_file_system, source_path, destination_path) do + source_path = FileSystem.Utils.normalize_path!(source_path) + destination_path = FileSystem.Utils.normalize_path!(destination_path) + FileSystem.Utils.assert_same_type!(source_path, destination_path) + + if File.exists?(destination_path) do + FileSystem.Utils.posix_error(:eexist) + else + containing_dir = Path.dirname(destination_path) + + with :ok <- File.mkdir_p(containing_dir), + :ok <- File.rename(source_path, destination_path) do + :ok + else + {:error, error} -> + FileSystem.Utils.posix_error(error) + end + end + end + + def etag_for(_file_system, path) do + path = FileSystem.Utils.normalize_path!(path) + + case File.stat(path) do + {:ok, stat} -> + %{size: size, mtime: mtime} = stat + hash = {size, mtime} |> :erlang.phash2() |> Integer.to_string(16) + etag = <> + {:ok, etag} + + {:error, error} -> + FileSystem.Utils.posix_error(error) + end + end + + def exists?(_file_system, path) do + path = FileSystem.Utils.normalize_path!(path) + + if FileSystem.Utils.dir_path?(path) do + {:ok, File.dir?(path)} + else + {:ok, File.exists?(path)} + end + end +end diff --git a/lib/livebook/file_system/s3.ex b/lib/livebook/file_system/s3.ex new file mode 100644 index 00000000000..575c80c7c4b --- /dev/null +++ b/lib/livebook/file_system/s3.ex @@ -0,0 +1,365 @@ +defmodule Livebook.FileSystem.S3 do + @moduledoc false + + # File system backed by an S3 bucket. + + defstruct [:bucket_url, :access_key_id, :secret_access_key] + + @type t :: %__MODULE__{ + bucket_url: String.t(), + access_key_id: String.t(), + secret_access_key: String.t() + } + + @doc """ + Returns a new file system struct. + """ + @spec new(String.t(), String.t(), String.t()) :: t() + def new(bucket_url, access_key_id, secret_access_key) do + bucket_url = String.trim_trailing(bucket_url, "/") + + %__MODULE__{ + bucket_url: bucket_url, + access_key_id: access_key_id, + secret_access_key: secret_access_key + } + end +end + +defimpl Livebook.FileSystem, for: Livebook.FileSystem.S3 do + alias Livebook.FileSystem + alias Livebook.Utils.HTTP + alias Livebook.FileSystem.S3.XML + + def default_path(_file_system) do + "/" + end + + def list(file_system, path, recursive) do + "/" <> dir_key = FileSystem.Utils.normalize_path!(path, type: :directory) + + delimiter = if recursive, do: nil, else: "/" + + with {:ok, %{keys: keys}} <- list_objects(file_system, prefix: dir_key, delimiter: delimiter) do + if keys == [] do + FileSystem.Utils.posix_error(:enoent) + else + paths = keys |> List.delete(dir_key) |> Enum.map(&("/" <> &1)) + {:ok, paths} + end + end + end + + def read(file_system, path) do + "/" <> key = FileSystem.Utils.normalize_path!(path, type: :regular) + get_object(file_system, key) + end + + def write(file_system, path, content) do + "/" <> key = FileSystem.Utils.normalize_path!(path, type: :regular) + put_object(file_system, key, content) + end + + def access(_file_system, path) do + FileSystem.Utils.normalize_path!(path) + {:ok, :read_write} + end + + def create_dir(file_system, path) do + "/" <> key = FileSystem.Utils.normalize_path!(path, type: :directory) + # S3 has no concept of directories, but keys with trailing + # slash are interpreted as such, so we create an empty + # object for the given key + put_object(file_system, key, nil) + end + + def remove(file_system, path) do + "/" <> key = FileSystem.Utils.normalize_path!(path) + + if FileSystem.Utils.dir_path?(path) do + with {:ok, %{keys: keys}} <- list_objects(file_system, prefix: key) do + if keys == [] do + FileSystem.Utils.posix_error(:enoent) + else + delete_objects(file_system, keys) + end + end + else + delete_object(file_system, key) + end + end + + def copy(file_system, source_path, destination_path) do + "/" <> source_key = source_path = FileSystem.Utils.normalize_path!(source_path) + "/" <> destination_key = destination_path = FileSystem.Utils.normalize_path!(destination_path) + FileSystem.Utils.assert_same_type!(source_path, destination_path) + + if FileSystem.Utils.dir_path?(source_path) do + with {:ok, %{bucket: bucket, keys: keys}} <- list_objects(file_system, prefix: source_key) do + if keys == [] do + FileSystem.Utils.posix_error(:enoent) + else + keys + |> Enum.map(fn key -> + renamed_key = String.replace_prefix(key, source_key, destination_key) + + Task.async(fn -> + copy_object(file_system, bucket, key, renamed_key) + end) + end) + |> Task.await_many(:infinity) + |> Enum.filter(&match?({:error, _}, &1)) + |> case do + [] -> :ok + [error | _] -> error + end + end + end + else + with {:ok, bucket} <- get_bucket_name(file_system) do + copy_object(file_system, bucket, source_key, destination_key) + end + end + end + + def rename(file_system, source_path, destination_path) do + source_path = FileSystem.Utils.normalize_path!(source_path) + "/" <> destination_key = destination_path = FileSystem.Utils.normalize_path!(destination_path) + FileSystem.Utils.assert_same_type!(source_path, destination_path) + + with {:ok, destination_exists?} <- object_exists(file_system, destination_key) do + if destination_exists? do + FileSystem.Utils.posix_error(:eexist) + else + # S3 doesn't support an atomic rename/move operation, + # so we implement it as copy followed by remove + with :ok <- copy(file_system, source_path, destination_path) do + remove(file_system, source_path) + end + end + end + end + + def etag_for(file_system, path) do + "/" <> key = FileSystem.Utils.normalize_path!(path, type: :regular) + + with {:ok, %{etag: etag}} <- head_object(file_system, key) do + {:ok, etag} + end + end + + def exists?(file_system, path) do + "/" <> key = FileSystem.Utils.normalize_path!(path) + object_exists(file_system, key) + end + + # Requests + + defp list_objects(file_system, opts) do + prefix = opts[:prefix] + delimiter = opts[:delimiter] + + query = %{"list-type" => "2", "prefix" => prefix, "delimiter" => delimiter} + + case request(file_system, :get, "/", query: query) |> encode() do + {:ok, 200, _headers, %{"ListBucketResult" => result}} -> + bucket = result["Name"] + file_keys = result |> xml_get_list("Contents") |> Enum.map(& &1["Key"]) + prefix_keys = result |> xml_get_list("CommonPrefixes") |> Enum.map(& &1["Prefix"]) + keys = file_keys ++ prefix_keys + {:ok, %{bucket: bucket, keys: keys}} + + other -> + request_response_to_error(other) + end + end + + defp get_bucket_name(file_system) do + # We have bucket URL, but it's not straightforward to extract + # bucket name from the URL, because it may be either the path + # or a part of the host. + # + # Endpoints that return bucket information doesn't include the + # name, but the listing endpoint does, so we just list keys + # with an upper limit of 0 and retrieve the bucket name. + + query = %{"list-type" => "2", "max-keys" => "0"} + + case request(file_system, :get, "/", query: query) |> encode() do + {:ok, 200, _headers, %{"ListBucketResult" => %{"Name" => bucket}}} -> + {:ok, bucket} + + other -> + request_response_to_error(other) + end + end + + defp get_object(file_system, key) do + case request(file_system, :get, "/" <> key) do + {:ok, 200, _headers, body} -> {:ok, body} + {:ok, 404, _headers, _body} -> FileSystem.Utils.posix_error(:enoent) + other -> request_response_to_error(other) + end + end + + defp put_object(file_system, key, content) do + case request(file_system, :put, "/" <> key, body: content) |> encode() do + {:ok, 200, _headers, _body} -> :ok + other -> request_response_to_error(other) + end + end + + defp head_object(file_system, key) do + case request(file_system, :head, "/" <> key) do + {:ok, 200, headers, _body} -> + {:ok, etag} = HTTP.fetch_header(headers, "etag") + {:ok, %{etag: etag}} + + {:ok, 404, _headers, _body} -> + FileSystem.Utils.posix_error(:enoent) + + other -> + request_response_to_error(other) + end + end + + defp copy_object(file_system, bucket, source_key, destination_key) do + copy_source = bucket <> "/" <> source_key + + headers = [{"x-amz-copy-source", copy_source}] + + case request(file_system, :put, "/" <> destination_key, headers: headers) |> encode() do + {:ok, 200, _headers, _body} -> :ok + {:ok, 404, _headers, _body} -> FileSystem.Utils.posix_error(:enoent) + other -> request_response_to_error(other) + end + end + + defp delete_object(file_system, key) do + case request(file_system, :delete, "/" <> key) |> encode() do + {:ok, 204, _headers, _body} -> :ok + {:ok, 404, _headers, _body} -> :ok + other -> request_response_to_error(other) + end + end + + defp delete_objects(file_system, keys) do + objects = Enum.map(keys, fn key -> %{"Key" => key} end) + + body = + %{"Delete" => %{"Object" => objects, "Quiet" => "true"}} + |> XML.encode_to_iodata!() + |> IO.iodata_to_binary() + + body_md5 = :crypto.hash(:md5, body) |> Base.encode64() + + headers = [{"Content-MD5", body_md5}] + + case request(file_system, :post, "/", query: %{"delete" => ""}, headers: headers, body: body) + |> encode() do + {:ok, 200, _headers, %{"Error" => errors}} -> {:error, format_errors(errors)} + {:ok, 200, _headers, _body} -> :ok + other -> request_response_to_error(other) + end + end + + defp object_exists(file_system, key) do + # It is possible for /dir/obj to exist without the /dir/ object, + # but we still consider it as existing. That's why we list + # objects instead of checking the key directly. + + with {:ok, %{keys: keys}} <- list_objects(file_system, prefix: key, delimiter: "/") do + exists? = + if String.ends_with?(key, "/") do + keys != [] + else + key in keys + end + + {:ok, exists?} + end + end + + defp request_response_to_error(error) + + defp request_response_to_error({:ok, 403, _headers, %{"Error" => %{"Message" => message}}}) do + {:error, "access denied, " <> Livebook.Utils.downcase_first(message)} + end + + defp request_response_to_error({:ok, 403, _headers, _body}) do + {:error, "access denied"} + end + + defp request_response_to_error({:ok, _status, _headers, %{"Error" => error}}) do + {:error, format_errors(error)} + end + + defp request_response_to_error({:ok, _status, _headers, _body}) do + {:error, "unexpected response"} + end + + defp request_response_to_error({:error, _error}) do + {:error, "failed to make an HTTP request"} + end + + defp format_errors(%{"Message" => message}) do + Livebook.Utils.downcase_first(message) + end + + defp format_errors([%{"Message" => message} | errors]) do + Livebook.Utils.downcase_first(message) <> ", and #{length(errors)} more errors" + end + + defp request(file_system, method, path, opts \\ []) do + query = opts[:query] || %{} + headers = opts[:headers] || [] + body = opts[:body] + + %{host: host} = URI.parse(file_system.bucket_url) + + url = file_system.bucket_url <> path <> "?" <> URI.encode_query(query) + + credentials = %{ + service: "s3", + access_key_id: file_system.access_key_id, + secret_access_key: file_system.secret_access_key, + region: region_from_uri(file_system.bucket_url) + } + + now = NaiveDateTime.utc_now() |> NaiveDateTime.truncate(:second) + + headers = [{"Host", host} | headers] + + headers = + Livebook.FileSystem.S3.Signature.sign_v4(credentials, now, method, url, headers, body || "") + + body = body && {"application/octet-stream", body} + HTTP.request(method, url, headers: headers, body: body) + end + + defp region_from_uri(uri) do + # For many services the API host is of the form *.[region].[rootdomain].com + %{host: host} = URI.parse(uri) + host |> String.split(".") |> Enum.reverse() |> Enum.at(2, "auto") + end + + defp encode({:ok, status, headers, body}) do + case HTTP.fetch_content_type(headers) do + {:ok, content_type} when content_type in ["text/xml", "application/xml"] -> + {:ok, status, headers, XML.decode!(body)} + + :error -> + {:ok, status, headers, body} + end + end + + defp encode(other), do: other + + defp xml_get_list(xml_map, key) do + case xml_map do + %{^key => item} when is_map(item) -> [item] + %{^key => items} when is_list(items) -> items + _ -> [] + end + end +end diff --git a/lib/livebook/file_system/s3/signature.ex b/lib/livebook/file_system/s3/signature.ex new file mode 100644 index 00000000000..2aa2107f364 --- /dev/null +++ b/lib/livebook/file_system/s3/signature.ex @@ -0,0 +1,202 @@ +defmodule Livebook.FileSystem.S3.Signature do + @moduledoc false + + # Adapted from https://github.com/aws-beam/aws-elixir/blob/v0.8.0/lib/aws/signature.ex + # + # Implements the Signature algorithm v4. + # See: https://docs.aws.amazon.com/general/latest/gr/signature-version-4.html + # + # Briefly speaking, for every request we compute a signature + # based on the auth credentials, headers, body and time. Upon + # receiving the request, the S3 service computes the signature + # on its own and compares with the one we sent. + + @doc """ + Generate headers with an AWS signature version 4 for the specified + request using the specified time. + """ + def sign_v4(credentials, now, method, url, headers, body) do + long_date = NaiveDateTime.to_iso8601(now, :basic) <> "Z" + short_date = Date.to_iso8601(now, :basic) + + headers = + headers + |> add_date_header(long_date) + |> add_content_hash(body) + + canonical_request = canonical_request(method, url, headers, body) + hashed_canonical_request = sha256_hexdigest(canonical_request) + credential_scope = credential_scope(short_date, credentials.region, credentials.service) + + signing_key = signing_key(credentials, short_date) + + string_to_sign = string_to_sign(long_date, credential_scope, hashed_canonical_request) + + signature = hmac_sha256_hexdigest(signing_key, string_to_sign) + signed_headers = signed_headers(headers) + + authorization = + authorization(credentials.access_key_id, credential_scope, signed_headers, signature) + + add_authorization_header(headers, authorization) + end + + defp add_date_header(headers, date) do + [{"X-Amz-Date", date} | headers] + end + + # Add an X-Amz-Content-SHA256 header which is the hash of the payload. + # This header is required for S3 when using the v4 signature. Adding it + # in requests for all services does not cause any issues. + defp add_content_hash(headers, body) do + [{"X-Amz-Content-SHA256", sha256_hexdigest(body)} | headers] + end + + defp add_authorization_header(headers, authorization) do + [{"Authorization", authorization} | headers] + end + + # Generate an AWS4-HMAC-SHA256 authorization signature. + defp authorization(access_key_id, credential_scope, signed_headers, signature) do + Enum.join( + [ + "AWS4-HMAC-SHA256 ", + "Credential=", + access_key_id, + "/", + credential_scope, + ", ", + "SignedHeaders=", + signed_headers, + ", ", + "Signature=", + signature + ], + "" + ) + end + + # Convert a list of headers to canonical header format. Leading and trailing + # whitespace around header names and values is stripped, header names are + # lowercased, and headers are newline-joined in alphabetical order (with a + # trailing newline). + defp canonical_headers(headers) do + headers + |> Enum.map(fn {name, value} -> + name = String.downcase(name) |> String.trim() + value = String.trim(value) + {name, value} + end) + |> Enum.sort(fn {a, _}, {b, _} -> a <= b end) + |> Enum.map(fn {name, value} -> [name, ":", value, "\n"] end) + |> Enum.join() + end + + # Process and merge request values into a canonical request for AWS signature + # version 4. + defp canonical_request(method, url, headers, body) when is_atom(method) do + Atom.to_string(method) + |> String.upcase() + |> canonical_request(url, headers, body) + end + + defp canonical_request(method, url, headers, body) do + {canonical_url, canonical_query_string} = split_url(url) + canonical_headers = canonical_headers(headers) + signed_headers = signed_headers(headers) + payload_hash = sha256_hexdigest(body) + + Enum.join( + [ + method, + canonical_url, + canonical_query_string, + canonical_headers, + signed_headers, + payload_hash + ], + "\n" + ) + end + + # Generate a credential scope from a short date in `YYMMDD` format, a region + # identifier and a service identifier. + defp credential_scope(short_date, region, service) do + Enum.join([short_date, region, service, "aws4_request"], "/") + end + + # Convert a list of headers to canonicals signed header format. Leading and + # trailing whitespace around names is stripped, header names are lowercased, + # and header names are semicolon-joined in alphabetical order. + @spec signed_headers([{binary(), binary()}]) :: binary() + defp signed_headers(headers) do + headers + |> Enum.map(fn {name, _value} -> name |> String.downcase() |> String.trim() end) + |> Enum.sort() + |> Enum.join(";") + end + + # Generate a signing key from a secret access key, a short date in `YYMMDD` + # format, a region identifier and a service identifier. + defp signing_key(%{} = credentials, short_date) do + ("AWS4" <> credentials.secret_access_key) + |> hmac_sha256(short_date) + |> hmac_sha256(credentials.region) + |> hmac_sha256(credentials.service) + |> hmac_sha256("aws4_request") + end + + # Strip the query string from the URL, if one if present, and return the URL + # and the normalized query string as separate values. + defp split_url(url) do + url = URI.parse(url) + + {uri_encode(url.path), normalize_query(url.query)} + end + + # Copied from https://github.com/ex-aws/ex_aws/blob/623478ed321ffc6c07fdd7236a2f0e03f1cbd517/lib/ex_aws/request/url.ex#L108 + defp uri_encode(url), do: URI.encode(url, &valid_path_char?/1) + + # Space character + defp valid_path_char?(?\ ), do: false + defp valid_path_char?(?/), do: true + + defp valid_path_char?(c) do + URI.char_unescaped?(c) && !URI.char_reserved?(c) + end + + # Sort query params by name first, then by value (if present). Append "=" to + # params with missing value. + # Example: "foo=bar&baz" becomes "baz=&foo=bar" + defp normalize_query(nil), do: "" + + defp normalize_query(query) do + query + |> URI.decode_query() + |> Enum.sort() + |> URI.encode_query() + end + + # Generate the text to sign from a long date in `YYMMDDTHHMMSSZ` format, a + # credential scope and a hashed canonical request. + defp string_to_sign(long_date, credential_scope, hashed_canonical_request) do + Enum.join( + ["AWS4-HMAC-SHA256", long_date, credential_scope, hashed_canonical_request], + "\n" + ) + end + + # Helpers + + defp hmac_sha256(key, message) do + :crypto.mac(:hmac, :sha256, key, message) + end + + defp hmac_sha256_hexdigest(key, message) do + hmac_sha256(key, message) |> Base.encode16(case: :lower) + end + + defp sha256_hexdigest(value) do + :crypto.hash(:sha256, value) |> Base.encode16(case: :lower) + end +end diff --git a/lib/livebook/file_system/s3/xml.ex b/lib/livebook/file_system/s3/xml.ex new file mode 100644 index 00000000000..7e0e7611cf6 --- /dev/null +++ b/lib/livebook/file_system/s3/xml.ex @@ -0,0 +1,144 @@ +defmodule Livebook.FileSystem.S3.XML do + @moduledoc false + + # Adapted from https://github.com/aws-beam/aws-elixir/blob/v0.8.0/lib/aws/xml.ex + + import Record + + @text "__text" + + defrecord(:xmlElement, extract(:xmlElement, from_lib: "xmerl/include/xmerl.hrl")) + defrecord(:xmlText, extract(:xmlText, from_lib: "xmerl/include/xmerl.hrl")) + + @doc """ + Encodes a map into XML iodata. + + Raises in case of errors. + """ + @spec encode_to_iodata!(map()) :: iodata() + def encode_to_iodata!(map) do + map + |> Map.to_list() + |> Enum.map(&encode_xml_key_value/1) + |> :erlang.iolist_to_binary() + end + + @doc """ + Decodes a XML into a map. + + Raises in case of errors. + """ + @spec decode!(iodata()) :: map() + def decode!(xml) do + xml_str = :unicode.characters_to_list(xml) + opts = [{:hook_fun, &hook_fun/2}] + {element, []} = :xmerl_scan.string(xml_str, opts) + element + end + + defp encode_xml_key_value({k, v}) when is_binary(k) and is_binary(v) do + ["<", k, ">", escape_xml_string(v), ""] + end + + defp encode_xml_key_value({k, values}) when is_binary(k) and is_list(values) do + for v <- values do + encode_xml_key_value({k, v}) + end + end + + defp encode_xml_key_value({k, v}) when is_binary(k) and is_integer(v) do + ["<", k, ">", Integer.to_charlist(v), ""] + end + + defp encode_xml_key_value({k, v}) when is_binary(k) and is_float(v) do + ["<", k, ">", Float.to_charlist(v), ""] + end + + defp encode_xml_key_value({k, v}) when is_binary(k) and is_map(v) do + inner = v |> Map.to_list() |> Enum.map(&encode_xml_key_value/1) + ["<", k, ">", inner, ""] + end + + # Callback hook_fun for xmerl parser + defp hook_fun(element, global_state) when Record.is_record(element, :xmlElement) do + tag = xmlElement(element, :name) + content = xmlElement(element, :content) + + value = + case List.foldr(content, :none, &content_to_map/2) do + v = %{@text => text} -> + case String.trim(text) do + "" -> Map.delete(v, @text) + trimmed -> Map.put(v, @text, trimmed) + end + + v -> + v + end + + {%{Atom.to_string(tag) => value}, global_state} + end + + defp hook_fun(text, global_state) when Record.is_record(text, :xmlText) do + text = xmlText(text, :value) + {:unicode.characters_to_binary(text), global_state} + end + + # Convert the content of an Xml node into a map. + # When there is more than one element with the same tag name, their + # values get merged into a list. + # If the content is only text then that is what gets returned. + # If the content is a mix between text and child elements, then the + # elements are processed as described above and all the text parts + # are merged under the `__text' key. + + defp content_to_map(x, :none) do + x + end + + defp content_to_map(x, acc) when is_map(x) and is_map(acc) do + [{tag, value}] = Map.to_list(x) + + case Map.has_key?(acc, tag) do + true -> + update_fun = fn + l when is_list(l) -> [value | l] + v -> [value, v] + end + + Map.update!(acc, tag, update_fun) + + false -> + Map.put(acc, tag, value) + end + end + + defp content_to_map(x, %{@text => text} = acc) when is_binary(x) and is_map(acc) do + %{acc | @text => <>} + end + + defp content_to_map(x, acc) when is_binary(x) and is_map(acc) do + Map.put(acc, @text, x) + end + + defp content_to_map(x, acc) when is_binary(x) and is_binary(acc) do + <> + end + + defp content_to_map(x, acc) when is_map(x) and is_binary(acc) do + Map.put(x, @text, acc) + end + + # See https://github.com/ex-aws/ex_aws_s3/blob/6973f72b0b78d928c86252767e7dcebab5be0ba8/lib/ex_aws/s3.ex#L1268 + defp escape_xml_string(value) do + String.replace(value, ["'", "\"", "&", "<", ">", "\r", "\n"], fn + "'" -> "'" + "\"" -> """ + "&" -> "&" + "<" -> "<" + ">" -> ">" + "\r" -> " " + "\n" -> " " + end) + end +end diff --git a/lib/livebook/file_system/utils.ex b/lib/livebook/file_system/utils.ex new file mode 100644 index 00000000000..5f7baed1114 --- /dev/null +++ b/lib/livebook/file_system/utils.ex @@ -0,0 +1,126 @@ +defmodule Livebook.FileSystem.Utils do + @moduledoc false + + alias Livebook.FileSystem + + @doc """ + Asserts that the given path is absolute and expands it. + + ## Options + + * `:type` - if set to `:directory` or `:regular`, the + path type is also asserted against + """ + @spec normalize_path!(String.t(), keyword()) :: FileSystem.path() + def normalize_path!(path, opts \\ []) do + unless absolute_path?(path) do + raise ArgumentError, "expected an absolute path, got: #{inspect(path)}" + end + + path = expand_path(path) + + case opts[:type] do + nil -> + :ok + + :directory -> + unless dir_path?(path) do + raise ArgumentError, "expected a directory path, got: #{inspect(path)}" + end + + :regular -> + unless regular_path?(path) do + raise ArgumentError, "expected a regular file path, got: #{inspect(path)}" + end + + other -> + raise ArgumentError, + "expected :type option to be either :direcotry, :regular or nil, got: #{inspect(other)}" + end + + path + end + + defp absolute_path?("/" <> _), do: true + defp absolute_path?(_), do: false + + @doc """ + Checks if the given path describes a directory. + """ + @spec dir_path?(FileSystem.path()) :: boolean() + def dir_path?(path) do + String.ends_with?(path, "/") + end + + @doc """ + Checks if the given path describes a regular file. + """ + @spec regular_path?(FileSystem.path()) :: boolean() + def regular_path?(path) do + not String.ends_with?(path, "/") + end + + @doc """ + Asserts that the given paths are of the same type. + """ + @spec assert_same_type!(FileSystem.path(), FileSystem.path()) :: :ok + def assert_same_type!(path1, path2) do + if dir_path?(path1) != dir_path?(path2) do + raise ArgumentError, + "expected paths of the same type, got: #{inspect(path1)} and #{inspect(path2)}" + end + + :ok + end + + @doc """ + Expands the given path, optionally against the given + absolute path. + + This works similarly to `Path.expand/2`, except it + takes trailing slashes into account. + """ + @spec expand_path(String.t(), FileSystem.path() | nil) :: FileSystem.path() + def expand_path(path, relative_to \\ nil) do + if not absolute_path?(path) and relative_to == nil do + raise ArgumentError, + "expected an absolute path to expand the relative path against, but none was given" + end + + relative_to = relative_to && normalize_path!(relative_to, type: :directory) + + if path == "" do + relative_to + else + dir? = dir_path?(path) or Path.basename(path) in [".", ".."] + + case {dir?, Path.expand(path, relative_to || "")} do + {_, "/"} -> "/" + {true, path} -> path <> "/" + {false, path} -> path + end + end + end + + @doc """ + Converts the given path into dir path by appending a trailing + slash if necessary. + """ + @spec ensure_dir_path(String.t()) :: FileSystem.path() + def ensure_dir_path(path) do + if String.ends_with?(path, "/") do + path + else + path <> "/" + end + end + + @doc """ + Converts the given posix error atom into readable error tuple. + """ + @spec posix_error(atom()) :: {:error, FileSystem.error()} + def posix_error(error) do + message = error |> :file.format_error() |> List.to_string() + {:error, message} + end +end diff --git a/lib/livebook/live_markdown/export.ex b/lib/livebook/live_markdown/export.ex index 06d2c2f6560..65be5f35825 100644 --- a/lib/livebook/live_markdown/export.ex +++ b/lib/livebook/live_markdown/export.ex @@ -35,7 +35,11 @@ defmodule Livebook.LiveMarkdown.Export do end defp notebook_metadata(notebook) do - put_unless_implicit(%{}, persist_outputs: notebook.persist_outputs) + put_unless_default( + %{}, + Map.take(notebook, [:persist_outputs, :autosave_interval_s]), + Map.take(Notebook.new(), [:persist_outputs, :autosave_interval_s]) + ) end defp render_section(section, notebook, ctx) do @@ -106,7 +110,10 @@ defmodule Livebook.LiveMarkdown.Export do name: cell.name, value: value } - |> put_unless_implicit(reactive: cell.reactive, props: cell.props) + |> put_unless_default( + Map.take(cell, [:reactive, :props]), + Map.take(Cell.Input.new(), [:reactive, :props]) + ) |> Jason.encode!() metadata = cell_metadata(cell) @@ -116,7 +123,11 @@ defmodule Livebook.LiveMarkdown.Export do end defp cell_metadata(%Cell.Elixir{} = cell) do - put_unless_implicit(%{}, disable_formatting: cell.disable_formatting) + put_unless_default( + %{}, + Map.take(cell, [:disable_formatting]), + Map.take(Cell.Elixir.new(), [:disable_formatting]) + ) end defp cell_metadata(_cell), do: %{} @@ -202,9 +213,9 @@ defmodule Livebook.LiveMarkdown.Export do end end - defp put_unless_implicit(map, entries) do + defp put_unless_default(map, entries, defaults) do Enum.reduce(entries, map, fn {key, value}, map -> - if value in [false, %{}] do + if value == defaults[key] do map else Map.put(map, key, value) diff --git a/lib/livebook/live_markdown/import.ex b/lib/livebook/live_markdown/import.ex index 2fe4a4b13c2..9e567b5a14c 100644 --- a/lib/livebook/live_markdown/import.ex +++ b/lib/livebook/live_markdown/import.ex @@ -294,6 +294,9 @@ defmodule Livebook.LiveMarkdown.Import do {"persist_outputs", persist_outputs}, attrs -> Map.put(attrs, :persist_outputs, persist_outputs) + {"autosave_interval_s", autosave_interval_s}, attrs -> + Map.put(attrs, :autosave_interval_s, autosave_interval_s) + _entry, attrs -> attrs end) diff --git a/lib/livebook/notebook.ex b/lib/livebook/notebook.ex index 30375752ef2..936bbb862bc 100644 --- a/lib/livebook/notebook.ex +++ b/lib/livebook/notebook.ex @@ -13,7 +13,7 @@ defmodule Livebook.Notebook do # A notebook is divided into a number of *sections*, each # containing a number of *cells*. - defstruct [:name, :version, :sections, :persist_outputs] + defstruct [:name, :version, :sections, :persist_outputs, :autosave_interval_s] alias Livebook.Notebook.{Section, Cell} alias Livebook.Utils.Graph @@ -23,7 +23,8 @@ defmodule Livebook.Notebook do name: String.t(), version: String.t(), sections: list(Section.t()), - persist_outputs: boolean() + persist_outputs: boolean(), + autosave_interval_s: non_neg_integer() | nil } @version "1.0" @@ -37,7 +38,32 @@ defmodule Livebook.Notebook do name: "Untitled notebook", version: @version, sections: [], - persist_outputs: false + persist_outputs: default_persist_outputs(), + autosave_interval_s: default_autosave_interval_s() + } + end + + @doc """ + Returns the default value of `persist_outputs`. + """ + @spec default_persist_outputs() :: boolean() + def default_persist_outputs(), do: false + + @doc """ + Returns the default value of `autosave_interval_s`. + """ + @spec default_autosave_interval_s() :: non_neg_integer() + def default_autosave_interval_s(), do: 5 + + @doc """ + Sets all persistence related properties to their default values. + """ + @spec reset_persistence_options(t()) :: t() + def reset_persistence_options(notebook) do + %{ + notebook + | persist_outputs: default_persist_outputs(), + autosave_interval_s: default_autosave_interval_s() } end diff --git a/lib/livebook/session.ex b/lib/livebook/session.ex index 4a5b0b62672..7f133b988e8 100644 --- a/lib/livebook/session.ex +++ b/lib/livebook/session.ex @@ -46,23 +46,25 @@ defmodule Livebook.Session do use GenServer, restart: :temporary alias Livebook.Session.{Data, FileGuard} - alias Livebook.{Utils, Notebook, Delta, Runtime, LiveMarkdown} + alias Livebook.{Utils, Notebook, Delta, Runtime, LiveMarkdown, FileSystem} alias Livebook.Users.User alias Livebook.Notebook.{Cell, Section} @type state :: %{ session_id: id(), data: Data.t(), - runtime_monitor_ref: reference() | nil + runtime_monitor_ref: reference() | nil, + autosave_timer_ref: reference() | nil, + save_task_pid: pid() | nil } @type summary :: %{ session_id: id(), pid: pid(), notebook_name: String.t(), - path: String.t() | nil, - images_dir: String.t(), - origin_url: String.t() | nil + file: FileSystem.File.t() | nil, + images_dir: FileSystem.File.t(), + origin: {:file, FileSystem.File.t()} | {:url, String.t()} | nil } @typedoc """ @@ -70,8 +72,6 @@ defmodule Livebook.Session do """ @type id :: Utils.id() - @autosave_interval 5_000 - ## API @doc """ @@ -84,12 +84,12 @@ defmodule Livebook.Session do * `:notebook` - the initial `Notebook` structure (e.g. imported from a file) - * `:origin_url` - location from where the notebook was obtained, - can be a local file:// URL or a remote http(s):// URL + * `:origin` - location from where the notebook was obtained, can be either + `{:file, file}`, a remote `{:url, url}`, or `nil` - * `:path` - the file to which the notebook should be saved + * `:file` - the file to which the notebook should be saved - * `:copy_images_from` - a directory path to copy notebook images from + * `:copy_images_from` - a directory file to copy notebook images from * `:images` - a map from image name to its binary content, an alternative to `:copy_images_from` when the images are in memory @@ -319,18 +319,19 @@ defmodule Livebook.Session do end @doc """ - Asynchronously sends path update request to the server. + Asynchronously sends file location update request to the server. """ - @spec set_path(id(), String.t() | nil) :: :ok - def set_path(session_id, path) do - GenServer.cast(name(session_id), {:set_path, self(), path}) + @spec set_file(id(), FileSystem.File.t() | nil) :: :ok + def set_file(session_id, file) do + GenServer.cast(name(session_id), {:set_file, self(), file}) end @doc """ Asynchronously sends save request to the server. - If there's a path set and the notebook changed since the last save, - it will be persisted to said path. + If there's a file set and the notebook changed since the last save, + it will be persisted to said file. + Note that notebooks are automatically persisted every @autosave_interval milliseconds. """ @spec save(id()) :: :ok @@ -343,7 +344,7 @@ defmodule Livebook.Session do """ @spec save_sync(id()) :: :ok def save_sync(session_id) do - GenServer.call(name(session_id), :save) + GenServer.call(name(session_id), :save_sync) end @doc """ @@ -361,54 +362,71 @@ defmodule Livebook.Session do @impl true def init(opts) do - Process.send_after(self(), :autosave, @autosave_interval) + with {:ok, state} <- init_state(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), + else: :ok + ) do + state = schedule_autosave(state) + {:ok, state} + else + {:error, error} -> + {:stop, error} + end + end + defp init_state(opts) do id = Keyword.fetch!(opts, :id) - case init_data(opts) do - {:ok, data} -> - state = %{ - session_id: id, - data: data, - runtime_monitor_ref: nil - } - - if copy_images_from = opts[:copy_images_from] do - copy_images(state, copy_images_from) - end - - if images = opts[:images] do - dump_images(state, images) - end - - {:ok, state} + with {:ok, data} <- init_data(opts) do + state = %{ + session_id: id, + data: data, + runtime_monitor_ref: nil, + autosave_timer_ref: nil, + save_task_pid: nil + } - {:error, error} -> - {:stop, error} + {:ok, state} end end defp init_data(opts) do notebook = opts[:notebook] - path = opts[:path] - origin_url = opts[:origin_url] + file = opts[:file] + origin = opts[:origin] data = if(notebook, do: Data.new(notebook), else: Data.new()) - data = %{data | origin_url: origin_url} + data = %{data | origin: origin} - if path do - case FileGuard.lock(path, self()) do + if file do + case FileGuard.lock(file, self()) do :ok -> - {:ok, %{data | path: path}} + {:ok, %{data | file: file}} {:error, :already_in_use} -> - {:error, "the given path is already in use"} + {:error, "the given file is already in use"} end else {:ok, data} end end + defp schedule_autosave(state) do + if interval_s = state.data.notebook.autosave_interval_s do + ref = Process.send_after(self(), :autosave, interval_s * 1000) + %{state | autosave_timer_ref: ref} + else + %{state | autosave_timer_ref: nil} + end + end + @impl true def handle_call({:register_client, client_pid, user}, _from, state) do Process.monitor(client_pid) @@ -430,8 +448,8 @@ defmodule Livebook.Session do {:reply, state.data.notebook, state} end - def handle_call(:save, _from, state) do - {:reply, :ok, maybe_save_notebook(state)} + def handle_call(:save_sync, _from, state) do + {:reply, :ok, maybe_save_notebook_sync(state)} end @impl true @@ -550,32 +568,32 @@ defmodule Livebook.Session do |> handle_operation({:set_runtime, client_pid, nil})} end - def handle_cast({:set_path, client_pid, path}, state) do - if path do - FileGuard.lock(path, self()) + def handle_cast({:set_file, client_pid, file}, state) do + if file do + FileGuard.lock(file, self()) else :ok end |> case do :ok -> - if state.data.path do - FileGuard.unlock(state.data.path) + if state.data.file do + FileGuard.unlock(state.data.file) end - {:noreply, handle_operation(state, {:set_path, client_pid, path})} + {:noreply, handle_operation(state, {:set_file, client_pid, file})} {:error, :already_in_use} -> - broadcast_error(state.session_id, "failed to set new path because it is already in use") + broadcast_error(state.session_id, "failed to set new file because it is already in use") {:noreply, state} end end def handle_cast(:save, state) do - {:noreply, maybe_save_notebook(state)} + {:noreply, maybe_save_notebook_async(state)} end def handle_cast(:close, state) do - maybe_save_notebook(state) + maybe_save_notebook_sync(state) broadcast_message(state.session_id, :session_closed) {:stop, :shutdown, state} @@ -649,8 +667,7 @@ defmodule Livebook.Session do end def handle_info(:autosave, state) do - Process.send_after(self(), :autosave, @autosave_interval) - {:noreply, maybe_save_notebook(state)} + {:noreply, state |> maybe_save_notebook_async() |> schedule_autosave()} end def handle_info({:user_change, user}, state) do @@ -658,6 +675,11 @@ defmodule Livebook.Session do {:noreply, handle_operation(state, operation)} end + def handle_info({:save_finished, pid, result}, %{save_task_pid: pid} = state) do + state = %{state | save_task_pid: nil} + {:noreply, handle_save_finished(state, result)} + end + def handle_info(_message, state), do: {:noreply, state} @impl true @@ -673,67 +695,87 @@ defmodule Livebook.Session do session_id: state.session_id, pid: self(), notebook_name: state.data.notebook.name, - path: state.data.path, + file: state.data.file, images_dir: images_dir_from_state(state), - origin_url: state.data.origin_url + origin: state.data.origin } end - defp images_dir_from_state(%{data: %{path: nil}, session_id: id}) do + defp images_dir_from_state(%{data: %{file: nil}, session_id: id}) do tmp_dir = session_tmp_dir(id) - Path.join(tmp_dir, "images") + FileSystem.File.relative(tmp_dir, "images/") end - defp images_dir_from_state(%{data: %{path: path}}) do - images_dir_for_notebook(path) + defp images_dir_from_state(%{data: %{file: file}}) do + images_dir_for_notebook(file) end @doc """ - Returns images directory corresponding to the given notebook path. + Returns images directory corresponding to the given notebook file. """ - @spec images_dir_for_notebook(Path.t()) :: Path.t() - def images_dir_for_notebook(path) do - dir = Path.dirname(path) - Path.join(dir, "images") + @spec images_dir_for_notebook(FileSystem.File.t()) :: FileSystem.File.t() + def images_dir_for_notebook(file) do + file + |> FileSystem.File.containing_dir() + |> FileSystem.File.relative("images/") end defp session_tmp_dir(session_id) do - tmp_dir = System.tmp_dir!() - Path.join([tmp_dir, "livebook", "sessions", session_id]) + path = Path.join([System.tmp_dir!(), "livebook", "sessions", session_id]) <> "/" + FileSystem.File.local(path) end defp cleanup_tmp_dir(session_id) do tmp_dir = session_tmp_dir(session_id) - - if File.exists?(tmp_dir) do - File.rm_rf!(tmp_dir) - end + FileSystem.File.remove(tmp_dir) end - defp copy_images(state, from) do - if File.dir?(from) do - images_dir = images_dir_from_state(state) - File.mkdir_p!(images_dir) - File.cp_r!(from, images_dir) + defp copy_images(state, source) do + images_dir = images_dir_from_state(state) + + with {:ok, source_exists?} <- FileSystem.File.exists?(source) do + if source_exists? do + FileSystem.File.copy(source, images_dir) + else + :ok + end end end - defp move_images(state, from) do - if File.dir?(from) do - images_dir = images_dir_from_state(state) - File.mkdir_p!(images_dir) - File.rename!(from, images_dir) + defp move_images(state, source) do + images_dir = images_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 + if not destination_exists? do + # 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) + else + # If the directory exists, we use copy to place + # the images there + with :ok <- FileSystem.File.copy(source, images_dir) do + FileSystem.File.remove(source) + end + end + end + else + :ok + end end end defp dump_images(state, images) do images_dir = images_dir_from_state(state) - File.mkdir_p!(images_dir) - for {filename, content} <- images do - path = Path.join(images_dir, filename) - File.write!(path, content) - end + Enum.reduce(images, :ok, fn {filename, content}, result -> + with :ok <- result do + file = FileSystem.File.relative(images_dir, filename) + FileSystem.File.write(file, content) + end + end) end # Given any operation on `Livebook.Session.Data`, the process @@ -761,18 +803,37 @@ defmodule Livebook.Session do end end - defp after_operation(state, prev_state, {:set_path, _pid, _path}) do + defp after_operation(state, prev_state, {:set_file, _pid, _file}) do prev_images_dir = images_dir_from_state(prev_state) - if prev_state.data.path do + if prev_state.data.file do copy_images(state, prev_images_dir) else move_images(state, prev_images_dir) end + |> case do + :ok -> + :ok + + {:error, message} -> + broadcast_error(state.session_id, "failed to copy images - #{message}") + end state end + defp after_operation( + state, + _prev_state, + {:set_notebook_attributes, _client_pid, %{autosave_interval_s: _}} + ) do + if ref = state.autosave_timer_ref do + Process.cancel_timer(ref) + end + + schedule_autosave(state) + end + defp after_operation(state, prev_state, {:client_join, _client_pid, user}) do unless Map.has_key?(prev_state.data.users_map, user.id) do Phoenix.PubSub.subscribe(Livebook.PubSub, "users:#{user.id}") @@ -832,7 +893,13 @@ defmodule Livebook.Session do end defp handle_action(state, {:start_evaluation, cell, section}) do - file = (state.data.path || "") <> "#cell" + path = + case state.data.file do + nil -> "" + file -> file.path + end + + file = path <> "#cell" opts = [file: file] locator = {container_ref_for_section(section), cell.id} @@ -877,26 +944,49 @@ defmodule Livebook.Session do Phoenix.PubSub.broadcast(Livebook.PubSub, "sessions:#{session_id}", message) end - defp maybe_save_notebook(state) do - if state.data.path != nil and state.data.dirty do + defp maybe_save_notebook_async(state) do + if should_save_notebook?(state) do + pid = self() + file = state.data.file content = LiveMarkdown.Export.notebook_to_markdown(state.data.notebook) - dir = Path.dirname(state.data.path) + {:ok, pid} = + Task.start(fn -> + result = FileSystem.File.write(file, content) + send(pid, {:save_finished, self(), result}) + end) - with :ok <- File.mkdir_p(dir), - :ok <- File.write(state.data.path, content) do - handle_operation(state, {:mark_as_not_dirty, self()}) - else - {:error, reason} -> - message = :file.format_error(reason) - broadcast_error(state.session_id, "failed to save notebook - #{message}") - state - end + %{state | save_task_pid: pid} + else + state + end + end + + defp maybe_save_notebook_sync(state) do + if should_save_notebook?(state) do + content = LiveMarkdown.Export.notebook_to_markdown(state.data.notebook) + result = FileSystem.File.write(state.data.file, content) + handle_save_finished(state, result) else state end end + defp should_save_notebook?(state) do + state.data.file != nil and state.data.dirty and state.save_task_pid == nil + end + + defp handle_save_finished(state, result) do + case result do + :ok -> + handle_operation(state, {:mark_as_not_dirty, self()}) + + {:error, message} -> + broadcast_error(state.session_id, "failed to save notebook - #{message}") + state + end + end + @doc """ Determines locator of the evaluation that the given cell depends on. diff --git a/lib/livebook/session/data.ex b/lib/livebook/session/data.ex index 071e449beec..8f0609ea613 100644 --- a/lib/livebook/session/data.ex +++ b/lib/livebook/session/data.ex @@ -16,8 +16,8 @@ defmodule Livebook.Session.Data do defstruct [ :notebook, - :origin_url, - :path, + :origin, + :file, :dirty, :section_infos, :cell_infos, @@ -27,15 +27,15 @@ defmodule Livebook.Session.Data do :users_map ] - alias Livebook.{Notebook, Delta, Runtime, JSInterop} + alias Livebook.{Notebook, Delta, Runtime, JSInterop, FileSystem} alias Livebook.Users.User alias Livebook.Notebook.{Cell, Section} alias Livebook.Utils.Graph @type t :: %__MODULE__{ notebook: Notebook.t(), - origin_url: String.t() | nil, - path: nil | String.t(), + origin: String.t() | nil, + file: FileSystem.File.t() | nil, dirty: boolean(), section_infos: %{Section.id() => section_info()}, cell_infos: %{Cell.id() => cell_info()}, @@ -121,7 +121,7 @@ defmodule Livebook.Session.Data do | {: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()} + | {:add_cell_evaluation_response, pid(), Cell.id(), term(), metadata :: map()} | {:bind_input, pid(), elixir_cell_id :: Cell.id(), input_cell_id :: Cell.id()} | {:reflect_main_evaluation_failure, pid()} | {:reflect_evaluation_failure, pid(), Section.id()} @@ -135,7 +135,8 @@ defmodule Livebook.Session.Data do | {:report_cell_revision, pid(), Cell.id(), cell_revision()} | {:set_cell_attributes, pid(), Cell.id(), map()} | {:set_runtime, pid(), Runtime.t() | nil} - | {:set_path, pid(), String.t() | nil} + | {:set_file, pid(), FileSystem.File.t() | nil} + | {:set_autosave_interval, pid(), non_neg_integer() | nil} | {:mark_as_not_dirty, pid()} @type action :: @@ -152,8 +153,8 @@ defmodule Livebook.Session.Data do def new(notebook \\ Notebook.new()) do %__MODULE__{ notebook: notebook, - origin_url: nil, - path: nil, + origin: nil, + file: nil, dirty: false, section_infos: initial_section_infos(notebook), cell_infos: initial_cell_infos(notebook), @@ -560,10 +561,10 @@ defmodule Livebook.Session.Data do |> wrap_ok() end - def apply_operation(data, {:set_path, _client_pid, path}) do + def apply_operation(data, {:set_file, _client_pid, file}) do data |> with_actions() - |> set!(path: path) + |> set!(file: file) |> set_dirty() |> wrap_ok() end diff --git a/lib/livebook/session/file_guard.ex b/lib/livebook/session/file_guard.ex index 9425ded5882..210788b5374 100644 --- a/lib/livebook/session/file_guard.ex +++ b/lib/livebook/session/file_guard.ex @@ -4,13 +4,15 @@ defmodule Livebook.Session.FileGuard do # Serves as a locking mechanism for notebook files. # # Every session process willing to persist notebook - # should turn to `FileGuard` to make sure the path + # should turn to `FileGuard` to make sure the file # is not already used by another session. use GenServer + alias Livebook.FileSystem + @type state :: %{ - path_with_owner_ref: %{String.t() => reference()} + file_with_owner_ref: %{FileSystem.File.t() => reference()} } @name __MODULE__ @@ -28,40 +30,40 @@ defmodule Livebook.Session.FileGuard do If the owner process dies the file is automatically unlocked. """ - @spec lock(String.t(), pid()) :: :ok | {:error, :already_in_use} - def lock(path, owner_pid) do - GenServer.call(@name, {:lock, path, owner_pid}) + @spec lock(FileSystem.File.t(), pid()) :: :ok | {:error, :already_in_use} + def lock(file, owner_pid) do + GenServer.call(@name, {:lock, file, owner_pid}) end @doc """ Unlocks the given file. """ - @spec unlock(String.t()) :: :ok - def unlock(path) do - GenServer.cast(@name, {:unlock, path}) + @spec unlock(FileSystem.File.t()) :: :ok + def unlock(file) do + GenServer.cast(@name, {:unlock, file}) end # Callbacks @impl true def init(_opts) do - {:ok, %{path_with_owner_ref: %{}}} + {:ok, %{file_with_owner_ref: %{}}} end @impl true - def handle_call({:lock, path, owner_pid}, _from, state) do - if Map.has_key?(state.path_with_owner_ref, path) do + def handle_call({:lock, file, owner_pid}, _from, state) do + if Map.has_key?(state.file_with_owner_ref, file) do {:reply, {:error, :already_in_use}, state} else monitor_ref = Process.monitor(owner_pid) - state = put_in(state.path_with_owner_ref[path], monitor_ref) + state = put_in(state.file_with_owner_ref[file], monitor_ref) {:reply, :ok, state} end end @impl true - def handle_cast({:unlock, path}, state) do - {maybe_ref, state} = pop_in(state.path_with_owner_ref[path]) + def handle_cast({:unlock, file}, state) do + {maybe_ref, state} = pop_in(state.file_with_owner_ref[file]) maybe_ref && Process.demonitor(maybe_ref, [:flush]) {:noreply, state} @@ -69,8 +71,8 @@ defmodule Livebook.Session.FileGuard do @impl true def handle_info({:DOWN, ref, :process, _, _}, state) do - {path, ^ref} = Enum.find(state.path_with_owner_ref, &(elem(&1, 1) == ref)) - {_, state} = pop_in(state.path_with_owner_ref[path]) + {file, ^ref} = Enum.find(state.file_with_owner_ref, &(elem(&1, 1) == ref)) + {_, state} = pop_in(state.file_with_owner_ref[file]) {:noreply, state} end end diff --git a/lib/livebook/utils.ex b/lib/livebook/utils.ex index d79ee77f3b7..b36f71abd75 100644 --- a/lib/livebook/utils.ex +++ b/lib/livebook/utils.ex @@ -156,6 +156,26 @@ defmodule Livebook.Utils do String.upcase(first) <> rest end + @doc """ + Changes the first letter in the given string to lower case. + + ## Examples + + iex> Livebook.Utils.downcase_first("Sippin tea") + "sippin tea" + + iex> Livebook.Utils.downcase_first("Short URL") + "short URL" + + iex> Livebook.Utils.downcase_first("") + "" + """ + @spec downcase_first(String.t()) :: String.t() + def downcase_first(string) do + {first, rest} = String.split_at(string, 1) + String.downcase(first) <> rest + end + @doc """ Expands a relative path in terms of the given URL. diff --git a/lib/livebook/utils/http.ex b/lib/livebook/utils/http.ex new file mode 100644 index 00000000000..355a3e60f2e --- /dev/null +++ b/lib/livebook/utils/http.ex @@ -0,0 +1,106 @@ +defmodule Livebook.Utils.HTTP do + @moduledoc false + + @type status :: non_neg_integer() + @type headers :: list(header()) + @type header :: {String.t(), String.t()} + + @doc """ + Retrieves the header value from response headers. + """ + @spec fetch_header(headers(), String.t()) :: {:ok, String.t()} | :error + def fetch_header(headers, key) do + case Enum.find(headers, &match?({^key, _}, &1)) do + {_, value} -> {:ok, value} + _ -> :error + end + end + + @doc """ + Retrieves content type from response headers. + """ + @spec fetch_content_type(headers()) :: {:ok, String.t()} | :error + def fetch_content_type(headers) do + with {:ok, value} <- fetch_header(headers, "content-type") do + {:ok, value |> String.split(";") |> hd()} + end + end + + @doc """ + Makes an HTTP request. + + ## Options + + * `headers` - request headers, defaults to `[]` + + * `body` - request body given as `{content_type, body}` + + * `timeout` - request timeout, defaults to 10 seconds + """ + @spec request(atom(), String.t(), keyword()) :: + {:ok, status(), headers(), binary()} | {:error, term()} + def request(method, url, opts \\ []) + when is_atom(method) and is_binary(url) and is_list(opts) do + headers = build_headers(opts[:headers] || []) + + request = + case opts[:body] do + nil -> {url, headers} + {content_type, body} -> {url, headers, to_charlist(content_type), body} + end + + http_opts = [ + ssl: http_ssl_opts(), + timeout: opts[:timeout] || 10_000 + ] + + opts = [ + body_format: :binary + ] + + case :httpc.request(method, request, http_opts, opts) do + {:ok, {{_, status, _}, headers, body}} -> + {:ok, status, parse_headers(headers), body} + + {:error, error} -> + {:error, error} + end + end + + defp build_headers(entries) do + headers = + Enum.map(entries, fn {key, value} -> + {to_charlist(key), to_charlist(value)} + end) + + [{'user-agent', 'livebook'} | headers] + end + + defp parse_headers(headers) do + Enum.map(headers, fn {key, val} -> + {String.downcase(to_string(key)), to_string(val)} + end) + end + + # Load SSL certificates + + crt_file = CAStore.file_path() + crt = File.read!(crt_file) + pems = :public_key.pem_decode(crt) + ders = Enum.map(pems, fn {:Certificate, der, _} -> der end) + + # Note: we need to load the certificates at compilation time, + # as we don't have access to package files in Escript. + @cacerts ders + + defp http_ssl_opts() do + # Use secure options, see https://gist.github.com/jonatanklosko/5e20ca84127f6b31bbe3906498e1a1d7 + [ + verify: :verify_peer, + cacerts: @cacerts, + customize_hostname_check: [ + match_fun: :public_key.pkix_verify_hostname_match_fun(:https) + ] + ] + end +end diff --git a/lib/livebook_cli/server.ex b/lib/livebook_cli/server.ex index dda208e5d65..25f7588bcbf 100644 --- a/lib/livebook_cli/server.ex +++ b/lib/livebook_cli/server.ex @@ -134,8 +134,12 @@ defmodule LivebookCLI.Server do end defp opts_to_config([{:root_path, root_path} | opts], config) do - root_path = Livebook.Config.root_path!("--root-path", root_path) - opts_to_config(opts, [{:livebook, :root_path, root_path} | config]) + root_path = + Livebook.Config.root_path!("--root-path", root_path) + |> Livebook.FileSystem.Utils.ensure_dir_path() + + local_file_system = Livebook.FileSystem.Local.new(default_path: root_path) + opts_to_config(opts, [{:livebook, :file_systems, [local_file_system]} | config]) end defp opts_to_config([{:sname, sname} | opts], config) do diff --git a/lib/livebook_web/controllers/session_controller.ex b/lib/livebook_web/controllers/session_controller.ex index 77d1b1738fc..c70f862a8e3 100644 --- a/lib/livebook_web/controllers/session_controller.ex +++ b/lib/livebook_web/controllers/session_controller.ex @@ -1,16 +1,15 @@ defmodule LivebookWeb.SessionController do use LivebookWeb, :controller - alias Livebook.{SessionSupervisor, Session} + alias Livebook.{SessionSupervisor, Session, FileSystem} def show_image(conn, %{"id" => id, "image" => image}) do - with true <- SessionSupervisor.session_exists?(id), - %{images_dir: images_dir} <- Session.get_summary(id), - path <- Path.join(images_dir, image), - true <- File.exists?(path) do - serve_static(conn, path) + if SessionSupervisor.session_exists?(id) do + %{images_dir: images_dir} = Session.get_summary(id) + file = FileSystem.File.relative(images_dir, image) + serve_static(conn, file) else - _ -> send_resp(conn, 404, "Not found") + send_resp(conn, 404, "Not found") end end @@ -47,39 +46,43 @@ defmodule LivebookWeb.SessionController do send_resp(conn, 400, "Invalid format, supported formats: livemd, exs") end - defp serve_static(conn, path) do - case put_cache_header(conn, path) do - {:stale, conn} -> - filename = Path.basename(path) - content_type = MIME.from_path(filename) + defp serve_static(conn, file) do + with {:ok, cache_state, conn} <- put_cache_header(conn, file), + {:ok, conn} <- serve_with_cache(conn, file, cache_state) do + conn + else + {:error, message} -> send_resp(conn, 404, Livebook.Utils.upcase_first(message)) + end + end + defp put_cache_header(conn, file) do + with {:ok, etag} <- FileSystem.File.etag_for(file) do + conn = conn - |> put_resp_header("content-type", content_type) - |> send_file(200, path) - - {:fresh, conn} -> - send_resp(conn, 304, "") + |> put_resp_header("cache-control", "public") + |> put_resp_header("etag", etag) + + if etag in get_req_header(conn, "if-none-match") do + {:ok, :fresh, conn} + else + {:ok, :stale, conn} + end end end - defp put_cache_header(conn, path) do - etag = etag_for_path(path) + defp serve_with_cache(conn, file, :stale) do + filename = FileSystem.File.name(file) + content_type = MIME.from_path(filename) - conn = + with {:ok, content} <- FileSystem.File.read(file) do conn - |> put_resp_header("cache-control", "public") - |> put_resp_header("etag", etag) - - if etag in get_req_header(conn, "if-none-match") do - {:fresh, conn} - else - {:stale, conn} + |> put_resp_header("content-type", content_type) + |> send_resp(200, content) + |> then(&{:ok, &1}) end end - defp etag_for_path(path) do - %{size: size, mtime: mtime} = File.stat!(path) - hash = {size, mtime} |> :erlang.phash2() |> Integer.to_string(16) - <> + defp serve_with_cache(conn, _file, :fresh) do + {:ok, send_resp(conn, 304, "")} end end diff --git a/lib/livebook_web/helpers.ex b/lib/livebook_web/helpers.ex index 7e78d5f000d..7a92f477ead 100644 --- a/lib/livebook_web/helpers.ex +++ b/lib/livebook_web/helpers.ex @@ -10,18 +10,33 @@ defmodule LivebookWeb.Helpers do the URL when the modal is closed. """ def live_modal(component, opts) do + {modal_opts, opts} = build_modal_opts(opts) + modal_opts = [{:render_spec, {:component, component, opts}} | modal_opts] + live_component(LivebookWeb.ModalComponent, modal_opts) + end + + @doc """ + Renders a live view inside the `Livebook.ModalComponent` component. + + See `live_modal/2` for more details. + """ + def live_modal(socket, live_view, opts) do + {modal_opts, opts} = build_modal_opts(opts) + modal_opts = [{:render_spec, {:live_view, socket, live_view, opts}} | modal_opts] + live_component(LivebookWeb.ModalComponent, modal_opts) + end + + defp build_modal_opts(opts) do path = Keyword.fetch!(opts, :return_to) - modal_class = Keyword.get(opts, :modal_class) + {modal_class, opts} = Keyword.pop(opts, :modal_class) modal_opts = [ id: "modal", return_to: path, - modal_class: modal_class, - component: component, - opts: opts + modal_class: modal_class ] - live_component(LivebookWeb.ModalComponent, modal_opts) + {modal_opts, opts} end @doc """ @@ -177,10 +192,11 @@ defmodule LivebookWeb.Helpers do assigns = assigns |> assign_new(:class, fn -> "" end) - |> assign(:attrs, assigns_to_attributes(assigns, [:active, :class])) + |> assign_new(:disabled, fn -> assigns.active end) + |> assign(:attrs, assigns_to_attributes(assigns, [:active, :class, :disabled])) ~H""" - """ @@ -208,6 +224,26 @@ defmodule LivebookWeb.Helpers do """ end + @doc """ + Renders text with a tiny label. + + ## Examples + + <.labeled_text label="Name" text="Sherlock Holmes" /> + """ + def labeled_text(assigns) do + ~H""" +
+ + <%= @label %> + + + <%= @text %> + +
+ """ + end + defdelegate ansi_string_to_html(string), to: LivebookWeb.Helpers.ANSI defdelegate ansi_string_to_html_lines(string), to: LivebookWeb.Helpers.ANSI end diff --git a/lib/livebook_web/live/explore_live.ex b/lib/livebook_web/live/explore_live.ex index ea05c9a8386..e5f3f57fe32 100644 --- a/lib/livebook_web/live/explore_live.ex +++ b/lib/livebook_web/live/explore_live.ex @@ -4,7 +4,7 @@ defmodule LivebookWeb.ExploreLive do import LivebookWeb.UserHelpers import LivebookWeb.SessionHelpers - alias LivebookWeb.{SidebarHelpers, ExploreHelpers} + alias LivebookWeb.{SidebarHelpers, ExploreHelpers, PageHelpers} alias Livebook.Notebook.Explore @impl true @@ -32,20 +32,17 @@ defmodule LivebookWeb.ExploreLive do +
-
- <%= live_patch to: Routes.home_path(@socket, :page), - class: "hidden md:block absolute top-[50%] left-[-12px] transform -translate-y-1/2 -translate-x-full" do %> - <.remix_icon icon="arrow-left-line" class="text-2xl align-middle" /> - <% end %> -

- Explore -

-
+

Check out a number of examples showcasing various parts of the Elixir ecosystem. Click on any notebook you like and start playing around with it! diff --git a/lib/livebook_web/live/file_select_component.ex b/lib/livebook_web/live/file_select_component.ex new file mode 100644 index 00000000000..a24f3497e44 --- /dev/null +++ b/lib/livebook_web/live/file_select_component.ex @@ -0,0 +1,563 @@ +defmodule LivebookWeb.FileSelectComponent do + use LivebookWeb, :live_component + + # The component expects: + # + # * `file` - the currently entered file + # + # * `running_files` - the list of notebook files that are already + # linked to running sessions + # + # * `extnames` - a list of file extensions that should be shown + # + # * `submit_event` - the process event sent on form submission, + # use `nil` for no action + # + # The parent live view receives a `{:set_file, file, %{exists: boolean()}}` + # message whenever the file changes. + # + # Optionally inner block may be passed (e.g. with action buttons) + # and it's rendered next to the text input. + # + # To force the component to refetch the displayed files you can + # `send_update` with `force_reload: true` to the component. + + alias Livebook.FileSystem + + @impl true + def mount(socket) do + {:ok, + socket + |> assign_new(:inner_block, fn -> nil end) + |> assign( + # Component default attribute values + inner_block: nil, + file_system_select_disabled: false, + submit_event: nil, + # State + current_dir: nil, + new_dir_name: nil, + deleting_file: nil, + renaming_file: nil, + renamed_name: nil, + error_message: nil, + file_systems: Livebook.Config.file_systems() + )} + end + + @impl true + def update(assigns, socket) do + {force_reload?, assigns} = Map.pop(assigns, :force_reload, false) + + socket = + socket + |> assign(assigns) + |> update_file_infos(force_reload?) + + {:ok, socket} + end + + @impl true + def render(assigns) do + ~H""" +

+
+
+ <.file_system_menu_button + file={@file} + file_systems={@file_systems} + file_system_select_disabled={@file_system_select_disabled} + socket={@socket} + myself={@myself} /> +
+ +
+
+
+ +
+ +
+
+ <%= if @inner_block do %> +
+ <%= render_block(@inner_block) %> +
+ <% end %> +
+
+ <%= if @error_message do %> +
+ <%= @error_message %> + +
+ <% end %> + <%= if @deleting_file do %> +
+

+ Are you sure you want to irreversibly delete + <%= @deleting_file.path %>? +

+
+ + +
+
+ <% end %> +
+
+ <%= if @new_dir_name do %> +
+
+ + <.remix_icon icon="folder-add-fill" class="text-xl align-middle text-gray-400" /> + + +
+ +
+ +
+
+ <% end %> + + <%= if any_highlighted?(@file_infos) do %> +
+ <%= for file_info <- @file_infos, file_info.highlighted != "" do %> + <.file + file_info={file_info} + myself={@myself} + renaming_file={@renaming_file} + renamed_name={@renamed_name} /> + <% end %> +
+ <% end %> + +
+ <%= for file_info <- @file_infos, file_info.highlighted == "" do %> + <.file + file_info={file_info} + myself={@myself} + renaming_file={@renaming_file} + renamed_name={@renamed_name} /> + <% end %> +
+
+
+ """ + end + + defp any_highlighted?(file_infos) do + Enum.any?(file_infos, &(&1.highlighted != "")) + end + + defp file_system_menu_button(assigns) do + ~H""" +
+ +
+ <%= for {file_system, index} <- @file_systems |> Enum.with_index() do %> + <%= if file_system == @file.file_system do %> + + <% else %> + + <% end %> + <% end %> + <%= live_patch to: Routes.settings_path(@socket, :page), + class: "menu__item text-gray-500 border-t border-gray-200" do %> + <.remix_icon icon="settings-3-line" /> + Configure + <% end %> +
+
+ """ + end + + defp file_system_icon(%{file_system: %FileSystem.Local{}} = assigns) do + ~H""" + <.remix_icon icon="hard-drive-2-line" /> + """ + end + + defp file_system_icon(%{file_system: %FileSystem.S3{}} = assigns) do + ~H""" + + S3 + + """ + end + + defp file_system_label(%FileSystem.Local{}), do: "Local disk" + defp file_system_label(%FileSystem.S3{} = fs), do: fs.bucket_url + + defp file(%{file_info: %{file: file}, renaming_file: file} = assigns) do + ~H""" +
+ + <.remix_icon icon="edit-line" class="text-xl align-middle text-gray-400" /> + + +
+ +
+ +
+ """ + end + + defp file(assigns) do + icon = + case assigns.file_info do + %{is_running: true} -> "play-circle-line" + %{is_dir: true} -> "folder-fill" + _ -> "file-code-line" + end + + assigns = assign(assigns, :icon, icon) + + ~H""" +
+ +
+ <%= if @file_info.editable do %> + + + <% end %> +
+
+ """ + end + + @impl true + def handle_event("set_file_system", %{"index" => index}, socket) do + index = String.to_integer(index) + file_system = Enum.at(socket.assigns.file_systems, index) + file = FileSystem.File.new(file_system) + + send(self(), {:set_file, file, %{exists: true}}) + + {:noreply, socket} + end + + def handle_event("set_path", %{"path" => path}, socket) do + file = FileSystem.File.new(socket.assigns.file.file_system) |> FileSystem.File.relative(path) + + info = + socket.assigns.file_infos + |> Enum.find(&(&1.file.path == path)) + |> case do + nil -> %{exists: false} + _info -> %{exists: true} + end + + send(self(), {:set_file, file, info}) + + {:noreply, socket} + end + + def handle_event("submit", %{}, socket) do + if submit_event = socket.assigns.submit_event do + send(self(), submit_event) + end + + {:noreply, socket} + end + + def handle_event("clear_error", %{}, socket) do + {:noreply, put_error(socket, nil)} + end + + def handle_event("new_dir", %{}, socket) do + {:noreply, assign(socket, new_dir_name: "")} + end + + def handle_event("cancel_new_dir", %{}, socket) do + {:noreply, assign(socket, new_dir_name: nil)} + end + + def handle_event("create_dir", %{"value" => name}, socket) do + socket = + case create_dir(socket.assigns.current_dir, name) do + :ok -> + socket + |> assign(new_dir_name: nil) + |> update_file_infos(true) + + {:error, error} -> + socket + |> assign(new_dir_name: name) + |> put_error(error) + end + + {:noreply, socket} + end + + def handle_event("delete_file", %{"path" => path}, socket) do + %{file: file} = Enum.find(socket.assigns.file_infos, &(&1.file.path == path)) + {:noreply, assign(socket, deleting_file: file)} + end + + def handle_event("cancel_delete_file", %{}, socket) do + {:noreply, assign(socket, deleting_file: nil)} + end + + def handle_event("do_delete_file", %{}, socket) do + socket = + case delete_file(socket.assigns.deleting_file) do + :ok -> + socket + |> assign(deleting_file: nil) + |> update_file_infos(true) + + {:error, error} -> + put_error(socket, error) + end + + {:noreply, socket} + end + + def handle_event("rename_file", %{"path" => path}, socket) do + file_info = Enum.find(socket.assigns.file_infos, &(&1.file.path == path)) + {:noreply, assign(socket, renaming_file: file_info.file, renamed_name: file_info.name)} + end + + def handle_event("cancel_rename_file", %{}, socket) do + {:noreply, assign(socket, renaming_file: nil)} + end + + def handle_event("do_rename_file", %{"value" => name}, socket) do + socket = + if renaming_file = socket.assigns.renaming_file do + case rename_file(renaming_file, name) do + :ok -> + socket + |> assign(renaming_file: nil) + |> update_file_infos(true) + + {:error, error} -> + socket + |> assign(renamed_name: name) + |> put_error(error) + end + else + socket + end + + {:noreply, socket} + end + + defp update_file_infos(%{assigns: assigns} = socket, force_reload?) do + current_file_infos = assigns[:file_infos] || [] + {dir, prefix} = dir_and_prefix(assigns.file) + + {file_infos, socket} = + if dir != assigns.current_dir or force_reload? do + case get_file_infos(dir, assigns.extnames, assigns.running_files) do + {:ok, file_infos} -> + {file_infos, assign(socket, :current_dir, dir)} + + {:error, error} -> + {current_file_infos, put_error(socket, error)} + end + else + {current_file_infos, socket} + end + + assign(socket, :file_infos, annotate_matching(file_infos, prefix)) + end + + defp annotate_matching(file_infos, prefix) do + for %{name: name} = info <- file_infos do + if String.starts_with?(name, prefix) do + %{info | highlighted: prefix, unhighlighted: String.replace_prefix(name, prefix, "")} + else + %{info | highlighted: "", unhighlighted: name} + end + end + end + + # Phrase after the last slash is used as a search prefix within + # the given directory. + # + # Given "/foo/bar", we use "bar" to filter files within "/foo/". + # Given "/foo/bar/", we use "" to filter files within "/foo/bar/". + defp dir_and_prefix(file) do + if FileSystem.File.dir?(file) do + {file, ""} + else + {FileSystem.File.containing_dir(file), FileSystem.File.name(file)} + end + end + + defp get_file_infos(dir, extnames, running_files) do + with {:ok, files} <- FileSystem.File.list(dir) do + file_infos = + files + |> Enum.map(fn file -> + name = FileSystem.File.name(file) + file_info(file, name, running_files) + end) + |> Enum.filter(fn info -> + not hidden?(info.name) and (info.is_dir or valid_extension?(info.name, extnames)) + end) + |> Kernel.++( + case FileSystem.File.containing_dir(dir) do + ^dir -> [] + parent -> [file_info(parent, "..", running_files, editable: false)] + end + ) + |> Enum.sort_by(fn file -> {!file.is_dir, file.name} end) + + {:ok, file_infos} + end + end + + defp file_info(file, name, running_files, opts \\ []) do + %{ + name: name, + highlighted: "", + unhighlighted: name, + file: file, + is_dir: FileSystem.File.dir?(file), + is_running: file in running_files, + editable: Keyword.get(opts, :editable, true) + } + end + + defp hidden?(filename) do + String.starts_with?(filename, ".") + end + + defp valid_extension?(filename, extnames) do + Path.extname(filename) in extnames + end + + defp put_error(socket, nil) do + assign(socket, :error_message, nil) + end + + defp put_error(socket, :ignore) do + socket + end + + defp put_error(socket, message) when is_binary(message) do + assign(socket, :error_message, Livebook.Utils.upcase_first(message)) + end + + defp create_dir(_parent_dir, ""), do: {:error, :ignore} + + defp create_dir(parent_dir, name) do + new_dir = FileSystem.File.relative(parent_dir, name <> "/") + FileSystem.File.create_dir(new_dir) + end + + defp delete_file(file) do + FileSystem.File.remove(file) + end + + defp rename_file(_file, ""), do: {:error, :ignore} + + defp rename_file(file, name) do + parent_dir = FileSystem.File.containing_dir(file) + new_name = if FileSystem.File.dir?(file), do: name <> "/", else: name + new_file = FileSystem.File.relative(parent_dir, new_name) + FileSystem.File.rename(file, new_file) + end +end diff --git a/lib/livebook_web/live/home_live.ex b/lib/livebook_web/live/home_live.ex index 8d18ccbc5c9..8775cf394a6 100644 --- a/lib/livebook_web/live/home_live.ex +++ b/lib/livebook_web/live/home_live.ex @@ -5,7 +5,7 @@ defmodule LivebookWeb.HomeLive do import LivebookWeb.SessionHelpers alias LivebookWeb.{SidebarHelpers, ExploreHelpers} - alias Livebook.{SessionSupervisor, Session, LiveMarkdown, Notebook} + alias Livebook.{SessionSupervisor, Session, LiveMarkdown, Notebook, FileSystem} @impl true def mount(_params, %{"current_user_id" => current_user_id} = session, socket) do @@ -21,7 +21,8 @@ defmodule LivebookWeb.HomeLive do {:ok, assign(socket, current_user: current_user, - path: default_path(), + file: Livebook.Config.default_dir(), + file_info: %{exists: true, access: :read_write}, session_summaries: session_summaries, notebook_infos: notebook_infos )} @@ -33,6 +34,11 @@ defmodule LivebookWeb.HomeLive do
+
@@ -53,29 +59,27 @@ defmodule LivebookWeb.HomeLive do
- <%= live_component LivebookWeb.PathSelectComponent, - id: "path_select", - path: @path, + <%= live_component LivebookWeb.FileSelectComponent, + id: "home-file-select", + file: @file, extnames: [LiveMarkdown.extension()], - running_paths: paths(@session_summaries), - phx_target: nil, - phx_submit: nil do %> + running_files: files(@session_summaries) do %>
- <%= if path_running?(@path, @session_summaries) do %> + <%= if file_running?(@file, @session_summaries) do %> <%= live_redirect "Join session", - to: Routes.session_path(@socket, :page, session_id_by_path(@path, @session_summaries)), + to: Routes.session_path(@socket, :page, session_id_by_file(@file, @session_summaries)), class: "button button-blue" %> <% else %> - + @@ -140,8 +144,8 @@ defmodule LivebookWeb.HomeLive do """ end - defp open_button_tooltip_attrs(path) do - if File.regular?(path) and not file_writable?(path) do + defp open_button_tooltip_attrs(file, file_info) do + if regular?(file, file_info) and not writable?(file_info) do [class: "tooltip top", aria_label: "This file is write-protected, please fork instead"] else [] @@ -174,7 +178,7 @@ defmodule LivebookWeb.HomeLive do to: Routes.session_path(@socket, :page, summary.session_id), class: "font-semibold text-gray-800 hover:text-gray-900" %>
- <%= summary.path || "No file" %> + <%= if summary.file, do: summary.file.path, else: "No file" %>
@@ -220,36 +224,49 @@ defmodule LivebookWeb.HomeLive do def handle_params(_params, _url, socket), do: {:noreply, socket} @impl true - def handle_event("set_path", %{"path" => path}, socket) do - {:noreply, assign(socket, path: path)} - end - def handle_event("new", %{}, socket) do {:noreply, create_session(socket)} end def handle_event("fork", %{}, socket) do - path = socket.assigns.path - {notebook, messages} = import_notebook(path) - socket = put_import_flash_messages(socket, messages) - notebook = Notebook.forked(notebook) - images_dir = Session.images_dir_for_notebook(path) + file = socket.assigns.file + + socket = + case import_notebook(file) do + {:ok, {notebook, messages}} -> + notebook = Notebook.forked(notebook) + images_dir = Session.images_dir_for_notebook(file) + + socket + |> put_import_flash_messages(messages) + |> create_session( + notebook: notebook, + copy_images_from: images_dir, + origin: {:file, file} + ) + + {:error, error} -> + put_flash(socket, :error, Livebook.Utils.upcase_first(error)) + end - {:noreply, - create_session(socket, - notebook: notebook, - copy_images_from: images_dir, - origin_url: "file://" <> path - )} + {:noreply, socket} end def handle_event("open", %{}, socket) do - path = socket.assigns.path - {notebook, messages} = import_notebook(path) - socket = put_import_flash_messages(socket, messages) + file = socket.assigns.file - {:noreply, - create_session(socket, notebook: notebook, path: path, origin_url: "file://" <> path)} + socket = + case import_notebook(file) do + {:ok, {notebook, messages}} -> + socket + |> put_import_flash_messages(messages) + |> create_session(notebook: notebook, file: file, origin: {:file, file}) + + {:error, error} -> + put_flash(socket, :error, Livebook.Utils.upcase_first(error)) + end + + {:noreply, socket} end def handle_event("fork_session", %{"id" => session_id}, socket) do @@ -257,22 +274,35 @@ defmodule LivebookWeb.HomeLive do notebook = Notebook.forked(data.notebook) %{images_dir: images_dir} = Session.get_summary(session_id) - origin_url = - if data.path do - "file://" <> data.path + origin = + if data.file do + {:file, data.file} else - data.origin_url + data.origin end {:noreply, create_session(socket, notebook: notebook, copy_images_from: images_dir, - origin_url: origin_url + origin: origin )} end @impl true + def handle_info({:set_file, file, info}, socket) do + file_info = %{ + exists: info.exists, + access: + case FileSystem.File.access(file) do + {:ok, access} -> access + {:error, _} -> :none + end + } + + {:noreply, assign(socket, file: file, file_info: file_info)} + end + def handle_info({:session_created, id}, socket) do summary = Session.get_summary(id) session_summaries = sort_session_summaries([summary | socket.assigns.session_summaries]) @@ -300,43 +330,44 @@ defmodule LivebookWeb.HomeLive do def handle_info(_message, socket), do: {:noreply, socket} - defp default_path(), do: Livebook.Config.root_path() <> "/" - defp sort_session_summaries(session_summaries) do Enum.sort_by(session_summaries, & &1.notebook_name) end - defp paths(session_summaries) do - Enum.map(session_summaries, & &1.path) + defp files(session_summaries) do + Enum.map(session_summaries, & &1.file) end - defp path_forkable?(path) do - File.regular?(path) + defp path_forkable?(file, file_info) do + regular?(file, file_info) end - defp path_openable?(path, session_summaries) do - File.regular?(path) and not path_running?(path, session_summaries) and file_writable?(path) + defp path_openable?(file, file_info, session_summaries) do + regular?(file, file_info) and not file_running?(file, session_summaries) and + writable?(file_info) end - defp path_running?(path, session_summaries) do - running_paths = paths(session_summaries) - path in running_paths + defp regular?(file, file_info) do + file_info.exists and not FileSystem.File.dir?(file) end - defp file_writable?(path) do - case File.stat(path) do - {:ok, stat} -> stat.access in [:read_write, :write] - {:error, _} -> false - end + defp writable?(file_info) do + file_info.access in [:read_write, :write] end - defp import_notebook(path) do - content = File.read!(path) - LiveMarkdown.Import.notebook_from_markdown(content) + defp file_running?(file, session_summaries) do + running_files = files(session_summaries) + file in running_files + end + + defp import_notebook(file) do + with {:ok, content} <- FileSystem.File.read(file) do + {:ok, LiveMarkdown.Import.notebook_from_markdown(content)} + end end - defp session_id_by_path(path, session_summaries) do - summary = Enum.find(session_summaries, &(&1.path == path)) + defp session_id_by_file(file, session_summaries) do + summary = Enum.find(session_summaries, &(&1.file == file)) summary.session_id end end diff --git a/lib/livebook_web/live/home_live/import_url_component.ex b/lib/livebook_web/live/home_live/import_url_component.ex index ac7e1eed079..73e6f94eabf 100644 --- a/lib/livebook_web/live/home_live/import_url_component.ex +++ b/lib/livebook_web/live/home_live/import_url_component.ex @@ -50,7 +50,7 @@ defmodule LivebookWeb.HomeLive.ImportUrlComponent do |> ContentLoader.fetch_content() |> case do {:ok, content} -> - send(self(), {:import_content, content, [origin_url: url]}) + send(self(), {:import_content, content, [origin: {:url, url}]}) {:noreply, socket} {:error, message} -> diff --git a/lib/livebook_web/live/modal_component.ex b/lib/livebook_web/live/modal_component.ex index 25578ba59c8..a7193daf2b2 100644 --- a/lib/livebook_web/live/modal_component.ex +++ b/lib/livebook_web/live/modal_component.ex @@ -27,7 +27,12 @@ defmodule LivebookWeb.ModalComponent do <.remix_icon icon="close-line" class="text-2xl" /> <% end %> - <%= live_component @component, @opts %> + <%= + case @render_spec do + {:component, component, opts} -> live_component(component, opts) + {:live_view, socket, live_view, opts} -> live_render(socket, live_view, opts) + end + %>
diff --git a/lib/livebook_web/live/page_helpers.ex b/lib/livebook_web/live/page_helpers.ex new file mode 100644 index 00000000000..9c5c4ba6acd --- /dev/null +++ b/lib/livebook_web/live/page_helpers.ex @@ -0,0 +1,28 @@ +defmodule LivebookWeb.PageHelpers do + use Phoenix.Component + + import LivebookWeb.Helpers + + alias LivebookWeb.Router.Helpers, as: Routes + + @doc """ + Renders page title. + + ## Examples + + <.user_avatar user={@user} class="h-20 w-20" text_class="text-3xl" /> + """ + def title(assigns) do + ~H""" +
+ <%= live_patch to: Routes.home_path(@socket, :page), + class: "hidden md:block absolute top-[50%] left-[-12px] transform -translate-y-1/2 -translate-x-full" do %> + <.remix_icon icon="arrow-left-line" class="text-2xl align-middle" /> + <% end %> +

+ <%= @text %> +

+
+ """ + end +end diff --git a/lib/livebook_web/live/path_select_component.ex b/lib/livebook_web/live/path_select_component.ex deleted file mode 100644 index 0be6018dab2..00000000000 --- a/lib/livebook_web/live/path_select_component.ex +++ /dev/null @@ -1,437 +0,0 @@ -defmodule LivebookWeb.PathSelectComponent do - use LivebookWeb, :live_component - - # The component expects: - # - # * `path` - the currently entered path - # * `running_paths` - the list of notebook paths that are already linked to running sessions - # * `extnames` - a list of file extensions that should be shown - # * `phx_target` - id of the component to send update events to or nil to send to the parent LV - # * `phx_submit` - the event name sent on form submission, use `nil` for no action - # - # The target receives `set_path` events with `%{"path" => path}` payload. - # - # Optionally inner block may be passed (e.g. with action buttons) - # and it's rendered next to the text input. - # - # To force the component refetch the displayed files - # you can `send_update` with `force_reload: true` to the component. - - @impl true - def mount(socket) do - {:ok, - socket - |> assign_new(:inner_block, fn -> nil end) - |> assign( - current_dir: nil, - new_directory_name: nil, - deleting_path: nil, - renaming_path: nil, - renamed_name: "" - )} - end - - @impl true - def update(assigns, socket) do - {force_reload?, assigns} = Map.pop(assigns, :force_reload, false) - - socket = - socket - |> assign(assigns) - |> update_files(force_reload?) - - {:ok, socket} - end - - defp update_files(%{assigns: assigns} = socket, force_reload?) do - {dir, basename} = split_path(assigns.path) - dir = Path.expand(dir) - - files = - if assigns.current_dir != dir or force_reload? do - list_files(dir, assigns.extnames, assigns.running_paths) - else - assigns.files - end - - assign(socket, files: annotate_matching(files, basename), current_dir: dir) - end - - @impl true - def render(assigns) do - ~H""" -
-
-
- -
-
- -
- -
-
- <%= if @inner_block do %> -
- <%= render_block(@inner_block) %> -
- <% end %> -
- <%= if @deleting_path do %> -
-

- Are you sure you want to irreversibly delete - <%= @deleting_path %>? -

-
- - -
-
- <% end %> -
- <%= if @new_directory_name do %> -
-
- - <.remix_icon icon="folder-add-fill" class="text-xl align-middle text-gray-400" /> - - -
- -
- -
-
- <% end %> - - <%= if highlighting?(@files) do %> -
- <%= for file <- @files, file.highlighted != "" do %> - <.file - file={file} - phx_target={@phx_target} - myself={@myself} - renaming_path={@renaming_path} - renamed_name={@renamed_name} /> - <% end %> -
- <% end %> - -
- <%= for file <- @files, file.highlighted == "" do %> - <.file - file={file} - phx_target={@phx_target} - myself={@myself} - renaming_path={@renaming_path} - renamed_name={@renamed_name} /> - <% end %> -
-
-
- """ - end - - defp highlighting?(files) do - Enum.any?(files, &(&1.highlighted != "")) - end - - defp file(%{file: %{path: path}, renaming_path: path} = assigns) do - ~H""" -
- - <.remix_icon icon="edit-line" class="text-xl align-middle text-gray-400" /> - - -
- -
- -
- """ - end - - defp file(assigns) do - icon = - case assigns.file do - %{is_running: true} -> "play-circle-line" - %{is_dir: true} -> "folder-fill" - _ -> "file-code-line" - end - - assigns = assign(assigns, :icon, icon) - - ~H""" -
- -
- - -
-
- """ - end - - defp annotate_matching(files, prefix) do - for %{name: name} = file <- files do - if String.starts_with?(name, prefix) do - %{file | highlighted: prefix, unhighlighted: String.replace_prefix(name, prefix, "")} - else - %{file | highlighted: "", unhighlighted: name} - end - end - end - - defp list_files(dir, extnames, running_paths) do - if File.exists?(dir) do - file_names = - case File.ls(dir) do - {:ok, names} -> names - {:error, _} -> [] - end - - file_infos = - file_names - |> Enum.map(fn name -> - file_info(name, Path.join(dir, name), running_paths) - end) - |> Enum.filter(fn file -> - not hidden?(file.name) and (file.is_dir or valid_extension?(file.name, extnames)) - end) - - parent = Path.dirname(dir) - - file_infos = - if parent == dir do - file_infos - else - [file_info("..", parent, running_paths) | file_infos] - end - - Enum.sort_by(file_infos, fn file -> {!file.is_dir, file.name} end) - else - [] - end - end - - defp file_info(name, path, running_paths) do - is_dir = File.dir?(path) - - %{ - name: name, - highlighted: "", - unhighlighted: name, - path: if(is_dir, do: ensure_trailing_slash(path), else: path), - is_dir: is_dir, - is_running: path in running_paths - } - end - - defp hidden?(filename) do - String.starts_with?(filename, ".") - end - - defp valid_extension?(filename, extnames) do - Path.extname(filename) in extnames - end - - # Note: to provide an intuitive behavior when typing the path - # we enter a new directory when it has a trailing slash, - # so given "/foo/bar" we list files in "foo" and given "/foo/bar/ - # we list files in "bar". - # - # The basename is kinda like search within the current directory, - # so we highlight files starting with that string. - defp split_path(path) do - if String.ends_with?(path, "/") do - {path, ""} - else - {Path.dirname(path), Path.basename(path)} - end - end - - defp ensure_trailing_slash(path) do - if String.ends_with?(path, "/") do - path - else - path <> "/" - end - end - - @impl true - def handle_event("new_directory", %{}, socket) do - {:noreply, assign(socket, new_directory_name: "")} - end - - def handle_event("cancel_new_directory", %{}, socket) do - {:noreply, assign(socket, new_directory_name: nil)} - end - - def handle_event("create_directory", %{"value" => name}, socket) do - socket = - case create_directory(socket.assigns.current_dir, name) do - :ok -> - socket - |> assign(new_directory_name: nil) - |> update_files(true) - - _ -> - assign(socket, new_directory_name: name) - end - - {:noreply, socket} - end - - def handle_event("delete_file", %{"path" => path}, socket) do - {:noreply, assign(socket, deleting_path: path)} - end - - def handle_event("cancel_delete_file", %{}, socket) do - {:noreply, assign(socket, deleting_path: nil)} - end - - def handle_event("do_delete_file", %{}, socket) do - socket = - case delete_file(socket.assigns.deleting_path) do - :ok -> - socket - |> assign(deleting_path: nil) - |> update_files(true) - - _ -> - socket - end - - {:noreply, socket} - end - - def handle_event("rename_file", %{"path" => path}, socket) do - {_, name} = path |> Path.expand() |> split_path() - {:noreply, assign(socket, renaming_path: path, renamed_name: name)} - end - - def handle_event("cancel_rename_file", %{}, socket) do - {:noreply, assign(socket, renaming_path: nil)} - end - - def handle_event("do_rename_file", %{"value" => name}, socket) do - socket = - case rename_file(socket.assigns.renaming_path, name) do - :ok -> - socket - |> assign(renaming_path: nil) - |> update_files(true) - - _ -> - assign(socket, renamed_name: name) - end - - {:noreply, socket} - end - - defp create_directory(_parent_dir, ""), do: {:error, :empty} - - defp create_directory(parent_dir, name) do - new_dir = Path.join(parent_dir, name) - File.mkdir(new_dir) - end - - defp delete_file(path) do - with {:ok, _paths} <- File.rm_rf(path) do - :ok - end - end - - defp rename_file(_path, ""), do: {:error, :empty} - - defp rename_file(path, name) do - dir = path |> Path.expand() |> Path.dirname() - new_path = Path.join(dir, name) - File.rename(path, new_path) - end -end diff --git a/lib/livebook_web/live/session_helpers.ex b/lib/livebook_web/live/session_helpers.ex index 6a2e94827e8..0b6cbe51bcb 100644 --- a/lib/livebook_web/live/session_helpers.ex +++ b/lib/livebook_web/live/session_helpers.ex @@ -10,6 +10,15 @@ defmodule LivebookWeb.SessionHelpers do """ @spec create_session(Phoenix.LiveView.Socket.t(), keyword()) :: Phoenix.LiveView.Socket.t() def create_session(socket, opts \\ []) do + # Revert persistence options to default values if there is + # no file attached to the new session + opts = + if opts[:notebook] != nil and opts[:file] == nil do + Keyword.update!(opts, :notebook, &Livebook.Notebook.reset_persistence_options/1) + else + opts + end + case Livebook.SessionSupervisor.create_session(opts) do {:ok, id} -> push_redirect(socket, to: Routes.session_path(socket, :page, id)) diff --git a/lib/livebook_web/live/session_live.ex b/lib/livebook_web/live/session_live.ex index d6db314b023..e286b64c0b8 100644 --- a/lib/livebook_web/live/session_live.ex +++ b/lib/livebook_web/live/session_live.ex @@ -6,7 +6,7 @@ defmodule LivebookWeb.SessionLive do import Livebook.Utils, only: [access_by_id: 1] alias LivebookWeb.SidebarHelpers - alias Livebook.{SessionSupervisor, Session, Delta, Notebook, Runtime, LiveMarkdown} + alias Livebook.{SessionSupervisor, Session, Delta, Notebook, Runtime, LiveMarkdown, FileSystem} alias Livebook.Notebook.Cell alias Livebook.JSInterop @@ -238,8 +238,9 @@ defmodule LivebookWeb.SessionLive do
<%= live_component LivebookWeb.SessionLive.IndicatorsComponent, session_id: @session_id, - path: @data_view.path, + file: @data_view.file, dirty: @data_view.dirty, + autosave_interval_s: @data_view.autosave_interval_s, runtime: @data_view.runtime, global_evaluation_status: @data_view.global_evaluation_status %>
@@ -263,13 +264,16 @@ defmodule LivebookWeb.SessionLive do <% end %> <%= if @live_action == :file_settings do %> - <%= live_modal LivebookWeb.SessionLive.PersistenceComponent, + <%= live_modal @socket, LivebookWeb.SessionLive.PersistenceLive, id: "persistence", modal_class: "w-full max-w-4xl", return_to: Routes.session_path(@socket, :page, @session_id), - session_id: @session_id, - path: @data_view.path, - persist_outputs: @data_view.persist_outputs %> + session: %{ + "session_id" => @session_id, + "file" => @data_view.file, + "persist_outputs" => @data_view.persist_outputs, + "autosave_interval_s" => @data_view.autosave_interval_s + } %> <% end %> <%= if @live_action == :shortcuts do %> @@ -586,7 +590,7 @@ defmodule LivebookWeb.SessionLive do end def handle_event("save", %{}, socket) do - if socket.private.data.path do + if socket.private.data.file do Session.save(socket.assigns.session_id) {:noreply, socket} else @@ -800,9 +804,9 @@ defmodule LivebookWeb.SessionLive do end defp handle_relative_notebook_path(socket, relative_path) do - resolution_url = location(socket.private.data) + resolution_location = location(socket.private.data) - case resolution_url do + case resolution_location do nil -> socket |> put_flash( @@ -811,21 +815,31 @@ defmodule LivebookWeb.SessionLive do ) |> redirect_to_self() - url -> - target_url = Livebook.Utils.expand_url(url, relative_path) + resolution_location -> + origin = + case resolution_location do + {:url, url} -> {:url, Livebook.Utils.expand_url(url, relative_path)} + {:file, file} -> {:file, FileSystem.File.relative(file, relative_path)} + end - case session_id_by_url(target_url) do + case session_id_by_location(origin) do {:ok, session_id} -> push_redirect(socket, to: Routes.session_path(socket, :page, session_id)) {:error, :none} -> - open_notebook(socket, target_url) + open_notebook(socket, origin) {:error, :many} -> + origin_str = + case origin do + {:url, url} -> url + {:file, file} -> file.path + end + socket |> put_flash( :error, - "Cannot navigate, because multiple sessions were found for #{target_url}" + "Cannot navigate, because multiple sessions were found for #{origin_str}" ) |> redirect_to_self() end @@ -833,24 +847,21 @@ defmodule LivebookWeb.SessionLive do end defp location(data) - defp location(%{path: path}) when is_binary(path), do: "file://" <> path - defp location(%{origin_url: origin_url}), do: origin_url + defp location(%{file: file}) when is_map(file), do: {:file, file} + defp location(%{origin: origin}), do: origin - defp open_notebook(socket, url) do - url - |> Livebook.ContentLoader.rewrite_url() - |> Livebook.ContentLoader.fetch_content() - |> case do + defp open_notebook(socket, origin) do + case load_content(origin) do {:ok, content} -> {notebook, messages} = Livebook.LiveMarkdown.Import.notebook_from_markdown(content) # If the current session has no path, fork the notebook - fork? = socket.private.data.path == nil - {path, notebook} = path_and_notebook(fork?, url, notebook) + fork? = socket.private.data.file == nil + {file, notebook} = file_and_notebook(fork?, origin, notebook) socket |> put_import_flash_messages(messages) - |> create_session(notebook: notebook, origin_url: url, path: path) + |> create_session(notebook: notebook, origin: origin, file: file) {:error, message} -> socket @@ -859,26 +870,39 @@ defmodule LivebookWeb.SessionLive do end end - defp path_and_notebook(fork?, url, notebook) - defp path_and_notebook(false, "file://" <> path, notebook), do: {path, notebook} - defp path_and_notebook(true, "file://" <> _path, notebook), do: {nil, Notebook.forked(notebook)} - defp path_and_notebook(_fork?, _url, notebook), do: {nil, notebook} + defp load_content({:file, file}) do + case FileSystem.File.read(file) do + {:ok, content} -> {:ok, content} + {:error, message} -> {:error, "failed to read #{file.path}, reason: #{message}"} + end + end + + defp load_content({:url, url}) do + url + |> Livebook.ContentLoader.rewrite_url() + |> Livebook.ContentLoader.fetch_content() + end + + defp file_and_notebook(fork?, origin, notebook) + defp file_and_notebook(false, {:file, file}, notebook), do: {file, notebook} + defp file_and_notebook(true, {:file, _file}, notebook), do: {nil, Notebook.forked(notebook)} + defp file_and_notebook(_fork?, _origin, notebook), do: {nil, notebook} - defp session_id_by_url(url) do + defp session_id_by_location(location) do session_summaries = SessionSupervisor.get_session_summaries() - session_with_path = + session_with_file = Enum.find(session_summaries, fn summary -> - summary.path && "file://" <> summary.path == url + summary.file && {:file, summary.file} == location end) - # A session associated with the given path takes - # precedence over sessions originating from this path - if session_with_path do - {:ok, session_with_path.session_id} + # A session associated with the given file takes + # precedence over sessions originating from this file + if session_with_file do + {:ok, session_with_file.session_id} else session_summaries - |> Enum.filter(fn summary -> summary.origin_url == url end) + |> Enum.filter(fn summary -> summary.origin == location end) |> case do [summary] -> {:ok, summary.session_id} [] -> {:error, :none} @@ -1075,8 +1099,9 @@ defmodule LivebookWeb.SessionLive do # have to traverse the whole template tree and no diff is sent to the client. defp data_to_view(data) do %{ - path: data.path, + file: data.file, persist_outputs: data.notebook.persist_outputs, + autosave_interval_s: data.notebook.autosave_interval_s, dirty: data.dirty, runtime: data.runtime, global_evaluation_status: global_evaluation_status(data), diff --git a/lib/livebook_web/live/session_live/cell_component.ex b/lib/livebook_web/live/session_live/cell_component.ex index a78be570146..db6e3f35cd9 100644 --- a/lib/livebook_web/live/session_live/cell_component.ex +++ b/lib/livebook_web/live/session_live/cell_component.ex @@ -237,7 +237,7 @@ defmodule LivebookWeb.SessionLive.CellComponent do ~H""" <%= live_patch to: Routes.session_path(@socket, :cell_settings, @session_id, @cell_id), class: "icon-button" do %> - <.remix_icon icon="list-settings-line" class="text-xl" /> + <.remix_icon icon="settings-3-line" class="text-xl" /> <% end %> """ diff --git a/lib/livebook_web/live/session_live/cell_upload_component.ex b/lib/livebook_web/live/session_live/cell_upload_component.ex index daa650f40b9..624419ffeb4 100644 --- a/lib/livebook_web/live/session_live/cell_upload_component.ex +++ b/lib/livebook_web/live/session_live/cell_upload_component.ex @@ -1,11 +1,11 @@ defmodule LivebookWeb.SessionLive.CellUploadComponent do use LivebookWeb, :live_component - alias Livebook.Session + alias Livebook.{Session, FileSystem} @impl true def mount(socket) do - {:ok, assign(socket, name: "")} + {:ok, assign(socket, name: "", error_message: nil)} end @impl true @@ -20,6 +20,11 @@ defmodule LivebookWeb.SessionLive.CellUploadComponent do Invalid image file. The image must be either GIF, JPEG, or PNG and cannot exceed 5MB in size.
<% end %> + <%= if @error_message do %> +
+ <%= @error_message %> +
+ <% end %> <%= for entry <- @uploads.cell_image.entries do %>
@@ -67,22 +72,28 @@ defmodule LivebookWeb.SessionLive.CellUploadComponent do def handle_event("save", %{"name" => name}, socket) do %{images_dir: images_dir} = Session.get_summary(socket.assigns.session_id) - File.mkdir_p!(images_dir) - [filename] = - consume_uploaded_entries(socket, :cell_image, fn %{path: path}, entry -> - ext = Path.extname(entry.client_name) - filename = name <> ext - dest = Path.join(images_dir, filename) - File.cp!(path, dest) - filename - end) + consume_uploaded_entries(socket, :cell_image, fn %{path: path}, entry -> + upload_file = FileSystem.File.local(path) + ext = Path.extname(entry.client_name) + filename = name <> ext + destination_file = FileSystem.File.relative(images_dir, filename) + + with :ok <- FileSystem.File.copy(upload_file, destination_file) do + {:ok, filename} + end + end) + |> case do + [{:ok, filename}] -> + src_path = "images/#{filename}" - src_path = "images/#{filename}" + {:noreply, + socket + |> push_patch(to: socket.assigns.return_to) + |> push_event("cell_upload", %{cell_id: socket.assigns.cell.id, url: src_path})} - {:noreply, - socket - |> push_patch(to: socket.assigns.return_to) - |> push_event("cell_upload", %{cell_id: socket.assigns.cell.id, url: src_path})} + [{:error, message}] -> + {:noreply, assign(socket, error_message: message)} + end end end diff --git a/lib/livebook_web/live/session_live/indicators_component.ex b/lib/livebook_web/live/session_live/indicators_component.ex index f4bf685e354..4cb2967c01a 100644 --- a/lib/livebook_web/live/session_live/indicators_component.ex +++ b/lib/livebook_web/live/session_live/indicators_component.ex @@ -5,14 +5,23 @@ defmodule LivebookWeb.SessionLive.IndicatorsComponent do def render(assigns) do ~H"""
- <%= if @path do %> + <%= if @file do %> <%= if @dirty do %> - - <%= live_patch to: Routes.session_path(@socket, :file_settings, @session_id), - class: "icon-button icon-outlined-button border-blue-400 hover:bg-blue-50 focus:bg-blue-50" do %> - <.remix_icon icon="save-line" class="text-xl text-blue-500" /> - <% end %> - + <%= if @autosave_interval_s do %> + + <%= live_patch to: Routes.session_path(@socket, :file_settings, @session_id), + class: "icon-button icon-outlined-button border-blue-400 hover:bg-blue-50 focus:bg-blue-50" do %> + <.remix_icon icon="save-line" class="text-xl text-blue-500" /> + <% end %> + + <% else %> + + <%= live_patch to: Routes.session_path(@socket, :file_settings, @session_id), + class: "icon-button icon-outlined-button border-yellow-200 hover:bg-red-50 focus:bg-red-50" do %> + <.remix_icon icon="save-line" class="text-xl text-yellow-300" /> + <% end %> + + <% end %> <% else %> <%= live_patch to: Routes.session_path(@socket, :file_settings, @session_id), diff --git a/lib/livebook_web/live/session_live/mix_standalone_live.ex b/lib/livebook_web/live/session_live/mix_standalone_live.ex index 781e5cf1bc8..1f8c79f12a5 100644 --- a/lib/livebook_web/live/session_live/mix_standalone_live.ex +++ b/lib/livebook_web/live/session_live/mix_standalone_live.ex @@ -1,7 +1,7 @@ defmodule LivebookWeb.SessionLive.MixStandaloneLive do use LivebookWeb, :live_view - alias Livebook.{Session, Runtime, Utils} + alias Livebook.{Session, Runtime, Utils, FileSystem} @type status :: :initial | :initializing | :finished @@ -16,7 +16,7 @@ defmodule LivebookWeb.SessionLive.MixStandaloneLive do session_id: session_id, status: :initial, current_runtime: current_runtime, - path: initial_path(current_runtime), + file: initial_file(current_runtime), outputs: [], emitter: nil ), temporary_assigns: [outputs: []]} @@ -39,16 +39,16 @@ defmodule LivebookWeb.SessionLive.MixStandaloneLive do

<%= if @status == :initial do %>
- <%= live_component LivebookWeb.PathSelectComponent, - id: "path_select", - path: @path, + <%= live_component LivebookWeb.FileSelectComponent, + id: "mix-project-dir", + file: @file, extnames: [], - running_paths: [], - phx_target: nil, - phx_submit: if(disabled?(@path), do: nil, else: "init") %> + running_files: [], + submit_event: if(disabled?(@file.path), do: nil, else: :init), + file_system_select_disabled: true %>
- <% end %> <%= if @status != :initial do %> @@ -65,17 +65,19 @@ defmodule LivebookWeb.SessionLive.MixStandaloneLive do end @impl true - def handle_event("set_path", %{"path" => path}, socket) do - {:noreply, assign(socket, path: path)} - end - def handle_event("init", _params, socket) do - emitter = Utils.Emitter.new(self()) - Runtime.MixStandalone.init_async(socket.assigns.path, emitter) - {:noreply, assign(socket, status: :initializing, emitter: emitter)} + handle_init(socket) end @impl true + def handle_info({:set_file, file, _info}, socket) do + {:noreply, assign(socket, :file, file)} + end + + def handle_info(:init, socket) do + handle_init(socket) + end + def handle_info({:emitter, ref, message}, %{assigns: %{emitter: %{ref: ref}}} = socket) do case message do {:output, output} -> @@ -96,18 +98,24 @@ defmodule LivebookWeb.SessionLive.MixStandaloneLive do def handle_info(_, socket), do: {:noreply, socket} + defp handle_init(socket) do + emitter = Utils.Emitter.new(self()) + Runtime.MixStandalone.init_async(socket.assigns.file.path, emitter) + {:noreply, assign(socket, status: :initializing, emitter: emitter)} + end + defp add_output(socket, output) do assign(socket, outputs: socket.assigns.outputs ++ [{output, Utils.random_id()}]) end - defp initial_path(%Runtime.MixStandalone{} = current_runtime) do - current_runtime.project_path + defp initial_file(%Runtime.MixStandalone{} = current_runtime) do + FileSystem.File.local(current_runtime.project_path) end - defp initial_path(_runtime), do: File.cwd!() <> "/" - - defp mix_project_root?(path) do - File.dir?(path) and File.exists?(Path.join(path, "mix.exs")) + defp initial_file(_runtime) do + Livebook.Config.file_systems() + |> Enum.find(&is_struct(&1, FileSystem.Local)) + |> FileSystem.File.new() end defp matching_runtime?(%Runtime.MixStandalone{} = runtime, path) do @@ -119,4 +127,8 @@ defmodule LivebookWeb.SessionLive.MixStandaloneLive do defp disabled?(path) do not mix_project_root?(path) end + + defp mix_project_root?(path) do + File.dir?(path) and File.exists?(Path.join(path, "mix.exs")) + end end diff --git a/lib/livebook_web/live/session_live/persistence_component.ex b/lib/livebook_web/live/session_live/persistence_component.ex deleted file mode 100644 index e0e19a57368..00000000000 --- a/lib/livebook_web/live/session_live/persistence_component.ex +++ /dev/null @@ -1,179 +0,0 @@ -defmodule LivebookWeb.SessionLive.PersistenceComponent do - use LivebookWeb, :live_component - - alias Livebook.{Session, SessionSupervisor, LiveMarkdown} - - @impl true - def mount(socket) do - session_summaries = SessionSupervisor.get_session_summaries() - running_paths = Enum.map(session_summaries, & &1.path) - {:ok, assign(socket, running_paths: running_paths)} - end - - @impl true - def update(assigns, socket) do - {path, assigns} = Map.pop!(assigns, :path) - {persist_outputs, assigns} = Map.pop!(assigns, :persist_outputs) - - attrs = %{path: path, persist_outputs: persist_outputs} - - socket = - socket - |> assign(assigns) - |> assign(attrs: attrs, new_attrs: attrs) - - {:ok, socket} - end - - @impl true - def render(assigns) do - ~H""" -
-

- File -

-
-
-
- <.switch_checkbox - name="persist_outputs" - label="Persist outputs" - checked={@new_attrs.persist_outputs} /> -
-
-
- <.choice_button - active={@new_attrs.path != nil} - phx-click="set_persistence_type" - phx-value-type="file" - phx-target={@myself}> - Save to file - - <.choice_button - active={@new_attrs.path == nil} - phx-click="set_persistence_type" - phx-value-type="memory" - phx-target={@myself}> - Memory only - -
- <%= if @new_attrs.path != nil do %> -
- <%= live_component LivebookWeb.PathSelectComponent, - id: "path_select", - path: @new_attrs.path, - extnames: [LiveMarkdown.extension()], - running_paths: @running_paths, - phx_target: @myself, - phx_submit: if(disabled?(@new_attrs, @attrs, @running_paths), do: nil, else: "save") %> -
- <% end %> -
- <%= if @new_attrs.path != nil do %> -
- File: <%= normalize_path(@new_attrs.path) %> -
- <% end %> -
- -
-
-
-
- """ - end - - @impl true - def handle_event("set_persistence_type", %{"type" => type}, socket) do - path = - case type do - "file" -> socket.assigns.attrs.path || default_path() - "memory" -> nil - end - - {:noreply, put_new_attr(socket, :path, path)} - end - - def handle_event("set_path", %{"path" => path}, socket) do - {:noreply, put_new_attr(socket, :path, path)} - end - - def handle_event("set_options", %{"persist_outputs" => persist_outputs}, socket) do - persist_outputs = persist_outputs == "true" - {:noreply, put_new_attr(socket, :persist_outputs, persist_outputs)} - end - - def handle_event("save", %{}, %{assigns: assigns} = socket) do - path = normalize_path(assigns.new_attrs.path) - - if path != assigns.attrs.path do - Session.set_path(assigns.session_id, path) - end - - Session.set_notebook_attributes(assigns.session_id, %{ - persist_outputs: assigns.new_attrs.persist_outputs - }) - - Session.save_sync(assigns.session_id) - - running_paths = - if path do - [path | assigns.running_paths] - else - List.delete(assigns.running_paths, path) - end - - # After saving the file reload the directory contents, - # so that the new file gets shown. - send_update(LivebookWeb.PathSelectComponent, - id: "path_select", - running_paths: running_paths, - force_reload: true - ) - - {:noreply, assign(socket, running_paths: running_paths)} - end - - defp put_new_attr(socket, key, value) do - new_attrs = socket.assigns.new_attrs - new_attrs = put_in(new_attrs[key], value) - assign(socket, :new_attrs, new_attrs) - end - - defp default_path() do - Livebook.Config.root_path() |> Path.join("notebook") - end - - defp path_savable?(nil, _running_paths), do: true - - defp path_savable?(path, running_paths) do - if File.exists?(path) do - File.regular?(path) and path not in running_paths - else - true - end - end - - defp normalize_path(nil), do: nil - - defp normalize_path(path) do - if String.ends_with?(path, LiveMarkdown.extension()) do - path - else - path <> LiveMarkdown.extension() - end - end - - defp disabled?(new_attrs, attrs, running_paths) do - if normalize_path(new_attrs.path) == attrs.path do - new_attrs.persist_outputs == attrs.persist_outputs - else - not path_savable?(normalize_path(new_attrs.path), running_paths) - end - end -end diff --git a/lib/livebook_web/live/session_live/persistence_live.ex b/lib/livebook_web/live/session_live/persistence_live.ex new file mode 100644 index 00000000000..c7d41856903 --- /dev/null +++ b/lib/livebook_web/live/session_live/persistence_live.ex @@ -0,0 +1,254 @@ +defmodule LivebookWeb.SessionLive.PersistenceLive do + use LivebookWeb, :live_view + + alias Livebook.{Session, SessionSupervisor, LiveMarkdown, FileSystem} + + @impl true + def mount( + _params, + %{ + "session_id" => session_id, + "file" => file, + "persist_outputs" => persist_outputs, + "autosave_interval_s" => autosave_interval_s + }, + socket + ) do + session_summaries = SessionSupervisor.get_session_summaries() + running_files = Enum.map(session_summaries, & &1.file) + + attrs = %{ + file: file, + persist_outputs: persist_outputs, + autosave_interval_s: autosave_interval_s + } + + {:ok, + assign(socket, + session_id: session_id, + running_files: running_files, + attrs: attrs, + new_attrs: attrs + )} + end + + @impl true + def render(assigns) do + ~H""" +
+

+ File +

+
+
+
+ <.switch_checkbox + name="persist_outputs" + label="Persist outputs" + checked={@new_attrs.persist_outputs} /> +
+ Autosave + <.select + name="autosave_interval_s" + selected={@new_attrs.autosave_interval_s} + options={[ + {5, "every 5 seconds"}, + {30, "every 30 seconds"}, + {60, "every minute"}, + {600, "every 10 minutes"}, + {nil, "never"} + ]} /> +
+
+
+
+ <.choice_button + active={@new_attrs.file != nil} + phx-click="set_persistence_type" + phx-value-type="file"> + Save to file + + <.choice_button + active={@new_attrs.file == nil} + phx-click="set_persistence_type" + phx-value-type="memory"> + Memory only + +
+ <%= if @new_attrs.file do %> +
+ <%= live_component LivebookWeb.FileSelectComponent, + id: "persistence_file_select", + file: @new_attrs.file, + extnames: [LiveMarkdown.extension()], + running_files: @running_files, + submit_event: if(disabled?(@new_attrs, @attrs, @running_files), do: nil, else: :save) %> +
+ <% end %> +
+ <%= if @new_attrs.file do %> +
+ File: <%= normalize_file(@new_attrs.file).path %> +
+ <% end %> +
+ +
+
+
+
+ """ + end + + @impl true + def handle_event("set_persistence_type", %{"type" => type}, socket) do + file = + case type do + "file" -> socket.assigns.attrs.file || Livebook.Config.default_dir() + "memory" -> nil + end + + {:noreply, put_new_file(socket, file)} + end + + def handle_event( + "set_options", + %{"persist_outputs" => persist_outputs, "autosave_interval_s" => autosave_interval_s}, + socket + ) do + persist_outputs = persist_outputs == "true" + autosave_interval_s = parse_optional_integer(autosave_interval_s) + + {:noreply, + socket + |> put_new_attr(:persist_outputs, persist_outputs) + |> put_new_attr(:autosave_interval_s, autosave_interval_s)} + end + + def handle_event("save", %{}, socket) do + handle_save(socket) + end + + @impl true + def handle_info({:set_file, file, _file_info}, socket) do + {:noreply, put_new_file(socket, file)} + end + + def handle_info(:save, socket) do + handle_save(socket) + end + + defp handle_save(%{assigns: assigns} = socket) do + file = normalize_file(assigns.new_attrs.file) + autosave_interval_s = assigns.new_attrs.autosave_interval_s + + if file != assigns.attrs.file do + Session.set_file(assigns.session_id, file) + end + + if autosave_interval_s != assigns.attrs.autosave_interval_s do + Session.set_notebook_attributes(assigns.session_id, %{ + autosave_interval_s: autosave_interval_s + }) + end + + Session.set_notebook_attributes(assigns.session_id, %{ + persist_outputs: assigns.new_attrs.persist_outputs + }) + + Session.save_sync(assigns.session_id) + + running_files = + if file do + [file | assigns.running_files] + else + List.delete(assigns.running_files, file) + end + + if file do + # After saving the file reload the directory contents, + # so that the new file gets shown. + send_update(LivebookWeb.FileSelectComponent, + id: "persistence_file_select", + running_files: running_files, + force_reload: true + ) + end + + {:noreply, assign(socket, running_files: running_files, attrs: assigns.new_attrs)} + end + + defp parse_optional_integer(string) do + case Integer.parse(string) do + {number, _} -> number + :error -> nil + end + end + + defp put_new_file(socket, file) do + new_attrs = socket.assigns.new_attrs + current_file_system = new_attrs.file && new_attrs.file.file_system + new_file_system = file && file.file_system + + autosave_interval_s = + case new_file_system do + ^current_file_system -> + new_attrs.autosave_interval_s + + nil -> + Livebook.Notebook.default_autosave_interval_s() + + %FileSystem.Local{} -> + Livebook.Notebook.default_autosave_interval_s() + + _other -> + nil + end + + socket + |> put_new_attr(:file, file) + |> put_new_attr(:autosave_interval_s, autosave_interval_s) + end + + defp put_new_attr(socket, key, value) do + new_attrs = socket.assigns.new_attrs + + if new_attrs[key] == value do + socket + else + new_attrs = put_in(new_attrs[key], value) + assign(socket, :new_attrs, new_attrs) + end + end + + defp normalize_file(nil), do: nil + + defp normalize_file(file) do + Map.update!(file, :path, fn path -> + if String.ends_with?(path, LiveMarkdown.extension()) do + path + else + path <> LiveMarkdown.extension() + end + end) + end + + defp disabled?(new_attrs, attrs, running_files) do + if normalize_file(new_attrs.file) == attrs.file do + new_attrs.persist_outputs == attrs.persist_outputs and + new_attrs.autosave_interval_s == attrs.autosave_interval_s + else + not file_savable?(normalize_file(new_attrs.file), running_files) + end + end + + defp file_savable?(nil, _running_files), do: true + + defp file_savable?(file, running_files) do + not FileSystem.File.dir?(file) and file not in running_files + end +end diff --git a/lib/livebook_web/live/session_live/runtime_component.ex b/lib/livebook_web/live/session_live/runtime_component.ex index c89a73305d1..fe6e3c79faa 100644 --- a/lib/livebook_web/live/session_live/runtime_component.ex +++ b/lib/livebook_web/live/session_live/runtime_component.ex @@ -41,22 +41,8 @@ defmodule LivebookWeb.SessionLive.RuntimeComponent do

<%= if @runtime do %> -
- - Type - - - <%= runtime_type_label(@runtime) %> - -
-
- - Node name - - - <%= @runtime.node %> - -
+ <.labeled_text label="Type" text={runtime_type_label(@runtime)} /> + <.labeled_text label="Node name" text={@runtime.node} /> +
+ +
+
+ """ + end + + @impl true + def handle_event("validate", %{"data" => data}, socket) do + {:noreply, assign(socket, :data, data)} + end + + def handle_event("add", %{"data" => data}, socket) do + file_system = data_to_file_system(data) + default_dir = FileSystem.File.new(file_system) + + case FileSystem.File.list(default_dir) do + {:ok, _} -> + file_systems = Livebook.Config.append_file_system(file_system) + send(self(), {:file_systems_updated, file_systems}) + {:noreply, push_patch(socket, to: socket.assigns.return_to)} + + {:error, message} -> + {:noreply, assign(socket, error_message: "Connection test failed: " <> message)} + end + end + + defp empty_data() do + %{"bucket_url" => "", "access_key_id" => "", "secret_access_key" => ""} + end + + defp data_valid?(data) do + Livebook.Utils.valid_url?(data["bucket_url"]) and data["access_key_id"] != "" and + data["secret_access_key"] != "" + end + + defp data_to_file_system(data) do + FileSystem.S3.new(data["bucket_url"], data["access_key_id"], data["secret_access_key"]) + end +end diff --git a/lib/livebook_web/live/settings_live/file_systems_component.ex b/lib/livebook_web/live/settings_live/file_systems_component.ex new file mode 100644 index 00000000000..731554ab8df --- /dev/null +++ b/lib/livebook_web/live/settings_live/file_systems_component.ex @@ -0,0 +1,45 @@ +defmodule LivebookWeb.SettingsLive.FileSystemsComponent do + use LivebookWeb, :live_component + + alias Livebook.FileSystem + + @impl true + def render(assigns) do + ~H""" +
+
+ <%= for {file_system, index} <- Enum.with_index(@file_systems) do %> +
+
+ <.file_system_info file_system={file_system} /> +
+ <%= unless is_struct(file_system, FileSystem.Local) do %> + <%= live_patch "Detach", + to: Routes.settings_path(@socket, :detach_file_system, index), + class: "button button-outlined-red" %> + <% end %> +
+ <% end %> +
+
+ <%= live_patch "Add file system", + to: Routes.settings_path(@socket, :add_file_system), + class: "button button-blue" %> +
+
+ """ + end + + defp file_system_info(%{file_system: %FileSystem.Local{}} = assigns) do + ~H""" + <.labeled_text label="Type" text="Local disk" /> + """ + end + + defp file_system_info(%{file_system: %FileSystem.S3{}} = assigns) do + ~H""" + <.labeled_text label="Type" text="S3" /> + <.labeled_text label="Bucket URL" text={@file_system.bucket_url} /> + """ + end +end diff --git a/lib/livebook_web/live/settings_live/remove_file_system_component.ex b/lib/livebook_web/live/settings_live/remove_file_system_component.ex new file mode 100644 index 00000000000..7975ecd01ab --- /dev/null +++ b/lib/livebook_web/live/settings_live/remove_file_system_component.ex @@ -0,0 +1,33 @@ +defmodule LivebookWeb.SettingsLive.RemoveFileSystemComponent do + use LivebookWeb, :live_component + + @impl true + def render(assigns) do + ~H""" +
+

+ Detach file system +

+

+ Are you sure you want to detach this file system? + Any sessions using it will keep the access until + they get closed. +

+
+ + <%= live_patch "Cancel", to: @return_to, class: "button button-outlined-gray" %> +
+
+ """ + end + + @impl true + def handle_event("detach", %{}, socket) do + file_systems = Livebook.Config.remove_file_system(socket.assigns.file_system) + send(self(), {:file_systems_updated, file_systems}) + {:noreply, push_patch(socket, to: socket.assigns.return_to)} + end +end diff --git a/lib/livebook_web/router.ex b/lib/livebook_web/router.ex index fdf1b6f1e42..38018a77cf0 100644 --- a/lib/livebook_web/router.ex +++ b/lib/livebook_web/router.ex @@ -24,6 +24,11 @@ defmodule LivebookWeb.Router do live "/home/import/:tab", HomeLive, :import live "/home/sessions/:session_id/close", HomeLive, :close_session + live "/settings", SettingsLive, :page + live "/settings/user-profile", SettingsLive, :user + live "/settings/add-file-system", SettingsLive, :add_file_system + live "/settings/detach-file-system/:file_system_index", SettingsLive, :detach_file_system + live "/explore", ExploreLive, :page live "/explore/user-profile", ExploreLive, :user live "/explore/notebooks/:slug", ExploreLive, :notebook diff --git a/mix.exs b/mix.exs index 5dabf4e983b..e7a191f00d9 100644 --- a/mix.exs +++ b/mix.exs @@ -25,7 +25,7 @@ defmodule Livebook.MixProject do def application do [ mod: {Livebook.Application, []}, - extra_applications: [:logger, :runtime_tools, :os_mon, :inets, :ssl] + extra_applications: [:logger, :runtime_tools, :os_mon, :inets, :ssl, :xmerl] ] end diff --git a/test/livebook/content_loader_test.exs b/test/livebook/content_loader_test.exs index 36e8c71fe09..b5a94d7f136 100644 --- a/test/livebook/content_loader_test.exs +++ b/test/livebook/content_loader_test.exs @@ -25,27 +25,7 @@ defmodule Livebook.ContentLoaderTest do end end - describe "fetch_content/1 with local file URL" do - @tag :tmp_dir - test "returns an error then the file cannot be read", %{tmp_dir: tmp_dir} do - path = Path.join(tmp_dir, "nonexistent.livemd") - url = "file://" <> path - - assert ContentLoader.fetch_content(url) == - {:error, "failed to read #{path}, reason: no such file or directory"} - end - - @tag :tmp_dir - test "returns file contents when read successfully", %{tmp_dir: tmp_dir} do - path = Path.join(tmp_dir, "notebook.livemd") - File.write!(path, "# My notebook") - url = "file://" <> path - - assert ContentLoader.fetch_content(url) == {:ok, "# My notebook"} - end - end - - describe "fetch_content/1 with remote URL" do + describe "fetch_content/1" do setup do bypass = Bypass.open() {:ok, bypass: bypass} diff --git a/test/livebook/file_system/file_test.exs b/test/livebook/file_system/file_test.exs new file mode 100644 index 00000000000..374b6dfac8e --- /dev/null +++ b/test/livebook/file_system/file_test.exs @@ -0,0 +1,413 @@ +defmodule Livebook.FileSystem.FileTest do + use ExUnit.Case, async: true + + import Livebook.TestHelpers + + alias Livebook.FileSystem + + describe "new/2" do + test "raises an error when a relative path is given" do + file_system = FileSystem.Local.new() + + assert_raise ArgumentError, ~s{expected an absolute path, got: "file.txt"}, fn -> + FileSystem.File.new(file_system, "file.txt") + end + end + + test "uses default file system path if non is given" do + file_system = FileSystem.Local.new(default_path: "/dir/") + assert %FileSystem.File{path: "/dir/"} = FileSystem.File.new(file_system) + end + + test "expands the given path" do + file_system = FileSystem.Local.new() + + assert %FileSystem.File{path: "/dir/file.txt"} = + FileSystem.File.new(file_system, "/dir/nested/../file.txt") + end + end + + describe "relative/2" do + test "ignores the file path if an absolute path is given" do + file_system = FileSystem.Local.new() + file = FileSystem.File.new(file_system, "/dir/nested/file.txt") + + assert %FileSystem.File{file_system: ^file_system, path: "/other/file.txt"} = + FileSystem.File.relative(file, "/other/file.txt") + end + + test "resolves a relative path against a regular file" do + file_system = FileSystem.Local.new() + file = FileSystem.File.new(file_system, "/dir/nested/file.txt") + + assert %FileSystem.File{file_system: ^file_system, path: "/dir/other/other_file.txt"} = + FileSystem.File.relative(file, "../other/other_file.txt") + end + + test "resolves a relative path against a directory file" do + file_system = FileSystem.Local.new() + dir = FileSystem.File.new(file_system, "/dir/nested/") + + assert %FileSystem.File{file_system: ^file_system, path: "/dir/nested/file.txt"} = + FileSystem.File.relative(dir, "file.txt") + end + + test "resolves a relative directory path" do + file_system = FileSystem.Local.new() + file = FileSystem.File.new(file_system, "/dir/nested/file.txt") + + assert %FileSystem.File{file_system: ^file_system, path: "/dir/other/"} = + FileSystem.File.relative(file, "../other/") + + assert %FileSystem.File{file_system: ^file_system, path: "/dir/nested/"} = + FileSystem.File.relative(file, ".") + + assert %FileSystem.File{file_system: ^file_system, path: "/dir/"} = + FileSystem.File.relative(file, "..") + end + end + + describe "dir?/1" do + test "returns true if file path has a trailing slash" do + file_system = FileSystem.Local.new() + + dir = FileSystem.File.new(file_system, "/dir/") + assert FileSystem.File.dir?(dir) + + file = FileSystem.File.new(file_system, "/dir/file.txt") + refute FileSystem.File.dir?(file) + end + end + + describe "regular?/1" do + test "returns true if file path has no trailing slash" do + file_system = FileSystem.Local.new() + + dir = FileSystem.File.new(file_system, "/dir/") + refute FileSystem.File.regular?(dir) + + file = FileSystem.File.new(file_system, "/dir/file.txt") + assert FileSystem.File.regular?(file) + end + end + + describe "name/1" do + test "returns path basename" do + file_system = FileSystem.Local.new() + + dir = FileSystem.File.new(file_system, "/dir/") + assert FileSystem.File.name(dir) == "dir" + + file = FileSystem.File.new(file_system, "/dir/file.txt") + assert FileSystem.File.name(file) == "file.txt" + end + end + + describe "containing_dir/1" do + test "given a directory, returns the parent directory" do + file_system = FileSystem.Local.new() + + dir = FileSystem.File.new(file_system, "/parent/dir/") + assert FileSystem.File.new(file_system, "/parent/") == FileSystem.File.containing_dir(dir) + end + + test "given a file, returns the containing directory" do + file_system = FileSystem.Local.new() + + file = FileSystem.File.new(file_system, "/dir/file.txt") + assert FileSystem.File.new(file_system, "/dir/") == FileSystem.File.containing_dir(file) + end + + test "given the root directory, returns itself" do + file_system = FileSystem.Local.new() + + file = FileSystem.File.new(file_system, "/") + assert file == FileSystem.File.containing_dir(file) + end + end + + # Note: file system operations are thoroughly tested for + # each file system separately, so here we only test the + # FileSystem.File interface, rather than the various edge + # cases + + describe "list/1" do + @tag :tmp_dir + test "lists files in the given directory", %{tmp_dir: tmp_dir} do + create_tree!(tmp_dir, + dir: [ + nested: [ + "file.txt": "content" + ] + ], + file: "content", + "file.txt": "content" + ) + + dir = FileSystem.File.local(tmp_dir <> "/") + + assert {:ok, files} = FileSystem.File.list(dir) + + assert files |> Enum.sort() == [ + FileSystem.File.relative(dir, "dir/"), + FileSystem.File.relative(dir, "file"), + FileSystem.File.relative(dir, "file.txt") + ] + end + + @tag :tmp_dir + test "includes nested files when called with the :recursive option", %{tmp_dir: tmp_dir} do + create_tree!(tmp_dir, + dir: [ + nested: [ + double_nested: [], + "file.txt": "content" + ] + ], + file: "content", + "file.txt": "content" + ) + + dir = FileSystem.File.local(tmp_dir <> "/") + + assert {:ok, files} = FileSystem.File.list(dir, recursive: true) + + assert files |> Enum.sort() == [ + FileSystem.File.relative(dir, "dir/"), + FileSystem.File.relative(dir, "dir/nested/"), + FileSystem.File.relative(dir, "dir/nested/double_nested/"), + FileSystem.File.relative(dir, "dir/nested/file.txt"), + FileSystem.File.relative(dir, "file"), + FileSystem.File.relative(dir, "file.txt") + ] + end + end + + describe "read/1" do + @tag :tmp_dir + test "returns file contents", %{tmp_dir: tmp_dir} do + create_tree!(tmp_dir, + dir: [ + "file.txt": "content" + ] + ) + + path = Path.join(tmp_dir, "dir/file.txt") + file = FileSystem.File.local(path) + + assert {:ok, "content"} = FileSystem.File.read(file) + end + end + + describe "write/2" do + @tag :tmp_dir + test "writes file contents", %{tmp_dir: tmp_dir} do + path = Path.join(tmp_dir, "dir/file.txt") + file = FileSystem.File.local(path) + + assert :ok = FileSystem.File.write(file, "content") + assert {:ok, "content"} = FileSystem.File.read(file) + end + end + + describe "access/1" do + @tag :tmp_dir + test "writes file contents", %{tmp_dir: tmp_dir} do + create_tree!(tmp_dir, + dir: [ + "file.txt": "content" + ] + ) + + path = Path.join(tmp_dir, "dir/file.txt") + file = FileSystem.File.local(path) + + assert {:ok, :read_write} = FileSystem.File.access(file) + end + end + + describe "create_dir/1" do + @tag :tmp_dir + test "writes file contents", %{tmp_dir: tmp_dir} do + path = Path.join(tmp_dir, "dir/nested") <> "/" + dir = FileSystem.File.local(path) + + assert :ok = FileSystem.File.create_dir(dir) + assert {:ok, true} = FileSystem.File.exists?(dir) + end + end + + describe "remove/1" do + @tag :tmp_dir + test "writes file contents", %{tmp_dir: tmp_dir} do + create_tree!(tmp_dir, + dir: [ + "file.txt": "content" + ] + ) + + path = Path.join(tmp_dir, "dir") <> "/" + dir = FileSystem.File.local(path) + + assert :ok = FileSystem.File.remove(dir) + assert {:ok, false} = FileSystem.File.exists?(dir) + end + end + + describe "copy/2" do + @tag :tmp_dir + test "supports regular files from different file systems via explicit read and write", + %{tmp_dir: tmp_dir} do + bypass = Bypass.open() + s3_fs = FileSystem.S3.new("http://localhost:#{bypass.port}/mybucket", "key", "secret") + local_fs = FileSystem.Local.new() + + create_tree!(tmp_dir, + "src_file.txt": "content" + ) + + src_file = FileSystem.File.new(local_fs, Path.join(tmp_dir, "src_file.txt")) + dest_file = FileSystem.File.new(s3_fs, "/dest_file.txt") + + Bypass.expect_once(bypass, "PUT", "/mybucket/dest_file.txt", fn conn -> + assert {:ok, "content", conn} = Plug.Conn.read_body(conn) + + Plug.Conn.resp(conn, 200, "") + end) + + assert :ok = FileSystem.File.copy(src_file, dest_file) + end + + @tag :tmp_dir + test "supports directories from different file systems via explicit read and write", + %{tmp_dir: tmp_dir} do + bypass = Bypass.open() + s3_fs = FileSystem.S3.new("http://localhost:#{bypass.port}/mybucket", "key", "secret") + local_fs = FileSystem.Local.new() + + create_tree!(tmp_dir, + src_dir: [ + nested: [ + "file.txt": "content" + ], + "file.txt": "content" + ] + ) + + src_dir = FileSystem.File.new(local_fs, Path.join(tmp_dir, "src_dir") <> "/") + dest_dir = FileSystem.File.new(s3_fs, "/dest_dir/") + + Bypass.expect_once(bypass, "PUT", "/mybucket/dest_dir/nested/file.txt", fn conn -> + assert {:ok, "content", conn} = Plug.Conn.read_body(conn) + Plug.Conn.resp(conn, 200, "") + end) + + Bypass.expect_once(bypass, "PUT", "/mybucket/dest_dir/file.txt", fn conn -> + assert {:ok, "content", conn} = Plug.Conn.read_body(conn) + Plug.Conn.resp(conn, 200, "") + end) + + assert :ok = FileSystem.File.copy(src_dir, dest_dir) + end + end + + describe "rename/2" do + @tag :tmp_dir + test "returns an error when files from different file systems are given and the destination file exists", + %{tmp_dir: tmp_dir} do + s3_fs = FileSystem.S3.new("https://example.com/mybucket", "key", "secret") + local_fs = FileSystem.Local.new() + + create_tree!(tmp_dir, + "dest_file.txt": "content" + ) + + src_file = FileSystem.File.new(s3_fs, "/src_file.txt") + dest_file = FileSystem.File.new(local_fs, Path.join(tmp_dir, "dest_file.txt")) + + assert {:error, "file already exists"} = FileSystem.File.rename(src_file, dest_file) + end + + # Rename is implemented as copy and delete, so just a single + # integration test + + @tag :tmp_dir + test "supports regular files from different file systems via explicit read, write, delete", + %{tmp_dir: tmp_dir} do + bypass = Bypass.open() + s3_fs = FileSystem.S3.new("http://localhost:#{bypass.port}/mybucket", "key", "secret") + local_fs = FileSystem.Local.new() + + create_tree!(tmp_dir, + "src_file.txt": "content" + ) + + src_file = FileSystem.File.new(local_fs, Path.join(tmp_dir, "src_file.txt")) + dest_file = FileSystem.File.new(s3_fs, "/dest_file.txt") + + # Existence is verified by listing + Bypass.expect_once(bypass, "GET", "/mybucket", fn conn -> + assert %{"prefix" => "dest_file.txt", "delimiter" => "/"} = conn.params + + conn + |> Plug.Conn.put_resp_content_type("application/xml") + |> Plug.Conn.resp(200, """ + + mybucket + + """) + end) + + Bypass.expect_once(bypass, "PUT", "/mybucket/dest_file.txt", fn conn -> + assert {:ok, "content", conn} = Plug.Conn.read_body(conn) + + Plug.Conn.resp(conn, 200, "") + end) + + assert :ok = FileSystem.File.rename(src_file, dest_file) + assert {:ok, false} = FileSystem.File.exists?(src_file) + end + end + + describe "etag_for/1" do + @tag :tmp_dir + test "returns different value only when the file is updated", %{tmp_dir: tmp_dir} do + create_tree!(tmp_dir, + dir: [ + "file.txt": "content" + ] + ) + + path = Path.join(tmp_dir, "dir/file.txt") + file = FileSystem.File.local(path) + + assert {:ok, etag1} = FileSystem.File.etag_for(file) + assert {:ok, ^etag1} = FileSystem.File.etag_for(file) + + FileSystem.File.write(file, "udptupdate") + + assert {:ok, etag2} = FileSystem.File.etag_for(file) + + assert etag1 != etag2 + end + end + + describe "exists?/1" do + @tag :tmp_dir + test "checks if the file exists", %{tmp_dir: tmp_dir} do + create_tree!(tmp_dir, + dir: [ + "file.txt": "content" + ] + ) + + path = Path.join(tmp_dir, "dir/file.txt") + file = FileSystem.File.local(path) + assert {:ok, true} = FileSystem.File.exists?(file) + + path = Path.join(tmp_dir, "dir/nonexistent.txt") + file = FileSystem.File.local(path) + assert {:ok, false} = FileSystem.File.exists?(file) + end + end +end diff --git a/test/livebook/file_system/local_test.exs b/test/livebook/file_system/local_test.exs new file mode 100644 index 00000000000..01ae4ef085c --- /dev/null +++ b/test/livebook/file_system/local_test.exs @@ -0,0 +1,487 @@ +defmodule Livebook.FileSystem.LocalTest do + use ExUnit.Case, async: true + + import Livebook.TestHelpers + + alias Livebook.FileSystem + alias Livebook.FileSystem.Local + + describe "new/1" do + test "raises when :default_path is not a directory" do + assert_raise ArgumentError, ~s{expected a directory path, got: "/notebook.livemd"}, fn -> + Local.new(default_path: "/notebook.livemd") + end + end + end + + describe "FileSystem.default_path/1" do + test "defaults to the current directory" do + file_system = Local.new() + assert FileSystem.default_path(file_system) == File.cwd!() <> "/" + end + + test "returns custom directory path if configured" do + file_system = Local.new(default_path: "/dir/") + assert FileSystem.default_path(file_system) == "/dir/" + end + end + + describe "FileSystem.list/3" do + @tag :tmp_dir + test "returns an error when a nonexistent directory is given", %{tmp_dir: tmp_dir} do + file_system = Local.new() + dir_path = Path.join(tmp_dir, "nonexistent") <> "/" + + assert {:error, "no such file or directory"} = FileSystem.list(file_system, dir_path, false) + end + + @tag :tmp_dir + test "returns a list of absolute child file paths", %{tmp_dir: tmp_dir} do + create_tree!(tmp_dir, + dir: [ + nested: [ + "file.txt": "content" + ] + ], + file: "content", + "file.txt": "content" + ) + + file_system = Local.new() + dir_path = tmp_dir <> "/" + + assert {:ok, paths} = FileSystem.list(file_system, dir_path, false) + + assert Enum.sort(paths) == [ + Path.join(tmp_dir, "dir") <> "/", + Path.join(tmp_dir, "file"), + Path.join(tmp_dir, "file.txt") + ] + end + + @tag :tmp_dir + test "includes nested files when called with recursive flag", %{tmp_dir: tmp_dir} do + create_tree!(tmp_dir, + dir: [ + nested: [ + double_nested: [], + "file.txt": "content" + ] + ], + file: "content", + "file.txt": "content" + ) + + file_system = Local.new() + dir_path = tmp_dir <> "/" + + assert {:ok, paths} = FileSystem.list(file_system, dir_path, true) + + assert Enum.sort(paths) == [ + Path.join(tmp_dir, "dir") <> "/", + Path.join(tmp_dir, "dir/nested") <> "/", + Path.join(tmp_dir, "dir/nested/double_nested") <> "/", + Path.join(tmp_dir, "dir/nested/file.txt"), + Path.join(tmp_dir, "file"), + Path.join(tmp_dir, "file.txt") + ] + end + end + + describe "FileSystem.read/2" do + @tag :tmp_dir + test "returns an error when a nonexistent file is given", %{tmp_dir: tmp_dir} do + file_system = Local.new() + file_path = Path.join(tmp_dir, "nonexistent.txt") + + assert {:error, "no such file or directory"} = FileSystem.read(file_system, file_path) + end + + @tag :tmp_dir + test "returns file contents", %{tmp_dir: tmp_dir} do + create_tree!(tmp_dir, + dir: [ + "file.txt": "content" + ] + ) + + file_system = Local.new() + file_path = Path.join(tmp_dir, "dir/file.txt") + + assert {:ok, "content"} = FileSystem.read(file_system, file_path) + end + end + + describe "FileSystem.write/3" do + @tag :tmp_dir + test "writes file contents", %{tmp_dir: tmp_dir} do + file_system = Local.new() + file_path = Path.join(tmp_dir, "file.txt") + + assert :ok = FileSystem.write(file_system, file_path, "content") + assert File.read!(file_path) == "content" + end + + @tag :tmp_dir + test "creates nonexistent directories", %{tmp_dir: tmp_dir} do + file_system = Local.new() + file_path = Path.join(tmp_dir, "dir/nested/file.txt") + + assert :ok = FileSystem.write(file_system, file_path, "content") + assert File.read!(file_path) == "content" + end + + @tag :tmp_dir + test "overrides existing files", %{tmp_dir: tmp_dir} do + create_tree!(tmp_dir, + "file.txt": "content" + ) + + file_system = Local.new() + file_path = Path.join(tmp_dir, "file.txt") + + assert :ok = FileSystem.write(file_system, file_path, "new content") + assert File.read!(file_path) == "new content" + end + end + + describe "FileSystem.access/2" do + @tag :tmp_dir + test "returns an error when a nonexistent file is given", %{tmp_dir: tmp_dir} do + file_system = Local.new() + file_path = Path.join(tmp_dir, "file.txt") + + assert {:error, "no such file or directory"} = FileSystem.access(file_system, file_path) + end + + @tag :tmp_dir + test "returns permissions atom", %{tmp_dir: tmp_dir} do + create_tree!(tmp_dir, + "file.txt": "content" + ) + + file_system = Local.new() + file_path = Path.join(tmp_dir, "file.txt") + + assert {:ok, :read_write} = FileSystem.access(file_system, file_path) + + File.chmod!(file_path, 0o444) + assert {:ok, :read} = FileSystem.access(file_system, file_path) + end + + @tag :tmp_dir + test "handles directory paths", %{tmp_dir: tmp_dir} do + create_tree!(tmp_dir, + dir: [] + ) + + file_system = Local.new() + dir_path = Path.join(tmp_dir, "dir") <> "/" + + assert {:ok, :read_write} = FileSystem.access(file_system, dir_path) + end + end + + describe "FileSystem.create_dir/2" do + @tag :tmp_dir + test "creates the given directory and all nonexistent parent directories", %{tmp_dir: tmp_dir} do + create_tree!(tmp_dir, + dir: [] + ) + + file_system = Local.new() + dir_path = Path.join(tmp_dir, "nested/child/dir") <> "/" + + assert :ok = FileSystem.create_dir(file_system, dir_path) + assert File.dir?(dir_path) + end + end + + describe "FileSystem.remove/2" do + @tag :tmp_dir + test "returns successful value when a nonexistent file is given", %{tmp_dir: tmp_dir} do + file_system = Local.new() + file_path = Path.join(tmp_dir, "file.txt") + + assert :ok = FileSystem.remove(file_system, file_path) + end + + @tag :tmp_dir + test "when a file is given, removes only this file", %{tmp_dir: tmp_dir} do + create_tree!(tmp_dir, + dir: [ + "file.txt": "content" + ] + ) + + file_system = Local.new() + file_path = Path.join(tmp_dir, "dir/file.txt") + + assert :ok = FileSystem.remove(file_system, file_path) + + refute File.exists?(file_path) + assert File.exists?(Path.join(tmp_dir, "dir")) + end + + @tag :tmp_dir + test "when a directory is given, removes it with all contents", %{tmp_dir: tmp_dir} do + create_tree!(tmp_dir, + dir: [ + nested: [ + "file.txt": "content" + ], + "file.txt": "content" + ], + dir2: [], + "file.txt": "content" + ) + + file_system = Local.new() + dir_path = Path.join(tmp_dir, "dir") <> "/" + + assert :ok = FileSystem.remove(file_system, dir_path) + + refute File.exists?(dir_path) + assert File.exists?(Path.join(tmp_dir, "dir2")) + assert File.exists?(Path.join(tmp_dir, "file.txt")) + end + end + + describe "FileSystem.copy/3" do + @tag :tmp_dir + test "raises an error if the given paths have different type", %{tmp_dir: tmp_dir} do + file_system = Local.new() + src_file_path = Path.join(tmp_dir, "src_file.txt") + dest_dir_path = Path.join(tmp_dir, "dir") <> "/" + + assert_raise ArgumentError, ~r/^expected paths of the same type/, fn -> + FileSystem.copy(file_system, src_file_path, dest_dir_path) + end + end + + @tag :tmp_dir + test "returns an error when the source file does not exist", %{tmp_dir: tmp_dir} do + file_system = Local.new() + src_file_path = Path.join(tmp_dir, "src_file.txt") + dest_file_path = Path.join(tmp_dir, "dest_file.txt") + + assert {:error, "no such file or directory"} = + FileSystem.copy(file_system, src_file_path, dest_file_path) + end + + @tag :tmp_dir + test "when files are given, copies the content", %{tmp_dir: tmp_dir} do + create_tree!(tmp_dir, + "src_file.txt": "content" + ) + + file_system = Local.new() + src_file_path = Path.join(tmp_dir, "src_file.txt") + dest_file_path = Path.join(tmp_dir, "dest_file.txt") + + assert :ok = FileSystem.copy(file_system, src_file_path, dest_file_path) + + assert File.read!(dest_file_path) == "content" + assert File.read!(src_file_path) == "content" + end + + @tag :tmp_dir + test "creates nonexistent directories if necessary", %{tmp_dir: tmp_dir} do + create_tree!(tmp_dir, + "src_file.txt": "content" + ) + + file_system = Local.new() + src_file_path = Path.join(tmp_dir, "src_file.txt") + dest_file_path = Path.join(tmp_dir, "dir/nested/dest_file.txt") + + assert :ok = FileSystem.copy(file_system, src_file_path, dest_file_path) + + assert File.read!(dest_file_path) == "content" + assert File.read!(src_file_path) == "content" + end + + @tag :tmp_dir + test "overrides destination file if already exists", %{tmp_dir: tmp_dir} do + create_tree!(tmp_dir, + "src_file.txt": "content", + "dest_file.txt": "current content" + ) + + file_system = Local.new() + src_file_path = Path.join(tmp_dir, "src_file.txt") + dest_file_path = Path.join(tmp_dir, "dest_file.txt") + + assert :ok = FileSystem.copy(file_system, src_file_path, dest_file_path) + + assert File.read!(dest_file_path) == "content" + end + + @tag :tmp_dir + test "when a directories are given, copies all content recursively", %{tmp_dir: tmp_dir} do + create_tree!(tmp_dir, + src_dir: [ + nested: [ + "file.txt": "content" + ], + "file.txt": "content" + ], + dest_dir: [] + ) + + file_system = Local.new() + src_dir_path = Path.join(tmp_dir, "src_dir") <> "/" + dest_dir_path = Path.join(tmp_dir, "dest_dir") <> "/" + + assert :ok = FileSystem.copy(file_system, src_dir_path, dest_dir_path) + + assert File.read!(Path.join(dest_dir_path, "nested/file.txt")) == "content" + assert File.read!(Path.join(src_dir_path, "nested/file.txt")) == "content" + assert File.read!(Path.join(dest_dir_path, "file.txt")) == "content" + assert File.read!(Path.join(src_dir_path, "file.txt")) == "content" + end + end + + describe "FileSystem.rename/3" do + @tag :tmp_dir + test "raises an error if the given paths have different type", %{tmp_dir: tmp_dir} do + file_system = Local.new() + src_file_path = Path.join(tmp_dir, "src_file.txt") + dest_dir_path = Path.join(tmp_dir, "dir") <> "/" + + assert_raise ArgumentError, ~r/^expected paths of the same type/, fn -> + FileSystem.rename(file_system, src_file_path, dest_dir_path) + end + end + + @tag :tmp_dir + test "returns an error when the souce file does not exist", %{tmp_dir: tmp_dir} do + file_system = Local.new() + src_file_path = Path.join(tmp_dir, "src_file.txt") + dest_file_path = Path.join(tmp_dir, "dest_file.txt") + + assert {:error, "no such file or directory"} = + FileSystem.rename(file_system, src_file_path, dest_file_path) + end + + @tag :tmp_dir + test "returns an error when the desination file exists", %{tmp_dir: tmp_dir} do + create_tree!(tmp_dir, + "src_file.txt": "content", + "dest_file.txt": "content" + ) + + file_system = Local.new() + src_file_path = Path.join(tmp_dir, "src_file.txt") + dest_file_path = Path.join(tmp_dir, "dest_file.txt") + + assert {:error, "file already exists"} = + FileSystem.rename(file_system, src_file_path, dest_file_path) + end + + @tag :tmp_dir + test "when files are given, moves source to destination", %{tmp_dir: tmp_dir} do + create_tree!(tmp_dir, + "src_file.txt": "content" + ) + + file_system = Local.new() + src_file_path = Path.join(tmp_dir, "src_file.txt") + dest_file_path = Path.join(tmp_dir, "dest_file.txt") + + assert :ok = FileSystem.rename(file_system, src_file_path, dest_file_path) + + assert File.read!(dest_file_path) == "content" + refute File.exists?(src_file_path) + end + + @tag :tmp_dir + test "creates nonexistent directories if necessary", %{tmp_dir: tmp_dir} do + create_tree!(tmp_dir, + "src_file.txt": "content" + ) + + file_system = Local.new() + src_file_path = Path.join(tmp_dir, "src_file.txt") + dest_file_path = Path.join(tmp_dir, "dir/nested/dest_file.txt") + + assert :ok = FileSystem.rename(file_system, src_file_path, dest_file_path) + + assert File.read!(dest_file_path) == "content" + refute File.exists?(src_file_path) == "content" + end + + @tag :tmp_dir + test "handles directories", %{tmp_dir: tmp_dir} do + create_tree!(tmp_dir, + src_dir: [ + nested: [ + "file.txt": "content" + ], + "file.txt": "content" + ] + ) + + file_system = Local.new() + src_dir_path = Path.join(tmp_dir, "src_dir") <> "/" + dest_dir_path = Path.join(tmp_dir, "dest_dir") <> "/" + + assert :ok = FileSystem.rename(file_system, src_dir_path, dest_dir_path) + + assert File.read!(Path.join(dest_dir_path, "nested/file.txt")) == "content" + assert File.read!(Path.join(dest_dir_path, "file.txt")) == "content" + refute File.exists?(src_dir_path) + end + end + + describe "FileSystem.etag_for/2" do + @tag :tmp_dir + test "returns an error when a nonexistent file is given", %{tmp_dir: tmp_dir} do + file_system = Local.new() + file_path = Path.join(tmp_dir, "file.txt") + + assert {:error, "no such file or directory"} = FileSystem.etag_for(file_system, file_path) + end + + @tag :tmp_dir + test "returns different value only when the file is updated", %{tmp_dir: tmp_dir} do + create_tree!(tmp_dir, + "file.txt": "content" + ) + + file_system = Local.new() + file_path = Path.join(tmp_dir, "file.txt") + + assert {:ok, etag1} = FileSystem.etag_for(file_system, file_path) + assert {:ok, ^etag1} = FileSystem.etag_for(file_system, file_path) + + File.write!(file_path, "update") + + assert {:ok, etag2} = FileSystem.etag_for(file_system, file_path) + + assert etag1 != etag2 + end + end + + describe "FileSystem.exists?/2" do + @tag :tmp_dir + test "returns false when the given file doesn't exist", %{tmp_dir: tmp_dir} do + file_system = Local.new() + file_path = Path.join(tmp_dir, "file.txt") + + assert {:ok, false} = FileSystem.exists?(file_system, file_path) + end + + @tag :tmp_dir + test "returns true when the given file exists", %{tmp_dir: tmp_dir} do + create_tree!(tmp_dir, + "file.txt": "content" + ) + + file_system = Local.new() + file_path = Path.join(tmp_dir, "file.txt") + + assert {:ok, true} = FileSystem.exists?(file_system, file_path) + end + end +end diff --git a/test/livebook/file_system/s3_test.exs b/test/livebook/file_system/s3_test.exs new file mode 100644 index 00000000000..07b7f208e4f --- /dev/null +++ b/test/livebook/file_system/s3_test.exs @@ -0,0 +1,652 @@ +defmodule Livebook.FileSystem.S3Test do + use ExUnit.Case, async: true + + alias Livebook.FileSystem + alias Livebook.FileSystem.S3 + + setup do + bypass = Bypass.open() + {:ok, bypass: bypass} + end + + describe "new/3" do + test "trims trailing slash in bucket URL" do + assert %{bucket_url: "https://example.com/mybucket"} = + S3.new("https://example.com/mybucket/", "key", "secret") + end + end + + describe "FileSystem.default_path/1" do + test "returns the root path" do + file_system = S3.new("https://example.com/mybucket", "key", "secret") + assert FileSystem.default_path(file_system) == "/" + end + end + + describe "common request errors" do + test "authorization failure", %{bypass: bypass} do + Bypass.expect_once(bypass, "GET", "/mybucket", fn conn -> + conn + |> Plug.Conn.put_resp_content_type("application/xml") + |> Plug.Conn.resp(403, """ + + Reason for authorization failure + + """) + end) + + file_system = S3.new(bucket_url(bypass.port), "key", "secret") + dir_path = "/dir/" + + assert {:error, "access denied, reason for authorization failure"} = + FileSystem.list(file_system, dir_path, false) + end + + test "an arbitrary error with message", %{bypass: bypass} do + Bypass.expect_once(bypass, "GET", "/mybucket", fn conn -> + conn + |> Plug.Conn.put_resp_content_type("application/xml") + |> Plug.Conn.resp(500, """ + + Error message + + """) + end) + + file_system = S3.new(bucket_url(bypass.port), "key", "secret") + dir_path = "/dir/" + + assert {:error, "error message"} = FileSystem.list(file_system, dir_path, false) + end + + test "successful response with unexpected body", %{bypass: bypass} do + Bypass.expect_once(bypass, "GET", "/mybucket", fn conn -> + conn + |> Plug.Conn.put_resp_content_type("application/xml") + |> Plug.Conn.resp(200, """ + + Idea + + """) + end) + + file_system = S3.new(bucket_url(bypass.port), "key", "secret") + dir_path = "/dir/" + + assert {:error, "unexpected response"} = FileSystem.list(file_system, dir_path, false) + end + end + + describe "FileSystem.list/3" do + test "returns an error when a nonexistent directory is given", %{bypass: bypass} do + # When the directory doesn't exist, we get an empty list of maches + + Bypass.expect_once(bypass, "GET", "/mybucket", fn conn -> + assert %{"delimiter" => "/"} = conn.params + + conn + |> Plug.Conn.put_resp_content_type("application/xml") + |> Plug.Conn.resp(200, """ + + mybucket + + """) + end) + + file_system = S3.new(bucket_url(bypass.port), "key", "secret") + dir_path = "/dir/" + + assert {:error, "no such file or directory"} = FileSystem.list(file_system, dir_path, false) + end + + test "returns a list of absolute child object paths", %{bypass: bypass} do + Bypass.expect_once(bypass, "GET", "/mybucket", fn conn -> + assert %{"delimiter" => "/"} = conn.params + + conn + |> Plug.Conn.put_resp_content_type("application/xml") + |> Plug.Conn.resp(200, """ + + mybucket + + dir/ + + + dir + + + file.txt + + + """) + end) + + file_system = S3.new(bucket_url(bypass.port), "key", "secret") + dir_path = "/" + + assert {:ok, paths} = FileSystem.list(file_system, dir_path, false) + + assert Enum.sort(paths) == [ + "/dir", + "/dir/", + "/file.txt" + ] + end + + test "includes nested objects when called with recursive flag", %{bypass: bypass} do + Bypass.expect_once(bypass, "GET", "/mybucket", fn conn -> + assert %{"delimiter" => ""} = conn.params + + conn + |> Plug.Conn.put_resp_content_type("application/xml") + |> Plug.Conn.resp(200, """ + + mybucket + + dir/ + + + dir/file.txt + + + dir/nested/ + + + dir/nested/file.txt + + + dir + + + file.txt + + + """) + end) + + file_system = S3.new(bucket_url(bypass.port), "key", "secret") + dir_path = "/" + + assert {:ok, paths} = FileSystem.list(file_system, dir_path, true) + + assert Enum.sort(paths) == [ + "/dir", + "/dir/", + "/dir/file.txt", + "/dir/nested/", + "/dir/nested/file.txt", + "/file.txt" + ] + end + end + + describe "FileSystem.read/2" do + test "returns an error when a nonexistent key is given", %{bypass: bypass} do + Bypass.expect_once(bypass, "GET", "/mybucket/nonexistent.txt", fn conn -> + conn + |> Plug.Conn.put_resp_content_type("application/xml") + |> Plug.Conn.resp(404, """ + + The specified key does not exist. + + """) + end) + + file_system = S3.new(bucket_url(bypass.port), "key", "secret") + file_path = "/nonexistent.txt" + + assert {:error, "no such file or directory"} = FileSystem.read(file_system, file_path) + end + + test "returns object contents under the given key", %{bypass: bypass} do + content = """ + + this should not be parsed + + """ + + Bypass.expect_once(bypass, "GET", "/mybucket/dir/file.txt", fn conn -> + # When reading the content should be returned as binary, + # regardless the content type + conn + |> Plug.Conn.put_resp_content_type("application/xml") + |> Plug.Conn.resp(200, content) + end) + + file_system = S3.new(bucket_url(bypass.port), "key", "secret") + file_path = "/dir/file.txt" + + assert {:ok, ^content} = FileSystem.read(file_system, file_path) + end + end + + describe "FileSystem.write/3" do + test "writes contents under the file key", %{bypass: bypass} do + content = "content" + + Bypass.expect_once(bypass, "PUT", "/mybucket/dir/file.txt", fn conn -> + assert {:ok, ^content, conn} = Plug.Conn.read_body(conn) + + Plug.Conn.resp(conn, 200, "") + end) + + file_system = S3.new(bucket_url(bypass.port), "key", "secret") + file_path = "/dir/file.txt" + + assert :ok = FileSystem.write(file_system, file_path, content) + end + end + + describe "FileSystem.create_dir/2" do + test "write empty content under the directory key", %{bypass: bypass} do + Bypass.expect_once(bypass, "PUT", "/mybucket/dir/", fn conn -> + assert {:ok, "", conn} = Plug.Conn.read_body(conn) + + Plug.Conn.resp(conn, 200, "") + end) + + file_system = S3.new(bucket_url(bypass.port), "key", "secret") + dir_path = "/dir/" + + assert :ok = FileSystem.create_dir(file_system, dir_path) + end + end + + describe "FileSystem.remove/2" do + test "returns successful value when a nonexistent key is given", %{bypass: bypass} do + Bypass.expect_once(bypass, "DELETE", "/mybucket/file.txt", fn conn -> + conn + |> Plug.Conn.put_resp_content_type("application/xml") + |> Plug.Conn.resp(404, """ + + The specified key does not exist. + + """) + end) + + file_system = S3.new(bucket_url(bypass.port), "key", "secret") + file_path = "/file.txt" + + assert :ok = FileSystem.remove(file_system, file_path) + end + + test "deletes object under the corresponding key", %{bypass: bypass} do + Bypass.expect_once(bypass, "DELETE", "/mybucket/file.txt", fn conn -> + Plug.Conn.resp(conn, 204, "") + end) + + file_system = S3.new(bucket_url(bypass.port), "key", "secret") + file_path = "/file.txt" + + assert :ok = FileSystem.remove(file_system, file_path) + end + + test "when a directory is given, recursively lists and batch deletes all matching keys", + %{bypass: bypass} do + Bypass.expect_once(bypass, "GET", "/mybucket", fn conn -> + assert %{"prefix" => "dir/", "delimiter" => ""} = conn.params + + conn + |> Plug.Conn.put_resp_content_type("application/xml") + |> Plug.Conn.resp(200, """ + + mybucket + + dir/ + + + dir/file.txt + + + dir/nested/ + + + dir/nested/file.txt + + + """) + end) + + expected_body = + """ + + + dir/ + + + dir/file.txt + + + dir/nested/ + + + dir/nested/file.txt + + true + + """ + |> String.replace(~r/\s/, "") + + Bypass.expect_once(bypass, "POST", "/mybucket", fn conn -> + assert %{"delete" => ""} = conn.params + + assert {:ok, ^expected_body, conn} = Plug.Conn.read_body(conn) + + conn + |> Plug.Conn.put_resp_content_type("application/xml") + |> Plug.Conn.resp(200, """ + + + """) + end) + + file_system = S3.new(bucket_url(bypass.port), "key", "secret") + dir_path = "/dir/" + + assert :ok = FileSystem.remove(file_system, dir_path) + end + end + + describe "FileSystem.copy/3" do + test "raises an error if the given paths have different type" do + file_system = S3.new("https://example.com/mybucket", "key", "secret") + src_file_path = "/src_file.txt" + dest_dir_path = "/dir/" + + assert_raise ArgumentError, ~r/^expected paths of the same type/, fn -> + FileSystem.copy(file_system, src_file_path, dest_dir_path) + end + end + + test "given file paths, returns an error if the source object does not exist", + %{bypass: bypass} do + # Request for the bucket name + Bypass.expect_once(bypass, "GET", "/mybucket", fn conn -> + conn + |> Plug.Conn.put_resp_content_type("application/xml") + |> Plug.Conn.resp(200, """ + + mybucket + + """) + end) + + Bypass.expect_once(bypass, "PUT", "/mybucket/dest_file.txt", fn conn -> + assert ["mybucket/src_file.txt"] = Plug.Conn.get_req_header(conn, "x-amz-copy-source") + + conn + |> Plug.Conn.put_resp_content_type("application/xml") + |> Plug.Conn.resp(404, """ + + The specified key does not exist. + + """) + end) + + file_system = S3.new(bucket_url(bypass.port), "key", "secret") + src_file_path = "/src_file.txt" + dest_file_path = "/dest_file.txt" + + assert {:error, "no such file or directory"} = + FileSystem.copy(file_system, src_file_path, dest_file_path) + end + + test "given file paths, copies contents into the new key", %{bypass: bypass} do + # Request for the bucket name + Bypass.expect_once(bypass, "GET", "/mybucket", fn conn -> + conn + |> Plug.Conn.put_resp_content_type("application/xml") + |> Plug.Conn.resp(200, """ + + mybucket + + """) + end) + + Bypass.expect_once(bypass, "PUT", "/mybucket/dest_file.txt", fn conn -> + assert ["mybucket/src_file.txt"] = Plug.Conn.get_req_header(conn, "x-amz-copy-source") + + conn + |> Plug.Conn.put_resp_content_type("application/xml") + |> Plug.Conn.resp(200, """ + + + """) + end) + + file_system = S3.new(bucket_url(bypass.port), "key", "secret") + src_file_path = "/src_file.txt" + dest_file_path = "/dest_file.txt" + + assert :ok = FileSystem.copy(file_system, src_file_path, dest_file_path) + end + + test "given directory paths, returns an error if the source directory does not exist", + %{bypass: bypass} do + # Directory listing with no results + Bypass.expect_once(bypass, "GET", "/mybucket", fn conn -> + assert %{"prefix" => "src_dir/", "delimiter" => ""} = conn.params + + conn + |> Plug.Conn.put_resp_content_type("application/xml") + |> Plug.Conn.resp(200, """ + + mybucket + + """) + end) + + file_system = S3.new(bucket_url(bypass.port), "key", "secret") + src_dir_path = "/src_dir/" + dest_dir_path = "/dest_dir/" + + assert {:error, "no such file or directory"} = + FileSystem.copy(file_system, src_dir_path, dest_dir_path) + end + + test "given directory paths, recursively lists all matching keys and individually copies objects", + %{bypass: bypass} do + # Directory listing + Bypass.expect_once(bypass, "GET", "/mybucket", fn conn -> + assert %{"prefix" => "src_dir/", "delimiter" => ""} = conn.params + + conn + |> Plug.Conn.put_resp_content_type("application/xml") + |> Plug.Conn.resp(200, """ + + mybucket + + src_dir/ + + + src_dir/file.txt + + + src_dir/nested/ + + + src_dir/nested/file.txt + + + """) + end) + + # Copy requests, one per object + for {src_key, dest_key} <- [ + {"src_dir/", "dest_dir/"}, + {"src_dir/file.txt", "dest_dir/file.txt"}, + {"src_dir/nested/", "dest_dir/nested/"}, + {"src_dir/nested/file.txt", "dest_dir/nested/file.txt"} + ] do + Bypass.expect_once(bypass, "PUT", "/mybucket/#{dest_key}", fn conn -> + assert [src_header] = Plug.Conn.get_req_header(conn, "x-amz-copy-source") + assert src_header == "mybucket/#{src_key}" + + conn + |> Plug.Conn.put_resp_content_type("application/xml") + |> Plug.Conn.resp(200, """ + + + """) + end) + end + + file_system = S3.new(bucket_url(bypass.port), "key", "secret") + src_dir_path = "/src_dir/" + dest_dir_path = "/dest_dir/" + + assert :ok = FileSystem.copy(file_system, src_dir_path, dest_dir_path) + end + end + + describe "FileSystem.rename/3" do + test "raises an error if the given paths have different type" do + file_system = S3.new("https://example.com/mybucket", "key", "secret") + src_file_path = "/src_file.txt" + dest_dir_path = "/dir/" + + assert_raise ArgumentError, ~r/^expected paths of the same type/, fn -> + FileSystem.rename(file_system, src_file_path, dest_dir_path) + end + end + + test "returns an error when the desination file exists", %{bypass: bypass} do + # Existence is verified by listing + Bypass.expect_once(bypass, "GET", "/mybucket", fn conn -> + assert %{"prefix" => "dest_file.txt", "delimiter" => "/"} = conn.params + + conn + |> Plug.Conn.put_resp_content_type("application/xml") + |> Plug.Conn.resp(200, """ + + mybucket + + dest_file.txt + + + """) + end) + + file_system = S3.new(bucket_url(bypass.port), "key", "secret") + src_file_path = "/src_file.txt" + dest_file_path = "/dest_file.txt" + + assert {:error, "file already exists"} = + FileSystem.rename(file_system, src_file_path, dest_file_path) + end + + # Rename is implemented as copy and delete, both of which are + # tested separately, so here's just one integration test to + # verify this behaviour + test "given file paths, copies the content and deletes the destination", %{bypass: bypass} do + # Expects two requests: + # * destination existence check by listing, we return no entries + # * bucket name request + Bypass.expect(bypass, "GET", "/mybucket", fn conn -> + conn + |> Plug.Conn.put_resp_content_type("application/xml") + |> Plug.Conn.resp(200, """ + + mybucket + + """) + end) + + Bypass.expect_once(bypass, "PUT", "/mybucket/dest_file.txt", fn conn -> + assert ["mybucket/src_file.txt"] = Plug.Conn.get_req_header(conn, "x-amz-copy-source") + + conn + |> Plug.Conn.put_resp_content_type("application/xml") + |> Plug.Conn.resp(200, """ + + + """) + end) + + Bypass.expect_once(bypass, "DELETE", "/mybucket/src_file.txt", fn conn -> + Plug.Conn.resp(conn, 204, "") + end) + + file_system = S3.new(bucket_url(bypass.port), "key", "secret") + src_file_path = "/src_file.txt" + dest_file_path = "/dest_file.txt" + + assert :ok = FileSystem.rename(file_system, src_file_path, dest_file_path) + end + end + + describe "FileSystem.etag_for/2" do + test "returns an error when a nonexistent key is given", %{bypass: bypass} do + Bypass.expect_once(bypass, "HEAD", "/mybucket/nonexistent.txt", fn conn -> + Plug.Conn.resp(conn, 404, "") + end) + + file_system = S3.new(bucket_url(bypass.port), "key", "secret") + file_path = "/nonexistent.txt" + + assert {:error, "no such file or directory"} = FileSystem.etag_for(file_system, file_path) + end + + test "returns the ETag value received from the server", %{bypass: bypass} do + Bypass.expect_once(bypass, "HEAD", "/mybucket/nonexistent.txt", fn conn -> + conn + |> Plug.Conn.put_resp_header("ETag", "value") + |> Plug.Conn.resp(200, "") + end) + + file_system = S3.new(bucket_url(bypass.port), "key", "secret") + file_path = "/nonexistent.txt" + + assert {:ok, "value"} = FileSystem.etag_for(file_system, file_path) + end + end + + describe "FileSystem.exists?/2" do + test "returns false when the given object doesn't exist", %{bypass: bypass} do + # Existence is verified by listing + Bypass.expect_once(bypass, "GET", "/mybucket", fn conn -> + assert %{"prefix" => "file.txt", "delimiter" => "/"} = conn.params + + conn + |> Plug.Conn.put_resp_content_type("application/xml") + |> Plug.Conn.resp(200, """ + + mybucket + + """) + end) + + file_system = S3.new(bucket_url(bypass.port), "key", "secret") + file_path = "/file.txt" + + assert {:ok, false} = FileSystem.exists?(file_system, file_path) + end + + test "returns true when the given object exists", %{bypass: bypass} do + # Existence is verified by listing + Bypass.expect_once(bypass, "GET", "/mybucket", fn conn -> + assert %{"prefix" => "file.txt", "delimiter" => "/"} = conn.params + + conn + |> Plug.Conn.put_resp_content_type("application/xml") + |> Plug.Conn.resp(200, """ + + mybucket + + file.txt + + + """) + end) + + file_system = S3.new(bucket_url(bypass.port), "key", "secret") + file_path = "/file.txt" + + assert {:ok, true} = FileSystem.exists?(file_system, file_path) + end + end + + # Helpers + + defp bucket_url(port), do: "http://localhost:#{port}/mybucket" +end diff --git a/test/livebook/live_markdown/export_test.exs b/test/livebook/live_markdown/export_test.exs index 642a11c4a2d..4b2598597dc 100644 --- a/test/livebook/live_markdown/export_test.exs +++ b/test/livebook/live_markdown/export_test.exs @@ -754,4 +754,22 @@ defmodule Livebook.LiveMarkdown.ExportTest do assert expected_document == document end + + test "persists :autosave_interval_s when other than default" do + notebook = %{ + Notebook.new() + | name: "My Notebook", + autosave_interval_s: 10 + } + + expected_document = """ + + + # My Notebook + """ + + document = Export.notebook_to_markdown(notebook) + + assert expected_document == document + end end diff --git a/test/livebook/live_markdown/import_test.exs b/test/livebook/live_markdown/import_test.exs index d5bbfad7396..b563d11df13 100644 --- a/test/livebook/live_markdown/import_test.exs +++ b/test/livebook/live_markdown/import_test.exs @@ -569,4 +569,16 @@ defmodule Livebook.LiveMarkdown.ImportTest do assert %Notebook{name: "My Notebook", persist_outputs: true} = notebook end end + + test "imports notebook :autosave_interval_s attribute" do + markdown = """ + + + # My Notebook + """ + + {notebook, []} = Import.notebook_from_markdown(markdown) + + assert %Notebook{name: "My Notebook", autosave_interval_s: 10} = notebook + end end diff --git a/test/livebook/session/data_test.exs b/test/livebook/session/data_test.exs index 55243e769f0..5dd12d63505 100644 --- a/test/livebook/session/data_test.exs +++ b/test/livebook/session/data_test.exs @@ -3017,12 +3017,14 @@ defmodule Livebook.Session.DataTest do end end - describe "apply_operation/2 given :set_path" do + describe "apply_operation/2 given :set_file" do test "updates data with the given path" do data = Data.new() - operation = {:set_path, self(), "path"} - assert {:ok, %{path: "path"}, []} = Data.apply_operation(data, operation) + file = Livebook.FileSystem.File.local("/path/to/file.livemd") + operation = {:set_file, self(), file} + + assert {:ok, %{file: ^file}, []} = Data.apply_operation(data, operation) end end diff --git a/test/livebook/session_test.exs b/test/livebook/session_test.exs index 9f9462810bf..70cfcc82356 100644 --- a/test/livebook/session_test.exs +++ b/test/livebook/session_test.exs @@ -1,7 +1,7 @@ defmodule Livebook.SessionTest do use ExUnit.Case, async: true - alias Livebook.{Session, Delta, Runtime, Utils, Notebook} + alias Livebook.{Session, Delta, Runtime, Utils, Notebook, FileSystem} # Note: queueing evaluation in most of the tests below # requires the runtime to synchronously start first, @@ -217,67 +217,76 @@ defmodule Livebook.SessionTest do end end - describe "set_path/1" do + describe "set_file/1" do @tag :tmp_dir - test "sends a path update operation to subscribers", + test "sends a file update operation to subscribers", %{session_id: session_id, tmp_dir: tmp_dir} do Phoenix.PubSub.subscribe(Livebook.PubSub, "sessions:#{session_id}") pid = self() - path = Path.join(tmp_dir, "notebook.livemd") - Session.set_path(session_id, path) + tmp_dir = FileSystem.File.local(tmp_dir <> "/") + file = FileSystem.File.relative(tmp_dir, "notebook.livemd") + Session.set_file(session_id, file) - assert_receive {:operation, {:set_path, ^pid, ^path}} + assert_receive {:operation, {:set_file, ^pid, ^file}} end @tag :tmp_dir test "broadcasts an error if the path is already in use", %{session_id: session_id, tmp_dir: tmp_dir} do - path = Path.join(tmp_dir, "notebook.livemd") - start_session(path: path) + tmp_dir = FileSystem.File.local(tmp_dir <> "/") + file = FileSystem.File.relative(tmp_dir, "notebook.livemd") + start_session(file: file) Phoenix.PubSub.subscribe(Livebook.PubSub, "sessions:#{session_id}") - Session.set_path(session_id, path) + Session.set_file(session_id, file) - assert_receive {:error, "failed to set new path because it is already in use"} + assert_receive {:error, "failed to set new file because it is already in use"} end @tag :tmp_dir test "moves images to the new directory", %{session_id: session_id, tmp_dir: tmp_dir} do + tmp_dir = FileSystem.File.local(tmp_dir <> "/") %{images_dir: images_dir} = Session.get_summary(session_id) - File.mkdir_p!(images_dir) - images_dir |> Path.join("test.jpg") |> File.touch!() - path = Path.join(tmp_dir, "notebook.livemd") - Session.set_path(session_id, path) + image_file = FileSystem.File.relative(images_dir, "test.jpg") + :ok = FileSystem.File.write(image_file, "") + + file = FileSystem.File.relative(tmp_dir, "notebook.livemd") + Session.set_file(session_id, file) # Wait for the session to deal with the files Process.sleep(50) - assert File.exists?(Path.join([tmp_dir, "images", "test.jpg"])) - refute File.exists?(images_dir) + assert {:ok, true} = + FileSystem.File.exists?(FileSystem.File.relative(tmp_dir, "images/test.jpg")) + + assert {:ok, false} = FileSystem.File.exists?(images_dir) end @tag :tmp_dir test "does not remove images from the previous dir if not temporary", %{session_id: session_id, tmp_dir: tmp_dir} do - path = Path.join(tmp_dir, "notebook.livemd") - Session.set_path(session_id, path) + tmp_dir = FileSystem.File.local(tmp_dir <> "/") + file = FileSystem.File.relative(tmp_dir, "notebook.livemd") + Session.set_file(session_id, file) %{images_dir: images_dir} = Session.get_summary(session_id) - File.mkdir_p!(images_dir) - images_dir |> Path.join("test.jpg") |> File.touch!() + image_file = FileSystem.File.relative(images_dir, "test.jpg") + :ok = FileSystem.File.write(image_file, "") - Session.set_path(session_id, nil) + Session.set_file(session_id, nil) # Wait for the session to deal with the files Process.sleep(50) - assert File.exists?(Path.join(images_dir, "test.jpg")) + assert {:ok, true} = FileSystem.File.exists?(image_file) %{images_dir: new_images_dir} = Session.get_summary(session_id) - assert File.exists?(Path.join(new_images_dir, "test.jpg")) + + assert {:ok, true} = + FileSystem.File.exists?(FileSystem.File.relative(new_images_dir, "test.jpg")) end end @@ -285,38 +294,38 @@ defmodule Livebook.SessionTest do @tag :tmp_dir test "persists the notebook to the associated file and notifies subscribers", %{session_id: session_id, tmp_dir: tmp_dir} do - path = Path.join(tmp_dir, "notebook.livemd") - Session.set_path(session_id, path) + tmp_dir = FileSystem.File.local(tmp_dir <> "/") + file = FileSystem.File.relative(tmp_dir, "notebook.livemd") + Session.set_file(session_id, file) # Perform a change, so the notebook is dirty Session.set_notebook_name(session_id, "My notebook") Phoenix.PubSub.subscribe(Livebook.PubSub, "sessions:#{session_id}") - refute File.exists?(path) + assert {:ok, false} = FileSystem.File.exists?(file) Session.save(session_id) assert_receive {:operation, {:mark_as_not_dirty, _}} - assert File.exists?(path) - assert File.read!(path) =~ "My notebook" + assert FileSystem.File.read(file) == {:ok, "# My notebook\n"} end @tag :tmp_dir test "creates nonexistent directories", %{session_id: session_id, tmp_dir: tmp_dir} do - path = Path.join(tmp_dir, "nonexistent/dir/notebook.livemd") - Session.set_path(session_id, path) + tmp_dir = FileSystem.File.local(tmp_dir <> "/") + file = FileSystem.File.relative(tmp_dir, "nonexistent/dir/notebook.livemd") + Session.set_file(session_id, file) # Perform a change, so the notebook is dirty Session.set_notebook_name(session_id, "My notebook") Phoenix.PubSub.subscribe(Livebook.PubSub, "sessions:#{session_id}") - refute File.exists?(path) + assert {:ok, false} = FileSystem.File.exists?(file) Session.save(session_id) assert_receive {:operation, {:mark_as_not_dirty, _}} - assert File.exists?(path) - assert File.read!(path) =~ "My notebook" + assert FileSystem.File.read(file) == {:ok, "# My notebook\n"} end end @@ -324,28 +333,28 @@ defmodule Livebook.SessionTest do @tag :tmp_dir test "saves the notebook and notifies subscribers once the session is closed", %{session_id: session_id, tmp_dir: tmp_dir} do - path = Path.join(tmp_dir, "notebook.livemd") - Session.set_path(session_id, path) + tmp_dir = FileSystem.File.local(tmp_dir <> "/") + file = FileSystem.File.relative(tmp_dir, "notebook.livemd") + Session.set_file(session_id, file) # Perform a change, so the notebook is dirty Session.set_notebook_name(session_id, "My notebook") Phoenix.PubSub.subscribe(Livebook.PubSub, "sessions:#{session_id}") - refute File.exists?(path) + assert {:ok, false} = FileSystem.File.exists?(file) Process.flag(:trap_exit, true) Session.close(session_id) assert_receive :session_closed - assert File.exists?(path) - assert File.read!(path) =~ "My notebook" + assert FileSystem.File.read(file) == {:ok, "# My notebook\n"} end test "clears session temporary directory", %{session_id: session_id} do %{images_dir: images_dir} = Session.get_summary(session_id) - File.mkdir_p!(images_dir) + :ok = FileSystem.File.create_dir(images_dir) - assert File.exists?(images_dir) + assert {:ok, true} = FileSystem.File.exists?(images_dir) Process.flag(:trap_exit, true) Session.close(session_id) @@ -353,28 +362,34 @@ defmodule Livebook.SessionTest do # Wait for the session to deal with the files Process.sleep(50) - refute File.exists?(images_dir) + assert {:ok, false} = FileSystem.File.exists?(images_dir) end end describe "start_link/1" do @tag :tmp_dir test "fails if the given path is already in use", %{tmp_dir: tmp_dir} do - path = Path.join(tmp_dir, "notebook.livemd") - start_session(path: path) + tmp_dir = FileSystem.File.local(tmp_dir <> "/") + file = FileSystem.File.relative(tmp_dir, "notebook.livemd") + start_session(file: file) - assert {:error, "the given path is already in use"} == - Session.start_link(id: Utils.random_id(), path: path) + assert {:error, "the given file is already in use"} == + Session.start_link(id: Utils.random_id(), file: file) end @tag :tmp_dir test "copies images when :copy_images_from option is specified", %{tmp_dir: tmp_dir} do - tmp_dir |> Path.join("image.jpg") |> File.touch!() + tmp_dir = FileSystem.File.local(tmp_dir <> "/") + + image_file = FileSystem.File.relative(tmp_dir, "image.jpg") + :ok = FileSystem.File.write(image_file, "") session_id = start_session(copy_images_from: tmp_dir) %{images_dir: images_dir} = Session.get_summary(session_id) - assert File.exists?(Path.join(images_dir, "image.jpg")) + + assert {:ok, true} = + FileSystem.File.exists?(FileSystem.File.relative(images_dir, "image.jpg")) end test "saves images when :images option is specified" do @@ -383,7 +398,9 @@ defmodule Livebook.SessionTest do session_id = start_session(images: images) %{images_dir: images_dir} = Session.get_summary(session_id) - assert Path.join(images_dir, "image.jpg") |> File.read!() == "binary content" + + assert FileSystem.File.relative(images_dir, "image.jpg") |> FileSystem.File.read() == + {:ok, "binary content"} end end diff --git a/test/livebook_web/controllers/session_controller_test.exs b/test/livebook_web/controllers/session_controller_test.exs index c840ab11b64..e71adc5d3a2 100644 --- a/test/livebook_web/controllers/session_controller_test.exs +++ b/test/livebook_web/controllers/session_controller_test.exs @@ -1,7 +1,7 @@ defmodule LivebookWeb.SessionControllerTest do use LivebookWeb.ConnCase, async: true - alias Livebook.{SessionSupervisor, Session, Notebook} + alias Livebook.{SessionSupervisor, Session, Notebook, FileSystem} describe "show_image" do test "returns not found when the given session does not exist", %{conn: conn} do @@ -17,7 +17,7 @@ defmodule LivebookWeb.SessionControllerTest do conn = get(conn, Routes.session_path(conn, :show_image, session_id, "nonexistent.jpg")) assert conn.status == 404 - assert conn.resp_body == "Not found" + assert conn.resp_body == "No such file or directory" SessionSupervisor.close_session(session_id) end @@ -25,8 +25,7 @@ defmodule LivebookWeb.SessionControllerTest do test "returns the image when it does exist", %{conn: conn} do {:ok, session_id} = SessionSupervisor.create_session() %{images_dir: images_dir} = Session.get_summary(session_id) - File.mkdir_p!(images_dir) - images_dir |> Path.join("test.jpg") |> File.touch!() + :ok = FileSystem.File.relative(images_dir, "test.jpg") |> FileSystem.File.write("") conn = get(conn, Routes.session_path(conn, :show_image, session_id, "test.jpg")) diff --git a/test/livebook_web/live/file_select_component_test.exs b/test/livebook_web/live/file_select_component_test.exs new file mode 100644 index 00000000000..3597b59bab3 --- /dev/null +++ b/test/livebook_web/live/file_select_component_test.exs @@ -0,0 +1,40 @@ +defmodule LivebookWeb.FileSelectComponentTest do + use LivebookWeb.ConnCase, async: true + + import Phoenix.LiveViewTest + + alias Livebook.FileSystem + alias LivebookWeb.FileSelectComponent + + test "when the path has a trailing slash, lists that directory" do + file = FileSystem.File.local(notebooks_path() <> "/") + assert render_component(FileSelectComponent, attrs(file: file)) =~ "basic.livemd" + assert render_component(FileSelectComponent, attrs(file: file)) =~ ".." + end + + test "when the path has no trailing slash, lists the parent directory" do + file = FileSystem.File.local(notebooks_path()) + assert render_component(FileSelectComponent, attrs(file: file)) =~ "notebooks" + end + + test "does not show parent directory when in root" do + file = FileSystem.File.local("/") + refute render_component(FileSelectComponent, attrs(file: file)) =~ ".." + end + + defp attrs(attrs) do + Keyword.merge( + [ + id: 1, + file: FileSystem.File.local("/"), + extnames: [".livemd"], + running_files: [] + ], + attrs + ) + end + + defp notebooks_path() do + Path.expand("../../support/notebooks", __DIR__) + end +end diff --git a/test/livebook_web/live/home_live_test.exs b/test/livebook_web/live/home_live_test.exs index 1e6207f45ad..339d0bee9d3 100644 --- a/test/livebook_web/live/home_live_test.exs +++ b/test/livebook_web/live/home_live_test.exs @@ -28,9 +28,12 @@ defmodule LivebookWeb.HomeLiveTest do path = Path.expand("../../../lib", __DIR__) <> "/" - assert view - |> element("form") - |> render_change(%{path: path}) =~ "livebook_web" + view + |> element("form") + |> render_change(%{path: path}) + + # Render the view separately to make sure it received the :set_file event + render(view) =~ "livebook_web" end test "allows importing when a notebook file is selected", %{conn: conn} do @@ -40,7 +43,11 @@ defmodule LivebookWeb.HomeLiveTest do view |> element("form") - |> render_change(%{path: path}) + |> render_change(%{path: Path.dirname(path) <> "/"}) + + view + |> element("button", "basic.livemd") + |> render_click() assert assert {:error, {:live_redirect, %{to: to}}} = view @@ -50,14 +57,13 @@ defmodule LivebookWeb.HomeLiveTest do assert to =~ "/sessions/" end - test "disables import when a directory is selected", %{conn: conn} do + @tag :tmp_dir + test "disables import when a directory is selected", %{conn: conn, tmp_dir: tmp_dir} do {:ok, view, _} = live(conn, "/") - path = File.cwd!() - view |> element("form") - |> render_change(%{path: path}) + |> render_change(%{path: tmp_dir <> "/"}) assert view |> element(~s{button[phx-click="fork"][disabled]}, "Fork") @@ -89,7 +95,11 @@ defmodule LivebookWeb.HomeLiveTest do view |> element("form") - |> render_change(%{path: path}) + |> render_change(%{path: tmp_dir <> "/"}) + + view + |> element("button", "write_protected.livemd") + |> render_click() assert view |> element(~s{button[phx-click="open"][disabled]}, "Open") diff --git a/test/livebook_web/live/path_select_component_test.exs b/test/livebook_web/live/path_select_component_test.exs deleted file mode 100644 index f3fe91513bd..00000000000 --- a/test/livebook_web/live/path_select_component_test.exs +++ /dev/null @@ -1,49 +0,0 @@ -defmodule LivebookWeb.PathSelectComponentTest do - # Note: we cannot run asynchronously, because we use `File.cd!` - use LivebookWeb.ConnCase, async: false - - import Phoenix.LiveViewTest - - alias LivebookWeb.PathSelectComponent - - test "when the path has a trailing slash, lists that directory" do - path = notebooks_path() <> "/" - assert render_component(PathSelectComponent, attrs(path: path)) =~ "basic.livemd" - assert render_component(PathSelectComponent, attrs(path: path)) =~ ".." - end - - test "when the path has no trailing slash, lists the parent directory" do - path = notebooks_path() - assert render_component(PathSelectComponent, attrs(path: path)) =~ "notebooks" - end - - test "does not show parent directory when in root" do - path = "/" - refute render_component(PathSelectComponent, attrs(path: path)) =~ ".." - end - - test "relative paths are expanded from the current working directory" do - File.cd!(notebooks_path(), fn -> - path = "" - assert render_component(PathSelectComponent, attrs(path: path)) =~ "basic.livemd" - end) - end - - defp attrs(attrs) do - Keyword.merge( - [ - id: 1, - path: "/", - extnames: [".livemd"], - running_paths: [], - phx_target: nil, - phx_submit: nil - ], - attrs - ) - end - - defp notebooks_path() do - Path.expand("../../support/notebooks", __DIR__) - end -end diff --git a/test/livebook_web/live/session_live_test.exs b/test/livebook_web/live/session_live_test.exs index fc94a7e146c..fac29d8f848 100644 --- a/test/livebook_web/live/session_live_test.exs +++ b/test/livebook_web/live/session_live_test.exs @@ -3,7 +3,7 @@ defmodule LivebookWeb.SessionLiveTest do import Phoenix.LiveViewTest - alias Livebook.{SessionSupervisor, Session, Delta, Runtime, Users} + alias Livebook.{SessionSupervisor, Session, Delta, Runtime, Users, FileSystem} alias Livebook.Notebook.Cell alias Livebook.Users.User @@ -210,6 +210,8 @@ defmodule LivebookWeb.SessionLiveTest do %{conn: conn, session_id: session_id, tmp_dir: tmp_dir} do {:ok, view, _} = live(conn, "/sessions/#{session_id}/settings/file") + assert view = find_live_child(view, "persistence") + path = Path.join(tmp_dir, "notebook.livemd") view @@ -232,6 +234,8 @@ defmodule LivebookWeb.SessionLiveTest do test "changing output persistence updates data", %{conn: conn, session_id: session_id} do {:ok, view, _} = live(conn, "/sessions/#{session_id}/settings/file") + assert view = find_live_child(view, "persistence") + view |> element("button", "Save to file") |> render_click() @@ -459,10 +463,11 @@ defmodule LivebookWeb.SessionLiveTest do @tag :tmp_dir test "renders an error message when the relative notebook does not exist", %{conn: conn, session_id: session_id, tmp_dir: tmp_dir} do - index_path = Path.join(tmp_dir, "index.livemd") - notebook_path = Path.join(tmp_dir, "notebook.livemd") + tmp_dir = FileSystem.File.local(tmp_dir <> "/") + index_file = FileSystem.File.relative(tmp_dir, "index.livemd") + notebook_file = FileSystem.File.relative(tmp_dir, "notebook.livemd") - Session.set_path(session_id, index_path) + Session.set_file(session_id, index_file) wait_for_session_update(session_id) session_path = "/sessions/#{session_id}" @@ -473,19 +478,20 @@ defmodule LivebookWeb.SessionLiveTest do {:ok, view, _} = follow_redirect(result, conn) assert render(view) =~ - "Cannot navigate, failed to read #{notebook_path}, reason: no such file or directory" + "Cannot navigate, failed to read #{notebook_file.path}, reason: no such file or directory" end @tag :tmp_dir test "opens a relative notebook if it exists", %{conn: conn, session_id: session_id, tmp_dir: tmp_dir} do - index_path = Path.join(tmp_dir, "index.livemd") - notebook_path = Path.join(tmp_dir, "notebook.livemd") + tmp_dir = FileSystem.File.local(tmp_dir <> "/") + index_file = FileSystem.File.relative(tmp_dir, "index.livemd") + notebook_file = FileSystem.File.relative(tmp_dir, "notebook.livemd") - Session.set_path(session_id, index_path) + Session.set_file(session_id, index_file) wait_for_session_update(session_id) - File.write!(notebook_path, "# Sibling notebook") + :ok = FileSystem.File.write(notebook_file, "# Sibling notebook") assert {:error, {:live_redirect, %{to: new_session_path}}} = result = live(conn, "/sessions/#{session_id}/notebook.livemd") @@ -495,18 +501,19 @@ defmodule LivebookWeb.SessionLiveTest do "/sessions/" <> session_id = new_session_path data = Session.get_data(session_id) - assert data.path == notebook_path + assert data.file == notebook_file end @tag :tmp_dir test "if the current session has no path, forks the relative notebook", %{conn: conn, tmp_dir: tmp_dir} do - index_path = Path.join(tmp_dir, "index.livemd") - notebook_path = Path.join(tmp_dir, "notebook.livemd") + tmp_dir = FileSystem.File.local(tmp_dir <> "/") + index_file = FileSystem.File.relative(tmp_dir, "index.livemd") + notebook_file = FileSystem.File.relative(tmp_dir, "notebook.livemd") - {:ok, session_id} = SessionSupervisor.create_session(origin_url: "file://" <> index_path) + {:ok, session_id} = SessionSupervisor.create_session(origin: {:file, index_file}) - File.write!(notebook_path, "# Sibling notebook") + :ok = FileSystem.File.write(notebook_file, "# Sibling notebook") assert {:error, {:live_redirect, %{to: new_session_path}}} = result = live(conn, "/sessions/#{session_id}/notebook.livemd") @@ -516,20 +523,21 @@ defmodule LivebookWeb.SessionLiveTest do "/sessions/" <> session_id = new_session_path data = Session.get_data(session_id) - assert data.path == nil - assert data.origin_url == "file://" <> notebook_path + assert data.file == nil + assert data.origin == {:file, notebook_file} end @tag :tmp_dir test "if the notebook is already open, redirects to the session", %{conn: conn, session_id: session_id, tmp_dir: tmp_dir} do - index_path = Path.join(tmp_dir, "index.livemd") - notebook_path = Path.join(tmp_dir, "notebook.livemd") + tmp_dir = FileSystem.File.local(tmp_dir <> "/") + index_file = FileSystem.File.relative(tmp_dir, "index.livemd") + notebook_file = FileSystem.File.relative(tmp_dir, "notebook.livemd") - Session.set_path(session_id, index_path) + Session.set_file(session_id, index_file) wait_for_session_update(session_id) - File.write!(notebook_path, "# Sibling notebook") + :ok = FileSystem.File.write(notebook_file, "# Sibling notebook") assert {:error, {:live_redirect, %{to: session_path}}} = live(conn, "/sessions/#{session_id}/notebook.livemd") @@ -540,15 +548,15 @@ defmodule LivebookWeb.SessionLiveTest do @tag :tmp_dir test "handles nested paths", %{conn: conn, session_id: session_id, tmp_dir: tmp_dir} do - parent_path = Path.join(tmp_dir, "parent.livemd") - child_dir = Path.join(tmp_dir, "dir") - child_path = Path.join(child_dir, "child.livemd") + tmp_dir = FileSystem.File.local(tmp_dir <> "/") + parent_file = FileSystem.File.relative(tmp_dir, "parent.livemd") + child_dir = FileSystem.File.relative(tmp_dir, "dir/") + child_file = FileSystem.File.relative(child_dir, "child.livemd") - Session.set_path(session_id, parent_path) + Session.set_file(session_id, parent_file) wait_for_session_update(session_id) - File.mkdir!(child_dir) - File.write!(child_path, "# Child notebook") + :ok = FileSystem.File.write(child_file, "# Child notebook") {:ok, view, _} = conn @@ -560,15 +568,15 @@ defmodule LivebookWeb.SessionLiveTest do @tag :tmp_dir test "handles parent paths", %{conn: conn, session_id: session_id, tmp_dir: tmp_dir} do - parent_path = Path.join(tmp_dir, "parent.livemd") - child_dir = Path.join(tmp_dir, "dir") - child_path = Path.join(child_dir, "child.livemd") + tmp_dir = FileSystem.File.local(tmp_dir <> "/") + parent_file = FileSystem.File.relative(tmp_dir, "parent.livemd") + child_dir = FileSystem.File.relative(tmp_dir, "dir/") + child_file = FileSystem.File.relative(child_dir, "child.livemd") - File.mkdir!(child_dir) - Session.set_path(session_id, child_path) + Session.set_file(session_id, child_file) wait_for_session_update(session_id) - File.write!(parent_path, "# Parent notebook") + :ok = FileSystem.File.write(parent_file, "# Parent notebook") {:ok, view, _} = conn @@ -588,7 +596,7 @@ defmodule LivebookWeb.SessionLiveTest do end) index_url = url(bypass.port) <> "/index.livemd" - {:ok, session_id} = SessionSupervisor.create_session(origin_url: index_url) + {:ok, session_id} = SessionSupervisor.create_session(origin: {:url, index_url}) {:ok, view, _} = conn @@ -607,7 +615,7 @@ defmodule LivebookWeb.SessionLiveTest do index_url = url(bypass.port) <> "/index.livemd" - {:ok, session_id} = SessionSupervisor.create_session(origin_url: index_url) + {:ok, session_id} = SessionSupervisor.create_session(origin: {:url, index_url}) session_path = "/sessions/#{session_id}" @@ -623,8 +631,8 @@ defmodule LivebookWeb.SessionLiveTest do index_url = "http://example.com/#{test}/index.livemd" notebook_url = "http://example.com/#{test}/notebook.livemd" - {:ok, index_session_id} = SessionSupervisor.create_session(origin_url: index_url) - {:ok, notebook_session_id} = SessionSupervisor.create_session(origin_url: notebook_url) + {:ok, index_session_id} = SessionSupervisor.create_session(origin: {:url, index_url}) + {:ok, notebook_session_id} = SessionSupervisor.create_session(origin: {:url, notebook_url}) notebook_session_path = "/sessions/#{notebook_session_id}" @@ -637,9 +645,13 @@ defmodule LivebookWeb.SessionLiveTest do index_url = "http://example.com/#{test}/index.livemd" notebook_url = "http://example.com/#{test}/notebook.livemd" - {:ok, index_session_id} = SessionSupervisor.create_session(origin_url: index_url) - {:ok, _notebook_session_id1} = SessionSupervisor.create_session(origin_url: notebook_url) - {:ok, _notebook_session_id2} = SessionSupervisor.create_session(origin_url: notebook_url) + {:ok, index_session_id} = SessionSupervisor.create_session(origin: {:url, index_url}) + + {:ok, _notebook_session_id1} = + SessionSupervisor.create_session(origin: {:url, notebook_url}) + + {:ok, _notebook_session_id2} = + SessionSupervisor.create_session(origin: {:url, notebook_url}) index_session_path = "/sessions/#{index_session_id}" diff --git a/test/support/test_helpers.ex b/test/support/test_helpers.ex new file mode 100644 index 00000000000..20014a7750a --- /dev/null +++ b/test/support/test_helpers.ex @@ -0,0 +1,21 @@ +defmodule Livebook.TestHelpers do + @moduledoc false + + @doc """ + Creates file structure according to the given specification. + """ + def create_tree!(path, items) do + for {name, content} <- items do + child_path = Path.join(path, to_string(name)) + + case content do + items when is_list(items) -> + File.mkdir!(child_path) + create_tree!(child_path, items) + + content when is_binary(content) -> + File.write!(child_path, content) + end + end + end +end From 28afcfd137460a30d21e24413fb0c8c84807ec66 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jonatan=20K=C5=82osko?= Date: Fri, 13 Aug 2021 15:36:04 +0200 Subject: [PATCH 2/8] Support arbitrary absolute paths and delegate resolution to file system --- lib/livebook/file_system.ex | 13 +++ lib/livebook/file_system/file.ex | 22 ++-- lib/livebook/file_system/local.ex | 39 +++---- lib/livebook/file_system/s3.ex | 33 +++--- lib/livebook/file_system/utils.ex | 103 +++++++----------- lib/livebook/session.ex | 6 +- .../controllers/session_controller.ex | 2 +- .../live/file_select_component.ex | 6 +- lib/livebook_web/live/session_live.ex | 2 +- .../session_live/cell_upload_component.ex | 2 +- test/livebook/file_system/file_test.exs | 49 +++++---- test/livebook/file_system/local_test.exs | 26 +++++ test/livebook/file_system/s3_test.exs | 26 +++++ test/livebook/session_test.exs | 30 ++--- .../controllers/session_controller_test.exs | 2 +- test/livebook_web/live/session_live_test.exs | 28 ++--- 16 files changed, 227 insertions(+), 162 deletions(-) diff --git a/lib/livebook/file_system.ex b/lib/livebook/file_system.ex index d6ab082270d..32afdb07cf0 100644 --- a/lib/livebook/file_system.ex +++ b/lib/livebook/file_system.ex @@ -130,4 +130,17 @@ defprotocol Livebook.FileSystem do """ @spec exists?(t(), path()) :: {:ok, boolean()} | {:error, error()} def exists?(file_system, path) + + @doc """ + Resolves `subject` against a valid directory path. + + The `subject` may be either relative or absolute, + contain special sequences such as ".." and ".", + but the interpretation is left up to the file system. + + In other words, this has the semantics of path join + followed by expand. + """ + @spec resolve_path(t(), path(), String.t()) :: path() + def resolve_path(file_system, dir_path, subject) end diff --git a/lib/livebook/file_system/file.ex b/lib/livebook/file_system/file.ex index 0095381cf60..4872e5493b1 100644 --- a/lib/livebook/file_system/file.ex +++ b/lib/livebook/file_system/file.ex @@ -25,11 +25,19 @@ defmodule Livebook.FileSystem.File do """ @spec new(FileSystem.t(), FileSystem.path() | nil) :: t() def new(file_system, path \\ nil) do + default_path = FileSystem.default_path(file_system) + path = if path do - FileSystem.Utils.normalize_path!(path) + resolved_path = FileSystem.resolve_path(file_system, default_path, path) + + unless path == resolved_path do + raise ArgumentError, "expected an expanded absolute path, got: #{inspect(path)}" + end + + path else - FileSystem.default_path(file_system) + default_path end %__MODULE__{file_system: file_system, path: path} @@ -45,16 +53,16 @@ defmodule Livebook.FileSystem.File do end @doc """ - Returns a new file resulting from expanding `path` + Returns a new file resulting from resolving `subject` against `file`. An absolute path may be given, in which case it replaces the file path altogether. """ - @spec relative(t(), String.t()) :: t() - def relative(file, path) do + @spec resolve(t(), String.t()) :: t() + def resolve(file, subject) do dir = if dir?(file), do: file, else: containing_dir(file) - path = FileSystem.Utils.expand_path(path, dir.path) + path = FileSystem.resolve_path(file.file_system, dir.path, subject) new(file.file_system, path) end @@ -190,7 +198,7 @@ defmodule Livebook.FileSystem.File do |> Enum.reduce(:ok, fn child_file, result -> with :ok <- result do path_sufix = String.replace_leading(child_file.path, source.path, "") - child_destination = relative(destination, path_sufix) + child_destination = resolve(destination, path_sufix) copy_regular_file(child_file, child_destination) end end) diff --git a/lib/livebook/file_system/local.ex b/lib/livebook/file_system/local.ex index bdbca00a1cc..a28b1850c77 100644 --- a/lib/livebook/file_system/local.ex +++ b/lib/livebook/file_system/local.ex @@ -22,11 +22,11 @@ defmodule Livebook.FileSystem.Local do @spec new(keyword()) :: t() def new(opts \\ []) do default_path = - opts - |> Keyword.get_lazy(:default_path, fn -> + Keyword.get_lazy(opts, :default_path, fn -> File.cwd!() |> FileSystem.Utils.ensure_dir_path() end) - |> FileSystem.Utils.normalize_path!(type: :directory) + + FileSystem.Utils.assert_dir_path!(default_path) %__MODULE__{default_path: default_path} end @@ -40,7 +40,7 @@ defimpl Livebook.FileSystem, for: Livebook.FileSystem.Local do end def list(file_system, path, recursive) do - path = FileSystem.Utils.normalize_path!(path, type: :directory) + FileSystem.Utils.assert_dir_path!(path) case File.ls(path) do {:ok, filenames} -> @@ -70,7 +70,7 @@ defimpl Livebook.FileSystem, for: Livebook.FileSystem.Local do end def read(_file_system, path) do - path = FileSystem.Utils.normalize_path!(path, type: :regular) + FileSystem.Utils.assert_regular_path!(path) case File.read(path) do {:ok, binary} -> {:ok, binary} @@ -79,7 +79,7 @@ defimpl Livebook.FileSystem, for: Livebook.FileSystem.Local do end def write(_file_system, path, content) do - path = FileSystem.Utils.normalize_path!(path, type: :regular) + FileSystem.Utils.assert_regular_path!(path) dir = Path.dirname(path) @@ -92,8 +92,6 @@ defimpl Livebook.FileSystem, for: Livebook.FileSystem.Local do end def access(_file_system, path) do - path = FileSystem.Utils.normalize_path!(path) - case File.stat(path) do {:ok, stat} -> {:ok, stat.access} {:error, error} -> FileSystem.Utils.posix_error(error) @@ -101,7 +99,7 @@ defimpl Livebook.FileSystem, for: Livebook.FileSystem.Local do end def create_dir(_file_system, path) do - path = FileSystem.Utils.normalize_path!(path, type: :directory) + FileSystem.Utils.assert_dir_path!(path) case File.mkdir_p(path) do :ok -> :ok @@ -110,8 +108,6 @@ defimpl Livebook.FileSystem, for: Livebook.FileSystem.Local do end def remove(_file_system, path) do - path = FileSystem.Utils.normalize_path!(path) - case File.rm_rf(path) do {:ok, _paths} -> :ok {:error, error} -> FileSystem.Utils.posix_error(error) @@ -119,8 +115,6 @@ defimpl Livebook.FileSystem, for: Livebook.FileSystem.Local do end def copy(_file_system, source_path, destination_path) do - source_path = FileSystem.Utils.normalize_path!(source_path) - destination_path = FileSystem.Utils.normalize_path!(destination_path) FileSystem.Utils.assert_same_type!(source_path, destination_path) containing_dir = Path.dirname(destination_path) @@ -138,8 +132,6 @@ defimpl Livebook.FileSystem, for: Livebook.FileSystem.Local do end def rename(_file_system, source_path, destination_path) do - source_path = FileSystem.Utils.normalize_path!(source_path) - destination_path = FileSystem.Utils.normalize_path!(destination_path) FileSystem.Utils.assert_same_type!(source_path, destination_path) if File.exists?(destination_path) do @@ -158,8 +150,6 @@ defimpl Livebook.FileSystem, for: Livebook.FileSystem.Local do end def etag_for(_file_system, path) do - path = FileSystem.Utils.normalize_path!(path) - case File.stat(path) do {:ok, stat} -> %{size: size, mtime: mtime} = stat @@ -173,12 +163,23 @@ defimpl Livebook.FileSystem, for: Livebook.FileSystem.Local do end def exists?(_file_system, path) do - path = FileSystem.Utils.normalize_path!(path) - if FileSystem.Utils.dir_path?(path) do {:ok, File.dir?(path)} else {:ok, File.exists?(path)} end end + + def resolve_path(_file_system, dir_path, ""), do: dir_path + + def resolve_path(_file_system, dir_path, subject) do + dir? = FileSystem.Utils.dir_path?(subject) or Path.basename(subject) in [".", ".."] + expanded_path = Path.expand(subject, dir_path) + + if dir? do + FileSystem.Utils.ensure_dir_path(expanded_path) + else + expanded_path + end + end end diff --git a/lib/livebook/file_system/s3.ex b/lib/livebook/file_system/s3.ex index 575c80c7c4b..f80204007c2 100644 --- a/lib/livebook/file_system/s3.ex +++ b/lib/livebook/file_system/s3.ex @@ -36,7 +36,8 @@ defimpl Livebook.FileSystem, for: Livebook.FileSystem.S3 do end def list(file_system, path, recursive) do - "/" <> dir_key = FileSystem.Utils.normalize_path!(path, type: :directory) + FileSystem.Utils.assert_dir_path!(path) + "/" <> dir_key = path delimiter = if recursive, do: nil, else: "/" @@ -51,22 +52,24 @@ defimpl Livebook.FileSystem, for: Livebook.FileSystem.S3 do end def read(file_system, path) do - "/" <> key = FileSystem.Utils.normalize_path!(path, type: :regular) + FileSystem.Utils.assert_regular_path!(path) + "/" <> key = path get_object(file_system, key) end def write(file_system, path, content) do - "/" <> key = FileSystem.Utils.normalize_path!(path, type: :regular) + FileSystem.Utils.assert_regular_path!(path) + "/" <> key = path put_object(file_system, key, content) end - def access(_file_system, path) do - FileSystem.Utils.normalize_path!(path) + def access(_file_system, _path) do {:ok, :read_write} end def create_dir(file_system, path) do - "/" <> key = FileSystem.Utils.normalize_path!(path, type: :directory) + FileSystem.Utils.assert_dir_path!(path) + "/" <> key = path # S3 has no concept of directories, but keys with trailing # slash are interpreted as such, so we create an empty # object for the given key @@ -74,7 +77,7 @@ defimpl Livebook.FileSystem, for: Livebook.FileSystem.S3 do end def remove(file_system, path) do - "/" <> key = FileSystem.Utils.normalize_path!(path) + "/" <> key = path if FileSystem.Utils.dir_path?(path) do with {:ok, %{keys: keys}} <- list_objects(file_system, prefix: key) do @@ -90,9 +93,9 @@ defimpl Livebook.FileSystem, for: Livebook.FileSystem.S3 do end def copy(file_system, source_path, destination_path) do - "/" <> source_key = source_path = FileSystem.Utils.normalize_path!(source_path) - "/" <> destination_key = destination_path = FileSystem.Utils.normalize_path!(destination_path) FileSystem.Utils.assert_same_type!(source_path, destination_path) + "/" <> source_key = source_path + "/" <> destination_key = destination_path if FileSystem.Utils.dir_path?(source_path) do with {:ok, %{bucket: bucket, keys: keys}} <- list_objects(file_system, prefix: source_key) do @@ -123,9 +126,8 @@ defimpl Livebook.FileSystem, for: Livebook.FileSystem.S3 do end def rename(file_system, source_path, destination_path) do - source_path = FileSystem.Utils.normalize_path!(source_path) - "/" <> destination_key = destination_path = FileSystem.Utils.normalize_path!(destination_path) FileSystem.Utils.assert_same_type!(source_path, destination_path) + "/" <> destination_key = destination_path with {:ok, destination_exists?} <- object_exists(file_system, destination_key) do if destination_exists? do @@ -141,7 +143,8 @@ defimpl Livebook.FileSystem, for: Livebook.FileSystem.S3 do end def etag_for(file_system, path) do - "/" <> key = FileSystem.Utils.normalize_path!(path, type: :regular) + FileSystem.Utils.assert_regular_path!(path) + "/" <> key = path with {:ok, %{etag: etag}} <- head_object(file_system, key) do {:ok, etag} @@ -149,10 +152,14 @@ defimpl Livebook.FileSystem, for: Livebook.FileSystem.S3 do end def exists?(file_system, path) do - "/" <> key = FileSystem.Utils.normalize_path!(path) + "/" <> key = path object_exists(file_system, key) end + def resolve_path(_file_system, dir_path, subject) do + FileSystem.Utils.resolve_unix_like_path(dir_path, subject) + end + # Requests defp list_objects(file_system, opts) do diff --git a/lib/livebook/file_system/utils.ex b/lib/livebook/file_system/utils.ex index 5f7baed1114..1ba8f81d78a 100644 --- a/lib/livebook/file_system/utils.ex +++ b/lib/livebook/file_system/utils.ex @@ -4,46 +4,29 @@ defmodule Livebook.FileSystem.Utils do alias Livebook.FileSystem @doc """ - Asserts that the given path is absolute and expands it. - - ## Options - - * `:type` - if set to `:directory` or `:regular`, the - path type is also asserted against + Asserts that the given path is a directory. """ - @spec normalize_path!(String.t(), keyword()) :: FileSystem.path() - def normalize_path!(path, opts \\ []) do - unless absolute_path?(path) do - raise ArgumentError, "expected an absolute path, got: #{inspect(path)}" + @spec assert_dir_path!(FileSystem.path()) :: :ok + def assert_dir_path!(path) do + unless dir_path?(path) do + raise ArgumentError, "expected a directory path, got: #{inspect(path)}" end - path = expand_path(path) - - case opts[:type] do - nil -> - :ok - - :directory -> - unless dir_path?(path) do - raise ArgumentError, "expected a directory path, got: #{inspect(path)}" - end - - :regular -> - unless regular_path?(path) do - raise ArgumentError, "expected a regular file path, got: #{inspect(path)}" - end + :ok + end - other -> - raise ArgumentError, - "expected :type option to be either :direcotry, :regular or nil, got: #{inspect(other)}" + @doc """ + Asserts that the given path is a regular file path. + """ + @spec assert_regular_path!(FileSystem.path()) :: :ok + def assert_regular_path!(path) do + unless regular_path?(path) do + raise ArgumentError, "expected a regular file path, got: #{inspect(path)}" end - path + :ok end - defp absolute_path?("/" <> _), do: true - defp absolute_path?(_), do: false - @doc """ Checks if the given path describes a directory. """ @@ -73,35 +56,6 @@ defmodule Livebook.FileSystem.Utils do :ok end - @doc """ - Expands the given path, optionally against the given - absolute path. - - This works similarly to `Path.expand/2`, except it - takes trailing slashes into account. - """ - @spec expand_path(String.t(), FileSystem.path() | nil) :: FileSystem.path() - def expand_path(path, relative_to \\ nil) do - if not absolute_path?(path) and relative_to == nil do - raise ArgumentError, - "expected an absolute path to expand the relative path against, but none was given" - end - - relative_to = relative_to && normalize_path!(relative_to, type: :directory) - - if path == "" do - relative_to - else - dir? = dir_path?(path) or Path.basename(path) in [".", ".."] - - case {dir?, Path.expand(path, relative_to || "")} do - {_, "/"} -> "/" - {true, path} -> path <> "/" - {false, path} -> path - end - end - end - @doc """ Converts the given path into dir path by appending a trailing slash if necessary. @@ -123,4 +77,31 @@ defmodule Livebook.FileSystem.Utils do message = error |> :file.format_error() |> List.to_string() {:error, message} end + + @doc """ + Implements `Livebook.FileSystem.resolve_path` assuming Unix-like + path conventions. + + This function assumes absolute paths to have a leading "/" + and handles sequences such as "." and "..". + """ + @spec resolve_unix_like_path(FileSystem.path(), String.t()) :: FileSystem.t() + def resolve_unix_like_path(dir_path, subject) do + subject = + if Path.basename(subject) in [".", ".."] do + FileSystem.Utils.ensure_dir_path(subject) + else + subject + end + + absolute_path? = String.starts_with?(subject, "/") + path = if absolute_path?, do: subject, else: dir_path <> subject + path |> String.split("/") |> expand_parts([]) |> Enum.join("/") + end + + defp expand_parts([], acc), do: Enum.reverse(acc) + defp expand_parts(["." | parts], acc), do: expand_parts(parts, acc) + defp expand_parts([".." | parts], [_parent] = acc), do: expand_parts(parts, acc) + defp expand_parts([".." | parts], [_parent | acc]), do: expand_parts(parts, acc) + defp expand_parts([part | parts], acc), do: expand_parts(parts, [part | acc]) end diff --git a/lib/livebook/session.ex b/lib/livebook/session.ex index 7f133b988e8..f39321eb224 100644 --- a/lib/livebook/session.ex +++ b/lib/livebook/session.ex @@ -703,7 +703,7 @@ defmodule Livebook.Session do defp images_dir_from_state(%{data: %{file: nil}, session_id: id}) do tmp_dir = session_tmp_dir(id) - FileSystem.File.relative(tmp_dir, "images/") + FileSystem.File.resolve(tmp_dir, "images/") end defp images_dir_from_state(%{data: %{file: file}}) do @@ -717,7 +717,7 @@ defmodule Livebook.Session do def images_dir_for_notebook(file) do file |> FileSystem.File.containing_dir() - |> FileSystem.File.relative("images/") + |> FileSystem.File.resolve("images/") end defp session_tmp_dir(session_id) do @@ -772,7 +772,7 @@ defmodule Livebook.Session do Enum.reduce(images, :ok, fn {filename, content}, result -> with :ok <- result do - file = FileSystem.File.relative(images_dir, filename) + file = FileSystem.File.resolve(images_dir, filename) FileSystem.File.write(file, content) end end) diff --git a/lib/livebook_web/controllers/session_controller.ex b/lib/livebook_web/controllers/session_controller.ex index c70f862a8e3..98ce2f4026b 100644 --- a/lib/livebook_web/controllers/session_controller.ex +++ b/lib/livebook_web/controllers/session_controller.ex @@ -6,7 +6,7 @@ defmodule LivebookWeb.SessionController do def show_image(conn, %{"id" => id, "image" => image}) do if SessionSupervisor.session_exists?(id) do %{images_dir: images_dir} = Session.get_summary(id) - file = FileSystem.File.relative(images_dir, image) + file = FileSystem.File.resolve(images_dir, image) serve_static(conn, file) else send_resp(conn, 404, "Not found") diff --git a/lib/livebook_web/live/file_select_component.ex b/lib/livebook_web/live/file_select_component.ex index a24f3497e44..b03ece23ce7 100644 --- a/lib/livebook_web/live/file_select_component.ex +++ b/lib/livebook_web/live/file_select_component.ex @@ -337,7 +337,7 @@ defmodule LivebookWeb.FileSelectComponent do end def handle_event("set_path", %{"path" => path}, socket) do - file = FileSystem.File.new(socket.assigns.file.file_system) |> FileSystem.File.relative(path) + file = FileSystem.File.new(socket.assigns.file.file_system) |> FileSystem.File.resolve(path) info = socket.assigns.file_infos @@ -544,7 +544,7 @@ defmodule LivebookWeb.FileSelectComponent do defp create_dir(_parent_dir, ""), do: {:error, :ignore} defp create_dir(parent_dir, name) do - new_dir = FileSystem.File.relative(parent_dir, name <> "/") + new_dir = FileSystem.File.resolve(parent_dir, name <> "/") FileSystem.File.create_dir(new_dir) end @@ -557,7 +557,7 @@ defmodule LivebookWeb.FileSelectComponent do defp rename_file(file, name) do parent_dir = FileSystem.File.containing_dir(file) new_name = if FileSystem.File.dir?(file), do: name <> "/", else: name - new_file = FileSystem.File.relative(parent_dir, new_name) + new_file = FileSystem.File.resolve(parent_dir, new_name) FileSystem.File.rename(file, new_file) end end diff --git a/lib/livebook_web/live/session_live.ex b/lib/livebook_web/live/session_live.ex index e286b64c0b8..3afec4a061a 100644 --- a/lib/livebook_web/live/session_live.ex +++ b/lib/livebook_web/live/session_live.ex @@ -819,7 +819,7 @@ defmodule LivebookWeb.SessionLive do origin = case resolution_location do {:url, url} -> {:url, Livebook.Utils.expand_url(url, relative_path)} - {:file, file} -> {:file, FileSystem.File.relative(file, relative_path)} + {:file, file} -> {:file, FileSystem.File.resolve(file, relative_path)} end case session_id_by_location(origin) do diff --git a/lib/livebook_web/live/session_live/cell_upload_component.ex b/lib/livebook_web/live/session_live/cell_upload_component.ex index 624419ffeb4..168d1611755 100644 --- a/lib/livebook_web/live/session_live/cell_upload_component.ex +++ b/lib/livebook_web/live/session_live/cell_upload_component.ex @@ -77,7 +77,7 @@ defmodule LivebookWeb.SessionLive.CellUploadComponent do upload_file = FileSystem.File.local(path) ext = Path.extname(entry.client_name) filename = name <> ext - destination_file = FileSystem.File.relative(images_dir, filename) + destination_file = FileSystem.File.resolve(images_dir, filename) with :ok <- FileSystem.File.copy(upload_file, destination_file) do {:ok, filename} diff --git a/test/livebook/file_system/file_test.exs b/test/livebook/file_system/file_test.exs index 374b6dfac8e..4da94e0b547 100644 --- a/test/livebook/file_system/file_test.exs +++ b/test/livebook/file_system/file_test.exs @@ -9,22 +9,25 @@ defmodule Livebook.FileSystem.FileTest do test "raises an error when a relative path is given" do file_system = FileSystem.Local.new() - assert_raise ArgumentError, ~s{expected an absolute path, got: "file.txt"}, fn -> + assert_raise ArgumentError, ~s{expected an expanded absolute path, got: "file.txt"}, fn -> FileSystem.File.new(file_system, "file.txt") end end + test "raises an error when a unexpanded path is given" do + file_system = FileSystem.Local.new() + + assert_raise ArgumentError, + ~s{expected an expanded absolute path, got: "/dir/nested/../file.txt"}, + fn -> + FileSystem.File.new(file_system, "/dir/nested/../file.txt") + end + end + test "uses default file system path if non is given" do file_system = FileSystem.Local.new(default_path: "/dir/") assert %FileSystem.File{path: "/dir/"} = FileSystem.File.new(file_system) end - - test "expands the given path" do - file_system = FileSystem.Local.new() - - assert %FileSystem.File{path: "/dir/file.txt"} = - FileSystem.File.new(file_system, "/dir/nested/../file.txt") - end end describe "relative/2" do @@ -33,7 +36,7 @@ defmodule Livebook.FileSystem.FileTest do file = FileSystem.File.new(file_system, "/dir/nested/file.txt") assert %FileSystem.File{file_system: ^file_system, path: "/other/file.txt"} = - FileSystem.File.relative(file, "/other/file.txt") + FileSystem.File.resolve(file, "/other/file.txt") end test "resolves a relative path against a regular file" do @@ -41,7 +44,7 @@ defmodule Livebook.FileSystem.FileTest do file = FileSystem.File.new(file_system, "/dir/nested/file.txt") assert %FileSystem.File{file_system: ^file_system, path: "/dir/other/other_file.txt"} = - FileSystem.File.relative(file, "../other/other_file.txt") + FileSystem.File.resolve(file, "../other/other_file.txt") end test "resolves a relative path against a directory file" do @@ -49,7 +52,7 @@ defmodule Livebook.FileSystem.FileTest do dir = FileSystem.File.new(file_system, "/dir/nested/") assert %FileSystem.File{file_system: ^file_system, path: "/dir/nested/file.txt"} = - FileSystem.File.relative(dir, "file.txt") + FileSystem.File.resolve(dir, "file.txt") end test "resolves a relative directory path" do @@ -57,13 +60,13 @@ defmodule Livebook.FileSystem.FileTest do file = FileSystem.File.new(file_system, "/dir/nested/file.txt") assert %FileSystem.File{file_system: ^file_system, path: "/dir/other/"} = - FileSystem.File.relative(file, "../other/") + FileSystem.File.resolve(file, "../other/") assert %FileSystem.File{file_system: ^file_system, path: "/dir/nested/"} = - FileSystem.File.relative(file, ".") + FileSystem.File.resolve(file, ".") assert %FileSystem.File{file_system: ^file_system, path: "/dir/"} = - FileSystem.File.relative(file, "..") + FileSystem.File.resolve(file, "..") end end @@ -149,9 +152,9 @@ defmodule Livebook.FileSystem.FileTest do assert {:ok, files} = FileSystem.File.list(dir) assert files |> Enum.sort() == [ - FileSystem.File.relative(dir, "dir/"), - FileSystem.File.relative(dir, "file"), - FileSystem.File.relative(dir, "file.txt") + FileSystem.File.resolve(dir, "dir/"), + FileSystem.File.resolve(dir, "file"), + FileSystem.File.resolve(dir, "file.txt") ] end @@ -173,12 +176,12 @@ defmodule Livebook.FileSystem.FileTest do assert {:ok, files} = FileSystem.File.list(dir, recursive: true) assert files |> Enum.sort() == [ - FileSystem.File.relative(dir, "dir/"), - FileSystem.File.relative(dir, "dir/nested/"), - FileSystem.File.relative(dir, "dir/nested/double_nested/"), - FileSystem.File.relative(dir, "dir/nested/file.txt"), - FileSystem.File.relative(dir, "file"), - FileSystem.File.relative(dir, "file.txt") + FileSystem.File.resolve(dir, "dir/"), + FileSystem.File.resolve(dir, "dir/nested/"), + FileSystem.File.resolve(dir, "dir/nested/double_nested/"), + FileSystem.File.resolve(dir, "dir/nested/file.txt"), + FileSystem.File.resolve(dir, "file"), + FileSystem.File.resolve(dir, "file.txt") ] end end diff --git a/test/livebook/file_system/local_test.exs b/test/livebook/file_system/local_test.exs index 01ae4ef085c..a1fac8fea4d 100644 --- a/test/livebook/file_system/local_test.exs +++ b/test/livebook/file_system/local_test.exs @@ -484,4 +484,30 @@ defmodule Livebook.FileSystem.LocalTest do assert {:ok, true} = FileSystem.exists?(file_system, file_path) end end + + describe "FileSystem.resolve_path/3" do + test "resolves relative paths" do + file_system = Local.new() + + assert "/dir/" = FileSystem.resolve_path(file_system, "/dir/", "") + assert "/dir/file.txt" = FileSystem.resolve_path(file_system, "/dir/", "file.txt") + assert "/dir/nested/" = FileSystem.resolve_path(file_system, "/dir/", "nested/") + assert "/dir/" = FileSystem.resolve_path(file_system, "/dir/", ".") + assert "/" = FileSystem.resolve_path(file_system, "/dir/", "..") + + assert "/file.txt" = + FileSystem.resolve_path(file_system, "/dir/", "nested/../.././file.txt") + end + + test "resolves absolute paths" do + file_system = Local.new() + + assert "/" = FileSystem.resolve_path(file_system, "/dir/", "/") + assert "/file.txt" = FileSystem.resolve_path(file_system, "/dir/", "/file.txt") + assert "/nested/" = FileSystem.resolve_path(file_system, "/dir/", "/nested/") + + assert "/nested/file.txt" = + FileSystem.resolve_path(file_system, "/dir/", "/nested/other/../file.txt") + end + end end diff --git a/test/livebook/file_system/s3_test.exs b/test/livebook/file_system/s3_test.exs index 07b7f208e4f..9348c3b7766 100644 --- a/test/livebook/file_system/s3_test.exs +++ b/test/livebook/file_system/s3_test.exs @@ -646,6 +646,32 @@ defmodule Livebook.FileSystem.S3Test do end end + describe "FileSystem.resolve_path/3" do + test "resolves relative paths" do + file_system = S3.new("https://example.com/mybucket/", "key", "secret") + + assert "/dir/" = FileSystem.resolve_path(file_system, "/dir/", "") + assert "/dir/file.txt" = FileSystem.resolve_path(file_system, "/dir/", "file.txt") + assert "/dir/nested/" = FileSystem.resolve_path(file_system, "/dir/", "nested/") + assert "/dir/" = FileSystem.resolve_path(file_system, "/dir/", ".") + assert "/" = FileSystem.resolve_path(file_system, "/dir/", "..") + + assert "/file.txt" = + FileSystem.resolve_path(file_system, "/dir/", "nested/../.././file.txt") + end + + test "resolves absolute paths" do + file_system = S3.new("https://example.com/mybucket/", "key", "secret") + + assert "/" = FileSystem.resolve_path(file_system, "/dir/", "/") + assert "/file.txt" = FileSystem.resolve_path(file_system, "/dir/", "/file.txt") + assert "/nested/" = FileSystem.resolve_path(file_system, "/dir/", "/nested/") + + assert "/nested/file.txt" = + FileSystem.resolve_path(file_system, "/dir/", "/nested/other/../file.txt") + end + end + # Helpers defp bucket_url(port), do: "http://localhost:#{port}/mybucket" diff --git a/test/livebook/session_test.exs b/test/livebook/session_test.exs index 70cfcc82356..adf9a6e76ab 100644 --- a/test/livebook/session_test.exs +++ b/test/livebook/session_test.exs @@ -225,7 +225,7 @@ defmodule Livebook.SessionTest do pid = self() tmp_dir = FileSystem.File.local(tmp_dir <> "/") - file = FileSystem.File.relative(tmp_dir, "notebook.livemd") + file = FileSystem.File.resolve(tmp_dir, "notebook.livemd") Session.set_file(session_id, file) assert_receive {:operation, {:set_file, ^pid, ^file}} @@ -235,7 +235,7 @@ defmodule Livebook.SessionTest do test "broadcasts an error if the path is already in use", %{session_id: session_id, tmp_dir: tmp_dir} do tmp_dir = FileSystem.File.local(tmp_dir <> "/") - file = FileSystem.File.relative(tmp_dir, "notebook.livemd") + file = FileSystem.File.resolve(tmp_dir, "notebook.livemd") start_session(file: file) Phoenix.PubSub.subscribe(Livebook.PubSub, "sessions:#{session_id}") @@ -250,17 +250,17 @@ defmodule Livebook.SessionTest do tmp_dir = FileSystem.File.local(tmp_dir <> "/") %{images_dir: images_dir} = Session.get_summary(session_id) - image_file = FileSystem.File.relative(images_dir, "test.jpg") + image_file = FileSystem.File.resolve(images_dir, "test.jpg") :ok = FileSystem.File.write(image_file, "") - file = FileSystem.File.relative(tmp_dir, "notebook.livemd") + file = FileSystem.File.resolve(tmp_dir, "notebook.livemd") Session.set_file(session_id, file) # Wait for the session to deal with the files Process.sleep(50) assert {:ok, true} = - FileSystem.File.exists?(FileSystem.File.relative(tmp_dir, "images/test.jpg")) + FileSystem.File.exists?(FileSystem.File.resolve(tmp_dir, "images/test.jpg")) assert {:ok, false} = FileSystem.File.exists?(images_dir) end @@ -269,11 +269,11 @@ defmodule Livebook.SessionTest do test "does not remove images from the previous dir if not temporary", %{session_id: session_id, tmp_dir: tmp_dir} do tmp_dir = FileSystem.File.local(tmp_dir <> "/") - file = FileSystem.File.relative(tmp_dir, "notebook.livemd") + file = FileSystem.File.resolve(tmp_dir, "notebook.livemd") Session.set_file(session_id, file) %{images_dir: images_dir} = Session.get_summary(session_id) - image_file = FileSystem.File.relative(images_dir, "test.jpg") + image_file = FileSystem.File.resolve(images_dir, "test.jpg") :ok = FileSystem.File.write(image_file, "") Session.set_file(session_id, nil) @@ -286,7 +286,7 @@ defmodule Livebook.SessionTest do %{images_dir: new_images_dir} = Session.get_summary(session_id) assert {:ok, true} = - FileSystem.File.exists?(FileSystem.File.relative(new_images_dir, "test.jpg")) + FileSystem.File.exists?(FileSystem.File.resolve(new_images_dir, "test.jpg")) end end @@ -295,7 +295,7 @@ defmodule Livebook.SessionTest do test "persists the notebook to the associated file and notifies subscribers", %{session_id: session_id, tmp_dir: tmp_dir} do tmp_dir = FileSystem.File.local(tmp_dir <> "/") - file = FileSystem.File.relative(tmp_dir, "notebook.livemd") + file = FileSystem.File.resolve(tmp_dir, "notebook.livemd") Session.set_file(session_id, file) # Perform a change, so the notebook is dirty Session.set_notebook_name(session_id, "My notebook") @@ -313,7 +313,7 @@ defmodule Livebook.SessionTest do @tag :tmp_dir test "creates nonexistent directories", %{session_id: session_id, tmp_dir: tmp_dir} do tmp_dir = FileSystem.File.local(tmp_dir <> "/") - file = FileSystem.File.relative(tmp_dir, "nonexistent/dir/notebook.livemd") + file = FileSystem.File.resolve(tmp_dir, "nonexistent/dir/notebook.livemd") Session.set_file(session_id, file) # Perform a change, so the notebook is dirty Session.set_notebook_name(session_id, "My notebook") @@ -334,7 +334,7 @@ defmodule Livebook.SessionTest do test "saves the notebook and notifies subscribers once the session is closed", %{session_id: session_id, tmp_dir: tmp_dir} do tmp_dir = FileSystem.File.local(tmp_dir <> "/") - file = FileSystem.File.relative(tmp_dir, "notebook.livemd") + file = FileSystem.File.resolve(tmp_dir, "notebook.livemd") Session.set_file(session_id, file) # Perform a change, so the notebook is dirty Session.set_notebook_name(session_id, "My notebook") @@ -370,7 +370,7 @@ defmodule Livebook.SessionTest do @tag :tmp_dir test "fails if the given path is already in use", %{tmp_dir: tmp_dir} do tmp_dir = FileSystem.File.local(tmp_dir <> "/") - file = FileSystem.File.relative(tmp_dir, "notebook.livemd") + file = FileSystem.File.resolve(tmp_dir, "notebook.livemd") start_session(file: file) assert {:error, "the given file is already in use"} == @@ -381,7 +381,7 @@ defmodule Livebook.SessionTest do test "copies images when :copy_images_from option is specified", %{tmp_dir: tmp_dir} do tmp_dir = FileSystem.File.local(tmp_dir <> "/") - image_file = FileSystem.File.relative(tmp_dir, "image.jpg") + image_file = FileSystem.File.resolve(tmp_dir, "image.jpg") :ok = FileSystem.File.write(image_file, "") session_id = start_session(copy_images_from: tmp_dir) @@ -389,7 +389,7 @@ defmodule Livebook.SessionTest do %{images_dir: images_dir} = Session.get_summary(session_id) assert {:ok, true} = - FileSystem.File.exists?(FileSystem.File.relative(images_dir, "image.jpg")) + FileSystem.File.exists?(FileSystem.File.resolve(images_dir, "image.jpg")) end test "saves images when :images option is specified" do @@ -399,7 +399,7 @@ defmodule Livebook.SessionTest do %{images_dir: images_dir} = Session.get_summary(session_id) - assert FileSystem.File.relative(images_dir, "image.jpg") |> FileSystem.File.read() == + assert FileSystem.File.resolve(images_dir, "image.jpg") |> FileSystem.File.read() == {:ok, "binary content"} end end diff --git a/test/livebook_web/controllers/session_controller_test.exs b/test/livebook_web/controllers/session_controller_test.exs index e71adc5d3a2..0dd8965be0d 100644 --- a/test/livebook_web/controllers/session_controller_test.exs +++ b/test/livebook_web/controllers/session_controller_test.exs @@ -25,7 +25,7 @@ defmodule LivebookWeb.SessionControllerTest do test "returns the image when it does exist", %{conn: conn} do {:ok, session_id} = SessionSupervisor.create_session() %{images_dir: images_dir} = Session.get_summary(session_id) - :ok = FileSystem.File.relative(images_dir, "test.jpg") |> FileSystem.File.write("") + :ok = FileSystem.File.resolve(images_dir, "test.jpg") |> FileSystem.File.write("") conn = get(conn, Routes.session_path(conn, :show_image, session_id, "test.jpg")) diff --git a/test/livebook_web/live/session_live_test.exs b/test/livebook_web/live/session_live_test.exs index fac29d8f848..ca02a0dacaa 100644 --- a/test/livebook_web/live/session_live_test.exs +++ b/test/livebook_web/live/session_live_test.exs @@ -464,8 +464,8 @@ defmodule LivebookWeb.SessionLiveTest do test "renders an error message when the relative notebook does not exist", %{conn: conn, session_id: session_id, tmp_dir: tmp_dir} do tmp_dir = FileSystem.File.local(tmp_dir <> "/") - index_file = FileSystem.File.relative(tmp_dir, "index.livemd") - notebook_file = FileSystem.File.relative(tmp_dir, "notebook.livemd") + index_file = FileSystem.File.resolve(tmp_dir, "index.livemd") + notebook_file = FileSystem.File.resolve(tmp_dir, "notebook.livemd") Session.set_file(session_id, index_file) wait_for_session_update(session_id) @@ -485,8 +485,8 @@ defmodule LivebookWeb.SessionLiveTest do test "opens a relative notebook if it exists", %{conn: conn, session_id: session_id, tmp_dir: tmp_dir} do tmp_dir = FileSystem.File.local(tmp_dir <> "/") - index_file = FileSystem.File.relative(tmp_dir, "index.livemd") - notebook_file = FileSystem.File.relative(tmp_dir, "notebook.livemd") + index_file = FileSystem.File.resolve(tmp_dir, "index.livemd") + notebook_file = FileSystem.File.resolve(tmp_dir, "notebook.livemd") Session.set_file(session_id, index_file) wait_for_session_update(session_id) @@ -508,8 +508,8 @@ defmodule LivebookWeb.SessionLiveTest do test "if the current session has no path, forks the relative notebook", %{conn: conn, tmp_dir: tmp_dir} do tmp_dir = FileSystem.File.local(tmp_dir <> "/") - index_file = FileSystem.File.relative(tmp_dir, "index.livemd") - notebook_file = FileSystem.File.relative(tmp_dir, "notebook.livemd") + index_file = FileSystem.File.resolve(tmp_dir, "index.livemd") + notebook_file = FileSystem.File.resolve(tmp_dir, "notebook.livemd") {:ok, session_id} = SessionSupervisor.create_session(origin: {:file, index_file}) @@ -531,8 +531,8 @@ defmodule LivebookWeb.SessionLiveTest do test "if the notebook is already open, redirects to the session", %{conn: conn, session_id: session_id, tmp_dir: tmp_dir} do tmp_dir = FileSystem.File.local(tmp_dir <> "/") - index_file = FileSystem.File.relative(tmp_dir, "index.livemd") - notebook_file = FileSystem.File.relative(tmp_dir, "notebook.livemd") + index_file = FileSystem.File.resolve(tmp_dir, "index.livemd") + notebook_file = FileSystem.File.resolve(tmp_dir, "notebook.livemd") Session.set_file(session_id, index_file) wait_for_session_update(session_id) @@ -549,9 +549,9 @@ defmodule LivebookWeb.SessionLiveTest do @tag :tmp_dir test "handles nested paths", %{conn: conn, session_id: session_id, tmp_dir: tmp_dir} do tmp_dir = FileSystem.File.local(tmp_dir <> "/") - parent_file = FileSystem.File.relative(tmp_dir, "parent.livemd") - child_dir = FileSystem.File.relative(tmp_dir, "dir/") - child_file = FileSystem.File.relative(child_dir, "child.livemd") + parent_file = FileSystem.File.resolve(tmp_dir, "parent.livemd") + child_dir = FileSystem.File.resolve(tmp_dir, "dir/") + child_file = FileSystem.File.resolve(child_dir, "child.livemd") Session.set_file(session_id, parent_file) wait_for_session_update(session_id) @@ -569,9 +569,9 @@ defmodule LivebookWeb.SessionLiveTest do @tag :tmp_dir test "handles parent paths", %{conn: conn, session_id: session_id, tmp_dir: tmp_dir} do tmp_dir = FileSystem.File.local(tmp_dir <> "/") - parent_file = FileSystem.File.relative(tmp_dir, "parent.livemd") - child_dir = FileSystem.File.relative(tmp_dir, "dir/") - child_file = FileSystem.File.relative(child_dir, "child.livemd") + parent_file = FileSystem.File.resolve(tmp_dir, "parent.livemd") + child_dir = FileSystem.File.resolve(tmp_dir, "dir/") + child_file = FileSystem.File.resolve(child_dir, "child.livemd") Session.set_file(session_id, child_file) wait_for_session_update(session_id) From 13fed68239e9ac42e00d06ecd593e93815b72511 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jonatan=20K=C5=82osko?= Date: Fri, 13 Aug 2021 15:40:56 +0200 Subject: [PATCH 3/8] Remove accidental notebook file --- .livemd | 3 --- 1 file changed, 3 deletions(-) delete mode 100644 .livemd diff --git a/.livemd b/.livemd deleted file mode 100644 index c32196c3ae7..00000000000 --- a/.livemd +++ /dev/null @@ -1,3 +0,0 @@ - - -# Untitled notebook From 257ee0d4a2725337e34960ed1ede9470902b09ce Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jonatan=20K=C5=82osko?= Date: Fri, 13 Aug 2021 15:42:09 +0200 Subject: [PATCH 4/8] Apply suggestions from code review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: José Valim --- lib/livebook/file_system/s3/signature.ex | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/livebook/file_system/s3/signature.ex b/lib/livebook/file_system/s3/signature.ex index 2aa2107f364..589dc3e02da 100644 --- a/lib/livebook/file_system/s3/signature.ex +++ b/lib/livebook/file_system/s3/signature.ex @@ -83,13 +83,13 @@ defmodule Livebook.FileSystem.S3.Signature do defp canonical_headers(headers) do headers |> Enum.map(fn {name, value} -> - name = String.downcase(name) |> String.trim() + name = String.downcase(name, :ascii) |> String.trim() value = String.trim(value) {name, value} end) |> Enum.sort(fn {a, _}, {b, _} -> a <= b end) |> Enum.map(fn {name, value} -> [name, ":", value, "\n"] end) - |> Enum.join() + |> IO.iodata_to_binary() end # Process and merge request values into a canonical request for AWS signature From 9def0e9d7d71ad13f4b3f87de3e5f60b35eb5c8e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jonatan=20K=C5=82osko?= Date: Fri, 13 Aug 2021 15:47:41 +0200 Subject: [PATCH 5/8] Apply review comments --- lib/livebook/file_system.ex | 3 +++ lib/livebook/file_system/s3.ex | 2 +- lib/livebook/file_system/s3/signature.ex | 1 + 3 files changed, 5 insertions(+), 1 deletion(-) diff --git a/lib/livebook/file_system.ex b/lib/livebook/file_system.ex index 32afdb07cf0..370c7788869 100644 --- a/lib/livebook/file_system.ex +++ b/lib/livebook/file_system.ex @@ -40,6 +40,9 @@ defprotocol Livebook.FileSystem do @doc """ Returns a list of files located in the given directory. + + When `recursive` is set to `true`, nested directories + are traversed and the final list includes all the paths. """ @spec list(t(), path(), boolean()) :: {:ok, list(path())} | {:error, error()} def list(file_system, path, recursive) diff --git a/lib/livebook/file_system/s3.ex b/lib/livebook/file_system/s3.ex index f80204007c2..8b05f7e6cd2 100644 --- a/lib/livebook/file_system/s3.ex +++ b/lib/livebook/file_system/s3.ex @@ -333,7 +333,7 @@ defimpl Livebook.FileSystem, for: Livebook.FileSystem.S3 do region: region_from_uri(file_system.bucket_url) } - now = NaiveDateTime.utc_now() |> NaiveDateTime.truncate(:second) + now = NaiveDateTime.utc_now() headers = [{"Host", host} | headers] diff --git a/lib/livebook/file_system/s3/signature.ex b/lib/livebook/file_system/s3/signature.ex index 589dc3e02da..247b9ff8f6c 100644 --- a/lib/livebook/file_system/s3/signature.ex +++ b/lib/livebook/file_system/s3/signature.ex @@ -16,6 +16,7 @@ defmodule Livebook.FileSystem.S3.Signature do request using the specified time. """ def sign_v4(credentials, now, method, url, headers, body) do + now = NaiveDateTime.truncate(now, :second) long_date = NaiveDateTime.to_iso8601(now, :basic) <> "Z" short_date = Date.to_iso8601(now, :basic) From 2870c186405739f0249698c11926eea8d52dbfbd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jonatan=20K=C5=82osko?= Date: Fri, 13 Aug 2021 16:48:58 +0200 Subject: [PATCH 6/8] Add missing path assertions --- lib/livebook/file_system/local.ex | 18 +++++++++++------- lib/livebook/file_system/utils.ex | 2 ++ 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/lib/livebook/file_system/local.ex b/lib/livebook/file_system/local.ex index a28b1850c77..2eb21fc0ff0 100644 --- a/lib/livebook/file_system/local.ex +++ b/lib/livebook/file_system/local.ex @@ -170,16 +170,20 @@ defimpl Livebook.FileSystem, for: Livebook.FileSystem.Local do end end - def resolve_path(_file_system, dir_path, ""), do: dir_path - def resolve_path(_file_system, dir_path, subject) do - dir? = FileSystem.Utils.dir_path?(subject) or Path.basename(subject) in [".", ".."] - expanded_path = Path.expand(subject, dir_path) + FileSystem.Utils.assert_dir_path!(dir_path) - if dir? do - FileSystem.Utils.ensure_dir_path(expanded_path) + if subject == "" do + dir_path else - expanded_path + dir? = FileSystem.Utils.dir_path?(subject) or Path.basename(subject) in [".", ".."] + expanded_path = Path.expand(subject, dir_path) + + if dir? do + FileSystem.Utils.ensure_dir_path(expanded_path) + else + expanded_path + end end end end diff --git a/lib/livebook/file_system/utils.ex b/lib/livebook/file_system/utils.ex index 1ba8f81d78a..8ffa08b0823 100644 --- a/lib/livebook/file_system/utils.ex +++ b/lib/livebook/file_system/utils.ex @@ -87,6 +87,8 @@ defmodule Livebook.FileSystem.Utils do """ @spec resolve_unix_like_path(FileSystem.path(), String.t()) :: FileSystem.t() def resolve_unix_like_path(dir_path, subject) do + assert_dir_path!(dir_path) + subject = if Path.basename(subject) in [".", ".."] do FileSystem.Utils.ensure_dir_path(subject) From 4ecd47e45546f46dc4e915f2820518d565835851 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jonatan=20K=C5=82osko?= Date: Fri, 13 Aug 2021 20:44:44 +0200 Subject: [PATCH 7/8] Apply review comments --- lib/livebook/file_system/utils.ex | 17 +++++++++++++++-- .../live/session_live/persistence_live.ex | 5 +++++ test/livebook/file_system/local_test.exs | 2 +- test/livebook/file_system/s3_test.exs | 2 +- 4 files changed, 22 insertions(+), 4 deletions(-) diff --git a/lib/livebook/file_system/utils.ex b/lib/livebook/file_system/utils.ex index 8ffa08b0823..8bdfbfd6dc2 100644 --- a/lib/livebook/file_system/utils.ex +++ b/lib/livebook/file_system/utils.ex @@ -91,16 +91,29 @@ defmodule Livebook.FileSystem.Utils do subject = if Path.basename(subject) in [".", ".."] do - FileSystem.Utils.ensure_dir_path(subject) + ensure_dir_path(subject) else subject end absolute_path? = String.starts_with?(subject, "/") path = if absolute_path?, do: subject, else: dir_path <> subject - path |> String.split("/") |> expand_parts([]) |> Enum.join("/") + + path + |> String.split("/") + |> remove_in_middle("") + |> expand_parts([]) + |> Enum.join("/") end + defp remove_in_middle([], _elem), do: [] + defp remove_in_middle([head], _elem), do: [head] + defp remove_in_middle([head | tail], elem), do: remove_in_middle(tail, elem, [head]) + + defp remove_in_middle([head], _elem, acc), do: Enum.reverse([head | acc]) + defp remove_in_middle([elem | tail], elem, acc), do: remove_in_middle(tail, elem, acc) + defp remove_in_middle([head | tail], elem, acc), do: remove_in_middle(tail, elem, [head | acc]) + defp expand_parts([], acc), do: Enum.reverse(acc) defp expand_parts(["." | parts], acc), do: expand_parts(parts, acc) defp expand_parts([".." | parts], [_parent] = acc), do: expand_parts(parts, acc) diff --git a/lib/livebook_web/live/session_live/persistence_live.ex b/lib/livebook_web/live/session_live/persistence_live.ex index c7d41856903..659680ef83e 100644 --- a/lib/livebook_web/live/session_live/persistence_live.ex +++ b/lib/livebook_web/live/session_live/persistence_live.ex @@ -1,4 +1,9 @@ defmodule LivebookWeb.SessionLive.PersistenceLive do + # TODO: rewrite this live view as a component, once live_view + # has a unified way of sending events programatically from a child + # component to parent live view or component. Currently we send an + # event to self() from FileSelectComponent and use handle_info in + # the parent live view. use LivebookWeb, :live_view alias Livebook.{Session, SessionSupervisor, LiveMarkdown, FileSystem} diff --git a/test/livebook/file_system/local_test.exs b/test/livebook/file_system/local_test.exs index a1fac8fea4d..3403cd2c5aa 100644 --- a/test/livebook/file_system/local_test.exs +++ b/test/livebook/file_system/local_test.exs @@ -507,7 +507,7 @@ defmodule Livebook.FileSystem.LocalTest do assert "/nested/" = FileSystem.resolve_path(file_system, "/dir/", "/nested/") assert "/nested/file.txt" = - FileSystem.resolve_path(file_system, "/dir/", "/nested/other/../file.txt") + FileSystem.resolve_path(file_system, "/dir/", "///nested///other/..///file.txt") end end end diff --git a/test/livebook/file_system/s3_test.exs b/test/livebook/file_system/s3_test.exs index 9348c3b7766..7d1fdff135d 100644 --- a/test/livebook/file_system/s3_test.exs +++ b/test/livebook/file_system/s3_test.exs @@ -668,7 +668,7 @@ defmodule Livebook.FileSystem.S3Test do assert "/nested/" = FileSystem.resolve_path(file_system, "/dir/", "/nested/") assert "/nested/file.txt" = - FileSystem.resolve_path(file_system, "/dir/", "/nested/other/../file.txt") + FileSystem.resolve_path(file_system, "/dir/", "///nested///other/..///file.txt") end end From 4ed089051bd222853ab2e26370d300d465ca2414 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jonatan=20K=C5=82osko?= Date: Fri, 13 Aug 2021 20:53:09 +0200 Subject: [PATCH 8/8] Fix test saving notebook in project root --- test/livebook_web/live/session_live_test.exs | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/test/livebook_web/live/session_live_test.exs b/test/livebook_web/live/session_live_test.exs index ca02a0dacaa..d8855b9300a 100644 --- a/test/livebook_web/live/session_live_test.exs +++ b/test/livebook_web/live/session_live_test.exs @@ -204,8 +204,8 @@ defmodule LivebookWeb.SessionLiveTest do end end - @tag :tmp_dir describe "persistence settings" do + @tag :tmp_dir test "saving to file shows the newly created file", %{conn: conn, session_id: session_id, tmp_dir: tmp_dir} do {:ok, view, _} = live(conn, "/sessions/#{session_id}/settings/file") @@ -231,15 +231,23 @@ defmodule LivebookWeb.SessionLiveTest do |> has_element?() end - test "changing output persistence updates data", %{conn: conn, session_id: session_id} do + @tag :tmp_dir + test "changing output persistence updates data", + %{conn: conn, session_id: session_id, tmp_dir: tmp_dir} do {:ok, view, _} = live(conn, "/sessions/#{session_id}/settings/file") assert view = find_live_child(view, "persistence") + path = Path.join(tmp_dir, "notebook.livemd") + view |> element("button", "Save to file") |> render_click() + view + |> element(~s{form[phx-change="set_path"]}) + |> render_change(%{path: path}) + view |> element(~s{form[phx-change="set_options"]}) |> render_change(%{persist_outputs: "true"})