feat: performance improvement

Author: christiangda

internal/core/sync.go

@@ -153,8 +153,6 @@ func (ss *SyncService) SyncGroupsAndTheirMembers(ctx context.Context) error {
 		}
 	}
 
-	// after be sure all the SCIM side is aligned with the identity provider side
-	// we can update the state with the last data coming from the reconciliation
 	newState := model.StateBuilder().
 		WithCodeVersion(version.Version).
 		WithLastSync(time.Now().Format(time.RFC3339)).

internal/core/sync_test.go

@@ -134,7 +134,6 @@ func TestSyncService_SyncGroupsAndTheirMembers(t *testing.T) {
 		assert.Equal(t, 0, len(state.Resources.GroupsMembers.Resources))
 		assert.NotEqual(t, "", state.LastSync)
 		assert.NotEqual(t, "", state.HashCode)
-		assert.Equal(t, "", state.CodeVersion)
 		assert.Equal(t, model.StateSchemaVersion, state.SchemaVersion)
 	})
 
@@ -539,7 +538,6 @@ func TestSyncService_SyncGroupsAndTheirMembers(t *testing.T) {
 		assert.Equal(t, 4, len(state.Resources.GroupsMembers.Resources))
 		assert.NotEqual(t, "", state.LastSync)
 		assert.NotEqual(t, "", state.HashCode)
-		assert.Equal(t, "", state.CodeVersion)
 		assert.Equal(t, model.StateSchemaVersion, state.SchemaVersion)
 		assert.Equal(t, 2, state.Resources.Groups.Items)
 		assert.Equal(t, 2, state.Resources.Users.Items)

internal/idp/idp.go

@@ -34,7 +34,6 @@ type GoogleProviderService interface {
 	GetUser(ctx context.Context, userID string) (*admin.User, error)
 
 	// Batch operations for performance optimization
-	GetUsersBatch(ctx context.Context, emails []string) ([]*admin.User, error)
 	ListGroupMembersBatch(ctx context.Context, groupIDs []string, queries ...google.GetGroupMembersOption) (map[string][]*admin.Member, error)
 }
 
@@ -182,32 +181,27 @@ func (i *IdentityProvider) GetUsersByGroupsMembers(ctx context.Context, gmr *mod
 		return uResult, nil
 	}
 
-	// Collect unique emails first
-	uniqEmails := make(map[string]struct{}, len(gmr.Resources))
-	emails := make([]string, 0, len(gmr.Resources))
+	uniqUsers := make(map[string]struct{}, len(gmr.Resources))
+	pUsers := make([]*model.User, 0, len(gmr.Resources))
 	for _, groupMembers := range gmr.Resources {
 		for _, member := range groupMembers.Resources {
-			if _, ok := uniqEmails[member.Email]; !ok {
-				uniqEmails[member.Email] = struct{}{}
-				emails = append(emails, member.Email)
+			if _, ok := uniqUsers[member.Email]; !ok {
+				uniqUsers[member.Email] = struct{}{}
+
+				u, err := i.ps.GetUser(ctx, member.Email)
+				if err != nil {
+					return nil, fmt.Errorf("idp: error getting user: %+v, email: %s, error: %w", member.IPID, member.Email, err)
+				}
+				gu := buildUser(u)
+
+				pUsers = append(pUsers, gu)
 			}
 		}
 	}
 
-	// Use batch operation instead of individual GetUser calls
-	pUsers, err := i.ps.GetUsersBatch(ctx, emails)
-	if err != nil {
-		return nil, fmt.Errorf("idp: error getting users batch: %w", err)
-	}
-
-	// Convert to model users
-	syncUsers := make([]*model.User, len(pUsers))
-	for i, usr := range pUsers {
-		gu := buildUser(usr)
-		syncUsers[i] = gu
-	}
+	pUsersResult := model.UsersResultBuilder().WithResources(pUsers).Build()
 
-	pUsersResult := model.UsersResultBuilder().WithResources(syncUsers).Build()
+	slog.Debug("idp: GetUsersByGroupsMembers()", "users", len(pUsers))
 
 	return pUsersResult, nil
 }

internal/idp/idp_test.go

@@ -559,7 +559,8 @@ func TestGetUsersByGroupsMembers(t *testing.T) {
 			name: "Should return error",
 			prepare: func(f *fields) {
 				ctx := context.Background()
-				f.ds.EXPECT().GetUsersBatch(ctx, gomock.Eq([]string{"user.1@mail.com"})).Return(nil, errors.New("test error")).Times(1)
+				// Expect a single user fetch and return error
+				f.ds.EXPECT().GetUser(ctx, "user.1@mail.com").Return(nil, errors.New("test error")).Times(1)
 			},
 			args: args{
 				ctx: context.Background(),
@@ -582,6 +583,7 @@ func TestGetUsersByGroupsMembers(t *testing.T) {
 			name: "Should return UsersResult and no error",
 			prepare: func(f *fields) {
 				ctx := context.Background()
+				// Expect individual user fetches for each unique member email
 				googleUser1 := &admin.User{
 					Id:           "1",
 					PrimaryEmail: "user.1@mail.com",
@@ -635,7 +637,10 @@ func TestGetUsersByGroupsMembers(t *testing.T) {
 					// },
 				}
 
-				f.ds.EXPECT().GetUsersBatch(ctx, gomock.Eq([]string{"user.1@mail.com", "user.2@mail.com", "user.3@mail.com", "user.4@mail.com"})).Return([]*admin.User{googleUser1, googleUser2, googleUser3, googleUser4}, nil).Times(1)
+				f.ds.EXPECT().GetUser(ctx, "user.1@mail.com").Return(googleUser1, nil).Times(1)
+				f.ds.EXPECT().GetUser(ctx, "user.2@mail.com").Return(googleUser2, nil).Times(1)
+				f.ds.EXPECT().GetUser(ctx, "user.3@mail.com").Return(googleUser3, nil).Times(1)
+				f.ds.EXPECT().GetUser(ctx, "user.4@mail.com").Return(googleUser4, nil).Times(1)
 			},
 			args: args{
 				ctx: context.Background(),

internal/scim/scim.go

@@ -5,6 +5,7 @@ import (
 	"fmt"
 	"log/slog"
 	"math/rand"
+	"sync"
 	"time"
 
 	"github.com/slashdevops/idp-scim-sync/internal/model"
@@ -516,6 +517,8 @@ func (s *Provider) GetGroupsMembersBruteForce(ctx context.Context, gr *model.Gro
 	// set a limit to the number of concurrent goroutines
 	// to avoid hitting the API rate limit
 	g.SetLimit(5)
+	// protect concurrent access to membersByGroup
+	var mu sync.Mutex
 
 	// brute force implemented here thanks to the fxxckin' aws sso scim api
 	// iterate over each group and user and check if the user is a member of the group
@@ -546,15 +549,18 @@ func (s *Provider) GetGroupsMembersBruteForce(ctx context.Context, gr *model.Gro
 
 				// AWS SSO SCIM API, it doesn't return the member into the Resources array
 				if lgr.TotalResults > 0 {
+					// build member and append to group map
 					m := model.MemberBuilder().
 						WithIPID(user.IPID).
 						WithSCIMID(user.SCIMID).
 						WithEmail(user.Emails[0].Value).
 						Build()
-
 					if user.Active {
 						m.Status = "ACTIVE"
 					}
+					mu.Lock()
+					membersByGroup[group.SCIMID] = append(membersByGroup[group.SCIMID], m)
+					mu.Unlock()
 				}
 
 				return nil

mocks/idp/idp_mocks.go

@@ -5,7 +5,6 @@ import (
 	"fmt"
 	"log/slog"
 	"net/http"
-	"strings"
 	"sync"
 
 	"golang.org/x/oauth2"
@@ -143,7 +142,9 @@ func (ds *DirectoryService) ListUsers(ctx context.Context, query []string) ([]*a
 	if len(query) > 0 {
 		for _, q := range query {
 			if q != "" {
+				slog.Debug("google: Listing users with query", "query", q)
 				err := ds.svc.Users.List().Query(q).Customer("my_customer").Fields(listUsersRequiredFields).Pages(ctx, func(users *admin.Users) error {
+					slog.Debug("google: Retrieved users page", "page_size", len(users.Users))
 					u = append(u, users.Users...)
 					return nil
 				})
@@ -304,92 +305,6 @@ func (ds *DirectoryService) GetGroup(ctx context.Context, groupID string) (*admi
 	return g, nil
 }
 
-// GetUsersBatch retrieves multiple users by their email addresses using batch queries.
-// This method is optimized for performance by using multiple ListUsers calls with email queries
-// instead of making individual GetUser calls.
-func (ds *DirectoryService) GetUsersBatch(ctx context.Context, emails []string) ([]*admin.User, error) {
-	if len(emails) == 0 {
-		return []*admin.User{}, nil
-	}
-
-	// Use chunked approach for better reliability and performance
-	// Optimal chunk size balances API complexity vs number of requests
-	const chunkSize = 15
-
-	var allUsers []*admin.User
-
-	slog.Debug("google: GetUsersBatch starting chunked processing", "total_emails", len(emails), "chunk_size", chunkSize) // Process emails in chunks to avoid overly complex OR queries
-	for i := 0; i < len(emails); i += chunkSize {
-		end := i + chunkSize
-		if end > len(emails) {
-			end = len(emails)
-		}
-
-		chunk := emails[i:end]
-
-		// Create OR query for this chunk
-		emailQueries := make([]string, len(chunk))
-		for j, email := range chunk {
-			emailQueries[j] = fmt.Sprintf("email:%s", email)
-		}
-
-		query := strings.Join(emailQueries, " OR ")
-
-		slog.Debug("google: processing chunk", "chunk_index", i/chunkSize+1, "chunk_size", len(chunk), "chunk_start", i)
-
-		// Execute ListUsers for this chunk
-		users, err := ds.ListUsers(ctx, []string{query})
-		if err != nil {
-			// If chunk query fails, fall back to individual calls for this chunk only
-			slog.Warn("google: GetUsersBatch chunk failed, falling back to individual calls for chunk",
-				"error", err, "chunk_size", len(chunk), "chunk_start", i)
-
-			individualUsers, individualErr := ds.getUsersIndividually(ctx, chunk)
-			if individualErr != nil {
-				return nil, fmt.Errorf("google: both batch and individual calls failed: batch_error=%w, individual_error=%v", err, individualErr)
-			}
-			users = individualUsers
-		} else if len(users) == 0 {
-			// If chunk query succeeds but returns no users, also fall back to individual calls
-			slog.Warn("google: GetUsersBatch chunk returned no users, falling back to individual calls for chunk",
-				"chunk_size", len(chunk), "chunk_start", i)
-
-			individualUsers, individualErr := ds.getUsersIndividually(ctx, chunk)
-			if individualErr != nil {
-				slog.Warn("google: individual calls also failed for chunk", "error", individualErr)
-				// Continue with empty users rather than failing completely
-			} else {
-				users = individualUsers
-			}
-		}
-
-		// Accumulate results from this chunk
-		allUsers = append(allUsers, users...)
-		slog.Debug("google: chunk completed", "chunk_users", len(users), "total_users_so_far", len(allUsers))
-	}
-
-	slog.Debug("google: GetUsersBatch completed", "total_users_returned", len(allUsers), "total_emails_requested", len(emails))
-
-	return allUsers, nil
-}
-
-// getUsersIndividually retrieves users one by one using GetUser calls
-func (ds *DirectoryService) getUsersIndividually(ctx context.Context, emails []string) ([]*admin.User, error) {
-	users := make([]*admin.User, 0, len(emails))
-
-	for _, email := range emails {
-		user, err := ds.GetUser(ctx, email)
-		if err != nil {
-			// Log the error but continue with other users
-			slog.Warn("google: failed to get individual user", "email", email, "error", err)
-			continue
-		}
-		users = append(users, user)
-	}
-
-	return users, nil
-}
-
 // ListGroupMembersBatch retrieves members for multiple groups concurrently.
 // Returns a map where keys are group IDs and values are slices of members.
 func (ds *DirectoryService) ListGroupMembersBatch(ctx context.Context, groupIDs []string, queries ...GetGroupMembersOption) (map[string][]*admin.Member, error) {