Merge pull request #195 from LAA-Software-Engineering/fix/114-handler-service-repository

refactor: handler / service / repository split (closes #114)

Author: leo-aa88

pkg/api/books.go

@@ -1,56 +1,26 @@
 package api
 
 import (
-	"context"
-	"encoding/json"
 	"errors"
-	"fmt"
-	"golang-rest-api-template/pkg/cache"
-	"golang-rest-api-template/pkg/database"
-	"golang-rest-api-template/pkg/models"
 	"net/http"
 	"strconv"
 	"strings"
-	"time"
+
+	"golang-rest-api-template/pkg/cache"
+	"golang-rest-api-template/pkg/database"
+	"golang-rest-api-template/pkg/models"
+	"golang-rest-api-template/pkg/repository"
+	"golang-rest-api-template/pkg/service"
 
 	"github.com/gin-gonic/gin"
-	"golang.org/x/sync/singleflight"
 )
 
-// booksListCacheGenKey is incremented on book writes so list pagination entries
-// (scoped by generation) go stale without Redis KEYS.
-const booksListCacheGenKey = "v1:books:list_cache_gen"
-
 const (
 	findBooksMinLimit = 1
 	findBooksMaxLimit = 100
 )
 
-func booksListDataCacheKey(gen int64, offset, limit int) string {
-	return fmt.Sprintf("books_g%d_offset_%d_limit_%d", gen, offset, limit)
-}
-
-func (r *bookRepository) booksListCacheGeneration(ctx context.Context) int64 {
-	if r == nil || r.RedisClient == nil {
-		return 0
-	}
-	n, err := r.RedisClient.Get(ctx, booksListCacheGenKey).Int64()
-	if err != nil {
-		return 0
-	}
-	if n < 0 {
-		return 0
-	}
-	return n
-}
-
-func (r *bookRepository) bumpBooksListCacheGeneration(ctx context.Context) {
-	if r == nil || r.RedisClient == nil {
-		return
-	}
-	_, _ = r.RedisClient.Incr(ctx, booksListCacheGenKey).Result()
-}
-
+// BookRepository is the HTTP surface for book routes (Gin handlers).
 type BookRepository interface {
 	Healthcheck(c *gin.Context)
 	FindBooks(c *gin.Context)
@@ -60,6 +30,16 @@ type BookRepository interface {
 	DeleteBook(c *gin.Context)
 }
 
+type bookRepository struct {
+	svc *service.BookService
+}
+
+// NewBookRepository wires persistence and cache into book HTTP handlers.
+func NewBookRepository(db database.Database, redisClient cache.Cache) *bookRepository {
+	store := repository.NewGormBookStore(db)
+	return &bookRepository{svc: service.NewBookService(store, redisClient)}
+}
+
 func parseIDParam(c *gin.Context) (uint, bool) {
 	if c == nil {
 		return 0, false
@@ -73,26 +53,32 @@ func parseIDParam(c *gin.Context) (uint, bool) {
 	return uint(id), true
 }
 
-// Sentinel errors for FindBooks singleflight so callers can map failures to HTTP responses.
-var (
-	errListBooksSFDB      = errors.New("listBooks singleflight: database")
-	errListBooksSFMarshal = errors.New("listBooks singleflight: marshal")
-	errListBooksSFRedis   = errors.New("listBooks singleflight: redis set")
-)
-
-// bookRepository holds shared resources like database and Redis client
-type bookRepository struct {
-	DB          database.Database
-	RedisClient cache.Cache
-	listBooksSF singleflight.Group
-}
+func parseOffsetLimit(c *gin.Context) (offset, limit int, ok bool) {
+	offsetQuery := strings.TrimSpace(c.DefaultQuery("offset", "0"))
+	limitQuery := strings.TrimSpace(c.DefaultQuery("limit", "10"))
 
-// NewBookRepository returns a bookRepository backed by db and optional redisClient.
-func NewBookRepository(db database.Database, redisClient cache.Cache) *bookRepository {
-	return &bookRepository{
-		DB:          db,
-		RedisClient: redisClient,
+	o, err := strconv.Atoi(offsetQuery)
+	if err != nil {
+		c.JSON(http.StatusBadRequest, gin.H{"error": "Invalid offset format"})
+		return 0, 0, false
+	}
+	l, err := strconv.Atoi(limitQuery)
+	if err != nil {
+		c.JSON(http.StatusBadRequest, gin.H{"error": "Invalid limit format"})
+		return 0, 0, false
 	}
+	if o < 0 {
+		c.JSON(http.StatusBadRequest, gin.H{"error": "offset must be >= 0"})
+		return 0, 0, false
+	}
+	if l < findBooksMinLimit {
+		c.JSON(http.StatusBadRequest, gin.H{"error": "limit must be at least 1"})
+		return 0, 0, false
+	}
+	if l > findBooksMaxLimit {
+		l = findBooksMaxLimit
+	}
+	return o, l, true
 }
 
 // @BasePath /api/v1
@@ -123,82 +109,26 @@ func (r *bookRepository) Healthcheck(c *gin.Context) {
 // @Failure 500 {string} string "Internal Server Error"
 // @Router /books [get]
 func (r *bookRepository) FindBooks(c *gin.Context) {
-	var books []models.Book
-
-	// Query params trimmed so cache keys match parsed integers (no cache fragmentation from " 0 " vs "0").
-	offsetQuery := strings.TrimSpace(c.DefaultQuery("offset", "0"))
-	limitQuery := strings.TrimSpace(c.DefaultQuery("limit", "10"))
-
-	offset, err := strconv.Atoi(offsetQuery)
-	if err != nil {
-		c.JSON(http.StatusBadRequest, gin.H{"error": "Invalid offset format"})
-		return
-	}
-
-	limit, err := strconv.Atoi(limitQuery)
-	if err != nil {
-		c.JSON(http.StatusBadRequest, gin.H{"error": "Invalid limit format"})
-		return
-	}
-
-	if offset < 0 {
-		c.JSON(http.StatusBadRequest, gin.H{"error": "offset must be >= 0"})
-		return
-	}
-	if limit < findBooksMinLimit {
-		c.JSON(http.StatusBadRequest, gin.H{"error": "limit must be at least 1"})
-		return
-	}
-	if limit > findBooksMaxLimit {
-		limit = findBooksMaxLimit
-	}
-
-	reqCtx := c.Request.Context()
-	gen := r.booksListCacheGeneration(reqCtx)
-	cacheKey := booksListDataCacheKey(gen, offset, limit)
-
-	// Try fetching the data from Redis first
-	cachedBooks, err := r.RedisClient.Get(reqCtx, cacheKey).Result()
-	if err == nil {
-		err := json.Unmarshal([]byte(cachedBooks), &books)
-		if err != nil {
-			c.JSON(http.StatusInternalServerError, gin.H{"error": "Failed to unmarshal cached data"})
-			return
-		}
-		c.JSON(http.StatusOK, gin.H{"data": books})
+	offset, limit, ok := parseOffsetLimit(c)
+	if !ok {
 		return
 	}
-
-	// If cache missed, coalesce concurrent loads on the same cache key (cache stampede protection).
-	out, err, _ := r.listBooksSF.Do(cacheKey, func() (interface{}, error) {
-		var loaded []models.Book
-		if err := r.DB.Offset(offset).Limit(limit).Find(&loaded).Error; err != nil {
-			return nil, fmt.Errorf("%w: %v", errListBooksSFDB, err)
-		}
-		serializedBooks, err := json.Marshal(loaded)
-		if err != nil {
-			return nil, fmt.Errorf("%w: %v", errListBooksSFMarshal, err)
-		}
-		if err := r.RedisClient.Set(reqCtx, cacheKey, serializedBooks, time.Minute).Err(); err != nil {
-			return nil, fmt.Errorf("%w: %v", errListBooksSFRedis, err)
-		}
-		return loaded, nil
-	})
+	books, err := r.svc.ListBooks(c.Request.Context(), offset, limit)
 	if err != nil {
 		switch {
-		case errors.Is(err, errListBooksSFDB):
+		case errors.Is(err, service.ErrListBooksDB):
 			c.JSON(http.StatusInternalServerError, gin.H{"error": "Failed to list books"})
-		case errors.Is(err, errListBooksSFMarshal):
+		case errors.Is(err, service.ErrListBooksMarshal):
 			c.JSON(http.StatusInternalServerError, gin.H{"error": "Failed to marshal data"})
-		case errors.Is(err, errListBooksSFRedis):
+		case errors.Is(err, service.ErrListBooksRedis):
 			c.JSON(http.StatusInternalServerError, gin.H{"error": "Failed to set cache"})
+		case errors.Is(err, service.ErrListBooksUnmarshal):
+			c.JSON(http.StatusInternalServerError, gin.H{"error": "Failed to unmarshal cached data"})
 		default:
 			c.JSON(http.StatusInternalServerError, gin.H{"error": "Failed to list books"})
 		}
 		return
 	}
-
-	books = out.([]models.Book)
 	c.JSON(http.StatusOK, gin.H{"data": books})
 }
 
@@ -218,21 +148,15 @@ func (r *bookRepository) FindBooks(c *gin.Context) {
 // @Router /books [post]
 func (r *bookRepository) CreateBook(c *gin.Context) {
 	var input models.CreateBook
-
 	if err := c.ShouldBindJSON(&input); err != nil {
 		c.JSON(http.StatusBadRequest, gin.H{"error": err.Error()})
 		return
 	}
-
-	book := models.Book{Title: input.Title, Author: input.Author}
-
-	if err := r.DB.Create(&book).Error; err != nil {
+	book, err := r.svc.CreateBook(c.Request.Context(), input.Title, input.Author)
+	if err != nil {
 		c.JSON(http.StatusInternalServerError, gin.H{"error": "Failed to create book"})
 		return
 	}
-
-	r.bumpBooksListCacheGeneration(c.Request.Context())
-
 	c.JSON(http.StatusCreated, gin.H{"data": book})
 }
 
@@ -247,18 +171,19 @@ func (r *bookRepository) CreateBook(c *gin.Context) {
 // @Failure 404 {string} string "Book not found"
 // @Router /books/{id} [get]
 func (r *bookRepository) FindBook(c *gin.Context) {
-	var book models.Book
-
 	id, ok := parseIDParam(c)
 	if !ok {
 		return
 	}
-
-	if err := r.DB.FirstByID(&book, id).Error(); err != nil {
-		c.JSON(http.StatusNotFound, gin.H{"error": "book not found"})
+	book, err := r.svc.GetBook(c.Request.Context(), id)
+	if err != nil {
+		if repository.IsBookNotFound(err) {
+			c.JSON(http.StatusNotFound, gin.H{"error": "book not found"})
+			return
+		}
+		c.JSON(http.StatusInternalServerError, gin.H{"error": "Failed to load book"})
 		return
 	}
-
 	c.JSON(http.StatusOK, gin.H{"data": book})
 }
 
@@ -279,31 +204,24 @@ func (r *bookRepository) FindBook(c *gin.Context) {
 // @Failure 500 {string} string "Internal Server Error"
 // @Router /books/{id} [put]
 func (r *bookRepository) UpdateBook(c *gin.Context) {
-	var book models.Book
 	var input models.UpdateBook
-
 	id, ok := parseIDParam(c)
 	if !ok {
 		return
 	}
-
-	if err := r.DB.FirstByID(&book, id).Error(); err != nil {
-		c.JSON(http.StatusNotFound, gin.H{"error": "book not found"})
-		return
-	}
-
 	if err := c.ShouldBindJSON(&input); err != nil {
 		c.JSON(http.StatusBadRequest, gin.H{"error": err.Error()})
 		return
 	}
-
-	if err := r.DB.Model(&book).Updates(models.Book{Title: input.Title, Author: input.Author}).Error; err != nil {
+	book, err := r.svc.UpdateBook(c.Request.Context(), id, input.Title, input.Author)
+	if err != nil {
+		if repository.IsBookNotFound(err) {
+			c.JSON(http.StatusNotFound, gin.H{"error": "book not found"})
+			return
+		}
 		c.JSON(http.StatusInternalServerError, gin.H{"error": "Failed to update book"})
 		return
 	}
-
-	r.bumpBooksListCacheGeneration(c.Request.Context())
-
 	c.JSON(http.StatusOK, gin.H{"data": book})
 }
 
@@ -321,24 +239,17 @@ func (r *bookRepository) UpdateBook(c *gin.Context) {
 // @Failure 500 {string} string "Internal Server Error"
 // @Router /books/{id} [delete]
 func (r *bookRepository) DeleteBook(c *gin.Context) {
-	var book models.Book
-
 	id, ok := parseIDParam(c)
 	if !ok {
 		return
 	}
-
-	if err := r.DB.FirstByID(&book, id).Error(); err != nil {
-		c.JSON(http.StatusNotFound, gin.H{"error": "book not found"})
-		return
-	}
-
-	if err := r.DB.Delete(&book).Error; err != nil {
+	if err := r.svc.DeleteBook(c.Request.Context(), id); err != nil {
+		if repository.IsBookNotFound(err) {
+			c.JSON(http.StatusNotFound, gin.H{"error": "book not found"})
+			return
+		}
 		c.JSON(http.StatusInternalServerError, gin.H{"error": "Failed to delete book"})
 		return
 	}
-
-	r.bumpBooksListCacheGeneration(c.Request.Context())
-
 	c.Status(http.StatusNoContent)
 }