Bug 859313: minimise "Lock wait timeout exceeded" errors in TagNewUser

3 files changed, 40 insertions, 46 deletions

diff --git a/extensions/TagNewUsers/Extension.pm b/extensions/TagNewUsers/Extension.pm
index f005d3e5e..4afebeb78 100644
--- a/extensions/TagNewUsers/Extension.pm
+++ b/extensions/TagNewUsers/Extension.pm
@@ -20,12 +20,6 @@ use constant PROFILE_AGE => 60;
 # users with fewer comments than COMMENT_COUNT will be tagged as new
 use constant COMMENT_COUNT => 25;
 
-# users to always treat as not-new
-# note: users in this list won't have their comment_count field updated
-use constant NEVER_NEW => (
-    'tbplbot@gmail.com',    # the TinderBoxPushLog robot is very frequent commenter
-);
-
 our $VERSION = '1';
 
 #
@@ -118,11 +112,6 @@ sub install_update_db {
 # objects
 #
 
-BEGIN {
-    *Bugzilla::User::update_comment_count = \&_update_comment_count;
-    *Bugzilla::User::first_patch_bug_id = \&_first_patch_bug_id;
-}
-
 sub object_columns {
     my ($self, $args) = @_;
     my ($class, $columns) = @$args{qw(class columns)};
@@ -145,20 +134,22 @@ sub object_before_create {
     }
 }
 
-sub bug_end_of_create {
-    Bugzilla->user->update_comment_count();
-}
+#
+# Bugzilla::User methods
+#
 
-sub bug_end_of_update {
-    Bugzilla->user->update_comment_count();
+BEGIN {
+    *Bugzilla::User::update_comment_count = \&_update_comment_count;
+    *Bugzilla::User::first_patch_bug_id = \&_first_patch_bug_id;
+    *Bugzilla::User::is_new = \&_is_new;
 }
 
 sub _update_comment_count {
     my $self = shift;
     my $dbh = Bugzilla->dbh;
 
-    my $login = $self->login;
-    return if grep { $_ eq $login } NEVER_NEW;
+    # no need to update this counter for users which are no longer new
+    return unless $self->is_new;
 
     my $id = $self->id;
     my ($count) = $dbh->selectrow_array(
@@ -184,10 +175,33 @@ sub _first_patch_bug_id {
     $self->{first_patch_bug_id} = $bug_id;
 }
 
+sub _is_new {
+    my ($self) = @_;
+
+    if (!exists $self->{is_new}) {
+        if ($self->in_group('canconfirm')) {
+            $self->{is_new} = 0;
+        } else {
+            $self->{is_new} = ($self->{comment_count} <= COMMENT_COUNT)
+                              || ($self->{creation_age} <= PROFILE_AGE);
+        }
+    }
+
+    return $self->{is_new};
+}
+
 #
-#
+# hooks
 #
 
+sub bug_end_of_create {
+    Bugzilla->user->update_comment_count();
+}
+
+sub bug_end_of_update {
+    Bugzilla->user->update_comment_count();
+}
+
 sub template_before_process {
     my ($self, $args) = @_;
     my ($vars, $file) = @$args{qw(vars file)};
@@ -198,30 +212,14 @@ sub template_before_process {
 
         # calculate if each user that has commented on the bug is new
         foreach my $comment (@{$vars->{bug}{comments}}) {
+            # store the age in days, for the 'new to bugzilla' tooltip
             my $user = $comment->author;
-            $user->{is_new} = $self->_user_is_new($user);
+            my $age = sprintf("%.0f", (time() - str2time($user->{creation_ts})) / 86400);
+            $user->{creation_age} = $age;
         }
     }
 }
 
-sub _user_is_new {
-    my ($self, $user) = (shift, shift);
-
-    my $login = $user->login;
-    return 0 if grep { $_ eq $login} NEVER_NEW;
-
-    # if the user can confirm bugs, they are no longer new
-    return 0 if $user->in_group('canconfirm');
-
-    # store the age in days, for the 'new to bugzilla' tooltip
-    my $age = sprintf("%.0f", (time() - str2time($user->{creation_ts})) / 86400);
-    $user->{creation_age} = $age;
-
-    return
-        ($user->{comment_count} <= COMMENT_COUNT)
-        || ($user->{creation_age} <= PROFILE_AGE);
-}
-
 sub mailer_before_send {
     my ($self, $args) = @_;
     my $email = $args->{email};
@@ -253,8 +251,7 @@ sub webservice_user_get {
         my $email = blessed $user->{'email'} ? $user->{'email'}->value : $user->{'email'};
         if ($email) {
             my $user_obj = Bugzilla::User->new({ name => $email });
-            $user->{'is_new'}
-                = $webservice->type('boolean', $self->_user_is_new($user_obj) ? 1 : 0);
+            $user->{'is_new'} = $webservice->type('boolean', $user_obj->is_new ? 1 : 0);
         }
     }
 }
diff --git a/extensions/TagNewUsers/template/en/default/hook/bug/comments-user.html.tmpl b/extensions/TagNewUsers/template/en/default/hook/bug/comments-user.html.tmpl
index 3ee9a3757..81cfc776a 100644
--- a/extensions/TagNewUsers/template/en/default/hook/bug/comments-user.html.tmpl
+++ b/extensions/TagNewUsers/template/en/default/hook/bug/comments-user.html.tmpl
@@ -6,10 +6,9 @@
   # defined by the Mozilla Public License, v. 2.0.
   #%]
 
+[% RETURN UNLESS user.in_group('canconfirm') %]
 [% IF comment.author.is_new %]
-<span
-  class="new_user"
-  title="
+<span class="new_user" title="
 [%- comment.author.comment_count FILTER html %] comment[% "s" IF comment.author.comment_count != 1 -%]
 , created [%
 IF comment.author.creation_age == 0 %]today[%
diff --git a/extensions/TagNewUsers/template/en/default/hook/bug/comments-comment_banner.html.tmpl b/extensions/TagNewUsers/template/en/default/hook/bug/show-header-end.html.tmpl
index 396f9c35a..bff73e963 100644
--- a/extensions/TagNewUsers/template/en/default/hook/bug/comments-comment_banner.html.tmpl
+++ b/extensions/TagNewUsers/template/en/default/hook/bug/show-header-end.html.tmpl
@@ -6,6 +6,4 @@
   # defined by the Mozilla Public License, v. 2.0.
   #%]
 
-<link
- href="[% 'extensions/TagNewUsers/web/style.css' FILTER mtime FILTER html %]"
- rel="stylesheet" type="text/css" >
+[% style_urls.push('extensions/TagNewUsers/web/style.css') IF user.in_group('canconfirm') %]