From c100b8e1e015daeb02ad83b64eabf848c9de8337 Mon Sep 17 00:00:00 2001 From: Anthony Wang Date: Sun, 17 Jul 2022 11:19:48 -0500 Subject: [PATCH] Apply suggestions from code review --- models/forgefed/branch.go | 2 +- models/forgefed/commit.go | 2 +- models/forgefed/forgefed.go | 11 ++++++----- models/forgefed/push.go | 2 +- models/forgefed/repository.go | 2 +- models/forgefed/ticket.go | 2 +- modules/activitypub/comment.go | 1 + modules/activitypub/transport.go | 2 +- routers/api/v1/activitypub/person.go | 3 ++- routers/api/v1/activitypub/repo.go | 2 +- 10 files changed, 16 insertions(+), 13 deletions(-) diff --git a/models/forgefed/branch.go b/models/forgefed/branch.go index d75d309d4e..d6606c07d6 100644 --- a/models/forgefed/branch.go +++ b/models/forgefed/branch.go @@ -29,7 +29,7 @@ func BranchNew() *Branch { func (br Branch) MarshalJSON() ([]byte, error) { b, err := br.Object.MarshalJSON() if len(b) == 0 || err != nil { - return make([]byte, 0), err + return nil, err } b = b[:len(b)-1] diff --git a/models/forgefed/commit.go b/models/forgefed/commit.go index 70b5e2621a..05b2d48ec4 100644 --- a/models/forgefed/commit.go +++ b/models/forgefed/commit.go @@ -33,7 +33,7 @@ func CommitNew() *Commit { func (c Commit) MarshalJSON() ([]byte, error) { b, err := c.Object.MarshalJSON() if len(b) == 0 || err != nil { - return make([]byte, 0), err + return nil, err } b = b[:len(b)-1] diff --git a/models/forgefed/forgefed.go b/models/forgefed/forgefed.go index fcc9b53fac..39872fa74d 100644 --- a/models/forgefed/forgefed.go +++ b/models/forgefed/forgefed.go @@ -13,15 +13,16 @@ const ForgeFedNamespaceURI = "https://forgefed.org/ns" // GetItemByType instantiates a new ForgeFed object if the type matches // otherwise it defaults to existing activitypub package typer function. func GetItemByType(typ ap.ActivityVocabularyType) (ap.Item, error) { - if typ == CommitType { + switch typ { + case CommitType: return CommitNew(), nil - } else if typ == BranchType { + case BranchType: return BranchNew(), nil - } else if typ == RepositoryType { + case RepositoryType: return RepositoryNew(""), nil - } else if typ == PushType { + case PushType: return PushNew(), nil - } else if typ == TicketType { + case TicketType: return TicketNew(), nil } return ap.GetItemByType(typ) diff --git a/models/forgefed/push.go b/models/forgefed/push.go index edcdf837e3..82ef45c0d4 100644 --- a/models/forgefed/push.go +++ b/models/forgefed/push.go @@ -33,7 +33,7 @@ func PushNew() *Push { func (p Push) MarshalJSON() ([]byte, error) { b, err := p.Object.MarshalJSON() if len(b) == 0 || err != nil { - return make([]byte, 0), err + return nil, err } b = b[:len(b)-1] diff --git a/models/forgefed/repository.go b/models/forgefed/repository.go index b127dea7c1..f8450d6d8f 100644 --- a/models/forgefed/repository.go +++ b/models/forgefed/repository.go @@ -34,7 +34,7 @@ func RepositoryNew(id ap.ID) *Repository { func (r Repository) MarshalJSON() ([]byte, error) { b, err := r.Actor.MarshalJSON() if len(b) == 0 || err != nil { - return make([]byte, 0), err + return nil, err } b = b[:len(b)-1] diff --git a/models/forgefed/ticket.go b/models/forgefed/ticket.go index 322cf87b5a..3281f9765b 100644 --- a/models/forgefed/ticket.go +++ b/models/forgefed/ticket.go @@ -43,7 +43,7 @@ func TicketNew() *Ticket { func (t Ticket) MarshalJSON() ([]byte, error) { b, err := t.Object.MarshalJSON() if len(b) == 0 || err != nil { - return make([]byte, 0), err + return nil, err } b = b[:len(b)-1] diff --git a/modules/activitypub/comment.go b/modules/activitypub/comment.go index b210de2525..d241ada7f9 100644 --- a/modules/activitypub/comment.go +++ b/modules/activitypub/comment.go @@ -21,6 +21,7 @@ func Comment(ctx context.Context, note ap.Note) { actorUser, err := personIRIToUser(ctx, note.AttributedTo.GetLink()) if err != nil { log.Warn("Couldn't find actor", err) + return } // TODO: Move IRI processing stuff to iri.go diff --git a/modules/activitypub/transport.go b/modules/activitypub/transport.go index 21d2fcf5de..f0aa12f1f3 100644 --- a/modules/activitypub/transport.go +++ b/modules/activitypub/transport.go @@ -55,6 +55,6 @@ func Send(user *user_model.User, activity *ap.Activity) { client, _ := NewClient(user, setting.AppURL+"api/v1/activitypub/user/"+user.Name+"#main-key") resp, _ := client.Post(binary, to.GetID().String()) respBody, _ := io.ReadAll(io.LimitReader(resp.Body, setting.Federation.MaxSize)) - log.Debug(string(respBody)) + log.Trace("Response from sending activity", string(respBody)) } } diff --git a/routers/api/v1/activitypub/person.go b/routers/api/v1/activitypub/person.go index c6ca3d5399..be8bdc1a44 100644 --- a/routers/api/v1/activitypub/person.go +++ b/routers/api/v1/activitypub/person.go @@ -102,6 +102,7 @@ func PersonInbox(ctx *context.APIContext) { body, err := io.ReadAll(io.LimitReader(ctx.Req.Body, setting.Federation.MaxSize)) if err != nil { ctx.ServerError("Error reading request body", err) + return } var activity ap.Activity @@ -112,7 +113,7 @@ func PersonInbox(ctx *context.APIContext) { case ap.UndoType: activitypub.Unfollow(ctx, activity) default: - log.Debug("ActivityStreams type not supported", activity) + log.Info("Incoming unsupported ActivityStreams type: %s", activity.GetType()) ctx.PlainText(http.StatusNotImplemented, "ActivityStreams type not supported") return } diff --git a/routers/api/v1/activitypub/repo.go b/routers/api/v1/activitypub/repo.go index 222b7188c3..668c358928 100644 --- a/routers/api/v1/activitypub/repo.go +++ b/routers/api/v1/activitypub/repo.go @@ -140,7 +140,7 @@ func RepoInbox(ctx *context.APIContext) { activitypub.Comment(ctx, note) } default: - log.Warn("ActivityStreams type not supported", activity) + log.Info("Incoming unsupported ActivityStreams type: %s", activity["type"]) ctx.PlainText(http.StatusNotImplemented, "ActivityStreams type not supported") return }