diff --git a/internal/roomname.go b/internal/roomname.go index 73eb0205..8e4ad465 100644 --- a/internal/roomname.go +++ b/internal/roomname.go @@ -13,7 +13,12 @@ type EventMetadata struct { Timestamp uint64 } -// RoomMetadata holds room-scoped data. It is primarily used in two places: +// RoomMetadata holds room-scoped data. +// TODO: This is a lie: we sometimes remove a user U from the list of heroes +// when calculating the sync response for that user U. Grep for `RemoveHero`. +// +// It is primarily used in two places: +// // - in the caches.GlobalCache, to hold the latest version of data that is consistent // between all users in the room; and // - in the sync3.RoomConnMetadata struct, to hold the version of data last seen by @@ -25,6 +30,7 @@ type RoomMetadata struct { RoomID string Heroes []Hero NameEvent string // the content of m.room.name, NOT the calculated name + AvatarEvent string // the content of m.room.avatar, NOT the resolved avatar CanonicalAlias string JoinCount int InviteCount int @@ -54,6 +60,32 @@ func NewRoomMetadata(roomID string) *RoomMetadata { } } +// CopyHeroes returns a version of the current RoomMetadata whose Heroes field is +// a brand-new copy of the original Heroes. The return value's Heroes field can be +// safely modified by the caller, but it is NOT safe for the caller to modify any other +// fields. +func (m *RoomMetadata) CopyHeroes() *RoomMetadata { + newMetadata := *m + + // XXX: We're doing this because we end up calling RemoveHero() to omit the + // currently-sycning user in various places. But this seems smelly. The set of + // heroes in the room is a global, room-scoped fact: it is a property of the room + // state and nothing else, and all users see the same set of heroes. + // + // I think the data model would be cleaner if we made the hero-reading functions + // aware of the currently syncing user, in order to ignore them without having to + // change the underlying data. + // + // copy the heroes or else we may modify the same slice which would be bad :( + newMetadata.Heroes = make([]Hero, len(m.Heroes)) + copy(newMetadata.Heroes, m.Heroes) + + // ⚠️ NB: there are other pointer fields (e.g. PredecessorRoomID *string) or + // and pointer-backed fields (e.g. LatestEventsByType map[string]EventMetadata) + // which are not deepcopied here. + return &newMetadata +} + // SameRoomName checks if the fields relevant for room names have changed between the two metadatas. // Returns true if there are no changes. func (m *RoomMetadata) SameRoomName(other *RoomMetadata) bool { @@ -62,7 +94,13 @@ func (m *RoomMetadata) SameRoomName(other *RoomMetadata) bool { m.CanonicalAlias == other.CanonicalAlias && m.JoinCount == other.JoinCount && m.InviteCount == other.InviteCount && - sameHeroes(m.Heroes, other.Heroes)) + sameHeroNames(m.Heroes, other.Heroes)) +} + +// SameRoomAvatar checks if the fields relevant for room avatars have changed between the two metadatas. +// Returns true if there are no changes. +func (m *RoomMetadata) SameRoomAvatar(other *RoomMetadata) bool { + return m.AvatarEvent == other.AvatarEvent && sameHeroAvatars(m.Heroes, other.Heroes) } func (m *RoomMetadata) SameJoinCount(other *RoomMetadata) bool { @@ -73,7 +111,7 @@ func (m *RoomMetadata) SameInviteCount(other *RoomMetadata) bool { return m.InviteCount == other.InviteCount } -func sameHeroes(a, b []Hero) bool { +func sameHeroNames(a, b []Hero) bool { if len(a) != len(b) { return false } @@ -88,6 +126,21 @@ func sameHeroes(a, b []Hero) bool { return true } +func sameHeroAvatars(a, b []Hero) bool { + if len(a) != len(b) { + return false + } + for i := range a { + if a[i].ID != b[i].ID { + return false + } + if a[i].Avatar != b[i].Avatar { + return false + } + } + return true +} + func (m *RoomMetadata) RemoveHero(userID string) { for i, h := range m.Heroes { if h.ID == userID { @@ -102,8 +155,9 @@ func (m *RoomMetadata) IsSpace() bool { } type Hero struct { - ID string - Name string + ID string + Name string + Avatar string } func CalculateRoomName(heroInfo *RoomMetadata, maxNumNamesPerRoom int) string { @@ -190,3 +244,18 @@ func disambiguate(heroes []Hero) []string { } return disambiguatedNames } + +const noAvatar = "" + +// CalculateAvatar computes the avatar for the room, based on the global room metadata. +// Assumption: metadata.RemoveHero has been called to remove the user who is syncing +// from the list of heroes. +func CalculateAvatar(metadata *RoomMetadata) string { + if metadata.AvatarEvent != "" { + return metadata.AvatarEvent + } + if len(metadata.Heroes) == 1 { + return metadata.Heroes[0].Avatar + } + return noAvatar +} diff --git a/internal/roomname_test.go b/internal/roomname_test.go index 4e942f43..01349eed 100644 --- a/internal/roomname_test.go +++ b/internal/roomname_test.go @@ -247,3 +247,45 @@ func TestCalculateRoomName(t *testing.T) { } } } + +func TestCopyHeroes(t *testing.T) { + const alice = "@alice:test" + const bob = "@bob:test" + const chris = "@chris:test" + m1 := RoomMetadata{Heroes: []Hero{ + {ID: alice}, + {ID: bob}, + {ID: chris}, + }} + + m2 := m1.CopyHeroes() + // Uncomment this to see why CopyHeroes is necessary! + //m2 := m1 + + t.Logf("Compare heroes:\n\tm1=%v\n\tm2=%v", m1.Heroes, m2.Heroes) + + t.Log("Remove chris from m1") + m1.RemoveHero(chris) + t.Logf("Compare heroes:\n\tm1=%v\n\tm2=%v", m1.Heroes, m2.Heroes) + + assertSliceIDs(t, "m1.Heroes", m1.Heroes, []string{alice, bob}) + assertSliceIDs(t, "m2.Heroes", m2.Heroes, []string{alice, bob, chris}) + + t.Log("Remove alice from m1") + m1.RemoveHero(alice) + t.Logf("Compare heroes:\n\tm1=%v\n\tm2=%v", m1.Heroes, m2.Heroes) + + assertSliceIDs(t, "m1.Heroes", m1.Heroes, []string{bob}) + assertSliceIDs(t, "m2.Heroes", m2.Heroes, []string{alice, bob, chris}) +} + +func assertSliceIDs(t *testing.T, desc string, h []Hero, ids []string) { + if len(h) != len(ids) { + t.Errorf("%s has length %d, expected %d", desc, len(h), len(ids)) + } + for index, id := range ids { + if h[index].ID != id { + t.Errorf("%s[%d] ID is %s, expected %s", desc, index, h[index].ID, id) + } + } +} diff --git a/state/storage.go b/state/storage.go index b51026cc..5e1e9338 100644 --- a/state/storage.go +++ b/state/storage.go @@ -232,7 +232,7 @@ func (s *Storage) MetadataForAllRooms(txn *sqlx.Tx, tempTableName string, result // Select the name / canonical alias for all rooms roomIDToStateEvents, err := s.currentNotMembershipStateEventsInAllRooms(txn, []string{ - "m.room.name", "m.room.canonical_alias", + "m.room.name", "m.room.canonical_alias", "m.room.avatar", }) if err != nil { return fmt.Errorf("failed to load state events for all rooms: %s", err) @@ -244,6 +244,8 @@ func (s *Storage) MetadataForAllRooms(txn *sqlx.Tx, tempTableName string, result metadata.NameEvent = gjson.ParseBytes(ev.JSON).Get("content.name").Str } else if ev.Type == "m.room.canonical_alias" && ev.StateKey == "" { metadata.CanonicalAlias = gjson.ParseBytes(ev.JSON).Get("content.alias").Str + } else if ev.Type == "m.room.avatar" && ev.StateKey == "" { + metadata.AvatarEvent = gjson.ParseBytes(ev.JSON).Get("content.url").Str } } result[roomID] = metadata @@ -282,8 +284,9 @@ func (s *Storage) MetadataForAllRooms(txn *sqlx.Tx, tempTableName string, result seen[key] = true metadata := result[roomID] metadata.Heroes = append(metadata.Heroes, internal.Hero{ - ID: targetUser, - Name: ev.Get("content.displayname").Str, + ID: targetUser, + Name: ev.Get("content.displayname").Str, + Avatar: ev.Get("content.avatar_url").Str, }) result[roomID] = metadata } diff --git a/sync3/avatar.go b/sync3/avatar.go new file mode 100644 index 00000000..63e494b8 --- /dev/null +++ b/sync3/avatar.go @@ -0,0 +1,42 @@ +package sync3 + +import ( + "bytes" + "encoding/json" +) + +// An AvatarChange represents a change to a room's avatar. There are three cases: +// - an empty string represents no change, and should be omitted when JSON-serialised; +// - the sentinel `` represents a room that has never had an avatar, +// or a room whose avatar has been removed. It is JSON-serialised as null. +// - All other strings represent the current avatar of the room and JSON-serialise as +// normal. +type AvatarChange string + +const DeletedAvatar = AvatarChange("") +const UnchangedAvatar AvatarChange = "" + +// NewAvatarChange interprets an optional avatar string as an AvatarChange. +func NewAvatarChange(avatar string) AvatarChange { + if avatar == "" { + return DeletedAvatar + } + return AvatarChange(avatar) +} + +func (a AvatarChange) MarshalJSON() ([]byte, error) { + if a == DeletedAvatar { + return []byte(`null`), nil + } else { + return json.Marshal(string(a)) + } +} + +// Note: the unmarshalling is only used in tests. +func (a *AvatarChange) UnmarshalJSON(data []byte) error { + if bytes.Equal(data, []byte("null")) { + *a = DeletedAvatar + return nil + } + return json.Unmarshal(data, (*string)(a)) +} diff --git a/sync3/caches/global.go b/sync3/caches/global.go index 36c2867a..537ce00b 100644 --- a/sync3/caches/global.go +++ b/sync3/caches/global.go @@ -118,13 +118,7 @@ func (c *GlobalCache) copyRoom(roomID string) *internal.RoomMetadata { logger.Warn().Str("room", roomID).Msg("GlobalCache.LoadRoom: no metadata for this room, returning stub") return internal.NewRoomMetadata(roomID) } - srCopy := *sr - // copy the heroes or else we may modify the same slice which would be bad :( - srCopy.Heroes = make([]internal.Hero, len(sr.Heroes)) - for i := range sr.Heroes { - srCopy.Heroes[i] = sr.Heroes[i] - } - return &srCopy + return sr.CopyHeroes() } // LoadJoinedRooms loads all current joined room metadata for the user given, together @@ -285,6 +279,10 @@ func (c *GlobalCache) OnNewEvent( if ed.StateKey != nil && *ed.StateKey == "" { metadata.NameEvent = ed.Content.Get("name").Str } + case "m.room.avatar": + if ed.StateKey != nil && *ed.StateKey == "" { + metadata.AvatarEvent = ed.Content.Get("url").Str + } case "m.room.encryption": if ed.StateKey != nil && *ed.StateKey == "" { metadata.Encrypted = true @@ -349,14 +347,16 @@ func (c *GlobalCache) OnNewEvent( for i := range metadata.Heroes { if metadata.Heroes[i].ID == *ed.StateKey { metadata.Heroes[i].Name = ed.Content.Get("displayname").Str + metadata.Heroes[i].Avatar = ed.Content.Get("avatar_url").Str found = true break } } if !found { metadata.Heroes = append(metadata.Heroes, internal.Hero{ - ID: *ed.StateKey, - Name: ed.Content.Get("displayname").Str, + ID: *ed.StateKey, + Name: ed.Content.Get("displayname").Str, + Avatar: ed.Content.Get("avatar_url").Str, }) } } diff --git a/sync3/caches/user.go b/sync3/caches/user.go index ef160f8b..9d6b4e6e 100644 --- a/sync3/caches/user.go +++ b/sync3/caches/user.go @@ -50,6 +50,14 @@ type UserRoomData struct { CanonicalisedName string // stripped leading symbols like #, all in lower case // Set of spaces this room is a part of, from the perspective of this user. This is NOT global room data // as the set of spaces may be different for different users. + + // ResolvedAvatarURL is the avatar that should be displayed to this user to + // represent this room. The empty string means that this room has no avatar. + // Avatars set in m.room.avatar take precedence; if this is missing and the room is + // a DM with one other user joined or invited, we fall back to that user's + // avatar (if any) as specified in their membership event in that room. + ResolvedAvatarURL string + Spaces map[string]struct{} // Map of tag to order float. // See https://spec.matrix.org/latest/client-server-api/#room-tagging @@ -73,6 +81,7 @@ type InviteData struct { Heroes []internal.Hero InviteEvent *EventData NameEvent string // the content of m.room.name, NOT the calculated name + AvatarEvent string // the content of m.room.avatar, NOT the calculated avatar CanonicalAlias string LastMessageTimestamp uint64 Encrypted bool @@ -108,12 +117,15 @@ func NewInviteData(ctx context.Context, userID, roomID string, inviteState []jso id.IsDM = j.Get("is_direct").Bool() } else if target == j.Get("sender").Str { id.Heroes = append(id.Heroes, internal.Hero{ - ID: target, - Name: j.Get("content.displayname").Str, + ID: target, + Name: j.Get("content.displayname").Str, + Avatar: j.Get("content.avatar_url").Str, }) } case "m.room.name": id.NameEvent = j.Get("content.name").Str + case "m.room.avatar": + id.AvatarEvent = j.Get("content.avatar_url").Str case "m.room.canonical_alias": id.CanonicalAlias = j.Get("content.alias").Str case "m.room.encryption": @@ -147,6 +159,7 @@ func (i *InviteData) RoomMetadata() *internal.RoomMetadata { metadata := internal.NewRoomMetadata(i.roomID) metadata.Heroes = i.Heroes metadata.NameEvent = i.NameEvent + metadata.AvatarEvent = i.AvatarEvent metadata.CanonicalAlias = i.CanonicalAlias metadata.InviteCount = 1 metadata.JoinCount = 1 @@ -212,7 +225,7 @@ func (c *UserCache) Unsubscribe(id int) { // OnRegistered is called after the sync3.Dispatcher has successfully registered this // cache to receive updates. We use this to run some final initialisation logic that // is sensitive to race conditions; confusingly, most of the initialisation is driven -// externally by sync3.SyncLiveHandler.userCache. It's importatn that we don't spend too +// externally by sync3.SyncLiveHandler.userCaches. It's important that we don't spend too // long inside this function, because it is called within a global lock on the // sync3.Dispatcher (see sync3.Dispatcher.Register). func (c *UserCache) OnRegistered(ctx context.Context) error { @@ -305,6 +318,7 @@ func (c *UserCache) LazyLoadTimelines(ctx context.Context, loadPos int64, roomID for _, requestedRoomID := range roomIDs { latestEvents := roomIDToLatestEvents[requestedRoomID] urd, ok := c.roomToData[requestedRoomID] + if !ok { urd = NewUserRoomData() } @@ -328,7 +342,10 @@ func (c *UserCache) LoadRoomData(roomID string) UserRoomData { } type roomUpdateCache struct { - roomID string + roomID string + // globalRoomData is a snapshot of the global metadata for this room immediately + // after this update. It is a copy, specific to the given user whose Heroes + // field can be freely modified. globalRoomData *internal.RoomMetadata userRoomData *UserRoomData } @@ -572,6 +589,11 @@ func (c *UserCache) OnInvite(ctx context.Context, roomID string, inviteStateEven urd.HasLeft = false urd.HighlightCount = InvitesAreHighlightsValue urd.IsDM = inviteData.IsDM + logger.Warn().Msgf("TODO set avatar on invite") + //urd.ResolvedAvatarURL = inviteData.AvatarEvent + //if urd.ResolvedAvatarURL == "" && urd.IsDM && len(inviteData.Heroes) == 1 { + // urd.ResolvedAvatarURL = inviteData.Heroes[0].Avatar + //} urd.Invite = inviteData c.roomToDataMu.Lock() c.roomToData[roomID] = urd diff --git a/sync3/handler/connstate.go b/sync3/handler/connstate.go index 9f3f79ab..838c427c 100644 --- a/sync3/handler/connstate.go +++ b/sync3/handler/connstate.go @@ -596,6 +596,7 @@ func (s *ConnState) getInitialRoomData(ctx context.Context, roomSub sync3.RoomSu } rooms[roomID] = sync3.Room{ Name: internal.CalculateRoomName(metadata, 5), // TODO: customisable? + AvatarChange: sync3.NewAvatarChange(internal.CalculateAvatar(metadata)), NotificationCount: int64(userRoomData.NotificationCount), HighlightCount: int64(userRoomData.HighlightCount), Timeline: roomToTimeline[roomID], diff --git a/sync3/handler/connstate_live.go b/sync3/handler/connstate_live.go index a01a7ef6..d5b0975d 100644 --- a/sync3/handler/connstate_live.go +++ b/sync3/handler/connstate_live.go @@ -218,6 +218,11 @@ func (s *connStateLive) processLiveUpdate(ctx context.Context, up caches.Update, metadata.RemoveHero(s.userID) thisRoom.Name = internal.CalculateRoomName(metadata, 5) // TODO: customisable? } + if delta.RoomAvatarChanged { + metadata := roomUpdate.GlobalRoomMetadata() + metadata.RemoveHero(s.userID) + thisRoom.AvatarChange = sync3.NewAvatarChange(internal.CalculateAvatar(metadata)) + } if delta.InviteCountChanged { thisRoom.InvitedCount = &roomUpdate.GlobalRoomMetadata().InviteCount } @@ -290,8 +295,17 @@ func (s *connStateLive) processGlobalUpdates(ctx context.Context, builder *Rooms } } + metadata := rup.GlobalRoomMetadata().CopyHeroes() + metadata.RemoveHero(s.userID) + // TODO: if we change a room from being a DM to not being a DM, we should call + // SetRoom and recalculate avatars. To do that we'd need to + // - listen to m.direct global account data events + // - compute the symmetric difference between old and new + // - call SetRooms for each room in the difference. + // I'm assuming this happens so rarely that we can ignore this for now. PRs + // welcome if you a strong opinion to the contrary. delta = s.lists.SetRoom(sync3.RoomConnMetadata{ - RoomMetadata: *rup.GlobalRoomMetadata(), + RoomMetadata: *metadata, UserRoomData: *rup.UserRoomMetadata(), LastInterestedEventTimestamps: bumpTimestampInList, }) diff --git a/sync3/lists.go b/sync3/lists.go index f5e1dfbf..d8b404df 100644 --- a/sync3/lists.go +++ b/sync3/lists.go @@ -33,6 +33,7 @@ type RoomListDelta struct { type RoomDelta struct { RoomNameChanged bool + RoomAvatarChanged bool JoinCountChanged bool InviteCountChanged bool NotificationCountChanged bool @@ -73,6 +74,10 @@ func (s *InternalRequestLists) SetRoom(r RoomConnMetadata) (delta RoomDelta) { strings.Trim(internal.CalculateRoomName(&r.RoomMetadata, 5), "#!():_@"), ) } + delta.RoomAvatarChanged = !existing.SameRoomAvatar(&r.RoomMetadata) + if delta.RoomAvatarChanged { + r.ResolvedAvatarURL = internal.CalculateAvatar(&r.RoomMetadata) + } // Interpret the timestamp map on r as the changes we should apply atop the // existing timestamps. @@ -97,6 +102,7 @@ func (s *InternalRequestLists) SetRoom(r RoomConnMetadata) (delta RoomDelta) { r.CanonicalisedName = strings.ToLower( strings.Trim(internal.CalculateRoomName(&r.RoomMetadata, 5), "#!():_@"), ) + r.ResolvedAvatarURL = internal.CalculateAvatar(&r.RoomMetadata) // We'll automatically use the LastInterestedEventTimestamps provided by the // caller, so that recency sorts work. } diff --git a/sync3/room.go b/sync3/room.go index 6bf7c35d..3a5c24e2 100644 --- a/sync3/room.go +++ b/sync3/room.go @@ -2,7 +2,6 @@ package sync3 import ( "encoding/json" - "github.com/matrix-org/sliding-sync/internal" "github.com/matrix-org/sliding-sync/sync3/caches" @@ -10,6 +9,7 @@ import ( type Room struct { Name string `json:"name,omitempty"` + AvatarChange AvatarChange `json:"avatar,omitempty"` RequiredState []json.RawMessage `json:"required_state,omitempty"` Timeline []json.RawMessage `json:"timeline,omitempty"` InviteState []json.RawMessage `json:"invite_state,omitempty"` diff --git a/sync3/room_test.go b/sync3/room_test.go new file mode 100644 index 00000000..09ec058a --- /dev/null +++ b/sync3/room_test.go @@ -0,0 +1,74 @@ +package sync3 + +import ( + "encoding/json" + "fmt" + "github.com/tidwall/gjson" + "reflect" + "testing" +) + +func TestAvatarChangeMarshalling(t *testing.T) { + var url = "mxc://..." + testCases := []struct { + Name string + AvatarChange AvatarChange + Check func(avatar gjson.Result) error + }{ + { + Name: "Avatar exists", + AvatarChange: NewAvatarChange(url), + Check: func(avatar gjson.Result) error { + if !(avatar.Exists() && avatar.Type == gjson.String && avatar.Str == url) { + return fmt.Errorf("unexpected marshalled avatar: got %#v want %s", avatar, url) + } + return nil + }, + }, + { + Name: "Avatar doesn't exist", + AvatarChange: DeletedAvatar, + Check: func(avatar gjson.Result) error { + if !(avatar.Exists() && avatar.Type == gjson.Null) { + return fmt.Errorf("unexpected marshalled Avatar: got %#v want null", avatar) + } + return nil + }, + }, + { + Name: "Avatar unchanged", + AvatarChange: UnchangedAvatar, + Check: func(avatar gjson.Result) error { + if avatar.Exists() { + return fmt.Errorf("unexpected marshalled Avatar: got %#v want omitted", avatar) + } + return nil + }, + }, + } + + for _, tc := range testCases { + t.Run(tc.Name, func(t *testing.T) { + room := Room{AvatarChange: tc.AvatarChange} + marshalled, err := json.Marshal(room) + t.Logf("Marshalled to %s", string(marshalled)) + if err != nil { + t.Fatal(err) + } + avatar := gjson.GetBytes(marshalled, "avatar") + if err = tc.Check(avatar); err != nil { + t.Fatal(err) + } + + var unmarshalled Room + err = json.Unmarshal(marshalled, &unmarshalled) + if err != nil { + t.Fatal(err) + } + t.Logf("Unmarshalled to %#v", unmarshalled.AvatarChange) + if !reflect.DeepEqual(unmarshalled, room) { + t.Fatalf("Unmarshalled struct is different from original") + } + }) + } +} diff --git a/tests-e2e/client_test.go b/tests-e2e/client_test.go index 3ef9c817..a6982437 100644 --- a/tests-e2e/client_test.go +++ b/tests-e2e/client_test.go @@ -134,6 +134,7 @@ type CSAPI struct { Localpart string AccessToken string DeviceID string + AvatarURL string BaseURL string Client *http.Client // how long are we willing to wait for MustSyncUntil.... calls @@ -159,6 +160,16 @@ func (c *CSAPI) UploadContent(t *testing.T, fileBody []byte, fileName string, co return GetJSONFieldStr(t, body, "content_uri") } +// Use an empty string to remove your avatar. +func (c *CSAPI) SetAvatar(t *testing.T, avatarURL string) { + t.Helper() + reqBody := map[string]interface{}{ + "avatar_url": avatarURL, + } + c.MustDoFunc(t, "PUT", []string{"_matrix", "client", "v3", "profile", c.UserID, "avatar_url"}, WithJSONBody(t, reqBody)) + c.AvatarURL = avatarURL +} + // DownloadContent downloads media from the server, returning the raw bytes and the Content-Type. Fails the test on error. func (c *CSAPI) DownloadContent(t *testing.T, mxcUri string) ([]byte, string) { t.Helper() @@ -678,16 +689,32 @@ func (c *CSAPI) SlidingSyncUntilMembership(t *testing.T, pos string, roomID stri }) } - return c.SlidingSyncUntilEvent(t, pos, sync3.Request{ + return c.SlidingSyncUntil(t, pos, sync3.Request{ RoomSubscriptions: map[string]sync3.RoomSubscription{ roomID: { TimelineLimit: 10, }, }, - }, roomID, Event{ - Type: "m.room.member", - StateKey: &target.UserID, - Content: content, + }, func(r *sync3.Response) error { + room, ok := r.Rooms[roomID] + if !ok { + return fmt.Errorf("missing room %s", roomID) + } + for _, got := range room.Timeline { + wantEvent := Event{ + Type: "m.room.member", + StateKey: &target.UserID, + } + if err := eventsEqual([]Event{wantEvent}, []json.RawMessage{got}); err == nil { + gotMembership := gjson.GetBytes(got, "content.membership") + if gotMembership.Exists() && gotMembership.Type == gjson.String && gotMembership.Str == membership { + return nil + } + } else { + t.Log(err) + } + } + return fmt.Errorf("found room %s but missing event", roomID) }) } diff --git a/tests-e2e/lists_test.go b/tests-e2e/lists_test.go index 0d60b783..11dd97c3 100644 --- a/tests-e2e/lists_test.go +++ b/tests-e2e/lists_test.go @@ -1297,3 +1297,391 @@ func TestRangeOutsideTotalRooms(t *testing.T) { ), ) } + +// Nicked from Synapse's tests, see +// https://github.com/matrix-org/synapse/blob/2cacd0849a02d43f88b6c15ee862398159ab827c/tests/test_utils/__init__.py#L154-L161 +// Resolution: 1×1, MIME type: image/png, Extension: png, Size: 67 B +var smallPNG = []byte( + "\x89PNG\r\n\x1a\n\x00\x00\x00\rIHDR\x00\x00\x00\x01\x00\x00\x00\x01\x08\x06\x00\x00\x00\x1f\x15\xc4\x89\x00\x00\x00\nIDATx\x9cc\x00\x01\x00\x00\x05\x00\x01\r\n-\xb4\x00\x00\x00\x00IEND\xaeB`\x82", +) + +func TestAvatarFieldInRoomResponse(t *testing.T) { + alice := registerNamedUser(t, "alice") + bob := registerNamedUser(t, "bob") + chris := registerNamedUser(t, "chris") + + avatarURLs := map[string]struct{}{} + uploadAvatar := func(client *CSAPI, filename string) string { + avatar := alice.UploadContent(t, smallPNG, filename, "image/png") + if _, exists := avatarURLs[avatar]; exists { + t.Fatalf("New avatar %s has already been uploaded", avatar) + } + t.Logf("%s is uploaded as %s", filename, avatar) + avatarURLs[avatar] = struct{}{} + return avatar + } + + t.Log("Alice, Bob and Chris upload and set an avatar.") + aliceAvatar := uploadAvatar(alice, "alice.png") + bobAvatar := uploadAvatar(bob, "bob.png") + chrisAvatar := uploadAvatar(chris, "chris.png") + + alice.SetAvatar(t, aliceAvatar) + bob.SetAvatar(t, bobAvatar) + chris.SetAvatar(t, chrisAvatar) + + t.Log("Alice makes a public room, a DM with herself, a DM with Bob, a DM with Chris, and a group-DM with Bob and Chris.") + public := alice.CreateRoom(t, map[string]interface{}{"preset": "public_chat"}) + // TODO: you can create a DM with yourself e.g. as below. It probably ought to have + // your own face as an avatar. + // dmAlice := alice.CreateRoom(t, map[string]interface{}{ + // "preset": "trusted_private_chat", + // "is_direct": true, + // }) + dmBob := alice.CreateRoom(t, map[string]interface{}{ + "preset": "trusted_private_chat", + "is_direct": true, + "invite": []string{bob.UserID}, + }) + dmChris := alice.CreateRoom(t, map[string]interface{}{ + "preset": "trusted_private_chat", + "is_direct": true, + "invite": []string{chris.UserID}, + }) + dmBobChris := alice.CreateRoom(t, map[string]interface{}{ + "preset": "trusted_private_chat", + "is_direct": true, + "invite": []string{bob.UserID, chris.UserID}, + }) + + t.Logf("Rooms:\npublic=%s\ndmBob=%s\ndmChris=%s\ndmBobChris=%s", public, dmBob, dmChris, dmBobChris) + t.Log("Bob accepts his invites. Chris accepts none.") + bob.JoinRoom(t, dmBob, nil) + bob.JoinRoom(t, dmBobChris, nil) + + t.Log("Alice makes an initial sliding sync.") + res := alice.SlidingSync(t, sync3.Request{ + Lists: map[string]sync3.RequestList{ + "rooms": { + Ranges: sync3.SliceRanges{{0, 4}}, + }, + }, + }) + + t.Log("Alice should see each room in the sync response with an appropriate avatar") + m.MatchResponse( + t, + res, + m.MatchRoomSubscription(public, m.MatchRoomUnsetAvatar()), + m.MatchRoomSubscription(dmBob, m.MatchRoomAvatar(bob.AvatarURL)), + m.MatchRoomSubscription(dmChris, m.MatchRoomAvatar(chris.AvatarURL)), + m.MatchRoomSubscription(dmBobChris, m.MatchRoomUnsetAvatar()), + ) + + t.Run("Avatar not resent on message", func(t *testing.T) { + t.Log("Bob sends a sentinel message.") + sentinel := bob.SendEventSynced(t, dmBob, Event{ + Type: "m.room.message", + Content: map[string]interface{}{ + "body": "Hello world", + "msgtype": "m.text", + }, + }) + + t.Log("Alice syncs until she sees the sentinel. She should not see the DM avatar change.") + res = alice.SlidingSyncUntil(t, res.Pos, sync3.Request{}, func(response *sync3.Response) error { + matchNoAvatarChange := m.MatchRoomSubscription(dmBob, m.MatchRoomUnchangedAvatar()) + if err := matchNoAvatarChange(response); err != nil { + t.Fatalf("Saw DM avatar change: %s", err) + } + matchSentinel := m.MatchRoomSubscription(dmBob, MatchRoomTimelineMostRecent(1, []Event{{ID: sentinel}})) + return matchSentinel(response) + }) + }) + + t.Run("DM declined", func(t *testing.T) { + t.Log("Chris leaves his DM with Alice.") + chris.LeaveRoom(t, dmChris) + + t.Log("Alice syncs until she sees Chris's leave.") + res = alice.SlidingSyncUntilMembership(t, res.Pos, dmChris, chris, "leave") + + t.Log("Alice sees Chris's avatar vanish.") + m.MatchResponse(t, res, m.MatchRoomSubscription(dmChris, m.MatchRoomUnsetAvatar())) + }) + + t.Run("Group DM declined", func(t *testing.T) { + t.Log("Chris leaves his group DM with Alice and Bob.") + chris.LeaveRoom(t, dmBobChris) + + t.Log("Alice syncs until she sees Chris's leave.") + res = alice.SlidingSyncUntilMembership(t, res.Pos, dmBobChris, chris, "leave") + + t.Log("Alice sees the room's avatar change to Bob's avatar.") + // Because this is now a DM room with exactly one other (joined|invited) member. + m.MatchResponse(t, res, m.MatchRoomSubscription(dmBobChris, m.MatchRoomAvatar(bob.AvatarURL))) + }) + + t.Run("Bob's avatar change propagates", func(t *testing.T) { + t.Log("Bob changes his avatar.") + bobAvatar2 := uploadAvatar(bob, "bob2.png") + bob.SetAvatar(t, bobAvatar2) + + avatarChangeInDM := false + avatarChangeInGroupDM := false + t.Log("Alice syncs until she sees Bob's new avatar.") + res = alice.SlidingSyncUntil( + t, + res.Pos, + sync3.Request{}, + func(response *sync3.Response) error { + if !avatarChangeInDM { + err := m.MatchRoomSubscription(dmBob, m.MatchRoomAvatar(bob.AvatarURL))(response) + if err == nil { + avatarChangeInDM = true + } + } + + if !avatarChangeInGroupDM { + err := m.MatchRoomSubscription(dmBobChris, m.MatchRoomAvatar(bob.AvatarURL))(response) + if err == nil { + avatarChangeInGroupDM = true + } + } + + if avatarChangeInDM && avatarChangeInGroupDM { + return nil + } + return fmt.Errorf("still waiting: avatarChangeInDM=%t avatarChangeInGroupDM=%t", avatarChangeInDM, avatarChangeInGroupDM) + }, + ) + + t.Log("Bob removes his avatar.") + bob.SetAvatar(t, "") + + avatarChangeInDM = false + avatarChangeInGroupDM = false + t.Log("Alice syncs until she sees Bob's avatars vanish.") + res = alice.SlidingSyncUntil( + t, + res.Pos, + sync3.Request{}, + func(response *sync3.Response) error { + if !avatarChangeInDM { + err := m.MatchRoomSubscription(dmBob, m.MatchRoomUnsetAvatar())(response) + if err == nil { + avatarChangeInDM = true + } else { + t.Log(err) + } + } + + if !avatarChangeInGroupDM { + err := m.MatchRoomSubscription(dmBobChris, m.MatchRoomUnsetAvatar())(response) + if err == nil { + avatarChangeInGroupDM = true + } else { + t.Log(err) + } + } + + if avatarChangeInDM && avatarChangeInGroupDM { + return nil + } + return fmt.Errorf("still waiting: avatarChangeInDM=%t avatarChangeInGroupDM=%t", avatarChangeInDM, avatarChangeInGroupDM) + }, + ) + + }) + + t.Run("Explicit avatar propagates in non-DM room", func(t *testing.T) { + t.Log("Alice sets an avatar for the public room.") + publicAvatar := uploadAvatar(alice, "public.png") + alice.SetState(t, public, "m.room.avatar", "", map[string]interface{}{ + "url": publicAvatar, + }) + t.Log("Alice syncs until she sees that avatar.") + res = alice.SlidingSyncUntil( + t, + res.Pos, + sync3.Request{}, + m.MatchRoomSubscriptions(map[string][]m.RoomMatcher{ + public: {m.MatchRoomAvatar(publicAvatar)}, + }), + ) + + t.Log("Alice changes the avatar for the public room.") + publicAvatar2 := uploadAvatar(alice, "public2.png") + alice.SetState(t, public, "m.room.avatar", "", map[string]interface{}{ + "url": publicAvatar2, + }) + t.Log("Alice syncs until she sees that avatar.") + res = alice.SlidingSyncUntil( + t, + res.Pos, + sync3.Request{}, + m.MatchRoomSubscriptions(map[string][]m.RoomMatcher{ + public: {m.MatchRoomAvatar(publicAvatar2)}, + }), + ) + + t.Log("Alice removes the avatar for the public room.") + alice.SetState(t, public, "m.room.avatar", "", map[string]interface{}{}) + t.Log("Alice syncs until she sees that avatar vanish.") + res = alice.SlidingSyncUntil( + t, + res.Pos, + sync3.Request{}, + m.MatchRoomSubscriptions(map[string][]m.RoomMatcher{ + public: {m.MatchRoomUnsetAvatar()}, + }), + ) + }) + + t.Run("Explicit avatar propagates in DM room", func(t *testing.T) { + t.Log("Alice re-invites Chris to their DM.") + alice.InviteRoom(t, dmChris, chris.UserID) + + t.Log("Alice syncs until she sees her invitation to Chris.") + res = alice.SlidingSyncUntilMembership(t, res.Pos, dmChris, chris, "invite") + + t.Log("Alice should see the DM with Chris's avatar.") + m.MatchResponse(t, res, m.MatchRoomSubscription(dmChris, m.MatchRoomAvatar(chris.AvatarURL))) + + t.Log("Chris joins the room.") + chris.JoinRoom(t, dmChris, nil) + + t.Log("Alice syncs until she sees Chris's join.") + res = alice.SlidingSyncUntilMembership(t, res.Pos, dmChris, chris, "join") + + t.Log("Alice shouldn't see the DM's avatar change..") + m.MatchResponse(t, res, m.MatchRoomSubscription(dmChris, m.MatchRoomUnchangedAvatar())) + + t.Log("Chris gives their DM a bespoke avatar.") + dmAvatar := uploadAvatar(chris, "dm.png") + chris.SetState(t, dmChris, "m.room.avatar", "", map[string]interface{}{ + "url": dmAvatar, + }) + + t.Log("Alice syncs until she sees that avatar.") + alice.SlidingSyncUntil(t, res.Pos, sync3.Request{}, m.MatchRoomSubscription(dmChris, m.MatchRoomAvatar(dmAvatar))) + + t.Log("Chris changes his global avatar, which adds a join event to the room.") + chrisAvatar2 := uploadAvatar(chris, "chris2.png") + chris.SetAvatar(t, chrisAvatar2) + + t.Log("Alice syncs until she sees that join event.") + res = alice.SlidingSyncUntilMembership(t, res.Pos, dmChris, chris, "join") + + t.Log("Her response should have either no avatar change, or the same bespoke avatar.") + // No change, ideally, but repeating the same avatar isn't _wrong_ + m.MatchResponse(t, res, m.MatchRoomSubscription(dmChris, func(r sync3.Room) error { + noChangeErr := m.MatchRoomUnchangedAvatar()(r) + sameBespokeAvatarErr := m.MatchRoomAvatar(dmAvatar)(r) + if noChangeErr == nil || sameBespokeAvatarErr == nil { + return nil + } + return fmt.Errorf("expected no change or the same bespoke avatar (%s), got '%s'", dmAvatar, r.AvatarChange) + })) + + t.Log("Chris updates the DM's avatar.") + dmAvatar2 := uploadAvatar(chris, "dm2.png") + chris.SetState(t, dmChris, "m.room.avatar", "", map[string]interface{}{ + "url": dmAvatar2, + }) + + t.Log("Alice syncs until she sees that avatar.") + res = alice.SlidingSyncUntil(t, res.Pos, sync3.Request{}, m.MatchRoomSubscription(dmChris, m.MatchRoomAvatar(dmAvatar2))) + + t.Log("Chris removes the DM's avatar.") + chris.SetState(t, dmChris, "m.room.avatar", "", map[string]interface{}{}) + + t.Log("Alice syncs until the DM avatar returns to Chris's most recent avatar.") + res = alice.SlidingSyncUntil(t, res.Pos, sync3.Request{}, m.MatchRoomSubscription(dmChris, m.MatchRoomAvatar(chris.AvatarURL))) + }) + + t.Run("Changing DM flag", func(t *testing.T) { + t.Skip("TODO: unimplemented") + t.Log("Alice clears the DM flag on Bob's room.") + alice.SetGlobalAccountData(t, "m.direct", map[string]interface{}{ + "content": map[string][]string{ + bob.UserID: {}, // no dmBob here + chris.UserID: {dmChris, dmBobChris}, + }, + }) + + t.Log("Alice syncs until she sees a new set of account data.") + res = alice.SlidingSyncUntil(t, res.Pos, sync3.Request{ + Extensions: extensions.Request{ + AccountData: &extensions.AccountDataRequest{ + extensions.Core{Enabled: &boolTrue}, + }, + }, + }, func(response *sync3.Response) error { + if response.Extensions.AccountData == nil { + return fmt.Errorf("no account data yet") + } + if len(response.Extensions.AccountData.Global) == 0 { + return fmt.Errorf("no global account data yet") + } + return nil + }) + + t.Log("The DM with Bob should no longer be a DM and should no longer have an avatar.") + m.MatchResponse(t, res, m.MatchRoomSubscription(dmBob, func(r sync3.Room) error { + if r.IsDM { + return fmt.Errorf("dmBob is still a DM") + } + return m.MatchRoomUnsetAvatar()(r) + })) + + t.Log("Alice sets the DM flag on Bob's room.") + alice.SetGlobalAccountData(t, "m.direct", map[string]interface{}{ + "content": map[string][]string{ + bob.UserID: {dmBob}, // dmBob reinstated + chris.UserID: {dmChris, dmBobChris}, + }, + }) + + t.Log("Alice syncs until she sees a new set of account data.") + res = alice.SlidingSyncUntil(t, res.Pos, sync3.Request{ + Extensions: extensions.Request{ + AccountData: &extensions.AccountDataRequest{ + extensions.Core{Enabled: &boolTrue}, + }, + }, + }, func(response *sync3.Response) error { + if response.Extensions.AccountData == nil { + return fmt.Errorf("no account data yet") + } + if len(response.Extensions.AccountData.Global) == 0 { + return fmt.Errorf("no global account data yet") + } + return nil + }) + + t.Log("The room should have Bob's avatar again.") + m.MatchResponse(t, res, m.MatchRoomSubscription(dmBob, func(r sync3.Room) error { + if !r.IsDM { + return fmt.Errorf("dmBob is still not a DM") + } + return m.MatchRoomAvatar(bob.AvatarURL)(r) + })) + + }) + + t.Run("See avatar when invited", func(t *testing.T) { + t.Log("Chris invites Alice to a DM.") + dmInvited := chris.CreateRoom(t, map[string]interface{}{ + "preset": "trusted_private_chat", + "is_direct": true, + "invite": []string{alice.UserID}, + }) + + t.Log("Alice syncs until she sees the invite.") + res = alice.SlidingSyncUntilMembership(t, res.Pos, dmInvited, alice, "invite") + + t.Log("The new room should use Chris's avatar.") + m.MatchResponse(t, res, m.MatchRoomSubscription(dmInvited, m.MatchRoomAvatar(chris.AvatarURL))) + }) +} diff --git a/testutils/m/match.go b/testutils/m/match.go index a7b2f635..001135bc 100644 --- a/testutils/m/match.go +++ b/testutils/m/match.go @@ -39,6 +39,39 @@ func MatchRoomName(name string) RoomMatcher { } } +// MatchRoomAvatar builds a RoomMatcher which checks that the given room response has +// set the room's avatar to the given value. +func MatchRoomAvatar(wantAvatar string) RoomMatcher { + return func(r sync3.Room) error { + if string(r.AvatarChange) != wantAvatar { + return fmt.Errorf("MatchRoomAvatar: got \"%s\" want \"%s\"", r.AvatarChange, wantAvatar) + } + return nil + } +} + +// MatchRoomUnsetAvatar builds a RoomMatcher which checks that the given room has no +// avatar, or has had its avatar deleted. +func MatchRoomUnsetAvatar() RoomMatcher { + return func(r sync3.Room) error { + if r.AvatarChange != sync3.DeletedAvatar { + return fmt.Errorf("MatchRoomAvatar: got \"%s\" want \"%s\"", r.AvatarChange, sync3.DeletedAvatar) + } + return nil + } +} + +// MatchRoomUnchangedAvatar builds a RoomMatcher which checks that the given room has no +// change to its avatar, or has had its avatar deleted. +func MatchRoomUnchangedAvatar() RoomMatcher { + return func(r sync3.Room) error { + if r.AvatarChange != sync3.UnchangedAvatar { + return fmt.Errorf("MatchRoomAvatar: got \"%s\" want \"%s\"", r.AvatarChange, sync3.UnchangedAvatar) + } + return nil + } +} + func MatchJoinCount(count int) RoomMatcher { return func(r sync3.Room) error { if r.JoinedCount != count { @@ -222,11 +255,11 @@ func MatchRoomSubscription(roomID string, matchers ...RoomMatcher) RespMatcher { return func(res *sync3.Response) error { room, ok := res.Rooms[roomID] if !ok { - return fmt.Errorf("MatchRoomSubscription: want sub for %s but it was missing", roomID) + return fmt.Errorf("MatchRoomSubscription[%s]: want sub but it was missing", roomID) } for _, m := range matchers { if err := m(room); err != nil { - return fmt.Errorf("MatchRoomSubscription: %s", err) + return fmt.Errorf("MatchRoomSubscription[%s]: %s", roomID, err) } } return nil @@ -644,6 +677,15 @@ func LogResponse(t *testing.T) RespMatcher { } } +// LogRooms is like LogResponse, but only logs the rooms section of the response. +func LogRooms(t *testing.T) RespMatcher { + return func(res *sync3.Response) error { + dump, _ := json.MarshalIndent(res.Rooms, "", " ") + t.Logf("Response rooms were: %s", dump) + return nil + } +} + func CheckList(listKey string, res sync3.ResponseList, matchers ...ListMatcher) error { for _, m := range matchers { if err := m(res); err != nil { @@ -686,13 +728,15 @@ func MatchLists(matchers map[string][]ListMatcher) RespMatcher { } } +const AnsiRedForeground = "\x1b[31m" +const AnsiResetForeground = "\x1b[39m " + func MatchResponse(t *testing.T, res *sync3.Response, matchers ...RespMatcher) { t.Helper() for _, m := range matchers { err := m(res) if err != nil { - b, _ := json.Marshal(res) - t.Errorf("MatchResponse: %s\n%+v", err, string(b)) + t.Errorf("%vMatchResponse: %s%v", AnsiRedForeground, err, AnsiResetForeground) } } }