Merge pull request #265 from matrix-org/rav/limit_pagination

Stop trying to paginate after we get a failure

src/components/structures/TimelinePanel.js

@@ -94,7 +94,33 @@ var TimelinePanel = React.createClass({
         return {
             events: [],
             timelineLoading: true, // track whether our room timeline is loading
-            canBackPaginate: true,
+
+            // canBackPaginate == false may mean:
+            //
+            // * we haven't (successfully) loaded the timeline yet, or:
+            //
+            // * we have got to the point where the room was created, or:
+            //
+            // * the server indicated that there were no more visible events
+            //  (normally implying we got to the start of the room), or:
+            //
+            // * we gave up asking the server for more events
+            canBackPaginate: false,
+
+            // canForwardPaginate == false may mean:
+            //
+            // * we haven't (successfully) loaded the timeline yet
+            //
+            // * we have got to the end of time and are now tracking the live
+            //   timeline, or:
+            //
+            // * the server indicated that there were no more visible events
+            //   (not sure if this ever happens when we're not at the live
+            //   timeline), or:
+            //
+            // * we are looking at some historical point, but gave up asking
+            //   the server for more events
+            canForwardPaginate: false,
 
             // start with the read-marker visible, so that we see its animated
             // disappearance when swtitching into the room.
@@ -172,19 +198,31 @@ var TimelinePanel = React.createClass({
     // set off a pagination request.
     onMessageListFillRequest: function(backwards) {
         var dir = backwards ? EventTimeline.BACKWARDS : EventTimeline.FORWARDS;
-        if(!this._timelineWindow.canPaginate(dir)) {
+        var canPaginateKey = backwards ? 'canBackPaginate' : 'canForwardPaginate';
-            debuglog("TimelinePanel: can't paginate at this time; backwards:"+backwards);
+        var paginatingKey = backwards ? 'backPaginating' : 'forwardPaginating';
+
+        if (!this.state[canPaginateKey]) {
+            debuglog("TimelinePanel: have given up", dir, "paginating this timeline");
             return q(false);
         }
+
+        if(!this._timelineWindow.canPaginate(dir)) {
+            debuglog("TimelinePanel: can't", dir, "paginate any further");
+            this.setState({[canPaginateKey]: false});
+            return q(false);
+        }
+
         debuglog("TimelinePanel: Initiating paginate; backwards:"+backwards);
-        var statekey = backwards ? 'backPaginating' : 'forwardPaginating';
+        this.setState({[paginatingKey]: true});
-        this.setState({[statekey]: true});
 
         return this._timelineWindow.paginate(dir, PAGINATE_SIZE).then((r) => {
             if (this.unmounted) { return; }
 
             debuglog("TimelinePanel: paginate complete backwards:"+backwards+"; success:"+r);
-            this.setState({[statekey]: false});
+            this.setState({
+                [paginatingKey]: false,
+                [canPaginateKey]: r,
+            });
             this._reloadEvents();
             return r;
         });
@@ -581,7 +619,11 @@ var TimelinePanel = React.createClass({
             // We need to skip over any which have subsequently been sent.
             this._advanceReadMarkerPastMyEvents();
 
-            this.setState({timelineLoading: false}, () => {
+            this.setState({
+                canBackPaginate: this._timelineWindow.canPaginate(EventTimeline.BACKWARDS),
+                canForwardPaginate: this._timelineWindow.canPaginate(EventTimeline.FORWARDS),
+                timelineLoading: false,
+            }, () => {
                 // initialise the scroll state of the message panel
                 if (!this.refs.messagePanel) {
                     // this shouldn't happen - we know we're mounted because
@@ -652,6 +694,8 @@ var TimelinePanel = React.createClass({
         } else {
             this.setState({
                 events: [],
+                canBackPaginate: false,
+                canForwardPaginate: false,
                 timelineLoading: true,
             });
 
@@ -678,7 +722,6 @@ var TimelinePanel = React.createClass({
 
         this.setState({
             events: events,
-            canBackPaginate: this._timelineWindow.canPaginate(EventTimeline.BACKWARDS),
         });
     },
 

test/components/structures/TimelinePanel-test.js

@@ -0,0 +1,124 @@
+/*
+Copyright 2016 OpenMarket Ltd
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+var React = require('react');
+var ReactDOM = require('react-dom');
+var ReactTestUtils = require('react-addons-test-utils');
+var expect = require('expect');
+var q = require('q');
+var sinon = require('sinon');
+
+var jssdk = require('matrix-js-sdk');
+var EventTimeline = jssdk.EventTimeline;
+
+var sdk = require('matrix-react-sdk');
+var TimelinePanel = sdk.getComponent('structures.TimelinePanel');
+var peg = require('../../../src/MatrixClientPeg');
+
+var test_utils = require('test-utils');
+
+var ROOM_ID = '!room:localhost';
+var USER_ID = '@me:localhost';
+
+describe('TimelinePanel', function() {
+    var sandbox;
+    var room;
+    var client;
+    var timeline;
+    var parentDiv;
+
+    beforeEach(function() {
+        test_utils.beforeEach(this);
+        sandbox = test_utils.stubClient(sandbox);
+
+        timeline = new jssdk.EventTimeline(ROOM_ID);
+        room = sinon.createStubInstance(jssdk.Room);
+        room.getLiveTimeline.returns(timeline);
+        room.getPendingEvents.returns([]);
+
+        client = peg.get();
+        client.credentials = {userId: USER_ID};
+
+        // create a div of a useful size to put our panel in, and attach it to
+        // the document so that we can interact with it properly.
+        parentDiv = document.createElement('div');
+        parentDiv.style.width = '800px';
+        parentDiv.style.height = '600px';
+        parentDiv.style.overflow = 'hidden';
+        document.body.appendChild(parentDiv);
+    });
+
+    afterEach(function() {
+        if (parentDiv) {
+            document.body.removeChild(parentDiv);
+            parentDiv = null;
+        }
+        sandbox.restore();
+    });
+
+    it('should not paginate forever if there are no events', function(done) {
+        // start with a handful of events in the timeline, as would happen when
+        // joining a room
+        var d = Date.now();
+        for (var i = 0; i < 3; i++) {
+            timeline.addEvent(test_utils.mkMessage(
+                {
+                    event: true, room: ROOM_ID, user: USER_ID,
+                    ts: d+i,
+                }
+            ));
+        }
+        timeline.setPaginationToken('tok', EventTimeline.BACKWARDS);
+
+        // back-pagination returns a promise for true, but adds no events
+        client.paginateEventTimeline = sinon.spy((tl, opts) => {
+            console.log("paginate:", opts);
+            expect(opts.backwards).toBe(true);
+            return q(true);
+        });
+
+        var panel = ReactDOM.render(
+            <TimelinePanel room={room}/>,
+            parentDiv
+        );
+
+        var messagePanel = ReactTestUtils.findRenderedComponentWithType(
+            panel, sdk.getComponent('structures.MessagePanel'));
+
+        expect(messagePanel.props.backPaginating).toBe(true);
+
+        // let the first round of pagination finish off
+        setTimeout(() => {
+            // at this point, the timeline window should have tried to paginate
+            // 5 times, and we should have given up paginating
+            expect(client.paginateEventTimeline.callCount).toEqual(5);
+            expect(messagePanel.props.backPaginating).toBe(false);
+            expect(messagePanel.props.suppressFirstDateSeparator).toBe(false);
+
+            // now, if we update the events, there shouldn't be any
+            // more requests.
+            client.paginateEventTimeline.reset();
+            panel.forceUpdate();
+            expect(messagePanel.props.backPaginating).toBe(false);
+            setTimeout(() => {
+                expect(client.paginateEventTimeline.callCount).toEqual(0);
+                done();
+            }, 0);
+        }, 0);
+    });
+});
+
+