Skip to content

fix(dynamo-gen): filter [UnixTimestamp] to DateTime/DateTimeOffset properties #74

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Im5tu merged 3 commits into from
Jun 18, 2026
Merged

fix(dynamo-gen): filter [UnixTimestamp] to DateTime/DateTimeOffset properties #74

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7749646
fix(dynamo-gen): filter [UnixTimestamp] to DateTime/DateTimeOffset pr…
Im5tu Jun 8, 2026
393df1f
test(dynamo-gen): add JsonMapperGenerator regression tests for collec…
Im5tu Jun 8, 2026
e2b5c8f
fix(dynamo-gen): robust DateTime/DateTimeOffset check for [UnixTimest…
Im5tu Jun 17, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 40 additions & 1 deletion src/Clients/Goa.Clients.Dynamo.Generator/Attributes/AttributeHandlerRegistry.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ public List<AttributeInfo> 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
Expand All @@ -42,6 +42,45 @@ public List<AttributeInfo> ProcessAttributes(ISymbol symbol)
return result;
}

/// <summary>
/// Checks if a parsed attribute is applicable to the given symbol.
/// Filters out attributes that are semantically invalid for the target type.
/// </summary>
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;

// Only unwrap genuine Nullable<T>, not arbitrary generics such as List<DateTime>.
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;
}

/// <summary>
/// Determines whether the given type is System.DateTime or System.DateTimeOffset
/// using robust symbol identity rather than brittle name comparisons.
/// </summary>
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";
}
Comment on lines +70 to +82

@coderabbitai coderabbitai Bot Jun 17, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Description: Verify UnixTimestampTypeHandler uses consistent type checking

# Search for the CanHandle method or type-checking logic in UnixTimestampTypeHandler
rg -n -A10 "class UnixTimestampTypeHandler|CanHandle|IsDateTimeType" src/Clients/Goa.Clients.Dynamo.Generator/TypeHandlers/UnixTimestampTypeHandler.cs

Repository: Im5tu/goa

Length of output: 692

Update UnixTimestampTypeHandler to use robust symbol-based type checking.

Line 18 currently uses brittle name-only comparisons (underlyingType.Name == nameof(DateTime) || underlyingType.Name == nameof(DateTimeOffset)). Update it to match the robust type identification in AttributeHandlerRegistry.IsDateTimeType(): use SpecialType.System_DateTime for DateTime and namespace-qualified name checking for DateTimeOffset.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@src/Clients/Goa.Clients.Dynamo.Generator/Attributes/AttributeHandlerRegistry.cs`
around lines 70 - 82, The UnixTimestampTypeHandler class uses brittle name-only
comparisons at line 18 to check for DateTime and DateTimeOffset types. Replace
the current comparison logic (underlyingType.Name == nameof(DateTime) ||
underlyingType.Name == nameof(DateTimeOffset)) with the same robust symbol-based
approach used in the IsDateTimeType method: check if the type's SpecialType is
System_DateTime, and for DateTimeOffset, verify both the type name and the
fully-qualified namespace using ToDisplayString() to ensure proper type
identification.


/// <summary>
/// Validates all attributes on a symbol and reports diagnostics.
/// </summary>
Expand Down
132 changes: 132 additions & 0 deletions tests/Clients/Goa.Clients.Dynamo.Generator.Tests/Attributes/AttributeHandlerRegistryTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -232,6 +232,138 @@ 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<IAttributeHandler>();
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<AttributeData>())).Returns(true);
mockHandler.Setup(h => h.ParseAttribute(It.IsAny<AttributeData>())).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<IAttributeHandler>();
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<AttributeData>())).Returns(true);
mockHandler.Setup(h => h.ParseAttribute(It.IsAny<AttributeData>())).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<UnixTimestampAttributeInfo>();
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.

[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<IAttributeHandler>();
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<AttributeData>())).Returns(true);
mockHandler.Setup(h => h.ParseAttribute(It.IsAny<AttributeData>())).Returns(unixTimestampInfo);

registry.RegisterHandler(mockHandler.Object);

// Create a property symbol with DateTime? (Nullable<DateTime>) 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<UnixTimestampAttributeInfo>();
}

[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<IAttributeHandler>();
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<AttributeData>())).Returns(true);
mockHandler.Setup(h => h.ParseAttribute(It.IsAny<AttributeData>())).Returns(unixTimestampInfo);

registry.RegisterHandler(mockHandler.Object);

// Create a property symbol with DateTimeOffset? (Nullable<DateTimeOffset>) 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<UnixTimestampAttributeInfo>();
}

[Test]
public async Task ProcessAttributes_HandlerReturnsNull_ShouldNotAddToResults()
{
Expand Down
158 changes: 158 additions & 0 deletions tests/Clients/Goa.Clients.Dynamo.Generator.Tests/CodeGeneration/JsonMapperGeneratorTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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<NestedType>
// 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<PropertyInfo>
{
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<Ingredient>
var listMock = MockSymbolFactory.CreateNamedTypeSymbol(
"List",
"System.Collections.Generic.List<TestNamespace.Ingredient>",
"System.Collections.Generic");
listMock.Setup(x => x.IsGenericType).Returns(true);
listMock.Setup(x => x.TypeArguments).Returns(ImmutableArray.Create<Microsoft.CodeAnalysis.ITypeSymbol>(nestedType.Symbol!));

var parentProperties = new List<PropertyInfo>
{
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<AttributeInfo>
{
TestModelBuilders.CreateDynamoModelAttribute("RECIPE#<Id>", "METADATA")
});

var types = new List<DynamoTypeInfo> { 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<NestedType>
var nestedProperties = new List<PropertyInfo>
{
TestModelBuilders.CreatePropertyInfo("Name", MockSymbolFactory.PrimitiveTypes.String),
};

var nestedType = TestModelBuilders.CreateDynamoTypeInfo(
"Tag",
"TestNamespace.Tag",
properties: nestedProperties);

var listMock = MockSymbolFactory.CreateNamedTypeSymbol(
"List",
"System.Collections.Generic.List<TestNamespace.Tag>",
"System.Collections.Generic");
listMock.Setup(x => x.IsGenericType).Returns(true);
listMock.Setup(x => x.TypeArguments).Returns(ImmutableArray.Create<Microsoft.CodeAnalysis.ITypeSymbol>(nestedType.Symbol!));

var ignoredAttr = new IgnoreAttributeInfo { Direction = Models.IgnoreDirection.Always };

var parentProperties = new List<PropertyInfo>
{
TestModelBuilders.CreatePropertyInfo("Id", MockSymbolFactory.PrimitiveTypes.String),
TestModelBuilders.CreatePropertyInfo("InternalState", MockSymbolFactory.PrimitiveTypes.String,
attributes: new List<AttributeInfo> { ignoredAttr }),
TestModelBuilders.CreateCollectionPropertyInfo(
"Tags",
listMock.Object,
nestedType.Symbol!,
isNullable: true),
};

var parentType = TestModelBuilders.CreateDynamoTypeInfo(
"Item",
"TestNamespace.Item",
properties: parentProperties,
attributes: new List<AttributeInfo>
{
TestModelBuilders.CreateDynamoModelAttribute("ITEM#<Id>", "METADATA")
});

var types = new List<DynamoTypeInfo> { 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;
}
}
Loading