diff --git a/src/Microsoft.AspNetCore.OData/Microsoft.AspNetCore.OData.xml b/src/Microsoft.AspNetCore.OData/Microsoft.AspNetCore.OData.xml index 0f8c11242..c92600988 100644 --- a/src/Microsoft.AspNetCore.OData/Microsoft.AspNetCore.OData.xml +++ b/src/Microsoft.AspNetCore.OData/Microsoft.AspNetCore.OData.xml @@ -9624,11 +9624,6 @@ Basically for $compute in $select and $expand - - - Gets or sets the query provider. - - Gets the parameter using parameter name. diff --git a/src/Microsoft.AspNetCore.OData/Query/Expressions/BinderExtensions.cs b/src/Microsoft.AspNetCore.OData/Query/Expressions/BinderExtensions.cs index 06bb3223a..477428413 100644 --- a/src/Microsoft.AspNetCore.OData/Query/Expressions/BinderExtensions.cs +++ b/src/Microsoft.AspNetCore.OData/Query/Expressions/BinderExtensions.cs @@ -273,11 +273,6 @@ public static IQueryable ApplyBind(this ISelectExpandBinder binder, IQueryable s throw Error.ArgumentNull(nameof(context)); } - if (string.IsNullOrEmpty(context.QueryProvider)) - { - context.QueryProvider = source.Provider.GetType().Namespace; - } - Type elementType = context.ElementClrType; LambdaExpression projectionLambda = binder.BindSelectExpand(selectExpandClause, context) as LambdaExpression; diff --git a/src/Microsoft.AspNetCore.OData/Query/Expressions/QueryBinderContext.cs b/src/Microsoft.AspNetCore.OData/Query/Expressions/QueryBinderContext.cs index 98588391d..909cd84a1 100644 --- a/src/Microsoft.AspNetCore.OData/Query/Expressions/QueryBinderContext.cs +++ b/src/Microsoft.AspNetCore.OData/Query/Expressions/QueryBinderContext.cs @@ -100,8 +100,6 @@ public QueryBinderContext(QueryBinderContext context, ODataQuerySettings querySe ElementType = Model.GetEdmTypeReference(ElementClrType)?.Definition; - QueryProvider = context.QueryProvider; - if (ElementType == null) { throw new ODataException(Error.Format(SRResources.ClrTypeNotInModel, ElementClrType.FullName)); @@ -170,11 +168,6 @@ public QueryBinderContext(QueryBinderContext context, ODataQuerySettings querySe /// public Expression Source { get;set; } - /// - /// Gets or sets the query provider. - /// - internal string QueryProvider { get; set; } - /// /// Gets the parameter using parameter name. /// diff --git a/src/Microsoft.AspNetCore.OData/Query/Expressions/SelectExpandBinder.cs b/src/Microsoft.AspNetCore.OData/Query/Expressions/SelectExpandBinder.cs index 9fab4f77a..c7d05325c 100644 --- a/src/Microsoft.AspNetCore.OData/Query/Expressions/SelectExpandBinder.cs +++ b/src/Microsoft.AspNetCore.OData/Query/Expressions/SelectExpandBinder.cs @@ -368,23 +368,9 @@ internal Expression ProjectElement(QueryBinderContext context, Expression source bool isSelectedAll = IsSelectAll(selectExpandClause); if (isSelectedAll) { - // If we have an EF query provider, then we remove the non-structural properties. The reason for this is to avoid - // Creating an expression that will generate duplicate LEFT joins when a LEFT join already exists in the IQueryable - // as observed here: https://github.com/OData/AspNetCoreOData/issues/497 - - Expression updatedExpression = null; - if (IsEfQueryProvider(context)) - { - updatedExpression = SelectExpandBinder.RemoveNonStructucalProperties(context, source, structuredType); - } - else - { - updatedExpression = source; - } - // Initialize property 'Instance' on the wrapper class wrapperProperty = wrapperType.GetProperty("Instance"); - wrapperTypeMemberAssignments.Add(Expression.Bind(wrapperProperty, updatedExpression)); + wrapperTypeMemberAssignments.Add(Expression.Bind(wrapperProperty, source)); wrapperProperty = wrapperType.GetProperty("UseInstanceForProperties"); wrapperTypeMemberAssignments.Add(Expression.Bind(wrapperProperty, Expression.Constant(true))); @@ -441,52 +427,6 @@ internal Expression ProjectElement(QueryBinderContext context, Expression source return Expression.MemberInit(Expression.New(wrapperType), wrapperTypeMemberAssignments); } - // Generates the expression - // { Instance = new Customer() {Id = $it.Id, Name= $it.Name}} - private static Expression RemoveNonStructucalProperties(QueryBinderContext context, Expression source, IEdmStructuredType structuredType) - { - IEdmModel model = context.Model; - Expression updatedSource = null; - - Type elementType = source.Type; - IEnumerable structuralProperties = structuredType.StructuralProperties(); - - PropertyInfo[] props = elementType.GetProperties(); - List bindings = new List(); - - foreach (PropertyInfo prop in props) - { - foreach (var sp in structuralProperties) - { - if (prop.CanWrite && model.GetClrPropertyName(sp) == prop.Name) - { - MemberExpression propertyExpression = Expression.Property(source, prop); - bindings.Add(Expression.Bind(prop, propertyExpression)); - break; - } - } - } - - NewExpression newExpression = Expression.New(source.Type); - updatedSource = Expression.MemberInit(newExpression, bindings); - - return updatedSource; - } - - private bool IsEfQueryProvider(QueryBinderContext context) - { - if (context.QueryProvider != null && - context.QueryProvider == HandleNullPropagationOptionHelper.EntityFrameworkQueryProviderNamespace || - context.QueryProvider == HandleNullPropagationOptionHelper.ObjectContextQueryProviderNamespaceEF5 || - context.QueryProvider == HandleNullPropagationOptionHelper.ObjectContextQueryProviderNamespaceEF6 || - context.QueryProvider == HandleNullPropagationOptionHelper.ObjectContextQueryProviderNamespaceEFCore2) - { - return true; - } - - return false; - } - private static bool ParseComputedDynamicProperties(QueryBinderContext context, IList dynamicPathSegments, bool isSelectedAll, out IList computedProperties) { diff --git a/test/Microsoft.AspNetCore.OData.E2E.Tests/Microsoft.AspNetCore.OData.E2E.Tests.csproj b/test/Microsoft.AspNetCore.OData.E2E.Tests/Microsoft.AspNetCore.OData.E2E.Tests.csproj index b4073ca90..8c34cdb81 100644 --- a/test/Microsoft.AspNetCore.OData.E2E.Tests/Microsoft.AspNetCore.OData.E2E.Tests.csproj +++ b/test/Microsoft.AspNetCore.OData.E2E.Tests/Microsoft.AspNetCore.OData.E2E.Tests.csproj @@ -37,6 +37,7 @@ + diff --git a/test/Microsoft.AspNetCore.OData.E2E.Tests/Regressions/RegressionsController.cs b/test/Microsoft.AspNetCore.OData.E2E.Tests/Regressions/RegressionsController.cs new file mode 100644 index 000000000..4f3adfc9a --- /dev/null +++ b/test/Microsoft.AspNetCore.OData.E2E.Tests/Regressions/RegressionsController.cs @@ -0,0 +1,96 @@ +//----------------------------------------------------------------------------- +// +// Copyright (c) .NET Foundation and Contributors. All rights reserved. +// See License.txt in the project root for license information. +// +//------------------------------------------------------------------------------ + +using System.Linq; +using Microsoft.AspNetCore.Mvc; +using Microsoft.AspNetCore.OData.Query; +using Microsoft.EntityFrameworkCore; + +namespace Microsoft.AspNetCore.OData.E2E.Tests.Regressions +{ + public class UsersController : Controller + { + private readonly RegressionsDbContext _dbContext; + + public UsersController(RegressionsDbContext dbContext) + { + dbContext.Database.EnsureCreated(); + _dbContext = dbContext; + if (!_dbContext.Users.Any()) + { + _dbContext.DataFiles.Add(new DataFile + { + FileId = 1, + FileName = "76x473626.pdf" + }); + + DataFile dataFile2 = new DataFile + { + FileId = 2, + FileName = "uyr65euit5.pdf" + }; + _dbContext.DataFiles.Add(dataFile2); + + _dbContext.DataFiles.Add(new DataFile + { + FileId = 3, + FileName = "hj7x87643.pdf" + }); + + _dbContext.Users.Add(new User + { + UserId = 1, + Name = "Alex", + Age = 35, + DataFileRef = null, + Files = null + }); + + _dbContext.Users.Add(new User + { + UserId = 2, + Name = "Amanda", + Age = 29, + DataFileRef = 2, + Files = dataFile2 + }); + + _dbContext.Users.Add(new User + { + UserId = 3, + Name = "Lara", + Age = 25, + DataFileRef = null, + Files = null + }); + + _dbContext.SaveChanges(); + } + } + + [HttpGet] + [EnableQuery] + public IQueryable Get() + { + IQueryable data = _dbContext.Users.AsQueryable(); + return data; + } + + [HttpGet] + [EnableQuery] + public IActionResult Get(int key) + { + User data = _dbContext.Users.Include(c => c.Files).FirstOrDefault(c => c.UserId == key); + if (data == null) + { + return NotFound(); + } + + return Ok(data); + } + } +} \ No newline at end of file diff --git a/test/Microsoft.AspNetCore.OData.E2E.Tests/Regressions/RegressionsDataModel.cs b/test/Microsoft.AspNetCore.OData.E2E.Tests/Regressions/RegressionsDataModel.cs new file mode 100644 index 000000000..69e26dd97 --- /dev/null +++ b/test/Microsoft.AspNetCore.OData.E2E.Tests/Regressions/RegressionsDataModel.cs @@ -0,0 +1,39 @@ +//----------------------------------------------------------------------------- +// +// Copyright (c) .NET Foundation and Contributors. All rights reserved. +// See License.txt in the project root for license information. +// +//------------------------------------------------------------------------------ + +using System.ComponentModel.DataAnnotations.Schema; +using System.ComponentModel.DataAnnotations; + +namespace Microsoft.AspNetCore.OData.E2E.Tests.Regressions +{ + public class User + { + [Key] + public int UserId { get; set; } + + [Required] + public string Name { get; set; } + + [Required] + public int Age { get; set; } + + //Navigations + [ForeignKey("Files")] + public int? DataFileRef { get; set; } + + public virtual DataFile Files { get; set; } + } + + public class DataFile + { + [Key] + public int FileId { get; set; } + + [Required] + public string FileName { get; set; } + } +} \ No newline at end of file diff --git a/test/Microsoft.AspNetCore.OData.E2E.Tests/Regressions/RegressionsDbContext.cs b/test/Microsoft.AspNetCore.OData.E2E.Tests/Regressions/RegressionsDbContext.cs new file mode 100644 index 000000000..902d91608 --- /dev/null +++ b/test/Microsoft.AspNetCore.OData.E2E.Tests/Regressions/RegressionsDbContext.cs @@ -0,0 +1,23 @@ +//----------------------------------------------------------------------------- +// +// Copyright (c) .NET Foundation and Contributors. All rights reserved. +// See License.txt in the project root for license information. +// +//------------------------------------------------------------------------------ + +using Microsoft.EntityFrameworkCore; + +namespace Microsoft.AspNetCore.OData.E2E.Tests.Regressions +{ + public class RegressionsDbContext : DbContext + { + public RegressionsDbContext(DbContextOptions options) + : base(options) + { + } + + public DbSet Users { get; set; } + + public DbSet DataFiles { get; set; } + } +} \ No newline at end of file diff --git a/test/Microsoft.AspNetCore.OData.E2E.Tests/Regressions/RegressionsTests.cs b/test/Microsoft.AspNetCore.OData.E2E.Tests/Regressions/RegressionsTests.cs new file mode 100644 index 000000000..ec076a716 --- /dev/null +++ b/test/Microsoft.AspNetCore.OData.E2E.Tests/Regressions/RegressionsTests.cs @@ -0,0 +1,149 @@ +//----------------------------------------------------------------------------- +// +// Copyright (c) .NET Foundation and Contributors. All rights reserved. +// See License.txt in the project root for license information. +// +//------------------------------------------------------------------------------ + +#if NET6_0_OR_GREATER // AddSqlite extension method applies to .NET 6 or greater +using System.Net.Http; +using System.Net; +using System.Threading.Tasks; +using Microsoft.AspNetCore.OData.TestCommon; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.OData.Edm; +using Microsoft.OData.ModelBuilder; +using Xunit; +using Xunit.Abstractions; + +namespace Microsoft.AspNetCore.OData.E2E.Tests.Regressions +{ + public class RegressionsTests : WebApiTestBase + { + private readonly ITestOutputHelper _output; + + public RegressionsTests(WebApiTestFixture fixture, ITestOutputHelper output) + : base(fixture) + { + _output = output; + } + + protected static void UpdateConfigureServices(IServiceCollection services) + { + services.AddSqlite($"Data Source=UsersWithNullContext.db"); + + IEdmModel edmModel = GetEdmModel(); + + services.ConfigureControllers(typeof(UsersController)); + + services.AddControllers().AddOData(opt => + opt.EnableQueryFeatures() + .AddRouteComponents("odata", edmModel)); + } + + [Fact] + public async Task QueryUsersWithNullReferenceKey_Works() + { + // Arrange + HttpClient client = CreateClient(); + + // Act + HttpResponseMessage response = await client.GetAsync("odata/users"); + + // Assert + Assert.NotNull(response); + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + Assert.NotNull(response.Content); + + string payloadBody = await response.Content.ReadAsStringAsync(); + + Assert.Equal("{\"@odata.context\":\"http://localhost/odata/$metadata#Users\"," + + "\"value\":[" + + "{\"UserId\":1,\"Name\":\"Alex\",\"Age\":35,\"DataFileRef\":null}," + + "{\"UserId\":2,\"Name\":\"Amanda\",\"Age\":29,\"DataFileRef\":2}," + + "{\"UserId\":3,\"Name\":\"Lara\",\"Age\":25,\"DataFileRef\":null}" + + "]" + + "}", payloadBody); + } + + // This is a failing regression test, see https://github.com/OData/AspNetCoreOData/issues/1035 + // It was fail with: System.InvalidOperationException : Nullable object must have a value. + [Fact] + public async Task QueryUsersWithNullReferenceKeyUsingDollarExpand_Works() + { + // Arrange + HttpClient client = CreateClient(); + + // Act + HttpResponseMessage response = await client.GetAsync($"odata/users?$expand=Files"); + + // Assert + Assert.NotNull(response); + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + Assert.NotNull(response.Content); + + string payloadBody = await response.Content.ReadAsStringAsync(); + + Assert.Equal("{\"@odata.context\":\"http://localhost/odata/$metadata#Users(Files())\"," + + "\"value\":[" + + "{\"UserId\":1,\"Name\":\"Alex\",\"Age\":35,\"DataFileRef\":null,\"Files\":null}," + + "{\"UserId\":2,\"Name\":\"Amanda\",\"Age\":29,\"DataFileRef\":2,\"Files\":{\"FileId\":2,\"FileName\":\"uyr65euit5.pdf\"}}," + + "{\"UserId\":3,\"Name\":\"Lara\",\"Age\":25,\"DataFileRef\":null,\"Files\":null}" + + "]" + + "}", payloadBody); + } + + public static TheoryDataSet SingleUserQueryCases + { + get + { + var data = new TheoryDataSet + { + { + "odata/users/1", + "{CONTEXTURI,\"UserId\":1,\"Name\":\"Alex\",\"Age\":35,\"DataFileRef\":null,\"Files\":null}" + }, + { + "odata/users/2", + "{CONTEXTURI,\"UserId\":2,\"Name\":\"Amanda\",\"Age\":29,\"DataFileRef\":2,\"Files\":{\"FileId\":2,\"FileName\":\"uyr65euit5.pdf\"}}" + }, + { + "odata/users/3", + "{CONTEXTURI,\"UserId\":3,\"Name\":\"Lara\",\"Age\":25,\"DataFileRef\":null,\"Files\":null}" + } + }; + + return data; + } + } + + [Theory] + [MemberData(nameof(SingleUserQueryCases))] + public async Task QuerySingleUserWithNullReferenceKeyUsingDollarExpand_Works(string request, string expected) + { + // Arrange + HttpClient client = CreateClient(); + expected = expected.Replace("CONTEXTURI", "\"@odata.context\":\"http://localhost/odata/$metadata#Users(Files())/$entity\""); + + // Act + HttpResponseMessage response = await client.GetAsync($"{request}?$expand=Files"); + + // Assert + Assert.NotNull(response); + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + Assert.NotNull(response.Content); + + string payloadBody = await response.Content.ReadAsStringAsync(); + + Assert.Equal(expected, payloadBody); + } + + public static IEdmModel GetEdmModel() + { + ODataConventionModelBuilder builder = new ODataConventionModelBuilder(); + builder.EntitySet("Users"); + return builder.GetEdmModel(); + } + } +} +#endif \ No newline at end of file diff --git a/test/Microsoft.AspNetCore.OData.Tests/Query/Expressions/SelectExpandBinderTest.cs b/test/Microsoft.AspNetCore.OData.Tests/Query/Expressions/SelectExpandBinderTest.cs index dd105644b..adae7f691 100644 --- a/test/Microsoft.AspNetCore.OData.Tests/Query/Expressions/SelectExpandBinderTest.cs +++ b/test/Microsoft.AspNetCore.OData.Tests/Query/Expressions/SelectExpandBinderTest.cs @@ -257,92 +257,6 @@ public void Bind_GeneratedExpression_ContainsExpandedObject() Assert.Same(_queryable.First(), innerInnerCustomer.Instance); } - [Theory] - [InlineData(HandleNullPropagationOptionHelper.EntityFrameworkQueryProviderNamespace)] - [InlineData(HandleNullPropagationOptionHelper.ObjectContextQueryProviderNamespaceEFCore2)] - [InlineData(HandleNullPropagationOptionHelper.ObjectContextQueryProviderNamespaceEF5)] - [InlineData(HandleNullPropagationOptionHelper.ObjectContextQueryProviderNamespaceEF6)] - public void Bind_UsingEFQueryProvider_GeneratedExpression__DoesNot_ContainExpandedObject(string queryProvider) - { - // Arrange - SelectExpandQueryOption selectExpand = new SelectExpandQueryOption("Orders", "Orders,Orders($expand=Customer)", _context); - - // Act - SelectExpandBinder binder = new SelectExpandBinder(); - _queryBinderContext.QueryProvider = queryProvider; - IQueryable queryable = binder.ApplyBind(_queryable, selectExpand.SelectExpandClause, _queryBinderContext); - - // Assert - IEnumerator enumerator = queryable.GetEnumerator(); - Assert.True(enumerator.MoveNext()); - var partialCustomer = Assert.IsAssignableFrom>(enumerator.Current); - Assert.False(enumerator.MoveNext()); - Assert.Null(partialCustomer.Instance); - Assert.Equal("Microsoft.AspNetCore.OData.Tests.Query.Expressions.QueryCustomer", partialCustomer.InstanceType); - IEnumerable> innerOrders = partialCustomer.Container - .ToDictionary(PropertyMapper)["Orders"] as IEnumerable>; - Assert.NotNull(innerOrders); - SelectExpandWrapper partialOrder = innerOrders.Single(); - - // We only write structural properties to the instance. - // This means that navigation properties on the instance property will be null - // when using any instance of EF query provider, and all structural properties - // will be assigned. - Assert.Null(partialOrder.Instance.Customer); - Assert.NotEqual(0, partialOrder.Instance.Id); - Assert.NotNull(partialOrder.Instance.Title); - - object customer = partialOrder.Container.ToDictionary(PropertyMapper)["Customer"]; - SelectExpandWrapper innerInnerCustomer = Assert.IsAssignableFrom>(customer); - - Assert.Null(innerInnerCustomer.Instance.Orders); - Assert.Equal(2, innerInnerCustomer.Instance.TestReadonlyProperty.Count); - Assert.Equal("Test1", innerInnerCustomer.Instance.TestReadonlyProperty[0]); - Assert.Equal("Test2", innerInnerCustomer.Instance.TestReadonlyProperty[1]); - Assert.Equal(2, innerInnerCustomer.Instance.TestReadOnlyWithAttribute); - } - - [Theory] - [InlineData(HandleNullPropagationOptionHelper.EntityFrameworkQueryProviderNamespace)] - [InlineData(HandleNullPropagationOptionHelper.ObjectContextQueryProviderNamespaceEFCore2)] - [InlineData(HandleNullPropagationOptionHelper.ObjectContextQueryProviderNamespaceEF5)] - [InlineData(HandleNullPropagationOptionHelper.ObjectContextQueryProviderNamespaceEF6)] - public void Bind_UsingEFQueryProvider_LowerCamelCasedModel_GeneratedExpression__DoesNot_ContainExpandedObject(string queryProvider) - { - // Arrange - SelectExpandQueryOption selectExpand = new SelectExpandQueryOption("Orders", "Orders,Orders($expand=Customer)", _context_lowerCamelCased); - - // Act - SelectExpandBinder binder = new SelectExpandBinder(); - _queryBinderContext_lowerCamelCased.QueryProvider = queryProvider; - IQueryable queryable = binder.ApplyBind(_queryable_lowerCamelCased, selectExpand.SelectExpandClause, _queryBinderContext_lowerCamelCased); - - // Assert - IEnumerator enumerator = queryable.GetEnumerator(); - Assert.True(enumerator.MoveNext()); - var partialCustomer = Assert.IsAssignableFrom>(enumerator.Current); - Assert.False(enumerator.MoveNext()); - Assert.Null(partialCustomer.Instance); - Assert.Equal("Microsoft.AspNetCore.OData.Tests.Query.Expressions.QueryCustomer", partialCustomer.InstanceType); - IEnumerable> innerOrders = partialCustomer.Container - .ToDictionary(PropertyMapper)["orders"] as IEnumerable>; - Assert.NotNull(innerOrders); - SelectExpandWrapper partialOrder = innerOrders.Single(); - - // We only write structural properties to the instance. - // This means that navigation properties on the instance property will be null - // when using any instance of EF query provider, and all structural properties - // will be assigned. - Assert.Null(partialOrder.Instance.Customer); - Assert.NotEqual(0, partialOrder.Instance.Id); - Assert.NotNull(partialOrder.Instance.Title); - - object customer = partialOrder.Container.ToDictionary(PropertyMapper)["customer"]; - SelectExpandWrapper innerInnerCustomer = Assert.IsAssignableFrom>(customer); - - Assert.Null(innerInnerCustomer.Instance.Orders); - } - [Fact] public void Bind_GeneratedExpression_CheckNullObjectWithinChainProjectionByKey() { diff --git a/tool/builder.versions.settings.targets b/tool/builder.versions.settings.targets index 0a475b0df..c2f36bbe1 100644 --- a/tool/builder.versions.settings.targets +++ b/tool/builder.versions.settings.targets @@ -3,7 +3,7 @@ 8 2 - 2 + 3