diff --git a/Application/EdFi.Ods.Api/ExceptionHandling/Translators/Postgres/PostgresDuplicatedKeyExceptionTranslator.cs b/Application/EdFi.Ods.Api/ExceptionHandling/Translators/Postgres/PostgresDuplicatedKeyExceptionTranslator.cs index bac6b26a56..a2221d5d63 100644 --- a/Application/EdFi.Ods.Api/ExceptionHandling/Translators/Postgres/PostgresDuplicatedKeyExceptionTranslator.cs +++ b/Application/EdFi.Ods.Api/ExceptionHandling/Translators/Postgres/PostgresDuplicatedKeyExceptionTranslator.cs @@ -4,9 +4,12 @@ // See the LICENSE and NOTICES files in the project root for more information. using System; +using System.Linq; using System.Net; using System.Text.RegularExpressions; using EdFi.Ods.Api.Models; +using EdFi.Ods.Common.Context; +using EdFi.Ods.Common.Security.Claims; using NHibernate.Exceptions; using Npgsql; @@ -14,10 +17,23 @@ namespace EdFi.Ods.Api.ExceptionHandling.Translators.Postgres { public class PostgresDuplicatedKeyExceptionTranslator : IExceptionTranslator { - private static readonly Regex _expression = new Regex(@"(?\d*): duplicate key value violates unique constraint ""(?.*?)"""); - private static readonly Regex _detailExpression = new Regex(@"Key \((?.*?)\)=\((?.*?)\) (?already exists)."); - private const string SimpleKeyMessageFormat = "The value {0} supplied for property '{1}' of entity '{2}' is not unique."; - private const string ComposedKeyMessageFormat = "The values {0} supplied for properties '{1}' of entity '{2}' are not unique."; + private readonly IContextProvider _dataManagementResourceContextProvider; + + private static readonly Regex _expression = new(@"(?\d*): duplicate key value violates unique constraint ""(?.*?)"""); + private static readonly Regex _detailExpression = new(@"Key \((?.*?)\)=\((?.*?)\) (?already exists)."); + + private const string GenericMessage = "The value(s) supplied for the resource are not unique."; + + private const string SimpleKeyMessageFormat = "The value supplied for property '{0}' of entity '{1}' is not unique."; + private const string CompositeKeyMessageFormat = "The values supplied for properties '{0}' of entity '{1}' are not unique."; + + private const string PrimaryKeyNameSuffix = "_pk"; + + public PostgresDuplicatedKeyExceptionTranslator( + IContextProvider dataManagementResourceContextProvider) + { + _dataManagementResourceContextProvider = dataManagementResourceContextProvider; + } public bool TryTranslateMessage(Exception ex, out RESTError webServiceError) { @@ -33,21 +49,68 @@ public bool TryTranslateMessage(Exception ex, out RESTError webServiceError) if (match.Success) { - var exceptionInfo = new PostgresExceptionInfo(postgresException, _detailExpression); + var constraintName = match.Groups["ConstraintName"].ValueSpan; - string message = string.Format(exceptionInfo.IsComposedKeyConstraint - ? ComposedKeyMessageFormat - : SimpleKeyMessageFormat, exceptionInfo.Values, exceptionInfo.ColumnNames, exceptionInfo.TableName); + string message = GetMessageUsingRequestContext(constraintName) + ?? GetMessageUsingPostgresException(); webServiceError = new RESTError - { - Code = (int)HttpStatusCode.Conflict, - Type = "Conflict", - Message = message - }; + { + Code = (int) HttpStatusCode.Conflict, + Type = "Conflict", + Message = message + }; return true; } + + string GetMessageUsingRequestContext(ReadOnlySpan constraintName) + { + // Rely on PK suffix naming convention to identify PK constraint violation (which covers almost all scenarios for this violation) + if (constraintName.EndsWith(PrimaryKeyNameSuffix, StringComparison.OrdinalIgnoreCase)) + { + var tableName = constraintName.Slice(0, constraintName.Length - PrimaryKeyNameSuffix.Length).ToString(); + + // Look for matching class in the request's targeted resource + if (_dataManagementResourceContextProvider.Get()?.Resource? + .ContainedItemTypeByName.TryGetValue(tableName, out var resourceClass) ?? false) + { + var pkPropertyNames = resourceClass.IdentifyingProperties.Select(p => p.PropertyName).ToArray(); + + return string.Format( + (pkPropertyNames.Length > 1) + ? CompositeKeyMessageFormat + : SimpleKeyMessageFormat, + string.Join(", ", pkPropertyNames), + resourceClass.Name); + } + } + + return null; + } + + string GetMessageUsingPostgresException() + { + string message; + var exceptionInfo = new PostgresExceptionInfo(postgresException, _detailExpression); + + // Column names will only be available form Postgres if a special argument is added to the connection string + if (exceptionInfo.ColumnNames.Length > 0 && exceptionInfo.ColumnNames != PostgresExceptionInfo.UnknownValue) + { + message = string.Format( + exceptionInfo.IsComposedKeyConstraint + ? CompositeKeyMessageFormat + : SimpleKeyMessageFormat, + exceptionInfo.ColumnNames, + exceptionInfo.TableName); + } + else + { + message = GenericMessage; + } + + return message; + } } return false; diff --git a/Application/EdFi.Ods.Api/ExceptionHandling/Translators/Postgres/PostgresExceptionInfo.cs b/Application/EdFi.Ods.Api/ExceptionHandling/Translators/Postgres/PostgresExceptionInfo.cs index 640c557a2b..1838e923d9 100644 --- a/Application/EdFi.Ods.Api/ExceptionHandling/Translators/Postgres/PostgresExceptionInfo.cs +++ b/Application/EdFi.Ods.Api/ExceptionHandling/Translators/Postgres/PostgresExceptionInfo.cs @@ -18,7 +18,7 @@ public enum PostgresExceptionConstraintType public class PostgresExceptionInfo { - private const string UnknownValue = "unknown"; + public const string UnknownValue = "unknown"; public string TableName { get; } public string ColumnNames { get; } diff --git a/Application/EdFi.Ods.Tests/EdFi.Ods.Api/Caching/PersonMapCacheInitializerTests.cs b/Application/EdFi.Ods.Tests/EdFi.Ods.Api/Caching/PersonMapCacheInitializerTests.cs index 0e07f481f3..214fbdbe2f 100644 --- a/Application/EdFi.Ods.Tests/EdFi.Ods.Api/Caching/PersonMapCacheInitializerTests.cs +++ b/Application/EdFi.Ods.Tests/EdFi.Ods.Api/Caching/PersonMapCacheInitializerTests.cs @@ -36,7 +36,7 @@ public void EnsurePersonMapsInitialized_ShouldInitializeCache_WhenNotAlreadyInit }; A.CallTo(() => fakePersonIdentifiersProvider.GetAllPersonIdentifiersAsync(personType)) - .Returns(TaskHelpers.FromResultWithDelay>(personIdentifiers, 25)); + .Returns(TaskHelpers.FromResultWithDelay>(personIdentifiers, 100)); var initializer = new PersonMapCacheInitializer( fakePersonIdentifiersProvider, diff --git a/Application/EdFi.Ods.Tests/EdFi.Ods.Api/ExceptionHandling/Translators/Postgres/PostgresDuplicatedKeyExceptionTranslatorTests.cs b/Application/EdFi.Ods.Tests/EdFi.Ods.Api/ExceptionHandling/Translators/Postgres/PostgresDuplicatedKeyExceptionTranslatorTests.cs index ed09bfd30a..46834cde14 100644 --- a/Application/EdFi.Ods.Tests/EdFi.Ods.Api/ExceptionHandling/Translators/Postgres/PostgresDuplicatedKeyExceptionTranslatorTests.cs +++ b/Application/EdFi.Ods.Tests/EdFi.Ods.Api/ExceptionHandling/Translators/Postgres/PostgresDuplicatedKeyExceptionTranslatorTests.cs @@ -4,11 +4,22 @@ // See the LICENSE and NOTICES files in the project root for more information. using System; +using System.Data; using System.Diagnostics.CodeAnalysis; +using System.Linq; using EdFi.Ods.Api.ExceptionHandling.Translators.Postgres; using EdFi.Ods.Api.Models; +using EdFi.Ods.Common.Context; +using EdFi.Ods.Common.Models; +using EdFi.Ods.Common.Models.Definitions; +using EdFi.Ods.Common.Models.Definitions.Transformers; +using EdFi.Ods.Common.Models.Domain; +using EdFi.Ods.Common.Models.Resource; +using EdFi.Ods.Common.Security.Claims; using EdFi.Ods.Tests._Builders; +using EdFi.Ods.Tests.TestExtension; using EdFi.TestFixture; +using FakeItEasy; using NHibernate.Exceptions; using NUnit.Framework; using Shouldly; @@ -17,6 +28,7 @@ namespace EdFi.Ods.Tests.EdFi.Ods.Common.ExceptionHandling { [SuppressMessage("ReSharper", "InconsistentNaming")] + [TestFixture] public class PostgresDuplicatedKeyExceptionTranslatorTests { private const string GenericSqlExceptionMessage = "Generic Sql exception message."; @@ -26,15 +38,17 @@ public class When_a_regular_exception_is_thrown : TestFixtureBase private Exception exception; private bool result; private RESTError actualError; + private IContextProvider _contextProvider; protected override void Arrange() { exception = new Exception(); + _contextProvider = Stub>(); } protected override void Act() { - var translator = new PostgresDuplicatedKeyExceptionTranslator(); + var translator = new PostgresDuplicatedKeyExceptionTranslator(_contextProvider); result = translator.TryTranslateMessage(exception, out actualError); } @@ -57,15 +71,17 @@ public class When_a_generic_ADO_exception_is_thrown_without_an_inner_exception private Exception exception; private bool wasHandled; private RESTError actualError; + private IContextProvider _contextProvider; protected override void Arrange() { exception = new GenericADOException(GenericSqlExceptionMessage, null); + _contextProvider = Stub>(); } protected override void Act() { - var translator = new PostgresDuplicatedKeyExceptionTranslator(); + var translator = new PostgresDuplicatedKeyExceptionTranslator(_contextProvider); wasHandled = translator.TryTranslateMessage(exception, out actualError); } @@ -88,6 +104,7 @@ public class When_a_generic_ADO_exception_is_thrown_with_an_inner_exception_with private Exception exception; private bool wasHandled; private RESTError actualError; + private IContextProvider _contextProvider; protected override void Arrange() { @@ -100,11 +117,13 @@ protected override void Arrange() null, null, null); + + _contextProvider = Stub>(); } protected override void Act() { - var translator = new PostgresDuplicatedKeyExceptionTranslator(); + var translator = new PostgresDuplicatedKeyExceptionTranslator(_contextProvider); wasHandled = translator.TryTranslateMessage(exception, out actualError); } @@ -121,15 +140,16 @@ public void Should_RestError_be_null() } } - public class When_an_insert_conflicts_with_a_unique_index_with_a_simple_key_constraint : TestFixtureBase + public class When_an_insert_or_update_conflicts_with_the_unique_index_on_a_single_column_primary_key : TestFixtureBase { private Exception exception; private bool wasHandled; private RESTError actualError; + private IContextProvider _contextProvider; protected override void Arrange() { - const string message = "duplicate key value violates unique constraint \"PK_ApplicationId\""; + const string message = "duplicate key value violates unique constraint \"something_pk\""; exception = NHibernateExceptionBuilder.CreateWrappedPostgresException( GenericSqlExceptionMessage, @@ -138,11 +158,15 @@ protected override void Arrange() null, null, null); + + _contextProvider = Stub>(); + var resource = PrepareTestResource(isCompositePrimaryKey: false); + A.CallTo(() => _contextProvider.Get()).Returns(new DataManagementResourceContext(resource)); } protected override void Act() { - var translator = new PostgresDuplicatedKeyExceptionTranslator(); + var translator = new PostgresDuplicatedKeyExceptionTranslator(_contextProvider); wasHandled = translator.TryTranslateMessage(exception, out actualError); } @@ -157,21 +181,23 @@ public void Should_RestError_show_single_value_message() { AssertHelper.All( () => actualError.ShouldNotBeNull(), - () => actualError.Code = 409, - () => actualError.Type = "Conflict" + () => actualError.Code.ShouldBe(409), + () => actualError.Type.ShouldBe("Conflict"), + () => actualError.Message.ShouldBe("The value supplied for property 'Property1' of entity 'Something' is not unique.") ); } } - public class When_an_insert_conflicts_with_a_unique_index_with_a_composed_key_constraint : TestFixtureBase + public class When_an_insert_or_update_conflicts_with_the_unique_index_on_a_composite_primary_key : TestFixtureBase { private Exception exception; private bool wasHandled; private RESTError actualError; + private IContextProvider _contextProvider; protected override void Arrange() { - const string message = "duplicate key value violates unique constraint \"PK_ApplicationId\""; + const string message = "duplicate key value violates unique constraint \"Something_PK\""; exception = NHibernateExceptionBuilder.CreateWrappedPostgresException( GenericSqlExceptionMessage, @@ -180,11 +206,15 @@ protected override void Arrange() null, null, null); + + _contextProvider = Stub>(); + var resource = PrepareTestResource(isCompositePrimaryKey: true); + A.CallTo(() => _contextProvider.Get()).Returns(new DataManagementResourceContext(resource)); } protected override void Act() { - var translator = new PostgresDuplicatedKeyExceptionTranslator(); + var translator = new PostgresDuplicatedKeyExceptionTranslator(_contextProvider); wasHandled = translator.TryTranslateMessage(exception, out actualError); } @@ -199,21 +229,23 @@ public void Should_RestError_show_multiple_values_message() { AssertHelper.All( () => actualError.ShouldNotBeNull(), - () => actualError.Code = 409, - () => actualError.Type = "Conflict" + () => actualError.Code.ShouldBe(409), + () => actualError.Type.ShouldBe("Conflict"), + () => actualError.Message.ShouldBe("The values supplied for properties 'Property1, Property2' of entity 'Something' are not unique.") ); } } - public class When_an_insert_conflicts_with_a_unique_index_and_metadata_is_not_provided : TestFixtureBase + public class When_an_insert_or_update_conflicts_with_a_non_pk_unique_index_and_details_are_not_available : TestFixtureBase { private Exception exception; private bool wasHandled; private RESTError actualError; + private IContextProvider _contextProvider; protected override void Arrange() { - const string message = "duplicate key value violates unique constraint \"PK_ApplicationId\""; + const string message = "duplicate key value violates unique constraint \"ux_something_property1\""; exception = NHibernateExceptionBuilder.CreateWrappedPostgresException( GenericSqlExceptionMessage, @@ -222,11 +254,63 @@ protected override void Arrange() null, null, null); + + _contextProvider = Stub>(); + } + + protected override void Act() + { + var translator = new PostgresDuplicatedKeyExceptionTranslator(_contextProvider); + wasHandled = translator.TryTranslateMessage(exception, out actualError); + } + + [Test] + public void Should_handle_exception() + { + wasHandled.ShouldBeTrue(); + } + + [Test] + public void Should_RestError_show_unknown_value_message() + { + AssertHelper.All( + () => actualError.ShouldNotBeNull(), + () => actualError.Code.ShouldBe(409), + () => actualError.Type.ShouldBe("Conflict"), + () => actualError.Message.ShouldBe("The value(s) supplied for the resource are not unique.") + ); + } + } + + public class When_an_insert_or_update_conflicts_with_a_non_pk_unique_index_and_details_are_available : TestFixtureBase + { + private Exception exception; + private bool wasHandled; + private RESTError actualError; + private IContextProvider _contextProvider; + + protected override void Arrange() + { + const string message = "duplicate key value violates unique constraint \"ux_something_property1\""; + + const string details = + "Key (property1, property2, property3)=(VAL-1, 2, It was three) already exists."; + + exception = NHibernateExceptionBuilder.CreateWrappedPostgresException( + GenericSqlExceptionMessage, + message, + PostgresSqlStates.UniqueViolation, + null, + "something", + null, + details); + + _contextProvider = Stub>(); } protected override void Act() { - var translator = new PostgresDuplicatedKeyExceptionTranslator(); + var translator = new PostgresDuplicatedKeyExceptionTranslator(_contextProvider); wasHandled = translator.TryTranslateMessage(exception, out actualError); } @@ -241,10 +325,54 @@ public void Should_RestError_show_unknown_value_message() { AssertHelper.All( () => actualError.ShouldNotBeNull(), - () => actualError.Code = 409, - () => actualError.Type = "Conflict" + () => actualError.Code.ShouldBe(409), + () => actualError.Type.ShouldBe("Conflict"), + () => actualError.Message.ShouldBe("The values supplied for properties 'property1, property2, property3' of entity 'something' are not unique.") ); } } + + private static Resource PrepareTestResource(bool isCompositePrimaryKey) + { + var definitions = new DomainModelDefinitions( + new SchemaDefinition("Ed-Fi", "edfi"), + new AggregateDefinition[] { new("edfi.Something", new FullName[] { new("edfi.Something") }) }, + new EntityDefinition[] + { + new( + "edfi", + "Something", + new EntityPropertyDefinition[] + { + new("Property1", new PropertyType(DbType.String), isIdentifying: true), + new("Property2", new PropertyType(DbType.String), isIdentifying: isCompositePrimaryKey), + new("Property3", new PropertyType(DbType.String)), + }, + new EntityIdentifierDefinition[] + { + new( + "Something_PK", + isCompositePrimaryKey + ? new[] { "Property1", "Property2" } + : new[] { "Property1" } + , + isPrimary: true) + }), + }, + Array.Empty(), + Array.Empty()); + + var definitionsProvider = A.Fake(); + A.CallTo(() => definitionsProvider.GetDomainModelDefinitions()).Returns(definitions); + + var domainModelProvider = new DomainModelProvider( + new[] { definitionsProvider }, + Array.Empty()); + + var resourceModel = new ResourceModel(domainModelProvider.GetDomainModel()); + var resource = resourceModel.GetAllResources().Single(); + + return resource; + } } -} \ No newline at end of file +} diff --git a/Application/EdFi.Ods.Tests/_Builders/NHibernateExceptionBuilder.cs b/Application/EdFi.Ods.Tests/_Builders/NHibernateExceptionBuilder.cs index 1a23784d74..9803769377 100644 --- a/Application/EdFi.Ods.Tests/_Builders/NHibernateExceptionBuilder.cs +++ b/Application/EdFi.Ods.Tests/_Builders/NHibernateExceptionBuilder.cs @@ -18,9 +18,9 @@ public static class NHibernateExceptionBuilder public static GenericADOException CreateException(string nHibernateMessage, string sqlMessage) => new GenericADOException(nHibernateMessage, CreateSqlServerException(sqlMessage, null)); - public static GenericADOException CreateWrappedPostgresException(string nHibernateMessage, string sqlMessage, string sqlState, string schemaName, string tableName, string constraintName) + public static GenericADOException CreateWrappedPostgresException(string nHibernateMessage, string sqlMessage, string sqlState, string schemaName, string tableName, string constraintName, string details = null) => new(nHibernateMessage, new PostgresException(sqlMessage, "ERROR", "ERROR", sqlState, - schemaName: schemaName, tableName: tableName, constraintName: constraintName)); + schemaName: schemaName, tableName: tableName, constraintName: constraintName, detail: details)); private static SqlException CreateSqlServerException(string message, Exception innerException) {