PERF: Send down gaps as the relevant posts load instead of front loading.

This commit is contained in:
Guo Xiang Tan 2018-06-26 12:47:08 +08:00
parent 0b6a2e9d1f
commit 4c3352528e
3 changed files with 99 additions and 27 deletions

View File

@ -850,6 +850,10 @@ export default RestModel.extend({
if (posts) {
posts.forEach(p => this.storePost(store.createRecord("post", p)));
}
if (result.post_stream.gaps) {
this.set("gaps", _.merge(this.get("gaps"), result.post_stream.gaps));
}
});
},

View File

@ -5,7 +5,15 @@ require_dependency 'gaps'
class TopicView
attr_reader :topic, :posts, :guardian, :filtered_posts, :chunk_size, :print, :message_bus_last_id
attr_reader :topic,
:posts,
:guardian,
:filtered_posts,
:chunk_size,
:print,
:message_bus_last_id,
:contains_gaps
attr_accessor :draft, :draft_key, :draft_sequence, :user_custom_fields, :post_custom_fields, :post_number
def self.slow_chunk_size
@ -98,13 +106,28 @@ class TopicView
path
end
def contains_gaps?
@contains_gaps
end
def gaps
return unless @contains_gaps
@gaps ||= Gaps.new(filtered_post_ids, unfiltered_posts.order(:sort_order).pluck(:id))
@gaps ||= begin
sort_order_max_range =
if @posts.offset(@limit).exists? && unfiltered_posts.order(:sort_order)
.where("sort_order > ?", @posts.last.sort_order)
.offset(@limit)
.exists?
@posts.last.sort_order
else
@topic.highest_post_number
end
unfiltered_ids = unfiltered_posts.order(:sort_order).where("posts.sort_order BETWEEN ? AND ?",
@posts.first.sort_order,
sort_order_max_range
).pluck(:id)
Gaps.new(@posts.pluck(:id), unfiltered_ids)
end
end
def last_post
@ -504,6 +527,7 @@ class TopicView
# This should be last - don't want to tell the admin about deleted posts that clicking the button won't show
# copy the filter for has_deleted? method
@predelete_filtered_posts = @filtered_posts.spawn
if @guardian.can_see_deleted_posts? && !@show_deleted && has_deleted?
@filtered_posts = @filtered_posts.where("deleted_at IS NULL OR post_number = 1")
@contains_gaps = true

View File

@ -344,23 +344,67 @@ describe TopicView do
# Update them to the sort order we're checking for
[p1, p2, p3, p4, p5, p6, p7].each_with_index do |p, idx|
p.sort_order = idx + 1
p.save
p.update!(sort_order: idx + 1)
end
p6.user_id = nil # user got nuked
p6.save!
p6.update!(user_id: nil) # user got nuked
end
describe "contains_gaps?" do
it "works" do
# does not contain contains_gaps with default filtering
expect(topic_view.contains_gaps?).to eq(false)
# contains contains_gaps when filtered by username" do
expect(TopicView.new(topic.id, evil_trout, username_filters: ['eviltrout']).contains_gaps?).to eq(true)
# contains contains_gaps when filtered by summary
expect(TopicView.new(topic.id, evil_trout, filter: 'summary').contains_gaps?).to eq(true)
# contains contains_gaps when filtered by best
expect(TopicView.new(topic.id, evil_trout, best: 5).contains_gaps?).to eq(true)
describe "#gaps" do
describe 'no filter' do
it 'should not contain gaps' do
expect(topic_view.contains_gaps).to eq(false)
expect(topic_view.gaps).to eq(nil)
end
describe 'as an admin' do
it 'should contain gaps' do
p2.update!(deleted_at: Time.zone.now)
topic_view = TopicView.new(topic.id, Fabricate(:admin))
expect(topic_view.contains_gaps).to eq(true)
expect(topic_view.posts).to eq([p1, p3, p5])
expect(topic_view.gaps.before).to eq(
p5.id => [p4.id],
p3.id => [p2.id]
)
expect(topic_view.gaps.after).to eq(p5.id => [p6.id, p7.id])
end
end
end
describe 'filtered by username' do
it 'should contain gaps' do
topic_view = TopicView.new(topic.id, evil_trout,
username_filters: ['eviltrout']
)
expect(topic_view.contains_gaps).to eq(true)
expect(topic_view.gaps.before).to eq(p5.id => [p3.id])
expect(topic_view.gaps.after).to eq({})
end
end
describe 'filtered by summary' do
it 'should contain gaps' do
topic_view = TopicView.new(topic.id, evil_trout, filter: 'summary')
expect(topic_view.contains_gaps).to eq(true)
expect(topic_view.gaps.before).to eq({})
expect(topic_view.gaps.after).to eq({})
end
end
describe 'filtered by best' do
it 'should contain gaps' do
topic_view = TopicView.new(topic.id, evil_trout, best: 5)
expect(topic_view.contains_gaps).to eq(true)
expect(topic_view.gaps.before).to eq({})
expect(topic_view.gaps.after).to eq({})
end
end
end
@ -403,21 +447,21 @@ describe TopicView do
near_view = topic_view_near(p1)
expect(near_view.desired_post).to eq(p1)
expect(near_view.posts).to eq([p1, p2, p3])
expect(near_view.contains_gaps?).to eq(false)
expect(near_view.contains_gaps).to eq(false)
end
it "snaps to the upper boundary" do
near_view = topic_view_near(p5)
expect(near_view.desired_post).to eq(p5)
expect(near_view.posts).to eq([p2, p3, p5])
expect(near_view.contains_gaps?).to eq(false)
expect(near_view.contains_gaps).to eq(false)
end
it "returns the posts in the middle" do
near_view = topic_view_near(p2)
expect(near_view.desired_post).to eq(p2)
expect(near_view.posts).to eq([p1, p2, p3])
expect(near_view.contains_gaps?).to eq(false)
expect(near_view.contains_gaps).to eq(false)
end
it "gaps deleted posts to an admin" do
@ -434,7 +478,7 @@ describe TopicView do
near_view = topic_view_near(p3, true)
expect(near_view.desired_post).to eq(p3)
expect(near_view.posts).to eq([p2, p3, p4])
expect(near_view.contains_gaps?).to eq(false)
expect(near_view.contains_gaps).to eq(false)
end
it "gaps deleted posts by nuked users to an admin" do
@ -452,7 +496,7 @@ describe TopicView do
near_view = topic_view_near(p5, true)
expect(near_view.desired_post).to eq(p5)
expect(near_view.posts).to eq([p4, p5, p6])
expect(near_view.contains_gaps?).to eq(false)
expect(near_view.contains_gaps).to eq(false)
end
context "when 'posts per page' exceeds the number of posts" do
@ -461,7 +505,7 @@ describe TopicView do
it 'returns all the posts' do
near_view = topic_view_near(p5)
expect(near_view.posts).to eq([p1, p2, p3, p5])
expect(near_view.contains_gaps?).to eq(false)
expect(near_view.contains_gaps).to eq(false)
end
it 'gaps deleted posts to admins' do
@ -476,7 +520,7 @@ describe TopicView do
evil_trout.admin = true
near_view = topic_view_near(p5, true)
expect(near_view.posts).to eq([p1, p2, p3, p4, p5, p6, p7])
expect(near_view.contains_gaps?).to eq(false)
expect(near_view.contains_gaps).to eq(false)
end
end
end