From 54fb1b115fa85e9c3fa4e985f65c9263b3baf159 Mon Sep 17 00:00:00 2001 From: Joymax Andrade do Nascimento Date: Fri, 4 Sep 2026 17:29:33 -0300 Subject: [PATCH 01/12] Honor field permissions when reading table schema --- .../MetadataProviders/SqlMetadataProvider.cs | 349 +++++++++++++++++- 1 file changed, 346 insertions(+), 3 deletions(-) diff --git a/src/Core/Services/MetadataProviders/SqlMetadataProvider.cs b/src/Core/Services/MetadataProviders/SqlMetadataProvider.cs index 9517c41781..6b31d7c700 100644 --- a/src/Core/Services/MetadataProviders/SqlMetadataProvider.cs +++ b/src/Core/Services/MetadataProviders/SqlMetadataProvider.cs @@ -69,6 +69,11 @@ public abstract class SqlMetadataProvider : protected const int NUMBER_OF_RESTRICTIONS = 4; + /// + /// Wildcard used in the permissions "fields" section to denote every field. + /// + private const string FIELD_WILDCARD = "*"; + protected string ConnectionString { get; init; } protected IQueryBuilder SqlQueryBuilder { get; init; } @@ -1528,10 +1533,26 @@ private async Task PopulateSourceDefinitionAsync( using DataTableReader reader = new(dataTable); DataTable schemaTable = reader.GetSchemaTable(); RuntimeConfig runtimeConfig = _runtimeConfigProvider.GetConfig(); + + // Columns the entity's permissions allow to be read. The schema DataTable is cached + // per schema.table and may therefore carry columns permitted only for a sibling + // entity, so the restriction is re-applied per entity here. + PermittedColumns? permittedColumns = entity is null + ? null + : ResolvePermittedColumnsForEntity(entity); + foreach (DataRow columnInfoFromAdapter in schemaTable.Rows) { string columnName = columnInfoFromAdapter["ColumnName"].ToString()!; + if (permittedColumns is not null + && !permittedColumns.IsUnrestricted + && !permittedColumns.IsColumnPermitted(columnName) + && !sourceDefinition.PrimaryKey.Contains(columnName)) + { + continue; + } + if (runtimeConfig.IsGraphQLEnabled && entity is not null && IsGraphQLReservedName(entity, columnName, graphQLEnabledGlobally: runtimeConfig.IsGraphQLEnabled)) @@ -1685,7 +1706,7 @@ private async Task GetTableWithSchemaFromDataSetAsync( { try { - dataTable = await FillSchemaForTableAsync(schemaName, tableName); + dataTable = await FillSchemaForTableAsync(schemaName, tableName, entityName); } catch (Exception ex) when (ex is not DataApiBuilderException) { @@ -1753,10 +1774,16 @@ private async Task ValidateDatabaseConnection() /// /// Using a data adapter, obtains the schema of the given table name /// and adds the corresponding DataTable to the entities data set. + /// When the entities backed by this database object restrict the readable + /// fields through permissions ("fields.include"/"fields.exclude"), the projection + /// is narrowed to those columns instead of "SELECT *". This avoids the provider + /// having to materialize CLR types it cannot handle (e.g. geometry/geography/hierarchyid), + /// which otherwise fails during schema discovery even though the column is not exposed. /// private async Task FillSchemaForTableAsync( string schemaName, - string tableName) + string tableName, + string? entityName = null) { using ConnectionT conn = new(); // If connection string is set to empty string @@ -1802,14 +1829,330 @@ private async Task FillSchemaForTableAsync( }; string tableNameWithSchemaPrefix = GetTableNameWithSchemaPrefix(schemaName, tableName); + + // Resolve the columns the configuration actually allows to be read for this + // database object. When nothing is restricted, the original SELECT * is preserved. + PermittedColumns permittedColumns = ResolvePermittedColumnsForDatabaseObject( + schemaName: schemaName, + tableName: tableName, + entityName: entityName); + + string projection = await BuildSchemaProjectionAsync(schemaName, tableName, permittedColumns); + selectCommand.CommandText - = $"SELECT * FROM {tableNameWithSchemaPrefix}"; + = $"SELECT {projection} FROM {tableNameWithSchemaPrefix}"; adapterForTable.SelectCommand = selectCommand; DataTable[] dataTable = adapterForTable.FillSchema(EntitiesDataSet, SchemaType.Source, tableNameWithSchemaPrefix); return dataTable[0]; } + /// + /// Describes which backing (database) columns of a database object the runtime + /// configuration allows to be read. + /// + /// + /// True when at least one permission reads every field ("fields" absent, "include" absent, + /// or "include": ["*"]). + /// + /// Backing columns explicitly listed in an "include" section. + /// + /// Backing columns excluded from every wildcard permission and never explicitly included. + /// + private sealed record PermittedColumns(bool AllColumns, HashSet Included, HashSet Excluded) + { + /// + /// No column restriction could be derived from the configuration. + /// + public bool IsUnrestricted => AllColumns && Excluded.Count == 0; + + /// + /// Whether the given backing column is readable per the configuration. + /// + public bool IsColumnPermitted(string columnName) + { + if (Included.Contains(columnName)) + { + return true; + } + + if (Excluded.Contains(columnName)) + { + return false; + } + + return AllColumns; + } + } + + /// + /// Resolves the readable columns for a database object. + /// Because the schema DataTable is cached per schema.table, the result combines the + /// permissions of every entity in this data source backed by that same object: a column + /// has to be read if any of those entities can read it. + /// + /// Schema of the database object. + /// Name of the database object. + /// Entity that triggered the schema discovery, when known. + private PermittedColumns ResolvePermittedColumnsForDatabaseObject( + string schemaName, + string tableName, + string? entityName) + { + HashSet included = new(StringComparer.Ordinal); + HashSet? excluded = null; + bool allColumns = false; + bool matchedAnyEntity = false; + + foreach ((string candidateEntityName, Entity candidateEntity) in Entities) + { + // Only consider entities backed by the same database object. + if (EntityToDatabaseObject.TryGetValue(candidateEntityName, out DatabaseObject? databaseObject)) + { + if (!string.Equals(databaseObject.SchemaName, schemaName, StringComparison.OrdinalIgnoreCase) + || !string.Equals(databaseObject.Name, tableName, StringComparison.OrdinalIgnoreCase)) + { + continue; + } + } + else if (!string.Equals(candidateEntityName, entityName, StringComparison.Ordinal)) + { + // Database object not inferred yet: only the entity that triggered the read applies. + continue; + } + + matchedAnyEntity = true; + PermittedColumns entityPermittedColumns = ResolvePermittedColumnsForEntity(candidateEntity); + + included.UnionWith(entityPermittedColumns.Included); + + if (entityPermittedColumns.AllColumns) + { + allColumns = true; + + // A column is only droppable when every wildcard permission excludes it. + if (excluded is null) + { + excluded = new(entityPermittedColumns.Excluded, StringComparer.Ordinal); + } + else + { + excluded.IntersectWith(entityPermittedColumns.Excluded); + } + } + } + + if (!matchedAnyEntity) + { + return new(AllColumns: true, Included: new(StringComparer.Ordinal), Excluded: new(StringComparer.Ordinal)); + } + + excluded ??= new(StringComparer.Ordinal); + excluded.ExceptWith(included); + + return new(allColumns, included, excluded); + } + + /// + /// Resolves the readable columns of a single entity from the "fields.include" / + /// "fields.exclude" sections of its permissions. Exposed names (mappings and field + /// aliases) are translated back to their database column names, and configured primary + /// key fields are always kept, since the runtime cannot operate on the entity without them. + /// + private static PermittedColumns ResolvePermittedColumnsForEntity(Entity entity) + { + HashSet included = new(StringComparer.Ordinal); + HashSet? excluded = null; + bool allColumns = false; + + // Exposed name -> backing column name. + Dictionary exposedToBackingName = new(StringComparer.Ordinal); + + if (entity.Mappings is not null) + { + foreach ((string backingName, string exposedName) in entity.Mappings) + { + if (!string.IsNullOrWhiteSpace(exposedName)) + { + exposedToBackingName[exposedName] = backingName; + } + } + } + + if (entity.Fields is not null) + { + foreach (FieldMetadata field in entity.Fields) + { + if (!string.IsNullOrWhiteSpace(field.Alias)) + { + exposedToBackingName[field.Alias!] = field.Name; + } + } + } + + if (entity.Permissions is null || entity.Permissions.Length == 0) + { + // Nothing configured: the whole object is read, as before. + return new(AllColumns: true, included, new HashSet(StringComparer.Ordinal)); + } + + foreach (EntityPermission permission in entity.Permissions) + { + if (permission.Actions is null) + { + allColumns = true; + excluded = new(StringComparer.Ordinal); + continue; + } + + foreach (EntityAction action in permission.Actions) + { + EntityActionFields? fields = action.Fields; + + HashSet actionExcluded = new(StringComparer.Ordinal); + if (fields?.Exclude is not null) + { + if (fields.Exclude.Contains(FIELD_WILDCARD)) + { + // This permission reads no field at all, so it contributes no column. + continue; + } + + foreach (string field in fields.Exclude) + { + actionExcluded.Add(ResolveBackingName(field, exposedToBackingName)); + } + } + + // No "fields" section, no "include" section, or an explicit wildcard, + // means every column not listed in "exclude" is readable. + bool includesEveryField = fields is null + || fields.Include is null + || fields.Include.Contains(FIELD_WILDCARD); + + if (includesEveryField) + { + allColumns = true; + + if (excluded is null) + { + excluded = actionExcluded; + } + else + { + excluded.IntersectWith(actionExcluded); + } + + continue; + } + + foreach (string field in fields!.Include!) + { + string backingName = ResolveBackingName(field, exposedToBackingName); + if (!actionExcluded.Contains(backingName)) + { + included.Add(backingName); + } + } + } + } + + // Primary keys are structural: never drop them from the projection. + if (entity.Source is not null && entity.Source.KeyFields is not null) + { + foreach (string keyField in entity.Source.KeyFields) + { + included.Add(ResolveBackingName(keyField, exposedToBackingName)); + } + } + + if (entity.Fields is not null) + { + foreach (FieldMetadata field in entity.Fields.Where(f => f.PrimaryKey)) + { + included.Add(ResolveBackingName(field.Name, exposedToBackingName)); + } + } + + excluded ??= new(StringComparer.Ordinal); + excluded.ExceptWith(included); + + return new(allColumns, included, excluded); + } + + /// + /// Translates a configured (exposed) field name into its backing column name. + /// + private static string ResolveBackingName(string fieldName, Dictionary exposedToBackingName) + { + return exposedToBackingName.TryGetValue(fieldName, out string? backingName) ? backingName : fieldName; + } + + /// + /// Builds the projection used to read the schema of a database object, narrowed to the + /// columns the configuration allows to be read. Returns "*" when no restriction applies + /// or when the column list cannot be determined. + /// + private async Task BuildSchemaProjectionAsync( + string schemaName, + string tableName, + PermittedColumns permittedColumns) + { + if (permittedColumns.IsUnrestricted) + { + return "*"; + } + + List columnsToRead; + + if (permittedColumns.AllColumns) + { + // "include": ["*"] with an "exclude" list: enumerate the columns from the catalog + // (metadata only, so unsupported CLR types are never materialized) and drop the + // excluded ones. + List allColumnNames = new(); + try + { + DataTable columnsInTable = await GetColumnsAsync(schemaName, tableName); + + foreach (DataRow columnInfo in columnsInTable.Rows) + { + if (columnInfo["COLUMN_NAME"] is string columnName) + { + allColumnNames.Add(columnName); + } + } + } + catch (Exception ex) + { + _logger.LogDebug( + "Unable to enumerate the columns of {schemaName}.{tableName} to honor the configured field exclusions: {message}", + schemaName, + tableName, + ex.Message); + return "*"; + } + + if (allColumnNames.Count == 0) + { + return "*"; + } + + columnsToRead = allColumnNames.Where(permittedColumns.IsColumnPermitted).ToList(); + } + else + { + columnsToRead = permittedColumns.Included.ToList(); + } + + if (columnsToRead.Count == 0) + { + return "*"; + } + + return string.Join(", ", columnsToRead.Select(column => SqlQueryBuilder.QuoteIdentifier(column))); + } + /// /// Gets the correctly formatted table name with schema as prefix, if one exists. /// A schema prefix is simply the correctly formatted and prefixed schema name that From c330065e9b0d3de3d5599f24bccdc96425d05007 Mon Sep 17 00:00:00 2001 From: Joymax Andrade do Nascimento Date: Fri, 4 Sep 2026 21:13:00 -0300 Subject: [PATCH 02/12] Add MSSQL fixture and test for column excluded by field permissions --- config-generators/mssql-commands.txt | 1 + src/Service.Tests/DatabaseSchema-MsSql.sql | 14 ++++++++ ...tReadingRuntimeConfigForMsSql.verified.txt | 32 ++++++++++++++++++ .../UnitTests/SqlMetadataProviderUnitTests.cs | 32 ++++++++++++++++++ src/Service.Tests/dab-config.MsSql.json | 33 +++++++++++++++++++ 5 files changed, 112 insertions(+) diff --git a/config-generators/mssql-commands.txt b/config-generators/mssql-commands.txt index 277b9878f0..0b1fe8ca1d 100644 --- a/config-generators/mssql-commands.txt +++ b/config-generators/mssql-commands.txt @@ -22,6 +22,7 @@ add VectorType --config "dab-config.MsSql.json" --source vector_type_table --res update VectorType --config "dab-config.MsSql.json" --permissions "authenticated:create,read,delete,update" update VectorOwner --config "dab-config.MsSql.json" --relationship vectors --target.entity VectorType --cardinality many --relationship.fields "id:owner_id" update VectorType --config "dab-config.MsSql.json" --relationship owner --target.entity VectorOwner --cardinality one --relationship.fields "owner_id:id" +add GeometryType --config "dab-config.MsSql.json" --source geometry_type_table --rest true --graphql true --permissions "anonymous:read" --fields.include "id,name" add Profile --config "dab-config.MsSql.json" --source profiles --rest true --graphql true --permissions "anonymous:create,read,delete,update" update Profile --config "dab-config.MsSql.json" --permissions "authenticated:create,read,delete,update" add stocks_price --config "dab-config.MsSql.json" --source stocks_price --permissions "authenticated:create,read,update,delete" diff --git a/src/Service.Tests/DatabaseSchema-MsSql.sql b/src/Service.Tests/DatabaseSchema-MsSql.sql index 587edb29c3..ce63144076 100644 --- a/src/Service.Tests/DatabaseSchema-MsSql.sql +++ b/src/Service.Tests/DatabaseSchema-MsSql.sql @@ -44,6 +44,7 @@ DROP TABLE IF EXISTS brokers; DROP TABLE IF EXISTS type_table; DROP TABLE IF EXISTS vector_type_table; DROP TABLE IF EXISTS vector_owners; +DROP TABLE IF EXISTS geometry_type_table; DROP TABLE IF EXISTS profiles; DROP TABLE IF EXISTS trees; DROP TABLE IF EXISTS fungi; @@ -252,6 +253,12 @@ CREATE TABLE vector_type_table( CONSTRAINT FK_vector_type_table_owner FOREIGN KEY (owner_id) REFERENCES vector_owners(id) ON DELETE CASCADE ); +CREATE TABLE geometry_type_table( + id int IDENTITY(5001, 1) PRIMARY KEY, + name varchar(100) NOT NULL, + geom geometry NULL +); + CREATE TABLE profiles( id int IDENTITY(5001, 1) PRIMARY KEY, metadata json NULL @@ -656,6 +663,13 @@ VALUES (7, CAST('[' + ( ) + ']' AS vector(1998))); SET IDENTITY_INSERT vector_type_table OFF +SET IDENTITY_INSERT geometry_type_table ON +INSERT INTO geometry_type_table(id, name, geom) +VALUES + (1, 'point', geometry::STGeomFromText('POINT(1 2)', 0)), + (2, 'null geometry', NULL); +SET IDENTITY_INSERT geometry_type_table OFF + SET IDENTITY_INSERT profiles ON INSERT INTO profiles(id, metadata) VALUES diff --git a/src/Service.Tests/Snapshots/ConfigurationTests.TestReadingRuntimeConfigForMsSql.verified.txt b/src/Service.Tests/Snapshots/ConfigurationTests.TestReadingRuntimeConfigForMsSql.verified.txt index 3c3b8224e4..57e7548405 100644 --- a/src/Service.Tests/Snapshots/ConfigurationTests.TestReadingRuntimeConfigForMsSql.verified.txt +++ b/src/Service.Tests/Snapshots/ConfigurationTests.TestReadingRuntimeConfigForMsSql.verified.txt @@ -1910,6 +1910,38 @@ } } }, + { + GeometryType: { + Source: { + Object: geometry_type_table, + Type: Table + }, + GraphQL: { + Singular: GeometryType, + Plural: GeometryTypes, + Enabled: true + }, + Rest: { + Enabled: true + }, + Permissions: [ + { + Role: anonymous, + Actions: [ + { + Action: Read, + Fields: { + Include: [ + id, + name + ] + } + } + ] + } + ] + } + }, { Profile: { Source: { diff --git a/src/Service.Tests/UnitTests/SqlMetadataProviderUnitTests.cs b/src/Service.Tests/UnitTests/SqlMetadataProviderUnitTests.cs index dd6ad7d27e..50ed2026f0 100644 --- a/src/Service.Tests/UnitTests/SqlMetadataProviderUnitTests.cs +++ b/src/Service.Tests/UnitTests/SqlMetadataProviderUnitTests.cs @@ -396,6 +396,38 @@ public async Task ValidateInferredRelationshipInfoForMsSql() ValidateInferredRelationshipInfoForTables(); } + /// + /// Test to validate that a table holding a column whose CLR type the data provider cannot + /// resolve - here a geometry column - is still usable when the entity permissions enumerate + /// the readable fields and that column is not among them. + /// Metadata inference must succeed and the unreadable column must be absent from the + /// inferred source definition, so it never reaches the OData or GraphQL type maps. + /// + [TestMethod, TestCategory(TestCategory.MSSQL)] + public async Task ValidateColumnExcludedByFieldPermissionsIsNotInferred() + { + DatabaseEngine = TestCategory.MSSQL; + await SetupTestFixtureAndInferMetadata(); + + Assert.IsTrue( + _sqlMetadataProvider.GetEntityNamesAndDbObjects().TryGetValue("GeometryType", out DatabaseObject databaseObject), + message: "Metadata inference failed for the entity backed by a table with a geometry column."); + + SourceDefinition sourceDefinition = databaseObject.SourceDefinition; + + Assert.IsTrue( + sourceDefinition.Columns.ContainsKey("id"), + message: "The primary key column is expected in the source definition."); + Assert.IsTrue( + sourceDefinition.Columns.ContainsKey("name"), + message: "A column listed in fields.include is expected in the source definition."); + Assert.IsFalse( + sourceDefinition.Columns.ContainsKey("geom"), + message: "A column absent from fields.include is not expected in the source definition."); + + TestHelper.UnsetAllDABEnvironmentVariables(); + } + /// /// Test to validate successful inference of relationship data based on data provided in the config and the metadata /// collected from the MySql database. diff --git a/src/Service.Tests/dab-config.MsSql.json b/src/Service.Tests/dab-config.MsSql.json index 670f390d4c..fa80aa2e29 100644 --- a/src/Service.Tests/dab-config.MsSql.json +++ b/src/Service.Tests/dab-config.MsSql.json @@ -1990,6 +1990,39 @@ } } }, + "GeometryType": { + "source": { + "object": "geometry_type_table", + "type": "table" + }, + "graphql": { + "enabled": true, + "type": { + "singular": "GeometryType", + "plural": "GeometryTypes" + } + }, + "rest": { + "enabled": true + }, + "permissions": [ + { + "role": "anonymous", + "actions": [ + { + "action": "read", + "fields": { + "exclude": [], + "include": [ + "id", + "name" + ] + } + } + ] + } + ] + }, "Profile": { "source": { "object": "profiles", From 0d1330376e652e1b8589879e7137628ae717e3e1 Mon Sep 17 00:00:00 2001 From: Joymax Andrade do Nascimento Date: Fri, 4 Sep 2026 21:37:01 -0300 Subject: [PATCH 03/12] Match column names case-insensitively when resolving field permissions --- .../MetadataProviders/SqlMetadataProvider.cs | 25 +++++++++++-------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/src/Core/Services/MetadataProviders/SqlMetadataProvider.cs b/src/Core/Services/MetadataProviders/SqlMetadataProvider.cs index 6b31d7c700..1b9c10ebac 100644 --- a/src/Core/Services/MetadataProviders/SqlMetadataProvider.cs +++ b/src/Core/Services/MetadataProviders/SqlMetadataProvider.cs @@ -1548,7 +1548,7 @@ private async Task PopulateSourceDefinitionAsync( if (permittedColumns is not null && !permittedColumns.IsUnrestricted && !permittedColumns.IsColumnPermitted(columnName) - && !sourceDefinition.PrimaryKey.Contains(columnName)) + && !sourceDefinition.PrimaryKey.Contains(columnName, StringComparer.OrdinalIgnoreCase)) { continue; } @@ -1850,6 +1850,9 @@ private async Task FillSchemaForTableAsync( /// /// Describes which backing (database) columns of a database object the runtime /// configuration allows to be read. + /// Column names are matched case-insensitively, consistent with the comparer used by + /// SourceDefinition.Columns and with the field name lookups in this class, so a + /// configuration whose casing differs from the database schema still resolves. /// /// /// True when at least one permission reads every field ("fields" absent, "include" absent, @@ -1899,7 +1902,7 @@ private PermittedColumns ResolvePermittedColumnsForDatabaseObject( string tableName, string? entityName) { - HashSet included = new(StringComparer.Ordinal); + HashSet included = new(StringComparer.OrdinalIgnoreCase); HashSet? excluded = null; bool allColumns = false; bool matchedAnyEntity = false; @@ -1933,7 +1936,7 @@ private PermittedColumns ResolvePermittedColumnsForDatabaseObject( // A column is only droppable when every wildcard permission excludes it. if (excluded is null) { - excluded = new(entityPermittedColumns.Excluded, StringComparer.Ordinal); + excluded = new(entityPermittedColumns.Excluded, StringComparer.OrdinalIgnoreCase); } else { @@ -1944,10 +1947,10 @@ private PermittedColumns ResolvePermittedColumnsForDatabaseObject( if (!matchedAnyEntity) { - return new(AllColumns: true, Included: new(StringComparer.Ordinal), Excluded: new(StringComparer.Ordinal)); + return new(AllColumns: true, Included: new(StringComparer.OrdinalIgnoreCase), Excluded: new(StringComparer.OrdinalIgnoreCase)); } - excluded ??= new(StringComparer.Ordinal); + excluded ??= new(StringComparer.OrdinalIgnoreCase); excluded.ExceptWith(included); return new(allColumns, included, excluded); @@ -1961,12 +1964,12 @@ private PermittedColumns ResolvePermittedColumnsForDatabaseObject( /// private static PermittedColumns ResolvePermittedColumnsForEntity(Entity entity) { - HashSet included = new(StringComparer.Ordinal); + HashSet included = new(StringComparer.OrdinalIgnoreCase); HashSet? excluded = null; bool allColumns = false; // Exposed name -> backing column name. - Dictionary exposedToBackingName = new(StringComparer.Ordinal); + Dictionary exposedToBackingName = new(StringComparer.OrdinalIgnoreCase); if (entity.Mappings is not null) { @@ -1993,7 +1996,7 @@ private static PermittedColumns ResolvePermittedColumnsForEntity(Entity entity) if (entity.Permissions is null || entity.Permissions.Length == 0) { // Nothing configured: the whole object is read, as before. - return new(AllColumns: true, included, new HashSet(StringComparer.Ordinal)); + return new(AllColumns: true, included, new HashSet(StringComparer.OrdinalIgnoreCase)); } foreach (EntityPermission permission in entity.Permissions) @@ -2001,7 +2004,7 @@ private static PermittedColumns ResolvePermittedColumnsForEntity(Entity entity) if (permission.Actions is null) { allColumns = true; - excluded = new(StringComparer.Ordinal); + excluded = new(StringComparer.OrdinalIgnoreCase); continue; } @@ -2009,7 +2012,7 @@ private static PermittedColumns ResolvePermittedColumnsForEntity(Entity entity) { EntityActionFields? fields = action.Fields; - HashSet actionExcluded = new(StringComparer.Ordinal); + HashSet actionExcluded = new(StringComparer.OrdinalIgnoreCase); if (fields?.Exclude is not null) { if (fields.Exclude.Contains(FIELD_WILDCARD)) @@ -2074,7 +2077,7 @@ private static PermittedColumns ResolvePermittedColumnsForEntity(Entity entity) } } - excluded ??= new(StringComparer.Ordinal); + excluded ??= new(StringComparer.OrdinalIgnoreCase); excluded.ExceptWith(included); return new(allColumns, included, excluded); From 9b7d458154acbef6a6869eea7a2d2cba5eb3e058 Mon Sep 17 00:00:00 2001 From: Joymax Andrade do Nascimento Date: Fri, 4 Sep 2026 21:59:27 -0300 Subject: [PATCH 04/12] Only narrow the schema projection when the primary key is configured --- .../MetadataProviders/SqlMetadataProvider.cs | 53 ++++++++++++------- 1 file changed, 33 insertions(+), 20 deletions(-) diff --git a/src/Core/Services/MetadataProviders/SqlMetadataProvider.cs b/src/Core/Services/MetadataProviders/SqlMetadataProvider.cs index 1b9c10ebac..d9b26da03d 100644 --- a/src/Core/Services/MetadataProviders/SqlMetadataProvider.cs +++ b/src/Core/Services/MetadataProviders/SqlMetadataProvider.cs @@ -1959,9 +1959,17 @@ private PermittedColumns ResolvePermittedColumnsForDatabaseObject( /// /// Resolves the readable columns of a single entity from the "fields.include" / /// "fields.exclude" sections of its permissions. Exposed names (mappings and field - /// aliases) are translated back to their database column names, and configured primary - /// key fields are always kept, since the runtime cannot operate on the entity without them. + /// aliases) are translated back to their database column names, and the configured + /// primary key is always kept, since the runtime cannot operate on the entity without it. /// + /// + /// The projection is only narrowed for entities whose primary key is known from the + /// configuration ("fields[].primary-key" or "source.key-fields"). When the primary key is + /// instead inferred from the schema read itself - see the fallback to + /// DataTable.PrimaryKey in PopulateObjectDefinitionForEntity - narrowing + /// could drop the key column and leave the entity with no primary key, so every column is + /// read as before. + /// private static PermittedColumns ResolvePermittedColumnsForEntity(Entity entity) { HashSet included = new(StringComparer.OrdinalIgnoreCase); @@ -1993,9 +2001,28 @@ private static PermittedColumns ResolvePermittedColumnsForEntity(Entity entity) } } - if (entity.Permissions is null || entity.Permissions.Length == 0) + HashSet configuredPrimaryKey = new(StringComparer.OrdinalIgnoreCase); + + if (entity.Fields is not null) + { + foreach (FieldMetadata field in entity.Fields.Where(f => f.PrimaryKey)) + { + configuredPrimaryKey.Add(ResolveBackingName(field.Name, exposedToBackingName)); + } + } + + if (configuredPrimaryKey.Count == 0 && entity.Source is not null && entity.Source.KeyFields is not null) + { + foreach (string keyField in entity.Source.KeyFields) + { + configuredPrimaryKey.Add(ResolveBackingName(keyField, exposedToBackingName)); + } + } + + // Without a configured primary key, the key itself is inferred from this schema read, + // so a narrowed projection could drop it and leave the entity unusable. Read it all. + if (entity.Permissions is null || entity.Permissions.Length == 0 || configuredPrimaryKey.Count == 0) { - // Nothing configured: the whole object is read, as before. return new(AllColumns: true, included, new HashSet(StringComparer.OrdinalIgnoreCase)); } @@ -2060,22 +2087,8 @@ private static PermittedColumns ResolvePermittedColumnsForEntity(Entity entity) } } - // Primary keys are structural: never drop them from the projection. - if (entity.Source is not null && entity.Source.KeyFields is not null) - { - foreach (string keyField in entity.Source.KeyFields) - { - included.Add(ResolveBackingName(keyField, exposedToBackingName)); - } - } - - if (entity.Fields is not null) - { - foreach (FieldMetadata field in entity.Fields.Where(f => f.PrimaryKey)) - { - included.Add(ResolveBackingName(field.Name, exposedToBackingName)); - } - } + // The primary key is structural: never drop it from the projection. + included.UnionWith(configuredPrimaryKey); excluded ??= new(StringComparer.OrdinalIgnoreCase); excluded.ExceptWith(included); From 7a82fa7865b8b1e120fd7f858f916fce4efc9714 Mon Sep 17 00:00:00 2001 From: Joymax Andrade do Nascimento Date: Fri, 4 Sep 2026 22:22:11 -0300 Subject: [PATCH 05/12] Skip columns whose data type the provider cannot map, instead of honoring field permissions --- config-generators/mssql-commands.txt | 2 +- .../MsSqlMetadataProvider.cs | 18 + .../MetadataProviders/SqlMetadataProvider.cs | 377 +++--------------- ...tReadingRuntimeConfigForMsSql.verified.txt | 8 +- .../UnitTests/SqlMetadataProviderUnitTests.cs | 17 +- src/Service.Tests/dab-config.MsSql.json | 9 +- 6 files changed, 82 insertions(+), 349 deletions(-) diff --git a/config-generators/mssql-commands.txt b/config-generators/mssql-commands.txt index 0b1fe8ca1d..53674ef59c 100644 --- a/config-generators/mssql-commands.txt +++ b/config-generators/mssql-commands.txt @@ -22,7 +22,7 @@ add VectorType --config "dab-config.MsSql.json" --source vector_type_table --res update VectorType --config "dab-config.MsSql.json" --permissions "authenticated:create,read,delete,update" update VectorOwner --config "dab-config.MsSql.json" --relationship vectors --target.entity VectorType --cardinality many --relationship.fields "id:owner_id" update VectorType --config "dab-config.MsSql.json" --relationship owner --target.entity VectorOwner --cardinality one --relationship.fields "owner_id:id" -add GeometryType --config "dab-config.MsSql.json" --source geometry_type_table --rest true --graphql true --permissions "anonymous:read" --fields.include "id,name" +add GeometryType --config "dab-config.MsSql.json" --source geometry_type_table --rest true --graphql true --permissions "anonymous:read" add Profile --config "dab-config.MsSql.json" --source profiles --rest true --graphql true --permissions "anonymous:create,read,delete,update" update Profile --config "dab-config.MsSql.json" --permissions "authenticated:create,read,delete,update" add stocks_price --config "dab-config.MsSql.json" --source stocks_price --permissions "authenticated:create,read,update,delete" diff --git a/src/Core/Services/MetadataProviders/MsSqlMetadataProvider.cs b/src/Core/Services/MetadataProviders/MsSqlMetadataProvider.cs index 20de74a96d..9176b60566 100644 --- a/src/Core/Services/MetadataProviders/MsSqlMetadataProvider.cs +++ b/src/Core/Services/MetadataProviders/MsSqlMetadataProvider.cs @@ -1,6 +1,7 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. +using System.Collections.Immutable; using System.Data; using System.Data.Common; using System.Net; @@ -44,6 +45,23 @@ public MsSqlMetadataProvider( _runtimeConfigProvider = runtimeConfigProvider; } + /// + /// SQL Server CLR user-defined types. Microsoft.Data.SqlClient resolves their CLR type + /// through the Microsoft.SqlServer.Types assembly, which Data API builder does not + /// reference, so the reader reports no type for the column and the data adapter fails. + /// Deliberately limited to the types that cannot be read at all: timestamp, xml and vector + /// columns do resolve to a CLR type and are left untouched. + /// + private static readonly ImmutableHashSet _unsupportedColumnDataTypes = + ImmutableHashSet.Create( + StringComparer.OrdinalIgnoreCase, + "geometry", + "geography", + "hierarchyid"); + + /// + protected override ImmutableHashSet UnsupportedColumnDataTypes => _unsupportedColumnDataTypes; + public override string GetDefaultSchemaName() { return "dbo"; diff --git a/src/Core/Services/MetadataProviders/SqlMetadataProvider.cs b/src/Core/Services/MetadataProviders/SqlMetadataProvider.cs index d9b26da03d..d801290044 100644 --- a/src/Core/Services/MetadataProviders/SqlMetadataProvider.cs +++ b/src/Core/Services/MetadataProviders/SqlMetadataProvider.cs @@ -1,6 +1,7 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. +using System.Collections.Immutable; using System.Collections.ObjectModel; using System.Data; using System.Data.Common; @@ -70,9 +71,15 @@ public abstract class SqlMetadataProvider : protected const int NUMBER_OF_RESTRICTIONS = 4; /// - /// Wildcard used in the permissions "fields" section to denote every field. + /// Column data types, as reported by the "Columns" schema collection, that the data + /// provider cannot map to a CLR type. Reading such a column makes + /// fail with + /// "DataReader.GetFieldType(N) returned null", which takes down the whole database object + /// even when the column itself is never exposed. They are therefore left out of the + /// projection used for schema discovery. + /// Empty by default: a provider only lists a type here when it genuinely cannot resolve it. /// - private const string FIELD_WILDCARD = "*"; + protected virtual ImmutableHashSet UnsupportedColumnDataTypes => ImmutableHashSet.Empty; protected string ConnectionString { get; init; } @@ -1534,25 +1541,10 @@ private async Task PopulateSourceDefinitionAsync( DataTable schemaTable = reader.GetSchemaTable(); RuntimeConfig runtimeConfig = _runtimeConfigProvider.GetConfig(); - // Columns the entity's permissions allow to be read. The schema DataTable is cached - // per schema.table and may therefore carry columns permitted only for a sibling - // entity, so the restriction is re-applied per entity here. - PermittedColumns? permittedColumns = entity is null - ? null - : ResolvePermittedColumnsForEntity(entity); - foreach (DataRow columnInfoFromAdapter in schemaTable.Rows) { string columnName = columnInfoFromAdapter["ColumnName"].ToString()!; - if (permittedColumns is not null - && !permittedColumns.IsUnrestricted - && !permittedColumns.IsColumnPermitted(columnName) - && !sourceDefinition.PrimaryKey.Contains(columnName, StringComparer.OrdinalIgnoreCase)) - { - continue; - } - if (runtimeConfig.IsGraphQLEnabled && entity is not null && IsGraphQLReservedName(entity, columnName, graphQLEnabledGlobally: runtimeConfig.IsGraphQLEnabled)) @@ -1706,7 +1698,7 @@ private async Task GetTableWithSchemaFromDataSetAsync( { try { - dataTable = await FillSchemaForTableAsync(schemaName, tableName, entityName); + dataTable = await FillSchemaForTableAsync(schemaName, tableName); } catch (Exception ex) when (ex is not DataApiBuilderException) { @@ -1774,16 +1766,13 @@ private async Task ValidateDatabaseConnection() /// /// Using a data adapter, obtains the schema of the given table name /// and adds the corresponding DataTable to the entities data set. - /// When the entities backed by this database object restrict the readable - /// fields through permissions ("fields.include"/"fields.exclude"), the projection - /// is narrowed to those columns instead of "SELECT *". This avoids the provider - /// having to materialize CLR types it cannot handle (e.g. geometry/geography/hierarchyid), - /// which otherwise fails during schema discovery even though the column is not exposed. + /// Columns whose data type the data provider cannot map to a CLR type are left out of the + /// projection, because the data adapter refuses to build a schema mapping for them and the + /// whole object would otherwise be unreachable. See . /// private async Task FillSchemaForTableAsync( string schemaName, - string tableName, - string? entityName = null) + string tableName) { using ConnectionT conn = new(); // If connection string is set to empty string @@ -1830,14 +1819,7 @@ private async Task FillSchemaForTableAsync( string tableNameWithSchemaPrefix = GetTableNameWithSchemaPrefix(schemaName, tableName); - // Resolve the columns the configuration actually allows to be read for this - // database object. When nothing is restricted, the original SELECT * is preserved. - PermittedColumns permittedColumns = ResolvePermittedColumnsForDatabaseObject( - schemaName: schemaName, - tableName: tableName, - entityName: entityName); - - string projection = await BuildSchemaProjectionAsync(schemaName, tableName, permittedColumns); + string projection = await BuildSchemaProjectionAsync(schemaName, tableName); selectCommand.CommandText = $"SELECT {projection} FROM {tableNameWithSchemaPrefix}"; @@ -1848,325 +1830,70 @@ private async Task FillSchemaForTableAsync( } /// - /// Describes which backing (database) columns of a database object the runtime - /// configuration allows to be read. - /// Column names are matched case-insensitively, consistent with the comparer used by - /// SourceDefinition.Columns and with the field name lookups in this class, so a - /// configuration whose casing differs from the database schema still resolves. + /// Builds the projection used to read the schema of a database object. Returns "*" unless + /// the object holds columns whose data type this provider cannot map to a CLR type, in + /// which case those columns are named out of the projection so the rest stays reachable. + /// The column list comes from the "Columns" schema collection, which reads catalog metadata + /// only and therefore never has to materialize the offending type. /// - /// - /// True when at least one permission reads every field ("fields" absent, "include" absent, - /// or "include": ["*"]). - /// - /// Backing columns explicitly listed in an "include" section. - /// - /// Backing columns excluded from every wildcard permission and never explicitly included. - /// - private sealed record PermittedColumns(bool AllColumns, HashSet Included, HashSet Excluded) - { - /// - /// No column restriction could be derived from the configuration. - /// - public bool IsUnrestricted => AllColumns && Excluded.Count == 0; - - /// - /// Whether the given backing column is readable per the configuration. - /// - public bool IsColumnPermitted(string columnName) - { - if (Included.Contains(columnName)) - { - return true; - } - - if (Excluded.Contains(columnName)) - { - return false; - } - - return AllColumns; - } - } - - /// - /// Resolves the readable columns for a database object. - /// Because the schema DataTable is cached per schema.table, the result combines the - /// permissions of every entity in this data source backed by that same object: a column - /// has to be read if any of those entities can read it. - /// - /// Schema of the database object. - /// Name of the database object. - /// Entity that triggered the schema discovery, when known. - private PermittedColumns ResolvePermittedColumnsForDatabaseObject( - string schemaName, - string tableName, - string? entityName) + private async Task BuildSchemaProjectionAsync(string schemaName, string tableName) { - HashSet included = new(StringComparer.OrdinalIgnoreCase); - HashSet? excluded = null; - bool allColumns = false; - bool matchedAnyEntity = false; - - foreach ((string candidateEntityName, Entity candidateEntity) in Entities) - { - // Only consider entities backed by the same database object. - if (EntityToDatabaseObject.TryGetValue(candidateEntityName, out DatabaseObject? databaseObject)) - { - if (!string.Equals(databaseObject.SchemaName, schemaName, StringComparison.OrdinalIgnoreCase) - || !string.Equals(databaseObject.Name, tableName, StringComparison.OrdinalIgnoreCase)) - { - continue; - } - } - else if (!string.Equals(candidateEntityName, entityName, StringComparison.Ordinal)) - { - // Database object not inferred yet: only the entity that triggered the read applies. - continue; - } - - matchedAnyEntity = true; - PermittedColumns entityPermittedColumns = ResolvePermittedColumnsForEntity(candidateEntity); - - included.UnionWith(entityPermittedColumns.Included); - - if (entityPermittedColumns.AllColumns) - { - allColumns = true; - - // A column is only droppable when every wildcard permission excludes it. - if (excluded is null) - { - excluded = new(entityPermittedColumns.Excluded, StringComparer.OrdinalIgnoreCase); - } - else - { - excluded.IntersectWith(entityPermittedColumns.Excluded); - } - } - } - - if (!matchedAnyEntity) + if (UnsupportedColumnDataTypes.Count == 0) { - return new(AllColumns: true, Included: new(StringComparer.OrdinalIgnoreCase), Excluded: new(StringComparer.OrdinalIgnoreCase)); - } - - excluded ??= new(StringComparer.OrdinalIgnoreCase); - excluded.ExceptWith(included); - - return new(allColumns, included, excluded); - } - - /// - /// Resolves the readable columns of a single entity from the "fields.include" / - /// "fields.exclude" sections of its permissions. Exposed names (mappings and field - /// aliases) are translated back to their database column names, and the configured - /// primary key is always kept, since the runtime cannot operate on the entity without it. - /// - /// - /// The projection is only narrowed for entities whose primary key is known from the - /// configuration ("fields[].primary-key" or "source.key-fields"). When the primary key is - /// instead inferred from the schema read itself - see the fallback to - /// DataTable.PrimaryKey in PopulateObjectDefinitionForEntity - narrowing - /// could drop the key column and leave the entity with no primary key, so every column is - /// read as before. - /// - private static PermittedColumns ResolvePermittedColumnsForEntity(Entity entity) - { - HashSet included = new(StringComparer.OrdinalIgnoreCase); - HashSet? excluded = null; - bool allColumns = false; - - // Exposed name -> backing column name. - Dictionary exposedToBackingName = new(StringComparer.OrdinalIgnoreCase); - - if (entity.Mappings is not null) - { - foreach ((string backingName, string exposedName) in entity.Mappings) - { - if (!string.IsNullOrWhiteSpace(exposedName)) - { - exposedToBackingName[exposedName] = backingName; - } - } - } - - if (entity.Fields is not null) - { - foreach (FieldMetadata field in entity.Fields) - { - if (!string.IsNullOrWhiteSpace(field.Alias)) - { - exposedToBackingName[field.Alias!] = field.Name; - } - } - } - - HashSet configuredPrimaryKey = new(StringComparer.OrdinalIgnoreCase); - - if (entity.Fields is not null) - { - foreach (FieldMetadata field in entity.Fields.Where(f => f.PrimaryKey)) - { - configuredPrimaryKey.Add(ResolveBackingName(field.Name, exposedToBackingName)); - } - } - - if (configuredPrimaryKey.Count == 0 && entity.Source is not null && entity.Source.KeyFields is not null) - { - foreach (string keyField in entity.Source.KeyFields) - { - configuredPrimaryKey.Add(ResolveBackingName(keyField, exposedToBackingName)); - } + return "*"; } - // Without a configured primary key, the key itself is inferred from this schema read, - // so a narrowed projection could drop it and leave the entity unusable. Read it all. - if (entity.Permissions is null || entity.Permissions.Length == 0 || configuredPrimaryKey.Count == 0) - { - return new(AllColumns: true, included, new HashSet(StringComparer.OrdinalIgnoreCase)); - } + List readableColumns = new(); + List skippedColumns = new(); - foreach (EntityPermission permission in entity.Permissions) + try { - if (permission.Actions is null) - { - allColumns = true; - excluded = new(StringComparer.OrdinalIgnoreCase); - continue; - } + DataTable columnsInTable = await GetColumnsAsync(schemaName, tableName); - foreach (EntityAction action in permission.Actions) + foreach (DataRow columnInfo in columnsInTable.Rows) { - EntityActionFields? fields = action.Fields; - - HashSet actionExcluded = new(StringComparer.OrdinalIgnoreCase); - if (fields?.Exclude is not null) + if (columnInfo["COLUMN_NAME"] is not string columnName) { - if (fields.Exclude.Contains(FIELD_WILDCARD)) - { - // This permission reads no field at all, so it contributes no column. - continue; - } - - foreach (string field in fields.Exclude) - { - actionExcluded.Add(ResolveBackingName(field, exposedToBackingName)); - } + continue; } - // No "fields" section, no "include" section, or an explicit wildcard, - // means every column not listed in "exclude" is readable. - bool includesEveryField = fields is null - || fields.Include is null - || fields.Include.Contains(FIELD_WILDCARD); + string? dataType = columnInfo["DATA_TYPE"] as string; - if (includesEveryField) + if (dataType is not null && UnsupportedColumnDataTypes.Contains(dataType)) { - allColumns = true; - - if (excluded is null) - { - excluded = actionExcluded; - } - else - { - excluded.IntersectWith(actionExcluded); - } - - continue; + skippedColumns.Add($"{columnName} ({dataType})"); } - - foreach (string field in fields!.Include!) + else { - string backingName = ResolveBackingName(field, exposedToBackingName); - if (!actionExcluded.Contains(backingName)) - { - included.Add(backingName); - } + readableColumns.Add(columnName); } } } - - // The primary key is structural: never drop it from the projection. - included.UnionWith(configuredPrimaryKey); - - excluded ??= new(StringComparer.OrdinalIgnoreCase); - excluded.ExceptWith(included); - - return new(allColumns, included, excluded); - } - - /// - /// Translates a configured (exposed) field name into its backing column name. - /// - private static string ResolveBackingName(string fieldName, Dictionary exposedToBackingName) - { - return exposedToBackingName.TryGetValue(fieldName, out string? backingName) ? backingName : fieldName; - } - - /// - /// Builds the projection used to read the schema of a database object, narrowed to the - /// columns the configuration allows to be read. Returns "*" when no restriction applies - /// or when the column list cannot be determined. - /// - private async Task BuildSchemaProjectionAsync( - string schemaName, - string tableName, - PermittedColumns permittedColumns) - { - if (permittedColumns.IsUnrestricted) + catch (Exception ex) when (ex is not DataApiBuilderException) { + // The column list is a best-effort optimization: without it the read below behaves + // exactly as it did before, failing loudly if an unsupported type is present. + _logger.LogDebug( + "Unable to enumerate the columns of {schemaName}.{tableName}: {message}", + schemaName, + tableName, + ex.Message); return "*"; } - List columnsToRead; - - if (permittedColumns.AllColumns) - { - // "include": ["*"] with an "exclude" list: enumerate the columns from the catalog - // (metadata only, so unsupported CLR types are never materialized) and drop the - // excluded ones. - List allColumnNames = new(); - try - { - DataTable columnsInTable = await GetColumnsAsync(schemaName, tableName); - - foreach (DataRow columnInfo in columnsInTable.Rows) - { - if (columnInfo["COLUMN_NAME"] is string columnName) - { - allColumnNames.Add(columnName); - } - } - } - catch (Exception ex) - { - _logger.LogDebug( - "Unable to enumerate the columns of {schemaName}.{tableName} to honor the configured field exclusions: {message}", - schemaName, - tableName, - ex.Message); - return "*"; - } - - if (allColumnNames.Count == 0) - { - return "*"; - } - - columnsToRead = allColumnNames.Where(permittedColumns.IsColumnPermitted).ToList(); - } - else - { - columnsToRead = permittedColumns.Included.ToList(); - } - - if (columnsToRead.Count == 0) + if (skippedColumns.Count == 0 || readableColumns.Count == 0) { return "*"; } - return string.Join(", ", columnsToRead.Select(column => SqlQueryBuilder.QuoteIdentifier(column))); + _logger.LogWarning( + "Skipping column(s) of {schemaName}.{tableName} whose data type is not supported: {skippedColumns}. " + + "They are not exposed through REST, GraphQL or MCP.", + schemaName, + tableName, + string.Join(", ", skippedColumns)); + + return string.Join(", ", readableColumns.Select(column => SqlQueryBuilder.QuoteIdentifier(column))); } /// diff --git a/src/Service.Tests/Snapshots/ConfigurationTests.TestReadingRuntimeConfigForMsSql.verified.txt b/src/Service.Tests/Snapshots/ConfigurationTests.TestReadingRuntimeConfigForMsSql.verified.txt index 57e7548405..b0ce94e022 100644 --- a/src/Service.Tests/Snapshots/ConfigurationTests.TestReadingRuntimeConfigForMsSql.verified.txt +++ b/src/Service.Tests/Snapshots/ConfigurationTests.TestReadingRuntimeConfigForMsSql.verified.txt @@ -1929,13 +1929,7 @@ Role: anonymous, Actions: [ { - Action: Read, - Fields: { - Include: [ - id, - name - ] - } + Action: Read } ] } diff --git a/src/Service.Tests/UnitTests/SqlMetadataProviderUnitTests.cs b/src/Service.Tests/UnitTests/SqlMetadataProviderUnitTests.cs index 50ed2026f0..c7dbe7d518 100644 --- a/src/Service.Tests/UnitTests/SqlMetadataProviderUnitTests.cs +++ b/src/Service.Tests/UnitTests/SqlMetadataProviderUnitTests.cs @@ -397,14 +397,15 @@ public async Task ValidateInferredRelationshipInfoForMsSql() } /// - /// Test to validate that a table holding a column whose CLR type the data provider cannot - /// resolve - here a geometry column - is still usable when the entity permissions enumerate - /// the readable fields and that column is not among them. - /// Metadata inference must succeed and the unreadable column must be absent from the - /// inferred source definition, so it never reaches the OData or GraphQL type maps. + /// Test to validate that a table holding a column whose data type the data provider cannot + /// map to a CLR type - here a geometry column - is still usable: metadata inference must + /// succeed and the unsupported column must be absent from the inferred source definition, + /// so it never reaches the OData or GraphQL type maps. + /// The entity places no field restriction, so this covers the column being skipped on the + /// strength of its type alone. /// [TestMethod, TestCategory(TestCategory.MSSQL)] - public async Task ValidateColumnExcludedByFieldPermissionsIsNotInferred() + public async Task ValidateUnsupportedColumnTypeIsNotInferred() { DatabaseEngine = TestCategory.MSSQL; await SetupTestFixtureAndInferMetadata(); @@ -420,10 +421,10 @@ public async Task ValidateColumnExcludedByFieldPermissionsIsNotInferred() message: "The primary key column is expected in the source definition."); Assert.IsTrue( sourceDefinition.Columns.ContainsKey("name"), - message: "A column listed in fields.include is expected in the source definition."); + message: "A column with a supported data type is expected in the source definition."); Assert.IsFalse( sourceDefinition.Columns.ContainsKey("geom"), - message: "A column absent from fields.include is not expected in the source definition."); + message: "A column whose data type cannot be mapped is not expected in the source definition."); TestHelper.UnsetAllDABEnvironmentVariables(); } diff --git a/src/Service.Tests/dab-config.MsSql.json b/src/Service.Tests/dab-config.MsSql.json index fa80aa2e29..fc614d0e86 100644 --- a/src/Service.Tests/dab-config.MsSql.json +++ b/src/Service.Tests/dab-config.MsSql.json @@ -2010,14 +2010,7 @@ "role": "anonymous", "actions": [ { - "action": "read", - "fields": { - "exclude": [], - "include": [ - "id", - "name" - ] - } + "action": "read" } ] } From 0df9ee491b80b554f4795c5eb22bf3171a84a15d Mon Sep 17 00:00:00 2001 From: Joymax Andrade do Nascimento Date: Fri, 11 Sep 2026 14:27:07 -0300 Subject: [PATCH 06/12] Reject unsupported primary keys and read the catalog once per object --- .../MetadataProviders/SqlMetadataProvider.cs | 150 ++++++++++++++++-- 1 file changed, 140 insertions(+), 10 deletions(-) diff --git a/src/Core/Services/MetadataProviders/SqlMetadataProvider.cs b/src/Core/Services/MetadataProviders/SqlMetadataProvider.cs index d801290044..87cb754bac 100644 --- a/src/Core/Services/MetadataProviders/SqlMetadataProvider.cs +++ b/src/Core/Services/MetadataProviders/SqlMetadataProvider.cs @@ -1,6 +1,7 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. +using System.Collections.Concurrent; using System.Collections.Immutable; using System.Collections.ObjectModel; using System.Data; @@ -95,6 +96,20 @@ public abstract class SqlMetadataProvider : private Dictionary> EntityExposedNamesToBackingColumnNames { get; } = new(); + /// + /// Caches the "Columns" schema collection per database object for the duration of metadata + /// initialization, so schema discovery and column definition population share one catalog + /// round trip instead of querying twice per object. Cleared once initialization completes. + /// + private readonly ConcurrentDictionary _columnsMetadataCache = new(StringComparer.OrdinalIgnoreCase); + + /// + /// Columns left out of the schema projection per database object, mapped to the data type + /// that made them unreadable. Used to explain the omission when a configured primary key + /// turns out to be one of them. + /// + private readonly ConcurrentDictionary> _skippedColumnsByObject = new(StringComparer.OrdinalIgnoreCase); + protected IAbstractQueryManagerFactory QueryManagerFactory { get; init; } /// @@ -347,7 +362,16 @@ public async Task InitializeAsync() _runtimeConfigValidator.ValidateEntityAndAutoentityConfigurations(runtimeConfig); GenerateDatabaseObjectForEntities(); - await PopulateObjectDefinitionForEntities(); + + try + { + await PopulateObjectDefinitionForEntities(); + } + finally + { + ReleaseCatalogMetadataCaches(); + } + GenerateExposedToBackingColumnMapsForEntities(); // When IsLateConfigured is true we are in a hosted scenario and do not reveal primary key information. @@ -1540,7 +1564,6 @@ private async Task PopulateSourceDefinitionAsync( using DataTableReader reader = new(dataTable); DataTable schemaTable = reader.GetSchemaTable(); RuntimeConfig runtimeConfig = _runtimeConfigProvider.GetConfig(); - foreach (DataRow columnInfoFromAdapter in schemaTable.Rows) { string columnName = columnInfoFromAdapter["ColumnName"].ToString()!; @@ -1582,7 +1605,9 @@ private async Task PopulateSourceDefinitionAsync( sourceDefinition.Columns.TryAdd(columnName, column); } - DataTable columnsInTable = await GetColumnsAsync(schemaName, tableName); + RejectPrimaryKeyOnUnsupportedColumn(schemaName, tableName, sourceDefinition); + + DataTable columnsInTable = await GetCachedColumnsAsync(schemaName, tableName); PopulateColumnDefinitionWithHasDefaultAndDbType( sourceDefinition, @@ -1836,6 +1861,11 @@ private async Task FillSchemaForTableAsync( /// The column list comes from the "Columns" schema collection, which reads catalog metadata /// only and therefore never has to materialize the offending type. /// + /// + /// Thrown when every column of the object has an unsupported data type. Returning "*" there + /// would re-issue the projection that cannot be read, hiding the reason behind the + /// provider's own error. + /// private async Task BuildSchemaProjectionAsync(string schemaName, string tableName) { if (UnsupportedColumnDataTypes.Count == 0) @@ -1844,11 +1874,11 @@ private async Task BuildSchemaProjectionAsync(string schemaName, string } List readableColumns = new(); - List skippedColumns = new(); + Dictionary skippedColumns = new(StringComparer.OrdinalIgnoreCase); try { - DataTable columnsInTable = await GetColumnsAsync(schemaName, tableName); + DataTable columnsInTable = await GetCachedColumnsAsync(schemaName, tableName); foreach (DataRow columnInfo in columnsInTable.Rows) { @@ -1857,11 +1887,16 @@ private async Task BuildSchemaProjectionAsync(string schemaName, string continue; } - string? dataType = columnInfo["DATA_TYPE"] as string; + if (columnInfo["DATA_TYPE"] is not string dataType) + { + // The catalog did not report a usable type name, so this column cannot be + // classified. Leave the projection alone rather than guess. + return "*"; + } - if (dataType is not null && UnsupportedColumnDataTypes.Contains(dataType)) + if (UnsupportedColumnDataTypes.Contains(dataType)) { - skippedColumns.Add($"{columnName} ({dataType})"); + skippedColumns[columnName] = dataType; } else { @@ -1881,21 +1916,116 @@ private async Task BuildSchemaProjectionAsync(string schemaName, string return "*"; } - if (skippedColumns.Count == 0 || readableColumns.Count == 0) + if (skippedColumns.Count == 0) { return "*"; } + if (readableColumns.Count == 0) + { + // Falling back to "*" here would re-issue the very projection that fails, so the + // caller would see the opaque provider error instead of the reason for it. + throw new DataApiBuilderException( + message: $"Every column of {schemaName}.{tableName} has a data type that is not supported: " + + $"{FormatSkippedColumns(skippedColumns)}. The object cannot be exposed.", + statusCode: HttpStatusCode.ServiceUnavailable, + subStatusCode: DataApiBuilderException.SubStatusCodes.ErrorInInitialization); + } + + _skippedColumnsByObject[GetObjectCacheKey(schemaName, tableName)] = skippedColumns; + _logger.LogWarning( "Skipping column(s) of {schemaName}.{tableName} whose data type is not supported: {skippedColumns}. " + "They are not exposed through REST, GraphQL or MCP.", schemaName, tableName, - string.Join(", ", skippedColumns)); + FormatSkippedColumns(skippedColumns)); return string.Join(", ", readableColumns.Select(column => SqlQueryBuilder.QuoteIdentifier(column))); } + /// + /// Fails initialization when a configured primary key names a column that was left out of + /// the projection because its data type is not supported. The key would otherwise stay in + /// while being absent from + /// , and the inconsistency surfaces much later as a + /// lookup failure while building queries, the OpenAPI document or the EDM model. + /// + private void RejectPrimaryKeyOnUnsupportedColumn( + string schemaName, + string tableName, + SourceDefinition sourceDefinition) + { + if (!_skippedColumnsByObject.TryGetValue(GetObjectCacheKey(schemaName, tableName), out Dictionary? skippedColumns)) + { + return; + } + + foreach (string primaryKey in sourceDefinition.PrimaryKey) + { + if (skippedColumns.TryGetValue(primaryKey, out string? dataType)) + { + throw new DataApiBuilderException( + message: $"The primary key column {primaryKey} of {schemaName}.{tableName} has the data type " + + $"{dataType}, which is not supported. A primary key cannot be omitted from the object " + + "metadata, so this object cannot be exposed.", + statusCode: HttpStatusCode.ServiceUnavailable, + subStatusCode: DataApiBuilderException.SubStatusCodes.ErrorInInitialization); + } + } + } + + /// + /// Renders skipped columns as "name (type)" pairs for log and error messages. + /// + private static string FormatSkippedColumns(Dictionary skippedColumns) + { + return string.Join(", ", skippedColumns.Select(entry => $"{entry.Key} ({entry.Value})")); + } + + /// + /// Key used by the per-object metadata caches held during initialization. + /// + private static string GetObjectCacheKey(string schemaName, string tableName) + { + return $"{schemaName}.{tableName}"; + } + + /// + /// Releases the catalog metadata gathered during initialization. Nothing reads these caches + /// once object definitions are populated, and the cached tables are disposable. + /// + private void ReleaseCatalogMetadataCaches() + { + foreach (DataTable columnsInTable in _columnsMetadataCache.Values) + { + columnsInTable.Dispose(); + } + + _columnsMetadataCache.Clear(); + _skippedColumnsByObject.Clear(); + } + + /// + /// Returns the "Columns" schema collection for a database object, reading it from the + /// catalog once per object. Schema discovery and column definition population both need it, + /// and each call opens its own connection. + /// + private async Task GetCachedColumnsAsync(string schemaName, string tableName) + { + string cacheKey = GetObjectCacheKey(schemaName, tableName); + + if (_columnsMetadataCache.TryGetValue(cacheKey, out DataTable? cachedColumns)) + { + return cachedColumns; + } + + DataTable columnsInTable = await GetColumnsAsync(schemaName, tableName); + _columnsMetadataCache[cacheKey] = columnsInTable; + + return columnsInTable; + } + /// /// Gets the correctly formatted table name with schema as prefix, if one exists. /// A schema prefix is simply the correctly formatted and prefixed schema name that From 3207712341c2b589ae6a3db73c9be6c7fceeb6e0 Mon Sep 17 00:00:00 2001 From: Joymax Andrade do Nascimento Date: Fri, 11 Sep 2026 15:15:58 -0300 Subject: [PATCH 07/12] Resolve the schema projection before opening the connection and disambiguate the cache key --- .../MetadataProviders/SqlMetadataProvider.cs | 25 +++++++++++-------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/src/Core/Services/MetadataProviders/SqlMetadataProvider.cs b/src/Core/Services/MetadataProviders/SqlMetadataProvider.cs index 87cb754bac..9de91717e4 100644 --- a/src/Core/Services/MetadataProviders/SqlMetadataProvider.cs +++ b/src/Core/Services/MetadataProviders/SqlMetadataProvider.cs @@ -1834,20 +1834,22 @@ private async Task FillSchemaForTableAsync( innerException: ex); } + string tableNameWithSchemaPrefix = GetTableNameWithSchemaPrefix(schemaName, tableName); + + // Resolved before the connection below is opened. Reading the catalog uses a connection + // of its own, and nesting that inside an already open one exhausts a small pool: with + // "Max Pool Size=1" the inner open waits for a connection the outer scope still holds. + string projection = await BuildSchemaProjectionAsync(schemaName, tableName); + await conn.OpenAsync(); DataAdapterT adapterForTable = new(); CommandT selectCommand = new() { - Connection = conn + Connection = conn, + CommandText + = $"SELECT {projection} FROM {tableNameWithSchemaPrefix}" }; - - string tableNameWithSchemaPrefix = GetTableNameWithSchemaPrefix(schemaName, tableName); - - string projection = await BuildSchemaProjectionAsync(schemaName, tableName); - - selectCommand.CommandText - = $"SELECT {projection} FROM {tableNameWithSchemaPrefix}"; adapterForTable.SelectCommand = selectCommand; DataTable[] dataTable = adapterForTable.FillSchema(EntitiesDataSet, SchemaType.Source, tableNameWithSchemaPrefix); @@ -1984,11 +1986,14 @@ private static string FormatSkippedColumns(Dictionary skippedCol } /// - /// Key used by the per-object metadata caches held during initialization. + /// Key used by the per-object metadata caches held during initialization. The schema name is + /// length-prefixed rather than joined with a dot, because bracketed identifiers may contain + /// dots: [a.b].[c] and [a].[b.c] are different objects that a "schema.table" + /// key would collide, letting one reuse the other's catalog rows. /// private static string GetObjectCacheKey(string schemaName, string tableName) { - return $"{schemaName}.{tableName}"; + return $"{schemaName.Length}:{schemaName}{tableName}"; } /// From 60ffd4c34133bd647a2112f71a5b2ce6fe8cb5d7 Mon Sep 17 00:00:00 2001 From: Joymax Andrade do Nascimento Date: Fri, 11 Sep 2026 16:36:52 -0300 Subject: [PATCH 08/12] Add MSSQL test for an object whose every column has an unsupported data type --- src/Service.Tests/DatabaseSchema-MsSql.sql | 2 + .../UnitTests/SqlMetadataProviderUnitTests.cs | 55 +++++++++++++++++++ 2 files changed, 57 insertions(+) diff --git a/src/Service.Tests/DatabaseSchema-MsSql.sql b/src/Service.Tests/DatabaseSchema-MsSql.sql index ce63144076..0850e0e551 100644 --- a/src/Service.Tests/DatabaseSchema-MsSql.sql +++ b/src/Service.Tests/DatabaseSchema-MsSql.sql @@ -11,6 +11,7 @@ DROP VIEW IF EXISTS books_view_with_mapping; DROP VIEW IF EXISTS stocks_view_selected; DROP VIEW IF EXISTS books_publishers_view_composite; DROP VIEW IF EXISTS books_publishers_view_composite_insertable; +DROP VIEW IF EXISTS geometry_only_view; DROP PROCEDURE IF EXISTS get_books; DROP PROCEDURE IF EXISTS get_book_by_id; DROP PROCEDURE IF EXISTS get_publisher_by_id; @@ -768,6 +769,7 @@ EXEC('CREATE VIEW books_publishers_view_composite_insertable as SELECT books.id, books.title, publishers.name, books.publisher_id FROM dbo.books,dbo.publishers where publishers.id = books.publisher_id'); +EXEC('CREATE VIEW geometry_only_view AS SELECT geom FROM dbo.geometry_type_table'); EXEC('CREATE PROCEDURE get_book_by_id @id int AS SELECT * FROM dbo.books WHERE id = @id'); diff --git a/src/Service.Tests/UnitTests/SqlMetadataProviderUnitTests.cs b/src/Service.Tests/UnitTests/SqlMetadataProviderUnitTests.cs index c7dbe7d518..f2080b11d3 100644 --- a/src/Service.Tests/UnitTests/SqlMetadataProviderUnitTests.cs +++ b/src/Service.Tests/UnitTests/SqlMetadataProviderUnitTests.cs @@ -429,6 +429,61 @@ public async Task ValidateUnsupportedColumnTypeIsNotInferred() TestHelper.UnsetAllDABEnvironmentVariables(); } + /// + /// Test to validate that a database object whose every column has a data type the data + /// provider cannot map fails initialization with a specific error, rather than falling back + /// to reading every column and surfacing the provider's opaque failure instead of the reason. + /// The entity is declared in an in-memory config rather than in dab-config.MsSql.json, + /// because this object fails by design and every MSSQL fixture initializes every configured + /// entity. + /// + [TestMethod, TestCategory(TestCategory.MSSQL)] + public async Task ValidateObjectWithOnlyUnsupportedColumnsFailsInitialization() + { + DatabaseEngine = TestCategory.MSSQL; + TestHelper.SetupDatabaseEnvironment(DatabaseEngine); + + Dictionary entities = new() + { + { + "GeometryOnlyView", + new Entity( + Source: new("dbo.geometry_only_view", EntitySourceType.View, null, new string[] { "geom" }), + Fields: null, + Rest: new(Enabled: true), + GraphQL: new("GeometryOnlyView", "GeometryOnlyViews", Enabled: true), + Permissions: new EntityPermission[] + { + new(Role: "anonymous", + Actions: new EntityAction[] { new(Action: EntityActionOperation.Read, Fields: null, Policy: null) }) + }, + Relationships: null, + Mappings: null) + } + }; + + RuntimeConfig runtimeConfig = SqlTestHelper.SetupRuntimeConfig() with { Entities = new RuntimeEntities(entities) }; + RuntimeConfigProvider runtimeConfigProvider = TestHelper.GenerateInMemoryRuntimeConfigProvider(runtimeConfig); + SetUpSQLMetadataProvider(runtimeConfigProvider); + await ResetDbStateAsync(); + + try + { + await _sqlMetadataProvider.InitializeAsync(); + Assert.Fail("Expected DataApiBuilderException was not thrown for an object whose every column has an unsupported data type."); + } + catch (DataApiBuilderException ex) + { + Assert.AreEqual(HttpStatusCode.ServiceUnavailable, ex.StatusCode); + Assert.AreEqual(DataApiBuilderException.SubStatusCodes.ErrorInInitialization, ex.SubStatusCode); + Assert.IsTrue( + ex.Message.Contains("has a data type that is not supported"), + message: $"Unexpected exception message: {ex.Message}"); + } + + TestHelper.UnsetAllDABEnvironmentVariables(); + } + /// /// Test to validate successful inference of relationship data based on data provided in the config and the metadata /// collected from the MySql database. From 5d65bda8baa9aa1ccb60e10b477d36758d246797 Mon Sep 17 00:00:00 2001 From: Joymax Andrade do Nascimento Date: Fri, 11 Sep 2026 18:00:38 -0300 Subject: [PATCH 09/12] Compare cached object names ordinally and cover the primary key guard --- .../MetadataProviders/SqlMetadataProvider.cs | 12 +++-- .../UnitTests/SqlMetadataProviderUnitTests.cs | 53 +++++++++++++++++++ 2 files changed, 62 insertions(+), 3 deletions(-) diff --git a/src/Core/Services/MetadataProviders/SqlMetadataProvider.cs b/src/Core/Services/MetadataProviders/SqlMetadataProvider.cs index 9de91717e4..570f6561b5 100644 --- a/src/Core/Services/MetadataProviders/SqlMetadataProvider.cs +++ b/src/Core/Services/MetadataProviders/SqlMetadataProvider.cs @@ -100,15 +100,21 @@ public abstract class SqlMetadataProvider : /// Caches the "Columns" schema collection per database object for the duration of metadata /// initialization, so schema discovery and column definition population share one catalog /// round trip instead of querying twice per object. Cleared once initialization completes. + /// The key is compared with an ordinal comparer: under a case-sensitive collation + /// dbo.Foo and dbo.foo are distinct objects, and aliasing them would serve one + /// object's catalog rows for the other. Case-insensitive collations are unaffected, since + /// they cannot hold both names at once. /// - private readonly ConcurrentDictionary _columnsMetadataCache = new(StringComparer.OrdinalIgnoreCase); + private readonly ConcurrentDictionary _columnsMetadataCache = new(StringComparer.Ordinal); /// /// Columns left out of the schema projection per database object, mapped to the data type /// that made them unreadable. Used to explain the omission when a configured primary key - /// turns out to be one of them. + /// turns out to be one of them. Keyed by object with an ordinal comparer for the reason + /// above; the inner column names stay case-insensitive, matching how this class resolves + /// configured field names against the schema. /// - private readonly ConcurrentDictionary> _skippedColumnsByObject = new(StringComparer.OrdinalIgnoreCase); + private readonly ConcurrentDictionary> _skippedColumnsByObject = new(StringComparer.Ordinal); protected IAbstractQueryManagerFactory QueryManagerFactory { get; init; } diff --git a/src/Service.Tests/UnitTests/SqlMetadataProviderUnitTests.cs b/src/Service.Tests/UnitTests/SqlMetadataProviderUnitTests.cs index f2080b11d3..180299603f 100644 --- a/src/Service.Tests/UnitTests/SqlMetadataProviderUnitTests.cs +++ b/src/Service.Tests/UnitTests/SqlMetadataProviderUnitTests.cs @@ -484,6 +484,59 @@ public async Task ValidateObjectWithOnlyUnsupportedColumnsFailsInitialization() TestHelper.UnsetAllDABEnvironmentVariables(); } + /// + /// Test to validate that a configured primary key naming a column of an unsupported data + /// type fails initialization instead of producing a source definition whose primary key is + /// absent from its columns. The error names both the column and the offending type. + /// The entity is declared in an in-memory config for the same reason as the test above. + /// + [TestMethod, TestCategory(TestCategory.MSSQL)] + public async Task ValidatePrimaryKeyOnUnsupportedColumnFailsInitialization() + { + DatabaseEngine = TestCategory.MSSQL; + TestHelper.SetupDatabaseEnvironment(DatabaseEngine); + + Dictionary entities = new() + { + { + "GeometryKeyed", + new Entity( + Source: new("dbo.geometry_type_table", EntitySourceType.Table, null, new string[] { "geom" }), + Fields: null, + Rest: new(Enabled: true), + GraphQL: new("GeometryKeyed", "GeometryKeyeds", Enabled: true), + Permissions: new EntityPermission[] + { + new(Role: "anonymous", + Actions: new EntityAction[] { new(Action: EntityActionOperation.Read, Fields: null, Policy: null) }) + }, + Relationships: null, + Mappings: null) + } + }; + + RuntimeConfig runtimeConfig = SqlTestHelper.SetupRuntimeConfig() with { Entities = new RuntimeEntities(entities) }; + RuntimeConfigProvider runtimeConfigProvider = TestHelper.GenerateInMemoryRuntimeConfigProvider(runtimeConfig); + SetUpSQLMetadataProvider(runtimeConfigProvider); + await ResetDbStateAsync(); + + try + { + await _sqlMetadataProvider.InitializeAsync(); + Assert.Fail("Expected DataApiBuilderException was not thrown for a primary key of an unsupported data type."); + } + catch (DataApiBuilderException ex) + { + Assert.AreEqual(HttpStatusCode.ServiceUnavailable, ex.StatusCode); + Assert.AreEqual(DataApiBuilderException.SubStatusCodes.ErrorInInitialization, ex.SubStatusCode); + Assert.IsTrue( + ex.Message.Contains("geom") && ex.Message.Contains("geometry"), + message: $"The error is expected to name the column and its data type. Actual message: {ex.Message}"); + } + + TestHelper.UnsetAllDABEnvironmentVariables(); + } + /// /// Test to validate successful inference of relationship data based on data provided in the config and the metadata /// collected from the MySql database. From 9b47156dec132eb99515cab179a682baa658f63e Mon Sep 17 00:00:00 2001 From: Joymax Andrade do Nascimento Date: Mon, 14 Sep 2026 21:38:38 -0300 Subject: [PATCH 10/12] Keep the narrowed projection authoritative and reject stale configured references Schema discovery reproduces the columns SELECT * exposes before subtracting unsupported data types, so HIDDEN period columns stay out of the exposed contract. When the projection is narrowed it reads the shape with CommandBehavior.SchemaOnly instead of DbDataAdapter.FillSchema, whose KeyInfo behaviour reintroduced excluded key columns as hidden reader columns, and takes the primary key from the catalog. Ordinary primary-key inference is untouched when nothing is skipped. Configuration naming a column left out of the projection now fails initialization. Addresses the three review findings on #3802. --- .../MsSqlMetadataProvider.cs | 100 ++++++ .../MetadataProviders/SqlMetadataProvider.cs | 319 +++++++++++++++++- src/Service.Tests/DatabaseSchema-MsSql.sql | 60 ++++ .../UnitTests/SqlMetadataProviderUnitTests.cs | 254 ++++++++++++++ 4 files changed, 729 insertions(+), 4 deletions(-) diff --git a/src/Core/Services/MetadataProviders/MsSqlMetadataProvider.cs b/src/Core/Services/MetadataProviders/MsSqlMetadataProvider.cs index 9176b60566..0a37d7e695 100644 --- a/src/Core/Services/MetadataProviders/MsSqlMetadataProvider.cs +++ b/src/Core/Services/MetadataProviders/MsSqlMetadataProvider.cs @@ -62,6 +62,106 @@ public MsSqlMetadataProvider( /// protected override ImmutableHashSet UnsupportedColumnDataTypes => _unsupportedColumnDataTypes; + /// + protected override async Task GetObjectCatalogMetadataAsync( + string schemaName, + string tableName) + { + string schemaParamName = $"{BaseQueryStructure.PARAM_NAME_PREFIX}param0"; + string tableParamName = $"{BaseQueryStructure.PARAM_NAME_PREFIX}param1"; + + // The object is resolved through object_id() from the schema and the quoted table name, + // the same way PopulateColumnDefinitionsWithReadOnlyFlag resolves it. + // is_hidden marks the period columns of a temporal table declared + // GENERATED ALWAYS ... HIDDEN, which "SELECT *" does not return. key_ordinal is null for + // every column outside the primary key, and 1-based within it. + string query = + "select c.name as COLUMN_NAME, c.is_hidden as IS_HIDDEN, ic.key_ordinal as KEY_ORDINAL " + + "from sys.columns as c " + + "left join sys.indexes as i on i.object_id = c.object_id and i.is_primary_key = 1 " + + "left join sys.index_columns as ic on ic.object_id = c.object_id " + + "and ic.index_id = i.index_id and ic.column_id = c.column_id " + + $"where c.object_id = object_id({schemaParamName}+'.'+{tableParamName});"; + + Dictionary parameters = new() + { + { schemaParamName, new(schemaName, DbType.String) }, + { tableParamName, new(SqlQueryBuilder.QuoteTableNameAsDBConnectionParam(tableName), DbType.String) } + }; + + try + { + return await QueryExecutor.ExecuteQueryAsync( + sqltext: query, + parameters: parameters, + dataReaderHandler: SummarizeObjectCatalogMetadataAsync, + dataSourceName: _dataSourceName); + } + catch (Exception ex) + { + // sys.columns.is_hidden exists from SQL Server 2016 on. Where the catalog cannot + // answer — a dedicated SQL pool, or a login without VIEW DEFINITION — returning null + // leaves the projection at "*", which is exactly the behavior before this change. + _logger.LogDebug( + "Unable to read catalog metadata for {schemaName}.{tableName}: {message}", + schemaName, + tableName, + ex.Message); + + return null; + } + } + + /// + /// Turns the catalog rows read by into + /// . + /// Returns null when the object has no rows: that means it was not found in the catalog, and + /// claiming it has no hidden columns would be a guess. + /// + private async Task SummarizeObjectCatalogMetadataAsync( + DbDataReader reader, + List? args = null) + { + DbResultSet catalogRows = await QueryExecutor.ExtractResultSetFromDbDataReaderAsync(reader); + + if (catalogRows.Rows.Count == 0) + { + return null; + } + + ObjectCatalogMetadata catalogMetadata = new(); + List<(string ColumnName, byte KeyOrdinal)> keyColumns = new(); + + foreach (DbResultSetRow catalogRow in catalogRows.Rows) + { + Dictionary columnInfo = catalogRow.Columns; + + if (columnInfo["COLUMN_NAME"] is not string columnName) + { + continue; + } + + if (columnInfo["IS_HIDDEN"] is bool isHidden && isHidden) + { + catalogMetadata.HiddenColumns.Add(columnName); + } + + if (columnInfo["KEY_ORDINAL"] is byte keyOrdinal && keyOrdinal > 0) + { + keyColumns.Add((columnName, keyOrdinal)); + } + } + + keyColumns.Sort((left, right) => left.KeyOrdinal.CompareTo(right.KeyOrdinal)); + + foreach ((string ColumnName, byte KeyOrdinal) keyColumn in keyColumns) + { + catalogMetadata.PrimaryKeyColumns.Add(keyColumn.ColumnName); + } + + return catalogMetadata; + } + public override string GetDefaultSchemaName() { return "dbo"; diff --git a/src/Core/Services/MetadataProviders/SqlMetadataProvider.cs b/src/Core/Services/MetadataProviders/SqlMetadataProvider.cs index 570f6561b5..1c731c44ec 100644 --- a/src/Core/Services/MetadataProviders/SqlMetadataProvider.cs +++ b/src/Core/Services/MetadataProviders/SqlMetadataProvider.cs @@ -116,6 +116,77 @@ public abstract class SqlMetadataProvider : /// private readonly ConcurrentDictionary> _skippedColumnsByObject = new(StringComparer.Ordinal); + /// + /// Catalog facts per database object that the "Columns" schema collection does not report. + /// Only populated for providers that declare , since + /// only those replace "*" with an explicit projection and therefore need them. Keyed by + /// object with an ordinal comparer, for the reason given on . + /// + private readonly ConcurrentDictionary _objectCatalogMetadataCache = new(StringComparer.Ordinal); + + /// + /// The two catalog facts an explicit schema projection needs and the "Columns" schema + /// collection does not carry: which columns the database hides from "SELECT *", and which + /// columns form the object's own primary key. + /// + protected sealed class ObjectCatalogMetadata + { + /// + /// Columns the database omits from "SELECT *" — for SQL Server, the period columns of a + /// temporal table declared GENERATED ALWAYS ... HIDDEN. Naming them in a projection + /// would expose columns that are invisible today, so they are subtracted before the + /// unsupported data types are. + /// + public HashSet HiddenColumns { get; } = new(StringComparer.OrdinalIgnoreCase); + + /// + /// The columns of the object's own primary key, in key order. Replaces the key discovery + /// the data adapter performs under CommandBehavior.KeyInfo, which cannot be used once + /// the projection is narrowed: the adapter appends key columns the projection left out + /// as hidden reader columns, and an unsupported type among them reintroduces the very + /// failure the narrowing avoids. + /// + public List PrimaryKeyColumns { get; } = new(); + } + + /// + /// Reads for a database object. Returns null by default: + /// a provider only implements this when it declares . + /// When it returns null the projection stays "*" and schema discovery behaves exactly as it + /// did before. + /// + protected virtual Task GetObjectCatalogMetadataAsync( + string schemaName, + string tableName) + { + return Task.FromResult(null); + } + + /// + /// Returns for a database object, reading the catalog + /// once per object for the duration of metadata initialization. + /// + private async Task GetCachedObjectCatalogMetadataAsync( + string schemaName, + string tableName) + { + string cacheKey = GetObjectCacheKey(schemaName, tableName); + + if (_objectCatalogMetadataCache.TryGetValue(cacheKey, out ObjectCatalogMetadata? cachedMetadata)) + { + return cachedMetadata; + } + + ObjectCatalogMetadata? catalogMetadata = await GetObjectCatalogMetadataAsync(schemaName, tableName); + + if (catalogMetadata is not null) + { + _objectCatalogMetadataCache[cacheKey] = catalogMetadata; + } + + return catalogMetadata; + } + protected IAbstractQueryManagerFactory QueryManagerFactory { get; init; } /// @@ -1554,6 +1625,10 @@ private async Task PopulateSourceDefinitionAsync( if (sourceDefinition.PrimaryKey.Count == 0) { + // When the object's own primary key is unreadable, say so. The message below reads + // as a configuration mistake, and no configuration can express that key. + RejectUnreadablePrimaryKey(schemaName, tableName); + throw new DataApiBuilderException( message: $"Primary key not configured on the given database object {tableName}", statusCode: HttpStatusCode.ServiceUnavailable, @@ -1613,6 +1688,8 @@ private async Task PopulateSourceDefinitionAsync( RejectPrimaryKeyOnUnsupportedColumn(schemaName, tableName, sourceDefinition); + RejectConfiguredReferencesToSkippedColumns(entityName, entity, schemaName, tableName); + DataTable columnsInTable = await GetCachedColumnsAsync(schemaName, tableName); PopulateColumnDefinitionWithHasDefaultAndDbType( @@ -1849,12 +1926,32 @@ private async Task FillSchemaForTableAsync( await conn.OpenAsync(); + string selectStatement = $"SELECT {projection} FROM {tableNameWithSchemaPrefix}"; + + if (!string.Equals(projection, "*", StringComparison.Ordinal)) + { + // The projection left columns out, so the data adapter cannot be used here: + // FillSchema runs with CommandBehavior.KeyInfo, under which the provider performs + // its own key discovery and appends key columns missing from the SELECT list as + // hidden reader columns. A column whose CLR type the provider cannot resolve + // reintroduces "DataReader.GetFieldType(N) returned null" that way even though the + // projection excluded it — reachable through the object's own primary key, and + // through any unique index, including when a supported key is configured through + // source.key-fields. Reading the shape without KeyInfo keeps the projection + // authoritative; the primary key comes from the catalog instead. + return await ReadSchemaWithoutKeyInfoAsync( + conn, + selectStatement, + tableNameWithSchemaPrefix, + schemaName, + tableName); + } + DataAdapterT adapterForTable = new(); CommandT selectCommand = new() { Connection = conn, - CommandText - = $"SELECT {projection} FROM {tableNameWithSchemaPrefix}" + CommandText = selectStatement }; adapterForTable.SelectCommand = selectCommand; @@ -1862,6 +1959,88 @@ private async Task FillSchemaForTableAsync( return dataTable[0]; } + /// + /// Reads the shape of a narrowed projection without CommandBehavior.KeyInfo and registers + /// the result in under the name the data adapter would have + /// used, so callers find it there on subsequent lookups. The primary key is taken from the + /// catalog, because the adapter's own key discovery is precisely what has to be avoided. + /// + private async Task ReadSchemaWithoutKeyInfoAsync( + ConnectionT conn, + string selectStatement, + string tableNameWithSchemaPrefix, + string schemaName, + string tableName) + { + DataTable dataTable = new(tableNameWithSchemaPrefix); + + using (CommandT selectCommand = new()) + { + selectCommand.Connection = conn; + selectCommand.CommandText = selectStatement; + + // SchemaOnly describes the statement without returning rows. Without KeyInfo the + // reader carries exactly the projected columns and nothing else. + using DbDataReader reader = + await selectCommand.ExecuteReaderAsync(CommandBehavior.SchemaOnly); + + using DataTable? schemaTable = reader.GetSchemaTable(); + + if (schemaTable is null) + { + throw new DataApiBuilderException( + message: $"The data provider reported no schema for {schemaName}.{tableName}.", + statusCode: HttpStatusCode.ServiceUnavailable, + subStatusCode: DataApiBuilderException.SubStatusCodes.ErrorInInitialization); + } + + foreach (DataRow columnInfo in schemaTable.Rows) + { + if (columnInfo["ColumnName"] is not string columnName) + { + continue; + } + + DataColumn column = new(columnName, (Type)columnInfo["DataType"]) + { + AllowDBNull = columnInfo["AllowDBNull"] is bool allowDbNull && allowDbNull, + AutoIncrement = columnInfo["IsAutoIncrement"] is bool isAutoIncrement && isAutoIncrement + }; + + dataTable.Columns.Add(column); + } + } + + ObjectCatalogMetadata? catalogMetadata = + await GetCachedObjectCatalogMetadataAsync(schemaName, tableName); + + if (catalogMetadata is not null && catalogMetadata.PrimaryKeyColumns.Count > 0) + { + List keyColumns = new(); + + foreach (string primaryKeyColumn in catalogMetadata.PrimaryKeyColumns) + { + if (dataTable.Columns.Contains(primaryKeyColumn)) + { + keyColumns.Add(dataTable.Columns[primaryKeyColumn]!); + } + } + + // A key column left out of the projection cannot be reported as a key here, and the + // key is not silently dropped either: PopulateSourceDefinitionAsync fails through + // RejectUnreadablePrimaryKey when that key is the one in effect, while a key + // configured through source.key-fields takes precedence over this one anyway. + if (keyColumns.Count == catalogMetadata.PrimaryKeyColumns.Count) + { + dataTable.PrimaryKey = keyColumns.ToArray(); + } + } + + EntitiesDataSet.Tables.Add(dataTable); + + return dataTable; + } + /// /// Builds the projection used to read the schema of a database object. Returns "*" unless /// the object holds columns whose data type this provider cannot map to a CLR type, in @@ -1886,6 +2065,17 @@ private async Task BuildSchemaProjectionAsync(string schemaName, string try { + ObjectCatalogMetadata? catalogMetadata = + await GetCachedObjectCatalogMetadataAsync(schemaName, tableName); + + if (catalogMetadata is null) + { + // Without the catalog there is no way to tell which columns the database hides + // from "SELECT *". Naming columns anyway would add the hidden period columns of + // a temporal table to the exposed contract, so leave the projection alone. + return "*"; + } + DataTable columnsInTable = await GetCachedColumnsAsync(schemaName, tableName); foreach (DataRow columnInfo in columnsInTable.Rows) @@ -1905,11 +2095,20 @@ private async Task BuildSchemaProjectionAsync(string schemaName, string if (UnsupportedColumnDataTypes.Contains(dataType)) { skippedColumns[columnName] = dataType; + continue; } - else + + if (catalogMetadata.HiddenColumns.Contains(columnName)) { - readableColumns.Add(columnName); + // "SELECT *" does not return this column, so naming it would widen the + // exposed contract instead of preserving it. It is not "skipped": it was + // never part of the object's shape as the engine sees it, and the read-only + // classification does not recognize generated-always period columns, so a + // PUT would try to null them. + continue; } + + readableColumns.Add(columnName); } } catch (Exception ex) when (ex is not DataApiBuilderException) @@ -1983,6 +2182,117 @@ private void RejectPrimaryKeyOnUnsupportedColumn( } } + /// + /// Fails initialization with the reason when the object's own primary key includes a column + /// left out of the projection because its data type is not supported. Without this the + /// caller reports a missing primary key, which reads as something the user forgot to + /// configure even though no configuration can express that key. + /// + private void RejectUnreadablePrimaryKey(string schemaName, string tableName) + { + string cacheKey = GetObjectCacheKey(schemaName, tableName); + + if (!_skippedColumnsByObject.TryGetValue(cacheKey, out Dictionary? skippedColumns) + || !_objectCatalogMetadataCache.TryGetValue(cacheKey, out ObjectCatalogMetadata? catalogMetadata)) + { + return; + } + + foreach (string primaryKeyColumn in catalogMetadata.PrimaryKeyColumns) + { + if (skippedColumns.TryGetValue(primaryKeyColumn, out string? dataType)) + { + throw new DataApiBuilderException( + message: $"The primary key of {schemaName}.{tableName} includes the column {primaryKeyColumn}, " + + $"whose data type {dataType} is not supported. The object cannot be exposed through that " + + "key. Configure source.key-fields with a supported column that identifies a row uniquely, " + + "if the object has one.", + statusCode: HttpStatusCode.ServiceUnavailable, + subStatusCode: DataApiBuilderException.SubStatusCodes.ErrorInInitialization); + } + } + } + + /// + /// Fails initialization when the configuration names a column that was left out of the + /// projection because its data type is not supported. + /// Such a name keeps resolving after the column is gone: the exposed and backing column maps + /// are built from entity fields and mappings without requiring the column to exist in + /// , and the authorization resolver accepts explicitly + /// included field names. The reference then reaches code that indexes + /// and fails per request instead of at startup — a + /// permission-derived default projection selects the column and the read fails serializing + /// it, a mutation throws while resolving the backing column, and MCP metadata advertises a + /// field no other surface has. Rejecting here keeps the configuration and the exposed + /// contract in agreement. Nothing that worked before stops working: an object with such a + /// column failed discovery outright, so the entity never loaded. + /// + private void RejectConfiguredReferencesToSkippedColumns( + string entityName, + Entity? entity, + string schemaName, + string tableName) + { + if (entity is null + || !_skippedColumnsByObject.TryGetValue( + GetObjectCacheKey(schemaName, tableName), + out Dictionary? skippedColumns)) + { + return; + } + + // "mappings" and "fields" both key on the backing column name. + if (entity.Mappings is not null) + { + foreach (string backingColumn in entity.Mappings.Keys) + { + RejectReference(backingColumn, "mappings"); + } + } + + if (entity.Fields is not null) + { + foreach (FieldMetadata field in entity.Fields) + { + RejectReference(field.Name, "fields"); + } + } + + foreach (EntityPermission permission in entity.Permissions) + { + foreach (EntityAction action in permission.Actions) + { + if (action.Fields?.Include is null) + { + continue; + } + + // An include list names columns that have to be readable. An exclude list naming + // one of these columns asks for what already happened, so it is left alone. + foreach (string includedField in action.Fields.Include) + { + RejectReference(includedField, $"permissions of role {permission.Role}"); + } + } + } + + void RejectReference(string configuredName, string configurationSection) + { + if (!skippedColumns.TryGetValue(configuredName, out string? dataType)) + { + return; + } + + throw new DataApiBuilderException( + message: $"The {configurationSection} of entity {entityName} reference the column {configuredName} " + + $"of {schemaName}.{tableName}, whose data type {dataType} is not supported. That column is not " + + "part of the exposed contract, so the reference cannot be honored. Remove it from the " + + "configuration.", + statusCode: HttpStatusCode.ServiceUnavailable, + subStatusCode: DataApiBuilderException.SubStatusCodes.ErrorInInitialization); + } + } + /// /// Renders skipped columns as "name (type)" pairs for log and error messages. /// @@ -2015,6 +2325,7 @@ private void ReleaseCatalogMetadataCaches() _columnsMetadataCache.Clear(); _skippedColumnsByObject.Clear(); + _objectCatalogMetadataCache.Clear(); } /// diff --git a/src/Service.Tests/DatabaseSchema-MsSql.sql b/src/Service.Tests/DatabaseSchema-MsSql.sql index 0850e0e551..d94ef77c48 100644 --- a/src/Service.Tests/DatabaseSchema-MsSql.sql +++ b/src/Service.Tests/DatabaseSchema-MsSql.sql @@ -46,6 +46,15 @@ DROP TABLE IF EXISTS type_table; DROP TABLE IF EXISTS vector_type_table; DROP TABLE IF EXISTS vector_owners; DROP TABLE IF EXISTS geometry_type_table; +DROP TABLE IF EXISTS hierarchyid_pk_table; +DROP TABLE IF EXISTS hierarchyid_composite_pk_table; +DROP TABLE IF EXISTS hierarchyid_unique_table; +-- System versioning has to be released before the temporal table can be dropped. +IF OBJECT_ID('dbo.temporal_geometry_type_table', 'U') IS NOT NULL + AND OBJECTPROPERTY(OBJECT_ID('dbo.temporal_geometry_type_table'), 'TableTemporalType') = 2 + ALTER TABLE temporal_geometry_type_table SET (SYSTEM_VERSIONING = OFF); +DROP TABLE IF EXISTS temporal_geometry_type_table; +DROP TABLE IF EXISTS temporal_geometry_type_table_history; DROP TABLE IF EXISTS profiles; DROP TABLE IF EXISTS trees; DROP TABLE IF EXISTS fungi; @@ -260,6 +269,43 @@ CREATE TABLE geometry_type_table( geom geometry NULL ); +-- The period columns are HIDDEN, so SELECT * does not return them. An explicit projection built +-- from the catalog would, which is what this fixture guards against. +CREATE TABLE temporal_geometry_type_table( + id int NOT NULL PRIMARY KEY, + name varchar(100) NOT NULL, + geom geometry NULL, + valid_from datetime2 GENERATED ALWAYS AS ROW START HIDDEN NOT NULL, + valid_to datetime2 GENERATED ALWAYS AS ROW END HIDDEN NOT NULL, + PERIOD FOR SYSTEM_TIME (valid_from, valid_to) +) +WITH (SYSTEM_VERSIONING = ON (HISTORY_TABLE = dbo.temporal_geometry_type_table_history)); + +-- The database primary key is itself of an unsupported type, so no projection can expose the +-- object: the engine needs the key it cannot read. +CREATE TABLE hierarchyid_pk_table( + node hierarchyid NOT NULL PRIMARY KEY, + name varchar(100) NOT NULL +); + +-- Same, with the unsupported column as one member of a composite key. +CREATE TABLE hierarchyid_composite_pk_table( + tenant_id int NOT NULL, + node hierarchyid NOT NULL, + name varchar(100) NOT NULL, + CONSTRAINT PK_hierarchyid_composite_pk_table PRIMARY KEY (tenant_id, node) +); + +-- No database primary key, and the unsupported column carries a unique index. This object is +-- expected to load, with the key supplied through source.key-fields: the data adapter's own key +-- discovery would otherwise pull the unique column back in as a hidden reader column. +CREATE TABLE hierarchyid_unique_table( + id int NOT NULL, + node hierarchyid NOT NULL, + name varchar(100) NOT NULL, + CONSTRAINT UQ_hierarchyid_unique_table_node UNIQUE (node) +); + CREATE TABLE profiles( id int IDENTITY(5001, 1) PRIMARY KEY, metadata json NULL @@ -671,6 +717,20 @@ VALUES (2, 'null geometry', NULL); SET IDENTITY_INSERT geometry_type_table OFF +INSERT INTO temporal_geometry_type_table(id, name, geom) +VALUES + (1, 'point', geometry::STGeomFromText('POINT(1 2)', 0)), + (2, 'null geometry', NULL); + +INSERT INTO hierarchyid_pk_table(node, name) +VALUES (hierarchyid::Parse('/1/'), 'root'); + +INSERT INTO hierarchyid_composite_pk_table(tenant_id, node, name) +VALUES (1, hierarchyid::Parse('/1/'), 'root'); + +INSERT INTO hierarchyid_unique_table(id, node, name) +VALUES (1, hierarchyid::Parse('/1/'), 'root'); + SET IDENTITY_INSERT profiles ON INSERT INTO profiles(id, metadata) VALUES diff --git a/src/Service.Tests/UnitTests/SqlMetadataProviderUnitTests.cs b/src/Service.Tests/UnitTests/SqlMetadataProviderUnitTests.cs index 180299603f..68f56a80c2 100644 --- a/src/Service.Tests/UnitTests/SqlMetadataProviderUnitTests.cs +++ b/src/Service.Tests/UnitTests/SqlMetadataProviderUnitTests.cs @@ -537,6 +537,260 @@ public async Task ValidatePrimaryKeyOnUnsupportedColumnFailsInitialization() TestHelper.UnsetAllDABEnvironmentVariables(); } + /// + /// Test to validate that the period columns of a temporal table declared + /// GENERATED ALWAYS ... HIDDEN stay out of the inferred source definition. + /// "SELECT *" does not return them, so an explicit projection built from the catalog must not + /// name them either: adding them widens the exposed contract, and the read-only + /// classification does not recognize generated-always period columns, so an overwriting PUT + /// would try to null them and fail. + /// + [TestMethod, TestCategory(TestCategory.MSSQL)] + public async Task ValidateHiddenPeriodColumnsAreNotInferred() + { + DatabaseEngine = TestCategory.MSSQL; + TestHelper.SetupDatabaseEnvironment(DatabaseEngine); + + await SetUpSingleEntityMetadataProviderAsync( + "TemporalGeometryType", + BuildReadOnlyEntity( + entityName: "TemporalGeometryType", + databaseObject: "dbo.temporal_geometry_type_table", + sourceType: EntitySourceType.Table, + keyFields: new string[] { "id" })); + + await _sqlMetadataProvider.InitializeAsync(); + + Assert.IsTrue( + _sqlMetadataProvider.GetEntityNamesAndDbObjects().TryGetValue("TemporalGeometryType", out DatabaseObject databaseObject), + message: "Metadata inference failed for the entity backed by a temporal table."); + + SourceDefinition sourceDefinition = databaseObject.SourceDefinition; + + Assert.IsTrue( + sourceDefinition.Columns.ContainsKey("id"), + message: "The configured key column is expected in the source definition."); + Assert.IsTrue( + sourceDefinition.Columns.ContainsKey("name"), + message: "A column with a supported data type is expected in the source definition."); + Assert.IsFalse( + sourceDefinition.Columns.ContainsKey("geom"), + message: "A column whose data type cannot be mapped is not expected in the source definition."); + Assert.IsFalse( + sourceDefinition.Columns.ContainsKey("valid_from"), + message: "A HIDDEN period column is not returned by SELECT * and is not expected in the source definition."); + Assert.IsFalse( + sourceDefinition.Columns.ContainsKey("valid_to"), + message: "A HIDDEN period column is not returned by SELECT * and is not expected in the source definition."); + + TestHelper.UnsetAllDABEnvironmentVariables(); + } + + /// + /// Test to validate that an object whose own database primary key is a column of an + /// unsupported data type fails initialization with the reason. + /// The projection cannot carry that column, and the engine cannot operate on the object + /// without its key, so the object is unreachable either way. What this asserts is that the + /// failure names the column and its type instead of reporting a missing primary key, which + /// reads as something the user forgot to configure. + /// + [TestMethod, TestCategory(TestCategory.MSSQL)] + public async Task ValidateUnsupportedDatabasePrimaryKeyFailsInitialization() + { + DatabaseEngine = TestCategory.MSSQL; + TestHelper.SetupDatabaseEnvironment(DatabaseEngine); + + await SetUpSingleEntityMetadataProviderAsync( + "HierarchyIdKeyed", + BuildReadOnlyEntity( + entityName: "HierarchyIdKeyed", + databaseObject: "dbo.hierarchyid_pk_table", + sourceType: EntitySourceType.Table, + keyFields: null)); + + try + { + await _sqlMetadataProvider.InitializeAsync(); + Assert.Fail("Expected DataApiBuilderException was not thrown for a database primary key of an unsupported data type."); + } + catch (DataApiBuilderException ex) + { + Assert.AreEqual(HttpStatusCode.ServiceUnavailable, ex.StatusCode); + Assert.AreEqual(DataApiBuilderException.SubStatusCodes.ErrorInInitialization, ex.SubStatusCode); + Assert.IsTrue( + ex.Message.Contains("node") && ex.Message.Contains("hierarchyid"), + message: $"The error is expected to name the key column and its data type. Actual message: {ex.Message}"); + } + + TestHelper.UnsetAllDABEnvironmentVariables(); + } + + /// + /// Test to validate the same rejection when the unsupported column is one member of a + /// composite database primary key. The supported member alone does not identify a row, so + /// the object cannot be exposed through a partial key. + /// + [TestMethod, TestCategory(TestCategory.MSSQL)] + public async Task ValidateUnsupportedColumnInCompositeDatabasePrimaryKeyFailsInitialization() + { + DatabaseEngine = TestCategory.MSSQL; + TestHelper.SetupDatabaseEnvironment(DatabaseEngine); + + await SetUpSingleEntityMetadataProviderAsync( + "HierarchyIdCompositeKeyed", + BuildReadOnlyEntity( + entityName: "HierarchyIdCompositeKeyed", + databaseObject: "dbo.hierarchyid_composite_pk_table", + sourceType: EntitySourceType.Table, + keyFields: null)); + + try + { + await _sqlMetadataProvider.InitializeAsync(); + Assert.Fail("Expected DataApiBuilderException was not thrown for a composite database primary key holding an unsupported data type."); + } + catch (DataApiBuilderException ex) + { + Assert.AreEqual(HttpStatusCode.ServiceUnavailable, ex.StatusCode); + Assert.AreEqual(DataApiBuilderException.SubStatusCodes.ErrorInInitialization, ex.SubStatusCode); + Assert.IsTrue( + ex.Message.Contains("node") && ex.Message.Contains("hierarchyid"), + message: $"The error is expected to name the key column and its data type. Actual message: {ex.Message}"); + } + + TestHelper.UnsetAllDABEnvironmentVariables(); + } + + /// + /// Test to validate that an object carrying a unique index over a column of an unsupported + /// data type loads when a supported key is configured through source.key-fields. + /// This is the case the data adapter cannot serve: FillSchema runs with + /// CommandBehavior.KeyInfo, under which the provider performs its own key discovery and + /// appends key columns missing from the SELECT list as hidden reader columns - so the + /// unsupported unique column comes back regardless of the projection, and configuring a + /// supported key does not change that. Schema discovery therefore reads the shape without + /// KeyInfo once the projection is narrowed, and takes the primary key from the catalog. + /// + [TestMethod, TestCategory(TestCategory.MSSQL)] + public async Task ValidateConfiguredKeyIsUsedWhenUniqueIndexColumnIsUnsupported() + { + DatabaseEngine = TestCategory.MSSQL; + TestHelper.SetupDatabaseEnvironment(DatabaseEngine); + + await SetUpSingleEntityMetadataProviderAsync( + "HierarchyIdUnique", + BuildReadOnlyEntity( + entityName: "HierarchyIdUnique", + databaseObject: "dbo.hierarchyid_unique_table", + sourceType: EntitySourceType.Table, + keyFields: new string[] { "id" })); + + await _sqlMetadataProvider.InitializeAsync(); + + Assert.IsTrue( + _sqlMetadataProvider.GetEntityNamesAndDbObjects().TryGetValue("HierarchyIdUnique", out DatabaseObject databaseObject), + message: "Metadata inference failed for an object whose unique index covers an unsupported column."); + + SourceDefinition sourceDefinition = databaseObject.SourceDefinition; + + Assert.IsTrue( + sourceDefinition.Columns.ContainsKey("id"), + message: "The configured key column is expected in the source definition."); + Assert.IsTrue( + sourceDefinition.Columns.ContainsKey("name"), + message: "A column with a supported data type is expected in the source definition."); + Assert.IsFalse( + sourceDefinition.Columns.ContainsKey("node"), + message: "The unsupported unique column is not expected in the source definition."); + CollectionAssert.AreEqual( + new List { "id" }, + sourceDefinition.PrimaryKey, + message: "The configured key is expected to be the primary key in effect."); + + TestHelper.UnsetAllDABEnvironmentVariables(); + } + + /// + /// Test to validate that configuration naming a column left out of the projection fails + /// initialization. + /// Such a name keeps resolving after the column is gone, because the exposed and backing + /// column maps are built from entity fields and mappings without requiring the column to + /// exist in the source definition. The reference then reaches code that indexes the source + /// definition columns and fails per request rather than at startup. + /// + [TestMethod, TestCategory(TestCategory.MSSQL)] + public async Task ValidateConfiguredReferenceToUnsupportedColumnFailsInitialization() + { + DatabaseEngine = TestCategory.MSSQL; + TestHelper.SetupDatabaseEnvironment(DatabaseEngine); + + await SetUpSingleEntityMetadataProviderAsync( + "GeometryAliased", + BuildReadOnlyEntity( + entityName: "GeometryAliased", + databaseObject: "dbo.geometry_type_table", + sourceType: EntitySourceType.Table, + keyFields: new string[] { "id" }, + mappings: new Dictionary { { "geom", "Position" } })); + + try + { + await _sqlMetadataProvider.InitializeAsync(); + Assert.Fail("Expected DataApiBuilderException was not thrown for a mapping over a column of an unsupported data type."); + } + catch (DataApiBuilderException ex) + { + Assert.AreEqual(HttpStatusCode.ServiceUnavailable, ex.StatusCode); + Assert.AreEqual(DataApiBuilderException.SubStatusCodes.ErrorInInitialization, ex.SubStatusCode); + Assert.IsTrue( + ex.Message.Contains("geom") && ex.Message.Contains("mappings"), + message: $"The error is expected to name the column and the configuration section referencing it. Actual message: {ex.Message}"); + } + + TestHelper.UnsetAllDABEnvironmentVariables(); + } + + /// + /// Builds a metadata provider over a single in-memory entity and resets the database state. + /// The objects exercised by the unsupported-data-type tests are declared in memory rather + /// than in dab-config.MsSql.json, because several of them fail by design and every MSSQL + /// fixture initializes every configured entity. + /// + private static async Task SetUpSingleEntityMetadataProviderAsync(string entityName, Entity entity) + { + RuntimeConfig runtimeConfig = SqlTestHelper.SetupRuntimeConfig() + with + { Entities = new RuntimeEntities(new Dictionary { { entityName, entity } }) }; + RuntimeConfigProvider runtimeConfigProvider = TestHelper.GenerateInMemoryRuntimeConfigProvider(runtimeConfig); + SetUpSQLMetadataProvider(runtimeConfigProvider); + await ResetDbStateAsync(); + } + + /// + /// Builds a read-only entity over a database object, with optional configured key fields and + /// mappings. + /// + private static Entity BuildReadOnlyEntity( + string entityName, + string databaseObject, + EntitySourceType sourceType, + string[] keyFields, + Dictionary mappings = null) + { + return new Entity( + Source: new(databaseObject, sourceType, null, keyFields), + Fields: null, + Rest: new(Enabled: true), + GraphQL: new(entityName, $"{entityName}s", Enabled: true), + Permissions: new EntityPermission[] + { + new(Role: "anonymous", + Actions: new EntityAction[] { new(Action: EntityActionOperation.Read, Fields: null, Policy: null) }) + }, + Relationships: null, + Mappings: mappings); + } + /// /// Test to validate successful inference of relationship data based on data provided in the config and the metadata /// collected from the MySql database. From fc0e094e93a9228a2deb375a2acb6d97cf42d4d6 Mon Sep 17 00:00:00 2001 From: Joymax Andrade do Nascimento Date: Mon, 14 Sep 2026 22:37:33 -0300 Subject: [PATCH 11/12] Correct identifier delimiting, identity typing and key inference on the narrowed path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit object_id() now delimits both name parts through QUOTENAME, so a schema or table needing quoting resolves instead of returning no rows and silently leaving the projection at SELECT *. Identity no longer travels through DataColumn.AutoIncrement, whose setter coerces a DataType it cannot increment to Int32 — SQL Server allows identity on tinyint, numeric and decimal. It comes from sys.columns.is_identity and is applied after the columns are populated, so the provider-reported DataType is preserved. Absent a database primary key, the first non-nullable unique index whose key columns are all readable is inferred, in index order. The data adapter does this on the unnarrowed path, and dropping it made the narrowed path demand source.key-fields for objects the other path resolves on its own. The catalog read is deferred until an object is known to hold an unsupported type, so objects that need no narrowing no longer pay an extra query per startup. Adds unique_key_geometry_table and ValidateUniqueKeyIsInferredWhenNoDatabasePrimaryKeyExists. Addresses the Copilot review on #3802. --- .../MsSqlMetadataProvider.cs | 101 +++++++- .../MetadataProviders/SqlMetadataProvider.cs | 242 ++++++++++++++---- src/Service.Tests/DatabaseSchema-MsSql.sql | 16 ++ .../UnitTests/SqlMetadataProviderUnitTests.cs | 43 ++++ 4 files changed, 336 insertions(+), 66 deletions(-) diff --git a/src/Core/Services/MetadataProviders/MsSqlMetadataProvider.cs b/src/Core/Services/MetadataProviders/MsSqlMetadataProvider.cs index 0a37d7e695..cde419fd6e 100644 --- a/src/Core/Services/MetadataProviders/MsSqlMetadataProvider.cs +++ b/src/Core/Services/MetadataProviders/MsSqlMetadataProvider.cs @@ -70,23 +70,36 @@ public MsSqlMetadataProvider( string schemaParamName = $"{BaseQueryStructure.PARAM_NAME_PREFIX}param0"; string tableParamName = $"{BaseQueryStructure.PARAM_NAME_PREFIX}param1"; - // The object is resolved through object_id() from the schema and the quoted table name, - // the same way PopulateColumnDefinitionsWithReadOnlyFlag resolves it. + // The object is resolved through object_id(). Both name parts go through QUOTENAME: + // object_id() parses its argument as a multi-part name, so an unquoted schema or table + // holding a dot, a space, a reserved word or a closing bracket resolves to the wrong + // object or to null — and a null object_id returns no rows, which would leave the + // projection at "SELECT *" and bring #3801 back for that object. QUOTENAME also doubles + // an embedded "]", so the names are passed raw and quoted by the server. // is_hidden marks the period columns of a temporal table declared // GENERATED ALWAYS ... HIDDEN, which "SELECT *" does not return. key_ordinal is null for - // every column outside the primary key, and 1-based within it. + // every column outside the primary key, and 1-based within it; an index's included + // columns report 0 and are not part of the key. + // Unique indexes are returned alongside the primary key, because absent a primary key + // the data adapter reports a non-nullable unique key as DataTable.PrimaryKey on the + // unnarrowed path, and the narrowed path has to do the same. Filtered and disabled + // indexes do not identify every row, and an index's included columns report key_ordinal + // 0 and are not part of its key. string query = - "select c.name as COLUMN_NAME, c.is_hidden as IS_HIDDEN, ic.key_ordinal as KEY_ORDINAL " + "select c.name as COLUMN_NAME, c.is_hidden as IS_HIDDEN, c.is_identity as IS_IDENTITY, " + + "c.is_nullable as IS_NULLABLE, i.index_id as INDEX_ID, " + + "i.is_primary_key as IS_PRIMARY_KEY, ic.key_ordinal as KEY_ORDINAL " + "from sys.columns as c " - + "left join sys.indexes as i on i.object_id = c.object_id and i.is_primary_key = 1 " + "left join sys.index_columns as ic on ic.object_id = c.object_id " - + "and ic.index_id = i.index_id and ic.column_id = c.column_id " - + $"where c.object_id = object_id({schemaParamName}+'.'+{tableParamName});"; + + "and ic.column_id = c.column_id and ic.key_ordinal > 0 " + + "left join sys.indexes as i on i.object_id = ic.object_id and i.index_id = ic.index_id " + + "and i.is_unique = 1 and i.is_disabled = 0 and i.has_filter = 0 " + + $"where c.object_id = object_id(quotename({schemaParamName})+'.'+quotename({tableParamName}));"; Dictionary parameters = new() { { schemaParamName, new(schemaName, DbType.String) }, - { tableParamName, new(SqlQueryBuilder.QuoteTableNameAsDBConnectionParam(tableName), DbType.String) } + { tableParamName, new(tableName, DbType.String) } }; try @@ -130,7 +143,8 @@ public MsSqlMetadataProvider( } ObjectCatalogMetadata catalogMetadata = new(); - List<(string ColumnName, byte KeyOrdinal)> keyColumns = new(); + Dictionary nullabilityByColumn = new(StringComparer.OrdinalIgnoreCase); + Dictionary indexKeysByIndexId = new(); foreach (DbResultSetRow catalogRow in catalogRows.Rows) { @@ -146,22 +160,81 @@ public MsSqlMetadataProvider( catalogMetadata.HiddenColumns.Add(columnName); } - if (columnInfo["KEY_ORDINAL"] is byte keyOrdinal && keyOrdinal > 0) + if (columnInfo["IS_IDENTITY"] is bool isIdentity && isIdentity) { - keyColumns.Add((columnName, keyOrdinal)); + catalogMetadata.IdentityColumns.Add(columnName); } + + // A column the catalog does not describe as non-nullable is treated as nullable, so + // an unreadable flag can only disqualify a unique key, never promote one. + nullabilityByColumn[columnName] = columnInfo["IS_NULLABLE"] is not bool isNullable || isNullable; + + // A column outside every eligible unique index carries nulls for the index members. + if (columnInfo["INDEX_ID"] is not int indexId + || columnInfo["KEY_ORDINAL"] is not byte keyOrdinal) + { + continue; + } + + if (!indexKeysByIndexId.TryGetValue(indexId, out CatalogIndexKey? indexKey)) + { + indexKey = new CatalogIndexKey + { + IsPrimaryKey = columnInfo["IS_PRIMARY_KEY"] is bool isPrimaryKey && isPrimaryKey + }; + indexKeysByIndexId[indexId] = indexKey; + } + + indexKey.Columns.Add((columnName, keyOrdinal)); } - keyColumns.Sort((left, right) => left.KeyOrdinal.CompareTo(right.KeyOrdinal)); + // Index order is creation order, which makes the candidate choice deterministic. + List indexIds = new(indexKeysByIndexId.Keys); + indexIds.Sort(); - foreach ((string ColumnName, byte KeyOrdinal) keyColumn in keyColumns) + foreach (int indexId in indexIds) { - catalogMetadata.PrimaryKeyColumns.Add(keyColumn.ColumnName); + CatalogIndexKey indexKey = indexKeysByIndexId[indexId]; + indexKey.Columns.Sort((left, right) => left.KeyOrdinal.CompareTo(right.KeyOrdinal)); + + List keyColumns = new(); + bool holdsNoNull = true; + + foreach ((string ColumnName, byte KeyOrdinal) keyColumn in indexKey.Columns) + { + keyColumns.Add(keyColumn.ColumnName); + + if (nullabilityByColumn.TryGetValue(keyColumn.ColumnName, out bool isNullable) && isNullable) + { + holdsNoNull = false; + } + } + + if (indexKey.IsPrimaryKey) + { + catalogMetadata.PrimaryKeyColumns.AddRange(keyColumns); + } + else if (holdsNoNull) + { + // Matches the data adapter, which promotes a unique key to the primary key only + // when none of its columns can hold a null. + catalogMetadata.UniqueKeyCandidates.Add(keyColumns); + } } return catalogMetadata; } + /// + /// One unique index of a database object, while its key columns are being collected. + /// + private sealed class CatalogIndexKey + { + public bool IsPrimaryKey { get; init; } + + public List<(string ColumnName, byte KeyOrdinal)> Columns { get; } = new(); + } + public override string GetDefaultSchemaName() { return "dbo"; diff --git a/src/Core/Services/MetadataProviders/SqlMetadataProvider.cs b/src/Core/Services/MetadataProviders/SqlMetadataProvider.cs index 1c731c44ec..872df289eb 100644 --- a/src/Core/Services/MetadataProviders/SqlMetadataProvider.cs +++ b/src/Core/Services/MetadataProviders/SqlMetadataProvider.cs @@ -125,9 +125,13 @@ public abstract class SqlMetadataProvider : private readonly ConcurrentDictionary _objectCatalogMetadataCache = new(StringComparer.Ordinal); /// - /// The two catalog facts an explicit schema projection needs and the "Columns" schema - /// collection does not carry: which columns the database hides from "SELECT *", and which - /// columns form the object's own primary key. + /// The catalog facts an explicit schema projection needs and the "Columns" schema collection + /// does not carry: which columns the database hides from "SELECT *", which columns identify + /// a row, and which are identity columns. Together they replace what the data adapter + /// reports under CommandBehavior.KeyInfo, which cannot be used once the projection is + /// narrowed: the adapter appends key columns the projection left out as hidden reader + /// columns, and an unsupported type among them reintroduces the very failure the narrowing + /// avoids. /// protected sealed class ObjectCatalogMetadata { @@ -140,13 +144,30 @@ protected sealed class ObjectCatalogMetadata public HashSet HiddenColumns { get; } = new(StringComparer.OrdinalIgnoreCase); /// - /// The columns of the object's own primary key, in key order. Replaces the key discovery - /// the data adapter performs under CommandBehavior.KeyInfo, which cannot be used once - /// the projection is narrowed: the adapter appends key columns the projection left out - /// as hidden reader columns, and an unsupported type among them reintroduces the very - /// failure the narrowing avoids. + /// Identity columns. Carried separately rather than through + /// , whose setter coerces a DataType it cannot + /// increment to Int32: SQL Server allows identity on tinyint, numeric and decimal, and + /// the coercion would report the wrong SystemType, which reaches parameter typing and + /// the generated API schemas. + /// + public HashSet IdentityColumns { get; } = new(StringComparer.OrdinalIgnoreCase); + + /// + /// The columns of the object's own primary key, in key order. Empty when the object has + /// none. /// public List PrimaryKeyColumns { get; } = new(); + + /// + /// Unique indexes eligible to identify a row when the object has no primary key, in + /// index order, each holding its key columns in key order. Only indexes whose every key + /// column is non-nullable qualify. + /// Preserves what the data adapter does on the unnarrowed path: absent a primary key it + /// reports such a unique key as , and dropping that + /// would make the narrowed path demand source.key-fields for an object the other path + /// resolves on its own. + /// + public List> UniqueKeyCandidates { get; } = new(); } /// @@ -1686,6 +1707,8 @@ private async Task PopulateSourceDefinitionAsync( sourceDefinition.Columns.TryAdd(columnName, column); } + ApplyIdentityColumnsFromCatalog(schemaName, tableName, sourceDefinition); + RejectPrimaryKeyOnUnsupportedColumn(schemaName, tableName, sourceDefinition); RejectConfiguredReferencesToSkippedColumns(entityName, entity, schemaName, tableName); @@ -1924,11 +1947,20 @@ private async Task FillSchemaForTableAsync( // "Max Pool Size=1" the inner open waits for a connection the outer scope still holds. string projection = await BuildSchemaProjectionAsync(schemaName, tableName); + bool isProjectionNarrowed = !string.Equals(projection, "*", StringComparison.Ordinal); + + // Resolved before the connection below is opened, for the same reason as the projection: + // reading the catalog uses a connection of its own, and nesting that inside an already + // open one exhausts a small pool. + ObjectCatalogMetadata? catalogMetadata = isProjectionNarrowed + ? await GetCachedObjectCatalogMetadataAsync(schemaName, tableName) + : null; + await conn.OpenAsync(); string selectStatement = $"SELECT {projection} FROM {tableNameWithSchemaPrefix}"; - if (!string.Equals(projection, "*", StringComparison.Ordinal)) + if (isProjectionNarrowed) { // The projection left columns out, so the data adapter cannot be used here: // FillSchema runs with CommandBehavior.KeyInfo, under which the provider performs @@ -1944,7 +1976,8 @@ private async Task FillSchemaForTableAsync( selectStatement, tableNameWithSchemaPrefix, schemaName, - tableName); + tableName, + catalogMetadata); } DataAdapterT adapterForTable = new(); @@ -1970,7 +2003,8 @@ private async Task ReadSchemaWithoutKeyInfoAsync( string selectStatement, string tableNameWithSchemaPrefix, string schemaName, - string tableName) + string tableName, + ObjectCatalogMetadata? catalogMetadata) { DataTable dataTable = new(tableNameWithSchemaPrefix); @@ -2001,44 +2035,87 @@ private async Task ReadSchemaWithoutKeyInfoAsync( continue; } + // The provider-reported DataType is carried through unchanged. Auto-increment is + // deliberately not set here — see ApplyIdentityColumnsFromCatalog for why + // DataColumn.AutoIncrement cannot be the transport for it. DataColumn column = new(columnName, (Type)columnInfo["DataType"]) { - AllowDBNull = columnInfo["AllowDBNull"] is bool allowDbNull && allowDbNull, - AutoIncrement = columnInfo["IsAutoIncrement"] is bool isAutoIncrement && isAutoIncrement + // Unknown nullability is treated as nullable. A column wrongly marked + // non-nullable is reported as required through REST, GraphQL and OpenAPI and + // rejects writes the database would accept, so the permissive direction is + // the safe one when the provider does not report the flag. + AllowDBNull = columnInfo["AllowDBNull"] is not bool allowDbNull || allowDbNull }; dataTable.Columns.Add(column); } } - ObjectCatalogMetadata? catalogMetadata = - await GetCachedObjectCatalogMetadataAsync(schemaName, tableName); - - if (catalogMetadata is not null && catalogMetadata.PrimaryKeyColumns.Count > 0) + if (catalogMetadata is not null) { - List keyColumns = new(); + DataColumn[]? keyColumns = ResolveKeyColumns(dataTable, catalogMetadata); - foreach (string primaryKeyColumn in catalogMetadata.PrimaryKeyColumns) + if (keyColumns is not null) { - if (dataTable.Columns.Contains(primaryKeyColumn)) - { - keyColumns.Add(dataTable.Columns[primaryKeyColumn]!); - } + dataTable.PrimaryKey = keyColumns; } + } + + EntitiesDataSet.Tables.Add(dataTable); + + return dataTable; + } + + /// + /// Picks the columns to report as on the narrowed path. + /// The object's own primary key wins. A key column the projection left out is not reported, + /// and the key is not silently replaced by another candidate either: + /// PopulateSourceDefinitionAsync fails through RejectUnreadablePrimaryKey when that key is + /// the one in effect, and a key configured through source.key-fields takes precedence over + /// this one anyway. + /// Absent a primary key, the first unique index whose every key column is present is used, + /// which is what the data adapter reports on the unnarrowed path. Returns null when nothing + /// identifies a row, leaving the caller to report a missing primary key as it does today. + /// + private static DataColumn[]? ResolveKeyColumns(DataTable dataTable, ObjectCatalogMetadata catalogMetadata) + { + if (catalogMetadata.PrimaryKeyColumns.Count > 0) + { + return TryResolveColumns(dataTable, catalogMetadata.PrimaryKeyColumns); + } + + foreach (List uniqueKeyCandidate in catalogMetadata.UniqueKeyCandidates) + { + DataColumn[]? keyColumns = TryResolveColumns(dataTable, uniqueKeyCandidate); - // A key column left out of the projection cannot be reported as a key here, and the - // key is not silently dropped either: PopulateSourceDefinitionAsync fails through - // RejectUnreadablePrimaryKey when that key is the one in effect, while a key - // configured through source.key-fields takes precedence over this one anyway. - if (keyColumns.Count == catalogMetadata.PrimaryKeyColumns.Count) + if (keyColumns is not null) { - dataTable.PrimaryKey = keyColumns.ToArray(); + return keyColumns; } } - EntitiesDataSet.Tables.Add(dataTable); + return null; + } - return dataTable; + /// + /// Resolves every named column against the table, in the order given, or returns null when + /// one of them is absent. + /// + private static DataColumn[]? TryResolveColumns(DataTable dataTable, List columnNames) + { + DataColumn[] columns = new DataColumn[columnNames.Count]; + + for (int index = 0; index < columnNames.Count; index++) + { + if (!dataTable.Columns.Contains(columnNames[index])) + { + return null; + } + + columns[index] = dataTable.Columns[columnNames[index]]!; + } + + return columns; } /// @@ -2065,19 +2142,14 @@ private async Task BuildSchemaProjectionAsync(string schemaName, string try { - ObjectCatalogMetadata? catalogMetadata = - await GetCachedObjectCatalogMetadataAsync(schemaName, tableName); - - if (catalogMetadata is null) - { - // Without the catalog there is no way to tell which columns the database hides - // from "SELECT *". Naming columns anyway would add the hidden period columns of - // a temporal table to the exposed contract, so leave the projection alone. - return "*"; - } - DataTable columnsInTable = await GetCachedColumnsAsync(schemaName, tableName); + // Classify from the "Columns" schema collection first, which is read for every + // object anyway. Only an object that actually holds an unsupported type needs the + // additional catalog facts below; asking for them up front would add a query per + // MSSQL and DWSQL object to every startup. + List supportedColumns = new(); + foreach (DataRow columnInfo in columnsInTable.Rows) { if (columnInfo["COLUMN_NAME"] is not string columnName) @@ -2095,9 +2167,42 @@ private async Task BuildSchemaProjectionAsync(string schemaName, string if (UnsupportedColumnDataTypes.Contains(dataType)) { skippedColumns[columnName] = dataType; - continue; } + else + { + supportedColumns.Add(columnName); + } + } + + if (skippedColumns.Count == 0) + { + return "*"; + } + + if (supportedColumns.Count == 0) + { + // Falling back to "*" here would re-issue the very projection that fails, so the + // caller would see the opaque provider error instead of the reason for it. + throw new DataApiBuilderException( + message: $"Every column of {schemaName}.{tableName} has a data type that is not supported: " + + $"{FormatSkippedColumns(skippedColumns)}. The object cannot be exposed.", + statusCode: HttpStatusCode.ServiceUnavailable, + subStatusCode: DataApiBuilderException.SubStatusCodes.ErrorInInitialization); + } + + ObjectCatalogMetadata? catalogMetadata = + await GetCachedObjectCatalogMetadataAsync(schemaName, tableName); + + if (catalogMetadata is null) + { + // Without the catalog there is no way to tell which columns the database hides + // from "SELECT *". Naming columns anyway would add the hidden period columns of + // a temporal table to the exposed contract, so leave the projection alone. + return "*"; + } + foreach (string columnName in supportedColumns) + { if (catalogMetadata.HiddenColumns.Contains(columnName)) { // "SELECT *" does not return this column, so naming it would widen the @@ -2123,18 +2228,16 @@ private async Task BuildSchemaProjectionAsync(string schemaName, string return "*"; } - if (skippedColumns.Count == 0) - { - return "*"; - } - if (readableColumns.Count == 0) { - // Falling back to "*" here would re-issue the very projection that fails, so the - // caller would see the opaque provider error instead of the reason for it. + // Every column that is not of an unsupported type is one the database hides from + // "SELECT *". Naming the hidden ones is not an option, and "*" would re-issue the + // projection that fails, so nothing about this object can be read. throw new DataApiBuilderException( - message: $"Every column of {schemaName}.{tableName} has a data type that is not supported: " - + $"{FormatSkippedColumns(skippedColumns)}. The object cannot be exposed.", + message: $"No column of {schemaName}.{tableName} can be read: " + + $"{FormatSkippedColumns(skippedColumns)} have a data type that is not supported, and " + + "every remaining column is one the database does not return from a SELECT *. " + + "The object cannot be exposed.", statusCode: HttpStatusCode.ServiceUnavailable, subStatusCode: DataApiBuilderException.SubStatusCodes.ErrorInInitialization); } @@ -2182,6 +2285,41 @@ private void RejectPrimaryKeyOnUnsupportedColumn( } } + /// + /// Marks identity columns on the narrowed schema discovery path, where the shape is read + /// without CommandBehavior.KeyInfo and the reader reports no auto-increment flag. + /// The flag is not carried through : its setter + /// coerces a DataType it cannot increment to Int32, and SQL Server allows identity on + /// tinyint, numeric and decimal, so doing that would report the wrong SystemType and reach + /// parameter typing and the generated API schemas. It comes from the catalog instead. + /// No-op for every object whose projection was not narrowed, which is where the adapter + /// still reports the flag itself. + /// + private void ApplyIdentityColumnsFromCatalog( + string schemaName, + string tableName, + SourceDefinition sourceDefinition) + { + if (!_objectCatalogMetadataCache.TryGetValue( + GetObjectCacheKey(schemaName, tableName), + out ObjectCatalogMetadata? catalogMetadata)) + { + return; + } + + foreach (string identityColumn in catalogMetadata.IdentityColumns) + { + if (sourceDefinition.Columns.TryGetValue(identityColumn, out ColumnDefinition? columnDefinition)) + { + columnDefinition.IsAutoGenerated = true; + + // Matches how the adapter-reported flag is treated above: an auto-increment + // column is also read-only. + columnDefinition.IsReadOnly = true; + } + } + } + /// /// Fails initialization with the reason when the object's own primary key includes a column /// left out of the projection because its data type is not supported. Without this the diff --git a/src/Service.Tests/DatabaseSchema-MsSql.sql b/src/Service.Tests/DatabaseSchema-MsSql.sql index d94ef77c48..01ae49df12 100644 --- a/src/Service.Tests/DatabaseSchema-MsSql.sql +++ b/src/Service.Tests/DatabaseSchema-MsSql.sql @@ -49,6 +49,7 @@ DROP TABLE IF EXISTS geometry_type_table; DROP TABLE IF EXISTS hierarchyid_pk_table; DROP TABLE IF EXISTS hierarchyid_composite_pk_table; DROP TABLE IF EXISTS hierarchyid_unique_table; +DROP TABLE IF EXISTS unique_key_geometry_table; -- System versioning has to be released before the temporal table can be dropped. IF OBJECT_ID('dbo.temporal_geometry_type_table', 'U') IS NOT NULL AND OBJECTPROPERTY(OBJECT_ID('dbo.temporal_geometry_type_table'), 'TableTemporalType') = 2 @@ -306,6 +307,16 @@ CREATE TABLE hierarchyid_unique_table( CONSTRAINT UQ_hierarchyid_unique_table_node UNIQUE (node) ); +-- No database primary key, and the unique index covers a supported non-null column. The data +-- adapter promotes such a key to DataTable.PrimaryKey on the unnarrowed path, so the narrowed path +-- has to infer it too instead of demanding source.key-fields. +CREATE TABLE unique_key_geometry_table( + code varchar(20) NOT NULL, + name varchar(100) NOT NULL, + geom geometry NULL, + CONSTRAINT UQ_unique_key_geometry_table_code UNIQUE (code) +); + CREATE TABLE profiles( id int IDENTITY(5001, 1) PRIMARY KEY, metadata json NULL @@ -731,6 +742,11 @@ VALUES (1, hierarchyid::Parse('/1/'), 'root'); INSERT INTO hierarchyid_unique_table(id, node, name) VALUES (1, hierarchyid::Parse('/1/'), 'root'); +INSERT INTO unique_key_geometry_table(code, name, geom) +VALUES + ('CAR-001', 'point', geometry::STGeomFromText('POINT(1 2)', 0)), + ('CAR-002', 'null geometry', NULL); + SET IDENTITY_INSERT profiles ON INSERT INTO profiles(id, metadata) VALUES diff --git a/src/Service.Tests/UnitTests/SqlMetadataProviderUnitTests.cs b/src/Service.Tests/UnitTests/SqlMetadataProviderUnitTests.cs index 68f56a80c2..9313551fb8 100644 --- a/src/Service.Tests/UnitTests/SqlMetadataProviderUnitTests.cs +++ b/src/Service.Tests/UnitTests/SqlMetadataProviderUnitTests.cs @@ -750,6 +750,49 @@ await SetUpSingleEntityMetadataProviderAsync( TestHelper.UnsetAllDABEnvironmentVariables(); } + /// + /// Test to validate that an object with no database primary key, whose unique index covers a + /// supported non-null column, has that key inferred without `source.key-fields`. + /// `DbDataAdapter.FillSchema` promotes such a unique key to `DataTable.PrimaryKey` on the + /// unnarrowed path, so dropping it once the projection is narrowed would make the two paths + /// disagree and demand configuration for an object the other path resolves on its own. + /// + [TestMethod, TestCategory(TestCategory.MSSQL)] + public async Task ValidateUniqueKeyIsInferredWhenNoDatabasePrimaryKeyExists() + { + DatabaseEngine = TestCategory.MSSQL; + TestHelper.SetupDatabaseEnvironment(DatabaseEngine); + + await SetUpSingleEntityMetadataProviderAsync( + "UniqueKeyGeometry", + BuildReadOnlyEntity( + entityName: "UniqueKeyGeometry", + databaseObject: "dbo.unique_key_geometry_table", + sourceType: EntitySourceType.Table, + keyFields: null)); + + await _sqlMetadataProvider.InitializeAsync(); + + Assert.IsTrue( + _sqlMetadataProvider.GetEntityNamesAndDbObjects().TryGetValue("UniqueKeyGeometry", out DatabaseObject databaseObject), + message: "Metadata inference failed for an object whose only key is a unique index over a supported column."); + + SourceDefinition sourceDefinition = databaseObject.SourceDefinition; + + CollectionAssert.AreEqual( + new List { "code" }, + sourceDefinition.PrimaryKey, + message: "The non-null unique column is expected to be inferred as the primary key."); + Assert.IsTrue( + sourceDefinition.Columns.ContainsKey("name"), + message: "A column with a supported data type is expected in the source definition."); + Assert.IsFalse( + sourceDefinition.Columns.ContainsKey("geom"), + message: "A column whose data type cannot be mapped is not expected in the source definition."); + + TestHelper.UnsetAllDABEnvironmentVariables(); + } + /// /// Builds a metadata provider over a single in-memory entity and resets the database state. /// The objects exercised by the unsupported-data-type tests are declared in memory rather From f4eacf61d31ba1e76a38b9b0e8dec53096ccde16 Mon Sep 17 00:00:00 2001 From: Joymax Andrade do Nascimento Date: Mon, 14 Sep 2026 22:57:50 -0300 Subject: [PATCH 12/12] Reject database policies over skipped columns and cover identity metadata A database policy is parsed per request against the OData model, which EdmModelBuilder builds from SourceDefinition.Columns, so one naming a column left out of the projection returns 400 on every request for that role. Configured references in mappings, fields, fields.include and policy.database are now all rejected at initialization. Policy field references are scanned rather than parsed, because the model the parser needs does not exist yet at that point. Adds identity assertions to ValidateUnsupportedColumnTypeIsNotInferred, plus decimal_identity_geometry_table and ValidateIdentityTypeIsPreservedOnTheNarrowedPath, which is what verifies that carrying identity from the catalog preserves a DataType that DataColumn.AutoIncrement would coerce to Int32. Addresses the second Copilot review on #3802. --- .../MetadataProviders/SqlMetadataProvider.cs | 50 ++++++++ src/Service.Tests/DatabaseSchema-MsSql.sql | 14 +++ .../UnitTests/SqlMetadataProviderUnitTests.cs | 107 ++++++++++++++++++ 3 files changed, 171 insertions(+) diff --git a/src/Core/Services/MetadataProviders/SqlMetadataProvider.cs b/src/Core/Services/MetadataProviders/SqlMetadataProvider.cs index 872df289eb..85891fc7bf 100644 --- a/src/Core/Services/MetadataProviders/SqlMetadataProvider.cs +++ b/src/Core/Services/MetadataProviders/SqlMetadataProvider.cs @@ -2400,6 +2400,15 @@ private void RejectConfiguredReferencesToSkippedColumns( { foreach (EntityAction action in permission.Actions) { + // A database policy is parsed per request against the OData model, which is + // built from SourceDefinition.Columns. A policy naming a column that is not + // there fails every request for that role instead of the configuration being + // rejected once, at startup. + foreach (string policyField in EnumeratePolicyFieldReferences(action.Policy?.Database)) + { + RejectReference(policyField, $"database policy of role {permission.Role}"); + } + if (action.Fields?.Include is null) { continue; @@ -2431,6 +2440,47 @@ void RejectReference(string configuredName, string configurationSection) } } + /// + /// Yields the field names a database policy references through its "@item." prefix. + /// Scanned by hand rather than parsed: the OData model the parser needs does not exist yet + /// at this point in initialization, and a reference this scan fails to recognize only means + /// one fewer configuration rejected at startup — never a wrong rejection. + /// A policy can only reach a skipped column by its backing name, because an alias over one + /// is rejected through mappings and fields before this runs. + /// + private static IEnumerable EnumeratePolicyFieldReferences(string? databasePolicy) + { + const string POLICY_FIELD_PREFIX = "@item."; + + if (string.IsNullOrWhiteSpace(databasePolicy)) + { + yield break; + } + + int prefixIndex = databasePolicy.IndexOf(POLICY_FIELD_PREFIX, StringComparison.OrdinalIgnoreCase); + + while (prefixIndex >= 0) + { + int fieldStart = prefixIndex + POLICY_FIELD_PREFIX.Length; + int fieldEnd = fieldStart; + + while (fieldEnd < databasePolicy.Length + && (char.IsLetterOrDigit(databasePolicy[fieldEnd]) || databasePolicy[fieldEnd] == '_')) + { + fieldEnd++; + } + + if (fieldEnd > fieldStart) + { + yield return databasePolicy[fieldStart..fieldEnd]; + } + + prefixIndex = fieldEnd >= databasePolicy.Length + ? -1 + : databasePolicy.IndexOf(POLICY_FIELD_PREFIX, fieldEnd, StringComparison.OrdinalIgnoreCase); + } + } + /// /// Renders skipped columns as "name (type)" pairs for log and error messages. /// diff --git a/src/Service.Tests/DatabaseSchema-MsSql.sql b/src/Service.Tests/DatabaseSchema-MsSql.sql index 01ae49df12..1d2a8be83f 100644 --- a/src/Service.Tests/DatabaseSchema-MsSql.sql +++ b/src/Service.Tests/DatabaseSchema-MsSql.sql @@ -50,6 +50,7 @@ DROP TABLE IF EXISTS hierarchyid_pk_table; DROP TABLE IF EXISTS hierarchyid_composite_pk_table; DROP TABLE IF EXISTS hierarchyid_unique_table; DROP TABLE IF EXISTS unique_key_geometry_table; +DROP TABLE IF EXISTS decimal_identity_geometry_table; -- System versioning has to be released before the temporal table can be dropped. IF OBJECT_ID('dbo.temporal_geometry_type_table', 'U') IS NOT NULL AND OBJECTPROPERTY(OBJECT_ID('dbo.temporal_geometry_type_table'), 'TableTemporalType') = 2 @@ -317,6 +318,14 @@ CREATE TABLE unique_key_geometry_table( CONSTRAINT UQ_unique_key_geometry_table_code UNIQUE (code) ); +-- A decimal identity column. DataColumn.AutoIncrement would coerce its CLR type to Int32, which is +-- why identity is carried from the catalog instead; this fixture is what proves the type survives. +CREATE TABLE decimal_identity_geometry_table( + id decimal(18, 0) IDENTITY(1, 1) NOT NULL PRIMARY KEY, + name varchar(100) NOT NULL, + geom geometry NULL +); + CREATE TABLE profiles( id int IDENTITY(5001, 1) PRIMARY KEY, metadata json NULL @@ -747,6 +756,11 @@ VALUES ('CAR-001', 'point', geometry::STGeomFromText('POINT(1 2)', 0)), ('CAR-002', 'null geometry', NULL); +INSERT INTO decimal_identity_geometry_table(name, geom) +VALUES + ('point', geometry::STGeomFromText('POINT(1 2)', 0)), + ('null geometry', NULL); + SET IDENTITY_INSERT profiles ON INSERT INTO profiles(id, metadata) VALUES diff --git a/src/Service.Tests/UnitTests/SqlMetadataProviderUnitTests.cs b/src/Service.Tests/UnitTests/SqlMetadataProviderUnitTests.cs index 9313551fb8..cc79832e0f 100644 --- a/src/Service.Tests/UnitTests/SqlMetadataProviderUnitTests.cs +++ b/src/Service.Tests/UnitTests/SqlMetadataProviderUnitTests.cs @@ -426,6 +426,113 @@ public async Task ValidateUnsupportedColumnTypeIsNotInferred() sourceDefinition.Columns.ContainsKey("geom"), message: "A column whose data type cannot be mapped is not expected in the source definition."); + // The narrowed path reads the shape without CommandBehavior.KeyInfo, so the reader + // reports no auto-increment flag and it is carried from the catalog instead. Asserted + // here because losing it would silently make creates require an identity value. + Assert.IsTrue( + sourceDefinition.Columns["id"].IsAutoGenerated, + message: "The identity column is expected to be marked auto-generated."); + Assert.IsTrue( + sourceDefinition.Columns["id"].IsReadOnly, + message: "An auto-generated column is expected to be read-only."); + + TestHelper.UnsetAllDABEnvironmentVariables(); + } + + /// + /// Test to validate that an identity column whose CLR type cannot be auto-incremented keeps + /// the type the provider reported. + /// `DataColumn.AutoIncrement` coerces such a DataType to Int32, and SQL Server allows + /// identity on tinyint, numeric and decimal, so carrying the flag through that property + /// would report the wrong SystemType and reach parameter typing and the generated API + /// schemas. This is the fixture that proves the type survives. + /// + [TestMethod, TestCategory(TestCategory.MSSQL)] + public async Task ValidateIdentityTypeIsPreservedOnTheNarrowedPath() + { + DatabaseEngine = TestCategory.MSSQL; + TestHelper.SetupDatabaseEnvironment(DatabaseEngine); + + await SetUpSingleEntityMetadataProviderAsync( + "DecimalIdentityGeometry", + BuildReadOnlyEntity( + entityName: "DecimalIdentityGeometry", + databaseObject: "dbo.decimal_identity_geometry_table", + sourceType: EntitySourceType.Table, + keyFields: null)); + + await _sqlMetadataProvider.InitializeAsync(); + + Assert.IsTrue( + _sqlMetadataProvider.GetEntityNamesAndDbObjects().TryGetValue("DecimalIdentityGeometry", out DatabaseObject databaseObject), + message: "Metadata inference failed for an object with a decimal identity column."); + + SourceDefinition sourceDefinition = databaseObject.SourceDefinition; + + Assert.AreEqual( + typeof(decimal), + sourceDefinition.Columns["id"].SystemType, + message: "The identity column is expected to keep the data type the provider reported."); + Assert.IsTrue( + sourceDefinition.Columns["id"].IsAutoGenerated, + message: "The identity column is expected to be marked auto-generated."); + Assert.IsTrue( + sourceDefinition.Columns["id"].IsReadOnly, + message: "An auto-generated column is expected to be read-only."); + Assert.IsFalse( + sourceDefinition.Columns.ContainsKey("geom"), + message: "A column whose data type cannot be mapped is not expected in the source definition."); + + TestHelper.UnsetAllDABEnvironmentVariables(); + } + + /// + /// Test to validate that a database policy referencing a column left out of the projection + /// fails initialization. + /// A policy is parsed per request against the OData model, which is built from + /// `SourceDefinition.Columns`, so a policy naming an absent column returns 400 on every + /// request for that role instead of the configuration being rejected once, at startup. + /// + [TestMethod, TestCategory(TestCategory.MSSQL)] + public async Task ValidateDatabasePolicyOverUnsupportedColumnFailsInitialization() + { + DatabaseEngine = TestCategory.MSSQL; + TestHelper.SetupDatabaseEnvironment(DatabaseEngine); + + Entity entity = new( + Source: new("dbo.geometry_type_table", EntitySourceType.Table, null, new string[] { "id" }), + Fields: null, + Rest: new(Enabled: true), + GraphQL: new("GeometryPolicy", "GeometryPolicys", Enabled: true), + Permissions: new EntityPermission[] + { + new(Role: "anonymous", + Actions: new EntityAction[] + { + new(Action: EntityActionOperation.Read, + Fields: null, + Policy: new(Request: null, Database: "@item.geom eq null")) + }) + }, + Relationships: null, + Mappings: null); + + await SetUpSingleEntityMetadataProviderAsync("GeometryPolicy", entity); + + try + { + await _sqlMetadataProvider.InitializeAsync(); + Assert.Fail("Expected DataApiBuilderException was not thrown for a database policy over a column of an unsupported data type."); + } + catch (DataApiBuilderException ex) + { + Assert.AreEqual(HttpStatusCode.ServiceUnavailable, ex.StatusCode); + Assert.AreEqual(DataApiBuilderException.SubStatusCodes.ErrorInInitialization, ex.SubStatusCode); + Assert.IsTrue( + ex.Message.Contains("geom") && ex.Message.Contains("policy"), + message: $"The error is expected to name the column and the policy referencing it. Actual message: {ex.Message}"); + } + TestHelper.UnsetAllDABEnvironmentVariables(); }