search

aspnet / MusicStore

[Archived] MusicStore test application that uses ASP.NET/EF Core. Project moved to https://github.com/aspnet/AspNetCore
1.3k stars 878 forks source link

React to ConfigureCors api removal #534

Closed kichalla closed 9 years ago

kichalla commented 9 years ago

@HaoK

HaoK commented 9 years ago

MVC adds cors I take it? Maybe its just best to call the AddCors method instead?

kichalla commented 9 years ago

@HaoK right, MVC adds Cors too...regarding calling AddCors instead I am not sure...actually this triggered some thought...this could apply to any service in general... Let's assume I call AddCors(with some option settings) and then AddMVC or any other service which also does AddCors but different options settings. Now since AddCors always does a TryAdd, the services are not overriden but in case of options the final options instance can overwrite the earlier ones...so probably that not desired?

HaoK commented 9 years ago

So its a bit muddled now, but Add now does two things, it will ALWAYS configure the options, and TryAdd the services. So all of the options tweaking will always run.

For this situation though I think its fine to just call AddCors explicitly since it reads better?

HaoK commented 9 years ago

In regards to the sideffects, really nothing has changed, Configure has the same behavior as calling Add does, both of them take effect in the order you call them.

kichalla commented 9 years ago

@HaoK thanks, updated it now

kichalla commented 9 years ago

bfe1de4819377f25ae0452280b8b06e9f7bfaacf