Refactor category validation

api/category.go

@@ -5,36 +5,32 @@
 package api // import "miniflux.app/api"
 
 import (
-	"errors"
+	json_parser "encoding/json"
 	"net/http"
 	"time"
 
 	"miniflux.app/http/request"
 	"miniflux.app/http/response/json"
 	"miniflux.app/model"
+	"miniflux.app/validator"
 )
 
 func (h *handler) createCategory(w http.ResponseWriter, r *http.Request) {
 	userID := request.UserID(r)
 
-	categoryRequest, err := decodeCategoryRequest(r.Body)
+	var categoryRequest model.CategoryRequest
+	if err := json_parser.NewDecoder(r.Body).Decode(&categoryRequest); err != nil {
+		json.BadRequest(w, r, err)
+		return
+	}
+
+	if validationErr := validator.ValidateCategoryCreation(h.store, userID, &categoryRequest); validationErr != nil {
+		json.BadRequest(w, r, validationErr.Error())
+		return
+	}
+
+	category, err := h.store.CreateCategory(userID, &categoryRequest)
 	if err != nil {
-		json.BadRequest(w, r, err)
-		return
-	}
-
-	category := &model.Category{UserID: userID, Title: categoryRequest.Title}
-	if err := category.ValidateCategoryCreation(); err != nil {
-		json.BadRequest(w, r, err)
-		return
-	}
-
-	if c, err := h.store.CategoryByTitle(userID, category.Title); err != nil || c != nil {
-		json.BadRequest(w, r, errors.New("This category already exists"))
-		return
-	}
-
-	if err := h.store.CreateCategory(category); err != nil {
 		json.ServerError(w, r, err)
 		return
 	}
@@ -57,18 +53,18 @@ func (h *handler) updateCategory(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	categoryRequest, err := decodeCategoryRequest(r.Body)
+	var categoryRequest model.CategoryRequest
-	if err != nil {
+	if err := json_parser.NewDecoder(r.Body).Decode(&categoryRequest); err != nil {
 		json.BadRequest(w, r, err)
 		return
 	}
 
-	category.Title = categoryRequest.Title
+	if validationErr := validator.ValidateCategoryModification(h.store, userID, category.ID, &categoryRequest); validationErr != nil {
-	if err := category.ValidateCategoryModification(); err != nil {
+		json.BadRequest(w, r, validationErr.Error())
-		json.BadRequest(w, r, err)
 		return
 	}
 
+	categoryRequest.Patch(category)
 	err = h.store.UpdateCategory(category)
 	if err != nil {
 		json.ServerError(w, r, err)
@@ -115,7 +111,7 @@ func (h *handler) removeCategory(w http.ResponseWriter, r *http.Request) {
 	userID := request.UserID(r)
 	categoryID := request.RouteInt64Param(r, "categoryID")
 
-	if !h.store.CategoryExists(userID, categoryID) {
+	if !h.store.CategoryIDExists(userID, categoryID) {
 		json.NotFound(w, r)
 		return
 	}

api/entry.go

@@ -95,7 +95,7 @@ func (h *handler) findEntries(w http.ResponseWriter, r *http.Request, feedID int
 
 	userID := request.UserID(r)
 	categoryID := request.QueryInt64Param(r, "category_id", 0)
-	if categoryID > 0 && !h.store.CategoryExists(userID, categoryID) {
+	if categoryID > 0 && !h.store.CategoryIDExists(userID, categoryID) {
 		json.BadRequest(w, r, errors.New("Invalid category ID"))
 		return
 	}

api/feed.go

@@ -38,7 +38,7 @@ func (h *handler) createFeed(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	if !h.store.CategoryExists(userID, feedInfo.CategoryID) {
+	if !h.store.CategoryIDExists(userID, feedInfo.CategoryID) {
 		json.BadRequest(w, r, errors.New("This category_id doesn't exists or doesn't belongs to this user"))
 		return
 	}
@@ -123,7 +123,7 @@ func (h *handler) updateFeed(w http.ResponseWriter, r *http.Request) {
 
 	feedChanges.Update(originalFeed)
 
-	if !h.store.CategoryExists(userID, originalFeed.Category.ID) {
+	if !h.store.CategoryIDExists(userID, originalFeed.Category.ID) {
 		json.BadRequest(w, r, errors.New("This category_id doesn't exists or doesn't belongs to this user"))
 		return
 	}

api/payload.go

@@ -182,19 +182,3 @@ func decodeEntryStatusRequest(r io.ReadCloser) ([]int64, string, error) {
 
 	return p.EntryIDs, p.Status, nil
 }
-
-type categoryRequest struct {
-	Title string `json:"title"`
-}
-
-func decodeCategoryRequest(r io.ReadCloser) (*categoryRequest, error) {
-	var payload categoryRequest
-
-	decoder := json.NewDecoder(r)
-	defer r.Close()
-	if err := decoder.Decode(&payload); err != nil {
-		return nil, fmt.Errorf("Unable to decode JSON object: %v", err)
-	}
-
-	return &payload, nil
-}

model/category.go

@@ -4,51 +4,28 @@
 
 package model // import "miniflux.app/model"
 
-import (
+import "fmt"
-	"errors"
-	"fmt"
-)
 
-// Category represents a category in the system.
+// Category represents a feed category.
 type Category struct {
-	ID        int64  `json:"id,omitempty"`
+	ID        int64  `json:"id"`
-	Title     string `json:"title,omitempty"`
+	Title     string `json:"title"`
-	UserID    int64  `json:"user_id,omitempty"`
+	UserID    int64  `json:"user_id"`
-	FeedCount int    `json:"nb_feeds,omitempty"`
+	FeedCount int    `json:"-"`
 }
 
 func (c *Category) String() string {
 	return fmt.Sprintf("ID=%d, UserID=%d, Title=%s", c.ID, c.UserID, c.Title)
 }
 
-// ValidateCategoryCreation validates a category during the creation.
+// CategoryRequest represents the request to create or update a category.
-func (c Category) ValidateCategoryCreation() error {
+type CategoryRequest struct {
-	if c.Title == "" {
+	Title string `json:"title"`
-		return errors.New("The title is mandatory")
-	}
-
-	if c.UserID == 0 {
-		return errors.New("The userID is mandatory")
-	}
-
-	return nil
 }
 
-// ValidateCategoryModification validates a category during the modification.
+// Patch updates category fields.
-func (c Category) ValidateCategoryModification() error {
+func (cr *CategoryRequest) Patch(category *Category) {
-	if c.Title == "" {
+	category.Title = cr.Title
-		return errors.New("The title is mandatory")
-	}
-
-	if c.UserID == 0 {
-		return errors.New("The userID is mandatory")
-	}
-
-	if c.ID <= 0 {
-		return errors.New("The ID is mandatory")
-	}
-
-	return nil
 }
 
 // Categories represents a list of categories.

model/category_test.go

@@ -1,61 +0,0 @@
-// Copyright 2017 Frédéric Guillot. All rights reserved.
-// Use of this source code is governed by the Apache 2.0
-// license that can be found in the LICENSE file.
-
-package model // import "miniflux.app/model"
-
-import "testing"
-
-func TestValidateCategoryCreation(t *testing.T) {
-	category := &Category{}
-	if err := category.ValidateCategoryCreation(); err == nil {
-		t.Error(`An empty category should generate an error`)
-	}
-
-	category = &Category{Title: "Test"}
-	if err := category.ValidateCategoryCreation(); err == nil {
-		t.Error(`A category without userID should generate an error`)
-	}
-
-	category = &Category{UserID: 42}
-	if err := category.ValidateCategoryCreation(); err == nil {
-		t.Error(`A category without title should generate an error`)
-	}
-
-	category = &Category{Title: "Test", UserID: 42}
-	if err := category.ValidateCategoryCreation(); err != nil {
-		t.Error(`All required fields are filled, it should not generate any error`)
-	}
-}
-
-func TestValidateCategoryModification(t *testing.T) {
-	category := &Category{}
-	if err := category.ValidateCategoryModification(); err == nil {
-		t.Error(`An empty category should generate an error`)
-	}
-
-	category = &Category{Title: "Test"}
-	if err := category.ValidateCategoryModification(); err == nil {
-		t.Error(`A category without userID should generate an error`)
-	}
-
-	category = &Category{UserID: 42}
-	if err := category.ValidateCategoryModification(); err == nil {
-		t.Error(`A category without title should generate an error`)
-	}
-
-	category = &Category{ID: -1, Title: "Test", UserID: 42}
-	if err := category.ValidateCategoryModification(); err == nil {
-		t.Error(`An invalid categoryID should generate an error`)
-	}
-
-	category = &Category{ID: 0, Title: "Test", UserID: 42}
-	if err := category.ValidateCategoryModification(); err == nil {
-		t.Error(`An invalid categoryID should generate an error`)
-	}
-
-	category = &Category{ID: 1, Title: "Test", UserID: 42}
-	if err := category.ValidateCategoryModification(); err != nil {
-		t.Error(`All required fields are filled, it should not generate any error`)
-	}
-}

reader/handler/handler.go

@@ -50,7 +50,7 @@ type FeedCreationArgs struct {
 func CreateFeed(store *storage.Storage, args *FeedCreationArgs) (*model.Feed, error) {
 	defer timer.ExecutionTime(time.Now(), fmt.Sprintf("[CreateFeed] FeedURL=%s", args.FeedURL))
 
-	if !store.CategoryExists(args.UserID, args.CategoryID) {
+	if !store.CategoryIDExists(args.UserID, args.CategoryID) {
 		return nil, errors.NewLocalizedError(errCategoryNotFound)
 	}
 

reader/opml/handler.go

@@ -65,12 +65,7 @@ func (h *Handler) Import(userID int64, data io.Reader) error {
 				}
 
 				if category == nil {
-					category = &model.Category{
+					category, err = h.store.CreateCategory(userID, &model.CategoryRequest{Title: subscription.CategoryName})
-						UserID: userID,
-						Title:  subscription.CategoryName,
-					}
-
-					err := h.store.CreateCategory(category)
 					if err != nil {
 						logger.Error("[OPML:Import] %v", err)
 						return fmt.Errorf(`unable to create this category: %q`, subscription.CategoryName)

storage/category.go

@@ -15,13 +15,21 @@ import (
 // AnotherCategoryExists checks if another category exists with the same title.
 func (s *Storage) AnotherCategoryExists(userID, categoryID int64, title string) bool {
 	var result bool
-	query := `SELECT true FROM categories WHERE user_id=$1 AND id != $2 AND title=$3`
+	query := `SELECT true FROM categories WHERE user_id=$1 AND id != $2 AND lower(title)=lower($3) LIMIT 1`
 	s.db.QueryRow(query, userID, categoryID, title).Scan(&result)
 	return result
 }
 
-// CategoryExists checks if the given category exists into the database.
+// CategoryTitleExists checks if the given category exists into the database.
-func (s *Storage) CategoryExists(userID, categoryID int64) bool {
+func (s *Storage) CategoryTitleExists(userID int64, title string) bool {
+	var result bool
+	query := `SELECT true FROM categories WHERE user_id=$1 AND lower(title)=lower($2) LIMIT 1`
+	s.db.QueryRow(query, userID, title).Scan(&result)
+	return result
+}
+
+// CategoryIDExists checks if the given category exists into the database.
+func (s *Storage) CategoryIDExists(userID, categoryID int64) bool {
 	var result bool
 	query := `SELECT true FROM categories WHERE user_id=$1 AND id=$2`
 	s.db.QueryRow(query, userID, categoryID).Scan(&result)
@@ -135,26 +143,34 @@ func (s *Storage) CategoriesWithFeedCount(userID int64) (model.Categories, error
 }
 
 // CreateCategory creates a new category.
-func (s *Storage) CreateCategory(category *model.Category) error {
+func (s *Storage) CreateCategory(userID int64, request *model.CategoryRequest) (*model.Category, error) {
+	var category model.Category
+
 	query := `
 		INSERT INTO categories
 			(user_id, title)
 		VALUES
 			($1, $2)
 		RETURNING
-			id
+			id,
+			user_id,
+			title
 	`
 	err := s.db.QueryRow(
 		query,
-		category.UserID,
+		userID,
-		category.Title,
+		request.Title,
-	).Scan(&category.ID)
+	).Scan(
+		&category.ID,
+		&category.UserID,
+		&category.Title,
+	)
 
 	if err != nil {
-		return fmt.Errorf(`store: unable to create category: %v`, err)
+		return nil, fmt.Errorf(`store: unable to create category %q: %v`, request.Title, err)
 	}
 
-	return nil
+	return &category, nil
 }
 
 // UpdateCategory updates an existing category.

ui/category_save.go

@@ -15,10 +15,11 @@ import (
 	"miniflux.app/ui/form"
 	"miniflux.app/ui/session"
 	"miniflux.app/ui/view"
+	"miniflux.app/validator"
 )
 
 func (h *handler) saveCategory(w http.ResponseWriter, r *http.Request) {
-	user, err := h.store.UserByID(request.UserID(r))
+	loggedUser, err := h.store.UserByID(request.UserID(r))
 	if err != nil {
 		html.ServerError(w, r, err)
 		return
@@ -30,34 +31,19 @@ func (h *handler) saveCategory(w http.ResponseWriter, r *http.Request) {
 	view := view.New(h.tpl, r, sess)
 	view.Set("form", categoryForm)
 	view.Set("menu", "categories")
-	view.Set("user", user)
+	view.Set("user", loggedUser)
-	view.Set("countUnread", h.store.CountUnreadEntries(user.ID))
+	view.Set("countUnread", h.store.CountUnreadEntries(loggedUser.ID))
-	view.Set("countErrorFeeds", h.store.CountUserFeedsWithErrors(user.ID))
+	view.Set("countErrorFeeds", h.store.CountUserFeedsWithErrors(loggedUser.ID))
 
-	if err := categoryForm.Validate(); err != nil {
+	categoryRequest := &model.CategoryRequest{Title: categoryForm.Title}
-		view.Set("errorMessage", err.Error())
+
+	if validationErr := validator.ValidateCategoryCreation(h.store, loggedUser.ID, categoryRequest); validationErr != nil {
+		view.Set("errorMessage", validationErr.TranslationKey)
 		html.OK(w, r, view.Render("create_category"))
 		return
 	}
 
-	duplicateCategory, err := h.store.CategoryByTitle(user.ID, categoryForm.Title)
+	if _, err = h.store.CreateCategory(loggedUser.ID, categoryRequest); err != nil {
-	if err != nil {
-		html.ServerError(w, r, err)
-		return
-	}
-
-	if duplicateCategory != nil {
-		view.Set("errorMessage", "error.category_already_exists")
-		html.OK(w, r, view.Render("create_category"))
-		return
-	}
-
-	category := model.Category{
-		Title:  categoryForm.Title,
-		UserID: user.ID,
-	}
-
-	if err = h.store.CreateCategory(&category); err != nil {
 		logger.Error("[UI:SaveCategory] %v", err)
 		view.Set("errorMessage", "error.unable_to_create_category")
 		html.OK(w, r, view.Render("create_category"))

ui/category_update.go

@@ -11,13 +11,15 @@ import (
 	"miniflux.app/http/response/html"
 	"miniflux.app/http/route"
 	"miniflux.app/logger"
+	"miniflux.app/model"
 	"miniflux.app/ui/form"
 	"miniflux.app/ui/session"
 	"miniflux.app/ui/view"
+	"miniflux.app/validator"
 )
 
 func (h *handler) updateCategory(w http.ResponseWriter, r *http.Request) {
-	user, err := h.store.UserByID(request.UserID(r))
+	loggedUser, err := h.store.UserByID(request.UserID(r))
 	if err != nil {
 		html.ServerError(w, r, err)
 		return
@@ -42,24 +44,20 @@ func (h *handler) updateCategory(w http.ResponseWriter, r *http.Request) {
 	view.Set("form", categoryForm)
 	view.Set("category", category)
 	view.Set("menu", "categories")
-	view.Set("user", user)
+	view.Set("user", loggedUser)
-	view.Set("countUnread", h.store.CountUnreadEntries(user.ID))
+	view.Set("countUnread", h.store.CountUnreadEntries(loggedUser.ID))
-	view.Set("countErrorFeeds", h.store.CountUserFeedsWithErrors(user.ID))
+	view.Set("countErrorFeeds", h.store.CountUserFeedsWithErrors(loggedUser.ID))
 
-	if err := categoryForm.Validate(); err != nil {
+	categoryRequest := &model.CategoryRequest{Title: categoryForm.Title}
-		view.Set("errorMessage", err.Error())
+
-		html.OK(w, r, view.Render("edit_category"))
+	if validationErr := validator.ValidateCategoryModification(h.store, loggedUser.ID, category.ID, categoryRequest); validationErr != nil {
+		view.Set("errorMessage", validationErr.TranslationKey)
+		html.OK(w, r, view.Render("create_category"))
 		return
 	}
 
-	if h.store.AnotherCategoryExists(user.ID, category.ID, categoryForm.Title) {
+	categoryRequest.Patch(category)
-		view.Set("errorMessage", "error.category_already_exists")
+	if err := h.store.UpdateCategory(category); err != nil {
-		html.OK(w, r, view.Render("edit_category"))
-		return
-	}
-
-	err = h.store.UpdateCategory(categoryForm.Merge(category))
-	if err != nil {
 		logger.Error("[UI:UpdateCategory] %v", err)
 		view.Set("errorMessage", "error.unable_to_update_category")
 		html.OK(w, r, view.Render("edit_category"))

ui/form/category.go

@@ -6,9 +6,6 @@ package form // import "miniflux.app/ui/form"
 
 import (
 	"net/http"
-
-	"miniflux.app/errors"
-	"miniflux.app/model"
 )
 
 // CategoryForm represents a feed form in the UI
@@ -16,20 +13,6 @@ type CategoryForm struct {
 	Title string
 }
 
-// Validate makes sure the form values are valid.
-func (c CategoryForm) Validate() error {
-	if c.Title == "" {
-		return errors.NewLocalizedError("error.title_required")
-	}
-	return nil
-}
-
-// Merge update the given category fields.
-func (c CategoryForm) Merge(category *model.Category) *model.Category {
-	category.Title = c.Title
-	return category
-}
-
 // NewCategoryForm returns a new CategoryForm.
 func NewCategoryForm(r *http.Request) *CategoryForm {
 	return &CategoryForm{

validator/category.go

@@ -0,0 +1,36 @@
+// Copyright 2021 Frédéric Guillot. All rights reserved.
+// Use of this source code is governed by the Apache 2.0
+// license that can be found in the LICENSE file.
+
+package validator // import "miniflux.app/validator"
+
+import (
+	"miniflux.app/model"
+	"miniflux.app/storage"
+)
+
+// ValidateCategoryCreation validates category creation.
+func ValidateCategoryCreation(store *storage.Storage, userID int64, request *model.CategoryRequest) *ValidationError {
+	if request.Title == "" {
+		return NewValidationError("error.title_required")
+	}
+
+	if store.CategoryTitleExists(userID, request.Title) {
+		return NewValidationError("error.category_already_exists")
+	}
+
+	return nil
+}
+
+// ValidateCategoryModification validates category modification.
+func ValidateCategoryModification(store *storage.Storage, userID, categoryID int64, request *model.CategoryRequest) *ValidationError {
+	if request.Title == "" {
+		return NewValidationError("error.title_required")
+	}
+
+	if store.AnotherCategoryExists(userID, categoryID, request.Title) {
+		return NewValidationError("error.category_already_exists")
+	}
+
+	return nil
+}