Conversation
|
@bachuv I just merged the latest from dev back into this branch. Can you please update this new sample to be consistent with the changes I made to the other samples yesterday - this will include adding your sample project to the Samples.sln in /samples directory and ensuring that the sample project compiles using the new CPM settings. Thanks |
| <PackageReference Include="Microsoft.ApplicationInsights.WorkerService" VersionOverride="2.23.0" /> | ||
| <PackageReference Include="Microsoft.Azure.Functions.Worker" VersionOverride="2.1.0" /> | ||
| <PackageReference Include="Microsoft.Azure.Functions.Worker.ApplicationInsights" VersionOverride="2.0.0" /> | ||
| <PackageReference Include="Microsoft.Azure.Functions.Worker.Extensions.DurableTask" VersionOverride="1.6.1" /> | ||
| <PackageReference Include="Microsoft.Azure.Functions.Worker.Extensions.Http" VersionOverride="3.3.0" /> | ||
| <PackageReference Include="Microsoft.Azure.Functions.Worker.Extensions.Http.AspNetCore" VersionOverride="2.0.2" /> | ||
| <PackageReference Include="Microsoft.Azure.Functions.Worker.Sdk" VersionOverride="2.0.5" /> |
There was a problem hiding this comment.
If possible, can we remove VersionOverride here and just update the relevant versions in /samples/Packages.Directory.props? The Samples.sln should ensure that all .NET samples compile with the changes. Hopefully everything just works, but If this breaks the other tests, VersionOverride should be fine for now
|
@bachuv - Thanks for updating the entities sample! I was able to run the sample locally. Noticed a few things -
For example, I added 3 chrips to Alice and expecting the GET request to return them, but it returned nothing
|
samples/isolated-entities/InventorySample/InventoryEntity.cs
Dismissed
Show dismissed
Hide dismissed
samples/isolated-entities/InventorySample/InventoryEntity.cs
Dismissed
Show dismissed
Hide dismissed
There was a problem hiding this comment.
Pull request overview
This PR adds two comprehensive .NET Isolated Durable Entities samples demonstrating Durable Functions v3 capabilities: an inventory management system (InventorySample) and a social media service (Chirper). Both samples showcase stateful entity patterns with the isolated worker model, moving away from the older v2 samples that used the in-process worker model.
Key Changes:
- Added InventorySample demonstrating e-commerce order processing with entity state management, orchestration coordination, and activity functions
- Added Chirper sample ported from the in-process model, showcasing social media entities, parallel entity queries, and REST API integration
- Both samples target .NET 8.0 and Azure Functions v4 with complete project configurations including Application Insights integration
Reviewed changes
Copilot reviewed 28 out of 28 changed files in this pull request and generated 23 comments.
Show a summary per file
| File | Description |
|---|---|
| samples/isolated-entities/InventorySample/InventorySample.csproj | Project configuration with package references for isolated worker Durable Functions |
| samples/isolated-entities/InventorySample/Program.cs | Application bootstrap with Functions Web Application and Application Insights setup |
| samples/isolated-entities/InventorySample/InventoryEntity.cs | Durable entity managing inventory stock with operations for add, update, and remove |
| samples/isolated-entities/InventorySample/OrderOrchestrator.cs | Orchestration coordinating order processing, inventory checks, payment, and email confirmation |
| samples/isolated-entities/InventorySample/InventoryEndpoints.cs | HTTP-triggered functions for order submission, inventory setup, and inventory queries |
| samples/isolated-entities/InventorySample/ChargePayment.cs | Activity function simulating payment processing |
| samples/isolated-entities/InventorySample/SendEmail.cs | Activity function simulating email notification |
| samples/isolated-entities/InventorySample/QueryInventory.cs | Orchestration for querying inventory entity state |
| samples/isolated-entities/InventorySample/host.json | Host configuration enabling distributed tracing for Durable Functions |
| samples/isolated-entities/InventorySample/README.md | Documentation explaining the sample architecture, API usage, and local setup |
| samples/isolated-entities/InventorySample/.gitignore | Excludes build artifacts and local settings from source control |
| samples/isolated-entities/InventorySample/Properties/serviceDependencies.json | Service dependency metadata for Application Insights and Azure Storage |
| samples/isolated-entities/InventorySample/Properties/serviceDependencies.local.json | Local development service dependencies configuration |
| samples/isolated-entities/Chirper/Chirper.csproj | Project configuration using project reference to Worker.Extensions.DurableTask |
| samples/isolated-entities/Chirper/Program.cs | Application bootstrap matching InventorySample setup pattern |
| samples/isolated-entities/Chirper/Entities/UserChirps.cs | Entity managing chirps (posts) for individual users |
| samples/isolated-entities/Chirper/Entities/UserFollows.cs | Entity managing follow relationships between users |
| samples/isolated-entities/Chirper/Entities/IUserChirps.cs | Interface defining chirp management operations |
| samples/isolated-entities/Chirper/Entities/IUserFollows.cs | Interface defining follow management operations |
| samples/isolated-entities/Chirper/Orchestrations/GetTimeline.cs | Orchestration querying multiple entities in parallel to build user timeline |
| samples/isolated-entities/Chirper/PublicRest/HttpSurface.cs | Seven HTTP-triggered functions implementing the complete REST API surface |
| samples/isolated-entities/Chirper/PublicRest/Chirp.cs | Data structure representing a chirp message |
| samples/isolated-entities/Chirper/host.json | Host configuration with Application Insights settings |
| samples/isolated-entities/Chirper/local.settings.json | Local development storage configuration |
| samples/isolated-entities/Chirper/README.md | Comprehensive documentation with API examples and design explanation |
| samples/isolated-entities/Chirper/.gitignore | Excludes build artifacts from source control |
| samples/isolated-entities/Chirper/Properties/serviceDependencies.json | Service dependency metadata for deployment |
| samples/isolated-entities/Chirper/Properties/serviceDependencies.local.json | Local development service dependencies |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 28 out of 28 changed files in this pull request and generated 18 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ILogger log, | ||
| string userId) | ||
| { | ||
| Authenticate(req, userId); | ||
| var instanceId = await client.ScheduleNewOrchestrationInstanceAsync(nameof(GetTimeline), userId); | ||
| OrchestrationMetadata metadata = await client.WaitForInstanceCompletionAsync(instanceId, getInputsAndOutputs: true); | ||
|
|
||
| var response = req.CreateResponse(HttpStatusCode.OK); | ||
|
|
||
| if (string.IsNullOrEmpty(metadata.SerializedOutput)) | ||
| { | ||
| await response.WriteStringAsync(""); | ||
| return response; | ||
| } | ||
|
|
||
| await response.WriteStringAsync(metadata.SerializedOutput); | ||
| return response; | ||
| } | ||
|
|
||
| [Function("UserChirpsGet")] | ||
| public static async Task<HttpResponseData> UserChirpsGet( | ||
| [HttpTrigger(AuthorizationLevel.Function, "get", Route = "user/{userId}/chirps")] HttpRequestData req, | ||
| [DurableClient] DurableTaskClient client, | ||
| ILogger log, | ||
| string userId) | ||
| { | ||
| Authenticate(req, userId); | ||
| var target = new EntityInstanceId(nameof(UserChirps), userId); | ||
| var chirps = await client.Entities.GetEntityAsync<UserChirps>(target); | ||
|
|
||
| if (chirps != null && chirps.State != null) | ||
| { | ||
| var response = req.CreateResponse(HttpStatusCode.OK); | ||
| await response.WriteAsJsonAsync(chirps.State.Chirps); | ||
| return response; | ||
| } | ||
|
|
||
| var notFoundResponse = req.CreateResponse(HttpStatusCode.NotFound); | ||
| return notFoundResponse; | ||
| } | ||
|
|
||
| [Function("UserChirpsPost")] | ||
| public static async Task<HttpResponseData> UserChirpsPost( | ||
| [HttpTrigger(AuthorizationLevel.Function, "post", Route = "user/{userId}/chirps")] HttpRequestData req, | ||
| [DurableClient] DurableTaskClient client, | ||
| ILogger log, | ||
| string userId) | ||
| { | ||
| Authenticate(req, userId); | ||
| var chirp = new Chirp() | ||
| { | ||
| UserId = userId, | ||
| Timestamp = DateTime.UtcNow, | ||
| Content = await req.ReadAsStringAsync() ?? string.Empty, | ||
| }; | ||
|
|
||
| var entityInstanceId = new EntityInstanceId(nameof(UserChirps), userId); | ||
| await client.Entities.SignalEntityAsync(entityInstanceId, "Add", chirp); | ||
|
|
||
| var response = req.CreateResponse(HttpStatusCode.Accepted); | ||
| await response.WriteAsJsonAsync(chirp); | ||
|
|
||
| return response; | ||
| } | ||
|
|
||
| [Function("UserChirpsDelete")] | ||
| public static async Task<HttpResponseData> UserChirpsDelete( | ||
| [HttpTrigger(AuthorizationLevel.Function, "delete", Route = "user/{userId}/chirps/{timestamp}")] HttpRequestData req, | ||
| [DurableClient] DurableTaskClient client, | ||
| ILogger log, | ||
| string userId, | ||
| DateTime timestamp) | ||
| { | ||
| Authenticate(req, userId); | ||
|
|
||
| var entityInstanceId = new EntityInstanceId(nameof(UserChirps), userId); | ||
| await client.Entities.SignalEntityAsync(entityInstanceId, "Remove", timestamp); | ||
|
|
||
| return req.CreateResponse(HttpStatusCode.Accepted); | ||
| } | ||
|
|
||
| [Function("UserFollowsGet")] | ||
| public static async Task<HttpResponseData> UserFollowsGet( | ||
| [HttpTrigger(AuthorizationLevel.Function, "get", Route = "user/{userId}/follows")] HttpRequestData req, | ||
| [DurableClient] DurableTaskClient client, | ||
| ILogger log, | ||
| string userId) | ||
| { | ||
| Authenticate(req, userId); | ||
| var target = new EntityInstanceId(nameof(UserFollows), userId); | ||
| EntityMetadata<UserFollows>? follows = await client.Entities.GetEntityAsync<UserFollows>(target); | ||
|
|
||
| if (follows != null) | ||
| { | ||
| HttpResponseData response = req.CreateResponse(HttpStatusCode.OK); | ||
| await response.WriteAsJsonAsync(follows.State.FollowedUsers); | ||
| return response; | ||
| } | ||
|
|
||
| return req.CreateResponse(HttpStatusCode.NotFound); | ||
| } | ||
|
|
||
| [Function("UserFollowsPost")] | ||
| public static async Task<HttpResponseData> UserFollowsPost( | ||
| [HttpTrigger(AuthorizationLevel.Function, "post", Route = "user/{userId}/follows/{userId2}")] HttpRequestData req, | ||
| [DurableClient] DurableTaskClient client, | ||
| ILogger log, | ||
| string userId, | ||
| string userId2) | ||
| { | ||
| Authenticate(req, userId); | ||
| var entityInstanceId = new EntityInstanceId(nameof(UserFollows), userId); | ||
| await client.Entities.SignalEntityAsync(entityInstanceId, "Add", userId2); | ||
| return req.CreateResponse(HttpStatusCode.Accepted); | ||
| } | ||
|
|
||
| [Function("UserFollowsDelete")] | ||
| public static async Task<HttpResponseData> UserFollowsDelete( | ||
| [HttpTrigger(AuthorizationLevel.Function, "delete", Route = "user/{userId}/follows/{userId2}")] HttpRequestData req, | ||
| [DurableClient] DurableTaskClient client, | ||
| ILogger log, |
There was a problem hiding this comment.
The ILogger parameter 'log' is declared but never used in any of the HTTP endpoint methods. Consider removing it to reduce clutter, or use it to add logging statements for better observability in production.
| @@ -0,0 +1,16 @@ | |||
| using System; | |||
| using System.Collections.Generic; | |||
| using System.Text; | |||
There was a problem hiding this comment.
The using directives System.Text and System.Collections.Generic are not used in this file. Consider removing them to keep the code clean.
| using System.Text; |
| public static async Task GetInventory( | ||
| [OrchestrationTrigger] TaskOrchestrationContext context) | ||
| { | ||
| var stock = await context.Entities.CallEntityAsync<List<Item>>( | ||
| new EntityInstanceId("InventoryEntity", "store"), | ||
| "GetAll"); | ||
|
|
||
| Console.WriteLine(JsonSerializer.Serialize(stock)); | ||
| } |
There was a problem hiding this comment.
This orchestration doesn't return a value, but orchestrators typically should return a result. The orchestration writes to Console.WriteLine which won't be captured by the HTTP endpoint that calls this. Consider returning the stock data so it can be accessed by the calling endpoint.
| <FrameworkReference Include="Microsoft.AspNetCore.App" /> | ||
| <PackageReference Include="Microsoft.ApplicationInsights.WorkerService" VersionOverride="2.23.0" /> | ||
| <PackageReference Include="Microsoft.Azure.Functions.Worker" VersionOverride="2.2.0" /> | ||
| <PackageReference Include="Microsoft.Azure.Functions.Worker.Core" VersionOverride="2.2.0" /> |
There was a problem hiding this comment.
The Microsoft.Azure.Functions.Worker.Core package reference is redundant. It's already included as a transitive dependency of Microsoft.Azure.Functions.Worker. Consider removing this explicit reference to avoid potential version conflicts.
| <PackageReference Include="Microsoft.Azure.Functions.Worker.Core" VersionOverride="2.2.0" /> |
| using InventorySample; | ||
| using Microsoft.Azure.Functions.Worker; |
There was a problem hiding this comment.
This using directive references a namespace that doesn't exist. The OrderRequest and OrderLine types are declared at the global level in this same file, not in an "InventorySample" namespace. Remove this using directive as it will cause compilation errors.
| using InventorySample; | |
| using Microsoft.Azure.Functions.Worker; | |
| using Microsoft.Azure.Functions.Worker; |
| var target = new EntityInstanceId(nameof(UserFollows), userId); | ||
| EntityMetadata<UserFollows>? follows = await client.Entities.GetEntityAsync<UserFollows>(target); | ||
|
|
||
| if (follows != null) |
There was a problem hiding this comment.
When follows is not null but State is null, this will throw a NullReferenceException when accessing follows.State.FollowedUsers. Consider checking follows.State separately or handling the null State case explicitly.
| if (follows != null) | |
| if (follows != null && follows.State != null) |
| [HttpTrigger(AuthorizationLevel.Function, "get", Route = "inventory")] HttpRequestData req, | ||
| [DurableClient] DurableTaskClient client) | ||
| { | ||
| string instanceId = $"query-inventory-{Guid.NewGuid()}"; | ||
|
|
||
| await client.ScheduleNewOrchestrationInstanceAsync("QueryInventory", instanceId); | ||
|
|
||
| return await client.CreateCheckStatusResponseAsync(req, instanceId); |
There was a problem hiding this comment.
The GetInventory endpoint uses CreateCheckStatusResponseAsync to return an orchestration status URL, but according to the README documentation (line 69), it's supposed to "Print the inventory to the console". This API design is inconsistent - users expect to GET inventory data, not receive a status URL. Consider either updating the documentation to reflect the actual behavior or changing the implementation to directly query and return the inventory data.
| [HttpTrigger(AuthorizationLevel.Function, "get", Route = "inventory")] HttpRequestData req, | |
| [DurableClient] DurableTaskClient client) | |
| { | |
| string instanceId = $"query-inventory-{Guid.NewGuid()}"; | |
| await client.ScheduleNewOrchestrationInstanceAsync("QueryInventory", instanceId); | |
| return await client.CreateCheckStatusResponseAsync(req, instanceId); | |
| [HttpTrigger(AuthorizationLevel.Function, "get", Route = "inventory")] HttpRequestData req, | |
| [DurableClient] DurableTaskClient client) | |
| { | |
| // Read the current inventory state directly from the InventoryEntity | |
| var entityId = new EntityInstanceId("InventoryEntity", "store"); | |
| var inventoryState = await client.Entities.GetEntityAsync<Dictionary<string, int>>(entityId); | |
| var res = req.CreateResponse(HttpStatusCode.OK); | |
| await res.WriteAsJsonAsync(inventoryState ?? new Dictionary<string, int>(), SerializerOptions()); | |
| return res; |
| public void AddOrUpdate(Item update) | ||
| { | ||
| if (!Stock.ContainsKey(update.Sku)) | ||
| Stock[update.Sku] = 0; | ||
|
|
||
| Stock[update.Sku] += update.Qty; | ||
| } |
There was a problem hiding this comment.
The AddOrUpdate method doesn't validate that the quantity is non-negative. If a negative quantity is passed, it will reduce the stock count when it should be adding or updating. Consider adding validation to ensure update.Qty is not negative, or clarify the intended behavior in the documentation.
| public void AddMany(List<OrderLine> items) | ||
| { | ||
| foreach (var item in items) | ||
| { | ||
| if (!Stock.ContainsKey(item.Sku)) | ||
| Stock[item.Sku] = 0; | ||
|
|
||
| Stock[item.Sku] += item.Qty; | ||
| } | ||
| } |
There was a problem hiding this comment.
The AddMany method doesn't validate that quantities are non-negative. If items with negative quantities are passed, it will reduce stock levels when the method name suggests it should only add. This is particularly problematic during rollback scenarios. Consider adding validation to ensure item.Qty is not negative for each item.
| foreach (var item in items) | ||
| if (!Stock.ContainsKey(item.Sku) || Stock[item.Sku] < item.Qty) | ||
| return false; |
There was a problem hiding this comment.
This foreach loop implicitly filters its target sequence - consider filtering the sequence explicitly using '.Where(...)'.
| foreach (var item in items) | |
| if (!Stock.ContainsKey(item.Sku) || Stock[item.Sku] < item.Qty) | |
| return false; | |
| var invalidItems = items.Where(item => !Stock.ContainsKey(item.Sku) || Stock[item.Sku] < item.Qty); | |
| if (invalidItems.Any()) | |
| return false; |
This PR adds a .NET Isolated Durable Entities sample which uses Durable Functions v3. The current samples use Durable Functions v2.
Pull request checklist
pending_docs.mdrelease_notes.md/src/Worker.Extensions.DurableTask/AssemblyInfo.csdevandmainbranches and will not be merged into thev2.xbranch.