Review Request: JDK-7142921,(fs) Files.probeContentType reports a MIME type of "text/plain" on Ubuntu 11.04

[Dan Xu]

Hi,

I have added a comment to UnixFileSystemProvider.chain and remove its 
"protected" keyword. The new webrev is at, 
http://cr.openjdk.java.net/~dxu/7142921/webrev.01/. Thanks!

-Dan

On Mon 26 Nov 2012 11:29:40 AM PST, Dan Xu wrote:
> Hi Alan,
>
> Thanks for your comments. Please see my answers inline.
>
>
> On 11/26/2012 05:46 AM, Alan Bateman wrote:
>> On 26/11/2012 06:40, Dan Xu wrote:
>>> Hi,
>>>
>>> Please help review the code changes for CR 7142921 and CR 7144997.
>>> The webrev is uploaded to,
>>> http://cr.openjdk.java.net/~dxu/7142921/webrev.00/.
>>>
>>> In the fix, I added two new file type detectors, one is
>>> MimeTypesFileTypeDetector, and the other is
>>> MagicFileTypeDetector.MimeTypesFileTypeDetector is using a file name
>>> extension to look up its MIME type by checking mime.types files,
>>> which can be used in Linux, Solaris, and Mac.MagicFileTypeDetector
>>> is using libmagic to detect a file type in Linux. The library,
>>> libmagic, is provided in the same package of file command, which is
>>> currently only available in Linux platform.
>>>
>>> Thanks,
>>>
>>> -Dan
>> Thanks for taking this on. I don't have time to do a detailed review
>> of the two new FileTypeDetector implementations but just some initial
>> comments on the the other changes:
>>
>> 1. I don't think AbstractFileTypeDetector should be changed to check
>> if the file exists and is a directory, maybe you intended this check
>> to be in the libmagic based FileTypeDetector?
> This is a shortcut for known MIME types for non-exist files and
> directories for all detectors. GnomeFileTypeDetector and
> MagicFileTypeDetector can detect correctly for those special cases.
> But MimeTypesFileTypeDetector may return wrong results. I can move the
> special check into MimeTypesFileTypeDetector if you think it is a
> better way.
>>
>> 2. If we are using /etc/mime.types then we might also have to look at
>> ${user.home}/mime.types as I think that can be used to override the
>> mapping in /etc.
> /etc/mime.types says "Users can add their own types if they wish by
> creating a ".mime.types"  file in their home directory.  Definitions
> included there will take precedence over those listed here." So I try
> to load ${user.home}/.mime.types before /etc/mime.types. And all files
> loaded earlier take precedence over files loaded later.
>
>>
>> 3. It would be good to add a comment to UnixFileSystemProvider.chain.
>> Also if the line length is a problem when you can probably drop
>> protected as the usage is package-private anyway.
> I am going to add the comment and remove "protected".
>>
>> Otherwise the make file updates looks fine. I will get back to you
>> soon on the magic and mimetypes detectors, just can't spend time on
>> it today. The other big thing with this proposal is to agree the
>> priority of the detectors, we also need to consider the
>> diagnosability on each platform (needs to be easy for the user to
>> understand how the file type is determined). Finally, we should
>> double check that /private/etc/apache2/mime.types is the right place
>> to look on Mac.
>>
>> -Alan
>>
>>
> Thanks,
>
> -Dan