-
Notifications
You must be signed in to change notification settings - Fork 69
feat: support for room v11 #858
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -15,6 +15,7 @@ import ( | |
| "github.com/matrix-org/complement/helpers" | ||
| "github.com/matrix-org/complement/match" | ||
| "github.com/matrix-org/complement/must" | ||
| "github.com/matrix-org/gomatrixserverlib" | ||
| ) | ||
|
|
||
| func TestRoomCreationReportsEventsToMyself(t *testing.T) { | ||
|
|
@@ -26,6 +27,7 @@ func TestRoomCreationReportsEventsToMyself(t *testing.T) { | |
| LocalpartSuffix: "bob", | ||
| Password: "bobpassword", | ||
| }) | ||
| defaultVer := alice.GetDefaultRoomVersion(t) | ||
| roomID := alice.MustCreateRoom(t, map[string]interface{}{}) | ||
|
|
||
| t.Run("parallel", func(t *testing.T) { | ||
|
|
@@ -38,7 +40,10 @@ func TestRoomCreationReportsEventsToMyself(t *testing.T) { | |
| return false | ||
| } | ||
| must.Equal(t, ev.Get("sender").Str, alice.UserID, "wrong sender") | ||
| must.Equal(t, ev.Get("content").Get("creator").Str, alice.UserID, "wrong content.creator") | ||
| // The creator field was removed in room version 11 (MSC4239). | ||
| if gomatrixserverlib.MustGetRoomVersion(defaultVer).CreatorInCreateEvent() { | ||
| must.Equal(t, ev.Get("content").Get("creator").Str, alice.UserID, "wrong content.creator") | ||
| } | ||
|
Comment on lines
+43
to
+46
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Looks like we can use the same
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yes indeed, done! |
||
| return true | ||
| })) | ||
| }) | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -497,6 +497,21 @@ func TestOutboundFederationEventSizeGetMissingEvents(t *testing.T) { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }).Methods("POST") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ver := alice.GetDefaultRoomVersion(t) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // This test crafts a "bad" event which state_key is 280 bytes but only 70 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // codepoints. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Room version 11 in Synapse switched from using codepoints to using | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // bytes. Which means the 280-byte state_key would be rejected immediately. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Use room version 10 in that case so the codepoint-based limit is in effect. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Since upgrading a room (for example from v10 to v11) won't carry the event | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // (a new room is created), we don't have to worry about v10 room events in a v11 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // room. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // So this test is essentially skipped for any default room v11 or higher. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if gomatrixserverlib.MustGetRoomVersion(ver).StrictEventByteLimits() { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ver = gomatrixserverlib.RoomVersion("10") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
499
to
+514
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This seems like yet another case that we should handle with a new You can see how we do this in Synapse
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yes indeed, done!
Comment on lines
499
to
+514
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Seems like this test would be better if we just always used room version 10 🤔 . The current setup is only better if we're trying to support homeservers that don't support room version 10 yet (do we care about that?)
Suggested change
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yea I thought it's better to be flexible (since it's not a big change in the test code), but I can apply your suggestion if you think it's better
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Probably simplest to converge on room version 10. If there is a use case for the alternative, we can go that route but the comment should better explain the intention. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| charlie := srv.UserID("charlie") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| room := srv.MustMakeRoom(t, ver, federation.InitialRoomEvents(ver, charlie)) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| roomAlias := srv.MakeAliasMapping("flibble", room.RoomID) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TODO: Wait for matrix-org/gomatrixserverlib#465 to merge and update the
gomatrixserverlibversion in this PR