-
Notifications
You must be signed in to change notification settings - Fork 186
Create ODataError for non-success responses #623
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
17 commits
Select commit
Hold shift + click to select a range
75833e2
Create ODataError in non-success responses
KenitoInc bf21517
Update public api
KenitoInc 3986f15
Update public api
KenitoInc 4b6dee9
Add E2E tests
KenitoInc 193e165
Add more E2E tests
KenitoInc 9039467
Update public api
KenitoInc 5ecaa86
Fix failing tests
KenitoInc 09fa9e6
Fix namespaces
KenitoInc a7703aa
Code clean up
KenitoInc 35c3957
Revert TestODataController
KenitoInc 444d5f5
Revert TestODataController
KenitoInc 96c2e9f
Dispose IDisposable objects
KenitoInc 73f2192
Improve error handling
KenitoInc f90b5fd
Improve validations
KenitoInc 9146f7a
Fix typo
KenitoInc 344b5f3
Revert change
KenitoInc 3dda6d0
formatting changes
KenitoInc File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
252 changes: 252 additions & 0 deletions
252
src/Microsoft.AspNetCore.OData/Microsoft.AspNetCore.OData.xml
Large diffs are not rendered by default.
Oops, something went wrong.
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
74 changes: 74 additions & 0 deletions
74
src/Microsoft.AspNetCore.OData/Results/BadRequestODataResult.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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,74 @@ | ||
| //----------------------------------------------------------------------------- | ||
| // <copyright file="BadRequestODataResult.cs" company=".NET Foundation"> | ||
| // Copyright (c) .NET Foundation and Contributors. All rights reserved. | ||
| // See License.txt in the project root for license information. | ||
| // </copyright> | ||
| //------------------------------------------------------------------------------ | ||
|
|
||
| using System.Threading.Tasks; | ||
| using Microsoft.AspNetCore.Http; | ||
| using Microsoft.AspNetCore.Mvc; | ||
| using Microsoft.OData; | ||
| using ErrorUtils = Microsoft.AspNetCore.OData.Error; | ||
|
|
||
| namespace Microsoft.AspNetCore.OData.Results | ||
| { | ||
| /// <summary> | ||
| /// Represents a result that when executed will produce a Bad Request (400) response. | ||
| /// </summary> | ||
| /// <remarks>This result creates an <see cref="ODataError"/> with status code: 400.</remarks> | ||
| public class BadRequestODataResult : BadRequestResult, IODataErrorResult | ||
| { | ||
| private const string errorCode = "400"; | ||
|
|
||
| /// <summary> | ||
| /// OData error. | ||
| /// </summary> | ||
| public ODataError Error { get; } | ||
|
|
||
| /// <summary> | ||
| /// Initializes a new instance of the class. | ||
| /// </summary> | ||
| /// <param name="message">Error message.</param> | ||
| public BadRequestODataResult(string message) | ||
| { | ||
| if (string.IsNullOrEmpty(message)) | ||
| { | ||
| throw ErrorUtils.ArgumentNullOrEmpty(nameof(message)); | ||
| } | ||
|
|
||
| Error = new ODataError | ||
| { | ||
| Message = message, | ||
| ErrorCode = errorCode | ||
| }; | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Initializes a new instance of the class. | ||
| /// </summary> | ||
| /// <param name="odataError">An <see cref="ODataError"/> object.</param> | ||
| public BadRequestODataResult(ODataError odataError) | ||
| { | ||
| if (odataError == null) | ||
| { | ||
| throw ErrorUtils.ArgumentNull(nameof(odataError)); | ||
| } | ||
|
|
||
| ErrorUtils.ValidateErrorCode(errorCode, odataError); | ||
|
|
||
| Error = odataError; | ||
|
corranrogue9 marked this conversation as resolved.
|
||
| } | ||
|
|
||
| /// <inheritdoc/> | ||
| public async override Task ExecuteResultAsync(ActionContext context) | ||
| { | ||
|
corranrogue9 marked this conversation as resolved.
|
||
| ObjectResult objectResult = new ObjectResult(Error) | ||
| { | ||
| StatusCode = StatusCodes.Status400BadRequest | ||
| }; | ||
|
|
||
| await objectResult.ExecuteResultAsync(context).ConfigureAwait(false); | ||
| } | ||
| } | ||
| } | ||
74 changes: 74 additions & 0 deletions
74
src/Microsoft.AspNetCore.OData/Results/ConflictODataResult.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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,74 @@ | ||
| //----------------------------------------------------------------------------- | ||
| // <copyright file="ConflictODataResult.cs" company=".NET Foundation"> | ||
| // Copyright (c) .NET Foundation and Contributors. All rights reserved. | ||
| // See License.txt in the project root for license information. | ||
| // </copyright> | ||
| //------------------------------------------------------------------------------ | ||
|
|
||
| using System.Threading.Tasks; | ||
| using Microsoft.AspNetCore.Http; | ||
| using Microsoft.AspNetCore.Mvc; | ||
| using Microsoft.OData; | ||
| using ErrorUtils = Microsoft.AspNetCore.OData.Error; | ||
|
|
||
| namespace Microsoft.AspNetCore.OData.Results | ||
| { | ||
| /// <summary> | ||
| /// Represents a result that when executed will produce a Conflict (409) response. | ||
| /// </summary> | ||
| /// <remarks>This result creates an <see cref="ODataError"/> with status code: 409.</remarks> | ||
| public class ConflictODataResult : ConflictResult, IODataErrorResult | ||
| { | ||
| private const string errorCode = "409"; | ||
|
|
||
| /// <summary> | ||
| /// OData error. | ||
| /// </summary> | ||
| public ODataError Error { get; } | ||
|
|
||
| /// <summary> | ||
| /// Initializes a new instance of the class. | ||
| /// </summary> | ||
| /// <param name="message">Error message.</param> | ||
| public ConflictODataResult(string message) | ||
| { | ||
| if (string.IsNullOrEmpty(message)) | ||
| { | ||
| throw ErrorUtils.ArgumentNullOrEmpty(nameof(message)); | ||
| } | ||
|
|
||
| Error = new ODataError | ||
| { | ||
| Message = message, | ||
| ErrorCode = errorCode | ||
| }; | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Initializes a new instance of the class. | ||
| /// </summary> | ||
| /// <param name="odataError">An <see cref="ODataError"/> object.</param> | ||
| public ConflictODataResult(ODataError odataError) | ||
| { | ||
| if (odataError == null) | ||
| { | ||
| throw ErrorUtils.ArgumentNull(nameof(odataError)); | ||
| } | ||
|
|
||
| ErrorUtils.ValidateErrorCode(errorCode, odataError); | ||
|
|
||
| Error = odataError; | ||
| } | ||
|
|
||
| /// <inheritdoc/> | ||
| public async override Task ExecuteResultAsync(ActionContext context) | ||
| { | ||
| ObjectResult objectResult = new ObjectResult(Error) | ||
| { | ||
| StatusCode = StatusCodes.Status409Conflict | ||
| }; | ||
|
|
||
| await objectResult.ExecuteResultAsync(context).ConfigureAwait(false); | ||
| } | ||
| } | ||
| } |
24 changes: 24 additions & 0 deletions
24
src/Microsoft.AspNetCore.OData/Results/IODataErrorResult.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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,24 @@ | ||
| //----------------------------------------------------------------------------- | ||
| // <copyright file="IODataErrorResult.cs" company=".NET Foundation"> | ||
| // Copyright (c) .NET Foundation and Contributors. All rights reserved. | ||
| // See License.txt in the project root for license information. | ||
| // </copyright> | ||
| //------------------------------------------------------------------------------ | ||
|
|
||
| using Microsoft.OData; | ||
|
|
||
| namespace Microsoft.AspNetCore.OData.Results | ||
| { | ||
| /// <summary> | ||
| /// Provide the interface for the details of a given OData error result. | ||
| /// </summary> | ||
| public interface IODataErrorResult | ||
| { | ||
| /// <summary> | ||
| /// OData error. | ||
| /// </summary> | ||
| #pragma warning disable CA1716 // Identifiers should not match keywords | ||
| ODataError Error { get; } | ||
| #pragma warning restore CA1716 // Identifiers should not match keywords | ||
| } | ||
| } |
74 changes: 74 additions & 0 deletions
74
src/Microsoft.AspNetCore.OData/Results/NotFoundODataResult.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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,74 @@ | ||
| //----------------------------------------------------------------------------- | ||
| // <copyright file="NotFoundODataResult.cs" company=".NET Foundation"> | ||
| // Copyright (c) .NET Foundation and Contributors. All rights reserved. | ||
| // See License.txt in the project root for license information. | ||
| // </copyright> | ||
| //------------------------------------------------------------------------------ | ||
|
|
||
| using System.Threading.Tasks; | ||
| using Microsoft.AspNetCore.Http; | ||
| using Microsoft.AspNetCore.Mvc; | ||
| using Microsoft.OData; | ||
| using ErrorUtils = Microsoft.AspNetCore.OData.Error; | ||
|
|
||
| namespace Microsoft.AspNetCore.OData.Results | ||
| { | ||
| /// <summary> | ||
| /// Represents a result that when executed will produce a Not Found (404) response. | ||
| /// </summary> | ||
| /// <remarks>This result creates an <see cref="ODataError"/> with status code: 404.</remarks> | ||
| public class NotFoundODataResult : NotFoundResult, IODataErrorResult | ||
| { | ||
| private const string errorCode = "404"; | ||
|
|
||
| /// <summary> | ||
| /// OData error. | ||
| /// </summary> | ||
| public ODataError Error { get; } | ||
|
|
||
| /// <summary> | ||
| /// Initializes a new instance of the class. | ||
| /// </summary> | ||
| /// <param name="message">Error message.</param> | ||
| public NotFoundODataResult(string message) | ||
| { | ||
| if (string.IsNullOrEmpty(message)) | ||
| { | ||
| throw ErrorUtils.ArgumentNullOrEmpty(nameof(message)); | ||
| } | ||
|
|
||
| Error = new ODataError | ||
| { | ||
| Message = message, | ||
| ErrorCode = errorCode | ||
| }; | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Initializes a new instance of the class. | ||
| /// </summary> | ||
| /// <param name="odataError">An <see cref="ODataError"/> object.</param> | ||
| public NotFoundODataResult(ODataError odataError) | ||
| { | ||
| if (odataError == null) | ||
| { | ||
| throw ErrorUtils.ArgumentNull(nameof(odataError)); | ||
| } | ||
|
|
||
| ErrorUtils.ValidateErrorCode(errorCode, odataError); | ||
|
|
||
| Error = odataError; | ||
| } | ||
|
|
||
| /// <inheritdoc/> | ||
| public async override Task ExecuteResultAsync(ActionContext context) | ||
| { | ||
| ObjectResult objectResult = new ObjectResult(Error) | ||
| { | ||
| StatusCode = StatusCodes.Status404NotFound | ||
| }; | ||
|
|
||
| await objectResult.ExecuteResultAsync(context).ConfigureAwait(false); | ||
| } | ||
| } | ||
| } |
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
folks, this line looks like creating additional expectation that error.ErrorCode must be the same as HTTP status code, while according to the OData spec, it can be " language-independent string" (see below for more details)
9.4 Error Response Body
The representation of an error response body is format-specific. It consists at least of the following information:
· code: required non-null, non-empty, language-independent string. Its value is a service-defined error code. This code serves as a sub-status for the HTTP error code specified in the response.
· message: required non-null, non-empty, language-dependent, human-readable string describing the error. The Content-Language header MUST contain the language code from [RFC5646] corresponding to the language in which the value for message is written.
· target: optional nullable, potentially empty string indicating the target of the error, for example, the name of the property in error.
· details: optional, potentially empty collection of structured instances with code, message, and target following the rules above.
· innererror: optional structured instance with service-defined content.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it is essentially a breaking change! it generates 500 internal server error for this code (suddenly the code that was valid before becomes invalid/not supported)
return NotFound(
new OData.ODataError()
{
ErrorCode = "ErrorEntityNotFound",
Message = "The specified entity was not found."
});
and may prevent clients from upgrading to the latest version.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@garegina You issue is valid. Fixed by #700