diff options
| -rw-r--r-- | internal/api/client/admin/emojicreate.go | 10 | ||||
| -rw-r--r-- | internal/api/client/admin/emojicreate_test.go | 50 | ||||
| -rw-r--r-- | internal/gtserror/withcode.go | 13 | ||||
| -rw-r--r-- | internal/media/processingemoji.go | 22 | ||||
| -rw-r--r-- | internal/media/processingmedia.go | 25 | ||||
| -rw-r--r-- | internal/processing/admin.go | 2 | ||||
| -rw-r--r-- | internal/processing/admin/admin.go | 2 | ||||
| -rw-r--r-- | internal/processing/admin/emoji.go | 17 | ||||
| -rw-r--r-- | internal/processing/processor.go | 2 | 
9 files changed, 104 insertions, 39 deletions
diff --git a/internal/api/client/admin/emojicreate.go b/internal/api/client/admin/emojicreate.go index de7a2979a..ef42d0a13 100644 --- a/internal/api/client/admin/emojicreate.go +++ b/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) -	if err != nil { -		l.Debugf("error creating emoji: %s", err) -		c.JSON(http.StatusBadRequest, gin.H{"error": err.Error()}) +	apiEmoji, errWithCode := m.processor.AdminEmojiCreate(c.Request.Context(), authed, form) +	if errWithCode != nil { +		l.Debugf("error creating emoji: %s", errWithCode.Error()) +		c.JSON(errWithCode.Code(), gin.H{"error": errWithCode.Safe()})  		return  	} diff --git a/internal/api/client/admin/emojicreate_test.go b/internal/api/client/admin/emojicreate_test.go index 14b83b534..2b7476da1 100644 --- a/internal/api/client/admin/emojicreate_test.go +++ b/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 := >smodel.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) { diff --git a/internal/gtserror/withcode.go b/internal/gtserror/withcode.go index a00cc8503..34889b961 100644 --- a/internal/gtserror/withcode.go +++ b/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, +	} +} diff --git a/internal/media/processingemoji.go b/internal/media/processingemoji.go index eeccdb281..467b500fc 100644 --- a/internal/media/processingemoji.go +++ b/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 diff --git a/internal/media/processingmedia.go b/internal/media/processingmedia.go index 1bfd7b629..fade64c24 100644 --- a/internal/media/processingmedia.go +++ b/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 diff --git a/internal/processing/admin.go b/internal/processing/admin.go index c70bd79d0..764e6d302 100644 --- a/internal/processing/admin.go +++ b/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)  } diff --git a/internal/processing/admin/admin.go b/internal/processing/admin/admin.go index 27a7da47a..bdb586588 100644 --- a/internal/processing/admin/admin.go +++ b/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 { diff --git a/internal/processing/admin/emoji.go b/internal/processing/admin/emoji.go index 77fa5102b..fcc17c4be 100644 --- a/internal/processing/admin/emoji.go +++ b/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 diff --git a/internal/processing/processor.go b/internal/processing/processor.go index 2626c1fea..2406681ea 100644 --- a/internal/processing/processor.go +++ b/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.  | 
