More improvements for Secrets and Hubs (#1693)

lib/livebook/hubs.ex

@@ -3,6 +3,7 @@ defmodule Livebook.Hubs do
 
   alias Livebook.Storage
   alias Livebook.Hubs.{Broadcasts, Enterprise, Fly, Local, Metadata, Provider}
+  alias Livebook.Secrets
   alias Livebook.Secrets.Secret
 
   @namespace :hubs
@@ -39,7 +40,7 @@ defmodule Livebook.Hubs do
   @spec get_metadatas() :: list(Metadata.t())
   def get_metadatas do
     for hub <- get_hubs() do
-      %{Provider.normalize(hub) | connected?: Provider.connected?(hub)}
+      Provider.to_metadata(hub)
     end
   end
 
@@ -128,7 +129,6 @@ defmodule Livebook.Hubs do
     * `:hub_connected`
     * `:hub_disconnected`
     * `{:hub_connection_failed, reason}`
-    * `{:hub_disconnection_failed, reason}`
 
   Topic `hubs:secrets`:
 
@@ -192,7 +192,7 @@ defmodule Livebook.Hubs do
   end
 
   defp connect_hub(hub) do
-    if child_spec = Provider.connect(hub) do
+    if child_spec = Provider.connection_spec(hub) do
       DynamicSupervisor.start_child(Livebook.HubsSupervisor, child_spec)
     end
 
@@ -221,11 +221,11 @@ defmodule Livebook.Hubs do
         if capability?(hub, [:secrets]) do
           Provider.create_secret(hub, secret)
         else
-          {:error, %{errors: [{"hub_id", {"is invalid", []}}]}}
+          {:error, Secrets.add_secret_error(secret, :origin, "is invalid")}
         end
 
       :error ->
-        {:error, %{errors: [{"hub_id", {"doest not exists", []}}]}}
+        {:error, Secrets.add_secret_error(secret, :origin, "is invalid")}
     end
   end
 

lib/livebook/hubs/enterprise.ex

@@ -119,22 +119,19 @@ defimpl Livebook.Hubs.Provider, for: Livebook.Hubs.Enterprise do
     }
   end
 
-  def normalize(enterprise) do
+  def to_metadata(enterprise) do
     %Livebook.Hubs.Metadata{
       id: enterprise.id,
       name: enterprise.hub_name,
       provider: enterprise,
-      emoji: enterprise.hub_emoji
+      emoji: enterprise.hub_emoji,
+      connected?: EnterpriseClient.connected?(enterprise.id)
     }
   end
 
   def type(_enterprise), do: "enterprise"
 
-  def connect(enterprise), do: {EnterpriseClient, enterprise}
-
-  def connected?(enterprise) do
-    EnterpriseClient.connected?(enterprise.id)
-  end
+  def connection_spec(enterprise), do: {EnterpriseClient, enterprise}
 
   def disconnect(enterprise) do
     EnterpriseClient.stop(enterprise.id)
@@ -146,19 +143,35 @@ defimpl Livebook.Hubs.Provider, for: Livebook.Hubs.Enterprise do
     EnterpriseClient.get_secrets(enterprise.id)
   end
 
-  def create_secret(enterprise, %Livebook.Secrets.Secret{name: name, value: value}) do
-    create_secret_request = LivebookProto.CreateSecretRequest.new!(name: name, value: value)
+  def create_secret(enterprise, secret) do
+    create_secret_request =
+      LivebookProto.CreateSecretRequest.new!(name: secret.name, value: secret.value)
 
     case EnterpriseClient.send_request(enterprise.id, create_secret_request) do
       {:create_secret, _} ->
         :ok
 
       {:changeset_error, errors} ->
-        errors =
-          for {field, values} <- errors,
-              do: {to_string(field), values}
-
-        {:error, %{errors: errors}}
+        changeset =
+          for {field, messages} <- errors,
+              message <- messages,
+              reduce: secret do
+            acc ->
+              Livebook.Secrets.add_secret_error(acc, field, message)
           end
+
+        {:error, changeset}
+
+      {:transport_error, reason} ->
+        message = "#{enterprise.hub_emoji} #{enterprise.hub_name}: #{reason}"
+        changeset = Livebook.Secrets.add_secret_error(secret, :origin, message)
+
+        {:error, changeset}
+    end
+  end
+
+  def connection_error(enterprise) do
+    reason = EnterpriseClient.get_connection_error(enterprise.id)
+    "Cannot connect to Hub: #{reason}. Will attempt to reconnect automatically..."
   end
 end

