Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Split files into references/attachments and support renaming #2167

Merged
merged 1 commit into from
Aug 17, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions assets/css/js_interop.css
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,10 @@ solely client-side operations.
@apply hidden;
}

[data-el-session][data-js-dragging="external"] [data-el-files-add-button] {
@apply hidden;
}

[data-el-cell][data-js-focused] {
@apply border-blue-300 border-opacity-100;
}
Expand Down
6 changes: 6 additions & 0 deletions lib/livebook/runtime.ex
Original file line number Diff line number Diff line change
Expand Up @@ -553,6 +553,12 @@ defprotocol Livebook.Runtime do
@spec transfer_file(t(), String.t(), String.t(), (path :: String.t() | nil -> any())) :: :ok
def transfer_file(runtime, path, file_id, callback)

@doc """
Updates the id by which the file is referenced.
"""
@spec relabel_file(t(), String.t(), String.t()) :: :ok
def relabel_file(runtime, file_id, new_file_id)

@doc """
Cleans up resources allocated with `transfer_file/4`, if any.
"""
Expand Down
4 changes: 4 additions & 0 deletions lib/livebook/runtime/attached.ex
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,10 @@ defimpl Livebook.Runtime, for: Livebook.Runtime.Attached do
RuntimeServer.read_file(runtime.server_pid, path)
end

def relabel_file(runtime, file_id, new_file_id) do
RuntimeServer.relabel_file(runtime.server_pid, file_id, new_file_id)
end

def transfer_file(runtime, path, file_id, callback) do
RuntimeServer.transfer_file(runtime.server_pid, path, file_id, callback)
end
Expand Down
4 changes: 4 additions & 0 deletions lib/livebook/runtime/elixir_standalone.ex
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,10 @@ defimpl Livebook.Runtime, for: Livebook.Runtime.ElixirStandalone do
RuntimeServer.transfer_file(runtime.server_pid, path, file_id, callback)
end

def relabel_file(runtime, file_id, new_file_id) do
RuntimeServer.relabel_file(runtime.server_pid, file_id, new_file_id)
end

def revoke_file(runtime, file_id) do
RuntimeServer.revoke_file(runtime.server_pid, file_id)
end
Expand Down
4 changes: 4 additions & 0 deletions lib/livebook/runtime/embedded.ex
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,10 @@ defimpl Livebook.Runtime, for: Livebook.Runtime.Embedded do
RuntimeServer.transfer_file(runtime.server_pid, path, file_id, callback)
end

def relabel_file(runtime, file_id, new_file_id) do
RuntimeServer.relabel_file(runtime.server_pid, file_id, new_file_id)
end

def revoke_file(runtime, file_id) do
RuntimeServer.revoke_file(runtime.server_pid, file_id)
end
Expand Down
22 changes: 22 additions & 0 deletions lib/livebook/runtime/erl_dist/runtime_server.ex
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,20 @@ defmodule Livebook.Runtime.ErlDist.RuntimeServer do
:ok
end

@doc """
Changes the file id set by `transfer_file/4`, if any.

See `Livebook.Runtime` for more details.
"""
@spec relabel_file(pid(), String.t(), String.t()) :: :ok
def relabel_file(pid, file_id, new_file_id) do
unless same_host?(pid) do
GenServer.cast(pid, {:relabel_file, file_id, new_file_id})
end

:ok
end

