summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
-rw-r--r--Bugzilla/Attachment.pm432
-rwxr-xr-xBugzilla/Bug.pm4
-rw-r--r--Bugzilla/Flag.pm51
-rw-r--r--template/en/default/attachment/list.html.tmpl12
-rw-r--r--template/en/default/bug/show.xml.tmpl4
-rw-r--r--template/en/default/filterexceptions.pl4
-rw-r--r--template/en/default/global/user-error.html.tmpl11
7 files changed, 407 insertions, 111 deletions
diff --git a/Bugzilla/Attachment.pm b/Bugzilla/Attachment.pm
index 558d7f8bc..578f67b1f 100644
--- a/Bugzilla/Attachment.pm
+++ b/Bugzilla/Attachment.pm
@@ -20,96 +20,384 @@
# Contributor(s): Terry Weissman <terry@mozilla.org>
# Myk Melez <myk@mozilla.org>
-############################################################################
-# Module Initialization
-############################################################################
-
use strict;
package Bugzilla::Attachment;
-# This module requires that its caller have said "require globals.pl" to import
-# relevant functions from that script.
+=head1 NAME
+
+Bugzilla::Attachment - a file related to a bug that a user has uploaded
+ to the Bugzilla server
+
+=head1 SYNOPSIS
+
+ use Bugzilla::Attachment;
+
+ # Get the attachment with the given ID.
+ my $attachment = Bugzilla::Attachment->get($attach_id);
+
+ # Get the attachments with the given IDs.
+ my $attachments = Bugzilla::Attachment->get_list($attach_ids);
+
+=head1 DESCRIPTION
+
+This module defines attachment objects, which represent files related to bugs
+that users upload to the Bugzilla server.
+
+=cut
+
+# This module requires that its caller have said "require globals.pl"
+# to import relevant functions from that script.
-# Use the Flag module to handle flags.
use Bugzilla::Flag;
use Bugzilla::Config qw(:locations);
use Bugzilla::User;
-############################################################################
-# Functions
-############################################################################
-
-sub new {
- # Returns a hash of information about the attachment with the given ID.
+sub get {
+ my $invocant = shift;
+ my $id = shift;
- my ($invocant, $id) = @_;
- return undef if !$id;
- my $self = { 'id' => $id };
- my $class = ref($invocant) || $invocant;
- bless($self, $class);
-
- &::PushGlobalSQLState();
- &::SendSQL("SELECT 1, description, bug_id, isprivate FROM attachments " .
- "WHERE attach_id = $id");
- ($self->{'exists'},
- $self->{'summary'},
- $self->{'bug_id'},
- $self->{'isprivate'}) = &::FetchSQLData();
- &::PopGlobalSQLState();
+ my $attachments = _retrieve([$id]);
+ my $self = $attachments->[0];
+ bless($self, ref($invocant) || $invocant) if $self;
return $self;
}
-sub query
-{
- # Retrieves and returns an array of attachment records for a given bug.
- # This data should be given to attachment/list.html.tmpl in an
- # "attachments" variable.
- my ($bugid) = @_;
-
- my $dbh = Bugzilla->dbh;
-
- # Retrieve a list of attachments for this bug and write them into an array
- # of hashes in which each hash represents a single attachment.
- my $list = $dbh->selectall_arrayref("SELECT attach_id, " .
- $dbh->sql_date_format('creation_ts', '%Y.%m.%d %H:%i') .
- ", mimetype, description, ispatch,
- isobsolete, isprivate, LENGTH(thedata),
- submitter_id
- FROM attachments
- INNER JOIN attach_data
- ON id = attach_id
- WHERE bug_id = ? ORDER BY attach_id",
- undef, $bugid);
-
- my @attachments = ();
- foreach my $row (@$list) {
- my %a;
- ($a{'attachid'}, $a{'date'}, $a{'contenttype'},
- $a{'description'}, $a{'ispatch'}, $a{'isobsolete'},
- $a{'isprivate'}, $a{'datasize'}, $a{'submitter_id'}) = @$row;
-
- $a{'submitter'} = new Bugzilla::User($a{'submitter_id'});
-
- # Retrieve a list of flags for this attachment.
- $a{'flags'} = Bugzilla::Flag::match({ 'attach_id' => $a{'attachid'},
- 'is_active' => 1 });
-
- # A zero size indicates that the attachment is stored locally.
- if ($a{'datasize'} == 0) {
- my $attachid = $a{'attachid'};
- my $hash = ($attachid % 100) + 100;
- $hash =~ s/.*(\d\d)$/group.$1/;
- if (open(AH, "$attachdir/$hash/attachment.$attachid")) {
- $a{'datasize'} = (stat(AH))[7];
+sub get_list {
+ my $invocant = shift;
+ my $ids = shift;
+
+ my $attachments = _retrieve($ids);
+ foreach my $attachment (@$attachments) {
+ bless($attachment, ref($invocant) || $invocant);
+ }
+
+ return $attachments;
+}
+
+sub _retrieve {
+ my ($ids) = @_;
+
+ return [] if scalar(@$ids) == 0;
+
+ my @columns = (
+ 'attachments.attach_id AS id',
+ 'attachments.bug_id AS bug_id',
+ 'attachments.description AS description',
+ 'attachments.mimetype AS contenttype',
+ 'attachments.submitter_id AS _attacher_id',
+ Bugzilla->dbh->sql_date_format('attachments.creation_ts',
+ '%Y.%m.%d %H:%i') . " AS attached",
+ 'attachments.filename AS filename',
+ 'attachments.ispatch AS ispatch',
+ 'attachments.isobsolete AS isobsolete',
+ 'attachments.isprivate AS isprivate'
+ );
+ my $columns = join(", ", @columns);
+
+ my $records = Bugzilla->dbh->selectall_arrayref("SELECT $columns
+ FROM attachments
+ WHERE attach_id IN (" .
+ join(",", @$ids) . ")",
+ { Slice => {} });
+ return $records;
+}
+
+=pod
+
+=head2 Instance Properties
+
+=over
+
+=item C<id>
+
+the unique identifier for the attachment
+
+=back
+
+=cut
+
+sub id {
+ my $self = shift;
+ return $self->{id};
+}
+
+=over
+
+=item C<bug_id>
+
+the ID of the bug to which the attachment is attached
+
+=back
+
+=cut
+
+# XXX Once Bug.pm slims down sufficiently this should become a reference
+# to a bug object.
+sub bug_id {
+ my $self = shift;
+ return $self->{bug_id};
+}
+
+=over
+
+=item C<description>
+
+user-provided text describing the attachment
+
+=back
+
+=cut
+
+sub description {
+ my $self = shift;
+ return $self->{description};
+}
+
+=over
+
+=item C<contenttype>
+
+the attachment's MIME media type
+
+=back
+
+=cut
+
+sub contenttype {
+ my $self = shift;
+ return $self->{contenttype};
+}
+
+=over
+
+=item C<attacher>
+
+the user who attached the attachment
+
+=back
+
+=cut
+
+sub attacher {
+ my $self = shift;
+ return $self->{attacher} if exists $self->{attacher};
+ $self->{attacher} = new Bugzilla::User($self->{_attacher_id});
+ return $self->{attacher};
+}
+
+=over
+
+=item C<attached>
+
+the date and time on which the attacher attached the attachment
+
+=back
+
+=cut
+
+sub attached {
+ my $self = shift;
+ return $self->{attached};
+}
+
+=over
+
+=item C<filename>
+
+the name of the file the attacher attached
+
+=back
+
+=cut
+
+sub filename {
+ my $self = shift;
+ return $self->{filename};
+}
+
+=over
+
+=item C<ispatch>
+
+whether or not the attachment is a patch
+
+=back
+
+=cut
+
+sub ispatch {
+ my $self = shift;
+ return $self->{ispatch};
+}
+
+=over
+
+=item C<isobsolete>
+
+whether or not the attachment is obsolete
+
+=back
+
+=cut
+
+sub isobsolete {
+ my $self = shift;
+ return $self->{isobsolete};
+}
+
+=over
+
+=item C<isprivate>
+
+whether or not the attachment is private
+
+=back
+
+=cut
+
+sub isprivate {
+ my $self = shift;
+ return $self->{isprivate};
+}
+
+=over
+
+=item C<data>
+
+the content of the attachment
+
+=back
+
+=cut
+
+sub data {
+ my $self = shift;
+ return $self->{data} if exists $self->{data};
+
+ # First try to get the attachment data from the database.
+ ($self->{data}) = Bugzilla->dbh->selectrow_array("SELECT thedata
+ FROM attach_data
+ WHERE id = ?",
+ undef,
+ $self->{id});
+
+ # If there's no attachment data in the database, the attachment is stored
+ # in a local file, so retrieve it from there.
+ if (length($self->{data}) == 0) {
+ if (open(AH, $self->_get_local_filename())) {
+ binmode AH;
+ $self->{data} = <AH>;
close(AH);
}
}
- push @attachments, \%a;
- }
+
+ return $self->{data};
+}
+
+=over
+
+=item C<datasize>
+
+the length (in characters) of the attachment content
+
+=back
+
+=cut
+
+# datasize is a property of the data itself, and it's unclear whether we should
+# expose it at all, since you can easily derive it from the data itself: in TT,
+# attachment.data.size; in Perl, length($attachment->{data}). But perhaps
+# it makes sense for performance reasons, since accessing the data forces it
+# to get retrieved from the database/filesystem and loaded into memory,
+# while datasize avoids loading the attachment into memory, calling SQL's
+# LENGTH() function or stat()ing the file instead. I've left it in for now.
+
+sub datasize {
+ my $self = shift;
+ return $self->{datasize} if exists $self->{datasize};
+
+ # If we have already retrieved the data, return its size.
+ return length($self->{data}) if exists $self->{data};
+
+ ($self->{datasize}) =
+ Bugzilla->dbh->selectrow_array("SELECT LENGTH(thedata)
+ FROM attach_data
+ WHERE id = ?",
+ undef,
+ $self->{id});
+
+ # If there's no attachment data in the database, the attachment
+ # is stored in a local file, so retrieve its size from the file.
+ if ($self->{datasize} == 0) {
+ if (open(AH, $self->_get_local_filename())) {
+ binmode AH;
+ $self->{datasize} = (stat(AH))[7];
+ close(AH);
+ }
+ }
+
+ return $self->{datasize};
+}
+
+=over
+
+=item C<flags>
+
+flags that have been set on the attachment
+
+=back
+
+=cut
+
+sub flags {
+ my $self = shift;
+ return $self->{flags} if exists $self->{flags};
+
+ $self->{flags} = Bugzilla::Flag::match({ attach_id => $self->id,
+ is_active => 1 });
+ return $self->{flags};
+}
+
+# Instance methods; no POD documentation here yet because the only one so far
+# is private.
+
+sub _get_local_filename {
+ my $self = shift;
+ my $hash = ($self->id % 100) + 100;
+ $hash =~ s/.*(\d\d)$/group.$1/;
+ return "$attachdir/$hash/attachment.$self->id";
+}
+
+=pod
+
+=head2 Class Methods
+
+=over
+
+=item C<get_attachments_by_bug($bug_id)>
+
+Description: retrieves and returns the attachments for the given bug.
+
+Params: C<$bug_id> - integer - the ID of the bug for which
+ to retrieve and return attachments.
+
+Returns: a reference to an array of attachment objects.
+
+=back
+
+=cut
- return \@attachments;
+sub get_attachments_by_bug {
+ my ($class, $bug_id) = @_;
+ my $attach_ids = Bugzilla->dbh->selectcol_arrayref("SELECT attach_id
+ FROM attachments
+ WHERE bug_id = ?
+ ORDER BY attach_id",
+ undef, $bug_id);
+ my $attachments = Bugzilla::Attachment->get_list($attach_ids);
+ return $attachments;
}
1;
diff --git a/Bugzilla/Bug.pm b/Bugzilla/Bug.pm
index c287d0c0c..dfa419316 100755
--- a/Bugzilla/Bug.pm
+++ b/Bugzilla/Bug.pm
@@ -354,7 +354,9 @@ sub attachments {
my ($self) = @_;
return $self->{'attachments'} if exists $self->{'attachments'};
return [] if $self->{'error'};
- $self->{'attachments'} = Bugzilla::Attachment::query($self->{bug_id});
+
+ $self->{'attachments'} =
+ Bugzilla::Attachment->get_attachments_by_bug($self->bug_id);
return $self->{'attachments'};
}
diff --git a/Bugzilla/Flag.pm b/Bugzilla/Flag.pm
index 3e72c5933..89dda08a5 100644
--- a/Bugzilla/Flag.pm
+++ b/Bugzilla/Flag.pm
@@ -347,26 +347,26 @@ sub validate {
# can_see_bug() will refer to old settings.
if (!$requestee->can_see_bug($bug_id)) {
ThrowUserError("flag_requestee_unauthorized",
- { flag_type => $flag->{'type'},
- requestee => $requestee,
- bug_id => $bug_id,
- attach_id =>
- $flag->{target}->{attachment}->{id} });
+ { flag_type => $flag->{'type'},
+ requestee => $requestee,
+ bug_id => $bug_id,
+ attachment => $flag->{target}->{attachment}
+ });
}
# Throw an error if the target is a private attachment and
# the requestee isn't in the group of insiders who can see it.
- if ($flag->{target}->{attachment}->{exists}
+ if ($flag->{target}->{attachment}
&& $cgi->param('isprivate')
&& Param("insidergroup")
&& !$requestee->in_group(Param("insidergroup")))
{
ThrowUserError("flag_requestee_unauthorized_attachment",
- { flag_type => $flag->{'type'},
- requestee => $requestee,
- bug_id => $bug_id,
- attach_id =>
- $flag->{target}->{attachment}->{id} });
+ { flag_type => $flag->{'type'},
+ requestee => $requestee,
+ bug_id => $bug_id,
+ attachment => $flag->{target}->{attachment}
+ });
}
}
}
@@ -532,7 +532,9 @@ sub create {
$flag->{'id'} = (&::FetchOneColumn() || 0) + 1;
# Insert a record for the flag into the flags table.
- my $attach_id = $flag->{'target'}->{'attachment'}->{'id'} || "NULL";
+ my $attach_id =
+ $flag->{target}->{attachment} ? $flag->{target}->{attachment}->{id}
+ : "NULL";
my $requestee_id = $flag->{'requestee'} ? $flag->{'requestee'}->id : "NULL";
&::SendSQL("INSERT INTO flags (id, type_id,
bug_id, attach_id,
@@ -807,7 +809,8 @@ sub FormToNewFlags {
{ 'type_id' => $type_id,
'target_type' => $target->{'type'},
'bug_id' => $target->{'bug'}->{'id'},
- 'attach_id' => $target->{'attachment'}->{'id'},
+ 'attach_id' => $target->{'attachment'} ?
+ $target->{'attachment'}->{'id'} : undef,
'is_active' => 1 });
# Do not create a new flag of this type if this flag type is
@@ -902,15 +905,18 @@ sub get_target {
my $target = { 'exists' => 0 };
if ($attach_id) {
- $target->{'attachment'} = new Bugzilla::Attachment($attach_id);
+ $target->{'attachment'} = Bugzilla::Attachment->get($attach_id);
if ($bug_id) {
# Make sure the bug and attachment IDs correspond to each other
# (i.e. this is the bug to which this attachment is attached).
- $bug_id == $target->{'attachment'}->{'bug_id'}
- || return { 'exists' => 0 };
+ if (!$target->{'attachment'}
+ || $target->{'attachment'}->{'bug_id'} != $bug_id)
+ {
+ return { 'exists' => 0 };
+ }
}
- $target->{'bug'} = GetBug($target->{'attachment'}->{'bug_id'});
- $target->{'exists'} = $target->{'attachment'}->{'exists'};
+ $target->{'bug'} = GetBug($bug_id);
+ $target->{'exists'} = 1;
$target->{'type'} = "attachment";
}
elsif ($bug_id) {
@@ -937,20 +943,21 @@ Sends an email notification about a flag being created or fulfilled.
sub notify {
my ($flag, $template_file) = @_;
+ my $attachment_is_private = $flag->{'target'}->{'attachment'} ?
+ $flag->{'target'}->{'attachment'}->{'isprivate'} : undef;
+
# If the target bug is restricted to one or more groups, then we need
# to make sure we don't send email about it to unauthorized users
# on the request type's CC: list, so we have to trawl the list for users
# not in those groups or email addresses that don't have an account.
- if ($flag->{'target'}->{'bug'}->{'restricted'}
- || $flag->{'target'}->{'attachment'}->{'isprivate'})
- {
+ if ($flag->{'target'}->{'bug'}->{'restricted'} || $attachment_is_private) {
my @new_cc_list;
foreach my $cc (split(/[, ]+/, $flag->{'type'}->{'cc_list'})) {
my $ccuser = Bugzilla::User->new_from_login($cc) || next;
next if $flag->{'target'}->{'bug'}->{'restricted'}
&& !$ccuser->can_see_bug($flag->{'target'}->{'bug'}->{'id'});
- next if $flag->{'target'}->{'attachment'}->{'isprivate'}
+ next if $attachment_is_private
&& Param("insidergroup")
&& !$ccuser->in_group(Param("insidergroup"));
push(@new_cc_list, $cc);
diff --git a/template/en/default/attachment/list.html.tmpl b/template/en/default/attachment/list.html.tmpl
index 4f66a5eb8..61b68ee53 100644
--- a/template/en/default/attachment/list.html.tmpl
+++ b/template/en/default/attachment/list.html.tmpl
@@ -37,7 +37,7 @@
[% IF !attachment.isprivate || canseeprivate %]
<tr [% "class=\"bz_private\"" IF attachment.isprivate %]>
<td valign="top">
- <a href="attachment.cgi?id=[% attachment.attachid %]">[% attachment.description FILTER html FILTER obsolete(attachment.isobsolete) %]</a>
+ <a href="attachment.cgi?id=[% attachment.id %]">[% attachment.description FILTER html FILTER obsolete(attachment.isobsolete) %]</a>
</td>
<td valign="top">
@@ -49,11 +49,11 @@
</td>
<td valign="top">
- <a href="mailto:[% attachment.submitter.email FILTER html %]">
- [% attachment.submitter.name || attachment.submitter.login FILTER html %]
+ <a href="mailto:[% attachment.attacher.email FILTER html %]">
+ [% attachment.attacher.name || attachment.attacher.login FILTER html %]
</a>
</td>
- <td valign="top">[% attachment.date FILTER time %]</td>
+ <td valign="top">[% attachment.attached FILTER time %]</td>
<td valign="top">[% attachment.datasize FILTER unitconvert %]</td>
[% IF show_attachment_flags %]
@@ -75,9 +75,9 @@
[% END %]
<td valign="top">
- <a href="attachment.cgi?id=[% attachment.attachid %]&amp;action=edit">Edit</a>
+ <a href="attachment.cgi?id=[% attachment.id %]&amp;action=edit">Edit</a>
[% IF attachment.ispatch && patchviewerinstalled %]
- | <a href="attachment.cgi?id=[% attachment.attachid %]&amp;action=diff">Diff</a>
+ | <a href="attachment.cgi?id=[% attachment.id %]&amp;action=diff">Diff</a>
[% END %]
[% Hook.process("action") %]
</td>
diff --git a/template/en/default/bug/show.xml.tmpl b/template/en/default/bug/show.xml.tmpl
index 222204936..2555600ed 100644
--- a/template/en/default/bug/show.xml.tmpl
+++ b/template/en/default/bug/show.xml.tmpl
@@ -73,8 +73,8 @@
ispatch="1"
[% END %]
>
- <attachid>[% a.attachid %]</attachid>
- <date>[% a.date FILTER time FILTER xml %]</date>
+ <attachid>[% a.id %]</attachid>
+ <date>[% a.attached FILTER time FILTER xml %]</date>
<desc>[% a.description FILTER xml %]</desc>
<ctype>[% a.contenttype FILTER xml %]</ctype>
[% FOREACH flag = a.flags %]
diff --git a/template/en/default/filterexceptions.pl b/template/en/default/filterexceptions.pl
index cc5a19d09..8ed71f008 100644
--- a/template/en/default/filterexceptions.pl
+++ b/template/en/default/filterexceptions.pl
@@ -368,7 +368,7 @@
'bug/show.xml.tmpl' => [
'VERSION',
- 'a.attachid',
+ 'a.id',
'field',
],
@@ -458,7 +458,7 @@
],
'attachment/list.html.tmpl' => [
- 'attachment.attachid',
+ 'attachment.id',
'flag.status',
'bugid',
],
diff --git a/template/en/default/global/user-error.html.tmpl b/template/en/default/global/user-error.html.tmpl
index 3e1a2a3ef..53fdcc59f 100644
--- a/template/en/default/global/user-error.html.tmpl
+++ b/template/en/default/global/user-error.html.tmpl
@@ -442,7 +442,7 @@
You asked [% requestee.identity FILTER html %]
for <code>[% flag_type.name FILTER html %]</code> on [% terms.bug %]
[% bug_id FILTER html -%]
- [% IF attach_id %], attachment [% attach_id FILTER html %][% END %],
+ [% IF attachment %], attachment [% attachment.id FILTER html %][% END %],
but that [% terms.bug %] has been restricted to users in certain groups,
and the user you asked isn't in all the groups to which
the [% terms.bug %] has been restricted.
@@ -455,11 +455,10 @@
You asked [% requestee.identity FILTER html %]
for <code>[% flag_type.name FILTER html %]</code> on
[%+ terms.bug %] [%+ bug_id FILTER html %],
- attachment [% attach_id FILTER html %], but that attachment is restricted
- to users
- in the [% Param("insidergroup") FILTER html %] group, and the user
- you asked isn't in that group. Please choose someone else to ask,
- or ask an administrator to add the user to the group.
+ attachment [% attachment.id FILTER html %], but that attachment
+ is restricted to users in the [% Param("insidergroup") FILTER html %] group,
+ and the user you asked isn't in that group. Please choose someone else
+ to ask, or ask an administrator to add the user to the group.
[% ELSIF error == "flag_type_cc_list_invalid" %]
[% title = "Flag Type CC List Invalid" %]