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

Fixes #1286: String Collections support in $select. #1282

Merged
merged 50 commits into from
Nov 3, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
50 commits
Select commit Hold shift + click to select a range
de5f4ec
Checking for primitive collection in SelectExpandBinder.ProjectAsWrap…
anasik Jul 16, 2024
6f942d2
Checking only for string since that seems to be the only case that's …
anasik Jul 17, 2024
3af92c9
tests hehe
anasik Jul 20, 2024
d0d3d3c
Added SQLite DB to aid with testing; Discovered that all collections …
anasik Jul 25, 2024
7610be8
Added target project, excluded db file from compilation, made test co…
anasik Jul 25, 2024
b88ac4a
Added DateOnly and put #if_NET8 around the if block. Also removed the…
anasik Jul 25, 2024
9344803
removed commented code
anasik Jul 25, 2024
d2b2c45
Uncorrupt
anasik Jul 25, 2024
633e00d
git attributes, last ditch effort to fix sqlite db
anasik Jul 25, 2024
5f90a20
Forgot to rename in some places
anasik Jul 25, 2024
9212f3a
db again fingers crossed
anasik Jul 25, 2024
cb9c48e
Removed toList from controller since the bug pertains to deferred exe…
anasik Jul 25, 2024
30fa2c3
Commented out DateTimeOffset filter test cases since the error itself…
anasik Jul 25, 2024
7bee0a6
Update src/Microsoft.AspNetCore.OData/Query/Expressions/SelectExpandB…
anasik Aug 9, 2024
32fe60a
Update test/Microsoft.AspNetCore.OData.E2E.Tests/Lists/ListsDataModel.cs
anasik Aug 9, 2024
a13ded0
Update test/Microsoft.AspNetCore.OData.E2E.Tests/Lists/ListsControlle…
anasik Aug 9, 2024
b5f1d0f
renamed ListTestOrders to ListTestOrder for consistency
anasik Aug 9, 2024
9681c4a
Init order
anasik Aug 9, 2024
b6c0bdb
ADDED tests for ListTestOrder
anasik Aug 9, 2024
c77b615
TypeHelper.isPrimitive
anasik Aug 9, 2024
e96ca7d
Orders table
anasik Aug 9, 2024
2a1ea20
Using direct table reference instead of Set. Dropping Orders table in…
anasik Aug 9, 2024
dcd6a3d
Added Orders to Context
anasik Aug 9, 2024
a44cafe
Added Orders to EDM
anasik Aug 9, 2024
446ae87
Added ListTestOrder to _map and $expand to both test cases
anasik Aug 9, 2024
6b9f717
removed unnecessary spaces
anasik Aug 9, 2024
8888e28
Update src/Microsoft.AspNetCore.OData/Query/Expressions/SelectExpandB…
anasik Aug 9, 2024
504072c
Renamed function. Added TimeOnly.
anasik Aug 9, 2024
cce4459
Added Decimal
anasik Aug 9, 2024
6896036
Return doc.
anasik Aug 9, 2024
a19a6f6
Update src/Microsoft.AspNetCore.OData/Common/TypeHelper.cs
anasik Aug 9, 2024
f1615dd
Remove duplicated doc
anasik Aug 9, 2024
3d44407
Added copyright comment to ListsContext and removed commented lines f…
anasik Aug 10, 2024
62d19d8
Removed *.db file in favor of in-memory SQLite
anasik Aug 15, 2024
eeadc22
removed db binary gitattribute
anasik Aug 19, 2024
d16f188
fixed indentation
anasik Sep 16, 2024
e3daf8a
using sqlite
anasik Sep 25, 2024
11374e0
Forgot to push this
anasik Sep 27, 2024
825d791
Updated Microsoft.AspNetCore.Mvc.Testing
anasik Sep 27, 2024
d221a05
I think this may be a better approach
anasik Sep 27, 2024
a166c04
Let's see if this cleans up the diff
anasik Sep 27, 2024
4fd88a8
Try and try again
anasik Sep 27, 2024
e2e08c5
Update test/Microsoft.AspNetCore.OData.E2E.Tests/Microsoft.AspNetCore…
anasik Sep 30, 2024
3599fbf
Update test/Microsoft.AspNetCore.OData.E2E.Tests/Microsoft.AspNetCore…
anasik Sep 30, 2024
05af092
Update test/Microsoft.AspNetCore.OData.E2E.Tests/Microsoft.AspNetCore…
anasik Sep 30, 2024
093078f
Update test/Microsoft.AspNetCore.OData.E2E.Tests/Microsoft.AspNetCore…
anasik Sep 30, 2024
cc6531c
Update test/Microsoft.AspNetCore.OData.E2E.Tests/Microsoft.AspNetCore…
anasik Sep 30, 2024
3fdfd3c
Update test/Microsoft.AspNetCore.OData.E2E.Tests/Microsoft.AspNetCore…
anasik Sep 30, 2024
471b4dd
Update test/Microsoft.AspNetCore.OData.E2E.Tests/Microsoft.AspNetCore…
anasik Sep 30, 2024
0180f05
ProjectReference
anasik Sep 30, 2024
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
20 changes: 20 additions & 0 deletions src/Microsoft.AspNetCore.OData/Common/TypeHelper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -252,6 +252,26 @@ Type collectionInterface
return false;
}

