refac: repository updates, image attachment and deletes report the domain not-found error for an unknown id, the same for articles, users and images, and the test fakes stop returning GORM errors
This commit is contained in:
1 parent
4278d96882
commit
6a2ca07b66
14 files changed
+553
-66
No files matched your search
@@ -0,0 +1,97 @@
|
||||
Feature: Domain not-found errors on repository writes, image attachment and deletes
|
||||
|
||||
As the maintainer of RenewCMS
|
||||
I want an update, an image attachment or a delete against an unknown article, user or image id
|
||||
to report a domain not-found error instead of a raw GORM error or a silent no-op
|
||||
So that a caller can rely on errors.Is(err, article.ErrArticleNotFound), errors.Is(err, user.ErrUserNotFound)
|
||||
or errors.Is(err, image.ErrImageNotFound) on every repository method, not just the read side,
|
||||
and so that DELETE /v1/article/{id} reports 404 instead of leaking a 500 for an id that no longer exists
|
||||
|
||||
Background:
|
||||
Given internal/domain/article defines the sentinel article.ErrArticleNotFound
|
||||
And internal/domain/user defines the sentinel user.ErrUserNotFound
|
||||
And internal/domain/image defines the sentinel image.ErrImageNotFound
|
||||
And ArticleRepository.UpdateBody, UpdateIsOnline, Delete and AddImage return article.ErrArticleNotFound,
|
||||
not gorm.ErrRecordNotFound and not a silent no-op, when the article id does not match any row
|
||||
And ArticleRepository.AddImage returns image.ErrImageNotFound, not gorm.ErrRecordNotFound,
|
||||
when the article exists but the image id does not match any row
|
||||
And UserRepository.UpdateVerificationStatus, UpdatePassword, UpdatePasswordResetCode and Delete return
|
||||
user.ErrUserNotFound, not gorm.ErrRecordNotFound and not a silent no-op, when the user id does not match any row
|
||||
And ImageRepository.Delete returns image.ErrImageNotFound, not gorm.ErrRecordNotFound, when the image id
|
||||
does not match any row
|
||||
And DELETE /v1/article/{id} sits behind the jwtauth Verifier and Authenticator group, and requires a valid jwt cookie
|
||||
And UpdateArticle, AddImage, UpdateUser, DeleteUser, CreateImage and DeleteImage have no handler mounted in
|
||||
api/router.go today, so this slice has no other HTTP-observable surface beyond DELETE /v1/article/{id}
|
||||
|
||||
Scenario Outline: The article repository reports a missing article as a domain error on a write
|
||||
Given a repository backed by an in-memory SQLite database containing an article titled "Backyard Renovation Update" with id 1
|
||||
When <method> is called with id 9 <extra>
|
||||
Then the result is the error article.ErrArticleNotFound
|
||||
And errors.Is(err, gorm.ErrRecordNotFound) is false
|
||||
|
||||
Examples:
|
||||
| method | extra |
|
||||
| UpdateBody | and body "New body" |
|
||||
| UpdateIsOnline | and is_online true |
|
||||
| Delete | |
|
||||
|
||||
Scenario: AddImage reports a missing article as a domain error, not a missing image
|
||||
Given a repository backed by an in-memory SQLite database containing an article with id 1
|
||||
And the same database containing an image with id 1
|
||||
When AddImage is called with article id 9 and image id 1
|
||||
Then the result is the error article.ErrArticleNotFound
|
||||
And errors.Is(err, gorm.ErrRecordNotFound) is false
|
||||
|
||||
Scenario: AddImage reports a missing image as a domain error once the article exists
|
||||
Given a repository backed by an in-memory SQLite database containing an article with id 1
|
||||
And the same database containing no image with id 9
|
||||
When AddImage is called with article id 1 and image id 9
|
||||
Then the result is the error image.ErrImageNotFound
|
||||
And errors.Is(err, gorm.ErrRecordNotFound) is false
|
||||
|
||||
Scenario Outline: The user repository reports a missing user as a domain error on a write
|
||||
Given a repository backed by an in-memory SQLite database containing a user with id 1, username "alice" and email "alice@example.com"
|
||||
When <method> is called with id 9 <extra>
|
||||
Then the result is the error user.ErrUserNotFound
|
||||
And errors.Is(err, gorm.ErrRecordNotFound) is false
|
||||
|
||||
Examples:
|
||||
| method | extra |
|
||||
| UpdateVerificationStatus | and is_verified true |
|
||||
| UpdatePassword | and password "newHashedPassword" |
|
||||
| UpdatePasswordResetCode | and code "newHashedResetCode" |
|
||||
| Delete | |
|
||||
|
||||
Scenario: The image repository reports a missing image as a domain error on delete
|
||||
Given a repository backed by an in-memory SQLite database containing an image with id 1
|
||||
When Delete is called with id 9
|
||||
Then the result is the error image.ErrImageNotFound
|
||||
And errors.Is(err, gorm.ErrRecordNotFound) is false
|
||||
|
||||
Scenario: The test fakes mirror the real repositories on every write not-found path
|
||||
Given the in-memory article, user and image fakes used in test/
|
||||
When UpdateBody, UpdateIsOnline, Delete or AddImage is called against an id absent from the fake's map
|
||||
Then the fake returns article.ErrArticleNotFound or image.ErrImageNotFound, matching the real repository
|
||||
And when UpdateVerificationStatus, UpdatePassword, UpdatePasswordResetCode or Delete is called against an id
|
||||
absent from the fake's map, the fake returns user.ErrUserNotFound, matching the real repository
|
||||
And none of the fakes construct or return gorm.ErrRecordNotFound anymore
|
||||
|
||||
Scenario: Deleting an unknown article answers 404, not a silent success or a 500
|
||||
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"
|
||||
|
||||
Scenario: Deleting an existing article still answers 204
|
||||
Given a valid jwt cookie for a user with id 1
|
||||
And the article repository contains an article with id 1
|
||||
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
|
||||
|
||||
Scenario: The delete-article route still requires authentication before existence is even checked
|
||||
Given no jwt cookie is sent
|
||||
And the article repository has no article with id 9999
|
||||
When the client sends DELETE /v1/article/9999 without a cookie
|
||||
Then the response status is 401
|
||||
@@ -0,0 +1,65 @@
|
||||
# QA procedure: write_not_found_errors
|
||||
|
||||
Package: `test` (black-box, extending `article_test.go` / `user_test.go` / `image_test.go` / `not_found_errors_test.go`).
|
||||
|
||||
## Setup shared by every case
|
||||
1. Call `validator.InitValidator()` once.
|
||||
2. Update the in-memory fakes (`mockArticleRepo`, `mockUserRepo`, `mockImageRepo` in `test/`) so that:
|
||||
- `mockArticleRepo.UpdateBody`, `UpdateIsOnline` and `AddImage` return `article.ErrArticleNotFound` /
|
||||
`image.ErrImageNotFound` (per the missing id) instead of `gorm.ErrRecordNotFound`;
|
||||
- `mockArticleRepo.Delete` returns `article.ErrArticleNotFound` when the id isn't in the map, instead of
|
||||
silently succeeding;
|
||||
- `mockUserRepo.UpdateVerificationStatus`, `UpdatePassword`, `UpdatePasswordResetCode` return
|
||||
`user.ErrUserNotFound` instead of `gorm.ErrRecordNotFound`;
|
||||
- `mockUserRepo.Delete` returns `user.ErrUserNotFound` when the id isn't in the map, instead of silently
|
||||
succeeding;
|
||||
- `mockImageRepo.Delete` returns `image.ErrImageNotFound` instead of `gorm.ErrRecordNotFound`.
|
||||
No fake in `test/` should reference `gorm.ErrRecordNotFound` (or import `gorm.io/gorm`) after this slice,
|
||||
except where still needed to assert a real repository's behavior in a repository-level test.
|
||||
3. Wire the real use cases (`useCases.NewUpdateArticleUseCase`, `useCases.NewDeleteArticleUseCase`,
|
||||
`useCases.NewUpdateUserUseCase`, `useCases.NewDeleteUserUseCase`, `useCases.NewDeleteImageUseCase`) over
|
||||
the fakes for the use-case-level checks.
|
||||
4. Build `article.NewHandler(...)` from the article use cases plus a test `jwtauth.New("HS256", []byte("test-secret"), nil)`,
|
||||
and mount it under a `chi.NewRouter()` group wrapped by `jwtauth.Verifier(token)` + `jwtauth.Authenticator(token)`,
|
||||
mirroring `api/router.go`. No live server.
|
||||
|
||||
## Repository-level checks (real persistence repositories, in-memory SQLite, no HTTP)
|
||||
Use the existing `openInMemoryDb(t)` helper from `test/not_found_errors_test.go`.
|
||||
|
||||
5. `db.AutoMigrate(&models.Article{}, &models.Image{})`. Build a real `persistence.NewArticleRepository(db)` and
|
||||
`persistence.NewImageRepository(db)`. Seed one article (id 1) and one image (id 1) via `Create`.
|
||||
- `UpdateBody(9, "New body")` → assert `errors.Is(err, article.ErrArticleNotFound)` true,
|
||||
`errors.Is(err, gorm.ErrRecordNotFound)` false.
|
||||
- `UpdateIsOnline(9, true)` → same assertions.
|
||||
- `Delete(9)` → same assertions. Then `Delete(1)` on the seeded article → assert `err == nil` and a
|
||||
subsequent `Get(1)` returns `article.ErrArticleNotFound` (the row is actually gone).
|
||||
- `AddImage(9, 1)` (unknown article, known image) → assert `errors.Is(err, article.ErrArticleNotFound)` true.
|
||||
- `AddImage(1, 9)` (known article, unknown image) → assert `errors.Is(err, image.ErrImageNotFound)` true,
|
||||
`errors.Is(err, gorm.ErrRecordNotFound)` false.
|
||||
6. On the same or a fresh in-memory DB, `db.AutoMigrate(&models.User{})`. Build a real
|
||||
`persistence.NewUserRepository(db)`, seed a user id 1.
|
||||
- `UpdateVerificationStatus(9, true)`, `UpdatePassword(9, "x")`, `UpdatePasswordResetCode(9, "y")` → each
|
||||
asserts `errors.Is(err, user.ErrUserNotFound)` true, `errors.Is(err, gorm.ErrRecordNotFound)` false.
|
||||
- `Delete(9)` → same assertions. Then `Delete(1)` on the seeded user → assert `err == nil` and a
|
||||
subsequent `Get(1)` returns `user.ErrUserNotFound`.
|
||||
7. `db.AutoMigrate(&models.Image{})`. Build a real `persistence.NewImageRepository(db)`, seed one image via
|
||||
direct DB insert (or `Create` with a fake `io.Reader` and `UPLOAD_DIR` pointed at `t.TempDir()`).
|
||||
- `Delete(9)` → assert `errors.Is(err, image.ErrImageNotFound)` true, `errors.Is(err, gorm.ErrRecordNotFound)` false.
|
||||
|
||||
## Fake-level checks (mirrors the real repositories, package-internal, no HTTP)
|
||||
8. For each of `mockArticleRepo`, `mockUserRepo`, `mockImageRepo`, call the write methods listed in step 2
|
||||
against an id absent from the fake's map and assert they return the matching domain sentinel, never
|
||||
`gorm.ErrRecordNotFound`.
|
||||
|
||||
## HTTP checks (fakes, per the shared setup above) — DELETE /v1/article/{id} only
|
||||
9. Mint a valid jwt cookie for user id 1 (as in `not_found_errors_test.go`, step 8 there).
|
||||
10. `DELETE /v1/article/9999` with that cookie, empty repo id 9999 → assert status `404`, body
|
||||
`"Article not found\n"`.
|
||||
11. `DELETE /v1/article/1` with that cookie, repo seeded with an article id 1 → assert status `204`, and that
|
||||
the fake no longer holds id 1 afterward.
|
||||
12. `DELETE /v1/article/9999` with **no** cookie → assert status `401` (regression: jwtauth still gates the route).
|
||||
|
||||
## Manual verification (not exercised by the Go test)
|
||||
- None. `UpdateArticle`, `AddImage`, `UpdateUser`, `DeleteUser`, `CreateImage` and `DeleteImage` have no HTTP
|
||||
route today (not mounted in `api/router.go`), so there is no SPA-visible behavior or manual HTTP check for
|
||||
them in this slice — only the repository/fake-level checks above apply. No SMTP involvement.
|
||||
Reference in new issue
Block a user