feat: media picker and image removal

test/remove_legacy_article_image_alias_http_test.go

@@ -0,0 +1,76 @@
+package test
+
+import (
+	"net/http"
+	"testing"
+)
+
+func TestRemoveLegacyArticleImageAliasHttp(t *testing.T) {
+	newEnv := func(t *testing.T) *mediaEnv {
+		env := newMediaEnv(t)
+		env.seedArticle(1, "Backyard Renovation Update", false, "unchanged body")
+		return env
+	}
+
+	t.Run("The removed routes are not found", func(t *testing.T) {
+		examples := []struct {
+			name   string
+			method string
+			path   string
+		}{
+			{"POST /v1/article/1/image", http.MethodPost, "/v1/article/1/image"},
+			{"DELETE /v1/article/1/image/10", http.MethodDelete, "/v1/article/1/image/10"},
+		}
+		for _, example := range examples {
+			t.Run(example.name, func(t *testing.T) {
+				env := newEnv(t)
+
+				req := newMultipartFileRequest(t, example.method, example.path, "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)
+				}
+				if env.images.createCalls != 0 {
+					t.Errorf("expected Create to never be called, got %d calls", env.images.createCalls)
+				}
+				if len(env.images.images) != 0 {
+					t.Errorf("expected no image to exist, got %d", len(env.images.images))
+				}
+			})
+		}
+	})
+
+	t.Run("Uploading still works through the media route", func(t *testing.T) {
+		env := newEnv(t)
+
+		req := newMultipartFileRequest(t, http.MethodPost, "/v1/media/", "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())
+		}
+		got := decodeObject(t, rec.Body.String())
+		if got["url"] == nil || numberOf(got["id"]) != 1 {
+			t.Errorf("expected a library item with a url and id 1, got %v", got)
+		}
+		listing := env.send(t, http.MethodGet, "/v1/media/", "", mediaVerifiedUserId)
+		items := decodeObject(t, listing.Body.String())["items"].([]any)
+		if len(items) != 1 {
+			t.Errorf("expected the list to hold the upload, got %v", items)
+		}
+	})
+
+	t.Run("The rest of the article router is unchanged", func(t *testing.T) {
+		env := newEnv(t)
+
+		rec := env.send(t, http.MethodPatch, "/v1/article/1/publish", "", mediaVerifiedUserId)
+
+		if rec.Code != http.StatusOK {
+			t.Fatalf("expected status 200, got %d, body %q", rec.Code, rec.Body.String())
+		}
+		if !env.articles.articles[1].IsOnline {
+			t.Errorf("expected article 1 to be online")
+		}
+	})
+}

test/upload_article_image_http_test.go

@@ -1,176 +0,0 @@
-package test
-
-import (
-	"RenewCMS/api/pkg/apperr"
-	"net/http"
-	"testing"
-
-	domainImage "RenewCMS/internal/domain/image"
-)
-
-func TestUploadArticleImageHttp(t *testing.T) {
-	newEnv := func(t *testing.T) *mediaEnv {
-		env := newMediaEnv(t)
-		env.seedArticle(1, "Backyard Renovation Update", true, "unchanged body")
-		return env
-	}
-
-	t.Run("Uploading through the alias creates a library image", func(t *testing.T) {
-		env := newEnv(t)
-
-		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())
-		}
-		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))
-		}
-		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)
-		}
-
-		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)
-		}
-		if env.articles.articles[1].Body != "unchanged body" {
-			t.Errorf("expected article 1 to be unchanged, got %+v", env.articles.articles[1])
-		}
-	})
-
-	t.Run("An unknown article is 404 and creates no image", func(t *testing.T) {
-		env := newEnv(t)
-
-		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)
-		}
-		if rec.Body.String() != apperr.ArticleNotFoundErrorMessage+"\n" {
-			t.Errorf("unexpected body: %q", rec.Body.String())
-		}
-		if env.images.createCalls != 0 {
-			t.Errorf("expected Create to never be called, got %d calls", env.images.createCalls)
-		}
-	})
-
-	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 := newMultipartFileRequest(t, http.MethodPost, "/v1/article/1/image", example.filename, example.contentType, example.content)
-				rec := env.serve(t, req, mediaVerifiedUserId)
-
-				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("The alias still requires authentication", func(t *testing.T) {
-		env := newEnv(t)
-
-		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 env.images.createCalls != 0 {
-			t.Errorf("expected Create to never be called, got %d calls", env.images.createCalls)
-		}
-	})
-
-	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) {
-		env := newEnv(t)
-
-		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 || 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) {
-		env := newEnv(t)
-
-		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 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) {
-		env := newEnv(t)
-		env.articles.fail.failNext("Get")
-
-		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("A repository failure while creating the image answers 500", func(t *testing.T) {
-		env := newEnv(t)
-		env.images.fail.failNext("Create")
-
-		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)
-		}
-	})
-}

