Skip to content

Commit 9fa20a5

Browse files
sebastienrossebastienros
authored
Fix caching conflicts (#786)
1 parent 1b2d874 commit 9fa20a5

9 files changed

+206
-67
lines changed

‎.github/workflows/pr.yml

+5-1
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,11 @@ on:
1111
jobs:
1212
build:
1313

14-
runs-on: ubuntu-latest
14+
strategy:
15+
matrix:
16+
os: [ubuntu-latest, macos-latest, windows-latest]
17+
18+
runs-on: ${{ matrix.os }}
1519
env:
1620
DOTNET_NOLOGO: true
1721
DOTNET_CLI_TELEMETRY_OPTOUT: 1

‎Fluid.Tests/IncludeStatementTests.cs

+138-36
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,19 @@
1+
using Fluid.Ast;
2+
using Fluid.Parser;
3+
using Fluid.Tests.Mocks;
4+
using Fluid.Values;
5+
using Microsoft.Extensions.FileProviders;
16
using System;
27
using System.Collections.Generic;
38
using System.IO;
49
using System.Linq;
10+
using System.Runtime.InteropServices;
511
using System.Text.Encodings.Web;
12+
using System.Threading;
613
using System.Threading.Tasks;
7-
using Fluid.Ast;
8-
using Fluid.Parser;
9-
using Fluid.Tests.Mocks;
10-
using Fluid.Values;
1114
using Xunit;
15+
using Xunit.Abstractions;
16+
using Xunit.Sdk;
1217

1318
namespace Fluid.Tests
1419
{
@@ -433,71 +438,168 @@ public void IncludeTag_Caches_Template(bool useExtension)
433438
[Fact]
434439
public void IncludeTag_Caches_ParsedTemplate()
435440
{
436-
var templates = "abcdefg".Select(x => new string(x, 10)).ToArray();
441+
var templates = new Dictionary<string, string>
442+
{
443+
["a.liquid"] = "content1",
444+
["folder/a.liquid"] = "content2",
445+
["folder/b.liquid"] = "content3",
446+
["folder/c.liquid"] = "content4",
447+
["folder/other/d.liquid"] = "content5",
448+
["b.liquid"] = "content6",
449+
["c.liquid"] = "content7",
450+
["d.liquid"] = "content8",
451+
};
437452

438-
var fileProvider = new MockFileProvider();
453+
var tempPath = Path.Combine(Path.GetTempPath(), "FluidTests", Path.GetRandomFileName());
454+
Directory.CreateDirectory(tempPath);
439455

440-
foreach (var t in templates)
441-
{
442-
fileProvider.Add($"{t[0]}.liquid", t);
443-
}
456+
var fileProvider = new PhysicalFileProvider(tempPath);
457+
458+
WriteFilesContent(templates, tempPath);
444459

445-
var fileInfos = templates.Select(x => fileProvider.GetFileInfo($"{x[0]}.liquid")).Cast<MockFileInfo>().ToArray();
460+
var fileInfos = templates.ToDictionary(t => t.Key, t => fileProvider.GetFileInfo(t.Key));
446461

447462
var options = new TemplateOptions() { FileProvider = fileProvider, MemberAccessStrategy = UnsafeMemberAccessStrategy.Instance };
448463
_parser.TryParse("{%- include file -%}", out var template);
449464

450465
// The first time a template is included it will be read from the file provider
451-
foreach (var f in fileInfos)
466+
foreach (var t in templates)
452467
{
453-
var filename = f.Name;
454-
455-
Assert.False(f.Accessed);
468+
var f = fileProvider.GetFileInfo(t.Key);
456469

457470
var context = new TemplateContext(options);
458-
context.SetValue("file", filename);
471+
context.SetValue("file", t.Key);
459472
var result = template.Render(context);
460473

461-
Assert.True(f.Accessed);
462-
}
474+
Assert.Equal(t.Value, result);
463475

464-
foreach (var f in fileInfos)
465-
{
466-
f.Accessed = false;
476+
Assert.True(options.TemplateCache.TryGetTemplate(t.Key, f.LastModified, out var cachedTemplate));
467477
}
468478

469479
// The next time a template is included it should not be accessed from the file provider but cached instead
470-
foreach (var f in fileInfos)
480+
foreach (var t in templates)
471481
{
472-
var filename = f.Name;
482+
var f = fileProvider.GetFileInfo(t.Key);
473483

474-
Assert.False(f.Accessed);
484+
options.TemplateCache.SetTemplate(t.Key, f.LastModified, new MockFluidTemplate(t.Key));
475485

476486
var context = new TemplateContext(options);
477-
context.SetValue("file", filename);
487+
context.SetValue("file", t.Key);
478488
var result = template.Render(context);
479489

480-
Assert.False(f.Accessed);
490+
Assert.Equal(t.Key, result);
481491
}
482492

483-
foreach (var f in fileInfos)
484-
{
485-
f.LastModified = DateTime.UtcNow;
486-
}
493+
// Update the files so they are accessed again
494+
WriteFilesContent(templates, tempPath);
495+
496+
Thread.Sleep(1000); // Wait for the file provider to update the last modified date
487497

488498
// If the attributes have changed then the template should be reloaded
489-
foreach (var f in fileInfos)
499+
foreach (var t in templates)
490500
{
491-
var filename = f.Name;
492-
493-
Assert.False(f.Accessed);
501+
var f = fileProvider.GetFileInfo(t.Key);
494502

495503
var context = new TemplateContext(options);
496-
context.SetValue("file", filename);
504+
context.SetValue("file", t.Key);
497505
var result = template.Render(context);
498506

499-
Assert.True(f.Accessed);
507+
Assert.Equal(t.Value, result);
500508
}
509+
510+
static void WriteFilesContent(Dictionary<string, string> templates, string tempPath)
511+
{
512+
foreach (var t in templates)
513+
{
514+
Directory.CreateDirectory(Path.GetDirectoryName(Path.Combine(tempPath, t.Key)));
515+
File.WriteAllText(Path.Combine(tempPath, t.Key), t.Value);
516+
}
517+
}
518+
}
519+
520+
[Fact]
521+
public void IncludeTag_Caches_DifferentFolders()
522+
{
523+
var tempPath = Path.Combine(Path.GetTempPath(), "FluidTests", Path.GetRandomFileName());
524+
Directory.CreateDirectory(tempPath);
525+
526+
Directory.CreateDirectory(tempPath + "/this-folder");
527+
Directory.CreateDirectory(tempPath + "/this-folder/that-folder");
528+
529+
var fileProvider = new PhysicalFileProvider(tempPath);
530+
531+
File.WriteAllText(tempPath + "/this-folder/this_file.liquid", "content1");
532+
File.WriteAllText(tempPath + "/this-folder/that-folder/this_file.liquid", "content2");
533+
534+
var options = new TemplateOptions() { FileProvider = fileProvider };
535+
_parser.TryParse("{%- include file -%}", out var template);
536+
537+
var context = new TemplateContext(options);
538+
context.SetValue("file", "this-folder/this_file.liquid");
539+
540+
Assert.Equal("content1", template.Render(context));
541+
542+
context.SetValue("file", "this-folder/that-folder/this_file.liquid");
543+
544+
Assert.Equal("content2", template.Render(context));
545+
546+
try
547+
{
548+
Directory.Delete(tempPath, true);
549+
}
550+
catch
551+
{
552+
// Ignore any exceptions
553+
}
554+
}
555+
556+
[Fact]
557+
public void IncludeTag_Caches_HandleFileSystemCasing()
558+
{
559+
// We can't rely on the OS to detect if the FS is case sensitive or not. c.f. MacOS
560+
string file = Path.GetTempPath() + Guid.NewGuid().ToString().ToLower();
561+
File.CreateText(file).Close();
562+
bool isCaseInsensitiveFilesystem = File.Exists(file.ToUpper());
563+
File.Delete(file);
564+
565+
var tempPath = Path.Combine(Path.GetTempPath(), "FluidTests", Path.GetRandomFileName());
566+
Directory.CreateDirectory(tempPath);
567+
568+
var fileProvider = new PhysicalFileProvider(tempPath);
569+
570+
File.WriteAllText(tempPath + "/this_file.liquid", "content1");
571+
File.WriteAllText(tempPath + "/This_file.liquid", "content2");
572+
573+
var options = new TemplateOptions() { FileProvider = fileProvider };
574+
_parser.TryParse("{%- include file -%}", out var template);
575+
576+
var context = new TemplateContext(options);
577+
578+
if (isCaseInsensitiveFilesystem)
579+
{
580+
// Windows is case insensitive, there should be only one file
581+
context.SetValue("file", "this_file.liquid");
582+
Assert.Equal("content2", template.Render(context));
583+
context.SetValue("file", "THIS_FILE.liquid");
584+
Assert.Equal("content2", template.Render(context));
585+
}
586+
else
587+
{
588+
// Linux is case sensitive, this should be a new cache entry
589+
context.SetValue("file", "this_file.liquid");
590+
Assert.Equal("content1", template.Render(context));
591+
context.SetValue("file", "This_file.liquid");
592+
Assert.Equal("content2", template.Render(context));
593+
}
594+
595+
try
596+
{
597+
Directory.Delete(tempPath, true);
598+
}
599+
catch
600+
{
601+
// Ignore any exceptions
602+
}
501603
}
502604
}
503605
}

