Improve integration tests and fix some intemittent tests (#3033)

Co-authored-by: Jonatan Kłosko <jonatanklosko@gmail.com>

Author: aleDsz

lib/livebook/apps.ex

@@ -76,6 +76,10 @@ defmodule Livebook.Apps do
 
   @doc """
   Returns if the given running app is authorized to given user.
+
+  Teams apps have authorization rules that can restrict access on
+  per-user basis. This kind of authorization is not applicable to
+  other types of apps, for those this function always returns `true`.
   """
   @spec authorized?(App.t(), Livebook.Users.User.t()) :: boolean()
   def authorized?(app, user)
@@ -86,6 +90,8 @@ defmodule Livebook.Apps do
     Livebook.Hubs.TeamClient.user_app_access?(id, user.groups, slug)
   end
 
+  def authorized?(_app, _user), do: true
+
   @doc """
   Updates the given app info across the cluster.
   """

lib/livebook/hubs/team.ex

@@ -103,14 +103,13 @@ defmodule Livebook.Hubs.Team do
 end
 
 defimpl Livebook.Hubs.Provider, for: Livebook.Hubs.Team do
-  alias Livebook.Hubs.Team
-  alias Livebook.Hubs.TeamClient
+  alias Livebook.Hubs.{Team, TeamClient}
   alias Livebook.Teams.Requests
   alias Livebook.FileSystem
   alias Livebook.Secrets.Secret
 
   @teams_key_prefix Livebook.Teams.Org.teams_key_prefix()
-  @public_key_prefix Livebook.Hubs.Team.public_key_prefix()
+  @public_key_prefix Team.public_key_prefix()
 
   def load(team, fields) do
     {offline?, fields} = Map.pop(fields, :offline?, false)

lib/livebook/hubs/team_client.ex

@@ -63,6 +63,8 @@ defmodule Livebook.Hubs.TeamClient do
   @spec get_secrets(String.t()) :: list(Secrets.Secret.t())
   def get_secrets(id) do
     GenServer.call(registry_name(id), :get_secrets)
+  catch
+    :exit, _ -> []
   end
 
   @doc """
@@ -71,6 +73,8 @@ defmodule Livebook.Hubs.TeamClient do
   @spec get_file_systems(String.t()) :: list(FileSystem.t())
   def get_file_systems(id) do
     GenServer.call(registry_name(id), :get_file_systems)
+  catch
+    :exit, _ -> []
   end
 
   @doc """
@@ -106,6 +110,8 @@ defmodule Livebook.Hubs.TeamClient do
   @spec get_agent_app_deployments(String.t()) :: list(Teams.AppDeployment.t())
   def get_agent_app_deployments(id) do
     GenServer.call(registry_name(id), :get_agent_app_deployments)
+  catch
+    :exit, _ -> []
   end
 
   @doc """
@@ -137,7 +143,7 @@ defmodule Livebook.Hubs.TeamClient do
   @doc """
   Returns a list of cached environment variables.
   """
-  @spec get_environment_variables(String.t()) :: list(Teams.Agent.t())
+  @spec get_environment_variables(String.t()) :: list(Teams.EnvironmentVariable.t())
   def get_environment_variables(id) do
     GenServer.call(registry_name(id), :get_environment_variables)
   end

lib/livebook/teams.ex

@@ -197,25 +197,17 @@ defmodule Livebook.Teams do
   end
 
   @doc """
-  Creates a new app deployment.
+  Deploys the given app deployment.
   """
   @spec deploy_app(Team.t(), Teams.AppDeployment.t()) ::
           :ok
           | {:error, Ecto.Changeset.t()}
           | {:transport_error, String.t()}
   def deploy_app(%Team{} = team, %Teams.AppDeployment{} = app_deployment) do
     case Requests.deploy_app(team, app_deployment) do
-      {:ok, %{"id" => _id}} ->
-        :ok
-
-      {:error, %{"errors" => %{"detail" => error}}} ->
-        {:error, add_external_errors(app_deployment, %{"file" => [error]})}
-
-      {:error, %{"errors" => errors}} ->
-        {:error, add_external_errors(app_deployment, errors)}
-
-      any ->
-        any
+      {:ok, %{"id" => _id}} -> :ok
+      {:error, %{"errors" => errors}} -> {:error, add_external_errors(app_deployment, errors)}
+      any -> any
     end
   end
 

lib/livebook/teams/app_deployment.ex

@@ -42,11 +42,27 @@ defmodule Livebook.Teams.AppDeployment do
   @doc """
   Creates a new app deployment from notebook.
   """
-  @spec new(Livebook.Notebook.t(), Livebook.FileSystem.File.t()) ::
+  @spec new(Livebook.Notebook.t() | String.t(), Livebook.FileSystem.File.t()) ::
           {:ok, t()} | {:warning, list(String.t())} | {:error, FileSystem.error()}
-  def new(notebook, files_dir) do
-    with {:ok, source} <- fetch_notebook_source(notebook),
-         {:ok, files} <- build_and_check_file_entries(notebook, source, files_dir),
+  def new(%Livebook.Notebook{} = notebook, files_dir) do
+    case Livebook.LiveMarkdown.notebook_to_livemd(notebook) do
+      {source, []} -> new(notebook, source, files_dir)
+      {_, warnings} -> {:warning, warnings}
+    end
+  end
+
+  @stamp_error "notebook does not have a stamp, disabling access to secrets and remote files"
+
+  def new(source, files_dir) when is_binary(source) do
+    case Livebook.LiveMarkdown.notebook_from_livemd(source) do
+      {notebook, %{warnings: [], stamp_verified?: true}} -> new(notebook, source, files_dir)
+      {_, %{warnings: [], stamp_verified?: false}} -> {:warning, [@stamp_error]}
+      {_, %{warnings: warnings}} -> {:warning, warnings}
+    end
+  end
+
+  def new(%Livebook.Notebook{} = notebook, source, files_dir) do
+    with {:ok, files} <- build_and_check_file_entries(notebook, source, files_dir),
          {:ok, {_, zip_content}} <- :zip.create(~c"app_deployment.zip", files, [:memory]),
          :ok <- validate_size(zip_content) do
       md5_hash = :crypto.hash(:md5, zip_content)
@@ -66,13 +82,6 @@ defmodule Livebook.Teams.AppDeployment do
     end
   end
 
-  defp fetch_notebook_source(notebook) do
-    case Livebook.LiveMarkdown.notebook_to_livemd(notebook) do
-      {source, []} -> {:ok, source}
-      {_, warnings} -> {:warning, warnings}
-    end
-  end
-
   defp build_and_check_file_entries(notebook, source, files_dir) do
     notebook.file_entries
     |> Enum.filter(&(&1.type == :attachment))

lib/livebook/teams/connection.ex

@@ -85,15 +85,15 @@ defmodule Livebook.Teams.Connection do
     :keep_state_and_data
   end
 
-  def handle_event(:info, message, @no_state, data) when elem(message, 0) in @expected_messages do
-    handle_websocket_message(message, data)
-  end
-
   def handle_event(:info, message, @no_state, %{http_conn: nil})
       when elem(message, 0) in @expected_messages do
     :keep_state_and_data
   end
 
+  def handle_event(:info, message, @no_state, data) when elem(message, 0) in @expected_messages do
+    handle_websocket_message(message, data)
+  end
+
   def handle_event(:info, _message, @no_state, _data) do
     :keep_state_and_data
   end
@@ -116,6 +116,10 @@ defmodule Livebook.Teams.Connection do
 
   # Private
 
+  defp handle_websocket_message(_message, %{http_conn: nil} = data) do
+    {:keep_state, data, {:next_event, :internal, :connect}}
+  end
+
   defp handle_websocket_message(message, data) do
     case WebSocket.receive(data.http_conn, data.ref, data.websocket, message) do
       {:ok, conn, websocket, binaries} ->