Refactor entry validation

api/api.go

@@ -13,6 +13,11 @@ import (
 	"github.com/gorilla/mux"
 )
 
+type handler struct {
+	store *storage.Storage
+	pool  *worker.Pool
+}
+
 // Serve declares API routes for the application.
 func Serve(router *mux.Router, store *storage.Storage, pool *worker.Pool) {
 	handler := &handler{store, pool}

api/doc.go

@@ -1,10 +0,0 @@
-// Copyright 2018 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 api implements API endpoints for the application.
-
-*/
-package api // import "miniflux.app/api"

api/entry.go

@@ -5,6 +5,7 @@
 package api // import "miniflux.app/api"
 
 import (
+	json_parser "encoding/json"
 	"errors"
 	"net/http"
 	"time"
@@ -13,6 +14,7 @@ import (
 	"miniflux.app/http/response/json"
 	"miniflux.app/model"
 	"miniflux.app/storage"
+	"miniflux.app/validator"
 )
 
 func (h *handler) getFeedEntry(w http.ResponseWriter, r *http.Request) {
@@ -68,27 +70,27 @@ func (h *handler) getEntries(w http.ResponseWriter, r *http.Request) {
 func (h *handler) findEntries(w http.ResponseWriter, r *http.Request, feedID int64) {
 	statuses := request.QueryStringParamList(r, "status")
 	for _, status := range statuses {
-		if err := model.ValidateEntryStatus(status); err != nil {
+		if err := validator.ValidateEntryStatus(status); err != nil {
 			json.BadRequest(w, r, err)
 			return
 		}
 	}
 
 	order := request.QueryStringParam(r, "order", model.DefaultSortingOrder)
-	if err := model.ValidateEntryOrder(order); err != nil {
+	if err := validator.ValidateEntryOrder(order); err != nil {
 		json.BadRequest(w, r, err)
 		return
 	}
 
 	direction := request.QueryStringParam(r, "direction", model.DefaultSortingDirection)
-	if err := model.ValidateDirection(direction); err != nil {
+	if err := validator.ValidateDirection(direction); err != nil {
 		json.BadRequest(w, r, err)
 		return
 	}
 
 	limit := request.QueryIntParam(r, "limit", 100)
 	offset := request.QueryIntParam(r, "offset", 0)
-	if err := model.ValidateRange(offset, limit); err != nil {
+	if err := validator.ValidateRange(offset, limit); err != nil {
 		json.BadRequest(w, r, err)
 		return
 	}
@@ -132,18 +134,18 @@ func (h *handler) findEntries(w http.ResponseWriter, r *http.Request, feedID int
 }
 
 func (h *handler) setEntryStatus(w http.ResponseWriter, r *http.Request) {
-	entryIDs, status, err := decodeEntryStatusRequest(r.Body)
-	if err != nil {
-		json.BadRequest(w, r, errors.New("Invalid JSON payload"))
+	var entriesStatusUpdateRequest model.EntriesStatusUpdateRequest
+	if err := json_parser.NewDecoder(r.Body).Decode(&entriesStatusUpdateRequest); err != nil {
+		json.BadRequest(w, r, err)
 		return
 	}
 
-	if err := model.ValidateEntryStatus(status); err != nil {
+	if err := validator.ValidateEntriesStatusUpdateRequest(&entriesStatusUpdateRequest); err != nil {
 		json.BadRequest(w, r, err)
 		return
 	}
 
-	if err := h.store.SetEntriesStatus(request.UserID(r), entryIDs, status); err != nil {
+	if err := h.store.SetEntriesStatus(request.UserID(r), entriesStatusUpdateRequest.EntryIDs, entriesStatusUpdateRequest.Status); err != nil {
 		json.ServerError(w, r, err)
 		return
 	}

api/handler.go

@@ -1,15 +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 api // import "miniflux.app/api"
-
-import (
-	"miniflux.app/storage"
-	"miniflux.app/worker"
-)
-
-type handler struct {
-	store *storage.Storage
-	pool  *worker.Pool
-}

api/payload.go

@@ -5,10 +5,6 @@
 package api // import "miniflux.app/api"
 
 import (
-	"encoding/json"
-	"fmt"
-	"io"
-
 	"miniflux.app/model"
 )
 
@@ -26,19 +22,3 @@ type entriesResponse struct {
 type feedCreationResponse struct {
 	FeedID int64 `json:"feed_id"`
 }
-
-func decodeEntryStatusRequest(r io.ReadCloser) ([]int64, string, error) {
-	type payload struct {
-		EntryIDs []int64 `json:"entry_ids"`
-		Status   string  `json:"status"`
-	}
-
-	var p payload
-	decoder := json.NewDecoder(r)
-	defer r.Close()
-	if err := decoder.Decode(&p); err != nil {
-		return nil, "", fmt.Errorf("invalid JSON payload: %v", err)
-	}
-
-	return p.EntryIDs, p.Status, nil
-}

model/entry.go

@@ -5,11 +5,10 @@
 package model // import "miniflux.app/model"
 
 import (
-	"fmt"
 	"time"
 )
 
-// Entry statuses
+// Entry statuses and default sorting order.
 const (
 	EntryStatusUnread       = "unread"
 	EntryStatusRead         = "read"
@@ -35,52 +34,15 @@ type Entry struct {
 	ShareCode   string        `json:"share_code"`
 	Starred     bool          `json:"starred"`
 	ReadingTime int           `json:"reading_time"`
-	Enclosures  EnclosureList `json:"enclosures,omitempty"`
+	Enclosures  EnclosureList `json:"enclosures"`
 	Feed        *Feed         `json:"feed,omitempty"`
 }
 
 // Entries represents a list of entries.
 type Entries []*Entry
 
-// ValidateEntryStatus makes sure the entry status is valid.
-func ValidateEntryStatus(status string) error {
-	switch status {
-	case EntryStatusRead, EntryStatusUnread, EntryStatusRemoved:
-		return nil
-	}
-
-	return fmt.Errorf(`Invalid entry status, valid status values are: "%s", "%s" and "%s"`, EntryStatusRead, EntryStatusUnread, EntryStatusRemoved)
-}
-
-// ValidateEntryOrder makes sure the sorting order is valid.
-func ValidateEntryOrder(order string) error {
-	switch order {
-	case "id", "status", "changed_at", "published_at", "created_at", "category_title", "category_id":
-		return nil
-	}
-
-	return fmt.Errorf(`Invalid entry order, valid order values are: "id", "status", "changed_at", "published_at", "created_at", "category_title", "category_id"`)
-}
-
-// ValidateDirection makes sure the sorting direction is valid.
-func ValidateDirection(direction string) error {
-	switch direction {
-	case "asc", "desc":
-		return nil
-	}
-
-	return fmt.Errorf(`Invalid direction, valid direction values are: "asc" or "desc"`)
-}
-
-// ValidateRange makes sure the offset/limit values are valid.
-func ValidateRange(offset, limit int) error {
-	if offset < 0 {
-		return fmt.Errorf(`Offset value should be >= 0`)
-	}
-
-	if limit < 0 {
-		return fmt.Errorf(`Limit value should be >= 0`)
-	}
-
-	return nil
+// EntriesStatusUpdateRequest represents a request to change entries status.
+type EntriesStatusUpdateRequest struct {
+	EntryIDs []int64 `json:"entry_ids"`
+	Status   string  `json:"status"`
 }

model/entry_test.go

@@ -1,57 +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 TestValidateEntryStatus(t *testing.T) {
-	for _, status := range []string{EntryStatusRead, EntryStatusUnread, EntryStatusRemoved} {
-		if err := ValidateEntryStatus(status); err != nil {
-			t.Error(`A valid status should not generate any error`)
-		}
-	}
-
-	if err := ValidateEntryStatus("invalid"); err == nil {
-		t.Error(`An invalid status should generate a error`)
-	}
-}
-
-func TestValidateEntryOrder(t *testing.T) {
-	for _, status := range []string{"id", "status", "changed_at", "published_at", "created_at", "category_title", "category_id"} {
-		if err := ValidateEntryOrder(status); err != nil {
-			t.Error(`A valid order should not generate any error`)
-		}
-	}
-
-	if err := ValidateEntryOrder("invalid"); err == nil {
-		t.Error(`An invalid order should generate a error`)
-	}
-}
-
-func TestValidateEntryDirection(t *testing.T) {
-	for _, status := range []string{"asc", "desc"} {
-		if err := ValidateDirection(status); err != nil {
-			t.Error(`A valid direction should not generate any error`)
-		}
-	}
-
-	if err := ValidateDirection("invalid"); err == nil {
-		t.Error(`An invalid direction should generate a error`)
-	}
-}
-
-func TestValidateRange(t *testing.T) {
-	if err := ValidateRange(-1, 0); err == nil {
-		t.Error(`An invalid offset should generate a error`)
-	}
-
-	if err := ValidateRange(0, -1); err == nil {
-		t.Error(`An invalid limit should generate a error`)
-	}
-
-	if err := ValidateRange(42, 42); err != nil {
-		t.Error(`A valid offset and limit should not generate any error`)
-	}
-}

tests/entry_test.go

@@ -154,15 +154,15 @@ func TestFilterEntriesByStatuses(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	if err := client.UpdateEntries([]int64{results.Entries[0].ID}, "read"); err != nil {
+	if err := client.UpdateEntries([]int64{results.Entries[0].ID}, miniflux.EntryStatusRead); err != nil {
 		t.Fatal(err)
 	}
 
-	if err := client.UpdateEntries([]int64{results.Entries[1].ID}, "removed"); err != nil {
+	if err := client.UpdateEntries([]int64{results.Entries[1].ID}, miniflux.EntryStatusRemoved); err != nil {
 		t.Fatal(err)
 	}
 
-	results, err = client.Entries(&miniflux.Filter{Statuses: []string{"read", "removed"}})
+	results, err = client.Entries(&miniflux.Filter{Statuses: []string{miniflux.EntryStatusRead, miniflux.EntryStatusRemoved}})
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -282,7 +282,12 @@ func TestUpdateStatus(t *testing.T) {
 
 	err = client.UpdateEntries([]int64{result.Entries[0].ID}, "invalid")
 	if err == nil {
-		t.Fatal(`Invalid entry status should ne be accepted`)
+		t.Fatal(`Invalid entry status should not be accepted`)
+	}
+
+	err = client.UpdateEntries([]int64{}, miniflux.EntryStatusRead)
+	if err == nil {
+		t.Fatal(`An empty list of entry should not be accepted`)
 	}
 }
 

ui/entry_update_status.go

@@ -5,27 +5,28 @@
 package ui // import "miniflux.app/ui"
 
 import (
-	"errors"
+	json_parser "encoding/json"
 	"net/http"
 
 	"miniflux.app/http/request"
 	"miniflux.app/http/response/json"
+	"miniflux.app/model"
+	"miniflux.app/validator"
 )
 
 func (h *handler) updateEntriesStatus(w http.ResponseWriter, r *http.Request) {
-	entryIDs, status, err := decodeEntryStatusPayload(r.Body)
-	if err != nil {
+	var entriesStatusUpdateRequest model.EntriesStatusUpdateRequest
+	if err := json_parser.NewDecoder(r.Body).Decode(&entriesStatusUpdateRequest); err != nil {
 		json.BadRequest(w, r, err)
 		return
 	}
 
-	if len(entryIDs) == 0 {
-		json.BadRequest(w, r, errors.New("The list of entry IDs is empty"))
+	if err := validator.ValidateEntriesStatusUpdateRequest(&entriesStatusUpdateRequest); err != nil {
+		json.BadRequest(w, r, err)
 		return
 	}
 
-	err = h.store.SetEntriesStatus(request.UserID(r), entryIDs, status)
-	if err != nil {
+	if err := h.store.SetEntriesStatus(request.UserID(r), entriesStatusUpdateRequest.EntryIDs, entriesStatusUpdateRequest.Status); err != nil {
 		json.ServerError(w, r, err)
 		return
 	}

ui/payload.go

@@ -1,33 +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 ui  // import "miniflux.app/ui"
-
-import (
-	"encoding/json"
-	"fmt"
-	"io"
-
-	"miniflux.app/model"
-)
-
-func decodeEntryStatusPayload(r io.ReadCloser) (entryIDs []int64, status string, err error) {
-	type payload struct {
-		EntryIDs []int64 `json:"entry_ids"`
-		Status   string  `json:"status"`
-	}
-
-	var p payload
-	decoder := json.NewDecoder(r)
-	defer r.Close()
-	if err = decoder.Decode(&p); err != nil {
-		return nil, "", fmt.Errorf("invalid JSON payload: %v", err)
-	}
-
-	if err := model.ValidateEntryStatus(p.Status); err != nil {
-		return nil, "", err
-	}
-
-	return p.EntryIDs, p.Status, nil
-}

validator/entry.go

@@ -0,0 +1,40 @@
+// 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 (
+	"fmt"
+
+	"miniflux.app/model"
+)
+
+// ValidateEntriesStatusUpdateRequest validates a status update for a list of entries.
+func ValidateEntriesStatusUpdateRequest(request *model.EntriesStatusUpdateRequest) error {
+	if len(request.EntryIDs) == 0 {
+		return fmt.Errorf(`The list of entries cannot be empty`)
+	}
+
+	return ValidateEntryStatus(request.Status)
+}
+
+// ValidateEntryStatus makes sure the entry status is valid.
+func ValidateEntryStatus(status string) error {
+	switch status {
+	case model.EntryStatusRead, model.EntryStatusUnread, model.EntryStatusRemoved:
+		return nil
+	}
+
+	return fmt.Errorf(`Invalid entry status, valid status values are: "%s", "%s" and "%s"`, model.EntryStatusRead, model.EntryStatusUnread, model.EntryStatusRemoved)
+}
+
+// ValidateEntryOrder makes sure the sorting order is valid.
+func ValidateEntryOrder(order string) error {
+	switch order {
+	case "id", "status", "changed_at", "published_at", "created_at", "category_title", "category_id":
+		return nil
+	}
+
+	return fmt.Errorf(`Invalid entry order, valid order values are: "id", "status", "changed_at", "published_at", "created_at", "category_title", "category_id"`)
+}

validator/entry_test.go

@@ -0,0 +1,60 @@
+// 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 (
+	"testing"
+
+	"miniflux.app/model"
+)
+
+func TestValidateEntriesStatusUpdateRequest(t *testing.T) {
+	err := ValidateEntriesStatusUpdateRequest(&model.EntriesStatusUpdateRequest{
+		Status:   model.EntryStatusRead,
+		EntryIDs: []int64{int64(123), int64(456)},
+	})
+	if err != nil {
+		t.Error(`A valid request should not be rejected`)
+	}
+
+	err = ValidateEntriesStatusUpdateRequest(&model.EntriesStatusUpdateRequest{
+		Status: model.EntryStatusRead,
+	})
+	if err == nil {
+		t.Error(`An empty list of entries is not valid`)
+	}
+
+	err = ValidateEntriesStatusUpdateRequest(&model.EntriesStatusUpdateRequest{
+		Status:   "invalid",
+		EntryIDs: []int64{int64(123)},
+	})
+	if err == nil {
+		t.Error(`Only a valid status should be accepted`)
+	}
+}
+
+func TestValidateEntryStatus(t *testing.T) {
+	for _, status := range []string{model.EntryStatusRead, model.EntryStatusUnread, model.EntryStatusRemoved} {
+		if err := ValidateEntryStatus(status); err != nil {
+			t.Error(`A valid status should not generate any error`)
+		}
+	}
+
+	if err := ValidateEntryStatus("invalid"); err == nil {
+		t.Error(`An invalid status should generate a error`)
+	}
+}
+
+func TestValidateEntryOrder(t *testing.T) {
+	for _, status := range []string{"id", "status", "changed_at", "published_at", "created_at", "category_title", "category_id"} {
+		if err := ValidateEntryOrder(status); err != nil {
+			t.Error(`A valid order should not generate any error`)
+		}
+	}
+
+	if err := ValidateEntryOrder("invalid"); err == nil {
+		t.Error(`An invalid order should generate a error`)
+	}
+}

validator/validator.go

@@ -6,6 +6,7 @@ package validator // import "miniflux.app/validator"
 
 import (
 	"errors"
+	"fmt"
 	"net/url"
 
 	"miniflux.app/locale"
@@ -29,6 +30,29 @@ func (v *ValidationError) Error() error {
 	return errors.New(v.String())
 }
 
+// ValidateRange makes sure the offset/limit values are valid.
+func ValidateRange(offset, limit int) error {
+	if offset < 0 {
+		return fmt.Errorf(`Offset value should be >= 0`)
+	}
+
+	if limit < 0 {
+		return fmt.Errorf(`Limit value should be >= 0`)
+	}
+
+	return nil
+}
+
+// ValidateDirection makes sure the sorting direction is valid.
+func ValidateDirection(direction string) error {
+	switch direction {
+	case "asc", "desc":
+		return nil
+	}
+
+	return fmt.Errorf(`Invalid direction, valid direction values are: "asc" or "desc"`)
+}
+
 func isValidURL(absoluteURL string) bool {
 	_, err := url.ParseRequestURI(absoluteURL)
 	return err == nil

validator/validator_test.go

@@ -20,3 +20,29 @@ func TestIsValidURL(t *testing.T) {
 		}
 	}
 }
+
+func TestValidateRange(t *testing.T) {
+	if err := ValidateRange(-1, 0); err == nil {
+		t.Error(`An invalid offset should generate a error`)
+	}
+
+	if err := ValidateRange(0, -1); err == nil {
+		t.Error(`An invalid limit should generate a error`)
+	}
+
+	if err := ValidateRange(42, 42); err != nil {
+		t.Error(`A valid offset and limit should not generate any error`)
+	}
+}
+
+func TestValidateDirection(t *testing.T) {
+	for _, status := range []string{"asc", "desc"} {
+		if err := ValidateDirection(status); err != nil {
+			t.Error(`A valid direction should not generate any error`)
+		}
+	}
+
+	if err := ValidateDirection("invalid"); err == nil {
+		t.Error(`An invalid direction should generate a error`)
+	}
+}