Fixes #40. Allow to upload profile information without a skin

Remove skin file uploading stubs
elyby · Dec 22, 2023 · cadb89f · cadb89f
1 parent 20ba789
commit cadb89f
Show file tree

Hide file tree

Showing 6 changed files with 65 additions and 91 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -6,6 +6,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased] - xxxx-xx-xx
 ### Added
+- Allow to remove a skin without removing all user information
 - New StatsD metrics:
   - Counters:
     - `ely.skinsystem.{hostname}.app.profiles.request`
@@ -14,6 +15,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Bumped Go version to 1.21.
 
 ### Removed
+- Removed mentioning and processing of skin uploading as a file, as this functionality was never implemented and was not planned to be implemented
 - StatsD metrics:
  - Gauges:
    - `ely.skinsystem.{hostname}.app.redis.pool.available`

diff --git a/README.md b/README.md
@@ -344,10 +344,9 @@ You can obtain token by executing `docker-compose run --rm app token`.
 
 #### `POST /api/skins`
 
-> **Warning**: skin uploading via `skin` field is not implemented for now.
+Endpoint allows you to create or update skin record for a username.
 
-Endpoint allows you to create or update skin record for a username. To upload skin, you have to send multipart
-form data. `form-urlencoded` also supported, but, as you may know, it doesn't support files uploading.
+The request body must be encoded as `application/x-www-form-urlencoded`.
 
 **Request params:**
 
@@ -361,8 +360,9 @@ form data. `form-urlencoded` also supported, but, as you may know, it doesn't su
 | isSlim          | bool   | Does skin have slim arms (Alex model).                                         |
 | mojangTextures  | string | Mojang textures field. It must be a base64 encoded json string. Not required.  |
 | mojangSignature | string | Signature for Mojang textures, which is required when `mojangTextures` passed. |
-| url             | string | Actual url of the skin. You have to pass this parameter or `skin`.             |
-| skin            | file   | Skin file. You have to pass this parameter or `url`.                           |
+| url             | string | Actual url of the skin.                                                        |
+
+**Important**: all parameters are always read at least as their default values. So, if you only want to update the username and not pass the skin data it will reset all skin information. If you want to keep the data, you should always pass the full set of parameters.
 
 If successful you'll receive `201` status code. In the case of failure there will be `400` status code and errors list
 as json:

