Add request context #171
Add request context #171
Conversation
WalkthroughThe PR adds support for propagating custom OxS-prefixed headers (oxs-*) through HTTP requests, message broker contexts, and MassTransit operations. A new OxSHeaders property is added to IRequestContext and its implementations, with header extraction and forwarding logic in multiple filter and middleware components. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20–25 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (5)
src/Simplic.OxS.Server/Middleware/HttpRequestInterceptor.cs (1)
19-26: Consider using.FirstOrDefault()for StringValues conversion.Line 24 assigns
header.Value(aStringValues) directly to the string dictionary. While this works via implicit conversion, it concatenates multiple values with commas. For consistency with other header handling in the codebase and to avoid unexpected behavior, use.FirstOrDefault()or.ToString().Apply this diff:
- requestContext.OxSHeaders[header.Key] = header.Value; + requestContext.OxSHeaders[header.Key] = header.Value.FirstOrDefault() ?? string.Empty;src/Simplic.OxS.Server/Filter/RequestContextHubFilter.cs (4)
28-30: Remove unused variables.The variables
userIdFromHeaderandorganizationIdFromHeaderare declared but never used. Lines 40-44 populateUserIdandOrganizationIdfrom JWT claims instead, making these header reads unnecessary.Apply this diff:
- // Get other headers if needed - var userIdFromHeader = GetFromHeader(httpContext, Constants.HttpHeaderUserIdKey); - var organizationIdFromHeader = GetFromHeader(httpContext, Constants.HttpHeaderOrganizationIdKey); -
55-66: Consider extracting duplicated helper to a shared utility class.The
GetFromHeadermethod is duplicated inRequestContextActionFilter.cs(lines 80-91). Consider extracting this and other common header parsing utilities to a shared class to reduce duplication and improve maintainability.
74-82: Remove unused method.The
GetStringFromHeadermethod is defined but never called in the codebase. Remove it unless it's needed for future planned work.Apply this diff:
- /// <summary> - /// Get a string value from the http header by its key - /// </summary> - /// <param name="httpContext"></param> - /// <param name="key"></param> - /// <returns></returns> - private string? GetStringFromHeader(HttpContext httpContext, string key) - { - if (httpContext.Request.Headers.TryGetValue(key, out StringValues headerValues)) - { - return headerValues.FirstOrDefault(); - } - - return null; - } -
89-100: Consider extracting OxS header extraction to a shared utility.The OxS header extraction logic is duplicated in
HttpRequestInterceptor.cs(lines 19-26). Consider extracting this to a shared utility method to reduce duplication and ensure consistent behavior across components.Note: This implementation correctly uses
.FirstOrDefault()forStringValuesconversion (line 97), which is more explicit than the direct assignment inHttpRequestInterceptor.cs.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
src/Simplic.OxS.GrpcSample.Client/Program.cs(1 hunks)src/Simplic.OxS.MessageBroker/Filter/ConsumeContextFilter.cs(1 hunks)src/Simplic.OxS.MessageBroker/Filter/PublishUserHeaderFilter.cs(1 hunks)src/Simplic.OxS.MessageBroker/Filter/SendUserHeaderFilter.cs(1 hunks)src/Simplic.OxS.Server/Filter/RequestContextActionFilter.cs(2 hunks)src/Simplic.OxS.Server/Filter/RequestContextHubFilter.cs(3 hunks)src/Simplic.OxS.Server/Implementations/RequestContext.cs(1 hunks)src/Simplic.OxS.Server/Middleware/HttpRequestInterceptor.cs(1 hunks)src/Simplic.OxS/Constants.cs(1 hunks)src/Simplic.OxS/IRequestContext.cs(1 hunks)
🧰 Additional context used
🧠 Learnings (5)
📚 Learning: 2025-11-26T09:38:34.885Z
Learnt from: CR
Repo: simplic/simplic-oxs-accounting PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-26T09:38:34.885Z
Learning: Applies to **/*.cs : Never include OrganizationId in response objects; client already knows it
Applied to files:
src/Simplic.OxS.GrpcSample.Client/Program.cs
📚 Learning: 2025-08-26T05:52:00.195Z
Learnt from: CR
Repo: simplic/simplic-oxs-storage-management PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-26T05:52:00.195Z
Learning: Applies to {Program.cs,Startup.cs,**/Controllers/**/*.cs,**/Authorization/**/*.cs} : Use JWT Bearer authentication; support OAuth 2.0/OpenID Connect; role-based and policy-based authorization; integrate with Microsoft Entra ID; apply consistent security to controllers and minimal APIs
Applied to files:
src/Simplic.OxS.Server/Middleware/HttpRequestInterceptor.cssrc/Simplic.OxS.Server/Filter/RequestContextActionFilter.cs
📚 Learning: 2025-08-26T05:52:00.195Z
Learnt from: CR
Repo: simplic/simplic-oxs-storage-management PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-26T05:52:00.195Z
Learning: Applies to {Program.cs,**/Middleware/**/*.cs,**/Models/**/*.cs,**/Validators/**/*.cs,**/Filters/**/*.cs} : Use Data Annotations and FluentValidation; customize validation pipeline responses; implement global exception handling middleware; produce consistent error responses using RFC 7807 Problem Details
Applied to files:
src/Simplic.OxS.Server/Filter/RequestContextActionFilter.cs
📚 Learning: 2025-11-26T09:38:34.885Z
Learnt from: CR
Repo: simplic/simplic-oxs-accounting PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-26T09:38:34.885Z
Learning: Use built-in ASP.NET Core features when possible before introducing third-party dependencies
Applied to files:
src/Simplic.OxS.Server/Filter/RequestContextActionFilter.cssrc/Simplic.OxS.Server/Filter/RequestContextHubFilter.cs
📚 Learning: 2025-08-26T05:52:00.195Z
Learnt from: CR
Repo: simplic/simplic-oxs-storage-management PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-26T05:52:00.195Z
Learning: Applies to {Program.cs,Startup.cs,**/Controllers/**/*.cs} : Enforce HTTP method semantics: GET (no body, return body, safe/idempotent/cacheable), POST (create, return resource+location), PUT (replace, idempotent), PATCH (partial update, idempotent), DELETE (may have body, idempotent)
Applied to files:
src/Simplic.OxS.Server/Filter/RequestContextActionFilter.cs
🧬 Code graph analysis (3)
src/Simplic.OxS.Server/Filter/RequestContextActionFilter.cs (2)
src/Simplic.OxS/Constants.cs (1)
Constants(6-39)src/Simplic.OxS.Server/Constants.cs (1)
Constants(6-37)
src/Simplic.OxS.Server/Filter/RequestContextHubFilter.cs (3)
src/Simplic.OxS.Server/Middleware/HttpRequestInterceptor.cs (3)
ValueTask(10-50)Guid(56-67)Guid(73-84)src/Simplic.OxS/Constants.cs (1)
Constants(6-39)src/Simplic.OxS.Server/Filter/RequestContextActionFilter.cs (3)
Guid(81-92)Guid(98-109)Guid(115-126)
src/Simplic.OxS.MessageBroker/Filter/ConsumeContextFilter.cs (1)
src/Simplic.OxS/Constants.cs (1)
Constants(6-39)
🔇 Additional comments (7)
src/Simplic.OxS/IRequestContext.cs (1)
22-27: LGTM! Interface extension for custom header propagation.The addition of
OxSHeadersto the interface is well-documented and appropriate for propagating custom OxS-prefixed headers through the request pipeline. The property typeIDictionary<string, string>aligns with typical header storage patterns.src/Simplic.OxS.MessageBroker/Filter/ConsumeContextFilter.cs (1)
40-47: LGTM! Correct header extraction from ConsumeContext.The implementation correctly filters and populates OxS-prefixed headers from the MassTransit ConsumeContext. The case-insensitive comparison and null-safe value handling are appropriate.
src/Simplic.OxS.MessageBroker/Filter/PublishUserHeaderFilter.cs (1)
28-31: LGTM! Header propagation to publish context.The implementation correctly forwards OxSHeaders from the request context into the MassTransit publish headers with proper null checking.
src/Simplic.OxS/Constants.cs (1)
32-38: LGTM! Well-defined header prefix constant.The
OxSHeaderPrefixconstant centralizes the "oxs-" prefix and is well-documented. This promotes consistency across the codebase when handling OxS-specific headers.src/Simplic.OxS.MessageBroker/Filter/SendUserHeaderFilter.cs (1)
31-34: LGTM! Header propagation to send context.The implementation correctly forwards OxSHeaders from the request context into the MassTransit send headers with proper null checking, consistent with the publish filter.
src/Simplic.OxS.GrpcSample.Client/Program.cs (1)
151-151: LGTM! Mock implementation updated correctly.The
MockRequestContexthas been properly updated to implement the newOxSHeadersinterface member with an appropriate default value.src/Simplic.OxS.Server/Implementations/RequestContext.cs (1)
23-28: LGTM! Clean implementation with sensible defaults.The
OxSHeadersproperty is properly implemented with a default empty dictionary, which prevents null reference issues. The documentation clearly explains the purpose with a helpful example.
|
|
||
| // Set request header if none are set yet | ||
| if (requestContext.OxSHeaders == null || requestContext.OxSHeaders.Any() == false) | ||
| { | ||
| requestContext.OxSHeaders = new Dictionary<string, string>(); | ||
| foreach (var header in requestContext.OxSHeaders) | ||
| { | ||
| if (header.Key.StartsWith(Simplic.OxS.Constants.OxSHeaderPrefix, StringComparison.OrdinalIgnoreCase)) | ||
| { | ||
| requestContext.OxSHeaders[header.Key] = header.Value; | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Fix critical logic error - iterating over empty dictionary instead of HTTP headers.
Lines 58-64 iterate over the newly created empty requestContext.OxSHeaders dictionary instead of the actual HTTP request headers from executionContext.HttpContext.Request.Headers. This means OxS headers will never be populated from incoming HTTP requests.
Apply this diff to fix the logic:
// Set request header if none are set yet
if (requestContext.OxSHeaders == null || requestContext.OxSHeaders.Any() == false)
{
requestContext.OxSHeaders = new Dictionary<string, string>();
- foreach (var header in requestContext.OxSHeaders)
+ foreach (var header in executionContext.HttpContext.Request.Headers)
{
if (header.Key.StartsWith(Simplic.OxS.Constants.OxSHeaderPrefix, StringComparison.OrdinalIgnoreCase))
{
requestContext.OxSHeaders[header.Key] = header.Value;
}
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Set request header if none are set yet | |
| if (requestContext.OxSHeaders == null || requestContext.OxSHeaders.Any() == false) | |
| { | |
| requestContext.OxSHeaders = new Dictionary<string, string>(); | |
| foreach (var header in requestContext.OxSHeaders) | |
| { | |
| if (header.Key.StartsWith(Simplic.OxS.Constants.OxSHeaderPrefix, StringComparison.OrdinalIgnoreCase)) | |
| { | |
| requestContext.OxSHeaders[header.Key] = header.Value; | |
| } | |
| } | |
| } | |
| // Set request header if none are set yet | |
| if (requestContext.OxSHeaders == null || requestContext.OxSHeaders.Any() == false) | |
| { | |
| requestContext.OxSHeaders = new Dictionary<string, string>(); | |
| foreach (var header in executionContext.HttpContext.Request.Headers) | |
| { | |
| if (header.Key.StartsWith(Simplic.OxS.Constants.OxSHeaderPrefix, StringComparison.OrdinalIgnoreCase)) | |
| { | |
| requestContext.OxSHeaders[header.Key] = header.Value; | |
| } | |
| } | |
| } |
🤖 Prompt for AI Agents
In src/Simplic.OxS.Server/Filter/RequestContextActionFilter.cs around lines 53
to 65, the current code creates a new empty requestContext.OxSHeaders and then
(incorrectly) iterates that empty dictionary instead of the incoming HTTP
request headers, so OxS headers are never populated; change the loop to
enumerate executionContext.HttpContext.Request.Headers, and for each header
whose key starts with Simplic.OxS.Constants.OxSHeaderPrefix (use
StringComparison.OrdinalIgnoreCase) add or set
requestContext.OxSHeaders[header.Key] = header.Value (convert header.Value to a
string or join multiple values) after ensuring requestContext.OxSHeaders is
instantiated when null or empty.
| if (context.Request.Headers.TryGetValue(Constants.HttpHeaderCorrelationIdKey, out StringValues correlationIds)) | ||
| { | ||
| correlationId = correlationIds.FirstOrDefault(k => k == Constants.HttpHeaderCorrelationIdKey) ?? | ||
| Guid.NewGuid().ToString(); | ||
| } |
There was a problem hiding this comment.
Fix incorrect correlation ID extraction logic.
Line 33 has a logic error. The lambda parameter k represents a header value from the StringValues collection, not a key. Comparing k == Constants.HttpHeaderCorrelationIdKey will never match and always fall back to generating a new GUID, defeating the purpose of reading the header.
Apply this diff:
- correlationId = correlationIds.FirstOrDefault(k => k == Constants.HttpHeaderCorrelationIdKey) ??
- Guid.NewGuid().ToString();
+ correlationId = correlationIds.FirstOrDefault() ?? Guid.NewGuid().ToString();📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (context.Request.Headers.TryGetValue(Constants.HttpHeaderCorrelationIdKey, out StringValues correlationIds)) | |
| { | |
| correlationId = correlationIds.FirstOrDefault(k => k == Constants.HttpHeaderCorrelationIdKey) ?? | |
| Guid.NewGuid().ToString(); | |
| } | |
| if (context.Request.Headers.TryGetValue(Constants.HttpHeaderCorrelationIdKey, out StringValues correlationIds)) | |
| { | |
| correlationId = correlationIds.FirstOrDefault() ?? Guid.NewGuid().ToString(); | |
| } |
🤖 Prompt for AI Agents
In src/Simplic.OxS.Server/Middleware/HttpRequestInterceptor.cs around lines 31
to 35, the code incorrectly compares header values to the header name; replace
the predicate so you extract the first header value (e.g.,
correlationIds.FirstOrDefault()) and fall back to Guid.NewGuid().ToString() if
empty or null; ensure you trim/check for empty string before using it so an
actual header value is used rather than always generating a new GUID.
Summary by CodeRabbit
Release Notes
✏️ Tip: You can customize this high-level summary in your review settings.