fix: upload part tmp+rename (B4), transactional last-admin DeleteUser (B5), RowsAffected order (B13)
This commit is contained in:
@@ -198,8 +198,11 @@ func (h *H) UploadPart(c *gin.Context) {
|
|||||||
}
|
}
|
||||||
lo, hi := chunkRange(m, idx)
|
lo, hi := chunkRange(m, idx)
|
||||||
c.Request.Body = http.MaxBytesReader(c.Writer, c.Request.Body, hi-lo)
|
c.Request.Body = http.MaxBytesReader(c.Writer, c.Request.Body, hi-lo)
|
||||||
|
// B4: write to .tmp then rename — prevents truncated parts from being
|
||||||
|
// reported as "received" by UploadStatus if the process crashes mid-write.
|
||||||
p := filepath.Join(dir, "parts", strconv.FormatInt(idx, 10))
|
p := filepath.Join(dir, "parts", strconv.FormatInt(idx, 10))
|
||||||
f, e := os.OpenFile(p, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0o644)
|
tmp := p + ".tmp"
|
||||||
|
f, e := os.OpenFile(tmp, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0o644)
|
||||||
if e != nil {
|
if e != nil {
|
||||||
err(c, http.StatusInternalServerError, "internal", "create part")
|
err(c, http.StatusInternalServerError, "internal", "create part")
|
||||||
return
|
return
|
||||||
@@ -207,7 +210,7 @@ func (h *H) UploadPart(c *gin.Context) {
|
|||||||
n, e := io.Copy(f, c.Request.Body)
|
n, e := io.Copy(f, c.Request.Body)
|
||||||
f.Close()
|
f.Close()
|
||||||
if e != nil || n != hi-lo {
|
if e != nil || n != hi-lo {
|
||||||
os.Remove(p)
|
os.Remove(tmp)
|
||||||
var mbe *http.MaxBytesError
|
var mbe *http.MaxBytesError
|
||||||
code, msg := "too_large", "part size mismatch"
|
code, msg := "too_large", "part size mismatch"
|
||||||
if errors.As(e, &mbe) {
|
if errors.As(e, &mbe) {
|
||||||
@@ -216,6 +219,11 @@ func (h *H) UploadPart(c *gin.Context) {
|
|||||||
err(c, http.StatusRequestEntityTooLarge, code, msg)
|
err(c, http.StatusRequestEntityTooLarge, code, msg)
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
if e := os.Rename(tmp, p); e != nil {
|
||||||
|
os.Remove(tmp)
|
||||||
|
err(c, http.StatusInternalServerError, "internal", "rename part")
|
||||||
|
return
|
||||||
|
}
|
||||||
c.JSON(http.StatusAccepted, gin.H{"accepted": true})
|
c.JSON(http.StatusAccepted, gin.H{"accepted": true})
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -8,15 +8,13 @@ import (
|
|||||||
|
|
||||||
"github.com/gin-gonic/gin"
|
"github.com/gin-gonic/gin"
|
||||||
"github.com/jackc/pgx/v5"
|
"github.com/jackc/pgx/v5"
|
||||||
"github.com/jackc/pgx/v5/pgconn"
|
|
||||||
|
|
||||||
"booklib/internal/auth"
|
"booklib/internal/auth"
|
||||||
|
"booklib/internal/store"
|
||||||
)
|
)
|
||||||
|
|
||||||
func isUnique(e error) bool {
|
// isUnique is a convenience alias for store.IsUniqueViolation.
|
||||||
var pgErr *pgconn.PgError
|
func isUnique(e error) bool { return store.IsUniqueViolation(e) }
|
||||||
return errors.As(e, &pgErr) && pgErr.Code == "23505"
|
|
||||||
}
|
|
||||||
|
|
||||||
func (h *H) ListUsers(c *gin.Context) {
|
func (h *H) ListUsers(c *gin.Context) {
|
||||||
users, e := h.st.ListUsers(c)
|
users, e := h.st.ListUsers(c)
|
||||||
@@ -73,23 +71,16 @@ func (h *H) DeleteUser(c *gin.Context) {
|
|||||||
err(c, http.StatusBadRequest, "bad_request", "cannot delete yourself")
|
err(c, http.StatusBadRequest, "bad_request", "cannot delete yourself")
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
target, e := h.st.GetUserByID(c, id)
|
// B5: transactional last-admin check eliminates TOCTOU race.
|
||||||
if e != nil {
|
if e := h.st.DeleteUser(c, id); e != nil {
|
||||||
if errors.Is(e, pgx.ErrNoRows) {
|
if errors.Is(e, pgx.ErrNoRows) {
|
||||||
err(c, http.StatusNotFound, "not_found", "no such user")
|
err(c, http.StatusNotFound, "not_found", "no such user")
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
dbErr(c, e)
|
if errors.Is(e, store.ErrLastAdmin) {
|
||||||
return
|
|
||||||
}
|
|
||||||
if target.Role == "admin" {
|
|
||||||
n, _ := h.st.CountAdmins(c) // 防删光最后一个 admin
|
|
||||||
if n <= 1 {
|
|
||||||
err(c, http.StatusBadRequest, "bad_request", "cannot delete the last admin")
|
err(c, http.StatusBadRequest, "bad_request", "cannot delete the last admin")
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
}
|
|
||||||
if e := h.st.DeleteUser(c, id); e != nil {
|
|
||||||
dbErr(c, e)
|
dbErr(c, e)
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -2,12 +2,23 @@ package store
|
|||||||
|
|
||||||
import (
|
import (
|
||||||
"context"
|
"context"
|
||||||
|
"errors"
|
||||||
"time"
|
"time"
|
||||||
|
|
||||||
"github.com/jackc/pgx/v5"
|
"github.com/jackc/pgx/v5"
|
||||||
|
"github.com/jackc/pgx/v5/pgconn"
|
||||||
"github.com/jackc/pgx/v5/pgxpool"
|
"github.com/jackc/pgx/v5/pgxpool"
|
||||||
)
|
)
|
||||||
|
|
||||||
|
// ErrLastAdmin is returned by DeleteUser when attempting to delete the last admin.
|
||||||
|
var ErrLastAdmin = errors.New("cannot delete the last admin")
|
||||||
|
|
||||||
|
// IsUniqueViolation reports whether err is a PostgreSQL unique constraint violation (23505).
|
||||||
|
func IsUniqueViolation(err error) bool {
|
||||||
|
var pgErr *pgconn.PgError
|
||||||
|
return errors.As(err, &pgErr) && pgErr.Code == "23505"
|
||||||
|
}
|
||||||
|
|
||||||
type Store struct{ P *pgxpool.Pool }
|
type Store struct{ P *pgxpool.Pool }
|
||||||
|
|
||||||
func New(p *pgxpool.Pool) *Store { return &Store{P: p} }
|
func New(p *pgxpool.Pool) *Store { return &Store{P: p} }
|
||||||
@@ -103,9 +114,31 @@ func (s *Store) ListUsers(ctx context.Context) ([]User, error) {
|
|||||||
return out, rows.Err()
|
return out, rows.Err()
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// DeleteUser deletes a user with a transactional last-admin check (B5).
|
||||||
|
// Returns ErrLastAdmin if the target is the last admin.
|
||||||
func (s *Store) DeleteUser(ctx context.Context, id int64) error {
|
func (s *Store) DeleteUser(ctx context.Context, id int64) error {
|
||||||
_, err := s.P.Exec(ctx, "DELETE FROM users WHERE id=$1", id)
|
tx, err := s.P.Begin(ctx)
|
||||||
return err
|
if err != nil {
|
||||||
|
return err
|
||||||
|
}
|
||||||
|
defer tx.Rollback(ctx)
|
||||||
|
var role string
|
||||||
|
if err := tx.QueryRow(ctx, "SELECT role FROM users WHERE id=$1 FOR UPDATE", id).Scan(&role); err != nil {
|
||||||
|
return err // includes pgx.ErrNoRows
|
||||||
|
}
|
||||||
|
if role == "admin" {
|
||||||
|
var n int
|
||||||
|
if err := tx.QueryRow(ctx, "SELECT count(*) FROM users WHERE role='admin'").Scan(&n); err != nil {
|
||||||
|
return err
|
||||||
|
}
|
||||||
|
if n <= 1 {
|
||||||
|
return ErrLastAdmin
|
||||||
|
}
|
||||||
|
}
|
||||||
|
if _, err := tx.Exec(ctx, "DELETE FROM users WHERE id=$1", id); err != nil {
|
||||||
|
return err
|
||||||
|
}
|
||||||
|
return tx.Commit(ctx)
|
||||||
}
|
}
|
||||||
|
|
||||||
func scanUser(row pgx.Row) (User, error) {
|
func scanUser(row pgx.Row) (User, error) {
|
||||||
@@ -114,12 +147,7 @@ func scanUser(row pgx.Row) (User, error) {
|
|||||||
return u, err
|
return u, err
|
||||||
}
|
}
|
||||||
|
|
||||||
// CountAdmins 供 Task 5 的"最后一个 admin 不可删"保护
|
|
||||||
func (s *Store) CountAdmins(ctx context.Context) (int, error) {
|
|
||||||
var n int
|
|
||||||
err := s.P.QueryRow(ctx, "SELECT count(*) FROM users WHERE role='admin'").Scan(&n)
|
|
||||||
return n, err
|
|
||||||
}
|
|
||||||
|
|
||||||
// ---------- libraries ----------
|
// ---------- libraries ----------
|
||||||
|
|
||||||
@@ -357,12 +385,18 @@ func (s *Store) ListBookmarks(ctx context.Context, userID, libID int64, bookPath
|
|||||||
|
|
||||||
func (s *Store) UpdateBookmarkNote(ctx context.Context, userID, id int64, note string) (bool, error) {
|
func (s *Store) UpdateBookmarkNote(ctx context.Context, userID, id int64, note string) (bool, error) {
|
||||||
res, err := s.P.Exec(ctx, `UPDATE bookmarks SET note=$3 WHERE id=$1 AND user_id=$2`, id, userID, note)
|
res, err := s.P.Exec(ctx, `UPDATE bookmarks SET note=$3 WHERE id=$1 AND user_id=$2`, id, userID, note)
|
||||||
return res.RowsAffected() > 0, err
|
if err != nil {
|
||||||
|
return false, err
|
||||||
|
}
|
||||||
|
return res.RowsAffected() > 0, nil
|
||||||
}
|
}
|
||||||
|
|
||||||
func (s *Store) DeleteBookmark(ctx context.Context, userID, id int64) (bool, error) {
|
func (s *Store) DeleteBookmark(ctx context.Context, userID, id int64) (bool, error) {
|
||||||
res, err := s.P.Exec(ctx, `DELETE FROM bookmarks WHERE id=$1 AND user_id=$2`, id, userID)
|
res, err := s.P.Exec(ctx, `DELETE FROM bookmarks WHERE id=$1 AND user_id=$2`, id, userID)
|
||||||
return res.RowsAffected() > 0, err
|
if err != nil {
|
||||||
|
return false, err
|
||||||
|
}
|
||||||
|
return res.RowsAffected() > 0, nil
|
||||||
}
|
}
|
||||||
|
|
||||||
func (s *Store) GetProgress(ctx context.Context, userID, libID int64, bookPath string) (Progress, error) {
|
func (s *Store) GetProgress(ctx context.Context, userID, libID int64, bookPath string) (Progress, error) {
|
||||||
|
|||||||
@@ -2,6 +2,7 @@ package store
|
|||||||
|
|
||||||
import (
|
import (
|
||||||
"context"
|
"context"
|
||||||
|
"errors"
|
||||||
"os"
|
"os"
|
||||||
"testing"
|
"testing"
|
||||||
|
|
||||||
@@ -129,3 +130,50 @@ func TestProgressUpsertAndJoin(t *testing.T) {
|
|||||||
t.Fatalf("views %+v", views)
|
t.Fatalf("views %+v", views)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func TestDeleteUserLastAdmin(t *testing.T) {
|
||||||
|
s := setup(t)
|
||||||
|
ctx := context.Background()
|
||||||
|
id, err := s.CreateUser(ctx, "onlyadmin", "hash", "admin")
|
||||||
|
if err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
err = s.DeleteUser(ctx, id)
|
||||||
|
if !errors.Is(err, ErrLastAdmin) {
|
||||||
|
t.Fatalf("expected ErrLastAdmin, got %v", err)
|
||||||
|
}
|
||||||
|
// Verify user still exists.
|
||||||
|
_, err = s.GetUserByID(ctx, id)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatal("admin should still exist after blocked delete")
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestDeleteUserNonLastAdmin(t *testing.T) {
|
||||||
|
s := setup(t)
|
||||||
|
ctx := context.Background()
|
||||||
|
id1, _ := s.CreateUser(ctx, "admin1", "h", "admin")
|
||||||
|
_, _ = s.CreateUser(ctx, "admin2", "h", "admin")
|
||||||
|
if err := s.DeleteUser(ctx, id1); err != nil {
|
||||||
|
t.Fatalf("should allow deleting non-last admin: %v", err)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestDeleteUserMember(t *testing.T) {
|
||||||
|
s := setup(t)
|
||||||
|
ctx := context.Background()
|
||||||
|
id, _ := s.CreateUser(ctx, "member", "h", "member")
|
||||||
|
if err := s.DeleteUser(ctx, id); err != nil {
|
||||||
|
t.Fatalf("member delete should succeed: %v", err)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestIsUniqueViolation(t *testing.T) {
|
||||||
|
s := setup(t)
|
||||||
|
ctx := context.Background()
|
||||||
|
_, _ = s.CreateUser(ctx, "dup", "h", "member")
|
||||||
|
_, err := s.CreateUser(ctx, "dup", "h", "member")
|
||||||
|
if !IsUniqueViolation(err) {
|
||||||
|
t.Fatalf("expected unique violation, got %v", err)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user