Skip to content

Commit 7804517

Browse files
authored
Handle Exception during dab validate when SqlMetadataProvider has not correctly initialized (#2202)
## Why make this change? Closes #2184 Closes #2180 ## What is this change? When we don't properly initialize the `SqlMetadataProvider`, certain functions will not behave as intended. This change adds some defensive programming around this fact so that we have more readable exceptions in these cases, as well as restructuring the validation to avoid the potential of such a circumstance. We refactor the `ValidateEntitiesMetadata` function into 2 functions, one called `ValidateRelationshipConfigCorrectness` that validates the relationships in the config without needing to cross reference database metadata, and leaving the remaining validations that do require such metadata within the original function. This validation is then handled by a mix of the initialization of the metadata provider (this init process does some validation), and the call to `ValidateRelationships` which happens from within `ValidateEntitiesMetadata` only when the initialization of the metadata provider resulted in no connection errors. Therefore, the normal code path when doing `dab validate` will be 1. Call `ValidateRelationshipConfigCorrectness` 2. Call `ValidateEntitiesMetadata` 2a. If there were no connection errors Call `ValidateRelationships` ![image](https://github.com/Azure/data-api-builder/assets/93220300/1e2feec5-ac73-4e57-b3eb-01f032f362cf) ## How was this tested? Manually, against our test suite, and we add a regression test that looks for the correct error messaging when the config that caused the error from the finding of the bug is used. ## Sample Request(s) This can be reprod by using the command `dab validate -c <path to config>
1 parent 910ba2a commit 7804517

File tree

8 files changed

+344
-171
lines changed

8 files changed

+344
-171
lines changed

src/Cli.Tests/TestHelper.cs

+98
Original file line numberDiff line numberDiff line change
@@ -1065,6 +1065,104 @@ public static Process ExecuteDabCommand(string command, string flags)
10651065
}
10661066
}";
10671067

1068+
public const string COMPLETE_CONFIG_WITH_RELATIONSHIPS_NON_WORKING_CONN_STRING = @"
1069+
{
1070+
""$schema"": ""https://github.com/Azure/data-api-builder/releases/download/vmajor.minor.patch/dab.draft.schema.json"",
1071+
""data-source"": {
1072+
""database-type"": ""mssql"",
1073+
""options"": {
1074+
""set-session-context"": false
1075+
},
1076+
""connection-string"": ""Server=XXXXX;Persist Security Info=False;User ID=<USERHERE>;Password=<PWD HERE> ;MultipleActiveResultSets=False;Encrypt=True;TrustServerCertificate=False;Connection Timeout=5;""
1077+
},
1078+
""runtime"": {
1079+
""rest"": {
1080+
""enabled"": true,
1081+
""path"": ""/api""
1082+
},
1083+
""graphql"": {
1084+
""allow-introspection"": true,
1085+
""enabled"": true,
1086+
""path"": ""/graphql""
1087+
},
1088+
""host"": {
1089+
""mode"": ""development"",
1090+
""cors"": {
1091+
""origins"": [],
1092+
""allow-credentials"": false
1093+
},
1094+
""authentication"": {
1095+
""provider"": ""StaticWebApps""
1096+
}
1097+
}
1098+
},
1099+
""entities"": {
1100+
""Publisher"": {
1101+
""source"": {
1102+
""object"": ""publishers"",
1103+
""type"": ""table"",
1104+
""key-fields"": [ ""id"" ]
1105+
},
1106+
""graphql"": {
1107+
""enabled"": true,
1108+
""type"": {
1109+
""singular"": ""Publisher"",
1110+
""plural"": ""Publishers""
1111+
}
1112+
},
1113+
""rest"": {
1114+
""enabled"": true
1115+
},
1116+
""permissions"": [
1117+
],
1118+
""relationships"": {
1119+
""books"": {
1120+
""cardinality"": ""many"",
1121+
""target.entity"": ""Book"",
1122+
""source.fields"": [ ""id"" ],
1123+
""target.fields"": [ ""publisher_id"" ],
1124+
""linking.source.fields"": [],
1125+
""linking.target.fields"": []
1126+
}
1127+
}
1128+
},
1129+
""Book"": {
1130+
""source"": {
1131+
""object"": ""books"",
1132+
""type"": ""table"",
1133+
""key-fields"": [ ""id"" ]
1134+
},
1135+
""graphql"": {
1136+
""enabled"": true,
1137+
""type"": {
1138+
""singular"": ""book"",
1139+
""plural"": ""books""
1140+
}
1141+
},
1142+
""rest"": {
1143+
""enabled"": true
1144+
},
1145+
""permissions"": [
1146+
],
1147+
""mappings"": {
1148+
""id"": ""id"",
1149+
""title"": ""title""
1150+
},
1151+
""relationships"": {
1152+
""publishers"": {
1153+
""cardinality"": ""one"",
1154+
""target.entity"": ""Publisher"",
1155+
""source.fields"": [ ""publisher_id"" ],
1156+
""target.fields"": [ ""id"" ],
1157+
""linking.source.fields"": [],
1158+
""linking.target.fields"": []
1159+
}
1160+
}
1161+
}
1162+
}
1163+
}
1164+
";
1165+
10681166
/// <summary>
10691167
/// Creates basic initialization options for MS SQL config.
10701168
/// </summary>

src/Cli.Tests/ValidateConfigTests.cs

+25
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,31 @@ public void TestConfigWithCustomPropertyAsInvalid()
4646
Assert.IsFalse(isConfigValid);
4747
}
4848

49+
/// <summary>
50+
/// This method verifies that the relationship validation does not cause unhandled
51+
/// exceptions, and that the errors generated include the expected messaging.
52+
/// This case is a regression test due to the metadata needed not always being
53+
/// populated in the SqlMetadataProvider if for example a bad connection string
54+
/// is given.
55+
/// </summary>
56+
[TestMethod]
57+
public void TestErrorHandlingForRelationshipValidationWithNonWorkingConnectionString()
58+
{
59+
// Arrange
60+
((MockFileSystem)_fileSystem!).AddFile(TEST_RUNTIME_CONFIG_FILE, COMPLETE_CONFIG_WITH_RELATIONSHIPS_NON_WORKING_CONN_STRING);
61+
ValidateOptions validateOptions = new(TEST_RUNTIME_CONFIG_FILE);
62+
StringWriter writer = new();
63+
// Capture console output to get error messaging.
64+
Console.SetOut(writer);
65+
66+
// Act
67+
ConfigGenerator.IsConfigValid(validateOptions, _runtimeConfigLoader!, _fileSystem!);
68+
string errorMessage = writer.ToString();
69+
70+
// Assert
71+
Assert.IsTrue(errorMessage.Contains(DataApiBuilderException.CONNECTION_STRING_ERROR_MESSAGE));
72+
}
73+
4974
/// <summary>
5075
/// Validates that the IsConfigValid method returns false when a config is passed with
5176
/// both rest and graphQL disabled globally.

src/Core/Configurations/RuntimeConfigValidator.cs

+185-136
Large diffs are not rendered by default.

src/Core/Services/MetadataProviders/ISqlMetadataProvider.cs

+8
Original file line numberDiff line numberDiff line change
@@ -92,23 +92,31 @@ bool VerifyForeignKeyExistsInDB(
9292
/// try to get the exposed name associated
9393
/// with the provided field, if it exists, save in out
9494
/// parameter, and return true, otherwise return false.
95+
/// If an entity name is provided that does not exist
96+
/// as metadata in this metadata provider, a KeyNotFoundException
97+
/// is thrown.
9598
/// </summary>
9699
/// <param name="entityName">The entity whose mapping we lookup.</param>
97100
/// <param name="backingFieldName">The field used for the lookup in the mapping.</param>
98101
/// <param name="name">Out parameter in which we will save exposed name.</param>
99102
/// <returns>True if exists, false otherwise.</returns>
103+
/// <throws>KeyNotFoundException if entity name not found.</throws>
100104
bool TryGetExposedColumnName(string entityName, string backingFieldName, [NotNullWhen(true)] out string? name);
101105

102106
/// <summary>
103107
/// For the entity that is provided as an argument,
104108
/// try to get the underlying backing column name associated
105109
/// with the provided field, if it exists, save in out
106110
/// parameter, and return true, otherwise return false.
111+
/// If an entity name is provided that does not exist
112+
/// as metadata in this metadata provider, a KeyNotFoundException
113+
/// is thrown.
107114
/// </summary>
108115
/// <param name="entityName">The entity whose mapping we lookup.</param>
109116
/// <param name="field">The field used for the lookup in the mapping.</param>
110117
/// <param name="name"/>Out parameter in which we will save backing column name.<param>
111118
/// <returns>True if exists, false otherwise.</returns>
119+
/// <throws>KeyNotFoundException if entity name not found.</throws>
112120
bool TryGetBackingColumn(string entityName, string field, [NotNullWhen(true)] out string? name);
113121

114122
/// <summary>

src/Core/Services/MetadataProviders/SqlMetadataProvider.cs

+14-2
Original file line numberDiff line numberDiff line change
@@ -209,13 +209,25 @@ public StoredProcedureDefinition GetStoredProcedureDefinition(string entityName)
209209
/// <inheritdoc />
210210
public bool TryGetExposedColumnName(string entityName, string backingFieldName, [NotNullWhen(true)] out string? name)
211211
{
212-
return EntityBackingColumnsToExposedNames[entityName].TryGetValue(backingFieldName, out name);
212+
Dictionary<string, string>? backingColumnsToExposedNamesMap;
213+
if (!EntityBackingColumnsToExposedNames.TryGetValue(entityName, out backingColumnsToExposedNamesMap))
214+
{
215+
throw new KeyNotFoundException($"Initialization of metadata incomplete for entity: {entityName}");
216+
}
217+
218+
return backingColumnsToExposedNamesMap.TryGetValue(backingFieldName, out name);
213219
}
214220

215221
/// <inheritdoc />
216222
public bool TryGetBackingColumn(string entityName, string field, [NotNullWhen(true)] out string? name)
217223
{
218-
return EntityExposedNamesToBackingColumnNames[entityName].TryGetValue(field, out name);
224+
Dictionary<string, string>? exposedNamesToBackingColumnsMap;
225+
if (!EntityExposedNamesToBackingColumnNames.TryGetValue(entityName, out exposedNamesToBackingColumnsMap))
226+
{
227+
throw new KeyNotFoundException($"Initialization of metadata incomplete for entity: {entityName}");
228+
}
229+
230+
return exposedNamesToBackingColumnsMap.TryGetValue(field, out name);
219231
}
220232

221233
/// <inheritdoc />

src/Service.Tests/Configuration/ConfigurationTests.cs

+3
Original file line numberDiff line numberDiff line change
@@ -1271,6 +1271,7 @@ public async Task TestSqlMetadataForValidConfigEntities()
12711271
configValidatorLogger.Object,
12721272
isValidateOnly: true);
12731273

1274+
configValidator.ValidateRelationshipConfigCorrectness(configProvider.GetConfig());
12741275
await configValidator.ValidateEntitiesMetadata(configProvider.GetConfig(), mockLoggerFactory);
12751276
Assert.IsTrue(configValidator.ConfigValidationExceptions.IsNullOrEmpty());
12761277
}
@@ -1337,6 +1338,7 @@ public async Task TestSqlMetadataForInvalidConfigEntities()
13371338

13381339
ILoggerFactory mockLoggerFactory = TestHelper.ProvisionLoggerFactory();
13391340

1341+
configValidator.ValidateRelationshipConfigCorrectness(configProvider.GetConfig());
13401342
await configValidator.ValidateEntitiesMetadata(configProvider.GetConfig(), mockLoggerFactory);
13411343

13421344
Assert.IsTrue(configValidator.ConfigValidationExceptions.Any());
@@ -1418,6 +1420,7 @@ public async Task TestSqlMetadataValidationForEntitiesWithInvalidSource()
14181420

14191421
try
14201422
{
1423+
configValidator.ValidateRelationshipConfigCorrectness(configProvider.GetConfig());
14211424
await configValidator.ValidateEntitiesMetadata(configProvider.GetConfig(), mockLoggerFactory);
14221425
}
14231426
catch

src/Service.Tests/Unittests/ConfigValidationUnitTests.cs

+9-32
Original file line numberDiff line numberDiff line change
@@ -262,13 +262,10 @@ public void TestAddingRelationshipWithInvalidTargetEntity()
262262
);
263263

264264
RuntimeConfigValidator configValidator = InitializeRuntimeConfigValidator();
265-
Mock<ISqlMetadataProvider> _sqlMetadataProvider = new();
266-
Mock<IMetadataProviderFactory> _metadataProviderFactory = new();
267-
_metadataProviderFactory.Setup(x => x.GetMetadataProvider(It.IsAny<string>())).Returns(_sqlMetadataProvider.Object);
268265

269266
// Assert that expected exception is thrown. Entity used in relationship is Invalid
270267
DataApiBuilderException ex = Assert.ThrowsException<DataApiBuilderException>(() =>
271-
configValidator.ValidateRelationshipsInConfig(runtimeConfig, _metadataProviderFactory.Object));
268+
configValidator.ValidateRelationshipConfigCorrectness(runtimeConfig));
272269
Assert.AreEqual($"Entity: {sampleRelationship.TargetEntity} used for relationship is not defined in the config.", ex.Message);
273270
Assert.AreEqual(HttpStatusCode.ServiceUnavailable, ex.StatusCode);
274271
}
@@ -325,13 +322,10 @@ public void TestAddingRelationshipWithDisabledGraphQL()
325322
);
326323

327324
RuntimeConfigValidator configValidator = InitializeRuntimeConfigValidator();
328-
Mock<ISqlMetadataProvider> _sqlMetadataProvider = new();
329-
Mock<IMetadataProviderFactory> _metadataProviderFactory = new();
330-
_metadataProviderFactory.Setup(x => x.GetMetadataProvider(It.IsAny<string>())).Returns(_sqlMetadataProvider.Object);
331325

332326
// Exception should be thrown as we cannot use an entity (with graphQL disabled) in a relationship.
333327
DataApiBuilderException ex = Assert.ThrowsException<DataApiBuilderException>(() =>
334-
configValidator.ValidateRelationshipsInConfig(runtimeConfig, _metadataProviderFactory.Object));
328+
configValidator.ValidateRelationshipConfigCorrectness(runtimeConfig));
335329
Assert.AreEqual($"Entity: {sampleRelationship.TargetEntity} is disabled for GraphQL.", ex.Message);
336330
Assert.AreEqual(HttpStatusCode.ServiceUnavailable, ex.StatusCode);
337331
}
@@ -417,7 +411,7 @@ string relationshipEntity
417411

418412
// Exception thrown as foreignKeyPair not found in the DB.
419413
DataApiBuilderException ex = Assert.ThrowsException<DataApiBuilderException>(() =>
420-
configValidator.ValidateRelationshipsInConfig(runtimeConfig, _metadataProviderFactory.Object));
414+
configValidator.ValidateRelationships(runtimeConfig, _metadataProviderFactory.Object));
421415
Assert.AreEqual($"Could not find relationship between Linking Object: TEST_SOURCE_LINK"
422416
+ $" and entity: {relationshipEntity}.", ex.Message);
423417
Assert.AreEqual(HttpStatusCode.ServiceUnavailable, ex.StatusCode);
@@ -435,7 +429,7 @@ string relationshipEntity
435429

436430
// Since, we have defined the relationship in Database,
437431
// the engine was able to find foreign key relation and validation will pass.
438-
configValidator.ValidateRelationshipsInConfig(runtimeConfig, _metadataProviderFactory.Object);
432+
configValidator.ValidateRelationships(runtimeConfig, _metadataProviderFactory.Object);
439433
}
440434

441435
/// <summary>
@@ -498,7 +492,7 @@ public void TestRelationshipWithNoLinkingObjectAndEitherSourceOrTargetFieldIsNul
498492
// Exception is thrown as foreignKey pair is not specified in the config, nor defined
499493
// in the database.
500494
DataApiBuilderException ex = Assert.ThrowsException<DataApiBuilderException>(() =>
501-
configValidator.ValidateRelationshipsInConfig(runtimeConfig, _metadataProviderFactory.Object));
495+
configValidator.ValidateRelationships(runtimeConfig, _metadataProviderFactory.Object));
502496
Assert.AreEqual($"Could not find relationship between entities:"
503497
+ $" SampleEntity1 and SampleEntity2.", ex.Message);
504498
Assert.AreEqual(HttpStatusCode.ServiceUnavailable, ex.StatusCode);
@@ -518,7 +512,7 @@ public void TestRelationshipWithNoLinkingObjectAndEitherSourceOrTargetFieldIsNul
518512

519513
// No Exception is thrown as foreignKey Pair was found in the DB between
520514
// source and target entity.
521-
configValidator.ValidateRelationshipsInConfig(runtimeConfig, _metadataProviderFactory.Object);
515+
configValidator.ValidateRelationships(runtimeConfig, _metadataProviderFactory.Object);
522516
}
523517

524518
/// <summary>
@@ -566,7 +560,6 @@ public void TestRelationshipWithoutSourceAndTargetFieldsMatching(
566560
FileSystemRuntimeConfigLoader loader = new(fileSystem);
567561
RuntimeConfigProvider provider = new(loader) { IsLateConfigured = true };
568562
RuntimeConfigValidator configValidator = new(provider, fileSystem, new Mock<ILogger<RuntimeConfigValidator>>().Object);
569-
Mock<ISqlMetadataProvider> _sqlMetadataProvider = new();
570563

571564
Dictionary<string, DatabaseObject> mockDictionaryForEntityDatabaseObject = new()
572565
{
@@ -581,16 +574,10 @@ public void TestRelationshipWithoutSourceAndTargetFieldsMatching(
581574
}
582575
};
583576

584-
_sqlMetadataProvider.Setup<Dictionary<string, DatabaseObject>>(x =>
585-
x.EntityToDatabaseObject).Returns(mockDictionaryForEntityDatabaseObject);
586-
587-
Mock<IMetadataProviderFactory> _metadataProviderFactory = new();
588-
_metadataProviderFactory.Setup(x => x.GetMetadataProvider(It.IsAny<string>())).Returns(_sqlMetadataProvider.Object);
589-
590577
// Exception is thrown since sourceFields and targetFields do not match in either their existence,
591578
// or their length.
592579
DataApiBuilderException ex = Assert.ThrowsException<DataApiBuilderException>(() =>
593-
configValidator.ValidateRelationshipsInConfig(runtimeConfig, _metadataProviderFactory.Object));
580+
configValidator.ValidateRelationshipConfigCorrectness(runtimeConfig));
594581
Assert.AreEqual(expectedExceptionMessage, ex.Message);
595582
Assert.AreEqual(HttpStatusCode.ServiceUnavailable, ex.StatusCode);
596583
Assert.AreEqual(DataApiBuilderException.SubStatusCodes.ConfigValidationError, ex.SubStatusCode);
@@ -672,7 +659,7 @@ public void TestRelationshipWithoutSourceAndTargetFieldsAsValidBackingColumns(
672659

673660
// Exception is thrown since either source or target field does not exist as a valid backing column in their respective entity.
674661
DataApiBuilderException ex = Assert.ThrowsException<DataApiBuilderException>(() =>
675-
configValidator.ValidateRelationshipsInConfig(runtimeConfig, _metadataProviderFactory.Object));
662+
configValidator.ValidateRelationships(runtimeConfig, _metadataProviderFactory.Object));
676663
Assert.AreEqual(expectedExceptionMessage, ex.Message);
677664
Assert.AreEqual(HttpStatusCode.ServiceUnavailable, ex.StatusCode);
678665
Assert.AreEqual(DataApiBuilderException.SubStatusCodes.ConfigValidationError, ex.SubStatusCode);
@@ -776,7 +763,6 @@ public void TestRelationshipWithoutLinkingSourceAndTargetFieldsMatching(
776763
FileSystemRuntimeConfigLoader loader = new(fileSystem);
777764
RuntimeConfigProvider provider = new(loader) { IsLateConfigured = true };
778765
RuntimeConfigValidator configValidator = new(provider, fileSystem, new Mock<ILogger<RuntimeConfigValidator>>().Object);
779-
Mock<ISqlMetadataProvider> _sqlMetadataProvider = new();
780766

781767
Dictionary<string, DatabaseObject> mockDictionaryForEntityDatabaseObject = new()
782768
{
@@ -791,19 +777,10 @@ public void TestRelationshipWithoutLinkingSourceAndTargetFieldsMatching(
791777
}
792778
};
793779

794-
_sqlMetadataProvider.Setup<Dictionary<string, DatabaseObject>>(x =>
795-
x.EntityToDatabaseObject).Returns(mockDictionaryForEntityDatabaseObject);
796-
797-
string discard;
798-
_sqlMetadataProvider.Setup(x => x.TryGetExposedColumnName(It.IsAny<string>(), It.IsAny<string>(), out discard)).Returns(true);
799-
800-
Mock<IMetadataProviderFactory> _metadataProviderFactory = new();
801-
_metadataProviderFactory.Setup(x => x.GetMetadataProvider(It.IsAny<string>())).Returns(_sqlMetadataProvider.Object);
802-
803780
// Exception is thrown since linkingSourceFields and linkingTargetFields do not match in either their existence,
804781
// or their length.
805782
DataApiBuilderException ex = Assert.ThrowsException<DataApiBuilderException>(() =>
806-
configValidator.ValidateRelationshipsInConfig(runtimeConfig, _metadataProviderFactory.Object));
783+
configValidator.ValidateRelationshipConfigCorrectness(runtimeConfig));
807784
Assert.AreEqual(expectedExceptionMessage, ex.Message);
808785
Assert.AreEqual(HttpStatusCode.ServiceUnavailable, ex.StatusCode);
809786
Assert.AreEqual(DataApiBuilderException.SubStatusCodes.ConfigValidationError, ex.SubStatusCode);

src/Service/Startup.cs

+2-1
Original file line numberDiff line numberDiff line change
@@ -635,7 +635,8 @@ private async Task<bool> PerformOnConfigChangeAsync(IApplicationBuilder app)
635635
if (runtimeConfig.IsDevelopmentMode())
636636
{
637637
// Running only in developer mode to ensure fast and smooth startup in production.
638-
runtimeConfigValidator.ValidateRelationshipsInConfig(runtimeConfig, sqlMetadataProviderFactory!);
638+
runtimeConfigValidator.ValidateRelationshipConfigCorrectness(runtimeConfig);
639+
runtimeConfigValidator.ValidateRelationships(runtimeConfig, sqlMetadataProviderFactory!);
639640
}
640641

641642
// OpenAPI document creation is only attempted for REST supporting database types.

0 commit comments

Comments
 (0)