base_image: correctly scope upload to the BaseImageCollection bucket

The extracted base_image_collection_id was nil in the current implementation,
leading to the image being put in the wrong bucket directory. This makes the
storage module behaviour more explicit in its requirement (instead of having to
pass the whole BaseImage) and adds a test to verify that the
base_image_collection_id received by the storage module actually matches the one
from the Base Image Collection.

Close edgehog-device-manager#586

Signed-off-by: Riccardo Binetti <[email protected]>

CHANGELOG.md

@@ -4,6 +4,10 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
+## [0.9.0] - Unreleased
+### Fixed
+- Correctly scope Base Image uploads to their Base Image Collection bucket.
+
 ## [0.9.0-rc.1] - 2024-07-10
 ### Fixed
 - Wrong input params used in GraphQL mutation when creating a base image, leading to a rejected operation ([#574](https://github.com/edgehog-device-manager/edgehog/pull/574)).

backend/lib/edgehog/base_images/base_image/changes/handle_file_upload.ex

@@ -46,9 +46,12 @@ defmodule Edgehog.BaseImages.BaseImage.Changes.HandleFileUpload do
   end
 
   defp upload_file(changeset, file) do
-    {:ok, base_image} = Ash.Changeset.apply_attributes(changeset)
+    tenant_id = changeset.to_tenant
 
-    case @storage_module.store(base_image, file) do
+    {:ok, base_image_collection_id} =
+      Ash.Changeset.fetch_argument(changeset, :base_image_collection_id)
+
+    case @storage_module.store(tenant_id, base_image_collection_id, file) do
       {:ok, file_url} ->
         changeset
         |> Ash.Changeset.force_change_attribute(:url, file_url)

backend/lib/edgehog/base_images/bucket_storage.ex

@@ -27,7 +27,9 @@ defmodule Edgehog.BaseImages.BucketStorage do
   alias Edgehog.BaseImages.Uploaders
 
   @impl Storage
-  def store(%BaseImage{} = scope, %Plug.Upload{} = upload) do
+  def store(tenant_id, base_image_collection_id, %Plug.Upload{} = upload) do
+    scope = %{tenant_id: tenant_id, base_image_collection_id: base_image_collection_id}
+
     with {:ok, file_name} <- Uploaders.BaseImage.store({upload, scope}) do
       # TODO: investigate URL signing instead of public access
       file_url = Uploaders.BaseImage.url({file_name, scope})

backend/lib/edgehog/base_images/storage.ex

@@ -24,7 +24,11 @@ defmodule Edgehog.BaseImages.Storage do
 
   @type upload :: %Plug.Upload{}
 
-  @callback store(scope :: %BaseImage{}, file :: upload()) ::
+  @callback store(
+              tenant_id :: String.t() | integer(),
+              base_image_id :: String.t() | integer(),
+              file :: upload()
+            ) ::
               {:ok, file_url :: String.t()} | {:error, reason :: any}
 
   @callback delete(base_image :: %BaseImage{}) ::

backend/test/edgehog_web/schema/mutation/create_base_image_test.exs

@@ -28,21 +28,25 @@ defmodule EdgehogWeb.Schema.Mutation.CreateBaseImageTest do
   describe "createBaseImage mutation" do
     setup do
       StorageMock
-      |> stub(:store, fn _, _ -> {:ok, "https://example.com/ota.bin"} end)
+      |> stub(:store, fn _, _, _ -> {:ok, "https://example.com/ota.bin"} end)
       |> stub(:delete, fn _ -> :ok end)
 
       :ok
     end
 
     test "creates base image with valid data", %{tenant: tenant} do
-      base_image_collection_id =
-        [tenant: tenant]
-        |> base_image_collection_fixture()
-        |> AshGraphql.Resource.encode_relay_id()
+      base_image_collection = base_image_collection_fixture(tenant: tenant)
+      base_image_collection_id = AshGraphql.Resource.encode_relay_id(base_image_collection)
 
       file_url = "https://example.com/ota.bin"
 
-      expect(StorageMock, :store, fn _, _ -> {:ok, file_url} end)
+      expect(StorageMock, :store, fn tenant_id, base_image_collection_id, _upload ->
+        assert tenant_id == tenant.tenant_id
+        # This is the DB id
+        assert base_image_collection_id == base_image_collection.id
+
+        {:ok, file_url}
+      end)
 
       result =
         create_base_image_mutation(
@@ -195,14 +199,14 @@ defmodule EdgehogWeb.Schema.Mutation.CreateBaseImageTest do
     end
 
     test "doesn't upload the base image to the storage with invalid data", %{tenant: tenant} do
-      expect(StorageMock, :store, 0, fn _, _ -> {:error, :unreachable} end)
+      expect(StorageMock, :store, 0, fn _, _, _ -> {:error, :unreachable} end)
       result = create_base_image_mutation(tenant: tenant, version: "invalid")
 
       assert %{message: "is not a valid version"} = extract_error!(result)
     end
 
     test "returns error if the upload to the storage fails", %{tenant: tenant} do
-      expect(StorageMock, :store, fn _, _ -> {:error, :bucket_is_full} end)
+      expect(StorageMock, :store, fn _, _, _ -> {:error, :bucket_is_full} end)
       result = create_base_image_mutation(tenant: tenant)
 
       assert %{