Skip to content

Add ServerRoomImpl to allow tests to reuse helpers with custom code #766

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

Merged
merged 5 commits into from
Mar 3, 2025

Conversation

kegsay
Copy link
Member

@kegsay kegsay commented Feb 28, 2025

With the addition of #765 it's now possible for tests to add a range of good/bad/broken rooms to Complement's internal federation server. However, these rooms cannot reuse any of the existing helper functions. To aid this, logic which was once part of the federation server itself (e.g mapping /send_join responses to internal ServerRoom state) is now transferred to a new ServerRoomImpl interface. The federation server's helper functions call this interface, allowing tests to swap out functions for their broken rooms.

For example, we may want to reuse the very convenient HandleMakeSendJoinRequests but modify the join proto event to include malformed fields. Before this PR this simply wasn't possible as we hard-coded the happy case. Tests can now do:

room := federation.NewServerRoom(gomatrixserverlib.RoomVersionV1, "!foo:localhost")
room.ServerRoomImpl = &federation.ServerRoomImplCustom{
	ServerRoomImplDefault: federation.ServerRoomImplDefault{Room: room},
	ProtoEventCreatorFn: func(def federation.ServerRoomImpl, ev federation.Event) (*gomatrixserverlib.ProtoEvent, error) {
		if ev.Type == spec.MRoomMember {
			ev.Unsigned = map[string]any{"something": "custom"}
		}
		return def.ProtoEventCreator(ev)
	},
}

This allows tests to replace how we map Complement Events to PDUs.
This does not yet do everything as the mapping of /send_join to Rooms
isn't factored out.
@kegsay kegsay requested review from a team as code owners February 28, 2025 16:36
@kegsay kegsay requested a review from devonh February 28, 2025 16:36
Copy link
Contributor

@devonh devonh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

New API functions should ideally have doc comments to explain their usage.
Even though they are pretty clear just from name & impl alone.

Otherwise these changes look promising and will make it much easier to mess around with future event changes without having to break into complement's internals.

Type: "m.room.member",
StateKey: &userID,
Content: map[string]interface{}{
"membership": spec.Join, // XXX this feels wrong?
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why? It seems okay to me.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's MakeRespMakeKnock so surely should be knock not join.

@kegsay
Copy link
Member Author

kegsay commented Mar 3, 2025

Will chuck some comments in then merge, thanks!

@kegsay kegsay merged commit 81c0d9a into main Mar 3, 2025
4 checks passed
@kegsay kegsay deleted the kegan/proto-event branch March 3, 2025 09:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants