diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml index c9318305..bef7ba97 100644 --- a/.github/workflows/pr.yml +++ b/.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 diff --git a/Fluid.Tests/IncludeStatementTests.cs b/Fluid.Tests/IncludeStatementTests.cs index d88ef5c0..52ccfbf8 100644 --- a/Fluid.Tests/IncludeStatementTests.cs +++ b/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 + { + ["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().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 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 + } } } } diff --git a/Fluid.Tests/Mocks/MockFileInfo.cs b/Fluid.Tests/Mocks/MockFileInfo.cs index 31dcdc79..feca3a71 100644 --- a/Fluid.Tests/Mocks/MockFileInfo.cs +++ b/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; } diff --git a/Fluid.Tests/Mocks/MockFileProvider.cs b/Fluid.Tests/Mocks/MockFileProvider.cs index 07406357..ba306568 100644 --- a/Fluid.Tests/Mocks/MockFileProvider.cs +++ b/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; diff --git a/Fluid.Tests/Mocks/MockFluidTemplate.cs b/Fluid.Tests/Mocks/MockFluidTemplate.cs new file mode 100644 index 00000000..d9069594 --- /dev/null +++ b/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; + } +} diff --git a/Fluid/Ast/IncludeStatement.cs b/Fluid/Ast/IncludeStatement.cs index d5e0f2c7..9c3fadc0 100644 --- a/Fluid/Ast/IncludeStatement.cs +++ b/Fluid/Ast/IncludeStatement.cs @@ -39,7 +39,7 @@ public override async ValueTask 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 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 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); diff --git a/Fluid/Ast/RenderStatement.cs b/Fluid/Ast/RenderStatement.cs index 08730ee1..9cc44212 100644 --- a/Fluid/Ast/RenderStatement.cs +++ b/Fluid/Ast/RenderStatement.cs @@ -49,7 +49,7 @@ public override async ValueTask 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 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); diff --git a/Fluid/DefaultTemplateCache.cs b/Fluid/DefaultTemplateCache.cs index 8c28786d..d35bada1 100644 --- a/Fluid/DefaultTemplateCache.cs +++ b/Fluid/DefaultTemplateCache.cs @@ -1,5 +1,5 @@ -using Microsoft.Extensions.FileProviders; using System.Collections.Concurrent; +using System.Runtime.InteropServices; namespace Fluid; @@ -10,32 +10,34 @@ namespace Fluid; /// 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 _cache; + private readonly ConcurrentDictionary _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; } } @@ -43,11 +45,8 @@ public bool TryGetTemplate(IFileInfo fileInfo, out IFluidTemplate template) 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); } } - - diff --git a/Fluid/ITemplateCache.cs b/Fluid/ITemplateCache.cs index ad63ecf0..5b94f7a0 100644 --- a/Fluid/ITemplateCache.cs +++ b/Fluid/ITemplateCache.cs @@ -1,13 +1,24 @@ -using Microsoft.Extensions.FileProviders; - namespace Fluid; /// -/// Provides methods to retrieve and store templates using a . +/// Interface for caching parsed templates in memory. /// public interface ITemplateCache { - bool TryGetTemplate(IFileInfo fileInfo, out IFluidTemplate template); + /// + /// Attempts to retrieve a cached template based on the provided subpath. + /// + /// The relative path that identifies the file. + /// The last modified time of the template file. + /// The cached template if found. + /// True if the template is found in the cache; otherwise, false. + bool TryGetTemplate(string subpath, DateTimeOffset lastModified, out IFluidTemplate template); - void SetTemplate(IFileInfo fileInfo, IFluidTemplate template); + /// + /// Stores a template in the cache with the specified subpath as the key. + /// + /// The relative path that identifies the file. + /// The last modified time of the template file. + /// The template to store in the cache. + void SetTemplate(string subpath, DateTimeOffset lastModified, IFluidTemplate template); }