Repository navigation
FileInfo.Delete() does not update .Exists #31425
Description
FileInfo.Delete() does not update .Exists
So I just noticed:
// this file exists
var fi = new FileInfo(@"S:\Ome\File.path");
// here fi.Exists == true
fi.Delete();
// still fi.Exists == true
fi.Refresh();
// finally fi.Exists == falseThe code I wrote assumed Delete() changes .Exists and it was failing very unexpectedly. Checked the actual source code and found Delete() is implemented as:
public override void Delete() => FileSystem.DeleteFile(FullPath);instead of:
public override void Delete()
{
FileSystem.DeleteFile(FullPath);
Invalidate();
}Why is this simple operation not updating .Exists if it succeeds? On the other hand MoveTo() changes the fi object completely updating to the new path. So if you do a Delete() you need to followup with a Refresh() while a MoveTo() is good (even if it would have been better if Move() returned a new FileInfo with the new details instead of changing the current one).
Is this inconsistency by design or was it just overlooked?
Activity
stephentoub commentedon Nov 7, 2019
https://docs.microsoft.com/en-us/dotnet/api/system.io.fileinfo.exists?view=netframework-4.8#remarks
"When first called, FileInfo calls Refresh and caches information about the file. On subsequent calls, you must call Refresh to get the latest copy of the information."
CodeAngry commentedon Nov 8, 2019
I understand that but shouldn't Delete invalidate things. This is not about .Exists but .Delete. Move invalidates the data.
wfurt commentedon Nov 19, 2019
AFAIK Refresh is expected for compatibility reasons even if not really intuitive. Create() has similar behavior.
cc: @JeremyKuhne
AntonLapounov commentedon Nov 19, 2019
Then we must revert dotnet/corefx#40677 as well. We should either preserve compatibility or fix the issue consistently, not just one method of many.
JeremyKuhne commentedon Mar 3, 2020
I agree that updating the behavior consistently when users explicitly change state is the way to go. We need to document this breaking change, of course.
Jlalond commentedon Mar 4, 2020
@JeremyKuhne you can assign me to this if it's still available
Jlalond commentedon Mar 25, 2020
Hey @JeremyKuhne / @carlossanlop to clarify, we're just calling refresh when we create a FileInfo, or delete, and the same for DirectoryInfo? @wfurt in the referenced issue you mentioned you had a way to more performantly do this other than a full refresh, do you still advise going down that route?
wfurt commentedon Mar 25, 2020
I would suggest to call
Invalidate()as proposed above @Jlalond . If you doRefresh()we will be forced to do work immediately. WithInvalidate()we would do work only if somebody asks again. I would expect that for example most developers assume file deleted afterFileInfo.Delete()if it does not throw. I see no reason to do fstat just to verify that.Jlalond commentedon Mar 25, 2020
That's fair @wfurt, but should we call refresh upon create then?
wfurt commentedon Mar 25, 2020
Yes, I think so. We should be consistent and as @JeremyKuhne suggested we should update state for explicit actions. It seems like dotnet/corefx#40677 did it only when creating directories.
I'm not sure how should we handle documentation @carlossanlop
carlossanlop commentedon Mar 25, 2020
I can take care of the documentation after this change is made. I also have to address dotnet/corefx#40677 as you requested.
Jlalond commentedon Mar 25, 2020
Alright so refresh upon create and invalidate on delete. Thanks everyone
wfurt commentedon Mar 25, 2020
dotnet/corefx#40677 use
Invalidate()forCreate(). I think same logic can apply for creation. One can assume the object exist ifCreate()does not throw. We can defer work unless somebody explicitly asks.andrew-boyarshin commentedon Jun 19, 2020
Is this already fixed in #34229? If so, milestone change is incorrect and this issue should be closed.
danmoseley commentedon Jun 19, 2020
Thank you yes @carlossanlop this it's fixed I think