Resolve links for imported notebooks (#445)

* Resolve links for imported notebooks

* Clarify error messages

lib/livebook/content_loader.ex

@@ -55,8 +55,23 @@ 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}} ->

lib/livebook/session.ex

@@ -22,7 +22,7 @@ defmodule Livebook.Session do
   @type state :: %{
           session_id: id(),
           data: Data.t(),
-          runtime_monitor_ref: reference()
+          runtime_monitor_ref: reference() | nil
         }
 
   @type summary :: %{
@@ -30,7 +30,8 @@ defmodule Livebook.Session do
           pid: pid(),
           notebook_name: String.t(),
           path: String.t() | nil,
-          images_dir: String.t()
+          images_dir: String.t(),
+          origin_url: String.t() | nil
         }
 
   @typedoc """
@@ -52,6 +53,9 @@ 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
+
     * `:path` - the file to which the notebook should be saved
 
     * `:copy_images_from` - a directory path to copy notebook images from
@@ -322,10 +326,12 @@ defmodule Livebook.Session do
   end
 
   defp init_data(opts) do
-    notebook = Keyword.get(opts, :notebook)
+    notebook = opts[:notebook]
-    path = Keyword.get(opts, :path)
+    path = opts[:path]
+    origin_url = opts[:origin_url]
 
     data = if(notebook, do: Data.new(notebook), else: Data.new())
+    data = %{data | origin_url: origin_url}
 
     if path do
       case FileGuard.lock(path, self()) do
@@ -579,7 +585,8 @@ defmodule Livebook.Session do
       pid: self(),
       notebook_name: state.data.notebook.name,
       path: state.data.path,
-      images_dir: images_dir_from_state(state)
+      images_dir: images_dir_from_state(state),
+      origin_url: state.data.origin_url
     }
   end
 

lib/livebook/session/data.ex

@@ -16,6 +16,7 @@ defmodule Livebook.Session.Data do
 
   defstruct [
     :notebook,
+    :origin_url,
     :path,
     :dirty,
     :section_infos,
@@ -32,6 +33,7 @@ defmodule Livebook.Session.Data do
 
   @type t :: %__MODULE__{
           notebook: Notebook.t(),
+          origin_url: String.t() | nil,
           path: nil | String.t(),
           dirty: boolean(),
           section_infos: %{Section.id() => section_info()},
@@ -126,6 +128,7 @@ defmodule Livebook.Session.Data do
   def new(notebook \\ Notebook.new()) do
     %__MODULE__{
       notebook: notebook,
+      origin_url: nil,
       path: nil,
       dirty: false,
       section_infos: initial_section_infos(notebook),

lib/livebook/utils.ex

@@ -121,18 +121,59 @@ defmodule Livebook.Utils do
 
   ## Examples
 
-    iex> Livebook.Utils.valid_hex_color?("#111111")
+      iex> Livebook.Utils.valid_hex_color?("#111111")
-    true
+      true
 
-    iex> Livebook.Utils.valid_hex_color?("#ABC123")
+      iex> Livebook.Utils.valid_hex_color?("#ABC123")
-    true
+      true
 
-    iex> Livebook.Utils.valid_hex_color?("ABCDEF")
+      iex> Livebook.Utils.valid_hex_color?("ABCDEF")
-    false
+      false
 
-    iex> Livebook.Utils.valid_hex_color?("#111")
+      iex> Livebook.Utils.valid_hex_color?("#111")
-    false
+      false
   """
   @spec valid_hex_color?(String.t()) :: boolean()
   def valid_hex_color?(hex_color), do: hex_color =~ ~r/^#[0-9a-fA-F]{6}$/
+
+  @doc """
+  Changes the first letter in the given string to upper case.
+
+  ## Examples
+
+      iex> Livebook.Utils.upcase_first("sippin tea")
+      "Sippin tea"
+
+      iex> Livebook.Utils.upcase_first("short URL")
+      "Short URL"
+
+      iex> Livebook.Utils.upcase_first("")
+      ""
+  """
+  @spec upcase_first(String.t()) :: String.t()
+  def upcase_first(string) do
+    {first, rest} = String.split_at(string, 1)
+    String.upcase(first) <> rest
+  end
+
+  @doc """
+  Expands a relative path in terms of the given URL.
+
+  ## Examples
+
+      iex> Livebook.Utils.expand_url("file:///home/user/lib/file.ex", "../root.ex")
+      "file:///home/user/root.ex"
+
+      iex> Livebook.Utils.expand_url("https://example.com/lib/file.ex?token=supersecret", "../root.ex")
+      "https://example.com/root.ex?token=supersecret"
+  """
+  @spec expand_url(String.t(), String.t()) :: String.t()
+  def expand_url(url, relative_path) do
+    url
+    |> URI.parse()
+    |> Map.update!(:path, fn path ->
+      path |> Path.dirname() |> Path.join(relative_path) |> Path.expand()
+    end)
+    |> URI.to_string()
+  end
 end

