Check device api version before requesting extensions (#1668)

* Check device api version before requesting extensions * Depend on default socket.assigns.device_api_version * Fix tests * Fix final failing test
nerves-hub · Dec 3, 2024 · 940c3f8 · 940c3f8
1 parent f0a563e
commit 940c3f8
Show file tree

Hide file tree

Showing 3 changed files with 117 additions and 40 deletions.
diff --git a/lib/nerves_hub_web/channels/device_channel.ex b/lib/nerves_hub_web/channels/device_channel.ex
@@ -49,9 +49,6 @@ defmodule NervesHubWeb.DeviceChannel do
 
     send(self(), :device_registation)
 
-    # Request device extension capabilities
-    push(socket, "extensions:get", %{})
-
     socket =
       socket
       |> assign(:device, device)
@@ -61,6 +58,12 @@ defmodule NervesHubWeb.DeviceChannel do
       |> maybe_start_penalty_timer()
       |> maybe_send_archive()
 
+    # Request device extension capabilities if possible
+    # Earlier versions of nerves_hub_link don't have a fallback for unknown messages,
+    # so check version before requesting extensions
+    if is_safe_to_request_extensions?(socket.assigns.device_api_version),
+      do: push(socket, "extensions:get", %{})
+
     {:noreply, socket}
   end
 
@@ -612,4 +615,6 @@ defmodule NervesHubWeb.DeviceChannel do
       0
     end
   end
+
+  defp is_safe_to_request_extensions?(version), do: Version.match?(version, "> 2.5.2")
 end
diff --git a/test/nerves_hub_web/channels/device_channel_test.exs b/test/nerves_hub_web/channels/device_channel_test.exs
@@ -10,15 +10,17 @@ defmodule NervesHubWeb.DeviceChannelTest do
   alias NervesHubWeb.DeviceSocket
   alias NervesHubWeb.ExtensionsChannel
 
-  test "basic connection to the channel" do
+  test "extensions are requested from device if version is above 2.5.2" do
     user = Fixtures.user_fixture()
     {device, _firmware, _deployment} = device_fixture(user, %{identifier: "123"})
     %{db_cert: certificate, cert: _cert} = Fixtures.device_certificate_fixture(device)
 
     {:ok, socket} =
       connect(DeviceSocket, %{}, connect_info: %{peer_data: %{ssl_cert: certificate.der}})
 
-    {:ok, _, socket} = subscribe_and_join(socket, DeviceChannel, "device")
+    {:ok, _, socket} =
+      subscribe_and_join(socket, DeviceChannel, "device", %{"device_api_version" => "2.5.3"})
+
     assert_push("extensions:get", _)
 
     {:ok, _, _socket} =

diff --git a/test/nerves_hub_web/channels/extensions_channel_test.exs b/test/nerves_hub_web/channels/extensions_channel_test.exs
@@ -16,7 +16,9 @@ defmodule NervesHubWeb.ExtensionsChannelTest do
     {:ok, socket} =
       connect(DeviceSocket, %{}, connect_info: %{peer_data: %{ssl_cert: certificate.der}})
 
-    {:ok, _, _} = subscribe_and_join(socket, DeviceChannel, "device")
+    {:ok, _, _} =
+      subscribe_and_join_with_default_device_api_version(socket, DeviceChannel, "device")
+
     assert_push("extensions:get", _extensions)
   end
 
@@ -28,14 +30,21 @@ defmodule NervesHubWeb.ExtensionsChannelTest do
     {:ok, socket} =
       connect(DeviceSocket, %{}, connect_info: %{peer_data: %{ssl_cert: certificate.der}})
 
-    {:ok, _, socket} = subscribe_and_join(socket, DeviceChannel, "device")
+    {:ok, _, socket} =
+      subscribe_and_join_with_default_device_api_version(socket, DeviceChannel, "device")
+
     assert_push("extensions:get", _extensions)
 
     assert {:ok, attach_list, _} =
-             subscribe_and_join(socket, ExtensionsChannel, "extensions", %{
-               "geo" => "0.0.1",
-               "health" => "0.0.1"
-             })
+             subscribe_and_join_with_default_device_api_version(
+               socket,
+               ExtensionsChannel,
+               "extensions",
+               %{
+                 "geo" => "0.0.1",
+                 "health" => "0.0.1"
+               }
+             )
 
     assert "health" in attach_list
     assert "geo" in attach_list
@@ -49,14 +58,21 @@ defmodule NervesHubWeb.ExtensionsChannelTest do
     {:ok, socket} =
       connect(DeviceSocket, %{}, connect_info: %{peer_data: %{ssl_cert: certificate.der}})
 
-    {:ok, _, socket} = subscribe_and_join(socket, DeviceChannel, "device")
+    {:ok, _, socket} =
+      subscribe_and_join_with_default_device_api_version(socket, DeviceChannel, "device")
+
     assert_push("extensions:get", _extensions)
 
     assert {:ok, ["health"], _} =
-             subscribe_and_join(socket, ExtensionsChannel, "extensions", %{
-               "goof" => "0.0.1",
-               "health" => "0.0.1"
-             })
+             subscribe_and_join_with_default_device_api_version(
+               socket,
+               ExtensionsChannel,
+               "extensions",
+               %{
+                 "goof" => "0.0.1",
+                 "health" => "0.0.1"
+               }
+             )
   end
 
   test "product with extensions disabled does not suggest attaching anything" do
@@ -70,14 +86,21 @@ defmodule NervesHubWeb.ExtensionsChannelTest do
     {:ok, socket} =
       connect(DeviceSocket, %{}, connect_info: %{peer_data: %{ssl_cert: certificate.der}})
 
-    {:ok, _, socket} = subscribe_and_join(socket, DeviceChannel, "device")
+    {:ok, _, socket} =
+      subscribe_and_join_with_default_device_api_version(socket, DeviceChannel, "device")
+
     assert_push("extensions:get", _extensions)
 
     assert {:ok, [], _} =
-             subscribe_and_join(socket, ExtensionsChannel, "extensions", %{
-               "geo" => "0.0.1",
-               "health" => "0.0.1"
-             })
+             subscribe_and_join_with_default_device_api_version(
+               socket,
+               ExtensionsChannel,
+               "extensions",
+               %{
+                 "geo" => "0.0.1",
+                 "health" => "0.0.1"
+               }
+             )
   end
 
   test "product with only health suggests only health" do
@@ -90,14 +113,21 @@ defmodule NervesHubWeb.ExtensionsChannelTest do
     {:ok, socket} =
       connect(DeviceSocket, %{}, connect_info: %{peer_data: %{ssl_cert: certificate.der}})
 
-    {:ok, _, socket} = subscribe_and_join(socket, DeviceChannel, "device")
+    {:ok, _, socket} =
+      subscribe_and_join_with_default_device_api_version(socket, DeviceChannel, "device")
+
     assert_push("extensions:get", _extensions)
 
     assert {:ok, ["health"], _} =
-             subscribe_and_join(socket, ExtensionsChannel, "extensions", %{
-               "geo" => "0.0.1",
-               "health" => "0.0.1"
-             })
+             subscribe_and_join_with_default_device_api_version(
+               socket,
+               ExtensionsChannel,
+               "extensions",
+               %{
+                 "geo" => "0.0.1",
+                 "health" => "0.0.1"
+               }
+             )
   end
 
   test "attached health extension will receive request for health report" do
@@ -108,11 +138,18 @@ defmodule NervesHubWeb.ExtensionsChannelTest do
     {:ok, socket} =
       connect(DeviceSocket, %{}, connect_info: %{peer_data: %{ssl_cert: certificate.der}})
 
-    {:ok, _, socket} = subscribe_and_join(socket, DeviceChannel, "device")
+    {:ok, _, socket} =
+      subscribe_and_join_with_default_device_api_version(socket, DeviceChannel, "device")
+
     assert_push("extensions:get", _extensions)
 
     assert {:ok, ["health"], socket} =
-             subscribe_and_join(socket, ExtensionsChannel, "extensions", %{"health" => "0.0.1"})
+             subscribe_and_join_with_default_device_api_version(
+               socket,
+               ExtensionsChannel,
+               "extensions",
+               %{"health" => "0.0.1"}
+             )
 
     push(socket, "health:attached")
     assert_push("health:check", _)
@@ -126,11 +163,18 @@ defmodule NervesHubWeb.ExtensionsChannelTest do
     {:ok, socket} =
       connect(DeviceSocket, %{}, connect_info: %{peer_data: %{ssl_cert: certificate.der}})
 
-    {:ok, _, socket} = subscribe_and_join(socket, DeviceChannel, "device")
+    {:ok, _, socket} =
+      subscribe_and_join_with_default_device_api_version(socket, DeviceChannel, "device")
+
     assert_push("extensions:get", _extensions)
 
     assert {:ok, ["geo"], socket} =
-             subscribe_and_join(socket, ExtensionsChannel, "extensions", %{"geo" => "0.0.1"})
+             subscribe_and_join_with_default_device_api_version(
+               socket,
+               ExtensionsChannel,
+               "extensions",
+               %{"geo" => "0.0.1"}
+             )
 
     push(socket, "geo:attached")
     assert_push("geo:location:request", _)
@@ -144,14 +188,21 @@ defmodule NervesHubWeb.ExtensionsChannelTest do
     {:ok, socket} =
       connect(DeviceSocket, %{}, connect_info: %{peer_data: %{ssl_cert: certificate.der}})
 
-    {:ok, _, socket} = subscribe_and_join(socket, DeviceChannel, "device")
+    {:ok, _, socket} =
+      subscribe_and_join_with_default_device_api_version(socket, DeviceChannel, "device")
+
     assert_push("extensions:get", _extensions)
 
     assert {:ok, attach_list, socket} =
-             subscribe_and_join(socket, ExtensionsChannel, "extensions", %{
-               "geo" => "0.0.1",
-               "health" => "0.0.1"
-             })
+             subscribe_and_join_with_default_device_api_version(
+               socket,
+               ExtensionsChannel,
+               "extensions",
+               %{
+                 "geo" => "0.0.1",
+                 "health" => "0.0.1"
+               }
+             )
 
     assert "health" in attach_list
     assert "geo" in attach_list
@@ -174,14 +225,21 @@ defmodule NervesHubWeb.ExtensionsChannelTest do
     {:ok, socket} =
       connect(DeviceSocket, %{}, connect_info: %{peer_data: %{ssl_cert: certificate.der}})
 
-    {:ok, _, socket} = subscribe_and_join(socket, DeviceChannel, "device")
+    {:ok, _, socket} =
+      subscribe_and_join_with_default_device_api_version(socket, DeviceChannel, "device")
+
     assert_push("extensions:get", _extensions)
 
     assert {:ok, attach_list, socket} =
-             subscribe_and_join(socket, ExtensionsChannel, "extensions", %{
-               "geo" => "0.0.1",
-               "health" => "0.0.1"
-             })
+             subscribe_and_join_with_default_device_api_version(
+               socket,
+               ExtensionsChannel,
+               "extensions",
+               %{
+                 "geo" => "0.0.1",
+                 "health" => "0.0.1"
+               }
+             )
 
     assert "health" in attach_list
     assert "geo" in attach_list
@@ -224,4 +282,16 @@ defmodule NervesHubWeb.ExtensionsChannelTest do
 
     {device, firmware, deployment}
   end
+
+  defp subscribe_and_join_with_default_device_api_version(socket, channel, topic),
+    do: subscribe_and_join(socket, channel, topic, %{"device_api_version" => "2.5.3"})
+
+  defp subscribe_and_join_with_default_device_api_version(socket, channel, topic, payload),
+    do:
+      subscribe_and_join(
+        socket,
+        channel,
+        topic,
+        Map.merge(%{"device_api_version" => "2.5.3"}, payload)
+      )
 end