emoji code passing muster

internal/api/client/admin/emojicreate.go

@@ -73,6 +73,8 @@ import (
 //      description: forbidden
 //   '400':
 //      description: bad request
+//   '409':
+//      description: conflict -- domain/shortcode combo for emoji already exists
 func (m *Module) EmojiCreatePOSTHandler(c *gin.Context) {
 	l := logrus.WithFields(logrus.Fields{
 		"func":        "emojiCreatePOSTHandler",
@@ -116,10 +118,10 @@ func (m *Module) EmojiCreatePOSTHandler(c *gin.Context) {
 		return
 	}
 
-	apiEmoji, err := m.processor.AdminEmojiCreate(c.Request.Context(), authed, form)
+	apiEmoji, errWithCode := m.processor.AdminEmojiCreate(c.Request.Context(), authed, form)
-	if err != nil {
+	if errWithCode != nil {
-		l.Debugf("error creating emoji: %s", err)
+		l.Debugf("error creating emoji: %s", errWithCode.Error())
-		c.JSON(http.StatusBadRequest, gin.H{"error": err.Error()})
+		c.JSON(errWithCode.Code(), gin.H{"error": errWithCode.Safe()})
 		return
 	}
 

internal/api/client/admin/emojicreate_test.go

@@ -25,7 +25,7 @@ func (suite *EmojiCreateTestSuite) TestEmojiCreate() {
 	requestBody, w, err := testrig.CreateMultipartFormData(
 		"image", "../../../../testrig/media/rainbow-original.png",
 		map[string]string{
-			"shortcode": "rainbow",
+			"shortcode": "new_emoji",
 		})
 	if err != nil {
 		panic(err)
@@ -55,24 +55,24 @@ func (suite *EmojiCreateTestSuite) TestEmojiCreate() {
 	suite.NoError(err)
 
 	// appropriate fields should be set
-	suite.Equal("rainbow", apiEmoji.Shortcode)
+	suite.Equal("new_emoji", apiEmoji.Shortcode)
 	suite.NotEmpty(apiEmoji.URL)
 	suite.NotEmpty(apiEmoji.StaticURL)
 	suite.True(apiEmoji.VisibleInPicker)
 
 	// emoji should be in the db
 	dbEmoji := &gtsmodel.Emoji{}
-	err = suite.db.GetWhere(context.Background(), []db.Where{{Key: "shortcode", Value: "rainbow"}}, dbEmoji)
+	err = suite.db.GetWhere(context.Background(), []db.Where{{Key: "shortcode", Value: "new_emoji"}}, dbEmoji)
 	suite.NoError(err)
 
 	// check fields on the emoji
 	suite.NotEmpty(dbEmoji.ID)
-	suite.Equal("rainbow", dbEmoji.Shortcode)
+	suite.Equal("new_emoji", dbEmoji.Shortcode)
 	suite.Empty(dbEmoji.Domain)
 	suite.Empty(dbEmoji.ImageRemoteURL)
 	suite.Empty(dbEmoji.ImageStaticRemoteURL)
 	suite.Equal(apiEmoji.URL, dbEmoji.ImageURL)
-	suite.Equal(apiEmoji.StaticURL, dbEmoji.ImageURL)
+	suite.Equal(apiEmoji.StaticURL, dbEmoji.ImageStaticURL)
 	suite.NotEmpty(dbEmoji.ImagePath)
 	suite.NotEmpty(dbEmoji.ImageStaticPath)
 	suite.Equal("image/png", dbEmoji.ImageContentType)
@@ -82,7 +82,45 @@ func (suite *EmojiCreateTestSuite) TestEmojiCreate() {
 	suite.False(dbEmoji.Disabled)
 	suite.NotEmpty(dbEmoji.URI)
 	suite.True(dbEmoji.VisibleInPicker)
-	suite.Empty(dbEmoji.CategoryID)aaaaaaaaa
+	suite.Empty(dbEmoji.CategoryID)
+
+	// emoji should be in storage
+	emojiBytes, err := suite.storage.Get(dbEmoji.ImagePath)
+	suite.NoError(err)
+	suite.Len(emojiBytes, dbEmoji.ImageFileSize)
+	emojiStaticBytes, err := suite.storage.Get(dbEmoji.ImageStaticPath)
+	suite.NoError(err)
+	suite.Len(emojiStaticBytes, dbEmoji.ImageStaticFileSize)
+}
+
+func (suite *EmojiCreateTestSuite) TestEmojiCreateAlreadyExists() {
+	// set up the request -- use a shortcode that already exists for an emoji in the database
+	requestBody, w, err := testrig.CreateMultipartFormData(
+		"image", "../../../../testrig/media/rainbow-original.png",
+		map[string]string{
+			"shortcode": "rainbow",
+		})
+	if err != nil {
+		panic(err)
+	}
+	bodyBytes := requestBody.Bytes()
+	recorder := httptest.NewRecorder()
+	ctx := suite.newContext(recorder, http.MethodPost, bodyBytes, admin.EmojiPath, w.FormDataContentType())
+
+	// call the handler
+	suite.adminModule.EmojiCreatePOSTHandler(ctx)
+
+	suite.Equal(http.StatusConflict, recorder.Code)
+
+	result := recorder.Result()
+	defer result.Body.Close()
+
+	// check the response
+	b, err := ioutil.ReadAll(result.Body)
+	suite.NoError(err)
+	suite.NotEmpty(b)
+
+	suite.Equal(`{"error":"conflict: emoji with shortcode rainbow already exists"}`, string(b))
 }
 
 func TestEmojiCreateTestSuite(t *testing.T) {

internal/gtserror/withcode.go

@@ -122,3 +122,16 @@ func NewErrorInternalError(original error, helpText ...string) WithCode {
 		code:     http.StatusInternalServerError,
 	}
 }
+
+// NewErrorConflict returns an ErrorWithCode 409 with the given original error and optional help text.
+func NewErrorConflict(original error, helpText ...string) WithCode {
+	safe := "conflict"
+	if helpText != nil {
+		safe = safe + ": " + strings.Join(helpText, ": ")
+	}
+	return withCode{
+		original: original,
+		safe:     errors.New(safe),
+		code:     http.StatusConflict,
+	}
+}

internal/media/processingemoji.go

@@ -72,6 +72,9 @@ type ProcessingEmoji struct {
 	storage  *kv.KVStore
 
 	err error // error created during processing, if any
+
+	// track whether this emoji has already been put in the databse
+	insertedInDB bool
 }
 
 // EmojiID returns the ID of the underlying emoji without blocking processing.
@@ -94,6 +97,16 @@ func (p *ProcessingEmoji) LoadEmoji(ctx context.Context) (*gtsmodel.Emoji, error
 		return nil, err
 	}
 
+	// store the result in the database before returning it
+	p.mu.Lock()
+	defer p.mu.Unlock()
+	if !p.insertedInDB {
+		if err := p.database.Put(ctx, p.emoji); err != nil {
+			return nil, err
+		}
+		p.insertedInDB = true
+	}
+
 	return p.emoji, nil
 }
 
@@ -127,13 +140,6 @@ func (p *ProcessingEmoji) loadStatic(ctx context.Context) (*ImageMeta, error) {
 		// set appropriate fields on the emoji based on the static version we derived
 		p.emoji.ImageStaticFileSize = len(static.image)
 
-		// update the emoji in the db
-		if err := putOrUpdate(ctx, p.database, p.emoji); err != nil {
-			p.err = err
-			p.staticState = errored
-			return nil, err
-		}
-
 		// set the static on the processing emoji
 		p.static = static
 
@@ -197,7 +203,7 @@ func (p *ProcessingEmoji) loadFullSize(ctx context.Context) (*ImageMeta, error)
 }
 
 // fetchRawData calls the data function attached to p if it hasn't been called yet,
-// and updates the underlying attachment fields as necessary.
+// and updates the underlying emoji fields as necessary.
 // It should only be called from within a function that already has a lock on p!
 func (p *ProcessingEmoji) fetchRawData(ctx context.Context) error {
 	// check if we've already done this and bail early if we have

internal/media/processingmedia.go

@@ -70,6 +70,9 @@ type ProcessingMedia struct {
 	storage  *kv.KVStore
 
 	err error // error created during processing, if any
+
+	// track whether this media has already been put in the databse
+	insertedInDB bool
 }
 
 // AttachmentID returns the ID of the underlying media attachment without blocking processing.
@@ -92,6 +95,16 @@ func (p *ProcessingMedia) LoadAttachment(ctx context.Context) (*gtsmodel.MediaAt
 		return nil, err
 	}
 
+	// store the result in the database before returning it
+	p.mu.Lock()
+	defer p.mu.Unlock()
+	if !p.insertedInDB {
+		if err := p.database.Put(ctx, p.attachment); err != nil {
+			return nil, err
+		}
+		p.insertedInDB = true
+	}
+
 	return p.attachment, nil
 }
 
@@ -143,12 +156,6 @@ func (p *ProcessingMedia) loadThumb(ctx context.Context) (*ImageMeta, error) {
 		}
 		p.attachment.Thumbnail.FileSize = len(thumb.image)
 
-		if err := putOrUpdate(ctx, p.database, p.attachment); err != nil {
-			p.err = err
-			p.thumbstate = errored
-			return nil, err
-		}
-
 		// set the thumbnail of this media
 		p.thumb = thumb
 
@@ -216,12 +223,6 @@ func (p *ProcessingMedia) loadFullSize(ctx context.Context) (*ImageMeta, error)
 		p.attachment.File.UpdatedAt = time.Now()
 		p.attachment.Processing = gtsmodel.ProcessingStatusProcessed
 
-		if err := putOrUpdate(ctx, p.database, p.attachment); err != nil {
-			p.err = err
-			p.fullSizeState = errored
-			return nil, err
-		}
-
 		// set the fullsize of this media
 		p.fullSize = decoded
 

internal/processing/admin.go

@@ -26,7 +26,7 @@ import (
 	"github.com/superseriousbusiness/gotosocial/internal/oauth"
 )
 
-func (p *processor) AdminEmojiCreate(ctx context.Context, authed *oauth.Auth, form *apimodel.EmojiCreateRequest) (*apimodel.Emoji, error) {
+func (p *processor) AdminEmojiCreate(ctx context.Context, authed *oauth.Auth, form *apimodel.EmojiCreateRequest) (*apimodel.Emoji, gtserror.WithCode) {
 	return p.adminProcessor.EmojiCreate(ctx, authed.Account, authed.User, form)
 }
 

internal/processing/admin/admin.go

@@ -38,7 +38,7 @@ type Processor interface {
 	DomainBlocksGet(ctx context.Context, account *gtsmodel.Account, export bool) ([]*apimodel.DomainBlock, gtserror.WithCode)
 	DomainBlockGet(ctx context.Context, account *gtsmodel.Account, id string, export bool) (*apimodel.DomainBlock, gtserror.WithCode)
 	DomainBlockDelete(ctx context.Context, account *gtsmodel.Account, id string) (*apimodel.DomainBlock, gtserror.WithCode)
-	EmojiCreate(ctx context.Context, account *gtsmodel.Account, user *gtsmodel.User, form *apimodel.EmojiCreateRequest) (*apimodel.Emoji, error)
+	EmojiCreate(ctx context.Context, account *gtsmodel.Account, user *gtsmodel.User, form *apimodel.EmojiCreateRequest) (*apimodel.Emoji, gtserror.WithCode)
 }
 
 type processor struct {

internal/processing/admin/emoji.go

@@ -26,14 +26,16 @@ import (
 	"io"
 
 	apimodel "github.com/superseriousbusiness/gotosocial/internal/api/model"
+	"github.com/superseriousbusiness/gotosocial/internal/db"
+	"github.com/superseriousbusiness/gotosocial/internal/gtserror"
 	"github.com/superseriousbusiness/gotosocial/internal/gtsmodel"
 	"github.com/superseriousbusiness/gotosocial/internal/id"
 	"github.com/superseriousbusiness/gotosocial/internal/uris"
 )
 
-func (p *processor) EmojiCreate(ctx context.Context, account *gtsmodel.Account, user *gtsmodel.User, form *apimodel.EmojiCreateRequest) (*apimodel.Emoji, error) {
+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, fmt.Errorf("user %s not an admin", user.ID)
+		return nil, gtserror.NewErrorNotAuthorized(fmt.Errorf("user %s not an admin", user.ID), "user is not an admin")
 	}
 
 	data := func(innerCtx context.Context) ([]byte, error) {
@@ -56,24 +58,27 @@ func (p *processor) EmojiCreate(ctx context.Context, account *gtsmodel.Account,
 
 	emojiID, err := id.NewRandomULID()
 	if err != nil {
-		return nil, fmt.Errorf("error creating id for new emoji: %s", err)
+		return nil, gtserror.NewErrorInternalError(fmt.Errorf("error creating id for new emoji: %s", err), "error creating emoji ID")
 	}
 
 	emojiURI := uris.GenerateURIForEmoji(emojiID)
 
 	processingEmoji, err := p.mediaManager.ProcessEmoji(ctx, data, form.Shortcode, emojiID, emojiURI, nil)
 	if err != nil {
-		return nil, err
+		return nil, gtserror.NewErrorInternalError(fmt.Errorf("error processing emoji: %s", err), "error processing emoji")
 	}
 
 	emoji, err := processingEmoji.LoadEmoji(ctx)
 	if err != nil {
-		return nil, err
+		if err == db.ErrAlreadyExists {
+			return nil, gtserror.NewErrorConflict(fmt.Errorf("emoji with shortcode %s already exists", form.Shortcode), fmt.Sprintf("emoji with shortcode %s already exists", form.Shortcode))
+		}
+		return nil, gtserror.NewErrorInternalError(fmt.Errorf("error loading emoji: %s", err), "error loading emoji")
 	}
 
 	apiEmoji, err := p.tc.EmojiToAPIEmoji(ctx, emoji)
 	if err != nil {
-		return nil, fmt.Errorf("error converting emoji to apitype: %s", err)
+		return nil, gtserror.NewErrorInternalError(fmt.Errorf("error converting emoji: %s", err), "error converting emoji to api representation")
 	}
 
 	return &apiEmoji, nil

internal/processing/processor.go

@@ -96,7 +96,7 @@ type Processor interface {
 	AccountBlockRemove(ctx context.Context, authed *oauth.Auth, targetAccountID string) (*apimodel.Relationship, gtserror.WithCode)
 
 	// AdminEmojiCreate handles the creation of a new instance emoji by an admin, using the given form.
-	AdminEmojiCreate(ctx context.Context, authed *oauth.Auth, form *apimodel.EmojiCreateRequest) (*apimodel.Emoji, error)
+	AdminEmojiCreate(ctx context.Context, authed *oauth.Auth, form *apimodel.EmojiCreateRequest) (*apimodel.Emoji, gtserror.WithCode)
 	// AdminDomainBlockCreate handles the creation of a new domain block by an admin, using the given form.
 	AdminDomainBlockCreate(ctx context.Context, authed *oauth.Auth, form *apimodel.DomainBlockCreateRequest) (*apimodel.DomainBlock, gtserror.WithCode)
 	// AdminDomainBlocksImport handles the import of multiple domain blocks by an admin, using the given form.