Skip to content
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

Consider extension method for IConfiguration #1012

Closed
rockfordlhotka opened this issue Nov 27, 2018 · 9 comments
Closed

Consider extension method for IConfiguration #1012

rockfordlhotka opened this issue Nov 27, 2018 · 9 comments
Assignees

Comments

@rockfordlhotka
Copy link
Member

Instead of code like this to configure CSLA:

      var config = new ConfigurationBuilder()
             .AddJsonFile("appsettings.coresettings.test.json")
             .Build();
      CslaConfigurationOptions cslaOptions = new CslaConfigurationOptions();
      config.Bind("csla", cslaOptions);

perhaps we should have an extension method to allow something like this:

      var config = new ConfigurationBuilder()
             .AddJsonFile("appsettings.coresettings.test.json")
             .Build()
             .ConfigureCsla();

or

    config.ConfigureCsla();

This extension method would look something like this:

    public static IConfigurationRoot ConfigureCsla(this IConfigurationRoot config)
    {
      config.Bind("csla", new CslaConfigurationOptions());
      return config;
    }
@rockfordlhotka
Copy link
Member Author

@itgoran what are your thoughts on this idea? Does it make sense?

@itgoran
Copy link
Member

itgoran commented Nov 27, 2018

@rockfordlhotka
This would be the natural step forward in improving CSLA configuration experience on the dotnet core.

Also, I would like to suggest that we add the extension method under the namespace
Microsoft.Extensions.Configuration since this would allow usage without additional namespaces.

@rockfordlhotka
Copy link
Member Author

I think it should go in Csla.Configuration. Over the years I've coopted Microsoft namespaces a couple times, and that generally doesn't turn out well.

@rockfordlhotka
Copy link
Member Author

@itgoran I'll do this, as I'm working on a similar change for aspnet core.

rockfordlhotka added a commit to rockfordlhotka/csla that referenced this issue Nov 27, 2018
rockfordlhotka added a commit to rockfordlhotka/csla that referenced this issue Nov 27, 2018
…ervice for use in .NET Core DI scenarios
rockfordlhotka added a commit to rockfordlhotka/csla that referenced this issue Nov 27, 2018
@rockfordlhotka rockfordlhotka changed the title Consider extension method for IConfigurationRoot Consider extension method for IConfiguration Nov 27, 2018
@rockfordlhotka
Copy link
Member Author

rockfordlhotka commented Nov 27, 2018

@itgoran another thought I had is that maybe there should be a UseCsla() method for use in the ASP.NET Configure method. In a console app you have an IConfiguration object, but in ASP.NET Core it seems like it is almost more natural to have an IApplicationBuilder?

Something like this:

    public void Configure(IApplicationBuilder app, IHostingEnvironment env)
    {
      if (env.IsDevelopment())
      {
        app.UseDeveloperExceptionPage();
      }
      else
      {
        app.UseExceptionHandler("/Error");
      }

      app.UseStaticFiles();
      app.UseCookiePolicy();

      app.UseMvc();

      app.UseCsla();
    }

Though as I think about this it really doesn't work does it, because we need access to that IConfiguration instance...

@itgoran
Copy link
Member

itgoran commented Nov 29, 2018

@rockfordlhotka

I had similar thoughts when I was making CSLA configuration for dotnet core.
I was thinking either Configure or ConfigureService method and I was more convinced that first suggestion ConfigureCsla is the best choice.

The reasons for this are the following:

  • Configure method is used for setup HTTP request pipeline (such as middleware, routing ....) and we don't have that,
  • ConfigureService is used to add services to the container, but we don't have it (or maybe the proxy factory, DP writer/reader and like, can be added here, although I don't see a usage of it later one from DI perspective)

@rockfordlhotka
Copy link
Member Author

I have created an AddCsla extension method for ConfigureServices, and a new IDataPortalFactory that is available via DI.

I'm still working through the details of that, because it seems that the CI container can't create generic types, which is really what would be ideal. But still, having a DataPortalFactory injected into the page is good, because it should allow for mocking of IDataPortal<T> in a standard way.

rockfordlhotka added a commit to rockfordlhotka/csla that referenced this issue Dec 12, 2018
Version 4.9.0 automation moved this from To do to Done Dec 12, 2018
@dazinator
Copy link
Member

dazinator commented Apr 8, 2019

The case for UseCsla() came into play for me, only when I wanted some middleware in the request pipeline to automatically set the "IServiceProvider" that will be used for DI purposes, to the one provided by the current request i.e HttpContext.RequestServices.

Csla doesn't do DI with .net core yet, but perhaps with items like #1104 this might come into play.

Though as I think about this it really doesn't work does it, because we need access to that IConfiguration instance...

You can resolve it from appBuilder.ApplicationServices - I know this is relevent right now, but just for the record. The way I achieved this was to register an options object, using the IOptions<T> pattern in ConfigureServices(). Then in Configure() - you can resolve that IOptions<T> from appBuilder.ApplicationServices.

@github-actions
Copy link

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 10, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
No open projects
Version 4.9.0
  
Done
Development

No branches or pull requests

3 participants