From 6c11b31e5386d069f491f53d594e50875978f46a Mon Sep 17 00:00:00 2001 From: Morten Holt Date: Tue, 4 Nov 2025 16:03:05 +0100 Subject: [PATCH 1/3] Add list of illegal column names, and ensure the generated code postfixes a counter if they are used. --- .../Generation/Mappers/ProxyClassMapper.cs | 75 +++++- .../ProxyClassMapperTests.cs | 246 ++++++++++++++++++ 2 files changed, 314 insertions(+), 7 deletions(-) diff --git a/src/DataverseProxyGenerator.Core/Generation/Mappers/ProxyClassMapper.cs b/src/DataverseProxyGenerator.Core/Generation/Mappers/ProxyClassMapper.cs index b25923a..9f9301e 100644 --- a/src/DataverseProxyGenerator.Core/Generation/Mappers/ProxyClassMapper.cs +++ b/src/DataverseProxyGenerator.Core/Generation/Mappers/ProxyClassMapper.cs @@ -5,6 +5,43 @@ namespace DataverseProxyGenerator.Core.Generation.Mappers; public static class ProxyClassMapper { + /// + /// Non-virtual public members of Microsoft.Xrm.Sdk.Entity base class that would conflict with generated properties. + /// These CANNOT be overridden and must be renamed if a column has the same name. + /// Note: Virtual members like "Id" are NOT included here as they can be overridden without conflict. + /// + private static readonly HashSet EntityBaseClassNonVirtualMembers = new(StringComparer.Ordinal) + { + // Non-virtual properties from Entity class + "Attributes", + "EntityState", + "ExtensionData", + "FormattedValues", + "KeyAttributes", + "LogicalName", + "RelatedEntities", + "RowVersion", + "HasLazyFileAttribute", + "LazyFileAttributeKey", + "LazyFileAttributeValue", + "LazyFileSizeAttributeKey", + "LazyFileSizeAttributeValue", + + // Methods from Entity class (properties should not use these names either) + "Contains", + "GetAttributeValue", + "GetFormattedAttributeValue", + "GetRelatedEntities", + "GetRelatedEntity", + "SetAttributeValue", + "SetRelatedEntities", + "SetRelatedEntity", + "ToEntity", + "ToEntityReference", + "TryGetAttributeValue", + "ShallowCopyTo", + }; + public static object MapToTemplateModel((TableModel Table, IReadOnlyList Interfaces) input, GenerationContext context) { ArgumentNullException.ThrowIfNull(context); @@ -14,10 +51,13 @@ public static object MapToTemplateModel((TableModel Table, IReadOnlyList var processedColumns = ProcessColumnsWithClassNameConflictResolution(table.Columns, table.SchemaName); - if (table.SchemaName == "EnvironmentVariableDefinition") { - foreach (var key in table.Keys) { + if (table.SchemaName == "EnvironmentVariableDefinition") + { + foreach (var key in table.Keys) + { Console.WriteLine(key.SchemaName); - foreach (var attr in key.KeyAttributes) { + foreach (var attr in key.KeyAttributes) + { Console.WriteLine($" {attr.SchemaName} : {attr.TypeName}"); } } @@ -45,6 +85,8 @@ public static object MapToTemplateModel((TableModel Table, IReadOnlyList private static IEnumerable ProcessColumnsWithClassNameConflictResolution(IEnumerable columns, string className) { + var usedNames = new HashSet(StringComparer.Ordinal); + return columns.Select(c => { var sanitizedColumn = c switch @@ -60,14 +102,33 @@ private static IEnumerable ProcessColumnsWithClassNameConflictResol }, }; + var finalName = sanitizedColumn.SchemaName; + // Check if sanitized schema name conflicts with class name (case-sensitive) - if (string.Equals(sanitizedColumn.SchemaName, className, StringComparison.Ordinal)) + if (string.Equals(finalName, className, StringComparison.Ordinal)) { - var finalName = $"{sanitizedColumn.SchemaName}_1"; - return sanitizedColumn with { SchemaName = finalName }; + finalName = $"{finalName}_1"; } - return sanitizedColumn; + // Check if name conflicts with non-virtual Entity base class members (case-sensitive) + // Virtual members can be overridden, so they don't cause conflicts + if (EntityBaseClassNonVirtualMembers.Contains(finalName)) + { + finalName = $"{finalName}_1"; + } + + // Ensure the final name is unique (handle edge case where _1 suffix also conflicts) + var candidateName = finalName; + var suffix = 1; + while (usedNames.Contains(candidateName)) + { + suffix++; + candidateName = $"{finalName}_{suffix}"; + } + + usedNames.Add(candidateName); + + return sanitizedColumn with { SchemaName = candidateName }; }); } } \ No newline at end of file diff --git a/tests/DataverseProxyGenerator.Tests/ProxyClassMapperTests.cs b/tests/DataverseProxyGenerator.Tests/ProxyClassMapperTests.cs index c4ad270..acfa047 100644 --- a/tests/DataverseProxyGenerator.Tests/ProxyClassMapperTests.cs +++ b/tests/DataverseProxyGenerator.Tests/ProxyClassMapperTests.cs @@ -151,6 +151,252 @@ public void MapToTemplateModel_WithCaseSensitiveClassNameConflict_AppliesRenamin Assert.Contains(resultColumns, c => c.SchemaName == "testentity"); } + [Fact] + public void MapToTemplateModel_WithEntityBaseClassPropertyConflicts_AppendsUnderscoreOne() + { + // Arrange + var table = new TableModel + { + SchemaName = "TestEntity", + LogicalName = "testentity", + DisplayName = "Test Entity", + Description = "Test entity", + EntityTypeCode = 10001, + PrimaryNameAttribute = "name", + PrimaryIdAttribute = "testentityid", + IsIntersect = false, + Columns = new ColumnModel[] + { + new StringColumnModel + { + SchemaName = "Attributes", // Conflicts with Entity.Attributes (non-virtual) + LogicalName = "attributes", + DisplayName = "Attributes", + MaxLength = 100, + }, + new StringColumnModel + { + SchemaName = "Id", // Does NOT conflict - Entity.Id is virtual and can be overridden + LogicalName = "id", + DisplayName = "Id", + MaxLength = 100, + }, + new StringColumnModel + { + SchemaName = "LogicalName", // Conflicts with Entity.LogicalName (non-virtual) + LogicalName = "logicalname", + DisplayName = "Logical Name", + MaxLength = 100, + }, + }, + Relationships = new List(), + }; + + var context = CreateTestContext(); + + // Act + var result = ProxyClassMapper.MapToTemplateModel((table, new List()), context); + var resultColumns = GetColumnsFromResult(result); + + // Assert + Assert.Equal(3, resultColumns.Count); + Assert.Contains(resultColumns, c => c.SchemaName == "Attributes_1"); // Renamed due to conflict + Assert.Contains(resultColumns, c => c.SchemaName == "Id"); // NOT renamed - virtual property can be overridden + Assert.Contains(resultColumns, c => c.SchemaName == "LogicalName_1"); // Renamed due to conflict + } + + [Fact] + public void MapToTemplateModel_WithEntityBaseClassMethodConflicts_AppendsUnderscoreOne() + { + // Arrange + var table = new TableModel + { + SchemaName = "TestEntity", + LogicalName = "testentity", + DisplayName = "Test Entity", + Description = "Test entity", + EntityTypeCode = 10001, + PrimaryNameAttribute = "name", + PrimaryIdAttribute = "testentityid", + IsIntersect = false, + Columns = new ColumnModel[] + { + new StringColumnModel + { + SchemaName = "Contains", // Conflicts with Entity.Contains method + LogicalName = "contains", + DisplayName = "Contains", + MaxLength = 100, + }, + new StringColumnModel + { + SchemaName = "ToEntity", // Conflicts with Entity.ToEntity method + LogicalName = "toentity", + DisplayName = "To Entity", + MaxLength = 100, + }, + }, + Relationships = new List(), + }; + + var context = CreateTestContext(); + + // Act + var result = ProxyClassMapper.MapToTemplateModel((table, new List()), context); + var resultColumns = GetColumnsFromResult(result); + + // Assert + Assert.Equal(2, resultColumns.Count); + Assert.Contains(resultColumns, c => c.SchemaName == "Contains_1"); + Assert.Contains(resultColumns, c => c.SchemaName == "ToEntity_1"); + } + + [Fact] + public void MapToTemplateModel_WithCaseSensitiveEntityBaseClassConflict_OnlyRenamesNonVirtualExactMatch() + { + // Arrange + var table = new TableModel + { + SchemaName = "TestEntity", + LogicalName = "testentity", + DisplayName = "Test Entity", + Description = "Test entity", + EntityTypeCode = 10001, + PrimaryNameAttribute = "name", + PrimaryIdAttribute = "testentityid", + IsIntersect = false, + Columns = new ColumnModel[] + { + new StringColumnModel + { + SchemaName = "Attributes", // Exact match with non-virtual property - should be renamed + LogicalName = "attributes", + DisplayName = "Attributes", + MaxLength = 100, + }, + new StringColumnModel + { + SchemaName = "attributes", // Different case - no conflict + LogicalName = "attributes_lower", + DisplayName = "attributes lower", + MaxLength = 100, + }, + new StringColumnModel + { + SchemaName = "ATTRIBUTES", // Different case - no conflict + LogicalName = "attributes_upper", + DisplayName = "ATTRIBUTES upper", + MaxLength = 100, + }, + }, + Relationships = new List(), + }; + + var context = CreateTestContext(); + + // Act + var result = ProxyClassMapper.MapToTemplateModel((table, new List()), context); + var resultColumns = GetColumnsFromResult(result); + + // Assert + Assert.Equal(3, resultColumns.Count); + Assert.Contains(resultColumns, c => c.SchemaName == "Attributes_1"); // Exact match renamed + Assert.Contains(resultColumns, c => c.SchemaName == "attributes"); // Different case kept + Assert.Contains(resultColumns, c => c.SchemaName == "ATTRIBUTES"); // Different case kept + } + + [Fact] + public void MapToTemplateModel_WithVirtualEntityBaseClassProperty_DoesNotRename() + { + // Arrange + var table = new TableModel + { + SchemaName = "TestEntity", + LogicalName = "testentity", + DisplayName = "Test Entity", + Description = "Test entity", + EntityTypeCode = 10001, + PrimaryNameAttribute = "name", + PrimaryIdAttribute = "testentityid", + IsIntersect = false, + Columns = new ColumnModel[] + { + new StringColumnModel + { + SchemaName = "Id", // Entity.Id is virtual - can be overridden, should NOT be renamed + LogicalName = "id", + DisplayName = "Id", + MaxLength = 100, + }, + }, + Relationships = new List(), + }; + + var context = CreateTestContext(); + + // Act + var result = ProxyClassMapper.MapToTemplateModel((table, new List()), context); + var resultColumns = GetColumnsFromResult(result); + + // Assert + Assert.Single(resultColumns); + Assert.Contains(resultColumns, c => c.SchemaName == "Id"); // NOT renamed - virtual property + } + + [Fact] + public void MapToTemplateModel_WithMultipleConflictTypes_AppliesAllRenames() + { + // Arrange + var table = new TableModel + { + SchemaName = "Account", + LogicalName = "account", + DisplayName = "Account", + Description = "Test account", + EntityTypeCode = 1, + PrimaryNameAttribute = "name", + PrimaryIdAttribute = "accountid", + IsIntersect = false, + Columns = new ColumnModel[] + { + new StringColumnModel + { + SchemaName = "Account", // Conflicts with class name + LogicalName = "account_field", + DisplayName = "Account Field", + MaxLength = 100, + }, + new StringColumnModel + { + SchemaName = "Attributes", // Conflicts with Entity base class (non-virtual) + LogicalName = "attributes", + DisplayName = "Attributes", + MaxLength = 100, + }, + new StringColumnModel + { + SchemaName = "Name", // No conflict + LogicalName = "name", + DisplayName = "Name", + MaxLength = 100, + }, + }, + Relationships = new List(), + }; + + var context = CreateTestContext(); + + // Act + var result = ProxyClassMapper.MapToTemplateModel((table, new List()), context); + var resultColumns = GetColumnsFromResult(result); + + // Assert + Assert.Equal(3, resultColumns.Count); + Assert.Contains(resultColumns, c => c.SchemaName == "Account_1"); // Class name conflict + Assert.Contains(resultColumns, c => c.SchemaName == "Attributes_1"); // Base class conflict (non-virtual) + Assert.Contains(resultColumns, c => c.SchemaName == "Name"); // No conflict + } + private static GenerationContext CreateTestContext() { return new GenerationContext From d6d10d60e96a4de516a2b19f945c9d95867e06b7 Mon Sep 17 00:00:00 2001 From: skovlund Date: Fri, 7 Nov 2025 15:58:31 +0100 Subject: [PATCH 2/3] Simplified Table attribute conflict resolution (test issue) --- .../Generation/Mappers/ProxyClassMapper.cs | 75 ++----------- .../ProxyClassMapperTests.cs | 106 +----------------- 2 files changed, 14 insertions(+), 167 deletions(-) diff --git a/src/DataverseProxyGenerator.Core/Generation/Mappers/ProxyClassMapper.cs b/src/DataverseProxyGenerator.Core/Generation/Mappers/ProxyClassMapper.cs index 9f9301e..fceeb2f 100644 --- a/src/DataverseProxyGenerator.Core/Generation/Mappers/ProxyClassMapper.cs +++ b/src/DataverseProxyGenerator.Core/Generation/Mappers/ProxyClassMapper.cs @@ -6,40 +6,11 @@ namespace DataverseProxyGenerator.Core.Generation.Mappers; public static class ProxyClassMapper { /// - /// Non-virtual public members of Microsoft.Xrm.Sdk.Entity base class that would conflict with generated properties. - /// These CANNOT be overridden and must be renamed if a column has the same name. - /// Note: Virtual members like "Id" are NOT included here as they can be overridden without conflict. + /// The list is limited to properties where collisions have been identified. /// - private static readonly HashSet EntityBaseClassNonVirtualMembers = new(StringComparer.Ordinal) + private static readonly HashSet RestrictedAttributeNames = new(StringComparer.Ordinal) { - // Non-virtual properties from Entity class - "Attributes", - "EntityState", - "ExtensionData", - "FormattedValues", - "KeyAttributes", - "LogicalName", - "RelatedEntities", - "RowVersion", - "HasLazyFileAttribute", - "LazyFileAttributeKey", - "LazyFileAttributeValue", - "LazyFileSizeAttributeKey", - "LazyFileSizeAttributeValue", - - // Methods from Entity class (properties should not use these names either) - "Contains", - "GetAttributeValue", - "GetFormattedAttributeValue", - "GetRelatedEntities", - "GetRelatedEntity", - "SetAttributeValue", - "SetRelatedEntities", - "SetRelatedEntity", - "ToEntity", - "ToEntityReference", - "TryGetAttributeValue", - "ShallowCopyTo", + "Attributes", // Collision on SdkMessageProcessingStepImage }; public static object MapToTemplateModel((TableModel Table, IReadOnlyList Interfaces) input, GenerationContext context) @@ -49,19 +20,7 @@ public static object MapToTemplateModel((TableModel Table, IReadOnlyList var (table, interfaces) = input; - var processedColumns = ProcessColumnsWithClassNameConflictResolution(table.Columns, table.SchemaName); - - if (table.SchemaName == "EnvironmentVariableDefinition") - { - foreach (var key in table.Keys) - { - Console.WriteLine(key.SchemaName); - foreach (var attr in key.KeyAttributes) - { - Console.WriteLine($" {attr.SchemaName} : {attr.TypeName}"); - } - } - } + var processedColumns = ProcessColumnsWithNameConflictResolution(table.Columns, table.SchemaName); return new { @@ -83,9 +42,10 @@ public static object MapToTemplateModel((TableModel Table, IReadOnlyList }; } - private static IEnumerable ProcessColumnsWithClassNameConflictResolution(IEnumerable columns, string className) + private static IEnumerable ProcessColumnsWithNameConflictResolution(IEnumerable columns, string className) { - var usedNames = new HashSet(StringComparer.Ordinal); + var usedNames = RestrictedAttributeNames; + usedNames.Add(className); return columns.Select(c => { @@ -102,28 +62,15 @@ private static IEnumerable ProcessColumnsWithClassNameConflictResol }, }; - var finalName = sanitizedColumn.SchemaName; - - // Check if sanitized schema name conflicts with class name (case-sensitive) - if (string.Equals(finalName, className, StringComparison.Ordinal)) - { - finalName = $"{finalName}_1"; - } - - // Check if name conflicts with non-virtual Entity base class members (case-sensitive) - // Virtual members can be overridden, so they don't cause conflicts - if (EntityBaseClassNonVirtualMembers.Contains(finalName)) - { - finalName = $"{finalName}_1"; - } + var defaultName = sanitizedColumn.SchemaName; // Ensure the final name is unique (handle edge case where _1 suffix also conflicts) - var candidateName = finalName; - var suffix = 1; + var candidateName = defaultName; + var suffix = 0; while (usedNames.Contains(candidateName)) { suffix++; - candidateName = $"{finalName}_{suffix}"; + candidateName = $"{defaultName}_{suffix}"; } usedNames.Add(candidateName); diff --git a/tests/DataverseProxyGenerator.Tests/ProxyClassMapperTests.cs b/tests/DataverseProxyGenerator.Tests/ProxyClassMapperTests.cs index acfa047..b30846a 100644 --- a/tests/DataverseProxyGenerator.Tests/ProxyClassMapperTests.cs +++ b/tests/DataverseProxyGenerator.Tests/ProxyClassMapperTests.cs @@ -169,25 +169,11 @@ public void MapToTemplateModel_WithEntityBaseClassPropertyConflicts_AppendsUnder { new StringColumnModel { - SchemaName = "Attributes", // Conflicts with Entity.Attributes (non-virtual) + SchemaName = "Attributes", // Conflicts with Entity.Attributes LogicalName = "attributes", DisplayName = "Attributes", MaxLength = 100, }, - new StringColumnModel - { - SchemaName = "Id", // Does NOT conflict - Entity.Id is virtual and can be overridden - LogicalName = "id", - DisplayName = "Id", - MaxLength = 100, - }, - new StringColumnModel - { - SchemaName = "LogicalName", // Conflicts with Entity.LogicalName (non-virtual) - LogicalName = "logicalname", - DisplayName = "Logical Name", - MaxLength = 100, - }, }, Relationships = new List(), }; @@ -199,56 +185,8 @@ public void MapToTemplateModel_WithEntityBaseClassPropertyConflicts_AppendsUnder var resultColumns = GetColumnsFromResult(result); // Assert - Assert.Equal(3, resultColumns.Count); + Assert.Single(resultColumns); Assert.Contains(resultColumns, c => c.SchemaName == "Attributes_1"); // Renamed due to conflict - Assert.Contains(resultColumns, c => c.SchemaName == "Id"); // NOT renamed - virtual property can be overridden - Assert.Contains(resultColumns, c => c.SchemaName == "LogicalName_1"); // Renamed due to conflict - } - - [Fact] - public void MapToTemplateModel_WithEntityBaseClassMethodConflicts_AppendsUnderscoreOne() - { - // Arrange - var table = new TableModel - { - SchemaName = "TestEntity", - LogicalName = "testentity", - DisplayName = "Test Entity", - Description = "Test entity", - EntityTypeCode = 10001, - PrimaryNameAttribute = "name", - PrimaryIdAttribute = "testentityid", - IsIntersect = false, - Columns = new ColumnModel[] - { - new StringColumnModel - { - SchemaName = "Contains", // Conflicts with Entity.Contains method - LogicalName = "contains", - DisplayName = "Contains", - MaxLength = 100, - }, - new StringColumnModel - { - SchemaName = "ToEntity", // Conflicts with Entity.ToEntity method - LogicalName = "toentity", - DisplayName = "To Entity", - MaxLength = 100, - }, - }, - Relationships = new List(), - }; - - var context = CreateTestContext(); - - // Act - var result = ProxyClassMapper.MapToTemplateModel((table, new List()), context); - var resultColumns = GetColumnsFromResult(result); - - // Assert - Assert.Equal(2, resultColumns.Count); - Assert.Contains(resultColumns, c => c.SchemaName == "Contains_1"); - Assert.Contains(resultColumns, c => c.SchemaName == "ToEntity_1"); } [Fact] @@ -305,44 +243,6 @@ public void MapToTemplateModel_WithCaseSensitiveEntityBaseClassConflict_OnlyRena Assert.Contains(resultColumns, c => c.SchemaName == "ATTRIBUTES"); // Different case kept } - [Fact] - public void MapToTemplateModel_WithVirtualEntityBaseClassProperty_DoesNotRename() - { - // Arrange - var table = new TableModel - { - SchemaName = "TestEntity", - LogicalName = "testentity", - DisplayName = "Test Entity", - Description = "Test entity", - EntityTypeCode = 10001, - PrimaryNameAttribute = "name", - PrimaryIdAttribute = "testentityid", - IsIntersect = false, - Columns = new ColumnModel[] - { - new StringColumnModel - { - SchemaName = "Id", // Entity.Id is virtual - can be overridden, should NOT be renamed - LogicalName = "id", - DisplayName = "Id", - MaxLength = 100, - }, - }, - Relationships = new List(), - }; - - var context = CreateTestContext(); - - // Act - var result = ProxyClassMapper.MapToTemplateModel((table, new List()), context); - var resultColumns = GetColumnsFromResult(result); - - // Assert - Assert.Single(resultColumns); - Assert.Contains(resultColumns, c => c.SchemaName == "Id"); // NOT renamed - virtual property - } - [Fact] public void MapToTemplateModel_WithMultipleConflictTypes_AppliesAllRenames() { @@ -368,7 +268,7 @@ public void MapToTemplateModel_WithMultipleConflictTypes_AppliesAllRenames() }, new StringColumnModel { - SchemaName = "Attributes", // Conflicts with Entity base class (non-virtual) + SchemaName = "Attributes", // Conflicts with Entity base class LogicalName = "attributes", DisplayName = "Attributes", MaxLength = 100, From dddc40721dbd7f3cef6096f044c8b4f7fdedc0ea Mon Sep 17 00:00:00 2001 From: skovlund Date: Fri, 7 Nov 2025 16:07:49 +0100 Subject: [PATCH 3/3] Fixed reuse of HashSet issue --- .../Generation/Mappers/ProxyClassMapper.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/DataverseProxyGenerator.Core/Generation/Mappers/ProxyClassMapper.cs b/src/DataverseProxyGenerator.Core/Generation/Mappers/ProxyClassMapper.cs index fceeb2f..9b49804 100644 --- a/src/DataverseProxyGenerator.Core/Generation/Mappers/ProxyClassMapper.cs +++ b/src/DataverseProxyGenerator.Core/Generation/Mappers/ProxyClassMapper.cs @@ -44,7 +44,7 @@ public static object MapToTemplateModel((TableModel Table, IReadOnlyList private static IEnumerable ProcessColumnsWithNameConflictResolution(IEnumerable columns, string className) { - var usedNames = RestrictedAttributeNames; + var usedNames = new HashSet(RestrictedAttributeNames, StringComparer.Ordinal); usedNames.Add(className); return columns.Select(c =>