Skip to content

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

Open
Open
MockFileSystem.AddFile() does not update MockFileInfo.Exists#899
Labels
state: needs discussionIssues that need further discussiontype: bugIssues that describe misbehaving functionality

Description

@rcdailey
rcdailey
opened on 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.

Activity

  1. rcdailey
    added
    state: needs discussionIssues that need further discussion
    type: bugIssues that describe misbehaving functionality
    on Oct 25, 2022

  2. siprbaum commented on Oct 25, 2022

    @siprbaum
    siprbaum
    on Oct 25, 2022
    Contributor

    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

  3. rcdailey commented on Oct 25, 2022

    @rcdailey
    rcdailey
    on Oct 25, 2022
    ContributorAuthor

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

  4. rcdailey commented on Oct 25, 2022

    @rcdailey
    rcdailey
    on Oct 25, 2022
    ContributorAuthor

    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!

  5. siprbaum commented on Oct 25, 2022

    @siprbaum
    siprbaum
    on Oct 25, 2022
    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

  6. rcdailey commented on Oct 25, 2022

    @rcdailey
    rcdailey
    on Oct 25, 2022
    ContributorAuthor

    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.

  7. hangy commented on Oct 25, 2022

    @hangy
    hangy
    on Oct 25, 2022
    Contributor

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

  8. rcdailey commented on Oct 25, 2022

    @rcdailey
    rcdailey
    on Oct 25, 2022
    ContributorAuthor

    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.

  9. hangy commented on Oct 25, 2022

    @hangy
    hangy
    on Oct 25, 2022
    Contributor

    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.

  10. hangy commented on Oct 25, 2022

    @hangy
    hangy
    on Oct 25, 2022
    Contributor

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

  11. rcdailey commented on Oct 25, 2022

    @rcdailey
    rcdailey
    on Oct 25, 2022
    ContributorAuthor

    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.

  12. siprbaum commented on Oct 26, 2022

    @siprbaum
    siprbaum
    on Oct 26, 2022
    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

  13. hangy commented on Oct 26, 2022

    @hangy
    hangy
    on Oct 26, 2022
    Contributor

    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

Metadata

Metadata

Assignees

No one assigned

    Labels

    state: needs discussionIssues that need further discussiontype: bugIssues that describe misbehaving functionality

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions