Browse Source

test(validator): add more unit tests for the validation functions

Frédéric Guillot 2 months ago
parent
commit
7b7fd48e15

+ 1 - 0
internal/validator/api_key.go

@@ -9,6 +9,7 @@ import (
 	"miniflux.app/v2/internal/storage"
 )
 
+// ValidateAPIKeyCreation ensures API key creation requests include a description and are unique per user.
 func ValidateAPIKeyCreation(store *storage.Storage, userID int64, request *model.APIKeyCreationRequest) *locale.LocalizedError {
 	if request.Description == "" {
 		return locale.NewLocalizedError("error.fields_mandatory")

+ 3 - 2
internal/validator/enclosure.go

@@ -1,7 +1,7 @@
 // SPDX-FileCopyrightText: Copyright The Miniflux Authors. All rights reserved.
 // SPDX-License-Identifier: Apache-2.0
 
-package validator
+package validator // import "miniflux.app/v2/internal/validator"
 
 import (
 	"errors"
@@ -9,9 +9,10 @@ import (
 	"miniflux.app/v2/internal/model"
 )
 
+// ValidateEnclosureUpdateRequest validates enclosure updates, ensuring media progression is not negative.
 func ValidateEnclosureUpdateRequest(request *model.EnclosureUpdateRequest) error {
 	if request.MediaProgression < 0 {
-		return errors.New(`media progression must an positive integer`)
+		return errors.New(`media progression must be a non-negative integer`)
 	}
 
 	return nil

+ 27 - 0
internal/validator/enclosure_test.go

@@ -0,0 +1,27 @@
+// SPDX-FileCopyrightText: Copyright The Miniflux Authors. All rights reserved.
+// SPDX-License-Identifier: Apache-2.0
+
+package validator // import "miniflux.app/v2/internal/validator"
+
+import (
+    "testing"
+
+    "miniflux.app/v2/internal/model"
+)
+
+func TestValidateEnclosureUpdateRequest(t *testing.T) {
+    request := &model.EnclosureUpdateRequest{MediaProgression: -1}
+    if err := ValidateEnclosureUpdateRequest(request); err == nil {
+        t.Error("A negative media progression should generate an error")
+    }
+
+    request.MediaProgression = 0
+    if err := ValidateEnclosureUpdateRequest(request); err != nil {
+        t.Fatalf("Zero media progression should be accepted: %v", err)
+    }
+
+    request.MediaProgression = 42
+    if err := ValidateEnclosureUpdateRequest(request); err != nil {
+        t.Fatalf("Positive media progression should be accepted: %v", err)
+    }
+}

+ 23 - 1
internal/validator/entry_test.go

@@ -47,7 +47,7 @@ func TestValidateEntryStatus(t *testing.T) {
 }
 
 func TestValidateEntryOrder(t *testing.T) {
-	for _, status := range []string{"id", "status", "changed_at", "published_at", "created_at", "category_title", "category_id"} {
+	for _, status := range []string{"id", "status", "changed_at", "published_at", "created_at", "category_title", "category_id", "title", "author"} {
 		if err := ValidateEntryOrder(status); err != nil {
 			t.Error(`A valid order should not generate any error`)
 		}
@@ -57,3 +57,25 @@ func TestValidateEntryOrder(t *testing.T) {
 		t.Error(`An invalid order should generate a error`)
 	}
 }
+
+func TestValidateEntryModification(t *testing.T) {
+	// Accepts no-op update.
+	if err := ValidateEntryModification(&model.EntryUpdateRequest{}); err != nil {
+		t.Errorf(`A request without changes should not generate any error: %v`, err)
+	}
+
+	empty := ""
+	if err := ValidateEntryModification(&model.EntryUpdateRequest{Title: &empty}); err == nil {
+		t.Error(`An empty title should generate an error`)
+	}
+
+	if err := ValidateEntryModification(&model.EntryUpdateRequest{Content: &empty}); err == nil {
+		t.Error(`An empty content should generate an error`)
+	}
+
+	title := "Title"
+	content := "Content"
+	if err := ValidateEntryModification(&model.EntryUpdateRequest{Title: &title, Content: &content}); err != nil {
+		t.Errorf(`A valid title and content should not generate any error: %v`, err)
+	}
+}

+ 43 - 0
internal/validator/filter.go

@@ -0,0 +1,43 @@
+// SPDX-FileCopyrightText: Copyright The Miniflux Authors. All rights reserved.
+// SPDX-License-Identifier: Apache-2.0
+
+package validator // import "miniflux.app/v2/internal/validator"
+
+import (
+	"slices"
+	"strings"
+
+	"miniflux.app/v2/internal/locale"
+)
+
+func isValidFilterRules(filterEntryRules string, filterType string) *locale.LocalizedError {
+	// Valid Format: FieldName=RegEx\nFieldName=RegEx...
+	fieldNames := []string{"EntryTitle", "EntryURL", "EntryCommentsURL", "EntryContent", "EntryAuthor", "EntryTag", "EntryDate"}
+
+	rules := strings.Split(filterEntryRules, "\n")
+	for i, rule := range rules {
+		// Check if rule starts with a valid fieldName
+		idx := slices.IndexFunc(fieldNames, func(fieldName string) bool { return strings.HasPrefix(rule, fieldName) })
+		if idx == -1 {
+			return locale.NewLocalizedError("error.settings_"+filterType+"_rule_fieldname_invalid", i+1, "'"+strings.Join(fieldNames, "', '")+"'")
+		}
+		fieldName := fieldNames[idx]
+		fieldRegEx, _ := strings.CutPrefix(rule, fieldName)
+
+		// Check if regex begins with a =
+		if !strings.HasPrefix(fieldRegEx, "=") {
+			return locale.NewLocalizedError("error.settings_"+filterType+"_rule_separator_required", i+1)
+		}
+		fieldRegEx = strings.TrimPrefix(fieldRegEx, "=")
+
+		if fieldRegEx == "" {
+			return locale.NewLocalizedError("error.settings_"+filterType+"_rule_regex_required", i+1)
+		}
+
+		// Check if provided pattern is a valid RegEx
+		if !IsValidRegex(fieldRegEx) {
+			return locale.NewLocalizedError("error.settings_"+filterType+"_rule_invalid_regex", i+1)
+		}
+	}
+	return nil
+}

+ 55 - 0
internal/validator/filter_test.go

@@ -0,0 +1,55 @@
+// SPDX-FileCopyrightText: Copyright The Miniflux Authors. All rights reserved.
+// SPDX-License-Identifier: Apache-2.0
+
+package validator // import "miniflux.app/v2/internal/validator"
+
+import "testing"
+
+func TestIsValidFilterRules(t *testing.T) {
+	tests := []struct {
+		name    string
+		rules   string
+		wantErr bool
+	}{
+		{
+			name:    "valid single rule",
+			rules:   "EntryTitle=foo",
+			wantErr: false,
+		},
+		{
+			name:    "valid multiple rules",
+			rules:   "EntryTitle=foo\nEntryContent=bar",
+			wantErr: false,
+		},
+		{
+			name:    "invalid field name",
+			rules:   "Title=foo",
+			wantErr: true,
+		},
+		{
+			name:    "missing separator",
+			rules:   "EntryTitle:foo",
+			wantErr: true,
+		},
+		{
+			name:    "empty regex",
+			rules:   "EntryTitle=",
+			wantErr: true,
+		},
+		{
+			name:    "invalid regex",
+			rules:   "EntryTitle=[",
+			wantErr: true,
+		},
+	}
+
+	for _, tt := range tests {
+		tc := tt
+		t.Run(tc.name, func(t *testing.T) {
+			err := isValidFilterRules(tc.rules, "block")
+			if (err != nil) != tc.wantErr {
+				t.Fatalf("expected error=%v, got %v", tc.wantErr, err)
+			}
+		})
+	}
+}

+ 43 - 0
internal/validator/subscription_test.go

@@ -0,0 +1,43 @@
+// SPDX-FileCopyrightText: Copyright The Miniflux Authors. All rights reserved.
+// SPDX-License-Identifier: Apache-2.0
+
+package validator // import "miniflux.app/v2/internal/validator"
+
+import (
+	"testing"
+
+	"miniflux.app/v2/internal/model"
+)
+
+func TestValidateSubscriptionDiscovery(t *testing.T) {
+	tests := []struct {
+		name    string
+		req     *model.SubscriptionDiscoveryRequest
+		wantErr bool
+	}{
+		{
+			name:    "valid site url",
+			req:     &model.SubscriptionDiscoveryRequest{URL: "https://example.org"},
+			wantErr: false,
+		},
+		{
+			name:    "invalid site url",
+			req:     &model.SubscriptionDiscoveryRequest{URL: "example.org"},
+			wantErr: true,
+		},
+		{
+			name:    "invalid proxy url",
+			req:     &model.SubscriptionDiscoveryRequest{URL: "https://example.org", ProxyURL: "example.org"},
+			wantErr: true,
+		},
+	}
+
+	for _, tc := range tests {
+		tc := tc
+		t.Run(tc.name, func(t *testing.T) {
+			if err := ValidateSubscriptionDiscovery(tc.req); (err != nil) != tc.wantErr {
+				t.Fatalf("expected error %v, got %v", tc.wantErr, err)
+			}
+		})
+	}
+}

+ 0 - 33
internal/validator/user.go

@@ -4,7 +4,6 @@
 package validator // import "miniflux.app/v2/internal/validator"
 
 import (
-	"slices"
 	"strings"
 	"unicode"
 
@@ -257,35 +256,3 @@ func validateMediaPlaybackRate(mediaPlaybackRate float64) *locale.LocalizedError
 	}
 	return nil
 }
-
-func isValidFilterRules(filterEntryRules string, filterType string) *locale.LocalizedError {
-	// Valid Format: FieldName=RegEx\nFieldName=RegEx...
-	fieldNames := []string{"EntryTitle", "EntryURL", "EntryCommentsURL", "EntryContent", "EntryAuthor", "EntryTag", "EntryDate"}
-
-	rules := strings.Split(filterEntryRules, "\n")
-	for i, rule := range rules {
-		// Check if rule starts with a valid fieldName
-		idx := slices.IndexFunc(fieldNames, func(fieldName string) bool { return strings.HasPrefix(rule, fieldName) })
-		if idx == -1 {
-			return locale.NewLocalizedError("error.settings_"+filterType+"_rule_fieldname_invalid", i+1, "'"+strings.Join(fieldNames, "', '")+"'")
-		}
-		fieldName := fieldNames[idx]
-		fieldRegEx, _ := strings.CutPrefix(rule, fieldName)
-
-		// Check if regex begins with a =
-		if !strings.HasPrefix(fieldRegEx, "=") {
-			return locale.NewLocalizedError("error.settings_"+filterType+"_rule_separator_required", i+1)
-		}
-		fieldRegEx = strings.TrimPrefix(fieldRegEx, "=")
-
-		if fieldRegEx == "" {
-			return locale.NewLocalizedError("error.settings_"+filterType+"_rule_regex_required", i+1)
-		}
-
-		// Check if provided pattern is a valid RegEx
-		if !IsValidRegex(fieldRegEx) {
-			return locale.NewLocalizedError("error.settings_"+filterType+"_rule_invalid_regex", i+1)
-		}
-	}
-	return nil
-}

+ 181 - 0
internal/validator/user_test.go

@@ -0,0 +1,181 @@
+// SPDX-FileCopyrightText: Copyright The Miniflux Authors. All rights reserved.
+// SPDX-License-Identifier: Apache-2.0
+
+package validator // import "miniflux.app/v2/internal/validator"
+
+import (
+	"testing"
+
+	"miniflux.app/v2/internal/locale"
+)
+
+func TestValidateUsername(t *testing.T) {
+	scenarios := map[string]*locale.LocalizedError{
+		"userone":          nil,
+		"user.name":        nil,
+		"user@example.com": nil,
+		"john_doe":         nil,
+		"john-doe":         nil,
+		"User123":          nil,
+		"invalid username": locale.NewLocalizedError("error.invalid_username"),
+		"user/path":        locale.NewLocalizedError("error.invalid_username"),
+		"user🙂":            locale.NewLocalizedError("error.invalid_username"),
+	}
+
+	for username, expected := range scenarios {
+		result := validateUsername(username)
+		if expected == nil {
+			if result != nil {
+				t.Errorf(`got an unexpected error for %q instead of nil: %v`, username, result)
+			}
+		} else {
+			if result == nil {
+				t.Errorf(`expected an error, got nil.`)
+			}
+		}
+	}
+}
+
+func TestValidateReadingSpeed(t *testing.T) {
+	tests := map[int]bool{
+		1:   false,
+		100: false,
+		0:   true,
+		-5:  true,
+	}
+
+	for speed, wantErr := range tests {
+		if err := validateReadingSpeed(speed); (err != nil) != wantErr {
+			t.Errorf("reading speed %d error mismatch: got %v wantErr %v", speed, err, wantErr)
+		}
+	}
+}
+
+func TestValidatePassword(t *testing.T) {
+	tests := map[string]bool{
+		"secret":   false,
+		"longpass": false,
+		"short":    true,
+		"":         true,
+	}
+
+	for password, wantErr := range tests {
+		if err := validatePassword(password); (err != nil) != wantErr {
+			t.Errorf("password %q error mismatch: got %v wantErr %v", password, err, wantErr)
+		}
+	}
+}
+
+func TestValidateTheme(t *testing.T) {
+	if err := validateTheme("light_serif"); err != nil {
+		t.Errorf("expected valid theme to pass, got %v", err)
+	}
+
+	if err := validateTheme("unknown"); err == nil {
+		t.Error("expected invalid theme to fail")
+	}
+}
+
+func TestValidateLanguage(t *testing.T) {
+	if err := validateLanguage("en_US"); err != nil {
+		t.Errorf("expected valid language to pass, got %v", err)
+	}
+
+	if err := validateLanguage("xx_YY"); err == nil {
+		t.Error("expected invalid language to fail")
+	}
+}
+
+func TestValidateTimezone(t *testing.T) {
+	if err := validateTimezone("UTC"); err != nil {
+		t.Errorf("expected valid timezone to pass, got %v", err)
+	}
+
+	if err := validateTimezone("Invalid/Zone"); err == nil {
+		t.Error("expected invalid timezone to fail")
+	}
+}
+
+func TestValidateEntryDirection(t *testing.T) {
+	for _, direction := range []string{"asc", "desc"} {
+		if err := validateEntryDirection(direction); err != nil {
+			t.Errorf("expected valid direction %q to pass, got %v", direction, err)
+		}
+	}
+
+	if err := validateEntryDirection("sideways"); err == nil {
+		t.Error("expected invalid direction to fail")
+	}
+}
+
+func TestValidateEntriesPerPage(t *testing.T) {
+	if err := validateEntriesPerPage(1); err != nil {
+		t.Errorf("expected positive entries per page to pass, got %v", err)
+	}
+
+	for _, value := range []int{0, -1} {
+		if err := validateEntriesPerPage(value); err == nil {
+			t.Errorf("expected %d to fail", value)
+		}
+	}
+}
+
+func TestValidateCategoriesSortingOrder(t *testing.T) {
+	for _, order := range []string{"alphabetical", "unread_count"} {
+		if err := validateCategoriesSortingOrder(order); err != nil {
+			t.Errorf("expected valid order %q to pass, got %v", order, err)
+		}
+	}
+
+	if err := validateCategoriesSortingOrder("popularity"); err == nil {
+		t.Error("expected invalid order to fail")
+	}
+}
+
+func TestValidateDisplayMode(t *testing.T) {
+	for _, mode := range []string{"fullscreen", "standalone", "minimal-ui", "browser"} {
+		if err := validateDisplayMode(mode); err != nil {
+			t.Errorf("expected valid mode %q to pass, got %v", mode, err)
+		}
+	}
+
+	if err := validateDisplayMode("windowed"); err == nil {
+		t.Error("expected invalid display mode to fail")
+	}
+}
+
+func TestValidateGestureNav(t *testing.T) {
+	for _, gesture := range []string{"none", "tap", "swipe"} {
+		if err := validateGestureNav(gesture); err != nil {
+			t.Errorf("expected valid gesture %q to pass, got %v", gesture, err)
+		}
+	}
+
+	if err := validateGestureNav("pinch"); err == nil {
+		t.Error("expected invalid gesture to fail")
+	}
+}
+
+func TestValidateDefaultHomePage(t *testing.T) {
+	if err := validateDefaultHomePage("unread"); err != nil {
+		t.Errorf("expected valid home page to pass, got %v", err)
+	}
+
+	if err := validateDefaultHomePage("dashboard"); err == nil {
+		t.Error("expected invalid home page to fail")
+	}
+}
+
+func TestValidateMediaPlaybackRate(t *testing.T) {
+	for _, rate := range []float64{0.25, 1.0, 4.0} {
+		if err := validateMediaPlaybackRate(rate); err != nil {
+			t.Errorf("expected valid rate %.2f to pass, got %v", rate, err)
+		}
+	}
+
+	for _, rate := range []float64{0.1, 4.1} {
+		if err := validateMediaPlaybackRate(rate); err == nil {
+			t.Errorf("expected invalid rate %.2f to fail", rate)
+		}
+	}
+}

+ 4 - 2
internal/validator/validator.go

@@ -47,6 +47,7 @@ func IsValidURL(absoluteURL string) bool {
 	return err == nil
 }
 
+// IsValidDomain verifies a single domain name against length and character constraints.
 func IsValidDomain(domain string) bool {
 	domain = strings.ToLower(domain)
 
@@ -57,9 +58,10 @@ func IsValidDomain(domain string) bool {
 	return domainRegex.MatchString(domain)
 }
 
+// IsValidDomainList verifies a space-separated list of domains for validity.
 func IsValidDomainList(value string) bool {
-	domains := strings.Split(strings.TrimSpace(value), " ")
-	for _, domain := range domains {
+	domains := strings.SplitSeq(strings.TrimSpace(value), " ")
+	for domain := range domains {
 		if !IsValidDomain(domain) {
 			return false
 		}

+ 11 - 18
internal/validator/validator_test.go

@@ -5,8 +5,6 @@ package validator // import "miniflux.app/v2/internal/validator"
 
 import (
 	"testing"
-
-	"miniflux.app/v2/internal/locale"
 )
 
 func TestIsValidURL(t *testing.T) {
@@ -82,24 +80,19 @@ func TestIsValidDomain(t *testing.T) {
 	}
 }
 
-func TestValidateUsername(t *testing.T) {
-	scenarios := map[string]*locale.LocalizedError{
-		"jvoisin":          nil,
-		"j.voisin":         nil,
-		"j@vois.in":        nil,
-		"invalid username": locale.NewLocalizedError("error.invalid_username"),
+func TestIsValidDomainList(t *testing.T) {
+	scenarios := map[string]bool{
+		"example.org":                 true,
+		"example.org example.com":     true,
+		"example.org invalid..domain": false,
+		"example.org example.com:443": false,
+		"":                            false,
 	}
 
-	for username, expected := range scenarios {
-		result := validateUsername(username)
-		if expected == nil {
-			if result != nil {
-				t.Errorf(`got an unexpected error for %q instead of nil: %v`, username, result)
-			}
-		} else {
-			if result == nil {
-				t.Errorf(`expected an error, got nil.`)
-			}
+	for domains, expected := range scenarios {
+		result := IsValidDomainList(domains)
+		if result != expected {
+			t.Errorf(`Unexpected result for %q, got %v instead of %v`, domains, result, expected)
 		}
 	}
 }