Skip to content

Fix caching conflicts#786

Merged
sebastienros merged 18 commits intomainfrom
sebros/fixcache
Apr 27, 2025
Merged

Fix caching conflicts#786
sebastienros merged 18 commits intomainfrom
sebros/fixcache

Conversation

@sebastienros
Copy link
Copy Markdown
Owner

No description provided.

@lahma
Copy link
Copy Markdown
Collaborator

lahma commented Apr 25, 2025

This is interesting...

@hishamco
Copy link
Copy Markdown
Collaborator

How does it fix the caching conflicts?!!

@sebastienros
Copy link
Copy Markdown
Owner Author

I am iterating on this PR, I got a bug report, so first trying to see if I can repro without changes but different OSes, then I will add a test, and then the fix. Using this PR as the way to run on the oses I don't have (macos).

template = null;

if (_cache.TryGetValue(fileInfo.Name, out var cachedTemplate))
if (_cache.TryGetValue(fileInfo.PhysicalPath, out var cachedTemplate))
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Physical path can be null for in-memory filesystems. I think this will break people unless you require this at some other level.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you use the path relative to the file provider root instead?

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is problematic because IFileInfo is very limited here:

        //
        // Summary:
        //     The name of the file or directory, not including any path.
        string Name { get; }
        //
        // Summary:
        //     The path to the file, including the file name. Return null if the file is not
        //     directly accessible.
        string? PhysicalPath { get; }

}

public bool TryGetTemplate(IFileInfo fileInfo, out IFluidTemplate template)
public bool TryGetTemplate(string subpath, DateTimeOffset lastModified, out IFluidTemplate template)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants