enhance test & fix reviews

.deadcode-out

@@ -131,6 +131,7 @@ package "code.gitea.io/gitea/models/user"
 	func (ErrUserInactive).Unwrap
 	func IsErrExternalLoginUserAlreadyExist
 	func IsErrExternalLoginUserNotExist
+	func NewFederatedUser
 	func IsErrUserSettingIsNotExist
 	func GetUserAllSettings
 	func DeleteUserSetting
@@ -319,6 +320,9 @@ package "code.gitea.io/gitea/modules/translation"
 package "code.gitea.io/gitea/modules/util/filebuffer"
 	func CreateFromReader
 
+package "code.gitea.io/gitea/modules/validation"
+	func IsErrNotValid
+
 package "code.gitea.io/gitea/modules/web"
 	func RouteMock
 	func RouteMockReset

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

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

models/forgefed/federationhost_test.go

@@ -4,6 +4,7 @@
 package forgefed
 
 import (
+	"strings"
 	"testing"
 	"time"
 
@@ -22,13 +23,35 @@ func Test_FederationHostValidation(t *testing.T) {
 		t.Errorf("sut should be valid but was %q", err)
 	}
 
+	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{

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 {

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)
 	}
 }

models/forgejo_migrations/migrate.go

@@ -66,6 +66,8 @@ var migrations = []*Migration{
 	NewMigration("Add `hide_archive_links` column to `release` table", AddHideArchiveLinksToRelease),
 	// v14 -> v15
 	NewMigration("Remove Gitea-specific columns from the repository and badge tables", RemoveGiteaSpecificColumnsFromRepositoryAndBadge),
+	// v15 -> v16
+	NewMigration("Create the `federation_host` table", CreateFederationHostTable),
 }
 
 // GetCurrentDBVersion returns the current Forgejo database version.

models/forgejo_migrations/v15.go

@@ -0,0 +1,33 @@
+// Copyright 2024 The Forgejo Authors. All rights reserved.
+// SPDX-License-Identifier: MIT
+
+package forgejo_migrations //nolint:revive
+
+import (
+	"time"
+
+	"code.gitea.io/gitea/modules/timeutil"
+
+	"xorm.io/xorm"
+)
+
+type (
+	SoftwareNameType string
+)
+
+type NodeInfo struct {
+	SoftwareName SoftwareNameType
+}
+
+type FederationHost struct {
+	ID             int64              `xorm:"pk autoincr"`
+	HostFqdn       string             `xorm:"host_fqdn UNIQUE INDEX VARCHAR(255) NOT NULL"`
+	NodeInfo       NodeInfo           `xorm:"extends NOT NULL"`
+	LatestActivity time.Time          `xorm:"NOT NULL"`
+	Created        timeutil.TimeStamp `xorm:"created"`
+	Updated        timeutil.TimeStamp `xorm:"updated"`
+}
+
+func CreateFederationHostTable(x *xorm.Engine) error {
+	return x.Sync(new(FederationHost))
+}

modules/activitypub/client.go

@@ -129,7 +129,7 @@ func (c *Client) Post(b []byte, to string) (resp *http.Response, err error) {
 }
 
 // Create an http GET request with forgejo/gitea specific headers
