address greptile review feedback (greploop iteration 1)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This commit is contained in:
Sipke Schoorstra 2026-06-21 01:37:57 +02:00
parent c7912fd2c8
commit 7449d2e703
5 changed files with 76 additions and 22 deletions

View file

@ -41,14 +41,10 @@ public class DefaultTriggerScheduler(IWorkflowScheduler workflowScheduler, ISyst
foreach (var trigger in startAtTriggers)
{
var executeAt = trigger.GetPayload<StartAtPayload>().ExecuteAt;
// If the trigger is in the past, log info and skip scheduling.
if (executeAt < now)
{
logger.LogInformation("StartAt trigger is in the past. TriggerId: {TriggerId}. ExecuteAt: {ExecuteAt}. Skipping scheduling", trigger.Id, executeAt);
continue;
}
logger.LogInformation("StartAt trigger is in the past. TriggerId: {TriggerId}. ExecuteAt: {ExecuteAt}. Scheduling catch-up", trigger.Id, executeAt);
var input = new { ExecuteAt = executeAt }.ToDictionary();
var request = new ScheduleNewWorkflowInstanceRequest
{

View file

@ -23,7 +23,12 @@ public class PastDueScheduleStaggerer(IOptions<SchedulingOptions> options)
if (staggerInterval <= TimeSpan.Zero || staggerWindow <= TimeSpan.Zero)
return minimumDelay;
var slotCount = Math.Max(1, staggerWindow.Ticks / staggerInterval.Ticks);
var availableWindow = staggerWindow - minimumDelay;
if (availableWindow <= TimeSpan.Zero)
return minimumDelay;
var slotCount = Math.Max(1, availableWindow.Ticks / staggerInterval.Ticks + 1);
var sequence = Interlocked.Increment(ref _sequence) - 1;
var slot = (sequence & long.MaxValue) % slotCount;

View file

@ -39,21 +39,9 @@ public interface IBookmarkStore
/// Returns a page of bookmarks matching the specified filter.
/// </summary>
/// <remarks>
/// The default implementation materializes all matching bookmarks and pages them in memory. Stores backed by external persistence should override this method.
/// Startup backlog catch-up depends on store-backed paging. Implementations should page at the persistence layer instead of materializing all matches in memory.
/// </remarks>
async ValueTask<Page<StoredBookmark>> FindManyAsync(BookmarkFilter filter, PageArgs pageArgs, CancellationToken cancellationToken = default)
{
var records = (await FindManyAsync(filter, cancellationToken)).OrderBy(x => x.Id).ToList();
IEnumerable<StoredBookmark> page = records;
if (pageArgs.Offset.HasValue)
page = page.Skip(pageArgs.Offset.Value);
if (pageArgs.Limit.HasValue)
page = page.Take(pageArgs.Limit.Value);
return Page.Of(page.ToList(), records.Count);
}
ValueTask<Page<StoredBookmark>> FindManyAsync(BookmarkFilter filter, PageArgs pageArgs, CancellationToken cancellationToken = default);
/// <summary>
/// Deletes a set of bookmarks matching the specified filter.

View file

@ -0,0 +1,42 @@
using Elsa.Common;
using Elsa.Scheduling.Activities;
using Elsa.Scheduling.Bookmarks;
using Elsa.Scheduling.Services;
using Elsa.Workflows.Helpers;
using Elsa.Workflows.Runtime.Entities;
using Microsoft.Extensions.Logging;
using NSubstitute;
namespace Elsa.Scheduling.UnitTests.Services;
public class DefaultTriggerSchedulerTests
{
[Fact]
public async Task ScheduleAsync_SchedulesPastDueStartAtTriggerForCatchUp()
{
var workflowScheduler = Substitute.For<IWorkflowScheduler>();
var systemClock = Substitute.For<ISystemClock>();
var logger = Substitute.For<ILogger<DefaultTriggerScheduler>>();
var scheduler = new DefaultTriggerScheduler(workflowScheduler, systemClock, logger);
var now = new DateTimeOffset(2025, 11, 06, 22, 50, 00, TimeSpan.Zero);
var executeAt = now.AddMinutes(-5);
ScheduleNewWorkflowInstanceRequest? scheduledRequest = null;
var trigger = new StoredTrigger
{
Id = "trigger-1",
Name = ActivityTypeNameHelper.GenerateTypeName<StartAt>(),
WorkflowDefinitionVersionId = "workflow-version",
ActivityId = "activity-1",
Payload = new StartAtPayload(executeAt)
};
systemClock.UtcNow.Returns(now);
workflowScheduler.ScheduleAtAsync(trigger.Id, Arg.Do<ScheduleNewWorkflowInstanceRequest>(x => scheduledRequest = x), executeAt, Arg.Any<CancellationToken>()).Returns(ValueTask.CompletedTask);
await scheduler.ScheduleAsync([trigger], CancellationToken.None);
await workflowScheduler.Received(1).ScheduleAtAsync(trigger.Id, Arg.Any<ScheduleNewWorkflowInstanceRequest>(), executeAt, Arg.Any<CancellationToken>());
Assert.NotNull(scheduledRequest);
Assert.Equal(trigger.ActivityId, scheduledRequest.TriggerActivityId);
Assert.Equal(trigger.WorkflowDefinitionVersionId, scheduledRequest.WorkflowDefinitionHandle.DefinitionVersionId);
}
}

View file

@ -0,0 +1,23 @@
using Elsa.Scheduling.Options;
using Elsa.Scheduling.Services;
using OptionsFactory = Microsoft.Extensions.Options.Options;
namespace Elsa.Scheduling.UnitTests.Services;
public class PastDueScheduleStaggererTests
{
[Fact]
public void GetDelay_DoesNotExceedConfiguredWindow()
{
var staggerer = new PastDueScheduleStaggerer(OptionsFactory.Create(new SchedulingOptions
{
MinimumPastDueScheduleDelay = TimeSpan.FromSeconds(1),
PastDueScheduleStaggerInterval = TimeSpan.FromMilliseconds(900),
PastDueScheduleStaggerWindow = TimeSpan.FromSeconds(5)
}));
var delays = Enumerable.Range(0, 16).Select(_ => staggerer.GetDelay(TimeSpan.Zero)).ToList();
Assert.All(delays, delay => Assert.InRange(delay, TimeSpan.FromSeconds(1), TimeSpan.FromSeconds(5)));
}
}