dotnet / aspnetcore

ASP.NET Core is a cross-platform .NET framework for building modern cloud-based web applications on Windows, Mac, or Linux.
https://asp.net
MIT License
35.5k stars 10.04k forks source link

KeyPerFileConfigurationProvider Doesn't Propagate Reload Events #32836

Closed CodyWanless closed 3 years ago

CodyWanless commented 3 years ago

Describe the bug

KeyPerFileConfigurationProvider isn't propagating reload events through the reload token. When used in conjunction with IConfiguration, any handlers setup to respond to changes on config.GetReloadToken() are never triggered.

I believe OnReload just needs to be invoked after Data is set here, here, and here.

To Reproduce

var directory = @"c:\<your test directory>";

var configurationBuilder = new ConfigurationBuilder();
configurationBuilder.AddKeyPerFile(directory, optional: false, reloadOnChange: true);
var config = configurationBuilder.Build();

IChangeToken token = config.GetReloadToken();
var tcs = new TaskCompletionSource<object>();

token.RegisterChangeCallback(state =>
    ((TaskCompletionSource<object>)state).TrySetResult(null), tcs);

await tcs.Task.ConfigureAwait(false);

Console.WriteLine("file changed");

Using file explorer or the vehicle of your choosing - creating, removing, or updating any files setup under directory never trigger the task to complete, leaving this hanging indefinitely.

Exceptions (if any)

Further technical details

ghost commented 3 years ago

Tagging subscribers to this area: @maryamariyan, @safern See info in area-owners.md if you want to be subscribed.

Issue Details
### Describe the bug [KeyPerFileConfigurationProvider](https://github.com/dotnet/aspnetcore/blob/main/src/Configuration.KeyPerFile/src/KeyPerFileConfigurationProvider.cs) isn't propagating reload events through the reload token. When used in conjunction with IConfiguration, any handlers setup to respond to changes on `config.GetReloadToken()` are never triggered. I believe `OnReload` just needs to be invoked after `Data` is set [here](https://github.com/dotnet/aspnetcore/blob/main/src/Configuration.KeyPerFile/src/KeyPerFileConfigurationProvider.cs#L100), [here](https://github.com/dotnet/aspnetcore/blob/main/src/Configuration.KeyPerFile/src/KeyPerFileConfigurationProvider.cs#L75), and [here](https://github.com/dotnet/aspnetcore/blob/main/src/Configuration.KeyPerFile/src/KeyPerFileConfigurationProvider.cs#L63). ### To Reproduce ```cs var directory = @"c:\"; var configurationBuilder = new ConfigurationBuilder(); configurationBuilder.AddKeyPerFile(directory, optional: false, reloadOnChange: true); var config = configurationBuilder.Build(); IChangeToken token = config.GetReloadToken(); var tcs = new TaskCompletionSource(); token.RegisterChangeCallback(state => ((TaskCompletionSource)state).TrySetResult(null), tcs); await tcs.Task.ConfigureAwait(false); Console.WriteLine("file changed"); ``` Using file explorer or the vehicle of your choosing - creating, removing, or updating any files setup under directory never trigger the task to complete, leaving this hanging indefinitely. ### Exceptions (if any) ### Further technical details - ASP.NET Core version - .NET 5.0.5 - Microsoft.Extensions.Configuration 5.0.0 - Microsoft.Extensions.Configuration.PerKeyFile 5.0.5 - Microsoft.Extensions.Configuration.FileProviders.Physical 5.0.0 - Include the output of `dotnet --info` - The IDE (VS / VS Code/ VS4Mac) you're running on, and its version - VS 16.9.4 Enterprise
Author: CodyWanless
Assignees: -
Labels: `area-Extensions-Configuration`, `untriaged`
Milestone: -
shirhatti commented 3 years ago

KeyPerFileConfigurationProvider is in the aspnetcore repo

shirhatti commented 3 years ago

@CodyWanless Are you interested in submitting a PR?

CodyWanless commented 3 years ago

@shirhatti sure! I'd be happy to take this. I'll have that opened in the aspnetcore repo today sometime.

ghost commented 3 years ago

Thanks for contacting us.

We're moving this issue to the Next sprint planning milestone for future evaluation / consideration. We would like to keep this around to collect more feedback, which can help us with prioritizing this work. We will re-evaluate this issue, during our next planning meeting(s). If we later determine, that the issue has no community involvement, or it's very rare and low-impact issue, we will close it - so that the team can focus on more important and high impact issues. To learn more about what to expect next and how this issue will be handled you can read more about our triage process here.

tkolo commented 3 years ago

Hi, is the mentioned PR going to be back ported to 5.0 release?