-
Notifications
You must be signed in to change notification settings - Fork 52
Extended sessions for entities in .NET isolated #507
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: main
Are you sure you want to change the base?
Changes from all commits
8ec7ed1
ef046b5
7b84f2e
464ad23
21512fd
e036914
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 | ||||
|---|---|---|---|---|---|---|
|
|
@@ -5,7 +5,8 @@ | |||||
| using DurableTask.Core.Entities.OperationFormat; | ||||||
| using Google.Protobuf; | ||||||
| using Microsoft.DurableTask.Entities; | ||||||
| using Microsoft.DurableTask.Worker.Shims; | ||||||
| using Microsoft.DurableTask.Worker.Shims; | ||||||
| using Microsoft.Extensions.Caching.Memory; | ||||||
| using Microsoft.Extensions.DependencyInjection; | ||||||
| using P = Microsoft.DurableTask.Protobuf; | ||||||
|
|
||||||
|
|
@@ -25,7 +26,7 @@ | |||||
| /// </para> | ||||||
| /// </remarks> | ||||||
| public static class GrpcEntityRunner | ||||||
| { | ||||||
| { | ||||||
| /// <summary> | ||||||
| /// Deserializes entity batch request from <paramref name="encodedEntityRequest"/> and uses it to invoke the | ||||||
| /// requested operations implemented by <paramref name="implementation"/>. | ||||||
|
|
@@ -51,24 +52,99 @@ | |||||
| /// </exception> | ||||||
| public static async Task<string> LoadAndRunAsync( | ||||||
| string encodedEntityRequest, ITaskEntity implementation, IServiceProvider? services = null) | ||||||
| { | ||||||
| return await LoadAndRunAsync(encodedEntityRequest, implementation, extendedSessionsCache: null, services: services); | ||||||
| } | ||||||
|
|
||||||
| /// <summary> | ||||||
| /// Deserializes entity batch request from <paramref name="encodedEntityRequest"/> and uses it to invoke the | ||||||
| /// requested operations implemented by <paramref name="implementation"/>. | ||||||
| /// </summary> | ||||||
| /// <param name="encodedEntityRequest"> | ||||||
| /// The encoded protobuf payload representing an entity batch request. This is a base64-encoded string. | ||||||
| /// </param> | ||||||
| /// <param name="implementation"> | ||||||
| /// An <see cref="ITaskEntity"/> implementation that defines the entity logic. | ||||||
| /// </param> | ||||||
| /// <param name="extendedSessionsCache"> | ||||||
| /// The cache of entity states which can be used to retrieve the entity state if this request is from within an extended session. | ||||||
| /// </param> | ||||||
| /// <param name="services"> | ||||||
| /// Optional <see cref="IServiceProvider"/> from which injected dependencies can be retrieved. | ||||||
| /// </param> | ||||||
| /// <returns> | ||||||
| /// Returns a serialized result of the entity batch that should be used as the return value of the entity function | ||||||
| /// trigger. | ||||||
| /// </returns> | ||||||
| /// <exception cref="ArgumentNullException"> | ||||||
| /// Thrown if <paramref name="encodedEntityRequest"/> or <paramref name="implementation"/> is <c>null</c>. | ||||||
| /// </exception> | ||||||
| /// <exception cref="ArgumentException"> | ||||||
| /// Thrown if <paramref name="encodedEntityRequest"/> contains invalid data. | ||||||
| /// </exception> | ||||||
| public static async Task<string> LoadAndRunAsync( | ||||||
| string encodedEntityRequest, ITaskEntity implementation, ExtendedSessionsCache? extendedSessionsCache, IServiceProvider? services = null) | ||||||
| { | ||||||
| Check.NotNullOrEmpty(encodedEntityRequest); | ||||||
| Check.NotNull(implementation); | ||||||
|
|
||||||
| P.EntityBatchRequest request = P.EntityBatchRequest.Parser.Base64Decode<P.EntityBatchRequest>( | ||||||
| encodedEntityRequest); | ||||||
| encodedEntityRequest); | ||||||
| Dictionary<string, object?> properties = request.Properties.ToDictionary( | ||||||
|
Check failure on line 93 in src/Worker/Grpc/GrpcEntityRunner.cs
|
||||||
| pair => pair.Key, | ||||||
| pair => ProtoUtils.ConvertValueToObject(pair.Value)); | ||||||
|
|
||||||
| EntityBatchRequest batch = request.ToEntityBatchRequest(); | ||||||
| EntityId id = EntityId.FromString(batch.InstanceId!); | ||||||
| TaskName entityName = new(id.Name); | ||||||
|
|
||||||
| TaskName entityName = new(id.Name); | ||||||
|
|
||||||
| bool addToExtendedSessions = false; | ||||||
| bool stateCached = false; | ||||||
| GrpcInstanceRunnerUtils.ParseRequestPropertiesAndInitializeCache( | ||||||
| properties, | ||||||
| extendedSessionsCache, | ||||||
| out double extendedSessionIdleTimeoutInSeconds, | ||||||
| out bool isExtendedSession, | ||||||
| out bool entityStateIncluded, | ||||||
| out MemoryCache? extendedSessions); | ||||||
|
|
||||||
| if (isExtendedSession && extendedSessions != null) | ||||||
| { | ||||||
| addToExtendedSessions = true; | ||||||
|
|
||||||
| // If an entity state was provided, even if we already have one stored, we always want to use the provided state. | ||||||
| if (!entityStateIncluded && extendedSessions.TryGetValue(request.InstanceId, out string? entityState)) | ||||||
| { | ||||||
| batch.EntityState = entityState; | ||||||
| stateCached = true; | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| if (!stateCached && !entityStateIncluded) | ||||||
| { | ||||||
| // No state was provided, and we do not have one cached, so we cannot execute the batch request. | ||||||
| return Convert.ToBase64String(new P.EntityBatchResult { RequiresState = true }.ToByteArray()); | ||||||
|
Check failure on line 126 in src/Worker/Grpc/GrpcEntityRunner.cs
|
||||||
| } | ||||||
|
|
||||||
| DurableTaskShimFactory factory = services is null | ||||||
| ? DurableTaskShimFactory.Default | ||||||
| : ActivatorUtilities.GetServiceOrCreateInstance<DurableTaskShimFactory>(services); | ||||||
|
|
||||||
| TaskEntity entity = factory.CreateEntity(entityName, implementation, id); | ||||||
| EntityBatchResult result = await entity.ExecuteOperationBatchAsync(batch); | ||||||
|
|
||||||
| : ActivatorUtilities.GetServiceOrCreateInstance<DurableTaskShimFactory>(services); | ||||||
|
|
||||||
| TaskEntity entity = factory.CreateEntity(entityName, implementation, id); | ||||||
| EntityBatchResult result = await entity.ExecuteOperationBatchAsync(batch); | ||||||
|
|
||||||
| if (addToExtendedSessions) | ||||||
| { | ||||||
| extendedSessions.Set( | ||||||
|
||||||
| extendedSessions.Set( | |
| extendedSessions?.Set( |
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,71 @@ | ||||||
| // Copyright (c) Microsoft Corporation. | ||||||
| // Licensed under the MIT License. | ||||||
|
|
||||||
| using System; | ||||||
| using System.Collections.Generic; | ||||||
| using System.Text; | ||||||
|
||||||
| using System.Text; |
Copilot
AI
Nov 20, 2025
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.
Duplicate word "are assigned" in the documentation. Should be "extended sessions are not enabled and default values are assigned to the returns."
| /// extended sessions are not enabled and default values are assigned are assigned to the returns. | |
| /// extended sessions are not enabled and default values are assigned to the returns. |
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.
Missing null check for cached entity state. If a null value is stored in the cache (similar to the scenario tested in
GrpcOrchestrationRunnerTests.NullExtendedSessionStored_Means_NeedsExtendedSessionNotUsed), this code will incorrectly setstateCached = trueand use the null state. The orchestration runner has an explicit&& extendedSessionState is not nullcheck on line 149 ofGrpcOrchestrationRunner.csthat should be mirrored here. The condition should be: