From e681aac5899b4a93d7c71e9a334a67c3f2b00e93 Mon Sep 17 00:00:00 2001 From: tobi <31960611+tsmethurst@users.noreply.github.com> Date: Fri, 10 Sep 2021 14:36:10 +0200 Subject: [PATCH] fixes + db changes (#204) * fixes + db changes * make duration more lenient --- internal/api/client/account/account_test.go | 56 ++++++++++- internal/api/client/account/accountupdate.go | 3 +- .../api/client/account/accountupdate_test.go | 87 +++++++---------- .../api/client/account/accountverify_test.go | 74 ++++++++++++++ internal/cliactions/admin/account/account.go | 10 +- internal/db/basic.go | 9 +- internal/db/bundb/basic.go | 4 +- internal/db/bundb/basic_test.go | 34 +++++++ internal/federation/dereferencing/status.go | 4 +- internal/processing/account/account_test.go | 97 +++++++++++++++++++ internal/processing/account/update.go | 16 +-- internal/processing/account/update_test.go | 75 ++++++++++++++ .../processing/admin/createdomainblock.go | 2 +- .../processing/admin/deletedomainblock.go | 2 +- internal/processing/instance.go | 2 +- internal/processing/media/update.go | 4 +- 16 files changed, 401 insertions(+), 78 deletions(-) create mode 100644 internal/processing/account/account_test.go create mode 100644 internal/processing/account/update_test.go diff --git a/internal/api/client/account/account_test.go b/internal/api/client/account/account_test.go index 02817984b..56f981ab1 100644 --- a/internal/api/client/account/account_test.go +++ b/internal/api/client/account/account_test.go @@ -1,6 +1,12 @@ package account_test import ( + "bytes" + "fmt" + "net/http" + "net/http/httptest" + + "github.com/gin-gonic/gin" "github.com/sirupsen/logrus" "github.com/stretchr/testify/suite" "github.com/superseriousbusiness/gotosocial/internal/api/client/account" @@ -9,11 +15,12 @@ import ( "github.com/superseriousbusiness/gotosocial/internal/db" "github.com/superseriousbusiness/gotosocial/internal/federation" "github.com/superseriousbusiness/gotosocial/internal/gtsmodel" + "github.com/superseriousbusiness/gotosocial/internal/oauth" "github.com/superseriousbusiness/gotosocial/internal/processing" "github.com/superseriousbusiness/gotosocial/internal/typeutils" + "github.com/superseriousbusiness/gotosocial/testrig" ) -// nolint type AccountStandardTestSuite struct { // standard suite interfaces suite.Suite @@ -37,3 +44,50 @@ type AccountStandardTestSuite struct { // module being tested accountModule *account.Module } + +func (suite *AccountStandardTestSuite) SetupSuite() { + suite.testTokens = testrig.NewTestTokens() + suite.testClients = testrig.NewTestClients() + suite.testApplications = testrig.NewTestApplications() + suite.testUsers = testrig.NewTestUsers() + suite.testAccounts = testrig.NewTestAccounts() + suite.testAttachments = testrig.NewTestAttachments() + suite.testStatuses = testrig.NewTestStatuses() +} + +func (suite *AccountStandardTestSuite) SetupTest() { + suite.config = testrig.NewTestConfig() + suite.db = testrig.NewTestDB() + suite.storage = testrig.NewTestStorage() + suite.log = testrig.NewTestLog() + suite.federator = testrig.NewTestFederator(suite.db, testrig.NewTestTransportController(testrig.NewMockHTTPClient(nil), suite.db), suite.storage) + suite.processor = testrig.NewTestProcessor(suite.db, suite.storage, suite.federator) + suite.accountModule = account.New(suite.config, suite.processor, suite.log).(*account.Module) + testrig.StandardDBSetup(suite.db, nil) + testrig.StandardStorageSetup(suite.storage, "../../../../testrig/media") +} + +func (suite *AccountStandardTestSuite) TearDownTest() { + testrig.StandardDBTeardown(suite.db) + testrig.StandardStorageTeardown(suite.storage) +} + +func (suite *AccountStandardTestSuite) newContext(recorder *httptest.ResponseRecorder, requestMethod string, requestBody []byte, requestPath string, bodyContentType string) *gin.Context { + ctx, _ := gin.CreateTestContext(recorder) + + ctx.Set(oauth.SessionAuthorizedAccount, suite.testAccounts["local_account_1"]) + ctx.Set(oauth.SessionAuthorizedToken, oauth.DBTokenToToken(suite.testTokens["local_account_1"])) + ctx.Set(oauth.SessionAuthorizedApplication, suite.testApplications["application_1"]) + ctx.Set(oauth.SessionAuthorizedUser, suite.testUsers["local_account_1"]) + + baseURI := fmt.Sprintf("%s://%s", suite.config.Protocol, suite.config.Host) + requestURI := fmt.Sprintf("%s/%s", baseURI, requestPath) + + ctx.Request = httptest.NewRequest(http.MethodPatch, requestURI, bytes.NewReader(requestBody)) // the endpoint we're hitting + + if bodyContentType != "" { + ctx.Request.Header.Set("Content-Type", bodyContentType) + } + + return ctx +} diff --git a/internal/api/client/account/accountupdate.go b/internal/api/client/account/accountupdate.go index 282d172ed..c38ede252 100644 --- a/internal/api/client/account/accountupdate.go +++ b/internal/api/client/account/accountupdate.go @@ -107,7 +107,6 @@ func (m *Module) AccountUpdateCredentialsPATCHHandler(c *gin.Context) { } l.Tracef("retrieved account %+v", authed.Account.ID) - l.Debugf("parsing request form %s", c.Request.Form) form := &model.UpdateCredentialsRequest{} if err := c.ShouldBind(&form); err != nil || form == nil { l.Debugf("could not parse form from request: %s", err) @@ -115,6 +114,8 @@ func (m *Module) AccountUpdateCredentialsPATCHHandler(c *gin.Context) { return } + l.Debugf("parsed request form %+v", form) + // if everything on the form is nil, then nothing has been set and we shouldn't continue if form.Discoverable == nil && form.Bot == nil && form.DisplayName == nil && form.Note == nil && form.Avatar == nil && form.Header == nil && form.Locked == nil && form.Source == nil && form.FieldsAttributes == nil { l.Debugf("could not parse form from request") diff --git a/internal/api/client/account/accountupdate_test.go b/internal/api/client/account/accountupdate_test.go index 8fc31171b..a02573631 100644 --- a/internal/api/client/account/accountupdate_test.go +++ b/internal/api/client/account/accountupdate_test.go @@ -19,18 +19,17 @@ package account_test import ( - "bytes" + "encoding/json" "fmt" "io/ioutil" "net/http" "net/http/httptest" "testing" - "github.com/gin-gonic/gin" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/suite" "github.com/superseriousbusiness/gotosocial/internal/api/client/account" - "github.com/superseriousbusiness/gotosocial/internal/oauth" + apimodel "github.com/superseriousbusiness/gotosocial/internal/api/model" "github.com/superseriousbusiness/gotosocial/testrig" ) @@ -38,69 +37,57 @@ type AccountUpdateTestSuite struct { AccountStandardTestSuite } -func (suite *AccountUpdateTestSuite) SetupSuite() { - suite.testTokens = testrig.NewTestTokens() - suite.testClients = testrig.NewTestClients() - suite.testApplications = testrig.NewTestApplications() - suite.testUsers = testrig.NewTestUsers() - suite.testAccounts = testrig.NewTestAccounts() - suite.testAttachments = testrig.NewTestAttachments() - suite.testStatuses = testrig.NewTestStatuses() -} - -func (suite *AccountUpdateTestSuite) SetupTest() { - suite.config = testrig.NewTestConfig() - suite.db = testrig.NewTestDB() - suite.storage = testrig.NewTestStorage() - suite.log = testrig.NewTestLog() - suite.federator = testrig.NewTestFederator(suite.db, testrig.NewTestTransportController(testrig.NewMockHTTPClient(nil), suite.db), suite.storage) - suite.processor = testrig.NewTestProcessor(suite.db, suite.storage, suite.federator) - suite.accountModule = account.New(suite.config, suite.processor, suite.log).(*account.Module) - testrig.StandardDBSetup(suite.db, nil) - testrig.StandardStorageSetup(suite.storage, "../../../../testrig/media") -} - -func (suite *AccountUpdateTestSuite) TearDownTest() { - testrig.StandardDBTeardown(suite.db) - testrig.StandardStorageTeardown(suite.storage) -} - -func (suite *AccountUpdateTestSuite) TestAccountUpdateCredentialsPATCHHandler() { - - requestBody, w, err := testrig.CreateMultipartFormData("header", "../../../../testrig/media/test-jpeg.jpg", map[string]string{ - "display_name": "updated zork display name!!!", - "locked": "true", - }) +func (suite *AccountUpdateTestSuite) TestAccountUpdateCredentialsPATCHHandlerSimple() { + // set up the request + // we're updating the header image, the display name, and the locked status of zork + // we're removing the note/bio + requestBody, w, err := testrig.CreateMultipartFormData( + "header", "../../../../testrig/media/test-jpeg.jpg", + map[string]string{ + "display_name": "updated zork display name!!!", + "note": "", + "locked": "true", + }) if err != nil { panic(err) } - - // setup recorder := httptest.NewRecorder() - ctx, _ := gin.CreateTestContext(recorder) - ctx.Set(oauth.SessionAuthorizedAccount, suite.testAccounts["local_account_1"]) - ctx.Set(oauth.SessionAuthorizedToken, oauth.DBTokenToToken(suite.testTokens["local_account_1"])) - ctx.Set(oauth.SessionAuthorizedApplication, suite.testApplications["application_1"]) - ctx.Set(oauth.SessionAuthorizedUser, suite.testUsers["local_account_1"]) - ctx.Request = httptest.NewRequest(http.MethodPatch, fmt.Sprintf("http://localhost:8080/%s", account.UpdateCredentialsPath), bytes.NewReader(requestBody.Bytes())) // the endpoint we're hitting - ctx.Request.Header.Set("Content-Type", w.FormDataContentType()) + ctx := suite.newContext(recorder, http.MethodPatch, requestBody.Bytes(), account.UpdateCredentialsPath, w.FormDataContentType()) + + // call the handler suite.accountModule.AccountUpdateCredentialsPATCHHandler(ctx) - // check response - // 1. we should have OK because our request was valid - suite.EqualValues(http.StatusOK, recorder.Code) + suite.Equal(http.StatusOK, recorder.Code) // 2. we should have no error message in the result body result := recorder.Result() defer result.Body.Close() + // check the response b, err := ioutil.ReadAll(result.Body) assert.NoError(suite.T(), err) - fmt.Println(string(b)) - // TODO write more assertions allee + // unmarshal the returned account + apimodelAccount := &apimodel.Account{} + err = json.Unmarshal(b, apimodelAccount) + suite.NoError(err) + + // check the returned api model account + // fields should be updated + suite.Equal("updated zork display name!!!", apimodelAccount.DisplayName) + suite.True(apimodelAccount.Locked) + suite.Empty(apimodelAccount.Note) + + // header values... + // should be set + suite.NotEmpty(apimodelAccount.Header) + suite.NotEmpty(apimodelAccount.HeaderStatic) + + // should be different from the values set before + suite.NotEqual("http://localhost:8080/fileserver/01F8MH1H7YV1Z7D2C8K2730QBF/header/original/01PFPMWK2FF0D9WMHEJHR07C3Q.jpeg", apimodelAccount.Header) + suite.NotEqual("http://localhost:8080/fileserver/01F8MH1H7YV1Z7D2C8K2730QBF/header/small/01PFPMWK2FF0D9WMHEJHR07C3Q.jpeg", apimodelAccount.HeaderStatic) } func TestAccountUpdateTestSuite(t *testing.T) { diff --git a/internal/api/client/account/accountverify_test.go b/internal/api/client/account/accountverify_test.go index 85b0dce50..746d2f5fe 100644 --- a/internal/api/client/account/accountverify_test.go +++ b/internal/api/client/account/accountverify_test.go @@ -17,3 +17,77 @@ */ package account_test + +import ( + "encoding/json" + "io/ioutil" + "net/http" + "net/http/httptest" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/suite" + "github.com/superseriousbusiness/gotosocial/internal/api/client/account" + apimodel "github.com/superseriousbusiness/gotosocial/internal/api/model" + "github.com/superseriousbusiness/gotosocial/internal/gtsmodel" +) + +type AccountVerifyTestSuite struct { + AccountStandardTestSuite +} + +func (suite *AccountVerifyTestSuite) TestAccountVerifyGet() { + testAccount := suite.testAccounts["local_account_1"] + + // set up the request + recorder := httptest.NewRecorder() + ctx := suite.newContext(recorder, http.MethodPatch, nil, account.UpdateCredentialsPath, "") + + // call the handler + suite.accountModule.AccountVerifyGETHandler(ctx) + + // 1. we should have OK because our request was valid + suite.Equal(http.StatusOK, recorder.Code) + + // 2. we should have no error message in the result body + result := recorder.Result() + defer result.Body.Close() + + // check the response + b, err := ioutil.ReadAll(result.Body) + assert.NoError(suite.T(), err) + + // unmarshal the returned account + apimodelAccount := &apimodel.Account{} + err = json.Unmarshal(b, apimodelAccount) + suite.NoError(err) + + createdAt, err := time.Parse(time.RFC3339, apimodelAccount.CreatedAt) + suite.NoError(err) + lastStatusAt, err := time.Parse(time.RFC3339, apimodelAccount.LastStatusAt) + suite.NoError(err) + + suite.Equal(testAccount.ID, apimodelAccount.ID) + suite.Equal(testAccount.Username, apimodelAccount.Username) + suite.Equal(testAccount.Username, apimodelAccount.Acct) + suite.Equal(testAccount.DisplayName, apimodelAccount.DisplayName) + suite.Equal(testAccount.Locked, apimodelAccount.Locked) + suite.Equal(testAccount.Bot, apimodelAccount.Bot) + suite.WithinDuration(testAccount.CreatedAt, createdAt, 30*time.Second) // we lose a bit of accuracy serializing so fuzz this a bit + suite.Equal(testAccount.URL, apimodelAccount.URL) + suite.Equal("http://localhost:8080/fileserver/01F8MH1H7YV1Z7D2C8K2730QBF/avatar/original/01F8MH58A357CV5K7R7TJMSH6S.jpeg", apimodelAccount.Avatar) + suite.Equal("http://localhost:8080/fileserver/01F8MH1H7YV1Z7D2C8K2730QBF/avatar/small/01F8MH58A357CV5K7R7TJMSH6S.jpeg", apimodelAccount.AvatarStatic) + suite.Equal("http://localhost:8080/fileserver/01F8MH1H7YV1Z7D2C8K2730QBF/header/original/01PFPMWK2FF0D9WMHEJHR07C3Q.jpeg", apimodelAccount.Header) + suite.Equal("http://localhost:8080/fileserver/01F8MH1H7YV1Z7D2C8K2730QBF/header/small/01PFPMWK2FF0D9WMHEJHR07C3Q.jpeg", apimodelAccount.HeaderStatic) + suite.Zero(apimodelAccount.FollowersCount) + suite.Equal(2, apimodelAccount.FollowingCount) + suite.Equal(5, apimodelAccount.StatusesCount) + suite.WithinDuration(time.Now(), lastStatusAt, 5*time.Minute) + suite.EqualValues(gtsmodel.VisibilityPublic, apimodelAccount.Source.Privacy) + suite.Equal(testAccount.Language, apimodelAccount.Source.Language) +} + +func TestAccountVerifyTestSuite(t *testing.T) { + suite.Run(t, new(AccountVerifyTestSuite)) +} diff --git a/internal/cliactions/admin/account/account.go b/internal/cliactions/admin/account/account.go index 369f2b800..e9a113f35 100644 --- a/internal/cliactions/admin/account/account.go +++ b/internal/cliactions/admin/account/account.go @@ -101,7 +101,7 @@ var Confirm cliactions.GTSAction = func(ctx context.Context, c *config.Config, l u.Approved = true u.Email = u.UnconfirmedEmail u.ConfirmedAt = time.Now() - if err := dbConn.UpdateByID(ctx, u.ID, u); err != nil { + if err := dbConn.UpdateByPrimaryKey(ctx, u); err != nil { return err } @@ -133,7 +133,7 @@ var Promote cliactions.GTSAction = func(ctx context.Context, c *config.Config, l return err } u.Admin = true - if err := dbConn.UpdateByID(ctx, u.ID, u); err != nil { + if err := dbConn.UpdateByPrimaryKey(ctx, u); err != nil { return err } @@ -165,7 +165,7 @@ var Demote cliactions.GTSAction = func(ctx context.Context, c *config.Config, lo return err } u.Admin = false - if err := dbConn.UpdateByID(ctx, u.ID, u); err != nil { + if err := dbConn.UpdateByPrimaryKey(ctx, u); err != nil { return err } @@ -197,7 +197,7 @@ var Disable cliactions.GTSAction = func(ctx context.Context, c *config.Config, l return err } u.Disabled = true - if err := dbConn.UpdateByID(ctx, u.ID, u); err != nil { + if err := dbConn.UpdateByPrimaryKey(ctx, u); err != nil { return err } @@ -250,7 +250,7 @@ var Password cliactions.GTSAction = func(ctx context.Context, c *config.Config, u.EncryptedPassword = string(pw) - if err := dbConn.UpdateByID(ctx, u.ID, u); err != nil { + if err := dbConn.UpdateByPrimaryKey(ctx, u); err != nil { return err } diff --git a/internal/db/basic.go b/internal/db/basic.go index 2a1141c8d..d94c98e45 100644 --- a/internal/db/basic.go +++ b/internal/db/basic.go @@ -62,12 +62,13 @@ type Basic interface { // The given interface i will be set to the result of the query, whatever it is. Use a pointer or a slice. Put(ctx context.Context, i interface{}) Error - // UpdateByID updates i with id id. + // UpdateByPrimaryKey updates all values of i based on its primary key. // The given interface i will be set to the result of the query, whatever it is. Use a pointer or a slice. - UpdateByID(ctx context.Context, id string, i interface{}) Error + UpdateByPrimaryKey(ctx context.Context, i interface{}) Error - // UpdateOneByID updates interface i with database the given database id. It will update one field of key key and value value. - UpdateOneByID(ctx context.Context, id string, key string, value interface{}, i interface{}) Error + // UpdateOneByPrimaryKey sets one column of interface, with the given key, to the given value. + // It uses the primary key of interface i to decide which row to update. This is usually the `id`. + UpdateOneByPrimaryKey(ctx context.Context, key string, value interface{}, i interface{}) Error // UpdateWhere updates column key of interface i with the given value, where the given parameters apply. UpdateWhere(ctx context.Context, where []Where, key string, value interface{}, i interface{}) Error diff --git a/internal/db/bundb/basic.go b/internal/db/bundb/basic.go index d4de5bb0b..1e7880379 100644 --- a/internal/db/bundb/basic.go +++ b/internal/db/bundb/basic.go @@ -95,7 +95,7 @@ func (b *basicDB) DeleteWhere(ctx context.Context, where []db.Where, i interface return b.conn.ProcessError(err) } -func (b *basicDB) UpdateByID(ctx context.Context, id string, i interface{}) db.Error { +func (b *basicDB) UpdateByPrimaryKey(ctx context.Context, i interface{}) db.Error { q := b.conn. NewUpdate(). Model(i). @@ -105,7 +105,7 @@ func (b *basicDB) UpdateByID(ctx context.Context, id string, i interface{}) db.E return b.conn.ProcessError(err) } -func (b *basicDB) UpdateOneByID(ctx context.Context, id string, key string, value interface{}, i interface{}) db.Error { +func (b *basicDB) UpdateOneByPrimaryKey(ctx context.Context, key string, value interface{}, i interface{}) db.Error { q := b.conn.NewUpdate(). Model(i). Set("? = ?", bun.Safe(key), value). diff --git a/internal/db/bundb/basic_test.go b/internal/db/bundb/basic_test.go index e5f7e159a..acdfb6640 100644 --- a/internal/db/bundb/basic_test.go +++ b/internal/db/bundb/basic_test.go @@ -64,6 +64,40 @@ func (suite *BasicTestSuite) TestGetAllNotNull() { } } +func (suite *BasicTestSuite) TestUpdateOneByPrimaryKeySetEmpty() { + testAccount := suite.testAccounts["local_account_1"] + + // try removing the note from zork + err := suite.db.UpdateOneByPrimaryKey(context.Background(), "note", "", testAccount) + suite.NoError(err) + + // get zork out of the database + dbAccount, err := suite.db.GetAccountByID(context.Background(), testAccount.ID) + suite.NoError(err) + suite.NotNil(dbAccount) + + // note should be empty now + suite.Empty(dbAccount.Note) +} + +func (suite *BasicTestSuite) TestUpdateOneByPrimaryKeySetValue() { + testAccount := suite.testAccounts["local_account_1"] + + note := "this is my new note :)" + + // try updating the note on zork + err := suite.db.UpdateOneByPrimaryKey(context.Background(), "note", note, testAccount) + suite.NoError(err) + + // get zork out of the database + dbAccount, err := suite.db.GetAccountByID(context.Background(), testAccount.ID) + suite.NoError(err) + suite.NotNil(dbAccount) + + // note should be set now + suite.Equal(note, dbAccount.Note) +} + func TestBasicTestSuite(t *testing.T) { suite.Run(t, new(BasicTestSuite)) } diff --git a/internal/federation/dereferencing/status.go b/internal/federation/dereferencing/status.go index 987285eee..1c9742797 100644 --- a/internal/federation/dereferencing/status.go +++ b/internal/federation/dereferencing/status.go @@ -44,7 +44,7 @@ func (d *deref) EnrichRemoteStatus(ctx context.Context, username string, status return nil, err } - if err := d.db.UpdateByID(ctx, status.ID, status); err != nil { + if err := d.db.UpdateByPrimaryKey(ctx, status); err != nil { return nil, fmt.Errorf("EnrichRemoteStatus: error updating status: %s", err) } @@ -119,7 +119,7 @@ func (d *deref) GetRemoteStatus(ctx context.Context, username string, remoteStat return nil, statusable, new, fmt.Errorf("GetRemoteStatus: error populating status fields: %s", err) } - if err := d.db.UpdateByID(ctx, gtsStatus.ID, gtsStatus); err != nil { + if err := d.db.UpdateByPrimaryKey(ctx, gtsStatus); err != nil { return nil, statusable, new, fmt.Errorf("GetRemoteStatus: error updating status: %s", err) } } diff --git a/internal/processing/account/account_test.go b/internal/processing/account/account_test.go new file mode 100644 index 000000000..1884f6057 --- /dev/null +++ b/internal/processing/account/account_test.go @@ -0,0 +1,97 @@ +/* + GoToSocial + Copyright (C) 2021 GoToSocial Authors admin@gotosocial.org + + 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 account_test + +import ( + "github.com/go-fed/activity/pub" + "github.com/sirupsen/logrus" + "github.com/stretchr/testify/suite" + "github.com/superseriousbusiness/gotosocial/internal/blob" + "github.com/superseriousbusiness/gotosocial/internal/config" + "github.com/superseriousbusiness/gotosocial/internal/db" + "github.com/superseriousbusiness/gotosocial/internal/federation" + "github.com/superseriousbusiness/gotosocial/internal/gtsmodel" + "github.com/superseriousbusiness/gotosocial/internal/media" + "github.com/superseriousbusiness/gotosocial/internal/messages" + "github.com/superseriousbusiness/gotosocial/internal/oauth" + "github.com/superseriousbusiness/gotosocial/internal/processing/account" + "github.com/superseriousbusiness/gotosocial/internal/transport" + "github.com/superseriousbusiness/gotosocial/internal/typeutils" + "github.com/superseriousbusiness/gotosocial/testrig" +) + +type AccountStandardTestSuite struct { + // standard suite interfaces + suite.Suite + config *config.Config + db db.DB + log *logrus.Logger + tc typeutils.TypeConverter + storage blob.Storage + mediaHandler media.Handler + oauthServer oauth.Server + fromClientAPIChan chan messages.FromClientAPI + httpClient pub.HttpClient + transportController transport.Controller + federator federation.Federator + + // standard suite models + testTokens map[string]*gtsmodel.Token + testClients map[string]*gtsmodel.Client + testApplications map[string]*gtsmodel.Application + testUsers map[string]*gtsmodel.User + testAccounts map[string]*gtsmodel.Account + testAttachments map[string]*gtsmodel.MediaAttachment + testStatuses map[string]*gtsmodel.Status + + // module being tested + accountProcessor account.Processor +} + +func (suite *AccountStandardTestSuite) SetupSuite() { + suite.testTokens = testrig.NewTestTokens() + suite.testClients = testrig.NewTestClients() + suite.testApplications = testrig.NewTestApplications() + suite.testUsers = testrig.NewTestUsers() + suite.testAccounts = testrig.NewTestAccounts() + suite.testAttachments = testrig.NewTestAttachments() + suite.testStatuses = testrig.NewTestStatuses() +} + +func (suite *AccountStandardTestSuite) SetupTest() { + suite.config = testrig.NewTestConfig() + suite.db = testrig.NewTestDB() + suite.log = testrig.NewTestLog() + suite.tc = testrig.NewTestTypeConverter(suite.db) + suite.storage = testrig.NewTestStorage() + suite.mediaHandler = testrig.NewTestMediaHandler(suite.db, suite.storage) + suite.oauthServer = testrig.NewTestOauthServer(suite.db) + suite.fromClientAPIChan = make(chan messages.FromClientAPI, 100) + suite.httpClient = testrig.NewMockHTTPClient(nil) + suite.transportController = testrig.NewTestTransportController(suite.httpClient, suite.db) + suite.federator = testrig.NewTestFederator(suite.db, suite.transportController, suite.storage) + suite.accountProcessor = account.New(suite.db, suite.tc, suite.mediaHandler, suite.oauthServer, suite.fromClientAPIChan, suite.federator, suite.config, suite.log) + testrig.StandardDBSetup(suite.db, nil) + testrig.StandardStorageSetup(suite.storage, "../../../testrig/media") +} + +func (suite *AccountStandardTestSuite) TearDownTest() { + testrig.StandardDBTeardown(suite.db) + testrig.StandardStorageTeardown(suite.storage) +} diff --git a/internal/processing/account/update.go b/internal/processing/account/update.go index c0fee8e25..e997a95c7 100644 --- a/internal/processing/account/update.go +++ b/internal/processing/account/update.go @@ -39,13 +39,13 @@ func (p *processor) Update(ctx context.Context, account *gtsmodel.Account, form l := p.log.WithField("func", "AccountUpdate") if form.Discoverable != nil { - if err := p.db.UpdateOneByID(ctx, account.ID, "discoverable", *form.Discoverable, >smodel.Account{}); err != nil { + if err := p.db.UpdateOneByPrimaryKey(ctx, "discoverable", *form.Discoverable, account); err != nil { return nil, fmt.Errorf("error updating discoverable: %s", err) } } if form.Bot != nil { - if err := p.db.UpdateOneByID(ctx, account.ID, "bot", *form.Bot, >smodel.Account{}); err != nil { + if err := p.db.UpdateOneByPrimaryKey(ctx, "bot", *form.Bot, account); err != nil { return nil, fmt.Errorf("error updating bot: %s", err) } } @@ -55,7 +55,7 @@ func (p *processor) Update(ctx context.Context, account *gtsmodel.Account, form return nil, err } displayName := text.RemoveHTML(*form.DisplayName) // no html allowed in display name - if err := p.db.UpdateOneByID(ctx, account.ID, "display_name", displayName, >smodel.Account{}); err != nil { + if err := p.db.UpdateOneByPrimaryKey(ctx, "display_name", displayName, account); err != nil { return nil, err } } @@ -65,7 +65,7 @@ func (p *processor) Update(ctx context.Context, account *gtsmodel.Account, form return nil, err } note := text.SanitizeHTML(*form.Note) // html OK in note but sanitize it - if err := p.db.UpdateOneByID(ctx, account.ID, "note", note, >smodel.Account{}); err != nil { + if err := p.db.UpdateOneByPrimaryKey(ctx, "note", note, account); err != nil { return nil, err } } @@ -87,7 +87,7 @@ func (p *processor) Update(ctx context.Context, account *gtsmodel.Account, form } if form.Locked != nil { - if err := p.db.UpdateOneByID(ctx, account.ID, "locked", *form.Locked, >smodel.Account{}); err != nil { + if err := p.db.UpdateOneByPrimaryKey(ctx, "locked", *form.Locked, account); err != nil { return nil, err } } @@ -97,13 +97,13 @@ func (p *processor) Update(ctx context.Context, account *gtsmodel.Account, form if err := validate.Language(*form.Source.Language); err != nil { return nil, err } - if err := p.db.UpdateOneByID(ctx, account.ID, "language", *form.Source.Language, >smodel.Account{}); err != nil { + if err := p.db.UpdateOneByPrimaryKey(ctx, "language", *form.Source.Language, account); err != nil { return nil, err } } if form.Source.Sensitive != nil { - if err := p.db.UpdateOneByID(ctx, account.ID, "locked", *form.Locked, >smodel.Account{}); err != nil { + if err := p.db.UpdateOneByPrimaryKey(ctx, "locked", *form.Locked, account); err != nil { return nil, err } } @@ -112,7 +112,7 @@ func (p *processor) Update(ctx context.Context, account *gtsmodel.Account, form if err := validate.Privacy(*form.Source.Privacy); err != nil { return nil, err } - if err := p.db.UpdateOneByID(ctx, account.ID, "privacy", *form.Source.Privacy, >smodel.Account{}); err != nil { + if err := p.db.UpdateOneByPrimaryKey(ctx, "privacy", *form.Source.Privacy, account); err != nil { return nil, err } } diff --git a/internal/processing/account/update_test.go b/internal/processing/account/update_test.go new file mode 100644 index 000000000..b18a5e42e --- /dev/null +++ b/internal/processing/account/update_test.go @@ -0,0 +1,75 @@ +/* + GoToSocial + Copyright (C) 2021 GoToSocial Authors admin@gotosocial.org + + 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 account_test + +import ( + "context" + "testing" + + "github.com/stretchr/testify/suite" + "github.com/superseriousbusiness/gotosocial/internal/ap" + apimodel "github.com/superseriousbusiness/gotosocial/internal/api/model" +) + +type AccountUpdateTestSuite struct { + AccountStandardTestSuite +} + +func (suite *AccountUpdateTestSuite) TestAccountUpdateSimple() { + testAccount := suite.testAccounts["local_account_1"] + + locked := true + displayName := "new display name" + note := "" + + form := &apimodel.UpdateCredentialsRequest{ + DisplayName: &displayName, + Locked: &locked, + Note: ¬e, + } + + // should get no error from the update function, and an api model account returned + apiAccount, err := suite.accountProcessor.Update(context.Background(), testAccount, form) + suite.NoError(err) + suite.NotNil(apiAccount) + + // fields on the profile should be updated + suite.True(apiAccount.Locked) + suite.Equal(displayName, apiAccount.DisplayName) + suite.Empty(apiAccount.Note) + + // we should have an update in the client api channel + msg := <-suite.fromClientAPIChan + suite.Equal(ap.ActivityUpdate, msg.APActivityType) + suite.Equal(ap.ObjectProfile, msg.APObjectType) + suite.NotNil(msg.OriginAccount) + suite.Equal(testAccount.ID, msg.OriginAccount.ID) + suite.Nil(msg.TargetAccount) + + // fields should be updated in the database as well + dbAccount, err := suite.db.GetAccountByID(context.Background(), testAccount.ID) + suite.NoError(err) + suite.True(dbAccount.Locked) + suite.Equal(displayName, dbAccount.DisplayName) + suite.Empty(dbAccount.Note) +} + +func TestAccountUpdateTestSuite(t *testing.T) { + suite.Run(t, new(AccountUpdateTestSuite)) +} diff --git a/internal/processing/admin/createdomainblock.go b/internal/processing/admin/createdomainblock.go index 9c4ff780f..50df056e5 100644 --- a/internal/processing/admin/createdomainblock.go +++ b/internal/processing/admin/createdomainblock.go @@ -108,7 +108,7 @@ func (p *processor) initiateDomainBlockSideEffects(ctx context.Context, account instance.ContactAccountUsername = "" instance.ContactAccountID = "" instance.Version = "" - if err := p.db.UpdateByID(ctx, instance.ID, instance); err != nil { + if err := p.db.UpdateByPrimaryKey(ctx, instance); err != nil { l.Errorf("domainBlockProcessSideEffects: db error updating instance: %s", err) } l.Debug("domainBlockProcessSideEffects: instance entry updated") diff --git a/internal/processing/admin/deletedomainblock.go b/internal/processing/admin/deletedomainblock.go index 2563b557d..d11374c78 100644 --- a/internal/processing/admin/deletedomainblock.go +++ b/internal/processing/admin/deletedomainblock.go @@ -60,7 +60,7 @@ func (p *processor) DomainBlockDelete(ctx context.Context, account *gtsmodel.Acc }, i); err == nil { i.SuspendedAt = time.Time{} i.DomainBlockID = "" - if err := p.db.UpdateByID(ctx, i.ID, i); err != nil { + if err := p.db.UpdateByPrimaryKey(ctx, i); err != nil { return nil, gtserror.NewErrorInternalError(fmt.Errorf("couldn't update database entry for instance %s: %s", domainBlock.Domain, err)) } } diff --git a/internal/processing/instance.go b/internal/processing/instance.go index e74d3077a..41139c491 100644 --- a/internal/processing/instance.go +++ b/internal/processing/instance.go @@ -147,7 +147,7 @@ func (p *processor) InstancePatch(ctx context.Context, form *apimodel.InstanceSe } } - if err := p.db.UpdateByID(ctx, i.ID, i); err != nil { + if err := p.db.UpdateByPrimaryKey(ctx, i); err != nil { return nil, gtserror.NewErrorInternalError(fmt.Errorf("db error updating instance %s: %s", p.config.Host, err)) } diff --git a/internal/processing/media/update.go b/internal/processing/media/update.go index 6f15f2ace..e6c78563d 100644 --- a/internal/processing/media/update.go +++ b/internal/processing/media/update.go @@ -46,7 +46,7 @@ func (p *processor) Update(ctx context.Context, account *gtsmodel.Account, media if form.Description != nil { attachment.Description = text.RemoveHTML(*form.Description) - if err := p.db.UpdateByID(ctx, mediaAttachmentID, attachment); err != nil { + if err := p.db.UpdateByPrimaryKey(ctx, attachment); err != nil { return nil, gtserror.NewErrorInternalError(fmt.Errorf("database error updating description: %s", err)) } } @@ -58,7 +58,7 @@ func (p *processor) Update(ctx context.Context, account *gtsmodel.Account, media } attachment.FileMeta.Focus.X = focusx attachment.FileMeta.Focus.Y = focusy - if err := p.db.UpdateByID(ctx, mediaAttachmentID, attachment); err != nil { + if err := p.db.UpdateByPrimaryKey(ctx, attachment); err != nil { return nil, gtserror.NewErrorInternalError(fmt.Errorf("database error updating focus: %s", err)) } }