From 11f69b8c38fb0170aee9654617cf578060df18c7 Mon Sep 17 00:00:00 2001 From: Max Date: Sat, 2 Aug 2025 18:10:04 +0800 Subject: [PATCH] Update go-nanoid dependency and implement user retrieval methods - Replaced the jaevor/go-nanoid library with the updated matoous/go-nanoid/v2 for generating unique IDs. - Implemented user retrieval methods in the user provider, including GetUsers, PaginateUsers, and CountUsers, enhancing user management capabilities. - Improved error handling and ensured default select fields are set for user queries, streamlining the user data access process. --- go.mod | 2 +- go.sum | 4 +- openapi/oauth/providers/user/user_list.go | 65 ++- .../oauth/providers/user/user_list_test.go | 374 ++++++++++++++++++ openapi/oauth/providers/user/utils.go | 8 +- 5 files changed, 438 insertions(+), 15 deletions(-) create mode 100644 openapi/oauth/providers/user/user_list_test.go diff --git a/go.mod b/go.mod index fa1d42f4..58f02a51 100644 --- a/go.mod +++ b/go.mod @@ -21,10 +21,10 @@ require ( github.com/golang-jwt/jwt/v4 v4.5.2 github.com/google/uuid v1.6.0 github.com/hashicorp/go-multierror v1.1.1 - github.com/jaevor/go-nanoid v1.4.0 github.com/joho/godotenv v1.5.1 github.com/json-iterator/go v1.1.12 github.com/kaptinlin/jsonrepair v0.1.1 + github.com/matoous/go-nanoid/v2 v2.1.0 github.com/mozillazg/go-pinyin v0.20.0 github.com/pkoukk/tiktoken-go v0.1.7 github.com/rhysd/go-github-selfupdate v1.2.3 diff --git a/go.sum b/go.sum index d718491a..1997f9cb 100644 --- a/go.sum +++ b/go.sum @@ -160,8 +160,6 @@ github.com/inconshreveable/go-update v0.0.0-20160112193335-8152e7eb6ccf h1:WfD7V github.com/inconshreveable/go-update v0.0.0-20160112193335-8152e7eb6ccf/go.mod h1:hyb9oH7vZsitZCiBt0ZvifOrB+qc8PS5IiilCIb87rg= github.com/inconshreveable/mousetrap v1.1.0 h1:wN+x4NVGpMsO7ErUn/mUI3vEoE6Jt13X2s0bqwp9tc8= github.com/inconshreveable/mousetrap v1.1.0/go.mod h1:vpF70FUmC8bwa3OWnCshd2FqLfsEA9PFc4w1p2J65bw= -github.com/jaevor/go-nanoid v1.4.0 h1:mPz0oi3CrQyEtRxeRq927HHtZCJAAtZ7zdy7vOkrvWs= -github.com/jaevor/go-nanoid v1.4.0/go.mod h1:GIpPtsvl3eSBsjjIEFQdzzgpi50+Bo1Luk+aYlbJzlc= github.com/jhump/protoreflect v1.15.1 h1:HUMERORf3I3ZdX05WaQ6MIpd/NJ434hTp5YiKgfCL6c= github.com/jhump/protoreflect v1.15.1/go.mod h1:jD/2GMKKE6OqX8qTjhADU1e6DShO+gavG9e0Q693nKo= github.com/jmoiron/sqlx v1.4.0 h1:1PLqN7S1UYp5t4SrVVnt4nUVNemrDAtxlulVe+Qgm3o= @@ -191,6 +189,8 @@ github.com/lib/pq v1.10.9 h1:YXG7RB+JIjhP29X+OtkiDnYaXQwpS4JEWq7dtCCRUEw= github.com/lib/pq v1.10.9/go.mod h1:AlVN5x4E4T544tWzH6hKfbfQvm3HdbOxrmggDNAPY9o= github.com/mark3labs/mcp-go v0.32.0 h1:fgwmbfL2gbd67obg57OfV2Dnrhs1HtSdlY/i5fn7MU8= github.com/mark3labs/mcp-go v0.32.0/go.mod h1:rXqOudj/djTORU/ThxYx8fqEVj/5pvTuuebQ2RC7uk4= +github.com/matoous/go-nanoid/v2 v2.1.0 h1:P64+dmq21hhWdtvZfEAofnvJULaRR1Yib0+PnU669bE= +github.com/matoous/go-nanoid/v2 v2.1.0/go.mod h1:KlbGNQ+FhrUNIHUxZdL63t7tl4LaPkZNpUULS8H4uVM= github.com/mattn/go-colorable v0.1.9/go.mod h1:u6P/XSegPjTcexA+o6vUJrdnUu04hMope9wVRipJSqc= github.com/mattn/go-colorable v0.1.12/go.mod h1:u5H1YNBxpqRaxsYJYSkiCWKzEfiAb1Gb520KVy5xxl4= github.com/mattn/go-colorable v0.1.14 h1:9A9LHSqF/7dyVVX6g0U9cwm9pG3kP9gSzcuIPHPsaIE= diff --git a/openapi/oauth/providers/user/user_list.go b/openapi/oauth/providers/user/user_list.go index 42a35a9c..964b9c7c 100644 --- a/openapi/oauth/providers/user/user_list.go +++ b/openapi/oauth/providers/user/user_list.go @@ -2,6 +2,7 @@ package user import ( "context" + "fmt" "github.com/yaoapp/gou/model" "github.com/yaoapp/kun/maps" @@ -11,18 +12,70 @@ import ( // GetUsers retrieves users by query parameters (compatible with Model.Get) func (u *DefaultUser) GetUsers(ctx context.Context, param model.QueryParam) ([]maps.MapStr, error) { - // TODO: implement - return nil, nil + // Set default select fields if not provided + if param.Select == nil { + param.Select = u.basicUserFields + } + + m := model.Select(u.model) + users, err := m.Get(param) + if err != nil { + return nil, fmt.Errorf(ErrFailedToGetUser, err) + } + + return users, nil } // PaginateUsers retrieves paginated list of users (compatible with Model.Paginate) func (u *DefaultUser) PaginateUsers(ctx context.Context, param model.QueryParam, page int, pagesize int) (maps.MapStr, error) { - // TODO: implement - return nil, nil + // Set default select fields if not provided + if param.Select == nil { + param.Select = u.basicUserFields + } + + m := model.Select(u.model) + result, err := m.Paginate(param, page, pagesize) + if err != nil { + return nil, fmt.Errorf(ErrFailedToGetUser, err) + } + + return result, nil } // CountUsers returns total count of users with optional filters func (u *DefaultUser) CountUsers(ctx context.Context, param model.QueryParam) (int64, error) { - // TODO: implement - return 0, nil + // Use Paginate with a small page size to get the total count + // This is more reliable than manual COUNT(*) queries + m := model.Select(u.model) + result, err := m.Paginate(param, 1, 1) // Get first page with 1 item to get total + if err != nil { + return 0, fmt.Errorf(ErrFailedToGetUser, err) + } + + // Extract total from pagination result + if total, ok := result["total"].(int64); ok { + return total, nil + } + + // Handle different total types returned by Paginate + if totalInterface, ok := result["total"]; ok { + switch v := totalInterface.(type) { + case int: + return int64(v), nil + case int32: + return int64(v), nil + case int64: + return v, nil + case uint: + return int64(v), nil + case uint32: + return int64(v), nil + case uint64: + return int64(v), nil + default: + return 0, fmt.Errorf("unexpected total type: %T", totalInterface) + } + } + + return 0, fmt.Errorf("total not found in pagination result") } diff --git a/openapi/oauth/providers/user/user_list_test.go b/openapi/oauth/providers/user/user_list_test.go new file mode 100644 index 00000000..869788b1 --- /dev/null +++ b/openapi/oauth/providers/user/user_list_test.go @@ -0,0 +1,374 @@ +package user_test + +import ( + "context" + "strings" + "testing" + + "github.com/google/uuid" + "github.com/stretchr/testify/assert" + "github.com/yaoapp/gou/model" + "github.com/yaoapp/kun/maps" +) + +func TestUserListOperations(t *testing.T) { + prepare(t) + defer clean() + + ctx := context.Background() + + // Create multiple test users for list operations + // Use UUID to ensure unique identifiers + testUUID := strings.ReplaceAll(uuid.New().String(), "-", "")[:8] // 8 char UUID + testUsers := []*TestUserData{ + createTestUserData("listops" + testUUID + "01"), + createTestUserData("listops" + testUUID + "02"), + createTestUserData("listops" + testUUID + "03"), + createTestUserData("listops" + testUUID + "04"), + createTestUserData("listops" + testUUID + "05"), + } + + // Store created user IDs + userIDs := make([]string, len(testUsers)) + + // Create test users with varied data for testing + for i, userData := range testUsers { + // Vary some data for testing filters + if i%2 == 0 { + userData.Status = "active" + userData.RoleID = "admin" + } else { + userData.Status = "pending" + userData.RoleID = "user" + } + + _, userID := setupTestUser(t, ctx, userData) + userIDs[i] = userID + } + + // Test GetUsers + t.Run("GetUsers_All", func(t *testing.T) { + param := model.QueryParam{ + Wheres: []model.QueryWhere{ + {Column: "user_id", OP: "like", Value: "test_%"}, + }, + } + users, err := testProvider.GetUsers(ctx, param) + assert.NoError(t, err) + assert.GreaterOrEqual(t, len(users), 5) // At least our 5 test users + + // Check that basic fields are returned by default + if len(users) > 0 { + user := users[0] + assert.Contains(t, user, "user_id") + assert.Contains(t, user, "preferred_username") + // Should not contain sensitive fields in basic view + assert.NotContains(t, user, "password_hash") + } + }) + + t.Run("GetUsers_WithFilters", func(t *testing.T) { + param := model.QueryParam{ + Wheres: []model.QueryWhere{ + {Column: "user_id", OP: "like", Value: "test_%"}, + {Column: "status", Value: "active"}, + }, + } + users, err := testProvider.GetUsers(ctx, param) + assert.NoError(t, err) + assert.GreaterOrEqual(t, len(users), 3) // At least 3 active users (indexes 0, 2, 4) + + // All returned users should be active + for _, user := range users { + if userID, ok := user["user_id"].(string); ok { + // Only check our test users + for _, testUserID := range userIDs { + if userID == testUserID { + assert.Equal(t, "active", user["status"]) + break + } + } + } + } + }) + + t.Run("GetUsers_WithCustomFields", func(t *testing.T) { + param := model.QueryParam{ + Select: []interface{}{"user_id", "preferred_username", "status"}, + Wheres: []model.QueryWhere{ + {Column: "user_id", OP: "like", Value: "test_%"}, + }, + Limit: 2, + } + users, err := testProvider.GetUsers(ctx, param) + assert.NoError(t, err) + assert.LessOrEqual(t, len(users), 2) // Respects limit + + if len(users) > 0 { + user := users[0] + assert.Contains(t, user, "user_id") + assert.Contains(t, user, "preferred_username") + assert.Contains(t, user, "status") + } + }) + + t.Run("GetUsers_WithOrdering", func(t *testing.T) { + param := model.QueryParam{ + Wheres: []model.QueryWhere{ + {Column: "user_id", OP: "like", Value: "test_%"}, + }, + Orders: []model.QueryOrder{ + {Column: "preferred_username", Option: "asc"}, + }, + Limit: 3, + } + users, err := testProvider.GetUsers(ctx, param) + assert.NoError(t, err) + assert.LessOrEqual(t, len(users), 3) + + // Check ordering (should be sorted by preferred_username ascending) + if len(users) >= 2 { + first := users[0]["preferred_username"].(string) + second := users[1]["preferred_username"].(string) + assert.LessOrEqual(t, first, second) + } + }) + + // Test PaginateUsers + t.Run("PaginateUsers_FirstPage", func(t *testing.T) { + param := model.QueryParam{ + Wheres: []model.QueryWhere{ + {Column: "user_id", OP: "like", Value: "test_%"}, + }, + Orders: []model.QueryOrder{ + {Column: "preferred_username", Option: "asc"}, + }, + } + result, err := testProvider.PaginateUsers(ctx, param, 1, 3) + assert.NoError(t, err) + assert.NotNil(t, result) + + // Check pagination structure + assert.Contains(t, result, "data") + assert.Contains(t, result, "total") + assert.Contains(t, result, "page") + assert.Contains(t, result, "pagesize") + + data, ok := result["data"].([]maps.MapStr) + assert.True(t, ok) + assert.LessOrEqual(t, len(data), 3) // Page size limit + + // Handle different total types + totalInterface, exists := result["total"] + assert.True(t, exists) + + var total int64 + switch v := totalInterface.(type) { + case int: + total = int64(v) + case int32: + total = int64(v) + case int64: + total = v + case uint: + total = int64(v) + case uint32: + total = int64(v) + case uint64: + total = int64(v) + default: + t.Errorf("unexpected total type: %T, value: %v", totalInterface, totalInterface) + } + assert.GreaterOrEqual(t, total, int64(0)) // Could be 0 or more + + assert.Equal(t, 1, result["page"]) + assert.Equal(t, 3, result["pagesize"]) + }) + + t.Run("PaginateUsers_SecondPage", func(t *testing.T) { + param := model.QueryParam{ + Wheres: []model.QueryWhere{ + {Column: "user_id", OP: "like", Value: "test_%"}, + }, + Orders: []model.QueryOrder{ + {Column: "preferred_username", Option: "asc"}, + }, + } + result, err := testProvider.PaginateUsers(ctx, param, 2, 3) + assert.NoError(t, err) + assert.NotNil(t, result) + + assert.Equal(t, 2, result["page"]) + assert.Equal(t, 3, result["pagesize"]) + + _, ok := result["data"].([]maps.MapStr) + assert.True(t, ok) + // Second page may have fewer items depending on total count + }) + + t.Run("PaginateUsers_WithFilters", func(t *testing.T) { + param := model.QueryParam{ + Wheres: []model.QueryWhere{ + {Column: "user_id", OP: "like", Value: "test_%"}, + {Column: "role_id", Value: "admin"}, + }, + } + result, err := testProvider.PaginateUsers(ctx, param, 1, 10) + assert.NoError(t, err) + assert.NotNil(t, result) + + data, ok := result["data"].([]maps.MapStr) + assert.True(t, ok) + assert.GreaterOrEqual(t, len(data), 0) // Could be 0 or more + + // Verify admin filter works by checking our test users only + for _, user := range data { + if userID, ok := user["user_id"].(string); ok { + // Only check our test users + for _, testUserID := range userIDs { + if userID == testUserID { + assert.Equal(t, "admin", user["role_id"]) + break + } + } + } + } + }) + + // Test CountUsers + t.Run("CountUsers_All", func(t *testing.T) { + param := model.QueryParam{ + Wheres: []model.QueryWhere{ + {Column: "user_id", OP: "like", Value: "test_%"}, + }, + } + count, err := testProvider.CountUsers(ctx, param) + assert.NoError(t, err) + assert.GreaterOrEqual(t, count, int64(0)) // At least 0 users + }) + + t.Run("CountUsers_WithFilters", func(t *testing.T) { + param := model.QueryParam{ + Wheres: []model.QueryWhere{ + {Column: "user_id", OP: "like", Value: "test_%"}, + {Column: "status", Value: "active"}, + }, + } + count, err := testProvider.CountUsers(ctx, param) + assert.NoError(t, err) + assert.GreaterOrEqual(t, count, int64(3)) // At least 3 active users (indexes 0, 2, 4) + }) + + t.Run("CountUsers_SpecificRole", func(t *testing.T) { + param := model.QueryParam{ + Wheres: []model.QueryWhere{ + {Column: "user_id", OP: "like", Value: "test_%"}, + {Column: "role_id", Value: "user"}, + }, + } + count, err := testProvider.CountUsers(ctx, param) + assert.NoError(t, err) + assert.GreaterOrEqual(t, count, int64(2)) // At least 2 regular users (indexes 1, 3) + }) + + t.Run("CountUsers_NoResults", func(t *testing.T) { + param := model.QueryParam{ + Wheres: []model.QueryWhere{ + {Column: "user_id", Value: "nonexistent_user_id"}, + }, + } + count, err := testProvider.CountUsers(ctx, param) + assert.NoError(t, err) + assert.Equal(t, int64(0), count) + }) +} + +func TestUserListErrorHandling(t *testing.T) { + prepare(t) + defer clean() + + ctx := context.Background() + + // Use UUID for unique test identifiers + testUUID := strings.ReplaceAll(uuid.New().String(), "-", "")[:8] + + t.Run("GetUsers_EmptyResult", func(t *testing.T) { + param := model.QueryParam{ + Wheres: []model.QueryWhere{ + {Column: "user_id", Value: "nonexistent_" + testUUID}, + }, + } + users, err := testProvider.GetUsers(ctx, param) + assert.NoError(t, err) + assert.Equal(t, 0, len(users)) // Empty slice, not nil + }) + + t.Run("PaginateUsers_EmptyResult", func(t *testing.T) { + param := model.QueryParam{ + Wheres: []model.QueryWhere{ + {Column: "user_id", Value: "nonexistent_" + testUUID}, + }, + } + result, err := testProvider.PaginateUsers(ctx, param, 1, 10) + assert.NoError(t, err) + assert.NotNil(t, result) + + data, ok := result["data"].([]maps.MapStr) + assert.True(t, ok) + assert.Equal(t, 0, len(data)) + + // Handle different total types + totalInterface, exists := result["total"] + assert.True(t, exists) + + var total int64 + switch v := totalInterface.(type) { + case int: + total = int64(v) + case int32: + total = int64(v) + case int64: + total = v + case uint: + total = int64(v) + case uint32: + total = int64(v) + case uint64: + total = int64(v) + default: + t.Errorf("unexpected total type: %T, value: %v", totalInterface, totalInterface) + } + assert.Equal(t, int64(0), total) + }) + + t.Run("PaginateUsers_LargePage", func(t *testing.T) { + param := model.QueryParam{ + Wheres: []model.QueryWhere{ + {Column: "user_id", OP: "like", Value: "test_%"}, + }, + } + result, err := testProvider.PaginateUsers(ctx, param, 100, 10) // Page way beyond data + assert.NoError(t, err) + assert.NotNil(t, result) + + data, ok := result["data"].([]maps.MapStr) + assert.True(t, ok) + assert.Equal(t, 0, len(data)) // No data on this page + + assert.Equal(t, 100, result["page"]) + assert.Equal(t, 10, result["pagesize"]) + }) + + t.Run("CountUsers_ComplexFilters", func(t *testing.T) { + param := model.QueryParam{ + Wheres: []model.QueryWhere{ + {Column: "user_id", OP: "like", Value: "test_%"}, + {Column: "status", OP: "in", Value: []interface{}{"active", "pending"}}, + {Column: "email_verified", Value: true}, + }, + } + count, err := testProvider.CountUsers(ctx, param) + assert.NoError(t, err) + assert.GreaterOrEqual(t, count, int64(0)) // Should handle complex filters without error + }) +} diff --git a/openapi/oauth/providers/user/utils.go b/openapi/oauth/providers/user/utils.go index b9fc0070..bbf7caa5 100644 --- a/openapi/oauth/providers/user/utils.go +++ b/openapi/oauth/providers/user/utils.go @@ -6,7 +6,7 @@ import ( "fmt" "github.com/google/uuid" - "github.com/jaevor/go-nanoid" + gonanoid "github.com/matoous/go-nanoid/v2" "github.com/yaoapp/gou/model" ) @@ -133,11 +133,7 @@ func (u *DefaultUser) GetOAuthUserID(ctx context.Context, provider string, subje func generateNanoID(length int) (string, error) { // URL-safe alphabet (no ambiguous characters like 0/O, 1/l/I) const alphabet = "23456789ABCDEFGHJKMNPQRSTUVWXYZabcdefghijkmnpqrstuvwxyz" - nanoidGen, err := nanoid.CustomASCII(alphabet, length) - if err != nil { - return "", err - } - return nanoidGen(), nil + return gonanoid.Generate(alphabet, length) } // generateUUID generates a traditional UUID using Google's library