Skip to content

Commit

Permalink
Fix smidge bundles not always invalidated between backoffice versions. (
Browse files Browse the repository at this point in the history
umbraco#11700)

* Introduced new smidge CacheBuster with sensible defaults.

* Bring back reset functionality in SmidgeRuntimeMinifier

* Clearer obsolete messaging.
  • Loading branch information
Paul Johnson authored Dec 2, 2021
1 parent 6c5851f commit 08f106a
Show file tree
Hide file tree
Showing 12 changed files with 257 additions and 15 deletions.
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ src/Umbraco.Web.UI/wwwroot/[Uu]mbraco/js/*
src/Umbraco.Web.UI/wwwroot/[Uu]mbraco/lib/*
src/Umbraco.Web.UI/wwwroot/[Uu]mbraco/views/*
src/Umbraco.Web.UI/wwwroot/Media/*
src/Umbraco.Web.UI/Smidge/

# Tests
cypress.env.json
Expand Down
24 changes: 24 additions & 0 deletions src/Umbraco.Core/Configuration/EntryAssemblyMetadata.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
using System.Reflection;

namespace Umbraco.Cms.Core.Configuration
{
internal class EntryAssemblyMetadata : IEntryAssemblyMetadata
{
public EntryAssemblyMetadata()
{
var entryAssembly = Assembly.GetEntryAssembly();

Name = entryAssembly
?.GetName()
?.Name ?? string.Empty;

InformationalVersion = entryAssembly
?.GetCustomAttribute<AssemblyInformationalVersionAttribute>()
?.InformationalVersion ?? string.Empty;
}

public string Name { get; }

public string InformationalVersion { get; }
}
}
18 changes: 18 additions & 0 deletions src/Umbraco.Core/Configuration/IEntryAssemblyMetadata.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
namespace Umbraco.Cms.Core.Configuration
{
/// <summary>
/// Provides metadata about the entry assembly.
/// </summary>
public interface IEntryAssemblyMetadata
{
/// <summary>
/// Gets the Name of entry assembly.
/// </summary>
public string Name { get; }

/// <summary>
/// Gets the InformationalVersion string for entry assembly.
/// </summary>
public string InformationalVersion { get; }
}
}
1 change: 1 addition & 0 deletions src/Umbraco.Core/DependencyInjection/UmbracoBuilder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,7 @@ private void AddCoreServices()
Services.AddUnique(factory => factory.GetRequiredService<AppCaches>().RequestCache);
Services.AddUnique<IProfilingLogger, ProfilingLogger>();
Services.AddUnique<IUmbracoVersion, UmbracoVersion>();
Services.AddUnique<IEntryAssemblyMetadata, EntryAssemblyMetadata>();

this.AddAllCoreCollectionBuilders();
this.AddNotificationHandler<UmbracoApplicationStartingNotification, EssentialDirectoryCreator>();
Expand Down
13 changes: 12 additions & 1 deletion src/Umbraco.Core/WebAssets/IRuntimeMinifier.cs
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,18 @@ public interface IRuntimeMinifier
/// <summary>
/// Ensures that all runtime minifications are refreshed on next request. E.g. Clearing cache.
/// </summary>
/// <remarks>
/// <para>
/// No longer necessary, invalidation occurs automatically if any of the following occur.
/// </para>
/// <list type="bullet">
/// <item>Your sites assembly information version changes.</item>
/// <item>Umbraco.Cms.Core assembly information version changes.</item>
/// <item>RuntimeMinificationSettings Version string changes.</item>
/// </list>
/// <see href="https://our.umbraco.com/documentation/Reference/V9-Config/RuntimeMinificationSettings/" /> for further details.
/// </remarks>
[Obsolete("Invalidation is handled automatically. Scheduled for removal V11.")]
void Reset();

}
}
3 changes: 0 additions & 3 deletions src/Umbraco.Web.BackOffice/Install/InstallController.cs
Original file line number Diff line number Diff line change
Expand Up @@ -72,9 +72,6 @@ public async Task<ActionResult> Index()
// TODO: Update for package migrations
if (_runtime.Level == RuntimeLevel.Upgrade)
{
// Update ClientDependency version and delete its temp directories to make sure we get fresh caches
_runtimeMinifier.Reset();

var authResult = await this.AuthenticateBackOfficeAsync();

if (!authResult.Succeeded)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
using Microsoft.Extensions.Logging;
using Serilog;
using Smidge;
using Smidge.Cache;
using Smidge.FileProcessors;
using Smidge.InMemory;
using Smidge.Nuglify;
Expand Down Expand Up @@ -274,6 +275,7 @@ public static IUmbracoBuilder AddRuntimeMinifier(this IUmbracoBuilder builder)
new[] { "/App_Plugins/**/*.js", "/App_Plugins/**/*.css" }));
});

builder.Services.AddUnique<ICacheBuster, UmbracoSmidgeConfigCacheBuster>();
builder.Services.AddSmidge(builder.Config.GetSection(Constants.Configuration.ConfigRuntimeMinification));
// Replace the Smidge request helper, in order to discourage the use of brotli since it's super slow
builder.Services.AddUnique<IRequestHelper, SmidgeRequestHelper>();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ public class SmidgeRuntimeMinifier : IRuntimeMinifier
private readonly IHostingEnvironment _hostingEnvironment;
private readonly IConfigManipulator _configManipulator;
private readonly CacheBusterResolver _cacheBusterResolver;
private readonly RuntimeMinificationSettings _runtimeMinificationSettings;
private readonly IBundleManager _bundles;
private readonly SmidgeHelperAccessor _smidge;

Expand Down Expand Up @@ -53,7 +52,6 @@ public SmidgeRuntimeMinifier(
_hostingEnvironment = hostingEnvironment;
_configManipulator = configManipulator;
_cacheBusterResolver = cacheBusterResolver;
_runtimeMinificationSettings = runtimeMinificationSettings.Value;
_jsMinPipeline = new Lazy<PreProcessPipeline>(() => _bundles.PipelineFactory.Create(typeof(JsMinifier)));
_cssMinPipeline = new Lazy<PreProcessPipeline>(() => _bundles.PipelineFactory.Create(typeof(NuglifyCss)));

Expand All @@ -76,10 +74,10 @@ public SmidgeRuntimeMinifier(
return defaultCss;
});

Type cacheBusterType = _runtimeMinificationSettings.CacheBuster switch
Type cacheBusterType = runtimeMinificationSettings.Value.CacheBuster switch
{
RuntimeMinificationCacheBuster.AppDomain => typeof(AppDomainLifetimeCacheBuster),
RuntimeMinificationCacheBuster.Version => typeof(ConfigCacheBuster),
RuntimeMinificationCacheBuster.Version => typeof(UmbracoSmidgeConfigCacheBuster),
RuntimeMinificationCacheBuster.Timestamp => typeof(TimestampCacheBuster),
_ => throw new NotImplementedException()
};
Expand Down Expand Up @@ -169,18 +167,12 @@ public async Task<string> MinifyAsync(string fileContent, AssetType assetType)
}
}


/// <inheritdoc />
/// <remarks>
/// Smidge uses the version number as cache buster (configurable).
/// We therefore can reset, by updating the version number in config
/// </remarks>
[Obsolete("Invalidation is handled automatically. Scheduled for removal V11.")]
public void Reset()
{
var version = DateTime.UtcNow.Ticks.ToString();
_configManipulator.SaveConfigValue(Cms.Core.Constants.Configuration.ConfigRuntimeMinificationVersion, version.ToString());
}


}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
using System;
using Microsoft.Extensions.Logging;
using Microsoft.Extensions.Options;
using Smidge;
using Smidge.Cache;
using Umbraco.Cms.Core.Configuration;
using Umbraco.Cms.Core.Configuration.Models;
using Umbraco.Extensions;

namespace Umbraco.Cms.Web.Common.RuntimeMinification
{
/// <summary>
/// Constructs a cache buster string with sensible defaults.
/// </summary>
/// <remarks>
/// <para>
/// Had planned on handling all of this in SmidgeRuntimeMinifier, but that only handles some urls.
/// </para>
/// <para>
/// A lot of the work is delegated e.g. to <see cref="SmidgeHelper.GenerateJsUrlsAsync(string, bool)"/>
/// which doesn't care about the cache buster string in our classes only the value returned by the resolved ICacheBuster.
/// </para>
/// <para>
/// Then I thought fine I'll just use a IConfigureOptions to tweak upstream <see cref="ConfigCacheBuster"/>, but that only cares about the <see cref="SmidgeConfig"/>
/// class we instantiate and pass through in <see cref="Umbraco.Extensions.UmbracoBuilderExtensions.AddRuntimeMinifier"/>
/// </para>
/// <para>
/// So here we are, create our own to ensure we cache bust in a reasonable fashion.
/// </para>
/// <br/><br/>
/// <para>
/// Note that this class makes some other bits of code pretty redundant e.g. <see cref="UrlHelperExtensions.GetUrlWithCacheBust"/> will
/// concatenate version with CacheBuster value and hash again, but there's no real harm so can think about that later.
/// </para>
/// </remarks>
internal class UmbracoSmidgeConfigCacheBuster : ICacheBuster
{
private readonly IOptions<RuntimeMinificationSettings> _runtimeMinificationSettings;
private readonly IUmbracoVersion _umbracoVersion;
private readonly IEntryAssemblyMetadata _entryAssemblyMetadata;

private string _cacheBusterValue;

public UmbracoSmidgeConfigCacheBuster(
IOptions<RuntimeMinificationSettings> runtimeMinificationSettings,
IUmbracoVersion umbracoVersion,
IEntryAssemblyMetadata entryAssemblyMetadata)
{
_runtimeMinificationSettings = runtimeMinificationSettings ?? throw new ArgumentNullException(nameof(runtimeMinificationSettings));
_umbracoVersion = umbracoVersion ?? throw new ArgumentNullException(nameof(umbracoVersion));
_entryAssemblyMetadata = entryAssemblyMetadata ?? throw new ArgumentNullException(nameof(entryAssemblyMetadata));
}

private string CacheBusterValue
{
get
{
if (_cacheBusterValue != null)
{
return _cacheBusterValue;
}

// Assembly Name adds a bit of uniqueness across sites when version missing from config.
// Adds a bit of security through obscurity that was asked for in standup.
var prefix = _runtimeMinificationSettings.Value.Version ?? _entryAssemblyMetadata.Name ?? string.Empty;
var umbracoVersion = _umbracoVersion.SemanticVersion.ToString();
var downstreamVersion = _entryAssemblyMetadata.InformationalVersion;

_cacheBusterValue = $"{prefix}_{umbracoVersion}_{downstreamVersion}".GenerateHash();

return _cacheBusterValue;
}
}

public string GetValue() => CacheBusterValue;
}
}
5 changes: 5 additions & 0 deletions tests/.editorconfig
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
root = false

[*.cs]
csharp_style_var_when_type_is_apparent = true:none
csharp_style_var_elsewhere = true:none
Original file line number Diff line number Diff line change
@@ -0,0 +1,113 @@
using AutoFixture;
using AutoFixture.AutoMoq;
using AutoFixture.Kernel;
using Microsoft.Extensions.Options;
using Moq;
using NUnit.Framework;
using Smidge;
using Smidge.Cache;
using Umbraco.Cms.Core.Configuration;
using Umbraco.Cms.Core.Configuration.Models;
using Umbraco.Cms.Core.Semver;
using Umbraco.Cms.Web.Common.RuntimeMinification;
using Umbraco.Extensions;

namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Web.Common.RuntimeMinification
{
/// <remarks>
/// UmbracoCustomizations kindly configures an IUmbracoVersion so we need to go verbose without AutoMoqData
/// </remarks>
[TestFixture]
public class UmbracoSmidgeConfigCacheBusterTests
{
[Test]
public void GetValue_DefaultReleaseSetupWithNoConfiguredVersion_HasSensibleDefaults()
{
var fixture = new Fixture();
fixture.Customize(new AutoMoqCustomization());

var umbracoVersion = fixture.Freeze<Mock<IUmbracoVersion>>();
var entryAssemblyMetadata = fixture.Freeze<Mock<IEntryAssemblyMetadata>>();
var sut = fixture.Create<UmbracoSmidgeConfigCacheBuster>();

umbracoVersion.Setup(x => x.SemanticVersion).Returns(new SemVersion(9, 4, 5, "beta", "41658f99"));
entryAssemblyMetadata.Setup(x => x.Name).Returns("Bills.Brilliant.Bakery");
entryAssemblyMetadata.Setup(x => x.InformationalVersion).Returns("42.1.2-alpha+41658f99");

var result = sut.GetValue();

var expected = $"Bills.Brilliant.Bakery_9.4.5-beta+41658f99_42.1.2-alpha+41658f99".GenerateHash();
Assert.AreEqual(expected, result);
}

[Test]
public void GetValue_DefaultReleaseSetupWithConfiguredVersion_HasSensibleDefaults()
{
var config = Options.Create(new RuntimeMinificationSettings { Version = "1" });
var fixture = new Fixture();
fixture.Customize(new AutoMoqCustomization());
fixture.Inject(config);

var umbracoVersion = fixture.Freeze<Mock<IUmbracoVersion>>();
var entryAssemblyMetadata = fixture.Freeze<Mock<IEntryAssemblyMetadata>>();
var sut = fixture.Create<UmbracoSmidgeConfigCacheBuster>();

umbracoVersion.Setup(x => x.SemanticVersion).Returns(new SemVersion(9, 4, 5, "beta", "41658f99"));
entryAssemblyMetadata.Setup(x => x.Name).Returns("Bills.Brilliant.Bakery");
entryAssemblyMetadata.Setup(x => x.InformationalVersion).Returns("42.1.2-alpha+41658f99");

var result = sut.GetValue();

var expected = $"1_9.4.5-beta+41658f99_42.1.2-alpha+41658f99".GenerateHash();
Assert.AreEqual(expected, result);
}

[Test]
public void GetValue_DefaultReleaseSetupWithNoConfiguredVersion_ChangesWhenUmbracoVersionChanges()
{
var fixture = new Fixture();
fixture.Customize(new AutoMoqCustomization());

var umbracoVersion = fixture.Freeze<Mock<IUmbracoVersion>>();
var entryAssemblyMetadata = fixture.Freeze<Mock<IEntryAssemblyMetadata>>();
var sut = fixture.Create<UmbracoSmidgeConfigCacheBuster>();

umbracoVersion.Setup(x => x.SemanticVersion).Returns(new SemVersion(9, 4, 5, "beta", "41658f99"));
entryAssemblyMetadata.Setup(x => x.Name).Returns("Bills.Brilliant.Bakery");
entryAssemblyMetadata.Setup(x => x.InformationalVersion).Returns("42.1.2-alpha+41658f99");

var before = sut.GetValue();

umbracoVersion.Setup(x => x.SemanticVersion).Returns(new SemVersion(9, 5, 0, "beta", "41658f99"));
sut = fixture.Create<UmbracoSmidgeConfigCacheBuster>();

var after = sut.GetValue();

Assert.AreNotEqual(before, after);
}

[Test]
public void GetValue_DefaultReleaseSetupWithNoConfiguredVersion_ChangesWhenDownstreamVersionChanges()
{
var fixture = new Fixture();
fixture.Customize(new AutoMoqCustomization());

var umbracoVersion = fixture.Freeze<Mock<IUmbracoVersion>>();
var entryAssemblyMetadata = fixture.Freeze<Mock<IEntryAssemblyMetadata>>();
var sut = fixture.Create<UmbracoSmidgeConfigCacheBuster>();

umbracoVersion.Setup(x => x.SemanticVersion).Returns(new SemVersion(9, 4, 5, "beta", "41658f99"));
entryAssemblyMetadata.Setup(x => x.Name).Returns("Bills.Brilliant.Bakery");
entryAssemblyMetadata.Setup(x => x.InformationalVersion).Returns("42.1.2-alpha+41658f99");

var before = sut.GetValue();

entryAssemblyMetadata.Setup(x => x.InformationalVersion).Returns("42.2.0-rc");
sut = fixture.Create<UmbracoSmidgeConfigCacheBuster>();

var after = sut.GetValue();

Assert.AreNotEqual(before, after);
}
}
}
1 change: 1 addition & 0 deletions umbraco.sln.DotSettings
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
<s:String x:Key="/Default/PatternsAndTemplates/StructuralSearch/Pattern/=2DA32DA040A7D74599ABE288C7224CF0/Severity/@EntryValue">HINT</s:String>
<s:Boolean x:Key="/Default/PatternsAndTemplates/StructuralSearch/Pattern/=37A0B37A0ABAA34AA5CB32A93653C4FE/@KeyIndexDefined">False</s:Boolean>
<s:String x:Key="/Default/CodeInspection/CSharpLanguageProject/LanguageLevel/@EntryValue">Default</s:String>
<s:Boolean x:Key="/Default/UserDictionary/Words/=Smidge/@EntryIndexedValue">True</s:Boolean>
<s:Boolean x:Key="/Default/UserDictionary/Words/=Umbraco/@EntryIndexedValue">True</s:Boolean>
<s:Boolean x:Key="/Default/UserDictionary/Words/=unpublish/@EntryIndexedValue">True</s:Boolean>
<s:Boolean x:Key="/Default/UserDictionary/Words/=unpublishing/@EntryIndexedValue">True</s:Boolean></wpf:ResourceDictionary>

0 comments on commit 08f106a

Please sign in to comment.