1
0
mirror of https://github.com/matrix-org/matrix-js-sdk.git synced 2025-11-26 17:03:12 +03:00

Avoid paginating forever in private rooms

In TimelineWindow.paginate, keep a count of the number of API requests we have
made, and bail out if it gets too high, to ensure that we don't get stuck in a
loop of paginating right back to the start of the room.
This commit is contained in:
Richard van der Hoff
2016-04-07 14:16:02 +01:00
parent 989e7cf78b
commit 7573171d05
2 changed files with 63 additions and 3 deletions

View File

@@ -30,6 +30,13 @@ var DEBUG = false;
*/
var debuglog = DEBUG ? console.log.bind(console) : function() {};
/**
* the number of times we ask the server for more events before giving up
*
* @private
*/
var DEFAULT_PAGINATE_LOOP_LIMIT = 5;
/**
* Construct a TimelineWindow.
*
@@ -179,10 +186,14 @@ TimelineWindow.prototype.canPaginate = function(direction) {
* even if there are fewer than 'size' of them, as we will just return those
* we already know about.)
*
* @param {number} [requestLimit=5] limit for the number of API requests we should
* make.
*
* @return {module:client.Promise} Resolves to a boolean which is true if more events
* were successfully retrieved.
*/
TimelineWindow.prototype.paginate = function(direction, size, makeRequest) {
TimelineWindow.prototype.paginate = function(direction, size, makeRequest,
requestLimit) {
// Either wind back the message cap (if there are enough events in the
// timeline to do so), or fire off a pagination request.
@@ -190,6 +201,10 @@ TimelineWindow.prototype.paginate = function(direction, size, makeRequest) {
makeRequest = true;
}
if (requestLimit === undefined) {
requestLimit = DEFAULT_PAGINATE_LOOP_LIMIT;
}
var tl;
if (direction == EventTimeline.BACKWARDS) {
tl = this._start;
@@ -224,7 +239,9 @@ TimelineWindow.prototype.paginate = function(direction, size, makeRequest) {
return q(true);
}
if (!makeRequest) {
if (!makeRequest || requestLimit === 0) {
// todo: should we return something different to indicate that there
// might be more envets out there, but we haven't found them yet?
return q(false);
}
@@ -256,7 +273,12 @@ TimelineWindow.prototype.paginate = function(direction, size, makeRequest) {
// token. In particular, we want to know if we've actually hit the
// start of the timeline, or if we just happened to know about all of
// the events thanks to https://matrix.org/jira/browse/SYN-645.
return self.paginate(direction, size, true);
//
// On the other hand, we necessarily want to wait forever for the
// server to make its mind up about whether there are other events,
// because it gives a bad user experience
// (https://github.com/vector-im/vector-web/issues/1204).
return self.paginate(direction, size, true, requestLimit - 1);
});
tl.pendingPaginate = prom;
return prom;

View File

@@ -418,5 +418,43 @@ describe("TimelineWindow", function() {
}).catch(utils.failTest).done(done);
});
it("should limit the number of unsuccessful pagination requests",
function(done) {
var timeline = createTimeline();
timeline.setPaginationToken("toktok", EventTimeline.FORWARDS);
var timelineWindow = createWindow(timeline, {windowLimit: 5});
var eventId = timeline.getEvents()[1].getId();
var paginateCount = 0;
client.paginateEventTimeline = function(timeline0, opts) {
expect(timeline0).toBe(timeline);
expect(opts.backwards).toBe(false);
expect(opts.limit).toEqual(2);
paginateCount += 1
return q(true);
};
timelineWindow.load(eventId, 3).then(function() {
var expectedEvents = timeline.getEvents();
expect(timelineWindow.getEvents()).toEqual(expectedEvents);
expect(timelineWindow.canPaginate(EventTimeline.BACKWARDS))
.toBe(false);
expect(timelineWindow.canPaginate(EventTimeline.FORWARDS))
.toBe(true);
return timelineWindow.paginate(EventTimeline.FORWARDS, 2, true, 3);
}).then(function(success) {
expect(success).toBe(false);
expect(paginateCount).toEqual(3);
var expectedEvents = timeline.getEvents().slice(0, 3);
expect(timelineWindow.getEvents()).toEqual(expectedEvents);
expect(timelineWindow.canPaginate(EventTimeline.BACKWARDS))
.toBe(false);
expect(timelineWindow.canPaginate(EventTimeline.FORWARDS))
.toBe(true);
}).catch(utils.failTest).done(done);
});
});
});