Skip to content

Commit 810f1c9

Browse files
committed
pr comments
1 parent 89097fb commit 810f1c9

6 files changed

Lines changed: 64 additions & 14 deletions

File tree

Libraries/src/Amazon.Lambda.Annotations.SourceGenerator/Validation/LambdaFunctionValidator.cs

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -555,14 +555,15 @@ private static void ValidateScheduleEvents(LambdaFunctionModel lambdaFunctionMod
555555
validationErrors.ForEach(errorMessage => diagnostics.Add(Diagnostic.Create(DiagnosticDescriptors.InvalidScheduleEventAttribute, methodLocation, errorMessage)));
556556
}
557557

558-
// Validate method parameters
558+
// Validate method parameters - When using ScheduleEventAttribute, the method signature must be (ScheduledEvent evnt) or (ScheduledEvent evnt, ILambdaContext context)
559559
var parameters = lambdaFunctionModel.LambdaMethod.Parameters;
560-
if (parameters.Count > 2 ||
561-
(parameters.Count >= 1 && parameters[0].Type.FullName != TypeFullNames.ScheduledEvent) ||
562-
(parameters.Count == 2 && parameters[1].Type.FullName != TypeFullNames.ILambdaContext))
560+
if (parameters.Count == 0 ||
561+
parameters.Count > 2 ||
562+
(parameters.Count == 1 && parameters[0].Type.FullName != TypeFullNames.ScheduledEvent) ||
563+
(parameters.Count == 2 && (parameters[0].Type.FullName != TypeFullNames.ScheduledEvent || parameters[1].Type.FullName != TypeFullNames.ILambdaContext)))
563564
{
564565
var errorMessage = $"When using the {nameof(ScheduleEventAttribute)}, the Lambda method can accept at most 2 parameters. " +
565-
$"The first parameter must be of type {TypeFullNames.ScheduledEvent}. " +
566+
$"The first parameter is required and must be of type {TypeFullNames.ScheduledEvent}. " +
566567
$"The second parameter is optional and must be of type {TypeFullNames.ILambdaContext}.";
567568
diagnostics.Add(Diagnostic.Create(DiagnosticDescriptors.InvalidLambdaMethodSignature, methodLocation, errorMessage));
568569
}

Libraries/src/Amazon.Lambda.Annotations/Schedule/ScheduleEventAttribute.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ public string ResourceName
3434
return resourceName;
3535
}
3636
// Generate a default resource name from the schedule expression
37-
var sanitized = string.Join(string.Empty, Schedule.Where(char.IsLetterOrDigit));
37+
var sanitized = string.Join(string.Empty, (Schedule ?? string.Empty).Where(char.IsLetterOrDigit));
3838
return sanitized.Length > 0 ? sanitized : "ScheduleEvent";
3939
}
4040
set => resourceName = value;
@@ -60,7 +60,7 @@ public string ResourceName
6060
/// </summary>
6161
public bool Enabled
6262
{
63-
get => enabled.GetValueOrDefault();
63+
get => enabled.GetValueOrDefault(true);
6464
set => enabled = value;
6565
}
6666
private bool? enabled { get; set; }

Libraries/test/Amazon.Lambda.Annotations.SourceGenerators.Tests/ScheduleEventAttributeTests.cs

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -221,6 +221,25 @@ public void Validate_MultipleErrors_ReturnsAll()
221221
Assert.Equal(2, errors.Count);
222222
}
223223

