AssemblyScanner improvements (#6822)

Particular · Aug 31, 2023 · 9f1563f · 9f1563f
1 parent a95ce68
commit 9f1563f
Show file tree

Hide file tree

Showing 2 changed files with 52 additions and 60 deletions.
diff --git a/src/NServiceBus.Core.Tests/AssemblyScanner/AssemblyScannerTests.cs b/src/NServiceBus.Core.Tests/AssemblyScanner/AssemblyScannerTests.cs
@@ -273,8 +273,11 @@ public void Skipped_dlls_should_be_excluded()
             });
 
             var scanner = CreateDefaultAssemblyScanner(busAssembly);
-            scanner.AssembliesToSkip.Add(excludedAssembly1.DynamicName); // without file extension
-            scanner.AssembliesToSkip.Add(excludedAssembly2.FileName); // with file extension
+            scanner.AssembliesToSkip = new[]
+            {
+                excludedAssembly1.DynamicName, // without file extension
+                excludedAssembly2.FileName // with file extension
+            };
 
             var result = scanner.GetScannableAssemblies();
             Assert.That(result.SkippedFiles.Any(s => s.FilePath == excludedAssembly1.FilePath));
@@ -301,8 +304,11 @@ public void Skipped_exes_should_be_excluded()
             }, executable: true);
 
             var scanner = CreateDefaultAssemblyScanner(busAssembly);
-            scanner.AssembliesToSkip.Add(excludedAssembly1.DynamicName); // without file extension
-            scanner.AssembliesToSkip.Add(excludedAssembly2.FileName); // with file extension
+            scanner.AssembliesToSkip = new[]
+            {
+                excludedAssembly1.DynamicName, // without file extension
+                excludedAssembly2.FileName // with file extension
+            };
 
             var result = scanner.GetScannableAssemblies();
             Assert.That(result.SkippedFiles.Any(s => s.FilePath == excludedAssembly1.FilePath));

diff --git a/src/NServiceBus.Core/Hosting/Helpers/AssemblyScanner.cs b/src/NServiceBus.Core/Hosting/Helpers/AssemblyScanner.cs
@@ -53,7 +53,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; }
 
@@ -253,7 +263,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();
@@ -334,65 +344,42 @@ 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)
             {
@@ -405,7 +392,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);
         }
@@ -437,22 +424,21 @@ bool ShouldScanDependencies(Assembly assembly)
             return true;
         }
 
-        readonly AssemblyValidator assemblyValidator = new AssemblyValidator();
-        internal List<string> AssembliesToSkip = new List<string>();
+        AssemblyValidator assemblyValidator = new AssemblyValidator();
         internal bool ScanNestedDirectories;
-        internal List<Type> TypesToSkip = new List<Type>();
-        readonly Assembly assemblyToScan;
-        readonly string baseDirectoryToScan;
-        const string NServicebusCoreAssemblyName = "NServiceBus.Core";
+        Assembly assemblyToScan;
+        string baseDirectoryToScan;
+        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",