From 20dba4b07b416db911aba35101119d7119b3dc71 Mon Sep 17 00:00:00 2001 From: "Stuart Blackler (@im5tu)" Date: Wed, 17 Jun 2026 14:49:15 +0100 Subject: [PATCH] fix(dynamo-gen): robust DateTime/DateTimeOffset check for [UnixTimestamp] filter Use SpecialType.System_DateTime and a namespace-qualified DateTimeOffset check instead of brittle type.Name comparisons, and only unwrap genuine Nullable (via OriginalDefinition.SpecialType) rather than any generic, so [UnixTimestamp] on List or a custom DateTime type is no longer erroneously accepted. Add nullable DateTime/DateTimeOffset tests. --- .../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 22cb5ca..70f43ea 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 1d9cf14..1c7fb35 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() {