refac: allowed image types and the size limit are a domain rule checked by the use case, not by the persistence layer
This commit is contained in:
1 parent
8a8a24434a
commit
5992c8598b
7 files changed
+249
-17
No files matched your search
@@ -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 <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
|
||||||
@@ -3,10 +3,35 @@ package image
|
|||||||
import (
|
import (
|
||||||
"errors"
|
"errors"
|
||||||
"io"
|
"io"
|
||||||
|
"sort"
|
||||||
"time"
|
"time"
|
||||||
)
|
)
|
||||||
|
|
||||||
var ErrImageNotFound = errors.New("image not found")
|
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 {
|
type ImageInput struct {
|
||||||
Content io.Reader
|
Content io.Reader
|
||||||
|
|||||||
@@ -4,7 +4,6 @@ import (
|
|||||||
domain "RenewCMS/internal/domain/image"
|
domain "RenewCMS/internal/domain/image"
|
||||||
"RenewCMS/internal/infrastructure/persistence/mappers"
|
"RenewCMS/internal/infrastructure/persistence/mappers"
|
||||||
entity "RenewCMS/internal/infrastructure/persistence/models"
|
entity "RenewCMS/internal/infrastructure/persistence/models"
|
||||||
"errors"
|
|
||||||
"io"
|
"io"
|
||||||
"os"
|
"os"
|
||||||
"path/filepath"
|
"path/filepath"
|
||||||
@@ -42,9 +41,6 @@ func (i ImageRepository) Create(input domain.ImageInput) (domain.Image, error) {
|
|||||||
}
|
}
|
||||||
|
|
||||||
extension := contentTypeExtensions[input.ContentType]
|
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
|
newName := uuid.NewString() + extension
|
||||||
finalPath := filepath.Join(uploadDir, newName)
|
finalPath := filepath.Join(uploadDir, newName)
|
||||||
|
|||||||
@@ -13,5 +13,11 @@ func NewCreateImageUseCase(imageRepository image.Repository) *CreateImageUseCase
|
|||||||
}
|
}
|
||||||
|
|
||||||
func (g *CreateImageUseCase) CreateImage(input image.ImageInput) (image.Image, error) {
|
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)
|
return g.imageRepository.Create(input)
|
||||||
}
|
}
|
||||||
@@ -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)
|
||||||
|
}
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -3,21 +3,11 @@ package test
|
|||||||
import (
|
import (
|
||||||
imageDomain "RenewCMS/internal/domain/image"
|
imageDomain "RenewCMS/internal/domain/image"
|
||||||
"RenewCMS/internal/infrastructure/useCases"
|
"RenewCMS/internal/infrastructure/useCases"
|
||||||
"errors"
|
|
||||||
"io"
|
"io"
|
||||||
"strings"
|
"strings"
|
||||||
"testing"
|
"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 {
|
type mockImageRepo struct {
|
||||||
images map[uint32]imageDomain.Image
|
images map[uint32]imageDomain.Image
|
||||||
nextID uint32
|
nextID uint32
|
||||||
@@ -34,9 +24,6 @@ func (m *mockImageRepo) Create(input imageDomain.ImageInput) (imageDomain.Image,
|
|||||||
if err := m.fail.trigger("Create"); err != nil {
|
if err := m.fail.trigger("Create"); err != nil {
|
||||||
return imageDomain.Image{}, err
|
return imageDomain.Image{}, err
|
||||||
}
|
}
|
||||||
if !supportedImageContentTypes[input.ContentType] {
|
|
||||||
return imageDomain.Image{}, errUnsupportedImageContentType
|
|
||||||
}
|
|
||||||
if input.Content != nil {
|
if input.Content != nil {
|
||||||
_, _ = io.ReadAll(input.Content)
|
_, _ = io.ReadAll(input.Content)
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -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)
|
||||||
|
}
|
||||||
|
})
|
||||||
|
}
|
||||||
Reference in new issue
Block a user