Load discussion and posts with one request

Speeds things up a heap.
Also fix a whole bunch of bugs with the post stream.
This commit is contained in:
Toby Zerner 2015-02-06 10:30:38 +10:30
parent b03740f363
commit a318bb4952
13 changed files with 279 additions and 164 deletions

View File

@ -24,6 +24,11 @@ export default Ember.Component.extend({
return this.get('loaded') && ! this.get('paused');
}.property('loaded', 'paused'),
refresh: function() {
this.set('paused', true);
clearTimeout(this.updateStateTimeout);
}.observes('stream'),
didInsertElement: function() {
$(window).on('scroll', {view: this}, this.windowWasScrolled);
},
@ -105,41 +110,41 @@ export default Ember.Component.extend({
}, 250);
}.observes('active'),
loadingNumber: function(number) {
loadingNumber: function(number, noAnimation) {
// The post with this number is being loaded. We want to scroll to where
// we think it will appear. We may be scrolling to the edge of the page,
// but we don't want to trigger any terminal post gaps to load by doing
// that. So, we'll disable the window's scroll handler for now.
this.set('paused', true);
this.jumpToNumber(number);
this.jumpToNumber(number, noAnimation);
},
loadedNumber: function(number) {
loadedNumber: function(number, noAnimation) {
// The post with this number has been loaded. After we scroll to this
// post, we want to resume scroll events.
var view = this;
Ember.run.scheduleOnce('afterRender', function() {
view.jumpToNumber(number).done(function() {
view.jumpToNumber(number, noAnimation).done(function() {
view.set('paused', false);
});
});
},
loadingIndex: function(index) {
loadingIndex: function(index, noAnimation) {
// The post at this index is being loaded. We want to scroll to where we
// think it will appear. We may be scrolling to the edge of the page,
// but we don't want to trigger any terminal post gaps to load by doing
// that. So, we'll disable the window's scroll handler for now.
this.set('paused', true);
this.jumpToIndex(index);
this.jumpToIndex(index, noAnimation);
},
loadedIndex: function(index) {
loadedIndex: function(index, noAnimation) {
// The post at this index has been loaded. After we scroll to this post,
// we want to resume scroll events.
var view = this;
Ember.run.scheduleOnce('afterRender', function() {
view.jumpToIndex(index).done(function() {
view.jumpToIndex(index, noAnimation).done(function() {
view.set('paused', false);
});
});
@ -147,7 +152,7 @@ export default Ember.Component.extend({
// Scroll down to a certain post by number (or the gap which we think the
// post is in) and highlight it.
jumpToNumber: function(number) {
jumpToNumber: function(number, noAnimation) {
// Clear the highlight class from all posts, and attempt to find and
// highlight a post with the specified number. However, we don't apply
// the highlight to the first post in the stream because it's pretty
@ -165,22 +170,26 @@ export default Ember.Component.extend({
$item = this.findNearestToNumber(number);
}
return this.scrollToItem($item);
return this.scrollToItem($item, noAnimation);
},
// Scroll down to a certain post by index (or the gap the post is in.)
jumpToIndex: function(index) {
jumpToIndex: function(index, noAnimation) {
var $item = this.findNearestToIndex(index);
return this.scrollToItem($item);
return this.scrollToItem($item, noAnimation);
},
scrollToItem: function($item) {
scrollToItem: function($item, noAnimation) {
var $container = $('html, body');
if ($item.length) {
var marginTop = this.getMarginTop();
var scrollTop = $item.is(':first-child') ? 0 : $item.offset().top - marginTop;
if (scrollTop !== $(document).scrollTop()) {
$container.stop(true).animate({scrollTop: scrollTop});
if (noAnimation) {
$container.stop(true).scrollTop(scrollTop);
} else {
$container.stop(true).animate({scrollTop: scrollTop});
}
}
}
return $container.promise();
@ -221,35 +230,35 @@ export default Ember.Component.extend({
},
actions: {
goToNumber: function(number) {
goToNumber: function(number, noAnimation) {
number = Math.max(number, 1);
// Let's start by telling our listeners that we're going to load
// posts near this number. Elsewhere we will listen and
// consequently scroll down to the appropriate position.
this.trigger('loadingNumber', number);
this.trigger('loadingNumber', number, noAnimation);
// Now we have to actually make sure the posts around this new start
// position are loaded. We will tell our listeners when they are.
// Again, a listener will scroll down to the appropriate post.
var controller = this;
this.get('stream').loadNearNumber(number).then(function() {
controller.trigger('loadedNumber', number);
controller.trigger('loadedNumber', number, noAnimation);
});
},
goToIndex: function(index, backwards) {
goToIndex: function(index, backwards, noAnimation) {
// Let's start by telling our listeners that we're going to load
// posts at this index. Elsewhere we will listen and consequently
// scroll down to the appropriate position.
this.trigger('loadingIndex', index);
this.trigger('loadingIndex', index, noAnimation);
// Now we have to actually make sure the posts around this index
// are loaded. We will tell our listeners when they are. Again, a
// listener will scroll down to the appropriate post.
var controller = this;
this.get('stream').loadNearIndex(index, backwards).then(function() {
controller.trigger('loadedIndex', index);
controller.trigger('loadedIndex', index, noAnimation);
});
},
@ -276,4 +285,4 @@ export default Ember.Component.extend({
this.get('stream').loadRange(start, end, backwards);
}
}
});
});

View File

@ -218,10 +218,13 @@ export default Ember.Component.extend({
// after a bunch of posts have been loaded), then we want to update the
// scrubber scrollbar according to the window's current scroll position.
resume: function() {
if (this.get('streamContent.active')) {
this.update();
this.updateScrollbar(true);
}
var scrubber = this;
Ember.run.scheduleOnce('afterRender', function() {
if (scrubber.get('streamContent.active')) {
scrubber.update();
scrubber.updateScrollbar(true);
}
});
}.observes('streamContent.active'),
// Update the index/visible/description properties according to the

View File

@ -1,12 +1,10 @@
import Ember from 'ember';
import PostStream from '../models/post-stream';
import ComposerReply from '../components/discussions/composer-reply';
import ActionButton from '../components/ui/controls/action-button';
import AlertMessage from '../components/alert-message';
export default Ember.ObjectController.extend(Ember.Evented, {
needs: ['application', 'composer'],
queryParams: ['start'],
@ -16,31 +14,6 @@ export default Ember.ObjectController.extend(Ember.Evented, {
loaded: false,
stream: null,
setup: function(discussion) {
this.set('model', discussion);
// Set up the post stream object. It needs to know about the discussion
// its representing the posts for, and we also need to inject the Ember
// data store.
var stream = PostStream.create();
stream.set('discussion', discussion);
stream.set('store', this.get('store'));
this.set('stream', stream);
// Next, we need to load a list of the discussion's post IDs into the
// post stream object. If we don't already have this information, we'll
// need to reload the discussion model.
var promise = discussion.get('posts') ? Ember.RSVP.resolve(discussion) : discussion.reload();
// When we know we have the post IDs, we can set up the post stream with
// them. Then the view will trigger the stream to load as it sees fit.
var controller = this;
promise.then(function(discussion) {
stream.setup(discussion.get('postIds'));
controller.set('loaded', true);
});
},
// Save a reply. This may be called by a composer-reply component that was
// set up on a different discussion, so we require a discussion model to
// be explicitly passed rather than using the controller's implicit one.
@ -61,7 +34,6 @@ export default Ember.ObjectController.extend(Ember.Evented, {
composer.send('hide');
discussion.setProperties({
posts: discussion.get('posts')+','+post.get('id'),
lastTime: post.get('time'),
lastUser: post.get('user'),
lastPost: post,
@ -74,8 +46,7 @@ export default Ember.ObjectController.extend(Ember.Evented, {
// If we're currently viewing the discussion which this reply was
// made in, then we can add the post to the end of the post
// stream.
if (discussion == controller.get('model')) {
stream.set('ids', discussion.get('postIds'));
if (discussion == controller.get('model') && stream) {
stream.addPostToEnd(post);
} else {
// Otherwise, we'll create an alert message to inform the user

View File

@ -0,0 +1,25 @@
import DS from 'ember-data';
// This can be removed when
// https://github.com/emberjs/data/pull/2584 is implemented.
export default {
name: 'find-query-one',
initialize: function(container) {
DS.Store.reopen({
findQueryOne: function(type, id, query) {
var store = this;
var typeClass = store.modelFor(type);
var adapter = store.adapterFor(typeClass);
var serializer = store.serializerFor(typeClass);
var url = adapter.buildURL(type, id);
var ajaxPromise = adapter.ajax(url, 'GET', { data: query });
return ajaxPromise.then(function(rawPayload) {
var extractedPayload = serializer.extract(store, typeClass, rawPayload, id, 'find');
return store.push(typeClass, extractedPayload);
});
}
});
}
};

View File

@ -15,15 +15,6 @@ export default Ember.ArrayProxy.extend(Ember.Evented, {
postLoadCount: 20,
_init: function() {
this.clear();
}.on('init'),
setup: function(ids) {
this.set('ids', ids);
this.get('content').objectAt(0).set('indexEnd', this.get('count') - 1);
},
count: Ember.computed.alias('ids.length'),
loadedCount: function() {
@ -40,12 +31,35 @@ export default Ember.ArrayProxy.extend(Ember.Evented, {
return last && last.content;
}.property('content.@each'),
init: function() {
this._super();
this.set('ids', Ember.A());
this.clear();
},
setup: function(ids) {
// Set our ids to the array provided and reset the content of the
// stream to a big gap that covers the amount of posts we now have.
this.set('ids', ids);
this.clear();
// Look in the store and see if we already have the data for any of
// these posts. If we do, we can add them to the stream.
var posts = [];
var store = this.get('store');
ids.forEach(function(id) {
if (store.hasRecordForId('post', id)) {
posts.pushObject(store.getById('post', id));
}
});
this.addPosts(posts);
},
// Clear the contents of the post stream, resetting it to one big gap.
clear: function() {
var content = Ember.A();
content.clear().pushObject(this.makeItem(0, this.get('count') - 1).set('loading', true));
this.set('content', content);
this.set('ids', Ember.A());
},
loadRange: function(start, end, backwards) {
@ -124,9 +138,12 @@ export default Ember.ArrayProxy.extend(Ember.Evented, {
this.trigger('postsLoaded', posts);
var stream = this;
var content = this.get('content');
content.beginPropertyChanges();
posts.forEach(function(post) {
stream.addPost(post);
});
content.endPropertyChanges();
this.trigger('postsAdded');
},
@ -156,6 +173,7 @@ export default Ember.ArrayProxy.extend(Ember.Evented, {
},
addPostToEnd: function(post) {
this.get('ids').pushObject(post.get('id'));
var index = this.get('count') - 1;
this.get('content').pushObject(this.makeItem(index, index, post));
},
@ -171,10 +189,10 @@ export default Ember.ArrayProxy.extend(Ember.Evented, {
findNearestTo: function(index, property) {
var nearestItem;
this.get('content').some(function(item) {
nearestItem = item;
if (item.get(property) > index) {
return true;
}
nearestItem = item;
});
return nearestItem;
},
@ -184,6 +202,6 @@ export default Ember.ArrayProxy.extend(Ember.Evented, {
},
findNearestToIndex: function(index) {
return this.findNearestTo(index, 'indexEnd');
return this.findNearestTo(index, 'indexStart');
}
});

View File

@ -1,13 +1,22 @@
import Ember from 'ember';
export default Ember.Route.extend({
import PostStream from '../models/post-stream';
export default Ember.Route.extend({
queryParams: {
start: {replace: true}
},
model: function(params) {
return this.store.find('discussion', params.id);
// Each time we view a discussion we want to reload its posts from
// scratch so that we have the most up-to-date data. Also, if we were
// to leave them in the store, the stream would try and render them
// which has the potential to be slow.
this.store.unloadAll('post');
return this.store.findQueryOne('discussion', params.id, {
include: 'posts',
near: params.start
});
},
resetController: function(controller) {
@ -19,18 +28,42 @@ export default Ember.Route.extend({
controller.set('stream', null);
},
setupController: function(controller, model) {
controller.setup(model);
setupController: function(controller, discussion) {
controller.set('model', discussion);
// Tell the discussions controller that the discussions list should be
// displayed as a pane, hidden on the side of the screen. Also set the
// application back button's target as the discussions controller.
this.controllerFor('index').set('paned', true);
this.controllerFor('application').set('backButtonTarget', this.controllerFor('index'));
// Set up the post stream object. It needs to know about the discussion
// it's representing the posts for, and we also need to inject the Ember
// Data store.
var stream = PostStream.create({
discussion: discussion,
store: this.store
});
controller.set('stream', stream);
// Next, we need to make sure we have a list of the discussion's post
// IDs. If we don't already have this information, we'll need to
// reload the discussion model.
var promise = discussion.get('posts') ? Ember.RSVP.resolve(discussion) : this.model({
id: discussion.get('id'),
start: controller.get('start')
});
// When we know we have the post IDs, we can set up the post stream with
// them. Then we will tell the view that we have finished loading so that
// it can scroll down to the appropriate post.
promise.then(function(discussion) {
stream.setup(discussion.get('postIds'));
controller.store.push('discussion', {id: discussion.get('id'), posts: ''});
if (controller.get('model') === discussion) {
controller.set('loaded', true);
Ember.run.scheduleOnce('afterRender', function() {
controller.trigger('loaded');
});
}
});
},
actions: {
queryParamsDidChange: function(params) {
// If the ?start param has changed, we want to tell the view to
// tell the streamContent component to jump to this start point.
@ -39,26 +72,25 @@ export default Ember.Route.extend({
// queryParamsDidChange is fired before the controller is reset.
// Thus, controller.loaded would still be true and the
// startWasChanged event would be triggered inappropriately.
var controller = this.get('controller'),
oldStart = parseInt(this.get('controller.start'));
var newStart = parseInt(params.start) || 1;
var controller = this.controllerFor('discussion');
var oldStart = parseInt(controller.get('start'));
Ember.run.next(function() {
if (! params.start || ! controller || ! controller.get('loaded') || parseInt(params.start) === oldStart) {
return;
if (controller.get('loaded') && newStart !== oldStart) {
controller.trigger('startWasChanged', newStart);
}
controller.trigger('startWasChanged', params.start);
});
},
willTransition: function() {
didTransition: function() {
// When we transition into a new discussion, we want to hide the
// discussions list pane. This means that when the user selects a
// different discussion within the pane, the pane will slide away.
this.controllerFor('index').set('paneShowing', false);
// We also minimize the composer.
this.controllerFor('index')
.set('paned', true)
.set('paneShowing', false);
this.controllerFor('composer').send('minimize');
},
didTransition: function() {
this.controllerFor('composer').send('minimize');
}
}
});

View File

@ -11,58 +11,37 @@ export default Ember.View.extend(Ember.Evented, {
sidebarItems: null,
loadingNumber: false,
didInsertElement: function() {
// Create and populate an array of items to be rendered in the sidebar.
var sidebarItems = TaggedArray.create();
this.trigger('populateSidebar', sidebarItems);
this.set('sidebarItems', sidebarItems);
// By this stage the discussion controller has initialized the post
// stream object, and there may or may not be posts loaded into it.
// Either way, we want to tell our stream content component to jump
// down to the start position specified in the controller's query
// params.
this.loadStreamContentForNewDiscussion();
// For that matter, whenever the controller's start query param
// changes, we want to tell our stream content component to jump down
// to it.
this.get('controller').on('startWasChanged', this, this.goToNumber);
this.get('streamContent').on('loadedNumber', this, this.loadedNumber);
this.get('controller').on('loaded', this, this.loaded);
this.get('controller').on('startWasChanged', this, this.startWasChanged);
},
willDestroyElement: function() {
this.get('controller').off('startWasChanged', this, this.goToNumber);
this.get('streamContent').off('loadedNumber', this, this.loadedNumber);
this.get('controller').off('loaded', this, this.loaded);
this.get('controller').off('startWasChanged', this, this.startWasChanged);
},
goToNumber: function(start) {
// We can only proceed if the controller has loaded the discussion
// details and the view has been rendered.
if (this.get('controller.loaded') && this.get('streamContent') && ! this.get('loadingNumber')) {
this.get('streamContent').send('goToNumber', start);
this.set('loadingNumber', true);
}
// When the controller has finished loading, we want to scroll down to the
// appropriate post instantly (without animation).
loaded: function() {
this.get('streamContent').send('goToNumber', this.get('controller.start'), true);
},
loadedNumber: function() {
this.set('loadingNumber', false);
// When the start position of the discussion changes, we want to scroll
// down to the appropriate post.
startWasChanged: function(start) {
this.get('streamContent').send('goToNumber', start);
},
// ------------------------------------------------------------------------
// OBSERVERS
// ------------------------------------------------------------------------
// Whenever the controller has switched out the old discussion model for a
// new one, we want to begin loading posts according to the ?start param.
loadStreamContentForNewDiscussion: function() {
if (this.get('controller.loaded')) {
this.goToNumber(this.get('controller.start'));
}
}.observes('controller.loaded'),
// Whenever the model's title changes, we want to update that document's
// title the reflect the new title.
updateTitle: function() {

View File

@ -26,7 +26,7 @@ abstract class Base extends Controller
abstract protected function run();
public function handle($request, $parameters)
public function handle($request, $parameters = [])
{
$this->registerErrorHandlers();
@ -39,6 +39,11 @@ abstract class Base extends Controller
return $this->run();
}
public function setRequest($request)
{
$this->request = $request;
}
public function param($key, $default = null)
{
return array_get($this->parameters, $key, $default);
@ -74,7 +79,7 @@ abstract class Base extends Controller
protected function included($available)
{
$requested = explode(',', $this->input('include'));
return array_intersect($available, $requested);
return array_intersect((array) $available, $requested);
}
protected function explodeIds($ids)

View File

@ -1,11 +1,32 @@
<?php namespace Flarum\Api\Actions\Discussions;
use Flarum\Core\Discussions\Discussion;
use Flarum\Core\Posts\PostRepository;
use Flarum\Api\Actions\Base;
use Flarum\Api\Actions\Posts\GetsPostsForDiscussion;
use Flarum\Api\Serializers\DiscussionSerializer;
class Show extends Base
{
use GetsPostsForDiscussion;
/**
* The post repository.
*
* @var PostRepository
*/
protected $posts;
/**
* Instantiate the action.
*
* @param PostRepository $posts
*/
public function __construct(PostRepository $posts)
{
$this->posts = $posts;
}
/**
* Show a single discussion.
*
@ -13,10 +34,19 @@ class Show extends Base
*/
protected function run()
{
$include = $this->included(['startPost', 'lastPost']);
$include = $this->included(['startPost', 'lastPost', 'posts']);
$discussion = Discussion::whereCanView()->findOrFail($this->param('id'));
if (in_array('posts', $include)) {
$relations = ['user', 'user.groups', 'editUser', 'deleteUser'];
$discussion->posts = $this->getPostsForDiscussion($this->posts, $discussion->id, $relations);
$include = array_merge($include, array_map(function ($relation) {
return 'posts.'.$relation;
}, $relations));
}
// Set up the discussion serializer, which we will use to create the
// document's primary resource. As well as including the requested
// relations, we will specify that we want the 'posts' relation to be

View File

@ -0,0 +1,19 @@
<?php namespace Flarum\Api\Actions\Posts;
trait GetsPostsForDiscussion
{
protected function getPostsForDiscussion($repository, $discussionId, $relations = [])
{
$sort = $this->sort(['time']);
$count = $this->count(20, 50);
if (($near = $this->input('near')) > 1) {
$start = $repository->getIndexForNumber($discussionId, $near);
$start = max(0, $start - $count / 2);
} else {
$start = 0;
}
return $repository->findByDiscussion($discussionId, $relations, $sort['by'], $sort['order'] ?: 'asc', $count, $start);
}
}

View File

@ -2,11 +2,31 @@
use Illuminate\Database\Eloquent\ModelNotFoundException;
use Flarum\Core\Posts\Post;
use Flarum\Core\Posts\PostRepository;
use Flarum\Api\Actions\Base;
use Flarum\Api\Serializers\PostSerializer;
class Index extends Base
{
use GetsPostsForDiscussion;
/**
* The post repository.
*
* @var PostRepository
*/
protected $posts;
/**
* Instantiate the action.
*
* @param PostRepository $posts
*/
public function __construct(PostRepository $posts)
{
$this->posts = $posts;
}
/**
* Show posts from a discussion, or by providing an array of IDs.
*
@ -14,47 +34,16 @@ class Index extends Base
*/
protected function run()
{
$discussionId = $this->input('discussions');
$postIds = (array) $this->input('ids');
$include = ['user', 'user.groups', 'editUser', 'deleteUser'];
$count = $this->count(20, 50);
if ($near = $this->input('near')) {
// fetch the nearest post
$post = Post::orderByRaw('ABS(CAST(number AS SIGNED) - ?)', [$near])->whereNotNull('number')->where('discussion_id', $discussionId)->take(1)->first();
$start = max(
0,
Post::whereCanView()
->where('discussion_id', $discussionId)
->where('time', '<=', $post->time)
->count() - round($count / 2)
);
} else {
$start = $this->start();
}
$include = $this->included([]);
$sort = $this->sort(['time']);
$relations = array_merge(['user', 'user.groups', 'editUser', 'deleteUser'], $include);
// @todo move to post repository
$posts = Post::with($relations)
->whereCanView();
if ($discussionId) {
$posts->where('discussion_id', $discussionId);
}
if (count($postIds)) {
$posts->whereIn('id', $postIds);
$posts = $this->posts->findMany($postIds, $include);
} else {
$discussionId = $this->input('discussions');
$posts = $this->getPostsForDiscussion($this->posts, $discussionId, $include);
}
$posts = $posts->skip($start)
->take($count)
->orderBy($sort['by'], $sort['order'] ?: 'asc')
->get();
if (! count($posts)) {
throw new ModelNotFoundException;
}
@ -62,7 +51,7 @@ class Index extends Base
// Finally, we can set up the post serializer and use it to create
// a post resource or collection, depending on how many posts were
// requested.
$serializer = new PostSerializer($relations);
$serializer = new PostSerializer($include);
$this->document->setPrimaryElement($serializer->collection($posts));
return $this->respondWithDocument();

View File

@ -58,7 +58,8 @@ class DiscussionSerializer extends DiscussionBasicSerializer
}
/**
* Get a collection containing a discussion's viewable posts.
* Get a collection containing a discussion's viewable posts. Assumes that
* the discussion model's posts attributes has been filled.
*
* @param Discussion $discussion
* @param array $relations
@ -66,7 +67,7 @@ class DiscussionSerializer extends DiscussionBasicSerializer
*/
public function includePosts(Discussion $discussion, $relations)
{
return (new PostSerializer($relations))->collection($discussion->posts()->whereCanView()->get());
return (new PostSerializer($relations))->collection($discussion->posts);
}
/**

View File

@ -18,6 +18,40 @@ class PostRepository
return $query->findOrFail($id);
}
public function getIndexForNumber($discussionId, $number)
{
return Post::whereCanView()
->where('discussion_id', $discussionId)
->where('time', '<', function ($query) use ($discussionId, $number) {
$query->select('time')
->from('posts')
->where('discussion_id', $discussionId)
->whereNotNull('number')
->orderByRaw('ABS(CAST(number AS SIGNED) - ?)', [$number])
->take(1);
})
->count();
}
public function findByDiscussion($discussionId, $relations = [], $sortBy = 'time', $sortOrder = 'asc', $count = null, $start = 0)
{
return Post::with($relations)
->whereCanView()
->where('discussion_id', $discussionId)
->skip($start)
->take($count)
->orderBy($sortBy, $sortOrder)
->get();
}
public function findMany($ids, $relations = [])
{
return Post::with($relations)
->whereCanView()
->whereIn('id', $ids)
->get();
}
public function save(Post $post)
{
$post->assertValid();