diff --git a/src/Core/Configurations/RuntimeConfigValidator.cs b/src/Core/Configurations/RuntimeConfigValidator.cs index c9d982a388..099bc4e379 100644 --- a/src/Core/Configurations/RuntimeConfigValidator.cs +++ b/src/Core/Configurations/RuntimeConfigValidator.cs @@ -910,6 +910,9 @@ public void ValidateEntitiesDoNotGenerateDuplicateQueriesOrMutation(DatabaseType { HashSet graphQLOperationNames = new(); + // Tracks which entity registered each operation name, used only for building conflict error messages. + Dictionary operationOwner = new(); + foreach ((string entityName, Entity entity) in entityCollection) { if (!entity.GraphQL.Enabled) @@ -917,7 +920,9 @@ public void ValidateEntitiesDoNotGenerateDuplicateQueriesOrMutation(DatabaseType continue; } + List conflictingOperationNames = new(); bool containsDuplicateOperationNames = false; + string conflictingEntityName = string.Empty; if (entity.Source.Type is EntitySourceType.StoredProcedure) { // For Stored Procedures a single query/mutation is generated. @@ -926,6 +931,12 @@ public void ValidateEntitiesDoNotGenerateDuplicateQueriesOrMutation(DatabaseType if (!graphQLOperationNames.Add(storedProcedureQueryName)) { containsDuplicateOperationNames = true; + conflictingEntityName = operationOwner.GetValueOrDefault(storedProcedureQueryName, string.Empty); + conflictingOperationNames.Add(storedProcedureQueryName); + } + else + { + operationOwner[storedProcedureQueryName] = entityName; } } else @@ -944,21 +955,52 @@ public void ValidateEntitiesDoNotGenerateDuplicateQueriesOrMutation(DatabaseType string deleteMutationName = $"delete{GraphQLNaming.GetDefinedSingularName(entityName, entity)}"; string patchMutationName = $"patch{GraphQLNaming.GetDefinedSingularName(entityName, entity)}"; - if (!graphQLOperationNames.Add(pkQueryName) - || !graphQLOperationNames.Add(listQueryName) - || !graphQLOperationNames.Add(createMutationName) - || !graphQLOperationNames.Add(updateMutationName) - || !graphQLOperationNames.Add(deleteMutationName) - || ((databaseType is DatabaseType.CosmosDB_NoSQL) && !graphQLOperationNames.Add(patchMutationName))) + List generatedOperationNames = new() { - containsDuplicateOperationNames = true; + pkQueryName, + listQueryName, + createMutationName, + updateMutationName, + deleteMutationName, + }; + + if (databaseType is DatabaseType.CosmosDB_NoSQL) + { + generatedOperationNames.Add(patchMutationName); + } + + foreach (string operationName in generatedOperationNames) + { + if (!graphQLOperationNames.Add(operationName)) + { + containsDuplicateOperationNames = true; + conflictingOperationNames.Add(operationName); + conflictingEntityName = operationOwner.GetValueOrDefault(operationName, conflictingEntityName); + } + else + { + operationOwner[operationName] = entityName; + } } } if (containsDuplicateOperationNames) { + string entitiesStr = string.IsNullOrEmpty(conflictingEntityName) + ? $" {entityName}" + : $" {conflictingEntityName}{Environment.NewLine} {entityName}"; + + string entityNamesStr = string.Join( + Environment.NewLine, + conflictingOperationNames.Select(name => $" {name}")); + + string message = $"{Environment.NewLine}GraphQL naming conflict detected." + + $"{Environment.NewLine}{Environment.NewLine}Entities:{Environment.NewLine}{entitiesStr}" + + $"{Environment.NewLine}{Environment.NewLine}The following GraphQL names are generated by more than one of these entities:{Environment.NewLine}{entityNamesStr}" + + $"{Environment.NewLine}{Environment.NewLine}Configure distinct GraphQL singular and plural names for one of the entities to resolve this conflict."; + HandleOrRecordException(new DataApiBuilderException( - message: $"Entity {entityName} generates queries/mutation that already exist", + message: message, statusCode: HttpStatusCode.ServiceUnavailable, subStatusCode: DataApiBuilderException.SubStatusCodes.ConfigValidationError)); } diff --git a/src/Service.Tests/Configuration/ConfigurationTests.cs b/src/Service.Tests/Configuration/ConfigurationTests.cs index 5ae4143730..13b17328b3 100644 --- a/src/Service.Tests/Configuration/ConfigurationTests.cs +++ b/src/Service.Tests/Configuration/ConfigurationTests.cs @@ -6292,8 +6292,12 @@ public async Task TestAutoentitiesGeneratedWithSpacesInObjectName(string tableNa [TestCategory(TestCategory.MSSQL)] [DataTestMethod] [DataRow("dbo_publishers", "uniqueSingularPublisher", "uniquePluralPublishers", "/unique/publisher", "Entity 'dbo_publishers' conflicts in autoentity pattern 'PublisherAutoEntity'. Use --patterns.exclude to skip it.", DisplayName = "Autoentities fail due to entity name")] - [DataRow("UniquePublisher", "dbo_publishers", "uniquePluralPublishers", "/unique/publisher", "Entity dbo_publishers generates queries/mutation that already exist", DisplayName = "Autoentities fail due to graphql singular type")] - [DataRow("UniquePublisher", "uniqueSingularPublisher", "dbo_publishers", "/unique/publisher", "Entity dbo_publishers generates queries/mutation that already exist", DisplayName = "Autoentities fail due to graphql plural type")] + [DataRow("UniquePublisher", "dbo_publishers", "uniquePluralPublishers", "/unique/publisher", + "\r\nGraphQL naming conflict detected.\r\n\r\nEntities:\r\n UniquePublisher\r\n dbo_publishers\r\n\r\nBoth entities generate the following GraphQL names:\r\n dbo_publishers\r\n dbo_publishers\r\n\r\nConfigure distinct GraphQL singular and plural names for one of the entities to resolve this conflict.", + DisplayName = "Autoentities fail due to graphql singular type")] + [DataRow("UniquePublisher", "uniqueSingularPublisher", "dbo_publishers", "/unique/publisher", + "\r\nGraphQL naming conflict detected.\r\n\r\nEntities:\r\n UniquePublisher\r\n dbo_publishers\r\n\r\nBoth entities generate the following GraphQL names:\r\n dbo_publishers\r\n dbo_publishers\r\n\r\nConfigure distinct GraphQL singular and plural names for one of the entities to resolve this conflict.", + DisplayName = "Autoentities fail due to graphql plural type")] [DataRow("UniquePublisher", "uniqueSingularPublisher", "uniquePluralPublishers", "/dbo_publishers", "The rest path: dbo_publishers specified for entity: dbo_publishers is already used by another entity.", DisplayName = "Autoentities fail due to rest path")] public async Task ValidateAutoentityGenerationConflicts(string entityName, string singular, string plural, string path, string exceptionMessage) { diff --git a/src/Service.Tests/UnitTests/ConfigValidationUnitTests.cs b/src/Service.Tests/UnitTests/ConfigValidationUnitTests.cs index 97ad829017..680a354729 100644 --- a/src/Service.Tests/UnitTests/ConfigValidationUnitTests.cs +++ b/src/Service.Tests/UnitTests/ConfigValidationUnitTests.cs @@ -1259,7 +1259,7 @@ public void ValidateEntitiesWithGraphQLExposedGenerateDuplicateQueries(DatabaseT { "book", book }, { "Book", bookWithUpperCase } }; - ValidateExceptionForDuplicateQueriesDueToEntityDefinitions(entityCollection, "Book", databaseType); + ValidateExceptionForDuplicateQueriesDueToEntityDefinitions(entityCollection, "Book", databaseType, "book"); } /// @@ -1302,7 +1302,7 @@ public void ValidateStoredProcedureAndTableGeneratedDuplicateQueries(DatabaseTyp { "executeBook", bookTable }, { "Book_by_pk", bookByPkStoredProcedure } }; - ValidateExceptionForDuplicateQueriesDueToEntityDefinitions(entityCollection, "executeBook", databaseType); + ValidateExceptionForDuplicateQueriesDueToEntityDefinitions(entityCollection, "executeBook", databaseType, "Book_by_pk"); } /// @@ -1346,7 +1346,7 @@ public void ValidateStoredProcedureAndTableGeneratedDuplicateMutation(DatabaseTy { "ExecuteBooks", bookTable }, { "AddBook", addBookStoredProcedure } }; - ValidateExceptionForDuplicateQueriesDueToEntityDefinitions(entityCollection, "ExecuteBooks", databaseType); + ValidateExceptionForDuplicateQueriesDueToEntityDefinitions(entityCollection, "ExecuteBooks", databaseType, "AddBook"); } /// @@ -1384,7 +1384,7 @@ public void ValidateEntitiesWithNameCollisionInGraphQLTypeGenerateDuplicateQueri { "book", book }, { "book_alt", book_alt } }; - ValidateExceptionForDuplicateQueriesDueToEntityDefinitions(entityCollection, "book_alt", databaseType); + ValidateExceptionForDuplicateQueriesDueToEntityDefinitions(entityCollection, "book_alt", databaseType, "book"); } /// @@ -1427,7 +1427,7 @@ public void ValidateEntitiesWithCollisionsInSingularPluralNamesGenerateDuplicate { "book", book }, { "book_alt", book_alt } }; - ValidateExceptionForDuplicateQueriesDueToEntityDefinitions(entityCollection, "book_alt", databaseType); + ValidateExceptionForDuplicateQueriesDueToEntityDefinitions(entityCollection, "book_alt", databaseType, "book"); } /// @@ -1465,7 +1465,45 @@ public void ValidateEntitiesWithNameCollisionInSingularPluralTypeGeneratesDuplic entityCollection.Add("book_alt", book_alt); entityCollection.Add("book", book); - ValidateExceptionForDuplicateQueriesDueToEntityDefinitions(entityCollection, "book_alt", databaseType); + ValidateExceptionForDuplicateQueriesDueToEntityDefinitions(entityCollection, "book_alt", databaseType, "book"); + } + + /// + /// Validates that a detailed error is thrown when autoentities includes objects whose names + /// differ only by singular/plural form (e.g. dbo.Category and dbo.Categories), causing + /// DAB to generate conflicting GraphQL type and operation names. + /// + /// "dbo_Category" entity → singular: Category, plural: Categories + /// "dbo_Categories" entity → singular: Category, plural: Categories (after pluralization) + /// + /// Both entities generate the same pk query, list query, and mutation names. + /// + [TestMethod] + [DataRow(DatabaseType.MSSQL)] // Relational Database + [DataRow(DatabaseType.CosmosDB_NoSQL)] // Non Relational Database + public void ValidateAutoEntitiesWithSingularPluralNameCollisionGenerateDuplicateQueries(DatabaseType databaseType) + { + // Entity Name: dbo_Category + // Singular: Category (from entity name processed by autoentities) + // Plural: Categories (pluralized from singular) + Entity categoryEntity = GraphQLTestHelpers.GenerateEntityWithSingularPlural("Category", "Categories"); + + // Entity Name: dbo_Categories + // Singular: Category (after singularization by autoentities) + // Plural: Categories + Entity categoriesEntity = GraphQLTestHelpers.GenerateEntityWithSingularPlural("Category", "Categories"); + + SortedDictionary entityCollection = new() + { + { "dbo_Categories", categoriesEntity }, + { "dbo_Category", categoryEntity } + }; + + ValidateExceptionForDuplicateQueriesDueToEntityDefinitions( + entityCollection, + "dbo_Category", + databaseType, + conflictingEntityName: "dbo_Categories"); } /// @@ -1612,14 +1650,22 @@ public void TestGlobalRouteValidation(string graphQLConfiguredPath, string restC /// queries with the same name. /// /// Entity definitions - /// Entity name to construct the expected exception message - private static void ValidateExceptionForDuplicateQueriesDueToEntityDefinitions(SortedDictionary entityCollection, string entityName, DatabaseType databaseType) + /// The entity name expected to appear in the conflict message as the conflicting entity. + /// Database type used during validation. + /// The other entity name expected to appear in the conflict message. + private static void ValidateExceptionForDuplicateQueriesDueToEntityDefinitions( + SortedDictionary entityCollection, + string entityName, + DatabaseType databaseType, + string conflictingEntityName) { RuntimeConfigValidator configValidator = InitializeRuntimeConfigValidator(); DataApiBuilderException dabException = Assert.ThrowsException( action: () => configValidator.ValidateEntitiesDoNotGenerateDuplicateQueriesOrMutation(databaseType, new(entityCollection))); - Assert.AreEqual(expected: $"Entity {entityName} generates queries/mutation that already exist", actual: dabException.Message); + StringAssert.Contains(dabException.Message, "GraphQL naming conflict detected."); + StringAssert.Contains(dabException.Message, entityName); + StringAssert.Contains(dabException.Message, conflictingEntityName); Assert.AreEqual(expected: HttpStatusCode.ServiceUnavailable, actual: dabException.StatusCode); Assert.AreEqual(expected: DataApiBuilderException.SubStatusCodes.ConfigValidationError, actual: dabException.SubStatusCode); } @@ -2464,6 +2510,56 @@ public void ValidateUserDelegatedAuth_ValidConfiguration_Succeeds() } } + /// + /// Regression test for validate-only mode losing operation ownership after an earlier conflict. + /// When an entity successfully registers some of its generated operation names but then fails on + /// a later name, the previously added names must still be attributed to that entity so that a + /// subsequent entity colliding on one of them reports the correct conflicting entity. + /// + /// Sequence: + /// - First: singular "Alpha", plural "Shared" -> registers alpha_by_pk, shared, ... + /// - Second: singular "Beta", plural "Shared" -> registers beta_by_pk, then collides on shared (owned by First). + /// - Third: singular "Beta", plural "Thirds" -> collides on beta_by_pk, which must be owned by Second. + /// + /// Because validate-only mode records exceptions instead of throwing, both conflicts are collected. + /// The conflict recorded for Third must identify Second (not just Third). + /// + [TestMethod] + public void ValidateOnlyMode_ConflictAfterPartialAdd_ReportsActualConflictingEntity() + { + Entity first = GraphQLTestHelpers.GenerateEntityWithSingularPlural("Alpha", "Shared"); + Entity second = GraphQLTestHelpers.GenerateEntityWithSingularPlural("Beta", "Shared"); + Entity third = GraphQLTestHelpers.GenerateEntityWithSingularPlural("Beta", "Thirds"); + + SortedDictionary entityCollection = new() + { + { "First", first }, + { "Second", second }, + { "Third", third } + }; + + MockFileSystem fileSystem = new(); + FileSystemRuntimeConfigLoader loader = new(fileSystem); + RuntimeConfigProvider provider = new(loader); + RuntimeConfigValidator configValidator = new( + provider, + fileSystem, + new Mock>().Object, + isValidateOnly: true); + + configValidator.ValidateEntitiesDoNotGenerateDuplicateQueriesOrMutation(DatabaseType.MySQL, new(entityCollection)); + + List exceptions = configValidator.ConfigValidationExceptions; + + // Two conflicts are expected: Second (vs First) and Third (vs Second). + Assert.AreEqual(expected: 2, actual: exceptions.Count); + + // Regression assertion: the conflict recorded for Third must identify Second as the entity + // that first registered beta_by_pk, even though Second failed on a later operation (shared). + Exception thirdConflict = exceptions.Single(e => e.Message.Contains("Third")); + StringAssert.Contains(thirdConflict.Message, "Second"); + } + /// /// Test to validate that user-delegated-auth with missing DAB_OBO_CLIENT_ID, DAB_OBO_TENANT_ID, /// or DAB_OBO_CLIENT_SECRET throws an error.