From 5992c8598bc4586fabdda13a4578676da001acfa Mon Sep 17 00:00:00 2001 From: Florian Sylvain Date: Thu, 24 Sep 2026 00:03:29 +0200 Subject: [PATCH] refac: allowed image types and the size limit are a domain rule checked by the use case, not by the persistence layer --- features/image_upload_rules.feature | 65 +++++++++++ internal/domain/image/imageDomain.go | 25 ++++ .../repositories/imageRepository.go | 4 - .../infrastructure/useCases/CreateImage.go | 6 + test/image_extension_mapping_test.go | 43 +++++++ test/image_test.go | 13 --- test/image_upload_rules_test.go | 110 ++++++++++++++++++ 7 files changed, 249 insertions(+), 17 deletions(-) create mode 100644 features/image_upload_rules.feature create mode 100644 test/image_extension_mapping_test.go create mode 100644 test/image_upload_rules_test.go diff --git a/features/image_upload_rules.feature b/features/image_upload_rules.feature new file mode 100644 index 0000000..4c2ea30 --- /dev/null +++ b/features/image_upload_rules.feature @@ -0,0 +1,65 @@ +Feature: Allowed image types and the size limit are a domain rule + + As the maintainer of RenewCMS + I want the image content-type allow-list and the maximum upload size enforced by + CreateImageUseCase before it ever calls image.Repository.Create + So that persistence stops deciding what counts as a valid image, the rule lives in + internal/domain/image where it can be asserted with errors.Is, and no bytes are ever + written to UPLOAD_DIR or the database for a rejected upload + + Background: + Given internal/domain/image defines the sentinel errors image.ErrImageTypeNotAllowed + and image.ErrImageTooLarge + And internal/domain/image defines the allowed content types image/png, image/jpeg, + image/webp and image/svg+xml, and a maximum size of 10485760 bytes (10 MiB) + And CreateImageUseCase.CreateImage checks input.ContentType against the allowed list + and input.Size against the maximum before calling imageRepository.Create, returning + the matching sentinel and calling the repository not at all when either check fails + And ImageRepository.Create no longer rejects an unrecognized content type itself; it + only maps an already-allowed content type to a file extension + And no image HTTP endpoint is mounted in api/router.go as of this slice, so this + behavior is observable only through CreateImageUseCase + + Scenario Outline: CreateImage accepts every allowed content type within the size limit + Given an empty image repository fake + When CreateImage is called with content type and a size of + 10485760 bytes + Then the image repository's Create is called once + And the result is the created image, with no error + + Examples: + | content_type | + | image/png | + | image/jpeg | + | image/webp | + | image/svg+xml | + + Scenario: CreateImage rejects a disallowed content type before reaching the repository + Given an empty image repository fake + When CreateImage is called with content type "application/pdf" and a size of 1024 bytes + Then the result is image.ErrImageTypeNotAllowed, asserted with errors.Is + And the image repository's Create is never called + + Scenario: CreateImage rejects a file that exceeds the size limit + Given an empty image repository fake + When CreateImage is called with content type "image/png" and a size of + 10485761 bytes + Then the result is image.ErrImageTooLarge, asserted with errors.Is + And the image repository's Create is never called + + Scenario: A file exactly at the size limit is accepted, one byte over is not + Given an empty image repository fake + When CreateImage is called with content type "image/png" and a size of + 10485760 bytes + Then the image repository's Create is called once with no error + When CreateImage is called again with content type "image/png" and a size of + 10485761 bytes + Then the result is image.ErrImageTooLarge, asserted with errors.Is + And the image repository's Create is still called exactly once in total + + Scenario: A repository failure unrelated to type or size still propagates unchanged + Given an image repository fake configured to fail its next Create call with a + generic repository error + When CreateImage is called with content type "image/png" and a size of 1024 bytes + Then the image repository's Create is called once + And the result is that generic error, unchanged diff --git a/internal/domain/image/imageDomain.go b/internal/domain/image/imageDomain.go index ad91adf..4649d95 100644 --- a/internal/domain/image/imageDomain.go +++ b/internal/domain/image/imageDomain.go @@ -3,10 +3,35 @@ package image import ( "errors" "io" + "sort" "time" ) var ErrImageNotFound = errors.New("image not found") +var ErrImageTypeNotAllowed = errors.New("the file must be a PNG, JPEG, WEBP, or SVG image") +var ErrImageTooLarge = errors.New("the file exceeds the maximum upload size") + +const MaxImageSize int64 = 10485760 + +var allowedContentTypes = map[string]bool{ + "image/png": true, + "image/jpeg": true, + "image/webp": true, + "image/svg+xml": true, +} + +func IsAllowedContentType(contentType string) bool { + return allowedContentTypes[contentType] +} + +func AllowedContentTypes() []string { + types := make([]string, 0, len(allowedContentTypes)) + for contentType := range allowedContentTypes { + types = append(types, contentType) + } + sort.Strings(types) + return types +} type ImageInput struct { Content io.Reader diff --git a/internal/infrastructure/persistence/repositories/imageRepository.go b/internal/infrastructure/persistence/repositories/imageRepository.go index bc1f160..d5329d9 100644 --- a/internal/infrastructure/persistence/repositories/imageRepository.go +++ b/internal/infrastructure/persistence/repositories/imageRepository.go @@ -4,7 +4,6 @@ import ( domain "RenewCMS/internal/domain/image" "RenewCMS/internal/infrastructure/persistence/mappers" entity "RenewCMS/internal/infrastructure/persistence/models" - "errors" "io" "os" "path/filepath" @@ -42,9 +41,6 @@ func (i ImageRepository) Create(input domain.ImageInput) (domain.Image, error) { } extension := contentTypeExtensions[input.ContentType] - if extension == "" { - return domain.Image{}, errors.New("the file must be a PNG, JPEG, WEBP, or SVG image") - } newName := uuid.NewString() + extension finalPath := filepath.Join(uploadDir, newName) diff --git a/internal/infrastructure/useCases/CreateImage.go b/internal/infrastructure/useCases/CreateImage.go index cc9c18f..833c17c 100644 --- a/internal/infrastructure/useCases/CreateImage.go +++ b/internal/infrastructure/useCases/CreateImage.go @@ -13,5 +13,11 @@ func NewCreateImageUseCase(imageRepository image.Repository) *CreateImageUseCase } func (g *CreateImageUseCase) CreateImage(input image.ImageInput) (image.Image, error) { + if !image.IsAllowedContentType(input.ContentType) { + return image.Image{}, image.ErrImageTypeNotAllowed + } + if input.Size > image.MaxImageSize { + return image.Image{}, image.ErrImageTooLarge + } return g.imageRepository.Create(input) } diff --git a/test/image_extension_mapping_test.go b/test/image_extension_mapping_test.go new file mode 100644 index 0000000..19e8e3c --- /dev/null +++ b/test/image_extension_mapping_test.go @@ -0,0 +1,43 @@ +package test + +import ( + imageDomain "RenewCMS/internal/domain/image" + "RenewCMS/internal/infrastructure/persistence/models" + persistence "RenewCMS/internal/infrastructure/persistence/repositories" + "path/filepath" + "reflect" + "strings" + "testing" +) + +func TestImageRepositoryHasAnExtensionForEveryAllowedContentType(t *testing.T) { + db := openInMemoryDb(t) + if err := db.AutoMigrate(&models.Image{}); err != nil { + t.Fatalf("failed to migrate: %v", err) + } + repo := persistence.NewImageRepository(db) + t.Setenv("UPLOAD_DIR", t.TempDir()) + + allowedContentTypes := imageDomain.AllowedContentTypes() + expectedContentTypes := []string{"image/jpeg", "image/png", "image/svg+xml", "image/webp"} + if !reflect.DeepEqual(allowedContentTypes, expectedContentTypes) { + t.Fatalf("expected AllowedContentTypes() to be %v, got %v", expectedContentTypes, allowedContentTypes) + } + + for _, contentType := range allowedContentTypes { + t.Run(contentType, func(t *testing.T) { + result, err := repo.Create(imageDomain.ImageInput{ + Content: strings.NewReader("fake-image-data"), + Filename: "upload", + ContentType: contentType, + }) + if err != nil { + t.Fatalf("Create failed: %v", err) + } + extension := filepath.Ext(result.Path) + if extension == "" { + t.Errorf("expected a non-empty extension for %q, got path %q", contentType, result.Path) + } + }) + } +} diff --git a/test/image_test.go b/test/image_test.go index aa642c3..8703147 100644 --- a/test/image_test.go +++ b/test/image_test.go @@ -3,21 +3,11 @@ package test import ( imageDomain "RenewCMS/internal/domain/image" "RenewCMS/internal/infrastructure/useCases" - "errors" "io" "strings" "testing" ) -var errUnsupportedImageContentType = errors.New("the file must be a PNG, JPEG, WEBP, or SVG image") - -var supportedImageContentTypes = map[string]bool{ - "image/png": true, - "image/jpeg": true, - "image/webp": true, - "image/svg+xml": true, -} - type mockImageRepo struct { images map[uint32]imageDomain.Image nextID uint32 @@ -34,9 +24,6 @@ func (m *mockImageRepo) Create(input imageDomain.ImageInput) (imageDomain.Image, if err := m.fail.trigger("Create"); err != nil { return imageDomain.Image{}, err } - if !supportedImageContentTypes[input.ContentType] { - return imageDomain.Image{}, errUnsupportedImageContentType - } if input.Content != nil { _, _ = io.ReadAll(input.Content) } diff --git a/test/image_upload_rules_test.go b/test/image_upload_rules_test.go new file mode 100644 index 0000000..bda7220 --- /dev/null +++ b/test/image_upload_rules_test.go @@ -0,0 +1,110 @@ +package test + +import ( + imageDomain "RenewCMS/internal/domain/image" + "RenewCMS/internal/infrastructure/useCases" + "errors" + "testing" +) + +func TestImageUploadRules(t *testing.T) { + t.Run("CreateImage accepts every allowed content type within the size limit", func(t *testing.T) { + for _, contentType := range []string{"image/png", "image/jpeg", "image/webp", "image/svg+xml"} { + t.Run(contentType, func(t *testing.T) { + repo := newMockImageRepo() + createUc := useCases.NewCreateImageUseCase(repo) + + result, err := createUc.CreateImage(imageDomain.ImageInput{ + ContentType: contentType, + Size: imageDomain.MaxImageSize, + }) + + if err != nil { + t.Fatalf("expected no error, got %v", err) + } + if result.ID == 0 { + t.Errorf("expected the created image, got zero value") + } + if len(repo.images) != 1 { + t.Errorf("expected Create to be called once, got %d calls", len(repo.images)) + } + }) + } + }) + + t.Run("CreateImage rejects a disallowed content type before reaching the repository", func(t *testing.T) { + repo := newMockImageRepo() + createUc := useCases.NewCreateImageUseCase(repo) + + _, err := createUc.CreateImage(imageDomain.ImageInput{ + ContentType: "application/pdf", + Size: 1024, + }) + + if !errors.Is(err, imageDomain.ErrImageTypeNotAllowed) { + t.Fatalf("expected ErrImageTypeNotAllowed, got %v", err) + } + if len(repo.images) != 0 { + t.Errorf("expected Create to never be called, got %d calls", len(repo.images)) + } + }) + + t.Run("CreateImage rejects a file that exceeds the size limit", func(t *testing.T) { + repo := newMockImageRepo() + createUc := useCases.NewCreateImageUseCase(repo) + + _, err := createUc.CreateImage(imageDomain.ImageInput{ + ContentType: "image/png", + Size: imageDomain.MaxImageSize + 1, + }) + + if !errors.Is(err, imageDomain.ErrImageTooLarge) { + t.Fatalf("expected ErrImageTooLarge, got %v", err) + } + if len(repo.images) != 0 { + t.Errorf("expected Create to never be called, got %d calls", len(repo.images)) + } + }) + + t.Run("A file exactly at the size limit is accepted, one byte over is not", func(t *testing.T) { + repo := newMockImageRepo() + createUc := useCases.NewCreateImageUseCase(repo) + + _, err := createUc.CreateImage(imageDomain.ImageInput{ + ContentType: "image/png", + Size: imageDomain.MaxImageSize, + }) + if err != nil { + t.Fatalf("expected no error at the exact limit, got %v", err) + } + if len(repo.images) != 1 { + t.Fatalf("expected Create to be called once, got %d calls", len(repo.images)) + } + + _, err = createUc.CreateImage(imageDomain.ImageInput{ + ContentType: "image/png", + Size: imageDomain.MaxImageSize + 1, + }) + if !errors.Is(err, imageDomain.ErrImageTooLarge) { + t.Fatalf("expected ErrImageTooLarge, got %v", err) + } + if len(repo.images) != 1 { + t.Errorf("expected Create to still be called exactly once in total, got %d calls", len(repo.images)) + } + }) + + t.Run("A repository failure unrelated to type or size still propagates unchanged", func(t *testing.T) { + repo := newMockImageRepo() + repo.fail.failNextWith("Create", errRepositoryFailure) + createUc := useCases.NewCreateImageUseCase(repo) + + _, err := createUc.CreateImage(imageDomain.ImageInput{ + ContentType: "image/png", + Size: 1024, + }) + + if !errors.Is(err, errRepositoryFailure) { + t.Fatalf("expected the generic repository error, got %v", err) + } + }) +}