Add Text Xml format and make serialisationformat pluggable#546
Merged
Daniel-Svensson merged 41 commits intoOpenRIAServices:mainfrom Apr 2, 2026
Merged
Conversation
…date options accordingly * add more nullable annotations and fix a potential minor issue
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
src/OpenRiaServices.Hosting.AspNetCore/Framework/AspNetCore/Operations/OperationInvoker.cs
Fixed
Show fixed
Hide fixed
src/OpenRiaServices.Hosting.AspNetCore/Framework/AspNetCore/Operations/OperationInvoker.cs
Dismissed
Show dismissed
Hide dismissed
Daniel-Svensson
commented
Jan 29, 2026
src/OpenRiaServices.Hosting.AspNetCore/Framework/AspNetCore/OpenRiaServicesOptionsBuilder.cs
Outdated
Show resolved
Hide resolved
src/OpenRiaServices.Hosting.AspNetCore/Framework/AspNetCore/Operations/OperationInvoker.cs
Dismissed
Show dismissed
Hide dismissed
src/OpenRiaServices.Hosting.AspNetCore/Framework/AspNetCore/Operations/OperationInvoker.cs
Dismissed
Show dismissed
Hide dismissed
src/OpenRiaServices.Hosting.AspNetCore/Framework/AspNetCore/Operations/OperationInvoker.cs
Dismissed
Show dismissed
Hide dismissed
Refactored the HTTP-based DomainClient infrastructure to support pluggable serialization formats (binary and XML) via new abstractions. Introduced DataContractHttpDomainClient and HttpDomainClientFactory base classes to centralize request/response and HttpClient logic. Added XmlHttpDomainClient and XmlHttpDomainClientFactory for XML support. Renamed and generalized the serialization helper. Removed duplicated code, making the system more modular and maintainable.
* Need special case "empty" element doe to differences with binary xml
src/OpenRiaServices.Client.DomainClients.Http/Framework/Http/DataContractHttpDomainClient.cs
Fixed
Show fixed
Hide fixed
src/OpenRiaServices.Client.DomainClients.Http/Framework/Http/DataContractHttpDomainClient.cs
Fixed
Show fixed
Hide fixed
src/OpenRiaServices.Hosting.AspNetCore/Framework/AspNetCore/OpenRiaServicesOptions.cs
Dismissed
Show dismissed
Hide dismissed
Daniel-Svensson
commented
Feb 2, 2026
src/OpenRiaServices.Client.DomainClients.Http/Framework/XmlHttpDomainClientFactory.cs
Show resolved
Hide resolved
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a pluggable binary/text XML serialization pipeline and providers, centralizes HTTP domain-client factories and a DataContract-based HTTP client base, moves operation parameter I/O and response/error writing through per-operation serializers with content negotiation, adds XML media-type support, and adds CodeQL CI configuration. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Invoker as OperationInvoker
participant Prov as SerializationProviders
participant Ser as RequestSerializer
participant Domain as DomainService
participant HttpCtx as HttpContext
Client->>Invoker: HTTP Request (Content-Type, Accept)
Invoker->>Invoker: SetDefaultResponseHeaders()
Invoker->>Prov: GetRequestSerializer(operation)
Prov-->>Invoker: RequestSerializer
alt No writer matches Accept
Invoker-->>Client: 406 Not Acceptable
else Writer available
Invoker->>Ser: TryGetSerializerForReading(HttpContext)
alt No reader matches Content-Type
Invoker-->>Client: 415 Unsupported Media Type
else Reader available
Ser->>Invoker: ReadParametersFromBodyAsync()/ReadSubmitRequestAsync()
Invoker->>Domain: Invoke operation with parameters
alt Exception or validation fault
Invoker->>Ser: WriteErrorAsync(...)
else Success
Invoker->>Ser: WriteResponseAsync(result)
end
Ser-->>Client: HTTP Response (negotiated format)
end
end
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Fix all issues with AI agents
In
`@src/OpenRiaServices.Client.DomainClients.Http/Framework/Http/DataContractHttpDomainClient.cs`:
- Around line 117-172: The await on responseTask in QueryAsyncCoreContinuation
captures the synchronization context; update the call site so responseTask is
awaited with ConfigureAwait(false). Specifically, where responseTask is defined
(var responseTask = ExecuteRequestAsync(...)) and then awaited inside the local
function QueryAsyncCoreContinuation, ensure the await uses await
responseTask.ConfigureAwait(false) (or await (awaitable).ConfigureAwait(false))
to match the other ConfigureAwait(false) usages; keep the ExecuteRequestAsync
invocation and QueryAsyncCore/QueryAsyncCoreContinuation structure unchanged
except for adding ConfigureAwait(false) when awaiting responseTask.
In
`@src/OpenRiaServices.Client.DomainClients.Http/Framework/XmlHttpDomainClientFactory.cs`:
- Around line 14-22: Add a matching constructor overload to
XmlHttpDomainClientFactory that accepts Func<HttpClient> to mirror
BinaryHttpDomainClientFactory: implement public XmlHttpDomainClientFactory(Uri
serverBaseUri, Func<HttpClient> httpClientFactory) that simply calls the
base(serverBaseUri, httpClientFactory) so callers can supply a Func<HttpClient>
the same way as with BinaryHttpDomainClientFactory and preserve API parity.
In
`@src/OpenRiaServices.Hosting.AspNetCore/Framework/AspNetCore/OpenRiaServicesOptionsBuilder.cs`:
- Around line 25-43: The WithTextXmlSerialization method duplicates provider
wiring (new Serialization.TextXmlSerializationProvider, cache sharing and list
concatenation) instead of using the centralized helper; replace the manual
creation and list manipulation inside Services.Configure<OpenRiaServicesOptions>
with a single call to options.AddTextXmlSerializer() so AddTextXmlSerializer
handles provider instantiation, cache sharing and ordering (remove
provider._perDomainServiceDataContractCache assignment and
options.SerializationProviders = [...] logic).
In
`@src/OpenRiaServices.Hosting.AspNetCore/Framework/AspNetCore/Operations/OperationInvoker.cs`:
- Around line 88-136: GetSerializerForWrite/DoContentNegotiation always falls
back to serializers[0], preventing callers from ever receiving null (and thus
never returning 406); change DoContentNegotiation so that when the client
explicitly provides Accept or Content-Type headers but none of the
RequestSerializer.CanWrite checks match, it returns null instead of
serializers[0], while preserving the current fallback-to-first behavior only
when no Accept/Content-Type header was provided; update references to
RequestSerializer and the GetSerializerForWrite call sites to handle a possible
null return (emit 406) where appropriate.
- Around line 42-63: The RequestSerializers getter currently assigns
_requestSerializers directly from CreateSerializersArray(), which can publish a
partially-constructed array to other threads; change CreateSerializersArray
(used by the RequestSerializers property) to build the RequestSerializer[] in a
local variable and then use Interlocked.CompareExchange(ref _requestSerializers,
result, null) to publish the array only if _requestSerializers is still null,
then return the value of _requestSerializers (so if another thread won the race
you return that instance). This ensures safe publication and visibility of all
array slots for _requestSerializers, referencing the existing symbols
RequestSerializers, CreateSerializersArray, _requestSerializers,
Options.SerializationProviders and RequestSerializer[].
In
`@src/OpenRiaServices.Hosting.AspNetCore/Framework/AspNetCore/Serialization/DataContractRequestSerializer.cs`:
- Around line 88-93: In DataContractRequestSerializer change the user-facing
error message passed to the BadHttpRequestException in the initialCapacity
switch (the branch that currently throws new BadHttpRequestException("invalid
lenght", ...)) to correct the typo—replace "invalid lenght" with "invalid
length" so the BadHttpRequestException message is spelled correctly.
- Around line 282-357: Make WriteErrorAsync and WriteResponseAsync async Task
methods and await the bufferMemory.WriteTo(response, ct) call instead of
returning its Task directly so the using var bufferMemory scope (and pooled
buffers returned by BinaryMessageWriter.Return) is not disposed before the write
completes; keep the existing cancellation checks, retain the try/catch that
calls messageWriter?.Clear(), and ensure you set messageWriter = null after
Return so the buffer stays alive until after the awaited WriteTo completes.
src/OpenRiaServices.Client.DomainClients.Http/Framework/Http/DataContractHttpDomainClient.cs
Show resolved
Hide resolved
src/OpenRiaServices.Client.DomainClients.Http/Framework/XmlHttpDomainClientFactory.cs
Show resolved
Hide resolved
src/OpenRiaServices.Hosting.AspNetCore/Framework/AspNetCore/OpenRiaServicesOptionsBuilder.cs
Show resolved
Hide resolved
src/OpenRiaServices.Hosting.AspNetCore/Framework/AspNetCore/Operations/OperationInvoker.cs
Show resolved
Hide resolved
src/OpenRiaServices.Hosting.AspNetCore/Framework/AspNetCore/Operations/OperationInvoker.cs
Show resolved
Hide resolved
...vices.Hosting.AspNetCore/Framework/AspNetCore/Serialization/DataContractRequestSerializer.cs
Show resolved
Hide resolved
...vices.Hosting.AspNetCore/Framework/AspNetCore/Serialization/DataContractRequestSerializer.cs
Outdated
Show resolved
Hide resolved
Member
Author
|
@coderabbitai generate docstrings |
Contributor
✅ Actions performedInitiated docstring generation; will generate only if new commits exist. |
Contributor
|
Note Docstrings generation - SUCCESS |
coderabbitai bot
added a commit
that referenced
this pull request
Feb 4, 2026
Docstrings generation was requested by @Daniel-Svensson. * #546 (comment) The following files were modified: * `src/OpenRiaServices.Client.DomainClients.Http/Framework/BinaryHttpDomainClientFactory.cs` * `src/OpenRiaServices.Client.DomainClients.Http/Framework/Http/BinaryHttpDomainClient.cs` * `src/OpenRiaServices.Client.DomainClients.Http/Framework/Http/DataContractHttpDomainClient.cs` * `src/OpenRiaServices.Client.DomainClients.Http/Framework/Http/DataContractSerializationHelper.cs` * `src/OpenRiaServices.Client.DomainClients.Http/Framework/HttpDomainClientFactory.cs` * `src/OpenRiaServices.Client.DomainClients.Http/Framework/XmlHttpDomainClientFactory.cs` * `src/OpenRiaServices.Hosting.AspNetCore/Framework/AspNetCore/OpenRiaServicesEndpointDataSource.cs` * `src/OpenRiaServices.Hosting.AspNetCore/Framework/AspNetCore/OpenRiaServicesOptions.cs` * `src/OpenRiaServices.Hosting.AspNetCore/Framework/AspNetCore/OpenRiaServicesOptionsBuilder.cs` * `src/OpenRiaServices.Hosting.AspNetCore/Framework/AspNetCore/Operations/InvokeOperationInvoker.cs` * `src/OpenRiaServices.Hosting.AspNetCore/Framework/AspNetCore/Operations/OperationInvoker.cs` * `src/OpenRiaServices.Hosting.AspNetCore/Framework/AspNetCore/Operations/QueryOperationInvoker.cs` * `src/OpenRiaServices.Hosting.AspNetCore/Framework/AspNetCore/Operations/SubmitOperationInvoker.cs` * `src/OpenRiaServices.Hosting.AspNetCore/Framework/AspNetCore/Serialization/BinaryMessageReader.cs` * `src/OpenRiaServices.Hosting.AspNetCore/Framework/AspNetCore/Serialization/BinaryMessageWriter.cs` * `src/OpenRiaServices.Hosting.AspNetCore/Framework/AspNetCore/Serialization/DataContractCache.cs` * `src/OpenRiaServices.Hosting.AspNetCore/Framework/AspNetCore/Serialization/DataContractRequestSerializer.cs` * `src/OpenRiaServices.Hosting.AspNetCore/Framework/AspNetCore/Serialization/DataContractSerializationProvider.cs` * `src/OpenRiaServices.Hosting.AspNetCore/Framework/AspNetCore/Serialization/SerializationProvider.cs` * `src/Test/OpenRiaservices.EndToEnd.AspNetCore.Test/Main.cs`
Member
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
1 similar comment
Contributor
✅ Actions performedReview triggered.
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 13
♻️ Duplicate comments (5)
src/OpenRiaServices.Client.DomainClients.Http/Framework/Http/DataContractHttpDomainClient.cs (3)
290-301: 🧹 Nitpick | 🔵 TrivialConsider using
.Where()to filter null parameter values.The foreach loop implicitly filters for non-null values. Using LINQ's
.Where()would make the filtering intent more explicit.♻️ Proposed refactor
if (parameters != null && parameters.Count > 0) { var methodParameters = GetMethodParameters(operationName); - foreach (var param in parameters) + foreach (var param in parameters.Where(p => p.Value != null)) { - if (param.Value != null) - { - uriBuilder.Append(i++ == 0 ? '?' : '&'); - uriBuilder.Append(Uri.EscapeDataString(param.Key)); - uriBuilder.Append('='); - var parameterType = methodParameters.GetTypeForMethodParameter(param.Key); - var value = WebQueryStringConverter.ConvertValueToString(param.Value, parameterType); - uriBuilder.Append(Uri.EscapeDataString(value)); - } + uriBuilder.Append(i++ == 0 ? '?' : '&'); + uriBuilder.Append(Uri.EscapeDataString(param.Key)); + uriBuilder.Append('='); + var parameterType = methodParameters.GetTypeForMethodParameter(param.Key); + var value = WebQueryStringConverter.ConvertValueToString(param.Value, parameterType); + uriBuilder.Append(Uri.EscapeDataString(value)); } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/OpenRiaServices.Client.DomainClients.Http/Framework/Http/DataContractHttpDomainClient.cs` around lines 290 - 301, Replace the implicit null-filtering foreach with an explicit LINQ filter to clarify intent: iterate over parameters.Where(p => p.Value != null) instead of checking inside the loop; keep the existing logic that appends '?' or '&' using the i counter, use methodParameters.GetTypeForMethodParameter(param.Key) and WebQueryStringConverter.ConvertValueToString(param.Value, parameterType) as before, and ensure the index variable i is still incremented for each included parameter (so move i++ into the filtered loop body).
46-51:⚠️ Potential issue | 🟡 MinorAdd null validation for
httpClientparameter.The constructor validates
serviceInterfacebut nothttpClient. Passing null will cause aNullReferenceExceptionlater whenHttpClient.SendAsyncorHttpClient.GetAsyncis called inPostAsync/GetAsync.🛡️ Proposed fix
private protected DataContractHttpDomainClient(HttpClient httpClient, Type serviceInterface) { + if (httpClient is null) + throw new ArgumentNullException(nameof(httpClient)); if (serviceInterface is null) throw new ArgumentNullException(nameof(serviceInterface)); HttpClient = httpClient;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/OpenRiaServices.Client.DomainClients.Http/Framework/Http/DataContractHttpDomainClient.cs` around lines 46 - 51, The constructor DataContractHttpDomainClient(HttpClient httpClient, Type serviceInterface) currently validates serviceInterface but not httpClient; add a null check for the httpClient parameter at the start of the constructor and throw an ArgumentNullException(nameof(httpClient)) if it is null so the HttpClient property is never set to null and subsequent calls in PostAsync/GetAsync won't throw NullReferenceException.
212-269:⚠️ Potential issue | 🟡 MinorDispose
HttpRequestMessageafter use.
HttpRequestMessageimplementsIDisposablebut is created at line 214 and sent at line 268 without being disposed. AfterSendAsynccompletes, the request should be disposed to release resources.🛡️ Proposed fix
private Task<HttpResponseMessage> PostAsync(string operationName, IDictionary<string, object> parameters, List<ServiceQueryPart> queryOptions, CancellationToken cancellationToken) { - var request = new HttpRequestMessage(HttpMethod.Post, operationName); - - using (var ms = new MemoryStream()) - using (var writer = CreateWriter(ms)) + async Task<HttpResponseMessage> PostAsyncCore() { - // Write message - var rootNamespace = "http://tempuri.org/"; - bool hasQueryOptions = queryOptions != null && queryOptions.Count > 0; - - if (hasQueryOptions) - { - writer.WriteStartElement("MessageRoot"); - writer.WriteStartElement("QueryOptions"); - foreach (var queryOption in queryOptions) - { - writer.WriteStartElement("QueryOption"); - writer.WriteAttributeString("Name", queryOption.QueryOperator); - writer.WriteAttributeString("Value", queryOption.Expression); - writer.WriteEndElement(); - } - writer.WriteEndElement(); - } - writer.WriteStartElement(operationName, rootNamespace); // <OperationName> - - // Write all parameters - if (parameters != null && parameters.Count > 0) + using var request = new HttpRequestMessage(HttpMethod.Post, operationName); + using (var ms = new MemoryStream()) + using (var writer = CreateWriter(ms)) { - MethodParameters methodParameter = GetMethodParameters(operationName); - foreach (var param in parameters) + // Write message + var rootNamespace = "http://tempuri.org/"; + bool hasQueryOptions = queryOptions != null && queryOptions.Count > 0; + + if (hasQueryOptions) { - writer.WriteStartElement(param.Key); // <ParameterName> - if (param.Value != null) - { - var parameterType = methodParameter.GetTypeForMethodParameter(param.Key); - var serializer = GetSerializer(parameterType); - serializer.WriteObjectContent(writer, param.Value); - } - else + writer.WriteStartElement("MessageRoot"); + writer.WriteStartElement("QueryOptions"); + foreach (var queryOption in queryOptions) { - // Null input - writer.WriteAttributeString("i", "nil", "http://www.w3.org/2001/XMLSchema-instance", "true"); + writer.WriteStartElement("QueryOption"); + writer.WriteAttributeString("Name", queryOption.QueryOperator); + writer.WriteAttributeString("Value", queryOption.Expression); + writer.WriteEndElement(); } - writer.WriteEndElement(); // </ParameterName> + writer.WriteEndElement(); } + writer.WriteStartElement(operationName, rootNamespace); // <OperationName> + + // Write all parameters + if (parameters != null && parameters.Count > 0) + { + MethodParameters methodParameter = GetMethodParameters(operationName); + foreach (var param in parameters) + { + writer.WriteStartElement(param.Key); // <ParameterName> + if (param.Value != null) + { + var parameterType = methodParameter.GetTypeForMethodParameter(param.Key); + var serializer = GetSerializer(parameterType); + serializer.WriteObjectContent(writer, param.Value); + } + else + { + // Null input + writer.WriteAttributeString("i", "nil", "http://www.w3.org/2001/XMLSchema-instance", "true"); + } + writer.WriteEndElement(); // </ParameterName> + } + } + + writer.WriteEndDocument(); // </OperationName> and </MessageRoot> if present + writer.Flush(); + + ms.TryGetBuffer(out ArraySegment<byte> buffer); + request.Content = new ByteArrayContent(buffer.Array, buffer.Offset, buffer.Count); + request.Content.Headers.ContentType = new System.Net.Http.Headers.MediaTypeHeaderValue(ContentType); } - writer.WriteEndDocument(); // </OperationName> and </MessageRoot> if present - writer.Flush(); - - ms.TryGetBuffer(out ArraySegment<byte> buffer); - request.Content = new ByteArrayContent(buffer.Array, buffer.Offset, buffer.Count); - request.Content.Headers.ContentType = new System.Net.Http.Headers.MediaTypeHeaderValue(ContentType); + return await HttpClient.SendAsync(request, DefaultHttpCompletionOption, cancellationToken).ConfigureAwait(false); } - return HttpClient.SendAsync(request, DefaultHttpCompletionOption, cancellationToken); + return PostAsyncCore(); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/OpenRiaServices.Client.DomainClients.Http/Framework/Http/DataContractHttpDomainClient.cs` around lines 212 - 269, The PostAsync method creates an HttpRequestMessage (in DataContractHttpDomainClient.PostAsync) but never disposes it; change the method to await the send and dispose the request after SendAsync completes (e.g. make PostAsync async, call var response = await HttpClient.SendAsync(request, ...), dispose the HttpRequestMessage in a using or try/finally, then return the HttpResponseMessage). Ensure you do not dispose the HttpResponseMessage before returning it.src/OpenRiaServices.Hosting.AspNetCore/Framework/AspNetCore/OpenRiaServicesOptionsBuilder.cs (1)
33-35:⚠️ Potential issue | 🟠 MajorInstantiate the serializer inside the options callback.
serializationProvideris created once here and then captured byServices.Configure(...). BecauseOpenRiaServicesOptions.AddSerializationProvider(...)mutates DataContract-based providers when it wires the shared cache, reusing one instance here can leak that mutable state across otherwise independentOpenRiaServicesOptionsinstances. Create the provider inside the callback, or pass a factory instead.♻️ Proposed refactor
- public OpenRiaServicesOptionsBuilder AddXmlSerialization(bool defaultProvider = false) - { - return AddSerializationProvider(new Serialization.TextXmlSerializationProvider(), defaultProvider); - } + public OpenRiaServicesOptionsBuilder AddXmlSerialization(bool defaultProvider = false) + { + return AddSerializationProvider(() => new Serialization.TextXmlSerializationProvider(), defaultProvider); + } @@ - private OpenRiaServicesOptionsBuilder AddSerializationProvider(Serialization.ISerializationProvider serializationProvider, bool defaultProvider) + private OpenRiaServicesOptionsBuilder AddSerializationProvider(Func<Serialization.ISerializationProvider> serializationProviderFactory, bool defaultProvider) { Services.Configure<OpenRiaServicesOptions>(options => { - options.AddSerializationProvider(serializationProvider, defaultProvider); + options.AddSerializationProvider(serializationProviderFactory(), defaultProvider); }); return this; }Also applies to: 52-63
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/OpenRiaServices.Hosting.AspNetCore/Framework/AspNetCore/OpenRiaServicesOptionsBuilder.cs` around lines 33 - 35, The issue is that AddXmlSerialization currently instantiates a single Serialization.TextXmlSerializationProvider and captures it into Services.Configure, which can leak mutable state when OpenRiaServicesOptions.AddSerializationProvider mutates DataContract-based providers; change AddXmlSerialization to create the provider inside the options callback (or accept/pass a factory) instead of constructing it once—i.e., move new Serialization.TextXmlSerializationProvider() into the Configure lambda used by Services.Configure (and apply the same fix to the other similar methods in the file such as the block around lines 52-63 that instantiate other serialization providers).src/OpenRiaServices.Hosting.AspNetCore/Framework/AspNetCore/Operations/OperationInvoker.cs (1)
99-147:⚠️ Potential issue | 🟠 MajorUnsupported
Acceptvalues still negotiate to the default writer.The early return for a single serializer and the unconditional fallback at the end still hand back
serializers[0]when the client explicitly asked for an unsupported format. That makes the 406 path unreachable and can return a payload the client already rejected. Only use the default when neitherAcceptnorContent-Typewas supplied; otherwise let callers emitStatus406NotAcceptable.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/OpenRiaServices.Hosting.AspNetCore/Framework/AspNetCore/Operations/OperationInvoker.cs` around lines 99 - 147, Change GetSerializerForWrite/DoContentNegotiation so we don't unconditionally fall back to serializers[0] when the client sent Accept or Content-Type headers; only return the default serializer if neither header was supplied. Specifically, remove the early "if (serializers.Length == 1) return serializers[0];" shortcut and modify DoContentNegotiation to return null (or a sentinel) when Accept and Content-Type are present but no serializer.CanWrite(...) matches; keep returning serializers[0] only when both Accept and Content-Type are absent. Ensure callers that invoke GetSerializerForWrite handle a null result and emit Status406NotAcceptable.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Changelog.md`:
- Around line 37-40: The release note uses a misspelled attribute name
`EntityAssocationAttribute`; update the Changelog entry to the correct API name
`EntityAssociationAttribute` wherever `EntityAssocationAttribute` appears and
ensure the sentence referencing `AssociationAttribute` remains unchanged (keep
the explanation that `EntityAssociationAttribute` should now be used instead of
`AssociationAttribute` and the note about backwards compatibility).
In
`@src/OpenRiaServices.Client.DomainClients.Http/Framework/Http/DataContractHttpDomainClient.cs`:
- Line 349: There is a lingering TODO about OpenRia 5.0 returning different
status codes in DataContractHttpDomainClient; either implement the new
status-code handling now or track it by creating an issue and replacing the TODO
with a clear issue/PR reference. Locate the DataContractHttpDomainClient class
(methods handling HTTP responses, e.g., the response processing/exception
mapping logic) and either add handling/mapping for the OpenRia 5.0 status codes
or remove the inline TODO and add a comment like "Tracked: GH-<issue-number>"
after opening an issue that describes the required status-code changes and
expected behavior. Ensure the change references DataContractHttpDomainClient so
future readers know where the behavior must be updated.
- Line 389: Update the inline comment in DataContractHttpDomainClient (around
the code handling the "ResultNode") to correct the typo: change "XmlElemtnt" to
"XmlElement" so the comment reads clearly (e.g., "XmlElement returns the
'ResultNode' unless we step into the contents"); no code logic changes required.
- Around line 384-385: Fix the typo in the inline comment above the call to
VerifyReaderIsAtNode(reader, operationName, "Result") in
DataContractHttpDomainClient (Http/DataContractHttpDomainClient.cs): change
"Validate that we are no on ****Result node" to "Validate that we are now on
****Result node" so the comment accurately reflects the check performed by
VerifyReaderIsAtNode.
- Around line 376-377: Update the inline comment above the call to
VerifyReaderIsAtNode in DataContractHttpDomainClient (the line referencing
reader and operationName) to correct the typo: change "no" to "now" so it reads
"Validate that we are now on ****Response node". Keep the surrounding comment
style and spacing consistent with the file's existing comments.
- Around line 346-351: Fix the comment typos in the DataContractHttpDomainClient
class around the using (response) block: change "respnse" to "response" and
"pase on msbin" to "parse on msbin" (and optionally "sens" → "sense" if present)
in the comment lines inside the method where the using (response) is declared so
the comments read clearly and correctly describe reading/parsing response
content for non-200 status codes.
- Around line 413-417: The string.Format call used when throwing
DomainOperationException in DataContractHttpDomainClient should use
CultureInfo.InvariantCulture for consistency and to silence locale-sensitive
warnings; update the string.Format invocation that builds
Resources.DomainClient_UnexpectedResultContent (which concatenates operationName
+ postfix and reader.LocalName) to pass CultureInfo.InvariantCulture as the
first argument so the formatted message uses the invariant culture before
creating the DomainOperationException.
- Line 486: Rename the misspelled local variable isEmtpy to isEmpty in
DataContractHttpDomainClient (the variable assigned from reader.IsEmptyElement)
and update all usages in that method/block to use isEmpty so the identifier
matches its intent and avoids the typo.
In
`@src/OpenRiaServices.Hosting.AspNetCore/Framework/AspNetCore/OpenRiaServicesServiceCollectionExtensions.cs`:
- Around line 30-35: The public overload AddOpenRiaServices(this
IServiceCollection services, Action<OpenRiaServicesOptions> configure) currently
calls services.Configure(...) before validating services, which can cause a
NullReferenceException; add a null guard for services
(ArgumentNullException.ThrowIfNull(services)) at the start of this method
(keeping the existing ThrowIfNull(configure) as well) so services is validated
before calling services.Configure and then call return
AddOpenRiaServices(services) as before.
In
`@src/OpenRiaServices.Hosting.AspNetCore/Framework/AspNetCore/Operations/OperationInvoker.cs`:
- Around line 165-171: The BadHttpRequestException currently includes the raw
query value which can leak PII; update the error thrown in the catch block
inside OperationInvoker (where s_queryStringConverter.ConvertStringToValue is
called and ExceptionHandlingUtility.IsFatal is checked) to omit the
user-supplied value and only reference the parameter name (and optionally
parameter type) in the message (e.g., "Failed to parse parameter 'paramName'");
keep the original inner exception when constructing the BadHttpRequestException
so debugging context is preserved.
In
`@src/OpenRiaServices.Hosting.AspNetCore/Framework/AspNetCore/Serialization/DataContractRequestSerializer.cs`:
- Around line 205-283: The ReadParameters method is too complex and should be
split: keep the Submit branch but delegate to a new helper ReadSubmitRequest
(reuse existing inner method signature) and extract per-parameter logic into a
helper ReadParameterValues/ReadParameter that takes XmlDictionaryReader and
DomainOperationEntry (or ParameterDescriptor) and returns object?[]/object? for
each parameter; move handling of zero-parameter, xsi:nil, XElement special-case,
serializer lookup (_dataContractCache.GetSerializer), and element consumption
into that helper so ReadParameters only dispatches to ReadSubmitRequest, handles
empty-operation shortcuts, and calls ReadParameterValues for the normal path,
preserving the current behavior and exceptions (BadHttpRequestException,
InvalidOperationException).
- Around line 20-24: CanRead and CanWrite currently use StartsWith on the raw
contentType span which lets malformed tokens (e.g., "application/xmlx") or
parameter/casing differences slip through; update both CanRead and CanWrite to
first extract and normalize the media-type token (trim, take substring before
';', and compare using case-insensitive equality) and then match against the
canonical media types (e.g., MimeTypes.TextXml, MimeTypes.ApplicationXml and any
binary-specific media type used elsewhere). Locate and change the
implementations of CanRead and CanWrite in DataContractRequestSerializer (and
the analogous methods at lines ~34-38) to parse the token from the
ReadOnlySpan<char>, normalize (trim and ToLowerInvariant or use
OrdinalIgnoreCase comparisons), and then perform exact equality checks against
the known MIME constants instead of StartsWith.
In `@src/OpenRiaServices.Hosting.AspNetCore/Framework/README.md`:
- Around line 117-133: The two code samples are missing terminating semicolons
and the XML-only example must also register the matching domain client factory
to avoid protocol mismatch; update both snippets to end the fluent call with a
semicolon, and in the XML-only snippet add the registration call for the client
factory (e.g., chain .AddDomainClientFactory<XmlHttpDomainClientFactory>() after
.AddXmlSerialization()) so the sample compiles and uses
XmlHttpDomainClientFactory with AddOpenRiaServices, ClearSerializationProviders,
and AddXmlSerialization.
---
Duplicate comments:
In
`@src/OpenRiaServices.Client.DomainClients.Http/Framework/Http/DataContractHttpDomainClient.cs`:
- Around line 290-301: Replace the implicit null-filtering foreach with an
explicit LINQ filter to clarify intent: iterate over parameters.Where(p =>
p.Value != null) instead of checking inside the loop; keep the existing logic
that appends '?' or '&' using the i counter, use
methodParameters.GetTypeForMethodParameter(param.Key) and
WebQueryStringConverter.ConvertValueToString(param.Value, parameterType) as
before, and ensure the index variable i is still incremented for each included
parameter (so move i++ into the filtered loop body).
- Around line 46-51: The constructor DataContractHttpDomainClient(HttpClient
httpClient, Type serviceInterface) currently validates serviceInterface but not
httpClient; add a null check for the httpClient parameter at the start of the
constructor and throw an ArgumentNullException(nameof(httpClient)) if it is null
so the HttpClient property is never set to null and subsequent calls in
PostAsync/GetAsync won't throw NullReferenceException.
- Around line 212-269: The PostAsync method creates an HttpRequestMessage (in
DataContractHttpDomainClient.PostAsync) but never disposes it; change the method
to await the send and dispose the request after SendAsync completes (e.g. make
PostAsync async, call var response = await HttpClient.SendAsync(request, ...),
dispose the HttpRequestMessage in a using or try/finally, then return the
HttpResponseMessage). Ensure you do not dispose the HttpResponseMessage before
returning it.
In
`@src/OpenRiaServices.Hosting.AspNetCore/Framework/AspNetCore/OpenRiaServicesOptionsBuilder.cs`:
- Around line 33-35: The issue is that AddXmlSerialization currently
instantiates a single Serialization.TextXmlSerializationProvider and captures it
into Services.Configure, which can leak mutable state when
OpenRiaServicesOptions.AddSerializationProvider mutates DataContract-based
providers; change AddXmlSerialization to create the provider inside the options
callback (or accept/pass a factory) instead of constructing it once—i.e., move
new Serialization.TextXmlSerializationProvider() into the Configure lambda used
by Services.Configure (and apply the same fix to the other similar methods in
the file such as the block around lines 52-63 that instantiate other
serialization providers).
In
`@src/OpenRiaServices.Hosting.AspNetCore/Framework/AspNetCore/Operations/OperationInvoker.cs`:
- Around line 99-147: Change GetSerializerForWrite/DoContentNegotiation so we
don't unconditionally fall back to serializers[0] when the client sent Accept or
Content-Type headers; only return the default serializer if neither header was
supplied. Specifically, remove the early "if (serializers.Length == 1) return
serializers[0];" shortcut and modify DoContentNegotiation to return null (or a
sentinel) when Accept and Content-Type are present but no
serializer.CanWrite(...) matches; keep returning serializers[0] only when both
Accept and Content-Type are absent. Ensure callers that invoke
GetSerializerForWrite handle a null result and emit Status406NotAcceptable.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: bc9cd75e-c01e-4b0f-8709-e161656a526f
📒 Files selected for processing (12)
.github/workflows/codeql.ymlChangelog.mdsrc/OpenRiaServices.Client.DomainClients.Http/Framework/Http/DataContractHttpDomainClient.cssrc/OpenRiaServices.Hosting.AspNetCore/Framework/AspNetCore/OpenRiaServicesEndpointDataSource.cssrc/OpenRiaServices.Hosting.AspNetCore/Framework/AspNetCore/OpenRiaServicesOptions.cssrc/OpenRiaServices.Hosting.AspNetCore/Framework/AspNetCore/OpenRiaServicesOptionsBuilder.cssrc/OpenRiaServices.Hosting.AspNetCore/Framework/AspNetCore/OpenRiaServicesServiceCollectionExtensions.cssrc/OpenRiaServices.Hosting.AspNetCore/Framework/AspNetCore/Operations/OperationInvoker.cssrc/OpenRiaServices.Hosting.AspNetCore/Framework/AspNetCore/Serialization/DataContractRequestSerializer.cssrc/OpenRiaServices.Hosting.AspNetCore/Framework/README.mdsrc/Test/AspNetCoreWebsite/Program.cssrc/Test/OpenRiaservices.EndToEnd.Wcf.Test/Data/InvokeOperationTests.cs
src/OpenRiaServices.Client.DomainClients.Http/Framework/Http/DataContractHttpDomainClient.cs
Outdated
Show resolved
Hide resolved
src/OpenRiaServices.Client.DomainClients.Http/Framework/Http/DataContractHttpDomainClient.cs
Outdated
Show resolved
Hide resolved
src/OpenRiaServices.Client.DomainClients.Http/Framework/Http/DataContractHttpDomainClient.cs
Outdated
Show resolved
Hide resolved
src/OpenRiaServices.Client.DomainClients.Http/Framework/Http/DataContractHttpDomainClient.cs
Outdated
Show resolved
Hide resolved
...rvices.Hosting.AspNetCore/Framework/AspNetCore/OpenRiaServicesServiceCollectionExtensions.cs
Show resolved
Hide resolved
src/OpenRiaServices.Hosting.AspNetCore/Framework/AspNetCore/Operations/OperationInvoker.cs
Outdated
Show resolved
Hide resolved
...vices.Hosting.AspNetCore/Framework/AspNetCore/Serialization/DataContractRequestSerializer.cs
Outdated
Show resolved
Hide resolved
...vices.Hosting.AspNetCore/Framework/AspNetCore/Serialization/DataContractRequestSerializer.cs
Show resolved
Hide resolved
…otFound) * Improve matching on application/xml * Fix parsing of emtpy string from server
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (9)
src/OpenRiaServices.Client.DomainClients.Http/Framework/Http/DataContractHttpDomainClient.cs (5)
47-52:⚠️ Potential issue | 🟡 MinorAdd null check for
httpClientparameter.The constructor validates
serviceInterfacebut nothttpClient. A nullHttpClientwill cause aNullReferenceExceptionlater whenHttpClient.SendAsyncis called, making debugging harder.🛡️ Proposed fix
private protected DataContractHttpDomainClient(HttpClient httpClient, Type serviceInterface) { + if (httpClient is null) + throw new ArgumentNullException(nameof(httpClient)); if (serviceInterface is null) throw new ArgumentNullException(nameof(serviceInterface)); HttpClient = httpClient;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/OpenRiaServices.Client.DomainClients.Http/Framework/Http/DataContractHttpDomainClient.cs` around lines 47 - 52, The constructor DataContractHttpDomainClient(HttpClient httpClient, Type serviceInterface) checks serviceInterface but not httpClient; add a null check at the start that throws ArgumentNullException(nameof(httpClient)) if httpClient is null, and then assign the HttpClient property; this ensures HttpClient.SendAsync won't later throw a NullReferenceException.
345-351:⚠️ Potential issue | 🟡 MinorFix typos in comments.
Several typos in comments:
- Line 347: "respnse" → "response"
- Line 350: "pase on msbin" → "parse on msbin"
✏️ Proposed fix
- // Always dispose using finally block below respnse or we can leak connections + // Always dispose response using finally block below or we can leak connections // TODO: OpenRia 5.0 returns different status codes // Need to read content and parse it even if status code is not 200 - // It would make sens to one check content type and only pase on msbin + // It would make sense to check content type and only parse on msbin🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/OpenRiaServices.Client.DomainClients.Http/Framework/Http/DataContractHttpDomainClient.cs` around lines 345 - 351, In ReadResponseAsync, fix the typos in the inline comments: change "respnse" to "response" in the disposal comment and correct "pase on msbin" to "parse on msbin" (or "parse on msbin" -> "parse on msbin" if intended; ensure spelling reads "parse on msbin") so the comments around the using(response) block and the content parsing logic referencing ContentType are clear and correctly spelled; update only the comment text in the ReadResponseAsync method.
391-397:⚠️ Potential issue | 🟡 MinorFix typo:
XmlElemtnt→XmlElement.✏️ Proposed fix
- // XmlElemtnt returns the "ResultNode" unless we step into the contents + // XmlElement returns the "ResultNode" unless we step into the contents🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/OpenRiaServices.Client.DomainClients.Http/Framework/Http/DataContractHttpDomainClient.cs` around lines 391 - 397, The inline comment has a typo "XmlElemtnt" — update the comment above the special-case handling in DataContractHttpDomainClient (around GetSerializer, returnType, reader.ReadStartElement, and serializer.ReadObject) to read "XmlElement" instead of "XmlElemtnt" so the comment correctly references System.Xml.Linq.XElement semantics.
213-269: 🧹 Nitpick | 🔵 Trivial
HttpRequestMessageshould be disposed to prevent resource leaks.The
HttpRequestMessagecreated at line 215 is not disposed. WhileHttpClient.SendAsynctakes ownership of the content, the request message itself should be disposed to release any associated resources.♻️ Suggested fix
private Task<HttpResponseMessage> PostAsync(string operationName, IDictionary<string, object> parameters, List<ServiceQueryPart> queryOptions, CancellationToken cancellationToken) { - var request = new HttpRequestMessage(HttpMethod.Post, operationName); + using var request = new HttpRequestMessage(HttpMethod.Post, operationName); // ... build request content ... - return HttpClient.SendAsync(request, DefaultHttpCompletionOption, cancellationToken); + var task = HttpClient.SendAsync(request, DefaultHttpCompletionOption, cancellationToken); + return task; }Note: If
usingcauses issues becauseSendAsyncneeds the request alive, wrap in a helper that disposes after the task completes.#!/bin/bash # Verify HttpRequestMessage disposal patterns in the codebase rg -n "new HttpRequestMessage" --type cs -A 5 -B 2 | head -50🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/OpenRiaServices.Client.DomainClients.Http/Framework/Http/DataContractHttpDomainClient.cs` around lines 213 - 269, The HttpRequestMessage created in PostAsync is never disposed; change PostAsync to an async Task<HttpResponseMessage>, wrap the HttpRequestMessage in a using (or using var request = new HttpRequestMessage(...)) so the request is disposed after SendAsync completes, and await HttpClient.SendAsync(request, DefaultHttpCompletionOption, cancellationToken) to return the HttpResponseMessage; keep existing content creation (ByteArrayContent and ContentType) but ensure disposal of the request object around the SendAsync call (reference PostAsync, HttpRequestMessage, HttpClient.SendAsync, DefaultHttpCompletionOption, ContentType).
488-497:⚠️ Potential issue | 🟡 MinorFix typo:
isEmtpy→isEmpty.✏️ Proposed fix
- bool isEmtpy = reader.IsEmptyElement; + bool isEmpty = reader.IsEmptyElement; // ... - if (!isEmtpy) + if (!isEmpty) reader.ReadEndElement();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/OpenRiaServices.Client.DomainClients.Http/Framework/Http/DataContractHttpDomainClient.cs` around lines 488 - 497, There's a typo in DataContractHttpDomainClient's parsing loop: the boolean is declared as "isEmtpy" but should be "isEmpty"; rename the variable (and any uses) to isEmpty within the while loop that reads <Text> elements so the identifier is spelled correctly (e.g., change isEmtpy to isEmpty in the block that reads reader.IsEmptyElement and the subsequent if (!isEmtpy) reader.ReadEndElement()).src/OpenRiaServices.Hosting.AspNetCore/Framework/AspNetCore/Operations/OperationInvoker.cs (2)
63-67:⚠️ Potential issue | 🟠 MajorAdd guard for empty serialization providers array.
If
Options.SerializationProvidersis empty,GetSerializerForWritewill accessserializers[0]causing anIndexOutOfRangeException. Fail fast with a clear error message during initialization.🛡️ Proposed fix
var providers = Options.SerializationProviders; + if (providers.Length == 0) + throw new InvalidOperationException("No serialization providers are configured for OpenRiaServices."); result = new RequestSerializer[providers.Length];🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/OpenRiaServices.Hosting.AspNetCore/Framework/AspNetCore/Operations/OperationInvoker.cs` around lines 63 - 67, GetSerializerForWrite can throw IndexOutOfRange when Options.SerializationProviders is empty because serializers[0] is accessed; add a guard during initialization (where RequestSerializer[] result / providers = Options.SerializationProviders is built) to check providers.Length == 0 and throw a clear, fast-fail ArgumentException/InvalidOperationException with a descriptive message; ensure the check is adjacent to the construction of the RequestSerializer[] and referenced symbols RequestSerializer, Options.SerializationProviders, GetSerializerForWrite, and serializers[0] are updated so initialization never proceeds with an empty providers array.
108-147: 🧹 Nitpick | 🔵 TrivialContent negotiation always returns a serializer, making 406 responses unreachable.
DoContentNegotiationfalls back toserializers[0]when noAcceptheader matches (line 146). If strict content negotiation is desired, consider returningnullwhen the client explicitly requests an unsupported format, allowing callers to return HTTP 406.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/OpenRiaServices.Hosting.AspNetCore/Framework/AspNetCore/Operations/OperationInvoker.cs` around lines 108 - 147, DoContentNegotiation currently always returns a serializer (falling back to serializers[0]) which prevents callers from returning 406; change its behavior so that when the request includes an explicit Accept header (or multiple Accepts) and none of the RequestSerializer.CanWrite checks match the requested media types, the method returns null instead of serializers[0], while preserving the existing fallback-to-first-only when there is no Accept header (e.g., rely on Content-Type or no Accept present); update references to DoContentNegotiation and consumers to handle a null return (allowing them to produce a 406).src/OpenRiaServices.Hosting.AspNetCore/Framework/README.md (1)
123-133:⚠️ Potential issue | 🟡 MinorFix grammar and clarify client requirements for XML-only configuration.
Two minor grammar issues and one usability concern:
- "built in" → "built-in" (line 123)
- "text based" → "text-based" (line 125)
- When using XML-only, clients must use
XmlHttpDomainClientFactory—otherwise they'll hit protocol mismatch errors.📘 Suggested fix
-You can remove default built in formats by calling `ClearSerializationProviders`. +You can remove default built-in formats by calling `ClearSerializationProviders`. -The following sample will only accept and return plain text based xml -using MIME-type `application/xml` +The following sample will only accept and return plain text-based XML +using MIME-type `application/xml`. Clients must use `XmlHttpDomainClientFactory`.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/OpenRiaServices.Hosting.AspNetCore/Framework/README.md` around lines 123 - 133, Fix grammar and clarify client requirement in the README: change "built in" to "built-in" and "text based" to "text-based" in the example paragraph, and add a sentence stating that when configuring XML-only serialization via ClearSerializationProviders() and AddXmlSerialization(), clients must use XmlHttpDomainClientFactory to avoid protocol mismatch errors; reference the methods ClearSerializationProviders, AddXmlSerialization and the XmlHttpDomainClientFactory class in the explanatory sentence so readers know exactly what to use.src/OpenRiaServices.Hosting.AspNetCore/Framework/AspNetCore/Serialization/DataContractRequestSerializer.cs (1)
34-38: 🧹 Nitpick | 🔵 TrivialBinary XML serializer should handle Content-Type parameters consistently with text XML.
BinaryXmlDataContractRequestSerializer.CanRead/CanWriteuses exact equality, whileTextXmlDataContractRequestSerializerusesMatchesMediaTypewhich strips parameters. If a client sendsapplication/msbin1; charset=utf-8, binary matching will fail unexpectedly.Consider using
MatchesMediaTypehere as well for consistency:♻️ Suggested fix
public override bool CanRead(ReadOnlySpan<char> contentType) - => MemoryExtensions.Equals(contentType, MimeTypes.BinaryXml, StringComparison.Ordinal); + => MatchesMediaType(contentType, MimeTypes.BinaryXml); public override bool CanWrite(ReadOnlySpan<char> contentType) - => MemoryExtensions.Equals(contentType, MimeTypes.BinaryXml, StringComparison.Ordinal); + => MatchesMediaType(contentType, MimeTypes.BinaryXml);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/OpenRiaServices.Hosting.AspNetCore/Framework/AspNetCore/Serialization/DataContractRequestSerializer.cs` around lines 34 - 38, The Binary XML serializer's CanRead/CanWrite (in BinaryXmlDataContractRequestSerializer) uses exact MemoryExtensions.Equals against MimeTypes.BinaryXml and thus fails when Content-Type has parameters; update both methods to use the same MatchesMediaType helper used by TextXmlDataContractRequestSerializer (i.e., call MatchesMediaType(contentType, MimeTypes.BinaryXml) or equivalent) so parameters like "; charset=utf-8" are ignored while still matching MimeTypes.BinaryXml.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Changelog.md`:
- Around line 36-44: In Changelog.md update the minor formatting: change "text
based" to "text-based" in the sentence mentioning
OpenRiaServices.Client.DomainClients.XmlHttpDomainClientFactory, remove the
trailing space at the end of the sample usage line, and insert a blank line
immediately before the opening code fence for the sample snippet that shows
DomainContext.DomainClientFactory = new XmlHttpDomainClientFactory(baseUri,
httpClientFactory); to ensure proper formatting.
In
`@src/OpenRiaServices.Hosting.AspNetCore/Framework/AspNetCore/Serialization/DataContractRequestSerializer.cs`:
- Around line 413-420: The MatchesMediaType method is using a case-sensitive
comparison (StringComparison.Ordinal) which contradicts its docs and RFC 7231;
update MatchesMediaType to perform a case-insensitive comparison (e.g., use
StringComparison.OrdinalIgnoreCase) when comparing the media-type tokens (after
removing parameters) and also trim any surrounding whitespace from the value
slice before comparison to ensure values like " Application/Xml ; charset=UTF-8"
match the expected token.
---
Duplicate comments:
In
`@src/OpenRiaServices.Client.DomainClients.Http/Framework/Http/DataContractHttpDomainClient.cs`:
- Around line 47-52: The constructor DataContractHttpDomainClient(HttpClient
httpClient, Type serviceInterface) checks serviceInterface but not httpClient;
add a null check at the start that throws
ArgumentNullException(nameof(httpClient)) if httpClient is null, and then assign
the HttpClient property; this ensures HttpClient.SendAsync won't later throw a
NullReferenceException.
- Around line 345-351: In ReadResponseAsync, fix the typos in the inline
comments: change "respnse" to "response" in the disposal comment and correct
"pase on msbin" to "parse on msbin" (or "parse on msbin" -> "parse on msbin" if
intended; ensure spelling reads "parse on msbin") so the comments around the
using(response) block and the content parsing logic referencing ContentType are
clear and correctly spelled; update only the comment text in the
ReadResponseAsync method.
- Around line 391-397: The inline comment has a typo "XmlElemtnt" — update the
comment above the special-case handling in DataContractHttpDomainClient (around
GetSerializer, returnType, reader.ReadStartElement, and serializer.ReadObject)
to read "XmlElement" instead of "XmlElemtnt" so the comment correctly references
System.Xml.Linq.XElement semantics.
- Around line 213-269: The HttpRequestMessage created in PostAsync is never
disposed; change PostAsync to an async Task<HttpResponseMessage>, wrap the
HttpRequestMessage in a using (or using var request = new
HttpRequestMessage(...)) so the request is disposed after SendAsync completes,
and await HttpClient.SendAsync(request, DefaultHttpCompletionOption,
cancellationToken) to return the HttpResponseMessage; keep existing content
creation (ByteArrayContent and ContentType) but ensure disposal of the request
object around the SendAsync call (reference PostAsync, HttpRequestMessage,
HttpClient.SendAsync, DefaultHttpCompletionOption, ContentType).
- Around line 488-497: There's a typo in DataContractHttpDomainClient's parsing
loop: the boolean is declared as "isEmtpy" but should be "isEmpty"; rename the
variable (and any uses) to isEmpty within the while loop that reads <Text>
elements so the identifier is spelled correctly (e.g., change isEmtpy to isEmpty
in the block that reads reader.IsEmptyElement and the subsequent if (!isEmtpy)
reader.ReadEndElement()).
In
`@src/OpenRiaServices.Hosting.AspNetCore/Framework/AspNetCore/Operations/OperationInvoker.cs`:
- Around line 63-67: GetSerializerForWrite can throw IndexOutOfRange when
Options.SerializationProviders is empty because serializers[0] is accessed; add
a guard during initialization (where RequestSerializer[] result / providers =
Options.SerializationProviders is built) to check providers.Length == 0 and
throw a clear, fast-fail ArgumentException/InvalidOperationException with a
descriptive message; ensure the check is adjacent to the construction of the
RequestSerializer[] and referenced symbols RequestSerializer,
Options.SerializationProviders, GetSerializerForWrite, and serializers[0] are
updated so initialization never proceeds with an empty providers array.
- Around line 108-147: DoContentNegotiation currently always returns a
serializer (falling back to serializers[0]) which prevents callers from
returning 406; change its behavior so that when the request includes an explicit
Accept header (or multiple Accepts) and none of the RequestSerializer.CanWrite
checks match the requested media types, the method returns null instead of
serializers[0], while preserving the existing fallback-to-first-only when there
is no Accept header (e.g., rely on Content-Type or no Accept present); update
references to DoContentNegotiation and consumers to handle a null return
(allowing them to produce a 406).
In
`@src/OpenRiaServices.Hosting.AspNetCore/Framework/AspNetCore/Serialization/DataContractRequestSerializer.cs`:
- Around line 34-38: The Binary XML serializer's CanRead/CanWrite (in
BinaryXmlDataContractRequestSerializer) uses exact MemoryExtensions.Equals
against MimeTypes.BinaryXml and thus fails when Content-Type has parameters;
update both methods to use the same MatchesMediaType helper used by
TextXmlDataContractRequestSerializer (i.e., call MatchesMediaType(contentType,
MimeTypes.BinaryXml) or equivalent) so parameters like "; charset=utf-8" are
ignored while still matching MimeTypes.BinaryXml.
In `@src/OpenRiaServices.Hosting.AspNetCore/Framework/README.md`:
- Around line 123-133: Fix grammar and clarify client requirement in the README:
change "built in" to "built-in" and "text based" to "text-based" in the example
paragraph, and add a sentence stating that when configuring XML-only
serialization via ClearSerializationProviders() and AddXmlSerialization(),
clients must use XmlHttpDomainClientFactory to avoid protocol mismatch errors;
reference the methods ClearSerializationProviders, AddXmlSerialization and the
XmlHttpDomainClientFactory class in the explanatory sentence so readers know
exactly what to use.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: eabaa55e-7777-49d4-a1a7-be543de7262a
📒 Files selected for processing (6)
Changelog.mdsrc/OpenRiaServices.Client.DomainClients.Http/Framework/Http/DataContractHttpDomainClient.cssrc/OpenRiaServices.Hosting.AspNetCore/Framework/AspNetCore/OpenRiaServicesServiceCollectionExtensions.cssrc/OpenRiaServices.Hosting.AspNetCore/Framework/AspNetCore/Operations/OperationInvoker.cssrc/OpenRiaServices.Hosting.AspNetCore/Framework/AspNetCore/Serialization/DataContractRequestSerializer.cssrc/OpenRiaServices.Hosting.AspNetCore/Framework/README.md
...vices.Hosting.AspNetCore/Framework/AspNetCore/Serialization/DataContractRequestSerializer.cs
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (2)
Changelog.md (1)
36-42:⚠️ Potential issue | 🟡 MinorXML support subsection still has prior formatting issues.
Line 37 should use “text-based”, and Lines 40–42 need blank lines around the fenced block in this list section.
✏️ Suggested fix
* XML Support -Added a new class `OpenRiaServices.Client.DomainClients.XmlHttpDomainClientFactory` that can be used to enable text based XML communication with the server. +Added a new class `OpenRiaServices.Client.DomainClients.XmlHttpDomainClientFactory` that can be used to enable text-based XML communication with the server. Sample usage: + ```csharp DomainContext.DomainClientFactory = new XmlHttpDomainClientFactory(baseUri, httpClientFactory);
</details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against the current code and only fix it if needed.
In
@Changelog.mdaround lines 36 - 42, Change the phrase "text based" to
"text-based" in the XML Support section and ensure there is a blank line before
and after the fenced C# code block that shows usage of
DomainContext.DomainClientFactory = new XmlHttpDomainClientFactory(baseUri,
httpClientFactory); so the list renders correctly; locate the subsection
mentioning OpenRiaServices.Client.DomainClients.XmlHttpDomainClientFactory and
update the wording plus insert one blank line above thecsharp fence and one blank line after the closingfence.</details> </blockquote></details> <details> <summary>src/OpenRiaServices.Hosting.AspNetCore/Framework/AspNetCore/Serialization/DataContractRequestSerializer.cs (1)</summary><blockquote> `34-38`: _⚠️ Potential issue_ | _🟠 Major_ **Normalize the binary media type the same way as the text path.** Lines 35-38 still compare the raw header with `Ordinal`, so valid case/parameter variants can miss the binary serializer. `MatchesMediaType` also needs to trim whitespace around the token, otherwise values like `application/xml ; charset=utf-8` are still rejected. <details> <summary>🛠️ Suggested patch</summary> ```diff public override bool CanRead(ReadOnlySpan<char> contentType) - => MemoryExtensions.Equals(contentType, MimeTypes.BinaryXml, StringComparison.Ordinal); + => MatchesMediaType(contentType, MimeTypes.BinaryXml); public override bool CanWrite(ReadOnlySpan<char> contentType) - => MemoryExtensions.Equals(contentType, MimeTypes.BinaryXml, StringComparison.Ordinal); + => MatchesMediaType(contentType, MimeTypes.BinaryXml); ... protected static bool MatchesMediaType(ReadOnlySpan<char> value, ReadOnlySpan<char> expected) { int separator = value.IndexOf(';'); if (separator >= 0) value = value[..separator]; - return value.Equals(expected, StringComparison.OrdinalIgnoreCase); + return value.Trim().Equals(expected, StringComparison.OrdinalIgnoreCase); } ``` </details> Also applies to: 413-419 <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In `@src/OpenRiaServices.Hosting.AspNetCore/Framework/AspNetCore/Serialization/DataContractRequestSerializer.cs` around lines 34 - 38, The CanRead and CanWrite overrides in DataContractRequestSerializer compare the raw contentType with Ordinal, so case/parameter variants and surrounding whitespace are rejected; change both methods to use the same normalized media-type check as the text path (call MatchesMediaType or equivalent normalization that trims whitespace and performs case-insensitive comparison and ignores parameters) so values like "application/xml; charset=utf-8" or differing case are accepted; apply the same fix to the corresponding binary checks in the other serializer methods referenced (the duplicate block around the later serializer methods). ``` </details> </blockquote></details> </blockquote></details> <details> <summary>🤖 Prompt for all review comments with AI agents</summary>Verify each finding against the current code and only fix it if needed.
Inline comments:
In@Changelog.md:
- Around line 21-22: The markdown heading "## EF Core 4.0" in Changelog.md needs
a separating blank line after it; update the file by inserting a single empty
line between the "## EF Core 4.0" heading and the following list item "* Add
.NET 10 target framework compiled against EF Core 10 (#549)" so the heading is
followed by a blank line for proper Markdown heading spacing.- Around line 31-33: The sentence is incomplete: replace the dangling phrase so
it explicitly instructs to use the new attribute; update the Changelog sentence
to read thatEntityAssociationAttributeshould be used instead of the obsolete
AssociationAttributeand finish the sentence with "please update all code to
useEntityAssociationAttribute" (also update any examples/docs referencing
AssociationAttribute).- Around line 9-10: Edit the Changelog.md entry that reads "Server can now
intelligently handles both binary and XML content types based on client requests
and configuration settings": correct the subject-verb mismatch by changing
"handles" to "handle" and remove any trailing whitespace at the end of the line
so the sentence reads "Server can now intelligently handle both binary and XML
content types based on client requests and configuration settings".In
@src/OpenRiaServices.Hosting.AspNetCore/Framework/AspNetCore/Serialization/DataContractRequestSerializer.cs:
- Around line 147-149: After calling reader.ReadEndElement() in
DataContractRequestSerializer (the XML deserialize path), skip any trailing XML
whitespace by invoking reader.MoveToContent() before validating EOF; then
perform the ReadState check (reader.ReadState != ReadState.EndOfFile) and throw
BadHttpRequestException(InvalidContentMessage) only if not at EndOfFile. This
ensures trailing whitespace/newlines are ignored while preserving the existing
error behavior.
Duplicate comments:
In@Changelog.md:
- Around line 36-42: Change the phrase "text based" to "text-based" in the XML
Support section and ensure there is a blank line before and after the fenced C#
code block that shows usage of DomainContext.DomainClientFactory = new
XmlHttpDomainClientFactory(baseUri, httpClientFactory); so the list renders
correctly; locate the subsection mentioning
OpenRiaServices.Client.DomainClients.XmlHttpDomainClientFactory and update the
wording plus insert one blank line above thecsharp fence and one blank line after the closingfence.In
@src/OpenRiaServices.Hosting.AspNetCore/Framework/AspNetCore/Serialization/DataContractRequestSerializer.cs:
- Around line 34-38: The CanRead and CanWrite overrides in
DataContractRequestSerializer compare the raw contentType with Ordinal, so
case/parameter variants and surrounding whitespace are rejected; change both
methods to use the same normalized media-type check as the text path (call
MatchesMediaType or equivalent normalization that trims whitespace and performs
case-insensitive comparison and ignores parameters) so values like
"application/xml; charset=utf-8" or differing case are accepted; apply the same
fix to the corresponding binary checks in the other serializer methods
referenced (the duplicate block around the later serializer methods).</details> <details> <summary>🪄 Autofix (Beta)</summary> Fix all unresolved CodeRabbit comments on this PR: - [ ] <!-- {"checkboxId": "4b0d0e0a-96d7-4f10-b296-3a18ea78f0b9"} --> Push a commit to this branch (recommended) - [ ] <!-- {"checkboxId": "ff5b1114-7d8c-49e6-8ac1-43f82af23a33"} --> Create a new PR with the fixes </details> --- <details> <summary>ℹ️ Review info</summary> <details> <summary>⚙️ Run configuration</summary> **Configuration used**: Organization UI **Review profile**: ASSERTIVE **Plan**: Pro **Run ID**: `2f40e7b6-c925-4a0e-8d77-1e38a56e6bc8` </details> <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between c46b5ccc5bc6882b304221f1cccabcfabff5e758 and 7c99342ff3866f4a0d5df9e88de36d70703398b3. </details> <details> <summary>📒 Files selected for processing (2)</summary> * `Changelog.md` * `src/OpenRiaServices.Hosting.AspNetCore/Framework/AspNetCore/Serialization/DataContractRequestSerializer.cs` </details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
...vices.Hosting.AspNetCore/Framework/AspNetCore/Serialization/DataContractRequestSerializer.cs
Show resolved
Hide resolved
...vices.Hosting.AspNetCore/Framework/AspNetCore/Serialization/DataContractRequestSerializer.cs
Outdated
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/OpenRiaServices.Client/Framework/DomainContext.cs (1)
884-905:⚠️ Potential issue | 🟠 MajorGuard
returnTypevsTValueto avoid runtime cast failuresLine 884 now drives deserialization from
returnType, but Line 905 still casts toTValue. If they are incompatible, this throws at runtime (InvalidCastException). Add an explicit compatibility check before creatingInvokeArgs.Proposed fix
if (returnType is null) { throw new ArgumentNullException(nameof(returnType)); } + + if (returnType != typeof(void) && !typeof(TValue).IsAssignableFrom(returnType)) + { + throw new ArgumentException( + string.Format( + CultureInfo.CurrentCulture, + "The specified returnType '{0}' is not assignable to TValue '{1}'.", + returnType, + typeof(TValue)), + nameof(returnType)); + } InvokeArgs invokeArgs = new InvokeArgs(operationName, returnType, parameters, hasSideEffects);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/OpenRiaServices.Client/Framework/DomainContext.cs` around lines 884 - 905, The code builds InvokeArgs using returnType but later casts results.ReturnValue to TValue, which can cause InvalidCastException; before creating InvokeArgs (or right after computing returnType) add an explicit compatibility check such as if (!typeof(TValue).IsAssignableFrom(returnType) && !(returnType.IsAssignableFrom(typeof(TValue)))) then throw a DomainOperationException with a clear message (use Resource.DomainContext_InvokeOperationFailed or similar) so the method fails fast; reference the DomainContext method that calls new InvokeArgs(...), the InvokeAsync(...) continuation and the creation of new InvokeResult<TValue> to locate and apply the check.
♻️ Duplicate comments (12)
Changelog.md (2)
9-10:⚠️ Potential issue | 🟡 MinorFix remaining markdown/grammar lint issues in release notes.
Line 9 has a grammar error (“can now intelligently handles”), Line 10 has trailing whitespace, Line 21 needs a blank line after the heading, and Line 37 needs a blank line before the fenced block.
✏️ Suggested patch
-* Server can now intelligently handles both binary and XML content types based on client requests and configuration settings - +* Server can now intelligently handle both binary and XML content types based on client requests and configuration settings + ## EF Core 4.0 + * Add .NET 10 target framework compiled against EF Core 10 (`#549`) @@ Added a new class `OpenRiaServices.Client.DomainClients.XmlHttpDomainClientFactory` that can be used to enable text-based XML communication with the server. Sample usage: + ```csharp DomainContext.DomainClientFactory = new XmlHttpDomainClientFactory(baseUri, httpClientFactory); ```Also applies to: 21-22, 36-39
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Changelog.md` around lines 9 - 10, Fix the grammar and markdown spacing issues in Changelog.md: change “can now intelligently handles” to “can now intelligently handle”, remove trailing whitespace on the sentence at the end of the original Line 10, add a blank line after the heading around Line 21, and add a blank line before the fenced code block around Line 37; also ensure the fenced block is properly delimited (triple backticks) and that the code inside (e.g., DomainContext.DomainClientFactory = new XmlHttpDomainClientFactory(baseUri, httpClientFactory);) is on its own line inside the fence.
42-44:⚠️ Potential issue | 🟡 MinorComplete the migration guidance sentence.
Line 43 is still incomplete and should explicitly name the replacement attribute so upgrade guidance is actionable.
✏️ Suggested patch
* `EntityAssociationAttribute` should now be used instead of `AssociationAttribute` (which is obsolete on .NET) - * While there is code for backwards compatibility please update all code to use + * While there is code for backwards compatibility, please update all code to use `EntityAssociationAttribute`.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Changelog.md` around lines 42 - 44, Finish the incomplete migration guidance by explicitly naming the replacement attribute: replace the trailing fragment with a full sentence such as "While there is code for backwards compatibility, please update all code to use EntityAssociationAttribute instead of the obsolete AssociationAttribute." Ensure both symbols AssociationAttribute and EntityAssociationAttribute are mentioned so readers know which attribute to replace.src/OpenRiaServices.Client.DomainClients.Http/Framework/Http/DataContractHttpDomainClient.cs (7)
345-348:⚠️ Potential issue | 🟡 MinorFix typo in comment.
Line 347: "respnse" should be "response" and there's an extra space.
📝 Proposed fix
- // Always dispose using finally block below respnse or we can leak connections + // Always dispose response using finally block below or we can leak connections🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/OpenRiaServices.Client.DomainClients.Http/Framework/Http/DataContractHttpDomainClient.cs` around lines 345 - 348, Fix the typo and spacing in the comment inside ReadResponseAsync: change "respnse" to "response" and remove the extra space so the comment reads something like "// Always dispose using finally block below response or we can leak connections" (or improve grammar while keeping meaning) in the DataContractHttpDomainClient.ReadResponseAsync method.
417-419: 🧹 Nitpick | 🔵 TrivialAdd
CultureInfo.InvariantCulturefor consistency.The
string.Formatcall should useCultureInfo.InvariantCulturefor consistency with line 353. While the format arguments are strings, this silences static analysis warnings.♻️ Proposed fix
throw new DomainOperationException( - string.Format(Resources.DomainClient_UnexpectedResultContent, operationName + postfix, reader.LocalName) + string.Format(CultureInfo.InvariantCulture, Resources.DomainClient_UnexpectedResultContent, operationName + postfix, reader.LocalName) , OperationErrorStatus.ServerError, 0, null);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/OpenRiaServices.Client.DomainClients.Http/Framework/Http/DataContractHttpDomainClient.cs` around lines 417 - 419, The string.Format call that throws DomainOperationException in DataContractHttpDomainClient.cs should use CultureInfo.InvariantCulture for consistency and to silence static analysis warnings; update the string.Format invocation in the throw that constructs the DomainOperationException (the one passing Resources.DomainClient_UnexpectedResultContent, operationName + postfix, reader.LocalName) to call string.Format(CultureInfo.InvariantCulture, ...) and ensure you add the necessary using or fully-qualified System.Globalization.CultureInfo where required.
489-496:⚠️ Potential issue | 🟡 MinorFix typo: "isEmtpy" should be "isEmpty".
📝 Proposed fix
- bool isEmtpy = reader.IsEmptyElement; + bool isEmpty = reader.IsEmptyElement; var lang = reader.XmlLang; reader.ReadStartElement("Text"); var text = reader.ReadContentAsString(); - if (!isEmtpy) + if (!isEmpty) reader.ReadEndElement();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/OpenRiaServices.Client.DomainClients.Http/Framework/Http/DataContractHttpDomainClient.cs` around lines 489 - 496, Rename the misspelled local variable isEmtpy to isEmpty in DataContractHttpDomainClient.cs (the block that reads the "Text" element using reader.XmlLang, reader.ReadStartElement("Text"), reader.ReadContentAsString(), and reader.ReadEndElement()); update all references to that variable in the same method so the conditional if (!isEmpty) reader.ReadEndElement() compiles and the name is consistent.
392-394:⚠️ Potential issue | 🟡 MinorFix typo: "XmlElemtnt" should be "XmlElement".
📝 Proposed fix
- // XmlElemtnt returns the "ResultNode" unless we step into the contents + // XmlElement returns the "ResultNode" unless we step into the contents🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/OpenRiaServices.Client.DomainClients.Http/Framework/Http/DataContractHttpDomainClient.cs` around lines 392 - 394, Fix the typo in the comment near the returnType check inside DataContractHttpDomainClient (the block that checks if returnType == typeof(System.Xml.Linq.XElement) and calls reader.ReadStartElement()); change the comment text from "XmlElemtnt" to "XmlElement" so it reads correctly (e.g., "XmlElement returns the 'ResultNode' unless we step into the contents").
291-302: 🧹 Nitpick | 🔵 TrivialConsider using
.Where()to filter parameters.The foreach loop implicitly filters out null values. Using
.Where()makes the intent clearer.♻️ Proposed refactor
- foreach (var param in parameters) + foreach (var param in parameters.Where(p => p.Value != null)) { - if (param.Value != null) - { uriBuilder.Append(i++ == 0 ? '?' : '&'); uriBuilder.Append(Uri.EscapeDataString(param.Key)); uriBuilder.Append('='); var parameterType = methodParameters.GetTypeForMethodParameter(param.Key); var value = WebQueryStringConverter.ConvertValueToString(param.Value, parameterType); uriBuilder.Append(Uri.EscapeDataString(value)); - } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/OpenRiaServices.Client.DomainClients.Http/Framework/Http/DataContractHttpDomainClient.cs` around lines 291 - 302, The loop over parameters should explicitly filter out null values to make intent clear: replace the foreach over parameters with an iteration over parameters.Where(p => p.Value != null) (or equivalent) before appending to uriBuilder; keep using methodParameters.GetTypeForMethodParameter(param.Key) and WebQueryStringConverter.ConvertValueToString(param.Value, parameterType) to obtain the string value and escape it before appending. This clarifies the null-filtering intent while preserving the existing logic that appends '?' or '&', escapes the key/value, and increments i.
47-52:⚠️ Potential issue | 🟡 MinorAdd null-argument guard for
httpClientparameter.The constructor validates
serviceInterfacebut nothttpClient. A nullhttpClientwill cause aNullReferenceExceptionlater during request execution rather than failing fast with a clear error at construction time.🛡️ Proposed fix
private protected DataContractHttpDomainClient(HttpClient httpClient, Type serviceInterface) { + if (httpClient is null) + throw new ArgumentNullException(nameof(httpClient)); if (serviceInterface is null) throw new ArgumentNullException(nameof(serviceInterface)); HttpClient = httpClient;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/OpenRiaServices.Client.DomainClients.Http/Framework/Http/DataContractHttpDomainClient.cs` around lines 47 - 52, The constructor DataContractHttpDomainClient(HttpClient httpClient, Type serviceInterface) currently validates serviceInterface but not httpClient; add a null-argument guard at the start of that constructor to throw ArgumentNullException(nameof(httpClient)) when httpClient is null so the HttpClient property assignment (HttpClient = httpClient) fails fast with a clear error instead of causing downstream NullReferenceException.
213-270:⚠️ Potential issue | 🟡 Minor
HttpRequestMessageis not disposed and may leak resources.The
HttpRequestMessagecreated at line 215 implementsIDisposablebut is never disposed. WhileHttpClient.SendAsyncconsumes the request, it does not dispose it. Consider wrapping the request lifetime or disposing after the response is received.♻️ Proposed fix using a wrapper approach
private Task<HttpResponseMessage> PostAsync(string operationName, IDictionary<string, object> parameters, List<ServiceQueryPart> queryOptions, CancellationToken cancellationToken) { - var request = new HttpRequestMessage(HttpMethod.Post, operationName); + return PostAsyncCore(); - using (var ms = new MemoryStream()) - using (var writer = CreateWriter(ms)) + async Task<HttpResponseMessage> PostAsyncCore() { - // ... existing code ... + using var request = new HttpRequestMessage(HttpMethod.Post, operationName); + + using (var ms = new MemoryStream()) + using (var writer = CreateWriter(ms)) + { + // ... existing serialization code ... + } + + return await HttpClient.SendAsync(request, DefaultHttpCompletionOption, cancellationToken).ConfigureAwait(false); } - - return HttpClient.SendAsync(request, DefaultHttpCompletionOption, cancellationToken); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/OpenRiaServices.Client.DomainClients.Http/Framework/Http/DataContractHttpDomainClient.cs` around lines 213 - 270, The HttpRequestMessage created in PostAsync is not disposed; change PostAsync to be async (async Task<HttpResponseMessage> PostAsync(...)) and await HttpClient.SendAsync(..., DefaultHttpCompletionOption, cancellationToken), then dispose the HttpRequestMessage (request.Dispose()) after the SendAsync completes but before returning the HttpResponseMessage so the request's resources are cleaned up; update references inside PostAsync (HttpRequestMessage, HttpClient.SendAsync, DefaultHttpCompletionOption) accordingly and ensure only the response is returned to the caller.src/OpenRiaServices.Hosting.AspNetCore/Framework/AspNetCore/Serialization/DataContractRequestSerializer.cs (3)
144-149:⚠️ Potential issue | 🟠 MajorCall
MoveToContent()before the closing reads.
IsStartElement(...)already skips insignificant whitespace during parsing, but the closingReadEndElement()/ EOF checks do not. A valid pretty-printed XML body with indentation before</SubmitChanges>,</MessageRoot>, or EOF will still fail on this path.🧹 Proposed fix
- if (hasMessageRoot) - reader.ReadEndElement(); + if (hasMessageRoot) + { + reader.MoveToContent(); + reader.ReadEndElement(); + } - // Verify at end + reader.MoveToContent(); + // Verify at end if (reader.ReadState != ReadState.EndOfFile) throw new BadHttpRequestException(InvalidContentMessage); ... - reader.ReadEndElement(); // operation.Name + reader.MoveToContent(); + reader.ReadEndElement(); // operation.Name return values; ... - reader.ReadEndElement(); + reader.MoveToContent(); + reader.ReadEndElement(); return new object?[] { changeSet };Also applies to: 257-281
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/OpenRiaServices.Hosting.AspNetCore/Framework/AspNetCore/Serialization/DataContractRequestSerializer.cs` around lines 144 - 149, The reader may be positioned on insignificant whitespace before the closing elements or EOF, so call reader.MoveToContent() before performing reader.ReadEndElement() and before checking reader.ReadState in the DataContractRequestSerializer code paths that currently do: if (hasMessageRoot) reader.ReadEndElement(); and then if (reader.ReadState != ReadState.EndOfFile) throw..., and likewise in the other block around lines 257-281; update both spots to invoke reader.MoveToContent() immediately before the closing ReadEndElement() and again immediately before the EOF/readstate check to skip whitespace and ensure correct parsing.
34-38:⚠️ Potential issue | 🟠 MajorNormalize binary media-type matching with the shared helper.
OperationInvokerpasses raw header values into these methods. The exact ordinal comparison here, plus the missing whitespace trim inMatchesMediaType, still rejects valid binary content types once casing, optional whitespace, or parameters are present, which can make negotiation fall back to the wrong serializer.🔧 Proposed fix
public override bool CanRead(ReadOnlySpan<char> contentType) - => MemoryExtensions.Equals(contentType, MimeTypes.BinaryXml, StringComparison.Ordinal); + => MatchesMediaType(contentType, MimeTypes.BinaryXml); public override bool CanWrite(ReadOnlySpan<char> contentType) - => MemoryExtensions.Equals(contentType, MimeTypes.BinaryXml, StringComparison.Ordinal); + => MatchesMediaType(contentType, MimeTypes.BinaryXml); ... protected static bool MatchesMediaType(ReadOnlySpan<char> value, ReadOnlySpan<char> expected) { int separator = value.IndexOf(';'); if (separator >= 0) value = value[..separator]; + while (!value.IsEmpty && char.IsWhiteSpace(value[0])) + value = value[1..]; + while (!value.IsEmpty && char.IsWhiteSpace(value[^1])) + value = value[..^1]; + return value.Equals(expected, StringComparison.OrdinalIgnoreCase); }Also applies to: 411-417
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/OpenRiaServices.Hosting.AspNetCore/Framework/AspNetCore/Serialization/DataContractRequestSerializer.cs` around lines 34 - 38, CanRead and CanWrite currently use MemoryExtensions.Equals which fails for raw header values with casing, whitespace or parameters; replace those comparisons to call the shared media-type helper MatchesMediaType (e.g. MatchesMediaType(contentType, MimeTypes.BinaryXml)) so it trims, ignores case, and tolerates parameters; apply the same change to the other matching methods (the duplicate at the later block that handles the same media-type) to ensure consistent binary media-type negotiation when OperationInvoker passes raw header values.
233-236:⚠️ Potential issue | 🟠 MajorReject
xsi:nilfor non-nullable value parameters.This branch assigns
nullunconditionally, so requests like<count xsi:nil="true" />bypass request validation here and fail later during invocation instead of returning a clean 400.🛡️ Proposed fix
if (reader.HasAttributes && reader.GetAttribute("nil", "http://www.w3.org/2001/XMLSchema-instance") == "true") { + if (parameter.ParameterType.IsValueType && + Nullable.GetUnderlyingType(parameter.ParameterType) is null) + { + throw new BadHttpRequestException(InvalidContentMessage); + } + values[i] = null; ReadElement(reader); // consume element }
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@src/OpenRiaServices.Client/Framework/DomainContext.cs`:
- Around line 884-905: The code builds InvokeArgs using returnType but later
casts results.ReturnValue to TValue, which can cause InvalidCastException;
before creating InvokeArgs (or right after computing returnType) add an explicit
compatibility check such as if (!typeof(TValue).IsAssignableFrom(returnType) &&
!(returnType.IsAssignableFrom(typeof(TValue)))) then throw a
DomainOperationException with a clear message (use
Resource.DomainContext_InvokeOperationFailed or similar) so the method fails
fast; reference the DomainContext method that calls new InvokeArgs(...), the
InvokeAsync(...) continuation and the creation of new InvokeResult<TValue> to
locate and apply the check.
---
Duplicate comments:
In `@Changelog.md`:
- Around line 9-10: Fix the grammar and markdown spacing issues in Changelog.md:
change “can now intelligently handles” to “can now intelligently handle”, remove
trailing whitespace on the sentence at the end of the original Line 10, add a
blank line after the heading around Line 21, and add a blank line before the
fenced code block around Line 37; also ensure the fenced block is properly
delimited (triple backticks) and that the code inside (e.g.,
DomainContext.DomainClientFactory = new XmlHttpDomainClientFactory(baseUri,
httpClientFactory);) is on its own line inside the fence.
- Around line 42-44: Finish the incomplete migration guidance by explicitly
naming the replacement attribute: replace the trailing fragment with a full
sentence such as "While there is code for backwards compatibility, please update
all code to use EntityAssociationAttribute instead of the obsolete
AssociationAttribute." Ensure both symbols AssociationAttribute and
EntityAssociationAttribute are mentioned so readers know which attribute to
replace.
In
`@src/OpenRiaServices.Client.DomainClients.Http/Framework/Http/DataContractHttpDomainClient.cs`:
- Around line 345-348: Fix the typo and spacing in the comment inside
ReadResponseAsync: change "respnse" to "response" and remove the extra space so
the comment reads something like "// Always dispose using finally block below
response or we can leak connections" (or improve grammar while keeping meaning)
in the DataContractHttpDomainClient.ReadResponseAsync method.
- Around line 417-419: The string.Format call that throws
DomainOperationException in DataContractHttpDomainClient.cs should use
CultureInfo.InvariantCulture for consistency and to silence static analysis
warnings; update the string.Format invocation in the throw that constructs the
DomainOperationException (the one passing
Resources.DomainClient_UnexpectedResultContent, operationName + postfix,
reader.LocalName) to call string.Format(CultureInfo.InvariantCulture, ...) and
ensure you add the necessary using or fully-qualified
System.Globalization.CultureInfo where required.
- Around line 489-496: Rename the misspelled local variable isEmtpy to isEmpty
in DataContractHttpDomainClient.cs (the block that reads the "Text" element
using reader.XmlLang, reader.ReadStartElement("Text"),
reader.ReadContentAsString(), and reader.ReadEndElement()); update all
references to that variable in the same method so the conditional if (!isEmpty)
reader.ReadEndElement() compiles and the name is consistent.
- Around line 392-394: Fix the typo in the comment near the returnType check
inside DataContractHttpDomainClient (the block that checks if returnType ==
typeof(System.Xml.Linq.XElement) and calls reader.ReadStartElement()); change
the comment text from "XmlElemtnt" to "XmlElement" so it reads correctly (e.g.,
"XmlElement returns the 'ResultNode' unless we step into the contents").
- Around line 291-302: The loop over parameters should explicitly filter out
null values to make intent clear: replace the foreach over parameters with an
iteration over parameters.Where(p => p.Value != null) (or equivalent) before
appending to uriBuilder; keep using
methodParameters.GetTypeForMethodParameter(param.Key) and
WebQueryStringConverter.ConvertValueToString(param.Value, parameterType) to
obtain the string value and escape it before appending. This clarifies the
null-filtering intent while preserving the existing logic that appends '?' or
'&', escapes the key/value, and increments i.
- Around line 47-52: The constructor DataContractHttpDomainClient(HttpClient
httpClient, Type serviceInterface) currently validates serviceInterface but not
httpClient; add a null-argument guard at the start of that constructor to throw
ArgumentNullException(nameof(httpClient)) when httpClient is null so the
HttpClient property assignment (HttpClient = httpClient) fails fast with a clear
error instead of causing downstream NullReferenceException.
- Around line 213-270: The HttpRequestMessage created in PostAsync is not
disposed; change PostAsync to be async (async Task<HttpResponseMessage>
PostAsync(...)) and await HttpClient.SendAsync(..., DefaultHttpCompletionOption,
cancellationToken), then dispose the HttpRequestMessage (request.Dispose())
after the SendAsync completes but before returning the HttpResponseMessage so
the request's resources are cleaned up; update references inside PostAsync
(HttpRequestMessage, HttpClient.SendAsync, DefaultHttpCompletionOption)
accordingly and ensure only the response is returned to the caller.
In
`@src/OpenRiaServices.Hosting.AspNetCore/Framework/AspNetCore/Serialization/DataContractRequestSerializer.cs`:
- Around line 144-149: The reader may be positioned on insignificant whitespace
before the closing elements or EOF, so call reader.MoveToContent() before
performing reader.ReadEndElement() and before checking reader.ReadState in the
DataContractRequestSerializer code paths that currently do: if (hasMessageRoot)
reader.ReadEndElement(); and then if (reader.ReadState != ReadState.EndOfFile)
throw..., and likewise in the other block around lines 257-281; update both
spots to invoke reader.MoveToContent() immediately before the closing
ReadEndElement() and again immediately before the EOF/readstate check to skip
whitespace and ensure correct parsing.
- Around line 34-38: CanRead and CanWrite currently use MemoryExtensions.Equals
which fails for raw header values with casing, whitespace or parameters; replace
those comparisons to call the shared media-type helper MatchesMediaType (e.g.
MatchesMediaType(contentType, MimeTypes.BinaryXml)) so it trims, ignores case,
and tolerates parameters; apply the same change to the other matching methods
(the duplicate at the later block that handles the same media-type) to ensure
consistent binary media-type negotiation when OperationInvoker passes raw header
values.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 2c886a86-63a0-410a-b5c4-dcb59b349f5f
📒 Files selected for processing (4)
Changelog.mdsrc/OpenRiaServices.Client.DomainClients.Http/Framework/Http/DataContractHttpDomainClient.cssrc/OpenRiaServices.Client/Framework/DomainContext.cssrc/OpenRiaServices.Hosting.AspNetCore/Framework/AspNetCore/Serialization/DataContractRequestSerializer.cs
src/OpenRiaServices.Client.DomainClients.Http/Framework/Http/DataContractHttpDomainClient.cs
Fixed
Show fixed
Hide fixed
src/OpenRiaServices.Client.DomainClients.Http/Framework/Http/DataContractHttpDomainClient.cs
Fixed
Show fixed
Hide fixed
…ch as int) * Rename GetParametersFromUri => ReadParametersFromUri to match Read..FromBody * update readme
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (4)
Changelog.md (2)
37-38:⚠️ Potential issue | 🟡 MinorAdd a blank line before the fenced code block.
Sample usage:should be followed by a blank line before the opening ```csharp fence to satisfy markdown fence spacing.✏️ Suggested fix
Sample usage: + ```csharp DomainContext.DomainClientFactory = new XmlHttpDomainClientFactory(baseUri, httpClientFactory); ```🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Changelog.md` around lines 37 - 38, Add a blank line between the "Sample usage:" text and the following fenced code block in Changelog.md so the markdown has proper spacing; locate the "Sample usage:" line and insert an empty line immediately before the opening ```csharp fence to separate the paragraph from the code block.
9-10:⚠️ Potential issue | 🟡 MinorFix grammar and trailing whitespace in the XML server bullet.
Line 9 should read “can now intelligently handle…”, and Line 10 should not contain trailing whitespace.
✏️ Suggested fix
-* Server can now intelligently handles both binary and XML content types based on client requests and configuration settings - +* Server can now intelligently handle both binary and XML content types based on client requests and configuration settings +🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Changelog.md` around lines 9 - 10, Update the changelog bullet that currently reads "Server can now intelligently handles both binary and XML content types based on client requests and configuration settings" to use the correct verb form "handle" (i.e., "can now intelligently handle both binary and XML content types...") and remove any trailing whitespace at the end of that line; locate the bullet text in Changelog.md and edit that single line accordingly.src/OpenRiaServices.Hosting.AspNetCore/Framework/AspNetCore/Serialization/DataContractRequestSerializer.cs (2)
418-425: 🧹 Nitpick | 🔵 TrivialTrim whitespace from media type before comparison.
The media type value should be trimmed before comparison to handle edge cases like
" text/xml "or values with leading/trailing whitespace. The current implementation strips parameters but doesn't trim.🔧 Suggested fix
protected static bool MatchesMediaType(ReadOnlySpan<char> value, ReadOnlySpan<char> expected) { int separator = value.IndexOf(';'); if (separator >= 0) value = value[..separator]; - return value.Equals(expected, StringComparison.OrdinalIgnoreCase); + return value.Trim().Equals(expected, StringComparison.OrdinalIgnoreCase); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/OpenRiaServices.Hosting.AspNetCore/Framework/AspNetCore/Serialization/DataContractRequestSerializer.cs` around lines 418 - 425, MatchesMediaType currently removes parameters but does not trim leading/trailing whitespace, so inputs like " text/xml " fail; update the MatchesMediaType method to trim whitespace from the media type ReadOnlySpan<char> (after slicing off parameters) before comparing to expected, then perform the case-insensitive Equals comparison (StringComparison.OrdinalIgnoreCase) so values with extra spaces match correctly.
34-38:⚠️ Potential issue | 🟡 MinorBinary XML media type matching is case-sensitive, inconsistent with text XML and RFC 7231.
BinaryXmlDataContractRequestSerializerusesStringComparison.Ordinal(case-sensitive exact match), whileTextXmlDataContractRequestSerializeruses the case-insensitiveMatchesMediaTypehelper. Per RFC 7231 §3.1.1.1, media type tokens are case-insensitive. A client sendingApplication/Xmlwould be rejected.Consider using
MatchesMediaTypehere as well for consistency:🔧 Suggested fix
public override bool CanRead(ReadOnlySpan<char> contentType) - => MemoryExtensions.Equals(contentType, MimeTypes.BinaryXml, StringComparison.Ordinal); + => MatchesMediaType(contentType, MimeTypes.BinaryXml); public override bool CanWrite(ReadOnlySpan<char> contentType) - => MemoryExtensions.Equals(contentType, MimeTypes.BinaryXml, StringComparison.Ordinal); + => MatchesMediaType(contentType, MimeTypes.BinaryXml);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/OpenRiaServices.Hosting.AspNetCore/Framework/AspNetCore/Serialization/DataContractRequestSerializer.cs` around lines 34 - 38, The BinaryXmlDataContractRequestSerializer currently uses case-sensitive comparison in CanRead and CanWrite (MemoryExtensions.Equals with StringComparison.Ordinal) which rejects valid media types like "Application/Xml"; update both CanRead and CanWrite in BinaryXmlDataContractRequestSerializer to use the same case-insensitive MatchesMediaType helper used by TextXmlDataContractRequestSerializer when comparing against MimeTypes.BinaryXml so media type token matching follows RFC 7231.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Changelog.md`:
- Line 7: Update the release note entry that currently reads "statuscode 400:
bad request" to use standard HTTP wording by changing it to "status code 400
(Bad Request)"; locate the exact string "statuscode 400: bad request" in the
Changelog entry and replace it with "status code 400 (Bad Request)" to match the
project's style and capitalization conventions.
In
`@src/OpenRiaServices.Hosting.AspNetCore/Framework/AspNetCore/Operations/OperationInvoker.cs`:
- Around line 99-148: GetSerializerForWrite currently always returns a non-null
RequestSerializer (falling back to serializers[0]) so the null-checks for
"writer" in QueryOperationInvoker and InvokeOperationInvoker are unreachable;
remove the redundant "if (writer is null) return 406" branches in those callers
(or alternatively change GetSerializerForWrite to return null on no-matching
Accept if you prefer that behavior) — locate the call sites that use the
returned "writer" variable in QueryOperationInvoker and InvokeOperationInvoker
and delete the unreachable null-check and early 406 return to keep behavior
consistent with GetSerializerForWrite.
---
Duplicate comments:
In `@Changelog.md`:
- Around line 37-38: Add a blank line between the "Sample usage:" text and the
following fenced code block in Changelog.md so the markdown has proper spacing;
locate the "Sample usage:" line and insert an empty line immediately before the
opening ```csharp fence to separate the paragraph from the code block.
- Around line 9-10: Update the changelog bullet that currently reads "Server can
now intelligently handles both binary and XML content types based on client
requests and configuration settings" to use the correct verb form "handle"
(i.e., "can now intelligently handle both binary and XML content types...") and
remove any trailing whitespace at the end of that line; locate the bullet text
in Changelog.md and edit that single line accordingly.
In
`@src/OpenRiaServices.Hosting.AspNetCore/Framework/AspNetCore/Serialization/DataContractRequestSerializer.cs`:
- Around line 418-425: MatchesMediaType currently removes parameters but does
not trim leading/trailing whitespace, so inputs like " text/xml " fail; update
the MatchesMediaType method to trim whitespace from the media type
ReadOnlySpan<char> (after slicing off parameters) before comparing to expected,
then perform the case-insensitive Equals comparison
(StringComparison.OrdinalIgnoreCase) so values with extra spaces match
correctly.
- Around line 34-38: The BinaryXmlDataContractRequestSerializer currently uses
case-sensitive comparison in CanRead and CanWrite (MemoryExtensions.Equals with
StringComparison.Ordinal) which rejects valid media types like
"Application/Xml"; update both CanRead and CanWrite in
BinaryXmlDataContractRequestSerializer to use the same case-insensitive
MatchesMediaType helper used by TextXmlDataContractRequestSerializer when
comparing against MimeTypes.BinaryXml so media type token matching follows RFC
7231.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 23db404e-16b2-4c3e-bd84-8f853e6cf93b
📒 Files selected for processing (5)
Changelog.mdsrc/OpenRiaServices.Hosting.AspNetCore/Framework/AspNetCore/Operations/InvokeOperationInvoker.cssrc/OpenRiaServices.Hosting.AspNetCore/Framework/AspNetCore/Operations/OperationInvoker.cssrc/OpenRiaServices.Hosting.AspNetCore/Framework/AspNetCore/Operations/QueryOperationInvoker.cssrc/OpenRiaServices.Hosting.AspNetCore/Framework/AspNetCore/Serialization/DataContractRequestSerializer.cs
src/OpenRiaServices.Hosting.AspNetCore/Framework/AspNetCore/Operations/OperationInvoker.cs
Show resolved
Hide resolved
This was
linked to
issues
Apr 2, 2026
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
New Features
This adds the same features as #400 (replaces it) and also makes the wire format pluggable so that additional formats such as messagepack or json can be added much easier.
Fixed bugs:
intwill give HTTP 400Public API Changes & Usage
Client
New class
OpenRiaServices.Client.DomainClients.XmlHttpDomainClientFactorythat can be used to enable text based XML communication with the server.Sample usage
Server
Alternative 1 - methods on options builder
Summary by CodeRabbit
New Features
Bug Fixes
Chores
Tests