From 5d7924a3d90418b97d88e48941ed6b7fdbd02c84 Mon Sep 17 00:00:00 2001 From: Morten Holt Date: Wed, 22 Apr 2026 20:56:55 +0200 Subject: [PATCH] Add: When specifying a Pre or Post image without filtering the attributes, we now generate a Pre or Post image wrapper with all attributes that have a logical name spacified. Add: Warning to avoid image registrations without attributes --- .../DiagnosticReportingTests.cs | 50 +++++++++++ .../WrapperClassGenerationTests.cs | 78 +++++++++++++++++ .../Helpers/TestFixtures.cs | 84 +++++++++++++++++++ .../AnalyzerReleases.Unshipped.md | 1 + .../Analyzers/FullEntityImageAnalyzer.cs | 47 +++++++++++ .../DiagnosticDescriptors.cs | 10 +++ .../Parsers/RegistrationParser.cs | 20 +++++ .../rules/XPC3005.md | 82 ++++++++++++++++++ 8 files changed, 372 insertions(+) create mode 100644 XrmPluginCore.SourceGenerator/Analyzers/FullEntityImageAnalyzer.cs create mode 100644 XrmPluginCore.SourceGenerator/rules/XPC3005.md diff --git a/XrmPluginCore.SourceGenerator.Tests/DiagnosticTests/DiagnosticReportingTests.cs b/XrmPluginCore.SourceGenerator.Tests/DiagnosticTests/DiagnosticReportingTests.cs index 3e62896..b5caa78 100644 --- a/XrmPluginCore.SourceGenerator.Tests/DiagnosticTests/DiagnosticReportingTests.cs +++ b/XrmPluginCore.SourceGenerator.Tests/DiagnosticTests/DiagnosticReportingTests.cs @@ -1064,6 +1064,56 @@ public void HandleUpdate() { } "Namespace2 source should not contain Namespace1 namespace declaration"); } + [Fact] + public async Task Should_Report_XPC3005_When_WithPreImage_Has_No_Arguments() + { + var source = TestFixtures.GetCompleteSource(TestFixtures.PluginWithFullEntityPreImage); + + var diagnostics = await GetAnalyzerDiagnosticsAsync(source, new FullEntityImageAnalyzer()); + + var warnings = diagnostics.Where(d => d.Id == "XPC3005").ToArray(); + + warnings.Should().NotBeEmpty("XPC3005 should be reported when WithPreImage() is called with no arguments"); + warnings.Should().OnlyContain(d => d.Severity == DiagnosticSeverity.Warning); + } + + [Fact] + public async Task Should_Report_XPC3005_When_WithPostImage_Has_No_Arguments() + { + var source = TestFixtures.GetCompleteSource(TestFixtures.PluginWithFullEntityPostImage); + + var diagnostics = await GetAnalyzerDiagnosticsAsync(source, new FullEntityImageAnalyzer()); + + var warnings = diagnostics.Where(d => d.Id == "XPC3005").ToArray(); + + warnings.Should().NotBeEmpty("XPC3005 should be reported when WithPostImage() is called with no arguments"); + warnings.Should().OnlyContain(d => d.Severity == DiagnosticSeverity.Warning); + } + + [Fact] + public async Task Should_Not_Report_XPC3005_When_WithPreImage_Has_Arguments() + { + var source = TestFixtures.GetCompleteSource(TestFixtures.GetPluginWithPreImage()); + + var diagnostics = await GetAnalyzerDiagnosticsAsync(source, new FullEntityImageAnalyzer()); + + var warnings = diagnostics.Where(d => d.Id == "XPC3005").ToArray(); + + warnings.Should().BeEmpty("XPC3005 should NOT be reported when WithPreImage() is called with specific attributes"); + } + + [Fact] + public async Task Should_Not_Report_XPC3005_When_WithPostImage_Has_Arguments() + { + var source = TestFixtures.GetCompleteSource(TestFixtures.PluginWithPostImage); + + var diagnostics = await GetAnalyzerDiagnosticsAsync(source, new FullEntityImageAnalyzer()); + + var warnings = diagnostics.Where(d => d.Id == "XPC3005").ToArray(); + + warnings.Should().BeEmpty("XPC3005 should NOT be reported when WithPostImage() is called with specific attributes"); + } + private static async Task> GetAnalyzerDiagnosticsAsync(string source, DiagnosticAnalyzer analyzer) { var compilation = CompilationHelper.CreateCompilation(source); diff --git a/XrmPluginCore.SourceGenerator.Tests/GenerationTests/WrapperClassGenerationTests.cs b/XrmPluginCore.SourceGenerator.Tests/GenerationTests/WrapperClassGenerationTests.cs index 2c035b9..9e08f7e 100644 --- a/XrmPluginCore.SourceGenerator.Tests/GenerationTests/WrapperClassGenerationTests.cs +++ b/XrmPluginCore.SourceGenerator.Tests/GenerationTests/WrapperClassGenerationTests.cs @@ -330,6 +330,84 @@ public void Should_Parse_Parameterless_Method_Reference() generatedSource.Should().Contain("service.HandleUpdate()"); } + [Fact] + public void Should_Generate_PreImage_With_All_Entity_Properties_When_No_Attributes_Specified() + { + // Arrange - WithPreImage() called with no arguments + var source = TestFixtures.GetCompleteSource( + TestFixtures.PluginWithFullEntityPreImage); + + // Act + var result = GeneratorTestHelper.RunGenerator( + CompilationHelper.CreateCompilation(source)); + + // Assert + result.GeneratedTrees.Should().NotBeEmpty("code should be generated for full entity PreImage"); + var generatedSource = result.GeneratedTrees[0].GetText().ToString(); + + // Verify PreImage class is generated + generatedSource.Should().Contain($"public sealed class PreImage : IEntityImageWrapper<{ContextNamespace}.Account>"); + + // Verify that multiple entity properties are present (full entity = all properties) + generatedSource.Should().Contain("public string? Name => Entity.Name;"); + generatedSource.Should().Contain("public decimal? Revenue => Entity.Revenue;"); + generatedSource.Should().Contain("public string? AccountNumber => Entity.AccountNumber;"); + + // Verify NO PostImage class is generated + generatedSource.Should().NotContain("public sealed class PostImage"); + } + + [Fact] + public void Should_Generate_PostImage_With_All_Entity_Properties_When_No_Attributes_Specified() + { + // Arrange - WithPostImage() called with no arguments + var source = TestFixtures.GetCompleteSource( + TestFixtures.PluginWithFullEntityPostImage); + + // Act + var result = GeneratorTestHelper.RunGenerator( + CompilationHelper.CreateCompilation(source)); + + // Assert + result.GeneratedTrees.Should().NotBeEmpty("code should be generated for full entity PostImage"); + var generatedSource = result.GeneratedTrees[0].GetText().ToString(); + + // Verify PostImage class is generated + generatedSource.Should().Contain($"public sealed class PostImage : IEntityImageWrapper<{ContextNamespace}.Account>"); + + // Verify that multiple entity properties are present (full entity = all properties) + generatedSource.Should().Contain("public string? Name => Entity.Name;"); + generatedSource.Should().Contain("public decimal? Revenue => Entity.Revenue;"); + generatedSource.Should().Contain("public string? AccountNumber => Entity.AccountNumber;"); + + // Verify NO PreImage class is generated + generatedSource.Should().NotContain("public sealed class PreImage"); + } + + [Fact] + public void Should_Generate_ActionWrapper_With_Full_Entity_PreImage_Call() + { + // Arrange - WithPreImage() called with no arguments + var source = TestFixtures.GetCompleteSource( + TestFixtures.PluginWithFullEntityPreImage); + + // Act + var result = GeneratorTestHelper.RunGenerator( + CompilationHelper.CreateCompilation(source)); + + // Assert + var generatedSource = result.GeneratedTrees[0].GetText().ToString(); + + // Verify ActionWrapper is generated and handles the PreImage + generatedSource.Should().Contain("internal sealed class ActionWrapper : IActionWrapper"); + generatedSource.Should().Contain("var preImageEntity = context?.PreEntityImages?.Values?.FirstOrDefault();"); + generatedSource.Should().Contain("var preImage = preImageEntity != null ? new PreImage(preImageEntity) : null;"); + generatedSource.Should().Contain("service.HandleAccountUpdate(preImage)"); + + // Should NOT have PostImage retrieval + generatedSource.Should().NotContain("PostEntityImages"); + } + [System.Text.RegularExpressions.GeneratedRegex(@"namespace\s+TestNamespace\.PluginRegistrations\.TestPlugin\.AccountUpdatePostOperation")] private static partial System.Text.RegularExpressions.Regex IsAccountUpdatePostOperationNamespace(); diff --git a/XrmPluginCore.SourceGenerator.Tests/Helpers/TestFixtures.cs b/XrmPluginCore.SourceGenerator.Tests/Helpers/TestFixtures.cs index c74e2be..af75d06 100644 --- a/XrmPluginCore.SourceGenerator.Tests/Helpers/TestFixtures.cs +++ b/XrmPluginCore.SourceGenerator.Tests/Helpers/TestFixtures.cs @@ -198,6 +198,90 @@ public void HandleUpdate() { } } """; + /// + /// Plugin with full entity PreImage (no attributes specified — captures all entity attributes). + /// + public const string PluginWithFullEntityPreImage = """ + + using XrmPluginCore; + using XrmPluginCore.Abstractions; + using XrmPluginCore.Enums; + using Microsoft.Extensions.DependencyInjection; + using TestNamespace; + using TestNamespace.PluginRegistrations.TestPlugin.AccountUpdatePostOperation; + + namespace TestNamespace + { + public class TestPlugin : Plugin + { + public TestPlugin() + { + RegisterStep(EventOperation.Update, ExecutionStage.PostOperation, + nameof(ITestService.HandleAccountUpdate)) + .AddFilteredAttributes(x => x.Name) + .WithPreImage(); + } + + protected override IServiceCollection OnBeforeBuildServiceProvider(IServiceCollection services) + { + return services.AddScoped(); + } + } + + public interface ITestService + { + void HandleAccountUpdate(PreImage preImage); + } + + public class TestService : ITestService + { + public void HandleAccountUpdate(PreImage preImage) { } + } + } + """; + + /// + /// Plugin with full entity PostImage (no attributes specified — captures all entity attributes). + /// + public const string PluginWithFullEntityPostImage = """ + + using XrmPluginCore; + using XrmPluginCore.Abstractions; + using XrmPluginCore.Enums; + using Microsoft.Extensions.DependencyInjection; + using TestNamespace; + using TestNamespace.PluginRegistrations.TestPlugin.AccountUpdatePostOperation; + + namespace TestNamespace + { + public class TestPlugin : Plugin + { + public TestPlugin() + { + RegisterStep(EventOperation.Update, ExecutionStage.PostOperation, + nameof(ITestService.HandleAccountUpdate)) + .AddFilteredAttributes(x => x.Name) + .WithPostImage(); + } + + protected override IServiceCollection OnBeforeBuildServiceProvider(IServiceCollection services) + { + return services.AddScoped(); + } + } + + public interface ITestService + { + void HandleAccountUpdate(PostImage postImage); + } + + public class TestService : ITestService + { + public void HandleAccountUpdate(PostImage postImage) { } + } + } + """; + /// /// Plugin using old AddImage API for backward compatibility testing. /// diff --git a/XrmPluginCore.SourceGenerator/AnalyzerReleases.Unshipped.md b/XrmPluginCore.SourceGenerator/AnalyzerReleases.Unshipped.md index 5ce08df..774e5a1 100644 --- a/XrmPluginCore.SourceGenerator/AnalyzerReleases.Unshipped.md +++ b/XrmPluginCore.SourceGenerator/AnalyzerReleases.Unshipped.md @@ -3,6 +3,7 @@ Rule ID | Category | Severity | Notes --------|----------|----------|------- XPC3004 | XrmPluginCore.SourceGenerator | Error | Do not use LocalPluginContext as TService in RegisterStep +XPC3005 | XrmPluginCore.SourceGenerator | Warning | Full entity image registration without specifying attributes ### Removed Rules diff --git a/XrmPluginCore.SourceGenerator/Analyzers/FullEntityImageAnalyzer.cs b/XrmPluginCore.SourceGenerator/Analyzers/FullEntityImageAnalyzer.cs new file mode 100644 index 0000000..72f82d0 --- /dev/null +++ b/XrmPluginCore.SourceGenerator/Analyzers/FullEntityImageAnalyzer.cs @@ -0,0 +1,47 @@ +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp; +using Microsoft.CodeAnalysis.CSharp.Syntax; +using Microsoft.CodeAnalysis.Diagnostics; +using System.Collections.Immutable; + +namespace XrmPluginCore.SourceGenerator.Analyzers; + +/// +/// Analyzer that reports XPC3005 when WithPreImage or WithPostImage is called +/// without specifying any attributes, resulting in a full entity image registration. +/// +[DiagnosticAnalyzer(LanguageNames.CSharp)] +public class FullEntityImageAnalyzer : DiagnosticAnalyzer +{ + public override ImmutableArray SupportedDiagnostics => + ImmutableArray.Create(DiagnosticDescriptors.FullEntityImage); + + public override void Initialize(AnalysisContext context) + { + context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None); + context.EnableConcurrentExecution(); + context.RegisterSyntaxNodeAction(AnalyzeInvocation, SyntaxKind.InvocationExpression); + } + + private void AnalyzeInvocation(SyntaxNodeAnalysisContext context) + { + var invocation = (InvocationExpressionSyntax)context.Node; + + if (invocation.Expression is not MemberAccessExpressionSyntax memberAccess) + return; + + var methodName = memberAccess.Name.Identifier.Text; + + if (methodName != Constants.WithPreImageMethodName && methodName != Constants.WithPostImageMethodName) + return; + + // Only report when called with no arguments (full entity image) + if (invocation.ArgumentList.Arguments.Count > 0) + return; + + context.ReportDiagnostic(Diagnostic.Create( + DiagnosticDescriptors.FullEntityImage, + invocation.GetLocation(), + methodName)); + } +} diff --git a/XrmPluginCore.SourceGenerator/DiagnosticDescriptors.cs b/XrmPluginCore.SourceGenerator/DiagnosticDescriptors.cs index 9b820da..f533dcc 100644 --- a/XrmPluginCore.SourceGenerator/DiagnosticDescriptors.cs +++ b/XrmPluginCore.SourceGenerator/DiagnosticDescriptors.cs @@ -106,6 +106,16 @@ public static class DiagnosticDescriptors DiagnosticSeverity.Warning, isEnabledByDefault: true); + public static readonly DiagnosticDescriptor FullEntityImage = new( + id: "XPC3005", + title: "Full entity image registration", + messageFormat: "'{0}' registered without specifying attributes will capture all entity attributes. Consider specifying only the attributes your handler needs for better performance.", + category: Category, + defaultSeverity: DiagnosticSeverity.Warning, + isEnabledByDefault: true, + description: "Registering an image without specifying attributes causes Dynamics 365 to serialize all entity attributes into the image, which may impact performance. Specify only the attributes your handler needs.", + helpLinkUri: $"{HelpLinkBaseUri}/XPC3005.md"); + public static readonly DiagnosticDescriptor GenerationError = new( id: "XPC5002", title: "Failed to generate wrapper classes", diff --git a/XrmPluginCore.SourceGenerator/Parsers/RegistrationParser.cs b/XrmPluginCore.SourceGenerator/Parsers/RegistrationParser.cs index 12104fe..65f8d1f 100644 --- a/XrmPluginCore.SourceGenerator/Parsers/RegistrationParser.cs +++ b/XrmPluginCore.SourceGenerator/Parsers/RegistrationParser.cs @@ -244,9 +244,29 @@ private static ImageMetadata ParseImageInvocation( imageMetadata.ImageName = imageMetadata.ImageType; } + // For WithPreImage/WithPostImage with no attributes specified, capture all entity attributes + if (!imageMetadata.Attributes.Any() && + (methodName == Constants.WithPreImageMethodName || methodName == Constants.WithPostImageMethodName)) + { + imageMetadata.Attributes.AddRange(GetAllEntityAttributes(entityType)); + } + return imageMetadata.Attributes.Any() ? imageMetadata : null; } + /// + /// Gets all attribute metadata for all entity properties that have an AttributeLogicalName attribute. + /// Used for full entity images where no specific attributes are specified. + /// + private static IEnumerable GetAllEntityAttributes(ITypeSymbol entityType) + { + return entityType.GetMembers() + .OfType() + .Where(p => p.GetAttributes().Any(a => a.AttributeClass?.Name == Constants.LogicalNameAttributeName)) + .Select(p => GetAttributeMetadata(p.Name, entityType)) + .Where(a => a != null); + } + /// /// Gets attribute metadata (property name, logical name, type) for a property /// diff --git a/XrmPluginCore.SourceGenerator/rules/XPC3005.md b/XrmPluginCore.SourceGenerator/rules/XPC3005.md new file mode 100644 index 0000000..ef001d5 --- /dev/null +++ b/XrmPluginCore.SourceGenerator/rules/XPC3005.md @@ -0,0 +1,82 @@ +# XPC3005: Full entity image registration + +## Severity + +Warning + +## Description + +This rule reports when `WithPreImage()` or `WithPostImage()` is called without specifying any attributes, resulting in a **full entity image** registration. A full entity image causes Dynamics 365 to serialize **all** entity attributes into the image for every plugin execution, regardless of which attributes your handler actually uses. This can be a significant performance overhead, especially for entities with many attributes. + +The source generator will still generate a wrapper class containing all entity properties, but the warning serves as a reminder to narrow down the attributes if possible. + +## ❌ Example of violation + +```csharp +public class AccountPlugin : Plugin +{ + public AccountPlugin() + { + // XPC3005: WithPreImage registered without specifying attributes + RegisterStep( + EventOperation.Update, + ExecutionStage.PostOperation, + nameof(IAccountService.HandleUpdate)) + .AddFilteredAttributes(x => x.Name) + .WithPreImage(); // No attributes — captures the entire entity + } +} + +public interface IAccountService +{ + void HandleUpdate(PreImage preImage); +} +``` + +## ✅ How to fix + +Specify only the attributes your handler actually needs: + +```csharp +public class AccountPlugin : Plugin +{ + public AccountPlugin() + { + // Correct: specify only the attributes your handler needs + RegisterStep( + EventOperation.Update, + ExecutionStage.PostOperation, + nameof(IAccountService.HandleUpdate)) + .AddFilteredAttributes(x => x.Name) + .WithPreImage(x => x.Name, x => x.Revenue); // Only what's needed + } +} + +public interface IAccountService +{ + void HandleUpdate(PreImage preImage); +} +``` + +### When full entity images are acceptable + +In some cases you genuinely need all entity attributes in your image — for example, when auditing or archiving the full state of a record. If this is intentional, you can suppress the warning: + +```csharp +#pragma warning disable XPC3005 + .WithPreImage() // Intentional: full entity audit image +#pragma warning restore XPC3005 +``` + +## Why this matters + +1. **Serialization overhead**: Dynamics 365 serializes every attribute in the registered image columns, even attributes your handler never reads. On entities with many attributes (some have 100+), this adds measurable latency to every plugin execution. + +2. **Network bandwidth**: In cloud deployments, the serialized image is transmitted across the wire. Fewer attributes means smaller payloads. + +3. **Explicit intent**: Specifying attributes makes the handler's dependencies explicit and visible in the registration, making the code easier to understand and audit. + +## See also + +- [XPC3001: Prefer nameof over string literal](XPC3001.md) +- [XPC3003: Image registration without method reference](XPC3003.md)