lib/livebook/hubs/enterprise_client.ex

@@ -10,7 +10,7 @@ defmodule Livebook.Hubs.EnterpriseClient do
   @registry Livebook.HubsRegistry
   @supervisor Livebook.HubsSupervisor
 
-  defstruct [:server, :hub, connected?: false, secrets: []]
+  defstruct [:server, :hub, :connection_error, connected?: false, secrets: []]
 
   @type registry_name :: {:via, Registry, {Livebook.HubsRegistry, String.t()}}
 
@@ -43,7 +43,9 @@ defmodule Livebook.Hubs.EnterpriseClient do
   end
 
   def send_request(pid, %_struct{} = data) do
-    ClientConnection.send_request(GenServer.call(pid, :get_server), data)
+    with {:ok, server} <- GenServer.call(pid, :fetch_server) do
+      ClientConnection.send_request(server, data)
+    end
   end
 
   @doc """
@@ -54,6 +56,14 @@ defmodule Livebook.Hubs.EnterpriseClient do
     GenServer.call(registry_name(id), :get_secrets)
   end
 
+  @doc """
+  Returns the latest error from connection.
+  """
+  @spec get_connection_error(String.t()) :: Secret.t() | nil
+  def get_connection_error(id) do
+    GenServer.call(registry_name(id), :get_connection_error)
+  end
+
   @doc """
   Returns if the given enterprise is connected.
   """
@@ -75,14 +85,22 @@ defmodule Livebook.Hubs.EnterpriseClient do
   end
 
   @impl true
-  def handle_call(:get_server, _caller, state) do
-    {:reply, state.server, state}
+  def handle_call(:fetch_server, _caller, state) do
+    if state.connected? do
+      {:reply, {:ok, state.server}, state}
+    else
+      {:reply, {:transport_error, state.connection_error}, state}
+    end
   end
 
   def handle_call(:get_secrets, _caller, state) do
     {:reply, state.secrets, state}
   end
 
+  def handle_call(:get_connection_error, _caller, state) do
+    {:reply, state.connection_error, state}
+  end
+
   def handle_call(:connected?, _caller, state) do
     {:reply, state.connected?, state}
   end
@@ -90,17 +108,12 @@ defmodule Livebook.Hubs.EnterpriseClient do
   @impl true
   def handle_info({:connect, :ok, _}, state) do
     Broadcasts.hub_connected()
-    {:noreply, %{state | connected?: true}}
+    {:noreply, %{state | connected?: true, connection_error: nil}}
   end
 
   def handle_info({:connect, :error, reason}, state) do
     Broadcasts.hub_connection_failed(reason)
-    {:noreply, %{state | connected?: false}}
-  end
-
-  def handle_info({:disconnect, :error, reason}, state) do
-    Broadcasts.hub_disconnection_failed(reason)
-    {:noreply, %{state | connected?: false}}
+    {:noreply, %{state | connected?: false, connection_error: reason}}
   end
 
   def handle_info({:event, :secret_created, %{name: name, value: value}}, state) do

lib/livebook/hubs/fly.ex

@@ -124,20 +124,19 @@ defimpl Livebook.Hubs.Provider, for: Livebook.Hubs.Fly do
     }
   end
 
-  def normalize(fly) do
+  def to_metadata(fly) do
     %Livebook.Hubs.Metadata{
       id: fly.id,
       name: fly.hub_name,
       provider: fly,
-      emoji: fly.hub_emoji
+      emoji: fly.hub_emoji,
+      connected?: false
     }
   end
 
   def type(_fly), do: "fly"
 
-  def connect(_fly), do: nil
-
-  def connected?(_fly), do: false
+  def connection_spec(_fly), do: nil
 
   def disconnect(_fly), do: :ok
 
@@ -146,4 +145,6 @@ defimpl Livebook.Hubs.Provider, for: Livebook.Hubs.Fly do
   def get_secrets(_fly), do: []
 
   def create_secret(_fly, _secret), do: :ok
+
+  def connection_error(_fly), do: nil
 end

lib/livebook/hubs/local.ex

@@ -9,20 +9,19 @@ defimpl Livebook.Hubs.Provider, for: Livebook.Hubs.Local do
     %{local | id: fields.id, hub_name: fields.hub_name, hub_emoji: fields.hub_emoji}
   end
 
-  def normalize(local) do
+  def to_metadata(local) do
     %Livebook.Hubs.Metadata{
       id: local.id,
       name: local.hub_name,
       provider: local,
-      emoji: local.hub_emoji
+      emoji: local.hub_emoji,
+      connected?: false
     }
   end
 
   def type(_local), do: "local"
 
-  def connect(_local), do: nil
-
-  def connected?(_local), do: false
+  def connection_spec(_local), do: nil
 
   def disconnect(_local), do: :ok
 
@@ -31,4 +30,6 @@ defimpl Livebook.Hubs.Provider, for: Livebook.Hubs.Local do
   def get_secrets(_local), do: []
 
   def create_secret(_local, _secret), do: :ok
+
+  def connection_error(_local), do: nil
 end

lib/livebook/hubs/provider.ex

@@ -3,61 +3,62 @@ defprotocol Livebook.Hubs.Provider do
 
   alias Livebook.Secrets.Secret
 
+  @type t :: Livebook.Hubs.Enterprise.t() | Livebook.Hubs.Fly.t() | Livebook.Hubs.Local.t()
   @type capability :: :connect | :secrets
   @type capabilities :: list(capability())
   @type changeset_errors :: %{required(:errors) => list({String.t(), {Stirng.t(), list()}})}
 
   @doc """
