1 files changed, 149 insertions, 88 deletions

diff --git a/Bugzilla/Attachment.pm b/Bugzilla/Attachment.pm
index fa8845358..1302fc716 100644
--- a/Bugzilla/Attachment.pm
+++ b/Bugzilla/Attachment.pm
@@ -22,10 +22,11 @@
 #                 Marc Schumann <wurblzap@gmail.com>
 #                 Frédéric Buclin <LpSolit@gmail.com>
 
-use strict;
-
 package Bugzilla::Attachment;
 
+use 5.10.0;
+use strict;
+
 =head1 NAME
 
 Bugzilla::Attachment - Bugzilla attachment class.
@@ -61,6 +62,7 @@ use Bugzilla::Hook;
 
 use File::Copy;
 use List::Util qw(max);
+use Storable qw(dclone);
 
 use base qw(Bugzilla::Object);
 
@@ -75,22 +77,19 @@ use constant LIST_ORDER => ID_FIELD;
 use constant AUDIT_CREATES => 0;
 use constant AUDIT_UPDATES => 0;
 
-sub DB_COLUMNS {
-    my $dbh = Bugzilla->dbh;
-
-    return qw(
-        attach_id
-        bug_id
-        description
-        filename
-        isobsolete
-        ispatch
-        isprivate
-        mimetype
-        modification_time
-        submitter_id),
-        $dbh->sql_date_format('attachments.creation_ts', '%Y.%m.%d %H:%i') . ' AS creation_ts';
-}
+use constant DB_COLUMNS => qw(
+    attach_id
+    bug_id
+    creation_ts
+    description
+    filename
+    isobsolete
+    ispatch
+    isprivate
+    mimetype
+    modification_time
+    submitter_id
+);
 
 use constant REQUIRED_FIELD_MAP => {
     bug_id => 'bug',
@@ -116,7 +115,8 @@ use constant VALIDATORS => {
 };
 
 use constant VALIDATOR_DEPENDENCIES => {
-    mimetype => ['ispatch'],
+    content_type => ['ispatch'],
+    mimetype     => ['ispatch'],
 };
 
 use constant UPDATE_VALIDATORS => {
@@ -142,8 +142,7 @@ the ID of the bug to which the attachment is attached
 =cut
 
 sub bug_id {
-    my $self = shift;
-    return $self->{bug_id};
+    return $_[0]->{bug_id};
 }
 
 =over
@@ -157,11 +156,8 @@ the bug object to which the attachment is attached
 =cut
 
 sub bug {
-    my $self = shift;
-
     require Bugzilla::Bug;
-    $self->{bug} ||= Bugzilla::Bug->new($self->bug_id);
-    return $self->{bug};
+    return $_[0]->{bug} //= Bugzilla::Bug->new({ id => $_[0]->bug_id, cache => 1 });
 }
 
 =over
@@ -175,8 +171,7 @@ user-provided text describing the attachment
 =cut
 
 sub description {
-    my $self = shift;
-    return $self->{description};
+    return $_[0]->{description};
 }
 
 =over
@@ -190,8 +185,7 @@ the attachment's MIME media type
 =cut
 
 sub contenttype {
-    my $self = shift;
-    return $self->{mimetype};
+    return $_[0]->{mimetype};
 }
 
 =over
@@ -205,10 +199,8 @@ the user who attached the attachment
 =cut
 
 sub attacher {
-    my $self = shift;
-    return $self->{attacher} if exists $self->{attacher};
-    $self->{attacher} = new Bugzilla::User($self->{submitter_id});
-    return $self->{attacher};
+    return $_[0]->{attacher}
+        //= new Bugzilla::User({ id => $_[0]->{submitter_id}, cache => 1 });
 }
 
 =over
@@ -222,8 +214,7 @@ the date and time on which the attacher attached the attachment
 =cut
 
 sub attached {
-    my $self = shift;
-    return $self->{creation_ts};
+    return $_[0]->{creation_ts};
 }
 
 =over
@@ -237,8 +228,7 @@ the date and time on which the attachment was last modified.
 =cut
 
 sub modification_time {
-    my $self = shift;
-    return $self->{modification_time};
+    return $_[0]->{modification_time};
 }
 
 =over
@@ -252,8 +242,7 @@ the name of the file the attacher attached
 =cut
 
 sub filename {
-    my $self = shift;
-    return $self->{filename};
+    return $_[0]->{filename};
 }
 
 =over
@@ -267,8 +256,7 @@ whether or not the attachment is a patch
 =cut
 
 sub ispatch {
-    my $self = shift;
-    return $self->{ispatch};
+    return $_[0]->{ispatch};
 }
 
 =over
@@ -282,8 +270,7 @@ whether or not the attachment is obsolete
 =cut
 
 sub isobsolete {
-    my $self = shift;
-    return $self->{isobsolete};
+    return $_[0]->{isobsolete};
 }
 
 =over
@@ -297,8 +284,7 @@ whether or not the attachment is private
 =cut
 
 sub isprivate {
-    my $self = shift;
-    return $self->{isprivate};
+    return $_[0]->{isprivate};
 }
 
 =over
@@ -315,8 +301,7 @@ matches, because this will return a value even if it's matched by the generic
 =cut
 
 sub is_viewable {
-    my $self = shift;
-    my $contenttype = $self->contenttype;
+    my $contenttype = $_[0]->contenttype;
     my $cgi = Bugzilla->cgi;
 
     # We assume we can view all text and image types.
@@ -390,7 +375,7 @@ the length (in characters) of the attachment content
 
 sub datasize {
     my $self = shift;
-    return $self->{datasize} if exists $self->{datasize};
+    return $self->{datasize} if defined $self->{datasize};
 
     # If we have already retrieved the data, return its size.
     return length($self->{data}) if exists $self->{data};
@@ -415,6 +400,53 @@ sub datasize {
     return $self->{datasize};
 }
 
+=over
+
+=item C<linecount>
+
+the number of lines of the attachment content
+
+=back
+
+=cut
+
+# linecount allows for getting the number of lines of an attachment
+# from the database directly if the data has not yet been loaded for
+# performance reasons.
+
+sub linecount {
+    my ($self) = @_;
+
+    return $self->{linecount} if exists $self->{linecount};
+
+    # Limit this to just text/* attachments as this could
+    # cause strange results for binary attachments.
+    return if $self->contenttype !~ /^text\//;
+
+    # If the data has already been loaded, we can just determine
+    # line count from the data directly. 
+    if ($self->{data}) {
+        $self->{linecount} = $self->{data} =~ tr/\n/\n/;
+    }
+    else {
+        $self->{linecount} = 
+            int(Bugzilla->dbh->selectrow_array("
+                SELECT LENGTH(attach_data.thedata)-LENGTH(REPLACE(attach_data.thedata,'\n',''))/LENGTH('\n') 
+                FROM attach_data WHERE id = ?", undef, $self->id));
+    
+    }
+
+    # If we still do not have a linecount either the attachment
+    # is stored in a local file or has been deleted. If the former,
+    # we call self->data to force a load from the filesystem and
+    # then do a split on newlines and count again.
+    unless ($self->{linecount}) {
+        $self->{linecount} = $self->data =~ tr/\n/\n/;
+    }
+
+    return $self->{linecount};
+}
+
 sub _get_local_filename {
     my $self = shift;
     my $hash = ($self->id % 100) + 100;
@@ -433,11 +465,8 @@ flags that have been set on the attachment
 =cut
 
 sub flags {
-    my $self = shift;
-
     # Don't cache it as it must be in sync with ->flag_types.
-    $self->{flags} = [map { @{$_->{flags}} } @{$self->flag_types}];
-    return $self->{flags};
+    return $_[0]->{flags} = [map { @{$_->{flags}} } @{$_[0]->flag_types}];
 }
 
 =over
@@ -458,10 +487,10 @@ sub flag_types {
     my $vars = { target_type  => 'attachment',
                  product_id   => $self->bug->product_id,
                  component_id => $self->bug->component_id,
-                 attach_id    => $self->id };
+                 attach_id    => $self->id, 
+                 active_or_has_flags => $self->bug_id };
 
-    $self->{flag_types} = Bugzilla::Flag->_flag_types($vars);
-    return $self->{flag_types};
+    return $self->{flag_types} = Bugzilla::Flag->_flag_types($vars);
 }
 
 ###############################
@@ -573,7 +602,7 @@ sub _check_filename {
         else {
             ThrowUserError('file_not_specified');
         }
-    }
+   }
 
     # Remove path info (if any) from the file name.  The browser should do this
     # for us, but some are buggy.  This may not work on Mac file names and could
@@ -608,12 +637,12 @@ sub _check_is_private {
 
 =over
 
-=item C<get_attachments_by_bug($bug_id)>
+=item C<get_attachments_by_bug($bug)>
 
 Description: retrieves and returns the attachments the currently logged in
              user can view for the given bug.
 
-Params:     C<$bug_id> - integer - the ID of the bug for which
+Params:     C<$bug> - Bugzilla::Bug object - the bug for which
             to retrieve and return attachments.
 
 Returns:    a reference to an array of attachment objects.
@@ -621,14 +650,14 @@ Returns:    a reference to an array of attachment objects.
 =cut
 
 sub get_attachments_by_bug {
-    my ($class, $bug_id, $vars) = @_;
+    my ($class, $bug, $vars) = @_;
     my $user = Bugzilla->user;
     my $dbh = Bugzilla->dbh;
 
     # By default, private attachments are not accessible, unless the user
     # is in the insider group or submitted the attachment.
     my $and_restriction = '';
-    my @values = ($bug_id);
+    my @values = ($bug->id);
 
     unless ($user->is_insider) {
         $and_restriction = 'AND (isprivate = 0 OR submitter_id = ?)';
@@ -645,45 +674,67 @@ sub get_attachments_by_bug {
     # attachment listed here, we collect all the data at once and
     # populate $attachment->{flags} ourselves.
     if ($vars->{preload}) {
-        $_->{flags} = [] foreach @$attachments;
-        my %att = map { $_->id => $_ } @$attachments;
+        # Preload flag types and flags
+        my $vars = { target_type  => 'attachment',
+                     product_id   => $bug->product_id,
+                     component_id => $bug->component_id,
+                     attach_id    => $attach_ids };
+        my $flag_types = Bugzilla::Flag->_flag_types($vars);
+
+        foreach my $attachment (@$attachments) {
+            $attachment->{flag_types} = [];
+            my $new_types = dclone($flag_types);
+            foreach my $new_type (@$new_types) {
+                $new_type->{flags} = [ grep($_->attach_id == $attachment->id,
+                                            @{ $new_type->{flags} }) ];
+                push(@{ $attachment->{flag_types} }, $new_type);
+            }
+        }
 
-        my $flags = Bugzilla::Flag->match({ bug_id      => $bug_id,
-                                            target_type => 'attachment' });
+        # Preload attachers.
+        my %user_ids = map { $_->{submitter_id} => 1 } @$attachments;
+        my $users = Bugzilla::User->new_from_list([keys %user_ids]);
+        my %user_map = map { $_->id => $_ } @$users;
+        foreach my $attachment (@$attachments) {
+            $attachment->{attacher} = $user_map{$attachment->{submitter_id}};
+        }
 
-        # Exclude flags for private attachments you cannot see.
-        @$flags = grep {exists $att{$_->attach_id}} @$flags;
+        # Preload datasizes.
+        my $sizes =
+          $dbh->selectall_hashref('SELECT attach_id, LENGTH(thedata) AS datasize
+                                   FROM attachments LEFT JOIN attach_data ON attach_id = id
+                                   WHERE bug_id = ?',
+                                   'attach_id', undef, $bug->id);
 
-        push(@{$att{$_->attach_id}->{flags}}, $_) foreach @$flags;
-        $attachments = [sort {$a->id <=> $b->id} values %att];
+        # Force the size of attachments not in the DB to be recalculated.
+        $_->{datasize} = $sizes->{$_->id}->{datasize} || undef foreach @$attachments;
     }
     return $attachments;
 }
 
 =pod
 
-=item C<validate_can_edit($attachment, $product_id)>
+=item C<validate_can_edit>
 
 Description: validates if the user is allowed to view and edit the attachment.
              Only the submitter or someone with editbugs privs can edit it.
              Only the submitter and users in the insider group can view
              private attachments.
 
-Params:      $attachment - the attachment object being edited.
-             $product_id - the product ID the attachment belongs to.
+Params:      none
 
 Returns:     1 on success, 0 otherwise.
 
 =cut
 
 sub validate_can_edit {
-    my ($attachment, $product_id) = @_;
+    my $self = shift;
     my $user = Bugzilla->user;
 
     # The submitter can edit their attachments.
-    return ($attachment->attacher->id == $user->id
-            || ((!$attachment->isprivate || $user->is_insider)
-                 && $user->in_group('editbugs', $product_id))) ? 1 : 0;
+    return ($self->attacher->id == $user->id
+            || ((!$self->isprivate || $user->is_insider)
+                 && $user->in_group('editbugs', $self->bug->product_id))) ? 1 : 0;
 }
 
 =item C<validate_obsolete($bug, $attach_ids)>
@@ -720,7 +771,7 @@ sub validate_obsolete {
           || ThrowUserError('invalid_attach_id', $vars);
 
         # Check that the user can view and edit this attachment.
-        $attachment->validate_can_edit($bug->product_id)
+        $attachment->validate_can_edit
           || ThrowUserError('illegal_attachment_edit', { attach_id => $attachment->id });
 
         if ($attachment->bug_id != $bug->bug_id) {
@@ -852,23 +903,23 @@ sub update {
     }
 
     # Record changes in the activity table.
-    my $sth = $dbh->prepare('INSERT INTO bugs_activity (bug_id, attach_id, who, bug_when,
-                                                        fieldid, removed, added)
-                             VALUES (?, ?, ?, ?, ?, ?, ?)');
-
+    require Bugzilla::Bug;
     foreach my $field (keys %$changes) {
         my $change = $changes->{$field};
         $field = "attachments.$field" unless $field eq "flagtypes.name";
-        my $fieldid = get_field_id($field);
-        $sth->execute($self->bug_id, $self->id, $user->id, $timestamp,
-                      $fieldid, $change->[0], $change->[1]);
+        Bugzilla::Bug::LogActivityEntry($self->bug_id, $field, $change->[0],
+            $change->[1], $user->id, $timestamp, undef, $self->id);
     }
 
     if (scalar(keys %$changes)) {
-      $dbh->do('UPDATE attachments SET modification_time = ? WHERE attach_id = ?',
-               undef, ($timestamp, $self->id));
-      $dbh->do('UPDATE bugs SET delta_ts = ? WHERE bug_id = ?',
-               undef, ($timestamp, $self->bug_id));
+        $dbh->do('UPDATE attachments SET modification_time = ? WHERE attach_id = ?',
+                 undef, ($timestamp, $self->id));
+        $dbh->do('UPDATE bugs SET delta_ts = ? WHERE bug_id = ?',
+                 undef, ($timestamp, $self->bug_id));
+        $self->{modification_time} = $timestamp;
+        # because we updated the attachments table after SUPER::update(), we
+        # need to ensure the cache is flushed.
+        Bugzilla->memcached->clear({ table => 'attachments', id => $self->id });
     }
 
     return $changes;
@@ -893,11 +944,21 @@ sub remove_from_db {
     my $dbh = Bugzilla->dbh;
 
     $dbh->bz_start_transaction();
-    $dbh->do('DELETE FROM flags WHERE attach_id = ?', undef, $self->id);
+    my $flag_ids = $dbh->selectcol_arrayref(
+        'SELECT id FROM flags WHERE attach_id = ?', undef, $self->id);
+    $dbh->do('DELETE FROM flags WHERE ' . $dbh->sql_in('id', $flag_ids))
+        if @$flag_ids;
     $dbh->do('DELETE FROM attach_data WHERE id = ?', undef, $self->id);
     $dbh->do('UPDATE attachments SET mimetype = ?, ispatch = ?, isobsolete = ?
               WHERE attach_id = ?', undef, ('text/plain', 0, 1, $self->id));
     $dbh->bz_commit_transaction();
+
+    # As we don't call SUPER->remove_from_db we need to manually clear
+    # memcached here.
+    Bugzilla->memcached->clear({ table => 'attachments', id => $self->id });
+    foreach my $flag_id (@$flag_ids) {
+        Bugzilla->memcached->clear({ table => 'flags', id => $flag_id });
+    }
 }
 
 ###############################