-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Fix DateTime timezone mismatch causing Delay step to hang indefinitely with PostgreSQL #1425
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.
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
base: master
Are you sure you want to change the base?
Changes from all commits
9e3362a
f3b7d6b
6da36df
3e217a5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -137,6 +137,15 @@ internal static PersistedScheduledCommand ToPersistable(this ScheduledCommand in | |
| return result; | ||
| } | ||
|
|
||
| private static DateTime EnsureUtc(DateTime dateTime) | ||
| { | ||
| if (dateTime.Kind == DateTimeKind.Local) | ||
| return dateTime.ToUniversalTime(); | ||
| if (dateTime.Kind == DateTimeKind.Utc) | ||
| return dateTime; | ||
| return DateTime.SpecifyKind(dateTime, DateTimeKind.Utc); | ||
| } | ||
|
Comment on lines
+140
to
+147
|
||
|
|
||
| internal static WorkflowInstance ToWorkflowInstance(this PersistedWorkflow instance) | ||
| { | ||
| WorkflowInstance result = new WorkflowInstance(); | ||
|
|
@@ -148,9 +157,9 @@ internal static WorkflowInstance ToWorkflowInstance(this PersistedWorkflow insta | |
| result.Version = instance.Version; | ||
| result.WorkflowDefinitionId = instance.WorkflowDefinitionId; | ||
| result.Status = instance.Status; | ||
| result.CreateTime = DateTime.SpecifyKind(instance.CreateTime, DateTimeKind.Utc); | ||
| result.CreateTime = EnsureUtc(instance.CreateTime); | ||
| if (instance.CompleteTime.HasValue) | ||
| result.CompleteTime = DateTime.SpecifyKind(instance.CompleteTime.Value, DateTimeKind.Utc); | ||
| result.CompleteTime = EnsureUtc(instance.CompleteTime.Value); | ||
|
|
||
| result.ExecutionPointers = new ExecutionPointerCollection(instance.ExecutionPointers.Count + 8); | ||
|
|
||
|
|
@@ -163,15 +172,15 @@ internal static WorkflowInstance ToWorkflowInstance(this PersistedWorkflow insta | |
| pointer.Active = ep.Active; | ||
|
|
||
| if (ep.SleepUntil.HasValue) | ||
| pointer.SleepUntil = DateTime.SpecifyKind(ep.SleepUntil.Value, DateTimeKind.Utc); | ||
| pointer.SleepUntil = EnsureUtc(ep.SleepUntil.Value); | ||
|
|
||
| pointer.PersistenceData = JsonConvert.DeserializeObject(ep.PersistenceData ?? string.Empty, SerializerSettings); | ||
|
|
||
| if (ep.StartTime.HasValue) | ||
| pointer.StartTime = DateTime.SpecifyKind(ep.StartTime.Value, DateTimeKind.Utc); | ||
| pointer.StartTime = EnsureUtc(ep.StartTime.Value); | ||
|
|
||
| if (ep.EndTime.HasValue) | ||
| pointer.EndTime = DateTime.SpecifyKind(ep.EndTime.Value, DateTimeKind.Utc); | ||
| pointer.EndTime = EnsureUtc(ep.EndTime.Value); | ||
|
|
||
| pointer.StepName = ep.StepName; | ||
|
|
||
|
|
@@ -212,10 +221,11 @@ internal static EventSubscription ToEventSubscription(this PersistedSubscription | |
| result.StepId = instance.StepId; | ||
| result.ExecutionPointerId = instance.ExecutionPointerId; | ||
| result.WorkflowId = instance.WorkflowId; | ||
| result.SubscribeAsOf = DateTime.SpecifyKind(instance.SubscribeAsOf, DateTimeKind.Utc); | ||
| result.SubscribeAsOf = EnsureUtc(instance.SubscribeAsOf); | ||
| result.SubscriptionData = JsonConvert.DeserializeObject(instance.SubscriptionData, SerializerSettings); | ||
| result.ExternalToken = instance.ExternalToken; | ||
| result.ExternalTokenExpiry = instance.ExternalTokenExpiry; | ||
| if (instance.ExternalTokenExpiry.HasValue) | ||
| result.ExternalTokenExpiry = EnsureUtc(instance.ExternalTokenExpiry.Value); | ||
| result.ExternalWorkerId = instance.ExternalWorkerId; | ||
|
|
||
| return result; | ||
|
|
@@ -227,7 +237,7 @@ internal static Event ToEvent(this PersistedEvent instance) | |
| result.Id = instance.EventId.ToString(); | ||
| result.EventKey = instance.EventKey; | ||
| result.EventName = instance.EventName; | ||
| result.EventTime = DateTime.SpecifyKind(instance.EventTime, DateTimeKind.Utc); | ||
| result.EventTime = EnsureUtc(instance.EventTime); | ||
| result.IsProcessed = instance.IsProcessed; | ||
| result.EventData = JsonConvert.DeserializeObject(instance.EventData, SerializerSettings); | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,93 @@ | ||
| using System; | ||
| using FluentAssertions; | ||
| using WorkflowCore.Models; | ||
| using WorkflowCore.Persistence.EntityFramework; | ||
| using WorkflowCore.Persistence.EntityFramework.Models; | ||
| using Xunit; | ||
|
|
||
| namespace WorkflowCore.Tests.Sqlite | ||
| { | ||
| public class ExtensionMethodsFixture | ||
| { | ||
| [Fact] | ||
| public void ToWorkflowInstance_CreateTime_Local_Kind_Should_Convert_To_Utc() | ||
| { | ||
| var localTime = new DateTime(2026, 1, 1, 12, 0, 0, DateTimeKind.Local); | ||
| var expectedUtc = localTime.ToUniversalTime(); | ||
|
|
||
| var persisted = BuildPersistedWorkflow(createTime: localTime); | ||
|
|
||
| var instance = persisted.ToWorkflowInstance(); | ||
|
|
||
| instance.CreateTime.Should().Be(expectedUtc); | ||
| instance.CreateTime.Kind.Should().Be(DateTimeKind.Utc); | ||
| } | ||
|
|
||
| [Fact] | ||
| public void ToWorkflowInstance_CreateTime_Utc_Kind_Should_Return_Same_Value() | ||
| { | ||
| var utcTime = new DateTime(2026, 1, 1, 12, 0, 0, DateTimeKind.Utc); | ||
|
|
||
| var persisted = BuildPersistedWorkflow(createTime: utcTime); | ||
|
|
||
| var instance = persisted.ToWorkflowInstance(); | ||
|
|
||
| instance.CreateTime.Should().Be(utcTime); | ||
| instance.CreateTime.Kind.Should().Be(DateTimeKind.Utc); | ||
| } | ||
|
|
||
| [Fact] | ||
| public void ToWorkflowInstance_CreateTime_Unspecified_Kind_Should_Treat_As_Utc() | ||
| { | ||
| var unspecifiedTime = new DateTime(2026, 1, 1, 12, 0, 0, DateTimeKind.Unspecified); | ||
|
|
||
| var persisted = BuildPersistedWorkflow(createTime: unspecifiedTime); | ||
|
|
||
| var instance = persisted.ToWorkflowInstance(); | ||
|
|
||
| instance.CreateTime.Should().Be(DateTime.SpecifyKind(unspecifiedTime, DateTimeKind.Utc)); | ||
| instance.CreateTime.Kind.Should().Be(DateTimeKind.Utc); | ||
| } | ||
|
|
||
| [Fact] | ||
| public void ToWorkflowInstance_SleepUntil_Local_Kind_Should_Convert_To_Utc() | ||
| { | ||
| var localTime = new DateTime(2026, 3, 26, 12, 30, 31, DateTimeKind.Local); | ||
| var expectedUtc = localTime.ToUniversalTime(); | ||
|
|
||
| var persisted = BuildPersistedWorkflow(); | ||
| persisted.ExecutionPointers.Add(new PersistedExecutionPointer | ||
| { | ||
| Id = "ep1", | ||
| StepId = 0, | ||
| Active = true, | ||
| SleepUntil = localTime, | ||
| PersistenceData = "null", | ||
| ContextItem = "null", | ||
| Scope = "", | ||
| Children = "", | ||
| EventData = "null", | ||
| Outcome = "null" | ||
| }); | ||
|
|
||
| var instance = persisted.ToWorkflowInstance(); | ||
|
|
||
| var pointer = instance.ExecutionPointers.FindById("ep1"); | ||
| pointer.SleepUntil.Should().Be(expectedUtc); | ||
| pointer.SleepUntil.Value.Kind.Should().Be(DateTimeKind.Utc); | ||
| } | ||
|
|
||
| private static PersistedWorkflow BuildPersistedWorkflow(DateTime? createTime = null) | ||
| { | ||
| return new PersistedWorkflow | ||
| { | ||
| InstanceId = Guid.NewGuid(), | ||
| WorkflowDefinitionId = "test", | ||
| Version = 1, | ||
| Status = WorkflowStatus.Runnable, | ||
| Data = "{}", | ||
| CreateTime = createTime ?? new DateTime(2026, 1, 1, 0, 0, 0, DateTimeKind.Utc) | ||
| }; | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
EnsureUtccurrently callsDateTime.SpecifyKind(dateTime, DateTimeKind.Utc)for inputs that are alreadyDateTimeKind.Utc. Returning the original value whendateTime.Kind == DateTimeKind.Utcwould make the intent clearer and avoid an unnecessary copy.