Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MockFileSystem.AddFile() does not update MockFileInfo.Exists #899

Open
rcdailey opened this issue Oct 25, 2022 · 12 comments · May be fixed by #900
Open

MockFileSystem.AddFile() does not update MockFileInfo.Exists #899

rcdailey opened this issue Oct 25, 2022 · 12 comments · May be fixed by #900
Labels
state: needs discussion Issues that need further discussion type: bug Issues that describe misbehaving functionality

Comments

@rcdailey
Copy link
Contributor

rcdailey commented Oct 25, 2022

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 rcdailey added state: needs discussion Issues that need further discussion type: bug Issues that describe misbehaving functionality labels Oct 25, 2022
@siprbaum
Copy link
Contributor

siprbaum commented Oct 25, 2022

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

@rcdailey
Copy link
Contributor Author

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
Copy link
Contributor Author

rcdailey commented Oct 25, 2022

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!

@siprbaum
Copy link
Contributor

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

@rcdailey
Copy link
Contributor Author

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.

@hangy
Copy link
Contributor

hangy commented Oct 25, 2022

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
------------------------------------------------------------------------------------------------------------------------

@rcdailey
Copy link
Contributor Author

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.

@hangy
Copy link
Contributor

hangy commented Oct 25, 2022

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
Copy link
Contributor

hangy commented Oct 25, 2022

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

@rcdailey
Copy link
Contributor Author

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.

@hangy hangy linked a pull request Oct 26, 2022 that will close this issue
@siprbaum
Copy link
Contributor

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

@hangy
Copy link
Contributor

hangy commented Oct 26, 2022

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
state: needs discussion Issues that need further discussion type: bug Issues that describe misbehaving functionality
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants