Skip to content

Commit

Permalink
AssemblyScanner improvements (#6814)
Browse files Browse the repository at this point in the history 
* Static assembly validator

* Style ScannerConfiguration

* Length instead of Any

* PatternMatch

* Adding range directly to let the collection grow by found files

* Style IsMatch

* Use pattern match and switch to Attribute.GetCustomAttribute because it is slightly faster

* HashSet for contains

* Style ScanningComponent

* No longer do ToLowerInvariant

* Remove Any

* Materialize allowed types eagerly to make sure result types can grow in one go and the enumerator is not used

* PublicKeyValidation

* Remove algorithmic complexity of exclusion check

* Better encapsulation

* Better names

* Remove unneeded comment

* Encapsulate token extraction into validator

* Tweaks

* Better comment

---------

Co-authored-by: Brandon Ording <[email protected]>
  • Loading branch information
@danielmarbach @bording
danielmarbach and bording authored Aug 31, 2023
1 parent 0321ec8 commit 2a7f3cd
Show file tree
Hide file tree
Showing 7 changed files with 101 additions and 138 deletions.
3 changes: 1 addition & 2 deletions src/NServiceBus.Core.Tests/AssemblyScanner/When_exclusion_predicate_is_used.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
[TestFixture]
public class When_exclusion_predicate_is_used
{

[Test]
public void No_files_explicitly_excluded_are_returned()
{
Expand All @@ -21,7 +20,7 @@ public void No_files_explicitly_excluded_are_returned()
},
ScanAppDomainAssemblies = false
}
.GetScannableAssemblies();
.GetScannableAssemblies();

var skippedFiles = results.SkippedFiles;
var explicitlySkippedDll = skippedFiles.FirstOrDefault(s => s.FilePath.Contains("dotNet.dll"));
Expand Down
11 changes: 3 additions & 8 deletions src/NServiceBus.Core.Tests/AssemblyScanner/When_validating_assemblies.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,11 @@ public class When_validating_assemblies
[Test]
public void Should_not_validate_system_assemblies()
{
var validator = new AssemblyValidator();
var systemAssemblies = AppDomain.CurrentDomain.GetAssemblies().Where(a => a.FullName.Contains("System"));

foreach (var assembly in systemAssemblies)
{
validator.ValidateAssemblyFile(assembly.Location, out var shouldLoad, out var reason);
AssemblyValidator.ValidateAssemblyFile(assembly.Location, out var shouldLoad, out var reason);

Assert.IsFalse(shouldLoad, $"Should not validate {assembly.FullName}");
Assert.That(reason == "File is a .NET runtime assembly.");
Expand All @@ -25,9 +24,7 @@ public void Should_not_validate_system_assemblies()
[Test]
public void Should_validate_NServiceBus_Core_assembly()
{
var validator = new AssemblyValidator();

validator.ValidateAssemblyFile(typeof(EndpointConfiguration).Assembly.Location, out var shouldLoad, out var reason);
AssemblyValidator.ValidateAssemblyFile(typeof(EndpointConfiguration).Assembly.Location, out var shouldLoad, out var reason);

Assert.IsTrue(shouldLoad);
Assert.That(reason == "File is a .NET assembly.");
Expand All @@ -36,9 +33,7 @@ public void Should_validate_NServiceBus_Core_assembly()
[Test]
public void Should_validate_non_system_assemblies()
{
var validator = new AssemblyValidator();

validator.ValidateAssemblyFile(GetType().Assembly.Location, out var shouldLoad, out var reason);
AssemblyValidator.ValidateAssemblyFile(GetType().Assembly.Location, out var shouldLoad, out var reason);

Assert.IsTrue(shouldLoad);
Assert.That(reason == "File is a .NET assembly.");
Expand Down
2 changes: 1 addition & 1 deletion src/NServiceBus.Core.Tests/Config/When_scanning_assemblies.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ IEnumerable<Assembly> GetAssembliesInDirectory(string path, params string[] asse

if (assembliesToSkip != null)
{
assemblyScanner.AssembliesToSkip = assembliesToSkip.ToList();
assemblyScanner.AssembliesToSkip = assembliesToSkip;
}
return assemblyScanner
.GetScannableAssemblies()
Expand Down
106 changes: 44 additions & 62 deletions src/NServiceBus.Core/Hosting/Helpers/AssemblyScanner.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,17 @@ internal AssemblyScanner(Assembly assemblyToScan)
/// </summary>
public bool ScanFileSystemAssemblies { get; set; } = true;

internal string CoreAssemblyName { get; set; } = NServicebusCoreAssemblyName;
internal string CoreAssemblyName { get; set; } = NServiceBusCoreAssemblyName;

internal IReadOnlyCollection<string> AssembliesToSkip
{
set => assembliesToSkip = new HashSet<string>(value.Select(Path.GetFileNameWithoutExtension), StringComparer.OrdinalIgnoreCase);
}

internal IReadOnlyCollection<Type> TypesToSkip
{
set => typesToSkip = new HashSet<Type>(value);
}

internal string AdditionalAssemblyScanningPath { get; set; }

Expand Down Expand Up @@ -156,7 +166,7 @@ bool TryLoadScannableAssembly(string assemblyPath, AssemblyScannerResults result
return false;
}

assemblyValidator.ValidateAssemblyFile(assemblyPath, out var shouldLoad, out var reason);
AssemblyValidator.ValidateAssemblyFile(assemblyPath, out var shouldLoad, out var reason);

if (!shouldLoad)
{
Expand Down Expand Up @@ -248,7 +258,7 @@ internal static string FormatReflectionTypeLoadException(string fileName, Reflec
{
var sb = new StringBuilder($"Could not enumerate all types for '{fileName}'.");

if (!e.LoaderExceptions.Any())
if (e.LoaderExceptions.Length == 0)
{
sb.NewLine($"Exception message: {e}");
return sb.ToString();
Expand All @@ -260,7 +270,7 @@ internal static string FormatReflectionTypeLoadException(string fileName, Reflec

foreach (var ex in e.LoaderExceptions)
{
if (ex is FileLoadException loadException && loadException.FileName != null)
if (ex is FileLoadException { FileName: not null } loadException)
{
if (!files.Contains(loadException.FileName))
{
Expand Down Expand Up @@ -297,65 +307,43 @@ static List<FileInfo> ScanDirectoryForAssemblyFiles(string directoryToScan, bool
var searchOption = scanNestedDirectories ? SearchOption.AllDirectories : SearchOption.TopDirectoryOnly;
foreach (var searchPattern in FileSearchPatternsToUse)
{
foreach (var info in baseDir.GetFiles(searchPattern, searchOption))
{
fileInfo.Add(info);
}
fileInfo.AddRange(baseDir.GetFiles(searchPattern, searchOption));
}
return fileInfo;
}

bool IsExcluded(string assemblyNameOrFileName)
{
var isExplicitlyExcluded = AssembliesToSkip.Any(excluded => IsMatch(excluded, assemblyNameOrFileName));
if (isExplicitlyExcluded)
{
return true;
}

var isExcludedByDefault = DefaultAssemblyExclusions.Any(exclusion => IsMatch(exclusion, assemblyNameOrFileName));
if (isExcludedByDefault)
{
return true;
}

return false;
}

static bool IsMatch(string expression1, string expression2)
{
return DistillLowerAssemblyName(expression1) == DistillLowerAssemblyName(expression2);
}
bool IsExcluded(string assemblyNameOrFileNameWithoutExtension) =>
assembliesToSkip.Contains(assemblyNameOrFileNameWithoutExtension) ||
DefaultAssemblyExclusions.Contains(assemblyNameOrFileNameWithoutExtension);

bool IsAllowedType(Type type)
// The parameter and return types of this method are deliberately using the most concrete types
// to avoid unnecessary allocations
List<Type> FilterAllowedTypes(Type[] types)
{
return type != null &&
!type.IsValueType &&
!IsCompilerGenerated(type) &&
!TypesToSkip.Contains(type);
}

static bool IsCompilerGenerated(Type type)
{
return type.GetCustomAttribute<CompilerGeneratedAttribute>(false) != null;
}

static string DistillLowerAssemblyName(string assemblyOrFileName)
{
var lowerAssemblyName = assemblyOrFileName.ToLowerInvariant();
if (lowerAssemblyName.EndsWith(".dll") || lowerAssemblyName.EndsWith(".exe"))
// assume the majority of types will be allowed to preallocate the list
var allowedTypes = new List<Type>(types.Length);
foreach (var typeToAdd in types)
{
lowerAssemblyName = lowerAssemblyName.Substring(0, lowerAssemblyName.Length - 4);
if (IsAllowedType(typeToAdd))
{
allowedTypes.Add(typeToAdd);
}
}
return lowerAssemblyName;
return allowedTypes;
}

bool IsAllowedType(Type type) =>
type is { IsValueType: false } &&
Attribute.GetCustomAttribute(type, typeof(CompilerGeneratedAttribute), false) == null &&
!typesToSkip.Contains(type);

void AddTypesToResult(Assembly assembly, AssemblyScannerResults results)
{
try
{
//will throw if assembly cannot be loaded
results.Types.AddRange(assembly.GetTypes().Where(IsAllowedType));
var types = assembly.GetTypes();
results.Types.AddRange(FilterAllowedTypes(types));
}
catch (ReflectionTypeLoadException e)
{
Expand All @@ -368,7 +356,7 @@ void AddTypesToResult(Assembly assembly, AssemblyScannerResults results)
}

LogManager.GetLogger<AssemblyScanner>().Warn(errorMessage);
results.Types.AddRange(e.Types.Where(IsAllowedType));
results.Types.AddRange(FilterAllowedTypes(e.Types));
}
results.Assemblies.Add(assembly);
}
Expand All @@ -387,35 +375,29 @@ bool ShouldScanDependencies(Assembly assembly)
return false;
}

if (AssemblyValidator.IsRuntimeAssembly(assemblyName.GetPublicKeyToken()))
{
return false;
}

if (IsExcluded(assemblyName.Name))
if (AssemblyValidator.IsRuntimeAssembly(assemblyName))
{
return false;
}

return true;
// AssemblyName.Name is without the file extension
return !IsExcluded(assemblyName.Name);
}

readonly AssemblyValidator assemblyValidator = new();
internal List<string> AssembliesToSkip = new();
internal bool ScanNestedDirectories;
internal List<Type> TypesToSkip = new();
readonly Assembly assemblyToScan;
readonly string baseDirectoryToScan;
const string NServicebusCoreAssemblyName = "NServiceBus.Core";
HashSet<Type> typesToSkip = new();
HashSet<string> assembliesToSkip = new(StringComparer.OrdinalIgnoreCase);
const string NServiceBusCoreAssemblyName = "NServiceBus.Core";

static readonly string[] FileSearchPatternsToUse =
{
"*.dll",
"*.exe"
};

//TODO: delete when we make message scanning lazy #1617
static readonly string[] DefaultAssemblyExclusions =
static readonly HashSet<string> DefaultAssemblyExclusions = new(StringComparer.OrdinalIgnoreCase)
{
// NSB Build-Dependencies
"nunit",
Expand Down
4 changes: 2 additions & 2 deletions src/NServiceBus.Core/Hosting/Helpers/AssemblyScannerConfiguration.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ public void ExcludeTypes(params Type[] types)
ExcludedTypes.AddRange(types);
}

internal List<string> ExcludedAssemblies { get; } = new List<string>(0);
internal List<Type> ExcludedTypes { get; } = new List<Type>(0);
internal List<string> ExcludedAssemblies { get; } = new(0);
internal List<Type> ExcludedTypes { get; } = new(0);
}
}
19 changes: 5 additions & 14 deletions src/NServiceBus.Core/Hosting/Helpers/AssemblyScanningComponent.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,9 @@
using System;
using System.Collections.Generic;
using System.Linq;
using Settings;
using Hosting.Helpers;
using System.Reflection;
using Hosting.Helpers;
using Settings;

class AssemblyScanningComponent
{
Expand Down Expand Up @@ -67,30 +67,21 @@ public static AssemblyScanningComponent Initialize(Configuration configuration,
return new AssemblyScanningComponent(availableTypes);
}

AssemblyScanningComponent(List<Type> availableTypes)
{
AvailableTypes = availableTypes;
}
AssemblyScanningComponent(List<Type> availableTypes) => AvailableTypes = availableTypes;

public List<Type> AvailableTypes { get; }

public class Configuration
{
public Configuration(SettingsHolder settings)
{
this.settings = settings;
}
public Configuration(SettingsHolder settings) => this.settings = settings;

public List<Type> UserProvidedTypes { get; set; }

public AssemblyScannerConfiguration AssemblyScannerConfiguration => settings.GetOrCreate<AssemblyScannerConfiguration>();

public IList<Type> AvailableTypes => settings.Get<IList<Type>>(TypesToScanSettingsKey);

public void SetDefaultAvailableTypes(IList<Type> scannedTypes)
{
settings.SetDefault(TypesToScanSettingsKey, scannedTypes);
}
public void SetDefaultAvailableTypes(IList<Type> scannedTypes) => settings.SetDefault(TypesToScanSettingsKey, scannedTypes);

readonly SettingsHolder settings;

Expand Down
Loading

0 comments on commit 2a7f3cd

Please sign in to comment.