/// <summary>
/// Check whether the given type is a primitive type or known type.
/// </summary>
/// <param name="type">The type to validate.</param>
/// <returns>True if type is primitive or known type, otherwise False.</returns>
public static bool IsPrimitiveOrKnownType(Type type)
anasik marked this conversation as resolved.
Show resolved Hide resolved
{
return type.IsPrimitive
|| type == typeof(string)
anasik marked this conversation as resolved.
Show resolved Hide resolved
|| type == typeof(Uri)
Copy link
Contributor

Choose a reason for hiding this comment

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

Team had a concern about Uri
We currently do not support Uri as a primitive type @gathogojr

Copy link
Author

@anasik anasik Nov 2, 2024

Choose a reason for hiding this comment

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

@WanjohiSammy Then why didn't you just say so? thought we'd already discussed this. But lets discuss it again.

Personally, I don't consider even string a primitive type, and neither does anyone else in our profession. But for the purpose of the task at hand, it is and the documentation for ASP NET Core says so. Just like it says the same thing for Uri. If it's too concerning, we can remove this for now and wait for someone else to attempt to add it back a few PRs down the line.

With all due respect sir, this is actually a pretty major bug, on our last stable version, that has been fixed since June but we are not rolling it out for unknown reasons. Me and my team are personally suffering from this enough to be using a fork in our project. And there could be people out there who have been experiencing this ever since it was first released.

All I am saying is I would appreciate us doing anything we can to move things along. Which means that if a line of code that adds an extra case for the URL type bothers the team so much, I don't mind removing it because that way the 99 other types don't retain the bug for another 6 months.

Copy link
Contributor

Choose a reason for hiding this comment

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

@WanjohiSammy Then why didn't you just say so? thought we'd already discussed this. But lets discuss it again.

Personally, I don't consider even string a primitive type, and neither does anyone else in our profession. But for the purpose of the task at hand, it is and the documentation for ASP NET Core says so. Just like it says the same thing for Uri. If it's too concerning, we can remove this for now and wait for someone else to attempt to add it back a few PRs down the line.

With all due respect sir, this is actually a pretty major bug, on our last stable version, that has been fixed since June but we are not rolling it out for unknown reasons. Me and my team are personally suffering from this enough to be using a fork in our project. And there could be people out there who have been experiencing this ever since it was first released.

All I am saying is I would appreciate us doing anything we can to move things along. Which means that if a line of code that adds an extra case for the URL type bothers the team so much, I don't mind removing it because that way the 99 other types don't retain the bug for another 6 months.

This looks good to me.
@gathogojr, @habbes what do you think about this change.
@anasik apologies

anasik marked this conversation as resolved.
Show resolved Hide resolved
|| type == typeof(DateTime)
#if NET6_0_OR_GREATER
|| type == typeof(DateOnly)
|| type == typeof(TimeOnly)
#endif
|| type == typeof(DateTimeOffset)
|| type == typeof(Guid)
|| type == typeof(Decimal);
}

