Skip to content
This repository has been archived by the owner on Oct 11, 2024. It is now read-only.

Commit

Permalink
filter out system messages (#4481)
Browse files Browse the repository at this point in the history
adds a filter that removes system messages from
channel message backups.

Also extends the preview size from 16 to 128 characters.

---

#### Does this PR need a docs update or release note?

- [x] ⛔ No

#### Type of change

- [x] 🧹 Tech Debt/Cleanup

#### Issue(s)

* #3988

#### Test Plan

- [x] ⚡ Unit test
- [x] 💚 E2E
  • Loading branch information
ryanfkeepers authored Oct 13, 2023
1 parent 5b8b371 commit d2e35a9
Show file tree
Hide file tree
Showing 5 changed files with 178 additions and 7 deletions.
2 changes: 1 addition & 1 deletion src/pkg/services/m365/api/channels.go
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ func ChannelMessageInfo(
LastReplyAt: lastReply,
Modified: modTime,
MessageCreator: GetChatMessageFrom(msg),
MessagePreview: str.Preview(content, 16),
MessagePreview: str.Preview(content, 128),
ReplyCount: len(msg.GetReplies()),
Size: int64(len(content)),
}
Expand Down
22 changes: 21 additions & 1 deletion src/pkg/services/m365/api/channels_pager.go
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,25 @@ func (c Channels) NewChannelMessageDeltaPager(
}
}

// this is the message content for system chatMessage entities with type
// unknownFutureValue.
const channelMessageSystemMessageContent = "<systemEventMessage/>"

func FilterOutSystemMessages(cm models.ChatMessageable) bool {
if ptr.Val(cm.GetMessageType()) == models.SYSTEMEVENTMESSAGE_CHATMESSAGETYPE {
return false
}

content := ""

if cm.GetBody() != nil {
content = ptr.Val(cm.GetBody().GetContent())
}

return !(ptr.Val(cm.GetMessageType()) == models.UNKNOWNFUTUREVALUE_CHATMESSAGETYPE &&
content == channelMessageSystemMessageContent)
}

// GetChannelMessageIDsDelta fetches a delta of all messages in the channel.
// returns two maps: addedItems, deletedItems
func (c Channels) GetChannelMessageIDs(
Expand All @@ -157,7 +176,8 @@ func (c Channels) GetChannelMessageIDs(
c.NewChannelMessageDeltaPager(teamID, channelID, prevDeltaLink),
prevDeltaLink,
canMakeDeltaQueries,
addedAndRemovedByDeletedDateTime[models.ChatMessageable])
addedAndRemovedByDeletedDateTime[models.ChatMessageable],
FilterOutSystemMessages)

return added, validModTimes, removed, du, clues.Stack(err).OrNil()
}
Expand Down
67 changes: 66 additions & 1 deletion src/pkg/services/m365/api/channels_pager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"time"

