search

Azure / data-api-builder

Data API builder provides modern REST and GraphQL endpoints to your Azure Databases and on-prem stores.
https://aka.ms/dab/docs
MIT License
786 stars 142 forks source link

Handle Exception during dab validate when SqlMetadataProvider has not correctly initialized #2202

Closed aaronburtle closed 1 month ago

aaronburtle commented 1 month ago

Why make this change?

Closes https://github.com/Azure/data-api-builder/issues/2184 Closes https://github.com/Azure/data-api-builder/issues/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

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

seantleonard commented 1 month ago

To make reviewing my feedback easier, I suggest looking at the review in GitHub Codespaces.

seantleonard commented 1 month ago

/azp run

aaronburtle commented 1 month ago

/azp run

ayush3797 commented 1 month ago

Might not need this clear: https://github.com/Azure/data-api-builder/blob/e895f356f2105132b9ea2cc6e3e396555f09b632/src/Core/Configurations/RuntimeConfigValidator.cs#L1143 based on the resolution of previous comments

seantleonard commented 1 month ago

/azp run