diff --git a/internal/api/client/account/accountupdate.go b/internal/api/client/account/accountupdate.go index c38ede252..9a377f3b8 100644 --- a/internal/api/client/account/accountupdate.go +++ b/internal/api/client/account/accountupdate.go @@ -19,7 +19,9 @@ package account import ( + "fmt" "net/http" + "strconv" "github.com/gin-gonic/gin" "github.com/superseriousbusiness/gotosocial/internal/api/model" @@ -107,17 +109,24 @@ func (m *Module) AccountUpdateCredentialsPATCHHandler(c *gin.Context) { } l.Tracef("retrieved account %+v", authed.Account.ID) - form := &model.UpdateCredentialsRequest{} - if err := c.ShouldBind(&form); err != nil || form == nil { - l.Debugf("could not parse form from request: %s", err) + form, err := parseUpdateAccountForm(c) + if err != nil { c.JSON(http.StatusBadRequest, gin.H{"error": err.Error()}) 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 { + if form.Discoverable == nil && + form.Bot == nil && + form.DisplayName == nil && + form.Note == nil && + form.Avatar == nil && + form.Header == nil && + form.Locked == nil && + form.Source.Privacy == nil && + form.Source.Sensitive == nil && + form.Source.Language == nil && + form.FieldsAttributes == nil { l.Debugf("could not parse form from request") c.JSON(http.StatusBadRequest, gin.H{"error": "empty form submitted"}) return @@ -133,3 +142,34 @@ func (m *Module) AccountUpdateCredentialsPATCHHandler(c *gin.Context) { l.Tracef("conversion successful, returning OK and mastosensitive account %+v", acctSensitive) c.JSON(http.StatusOK, acctSensitive) } + +func parseUpdateAccountForm(c *gin.Context) (*model.UpdateCredentialsRequest, error) { + // parse main fields from request + form := &model.UpdateCredentialsRequest{ + Source: &model.UpdateSource{}, + } + if err := c.ShouldBind(&form); err != nil || form == nil { + return nil, fmt.Errorf("could not parse form from request: %s", err) + } + + // parse source field-by-field + sourceMap := c.PostFormMap("source") + + if privacy, ok := sourceMap["privacy"]; ok { + form.Source.Privacy = &privacy + } + + if sensitive, ok := sourceMap["sensitive"]; ok { + sensitiveBool, err := strconv.ParseBool(sensitive) + if err != nil { + return nil, fmt.Errorf("error parsing form source[sensitive]: %s", err) + } + form.Source.Sensitive = &sensitiveBool + } + + if language, ok := sourceMap["language"]; ok { + form.Source.Language = &language + } + + return form, nil +} diff --git a/internal/api/client/account/accountupdate_test.go b/internal/api/client/account/accountupdate_test.go index a02573631..bafda0e01 100644 --- a/internal/api/client/account/accountupdate_test.go +++ b/internal/api/client/account/accountupdate_test.go @@ -19,8 +19,8 @@ package account_test import ( + "context" "encoding/json" - "fmt" "io/ioutil" "net/http" "net/http/httptest" @@ -37,22 +37,224 @@ type AccountUpdateTestSuite struct { AccountStandardTestSuite } -func (suite *AccountUpdateTestSuite) TestAccountUpdateCredentialsPATCHHandlerSimple() { +func (suite *AccountUpdateTestSuite) TestAccountUpdateCredentialsPATCHHandler() { // 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 + // we're updating the note of zork + newBio := "this is my new bio read it and weep" requestBody, w, err := testrig.CreateMultipartFormData( - "header", "../../../../testrig/media/test-jpeg.jpg", + "", "", map[string]string{ - "display_name": "updated zork display name!!!", - "note": "", - "locked": "true", + "note": newBio, }) if err != nil { panic(err) } + bodyBytes := requestBody.Bytes() recorder := httptest.NewRecorder() - ctx := suite.newContext(recorder, http.MethodPatch, requestBody.Bytes(), account.UpdateCredentialsPath, w.FormDataContentType()) + ctx := suite.newContext(recorder, http.MethodPatch, bodyBytes, account.UpdateCredentialsPath, w.FormDataContentType()) + + // call the handler + suite.accountModule.AccountUpdateCredentialsPATCHHandler(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) + + // check the returned api model account + // fields should be updated + suite.Equal("
this is my new bio read it and weep
", apimodelAccount.Note) +} + +func (suite *AccountUpdateTestSuite) TestAccountUpdateCredentialsPATCHHandlerUnlockLock() { + // set up the first request + requestBody1, w1, err := testrig.CreateMultipartFormData( + "", "", + map[string]string{ + "locked": "false", + }) + if err != nil { + panic(err) + } + bodyBytes1 := requestBody1.Bytes() + recorder1 := httptest.NewRecorder() + ctx1 := suite.newContext(recorder1, http.MethodPatch, bodyBytes1, account.UpdateCredentialsPath, w1.FormDataContentType()) + + // call the handler + suite.accountModule.AccountUpdateCredentialsPATCHHandler(ctx1) + + // 1. we should have OK because our request was valid + suite.Equal(http.StatusOK, recorder1.Code) + + // 2. we should have no error message in the result body + result1 := recorder1.Result() + defer result1.Body.Close() + + // check the response + b1, err := ioutil.ReadAll(result1.Body) + assert.NoError(suite.T(), err) + + // unmarshal the returned account + apimodelAccount1 := &apimodel.Account{} + err = json.Unmarshal(b1, apimodelAccount1) + suite.NoError(err) + + // check the returned api model account + // fields should be updated + suite.False(apimodelAccount1.Locked) + + // set up the first request + requestBody2, w2, err := testrig.CreateMultipartFormData( + "", "", + map[string]string{ + "locked": "true", + }) + if err != nil { + panic(err) + } + bodyBytes2 := requestBody2.Bytes() + recorder2 := httptest.NewRecorder() + ctx2 := suite.newContext(recorder2, http.MethodPatch, bodyBytes2, account.UpdateCredentialsPath, w2.FormDataContentType()) + + // call the handler + suite.accountModule.AccountUpdateCredentialsPATCHHandler(ctx2) + + // 1. we should have OK because our request was valid + suite.Equal(http.StatusOK, recorder1.Code) + + // 2. we should have no error message in the result body + result2 := recorder2.Result() + defer result2.Body.Close() + + // check the response + b2, err := ioutil.ReadAll(result2.Body) + suite.NoError(err) + + // unmarshal the returned account + apimodelAccount2 := &apimodel.Account{} + err = json.Unmarshal(b2, apimodelAccount2) + suite.NoError(err) + + // check the returned api model account + // fields should be updated + suite.True(apimodelAccount2.Locked) +} + +func (suite *AccountUpdateTestSuite) TestAccountUpdateCredentialsPATCHHandlerGetAccountFirst() { + // get the account first to make sure it's in the database cache -- when the account is updated via + // the PATCH handler, it should invalidate the cache and not return the old version + _, err := suite.db.GetAccountByID(context.Background(), suite.testAccounts["local_account_1"].ID) + suite.NoError(err) + + // set up the request + // we're updating the note of zork + newBio := "this is my new bio read it and weep" + requestBody, w, err := testrig.CreateMultipartFormData( + "", "", + map[string]string{ + "note": newBio, + }) + if err != nil { + panic(err) + } + bodyBytes := requestBody.Bytes() + recorder := httptest.NewRecorder() + ctx := suite.newContext(recorder, http.MethodPatch, bodyBytes, account.UpdateCredentialsPath, w.FormDataContentType()) + + // call the handler + suite.accountModule.AccountUpdateCredentialsPATCHHandler(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) + + // check the returned api model account + // fields should be updated + suite.Equal("this is my new bio read it and weep
", apimodelAccount.Note) +} + +func (suite *AccountUpdateTestSuite) TestAccountUpdateCredentialsPATCHHandlerTwoFields() { + // set up the request + // we're updating the note of zork, and setting locked to true + newBio := "this is my new bio read it and weep" + requestBody, w, err := testrig.CreateMultipartFormData( + "", "", + map[string]string{ + "note": newBio, + "locked": "true", + }) + if err != nil { + panic(err) + } + bodyBytes := requestBody.Bytes() + recorder := httptest.NewRecorder() + ctx := suite.newContext(recorder, http.MethodPatch, bodyBytes, account.UpdateCredentialsPath, w.FormDataContentType()) + + // call the handler + suite.accountModule.AccountUpdateCredentialsPATCHHandler(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) + + // check the returned api model account + // fields should be updated + suite.Equal("this is my new bio read it and weep
", apimodelAccount.Note) + suite.True(apimodelAccount.Locked) +} + +func (suite *AccountUpdateTestSuite) TestAccountUpdateCredentialsPATCHHandlerWithMedia() { + // 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) + } + bodyBytes := requestBody.Bytes() + recorder := httptest.NewRecorder() + ctx := suite.newContext(recorder, http.MethodPatch, bodyBytes, account.UpdateCredentialsPath, w.FormDataContentType()) // call the handler suite.accountModule.AccountUpdateCredentialsPATCHHandler(ctx) @@ -67,7 +269,6 @@ func (suite *AccountUpdateTestSuite) TestAccountUpdateCredentialsPATCHHandlerSim // check the response b, err := ioutil.ReadAll(result.Body) assert.NoError(suite.T(), err) - fmt.Println(string(b)) // unmarshal the returned account apimodelAccount := &apimodel.Account{} @@ -90,6 +291,74 @@ func (suite *AccountUpdateTestSuite) TestAccountUpdateCredentialsPATCHHandlerSim suite.NotEqual("http://localhost:8080/fileserver/01F8MH1H7YV1Z7D2C8K2730QBF/header/small/01PFPMWK2FF0D9WMHEJHR07C3Q.jpeg", apimodelAccount.HeaderStatic) } +func (suite *AccountUpdateTestSuite) TestAccountUpdateCredentialsPATCHHandlerEmptyForm() { + // set up the request + bodyBytes := []byte{} + recorder := httptest.NewRecorder() + ctx := suite.newContext(recorder, http.MethodPatch, bodyBytes, account.UpdateCredentialsPath, "") + + // call the handler + suite.accountModule.AccountUpdateCredentialsPATCHHandler(ctx) + + // 1. we should have OK because our request was valid + suite.Equal(http.StatusBadRequest, 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) + suite.Equal(`{"error":"empty form submitted"}`, string(b)) +} + +func (suite *AccountUpdateTestSuite) TestAccountUpdateCredentialsPATCHHandlerUpdateSource() { + // set up the request + // we're updating the language of zork + newLanguage := "de" + requestBody, w, err := testrig.CreateMultipartFormData( + "", "", + map[string]string{ + "source[privacy]": string(apimodel.VisibilityPrivate), + "source[language]": "de", + "source[sensitive]": "true", + "locked": "true", + }) + if err != nil { + panic(err) + } + bodyBytes := requestBody.Bytes() + recorder := httptest.NewRecorder() + ctx := suite.newContext(recorder, http.MethodPatch, bodyBytes, account.UpdateCredentialsPath, w.FormDataContentType()) + + // call the handler + suite.accountModule.AccountUpdateCredentialsPATCHHandler(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) + + // check the returned api model account + // fields should be updated + suite.Equal(newLanguage, apimodelAccount.Source.Language) + suite.EqualValues(apimodel.VisibilityPrivate, apimodelAccount.Source.Privacy) + suite.True(apimodelAccount.Source.Sensitive) + suite.True(apimodelAccount.Locked) +} + func TestAccountUpdateTestSuite(t *testing.T) { suite.Run(t, new(AccountUpdateTestSuite)) } diff --git a/internal/api/model/status.go b/internal/api/model/status.go index c5b5a4640..8be1a4870 100644 --- a/internal/api/model/status.go +++ b/internal/api/model/status.go @@ -160,6 +160,7 @@ type StatusCreateRequest struct { // - public // - unlisted // - private +// - mutuals_only // - direct type Visibility string diff --git a/internal/db/basic.go b/internal/db/basic.go index d94c98e45..44000ef24 100644 --- a/internal/db/basic.go +++ b/internal/db/basic.go @@ -66,10 +66,6 @@ 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. UpdateByPrimaryKey(ctx context.Context, 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/account.go b/internal/db/bundb/account.go index 32a70f7cd..745e41567 100644 --- a/internal/db/bundb/account.go +++ b/internal/db/bundb/account.go @@ -22,7 +22,6 @@ import ( "context" "errors" "fmt" - "strings" "time" "github.com/superseriousbusiness/gotosocial/internal/cache" @@ -103,16 +102,15 @@ func (a *accountDB) getAccount(ctx context.Context, cacheGet func() (*gtsmodel.A } func (a *accountDB) UpdateAccount(ctx context.Context, account *gtsmodel.Account) (*gtsmodel.Account, db.Error) { - if strings.TrimSpace(account.ID) == "" { - // TODO: we should not need this check here - return nil, errors.New("account had no ID") - } - - // Update the account's last-used + // Update the account's last-updated account.UpdatedAt = time.Now() // Update the account model in the DB - _, err := a.conn.NewUpdate().Model(account).WherePK().Exec(ctx) + _, err := a.conn. + NewUpdate(). + Model(account). + WherePK(). + Exec(ctx) if err != nil { return nil, a.conn.ProcessError(err) } diff --git a/internal/db/bundb/basic.go b/internal/db/bundb/basic.go index 1e7880379..e5a1fbaf9 100644 --- a/internal/db/bundb/basic.go +++ b/internal/db/bundb/basic.go @@ -105,16 +105,6 @@ func (b *basicDB) UpdateByPrimaryKey(ctx context.Context, i interface{}) db.Erro return b.conn.ProcessError(err) } -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). - WherePK() - - _, err := q.Exec(ctx) - return b.conn.ProcessError(err) -} - func (b *basicDB) UpdateWhere(ctx context.Context, where []db.Where, key string, value interface{}, i interface{}) db.Error { q := b.conn.NewUpdate().Model(i) diff --git a/internal/db/bundb/basic_test.go b/internal/db/bundb/basic_test.go index acdfb6640..e5f7e159a 100644 --- a/internal/db/bundb/basic_test.go +++ b/internal/db/bundb/basic_test.go @@ -64,40 +64,6 @@ 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/db/bundb/bundb.go b/internal/db/bundb/bundb.go index 7ddcab5c7..400535da7 100644 --- a/internal/db/bundb/bundb.go +++ b/internal/db/bundb/bundb.go @@ -147,6 +147,11 @@ func NewBunDBService(ctx context.Context, c *config.Config, log *logrus.Logger) return nil, fmt.Errorf("database type %s not supported for bundb", strings.ToLower(c.DBConfig.Type)) } + if log.Level >= logrus.TraceLevel { + // add a hook to just log queries and the time they take + conn.DB.AddQueryHook(newDebugQueryHook(log)) + } + // actually *begin* the connection so that we can tell if the db is there and listening if err := conn.Ping(); err != nil { return nil, fmt.Errorf("db connection error: %s", err) @@ -402,7 +407,7 @@ func (ps *bunDBService) MentionStringsToMentions(ctx context.Context, targetAcco return menchies, nil } -func (ps *bunDBService) TagStringsToTags(ctx context.Context, tags []string, originAccountID string, statusID string) ([]*gtsmodel.Tag, error) { +func (ps *bunDBService) TagStringsToTags(ctx context.Context, tags []string, originAccountID string) ([]*gtsmodel.Tag, error) { newTags := []*gtsmodel.Tag{} for _, t := range tags { tag := >smodel.Tag{} @@ -438,7 +443,7 @@ func (ps *bunDBService) TagStringsToTags(ctx context.Context, tags []string, ori return newTags, nil } -func (ps *bunDBService) EmojiStringsToEmojis(ctx context.Context, emojis []string, originAccountID string, statusID string) ([]*gtsmodel.Emoji, error) { +func (ps *bunDBService) EmojiStringsToEmojis(ctx context.Context, emojis []string) ([]*gtsmodel.Emoji, error) { newEmojis := []*gtsmodel.Emoji{} for _, e := range emojis { emoji := >smodel.Emoji{} diff --git a/internal/db/bundb/trace.go b/internal/db/bundb/trace.go new file mode 100644 index 000000000..e62e8c01f --- /dev/null +++ b/internal/db/bundb/trace.go @@ -0,0 +1,53 @@ +/* + 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#hello here i am!
`, apiAccount.Note) // we should have an update in the client api channel msg := <-suite.fromClientAPIChan @@ -67,7 +67,50 @@ func (suite *AccountUpdateTestSuite) TestAccountUpdateSimple() { suite.NoError(err) suite.True(dbAccount.Locked) suite.Equal(displayName, dbAccount.DisplayName) - suite.Empty(dbAccount.Note) + suite.Equal(`#hello here i am!
`, dbAccount.Note) +} + +func (suite *AccountUpdateTestSuite) TestAccountUpdateWithMention() { + testAccount := suite.testAccounts["local_account_1"] + + locked := true + displayName := "new display name" + note := `#hello here i am! + +go check out @1happyturtle, they have a cool account! +` + noteExpected := `#hello here i am!
go check out @1happyturtle, they have a cool account!