Fixing some issues with Git file system (#3079)

Author: aleDsz

lib/livebook/file_system/git.ex

@@ -116,7 +116,13 @@ defimpl Livebook.FileSystem, for: Livebook.FileSystem.Git do
 
   def write_stream_halt(_file_system, _state), do: raise("not implemented")
 
-  def read_stream_into(_file_system, _path, _collectable), do: raise("not implemented")
+  def read_stream_into(file_system, path, collectable) do
+    try do
+      Git.Client.stream_file(file_system, path, collectable)
+    rescue
+      error in File.Error -> FileSystem.Utils.posix_error(error.reason)
+    end
+  end
 
   def load(file_system, %{"hub_id" => _} = fields) do
     load(file_system, %{

lib/livebook/file_system/git/client.ex

@@ -43,7 +43,20 @@ defmodule Livebook.FileSystem.Git.Client do
     path = relative_path(path)
 
     with {:ok, git_dir} <- fetch_repository(file_system) do
-      show(git_dir, file_system.branch, path)
+      git(git_dir, ["show", "#{file_system.branch}:#{path}"])
+    end
+  end
+
+  @doc """
+  Streams the content of the given file from given repository.
+  """
+  @spec stream_file(FileSystem.Git.t(), String.t(), Collectable.t()) ::
+          {:ok, String.t()} | {:error, FileSystem.error()}
+  def stream_file(%FileSystem.Git{} = file_system, path, collectable) do
+    path = relative_path(path)
+
+    with {:ok, git_dir} <- fetch_repository(file_system) do
+      git(git_dir, ["show", "#{file_system.branch}:#{path}"], into: collectable)
     end
   end
 
@@ -90,10 +103,6 @@ defmodule Livebook.FileSystem.Git.Client do
     end
   end
 
-  defp show(git_dir, branch, path) do
-    git(git_dir, ["show", "#{branch}:#{path}"])
-  end
-
   defp rev_parse(git_dir, branch, path) do
     with {:ok, etag} <- git(git_dir, ["rev-parse", "#{branch}:#{path}"]) do
       {:ok, String.trim(etag)}

lib/livebook_web/live/file_select_component.ex

@@ -499,8 +499,8 @@ defmodule LivebookWeb.FileSelectComponent do
 
   def handle_event("set_file_system", %{"id" => file_system_id}, socket) do
     file_system = Enum.find(socket.assigns.file_systems, &(&1.id == file_system_id))
-
     file = FileSystem.File.new(file_system)
+    :ok = FileSystem.mount(file_system)
 
     send_event(socket.assigns.target, {:set_file, file, %{exists: true}})
 

test/livebook/file_system/git_test.exs

@@ -51,6 +51,7 @@ defmodule Livebook.FileSystem.GitTest do
 
   describe "FileSystem.read/2" do
     @describetag init: true
+
     test "returns an error when a nonexistent key is given", %{file_system: file_system} do
       assert FileSystem.read(file_system, "/another_file.txt") ==
                {:error, "fatal: path 'another_file.txt' does not exist in 'main'"}
@@ -104,6 +105,7 @@ defmodule Livebook.FileSystem.GitTest do
 
   describe "FileSystem.etag_for/2" do
     @describetag init: true
+
     test "returns an error when a nonexistent key is given", %{file_system: file_system} do
       assert {:error, reason} = FileSystem.etag_for(file_system, "/another_file.txt")
       assert reason =~ "path 'another_file.txt' does not exist in 'main'"
@@ -116,6 +118,7 @@ defmodule Livebook.FileSystem.GitTest do
 
   describe "FileSystem.exists?/2" do
     @describetag init: true
+
     test "returns valid response", %{file_system: file_system} do
       assert {:ok, true} = FileSystem.exists?(file_system, "/file.txt")
       assert {:ok, false} = FileSystem.exists?(file_system, "/another_file.txt")
@@ -158,10 +161,27 @@ defmodule Livebook.FileSystem.GitTest do
   end
 
   describe "FileSystem.read_stream_into/2" do
-    test "not implemented", %{file_system: file_system} do
-      assert_raise RuntimeError, "not implemented", fn ->
-        FileSystem.read_stream_into(file_system, "/file.txt", <<>>)
-      end
+    @describetag init: true
+
+    test "returns an error when a nonexistent key is given", %{file_system: file_system} do
+      assert FileSystem.read_stream_into(file_system, "/another_file.txt", <<>>) ==
+               {:error, "fatal: path 'another_file.txt' does not exist in 'main'"}
+    end
+
+    test "returns object contents under the given key", %{file_system: file_system} do
+      assert {:ok, content} = FileSystem.read_stream_into(file_system, "/file.txt", <<>>)
+      assert content =~ "git file storage works"
+    end
+
+    @tag :tmp_dir
+    test "collects file contents into another file", %{file_system: file_system, tmp_dir: tmp_dir} do
+      file_path = Path.join(tmp_dir, "myfile.txt")
+      collectable = FileSystem.File.new(FileSystem.Local.new(), file_path)
+
+      assert {:ok, %FileSystem.File{path: ^file_path}} =
+               FileSystem.read_stream_into(file_system, "/file.txt", collectable)
+
+      assert File.read!(file_path) =~ "git file storage works"
     end
   end
 

test/support/integration/teams_rpc.ex

@@ -98,12 +98,13 @@ defmodule Livebook.TeamsRPC do
         hub_id: team.id
     }
 
-    attrs = %{
+    attrs = [
       name: name,
       type: String.to_atom(type),
       value: Livebook.HubHelpers.generate_file_system_json(team, file_system),
-      org_key: org_key
-    }
+      org_key: org_key,
+      livebook_version: Livebook.Config.app_version()
+    ]
 
     external_id = :erpc.call(node, TeamsRPC, :create_file_system, [attrs]).id
     Map.replace!(file_system, :external_id, to_string(external_id))