On Wed, Mar 25, 2009 at 3:20 PM, Paul Sandoz <Paul.Sandoz_at_sun.com> wrote:
> Hi Imran,
>
> Looks good. Thanks very much for spending the time on this.
>
> I will integrate this week.
>
Thanks a lot, looking forward to the integration :). I will work on
FormDataParam sample next week :) provided I get time.
Best regards,
Imran
> Paul.
>
> On Mar 25, 2009, at 9:52 AM, Imran M Yousuf wrote:
>
>> 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
>>
>> <0001-Fix-content-disposition-construction-with-null-name.patch><0002-Add-convenience-form-body-part-extension-for-java.io.patch><0003-Fix-Date-to-String-conversion-in-Content-Disposition.patch><0004-Add-test-case-for-ContentDisposition-and-FormDataCon.patch>---------------------------------------------------------------------
>> To unsubscribe, e-mail: users-unsubscribe_at_jersey.dev.java.net
>> For additional commands, e-mail: users-help_at_jersey.dev.java.net
>
>
> ---------------------------------------------------------------------
> 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