Remove empty fields from `/sync` response (#2755)

First attempt at removing empty fields from `/sync` responses. Needs
https://github.com/matrix-org/sytest/pull/1298 to keep Sytest happy.

Co-authored-by: Neil Alexander <neilalexander@users.noreply.github.com>

7 files changed, 178 insertions, 90 deletions

diff --git a/syncapi/internal/keychange_test.go b/syncapi/internal/keychange_test.go
index 3b9c8221..53f3e5a4 100644
--- a/syncapi/internal/keychange_test.go
+++ b/syncapi/internal/keychange_test.go
@@ -170,9 +170,12 @@ func joinResponseWithRooms(syncResponse *types.Response, userID string, roomIDs
 				Content:  []byte(`{"membership":"join"}`),
 			},
 		}
-
-		jr := syncResponse.Rooms.Join[roomID]
-		jr.State.Events = roomEvents
+		jr, ok := syncResponse.Rooms.Join[roomID]
+		if !ok {
+			jr = types.NewJoinResponse()
+		}
+		jr.Timeline = &types.Timeline{}
+		jr.State = &types.ClientEvents{Events: roomEvents}
 		syncResponse.Rooms.Join[roomID] = jr
 	}
 	return syncResponse
@@ -191,8 +194,11 @@ func leaveResponseWithRooms(syncResponse *types.Response, userID string, roomIDs
 			},
 		}
 
-		lr := syncResponse.Rooms.Leave[roomID]
-		lr.Timeline.Events = roomEvents
+		lr, ok := syncResponse.Rooms.Leave[roomID]
+		if !ok {
+			lr = types.NewLeaveResponse()
+		}
+		lr.Timeline = &types.Timeline{Events: roomEvents}
 		syncResponse.Rooms.Leave[roomID] = lr
 	}
 	return syncResponse
@@ -328,9 +334,13 @@ func TestKeyChangeCatchupNoNewJoinsButMessages(t *testing.T) {
 		},
 	}
 
-	jr := syncResponse.Rooms.Join[roomID]
-	jr.State.Events = roomStateEvents
-	jr.Timeline.Events = roomTimelineEvents
+	jr, ok := syncResponse.Rooms.Join[roomID]
+	if !ok {
+		jr = types.NewJoinResponse()
+	}
+
+	jr.State = &types.ClientEvents{Events: roomStateEvents}
+	jr.Timeline = &types.Timeline{Events: roomTimelineEvents}
 	syncResponse.Rooms.Join[roomID] = jr
 
 	rsAPI := &mockRoomserverAPI{
@@ -442,8 +452,11 @@ func TestKeyChangeCatchupChangeAndLeftSameRoom(t *testing.T) {
 		},
 	}
 
-	lr := syncResponse.Rooms.Leave[roomID]
-	lr.Timeline.Events = roomEvents
+	lr, ok := syncResponse.Rooms.Leave[roomID]
+	if !ok {
+		lr = types.NewLeaveResponse()
+	}
+	lr.Timeline = &types.Timeline{Events: roomEvents}
 	syncResponse.Rooms.Leave[roomID] = lr
 
 	rsAPI := &mockRoomserverAPI{
diff --git a/syncapi/streams/stream_accountdata.go b/syncapi/streams/stream_accountdata.go
index 3f2f7d13..3593a656 100644
--- a/syncapi/streams/stream_accountdata.go
+++ b/syncapi/streams/stream_accountdata.go
@@ -90,9 +90,9 @@ func (p *AccountDataStreamProvider) IncrementalSync(
 				}
 			} else {
 				if roomData, ok := dataRes.RoomAccountData[roomID][dataType]; ok {
-					joinData := *types.NewJoinResponse()
-					if existing, ok := req.Response.Rooms.Join[roomID]; ok {
-						joinData = existing
+					joinData, ok := req.Response.Rooms.Join[roomID]
+					if !ok {
+						joinData = types.NewJoinResponse()
 					}
 					joinData.AccountData.Events = append(
 						joinData.AccountData.Events,
diff --git a/syncapi/streams/stream_invite.go b/syncapi/streams/stream_invite.go
index 17b3b843..7875ffa3 100644
--- a/syncapi/streams/stream_invite.go
+++ b/syncapi/streams/stream_invite.go
@@ -65,7 +65,7 @@ func (p *InviteStreamProvider) IncrementalSync(
 			continue
 		}
 		ir := types.NewInviteResponse(inviteEvent)
-		req.Response.Rooms.Invite[roomID] = *ir
+		req.Response.Rooms.Invite[roomID] = ir
 	}
 
 	// When doing an initial sync, we don't want to add retired invites, as this
@@ -87,7 +87,7 @@ func (p *InviteStreamProvider) IncrementalSync(
 				Type:           "m.room.member",
 				Content:        gomatrixserverlib.RawJSON(`{"membership":"leave"}`),
 			})
-			req.Response.Rooms.Leave[roomID] = *lr
+			req.Response.Rooms.Leave[roomID] = lr
 		}
 	}
 