‎Fluid.Tests/Mocks/MockFileInfo.cs

+3-2
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,8 @@ public class MockFileInfo : IFileInfo
1111

1212
public MockFileInfo(string name, string content)
1313
{
14-
Name = name;
14+
Name = Path.GetFileName(name);
15+
PhysicalPath = name.Replace('\\', Path.PathSeparator).Replace('/', Path.PathSeparator);
1516
Content = content;
1617
Exists = true;
1718
}
@@ -28,7 +29,7 @@ public MockFileInfo(string name, string content)
2829

2930
public string Name { get; }
3031

31-
public string PhysicalPath => null;
32+
public string PhysicalPath { get; }
3233

3334
public bool Accessed { get; set; }
3435

‎Fluid.Tests/Mocks/MockFileProvider.cs

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
using Microsoft.Extensions.FileProviders;
1+
using Microsoft.Extensions.FileProviders;
22
using Microsoft.Extensions.Primitives;
33
using System;
44
using System.Collections.Generic;

‎Fluid.Tests/Mocks/MockFluidTemplate.cs

+22
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
using System.IO;
2+
using System.Text.Encodings.Web;
3+
using System.Threading.Tasks;
4+
5+
namespace Fluid.Tests.Mocks;
6+
7+
public class MockFluidTemplate : IFluidTemplate
8+
{
9+
private readonly string _content;
10+
11+
public MockFluidTemplate(string content)
12+
{
13+
_content = content;
14+
}
15+
16+
public ValueTask RenderAsync(TextWriter writer, TextEncoder encoder, TemplateContext context)
17+
{
18+
writer.Write(_content);
19+
20+
return ValueTask.CompletedTask;
21+
}
22+
}