test/verified_account_required_http_test.go

@@ -2,7 +2,6 @@ package test
 
 import (
 	"RenewCMS/api/handlers/article"
-	"RenewCMS/api/handlers/image"
 	"RenewCMS/api/middleware"
 	"RenewCMS/api/pkg/apperr"
 	validator "RenewCMS/api/pkg/validator"
@@ -11,8 +10,6 @@ import (
 	"RenewCMS/internal/infrastructure/useCases"
 	"bytes"
 	"encoding/json"
-	"io"
-	"mime/multipart"
 	"net/http"
 	"net/http/httptest"
 	"testing"
@@ -21,7 +18,7 @@ import (
 	"github.com/go-chi/jwtauth/v5"
 )
 
-func newVerifiedAccountRequiredRouter(articleRepo *mockArticleRepo, userRepo *mockUserRepo, imageRepo *mockImageRepo, token *jwtauth.JWTAuth) *chi.Mux {
+func newVerifiedAccountRequiredRouter(articleRepo *mockArticleRepo, userRepo *mockUserRepo, token *jwtauth.JWTAuth) *chi.Mux {
 	getUserUseCase := useCases.NewGetUserUseCase(userRepo)
 	articleHandler := article.NewHandler(
 		useCases.NewCreateArticleUseCase(articleRepo),
@@ -31,10 +28,6 @@ func newVerifiedAccountRequiredRouter(articleRepo *mockArticleRepo, userRepo *mo
 		useCases.NewUpdateArticleUseCase(articleRepo),
 		getUserUseCase,
 	)
-	imageHandler := image.NewHandler(
-		useCases.NewCreateImageUseCase(imageRepo),
-		useCases.NewGetArticleUseCase(articleRepo),
-	)
 
 	r := chi.NewRouter()
 	r.Route("/v1", func(r chi.Router) {
@@ -50,7 +43,6 @@ 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.PostLegacyArticleImage)
 			})
 		})
 	})