"github.com/alcionai/clues"
"github.com/microsoftgraph/msgraph-sdk-go/models"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"github.com/stretchr/testify/suite"
Expand Down Expand Up @@ -125,7 +126,7 @@ func testEnumerateChannelMessageReplies(
assert.Equal(t, len(replies), info.ReplyCount)
assert.Equal(t, msg.GetFrom().GetUser().GetDisplayName(), info.MessageCreator)
assert.Equal(t, lastReply, info.LastReplyAt)
assert.Equal(t, str.Preview(ptr.Val(msg.GetBody().GetContent()), 16), info.MessagePreview)
assert.Equal(t, str.Preview(ptr.Val(msg.GetBody().GetContent()), 128), info.MessagePreview)

msgReplyIDs := map[string]struct{}{}

Expand All @@ -135,3 +136,67 @@ func testEnumerateChannelMessageReplies(

assert.Equal(t, replyIDs, msgReplyIDs)
}

func (suite *ChannelsPagerIntgSuite) TestFilterOutSystemMessages() {
systemMessage := models.NewChatMessage()
systemMessage.SetMessageType(ptr.To(models.SYSTEMEVENTMESSAGE_CHATMESSAGETYPE))

systemMessageBody := models.NewItemBody()
systemMessageBody.SetContent(ptr.To("<systemEventMessage/>"))

messageBody := models.NewItemBody()
messageBody.SetContent(ptr.To("message"))

unknownFutureSystemMessage := models.NewChatMessage()
unknownFutureSystemMessage.SetMessageType(ptr.To(models.UNKNOWNFUTUREVALUE_CHATMESSAGETYPE))
unknownFutureSystemMessage.SetBody(systemMessageBody)

unknownFutureMessage := models.NewChatMessage()
unknownFutureMessage.SetMessageType(ptr.To(models.UNKNOWNFUTUREVALUE_CHATMESSAGETYPE))
unknownFutureMessage.SetBody(messageBody)

regularSystemMessage := models.NewChatMessage()
regularSystemMessage.SetMessageType(ptr.To(models.MESSAGE_CHATMESSAGETYPE))
regularSystemMessage.SetBody(systemMessageBody)

regularMessage := models.NewChatMessage()
regularMessage.SetMessageType(ptr.To(models.MESSAGE_CHATMESSAGETYPE))
regularMessage.SetBody(messageBody)

table := []struct {
name string
cm models.ChatMessageable
expect assert.BoolAssertionFunc
}{
{
name: "system message type",
cm: systemMessage,
expect: assert.False,
},
{
name: "unknown future type system body",
cm: unknownFutureSystemMessage,
expect: assert.False,
},
{
name: "unknown future type message body",
cm: unknownFutureMessage,
expect: assert.True,
},
{
name: "message type system body",
cm: regularSystemMessage,
expect: assert.True,
},
{
name: "message type message body",
cm: regularMessage,
expect: assert.True,
},
}
for _, test := range table {
suite.Run(test.name, func() {
test.expect(suite.T(), api.FilterOutSystemMessages(test.cm))
})
}
}
34 changes: 31 additions & 3 deletions src/pkg/services/m365/api/item_pager.go
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,12 @@ func deltaEnumerateItems[T any](

type addedAndRemovedHandler[T any] func(
items []T,
) (map[string]time.Time, []string, error)
filters ...func(T) bool, // false -> remove, true -> keep
) (
map[string]time.Time,
[]string,
error,
)

func getAddedAndRemovedItemIDs[T any](
ctx context.Context,
Expand All @@ -218,6 +223,7 @@ func getAddedAndRemovedItemIDs[T any](
prevDeltaLink string,
canMakeDeltaQueries bool,
aarh addedAndRemovedHandler[T],
filters ...func(T) bool,
) (map[string]time.Time, bool, []string, DeltaUpdate, error) {
if canMakeDeltaQueries {
ts, du, err := deltaEnumerateItems[T](ctx, deltaPager, prevDeltaLink)
Expand All @@ -226,7 +232,7 @@ func getAddedAndRemovedItemIDs[T any](
}

if err == nil {
a, r, err := aarh(ts)
a, r, err := aarh(ts, filters...)
return a, deltaPager.ValidModTimes(), r, du, graph.Stack(ctx, err).OrNil()
}
}
Expand All @@ -238,7 +244,7 @@ func getAddedAndRemovedItemIDs[T any](
return nil, false, nil, DeltaUpdate{}, graph.Stack(ctx, err)
}

a, r, err := aarh(ts)
a, r, err := aarh(ts, filters...)

return a, pager.ValidModTimes(), r, du, graph.Stack(ctx, err).OrNil()
}
Expand All @@ -264,11 +270,22 @@ type getModTimer interface {

func addedAndRemovedByAddtlData[T any](
items []T,
filters ...func(T) bool,
) (map[string]time.Time, []string, error) {
added := map[string]time.Time{}
removed := []string{}

for _, item := range items {
passAllFilters := true

for _, passes := range filters {
passAllFilters = passAllFilters && passes(item)
}

if !passAllFilters {
continue
}

giaa, ok := any(item).(getIDModAndAddtler)
if !ok {
return nil, nil, clues.New("item does not provide id and additional data getters").
Expand Down Expand Up @@ -312,11 +329,22 @@ type getIDModAndDeletedDateTimer interface {

func addedAndRemovedByDeletedDateTime[T any](
items []T,
filters ...func(T) bool,
) (map[string]time.Time, []string, error) {
added := map[string]time.Time{}
removed := []string{}

for _, item := range items {
passAllFilters := true

for _, passes := range filters {
passAllFilters = passAllFilters && passes(item)
}

if !passAllFilters {
continue
}

giaddt, ok := any(item).(getIDModAndDeletedDateTimer)
if !ok {
return nil, nil, clues.New("item does not provide id and deleted date time getters").
Expand Down
60 changes: 59 additions & 1 deletion src/pkg/services/m365/api/item_pager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -313,6 +313,7 @@ func (suite *PagerUnitSuite) TestGetAddedAndRemovedItemIDs() {
*testing.T,
) DeltaPager[testItem]
prevDelta string
filter func(a testItem) bool
expect expected
canDelta bool
validModTimes bool
Expand Down Expand Up @@ -454,6 +455,57 @@ func (suite *PagerUnitSuite) TestGetAddedAndRemovedItemIDs() {
},
canDelta: false,
},
{
name: "no prev delta and fail all filter",
pagerGetter: func(t *testing.T) Pager[testItem] {
return nil
},
deltaPagerGetter: func(t *testing.T) DeltaPager[testItem] {
return &testIDsDeltaPager{
t: t,
added: map[string]time.Time{
"uno": now.Add(time.Minute),
"dos": now.Add(2 * time.Minute),
},
removed: []string{"tres", "quatro"},
validModTimes: true,
}
},
filter: func(testItem) bool { return false },
expect: expected{
added: map[string]time.Time{},
removed: []string{},
deltaUpdate: DeltaUpdate{Reset: true},
validModTimes: true,
},
canDelta: true,
},
{
name: "with prev delta and fail all filter",
pagerGetter: func(t *testing.T) Pager[testItem] {
return nil
},
deltaPagerGetter: func(t *testing.T) DeltaPager[testItem] {
return &testIDsDeltaPager{
t: t,
added: map[string]time.Time{
"uno": now.Add(time.Minute),
"dos": now.Add(2 * time.Minute),
},
removed: []string{"tres", "quatro"},
validModTimes: true,
}
},
filter: func(testItem) bool { return false },
prevDelta: "delta",
expect: expected{
added: map[string]time.Time{},
removed: []string{},
deltaUpdate: DeltaUpdate{Reset: false},
validModTimes: true,
},
canDelta: true,
},
}

for _, test := range tests {
Expand All @@ -463,13 +515,19 @@ func (suite *PagerUnitSuite) TestGetAddedAndRemovedItemIDs() {
ctx, flush := tester.NewContext(t)
defer flush()

filters := []func(testItem) bool{}
if test.filter != nil {
filters = append(filters, test.filter)
}

added, validModTimes, removed, deltaUpdate, err := getAddedAndRemovedItemIDs[testItem](
ctx,
test.pagerGetter(t),
test.deltaPagerGetter(t),
test.prevDelta,
test.canDelta,
addedAndRemovedByAddtlData[testItem])
addedAndRemovedByAddtlData[testItem],
filters...)

require.NoErrorf(t, err, "getting added and removed item IDs: %+v", clues.ToCore(err))
if validModTimes {
Expand Down

0 comments on commit d2e35a9

Please sign in to comment.