Add a "size" return value to Bug.attachments

RESOLVED FIXED in Bugzilla 4.4

Status

()

P3
enhancement
RESOLVED FIXED
8 years ago
6 years ago

People

(Reporter: konstantin.petrukhnov, Assigned: reed)

Tracking

unspecified
Bugzilla 4.4
Bug Flags:
approval +
testcase +

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Reporter)

Description

8 years ago
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.2.15) Gecko/20110303 Firefox/3.6.15
Build Identifier: Bugzilla 4.0

Bugzilla::Webservice::Bug.attachments size field

Size value is missing from return map. It would be good to have possibility to know size of file, before retrieving data.


Reproducible: Always




Ideally there should be possible to get data by chunks, so memory could be used efficiently, and big files could be transferred. E.g. Bug.attachmentData, and pass limit and offset values to it.

Updated

8 years ago
Severity: minor → enhancement
Status: UNCONFIRMED → NEW
Ever confirmed: true
Priority: -- → P3
Hardware: x86 → All
Summary: Bugzilla::Webservice::Bug.attachments size → Add a "size" return value to Bug.attachments

Updated

8 years ago
Whiteboard: [Good Intro Bug]
(Assignee)

Comment 1

7 years ago
Created attachment 585022 [details] [diff] [review]
patch - v1
Assignee: webservice → reed
Status: NEW → ASSIGNED
Attachment #585022 - Flags: review?(dkl)
Comment on attachment 585022 [details] [diff] [review]
patch - v1

Review of attachment 585022 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good and works as expected. r=dkl
Attachment #585022 - Flags: review?(dkl) → review+

Updated

7 years ago
Target Milestone: --- → Bugzilla 5.0

Comment 3

7 years ago
Comment on attachment 585022 [details] [diff] [review]
patch - v1

Review of attachment 585022 [details] [diff] [review]:
-----------------------------------------------------------------

::: Bugzilla/WebService/Bug.pm
@@ +933,4 @@
>          $item->{'data'} = $self->type('base64', $attach->data);
>      }
>  
> +    if (filter_wants $filters, 'datasize') {

Can we just call this "size"?

@@ +1381,4 @@
>  =item In Bugzilla B<4.2>, the C<is_url> return value was removed
>  (this attribute no longer exists for attachments).
>  
> +=item The C<datasize> return value was added in Bugzilla B<4.2>.

5.0--4.2 is frozen.
(Assignee)

Comment 4

7 years ago
Created attachment 585047 [details] [diff] [review]
patch - v2
Attachment #585022 - Attachment is obsolete: true
Attachment #585047 - Flags: review+
(Assignee)

Updated

7 years ago
Flags: approval?

Comment 5

7 years ago
Comment on attachment 585047 [details] [diff] [review]
patch - v2

>=== modified file 'Bugzilla/WebService/Bug.pm'

>+=item C<size>
>+
>+C<int> The length (in characters) of the attachment.

s/characters/bytes/
(Assignee)

Comment 6

7 years ago
(In reply to Frédéric Buclin from comment #5)
> >+C<int> The length (in characters) of the attachment.
> 
> s/characters/bytes/

Believe it or not, incorrect! http://perldoc.perl.org/functions/length.html

:)
(In reply to Max Kanat-Alexander from comment #3)
> ::: Bugzilla/WebService/Bug.pm
> @@ +933,4 @@
> >          $item->{'data'} = $self->type('base64', $attach->data);
> >      }
> >  
> > +    if (filter_wants $filters, 'datasize') {
> 
> Can we just call this "size"?

I thought of that as well but Attachment.pm already exported it as datasize so though
it would be ok to leave them the same. But I am fine either way as the client will not
know what the internal name is and shorter names are better.

dkl

Comment 8

7 years ago
(In reply to Reed Loden [:reed] (very busy) from comment #6)
> Believe it or not, incorrect! http://perldoc.perl.org/functions/length.html

  No, all attachment data is retrieved as non-UTF8 data, so it is indeed bytes. The documentation should say bytes.

Comment 9

7 years ago
(In reply to David Lawrence [:dkl] from comment #7)
> I thought of that as well but Attachment.pm already exported it as datasize
> so though
> it would be ok to leave them the same. 

  Yeah, in general it's best to assume that our internal names are bad, particularly if they have been around for a long time.
(Assignee)

Comment 10

7 years ago
(In reply to Max Kanat-Alexander from comment #8)
> (In reply to Reed Loden [:reed] (very busy) from comment #6)
> > Believe it or not, incorrect! http://perldoc.perl.org/functions/length.html
> 
>   No, all attachment data is retrieved as non-UTF8 data, so it is indeed
> bytes. The documentation should say bytes.

Then somebody should also fix this existing documentation:
http://mxr.mozilla.org/bugzilla/source/Bugzilla/Attachment.pm#374
(Assignee)

Comment 11

7 years ago
Created attachment 585066 [details] [diff] [review]
patch - v3
Attachment #585047 - Attachment is obsolete: true
Attachment #585066 - Flags: review+

Comment 12

7 years ago
(In reply to Reed Loden [:reed] (very busy) from comment #10)
> Then somebody should also fix this existing documentation:
> http://mxr.mozilla.org/bugzilla/source/Bugzilla/Attachment.pm#374

  Ah yeah, true, that should be fixed.

Comment 13

7 years ago
Comment on attachment 585066 [details] [diff] [review]
patch - v3

Review of attachment 585066 [details] [diff] [review]:
-----------------------------------------------------------------

Looks great, thank you. :-)
Attachment #585066 - Flags: review+

Updated

7 years ago
Flags: approval? → approval+
(Assignee)

Comment 14

7 years ago
Committing to: bzr+ssh://bzr.mozilla.org/bugzilla/trunk/
modified Bugzilla/Attachment.pm
modified Bugzilla/WebService/Bug.pm
Committed revision 8056.
Status: ASSIGNED → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED

Updated

7 years ago
Whiteboard: [Good Intro Bug]

Updated

7 years ago
Keywords: relnote

Updated

7 years ago
Flags: testcase?

Comment 15

6 years ago
Committing to: bzr+ssh://lpsolit%40gmail.com@bzr.mozilla.org/bugzilla/qa/4.4/
modified t/webservice_bug_attachments.t
Committed revision 234.
Flags: testcase? → testcase+

Comment 16

6 years ago
Added to the relnotes for 4.4.
Keywords: relnote
You need to log in before you can comment on or make changes to this bug.