Bug 412074: Ability to add attachments to a bug via the WebService

(Bug.add_attachment)
r=timello, a=mkanat

5 files changed, 207 insertions, 3 deletions

diff --git a/Bugzilla/Attachment.pm b/Bugzilla/Attachment.pm
index 39afe5df1..0f91d29c4 100644
--- a/Bugzilla/Attachment.pm
+++ b/Bugzilla/Attachment.pm
@@ -838,6 +838,8 @@ sub create {
     $sth->bind_param(1, $data, $dbh->BLOB_TYPE);
     $sth->execute();
 
+    $attachment->{bug} = $bug;
+
     # Return the new attachment object.
     return $attachment;
 }
diff --git a/Bugzilla/WebService.pm b/Bugzilla/WebService.pm
index fe7766ad1..9e83a5a64 100644
--- a/Bugzilla/WebService.pm
+++ b/Bugzilla/WebService.pm
@@ -25,6 +25,8 @@ use XMLRPC::Lite;
 
 # Used by the JSON-RPC server to convert incoming date fields apprpriately.
 use constant DATE_FIELDS => {};
+# Used by the JSON-RPC server to convert incoming base64 fields appropriately.
+use constant BASE64_FIELDS => {};
 
 # For some methods, we shouldn't call Bugzilla->login before we call them
 use constant LOGIN_EXEMPT => { };
@@ -106,6 +108,11 @@ May be null.
 
 True or false.
 
+=item C<base64>
+
+A base64-encoded string. This is the only way to transfer
+binary data via the WebService.
+
 =item C<array>
 
 An array. There may be mixed types in an array.
diff --git a/Bugzilla/WebService/Bug.pm b/Bugzilla/WebService/Bug.pm
index c083bd491..78709d81e 100644
--- a/Bugzilla/WebService/Bug.pm
+++ b/Bugzilla/WebService/Bug.pm
@@ -50,6 +50,10 @@ use constant DATE_FIELDS => {
     update   => ['deadline'],
 };
 
