users@jersey.java.net

Re: [Jersey] [RFC] Patches regarding file body part - Re: [Jersey] Uploading files using Jersey client

From: Paul Sandoz <Paul.Sandoz_at_Sun.COM>
Date: Tue, 24 Mar 2009 16:25:23 +0100

On Mar 24, 2009, at 9:55 AM, Imran M Yousuf wrote:

> On Tue, Mar 24, 2009 at 2:51 PM, Paul Sandoz <Paul.Sandoz_at_sun.com>
> wrote:
>>
>> On Mar 24, 2009, at 4:13 AM, Imran M Yousuf wrote:
>>>
>>> The attached patches are also attached to the following issue -
>>> https://jersey.dev.java.net/issues/show_bug.cgi?id=246
>>>
>>> Thank you,
>>>
>>
>> Great, i will send an email today reviewing the patch.
>>
>
> Can't wait for it :), I am sooo excited :).
>

Patch one:

diff --git a/contribs/jersey-multipart/src/main/java/com/sun/jersey/
multipart/FormDataBodyPart.java b/contribs/jersey-multipart/src/main/
java/com/sun/jersey/multipart/FormDataBodyPart.java
index 3b9317b..75d7bcd 100644
--- a/contribs/jersey-multipart/src/main/java/com/sun/jersey/multipart/
FormDataBodyPart.java
+++ b/contribs/jersey-multipart/src/main/java/com/sun/jersey/multipart/
FormDataBodyPart.java
@@ -241,7 +241,15 @@ public class FormDataBodyPart extends BodyPart {
       */
      public void setName(String name) {
          if (getFormDataContentDisposition() == null) {
-
super
.setContentDisposition(FormDataContentDisposition.name(name).build());
+ FormDataContentDisposition contentDisposition;
+ try {
+ contentDisposition =
FormDataContentDisposition.name(name)
+ .build();
+ }
+ catch(Exception ex) {
+ contentDisposition = null;
+ }
+ super.setContentDisposition(contentDisposition);


Would it not be simpler to check if name == null and throw an
IllegalArgumentException in *either* case of there being an existing
content disposition value or not? rather than silently consuming the
exception from the builder?


Patch two:

I like the idea of FileDataBodyPart. We should have a corresponding
"field" builder method for this.

Note that there is extensible support in SE 6 for guessing the media
type from a file [1], and i think those using SE 6 should be able to
leverage that. So even though we depend on SE 5 and above i am a bit
reluctant to include such functionality as it is rather limited in the
sense of extensibility of the media types and i would prefer not to
duplicate the functionality in SE 6.

Perhaps instead developers should be allowed to extend this class with
the ability to guess the media type?


Patch three & four

Looks good!

Paul.

[1] http://java.sun.com/javase/6/docs/api/javax/activation/MimetypesFileTypeMap.html


> Thanks,
>
> Imran
>
>> Paul.
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: users-unsubscribe_at_jersey.dev.java.net
>> For additional commands, e-mail: users-help_at_jersey.dev.java.net
>>
>>
>
>
>
> --
> Imran M Yousuf
> Entrepreneur & Software Engineer
> Smart IT Engineering
> Dhaka, Bangladesh
> Email: imran_at_smartitengineering.com
> Blog: http://imyousuf-tech.blogs.smartitengineering.com/
> Mobile: +880-1711402557
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: users-unsubscribe_at_jersey.dev.java.net
> For additional commands, e-mail: users-help_at_jersey.dev.java.net
>