refac: missing user or article is reported with a domain not-found error instead of a raw GORM error, and the API answers 404 for it
This commit is contained in:
1 parent
7c57173497
commit
8460a078e3
13 files changed
+426
-32
No files matched your search
@@ -55,7 +55,7 @@ func (m *mockArticleRepo) Get(id uint32) (article.Article, error) {
|
||||
}
|
||||
a, ok := m.articles[id]
|
||||
if !ok {
|
||||
return article.Article{}, gorm.ErrRecordNotFound
|
||||
return article.Article{}, article.ErrArticleNotFound
|
||||
}
|
||||
return a, nil
|
||||
}
|
||||
@@ -70,7 +70,7 @@ func (m *mockArticleRepo) GetByName(name string) (article.Article, error) {
|
||||
return a, nil
|
||||
}
|
||||
}
|
||||
return article.Article{}, gorm.ErrRecordNotFound
|
||||
return article.Article{}, article.ErrArticleNotFound
|
||||
}
|
||||
|
||||
func (m *mockArticleRepo) GetAll() []article.Article {
|
||||
|
||||
@@ -0,0 +1,278 @@
|
||||
package test
|
||||
|
||||
import (
|
||||
"RenewCMS/api/handlers/article"
|
||||
"RenewCMS/api/handlers/auth"
|
||||
"RenewCMS/api/pkg/apperr"
|
||||
validator "RenewCMS/api/pkg/validator"
|
||||
domainArticle "RenewCMS/internal/domain/article"
|
||||
domainUser "RenewCMS/internal/domain/user"
|
||||
"RenewCMS/internal/infrastructure/persistence/models"
|
||||
persistence "RenewCMS/internal/infrastructure/persistence/repositories"
|
||||
"RenewCMS/internal/infrastructure/useCases"
|
||||
"bytes"
|
||||
"encoding/json"
|
||||
"errors"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"testing"
|
||||
|
||||
"github.com/glebarez/sqlite"
|
||||
"github.com/go-chi/chi/v5"
|
||||
"github.com/go-chi/jwtauth/v5"
|
||||
"golang.org/x/crypto/bcrypt"
|
||||
"gorm.io/gorm"
|
||||
)
|
||||
|
||||
func openInMemoryDb(t *testing.T) *gorm.DB {
|
||||
t.Helper()
|
||||
db, err := gorm.Open(sqlite.Open("file:"+t.Name()+"?mode=memory&cache=shared"), &gorm.Config{})
|
||||
if err != nil {
|
||||
t.Fatalf("failed to open in-memory database: %v", err)
|
||||
}
|
||||
return db
|
||||
}
|
||||
|
||||
func TestArticleRepositoryReportsMissingArticleAsDomainError(t *testing.T) {
|
||||
db := openInMemoryDb(t)
|
||||
if err := db.AutoMigrate(&models.Article{}); err != nil {
|
||||
t.Fatalf("failed to migrate: %v", err)
|
||||
}
|
||||
repo := persistence.NewArticleRepository(db)
|
||||
if _, err := repo.Create(domainArticle.Article{Title: "Backyard Renovation Update", Body: "Body"}); err != nil {
|
||||
t.Fatalf("failed to seed article: %v", err)
|
||||
}
|
||||
|
||||
t.Run("Get id 9", func(t *testing.T) {
|
||||
_, err := repo.Get(9)
|
||||
if !errors.Is(err, domainArticle.ErrArticleNotFound) {
|
||||
t.Errorf("expected article.ErrArticleNotFound, got %v", err)
|
||||
}
|
||||
if errors.Is(err, gorm.ErrRecordNotFound) {
|
||||
t.Errorf("expected the domain error, not gorm.ErrRecordNotFound")
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("GetByName Unknown Title", func(t *testing.T) {
|
||||
_, err := repo.GetByName("Unknown Title")
|
||||
if !errors.Is(err, domainArticle.ErrArticleNotFound) {
|
||||
t.Errorf("expected article.ErrArticleNotFound, got %v", err)
|
||||
}
|
||||
if errors.Is(err, gorm.ErrRecordNotFound) {
|
||||
t.Errorf("expected the domain error, not gorm.ErrRecordNotFound")
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("Get propagates a non-not-found database error unchanged", func(t *testing.T) {
|
||||
sqlDb, err := db.DB()
|
||||
if err != nil {
|
||||
t.Fatalf("failed to get sql db: %v", err)
|
||||
}
|
||||
if err := sqlDb.Close(); err != nil {
|
||||
t.Fatalf("failed to close db: %v", err)
|
||||
}
|
||||
|
||||
_, err = repo.Get(1)
|
||||
if err == nil {
|
||||
t.Fatal("expected an error, got nil")
|
||||
}
|
||||
if errors.Is(err, domainArticle.ErrArticleNotFound) {
|
||||
t.Errorf("expected a non-not-found error, got domain.ErrArticleNotFound")
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
func TestUserRepositoryReportsMissingUserAsDomainError(t *testing.T) {
|
||||
db := openInMemoryDb(t)
|
||||
if err := db.AutoMigrate(&models.User{}); err != nil {
|
||||
t.Fatalf("failed to migrate: %v", err)
|
||||
}
|
||||
repo := persistence.NewUserRepository(db)
|
||||
if _, err := repo.Create(domainUser.User{Username: "alice", Password: "hunter2example", Email: "alice@example.com", VerificationCode: "code"}); err != nil {
|
||||
t.Fatalf("failed to seed user: %v", err)
|
||||
}
|
||||
|
||||
t.Run("Get id 9", func(t *testing.T) {
|
||||
_, err := repo.Get(9)
|
||||
if !errors.Is(err, domainUser.ErrUserNotFound) {
|
||||
t.Errorf("expected user.ErrUserNotFound, got %v", err)
|
||||
}
|
||||
if errors.Is(err, gorm.ErrRecordNotFound) {
|
||||
t.Errorf("expected the domain error, not gorm.ErrRecordNotFound")
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("GetByUsername bob", func(t *testing.T) {
|
||||
_, err := repo.GetByUsername("bob")
|
||||
if !errors.Is(err, domainUser.ErrUserNotFound) {
|
||||
t.Errorf("expected user.ErrUserNotFound, got %v", err)
|
||||
}
|
||||
if errors.Is(err, gorm.ErrRecordNotFound) {
|
||||
t.Errorf("expected the domain error, not gorm.ErrRecordNotFound")
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("GetByEmail bob@example.com", func(t *testing.T) {
|
||||
_, err := repo.GetByEmail("bob@example.com")
|
||||
if !errors.Is(err, domainUser.ErrUserNotFound) {
|
||||
t.Errorf("expected user.ErrUserNotFound, got %v", err)
|
||||
}
|
||||
if errors.Is(err, gorm.ErrRecordNotFound) {
|
||||
t.Errorf("expected the domain error, not gorm.ErrRecordNotFound")
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
func newNotFoundErrorsRouter(articleRepo *mockArticleRepo, userRepo *mockUserRepo, token *jwtauth.JWTAuth) *chi.Mux {
|
||||
articleHandler := article.NewHandler(
|
||||
useCases.NewCreateArticleUseCase(articleRepo),
|
||||
useCases.NewGetArticleUseCase(articleRepo),
|
||||
useCases.NewListArticlesUseCase(articleRepo),
|
||||
useCases.NewDeleteArticleUseCase(articleRepo),
|
||||
)
|
||||
authHandler := auth.NewHandler(
|
||||
token,
|
||||
useCases.NewGetUserUseCase(userRepo),
|
||||
useCases.NewListUsersUseCase(userRepo),
|
||||
useCases.NewCreateUserUseCase(userRepo),
|
||||
)
|
||||
|
||||
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.Get("/{id}", articleHandler.GetArticle)
|
||||
})
|
||||
})
|
||||
r.Route("/auth", func(r chi.Router) {
|
||||
r.Post("/login", authHandler.Login)
|
||||
})
|
||||
})
|
||||
return r
|
||||
}
|
||||
|
||||
func setJwtCookie(t *testing.T, req *http.Request, token *jwtauth.JWTAuth, userId uint32) {
|
||||
t.Helper()
|
||||
_, tokenString, err := token.Encode(map[string]any{"user_id": userId})
|
||||
if err != nil {
|
||||
t.Fatalf("failed to encode jwt: %v", err)
|
||||
}
|
||||
req.AddCookie(&http.Cookie{Name: "jwt", Value: tokenString})
|
||||
}
|
||||
|
||||
func TestNotFoundErrorsHttp(t *testing.T) {
|
||||
validator.InitValidator()
|
||||
token := jwtauth.New("HS256", []byte("test-secret"), nil)
|
||||
|
||||
newArticleRepo := func() *mockArticleRepo {
|
||||
repo := newMockArticleRepo()
|
||||
repo.articles[1] = domainArticle.Article{ID: 1, Title: "Backyard Renovation Update", IsOnline: true}
|
||||
repo.articles[2] = domainArticle.Article{ID: 2, Title: "Offline Article", IsOnline: false}
|
||||
return repo
|
||||
}
|
||||
|
||||
t.Run("Fetching a nonexistent article answers 404, not 500", func(t *testing.T) {
|
||||
router := newNotFoundErrorsRouter(newArticleRepo(), newMockUserRepo(), token)
|
||||
req := httptest.NewRequest(http.MethodGet, "/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())
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("Fetching an offline article still answers 404 with the same message", func(t *testing.T) {
|
||||
router := newNotFoundErrorsRouter(newArticleRepo(), newMockUserRepo(), token)
|
||||
req := httptest.NewRequest(http.MethodGet, "/v1/article/2", 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())
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("Fetching an online article still answers 200", func(t *testing.T) {
|
||||
router := newNotFoundErrorsRouter(newArticleRepo(), newMockUserRepo(), token)
|
||||
req := httptest.NewRequest(http.MethodGet, "/v1/article/1", nil)
|
||||
setJwtCookie(t, req, token, 1)
|
||||
rec := httptest.NewRecorder()
|
||||
router.ServeHTTP(rec, req)
|
||||
|
||||
if rec.Code != http.StatusOK {
|
||||
t.Fatalf("expected status 200, got %d", rec.Code)
|
||||
}
|
||||
var got domainArticle.Article
|
||||
if err := json.Unmarshal(rec.Body.Bytes(), &got); err != nil {
|
||||
t.Fatalf("failed to decode response: %v", err)
|
||||
}
|
||||
if got.ID != 1 {
|
||||
t.Errorf("expected article id 1, got %d", got.ID)
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("The article route still requires authentication before existence is even checked", func(t *testing.T) {
|
||||
router := newNotFoundErrorsRouter(newArticleRepo(), newMockUserRepo(), token)
|
||||
req := httptest.NewRequest(http.MethodGet, "/v1/article/9999", nil)
|
||||
rec := httptest.NewRecorder()
|
||||
router.ServeHTTP(rec, req)
|
||||
|
||||
if rec.Code != http.StatusUnauthorized {
|
||||
t.Fatalf("expected status 401, got %d", rec.Code)
|
||||
}
|
||||
})
|
||||
|
||||
newAliceUserRepo := func() *mockUserRepo {
|
||||
repo := newMockUserRepo()
|
||||
hashed, err := bcrypt.GenerateFromPassword([]byte("hunter2example"), 12)
|
||||
if err != nil {
|
||||
t.Fatalf("failed to hash password: %v", err)
|
||||
}
|
||||
repo.users[1] = domainUser.User{ID: 1, Username: "alice", Password: string(hashed)}
|
||||
return repo
|
||||
}
|
||||
|
||||
loginCases := []struct {
|
||||
name string
|
||||
username string
|
||||
password string
|
||||
}{
|
||||
{"wrong password for existing user", "alice", "wrongpassword12"},
|
||||
{"nonexistent username", "bob", "hunter2example"},
|
||||
}
|
||||
|
||||
for _, tc := range loginCases {
|
||||
t.Run("Login never reveals whether the username exists: "+tc.name, func(t *testing.T) {
|
||||
router := newNotFoundErrorsRouter(newArticleRepo(), newAliceUserRepo(), token)
|
||||
body, err := json.Marshal(map[string]string{"username": tc.username, "password": tc.password})
|
||||
if err != nil {
|
||||
t.Fatalf("failed to marshal body: %v", err)
|
||||
}
|
||||
req := httptest.NewRequest(http.MethodPost, "/v1/auth/login", bytes.NewReader(body))
|
||||
req.Header.Set("Content-Type", "application/json")
|
||||
rec := httptest.NewRecorder()
|
||||
router.ServeHTTP(rec, req)
|
||||
|
||||
if rec.Code != http.StatusForbidden {
|
||||
t.Fatalf("expected status 403, got %d", rec.Code)
|
||||
}
|
||||
if rec.Body.String() != apperr.LogsErrorMessage+"\n" {
|
||||
t.Errorf("unexpected body: %q", rec.Body.String())
|
||||
}
|
||||
if rec.Header().Get("Set-Cookie") != "" {
|
||||
t.Errorf("expected no Set-Cookie header, got %q", rec.Header().Get("Set-Cookie"))
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
@@ -111,8 +111,8 @@ func TestGetArticleAndGetOnlineArticle(t *testing.T) {
|
||||
repo := newRepo()
|
||||
uc := useCases.NewGetArticleUseCase(repo)
|
||||
_, err := uc.GetArticle(9)
|
||||
if !errors.Is(err, gorm.ErrRecordNotFound) {
|
||||
t.Errorf("expected gorm.ErrRecordNotFound, got %v", err)
|
||||
if !errors.Is(err, article.ErrArticleNotFound) {
|
||||
t.Errorf("expected article.ErrArticleNotFound, got %v", err)
|
||||
}
|
||||
})
|
||||
|
||||
@@ -141,11 +141,8 @@ func TestGetArticleAndGetOnlineArticle(t *testing.T) {
|
||||
repo := newRepo()
|
||||
uc := useCases.NewGetArticleUseCase(repo)
|
||||
_, err := uc.GetOnlineArticle(9)
|
||||
if !errors.Is(err, gorm.ErrRecordNotFound) {
|
||||
t.Errorf("expected gorm.ErrRecordNotFound, got %v", err)
|
||||
}
|
||||
if errors.Is(err, article.ErrArticleNotFound) {
|
||||
t.Errorf("expected the raw not-found error, not article.ErrArticleNotFound")
|
||||
if !errors.Is(err, article.ErrArticleNotFound) {
|
||||
t.Errorf("expected article.ErrArticleNotFound, got %v", err)
|
||||
}
|
||||
})
|
||||
}
|
||||
@@ -173,8 +170,8 @@ func TestGetArticleByNameDelegatesVerbatim(t *testing.T) {
|
||||
repo := newRepo()
|
||||
uc := useCases.NewGetArticleUseCase(repo)
|
||||
_, err := uc.GetArticleByName("Unknown Title")
|
||||
if !errors.Is(err, gorm.ErrRecordNotFound) {
|
||||
t.Errorf("expected gorm.ErrRecordNotFound, got %v", err)
|
||||
if !errors.Is(err, article.ErrArticleNotFound) {
|
||||
t.Errorf("expected article.ErrArticleNotFound, got %v", err)
|
||||
}
|
||||
})
|
||||
}
|
||||
@@ -519,8 +516,8 @@ func TestGetUserReadsByIdUsernameOrEmail(t *testing.T) {
|
||||
repo := newRepo()
|
||||
uc := useCases.NewGetUserUseCase(repo)
|
||||
_, err := uc.GetUser(9)
|
||||
if !errors.Is(err, gorm.ErrRecordNotFound) {
|
||||
t.Errorf("expected gorm.ErrRecordNotFound, got %v", err)
|
||||
if !errors.Is(err, user.ErrUserNotFound) {
|
||||
t.Errorf("expected user.ErrUserNotFound, got %v", err)
|
||||
}
|
||||
})
|
||||
|
||||
@@ -540,8 +537,8 @@ func TestGetUserReadsByIdUsernameOrEmail(t *testing.T) {
|
||||
repo := newRepo()
|
||||
uc := useCases.NewGetUserUseCase(repo)
|
||||
_, err := uc.GetUserByUsername("bob")
|
||||
if !errors.Is(err, gorm.ErrRecordNotFound) {
|
||||
t.Errorf("expected gorm.ErrRecordNotFound, got %v", err)
|
||||
if !errors.Is(err, user.ErrUserNotFound) {
|
||||
t.Errorf("expected user.ErrUserNotFound, got %v", err)
|
||||
}
|
||||
})
|
||||
|
||||
@@ -561,8 +558,8 @@ func TestGetUserReadsByIdUsernameOrEmail(t *testing.T) {
|
||||
repo := newRepo()
|
||||
uc := useCases.NewGetUserUseCase(repo)
|
||||
_, err := uc.GetUserByEmail("bob@example.com")
|
||||
if !errors.Is(err, gorm.ErrRecordNotFound) {
|
||||
t.Errorf("expected gorm.ErrRecordNotFound, got %v", err)
|
||||
if !errors.Is(err, user.ErrUserNotFound) {
|
||||
t.Errorf("expected user.ErrUserNotFound, got %v", err)
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
+3
-3
@@ -34,7 +34,7 @@ func (m *mockUserRepo) Get(id uint32) (user.User, error) {
|
||||
}
|
||||
u, ok := m.users[id]
|
||||
if !ok {
|
||||
return user.User{}, gorm.ErrRecordNotFound
|
||||
return user.User{}, user.ErrUserNotFound
|
||||
}
|
||||
return u, nil
|
||||
}
|
||||
@@ -49,7 +49,7 @@ func (m *mockUserRepo) GetByUsername(username string) (user.User, error) {
|
||||
return u, nil
|
||||
}
|
||||
}
|
||||
return user.User{}, gorm.ErrRecordNotFound
|
||||
return user.User{}, user.ErrUserNotFound
|
||||
}
|
||||
|
||||
func (m *mockUserRepo) GetByEmail(email string) (user.User, error) {
|
||||
@@ -62,7 +62,7 @@ func (m *mockUserRepo) GetByEmail(email string) (user.User, error) {
|
||||
return u, nil
|
||||
}
|
||||
}
|
||||
return user.User{}, gorm.ErrRecordNotFound
|
||||
return user.User{}, user.ErrUserNotFound
|
||||
}
|
||||
|
||||
func (m *mockUserRepo) GetAll() []user.User {
|
||||
|
||||
Reference in new issue
Block a user