@@ -106,7 +98,7 @@ func TestVerifiedAccountRequiredHttp(t *testing.T) {
 		t.Run("An authenticated but unverified user is refused on every protected article route: "+tc.method+" "+tc.path, func(t *testing.T) {
 			articleRepo := seedVerifiedAccountRequiredArticleRepo()
 			userRepo := seedVerifiedAccountRequiredUserRepo(false)
-			router := newVerifiedAccountRequiredRouter(articleRepo, userRepo, newMockImageRepo(), token)
+			router := newVerifiedAccountRequiredRouter(articleRepo, userRepo, token)
 
 			req := newVerifiedAccountRequiredRequest(tc.method, tc.path, tc.body)
 			req.AddCookie(mintVerifiedAccountRequiredCookie(t, token, map[string]any{"user_id": uint32(1)}))
@@ -125,51 +117,10 @@ func TestVerifiedAccountRequiredHttp(t *testing.T) {
 		})
 	}
 
-	t.Run("An unverified user is also refused when attaching an image", func(t *testing.T) {
-		articleRepo := seedVerifiedAccountRequiredArticleRepo()
-		userRepo := seedVerifiedAccountRequiredUserRepo(false)
-		imageRepo := newMockImageRepo()
-		router := newVerifiedAccountRequiredRouter(articleRepo, userRepo, imageRepo, token)
-
-		body := &bytes.Buffer{}
-		writer := multipart.NewWriter(body)
-		part, err := writer.CreatePart(map[string][]string{
-			"Content-Disposition": {`form-data; name="file"; filename="before.png"`},
-			"Content-Type":        {"image/png"},
-		})
-		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, "/v1/article/1/image", body)
-		req.Header.Set("Content-Type", writer.FormDataContentType())
-		req.AddCookie(mintVerifiedAccountRequiredCookie(t, token, map[string]any{"user_id": uint32(1)}))
-		rec := httptest.NewRecorder()
-		router.ServeHTTP(rec, req)
-
-		if rec.Code != http.StatusForbidden {
-			t.Fatalf("expected status 403, got %d, body %q", rec.Code, rec.Body.String())
-		}
-		if rec.Body.String() != apperr.VerificationRequiredErrorMessage+"\n" {
-			t.Errorf("unexpected body: %q", rec.Body.String())
-		}
-		if imageRepo.images == nil {
-			t.Fatalf("expected imageRepo.images to be initialized")
-		}
-		if len(imageRepo.images) != 0 {
-			t.Errorf("expected the image repository's Create to never be called, got %d images", len(imageRepo.images))
-		}
-	})
-
 	t.Run("A verified user still exercises the protected routes exactly as before", func(t *testing.T) {
 		articleRepo := newMockArticleRepo()
 		userRepo := seedVerifiedAccountRequiredUserRepo(true)
-		router := newVerifiedAccountRequiredRouter(articleRepo, userRepo, newMockImageRepo(), token)
+		router := newVerifiedAccountRequiredRouter(articleRepo, userRepo, token)
 
 		req := newVerifiedAccountRequiredRequest(http.MethodPost, "/v1/article/", `{"title": "Valid Title", "body": "Some content"}`)
 		req.AddCookie(mintVerifiedAccountRequiredCookie(t, token, map[string]any{"user_id": uint32(1)}))
@@ -202,7 +153,7 @@ func TestVerifiedAccountRequiredHttp(t *testing.T) {
 		t.Run("A token that resolves to no usable account still answers 401, never 403: "+tc.name, func(t *testing.T) {
 			articleRepo := seedVerifiedAccountRequiredArticleRepo()
 			userRepo := seedVerifiedAccountRequiredUserRepo(false)
-			router := newVerifiedAccountRequiredRouter(articleRepo, userRepo, newMockImageRepo(), token)
+			router := newVerifiedAccountRequiredRouter(articleRepo, userRepo, token)
 
 			req := newVerifiedAccountRequiredRequest(http.MethodPost, "/v1/article/", `{"title": "Valid Title", "body": "Some content"}`)
 			req.AddCookie(mintVerifiedAccountRequiredCookie(t, token, tc.claims))
@@ -222,7 +173,7 @@ func TestVerifiedAccountRequiredHttp(t *testing.T) {
 		articleRepo := newMockArticleRepo()
 		userRepo := newMockUserRepo()
 		userRepo.users[7] = domainUser.User{ID: 7, IsVerified: true}
-		router := newVerifiedAccountRequiredRouter(articleRepo, userRepo, newMockImageRepo(), token)
+		router := newVerifiedAccountRequiredRouter(articleRepo, userRepo, token)
 
 		req := newVerifiedAccountRequiredRequest(http.MethodPost, "/v1/article/", `{"title": "Valid Title", "body": "Some content"}`)
 		req.AddCookie(mintVerifiedAccountRequiredCookie(t, token, map[string]any{"user_id": 7.5}))
@@ -250,7 +201,7 @@ func TestVerifiedAccountRequiredHttp(t *testing.T) {
 		t.Run("A request with no jwt cookie is still refused with 401, never 403: "+tc.method+" "+tc.path, func(t *testing.T) {
 			articleRepo := seedVerifiedAccountRequiredArticleRepo()
 			userRepo := seedVerifiedAccountRequiredUserRepo(false)
-			router := newVerifiedAccountRequiredRouter(articleRepo, userRepo, newMockImageRepo(), token)
+			router := newVerifiedAccountRequiredRouter(articleRepo, userRepo, token)
 
 			req := newVerifiedAccountRequiredRequest(tc.method, tc.path, "")
 			rec := httptest.NewRecorder()
@@ -266,7 +217,7 @@ func TestVerifiedAccountRequiredHttp(t *testing.T) {
 		articleRepo := seedVerifiedAccountRequiredArticleRepo()
 		userRepo := seedVerifiedAccountRequiredUserRepo(false)
 		userRepo.fail.failNext("Get")
-		router := newVerifiedAccountRequiredRouter(articleRepo, userRepo, newMockImageRepo(), token)
+		router := newVerifiedAccountRequiredRouter(articleRepo, userRepo, token)
 
 		req := newVerifiedAccountRequiredRequest(http.MethodDelete, "/v1/article/1", "")
 		req.AddCookie(mintVerifiedAccountRequiredCookie(t, token, map[string]any{"user_id": uint32(1)}))
@@ -308,7 +259,7 @@ func TestVerifiedAccountRequiredHttp(t *testing.T) {
 	t.Run("Listing falls back to the online-only list for an authenticated but unverified caller", func(t *testing.T) {
 		articleRepo := seedListingArticleRepo()
 		userRepo := seedVerifiedAccountRequiredUserRepo(false)
-		router := newVerifiedAccountRequiredRouter(articleRepo, userRepo, newMockImageRepo(), token)
+		router := newVerifiedAccountRequiredRouter(articleRepo, userRepo, token)
 
 		req := newVerifiedAccountRequiredRequest(http.MethodGet, "/v1/article/", "")
 		req.AddCookie(mintVerifiedAccountRequiredCookie(t, token, map[string]any{"user_id": uint32(1)}))
@@ -321,7 +272,7 @@ func TestVerifiedAccountRequiredHttp(t *testing.T) {
 	t.Run("Listing still returns every article for an authenticated and verified caller", func(t *testing.T) {
 		articleRepo := seedListingArticleRepo()
 		userRepo := seedVerifiedAccountRequiredUserRepo(true)
-		router := newVerifiedAccountRequiredRouter(articleRepo, userRepo, newMockImageRepo(), token)
+		router := newVerifiedAccountRequiredRouter(articleRepo, userRepo, token)
 
 		req := newVerifiedAccountRequiredRequest(http.MethodGet, "/v1/article/", "")
 		req.AddCookie(mintVerifiedAccountRequiredCookie(t, token, map[string]any{"user_id": uint32(1)}))
@@ -347,7 +298,7 @@ func TestVerifiedAccountRequiredHttp(t *testing.T) {
 	t.Run("Listing falls back to the online-only list when the caller's account no longer exists", func(t *testing.T) {
 		articleRepo := seedListingArticleRepo()
 		userRepo := newMockUserRepo()
-		router := newVerifiedAccountRequiredRouter(articleRepo, userRepo, newMockImageRepo(), token)
+		router := newVerifiedAccountRequiredRouter(articleRepo, userRepo, token)
 
 		req := newVerifiedAccountRequiredRequest(http.MethodGet, "/v1/article/", "")
 		req.AddCookie(mintVerifiedAccountRequiredCookie(t, token, map[string]any{"user_id": uint32(999)}))
@@ -361,7 +312,7 @@ func TestVerifiedAccountRequiredHttp(t *testing.T) {
 		articleRepo := seedListingArticleRepo()
 		userRepo := seedVerifiedAccountRequiredUserRepo(true)
 		userRepo.fail.failNext("Get")
-		router := newVerifiedAccountRequiredRouter(articleRepo, userRepo, newMockImageRepo(), token)
+		router := newVerifiedAccountRequiredRouter(articleRepo, userRepo, token)
 
 		req := newVerifiedAccountRequiredRequest(http.MethodGet, "/v1/article/", "")
 		req.AddCookie(mintVerifiedAccountRequiredCookie(t, token, map[string]any{"user_id": uint32(1)}))