Skip to content

Commit 24a27fc

Browse files
jzabroskijzabroski
authored
Assets respect underlying source files order (#349)
* Assets respect underlying source files order * Add missing file from previous commit * Add logging functionality to internal Asset class * Fix tests to use _assetLogger
1 parent 3d52040 commit 24a27fc

File tree

16 files changed

+195
-101
lines changed

16 files changed

+195
-101
lines changed

‎src/WebOptimizer.Core/Asset.cs‎

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -13,34 +13,34 @@
1313
using Microsoft.Extensions.DependencyInjection;
1414
using Microsoft.Extensions.FileProviders;
1515
using Microsoft.Extensions.FileSystemGlobbing;
16+
using Microsoft.Extensions.Logging;
1617
using Microsoft.Extensions.Primitives;
1718
using Microsoft.Net.Http.Headers;
1819

1920
namespace WebOptimizer
2021
{
2122
internal class Asset : IAsset
2223
{
24+
private readonly ILogger<Asset> _logger;
2325
internal const string PhysicalFilesKey = "PhysicalFiles";
2426
private readonly object _sync = new object();
2527

26-
public Asset(string route, string contentType, IAssetPipeline pipeline, IEnumerable<string> sourceFiles)
27-
: this(route, contentType, sourceFiles)
28-
{ }
29-
30-
public Asset(string route, string contentType, IEnumerable<string> sourceFiles)
28+
public Asset(string route, string contentType, IEnumerable<string> sourceFiles, ILogger<Asset> logger)
3129
{
30+
ArgumentNullException.ThrowIfNull(logger);
3231
Route = route ?? throw new ArgumentNullException(nameof(route));
3332
ContentType = contentType ?? throw new ArgumentNullException(nameof(contentType));
34-
SourceFiles = new HashSet<string>(sourceFiles ?? throw new ArgumentNullException(nameof(sourceFiles)));
33+
SourceFiles = sourceFiles?.ToList() ?? throw new ArgumentNullException(nameof(sourceFiles));
3534
Processors = new List<IProcessor>();
3635
Items = new ConcurrentDictionary<string, object>();
36+
_logger = logger;
3737
}
3838

3939
public string Route { get; private set; }
4040

4141
public IList<string> ExcludeFiles { get; } = new List<string>();
4242

43-
public HashSet<string> SourceFiles { get; }
43+
public IList<string> SourceFiles { get; }
4444

4545
public string ContentType { get; private set; }
4646

@@ -233,7 +233,9 @@ public void TryAddSourceFile(string route)
233233

234234
lock (_sync)
235235
{
236-
if (SourceFiles.Add(cleanRoute) && Items.ContainsKey(PhysicalFilesKey))
236+
if (SourceFiles.Contains(cleanRoute))
237+
_logger.LogSourceFileAlreadyAdded(route, cleanRoute);
238+
if (SourceFiles.Contains(cleanRoute) && Items.ContainsKey(PhysicalFilesKey))
237239
Items.Remove(Asset.PhysicalFilesKey); //remove to calc a new cache key
238240
}
239241
}
@@ -266,7 +268,7 @@ public static IFileProvider GetFileProvider(this IAsset asset, IWebHostEnvironme
266268
? provider.FileProviders.Last()
267269
: env.WebRootFileProvider);
268270
}
269-
271+
270272
/// <summary>
271273
/// Gets the asset file provider. This method works for _content locations in RCL projects.
272274
/// </summary>
@@ -355,4 +357,4 @@ internal static IReadOnlyList<string> GetAllFiles(this IFileProvider provider, s
355357
return files;
356358
}
357359
}
358-
}
360+
}

‎src/WebOptimizer.Core/AssetPipeline.cs‎

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -3,13 +3,17 @@
33
using System.Collections.Generic;
44
using System.Linq;
55
using Microsoft.Extensions.FileSystemGlobbing;
6+
using Microsoft.Extensions.Logging;
67

78
namespace WebOptimizer
89
{
910
internal class AssetPipeline : IAssetPipeline
1011
{
1112
internal ConcurrentDictionary<string, IAsset> _assets = new ConcurrentDictionary<string, IAsset>(StringComparer.OrdinalIgnoreCase);
12-
13+
/// <summary>
14+
/// For use by the Asset constructor only. Do not use for logging messages inside <see cref="AssetPipeline"/>.
15+
/// </summary>
16+
internal ILogger<Asset> _assetLogger;
1317
public IReadOnlyList<IAsset> Assets => _assets.Values.ToList();
1418

1519
public bool TryGetAssetFromRoute(string route, out IAsset asset)
@@ -52,10 +56,10 @@ public bool TryGetAssetFromRoute(string route, out IAsset asset)
5256

5357
if (result.HasMatches)
5458
{
55-
asset = new Asset(cleanRoute, existing.ContentType, this, new[]
59+
asset = new Asset(cleanRoute, existing.ContentType, new[]
5660
{
5761
cleanRoute
58-
});
62+
}, _assetLogger);
5963

6064
foreach (IProcessor processor in existing.Processors)
6165
{
@@ -113,7 +117,7 @@ public IAsset AddBundle(string route, string contentType, params string[] source
113117

114118
route = NormalizeRoute(route);
115119

116-
IAsset asset = new Asset(route, contentType, this, sourceFiles);
120+
IAsset asset = new Asset(route, contentType, sourceFiles, _assetLogger);
117121
_assets.TryAdd(route, asset);
118122

119123
return asset;
@@ -124,7 +128,7 @@ public IAsset AddAsset(string route, string contentType)
124128
{
125129
route = NormalizeRoute(route);
126130

127-
IAsset asset = new Asset(route, contentType, this, new string[0]);
131+
IAsset asset = new Asset(route, contentType, new string[0], _assetLogger);
128132
_assets.TryAdd(route, asset);
129133

130134
return asset;
@@ -146,7 +150,7 @@ public IEnumerable<IAsset> AddFiles(string contentType, params string[] sourceFi
146150

147151
foreach (string file in sourceFiles)
148152
{
149-
IAsset asset = new Asset(NormalizeRoute(file), contentType, this, new[] { file });
153+
IAsset asset = new Asset(NormalizeRoute(file), contentType, [file], _assetLogger);
150154
list.Add(asset);
151155
_assets.TryAdd(asset.Route, asset);
152156
}
@@ -167,13 +171,13 @@ public static string NormalizeRoute(string route)
167171
{
168172
cleanRoute = cleanRoute.Substring(0, index);
169173
}
170-
171-
if(!cleanRoute.StartsWith("/"))
174+
175+
if (!cleanRoute.StartsWith("/"))
172176
{
173177
cleanRoute = "/" + cleanRoute;
174178
}
175179

176180
return cleanRoute;
177181
}
178182
}
179-
}
183+
}

‎src/WebOptimizer.Core/Extensions/LoggerExtensions.cs‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,11 @@ internal static class LoggerExtensions
2929
logLevel: LogLevel.Information,
3030
eventId: 1005,
3131
formatString: "File '{Path}' not found. Passing on to next middleware.");
32-
32+
private static Action<ILogger, string, string, Exception> _logSourceFileAlreadyAdded = LoggerMessage.Define<string, string>(
33+
logLevel: LogLevel.Information,
34+
eventId: 1006,
35+
formatString: "Source file route '{SourceFileRoute}' already added as clean route '{SourceFileCleanRoute}'.");
36+
3337
public static void LogRequestForAssetStarted(this ILogger logger, string path) =>
3438
_logRequestForAssetStarted(logger, path, null);
3539

@@ -47,5 +51,8 @@ public static void LogZeroByteResponse(this ILogger logger, string path) =>
4751

4852
public static void LogFileNotFound(this ILogger logger, string path) =>
4953
_logFileNotFound(logger, path, null);
54+
55+
public static void LogSourceFileAlreadyAdded(this ILogger logger, string path, string cleanPath) =>
56+
_logSourceFileAlreadyAdded(logger, path, cleanPath, null);
5057
}
5158
}

‎src/WebOptimizer.Core/Extensions/ServiceExtensions.cs‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
using Microsoft.AspNetCore.Mvc.Infrastructure;
44
using Microsoft.Extensions.Caching.Memory;
55
using Microsoft.Extensions.DependencyInjection.Extensions;
6+
using Microsoft.Extensions.Logging;
67
using Microsoft.Extensions.Options;
78
using WebOptimizer;
89

