![[site community profile]](https://www.dreamwidth.org/img/comm_staff.png){kind=small}
[dw-free] http://bugs.dwscoalition.org/show_bug.cgi?id=3641
[commit: http://hg.dwscoalition.org/dw-free/rev/40cdb938430c]
http://bugs.dwscoalition.org/show_bug.cgi?id=3641
Better caching, and short-circuit when we hit something in the cache.
Patch by![[personal profile]](https://www.dreamwidth.org/img/silk/identity/user.png)
fu.
Files modified:
http://bugs.dwscoalition.org/show_bug.cgi?id=3641
Better caching, and short-circuit when we hit something in the cache.
Patch by
fu.Files modified:
- cgi-bin/LJ/Comment.pm
- cgi-bin/LJ/S2.pm
- htdocs/talkread.bml
- t/subscription-nested.t
--------------------------------------------------------------------------------
diff -r 116559a9f5bb -r 40cdb938430c cgi-bin/LJ/Comment.pm
--- a/cgi-bin/LJ/Comment.pm Tue May 17 10:20:10 2011 +0800
+++ b/cgi-bin/LJ/Comment.pm Tue May 17 10:38:49 2011 +0800
@@ -1086,6 +1086,54 @@
};
}
+sub thread_has_subscription {
+ my ( $comment, $remote, $u ) = @_;
+
+ my @unknown_tracking_status;
+ my $watched = 0;
+
+ while ( $comment && $comment->valid && $comment->parenttalkid ) {
+ # check cache
+ $comment->{_watchedby} ||= {};
+ my $thread_watched = $comment->{_watchedby}->{$u->{userid}};
+
+ my $had_cached = defined $thread_watched;
+
+ unless ( $had_cached ) {
+ $thread_watched = $remote->has_subscription(
+ event => "JournalNewComment",
+ journal => $u,
+ arg2 => $comment->parenttalkid,
+ require_active => 1,
+ );
+ }
+
+ if ( $thread_watched ) {
+ $watched = 1;
+
+ # we had to go up a couple of levels before we could figure out
+ # whether we were watching or not
+ # so fix the status of intervening levels
+ foreach ( @unknown_tracking_status ) {
+ my $c = LJ::Comment->new( $u, dtalkid => $_ );
+ $c->{_watchedby}->{$u->{userid}} = $thread_watched;
+ }
+ @unknown_tracking_status = ();
+ }
+
+ # cache in this comment object if it's being watched by this user
+ $comment->{_watchedby}->{$u->{userid}} = $thread_watched;
+
+ # shortcircuit and stop going up the tree because:
+ last if $thread_watched; # current comment is watched
+ last if $had_cached; # we've been to this section of the ancestor tree already
+
+ push @unknown_tracking_status, $comment->dtalkid;
+ $comment = $comment->parent;
+ }
+
+ return $watched;
+}
sub indent {
return LJ::Talk::Post::indent(@_);
}
diff -r 116559a9f5bb -r 40cdb938430c cgi-bin/LJ/S2.pm
--- a/cgi-bin/LJ/S2.pm Tue May 17 10:20:10 2011 +0800
+++ b/cgi-bin/LJ/S2.pm Tue May 17 10:38:49 2011 +0800
@@ -3271,24 +3271,7 @@
# in other words, the user is not subscribed to this particular comment
# see if any parents are being watched
- my $watching_parent = 0;
- while ($comment && $comment->valid && $comment->parenttalkid) {
- # check cache
- $comment->{_watchedby} ||= {};
- my $thread_watched = $comment->{_watchedby}->{$u->{userid}};
-
- # not cached
- if (! defined $thread_watched) {
- $thread_watched = $remote->has_subscription(journal => $u, event => "JournalNewComment", arg2 => $comment->parenttalkid);
- }
-
- $watching_parent = 1 if ($thread_watched);
-
- # cache in this comment object if it's being watched by this user
- $comment->{_watchedby}->{$u->{userid}} = $thread_watched;
-
- $comment = $comment->parent;
- }
+ my $watching_parent = $comment->thread_has_subscription( $remote, $u );
my $etypeid = 'LJ::Event::JournalNewComment'->etypeid;
my %subparams = (
diff -r 116559a9f5bb -r 40cdb938430c htdocs/talkread.bml
--- a/htdocs/talkread.bml Tue May 17 10:20:10 2011 +0800
+++ b/htdocs/talkread.bml Tue May 17 10:38:49 2011 +0800
@@ -535,28 +535,7 @@
$track_img = 'track_active';
} else {
# see if any parents are being watched
- while ($comment && $comment->valid && $comment->parenttalkid) {
- # check cache
- $comment->{_watchedby} ||= {};
- my $thread_watched = $comment->{_watchedby}->{$u->{userid}};
-
- # not cached
- if (! defined $thread_watched) {
- $thread_watched = $remote->has_subscription(
- event => "JournalNewComment",
- journal => $u,
- arg2 => $comment->parenttalkid,
- require_active => 1,
- );
- }
-
- $track_img = 'track_thread_active' if ($thread_watched);
-
- # cache in this comment object if it's being watched by this user
- $comment->{_watchedby}->{$u->{userid}} = $thread_watched;
-
- $comment = $comment->parent;
- }
+ $track_img = 'track_thread_active' if $comment->thread_has_subscription( $remote, $u );
}
my $track_url = "$LJ::SITEROOT/manage/tracking/comments?journal=$u->{'user'}&talkid=$dtid";
diff -r 116559a9f5bb -r 40cdb938430c t/subscription-nested.t
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/t/subscription-nested.t Tue May 17 10:38:49 2011 +0800
@@ -0,0 +1,86 @@
+use strict;
+use Test::More;
+use lib "$ENV{LJHOME}/cgi-bin";
+require 'ljlib.pl';
+
+use LJ::Comment;
+use LJ::Talk;
+use LJ::Test qw( temp_user );
+
+sub check_thread_subscription {
+ my ( $entry, $u, $no_sub ) = @_;
+
+ subtest "checking thread subscription" => sub {
+ my $time = time();
+ my @comments = $entry->comment_list;
+
+ foreach my $comment ( @comments ) {
+ my $time = time();
+ my $has_sub = $comment->thread_has_subscription( $u, $entry->journal );
+
+ if ( $no_sub->{$comment->jtalkid} ) {
+ ok( ! $has_sub, $no_sub->{$comment->jtalkid} );
+ } else {
+ ok( $has_sub, "ancestor of " . $comment->jtalkid . " is tracked" );
+ }
+ }
+ }
+}
+
+note("shallow comment threads");
+{
+ my $u = temp_user();
+ my $e = $u->t_post_fake_entry;
+
+ my @comments;
+ my $c = $e->t_enter_comment;
+ push @comments, $c;
+
+ foreach ( 1...10 ) {
+ $c = $c->t_reply;
+ push @comments, $c;
+ }
+
+ $u->subscribe( event => "JournalNewComment",
+ method => "Email",
+ journal => $u,
+ arg2 => $comments[2]->jtalkid,
+ );
+
+ check_thread_subscription( $e, $u, {
+ # not subscribed to # because....
+ 1 => "above subscription point",
+ 2 => "above subscription point",
+ 3 => "subscribed starting from this comment (but not subscribed to ancestor)",
+ } );
+}
+
+note("deep comment thread");
+{
+ my $u = temp_user();
+ my $e = $u->t_post_fake_entry;
+
+ my @comments;
+ my $c = $e->t_enter_comment;
+ push @comments, $c;
+
+ foreach ( 1...500 ) {
+ $c = $c->t_reply;
+ push @comments, $c;
+ }
+
+ $u->subscribe( event => "JournalNewComment",
+ method => "Email",
+ journal => $u,
+ arg2 => $comments[2]->jtalkid,
+ );
+
+ check_thread_subscription( $e, $u, {
+ # not subscribed to # because....
+ 1 => "above subscription point",
+ 2 => "above subscription point",
+ 3 => "subscribed starting from this comment (but not subscribed to ancestor)",
+ } );
+}
+
+done_testing();
--------------------------------------------------------------------------------