Fixes hot reload of CRUD API data file. Closes #1390 #1484
Conversation
There was a problem hiding this comment.
Pull request overview
This PR refactors the CRUD API plugin to enable hot reload of the data file by introducing a dedicated CrudApiDataLoader class that leverages the existing BaseLoader infrastructure with file watching capabilities. The changes separate concerns between API definition loading and data loading, allowing both to be independently monitored and reloaded.
Key changes:
- Introduced
CrudApiDataLoaderto handle data file loading with hot reload support - Renamed
_loaderto_definitionLoaderinCrudApiPluginfor clarity - Updated
BaseLoaderto skip schema validation for JSON arrays (data files)
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| DevProxy.Plugins/Mocking/CrudApiPlugin.cs | Refactored to use separate loaders for API definition and data, removing inline LoadData() method |
| DevProxy.Plugins/Mocking/CrudApiDataLoader.cs | New loader class that extends BaseLoader to provide hot reload functionality for CRUD API data files |
| DevProxy.Abstractions/Plugins/BaseLoader.cs | Updated schema validation to skip JSON arrays, enabling validation of data files |
| protected override void LoadData(string fileContents) | ||
| { | ||
| try | ||
| { | ||
| var data = JArray.Parse(fileContents); | ||
| _onDataLoaded(data); | ||
| Logger.LogInformation("Data for CRUD API loaded from {DataFile}", _configuration.DataFile); | ||
| } | ||
| catch (Exception ex) | ||
| { | ||
| Logger.LogError(ex, "An error has occurred while reading {DataFile}", _configuration.DataFile); | ||
| _onDataLoaded(null); | ||
| } | ||
| } |
There was a problem hiding this comment.
The error handling in LoadData doesn't replicate the behavior from the removed LoadData method in CrudApiPlugin. The old implementation would disable the API by setting Configuration.Actions to an empty array when the data file doesn't exist, preventing the plugin from processing requests. The new implementation only sets data to null via the callback, which means the API remains enabled but _data is null. This could lead to issues when API actions are invoked with null data. Consider either:
- Passing the Configuration to the data loader so it can disable actions on file not found, or
- Returning a success/failure status from InitFileWatcherAsync so the plugin can disable itself in InitializeAsync when data loading fails.
There was a problem hiding this comment.
@copilot open a new pull request to apply changes based on this feedback
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
garrytrinder
left a comment
garrytrinder
left a comment
There was a problem hiding this comment.
Works like a charm. I tested changing data and changing actions, all worked perfectly.
2 participants
Fixes hot reload of CRUD API data file. Closes #1390