internal static bool IsDictionary(Type clrType)
{
if (clrType == null)
Expand Down
7 changes: 7 additions & 0 deletions src/Microsoft.AspNetCore.OData/Microsoft.AspNetCore.OData.xml
Original file line number Diff line number Diff line change
Expand Up @@ -1181,6 +1181,13 @@
<param name="elementType">out: the element type of the collection.</param>
<returns>True if the type is an enumeration; false otherwise.</returns>
</member>
<member name="M:Microsoft.AspNetCore.OData.Common.TypeHelper.IsPrimitiveOrKnownType(System.Type)">
<summary>
Check whether the given type is a primitive type or known type.
</summary>
<param name="type">The type to validate.</param>
<returns>True if type is primitive or known type, otherwise False.</returns>
</member>
<member name="M:Microsoft.AspNetCore.OData.Common.TypeHelper.GetImplementedIEnumerableType(System.Type)">
<summary>
Returns type of T if the type implements IEnumerable of T, otherwise, return null.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1357,7 +1357,9 @@ private Expression ProjectCollection(QueryBinderContext context, Expression sour
// expression
// source.Select((ElementType element) => new Wrapper { })
var selectMethod = GetSelectMethod(elementType, projection.Type);
Expression selectedExpresion = Expression.Call(selectMethod, source, selector);
bool isPrimitiveCollection = TypeHelper.IsPrimitiveOrKnownType(elementType);
Expression selectedExpresion =
isPrimitiveCollection ? source : Expression.Call(selectMethod, source, selector);

// Append ToList() to collection as a hint to LINQ provider to buffer correlated sub-queries in memory and avoid executing N+1 queries
if (settings.EnableCorrelatedSubqueryBuffering)
Expand Down
26 changes: 26 additions & 0 deletions test/Microsoft.AspNetCore.OData.E2E.Tests/Lists/ListsContext.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
//-----------------------------------------------------------------------------
// <copyright file="ListsContext.cs" company=".NET Foundation">
// Copyright (c) .NET Foundation and Contributors. All rights reserved.
// See License.txt in the project root for license information.
// </copyright>
//------------------------------------------------------------------------------

using System;
using System.IO;
using Microsoft.EntityFrameworkCore;

anasik marked this conversation as resolved.
Show resolved Hide resolved
namespace Microsoft.AspNetCore.OData.E2E.Tests.Lists
{
public class ListsContext : DbContext
{
public ListsContext(DbContextOptions<ListsContext> options)
: base(options)
{

}

public DbSet<Product> Products{ get; set; }
public DbSet<Order> Orders{ get; set; }
}
}

186 changes: 186 additions & 0 deletions test/Microsoft.AspNetCore.OData.E2E.Tests/Lists/ListsController.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,186 @@
//-----------------------------------------------------------------------------
// <copyright file="ListsController.cs" company=".NET Foundation">
// Copyright (c) .NET Foundation and Contributors. All rights reserved.
// See License.txt in the project root for license information.
// </copyright>
//------------------------------------------------------------------------------

using System;
using System.Collections.Generic;
using System.Linq;
using Microsoft.AspNetCore.Http;
using Microsoft.AspNetCore.Mvc;
using Microsoft.AspNetCore.OData.Deltas;
using Microsoft.AspNetCore.OData.Formatter;
using Microsoft.AspNetCore.OData.Query;
using Microsoft.AspNetCore.OData.Routing.Attributes;
using Microsoft.AspNetCore.OData.Routing.Controllers;
using Microsoft.EntityFrameworkCore;
using Xunit;

namespace Microsoft.AspNetCore.OData.E2E.Tests.Lists
{
[Route("convention")]
public class ProductsController : ODataController
{
private readonly ListsContext _dbContext;
public ProductsController(ListsContext context)
{
_dbContext = context;
}

[EnableQuery(PageSize = 10, MaxExpansionDepth = 5)]
public IActionResult Get()
{
return Ok(_dbContext.Products);
}

[EnableQuery]
public IActionResult Get(string key)
{
return Ok(_dbContext.Products.Find(key));
}

public IActionResult Post([FromBody] Product Product)
{
Product.ProductId = _dbContext.Products.Count() + 1+"";
_dbContext.Products.Add(Product);
_dbContext.SaveChanges();

return Created(Product);
}

public IActionResult Put(int key, [FromBody] Product Product)
{
Product.ProductId = key+"";
Product originalProduct = _dbContext.Products.Find(key);

if (originalProduct == null)
{
_dbContext.Products.Add(Product);

return Created(Product);
}

_dbContext.Products.Remove(originalProduct);
_dbContext.Products.Add(Product);
return Ok(Product);
}

public IActionResult Patch(int key, [FromBody] Delta<Product> delta)
{
Product originalProduct = _dbContext.Products.Find(key);

if (originalProduct == null)
{
Product temp = new Product();
delta.Patch(temp);
_dbContext.Products.Add(temp);
return Created(temp);
}

delta.Patch(originalProduct);
return Ok(delta);
}

public IActionResult Delete(int key)
{
Product Product = _dbContext.Products.Find(key);

_dbContext.Products.Remove(Product);
return this.StatusCode(StatusCodes.Status204NoContent);
}

[HttpPost("ResetDataSource")]
public IActionResult ResetDataSource()
{
_dbContext.Orders.RemoveRange(_dbContext.Orders);
_dbContext.Products.RemoveRange(_dbContext.Products);
_dbContext.SaveChanges();

// Add new seed data
_dbContext.Products.AddRange(
new Product
{
ProductId = "1",
Name = "Product1",
Category = "Category1",
ListTestString = new List<string> { "Test1", "Test2", "Test99" },
ListTestBool = new List<bool> { true, false },
ListTestInt = new List<int> { 1, 2, 3 },
ListTestDouble = new List<double> { 1.1, 2.2 },
ListTestFloat = new List<float> { 1.1f, 2.2f },
ListTestDateTime = new List<DateTimeOffset> { DateTime.Now, DateTime.UtcNow },
ListTestUri = new List<Uri> { new Uri("https://example.com") },
ListTestUint = new uint[] { 1, 2, 3 },
ListTestOrder = new List<Order>
{
new Order { OrderId = "Order1" },
new Order { OrderId = "Order2" }
}
},
new Product
{
ProductId = "2",
Name = "Product2",
Category = "Category2",
ListTestString = new List<string> { "Test3", "Test4", "Test99" },
ListTestBool = new List<bool> { false, true },
ListTestInt = new List<int> { 4, 5, 6 },
ListTestDouble = new List<double> { 3.3, 4.4 },
ListTestFloat = new List<float> { 3.3f, 4.4f },
ListTestDateTime = new List<DateTimeOffset> { DateTime.Now.AddDays(1), DateTime.UtcNow.AddDays(1) },
ListTestUri = new List<Uri> { new Uri("https://example.org") },
ListTestUint = new uint[] { 4, 5, 6 }
},
new Product
{
ProductId = "3",
Name = "Product3",
Category = "Category3",
ListTestString = new List<string> { "Test5", "Test6" },
ListTestBool = new List<bool> { true, true },
ListTestInt = new List<int> { 7, 8, 9 },
ListTestDouble = new List<double> { 5.5, 6.6 },
ListTestFloat = new List<float> { 5.5f, 6.6f },
ListTestDateTime = new List<DateTimeOffset> { DateTime.Now.AddDays(2), DateTime.UtcNow.AddDays(2) },
ListTestUri = new List<Uri> { new Uri("https://example.net") },
ListTestUint = new uint[] { 7, 8, 9 }
},
new Product
{
ProductId = "4",
Name = "Product4",
Category = "Category4",
ListTestString = new List<string> { "Test98", "Test98" },
ListTestBool = new List<bool> { false, false },
ListTestInt = new List<int> { 10, 11, 12 },
ListTestDouble = new List<double> { 7.7, 8.8 },
ListTestFloat = new List<float> { 7.7f, 8.8f },
ListTestDateTime = new List<DateTimeOffset> { DateTime.Now.AddDays(3), DateTime.UtcNow.AddDays(3) },
ListTestUri = new List<Uri> { new Uri("https://example.edu") },
ListTestUint = new uint[] { 10, 11, 12 }
},
new Product
{
ProductId = "5",
Name = "Product5",
Category = "Category5",
ListTestString = new List<string> { "Test98", "Test98" },
ListTestBool = new List<bool> { true, false },
ListTestInt = new List<int> { 13, 14, 15 },
ListTestDouble = new List<double> { 9.9, 10.10 },
ListTestFloat = new List<float> { 9.9f, 10.10f },
ListTestDateTime = new List<DateTimeOffset> { DateTime.Now.AddDays(4), DateTime.UtcNow.AddDays(4) },
ListTestUri = new List<Uri> { new Uri("https://example.gov") },
ListTestUint = new uint[] { 13, 14, 15 }
}
);
_dbContext.SaveChanges();
return this.StatusCode(StatusCodes.Status204NoContent);
}


}

}
36 changes: 36 additions & 0 deletions test/Microsoft.AspNetCore.OData.E2E.Tests/Lists/ListsDataModel.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
//-----------------------------------------------------------------------------
// <copyright file="ListsDataModel.cs" company=".NET Foundation">
// Copyright (c) .NET Foundation and Contributors. All rights reserved.
// See License.txt in the project root for license information.
// </copyright>
//------------------------------------------------------------------------------

using System;
using System.Collections.Generic;
using System.ComponentModel.DataAnnotations;
using System.Runtime.Serialization;

namespace Microsoft.AspNetCore.OData.E2E.Tests.Lists
{
public class Product
{
[Key]
public string ProductId { get; set; }
public string Name { get; set; }
public string Category { get; set; }
public IList<string> ListTestString { get; set; } = new List<string>();
public IList<bool> ListTestBool { get; set; }
public IList<int> ListTestInt { get; set; }
public IList<double> ListTestDouble { get; set; }
public IList<float> ListTestFloat { get; set; }
public IList<DateTimeOffset> ListTestDateTime { get; set; }
public IList<Uri> ListTestUri { get; set; }
public uint[] ListTestUint { get; set; }
public IList<Order>? ListTestOrder { get; set; }
}
anasik marked this conversation as resolved.
Show resolved Hide resolved

public class Order
WanjohiSammy marked this conversation as resolved.
Show resolved Hide resolved
{
[Key] public string OrderId { get; set; }
}
}
28 changes: 28 additions & 0 deletions test/Microsoft.AspNetCore.OData.E2E.Tests/Lists/ListsEdmModel.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
//-----------------------------------------------------------------------------
// <copyright file="ListsEdmModel.cs" company=".NET Foundation">
// Copyright (c) .NET Foundation and Contributors. All rights reserved.
// See License.txt in the project root for license information.
// </copyright>
//------------------------------------------------------------------------------

using Microsoft.OData.Edm;
using Microsoft.OData.ModelBuilder;

namespace Microsoft.AspNetCore.OData.E2E.Tests.Lists
{
internal class ListsEdmModel
{
public static IEdmModel GetConventionModel()
{
ODataConventionModelBuilder builder = new ODataConventionModelBuilder();
EntitySetConfiguration<Product> Products = builder.EntitySet<Product>("Products");
EntitySetConfiguration<Order> Orders = builder.EntitySet<Order>("Orders");

builder.Namespace = typeof(Product).Namespace;
builder.Namespace = typeof(Order).Namespace;

var edmModel = builder.GetEdmModel();
return edmModel;
}
}
}
Loading