diff --git a/CHANGELOG.md b/CHANGELOG.md index 3f9000e59..e61ac2d6d 100644 --- a/CHANGELOG.md +++ b/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)). diff --git a/backend/lib/edgehog/base_images/base_image/base_image.ex b/backend/lib/edgehog/base_images/base_image/base_image.ex index 164d21133..fb24479ae 100644 --- a/backend/lib/edgehog/base_images/base_image/base_image.ex +++ b/backend/lib/edgehog/base_images/base_image/base_image.ex @@ -71,7 +71,7 @@ defmodule Edgehog.BaseImages.BaseImage do description "A list of release display names in different languages." end - change Changes.HandleFileUpload + change Changes.HandleFileUpload, only_when_valid?: true change manage_relationship(:base_image_collection_id, :base_image_collection, type: :append) change {UpsertLocalizedAttribute, input_argument: :localized_descriptions, target_attribute: :description} diff --git a/backend/lib/edgehog/base_images/base_image/changes/handle_file_upload.ex b/backend/lib/edgehog/base_images/base_image/changes/handle_file_upload.ex index 98a544bd9..2902c7362 100644 --- a/backend/lib/edgehog/base_images/base_image/changes/handle_file_upload.ex +++ b/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) diff --git a/backend/lib/edgehog/base_images/bucket_storage.ex b/backend/lib/edgehog/base_images/bucket_storage.ex index 50dab1c3a..25c018bee 100644 --- a/backend/lib/edgehog/base_images/bucket_storage.ex +++ b/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}) diff --git a/backend/lib/edgehog/base_images/storage.ex b/backend/lib/edgehog/base_images/storage.ex index 9de4ee320..ed6b03d38 100644 --- a/backend/lib/edgehog/base_images/storage.ex +++ b/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{}) :: diff --git a/backend/test/edgehog_web/schema/mutation/create_base_image_test.exs b/backend/test/edgehog_web/schema/mutation/create_base_image_test.exs index 84b05e7b2..e426b87d5 100644 --- a/backend/test/edgehog_web/schema/mutation/create_base_image_test.exs +++ b/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 %{