lib/livebook_web/live/home_live.ex

@@ -229,24 +229,47 @@ defmodule LivebookWeb.HomeLive do
   end
 
   def handle_event("fork", %{}, socket) do
-    {notebook, messages} = import_notebook(socket.assigns.path)
+    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(socket.assigns.path)
+    images_dir = Session.images_dir_for_notebook(path)
-    {:noreply, create_session(socket, notebook: notebook, copy_images_from: images_dir)}
+
+    {:noreply,
+     create_session(socket,
+       notebook: notebook,
+       copy_images_from: images_dir,
+       origin_url: "file://" <> path
+     )}
   end
 
   def handle_event("open", %{}, socket) do
-    {notebook, messages} = import_notebook(socket.assigns.path)
+    path = socket.assigns.path
+    {notebook, messages} = import_notebook(path)
     socket = put_import_flash_messages(socket, messages)
-    {:noreply, create_session(socket, notebook: notebook, path: socket.assigns.path)}
+
+    {:noreply,
+     create_session(socket, notebook: notebook, path: path, origin_url: "file://" <> path)}
   end
 
   def handle_event("fork_session", %{"id" => session_id}, socket) do
     data = Session.get_data(session_id)
     notebook = Notebook.forked(data.notebook)
     %{images_dir: images_dir} = Session.get_summary(session_id)
-    {:noreply, create_session(socket, notebook: notebook, copy_images_from: images_dir)}
+
+    origin_url =
+      if data.path do
+        "file://" <> data.path
+      else
+        data.origin_url
+      end
+
+    {:noreply,
+     create_session(socket,
+       notebook: notebook,
+       copy_images_from: images_dir,
+       origin_url: origin_url
+     )}
   end
 
   @impl true
@@ -261,10 +284,11 @@ defmodule LivebookWeb.HomeLive do
     {:noreply, assign(socket, session_summaries: session_summaries)}
   end
 
-  def handle_info({:import_content, content}, socket) do
+  def handle_info({:import_content, content, session_opts}, socket) do
     {notebook, messages} = Livebook.LiveMarkdown.Import.notebook_from_markdown(content)
     socket = put_import_flash_messages(socket, messages)
