Fix code review findings: image read, whitespace validation, offset reset, cleanup error
- Replace src.Read(data) with io.ReadAll(src) to prevent silent truncation on large uploads - Reject zero-byte file uploads with 400 rather than storing empty BYTEA - Add strings.TrimSpace to validateFruit so whitespace-only name/osdb_number returns 422 - Reset offset to 0 in fruitStore.create() so navigating back to list after create shows page 1 - Log t.Cleanup DELETE error in integration test to surface constraint failures clearly Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -3,8 +3,10 @@ package handler
|
|||||||
import (
|
import (
|
||||||
"context"
|
"context"
|
||||||
"errors"
|
"errors"
|
||||||
|
"io"
|
||||||
"net/http"
|
"net/http"
|
||||||
"strconv"
|
"strconv"
|
||||||
|
"strings"
|
||||||
|
|
||||||
"github.com/labstack/echo/v4"
|
"github.com/labstack/echo/v4"
|
||||||
|
|
||||||
@@ -60,10 +62,10 @@ func NewFruitHandler(repo FruitRepository) *FruitHandler {
|
|||||||
|
|
||||||
func validateFruit(dto domain.FruitWriteDTO) []string {
|
func validateFruit(dto domain.FruitWriteDTO) []string {
|
||||||
var errs []string
|
var errs []string
|
||||||
if dto.Name == "" {
|
if strings.TrimSpace(dto.Name) == "" {
|
||||||
errs = append(errs, "name is required")
|
errs = append(errs, "name is required")
|
||||||
}
|
}
|
||||||
if dto.OSDBNumber == "" {
|
if strings.TrimSpace(dto.OSDBNumber) == "" {
|
||||||
errs = append(errs, "osdb_number is required")
|
errs = append(errs, "osdb_number is required")
|
||||||
}
|
}
|
||||||
if _, ok := validFruitTypes[dto.FruitType]; !ok {
|
if _, ok := validFruitTypes[dto.FruitType]; !ok {
|
||||||
@@ -220,8 +222,12 @@ func (h *FruitHandler) UploadImage(c echo.Context) error {
|
|||||||
}
|
}
|
||||||
defer src.Close()
|
defer src.Close()
|
||||||
|
|
||||||
data := make([]byte, file.Size)
|
if file.Size == 0 {
|
||||||
if _, err := src.Read(data); err != nil {
|
return c.JSON(http.StatusBadRequest, map[string]string{"error": "image file is empty"})
|
||||||
|
}
|
||||||
|
|
||||||
|
data, err := io.ReadAll(src)
|
||||||
|
if err != nil {
|
||||||
return c.JSON(http.StatusInternalServerError, map[string]string{"error": "internal server error"})
|
return c.JSON(http.StatusInternalServerError, map[string]string{"error": "internal server error"})
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -41,7 +41,9 @@ func TestFruitRepoIntegration(t *testing.T) {
|
|||||||
|
|
||||||
// cleanup after test
|
// cleanup after test
|
||||||
t.Cleanup(func() {
|
t.Cleanup(func() {
|
||||||
pool.Exec(ctx, `DELETE FROM fruits WHERE osdb_number LIKE 'TEST-%'`)
|
if _, err := pool.Exec(ctx, `DELETE FROM fruits WHERE osdb_number LIKE 'TEST-%'`); err != nil {
|
||||||
|
t.Logf("cleanup: %v", err)
|
||||||
|
}
|
||||||
})
|
})
|
||||||
|
|
||||||
// Create
|
// Create
|
||||||
|
|||||||
@@ -52,6 +52,7 @@ export const useFruitStore = defineStore('fruit', () => {
|
|||||||
const fruit = await createFruit(dto)
|
const fruit = await createFruit(dto)
|
||||||
fruits.value = [fruit, ...fruits.value]
|
fruits.value = [fruit, ...fruits.value]
|
||||||
total.value += 1
|
total.value += 1
|
||||||
|
offset.value = 0
|
||||||
return fruit
|
return fruit
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user