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

Adds dependency injection support in non-model scenario #439

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 12 additions & 12 deletions src/Microsoft.AspNetCore.OData/Extensions/HttpRequestExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -271,12 +271,6 @@ public static IServiceProvider GetRouteServices(this HttpRequest request)
return requestContainer;
}

// if the prefixName == null, it's a non-model scenario
if (request.ODataFeature().RoutePrefix == null)
{
return null;
}

// HTTP routes will not have chance to call CreateRequestContainer. We have to call it.
return request.CreateRouteServices(request.ODataFeature().RoutePrefix);
}
Expand All @@ -300,6 +294,11 @@ public static IServiceProvider CreateRouteServices(this HttpRequest request, str
}

IServiceScope requestScope = request.CreateRequestScope(routePrefix);
if (requestScope == null)
{
// non-model scenario with dependency injection non enabled
return null;
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

insert whiteline

IServiceProvider requestContainer = requestScope.ServiceProvider;

request.ODataFeature().RequestScope = requestScope;
Expand Down Expand Up @@ -341,14 +340,15 @@ private static IServiceScope CreateRequestScope(this HttpRequest request, string
{
ODataOptions options = request.ODataOptions();

IServiceProvider rootContainer = options.GetRouteServices(routePrefix);
IServiceScope scope = rootContainer.GetRequiredService<IServiceScopeFactory>().CreateScope();

// Bind scoping request into the OData container.
if (!string.IsNullOrEmpty(routePrefix))
IServiceProvider rootContainer = options?.GetRouteServices(routePrefix);
if (rootContainer == null)
{
scope.ServiceProvider.GetRequiredService<HttpRequestScope>().HttpRequest = request;
return null;
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

insert whiteline after {} block

IServiceScope scope = rootContainer.GetRequiredService<IServiceScopeFactory>().CreateScope();

// Bind scoping request into the OData container.
scope.ServiceProvider.GetRequiredService<HttpRequestScope>().HttpRequest = request;
kakone marked this conversation as resolved.
Show resolved Hide resolved

return scope;
}
Expand Down
11 changes: 9 additions & 2 deletions src/Microsoft.AspNetCore.OData/Microsoft.AspNetCore.OData.xml
Original file line number Diff line number Diff line change
Expand Up @@ -6117,6 +6117,13 @@
Gets the <see cref="P:Microsoft.AspNetCore.OData.ODataOptions.RouteOptions"/> instance responsible for configuring the route template.
</summary>
</member>
<member name="M:Microsoft.AspNetCore.OData.ODataOptions.ConfigureServiceCollection(System.Action{Microsoft.Extensions.DependencyInjection.IServiceCollection})">
<summary>
Configures service collection for non-EDM scenario
</summary>
<param name="configureServices">The configuring action to add the services to the container.</param>
<returns>The current <see cref="T:Microsoft.AspNetCore.OData.ODataOptions"/> instance to enable fluent configuration.</returns>
</member>
<member name="P:Microsoft.AspNetCore.OData.ODataOptions.RouteComponents">
<summary>
Contains the OData <see cref="T:Microsoft.OData.Edm.IEdmModel"/> instances and dependency injection containers for specific routes.
Expand Down Expand Up @@ -6233,12 +6240,12 @@
Gets the query setting.
</summary>
</member>
<member name="M:Microsoft.AspNetCore.OData.ODataOptions.BuildRouteContainer(Microsoft.OData.Edm.IEdmModel,System.Action{Microsoft.Extensions.DependencyInjection.IServiceCollection})">
<member name="M:Microsoft.AspNetCore.OData.ODataOptions.BuildContainer(System.Action{Microsoft.Extensions.DependencyInjection.IServiceCollection},Microsoft.OData.Edm.IEdmModel)">
<summary>
Build the container.
</summary>
<param name="model">The Edm model.</param>
<param name="setupAction">The setup config.</param>
<param name="model">The Edm model.</param>
<returns>The built service provider.</returns>
</member>
<member name="M:Microsoft.AspNetCore.OData.ODataOptions.SanitizeRoutePrefix(System.String)">
Expand Down
34 changes: 24 additions & 10 deletions src/Microsoft.AspNetCore.OData/ODataOptions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,19 @@ public class ODataOptions
/// </summary>
public ODataRouteOptions RouteOptions { get; } = new ODataRouteOptions();

private IServiceProvider ServiceProvider { get; set; }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

private IServiceProvider ServiceProvider { get; set; }

move the private field ahead

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

make it a field, it seems we don't need a property


/// <summary>
/// Configures service collection for non-EDM scenario
/// </summary>
/// <param name="configureServices">The configuring action to add the services to the container.</param>
/// <returns>The current <see cref="ODataOptions"/> instance to enable fluent configuration.</returns>
public ODataOptions ConfigureServiceCollection(Action<IServiceCollection> configureServices)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ConfigureServiceCollection

maybe just use AddRouteComponents as the method name

{
ServiceProvider = BuildContainer(configureServices);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BuildContainer

call the method using model=null as parameter?

return this;
}

#endregion

#region RouteComponents
Expand Down Expand Up @@ -144,7 +157,7 @@ public ODataOptions AddRouteComponents(string routePrefix, IEdmModel model, Acti


// Consider to use Lazy<IServiceProvider> ?
IServiceProvider serviceProvider = BuildRouteContainer(model, configureServices);
IServiceProvider serviceProvider = BuildContainer(configureServices, model);
RouteComponents[sanitizedRoutePrefix] = (model, serviceProvider);
return this;
}
Expand All @@ -158,7 +171,7 @@ public IServiceProvider GetRouteServices(string routePrefix)
{
if (routePrefix == null)
{
return null;
return ServiceProvider;
}

string sanitizedRoutePrefix = SanitizeRoutePrefix(routePrefix);
Expand Down Expand Up @@ -284,13 +297,11 @@ public ODataOptions SetMaxTop(int? maxTopValue)
/// <summary>
/// Build the container.
/// </summary>
/// <param name="model">The Edm model.</param>
/// <param name="setupAction">The setup config.</param>
/// <param name="model">The Edm model.</param>
/// <returns>The built service provider.</returns>
private IServiceProvider BuildRouteContainer(IEdmModel model, Action<IServiceCollection> setupAction)
private IServiceProvider BuildContainer(Action<IServiceCollection> setupAction, IEdmModel model = null)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BuildContainer

it seems we don't need to change the method name

{
Contract.Assert(model != null);

ServiceCollection services = new ServiceCollection();
DefaultContainerBuilder builder = new DefaultContainerBuilder();

Expand All @@ -311,10 +322,13 @@ private IServiceProvider BuildRouteContainer(IEdmModel model, Action<IServiceCol
EnableNoDollarQueryOptions = EnableNoDollarQueryOptions // retrieve it from global setting
});

// Inject the Edm model.
// From Current ODL implement, such injection only be used in reader and writer if the input
// model is null.
builder.Services.AddSingleton(sp => model);
if (model != null)
{
// Inject the Edm model.
// From Current ODL implement, such injection only be used in reader and writer if the input
// model is null.
builder.Services.AddSingleton(sp => model);
}

// Inject the customized services.
setupAction?.Invoke(builder.Services);
Expand Down
1 change: 1 addition & 0 deletions src/Microsoft.AspNetCore.OData/PublicAPI.Unshipped.txt
Original file line number Diff line number Diff line change
Expand Up @@ -659,6 +659,7 @@ Microsoft.AspNetCore.OData.ODataOptions.EnableAttributeRouting.get -> bool
Microsoft.AspNetCore.OData.ODataOptions.EnableAttributeRouting.set -> void
Microsoft.AspNetCore.OData.ODataOptions.EnableContinueOnErrorHeader.get -> bool
Microsoft.AspNetCore.OData.ODataOptions.EnableContinueOnErrorHeader.set -> void
Microsoft.AspNetCore.OData.ODataOptions.ConfigureServiceCollection(System.Action<Microsoft.Extensions.DependencyInjection.IServiceCollection> configureServices) -> Microsoft.AspNetCore.OData.ODataOptions
Microsoft.AspNetCore.OData.ODataOptions.EnableNoDollarQueryOptions.get -> bool
Microsoft.AspNetCore.OData.ODataOptions.EnableNoDollarQueryOptions.set -> void
Microsoft.AspNetCore.OData.ODataOptions.EnableQueryFeatures(int? maxTopValue = null) -> Microsoft.AspNetCore.OData.ODataOptions
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

<PropertyGroup>
<TargetFrameworks>netcoreapp3.1;net5.0</TargetFrameworks>
<TargetFrameworks Condition="'$(VisualStudioVersion)' == '' or '$(VisualStudioVersion)' >= '17.0'">$(TargetFrameworks);net6.0</TargetFrameworks>
<TargetFrameworks Condition="'$(VisualStudioVersion)' == '' or '$(VisualStudioVersion)' &gt;= '17.0'">$(TargetFrameworks);net6.0</TargetFrameworks>
<AssemblyName>Microsoft.AspNetCore.OData.E2E.Tests</AssemblyName>
<RootNamespace>Microsoft.AspNetCore.OData.E2E.Tests</RootNamespace>

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
using Microsoft.AspNetCore.OData.E2E.Tests.Extensions;
using Microsoft.AspNetCore.OData.TestCommon;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.OData.UriParser;
using Newtonsoft.Json.Linq;
using System.Net.Http;
using System.Threading.Tasks;
using Xunit;

namespace Microsoft.AspNetCore.OData.E2E.Tests.NonEdm
{
public class ConfigureServiceCollectionTest : WebApiTestBase<ConfigureServiceCollectionTest>
{
public ConfigureServiceCollectionTest(WebApiTestFixture<ConfigureServiceCollectionTest> fixture)
: base(fixture)
{
}

protected static void UpdateConfigureServices(IServiceCollection services)
{
services.ConfigureControllers(typeof(CustomersController));
services.AddControllers().AddOData(opt =>
{
opt.EnableQueryFeatures();
opt.ConfigureServiceCollection(services =>
{
services.AddSingleton<ODataUriResolver>(sp => new StringAsEnumResolver() { EnableCaseInsensitive = true });
});
});
}

[Fact]
public async Task EnableConfigureServiceCollectionTest()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

EnableConfigureServiceCollectionTest

add new tests, for example, $select, $expand, $top to make sure it's working.

{
using (var response = await CreateClient().SendAsync(new HttpRequestMessage(HttpMethod.Get, $"api/Customers?$filter=Gender eq 'MaLe'")))
{
var values = await response.Content.ReadAsObject<JArray>();
Assert.Equal(3, values.Count);
}
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
using Microsoft.AspNetCore.Mvc;
using Microsoft.AspNetCore.OData.Query;
using System.Collections.Generic;
using System.Linq;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

copy right head


Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

move using System.* ahead
and sort

namespace Microsoft.AspNetCore.OData.E2E.Tests.NonEdm
{
[ApiController]
[Route("api/[controller]")]
public class CustomersController : ControllerBase
{
[HttpGet]
public IActionResult Get(ODataQueryOptions<Customer> options)
{
return Ok(options.ApplyTo(NonEdmDbContext.GetCustomers().AsQueryable()));
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add a test using [EnableQuery] attribute

}

public class NonEdmDbContext
{
private static IList<Customer> _customers;

public static IList<Customer> GetCustomers()
{
if (_customers == null)
{
Generate();
}
return _customers;
}

private static void Generate()
{
_customers = Enumerable.Range(1, 5).Select(e =>
new Customer
{
Id = e,
Name = "Customer #" + e,
Gender = e%2 == 0 ? Gender.Female : Gender.Male,
}).ToList();
}
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

put it into a new file

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
namespace Microsoft.AspNetCore.OData.E2E.Tests.NonEdm
{
public class Customer
{
public int Id { get; set; }
public string Name { get; set; }
public Gender Gender { get; set; }
}

public enum Gender
{
Male = 1,
Female = 2
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,7 @@ public class Microsoft.AspNetCore.OData.ODataOptions {
public Microsoft.AspNetCore.OData.ODataOptions AddRouteComponents (string routePrefix, Microsoft.OData.Edm.IEdmModel model)
public Microsoft.AspNetCore.OData.ODataOptions AddRouteComponents (string routePrefix, Microsoft.OData.Edm.IEdmModel model, Microsoft.AspNetCore.OData.Batch.ODataBatchHandler batchHandler)
public Microsoft.AspNetCore.OData.ODataOptions AddRouteComponents (string routePrefix, Microsoft.OData.Edm.IEdmModel model, System.Action`1[[Microsoft.Extensions.DependencyInjection.IServiceCollection]] configureServices)
public Microsoft.AspNetCore.OData.ODataOptions ConfigureServiceCollection (System.Action`1[[Microsoft.Extensions.DependencyInjection.IServiceCollection]] configureServices)
public Microsoft.AspNetCore.OData.ODataOptions Count ()
public Microsoft.AspNetCore.OData.ODataOptions EnableQueryFeatures (params System.Nullable`1[[System.Int32]] maxTopValue)
public Microsoft.AspNetCore.OData.ODataOptions Expand ()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,7 @@ public class Microsoft.AspNetCore.OData.ODataOptions {
public Microsoft.AspNetCore.OData.ODataOptions AddRouteComponents (string routePrefix, Microsoft.OData.Edm.IEdmModel model)
public Microsoft.AspNetCore.OData.ODataOptions AddRouteComponents (string routePrefix, Microsoft.OData.Edm.IEdmModel model, Microsoft.AspNetCore.OData.Batch.ODataBatchHandler batchHandler)
public Microsoft.AspNetCore.OData.ODataOptions AddRouteComponents (string routePrefix, Microsoft.OData.Edm.IEdmModel model, System.Action`1[[Microsoft.Extensions.DependencyInjection.IServiceCollection]] configureServices)
public Microsoft.AspNetCore.OData.ODataOptions ConfigureServiceCollection (System.Action`1[[Microsoft.Extensions.DependencyInjection.IServiceCollection]] configureServices)
public Microsoft.AspNetCore.OData.ODataOptions Count ()
public Microsoft.AspNetCore.OData.ODataOptions EnableQueryFeatures (params System.Nullable`1[[System.Int32]] maxTopValue)
public Microsoft.AspNetCore.OData.ODataOptions Expand ()
Expand Down