From a16b0c900420cd70b0c79d4ac824a61a1e02585d Mon Sep 17 00:00:00 2001 From: Valley Date: Fri, 15 Jul 2022 00:26:08 +0800 Subject: [PATCH] [#114] simplified some code by returning early and added cap for slices --- daos/record_expand.go | 18 ++--- daos/user.go | 6 +- forms/record_upsert.go | 121 +++++++++++++++-------------- forms/user_oauth2_login.go | 48 ++++++------ forms/validators/record_data.go | 10 +-- models/record.go | 7 +- models/user.go | 4 +- resolvers/record_field_resolver.go | 2 +- tests/api.go | 2 +- tools/search/filter.go | 46 ++++------- tools/security/encrypt.go | 7 +- 11 files changed, 124 insertions(+), 147 deletions(-) diff --git a/daos/record_expand.go b/daos/record_expand.go index 9a023bc5..7deed20c 100644 --- a/daos/record_expand.go +++ b/daos/record_expand.go @@ -64,7 +64,7 @@ func (dao *Dao) expandRecords(records []*models.Record, expandPath string, fetch } // extract the id of the relations to expand - relIds := []string{} + relIds := make([]string, 0, len(records)) for _, record := range records { relIds = append(relIds, record.GetStringSliceDataValue(relField.Name)...) } @@ -92,7 +92,7 @@ func (dao *Dao) expandRecords(records []*models.Record, expandPath string, fetch for _, model := range records { relIds := model.GetStringSliceDataValue(relField.Name) - validRels := []*models.Record{} + validRels := make([]*models.Record, 0, len(relIds)) for _, id := range relIds { if rel, ok := indexedRels[id]; ok { validRels = append(validRels, rel) @@ -120,20 +120,18 @@ func (dao *Dao) expandRecords(records []*models.Record, expandPath string, fetch // normalizeExpands normalizes expand strings and merges self containing paths // (eg. ["a.b.c", "a.b", " test ", " ", "test"] -> ["a.b.c", "test"]). func normalizeExpands(paths []string) []string { - result := []string{} - // normalize paths - normalized := []string{} + normalized := make([]string, 0, len(paths)) for _, p := range paths { - p := strings.ReplaceAll(p, " ", "") // replace spaces - p = strings.Trim(p, ".") // trim incomplete paths - if p == "" { - continue + p = strings.ReplaceAll(p, " ", "") // replace spaces + p = strings.Trim(p, ".") // trim incomplete paths + if p != "" { + normalized = append(normalized, p) } - normalized = append(normalized, p) } // merge containing paths + result := make([]string, 0, len(normalized)) for i, p1 := range normalized { var skip bool for j, p2 := range normalized { diff --git a/daos/user.go b/daos/user.go index 29fa8f8a..9a13e85c 100644 --- a/daos/user.go +++ b/daos/user.go @@ -43,10 +43,10 @@ func (dao *Dao) LoadProfiles(users []*models.User) error { } // extract user ids - ids := []string{} + ids := make([]string, len(users)) usersMap := map[string]*models.User{} - for _, user := range users { - ids = append(ids, user.Id) + for i, user := range users { + ids[i] = user.Id usersMap[user.Id] = user } diff --git a/forms/record_upsert.go b/forms/record_upsert.go index 46f8b073..bb7d75b5 100644 --- a/forms/record_upsert.go +++ b/forms/record_upsert.go @@ -150,76 +150,77 @@ func (form *RecordUpsert) LoadData(r *http.Request) error { value := extendedData[key] value = field.PrepareValue(value) - if field.Type == schema.FieldTypeFile { - options, _ := field.Options.(*schema.FileOptions) - oldNames := list.ToUniqueStringSlice(form.Data[key]) + if field.Type != schema.FieldTypeFile { + form.Data[key] = value + continue + } - // delete previously uploaded file(s) - if options.MaxSelect == 1 { - // search for unset zero indexed key as a fallback - indexedKeyValue, hasIndexedKey := extendedData[key+".0"] + options, _ := field.Options.(*schema.FileOptions) + oldNames := list.ToUniqueStringSlice(form.Data[key]) - if cast.ToString(value) == "" || (hasIndexedKey && cast.ToString(indexedKeyValue) == "") { - if len(oldNames) > 0 { - form.filesToDelete = append(form.filesToDelete, oldNames...) - } - form.Data[key] = nil + // delete previously uploaded file(s) + if options.MaxSelect == 1 { + // search for unset zero indexed key as a fallback + indexedKeyValue, hasIndexedKey := extendedData[key+".0"] + + if cast.ToString(value) == "" || (hasIndexedKey && cast.ToString(indexedKeyValue) == "") { + if len(oldNames) > 0 { + form.filesToDelete = append(form.filesToDelete, oldNames...) } - } else if options.MaxSelect > 1 { - // search for individual file index to delete (eg. "file.0") - keyExp, _ := regexp.Compile(`^` + regexp.QuoteMeta(key) + `\.\d+$`) - indexesToDelete := []int{} - for indexedKey := range extendedData { - if keyExp.MatchString(indexedKey) && cast.ToString(extendedData[indexedKey]) == "" { - index, indexErr := strconv.Atoi(indexedKey[len(key)+1:]) - if indexErr != nil || index >= len(oldNames) { - continue - } - indexesToDelete = append(indexesToDelete, index) - } - } - - // slice to fill only with the non-deleted indexes - nonDeleted := []string{} - for i, name := range oldNames { - // not marked for deletion - if !list.ExistInSlice(i, indexesToDelete) { - nonDeleted = append(nonDeleted, name) + form.Data[key] = nil + } + } else if options.MaxSelect > 1 { + // search for individual file index to delete (eg. "file.0") + keyExp, _ := regexp.Compile(`^` + regexp.QuoteMeta(key) + `\.\d+$`) + indexesToDelete := make([]int, 0, len(extendedData)) + for indexedKey := range extendedData { + if keyExp.MatchString(indexedKey) && cast.ToString(extendedData[indexedKey]) == "" { + index, indexErr := strconv.Atoi(indexedKey[len(key)+1:]) + if indexErr != nil || index >= len(oldNames) { continue } - - // store the id to actually delete the file later - form.filesToDelete = append(form.filesToDelete, name) + indexesToDelete = append(indexesToDelete, index) } - form.Data[key] = nonDeleted } - // check if there are any new uploaded form files - files, err := rest.FindUploadedFiles(r, key) - if err != nil { - continue // skip invalid or missing file(s) - } - - // refresh oldNames list - oldNames = list.ToUniqueStringSlice(form.Data[key]) - - if options.MaxSelect == 1 { - // delete previous file(s) before replacing - if len(oldNames) > 0 { - form.filesToDelete = list.ToUniqueStringSlice(append(form.filesToDelete, oldNames...)) + // slice to fill only with the non-deleted indexes + nonDeleted := make([]string, 0, len(oldNames)) + for i, name := range oldNames { + // not marked for deletion + if !list.ExistInSlice(i, indexesToDelete) { + nonDeleted = append(nonDeleted, name) + continue } - form.filesToUpload = append(form.filesToUpload, files[0]) - form.Data[key] = files[0].Name() - } else if options.MaxSelect > 1 { - // append the id of each uploaded file instance - form.filesToUpload = append(form.filesToUpload, files...) - for _, file := range files { - oldNames = append(oldNames, file.Name()) - } - form.Data[key] = oldNames + + // store the id to actually delete the file later + form.filesToDelete = append(form.filesToDelete, name) } - } else { - form.Data[key] = value + form.Data[key] = nonDeleted + } + + // check if there are any new uploaded form files + files, err := rest.FindUploadedFiles(r, key) + if err != nil { + continue // skip invalid or missing file(s) + } + + // refresh oldNames list + oldNames = list.ToUniqueStringSlice(form.Data[key]) + + if options.MaxSelect == 1 { + // delete previous file(s) before replacing + if len(oldNames) > 0 { + form.filesToDelete = list.ToUniqueStringSlice(append(form.filesToDelete, oldNames...)) + } + form.filesToUpload = append(form.filesToUpload, files[0]) + form.Data[key] = files[0].Name() + } else if options.MaxSelect > 1 { + // append the id of each uploaded file instance + form.filesToUpload = append(form.filesToUpload, files...) + for _, file := range files { + oldNames = append(oldNames, file.Name()) + } + form.Data[key] = oldNames } } diff --git a/forms/user_oauth2_login.go b/forms/user_oauth2_login.go index 450aa1bd..a869277c 100644 --- a/forms/user_oauth2_login.go +++ b/forms/user_oauth2_login.go @@ -98,35 +98,35 @@ func (form *UserOauth2Login) Submit() (*models.User, *auth.AuthUser, error) { return nil, authData, err } } - } else { - if !config.AllowRegistrations { - // registration of new users is not allowed via the Oauth2 provider - return nil, authData, errors.New("Cannot find user with the authorized email.") - } + return user, authData, nil + } + if !config.AllowRegistrations { + // registration of new users is not allowed via the Oauth2 provider + return nil, authData, errors.New("Cannot find user with the authorized email.") + } - // create new user - user = &models.User{Verified: true} - upsertForm := NewUserUpsert(form.app, user) - upsertForm.Email = authData.Email - upsertForm.Password = security.RandomString(30) - upsertForm.PasswordConfirm = upsertForm.Password + // create new user + user = &models.User{Verified: true} + upsertForm := NewUserUpsert(form.app, user) + upsertForm.Email = authData.Email + upsertForm.Password = security.RandomString(30) + upsertForm.PasswordConfirm = upsertForm.Password - event := &core.UserOauth2RegisterEvent{ - User: user, - AuthData: authData, - } + event := &core.UserOauth2RegisterEvent{ + User: user, + AuthData: authData, + } - if err := form.app.OnUserBeforeOauth2Register().Trigger(event); err != nil { - return nil, authData, err - } + if err := form.app.OnUserBeforeOauth2Register().Trigger(event); err != nil { + return nil, authData, err + } - if err := upsertForm.Submit(); err != nil { - return nil, authData, err - } + if err := upsertForm.Submit(); err != nil { + return nil, authData, err + } - if err := form.app.OnUserAfterOauth2Register().Trigger(event); err != nil { - return nil, authData, err - } + if err := form.app.OnUserAfterOauth2Register().Trigger(event); err != nil { + return nil, authData, err } return user, authData, nil diff --git a/forms/validators/record_data.go b/forms/validators/record_data.go index 10df86e9..67a7fe28 100644 --- a/forms/validators/record_data.go +++ b/forms/validators/record_data.go @@ -318,12 +318,10 @@ func (validator *RecordDataValidator) checkFileValue(field *schema.SchemaField, } // extract the uploaded files - files := []*rest.UploadedFile{} - if len(validator.uploadedFiles) > 0 { - for _, file := range validator.uploadedFiles { - if list.ExistInSlice(file.Name(), names) { - files = append(files, file) - } + files := make([]*rest.UploadedFile, 0, len(validator.uploadedFiles)) + for _, file := range validator.uploadedFiles { + if list.ExistInSlice(file.Name(), names) { + files = append(files, file) } } diff --git a/models/record.go b/models/record.go index e24f6981..979a714e 100644 --- a/models/record.go +++ b/models/record.go @@ -68,10 +68,9 @@ func NewRecordFromNullStringMap(collection *Collection, data dbx.NullStringMap) // NewRecordsFromNullStringMaps initializes a new Record model for // each row in the provided NullStringMap slice. func NewRecordsFromNullStringMaps(collection *Collection, rows []dbx.NullStringMap) []*Record { - result := []*Record{} - - for _, row := range rows { - result = append(result, NewRecordFromNullStringMap(collection, row)) + result := make([]*Record, len(rows)) + for i, row := range rows { + result[i] = NewRecordFromNullStringMap(collection, row) } return result diff --git a/models/user.go b/models/user.go index 45793b51..6982c0e3 100644 --- a/models/user.go +++ b/models/user.go @@ -9,10 +9,10 @@ import ( var _ Model = (*User)(nil) const ( - // The name of the system user profiles collection. + // ProfileCollectionName is the name of the system user profiles collection. ProfileCollectionName = "profiles" - // The name of the user field from the system user profiles collection. + // ProfileCollectionUserFieldName is the name of the user field from the system user profiles collection. ProfileCollectionUserFieldName = "userId" ) diff --git a/resolvers/record_field_resolver.go b/resolvers/record_field_resolver.go index 76291236..3e3a42b3 100644 --- a/resolvers/record_field_resolver.go +++ b/resolvers/record_field_resolver.go @@ -168,7 +168,7 @@ func (r *RecordFieldResolver) Resolve(fieldName string) (resultName string, plac return "", nil, fmt.Errorf("Failed to find field %q collection.", prop) } newCollectionName := relCollection.Name - newTableAlias := (currentTableAlias + "_" + field.Name) + newTableAlias := currentTableAlias + "_" + field.Name r.addJoin( newCollectionName, diff --git a/tests/api.go b/tests/api.go index 7bccb5fc..bb3b3802 100644 --- a/tests/api.go +++ b/tests/api.go @@ -48,7 +48,7 @@ func (scenario *ApiScenario) Test(t *testing.T) { recorder := httptest.NewRecorder() req := httptest.NewRequest(scenario.Method, scenario.Url, scenario.Body) - // add middeware to timeout long running requests (eg. keep-alive routes) + // add middleware to timeout long-running requests (eg. keep-alive routes) e.Pre(func(next echo.HandlerFunc) echo.HandlerFunc { return func(c echo.Context) error { ctx, cancelFunc := context.WithTimeout(c.Request().Context(), 100*time.Millisecond) diff --git a/tools/search/filter.go b/tools/search/filter.go index 0d221c94..9efccee4 100644 --- a/tools/search/filter.go +++ b/tools/search/filter.go @@ -89,15 +89,11 @@ func (f FilterData) resolveTokenizedExpr(expr fexpr.Expr, fieldResolver FieldRes // merge both operands parameters (if any) params := dbx.Params{} - if len(lParams) > 0 { - for k, v := range lParams { - params[k] = v - } + for k, v := range lParams { + params[k] = v } - if len(rParams) > 0 { - for k, v := range rParams { - params[k] = v - } + for k, v := range rParams { + params[k] = v } switch expr.Op { @@ -139,32 +135,26 @@ func (f FilterData) resolveTokenizedExpr(expr fexpr.Expr, fieldResolver FieldRes } func (f FilterData) resolveToken(token fexpr.Token, fieldResolver FieldResolver) (name string, params dbx.Params, err error) { - if token.Type == fexpr.TokenIdentifier { + switch token.Type { + case fexpr.TokenIdentifier: name, params, err := fieldResolver.Resolve(token.Literal) if name == "" || err != nil { - // if `null` field is missing, treat `null` identifier as NULL token - if strings.ToLower(token.Literal) == "null" { - return "NULL", nil, nil + m := map[string]string{ + // if `null` field is missing, treat `null` identifier as NULL token + "null": "NULL", + // if `true` field is missing, treat `true` identifier as TRUE token + "true": "1", + // if `false` field is missing, treat `false` identifier as FALSE token + "false": "0", } - - // if `true` field is missing, treat `true` identifier as TRUE token - if strings.ToLower(token.Literal) == "true" { - return "1", nil, nil + if v, ok := m[strings.ToLower(token.Literal)]; ok { + return v, nil, nil } - - // if `false` field is missing, treat `false` identifier as FALSE token - if strings.ToLower(token.Literal) == "false" { - return "0", nil, nil - } - return "", nil, err } - return name, params, err - } - - if token.Type == fexpr.TokenNumber || token.Type == fexpr.TokenText { + case fexpr.TokenNumber, fexpr.TokenText: placeholder := "t" + security.RandomString(7) name := fmt.Sprintf("{:%s}", placeholder) params := dbx.Params{placeholder: token.Literal} @@ -177,10 +167,6 @@ func (f FilterData) resolveToken(token fexpr.Token, fieldResolver FieldResolver) func (f FilterData) normalizeLikeParams(params dbx.Params) dbx.Params { result := dbx.Params{} - if len(params) == 0 { - return result - } - for k, v := range params { vStr := cast.ToString(v) if !strings.Contains(vStr, "%") { diff --git a/tools/security/encrypt.go b/tools/security/encrypt.go index 2dfe2b9e..2d15b703 100644 --- a/tools/security/encrypt.go +++ b/tools/security/encrypt.go @@ -66,10 +66,5 @@ func Decrypt(cipherText string, key string) ([]byte, error) { } nonce, cipherByteClean := cipherByte[:nonceSize], cipherByte[nonceSize:] - plainData, err := gcm.Open(nil, nonce, cipherByteClean, nil) - if err != nil { - return nil, err - } - - return plainData, nil + return gcm.Open(nil, nonce, cipherByteClean, nil) }