PLT-8297: Message Export Should Still Produce Valid Exports if ChannelMemberHistory Data is Missing (#7951)

* Added a less accurate ChannelMembers fallback that is used if export period occurs before MessageExport was enabled

* Fixed test names

* Improved testing

* Made hasDataFromBefore() a little less strict

* Fixed the test to cleanly truncate the ChannelMemberHistory table without exposing the db via the interface

* Renamed a function for better clarity

* Binary logic is hard

store/sqlstore/channel_member_history_store.go

@@ -6,6 +6,8 @@ package sqlstore
 import (
 	"net/http"
 
+	"database/sql"
+
 	l4g "github.com/alecthomas/log4go"
 	"github.com/mattermost/mattermost-server/model"
 	"github.com/mattermost/mattermost-server/store"
@@ -65,7 +67,47 @@ func (s SqlChannelMemberHistoryStore) LogLeaveEvent(userId string, channelId str
 
 func (s SqlChannelMemberHistoryStore) GetUsersInChannelDuring(startTime int64, endTime int64, channelId string) store.StoreChannel {
 	return store.Do(func(result *store.StoreResult) {
-		query := `
+		if useChannelMemberHistory, err := s.hasDataAtOrBefore(startTime); err != nil {
+			result.Err = model.NewAppError("SqlChannelMemberHistoryStore.GetUsersInChannelAt", "store.sql_channel_member_history.get_users_in_channel_during.app_error", nil, err.Error(), http.StatusInternalServerError)
+		} else if useChannelMemberHistory {
+			// the export period starts after the ChannelMemberHistory table was first introduced, so we can use the
+			// data from it for our export
+			if channelMemberHistories, err := s.getFromChannelMemberHistoryTable(startTime, endTime, channelId); err != nil {
+				result.Err = model.NewAppError("SqlChannelMemberHistoryStore.GetUsersInChannelAt", "store.sql_channel_member_history.get_users_in_channel_during.app_error", nil, err.Error(), http.StatusInternalServerError)
+			} else {
+				result.Data = channelMemberHistories
+			}
+		} else {
+			// the export period starts before the ChannelMemberHistory table was introduced, so we need to fake the
+			// data by assuming that anybody who has ever joined the channel in question was present during the export period.
+			// this may not always be true, but it's better than saying that somebody wasn't there when they were
+			if channelMemberHistories, err := s.getFromChannelMembersTable(startTime, endTime, channelId); err != nil {
+				result.Err = model.NewAppError("SqlChannelMemberHistoryStore.GetUsersInChannelAt", "store.sql_channel_member_history.get_users_in_channel_during.app_error", nil, err.Error(), http.StatusInternalServerError)
+			} else {
+				result.Data = channelMemberHistories
+			}
+		}
+	})
+}
+
+func (s SqlChannelMemberHistoryStore) hasDataAtOrBefore(time int64) (bool, error) {
+	type NullableCountResult struct {
+		Min sql.NullInt64
+	}
+	var result NullableCountResult
+	query := "SELECT MIN(JoinTime) AS Min FROM ChannelMemberHistory"
+	if err := s.GetReplica().SelectOne(&result, query); err != nil {
+		return false, err
+	} else if result.Min.Valid {
+		return result.Min.Int64 <= time, nil
+	} else {
+		// if the result was null, there are no rows in the table, so there is no data from before
+		return false, nil
+	}
+}
+
+func (s SqlChannelMemberHistoryStore) getFromChannelMemberHistoryTable(startTime int64, endTime int64, channelId string) ([]*model.ChannelMemberHistory, error) {
+	query := `
 			SELECT
 				cmh.*,
 				u.Email
@@ -76,14 +118,37 @@ func (s SqlChannelMemberHistoryStore) GetUsersInChannelDuring(startTime int64, e
 			AND (cmh.LeaveTime IS NULL OR cmh.LeaveTime >= :StartTime)
 			ORDER BY cmh.JoinTime ASC`
 
-		params := map[string]interface{}{"ChannelId": channelId, "StartTime": startTime, "EndTime": endTime}
+	params := map[string]interface{}{"ChannelId": channelId, "StartTime": startTime, "EndTime": endTime}
-		var histories []*model.ChannelMemberHistory
+	var histories []*model.ChannelMemberHistory
-		if _, err := s.GetReplica().Select(&histories, query, params); err != nil {
+	if _, err := s.GetReplica().Select(&histories, query, params); err != nil {
-			result.Err = model.NewAppError("SqlChannelMemberHistoryStore.GetUsersInChannelAt", "store.sql_channel_member_history.get_users_in_channel_during.app_error", params, err.Error(), http.StatusInternalServerError)
+		return nil, err
-		} else {
+	} else {
-			result.Data = histories
+		return histories, nil
+	}
+}
+
+func (s SqlChannelMemberHistoryStore) getFromChannelMembersTable(startTime int64, endTime int64, channelId string) ([]*model.ChannelMemberHistory, error) {
+	query := `
+		SELECT DISTINCT
+  			ch.ChannelId,
+  			ch.UserId,
+  			u.email
+		FROM ChannelMembers AS ch
+		INNER JOIN Users AS u ON ch.UserId = u.id
+		WHERE ch.ChannelId = :ChannelId`
+
+	params := map[string]interface{}{"ChannelId": channelId}
+	var histories []*model.ChannelMemberHistory
+	if _, err := s.GetReplica().Select(&histories, query, params); err != nil {
+		return nil, err
+	} else {
+		// we have to fill in the join/leave times, because that data doesn't exist in the channel members table
+		for _, channelMemberHistory := range histories {
+			channelMemberHistory.JoinTime = startTime
+			channelMemberHistory.LeaveTime = model.NewInt64(endTime)
 		}
-	})
+		return histories, nil
+	}
 }
 
 func (s SqlChannelMemberHistoryStore) PermanentDeleteBatch(endTime int64, limit int64) store.StoreChannel {
@@ -112,7 +177,6 @@ func (s SqlChannelMemberHistoryStore) PermanentDeleteBatch(endTime int64, limit
 		} else {
 			if rowsAffected, err1 := sqlResult.RowsAffected(); err1 != nil {
 				result.Err = model.NewAppError("SqlChannelMemberHistoryStore.PermanentDeleteBatchForChannel", "store.sql_channel_member_history.permanent_delete_batch.app_error", params, err.Error(), http.StatusInternalServerError)
-				result.Data = int64(0)
 			} else {
 				result.Data = rowsAffected
 			}

store/storetest/channel_member_history_store.go

@@ -14,10 +14,11 @@ import (
 )
 
 func TestChannelMemberHistoryStore(t *testing.T, ss store.Store) {
-	t.Run("Log Join Event", func(t *testing.T) { testLogJoinEvent(t, ss) })
+	t.Run("TestLogJoinEvent", func(t *testing.T) { testLogJoinEvent(t, ss) })
-	t.Run("Log Leave Event", func(t *testing.T) { testLogLeaveEvent(t, ss) })
+	t.Run("TestLogLeaveEvent", func(t *testing.T) { testLogLeaveEvent(t, ss) })
-	t.Run("Get Users In Channel At Time", func(t *testing.T) { testGetUsersInChannelAt(t, ss) })
+	t.Run("TestGetUsersInChannelAtChannelMemberHistory", func(t *testing.T) { testGetUsersInChannelAtChannelMemberHistory(t, ss) })
-	t.Run("Purge History", func(t *testing.T) { testPermanentDeleteBatch(t, ss) })
+	t.Run("TestGetUsersInChannelAtChannelMembers", func(t *testing.T) { testGetUsersInChannelAtChannelMembers(t, ss) })
+	t.Run("TestPermanentDeleteBatch", func(t *testing.T) { testPermanentDeleteBatch(t, ss) })
 }
 
 func testLogJoinEvent(t *testing.T, ss store.Store) {
@@ -67,7 +68,7 @@ func testLogLeaveEvent(t *testing.T, ss store.Store) {
 	assert.Nil(t, result.Err)
 }
 
-func testGetUsersInChannelAt(t *testing.T, ss store.Store) {
+func testGetUsersInChannelAtChannelMemberHistory(t *testing.T, ss store.Store) {
 	// create a test channel
 	channel := model.Channel{
 		TeamId:      model.NewId(),
@@ -84,17 +85,25 @@ func testGetUsersInChannelAt(t *testing.T, ss store.Store) {
 	}
 	user = *store.Must(ss.User().Save(&user)).(*model.User)
 
-	// log a join event
+	// the user was previously in the channel a long time ago, before the export period starts
-	leaveTime := model.GetMillis()
+	// the existence of this record makes it look like the MessageExport feature has been active for awhile, and prevents
+	// us from looking in the ChannelMembers table for data that isn't found in the ChannelMemberHistory table
+	leaveTime := model.GetMillis() - 20000
 	joinTime := leaveTime - 10000
 	store.Must(ss.ChannelMemberHistory().LogJoinEvent(user.Id, channel.Id, joinTime))
+	store.Must(ss.ChannelMemberHistory().LogLeaveEvent(user.Id, channel.Id, leaveTime))
 
-	// case 1: both start and end before join time
+	// log a join event
+	leaveTime = model.GetMillis()
+	joinTime = leaveTime - 10000
+	store.Must(ss.ChannelMemberHistory().LogJoinEvent(user.Id, channel.Id, joinTime))
+
+	// case 1: user joins and leaves the channel before the export period begins
 	channelMembers := store.Must(ss.ChannelMemberHistory().GetUsersInChannelDuring(joinTime-500, joinTime-100, channel.Id)).([]*model.ChannelMemberHistory)
 	assert.Len(t, channelMembers, 0)
 
-	// case 2: start before join time, no leave time
+	// case 2: user joins the channel after the export period begins, but has not yet left the channel when the export period ends
-	channelMembers = store.Must(ss.ChannelMemberHistory().GetUsersInChannelDuring(joinTime-100, joinTime+100, channel.Id)).([]*model.ChannelMemberHistory)
+	channelMembers = store.Must(ss.ChannelMemberHistory().GetUsersInChannelDuring(joinTime-100, joinTime+500, channel.Id)).([]*model.ChannelMemberHistory)
 	assert.Len(t, channelMembers, 1)
 	assert.Equal(t, channel.Id, channelMembers[0].ChannelId)
 	assert.Equal(t, user.Id, channelMembers[0].UserId)
@@ -102,7 +111,7 @@ func testGetUsersInChannelAt(t *testing.T, ss store.Store) {
 	assert.Equal(t, joinTime, channelMembers[0].JoinTime)
 	assert.Nil(t, channelMembers[0].LeaveTime)
 
-	// case 3: start after join time, no leave time
+	// case 3: user joins the channel before the export period begins, but has not yet left the channel when the export period ends
 	channelMembers = store.Must(ss.ChannelMemberHistory().GetUsersInChannelDuring(joinTime+100, joinTime+500, channel.Id)).([]*model.ChannelMemberHistory)
 	assert.Len(t, channelMembers, 1)
 	assert.Equal(t, channel.Id, channelMembers[0].ChannelId)
@@ -114,7 +123,7 @@ func testGetUsersInChannelAt(t *testing.T, ss store.Store) {
 	// add a leave time for the user
 	store.Must(ss.ChannelMemberHistory().LogLeaveEvent(user.Id, channel.Id, leaveTime))
 
-	// case 4: start after join time, end before leave time
+	// case 4: user joins the channel before the export period begins, but has not yet left the channel when the export period ends
 	channelMembers = store.Must(ss.ChannelMemberHistory().GetUsersInChannelDuring(joinTime+100, leaveTime-100, channel.Id)).([]*model.ChannelMemberHistory)
 	assert.Len(t, channelMembers, 1)
 	assert.Equal(t, channel.Id, channelMembers[0].ChannelId)
@@ -123,7 +132,7 @@ func testGetUsersInChannelAt(t *testing.T, ss store.Store) {
 	assert.Equal(t, joinTime, channelMembers[0].JoinTime)
 	assert.Equal(t, leaveTime, *channelMembers[0].LeaveTime)
 
-	// case 5: start before join time, end after leave time
+	// case 5: user joins the channel after the export period begins, and leaves the channel before the export period ends
 	channelMembers = store.Must(ss.ChannelMemberHistory().GetUsersInChannelDuring(joinTime-100, leaveTime+100, channel.Id)).([]*model.ChannelMemberHistory)
 	assert.Len(t, channelMembers, 1)
 	assert.Equal(t, channel.Id, channelMembers[0].ChannelId)
@@ -132,11 +141,106 @@ func testGetUsersInChannelAt(t *testing.T, ss store.Store) {
 	assert.Equal(t, joinTime, channelMembers[0].JoinTime)
 	assert.Equal(t, leaveTime, *channelMembers[0].LeaveTime)
 
-	// case 6: start and end after leave time
+	// case 6: user has joined and left the channel long before the export period begins
 	channelMembers = store.Must(ss.ChannelMemberHistory().GetUsersInChannelDuring(leaveTime+100, leaveTime+200, channel.Id)).([]*model.ChannelMemberHistory)
 	assert.Len(t, channelMembers, 0)
 }
 
+func testGetUsersInChannelAtChannelMembers(t *testing.T, ss store.Store) {
+	// create a test channel
+	channel := model.Channel{
+		TeamId:      model.NewId(),
+		DisplayName: "Display " + model.NewId(),
+		Name:        "zz" + model.NewId() + "b",
+		Type:        model.CHANNEL_OPEN,
+	}
+	channel = *store.Must(ss.Channel().Save(&channel, -1)).(*model.Channel)
+
+	// and a test user
+	user := model.User{
+		Email:    model.NewId() + "@mattermost.com",
+		Nickname: model.NewId(),
+	}
+	user = *store.Must(ss.User().Save(&user)).(*model.User)
+
+	// clear any existing ChannelMemberHistory data that might interfere with our test
+	var tableDataTruncated = false
+	for !tableDataTruncated {
+		if result := <-ss.ChannelMemberHistory().PermanentDeleteBatch(model.GetMillis(), 1000); result.Err != nil {
+			assert.Fail(t, "Failed to truncate ChannelMemberHistory contents", result.Err.Error())
+		} else {
+			tableDataTruncated = result.Data.(int64) == int64(0)
+		}
+	}
+
+	// in this test, we're pretending that Message Export was not activated during the export period, so there's no data
+	// available in the ChannelMemberHistory table. Instead, we'll fall back to the ChannelMembers table for a rough approximation
+	joinTime := int64(1000)
+	leaveTime := joinTime + 5000
+	store.Must(ss.Channel().SaveMember(&model.ChannelMember{
+		ChannelId:   channel.Id,
+		UserId:      user.Id,
+		NotifyProps: model.GetDefaultChannelNotifyProps(),
+	}))
+
+	// in every single case, the user will be included in the export, because ChannelMembers says they were in the channel at some point in
+	// the past, even though the time that they were actually in the channel doesn't necessarily overlap with the export period
+
+	// case 1: user joins and leaves the channel before the export period begins
+	channelMembers := store.Must(ss.ChannelMemberHistory().GetUsersInChannelDuring(joinTime-500, joinTime-100, channel.Id)).([]*model.ChannelMemberHistory)
+	assert.Len(t, channelMembers, 1)
+	assert.Equal(t, channel.Id, channelMembers[0].ChannelId)
+	assert.Equal(t, user.Id, channelMembers[0].UserId)
+	assert.Equal(t, user.Email, channelMembers[0].UserEmail)
+	assert.Equal(t, joinTime-500, channelMembers[0].JoinTime)
+	assert.Equal(t, joinTime-100, *channelMembers[0].LeaveTime)
+
+	// case 2: user joins the channel after the export period begins, but has not yet left the channel when the export period ends
+	channelMembers = store.Must(ss.ChannelMemberHistory().GetUsersInChannelDuring(joinTime-100, joinTime+500, channel.Id)).([]*model.ChannelMemberHistory)
+	assert.Len(t, channelMembers, 1)
+	assert.Equal(t, channel.Id, channelMembers[0].ChannelId)
+	assert.Equal(t, user.Id, channelMembers[0].UserId)
+	assert.Equal(t, user.Email, channelMembers[0].UserEmail)
+	assert.Equal(t, joinTime-100, channelMembers[0].JoinTime)
+	assert.Equal(t, joinTime+500, *channelMembers[0].LeaveTime)
+
+	// case 3: user joins the channel before the export period begins, but has not yet left the channel when the export period ends
+	channelMembers = store.Must(ss.ChannelMemberHistory().GetUsersInChannelDuring(joinTime+100, joinTime+500, channel.Id)).([]*model.ChannelMemberHistory)
+	assert.Len(t, channelMembers, 1)
+	assert.Equal(t, channel.Id, channelMembers[0].ChannelId)
+	assert.Equal(t, user.Id, channelMembers[0].UserId)
+	assert.Equal(t, user.Email, channelMembers[0].UserEmail)
+	assert.Equal(t, joinTime+100, channelMembers[0].JoinTime)
+	assert.Equal(t, joinTime+500, *channelMembers[0].LeaveTime)
+
+	// case 4: user joins the channel before the export period begins, but has not yet left the channel when the export period ends
+	channelMembers = store.Must(ss.ChannelMemberHistory().GetUsersInChannelDuring(joinTime+100, leaveTime-100, channel.Id)).([]*model.ChannelMemberHistory)
+	assert.Len(t, channelMembers, 1)
+	assert.Equal(t, channel.Id, channelMembers[0].ChannelId)
+	assert.Equal(t, user.Id, channelMembers[0].UserId)
+	assert.Equal(t, user.Email, channelMembers[0].UserEmail)
+	assert.Equal(t, joinTime+100, channelMembers[0].JoinTime)
+	assert.Equal(t, leaveTime-100, *channelMembers[0].LeaveTime)
+
+	// case 5: user joins the channel after the export period begins, and leaves the channel before the export period ends
+	channelMembers = store.Must(ss.ChannelMemberHistory().GetUsersInChannelDuring(joinTime-100, leaveTime+100, channel.Id)).([]*model.ChannelMemberHistory)
+	assert.Len(t, channelMembers, 1)
+	assert.Equal(t, channel.Id, channelMembers[0].ChannelId)
+	assert.Equal(t, user.Id, channelMembers[0].UserId)
+	assert.Equal(t, user.Email, channelMembers[0].UserEmail)
+	assert.Equal(t, joinTime-100, channelMembers[0].JoinTime)
+	assert.Equal(t, leaveTime+100, *channelMembers[0].LeaveTime)
+
+	// case 6: user has joined and left the channel long before the export period begins
+	channelMembers = store.Must(ss.ChannelMemberHistory().GetUsersInChannelDuring(leaveTime+100, leaveTime+200, channel.Id)).([]*model.ChannelMemberHistory)
+	assert.Len(t, channelMembers, 1)
+	assert.Equal(t, channel.Id, channelMembers[0].ChannelId)
+	assert.Equal(t, user.Id, channelMembers[0].UserId)
+	assert.Equal(t, user.Email, channelMembers[0].UserEmail)
+	assert.Equal(t, leaveTime+100, channelMembers[0].JoinTime)
+	assert.Equal(t, leaveTime+200, *channelMembers[0].LeaveTime)
+}
+
 func testPermanentDeleteBatch(t *testing.T, ss store.Store) {
 	// create a test channel
 	channel := model.Channel{