enhance validateable interface (#7714)
This PR is part of https://codeberg.org/forgejo/forgejo/pulls/4767 filed by @algernon Validateable is enhanced for less duplication. ## Checklist The [contributor guide](https://forgejo.org/docs/next/contributor/) contains information that will be helpful to first time contributors. There also are a few [conditions for merging Pull Requests in Forgejo repositories](https://codeberg.org/forgejo/governance/src/branch/main/PullRequestsAgreement.md). You are also welcome to join the [Forgejo development chatroom](https://matrix.to/#/#forgejo-development:matrix.org). ### Tests - I added test coverage for Go changes... - [x] in their respective `*_test.go` for unit tests. ### Release notes - [x] I do not want this change to show in the release notes. - [ ] I want the title to show in the release notes with a link to this pull request. - [ ] I want the content of the `release-notes/<pull request number>.md` to be be used for the release notes instead of the title. Co-authored-by: zam <mirco.zachmann@meissa.de> Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/7714 Reviewed-by: Earl Warren <earl-warren@noreply.codeberg.org> Co-authored-by: Michael Jerger <michael.jerger@meissa-gmbh.de> Co-committed-by: Michael Jerger <michael.jerger@meissa-gmbh.de>
This commit is contained in:
		
					parent
					
						
							
								161924abf8
							
						
					
				
			
			
				commit
				
					
						baa93ec66e
					
				
			
		
					 5 changed files with 51 additions and 29 deletions
				
			
		| 
						 | 
				
			
			@ -204,6 +204,7 @@ forgejo.org/modules/util/filebuffer
 | 
			
		|||
 | 
			
		||||
forgejo.org/modules/validation
 | 
			
		||||
	IsErrNotValid
 | 
			
		||||
	ValidateIDExists
 | 
			
		||||
 | 
			
		||||
forgejo.org/modules/web
 | 
			
		||||
	RouteMock
 | 
			
		||||
| 
						 | 
				
			
			
 | 
			
		|||
| 
						 | 
				
			
			@ -143,7 +143,7 @@ func Test_ForgeLikeValidation(t *testing.T) {
 | 
			
		|||
		"actor":"https://repo.prod.meissa.de/api/activitypub/user-id/1",
 | 
			
		||||
	"object":"https://codeberg.org/api/activitypub/repository-id/1",
 | 
			
		||||
	"startTime": "2014-12-31T23:00:00-08:00"}`))
 | 
			
		||||
	if err := validateAndCheckError(sut, "Value bad-type is not contained in allowed values [Like]"); err != nil {
 | 
			
		||||
	if err := validateAndCheckError(sut, "Field type contains the value bad-type, which is not in allowed subset [Like]"); err != nil {
 | 
			
		||||
		t.Error(err)
 | 
			
		||||
	}
 | 
			
		||||
 | 
			
		||||
| 
						 | 
				
			
			@ -154,14 +154,6 @@ func Test_ForgeLikeValidation(t *testing.T) {
 | 
			
		|||
	if err := validateAndCheckError(sut, "StartTime was invalid."); err != nil {
 | 
			
		||||
		t.Error(err)
 | 
			
		||||
	}
 | 
			
		||||
 | 
			
		||||
	sut.UnmarshalJSON([]byte(`{"type":"Wrong",
 | 
			
		||||
		"actor":"https://repo.prod.meissa.de/api/activitypub/user-id/1",
 | 
			
		||||
	  "object":"https://codeberg.org/api/activitypub/repository-id/1",
 | 
			
		||||
	  "startTime": "2014-12-31T23:00:00-08:00"}`))
 | 
			
		||||
	if err := validateAndCheckError(sut, "Value Wrong is not contained in allowed values [Like]"); err != nil {
 | 
			
		||||
		t.Error(err)
 | 
			
		||||
	}
 | 
			
		||||
}
 | 
			
		||||
 | 
			
		||||
func TestActivityValidation_Attack(t *testing.T) {
 | 
			
		||||
| 
						 | 
				
			
			
 | 
			
		|||
| 
						 | 
				
			
			@ -145,7 +145,7 @@ func TestPersonIdValidation(t *testing.T) {
 | 
			
		|||
	sut.HostPort = 443
 | 
			
		||||
	sut.IsPortSupplemented = true
 | 
			
		||||
	sut.UnvalidatedInput = "https://an.other.host/api/v1/activitypub/user-id/1"
 | 
			
		||||
	if sut.Validate()[0] != "Value forgejox is not contained in allowed values [forgejo gitea]" {
 | 
			
		||||
	if sut.Validate()[0] != "Field Source contains the value forgejox, which is not in allowed subset [forgejo gitea]" {
 | 
			
		||||
		t.Errorf("validation error expected but was: %v\n", sut.Validate()[0])
 | 
			
		||||
	}
 | 
			
		||||
}
 | 
			
		||||
| 
						 | 
				
			
			
 | 
			
		|||
| 
						 | 
				
			
			@ -10,6 +10,8 @@ import (
 | 
			
		|||
	"unicode/utf8"
 | 
			
		||||
 | 
			
		||||
	"forgejo.org/modules/timeutil"
 | 
			
		||||
 | 
			
		||||
	ap "github.com/go-ap/activitypub"
 | 
			
		||||
)
 | 
			
		||||
 | 
			
		||||
// ErrNotValid represents an validation error
 | 
			
		||||
| 
						 | 
				
			
			@ -41,6 +43,13 @@ func IsValid(v Validateable) (bool, error) {
 | 
			
		|||
	return true, nil
 | 
			
		||||
}
 | 
			
		||||
 | 
			
		||||
func ValidateIDExists(value ap.Item, name string) []string {
 | 
			
		||||
	if value == nil {
 | 
			
		||||
		return []string{fmt.Sprintf("%v should not be nil", name)}
 | 
			
		||||
	}
 | 
			
		||||
	return ValidateNotEmpty(value.GetID().String(), name)
 | 
			
		||||
}
 | 
			
		||||
 | 
			
		||||
func ValidateNotEmpty(value any, name string) []string {
 | 
			
		||||
	isValid := true
 | 
			
		||||
	switch v := value.(type) {
 | 
			
		||||
| 
						 | 
				
			
			@ -83,5 +92,5 @@ func ValidateOneOf(value any, allowed []any, name string) []string {
 | 
			
		|||
			return []string{}
 | 
			
		||||
		}
 | 
			
		||||
	}
 | 
			
		||||
	return []string{fmt.Sprintf("Value %v is not contained in allowed values %v", value, allowed)}
 | 
			
		||||
	return []string{fmt.Sprintf("Field %s contains the value %v, which is not in allowed subset %v", name, value, allowed)}
 | 
			
		||||
}
 | 
			
		||||
| 
						 | 
				
			
			
 | 
			
		|||
| 
						 | 
				
			
			@ -1,4 +1,4 @@
 | 
			
		|||
// Copyright 2024 The Forgejo Authors. All rights reserved.
 | 
			
		||||
// Copyright 2024, 2025 The Forgejo Authors. All rights reserved.
 | 
			
		||||
// SPDX-License-Identifier: MIT
 | 
			
		||||
 | 
			
		||||
package validation
 | 
			
		||||
| 
						 | 
				
			
			@ -7,6 +7,9 @@ import (
 | 
			
		|||
	"testing"
 | 
			
		||||
 | 
			
		||||
	"forgejo.org/modules/timeutil"
 | 
			
		||||
 | 
			
		||||
	ap "github.com/go-ap/activitypub"
 | 
			
		||||
	"github.com/stretchr/testify/assert"
 | 
			
		||||
)
 | 
			
		||||
 | 
			
		||||
type Sut struct {
 | 
			
		||||
| 
						 | 
				
			
			@ -37,33 +40,50 @@ func Test_IsValid(t *testing.T) {
 | 
			
		|||
 | 
			
		||||
func Test_ValidateNotEmpty_ForString(t *testing.T) {
 | 
			
		||||
	sut := ""
 | 
			
		||||
	if len(ValidateNotEmpty(sut, "dummyField")) == 0 {
 | 
			
		||||
		t.Error("sut should be invalid")
 | 
			
		||||
	}
 | 
			
		||||
	res := ValidateNotEmpty(sut, "dummyField")
 | 
			
		||||
	assert.Len(t, res, 1)
 | 
			
		||||
 | 
			
		||||
	sut = "not empty"
 | 
			
		||||
	if res := ValidateNotEmpty(sut, "dummyField"); len(res) > 0 {
 | 
			
		||||
		t.Errorf("sut should be valid but was %q", res)
 | 
			
		||||
	}
 | 
			
		||||
	res = ValidateNotEmpty(sut, "dummyField")
 | 
			
		||||
	assert.Empty(t, res, 0)
 | 
			
		||||
}
 | 
			
		||||
 | 
			
		||||
func Test_ValidateNotEmpty_ForTimestamp(t *testing.T) {
 | 
			
		||||
	sut := timeutil.TimeStamp(0)
 | 
			
		||||
	if res := ValidateNotEmpty(sut, "dummyField"); len(res) == 0 {
 | 
			
		||||
		t.Error("sut should be invalid")
 | 
			
		||||
	}
 | 
			
		||||
	res := ValidateNotEmpty(sut, "dummyField")
 | 
			
		||||
	assert.Len(t, res, 1)
 | 
			
		||||
 | 
			
		||||
	sut = timeutil.TimeStampNow()
 | 
			
		||||
	if res := ValidateNotEmpty(sut, "dummyField"); len(res) > 0 {
 | 
			
		||||
		t.Errorf("sut should be valid but was %q", res)
 | 
			
		||||
	res = ValidateNotEmpty(sut, "dummyField")
 | 
			
		||||
	assert.Empty(t, res, 0)
 | 
			
		||||
}
 | 
			
		||||
 | 
			
		||||
func Test_ValidateIDExists_ForItem(t *testing.T) {
 | 
			
		||||
	sut := ap.Activity{
 | 
			
		||||
		Object: nil,
 | 
			
		||||
	}
 | 
			
		||||
	res := ValidateIDExists(sut.Object, "dummyField")
 | 
			
		||||
	assert.Len(t, res, 1)
 | 
			
		||||
 | 
			
		||||
	sut = ap.Activity{
 | 
			
		||||
		Object: ap.IRI(""),
 | 
			
		||||
	}
 | 
			
		||||
	res = ValidateIDExists(sut.Object, "dummyField")
 | 
			
		||||
	assert.Len(t, res, 1)
 | 
			
		||||
 | 
			
		||||
	sut = ap.Activity{
 | 
			
		||||
		Object: ap.IRI("https://dummy.link/id"),
 | 
			
		||||
	}
 | 
			
		||||
	res = ValidateIDExists(sut.Object, "dummyField")
 | 
			
		||||
	assert.Empty(t, res, 0)
 | 
			
		||||
}
 | 
			
		||||
 | 
			
		||||
func Test_ValidateMaxLen(t *testing.T) {
 | 
			
		||||
	sut := "0123456789"
 | 
			
		||||
	if len(ValidateMaxLen(sut, 9, "dummyField")) == 0 {
 | 
			
		||||
		t.Error("sut should be invalid")
 | 
			
		||||
	}
 | 
			
		||||
	res := ValidateMaxLen(sut, 9, "dummyField")
 | 
			
		||||
	assert.Len(t, res, 1)
 | 
			
		||||
 | 
			
		||||
	sut = "0123456789"
 | 
			
		||||
	if res := ValidateMaxLen(sut, 11, "dummyField"); len(res) > 0 {
 | 
			
		||||
		t.Errorf("sut should be valid but was %q", res)
 | 
			
		||||
	}
 | 
			
		||||
	res = ValidateMaxLen(sut, 11, "dummyField")
 | 
			
		||||
	assert.Empty(t, res, 0)
 | 
			
		||||
}
 | 
			
		||||
| 
						 | 
				
			
			
 | 
			
		|||
		Loading…
	
	Add table
		Add a link
		
	
		Reference in a new issue