From 940abc279cb84e6bdb62326565b04c279bfb596b Mon Sep 17 00:00:00 2001 From: tobi <31960611+tsmethurst@users.noreply.github.com> Date: Wed, 16 Nov 2022 11:27:08 +0100 Subject: [PATCH] [chore] reversion: use specific columns for updating user again (#1059) --- .../action/admin/account/account.go | 12 +++------ internal/api/client/auth/authorize_test.go | 21 ++++++++------- internal/db/bundb/user.go | 27 +++++++++++++------ internal/db/user.go | 4 +-- 4 files changed, 37 insertions(+), 27 deletions(-) diff --git a/cmd/gotosocial/action/admin/account/account.go b/cmd/gotosocial/action/admin/account/account.go index 070e1fdcb..e302fff57 100644 --- a/cmd/gotosocial/action/admin/account/account.go +++ b/cmd/gotosocial/action/admin/account/account.go @@ -151,8 +151,7 @@ admin := true u.Admin = &admin - u.UpdatedAt = time.Now() - if err := dbConn.UpdateUser(ctx, u); err != nil { + if err := dbConn.UpdateUser(ctx, u, "admin"); err != nil { return err } @@ -186,8 +185,7 @@ admin := false u.Admin = &admin - u.UpdatedAt = time.Now() - if err := dbConn.UpdateUser(ctx, u); err != nil { + if err := dbConn.UpdateUser(ctx, u, "admin"); err != nil { return err } @@ -221,8 +219,7 @@ disabled := true u.Disabled = &disabled - u.UpdatedAt = time.Now() - if err := dbConn.UpdateUser(ctx, u); err != nil { + if err := dbConn.UpdateUser(ctx, u, "disabled"); err != nil { return err } @@ -268,8 +265,7 @@ } u.EncryptedPassword = string(pw) - u.UpdatedAt = time.Now() - if err := dbConn.UpdateUser(ctx, u); err != nil { + if err := dbConn.UpdateUser(ctx, u, "encrypted_password"); err != nil { return err } diff --git a/internal/api/client/auth/authorize_test.go b/internal/api/client/auth/authorize_test.go index e3e4ce9ee..738b3b910 100644 --- a/internal/api/client/auth/authorize_test.go +++ b/internal/api/client/auth/authorize_test.go @@ -20,7 +20,7 @@ type AuthAuthorizeTestSuite struct { type authorizeHandlerTestCase struct { description string - mutateUserAccount func(*gtsmodel.User, *gtsmodel.Account) + mutateUserAccount func(*gtsmodel.User, *gtsmodel.Account) []string expectedStatusCode int expectedLocationHeader string } @@ -29,40 +29,44 @@ func (suite *AuthAuthorizeTestSuite) TestAccountAuthorizeHandler() { tests := []authorizeHandlerTestCase{ { description: "user has their email unconfirmed", - mutateUserAccount: func(user *gtsmodel.User, account *gtsmodel.Account) { - // nothing to do, weed_lord420 already has their email unconfirmed + mutateUserAccount: func(user *gtsmodel.User, account *gtsmodel.Account) []string { + user.ConfirmedAt = time.Time{} + return []string{"confirmed_at"} }, expectedStatusCode: http.StatusSeeOther, expectedLocationHeader: auth.CheckYourEmailPath, }, { description: "user has their email confirmed but is not approved", - mutateUserAccount: func(user *gtsmodel.User, account *gtsmodel.Account) { + mutateUserAccount: func(user *gtsmodel.User, account *gtsmodel.Account) []string { user.ConfirmedAt = time.Now() user.Email = user.UnconfirmedEmail + return []string{"confirmed_at", "email"} }, expectedStatusCode: http.StatusSeeOther, expectedLocationHeader: auth.WaitForApprovalPath, }, { description: "user has their email confirmed and is approved, but User entity has been disabled", - mutateUserAccount: func(user *gtsmodel.User, account *gtsmodel.Account) { + mutateUserAccount: func(user *gtsmodel.User, account *gtsmodel.Account) []string { user.ConfirmedAt = time.Now() user.Email = user.UnconfirmedEmail user.Approved = testrig.TrueBool() user.Disabled = testrig.TrueBool() + return []string{"confirmed_at", "email", "approved", "disabled"} }, expectedStatusCode: http.StatusSeeOther, expectedLocationHeader: auth.AccountDisabledPath, }, { description: "user has their email confirmed and is approved, but Account entity has been suspended", - mutateUserAccount: func(user *gtsmodel.User, account *gtsmodel.Account) { + mutateUserAccount: func(user *gtsmodel.User, account *gtsmodel.Account) []string { user.ConfirmedAt = time.Now() user.Email = user.UnconfirmedEmail user.Approved = testrig.TrueBool() user.Disabled = testrig.FalseBool() account.SuspendedAt = time.Now() + return []string{"confirmed_at", "email", "approved", "disabled"} }, expectedStatusCode: http.StatusSeeOther, expectedLocationHeader: auth.AccountDisabledPath, @@ -77,7 +81,6 @@ func (suite *AuthAuthorizeTestSuite) TestAccountAuthorizeHandler() { *user = *suite.testUsers["unconfirmed_account"] *account = *suite.testAccounts["unconfirmed_account"] - user.SignInCount++ // cannot be 0 or fails NULL constraint testSession := sessions.Default(ctx) testSession.Set(sessionUserID, user.ID) @@ -86,11 +89,11 @@ func (suite *AuthAuthorizeTestSuite) TestAccountAuthorizeHandler() { panic(fmt.Errorf("failed on case %s: %w", testCase.description, err)) } - testCase.mutateUserAccount(user, account) + columns := testCase.mutateUserAccount(user, account) testCase.description = fmt.Sprintf("%s, %t, %s", user.Email, *user.Disabled, account.SuspendedAt) - err := suite.db.UpdateUser(context.Background(), user) + err := suite.db.UpdateUser(context.Background(), user, columns...) suite.NoError(err) err = suite.db.UpdateAccount(context.Background(), account) suite.NoError(err) diff --git a/internal/db/bundb/user.go b/internal/db/bundb/user.go index d9b281a6f..ab3cc0f67 100644 --- a/internal/db/bundb/user.go +++ b/internal/db/bundb/user.go @@ -133,18 +133,29 @@ func (u *userDB) PutUser(ctx context.Context, user *gtsmodel.User) db.Error { }) } -func (u *userDB) UpdateUser(ctx context.Context, user *gtsmodel.User) db.Error { +func (u *userDB) UpdateUser(ctx context.Context, user *gtsmodel.User, columns ...string) db.Error { // Update the user's last-updated user.UpdatedAt = time.Now() - return u.cache.Store(user, func() error { - _, err := u.conn. - NewUpdate(). - Model(user). - Where("? = ?", bun.Ident("user.id"), user.ID). - Exec(ctx) + if len(columns) > 0 { + // If we're updating by column, ensure "updated_at" is included + columns = append(columns, "updated_at") + } + + // Update the user in DB + _, err := u.conn. + NewUpdate(). + Model(user). + Where("? = ?", bun.Ident("user.id"), user.ID). + Column(columns...). + Exec(ctx) + if err != nil { return u.conn.ProcessError(err) - }) + } + + // Invalidate in cache + u.cache.Invalidate("ID", user.ID) + return nil } func (u *userDB) DeleteUserByID(ctx context.Context, userID string) db.Error { diff --git a/internal/db/user.go b/internal/db/user.go index d01a8862a..1c6118fce 100644 --- a/internal/db/user.go +++ b/internal/db/user.go @@ -36,8 +36,8 @@ type User interface { GetUserByConfirmationToken(ctx context.Context, confirmationToken string) (*gtsmodel.User, Error) // PutUser will attempt to place user in the database PutUser(ctx context.Context, user *gtsmodel.User) Error - // UpdateUser updates one user by its primary key. - UpdateUser(ctx context.Context, user *gtsmodel.User) Error + // UpdateUser updates one user by its primary key, updating either only the specified columns, or all of them. + UpdateUser(ctx context.Context, user *gtsmodel.User, columns ...string) Error // DeleteUserByID deletes one user by its ID. DeleteUserByID(ctx context.Context, userID string) Error }