From 0d99544579da98f3aa21891ff08e5affacf72976 Mon Sep 17 00:00:00 2001 From: Florian Sylvain Date: Wed, 30 Sep 2026 18:35:28 +0200 Subject: [PATCH] feat(API): media library --- api/dependecyInjection.go | 24 +- api/handlers/image/imageHandler.go | 67 +- api/handlers/media/mediaHandler.go | 317 +++++ api/pkg/apperr/messages.go | 4 + api/pkg/upload/upload.go | 49 + api/router.go | 25 +- docs/architecture.md | 7 + .../delete_article_cascades_images.feature | 84 -- features/delete_article_cascades_images.qa.md | 67 - features/delete_article_keeps_images.feature | 39 + features/delete_article_keeps_images.qa.md | 12 + features/image_upload_rules.feature | 4 +- features/media_library.feature | 391 ++++++ features/media_library.qa.md | 39 + features/remove_article_image.feature | 99 -- features/remove_article_image.qa.md | 56 - features/upload_article_image.feature | 128 +- features/upload_article_image.qa.md | 76 +- go.mod | 5 +- go.sum | 22 +- internal/domain/article/articleDomain.go | 14 +- .../article/articleRepositoryInterface.go | 2 +- internal/domain/image/imageDomain.go | 175 ++- .../domain/image/imageRepositoryInterface.go | 11 + .../persistence/mappers/articleMapper.go | 15 - .../persistence/mappers/imageMapper.go | 31 +- .../persistence/models/articleModel.go | 1 - .../persistence/models/imageModel.go | 16 +- .../repositories/articleRepository.go | 28 +- .../persistence/repositories/imageBackfill.go | 77 ++ .../repositories/imageRepository.go | 131 +- .../infrastructure/useCases/CreateImage.go | 84 +- .../infrastructure/useCases/DeleteArticle.go | 21 +- .../infrastructure/useCases/DeleteImage.go | 15 - .../infrastructure/useCases/DeleteMedia.go | 15 + .../infrastructure/useCases/GetMediaUsage.go | 23 + internal/infrastructure/useCases/ListMedia.go | 199 +++ .../infrastructure/useCases/MergeMedia.go | 137 +++ .../infrastructure/useCases/RenameMedia.go | 25 + .../infrastructure/useCases/UpdateArticle.go | 4 - .../infrastructure/useCases/mediaUploader.go | 40 + test/article_publishing_http_test.go | 2 +- test/article_test.go | 39 +- test/article_title_rules_test.go | 2 +- test/articles_list_http_test.go | 38 +- test/delete_article_cascades_images_test.go | 230 ---- test/delete_article_keeps_images_test.go | 84 ++ test/edit_article_body_http_test.go | 2 +- test/image_test.go | 152 ++- test/image_upload_rules_test.go | 57 +- test/media_cleanup_test.go | 72 ++ test/media_env_test.go | 160 +++ test/media_fixtures_test.go | 103 ++ test/media_library_http_test.go | 1073 +++++++++++++++++ test/media_persistence_test.go | 253 ++++ test/not_found_errors_test.go | 2 +- test/public_article_listing_test.go | 2 +- test/public_article_read_test.go | 2 +- test/remove_article_image_http_test.go | 196 --- test/runtime_settings_injection_http_test.go | 8 +- test/stale_token_invalidation_http_test.go | 2 +- test/upload_article_image_http_test.go | 377 ++---- test/use_case_baseline_test.go | 58 +- test/verified_account_required_http_test.go | 6 +- test/write_not_found_errors_test.go | 23 +- 65 files changed, 3992 insertions(+), 1530 deletions(-) create mode 100644 api/handlers/media/mediaHandler.go create mode 100644 api/pkg/upload/upload.go delete mode 100644 features/delete_article_cascades_images.feature delete mode 100644 features/delete_article_cascades_images.qa.md create mode 100644 features/delete_article_keeps_images.feature create mode 100644 features/delete_article_keeps_images.qa.md create mode 100644 features/media_library.feature create mode 100644 features/media_library.qa.md delete mode 100644 features/remove_article_image.feature delete mode 100644 features/remove_article_image.qa.md create mode 100644 internal/infrastructure/persistence/repositories/imageBackfill.go delete mode 100644 internal/infrastructure/useCases/DeleteImage.go create mode 100644 internal/infrastructure/useCases/DeleteMedia.go create mode 100644 internal/infrastructure/useCases/GetMediaUsage.go create mode 100644 internal/infrastructure/useCases/ListMedia.go create mode 100644 internal/infrastructure/useCases/MergeMedia.go create mode 100644 internal/infrastructure/useCases/RenameMedia.go create mode 100644 internal/infrastructure/useCases/mediaUploader.go delete mode 100644 test/delete_article_cascades_images_test.go create mode 100644 test/delete_article_keeps_images_test.go create mode 100644 test/media_cleanup_test.go create mode 100644 test/media_env_test.go create mode 100644 test/media_fixtures_test.go create mode 100644 test/media_library_http_test.go create mode 100644 test/media_persistence_test.go delete mode 100644 test/remove_article_image_http_test.go diff --git a/api/dependecyInjection.go b/api/dependecyInjection.go index 8a43da2..e181870 100644 --- a/api/dependecyInjection.go +++ b/api/dependecyInjection.go @@ -45,7 +45,11 @@ type UseCases struct { AcceptInviteUseCase *useCases.AcceptInviteUseCase SendMailUseCase *useCases.SendMailUseCase CreateImageUseCase *useCases.CreateImageUseCase - DeleteImageUseCase *useCases.DeleteImageUseCase + ListMediaUseCase *useCases.ListMediaUseCase + RenameMediaUseCase *useCases.RenameMediaUseCase + DeleteMediaUseCase *useCases.DeleteMediaUseCase + MergeMediaUseCase *useCases.MergeMediaUseCase + GetMediaUsageUseCase *useCases.GetMediaUsageUseCase GetSettingsUseCase *useCases.GetSettingsUseCase } @@ -63,7 +67,7 @@ func SQLiteDSN(dbName string) string { return fmt.Sprintf("%s?_pragma=busy_timeout(%d)", dbName, sqliteBusyTimeoutMs) } -func getDb(dbName string) *gorm.DB { +func getDb(dbName string, uploadDir string) *gorm.DB { if err := os.MkdirAll(filepath.Dir(dbName), os.ModePerm); err != nil { panic("Unable to create necessary subdirectories: " + err.Error()) } @@ -72,7 +76,7 @@ func getDb(dbName string) *gorm.DB { panic("Unable to open the database: " + err.Error()) } - if err := db.AutoMigrate(&models.Article{}, &models.User{}, &models.Invite{}); err != nil { + if err := db.AutoMigrate(&models.Article{}, &models.Image{}, &models.User{}, &models.Invite{}); err != nil { panic("Failed to migrate database: " + err.Error()) } @@ -80,6 +84,10 @@ func getDb(dbName string) *gorm.DB { panic("Failed to backfill user roles: " + err.Error()) } + if err := persistence.BackfillImageMetadata(db, uploadDir); err != nil { + panic("Failed to backfill image metadata: " + err.Error()) + } + return db } @@ -102,7 +110,7 @@ func getUseCases(repos *Repositories, settings config.Settings) *UseCases { GetArticleUseCase: useCases.NewGetArticleUseCase(repos.ArticleRepo), ListArticlesUseCase: useCases.NewListArticlesUseCase(repos.ArticleRepo), UpdateArticleUseCase: useCases.NewUpdateArticleUseCase(repos.ArticleRepo), - DeleteArticleUseCase: useCases.NewDeleteArticleUseCase(repos.ArticleRepo, repos.ImageRepo), + DeleteArticleUseCase: useCases.NewDeleteArticleUseCase(repos.ArticleRepo), GetUserUseCase: useCases.NewGetUserUseCase(repos.UserRepo), CreateUserUseCase: useCases.NewCreateUserUseCase(repos.UserRepo, hasher, repos.MailRepo, settings.PublicURL), AuthenticateUserUseCase: useCases.NewAuthenticateUserUseCase(repos.UserRepo, hasher), @@ -120,13 +128,17 @@ func getUseCases(repos *Repositories, settings config.Settings) *UseCases { AcceptInviteUseCase: useCases.NewAcceptInviteUseCase(repos.UserRepo, repos.InviteRepo, hasher), SendMailUseCase: useCases.NewSendMailUseCase(repos.MailRepo), CreateImageUseCase: useCases.NewCreateImageUseCase(repos.ImageRepo), - DeleteImageUseCase: useCases.NewDeleteImageUseCase(repos.ImageRepo), + ListMediaUseCase: useCases.NewListMediaUseCase(repos.ImageRepo, repos.ArticleRepo, repos.UserRepo), + RenameMediaUseCase: useCases.NewRenameMediaUseCase(repos.ImageRepo), + DeleteMediaUseCase: useCases.NewDeleteMediaUseCase(repos.ImageRepo), + MergeMediaUseCase: useCases.NewMergeMediaUseCase(repos.ImageRepo, repos.ArticleRepo), + GetMediaUsageUseCase: useCases.NewGetMediaUsageUseCase(repos.ImageRepo, repos.ArticleRepo), GetSettingsUseCase: useCases.NewGetSettingsUseCase(settings.Version), } } func InitContainer(settings config.Settings) (*UseCases, func() error) { - db := getDb(settings.DBFile) + db := getDb(settings.DBFile, settings.UploadDir) repos := getRepositories(db, settings) return getUseCases(repos, settings), closeDb(db) } diff --git a/api/handlers/image/imageHandler.go b/api/handlers/image/imageHandler.go index fa82e56..9ac5f1e 100644 --- a/api/handlers/image/imageHandler.go +++ b/api/handlers/image/imageHandler.go @@ -2,8 +2,9 @@ package image import ( "RenewCMS/api/pkg/apperr" + "RenewCMS/api/pkg/claims" + "RenewCMS/api/pkg/upload" domainArticle "RenewCMS/internal/domain/article" - domainImage "RenewCMS/internal/domain/image" "RenewCMS/internal/infrastructure/useCases" "encoding/json" "errors" @@ -16,16 +17,12 @@ import ( type Handler struct { CreateUseCase *useCases.CreateImageUseCase GetUseCase *useCases.GetArticleUseCase - UpdateUseCase *useCases.UpdateArticleUseCase - DeleteUseCase *useCases.DeleteImageUseCase } -func NewHandler(create *useCases.CreateImageUseCase, get *useCases.GetArticleUseCase, update *useCases.UpdateArticleUseCase, deleteUseCase *useCases.DeleteImageUseCase) *Handler { +func NewHandler(create *useCases.CreateImageUseCase, get *useCases.GetArticleUseCase) *Handler { return &Handler{ CreateUseCase: create, GetUseCase: get, - UpdateUseCase: update, - DeleteUseCase: deleteUseCase, } } @@ -41,7 +38,7 @@ func writeArticleError(w http.ResponseWriter, err error) { writeInternalServerError(w) } -func (h *Handler) PostImage(w http.ResponseWriter, r *http.Request) { +func (h *Handler) PostLegacyArticleImage(w http.ResponseWriter, r *http.Request) { articleId, err := strconv.ParseUint(chi.URLParam(r, "id"), 10, 32) if err != nil { http.Error(w, apperr.IdUint32ErrorMessage, http.StatusBadRequest) @@ -53,30 +50,16 @@ func (h *Handler) PostImage(w http.ResponseWriter, r *http.Request) { return } - file, fileHeader, err := r.FormFile("file") - if err != nil { - http.Error(w, err.Error(), http.StatusBadRequest) + userId, _ := claims.UserId(r) + input, closeFile, ok := upload.ReadImageInput(w, r, userId) + if !ok { return } - defer file.Close() + defer closeFile() - newImage, err := h.CreateUseCase.CreateImage(domainImage.ImageInput{ - Content: file, - Filename: fileHeader.Filename, - Size: fileHeader.Size, - ContentType: fileHeader.Header.Get("Content-Type"), - }) + newImage, err := h.CreateUseCase.CreateImage(input) if err != nil { - if errors.Is(err, domainImage.ErrImageTypeNotAllowed) || errors.Is(err, domainImage.ErrImageTooLarge) { - http.Error(w, err.Error(), http.StatusBadRequest) - return - } - writeInternalServerError(w) - return - } - - if err := h.UpdateUseCase.AddImage(uint32(articleId), newImage.ID); err != nil { - writeArticleError(w, err) + upload.WriteCreateError(w, err) return } @@ -89,33 +72,3 @@ func (h *Handler) PostImage(w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusCreated) _, _ = w.Write(responseJson) } - -func (h *Handler) DeleteImage(w http.ResponseWriter, r *http.Request) { - articleId, err := strconv.ParseUint(chi.URLParam(r, "id"), 10, 32) - if err != nil { - http.Error(w, apperr.IdUint32ErrorMessage, http.StatusBadRequest) - return - } - - imageId, err := strconv.ParseUint(chi.URLParam(r, "imageId"), 10, 32) - if err != nil { - http.Error(w, apperr.IdUint32ErrorMessage, http.StatusBadRequest) - return - } - - if _, err := h.GetUseCase.GetArticle(uint32(articleId)); err != nil { - writeArticleError(w, err) - return - } - - if err := h.DeleteUseCase.DeleteImage(uint32(imageId)); err != nil { - if errors.Is(err, domainImage.ErrImageNotFound) { - http.Error(w, apperr.ImageNotFoundErrorMessage, http.StatusNotFound) - return - } - writeInternalServerError(w) - return - } - - w.WriteHeader(http.StatusNoContent) -} diff --git a/api/handlers/media/mediaHandler.go b/api/handlers/media/mediaHandler.go new file mode 100644 index 0000000..274d895 --- /dev/null +++ b/api/handlers/media/mediaHandler.go @@ -0,0 +1,317 @@ +package media + +import ( + "RenewCMS/api/pkg/apperr" + "RenewCMS/api/pkg/claims" + "RenewCMS/api/pkg/upload" + validator "RenewCMS/api/pkg/validator" + domainImage "RenewCMS/internal/domain/image" + "RenewCMS/internal/infrastructure/useCases" + "encoding/json" + "errors" + "net/http" + "net/url" + "strconv" + "strings" + "time" + + "github.com/go-chi/chi/v5" +) + +type Handler struct { + CreateUseCase *useCases.CreateImageUseCase + ListUseCase *useCases.ListMediaUseCase + RenameUseCase *useCases.RenameMediaUseCase + DeleteUseCase *useCases.DeleteMediaUseCase + MergeUseCase *useCases.MergeMediaUseCase + UsageUseCase *useCases.GetMediaUsageUseCase +} + +type RenameRequest struct { + Filename string `json:"filename" validate:"required"` +} + +type MergeRequest struct { + KeepID uint32 `json:"keep_id" validate:"required,gt=0"` + MergeIDs []uint32 `json:"merge_ids" validate:"required,min=1,dive,gt=0"` +} + +type mediaItemResponse struct { + ID uint32 `json:"id"` + Filename string `json:"filename"` + URL string `json:"url"` + Size int64 `json:"size"` + ContentType string `json:"content_type"` + Width *int `json:"width"` + Height *int `json:"height"` + SHA256 string `json:"sha256"` + UploadedBy string `json:"uploaded_by"` + CreatedAt time.Time `json:"created_at"` + UsedIn int `json:"used_in"` + Duplicate bool `json:"duplicate"` +} + +type countsResponse struct { + All int `json:"all"` + Unused int `json:"unused"` + Duplicates int `json:"duplicates"` +} + +type listResponse struct { + Items []mediaItemResponse `json:"items"` + NextCursor *string `json:"next_cursor"` + Counts countsResponse `json:"counts"` +} + +type usageResponse struct { + ID uint32 `json:"id"` + Title string `json:"title"` + Published bool `json:"published"` +} + +type mergeResponse struct { + ArticlesUpdated int `json:"articles_updated"` + Merged int `json:"merged"` +} + +func NewHandler(create *useCases.CreateImageUseCase, list *useCases.ListMediaUseCase, rename *useCases.RenameMediaUseCase, deleteUseCase *useCases.DeleteMediaUseCase, merge *useCases.MergeMediaUseCase, usage *useCases.GetMediaUsageUseCase) *Handler { + return &Handler{ + CreateUseCase: create, + ListUseCase: list, + RenameUseCase: rename, + DeleteUseCase: deleteUseCase, + MergeUseCase: merge, + UsageUseCase: usage, + } +} + +func writeInternalServerError(w http.ResponseWriter) { + http.Error(w, "Internal server error", http.StatusInternalServerError) +} + +func writeJSON(w http.ResponseWriter, status int, payload any) { + body, err := json.Marshal(payload) + if err != nil { + writeInternalServerError(w) + return + } + w.WriteHeader(status) + _, _ = w.Write(body) +} + +func writeImageError(w http.ResponseWriter, err error) { + if errors.Is(err, domainImage.ErrImageNotFound) { + http.Error(w, apperr.ImageNotFoundErrorMessage, http.StatusNotFound) + return + } + writeInternalServerError(w) +} + +func parseImageId(w http.ResponseWriter, r *http.Request) (uint32, bool) { + id, err := strconv.ParseUint(chi.URLParam(r, "id"), 10, 32) + if err != nil { + http.Error(w, apperr.IdUint32ErrorMessage, http.StatusBadRequest) + return 0, false + } + return uint32(id), true +} + +func dimension(value int) *int { + if value == 0 { + return nil + } + return &value +} + +func toResponse(item useCases.MediaItem) mediaItemResponse { + return mediaItemResponse{ + ID: item.Image.ID, + Filename: item.Image.Filename, + URL: item.Image.Path, + Size: item.Image.Size, + ContentType: item.Image.ContentType, + Width: dimension(item.Image.Width), + Height: dimension(item.Image.Height), + SHA256: item.Image.SHA256, + UploadedBy: item.UploadedBy, + CreatedAt: item.Image.CreatedAt, + UsedIn: item.UsedIn, + Duplicate: item.Duplicate, + } +} + +func parseListQuery(r *http.Request) (domainImage.ListQuery, error) { + values, err := url.ParseQuery(r.URL.RawQuery) + if err != nil { + return domainImage.ListQuery{}, domainImage.ErrInvalidQuery + } + + query := domainImage.NewListQuery() + query.Search = values.Get("q") + query.Hash = strings.ToLower(values.Get("hash")) + query.Cursor = values.Get("cursor") + if values.Has("filter") { + query.Filter = domainImage.Filter(values.Get("filter")) + } + if values.Has("sort") { + query.Sort = domainImage.Sort(values.Get("sort")) + } + if values.Has("limit") { + limit, err := strconv.Atoi(values.Get("limit")) + if err != nil { + return domainImage.ListQuery{}, domainImage.ErrInvalidQuery + } + query.Limit = limit + } + return query, nil +} + +func (h *Handler) List(w http.ResponseWriter, r *http.Request) { + query, err := parseListQuery(r) + if err != nil { + http.Error(w, apperr.MediaQueryErrorMessage, http.StatusBadRequest) + return + } + + page, err := h.ListUseCase.List(query) + if err != nil { + if errors.Is(err, domainImage.ErrInvalidQuery) { + http.Error(w, apperr.MediaQueryErrorMessage, http.StatusBadRequest) + return + } + writeInternalServerError(w) + return + } + + items := make([]mediaItemResponse, 0, len(page.Items)) + for _, item := range page.Items { + items = append(items, toResponse(item)) + } + response := listResponse{ + Items: items, + Counts: countsResponse{All: page.Counts.All, Unused: page.Counts.Unused, Duplicates: page.Counts.Duplicates}, + } + if page.NextCursor != "" { + response.NextCursor = &page.NextCursor + } + writeJSON(w, http.StatusOK, response) +} + +func (h *Handler) Upload(w http.ResponseWriter, r *http.Request) { + userId, _ := claims.UserId(r) + input, closeFile, ok := upload.ReadImageInput(w, r, userId) + if !ok { + return + } + defer closeFile() + + created, err := h.CreateUseCase.CreateImage(input) + if err != nil { + upload.WriteCreateError(w, err) + return + } + + h.writeItem(w, http.StatusCreated, created) +} + +func (h *Handler) writeItem(w http.ResponseWriter, status int, img domainImage.Image) { + item, err := h.ListUseCase.Describe(img) + if err != nil { + writeInternalServerError(w) + return + } + writeJSON(w, status, toResponse(item)) +} + +func (h *Handler) Rename(w http.ResponseWriter, r *http.Request) { + id, ok := parseImageId(w, r) + if !ok { + return + } + + var request RenameRequest + if err := json.NewDecoder(r.Body).Decode(&request); err != nil { + http.Error(w, apperr.BodyErrorMessage, http.StatusBadRequest) + return + } + if err := validator.Validate.Struct(request); err != nil { + http.Error(w, apperr.MediaFilenameErrorMessage, http.StatusBadRequest) + return + } + + renamed, err := h.RenameUseCase.Rename(id, request.Filename) + if err != nil { + if errors.Is(err, domainImage.ErrInvalidFilename) { + http.Error(w, apperr.MediaFilenameErrorMessage, http.StatusBadRequest) + return + } + writeImageError(w, err) + return + } + + h.writeItem(w, http.StatusOK, renamed) +} + +func (h *Handler) Delete(w http.ResponseWriter, r *http.Request) { + id, ok := parseImageId(w, r) + if !ok { + return + } + + if err := h.DeleteUseCase.Delete(id); err != nil { + writeImageError(w, err) + return + } + + w.WriteHeader(http.StatusNoContent) +} + +func (h *Handler) Usage(w http.ResponseWriter, r *http.Request) { + id, ok := parseImageId(w, r) + if !ok { + return + } + + articles, err := h.UsageUseCase.Usage(id) + if err != nil { + writeImageError(w, err) + return + } + + response := make([]usageResponse, 0, len(articles)) + for _, a := range articles { + response = append(response, usageResponse{ID: a.ID, Title: a.Title, Published: a.IsOnline}) + } + writeJSON(w, http.StatusOK, response) +} + +func (h *Handler) Merge(w http.ResponseWriter, r *http.Request) { + var request MergeRequest + if err := json.NewDecoder(r.Body).Decode(&request); err != nil { + http.Error(w, apperr.MediaMergeRequestErrorMessage, http.StatusBadRequest) + return + } + if err := validator.Validate.Struct(request); err != nil { + http.Error(w, apperr.MediaMergeRequestErrorMessage, http.StatusBadRequest) + return + } + + result, err := h.MergeUseCase.Merge(request.KeepID, request.MergeIDs) + if err != nil { + writeMergeError(w, err) + return + } + + writeJSON(w, http.StatusOK, mergeResponse{ArticlesUpdated: result.ArticlesUpdated, Merged: result.Merged}) +} + +func writeMergeError(w http.ResponseWriter, err error) { + switch { + case errors.Is(err, domainImage.ErrInvalidMergeRequest): + http.Error(w, apperr.MediaMergeRequestErrorMessage, http.StatusBadRequest) + case errors.Is(err, domainImage.ErrMergeCopyMissing), errors.Is(err, domainImage.ErrMergeHashMismatch): + http.Error(w, apperr.MediaMergeConflictErrorMessage, http.StatusConflict) + default: + writeImageError(w, err) + } +} diff --git a/api/pkg/apperr/messages.go b/api/pkg/apperr/messages.go index eee780b..5204d86 100644 --- a/api/pkg/apperr/messages.go +++ b/api/pkg/apperr/messages.go @@ -25,3 +25,7 @@ const InviteInvalidErrorMessage = "This invite link doesn't work." const InviteExpiredErrorMessage = "This invite has expired." const OwnerRequiredErrorMessage = "Only the owner can do this." const UsernameTakenErrorMessage = "That username is taken. Try another." +const MediaQueryErrorMessage = "The media query is invalid." +const MediaFilenameErrorMessage = "Enter a filename." +const MediaMergeConflictErrorMessage = "Some of the images to merge changed. Refresh and try again." +const MediaMergeRequestErrorMessage = "Choose an image to keep and at least one other image to merge into it." diff --git a/api/pkg/upload/upload.go b/api/pkg/upload/upload.go new file mode 100644 index 0000000..97e91f0 --- /dev/null +++ b/api/pkg/upload/upload.go @@ -0,0 +1,49 @@ +package upload + +import ( + "RenewCMS/api/pkg/apperr" + "RenewCMS/internal/domain/image" + "errors" + "net/http" +) + +const multipartOverheadAllowance = 1 << 20 + +const fileField = "file" + +func ReadImageInput(w http.ResponseWriter, r *http.Request, uploadedById uint32) (image.ImageInput, func(), bool) { + r.Body = http.MaxBytesReader(w, r.Body, image.MaxImageSize+multipartOverheadAllowance) + + file, header, err := r.FormFile(fileField) + if err != nil { + var tooLarge *http.MaxBytesError + if errors.As(err, &tooLarge) { + http.Error(w, image.ErrImageTooLarge.Error(), http.StatusRequestEntityTooLarge) + return image.ImageInput{}, nil, false + } + http.Error(w, apperr.BodyErrorMessage, http.StatusBadRequest) + return image.ImageInput{}, nil, false + } + + input := image.ImageInput{ + Content: file, + Filename: header.Filename, + Size: header.Size, + ContentType: header.Header.Get("Content-Type"), + UploadedByID: uploadedById, + } + return input, func() { _ = file.Close() }, true +} + +func WriteCreateError(w http.ResponseWriter, err error) { + switch { + case errors.Is(err, image.ErrImageTypeNotAllowed): + http.Error(w, err.Error(), http.StatusUnsupportedMediaType) + case errors.Is(err, image.ErrImageTooLarge): + http.Error(w, err.Error(), http.StatusRequestEntityTooLarge) + case errors.Is(err, image.ErrImageUnreadable): + http.Error(w, err.Error(), http.StatusBadRequest) + default: + http.Error(w, "Internal server error", http.StatusInternalServerError) + } +} diff --git a/api/router.go b/api/router.go index 63912e8..03e4c7f 100644 --- a/api/router.go +++ b/api/router.go @@ -5,6 +5,7 @@ import ( "RenewCMS/api/handlers/auth" "RenewCMS/api/handlers/frontend" "RenewCMS/api/handlers/image" + "RenewCMS/api/handlers/media" "RenewCMS/api/handlers/settings" "RenewCMS/api/handlers/staticimages" "RenewCMS/api/handlers/user" @@ -35,7 +36,7 @@ func NewArticleRouter(container *UseCases) http.Handler { r := chi.NewRouter() articleHandler := article.NewHandler(container.CreateArticleUseCase, container.GetArticleUseCase, container.ListArticlesUseCase, container.DeleteArticleUseCase, container.UpdateArticleUseCase, container.GetUserUseCase) - imageHandler := image.NewHandler(container.CreateImageUseCase, container.GetArticleUseCase, container.UpdateArticleUseCase, container.DeleteImageUseCase) + imageHandler := image.NewHandler(container.CreateImageUseCase, container.GetArticleUseCase) r.Group(func(r chi.Router) { r.Get("/", articleHandler.ListArticles) @@ -50,8 +51,7 @@ func NewArticleRouter(container *UseCases) http.Handler { r.Patch("/{id}", articleHandler.UpdateArticleBody) r.Patch("/{id}/publish", articleHandler.PublishArticle) r.Patch("/{id}/unpublish", articleHandler.UnpublishArticle) - r.Post("/{id}/image", imageHandler.PostImage) - r.Delete("/{id}/image/{imageId}", imageHandler.DeleteImage) + r.Post("/{id}/image", imageHandler.PostLegacyArticleImage) }) return r } @@ -132,6 +132,24 @@ func NewSettingsRouter(container *UseCases) http.Handler { return r } +func NewMediaRouter(container *UseCases) http.Handler { + r := chi.NewRouter() + + mediaHandler := media.NewHandler(container.CreateImageUseCase, container.ListMediaUseCase, container.RenameMediaUseCase, container.DeleteMediaUseCase, container.MergeMediaUseCase, container.GetMediaUsageUseCase) + + r.Use(jwtauth.Verifier(container.Token)) + r.Use(jwtauth.Authenticator(container.Token)) + r.Use(middleware.RequireCurrentToken(container.GetUserUseCase)) + r.Use(middleware.RequireVerifiedUser(container.GetUserUseCase)) + r.Get("/", mediaHandler.List) + r.Post("/", mediaHandler.Upload) + r.Post("/merge", mediaHandler.Merge) + r.Patch("/{id}", mediaHandler.Rename) + r.Delete("/{id}", mediaHandler.Delete) + r.Get("/{id}/usage", mediaHandler.Usage) + return r +} + func InitBackendRoutes(container *UseCases, settings config.Settings) *chi.Mux { r := chi.NewRouter() @@ -146,6 +164,7 @@ func InitBackendRoutes(container *UseCases, settings config.Settings) *chi.Mux { }) r.Mount("/user", NewUserRouter(container, settings.AuthRateLimit, settings.TrustedProxies)) r.Mount("/settings", NewSettingsRouter(container)) + r.Mount("/media", NewMediaRouter(container)) r.Mount("/auth", NewAuthRouter(container, settings.IsProduction(), settings.AuthRateLimit, settings.TrustedProxies)) return r diff --git a/docs/architecture.md b/docs/architecture.md index 777685f..8a69ef9 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -26,8 +26,15 @@ Nothing under `internal/` imports `api/`. | Package | Role | |---|---| | `api/handlers/user` | Authenticated user endpoints: profile update (`UpdateMe`) and the users list with invitations (`CreateInvite`, `ListUsers`). | +| `api/handlers/media` | The media library under `/v1/media` (list, upload, rename, delete, merge, usage). Images belong to the library, not to an article: deleting an article never deletes an image, and an image's "used in" is read from article bodies each time. | +| `api/handlers/image` | `POST /v1/article/{id}/image`, a temporary alias that creates a library image and attaches nothing. `PostLegacyArticleImage` goes away when the editor moves to `/v1/media`. | +| `api/pkg/upload` | Multipart `file` reading and the mapping of upload errors (415, 413, 400) shared by both upload routes. | | `api/pkg/publicuser` | `PublicUser`, the JSON shape of a user with no password, codes or token version. `publicuser.New` converts a domain `user.User`, so handlers never serialise the domain struct directly. | +### Image metadata backfill + +At boot, `BackfillImageMetadata` fills `filename`, `size`, `content_type`, `width`, `height` and `sha256` for image rows that lack them, reading the files in `UPLOAD_DIR`. It is idempotent, and a row whose file is missing or unreadable is left as it is without stopping the boot. The old `article_id` column and the `article_images` table are kept and never written. + ## Frontend ↔ backend ``` diff --git a/features/delete_article_cascades_images.feature b/features/delete_article_cascades_images.feature deleted file mode 100644 index 87898da..0000000 --- a/features/delete_article_cascades_images.feature +++ /dev/null @@ -1,84 +0,0 @@ -Feature: Deleting an article also deletes its images - - As an authenticated RenewCMS user managing the CMS - I want deleting an article to also delete every image attached to it - So that removing an article doesn't leave orphaned image rows and files behind, - and so that an id that doesn't exist still answers 404 the same way every other - article write does - - Background: - Given internal/domain/article.Article carries Images []*image.Image, populated by - ArticleRepository.Get via Preload("Images") - And internal/domain/image.Repository already exposes Delete(id uint32) error, which - on the real ImageRepository also removes the uploaded file from UPLOAD_DIR - And before this slice, DeleteArticleUseCase.DeleteArticle only called - articleRepository.Delete(id) and never touched image.Repository, so an article's - images were never deleted along with it - And this slice changes NewDeleteArticleUseCase to also take an image.Repository, and - changes DeleteArticleUseCase.DeleteArticle(id) to look up the article, delete every - image in its Images slice through image.Repository.Delete, and only then delete the - article itself through article.Repository.Delete - And this slice rewires api/dependecyInjection.go so - useCases.NewDeleteArticleUseCase(repos.ArticleRepo, repos.ImageRepo) supplies both - repositories - And DELETE /v1/article/{id} keeps sitting behind the jwtauth Verifier and - Authenticator group in api/router.go, unchanged from - features/write_not_found_errors.feature - And DELETE /v1/article/{id} keeps answering 204 with an empty body on success and 404 - with body apperr.ArticleNotFoundErrorMessage for an id absent from the article - repository, backed by errors.Is(err, article.ErrArticleNotFound), unchanged from - features/write_not_found_errors.feature - And no image HTTP endpoint is mounted in api/router.go as of this slice, so cascading - image deletion is only observable through the image repository's state, not through - a separate HTTP call - And the use case deletes images in the order they appear in the article's Images - slice, and aborts on the first image.Repository.Delete error, leaving the article - and any not-yet-deleted images untouched, and answering the same generic 500 the - DeleteArticle handler already falls back to on any non-not-found error - - Scenario: Deleting an article with images removes the article and every one of its images - Given a valid jwt cookie for a user with id 1 - And the article repository contains an article with id 1 whose Images slice holds - image id 10 and image id 11 - And the image repository contains images with id 10 and id 11 - When the client sends DELETE /v1/article/1 with that cookie - Then the response status is 204 - And the article repository no longer contains an article with id 1 - And the image repository no longer contains an image with id 10 or id 11 - - Scenario: Deleting an article with no images still succeeds - Given a valid jwt cookie for a user with id 1 - And the article repository contains an article with id 2 whose Images slice is empty - When the client sends DELETE /v1/article/2 with that cookie - Then the response status is 204 - And the article repository no longer contains an article with id 2 - And no call reaches the image repository's Delete method - - Scenario: Deleting an unknown article still answers 404 and deletes no images - Given a valid jwt cookie for a user with id 1 - And the article repository has no article with id 9999 - When the client sends DELETE /v1/article/9999 with that cookie - Then the response status is 404 - And the response body is the text "Article not found" - And no call reaches the image repository's Delete method - - Scenario: Deleting an article still requires authentication - Given no jwt cookie is sent - And the article repository contains an article with id 1 whose Images slice holds - image id 10 - When the client sends DELETE /v1/article/1 without a cookie - Then the response status is 401 - And the article repository still contains the article with id 1 - And the image repository still contains the image with id 10 - - Scenario: A failure deleting one of an article's images aborts the whole delete - Given a valid jwt cookie for a user with id 1 - And the article repository contains an article with id 3 whose Images slice holds - image id 20 followed by image id 21 - And the image repository contains images with id 20 and id 21 - And the image repository's Delete fails for image id 20 - When the client sends DELETE /v1/article/3 with that cookie - Then the response status is 500 - And the response body is the text "Failed to delete article" - And the article repository still contains the article with id 3 - And the image repository still contains the image with id 21 diff --git a/features/delete_article_cascades_images.qa.md b/features/delete_article_cascades_images.qa.md deleted file mode 100644 index ac26dd1..0000000 --- a/features/delete_article_cascades_images.qa.md +++ /dev/null @@ -1,67 +0,0 @@ -# QA procedure: delete_article_cascades_images - -Package: `test` (black-box, extending `article_test.go`). - -## Setup shared by every case -1. Call `validator.InitValidator()` once. -2. Reuse `mockArticleRepo` (`test/article_test.go`) and `mockImageRepo` - (`test/image_test.go`) as-is. Extend `mockArticleRepo` seeding so a seeded article's - `Images []*image.Image` can hold pointers into the same `mockImageRepo.images` map used - by the test, so deleting through the article side and asserting through the image side - observe the same state. -3. Add a call counter (or reuse the existing `failSwitch`/call-tracking pattern already in - `test/`) on `mockImageRepo.Delete` so a scenario can assert it was never called, and reuse - `mockImageRepo.fail` (the existing `failSwitch`) to make `Delete` fail for a specific - image id. -4. Wire the real use cases over the fakes: `useCases.NewCreateArticleUseCase`, - `useCases.NewGetArticleUseCase`, `useCases.NewListArticlesUseCase`, - `useCases.NewUpdateArticleUseCase`, and the new two-argument - `useCases.NewDeleteArticleUseCase(articleRepo, imageRepo)`. Build `article.NewHandler(...)` - with the same constructor shape it already has (the use case's internal dependency count - changes, not the handler's). -5. Build a test `jwtauth.New("HS256", []byte("test-secret"), nil)`. -6. Mount the routes on a fresh `chi.NewRouter()` mirroring `api/router.go`'s current shape: - `GET /` under `jwtauth.Verifier` only; `GET /{id}`, `POST /`, `DELETE /{id}`, - `PATCH /{id}`, `PATCH /{id}/publish`, `PATCH /{id}/unpublish` under `jwtauth.Verifier` + - `jwtauth.Authenticator`. -7. Mint a valid jwt cookie for user id 1: `token.Encode(map[string]any{"user_id": uint32(1)})`, - set as the `jwt` cookie on the relevant `httptest.NewRequest`. - -## HTTP checks -8. Seed `imageRepo.images[10]` and `imageRepo.images[11]`, then seed - `articleRepo.articles[1] = article.Article{ID: 1, Title: "Backyard Renovation Update", - Images: []*image.Image{imageRepo.images[10], imageRepo.images[11]}}` (pointers into the - same map). Send `DELETE /v1/article/1` with the valid cookie -> status `204`; assert - `articleRepo.articles` no longer has key `1`; assert `imageRepo.images` no longer has - keys `10` or `11`. -9. Seed `articleRepo.articles[2] = article.Article{ID: 2, Title: "Draft: Kitchen Remodel"}` - with a nil/empty `Images` slice. Send `DELETE /v1/article/2` with the valid cookie -> - status `204`; assert `articleRepo.articles` no longer has key `2`; assert the image-delete - call counter is `0`. -10. Empty `articleRepo`, empty `imageRepo`. Send `DELETE /v1/article/9999` with the valid - cookie -> status `404`, body `"Article not found\n"`; assert the image-delete call - counter is `0`. -11. Seed `imageRepo.images[10]` and `articleRepo.articles[1]` with `Images` holding image id - 10, per step 8's pattern. Send `DELETE /v1/article/1` with **no** cookie -> status `401`; - assert `articleRepo.articles[1]` is still present and `imageRepo.images[10]` is still - present. -12. Seed `imageRepo.images[20]` and `imageRepo.images[21]`, then seed - `articleRepo.articles[3] = article.Article{ID: 3, Title: "Kitchen Progress Photos", - Images: []*image.Image{imageRepo.images[20], imageRepo.images[21]}}` (image 20 before - image 21 in the slice). Arm `imageRepo.fail` so `Delete` returns an error only when - called with id `20` (leave id `21` unarmed). Send `DELETE /v1/article/3` with the valid - cookie -> status `500`, body `"Failed to delete article\n"`; assert - `articleRepo.articles[3]` is still present; assert `imageRepo.images[21]` is still - present (never reached because the delete aborted on image `20` first). - -## Manual verification (not exercised by the Go test) -- Real cascading delete against a real SQLite file and real `UPLOAD_DIR`: confirm that after - `DELETE /v1/article/{id}` on a real running instance, the uploaded image files under - `UPLOAD_DIR` for that article are gone from disk and the `images` table no longer holds - those rows, and that no orphaned rows are left behind in the `article_images` join table. - Not exercised by `task test` (fakes only); requires `task dev` or a built `task build` - binary with a scratch DB/upload dir. -- SPA-visible behaviour: confirm the back-office article deletion flow (if/when one exists) - reflects that any images shown for that article are gone too. No such UI exists yet as of - this slice. -- No SMTP involvement. diff --git a/features/delete_article_keeps_images.feature b/features/delete_article_keeps_images.feature new file mode 100644 index 0000000..b3a5bc7 --- /dev/null +++ b/features/delete_article_keeps_images.feature @@ -0,0 +1,39 @@ +Feature: Deleting an article never deletes images + + As an authenticated RenewCMS user + I want deleting an article to leave the media library alone + So that an image used by other articles, or waiting in the library, is never lost + + Background: + Given images belong to the library and internal/domain/article.Article carries no Images + And DeleteArticleUseCase depends only on article.Repository and calls its Delete + And api/dependecyInjection.go builds useCases.NewDeleteArticleUseCase(repos.ArticleRepo) + And DELETE /v1/article/{id} stays behind the same jwtauth group, answering 204 with an + empty body, 404 with apperr.ArticleNotFoundErrorMessage for errors.Is(err, + article.ErrArticleNotFound), and the generic 500 "Failed to delete article" otherwise + + Scenario: Deleting an article leaves every image it used + Given a valid jwt cookie for user 1 + And article 1 whose body contains the urls of images 10 and 11 + And the image repository contains images 10 and 11 + When the client sends DELETE /v1/article/1 + Then the response status is 204 + And the article repository no longer contains article 1 + And the image repository still contains images 10 and 11 and Delete was never called + + Scenario: Deleting an unknown article is 404 and touches no image + Given a valid jwt cookie for user 1 and no article 9999 + When the client sends DELETE /v1/article/9999 + Then the response status is 404 and the body is "Article not found" + And the image repository is untouched + + Scenario: Deleting still requires authentication + Given no jwt cookie + When the client sends DELETE /v1/article/1 + Then the response status is 401 and article 1 and image 10 still exist + + Scenario: An article repository failure is 500 and everything stays + Given the article repository's Delete fails + When the client sends DELETE /v1/article/1 with a valid cookie + Then the response status is 500 and the body is "Failed to delete article" + And the image repository is untouched diff --git a/features/delete_article_keeps_images.qa.md b/features/delete_article_keeps_images.qa.md new file mode 100644 index 0000000..e9858cc --- /dev/null +++ b/features/delete_article_keeps_images.qa.md @@ -0,0 +1,12 @@ +# QA procedure: delete_article_keeps_images + +Package: `test` (black-box). Wire `useCases.NewDeleteArticleUseCase(articleRepo)` (single argument) with the other article use cases and mount as in the retired `delete_article_cascades_images.qa.md`: `Verifier` plus `Authenticator`, `RequireCurrentToken`, `RequireVerifiedUser`, jwt cookie for user id 1. Seed images 10 and 11 in `mockImageRepo` and the article body with their urls. Call `validator.InitValidator()` first. + +## HTTP checks +1. `DELETE /v1/article/1` with a valid cookie -> 204, `articleRepo` no longer holds article 1, `imageRepo.images` still holds 10 and 11 and `deleteCalls` is empty. +2. `DELETE /v1/article/9999` -> 404, body `"Article not found\n"`, image repo untouched. +3. `DELETE /v1/article/1` without cookie -> 401, article and images still present. +4. Article fake's `Delete` armed to fail -> 500, body `"Failed to delete article\n"`, image repo untouched. + +## Manual verification +With a real SQLite file and `UPLOAD_DIR`, delete an article and confirm the image files and rows remain and still load. No SMTP. diff --git a/features/image_upload_rules.feature b/features/image_upload_rules.feature index 4c2ea30..133b013 100644 --- a/features/image_upload_rules.feature +++ b/features/image_upload_rules.feature @@ -17,8 +17,8 @@ Feature: Allowed image types and the size limit are a domain rule 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 + And the HTTP mapping of these sentinels is 413, 415 and 400, specified in + features/media_library.feature Scenario Outline: CreateImage accepts every allowed content type within the size limit Given an empty image repository fake diff --git a/features/media_library.feature b/features/media_library.feature new file mode 100644 index 0000000..b6d6e57 --- /dev/null +++ b/features/media_library.feature @@ -0,0 +1,391 @@ +Feature: The media library holds every uploaded image as a library-level asset + + As a verified RenewCMS user + I want to upload, browse, rename, delete, merge and inspect images that belong to the + library and not to any article + So that one image can be used in many articles, deleting an article never deletes an image, + and duplicates can be merged without leaving a broken image behind + + Background: + Given internal/domain/image.Image carries ID, Path, Filename, Size, ContentType, Width, + Height (0 for SVG), SHA256, UploadedByID, CreatedAt and UpdatedAt, and no article id + And image.ImageInput gains Width, Height, SHA256 and UploadedByID + And internal/domain/image defines the sentinels ErrImageUnreadable, ErrInvalidFilename, + ErrMergeCopyMissing and ErrMergeHashMismatch, plus the existing ErrImageNotFound, + ErrImageTypeNotAllowed and ErrImageTooLarge, all asserted with errors.Is + And image.Repository exposes Create, Get, Delete, List(query), FindByHash, Rename, + ListByHash groups and Counts, and article.Repository exposes + FindByBodyContaining(filename) and the existing UpdateBody, and no longer AddImage + And CreateImageUseCase reads the bytes fully, rejects a type outside image/png, image/jpeg, + image/webp and image/svg+xml with ErrImageTypeNotAllowed, rejects more than 10485760 + bytes actually read with ErrImageTooLarge, computes SHA-256 hex and, for raster images, + width and height from the bytes, and rejects bytes that cannot be decoded (or an SVG + without an svg root) with ErrImageUnreadable, all before image.Repository.Create is called + And an image's "used in" is the count of articles, online or draft, whose body contains + the image's url, read from article bodies each time + And /v1/media is mounted in api/router.go behind jwtauth.Verifier, jwtauth.Authenticator, + middleware.RequireCurrentToken and middleware.RequireVerifiedUser + And every media item is JSON with snake_case fields: + {"id","filename","url","size","content_type","width","height","sha256","uploaded_by", + "created_at","used_in","duplicate"}, where url is the stored path + "/static/uploadedImages/", width and height are null for SVG, uploaded_by is + the uploader's display name (username when empty, "" when unknown) and duplicate is true + when another image has the same sha256 + And request bodies are validated with validate tags and validator.Validate.Struct, and + errors are plain text from http.Error + + Scenario Outline: Every media route refuses callers that are not logged in and verified + When the client sends + Then the response status is + And no image or article is read or changed + + Examples: + | method | path | credentials | status | + | GET | /v1/media/ | no cookie | 401 | + | POST | /v1/media/ | no cookie | 401 | + | PATCH | /v1/media/1 | no cookie | 401 | + | DELETE | /v1/media/1 | no cookie | 401 | + | POST | /v1/media/merge | no cookie | 401 | + | GET | /v1/media/1/usage | no cookie | 401 | + | GET | /v1/media/ | a stale token_version cookie | 401 | + | GET | /v1/media/ | cookie of an unverified user | 403 | + | DELETE | /v1/media/1 | cookie of an unverified user | 403 | + + Rule: Listing + + Background: + Given user 1 has display name "Inès Duarte" and user 2 has no display name and username "marta" + And the library holds, uploaded in this order (oldest first): + | id | filename | size | content_type | width | height | sha256 | uploaded by | + | 1 | north-wall.jpg | 4089446 | image/jpeg | 3600 | 2400 | A | 1 | + | 2 | floorboards.jpg | 2516582 | image/jpeg | 3600 | 2400 | B | 1 | + | 3 | floorboards-copy.jpg | 2516582 | image/jpeg | 3600 | 2400 | B | 2 | + | 4 | window-frame.jpg | 812345 | image/jpeg | 1200 | 800 | C | 2 | + | 5 | logo.svg | 1204 | image/svg+xml | 0 | 0 | D | 1 | + And article 1 "Kitchen Renovation" is online and its body contains the urls of images 1 and 2 + And article 2 "Backyard Notes" is a draft and its body contains the url of image 1 + And article 3 "Empty" has a body with no image url + + Scenario: The default list is newest first with used_in, duplicate and tab counts + When the client sends GET /v1/media/ with a valid cookie + Then the response status is 200 + And the "items" ids are 5, 4, 3, 2, 1 in that order + And item 1 has used_in 2, duplicate false and uploaded_by "Inès Duarte" + And items 2 and 3 have duplicate true, and item 3 has uploaded_by "marta" + And item 5 has width null, height null and used_in 0 + And "next_cursor" is null + And "counts" is {"all": 5, "unused": 3, "duplicates": 1} + + Scenario: An empty library answers an empty list, not null + Given the library holds no image + When the client sends GET /v1/media/ with a valid cookie + Then the response status is 200 + And "items" is an empty array and "counts" is {"all": 0, "unused": 0, "duplicates": 0} + + Scenario Outline: sort orders the list + When the client sends GET /v1/media/?sort= with a valid cookie + Then the "items" ids are in that order + + Examples: + | sort | ids | + | newest | 5, 4, 3, 2, 1 | + | oldest | 1, 2, 3, 4, 5 | + | name | 2, 3, 5, 1, 4 | + | largest | 1, 2, 3, 4, 5 | + + Scenario Outline: filter narrows the list and the counts stay library-wide + When the client sends GET /v1/media/?filter= with a valid cookie + Then the "items" ids, as a set, are + And "counts" is still {"all": 5, "unused": 3, "duplicates": 1} + + Examples: + | filter | ids | + | all | 1,2,3,4,5 | + | unused | 3, 4, 5 | + | duplicates | 2, 3 | + + Scenario Outline: q matches the filename case-insensitively + When the client sends GET /v1/media/?q= with a valid cookie + Then the "items" ids are in that order + And "counts" is still {"all": 5, "unused": 3, "duplicates": 1} + + Examples: + | q | ids | + | FLOORBOARD | 3, 2 | + | wall | 1 | + | zzz | | + + Scenario: q, filter and sort combine + When the client sends GET /v1/media/?q=floorboards&filter=unused&sort=oldest with a valid cookie + Then the "items" ids are 3 + + Scenario: hash returns only the images with that content + When the client sends GET /v1/media/?hash= with a valid cookie + Then the "items" ids, as a set, are 2, 3 + + Scenario: hash of unknown content returns no item + When the client sends GET /v1/media/?hash=<64 hex zeros> with a valid cookie + Then the response status is 200 and "items" is empty + + Scenario: limit and cursor page through the list + When the client sends GET /v1/media/?limit=2 with a valid cookie + Then the "items" ids are 5, 4 and "next_cursor" is a non-empty string + When the client sends GET /v1/media/?limit=2&cursor= + Then the "items" ids are 3, 2 and "next_cursor" is a non-empty string + When the client sends GET /v1/media/?limit=2&cursor= + Then the "items" ids are 1 and "next_cursor" is null + + Scenario Outline: A malformed query answers 400 + When the client sends GET /v1/media/? with a valid cookie + Then the response status is 400 + And the response body is the text apperr.MediaQueryErrorMessage + + Examples: + | query | + | filter=bogus | + | sort=bogus | + | limit=0 | + | limit=101 | + | limit=abc | + | cursor=%%% | + | hash=notahash | + + Rule: Uploading + + Scenario Outline: An allowed image is always stored, with its metadata and hash + Given a valid cookie for user 1 whose display name is "Inès Duarte" + When the client sends POST /v1/media/ with multipart field "file" named , content type and real bytes + Then the response status is 201 + And the body is a media item whose filename is , content_type is , size is the byte count, width and height are and , sha256 is the SHA-256 hex of the bytes, uploaded_by is "Inès Duarte", used_in is 0 and duplicate is false + And the item's url matches "/static/uploadedImages/" + And the image repository holds one new image with UploadedByID 1 + + Examples: + | name | content_type | width | height | + | north-wall.png | image/png | 3 | 2 | + | IMG_2208.jpg | image/jpeg | 3 | 2 | + | hero.webp | image/webp | 1 | 1 | + | logo.svg | image/svg+xml | null | null | + + Scenario: Uploading the same bytes twice stores both and marks both as duplicates + Given the library holds image 1 with the same bytes + When the client sends POST /v1/media/ with those bytes named "north-wall-again.png" + Then the response status is 201 and the item has duplicate true + And the library now holds two images with the same sha256 and different urls + And "counts".duplicates is 1 when listed + + Scenario: A filename without extension gets the canonical one, a path is stripped + When the client uploads real PNG bytes named "C:\\photos\\north wall" with content type "image/png" + Then the item filename is "north wall.png" + + Scenario: A file exactly at 10485760 bytes is accepted, one byte more is 413 + When the client uploads a valid PNG padded to 10485760 bytes + Then the response status is 201 + When the client uploads a valid PNG padded to 10485761 bytes + Then the response status is 413 + And the response body is the text "the file exceeds the maximum upload size" + And the image repository's Create was called once in total + + Scenario: A disallowed content type is 415 + When the client uploads "notes.pdf" with content type "application/pdf" + Then the response status is 415 + And the response body is the text "the file must be a PNG, JPEG, WEBP, or SVG image" + And the image repository's Create is never called + + Scenario Outline: Bytes that cannot be read as the declared type are 400 + When the client uploads with content type and the bytes "not an image" + Then the response status is 400 + And the response body is the text "the image could not be read" + And the image repository's Create is never called + + Examples: + | name | content_type | + | broken.png | image/png | + | broken.jpg | image/jpeg | + | broken.webp | image/webp | + | broken.svg | image/svg+xml | + + Scenario: A request without the "file" part is 400 + When the client sends POST /v1/media/ with a multipart body that has no "file" field + Then the response status is 400 + And the image repository's Create is never called + + Scenario: A repository failure is a generic 500 + Given the image repository's next Create fails + When the client uploads a valid PNG + Then the response status is 500 and the body is "Internal server error" + + Rule: Renaming + + Background: + Given image 1 is "north-wall.jpg" with url "/static/uploadedImages/aaa.jpeg" and article 1 body contains that url + + Scenario: Rename changes the display name, keeps the extension and the url + When the client sends PATCH /v1/media/1 with {"filename": "north-wall-01"} + Then the response status is 200 + And the body is the media item with filename "north-wall-01.jpg" and url "/static/uploadedImages/aaa.jpeg" + And article 1's body is unchanged + And a following GET /v1/media/?q=north-wall-01 returns image 1 + + Scenario: Surrounding whitespace is trimmed and unicode letters are allowed + When the client sends PATCH /v1/media/1 with {"filename": " Été_2026-a "} + Then the response status is 200 and the filename is "Été_2026-a.jpg" + + Scenario Outline: An invalid new name is 400 and nothing changes + When the client sends PATCH /v1/media/1 with + Then the response status is 400 + And the response body is the text "Enter a filename." for every row except "not json" + And the image's filename is still "north-wall.jpg" + + Examples: + | body | + | {"filename": ""} | + | {"filename": " "} | + | {"filename": "***"} | + | {"filename": "a.b"} | + | {"filename": "a b"} | + | {"filename": "<101 letters>"} | + | {} | + | not json | + + Scenario: Renaming an unknown image is 404 + When the client sends PATCH /v1/media/9999 with {"filename": "x"} + Then the response status is 404 and the body is "Image not found" + + Scenario: A malformed id is 400 + When the client sends PATCH /v1/media/abc with {"filename": "x"} + Then the response status is 400 and the body is apperr.IdUint32ErrorMessage + + Rule: Deleting + + Scenario: An image in use can be deleted and article bodies are left alone + Given image 1 is used in article 1's body + When the client sends DELETE /v1/media/1 with a valid cookie + Then the response status is 204 and the body is empty + And image 1 is gone from the repository and its file from UPLOAD_DIR + And article 1's body is byte-for-byte unchanged + + Scenario: A file already missing from disk still deletes the row + Given image 4's file is not in UPLOAD_DIR + When the client sends DELETE /v1/media/4 + Then the response status is 204 and image 4 is gone from the repository + + Scenario: Deleting an unknown image is 404 + When the client sends DELETE /v1/media/9999 + Then the response status is 404 and the body is "Image not found" + + Scenario: A malformed id is 400 + When the client sends DELETE /v1/media/abc + Then the response status is 400 and the body is apperr.IdUint32ErrorMessage + + Scenario: A repository failure is a generic 500 and the image stays + Given the image repository's Delete fails + When the client sends DELETE /v1/media/1 + Then the response status is 500 and image 1 is still in the repository + + Rule: Usage + + Scenario: Usage lists the articles whose body contains the url + Given article 1 "Kitchen Renovation" is online and article 2 "Backyard Notes" is a draft, both containing image 1's url, and article 3 does not + When the client sends GET /v1/media/1/usage with a valid cookie + Then the response status is 200 + And the body is [{"id": 1, "title": "Kitchen Renovation", "published": true}, {"id": 2, "title": "Backyard Notes", "published": false}] + + Scenario: An unused image has an empty array, not null + When the client sends GET /v1/media/4/usage + Then the response status is 200 and the body is [] + + Scenario: An absolute url still counts as used + Given article 3's body contains "https://cms.example.com/static/uploadedImages/aaa.jpeg" for image 1 + Then GET /v1/media/1/usage includes article 3 + + Scenario Outline: Unknown or malformed ids + When the client sends GET /v1/media//usage + Then the response status is and the body is + + Examples: + | id | status | body | + | 9999 | 404 | Image not found | + | abc | 400 | apperr.IdUint32ErrorMessage | + + Rule: Merging + + Background: + Given images 2, 3 and 6 share sha256 B, image 1 has sha256 A + And article 1 body contains the urls of images 2 and 3, article 2 body contains image 3's url twice, article 3 body contains image 2's url only, and article 4 has none of them + + Scenario: Merge switches bodies to the kept url and deletes the copies + When the client sends POST /v1/media/merge with {"keep_id": 2, "merge_ids": [3, 6]} + Then the response status is 200 + And the body is {"articles_updated": 2, "merged": 2} + And article 1's body has image 3's url replaced by image 2's url + And article 2's body has both occurrences of image 3's url replaced by image 2's url + And article 3 and article 4 are unchanged + And images 3 and 6 are gone from the repository and from UPLOAD_DIR, and image 2 remains + + Scenario: Merging copies that no article uses updates zero articles + When the client sends POST /v1/media/merge with {"keep_id": 2, "merge_ids": [6]} + Then the response status is 200 and the body is {"articles_updated": 0, "merged": 1} + + Scenario: A copy that is gone answers 409 and changes nothing + When the client sends POST /v1/media/merge with {"keep_id": 2, "merge_ids": [3, 9999]} + Then the response status is 409 and the body is apperr.MediaMergeConflictErrorMessage + And no article body changed and images 2 and 3 still exist + + Scenario: A copy that does not share the hash answers 409 and changes nothing + When the client sends POST /v1/media/merge with {"keep_id": 2, "merge_ids": [3, 1]} + Then the response status is 409 + And no article body changed and images 1, 2 and 3 still exist + + Scenario: A kept image that is gone answers 404 + When the client sends POST /v1/media/merge with {"keep_id": 9999, "merge_ids": [3]} + Then the response status is 404 and the body is "Image not found" + And nothing changed + + Scenario Outline: An invalid merge request is 400 and nothing changes + When the client sends POST /v1/media/merge with + Then the response status is 400 and the body is apperr.MediaMergeRequestErrorMessage + + Examples: + | body | + | {"keep_id": 2, "merge_ids": []} | + | {"keep_id": 2} | + | {"merge_ids": [3]} | + | {"keep_id": 2, "merge_ids": [2, 3]} | + | {"keep_id": 2, "merge_ids": [3, 3]} | + | {"keep_id": 0, "merge_ids": [3]} | + | not json | + + Scenario: A failure rewriting a body restores the bodies already rewritten + Given the article repository's UpdateBody fails for article 2 + When the client sends POST /v1/media/merge with {"keep_id": 2, "merge_ids": [3]} + Then the response status is 500 + And article 1's body is back to its original text, article 2 is unchanged + And images 2 and 3 still exist + + Scenario: A failure deleting a copy after the rewrite is 500 and the bodies stay switched + Given the image repository's Delete fails for image 3 + When the client sends POST /v1/media/merge with {"keep_id": 2, "merge_ids": [3]} + Then the response status is 500 + And article 1's and article 2's bodies use image 2's url and image 3 still exists + + @manual + Rule: Startup backfill and migration (persistence, verified by hand, see the qa.md) + + Scenario: Existing rows get metadata and hash from the files on disk + Given the images table has rows created before this slice, with a path and no filename, size, content_type, width, height or sha256, and their files are in UPLOAD_DIR + When the server boots + Then each row gets filename equal to the stored file name, size, content_type, width and height (0 for SVG) and sha256 read from its file, and UploadedByID stays 0 + And the old article_id column and the article_images table are kept and not written + + Scenario: A row whose file is missing does not stop the boot + Given a row whose file is not in UPLOAD_DIR + When the server boots + Then the boot completes, the row is left without metadata, and the server serves requests + + Scenario: The backfill is idempotent + When the server boots a second time + Then no row is changed again + + Scenario: A row without metadata still lists + Then GET /v1/media/ returns it with size 0, width null, height null, sha256 "" and duplicate false diff --git a/features/media_library.qa.md b/features/media_library.qa.md new file mode 100644 index 0000000..c77b12d --- /dev/null +++ b/features/media_library.qa.md @@ -0,0 +1,39 @@ +# QA procedure: media_library + +Package: `test` (black-box). New file `test/media_library_http_test.go`. Fake type names must be unique in the package. + +## Setup shared by every case +1. Call `validator.InitValidator()` once. +2. Rewrite `mockImageRepo` (`test/image_test.go`) to the new `image.Repository`: Create (stores the metadata from the ImageInput and derives a `/static/uploadedImages/.` path), Get, Delete (records calls, honours `failDeleteFor`), List(query), FindByHash, Rename, ListByHash groups, Counts. Add fields for seeding `Filename`, `Size`, `ContentType`, `Width`, `Height`, `SHA256`, `UploadedByID`, `CreatedAt`. +3. Extend `mockArticleRepo` (`test/article_test.go`) with `FindByBodyContaining(filename)` (substring over `Body`) and a per-id failure switch for `UpdateBody`. Remove `AddImage` and the article `Images` field from the fake and from every test that uses them. +4. Use the existing user fake so `uploaded_by` resolves: user 1 display name "Inès Duarte", user 2 username "marta" with empty display name. +5. Wire the real use cases over the fakes: create, list, rename, delete, merge and usage media use cases, `NewCreateImageUseCase(imageRepo)`, `NewGetUserUseCase(userRepo)`. Build `media.NewHandler(...)` (`api/handlers/media`) and `image.NewHandler(...)` for the alias. +6. Build `jwtauth.New("HS256", []byte("test-secret"), nil)`. Mount on a fresh `chi.NewRouter()` mirroring `api/router.go`: `jwtauth.Verifier` at the top, then in the media group `Authenticator`, `RequireCurrentToken(getUser)`, `RequireVerifiedUser(getUser)`, with the six routes under `/v1/media`. +7. Mint cookies via `token.Encode(map[string]any{"user_id": uint32(n), ...})` as in the other HTTP tests. Build a stale-token cookie with a `token_version` lower than the user's, and use an unverified user for the 403 case. +8. Build multipart bodies with `mime/multipart.Writer`, using `CreatePart` with an explicit part `Content-Type` header. Generate real bytes in the test: + - PNG (3x2) with `image/png`; + - JPEG (3x2) with `image/jpeg`; + - a tiny valid WEBP fixture (1x1) as a byte constant; + - SVG ``. + + Compute expected sha256 in the test with `crypto/sha256`. For size limits, pad valid PNG bytes to 10485760 and 10485761 bytes. +9. Seed the Listing background exactly as in the feature (5 images, 3 articles, 2 users), with `created_at` strictly increasing by id. + +## HTTP checks +10. Auth table: each method+path from the outline with no cookie -> 401, stale cookie -> 401 with `apperr.UnauthorizedErrorMessage`, unverified cookie -> 403 with `apperr.VerificationRequiredErrorMessage`. Assert no fake method beyond the middleware's user lookup was called. +11. List: decode `{items, next_cursor, counts}`. Assert each row of the sort, filter, `q` and `hash` tables, the combined query, the paging walk (limit 2 across three pages ending in `next_cursor == null`), the empty library (`items` is `[]` in the raw JSON, not `null`), SVG `width`/`height` are JSON `null` (assert on the raw body), and `counts` is unchanged by `q`/`filter`. Assert every 400 query row, body equals `apperr.MediaQueryErrorMessage + "\n"`. +12. Upload: for each allowed type assert 201, the item fields, `sha256` equals the test's own hash, `uploaded_by`, the url regex, and one repo entry with `UploadedByID == 1`. Upload identical bytes twice: both 201, second has `duplicate: true`, list `counts.duplicates == 1`. Assert filename rules (no extension gets `.png`, path stripped). Assert 413/415/400 statuses and bodies, that `Create` is never called on rejection, boundary 10485760 vs 10485761, a missing `file` part -> 400, and an armed `Create` failure -> 500 `"Internal server error\n"`. +13. Rename: assert 200 with the new filename, the same `url`, the article body unchanged, and every invalid row -> 400 with `"Enter a filename.\n"` for the filename rows; 404 for 9999; 400 with `apperr.IdUint32ErrorMessage` for `abc`. Assert the repository filename is unchanged after each 400. +14. Delete: in-use image -> 204 with an empty body, article bodies byte-identical, `deleteCalls == [1]`; 404, 400, 500 rows. The "file already gone" case is a persistence behaviour and is a manual item. +15. Usage: assert the exact JSON arrays (raw body compared as JSON), the empty `[]`, the absolute-url article, 404 and 400. +16. Merge: assert 200 `{articles_updated, merged}`, resulting bodies (both occurrences replaced), unchanged bodies for the other articles, and only image 2 left. Assert the 409 rows and 404 row, with no body change and no `Delete` calls (`deleteCalls` empty). Assert every 400 row with `apperr.MediaMergeRequestErrorMessage`. Assert the two failure rows: `UpdateBody` failing for article 2 leaves article 1 restored and no `Delete` call; `Delete` failing for image 3 leaves bodies switched and returns 500. +17. Unit-level asserts (same file, no HTTP) with `errors.Is` on the sentinels `ErrImageUnreadable`, `ErrMergeCopyMissing`, `ErrMergeHashMismatch`, `ErrInvalidFilename`. +18. Layering guard: `go list -deps` or a grep-style test that `api/handlers/media` imports no `persistence`, `mailer`, `models` or `gorm` package. + +## Manual verification (not exercised by `task test`) +- Real upload, rename, delete, merge against a scratch SQLite file and `UPLOAD_DIR` via `task dev`: the files land on disk, `url` loads through `/static/uploadedImages/`, deleting an in-use image leaves the article body text untouched, and a merge removes the copy files. +- Delete when the file was already removed from `UPLOAD_DIR` still answers 204. +- Startup backfill: copy an old database (rows with only `path`), boot, and check that filename, size, content_type, width, height and sha256 are filled; check idempotency on a second boot; check that a row whose file is missing does not stop the boot; check that the `article_images` table and `article_id` column are still present and not written. +- `UploadedByID` 0 rows display an empty `uploaded_by`. +- WEBP dimensions with a real WEBP file (the fixture is minimal). +- No SMTP involvement. No real rate limit (out of scope for this slice). diff --git a/features/remove_article_image.feature b/features/remove_article_image.feature deleted file mode 100644 index 510268e..0000000 --- a/features/remove_article_image.feature +++ /dev/null @@ -1,99 +0,0 @@ -Feature: Removing an image from an article - - As an authenticated RenewCMS user managing the CMS - I want to delete an already-uploaded image by its id - So that a removed image's file and row are actually gone, and a delete on - an unknown article or an unknown image answers 404 the same way every - other article write does - - Background: - Given internal/infrastructure/useCases.DeleteImageUseCase.DeleteImage(imageId) - already exists and calls image.Repository.Delete(imageId), which returns - image.ErrImageNotFound for an unknown id and, on the real ImageRepository, - also removes the uploaded file from UPLOAD_DIR, per - features/delete_article_cascades_images.feature - And this slice changes POST /v1/article/{id}/image's success response - (api/handlers/image.Handler.PostImage, features/upload_article_image.feature) - to also return the created image's numeric id, so the 201 body becomes - {"id": ..., "location": ...}; this lets a client address the image later - through this slice's DELETE route - And this slice mounts DELETE /v1/article/{articleId}/image/{imageId} in - api/router.go, in the same jwtauth Verifier + Authenticator group as - POST .../image - And this slice adds a DeleteImage method to api/handlers/image.Handler, - grows NewHandler's signature to also take DeleteImageUseCase, and wires - it in api/router.go - And DeleteImage parses articleId and imageId from the URL, answering 400 - with apperr.IdUint32ErrorMessage if either isn't a valid uint32 - And DeleteImage first calls GetArticleUseCase.GetArticle(articleId); - errors.Is(err, article.ErrArticleNotFound) answers 404 with - apperr.ArticleNotFoundErrorMessage; this only confirms the article id - in the URL names a real article, and does not verify that the image id - belongs to that article's Images slice (a shallow existence check on - both ids, not an ownership check) - And DeleteImage then calls DeleteImageUseCase.DeleteImage(imageId); - errors.Is(err, image.ErrImageNotFound) answers 404 with the new - apperr.ImageNotFoundErrorMessage = "Image not found"; any other error - falls back to the existing generic 500 - And on success DeleteImage answers 204 with an empty body, the same shape - as DELETE /v1/article/{id} - - Scenario: Removing an image from an article it belongs to succeeds - Given a valid jwt cookie for a user with id 1 - And the article repository contains an article with id 1 titled - "Backyard Renovation Update" whose Images slice holds image id 10 - And the image repository contains an image with id 10 - When the client sends DELETE /v1/article/1/image/10 with that cookie - Then the response status is 204 - And the response body is empty - And the image repository no longer contains an image with id 10 - - Scenario: Removing an image from an unknown article answers 404 and never touches the image repository - Given a valid jwt cookie for a user with id 1 - And the article repository has no article with id 9999 - And the image repository contains an image with id 10 - When the client sends DELETE /v1/article/9999/image/10 with that cookie - Then the response status is 404 - And the response body is the text "Article not found" - And the image repository's Delete is never called - And the image repository still contains the image with id 10 - - Scenario: Removing an unknown image from an existing article answers 404 - Given a valid jwt cookie for a user with id 1 - And the article repository contains an article with id 1 titled - "Backyard Renovation Update" - And the image repository has no image with id 9999 - When the client sends DELETE /v1/article/1/image/9999 with that cookie - Then the response status is 404 - And the response body is the text "Image not found" - - Scenario: Removing an image id that belongs to a different article still succeeds, because the check is shallow - Given a valid jwt cookie for a user with id 1 - And the article repository contains an article with id 1 titled - "Backyard Renovation Update" whose Images slice is empty - And the article repository contains a second article with id 2 whose - Images slice holds image id 10 - And the image repository contains an image with id 10 - When the client sends DELETE /v1/article/1/image/10 with that cookie - Then the response status is 204 - And the image repository no longer contains an image with id 10 - - Scenario: Removing an image still requires authentication - Given no jwt cookie is sent - And the article repository contains an article with id 1 titled - "Backyard Renovation Update" whose Images slice holds image id 10 - And the image repository contains an image with id 10 - When the client sends DELETE /v1/article/1/image/10 without a cookie - Then the response status is 401 - And the image repository still contains the image with id 10 - - Scenario Outline: A malformed id in the URL answers 400 before either repository is touched - Given a valid jwt cookie for a user with id 1 - When the client sends DELETE /v1/article//image/ with that cookie - Then the response status is 400 - And the response body is the text "The server expects the ID to be in the format of an unsigned 32-bit integer (uint32)." - - Examples: - | article_id | image_id | - | abc | 10 | - | 1 | abc | diff --git a/features/remove_article_image.qa.md b/features/remove_article_image.qa.md deleted file mode 100644 index 2933a58..0000000 --- a/features/remove_article_image.qa.md +++ /dev/null @@ -1,56 +0,0 @@ -# QA procedure: remove_article_image - -Package: `test` (black-box, extending `article_test.go` / `image_test.go`). - -## Setup shared by every case -1. Call `validator.InitValidator()` once. -2. Reuse `mockArticleRepo` and `mockImageRepo` as-is. -3. Wire `useCases.NewDeleteImageUseCase(imageRepo)` (already used by - `delete_article_cascades_images.qa.md`'s suite) alongside the existing - use cases. -4. Build `image.NewHandler(...)` with its grown constructor including the - delete use case, matching whatever parameter order the coder settles on. -5. Build a test `jwtauth.New("HS256", []byte("test-secret"), nil)`. -6. Mount `DELETE /{id}/image/{imageId}` alongside the existing article - routes under the authenticated group, mirroring `api/router.go` after - this slice. -7. Mint a valid jwt cookie for user id 1 as in `upload_article_image.qa.md`. - -## HTTP checks -8. Seed `articleRepo.articles[1]` with `Images: []*image.Image{{ID: 10}}` - and `imageRepo.images[10] = image.Image{ID: 10}`. Send - `DELETE /v1/article/1/image/10` with the valid cookie. Assert status - 204, empty body; assert `imageRepo.images` no longer has key 10. -9. Empty `articleRepo`; seed `imageRepo.images[10]`. Send - `DELETE /v1/article/9999/image/10`. Assert 404, body - `"Article not found\n"`; assert `imageRepo.images` still has key 10 - (Delete never called — track via a call-count field on the fake, the - same way the existing fakes already track `Create`). -10. Seed `articleRepo.articles[1]`; empty `imageRepo`. Send - `DELETE /v1/article/1/image/9999`. Assert 404, body - `"Image not found\n"`. -11. Seed `articleRepo.articles[1]` (empty Images) and `articleRepo.articles[2]` - with `Images: []*image.Image{{ID: 10}}`; seed `imageRepo.images[10]`. - Send `DELETE /v1/article/1/image/10`. Assert 204; assert - `imageRepo.images` no longer has key 10 — asserts the shallow-check - contract from the feature's fourth scenario. -12. Seed as in step 8. Send `DELETE /v1/article/1/image/10` with **no** - cookie. Assert 401; assert `imageRepo.images` still has key 10. -13. Examples-driven: send `DELETE /v1/article/abc/image/10` and - `DELETE /v1/article/1/image/abc` with the valid cookie. Assert 400, - body `"The server expects the ID to be in the format of an unsigned - 32-bit integer (uint32).\n"` for each. - -## Also touched by this slice: upload_article_image's 201 body -14. Extend `upload_article_image.qa.md`'s existing 201-body assertion to - also decode and check the `id` field equals the newly created image's - `ID` in `imageRepo.images`. - -## Manual verification (not exercised by the Go test) -- Real DELETE against a real SQLite file and real UPLOAD_DIR: confirm the - file is actually removed from disk and the `article_images` join row is - gone, the same way `upload_article_image.qa.md`'s manual item covers the - POST side. -- SPA-visible behaviour: covered separately by - `features/editor_image_removal.qa.md`. -- No SMTP involvement. diff --git a/features/upload_article_image.feature b/features/upload_article_image.feature index fb7a0af..195930d 100644 --- a/features/upload_article_image.feature +++ b/features/upload_article_image.feature @@ -1,99 +1,47 @@ -Feature: Uploading an image to an article +Feature: POST /v1/article/{id}/image is a thin alias that creates a library image - As an authenticated RenewCMS user managing the CMS - I want to attach an image file to an existing article and get back the - image's public location - So that an unknown article answers 404 the same way every other article - write does, without ever creating an orphaned image, and a disallowed - content type is rejected with 400 before it becomes part of the - article, without a maintainer needing to trust the client to send a - sane file + As the current editor, until the editor slice moves it to /v1/media + I want to keep uploading through the article route and get {id, location} back + So that the editor keeps working while images are library assets Background: - Given internal/infrastructure/useCases.CreateImageUseCase.CreateImage - already checks the uploaded content type against the allowed list - (image/png, image/jpeg, image/webp, image/svg+xml) and the size - against the 10485760-byte maximum before calling image.Repository.Create, - returning image.ErrImageTypeNotAllowed or image.ErrImageTooLarge and - calling the repository not at all when either check fails - And internal/infrastructure/persistence's ArticleRepository.AddImage - already returns article.ErrArticleNotFound when the article id doesn't - match any row, and image.ErrImageNotFound when the article exists but - the image id doesn't, both asserted with errors.Is - And before this slice, api/handlers/image.Handler.PostImage existed and - was wired to CreateImageUseCase and UpdateArticleUseCase.AddImage, but - no route called it, and every error it got back from either use case - fell through to a blanket 500 - And this slice mounts POST /v1/article/{id}/image in api/router.go, in - the same jwtauth Verifier + Authenticator group as the other article - write routes, and changes the upload flow so that the article's - existence is confirmed before the image is created, so that an - unknown article id never leaves an orphaned image behind - And this slice changes PostImage's error handling so that errors.Is(err, - image.ErrImageTypeNotAllowed) or errors.Is(err, image.ErrImageTooLarge) - answers 400 with the sentinel's own message, errors.Is(err, - article.ErrArticleNotFound) answers 404 with body - apperr.ArticleNotFoundErrorMessage, and any other error still falls - back to a generic 500 - And PostImage reads the uploaded file from the multipart form field - "file" - And on success PostImage answers 201 with a JSON body - {"id": ..., "location": ...} holding the created image's numeric id - and public path, e.g. "/static/uploadedImages/" - And this slice (features/remove_article_image.feature) adds the "id" - field to that body; the id lets a client later address the image - through DELETE /v1/article/{id}/image/{imageId} + Given the route stays in the article router's jwtauth group with RequireCurrentToken and + RequireVerifiedUser, and is marked for removal in the editor slice + And the handler calls GetArticleUseCase first, so an unknown article is 404 + apperr.ArticleNotFoundErrorMessage and nothing is created + And it then calls the same CreateImageUseCase as POST /v1/media/, with UploadedByID from the + jwt user_id claim, attaches nothing to the article, and answers 201 {"id", "location"} + where location is the image's url + And type, size and unreadable errors map as on /v1/media/: 415, 413, 400 + And DELETE /v1/article/{id}/image/{imageId} is not mounted - Scenario: Uploading an allowed image type to an existing article succeeds - Given a valid jwt cookie for a user with id 1 - And the article repository contains an article with id 1 titled - "Backyard Renovation Update" - When the client sends POST /v1/article/1/image with that cookie and a - multipart form field "file" holding a file named "before.png" with - content type "image/png" + Scenario: Uploading through the alias creates a library image + Given a valid jwt cookie for user 1 and article 1 "Backyard Renovation Update" + When the client sends POST /v1/article/1/image with a real PNG named "before.png" Then the response status is 201 - And the response body is JSON with a "location" field whose value is - "/static/uploadedImages/before.png" - And the response body's "id" field equals the new image's id in the - image repository - And the image repository contains a new image whose Path is - "/static/uploadedImages/before.png" - And the article repository's article with id 1 has that new image's id - in its Images slice + And the body has "id" equal to the new image's id and "location" equal to its url + And GET /v1/media/ lists the image with uploaded_by of user 1 + And article 1 is unchanged - Scenario: Uploading to an unknown article answers 404 and creates no image - Given a valid jwt cookie for a user with id 1 - And the article repository has no article with id 9999 - When the client sends POST /v1/article/9999/image with that cookie and a - multipart form field "file" holding a file named "before.png" with - content type "image/png" - Then the response status is 404 - And the response body is the text "Article not found" + Scenario: An unknown article is 404 and creates no image + When the client sends POST /v1/article/9999/image with a valid PNG + Then the response status is 404 and the body is "Article not found" And the image repository's Create is never called - And no image exists in the image repository - Scenario: Uploading a disallowed content type answers 400 before the article is touched - Given a valid jwt cookie for a user with id 1 - And the article repository contains an article with id 1 titled - "Backyard Renovation Update" - When the client sends POST /v1/article/1/image with that cookie and a - multipart form field "file" holding a file named "notes.pdf" with - content type "application/pdf" - Then the response status is 400 - And the response body is the text - "the file must be a PNG, JPEG, WEBP, or SVG image" - And the image repository's Create is never called - And the article repository's article with id 1 still has an empty - Images slice + Scenario Outline: Rejected files use the media status codes + When the client sends POST /v1/article/1/image with + Then the response status is and Create is never called - Scenario: Uploading an image still requires authentication - Given no jwt cookie is sent - And the article repository contains an article with id 1 titled - "Backyard Renovation Update" - When the client sends POST /v1/article/1/image without a cookie and a - multipart form field "file" holding a file named "before.png" with - content type "image/png" - Then the response status is 401 - And the image repository's Create is never called - And the article repository's article with id 1 still has an empty - Images slice + Examples: + | file | status | + | "notes.pdf" as application/pdf | 415 | + | a valid PNG padded to 10485761 bytes | 413 | + | "broken.png" as image/png with text bytes | 400 | + + Scenario: The alias still requires authentication + When the client sends POST /v1/article/1/image with no cookie + Then the response status is 401 and Create is never called + + Scenario: The removed delete route is gone + When the client sends DELETE /v1/article/1/image/10 with a valid cookie + Then the response status is 405 and image 10 still exists diff --git a/features/upload_article_image.qa.md b/features/upload_article_image.qa.md index 3270aee..e7fae43 100644 --- a/features/upload_article_image.qa.md +++ b/features/upload_article_image.qa.md @@ -4,70 +4,18 @@ Package: `test` (black-box, extending `article_test.go` / `image_test.go`). ## Setup shared by every case 1. Call `validator.InitValidator()` once. -2. Reuse `mockArticleRepo` (`test/article_test.go`) and `mockImageRepo` - (`test/image_test.go`) as-is; both already implement `AddImage` / - `Create` returning `article.ErrArticleNotFound` / `image.ErrImageNotFound` - for an unknown id, per `features/write_not_found_errors.feature`. -3. Wire the real use cases over the fakes: `useCases.NewCreateArticleUseCase`, - `useCases.NewGetArticleUseCase`, `useCases.NewListArticlesUseCase`, - `useCases.NewUpdateArticleUseCase(articleRepo)`, - `useCases.NewDeleteArticleUseCase(articleRepo, imageRepo)`, - `useCases.NewCreateImageUseCase(imageRepo)`. -4. Build `article.NewHandler(...)` with its existing constructor shape, and - `image.NewHandler(createImageUseCase, updateArticleUseCase)` - (`api/handlers/image`). -5. Build a test `jwtauth.New("HS256", []byte("test-secret"), nil)`. -6. Mount the routes on a fresh `chi.NewRouter()` mirroring `api/router.go`'s - shape after this slice: `GET /` under `jwtauth.Verifier` only; - `GET /{id}`, `POST /`, `DELETE /{id}`, `PATCH /{id}`, - `PATCH /{id}/publish`, `PATCH /{id}/unpublish` and the new - `POST /{id}/image` (routed to `imageHandler.PostImage`) under - `jwtauth.Verifier` + `jwtauth.Authenticator`. -7. Mint a valid jwt cookie for user id 1: - `token.Encode(map[string]any{"user_id": uint32(1)})`, set as the `jwt` - cookie on the relevant `httptest.NewRequest`. -8. Build multipart request bodies with `mime/multipart.Writer`, field name - `"file"`, setting the part's `Content-Type` header explicitly (Go's - multipart writer defaults to sniffing otherwise, so set it via - `CreatePart` with an explicit MIME header rather than - `CreateFormFile`), and set the request's `Content-Type` header to the - writer's `FormDataContentType()`. +2. Wire `image.NewHandler(createImageUseCase, getArticleUseCase)` (no update or delete use cases) and mount only `POST /{id}/image` in the article group, behind `Verifier`, `Authenticator`, `RequireCurrentToken`, `RequireVerifiedUser`. +3. Use real image bytes generated in the test, as in `media_library.qa.md` step 8, and multipart bodies with an explicit part `Content-Type`. ## HTTP checks -9. Seed `articleRepo.articles[1] = article.Article{ID: 1, Title: "Backyard - Renovation Update"}`. Send `POST /v1/article/1/image` with the valid - cookie and a multipart `"file"` part named `"before.png"`, - content type `"image/png"`, some non-empty byte content. Assert status - `201`; decode the JSON body and assert `location` equals - `"/static/uploadedImages/before.png"`; assert `imageRepo.images` gained - exactly one entry whose `Path` matches that location; assert - `articleRepo.articles[1].Images` now contains that image's id. -10. Empty `articleRepo`, empty `imageRepo`. Send `POST /v1/article/9999/image` - with the valid cookie and the same `"before.png"` / `"image/png"` part. - Assert status `404`, body `"Article not found\n"`; assert - `imageRepo.images` is still empty (the article's existence is confirmed - before any image is created, so nothing is left orphaned). -11. Seed `articleRepo.articles[1] = article.Article{ID: 1, Title: "Backyard - Renovation Update"}` with an empty `Images` slice. Send - `POST /v1/article/1/image` with the valid cookie and a multipart - `"file"` part named `"notes.pdf"`, content type `"application/pdf"`. - Assert status `400`, body - `"the file must be a PNG, JPEG, WEBP, or SVG image\n"`; assert - `imageRepo.images` is still empty; assert - `articleRepo.articles[1].Images` is still empty. -12. Seed the same article id 1 as step 11. Send `POST /v1/article/1/image` - with **no** cookie and the `"before.png"` / `"image/png"` part. Assert - status `401`; assert `imageRepo.images` is still empty; assert - `articleRepo.articles[1].Images` is still empty. +4. Valid PNG on article 1 -> 201 `{id, location}` matching the created image, and `articleRepo.articles[1]` has an unchanged body. +5. Unknown article 9999 -> 404 `"Article not found\n"`, `Create` never called. +6. Rejected files: `application/pdf` -> 415, PNG padded to 10485761 bytes -> 413, text bytes as `image/png` -> 400; `Create` never called. +7. No cookie -> 401, `Create` never called. +8. `DELETE /v1/article/1/image/10` on this router -> 405, image 10 still exists. -## Manual verification (not exercised by the Go test) -- Real upload against a real SQLite file and real `UPLOAD_DIR`: confirm that - after `POST /v1/article/{id}/image` on a real running instance, the file - actually lands on disk under `UPLOAD_DIR`, the `images` row's `path` - resolves to a URL the SPA can load, and the `article_images` join table - gets the new row. Not exercised by `task test` (fakes only); requires - `task dev` or a built `task build` binary with a scratch DB/upload dir. -- SPA-visible behaviour: confirm the back-office article editor (if/when an - image-upload UI exists) shows the uploaded image at that location. No such - UI exists yet as of this slice. -- No SMTP involvement. +## Cleanup +Delete `test/remove_article_image_http_test.go` and update `test/upload_article_image_http_test.go` to the cases above. + +## Manual verification +With a real instance, `POST /v1/article/{id}/image` writes a file to `UPLOAD_DIR` and a row visible in `GET /v1/media/`, and no article association is written. diff --git a/go.mod b/go.mod index b41594a..65ba77a 100644 --- a/go.mod +++ b/go.mod @@ -12,6 +12,7 @@ require ( github.com/google/uuid v1.6.0 github.com/joho/godotenv v1.5.1 golang.org/x/crypto v0.44.0 + golang.org/x/image v0.46.0 gopkg.in/gomail.v2 v2.0.0-20160411212932-81ebce5c23df gorm.io/gorm v1.31.1 ) @@ -41,8 +42,8 @@ require ( github.com/remyoudompheng/bigfft v0.0.0-20230129092748-24d4a6f8daec // indirect github.com/segmentio/asm v1.2.1 // indirect golang.org/x/exp v0.0.0-20251113190631-e25ba8c21ef6 // indirect - golang.org/x/sys v0.38.0 // indirect - golang.org/x/text v0.31.0 // indirect + golang.org/x/sys v0.48.0 // indirect + golang.org/x/text v0.42.0 // indirect gopkg.in/alexcesaro/quotedprintable.v3 v3.0.0-20150716171945-2caba252f4dc // indirect modernc.org/libc v1.67.0 // indirect modernc.org/mathutil v1.7.1 // indirect diff --git a/go.sum b/go.sum index 82dfee6..de28d55 100644 --- a/go.sum +++ b/go.sum @@ -82,17 +82,19 @@ golang.org/x/crypto v0.44.0 h1:A97SsFvM3AIwEEmTBiaxPPTYpDC47w720rdiiUvgoAU= golang.org/x/crypto v0.44.0/go.mod h1:013i+Nw79BMiQiMsOPcVCB5ZIJbYkerPrGnOa00tvmc= golang.org/x/exp v0.0.0-20251113190631-e25ba8c21ef6 h1:zfMcR1Cs4KNuomFFgGefv5N0czO2XZpUbxGUy8i8ug0= golang.org/x/exp v0.0.0-20251113190631-e25ba8c21ef6/go.mod h1:46edojNIoXTNOhySWIWdix628clX9ODXwPsQuG6hsK0= -golang.org/x/mod v0.30.0 h1:fDEXFVZ/fmCKProc/yAXXUijritrDzahmwwefnjoPFk= -golang.org/x/mod v0.30.0/go.mod h1:lAsf5O2EvJeSFMiBxXDki7sCgAxEUcZHXoXMKT4GJKc= -golang.org/x/sync v0.18.0 h1:kr88TuHDroi+UVf+0hZnirlk8o8T+4MrK6mr60WkH/I= -golang.org/x/sync v0.18.0/go.mod h1:9KTHXmSnoGruLpwFjVSX0lNNA75CykiMECbovNTZqGI= +golang.org/x/image v0.46.0 h1:b1+oYj0Jbp6K5MDT4i4/eZpYlk3V8SJhhDKh6LBHAyQ= +golang.org/x/image v0.46.0/go.mod h1:3B3W05VGVQyuXucLINLjXKrqISASfi4Xj+iCVkLMwew= +golang.org/x/mod v0.41.0 h1:qJmnOUb4YB+FsEuM3HcWucdZASCPGhsX6uljO6pog0c= +golang.org/x/mod v0.41.0/go.mod h1:Ek9pY8RKWXwsWvd3rQiHYtMqkjSUV+s1Rj7j4H5Ur6o= +golang.org/x/sync v0.23.0 h1:KameEIfc1IkluZyXWLn39Wd4tURc6GbCiISGiZm2bQk= +golang.org/x/sync v0.23.0/go.mod h1:sUUOizhqBxiL6pEWpqNLUiaJn1ShEbZ6BBqskPbjZm0= golang.org/x/sys v0.6.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/sys v0.38.0 h1:3yZWxaJjBmCWXqhN1qh02AkOnCQ1poK6oF+a7xWL6Gc= -golang.org/x/sys v0.38.0/go.mod h1:OgkHotnGiDImocRcuBABYBEXf8A9a87e/uXjp9XT3ks= -golang.org/x/text v0.31.0 h1:aC8ghyu4JhP8VojJ2lEHBnochRno1sgL6nEi9WGFGMM= -golang.org/x/text v0.31.0/go.mod h1:tKRAlv61yKIjGGHX/4tP1LTbc13YSec1pxVEWXzfoeM= -golang.org/x/tools v0.39.0 h1:ik4ho21kwuQln40uelmciQPp9SipgNDdrafrYA4TmQQ= -golang.org/x/tools v0.39.0/go.mod h1:JnefbkDPyD8UU2kI5fuf8ZX4/yUeh9W877ZeBONxUqQ= +golang.org/x/sys v0.48.0 h1:bbX/i/6MgT9BVLM9RT1thmxL04yeTAhbEz4SyadbXoo= +golang.org/x/sys v0.48.0/go.mod h1:hNLxWAXmnKAxqDtdwIYC4bM9oQPEecfsnNMuSxOs3og= +golang.org/x/text v0.42.0 h1:JbOZXgfeCPU9gacVtYliJqOhD+zhrEqK4LfdpmlUZqI= +golang.org/x/text v0.42.0/go.mod h1:ojzP1Z+2QtioaF8DTtO8K5q7JWVVYwZKenzujK0Zd0E= +golang.org/x/tools v0.49.0 h1:3NI7VXzL9+1WZD52Dx2ttoPwD5DWrFGpl9mFZDlmisI= +golang.org/x/tools v0.49.0/go.mod h1:SJNXV9DBKT0UbdttsQjbfJlAE/q+y36++zo3uL3N0Oo= gopkg.in/alexcesaro/quotedprintable.v3 v3.0.0-20150716171945-2caba252f4dc h1:2gGKlE2+asNV9m7xrywl36YYNnBG5ZQ0r/BOOxqPpmk= gopkg.in/alexcesaro/quotedprintable.v3 v3.0.0-20150716171945-2caba252f4dc/go.mod h1:m7x9LTH6d71AHyAX77c9yqWCCa3UKHcVEj9y7hAtKDk= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= diff --git a/internal/domain/article/articleDomain.go b/internal/domain/article/articleDomain.go index c144161..8260fef 100644 --- a/internal/domain/article/articleDomain.go +++ b/internal/domain/article/articleDomain.go @@ -1,7 +1,6 @@ package article import ( - domain "RenewCMS/internal/domain/image" "errors" "time" "unicode/utf8" @@ -27,11 +26,10 @@ func ValidateTitle(title string) error { } type Article struct { - ID uint32 `json:"id"` - Title string `json:"title"` - Body string `json:"body"` - Images []*domain.Image `json:"images"` - IsOnline bool `json:"is_online"` - CreatedAt time.Time `json:"created_at"` - UpdatedAt time.Time `json:"updated_at"` + ID uint32 `json:"id"` + Title string `json:"title"` + Body string `json:"body"` + IsOnline bool `json:"is_online"` + CreatedAt time.Time `json:"created_at"` + UpdatedAt time.Time `json:"updated_at"` } diff --git a/internal/domain/article/articleRepositoryInterface.go b/internal/domain/article/articleRepositoryInterface.go index d1fbd40..dd47b1c 100644 --- a/internal/domain/article/articleRepositoryInterface.go +++ b/internal/domain/article/articleRepositoryInterface.go @@ -13,5 +13,5 @@ type Repository interface { UpdateBody(id uint32, body string) (Article, error) UpdateIsOnline(id uint32, isOnline bool) (Article, error) Delete(id uint32) error - AddImage(postId uint32, imageId uint32) error + FindByBodyContaining(filename string) ([]Article, error) } diff --git a/internal/domain/image/imageDomain.go b/internal/domain/image/imageDomain.go index 4649d95..6d2ca68 100644 --- a/internal/domain/image/imageDomain.go +++ b/internal/domain/image/imageDomain.go @@ -3,16 +3,33 @@ package image import ( "errors" "io" + "path" + "regexp" "sort" + "strings" "time" + "unicode/utf8" ) 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") +var ErrImageUnreadable = errors.New("the image could not be read") +var ErrInvalidFilename = errors.New("invalid filename") +var ErrInvalidQuery = errors.New("invalid media query") +var ErrInvalidMergeRequest = errors.New("invalid merge request") +var ErrMergeCopyMissing = errors.New("a merged copy no longer exists") +var ErrMergeHashMismatch = errors.New("a merged copy does not share the kept image's content") const MaxImageSize int64 = 10485760 +const ( + MinFilenameRunes = 1 + MaxFilenameRunes = 100 + DefaultListLimit = 24 + MaxListLimit = 100 +) + var allowedContentTypes = map[string]bool{ "image/png": true, "image/jpeg": true, @@ -20,6 +37,16 @@ var allowedContentTypes = map[string]bool{ "image/svg+xml": true, } +var canonicalExtensions = map[string]string{ + "image/png": ".png", + "image/jpeg": ".jpg", + "image/webp": ".webp", + "image/svg+xml": ".svg", +} + +var filenameStemPattern = regexp.MustCompile(`^[\p{L}\p{N}_-]+$`) +var sha256Pattern = regexp.MustCompile(`^[0-9a-f]{64}$`) + func IsAllowedContentType(contentType string) bool { return allowedContentTypes[contentType] } @@ -33,17 +60,147 @@ func AllowedContentTypes() []string { return types } +func NormalizeUploadFilename(original string, contentType string) string { + name := original[strings.LastIndexAny(original, `/\`)+1:] + name = strings.TrimSpace(name) + if name == "" || name == "." { + name = "image" + } + if path.Ext(name) == "" { + name += canonicalExtensions[contentType] + } + return name +} + +func ExtensionOf(filename string) string { + return path.Ext(filename) +} + +func StemOf(filename string) string { + return strings.TrimSuffix(filename, path.Ext(filename)) +} + +func ValidateFilenameStem(stem string) (string, error) { + trimmed := strings.TrimSpace(stem) + runeCount := utf8.RuneCountInString(trimmed) + if runeCount < MinFilenameRunes || runeCount > MaxFilenameRunes { + return "", ErrInvalidFilename + } + if !filenameStemPattern.MatchString(trimmed) { + return "", ErrInvalidFilename + } + return trimmed, nil +} + +func StoredName(imagePath string) string { + return path.Base(imagePath) +} + +type Sort string + +const ( + SortNewest Sort = "newest" + SortOldest Sort = "oldest" + SortName Sort = "name" + SortLargest Sort = "largest" +) + +type Filter string + +const ( + FilterAll Filter = "all" + FilterUnused Filter = "unused" + FilterDuplicates Filter = "duplicates" +) + +type ListQuery struct { + Search string + Hash string + Filter Filter + Sort Sort + Limit int + Cursor string +} + +func NewListQuery() ListQuery { + return ListQuery{Filter: FilterAll, Sort: SortNewest, Limit: DefaultListLimit} +} + +func (q ListQuery) Validate() error { + switch q.Filter { + case FilterAll, FilterUnused, FilterDuplicates: + default: + return ErrInvalidQuery + } + switch q.Sort { + case SortNewest, SortOldest, SortName, SortLargest: + default: + return ErrInvalidQuery + } + if q.Limit < 1 || q.Limit > MaxListLimit { + return ErrInvalidQuery + } + if q.Hash != "" && !sha256Pattern.MatchString(q.Hash) { + return ErrInvalidQuery + } + return nil +} + +func SortImages(images []Image, order Sort) { + sort.SliceStable(images, func(i, j int) bool { + a, b := images[i], images[j] + switch order { + case SortOldest: + if !a.CreatedAt.Equal(b.CreatedAt) { + return a.CreatedAt.Before(b.CreatedAt) + } + return a.ID < b.ID + case SortName: + nameA, nameB := strings.ToLower(StemOf(a.Filename)), strings.ToLower(StemOf(b.Filename)) + if nameA != nameB { + return nameA < nameB + } + return a.ID < b.ID + case SortLargest: + if a.Size != b.Size { + return a.Size > b.Size + } + return a.ID < b.ID + default: + if !a.CreatedAt.Equal(b.CreatedAt) { + return a.CreatedAt.After(b.CreatedAt) + } + return a.ID > b.ID + } + }) +} + +type Counts struct { + All int + DuplicateGroups int +} + type ImageInput struct { - Content io.Reader - Filename string - Size int64 - ContentType string + Content io.Reader + Filename string + Size int64 + ContentType string + Width int + Height int + SHA256 string + UploadedByID uint32 } type Image struct { - ID uint32 `json:"id"` - Path string `json:"path"` - ArticleID uint32 `json:"article_id"` - CreatedAt time.Time `json:"created_at"` - UpdatedAt time.Time `json:"updated_at"` + ID uint32 `json:"id"` + Path string `json:"path"` + Filename string `json:"filename"` + Size int64 `json:"size"` + ContentType string `json:"content_type"` + Width int `json:"width"` + Height int `json:"height"` + SHA256 string `json:"sha256"` + UploadedByID uint32 `json:"uploaded_by_id"` + CreatedAt time.Time `json:"created_at"` + UpdatedAt time.Time `json:"updated_at"` } diff --git a/internal/domain/image/imageRepositoryInterface.go b/internal/domain/image/imageRepositoryInterface.go index e10f9ca..00f2664 100644 --- a/internal/domain/image/imageRepositoryInterface.go +++ b/internal/domain/image/imageRepositoryInterface.go @@ -1,6 +1,17 @@ package image +type SearchQuery struct { + Search string + Hash string +} + type Repository interface { Create(input ImageInput) (Image, error) + Get(id uint32) (Image, error) Delete(id uint32) error + List(query SearchQuery) ([]Image, error) + FindByHash(hash string) ([]Image, error) + Rename(id uint32, filename string) (Image, error) + ListDuplicates() ([]Image, error) + Counts() (Counts, error) } diff --git a/internal/infrastructure/persistence/mappers/articleMapper.go b/internal/infrastructure/persistence/mappers/articleMapper.go index 1df395b..b7bf829 100644 --- a/internal/infrastructure/persistence/mappers/articleMapper.go +++ b/internal/infrastructure/persistence/mappers/articleMapper.go @@ -2,22 +2,14 @@ package mappers import ( "RenewCMS/internal/domain/article" - "RenewCMS/internal/domain/image" "RenewCMS/internal/infrastructure/persistence/models" ) func ArticleToDomain(m models.Article) article.Article { - domainImages := make([]*image.Image, len(m.Images)) - for i, img := range m.Images { - domainImg := ImageToDomain(*img) - domainImages[i] = &domainImg - } - return article.Article{ ID: m.ID, Title: m.Title, Body: m.Body, - Images: domainImages, IsOnline: m.IsOnline, CreatedAt: m.CreatedAt, UpdatedAt: m.UpdatedAt, @@ -25,16 +17,9 @@ func ArticleToDomain(m models.Article) article.Article { } func ArticleToModel(a article.Article) models.Article { - modelImages := make([]*models.Image, len(a.Images)) - for i, img := range a.Images { - modelImage := ImageToModel(*img) - modelImages[i] = &modelImage - } - return models.Article{ Title: a.Title, Body: a.Body, - Images: modelImages, IsOnline: a.IsOnline, CreatedAt: a.CreatedAt, UpdatedAt: a.UpdatedAt, diff --git a/internal/infrastructure/persistence/mappers/imageMapper.go b/internal/infrastructure/persistence/mappers/imageMapper.go index ca85a8b..8086ec0 100644 --- a/internal/infrastructure/persistence/mappers/imageMapper.go +++ b/internal/infrastructure/persistence/mappers/imageMapper.go @@ -7,19 +7,32 @@ import ( func ImageToDomain(imageModel models.Image) image.Image { return image.Image{ - ID: imageModel.ID, - Path: imageModel.Path, - ArticleID: imageModel.ArticleID, - CreatedAt: imageModel.CreatedAt, - UpdatedAt: imageModel.UpdatedAt, + ID: imageModel.ID, + Path: imageModel.Path, + Filename: imageModel.Filename, + Size: imageModel.Size, + ContentType: imageModel.ContentType, + Width: imageModel.Width, + Height: imageModel.Height, + SHA256: imageModel.SHA256, + UploadedByID: imageModel.UploadedByID, + CreatedAt: imageModel.CreatedAt, + UpdatedAt: imageModel.UpdatedAt, } } func ImageToModel(imageDomain image.Image) models.Image { return models.Image{ - Path: imageDomain.Path, - ArticleID: imageDomain.ArticleID, - CreatedAt: imageDomain.CreatedAt, - UpdatedAt: imageDomain.UpdatedAt, + ID: imageDomain.ID, + Path: imageDomain.Path, + Filename: imageDomain.Filename, + Size: imageDomain.Size, + ContentType: imageDomain.ContentType, + Width: imageDomain.Width, + Height: imageDomain.Height, + SHA256: imageDomain.SHA256, + UploadedByID: imageDomain.UploadedByID, + CreatedAt: imageDomain.CreatedAt, + UpdatedAt: imageDomain.UpdatedAt, } } diff --git a/internal/infrastructure/persistence/models/articleModel.go b/internal/infrastructure/persistence/models/articleModel.go index 5388146..2f8bd55 100644 --- a/internal/infrastructure/persistence/models/articleModel.go +++ b/internal/infrastructure/persistence/models/articleModel.go @@ -11,7 +11,6 @@ type Article struct { ID uint32 `gorm:"primary_key;auto_increment;not_null"` Title string Body string - Images []*Image `gorm:"many2many:article_images;"` IsOnline bool `gorm:"not_null;default:false"` CreatedAt time.Time `gorm:"autoCreateTime"` UpdatedAt time.Time `gorm:"autoUpdateTime"` diff --git a/internal/infrastructure/persistence/models/imageModel.go b/internal/infrastructure/persistence/models/imageModel.go index 3e6d0f2..8a48323 100644 --- a/internal/infrastructure/persistence/models/imageModel.go +++ b/internal/infrastructure/persistence/models/imageModel.go @@ -8,9 +8,15 @@ import ( type Image struct { gorm.Model - ID uint32 `gorm:"primary_key;auto_increment;not_null"` - Path string - ArticleID uint32 - CreatedAt time.Time `gorm:"autoCreateTime"` - UpdatedAt time.Time `gorm:"autoUpdateTime"` + ID uint32 `gorm:"primary_key;auto_increment;not_null"` + Path string + Filename string + Size int64 + ContentType string + Width int + Height int + SHA256 string `gorm:"column:sha256;index"` + UploadedByID uint32 + CreatedAt time.Time `gorm:"autoCreateTime"` + UpdatedAt time.Time `gorm:"autoUpdateTime"` } diff --git a/internal/infrastructure/persistence/repositories/articleRepository.go b/internal/infrastructure/persistence/repositories/articleRepository.go index ac05453..ac4fb99 100644 --- a/internal/infrastructure/persistence/repositories/articleRepository.go +++ b/internal/infrastructure/persistence/repositories/articleRepository.go @@ -2,7 +2,6 @@ package persistence import ( domain "RenewCMS/internal/domain/article" - imageDomain "RenewCMS/internal/domain/image" "RenewCMS/internal/infrastructure/persistence/mappers" entity "RenewCMS/internal/infrastructure/persistence/models" @@ -18,7 +17,7 @@ func NewArticleRepository(db *gorm.DB) *ArticleRepository { } func (a *ArticleRepository) FindByFilters(filters domain.Filters) ([]domain.Article, error) { - query := a.db.Model(&entity.Article{}).Preload("Images") + query := a.db.Model(&entity.Article{}) if filters.IsOnline != nil { query = query.Where("is_online = ?", *filters.IsOnline) @@ -37,7 +36,7 @@ func (a *ArticleRepository) FindByFilters(filters domain.Filters) ([]domain.Arti func (a *ArticleRepository) Get(id uint32) (domain.Article, error) { var article entity.Article - err := a.db.Model(&entity.Article{}).Preload("Images").First(&article, id).Error + err := a.db.Model(&entity.Article{}).First(&article, id).Error if err != nil { return domain.Article{}, mapNotFoundErr(err, domain.ErrArticleNotFound) } @@ -125,25 +124,18 @@ func (a *ArticleRepository) Delete(id uint32) error { return deleteOrNotFound(result, domain.ErrArticleNotFound) } -func (a *ArticleRepository) AddImage(articleId uint32, imageId uint32) error { - var localArticle entity.Article - err := a.db.Model(&entity.Article{}).First(&localArticle, articleId).Error +func (a *ArticleRepository) FindByBodyContaining(filename string) ([]domain.Article, error) { + var entityArticles []entity.Article + err := a.db.Model(&entity.Article{}).Where("instr(body, ?) > 0", filename).Order("id asc").Find(&entityArticles).Error if err != nil { - return mapNotFoundErr(err, domain.ErrArticleNotFound) + return nil, err } - var localImage entity.Image - err = a.db.Model(&entity.Image{}).First(&localImage, imageId).Error - if err != nil { - return mapNotFoundErr(err, imageDomain.ErrImageNotFound) + domainArticles := make([]domain.Article, 0, len(entityArticles)) + for _, entityArticle := range entityArticles { + domainArticles = append(domainArticles, mappers.ArticleToDomain(entityArticle)) } - - err = a.db.Model(&localArticle).Association("Images").Append(&localImage) - if err != nil { - return err - } - - return nil + return domainArticles, nil } var _ domain.Repository = &ArticleRepository{} diff --git a/internal/infrastructure/persistence/repositories/imageBackfill.go b/internal/infrastructure/persistence/repositories/imageBackfill.go new file mode 100644 index 0000000..93be4f8 --- /dev/null +++ b/internal/infrastructure/persistence/repositories/imageBackfill.go @@ -0,0 +1,77 @@ +package persistence + +import ( + entity "RenewCMS/internal/infrastructure/persistence/models" + "bytes" + "crypto/sha256" + "encoding/hex" + stdimage "image" + "os" + "path/filepath" + + _ "golang.org/x/image/webp" + _ "image/jpeg" + _ "image/png" + + "gorm.io/gorm" +) + +var extensionContentTypes = map[string]string{ + ".png": "image/png", + ".jpg": "image/jpeg", + ".jpeg": "image/jpeg", + ".webp": "image/webp", + ".svg": "image/svg+xml", +} + +func BackfillImageMetadata(db *gorm.DB, uploadDir string) error { + var pending []entity.Image + err := db.Where("filename = '' OR filename IS NULL").Order("id asc").Find(&pending).Error + if err != nil { + return err + } + + for _, row := range pending { + metadata, ok := readStoredImage(uploadDir, row.Path) + if !ok { + continue + } + err := db.Model(&entity.Image{}).Where("id = ?", row.ID).UpdateColumns(metadata).Error + if err != nil { + return err + } + } + return nil +} + +func readStoredImage(uploadDir string, storedPath string) (map[string]any, bool) { + fileName := filepath.Base(storedPath) + contentType, known := extensionContentTypes[filepath.Ext(fileName)] + if !known { + return nil, false + } + + data, err := os.ReadFile(filepath.Join(uploadDir, fileName)) + if err != nil { + return nil, false + } + + width, height := 0, 0 + if contentType != "image/svg+xml" { + config, _, err := stdimage.DecodeConfig(bytes.NewReader(data)) + if err != nil { + return nil, false + } + width, height = config.Width, config.Height + } + + checksum := sha256.Sum256(data) + return map[string]any{ + "filename": fileName, + "size": int64(len(data)), + "content_type": contentType, + "width": width, + "height": height, + "sha256": hex.EncodeToString(checksum[:]), + }, true +} diff --git a/internal/infrastructure/persistence/repositories/imageRepository.go b/internal/infrastructure/persistence/repositories/imageRepository.go index d6bddc7..16144e4 100644 --- a/internal/infrastructure/persistence/repositories/imageRepository.go +++ b/internal/infrastructure/persistence/repositories/imageRepository.go @@ -4,14 +4,18 @@ import ( domain "RenewCMS/internal/domain/image" "RenewCMS/internal/infrastructure/persistence/mappers" entity "RenewCMS/internal/infrastructure/persistence/models" + "errors" "io" "os" "path/filepath" + "strings" "github.com/google/uuid" "gorm.io/gorm" ) +const uploadedImagesURLPrefix = "/static/uploadedImages/" + type ImageRepository struct { db *gorm.DB uploadDir string @@ -29,49 +33,142 @@ var contentTypeExtensions = map[string]string{ } func (i ImageRepository) Create(input domain.ImageInput) (domain.Image, error) { - uploadDir := i.uploadDir - fileBytes, err := io.ReadAll(input.Content) if err != nil { return domain.Image{}, err } - err = os.MkdirAll(uploadDir, 0755) + err = os.MkdirAll(i.uploadDir, 0755) if err != nil { return domain.Image{}, err } - extension := contentTypeExtensions[input.ContentType] - - newName := uuid.NewString() + extension - finalPath := filepath.Join(uploadDir, newName) + newName := uuid.NewString() + contentTypeExtensions[input.ContentType] + finalPath := filepath.Join(i.uploadDir, newName) err = os.WriteFile(finalPath, fileBytes, 0666) if err != nil { return domain.Image{}, err } - newImage := i.db.Create(&domain.Image{Path: "/static/uploadedImages/" + newName}) - var createdImage entity.Image - newImage.Scan(&createdImage) + model := mappers.ImageToModel(domain.Image{ + Path: uploadedImagesURLPrefix + newName, + Filename: input.Filename, + Size: input.Size, + ContentType: input.ContentType, + Width: input.Width, + Height: input.Height, + SHA256: input.SHA256, + UploadedByID: input.UploadedByID, + }) + if err := i.db.Create(&model).Error; err != nil { + _ = os.Remove(finalPath) + return domain.Image{}, err + } - return mappers.ImageToDomain(createdImage), nil + return mappers.ImageToDomain(model), nil +} + +func (i ImageRepository) Get(id uint32) (domain.Image, error) { + var model entity.Image + err := i.db.First(&model, id).Error + if err != nil { + return domain.Image{}, mapNotFoundErr(err, domain.ErrImageNotFound) + } + return mappers.ImageToDomain(model), nil } func (i ImageRepository) Delete(id uint32) error { - uploadDir := i.uploadDir - var entityImage entity.Image - err := i.db.Model(&entity.Image{}).First(&entityImage, id).Error + var model entity.Image + err := i.db.First(&model, id).Error if err != nil { return mapNotFoundErr(err, domain.ErrImageNotFound) } - fileName := filepath.Base(entityImage.Path) - err = os.Remove(filepath.Join(uploadDir, fileName)) - if err != nil { + err = os.Remove(filepath.Join(i.uploadDir, filepath.Base(model.Path))) + if err != nil && !errors.Is(err, os.ErrNotExist) { return err } return i.db.Delete(&entity.Image{}, id).Error } +func (i ImageRepository) List(query domain.SearchQuery) ([]domain.Image, error) { + statement := i.db.Model(&entity.Image{}) + if query.Search != "" { + statement = statement.Where("instr(lower(filename), ?) > 0", strings.ToLower(query.Search)) + } + if query.Hash != "" { + statement = statement.Where("sha256 = ?", query.Hash) + } + + var models []entity.Image + if err := statement.Order("id asc").Find(&models).Error; err != nil { + return nil, err + } + return imagesToDomain(models), nil +} + +func (i ImageRepository) FindByHash(hash string) ([]domain.Image, error) { + if hash == "" { + return []domain.Image{}, nil + } + var models []entity.Image + if err := i.db.Where("sha256 = ?", hash).Order("id asc").Find(&models).Error; err != nil { + return nil, err + } + return imagesToDomain(models), nil +} + +func (i ImageRepository) Rename(id uint32, filename string) (domain.Image, error) { + var model entity.Image + err := i.db.First(&model, id).Error + if err != nil { + return domain.Image{}, mapNotFoundErr(err, domain.ErrImageNotFound) + } + + model.Filename = filename + if err := i.db.Save(&model).Error; err != nil { + return domain.Image{}, err + } + return mappers.ImageToDomain(model), nil +} + +func (i ImageRepository) ListDuplicates() ([]domain.Image, error) { + var models []entity.Image + err := i.db.Where("sha256 IN (?)", i.duplicatedHashes()).Order("sha256 asc, id asc").Find(&models).Error + if err != nil { + return nil, err + } + return imagesToDomain(models), nil +} + +func (i ImageRepository) Counts() (domain.Counts, error) { + var all int64 + if err := i.db.Model(&entity.Image{}).Count(&all).Error; err != nil { + return domain.Counts{}, err + } + + var groups int64 + if err := i.db.Table("(?) as duplicated", i.duplicatedHashes()).Count(&groups).Error; err != nil { + return domain.Counts{}, err + } + return domain.Counts{All: int(all), DuplicateGroups: int(groups)}, nil +} + +func (i ImageRepository) duplicatedHashes() *gorm.DB { + return i.db.Model(&entity.Image{}). + Select("sha256"). + Where("sha256 <> ''"). + Group("sha256"). + Having("COUNT(*) > 1") +} + +func imagesToDomain(models []entity.Image) []domain.Image { + images := make([]domain.Image, 0, len(models)) + for _, model := range models { + images = append(images, mappers.ImageToDomain(model)) + } + return images +} + var _ domain.Repository = &ImageRepository{} diff --git a/internal/infrastructure/useCases/CreateImage.go b/internal/infrastructure/useCases/CreateImage.go index 833c17c..eedab90 100644 --- a/internal/infrastructure/useCases/CreateImage.go +++ b/internal/infrastructure/useCases/CreateImage.go @@ -2,8 +2,26 @@ package useCases import ( "RenewCMS/internal/domain/image" + "bytes" + "crypto/sha256" + "encoding/hex" + "encoding/xml" + stdimage "image" + "io" + + _ "golang.org/x/image/webp" + _ "image/jpeg" + _ "image/png" ) +const svgRootElement = "svg" + +var rasterFormats = map[string]string{ + "image/png": "png", + "image/jpeg": "jpeg", + "image/webp": "webp", +} + type CreateImageUseCase struct { imageRepository image.Repository } @@ -16,8 +34,68 @@ func (g *CreateImageUseCase) CreateImage(input image.ImageInput) (image.Image, e if !image.IsAllowedContentType(input.ContentType) { return image.Image{}, image.ErrImageTypeNotAllowed } - if input.Size > image.MaxImageSize { - return image.Image{}, image.ErrImageTooLarge + + data, err := readWithinLimit(input.Content) + if err != nil { + return image.Image{}, err + } + + width, height, err := measure(data, input.ContentType) + if err != nil { + return image.Image{}, err + } + + checksum := sha256.Sum256(data) + return g.imageRepository.Create(image.ImageInput{ + Content: bytes.NewReader(data), + Filename: image.NormalizeUploadFilename(input.Filename, input.ContentType), + Size: int64(len(data)), + ContentType: input.ContentType, + Width: width, + Height: height, + SHA256: hex.EncodeToString(checksum[:]), + UploadedByID: input.UploadedByID, + }) +} + +func readWithinLimit(content io.Reader) ([]byte, error) { + if content == nil { + return nil, image.ErrImageUnreadable + } + data, err := io.ReadAll(io.LimitReader(content, image.MaxImageSize+1)) + if err != nil { + return nil, err + } + if int64(len(data)) > image.MaxImageSize { + return nil, image.ErrImageTooLarge + } + return data, nil +} + +func measure(data []byte, contentType string) (int, int, error) { + if contentType == "image/svg+xml" { + if !hasSvgRoot(data) { + return 0, 0, image.ErrImageUnreadable + } + return 0, 0, nil + } + + config, format, err := stdimage.DecodeConfig(bytes.NewReader(data)) + if err != nil || format != rasterFormats[contentType] { + return 0, 0, image.ErrImageUnreadable + } + return config.Width, config.Height, nil +} + +func hasSvgRoot(data []byte) bool { + decoder := xml.NewDecoder(bytes.NewReader(data)) + for { + token, err := decoder.Token() + if err != nil { + return false + } + if start, ok := token.(xml.StartElement); ok { + return start.Name.Local == svgRootElement + } } - return g.imageRepository.Create(input) } diff --git a/internal/infrastructure/useCases/DeleteArticle.go b/internal/infrastructure/useCases/DeleteArticle.go index 3f63b66..9681abe 100644 --- a/internal/infrastructure/useCases/DeleteArticle.go +++ b/internal/infrastructure/useCases/DeleteArticle.go @@ -1,30 +1,15 @@ package useCases -import ( - "RenewCMS/internal/domain/article" - "RenewCMS/internal/domain/image" -) +import "RenewCMS/internal/domain/article" type DeleteArticleUseCase struct { articleRepository article.Repository - imageRepository image.Repository } -func NewDeleteArticleUseCase(articleRepository article.Repository, imageRepository image.Repository) *DeleteArticleUseCase { - return &DeleteArticleUseCase{articleRepository, imageRepository} +func NewDeleteArticleUseCase(articleRepository article.Repository) *DeleteArticleUseCase { + return &DeleteArticleUseCase{articleRepository} } func (g *DeleteArticleUseCase) DeleteArticle(id uint32) error { - a, err := g.articleRepository.Get(id) - if err != nil { - return err - } - - for _, img := range a.Images { - if err := g.imageRepository.Delete(img.ID); err != nil { - return err - } - } - return g.articleRepository.Delete(id) } diff --git a/internal/infrastructure/useCases/DeleteImage.go b/internal/infrastructure/useCases/DeleteImage.go deleted file mode 100644 index 81fca69..0000000 --- a/internal/infrastructure/useCases/DeleteImage.go +++ /dev/null @@ -1,15 +0,0 @@ -package useCases - -import "RenewCMS/internal/domain/image" - -type DeleteImageUseCase struct { - imageRepository image.Repository -} - -func NewDeleteImageUseCase(imageRepository image.Repository) *DeleteImageUseCase { - return &DeleteImageUseCase{imageRepository} -} - -func (g *DeleteImageUseCase) DeleteImage(imageId uint32) error { - return g.imageRepository.Delete(imageId) -} diff --git a/internal/infrastructure/useCases/DeleteMedia.go b/internal/infrastructure/useCases/DeleteMedia.go new file mode 100644 index 0000000..accb77b --- /dev/null +++ b/internal/infrastructure/useCases/DeleteMedia.go @@ -0,0 +1,15 @@ +package useCases + +import "RenewCMS/internal/domain/image" + +type DeleteMediaUseCase struct { + imageRepository image.Repository +} + +func NewDeleteMediaUseCase(imageRepository image.Repository) *DeleteMediaUseCase { + return &DeleteMediaUseCase{imageRepository} +} + +func (u *DeleteMediaUseCase) Delete(id uint32) error { + return u.imageRepository.Delete(id) +} diff --git a/internal/infrastructure/useCases/GetMediaUsage.go b/internal/infrastructure/useCases/GetMediaUsage.go new file mode 100644 index 0000000..c505d74 --- /dev/null +++ b/internal/infrastructure/useCases/GetMediaUsage.go @@ -0,0 +1,23 @@ +package useCases + +import ( + "RenewCMS/internal/domain/article" + "RenewCMS/internal/domain/image" +) + +type GetMediaUsageUseCase struct { + imageRepository image.Repository + articleRepository article.Repository +} + +func NewGetMediaUsageUseCase(imageRepository image.Repository, articleRepository article.Repository) *GetMediaUsageUseCase { + return &GetMediaUsageUseCase{imageRepository, articleRepository} +} + +func (u *GetMediaUsageUseCase) Usage(id uint32) ([]article.Article, error) { + img, err := u.imageRepository.Get(id) + if err != nil { + return nil, err + } + return u.articleRepository.FindByBodyContaining(image.StoredName(img.Path)) +} diff --git a/internal/infrastructure/useCases/ListMedia.go b/internal/infrastructure/useCases/ListMedia.go new file mode 100644 index 0000000..73d9628 --- /dev/null +++ b/internal/infrastructure/useCases/ListMedia.go @@ -0,0 +1,199 @@ +package useCases + +import ( + "RenewCMS/internal/domain/article" + "RenewCMS/internal/domain/image" + "RenewCMS/internal/domain/user" + "encoding/base64" + "strconv" + "strings" +) + +const cursorPrefix = "offset:" + +type MediaItem struct { + Image image.Image + UploadedBy string + UsedIn int + Duplicate bool +} + +type MediaCounts struct { + All int + Unused int + Duplicates int +} + +type MediaPage struct { + Items []MediaItem + NextCursor string + Counts MediaCounts +} + +type ListMediaUseCase struct { + imageRepository image.Repository + articleRepository article.Repository + userRepository user.Repository +} + +func NewListMediaUseCase(imageRepository image.Repository, articleRepository article.Repository, userRepository user.Repository) *ListMediaUseCase { + return &ListMediaUseCase{imageRepository, articleRepository, userRepository} +} + +func (l *ListMediaUseCase) List(query image.ListQuery) (MediaPage, error) { + if err := query.Validate(); err != nil { + return MediaPage{}, err + } + offset, err := decodeCursor(query.Cursor) + if err != nil { + return MediaPage{}, err + } + + usage, err := l.usageCounter() + if err != nil { + return MediaPage{}, err + } + duplicateHashes, err := l.duplicateHashes() + if err != nil { + return MediaPage{}, err + } + counts, err := l.counts(usage) + if err != nil { + return MediaPage{}, err + } + + images, err := l.imageRepository.List(image.SearchQuery{Search: strings.TrimSpace(query.Search), Hash: query.Hash}) + if err != nil { + return MediaPage{}, err + } + image.SortImages(images, query.Sort) + + matching := make([]image.Image, 0, len(images)) + for _, img := range images { + if matchesFilter(query.Filter, usage(img), duplicateHashes[img.SHA256] && img.SHA256 != "") { + matching = append(matching, img) + } + } + + pageEnd := min(offset+query.Limit, len(matching)) + pageStart := min(offset, pageEnd) + names := newUploaderNames(l.userRepository) + items := []MediaItem{} + for _, img := range matching[pageStart:pageEnd] { + uploadedBy, err := names.nameOf(img.UploadedByID) + if err != nil { + return MediaPage{}, err + } + items = append(items, MediaItem{ + Image: img, + UploadedBy: uploadedBy, + UsedIn: usage(img), + Duplicate: img.SHA256 != "" && duplicateHashes[img.SHA256], + }) + } + + page := MediaPage{Items: items, Counts: counts} + if pageEnd < len(matching) { + page.NextCursor = encodeCursor(pageEnd) + } + return page, nil +} + +func (l *ListMediaUseCase) Describe(img image.Image) (MediaItem, error) { + articles, err := l.articleRepository.FindByBodyContaining(image.StoredName(img.Path)) + if err != nil { + return MediaItem{}, err + } + uploadedBy, err := newUploaderNames(l.userRepository).nameOf(img.UploadedByID) + if err != nil { + return MediaItem{}, err + } + + duplicate := false + if img.SHA256 != "" { + sameContent, err := l.imageRepository.FindByHash(img.SHA256) + if err != nil { + return MediaItem{}, err + } + duplicate = len(sameContent) > 1 + } + + return MediaItem{Image: img, UploadedBy: uploadedBy, UsedIn: len(articles), Duplicate: duplicate}, nil +} + +func (l *ListMediaUseCase) usageCounter() (func(image.Image) int, error) { + articles, err := l.articleRepository.FindByFilters(article.Filters{}) + if err != nil { + return nil, err + } + return func(img image.Image) int { + storedName := image.StoredName(img.Path) + used := 0 + for _, a := range articles { + if strings.Contains(a.Body, storedName) { + used++ + } + } + return used + }, nil +} + +func (l *ListMediaUseCase) duplicateHashes() (map[string]bool, error) { + duplicates, err := l.imageRepository.ListDuplicates() + if err != nil { + return nil, err + } + hashes := make(map[string]bool, len(duplicates)) + for _, img := range duplicates { + hashes[img.SHA256] = true + } + return hashes, nil +} + +func (l *ListMediaUseCase) counts(usage func(image.Image) int) (MediaCounts, error) { + libraryCounts, err := l.imageRepository.Counts() + if err != nil { + return MediaCounts{}, err + } + everything, err := l.imageRepository.List(image.SearchQuery{}) + if err != nil { + return MediaCounts{}, err + } + unused := 0 + for _, img := range everything { + if usage(img) == 0 { + unused++ + } + } + return MediaCounts{All: libraryCounts.All, Unused: unused, Duplicates: libraryCounts.DuplicateGroups}, nil +} + +func matchesFilter(filter image.Filter, usedIn int, duplicate bool) bool { + switch filter { + case image.FilterUnused: + return usedIn == 0 + case image.FilterDuplicates: + return duplicate + default: + return true + } +} + +func encodeCursor(offset int) string { + return base64.RawURLEncoding.EncodeToString([]byte(cursorPrefix + strconv.Itoa(offset))) +} + +func decodeCursor(cursor string) (int, error) { + if cursor == "" { + return 0, nil + } + raw, err := base64.RawURLEncoding.DecodeString(cursor) + if err != nil || !strings.HasPrefix(string(raw), cursorPrefix) { + return 0, image.ErrInvalidQuery + } + offset, err := strconv.Atoi(strings.TrimPrefix(string(raw), cursorPrefix)) + if err != nil || offset < 0 { + return 0, image.ErrInvalidQuery + } + return offset, nil +} diff --git a/internal/infrastructure/useCases/MergeMedia.go b/internal/infrastructure/useCases/MergeMedia.go new file mode 100644 index 0000000..ad4616e --- /dev/null +++ b/internal/infrastructure/useCases/MergeMedia.go @@ -0,0 +1,137 @@ +package useCases + +import ( + "RenewCMS/internal/domain/article" + "RenewCMS/internal/domain/image" + "cmp" + "errors" + "slices" + "strings" +) + +type MergeMediaResult struct { + ArticlesUpdated int + Merged int +} + +type MergeMediaUseCase struct { + imageRepository image.Repository + articleRepository article.Repository +} + +func NewMergeMediaUseCase(imageRepository image.Repository, articleRepository article.Repository) *MergeMediaUseCase { + return &MergeMediaUseCase{imageRepository, articleRepository} +} + +type bodyRewrite struct { + articleId uint32 + original string + rewritten string +} + +func (u *MergeMediaUseCase) Merge(keepId uint32, mergeIds []uint32) (MergeMediaResult, error) { + if err := validateMergeRequest(keepId, mergeIds); err != nil { + return MergeMediaResult{}, err + } + + kept, err := u.imageRepository.Get(keepId) + if err != nil { + return MergeMediaResult{}, err + } + copies, err := u.loadCopies(kept, mergeIds) + if err != nil { + return MergeMediaResult{}, err + } + + rewrites, err := u.planRewrites(kept, copies) + if err != nil { + return MergeMediaResult{}, err + } + if err := u.applyRewrites(rewrites); err != nil { + return MergeMediaResult{}, err + } + + for _, duplicate := range copies { + if err := u.imageRepository.Delete(duplicate.ID); err != nil { + return MergeMediaResult{}, err + } + } + + return MergeMediaResult{ArticlesUpdated: len(rewrites), Merged: len(copies)}, nil +} + +func validateMergeRequest(keepId uint32, mergeIds []uint32) error { + if keepId == 0 || len(mergeIds) == 0 { + return image.ErrInvalidMergeRequest + } + seen := map[uint32]bool{keepId: true} + for _, id := range mergeIds { + if id == 0 || seen[id] { + return image.ErrInvalidMergeRequest + } + seen[id] = true + } + return nil +} + +func (u *MergeMediaUseCase) loadCopies(kept image.Image, mergeIds []uint32) ([]image.Image, error) { + copies := make([]image.Image, 0, len(mergeIds)) + for _, id := range mergeIds { + duplicate, err := u.imageRepository.Get(id) + if errors.Is(err, image.ErrImageNotFound) { + return nil, image.ErrMergeCopyMissing + } + if err != nil { + return nil, err + } + if duplicate.SHA256 == "" || duplicate.SHA256 != kept.SHA256 { + return nil, image.ErrMergeHashMismatch + } + copies = append(copies, duplicate) + } + return copies, nil +} + +func (u *MergeMediaUseCase) planRewrites(kept image.Image, copies []image.Image) ([]bodyRewrite, error) { + keptName := image.StoredName(kept.Path) + rewritten := map[uint32]*bodyRewrite{} + + for _, duplicate := range copies { + copyName := image.StoredName(duplicate.Path) + articles, err := u.articleRepository.FindByBodyContaining(copyName) + if err != nil { + return nil, err + } + for _, a := range articles { + plan, ok := rewritten[a.ID] + if !ok { + plan = &bodyRewrite{articleId: a.ID, original: a.Body, rewritten: a.Body} + rewritten[a.ID] = plan + } + plan.rewritten = strings.ReplaceAll(plan.rewritten, copyName, keptName) + } + } + + plans := make([]bodyRewrite, 0, len(rewritten)) + for _, plan := range rewritten { + plans = append(plans, *plan) + } + slices.SortFunc(plans, func(a, b bodyRewrite) int { return cmp.Compare(a.articleId, b.articleId) }) + return plans, nil +} + +func (u *MergeMediaUseCase) applyRewrites(rewrites []bodyRewrite) error { + for applied, plan := range rewrites { + if _, err := u.articleRepository.UpdateBody(plan.articleId, plan.rewritten); err != nil { + u.restoreBodies(rewrites[:applied]) + return err + } + } + return nil +} + +func (u *MergeMediaUseCase) restoreBodies(applied []bodyRewrite) { + for _, plan := range applied { + _, _ = u.articleRepository.UpdateBody(plan.articleId, plan.original) + } +} diff --git a/internal/infrastructure/useCases/RenameMedia.go b/internal/infrastructure/useCases/RenameMedia.go new file mode 100644 index 0000000..3875338 --- /dev/null +++ b/internal/infrastructure/useCases/RenameMedia.go @@ -0,0 +1,25 @@ +package useCases + +import "RenewCMS/internal/domain/image" + +type RenameMediaUseCase struct { + imageRepository image.Repository +} + +func NewRenameMediaUseCase(imageRepository image.Repository) *RenameMediaUseCase { + return &RenameMediaUseCase{imageRepository} +} + +func (u *RenameMediaUseCase) Rename(id uint32, stem string) (image.Image, error) { + validStem, err := image.ValidateFilenameStem(stem) + if err != nil { + return image.Image{}, err + } + + current, err := u.imageRepository.Get(id) + if err != nil { + return image.Image{}, err + } + + return u.imageRepository.Rename(id, validStem+image.ExtensionOf(current.Filename)) +} diff --git a/internal/infrastructure/useCases/UpdateArticle.go b/internal/infrastructure/useCases/UpdateArticle.go index 5baafb6..db3dec7 100644 --- a/internal/infrastructure/useCases/UpdateArticle.go +++ b/internal/infrastructure/useCases/UpdateArticle.go @@ -14,10 +14,6 @@ func (g *UpdateArticleUseCase) UpdateBody(id uint32, body string) (article.Artic return g.articleRepository.UpdateBody(id, body) } -func (g *UpdateArticleUseCase) AddImage(articleId uint32, imageId uint32) error { - return g.articleRepository.AddImage(articleId, imageId) -} - func (g *UpdateArticleUseCase) UpdateIsOnline(id uint32, isOnline bool) (article.Article, error) { return g.articleRepository.UpdateIsOnline(id, isOnline) } diff --git a/internal/infrastructure/useCases/mediaUploader.go b/internal/infrastructure/useCases/mediaUploader.go new file mode 100644 index 0000000..2f2bff4 --- /dev/null +++ b/internal/infrastructure/useCases/mediaUploader.go @@ -0,0 +1,40 @@ +package useCases + +import ( + "RenewCMS/internal/domain/user" + "errors" +) + +type uploaderNames struct { + userRepository user.Repository + names map[uint32]string +} + +func newUploaderNames(userRepository user.Repository) *uploaderNames { + return &uploaderNames{userRepository: userRepository, names: map[uint32]string{}} +} + +func (u *uploaderNames) nameOf(userId uint32) (string, error) { + if userId == 0 { + return "", nil + } + if name, ok := u.names[userId]; ok { + return name, nil + } + + uploader, err := u.userRepository.Get(userId) + if errors.Is(err, user.ErrUserNotFound) { + u.names[userId] = "" + return "", nil + } + if err != nil { + return "", err + } + + name := uploader.DisplayName + if name == "" { + name = uploader.Username + } + u.names[userId] = name + return name, nil +} diff --git a/test/article_publishing_http_test.go b/test/article_publishing_http_test.go index 3d72ec9..458424b 100644 --- a/test/article_publishing_http_test.go +++ b/test/article_publishing_http_test.go @@ -26,7 +26,7 @@ func newArticlePublishingRouter(articleRepo *mockArticleRepo, token *jwtauth.JWT useCases.NewCreateArticleUseCase(articleRepo), useCases.NewGetArticleUseCase(articleRepo), useCases.NewListArticlesUseCase(articleRepo), - useCases.NewDeleteArticleUseCase(articleRepo, newMockImageRepo()), + useCases.NewDeleteArticleUseCase(articleRepo), useCases.NewUpdateArticleUseCase(articleRepo), getUserUseCase, ) diff --git a/test/article_test.go b/test/article_test.go index c040d2a..532cd0c 100644 --- a/test/article_test.go +++ b/test/article_test.go @@ -2,25 +2,24 @@ package test import ( "RenewCMS/internal/domain/article" - imageDomain "RenewCMS/internal/domain/image" "RenewCMS/internal/infrastructure/useCases" "errors" "sort" + "strings" "testing" ) type mockArticleRepo struct { - articles map[uint32]article.Article - images map[uint32]*imageDomain.Image - nextID uint32 - fail failSwitch - createCalled bool + articles map[uint32]article.Article + failUpdateBodyFor map[uint32]error + nextID uint32 + fail failSwitch + createCalled bool } func newMockArticleRepo() *mockArticleRepo { return &mockArticleRepo{ articles: make(map[uint32]article.Article), - images: make(map[uint32]*imageDomain.Image), } } @@ -95,6 +94,9 @@ func (m *mockArticleRepo) UpdateBody(id uint32, body string) (article.Article, e if err := m.fail.trigger("UpdateBody"); err != nil { return article.Article{}, err } + if err, ok := m.failUpdateBodyFor[id]; ok { + return article.Article{}, err + } a, ok := m.articles[id] if !ok { return article.Article{}, article.ErrArticleNotFound @@ -128,21 +130,18 @@ func (m *mockArticleRepo) Delete(id uint32) error { return nil } -func (m *mockArticleRepo) AddImage(articleId uint32, imageId uint32) error { - if err := m.fail.trigger("AddImage"); err != nil { - return err +func (m *mockArticleRepo) FindByBodyContaining(filename string) ([]article.Article, error) { + if err := m.fail.trigger("FindByBodyContaining"); err != nil { + return nil, err } - a, ok := m.articles[articleId] - if !ok { - return article.ErrArticleNotFound + result := make([]article.Article, 0) + for _, id := range m.sortedIds() { + a := m.articles[id] + if strings.Contains(a.Body, filename) { + result = append(result, a) + } } - img, ok := m.images[imageId] - if !ok { - return imageDomain.ErrImageNotFound - } - a.Images = append(a.Images, img) - m.articles[articleId] = a - return nil + return result, nil } var _ article.Repository = &mockArticleRepo{} diff --git a/test/article_title_rules_test.go b/test/article_title_rules_test.go index fd4b71b..534ab53 100644 --- a/test/article_title_rules_test.go +++ b/test/article_title_rules_test.go @@ -29,7 +29,7 @@ func newArticleTitleRulesRouter(repo *mockArticleRepo, token *jwtauth.JWTAuth) * useCases.NewCreateArticleUseCase(repo), useCases.NewGetArticleUseCase(repo), useCases.NewListArticlesUseCase(repo), - useCases.NewDeleteArticleUseCase(repo, newMockImageRepo()), + useCases.NewDeleteArticleUseCase(repo), useCases.NewUpdateArticleUseCase(repo), getUserUseCase, ) diff --git a/test/articles_list_http_test.go b/test/articles_list_http_test.go index 95998b9..5c579db 100644 --- a/test/articles_list_http_test.go +++ b/test/articles_list_http_test.go @@ -3,7 +3,6 @@ package test import ( "RenewCMS/api/handlers/article" domainArticle "RenewCMS/internal/domain/article" - domainImage "RenewCMS/internal/domain/image" "RenewCMS/internal/infrastructure/useCases" "encoding/json" "net/http" @@ -24,7 +23,7 @@ func newArticlesListRouter(articleRepo *mockArticleRepo, token *jwtauth.JWTAuth) useCases.NewCreateArticleUseCase(articleRepo), useCases.NewGetArticleUseCase(articleRepo), useCases.NewListArticlesUseCase(articleRepo), - useCases.NewDeleteArticleUseCase(articleRepo, newMockImageRepo()), + useCases.NewDeleteArticleUseCase(articleRepo), useCases.NewUpdateArticleUseCase(articleRepo), getUserUseCase, ) @@ -43,16 +42,9 @@ func TestArticlesListHttp(t *testing.T) { validator.InitValidator() token := jwtauth.New("HS256", []byte("test-secret"), nil) - t.Run("GET /v1/article/ returns each article's images array populated", func(t *testing.T) { + t.Run("GET /v1/article/ items carry no images field now that images belong to the library", func(t *testing.T) { repo := newMockArticleRepo() - repo.articles[4] = domainArticle.Article{ - ID: 4, - Title: "Winter checklist for timber windows", - Images: []*domainImage.Image{ - {ID: 10, ArticleID: 4}, - {ID: 11, ArticleID: 4}, - }, - } + repo.articles[4] = domainArticle.Article{ID: 4, Title: "Winter checklist for timber windows"} router := newArticlesListRouter(repo, token) req := httptest.NewRequest(http.MethodGet, "/v1/article/", nil) @@ -64,29 +56,15 @@ func TestArticlesListHttp(t *testing.T) { t.Fatalf("expected status 200, got %d", rec.Code) } - var got []domainArticle.Article + var got []map[string]any if err := json.Unmarshal(rec.Body.Bytes(), &got); err != nil { t.Fatalf("failed to decode response: %v", err) } - - var found *domainArticle.Article - for i := range got { - if got[i].ID == 4 { - found = &got[i] - } + if len(got) != 1 { + t.Fatalf("expected 1 article, got %d", len(got)) } - if found == nil { - t.Fatalf("expected article id 4 in response, got %v", got) - } - if len(found.Images) != 2 { - t.Fatalf("expected 2 images, got %d", len(found.Images)) - } - ids := map[uint32]bool{} - for _, img := range found.Images { - ids[img.ID] = true - } - if !ids[10] || !ids[11] { - t.Errorf("expected images 10 and 11, got %v", found.Images) + if _, present := got[0]["images"]; present { + t.Errorf("expected no images field, got %v", got[0]["images"]) } }) } diff --git a/test/delete_article_cascades_images_test.go b/test/delete_article_cascades_images_test.go deleted file mode 100644 index 127de76..0000000 --- a/test/delete_article_cascades_images_test.go +++ /dev/null @@ -1,230 +0,0 @@ -package test - -import ( - "RenewCMS/api/handlers/article" - "RenewCMS/api/middleware" - "RenewCMS/api/pkg/apperr" - validator "RenewCMS/api/pkg/validator" - domainArticle "RenewCMS/internal/domain/article" - domainImage "RenewCMS/internal/domain/image" - "RenewCMS/internal/infrastructure/useCases" - "errors" - "net/http" - "net/http/httptest" - "testing" - - "github.com/go-chi/chi/v5" - "github.com/go-chi/jwtauth/v5" -) - -func seedArticleImages(imageRepo *mockImageRepo, ids ...uint32) []*domainImage.Image { - images := make([]*domainImage.Image, len(ids)) - for i, id := range ids { - imageRepo.images[id] = domainImage.Image{ID: id} - seeded := imageRepo.images[id] - images[i] = &seeded - } - return images -} - -func TestDeleteArticleUseCaseCascadesImageDeletion(t *testing.T) { - t.Run("deletes every image before deleting the article, in Images slice order", func(t *testing.T) { - articleRepo := newMockArticleRepo() - imageRepo := newMockImageRepo() - articleRepo.articles[1] = domainArticle.Article{ID: 1, Images: seedArticleImages(imageRepo, 10, 11)} - uc := useCases.NewDeleteArticleUseCase(articleRepo, imageRepo) - - if err := uc.DeleteArticle(1); err != nil { - t.Fatalf("expected no error, got %v", err) - } - if got := imageRepo.deleteCalls; len(got) != 2 || got[0] != 10 || got[1] != 11 { - t.Errorf("expected image Delete to be called with [10 11] in order, got %v", got) - } - if _, exists := articleRepo.articles[1]; exists { - t.Errorf("expected article 1 to have been deleted") - } - }) - - t.Run("stops at the first image Delete failure and leaves the article untouched", func(t *testing.T) { - articleRepo := newMockArticleRepo() - imageRepo := newMockImageRepo() - articleRepo.articles[3] = domainArticle.Article{ID: 3, Images: seedArticleImages(imageRepo, 20, 21)} - imageRepo.failDeleteFor = map[uint32]error{20: errRepositoryFailure} - uc := useCases.NewDeleteArticleUseCase(articleRepo, imageRepo) - - err := uc.DeleteArticle(3) - if !errors.Is(err, errRepositoryFailure) { - t.Fatalf("expected errRepositoryFailure, got %v", err) - } - if got := imageRepo.deleteCalls; len(got) != 1 || got[0] != 20 { - t.Errorf("expected image Delete to be called only once with 20, got %v", got) - } - if _, exists := articleRepo.articles[3]; !exists { - t.Errorf("expected article 3 to still be present") - } - if _, exists := imageRepo.images[21]; !exists { - t.Errorf("expected image 21 to still be present") - } - }) - - t.Run("propagates ErrArticleNotFound without touching the image repository", func(t *testing.T) { - articleRepo := newMockArticleRepo() - imageRepo := newMockImageRepo() - uc := useCases.NewDeleteArticleUseCase(articleRepo, imageRepo) - - err := uc.DeleteArticle(9999) - if !errors.Is(err, domainArticle.ErrArticleNotFound) { - t.Fatalf("expected article.ErrArticleNotFound, got %v", err) - } - if len(imageRepo.deleteCalls) != 0 { - t.Errorf("expected no call to the image repository's Delete method, got %v", imageRepo.deleteCalls) - } - }) -} - -func newDeleteArticleCascadesImagesRouter(articleRepo *mockArticleRepo, imageRepo *mockImageRepo, token *jwtauth.JWTAuth) *chi.Mux { - userRepo := newVerifiedUserRepo() - getUserUseCase := useCases.NewGetUserUseCase(userRepo) - - articleHandler := article.NewHandler( - useCases.NewCreateArticleUseCase(articleRepo), - useCases.NewGetArticleUseCase(articleRepo), - useCases.NewListArticlesUseCase(articleRepo), - useCases.NewDeleteArticleUseCase(articleRepo, imageRepo), - useCases.NewUpdateArticleUseCase(articleRepo), - getUserUseCase, - ) - - r := chi.NewRouter() - r.Route("/v1", func(r chi.Router) { - r.Group(func(r chi.Router) { - r.Use(jwtauth.Verifier(token)) - r.Use(jwtauth.Authenticator(token)) - r.Use(middleware.RequireVerifiedUser(getUserUseCase)) - r.Route("/article", func(r chi.Router) { - r.Delete("/{id}", articleHandler.DeleteArticle) - }) - }) - }) - return r -} - -func TestDeleteArticleCascadesImages(t *testing.T) { - validator.InitValidator() - token := jwtauth.New("HS256", []byte("test-secret"), nil) - - t.Run("Deleting an article with images removes the article and every one of its images", func(t *testing.T) { - articleRepo := newMockArticleRepo() - imageRepo := newMockImageRepo() - articleRepo.articles[1] = domainArticle.Article{ID: 1, Images: seedArticleImages(imageRepo, 10, 11)} - - router := newDeleteArticleCascadesImagesRouter(articleRepo, imageRepo, token) - req := httptest.NewRequest(http.MethodDelete, "/v1/article/1", nil) - setJwtCookie(t, req, token, 1) - rec := httptest.NewRecorder() - router.ServeHTTP(rec, req) - - if rec.Code != http.StatusNoContent { - t.Fatalf("expected status 204, got %d", rec.Code) - } - if _, exists := articleRepo.articles[1]; exists { - t.Errorf("expected article 1 to have been deleted") - } - if _, exists := imageRepo.images[10]; exists { - t.Errorf("expected image 10 to have been deleted") - } - if _, exists := imageRepo.images[11]; exists { - t.Errorf("expected image 11 to have been deleted") - } - }) - - t.Run("Deleting an article with no images still succeeds", func(t *testing.T) { - articleRepo := newMockArticleRepo() - imageRepo := newMockImageRepo() - articleRepo.articles[2] = domainArticle.Article{ID: 2} - - router := newDeleteArticleCascadesImagesRouter(articleRepo, imageRepo, token) - req := httptest.NewRequest(http.MethodDelete, "/v1/article/2", nil) - setJwtCookie(t, req, token, 1) - rec := httptest.NewRecorder() - router.ServeHTTP(rec, req) - - if rec.Code != http.StatusNoContent { - t.Fatalf("expected status 204, got %d", rec.Code) - } - if _, exists := articleRepo.articles[2]; exists { - t.Errorf("expected article 2 to have been deleted") - } - if len(imageRepo.deleteCalls) != 0 { - t.Errorf("expected no call to the image repository's Delete method, got %v", imageRepo.deleteCalls) - } - }) - - t.Run("Deleting an unknown article still answers 404 and deletes no images", func(t *testing.T) { - articleRepo := newMockArticleRepo() - imageRepo := newMockImageRepo() - - router := newDeleteArticleCascadesImagesRouter(articleRepo, imageRepo, token) - req := httptest.NewRequest(http.MethodDelete, "/v1/article/9999", nil) - setJwtCookie(t, req, token, 1) - rec := httptest.NewRecorder() - router.ServeHTTP(rec, req) - - if rec.Code != http.StatusNotFound { - t.Fatalf("expected status 404, got %d", rec.Code) - } - if rec.Body.String() != apperr.ArticleNotFoundErrorMessage+"\n" { - t.Errorf("unexpected body: %q", rec.Body.String()) - } - if len(imageRepo.deleteCalls) != 0 { - t.Errorf("expected no call to the image repository's Delete method, got %v", imageRepo.deleteCalls) - } - }) - - t.Run("Deleting an article still requires authentication", func(t *testing.T) { - articleRepo := newMockArticleRepo() - imageRepo := newMockImageRepo() - articleRepo.articles[1] = domainArticle.Article{ID: 1, Images: seedArticleImages(imageRepo, 10)} - - router := newDeleteArticleCascadesImagesRouter(articleRepo, imageRepo, token) - req := httptest.NewRequest(http.MethodDelete, "/v1/article/1", nil) - rec := httptest.NewRecorder() - router.ServeHTTP(rec, req) - - if rec.Code != http.StatusUnauthorized { - t.Fatalf("expected status 401, got %d", rec.Code) - } - if _, exists := articleRepo.articles[1]; !exists { - t.Errorf("expected article 1 to still be present") - } - if _, exists := imageRepo.images[10]; !exists { - t.Errorf("expected image 10 to still be present") - } - }) - - t.Run("A failure deleting one of an article's images aborts the whole delete", func(t *testing.T) { - articleRepo := newMockArticleRepo() - imageRepo := newMockImageRepo() - articleRepo.articles[3] = domainArticle.Article{ID: 3, Images: seedArticleImages(imageRepo, 20, 21)} - imageRepo.failDeleteFor = map[uint32]error{20: errRepositoryFailure} - - router := newDeleteArticleCascadesImagesRouter(articleRepo, imageRepo, token) - req := httptest.NewRequest(http.MethodDelete, "/v1/article/3", nil) - setJwtCookie(t, req, token, 1) - rec := httptest.NewRecorder() - router.ServeHTTP(rec, req) - - if rec.Code != http.StatusInternalServerError { - t.Fatalf("expected status 500, got %d", rec.Code) - } - if rec.Body.String() != "Failed to delete article\n" { - t.Errorf("unexpected body: %q", rec.Body.String()) - } - if _, exists := articleRepo.articles[3]; !exists { - t.Errorf("expected article 3 to still be present") - } - if _, exists := imageRepo.images[21]; !exists { - t.Errorf("expected image 21 to still be present") - } - }) -} diff --git a/test/delete_article_keeps_images_test.go b/test/delete_article_keeps_images_test.go new file mode 100644 index 0000000..c6e61a7 --- /dev/null +++ b/test/delete_article_keeps_images_test.go @@ -0,0 +1,84 @@ +package test + +import ( + "RenewCMS/api/pkg/apperr" + "net/http" + "reflect" + "testing" +) + +func TestDeleteArticleKeepsImages(t *testing.T) { + newEnv := func(t *testing.T) *mediaEnv { + env := newMediaEnv(t) + env.seedImage(10, "ten.png", 1, "image/png", 1, 1, hashA, 1) + env.seedImage(11, "eleven.png", 1, "image/png", 1, 1, hashB, 1) + env.seedArticle(1, "Kitchen Renovation", true, env.urlOf(10)+" "+env.urlOf(11)) + return env + } + + t.Run("Deleting an article leaves every image it used", func(t *testing.T) { + env := newEnv(t) + + rec := env.send(t, http.MethodDelete, "/v1/article/1", "", mediaVerifiedUserId) + + if rec.Code != http.StatusNoContent { + t.Fatalf("expected status 204, got %d", rec.Code) + } + if _, ok := env.articles.articles[1]; ok { + t.Errorf("expected article 1 to be gone") + } + expectIds(t, env.imageIds(), []uint32{10, 11}) + if len(env.images.deleteCalls) != 0 { + t.Errorf("expected image Delete to never be called, got %v", env.images.deleteCalls) + } + }) + + t.Run("Deleting an unknown article is 404 and touches no image", func(t *testing.T) { + env := newEnv(t) + + rec := env.send(t, http.MethodDelete, "/v1/article/9999", "", mediaVerifiedUserId) + + if rec.Code != http.StatusNotFound || rec.Body.String() != apperr.ArticleNotFoundErrorMessage+"\n" { + t.Errorf("expected 404 Article not found, got %d %q", rec.Code, rec.Body.String()) + } + expectIds(t, env.imageIds(), []uint32{10, 11}) + if len(env.images.deleteCalls) != 0 { + t.Errorf("expected the image repository to be untouched") + } + }) + + t.Run("Deleting still requires authentication", func(t *testing.T) { + env := newEnv(t) + + rec := env.send(t, http.MethodDelete, "/v1/article/1", "", 0) + + if rec.Code != http.StatusUnauthorized { + t.Fatalf("expected status 401, got %d", rec.Code) + } + if _, ok := env.articles.articles[1]; !ok { + t.Errorf("expected article 1 to still exist") + } + if _, ok := env.images.images[10]; !ok { + t.Errorf("expected image 10 to still exist") + } + }) + + t.Run("An article repository failure is 500 and everything stays", func(t *testing.T) { + env := newEnv(t) + bodiesBefore := env.bodies() + env.articles.fail.failNext("Delete") + + rec := env.send(t, http.MethodDelete, "/v1/article/1", "", mediaVerifiedUserId) + + if rec.Code != http.StatusInternalServerError || rec.Body.String() != "Failed to delete article\n" { + t.Errorf("expected 500 Failed to delete article, got %d %q", rec.Code, rec.Body.String()) + } + if !reflect.DeepEqual(env.bodies(), bodiesBefore) { + t.Errorf("expected the article to stay") + } + expectIds(t, env.imageIds(), []uint32{10, 11}) + if len(env.images.deleteCalls) != 0 { + t.Errorf("expected the image repository to be untouched") + } + }) +} diff --git a/test/edit_article_body_http_test.go b/test/edit_article_body_http_test.go index 35ca079..e4af86b 100644 --- a/test/edit_article_body_http_test.go +++ b/test/edit_article_body_http_test.go @@ -27,7 +27,7 @@ func newEditArticleBodyRouter(articleRepo *mockArticleRepo, token *jwtauth.JWTAu useCases.NewCreateArticleUseCase(articleRepo), useCases.NewGetArticleUseCase(articleRepo), useCases.NewListArticlesUseCase(articleRepo), - useCases.NewDeleteArticleUseCase(articleRepo, newMockImageRepo()), + useCases.NewDeleteArticleUseCase(articleRepo), useCases.NewUpdateArticleUseCase(articleRepo), getUserUseCase, ) diff --git a/test/image_test.go b/test/image_test.go index 8703147..ffd8c11 100644 --- a/test/image_test.go +++ b/test/image_test.go @@ -3,36 +3,86 @@ package test import ( imageDomain "RenewCMS/internal/domain/image" "RenewCMS/internal/infrastructure/useCases" + "errors" "io" + "sort" + "strconv" "strings" "testing" + "time" ) +var mockImageEpoch = time.Date(2026, time.June, 1, 12, 0, 0, 0, time.UTC) + +var mockImageExtensions = map[string]string{ + "image/png": ".png", + "image/jpeg": ".jpeg", + "image/webp": ".webp", + "image/svg+xml": ".svg", +} + type mockImageRepo struct { images map[uint32]imageDomain.Image nextID uint32 fail failSwitch failDeleteFor map[uint32]error deleteCalls []uint32 + createCalls int + lastContent []byte } func newMockImageRepo() *mockImageRepo { return &mockImageRepo{images: make(map[uint32]imageDomain.Image)} } +func (m *mockImageRepo) sortedImages() []imageDomain.Image { + result := make([]imageDomain.Image, 0, len(m.images)) + for _, img := range m.images { + result = append(result, img) + } + sort.Slice(result, func(i, j int) bool { return result[i].ID < result[j].ID }) + return result +} + func (m *mockImageRepo) Create(input imageDomain.ImageInput) (imageDomain.Image, error) { + m.createCalls++ if err := m.fail.trigger("Create"); err != nil { return imageDomain.Image{}, err } if input.Content != nil { - _, _ = io.ReadAll(input.Content) + m.lastContent, _ = io.ReadAll(input.Content) + } + for m.images[m.nextID+1].ID != 0 { + m.nextID++ } m.nextID++ - img := imageDomain.Image{ID: m.nextID, Path: "/static/uploadedImages/" + input.Filename} + img := imageDomain.Image{ + ID: m.nextID, + Path: "/static/uploadedImages/" + strconv.Itoa(int(m.nextID)) + mockImageExtensions[input.ContentType], + Filename: input.Filename, + Size: input.Size, + ContentType: input.ContentType, + Width: input.Width, + Height: input.Height, + SHA256: input.SHA256, + UploadedByID: input.UploadedByID, + CreatedAt: mockImageEpoch.Add(time.Duration(m.nextID) * time.Minute), + } m.images[img.ID] = img return img, nil } +func (m *mockImageRepo) Get(id uint32) (imageDomain.Image, error) { + if err := m.fail.trigger("Get"); err != nil { + return imageDomain.Image{}, err + } + img, ok := m.images[id] + if !ok { + return imageDomain.Image{}, imageDomain.ErrImageNotFound + } + return img, nil +} + func (m *mockImageRepo) Delete(id uint32) error { m.deleteCalls = append(m.deleteCalls, id) if err := m.fail.trigger("Delete"); err != nil { @@ -48,16 +98,96 @@ func (m *mockImageRepo) Delete(id uint32) error { return nil } +func (m *mockImageRepo) List(query imageDomain.SearchQuery) ([]imageDomain.Image, error) { + if err := m.fail.trigger("List"); err != nil { + return nil, err + } + result := make([]imageDomain.Image, 0) + for _, img := range m.sortedImages() { + if query.Search != "" && !strings.Contains(strings.ToLower(img.Filename), strings.ToLower(query.Search)) { + continue + } + if query.Hash != "" && img.SHA256 != query.Hash { + continue + } + result = append(result, img) + } + return result, nil +} + +func (m *mockImageRepo) FindByHash(hash string) ([]imageDomain.Image, error) { + if err := m.fail.trigger("FindByHash"); err != nil { + return nil, err + } + result := make([]imageDomain.Image, 0) + for _, img := range m.sortedImages() { + if hash != "" && img.SHA256 == hash { + result = append(result, img) + } + } + return result, nil +} + +func (m *mockImageRepo) Rename(id uint32, filename string) (imageDomain.Image, error) { + if err := m.fail.trigger("Rename"); err != nil { + return imageDomain.Image{}, err + } + img, ok := m.images[id] + if !ok { + return imageDomain.Image{}, imageDomain.ErrImageNotFound + } + img.Filename = filename + m.images[id] = img + return img, nil +} + +func (m *mockImageRepo) hashCounts() map[string]int { + counts := map[string]int{} + for _, img := range m.images { + if img.SHA256 != "" { + counts[img.SHA256]++ + } + } + return counts +} + +func (m *mockImageRepo) ListDuplicates() ([]imageDomain.Image, error) { + if err := m.fail.trigger("ListDuplicates"); err != nil { + return nil, err + } + counts := m.hashCounts() + result := make([]imageDomain.Image, 0) + for _, img := range m.sortedImages() { + if img.SHA256 != "" && counts[img.SHA256] > 1 { + result = append(result, img) + } + } + return result, nil +} + +func (m *mockImageRepo) Counts() (imageDomain.Counts, error) { + if err := m.fail.trigger("Counts"); err != nil { + return imageDomain.Counts{}, err + } + groups := 0 + for _, count := range m.hashCounts() { + if count > 1 { + groups++ + } + } + return imageDomain.Counts{All: len(m.images), DuplicateGroups: groups}, nil +} + var _ imageDomain.Repository = &mockImageRepo{} func TestImageUseCases(t *testing.T) { repo := newMockImageRepo() createUc := useCases.NewCreateImageUseCase(repo) - deleteUc := useCases.NewDeleteImageUseCase(repo) + deleteUc := useCases.NewDeleteMediaUseCase(repo) t.Run("Create Image", func(t *testing.T) { input := imageDomain.ImageInput{ - Content: strings.NewReader("fake-image-data"), + Content: strings.NewReader(string(pngBytes(t, 3, 2))), Filename: "test.png", ContentType: "image/png", } @@ -69,21 +199,27 @@ func TestImageUseCases(t *testing.T) { if res.ID == 0 { t.Errorf("Expected valid ID, got 0") } - if !strings.Contains(res.Path, "test.png") { - t.Errorf("Path mismatch: %s", res.Path) + if res.Filename != "test.png" { + t.Errorf("Filename mismatch: %s", res.Filename) } }) t.Run("Delete Image", func(t *testing.T) { repo.images[99] = imageDomain.Image{ID: 99, Path: "/static/delete-me.png"} - err := deleteUc.DeleteImage(99) + err := deleteUc.Delete(99) if err != nil { - t.Fatalf("DeleteImage failed: %v", err) + t.Fatalf("Delete failed: %v", err) } if _, exists := repo.images[99]; exists { t.Errorf("Image 99 should have been deleted") } }) + + t.Run("Delete unknown image", func(t *testing.T) { + if err := deleteUc.Delete(12345); !errors.Is(err, imageDomain.ErrImageNotFound) { + t.Errorf("expected ErrImageNotFound, got %v", err) + } + }) } diff --git a/test/image_upload_rules_test.go b/test/image_upload_rules_test.go index bda7220..2df62dc 100644 --- a/test/image_upload_rules_test.go +++ b/test/image_upload_rules_test.go @@ -3,10 +3,37 @@ package test import ( imageDomain "RenewCMS/internal/domain/image" "RenewCMS/internal/infrastructure/useCases" + "bytes" "errors" "testing" ) +func validBytesFor(t *testing.T, contentType string) []byte { + t.Helper() + switch contentType { + case "image/png": + return pngBytes(t, 3, 2) + case "image/jpeg": + return jpegBytes(t, 3, 2) + case "image/webp": + return webpBytes(t) + default: + return []byte(svgFixture) + } +} + +func paddedTo(data []byte, size int) []byte { + padding := byte(0) + if bytes.HasPrefix(data, []byte(" e.images.nextID { + e.images.nextID = id + } +} + +func (e *mediaEnv) urlOf(id uint32) string { + return e.images.images[id].Path +} + +func (e *mediaEnv) seedArticle(id uint32, title string, online bool, body string) { + e.articles.articles[id] = domainArticle.Article{ID: id, Title: title, IsOnline: online, Body: body} + if id > e.articles.nextID { + e.articles.nextID = id + } +} + +func (e *mediaEnv) bodies() map[uint32]string { + result := map[uint32]string{} + for id, a := range e.articles.articles { + result[id] = a.Body + } + return result +} + +func (e *mediaEnv) imageIds() []uint32 { + ids := make([]uint32, 0) + for _, img := range e.images.sortedImages() { + ids = append(ids, img.ID) + } + return ids +} + +func decodeObject(t *testing.T, body string) map[string]any { + t.Helper() + var decoded map[string]any + if err := json.Unmarshal([]byte(body), &decoded); err != nil { + t.Fatalf("failed to decode %q: %v", body, err) + } + return decoded +} + +func numberOf(value any) uint32 { + number, _ := value.(float64) + return uint32(number) +} diff --git a/test/media_fixtures_test.go b/test/media_fixtures_test.go new file mode 100644 index 0000000..af1075b --- /dev/null +++ b/test/media_fixtures_test.go @@ -0,0 +1,103 @@ +package test + +import ( + "bytes" + "crypto/sha256" + "encoding/base64" + "encoding/hex" + "image" + "image/color" + "image/jpeg" + "image/png" + "io" + "mime/multipart" + "net/http" + "net/http/httptest" + "testing" +) + +const webpFixtureBase64 = "UklGRhoAAABXRUJQVlA4TA0AAAAvAAAAEAcQERGIiP4HAA==" + +const svgFixture = `` + +func solidImage(width int, height int) image.Image { + canvas := image.NewRGBA(image.Rect(0, 0, width, height)) + for x := 0; x < width; x++ { + for y := 0; y < height; y++ { + canvas.Set(x, y, color.RGBA{R: uint8(x * 40), G: uint8(y * 40), B: 200, A: 255}) + } + } + return canvas +} + +func pngBytes(t *testing.T, width int, height int) []byte { + t.Helper() + buffer := &bytes.Buffer{} + if err := png.Encode(buffer, solidImage(width, height)); err != nil { + t.Fatalf("failed to encode png: %v", err) + } + return buffer.Bytes() +} + +func jpegBytes(t *testing.T, width int, height int) []byte { + t.Helper() + buffer := &bytes.Buffer{} + if err := jpeg.Encode(buffer, solidImage(width, height), nil); err != nil { + t.Fatalf("failed to encode jpeg: %v", err) + } + return buffer.Bytes() +} + +func webpBytes(t *testing.T) []byte { + t.Helper() + data, err := base64.StdEncoding.DecodeString(webpFixtureBase64) + if err != nil { + t.Fatalf("failed to decode webp fixture: %v", err) + } + return data +} + +func paddedPngBytes(t *testing.T, size int) []byte { + t.Helper() + data := pngBytes(t, 3, 2) + return append(data, bytes.Repeat([]byte{0}, size-len(data))...) +} + +func sha256Hex(data []byte) string { + sum := sha256.Sum256(data) + return hex.EncodeToString(sum[:]) +} + +func newMultipartFileRequest(t *testing.T, method string, url string, filename string, contentType string, content []byte) *http.Request { + t.Helper() + body := &bytes.Buffer{} + writer := multipart.NewWriter(body) + part, err := writer.CreatePart(map[string][]string{ + "Content-Disposition": {`form-data; name="file"; filename="` + filename + `"`}, + "Content-Type": {contentType}, + }) + if err != nil { + t.Fatalf("failed to create multipart part: %v", err) + } + if _, err := io.Copy(part, bytes.NewReader(content)); err != nil { + t.Fatalf("failed to write file content: %v", err) + } + if err := writer.Close(); err != nil { + t.Fatalf("failed to close multipart writer: %v", err) + } + req := httptest.NewRequest(method, url, body) + req.Header.Set("Content-Type", writer.FormDataContentType()) + return req +} + +func newMultipartWithoutFileRequest(t *testing.T, url string) *http.Request { + t.Helper() + body := &bytes.Buffer{} + writer := multipart.NewWriter(body) + if err := writer.Close(); err != nil { + t.Fatalf("failed to close multipart writer: %v", err) + } + req := httptest.NewRequest(http.MethodPost, url, body) + req.Header.Set("Content-Type", writer.FormDataContentType()) + return req +} diff --git a/test/media_library_http_test.go b/test/media_library_http_test.go new file mode 100644 index 0000000..548bb48 --- /dev/null +++ b/test/media_library_http_test.go @@ -0,0 +1,1073 @@ +package test + +import ( + "RenewCMS/api/pkg/apperr" + domainImage "RenewCMS/internal/domain/image" + "RenewCMS/internal/infrastructure/useCases" + "encoding/json" + "errors" + "net/http" + "net/url" + "os/exec" + "reflect" + "regexp" + "sort" + "strings" + "testing" +) + +var ( + hashA = sha256Hex([]byte("A")) + hashB = sha256Hex([]byte("B")) + hashC = sha256Hex([]byte("C")) + hashD = sha256Hex([]byte("D")) +) + +func seedListingLibrary(env *mediaEnv) { + env.seedImage(1, "north-wall.jpg", 4089446, "image/jpeg", 3600, 2400, hashA, 1) + env.seedImage(2, "floorboards.jpg", 2516582, "image/jpeg", 3600, 2400, hashB, 1) + env.seedImage(3, "floorboards-copy.jpg", 2516582, "image/jpeg", 3600, 2400, hashB, 2) + env.seedImage(4, "window-frame.jpg", 812345, "image/jpeg", 1200, 800, hashC, 2) + env.seedImage(5, "logo.svg", 1204, "image/svg+xml", 0, 0, hashD, 1) + env.seedArticle(1, "Kitchen Renovation", true, ``) + env.seedArticle(2, "Backyard Notes", false, ``) + env.seedArticle(3, "Empty", true, "no image here") +} + +type listBody struct { + Items []map[string]any `json:"items"` + NextCursor *string `json:"next_cursor"` + Counts map[string]int `json:"counts"` +} + +func (e *mediaEnv) list(t *testing.T, query string) (listBody, string) { + t.Helper() + rec := e.send(t, http.MethodGet, "/v1/media/"+query, "", mediaVerifiedUserId) + if rec.Code != http.StatusOK { + t.Fatalf("expected status 200, got %d, body %q", rec.Code, rec.Body.String()) + } + var decoded listBody + if err := json.Unmarshal(rec.Body.Bytes(), &decoded); err != nil { + t.Fatalf("failed to decode list: %v", err) + } + return decoded, rec.Body.String() +} + +func idsOf(items []map[string]any) []uint32 { + ids := make([]uint32, 0, len(items)) + for _, item := range items { + ids = append(ids, numberOf(item["id"])) + } + return ids +} + +func sortedIdsOf(items []map[string]any) []uint32 { + ids := idsOf(items) + sort.Slice(ids, func(i, j int) bool { return ids[i] < ids[j] }) + return ids +} + +func itemById(t *testing.T, items []map[string]any, id uint32) map[string]any { + t.Helper() + for _, item := range items { + if numberOf(item["id"]) == id { + return item + } + } + t.Fatalf("expected item %d in %v", id, items) + return nil +} + +func expectIds(t *testing.T, got []uint32, want []uint32) { + t.Helper() + if !reflect.DeepEqual(got, want) { + t.Errorf("expected ids %v, got %v", want, got) + } +} + +func TestMediaAuthentication(t *testing.T) { + examples := []struct { + method string + path string + userId uint32 + description string + status int + message string + }{ + {http.MethodGet, "/v1/media/", 0, "no cookie", 401, ""}, + {http.MethodPost, "/v1/media/", 0, "no cookie", 401, ""}, + {http.MethodPatch, "/v1/media/1", 0, "no cookie", 401, ""}, + {http.MethodDelete, "/v1/media/1", 0, "no cookie", 401, ""}, + {http.MethodPost, "/v1/media/merge", 0, "no cookie", 401, ""}, + {http.MethodGet, "/v1/media/1/usage", 0, "no cookie", 401, ""}, + {http.MethodGet, "/v1/media/", mediaStaleUserId, "a stale token_version cookie", 401, apperr.UnauthorizedErrorMessage}, + {http.MethodGet, "/v1/media/", mediaUnverifiedUserId, "cookie of an unverified user", 403, apperr.VerificationRequiredErrorMessage}, + {http.MethodDelete, "/v1/media/1", mediaUnverifiedUserId, "cookie of an unverified user", 403, apperr.VerificationRequiredErrorMessage}, + } + + for _, example := range examples { + t.Run(example.method+" "+example.path+" "+example.description, func(t *testing.T) { + env := newMediaEnv(t) + seedListingLibrary(env) + bodiesBefore := env.bodies() + imagesBefore := env.imageIds() + + rec := env.send(t, example.method, example.path, "", example.userId) + + if rec.Code != example.status { + t.Fatalf("expected status %d, got %d", example.status, rec.Code) + } + if example.message != "" && rec.Body.String() != example.message+"\n" { + t.Errorf("unexpected body %q", rec.Body.String()) + } + if !reflect.DeepEqual(env.bodies(), bodiesBefore) || !reflect.DeepEqual(env.imageIds(), imagesBefore) { + t.Errorf("expected no image or article to change") + } + if len(env.images.deleteCalls) != 0 || env.images.createCalls != 0 { + t.Errorf("expected no image write") + } + }) + } +} + +func TestMediaListing(t *testing.T) { + t.Run("The default list is newest first with used_in, duplicate and tab counts", func(t *testing.T) { + env := newMediaEnv(t) + seedListingLibrary(env) + + got, raw := env.list(t, "") + + expectIds(t, idsOf(got.Items), []uint32{5, 4, 3, 2, 1}) + first := itemById(t, got.Items, 1) + if numberOf(first["used_in"]) != 2 || first["duplicate"] != false || first["uploaded_by"] != "Inès Duarte" { + t.Errorf("unexpected item 1: %v", first) + } + if itemById(t, got.Items, 2)["duplicate"] != true || itemById(t, got.Items, 3)["duplicate"] != true { + t.Errorf("expected items 2 and 3 to be duplicates") + } + if itemById(t, got.Items, 3)["uploaded_by"] != "marta" { + t.Errorf("expected item 3 uploaded_by marta, got %v", itemById(t, got.Items, 3)["uploaded_by"]) + } + logo := itemById(t, got.Items, 5) + if logo["width"] != nil || logo["height"] != nil || numberOf(logo["used_in"]) != 0 { + t.Errorf("unexpected item 5: %v", logo) + } + if !strings.Contains(raw, `"width":null`) || !strings.Contains(raw, `"height":null`) { + t.Errorf("expected null width and height in the raw body, got %s", raw) + } + if got.NextCursor != nil { + t.Errorf("expected next_cursor null, got %v", *got.NextCursor) + } + if !strings.Contains(raw, `"next_cursor":null`) { + t.Errorf("expected next_cursor null in the raw body") + } + if !reflect.DeepEqual(got.Counts, map[string]int{"all": 5, "unused": 3, "duplicates": 1}) { + t.Errorf("unexpected counts %v", got.Counts) + } + if first["url"] != env.urlOf(1) || first["sha256"] != hashA || first["content_type"] != "image/jpeg" || numberOf(first["size"]) != 4089446 { + t.Errorf("unexpected metadata on item 1: %v", first) + } + }) + + t.Run("An empty library answers an empty list, not null", func(t *testing.T) { + env := newMediaEnv(t) + + got, raw := env.list(t, "") + + if !strings.Contains(raw, `"items":[]`) { + t.Errorf("expected an empty items array, got %s", raw) + } + if !reflect.DeepEqual(got.Counts, map[string]int{"all": 0, "unused": 0, "duplicates": 0}) { + t.Errorf("unexpected counts %v", got.Counts) + } + }) + + t.Run("sort orders the list", func(t *testing.T) { + examples := map[string][]uint32{ + "newest": {5, 4, 3, 2, 1}, + "oldest": {1, 2, 3, 4, 5}, + "name": {2, 3, 5, 1, 4}, + "largest": {1, 2, 3, 4, 5}, + } + for sortName, want := range examples { + t.Run(sortName, func(t *testing.T) { + env := newMediaEnv(t) + seedListingLibrary(env) + + got, _ := env.list(t, "?sort="+sortName) + + expectIds(t, idsOf(got.Items), want) + }) + } + }) + + t.Run("filter narrows the list and the counts stay library-wide", func(t *testing.T) { + examples := map[string][]uint32{ + "all": {1, 2, 3, 4, 5}, + "unused": {3, 4, 5}, + "duplicates": {2, 3}, + } + for filter, want := range examples { + t.Run(filter, func(t *testing.T) { + env := newMediaEnv(t) + seedListingLibrary(env) + + got, _ := env.list(t, "?filter="+filter) + + expectIds(t, sortedIdsOf(got.Items), want) + if !reflect.DeepEqual(got.Counts, map[string]int{"all": 5, "unused": 3, "duplicates": 1}) { + t.Errorf("unexpected counts %v", got.Counts) + } + }) + } + }) + + t.Run("filter keeps the newest first order", func(t *testing.T) { + examples := map[string][]uint32{ + "all": {5, 4, 3, 2, 1}, + "unused": {5, 4, 3}, + "duplicates": {3, 2}, + } + for filter, want := range examples { + t.Run(filter, func(t *testing.T) { + env := newMediaEnv(t) + seedListingLibrary(env) + + got, _ := env.list(t, "?filter="+filter) + + expectIds(t, idsOf(got.Items), want) + }) + } + }) + + t.Run("q matches the filename case-insensitively", func(t *testing.T) { + examples := []struct { + q string + want []uint32 + }{ + {"FLOORBOARD", []uint32{3, 2}}, + {"wall", []uint32{1}}, + {"zzz", []uint32{}}, + } + for _, example := range examples { + t.Run(example.q, func(t *testing.T) { + env := newMediaEnv(t) + seedListingLibrary(env) + + got, _ := env.list(t, "?q="+example.q) + + expectIds(t, idsOf(got.Items), example.want) + if !reflect.DeepEqual(got.Counts, map[string]int{"all": 5, "unused": 3, "duplicates": 1}) { + t.Errorf("unexpected counts %v", got.Counts) + } + }) + } + }) + + t.Run("q, filter and sort combine", func(t *testing.T) { + env := newMediaEnv(t) + seedListingLibrary(env) + + got, _ := env.list(t, "?q=floorboards&filter=unused&sort=oldest") + + expectIds(t, idsOf(got.Items), []uint32{3}) + }) + + t.Run("hash returns only the images with that content", func(t *testing.T) { + env := newMediaEnv(t) + seedListingLibrary(env) + + got, _ := env.list(t, "?hash="+hashB) + + expectIds(t, sortedIdsOf(got.Items), []uint32{2, 3}) + }) + + t.Run("hash of unknown content returns no item", func(t *testing.T) { + env := newMediaEnv(t) + seedListingLibrary(env) + + got, _ := env.list(t, "?hash="+strings.Repeat("0", 64)) + + if len(got.Items) != 0 { + t.Errorf("expected no item, got %v", got.Items) + } + }) + + t.Run("limit and cursor page through the list", func(t *testing.T) { + env := newMediaEnv(t) + seedListingLibrary(env) + + first, _ := env.list(t, "?limit=2") + expectIds(t, idsOf(first.Items), []uint32{5, 4}) + if first.NextCursor == nil || *first.NextCursor == "" { + t.Fatalf("expected a next_cursor on page 1") + } + + second, _ := env.list(t, "?limit=2&cursor="+url.QueryEscape(*first.NextCursor)) + expectIds(t, idsOf(second.Items), []uint32{3, 2}) + if second.NextCursor == nil || *second.NextCursor == "" { + t.Fatalf("expected a next_cursor on page 2") + } + + third, _ := env.list(t, "?limit=2&cursor="+url.QueryEscape(*second.NextCursor)) + expectIds(t, idsOf(third.Items), []uint32{1}) + if third.NextCursor != nil { + t.Errorf("expected next_cursor null on the last page, got %v", *third.NextCursor) + } + }) + + t.Run("A malformed query answers 400", func(t *testing.T) { + for _, query := range []string{"filter=bogus", "sort=bogus", "limit=0", "limit=101", "limit=abc", "cursor=%%%", "cursor=bogus", "hash=notahash"} { + t.Run(query, func(t *testing.T) { + env := newMediaEnv(t) + seedListingLibrary(env) + + rec := env.send(t, http.MethodGet, "/v1/media/?"+query, "", mediaVerifiedUserId) + + if rec.Code != http.StatusBadRequest { + t.Fatalf("expected status 400, got %d", rec.Code) + } + if rec.Body.String() != apperr.MediaQueryErrorMessage+"\n" { + t.Errorf("unexpected body %q", rec.Body.String()) + } + }) + } + }) + + t.Run("A row without metadata still lists", func(t *testing.T) { + env := newMediaEnv(t) + env.images.images[1] = domainImage.Image{ID: 1, Path: "/static/uploadedImages/legacy.png"} + + got, raw := env.list(t, "") + + item := itemById(t, got.Items, 1) + if numberOf(item["size"]) != 0 || item["width"] != nil || item["height"] != nil || item["sha256"] != "" || item["duplicate"] != false || item["uploaded_by"] != "" { + t.Errorf("unexpected item %v in %s", item, raw) + } + }) + + t.Run("A repository failure is a generic 500", func(t *testing.T) { + env := newMediaEnv(t) + env.images.fail.failNext("List") + + rec := env.send(t, http.MethodGet, "/v1/media/", "", mediaVerifiedUserId) + + if rec.Code != http.StatusInternalServerError || rec.Body.String() != "Internal server error\n" { + t.Errorf("expected generic 500, got %d %q", rec.Code, rec.Body.String()) + } + }) +} + +func TestMediaUploading(t *testing.T) { + allowed := []struct { + name string + contentType string + content func(t *testing.T) []byte + width any + height any + }{ + {"north-wall.png", "image/png", func(t *testing.T) []byte { return pngBytes(t, 3, 2) }, 3, 2}, + {"IMG_2208.jpg", "image/jpeg", func(t *testing.T) []byte { return jpegBytes(t, 3, 2) }, 3, 2}, + {"hero.webp", "image/webp", webpBytes, 1, 1}, + {"logo.svg", "image/svg+xml", func(t *testing.T) []byte { return []byte(svgFixture) }, nil, nil}, + } + + t.Run("An allowed image is always stored, with its metadata and hash", func(t *testing.T) { + urlPattern := regexp.MustCompile(`^/static/uploadedImages/[^/]+\.(png|jpeg|jpg|webp|svg)$`) + for _, example := range allowed { + t.Run(example.name, func(t *testing.T) { + env := newMediaEnv(t) + content := example.content(t) + + req := newMultipartFileRequest(t, http.MethodPost, "/v1/media/", example.name, example.contentType, content) + rec := env.serve(t, req, mediaVerifiedUserId) + + if rec.Code != http.StatusCreated { + t.Fatalf("expected status 201, got %d, body %q", rec.Code, rec.Body.String()) + } + item := decodeObject(t, rec.Body.String()) + if item["filename"] != example.name || item["content_type"] != example.contentType { + t.Errorf("unexpected filename or type: %v", item) + } + if int(numberOf(item["size"])) != len(content) { + t.Errorf("expected size %d, got %v", len(content), item["size"]) + } + expectedWidth, expectedHeight := example.width, example.height + if expectedWidth != nil { + expectedWidth, expectedHeight = float64(expectedWidth.(int)), float64(expectedHeight.(int)) + } + if item["width"] != expectedWidth || item["height"] != expectedHeight { + t.Errorf("expected %vx%v, got %vx%v", expectedWidth, expectedHeight, item["width"], item["height"]) + } + if item["sha256"] != sha256Hex(content) { + t.Errorf("unexpected sha256 %v", item["sha256"]) + } + if item["uploaded_by"] != "Inès Duarte" || numberOf(item["used_in"]) != 0 || item["duplicate"] != false { + t.Errorf("unexpected item %v", item) + } + if !urlPattern.MatchString(item["url"].(string)) { + t.Errorf("unexpected url %v", item["url"]) + } + if len(env.images.images) != 1 || env.images.images[1].UploadedByID != mediaVerifiedUserId { + t.Errorf("expected one new image with UploadedByID 1, got %+v", env.images.images) + } + if !reflect.DeepEqual(env.images.lastContent, content) { + t.Errorf("expected the repository to receive the uploaded bytes") + } + }) + } + }) + + t.Run("Uploading the same bytes twice stores both and marks both as duplicates", func(t *testing.T) { + env := newMediaEnv(t) + content := pngBytes(t, 3, 2) + env.seedImage(1, "north-wall.png", int64(len(content)), "image/png", 3, 2, sha256Hex(content), 1) + + req := newMultipartFileRequest(t, http.MethodPost, "/v1/media/", "north-wall-again.png", "image/png", content) + rec := env.serve(t, req, mediaVerifiedUserId) + + if rec.Code != http.StatusCreated || decodeObject(t, rec.Body.String())["duplicate"] != true { + t.Fatalf("expected 201 with duplicate true, got %d %q", rec.Code, rec.Body.String()) + } + if len(env.images.images) != 2 || env.images.images[1].Path == env.images.images[2].Path { + t.Errorf("expected two images with different urls, got %+v", env.images.images) + } + got, _ := env.list(t, "") + if got.Counts["duplicates"] != 1 { + t.Errorf("expected duplicates 1, got %v", got.Counts) + } + }) + + t.Run("A filename without extension gets the canonical one, a path is stripped", func(t *testing.T) { + env := newMediaEnv(t) + + req := newMultipartFileRequest(t, http.MethodPost, "/v1/media/", `C:\\photos\\north wall`, "image/png", pngBytes(t, 3, 2)) + rec := env.serve(t, req, mediaVerifiedUserId) + + if rec.Code != http.StatusCreated { + t.Fatalf("expected status 201, got %d, body %q", rec.Code, rec.Body.String()) + } + if got := decodeObject(t, rec.Body.String())["filename"]; got != "north wall.png" { + t.Errorf("expected filename %q, got %v", "north wall.png", got) + } + }) + + t.Run("A file exactly at 10485760 bytes is accepted, one byte more is 413", func(t *testing.T) { + env := newMediaEnv(t) + + req := newMultipartFileRequest(t, http.MethodPost, "/v1/media/", "max.png", "image/png", paddedPngBytes(t, int(domainImage.MaxImageSize))) + if rec := env.serve(t, req, mediaVerifiedUserId); rec.Code != http.StatusCreated { + t.Fatalf("expected status 201 at the limit, got %d, body %q", rec.Code, rec.Body.String()) + } + + req = newMultipartFileRequest(t, http.MethodPost, "/v1/media/", "over.png", "image/png", paddedPngBytes(t, int(domainImage.MaxImageSize)+1)) + rec := env.serve(t, req, mediaVerifiedUserId) + if rec.Code != http.StatusRequestEntityTooLarge { + t.Fatalf("expected status 413, got %d", rec.Code) + } + if rec.Body.String() != "the file exceeds the maximum upload size\n" { + t.Errorf("unexpected body %q", rec.Body.String()) + } + if env.images.createCalls != 1 { + t.Errorf("expected Create to be called once in total, got %d", env.images.createCalls) + } + }) + + t.Run("A disallowed content type is 415", func(t *testing.T) { + env := newMediaEnv(t) + + req := newMultipartFileRequest(t, http.MethodPost, "/v1/media/", "notes.pdf", "application/pdf", []byte("%PDF-1.4")) + rec := env.serve(t, req, mediaVerifiedUserId) + + if rec.Code != http.StatusUnsupportedMediaType { + t.Fatalf("expected status 415, got %d", rec.Code) + } + if rec.Body.String() != "the file must be a PNG, JPEG, WEBP, or SVG image\n" { + t.Errorf("unexpected body %q", rec.Body.String()) + } + if env.images.createCalls != 0 { + t.Errorf("expected Create to never be called") + } + }) + + t.Run("Bytes that cannot be read as the declared type are 400", func(t *testing.T) { + examples := []struct{ name, contentType string }{ + {"broken.png", "image/png"}, + {"broken.jpg", "image/jpeg"}, + {"broken.webp", "image/webp"}, + {"broken.svg", "image/svg+xml"}, + } + for _, example := range examples { + t.Run(example.name, func(t *testing.T) { + env := newMediaEnv(t) + + req := newMultipartFileRequest(t, http.MethodPost, "/v1/media/", example.name, example.contentType, []byte("not an image")) + rec := env.serve(t, req, mediaVerifiedUserId) + + if rec.Code != http.StatusBadRequest { + t.Fatalf("expected status 400, got %d", rec.Code) + } + if rec.Body.String() != "the image could not be read\n" { + t.Errorf("unexpected body %q", rec.Body.String()) + } + if env.images.createCalls != 0 { + t.Errorf("expected Create to never be called") + } + }) + } + }) + + t.Run("A request without the \"file\" part is 400", func(t *testing.T) { + env := newMediaEnv(t) + + rec := env.serve(t, newMultipartWithoutFileRequest(t, "/v1/media/"), mediaVerifiedUserId) + + if rec.Code != http.StatusBadRequest { + t.Fatalf("expected status 400, got %d", rec.Code) + } + if env.images.createCalls != 0 { + t.Errorf("expected Create to never be called") + } + }) + + t.Run("A repository failure is a generic 500", func(t *testing.T) { + env := newMediaEnv(t) + env.images.fail.failNext("Create") + + req := newMultipartFileRequest(t, http.MethodPost, "/v1/media/", "a.png", "image/png", pngBytes(t, 3, 2)) + rec := env.serve(t, req, mediaVerifiedUserId) + + if rec.Code != http.StatusInternalServerError || rec.Body.String() != "Internal server error\n" { + t.Errorf("expected generic 500, got %d %q", rec.Code, rec.Body.String()) + } + }) +} + +func TestMediaRenaming(t *testing.T) { + newEnv := func(t *testing.T) *mediaEnv { + env := newMediaEnv(t) + env.images.images[1] = domainImage.Image{ID: 1, Path: "/static/uploadedImages/aaa.jpeg", Filename: "north-wall.jpg", ContentType: "image/jpeg", UploadedByID: 1} + env.seedArticle(1, "Kitchen Renovation", true, ``) + return env + } + + t.Run("Rename changes the display name, keeps the extension and the url", func(t *testing.T) { + env := newEnv(t) + bodiesBefore := env.bodies() + + rec := env.send(t, http.MethodPatch, "/v1/media/1", `{"filename": "north-wall-01"}`, mediaVerifiedUserId) + + if rec.Code != http.StatusOK { + t.Fatalf("expected status 200, got %d, body %q", rec.Code, rec.Body.String()) + } + item := decodeObject(t, rec.Body.String()) + if item["filename"] != "north-wall-01.jpg" || item["url"] != "/static/uploadedImages/aaa.jpeg" { + t.Errorf("unexpected item %v", item) + } + if !reflect.DeepEqual(env.bodies(), bodiesBefore) { + t.Errorf("expected article bodies to be unchanged") + } + got, _ := env.list(t, "?q=north-wall-01") + expectIds(t, idsOf(got.Items), []uint32{1}) + }) + + t.Run("Surrounding whitespace is trimmed and unicode letters are allowed", func(t *testing.T) { + env := newEnv(t) + + rec := env.send(t, http.MethodPatch, "/v1/media/1", `{"filename": " Été_2026-a "}`, mediaVerifiedUserId) + + if rec.Code != http.StatusOK { + t.Fatalf("expected status 200, got %d", rec.Code) + } + if got := decodeObject(t, rec.Body.String())["filename"]; got != "Été_2026-a.jpg" { + t.Errorf("unexpected filename %v", got) + } + }) + + t.Run("An invalid new name is 400 and nothing changes", func(t *testing.T) { + examples := []struct { + name string + body string + }{ + {"empty", `{"filename": ""}`}, + {"blank", `{"filename": " "}`}, + {"symbols", `{"filename": "***"}`}, + {"dot", `{"filename": "a.b"}`}, + {"space", `{"filename": "a b"}`}, + {"101 letters", `{"filename": "` + strings.Repeat("a", 101) + `"}`}, + {"no field", `{}`}, + {"not json", `not json`}, + } + for _, example := range examples { + t.Run(example.name, func(t *testing.T) { + env := newEnv(t) + + rec := env.send(t, http.MethodPatch, "/v1/media/1", example.body, mediaVerifiedUserId) + + if rec.Code != http.StatusBadRequest { + t.Fatalf("expected status 400, got %d", rec.Code) + } + if example.name == "not json" { + if rec.Body.String() != apperr.BodyErrorMessage+"\n" { + t.Errorf("unexpected body %q", rec.Body.String()) + } + } else if rec.Body.String() != apperr.MediaFilenameErrorMessage+"\n" { + t.Errorf("unexpected body %q", rec.Body.String()) + } + if env.images.images[1].Filename != "north-wall.jpg" { + t.Errorf("expected the filename to be unchanged, got %q", env.images.images[1].Filename) + } + }) + } + }) + + t.Run("A name of exactly 100 letters is accepted", func(t *testing.T) { + env := newEnv(t) + + rec := env.send(t, http.MethodPatch, "/v1/media/1", `{"filename": "`+strings.Repeat("é", 100)+`"}`, mediaVerifiedUserId) + + if rec.Code != http.StatusOK { + t.Fatalf("expected status 200, got %d", rec.Code) + } + }) + + t.Run("Renaming an unknown image is 404", func(t *testing.T) { + env := newEnv(t) + + rec := env.send(t, http.MethodPatch, "/v1/media/9999", `{"filename": "x"}`, mediaVerifiedUserId) + + if rec.Code != http.StatusNotFound || rec.Body.String() != "Image not found\n" { + t.Errorf("expected 404 Image not found, got %d %q", rec.Code, rec.Body.String()) + } + }) + + t.Run("A malformed id is 400", func(t *testing.T) { + env := newEnv(t) + + rec := env.send(t, http.MethodPatch, "/v1/media/abc", `{"filename": "x"}`, mediaVerifiedUserId) + + if rec.Code != http.StatusBadRequest || rec.Body.String() != apperr.IdUint32ErrorMessage+"\n" { + t.Errorf("expected 400 with the id message, got %d %q", rec.Code, rec.Body.String()) + } + }) + + t.Run("A repository failure is a generic 500", func(t *testing.T) { + env := newEnv(t) + env.images.fail.failNext("Rename") + + rec := env.send(t, http.MethodPatch, "/v1/media/1", `{"filename": "x"}`, mediaVerifiedUserId) + + if rec.Code != http.StatusInternalServerError { + t.Errorf("expected status 500, got %d", rec.Code) + } + }) +} + +func TestMediaDeleting(t *testing.T) { + newEnv := func(t *testing.T) *mediaEnv { + env := newMediaEnv(t) + seedListingLibrary(env) + return env + } + + t.Run("An image in use can be deleted and article bodies are left alone", func(t *testing.T) { + env := newEnv(t) + bodiesBefore := env.bodies() + + rec := env.send(t, http.MethodDelete, "/v1/media/1", "", mediaVerifiedUserId) + + if rec.Code != http.StatusNoContent || rec.Body.Len() != 0 { + t.Fatalf("expected an empty 204, got %d %q", rec.Code, rec.Body.String()) + } + if _, ok := env.images.images[1]; ok { + t.Errorf("expected image 1 to be gone") + } + if !reflect.DeepEqual(env.images.deleteCalls, []uint32{1}) { + t.Errorf("expected deleteCalls [1], got %v", env.images.deleteCalls) + } + if !reflect.DeepEqual(env.bodies(), bodiesBefore) { + t.Errorf("expected article bodies to be byte-for-byte unchanged") + } + }) + + t.Run("Deleting an unknown image is 404", func(t *testing.T) { + env := newEnv(t) + + rec := env.send(t, http.MethodDelete, "/v1/media/9999", "", mediaVerifiedUserId) + + if rec.Code != http.StatusNotFound || rec.Body.String() != "Image not found\n" { + t.Errorf("expected 404 Image not found, got %d %q", rec.Code, rec.Body.String()) + } + }) + + t.Run("A malformed id is 400", func(t *testing.T) { + env := newEnv(t) + + rec := env.send(t, http.MethodDelete, "/v1/media/abc", "", mediaVerifiedUserId) + + if rec.Code != http.StatusBadRequest || rec.Body.String() != apperr.IdUint32ErrorMessage+"\n" { + t.Errorf("expected 400 with the id message, got %d %q", rec.Code, rec.Body.String()) + } + }) + + t.Run("A repository failure is a generic 500 and the image stays", func(t *testing.T) { + env := newEnv(t) + env.images.fail.failNext("Delete") + + rec := env.send(t, http.MethodDelete, "/v1/media/1", "", mediaVerifiedUserId) + + if rec.Code != http.StatusInternalServerError { + t.Errorf("expected status 500, got %d", rec.Code) + } + if _, ok := env.images.images[1]; !ok { + t.Errorf("expected image 1 to still exist") + } + }) +} + +func TestMediaUsage(t *testing.T) { + newEnv := func(t *testing.T) *mediaEnv { + env := newMediaEnv(t) + seedListingLibrary(env) + return env + } + + t.Run("Usage lists the articles whose body contains the url", func(t *testing.T) { + env := newEnv(t) + + rec := env.send(t, http.MethodGet, "/v1/media/1/usage", "", mediaVerifiedUserId) + + if rec.Code != http.StatusOK { + t.Fatalf("expected status 200, got %d", rec.Code) + } + want := `[{"id": 1, "title": "Kitchen Renovation", "published": true}, {"id": 2, "title": "Backyard Notes", "published": false}]` + expectJSONEqual(t, rec.Body.String(), want) + }) + + t.Run("An unused image has an empty array, not null", func(t *testing.T) { + env := newEnv(t) + + rec := env.send(t, http.MethodGet, "/v1/media/4/usage", "", mediaVerifiedUserId) + + if rec.Code != http.StatusOK || strings.TrimSpace(rec.Body.String()) != "[]" { + t.Errorf("expected [], got %d %q", rec.Code, rec.Body.String()) + } + }) + + t.Run("An absolute url still counts as used", func(t *testing.T) { + env := newEnv(t) + env.seedArticle(3, "Empty", true, "https://cms.example.com"+env.urlOf(1)) + + rec := env.send(t, http.MethodGet, "/v1/media/1/usage", "", mediaVerifiedUserId) + + var usage []map[string]any + if err := json.Unmarshal(rec.Body.Bytes(), &usage); err != nil { + t.Fatalf("failed to decode usage: %v", err) + } + found := false + for _, entry := range usage { + if numberOf(entry["id"]) == 3 { + found = true + } + } + if !found { + t.Errorf("expected article 3 in %v", usage) + } + }) + + t.Run("Unknown or malformed ids", func(t *testing.T) { + examples := []struct { + id string + status int + message string + }{ + {"9999", http.StatusNotFound, "Image not found"}, + {"abc", http.StatusBadRequest, apperr.IdUint32ErrorMessage}, + } + for _, example := range examples { + t.Run(example.id, func(t *testing.T) { + env := newEnv(t) + + rec := env.send(t, http.MethodGet, "/v1/media/"+example.id+"/usage", "", mediaVerifiedUserId) + + if rec.Code != example.status || rec.Body.String() != example.message+"\n" { + t.Errorf("expected %d %q, got %d %q", example.status, example.message, rec.Code, rec.Body.String()) + } + }) + } + }) + + t.Run("A repository failure is a generic 500", func(t *testing.T) { + env := newEnv(t) + env.articles.fail.failNext("FindByBodyContaining") + + rec := env.send(t, http.MethodGet, "/v1/media/1/usage", "", mediaVerifiedUserId) + + if rec.Code != http.StatusInternalServerError { + t.Errorf("expected status 500, got %d", rec.Code) + } + }) +} + +func expectJSONEqual(t *testing.T, got string, want string) { + t.Helper() + var gotValue, wantValue any + if err := json.Unmarshal([]byte(got), &gotValue); err != nil { + t.Fatalf("failed to decode %q: %v", got, err) + } + if err := json.Unmarshal([]byte(want), &wantValue); err != nil { + t.Fatalf("failed to decode %q: %v", want, err) + } + if !reflect.DeepEqual(gotValue, wantValue) { + t.Errorf("expected %s, got %s", want, got) + } +} + +func TestMediaMerging(t *testing.T) { + newEnv := func(t *testing.T) *mediaEnv { + env := newMediaEnv(t) + env.seedImage(1, "north-wall.jpg", 10, "image/jpeg", 3, 2, hashA, 1) + env.seedImage(2, "floorboards.jpg", 20, "image/jpeg", 3, 2, hashB, 1) + env.seedImage(3, "floorboards-copy.jpg", 20, "image/jpeg", 3, 2, hashB, 2) + env.seedImage(6, "floorboards-2.jpg", 20, "image/jpeg", 3, 2, hashB, 2) + env.seedArticle(1, "One", true, "a "+env.urlOf(2)+" b "+env.urlOf(3)) + env.seedArticle(2, "Two", false, env.urlOf(3)+" and "+env.urlOf(3)) + env.seedArticle(3, "Three", true, env.urlOf(2)) + env.seedArticle(4, "Four", true, "nothing") + return env + } + + t.Run("Merge switches bodies to the kept url and deletes the copies", func(t *testing.T) { + env := newEnv(t) + keep, copy3 := env.urlOf(2), env.urlOf(3) + + rec := env.send(t, http.MethodPost, "/v1/media/merge", `{"keep_id": 2, "merge_ids": [3, 6]}`, mediaVerifiedUserId) + + if rec.Code != http.StatusOK { + t.Fatalf("expected status 200, got %d, body %q", rec.Code, rec.Body.String()) + } + expectJSONEqual(t, rec.Body.String(), `{"articles_updated": 2, "merged": 2}`) + if got, want := env.articles.articles[1].Body, "a "+keep+" b "+keep; got != want { + t.Errorf("article 1 body: want %q, got %q", want, got) + } + if got, want := env.articles.articles[2].Body, keep+" and "+keep; got != want { + t.Errorf("article 2 body: want %q, got %q", want, got) + } + if env.articles.articles[3].Body != keep || env.articles.articles[4].Body != "nothing" { + t.Errorf("expected articles 3 and 4 to be unchanged") + } + if strings.Contains(env.articles.articles[1].Body, copy3) { + t.Errorf("expected image 3's url to be gone") + } + expectIds(t, env.imageIds(), []uint32{1, 2}) + }) + + t.Run("Merging copies that no article uses updates zero articles", func(t *testing.T) { + env := newEnv(t) + + rec := env.send(t, http.MethodPost, "/v1/media/merge", `{"keep_id": 2, "merge_ids": [6]}`, mediaVerifiedUserId) + + if rec.Code != http.StatusOK { + t.Fatalf("expected status 200, got %d", rec.Code) + } + expectJSONEqual(t, rec.Body.String(), `{"articles_updated": 0, "merged": 1}`) + }) + + t.Run("A copy that is gone answers 409 and changes nothing", func(t *testing.T) { + env := newEnv(t) + bodiesBefore := env.bodies() + + rec := env.send(t, http.MethodPost, "/v1/media/merge", `{"keep_id": 2, "merge_ids": [3, 9999]}`, mediaVerifiedUserId) + + if rec.Code != http.StatusConflict || rec.Body.String() != apperr.MediaMergeConflictErrorMessage+"\n" { + t.Fatalf("expected 409 with the conflict message, got %d %q", rec.Code, rec.Body.String()) + } + if !reflect.DeepEqual(env.bodies(), bodiesBefore) || len(env.images.deleteCalls) != 0 { + t.Errorf("expected nothing to change") + } + expectIds(t, env.imageIds(), []uint32{1, 2, 3, 6}) + }) + + t.Run("A copy that does not share the hash answers 409 and changes nothing", func(t *testing.T) { + env := newEnv(t) + bodiesBefore := env.bodies() + + rec := env.send(t, http.MethodPost, "/v1/media/merge", `{"keep_id": 2, "merge_ids": [3, 1]}`, mediaVerifiedUserId) + + if rec.Code != http.StatusConflict { + t.Fatalf("expected status 409, got %d", rec.Code) + } + if !reflect.DeepEqual(env.bodies(), bodiesBefore) || len(env.images.deleteCalls) != 0 { + t.Errorf("expected nothing to change") + } + expectIds(t, env.imageIds(), []uint32{1, 2, 3, 6}) + }) + + t.Run("A kept image that is gone answers 404", func(t *testing.T) { + env := newEnv(t) + bodiesBefore := env.bodies() + + rec := env.send(t, http.MethodPost, "/v1/media/merge", `{"keep_id": 9999, "merge_ids": [3]}`, mediaVerifiedUserId) + + if rec.Code != http.StatusNotFound || rec.Body.String() != "Image not found\n" { + t.Fatalf("expected 404 Image not found, got %d %q", rec.Code, rec.Body.String()) + } + if !reflect.DeepEqual(env.bodies(), bodiesBefore) || len(env.images.deleteCalls) != 0 { + t.Errorf("expected nothing to change") + } + }) + + t.Run("An invalid merge request is 400 and nothing changes", func(t *testing.T) { + bodies := []string{ + `{"keep_id": 2, "merge_ids": []}`, + `{"keep_id": 2}`, + `{"merge_ids": [3]}`, + `{"keep_id": 2, "merge_ids": [2, 3]}`, + `{"keep_id": 2, "merge_ids": [3, 3]}`, + `{"keep_id": 0, "merge_ids": [3]}`, + `{"keep_id": 2, "merge_ids": [0]}`, + `not json`, + } + for _, body := range bodies { + t.Run(body, func(t *testing.T) { + env := newEnv(t) + bodiesBefore := env.bodies() + + rec := env.send(t, http.MethodPost, "/v1/media/merge", body, mediaVerifiedUserId) + + if rec.Code != http.StatusBadRequest || rec.Body.String() != apperr.MediaMergeRequestErrorMessage+"\n" { + t.Fatalf("expected 400 with the merge message, got %d %q", rec.Code, rec.Body.String()) + } + if !reflect.DeepEqual(env.bodies(), bodiesBefore) || len(env.images.deleteCalls) != 0 { + t.Errorf("expected nothing to change") + } + }) + } + }) + + t.Run("A duplicated id reaching the use case is refused before anything is read", func(t *testing.T) { + env := newEnv(t) + uc := useCases.NewMergeMediaUseCase(env.images, env.articles) + + _, err := uc.Merge(2, []uint32{3, 3}) + + if !errors.Is(err, domainImage.ErrInvalidMergeRequest) { + t.Errorf("expected ErrInvalidMergeRequest, got %v", err) + } + }) + + t.Run("A failure rewriting a body restores the bodies already rewritten", func(t *testing.T) { + env := newEnv(t) + bodiesBefore := env.bodies() + env.articles.failUpdateBodyFor = map[uint32]error{2: errRepositoryFailure} + + rec := env.send(t, http.MethodPost, "/v1/media/merge", `{"keep_id": 2, "merge_ids": [3]}`, mediaVerifiedUserId) + + if rec.Code != http.StatusInternalServerError { + t.Fatalf("expected status 500, got %d", rec.Code) + } + if !reflect.DeepEqual(env.bodies(), bodiesBefore) { + t.Errorf("expected every body back to its original text, got %v", env.bodies()) + } + if len(env.images.deleteCalls) != 0 { + t.Errorf("expected no Delete call, got %v", env.images.deleteCalls) + } + expectIds(t, env.imageIds(), []uint32{1, 2, 3, 6}) + }) + + t.Run("A failure deleting a copy after the rewrite is 500 and the bodies stay switched", func(t *testing.T) { + env := newEnv(t) + env.images.failDeleteFor = map[uint32]error{3: errRepositoryFailure} + keep := env.urlOf(2) + + rec := env.send(t, http.MethodPost, "/v1/media/merge", `{"keep_id": 2, "merge_ids": [3]}`, mediaVerifiedUserId) + + if rec.Code != http.StatusInternalServerError { + t.Fatalf("expected status 500, got %d", rec.Code) + } + if !strings.Contains(env.articles.articles[1].Body, keep) || strings.Contains(env.articles.articles[1].Body, env.urlOf(3)) { + t.Errorf("expected article 1 to use image 2's url, got %q", env.articles.articles[1].Body) + } + if _, ok := env.images.images[3]; !ok { + t.Errorf("expected image 3 to still exist") + } + }) +} + +func TestMediaSentinels(t *testing.T) { + t.Run("Create reports unreadable bytes as ErrImageUnreadable", func(t *testing.T) { + uc := useCases.NewCreateImageUseCase(newMockImageRepo()) + + for _, contentType := range domainImage.AllowedContentTypes() { + _, err := uc.CreateImage(inputOf([]byte("not an image"), contentType)) + if !errors.Is(err, domainImage.ErrImageUnreadable) { + t.Errorf("%s: expected ErrImageUnreadable, got %v", contentType, err) + } + } + }) + + t.Run("An svg without an svg root is unreadable", func(t *testing.T) { + uc := useCases.NewCreateImageUseCase(newMockImageRepo()) + + _, err := uc.CreateImage(inputOf([]byte(""), "image/svg+xml")) + + if !errors.Is(err, domainImage.ErrImageUnreadable) { + t.Errorf("expected ErrImageUnreadable, got %v", err) + } + }) + + t.Run("Bytes of another raster format than the declared type are unreadable", func(t *testing.T) { + uc := useCases.NewCreateImageUseCase(newMockImageRepo()) + + _, err := uc.CreateImage(inputOf(jpegBytes(t, 3, 2), "image/png")) + + if !errors.Is(err, domainImage.ErrImageUnreadable) { + t.Errorf("expected ErrImageUnreadable, got %v", err) + } + }) + + t.Run("Rename reports an invalid stem as ErrInvalidFilename", func(t *testing.T) { + repo := newMockImageRepo() + repo.images[1] = domainImage.Image{ID: 1, Filename: "a.png"} + uc := useCases.NewRenameMediaUseCase(repo) + + if _, err := uc.Rename(1, "a b"); !errors.Is(err, domainImage.ErrInvalidFilename) { + t.Errorf("expected ErrInvalidFilename, got %v", err) + } + }) + + t.Run("Merge reports a missing copy and a hash mismatch", func(t *testing.T) { + env := newMediaEnv(t) + env.seedImage(1, "a.png", 1, "image/png", 1, 1, hashA, 1) + env.seedImage(2, "b.png", 1, "image/png", 1, 1, hashB, 1) + env.seedImage(3, "c.png", 1, "image/png", 1, 1, "", 1) + env.seedImage(4, "d.png", 1, "image/png", 1, 1, "", 1) + uc := useCases.NewMergeMediaUseCase(env.images, env.articles) + + if _, err := uc.Merge(1, []uint32{9999}); !errors.Is(err, domainImage.ErrMergeCopyMissing) { + t.Errorf("expected ErrMergeCopyMissing, got %v", err) + } + if _, err := uc.Merge(1, []uint32{2}); !errors.Is(err, domainImage.ErrMergeHashMismatch) { + t.Errorf("expected ErrMergeHashMismatch, got %v", err) + } + if _, err := uc.Merge(3, []uint32{4}); !errors.Is(err, domainImage.ErrMergeHashMismatch) { + t.Errorf("expected images without a hash never to merge, got %v", err) + } + }) +} + +func TestMediaHandlerLayering(t *testing.T) { + t.Run("api/handlers/media imports no persistence, mailer, models or gorm package", func(t *testing.T) { + output, err := exec.Command("go", "list", "-tags", "nofrontend", "-deps", "RenewCMS/api/handlers/media").CombinedOutput() + if err != nil { + t.Fatalf("go list failed: %v: %s", err, output) + } + for _, dependency := range strings.Split(string(output), "\n") { + for _, forbidden := range []string{"persistence", "mailer", "gorm"} { + if strings.Contains(dependency, forbidden) { + t.Errorf("api/handlers/media must not depend on %s", dependency) + } + } + } + }) +} diff --git a/test/media_persistence_test.go b/test/media_persistence_test.go new file mode 100644 index 0000000..9c91ccf --- /dev/null +++ b/test/media_persistence_test.go @@ -0,0 +1,253 @@ +package test + +import ( + domainArticle "RenewCMS/internal/domain/article" + domainImage "RenewCMS/internal/domain/image" + "RenewCMS/internal/infrastructure/persistence/models" + persistence "RenewCMS/internal/infrastructure/persistence/repositories" + "bytes" + "errors" + "os" + "path/filepath" + "testing" + + "gorm.io/gorm" +) + +func newMediaPersistence(t *testing.T) (*gorm.DB, *persistence.ImageRepository, string) { + t.Helper() + db := openInMemoryDb(t) + if err := db.AutoMigrate(&models.Article{}, &models.Image{}); err != nil { + t.Fatalf("failed to migrate: %v", err) + } + uploadDir := t.TempDir() + return db, persistence.NewImageRepository(db, uploadDir), uploadDir +} + +func createStoredImage(t *testing.T, repo *persistence.ImageRepository, filename string, content []byte, contentType string) domainImage.Image { + t.Helper() + created, err := repo.Create(domainImage.ImageInput{ + Content: bytes.NewReader(content), + Filename: filename, + Size: int64(len(content)), + ContentType: contentType, + Width: 3, + Height: 2, + SHA256: sha256Hex(content), + }) + if err != nil { + t.Fatalf("Create failed: %v", err) + } + return created +} + +func TestMediaImageRepository(t *testing.T) { + t.Run("Create stores the file and the metadata and returns the assigned id", func(t *testing.T) { + _, repo, uploadDir := newMediaPersistence(t) + content := pngBytes(t, 3, 2) + + created := createStoredImage(t, repo, "a.png", content, "image/png") + + if created.ID == 0 || created.Filename != "a.png" || created.SHA256 != sha256Hex(content) || created.Width != 3 || created.Size != int64(len(content)) { + t.Errorf("unexpected image %+v", created) + } + onDisk, err := os.ReadFile(filepath.Join(uploadDir, filepath.Base(created.Path))) + if err != nil || !bytes.Equal(onDisk, content) { + t.Errorf("expected the file on disk, got %v", err) + } + got, err := repo.Get(created.ID) + if err != nil || got.SHA256 != created.SHA256 { + t.Errorf("Get returned %+v, %v", got, err) + } + }) + + t.Run("Get reports a missing image as ErrImageNotFound", func(t *testing.T) { + _, repo, _ := newMediaPersistence(t) + + if _, err := repo.Get(9); !errors.Is(err, domainImage.ErrImageNotFound) { + t.Errorf("expected ErrImageNotFound, got %v", err) + } + }) + + t.Run("Delete removes the file and the row", func(t *testing.T) { + _, repo, uploadDir := newMediaPersistence(t) + created := createStoredImage(t, repo, "a.png", pngBytes(t, 3, 2), "image/png") + + if err := repo.Delete(created.ID); err != nil { + t.Fatalf("Delete failed: %v", err) + } + if _, err := os.Stat(filepath.Join(uploadDir, filepath.Base(created.Path))); !os.IsNotExist(err) { + t.Errorf("expected the file to be gone") + } + if _, err := repo.Get(created.ID); !errors.Is(err, domainImage.ErrImageNotFound) { + t.Errorf("expected the row to be gone, got %v", err) + } + }) + + t.Run("A file already missing from disk still deletes the row", func(t *testing.T) { + _, repo, uploadDir := newMediaPersistence(t) + created := createStoredImage(t, repo, "a.png", pngBytes(t, 3, 2), "image/png") + if err := os.Remove(filepath.Join(uploadDir, filepath.Base(created.Path))); err != nil { + t.Fatalf("failed to remove the file: %v", err) + } + + if err := repo.Delete(created.ID); err != nil { + t.Fatalf("Delete failed: %v", err) + } + if _, err := repo.Get(created.ID); !errors.Is(err, domainImage.ErrImageNotFound) { + t.Errorf("expected the row to be gone, got %v", err) + } + }) + + t.Run("List, FindByHash, Rename, ListDuplicates and Counts", func(t *testing.T) { + _, repo, _ := newMediaPersistence(t) + same := pngBytes(t, 3, 2) + first := createStoredImage(t, repo, "North-Wall.png", same, "image/png") + second := createStoredImage(t, repo, "copy.png", same, "image/png") + createStoredImage(t, repo, "other.png", pngBytes(t, 4, 4), "image/png") + + found, err := repo.List(domainImage.SearchQuery{Search: "NORTH"}) + if err != nil || len(found) != 1 || found[0].ID != first.ID { + t.Errorf("unexpected search result %+v, %v", found, err) + } + byHash, err := repo.List(domainImage.SearchQuery{Hash: sha256Hex(same)}) + if err != nil || len(byHash) != 2 { + t.Errorf("unexpected hash result %+v, %v", byHash, err) + } + sameContent, err := repo.FindByHash(sha256Hex(same)) + if err != nil || len(sameContent) != 2 || sameContent[1].ID != second.ID { + t.Errorf("unexpected FindByHash result %+v, %v", sameContent, err) + } + if none, err := repo.FindByHash(""); err != nil || len(none) != 0 { + t.Errorf("expected no result for an empty hash, got %+v, %v", none, err) + } + duplicates, err := repo.ListDuplicates() + if err != nil || len(duplicates) != 2 { + t.Errorf("unexpected duplicates %+v, %v", duplicates, err) + } + counts, err := repo.Counts() + if err != nil || counts.All != 3 || counts.DuplicateGroups != 1 { + t.Errorf("unexpected counts %+v, %v", counts, err) + } + + renamed, err := repo.Rename(first.ID, "renamed.png") + if err != nil || renamed.Filename != "renamed.png" || renamed.Path != first.Path { + t.Errorf("unexpected rename result %+v, %v", renamed, err) + } + if _, err := repo.Rename(9999, "x.png"); !errors.Is(err, domainImage.ErrImageNotFound) { + t.Errorf("expected ErrImageNotFound, got %v", err) + } + }) +} + +func TestMediaArticleRepositoryFindsBodiesContainingAFilename(t *testing.T) { + db, _, _ := newMediaPersistence(t) + repo := persistence.NewArticleRepository(db) + if _, err := repo.Create(domainArticle.Article{Title: "Kitchen Renovation", Body: "see /static/uploadedImages/aaa.png"}); err != nil { + t.Fatalf("failed to seed article: %v", err) + } + if _, err := repo.Create(domainArticle.Article{Title: "Backyard Notes", Body: "nothing"}); err != nil { + t.Fatalf("failed to seed article: %v", err) + } + + found, err := repo.FindByBodyContaining("aaa.png") + + if err != nil || len(found) != 1 || found[0].Title != "Kitchen Renovation" { + t.Errorf("unexpected result %+v, %v", found, err) + } + if none, err := repo.FindByBodyContaining("zzz.png"); err != nil || none == nil || len(none) != 0 { + t.Errorf("expected an empty non-nil slice, got %+v, %v", none, err) + } +} + +func TestMediaImageBackfill(t *testing.T) { + seedLegacyRow := func(t *testing.T, db *gorm.DB, name string) models.Image { + t.Helper() + row := models.Image{Path: "/static/uploadedImages/" + name} + if err := db.Create(&row).Error; err != nil { + t.Fatalf("failed to seed a legacy row: %v", err) + } + return row + } + writeFile := func(t *testing.T, dir string, name string, content []byte) { + t.Helper() + if err := os.WriteFile(filepath.Join(dir, name), content, 0666); err != nil { + t.Fatalf("failed to write %s: %v", name, err) + } + } + + t.Run("Existing rows get metadata and hash from the files on disk", func(t *testing.T) { + db, repo, uploadDir := newMediaPersistence(t) + png, svg := pngBytes(t, 3, 2), []byte(svgFixture) + writeFile(t, uploadDir, "one.png", png) + writeFile(t, uploadDir, "two.svg", svg) + pngRow, svgRow := seedLegacyRow(t, db, "one.png"), seedLegacyRow(t, db, "two.svg") + + if err := persistence.BackfillImageMetadata(db, uploadDir); err != nil { + t.Fatalf("backfill failed: %v", err) + } + + gotPng, _ := repo.Get(pngRow.ID) + if gotPng.Filename != "one.png" || gotPng.Size != int64(len(png)) || gotPng.ContentType != "image/png" || gotPng.Width != 3 || gotPng.Height != 2 || gotPng.SHA256 != sha256Hex(png) || gotPng.UploadedByID != 0 { + t.Errorf("unexpected png row %+v", gotPng) + } + gotSvg, _ := repo.Get(svgRow.ID) + if gotSvg.ContentType != "image/svg+xml" || gotSvg.Width != 0 || gotSvg.Height != 0 || gotSvg.SHA256 != sha256Hex(svg) { + t.Errorf("unexpected svg row %+v", gotSvg) + } + }) + + t.Run("A row whose file is missing does not stop the boot", func(t *testing.T) { + db, repo, uploadDir := newMediaPersistence(t) + row := seedLegacyRow(t, db, "gone.png") + + if err := persistence.BackfillImageMetadata(db, uploadDir); err != nil { + t.Fatalf("backfill failed: %v", err) + } + + got, _ := repo.Get(row.ID) + if got.Filename != "" || got.SHA256 != "" { + t.Errorf("expected the row to be left as is, got %+v", got) + } + }) + + t.Run("An unreadable file leaves its row as is and the others are still filled", func(t *testing.T) { + db, repo, uploadDir := newMediaPersistence(t) + writeFile(t, uploadDir, "broken.png", []byte("not an image")) + writeFile(t, uploadDir, "fine.png", pngBytes(t, 3, 2)) + broken, fine := seedLegacyRow(t, db, "broken.png"), seedLegacyRow(t, db, "fine.png") + + if err := persistence.BackfillImageMetadata(db, uploadDir); err != nil { + t.Fatalf("backfill failed: %v", err) + } + + if got, _ := repo.Get(broken.ID); got.Filename != "" { + t.Errorf("expected the broken row to be left as is, got %+v", got) + } + if got, _ := repo.Get(fine.ID); got.Filename != "fine.png" { + t.Errorf("expected the fine row to be filled, got %+v", got) + } + }) + + t.Run("The backfill is idempotent", func(t *testing.T) { + db, repo, uploadDir := newMediaPersistence(t) + writeFile(t, uploadDir, "one.png", pngBytes(t, 3, 2)) + row := seedLegacyRow(t, db, "one.png") + if err := persistence.BackfillImageMetadata(db, uploadDir); err != nil { + t.Fatalf("backfill failed: %v", err) + } + afterFirst, _ := repo.Get(row.ID) + if _, err := repo.Rename(row.ID, "renamed.png"); err != nil { + t.Fatal(err) + } + + if err := persistence.BackfillImageMetadata(db, uploadDir); err != nil { + t.Fatalf("second backfill failed: %v", err) + } + + afterSecond, _ := repo.Get(row.ID) + if afterSecond.Filename != "renamed.png" || afterSecond.SHA256 != afterFirst.SHA256 { + t.Errorf("expected the second run to change nothing, got %+v", afterSecond) + } + }) +} diff --git a/test/not_found_errors_test.go b/test/not_found_errors_test.go index 9eb894f..30800aa 100644 --- a/test/not_found_errors_test.go +++ b/test/not_found_errors_test.go @@ -129,7 +129,7 @@ func newNotFoundErrorsRouter(articleRepo *mockArticleRepo, userRepo *mockUserRep useCases.NewCreateArticleUseCase(articleRepo), useCases.NewGetArticleUseCase(articleRepo), useCases.NewListArticlesUseCase(articleRepo), - useCases.NewDeleteArticleUseCase(articleRepo, newMockImageRepo()), + useCases.NewDeleteArticleUseCase(articleRepo), useCases.NewUpdateArticleUseCase(articleRepo), getUserUseCase, ) diff --git a/test/public_article_listing_test.go b/test/public_article_listing_test.go index e521302..91a408f 100644 --- a/test/public_article_listing_test.go +++ b/test/public_article_listing_test.go @@ -45,7 +45,7 @@ func newPublicArticleListingRouter(articleRepo *mockArticleRepo, token *jwtauth. useCases.NewCreateArticleUseCase(articleRepo), useCases.NewGetArticleUseCase(articleRepo), useCases.NewListArticlesUseCase(articleRepo), - useCases.NewDeleteArticleUseCase(articleRepo, newMockImageRepo()), + useCases.NewDeleteArticleUseCase(articleRepo), useCases.NewUpdateArticleUseCase(articleRepo), getUserUseCase, ) diff --git a/test/public_article_read_test.go b/test/public_article_read_test.go index 31261fa..f461910 100644 --- a/test/public_article_read_test.go +++ b/test/public_article_read_test.go @@ -27,7 +27,7 @@ func newPublicArticleReadRouter(articleRepo *mockArticleRepo, userRepo *mockUser useCases.NewCreateArticleUseCase(articleRepo), useCases.NewGetArticleUseCase(articleRepo), useCases.NewListArticlesUseCase(articleRepo), - useCases.NewDeleteArticleUseCase(articleRepo, newMockImageRepo()), + useCases.NewDeleteArticleUseCase(articleRepo), useCases.NewUpdateArticleUseCase(articleRepo), getUserUseCase, ) diff --git a/test/remove_article_image_http_test.go b/test/remove_article_image_http_test.go deleted file mode 100644 index a46a42a..0000000 --- a/test/remove_article_image_http_test.go +++ /dev/null @@ -1,196 +0,0 @@ -package test - -import ( - "RenewCMS/api/handlers/image" - "RenewCMS/api/middleware" - "RenewCMS/api/pkg/apperr" - validator "RenewCMS/api/pkg/validator" - domainArticle "RenewCMS/internal/domain/article" - domainImage "RenewCMS/internal/domain/image" - "RenewCMS/internal/infrastructure/useCases" - "net/http" - "net/http/httptest" - "testing" - - "github.com/go-chi/chi/v5" - "github.com/go-chi/jwtauth/v5" -) - -func newRemoveArticleImageRouter(articleRepo *mockArticleRepo, imageRepo *mockImageRepo, token *jwtauth.JWTAuth) *chi.Mux { - userRepo := newVerifiedUserRepo() - getUserUseCase := useCases.NewGetUserUseCase(userRepo) - - imageHandler := image.NewHandler( - useCases.NewCreateImageUseCase(imageRepo), - useCases.NewGetArticleUseCase(articleRepo), - useCases.NewUpdateArticleUseCase(articleRepo), - useCases.NewDeleteImageUseCase(imageRepo), - ) - - r := chi.NewRouter() - r.Route("/v1", func(r chi.Router) { - r.Route("/article", func(r chi.Router) { - r.Group(func(r chi.Router) { - r.Use(jwtauth.Verifier(token)) - r.Group(func(r chi.Router) { - r.Use(jwtauth.Authenticator(token)) - r.Use(middleware.RequireVerifiedUser(getUserUseCase)) - r.Delete("/{id}/image/{imageId}", imageHandler.DeleteImage) - }) - }) - }) - }) - return r -} - -func setRemoveImageJwtCookie(t *testing.T, req *http.Request, token *jwtauth.JWTAuth) { - t.Helper() - _, tokenString, err := token.Encode(map[string]any{"user_id": uint32(1)}) - if err != nil { - t.Fatalf("failed to encode jwt: %v", err) - } - req.AddCookie(&http.Cookie{Name: "jwt", Value: tokenString}) -} - -func TestRemoveArticleImageHttp(t *testing.T) { - validator.InitValidator() - token := jwtauth.New("HS256", []byte("test-secret"), nil) - - t.Run("Removing an image from an article it belongs to succeeds", func(t *testing.T) { - articleRepo := newMockArticleRepo() - articleRepo.articles[1] = domainArticle.Article{ID: 1, Title: "Backyard Renovation Update", Images: []*domainImage.Image{{ID: 10}}} - imageRepo := newMockImageRepo() - imageRepo.images[10] = domainImage.Image{ID: 10, Path: "/static/uploadedImages/before.png"} - router := newRemoveArticleImageRouter(articleRepo, imageRepo, token) - - req := httptest.NewRequest(http.MethodDelete, "/v1/article/1/image/10", nil) - setRemoveImageJwtCookie(t, req, token) - rec := httptest.NewRecorder() - router.ServeHTTP(rec, req) - - if rec.Code != http.StatusNoContent { - t.Fatalf("expected status 204, got %d, body %q", rec.Code, rec.Body.String()) - } - if rec.Body.Len() != 0 { - t.Errorf("expected empty body, got %q", rec.Body.String()) - } - if _, ok := imageRepo.images[10]; ok { - t.Errorf("expected image 10 to have been removed") - } - }) - - t.Run("Removing an image from an unknown article answers 404 and never touches the image repository", func(t *testing.T) { - articleRepo := newMockArticleRepo() - imageRepo := newMockImageRepo() - imageRepo.images[10] = domainImage.Image{ID: 10, Path: "/static/uploadedImages/before.png"} - router := newRemoveArticleImageRouter(articleRepo, imageRepo, token) - - req := httptest.NewRequest(http.MethodDelete, "/v1/article/9999/image/10", nil) - setRemoveImageJwtCookie(t, req, token) - rec := httptest.NewRecorder() - router.ServeHTTP(rec, req) - - if rec.Code != http.StatusNotFound { - t.Fatalf("expected status 404, got %d", rec.Code) - } - if rec.Body.String() != apperr.ArticleNotFoundErrorMessage+"\n" { - t.Errorf("unexpected body: %q", rec.Body.String()) - } - if len(imageRepo.deleteCalls) != 0 { - t.Errorf("expected Delete to never be called, got %d calls", len(imageRepo.deleteCalls)) - } - if _, ok := imageRepo.images[10]; !ok { - t.Errorf("expected image 10 to still exist") - } - }) - - t.Run("Removing an unknown image from an existing article answers 404", func(t *testing.T) { - articleRepo := newMockArticleRepo() - articleRepo.articles[1] = domainArticle.Article{ID: 1, Title: "Backyard Renovation Update"} - imageRepo := newMockImageRepo() - router := newRemoveArticleImageRouter(articleRepo, imageRepo, token) - - req := httptest.NewRequest(http.MethodDelete, "/v1/article/1/image/9999", nil) - setRemoveImageJwtCookie(t, req, token) - rec := httptest.NewRecorder() - router.ServeHTTP(rec, req) - - if rec.Code != http.StatusNotFound { - t.Fatalf("expected status 404, got %d", rec.Code) - } - if rec.Body.String() != apperr.ImageNotFoundErrorMessage+"\n" { - t.Errorf("unexpected body: %q", rec.Body.String()) - } - }) - - t.Run("Removing an image id that belongs to a different article still succeeds, because the check is shallow", func(t *testing.T) { - articleRepo := newMockArticleRepo() - articleRepo.articles[1] = domainArticle.Article{ID: 1, Title: "Backyard Renovation Update"} - articleRepo.articles[2] = domainArticle.Article{ID: 2, Images: []*domainImage.Image{{ID: 10}}} - imageRepo := newMockImageRepo() - imageRepo.images[10] = domainImage.Image{ID: 10, Path: "/static/uploadedImages/before.png"} - router := newRemoveArticleImageRouter(articleRepo, imageRepo, token) - - req := httptest.NewRequest(http.MethodDelete, "/v1/article/1/image/10", nil) - setRemoveImageJwtCookie(t, req, token) - rec := httptest.NewRecorder() - router.ServeHTTP(rec, req) - - if rec.Code != http.StatusNoContent { - t.Fatalf("expected status 204, got %d, body %q", rec.Code, rec.Body.String()) - } - if _, ok := imageRepo.images[10]; ok { - t.Errorf("expected image 10 to have been removed") - } - }) - - t.Run("Removing an image still requires authentication", func(t *testing.T) { - articleRepo := newMockArticleRepo() - articleRepo.articles[1] = domainArticle.Article{ID: 1, Title: "Backyard Renovation Update", Images: []*domainImage.Image{{ID: 10}}} - imageRepo := newMockImageRepo() - imageRepo.images[10] = domainImage.Image{ID: 10, Path: "/static/uploadedImages/before.png"} - router := newRemoveArticleImageRouter(articleRepo, imageRepo, token) - - req := httptest.NewRequest(http.MethodDelete, "/v1/article/1/image/10", nil) - rec := httptest.NewRecorder() - router.ServeHTTP(rec, req) - - if rec.Code != http.StatusUnauthorized { - t.Fatalf("expected status 401, got %d", rec.Code) - } - if _, ok := imageRepo.images[10]; !ok { - t.Errorf("expected image 10 to still exist") - } - }) - - t.Run("A malformed id in the URL answers 400 before either repository is touched", func(t *testing.T) { - testCases := []struct { - name string - articleId string - imageId string - }{ - {"malformed article id", "abc", "10"}, - {"malformed image id", "1", "abc"}, - } - - for _, tc := range testCases { - t.Run(tc.name, func(t *testing.T) { - articleRepo := newMockArticleRepo() - imageRepo := newMockImageRepo() - router := newRemoveArticleImageRouter(articleRepo, imageRepo, token) - - req := httptest.NewRequest(http.MethodDelete, "/v1/article/"+tc.articleId+"/image/"+tc.imageId, nil) - setRemoveImageJwtCookie(t, req, token) - rec := httptest.NewRecorder() - router.ServeHTTP(rec, req) - - if rec.Code != http.StatusBadRequest { - t.Fatalf("expected status 400, got %d", rec.Code) - } - if rec.Body.String() != apperr.IdUint32ErrorMessage+"\n" { - t.Errorf("unexpected body: %q", rec.Body.String()) - } - }) - } - }) -} diff --git a/test/runtime_settings_injection_http_test.go b/test/runtime_settings_injection_http_test.go index d6bca17..d7a31df 100644 --- a/test/runtime_settings_injection_http_test.go +++ b/test/runtime_settings_injection_http_test.go @@ -136,7 +136,7 @@ func newRuntimeSettingsContainer(t *testing.T) *api.UseCases { GetArticleUseCase: useCases.NewGetArticleUseCase(articleRepo), ListArticlesUseCase: useCases.NewListArticlesUseCase(articleRepo), UpdateArticleUseCase: useCases.NewUpdateArticleUseCase(articleRepo), - DeleteArticleUseCase: useCases.NewDeleteArticleUseCase(articleRepo, imageRepo), + DeleteArticleUseCase: useCases.NewDeleteArticleUseCase(articleRepo), GetUserUseCase: useCases.NewGetUserUseCase(userRepo), CreateUserUseCase: useCases.NewCreateUserUseCase(userRepo, hasher, mailRepo, "https://cms.example.com"), AuthenticateUserUseCase: useCases.NewAuthenticateUserUseCase(userRepo, hasher), @@ -144,7 +144,11 @@ func newRuntimeSettingsContainer(t *testing.T) *api.UseCases { ListUsersUseCase: useCases.NewListUsersUseCase(userRepo), SendMailUseCase: useCases.NewSendMailUseCase(mailRepo), CreateImageUseCase: useCases.NewCreateImageUseCase(imageRepo), - DeleteImageUseCase: useCases.NewDeleteImageUseCase(imageRepo), + ListMediaUseCase: useCases.NewListMediaUseCase(imageRepo, articleRepo, userRepo), + RenameMediaUseCase: useCases.NewRenameMediaUseCase(imageRepo), + DeleteMediaUseCase: useCases.NewDeleteMediaUseCase(imageRepo), + MergeMediaUseCase: useCases.NewMergeMediaUseCase(imageRepo, articleRepo), + GetMediaUsageUseCase: useCases.NewGetMediaUsageUseCase(imageRepo, articleRepo), } } diff --git a/test/stale_token_invalidation_http_test.go b/test/stale_token_invalidation_http_test.go index 848cf4c..6559fbd 100644 --- a/test/stale_token_invalidation_http_test.go +++ b/test/stale_token_invalidation_http_test.go @@ -26,7 +26,7 @@ func newStaleTokenInvalidationRouter(userRepo *mockUserRepo, articleRepo *mockAr useCases.NewCreateArticleUseCase(articleRepo), useCases.NewGetArticleUseCase(articleRepo), useCases.NewListArticlesUseCase(articleRepo), - useCases.NewDeleteArticleUseCase(articleRepo, imageRepo), + useCases.NewDeleteArticleUseCase(articleRepo), useCases.NewUpdateArticleUseCase(articleRepo), getUserUseCase, ) diff --git a/test/upload_article_image_http_test.go b/test/upload_article_image_http_test.go index 735c010..6d977b1 100644 --- a/test/upload_article_image_http_test.go +++ b/test/upload_article_image_http_test.go @@ -1,152 +1,59 @@ package test import ( - "RenewCMS/api/handlers/image" - "RenewCMS/api/middleware" "RenewCMS/api/pkg/apperr" - validator "RenewCMS/api/pkg/validator" - domainArticle "RenewCMS/internal/domain/article" - domainImage "RenewCMS/internal/domain/image" - "RenewCMS/internal/infrastructure/useCases" - "bytes" - "encoding/json" - "io" - "mime/multipart" "net/http" - "net/http/httptest" - "strings" "testing" - "github.com/go-chi/chi/v5" - "github.com/go-chi/jwtauth/v5" + domainImage "RenewCMS/internal/domain/image" ) -func newUploadArticleImageRouter(articleRepo *mockArticleRepo, imageRepo *mockImageRepo, token *jwtauth.JWTAuth) *chi.Mux { - userRepo := newVerifiedUserRepo() - getUserUseCase := useCases.NewGetUserUseCase(userRepo) - - imageHandler := image.NewHandler( - useCases.NewCreateImageUseCase(imageRepo), - useCases.NewGetArticleUseCase(articleRepo), - useCases.NewUpdateArticleUseCase(articleRepo), - useCases.NewDeleteImageUseCase(imageRepo), - ) - - r := chi.NewRouter() - r.Route("/v1", func(r chi.Router) { - r.Route("/article", func(r chi.Router) { - r.Group(func(r chi.Router) { - r.Use(jwtauth.Verifier(token)) - r.Group(func(r chi.Router) { - r.Use(jwtauth.Authenticator(token)) - r.Use(middleware.RequireVerifiedUser(getUserUseCase)) - r.Post("/{id}/image", imageHandler.PostImage) - }) - }) - }) - }) - return r -} - -func setUploadJwtCookie(t *testing.T, req *http.Request, token *jwtauth.JWTAuth) { - t.Helper() - _, tokenString, err := token.Encode(map[string]any{"user_id": uint32(1)}) - if err != nil { - t.Fatalf("failed to encode jwt: %v", err) - } - req.AddCookie(&http.Cookie{Name: "jwt", Value: tokenString}) -} - -func newMultipartImageUploadRequest(t *testing.T, url string, filename string, contentType string) *http.Request { - t.Helper() - body := &bytes.Buffer{} - writer := multipart.NewWriter(body) - part, err := writer.CreatePart(map[string][]string{ - "Content-Disposition": {`form-data; name="file"; filename="` + filename + `"`}, - "Content-Type": {contentType}, - }) - if err != nil { - t.Fatalf("failed to create multipart part: %v", err) - } - if _, err := io.WriteString(part, "fake-image-data"); err != nil { - t.Fatalf("failed to write file content: %v", err) - } - if err := writer.Close(); err != nil { - t.Fatalf("failed to close multipart writer: %v", err) - } - req := httptest.NewRequest(http.MethodPost, url, body) - req.Header.Set("Content-Type", writer.FormDataContentType()) - return req -} - func TestUploadArticleImageHttp(t *testing.T) { - validator.InitValidator() - token := jwtauth.New("HS256", []byte("test-secret"), nil) + newEnv := func(t *testing.T) *mediaEnv { + env := newMediaEnv(t) + env.seedArticle(1, "Backyard Renovation Update", true, "unchanged body") + return env + } - t.Run("Uploading an allowed image type to an existing article succeeds", func(t *testing.T) { - articleRepo := newMockArticleRepo() - articleRepo.articles[1] = domainArticle.Article{ID: 1, Title: "Backyard Renovation Update"} - articleRepo.images[1] = &domainImage.Image{ID: 1, Path: "/static/uploadedImages/before.png"} - imageRepo := newMockImageRepo() - router := newUploadArticleImageRouter(articleRepo, imageRepo, token) + t.Run("Uploading through the alias creates a library image", func(t *testing.T) { + env := newEnv(t) - req := newMultipartImageUploadRequest(t, "/v1/article/1/image", "before.png", "image/png") - setUploadJwtCookie(t, req, token) - rec := httptest.NewRecorder() - router.ServeHTTP(rec, req) + req := newMultipartFileRequest(t, http.MethodPost, "/v1/article/1/image", "before.png", "image/png", pngBytes(t, 3, 2)) + rec := env.serve(t, req, mediaVerifiedUserId) if rec.Code != http.StatusCreated { t.Fatalf("expected status 201, got %d, body %q", rec.Code, rec.Body.String()) } - - var got map[string]any - if err := json.Unmarshal(rec.Body.Bytes(), &got); err != nil { - t.Fatalf("failed to decode response: %v", err) + got := decodeObject(t, rec.Body.String()) + if len(env.images.images) != 1 { + t.Fatalf("expected exactly one library image, got %d", len(env.images.images)) } - if got["location"] != "/static/uploadedImages/before.png" { - t.Errorf("expected location %q, got %q", "/static/uploadedImages/before.png", got["location"]) + created := env.images.images[1] + if numberOf(got["id"]) != created.ID { + t.Errorf("expected id %d, got %v", created.ID, got["id"]) + } + if got["location"] != created.Path { + t.Errorf("expected location %q, got %v", created.Path, got["location"]) + } + if created.UploadedByID != mediaVerifiedUserId { + t.Errorf("expected UploadedByID 1, got %d", created.UploadedByID) } - if len(imageRepo.images) != 1 { - t.Fatalf("expected the image repository to gain exactly one entry, got %d", len(imageRepo.images)) + listing := env.send(t, http.MethodGet, "/v1/media/", "", mediaVerifiedUserId) + items := decodeObject(t, listing.Body.String())["items"].([]any) + if len(items) != 1 || items[0].(map[string]any)["uploaded_by"] != "Inès Duarte" { + t.Errorf("expected the media list to hold the image uploaded by user 1, got %v", items) } - var storedImageID uint32 - found := false - for id, img := range imageRepo.images { - if img.Path == "/static/uploadedImages/before.png" { - storedImageID = id - found = true - } - } - if !found { - t.Fatalf("expected the image repository to contain the new image") - } - gotId, ok := got["id"].(float64) - if !ok || uint32(gotId) != storedImageID { - t.Errorf("expected id %v, got %v", storedImageID, got["id"]) - } - - updatedArticle := articleRepo.articles[1] - imageAttached := false - for _, img := range updatedArticle.Images { - if img.ID == storedImageID { - imageAttached = true - } - } - if !imageAttached { - t.Errorf("expected article 1 to reference the new image's id, got %+v", updatedArticle.Images) + if env.articles.articles[1].Body != "unchanged body" { + t.Errorf("expected article 1 to be unchanged, got %+v", env.articles.articles[1]) } }) - t.Run("Uploading to an unknown article answers 404 and creates no image", func(t *testing.T) { - articleRepo := newMockArticleRepo() - imageRepo := newMockImageRepo() - router := newUploadArticleImageRouter(articleRepo, imageRepo, token) + t.Run("An unknown article is 404 and creates no image", func(t *testing.T) { + env := newEnv(t) - req := newMultipartImageUploadRequest(t, "/v1/article/9999/image", "before.png", "image/png") - setUploadJwtCookie(t, req, token) - rec := httptest.NewRecorder() - router.ServeHTTP(rec, req) + req := newMultipartFileRequest(t, http.MethodPost, "/v1/article/9999/image", "before.png", "image/png", pngBytes(t, 3, 2)) + rec := env.serve(t, req, mediaVerifiedUserId) if rec.Code != http.StatusNotFound { t.Fatalf("expected status 404, got %d", rec.Code) @@ -154,111 +61,101 @@ func TestUploadArticleImageHttp(t *testing.T) { if rec.Body.String() != apperr.ArticleNotFoundErrorMessage+"\n" { t.Errorf("unexpected body: %q", rec.Body.String()) } - if len(imageRepo.images) != 0 { - t.Errorf("expected no image to be created, got %d", len(imageRepo.images)) + if env.images.createCalls != 0 { + t.Errorf("expected Create to never be called, got %d calls", env.images.createCalls) } }) - t.Run("Uploading a disallowed content type answers 400 before the article is touched", func(t *testing.T) { - articleRepo := newMockArticleRepo() - articleRepo.articles[1] = domainArticle.Article{ID: 1, Title: "Backyard Renovation Update"} - imageRepo := newMockImageRepo() - router := newUploadArticleImageRouter(articleRepo, imageRepo, token) + t.Run("Rejected files use the media status codes", func(t *testing.T) { + examples := []struct { + name string + filename string + contentType string + content []byte + status int + }{ + {"notes.pdf as application/pdf", "notes.pdf", "application/pdf", []byte("%PDF"), http.StatusUnsupportedMediaType}, + {"a valid PNG padded to 10485761 bytes", "huge.png", "image/png", paddedPngBytes(t, int(domainImage.MaxImageSize)+1), http.StatusRequestEntityTooLarge}, + {"broken.png as image/png with text bytes", "broken.png", "image/png", []byte("not an image"), http.StatusBadRequest}, + } + for _, example := range examples { + t.Run(example.name, func(t *testing.T) { + env := newEnv(t) - req := newMultipartImageUploadRequest(t, "/v1/article/1/image", "notes.pdf", "application/pdf") - setUploadJwtCookie(t, req, token) - rec := httptest.NewRecorder() - router.ServeHTTP(rec, req) + req := newMultipartFileRequest(t, http.MethodPost, "/v1/article/1/image", example.filename, example.contentType, example.content) + rec := env.serve(t, req, mediaVerifiedUserId) - if rec.Code != http.StatusBadRequest { - t.Fatalf("expected status 400, got %d", rec.Code) - } - if rec.Body.String() != "the file must be a PNG, JPEG, WEBP, or SVG image\n" { - t.Errorf("unexpected body: %q", rec.Body.String()) - } - if len(imageRepo.images) != 0 { - t.Errorf("expected no image to be created, got %d", len(imageRepo.images)) - } - if len(articleRepo.articles[1].Images) != 0 { - t.Errorf("expected article 1 to still have an empty Images slice, got %+v", articleRepo.articles[1].Images) + if rec.Code != example.status { + t.Fatalf("expected status %d, got %d", example.status, rec.Code) + } + if env.images.createCalls != 0 { + t.Errorf("expected Create to never be called, got %d calls", env.images.createCalls) + } + }) } }) - t.Run("Uploading an image still requires authentication", func(t *testing.T) { - articleRepo := newMockArticleRepo() - articleRepo.articles[1] = domainArticle.Article{ID: 1, Title: "Backyard Renovation Update"} - imageRepo := newMockImageRepo() - router := newUploadArticleImageRouter(articleRepo, imageRepo, token) + t.Run("The alias still requires authentication", func(t *testing.T) { + env := newEnv(t) - req := newMultipartImageUploadRequest(t, "/v1/article/1/image", "before.png", "image/png") - rec := httptest.NewRecorder() - router.ServeHTTP(rec, req) + req := newMultipartFileRequest(t, http.MethodPost, "/v1/article/1/image", "before.png", "image/png", pngBytes(t, 3, 2)) + rec := env.serve(t, req, 0) if rec.Code != http.StatusUnauthorized { t.Fatalf("expected status 401, got %d", rec.Code) } - if len(imageRepo.images) != 0 { - t.Errorf("expected no image to be created, got %d", len(imageRepo.images)) + if env.images.createCalls != 0 { + t.Errorf("expected Create to never be called, got %d calls", env.images.createCalls) } - if len(articleRepo.articles[1].Images) != 0 { - t.Errorf("expected article 1 to still have an empty Images slice, got %+v", articleRepo.articles[1].Images) + }) + + t.Run("The removed delete route is gone", func(t *testing.T) { + env := newEnv(t) + env.seedImage(10, "before.png", 10, "image/png", 3, 2, sha256Hex([]byte("x")), 1) + + rec := env.send(t, http.MethodDelete, "/v1/article/1/image/10", "", mediaVerifiedUserId) + + if rec.Code != http.StatusNotFound && rec.Code != http.StatusMethodNotAllowed { + t.Fatalf("expected status 404 or 405, got %d", rec.Code) + } + if _, ok := env.images.images[10]; !ok { + t.Errorf("expected image 10 to still exist") + } + if len(env.images.deleteCalls) != 0 { + t.Errorf("expected Delete to never be called, got %v", env.images.deleteCalls) } }) t.Run("Uploading with a non-numeric article id answers 400", func(t *testing.T) { - articleRepo := newMockArticleRepo() - imageRepo := newMockImageRepo() - router := newUploadArticleImageRouter(articleRepo, imageRepo, token) + env := newEnv(t) - req := newMultipartImageUploadRequest(t, "/v1/article/abc/image", "before.png", "image/png") - setUploadJwtCookie(t, req, token) - rec := httptest.NewRecorder() - router.ServeHTTP(rec, req) + req := newMultipartFileRequest(t, http.MethodPost, "/v1/article/abc/image", "before.png", "image/png", pngBytes(t, 3, 2)) + rec := env.serve(t, req, mediaVerifiedUserId) - if rec.Code != http.StatusBadRequest { - t.Fatalf("expected status 400, got %d", rec.Code) - } - if rec.Body.String() != apperr.IdUint32ErrorMessage+"\n" { - t.Errorf("unexpected body: %q", rec.Body.String()) + if rec.Code != http.StatusBadRequest || rec.Body.String() != apperr.IdUint32ErrorMessage+"\n" { + t.Fatalf("expected 400 with the id message, got %d %q", rec.Code, rec.Body.String()) } }) t.Run("Uploading without a file field answers 400", func(t *testing.T) { - articleRepo := newMockArticleRepo() - articleRepo.articles[1] = domainArticle.Article{ID: 1, Title: "Backyard Renovation Update"} - imageRepo := newMockImageRepo() - router := newUploadArticleImageRouter(articleRepo, imageRepo, token) + env := newEnv(t) - body := &bytes.Buffer{} - writer := multipart.NewWriter(body) - if err := writer.Close(); err != nil { - t.Fatalf("failed to close multipart writer: %v", err) - } - req := httptest.NewRequest(http.MethodPost, "/v1/article/1/image", body) - req.Header.Set("Content-Type", writer.FormDataContentType()) - setUploadJwtCookie(t, req, token) - rec := httptest.NewRecorder() - router.ServeHTTP(rec, req) + rec := env.serve(t, newMultipartWithoutFileRequest(t, "/v1/article/1/image"), mediaVerifiedUserId) if rec.Code != http.StatusBadRequest { t.Fatalf("expected status 400, got %d", rec.Code) } - if len(imageRepo.images) != 0 { - t.Errorf("expected no image to be created, got %d", len(imageRepo.images)) + if env.images.createCalls != 0 { + t.Errorf("expected Create to never be called, got %d calls", env.images.createCalls) } }) t.Run("A repository failure while fetching the article answers 500", func(t *testing.T) { - articleRepo := newMockArticleRepo() - articleRepo.articles[1] = domainArticle.Article{ID: 1, Title: "Backyard Renovation Update"} - articleRepo.fail.failNext("Get") - imageRepo := newMockImageRepo() - router := newUploadArticleImageRouter(articleRepo, imageRepo, token) + env := newEnv(t) + env.articles.fail.failNext("Get") - req := newMultipartImageUploadRequest(t, "/v1/article/1/image", "before.png", "image/png") - setUploadJwtCookie(t, req, token) - rec := httptest.NewRecorder() - router.ServeHTTP(rec, req) + req := newMultipartFileRequest(t, http.MethodPost, "/v1/article/1/image", "before.png", "image/png", pngBytes(t, 3, 2)) + rec := env.serve(t, req, mediaVerifiedUserId) if rec.Code != http.StatusInternalServerError { t.Fatalf("expected status 500, got %d", rec.Code) @@ -266,94 +163,14 @@ func TestUploadArticleImageHttp(t *testing.T) { }) t.Run("A repository failure while creating the image answers 500", func(t *testing.T) { - articleRepo := newMockArticleRepo() - articleRepo.articles[1] = domainArticle.Article{ID: 1, Title: "Backyard Renovation Update"} - imageRepo := newMockImageRepo() - imageRepo.fail.failNext("Create") - router := newUploadArticleImageRouter(articleRepo, imageRepo, token) + env := newEnv(t) + env.images.fail.failNext("Create") - req := newMultipartImageUploadRequest(t, "/v1/article/1/image", "before.png", "image/png") - setUploadJwtCookie(t, req, token) - rec := httptest.NewRecorder() - router.ServeHTTP(rec, req) + req := newMultipartFileRequest(t, http.MethodPost, "/v1/article/1/image", "before.png", "image/png", pngBytes(t, 3, 2)) + rec := env.serve(t, req, mediaVerifiedUserId) if rec.Code != http.StatusInternalServerError { t.Fatalf("expected status 500, got %d", rec.Code) } }) - - t.Run("Uploading a file over the size limit answers 400", func(t *testing.T) { - articleRepo := newMockArticleRepo() - articleRepo.articles[1] = domainArticle.Article{ID: 1, Title: "Backyard Renovation Update"} - imageRepo := newMockImageRepo() - router := newUploadArticleImageRouter(articleRepo, imageRepo, token) - - body := &bytes.Buffer{} - writer := multipart.NewWriter(body) - part, err := writer.CreatePart(map[string][]string{ - "Content-Disposition": {`form-data; name="file"; filename="huge.png"`}, - "Content-Type": {"image/png"}, - }) - if err != nil { - t.Fatalf("failed to create multipart part: %v", err) - } - if _, err := io.Copy(part, strings.NewReader(strings.Repeat("a", int(domainImage.MaxImageSize)+1))); err != nil { - t.Fatalf("failed to write file content: %v", err) - } - if err := writer.Close(); err != nil { - t.Fatalf("failed to close multipart writer: %v", err) - } - req := httptest.NewRequest(http.MethodPost, "/v1/article/1/image", body) - req.Header.Set("Content-Type", writer.FormDataContentType()) - setUploadJwtCookie(t, req, token) - rec := httptest.NewRecorder() - router.ServeHTTP(rec, req) - - if rec.Code != http.StatusBadRequest { - t.Fatalf("expected status 400, got %d", rec.Code) - } - if rec.Body.String() != domainImage.ErrImageTooLarge.Error()+"\n" { - t.Errorf("unexpected body: %q", rec.Body.String()) - } - if len(imageRepo.images) != 0 { - t.Errorf("expected no image to be created, got %d", len(imageRepo.images)) - } - }) - - t.Run("A repository failure while attaching the image to the article answers 500", func(t *testing.T) { - articleRepo := newMockArticleRepo() - articleRepo.articles[1] = domainArticle.Article{ID: 1, Title: "Backyard Renovation Update"} - articleRepo.fail.failNext("AddImage") - imageRepo := newMockImageRepo() - router := newUploadArticleImageRouter(articleRepo, imageRepo, token) - - req := newMultipartImageUploadRequest(t, "/v1/article/1/image", "before.png", "image/png") - setUploadJwtCookie(t, req, token) - rec := httptest.NewRecorder() - router.ServeHTTP(rec, req) - - if rec.Code != http.StatusInternalServerError { - t.Fatalf("expected status 500, got %d", rec.Code) - } - }) - - t.Run("AddImage reporting a missing article after the initial lookup still answers 404", func(t *testing.T) { - articleRepo := newMockArticleRepo() - articleRepo.articles[1] = domainArticle.Article{ID: 1, Title: "Backyard Renovation Update"} - articleRepo.fail.failNextWith("AddImage", domainArticle.ErrArticleNotFound) - imageRepo := newMockImageRepo() - router := newUploadArticleImageRouter(articleRepo, imageRepo, token) - - req := newMultipartImageUploadRequest(t, "/v1/article/1/image", "before.png", "image/png") - setUploadJwtCookie(t, req, token) - rec := httptest.NewRecorder() - router.ServeHTTP(rec, req) - - if rec.Code != http.StatusNotFound { - t.Fatalf("expected status 404, got %d", rec.Code) - } - if rec.Body.String() != apperr.ArticleNotFoundErrorMessage+"\n" { - t.Errorf("unexpected body: %q", rec.Body.String()) - } - }) } diff --git a/test/use_case_baseline_test.go b/test/use_case_baseline_test.go index e02cad2..7286799 100644 --- a/test/use_case_baseline_test.go +++ b/test/use_case_baseline_test.go @@ -5,6 +5,7 @@ import ( imageDomain "RenewCMS/internal/domain/image" "RenewCMS/internal/domain/user" "RenewCMS/internal/infrastructure/useCases" + "bytes" "errors" "fmt" "reflect" @@ -236,7 +237,6 @@ func TestUpdateArticleDelegatesEachOperation(t *testing.T) { newRepo := func() *mockArticleRepo { repo := newMockArticleRepo() repo.articles[1] = article.Article{ID: 1, Title: "Existing"} - repo.images[1] = &imageDomain.Image{ID: 1, Path: "/static/one.png"} return repo } @@ -281,47 +281,13 @@ func TestUpdateArticleDelegatesEachOperation(t *testing.T) { t.Errorf("expected article.ErrArticleNotFound, got %v", err) } }) - - t.Run("AddImage_article1_image1", func(t *testing.T) { - repo := newRepo() - uc := useCases.NewUpdateArticleUseCase(repo) - err := uc.AddImage(1, 1) - if err != nil { - t.Fatalf("expected no error, got %v", err) - } - updated, err := repo.Get(1) - if err != nil { - t.Fatalf("expected no error, got %v", err) - } - if len(updated.Images) != 1 || updated.Images[0].ID != 1 { - t.Errorf("expected image 1 attached, got %+v", updated.Images) - } - }) - - t.Run("AddImage_article9_image1", func(t *testing.T) { - repo := newRepo() - uc := useCases.NewUpdateArticleUseCase(repo) - err := uc.AddImage(9, 1) - if !errors.Is(err, article.ErrArticleNotFound) { - t.Errorf("expected article.ErrArticleNotFound, got %v", err) - } - }) - - t.Run("AddImage_article1_image9", func(t *testing.T) { - repo := newRepo() - uc := useCases.NewUpdateArticleUseCase(repo) - err := uc.AddImage(1, 9) - if !errors.Is(err, imageDomain.ErrImageNotFound) { - t.Errorf("expected image.ErrImageNotFound, got %v", err) - } - }) } func TestDeleteArticleReportsMissingArticle(t *testing.T) { t.Run("id_1", func(t *testing.T) { repo := newMockArticleRepo() repo.articles[1] = article.Article{ID: 1} - uc := useCases.NewDeleteArticleUseCase(repo, newMockImageRepo()) + uc := useCases.NewDeleteArticleUseCase(repo) err := uc.DeleteArticle(1) if err != nil { @@ -332,7 +298,7 @@ func TestDeleteArticleReportsMissingArticle(t *testing.T) { t.Run("id_9", func(t *testing.T) { repo := newMockArticleRepo() repo.articles[1] = article.Article{ID: 1} - uc := useCases.NewDeleteArticleUseCase(repo, newMockImageRepo()) + uc := useCases.NewDeleteArticleUseCase(repo) err := uc.DeleteArticle(9) if !errors.Is(err, article.ErrArticleNotFound) { @@ -345,7 +311,7 @@ func TestDeleteArticlePropagatesRepositoryFailure(t *testing.T) { repo := newMockArticleRepo() repo.articles[1] = article.Article{ID: 1} repo.fail.failNext("Delete") - uc := useCases.NewDeleteArticleUseCase(repo, newMockImageRepo()) + uc := useCases.NewDeleteArticleUseCase(repo) err := uc.DeleteArticle(1) if !errors.Is(err, errRepositoryFailure) { @@ -370,8 +336,12 @@ func TestCreateImageDelegatesContentTypeCheck(t *testing.T) { repo := newMockImageRepo() uc := useCases.NewCreateImageUseCase(repo) + content := []byte("fake-image-data") + if tc.valid { + content = validBytesFor(t, tc.contentType) + } result, err := uc.CreateImage(imageDomain.ImageInput{ - Content: strings.NewReader("fake-image-data"), + Content: bytes.NewReader(content), Filename: "test.img", ContentType: tc.contentType, }) @@ -401,7 +371,7 @@ func TestCreateImagePropagatesRepositoryFailure(t *testing.T) { uc := useCases.NewCreateImageUseCase(repo) _, err := uc.CreateImage(imageDomain.ImageInput{ - Content: strings.NewReader("data"), + Content: bytes.NewReader(pngBytes(t, 3, 2)), Filename: "test.png", ContentType: "image/png", }) @@ -415,9 +385,9 @@ func TestDeleteImageRequiresExistence(t *testing.T) { t.Run("id_1", func(t *testing.T) { repo := newMockImageRepo() repo.images[1] = imageDomain.Image{ID: 1} - uc := useCases.NewDeleteImageUseCase(repo) + uc := useCases.NewDeleteMediaUseCase(repo) - err := uc.DeleteImage(1) + err := uc.Delete(1) if err != nil { t.Fatalf("expected no error, got %v", err) } @@ -429,9 +399,9 @@ func TestDeleteImageRequiresExistence(t *testing.T) { t.Run("id_9", func(t *testing.T) { repo := newMockImageRepo() repo.images[1] = imageDomain.Image{ID: 1} - uc := useCases.NewDeleteImageUseCase(repo) + uc := useCases.NewDeleteMediaUseCase(repo) - err := uc.DeleteImage(9) + err := uc.Delete(9) if !errors.Is(err, imageDomain.ErrImageNotFound) { t.Errorf("expected image.ErrImageNotFound, got %v", err) } diff --git a/test/verified_account_required_http_test.go b/test/verified_account_required_http_test.go index 7ae340e..761918f 100644 --- a/test/verified_account_required_http_test.go +++ b/test/verified_account_required_http_test.go @@ -27,15 +27,13 @@ func newVerifiedAccountRequiredRouter(articleRepo *mockArticleRepo, userRepo *mo useCases.NewCreateArticleUseCase(articleRepo), useCases.NewGetArticleUseCase(articleRepo), useCases.NewListArticlesUseCase(articleRepo), - useCases.NewDeleteArticleUseCase(articleRepo, imageRepo), + useCases.NewDeleteArticleUseCase(articleRepo), useCases.NewUpdateArticleUseCase(articleRepo), getUserUseCase, ) imageHandler := image.NewHandler( useCases.NewCreateImageUseCase(imageRepo), useCases.NewGetArticleUseCase(articleRepo), - useCases.NewUpdateArticleUseCase(articleRepo), - useCases.NewDeleteImageUseCase(imageRepo), ) r := chi.NewRouter() @@ -52,7 +50,7 @@ func newVerifiedAccountRequiredRouter(articleRepo *mockArticleRepo, userRepo *mo r.Patch("/{id}", articleHandler.UpdateArticleBody) r.Patch("/{id}/publish", articleHandler.PublishArticle) r.Patch("/{id}/unpublish", articleHandler.UnpublishArticle) - r.Post("/{id}/image", imageHandler.PostImage) + r.Post("/{id}/image", imageHandler.PostLegacyArticleImage) }) }) }) diff --git a/test/write_not_found_errors_test.go b/test/write_not_found_errors_test.go index fa61e20..6364d57 100644 --- a/test/write_not_found_errors_test.go +++ b/test/write_not_found_errors_test.go @@ -85,20 +85,6 @@ func TestTheArticleRepositoryReportsAMissingArticleAsADomainErrorOnAWrite(t *tes }) } -func TestAddImageReportsAMissingArticleAsADomainErrorNotAMissingImage(t *testing.T) { - repo := newSeededArticleRepository(t) - - err := repo.AddImage(9, 1) - assertWriteResultIsArticleNotFound(t, err) -} - -func TestAddImageReportsAMissingImageAsADomainErrorOnceTheArticleExists(t *testing.T) { - repo := newSeededArticleRepository(t) - - err := repo.AddImage(1, 9) - assertWriteResultIsImageNotFound(t, err) -} - func newSeededUserRepository(t *testing.T) *persistence.UserRepository { t.Helper() db := openInMemoryDb(t) @@ -205,13 +191,6 @@ func TestTheFakesMirrorTheRealRepositoriesOnEveryWriteNotFoundPath(t *testing.T) if err := articleRepo.Delete(9); !errors.Is(err, domainArticle.ErrArticleNotFound) { t.Errorf("expected article.ErrArticleNotFound, got %v", err) } - if err := articleRepo.AddImage(9, 1); !errors.Is(err, domainArticle.ErrArticleNotFound) { - t.Errorf("expected article.ErrArticleNotFound, got %v", err) - } - articleRepo.images[1] = &domainImage.Image{ID: 1} - if err := articleRepo.AddImage(1, 9); !errors.Is(err, domainImage.ErrImageNotFound) { - t.Errorf("expected image.ErrImageNotFound, got %v", err) - } userRepo := newMockUserRepo() userRepo.users[1] = domainUser.User{ID: 1, Username: "alice"} @@ -237,7 +216,7 @@ func newDeleteArticleRouter(articleRepo *mockArticleRepo, token *jwtauth.JWTAuth useCases.NewCreateArticleUseCase(articleRepo), useCases.NewGetArticleUseCase(articleRepo), useCases.NewListArticlesUseCase(articleRepo), - useCases.NewDeleteArticleUseCase(articleRepo, newMockImageRepo()), + useCases.NewDeleteArticleUseCase(articleRepo), useCases.NewUpdateArticleUseCase(articleRepo), getUserUseCase, )