Skip to content
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

Changing the required state but not the timeline limit re-sends the timeline #354

Open
DMRobertson opened this issue Oct 23, 2023 · 1 comment

Comments

@DMRobertson
Copy link
Contributor

In this situation I'd expect to see a new required_state blob, but only new entries in the timeline.

Test case, from #329:

func TestTimelineAfterRequestingStateAfterGappyPoll(t *testing.T) {
	pqString := testutils.PrepareDBConnectionString()
	v2 := runTestV2Server(t)
	defer v2.close()
	v3 := runTestServer(t, v2, pqString)
	defer v3.close()

	alice := "alice"
	aliceToken := "alicetoken"
	bob := "bob"
	roomID := "!unimportant"

	v2.addAccount(t, alice, aliceToken)

	t.Log("alice creates a public room.")
	timeline1 := createRoomState(t, alice, time.Now())
	v2.queueResponse(aliceToken, sync2.SyncResponse{
		Rooms: sync2.SyncRoomsResponse{
			Join: map[string]sync2.SyncV2JoinResponse{
				roomID: {
					Timeline: sync2.TimelineResponse{
						Events:    timeline1,
						PrevBatch: "alicePublicPrevBatch1",
					},
				},
			},
		},
		NextBatch: "aliceSync1",
	})

	t.Log("alice sliding syncs.")
	aliceRes := v3.mustDoV3Request(t, aliceToken, sync3.Request{
		RoomSubscriptions: map[string]sync3.RoomSubscription{
			roomID: {TimelineLimit: 20},
		},
	})

	t.Log("She sees herself joined to her room, with an appropriate timeline.")
	// Note: we only expect timeline1[1:] here, excluding the create event. See
	// https://github.com/matrix-org/sliding-sync/issues/343
	m.MatchResponse(t, aliceRes,
		m.LogResponse(t),
		m.MatchRoomSubscription(roomID, m.MatchRoomTimeline(timeline1[1:])),
	)

	t.Logf("Alice's poller gets a gappy sync response for the public room. bob's membership is now join, and alice has sent 10 messages.")
	timeline2 := make([]json.RawMessage, 10)
	for i := range timeline2 {
		timeline2[i] = testutils.NewMessageEvent(t, alice, fmt.Sprintf("hello %d", i))
	}

	newMembership := testutils.NewJoinEvent(t, bob)

	v2.queueResponse(aliceToken, sync2.SyncResponse{
		NextBatch: "alice2",
		Rooms: sync2.SyncRoomsResponse{
			Join: map[string]sync2.SyncV2JoinResponse{
				roomID: {
					State: sync2.EventsResponse{
						Events: []json.RawMessage{newMembership},
					},
					Timeline: sync2.TimelineResponse{
						Events:    timeline2,
						Limited:   true,
						PrevBatch: "alicePublicPrevBatch2",
					},
				},
			},
		},
	})
	v2.waitUntilEmpty(t, aliceToken)

	t.Log("alice syncs, requesting Bob's membership. She sees it, and her new timeline.")
	aliceRes = v3.mustDoV3RequestWithPos(t, aliceToken, aliceRes.Pos, sync3.Request{
		RoomSubscriptions: map[string]sync3.RoomSubscription{
			roomID: {
				TimelineLimit: 20,
				RequiredState: [][2]string{{"m.room.member", bob}},
			},
		},
	})
	m.MatchResponse(t, aliceRes, m.MatchRoomSubscription(roomID,
		m.MatchRoomTimeline(timeline2),
	))
}

Notes on how to fix:

  • Move the timeline limit changed method onto room subscription for tidying up.
  • Core problem: RoomDelta.Subs is a []string (roomIDs). Would need to make this a struct that describes how the subscription has changed.

Related: if I shrink the timeline limit, I would only expect to see new entries in the timeline. If I grow the timeline limit... not sure, maybe it's fine to resend the timeline in this situation.

@DMRobertson
Copy link
Contributor Author

Recording for posterity, but we're not going to prioritise fixing this.

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

No branches or pull requests

1 participant