-func (c *Client) Get(to string) (resp *http.Response, err error) { // ToDo: we might not need the b parameter
+func (c *Client) Get(to string) (resp *http.Response, err error) {
 	var req *http.Request
 	emptyBody := []byte{0}
 	if req, err = c.NewRequest(http.MethodGet, emptyBody, to); err != nil {

modules/forgefed/actor.go

@@ -32,8 +32,8 @@ func NewActorID(uri string) (ActorID, error) {
 		return ActorID{}, err
 	}
 
-	if valid, outcome := validation.IsValid(result); !valid {
+	if valid, err := validation.IsValid(result); !valid {
-		return ActorID{}, outcome
+		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 {
+	if valid, err := validation.IsValid(personID); !valid {
-		return PersonID{}, outcome
+		return PersonID{}, err
 	}
 
 	return personID, nil

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)
 	}
 

tests/integration/api_activitypub_repository_test.go

@@ -91,6 +91,22 @@ func TestActivityPubRepositoryInboxValid(t *testing.T) {
 				`"openRegistrations":true,"usage":{"users":{"total":14,"activeHalfyear":2}},"metadata":{}}`)
 			fmt.Fprint(res, responseBody)
 		})
+	federatedRoutes.HandleFunc("/api/v1/activitypub/user-id/2",
+		func(res http.ResponseWriter, req *http.Request) {
+			// curl -H "Accept: application/json" https://federated-repo.prod.meissa.de/api/v1/activitypub/user-id/2
+			responseBody := fmt.Sprintf(`{"@context":["https://www.w3.org/ns/activitystreams","https://w3id.org/security/v1"],` +
+				`"id":"https://federated-repo.prod.meissa.de/api/v1/activitypub/user-id/2","type":"Person",` +
+				`"icon":{"type":"Image","mediaType":"image/png","url":"https://federated-repo.prod.meissa.de/avatars/1bb05d9a5f6675ed0272af9ea193063c"},` +
+				`"url":"https://federated-repo.prod.meissa.de/stargoose1","inbox":"https://federated-repo.prod.meissa.de/api/v1/activitypub/user-id/2/inbox",` +
+				`"outbox":"https://federated-repo.prod.meissa.de/api/v1/activitypub/user-id/2/outbox","preferredUsername":"stargoose1",` +
+				`"publicKey":{"id":"https://federated-repo.prod.meissa.de/api/v1/activitypub/user-id/2#main-key","owner":"https://federated-repo.prod.meissa.de/api/v1/activitypub/user-id/2",` +
+				`"publicKeyPem":"-----BEGIN PUBLIC KEY-----\nMIIBojANBgkqhkiG9w0BAQEFAAOCAY8AMIIBigKCAYEA18H5s7N6ItZUAh9tneII\nIuZdTTa3cZlLa/9ejWAHTkcp3WLW+/zbsumlMrWYfBy2/yTm56qasWt38iY4D6ul\n` +
+				`CPiwhAqX3REvVq8tM79a2CEqZn9ka6vuXoDgBg/sBf/BUWqf7orkjUXwk/U0Egjf\nk5jcurF4vqf1u+rlAHH37dvSBaDjNj6Qnj4OP12bjfaY/yvs7+jue/eNXFHjzN4E\n` +
+				`T2H4B/yeKTJ4UuAwTlLaNbZJul2baLlHelJPAsxiYaziVuV5P+IGWckY6RSerRaZ\nAkc4mmGGtjAyfN9aewe+lNVfwS7ElFx546PlLgdQgjmeSwLX8FWxbPE5A/PmaXCs\n` +
+				`nx+nou+3dD7NluULLtdd7K+2x02trObKXCAzmi5/Dc+yKTzpFqEz+hLNCz7TImP/\ncK//NV9Q+X67J9O27baH9R9ZF4zMw8rv2Pg0WLSw1z7lLXwlgIsDapeMCsrxkVO4\n` +
+				`LXX5AQ1xQNtlssnVoUBqBrvZsX2jUUKUocvZqMGuE4hfAgMBAAE=\n-----END PUBLIC KEY-----\n"}}`)
+			fmt.Fprint(res, responseBody)
+		})
 	federatedRoutes.HandleFunc("/",
 		func(res http.ResponseWriter, req *http.Request) {
 			t.Errorf("Unhandled request: %q", req.URL.EscapedPath())
@@ -126,9 +142,7 @@ func TestActivityPubRepositoryInboxValid(t *testing.T) {
 		assert.NoError(t, err)
 		assert.Equal(t, http.StatusNoContent, resp.StatusCode)
 
-		federationHost := unittest.AssertExistsAndLoadBean(t, &forgefed.FederationHost{ID: 1})
+		unittest.AssertExistsAndLoadBean(t, &forgefed.FederationHost{HostFqdn: "127.0.0.1"})
-		assert.Equal(t, "127.0.0.1", federationHost.HostFqdn)
-
 	})
 }