From c917139d1c734f5bbeb031ea56ef78c59aedb644 Mon Sep 17 00:00:00 2001 From: Shannon Date: Wed, 24 Jul 2024 11:16:50 -0600 Subject: [PATCH] Adds a lock to bundling --- src/Smidge/Controllers/SmidgeController.cs | 138 +++++++++++++-------- 1 file changed, 86 insertions(+), 52 deletions(-) diff --git a/src/Smidge/Controllers/SmidgeController.cs b/src/Smidge/Controllers/SmidgeController.cs index 2f76f03..ec1590f 100644 --- a/src/Smidge/Controllers/SmidgeController.cs +++ b/src/Smidge/Controllers/SmidgeController.cs @@ -1,18 +1,19 @@ using System; -using Microsoft.AspNetCore.Mvc; -using Smidge.CompositeFiles; -using Smidge.Models; +using System.Collections.Concurrent; using System.Collections.Generic; using System.Diagnostics; using System.IO; -using System.IO.Compression; using System.Linq; +using System.Threading; using System.Threading.Tasks; +using Microsoft.AspNetCore.Authorization; +using Microsoft.AspNetCore.Mvc; using Microsoft.Extensions.FileProviders; using Microsoft.Extensions.Logging; -using Smidge.FileProcessors; using Smidge.Cache; -using Microsoft.AspNetCore.Authorization; +using Smidge.CompositeFiles; +using Smidge.FileProcessors; +using Smidge.Models; namespace Smidge.Controllers { @@ -27,6 +28,8 @@ namespace Smidge.Controllers [AllowAnonymous] public class SmidgeController : Controller { + private static readonly ConcurrentDictionary s_locks; + private readonly ISmidgeFileSystem _fileSystem; private readonly IBundleManager _bundleManager; private readonly IBundleFileSetGenerator _fileSetGenerator; @@ -72,46 +75,41 @@ public async Task Bundle( return NotFound(); } - var bundleOptions = foundBundle.GetBundleOptions(_bundleManager, bundleModel.Debug); - - var cacheBusterValue = bundleModel.ParsedPath.CacheBusterValue; + Options.BundleOptions bundleOptions = foundBundle.GetBundleOptions(_bundleManager, bundleModel.Debug); - //now we need to determine if this bundle has already been created - var cacheFile = _fileSystem.CacheFileSystem.GetCachedCompositeFile(cacheBusterValue, bundleModel.Compression, bundleModel.FileKey, out var cacheFilePath); - if (cacheFile.Exists) + if (TryGetBundle(bundleModel, out IActionResult actionResult, out var cacheFilePath)) { - _logger.LogDebug($"Returning bundle '{bundleModel.FileKey}' from cache"); - + return actionResult; + } - if (!string.IsNullOrWhiteSpace(cacheFile.PhysicalPath)) + SemaphoreSlim bundleLock = s_locks.GetOrAdd(foundBundle.Name, s => new SemaphoreSlim(1, 1)); + await bundleLock.WaitAsync(); + try + { + // Double check, might be available now + if (TryGetBundle(bundleModel, out actionResult, out _)) { - //if physical path is available then it's the physical file system, in which case we'll deliver the file with the PhysicalFileResult - //FilePathResult uses IHttpSendFileFeature which is a native host option for sending static files - return PhysicalFile(cacheFile.PhysicalPath, bundleModel.Mime); + return actionResult; } - else + + //the bundle doesn't exist so we'll go get the files, process them and create the bundle + + //get the files for the bundle + IWebFile[] files = _fileSetGenerator.GetOrderedFileSet(foundBundle, + _processorFactory.CreateDefault( + //the file type in the bundle will always be the same + foundBundle.Files[0].DependencyType)) + .ToArray(); + + if (files.Length == 0) { - return File(cacheFile.CreateReadStream(), bundleModel.Mime); + return NotFound(); } - } - //the bundle doesn't exist so we'll go get the files, process them and create the bundle - //TODO: We should probably lock here right?! we don't want multiple threads trying to do this at the same time, we'll need a dictionary of locks to do this effectively + var cacheBusterValue = bundleModel.ParsedPath.CacheBusterValue; - //get the files for the bundle - var files = _fileSetGenerator.GetOrderedFileSet(foundBundle, - _processorFactory.CreateDefault( - //the file type in the bundle will always be the same - foundBundle.Files[0].DependencyType)) - .ToArray(); - - if (files.Length == 0) - { - return NotFound(); - } + using var bundleContext = new BundleContext(cacheBusterValue, bundleModel, cacheFilePath); - using (var bundleContext = new BundleContext(cacheBusterValue, bundleModel, cacheFilePath)) - { var watch = new Stopwatch(); watch.Start(); _logger.LogDebug($"Processing bundle '{bundleModel.FileKey}', debug? {bundleModel.Debug} ..."); @@ -123,7 +121,7 @@ public async Task Bundle( } //Get each file path to it's hashed location since that is what the pre-processed file will be saved as - var fileInfos = files.Select(x => _fileSystem.CacheFileSystem.GetCacheFile( + IEnumerable fileInfos = files.Select(x => _fileSystem.CacheFileSystem.GetCacheFile( x, () => _fileSystem.GetRequiredFileInfo(x), bundleOptions.FileWatchOptions.Enabled, @@ -131,23 +129,30 @@ public async Task Bundle( cacheBusterValue, out _)); - using (var resultStream = await GetCombinedStreamAsync(fileInfos, bundleContext)) - { - //compress the response (if enabled) - //do not compress anything if it's not enabled in the bundle options - var compressedStream = await Compressor.CompressAsync(bundleOptions.CompressResult ? bundleModel.Compression : CompressionType.None, - bundleOptions.CompressionLevel, - resultStream); - - //save the resulting compressed file, if compression is not enabled it will just save the non compressed format - // this persisted file will be used in the CheckNotModifiedAttribute which will short circuit the request and return - // the raw file if it exists for further requests to this path - await CacheCompositeFileAsync(_fileSystem.CacheFileSystem, cacheFilePath, compressedStream); + using Stream resultStream = await GetCombinedStreamAsync(fileInfos, bundleContext); - _logger.LogDebug($"Processed bundle '{bundleModel.FileKey}' in {watch.ElapsedMilliseconds}ms"); + //compress the response (if enabled) + //do not compress anything if it's not enabled in the bundle options + Stream compressedStream = await Compressor.CompressAsync(bundleOptions.CompressResult ? bundleModel.Compression : CompressionType.None, + bundleOptions.CompressionLevel, + resultStream); - //return the stream - return File(compressedStream, bundleModel.Mime); + //save the resulting compressed file, if compression is not enabled it will just save the non compressed format + // this persisted file will be used in the CheckNotModifiedAttribute which will short circuit the request and return + // the raw file if it exists for further requests to this path + await CacheCompositeFileAsync(_fileSystem.CacheFileSystem, cacheFilePath, compressedStream); + + _logger.LogDebug($"Processed bundle '{bundleModel.FileKey}' in {watch.ElapsedMilliseconds}ms"); + + //return the stream + return File(compressedStream, bundleModel.Mime); + } + finally + { + // Remove the lock from the dictionary and release the lock. + if (s_locks.TryRemove(foundBundle.Name, out SemaphoreSlim lck)) + { + lck.Release(); } } } @@ -201,6 +206,35 @@ public async Task Composite( } } + private bool TryGetBundle(BundleRequestModel bundleModel, out IActionResult actionResult, out string cacheFilePath) + { + var cacheBusterValue = bundleModel.ParsedPath.CacheBusterValue; + + //now we need to determine if this bundle has already been created + IFileInfo cacheFile = _fileSystem.CacheFileSystem.GetCachedCompositeFile(cacheBusterValue, bundleModel.Compression, bundleModel.FileKey, out cacheFilePath); + if (cacheFile.Exists) + { + _logger.LogDebug($"Returning bundle '{bundleModel.FileKey}' from cache"); + + + if (!string.IsNullOrWhiteSpace(cacheFile.PhysicalPath)) + { + //if physical path is available then it's the physical file system, in which case we'll deliver the file with the PhysicalFileResult + //FilePathResult uses IHttpSendFileFeature which is a native host option for sending static files + actionResult = PhysicalFile(cacheFile.PhysicalPath, bundleModel.Mime); + return true; + } + else + { + actionResult = File(cacheFile.CreateReadStream(), bundleModel.Mime); + return true; + } + } + + actionResult = null; + return false; + } + private static async Task CacheCompositeFileAsync(ICacheFileSystem cacheProvider, string filePath, Stream compositeStream) { await cacheProvider.WriteFileAsync(filePath, compositeStream);