Skip to content

Commit

Permalink
Refine 409 Problem Details (RFC 9457) error messages Added
Browse files Browse the repository at this point in the history
  • Loading branch information
semalaiappan committed Feb 16, 2024
1 parent 4b5d6f2 commit de55a5f
Show file tree
Hide file tree
Showing 20 changed files with 204 additions and 32 deletions.
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 ConflictException("A problem occurred while processing the request.",
problemDetails = new NonUniqueConflictException("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 ConflictException(message);
problemDetails = new NonUniqueConflictException(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 ConflictException(noDetailsMessage);
problemDetails = new InvalidReferenceConflictException(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 ConflictException(message);
problemDetails = new InvalidReferenceConflictException(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 ConflictException(errorMessage);
problemDetails = new InvalidReferenceConflictException(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 ConflictException(message);
problemDetails = new NaturalKeyConflictException(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 ConflictException(message);
problemDetails = new NonUniqueConflictException(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 ConflictException(
problemDetails = new NaturalKeyConflictException(
"A natural key conflict occurred when attempting to update a new resource with a duplicate key.");

return true;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,16 @@ namespace EdFi.Ods.Common.Exceptions;
│ ┌───────────────────┐
├──┤ ConflictException | 409 Conflict
│ └───────────────────┘
│ △
│ │ ┌─────────────────────────────┐
│ ├──┤ NonUniqueConflictException |
│ │ └─────────────────────────────┘
│ │ ┌──────────────────────────────────────┐
│ │──┤ InvalidReferenceConflictException |
│ │ └──────────────────────────────────────┘
│ │ ┌──────────────────────────────────────┐
│ └──┤ NaturalKeyConflictException |
│ └──────────────────────────────────────┘
│ ┌──────────────────────┐
├──┤ ConcurrencyException | 412 Precondition Failed
│ └──────────────────────┘
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
// SPDX-License-Identifier: Apache-2.0
// Licensed to the Ed-Fi Alliance under one or more agreements.
// 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.

using System;
using System.Collections.Generic;
using Microsoft.AspNetCore.Http;

namespace EdFi.Ods.Common.Exceptions;

public class InvalidReferenceConflictException : ConflictException
{
// Fields containing override values for Problem Details
private const string TypePart = "invalid-reference";
private const string TitleText = "Resource Not Unique Conflict due to invalid-reference";
private const int StatusValue = StatusCodes.Status409Conflict;

public InvalidReferenceConflictException(string detail)
: base(detail) { }

public InvalidReferenceConflictException(string detail, Exception innerException)
: base(detail, innerException) { }


public InvalidReferenceConflictException(string detail, string[] errors)
: base(detail)
{
((IEdFiProblemDetails)this).Errors = errors;
}
// ---------------------------
// Boilerplate for overrides
// ---------------------------
public override string Title { get => TitleText; }

public override int Status { get => StatusValue; }

protected override IEnumerable<string> GetTypeParts()
{
foreach (var part in base.GetTypeParts())
{
yield return part;
}

yield return TypePart;
}
// ---------------------------
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
// SPDX-License-Identifier: Apache-2.0
// Licensed to the Ed-Fi Alliance under one or more agreements.
// 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.

using System;
using System.Collections.Generic;
using Microsoft.AspNetCore.Http;

namespace EdFi.Ods.Common.Exceptions;

public class NaturalKeyConflictException : ConflictException
{
// Fields containing override values for Problem Details
private const string TypePart = "natural-key";
private const string TitleText = "Resource Not Unique Conflict due to natural-key";
private const int StatusValue = StatusCodes.Status409Conflict;

public NaturalKeyConflictException(string detail)
: base(detail) { }

public NaturalKeyConflictException(string detail, Exception innerException)
: base(detail, innerException) { }


public NaturalKeyConflictException(string detail, string[] errors)
: base(detail)
{
((IEdFiProblemDetails)this).Errors = errors;
}
// ---------------------------
// Boilerplate for overrides
// ---------------------------
public override string Title { get => TitleText; }

public override int Status { get => StatusValue; }

protected override IEnumerable<string> GetTypeParts()
{
foreach (var part in base.GetTypeParts())
{
yield return part;
}

yield return TypePart;
}
// ---------------------------
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
// SPDX-License-Identifier: Apache-2.0
// Licensed to the Ed-Fi Alliance under one or more agreements.
// 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.

using System;
using System.Collections.Generic;
using Microsoft.AspNetCore.Http;

namespace EdFi.Ods.Common.Exceptions;

public class NonUniqueConflictException : ConflictException
{
// Fields containing override values for Problem Details
private const string TypePart = "not-unique";
private const string TitleText = "Resource Not Unique Conflict Due to Not-Unique";
private const int StatusValue = StatusCodes.Status409Conflict;

public NonUniqueConflictException(string detail)
: base(detail) { }

public NonUniqueConflictException(string detail, Exception innerException)
: base(detail, innerException) { }


public NonUniqueConflictException(string detail, string[] errors)
: base(detail)
{
((IEdFiProblemDetails)this).Errors = errors;
}
// ---------------------------
// Boilerplate for overrides
// ---------------------------
public override string Title { get => TitleText; }

public override int Status { get => StatusValue; }

protected override IEnumerable<string> GetTypeParts()
{
foreach (var part in base.GetTypeParts())
{
yield return part;
}

yield return TypePart;
}
// ---------------------------
}
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<ConflictException>();
problemDetails.ShouldBeOfType<NonUniqueConflictException>();
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")),
() => actualError.Type.ShouldBe(string.Join(':', EdFiProblemDetailsExceptionBase.BaseTypePrefix, "conflict:not-unique")),
() => actualError.Detail.ShouldBe("The value supplied for property 'Property1' of entity 'Something' is not unique.")
);
}
Expand Down Expand Up @@ -231,7 +231,7 @@ public void Should_RestError_show_multiple_values_message()
AssertHelper.All(
() => actualError.ShouldNotBeNull(),
() => actualError.Status.ShouldBe(409),
() => actualError.Type.ShouldBe(string.Join(':', EdFiProblemDetailsExceptionBase.BaseTypePrefix, "conflict")),
() => actualError.Type.ShouldBe(string.Join(':', EdFiProblemDetailsExceptionBase.BaseTypePrefix, "conflict:not-unique")),
() => actualError.Detail.ShouldBe("The values supplied for properties 'Property1', 'Property2' of entity 'Something' are not unique.")
);
}
Expand Down Expand Up @@ -277,7 +277,7 @@ public void Should_RestError_show_unknown_value_message()
AssertHelper.All(
() => actualError.ShouldNotBeNull(),
() => actualError.Status.ShouldBe(409),
() => actualError.Type.ShouldBe(string.Join(':', EdFiProblemDetailsExceptionBase.BaseTypePrefix, "conflict")),
() => actualError.Type.ShouldBe(string.Join(':', EdFiProblemDetailsExceptionBase.BaseTypePrefix, "conflict:not-unique")),
() => actualError.Detail.ShouldBe("The value(s) supplied for the resource are not unique.")
);
}
Expand Down Expand Up @@ -327,7 +327,7 @@ public void Should_RestError_show_unknown_value_message()
AssertHelper.All(
() => actualError.ShouldNotBeNull(),
() => actualError.Status.ShouldBe(409),
() => actualError.Type.ShouldBe(string.Join(':', EdFiProblemDetailsExceptionBase.BaseTypePrefix, "conflict")),
() => actualError.Type.ShouldBe(string.Join(':', EdFiProblemDetailsExceptionBase.BaseTypePrefix, "conflict:not-unique")),
() => 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")),
() => _actualError.Type.ShouldBe(string.Join(':', EdFiProblemDetailsExceptionBase.BaseTypePrefix, "conflict:invalid-reference")),
() => _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")),
() => actualError.Type.ShouldBe(string.Join(':', EdFiProblemDetailsExceptionBase.BaseTypePrefix, "conflict:invalid-reference")),
() => 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")),
e => e.Type.ShouldBe(string.Join(':', EdFiProblemDetailsExceptionBase.BaseTypePrefix, "conflict:invalid-reference")),
e => e.Detail.ShouldBe(
"The operation cannot be performed because the resource is a dependency of another resource."));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ protected override void Act()
public virtual void Should_respond_with_a_409_Conflict()
{
Assert.That(_actualError.Status, Is.EqualTo((int) HttpStatusCode.Conflict));
Assert.That(_actualError.Type, Is.EqualTo(string.Join(':', EdFiProblemDetailsExceptionBase.BaseTypePrefix, "conflict")));
Assert.That(_actualError.Type, Is.EqualTo(string.Join(':', EdFiProblemDetailsExceptionBase.BaseTypePrefix, "conflict:invalid-reference")));
}

[Test]
Expand Down Expand Up @@ -76,7 +76,7 @@ protected override void Act()
public virtual void Should_respond_with_a_409_Conflict()
{
Assert.That(_actualError.Status, Is.EqualTo((int) HttpStatusCode.Conflict));
Assert.That(_actualError.Type, Is.EqualTo(string.Join(':', EdFiProblemDetailsExceptionBase.BaseTypePrefix, "conflict")));
Assert.That(_actualError.Type, Is.EqualTo(string.Join(':', EdFiProblemDetailsExceptionBase.BaseTypePrefix, "conflict:invalid-reference")));
}

[Test]
Expand Down Expand Up @@ -111,7 +111,7 @@ protected override void Act()
public virtual void Should_respond_with_a_409_Conflict()
{
Assert.That(_actualError.Status, Is.EqualTo((int) HttpStatusCode.Conflict));
Assert.That(_actualError.Type, Is.EqualTo(string.Join(':', EdFiProblemDetailsExceptionBase.BaseTypePrefix, "conflict")));
Assert.That(_actualError.Type, Is.EqualTo(string.Join(':', EdFiProblemDetailsExceptionBase.BaseTypePrefix, "conflict:invalid-reference")));
}

[Test]
Expand Down Expand Up @@ -147,7 +147,7 @@ protected override void Act()
public virtual void Should_respond_with_a_409_Conflict()
{
Assert.That(_actualError.Status, Is.EqualTo((int) HttpStatusCode.Conflict));
Assert.That(_actualError.Type, Is.EqualTo(string.Join(':', EdFiProblemDetailsExceptionBase.BaseTypePrefix, "conflict")));
Assert.That(_actualError.Type, Is.EqualTo(string.Join(':', EdFiProblemDetailsExceptionBase.BaseTypePrefix, "conflict:invalid-reference")));
}

[Test]
Expand Down Expand Up @@ -196,7 +196,7 @@ protected override void Act()
public virtual void Should_respond_with_a_409_Conflict()
{
Assert.That(_actualError.Status, Is.EqualTo((int) HttpStatusCode.Conflict));
Assert.That(_actualError.Type, Is.EqualTo(string.Join(':', EdFiProblemDetailsExceptionBase.BaseTypePrefix, "conflict")));
Assert.That(_actualError.Type, Is.EqualTo(string.Join(':', EdFiProblemDetailsExceptionBase.BaseTypePrefix, "conflict:not-unique")));
}

[Test]
Expand Down Expand Up @@ -246,7 +246,7 @@ protected override void Act()
public virtual void Should_respond_with_a_409_Conflict()
{
Assert.That(_actualError.Status, Is.EqualTo((int) HttpStatusCode.Conflict));
Assert.That(_actualError.Type, Is.EqualTo(string.Join(':', EdFiProblemDetailsExceptionBase.BaseTypePrefix, "conflict")));
Assert.That(_actualError.Type, Is.EqualTo(string.Join(':', EdFiProblemDetailsExceptionBase.BaseTypePrefix, "conflict:not-unique")));
}

[Test]
Expand Down Expand Up @@ -294,7 +294,7 @@ protected override void Act()
public virtual void Should_respond_with_a_409_Conflict()
{
Assert.That(_actualError.Status, Is.EqualTo((int) HttpStatusCode.Conflict));
Assert.That(_actualError.Type, Is.EqualTo(string.Join(':', EdFiProblemDetailsExceptionBase.BaseTypePrefix, "conflict")));
Assert.That(_actualError.Type, Is.EqualTo(string.Join(':', EdFiProblemDetailsExceptionBase.BaseTypePrefix, "conflict:not-unique")));
}

[Test]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ public void Should_set_a_reasonable_message()
[Test]
public void Should_set_the_exception_type_to_conflict()
{
actualError.Type.ShouldBe(string.Join(':', EdFiProblemDetailsExceptionBase.BaseTypePrefix, "conflict"));
actualError.Type.ShouldBe(string.Join(':', EdFiProblemDetailsExceptionBase.BaseTypePrefix, "conflict:natural-key"));
}

[Test]
Expand Down Expand Up @@ -199,7 +199,7 @@ public void Should_set_a_reasonable_message()
[Test]
public void Should_set_the_exception_type_to_conflict()
{
actualError.Type.ShouldBe(string.Join(':', EdFiProblemDetailsExceptionBase.BaseTypePrefix, "conflict"));
actualError.Type.ShouldBe(string.Join(':', EdFiProblemDetailsExceptionBase.BaseTypePrefix, "conflict:natural-key"));
}

[Test]
Expand Down
Loading

0 comments on commit de55a5f

Please sign in to comment.