-  Normalize given hub to `Livebook.Hubs.Metadata` struct.
+  Transforms given hub to `Livebook.Hubs.Metadata` struct.
   """
-  @spec normalize(struct()) :: Livebook.Hubs.Metadata.t()
-  def normalize(struct)
+  @spec to_metadata(t()) :: Livebook.Hubs.Metadata.t()
+  def to_metadata(hub)
 
   @doc """
   Loads fields into given hub.
   """
-  @spec load(struct(), map() | keyword()) :: struct()
-  def load(struct, fields)
+  @spec load(t(), map() | keyword()) :: struct()
+  def load(hub, fields)
 
   @doc """
   Gets the type from hub.
   """
-  @spec type(struct()) :: String.t()
-  def type(struct)
+  @spec type(t()) :: String.t()
+  def type(hub)
 
   @doc """
   Gets the child spec of the given hub.
   """
-  @spec connect(struct()) :: Supervisor.child_spec() | module() | {module(), any()} | nil
-  def connect(struct)
-
-  @doc """
-  Gets the connection status of the given hub.
-  """
-  @spec connected?(struct()) :: boolean()
-  def connected?(struct)
+  @spec connection_spec(t()) :: Supervisor.child_spec() | module() | {module(), any()} | nil
+  def connection_spec(hub)
 
   @doc """
   Disconnects the given hub.
   """
-  @spec disconnect(struct()) :: :ok
-  def disconnect(struct)
+  @spec disconnect(t()) :: :ok
+  def disconnect(hub)
 
   @doc """
   Gets the capabilities of the given hub.
   """
-  @spec capabilities(struct()) :: capabilities()
-  def capabilities(struct)
+  @spec capabilities(t()) :: capabilities()
+  def capabilities(hub)
 
   @doc """
   Gets the secrets of the given hub.
   """
-  @spec get_secrets(struct()) :: list(Secret.t())
-  def get_secrets(struct)
+  @spec get_secrets(t()) :: list(Secret.t())
+  def get_secrets(hub)
 
   @doc """
   Creates a secret of  the given hub.
   """
-  @spec create_secret(struct(), Secret.t()) :: :ok | {:error, changeset_errors()}
-  def create_secret(struct, secret)
+  @spec create_secret(t(), Secret.t()) :: :ok | {:error, changeset_errors()}
+  def create_secret(hub, secret)
+
+  @doc """
+  Gets the connection error from hub.
+  """
+  @spec connection_error(t()) :: String.t() | nil
+  def connection_error(hub)
 end

lib/livebook/secrets.ex

@@ -1,7 +1,7 @@
 defmodule Livebook.Secrets do
   @moduledoc false
 
-  import Ecto.Changeset, only: [apply_action: 2]
+  import Ecto.Changeset, only: [apply_action: 2, add_error: 3, get_field: 2, put_change: 3]
 
   alias Livebook.Storage
   alias Livebook.Secrets.Secret
@@ -50,12 +50,55 @@ defmodule Livebook.Secrets do
   end
 
   @doc """
