Update autoentities when hot-reload succeeds#3297
Update autoentities when hot-reload succeeds#3297RubenCerna2079 wants to merge 5 commits intomainfrom
Conversation
RubenCerna2079
requested review from
Aniruddh25,
JerryNixon,
aaronburtle,
anushakolan and
souvikghosh04
as code owners
RubenCerna2079
requested review from
Alekhya-Polavarapu,
rusamant and
stuartpa
as code owners
|
/azp run |
|
Azure Pipelines successfully started running 6 pipeline(s). |
RubenCerna2079
changed the title
There was a problem hiding this comment.
Pull request overview
Fixes the hot-reload workflow for MSSQL autoentities by preventing validate-only initialization from leaving generated entities in the in-memory config (which would otherwise cause duplicate-entity failures on the subsequent “real” initialization).
Changes:
- Track whether an
Entityinstance was generated viaautoentities(IsAutoentity) and mark generated MSSQL entities accordingly. - Add a config-provider helper to remove generated autoentities (and associated datasource mappings) after validate-only metadata initialization.
- Add a hot-reload test covering autoentities, and tighten config validation to reject entities with missing/empty permissions.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Service.Tests/Configuration/HotReload/ConfigurationHotReloadTests.cs | Adds autoentities to the hot-reload config template and a new hot-reload test for autoentities. |
| src/Core/Services/MetadataProviders/SqlMetadataProvider.cs | Calls autoentity cleanup after validate-only initialization. |
| src/Core/Services/MetadataProviders/MsSqlMetadataProvider.cs | Marks generated entities as autoentities. |
| src/Core/Configurations/RuntimeConfigValidator.cs | Flags entities with no permissions as a config validation error. |
| src/Core/Configurations/RuntimeConfigProvider.cs | Adds removal of generated autoentities from the loaded runtime config. |
| src/Config/ObjectModel/RuntimeConfig.cs | Adds removal helper for entity->datasource name mapping. |
| src/Config/ObjectModel/Entity.cs | Adds IsAutoentity flag to the entity model. |
You can also share your feedback on Copilot code review. Take the survey.
src/Service.Tests/Configuration/HotReload/ConfigurationHotReloadTests.cs
Outdated
Show resolved
Hide resolved
src/Service.Tests/Configuration/HotReload/ConfigurationHotReloadTests.cs
Outdated
Show resolved
Hide resolved
| foreach ((string name, Entity entity) in entities) | ||
| { | ||
| if (entity.IsAutoentity) | ||
| { | ||
| entities.Remove(name); | ||
| _configLoader.RuntimeConfig!.RemoveGeneratedAutoentityNameFromDataSourceName(name); | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
It does not throw any error when we remove the autoentities from the collection
There was a problem hiding this comment.
I think Copilot has a valid point. we should not modify the same collection in the loop. the suggestion from Copilot is a fail-safe approach. collect the keys to remove first and remove them separately.
|
/azp run |
|
Azure Pipelines successfully started running 6 pipeline(s). |
RubenCerna2079
changed the title
RubenCerna2079
changed the title
There was a problem hiding this comment.
added couple of comments. approving since rest all looks good.
- avoid removing entries from same collection in the loop: #3297 (comment)
- consider DAB to start/available even if other entities have permissions
| { | ||
| HandleOrRecordException(new DataApiBuilderException( | ||
| message: $"Entity: {entityName} has no permissions defined.", | ||
| statusCode: HttpStatusCode.ServiceUnavailable, |
There was a problem hiding this comment.
should we let DAB to be entirely unavailble for a single entity having no permissions or let it pass for others and log in the missing permission entities as warnings/errors and have DAB available?
Why make this change?
Currently the inclusion of the autoentities feature causes hot-reload to fail when it should succeed.
What is this change?
Now the autoentities are updated when the hot-reload succeeds by ensuring that the generated entities from the autoentities property are removed if the initialization to connect to a database is done specifically for validation purposes. Since currently, hot-reload first validates before running the actual initialization, which would cause the autoentities to fail by trying to add the generated entities that already existed.
Entity.cs&MsSqlMetadataProvider.cs: Added new parameter that shows if an entity was generated through the autoentities property or just a regular entity.RuntimeConfigProvider.cs,RuntimeConfig.cs&SqlMetadataProvider.cs: Added function that removes the generated autoentities from the RuntimeConfig object and also removes the relation between the autoentity with the data source.ConfigurationHotReloadTests.cs: Added test to ensure autoentities works with hot-reload.RuntimeConfigValidator: Fixed small bug that allowed theautoentities.<definition>.permissionsproperty to be missing. This property should always be included for entitites and autoentities.How was this tested?