-    {:noreply, create_session(socket, notebook: notebook)}
+    session_opts = Keyword.merge(session_opts, notebook: notebook)
+    {:noreply, create_session(socket, session_opts)}
   end
 
   def handle_info(

lib/livebook_web/live/home_live/import_content_component.ex

@@ -37,7 +37,7 @@ defmodule LivebookWeb.HomeLive.ImportContentComponent do
   end
 
   def handle_event("import", %{"data" => %{"content" => content}}, socket) do
-    send(self(), {:import_content, content})
+    send(self(), {:import_content, content, []})
 
     {:noreply, socket}
   end

lib/livebook_web/live/home_live/import_url_component.ex

@@ -50,11 +50,11 @@ defmodule LivebookWeb.HomeLive.ImportUrlComponent do
     |> ContentLoader.fetch_content()
     |> case do
       {:ok, content} ->
-        send(self(), {:import_content, content})
+        send(self(), {:import_content, content, [origin_url: url]})
         {:noreply, socket}
 
       {:error, message} ->
-        {:noreply, assign(socket, error_message: String.capitalize(message))}
+        {:noreply, assign(socket, error_message: Utils.upcase_first(message))}
     end
   end
 end

lib/livebook_web/live/session_live.ex

@@ -733,58 +733,104 @@ defmodule LivebookWeb.SessionLive do
 
       true ->
         socket
-        |> push_patch(to: Routes.session_path(socket, :page, socket.assigns.session_id))
         |> put_flash(
           :error,
           "Got unrecognised session path: #{path}\nIf you want to link another notebook, make sure to include the .livemd extension"
         )
+        |> redirect_to_self()
     end
   end
 
   defp handle_relative_notebook_path(socket, relative_path) do
-    case socket.private.data.path do
+    resolution_url = location(socket.private.data)
+
+    case resolution_url do
       nil ->
         socket
         |> put_flash(
           :info,
-          "Cannot resolve notebook path #{relative_path}, because the current notebook has no file"
+          "Cannot resolve notebook path #{relative_path}, because the current notebook has no location"
         )
-        |> push_patch(to: Routes.session_path(socket, :page, socket.assigns.session_id))
+        |> redirect_to_self()
 
-      path ->
+      url ->
-        target_path = path |> Path.dirname() |> Path.join(relative_path) |> Path.expand()
+        target_url = Livebook.Utils.expand_url(url, relative_path)
-        maybe_open_notebook(socket, target_path)
+
+        case session_id_by_url(target_url) do
+          {:ok, session_id} ->
+            push_redirect(socket, to: Routes.session_path(socket, :page, session_id))
+
+          {:error, :none} ->
+            open_notebook(socket, target_url)
+
+          {:error, :many} ->
+            socket
+            |> put_flash(
+              :error,
+              "Cannot navigate, because multiple sessions were found for #{target_url}"
+            )
+            |> redirect_to_self()
+        end
     end
   end
 
-  defp maybe_open_notebook(socket, path) do
+  defp location(data)
-    if session_id = session_id_by_path(path) do
+  defp location(%{path: path}) when is_binary(path), do: "file://" <> path
-      push_redirect(socket, to: Routes.session_path(socket, :page, session_id))
+  defp location(%{origin_url: origin_url}), do: origin_url
+
+  defp open_notebook(socket, url) do
+    url
+    |> Livebook.ContentLoader.rewrite_url()
+    |> Livebook.ContentLoader.fetch_content()
+    |> case 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)
+
+        socket
+        |> put_import_flash_messages(messages)
+        |> create_session(notebook: notebook, origin_url: url, path: path)
+
+      {:error, message} ->
+        socket
+        |> put_flash(:error, "Cannot navigate, " <> message)
+        |> redirect_to_self()
+    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 session_id_by_url(url) do
+    session_summaries = SessionSupervisor.get_session_summaries()
+
+    session_with_path =
+      Enum.find(session_summaries, fn summary ->
+        summary.path && "file://" <> summary.path == url
+      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}
     else
-      case File.read(path) do
+      session_summaries
-        {:ok, content} ->
+      |> Enum.filter(fn summary -> summary.origin_url == url end)
-          {notebook, messages} = LiveMarkdown.Import.notebook_from_markdown(content)
+      |> case do
-
+        [summary] -> {:ok, summary.session_id}
-          socket
+        [] -> {:error, :none}
-          |> put_import_flash_messages(messages)
+        _ -> {:error, :many}
-          |> create_session(notebook: notebook, path: path)
-
-        {:error, error} ->
-          message = :file.format_error(error)
-
-          socket
-          |> put_flash(:error, "Failed to open #{path}, reason: #{message}")
-          |> push_patch(to: Routes.session_path(socket, :page, socket.assigns.session_id))
       end
     end
   end
 
-  defp session_id_by_path(path) do
+  defp redirect_to_self(socket) do
-    session_summaries = SessionSupervisor.get_session_summaries()
+    push_patch(socket, to: Routes.session_path(socket, :page, socket.assigns.session_id))
-
-    Enum.find_value(session_summaries, fn summary ->
-      summary.path == path && summary.session_id
-    end)
   end
 
   defp after_operation(socket, _prev_socket, {:client_join, client_pid, user}) do

test/livebook/content_loader_test.exs

@@ -25,7 +25,27 @@ defmodule Livebook.ContentLoaderTest do
     end
   end
 
-  describe "fetch_content/1" do
+  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
     setup do
       bypass = Bypass.open()
       {:ok, bypass: bypass}

test/livebook_web/live/session_live_test.exs

@@ -417,7 +417,7 @@ defmodule LivebookWeb.SessionLiveTest do
       assert render(view) =~ "Got unrecognised session path: document.pdf"
     end
 
-    test "renders an info message when the session has no associated path",
+    test "renders an info message when the session has neither original url nor path",
          %{conn: conn, session_id: session_id} do
       session_path = "/sessions/#{session_id}"
 
@@ -427,7 +427,7 @@ defmodule LivebookWeb.SessionLiveTest do
       {:ok, view, _} = follow_redirect(result, conn)
 
       assert render(view) =~
-               "Cannot resolve notebook path notebook.livemd, because the current notebook has no file"
+               "Cannot resolve notebook path notebook.livemd, because the current notebook has no location"
     end
 
     @tag :tmp_dir
@@ -447,7 +447,7 @@ defmodule LivebookWeb.SessionLiveTest do
       {:ok, view, _} = follow_redirect(result, conn)
 
       assert render(view) =~