diff --git a/http/api.go b/http/api.go
@@ -19,14 +19,6 @@ const UUID_ANY = "^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$
 var regexUuidAny = regexp.MustCompile(UUID_ANY)
 
 func init() {
-	govalidator.AddCustomRule("skinUploadingNotAvailable", func(field string, rule string, message string, value interface{}) error {
-		if message == "" {
-			message = "Skin uploading is temporary unavailable"
-		}
-
-		return errors.New(message)
-	})
-
 	// Add ability to validate any possible uuid form
 	govalidator.AddCustomRule("uuid_any", func(field string, rule string, message string, value interface{}) error {
 		str := value.(string)
@@ -163,50 +155,41 @@ func (ctx *Api) findIdentityOrCleanup(identityId int, username string) (*model.S
 }
 
 func validatePostSkinRequest(request *http.Request) map[string][]string {
-	const maxMultipartMemory int64 = 32 << 20
-	const oneOfSkinOrUrlMessage = "One of url or skin should be provided, but not both"
-
-	_ = request.ParseMultipartForm(maxMultipartMemory)
+	_ = request.ParseForm()
 
 	validationRules := govalidator.MapData{
-		"identityId": {"required", "numeric", "min:1"},
-		"username":   {"required"},
-		"uuid":       {"required", "uuid_any"},
-		"skinId":     {"required", "numeric", "min:1"},
-		"url":        {"url"},
-		"file:skin":  {"ext:png", "size:24576", "mime:image/png"},
-		"is1_8":      {"bool"},
-		"isSlim":     {"bool"},
+		"identityId":      {"required", "numeric", "min:1"},
+		"username":        {"required"},
+		"uuid":            {"required", "uuid_any"},
+		"skinId":          {"required", "numeric"},
+		"url":             {},
+		"is1_8":           {"bool"},
+		"isSlim":          {"bool"},
+		"mojangTextures":  {},
+		"mojangSignature": {},
 	}
 
-	shouldAppendSkinRequiredError := false
 	url := request.Form.Get("url")
-	_, _, skinErr := request.FormFile("skin")
-	if (url != "" && skinErr == nil) || (url == "" && skinErr != nil) {
-		shouldAppendSkinRequiredError = true
-	} else if skinErr == nil {
-		validationRules["file:skin"] = append(validationRules["file:skin"], "skinUploadingNotAvailable")
-	} else if url != "" {
+	if url == "" {
+		validationRules["skinId"] = append(validationRules["skinId"], "numeric_between:0,0")
+	} else {
+		validationRules["url"] = append(validationRules["url"], "url")
+		validationRules["skinId"] = append(validationRules["skinId"], "numeric_between:1,")
 		validationRules["is1_8"] = append(validationRules["is1_8"], "required")
 		validationRules["isSlim"] = append(validationRules["isSlim"], "required")
 	}
 
 	mojangTextures := request.Form.Get("mojangTextures")
 	if mojangTextures != "" {
-		validationRules["mojangSignature"] = []string{"required"}
+		validationRules["mojangSignature"] = append(validationRules["mojangSignature"], "required")
 	}
 
 	validator := govalidator.New(govalidator.Options{
 		Request:         request,
 		Rules:           validationRules,
 		RequiredDefault: false,
-		FormSize:        maxMultipartMemory,
 	})
 	validationResults := validator.Validate()
-	if shouldAppendSkinRequiredError {
-		validationResults["url"] = append(validationResults["url"], oneOfSkinOrUrlMessage)
-		validationResults["skin"] = append(validationResults["skin"], oneOfSkinOrUrlMessage)
-	}
 
 	if len(validationResults) != 0 {
 		return validationResults

diff --git a/http/api_test.go b/http/api_test.go
@@ -6,7 +6,6 @@ import (
 	"errors"
 	"io"
 	"io/ioutil"
-	"mime/multipart"
 	"net/http"
 	"net/http/httptest"
 	"net/url"
@@ -136,6 +135,41 @@ var postSkinTestsCases = []*postSkinTestCase{
 			suite.Empty(body)
 		},
 	},
+	{
+		Name: "Update exists identity by changing textures data to empty",
+		Form: bytes.NewBufferString(url.Values{
+			"identityId":      {"1"},
+			"username":        {"mock_username"},
+			"uuid":            {"0f657aa8-bfbe-415d-b700-5750090d3af3"},
+			"skinId":          {"0"},
+			"is1_8":           {"0"},
+			"isSlim":          {"0"},
+			"url":             {""},
+			"mojangTextures":  {""},
+			"mojangSignature": {""},
+		}.Encode()),
+		BeforeTest: func(suite *apiTestSuite) {
+			suite.SkinsRepository.On("FindSkinByUserId", 1).Return(createSkinModel("mock_username", false), nil)
+			suite.SkinsRepository.On("SaveSkin", mock.MatchedBy(func(model *model.Skin) bool {
+				suite.Equal(1, model.UserId)
+				suite.Equal("mock_username", model.Username)
+				suite.Equal("0f657aa8-bfbe-415d-b700-5750090d3af3", model.Uuid)
+				suite.Equal(0, model.SkinId)
+				suite.False(model.Is1_8)
+				suite.False(model.IsSlim)
+				suite.Equal("", model.Url)
+				suite.Equal("", model.MojangTextures)
+				suite.Equal("", model.MojangSignature)
+
+				return true
+			})).Times(1).Return(nil)
+		},
+		AfterTest: func(suite *apiTestSuite, response *http.Response) {
+			suite.Equal(201, response.StatusCode)
+			body, _ := io.ReadAll(response.Body)
+			suite.Equal("", string(body))
+		},
+	},
 	{
 		Name: "Update exists identity by changing its identityId",
 		Form: bytes.NewBufferString(url.Values{
@@ -271,7 +305,7 @@ func (suite *apiTestSuite) TestPostSkin() {
 				"skinId": [
 					"The skinId field is required",
 					"The skinId field must be numeric",
-					"The skinId field must be minimum 1 char"
+					"The skinId field must be numeric value between 0 and 0"
 				],
 				"username": [
 					"The username field is required"
@@ -280,54 +314,12 @@ func (suite *apiTestSuite) TestPostSkin() {
 					"The uuid field is required",
 					"The uuid field must contain valid UUID"
 				],
-				"url": [
-					"One of url or skin should be provided, but not both"
-				],
-				"skin": [
-					"One of url or skin should be provided, but not both"
-				],
 				"mojangSignature": [
 					"The mojangSignature field is required"
 				]
 			}
 		}`, string(body))
 	})
-
-	suite.RunSubTest("Upload textures with skin as file", func() {
-		inputBody := &bytes.Buffer{}
-		writer := multipart.NewWriter(inputBody)
-
-		part, _ := writer.CreateFormFile("skin", "char.png")
-		_, _ = part.Write(loadSkinFile())
-
-		_ = writer.WriteField("identityId", "1")
-		_ = writer.WriteField("username", "mock_user")
-		_ = writer.WriteField("uuid", "0f657aa8-bfbe-415d-b700-5750090d3af3")
-		_ = writer.WriteField("skinId", "5")
-
-		err := writer.Close()
-		if err != nil {
-			panic(err)
-		}
-
-		req := httptest.NewRequest("POST", "http://chrly/skins", inputBody)
-		req.Header.Add("Content-Type", writer.FormDataContentType())
-		w := httptest.NewRecorder()
-
-		suite.App.Handler().ServeHTTP(w, req)
-
-		resp := w.Result()
-		defer resp.Body.Close()
-		suite.Equal(400, resp.StatusCode)
-		responseBody, _ := ioutil.ReadAll(resp.Body)
-		suite.JSONEq(`{
-			"errors": {
-				"skin": [
-					"Skin uploading is temporary unavailable"
-				]
-			}
-		}`, string(responseBody))
-	})
 }
 
 /**************************************

diff --git a/http/skinsystem.go b/http/skinsystem.go
@@ -6,7 +6,6 @@ import (
 	"encoding/base64"
 	"encoding/json"
 	"encoding/pem"
-	"github.com/elyby/chrly/utils"
 	"io"
 	"net/http"
 	"strings"
@@ -16,6 +15,7 @@ import (
 
 	"github.com/elyby/chrly/api/mojang"
 	"github.com/elyby/chrly/model"
+	"github.com/elyby/chrly/utils"
 )
 
 var timeNow = time.Now
@@ -275,20 +275,15 @@ func (ctx *Skinsystem) getProfile(request *http.Request, proxy bool) (*profile,
 	}
 
 	profile := &profile{
-		Id:              "",
-		Username:        "",
-		Textures:        &mojang.TexturesResponse{}, // Field must be initialized to avoid "null" after json encoding
-		CapeFile:        nil,
-		MojangTextures:  "",
-		MojangSignature: "",
+		Textures: &mojang.TexturesResponse{}, // Field must be initialized to avoid "null" after json encoding
 	}
 
 	if skin != nil {
 		profile.Id = strings.Replace(skin.Uuid, "-", "", -1)
 		profile.Username = skin.Username
 	}
 
-	if skin != nil && skin.SkinId != 0 {
+	if skin != nil && skin.Url != "" {
 		profile.Textures.Skin = &mojang.SkinTexturesResponse{
 			Url: skin.Url,
 		}
@@ -350,6 +345,8 @@ func (ctx *Skinsystem) getProfile(request *http.Request, proxy bool) (*profile,
 			profile.Id = mojangProfile.Id
 			profile.Username = mojangProfile.Name
 		}
+	} else if profile.Id != "" {
+		return profile, nil
 	} else {
 		return nil, nil
 	}

diff --git a/model/skin.go b/model/skin.go
@@ -4,7 +4,7 @@ type Skin struct {
 	UserId          int    `json:"userId"`
 	Uuid            string `json:"uuid"`
 	Username        string `json:"username"`
-	SkinId          int    `json:"skinId"`
+	SkinId          int    `json:"skinId"` // deprecated
 	Url             string `json:"url"`
 	Is1_8           bool   `json:"is1_8"`
 	IsSlim          bool   `json:"isSlim"`