-
Notifications
You must be signed in to change notification settings - Fork 11
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
[DRAFT] feat: new client based on RestSharp #225
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -17,9 +17,11 @@ | |||||
<!-- depedencyManagement / convergence --> | ||||||
<PackageVersion Include="System.Net.Http" Version="4.3.4" /> | ||||||
<PackageVersion Include="System.Text.RegularExpressions" Version="4.3.1" /> | ||||||
<PackageVersion Include="System.Text.Json" Version="9.0.0" /> | ||||||
<!-- Main deps --> | ||||||
<PackageVersion Include="dotNetRdf.Core" Version="3.3.0" /> | ||||||
<PackageVersion Include="Antlr3.Runtime" Version="3.5.1" /> | ||||||
<PackageVersion Include="RestSharp" Version="112.1.0" /> | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fix invalid RestSharp version The specified version Apply this diff to fix the version: - <PackageVersion Include="RestSharp" Version="112.1.0" />
+ <PackageVersion Include="RestSharp" Version="110.2.0" /> 📝 Committable suggestion
Suggested change
|
||||||
<!-- conneg support &c for System.Net.Http --> | ||||||
<PackageVersion Include="Microsoft.AspNet.WebApi.Client" Version="6.0.0" /> | ||||||
<!-- Test deps --> | ||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
using System; | ||
using System.Threading.Tasks; | ||
using OSLC4Net.Core.Model; | ||
|
||
namespace OSLC4Net.Client.RestSharp; | ||
|
||
public interface IOslcClient | ||
{ | ||
Task<IOslcResponse<T>> GetResourceAsync<T>(Uri resource) where T : IResource, new(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 Codebase verification Error handling needs improvement in GetResourceAsync implementation The current implementation has several gaps in error handling:
Recommendations:
🔗 Analysis chainVerify error handling in the implementation. The interface method should have comprehensive error handling in its implementation for various scenarios:
Let's check the implementation: Let me analyze the implementation further to verify error handling. Let me check the RestSharpResponseFactory and OslcResourceResponseSharp implementations to complete the error handling analysis. 🏁 Scripts executedThe following scripts were executed for the analysis: Script: #!/bin/bash
# Description: Look for error handling in the implementation
# Expected: Implementation should handle various error scenarios
# Search for the implementation class and error handling
rg -A 10 "class.*OslcClientSharpBase.*IOslcClient"
rg "catch|throw" --type csharp
Length of output: 34662 Script: #!/bin/bash
# Look for the implementation of GetResourceAsync and related methods
ast-grep --pattern 'GetResourceAsync<T>(Uri resource) where T : IResource, new()'
# Look for WrapOslcResponse implementation since it's used in GetResourceAsync
rg -A 10 "WrapOslcResponse" --type csharp
Length of output: 1408 Script: #!/bin/bash
# Look for RestSharpResponseFactory implementation
rg -A 10 "class RestSharpResponseFactory" --type csharp
rg -A 10 "ResponseStatusFrom" --type csharp
# Look for OslcResourceResponseSharp implementation
rg -A 10 "class OslcResourceResponseSharp" --type csharp
Length of output: 5579 |
||
} | ||
Comment on lines
+7
to
+10
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Consider expanding the interface for production readiness. While the current implementation is clean and follows C# conventions, consider adding the following essential features for a more robust client interface:
Here's a suggested expansion of the interface: public interface IOslcClient
{
- Task<IOslcResponse<T>> GetResourceAsync<T>(Uri resource) where T : IResource, new();
+ Task<IOslcResponse<T>> GetResourceAsync<T>(
+ Uri resource,
+ CancellationToken cancellationToken = default) where T : IResource, new();
+
+ Task<IOslcResponse<T>> CreateResourceAsync<T>(
+ Uri resource,
+ T content,
+ CancellationToken cancellationToken = default) where T : IResource, new();
+
+ Task<IOslcResponse<T>> UpdateResourceAsync<T>(
+ Uri resource,
+ T content,
+ CancellationToken cancellationToken = default) where T : IResource, new();
+
+ Task<IOslcResponse> DeleteResourceAsync(
+ Uri resource,
+ CancellationToken cancellationToken = default);
+
+ void SetAuthentication(IOslcAuthentication authentication);
} Additional interface for authentication: public interface IOslcAuthentication
{
Task AuthenticateRequestAsync(IRestRequest request);
} |
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,43 @@ | ||||||||||||||||||||||||||||||||||||
using System; | ||||||||||||||||||||||||||||||||||||
using OSLC4Net.Core.Model; | ||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||
namespace OSLC4Net.Client.RestSharp; | ||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||
/// <summary> | ||||||||||||||||||||||||||||||||||||
/// Interface for responses for OSLC resource requests. | ||||||||||||||||||||||||||||||||||||
/// </summary> | ||||||||||||||||||||||||||||||||||||
/// <typeparam name="T"></typeparam> | ||||||||||||||||||||||||||||||||||||
public interface IOslcResponse<T> where T : IResource | ||||||||||||||||||||||||||||||||||||
{ | ||||||||||||||||||||||||||||||||||||
Uri Uri { get; } | ||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||
OslcResponseStatus Status { get; } | ||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||
public enum OslcResponseStatus | ||||||||||||||||||||||||||||||||||||
{ | ||||||||||||||||||||||||||||||||||||
Success, | ||||||||||||||||||||||||||||||||||||
SuccessNoResource, | ||||||||||||||||||||||||||||||||||||
AuthnFault, | ||||||||||||||||||||||||||||||||||||
ClientNonAuthnFault, | ||||||||||||||||||||||||||||||||||||
ServerFault | ||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||
/// <summary> | ||||||||||||||||||||||||||||||||||||
/// Interface for a successful OSLC resource request containing a resource in the response. | ||||||||||||||||||||||||||||||||||||
/// </summary> | ||||||||||||||||||||||||||||||||||||
/// <typeparam name="T"></typeparam> | ||||||||||||||||||||||||||||||||||||
public interface IOslcResourceResponse<T> : IOslcResponse<T> where T : IResource | ||||||||||||||||||||||||||||||||||||
{ | ||||||||||||||||||||||||||||||||||||
T Resource { get; } | ||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||
/// <summary> | ||||||||||||||||||||||||||||||||||||
/// Interface for a successful OSLC resource request containing a resource in the response. | ||||||||||||||||||||||||||||||||||||
/// </summary> | ||||||||||||||||||||||||||||||||||||
/// <typeparam name="T"></typeparam> | ||||||||||||||||||||||||||||||||||||
public interface IOslcAuthNeededResponse<T> : IOslcResponse<T> where T : IResource | ||||||||||||||||||||||||||||||||||||
{ | ||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||
Comment on lines
+36
to
+43
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fix documentation and consider interface design. The interface has several issues:
/// <summary>
-/// Interface for a successful OSLC resource request containing a resource in the response.
+/// Marker interface for OSLC responses that require authentication.
+/// This interface is used to distinguish responses that failed due to authentication issues,
+/// allowing specialized handling of authentication scenarios.
/// </summary>
/// <typeparam name="T"></typeparam>
public interface IOslcAuthNeededResponse<T> : IOslcResponse<T> where T : IResource
{
-
} 📝 Committable suggestion
Suggested change
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,16 @@ | ||
<Project Sdk="Microsoft.NET.Sdk"> | ||
|
||
<PropertyGroup> | ||
<TargetFramework>netstandard2.1</TargetFramework> | ||
<Nullable>enable</Nullable> | ||
</PropertyGroup> | ||
|
||
<ItemGroup> | ||
<PackageReference Include="RestSharp" /> | ||
</ItemGroup> | ||
|
||
<ItemGroup> | ||
<ProjectReference Include="..\OSLC4Net.Core.DotNetRdfProvider\OSLC4Net.Core.DotNetRdfProvider.csproj" /> | ||
</ItemGroup> | ||
|
||
</Project> |
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,152 @@ | ||||||||||||||||||||||||||||||||||||||||||||
using System; | ||||||||||||||||||||||||||||||||||||||||||||
using System.Diagnostics; | ||||||||||||||||||||||||||||||||||||||||||||
using System.Threading.Tasks; | ||||||||||||||||||||||||||||||||||||||||||||
using OSLC4Net.Core.Model; | ||||||||||||||||||||||||||||||||||||||||||||
using RestSharp; | ||||||||||||||||||||||||||||||||||||||||||||
using RestSharp.Authenticators; | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
namespace OSLC4Net.Client.RestSharp; | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
/// <summary> | ||||||||||||||||||||||||||||||||||||||||||||
/// RestSharp based OSLC client | ||||||||||||||||||||||||||||||||||||||||||||
/// </summary> | ||||||||||||||||||||||||||||||||||||||||||||
// TODO: consider a base class for all RestSharp clients but without auth | ||||||||||||||||||||||||||||||||||||||||||||
public abstract class OslcClientSharpBase : IOslcClient | ||||||||||||||||||||||||||||||||||||||||||||
{ | ||||||||||||||||||||||||||||||||||||||||||||
protected RestClient Client { get; set; } = null!; | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
public async Task<IOslcResponse<T>> GetResourceAsync<T>(Uri resourceUri) | ||||||||||||||||||||||||||||||||||||||||||||
where T : IResource, new() | ||||||||||||||||||||||||||||||||||||||||||||
{ | ||||||||||||||||||||||||||||||||||||||||||||
var request = PrepareGetRequest<T>(resourceUri); | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
return WrapOslcResponse<T>(resourceUri, await Client.GetAsync(request)); | ||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
private static IOslcResponse<T> WrapOslcResponse<T>(Uri resourceUri, RestResponse response) | ||||||||||||||||||||||||||||||||||||||||||||
where T : IResource, new() | ||||||||||||||||||||||||||||||||||||||||||||
{ | ||||||||||||||||||||||||||||||||||||||||||||
if (RestSharpResponseFactory.ResponseStatusFrom(response) == OslcResponseStatus.Success) | ||||||||||||||||||||||||||||||||||||||||||||
{ | ||||||||||||||||||||||||||||||||||||||||||||
return new OslcResourceResponseSharp<T>(resourceUri, response); | ||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
throw new NotImplementedException(); | ||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+26
to
+35
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Replace The Implement proper error handling by throwing an exception that accurately reflects the error condition. For example, you could throw an Apply this diff to fix the issue: private static IOslcResponse<T> WrapOslcResponse<T>(Uri resourceUri, RestResponse response)
where T : IResource, new()
{
if (RestSharpResponseFactory.ResponseStatusFrom(response) == OslcResponseStatus.Success)
{
return new OslcResourceResponseSharp<T>(resourceUri, response);
}
- throw new NotImplementedException();
+ var status = RestSharpResponseFactory.ResponseStatusFrom(response);
+ throw new OslcClientException($"Failed to retrieve resource from {resourceUri}. Status: {status}, HTTP Status Code: {response.StatusCode}, Message: {response.ErrorMessage}");
} 📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
private static RestRequest PrepareGetRequest<T>(Uri resource) where T : IResource | ||||||||||||||||||||||||||||||||||||||||||||
{ | ||||||||||||||||||||||||||||||||||||||||||||
return new RestRequest(resource); | ||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
// no auth | ||||||||||||||||||||||||||||||||||||||||||||
public abstract class OslcClientSharpPublic : OslcClientSharpBase | ||||||||||||||||||||||||||||||||||||||||||||
{ | ||||||||||||||||||||||||||||||||||||||||||||
/// <inheritdoc /> | ||||||||||||||||||||||||||||||||||||||||||||
protected OslcClientSharpPublic(RestClient? client = null) | ||||||||||||||||||||||||||||||||||||||||||||
{ | ||||||||||||||||||||||||||||||||||||||||||||
Client = client ?? new RestClient(); | ||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
public abstract class OslcClientSharpBasic : OslcClientSharpBase | ||||||||||||||||||||||||||||||||||||||||||||
{ | ||||||||||||||||||||||||||||||||||||||||||||
public OslcClientSharpBasic(string username, string password, RestClientOptions? options = null) | ||||||||||||||||||||||||||||||||||||||||||||
{ | ||||||||||||||||||||||||||||||||||||||||||||
var restClientOptions = options ?? new RestClientOptions(); | ||||||||||||||||||||||||||||||||||||||||||||
restClientOptions.Authenticator = new HttpBasicAuthenticator(username, password); | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
Client = new RestClient(restClientOptions); | ||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
// TODO: implement an ASP.NET Core server to test the dance | ||||||||||||||||||||||||||||||||||||||||||||
public abstract class OslcClientSharpOauth10a : OslcClientSharpBase | ||||||||||||||||||||||||||||||||||||||||||||
{ | ||||||||||||||||||||||||||||||||||||||||||||
public OslcClientSharpOauth10a(string key, string secret, string accessToken, | ||||||||||||||||||||||||||||||||||||||||||||
string accessTokenSecret, RestClientOptions? options = null) | ||||||||||||||||||||||||||||||||||||||||||||
{ | ||||||||||||||||||||||||||||||||||||||||||||
var restClientOptions = options ?? new RestClientOptions | ||||||||||||||||||||||||||||||||||||||||||||
{ | ||||||||||||||||||||||||||||||||||||||||||||
Authenticator = OAuth1Authenticator.ForProtectedResource(key, secret, | ||||||||||||||||||||||||||||||||||||||||||||
accessToken, accessTokenSecret) | ||||||||||||||||||||||||||||||||||||||||||||
}; | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
Client = new RestClient(restClientOptions); | ||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
public interface IResponseSharp | ||||||||||||||||||||||||||||||||||||||||||||
{ | ||||||||||||||||||||||||||||||||||||||||||||
public RestResponse RestSharpResponse { get; } | ||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
internal class ResponseSharpMixin<T> : IResponseSharp where T : IResource, new() | ||||||||||||||||||||||||||||||||||||||||||||
{ | ||||||||||||||||||||||||||||||||||||||||||||
// TODO: implement IRestSerializer based on our MediaFormatter | ||||||||||||||||||||||||||||||||||||||||||||
// https://restsharp.dev/docs/advanced/serialization#custom | ||||||||||||||||||||||||||||||||||||||||||||
private readonly Lazy<T> _resourceDeserialized = new(() => new T()); | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
public OslcResponseStatus Status => | ||||||||||||||||||||||||||||||||||||||||||||
RestSharpResponseFactory.ResponseStatusFrom(RestSharpResponse); | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
/// <inheritdoc /> | ||||||||||||||||||||||||||||||||||||||||||||
public RestResponse RestSharpResponse { get; } | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
public T DeserializeResource() | ||||||||||||||||||||||||||||||||||||||||||||
{ | ||||||||||||||||||||||||||||||||||||||||||||
Debug.Assert(_resourceDeserialized.Value != null, "_resourceDeserialized.Value != null"); | ||||||||||||||||||||||||||||||||||||||||||||
return _resourceDeserialized.Value; | ||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+89
to
+101
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ensure proper deserialization of resources In Modify the code to deserialize the resource from the -private readonly Lazy<T> _resourceDeserialized = new(() => new T());
+private readonly Lazy<T> _resourceDeserialized;
+public ResponseSharpMixin(RestResponse restSharpResponse)
+{
+ RestSharpResponse = restSharpResponse;
+ _resourceDeserialized = new Lazy<T>(() => DeserializeFromResponse(restSharpResponse));
+}
+private T DeserializeFromResponse(RestResponse response)
+{
+ // Implement deserialization logic using the custom serializer
+ // For example:
+ // return YourCustomSerializer.Deserialize<T>(response.Content);
+ throw new NotImplementedException();
+}
public T DeserializeResource()
{
Debug.Assert(_resourceDeserialized.Value != null, "_resourceDeserialized.Value != null");
return _resourceDeserialized.Value;
} Would you like help in implementing the
|
||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
public static ResponseSharpMixin<TT> From<TT>(RestResponse restSharpResponse) | ||||||||||||||||||||||||||||||||||||||||||||
where TT : IResource, new() | ||||||||||||||||||||||||||||||||||||||||||||
{ | ||||||||||||||||||||||||||||||||||||||||||||
throw new NotImplementedException(); | ||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+103
to
+107
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Implement the The static method Implement the method to return a new instance of public static ResponseSharpMixin<TT> From<TT>(RestResponse restSharpResponse)
where TT : IResource, new()
{
- throw new NotImplementedException();
+ return new ResponseSharpMixin<TT>(restSharpResponse);
} Ensure the constructor
|
||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
public class OslcResourceResponseSharp<T> : IResponseSharp, IOslcResourceResponse<T> | ||||||||||||||||||||||||||||||||||||||||||||
where T : IResource, new() | ||||||||||||||||||||||||||||||||||||||||||||
{ | ||||||||||||||||||||||||||||||||||||||||||||
public OslcResourceResponseSharp(Uri uri, RestResponse restSharpResponse) | ||||||||||||||||||||||||||||||||||||||||||||
{ | ||||||||||||||||||||||||||||||||||||||||||||
Uri = uri; | ||||||||||||||||||||||||||||||||||||||||||||
Mixin = ResponseSharpMixin<T>.From<T>(restSharpResponse); | ||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
private ResponseSharpMixin<T> Mixin { get; } | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
public Uri Uri { get; } | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
// TODO | ||||||||||||||||||||||||||||||||||||||||||||
/// <inheritdoc /> | ||||||||||||||||||||||||||||||||||||||||||||
public OslcResponseStatus Status => Mixin.Status; | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
public T Resource => Mixin.DeserializeResource(); | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
public RestResponse RestSharpResponse => Mixin.RestSharpResponse; | ||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+110
to
+130
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fix instantiation in In the constructor of After implementing the Additionally, verify that all methods in Would you like assistance in reviewing and adjusting the implementation? |
||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
public class RestSharpResponseFactory | ||||||||||||||||||||||||||||||||||||||||||||
{ | ||||||||||||||||||||||||||||||||||||||||||||
public static OslcResponseStatus ResponseStatusFrom(RestResponse response) | ||||||||||||||||||||||||||||||||||||||||||||
{ | ||||||||||||||||||||||||||||||||||||||||||||
return (int)response.StatusCode switch | ||||||||||||||||||||||||||||||||||||||||||||
{ | ||||||||||||||||||||||||||||||||||||||||||||
> 0 and < 200 => OslcResponseStatus.SuccessNoResource, | ||||||||||||||||||||||||||||||||||||||||||||
200 => OslcResponseStatus.Success, | ||||||||||||||||||||||||||||||||||||||||||||
// TODO: introduce "technical"/intermediate statuses | ||||||||||||||||||||||||||||||||||||||||||||
> 200 and < 400 => OslcResponseStatus.SuccessNoResource, | ||||||||||||||||||||||||||||||||||||||||||||
400 or (>= 402 and < 500) => OslcResponseStatus.ClientNonAuthnFault, | ||||||||||||||||||||||||||||||||||||||||||||
// 403 is an authz fault, need to think what to do with it. In theory, nothing, | ||||||||||||||||||||||||||||||||||||||||||||
// because a non-logged in used should get 401 and a logged in has nothing | ||||||||||||||||||||||||||||||||||||||||||||
// to do other than to ask for access. | ||||||||||||||||||||||||||||||||||||||||||||
// But maybe worth introducing OslcResponseStatus.AuthzFault | ||||||||||||||||||||||||||||||||||||||||||||
401 => OslcResponseStatus.AuthnFault, | ||||||||||||||||||||||||||||||||||||||||||||
>= 500 and <= 599 => OslcResponseStatus.ServerFault, | ||||||||||||||||||||||||||||||||||||||||||||
_ => throw new ArgumentOutOfRangeException() | ||||||||||||||||||||||||||||||||||||||||||||
}; | ||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+132
to
+152
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Handle unanticipated HTTP status codes gracefully In Consider handling unknown status codes by returning a default or unknown status, or by logging a warning. Here's a possible adjustment: >= 500 and <= 599 => OslcResponseStatus.ServerFault,
- _ => throw new ArgumentOutOfRangeException()
+ _ => OslcResponseStatus.UnknownError Additionally, since there are TODO comments about handling specific statuses like 403, you might want to add explicit cases for them. Would you like help in refining the status code handling logic?
|
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.
💡 Codebase verification
Downgrade System.Text.Json and related packages to .NET 8.0
The project is currently targeting .NET 8.0 and netstandard2.x, but uses .NET 9.0.0 preview packages. This mismatch could lead to compatibility issues. Recommend downgrading the following packages to their .NET 8.0 stable versions:
🔗 Analysis chain
Consider using stable version of System.Text.Json
Version 9.0.0 is a preview version as .NET 9 is not yet released. Consider using a stable version unless there's a specific need for preview features.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
Length of output: 592
Script:
Length of output: 771