enhance test & fix reviews

5 files changed, 37 insertions, 11 deletions

diff --git a/models/forgefed/federationhost.go b/models/forgefed/federationhost.go
index eb1183a2a7..b60c0c39cf 100644
--- a/models/forgefed/federationhost.go
+++ b/models/forgefed/federationhost.go
@@ -19,11 +19,11 @@ type FederationHost struct {
 	HostFqdn       string             `xorm:"host_fqdn UNIQUE INDEX VARCHAR(255) NOT NULL"`
 	NodeInfo       NodeInfo           `xorm:"extends NOT NULL"`
 	LatestActivity time.Time          `xorm:"NOT NULL"`
-	Create         timeutil.TimeStamp `xorm:"created"`
+	Created        timeutil.TimeStamp `xorm:"created"`
 	Updated        timeutil.TimeStamp `xorm:"updated"`
 }
 
-// Factory function for PersonID. Created struct is asserted to be valid
+// Factory function for FederationHost. Created struct is asserted to be valid.
 func NewFederationHost(nodeInfo NodeInfo, hostFqdn string) (FederationHost, error) {
 	result := FederationHost{
 		HostFqdn: strings.ToLower(hostFqdn),
@@ -45,7 +45,7 @@ func (host FederationHost) Validate() []string {
 		result = append(result, fmt.Sprintf("HostFqdn has to be lower case but was: %v", host.HostFqdn))
 	}
 	if !host.LatestActivity.IsZero() && host.LatestActivity.After(time.Now().Add(10*time.Minute)) {
-		result = append(result, fmt.Sprintf("Latest Activity may not be far futurer: %v", host.LatestActivity))
+		result = append(result, fmt.Sprintf("Latest Activity cannot be in the far future: %v", host.LatestActivity))
 	}
 
 	return result
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/federationhost_test.go b/models/forgefed/federationhost_test.go
index 04f941d93d..ea5494c6e9 100644
--- a/models/forgefed/federationhost_test.go
+++ b/models/forgefed/federationhost_test.go
@@ -4,6 +4,7 @@
 package forgefed
 
 import (
+	"strings"
 	"testing"
 	"time"
 
@@ -23,12 +24,34 @@ func Test_FederationHostValidation(t *testing.T) {
 	}
 
 	sut = FederationHost{
+		HostFqdn: "",
+		NodeInfo: NodeInfo{
+			SoftwareName: "forgejo",
+		},
+		LatestActivity: time.Now(),
+	}
+	if res, _ := validation.IsValid(sut); res {
+		t.Errorf("sut should be invalid: HostFqdn empty")
+	}
+
+	sut = FederationHost{
+		HostFqdn: strings.Repeat("fill", 64),
+		NodeInfo: NodeInfo{
+			SoftwareName: "forgejo",
+		},
+		LatestActivity: time.Now(),
+	}
+	if res, _ := validation.IsValid(sut); res {
+		t.Errorf("sut should be invalid: HostFqdn too long (len=256)")
+	}
+
+	sut = FederationHost{
 		HostFqdn:       "host.do.main",
 		NodeInfo:       NodeInfo{},
 		LatestActivity: time.Now(),
 	}
 	if res, _ := validation.IsValid(sut); res {
-		t.Errorf("sut should be invalid")
+		t.Errorf("sut should be invalid: NodeInfo invalid")
 	}
 
 	sut = FederationHost{
diff --git a/models/forgefed/nodeinfo.go b/models/forgefed/nodeinfo.go
index bb56570635..66d2eca7aa 100644
--- a/models/forgefed/nodeinfo.go
+++ b/models/forgefed/nodeinfo.go
@@ -33,7 +33,7 @@ type NodeInfoWellKnown struct {
 	Href string
 }
 
-// Factory function for PersonID. Created struct is asserted to be valid
+// Factory function for NodeInfoWellKnown. Created struct is asserted to be valid.
 func NewNodeInfoWellKnown(body []byte) (NodeInfoWellKnown, error) {
 	result, err := NodeInfoWellKnownUnmarshalJSON(body)
 	if err != nil {
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)
 	}
 }