From aa8bbe6ad2e3908bd55bd6522524784cd4383ae2 Mon Sep 17 00:00:00 2001 From: tobi <31960611+tsmethurst@users.noreply.github.com> Date: Mon, 29 Jan 2024 15:57:22 +0100 Subject: [PATCH] [bugfix] Fix Postgres emoji delete, emoji category change (#2570) * [bugfix] Fix Postgres emoji delete, emoji category change * revert trace logging * caching issue * update tests --- internal/api/client/admin/emojicreate.go | 2 +- internal/api/client/admin/emojidelete_test.go | 2 +- internal/api/client/admin/emojiget.go | 2 +- internal/api/client/admin/emojisget.go | 12 +- internal/api/client/admin/emojiupdate_test.go | 8 +- internal/db/bundb/emoji.go | 54 +- internal/db/bundb/emoji_test.go | 11 + internal/gtsmodel/emoji.go | 7 + internal/media/processingemoji.go | 2 +- internal/media/refetch.go | 2 +- internal/processing/admin/admin_test.go | 2 + internal/processing/admin/emoji.go | 558 +++++++++++------- internal/processing/admin/emoji_test.go | 67 +++ internal/processing/fedi/emoji.go | 2 +- internal/typeutils/internaltofrontend.go | 2 +- 15 files changed, 500 insertions(+), 233 deletions(-) create mode 100644 internal/processing/admin/emoji_test.go diff --git a/internal/api/client/admin/emojicreate.go b/internal/api/client/admin/emojicreate.go index d916a76c1..5d024f039 100644 --- a/internal/api/client/admin/emojicreate.go +++ b/internal/api/client/admin/emojicreate.go @@ -125,7 +125,7 @@ func (m *Module) EmojiCreatePOSTHandler(c *gin.Context) { return } - apiEmoji, errWithCode := m.processor.Admin().EmojiCreate(c.Request.Context(), authed.Account, authed.User, form) + apiEmoji, errWithCode := m.processor.Admin().EmojiCreate(c.Request.Context(), authed.Account, form) if errWithCode != nil { apiutil.ErrorHandler(c, errWithCode, m.processor.InstanceGetV1) return diff --git a/internal/api/client/admin/emojidelete_test.go b/internal/api/client/admin/emojidelete_test.go index 551054036..e27592baf 100644 --- a/internal/api/client/admin/emojidelete_test.go +++ b/internal/api/client/admin/emojidelete_test.go @@ -87,7 +87,7 @@ func (suite *EmojiDeleteTestSuite) TestEmojiDelete2() { suite.NoError(err) suite.NotNil(b) - suite.Equal(`{"error":"Bad Request: EmojiDelete: emoji with id 01GD5KP5CQEE1R3X43Y1EHS2CW was not a local emoji, will not delete"}`, string(b)) + suite.Equal(`{"error":"Bad Request: emoji with id 01GD5KP5CQEE1R3X43Y1EHS2CW was not a local emoji, will not delete"}`, string(b)) // emoji should still be in the db dbEmoji, err := suite.db.GetEmojiByID(context.Background(), testEmoji.ID) diff --git a/internal/api/client/admin/emojiget.go b/internal/api/client/admin/emojiget.go index 710094551..730d92db1 100644 --- a/internal/api/client/admin/emojiget.go +++ b/internal/api/client/admin/emojiget.go @@ -89,7 +89,7 @@ func (m *Module) EmojiGETHandler(c *gin.Context) { return } - emoji, errWithCode := m.processor.Admin().EmojiGet(c.Request.Context(), authed.Account, authed.User, emojiID) + emoji, errWithCode := m.processor.Admin().EmojiGet(c.Request.Context(), authed.Account, emojiID) if errWithCode != nil { apiutil.ErrorHandler(c, errWithCode, m.processor.InstanceGetV1) return diff --git a/internal/api/client/admin/emojisget.go b/internal/api/client/admin/emojisget.go index 212401117..4013e1836 100644 --- a/internal/api/client/admin/emojisget.go +++ b/internal/api/client/admin/emojisget.go @@ -197,7 +197,17 @@ func (m *Module) EmojisGETHandler(c *gin.Context) { includeEnabled = true } - resp, errWithCode := m.processor.Admin().EmojisGet(c.Request.Context(), authed.Account, authed.User, domain, includeDisabled, includeEnabled, shortcode, maxShortcodeDomain, minShortcodeDomain, limit) + resp, errWithCode := m.processor.Admin().EmojisGet( + c.Request.Context(), + authed.Account, + domain, + includeDisabled, + includeEnabled, + shortcode, + maxShortcodeDomain, + minShortcodeDomain, + limit, + ) if errWithCode != nil { apiutil.ErrorHandler(c, errWithCode, m.processor.InstanceGetV1) return diff --git a/internal/api/client/admin/emojiupdate_test.go b/internal/api/client/admin/emojiupdate_test.go index 35aeb08ed..eb9d9d866 100644 --- a/internal/api/client/admin/emojiupdate_test.go +++ b/internal/api/client/admin/emojiupdate_test.go @@ -339,7 +339,7 @@ func (suite *EmojiUpdateTestSuite) TestEmojiUpdateDisableLocalEmoji() { b, err := ioutil.ReadAll(result.Body) suite.NoError(err) - suite.Equal(`{"error":"Bad Request: emojiUpdateDisable: emoji 01F8MH9H8E4VG3KDYJR9EGPXCQ is not a remote emoji, cannot disable it via this endpoint"}`, string(b)) + suite.Equal(`{"error":"Bad Request: emoji 01F8MH9H8E4VG3KDYJR9EGPXCQ is not a remote emoji, cannot disable it via this endpoint"}`, string(b)) } func (suite *EmojiUpdateTestSuite) TestEmojiUpdateModifyRemoteEmoji() { @@ -372,7 +372,7 @@ func (suite *EmojiUpdateTestSuite) TestEmojiUpdateModifyRemoteEmoji() { b, err := ioutil.ReadAll(result.Body) suite.NoError(err) - suite.Equal(`{"error":"Bad Request: emojiUpdateModify: emoji 01GD5KP5CQEE1R3X43Y1EHS2CW is not a local emoji, cannot do a modify action on it"}`, string(b)) + suite.Equal(`{"error":"Bad Request: emoji 01GD5KP5CQEE1R3X43Y1EHS2CW is not a local emoji, cannot update it via this endpoint"}`, string(b)) } func (suite *EmojiUpdateTestSuite) TestEmojiUpdateModifyNoParams() { @@ -439,7 +439,7 @@ func (suite *EmojiUpdateTestSuite) TestEmojiUpdateCopyLocalToLocal() { b, err := ioutil.ReadAll(result.Body) suite.NoError(err) - suite.Equal(`{"error":"Bad Request: emojiUpdateCopy: emoji 01F8MH9H8E4VG3KDYJR9EGPXCQ is not a remote emoji, cannot copy it to local"}`, string(b)) + suite.Equal(`{"error":"Bad Request: emoji 01F8MH9H8E4VG3KDYJR9EGPXCQ is not a remote emoji, cannot copy it to local"}`, string(b)) } func (suite *EmojiUpdateTestSuite) TestEmojiUpdateCopyEmptyShortcode() { @@ -540,7 +540,7 @@ func (suite *EmojiUpdateTestSuite) TestEmojiUpdateCopyShortcodeAlreadyInUse() { b, err := ioutil.ReadAll(result.Body) suite.NoError(err) - suite.Equal(`{"error":"Conflict: emojiUpdateCopy: emoji 01GD5KP5CQEE1R3X43Y1EHS2CW could not be copied, emoji with shortcode rainbow already exists on this instance"}`, string(b)) + suite.Equal(`{"error":"Conflict: emoji with shortcode rainbow already exists on this instance"}`, string(b)) } func TestEmojiUpdateTestSuite(t *testing.T) { diff --git a/internal/db/bundb/emoji.go b/internal/db/bundb/emoji.go index 31092d0d2..d1cb9dfbd 100644 --- a/internal/db/bundb/emoji.go +++ b/internal/db/bundb/emoji.go @@ -132,28 +132,32 @@ func (e *emojiDB) DeleteEmojiByID(ctx context.Context, id string) error { } for _, statusID := range statusIDs { - var emojiIDs []string + status := new(gtsmodel.Status) - // Select statuses with ID. - if _, err := tx.NewSelect(). - Table("statuses"). + // Select status emoji IDs. + if err := tx.NewSelect(). + Model(status). Column("emojis"). Where("? = ?", bun.Ident("id"), statusID). - Exec(ctx); err != nil && + Scan(ctx); err != nil && err != sql.ErrNoRows { return err } - // Delete all instances of this emoji ID from status emojis. - emojiIDs = slices.DeleteFunc(emojiIDs, func(emojiID string) bool { - return emojiID == id - }) + // Delete all instances of this + // emoji ID from status emoji IDs. + status.EmojiIDs = slices.DeleteFunc( + status.EmojiIDs, + func(emojiID string) bool { + return emojiID == id + }, + ) // Update status emoji IDs. if _, err := tx.NewUpdate(). - Table("statuses"). + Model(status). Where("? = ?", bun.Ident("id"), statusID). - Set("emojis = ?", emojiIDs). + Column("emojis"). Exec(ctx); err != nil && err != sql.ErrNoRows { return err @@ -161,35 +165,39 @@ func (e *emojiDB) DeleteEmojiByID(ctx context.Context, id string) error { } for _, accountID := range accountIDs { - var emojiIDs []string + account := new(gtsmodel.Account) - // Select account with ID. - if _, err := tx.NewSelect(). - Table("accounts"). + // Select account emoji IDs. + if err := tx.NewSelect(). + Model(account). Column("emojis"). Where("? = ?", bun.Ident("id"), accountID). - Exec(ctx); err != nil && + Scan(ctx); err != nil && err != sql.ErrNoRows { return err } - // Delete all instances of this emoji ID from account emojis. - emojiIDs = slices.DeleteFunc(emojiIDs, func(emojiID string) bool { - return emojiID == id - }) + // Delete all instances of this + // emoji ID from account emoji IDs. + account.EmojiIDs = slices.DeleteFunc( + account.EmojiIDs, + func(emojiID string) bool { + return emojiID == id + }, + ) // Update account emoji IDs. if _, err := tx.NewUpdate(). - Table("accounts"). + Model(account). Where("? = ?", bun.Ident("id"), accountID). - Set("emojis = ?", emojiIDs). + Column("emojis"). Exec(ctx); err != nil && err != sql.ErrNoRows { return err } } - // Delete emoji from database. + // Finally, delete emoji from database. if _, err := tx.NewDelete(). Table("emojis"). Where("? = ?", bun.Ident("id"), id). diff --git a/internal/db/bundb/emoji_test.go b/internal/db/bundb/emoji_test.go index f75334d90..a4bf18f6e 100644 --- a/internal/db/bundb/emoji_test.go +++ b/internal/db/bundb/emoji_test.go @@ -23,6 +23,7 @@ import ( "github.com/stretchr/testify/suite" "github.com/superseriousbusiness/gotosocial/internal/db" + "github.com/superseriousbusiness/gotosocial/internal/gtsmodel" "github.com/superseriousbusiness/gotosocial/testrig" ) @@ -160,6 +161,16 @@ func (suite *EmojiTestSuite) TestGetEmojiCategory() { suite.NotNil(category) } +func (suite *EmojiTestSuite) TestUpdateEmojiCategory() { + testEmoji := new(gtsmodel.Emoji) + *testEmoji = *suite.testEmojis["rainbow"] + + testEmoji.CategoryID = "" + + err := suite.db.UpdateEmoji(context.Background(), testEmoji, "category_id") + suite.NoError(err) +} + func TestEmojiTestSuite(t *testing.T) { suite.Run(t, new(EmojiTestSuite)) } diff --git a/internal/gtsmodel/emoji.go b/internal/gtsmodel/emoji.go index 596a64110..b377b1440 100644 --- a/internal/gtsmodel/emoji.go +++ b/internal/gtsmodel/emoji.go @@ -44,3 +44,10 @@ type Emoji struct { CategoryID string `bun:"type:CHAR(26),nullzero"` // ID of the category this emoji belongs to. Cached *bool `bun:",nullzero,notnull,default:false"` } + +// IsLocal returns true if the emoji is +// local to this instance., ie., it did +// not originate from a remote instance. +func (e *Emoji) IsLocal() bool { + return e.Domain == "" +} diff --git a/internal/media/processingemoji.go b/internal/media/processingemoji.go index 9287f14ce..dfe7c1883 100644 --- a/internal/media/processingemoji.go +++ b/internal/media/processingemoji.go @@ -158,7 +158,7 @@ func (p *ProcessingEmoji) store(ctx context.Context) error { var maxSize bytesize.Size - if p.emoji.Domain == "" { + if p.emoji.IsLocal() { // this is a local emoji upload maxSize = config.GetMediaEmojiLocalMaxSize() } else { diff --git a/internal/media/refetch.go b/internal/media/refetch.go index 8e5188178..a1483ccd4 100644 --- a/internal/media/refetch.go +++ b/internal/media/refetch.go @@ -61,7 +61,7 @@ func (m *Manager) RefetchEmojis(ctx context.Context, domain string, dereferenceM } for _, emoji := range emojis { - if emoji.Domain == "" { + if emoji.IsLocal() { // never try to refetch local emojis continue } diff --git a/internal/processing/admin/admin_test.go b/internal/processing/admin/admin_test.go index 01a3a88ff..1bd355b3d 100644 --- a/internal/processing/admin/admin_test.go +++ b/internal/processing/admin/admin_test.go @@ -62,6 +62,7 @@ type AdminStandardTestSuite struct { testFollows map[string]*gtsmodel.Follow testAttachments map[string]*gtsmodel.MediaAttachment testStatuses map[string]*gtsmodel.Status + testEmojis map[string]*gtsmodel.Emoji // module being tested adminProcessor *admin.Processor @@ -76,6 +77,7 @@ func (suite *AdminStandardTestSuite) SetupSuite() { suite.testFollows = testrig.NewTestFollows() suite.testAttachments = testrig.NewTestAttachments() suite.testStatuses = testrig.NewTestStatuses() + suite.testEmojis = testrig.NewTestEmojis() } func (suite *AdminStandardTestSuite) SetupTest() { diff --git a/internal/processing/admin/emoji.go b/internal/processing/admin/emoji.go index 689aad9dc..dcdf77642 100644 --- a/internal/processing/admin/emoji.go +++ b/internal/processing/admin/emoji.go @@ -36,37 +36,39 @@ import ( ) // EmojiCreate creates a custom emoji on this instance. -func (p *Processor) EmojiCreate(ctx context.Context, account *gtsmodel.Account, user *gtsmodel.User, form *apimodel.EmojiCreateRequest) (*apimodel.Emoji, gtserror.WithCode) { - if !*user.Admin { - return nil, gtserror.NewErrorUnauthorized(fmt.Errorf("user %s not an admin", user.ID), "user is not an admin") - } - +func (p *Processor) EmojiCreate( + ctx context.Context, + account *gtsmodel.Account, + form *apimodel.EmojiCreateRequest, +) (*apimodel.Emoji, gtserror.WithCode) { + // Ensure emoji with this shortcode + // doesn't already exist on the instance. maybeExisting, err := p.state.DB.GetEmojiByShortcodeDomain(ctx, form.Shortcode, "") + if err != nil && !errors.Is(err, db.ErrNoEntries) { + err := gtserror.Newf("error checking existence of emoji with shortcode %s: %w", form.Shortcode, err) + return nil, gtserror.NewErrorInternalError(err) + } + if maybeExisting != nil { - return nil, gtserror.NewErrorConflict(fmt.Errorf("emoji with shortcode %s already exists", form.Shortcode), fmt.Sprintf("emoji with shortcode %s already exists", form.Shortcode)) + err := fmt.Errorf("emoji with shortcode %s already exists", form.Shortcode) + return nil, gtserror.NewErrorConflict(err, err.Error()) } - if err != nil && err != db.ErrNoEntries { - return nil, gtserror.NewErrorInternalError(fmt.Errorf("error checking existence of emoji with shortcode %s: %s", form.Shortcode, err)) - } - - emojiID, err := id.NewRandomULID() - if err != nil { - return nil, gtserror.NewErrorInternalError(fmt.Errorf("error creating id for new emoji: %s", err), "error creating emoji ID") - } - - emojiURI := uris.URIForEmoji(emojiID) - + // Prepare data function for emoji processing + // (just read data from the submitted form). data := func(innerCtx context.Context) (io.ReadCloser, int64, error) { f, err := form.Image.Open() return f, form.Image.Size, err } + // If category was supplied on the form, + // ensure the category exists and provide + // it as additional info to emoji processing. var ai *media.AdditionalEmojiInfo if form.CategoryName != "" { category, err := p.getOrCreateEmojiCategory(ctx, form.CategoryName) if err != nil { - return nil, gtserror.NewErrorInternalError(fmt.Errorf("error putting id in category: %s", err), "error putting id in category") + return nil, gtserror.NewErrorInternalError(err) } ai = &media.AdditionalEmojiInfo{ @@ -74,73 +76,68 @@ func (p *Processor) EmojiCreate(ctx context.Context, account *gtsmodel.Account, } } - processingEmoji, err := p.mediaManager.PreProcessEmoji(ctx, data, form.Shortcode, emojiID, emojiURI, ai, false) + // Generate new emoji ID and URI. + emojiID, err := id.NewRandomULID() if err != nil { - return nil, gtserror.NewErrorInternalError(fmt.Errorf("error processing emoji: %s", err), "error processing emoji") + err := gtserror.Newf("error creating id for new emoji: %w", err) + return nil, gtserror.NewErrorInternalError(err) } + emojiURI := uris.URIForEmoji(emojiID) + + // Begin media processing. + processingEmoji, err := p.mediaManager.PreProcessEmoji(ctx, + data, form.Shortcode, emojiID, emojiURI, ai, false, + ) + if err != nil { + err := gtserror.Newf("error processing emoji: %w", err) + return nil, gtserror.NewErrorInternalError(err) + } + + // Complete processing immediately. emoji, err := processingEmoji.LoadEmoji(ctx) if err != nil { - return nil, gtserror.NewErrorInternalError(fmt.Errorf("error loading emoji: %s", err), "error loading emoji") + err := gtserror.Newf("error loading emoji: %w", err) + return nil, gtserror.NewErrorInternalError(err) } apiEmoji, err := p.converter.EmojiToAPIEmoji(ctx, emoji) if err != nil { - return nil, gtserror.NewErrorInternalError(fmt.Errorf("error converting emoji: %s", err), "error converting emoji to api representation") + err := gtserror.Newf("error converting emoji: %w", err) + return nil, gtserror.NewErrorInternalError(err) } return &apiEmoji, nil } -// EmojisGet returns an admin view of custom emojis, filtered with the given parameters. -func (p *Processor) EmojisGet( - ctx context.Context, - account *gtsmodel.Account, - user *gtsmodel.User, +// emojisGetFilterParams builds extra +// query parameters to return as part +// of an Emojis pageable response. +// +// The returned string will look like: +// +// "filter=domain:all,enabled,shortcode:example" +func emojisGetFilterParams( + shortcode string, domain string, includeDisabled bool, includeEnabled bool, - shortcode string, - maxShortcodeDomain string, - minShortcodeDomain string, - limit int, -) (*apimodel.PageableResponse, gtserror.WithCode) { - if !*user.Admin { - return nil, gtserror.NewErrorUnauthorized(fmt.Errorf("user %s not an admin", user.ID), "user is not an admin") - } - - emojis, err := p.state.DB.GetEmojisBy(ctx, domain, includeDisabled, includeEnabled, shortcode, maxShortcodeDomain, minShortcodeDomain, limit) - if err != nil && !errors.Is(err, db.ErrNoEntries) { - err := fmt.Errorf("EmojisGet: db error: %s", err) - return nil, gtserror.NewErrorInternalError(err) - } - - count := len(emojis) - if count == 0 { - return util.EmptyPageableResponse(), nil - } - - items := make([]interface{}, 0, count) - for _, emoji := range emojis { - adminEmoji, err := p.converter.EmojiToAdminAPIEmoji(ctx, emoji) - if err != nil { - err := fmt.Errorf("EmojisGet: error converting emoji to admin model emoji: %s", err) - return nil, gtserror.NewErrorInternalError(err) - } - items = append(items, adminEmoji) - } - - filterBuilder := strings.Builder{} +) string { + var filterBuilder strings.Builder filterBuilder.WriteString("filter=") switch domain { case "", "local": + // Local emojis only. filterBuilder.WriteString("domain:local") + case db.EmojiAllDomains: + // Local or remote. filterBuilder.WriteString("domain:all") + default: - filterBuilder.WriteString("domain:") - filterBuilder.WriteString(domain) + // Specific domain only. + filterBuilder.WriteString("domain:" + domain) } if includeDisabled != includeEnabled { @@ -153,108 +150,182 @@ func (p *Processor) EmojisGet( } if shortcode != "" { - filterBuilder.WriteString(",shortcode:") - filterBuilder.WriteString(shortcode) + // Specific shortcode only. + filterBuilder.WriteString(",shortcode:" + shortcode) + } + + return filterBuilder.String() +} + +// EmojisGet returns an admin view of custom +// emojis, filtered with the given parameters. +func (p *Processor) EmojisGet( + ctx context.Context, + account *gtsmodel.Account, + domain string, + includeDisabled bool, + includeEnabled bool, + shortcode string, + maxShortcodeDomain string, + minShortcodeDomain string, + limit int, +) (*apimodel.PageableResponse, gtserror.WithCode) { + emojis, err := p.state.DB.GetEmojisBy(ctx, + domain, + includeDisabled, + includeEnabled, + shortcode, + maxShortcodeDomain, + minShortcodeDomain, + limit, + ) + if err != nil && !errors.Is(err, db.ErrNoEntries) { + err := gtserror.Newf("db error: %w", err) + return nil, gtserror.NewErrorInternalError(err) + } + + count := len(emojis) + if count == 0 { + return util.EmptyPageableResponse(), nil + } + + items := make([]interface{}, 0, count) + for _, emoji := range emojis { + adminEmoji, err := p.converter.EmojiToAdminAPIEmoji(ctx, emoji) + if err != nil { + err := gtserror.Newf("error converting emoji to admin model emoji: %w", err) + return nil, gtserror.NewErrorInternalError(err) + } + items = append(items, adminEmoji) } return util.PackagePageableResponse(util.PageableResponseParams{ - Items: items, - Path: "api/v1/admin/custom_emojis", - NextMaxIDKey: "max_shortcode_domain", - NextMaxIDValue: util.ShortcodeDomain(emojis[count-1]), - PrevMinIDKey: "min_shortcode_domain", - PrevMinIDValue: util.ShortcodeDomain(emojis[0]), - Limit: limit, - ExtraQueryParams: []string{filterBuilder.String()}, + Items: items, + Path: "api/v1/admin/custom_emojis", + NextMaxIDKey: "max_shortcode_domain", + NextMaxIDValue: util.ShortcodeDomain(emojis[count-1]), + PrevMinIDKey: "min_shortcode_domain", + PrevMinIDValue: util.ShortcodeDomain(emojis[0]), + Limit: limit, + ExtraQueryParams: []string{ + emojisGetFilterParams( + shortcode, + domain, + includeDisabled, + includeEnabled, + ), + }, }) } -// EmojiGet returns the admin view of one custom emoji with the given id. -func (p *Processor) EmojiGet(ctx context.Context, account *gtsmodel.Account, user *gtsmodel.User, id string) (*apimodel.AdminEmoji, gtserror.WithCode) { - if !*user.Admin { - return nil, gtserror.NewErrorUnauthorized(fmt.Errorf("user %s not an admin", user.ID), "user is not an admin") +// EmojiGet returns the admin view of +// one custom emoji with the given id. +func (p *Processor) EmojiGet( + ctx context.Context, + account *gtsmodel.Account, + id string, +) (*apimodel.AdminEmoji, gtserror.WithCode) { + emoji, err := p.state.DB.GetEmojiByID(ctx, id) + if err != nil && !errors.Is(err, db.ErrNoEntries) { + err := gtserror.Newf("db error: %w", err) + return nil, gtserror.NewErrorInternalError(err) } - emoji, err := p.state.DB.GetEmojiByID(ctx, id) - if err != nil { - if errors.Is(err, db.ErrNoEntries) { - err = fmt.Errorf("EmojiGet: no emoji with id %s found in the db", id) - return nil, gtserror.NewErrorNotFound(err) - } - err := fmt.Errorf("EmojiGet: db error: %s", err) - return nil, gtserror.NewErrorInternalError(err) + if emoji == nil { + err := gtserror.Newf("no emoji with id %s found in the db", id) + return nil, gtserror.NewErrorNotFound(err) } adminEmoji, err := p.converter.EmojiToAdminAPIEmoji(ctx, emoji) if err != nil { - err = fmt.Errorf("EmojiGet: error converting emoji to admin api emoji: %s", err) + err := gtserror.Newf("error converting emoji to admin api emoji: %w", err) return nil, gtserror.NewErrorInternalError(err) } return adminEmoji, nil } -// EmojiDelete deletes one emoji from the database, with the given id. -func (p *Processor) EmojiDelete(ctx context.Context, id string) (*apimodel.AdminEmoji, gtserror.WithCode) { +// EmojiDelete deletes one *local* emoji +// from the database, with the given id. +func (p *Processor) EmojiDelete( + ctx context.Context, + id string, +) (*apimodel.AdminEmoji, gtserror.WithCode) { emoji, err := p.state.DB.GetEmojiByID(ctx, id) - if err != nil { - if errors.Is(err, db.ErrNoEntries) { - err = fmt.Errorf("EmojiDelete: no emoji with id %s found in the db", id) - return nil, gtserror.NewErrorNotFound(err) - } - err := fmt.Errorf("EmojiDelete: db error: %s", err) + if err != nil && !errors.Is(err, db.ErrNoEntries) { + err := gtserror.Newf("db error: %w", err) return nil, gtserror.NewErrorInternalError(err) } - if emoji.Domain != "" { - err = fmt.Errorf("EmojiDelete: emoji with id %s was not a local emoji, will not delete", id) + if emoji == nil { + err := gtserror.Newf("no emoji with id %s found in the db", id) + return nil, gtserror.NewErrorNotFound(err) + } + + if !emoji.IsLocal() { + err := fmt.Errorf("emoji with id %s was not a local emoji, will not delete", id) return nil, gtserror.NewErrorBadRequest(err, err.Error()) } + // Convert to admin emoji before deletion, + // so we can return the deleted emoji. adminEmoji, err := p.converter.EmojiToAdminAPIEmoji(ctx, emoji) if err != nil { - err = fmt.Errorf("EmojiDelete: error converting emoji to admin api emoji: %s", err) + err := gtserror.Newf("error converting emoji to admin api emoji: %w", err) return nil, gtserror.NewErrorInternalError(err) } if err := p.state.DB.DeleteEmojiByID(ctx, id); err != nil { - err := fmt.Errorf("EmojiDelete: db error: %s", err) + err := gtserror.Newf("db error deleting emoji %s: %w", id, err) return nil, gtserror.NewErrorInternalError(err) } return adminEmoji, nil } -// EmojiUpdate updates one emoji with the given id, using the provided form parameters. -func (p *Processor) EmojiUpdate(ctx context.Context, id string, form *apimodel.EmojiUpdateRequest) (*apimodel.AdminEmoji, gtserror.WithCode) { +// EmojiUpdate updates one emoji with the +// given id, using the provided form parameters. +func (p *Processor) EmojiUpdate( + ctx context.Context, + id string, + form *apimodel.EmojiUpdateRequest, +) (*apimodel.AdminEmoji, gtserror.WithCode) { emoji, err := p.state.DB.GetEmojiByID(ctx, id) - if err != nil { - if errors.Is(err, db.ErrNoEntries) { - err = fmt.Errorf("EmojiUpdate: no emoji with id %s found in the db", id) - return nil, gtserror.NewErrorNotFound(err) - } - err := fmt.Errorf("EmojiUpdate: db error: %s", err) + if err != nil && !errors.Is(err, db.ErrNoEntries) { + err := gtserror.Newf("db error: %w", err) return nil, gtserror.NewErrorInternalError(err) } - switch form.Type { + if emoji == nil { + err := gtserror.Newf("no emoji with id %s found in the db", id) + return nil, gtserror.NewErrorNotFound(err) + } + + switch t := form.Type; t { + case apimodel.EmojiUpdateCopy: return p.emojiUpdateCopy(ctx, emoji, form.Shortcode, form.CategoryName) + case apimodel.EmojiUpdateDisable: return p.emojiUpdateDisable(ctx, emoji) + case apimodel.EmojiUpdateModify: return p.emojiUpdateModify(ctx, emoji, form.Image, form.CategoryName) + default: - err := errors.New("unrecognized emoji action type") + err := fmt.Errorf("unrecognized emoji action type %s", t) return nil, gtserror.NewErrorBadRequest(err, err.Error()) } } -// EmojiCategoriesGet returns all custom emoji categories that exist on this instance. -func (p *Processor) EmojiCategoriesGet(ctx context.Context) ([]*apimodel.EmojiCategory, gtserror.WithCode) { +// EmojiCategoriesGet returns all custom emoji +// categories that exist on this instance. +func (p *Processor) EmojiCategoriesGet( + ctx context.Context, +) ([]*apimodel.EmojiCategory, gtserror.WithCode) { categories, err := p.state.DB.GetEmojiCategories(ctx) if err != nil { - err := fmt.Errorf("EmojiCategoriesGet: db error: %s", err) + err := gtserror.Newf("db error getting emoji categories: %w", err) return nil, gtserror.NewErrorInternalError(err) } @@ -262,7 +333,7 @@ func (p *Processor) EmojiCategoriesGet(ctx context.Context) ([]*apimodel.EmojiCa for _, category := range categories { apiCategory, err := p.converter.EmojiCategoryToAPIEmojiCategory(ctx, category) if err != nil { - err := fmt.Errorf("EmojiCategoriesGet: error converting emoji category to api emoji category: %s", err) + err := gtserror.Newf("error converting emoji category to api emoji category: %w", err) return nil, gtserror.NewErrorInternalError(err) } apiCategories = append(apiCategories, apiCategory) @@ -275,22 +346,35 @@ func (p *Processor) EmojiCategoriesGet(ctx context.Context) ([]*apimodel.EmojiCa UTIL FUNCTIONS */ -func (p *Processor) getOrCreateEmojiCategory(ctx context.Context, name string) (*gtsmodel.EmojiCategory, error) { +// getOrCreateEmojiCategory either gets an existing +// category with the given name from the database, +// or, if the category doesn't yet exist, it creates +// the category and then returns it. +func (p *Processor) getOrCreateEmojiCategory( + ctx context.Context, + name string, +) (*gtsmodel.EmojiCategory, error) { category, err := p.state.DB.GetEmojiCategoryByName(ctx, name) - if err == nil { + if err != nil && !errors.Is(err, db.ErrNoEntries) { + return nil, gtserror.Newf( + "database error trying get emoji category %s: %w", + name, err, + ) + } + + if category != nil { + // We had it already. return category, nil } - if err != nil && !errors.Is(err, db.ErrNoEntries) { - err = fmt.Errorf("GetOrCreateEmojiCategory: database error trying get emoji category by name: %s", err) - return nil, err - } - - // we don't have the category yet, just create it with the given name + // We don't have the category yet, + // create it with the given name. categoryID, err := id.NewRandomULID() if err != nil { - err = fmt.Errorf("GetOrCreateEmojiCategory: error generating id for new emoji category: %s", err) - return nil, err + return nil, gtserror.Newf( + "error generating id for new emoji category %s: %w", + name, err, + ) } category = >smodel.EmojiCategory{ @@ -299,54 +383,85 @@ func (p *Processor) getOrCreateEmojiCategory(ctx context.Context, name string) ( } if err := p.state.DB.PutEmojiCategory(ctx, category); err != nil { - err = fmt.Errorf("GetOrCreateEmojiCategory: error putting new emoji category in the database: %s", err) - return nil, err + return nil, gtserror.Newf( + "db error putting new emoji category %s: %w", + name, err, + ) } return category, nil } -// copy an emoji from remote to local -func (p *Processor) emojiUpdateCopy(ctx context.Context, emoji *gtsmodel.Emoji, shortcode *string, categoryName *string) (*apimodel.AdminEmoji, gtserror.WithCode) { - if emoji.Domain == "" { - err := fmt.Errorf("emojiUpdateCopy: emoji %s is not a remote emoji, cannot copy it to local", emoji.ID) +// emojiUpdateCopy copies and stores the given +// *remote* emoji as a *local* emoji, preserving +// the same image, and using the provided shortcode. +// +// The provided emoji model must correspond to an +// emoji already stored in the database + storage. +func (p *Processor) emojiUpdateCopy( + ctx context.Context, + targetEmoji *gtsmodel.Emoji, + shortcode *string, + category *string, +) (*apimodel.AdminEmoji, gtserror.WithCode) { + if targetEmoji.IsLocal() { + err := fmt.Errorf("emoji %s is not a remote emoji, cannot copy it to local", targetEmoji.ID) return nil, gtserror.NewErrorBadRequest(err, err.Error()) } if shortcode == nil { - err := fmt.Errorf("emojiUpdateCopy: emoji %s could not be copied, no shortcode provided", emoji.ID) + err := errors.New("no shortcode provided") return nil, gtserror.NewErrorBadRequest(err, err.Error()) } - maybeExisting, err := p.state.DB.GetEmojiByShortcodeDomain(ctx, *shortcode, "") + sc := *shortcode + if sc == "" { + err := errors.New("empty shortcode provided") + return nil, gtserror.NewErrorBadRequest(err, err.Error()) + } + + // Ensure we don't already have an emoji + // stored locally with this shortcode. + maybeExisting, err := p.state.DB.GetEmojiByShortcodeDomain(ctx, sc, "") + if err != nil && !errors.Is(err, db.ErrNoEntries) { + err := gtserror.Newf("db error checking for emoji with shortcode %s: %w", sc, err) + return nil, gtserror.NewErrorInternalError(err) + } + if maybeExisting != nil { - err := fmt.Errorf("emojiUpdateCopy: emoji %s could not be copied, emoji with shortcode %s already exists on this instance", emoji.ID, *shortcode) + err := fmt.Errorf("emoji with shortcode %s already exists on this instance", sc) return nil, gtserror.NewErrorConflict(err, err.Error()) } - if err != nil && err != db.ErrNoEntries { - err := fmt.Errorf("emojiUpdateCopy: emoji %s could not be copied, error checking existence of emoji with shortcode %s: %s", emoji.ID, *shortcode, err) - return nil, gtserror.NewErrorInternalError(err) + // We don't have an emoji with this + // shortcode yet! Prepare to create it. + + // Data function for copying just streams media + // out of storage into an additional location. + // + // This means that data for the copy persists even + // if the remote copied emoji gets deleted at some point. + data := func(ctx context.Context) (io.ReadCloser, int64, error) { + rc, err := p.state.Storage.GetStream(ctx, targetEmoji.ImagePath) + return rc, int64(targetEmoji.ImageFileSize), err } - newEmojiID, err := id.NewRandomULID() + // Generate new emoji ID and URI. + emojiID, err := id.NewRandomULID() if err != nil { - err := fmt.Errorf("emojiUpdateCopy: emoji %s could not be copied, error creating id for new emoji: %s", emoji.ID, err) + err := gtserror.Newf("error creating id for new emoji: %w", err) return nil, gtserror.NewErrorInternalError(err) } - newEmojiURI := uris.URIForEmoji(newEmojiID) - - data := func(ctx context.Context) (reader io.ReadCloser, fileSize int64, err error) { - rc, err := p.state.Storage.GetStream(ctx, emoji.ImagePath) - return rc, int64(emoji.ImageFileSize), err - } + emojiURI := uris.URIForEmoji(emojiID) + // If category was supplied, ensure the + // category exists and provide it as + // additional info to emoji processing. var ai *media.AdditionalEmojiInfo - if categoryName != nil { - category, err := p.getOrCreateEmojiCategory(ctx, *categoryName) + if category != nil && *category != "" { + category, err := p.getOrCreateEmojiCategory(ctx, *category) if err != nil { - err = fmt.Errorf("emojiUpdateCopy: error getting or creating category: %s", err) return nil, gtserror.NewErrorInternalError(err) } @@ -355,126 +470,173 @@ func (p *Processor) emojiUpdateCopy(ctx context.Context, emoji *gtsmodel.Emoji, } } - processingEmoji, err := p.mediaManager.PreProcessEmoji(ctx, data, *shortcode, newEmojiID, newEmojiURI, ai, false) + // Begin media processing. + processingEmoji, err := p.mediaManager.PreProcessEmoji(ctx, + data, sc, emojiID, emojiURI, ai, false, + ) if err != nil { - err = fmt.Errorf("emojiUpdateCopy: error processing emoji %s: %s", emoji.ID, err) + err := gtserror.Newf("error processing emoji: %w", err) return nil, gtserror.NewErrorInternalError(err) } + // Complete processing immediately. newEmoji, err := processingEmoji.LoadEmoji(ctx) if err != nil { - err = fmt.Errorf("emojiUpdateCopy: error loading processed emoji %s: %s", emoji.ID, err) + err := gtserror.Newf("error loading emoji: %w", err) return nil, gtserror.NewErrorInternalError(err) } adminEmoji, err := p.converter.EmojiToAdminAPIEmoji(ctx, newEmoji) if err != nil { - err = fmt.Errorf("emojiUpdateCopy: error converting updated emoji %s to admin emoji: %s", emoji.ID, err) + err := gtserror.Newf("error converting emoji %s to admin emoji: %w", newEmoji.ID, err) return nil, gtserror.NewErrorInternalError(err) } return adminEmoji, nil } -// disable a remote emoji -func (p *Processor) emojiUpdateDisable(ctx context.Context, emoji *gtsmodel.Emoji) (*apimodel.AdminEmoji, gtserror.WithCode) { - if emoji.Domain == "" { - err := fmt.Errorf("emojiUpdateDisable: emoji %s is not a remote emoji, cannot disable it via this endpoint", emoji.ID) +// emojiUpdateDisable marks the given *remote* +// emoji as disabled by setting disabled = true. +// +// The provided emoji model must correspond to an +// emoji already stored in the database + storage. +func (p *Processor) emojiUpdateDisable( + ctx context.Context, + emoji *gtsmodel.Emoji, +) (*apimodel.AdminEmoji, gtserror.WithCode) { + if emoji.IsLocal() { + err := fmt.Errorf("emoji %s is not a remote emoji, cannot disable it via this endpoint", emoji.ID) return nil, gtserror.NewErrorBadRequest(err, err.Error()) } - emojiDisabled := true - emoji.Disabled = &emojiDisabled - err := p.state.DB.UpdateEmoji(ctx, emoji, "disabled") - if err != nil { - err = fmt.Errorf("emojiUpdateDisable: error updating emoji %s: %s", emoji.ID, err) - return nil, gtserror.NewErrorInternalError(err) + // Only bother with a db call + // if emoji not already disabled. + if !*emoji.Disabled { + emoji.Disabled = util.Ptr(true) + if err := p.state.DB.UpdateEmoji(ctx, emoji, "disabled"); err != nil { + err := gtserror.Newf("db error updating emoji %s: %w", emoji.ID, err) + return nil, gtserror.NewErrorInternalError(err) + } } adminEmoji, err := p.converter.EmojiToAdminAPIEmoji(ctx, emoji) if err != nil { - err = fmt.Errorf("emojiUpdateDisable: error converting updated emoji %s to admin emoji: %s", emoji.ID, err) + err := gtserror.Newf("error converting emoji %s to admin emoji: %w", emoji.ID, err) return nil, gtserror.NewErrorInternalError(err) } return adminEmoji, nil } -// modify a local emoji -func (p *Processor) emojiUpdateModify(ctx context.Context, emoji *gtsmodel.Emoji, image *multipart.FileHeader, categoryName *string) (*apimodel.AdminEmoji, gtserror.WithCode) { - if emoji.Domain != "" { - err := fmt.Errorf("emojiUpdateModify: emoji %s is not a local emoji, cannot do a modify action on it", emoji.ID) +// emojiUpdateModify modifies the given *local* emoji. +// +// Either one of image or category must be non-nil, +// otherwise there's nothing to modify. If category +// is non-nil and dereferences to an empty string, +// category will be cleared. +// +// The provided emoji model must correspond to an +// emoji already stored in the database + storage. +func (p *Processor) emojiUpdateModify( + ctx context.Context, + emoji *gtsmodel.Emoji, + image *multipart.FileHeader, + category *string, +) (*apimodel.AdminEmoji, gtserror.WithCode) { + if !emoji.IsLocal() { + err := fmt.Errorf("emoji %s is not a local emoji, cannot update it via this endpoint", emoji.ID) return nil, gtserror.NewErrorBadRequest(err, err.Error()) } - // keep existing categoryID unless a new one is defined - var ( - updatedCategoryID = emoji.CategoryID - updateCategoryID bool - ) - if categoryName != nil { - category, err := p.getOrCreateEmojiCategory(ctx, *categoryName) - if err != nil { - err = fmt.Errorf("emojiUpdateModify: error getting or creating category: %s", err) - return nil, gtserror.NewErrorInternalError(err) - } - - updatedCategoryID = category.ID - updateCategoryID = true + // Ensure there's actually something to update. + if image == nil && category == nil { + err := errors.New("neither new category nor new image set, cannot update") + return nil, gtserror.NewErrorBadRequest(err, err.Error()) } - // only update image if provided with one + // Only update category + // if it's changed. + var ( + newCategory *gtsmodel.EmojiCategory + newCategoryID string + updateCategoryID bool + ) + + if category != nil { + catName := *category + if catName != "" { + // Set new category. + var err error + newCategory, err = p.getOrCreateEmojiCategory(ctx, catName) + if err != nil { + err := gtserror.Newf("error getting or creating category: %w", err) + return nil, gtserror.NewErrorInternalError(err) + } + + newCategoryID = newCategory.ID + } else { + // Clear existing category. + newCategoryID = "" + } + + updateCategoryID = emoji.CategoryID != newCategoryID + } + + // Only update image + // if one is provided. var updateImage bool if image != nil && image.Size != 0 { updateImage = true } - if !updateImage { - // only updating fields, we only need - // to do a database update for this - var columns []string - - if updateCategoryID { - emoji.CategoryID = updatedCategoryID - columns = append(columns, "category_id") - } - - var err error - err = p.state.DB.UpdateEmoji(ctx, emoji, columns...) - if err != nil { - err = fmt.Errorf("emojiUpdateModify: error updating emoji %s: %s", emoji.ID, err) + if updateCategoryID && !updateImage { + // Only updating category; we only + // need to do a db update for this. + emoji.CategoryID = newCategoryID + emoji.Category = newCategory + if err := p.state.DB.UpdateEmoji(ctx, emoji, "category_id"); err != nil { + err := gtserror.Newf("db error updating emoji %s: %w", emoji.ID, err) return nil, gtserror.NewErrorInternalError(err) } - } else { - // new image, so we need to reprocess the emoji - data := func(ctx context.Context) (reader io.ReadCloser, fileSize int64, err error) { + } else if updateImage { + // Updating image and maybe categoryID. + // We can do both at the same time :) + + // Set data function to provided image. + data := func(ctx context.Context) (io.ReadCloser, int64, error) { i, err := image.Open() return i, image.Size, err } + // If necessary, include + // update to categoryID too. var ai *media.AdditionalEmojiInfo if updateCategoryID { ai = &media.AdditionalEmojiInfo{ - CategoryID: &updatedCategoryID, + CategoryID: &newCategoryID, } } - processingEmoji, err := p.mediaManager.PreProcessEmoji(ctx, data, emoji.Shortcode, emoji.ID, emoji.URI, ai, true) + // Begin media processing. + processingEmoji, err := p.mediaManager.PreProcessEmoji(ctx, + data, emoji.Shortcode, emoji.ID, emoji.URI, ai, false, + ) if err != nil { - err = fmt.Errorf("emojiUpdateModify: error processing emoji %s: %s", emoji.ID, err) + err := gtserror.Newf("error processing emoji: %w", err) return nil, gtserror.NewErrorInternalError(err) } + // Replace emoji ptr with newly-processed version. emoji, err = processingEmoji.LoadEmoji(ctx) if err != nil { - err = fmt.Errorf("emojiUpdateModify: error loading processed emoji %s: %s", emoji.ID, err) + err := gtserror.Newf("error loading emoji: %w", err) return nil, gtserror.NewErrorInternalError(err) } } adminEmoji, err := p.converter.EmojiToAdminAPIEmoji(ctx, emoji) if err != nil { - err = fmt.Errorf("emojiUpdateModify: error converting updated emoji %s to admin emoji: %s", emoji.ID, err) + err := gtserror.Newf("error converting emoji %s to admin emoji: %w", emoji.ID, err) return nil, gtserror.NewErrorInternalError(err) } diff --git a/internal/processing/admin/emoji_test.go b/internal/processing/admin/emoji_test.go new file mode 100644 index 000000000..17f5fc864 --- /dev/null +++ b/internal/processing/admin/emoji_test.go @@ -0,0 +1,67 @@ +// 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 admin_test + +import ( + "context" + "testing" + + "github.com/stretchr/testify/suite" + apimodel "github.com/superseriousbusiness/gotosocial/internal/api/model" + "github.com/superseriousbusiness/gotosocial/internal/gtsmodel" + "github.com/superseriousbusiness/gotosocial/internal/util" +) + +type EmojiTestSuite struct { + AdminStandardTestSuite +} + +func (suite *EmojiTestSuite) TestUpdateEmojiCategory() { + ctx := context.Background() + testEmoji := new(gtsmodel.Emoji) + *testEmoji = *suite.testEmojis["rainbow"] + + // Toggle the emoji category around. + for _, categoryName := range []string{ + "", + "newCategory", + "newCategory", + "newCategory2", + "", + "reactions", + "", + "", + } { + emoji, err := suite.adminProcessor.EmojiUpdate(ctx, + testEmoji.ID, + &apimodel.EmojiUpdateRequest{ + Type: apimodel.EmojiUpdateModify, + CategoryName: util.Ptr(categoryName), + }, + ) + if err != nil { + suite.FailNow(err.Error()) + } + + suite.Equal(categoryName, emoji.Category) + } +} + +func TestEmojiTestSuite(t *testing.T) { + suite.Run(t, new(EmojiTestSuite)) +} diff --git a/internal/processing/fedi/emoji.go b/internal/processing/fedi/emoji.go index 4acf8c671..9ac0ea244 100644 --- a/internal/processing/fedi/emoji.go +++ b/internal/processing/fedi/emoji.go @@ -36,7 +36,7 @@ func (p *Processor) EmojiGet(ctx context.Context, requestedEmojiID string) (inte return nil, gtserror.NewErrorNotFound(fmt.Errorf("database error getting emoji with id %s: %s", requestedEmojiID, err)) } - if requestedEmoji.Domain != "" { + if !requestedEmoji.IsLocal() { return nil, gtserror.NewErrorNotFound(fmt.Errorf("emoji with id %s doesn't belong to this instance (domain %s)", requestedEmojiID, requestedEmoji.Domain)) } diff --git a/internal/typeutils/internaltofrontend.go b/internal/typeutils/internaltofrontend.go index ed2979049..f92f0bb65 100644 --- a/internal/typeutils/internaltofrontend.go +++ b/internal/typeutils/internaltofrontend.go @@ -612,7 +612,7 @@ func (c *Converter) EmojiToAdminAPIEmoji(ctx context.Context, e *gtsmodel.Emoji) return nil, err } - if e.Domain != "" { + if !e.IsLocal() { // Domain may be in Punycode, // de-punify it just in case. var err error