gtsocial-umbx

commit bfccf4e450d22d9a3fc0c58d94ed728207d305e0
parent e6cde25466e03ec864cd9defed96957e741b5e7c
Author: tobi <31960611+tsmethurst@users.noreply.github.com>
Date:   Thu,  2 Mar 2023 16:58:23 +0100

[bugfix] add ON CONFLICT statements to status updates (#1580)


Diffstat:
M
internal/db/bundb/status.go
 | 
18
+++++++++++++-----
M
internal/db/bundb/status_test.go
 | 
31
++++++++++++++++++++++++++++++-

2 files changed, 43 insertions(+), 6 deletions(-)
diff --git a/internal/db/bundb/status.go b/internal/db/bundb/status.go
@@ -200,7 +200,9 @@ func (s *statusDB) PutStatus(ctx context.Context, status *gtsmodel.Status) db.Er
 					Model(&gtsmodel.StatusToEmoji{
 						StatusID: status.ID,
 						EmojiID:  i,
-					}).Exec(ctx); err != nil {
+					}).
+					On("CONFLICT (?, ?) DO NOTHING", bun.Ident("status_id"), bun.Ident("emoji_id")).
+					Exec(ctx); err != nil {
 					err = s.conn.ProcessError(err)
 					if !errors.Is(err, db.ErrAlreadyExists) {
 						return err
@@ -215,7 +217,9 @@ func (s *statusDB) PutStatus(ctx context.Context, status *gtsmodel.Status) db.Er
 					Model(&gtsmodel.StatusToTag{
 						StatusID: status.ID,
 						TagID:    i,
-					}).Exec(ctx); err != nil {
+					}).
+					On("CONFLICT (?, ?) DO NOTHING", bun.Ident("status_id"), bun.Ident("tag_id")).
+					Exec(ctx); err != nil {
 					err = s.conn.ProcessError(err)
 					if !errors.Is(err, db.ErrAlreadyExists) {
 						return err
@@ -261,7 +265,9 @@ func (s *statusDB) UpdateStatus(ctx context.Context, status *gtsmodel.Status, co
 				Model(&gtsmodel.StatusToEmoji{
 					StatusID: status.ID,
 					EmojiID:  i,
-				}).Exec(ctx); err != nil {
+				}).
+				On("CONFLICT (?, ?) DO NOTHING", bun.Ident("status_id"), bun.Ident("emoji_id")).
+				Exec(ctx); err != nil {
 				err = s.conn.ProcessError(err)
 				if !errors.Is(err, db.ErrAlreadyExists) {
 					return err
@@ -276,7 +282,9 @@ func (s *statusDB) UpdateStatus(ctx context.Context, status *gtsmodel.Status, co
 				Model(&gtsmodel.StatusToTag{
 					StatusID: status.ID,
 					TagID:    i,
-				}).Exec(ctx); err != nil {
+				}).
+				On("CONFLICT (?, ?) DO NOTHING", bun.Ident("status_id"), bun.Ident("tag_id")).
+				Exec(ctx); err != nil {
 				err = s.conn.ProcessError(err)
 				if !errors.Is(err, db.ErrAlreadyExists) {
 					return err
@@ -300,7 +308,7 @@ func (s *statusDB) UpdateStatus(ctx context.Context, status *gtsmodel.Status, co
 			}
 		}
 
-		// Finally, insert the status
+		// Finally, update the status
 		_, err := tx.
 			NewUpdate().
 			Model(status).
diff --git a/internal/db/bundb/status_test.go b/internal/db/bundb/status_test.go
@@ -26,6 +26,7 @@ import (
 
 	"github.com/stretchr/testify/suite"
 	"github.com/superseriousbusiness/gotosocial/internal/db"
+	"github.com/superseriousbusiness/gotosocial/internal/gtsmodel"
 )
 
 type StatusTestSuite struct {
@@ -176,7 +177,10 @@ func (suite *StatusTestSuite) TestGetStatusChildren() {
 }
 
 func (suite *StatusTestSuite) TestDeleteStatus() {
-	targetStatus := suite.testStatuses["admin_account_status_1"]
+	// Take a copy of the status.
+	targetStatus := &gtsmodel.Status{}
+	*targetStatus = *suite.testStatuses["admin_account_status_1"]
+
 	err := suite.db.DeleteStatusByID(context.Background(), targetStatus.ID)
 	suite.NoError(err)
 
@@ -184,6 +188,31 @@ func (suite *StatusTestSuite) TestDeleteStatus() {
 	suite.ErrorIs(err, db.ErrNoEntries)
 }
 
+// This test was added specifically to ensure that Postgres wasn't getting upset
+// about trying to use a transaction in which an error has already occurred, which
+// was previously leading to errors like 'current transaction is aborted, commands
+// ignored until end of transaction block' when updating a status that already had
+// emojis or tags set on it.
+//
+// To run this test for postgres specifically, start a postgres container on localhost
+// and then run:
+//
+// GTS_DB_TYPE=postgres GTS_DB_ADDRESS=localhost go test ./internal/db/bundb -run '^TestStatusTestSuite$' -testify.m '^(TestUpdateStatus)$' github.com/superseriousbusiness/gotosocial/internal/db/bundb
+func (suite *StatusTestSuite) TestUpdateStatus() {
+	// Take a copy of the status.
+	targetStatus := &gtsmodel.Status{}
+	*targetStatus = *suite.testStatuses["admin_account_status_1"]
+
+	targetStatus.PinnedAt = time.Time{}
+
+	err := suite.db.UpdateStatus(context.Background(), targetStatus, "pinned_at")
+	suite.NoError(err)
+
+	updated, err := suite.db.GetStatusByID(context.Background(), targetStatus.ID)
+	suite.NoError(err)
+	suite.True(updated.PinnedAt.IsZero())
+}
+
 func TestStatusTestSuite(t *testing.T) {
 	suite.Run(t, new(StatusTestSuite))
 }