‎Fluid/Ast/IncludeStatement.cs

+3-3
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ public override async ValueTask<Completion> WriteToAsync(TextWriter writer, Text
3939

4040
var fileProvider = context.Options.FileProvider;
4141

42-
// The file info is requested again to ensure the file hasn't changed and is still existing.
42+
// The file info is requested again to ensure the file hasn't changed and or was deleted
4343

4444
var fileInfo = fileProvider.GetFileInfo(relativePath);
4545

@@ -48,7 +48,7 @@ public override async ValueTask<Completion> WriteToAsync(TextWriter writer, Text
4848
throw new FileNotFoundException(relativePath);
4949
}
5050

51-
if (context.Options.TemplateCache == null || !context.Options.TemplateCache.TryGetTemplate(fileInfo, out var template))
51+
if (context.Options.TemplateCache == null || !context.Options.TemplateCache.TryGetTemplate(relativePath, fileInfo.LastModified, out var template))
5252
{
5353
var content = "";
5454

@@ -63,7 +63,7 @@ public override async ValueTask<Completion> WriteToAsync(TextWriter writer, Text
6363
throw new ParseException(errors);
6464
}
6565

66-
context.Options.TemplateCache?.SetTemplate(fileInfo, template);
66+
context.Options.TemplateCache?.SetTemplate(relativePath, fileInfo.LastModified, template);
6767
}
6868

6969
var identifier = System.IO.Path.GetFileNameWithoutExtension(relativePath);

‎Fluid/Ast/RenderStatement.cs

+2-2
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ public override async ValueTask<Completion> WriteToAsync(TextWriter writer, Text
4949
throw new FileNotFoundException(relativePath);
5050
}
5151

52-
if (context.Options.TemplateCache == null || !context.Options.TemplateCache.TryGetTemplate(fileInfo, out var template))
52+
if (context.Options.TemplateCache == null || !context.Options.TemplateCache.TryGetTemplate(relativePath, fileInfo.LastModified, out var template))
5353
{
5454
var content = "";
5555

@@ -64,7 +64,7 @@ public override async ValueTask<Completion> WriteToAsync(TextWriter writer, Text
6464
throw new ParseException(errors);
6565
}
6666

67-
context.Options.TemplateCache?.SetTemplate(fileInfo, template);
67+
context.Options.TemplateCache?.SetTemplate(relativePath, fileInfo.LastModified, template);
6868
}
6969

7070
var identifier = System.IO.Path.GetFileNameWithoutExtension(relativePath);

‎Fluid/DefaultTemplateCache.cs

+16-17
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
1-
using Microsoft.Extensions.FileProviders;
21
using System.Collections.Concurrent;
2+
using System.Runtime.InteropServices;
33

44
namespace Fluid;
55

@@ -10,44 +10,43 @@ namespace Fluid;
1010
/// </summary>
1111
sealed class TemplateCache : ITemplateCache
1212
{
13-
record struct CachedTemplate(string Name, DateTimeOffset LastModified, IFluidTemplate Template);
13+
private sealed record class TemplateCacheEntry(string Subpath, DateTimeOffset LastModified, IFluidTemplate Template);
1414

15-
private readonly ConcurrentDictionary<string, CachedTemplate> _cache;
15+
private readonly ConcurrentDictionary<string, TemplateCacheEntry> _cache;
1616

1717
public TemplateCache()
1818
{
19-
_cache = new(Environment.OSVersion.Platform == PlatformID.Unix ? StringComparer.Ordinal : StringComparer.OrdinalIgnoreCase);
19+
// Use case-insensitive comparison only on Windows. Create a dedicated cache entry in other cases, even
20+
// on MacOS when the file system coulb be case-sensitive too.
21+
22+
_cache = new(RuntimeInformation.IsOSPlatform(OSPlatform.Windows) ? StringComparer.OrdinalIgnoreCase : StringComparer.Ordinal);
2023
}
2124

22-
public bool TryGetTemplate(IFileInfo fileInfo, out IFluidTemplate template)
25+
public bool TryGetTemplate(string subpath, DateTimeOffset lastModified, out IFluidTemplate template)
2326
{
24-
template = null;
27+
template = default;
2528

26-
if (_cache.TryGetValue(fileInfo.Name, out var cachedTemplate))
29+
if (_cache.TryGetValue(subpath, out var templateCacheEntry))
2730
{
28-
if (cachedTemplate.LastModified < fileInfo.LastModified)
31+
if (templateCacheEntry.LastModified < lastModified)
2932
{
30-
// The template has been modified, so we need to remove it from the cache
31-
_cache.TryRemove(fileInfo.Name, out _);
33+
// The template has been modified, so we can remove it from the cache
34+
_cache.TryRemove(subpath, out _);
3235

3336
return false;
3437
}
3538
else
3639
{
37-
// Return the cached template if it is still valid
38-
template = cachedTemplate.Template;
40+
template = templateCacheEntry.Template;
3941
return true;
4042
}
4143
}
4244

4345
return false;
4446
}
4547

46-
public void SetTemplate(IFileInfo fileInfo, IFluidTemplate template)
48+
public void SetTemplate(string subpath, DateTimeOffset lastModified, IFluidTemplate template)
4749
{
48-
var cachedTemplate = new CachedTemplate(fileInfo.Name, fileInfo.LastModified, template);
49-
_cache[fileInfo.Name] = cachedTemplate;
50+
_cache[subpath] = new TemplateCacheEntry(subpath, lastModified, template);
5051
}
5152
}
52-
53-

0 commit comments

Comments
 (0)