System.IO.Abstractions icon indicating copy to clipboard operation
System.IO.Abstractions copied to clipboard

MockFileSystem.AddFile() does not update MockFileInfo.Exists

Open rcdailey opened this issue 3 years ago • 12 comments
trafficstars

Similar to #822, except I'm not invoking MockFileInfo.Create(). I basically do this:

var fs = new MockFileSystem();
var existingFile = fs.CurrentDirectory().File("test.json");
fs.AddFile(existingFile, new MockFileData(""));
existingFile.Refresh();
existingFile.Exists;

Without the Refresh(), Exists yields false. Having peaked at the code, I don't really see an easy way to solve this. My first thought is an event subscription between the FileSystem object and any MockFileInfo / MockDirectoryInfo objects created, so that when AddFile is called, it can notify them to set their dirty flags. But I'm sure that creates other issues like circular dependencies, issues during cleanup, etc.

What do you think the best solution is here?

I'm using version 17.2.3.

rcdailey avatar Oct 25 '22 14:10 rcdailey

Edited due to the fix in the code example above

~~From your code example, it is not clear what path is, but I assume it is a FileInfo.~~ IIRC, FileInfo caches the result, so I assume (but I haven't checked) that the behavior you see would also be the same when using a real filesystem.

https://learn.microsoft.com/en-us/dotnet/api/system.io.fileinfo.exists?view=net-6.0#remarks

siprbaum avatar Oct 25 '22 16:10 siprbaum

I apologize, I pieced together an example without actually compiling it. I have corrected it. path was supposed to be existingFile.

Also I don't think there's a 1-to-1 comparison here with a real filesystem because I'm talking about mock-specific behavior. In other words, a real filesystem doesn't have AddFile. If I did existingFile.Create(), that would be closer to what a real filesystem would do and that does work (the issue I linked to addresses that specific scenario).

rcdailey avatar Oct 25 '22 16:10 rcdailey

Maybe a good solution here is to create an overload of AddFile that takes an IFileInfo and in that scenario calls Create() on it? That would solve this issue. This also addresses a peeve of mine I've had for a while. I work exclusively with IFileInfo and IDirectoryInfo instead of strings for file paths. It's a bit cumbersome to specify .FullName before passing it into AddFile() in unit tests. So, two birds one stone!

rcdailey avatar Oct 25 '22 16:10 rcdailey

I am away from my machine, so I cant test, but from what I see, you are creating the FileInfi before the actual file.

To my understanding, this would also happen in the real filesystem. Whenever the FileInfo is created, it caches the results.

Try moving line 2 after you create the file

siprbaum avatar Oct 25 '22 16:10 siprbaum

It caches the results conditionally. There's a dirty flag that gets triggered in many conditions. Again if you could please check the issue I linked in my OP you would see what I'm talking about.

rcdailey avatar Oct 25 '22 18:10 rcdailey

It caches the results conditionally. There's a dirty flag that gets triggered in many conditions.

Any clue when it actually is considered dirty? I believe that deleting or moving a file via a FileInfo object dirties the cache, but even creating a file from the instance with CreateText doesn't force the cached data to be renewed.

static async Task TestIt(bool checkExistenceBeforeCreation)
{
    Console.WriteLine("{0} = {1}", nameof(checkExistenceBeforeCreation), checkExistenceBeforeCreation);

    string fileName = Path.ChangeExtension(Guid.NewGuid().ToString(), ".txt");
    FileInfo initiallyCachedFileInfo = new (fileName);
    FileInfo notInitiallyCachedFileInfo = new (fileName);
    Console.WriteLine("Cached exists: {0}", initiallyCachedFileInfo.Exists);

    FileInfo creatingFileInfo = new (fileName);
    if (checkExistenceBeforeCreation)
    {
        Console.WriteLine("Exists according to creator: {0}", creatingFileInfo.Exists);
    }

    using (StreamWriter w = creatingFileInfo.CreateText())
    {
        await w.WriteLineAsync("foobar").ConfigureAwait(false);
    }

    Console.WriteLine("File {0} was created.", fileName);

    Console.WriteLine("Cached exists: {0}", initiallyCachedFileInfo.Exists);
    Console.WriteLine("Non-cached exists: {0}", notInitiallyCachedFileInfo.Exists);
    Console.WriteLine("Exists according to creator: {0}", creatingFileInfo.Exists);

    FileInfo renewedFileInfo = new (fileName);
    Console.WriteLine("Up-to-date exists: {0}", renewedFileInfo.Exists);

    Console.WriteLine(new string('-', Console.WindowWidth));
}

await TestIt(false).ConfigureAwait(false);
await TestIt(true).ConfigureAwait(false);

Output (note how Exists according to creator differs between both method calls):

checkExistenceBeforeCreation = False
Cached exists: False
File 92091c08-cad8-4712-860a-da7bb60e5348.txt was created.
Cached exists: False
Non-cached exists: True
Exists according to creator: True
Up-to-date exists: True
------------------------------------------------------------------------------------------------------------------------
checkExistenceBeforeCreation = True
Cached exists: False
Exists according to creator: False
File 18a1f3a8-fcc9-477a-9cbe-952a6f6b1aa7.txt was created.
Cached exists: False
Non-cached exists: True
Exists according to creator: False
Up-to-date exists: True
------------------------------------------------------------------------------------------------------------------------

hangy avatar Oct 25 '22 20:10 hangy

There's a lot of gaps for sure. For creation, you have to invoke IFileInfo.Create() in order to set the dirty flag to true, which is evaluated again next time you invoke Exists.

rcdailey avatar Oct 25 '22 20:10 rcdailey

Ouch. System.IO.FileInfo.Create() calls Invalidate(), but System.IO.FileInfo.CreateText() does not. 🤯 (In .NET 6)

There's a lot of gaps for sure. For creation, you have to invoke IFileInfo.Create() in order to set the dirty flag to true, which is evaluated again next time you invoke Exists.

Can't call Create() on an already existing file, because that will replace an existing file, though.

hangy avatar Oct 25 '22 20:10 hangy

Based on dotnet/runtime#34229, the behaviour might actually differ between .NET Framework 4.8 and .NET 6. 🤔

hangy avatar Oct 25 '22 21:10 hangy

The whole thing is a mess for sure. I'm not sure what the right answer is. Ultimately I think the whole system needs to be refactored for consistency and to handle a wider range of corner cases.

rcdailey avatar Oct 25 '22 22:10 rcdailey

To sum it up (at least to my understanding): FileInfo is kind of lazily initialized and caches its data on the first call to its properties. According to the documentation, this is true for FileInfo.Exists, FileInfo.IsReadOnly, FileInfo.Name

Other properties inherited from FileSystemInfo also have specific semantics. FileSystemInfo.Attributes FileSystemInfo.CreationTime and all the other FileSystemInfo.*Time* values use a pre-cached result if the current instance was returned from any of the following DirectoryInfo methods:

To get the latest value, the Refresh method must be called.

To my understanding MockFileInfo.Refresh() need to be called in the above equivalent methods in this library

siprbaum avatar Oct 26 '22 15:10 siprbaum

To get the latest value, the Refresh method must be called.

That this isn't true for all methods/properties from .NET 5 any more. According to dotnet/dotnet-api-docs#4061, the docs still need to be updated.

To my understanding MockFileInfo.Refresh() need to be called in the above equivalent methods in this library

I wonder if/how we want to support the differences between .NET Framework and .NET.

  • .NET 5 invalidates, for example on FileInfo.Create: https://github.com/dotnet/runtime/blob/1350579859435ee18bd530e18022a880ae418132/src/libraries/System.Private.CoreLib/src/System/IO/FileInfo.cs#L103-L108
  • .NET Framework 4.8 doesn't: https://github.com/microsoft/referencesource/blob/dae14279dd0672adead5de00ac8f117dcf74c184/mscorlib/system/io/fileinfo.cs#L320-L322

hangy avatar Oct 26 '22 15:10 hangy