From 15d0df5d3bd02068e09ba2dfca4e6f6d20b3d804 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Fri, 8 Apr 2016 14:58:24 +0100 Subject: [PATCH] Stop trying to paginate after we get a failure Once TimelineWindow.paginate fails to return any results, set can*Paginate=false, and use it as a flag to stop us trying to paginate any further. --- src/components/structures/TimelinePanel.js | 59 +++++++-- .../structures/TimelinePanel-test.js | 124 ++++++++++++++++++ 2 files changed, 175 insertions(+), 8 deletions(-) create mode 100644 test/components/structures/TimelinePanel-test.js diff --git a/src/components/structures/TimelinePanel.js b/src/components/structures/TimelinePanel.js index 12931fed37..84bbe1a67e 100644 --- a/src/components/structures/TimelinePanel.js +++ b/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)) { - debuglog("TimelinePanel: can't paginate at this time; backwards:"+backwards); + var canPaginateKey = backwards ? 'canBackPaginate' : 'canForwardPaginate'; + 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({[statekey]: true}); + this.setState({[paginatingKey]: 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), }); }, diff --git a/test/components/structures/TimelinePanel-test.js b/test/components/structures/TimelinePanel-test.js new file mode 100644 index 0000000000..b6970cb430 --- /dev/null +++ b/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( + , + 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); + }); +}); + +