@@ -127,7 +128,11 @@ private static IServiceCollection RegisterComponents(this IServiceCollection ser
127128

128129
services.TryAddSingleton<IMemoryCache, MemoryCache>();
129130
services.TryAddSingleton<IAssetResponseStore, AssetResponseStore>();
130-
services.TryAddSingleton<IAssetPipeline>(factory => pipeline);
131+
services.TryAddSingleton<IAssetPipeline>(factory =>
132+
{
133+
pipeline._assetLogger = factory.GetService<ILogger<Asset>>();
134+
return pipeline;
135+
});
131136
services.TryAddSingleton<IAssetBuilder, AssetBuilder>();
132137
services.TryAddEnumerable(configureOptions);
133138

‎src/WebOptimizer.Core/IAsset.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ public interface IAsset
3737
/// <summary>
3838
/// Gets the webroot relative source files.
3939
/// </summary>
40-
HashSet<string> SourceFiles { get; }
40+
IList<string> SourceFiles { get; }
4141

4242
/// <summary>
4343
/// Executes the processors and returns the modified content.

‎test/WebOptimizer.Core.Test/AssetBuilderTest.cs‎

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22
using System.Collections.Concurrent;
33
using System.Collections.Generic;
44
using System.IO;
5-
using System.Reflection;
65
using System.Threading.Tasks;
76
using Microsoft.AspNetCore.Hosting;
87
using Microsoft.AspNetCore.Http;
@@ -71,7 +70,7 @@ public async Task AssetBuilder_NoDiskCache()
7170
var pipeline = new AssetPipeline();
7271
var options = new WebOptimizerOptions() { EnableDiskCache = false };
7372
var asset = new Mock<IAsset>().SetupAllProperties();
74-
asset.SetupGet(a => a.SourceFiles).Returns(new HashSet<string>());
73+
asset.SetupGet(a => a.SourceFiles).Returns(new List<string>());
7574
asset.SetupGet(a => a.ContentType).Returns("text/css");
7675
asset.SetupGet(a => a.Route).Returns("/file.css");
7776
asset.Setup(a => a.GenerateCacheKey(It.IsAny<HttpContext>(), options)).Returns("cachekey");
@@ -117,7 +116,7 @@ public async Task AssetBuilder_NoMemoryCache_and_NoDiskCache()
117116
var pipeline = new AssetPipeline();
118117
var options = new WebOptimizerOptions() { EnableMemoryCache = false, EnableDiskCache = false };
119118
var asset = new Mock<IAsset>().SetupAllProperties();
120-
asset.SetupGet(a => a.SourceFiles).Returns(new HashSet<string>());
119+
asset.SetupGet(a => a.SourceFiles).Returns(new List<string>());
121120
asset.SetupGet(a => a.ContentType).Returns("text/css");
122121
asset.SetupGet(a => a.Route).Returns("/file.css");
123122
asset.Setup(a => a.GenerateCacheKey(It.IsAny<HttpContext>(), options)).Returns("cachekey");
@@ -153,23 +152,23 @@ public async Task AssetBuilder_NoMemoryCache_and_NoDiskCache()
153152

154153
Assert.Equal(cssContent, result.Body);
155154
}
156-
155+
157156
[Fact2]
158157
public async Task AssetBiulder_NonExistentFileRequested()
159158
{
160159
var options = new WebOptimizerOptions();
161160
var asset = new Mock<IAsset>().SetupAllProperties();
162161
asset.Setup(a => a.GenerateCacheKey(It.IsAny<HttpContext>(), options)).Throws<FileNotFoundException>();
163-
164-
162+
163+
165164
StringValues values;
166165
var response = new Mock<HttpResponse>().SetupAllProperties();
167166
response.Setup(r => r.Headers.Keys).Returns(new string[] { });
168167
var context = new Mock<HttpContext>().SetupAllProperties();
169168
context.Setup(s => s.Request.Headers.TryGetValue("Accept-Encoding", out values)).Returns(false);
170169
context.Setup(c => c.Response).Returns(response.Object);
171170
context.Setup(c => c.Request.Path).Returns("/nonexist_file.css");
172-
171+
173172
var cache = new Mock<IMemoryCache>();
174173

175174
var store = new Mock<IAssetResponseStore>();
@@ -181,4 +180,4 @@ public async Task AssetBiulder_NonExistentFileRequested()
181180
Assert.Null(result);
182181
}
183182
}
184-
}
183+
}

‎test/WebOptimizer.Core.Test/AssetContextTest.cs‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
using System;
22
using Microsoft.AspNetCore.Http;
3+
using Microsoft.Extensions.Logging;
4+
using Moq;
35
using Xunit;
46

57
namespace WebOptimizer.Test
@@ -13,8 +15,9 @@ public void AssetContextConstructor_Success()
1315
string contentType = "text/css";
1416
var sourcefiles = new[] { "file1.css" };
1517
var httpContext = new DefaultHttpContext();
18+
var logger = new Mock<ILogger<Asset>>();
1619

17-
var asset = new Asset(route, contentType, sourcefiles);
20+
var asset = new Asset(route, contentType, sourcefiles, logger.Object);
1821
var assetContext = new AssetContext(httpContext, asset, new WebOptimizerOptions());
1922

2023
Assert.Equal(asset, assetContext.Asset);
@@ -37,8 +40,9 @@ public void AssetContextConstructor_NullHttpContext()
3740
string contentType = "text/css";
3841
var sourcefiles = new[] { "file1.css" };
3942
var httpContext = new DefaultHttpContext();
43+
var logger = new Mock<ILogger<Asset>>();
4044

41-
var asset = new Asset(route, contentType, sourcefiles);
45+
var asset = new Asset(route, contentType, sourcefiles, logger.Object);
4246

4347
Assert.Throws<ArgumentNullException>(() => new AssetContext(null, asset, null));
4448
}

0 commit comments

Comments
 (0)
⚡