Add support for clear and update deployment broadcasts (#1864)

More focused events, and use the information from the event instead of
reloading data.

Author: joshk

lib/nerves_hub/devices.ex

@@ -739,7 +739,7 @@ defmodule NervesHub.Devices do
       |> Ecto.Changeset.put_change(:deployment_id, deployment.id)
       |> Repo.update!()
 
-    _ = broadcast(device, "devices/updated")
+    _ = broadcast(device, "devices/deployment-updated", %{deployment_id: deployment.id})
 
     Map.put(device, :deployment, deployment)
   end
@@ -752,7 +752,7 @@ defmodule NervesHub.Devices do
       |> Ecto.Changeset.put_change(:deployment_id, nil)
       |> Repo.update!()
 
-    _ = broadcast(device, "devices/updated")
+    _ = broadcast(device, "devices/deployment-cleared")
 
     Map.put(device, :deployment, nil)
   end
@@ -1126,11 +1126,13 @@ defmodule NervesHub.Devices do
     end
   end
 
-  defp broadcast(%Device{id: id}, event) do
+  defp broadcast(device, event), do: broadcast(device, event, %{})
+
+  defp broadcast(%Device{id: id}, event, payload) do
     Phoenix.PubSub.broadcast(
       NervesHub.PubSub,
       "device:#{id}",
-      %Phoenix.Socket.Broadcast{event: event}
+      %Phoenix.Socket.Broadcast{event: event, payload: payload}
     )
   end
 

lib/nerves_hub_web/channels/device_channel.ex

@@ -163,12 +163,22 @@ defmodule NervesHubWeb.DeviceChannel do
     {:noreply, socket}
   end
 
-  @decorate with_span("Channels.DeviceChannel.handle_info:clear-deployment")
+  @decorate with_span("Channels.DeviceChannel.handle_info:deployment-cleared")
   def handle_info(
-        %Broadcast{event: "devices/clear-deployment"},
+        %Broadcast{event: "devices/deployment-cleared"},
         %{assigns: %{device: device}} = socket
       ) do
-    device = %{device | deployment_id: nil, deployment: nil}
+    device = %{device | deployment_id: nil}
+
+    {:noreply, update_device(socket, device)}
+  end
+
+  @decorate with_span("Channels.DeviceChannel.handle_info:deployment-updated")
+  def handle_info(
+        %Broadcast{event: "devices/deployment-updated", payload: %{deployment_id: deployment_id}},
+        %{assigns: %{device: device}} = socket
+      ) do
+    device = %{device | deployment_id: deployment_id}
 
     {:noreply, update_device(socket, device)}
   end
@@ -380,15 +390,19 @@ defmodule NervesHubWeb.DeviceChannel do
     :ok
   end
 
-  defp subscribe(topic) do
+  defp subscribe(topic) when not is_nil(topic) do
     _ = Phoenix.PubSub.subscribe(NervesHub.PubSub, topic)
     :ok
   end
 
-  defp unsubscribe(topic) do
+  defp subscribe(nil), do: :ok
+
+  defp unsubscribe(topic) when not is_nil(topic) do
     Phoenix.PubSub.unsubscribe(NervesHub.PubSub, topic)
   end
 
+  defp unsubscribe(nil), do: :ok
+
   defp device_internal_broadcast!(socket, device, event, payload) do
     topic = "device:#{device.identifier}:internal"
     socket.endpoint.broadcast_from!(self(), topic, event, payload)
@@ -464,8 +478,6 @@ defmodule NervesHubWeb.DeviceChannel do
   defp deployment_channel(device) do
     if device.deployment_id do
       "deployment:#{device.deployment_id}"
-    else
-      "deployment:none"
     end
   end
 

test/nerves_hub/devices_test.exs

@@ -787,7 +787,7 @@ defmodule NervesHub.DevicesTest do
   end
 
   describe "update_deployment/2" do
-    test "updates deployment and broadcasts 'devices/updated'", %{
+    test "updates deployment and broadcasts 'devices/deployment-updated'", %{
       device: device,
       deployment: deployment
     } do
@@ -797,12 +797,12 @@ defmodule NervesHub.DevicesTest do
       device = Devices.update_deployment(device, deployment)
 
       assert device.deployment_id == deployment.id
-      assert_receive %{event: "devices/updated"}
+      assert_receive %{event: "devices/deployment-updated"}
     end
   end
 
   describe "clear_deployment1/2" do
-    test "clears deployment and broadcasts 'devices/updated'", %{
+    test "clears deployment and broadcasts 'devices/deployment-cleared'", %{
       device: device,
       deployment: deployment
     } do
@@ -812,7 +812,7 @@ defmodule NervesHub.DevicesTest do
       device = Devices.clear_deployment(device)
 
       refute device.deployment_id
-      assert_receive %{event: "devices/updated"}
+      assert_receive %{event: "devices/deployment-cleared"}
     end
   end
 end

test/nerves_hub_web/channels/device_channel_test.exs

@@ -4,6 +4,7 @@ defmodule NervesHubWeb.DeviceChannelTest do
 
   import TrackerHelper
 
+  alias NervesHub.Devices
   alias NervesHub.Fixtures
   alias NervesHubWeb.DeviceChannel
   alias NervesHubWeb.DeviceSocket
@@ -130,6 +131,63 @@ defmodule NervesHubWeb.DeviceChannelTest do
     assert device.latest_connection.metadata["connection_types"] == ["ethernet", "wifi"]
   end
 
+  test "deployment information is updated when the deployment is cleared" do
+    user = Fixtures.user_fixture()
+    {device, _firmware, deployment} = device_fixture(user, %{identifier: "123"})
+    Devices.update_deployment(device, deployment)
+
+    %{db_cert: certificate, cert: _cert} = Fixtures.device_certificate_fixture(device)
+
+    {:ok, socket} =
+      connect(DeviceSocket, %{}, connect_info: %{peer_data: %{ssl_cert: certificate.der}})
+
+    {:ok, _join_reply, socket} =
+      subscribe_and_join(socket, DeviceChannel, "device")
+
+    refute is_nil(socket.assigns.device.deployment_id)
+    refute is_nil(socket.assigns.deployment_channel)
+
+    Devices.clear_deployment(device)
+
+    # we need to let the channel process all messages before we can
+    # check the state of the device's connection types
+    socket = :sys.get_state(socket.channel_pid)
+
+    assert is_nil(socket.assigns.device.deployment_id)
+    assert is_nil(socket.assigns.deployment_channel)
+  end
+
+  test "deployment information is updated when the device joins a new deployment" do
+    user = Fixtures.user_fixture()
+    {device, firmware, deployment} = device_fixture(user, %{identifier: "123"})
+    Devices.update_deployment(device, deployment)
+
+    %{db_cert: certificate, cert: _cert} = Fixtures.device_certificate_fixture(device)
+
+    {:ok, socket} =
+      connect(DeviceSocket, %{}, connect_info: %{peer_data: %{ssl_cert: certificate.der}})
+
+    {:ok, _join_reply, socket} =
+      subscribe_and_join(socket, DeviceChannel, "device")
+
+    assert socket.assigns.device.deployment_id == deployment.id
+    refute is_nil(socket.assigns.deployment_channel)
+
+    device = NervesHub.Repo.preload(device, :org)
+
+    new_deployment =
+      Fixtures.deployment_fixture(device.org, firmware, %{name: "Super Deployment"})
+
+    Devices.update_deployment(device, new_deployment)
+
+    # we need to let the channel process all messages before we can
+    # check the state of the device's connection types
+    socket = :sys.get_state(socket.channel_pid)
+
+    assert socket.assigns.device.deployment_id == new_deployment.id
+    refute is_nil(socket.assigns.deployment_channel)
+  end
+
   describe "unhandled messages are caught" do
     test "handle_info" do
       user = Fixtures.user_fixture()

test/nerves_hub_web/live/devices/show_test.exs

@@ -578,6 +578,8 @@ defmodule NervesHubWeb.Live.Devices.ShowTest do
       end)
       |> assert_has("div", text: "Eligible Deployments")
 
+      assert_receive %Phoenix.Socket.Broadcast{event: "devices/deployment-cleared"}
+
       refute Repo.reload(device) |> Map.get(:deployment_id)
     end
 
@@ -667,7 +669,7 @@ defmodule NervesHubWeb.Live.Devices.ShowTest do
         render_change(view, "set-deployment", %{"deployment_id" => deployment.id})
       end)
 
-      assert_receive %Phoenix.Socket.Broadcast{event: "devices/updated"}
+      assert_receive %Phoenix.Socket.Broadcast{event: "devices/deployment-updated"}
     end
   end