FileUtilities#remove(): do nothing if file does not exist #2159
Conversation
|
What would happen if the to be deleted file is a directory? |
|
@madoar Probably the same as removing the file because PhoenicisOrg/scripts#1124 is still open |
|
If the directory doesn't exist, nothing. Otherwise: FileUtils.deleteDirectory(fileToDelete); |
|
Which order is wrong? |
| @@ -162,7 +166,8 @@ public void remove(String path) throws IOException { | |||
| final File fileToDelete = new File(path); | |||
|
|
|||
| if (!fileToDelete.exists()) { | |||
| throw new IllegalArgumentException(String.format("Path \"%s\" does not exist", path)); | |||
| LOGGER.debug(String.format("Cannot remove file or directory: path \"%s\" does not exist", path)); | |||
| return; | |||
| } | |||
|
|
|||
| assertInDirectory(fileToDelete); | |||
There was a problem hiding this comment.
I think the assertion should then be moved before if (!fileToDelete.exists()) { .. }
There was a problem hiding this comment.
Why should it matter?
There was a problem hiding this comment.
The order seems wrong like this. Let us say the script wants to execute remove("/home/<your/home>/important-file"); then I wouldn't expect a debug message that says the file does not exist but an error message that tells me that the script tries to access files it has no access to
There was a problem hiding this comment.
If you want we can handle this in another PR. I just want to talk about this now that it is still fresh on my mind
fixes PhoenicisOrg/scripts#1162
It seems that this behavior was intended anyways because we use