@doc """
Removes the file created by `transfer_file/4`, if any.

Expand Down Expand Up @@ -614,6 +628,14 @@ defmodule Livebook.Runtime.ErlDist.RuntimeServer do
{:noreply, state}
end

def handle_cast({:relabel_file, file_id, new_file_id}, state) do
path = file_path(state, file_id)
new_path = file_path(state, new_file_id)
File.rename(path, new_path)

{:noreply, state}
end

def handle_cast({:revoke_file, file_id}, state) do
target_path = file_path(state, file_id)
File.rm(target_path)
Expand Down
57 changes: 52 additions & 5 deletions lib/livebook/session.ex
Original file line number Diff line number Diff line change
Expand Up @@ -615,6 +615,14 @@ defmodule Livebook.Session do
GenServer.cast(pid, {:add_file_entries, self(), file_entries})
end

@doc """
Sends a file entry rename request to the server.
"""
@spec rename_file_entry(pid(), String.t(), String.t()) :: :ok
def rename_file_entry(pid, name, new_name) do
GenServer.cast(pid, {:rename_file_entry, self(), name, new_name})
end

@doc """
Sends a file entry deletion request to the server.
"""
Expand Down Expand Up @@ -1380,6 +1388,12 @@ defmodule Livebook.Session do
{:noreply, handle_operation(state, operation)}
end

def handle_cast({:rename_file_entry, client_pid, name, new_name}, state) do
client_id = client_id(state, client_pid)
operation = {:rename_file_entry, client_id, name, new_name}
{:noreply, handle_operation(state, operation)}
end

def handle_cast({:delete_file_entry, client_pid, name}, state) do
client_id = client_id(state, client_pid)
operation = {:delete_file_entry, client_id, name}
Expand Down Expand Up @@ -2136,14 +2150,26 @@ defmodule Livebook.Session do
notify_update(state)
end

defp after_operation(state, prev_state, {:add_file_entries, _client_id, _file_entries}) do
defp after_operation(state, prev_state, {:add_file_entries, _client_id, file_entries}) do
names = for entry <- file_entries, do: entry.name, into: MapSet.new()

replaced_names =
Enum.map(
prev_state.data.notebook.file_entries -- state.data.notebook.file_entries,
& &1.name
)
for file_entry <- prev_state.data.notebook.file_entries,
file_entry.name in names,
do: file_entry.name

cleanup_file_entries(state, replaced_names)
state
end

defp after_operation(state, prev_state, {:rename_file_entry, _client_id, name, new_name}) do
replaced_names =
for file_entry <- prev_state.data.notebook.file_entries,
file_entry.name == new_name,
do: file_entry.name

cleanup_file_entries(state, replaced_names)
remap_file_entry(state, name, new_name)
state
end

Expand Down Expand Up @@ -2650,6 +2676,27 @@ defmodule Livebook.Session do
end
end

defp remap_file_entry(state, name, new_name) do
cache_file = file_entry_cache_file(state.session_id, name)
new_cache_file = file_entry_cache_file(state.session_id, new_name)
FileSystem.File.rename(cache_file, new_cache_file)

file_entry = Enum.find(state.data.notebook.file_entries, &(&1.name == new_name))

if file_entry.type == :attachment do
files_dir = files_dir_from_state(state)
file = FileSystem.File.resolve(files_dir, name)
new_file = FileSystem.File.resolve(files_dir, new_name)
FileSystem.File.rename(file, new_file)
end

if Runtime.connected?(state.data.runtime) do
file_id = file_entry_file_id(name)
new_file_id = file_entry_file_id(new_name)
Runtime.relabel_file(state.data.runtime, file_id, new_file_id)
end
end

defp before_close(state) do
maybe_save_notebook_sync(state)
broadcast_message(state.session_id, :session_closed)
Expand Down
46 changes: 46 additions & 0 deletions lib/livebook/session/data.ex
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,7 @@ defmodule Livebook.Session.Data do
| {:set_notebook_hub, client_id(), String.t()}
| {:sync_hub_secrets, client_id()}
| {:add_file_entries, client_id(), list(Notebook.file_entry())}
| {:rename_file_entry, client_id(), name :: String.t(), new_name :: String.t()}
| {:delete_file_entry, client_id(), String.t()}
| {:allow_file_entry, client_id(), String.t()}
| {:set_app_settings, client_id(), AppSettings.t()}
Expand Down Expand Up @@ -913,6 +914,19 @@ defmodule Livebook.Session.Data do
|> wrap_ok()
end

def apply_operation(data, {:rename_file_entry, _client_id, name, new_name}) do
with {:ok, file_entry} <- fetch_file_entry(data.notebook, name),
true <- name != new_name do
data
|> with_actions()
|> rename_file_entry(file_entry, new_name)
|> set_dirty()
|> wrap_ok()
else
_ -> :error
end
end

def apply_operation(data, {:delete_file_entry, _client_id, name}) do
with {:ok, file_entry} <- fetch_file_entry(data.notebook, name) do
data
Expand Down Expand Up @@ -1712,6 +1726,38 @@ defmodule Livebook.Session.Data do
|> set!(notebook: %{data.notebook | file_entries: file_entries ++ kept_file_entries})
end

defp rename_file_entry({data, _} = data_actions, file_entry, new_name) do
file_entries =
for entry <- data.notebook.file_entries, entry.name != new_name do
if entry == file_entry do
%{entry | name: new_name}
else
entry
end
end

quarantine_file_entry_names =
MapSet.delete(data.notebook.quarantine_file_entry_names, new_name)

quarantine_file_entry_names =
if MapSet.member?(quarantine_file_entry_names, file_entry.name) do
quarantine_file_entry_names
|> MapSet.delete(file_entry.name)
|> MapSet.put(new_name)
else
quarantine_file_entry_names
end

data_actions
|> set!(
notebook: %{
data.notebook
| file_entries: file_entries,
quarantine_file_entry_names: quarantine_file_entry_names
}
)
end

defp delete_file_entry({data, _} = data_actions, file_entry) do
file_entries = data.notebook.file_entries -- [file_entry]

Expand Down
24 changes: 24 additions & 0 deletions lib/livebook_web/live/session_helpers.ex
Original file line number Diff line number Diff line change
Expand Up @@ -248,4 +248,28 @@ defmodule LivebookWeb.SessionHelpers do
confirm_icon: "close-circle-line"
)
end

@doc """
Converts the given arbitrary name to a file entry name.

