[bugfix] fix emoji recaching operations (#3167)

* add test for emoji update image * update emoji recache to set the instance account id * don't refresh emoji if only not cached. in that case literally just recache * code comment * rename + move a few things * add some more code comments, and rename some functions to make logic a bit clearer * remove unnecessary nil check (the value can be nil) * comment wording * remove test data output * handle the case of caching an emoji which has been refreshed then uncached * allow overwriting on testrig storage as we do now on regular storage * fix emoji category ID not getting updated --------- Co-authored-by: tobi <tobi.smethurst@protonmail.com>
2024-11-07 08:54:39 +01:00 · 2024-08-03 17:05:38 +00:00 · 2024-08-03 17:05:38 +00:00 · b85a9983d0
commit b85a9983d0
parent fa59c3713c
10 changed files with 305 additions and 91 deletions
--- a/internal/api/client/admin/emojiupdate_test.go
+++ b/internal/api/client/admin/emojiupdate_test.go
@ -21,9 +21,9 @@
 	"context"
 	"encoding/json"
 	"io"
-	"io/ioutil"
 	"net/http"
 	"net/http/httptest"
+	"strings"
 	"testing"

 	"github.com/stretchr/testify/suite"
@ -68,7 +68,7 @@ func (suite *EmojiUpdateTestSuite) TestEmojiUpdateNewCategory() {
 	defer result.Body.Close()

 	// check the response
-	b, err := ioutil.ReadAll(result.Body)
+	b, err := io.ReadAll(result.Body)
 	suite.NoError(err)
 	suite.NotEmpty(b)

@ -145,7 +145,7 @@ func (suite *EmojiUpdateTestSuite) TestEmojiUpdateSwitchCategory() {
 	defer result.Body.Close()

 	// check the response
-	b, err := ioutil.ReadAll(result.Body)
+	b, err := io.ReadAll(result.Body)
 	suite.NoError(err)
 	suite.NotEmpty(b)

@ -223,7 +223,7 @@ func (suite *EmojiUpdateTestSuite) TestEmojiUpdateCopyRemoteToLocal() {
 	defer result.Body.Close()

 	// check the response
-	b, err := ioutil.ReadAll(result.Body)
+	b, err := io.ReadAll(result.Body)
 	suite.NoError(err)
 	suite.NotEmpty(b)

@ -299,7 +299,7 @@ func (suite *EmojiUpdateTestSuite) TestEmojiUpdateDisableEmoji() {
 	defer result.Body.Close()

 	// check the response
-	b, err := ioutil.ReadAll(result.Body)
+	b, err := io.ReadAll(result.Body)
 	suite.NoError(err)
 	suite.NotEmpty(b)

@ -338,12 +338,97 @@ func (suite *EmojiUpdateTestSuite) TestEmojiUpdateDisableLocalEmoji() {
 	defer result.Body.Close()

 	// check the response
-	b, err := ioutil.ReadAll(result.Body)
+	b, err := io.ReadAll(result.Body)
 	suite.NoError(err)

 	suite.Equal(`{"error":"Bad Request: emoji 01F8MH9H8E4VG3KDYJR9EGPXCQ is not a remote emoji, cannot disable it via this endpoint"}`, string(b))
 }

+func (suite *EmojiUpdateTestSuite) TestEmojiUpdateModify() {
+	testEmoji := &gtsmodel.Emoji{}
+	*testEmoji = *suite.testEmojis["rainbow"]
+
+	// set up the request
+	requestBody, w, err := testrig.CreateMultipartFormData(
+		testrig.FileToDataF("image", "../../../../testrig/media/kip-original.gif"),
+		map[string][]string{
+			"type": {"modify"},
+		})
+	if err != nil {
+		panic(err)
+	}
+	bodyBytes := requestBody.Bytes()
+	recorder := httptest.NewRecorder()
+	ctx := suite.newContext(recorder, http.MethodPost, bodyBytes, admin.EmojiPathWithID, w.FormDataContentType())
+	ctx.AddParam(apiutil.IDKey, testEmoji.ID)
+
+	// call the handler
+	suite.adminModule.EmojiPATCHHandler(ctx)
+	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 := io.ReadAll(result.Body)
+	suite.NoError(err)
+	suite.NotEmpty(b)
+
+	// response should be an admin model emoji
+	adminEmoji := &apimodel.AdminEmoji{}
+	err = json.Unmarshal(b, adminEmoji)
+	suite.NoError(err)
+
+	// appropriate fields should be set
+	suite.Equal("rainbow", adminEmoji.Shortcode)
+	suite.NotEmpty(adminEmoji.URL)
+	suite.NotEmpty(adminEmoji.StaticURL)
+	suite.True(adminEmoji.VisibleInPicker)
+
+	// emoji should be in the db
+	dbEmoji, err := suite.db.GetEmojiByShortcodeDomain(context.Background(), adminEmoji.Shortcode, "")
+	suite.NoError(err)
+
+	// check fields on the emoji
+	suite.NotEmpty(dbEmoji.ID)
+	suite.Equal("rainbow", dbEmoji.Shortcode)
+	suite.Empty(dbEmoji.Domain)
+	suite.Empty(dbEmoji.ImageRemoteURL)
+	suite.Empty(dbEmoji.ImageStaticRemoteURL)
+	suite.Equal(adminEmoji.URL, dbEmoji.ImageURL)
+	suite.Equal(adminEmoji.StaticURL, dbEmoji.ImageStaticURL)
+
+	// Ensure image path as expected.
+	suite.NotEmpty(dbEmoji.ImagePath)
+	if !strings.HasPrefix(dbEmoji.ImagePath, suite.testAccounts["instance_account"].ID+"/emoji/original") {
+		suite.FailNow("", "image path %s not valid", dbEmoji.ImagePath)
+	}
+
+	// Ensure static image path as expected.
+	suite.NotEmpty(dbEmoji.ImageStaticPath)
+	if !strings.HasPrefix(dbEmoji.ImageStaticPath, suite.testAccounts["instance_account"].ID+"/emoji/static") {
+		suite.FailNow("", "image path %s not valid", dbEmoji.ImageStaticPath)
+	}
+
+	suite.Equal("image/gif", dbEmoji.ImageContentType)
+	suite.Equal("image/png", dbEmoji.ImageStaticContentType)
+	suite.Equal(1428, dbEmoji.ImageFileSize)
+	suite.Equal(1056, dbEmoji.ImageStaticFileSize)
+	suite.False(*dbEmoji.Disabled)
+	suite.NotEmpty(dbEmoji.URI)
+	suite.True(*dbEmoji.VisibleInPicker)
+	suite.NotEmpty(dbEmoji.CategoryID)
+
+	// emoji should be in storage
+	entry, err := suite.storage.Storage.Stat(ctx, dbEmoji.ImagePath)
+	suite.NoError(err)
+	suite.Equal(int64(dbEmoji.ImageFileSize), entry.Size)
+	entry, err = suite.storage.Storage.Stat(ctx, dbEmoji.ImageStaticPath)
+	suite.NoError(err)
+	suite.Equal(int64(dbEmoji.ImageStaticFileSize), entry.Size)
+}
+
 func (suite *EmojiUpdateTestSuite) TestEmojiUpdateModifyRemoteEmoji() {
 	testEmoji := &gtsmodel.Emoji{}
 	*testEmoji = *suite.testEmojis["yell"]
@ -404,7 +489,7 @@ func (suite *EmojiUpdateTestSuite) TestEmojiUpdateModifyNoParams() {
 	defer result.Body.Close()

 	// check the response
-	b, err := ioutil.ReadAll(result.Body)
+	b, err := io.ReadAll(result.Body)
 	suite.NoError(err)

 	suite.Equal(`{"error":"Bad Request: emoji action type was 'modify' but no image or category name was provided"}`, string(b))
@ -438,7 +523,7 @@ func (suite *EmojiUpdateTestSuite) TestEmojiUpdateCopyLocalToLocal() {
 	defer result.Body.Close()

 	// check the response
-	b, err := ioutil.ReadAll(result.Body)
+	b, err := io.ReadAll(result.Body)
 	suite.NoError(err)

 	suite.Equal(`{"error":"Bad Request: target emoji is not remote; cannot copy to local"}`, string(b))
@ -472,7 +557,7 @@ func (suite *EmojiUpdateTestSuite) TestEmojiUpdateCopyEmptyShortcode() {
 	defer result.Body.Close()

 	// check the response
-	b, err := ioutil.ReadAll(result.Body)
+	b, err := io.ReadAll(result.Body)
 	suite.NoError(err)

 	suite.Equal(`{"error":"Bad Request: shortcode  did not pass validation, must be between 2 and 30 characters, letters, numbers, and underscores only"}`, string(b))
@ -505,7 +590,7 @@ func (suite *EmojiUpdateTestSuite) TestEmojiUpdateCopyNoShortcode() {
 	defer result.Body.Close()

 	// check the response
-	b, err := ioutil.ReadAll(result.Body)
+	b, err := io.ReadAll(result.Body)
 	suite.NoError(err)

 	suite.Equal(`{"error":"Bad Request: emoji action type was 'copy' but no shortcode was provided"}`, string(b))
@ -539,7 +624,7 @@ func (suite *EmojiUpdateTestSuite) TestEmojiUpdateCopyShortcodeAlreadyInUse() {
 	defer result.Body.Close()

 	// check the response
-	b, err := ioutil.ReadAll(result.Body)
+	b, err := io.ReadAll(result.Body)
 	suite.NoError(err)

 	suite.Equal(`{"error":"Conflict: emoji with shortcode already exists"}`, string(b))
--- a/internal/cleaner/media_test.go
+++ b/internal/cleaner/media_test.go
@ -386,7 +386,7 @@ func (suite *MediaTestSuite) TestUncacheAndRecache() {
 		testStatusAttachment,
 		testHeader,
 	} {
-		processing := suite.manager.RecacheMedia(original, data)
+		processing := suite.manager.CacheMedia(original, data)

 		// synchronously load the recached attachment
 		recachedAttachment, err := processing.Load(ctx)
--- a/internal/federation/dereferencing/emoji.go
+++ b/internal/federation/dereferencing/emoji.go
@ -134,11 +134,6 @@ func (d *Dereferencer) RefreshEmoji(
 	*gtsmodel.Emoji,
 	error,
 ) {
-	// Can't refresh local.
-	if emoji.IsLocal() {
-		return emoji, nil
-	}
-
 	// Check emoji is up-to-date
 	// with provided extra info.
 	switch {
@ -156,8 +151,18 @@ func (d *Dereferencer) RefreshEmoji(
 		force = true
 	}

-	// Check if needs updating.
-	if *emoji.Cached && !force {
+	// Check if needs
+	// force refresh.
+	if !force {
+
+		// We still want to make sure
+		// the emoji is cached. Simply
+		// check whether emoji is cached.
+		return d.RecacheEmoji(ctx, emoji)
+	}
+
+	// Can't refresh local.
+	if emoji.IsLocal() {
 		return emoji, nil
 	}

@ -191,8 +196,8 @@ func() (*media.ProcessingEmoji, error) {
 				return tsport.DereferenceMedia(ctx, url, int64(maxsz))
 			}

-			// Refresh emoji with prepared info.
-			return d.mediaManager.RefreshEmoji(ctx,
+			// Update emoji with prepared info.
+			return d.mediaManager.UpdateEmoji(ctx,
 				emoji,
 				data,
 				info,
@ -201,6 +206,72 @@ func() (*media.ProcessingEmoji, error) {
 	)
 }

+// RecacheEmoji handles the simplest case which is that
+// of an existing emoji that only needs to be recached.
+// It handles the case of both local emojis, and those
+// already cached as no-ops.
+//
+// Please note that even if an error is returned,
+// an emoji model may still be returned if the error
+// was only encountered during actual dereferencing.
+// In this case, it will act as a placeholder.
+func (d *Dereferencer) RecacheEmoji(
+	ctx context.Context,
+	emoji *gtsmodel.Emoji,
+) (
+	*gtsmodel.Emoji,
+	error,
+) {
+	// Can't recache local.
+	if emoji.IsLocal() {
+		return emoji, nil
+	}
+
+	if *emoji.Cached {
+		// Already cached.
+		return emoji, nil
+	}
+
+	// Get shortcode domain for locks + logging.
+	shortcodeDomain := emoji.ShortcodeDomain()
+
+	// Ensure we have a valid image remote URL.
+	url, err := url.Parse(emoji.ImageRemoteURL)
+	if err != nil {
+		err := gtserror.Newf("invalid image remote url %s for emoji %s: %w", emoji.ImageRemoteURL, shortcodeDomain, err)
+		return nil, err
+	}
+
+	// Pass along for safe processing.
+	return d.processEmojiSafely(ctx,
+		shortcodeDomain,
+		func() (*media.ProcessingEmoji, error) {
+
+			// Acquire new instance account transport for emoji dereferencing.
+			tsport, err := d.transportController.NewTransportForUsername(ctx, "")
+			if err != nil {
+				err := gtserror.Newf("error getting instance transport: %w", err)
+				return nil, err
+			}
+
+			// Get maximum supported remote emoji size.
+			maxsz := config.GetMediaEmojiRemoteMaxSize()
+
+			// Prepare data function to dereference remote emoji media.
+			data := func(context.Context) (io.ReadCloser, error) {
+				return tsport.DereferenceMedia(ctx, url, int64(maxsz))
+			}
+
+			// Recache emoji with prepared info.
+			return d.mediaManager.CacheEmoji(ctx,
+				emoji,
+				data,
+			)
+		},
+	)
+
+}
+
 // processingEmojiSafely provides concurrency-safe processing of
 // an emoji with given shortcode+domain. if a copy of the emoji is
 // not already being processed, the given 'process' callback will
--- a/internal/federation/dereferencing/media.go
+++ b/internal/federation/dereferencing/media.go
@ -172,7 +172,7 @@ func() (*media.ProcessingMedia, error) {

 			// Recache media with prepared info,
 			// this will also update media in db.
-			return d.mediaManager.RecacheMedia(
+			return d.mediaManager.CacheMedia(
 				attach,
 				func(ctx context.Context) (io.ReadCloser, error) {
 					return tsport.DereferenceMedia(ctx, url, int64(maxsz))
--- a/internal/media/manager.go
+++ b/internal/media/manager.go
@ -20,6 +20,7 @@
 import (
 	"context"
 	"io"
+	"strings"
 	"time"

 	"codeberg.org/gruf/go-iotools"
@ -161,14 +162,14 @@ func (m *Manager) CreateMedia(
 	}

 	// Pass prepared media as ready to be cached.
-	return m.RecacheMedia(attachment, data), nil
+	return m.CacheMedia(attachment, data), nil
 }

-// RecacheMedia wraps a media model (assumed already
+// CacheMedia wraps a media model (assumed already
 // inserted in the database!) with given data function
 // to perform a blocking dereference / decode operation
 // from the data stream returned.
-func (m *Manager) RecacheMedia(
+func (m *Manager) CacheMedia(
 	media *gtsmodel.MediaAttachment,
 	data DataFunc,
 ) *ProcessingMedia {
@ -220,7 +221,7 @@ func (m *Manager) CreateEmoji(
 	}

 	// Finally, create new emoji.
-	return m.createEmoji(ctx,
+	return m.createOrUpdateEmoji(ctx,
 		m.state.DB.PutEmoji,
 		data,
 		emoji,
@ -228,12 +229,14 @@ func (m *Manager) CreateEmoji(
 	)
 }

-// RefreshEmoji will prepare a recache operation
-// for the given emoji, updating it with extra
-// information, and in particular using new storage
-// paths for the dereferenced media files to skirt
-// around browser caching of the old files.
-func (m *Manager) RefreshEmoji(
+// UpdateEmoji prepares an update operation for the given emoji,
+// which is assumed to already exist in the database.
+//
+// Calling load on the returned *ProcessingEmoji will update the
+// db entry with provided extra information, ensure emoji images
+// are cached, and use new storage paths for the dereferenced media
+// files to skirt around browser caching of the old files.
+func (m *Manager) UpdateEmoji(
 	ctx context.Context,
 	emoji *gtsmodel.Emoji,
 	data DataFunc,
@ -289,8 +292,8 @@ func (m *Manager) RefreshEmoji(
 		return rct, nil
 	}

-	// Finally, create new emoji in database.
-	processingEmoji, err := m.createEmoji(ctx,
+	// Update existing emoji in database.
+	processingEmoji, err := m.createOrUpdateEmoji(ctx,
 		func(ctx context.Context, emoji *gtsmodel.Emoji) error {
 			return m.state.DB.UpdateEmoji(ctx, emoji)
 		},
@ -308,9 +311,49 @@ func(ctx context.Context, emoji *gtsmodel.Emoji) error {
 	return processingEmoji, nil
 }

-func (m *Manager) createEmoji(
+// CacheEmoji wraps an emoji model (assumed already
+// inserted in the database!) with given data function
+// to perform a blocking dereference / decode operation
+// from the data stream returned.
+func (m *Manager) CacheEmoji(
 	ctx context.Context,
-	putDB func(context.Context, *gtsmodel.Emoji) error,
+	emoji *gtsmodel.Emoji,
+	data DataFunc,
+) (
+	*ProcessingEmoji,
+	error,
+) {
+	// Fetch the local instance account for emoji path generation.
+	instanceAcc, err := m.state.DB.GetInstanceAccount(ctx, "")
+	if err != nil {
+		return nil, gtserror.Newf("error fetching instance account: %w", err)
+	}
+
+	var pathID string
+
+	// Look for an emoji path ID that differs from its actual ID, this indicates
+	// a previous 'refresh'. We need to be sure to set this on the ProcessingEmoji{}
+	// so it knows to store the emoji under this path, and not default to emoji.ID.
+	if id := extractEmojiPathID(emoji.ImagePath); id != emoji.ID {
+		pathID = id
+	}
+
+	return &ProcessingEmoji{
+		newPathID: pathID,
+		instAccID: instanceAcc.ID,
+		emoji:     emoji,
+		dataFn:    data,
+		mgr:       m,
+	}, nil
+}
+
+// createOrUpdateEmoji updates the emoji according to
+// provided additional data, and performs the actual
+// database write, finally returning an emoji ready
+// for processing (i.e. caching to local storage).
+func (m *Manager) createOrUpdateEmoji(
+	ctx context.Context,
+	storeDB func(context.Context, *gtsmodel.Emoji) error,
 	data DataFunc,
 	emoji *gtsmodel.Emoji,
 	info AdditionalEmojiInfo,
@ -351,8 +394,8 @@ func (m *Manager) createEmoji(
 		emoji.CategoryID = *info.CategoryID
 	}

-	// Store emoji in database in initial form.
-	if err := putDB(ctx, emoji); err != nil {
+	// Put or update emoji in database.
+	if err := storeDB(ctx, emoji); err != nil {
 		return nil, err
 	}

@ -367,17 +410,26 @@ func (m *Manager) createEmoji(
 	return processingEmoji, nil
 }

-// RecacheEmoji wraps an emoji model (assumed already
-// inserted in the database!) with given data function
-// to perform a blocking dereference / decode operation
-// from the data stream returned.
-func (m *Manager) RecacheEmoji(
-	emoji *gtsmodel.Emoji,
-	data DataFunc,
-) *ProcessingEmoji {
-	return &ProcessingEmoji{
-		emoji:  emoji,
-		dataFn: data,
-		mgr:    m,
+// extractEmojiPathID pulls the ID used in the final path segment of an emoji path (can be URL).
+func extractEmojiPathID(path string) string {
+	// Look for '.' indicating file ext.
+	i := strings.LastIndexByte(path, '.')
+	if i == -1 {
+		return ""
 	}
+
+	// Strip ext.
+	path = path[:i]
+
+	// Look for '/' of final path sep.
+	i = strings.LastIndexByte(path, '/')
+	if i == -1 {
+		return ""
+	}
+
+	// Strip up to
+	// final segment.
+	path = path[i+1:]
+
+	return path
 }
--- a/internal/media/manager_test.go
+++ b/internal/media/manager_test.go
@ -105,7 +105,7 @@ func (suite *ManagerTestSuite) TestEmojiProcessRefresh() {
 		return io.NopCloser(bytes.NewBuffer(b)), nil
 	}

-	processing, err := suite.manager.RefreshEmoji(ctx,
+	processing, err := suite.manager.UpdateEmoji(ctx,
 		emojiToUpdate,
 		data,
 		media.AdditionalEmojiInfo{
--- a/internal/media/refetch.go
+++ b/internal/media/refetch.go
@ -115,7 +115,7 @@ func (m *Manager) RefetchEmojis(ctx context.Context, domain string, dereferenceM
 			return dereferenceMedia(ctx, emojiImageIRI, int64(maxsz))
 		}

-		processingEmoji, err := m.RefreshEmoji(ctx, emoji, dataFunc, AdditionalEmojiInfo{
+		processingEmoji, err := m.UpdateEmoji(ctx, emoji, dataFunc, AdditionalEmojiInfo{
 			Domain:               &emoji.Domain,
 			ImageRemoteURL:       &emoji.ImageRemoteURL,
 			ImageStaticRemoteURL: &emoji.ImageStaticRemoteURL,
@ -123,16 +123,16 @@ func (m *Manager) RefetchEmojis(ctx context.Context, domain string, dereferenceM
 			VisibleInPicker:      emoji.VisibleInPicker,
 		})
 		if err != nil {
-			log.Errorf(ctx, "emoji %s could not be refreshed because of an error during processing: %s", shortcodeDomain, err)
+			log.Errorf(ctx, "emoji %s could not be updated because of an error during processing: %s", shortcodeDomain, err)
 			continue
 		}

 		if _, err := processingEmoji.Load(ctx); err != nil {
-			log.Errorf(ctx, "emoji %s could not be refreshed because of an error during loading: %s", shortcodeDomain, err)
+			log.Errorf(ctx, "emoji %s could not be updated because of an error during loading: %s", shortcodeDomain, err)
 			continue
 		}

-		log.Tracef(ctx, "refetched emoji %s successfully from remote", shortcodeDomain)
+		log.Tracef(ctx, "refetched + updated emoji %s successfully from remote", shortcodeDomain)
 		totalRefetched++
 	}

--- a/internal/processing/admin/emoji.go
+++ b/internal/processing/admin/emoji.go
@ -291,13 +291,8 @@ func (p *Processor) emojiUpdateCopy(
 	}

 	// Ensure target emoji is locally cached.
-	target, err := p.federator.RefreshEmoji(
-		ctx,
+	target, err := p.federator.RecacheEmoji(ctx,
 		target,
-
-		// no changes we want to make.
-		media.AdditionalEmojiInfo{},
-		false,
 	)
 	if err != nil {
 		err := gtserror.Newf("error recaching emoji %s: %w", target.ImageRemoteURL, err)
@ -325,8 +320,8 @@ func (p *Processor) emojiUpdateCopy(

 	// Attempt to create the new local emoji.
 	emoji, errWithCode := p.createEmoji(ctx,
-		util.PtrOrValue(shortcode, ""),
-		util.PtrOrValue(categoryName, ""),
+		util.PtrOrZero(shortcode),
+		util.PtrOrZero(categoryName),
 		data,
 	)
 	if errWithCode != nil {
@ -401,35 +396,39 @@ func (p *Processor) emojiUpdateModify(
 		return nil, gtserror.NewErrorBadRequest(errors.New(text), text)
 	}

-	if categoryName != nil {
-		if *categoryName != "" {
-			// A category was provided, get / create relevant emoji category.
-			category, errWithCode := p.mustGetEmojiCategory(ctx, *categoryName)
-			if errWithCode != nil {
-				return nil, errWithCode
-			}
+	// Check if we need to
+	// set a new category ID.
+	var newCategoryID *string
+	switch {
+	case categoryName == nil:
+		// No changes.

-			if category.ID == emoji.CategoryID {
-				// There was no change,
-				// indicate this by unsetting
-				// the category name pointer.
-				categoryName = nil
-			} else {
-				// Update emoji category.
-				emoji.CategoryID = category.ID
-				emoji.Category = category
-			}
-		} else {
-			// Emoji category was unset.
-			emoji.CategoryID = ""
-			emoji.Category = nil
+	case *categoryName == "":
+		// Emoji category was unset.
+		newCategoryID = util.Ptr("")
+		emoji.CategoryID = ""
+		emoji.Category = nil
+
+	case *categoryName != "":
+		// A category was provided, get or create relevant emoji category.
+		category, errWithCode := p.mustGetEmojiCategory(ctx, *categoryName)
+		if errWithCode != nil {
+			return nil, errWithCode
+		}
+
+		// Update emoji category if
+		// it's different from before.
+		if category.ID != emoji.CategoryID {
+			newCategoryID = &category.ID
+			emoji.CategoryID = category.ID
+			emoji.Category = category
 		}
 	}

 	// Check whether any image changes were requested.
 	imageUpdated := (image != nil && image.Size > 0)

-	if !imageUpdated && categoryName != nil {
+	if !imageUpdated && newCategoryID != nil {
 		// Only updating category; only a single database update required.
 		if err := p.state.DB.UpdateEmoji(ctx, emoji, "category_id"); err != nil {
 			err := gtserror.Newf("error updating emoji in db: %w", err)
@ -463,8 +462,17 @@ func (p *Processor) emojiUpdateModify(
 			return rc, nil
 		}

-		// Prepare emoji model for recache from new data.
-		processing := p.media.RecacheEmoji(emoji, data)
+		// Include category ID
+		// update if necessary.
+		ai := media.AdditionalEmojiInfo{}
+		ai.CategoryID = newCategoryID
+
+		// Prepare emoji model for update+recache from new data.
+		processing, err := p.media.UpdateEmoji(ctx, emoji, data, ai)
+		if err != nil {
+			err := gtserror.Newf("error preparing recache: %w", err)
+			return nil, gtserror.NewErrorInternalError(err)
+		}

 		// Load to trigger update + write.
 		emoji, err = processing.Load(ctx)
--- a/internal/processing/media/getfile.go
+++ b/internal/processing/media/getfile.go
@ -246,11 +246,9 @@ func (p *Processor) getEmojiContent(

 	// Ensure that stored emoji is cached.
 	// (this handles local emoji / recaches).
-	emoji, err = p.federator.RefreshEmoji(
+	emoji, err = p.federator.RecacheEmoji(
 		ctx,
 		emoji,
-		media.AdditionalEmojiInfo{},
-		false,
 	)
 	if err != nil {
 		err := gtserror.Newf("error recaching emoji: %w", err)
--- a/testrig/storage.go
+++ b/testrig/storage.go
@ -29,7 +29,7 @@

 // NewInMemoryStorage returns a new in memory storage with the default test config
 func NewInMemoryStorage() *gtsstorage.Driver {
-	storage := memory.Open(200, false)
+	storage := memory.Open(200, true)
 	return &gtsstorage.Driver{
 		Storage: storage,
 	}