-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
refactor(service-config): Restructure service registration logic #1127
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,7 @@ | ||
| using System; | ||
| using System.Collections.Generic; | ||
| using System.Diagnostics; | ||
| using System.IdentityModel.Tokens.Jwt; | ||
| using System.IO; | ||
| using System.Linq; | ||
| using System.Net; | ||
|
|
@@ -11,21 +12,20 @@ | |
| using Microsoft.AspNetCore.Authentication.Cookies; | ||
| using Microsoft.AspNetCore.Authentication.JwtBearer; | ||
| using Microsoft.AspNetCore.Authentication.OAuth; | ||
| using Microsoft.AspNetCore.Identity; | ||
| using Microsoft.AspNetCore.Http; | ||
| using Microsoft.AspNetCore.Identity; | ||
| using Microsoft.AspNetCore.Mvc.ApplicationParts; | ||
| using Microsoft.Extensions.Localization; | ||
| using Microsoft.EntityFrameworkCore; | ||
| using Microsoft.Extensions.Configuration; | ||
| using Microsoft.Extensions.DependencyInjection; | ||
| using Microsoft.Extensions.Localization; | ||
| using SimplCommerce.Infrastructure; | ||
| using SimplCommerce.Infrastructure.Modules; | ||
| using SimplCommerce.Infrastructure.Web.ModelBinders; | ||
| using SimplCommerce.Module.Core.Data; | ||
| using SimplCommerce.Module.Core.Extensions; | ||
| using SimplCommerce.Module.Core.Models; | ||
| using SimplCommerce.WebHost.IdentityServer; | ||
| using System.IdentityModel.Tokens.Jwt; | ||
|
|
||
| namespace SimplCommerce.WebHost.Extensions | ||
| { | ||
|
|
@@ -226,6 +226,29 @@ public static IServiceCollection AddCustomizedDataStore(this IServiceCollection | |
| return services; | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Discovers and configures all modules in the application by finding and initializing their module initializers. | ||
| /// </summary> | ||
| /// <param name="services">The <see cref="IServiceCollection"/> to add module services to.</param> | ||
| /// <returns>The same service collection so that multiple calls can be chained.</returns> | ||
| public static IServiceCollection ConfigureModules(this IServiceCollection services) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm agree with this extension, please revert the others, coz the order sometimes is matter |
||
| { | ||
| foreach (var module in GlobalConfiguration.Modules) | ||
| { | ||
| var moduleInitializerType = module.Assembly.GetTypes() | ||
| .FirstOrDefault(t => typeof(IModuleInitializer).IsAssignableFrom(t)); | ||
|
|
||
| if (moduleInitializerType != null && moduleInitializerType != typeof(IModuleInitializer)) | ||
| { | ||
| var moduleInitializer = (IModuleInitializer)Activator.CreateInstance(moduleInitializerType); | ||
| services.AddSingleton(typeof(IModuleInitializer), moduleInitializer); | ||
| moduleInitializer.ConfigureServices(services); | ||
| } | ||
| } | ||
|
|
||
| return services; | ||
| } | ||
|
|
||
| private static void TryLoadModuleAssembly(string moduleFolderPath, ModuleInfo module) | ||
| { | ||
| const string binariesFolderName = "bin"; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,4 @@ | ||
| using System; | ||
| using System.Linq; | ||
| using System.Text.Encodings.Web; | ||
| using System.Text.Unicode; | ||
| using Microsoft.AspNetCore.Builder; | ||
|
|
@@ -48,7 +47,6 @@ void ConfigureService() | |
| builder.Services.AddScoped<SlugRouteValueTransformer>(); | ||
|
|
||
| builder.Services.AddCustomizedLocalization(); | ||
|
|
||
| builder.Services.AddCustomizedMvc(GlobalConfiguration.Modules); | ||
| builder.Services.Configure<RazorViewEngineOptions>( | ||
| options => { options.ViewLocationExpanders.Add(new ThemeableViewLocationExpander()); }); | ||
|
|
@@ -60,18 +58,7 @@ void ConfigureService() | |
| builder.Services.AddTransient<IRazorViewRenderer, RazorViewRenderer>(); | ||
| builder.Services.AddAntiforgery(options => options.HeaderName = "X-XSRF-Token"); | ||
| builder.Services.AddCloudscribePagination(); | ||
|
|
||
| foreach (var module in GlobalConfiguration.Modules) | ||
| { | ||
| var moduleInitializerType = module.Assembly.GetTypes() | ||
| .FirstOrDefault(t => typeof(IModuleInitializer).IsAssignableFrom(t)); | ||
| if ((moduleInitializerType != null) && (moduleInitializerType != typeof(IModuleInitializer))) | ||
| { | ||
| var moduleInitializer = (IModuleInitializer)Activator.CreateInstance(moduleInitializerType); | ||
| builder.Services.AddSingleton(typeof(IModuleInitializer), moduleInitializer); | ||
| moduleInitializer.ConfigureServices(builder.Services); | ||
| } | ||
| } | ||
| builder.Services.ConfigureModules(); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Some of the moved registration is not a part of the modules configuration?!!
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @hishamco Could you please point out which specific registrations you feel don't belong in the modules configuration? I'd like to understand the intended organization better.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 48-51, 78 |
||
|
|
||
| builder.Services.AddMediatR(cfg => cfg.RegisterServicesFromAssembly(typeof(Program).Assembly)); | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Last note make sure from the added usings
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This line was already there I have just Removed ununsed usings and sorted them alphabetically
