Deployments context specs (and some improvements) (#1733)

- Add specs to all functions
- Fix a function return
- Simplify some function arguments

lib/nerves_hub/deployments.ex

@@ -13,12 +13,13 @@ defmodule NervesHub.Deployments do
   alias NervesHub.Repo
   alias Ecto.Changeset
 
+  @spec all() :: [Deployment.t()]
   def all() do
     Repo.all(Deployment)
   end
 
-  @spec get_deployments_by_product(integer()) :: [Deployment.t()]
-  def get_deployments_by_product(product_id) do
+  @spec get_deployments_by_product(Product.t()) :: [Deployment.t()]
+  def get_deployments_by_product(%Product{id: product_id}) do
     from(
       d in Deployment,
       join: f in assoc(d, :firmware),
@@ -28,8 +29,8 @@ defmodule NervesHub.Deployments do
     |> Repo.all()
   end
 
-  @spec get_deployment_device_counts_by_product(integer()) :: %{integer() => integer()}
-  def get_deployment_device_counts_by_product(product_id) do
+  @spec get_device_counts_by_product(Product.t()) :: %{integer() => integer()}
+  def get_device_counts_by_product(%Product{id: product_id}) do
     Device
     |> select([d], {d.deployment_id, count(d.id)})
     |> where([d], d.product_id == ^product_id)
@@ -38,25 +39,22 @@ defmodule NervesHub.Deployments do
     |> Map.new()
   end
 
-  @spec get_deployment_device_count(Deployment.t()) :: term() | nil
-  def get_deployment_device_count(%Deployment{id: id}) do
-    get_deployment_device_count(id)
-  end
-
-  @spec get_deployment_device_count(integer()) :: term() | nil
-  def get_deployment_device_count(deployment_id) do
+  @spec get_device_count(Deployment.t()) :: term() | nil
+  def get_device_count(%Deployment{id: id}) do
     Device
-    |> where([d], d.deployment_id == ^deployment_id)
+    |> where([d], d.deployment_id == ^id)
     |> Repo.exclude_deleted()
     |> Repo.aggregate(:count)
   end
 
   @spec get_deployments_by_firmware(integer()) :: [Deployment.t()]
   def get_deployments_by_firmware(firmware_id) do
-    from(d in Deployment, where: d.firmware_id == ^firmware_id)
+    Deployment
+    |> where([d], d.firmware_id == ^firmware_id)
     |> Repo.all()
   end
 
+  @spec get(integer()) :: {:ok, Deployment.t()} | {:error, :not_found}
   def get(id) when is_integer(id) do
     case Repo.get(Deployment, id) do
       nil ->
@@ -237,7 +235,7 @@ defmodule NervesHub.Deployments do
     Deployment.changeset(deployment, params)
   end
 
-  @spec create_deployment(map) :: {:ok, Deployment.t()} | {:error, Changeset.t()}
+  @spec create_deployment(map()) :: {:ok, Deployment.t()} | {:error, Changeset.t()}
   def create_deployment(params) do
     changeset = Deployment.creation_changeset(%Deployment{}, params)
 
@@ -252,6 +250,7 @@ defmodule NervesHub.Deployments do
     end
   end
 
+  @spec broadcast(Deployment.t() | atom(), String.t(), map()) :: :ok | {:error, term()}
   def broadcast(deployment, event, payload \\ %{})
 
   def broadcast(:none, event, payload) do
@@ -287,6 +286,7 @@ defmodule NervesHub.Deployments do
 
   Based on the product, firmware platform, firmware architecture, and device tags
   """
+  @spec estimate_devices_matched_by_conditions(integer(), String.t(), map()) :: integer()
   def estimate_devices_matched_by_conditions(product_id, platform, conditions) do
     Device
     |> where([dev], dev.product_id == ^product_id)
@@ -297,18 +297,15 @@ defmodule NervesHub.Deployments do
     |> Enum.count()
   end
 
-  @doc """
-  Check that a device version matches for a deployment's conditions
-
-  A deployment not having a version condition returns true
-  """
-  def version_match?(_device, %{conditions: %{"version" => ""}}), do: true
+  # Check that a device version matches for a deployment's conditions
+  # A deployment not having a version condition returns true
+  defp version_match?(_device, %{conditions: %{"version" => ""}}), do: true
 
-  def version_match?(device, %{conditions: %{"version" => version}}) when not is_nil(version) do
+  defp version_match?(device, %{conditions: %{"version" => version}}) when not is_nil(version) do
     Version.match?(device.firmware_metadata.version, version)
   end
 
-  def version_match?(_device, _deployment), do: true
+  defp version_match?(_device, _deployment), do: true
 
   @spec verify_deployment_membership(Device.t()) :: Device.t()
   def verify_deployment_membership(%Device{deployment_id: deployment_id} = device)
@@ -343,6 +340,8 @@ defmodule NervesHub.Deployments do
         device,
         "device no longer matches deployment #{deployment.name}'s requirements because of #{reason}"
       )
+
+      device
     else
       device
     end
@@ -355,6 +354,7 @@ defmodule NervesHub.Deployments do
 
   Do nothing if a deployment is already set
   """
+  @spec set_deployment(Device.t()) :: Device.t()
   def set_deployment(%{deployment_id: nil} = device) do
     case matching_deployments(device, [true]) do
       [] ->
@@ -403,6 +403,7 @@ defmodule NervesHub.Deployments do
     )
   end
 
+  @spec preload_with_firmware_and_archive(Device.t(), boolean()) :: Device.t()
   def preload_with_firmware_and_archive(device, force \\ false) do
     Repo.preload(device, [deployment: [:archive, :firmware]], force: force)
   end
@@ -412,6 +413,7 @@ defmodule NervesHub.Deployments do
 
   Based on the product, firmware platform, firmware architecture, and device tags
   """
+  @spec matching_deployments(Device.t(), [boolean()]) :: [Deployment.t()]
   def matching_deployments(device, active \\ [true, false])
   def matching_deployments(%Device{firmware_metadata: nil}, _active), do: []
 

lib/nerves_hub_web/controllers/api/deployment_controller.ex

@@ -14,7 +14,7 @@ defmodule NervesHubWeb.API.DeploymentController do
   @whitelist_fields [:name, :org_id, :firmware_id, :conditions, :is_active]
 
   def index(%{assigns: %{product: product}} = conn, _params) do
-    deployments = Deployments.get_deployments_by_product(product.id)
+    deployments = Deployments.get_deployments_by_product(product)
     render(conn, "index.json", deployments: deployments)
   end
 

lib/nerves_hub_web/live/deployments/edit.ex

@@ -13,10 +13,9 @@ defmodule NervesHubWeb.Live.Deployments.Edit do
     %{"name" => name} = params
     %{product: product} = socket.assigns
 
-    deployment =
-      Deployments.get_by_product_and_name!(product, name) |> NervesHub.Repo.preload(:firmware)
+    deployment = Deployments.get_by_product_and_name!(product, name)
 
-    current_device_count = Deployments.get_deployment_device_count(deployment)
+    current_device_count = Deployments.get_device_count(deployment)
 
     archives = Archives.all_by_product(deployment.product)
     firmwares = Firmwares.get_firmwares_for_deployment(deployment)

lib/nerves_hub_web/live/deployments/index.ex

@@ -7,8 +7,8 @@ defmodule NervesHubWeb.Live.Deployments.Index do
 
   @impl Phoenix.LiveView
   def mount(_params, _session, %{assigns: %{product: product}} = socket) do
-    deployments = Deployments.get_deployments_by_product(product.id)
-    counts = Deployments.get_deployment_device_counts_by_product(product.id)
+    deployments = Deployments.get_deployments_by_product(product)
+    counts = Deployments.get_device_counts_by_product(product)
 
     deployments =
       deployments

lib/nerves_hub_web/live/deployments/show.ex

@@ -31,7 +31,7 @@ defmodule NervesHubWeb.Live.Deployments.Show do
       |> Map.put(:anchor, "latest-activity")
 
     inflight_updates = Devices.inflight_updates_for(deployment)
-    current_device_count = Deployments.get_deployment_device_count(deployment)
+    current_device_count = Deployments.get_device_count(deployment)
 
     socket
     |> page_title("Deployment - #{deployment.name} - #{product.name}")

test/nerves_hub_web/channels/websocket_test.exs

@@ -785,6 +785,7 @@ defmodule NervesHubWeb.WebsocketTest do
       different_platform = "tester"
 
       SocketClient.join_and_wait(socket, %{
+        "device_api_version" => "2.2.0",
         "nerves_fw_uuid" => Ecto.UUID.generate(),
         "nerves_fw_product" => "test",
         "nerves_fw_architecture" => different_architecture,