You are viewing a plain text version of this content. The canonical link for it is here.
Posted to server-dev@james.apache.org by "Felix Knecht (JIRA)" <se...@james.apache.org> on 2011/09/09 13:10:08 UTC

[jira] [Commented] (JAMES-1310) Return value of File.[delete|mkdir] is ignored

    [ https://issues.apache.org/jira/browse/JAMES-1310?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13101120#comment-13101120 ] 

Felix Knecht commented on JAMES-1310:
-------------------------------------

I do have some problems to understand how it exactly should behave in case of a non existing file. I try to explain.

Referencing the file org.apache.james.repository.file.AbstractFileRepository

a) Method "File getFile(final String key)" [1] declares throwing an IOException. In fact no code inside the method will ever throw such an exception. Should this IOException indicate that the file doesn't exists?

b) Method "void remove(final String key)" [2] throws a RuntimeException if any exception occurs. The only exception which could be thrown inside the try block is a NullPointerException, which is thrown in a) if the Method "encode(key)" [3] returns null. This is never the case (it contains at least 'File.separator'). The only thing what could happen is that [3] throws a NullPointerException (if 'key' is null). But a) declares only throwing IOException.

Let b) throw an IOException when unable to delete the file (e.g. file does not exist) there's a failing test case [4]. This test case has comment "// removing an unexisting message does not throw/return errors".

My questions:
- Why does a) declares an IOException? IMO this makes only sense if a) makes a test for file existence.
- Do we just ignore the delete failure in case of not existing file to delete or is the test case [4] wrong and should expect an IOException?

[1] https://builds.apache.org/view/G-L/view/James/job/james-server-trunk-site/site/xref/org/apache/james/repository/file/AbstractFileRepository.html#233
[2] https://builds.apache.org/view/G-L/view/James/job/james-server-trunk-site/site/xref/org/apache/james/repository/file/AbstractFileRepository.html#277
[3] https://builds.apache.org/view/G-L/view/James/job/james-server-trunk-site/site/xref/org/apache/james/repository/file/AbstractFileRepository.html#332
[4] https://builds.apache.org/view/G-L/view/James/job/james-server-trunk-site/site/xref-test/org/apache/james/mailrepository/AbstractMailRepositoryTest.html#99 

> Return value of File.[delete|mkdir] is ignored
> ----------------------------------------------
>
>                 Key: JAMES-1310
>                 URL: https://issues.apache.org/jira/browse/JAMES-1310
>             Project: JAMES Server
>          Issue Type: Bug
>    Affects Versions: 3.0-beta3
>         Environment: All
>            Reporter: Felix Knecht
>            Assignee: Felix Knecht
>             Fix For: 3.0-beta4
>
>
> In various places we use java.io.File.[delete|mkdir] without respecting the return value. File.delete e.g. may just return false if the file/directory can't be deleted/created because of any other reason than a SecurityException [1]. 
> In case the return value is false an IOException should be thrown

--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org


Re: [jira] [Commented] (JAMES-1310) Return value of File.[delete|mkdir] is ignored

Posted by Felix Knecht <fe...@apache.org>.
Still not having an idea if the test case is wrong or of no IOException 
should be thrown in this case. Any hints are welcome.

Thanks and regards
Felix

On 09/09/2011 01:10 PM, Felix Knecht (JIRA) wrote:
>
>      [ https://issues.apache.org/jira/browse/JAMES-1310?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13101120#comment-13101120 ]
>
> Felix Knecht commented on JAMES-1310:
> -------------------------------------
>
> I do have some problems to understand how it exactly should behave in case of a non existing file. I try to explain.
>
> Referencing the file org.apache.james.repository.file.AbstractFileRepository
>
> a) Method "File getFile(final String key)" [1] declares throwing an IOException. In fact no code inside the method will ever throw such an exception. Should this IOException indicate that the file doesn't exists?
>
> b) Method "void remove(final String key)" [2] throws a RuntimeException if any exception occurs. The only exception which could be thrown inside the try block is a NullPointerException, which is thrown in a) if the Method "encode(key)" [3] returns null. This is never the case (it contains at least 'File.separator'). The only thing what could happen is that [3] throws a NullPointerException (if 'key' is null). But a) declares only throwing IOException.
>
> Let b) throw an IOException when unable to delete the file (e.g. file does not exist) there's a failing test case [4]. This test case has comment "// removing an unexisting message does not throw/return errors".
>
> My questions:
> - Why does a) declares an IOException? IMO this makes only sense if a) makes a test for file existence.
> - Do we just ignore the delete failure in case of not existing file to delete or is the test case [4] wrong and should expect an IOException?
>
> [1] https://builds.apache.org/view/G-L/view/James/job/james-server-trunk-site/site/xref/org/apache/james/repository/file/AbstractFileRepository.html#233
> [2] https://builds.apache.org/view/G-L/view/James/job/james-server-trunk-site/site/xref/org/apache/james/repository/file/AbstractFileRepository.html#277
> [3] https://builds.apache.org/view/G-L/view/James/job/james-server-trunk-site/site/xref/org/apache/james/repository/file/AbstractFileRepository.html#332
> [4] https://builds.apache.org/view/G-L/view/James/job/james-server-trunk-site/site/xref-test/org/apache/james/mailrepository/AbstractMailRepositoryTest.html#99
>
>> Return value of File.[delete|mkdir] is ignored
>> ----------------------------------------------
>>
>>                  Key: JAMES-1310
>>                  URL: https://issues.apache.org/jira/browse/JAMES-1310
>>              Project: JAMES Server
>>           Issue Type: Bug
>>     Affects Versions: 3.0-beta3
>>          Environment: All
>>             Reporter: Felix Knecht
>>             Assignee: Felix Knecht
>>              Fix For: 3.0-beta4
>>
>>
>> In various places we use java.io.File.[delete|mkdir] without respecting the return value. File.delete e.g. may just return false if the file/directory can't be deleted/created because of any other reason than a SecurityException [1].
>> In case the return value is false an IOException should be thrown
>
> --
> This message is automatically generated by JIRA.
> For more information on JIRA, see: http://www.atlassian.com/software/jira
>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
> For additional commands, e-mail: server-dev-help@james.apache.org
>


---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org