-  Validates a secret map and either returns a struct struct or changeset.
+  Validates a secret map and either returns a tuple.
+
+  ## Examples
+
+      iex> validate_secret(%{name: "FOO", value: "bar", origin: "session"})
+      {:ok, %Secret{}}
+
+      iex> validate_secret(%{})
+      {:error, %Ecto.Changeset{}}
+
   """
   @spec validate_secret(map()) :: {:ok, Secret.t()} | {:error, Ecto.Changeset.t()}
   def validate_secret(attrs) do
-    changeset = Secret.changeset(%Secret{}, attrs)
-    apply_action(changeset, :validate)
+    %Secret{}
+    |> Secret.changeset(attrs)
+    |> apply_action(:validate)
+  end
+
+  @doc """
+  Returns an `%Ecto.Changeset{}` for tracking secret changes.
+  """
+  @spec change_secret(Secret.t(), map()) :: Ecto.Changeset.t()
+  def change_secret(%Secret{} = secret, attrs) do
+    secret
+    |> Secret.changeset(attrs)
+    |> Map.replace!(:action, :validate)
+    |> normalize_origin()
+  end
+
+  defp normalize_origin(changeset) do
+    case get_field(changeset, :origin) do
+      {:hub, id} -> put_change(changeset, :origin, "hub-#{id}")
+      _ -> changeset
+    end
+  end
+
+  @doc """
+  Returns an `%Ecto.Changeset{}` with errors.
+  """
+  @spec add_secret_error(Ecto.Changeset.t() | Secret.t(), atom(), String.t()) ::
+          Ecto.Changeset.t()
+  def add_secret_error(%Secret{} = secret, field, message) do
+    secret
+    |> change_secret(%{})
+    |> add_error(field, message)
+  end
+
+  def add_secret_error(%Ecto.Changeset{} = changeset, field, message) do
+    add_error(changeset, field, message)
   end
 
   @doc """

lib/livebook/web_socket/client_connection.ex

@@ -8,7 +8,7 @@ defmodule Livebook.WebSocket.ClientConnection do
   alias Livebook.WebSocket.Client
 
   @timeout 10_000
-  @backoff 1_490
+  @backoff 5_000
 
   defstruct [:url, :listener, :headers, :http_conn, :websocket, :ref, id: 0, reply: %{}]
 

lib/livebook_web/live/hooks/sidebar_hook.ex

@@ -30,9 +30,7 @@ defmodule LivebookWeb.SidebarHook do
     {:cont, assign(socket, saved_hubs: Livebook.Hubs.get_metadatas())}
   end
 
-  @error_events ~w(hub_connection_failed hub_disconnection_failed)a
-
-  defp handle_info({event, _reason}, socket) when event in @error_events do
+  defp handle_info({:hub_connection_failed, _reason}, socket) do
     {:cont, assign(socket, saved_hubs: Livebook.Hubs.get_metadatas())}
   end
 

lib/livebook_web/live/hub/new/enterprise_component.ex

@@ -140,7 +140,7 @@ defmodule LivebookWeb.Hub.New.EnterpriseComponent do
 
             {:noreply, assign(socket, pid: pid, changeset: changeset, base: base)}
 
