refac: article title rules are the same in the API request and the domain; an invalid title returns 400 instead of 500

This commit is contained in:
fsylvain committed 2026-09-23 20:04:09 +02:00
1 parent ff8cd5f0ec
commit 4278d96882
9 files changed
+452 -3

No files matched your search

+5 -1
View File
@@ -23,7 +23,7 @@ type Handler struct {
}
type PostArticle struct {
Title string `json:"title" validate:"required,min=3,max=50"`
Title string `json:"title" validate:"required,min=3,max=200"`
Body string `json:"body" validate:"required,max=10000"`
}
@@ -81,6 +81,10 @@ func (h *Handler) PostArticle(w http.ResponseWriter, r *http.Request) {
Body: localArticle.Body,
})
if err != nil {
if errors.Is(err, domain.ErrInvalidTitle) {
http.Error(w, apperr.ArticleTitleErrorMessage, http.StatusBadRequest)
return
}
http.Error(w, "Failed to create article", http.StatusInternalServerError)
return
}
+1
View File
@@ -4,3 +4,4 @@ const LogsErrorMessage = "Access to the requested resource is forbidden due to i
const BodyErrorMessage = "The request cannot be processed due to a mismatch in the format of the body."
const IdUint32ErrorMessage = "The server expects the ID to be in the format of an unsigned 32-bit integer (uint32)."
const ArticleNotFoundErrorMessage = "Article not found"
const ArticleTitleErrorMessage = "Article title must be between 3 and 200 characters."
+85
View File
@@ -0,0 +1,85 @@
Feature: The article title rule is the same in the request and the domain
As the maintainer of RenewCMS
I want PostArticle's title validation and article.CreateArticle's title validation to
enforce the identical rule, counted the identical way
So that a title the request layer accepts can never be rejected by the domain as a
surprise 500, and a domain title rejection that does happen is always reported as a
400 with a client-facing apperr message instead of a raw 500
Background:
Given internal/domain/article defines the sentinel article.ErrInvalidTitle
And CreateArticleUseCase.CreateArticle rejects a title whose length, counted with
utf8.RuneCountInString (runes, not bytes), is below 3 or above 200, returning
article.ErrInvalidTitle and storing nothing
And api/handlers/article/articleHandler.go's PostArticle.Title carries
validate:"required,min=3,max=200", and go-playground/validator's min/max on a
string already count runes with utf8.RuneCountInString, so the request tag and the
domain check now agree on both the bound (3-200) and the unit (runes)
And api/pkg/apperr defines ArticleTitleErrorMessage as "Article title must be
between 3 and 200 characters.", a client-facing string distinct from both the raw
validator error text and "Failed to create article"
And Handler.PostArticle maps errors.Is(err, article.ErrInvalidTitle) from
CreateArticleUseCase.CreateArticle to a 400 response with body
apperr.ArticleTitleErrorMessage; any other error from CreateArticle still answers
500 with body "Failed to create article"
And POST /v1/article/ sits behind the jwtauth Verifier and Authenticator group and
requires a valid jwt cookie
And UpdateArticleUseCase exposes only UpdateBody, AddImage and UpdateIsOnline; no
route or use case updates an article's title after creation, so this feature only
concerns article creation
Scenario Outline: CreateArticle now measures title length in runes, not bytes
Given an empty article repository fake
When CreateArticle is called with a title made of <rune_count> repetitions of the
4-byte rune "😀" and a body "Some body text"
Then the result is <outcome>
Examples:
| rune_count | outcome |
| 2 | ErrInvalidTitle, nothing stored |
| 3 | created |
| 200 | created |
| 201 | ErrInvalidTitle, nothing stored |
Scenario Outline: POST /v1/article/ enforces the same 3-200 rune boundary over HTTP
Given a valid jwt cookie for a user with id 1
When the client sends POST /v1/article/ with that cookie, a title of <length> ASCII
characters and a body "Some body text"
Then the response status is <status>
And <storage_effect>
Examples:
| length | status | storage_effect |
| 2 | 400 | not created |
| 3 | 201 | created, is_online false |
| 200 | 201 | created, is_online false |
| 201 | 400 | not created |
Scenario: A 200-rune multibyte title is accepted end to end over HTTP
Given a valid jwt cookie for a user with id 1
When the client sends POST /v1/article/ with that cookie, a title made of 200
repetitions of the 4-byte rune "😀" (200 runes, 800 bytes) and a body
"Some body text"
Then the response status is 201
And the response body is the created article's JSON with that title
Scenario: The handler maps a domain title rejection to 400, not 500
Given a valid jwt cookie for a user with id 1
And the article repository fake is configured so its next Create call returns
article.ErrInvalidTitle directly, standing in for any layer beneath the use
case's own check rejecting the title with that same sentinel
When the client sends POST /v1/article/ with that cookie, title "Valid Title" and
body "Some body text" (both already satisfying PostArticle's own validate tags)
Then the response status is 400
And the response body is the text apperr.ArticleTitleErrorMessage
And the response body is not "Failed to create article"
Scenario: A non-title repository failure during creation still answers 500
Given a valid jwt cookie for a user with id 1
And the article repository fake is configured to fail on its next Create call with
a generic repository error, not article.ErrInvalidTitle
When the client sends POST /v1/article/ with that cookie, title "Valid Title" and
body "Some body text"
Then the response status is 500
And the response body is the text "Failed to create article"
+76
View File
@@ -0,0 +1,76 @@
# QA procedure: article_title_rules
Package: `test` (black-box, alongside `article_test.go` / `use_case_baseline_test.go` /
`not_found_errors_test.go` / the hasher-domain-interface tests).
## Fakes needed
1. Extend `mockArticleRepo` (`test/article_test.go`) / its shared `failSwitch`
(`test/fakes_common.go`) so a configured failure can return a specific error, not
only the generic `errRepositoryFailure`. Today `failSwitch.trigger` always returns
the same sentinel; add a way to say "the next call to `Create` returns
`article.ErrInvalidTitle`" specifically, distinct from "the next call to `Create`
returns a generic repository error" (e.g. a `failNextWith(method string, err error)`
on `failSwitch`, keeping `errRepositoryFailure` as the default so existing
generic-failure tests don't change).
2. Add a `createCalled` flag (or equivalent) to `mockArticleRepo` if not already
present, so a test can assert `Create` was never reached when request-layer
validation rejects a title first.
## Use-case-level checks (fake repo, no HTTP)
3. `useCases.NewCreateArticleUseCase(repo)`, `CreateArticle(...)` with an empty fake
repo:
- Title `strings.Repeat("😀", 2)` (2 runes, 8 bytes) -> `errors.Is(err,
article.ErrInvalidTitle)`, nothing stored.
- Title `strings.Repeat("😀", 3)` (3 runes, 12 bytes) -> no error, created,
`is_online` false, one article stored.
- Title `strings.Repeat("😀", 200)` (200 runes, 800 bytes) -> no error, created.
Proves the check counts runes (`utf8.RuneCountInString`), not `len()` bytes.
- Title `strings.Repeat("😀", 201)` (201 runes, 804 bytes) -> `errors.Is(err,
article.ErrInvalidTitle)`, nothing stored.
- Keep the existing ASCII boundary cases in `TestCreateArticleTitleLengthLimit`
(0/2/3/200/201) passing unchanged, as a regression check that byte length and
rune length coincide for ASCII.
## HTTP checks (real CreateArticleUseCase over the fake repo, mirroring api/router.go)
4. Call `validator.InitValidator()`.
5. Build `useCases.NewCreateArticleUseCase(fakeRepo)` plus `GetArticleUseCase`,
`ListArticlesUseCase`, `DeleteArticleUseCase` (real, over the same fake repo). Build
`article.NewHandler(create, get, list, delete)`. Mount its routes inside a group
wrapped by `jwtauth.Verifier(token)` + `jwtauth.Authenticator(token)` on a fresh
`chi.NewRouter()`, with a test `jwtauth.New("HS256", []byte("test-secret"), nil)`, as
in `api/router.go`.
6. Mint a valid jwt cookie for user id 1 (`token.Encode(map[string]any{"user_id":
uint32(1)})`, set as the `jwt` cookie on each `httptest.NewRequest`).
7. `POST /v1/article/` with a 2-ASCII-character title (`"ab"`) and a valid body ->
status `400`, `fakeRepo.createCalled` is false. Assert the body is neither
`apperr.ArticleTitleErrorMessage` nor `"Failed to create article"`, to keep the two
400 sources (request-layer vs. domain-layer) distinguishable in the test.
8. `POST /v1/article/` with a 3-ASCII-character title -> status `201`, response body
decodes to the created article JSON, `is_online` false.
9. `POST /v1/article/` with a 200-ASCII-character title (`strings.Repeat("a", 200)`) ->
status `201`, created.
10. `POST /v1/article/` with a 201-ASCII-character title -> status `400`, `Create`
never called.
11. `POST /v1/article/` with a title of 200 repetitions of "😀" (200 runes, 800 bytes)
and a valid body -> status `201`, created, response JSON's `title` equals the sent
title.
12. Configure the fake repo (per step 1) so its next `Create` call returns
`article.ErrInvalidTitle` directly. `POST /v1/article/` with title "Valid Title"
and a valid body (both well inside 3-200 runes, so `validator.Validate.Struct`
passes and `CreateArticleUseCase`'s own check would also pass) -> status `400`,
body `apperr.ArticleTitleErrorMessage + "\n"`. This scenario is deliberately
artificial: with the rule aligned, no real HTTP input can make
`CreateArticleUseCase.CreateArticle`'s own check return `article.ErrInvalidTitle`
once `validator.Validate.Struct` has already accepted the same title, so the only
way to exercise `Handler.PostArticle`'s mapping branch today is to have the fake
repository hand back that sentinel directly, standing in for a future or
repository-level source of the same error.
13. Configure the fake repo to fail on its next `Create` with the generic
`errRepositoryFailure` (existing `failNext("Create")`). `POST /v1/article/` with
title "Valid Title" and a valid body -> status `500`, body `"Failed to create
article\n"` -- regression proof a non-title failure still answers 500.
14. `POST /v1/article/` with no jwt cookie -> status `401` (regression: the route is
still gated).
## Manual verification (not exercised by the Go test)
- None. This slice has no SPA-visible behaviour and no SMTP involvement.
+14
View File
@@ -4,6 +4,7 @@ import (
domain "RenewCMS/internal/domain/image"
"errors"
"time"
"unicode/utf8"
)
var (
@@ -12,6 +13,19 @@ var (
ErrArticleOffline = errors.New("article is offline")
)
const (
MinTitleRunes = 3
MaxTitleRunes = 200
)
func ValidateTitle(title string) error {
runeCount := utf8.RuneCountInString(title)
if runeCount < MinTitleRunes || runeCount > MaxTitleRunes {
return ErrInvalidTitle
}
return nil
}
type Article struct {
ID uint32 `json:"id"`
Title string `json:"title"`
@@ -18,8 +18,8 @@ func NewCreateArticleUseCase(articleRepository article.Repository) *CreateArticl
}
func (g *CreateArticleUseCase) CreateArticle(createArticle CreateArticleCommand) (article.Article, error) {
if len(createArticle.Title) < 3 || len(createArticle.Title) > 200 {
return article.Article{}, article.ErrInvalidTitle
if err := article.ValidateTitle(createArticle.Title); err != nil {
return article.Article{}, err
}
return g.articleRepository.Create(article.Article{
+2
View File
@@ -16,6 +16,7 @@ type mockArticleRepo struct {
images map[uint32]*imageDomain.Image
nextID uint32
fail failSwitch
createCalled bool
}
func newMockArticleRepo() *mockArticleRepo {
@@ -82,6 +83,7 @@ func (m *mockArticleRepo) GetAll() []article.Article {
}
func (m *mockArticleRepo) Create(a article.Article) (article.Article, error) {
m.createCalled = true
if err := m.fail.trigger("Create"); err != nil {
return article.Article{}, err
}
+255
View File
@@ -0,0 +1,255 @@
package test
import (
"RenewCMS/api/handlers/article"
"RenewCMS/api/pkg/apperr"
validator "RenewCMS/api/pkg/validator"
domainArticle "RenewCMS/internal/domain/article"
"RenewCMS/internal/infrastructure/useCases"
"bytes"
"encoding/json"
"errors"
"fmt"
"net/http"
"net/http/httptest"
"reflect"
"strings"
"testing"
"github.com/go-chi/chi/v5"
"github.com/go-chi/jwtauth/v5"
)
func newArticleTitleRulesRouter(repo *mockArticleRepo, token *jwtauth.JWTAuth) *chi.Mux {
articleHandler := article.NewHandler(
useCases.NewCreateArticleUseCase(repo),
useCases.NewGetArticleUseCase(repo),
useCases.NewListArticlesUseCase(repo),
useCases.NewDeleteArticleUseCase(repo),
)
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.Route("/article", func(r chi.Router) {
r.Post("/", articleHandler.PostArticle)
})
})
})
return r
}
func postArticleRequest(t *testing.T, token *jwtauth.JWTAuth, title string, body string) *http.Request {
t.Helper()
payload, err := json.Marshal(map[string]string{"title": title, "body": body})
if err != nil {
t.Fatalf("failed to marshal body: %v", err)
}
req := httptest.NewRequest(http.MethodPost, "/v1/article/", bytes.NewReader(payload))
req.Header.Set("Content-Type", "application/json")
setJwtCookie(t, req, token, 1)
return req
}
func TestArticleTitleTagMatchesDomainBounds(t *testing.T) {
field, ok := reflect.TypeOf(article.PostArticle{}).FieldByName("Title")
if !ok {
t.Fatal("expected PostArticle to have a Title field")
}
expected := fmt.Sprintf("required,min=%d,max=%d", domainArticle.MinTitleRunes, domainArticle.MaxTitleRunes)
got := field.Tag.Get("validate")
if got != expected {
t.Errorf("expected validate tag %q, got %q", expected, got)
}
}
func TestArticleTitleRunesInUseCase(t *testing.T) {
cases := []struct {
runeCount int
created bool
}{
{2, false},
{3, true},
{200, true},
{201, false},
}
for _, tc := range cases {
t.Run(fmt.Sprintf("rune_count_%d", tc.runeCount), func(t *testing.T) {
repo := newMockArticleRepo()
uc := useCases.NewCreateArticleUseCase(repo)
title := strings.Repeat("\U0001F600", tc.runeCount)
result, err := uc.CreateArticle(useCases.CreateArticleCommand{Title: title, Body: "Some body text"})
if tc.created {
if err != nil {
t.Fatalf("expected no error, got %v", err)
}
if !repo.createCalled {
t.Errorf("expected the repository's Create to have been called")
}
if len(repo.articles) != 1 {
t.Errorf("expected 1 article stored, got %d", len(repo.articles))
}
if result.Title != title {
t.Errorf("expected title %q, got %q", title, result.Title)
}
} else {
if !errors.Is(err, domainArticle.ErrInvalidTitle) {
t.Errorf("expected ErrInvalidTitle, got %v", err)
}
if repo.createCalled {
t.Errorf("expected the repository's Create not to have been called")
}
if len(repo.articles) != 0 {
t.Errorf("expected nothing stored, got %d article(s)", len(repo.articles))
}
}
})
}
}
func TestArticleTitleRunesOverHttp(t *testing.T) {
validator.InitValidator()
token := jwtauth.New("HS256", []byte("test-secret"), nil)
cases := []struct {
length int
status int
created bool
}{
{2, http.StatusBadRequest, false},
{3, http.StatusCreated, true},
{200, http.StatusCreated, true},
{201, http.StatusBadRequest, false},
}
for _, tc := range cases {
t.Run(fmt.Sprintf("length_%d", tc.length), func(t *testing.T) {
repo := newMockArticleRepo()
router := newArticleTitleRulesRouter(repo, token)
title := strings.Repeat("a", tc.length)
req := postArticleRequest(t, token, title, "Some body text")
rec := httptest.NewRecorder()
router.ServeHTTP(rec, req)
if rec.Code != tc.status {
t.Fatalf("expected status %d, got %d, body %q", tc.status, rec.Code, rec.Body.String())
}
if !tc.created {
if repo.createCalled {
t.Errorf("expected Create not to have been called")
}
if strings.Contains(rec.Body.String(), apperr.ArticleTitleErrorMessage) {
t.Errorf("expected a request-layer validation error, got the domain error message: %q", rec.Body.String())
}
if strings.Contains(rec.Body.String(), "Failed to create article") {
t.Errorf("expected a request-layer validation error, got the generic failure message: %q", rec.Body.String())
}
}
if tc.created {
var got domainArticle.Article
if err := json.Unmarshal(rec.Body.Bytes(), &got); err != nil {
t.Fatalf("failed to decode response: %v", err)
}
if got.IsOnline {
t.Errorf("expected is_online false, got true")
}
if len(repo.articles) != 1 {
t.Errorf("expected 1 article stored, got %d", len(repo.articles))
}
} else {
if len(repo.articles) != 0 {
t.Errorf("expected nothing stored, got %d article(s)", len(repo.articles))
}
}
})
}
t.Run("A 200-rune multibyte title is accepted end to end over HTTP", func(t *testing.T) {
repo := newMockArticleRepo()
router := newArticleTitleRulesRouter(repo, token)
title := strings.Repeat("\U0001F600", 200)
req := postArticleRequest(t, token, title, "Some body text")
rec := httptest.NewRecorder()
router.ServeHTTP(rec, req)
if rec.Code != http.StatusCreated {
t.Fatalf("expected status 201, got %d, body %q", rec.Code, rec.Body.String())
}
var got domainArticle.Article
if err := json.Unmarshal(rec.Body.Bytes(), &got); err != nil {
t.Fatalf("failed to decode response: %v", err)
}
if got.Title != title {
t.Errorf("expected title %q, got %q", title, got.Title)
}
})
t.Run("The handler maps a domain title rejection to 400, not 500", func(t *testing.T) {
repo := newMockArticleRepo()
repo.fail.failNextWith("Create", domainArticle.ErrInvalidTitle)
router := newArticleTitleRulesRouter(repo, token)
req := postArticleRequest(t, token, "Valid Title", "Some body text")
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.ArticleTitleErrorMessage+"\n" {
t.Errorf("unexpected body: %q", rec.Body.String())
}
if strings.Contains(rec.Body.String(), "Failed to create article") {
t.Errorf("did not expect the generic failure message, got %q", rec.Body.String())
}
})
t.Run("A non-title repository failure during creation still answers 500", func(t *testing.T) {
repo := newMockArticleRepo()
repo.fail.failNext("Create")
router := newArticleTitleRulesRouter(repo, token)
req := postArticleRequest(t, token, "Valid Title", "Some body text")
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 create article\n" {
t.Errorf("unexpected body: %q", rec.Body.String())
}
})
t.Run("POST /v1/article/ without a jwt cookie is rejected", func(t *testing.T) {
repo := newMockArticleRepo()
router := newArticleTitleRulesRouter(repo, token)
payload, err := json.Marshal(map[string]string{"title": "Valid Title", "body": "Some body text"})
if err != nil {
t.Fatalf("failed to marshal body: %v", err)
}
req := httptest.NewRequest(http.MethodPost, "/v1/article/", bytes.NewReader(payload))
req.Header.Set("Content-Type", "application/json")
rec := httptest.NewRecorder()
router.ServeHTTP(rec, req)
if rec.Code != http.StatusUnauthorized {
t.Fatalf("expected status 401, got %d", rec.Code)
}
if repo.createCalled {
t.Errorf("expected Create not to have been called")
}
})
}
+12
View File
@@ -6,15 +6,27 @@ var errRepositoryFailure = errors.New("repository failure")
type failSwitch struct {
method string
err error
}
func (f *failSwitch) failNext(method string) {
f.method = method
f.err = nil
}
func (f *failSwitch) failNextWith(method string, err error) {
f.method = method
f.err = err
}
func (f *failSwitch) trigger(method string) error {
if f.method == method {
f.method = ""
if f.err != nil {
err := f.err
f.err = nil
return err
}
return errRepositoryFailure
}
return nil