The returned name is either valid or empty.
"""
@spec sanitize_file_entry_name(String.t()) :: String.t() | nil
def sanitize_file_entry_name(client_name) do
client_name
|> String.replace(~r/[^\s\w-.]/u, "")
|> String.trim()
|> String.replace(~r/\s+/u, "_")
|> case do
"" ->
""

name ->
if String.contains?(name, ".") do
name
else
name <> ".bin"
end
end
end
end
24 changes: 24 additions & 0 deletions lib/livebook_web/live/session_live.ex
Original file line number Diff line number Diff line change
Expand Up @@ -491,6 +491,21 @@ defmodule LivebookWeb.SessionLive do
<.add_file_entry_content session={@session} file_entries={@data_view.file_entries} tab={@tab} />
</.modal>

<.modal
:if={@live_action == :rename_file_entry}
id="rename-file-entry-modal"
show
width={:big}
patch={@self_path}
>
<.live_component
module={LivebookWeb.SessionLive.RenameFileEntryComponent}
id="rename-file-entry"
session={@session}
file_entry={@renaming_file_entry}
/>
</.modal>

<.modal
:if={@live_action == :shortcuts}
id="shortcuts-modal"
Expand Down Expand Up @@ -989,6 +1004,15 @@ defmodule LivebookWeb.SessionLive do
{:noreply, redirect_to_self(socket)}
end

def handle_params(%{"name" => name}, _url, socket)
when socket.assigns.live_action == :rename_file_entry do
if file_entry = find_file_entry(socket, name) do
{:noreply, assign(socket, renaming_file_entry: file_entry)}
else
{:noreply, redirect_to_self(socket)}
end
end

def handle_params(%{"path_parts" => path_parts}, requested_url, socket)
when socket.assigns.live_action == :catch_all do
path_parts =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ defmodule LivebookWeb.SessionLive.AddFileEntryFileComponent do
def update(%{event: {:set_file, file, info}}, socket) do
file_info = %{exists: info.exists}
name = if FileSystem.File.dir?(file), do: "", else: FileSystem.File.name(file)
name = LivebookWeb.SessionHelpers.sanitize_file_entry_name(name)
changeset = changeset(Map.put(socket.assigns.changeset.params, "name", name))
{:ok, assign(socket, file: file, file_info: file_info, changeset: changeset)}
end
Expand Down Expand Up @@ -84,7 +85,7 @@ defmodule LivebookWeb.SessionLive.AddFileEntryFileComponent do
field={f[:copy]}
options={[
{"false", "Store only file location"},
{"true", "Copy file contents to the notebook files directory"}
{"true", "Save file as an attachment in the notebook files directory"}
]}
/>
</div>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,8 @@ defmodule LivebookWeb.SessionLive.AddFileEntryUploadComponent do
|> JS.dispatch("blur", to: "#add-file-entry-form-name")
)

{%{data | "name" => entry.client_name}, socket}
name = LivebookWeb.SessionHelpers.sanitize_file_entry_name(entry.client_name)
{%{data | "name" => name}, socket}

_ ->
{data, socket}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ defmodule LivebookWeb.SessionLive.AddFileEntryUrlComponent do
field={f[:copy]}
options={[
{"false", "Store only URL location"},
{"true", "Download URL contents to the notebook files directory"}
{"true", "Download file as an attachment to the notebook files directory"}
]}
/>
</div>
Expand Down Expand Up @@ -104,7 +104,10 @@ defmodule LivebookWeb.SessionLive.AddFileEntryUrlComponent do
end

def handle_event("url_blur", %{"value" => url}, socket) do
name = Livebook.Utils.url_basename(url)
name =
url
|> Livebook.Utils.url_basename()
|> LivebookWeb.SessionHelpers.sanitize_file_entry_name()

socket =
if socket.assigns.changeset.params["name"] == "" and name != "" do
Expand Down
Loading
Loading