-          {:error, reason} ->
+          {:transport_error, reason} ->
             EnterpriseClient.stop(base.id)
 
             {:noreply,

lib/livebook_web/live/layout_helpers.ex

@@ -181,16 +181,29 @@ defmodule LivebookWeb.LayoutHelpers do
   defp sidebar_hub_link(assigns) do
     ~H"""
     <%= live_redirect to: @to, class: "h-7 flex items-center hover:text-white #{sidebar_link_text_color(@to, @current)} border-l-4 #{sidebar_link_border_color(@to, @current)} hover:border-white" do %>
+      <div class="text-lg leading-6 w-[56px] flex justify-center">
+        <span class="relative">
+          <%= @hub.emoji %>
+        </span>
+      </div>
+      <span class="text-sm font-medium">
+        <%= @hub.name %>
+      </span>
+    <% end %>
+    """
+  end
+
+  defp sidebar_hub_link_with_tooltip(assigns) do
+    ~H"""
+    <%= live_redirect hub_connection_link_opts(@hub.provider, @to, @current) do %>
       <div class="text-lg leading-6 w-[56px] flex justify-center">
         <span class="relative">
           <%= @hub.emoji %>
 
-          <%= if Provider.connect(@hub.provider) do %>
           <div class={[
             "absolute w-[10px] h-[10px] border-gray-900 border-2 rounded-full right-0 bottom-0",
             if(@hub.connected?, do: "bg-green-400", else: "bg-red-400")
           ]} />
-          <% end %>
         </span>
       </div>
       <span class="text-sm font-medium">
@@ -210,12 +223,20 @@ defmodule LivebookWeb.LayoutHelpers do
           </div>
 
           <%= for hub <- @hubs do %>
+            <%= if Provider.connection_spec(hub.provider) do %>
+              <.sidebar_hub_link_with_tooltip
+                hub={hub}
+                to={Routes.hub_path(@socket, :edit, hub.id)}
+                current={@current_page}
+              />
+            <% else %>
               <.sidebar_hub_link
                 hub={hub}
                 to={Routes.hub_path(@socket, :edit, hub.id)}
                 current={@current_page}
               />
             <% end %>
+          <% end %>
 
           <.sidebar_link
             title="Add Hub"
@@ -234,4 +255,18 @@ defmodule LivebookWeb.LayoutHelpers do
 
   defp sidebar_link_border_color(to, current) when to == current, do: "border-white"
   defp sidebar_link_border_color(_to, _current), do: "border-transparent"
+
+  defp hub_connection_link_opts(hub, to, current) do
+    text_color = sidebar_link_text_color(to, current)
+    border_color = sidebar_link_border_color(to, current)
+
+    class =
+      "h-7 flex items-center hover:text-white #{text_color} border-l-4 #{border_color} hover:border-white"
+
+    if tooltip = Provider.connection_error(hub) do
+      [to: to, data_tooltip: tooltip, class: "tooltip top " <> class]
+    else
+      [to: to, class: class]
+    end
+  end
 end

lib/livebook_web/live/session_live/secrets_component.ex

@@ -1,7 +1,10 @@
 defmodule LivebookWeb.SessionLive.SecretsComponent do
   use LivebookWeb, :live_component
 
+  alias Livebook.Hubs
+  alias Livebook.Secrets
   alias Livebook.Secrets.Secret
+  alias Livebook.Session
 
   @impl true
   def update(assigns, socket) do
@@ -70,12 +73,11 @@ defmodule LivebookWeb.SessionLive.SecretsComponent do
         <% end %>
         <.form
           :let={f}
-          for={:data}
+          for={@changeset}
           phx-target={@myself}
           phx-change="validate"
           phx-submit="save"
           autocomplete="off"
-          errors={@errors}
           class="basis-1/2 grow"
         >
           <div class="flex flex-col space-y-4">
@@ -92,43 +94,43 @@ defmodule LivebookWeb.SessionLive.SecretsComponent do
               </div>
 
               <%= text_input(f, :name,
-                value: @data["name"],
-                class: "input",
+                class: "input w-full phx-form-error:border-red-300",
                 autofocus: !@has_prefill,
-                spellcheck: "false"
+                spellcheck: "false",
+                autocomplete: "off",
+                phx_debounce: "blur"
               ) %>
             </.input_wrapper>
             <.input_wrapper form={f} field={:value}>
               <div class="input-label"><label for={input_id(f, :value)}>Value</label></div>
               <%= text_input(f, :value,
-                value: @data["value"],
                 class: "input",
                 autofocus: @has_prefill,
-                spellcheck: "false"
+                spellcheck: "false",
+                autocomplete: "off",
+                phx_debounce: "blur"
               ) %>
             </.input_wrapper>
-            <div>
-              <div class="input-label">Storage</div>
+            <.input_wrapper form={f} field={:origin}>
+              <div class="input-label"><label for={input_id(f, :origin)}>Storage</label></div>
               <div class="my-2 space-y-1 text-sm">
                 <%= label class: "flex items-center gap-2 text-gray-600" do %>
-                  <%= radio_button(f, :store, "session", checked: @data["store"] == "session") %> only this session
+                  <%= radio_button(f, :origin, "session") %> only this session
                 <% end %>
 
                 <%= label class: "flex items-center gap-2 text-gray-600" do %>
-                  <%= radio_button(f, :store, "app", checked: @data["store"] == "app") %> in the Livebook app
+                  <%= radio_button(f, :origin, "app") %> in the Livebook app
                 <% end %>
 
                 <%= if Livebook.Config.feature_flag_enabled?(:hub) do %>
                   <%= for hub <- @hubs do %>
                     <%= label class: "flex items-center gap-2 text-gray-600" do %>
-                      <%= radio_button(f, :store, "hub-#{hub.id}",
-                        checked: @data["store"] == "hub-#{hub.id}"
-                      ) %> in <%= hub.hub_emoji %> <%= hub.hub_name %>
+                      <%= radio_button(f, :origin, "hub-#{hub.id}") %> in <%= hub.hub_emoji %> <%= hub.hub_name %>
                     <% end %>
                   <% end %>
                 <% end %>
               </div>
-            </div>
+            </.input_wrapper>
             <div class="flex space-x-2">
               <button class="button-base button-blue" type="submit" disabled={f.errors != []}>
                 <.remix_icon icon="add-line" class="align-middle" />
@@ -156,8 +158,7 @@ defmodule LivebookWeb.SessionLive.SecretsComponent do
         end
       ]}
       phx-value-name={@secret_name}
-      phx-value-store="hub"
-      phx-value-hub_id={@secret_origin}
+      phx-value-origin={"hub-" <> @secret_origin}
       phx-target={@target}
       phx-click={@action}
     >
@@ -194,7 +195,7 @@ defmodule LivebookWeb.SessionLive.SecretsComponent do
         end
       ]}
       phx-value-name={@secret_name}
-      phx-value-store={@secret_store}
+      phx-value-origin={@secret_store}
       phx-target={@target}
       phx-click={@action}
     >
@@ -248,7 +249,7 @@ defmodule LivebookWeb.SessionLive.SecretsComponent do
                 class="button-base button-gray"
                 phx-click="grant_access"
                 phx-value-name={@secret_name}
-                phx-value-store={@secret_origin}
+                phx-value-origin={@secret_origin}
                 phx-target={@target}
               >
                 Grant access
@@ -258,8 +259,7 @@ defmodule LivebookWeb.SessionLive.SecretsComponent do
                 class="button-base button-gray"
                 phx-click="grant_access"
                 phx-value-name={@secret_name}
-                phx-value-store="hub"
-                phx-value-hub_id={@secret_origin}
+                phx-value-origin={"hub-" <> @secret_origin}
                 phx-target={@target}
               >
                 Grant access
@@ -274,10 +274,10 @@ defmodule LivebookWeb.SessionLive.SecretsComponent do
 
   defp prefill_assigns(socket) do
     secret_name = socket.assigns[:prefill_secret_name]
+    attrs = %{name: secret_name, value: nil, origin: "session"}
 
     assigns = %{
-      data: %{"name" => secret_name, "value" => "", "store" => "session"},
-      errors: [{"value", {"can't be blank", []}}],
+      changeset: Secrets.change_secret(%Secret{}, attrs),
       title: title(socket),
       grant_access_name: nil,
       grant_access_origin: "app",
@@ -306,22 +306,16 @@ defmodule LivebookWeb.SessionLive.SecretsComponent do
   defp stored(%{origin: origin}) when origin in [:app, :startup], do: "Livebook"
 
   @impl true
-  def handle_event("save", %{"data" => data}, socket) do
-    with attrs <- build_attrs(data),
-         {:ok, secret} <- Livebook.Secrets.validate_secret(attrs),
+  def handle_event("save", %{"secret" => attrs}, socket) do
+    with {:ok, secret} <- Secrets.validate_secret(build_attrs(attrs)),
          :ok <- set_secret(socket, secret) do
       {:noreply,
        socket
        |> push_patch(to: socket.assigns.return_to)
        |> push_secret_selected(secret.name)}
     else
-      {:error, %{errors: errors}} ->
-        errors =
-          for {name, messages} <- errors, message <- messages do
-            {String.to_existing_atom(name), {message, []}}
-          end
-
-        {:noreply, assign(socket, errors: errors)}
+      {:error, changeset} ->
+        {:noreply, assign(socket, changeset: changeset)}
     end
   end
 
@@ -334,13 +328,8 @@ defmodule LivebookWeb.SessionLive.SecretsComponent do
      |> push_secret_selected(secret_name)}
   end
 
-  def handle_event("validate", %{"data" => data}, socket) do
-    socket = assign(socket, data: data)
-
-    case Livebook.Secrets.validate_secret(data) do
-      {:ok, _secret} -> {:noreply, assign(socket, errors: [])}
-      {:error, changeset} -> {:noreply, assign(socket, errors: changeset.errors)}
-    end
+  def handle_event("validate", %{"secret" => attrs}, socket) do
+    {:noreply, assign(socket, changeset: Secrets.change_secret(%Secret{}, build_attrs(attrs)))}
   end
 
   def handle_event("grant_access", %{"name" => secret_name} = attrs, socket) do
@@ -362,25 +351,25 @@ defmodule LivebookWeb.SessionLive.SecretsComponent do
   defp title(%{assigns: %{select_secret_options: %{"title" => title}}}), do: title
   defp title(_), do: "Select secret"
 
-  defp build_origin(%{"store" => "hub-" <> id}), do: {:hub, id}
-  defp build_origin(%{"store" => store}), do: String.to_existing_atom(store)
+  defp build_origin(%{"origin" => "hub-" <> id}), do: {:hub, id}
+  defp build_origin(%{"origin" => store}), do: String.to_existing_atom(store)
 
   defp build_attrs(%{"name" => name, "value" => value} = attrs) do
     %{name: name, value: value, origin: build_origin(attrs)}
   end
 
   defp set_secret(socket, %Secret{origin: :session} = secret) do
-    Livebook.Session.set_secret(socket.assigns.session.pid, secret)
+    Session.set_secret(socket.assigns.session.pid, secret)
   end
 
   defp set_secret(socket, %Secret{origin: :app} = secret) do
-    Livebook.Secrets.set_secret(secret)
-    Livebook.Session.set_secret(socket.assigns.session.pid, secret)
+    Secrets.set_secret(secret)
+    Session.set_secret(socket.assigns.session.pid, secret)
   end
 
   defp set_secret(socket, %Secret{origin: {:hub, id}} = secret) when is_binary(id) do
-    with :ok <- Livebook.Hubs.create_secret(secret) do
-      Livebook.Session.set_secret(socket.assigns.session.pid, secret)
+    with :ok <- Hubs.create_secret(secret) do
+      Session.set_secret(socket.assigns.session.pid, secret)
     end
   end
 

test/livebook/hubs/provider_test.exs

@@ -4,14 +4,15 @@ defmodule Livebook.Hubs.ProviderTest do
   alias Livebook.Hubs.{Fly, Metadata, Provider}
 
   describe "Fly" do
-    test "normalize/1" do
+    test "to_metadata/1" do
       fly = build(:fly)
 
-      assert Provider.normalize(fly) == %Metadata{
+      assert Provider.to_metadata(fly) == %Metadata{
                id: fly.id,
                name: fly.hub_name,
                emoji: fly.hub_emoji,
-               provider: fly
+               provider: fly,
+               connected?: false
              }
     end
 

test/livebook/web_socket/client_connection_test.exs

@@ -113,7 +113,7 @@ defmodule Livebook.WebSocket.ClientConnectionTest do
       # Wait until the server is up again
       assert EnterpriseServer.reconnect(name) == :ok
 
-      assert_receive {:connect, :ok, :connected}, 3000
+      assert_receive {:connect, :ok, :connected}, 5000
     end
   end
 

test/livebook_web/live/session_live/secrets_component_test.exs

@@ -47,11 +47,10 @@ defmodule LivebookWeb.SessionLive.SecretsComponentTest do
       {:ok, view, _html} = live(conn, Routes.session_path(conn, :secrets, session.id))
 
       attrs = %{
-        data: %{
+        secret: %{
           name: secret.name,
           value: secret.value,
-          store: "hub",
-          hub_id: enterprise.id
+          origin: "hub-#{enterprise.id}"
         }
       }
 
@@ -61,7 +60,7 @@ defmodule LivebookWeb.SessionLive.SecretsComponentTest do
 
       assert_receive {:secret_created, ^secret}
       assert render(view) =~ "A new secret has been created on your Livebook Enterprise"
-      assert has_element?(view, "#hub-#{enterprise.id}-secret-#{attrs.data.name}-title")
+      assert has_element?(view, "#hub-#{enterprise.id}-secret-#{secret.name}-title")
 
       assert has_element?(
                view,
@@ -119,8 +118,8 @@ defmodule LivebookWeb.SessionLive.SecretsComponentTest do
       secrets_component = with_target(view, "#secrets-modal")
       form_element = element(secrets_component, "form[phx-submit='save']")
       assert has_element?(form_element)
-      data = %{value: secret.value, store: "hub", hub_id: enterprise.id}
-      render_submit(form_element, %{data: data})
+      attrs = %{value: secret.value, origin: "hub-#{enterprise.id}"}
+      render_submit(form_element, %{secret: attrs})
 
       # Checks we received the secret created event from Enterprise
       assert_receive {:secret_created, ^secret}

test/livebook_web/live/session_live_test.exs

@@ -1051,7 +1051,7 @@ defmodule LivebookWeb.SessionLiveTest do
 
       view
       |> element(~s{form[phx-submit="save"]})
-      |> render_submit(%{data: %{name: secret.name, value: secret.value, store: "session"}})
+      |> render_submit(%{secret: %{name: secret.name, value: secret.value, origin: "session"}})
 
       assert_session_secret(view, session.pid, secret)
     end
@@ -1062,7 +1062,7 @@ defmodule LivebookWeb.SessionLiveTest do
 
       view
       |> element(~s{form[phx-submit="save"]})
-      |> render_submit(%{data: %{name: secret.name, value: secret.value, store: "app"}})
+      |> render_submit(%{secret: %{name: secret.name, value: secret.value, origin: "app"}})
 
       assert secret in Livebook.Secrets.get_secrets()
     end
@@ -1075,7 +1075,7 @@ defmodule LivebookWeb.SessionLiveTest do
 
       view
       |> element(~s{form[phx-submit="save"]})
-      |> render_submit(%{data: %{name: secret.name, value: secret.value, store: "app"}})
+      |> render_submit(%{secret: %{name: secret.name, value: secret.value, origin: "app"}})
 
       assert_session_secret(view, session.pid, secret)
       assert secret in Livebook.Secrets.get_secrets()
@@ -1087,7 +1087,7 @@ defmodule LivebookWeb.SessionLiveTest do
 
       view
       |> element(~s{form[phx-submit="save"]})
-      |> render_submit(%{data: %{name: secret.name, value: secret.value, store: "app"}})
+      |> render_submit(%{secret: %{name: secret.name, value: secret.value, origin: "app"}})
 
       assert_session_secret(view, session.pid, secret)
       assert secret in Livebook.Secrets.get_secrets()
@@ -1102,7 +1102,7 @@ defmodule LivebookWeb.SessionLiveTest do
 
       view
       |> element(~s{form[phx-submit="save"]})
-      |> render_submit(%{data: %{name: secret.name, value: secret.value, store: "session"}})
+      |> render_submit(%{secret: %{name: secret.name, value: secret.value, origin: "session"}})
 
       assert_session_secret(view, session.pid, secret)
       refute secret in Livebook.Secrets.get_secrets()
@@ -1152,7 +1152,7 @@ defmodule LivebookWeb.SessionLiveTest do
       secrets_component = with_target(view, "#secrets-modal")
       form_element = element(secrets_component, "form[phx-submit='save']")
       assert has_element?(form_element)
-      render_submit(form_element, %{data: %{value: secret.value, store: "session"}})
+      render_submit(form_element, %{secret: %{value: secret.value, origin: "session"}})
 
       # Checks if the secret isn't an app secret
       refute secret in Livebook.Secrets.get_secrets()

test/support/factory.ex

@@ -10,7 +10,7 @@ defmodule Livebook.Factory do
   end
 
   def build(:fly_metadata) do
-    :fly |> build() |> Livebook.Hubs.Provider.normalize()
+    :fly |> build() |> Livebook.Hubs.Provider.to_metadata()
   end
 
   def build(:fly) do
@@ -27,7 +27,7 @@ defmodule Livebook.Factory do
   end
 
   def build(:enterprise_metadata) do
-    :enterprise |> build() |> Livebook.Hubs.Provider.normalize()
+    :enterprise |> build() |> Livebook.Hubs.Provider.to_metadata()
   end
 
   def build(:enterprise) do