AssemblyScanner doesn't scan message assemblies that reference Message Interfaces (#7081) (#7089)

* Add a test to verify the messages referencing core are scanned

* Failing test for messages that reference message interfaces

* Unify in one test due avoid assembly loading issues

* Cleanup

* AssemblyScanner should scan assemblies that reference the message interfaces assembly to make sure messages using those interfaces can be discovered and do not act like unobtrusive messages

* Extract into method with a huge comment and inline hints

* Add a type forwarding test as a safety net

* Update src/NServiceBus.Core.Tests/AssemblyScanner/When_using_type_forwarding.cs

Co-authored-by: Phil Bastian <[email protected]>

* Apply suggestions from code review

Co-authored-by: Brandon Ording <[email protected]>

* string.Equals

Co-authored-by: Brandon Ording <[email protected]>

---------

Co-authored-by: danielmarbach <[email protected]>
Co-authored-by: Phil Bastian <[email protected]>
Co-authored-by: Brandon Ording <[email protected]>
(cherry picked from commit 3a7c74c)

Author: 4 people

src/NServiceBus.Core.Tests/AssemblyScanner/AssemblyScanningComponentTests.cs

@@ -14,9 +14,10 @@ public void Should_initialize_scanner_with_custom_path_when_provided()
         var settingsHolder = new SettingsHolder();
         settingsHolder.Set(new HostingComponent.Settings(settingsHolder));
 
-        var configuration = new AssemblyScanningComponent.Configuration(settingsHolder);
-
-        configuration.AssemblyScannerConfiguration.AdditionalAssemblyScanningPath = Path.Combine(TestContext.CurrentContext.TestDirectory, "TestDlls", "Nested", "Subfolder");
+        var configuration = new AssemblyScanningComponent.Configuration(settingsHolder)
+        {
+            AssemblyScannerConfiguration = { AdditionalAssemblyScanningPath = Path.Combine(TestContext.CurrentContext.TestDirectory, "TestDlls", "Nested", "Subfolder") }
+        };
 
         var component = AssemblyScanningComponent.Initialize(configuration, settingsHolder);
 

src/NServiceBus.Core.Tests/AssemblyScanner/When_directory_with_messages_referencing_core_or_interfaces_is_scanned.cs

@@ -0,0 +1,25 @@
+namespace NServiceBus.Core.Tests.AssemblyScanner;
+
+using System.IO;
+using System.Linq;
+using Hosting.Helpers;
+using NUnit.Framework;
+
+[TestFixture]
+public class When_directory_with_messages_referencing_core_or_interfaces_is_scanned
+{
+    [Test]
+    public void Assemblies_should_be_scanned()
+    {
+        var scanner = new AssemblyScanner(Path.Combine(TestContext.CurrentContext.TestDirectory, "TestDlls", "Messages"))
+        {
+            ScanAppDomainAssemblies = false
+        };
+
+        var result = scanner.GetScannableAssemblies();
+        var assemblyFullNames = result.Assemblies.Select(a => a.GetName().Name).ToList();
+
+        CollectionAssert.Contains(assemblyFullNames, "Messages.Referencing.Core");
+        CollectionAssert.Contains(assemblyFullNames, "Messages.Referencing.MessageInterfaces");
+    }
+}

src/NServiceBus.Core.Tests/AssemblyScanner/When_using_type_forwarding.cs

@@ -0,0 +1,46 @@
+namespace NServiceBus.Core.Tests.AssemblyScanner;
+
+using System.IO;
+using System.Linq;
+using System.Reflection.Metadata;
+using System.Reflection.PortableExecutable;
+using Hosting.Helpers;
+using NUnit.Framework;
+
+[TestFixture]
+public class When_using_type_forwarding
+{
+    // This test is not perfect since it relies on existing binaries to covered assembly scanning scenarios. Since we
+    // already use those though the idea of this test is to make sure that the assembly scanner is able to scan all
+    // assemblies that have a type forwarding rule within the core assembly. This might turn out to be a broad assumption
+    // in the future, and we might have to explicitly remove some but in the meantime this test would have covered us
+    // when we moved ICommand, IEvent and IMessages to the message interfaces assembly.
+    [Test]
+    public void Should_scan_assemblies_indicated_by_the_forwarding_metadata()
+    {
+        using var fs = File.OpenRead(typeof(AssemblyScanner).Assembly.Location);
+        using var peReader = new PEReader(fs);
+        var metadataReader = peReader.GetMetadataReader();
+
+        // Exported types only contains a small subset of types, so it's safe to enumerate all of them
+        var assemblyNamesOfForwardedTypes = metadataReader.ExportedTypes
+            .Select(exportedTypeHandle => metadataReader.GetExportedType(exportedTypeHandle))
+            .Where(exportedType => exportedType.IsForwarder)
+            .Select(exportedType => (AssemblyReferenceHandle)exportedType.Implementation)
+            .Select(assemblyReferenceHandle => metadataReader.GetAssemblyReference(assemblyReferenceHandle))
+            .Select(assemblyReference => metadataReader.GetString(assemblyReference.Name))
+            .Where(assemblyName => assemblyName.StartsWith("NServiceBus") || assemblyName.StartsWith("Particular"))
+            .Distinct()
+            .ToList();
+
+        var scanner = new AssemblyScanner(Path.Combine(TestContext.CurrentContext.TestDirectory, "TestDlls"))
+        {
+            ScanAppDomainAssemblies = false
+        };
+
+        var result = scanner.GetScannableAssemblies();
+        var assemblyFullNames = result.Assemblies.Select(a => a.GetName().Name).ToList();
+
+        CollectionAssert.IsSubsetOf(assemblyNamesOfForwardedTypes, assemblyFullNames);
+    }
+}

src/NServiceBus.Core.Tests/TestDlls/Messages/Messages.Referencing.Core.dll

@@ -53,6 +53,8 @@ internal AssemblyScanner(Assembly assemblyToScan)
 
     internal string CoreAssemblyName { get; set; } = NServiceBusCoreAssemblyName;
 
+    internal string MessageInterfacesAssemblyName { get; set; } = NServiceBusMessageInterfacesAssemblyName;
+
     internal IReadOnlyCollection<string> AssembliesToSkip
     {
         set => assembliesToSkip = new HashSet<string>(value.Select(RemoveExtension), StringComparer.OrdinalIgnoreCase);
@@ -219,7 +221,8 @@ bool ScanAssembly(Assembly assembly, Dictionary<string, bool> processed)
 
         processed[assembly.FullName] = false;
 
-        if (assembly.GetName().Name == CoreAssemblyName)
+        var assemblyName = assembly.GetName();
+        if (IsCoreOrMessageInterfaceAssembly(assemblyName))
         {
             return processed[assembly.FullName] = true;
         }
@@ -373,7 +376,7 @@ bool ShouldScanDependencies(Assembly assembly)
 
         var assemblyName = assembly.GetName();
 
-        if (assemblyName.Name == CoreAssemblyName)
+        if (IsCoreOrMessageInterfaceAssembly(assemblyName))
         {
             return false;
         }
@@ -387,12 +390,22 @@ bool ShouldScanDependencies(Assembly assembly)
         return !IsExcluded(assemblyName.Name);
     }
 
+    // We are deliberately checking here against the MessageInterfaces assembly name because
+    // the command, event, and message interfaces have been moved there by using type forwarding.
+    // While it would be possible to read the type forwarding information from the assembly, that imposes
+    // some performance overhead, and we don't expect that the assembly name will change nor that we will add many
+    // more type forwarding cases. Should that be the case we might want to revisit the idea of reading the metadata
+    // information from the assembly.
+    [MethodImpl(MethodImplOptions.AggressiveInlining)]
+    bool IsCoreOrMessageInterfaceAssembly(AssemblyName assemblyName) => string.Equals(assemblyName.Name, CoreAssemblyName, StringComparison.Ordinal) || string.Equals(assemblyName.Name, MessageInterfacesAssemblyName, StringComparison.Ordinal);
+
     internal bool ScanNestedDirectories;
     readonly Assembly assemblyToScan;
     readonly string baseDirectoryToScan;
     HashSet<Type> typesToSkip = [];
     HashSet<string> assembliesToSkip = new(StringComparer.OrdinalIgnoreCase);
     const string NServiceBusCoreAssemblyName = "NServiceBus.Core";
+    const string NServiceBusMessageInterfacesAssemblyName = "NServiceBus.MessageInterfaces";
 
     static readonly string[] FileSearchPatternsToUse =
     {