diff --git a/syncapi/streams/stream_pdu.go b/syncapi/streams/stream_pdu.go
index d252265f..613ac434 100644
--- a/syncapi/streams/stream_pdu.go
+++ b/syncapi/streams/stream_pdu.go
@@ -106,7 +106,7 @@ func (p *PDUStreamProvider) CompleteSync(
 			}
 			continue
 		}
-		req.Response.Rooms.Join[roomID] = *jr
+		req.Response.Rooms.Join[roomID] = jr
 		req.Rooms[roomID] = gomatrixserverlib.Join
 	}
 
@@ -129,7 +129,7 @@ func (p *PDUStreamProvider) CompleteSync(
 				}
 				continue
 			}
-			req.Response.Rooms.Peek[peek.RoomID] = *jr
+			req.Response.Rooms.Peek[peek.RoomID] = jr
 		}
 	}
 
@@ -320,7 +320,7 @@ func (p *PDUStreamProvider) addRoomDeltaToResponse(
 		// didn't "remove" events, return that the response is limited.
 		jr.Timeline.Limited = limited && len(events) == len(recentEvents)
 		jr.State.Events = gomatrixserverlib.HeaderedToClientEvents(delta.StateEvents, gomatrixserverlib.FormatSync)
-		res.Rooms.Join[delta.RoomID] = *jr
+		res.Rooms.Join[delta.RoomID] = jr
 
 	case gomatrixserverlib.Peek:
 		jr := types.NewJoinResponse()
@@ -329,7 +329,7 @@ func (p *PDUStreamProvider) addRoomDeltaToResponse(
 		jr.Timeline.Events = gomatrixserverlib.HeaderedToClientEvents(recentEvents, gomatrixserverlib.FormatSync)
 		jr.Timeline.Limited = limited
 		jr.State.Events = gomatrixserverlib.HeaderedToClientEvents(delta.StateEvents, gomatrixserverlib.FormatSync)
-		res.Rooms.Peek[delta.RoomID] = *jr
+		res.Rooms.Peek[delta.RoomID] = jr
 
 	case gomatrixserverlib.Leave:
 		fallthrough // transitions to leave are the same as ban
@@ -342,7 +342,7 @@ func (p *PDUStreamProvider) addRoomDeltaToResponse(
 		// didn't "remove" events, return that the response is limited.
 		lr.Timeline.Limited = limited && len(events) == len(recentEvents)
 		lr.State.Events = gomatrixserverlib.HeaderedToClientEvents(delta.StateEvents, gomatrixserverlib.FormatSync)
-		res.Rooms.Leave[delta.RoomID] = *lr
+		res.Rooms.Leave[delta.RoomID] = lr
 	}
 
 	return latestPosition, nil
diff --git a/syncapi/streams/stream_receipt.go b/syncapi/streams/stream_receipt.go
index 8818a553..76927cc3 100644
--- a/syncapi/streams/stream_receipt.go
+++ b/syncapi/streams/stream_receipt.go
@@ -4,9 +4,10 @@ import (
 	"context"
 	"encoding/json"
 
+	"github.com/matrix-org/gomatrixserverlib"
+
 	"github.com/matrix-org/dendrite/syncapi/storage"
 	"github.com/matrix-org/dendrite/syncapi/types"
-	"github.com/matrix-org/gomatrixserverlib"
 )
 
 type ReceiptStreamProvider struct {
@@ -76,7 +77,7 @@ func (p *ReceiptStreamProvider) IncrementalSync(
 			continue
 		}
 
-		jr := *types.NewJoinResponse()
+		jr := types.NewJoinResponse()
 		if existing, ok := req.Response.Rooms.Join[roomID]; ok {
 			jr = existing
 		}
diff --git a/syncapi/streams/stream_typing.go b/syncapi/streams/stream_typing.go
index a6f7c7a0..84c199b3 100644
--- a/syncapi/streams/stream_typing.go
+++ b/syncapi/streams/stream_typing.go
@@ -4,10 +4,11 @@ import (
 	"context"
 	"encoding/json"
 
+	"github.com/matrix-org/gomatrixserverlib"
+
 	"github.com/matrix-org/dendrite/internal/caching"
 	"github.com/matrix-org/dendrite/syncapi/storage"
 	"github.com/matrix-org/dendrite/syncapi/types"
-	"github.com/matrix-org/gomatrixserverlib"
 )
 
 type TypingStreamProvider struct {
@@ -35,9 +36,9 @@ func (p *TypingStreamProvider) IncrementalSync(
 			continue
 		}
 
-		jr := *types.NewJoinResponse()
-		if existing, ok := req.Response.Rooms.Join[roomID]; ok {
-			jr = existing
+		jr, ok := req.Response.Rooms.Join[roomID]
+		if !ok {
+			jr = types.NewJoinResponse()
 		}
 
 		if users, updated := p.EDUCache.GetTypingUsersIfUpdatedAfter(
diff --git a/syncapi/types/types.go b/syncapi/types/types.go
index 3b85db4a..b6d340f9 100644
--- a/syncapi/types/types.go
+++ b/syncapi/types/types.go
@@ -327,29 +327,57 @@ type PrevEventRef struct {
 	PrevSender    string          `json:"prev_sender"`
 }
 
+type DeviceLists struct {
+	Changed []string `json:"changed,omitempty"`
+	Left    []string `json:"left,omitempty"`
+}
+
+type RoomsResponse struct {
+	Join   map[string]*JoinResponse   `json:"join,omitempty"`
+	Peek   map[string]*JoinResponse   `json:"peek,omitempty"`
+	Invite map[string]*InviteResponse `json:"invite,omitempty"`
+	Leave  map[string]*LeaveResponse  `json:"leave,omitempty"`
+}
+
+type ToDeviceResponse struct {
+	Events []gomatrixserverlib.SendToDeviceEvent `json:"events,omitempty"`
+}
+
 // Response represents a /sync API response. See https://matrix.org/docs/spec/client_server/r0.2.0.html#get-matrix-client-r0-sync
 type Response struct {
-	NextBatch   StreamingToken `json:"next_batch"`
-	AccountData struct {
-		Events []gomatrixserverlib.ClientEvent `json:"events,omitempty"`
-	} `json:"account_data,omitempty"`
-	Presence struct {
-		Events []gomatrixserverlib.ClientEvent `json:"events,omitempty"`
-	} `json:"presence,omitempty"`
-	Rooms struct {
-		Join   map[string]JoinResponse   `json:"join,omitempty"`
-		Peek   map[string]JoinResponse   `json:"peek,omitempty"`
-		Invite map[string]InviteResponse `json:"invite,omitempty"`
-		Leave  map[string]LeaveResponse  `json:"leave,omitempty"`
-	} `json:"rooms,omitempty"`
-	ToDevice struct {
-		Events []gomatrixserverlib.SendToDeviceEvent `json:"events,omitempty"`
-	} `json:"to_device,omitempty"`
-	DeviceLists struct {
-		Changed []string `json:"changed,omitempty"`
-		Left    []string `json:"left,omitempty"`
-	} `json:"device_lists,omitempty"`
-	DeviceListsOTKCount map[string]int `json:"device_one_time_keys_count,omitempty"`
+	NextBatch           StreamingToken    `json:"next_batch"`
+	AccountData         *ClientEvents     `json:"account_data,omitempty"`
+	Presence            *ClientEvents     `json:"presence,omitempty"`
+	Rooms               *RoomsResponse    `json:"rooms,omitempty"`
+	ToDevice            *ToDeviceResponse `json:"to_device,omitempty"`
+	DeviceLists         *DeviceLists      `json:"device_lists,omitempty"`
+	DeviceListsOTKCount map[string]int    `json:"device_one_time_keys_count,omitempty"`
+}
+
+func (r Response) MarshalJSON() ([]byte, error) {
+	type alias Response
+	a := alias(r)
+	if r.AccountData != nil && len(r.AccountData.Events) == 0 {
+		a.AccountData = nil
+	}
+	if r.Presence != nil && len(r.Presence.Events) == 0 {
+		a.Presence = nil
+	}
+	if r.DeviceLists != nil {
+		if len(r.DeviceLists.Left) == 0 && len(r.DeviceLists.Changed) == 0 {
+			a.DeviceLists = nil
+		}
+	}
+	if r.Rooms != nil {
+		if len(r.Rooms.Join) == 0 && len(r.Rooms.Peek) == 0 &&
+			len(r.Rooms.Invite) == 0 && len(r.Rooms.Leave) == 0 {
+			a.Rooms = nil
+		}
+	}
+	if r.ToDevice != nil && len(r.ToDevice.Events) == 0 {
+		a.ToDevice = nil
+	}
+	return json.Marshal(a)
 }
 
 func (r *Response) HasUpdates() bool {
@@ -370,18 +398,21 @@ func NewResponse() *Response {
 	res := Response{}
 	// Pre-initialise the maps. Synapse will return {} even if there are no rooms under a specific section,
 	// so let's do the same thing. Bonus: this means we can't get dreaded 'assignment to entry in nil map' errors.
-	res.Rooms.Join = map[string]JoinResponse{}
-	res.Rooms.Peek = map[string]JoinResponse{}
-	res.Rooms.Invite = map[string]InviteResponse{}
-	res.Rooms.Leave = map[string]LeaveResponse{}
+	res.Rooms = &RoomsResponse{
+		Join:   map[string]*JoinResponse{},
+		Peek:   map[string]*JoinResponse{},
+		Invite: map[string]*InviteResponse{},
+		Leave:  map[string]*LeaveResponse{},
+	}
 
 	// Also pre-intialise empty slices or else we'll insert 'null' instead of '[]' for the value.
 	// TODO: We really shouldn't have to do all this to coerce encoding/json to Do The Right Thing. We should
 	//       really be using our own Marshal/Unmarshal implementations otherwise this may prove to be a CPU bottleneck.
 	//       This also applies to NewJoinResponse, NewInviteResponse and NewLeaveResponse.
-	res.AccountData.Events = []gomatrixserverlib.ClientEvent{}
-	res.Presence.Events = []gomatrixserverlib.ClientEvent{}
-	res.ToDevice.Events = []gomatrixserverlib.SendToDeviceEvent{}
+	res.AccountData = &ClientEvents{}
+	res.Presence = &ClientEvents{}
+	res.DeviceLists = &DeviceLists{}
+	res.ToDevice = &ToDeviceResponse{}
 	res.DeviceListsOTKCount = map[string]int{}
 
 	return &res
@@ -403,38 +434,73 @@ type UnreadNotifications struct {
 	NotificationCount int `json:"notification_count"`
 }
 
+type ClientEvents struct {
+	Events []gomatrixserverlib.ClientEvent `json:"events,omitempty"`
+}
+
+type Timeline struct {
+	Events    []gomatrixserverlib.ClientEvent `json:"events"`
+	Limited   bool                            `json:"limited"`
+	PrevBatch *TopologyToken                  `json:"prev_batch,omitempty"`
+}
+
+type Summary struct {
+	Heroes             []string `json:"m.heroes,omitempty"`
+	JoinedMemberCount  *int     `json:"m.joined_member_count,omitempty"`
+	InvitedMemberCount *int     `json:"m.invited_member_count,omitempty"`
+}
+
 // JoinResponse represents a /sync response for a room which is under the 'join' or 'peek' key.
 type JoinResponse struct {
-	Summary struct {
-		Heroes             []string `json:"m.heroes,omitempty"`
-		JoinedMemberCount  *int     `json:"m.joined_member_count,omitempty"`
-		InvitedMemberCount *int     `json:"m.invited_member_count,omitempty"`
-	} `json:"summary"`
-	State struct {
-		Events []gomatrixserverlib.ClientEvent `json:"events"`
-	} `json:"state"`
-	Timeline struct {
-		Events    []gomatrixserverlib.ClientEvent `json:"events"`
-		Limited   bool                            `json:"limited"`
-		PrevBatch *TopologyToken                  `json:"prev_batch,omitempty"`
-	} `json:"timeline"`
-	Ephemeral struct {
-		Events []gomatrixserverlib.ClientEvent `json:"events"`
-	} `json:"ephemeral"`
-	AccountData struct {
-		Events []gomatrixserverlib.ClientEvent `json:"events"`
-	} `json:"account_data"`
+	Summary              *Summary      `json:"summary,omitempty"`
+	State                *ClientEvents `json:"state,omitempty"`
+	Timeline             *Timeline     `json:"timeline,omitempty"`
+	Ephemeral            *ClientEvents `json:"ephemeral,omitempty"`
+	AccountData          *ClientEvents `json:"account_data,omitempty"`
 	*UnreadNotifications `json:"unread_notifications,omitempty"`
 }
 
+func (jr JoinResponse) MarshalJSON() ([]byte, error) {
+	type alias JoinResponse
+	a := alias(jr)
+	if jr.State != nil && len(jr.State.Events) == 0 {
+		a.State = nil
+	}
+	if jr.Ephemeral != nil && len(jr.Ephemeral.Events) == 0 {
+		a.Ephemeral = nil
+	}
+	if jr.AccountData != nil && len(jr.AccountData.Events) == 0 {
+		a.AccountData = nil
+	}
+	if jr.Timeline != nil && len(jr.Timeline.Events) == 0 {
+		a.Timeline = nil
+	}
+	if jr.Summary != nil {
+		var nilPtr int
+		joinedEmpty := jr.Summary.JoinedMemberCount == nil || jr.Summary.JoinedMemberCount == &nilPtr
+		invitedEmpty := jr.Summary.InvitedMemberCount == nil || jr.Summary.InvitedMemberCount == &nilPtr
+		if joinedEmpty && invitedEmpty && len(jr.Summary.Heroes) == 0 {
+			a.Summary = nil
+		}
+
+	}
+	if jr.UnreadNotifications != nil &&
+		jr.UnreadNotifications.NotificationCount == 0 && jr.UnreadNotifications.HighlightCount == 0 {
+		a.UnreadNotifications = nil
+	}
+	return json.Marshal(a)
+}
+
 // NewJoinResponse creates an empty response with initialised arrays.
 func NewJoinResponse() *JoinResponse {
-	res := JoinResponse{}
-	res.State.Events = []gomatrixserverlib.ClientEvent{}
-	res.Timeline.Events = []gomatrixserverlib.ClientEvent{}
-	res.Ephemeral.Events = []gomatrixserverlib.ClientEvent{}
-	res.AccountData.Events = []gomatrixserverlib.ClientEvent{}
-	return &res
+	return &JoinResponse{
+		Summary:             &Summary{},
+		State:               &ClientEvents{},
+		Timeline:            &Timeline{},
+		Ephemeral:           &ClientEvents{},
+		AccountData:         &ClientEvents{},
+		UnreadNotifications: &UnreadNotifications{},
+	}
 }
 
 // InviteResponse represents a /sync response for a room which is under the 'invite' key.
@@ -469,21 +535,28 @@ func NewInviteResponse(event *gomatrixserverlib.HeaderedEvent) *InviteResponse {
 
 // LeaveResponse represents a /sync response for a room which is under the 'leave' key.
 type LeaveResponse struct {
-	State struct {
-		Events []gomatrixserverlib.ClientEvent `json:"events"`
-	} `json:"state"`
-	Timeline struct {
-		Events    []gomatrixserverlib.ClientEvent `json:"events"`
-		Limited   bool                            `json:"limited"`
-		PrevBatch *TopologyToken                  `json:"prev_batch,omitempty"`
-	} `json:"timeline"`
+	State    *ClientEvents `json:"state,omitempty"`
+	Timeline *Timeline     `json:"timeline,omitempty"`
+}
+
+func (lr LeaveResponse) MarshalJSON() ([]byte, error) {
+	type alias LeaveResponse
+	a := alias(lr)
+	if lr.State != nil && len(lr.State.Events) == 0 {
+		a.State = nil
+	}
+	if lr.Timeline != nil && len(lr.Timeline.Events) == 0 {
+		a.Timeline = nil
+	}
+	return json.Marshal(a)
 }
 
 // NewLeaveResponse creates an empty response with initialised arrays.
 func NewLeaveResponse() *LeaveResponse {
-	res := LeaveResponse{}
-	res.State.Events = []gomatrixserverlib.ClientEvent{}
-	res.Timeline.Events = []gomatrixserverlib.ClientEvent{}
+	res := LeaveResponse{
+		State:    &ClientEvents{},
+		Timeline: &Timeline{},
+	}
 	return &res
 }