users@jersey.java.net

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

From: Imran M Yousuf <imran_at_smartitengineering.com>
Date: Wed, 25 Mar 2009 14:52:12 +0600

On Wed, Mar 25, 2009 at 11:31 AM, Imran M Yousuf
<imran_at_smartitengineering.com> wrote:
> On Tue, Mar 24, 2009 at 9:25 PM, Paul Sandoz <Paul.Sandoz_at_sun.com> wrote:
>>
>> <snip />
>> Patch one:
>>
>> 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?
>>
>
> I did not want to throw the exception, but I agree that I should have
> done the same thing using name == null checking and will do that and
> re-submit the patch and will also throw the exception if name == null.
>

Now, if name == null it will throw IlleagalArgumentException and the
change reflects in the test cases as well.

>>
>> Patch two:
>>
>> I like the idea of FileDataBodyPart. We should have a corresponding "field"
>> builder method for this.
>>
>
> Hmm, I did not think of the builder, will add it to this patch as well
> and it will take 2 params, 'name' and 'file', what do you think?
>

I could find the usage of the builder and thus skipped it, if you can
add some specification for it I will add them as another patch.

>> 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?
>>
>
> I prefer the second option and would have the current mechanism as the
> default, will add a micro API for this in this patch. I prefer no
> dependency of SE 6 libs and also the later option will provide greater
> extensibility.
>

Implemented the second option.

>>
>> Patch three & four
>>
>> Looks good!
>>
>
> Cool!
>
> I will rewrite the patches tonight and will submit them again
> tomorrow. Thank you for your detailed review.
>

All the patches are added to the issue as well as in this email.
Please let me know if any other additions are necessary.

Thank you,

- Imran

> Best regards,
>
> - Imran
>
>> Paul.
>>
>> [1]
>> http://java.sun.com/javase/6/docs/api/javax/activation/MimetypesFileTypeMap.html
>>
>>
>>> Thanks,
>>>
>>> Imran
>>>
>>>> Paul.
>>>>
>



-- 
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