Skip to content

Commit

Permalink
[ODS-6139] Use Problem Details (RFC 9457) for reporting errors and va…
Browse files Browse the repository at this point in the history
…lidation errors (#903)
  • Loading branch information
gmcelhanon authored and semalaiappan committed Mar 11, 2024
1 parent 31c673c commit bcea958
Show file tree
Hide file tree
Showing 34 changed files with 197 additions and 503 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -497,13 +497,10 @@ void RegisterMiddleware()
.SingleInstance();

builder.RegisterType<ErrorTranslator>().SingleInstance();

builder.RegisterType<AuthenticateResultTranslator>().SingleInstance();

builder.RegisterType<ModelStateKeyConverter>().EnableClassInterceptors().SingleInstance();

builder.RegisterType<CachingInterceptor>()
.Named<IInterceptor>(InterceptorCacheKeys.ModelStateKey)
.Named<IInterceptor>("cache-model-state-key")
.WithParameter(ctx => (ICacheProvider<ulong>) new ConcurrentDictionaryCacheProvider<ulong>())
.SingleInstance();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,11 @@
using System;
using System.Collections.Generic;
using Autofac.Extras.DynamicProxy;
using EdFi.Ods.Common.Caching;
using EdFi.Ods.Common.Models.Resource;

namespace EdFi.Ods.Api.ExceptionHandling;

[Intercept(InterceptorCacheKeys.ModelStateKey)]
[Intercept("cache-model-state-key")]
// ReSharper disable once ClassWithVirtualMembersNeverInherited.Global
public class ModelStateKeyConverter
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ public bool TryTranslate(Exception ex, out IEdFiProblemDetails problemDetails)
match.Groups["property"].Value,
match.Groups["entityPropertyId"].Value));

problemDetails = new NonUniqueConflictException("A problem occurred while processing the request.",
problemDetails = new ConflictException("A problem occurred while processing the request.",
new[] { $"Two {match.Groups["subject"]} entities with the same identifier were associated with the session. See log for additional details." });

return true;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ public bool TryTranslate(Exception ex, out IEdFiProblemDetails problemDetails)
string message = GetMessageUsingRequestContext(constraintName)
?? GetMessageUsingPostgresException();

problemDetails = new NonUniqueConflictException(message);
problemDetails = new ConflictException(message);

return true;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ public bool TryTranslate(Exception ex, out IEdFiProblemDetails problemDetails)
? NoDetailsUpdateOrDeleteMessage
: NoDetailsInsertOrUpdateMessage;

problemDetails = new InvalidReferenceConflictException(noDetailsMessage);
problemDetails = new ConflictException(noDetailsMessage);

return true;
}
Expand All @@ -66,7 +66,7 @@ public bool TryTranslate(Exception ex, out IEdFiProblemDetails problemDetails)
? string.Format(UpdateOrDeleteMessageFormat, association.ThisEntity.Name)
: string.Format(InsertOrUpdateMessageFormat, association.OtherEntity.Name);

problemDetails = new InvalidReferenceConflictException(message);
problemDetails = new ConflictException(message);

return true;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ public bool TryTranslate(Exception ex, out IEdFiProblemDetails problemDetails)

string errorMessage = string.Format(errorMessageFormat, tableName, columnName);

problemDetails = new InvalidReferenceConflictException(errorMessage);
problemDetails = new ConflictException(errorMessage);

return true;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ public bool TryTranslate(Exception ex, out IEdFiProblemDetails problemDetails)

var message = string.Format(MessageFormat, resourceEntity.Name, columnNames, values);

problemDetails = new NaturalKeyConflictException(message);
problemDetails = new ConflictException(message);
return true;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ public bool TryTranslate(Exception ex, out IEdFiProblemDetails problemDetails)
message = string.Format(MultipleMessageFormat, values, columnNames, tableName);
}