224+
[Fact]
225+
public void Enabled_DefaultValueIsTrue_WhenNotExplicitlySet()
226+
{
227+
var attr = new ScheduleEventAttribute("rate(5 minutes)");
228+
229+
Assert.False(attr.IsEnabledSet);
230+
Assert.True(attr.Enabled);
231+
}
232+
233+
[Fact]
234+
public void ResourceName_NullSchedule_DoesNotThrow()
235+
{
236+
var attr = new ScheduleEventAttribute(null);
237+
238+
// Should not throw NullReferenceException
239+
var resourceName = attr.ResourceName;
240+
Assert.Equal("ScheduleEvent", resourceName);
241+
}
242+
224243
[Fact]
225244
public void Validate_AllValidWithOptionals_ReturnsNoErrors()
226245
{

Libraries/test/Amazon.Lambda.Annotations.SourceGenerators.Tests/SourceGeneratorTests.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1681,7 +1681,7 @@ public async Task VerifyInvalidScheduleEvents_ThrowsCompilationErrors()
16811681
DiagnosticResult.CompilerError("AWSLambda0117")
16821682
.WithSpan($"TestServerlessApp{Path.DirectorySeparatorChar}ScheduleEventExamples{Path.DirectorySeparatorChar}InvalidScheduleEvents.cs", 24, 9, 29, 10)
16831683
.WithArguments("When using the ScheduleEventAttribute, the Lambda method can accept at most 2 parameters. " +
1684-
"The first parameter must be of type Amazon.Lambda.CloudWatchEvents.ScheduledEvents.ScheduledEvent. " +
1684+
"The first parameter is required and must be of type Amazon.Lambda.CloudWatchEvents.ScheduledEvents.ScheduledEvent. " +
16851685
"The second parameter is optional and must be of type Amazon.Lambda.Core.ILambdaContext."),
16861686

16871687
// ProcessMessageWithInvalidReturnType: returns bool

Libraries/test/TestServerlessApp.IntegrationTests/ScheduleEventRule.cs

Lines changed: 35 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -25,13 +25,43 @@ public async Task VerifyScheduleEventRuleConfiguration()
2525
var lambdaFunctionName = _fixture.LambdaFunctions.FirstOrDefault(x => string.Equals(x.LogicalId, "ScheduledHandler"))?.Name;
2626
Assert.NotNull(lambdaFunctionName);
2727

28-
// SAM creates EventBridge rules with names derived from the stack
2928
var eventsClient = new AmazonCloudWatchEventsClient(Amazon.RegionEndpoint.USWest2);
30-
var rulesResponse = await eventsClient.ListRulesAsync(new ListRulesRequest());
3129

32-
// Find the rule targeting our function
33-
var matchingRule = rulesResponse.Rules.FirstOrDefault(r =>
34-
r.Name.Contains("FiveMinuteSchedule") || r.Name.Contains("ScheduledHandler"));
30+
// Paginate through all rules and verify the rule targets the correct Lambda function
31+
Rule matchingRule = null;
32+
string nextToken = null;
33+
34+
do
35+
{
36+
var rulesResponse = await eventsClient.ListRulesAsync(new ListRulesRequest
37+
{
38+
NextToken = nextToken
39+
});
40+
41+
foreach (var rule in rulesResponse.Rules.Where(r =>
42+
string.Equals(r.ScheduleExpression, "rate(5 minutes)") &&
43+
string.Equals(r.Description, "Runs every 5 minutes")))
44+
{
45+
var targetsResponse = await eventsClient.ListTargetsByRuleAsync(new ListTargetsByRuleRequest
46+
{
47+
Rule = rule.Name
48+
});
49+
50+
if (targetsResponse.Targets.Any(t => t.Arn != null && t.Arn.Contains($":function:{lambdaFunctionName}")))
51+
{
52+
matchingRule = rule;
53+
break;
54+
}
55+
}
56+
57+
if (matchingRule != null)
58+
{
59+
break;
60+
}
61+
62+
nextToken = rulesResponse.NextToken;
63+
}
64+
while (!string.IsNullOrEmpty(nextToken));
3565

3666
Assert.NotNull(matchingRule);
3767
Assert.Equal("rate(5 minutes)", matchingRule.ScheduleExpression);

Libraries/test/TestServerlessApp.IntegrationTests/TestServerlessApp.IntegrationTests.csproj

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010
<PackageReference Include="AWSSDK.SecurityToken" Version="3.7.1.99" />
1111
<PackageReference Include="AWSSDK.DynamoDBv2" Version="3.7.513.1" />
1212
<PackageReference Include="AWSSDK.SimpleNotificationService" Version="3.7.502.58" />
13-
<PackageReference Include="AWSSDK.CloudWatchEvents" Version="3.7.*" />
13+
<PackageReference Include="AWSSDK.CloudWatchEvents" Version="3.7.1.99" />
1414
<PackageReference Include="Microsoft.NET.Test.Sdk" Version="17.0.0" />
1515
<PackageReference Include="Newtonsoft.Json" Version="13.0.3" />
1616
<PackageReference Include="xunit" Version="2.4.1" />

0 commit comments

Comments
 (0)