+use constant BASE64_FIELDS => {
+    add_attachment => ['data'],
+};
+
 use constant READ_ONLY => qw(
     attachments
     comments
@@ -592,6 +596,53 @@ sub legal_values {
     return { values => \@result };
 }
 
+sub add_attachment {
+    my ($self, $params) = validate(@_, 'ids');
+    my $dbh = Bugzilla->dbh;
+
+    Bugzilla->login(LOGIN_REQUIRED);
+    defined $params->{ids}
+        || ThrowCodeError('param_required', { param => 'ids' });
+    defined $params->{data}
+        || ThrowCodeError('param_required', { param => 'data' });
+
+    my @bugs = map { Bugzilla::Bug->check($_) } @{ $params->{ids} };
+    foreach my $bug (@bugs) {
+        Bugzilla->user->can_edit_product($bug->product_id)
+          || ThrowUserError("product_edit_denied", {product => $bug->product});
+    }
+
+    my @created;
+    $dbh->bz_start_transaction();
+    foreach my $bug (@bugs) {
+        my $attachment = Bugzilla::Attachment->create({
+            bug         => $bug,
+            data        => $params->{data},
+            description => $params->{summary},
+            filename    => $params->{file_name},
+            mimetype    => $params->{content_type},
+            ispatch     => $params->{is_patch},
+            isprivate   => $params->{is_private},
+            isurl       => $params->{is_url},
+        });
+        my $comment = $params->{comment} || '';
+        $attachment->bug->add_comment($comment, 
+            { isprivate  => $attachment->isprivate,
+              type       => CMT_ATTACHMENT_CREATED,
+              extra_data => $attachment->id });
+        push(@created, $attachment);
+    }
+    $_->bug->update($_->attached) foreach @created;
+    $dbh->bz_commit_transaction();
+
+    $_->send_changes() foreach @bugs;
+
+    my %attachments = map { $_->id => $self->_attachment_to_hash($_, $params) }
+                          @created;
+
+    return { attachments => \%attachments };
+}
+
 sub add_comment {
     my ($self, $params) = @_;
     
@@ -790,6 +841,7 @@ sub _attachment_to_hash {
         id               => $self->type('int', $attach->id),
         bug_id           => $self->type('int', $attach->bug->id),
         file_name        => $self->type('string', $attach->filename),
+        summary          => $self->type('string', $attach->description),
         description      => $self->type('string', $attach->description),
         content_type     => $self->type('string', $attach->contenttype),
         is_private       => $self->type('int', $attach->isprivate),
@@ -1158,9 +1210,13 @@ C<int> The numeric id of the bug that the attachment is attached to.
 
 C<string> The file name of the attachment.
 
-=item C<description>
+=item C<summary>
+
+C<string> A short string describing the attachment.
 
-C<string> The description for the attachment.
+Also returned as C<description>, for backwards-compatibility with older
+Bugzillas. (However, this backwards-compatibility will go away in Bugzilla
+5.0.)
 
 =item C<content_type>
 
@@ -1220,6 +1276,9 @@ private attachments.
 =item In Bugzilla B<4.0>, the C<attacher> return value was renamed to
 C<creator>.
 
+=item In Bugzilla B<4.0>, the C<description> return value was renamed to
+C<summary>.
+
 =back
 
 =back
@@ -2043,6 +2102,120 @@ method.
 
 =back
 
+
+=item C<add_attachment>
+
+B<UNSTABLE>
+
+=over
+
+=item B<Description>
+
+This allows you to add an attachment to a bug in Bugzilla.
+
+=item B<Params>
+
+=over
+
+=item C<ids>
+
+B<Required> C<array> An array of ints and/or strings--the ids
+or aliases of bugs that you want to add this attachment to.
+The same attachment and comment will be added to all
+these bugs.
+
+=item C<data>
+
+B<Required> C<base64> The content of the attachment.
+
+=item C<file_name>
+
+B<Required> C<string> The "file name" that will be displayed
+in the UI for this attachment.
+
+=item C<summary>
+
+B<Required> C<string> A short string describing the
+attachment.
+
+=item C<content_type>
+
+B<Required> C<string> The MIME type of the attachment, like
+C<text/plain> or C<image/png>.
+
+=item C<comment>
+
+C<string> A comment to add along with this attachment.
+
+=item C<is_patch>
+
+C<boolean> True if Bugzilla should treat this attachment as a patch.
+If you specify this, the C<content_type> should be C<text/plain>.
+(Future versions of Bugzilla will force the C<content_type> setting
+to C<text/plain> for patches and you will not have to specify it manually.)
+
+Defaults to False if not specified.
+
+=item C<is_private>
+
+C<boolean> True if the attachment should be private (restricted
+to the "insidergroup"), False if the attachment should be public.
+
+Defaults to False if not specified.
+
+=item C<is_url>
+
+C<boolean> True if the attachment is just a URL, pointing to data elsewhere.
+If so, the C<data> item should just contain the URL.
+
+Defaults to False if not specified.
+
+=back
+
+=item B<Returns>
+
+A single item C<attachments>, which contains the created
+attachments in the same format as the C<attachments> return
+value from L</attachments>.
+
+=item B<Errors>
+
+This method can throw all the same errors as L</get>, plus:
+
+=over
+
+=item 600 (Attachment Too Large)
+
+You tried to attach a file that was larger than Bugzilla will accept.
+
+=item 601 (Invalid MIME Type)
+
+You specified a C<content_type> argument that was blank, not a valid
+MIME type, or not a MIME type that Bugzilla accepts for attachments.
+
+=item 602 (Illegal URL)
+
+You specified C<is_url> as True, but the data that you attempted
+to attach was not a valid URL.
+
+=item 603 (File Name Not Specified)
+
+You did not specify a valid for the C<file_name> argument.
+
+=item 604 (Summary Required)
+
+You did not specify a value for the C<summary> argument.
+
+=item 605 (URL Attaching Disabled)
+
+You attempted to attach a URL, setting C<is_url> to True,
+but this Bugzilla does not support attaching URLs.
+
+=back
+
+=back
+
+
 =item C<add_comment> 
 
 B<STABLE>
diff --git a/Bugzilla/WebService/Constants.pm b/Bugzilla/WebService/Constants.pm
index 18d152d3f..8a41db951 100644
--- a/Bugzilla/WebService/Constants.pm
+++ b/Bugzilla/WebService/Constants.pm
@@ -121,6 +121,16 @@ use constant WS_ERROR_CODE => {
     user_access_by_id_denied    => 505,
     user_access_by_match_denied => 505,
 
+    # Attachment errors are 600-700.
+    patch_too_large        => 600,
+    local_file_too_large   => 600,
+    file_too_large         => 600,
+    invalid_content_type   => 601,
+    attachment_illegal_url => 602,
+    file_not_specified     => 603,
+    missing_attachment_description => 604,
+    attachment_url_disabled => 605,
+
     # Errors thrown by the WebService itself. The ones that are negative 
     # conform to http://xmlrpc-epi.sourceforge.net/specs/rfc.fault_codes.php
     xmlrpc_invalid_value => -32600,
diff --git a/Bugzilla/WebService/Server/JSONRPC.pm b/Bugzilla/WebService/Server/JSONRPC.pm
index 5ab5e4a7b..3ff875361 100644
--- a/Bugzilla/WebService/Server/JSONRPC.pm
+++ b/Bugzilla/WebService/Server/JSONRPC.pm
@@ -27,9 +27,10 @@ use base qw(JSON::RPC::Server::CGI Bugzilla::WebService::Server);
 use Bugzilla::Error;
 use Bugzilla::WebService::Constants;
 use Bugzilla::WebService::Util qw(taint_data);
-
 use Bugzilla::Util qw(correct_urlbase trim);
 
+use MIME::Base64 qw(decode_base64);
+
 #####################################
 # Public JSON::RPC Method Overrides #
 #####################################
@@ -326,6 +327,12 @@ sub _argument_type_check {
             }
         }
     }
+    my @base64_fields = @{ $pkg->BASE64_FIELDS->{$method} || [] };
+    foreach my $field (@base64_fields) {
+        if (defined $params->{$field}) {
+            $params->{$field} = decode_base64($params->{$field});
+        }
+    }
 
     Bugzilla->input_params($params);
 
@@ -503,6 +510,11 @@ to be fully safe for forward-compatibility with all future versions of
 Bugzilla, it is safest to pass in all times as UTC with the "Z" timezone
 specifier.)
 
+C<base64> fields are strings that have been base64 encoded. Note that
+although normal base64 encoding includes newlines to break up the data,
+newlines within a string are not valid JSON, so you should not insert
+newlines into your base64-encoded string.
+
 All other types are standard JSON types.
 
 =head1 ERRORS