From 185957c34d5f22abda6691fac25b66d132815d26 Mon Sep 17 00:00:00 2001 From: Daniel Marbach Date: Tue, 2 Jul 2024 09:49:02 +0200 Subject: [PATCH] AssemblyScanner doesn't scan message assemblies that reference Message Interfaces (#7091) * AssemblyScanner doesn't scan message assemblies that reference Message Interfaces (#7081) * 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 <155411597+PhilBastian@users.noreply.github.com> * Apply suggestions from code review Co-authored-by: Brandon Ording * string.Equals Co-authored-by: Brandon Ording --------- Co-authored-by: danielmarbach Co-authored-by: Phil Bastian <155411597+PhilBastian@users.noreply.github.com> Co-authored-by: Brandon Ording (cherry picked from commit 3a7c74ce7db86b1a2a2b22c15a68ed19e5970bba) * Set MessageInterfacesAssemblyName to null in tests * Compatible v8 settings for tests --------- Co-authored-by: Brandon Ording --- .../AssemblyScanner/AssemblyScannerTests.cs | 4 +- .../AssemblyScanningComponentTests.cs | 7 +-- ...ferencing_core_or_interfaces_is_scanned.cs | 23 ++++++++++ .../When_using_type_forwarding.cs | 43 ++++++++++++++++++ .../Messages/Messages.Referencing.Core.dll | Bin 0 -> 4096 bytes ...Messages.Referencing.MessageInterfaces.dll | Bin 0 -> 4608 bytes .../Hosting/Helpers/AssemblyScanner.cs | 17 ++++++- 7 files changed, 88 insertions(+), 6 deletions(-) create mode 100644 src/NServiceBus.Core.Tests/AssemblyScanner/When_directory_with_messages_referencing_core_or_interfaces_is_scanned.cs create mode 100644 src/NServiceBus.Core.Tests/AssemblyScanner/When_using_type_forwarding.cs create mode 100644 src/NServiceBus.Core.Tests/TestDlls/Messages/Messages.Referencing.Core.dll create mode 100644 src/NServiceBus.Core.Tests/TestDlls/Messages/Messages.Referencing.MessageInterfaces.dll diff --git a/src/NServiceBus.Core.Tests/AssemblyScanner/AssemblyScannerTests.cs b/src/NServiceBus.Core.Tests/AssemblyScanner/AssemblyScannerTests.cs index 3775c4a16b..d66d562fa9 100644 --- a/src/NServiceBus.Core.Tests/AssemblyScanner/AssemblyScannerTests.cs +++ b/src/NServiceBus.Core.Tests/AssemblyScanner/AssemblyScannerTests.cs @@ -96,7 +96,8 @@ public void Assemblies_which_reference_older_core_version_are_included() { ThrowExceptions = false, ScanAppDomainAssemblies = false, - CoreAssemblyName = busAssemblyV2.Name + CoreAssemblyName = busAssemblyV2.Name, + MessageInterfacesAssemblyName = null }; var result = scanner.GetScannableAssemblies(); @@ -320,6 +321,7 @@ static AssemblyScanner CreateDefaultAssemblyScanner(DynamicAssembly coreAssembly new AssemblyScanner(DynamicAssembly.TestAssemblyDirectory) { CoreAssemblyName = coreAssembly.DynamicName, + MessageInterfacesAssemblyName = null, ScanAppDomainAssemblies = true, ScanFileSystemAssemblies = true, ThrowExceptions = true diff --git a/src/NServiceBus.Core.Tests/AssemblyScanner/AssemblyScanningComponentTests.cs b/src/NServiceBus.Core.Tests/AssemblyScanner/AssemblyScanningComponentTests.cs index 250f79d3a9..f81de91795 100644 --- a/src/NServiceBus.Core.Tests/AssemblyScanner/AssemblyScanningComponentTests.cs +++ b/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); diff --git a/src/NServiceBus.Core.Tests/AssemblyScanner/When_directory_with_messages_referencing_core_or_interfaces_is_scanned.cs b/src/NServiceBus.Core.Tests/AssemblyScanner/When_directory_with_messages_referencing_core_or_interfaces_is_scanned.cs new file mode 100644 index 0000000000..f3f07ed23d --- /dev/null +++ b/src/NServiceBus.Core.Tests/AssemblyScanner/When_directory_with_messages_referencing_core_or_interfaces_is_scanned.cs @@ -0,0 +1,23 @@ +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")); + + 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"); + } +} \ No newline at end of file diff --git a/src/NServiceBus.Core.Tests/AssemblyScanner/When_using_type_forwarding.cs b/src/NServiceBus.Core.Tests/AssemblyScanner/When_using_type_forwarding.cs new file mode 100644 index 0000000000..a40703d89e --- /dev/null +++ b/src/NServiceBus.Core.Tests/AssemblyScanner/When_using_type_forwarding.cs @@ -0,0 +1,43 @@ +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")); + + var result = scanner.GetScannableAssemblies(); + var assemblyFullNames = result.Assemblies.Select(a => a.GetName().Name).ToList(); + + CollectionAssert.IsSubsetOf(assemblyNamesOfForwardedTypes, assemblyFullNames); + } +} \ No newline at end of file diff --git a/src/NServiceBus.Core.Tests/TestDlls/Messages/Messages.Referencing.Core.dll b/src/NServiceBus.Core.Tests/TestDlls/Messages/Messages.Referencing.Core.dll new file mode 100644 index 0000000000000000000000000000000000000000..ec17106ef11f8b4cf4f8283ce72f3fe13559eb73 GIT binary patch literal 4096 zcmeHKU2GIp6h5>4p%f|9DglgTpdcVJyDgv=LHo09S*1U8mx{)Oot?ei4$jVOW@ZZ; zlr&KZ`eb~O2Ms19#-I-*Dj^1<(U1_s&x_GWqA`)gL=yGECu6MNxii~tTT3F67!%L- zo_qf8x#yhwH|>A=RjMMQYP88oqO<53IwZb1nt-}t)kh8V!NSX{&nmkwuTG7bo?397 zoU7+mL$__mS2IF&i?(XoYJ4!M=AEq27B8+}8@e9qA=<4}(Jx1iohr}v9j#FpDKVlt zNNNM==z8=jS_c}@Qb|XGxLLsQOPBzLoS}-+FR>~AP4^+oBEX&SJIF)_(K#-}Y)BK` z0onC+M5)=joAGK8e%YiB{H`*CUh~BvAHB5zNJZmZ!?;07P>Ir->$wIv8CwSkPP87a z0y+@A<_gP!A;(H-H1^eiRskJET@|b(d%3Ovjx9npG^P?gS3{(*@D`vsvSASdynlAz zm3+KQ3B7WR<6E_uwk5WuwVfxaAq)5mOxkh;$sqPPT1(P*O*`kY#f!+BQ*YUuq+=j> zNn85%CgSLigTKyvm*r&8Ghn#Vw~S~ZGruWY2sG!k1P$hL&m&Dju(F)2!x+HpN|1Xk zEmnR|YH7d3IYhUF7Qm#IUZdwB?^VtL|3r-6mH48>o2C4)q&G!ARO;b>RivIaO57G% zC8r7I;EE{7rHPhBS|!a~dnDi@Qr8yhs%S{%_>L~Wf*BRsH-k&h;3BjQ*|a0eooMaI zxE+V5^y(t4g7?ZCxNRcX78yTRIWG(z}1G;{J5pn;@t$4x_Y6}>0|R{ioT#u&lCBKRZ5wDr8uR#IpO!>rV|Gp_u0Ajy{08bgzK4(Jy)5q zM;$k>`%voEyjIo7R^j9&%AL!=I~Q2fAIFRty(y z-^>ds_sxuHnSO;fS@L|5*FqyL3>Q_Tu*7pqt7R>Vf^lKT)whJf8IxxCj$4+;O+9Bj zo^KjnSyY}WNL0AtGI_OT6uKHNtujbGE-zx-bbbI1?{PAN6!ItM)kcEK&ur_c&*peL0K_vHg&(RPlvX)$7@Z|q(&a1mdB_t#e6H~Ei$fqg%)572;CC_(~4^qKU``G)kr2t{N&5ueo9LJ5J&eu z+Qavu@tYF@-$zYo{2bhX?nZ@~GLC4YJNX|{Nj7t!xxbgO);%9 zH5bY+XQ0`S`y_5z{kmyW-ZOBy2zdko?c_RmnO(xUF8_52zdl6Ei6&--61?2lRdeKQ z$KMY7I`G+1@%@yp+6w6iC22o!4-JD#;KdjKp8)O+zPjjq_0N9#PG5N4EAPuT2nOGGW z{{0v<%EAMG<88qGsL*cs%}K9ZfdZ9era2nJj3f_SR>m+G%6Wx2hH`zF3#$E3VhPr9 z2y-}yuZVT=E3&+`b&TQrP@$PVXIFf>-nfJ1+H}hOB9K!CS(Z?N`N!}&-9}J@{~5*} Nx{W^nclDpCz@HGsdTIaw literal 0 HcmV?d00001 diff --git a/src/NServiceBus.Core.Tests/TestDlls/Messages/Messages.Referencing.MessageInterfaces.dll b/src/NServiceBus.Core.Tests/TestDlls/Messages/Messages.Referencing.MessageInterfaces.dll new file mode 100644 index 0000000000000000000000000000000000000000..4b42adeb279ffeb7b880ca69f4fc5b65a804aec8 GIT binary patch literal 4608 zcmeHKO>7%g5T12@nx<{qC{XnW%C;$Op|Ey-3RpDYB+ic^PSV&7Eth(2zc^dgyVmYH zt%X!_p+Xg?9EuPUP~gM?P=qQ}RUjet0#YM!-~b0CR3T24;7~Y#)bh>Sb>buxMO8Ue z>Ns!S%)EK?X6DV?_mX_^O{ycJdbFE2i7ujNh)Mk4XcogQ4_#`Z3v1_hUQ|Zrcc!Ne zS1mbq!O@FqPPZ)EQ?o*K%9d(aYJ4oE7VW&yHf(6#9he>;AR1BXsQlqOSE{T1OuN)| zN`$Bh5^jLc_X4SCF*Kr0k`DQPvw;1VFbfPhLmg#aWmW!L>;slXfDG)8F%csgUNFLf z05U}DL2lVYl&;EF)2j=vOYJ7`!!`3XPn`6?Ut9x(F&SszH#i9@+|wM_$$^u;#XvNo zHKWx)4Bl&wFl{KZuarS!TQY9+0mO*bX88u*yEpbZZfU@O>JsJ>@xuRd8A3W9$uYPy68% zeFB0*YacqCh@<}!{B`F0OgoF70n?SC%|vUN`Ag{{&>Yi7G`7Pvk1z?&%5?G=<^W8^ zkG+wuQcI|jx)sI;=?i5eX;5jT0!=85v|V{0_%bn`mH0i0LsC92>50(WN;B*ahMMUS ziI0W`66?T?q#;{-f!inoydCF8qjow1 zyq`V*?xHKePt%XU{q!sF^HQDyuAg0AQ{N&5e#S|{{q&&h{ml5X9rSyZJ(#rfWm7yu ziGI6S)U7-vD^!G5uWlvDA>xyF5tfURE zR-D$Ig760M`4T5==cSeUgN7+4h2t8wwQ?|FP1#OS_b{lNx76ylttq2WcBI~-5d>2* zOevfYrhZaV?&5gW<8cRx$ayOV*nn7L7Fvlf-$Kf`LMl7ybYn8^b$Kfj_24;lip<4W&>+Xlo zUf;IukM+HuUauqvzfoJLUR9K)I+c_L5Us7uwlrONx$Wls<3Am0I)C}}(D^gF%qAtc ztak9BPKP$X;OM1M+Zs5T6D3YcdfKs1xC-?Ax*t+#t2R22P6f65Sg;2@Go4xl7FzCG zD8#tomQ1}ein?s&DQb0&3f27z?OENP;eJqP4a!UCuJD--e24MFS5Tn_$@oVP_`3_4 zGo&QP+1q(E6L+8)+{diMN3#u$uaGEERDNv11hDM-p@~#H)w#K}=Ir0mJv$jb_X(n~UqN1C#9U9f*uy0)E4r4(|X( z?t~wOg(%FL%-l);G|Iy^zZ!}5;d`pk2>dI^6?tb$RFN4f&@^&H^4Oh{{*484-gWk` znpfuh9sIxi^y_9E>)Y_Yj2-rBvUBI_CW5!BLQB_MzKe@{Nc5E4?_OEE0=dW{I~8Q^ c)^m98@4;Q20oDJX@xqgLbrkoUzs(H%0}qm|3jhEB literal 0 HcmV?d00001 diff --git a/src/NServiceBus.Core/Hosting/Helpers/AssemblyScanner.cs b/src/NServiceBus.Core/Hosting/Helpers/AssemblyScanner.cs index dac4d62ebb..c6754aa503 100644 --- a/src/NServiceBus.Core/Hosting/Helpers/AssemblyScanner.cs +++ b/src/NServiceBus.Core/Hosting/Helpers/AssemblyScanner.cs @@ -55,6 +55,8 @@ internal AssemblyScanner(Assembly assemblyToScan) internal string CoreAssemblyName { get; set; } = NServiceBusCoreAssemblyName; + internal string MessageInterfacesAssemblyName { get; set; } = NServiceBusMessageInterfacesAssemblyName; + internal IReadOnlyCollection AssembliesToSkip { set => assembliesToSkip = new HashSet(value.Select(RemoveExtension), StringComparer.OrdinalIgnoreCase); @@ -224,7 +226,8 @@ bool ScanAssembly(Assembly assembly, Dictionary processed) processed[assembly.FullName] = false; - if (assembly.GetName().Name == CoreAssemblyName) + var assemblyName = assembly.GetName(); + if (IsCoreOrMessageInterfaceAssembly(assemblyName)) { return processed[assembly.FullName] = true; } @@ -409,7 +412,7 @@ bool ShouldScanDependencies(Assembly assembly) var assemblyName = assembly.GetName(); - if (assemblyName.Name == CoreAssemblyName) + if (IsCoreOrMessageInterfaceAssembly(assemblyName)) { return false; } @@ -427,6 +430,15 @@ bool ShouldScanDependencies(Assembly assembly) return true; } + // 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); + AssemblyValidator assemblyValidator = new AssemblyValidator(); internal bool ScanNestedDirectories; Assembly assemblyToScan; @@ -434,6 +446,7 @@ bool ShouldScanDependencies(Assembly assembly) HashSet typesToSkip = new(); HashSet assembliesToSkip = new(StringComparer.OrdinalIgnoreCase); const string NServiceBusCoreAssemblyName = "NServiceBus.Core"; + const string NServiceBusMessageInterfacesAssemblyName = "NServiceBus.MessageInterfaces"; static readonly string[] FileSearchPatternsToUse = {