Add additional Nullable annotations#569
Merged
Daniel-Svensson merged 19 commits intoApr 24, 2026
Merged
Conversation
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 nullable reference types across many Client and Server source files, updating fields, properties, method signatures, events, and constructors to nullable-aware types; introduces small polyfills and nullable helper attributes; adjusts analyzer settings and a test to retry on failure. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (13)
src/OpenRiaServices.Server/Framework/Data/AuthorizationContext.cs (2)
196-212: 🧹 Nitpick | 🔵 TrivialExtend
MemberNotNullto cover_operationTypeand drop the!.
_operationand_operationTypeare always set together inSetup, which is what the comment on line 202 relies on. You can encode that invariant directly in the attribute so the compiler stops needing the null-forgiving operator, and future refactors can't accidentally desync the two fields without the compiler noticing.♻️ Proposed fix
- return this._operationType!; // _operation and _operationType are always set together, so we can be sure that if _operation is not null, then _operationType is not null either + return this._operationType;`#pragma` warning disable CS3016 // Arrays as attribute arguments is not CLS-compliant - [System.Diagnostics.CodeAnalysis.MemberNotNull(nameof(_operation))] + [System.Diagnostics.CodeAnalysis.MemberNotNull(nameof(_operation), nameof(_operationType))] `#pragma` warning restore CS3016 // Arrays as attribute arguments is not CLS-compliant private void EnsureNotTemplate()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/OpenRiaServices.Server/Framework/Data/AuthorizationContext.cs` around lines 196 - 212, Update the MemberNotNull attribute on EnsureNotTemplate to include both backing fields so the compiler knows _operation and _operationType are non-null together: modify the attribute on EnsureNotTemplate to list nameof(_operation) and nameof(_operationType), then remove the null-forgiving operator in the OperationType getter (refer to OperationType, EnsureNotTemplate, _operation and _operationType and the Setup method that sets them together).
103-113:⚠️ Potential issue | 🟡 MinorNull-check
authorizationContextbefore passing it tothis(...).
: this((IServiceProvider) authorizationContext)dereferences/evaluatesauthorizationContextbefore the null-check on line 106-109 runs. Under#nullable enable, the parameter is declared non-nullable so this is mostly a contract issue, but the existing explicit null-check then becomes unreachable-for-nullable-oblivious-callers only by accident of evaluation order. Consider either removing the now-redundant runtime null-check (trusting the non-nullable annotation) or using a helper that validates first — mixed signaling here is confusing. Minor.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/OpenRiaServices.Server/Framework/Data/AuthorizationContext.cs` around lines 103 - 113, The constructor AuthorizationContext(object? instance, string operation, string operationType, AuthorizationContext authorizationContext) currently calls this((IServiceProvider)authorizationContext) before performing a null-check on authorizationContext, which is confusing; remove the redundant runtime null-check (the parameter is non-nullable) so the code consistently relies on the non-nullable contract, and keep the call to this((IServiceProvider)authorizationContext) and the subsequent Setup call that uses authorizationContext._items and the existing comment about preserving lazy instantiation.src/OpenRiaServices.Client/Framework/SubmitResult.cs (1)
17-28:⚠️ Potential issue | 🟡 Minor
ChangeSetis declared non-nullable but the constructor does not validate the argument.Under
#nullable enable,ChangeSetadvertises a non-null contract to consumers, yetnew SubmitResult(null!)(or a null flowing in through a nullable-oblivious caller) will silently produce aSubmitResultwhoseChangeSetviolates its declared type. AThrowIfNullon construction would make the contract enforceable at runtime as well.🛡️ Proposed fix
public SubmitResult(EntityChangeSet changeSet) { + ArgumentNullException.ThrowIfNull(changeSet); ChangeSet = changeSet; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/OpenRiaServices.Client/Framework/SubmitResult.cs` around lines 17 - 28, The SubmitResult constructor allows a null EntityChangeSet even though the ChangeSet property is non-nullable; update the SubmitResult(EntityChangeSet changeSet) constructor to validate changeSet (e.g., throw new ArgumentNullException(nameof(changeSet)) or call a ThrowIfNull helper) before assigning to the ChangeSet property so the non-null contract is enforced at runtime.src/OpenRiaServices.Server/Framework/Data/DomainOperationEntry.cs (2)
158-164:⚠️ Potential issue | 🟠 MajorMake
OperationAttributenullable or enforce the invariant.Line 163 returns
_operationAttributeafter the field was changed toAttribute?. Since the default branch can leave it unset, this public non-null contract is now misleading and can surface nullable warnings or downstream NREs.🛠️ Proposed nullable-contract fix
- public Attribute OperationAttribute + public Attribute? OperationAttribute { get { this.InitializeOperationAttribute(); return this._operationAttribute; } }If callers require a non-null attribute, throw here instead of returning the nullable field.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/OpenRiaServices.Server/Framework/Data/DomainOperationEntry.cs` around lines 158 - 164, The OperationAttribute getter returns the field _operationAttribute which was changed to Attribute?; update the contract by either making the OperationAttribute property type nullable (Attribute?) or enforce the invariant by throwing when _operationAttribute is null—call InitializeOperationAttribute(), check this._operationAttribute, and if still null throw an InvalidOperationException (or similar) so callers never receive a null; reference: OperationAttribute property, InitializeOperationAttribute(), and the _operationAttribute field.
371-400:⚠️ Potential issue | 🟠 MajorPropagate the nullable
AssociatedTypecontract to existing callers.Line 371 makes
AssociatedTypenullable, but known call sites still use it as non-null:DomainServiceDescription.AddQueryMethodcallsAddEntityType(method.AssociatedType),DomainServiceDescription.AddSubmitMethodassigns it toType entityType, andDomainServicestores it directly incontext.Items. Guard those paths or preserve a non-null invariant for operations that require an associated entity type.Example downstream pattern:
- Type entityType = method.AssociatedType; + Type entityType = method.AssociatedType + ?? throw new InvalidOperationException("Submit methods must have an associated entity type.");🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/OpenRiaServices.Server/Framework/Data/DomainOperationEntry.cs` around lines 371 - 400, AssociatedType was changed to be nullable but callers assume non-null; update all call sites to handle null instead of forcing a Type. Specifically, in DomainServiceDescription.AddQueryMethod avoid calling AddEntityType(method.AssociatedType) when method.AssociatedType is null (guard and skip or handle appropriately), in DomainServiceDescription.AddSubmitMethod check method.AssociatedType for null before assigning to a local Type entityType (or use a nullable Type? and branch accordingly), and in DomainService where the AssociatedType is stored into context.Items only store it when non-null (or store a nullable marker and handle it where read). Locate references to AssociatedType on DomainOperationEntry and add null checks/guards around AddEntityType, AddSubmitMethod assignments, and context.Items usage so the nullable contract is respected.src/OpenRiaServices.Client/Framework/InvokeArgs.cs (1)
21-52:⚠️ Potential issue | 🟡 MinorAllow nullable values inside the invoke parameter bag.
Line 21 allows the parameter dictionary itself to be null, but
IDictionary<string, object>still says individual argument values cannot be null. Nullable invoke parameters needobject?values.🛠️ Proposed nullable-contract fix
- public InvokeArgs(string operationName, Type returnType, IDictionary<string, object>? parameters, bool hasSideEffects) + public InvokeArgs(string operationName, Type returnType, IDictionary<string, object?>? parameters, bool hasSideEffects) @@ - public IDictionary<string, object>? Parameters { get; } + public IDictionary<string, object?>? Parameters { get; }Apply the same value-nullability annotation to downstream parameter-bag consumers as needed.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/OpenRiaServices.Client/Framework/InvokeArgs.cs` around lines 21 - 52, The Parameters bag currently uses IDictionary<string, object>? which disallows null values for individual entries; change the Parameters type in the InvokeArgs constructor signature and the Parameters property to IDictionary<string, object?>? (i.e., allow object? values) so callers can pass nullable argument values, update the constructor parameter name and assignment to match that nullable type (constructor InvokeArgs(string operationName, Type returnType, IDictionary<string, object?>? parameters, bool hasSideEffects) and the property Parameters { get; }), and then search for and update any downstream consumers/places accessing InvokeArgs.Parameters to handle possible null entry values (add null checks, null-coalescing or pattern matches as appropriate).src/OpenRiaServices.Client/Framework/Entity.INotifyDataErrorInfo.cs (1)
51-57:⚠️ Potential issue | 🟡 MinorReturn both null and empty member-name errors for entity-level queries.
Line 56 only checks for the exact
propertyNamevalue, soGetErrors(null)misses entity-level errors stored with"", andGetErrors("")misses errors stored withnull, despite the comment saying both should be included.🐛 Proposed fix
- results = this.ValidationErrors.Where(e => !e.MemberNames.Any() || e.MemberNames.Contains(propertyName)); + results = this.ValidationErrors.Where(e => !e.MemberNames.Any() || e.MemberNames.Any(string.IsNullOrEmpty));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/OpenRiaServices.Client/Framework/Entity.INotifyDataErrorInfo.cs` around lines 51 - 57, The predicate in the GetErrors branch that handles entity-level queries only checks e.MemberNames.Contains(propertyName) and therefore misses errors recorded with the opposite null/empty member-name; update the condition in the if (string.IsNullOrEmpty(propertyName)) block (the logic operating on this.ValidationErrors inside GetErrors/Entity.INotifyDataErrorInfo) to include member names that are null or empty as well as the queried propertyName—for example, replace the current Where(...) with one that matches e.MemberNames.Any(m => string.IsNullOrEmpty(m) || m == propertyName) or equivalent so both "" and null stored member names are returned for entity-level queries.src/OpenRiaServices.Client/Framework/EntityQuery.cs (1)
188-189: 🛠️ Refactor suggestion | 🟠 MajorPropagate nullable
parametersto the generic derived constructor too.The base
EntityQueryctor now acceptsIDictionary<string, object>?, butEntityQuery<TEntity>'s internal ctor still declaresIDictionary<string, object> parameters(non-nullable). Callers that legitimately passnull(as supported by the base contract and XML docs) will now produce a nullable-analysis warning. For consistency, make this parameter nullable too.♻️ Proposed change
- internal EntityQuery(DomainClient domainClient, string queryName, IDictionary<string, object> parameters, bool hasSideEffects, bool isComposable) + internal EntityQuery(DomainClient domainClient, string queryName, IDictionary<string, object>? parameters, bool hasSideEffects, bool isComposable) : base(domainClient, queryName, typeof(TEntity), parameters, hasSideEffects, isComposable)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/OpenRiaServices.Client/Framework/EntityQuery.cs` around lines 188 - 189, The generic constructor EntityQuery<TEntity>(DomainClient domainClient, string queryName, IDictionary<string, object> parameters, bool hasSideEffects, bool isComposable) should accept a nullable parameters type to match the base EntityQuery constructor; change the parameters parameter to IDictionary<string, object>? so callers can pass null without nullable warnings, and ensure the call to base(domainClient, queryName, typeof(TEntity), parameters, hasSideEffects, isComposable) continues to pass the (now nullable) parameter value unchanged.src/OpenRiaServices.Server/Framework/Data/DomainServiceContext.cs (1)
26-78: 🧹 Nitpick | 🔵 TrivialLGTM — but note this is a binary/source-compat-sensitive public API change.
Making
User,Operation, andGetServicereturn types nullable, and relaxing the ctoruserparameter toIPrincipal?, is a public surface change for server authors. Source-breaking for any override ofGetServicedeclared with non-nullableobjectreturn in consuming code compiled with nullable-warnings-as-errors. Consider mentioning this in the release notes / migration section of the PR.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/OpenRiaServices.Server/Framework/Data/DomainServiceContext.cs` around lines 26 - 78, The public API was changed to nullable for DomainServiceContext.User, DomainServiceContext.Operation, DomainServiceContext.GetService return type and the constructor parameter DomainServiceContext(IServiceProvider, IPrincipal? ...) which can break source/binary compatibility for consumers (e.g., overrides of GetService compiled with non-nullable returns); update the release notes and migration guide to explicitly list these symbols (User, Operation, GetService, and the nullable ctor overload) and provide guidance for consumers to recompile or add nullable annotations/adjust overrides (or keep a non-nullable wrapper) to avoid build errors under nullable-warnings-as-errors.src/OpenRiaServices.Client/Framework/EntityRef.cs (1)
410-417:⚠️ Potential issue | 🟠 Major
[MemberNotNullWhen(true, nameof(Entity))]onHasValueviolates its contract.
IEntityRef.HasValuereturnstruewhenever_entity != null, but theEntitygetter (lines 80–117) can still returnnulleven whenHasValueistrue. WhenEntityis accessed, if!_hasAssignedEntity && !_entityPredicate(_entity), the getter resets_entity = null(line 92) and attempts to load viaGetSingleMatch, which may returnnull. Callers relying on this attribute contract (e.g.,if (entityRef.HasValue) use(entityRef.Entity);) inEntitySet.cs:1160,ChangeSetBuilder.cs:407, andChangeSetBuilder.cs:485are vulnerable to null dereference.Unlike
OperationBase.HasError, which safely guarantees its contract with a simple null check,HasValue's promise is broken by theEntitygetter's side effects. Either remove the[MemberNotNullWhen]attribute from bothIEntityRef.HasValue(line 456) and the implementation (line 410), or tightenHasValueto returnthis.Entity != null(accepting the deferred load side effect).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/OpenRiaServices.Client/Framework/EntityRef.cs` around lines 410 - 417, The MemberNotNullWhen attribute on IEntityRef.HasValue/EntityRef.HasValue is incorrect because HasValue only checks _entity != null while the Entity getter can null-out _entity and return null (due to _hasAssignedEntity, _entityPredicate and GetSingleMatch), breaking the attribute contract; fix by either removing the [System.Diagnostics.CodeAnalysis.MemberNotNullWhen(true, nameof(Entity))] annotation from the interface member (IEntityRef.HasValue) and the implementation (EntityRef.HasValue) or change the HasValue implementation to force the same semantics as the Entity getter (return this.Entity != null) so the attribute holds; update callers in EntitySet.cs and ChangeSetBuilder.cs as needed to rely on the chosen behavior.src/OpenRiaServices.Server/Framework/Data/DomainService.cs (3)
585-609: 🧹 Nitpick | 🔵 TrivialRedundant null-forgiving operators.
attributes,principal, andauthorizationContextare all declared as non-nullable parameters onEvaluateAuthorization, soattributes!,principal!, andauthorizationContext!here are no-ops that only add noise.♻️ Proposed cleanup
- foreach (AuthorizationAttribute attribute in attributes!) + foreach (AuthorizationAttribute attribute in attributes) { if (attribute is RequiresAuthenticationAttribute) { - AuthorizationResult result = attribute.Authorize(principal!, authorizationContext!); + AuthorizationResult result = attribute.Authorize(principal, authorizationContext); @@ - AuthorizationResult result = attribute.Authorize(principal!, authorizationContext!); + AuthorizationResult result = attribute.Authorize(principal, authorizationContext);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/OpenRiaServices.Server/Framework/Data/DomainService.cs` around lines 585 - 609, The null-forgiving operators are redundant: remove the unnecessary '!' usages on attributes, principal, and authorizationContext in the EvaluateAuthorization logic; update the two foreach blocks to iterate over attributes (not attributes!) and call attribute.Authorize(principal, authorizationContext) (not principal!/authorizationContext!), keeping the same RequiresAuthenticationAttribute checks and return logic in DomainService.EvaluateAuthorization.
1135-1185:⚠️ Potential issue | 🔴 CriticalWrong return value: boxing a
ValueTask<object?>as theobject?result.
InvokeDomainOperationEntryAsyncisasync ValueTask<object?>, so the body mustreturnanobject?value (the async state machine wraps it in aValueTask<object?>). The fallthrough statement returnsnew ValueTask<object?>((object?)null)which gets boxed intoobject?and becomes the awaited result — callers that ever observe this return value would receive a boxedValueTask<object?>instead ofnull.Current callers (
InvokeCudOperationsAsync,InvokeCustomOperations) discard the result so it isn't observably broken today, but the code is semantically wrong and will bite any future caller.🐛 Proposed fix
- return new ValueTask<object?>((object?)null); + return null;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/OpenRiaServices.Server/Framework/Data/DomainService.cs` around lines 1135 - 1185, The method InvokeDomainOperationEntryAsync incorrectly returns new ValueTask<object?>((object?)null) at the end, which boxes a ValueTask into the object result; change the final return to return null; so the async state machine produces a ValueTask<object?> with a null result. Locate the method InvokeDomainOperationEntryAsync and replace the fall-through return expression with a plain null return (return null;) while leaving the existing exception handling and ServiceContext cleanup intact.
175-209: 🧹 Nitpick | 🔵 Trivial
resultshould not be nullable.
EvaluateAuthorizationreturns a non-nullableAuthorizationResultand the using block unconditionally assigns toresult. DeclaringAuthorizationResult? result = null;makes the return at line 209 a potential CS8603 (nullable-to-non-nullable) and is semantically misleading — the final value is always non-null.♻️ Proposed cleanup
- AuthorizationContext? contextTemplate = this.AuthorizationContext; - AuthorizationResult? result = null; + AuthorizationContext? contextTemplate = this.AuthorizationContext; + AuthorizationResult result;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/OpenRiaServices.Server/Framework/Data/DomainService.cs` around lines 175 - 209, The local variable `AuthorizationResult? result = null;` is incorrectly nullable; change it to a non-nullable `AuthorizationResult result;` (remove the `?` and the `= null` initializer) because `DomainService.EvaluateAuthorization` returns a non-null `AuthorizationResult` and the using block always assigns `result` before it is returned; update any related nullable annotations if necessary so the method returns a non-null `AuthorizationResult`.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/OpenRiaServices.Client/Framework/Polyfills.cs`:
- Around line 13-30: The file uses C# 14 extension member syntax inside the
Polyfills class (the extension(ArgumentNullException) block and the ThrowIfNull
method) which won't compile under older language versions for
net472/netstandard2.0; update the consuming project files to set
<LangVersion>14.0</LangVersion> (or latest/preview) so the extension member
compiles, and remove the unused private ThrowArgumentNullException method
(ThrowArgumentNullException) from Polyfills to eliminate dead code.
In `@src/OpenRiaServices.Server/Framework/Data/AuthorizationAttribute.cs`:
- Around line 158-161: The local variable `message` is declared as nullable but
`ErrorMessageAccessor` returns a non-nullable string; change the local from
`string? message` to `string message` so the call to
`string.Format(CultureInfo.CurrentCulture, message, operation)` uses a
non-nullable format string; update the declaration where
`ErrorMessageAccessor()` is invoked (refer to `ErrorMessageAccessor`, local
`message`, and the `string.Format(..., message, operation)` call).
In `@src/OpenRiaServices.Server/Framework/Data/DomainService.cs`:
- Around line 661-671: Remove the superfluous null-forgiving initializer and
unnecessary null-forgiving call: declare customMethodOperation as
DomainOperationEntry? without initializing it (or initialize to null), drop the
trailing ! on domainServiceDescription.GetCustomMethod(...) so the
null-coalescing assignment reads naturally, and keep the existing comment about
ResolveOperations validating non-null; references: customMethodOperation,
invokedActions, domainServiceDescription.GetCustomMethod and ResolveOperations.
In `@src/OpenRiaServices.Server/Framework/Data/DomainServiceErrorInfo.cs`:
- Line 21: Remove the explicit nameof(exception) when calling
ArgumentNullException.ThrowIfNull so the CallerArgumentExpression can supply the
parameter name (replace ArgumentNullException.ThrowIfNull(exception,
nameof(exception)) with a single-argument ThrowIfNull(exception)); also update
the Error property's setter (the setter around lines 37-40 in
DomainServiceErrorInfo) to use ArgumentNullException.ThrowIfNull(error) the same
way for consistency.
---
Outside diff comments:
In `@src/OpenRiaServices.Client/Framework/Entity.INotifyDataErrorInfo.cs`:
- Around line 51-57: The predicate in the GetErrors branch that handles
entity-level queries only checks e.MemberNames.Contains(propertyName) and
therefore misses errors recorded with the opposite null/empty member-name;
update the condition in the if (string.IsNullOrEmpty(propertyName)) block (the
logic operating on this.ValidationErrors inside
GetErrors/Entity.INotifyDataErrorInfo) to include member names that are null or
empty as well as the queried propertyName—for example, replace the current
Where(...) with one that matches e.MemberNames.Any(m => string.IsNullOrEmpty(m)
|| m == propertyName) or equivalent so both "" and null stored member names are
returned for entity-level queries.
In `@src/OpenRiaServices.Client/Framework/EntityQuery.cs`:
- Around line 188-189: The generic constructor EntityQuery<TEntity>(DomainClient
domainClient, string queryName, IDictionary<string, object> parameters, bool
hasSideEffects, bool isComposable) should accept a nullable parameters type to
match the base EntityQuery constructor; change the parameters parameter to
IDictionary<string, object>? so callers can pass null without nullable warnings,
and ensure the call to base(domainClient, queryName, typeof(TEntity),
parameters, hasSideEffects, isComposable) continues to pass the (now nullable)
parameter value unchanged.
In `@src/OpenRiaServices.Client/Framework/EntityRef.cs`:
- Around line 410-417: The MemberNotNullWhen attribute on
IEntityRef.HasValue/EntityRef.HasValue is incorrect because HasValue only checks
_entity != null while the Entity getter can null-out _entity and return null
(due to _hasAssignedEntity, _entityPredicate and GetSingleMatch), breaking the
attribute contract; fix by either removing the
[System.Diagnostics.CodeAnalysis.MemberNotNullWhen(true, nameof(Entity))]
annotation from the interface member (IEntityRef.HasValue) and the
implementation (EntityRef.HasValue) or change the HasValue implementation to
force the same semantics as the Entity getter (return this.Entity != null) so
the attribute holds; update callers in EntitySet.cs and ChangeSetBuilder.cs as
needed to rely on the chosen behavior.
In `@src/OpenRiaServices.Client/Framework/InvokeArgs.cs`:
- Around line 21-52: The Parameters bag currently uses IDictionary<string,
object>? which disallows null values for individual entries; change the
Parameters type in the InvokeArgs constructor signature and the Parameters
property to IDictionary<string, object?>? (i.e., allow object? values) so
callers can pass nullable argument values, update the constructor parameter name
and assignment to match that nullable type (constructor InvokeArgs(string
operationName, Type returnType, IDictionary<string, object?>? parameters, bool
hasSideEffects) and the property Parameters { get; }), and then search for and
update any downstream consumers/places accessing InvokeArgs.Parameters to handle
possible null entry values (add null checks, null-coalescing or pattern matches
as appropriate).
In `@src/OpenRiaServices.Client/Framework/SubmitResult.cs`:
- Around line 17-28: The SubmitResult constructor allows a null EntityChangeSet
even though the ChangeSet property is non-nullable; update the
SubmitResult(EntityChangeSet changeSet) constructor to validate changeSet (e.g.,
throw new ArgumentNullException(nameof(changeSet)) or call a ThrowIfNull helper)
before assigning to the ChangeSet property so the non-null contract is enforced
at runtime.
In `@src/OpenRiaServices.Server/Framework/Data/AuthorizationContext.cs`:
- Around line 196-212: Update the MemberNotNull attribute on EnsureNotTemplate
to include both backing fields so the compiler knows _operation and
_operationType are non-null together: modify the attribute on EnsureNotTemplate
to list nameof(_operation) and nameof(_operationType), then remove the
null-forgiving operator in the OperationType getter (refer to OperationType,
EnsureNotTemplate, _operation and _operationType and the Setup method that sets
them together).
- Around line 103-113: The constructor AuthorizationContext(object? instance,
string operation, string operationType, AuthorizationContext
authorizationContext) currently calls
this((IServiceProvider)authorizationContext) before performing a null-check on
authorizationContext, which is confusing; remove the redundant runtime
null-check (the parameter is non-nullable) so the code consistently relies on
the non-nullable contract, and keep the call to
this((IServiceProvider)authorizationContext) and the subsequent Setup call that
uses authorizationContext._items and the existing comment about preserving lazy
instantiation.
In `@src/OpenRiaServices.Server/Framework/Data/DomainOperationEntry.cs`:
- Around line 158-164: The OperationAttribute getter returns the field
_operationAttribute which was changed to Attribute?; update the contract by
either making the OperationAttribute property type nullable (Attribute?) or
enforce the invariant by throwing when _operationAttribute is null—call
InitializeOperationAttribute(), check this._operationAttribute, and if still
null throw an InvalidOperationException (or similar) so callers never receive a
null; reference: OperationAttribute property, InitializeOperationAttribute(),
and the _operationAttribute field.
- Around line 371-400: AssociatedType was changed to be nullable but callers
assume non-null; update all call sites to handle null instead of forcing a Type.
Specifically, in DomainServiceDescription.AddQueryMethod avoid calling
AddEntityType(method.AssociatedType) when method.AssociatedType is null (guard
and skip or handle appropriately), in DomainServiceDescription.AddSubmitMethod
check method.AssociatedType for null before assigning to a local Type entityType
(or use a nullable Type? and branch accordingly), and in DomainService where the
AssociatedType is stored into context.Items only store it when non-null (or
store a nullable marker and handle it where read). Locate references to
AssociatedType on DomainOperationEntry and add null checks/guards around
AddEntityType, AddSubmitMethod assignments, and context.Items usage so the
nullable contract is respected.
In `@src/OpenRiaServices.Server/Framework/Data/DomainService.cs`:
- Around line 585-609: The null-forgiving operators are redundant: remove the
unnecessary '!' usages on attributes, principal, and authorizationContext in the
EvaluateAuthorization logic; update the two foreach blocks to iterate over
attributes (not attributes!) and call attribute.Authorize(principal,
authorizationContext) (not principal!/authorizationContext!), keeping the same
RequiresAuthenticationAttribute checks and return logic in
DomainService.EvaluateAuthorization.
- Around line 1135-1185: The method InvokeDomainOperationEntryAsync incorrectly
returns new ValueTask<object?>((object?)null) at the end, which boxes a
ValueTask into the object result; change the final return to return null; so the
async state machine produces a ValueTask<object?> with a null result. Locate the
method InvokeDomainOperationEntryAsync and replace the fall-through return
expression with a plain null return (return null;) while leaving the existing
exception handling and ServiceContext cleanup intact.
- Around line 175-209: The local variable `AuthorizationResult? result = null;`
is incorrectly nullable; change it to a non-nullable `AuthorizationResult
result;` (remove the `?` and the `= null` initializer) because
`DomainService.EvaluateAuthorization` returns a non-null `AuthorizationResult`
and the using block always assigns `result` before it is returned; update any
related nullable annotations if necessary so the method returns a non-null
`AuthorizationResult`.
In `@src/OpenRiaServices.Server/Framework/Data/DomainServiceContext.cs`:
- Around line 26-78: The public API was changed to nullable for
DomainServiceContext.User, DomainServiceContext.Operation,
DomainServiceContext.GetService return type and the constructor parameter
DomainServiceContext(IServiceProvider, IPrincipal? ...) which can break
source/binary compatibility for consumers (e.g., overrides of GetService
compiled with non-nullable returns); update the release notes and migration
guide to explicitly list these symbols (User, Operation, GetService, and the
nullable ctor overload) and provide guidance for consumers to recompile or add
nullable annotations/adjust overrides (or keep a non-nullable wrapper) to avoid
build errors under nullable-warnings-as-errors.
🪄 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: babb1169-a9d8-4ef7-ae02-11f59a5db901
📒 Files selected for processing (26)
src/.editorconfigsrc/OpenRiaServices.Client/Framework/Entity.INotifyDataErrorInfo.cssrc/OpenRiaServices.Client/Framework/EntityQuery.cssrc/OpenRiaServices.Client/Framework/EntityRef.cssrc/OpenRiaServices.Client/Framework/IDomainClientFactory.cssrc/OpenRiaServices.Client/Framework/IEntityCollection.cssrc/OpenRiaServices.Client/Framework/IEntityCollection1.cs`src/OpenRiaServices.Client/Framework/InvokeArgs.cssrc/OpenRiaServices.Client/Framework/InvokeCompletedResult.cssrc/OpenRiaServices.Client/Framework/InvokeOperation.cssrc/OpenRiaServices.Client/Framework/LoadOperation.cssrc/OpenRiaServices.Client/Framework/LoadResult.cssrc/OpenRiaServices.Client/Framework/OperationBase.cssrc/OpenRiaServices.Client/Framework/Polyfills.cssrc/OpenRiaServices.Client/Framework/SubmitOperation.cssrc/OpenRiaServices.Client/Framework/SubmitResult.cssrc/OpenRiaServices.Client/Framework/ValidationUtilities.cssrc/OpenRiaServices.Server/Framework/Data/AuthorizationAttribute.cssrc/OpenRiaServices.Server/Framework/Data/AuthorizationContext.cssrc/OpenRiaServices.Server/Framework/Data/DomainOperationEntry.cssrc/OpenRiaServices.Server/Framework/Data/DomainOperationParameter.cssrc/OpenRiaServices.Server/Framework/Data/DomainService.cssrc/OpenRiaServices.Server/Framework/Data/DomainServiceContext.cssrc/OpenRiaServices.Server/Framework/Data/DomainServiceErrorInfo.cssrc/OpenRiaServices.Server/Framework/Data/IDomainServiceFactory.cssrc/Test/OpenRiaservices.EndToEnd.Wcf.Test/Data/UpdateTests.cs
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/OpenRiaServices.Server/Framework/Data/DomainOperationEntry.cs (1)
158-165:⚠️ Potential issue | 🟡 Minor
OperationAttributegetter can returnnulldespite the non-nullable return type.
_operationAttributeis nowAttribute?(line 22).InitializeOperationAttributehas adefault:branch (line 298) that intentionally does nothing whenOperationisDomainOperation.None(per the comment aboutClassifyDomainOperation), so this getter can leak anullthrough a property typed as non-nullableAttribute. That should either warn (CS8603) or, if warnings-as-errors are disabled, silently produce a contract violation for consumers using nullable context.🛡️ Options
Either throw when accessed before the operation is classified:
public Attribute OperationAttribute { get { this.InitializeOperationAttribute(); - return this._operationAttribute; + return this._operationAttribute + ?? throw new InvalidOperationException("OperationAttribute accessed before DomainOperation was classified."); } }…or make the property nullable (
Attribute?) to reflect actual behavior (note: this is a public-API change, so may need a changelog entry).The two
_operationAttribute!usages at lines 307/311 are fine because they are only reached whenattributeCreated == true.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/OpenRiaServices.Server/Framework/Data/DomainOperationEntry.cs` around lines 158 - 165, The OperationAttribute getter currently calls InitializeOperationAttribute() but can return a null _operationAttribute (now Attribute?) when Operation is DomainOperation.None; fix by either making the property return Attribute? (change OperationAttribute's return type to nullable) or enforce non-null by throwing a meaningful InvalidOperationException when _operationAttribute is null (e.g., after InitializeOperationAttribute() check _operationAttribute and throw with context), and adjust any callers/tests accordingly; reference the OperationAttribute property, the _operationAttribute field, and InitializeOperationAttribute (which has the default branch for DomainOperation.None) when making the change.src/OpenRiaServices.Server/Framework/Data/DomainService.cs (1)
576-612: 🧹 Nitpick | 🔵 TrivialRedundant null-forgiving operators on already-non-nullable parameters.
attributes,principal, andauthorizationContextare all declared non-nullable on the method signature (and areDebug.Assert'd as non-null on lines 578-580). The!suffixes at lines 585, 589, and 603 don't change semantics and just add noise from the nullable conversion pass.♻️ Proposed cleanup
- foreach (AuthorizationAttribute attribute in attributes!) + foreach (AuthorizationAttribute attribute in attributes) { if (attribute is RequiresAuthenticationAttribute) { - AuthorizationResult result = attribute.Authorize(principal!, authorizationContext!); + AuthorizationResult result = attribute.Authorize(principal, authorizationContext); @@ - AuthorizationResult result = attribute.Authorize(principal!, authorizationContext!); + AuthorizationResult result = attribute.Authorize(principal, authorizationContext);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/OpenRiaServices.Server/Framework/Data/DomainService.cs` around lines 576 - 612, In EvaluateAuthorization, remove the redundant null-forgiving operators used on the already non-nullable parameters (attributes!, principal!, authorizationContext!) — locate the two foreach loops and the calls to attribute.Authorize and drop the trailing '!' from attributes, principal, and authorizationContext so the code uses the declared non-nullable variables directly (e.g., change attribute.Authorize(principal!, authorizationContext!) to attribute.Authorize(principal, authorizationContext) and similarly remove '!' on attributes in the foreach headers).
♻️ Duplicate comments (1)
src/OpenRiaServices.Client/Framework/Polyfills.cs (1)
1-38:⚠️ Potential issue | 🟠 MajorMake these shims shared across non-NET targets and verify the required LangVersion.
These polyfills live under the Client project, but Server files in this PR also call
ArgumentNullException.ThrowIfNulland use attributes likeMemberNotNull; non-NET Server builds need equivalent shims compiled into the Server assembly. Also, theextension(ArgumentNullException)block and collection expressions require a sufficiently new C# language version; the LangVersion portion matches the earlier unresolved review concern.#!/bin/bash # Description: Verify whether all non-NET projects using new BCL/nullability APIs # have the required shared shims and a language version that supports this syntax. # Expected: # 1. Any non-NET Server project using ThrowIfNull/MemberNotNull has local/shared polyfill sources. # 2. Projects compiling this file use LangVersion 14.0/latest/preview or otherwise support extension blocks. echo "Project target frameworks and language versions:" fd -e csproj | xargs rg -nP -C2 '<(TargetFrameworks?|LangVersion)>' echo echo "Polyfills.cs locations:" fd -i '^Polyfills\.cs$' echo echo "Server/Client usage of APIs that require non-NET shims:" rg -nP -C2 'ArgumentNullException\.ThrowIfNull|ArgumentNullException\.ThrowIfNullOrEmpty|\bMemberNotNull(When)?\b|\bNotNull\b|\bDoesNotReturn\b' \ src/OpenRiaServices.Server src/OpenRiaServices.Client --glob '*.cs'Also applies to: 179-246
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/OpenRiaServices.Client/Framework/Polyfills.cs` around lines 1 - 38, Polyfills defined in Polyfills.cs (the extension(ArgumentNullException) block implementing ThrowIfNull and ThrowIfNullOrEmpty and the ThrowArgumentNullException helper) must be available to all non-.NET builds that use these APIs (including Server), so either move this file into a shared compilation item (Shared project or Shared folder included by both Client and Server) or add an equivalent Polyfills.cs to the Server assembly; also ensure projects compiling this syntax set LangVersion to a C# version that supports extension blocks/collection expressions (e.g., latest/preview or LangVersion 14.0) and that the Server project references System.Diagnostics.CodeAnalysis types (or includes the necessary attributes like MemberNotNull/NotNull/DoesNotReturn) so the attributes used in Polyfills compile correctly.
🤖 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 17-22: Add a blank line immediately after the "### Server" heading
in Changelog.md (ensure there's an empty line between "### Server" and the
following list item line "* Added Nullability annotations for a few core types
of public API"); optionally also ensure there's a blank line above the "###
Server" heading to match spacing used for other top-level sections for
consistency.
In `@src/OpenRiaServices.Client/Framework/EntityQuery.cs`:
- Around line 188-189: The EntityQuery constructor currently declares parameters
as non-nullable; change its signature to accept a nullable parameter dictionary
so it matches the base constructor and call patterns: modify the
EntityQuery(DomainClient domainClient, string queryName, IDictionary<string,
object?> parameters, bool hasSideEffects, bool isComposable) constructor to use
IDictionary<string, object?>? parameters and pass that nullable value through to
base(..., parameters, ...), ensuring the nullable contract is preserved for
no-parameter queries.
In `@src/OpenRiaServices.Server/Framework/Data/DomainOperationEntry.cs`:
- Line 359: Update the two override method signatures to match the changed
abstract return type: change ReflectionDomainOperationEntry.InvokeAsync and
TestDomainOperationEntry.InvokeAsync to return ValueTask<object?> instead of
ValueTask<object>, and update any return expressions or local variables as
needed to allow returning null (or cast to object?) to satisfy nullable
reference types; keep the method bodies and parameters unchanged aside from the
return type so they correctly override InvokeAsync in DomainOperationEntry.
In `@src/OpenRiaServices.Server/Framework/Data/DomainService.cs`:
- Line 1183: Replace the explicit allocated ValueTask containing a null with the
idiomatic default ValueTask to avoid unnecessary allocation: change the
expression "new ValueTask<object?>((object?)null)" to
"default(ValueTask<object?>)". Locate the return statement that currently
returns the allocated ValueTask<object?> and update it to return
default(ValueTask<object?>) (or simply default) so the method returns a
synchronously-completed null ValueTask without allocation.
- Line 189: Replace the redundant null-check expression when obtaining the
principal by directly assigning IPrincipal? principal = this.ServiceContext.User
(remove the unnecessary "this.ServiceContext != null ? ... : null" ternary),
while leaving the subsequent defensive check "if (principal == null)" in place
to cover the rare obsolete-constructor case; refer to the ServiceContext
property and the DomainServiceContext type to locate where EnsureInitialized
guarantees non-null ServiceContext and to update the assignment in
DomainService.cs.
In `@src/OpenRiaServices.Server/Framework/Data/DomainServiceContext.cs`:
- Around line 40-43: The constructor DomainServiceContext(IServiceProvider
serviceProvider, DomainOperationType operationType) currently uses
serviceProvider.GetService(typeof(IPrincipal))! which can return null and then
assigns a null to the non-nullable User property; update this constructor to
explicitly resolve the principal from serviceProvider, check for null, and throw
a clear ArgumentException/InvalidOperationException (e.g. "IPrincipal service
not registered") if it is null so User is never populated with null;
alternatively, if you prefer nullable semantics, change the User property to
IPrincipal? and adjust consumers such as DomainService.IsAuthorized to handle
nulls—but prefer the explicit null-check + throw in the DomainServiceContext
constructor to maintain the non-nullable contract.
---
Outside diff comments:
In `@src/OpenRiaServices.Server/Framework/Data/DomainOperationEntry.cs`:
- Around line 158-165: The OperationAttribute getter currently calls
InitializeOperationAttribute() but can return a null _operationAttribute (now
Attribute?) when Operation is DomainOperation.None; fix by either making the
property return Attribute? (change OperationAttribute's return type to nullable)
or enforce non-null by throwing a meaningful InvalidOperationException when
_operationAttribute is null (e.g., after InitializeOperationAttribute() check
_operationAttribute and throw with context), and adjust any callers/tests
accordingly; reference the OperationAttribute property, the _operationAttribute
field, and InitializeOperationAttribute (which has the default branch for
DomainOperation.None) when making the change.
In `@src/OpenRiaServices.Server/Framework/Data/DomainService.cs`:
- Around line 576-612: In EvaluateAuthorization, remove the redundant
null-forgiving operators used on the already non-nullable parameters
(attributes!, principal!, authorizationContext!) — locate the two foreach loops
and the calls to attribute.Authorize and drop the trailing '!' from attributes,
principal, and authorizationContext so the code uses the declared non-nullable
variables directly (e.g., change attribute.Authorize(principal!,
authorizationContext!) to attribute.Authorize(principal, authorizationContext)
and similarly remove '!' on attributes in the foreach headers).
---
Duplicate comments:
In `@src/OpenRiaServices.Client/Framework/Polyfills.cs`:
- Around line 1-38: Polyfills defined in Polyfills.cs (the
extension(ArgumentNullException) block implementing ThrowIfNull and
ThrowIfNullOrEmpty and the ThrowArgumentNullException helper) must be available
to all non-.NET builds that use these APIs (including Server), so either move
this file into a shared compilation item (Shared project or Shared folder
included by both Client and Server) or add an equivalent Polyfills.cs to the
Server assembly; also ensure projects compiling this syntax set LangVersion to a
C# version that supports extension blocks/collection expressions (e.g.,
latest/preview or LangVersion 14.0) and that the Server project references
System.Diagnostics.CodeAnalysis types (or includes the necessary attributes like
MemberNotNull/NotNull/DoesNotReturn) so the attributes used in Polyfills compile
correctly.
🪄 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: 51862947-cbee-40cc-a5cf-418fda2903fc
📒 Files selected for processing (15)
Changelog.mdsrc/OpenRiaServices.Client/Framework/Entity.INotifyDataErrorInfo.cssrc/OpenRiaServices.Client/Framework/EntityQuery.cssrc/OpenRiaServices.Client/Framework/EntityRef.cssrc/OpenRiaServices.Client/Framework/InvokeArgs.cssrc/OpenRiaServices.Client/Framework/Polyfills.cssrc/OpenRiaServices.Client/Framework/SubmitResult.cssrc/OpenRiaServices.Hosting.AspNetCore/Framework/AspNetCore/Serialization/DataContractRequestSerializer.cssrc/OpenRiaServices.Server/Framework/Data/AuthorizationAttribute.cssrc/OpenRiaServices.Server/Framework/Data/AuthorizationContext.cssrc/OpenRiaServices.Server/Framework/Data/DomainOperationEntry.cssrc/OpenRiaServices.Server/Framework/Data/DomainService.cssrc/OpenRiaServices.Server/Framework/Data/DomainServiceContext.cssrc/OpenRiaServices.Server/Framework/Data/DomainServiceErrorInfo.cssrc/OpenRiaServices.Server/Framework/Data/ServiceInvokeResult.cs
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 17-18: Consolidate the duplicated changelog bullets by removing
one of the two overlapping lines and leaving a single clear bullet that covers
the change; for example keep a single entry mentioning "Nullability annotations
for EntitySet and EntityCollection" (and optionally combine with "Nullability
annotations for many parts of the most used public API") so the note references
EntitySet, EntityCollection and the public API without duplication.
In `@src/OpenRiaServices.Client/Framework/Entity.cs`:
- Around line 1963-1973: The two extension methods named ExtractState should be
renamed to ApplyState so their public API matches their behavior and XML
inheritdoc; change the method names for the overloads "public static void
ExtractState(this Entity targetEntity, IDictionary<string, object>
entityStateToApply)" and "public static void ExtractState(this Entity
targetEntity, IDictionary<string, object> entityStateToApply, LoadBehavior
loadBehavior)" to "ApplyState" (keeping signatures and bodies unchanged) so they
correctly act as overloads of Entity.ApplyState and preserve the inherited docs
referencing ApplyState.
In `@src/OpenRiaServices.Client/Framework/EntityQuery.cs`:
- Around line 198-199: The XML documentation for the internal property
EntityQuery.Query uses a malformed <inheritdoc> tag; change it to a self-closing
tag (e.g., <inheritdoc />) so the doc comment is well-formed and the inherited
documentation is picked up by tooling—update the XML comment immediately above
the internal new IQueryable<TEntity>? Query property (symbol: EntityQuery.Query)
to use a properly closed <inheritdoc /> tag.
🪄 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: bce0b230-748b-4669-bee1-a28ff5d96f4c
📒 Files selected for processing (5)
Changelog.mdsrc/OpenRiaServices.Client/Framework/Entity.cssrc/OpenRiaServices.Client/Framework/EntityQuery.cssrc/OpenRiaServices.Client/Framework/InvokeOperation.cssrc/OpenRiaServices.Client/Framework/InvokeResult.cs
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/OpenRiaServices.Server/Framework/Data/DomainService.cs (2)
175-209: 🧹 Nitpick | 🔵 TrivialOptional: tighten
resulttyping.
resultis declaredAuthorizationResult?and initialized tonull(line 176), but it is unconditionally assigned at line 203 before being returned at line 209. The nullable annotation is not needed; declaring it non-nullable (and moving it into theusingblock) more accurately reflects the control flow and removes a needlessnullinitialization.♻️ Proposed cleanup
AuthorizationContext? contextTemplate = this.AuthorizationContext; - AuthorizationResult? result = null; + AuthorizationResult result;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/OpenRiaServices.Server/Framework/Data/DomainService.cs` around lines 175 - 209, Declare AuthorizationResult as non-nullable and scope it inside the using block where it is assigned: remove the nullable initialization AuthorizationResult? result = null at the top, and instead declare AuthorizationResult result (non-nullable) within the using (AuthorizationContext context ...) block just before the first assignment from DomainService.EvaluateAuthorization; this eliminates the unnecessary null state and keeps the lifetime of result local to the AuthorizationContext scope while preserving the existing calls to DomainService.EvaluateAuthorization(typeLevelAttributes, principal, context) and the subsequent conditional reassignment.
585-603: 🧹 Nitpick | 🔵 TrivialDrop the redundant null-forgiving operators on non-nullable parameters.
EvaluateAuthorizationdeclaresattributes,principal, andauthorizationContextas non-nullable (line 576), and the caller at lines 203/206 also passes non-nullable values (principalis null-checked at 194,contextcomes from ausinginitializer). The!suppressions at lines 585, 589, and 603 are unnecessary noise and actively misleading — they imply a nullability concern that does not exist in the signature.♻️ Proposed cleanup
- foreach (AuthorizationAttribute attribute in attributes!) + foreach (AuthorizationAttribute attribute in attributes) { if (attribute is RequiresAuthenticationAttribute) { - AuthorizationResult result = attribute.Authorize(principal!, authorizationContext!); + AuthorizationResult result = attribute.Authorize(principal, authorizationContext); @@ foreach (AuthorizationAttribute attribute in attributes) { if (!(attribute is RequiresAuthenticationAttribute)) { - AuthorizationResult result = attribute.Authorize(principal!, authorizationContext!); + AuthorizationResult result = attribute.Authorize(principal, authorizationContext);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/OpenRiaServices.Server/Framework/Data/DomainService.cs` around lines 585 - 603, Remove the redundant null-forgiving operators used on non-nullable parameters in EvaluateAuthorization: drop the "!" suffixes on attributes, principal, and authorizationContext in the foreach loops and Authorize calls so the code uses the declared non-nullable variables directly (refer to the EvaluateAuthorization method and the variables attributes, principal, authorizationContext and the AuthorizationAttribute.Authorize calls).src/OpenRiaServices.Client/Framework/Entity.cs (1)
280-311:⚠️ Potential issue | 🟡 MinorPossible null dereference on
LastSetafter nullable enablement.
LastSetnow returnsEntitySet?, but line 288 dereferences it directly asthis.LastSet.EntityContainer.HasChildChanges(this). Although in practice a tracked entity should be attached, flow analysis cannot prove that here and this should produce aCS8602warning under#nullable enable. Contrast with line 310, which correctly uses?.onEntitySet. Please either add a null check,!suppression with a brief comment explaining the invariant, or switch tothis.LastSet?.EntityContainer.HasChildChanges(this) ?? false.🛡️ Suggested fix
- this._hasChildChanges = this.LastSet.EntityContainer.HasChildChanges(this); + // _trackChanges is only true once attached, so LastSet must be non-null here. + this._hasChildChanges = this.LastSet!.EntityContainer.HasChildChanges(this);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/OpenRiaServices.Client/Framework/Entity.cs` around lines 280 - 311, OnChildUpdate currently dereferences this.LastSet (which is now nullable) when calling LastSet.EntityContainer.HasChildChanges(this), causing a possible null dereference; update the method to guard that call by either (a) checking LastSet != null before accessing EntityContainer, (b) using the null-conditional pattern this.LastSet?.EntityContainer.HasChildChanges(this) ?? false, or (c) using a null-forgiving operator with a short comment asserting the invariant (e.g., // LastSet is non-null for tracked entities) — apply the change around the HasChildChanges call in OnChildUpdate and keep the existing safe null-conditional usage for EntitySet.TrackAsInteresting.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/OpenRiaServices.Client/Framework/Entity.cs`:
- Around line 1967-1979: The Obsolete attribute messages on the two ExtractState
extension methods are grammatically wrong; update both [Obsolete("Use ApplyState
to instead.")] occurrences on the ExtractState(this Entity, IDictionary<string,
object?>) and ExtractState(this Entity, IDictionary<string, object?>,
LoadBehavior) methods to read "Use ApplyState instead." so the compiler warning
shows the corrected wording when consumers see the deprecation message.
- Around line 1961-2015: The XML doc cref tags on the extension wrappers
reference non-nullable IDictionary signatures; update the cref values to match
the actual nullable signatures (IDictionary{string, object?}) for the affected
members: change the cref on the ApplyState(this Entity, IDictionary<string,
object?>, LoadBehavior) wrapper currently pointing to
Entity.ApplyState(IDictionary{string, object}, LoadBehavior) to
Entity.ApplyState(IDictionary{string, object?}, LoadBehavior), change the cref
on the ApplyState(this Entity, IDictionary<string, object?>) wrapper to
Entity.ApplyState(IDictionary{string, object?}), change the cref on the other
ApplyState(this Entity, IDictionary<string, object?>, LoadBehavior) overload to
Entity.ApplyState(IDictionary{string, object?}, LoadBehavior), and change the
cref on UpdateOriginalValues(this Entity, IDictionary<string, object?>) to
Entity.UpdateOriginalValues(IDictionary{string, object?}); ensure the
curly-brace generic cref format matches the other docs exactly.
- Around line 1546-1548: The comment and assumption around
MetaType.GetEntityAction returning non-null are incorrect; update the code in
the block where EntityActionAttribute customUpdate =
MetaType.GetEntityAction(action.Name) is used (and the similar call at line
~1880) to either (1) add a null check and guard usage of customUpdate (e.g.,
only call RaisePropertyChanged(customUpdate.CanInvokePropertyName) when
customUpdate != null, or (2) change the comment to state that
MetaType.GetEntityAction may return null and that a fallback/dummy
EntityActionAttribute may exist from the CustomMethodInvocation setter), so
ensure references to customUpdate.CanInvokePropertyName are only used when
customUpdate is non-null or explicitly handled; update the comment text
accordingly to reflect this invariant change.
In `@src/OpenRiaServices.Client/Framework/EntityQuery.cs`:
- Around line 198-199: Remove the duplicate XML doc inheritdoc tag for the Query
member: in the EntityQuery class where the member documentation contains two
identical lines <inheritdoc cref="EntityQuery.Query" />, delete the redundant
line so only one <inheritdoc cref="EntityQuery.Query" /> remains (or replace the
duplicate with the intended distinct summary if a custom comment was meant).
Ensure the documentation for EntityQuery.Query contains a single, correct XML
comment tag.
In `@src/OpenRiaServices.Client/Framework/ILoadResult.cs`:
- Around line 3-4: The LoadResult<TEntity> constructor must validate
non-nullable inputs: add guard clauses in the LoadResult(EntityQuery<TEntity>
query, LoadBehavior loadBehavior, IEnumerable<TEntity> entities,
IReadOnlyCollection<Entity> allEntities, int totalEntityCount) constructor to
throw ArgumentNullException for both allEntities and entities before using them;
then perform the existing conversion of entities into _loadedEntites (using
Data.ReadOnlyObservableLoaderCollection<TEntity> or ToList()) and assign
EntityQuery, LoadBehavior, AllEntities and TotalEntityCount as before so
Entities/AllEntities can never be null at runtime.
In `@src/OpenRiaServices.Client/Framework/Polyfills.cs`:
- Around line 24-31: The current polyfill defines ThrowIfNullOrEmpty as an
extension on ArgumentNullException; move it to an extension on ArgumentException
(i.e., declare the method in an extension(ArgumentException) block) so callsites
using ArgumentException.ThrowIfNullOrEmpty(...) compile like the BCL; update the
implementation of ThrowIfNullOrEmpty(string? argument, string? paramName = null)
to throw ArgumentNullException when argument is null and ArgumentException when
argument is empty (preserving CallerArgumentExpression usage for paramName), and
keep the aggressive inlining and nullable annotations unchanged.
In `@src/Test/OpenRiaservices.EndToEnd.Wcf.Test/Data/OperationTests.cs`:
- Around line 380-382: Capture the change set once and reuse it instead of
calling cities.EntityContainer.GetChanges() twice: call
cities.EntityContainer.GetChanges() into a local variable (e.g., changeSet) and
pass that same changeSet to the SubmitOperation constructor (where
submitOperation is created) and to the new SubmitResult(...) used in
AssertExceptionIsCorrectlyRaisedOnSyncContext; this ensures submitTaskSource,
SubmitOperation, and SubmitResult all reference the exact same EntityChangeSet
instance.
---
Outside diff comments:
In `@src/OpenRiaServices.Client/Framework/Entity.cs`:
- Around line 280-311: OnChildUpdate currently dereferences this.LastSet (which
is now nullable) when calling LastSet.EntityContainer.HasChildChanges(this),
causing a possible null dereference; update the method to guard that call by
either (a) checking LastSet != null before accessing EntityContainer, (b) using
the null-conditional pattern this.LastSet?.EntityContainer.HasChildChanges(this)
?? false, or (c) using a null-forgiving operator with a short comment asserting
the invariant (e.g., // LastSet is non-null for tracked entities) — apply the
change around the HasChildChanges call in OnChildUpdate and keep the existing
safe null-conditional usage for EntitySet.TrackAsInteresting.
In `@src/OpenRiaServices.Server/Framework/Data/DomainService.cs`:
- Around line 175-209: Declare AuthorizationResult as non-nullable and scope it
inside the using block where it is assigned: remove the nullable initialization
AuthorizationResult? result = null at the top, and instead declare
AuthorizationResult result (non-nullable) within the using (AuthorizationContext
context ...) block just before the first assignment from
DomainService.EvaluateAuthorization; this eliminates the unnecessary null state
and keeps the lifetime of result local to the AuthorizationContext scope while
preserving the existing calls to
DomainService.EvaluateAuthorization(typeLevelAttributes, principal, context) and
the subsequent conditional reassignment.
- Around line 585-603: Remove the redundant null-forgiving operators used on
non-nullable parameters in EvaluateAuthorization: drop the "!" suffixes on
attributes, principal, and authorizationContext in the foreach loops and
Authorize calls so the code uses the declared non-nullable variables directly
(refer to the EvaluateAuthorization method and the variables attributes,
principal, authorizationContext and the AuthorizationAttribute.Authorize calls).
🪄 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: 6e7b0fb5-935b-429e-8bb7-7e823a54177e
📒 Files selected for processing (15)
src/OpenRiaServices.Client/Framework/ComplexObject.INotifyDataErrorInfo.cssrc/OpenRiaServices.Client/Framework/ComplexObject.cssrc/OpenRiaServices.Client/Framework/Entity.cssrc/OpenRiaServices.Client/Framework/EntityAction.cssrc/OpenRiaServices.Client/Framework/EntityCollection.cssrc/OpenRiaServices.Client/Framework/EntityCollectionChangedEventArgs.cssrc/OpenRiaServices.Client/Framework/EntityQuery.cssrc/OpenRiaServices.Client/Framework/EntitySet.cssrc/OpenRiaServices.Client/Framework/ILoadResult.cssrc/OpenRiaServices.Client/Framework/ObjectStateUtility.cssrc/OpenRiaServices.Client/Framework/Polyfills.cssrc/OpenRiaServices.Client/Framework/SubmitResult.cssrc/OpenRiaServices.Client/Framework/ValidationUtilities.cssrc/OpenRiaServices.Server/Framework/Data/DomainService.cssrc/Test/OpenRiaservices.EndToEnd.Wcf.Test/Data/OperationTests.cs
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Daniel-Svensson
force-pushed
the
nullable2
branch
from
a70bc43 to
b536e85
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
src/OpenRiaServices.Server/Framework/Data/DomainOperationEntry.cs (1)
158-165:⚠️ Potential issue | 🟡 Minor
OperationAttributepublic return type should be nullable to match the backing field.
_operationAttributeis declaredAttribute?and thedefault:branch ofInitializeOperationAttribute(theDomainOperation.None/pre-classification path) intentionally leaves it unset. The property getter on Line 163 therefore can returnnull, but its declared return type is the non-nullableAttribute— either a!silencer or a declaredAttribute?would reflect reality more accurately.Since this is part of the public API, changing the signature is technically a nullable-contract change; if you'd prefer to keep the existing non-null contract, consider either (a) asserting non-null via
_operationAttribute!with a debug guard, or (b) marking the propertyAttribute?now that nullable is enabled on this file.Also applies to: 298-301
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/OpenRiaServices.Server/Framework/Data/DomainOperationEntry.cs` around lines 158 - 165, The public getter OperationAttribute can return null because it reads the backing field _operationAttribute which is declared Attribute? and InitializeOperationAttribute may leave it unset; update the public contract by changing the OperationAttribute property's return type to Attribute? (nullable) so it accurately reflects possible nulls, and do the same for the other public property in this file that returns an Attribute and reads the same backing field; alternatively, if you must preserve a non-null contract, explicitly assert non-null with _operationAttribute! and add a defensive debug guard or exception in InitializeOperationAttribute to guarantee non-null before returning.src/OpenRiaServices.Server/Framework/Data/DomainService.cs (2)
585-599: 🧹 Nitpick | 🔵 TrivialRedundant null-forgiving on
authorizationContext.
EvaluateAuthorization'sauthorizationContextparameter is declared as non-nullableAuthorizationContext, soauthorizationContext!at lines 585 and 599 adds no value. Drop the!.♻️ Proposed cleanup
- AuthorizationResult result = attribute.Authorize(principal, authorizationContext!); + AuthorizationResult result = attribute.Authorize(principal, authorizationContext);(apply in both locations)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/OpenRiaServices.Server/Framework/Data/DomainService.cs` around lines 585 - 599, In EvaluateAuthorization, remove the unnecessary null-forgiving operator from the AuthorizationContext parameter when calling AuthorizationAttribute.Authorize: replace calls using authorizationContext! with authorizationContext (both occurrences where AuthorizationResult result = attribute.Authorize(principal, authorizationContext!) is invoked) so the non-nullable AuthorizationContext parameter is used directly in the AuthorizationAttribute.Authorize calls.
733-759: 🧹 Nitpick | 🔵 TrivialConsolidate the repeated
this.ChangeSet!.pattern.
ChangeSetis legitimately non-null insideAuthorizeChangeSet,ValidateChangeSetAsync,ExecuteChangeSetAsync,PersistChangeSetAsyncInternal,InvokeCudOperationsAsync,InvokeCustomOperations, andResolveOperationsbecauseSubmitAsyncassigns it before any of them runs. Rather than sprinkling!eight times across the file, consider a private helper (e.g.,private ChangeSet CurrentChangeSet => _changeSet ?? throw new InvalidOperationException(...);) or a[MemberNotNull(nameof(_changeSet))]helper analogous toEnsureInitialized. This keeps the nullable contract explicit and centralised, and also guards against a developer inadvertently invoking these methods outside the submit pipeline.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/OpenRiaServices.Server/Framework/Data/DomainService.cs` around lines 733 - 759, Consolidate the repeated nullable-force usage of this.ChangeSet! by adding a centralized non-null accessor and using it across methods: introduce a private property or helper (e.g., CurrentChangeSet or EnsureChangeSetInitialized with [MemberNotNull(nameof(_changeSet))]) that throws InvalidOperationException if _changeSet is null, and replace all occurrences of this.ChangeSet! in AuthorizeChangeSet, ValidateChangeSetAsync, ExecuteChangeSetAsync, PersistChangeSetAsyncInternal, InvokeCudOperationsAsync, InvokeCustomOperations, and ResolveOperations with the new accessor; ensure SubmitAsync still assigns _changeSet before calling these methods and update any existing EnsureInitialized usage to match the new pattern.src/OpenRiaServices.Client/Framework/Entity.INotifyDataErrorInfo.cs (1)
51-62:⚠️ Potential issue | 🟡 MinorFilter logic now diverges from
ComplexObject.INotifyDataErrorInfo.GetErrors.Entity's null/empty branch was changed to
e.MemberNames.Any(string.IsNullOrEmpty), but the parallel implementation insrc/OpenRiaServices.Client/Framework/ComplexObject.INotifyDataErrorInfo.cs(line 55) still usese.MemberNames.Contains(propertyName). For identical validation error shapes, the two types will now return different sets (e.g., a result whoseMemberNamescontains""would be returned byEntity.GetErrors(null)but not byComplexObject.GetErrors(null), and vice-versa). Either updateComplexObjectto use the same predicate, or revert this one for consistency.🔧 Suggested change to ComplexObject.INotifyDataErrorInfo.cs to align
- results = this.ValidationResultCollection.Where(e => !e.MemberNames.Any() || e.MemberNames.Contains(propertyName)); + results = this.ValidationResultCollection.Where(e => !e.MemberNames.Any() || e.MemberNames.Any(string.IsNullOrEmpty));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/OpenRiaServices.Client/Framework/Entity.INotifyDataErrorInfo.cs` around lines 51 - 62, The Entity INofityDataErrorInfo GetErrors implementation diverges from ComplexObject.INotifyDataErrorInfo: Entity uses e.MemberNames.Any(string.IsNullOrEmpty) for the null/empty propertyName branch while ComplexObject still uses e.MemberNames.Contains(propertyName); make them consistent by updating the ComplexObject.INotifyDataErrorInfo.GetErrors (or revert the Entity logic) so both use the same predicate for the empty propertyName case—specifically replace the ComplexObject null/empty branch with the same check (e.MemberNames.Any() || e.MemberNames.Any(string.IsNullOrEmpty)) or change Entity back to the original Contains-based behavior so GetErrors returns identical sets across both types.
♻️ Duplicate comments (1)
src/Test/OpenRiaservices.EndToEnd.Wcf.Test/Data/OperationTests.cs (1)
380-382: 🧹 Nitpick | 🔵 TrivialConsider capturing the changeset once here too, for consistency.
The same pattern was applied at Lines 357–358 (capture
changeSetonce, reuse). Here at Lines 381–382,cities.EntityContainer.GetChanges()is still called twice, producing two distinctEntityChangeSetinstances passed to theSubmitOperationand to the assertedSubmitResult. Aligning with the earlier fix avoids any future confusion about whether the operation and its completion result reference the same change set.♻️ Suggested tweak
var submitTaskSource = new TaskCompletionSource<SubmitResult>(TaskCreationOptions.RunContinuationsAsynchronously); - var submitOperation = new SubmitOperation(cities.EntityContainer.GetChanges(), soCallback, null, submitTaskSource.Task, null); - await AssertExceptionIsCorrectlyRaisedOnSyncContext(submitOperation, submitTaskSource, new SubmitResult(cities.EntityContainer.GetChanges())); + var submitChangeSet = cities.EntityContainer.GetChanges(); + var submitOperation = new SubmitOperation(submitChangeSet, soCallback, null, submitTaskSource.Task, null); + await AssertExceptionIsCorrectlyRaisedOnSyncContext(submitOperation, submitTaskSource, new SubmitResult(submitChangeSet));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Test/OpenRiaservices.EndToEnd.Wcf.Test/Data/OperationTests.cs` around lines 380 - 382, Capture the EntityChangeSet once into a local variable (e.g., changeSet) and reuse it for both constructing the SubmitOperation and the expected SubmitResult so both reference the identical change set; specifically, replace the two calls to cities.EntityContainer.GetChanges() with a single var changeSet = cities.EntityContainer.GetChanges() and pass changeSet into the SubmitOperation constructor (submitOperation) and into new SubmitResult.
🤖 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 30: Update the changelog bullet that currently reads "Fix MSTEST****:
varius test related" by replacing the placeholder "MSTEST****" with the actual
analyzer/rule ID(s) being fixed (e.g., MSTESTxxxx or the appropriate rule
identifiers) and correct the typo "varius" to "various", so the line becomes
something like "Fix MSTEST1234: various test related" (use the real rule IDs and
keep the phrasing consistent with other bullets).
In `@src/OpenRiaServices.Client/Framework/DomainContext.cs`:
- Around line 900-909: The null-forgiving operator on results.ReturnValue is
misleading because the target cast is already TValue?; remove the unnecessary
'!' so the code returns new InvokeResult<TValue>((TValue?)results.ReturnValue);
if the real contract guarantees a non-null return when
results.ValidationErrors.Count == 0 and TValue must be non-null, instead change
the cast to the non-nullable form and assert non-null explicitly: new
InvokeResult<TValue>((TValue)results.ReturnValue!) — update the code in the
InvokeResult<TValue> return site (and related call sites in
DomainContext.InvokeOperation) to reflect the chosen contract.
In `@src/OpenRiaServices.Client/Framework/Entity.cs`:
- Around line 1471-1473: Replace the use of CultureInfo.InvariantCulture when
formatting the localized resource message in the Entity action lookup: in the
code path that calls MetaType.GetEntityAction(actionName) and throws new
InvalidOperationException(string.Format(CultureInfo.InvariantCulture,
Resource.Entity_NoEntityActionWithName, actionName)), switch to using
CultureInfo.CurrentCulture so the Resource.Entity_NoEntityActionWithName message
is formatted with the current UI culture (keep the same exception type and
message parameters, only change the CultureInfo argument).
In `@src/Test/OpenRiaservices.EndToEnd.Wcf.Test/Data/UpdateTests.cs`:
- Line 2315: Add the same explanatory trailing comment to the [Retry(3)]
attribute in UpdateTests so its rationale matches the other occurrences; locate
the [Retry(3)] attribute in UpdateTests (the flaky test annotation) and append
the comment used elsewhere in this file (e.g. // Known flaky test, retry until
it is rewritten) so intent is discoverable and consistent.
---
Outside diff comments:
In `@src/OpenRiaServices.Client/Framework/Entity.INotifyDataErrorInfo.cs`:
- Around line 51-62: The Entity INofityDataErrorInfo GetErrors implementation
diverges from ComplexObject.INotifyDataErrorInfo: Entity uses
e.MemberNames.Any(string.IsNullOrEmpty) for the null/empty propertyName branch
while ComplexObject still uses e.MemberNames.Contains(propertyName); make them
consistent by updating the ComplexObject.INotifyDataErrorInfo.GetErrors (or
revert the Entity logic) so both use the same predicate for the empty
propertyName case—specifically replace the ComplexObject null/empty branch with
the same check (e.MemberNames.Any() || e.MemberNames.Any(string.IsNullOrEmpty))
or change Entity back to the original Contains-based behavior so GetErrors
returns identical sets across both types.
In `@src/OpenRiaServices.Server/Framework/Data/DomainOperationEntry.cs`:
- Around line 158-165: The public getter OperationAttribute can return null
because it reads the backing field _operationAttribute which is declared
Attribute? and InitializeOperationAttribute may leave it unset; update the
public contract by changing the OperationAttribute property's return type to
Attribute? (nullable) so it accurately reflects possible nulls, and do the same
for the other public property in this file that returns an Attribute and reads
the same backing field; alternatively, if you must preserve a non-null contract,
explicitly assert non-null with _operationAttribute! and add a defensive debug
guard or exception in InitializeOperationAttribute to guarantee non-null before
returning.
In `@src/OpenRiaServices.Server/Framework/Data/DomainService.cs`:
- Around line 585-599: In EvaluateAuthorization, remove the unnecessary
null-forgiving operator from the AuthorizationContext parameter when calling
AuthorizationAttribute.Authorize: replace calls using authorizationContext! with
authorizationContext (both occurrences where AuthorizationResult result =
attribute.Authorize(principal, authorizationContext!) is invoked) so the
non-nullable AuthorizationContext parameter is used directly in the
AuthorizationAttribute.Authorize calls.
- Around line 733-759: Consolidate the repeated nullable-force usage of
this.ChangeSet! by adding a centralized non-null accessor and using it across
methods: introduce a private property or helper (e.g., CurrentChangeSet or
EnsureChangeSetInitialized with [MemberNotNull(nameof(_changeSet))]) that throws
InvalidOperationException if _changeSet is null, and replace all occurrences of
this.ChangeSet! in AuthorizeChangeSet, ValidateChangeSetAsync,
ExecuteChangeSetAsync, PersistChangeSetAsyncInternal, InvokeCudOperationsAsync,
InvokeCustomOperations, and ResolveOperations with the new accessor; ensure
SubmitAsync still assigns _changeSet before calling these methods and update any
existing EnsureInitialized usage to match the new pattern.
---
Duplicate comments:
In `@src/Test/OpenRiaservices.EndToEnd.Wcf.Test/Data/OperationTests.cs`:
- Around line 380-382: Capture the EntityChangeSet once into a local variable
(e.g., changeSet) and reuse it for both constructing the SubmitOperation and the
expected SubmitResult so both reference the identical change set; specifically,
replace the two calls to cities.EntityContainer.GetChanges() with a single var
changeSet = cities.EntityContainer.GetChanges() and pass changeSet into the
SubmitOperation constructor (submitOperation) and into new SubmitResult.
🪄 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: c2786b3d-9e41-46b4-89f0-75f6d19ad6e4
📒 Files selected for processing (44)
Changelog.mdsrc/.editorconfigsrc/OpenRiaServices.Client/Framework/ComplexObject.INotifyDataErrorInfo.cssrc/OpenRiaServices.Client/Framework/ComplexObject.cssrc/OpenRiaServices.Client/Framework/DomainContext.cssrc/OpenRiaServices.Client/Framework/DomainIdentifierAttribute.cssrc/OpenRiaServices.Client/Framework/Entity.INotifyDataErrorInfo.cssrc/OpenRiaServices.Client/Framework/Entity.cssrc/OpenRiaServices.Client/Framework/EntityAction.cssrc/OpenRiaServices.Client/Framework/EntityChangeSet.cssrc/OpenRiaServices.Client/Framework/EntityCollection.cssrc/OpenRiaServices.Client/Framework/EntityCollectionChangedEventArgs.cssrc/OpenRiaServices.Client/Framework/EntityQuery.cssrc/OpenRiaServices.Client/Framework/EntityRef.cssrc/OpenRiaServices.Client/Framework/EntitySet.cssrc/OpenRiaServices.Client/Framework/IDomainClientFactory.cssrc/OpenRiaServices.Client/Framework/IEntityCollection.cssrc/OpenRiaServices.Client/Framework/IEntityCollection1.cs`src/OpenRiaServices.Client/Framework/ILoadResult.cssrc/OpenRiaServices.Client/Framework/InvokeArgs.cssrc/OpenRiaServices.Client/Framework/InvokeCompletedResult.cssrc/OpenRiaServices.Client/Framework/InvokeOperation.cssrc/OpenRiaServices.Client/Framework/InvokeResult.cssrc/OpenRiaServices.Client/Framework/LoadOperation.cssrc/OpenRiaServices.Client/Framework/LoadResult.cssrc/OpenRiaServices.Client/Framework/ObjectStateUtility.cssrc/OpenRiaServices.Client/Framework/OperationBase.cssrc/OpenRiaServices.Client/Framework/Polyfills.cssrc/OpenRiaServices.Client/Framework/SubmitOperation.cssrc/OpenRiaServices.Client/Framework/SubmitResult.cssrc/OpenRiaServices.Client/Framework/ValidationResultCollection.cssrc/OpenRiaServices.Client/Framework/ValidationUtilities.cssrc/OpenRiaServices.Hosting.AspNetCore/Framework/AspNetCore/Serialization/DataContractRequestSerializer.cssrc/OpenRiaServices.Server/Framework/Data/AuthorizationAttribute.cssrc/OpenRiaServices.Server/Framework/Data/AuthorizationContext.cssrc/OpenRiaServices.Server/Framework/Data/DomainOperationEntry.cssrc/OpenRiaServices.Server/Framework/Data/DomainOperationParameter.cssrc/OpenRiaServices.Server/Framework/Data/DomainService.cssrc/OpenRiaServices.Server/Framework/Data/DomainServiceContext.cssrc/OpenRiaServices.Server/Framework/Data/DomainServiceErrorInfo.cssrc/OpenRiaServices.Server/Framework/Data/IDomainServiceFactory.cssrc/OpenRiaServices.Server/Framework/Data/ServiceInvokeResult.cssrc/Test/OpenRiaservices.EndToEnd.Wcf.Test/Data/OperationTests.cssrc/Test/OpenRiaservices.EndToEnd.Wcf.Test/Data/UpdateTests.cs
|
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
2 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.
add more nullable annotations
Summary by CodeRabbit
Chores
New Features
Tests