From c4f9f8578a665bcf52161d516121c6319fa4b8b8 Mon Sep 17 00:00:00 2001 From: Clemens Date: Mon, 13 May 2024 11:14:04 +0200 Subject: [PATCH] Introduced ErrNotValid --- models/forgefed/federationhost_repository.go | 8 ++++---- models/forgefed/nodeinfo_test.go | 7 +++++-- models/repo/repo_repository.go | 5 ++--- models/user/user_repository.go | 8 ++++---- modules/forgefed/actor.go | 12 ++++++------ modules/forgefed/actor_test.go | 4 +++- modules/validation/validatable.go | 19 ++++++++++++++++++- modules/validation/validatable_test.go | 6 +++++- 8 files changed, 47 insertions(+), 22 deletions(-) diff --git a/models/forgefed/federationhost_repository.go b/models/forgefed/federationhost_repository.go index b4e72b0ce1..03d8741c58 100644 --- a/models/forgefed/federationhost_repository.go +++ b/models/forgefed/federationhost_repository.go @@ -25,7 +25,7 @@ func GetFederationHost(ctx context.Context, ID int64) (*FederationHost, error) { return nil, fmt.Errorf("FederationInfo record %v does not exist", ID) } if res, err := validation.IsValid(host); !res { - return nil, fmt.Errorf("FederationInfo is not valid: %v", err) + return nil, err } return host, nil } @@ -39,14 +39,14 @@ func FindFederationHostByFqdn(ctx context.Context, fqdn string) (*FederationHost return nil, nil } if res, err := validation.IsValid(host); !res { - return nil, fmt.Errorf("FederationInfo is not valid: %v", err) + return nil, err } return host, nil } func CreateFederationHost(ctx context.Context, host *FederationHost) error { if res, err := validation.IsValid(host); !res { - return fmt.Errorf("FederationInfo is not valid: %v", err) + return err } _, err := db.GetEngine(ctx).Insert(host) return err @@ -54,7 +54,7 @@ func CreateFederationHost(ctx context.Context, host *FederationHost) error { func UpdateFederationHost(ctx context.Context, host *FederationHost) error { if res, err := validation.IsValid(host); !res { - return fmt.Errorf("FederationInfo is not valid: %v", err) + return err } _, err := db.GetEngine(ctx).ID(host.ID).Update(host) return err diff --git a/models/forgefed/nodeinfo_test.go b/models/forgefed/nodeinfo_test.go index ba1bd90be8..4c73bb44d8 100644 --- a/models/forgefed/nodeinfo_test.go +++ b/models/forgefed/nodeinfo_test.go @@ -6,6 +6,7 @@ package forgefed import ( "fmt" "reflect" + "strings" "testing" "code.gitea.io/gitea/modules/validation" @@ -52,12 +53,14 @@ func Test_NodeInfoWellKnownValidate(t *testing.T) { } sut = NodeInfoWellKnown{Href: "./federated-repo.prod.meissa.de/api/v1/nodeinfo"} - if _, err := validation.IsValid(sut); err.Error() != "Href has to be absolute\nValue is not contained in allowed values [http https]" { + _, err := validation.IsValid(sut) + if !validation.IsErrNotValid(err) && strings.Contains(err.Error(), "Href has to be absolute\nValue is not contained in allowed values [http https]") { t.Errorf("validation error expected but was: %v\n", err) } sut = NodeInfoWellKnown{Href: "https://federated-repo.prod.meissa.de/api/v1/nodeinfo?alert=1"} - if _, err := validation.IsValid(sut); err.Error() != "Href may not contain query" { + _, err = validation.IsValid(sut) + if !validation.IsErrNotValid(err) && strings.Contains(err.Error(), "Href has to be absolute\nValue is not contained in allowed values [http https]") { t.Errorf("sut should be valid, %v, %v", sut, err) } } diff --git a/models/repo/repo_repository.go b/models/repo/repo_repository.go index 1835e440fd..8f62e82122 100644 --- a/models/repo/repo_repository.go +++ b/models/repo/repo_repository.go @@ -5,7 +5,6 @@ package repo import ( "context" - "fmt" "code.gitea.io/gitea/models/db" "code.gitea.io/gitea/modules/validation" @@ -26,7 +25,7 @@ func FindFollowingReposByRepoID(ctx context.Context, repoID int64) ([]*Following } for _, followingRepo := range followingRepoList { if res, err := validation.IsValid(*followingRepo); !res { - return make([]*FollowingRepo, 0, maxFollowingRepos), fmt.Errorf("FederationInfo is not valid: %v", err) + return make([]*FollowingRepo, 0, maxFollowingRepos), err } } return followingRepoList, nil @@ -35,7 +34,7 @@ func FindFollowingReposByRepoID(ctx context.Context, repoID int64) ([]*Following func StoreFollowingRepos(ctx context.Context, localRepoID int64, followingRepoList []*FollowingRepo) error { for _, followingRepo := range followingRepoList { if res, err := validation.IsValid(*followingRepo); !res { - return fmt.Errorf("FederationInfo is not valid: %v", err) + return err } } diff --git a/models/user/user_repository.go b/models/user/user_repository.go index 02109eab16..c06441b5c8 100644 --- a/models/user/user_repository.go +++ b/models/user/user_repository.go @@ -18,7 +18,7 @@ func init() { func CreateFederatedUser(ctx context.Context, user *User, federatedUser *FederatedUser) error { if res, err := validation.IsValid(user); !res { - return fmt.Errorf("User is not valid: %v", err) + return err } overwrite := CreateUserOverwriteOptions{ IsActive: optional.Some(false), @@ -38,7 +38,7 @@ func CreateFederatedUser(ctx context.Context, user *User, federatedUser *Federat federatedUser.UserID = user.ID if res, err := validation.IsValid(federatedUser); !res { - return fmt.Errorf("FederatedUser is not valid: %v", err) + return err } _, err = db.GetEngine(ctx).Insert(federatedUser) @@ -69,10 +69,10 @@ func FindFederatedUser(ctx context.Context, externalID string, } if res, err := validation.IsValid(*user); !res { - return nil, nil, fmt.Errorf("FederatedUser is not valid: %v", err) + return nil, nil, err } if res, err := validation.IsValid(*federatedUser); !res { - return nil, nil, fmt.Errorf("FederatedUser is not valid: %v", err) + return nil, nil, err } return user, federatedUser, nil } diff --git a/modules/forgefed/actor.go b/modules/forgefed/actor.go index a34abc2075..d3cae20dec 100644 --- a/modules/forgefed/actor.go +++ b/modules/forgefed/actor.go @@ -32,8 +32,8 @@ func NewActorID(uri string) (ActorID, error) { return ActorID{}, err } - if valid, outcome := validation.IsValid(result); !valid { - return ActorID{}, outcome + if valid, err := validation.IsValid(result); !valid { + return ActorID{}, err } return result, nil @@ -83,8 +83,8 @@ func NewPersonID(uri, source string) (PersonID, error) { // validate Person specific path personID := PersonID{result} - if valid, outcome := validation.IsValid(personID); !valid { - return PersonID{}, outcome + if valid, err := validation.IsValid(personID); !valid { + return PersonID{}, err } return personID, nil @@ -137,8 +137,8 @@ func NewRepositoryID(uri, source string) (RepositoryID, error) { // validate Person specific path repoID := RepositoryID{result} - if valid, outcome := validation.IsValid(repoID); !valid { - return RepositoryID{}, outcome + if valid, err := validation.IsValid(repoID); !valid { + return RepositoryID{}, err } return repoID, nil diff --git a/modules/forgefed/actor_test.go b/modules/forgefed/actor_test.go index 9a1dbd4c3d..a3c01eceb0 100644 --- a/modules/forgefed/actor_test.go +++ b/modules/forgefed/actor_test.go @@ -92,7 +92,9 @@ func TestPersonIdValidation(t *testing.T) { sut.Host = "an.other.host" sut.Port = "" sut.UnvalidatedInput = "https://an.other.host/path/1" - if _, err := validation.IsValid(sut); err.Error() != "path: \"path\" has to be a person specific api path" { + + _, err := validation.IsValid(sut) + if validation.IsErrNotValid(err) && strings.Contains(err.Error(), "path: \"path\" has to be a person specific api path\n") { t.Errorf("validation error expected but was: %v\n", err) } diff --git a/modules/validation/validatable.go b/modules/validation/validatable.go index fc38ad2524..94b5cc135c 100644 --- a/modules/validation/validatable.go +++ b/modules/validation/validatable.go @@ -6,20 +6,37 @@ package validation import ( "fmt" + "reflect" "strings" "unicode/utf8" "code.gitea.io/gitea/modules/timeutil" ) +// ErrNotValid represents an validation error +type ErrNotValid struct { + Message string +} + +func (err ErrNotValid) Error() string { + return fmt.Sprintf("Validation Error: %v", err.Message) +} + +// IsErrNotValid checks if an error is a ErrNotValid. +func IsErrNotValid(err error) bool { + _, ok := err.(ErrNotValid) + return ok +} + type Validateable interface { Validate() []string } func IsValid(v Validateable) (bool, error) { if err := v.Validate(); len(err) > 0 { + typeof := reflect.TypeOf(v) errString := strings.Join(err, "\n") - return false, fmt.Errorf(errString) + return false, ErrNotValid{fmt.Sprint(typeof, ": ", errString)} } return true, nil diff --git a/modules/validation/validatable_test.go b/modules/validation/validatable_test.go index fdc21f3223..919f5a3183 100644 --- a/modules/validation/validatable_test.go +++ b/modules/validation/validatable_test.go @@ -26,9 +26,13 @@ func Test_IsValid(t *testing.T) { t.Errorf("sut expected to be valid: %v\n", sut.Validate()) } sut = Sut{valid: false} - if res, _ := IsValid(sut); res { + res, err := IsValid(sut) + if res { t.Errorf("sut expected to be invalid: %v\n", sut.Validate()) } + if err == nil || !IsErrNotValid(err) || err.Error() != "Validation Error: validation.Sut: invalid" { + t.Errorf("validation error expected, but was %v", err) + } } func Test_ValidateNotEmpty_ForString(t *testing.T) {