Fix caching conflicts

.github/workflows/pr.yml

@@ -11,7 +11,11 @@ on:
 jobs:
   build:
 
-    runs-on: ubuntu-latest
+    strategy:
+      matrix:
+        os: [ubuntu-latest, macos-latest, windows-latest]
+
+    runs-on: ${{ matrix.os }}
     env:
         DOTNET_NOLOGO: true
         DOTNET_CLI_TELEMETRY_OPTOUT: 1

Fluid.Tests/IncludeStatementTests.cs

@@ -1,14 +1,19 @@
+using Fluid.Ast;
+using Fluid.Parser;
+using Fluid.Tests.Mocks;
+using Fluid.Values;
+using Microsoft.Extensions.FileProviders;
 using System;
 using System.Collections.Generic;
 using System.IO;
 using System.Linq;
+using System.Runtime.InteropServices;
 using System.Text.Encodings.Web;
+using System.Threading;
 using System.Threading.Tasks;
-using Fluid.Ast;
-using Fluid.Parser;
-using Fluid.Tests.Mocks;
-using Fluid.Values;
 using Xunit;
+using Xunit.Abstractions;
+using Xunit.Sdk;
 
 namespace Fluid.Tests
 {
@@ -433,71 +438,168 @@ public void IncludeTag_Caches_Template(bool useExtension)
         [Fact]
         public void IncludeTag_Caches_ParsedTemplate()
         {
-            var templates = "abcdefg".Select(x => new string(x, 10)).ToArray();
+            var templates = new Dictionary<string, string>
+            {
+                ["a.liquid"] = "content1",
+                ["folder/a.liquid"] = "content2",
+                ["folder/b.liquid"] = "content3",
+                ["folder/c.liquid"] = "content4",
+                ["folder/other/d.liquid"] = "content5",
+                ["b.liquid"] = "content6",
+                ["c.liquid"] = "content7",
+                ["d.liquid"] = "content8",
+            };
 
-            var fileProvider = new MockFileProvider();
+            var tempPath = Path.Combine(Path.GetTempPath(), "FluidTests", Path.GetRandomFileName());
+            Directory.CreateDirectory(tempPath);
 
-            foreach (var t in templates)
-            {
-                fileProvider.Add($"{t[0]}.liquid", t);
-            }
+            var fileProvider = new PhysicalFileProvider(tempPath);
+
+            WriteFilesContent(templates, tempPath);
 
-            var fileInfos = templates.Select(x => fileProvider.GetFileInfo($"{x[0]}.liquid")).Cast<MockFileInfo>().ToArray();
+            var fileInfos = templates.ToDictionary(t => t.Key, t => fileProvider.GetFileInfo(t.Key));
 
             var options = new TemplateOptions() { FileProvider = fileProvider, MemberAccessStrategy = UnsafeMemberAccessStrategy.Instance };
             _parser.TryParse("{%- include file -%}", out var template);
 
             // The first time a template is included it will be read from the file provider
-            foreach (var f in fileInfos)
+            foreach (var t in templates)
             {
-                var filename = f.Name;
-
-                Assert.False(f.Accessed);
+                var f = fileProvider.GetFileInfo(t.Key);
 
                 var context = new TemplateContext(options);
-                context.SetValue("file", filename);
+                context.SetValue("file", t.Key);
                 var result = template.Render(context);
 
-                Assert.True(f.Accessed);
-            }
+                Assert.Equal(t.Value, result);
 
-            foreach (var f in fileInfos)
-            {
-                f.Accessed = false;
+                Assert.True(options.TemplateCache.TryGetTemplate(t.Key, f.LastModified, out var cachedTemplate));
             }
 
             // The next time a template is included it should not be accessed from the file provider but cached instead
-            foreach (var f in fileInfos)
+            foreach (var t in templates)
             {
-                var filename = f.Name;
+                var f = fileProvider.GetFileInfo(t.Key);
 
-                Assert.False(f.Accessed);
+                options.TemplateCache.SetTemplate(t.Key, f.LastModified, new MockFluidTemplate(t.Key));
 
                 var context = new TemplateContext(options);
-                context.SetValue("file", filename);
+                context.SetValue("file", t.Key);
                 var result = template.Render(context);
 
-                Assert.False(f.Accessed);
+                Assert.Equal(t.Key, result);
             }
 
-            foreach (var f in fileInfos)
-            {
-                f.LastModified = DateTime.UtcNow;
-            }
+            // Update the files so they are accessed again
+            WriteFilesContent(templates, tempPath);
+
+            Thread.Sleep(1000); // Wait for the file provider to update the last modified date
 
             // If the attributes have changed then the template should be reloaded
-            foreach (var f in fileInfos)
+            foreach (var t in templates)
             {
-                var filename = f.Name;
-
-                Assert.False(f.Accessed);
+                var f = fileProvider.GetFileInfo(t.Key);
 
                 var context = new TemplateContext(options);
-                context.SetValue("file", filename);
+                context.SetValue("file", t.Key);
                 var result = template.Render(context);
 
-                Assert.True(f.Accessed);
+                Assert.Equal(t.Value, result);
             }
+
+            static void WriteFilesContent(Dictionary<string, string> templates, string tempPath)
+            {
+                foreach (var t in templates)
+                {
+                    Directory.CreateDirectory(Path.GetDirectoryName(Path.Combine(tempPath, t.Key)));
+                    File.WriteAllText(Path.Combine(tempPath, t.Key), t.Value);
+                }
+            }
+        }
+
+        [Fact]
+        public void IncludeTag_Caches_DifferentFolders()
+        {
+            var tempPath = Path.Combine(Path.GetTempPath(), "FluidTests", Path.GetRandomFileName());
+            Directory.CreateDirectory(tempPath);
+
+            Directory.CreateDirectory(tempPath + "/this-folder");
+            Directory.CreateDirectory(tempPath + "/this-folder/that-folder");
+
+            var fileProvider = new PhysicalFileProvider(tempPath);
+
+            File.WriteAllText(tempPath + "/this-folder/this_file.liquid", "content1");
+            File.WriteAllText(tempPath + "/this-folder/that-folder/this_file.liquid", "content2");
+
+            var options = new TemplateOptions() { FileProvider = fileProvider };
+            _parser.TryParse("{%- include file -%}", out var template);
+
+            var context = new TemplateContext(options);
+            context.SetValue("file", "this-folder/this_file.liquid");
+
+            Assert.Equal("content1", template.Render(context));
+
+            context.SetValue("file", "this-folder/that-folder/this_file.liquid");
+
+            Assert.Equal("content2", template.Render(context));
+
+            try
+            {
+                Directory.Delete(tempPath, true);
+            }
+            catch
+            {
+                // Ignore any exceptions
+            }
+        }
+
+        [Fact]
+        public void IncludeTag_Caches_HandleFileSystemCasing()
+        {
+            // We can't rely on the OS to detect if the FS is case sensitive or not. c.f. MacOS
+            string file = Path.GetTempPath() + Guid.NewGuid().ToString().ToLower();
+            File.CreateText(file).Close();
+            bool isCaseInsensitiveFilesystem = File.Exists(file.ToUpper());
+            File.Delete(file);
+
+            var tempPath = Path.Combine(Path.GetTempPath(), "FluidTests", Path.GetRandomFileName());
+            Directory.CreateDirectory(tempPath);
+
+            var fileProvider = new PhysicalFileProvider(tempPath);
+
+            File.WriteAllText(tempPath + "/this_file.liquid", "content1");
+            File.WriteAllText(tempPath + "/This_file.liquid", "content2");
+
+            var options = new TemplateOptions() { FileProvider = fileProvider };
+            _parser.TryParse("{%- include file -%}", out var template);
+
+            var context = new TemplateContext(options);
+
+            if (isCaseInsensitiveFilesystem)
+            {
+                // Windows is case insensitive, there should be only one file
+                context.SetValue("file", "this_file.liquid");
+                Assert.Equal("content2", template.Render(context));
+                context.SetValue("file", "THIS_FILE.liquid");
+                Assert.Equal("content2", template.Render(context));
+            }
+            else
+            {
+                // Linux is case sensitive, this should be a new cache entry
+                context.SetValue("file", "this_file.liquid");
+                Assert.Equal("content1", template.Render(context));
+                context.SetValue("file", "This_file.liquid");
+                Assert.Equal("content2", template.Render(context));
+            }
+
+            try
+                {
+                    Directory.Delete(tempPath, true);
+                }
+                catch
+                {
+                    // Ignore any exceptions
+                }
         }
     }
 }

Fluid.Tests/Mocks/MockFileInfo.cs

@@ -11,7 +11,8 @@ public class MockFileInfo : IFileInfo
 
         public MockFileInfo(string name, string content)
         {
-            Name = name;
+            Name = Path.GetFileName(name);
+            PhysicalPath = name.Replace('\\', Path.PathSeparator).Replace('/', Path.PathSeparator);
             Content = content;
             Exists = true;
         }
@@ -28,7 +29,7 @@ public MockFileInfo(string name, string content)
 
         public string Name { get; }
 
-        public string PhysicalPath => null;
+        public string PhysicalPath { get; }
 
         public bool Accessed { get; set; }
 

Fluid.Tests/Mocks/MockFileProvider.cs

@@ -1,4 +1,4 @@
-using Microsoft.Extensions.FileProviders;
+using Microsoft.Extensions.FileProviders;
 using Microsoft.Extensions.Primitives;
 using System;
 using System.Collections.Generic;

Fluid.Tests/Mocks/MockFluidTemplate.cs

@@ -0,0 +1,22 @@
+using System.IO;
+using System.Text.Encodings.Web;
+using System.Threading.Tasks;
+
+namespace Fluid.Tests.Mocks;
+
+public class MockFluidTemplate : IFluidTemplate
+{
+    private readonly string _content;
+
+    public MockFluidTemplate(string content)
+    {
+        _content = content;
+    }
+
+    public ValueTask RenderAsync(TextWriter writer, TextEncoder encoder, TemplateContext context)
+    {
+        writer.Write(_content);
+
+        return ValueTask.CompletedTask;
+    }
+}

