Implement GET /users endpoint for APIv4 (#5277)

3 files changed, 151 insertions, 40 deletions

diff --git a/store/sql_user_store.go b/store/sql_user_store.go
index 827c5a064..7ca33fd78 100644
--- a/store/sql_user_store.go
+++ b/store/sql_user_store.go
@@ -510,16 +510,13 @@ func (us SqlUserStore) GetAllProfiles(offset int, limit int) StoreChannel {
 			result.Err = model.NewLocAppError("SqlUserStore.GetAllProfiles", "store.sql_user.get_profiles.app_error", nil, err.Error())
 		} else {
 
-			userMap := make(map[string]*model.User)
-
 			for _, u := range users {
 				u.Password = ""
 				u.AuthData = new(string)
 				*u.AuthData = ""
-				userMap[u.Id] = u
 			}
 
-			result.Data = userMap
+			result.Data = users
 		}
 
 		storeChannel <- result
@@ -562,16 +559,13 @@ func (us SqlUserStore) GetProfiles(teamId string, offset int, limit int) StoreCh
 			result.Err = model.NewLocAppError("SqlUserStore.GetProfiles", "store.sql_user.get_profiles.app_error", nil, err.Error())
 		} else {
 
-			userMap := make(map[string]*model.User)
-
 			for _, u := range users {
 				u.Password = ""
 				u.AuthData = new(string)
 				*u.AuthData = ""
-				userMap[u.Id] = u
 			}
 
-			result.Data = userMap
+			result.Data = users
 		}
 
 		storeChannel <- result
@@ -598,7 +592,38 @@ func (us SqlUserStore) InvalidateProfilesInChannelCache(channelId string) {
 	profilesInChannelCache.Remove(channelId)
 }
 
-func (us SqlUserStore) GetProfilesInChannel(channelId string, offset int, limit int, allowFromCache bool) StoreChannel {
+func (us SqlUserStore) GetProfilesInChannel(channelId string, offset int, limit int) StoreChannel {
+
+	storeChannel := make(StoreChannel)
+
+	go func() {
+		result := StoreResult{}
+
+		var users []*model.User
+
+		query := "SELECT Users.* FROM Users, ChannelMembers WHERE ChannelMembers.ChannelId = :ChannelId AND Users.Id = ChannelMembers.UserId AND Users.DeleteAt = 0 ORDER BY Users.Username ASC LIMIT :Limit OFFSET :Offset"
+
+		if _, err := us.GetReplica().Select(&users, query, map[string]interface{}{"ChannelId": channelId, "Offset": offset, "Limit": limit}); err != nil {
+			result.Err = model.NewLocAppError("SqlUserStore.GetProfilesInChannel", "store.sql_user.get_profiles.app_error", nil, err.Error())
+		} else {
+
+			for _, u := range users {
+				u.Password = ""
+				u.AuthData = new(string)
+				*u.AuthData = ""
+			}
+
+			result.Data = users
+		}
+
+		storeChannel <- result
+		close(storeChannel)
+	}()
+
+	return storeChannel
+}
+
+func (us SqlUserStore) GetAllProfilesInChannel(channelId string, allowFromCache bool) StoreChannel {
 
 	storeChannel := make(StoreChannel)
 
@@ -606,7 +631,7 @@ func (us SqlUserStore) GetProfilesInChannel(channelId string, offset int, limit
 		result := StoreResult{}
 		metrics := einterfaces.GetMetricsInterface()
 
-		if allowFromCache && offset == -1 && limit == -1 {
+		if allowFromCache {
 			if cacheItem, ok := profilesInChannelCache.Get(channelId); ok {
 				if metrics != nil {
 					metrics.IncrementMemCacheHitCounter("Profiles in Channel")
@@ -630,12 +655,8 @@ func (us SqlUserStore) GetProfilesInChannel(channelId string, offset int, limit
 
 		query := "SELECT Users.* FROM Users, ChannelMembers WHERE ChannelMembers.ChannelId = :ChannelId AND Users.Id = ChannelMembers.UserId AND Users.DeleteAt = 0"
 
-		if limit >= 0 && offset >= 0 {
-			query += " ORDER BY Users.Username ASC LIMIT :Limit OFFSET :Offset"
-		}
-
-		if _, err := us.GetReplica().Select(&users, query, map[string]interface{}{"ChannelId": channelId, "Offset": offset, "Limit": limit}); err != nil {
-			result.Err = model.NewLocAppError("SqlUserStore.GetProfilesInChannel", "store.sql_user.get_profiles.app_error", nil, err.Error())
+		if _, err := us.GetReplica().Select(&users, query, map[string]interface{}{"ChannelId": channelId}); err != nil {
+			result.Err = model.NewLocAppError("SqlUserStore.GetAllProfilesInChannel", "store.sql_user.get_profiles.app_error", nil, err.Error())
 		} else {
 
 			userMap := make(map[string]*model.User)
@@ -649,7 +670,7 @@ func (us SqlUserStore) GetProfilesInChannel(channelId string, offset int, limit
 
 			result.Data = userMap
 
-			if allowFromCache && offset == -1 && limit == -1 {
+			if allowFromCache {
 				profilesInChannelCache.AddWithExpiresInSecs(channelId, userMap, PROFILES_IN_CHANNEL_CACHE_SEC)
 			}
 		}
@@ -688,16 +709,13 @@ func (us SqlUserStore) GetProfilesNotInChannel(teamId string, channelId string,
 			result.Err = model.NewLocAppError("SqlUserStore.GetProfilesNotInChannel", "store.sql_user.get_profiles.app_error", nil, err.Error())
 		} else {
 
-			userMap := make(map[string]*model.User)
-
 			for _, u := range users {
 				u.Password = ""
 				u.AuthData = new(string)
 				*u.AuthData = ""
-				userMap[u.Id] = u
 			}
 
-			result.Data = userMap
+			result.Data = users
 		}
 
 		storeChannel <- result
diff --git a/store/sql_user_store_test.go b/store/sql_user_store_test.go
index 449c6aa52..95be3e258 100644
--- a/store/sql_user_store_test.go
+++ b/store/sql_user_store_test.go
@@ -208,7 +208,7 @@ func TestUserStoreGetAllProfiles(t *testing.T) {
 	if r1 := <-store.User().GetAllProfiles(0, 100); r1.Err != nil {
 		t.Fatal(r1.Err)
 	} else {
-		users := r1.Data.(map[string]*model.User)
+		users := r1.Data.([]*model.User)
 		if len(users) < 2 {
 			t.Fatal("invalid returned users")
 		}
@@ -217,7 +217,7 @@ func TestUserStoreGetAllProfiles(t *testing.T) {
 	if r2 := <-store.User().GetAllProfiles(0, 1); r2.Err != nil {
 		t.Fatal(r2.Err)
 	} else {
-		users := r2.Data.(map[string]*model.User)
+		users := r2.Data.([]*model.User)
 		if len(users) != 1 {
 			t.Fatal("invalid returned users, limit did not work")
 		}
@@ -242,20 +242,27 @@ func TestUserStoreGetProfiles(t *testing.T) {
 	if r1 := <-store.User().GetProfiles(teamId, 0, 100); r1.Err != nil {
 		t.Fatal(r1.Err)
 	} else {
-		users := r1.Data.(map[string]*model.User)
+		users := r1.Data.([]*model.User)
 		if len(users) != 2 {
 			t.Fatal("invalid returned users")
 		}
 
-		if users[u1.Id].Id != u1.Id {
-			t.Fatal("invalid returned user")
+		found := false
+		for _, u := range users {
+			if u.Id == u1.Id {
+				found = true
+			}
+		}
+
+		if !found {
+			t.Fatal("missing user")
 		}
 	}
 
 	if r2 := <-store.User().GetProfiles("123", 0, 100); r2.Err != nil {
 		t.Fatal(r2.Err)
 	} else {
-		if len(r2.Data.(map[string]*model.User)) != 0 {
+		if len(r2.Data.([]*model.User)) != 0 {
 			t.Fatal("should have returned empty map")
 		}
 	}
@@ -310,7 +317,85 @@ func TestUserStoreGetProfilesInChannel(t *testing.T) {
 	Must(store.Channel().SaveMember(&m2))
 	Must(store.Channel().SaveMember(&m3))
 
-	if r1 := <-store.User().GetProfilesInChannel(c1.Id, -1, -1, false); r1.Err != nil {
+	if r1 := <-store.User().GetProfilesInChannel(c1.Id, 0, 100); r1.Err != nil {
+		t.Fatal(r1.Err)
+	} else {
+		users := r1.Data.([]*model.User)
+		if len(users) != 2 {
+			t.Fatal("invalid returned users")
+		}
+
+		found := false
+		for _, u := range users {
+			if u.Id == u1.Id {
+				found = true
+			}
+		}
+
+		if !found {
+			t.Fatal("missing user")
+		}
+	}
+
+	if r2 := <-store.User().GetProfilesInChannel(c2.Id, 0, 1); r2.Err != nil {
+		t.Fatal(r2.Err)
+	} else {
+		if len(r2.Data.([]*model.User)) != 1 {
+			t.Fatal("should have returned only 1 user")
+		}
+	}
+}
+
+func TestUserStoreGetAllProfilesInChannel(t *testing.T) {
+	Setup()
+
+	teamId := model.NewId()
+
+	u1 := &model.User{}
+	u1.Email = model.NewId()
+	Must(store.User().Save(u1))
+	Must(store.Team().SaveMember(&model.TeamMember{TeamId: teamId, UserId: u1.Id}))
+
+	u2 := &model.User{}
+	u2.Email = model.NewId()
+	Must(store.User().Save(u2))
+	Must(store.Team().SaveMember(&model.TeamMember{TeamId: teamId, UserId: u2.Id}))
+
+	c1 := model.Channel{}
+	c1.TeamId = teamId
+	c1.DisplayName = "Profiles in channel"
+	c1.Name = "profiles-" + model.NewId()
+	c1.Type = model.CHANNEL_OPEN
+
+	c2 := model.Channel{}
+	c2.TeamId = teamId
+	c2.DisplayName = "Profiles in private"
+	c2.Name = "profiles-" + model.NewId()
+	c2.Type = model.CHANNEL_PRIVATE
+
+	Must(store.Channel().Save(&c1))
+	Must(store.Channel().Save(&c2))
+
+	m1 := model.ChannelMember{}
+	m1.ChannelId = c1.Id
+	m1.UserId = u1.Id
+	m1.NotifyProps = model.GetDefaultChannelNotifyProps()
+
+	m2 := model.ChannelMember{}
+	m2.ChannelId = c1.Id
+	m2.UserId = u2.Id
+	m2.NotifyProps = model.GetDefaultChannelNotifyProps()
+
+	m3 := model.ChannelMember{}
+	m3.ChannelId = c2.Id
+	m3.UserId = u1.Id
+	m3.NotifyProps = model.GetDefaultChannelNotifyProps()
+
+	Must(store.Channel().SaveMember(&m1))
+	Must(store.Channel().SaveMember(&m2))
+	Must(store.Channel().SaveMember(&m3))
+
+	if r1 := <-store.User().GetAllProfilesInChannel(c1.Id, false); r1.Err != nil {
 		t.Fatal(r1.Err)
 	} else {
 		users := r1.Data.(map[string]*model.User)
@@ -323,7 +408,7 @@ func TestUserStoreGetProfilesInChannel(t *testing.T) {
 		}
 	}
 
-	if r2 := <-store.User().GetProfilesInChannel(c2.Id, -1, -1, false); r2.Err != nil {
+	if r2 := <-store.User().GetAllProfilesInChannel(c2.Id, false); r2.Err != nil {
 		t.Fatal(r2.Err)
 	} else {
 		if len(r2.Data.(map[string]*model.User)) != 1 {
@@ -331,7 +416,7 @@ func TestUserStoreGetProfilesInChannel(t *testing.T) {
 		}
 	}
 
-	if r2 := <-store.User().GetProfilesInChannel(c2.Id, -1, -1, true); r2.Err != nil {
+	if r2 := <-store.User().GetAllProfilesInChannel(c2.Id, true); r2.Err != nil {
 		t.Fatal(r2.Err)
 	} else {
 		if len(r2.Data.(map[string]*model.User)) != 1 {
@@ -339,7 +424,7 @@ func TestUserStoreGetProfilesInChannel(t *testing.T) {
 		}
 	}
 
-	if r2 := <-store.User().GetProfilesInChannel(c2.Id, -1, -1, true); r2.Err != nil {
+	if r2 := <-store.User().GetAllProfilesInChannel(c2.Id, true); r2.Err != nil {
 		t.Fatal(r2.Err)
 	} else {
 		if len(r2.Data.(map[string]*model.User)) != 1 {
@@ -383,20 +468,27 @@ func TestUserStoreGetProfilesNotInChannel(t *testing.T) {
 	if r1 := <-store.User().GetProfilesNotInChannel(teamId, c1.Id, 0, 100); r1.Err != nil {
 		t.Fatal(r1.Err)
 	} else {
-		users := r1.Data.(map[string]*model.User)
+		users := r1.Data.([]*model.User)
 		if len(users) != 2 {
 			t.Fatal("invalid returned users")
 		}
 
-		if users[u1.Id].Id != u1.Id {
-			t.Fatal("invalid returned user")
+		found := false
+		for _, u := range users {
+			if u.Id == u1.Id {
+				found = true
+			}
+		}
+
+		if !found {
+			t.Fatal("missing user")
 		}
 	}
 
 	if r2 := <-store.User().GetProfilesNotInChannel(teamId, c2.Id, 0, 100); r2.Err != nil {
 		t.Fatal(r2.Err)
 	} else {
-		if len(r2.Data.(map[string]*model.User)) != 2 {
+		if len(r2.Data.([]*model.User)) != 2 {
 			t.Fatal("invalid returned users")
 		}
 	}
@@ -423,7 +515,7 @@ func TestUserStoreGetProfilesNotInChannel(t *testing.T) {
 	if r1 := <-store.User().GetProfilesNotInChannel(teamId, c1.Id, 0, 100); r1.Err != nil {
 		t.Fatal(r1.Err)
 	} else {
-		users := r1.Data.(map[string]*model.User)
+		users := r1.Data.([]*model.User)
 		if len(users) != 0 {
 			t.Fatal("invalid returned users")
 		}
@@ -432,7 +524,7 @@ func TestUserStoreGetProfilesNotInChannel(t *testing.T) {
 	if r2 := <-store.User().GetProfilesNotInChannel(teamId, c2.Id, 0, 100); r2.Err != nil {
 		t.Fatal(r2.Err)
 	} else {
-		if len(r2.Data.(map[string]*model.User)) != 1 {
+		if len(r2.Data.([]*model.User)) != 1 {
 			t.Fatal("should have had 1 user not in channel")
 		}
 	}
@@ -576,8 +668,8 @@ func TestUserStoreGetProfilesByIds(t *testing.T) {
 	if r2 := <-store.User().GetProfiles("123", 0, 100); r2.Err != nil {
 		t.Fatal(r2.Err)
 	} else {
-		if len(r2.Data.(map[string]*model.User)) != 0 {
-			t.Fatal("should have returned empty map")
+		if len(r2.Data.([]*model.User)) != 0 {
+			t.Fatal("should have returned empty array")
 		}
 	}
 }
diff --git a/store/store.go b/store/store.go
index c75ec4554..96d9509b8 100644
--- a/store/store.go
+++ b/store/store.go
@@ -162,7 +162,8 @@ type UserStore interface {
 	GetAll() StoreChannel
 	InvalidateProfilesInChannelCacheByUser(userId string)
 	InvalidateProfilesInChannelCache(channelId string)
-	GetProfilesInChannel(channelId string, offset int, limit int, allowFromCache bool) StoreChannel
+	GetProfilesInChannel(channelId string, offset int, limit int) StoreChannel
+	GetAllProfilesInChannel(channelId string, allowFromCache bool) StoreChannel
 	GetProfilesNotInChannel(teamId string, channelId string, offset int, limit int) StoreChannel
 	GetProfilesByUsernames(usernames []string, teamId string) StoreChannel
 	GetAllProfiles(offset int, limit int) StoreChannel