-               "Failed to open #{notebook_path}, reason: no such file or directory"
+               "Cannot navigate, failed to read #{notebook_path}, reason: no such file or directory"
     end
 
     @tag :tmp_dir
@@ -461,12 +461,37 @@ defmodule LivebookWeb.SessionLiveTest do
 
       File.write!(notebook_path, "# Sibling notebook")
 
-      assert {:error, {:live_redirect, %{to: _session_path}}} =
+      assert {:error, {:live_redirect, %{to: new_session_path}}} =
                result = live(conn, "/sessions/#{session_id}/notebook.livemd")
 
       {:ok, view, _} = follow_redirect(result, conn)
-
       assert render(view) =~ "Sibling notebook"
+
+      "/sessions/" <> session_id = new_session_path
+      data = Session.get_data(session_id)
+      assert data.path == notebook_path
+    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")
+
+      {:ok, session_id} = SessionSupervisor.create_session(origin_url: "file://" <> index_path)
+
+      File.write!(notebook_path, "# Sibling notebook")
+
+      assert {:error, {:live_redirect, %{to: new_session_path}}} =
+               result = live(conn, "/sessions/#{session_id}/notebook.livemd")
+
+      {:ok, view, _} = follow_redirect(result, conn)
+      assert render(view) =~ "Sibling notebook - fork"
+
+      "/sessions/" <> session_id = new_session_path
+      data = Session.get_data(session_id)
+      assert data.path == nil
+      assert data.origin_url == "file://" <> notebook_path
     end
 
     @tag :tmp_dir
@@ -526,6 +551,80 @@ defmodule LivebookWeb.SessionLiveTest do
 
       assert render(view) =~ "Parent notebook"
     end
+
+    test "resolves remote URLs", %{conn: conn} do
+      bypass = Bypass.open()
+
+      Bypass.expect_once(bypass, "GET", "/notebook.livemd", fn conn ->
+        conn
+        |> Plug.Conn.put_resp_content_type("text/plain")
+        |> Plug.Conn.resp(200, "# My notebook")
+      end)
+
+      index_url = url(bypass.port) <> "/index.livemd"
+      {:ok, session_id} = SessionSupervisor.create_session(origin_url: index_url)
+
+      {:ok, view, _} =
+        conn
+        |> live("/sessions/#{session_id}/notebook.livemd")
+        |> follow_redirect(conn)
+
+      assert render(view) =~ "My notebook"
+    end
+
+    test "renders an error message if relative remote notebook cannot be loaded", %{conn: conn} do
+      bypass = Bypass.open()
+
+      Bypass.expect_once(bypass, "GET", "/notebook.livemd", fn conn ->
+        Plug.Conn.resp(conn, 500, "Error")
+      end)
+
+      index_url = url(bypass.port) <> "/index.livemd"
+
+      {:ok, session_id} = SessionSupervisor.create_session(origin_url: index_url)
+
+      session_path = "/sessions/#{session_id}"
+
+      assert {:error, {:live_redirect, %{to: ^session_path}}} =
+               result = live(conn, "/sessions/#{session_id}/notebook.livemd")
+
+      {:ok, view, _} = follow_redirect(result, conn)
+      assert render(view) =~ "Cannot navigate, failed to download notebook from the given URL"
+    end
+
+    test "if the remote notebook is already imported, redirects to the session",
+         %{conn: conn, test: test} 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)
+
+      notebook_session_path = "/sessions/#{notebook_session_id}"
+
+      assert {:error, {:live_redirect, %{to: ^notebook_session_path}}} =
+               live(conn, "/sessions/#{index_session_id}/notebook.livemd")
+    end
+
+    test "renders an error message if there are already multiple session imported from the relative URL",
+         %{conn: conn, test: test} 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)
+
+      index_session_path = "/sessions/#{index_session_id}"
+
+      assert {:error, {:live_redirect, %{to: ^index_session_path}}} =
+               result = live(conn, "/sessions/#{index_session_id}/notebook.livemd")
+
+      {:ok, view, _} = follow_redirect(result, conn)
+
+      assert render(view) =~
+               "Cannot navigate, because multiple sessions were found for #{notebook_url}"
+    end
   end
 
   # Helpers
@@ -572,4 +671,6 @@ defmodule LivebookWeb.SessionLiveTest do
     {:ok, user} = User.new() |> User.change(%{"name" => name})
     user
   end
+
+  defp url(port), do: "http://localhost:#{port}"
 end