commit 80c26d61f75ca739e6071cc25b594033face3aa1
parent 04ac3f8acf30a428ad32fff4d5d02558e2eaf379
Author: tobi <31960611+tsmethurst@users.noreply.github.com>
Date: Sat, 4 Feb 2023 15:53:11 +0100
[bugfix] Allow instance thumbnail description to be set separately from image (#1417)
Diffstat:
3 files changed, 212 insertions(+), 171 deletions(-)
diff --git a/internal/api/client/instance/instancepatch.go b/internal/api/client/instance/instancepatch.go
@@ -178,19 +178,17 @@ func validateInstanceUpdate(form *apimodel.InstanceSettingsUpdateRequest) error
return errors.New("empty form submitted")
}
- maxImageSize := config.GetMediaImageMaxSize()
- maxDescriptionChars := config.GetMediaDescriptionMaxChars()
-
- // validate avatar if present
if form.Avatar != nil {
+ maxImageSize := config.GetMediaImageMaxSize()
if size := form.Avatar.Size; size > int64(maxImageSize) {
return fmt.Errorf("file size limit exceeded: limit is %d bytes but desired instance avatar was %d bytes", maxImageSize, size)
}
+ }
- if form.AvatarDescription != nil {
- if length := len([]rune(*form.AvatarDescription)); length > maxDescriptionChars {
- return fmt.Errorf("avatar description length must be less than %d characters (inclusive), but provided avatar description was %d chars", maxDescriptionChars, length)
- }
+ if form.AvatarDescription != nil {
+ maxDescriptionChars := config.GetMediaDescriptionMaxChars()
+ if length := len([]rune(*form.AvatarDescription)); length > maxDescriptionChars {
+ return fmt.Errorf("avatar description length must be less than %d characters (inclusive), but provided avatar description was %d chars", maxDescriptionChars, length)
}
}
diff --git a/internal/api/client/instance/instancepatch_test.go b/internal/api/client/instance/instancepatch_test.go
@@ -37,37 +37,44 @@ type InstancePatchTestSuite struct {
InstanceStandardTestSuite
}
-func (suite *InstancePatchTestSuite) TestInstancePatch1() {
- requestBody, w, err := testrig.CreateMultipartFormData(
- "", "",
- map[string]string{
- "title": "Example Instance",
- "contact_username": "admin",
- "contact_email": "someone@example.org",
- })
+func (suite *InstancePatchTestSuite) instancePatch(fieldName string, fileName string, extraFields map[string]string) (code int, body []byte) {
+ requestBody, w, err := testrig.CreateMultipartFormData(fieldName, fileName, extraFields)
if err != nil {
- panic(err)
+ suite.FailNow(err.Error())
}
- bodyBytes := requestBody.Bytes()
- // set up the request
recorder := httptest.NewRecorder()
- ctx := suite.newContext(recorder, http.MethodPatch, instance.InstanceInformationPathV1, bodyBytes, w.FormDataContentType(), true)
+ ctx := suite.newContext(recorder, http.MethodPatch, instance.InstanceInformationPathV1, requestBody.Bytes(), w.FormDataContentType(), true)
- // call the handler
suite.instanceModule.InstanceUpdatePATCHHandler(ctx)
- // we should have OK because our request was valid
- suite.Equal(http.StatusOK, recorder.Code)
-
result := recorder.Result()
defer result.Body.Close()
b, err := io.ReadAll(result.Body)
- suite.NoError(err)
+ if err != nil {
+ suite.FailNow(err.Error())
+ }
+
+ return recorder.Code, b
+}
+
+func (suite *InstancePatchTestSuite) TestInstancePatch1() {
+ code, b := suite.instancePatch("", "", map[string]string{
+ "title": "Example Instance",
+ "contact_username": "admin",
+ "contact_email": "someone@example.org",
+ })
+
+ if expectedCode := http.StatusOK; code != expectedCode {
+ suite.FailNowf("wrong status code", "expected %d but got %d", expectedCode, code)
+ }
+
dst := new(bytes.Buffer)
- err = json.Indent(dst, b, "", " ")
- suite.NoError(err)
+ if err := json.Indent(dst, b, "", " "); err != nil {
+ suite.FailNow(err.Error())
+ }
+
suite.Equal(`{
"uri": "http://localhost:8080",
"account_domain": "localhost:8080",
@@ -150,34 +157,19 @@ func (suite *InstancePatchTestSuite) TestInstancePatch1() {
}
func (suite *InstancePatchTestSuite) TestInstancePatch2() {
- requestBody, w, err := testrig.CreateMultipartFormData(
- "", "",
- map[string]string{
- "title": "<p>Geoff's Instance</p>",
- })
- if err != nil {
- panic(err)
- }
- bodyBytes := requestBody.Bytes()
+ code, b := suite.instancePatch("", "", map[string]string{
+ "title": "<p>Geoff's Instance</p>",
+ })
- // set up the request
- recorder := httptest.NewRecorder()
- ctx := suite.newContext(recorder, http.MethodPatch, instance.InstanceInformationPathV1, bodyBytes, w.FormDataContentType(), true)
-
- // call the handler
- suite.instanceModule.InstanceUpdatePATCHHandler(ctx)
-
- // we should have OK because our request was valid
- suite.Equal(http.StatusOK, recorder.Code)
-
- result := recorder.Result()
- defer result.Body.Close()
+ if expectedCode := http.StatusOK; code != expectedCode {
+ suite.FailNowf("wrong status code", "expected %d but got %d", expectedCode, code)
+ }
- b, err := io.ReadAll(result.Body)
- suite.NoError(err)
dst := new(bytes.Buffer)
- err = json.Indent(dst, b, "", " ")
- suite.NoError(err)
+ if err := json.Indent(dst, b, "", " "); err != nil {
+ suite.FailNow(err.Error())
+ }
+
suite.Equal(`{
"uri": "http://localhost:8080",
"account_domain": "localhost:8080",
@@ -260,34 +252,19 @@ func (suite *InstancePatchTestSuite) TestInstancePatch2() {
}
func (suite *InstancePatchTestSuite) TestInstancePatch3() {
- requestBody, w, err := testrig.CreateMultipartFormData(
- "", "",
- map[string]string{
- "short_description": "<p>This is some html, which is <em>allowed</em> in short descriptions.</p>",
- })
- if err != nil {
- panic(err)
- }
- bodyBytes := requestBody.Bytes()
-
- // set up the request
- recorder := httptest.NewRecorder()
- ctx := suite.newContext(recorder, http.MethodPatch, instance.InstanceInformationPathV1, bodyBytes, w.FormDataContentType(), true)
-
- // call the handler
- suite.instanceModule.InstanceUpdatePATCHHandler(ctx)
+ code, b := suite.instancePatch("", "", map[string]string{
+ "short_description": "<p>This is some html, which is <em>allowed</em> in short descriptions.</p>",
+ })
- // we should have OK because our request was valid
- suite.Equal(http.StatusOK, recorder.Code)
-
- result := recorder.Result()
- defer result.Body.Close()
+ if expectedCode := http.StatusOK; code != expectedCode {
+ suite.FailNowf("wrong status code", "expected %d but got %d", expectedCode, code)
+ }
- b, err := io.ReadAll(result.Body)
- suite.NoError(err)
dst := new(bytes.Buffer)
- err = json.Indent(dst, b, "", " ")
- suite.NoError(err)
+ if err := json.Indent(dst, b, "", " "); err != nil {
+ suite.FailNow(err.Error())
+ }
+
suite.Equal(`{
"uri": "http://localhost:8080",
"account_domain": "localhost:8080",
@@ -370,28 +347,18 @@ func (suite *InstancePatchTestSuite) TestInstancePatch3() {
}
func (suite *InstancePatchTestSuite) TestInstancePatch4() {
- requestBody, w, err := testrig.CreateMultipartFormData(
- "", "",
- map[string]string{})
- if err != nil {
- panic(err)
- }
- bodyBytes := requestBody.Bytes()
+ code, b := suite.instancePatch("", "", map[string]string{
+ "": "",
+ })
- // set up the request
- recorder := httptest.NewRecorder()
- ctx := suite.newContext(recorder, http.MethodPatch, instance.InstanceInformationPathV1, bodyBytes, w.FormDataContentType(), true)
-
- // call the handler
- suite.instanceModule.InstanceUpdatePATCHHandler(ctx)
-
- suite.Equal(http.StatusBadRequest, recorder.Code)
-
- result := recorder.Result()
- defer result.Body.Close()
+ if expectedCode := http.StatusBadRequest; code != expectedCode {
+ suite.FailNowf("wrong status code", "expected %d but got %d", expectedCode, code)
+ }
- b, err := io.ReadAll(result.Body)
- suite.NoError(err)
+ dst := new(bytes.Buffer)
+ if err := json.Indent(dst, b, "", " "); err != nil {
+ suite.FailNow(err.Error())
+ }
suite.Equal(`{"error":"Bad Request: empty form submitted"}`, string(b))
}
@@ -431,34 +398,19 @@ func (suite *InstancePatchTestSuite) TestInstancePatch5() {
}
func (suite *InstancePatchTestSuite) TestInstancePatch6() {
- requestBody, w, err := testrig.CreateMultipartFormData(
- "", "",
- map[string]string{
- "contact_email": "",
- })
- if err != nil {
- panic(err)
- }
- bodyBytes := requestBody.Bytes()
+ code, b := suite.instancePatch("", "", map[string]string{
+ "contact_email": "",
+ })
- // set up the request
- recorder := httptest.NewRecorder()
- ctx := suite.newContext(recorder, http.MethodPatch, instance.InstanceInformationPathV1, bodyBytes, w.FormDataContentType(), true)
-
- // call the handler
- suite.instanceModule.InstanceUpdatePATCHHandler(ctx)
-
- // we should have OK because our request was valid
- suite.Equal(http.StatusOK, recorder.Code)
-
- result := recorder.Result()
- defer result.Body.Close()
+ if expectedCode := http.StatusOK; code != expectedCode {
+ suite.FailNowf("wrong status code", "expected %d but got %d", expectedCode, code)
+ }
- b, err := io.ReadAll(result.Body)
- suite.NoError(err)
dst := new(bytes.Buffer)
- err = json.Indent(dst, b, "", " ")
- suite.NoError(err)
+ if err := json.Indent(dst, b, "", " "); err != nil {
+ suite.FailNow(err.Error())
+ }
+
suite.Equal(`{
"uri": "http://localhost:8080",
"account_domain": "localhost:8080",
@@ -541,67 +493,40 @@ func (suite *InstancePatchTestSuite) TestInstancePatch6() {
}
func (suite *InstancePatchTestSuite) TestInstancePatch7() {
- requestBody, w, err := testrig.CreateMultipartFormData(
- "", "",
- map[string]string{
- "contact_email": "not.an.email.address",
- })
- if err != nil {
- panic(err)
- }
- bodyBytes := requestBody.Bytes()
-
- // set up the request
- recorder := httptest.NewRecorder()
- ctx := suite.newContext(recorder, http.MethodPatch, instance.InstanceInformationPathV1, bodyBytes, w.FormDataContentType(), true)
+ code, b := suite.instancePatch("", "", map[string]string{
+ "contact_email": "not.an.email.address",
+ })
- // call the handler
- suite.instanceModule.InstanceUpdatePATCHHandler(ctx)
-
- suite.Equal(http.StatusBadRequest, recorder.Code)
-
- result := recorder.Result()
- defer result.Body.Close()
+ if expectedCode := http.StatusBadRequest; code != expectedCode {
+ suite.FailNowf("wrong status code", "expected %d but got %d", expectedCode, code)
+ }
- b, err := io.ReadAll(result.Body)
- suite.NoError(err)
+ dst := new(bytes.Buffer)
+ if err := json.Indent(dst, b, "", " "); err != nil {
+ suite.FailNow(err.Error())
+ }
suite.Equal(`{"error":"Bad Request: mail: missing '@' or angle-addr"}`, string(b))
}
func (suite *InstancePatchTestSuite) TestInstancePatch8() {
- requestBody, w, err := testrig.CreateMultipartFormData(
- "thumbnail", "../../../../testrig/media/peglin.gif",
- map[string]string{
- "thumbnail_description": "A bouncing little green peglin.",
- })
- if err != nil {
- panic(err)
- }
- bodyBytes := requestBody.Bytes()
-
- // set up the request
- recorder := httptest.NewRecorder()
- ctx := suite.newContext(recorder, http.MethodPatch, instance.InstanceInformationPathV1, bodyBytes, w.FormDataContentType(), true)
+ code, b := suite.instancePatch("thumbnail", "../../../../testrig/media/peglin.gif", map[string]string{
+ "thumbnail_description": "A bouncing little green peglin."})
- // call the handler
- suite.instanceModule.InstanceUpdatePATCHHandler(ctx)
- suite.Equal(http.StatusOK, recorder.Code)
+ if expectedCode := http.StatusOK; code != expectedCode {
+ suite.FailNowf("wrong status code", "expected %d but got %d", expectedCode, code)
+ }
- result := recorder.Result()
- defer result.Body.Close()
+ dst := new(bytes.Buffer)
+ if err := json.Indent(dst, b, "", " "); err != nil {
+ suite.FailNow(err.Error())
+ }
instanceAccount, err := suite.db.GetInstanceAccount(context.Background(), "")
if err != nil {
suite.FailNow(err.Error())
}
- suite.NotEmpty(instanceAccount.AvatarMediaAttachmentID)
- b, err := io.ReadAll(result.Body)
- suite.NoError(err)
- dst := new(bytes.Buffer)
- err = json.Indent(dst, b, "", " ")
- suite.NoError(err)
suite.Equal(`{
"uri": "http://localhost:8080",
"account_domain": "localhost:8080",
@@ -685,7 +610,7 @@ func (suite *InstancePatchTestSuite) TestInstancePatch8() {
}`, dst.String())
// extra bonus: check the v2 model thumbnail after the patch
- instanceV2, err := suite.processor.InstanceGetV2(ctx)
+ instanceV2, err := suite.processor.InstanceGetV2(context.Background())
if err != nil {
suite.FailNow(err.Error())
}
@@ -701,6 +626,118 @@ func (suite *InstancePatchTestSuite) TestInstancePatch8() {
"thumbnail_description": "A bouncing little green peglin.",
"blurhash": "LG9t;qRS4YtO.4WDRlt5IXoxtPj["
}`, string(instanceV2ThumbnailJson))
+
+ // double extra special bonus: now update the image description without changing the image
+ code2, b2 := suite.instancePatch("", "", map[string]string{
+ "thumbnail_description": "updating the thumbnail description without changing anything else!",
+ })
+
+ if expectedCode := http.StatusOK; code2 != expectedCode {
+ suite.FailNowf("wrong status code", "expected %d but got %d", expectedCode, code2)
+ }
+
+ // just extract the value we wanna check, no need to print the whole thing again
+ i := make(map[string]interface{})
+ if err := json.Unmarshal(b2, &i); err != nil {
+ suite.FailNow(err.Error())
+ }
+
+ suite.EqualValues("updating the thumbnail description without changing anything else!", i["thumbnail_description"])
+}
+
+func (suite *InstancePatchTestSuite) TestInstancePatch9() {
+ code, b := suite.instancePatch("", "", map[string]string{
+ "thumbnail_description": "setting a new description without having a custom image set; this should change nothing!",
+ })
+
+ if expectedCode := http.StatusOK; code != expectedCode {
+ suite.FailNowf("wrong status code", "expected %d but got %d", expectedCode, code)
+ }
+
+ dst := new(bytes.Buffer)
+ if err := json.Indent(dst, b, "", " "); err != nil {
+ suite.FailNow(err.Error())
+ }
+
+ suite.Equal(`{
+ "uri": "http://localhost:8080",
+ "account_domain": "localhost:8080",
+ "title": "GoToSocial Testrig Instance",
+ "description": "\u003cp\u003eThis is the GoToSocial testrig. It doesn't federate or anything.\u003c/p\u003e\u003cp\u003eWhen the testrig is shut down, all data on it will be deleted.\u003c/p\u003e\u003cp\u003eDon't use this in production!\u003c/p\u003e",
+ "short_description": "\u003cp\u003eThis is the GoToSocial testrig. It doesn't federate or anything.\u003c/p\u003e\u003cp\u003eWhen the testrig is shut down, all data on it will be deleted.\u003c/p\u003e\u003cp\u003eDon't use this in production!\u003c/p\u003e",
+ "email": "admin@example.org",
+ "version": "0.0.0-testrig",
+ "registrations": true,
+ "approval_required": true,
+ "invites_enabled": false,
+ "configuration": {
+ "statuses": {
+ "max_characters": 5000,
+ "max_media_attachments": 6,
+ "characters_reserved_per_url": 25
+ },
+ "media_attachments": {
+ "supported_mime_types": [
+ "image/jpeg",
+ "image/gif",
+ "image/png",
+ "image/webp",
+ "video/mp4"
+ ],
+ "image_size_limit": 10485760,
+ "image_matrix_limit": 16777216,
+ "video_size_limit": 41943040,
+ "video_frame_rate_limit": 60,
+ "video_matrix_limit": 16777216
+ },
+ "polls": {
+ "max_options": 6,
+ "max_characters_per_option": 50,
+ "min_expiration": 300,
+ "max_expiration": 2629746
+ },
+ "accounts": {
+ "allow_custom_css": true,
+ "max_featured_tags": 10
+ },
+ "emojis": {
+ "emoji_size_limit": 51200
+ }
+ },
+ "urls": {
+ "streaming_api": "wss://localhost:8080"
+ },
+ "stats": {
+ "domain_count": 2,
+ "status_count": 16,
+ "user_count": 4
+ },
+ "thumbnail": "http://localhost:8080/assets/logo.png",
+ "contact_account": {
+ "id": "01F8MH17FWEB39HZJ76B6VXSKF",
+ "username": "admin",
+ "acct": "admin",
+ "display_name": "",
+ "locked": false,
+ "bot": false,
+ "created_at": "2022-05-17T13:10:59.000Z",
+ "note": "",
+ "url": "http://localhost:8080/@admin",
+ "avatar": "",
+ "avatar_static": "",
+ "header": "http://localhost:8080/assets/default_header.png",
+ "header_static": "http://localhost:8080/assets/default_header.png",
+ "followers_count": 1,
+ "following_count": 1,
+ "statuses_count": 4,
+ "last_status_at": "2021-10-20T10:41:37.000Z",
+ "emojis": [],
+ "fields": [],
+ "enable_rss": true,
+ "role": "admin"
+ },
+ "max_toot_chars": 5000
+}`, dst.String())
}
func TestInstancePatchTestSuite(t *testing.T) {
diff --git a/internal/processing/instance.go b/internal/processing/instance.go
@@ -221,8 +221,8 @@ func (p *processor) InstancePatch(ctx context.Context, form *apimodel.InstanceSe
var updateInstanceAccount bool
- // process instance avatar if provided
if form.Avatar != nil && form.Avatar.Size != 0 {
+ // process instance avatar image + description
avatarInfo, err := p.accountProcessor.UpdateAvatar(ctx, form.Avatar, form.AvatarDescription, ia.ID)
if err != nil {
return nil, gtserror.NewErrorBadRequest(err, "error processing avatar")
@@ -230,10 +230,16 @@ func (p *processor) InstancePatch(ctx context.Context, form *apimodel.InstanceSe
ia.AvatarMediaAttachmentID = avatarInfo.ID
ia.AvatarMediaAttachment = avatarInfo
updateInstanceAccount = true
+ } else if form.AvatarDescription != nil && ia.AvatarMediaAttachment != nil {
+ // process just the description for the existing avatar
+ ia.AvatarMediaAttachment.Description = *form.AvatarDescription
+ if err := p.db.UpdateByID(ctx, ia.AvatarMediaAttachment, ia.AvatarMediaAttachmentID, "description"); err != nil {
+ return nil, gtserror.NewErrorInternalError(fmt.Errorf("db error updating instance avatar description: %s", err))
+ }
}
- // process instance header if provided
if form.Header != nil && form.Header.Size != 0 {
+ // process instance header image
headerInfo, err := p.accountProcessor.UpdateHeader(ctx, form.Header, nil, ia.ID)
if err != nil {
return nil, gtserror.NewErrorBadRequest(err, "error processing header")
