From 259c0202c71714c8564b257fc7e36af59df74119 Mon Sep 17 00:00:00 2001 From: Michael Jerger Date: Wed, 27 Mar 2024 19:56:32 +0100 Subject: [PATCH] fix test & add some review --- models/forgefed/activity.go | 4 ++-- models/forgefed/activity_test.go | 8 +++++--- models/repo/star.go | 7 +++++-- 3 files changed, 12 insertions(+), 7 deletions(-) diff --git a/models/forgefed/activity.go b/models/forgefed/activity.go index 19075a76b3..44214973e6 100644 --- a/models/forgefed/activity.go +++ b/models/forgefed/activity.go @@ -18,13 +18,13 @@ type ForgeLike struct { ap.Activity } -func NewForgeLike(actorIRI string, objectIRI string) (ForgeLike, error) { +func NewForgeLike(actorIRI string, objectIRI string, startTime time.Time) (ForgeLike, error) { result := ForgeLike{} result.Type = ap.LikeType // ToDo: Would validating the source by Actor.Type field make sense? result.Actor = ap.IRI(actorIRI) // Thats us, a User result.Object = ap.IRI(objectIRI) // Thats them, a Repository - result.StartTime = time.Now() + result.StartTime = startTime if valid, err := validation.IsValid(result); !valid { return ForgeLike{}, err } diff --git a/models/forgefed/activity_test.go b/models/forgefed/activity_test.go index 6fb455db11..823470ea7e 100644 --- a/models/forgefed/activity_test.go +++ b/models/forgefed/activity_test.go @@ -1,4 +1,4 @@ -// Copyright 2023 The Forgejo Authors. All rights reserved. +// Copyright 2023, 2024 The Forgejo Authors. All rights reserved. // SPDX-License-Identifier: MIT package forgefed @@ -6,6 +6,7 @@ package forgefed import ( "reflect" "testing" + "time" "code.gitea.io/gitea/modules/validation" @@ -16,9 +17,10 @@ import ( func Test_NewForgeLike(t *testing.T) { actorIRI := "https://repo.prod.meissa.de/api/v1/activitypub/user-id/1" objectIRI := "https://codeberg.org/api/v1/activitypub/repository-id/1" - want := []byte(`{"type":"Like","actor":"https://repo.prod.meissa.de/api/v1/activitypub/user-id/1","object":"https://codeberg.org/api/v1/activitypub/repository-id/1"}`) + want := []byte(`{"type":"Like","startTime":"2024-03-27T00:00:00Z","actor":"https://repo.prod.meissa.de/api/v1/activitypub/user-id/1","object":"https://codeberg.org/api/v1/activitypub/repository-id/1"}`) - sut, err := NewForgeLike(actorIRI, objectIRI) + startTime, _ := time.Parse("2006-Jan-02", "2024-Mar-27") + sut, err := NewForgeLike(actorIRI, objectIRI, startTime) if err != nil { t.Errorf("unexpected error: %v\n", err) } diff --git a/models/repo/star.go b/models/repo/star.go index 4f3cd05631..6e4a25ae8d 100644 --- a/models/repo/star.go +++ b/models/repo/star.go @@ -5,6 +5,7 @@ package repo import ( "context" + "time" "code.gitea.io/gitea/models/db" "code.gitea.io/gitea/models/forgefed" @@ -86,7 +87,7 @@ func starLocalRepo(ctx context.Context, userID, repoID int64, star bool) error { // ToDo: Move to federation service or simillar func sendLikeActivities(ctx context.Context, userID int64, repoID int64) error { - // TODO: is user loading necessary here? + // TODO: is user loading necessary here? - imho no! log.Info("User ID: %v, Repo ID: %v", userID, repoID) user, err := user_model.GetUserByID(ctx, userID) log.Info("User is: %v", user) @@ -100,6 +101,7 @@ func sendLikeActivities(ctx context.Context, userID int64, repoID int64) error { return err } + // TODO: That's wrong - we've to send the activities to repo not to user! apclient, err := activitypub.NewClient(ctx, user, user.APAPIURL()) if err != nil { return err @@ -108,7 +110,7 @@ func sendLikeActivities(ctx context.Context, userID int64, repoID int64) error { for _, federatedRepo := range federatedRepos { target := federatedRepo.Uri + "/inbox/" // A like goes to the inbox of the federated repo log.Info("Federated Repo URI is: %v", target) - likeActivity, err := forgefed.NewForgeLike(user.APAPIURL(), target) + likeActivity, err := forgefed.NewForgeLike(user.APAPIURL(), target, time.Now()) if err != nil { return err } @@ -118,6 +120,7 @@ func sendLikeActivities(ctx context.Context, userID int64, repoID int64) error { return err } + // TODO: decouple loading & creating activities from sending them - use two loops. // TODO: set timeouts for outgoing request in oder to mitigate DOS by slow lories // TODO: Check if we need to respect rate limits // ToDo: Change this to the standalone table of FederatedRepos