From d98b6318ace5f8a00a6d1776be2e78782f7eb429 Mon Sep 17 00:00:00 2001 From: tobi <31960611+tsmethurst@users.noreply.github.com> Date: Tue, 27 Jun 2023 11:37:42 +0200 Subject: [PATCH] [bugfix] Use gtserror package for WrongType errs (#1930) * [bugfix] Use gtserror package for WrongType errs * test --- internal/ap/{extract_test.go => ap_test.go} | 33 ++++++++++++- internal/ap/error.go | 35 -------------- internal/ap/extractattachments_test.go | 2 +- internal/ap/extractcontent_test.go | 2 +- internal/ap/extractmentions_test.go | 2 +- internal/ap/extractsensitive_test.go | 2 +- internal/ap/extractvisibility_test.go | 2 +- internal/ap/normalize_test.go | 34 +------------- internal/ap/resolve.go | 18 +++---- internal/ap/resolve_test.go | 52 +++++++++++++++++++++ internal/gtserror/error.go | 19 ++++++-- 11 files changed, 112 insertions(+), 89 deletions(-) rename internal/ap/{extract_test.go => ap_test.go} (91%) delete mode 100644 internal/ap/error.go create mode 100644 internal/ap/resolve_test.go diff --git a/internal/ap/extract_test.go b/internal/ap/ap_test.go similarity index 91% rename from internal/ap/extract_test.go rename to internal/ap/ap_test.go index deb5ec041..105bc1fcf 100644 --- a/internal/ap/extract_test.go +++ b/internal/ap/ap_test.go @@ -184,7 +184,7 @@ func addressable5() ap.Addressable { return note } -type ExtractTestSuite struct { +type APTestSuite struct { suite.Suite document1 vocab.ActivityStreamsDocument attachment1 vocab.ActivityStreamsAttachmentProperty @@ -196,7 +196,36 @@ type ExtractTestSuite struct { addressable5 ap.Addressable } -func (suite *ExtractTestSuite) SetupTest() { +func (suite *APTestSuite) jsonToType(rawJson string) (vocab.Type, map[string]interface{}) { + var raw map[string]interface{} + err := json.Unmarshal([]byte(rawJson), &raw) + if err != nil { + panic(err) + } + + t, err := streams.ToType(context.Background(), raw) + if err != nil { + panic(err) + } + + return t, raw +} + +func (suite *APTestSuite) typeToJson(t vocab.Type) string { + m, err := ap.Serialize(t) + if err != nil { + suite.FailNow(err.Error()) + } + + b, err := json.MarshalIndent(m, "", " ") + if err != nil { + suite.FailNow(err.Error()) + } + + return string(b) +} + +func (suite *APTestSuite) SetupTest() { suite.document1 = document1() suite.attachment1 = attachment1() suite.noteWithMentions1 = noteWithMentions1() diff --git a/internal/ap/error.go b/internal/ap/error.go deleted file mode 100644 index ef27d5ac7..000000000 --- a/internal/ap/error.go +++ /dev/null @@ -1,35 +0,0 @@ -// GoToSocial -// Copyright (C) GoToSocial Authors admin@gotosocial.org -// SPDX-License-Identifier: AGPL-3.0-or-later -// -// This program is free software: you can redistribute it and/or modify -// it under the terms of the GNU Affero General Public License as published by -// the Free Software Foundation, either version 3 of the License, or -// (at your option) any later version. -// -// This program is distributed in the hope that it will be useful, -// but WITHOUT ANY WARRANTY; without even the implied warranty of -// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -// GNU Affero General Public License for more details. -// -// You should have received a copy of the GNU Affero General Public License -// along with this program. If not, see . - -package ap - -import "fmt" - -// ErrWrongType indicates that we tried to resolve a type into -// an interface that it's not compatible with, eg a Person into -// a Statusable. -type ErrWrongType struct { - wrapped error -} - -func (err *ErrWrongType) Error() string { - return fmt.Sprintf("wrong received type: %v", err.wrapped) -} - -func newErrWrongType(err error) error { - return &ErrWrongType{wrapped: err} -} diff --git a/internal/ap/extractattachments_test.go b/internal/ap/extractattachments_test.go index e3a40c7bb..3d5fc7e89 100644 --- a/internal/ap/extractattachments_test.go +++ b/internal/ap/extractattachments_test.go @@ -26,7 +26,7 @@ import ( ) type ExtractAttachmentsTestSuite struct { - ExtractTestSuite + APTestSuite } func (suite *ExtractAttachmentsTestSuite) TestExtractAttachmentMissingURL() { diff --git a/internal/ap/extractcontent_test.go b/internal/ap/extractcontent_test.go index d4b4808ab..590d1b931 100644 --- a/internal/ap/extractcontent_test.go +++ b/internal/ap/extractcontent_test.go @@ -25,7 +25,7 @@ import ( ) type ExtractContentTestSuite struct { - ExtractTestSuite + APTestSuite } func (suite *ExtractContentTestSuite) TestExtractContent1() { diff --git a/internal/ap/extractmentions_test.go b/internal/ap/extractmentions_test.go index 2f1929c9a..fbfee34f5 100644 --- a/internal/ap/extractmentions_test.go +++ b/internal/ap/extractmentions_test.go @@ -25,7 +25,7 @@ import ( ) type ExtractMentionsTestSuite struct { - ExtractTestSuite + APTestSuite } func (suite *ExtractMentionsTestSuite) TestExtractMentions() { diff --git a/internal/ap/extractsensitive_test.go b/internal/ap/extractsensitive_test.go index f486877f8..ce5571680 100644 --- a/internal/ap/extractsensitive_test.go +++ b/internal/ap/extractsensitive_test.go @@ -25,7 +25,7 @@ import ( ) type ExtractSensitiveTestSuite struct { - ExtractTestSuite + APTestSuite } func (suite *ExtractMentionsTestSuite) TestExtractSensitive() { diff --git a/internal/ap/extractvisibility_test.go b/internal/ap/extractvisibility_test.go index 24fb50ec3..3c894f4a5 100644 --- a/internal/ap/extractvisibility_test.go +++ b/internal/ap/extractvisibility_test.go @@ -26,7 +26,7 @@ import ( ) type ExtractVisibilityTestSuite struct { - ExtractTestSuite + APTestSuite } func (suite *ExtractVisibilityTestSuite) TestExtractVisibilityPublic() { diff --git a/internal/ap/normalize_test.go b/internal/ap/normalize_test.go index 2c9a1907a..cde807f21 100644 --- a/internal/ap/normalize_test.go +++ b/internal/ap/normalize_test.go @@ -18,48 +18,16 @@ package ap_test import ( - "context" - "encoding/json" "testing" "github.com/stretchr/testify/suite" - "github.com/superseriousbusiness/activity/streams" "github.com/superseriousbusiness/activity/streams/vocab" "github.com/superseriousbusiness/gotosocial/internal/ap" "github.com/superseriousbusiness/gotosocial/testrig" ) type NormalizeTestSuite struct { - suite.Suite -} - -func (suite *NormalizeTestSuite) jsonToType(rawJson string) (vocab.Type, map[string]interface{}) { - var raw map[string]interface{} - err := json.Unmarshal([]byte(rawJson), &raw) - if err != nil { - panic(err) - } - - t, err := streams.ToType(context.Background(), raw) - if err != nil { - panic(err) - } - - return t, raw -} - -func (suite *NormalizeTestSuite) typeToJson(t vocab.Type) string { - m, err := ap.Serialize(t) - if err != nil { - suite.FailNow(err.Error()) - } - - b, err := json.MarshalIndent(m, "", " ") - if err != nil { - suite.FailNow(err.Error()) - } - - return string(b) + APTestSuite } func (suite *NormalizeTestSuite) getStatusable() (vocab.ActivityStreamsNote, map[string]interface{}) { diff --git a/internal/ap/resolve.go b/internal/ap/resolve.go index ef4d0b50f..a9955be3f 100644 --- a/internal/ap/resolve.go +++ b/internal/ap/resolve.go @@ -20,10 +20,10 @@ package ap import ( "context" "encoding/json" - "fmt" "github.com/superseriousbusiness/activity/streams" "github.com/superseriousbusiness/activity/streams/vocab" + "github.com/superseriousbusiness/gotosocial/internal/gtserror" ) // ResolveStatusable tries to resolve the given bytes into an ActivityPub Statusable representation. @@ -33,12 +33,12 @@ import ( func ResolveStatusable(ctx context.Context, b []byte) (Statusable, error) { rawStatusable := make(map[string]interface{}) if err := json.Unmarshal(b, &rawStatusable); err != nil { - return nil, fmt.Errorf("ResolveStatusable: error unmarshalling bytes into json: %w", err) + return nil, gtserror.Newf("error unmarshalling bytes into json: %w", err) } t, err := streams.ToType(ctx, rawStatusable) if err != nil { - return nil, fmt.Errorf("ResolveStatusable: error resolving json into ap vocab type: %w", err) + return nil, gtserror.Newf("error resolving json into ap vocab type: %w", err) } var ( @@ -68,8 +68,8 @@ func ResolveStatusable(ctx context.Context, b []byte) (Statusable, error) { } if !ok { - err = fmt.Errorf("ResolveStatusable: could not resolve %T to Statusable", t) - return nil, newErrWrongType(err) + err = gtserror.Newf("could not resolve %T to Statusable", t) + return nil, gtserror.SetWrongType(err) } NormalizeIncomingContent(statusable, rawStatusable) @@ -87,12 +87,12 @@ func ResolveStatusable(ctx context.Context, b []byte) (Statusable, error) { func ResolveAccountable(ctx context.Context, b []byte) (Accountable, error) { rawAccountable := make(map[string]interface{}) if err := json.Unmarshal(b, &rawAccountable); err != nil { - return nil, fmt.Errorf("ResolveAccountable: error unmarshalling bytes into json: %w", err) + return nil, gtserror.Newf("error unmarshalling bytes into json: %w", err) } t, err := streams.ToType(ctx, rawAccountable) if err != nil { - return nil, fmt.Errorf("ResolveAccountable: error resolving json into ap vocab type: %w", err) + return nil, gtserror.Newf("error resolving json into ap vocab type: %w", err) } var ( @@ -114,8 +114,8 @@ func ResolveAccountable(ctx context.Context, b []byte) (Accountable, error) { } if !ok { - err = fmt.Errorf("ResolveAccountable: could not resolve %T to Accountable", t) - return nil, newErrWrongType(err) + err = gtserror.Newf("could not resolve %T to Accountable", t) + return nil, gtserror.SetWrongType(err) } NormalizeIncomingSummary(accountable, rawAccountable) diff --git a/internal/ap/resolve_test.go b/internal/ap/resolve_test.go new file mode 100644 index 000000000..efb56b1c4 --- /dev/null +++ b/internal/ap/resolve_test.go @@ -0,0 +1,52 @@ +// GoToSocial +// Copyright (C) GoToSocial Authors admin@gotosocial.org +// SPDX-License-Identifier: AGPL-3.0-or-later +// +// This program is free software: you can redistribute it and/or modify +// it under the terms of the GNU Affero General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU Affero General Public License for more details. +// +// You should have received a copy of the GNU Affero General Public License +// along with this program. If not, see . + +package ap_test + +import ( + "context" + "testing" + + "github.com/stretchr/testify/suite" + "github.com/superseriousbusiness/gotosocial/internal/ap" + "github.com/superseriousbusiness/gotosocial/internal/gtserror" +) + +type ResolveTestSuite struct { + APTestSuite +} + +func (suite *ResolveTestSuite) TestResolveDocumentAsStatusable() { + b := []byte(suite.typeToJson(suite.document1)) + + statusable, err := ap.ResolveStatusable(context.Background(), b) + suite.NoError(err) + suite.NotNil(statusable) +} + +func (suite *ResolveTestSuite) TestResolveDocumentAsAccountable() { + b := []byte(suite.typeToJson(suite.document1)) + + accountable, err := ap.ResolveAccountable(context.Background(), b) + suite.True(gtserror.WrongType(err)) + suite.EqualError(err, "ResolveAccountable: could not resolve *typedocument.ActivityStreamsDocument to Accountable") + suite.Nil(accountable) +} + +func TestResolveTestSuite(t *testing.T) { + suite.Run(t, &ResolveTestSuite{}) +} diff --git a/internal/gtserror/error.go b/internal/gtserror/error.go index 6eaa3db63..85dc0d54c 100644 --- a/internal/gtserror/error.go +++ b/internal/gtserror/error.go @@ -40,26 +40,35 @@ const ( TypeSMTP ErrorType = "smtp" // smtp (mail) ) -// Unretrievable ... +// Unretrievable checks error for a stored "unretrievable" flag. +// +// Unretrievable indicates that a call to retrieve a resource +// (account, status, etc) could not be fulfilled, either because +// it was not found locally, or because some prerequisite remote +// resource call failed, making it impossible to return the item. func Unretrievable(err error) bool { _, ok := errors.Value(err, unrtrvableKey).(struct{}) return ok } -// SetUnretrievable ... +// SetUnretrievable will wrap the given error to store an "unretrievable" +// flag, returning wrapped error. See "Unretrievable" for example use-cases. func SetUnretrievable(err error) error { return errors.WithValue(err, unrtrvableKey, struct{}{}) } -// WrongType ... +// WrongType checks error for a stored "wrong type" flag. Wrong type +// indicates that an ActivityPub URI returned a type we weren't expecting: +// Statusable instead of Accountable, or vice versa, for example. func WrongType(err error) bool { _, ok := errors.Value(err, wrongTypeKey).(struct{}) return ok } -// SetWrongType ... +// SetWrongType will wrap the given error to store a "wrong type" flag, +// returning wrapped error. See "WrongType" for example use-cases. func SetWrongType(err error) error { - return errors.WithValue(err, unrtrvableKey, struct{}{}) + return errors.WithValue(err, wrongTypeKey, struct{}{}) } // StatusCode checks error for a stored status code value. For example