Fluid/Ast/IncludeStatement.cs

@@ -39,7 +39,7 @@ public override async ValueTask<Completion> WriteToAsync(TextWriter writer, Text
 
             var fileProvider = context.Options.FileProvider;
 
-            // The file info is requested again to ensure the file hasn't changed and is still existing.
+            // The file info is requested again to ensure the file hasn't changed and or was deleted
 
             var fileInfo = fileProvider.GetFileInfo(relativePath);
 
@@ -48,7 +48,7 @@ public override async ValueTask<Completion> WriteToAsync(TextWriter writer, Text
                 throw new FileNotFoundException(relativePath);
             }
 
-            if (context.Options.TemplateCache == null || !context.Options.TemplateCache.TryGetTemplate(fileInfo, out var template))
+            if (context.Options.TemplateCache == null || !context.Options.TemplateCache.TryGetTemplate(relativePath, fileInfo.LastModified, out var template))
             {
                 var content = "";
 
@@ -63,7 +63,7 @@ public override async ValueTask<Completion> WriteToAsync(TextWriter writer, Text
                     throw new ParseException(errors);
                 }
 
-                context.Options.TemplateCache?.SetTemplate(fileInfo, template);
+                context.Options.TemplateCache?.SetTemplate(relativePath, fileInfo.LastModified, template);
             }
 
             var identifier = System.IO.Path.GetFileNameWithoutExtension(relativePath);

Fluid/Ast/RenderStatement.cs

@@ -49,7 +49,7 @@ public override async ValueTask<Completion> WriteToAsync(TextWriter writer, Text
                 throw new FileNotFoundException(relativePath);
             }
 
-            if (context.Options.TemplateCache == null || !context.Options.TemplateCache.TryGetTemplate(fileInfo, out var template))
+            if (context.Options.TemplateCache == null || !context.Options.TemplateCache.TryGetTemplate(relativePath, fileInfo.LastModified, out var template))
             {
                 var content = "";
 
@@ -64,7 +64,7 @@ public override async ValueTask<Completion> WriteToAsync(TextWriter writer, Text
                     throw new ParseException(errors);
                 }
 
-                context.Options.TemplateCache?.SetTemplate(fileInfo, template);
+                context.Options.TemplateCache?.SetTemplate(relativePath, fileInfo.LastModified, template);
             }
 
             var identifier = System.IO.Path.GetFileNameWithoutExtension(relativePath);

Fluid/DefaultTemplateCache.cs

@@ -1,5 +1,5 @@
-using Microsoft.Extensions.FileProviders;
 using System.Collections.Concurrent;
+using System.Runtime.InteropServices;
 
 namespace Fluid;
 
@@ -10,44 +10,43 @@ namespace Fluid;
 /// </summary>
 sealed class TemplateCache : ITemplateCache
 {
-    record struct CachedTemplate(string Name, DateTimeOffset LastModified, IFluidTemplate Template);
+    private sealed record class TemplateCacheEntry(string Subpath, DateTimeOffset LastModified, IFluidTemplate Template);
 
-    private readonly ConcurrentDictionary<string, CachedTemplate> _cache;
+    private readonly ConcurrentDictionary<string, TemplateCacheEntry> _cache;
 
     public TemplateCache()
     {
-        _cache = new(Environment.OSVersion.Platform == PlatformID.Unix ? StringComparer.Ordinal : StringComparer.OrdinalIgnoreCase);
+        // Use case-insensitive comparison only on Windows. Create a dedicated cache entry in other cases, even
+        // on MacOS when the file system coulb be case-sensitive too.
+
+        _cache = new(RuntimeInformation.IsOSPlatform(OSPlatform.Windows) ? StringComparer.OrdinalIgnoreCase : StringComparer.Ordinal);
     }
 
-    public bool TryGetTemplate(IFileInfo fileInfo, out IFluidTemplate template)
+    public bool TryGetTemplate(string subpath, DateTimeOffset lastModified, out IFluidTemplate template)
     {
-        template = null;
+        template = default;
 
-        if (_cache.TryGetValue(fileInfo.Name, out var cachedTemplate))
+        if (_cache.TryGetValue(subpath, out var templateCacheEntry))
         {
-            if (cachedTemplate.LastModified < fileInfo.LastModified)
+            if (templateCacheEntry.LastModified < lastModified)
             {
-                // The template has been modified, so we need to remove it from the cache
-                _cache.TryRemove(fileInfo.Name, out _);
+                // The template has been modified, so we can remove it from the cache
+                _cache.TryRemove(subpath, out _);
 
                 return false;
             }
             else
             {
-                // Return the cached template if it is still valid
-                template = cachedTemplate.Template;
+                template = templateCacheEntry.Template;
                 return true;
             }
         }
 
         return false;
     }
 
-    public void SetTemplate(IFileInfo fileInfo, IFluidTemplate template)
+    public void SetTemplate(string subpath, DateTimeOffset lastModified, IFluidTemplate template)
     {
-        var cachedTemplate = new CachedTemplate(fileInfo.Name, fileInfo.LastModified, template);
-        _cache[fileInfo.Name] = cachedTemplate;
+        _cache[subpath] = new TemplateCacheEntry(subpath, lastModified, template);
     }
 }
-
-