diff --git a/src/Core/Services/OpenAPI/OpenApiDocumentor.cs b/src/Core/Services/OpenAPI/OpenApiDocumentor.cs index c3ccf7b47f..2aaba3eed3 100644 --- a/src/Core/Services/OpenAPI/OpenApiDocumentor.cs +++ b/src/Core/Services/OpenAPI/OpenApiDocumentor.cs @@ -201,7 +201,9 @@ private OpenApiDocument BuildOpenApiDocument(RuntimeConfig runtimeConfig, string Schemas = CreateComponentSchemas(runtimeConfig.Entities, runtimeConfig.DefaultDataSourceName, role, isRequestBodyStrict: runtimeConfig.IsRequestBodyStrict) }; - List globalTags = new(); + // Store tags in a dictionary keyed by normalized REST path to ensure we can + // reuse the same tag instances in BuildPaths, preventing duplicate groups in Swagger UI. + Dictionary globalTagsDict = new(); foreach (KeyValuePair kvp in runtimeConfig.Entities) { Entity entity = kvp.Value; @@ -210,8 +212,12 @@ private OpenApiDocument BuildOpenApiDocument(RuntimeConfig runtimeConfig, string continue; } - string restPath = entity.Rest?.Path ?? kvp.Key; - globalTags.Add(new OpenApiTag + // Use GetEntityRestPath to ensure consistent path normalization (with leading slash trimmed) + // matching the same computation used in BuildPaths. + string restPath = GetEntityRestPath(entity.Rest, kvp.Key); + + // First entity's description wins when multiple entities share the same REST path. + globalTagsDict.TryAdd(restPath, new OpenApiTag { Name = restPath, Description = string.IsNullOrWhiteSpace(entity.Description) ? null : entity.Description @@ -229,9 +235,9 @@ private OpenApiDocument BuildOpenApiDocument(RuntimeConfig runtimeConfig, string { new() { Url = url } }, - Paths = BuildPaths(runtimeConfig.Entities, runtimeConfig.DefaultDataSourceName, role), + Paths = BuildPaths(runtimeConfig.Entities, runtimeConfig.DefaultDataSourceName, globalTagsDict, role), Components = components, - Tags = globalTags + Tags = globalTagsDict.Values.ToList() }; } @@ -291,9 +297,10 @@ public void CreateDocument(bool doOverrideExistingDocument = false) /// A path with no primary key nor parameter representing the primary key value: /// "/EntityName" /// + /// Dictionary of global tags keyed by normalized REST path for reuse. /// Optional role to filter permissions. If null, returns superset of all roles. /// All possible paths in the DAB engine's REST API endpoint. - private OpenApiPaths BuildPaths(RuntimeEntities entities, string defaultDataSourceName, string? role = null) + private OpenApiPaths BuildPaths(RuntimeEntities entities, string defaultDataSourceName, Dictionary globalTags, string? role = null) { OpenApiPaths pathsCollection = new(); @@ -301,54 +308,48 @@ private OpenApiPaths BuildPaths(RuntimeEntities entities, string defaultDataSour foreach (KeyValuePair entityDbMetadataMap in metadataProvider.EntityToDatabaseObject) { string entityName = entityDbMetadataMap.Key; - if (!entities.ContainsKey(entityName)) + if (!entities.TryGetValue(entityName, out Entity? entity) || entity is null) { // This can happen for linking entities which are not present in runtime config. continue; } - string entityRestPath = GetEntityRestPath(entities[entityName].Rest, entityName); - string entityBasePathComponent = $"/{entityRestPath}"; - - DatabaseObject dbObject = entityDbMetadataMap.Value; - SourceDefinition sourceDefinition = metadataProvider.GetSourceDefinition(entityName); - // Entities which disable their REST endpoint must not be included in // the OpenAPI description document. - if (entities.TryGetValue(entityName, out Entity? entity) && entity is not null) - { - if (!entity.Rest.Enabled) - { - continue; - } - } - else + if (!entity.Rest.Enabled) { continue; } - // Set the tag's Description property to the entity's semantic description if present. - OpenApiTag openApiTag = new() - { - Name = entityRestPath, - Description = string.IsNullOrWhiteSpace(entity.Description) ? null : entity.Description - }; + string entityRestPath = GetEntityRestPath(entity.Rest, entityName); + string entityBasePathComponent = $"/{entityRestPath}"; - // The OpenApiTag will categorize all paths created using the entity's name or overridden REST path value. - // The tag categorization will instruct OpenAPI document visualization tooling to display all generated paths together. - List tags = new() - { - openApiTag - }; + DatabaseObject dbObject = entityDbMetadataMap.Value; + SourceDefinition sourceDefinition = metadataProvider.GetSourceDefinition(entityName); Dictionary configuredRestOperations = GetConfiguredRestOperations(entity, dbObject, role); - // Skip entities with no available operations + // Skip entities with no available operations before looking up the tag. + // This prevents noisy warnings for entities that are legitimately excluded from + // the global tags dictionary due to role-based permission filtering. if (!configuredRestOperations.ContainsValue(true)) { continue; } + // Reuse the existing tag from the global tags dictionary instead of creating a new instance. + // This ensures Swagger UI displays only one group per entity by using the same object reference. + if (!globalTags.TryGetValue(entityRestPath, out OpenApiTag? existingTag)) + { + _logger.LogWarning("Tag for REST path '{EntityRestPath}' not found in global tags dictionary. This indicates a key mismatch between BuildOpenApiDocument and BuildPaths.", entityRestPath); + continue; + } + + List tags = new() + { + existingTag + }; + if (dbObject.SourceType is EntitySourceType.StoredProcedure) { Dictionary operations = CreateStoredProcedureOperations( diff --git a/src/Service.Tests/OpenApiDocumentor/TagValidationTests.cs b/src/Service.Tests/OpenApiDocumentor/TagValidationTests.cs new file mode 100644 index 0000000000..4c8e3b0216 --- /dev/null +++ b/src/Service.Tests/OpenApiDocumentor/TagValidationTests.cs @@ -0,0 +1,266 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +using System.Collections.Generic; +using System.Linq; +using System.Threading.Tasks; +using Azure.DataApiBuilder.Config.ObjectModel; +using Microsoft.OpenApi.Models; +using Microsoft.VisualStudio.TestTools.UnitTesting; + +namespace Azure.DataApiBuilder.Service.Tests.OpenApiIntegration +{ + /// + /// Integration tests validating that OpenAPI tags are correctly deduplicated + /// and shared between global document tags and operation-level tags. + /// Covers bug fix for duplicate entity groups in Swagger UI (#2968). + /// + [TestCategory(TestCategory.MSSQL)] + [TestClass] + public class TagValidationTests + { + private const string CONFIG_FILE = "tag-validation-config.MsSql.json"; + private const string DB_ENV = TestCategory.MSSQL; + + /// + /// Validates no duplicate tags and shared tag instances across various entity configurations. + /// Exercises: + /// - Multiple entities (one with description, one without) + /// - Leading slash in REST path + /// - Default REST path (entity name as path) + /// - Stored procedure entity + /// + /// Entity name. + /// REST path override (null means use entity name). + /// Entity description (null means no description). + /// Source type: Table or StoredProcedure. + /// Database source object name. + [DataRow("book", null, "A book entity", EntitySourceType.Table, "books", + DisplayName = "Table entity with description and default REST path")] + [DataRow("author", null, null, EntitySourceType.Table, "authors", + DisplayName = "Table entity without description and default REST path")] + [DataRow("genre", "/Genre", "Genre entity", EntitySourceType.Table, "brokers", + DisplayName = "Table entity with leading slash REST path and description")] + [DataRow("sp_entity", null, "SP description", EntitySourceType.StoredProcedure, "insert_and_display_all_books_for_given_publisher", + DisplayName = "Stored procedure entity with description")] + [DataTestMethod] + public async Task NoDuplicateTags_AndSharedInstances( + string entityName, + string configuredRestPath, + string description, + EntitySourceType sourceType, + string sourceObject) + { + // Arrange: Create a multi-entity configuration. + // Always include a secondary entity so we exercise multi-entity deduplication. + Entity primaryEntity = CreateEntity(sourceObject, sourceType, configuredRestPath, description); + Entity secondaryEntity = CreateEntity("publishers", EntitySourceType.Table, null, "Secondary entity for dedup test"); + + Dictionary entities = new() + { + { entityName, primaryEntity }, + { "publisher", secondaryEntity } + }; + + RuntimeEntities runtimeEntities = new(entities); + OpenApiDocument doc = await OpenApiTestBootstrap.GenerateOpenApiDocumentAsync( + runtimeEntities: runtimeEntities, + configFileName: CONFIG_FILE, + databaseEnvironment: DB_ENV); + + // Assert: No duplicate tag names in global tags + List tagNames = doc.Tags.Select(t => t.Name).ToList(); + List distinctTagNames = tagNames.Distinct().ToList(); + Assert.AreEqual(distinctTagNames.Count, tagNames.Count, + $"Duplicate tags found in OpenAPI document. Tags: {string.Join(", ", tagNames)}"); + + // Assert: The expected REST path (normalized, no leading slash) is present as a tag + string expectedTagName = configuredRestPath?.TrimStart('/') ?? entityName; + Assert.IsTrue(doc.Tags.Any(t => t.Name == expectedTagName), + $"Expected tag '{expectedTagName}' not found. Actual tags: {string.Join(", ", tagNames)}"); + + // Assert: All operation tags reference the same instance as global tags + AssertOperationTagsAreSharedInstances(doc); + } + + // Note: A test for duplicate REST paths (e.g., two entities both mapped to "/SharedPath") is intentionally + // omitted because RuntimeConfigValidator rejects duplicate REST paths at startup (see RuntimeConfigValidator + // line ~685). The TryAdd in BuildOpenApiDocument is defensive code for this edge case, but it cannot be + // exercised through integration tests since the server won't start with an invalid configuration. + + /// + /// Validates REST-disabled entities produce no tags and no paths. + /// + [TestMethod] + public async Task RestDisabledEntity_ProducesNoTagOrPath() + { + Entity disabledEntity = new( + Source: new("books", EntitySourceType.Table, null, null), + Fields: null, + GraphQL: new(Singular: null, Plural: null, Enabled: false), + Rest: new(Methods: EntityRestOptions.DEFAULT_SUPPORTED_VERBS, Path: null, Enabled: false), + Permissions: OpenApiTestBootstrap.CreateBasicPermissions(), + Mappings: null, + Relationships: null, + Description: "Should not appear"); + + Entity enabledEntity = CreateEntity("publishers", EntitySourceType.Table, null, "Enabled entity"); + + Dictionary entities = new() + { + { "disabled_book", disabledEntity }, + { "publisher", enabledEntity } + }; + + RuntimeEntities runtimeEntities = new(entities); + OpenApiDocument doc = await OpenApiTestBootstrap.GenerateOpenApiDocumentAsync( + runtimeEntities: runtimeEntities, + configFileName: CONFIG_FILE, + databaseEnvironment: DB_ENV); + + Assert.IsFalse(doc.Tags.Any(t => t.Name == "disabled_book"), + "REST-disabled entity should not have a tag in the OpenAPI document."); + Assert.IsFalse(doc.Paths.Any(p => p.Key.Contains("disabled_book")), + "REST-disabled entity should not have paths in the OpenAPI document."); + Assert.IsTrue(doc.Tags.Any(t => t.Name == "publisher"), + "Enabled entity should still have a tag."); + + AssertOperationTagsAreSharedInstances(doc); + } + + /// + /// Validates that entities with no permissions produce no tag when viewed + /// for a specific role that has no access. + /// + [TestMethod] + public async Task EntityWithNoPermissionsForRole_ProducesNoTag() + { + EntityPermission[] permissions = new[] + { + new EntityPermission(Role: "admin", Actions: new[] + { + new EntityAction(EntityActionOperation.All, null, new()) + }) + }; + + Entity entity = new( + Source: new("books", EntitySourceType.Table, null, null), + Fields: null, + GraphQL: new(Singular: null, Plural: null, Enabled: false), + Rest: new(Methods: EntityRestOptions.DEFAULT_SUPPORTED_VERBS), + Permissions: permissions, + Mappings: null, + Relationships: null, + Description: "Admin-only entity"); + + Entity publicEntity = CreateEntity("publishers", EntitySourceType.Table, null, "Public entity"); + + Dictionary entities = new() + { + { "book", entity }, + { "publisher", publicEntity } + }; + + RuntimeEntities runtimeEntities = new(entities); + + // Request OpenAPI doc for "anonymous" role - book should not appear + OpenApiDocument doc = await OpenApiTestBootstrap.GenerateOpenApiDocumentAsync( + runtimeEntities: runtimeEntities, + configFileName: CONFIG_FILE, + databaseEnvironment: DB_ENV, + role: "anonymous"); + + Assert.IsFalse(doc.Tags.Any(t => t.Name == "book"), + "Entity with no permissions for 'anonymous' role should not have a tag."); + Assert.IsFalse(doc.Paths.Any(p => p.Key.Contains("book")), + "Entity with no permissions for 'anonymous' role should not have paths."); + + AssertOperationTagsAreSharedInstances(doc); + } + + /// + /// Validates that entity descriptions are correctly reflected in OpenAPI tags. + /// + /// Entity description to test. + /// Whether the tag should have a description. + [DataRow("A meaningful description", true, DisplayName = "Entity with description")] + [DataRow(null, false, DisplayName = "Entity without description")] + [DataRow("", false, DisplayName = "Entity with empty description")] + [DataRow(" ", false, DisplayName = "Entity with whitespace description")] + [DataTestMethod] + public async Task TagDescription_MatchesEntityDescription(string description, bool shouldHaveDescription) + { + Entity entity = CreateEntity("books", EntitySourceType.Table, null, description); + + Dictionary entities = new() + { + { "book", entity } + }; + + RuntimeEntities runtimeEntities = new(entities); + OpenApiDocument doc = await OpenApiTestBootstrap.GenerateOpenApiDocumentAsync( + runtimeEntities: runtimeEntities, + configFileName: CONFIG_FILE, + databaseEnvironment: DB_ENV); + + OpenApiTag tag = doc.Tags.FirstOrDefault(t => t.Name == "book"); + Assert.IsNotNull(tag, "Expected tag 'book' to exist."); + + if (shouldHaveDescription) + { + Assert.AreEqual(description, tag.Description, + $"Tag description should match entity description."); + } + else + { + Assert.IsNull(tag.Description, + "Tag description should be null for empty/whitespace/null entity descriptions."); + } + } + + /// + /// Asserts that every operation tag in the document is the exact same object instance + /// as the corresponding tag in the global Tags list. This prevents Swagger UI from + /// treating them as separate groups. + /// + /// OpenAPI document to validate. + private static void AssertOperationTagsAreSharedInstances(OpenApiDocument doc) + { + foreach (KeyValuePair path in doc.Paths) + { + foreach (KeyValuePair operation in path.Value.Operations) + { + foreach (OpenApiTag operationTag in operation.Value.Tags) + { + bool isSharedInstance = doc.Tags.Any(globalTag => ReferenceEquals(globalTag, operationTag)); + Assert.IsTrue(isSharedInstance, + $"Operation tag '{operationTag.Name}' at path '{path.Key}' ({operation.Key}) " + + $"is not the same instance as the global tag. This will cause duplicate groups in Swagger UI."); + } + } + } + } + + /// + /// Helper to create an Entity with common defaults for tag validation tests. + /// + private static Entity CreateEntity( + string sourceObject, + EntitySourceType sourceType, + string configuredRestPath, + string description) + { + return new Entity( + Source: new(sourceObject, sourceType, null, null), + Fields: null, + GraphQL: new(Singular: null, Plural: null, Enabled: false), + Rest: sourceType == EntitySourceType.StoredProcedure + ? new(Methods: EntityRestOptions.DEFAULT_SUPPORTED_VERBS, Path: configuredRestPath) + : new(Methods: EntityRestOptions.DEFAULT_SUPPORTED_VERBS, Path: configuredRestPath), + Permissions: OpenApiTestBootstrap.CreateBasicPermissions(), + Mappings: null, + Relationships: null, + Description: description); + } + } +}