From 7749646623d4c1a1cae31f5294c739330fe15806 Mon Sep 17 00:00:00 2001 From: "Stuart Blackler (@im5tu)" Date: Mon, 8 Jun 2026 21:02:14 +0100 Subject: [PATCH 1/3] fix(dynamo-gen): filter [UnixTimestamp] to DateTime/DateTimeOffset properties [UnixTimestamp] applied to an incompatible property type (e.g. a long TTL) is now filtered out during attribute processing rather than flowing into code generation. Covers nullable types by unwrapping the underlying type. --- .../Attributes/AttributeHandlerRegistry.cs | 21 +++++- .../AttributeHandlerRegistryTests.cs | 64 +++++++++++++++++++ 2 files changed, 84 insertions(+), 1 deletion(-) diff --git a/src/Clients/Goa.Clients.Dynamo.Generator/Attributes/AttributeHandlerRegistry.cs b/src/Clients/Goa.Clients.Dynamo.Generator/Attributes/AttributeHandlerRegistry.cs index 6b848f4c..22cb5ca6 100644 --- a/src/Clients/Goa.Clients.Dynamo.Generator/Attributes/AttributeHandlerRegistry.cs +++ b/src/Clients/Goa.Clients.Dynamo.Generator/Attributes/AttributeHandlerRegistry.cs @@ -30,7 +30,7 @@ public List ProcessAttributes(ISymbol symbol) if (handler.CanHandle(attributeData)) { var attributeInfo = handler.ParseAttribute(attributeData); - if (attributeInfo != null) + if (attributeInfo != null && IsAttributeApplicable(attributeInfo, symbol)) { result.Add(attributeInfo); break; // Only first handler that can process it @@ -42,6 +42,25 @@ public List ProcessAttributes(ISymbol symbol) return result; } + /// + /// Checks if a parsed attribute is applicable to the given symbol. + /// Filters out attributes that are semantically invalid for the target type. + /// + private static bool IsAttributeApplicable(AttributeInfo attributeInfo, ISymbol symbol) + { + // UnixTimestamp is only valid on DateTime/DateTimeOffset properties + if (attributeInfo is UnixTimestampAttributeInfo && symbol is IPropertySymbol property) + { + var type = property.Type; + if (type is INamedTypeSymbol { IsGenericType: true } nullable) + type = nullable.TypeArguments[0]; + + return type.Name is nameof(DateTime) or nameof(DateTimeOffset); + } + + return true; + } + /// /// Validates all attributes on a symbol and reports diagnostics. /// diff --git a/tests/Clients/Goa.Clients.Dynamo.Generator.Tests/Attributes/AttributeHandlerRegistryTests.cs b/tests/Clients/Goa.Clients.Dynamo.Generator.Tests/Attributes/AttributeHandlerRegistryTests.cs index 88a0ceda..1d9cf14f 100644 --- a/tests/Clients/Goa.Clients.Dynamo.Generator.Tests/Attributes/AttributeHandlerRegistryTests.cs +++ b/tests/Clients/Goa.Clients.Dynamo.Generator.Tests/Attributes/AttributeHandlerRegistryTests.cs @@ -232,6 +232,70 @@ public async Task GetAttributes_ShouldReturnOnlyAttributesOfSpecifiedType() await Assert.That(dynamoAttributes[0]).IsEqualTo(attribute2); } + [Test] + public async Task ProcessAttributes_UnixTimestampOnLongProperty_ShouldBeFilteredOut() + { + // Arrange: [UnixTimestamp] is only valid on DateTime/DateTimeOffset. + // When applied to a long property (e.g. TTL), it should be silently filtered out. + var registry = new AttributeHandlerRegistry(); + var mockHandler = new Mock(); + var mockAttributeData = MockSymbolFactory.CreateAttributeData("Goa.Clients.Dynamo.UnixTimestampAttribute"); + var unixTimestampInfo = new UnixTimestampAttributeInfo + { + AttributeData = mockAttributeData, + AttributeTypeName = "Goa.Clients.Dynamo.UnixTimestampAttribute", + Format = Generator.Models.UnixTimestampFormat.Seconds + }; + + mockHandler.Setup(h => h.CanHandle(It.IsAny())).Returns(true); + mockHandler.Setup(h => h.ParseAttribute(It.IsAny())).Returns(unixTimestampInfo); + + registry.RegisterHandler(mockHandler.Object); + + // Create a property symbol with Int64 type (long) + var propertySymbol = MockSymbolFactory.CreatePropertySymbol("TTL", MockSymbolFactory.PrimitiveTypes.Int64); + propertySymbol.Setup(p => p.GetAttributes()) + .Returns(System.Collections.Immutable.ImmutableArray.Create(mockAttributeData)); + + // Act + var attributes = registry.ProcessAttributes(propertySymbol.Object); + + // Assert: UnixTimestamp on long should be filtered out + await Assert.That(attributes).IsEmpty(); + } + + [Test] + public async Task ProcessAttributes_UnixTimestampOnDateTimeOffsetProperty_ShouldBeIncluded() + { + // Arrange: [UnixTimestamp] on DateTimeOffset is valid and should be kept. + var registry = new AttributeHandlerRegistry(); + var mockHandler = new Mock(); + var mockAttributeData = MockSymbolFactory.CreateAttributeData("Goa.Clients.Dynamo.UnixTimestampAttribute"); + var unixTimestampInfo = new UnixTimestampAttributeInfo + { + AttributeData = mockAttributeData, + AttributeTypeName = "Goa.Clients.Dynamo.UnixTimestampAttribute", + Format = Generator.Models.UnixTimestampFormat.Seconds + }; + + mockHandler.Setup(h => h.CanHandle(It.IsAny())).Returns(true); + mockHandler.Setup(h => h.ParseAttribute(It.IsAny())).Returns(unixTimestampInfo); + + registry.RegisterHandler(mockHandler.Object); + + // Create a property symbol with DateTimeOffset type + var propertySymbol = MockSymbolFactory.CreatePropertySymbol("UpdatedAt", MockSymbolFactory.PrimitiveTypes.DateTimeOffset); + propertySymbol.Setup(p => p.GetAttributes()) + .Returns(System.Collections.Immutable.ImmutableArray.Create(mockAttributeData)); + + // Act + var attributes = registry.ProcessAttributes(propertySymbol.Object); + + // Assert: UnixTimestamp on DateTimeOffset should be kept + await Assert.That(attributes).Count().IsEqualTo(1); + await Assert.That(attributes[0]).IsTypeOf(); + } + [Test] public async Task ProcessAttributes_HandlerReturnsNull_ShouldNotAddToResults() { From 393df1f08e8679d51758b8984e6285009a264722 Mon Sep 17 00:00:00 2001 From: "Stuart Blackler (@im5tu)" Date: Mon, 8 Jun 2026 21:02:24 +0100 Subject: [PATCH 2/3] test(dynamo-gen): add JsonMapperGenerator regression tests for collections Verify that the presence of a complex collection property does not break [UnixTimestamp] conversion (FromUnixTimeSeconds/ToUnixTimeSeconds) or [Ignore] handling on sibling properties. --- .../JsonMapperGeneratorTests.cs | 158 ++++++++++++++++++ 1 file changed, 158 insertions(+) create mode 100644 tests/Clients/Goa.Clients.Dynamo.Generator.Tests/CodeGeneration/JsonMapperGeneratorTests.cs diff --git a/tests/Clients/Goa.Clients.Dynamo.Generator.Tests/CodeGeneration/JsonMapperGeneratorTests.cs b/tests/Clients/Goa.Clients.Dynamo.Generator.Tests/CodeGeneration/JsonMapperGeneratorTests.cs new file mode 100644 index 00000000..87c7dc47 --- /dev/null +++ b/tests/Clients/Goa.Clients.Dynamo.Generator.Tests/CodeGeneration/JsonMapperGeneratorTests.cs @@ -0,0 +1,158 @@ +using Goa.Clients.Dynamo.Generator.CodeGeneration; +using Goa.Clients.Dynamo.Generator.TypeHandlers; +using Goa.Clients.Dynamo.Generator.Tests.Helpers; +using Goa.Clients.Dynamo.Generator.Models; +using System.Collections.Immutable; + +namespace Goa.Clients.Dynamo.Generator.Tests.CodeGeneration; + +public class JsonMapperGeneratorTests +{ + private readonly TypeHandlerRegistry _typeHandlerRegistry; + private readonly JsonMapperGenerator _generator; + + public JsonMapperGeneratorTests() + { + _typeHandlerRegistry = CreateTypeHandlerRegistry(); + _generator = new JsonMapperGenerator(_typeHandlerRegistry); + } + + [Test] + public async Task GenerateCode_WithUnixTimestampAndCollectionOfComplexType_ShouldEmitFromUnixTimeConversion() + { + // Arrange: A parent type with [UnixTimestamp] DateTimeOffset + List + // This reproduces the bug where adding a referenced type via collection + // causes UnixTimestamp handling to break in the generated JSON mapper. + + var nestedProperties = new List + { + TestModelBuilders.CreatePropertyInfo("Name", MockSymbolFactory.PrimitiveTypes.String), + TestModelBuilders.CreatePropertyInfo("Quantity", MockSymbolFactory.PrimitiveTypes.Int32), + }; + + var nestedType = TestModelBuilders.CreateDynamoTypeInfo( + "Ingredient", + "TestNamespace.Ingredient", + properties: nestedProperties); + + // Create a collection type: List + var listMock = MockSymbolFactory.CreateNamedTypeSymbol( + "List", + "System.Collections.Generic.List", + "System.Collections.Generic"); + listMock.Setup(x => x.IsGenericType).Returns(true); + listMock.Setup(x => x.TypeArguments).Returns(ImmutableArray.Create(nestedType.Symbol!)); + + var parentProperties = new List + { + TestModelBuilders.CreatePropertyInfo("Id", MockSymbolFactory.PrimitiveTypes.String), + TestModelBuilders.CreatePropertyInfo("Name", MockSymbolFactory.PrimitiveTypes.String), + TestModelBuilders.CreateUnixTimestampPropertyInfo("UpdatedAt", MockSymbolFactory.PrimitiveTypes.DateTimeOffset), + TestModelBuilders.CreateCollectionPropertyInfo( + "Ingredients", + listMock.Object, + nestedType.Symbol!, + isNullable: true), + }; + + var parentType = TestModelBuilders.CreateDynamoTypeInfo( + "Recipe", + "TestNamespace.Recipe", + properties: parentProperties, + attributes: new List + { + TestModelBuilders.CreateDynamoModelAttribute("RECIPE#", "METADATA") + }); + + var types = new List { parentType, nestedType }; + var context = new GenerationContext(); + + // Act + var result = _generator.GenerateCode(types, context); + + // Assert: The generated code must contain FromUnixTimeSeconds for the UpdatedAt read path + await Assert.That(result) + .Contains("FromUnixTimeSeconds"); + + // Assert: The generated code must contain ToUnixTimeSeconds for the UpdatedAt write path + await Assert.That(result) + .Contains("ToUnixTimeSeconds"); + + // Assert: The generated code should NOT directly assign long to DateTimeOffset without conversion + await Assert.That(result) + .DoesNotContain("= (DateTimeOffset)"); + } + + [Test] + public async Task GenerateCode_WithIgnoredPropertyAndCollectionOfComplexType_ShouldSkipIgnoredProperty() + { + // Arrange: A parent type with [Ignore] property + List + var nestedProperties = new List + { + TestModelBuilders.CreatePropertyInfo("Name", MockSymbolFactory.PrimitiveTypes.String), + }; + + var nestedType = TestModelBuilders.CreateDynamoTypeInfo( + "Tag", + "TestNamespace.Tag", + properties: nestedProperties); + + var listMock = MockSymbolFactory.CreateNamedTypeSymbol( + "List", + "System.Collections.Generic.List", + "System.Collections.Generic"); + listMock.Setup(x => x.IsGenericType).Returns(true); + listMock.Setup(x => x.TypeArguments).Returns(ImmutableArray.Create(nestedType.Symbol!)); + + var ignoredAttr = new IgnoreAttributeInfo { Direction = Models.IgnoreDirection.Always }; + + var parentProperties = new List + { + TestModelBuilders.CreatePropertyInfo("Id", MockSymbolFactory.PrimitiveTypes.String), + TestModelBuilders.CreatePropertyInfo("InternalState", MockSymbolFactory.PrimitiveTypes.String, + attributes: new List { ignoredAttr }), + TestModelBuilders.CreateCollectionPropertyInfo( + "Tags", + listMock.Object, + nestedType.Symbol!, + isNullable: true), + }; + + var parentType = TestModelBuilders.CreateDynamoTypeInfo( + "Item", + "TestNamespace.Item", + properties: parentProperties, + attributes: new List + { + TestModelBuilders.CreateDynamoModelAttribute("ITEM#", "METADATA") + }); + + var types = new List { parentType, nestedType }; + var context = new GenerationContext(); + + // Act + var result = _generator.GenerateCode(types, context); + + // Assert: InternalState should NOT appear in the generated write code + await Assert.That(result) + .DoesNotContain("\"InternalState\""); + } + + private static TypeHandlerRegistry CreateTypeHandlerRegistry() + { + var registry = new TypeHandlerRegistry(); + registry.RegisterHandler(new PrimitiveTypeHandler()); + registry.RegisterHandler(new EnumTypeHandler()); + registry.RegisterHandler(new DateOnlyTypeHandler()); + registry.RegisterHandler(new TimeOnlyTypeHandler()); + registry.RegisterHandler(new DateTimeTypeHandler()); + registry.RegisterHandler(new UnixTimestampTypeHandler()); + var collectionHandler = new CollectionTypeHandler(); + collectionHandler.SetRegistry(registry); + registry.RegisterHandler(collectionHandler); + var complexHandler = new ComplexTypeHandler(); + complexHandler.SetRegistry(registry); + registry.RegisterHandler(complexHandler); + return registry; + } +} From e2b5c8fc1c6ba8493ef8c7e663cbbf5d37ba3647 Mon Sep 17 00:00:00 2001 From: Stuart Blackler Date: Wed, 17 Jun 2026 14:52:52 +0100 Subject: [PATCH 3/3] =?UTF-8?q?fix(dynamo-gen):=20robust=20DateTime/DateTi?= =?UTF-8?q?meOffset=20check=20for=20[UnixTimest=E2=80=A6=20(#77)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../Attributes/AttributeHandlerRegistry.cs | 26 ++++++- .../AttributeHandlerRegistryTests.cs | 68 +++++++++++++++++++ 2 files changed, 91 insertions(+), 3 deletions(-) diff --git a/src/Clients/Goa.Clients.Dynamo.Generator/Attributes/AttributeHandlerRegistry.cs b/src/Clients/Goa.Clients.Dynamo.Generator/Attributes/AttributeHandlerRegistry.cs index 22cb5ca6..70f43ea4 100644 --- a/src/Clients/Goa.Clients.Dynamo.Generator/Attributes/AttributeHandlerRegistry.cs +++ b/src/Clients/Goa.Clients.Dynamo.Generator/Attributes/AttributeHandlerRegistry.cs @@ -52,15 +52,35 @@ private static bool IsAttributeApplicable(AttributeInfo attributeInfo, ISymbol s if (attributeInfo is UnixTimestampAttributeInfo && symbol is IPropertySymbol property) { var type = property.Type; - if (type is INamedTypeSymbol { IsGenericType: true } nullable) - type = nullable.TypeArguments[0]; - return type.Name is nameof(DateTime) or nameof(DateTimeOffset); + // Only unwrap genuine Nullable, not arbitrary generics such as List. + if (type is INamedTypeSymbol named && + named.OriginalDefinition.SpecialType == SpecialType.System_Nullable_T && + named.TypeArguments.Length == 1) + { + type = named.TypeArguments[0]; + } + + return IsDateTimeType(type); } return true; } + /// + /// Determines whether the given type is System.DateTime or System.DateTimeOffset + /// using robust symbol identity rather than brittle name comparisons. + /// + private static bool IsDateTimeType(ITypeSymbol type) + { + if (type.SpecialType == SpecialType.System_DateTime) + return true; + + // DateTimeOffset has no SpecialType, so identify it by its fully-qualified name. + return type.Name == nameof(DateTimeOffset) + && type.ContainingNamespace?.ToDisplayString() == "System"; + } + /// /// Validates all attributes on a symbol and reports diagnostics. /// diff --git a/tests/Clients/Goa.Clients.Dynamo.Generator.Tests/Attributes/AttributeHandlerRegistryTests.cs b/tests/Clients/Goa.Clients.Dynamo.Generator.Tests/Attributes/AttributeHandlerRegistryTests.cs index 1d9cf14f..1c7fb352 100644 --- a/tests/Clients/Goa.Clients.Dynamo.Generator.Tests/Attributes/AttributeHandlerRegistryTests.cs +++ b/tests/Clients/Goa.Clients.Dynamo.Generator.Tests/Attributes/AttributeHandlerRegistryTests.cs @@ -296,6 +296,74 @@ public async Task ProcessAttributes_UnixTimestampOnDateTimeOffsetProperty_Should await Assert.That(attributes[0]).IsTypeOf(); } + [Test] + public async Task ProcessAttributes_UnixTimestampOnNullableDateTimeProperty_ShouldBeIncluded() + { + // Arrange: [UnixTimestamp] on DateTime? must be unwrapped before the compatibility check + // and kept, just like the non-nullable variant. + var registry = new AttributeHandlerRegistry(); + var mockHandler = new Mock(); + var mockAttributeData = MockSymbolFactory.CreateAttributeData("Goa.Clients.Dynamo.UnixTimestampAttribute"); + var unixTimestampInfo = new UnixTimestampAttributeInfo + { + AttributeData = mockAttributeData, + AttributeTypeName = "Goa.Clients.Dynamo.UnixTimestampAttribute", + Format = Generator.Models.UnixTimestampFormat.Seconds + }; + + mockHandler.Setup(h => h.CanHandle(It.IsAny())).Returns(true); + mockHandler.Setup(h => h.ParseAttribute(It.IsAny())).Returns(unixTimestampInfo); + + registry.RegisterHandler(mockHandler.Object); + + // Create a property symbol with DateTime? (Nullable) type + var nullableDateTime = MockSymbolFactory.CreateNullableType(MockSymbolFactory.PrimitiveTypes.DateTime).Object; + var propertySymbol = MockSymbolFactory.CreatePropertySymbol("ExpiresAt", nullableDateTime); + propertySymbol.Setup(p => p.GetAttributes()) + .Returns(System.Collections.Immutable.ImmutableArray.Create(mockAttributeData)); + + // Act + var attributes = registry.ProcessAttributes(propertySymbol.Object); + + // Assert: UnixTimestamp on DateTime? should be kept + await Assert.That(attributes).Count().IsEqualTo(1); + await Assert.That(attributes[0]).IsTypeOf(); + } + + [Test] + public async Task ProcessAttributes_UnixTimestampOnNullableDateTimeOffsetProperty_ShouldBeIncluded() + { + // Arrange: [UnixTimestamp] on DateTimeOffset? must be unwrapped before the compatibility check + // and kept, just like the non-nullable variant. + var registry = new AttributeHandlerRegistry(); + var mockHandler = new Mock(); + var mockAttributeData = MockSymbolFactory.CreateAttributeData("Goa.Clients.Dynamo.UnixTimestampAttribute"); + var unixTimestampInfo = new UnixTimestampAttributeInfo + { + AttributeData = mockAttributeData, + AttributeTypeName = "Goa.Clients.Dynamo.UnixTimestampAttribute", + Format = Generator.Models.UnixTimestampFormat.Seconds + }; + + mockHandler.Setup(h => h.CanHandle(It.IsAny())).Returns(true); + mockHandler.Setup(h => h.ParseAttribute(It.IsAny())).Returns(unixTimestampInfo); + + registry.RegisterHandler(mockHandler.Object); + + // Create a property symbol with DateTimeOffset? (Nullable) type + var nullableDateTimeOffset = MockSymbolFactory.CreateNullableType(MockSymbolFactory.PrimitiveTypes.DateTimeOffset).Object; + var propertySymbol = MockSymbolFactory.CreatePropertySymbol("UpdatedAt", nullableDateTimeOffset); + propertySymbol.Setup(p => p.GetAttributes()) + .Returns(System.Collections.Immutable.ImmutableArray.Create(mockAttributeData)); + + // Act + var attributes = registry.ProcessAttributes(propertySymbol.Object); + + // Assert: UnixTimestamp on DateTimeOffset? should be kept + await Assert.That(attributes).Count().IsEqualTo(1); + await Assert.That(attributes[0]).IsTypeOf(); + } + [Test] public async Task ProcessAttributes_HandlerReturnsNull_ShouldNotAddToResults() {