problemDetails = new NonUniqueConflictException(message);
problemDetails = new ConflictException(message);
return true;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ public bool TryTranslate(Exception ex, out IEdFiProblemDetails problemDetails)
// This is probably a timing issue and is not expected under normal operations, so we'll log it.
_logger.Error(ex);

problemDetails = new NaturalKeyConflictException(
problemDetails = new ConflictException(
"A natural key conflict occurred when attempting to update a new resource with a duplicate key.");

return true;
Expand Down
1 change: 0 additions & 1 deletion Application/EdFi.Ods.Api/Startup/OdsStartupBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@
using EdFi.Ods.Common.Database;
using EdFi.Ods.Common.Dependencies;
using EdFi.Ods.Common.Descriptors;
using EdFi.Ods.Common.Exceptions;
using EdFi.Ods.Common.Infrastructure.Configuration;
using EdFi.Ods.Common.Infrastructure.Extensibility;
using EdFi.Ods.Common.Models;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,17 +22,11 @@ namespace EdFi.Ods.Common.Exceptions;
│ │ └─────────────────────────┘
│ │ △
│ │ │ ┌────────────────────────────────┐
│ │ ├──┤ KeyChangeNotSupportedException |
│ │ │ └────────────────────────────────┘
│ │ │ ┌─────────────────────┐
│ │ └──┤ DataPolicyException |
│ │ └─────────────────────┘
│ │ └──┤ KeyChangeNotSupportedException |
│ │ └────────────────────────────────┘
│ │ ┌──────────────────────────────┐
│ └──┤ BadRequestParameterException |
│ └──────────────────────────────┘
│ ┌─────────────────────────────────┐
├──┤ SecurityAuthenticationException | 401 Unauthorized
│ └─────────────────────────────────┘
│ ┌────────────────────────────────┐
├──┤ SecurityAuthorizationException | 403 Forbidden
│ └────────────────────────────────┘
Expand All @@ -46,16 +40,6 @@ namespace EdFi.Ods.Common.Exceptions;
│ ┌───────────────────┐
├──┤ ConflictException | 409 Conflict
│ └───────────────────┘
│ △
│ │ ┌─────────────────────────────┐
│ ├──┤ NonUniqueConflictException |
│ │ └─────────────────────────────┘
│ │ ┌──────────────────────────────────────┐
│ │──┤ InvalidReferenceConflictException |
│ │ └──────────────────────────────────────┘
│ │ ┌──────────────────────────────────────┐
│ └──┤ NaturalKeyConflictException |
│ └──────────────────────────────────────┘
│ ┌──────────────────────┐
├──┤ ConcurrencyException | 412 Precondition Failed
│ └──────────────────────┘
Expand Down Expand Up @@ -92,9 +76,6 @@ namespace EdFi.Ods.Common.Exceptions;
│ ┌───────────────────────────────┐
├──┤ SnapshotsAreReadOnlyException | 405 Method Not Allowed
│ └───────────────────────────────┘
│ ┌───────────────────────────┐
├──┤ MethodNotAllowedException | 405 Method Not Allowed
│ └───────────────────────────┘
│ ┌──────────────────────┐
├──┤ NotModifiedException | 304 Not Modified
│ └──────────────────────┘
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,6 @@
using System.Net;
using System.Threading.Tasks;
using EdFi.Ods.Common.Exceptions;
using EdFi.Ods.Api.Models;
using Microsoft.AspNetCore.Http;
using Microsoft.AspNetCore.Mvc;

namespace EdFi.Ods.Features.ChangeQueries.ActionResults
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
using System;
using System.Collections.Concurrent;
using EdFi.Ods.Api.Caching.Person;
using EdFi.Ods.Features.Services.Redis;

namespace EdFi.Ods.Features.ExternalCache.Redis;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,7 @@ await WriteResponseMessage(
await WriteResponseMessage(
response,
StatusCodes.Status400BadRequest,
headerName,
$"A profile-based content type that is readable cannot be used with {request.Method.ToUpper()} requests.");

return (false, null);
Expand Down Expand Up @@ -208,33 +209,16 @@ await WriteResponseUsingFormat(
return (true, null);
}

Task WriteResponseUsingFormat(
HttpResponse response,
int statusCode,
string headerName,
LogMessageFormatType logMessageFormatType)
{
string correlationId = (string)_logContextAccessor.GetValue(CorrelationConstants.LogContextKey);
string errorMessage = string.Format(_logMessageFormatSpecifiers[(int)logMessageFormatType], headerName);

return response.WriteProblemDetailsAsync(
statusCode,
ProfileContentTypeUsageException.TitleText,
ProfileContentTypeUsageException.DefaultDetail,
new[] { errorMessage },
correlationId,
ProfileContentTypeUsageException.TypePart);
}

Task WriteResponseMessage(HttpResponse response, int statusCode, string errorMessage)
Task WriteResponse(HttpResponse response, int statusCode, string headerName, string logMessageFormat)
{
string correlationId = (string)_logContextAccessor.GetValue(CorrelationConstants.LogContextKey);
string correlationId = (string) _logContextAccessor.GetValue(CorrelationConstants.LogContextKey);
string errorMessage = string.Format(logMessageFormat, headerName);

return response.WriteProblemDetailsAsync(
statusCode,
ProfileContentTypeUsageException.TitleText,
ProfileContentTypeUsageException.DefaultDetail,
new[] { errorMessage },
new[] {errorMessage },
correlationId,
ProfileContentTypeUsageException.TypePart);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,20 @@
// The Ed-Fi Alliance licenses this file to you under the Apache License, Version 2.0.
// See the LICENSE and NOTICES files in the project root for more information.

<<<<<<<< HEAD:Application/EdFi.Ods.Features/Services/Redis/IRedisConnectionProvider.cs
using StackExchange.Redis;

namespace EdFi.Ods.Features.Services.Redis;

public interface IRedisConnectionProvider
{
IDatabase Get();
========
namespace EdFi.Ods.Common.Validation;

public static class ValidationContextKeys
{
public const string PathBuilder = "PB";
public const string ResourceClass = "RC";
>>>>>>>> 3d929701 ([ODS-6139] Use Problem Details (RFC 9457) for reporting errors and validation errors (#903)):Application/EdFi.Ods.Common/Validation/ValidationContextKeys.cs
}
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ public void TryTranslate_WithNonUniqueObjectException_ShouldReturnTrueAndSetProb
// Assert
result.ShouldBeTrue();
problemDetails.ShouldNotBeNull();
problemDetails.ShouldBeOfType<NonUniqueConflictException>();
problemDetails.ShouldBeOfType<ConflictException>();
problemDetails.Detail.ShouldBe("A problem occurred while processing the request.");

problemDetails.Errors.ShouldContain(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ public void Should_RestError_show_single_value_message()
AssertHelper.All(
() => actualError.ShouldNotBeNull(),
() => actualError.Status.ShouldBe(409),
() => actualError.Type.ShouldBe(string.Join(':', EdFiProblemDetailsExceptionBase.BaseTypePrefix, "conflict:not-unique")),
() => actualError.Type.ShouldBe(string.Join(':', EdFiProblemDetailsExceptionBase.BaseTypePrefix, "conflict")),
() => actualError.Detail.ShouldBe("The value supplied for property 'Property1' of entity 'Something' is not unique.")
);
}
Expand Down Expand Up @@ -231,7 +231,11 @@ public void Should_RestError_show_multiple_values_message()
AssertHelper.All(
() => actualError.ShouldNotBeNull(),
() => actualError.Status.ShouldBe(409),
<<<<<<< HEAD
() => actualError.Type.ShouldBe(string.Join(':', EdFiProblemDetailsExceptionBase.BaseTypePrefix, "conflict:not-unique")),
=======
() => actualError.Type.ShouldBe(string.Join(':', EdFiProblemDetailsExceptionBase.BaseTypePrefix, "conflict")),
>>>>>>> 3d929701 ([ODS-6139] Use Problem Details (RFC 9457) for reporting errors and validation errors (#903))
() => actualError.Detail.ShouldBe("The values supplied for properties 'Property1', 'Property2' of entity 'Something' are not unique.")
);
}
Expand Down Expand Up @@ -277,7 +281,11 @@ public void Should_RestError_show_unknown_value_message()
AssertHelper.All(
() => actualError.ShouldNotBeNull(),
() => actualError.Status.ShouldBe(409),
<<<<<<< HEAD
() => actualError.Type.ShouldBe(string.Join(':', EdFiProblemDetailsExceptionBase.BaseTypePrefix, "conflict:not-unique")),
=======
() => actualError.Type.ShouldBe(string.Join(':', EdFiProblemDetailsExceptionBase.BaseTypePrefix, "conflict")),
>>>>>>> 3d929701 ([ODS-6139] Use Problem Details (RFC 9457) for reporting errors and validation errors (#903))
() => actualError.Detail.ShouldBe("The value(s) supplied for the resource are not unique.")
);
}
Expand Down Expand Up @@ -327,7 +335,11 @@ public void Should_RestError_show_unknown_value_message()
AssertHelper.All(
() => actualError.ShouldNotBeNull(),
() => actualError.Status.ShouldBe(409),
<<<<<<< HEAD
() => actualError.Type.ShouldBe(string.Join(':', EdFiProblemDetailsExceptionBase.BaseTypePrefix, "conflict:not-unique")),
=======
() => actualError.Type.ShouldBe(string.Join(':', EdFiProblemDetailsExceptionBase.BaseTypePrefix, "conflict")),
>>>>>>> 3d929701 ([ODS-6139] Use Problem Details (RFC 9457) for reporting errors and validation errors (#903))
() => actualError.Detail.ShouldBe("The values supplied for properties 'property1, property2, property3' of entity 'something' are not unique.")
);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ public void Should_RestError_show_simple_constraint_message()
AssertHelper.All(
() => _actualError.ShouldNotBeNull(),
() => _actualError.Status.ShouldBe(409),
() => _actualError.Type.ShouldBe(string.Join(':', EdFiProblemDetailsExceptionBase.BaseTypePrefix, "conflict:invalid-reference")),
() => _actualError.Type.ShouldBe(string.Join(':', EdFiProblemDetailsExceptionBase.BaseTypePrefix, "conflict")),
() => _actualError.Detail.ShouldBe("The referenced 'School' resource does not exist.")
);
}
Expand Down Expand Up @@ -214,7 +214,7 @@ public void Should_return_a_409_Conflict_error_with_a_message_identifying_the_de
AssertHelper.All(
() => actualError.ShouldNotBeNull(),
() => actualError.Status.ShouldBe(409),
() => actualError.Type.ShouldBe(string.Join(':', EdFiProblemDetailsExceptionBase.BaseTypePrefix, "conflict:invalid-reference")),
() => actualError.Type.ShouldBe(string.Join(':', EdFiProblemDetailsExceptionBase.BaseTypePrefix, "conflict")),
() => actualError.Detail.ShouldBe("The operation cannot be performed because the resource is a dependency of the 'StudentSchoolAssociation' resource.")
);
}
Expand Down Expand Up @@ -279,7 +279,7 @@ public void Should_return_a_409_Conflict_error_with_a_message_indicating_a_depen
actualError.ShouldSatisfyAllConditions(
e => e.ShouldNotBeNull(),
e => e.Status.ShouldBe(409),
e => e.Type.ShouldBe(string.Join(':', EdFiProblemDetailsExceptionBase.BaseTypePrefix, "conflict:invalid-reference")),
e => e.Type.ShouldBe(string.Join(':', EdFiProblemDetailsExceptionBase.BaseTypePrefix, "conflict")),
e => e.Detail.ShouldBe(
"The operation cannot be performed because the resource is a dependency of another resource."));
}
Expand Down
Loading

0 comments on commit bcea958

Please sign in to comment.