From 49492771f392b8263bf663ed8bc9191ec5e07778 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Fri, 13 May 2022 14:41:05 +0200 Subject: [PATCH] Ignore asset directories if they are empty macOS removes files from the temporary directory but keeps the directory. This would cause LiveView to misbehave as it would assume all assets are checked out. --- lib/livebook/session.ex | 13 ++++++++++--- .../controllers/session_controller_test.exs | 16 ++++++++++++++++ 2 files changed, 26 insertions(+), 3 deletions(-) diff --git a/lib/livebook/session.ex b/lib/livebook/session.ex index 3502c3dc2..a8d81951b 100644 --- a/lib/livebook/session.ex +++ b/lib/livebook/session.ex @@ -209,14 +209,14 @@ defmodule Livebook.Session do def fetch_assets(pid, hash) do local_assets_path = local_assets_path(hash) - if File.exists?(local_assets_path) do + if non_empty_dir?(local_assets_path) do :ok else with {:ok, runtime, archive_path} <- GenServer.call(pid, {:get_runtime_and_archive_path, hash}, @timeout) do fun = fn -> # Make sure the file hasn't been fetched by this point - unless File.exists?(local_assets_path) do + unless non_empty_dir?(local_assets_path) do {:ok, archive_binary} = Runtime.read_file(runtime, archive_path) extract_archive!(archive_binary, local_assets_path) end @@ -232,6 +232,10 @@ defmodule Livebook.Session do end end + defp non_empty_dir?(path) do + match?({:ok, [_ | _]}, File.ls(path)) + end + @doc """ Sends notebook attributes update to the server. """ @@ -1067,7 +1071,10 @@ defmodule Livebook.Session do FileSystem.File.remove(tmp_dir) end - defp local_assets_path(hash) do + @doc """ + Returns a local path to the directory for all assets for hash. + """ + def local_assets_path(hash) do Path.join([livebook_tmp_path(), "assets", encode_path_component(hash)]) end diff --git a/test/livebook_web/controllers/session_controller_test.exs b/test/livebook_web/controllers/session_controller_test.exs index 70d2815d8..3bc801f29 100644 --- a/test/livebook_web/controllers/session_controller_test.exs +++ b/test/livebook_web/controllers/session_controller_test.exs @@ -152,6 +152,22 @@ defmodule LivebookWeb.SessionControllerTest do assert redirected_to(conn, 301) == Routes.session_path(conn, :show_cached_asset, hash, ["main.js"]) + + {:ok, asset_path} = Session.local_asset_path(hash, "main.js") + assert File.exists?(asset_path) + end + + test "fetches assets and redirects even on empty asset directories", %{conn: conn} do + %{notebook: notebook, hash: hash} = notebook_with_js_output() + assets_path = Session.local_assets_path(hash) + File.mkdir_p!(assets_path) + + conn = start_session_and_request_asset(conn, notebook, hash) + + assert redirected_to(conn, 301) == + Routes.session_path(conn, :show_cached_asset, hash, ["main.js"]) + + assert File.exists?(Path.join(assets_path, "main.js")) end test "skips the session if assets are in cache", %{conn: conn} do