Skip to content

Commit

Permalink
Fix memory issue with expression constants (#93)
Browse files Browse the repository at this point in the history
* Bump sample project to .NET 8

* Add method to replace constant dynamic values

* Bump version to 3.0.2

* Update github workflows to use .NET 8

---------

Co-authored-by: Nino Schoch <[email protected]>
  • Loading branch information
nino-s and Nino Schoch committed Mar 20, 2024
1 parent 51ab5b7 commit 732d6d9
Show file tree
Hide file tree
Showing 9 changed files with 65 additions and 36 deletions.
4 changes: 2 additions & 2 deletions .github/workflows/main.yml
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ jobs:
strategy:
matrix:
operating-system: [ubuntu-latest]
dotnet-version: ['6.0.x']
dotnet-version: ['8.0.x']

steps:
- uses: actions/checkout@v4
Expand All @@ -26,7 +26,7 @@ jobs:
- name: Setup .NET
uses: actions/setup-dotnet@v4
with:
dotnet-version: '6.0.x'
dotnet-version: '8.0.x'

- name: Build package
run: dotnet build --configuration 'Release'
2 changes: 1 addition & 1 deletion .github/workflows/publish.yml
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ jobs:
- name: Setup .NET
uses: actions/setup-dotnet@v4
with:
dotnet-version: '6.0.x'
dotnet-version: '8.0.x'

- name: Build packages
run: dotnet build --configuration 'Release'
Expand Down
Original file line number Diff line number Diff line change
@@ -1,16 +1,19 @@
<Project Sdk="Microsoft.NET.Sdk.Web">
<Project Sdk="Microsoft.NET.Sdk.Web">

<PropertyGroup>
<TargetFramework>net6.0</TargetFramework>
<TargetFramework>net8.0</TargetFramework>
<PackageLicenseExpression>MIT</PackageLicenseExpression>
<UserSecretsId>3635bc4d-44f6-4d28-930e-d7d3b2ea765e</UserSecretsId>
</PropertyGroup>

<ItemGroup>
<PackageReference Include="AutoMapper" Version="12.0.1" />
<PackageReference Include="AutoMapper.Extensions.Microsoft.DependencyInjection" Version="12.0.1" />
<PackageReference Include="AutoMapper" Version="13.0.1" />
<PackageReference Include="DataTables.NetStandard.TemplateMapper" Version="1.0.0" />
<PackageReference Include="Microsoft.EntityFrameworkCore.SqlServer" Version="7.0.16" />
<PackageReference Include="Microsoft.EntityFrameworkCore.SqlServer.Design" Version="1.1.6" />
<PackageReference Include="Microsoft.EntityFrameworkCore.Design" Version="8.0.3">
<PrivateAssets>all</PrivateAssets>
<IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
</PackageReference>
<PackageReference Include="Microsoft.EntityFrameworkCore.SqlServer" Version="8.0.3" />
</ItemGroup>

<ItemGroup>
Expand Down
6 changes: 3 additions & 3 deletions DataTables.NetStandard.Sample/DataTables/PersonDataTable.cs
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,9 @@ public PersonDataTable(IMapper mapper, SampleDbContext dbContext) : base(mapper)
PrivatePropertyName = nameof(Person.Id),
IsOrderable = true,
IsSearchable = true,
SearchPredicate = (p, s) => false, // The fallback predicate will never match, but since we declared a provider, it is not used anyway.
SearchPredicateProvider = (s) => (p, s) => true, // The provider will return a predicate matching all entities (used for global search). This is just for illustration, it makes no sense.
ColumnSearchPredicateProvider = (s) => // The column provider will return a predicate matching entities in a numeric range if the search term is properly formatted.
SearchPredicate = (p, s) => false, // The fallback predicate will never match, but since we declared a provider, it is not used anyway.
//SearchPredicateProvider = (s) => (p, s) => true, // The provider will return a predicate matching all entities (used for global search). This is just for illustration, it makes no sense.
ColumnSearchPredicateProvider = (s) => // The column provider will return a predicate matching entities in a numeric range if the search term is properly formatted.
{
var minMax = s.Split("-delim-", System.StringSplitOptions.RemoveEmptyEntries);
if (minMax.Length >= 2)
Expand Down
32 changes: 21 additions & 11 deletions DataTables.NetStandard.Sample/Properties/launchSettings.json
Original file line number Diff line number Diff line change
@@ -1,12 +1,4 @@
{
"iisSettings": {
"windowsAuthentication": false,
"anonymousAuthentication": true,
"iisExpress": {
"applicationUrl": "http://localhost:53215",
"sslPort": 44331
}
},
{
"profiles": {
"IIS Express": {
"commandName": "IISExpress",
Expand All @@ -18,10 +10,28 @@
"DataTables.NetStandard.Sample": {
"commandName": "Project",
"launchBrowser": true,
"applicationUrl": "https://localhost:5001;http://localhost:5000",
"environmentVariables": {
"ASPNETCORE_ENVIRONMENT": "Development"
}
},
"applicationUrl": "https://localhost:5003;http://localhost:5002"
},
"WSL": {
"commandName": "WSL2",
"launchBrowser": true,
"launchUrl": "https://localhost:5001",
"environmentVariables": {
"ASPNETCORE_ENVIRONMENT": "Development",
"ASPNETCORE_URLS": "https://localhost:5003;http://localhost:5002"
},
"distributionName": ""
}
},
"iisSettings": {
"windowsAuthentication": false,
"anonymousAuthentication": true,
"iisExpress": {
"applicationUrl": "http://localhost:53215",
"sslPort": 44331
}
}
}
6 changes: 3 additions & 3 deletions DataTables.NetStandard/DataTables.NetStandard.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,9 @@
<TargetFramework>netstandard2.1</TargetFramework>
<LangVersion>latest</LangVersion>
<GeneratePackageOnBuild>true</GeneratePackageOnBuild>
<Version>3.0.1</Version>
<AssemblyVersion>3.0.1.0</AssemblyVersion>
<FileVersion>3.0.1.0</FileVersion>
<Version>3.0.2</Version>
<AssemblyVersion>3.0.2.0</AssemblyVersion>
<FileVersion>3.0.2.0</FileVersion>
<Authors>Namoshek (Marvin Mall)</Authors>
<Company>Namoshek (Marvin Mall)</Company>
<PackageId>DataTables.NetStandard</PackageId>
Expand Down
8 changes: 4 additions & 4 deletions DataTables.NetStandard/DataTablesRequest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -51,13 +51,13 @@ public class DataTablesRequest<TEntity, TEntityViewModel>

/// <summary>
/// Collection of DataTables column info.
/// Each column can be acessed via indexer by corresponding property name or by property selector.
/// Each column can be acessed via indexer by corresponding property name or by property selector.
/// </summary>
/// <example>
/// Example for an entity Student that has public property FirstName.
/// <code>
/// // Get DataTables request from Http query parameters
/// var request = new DataTablesRequest&lt;Student&gt;(url);
/// var request = new DataTablesRequest&lt;Student&gt;(url);
///
/// // Access by property name
/// var column = request.Columns["FirstName"];
Expand All @@ -70,7 +70,7 @@ public class DataTablesRequest<TEntity, TEntityViewModel>
new List<DataTablesColumn<TEntity, TEntityViewModel>>();

/// <summary>
/// Set this property to log incoming request parameters and resulting queries to the given delegate.
/// Set this property to log incoming request parameters and resulting queries to the given delegate.
/// For example, to log to the console, set this property to <see cref="Console.Write(string)"/>.
/// </summary>
public Action<string> Log { get; set; }
Expand Down Expand Up @@ -194,7 +194,7 @@ protected void ParseGlobalConfigurationFromQuery(NameValueCollection query)
{
int start = int.TryParse(query["start"], out start) ? start : 0;
PageSize = int.TryParse(query["length"], out int length) ? length : 15;
PageNumber = start / PageSize + 1;
PageNumber = (start / PageSize) + 1;

Draw = int.TryParse(query["draw"], out int draw) ? draw : 0;

Expand Down
6 changes: 3 additions & 3 deletions DataTables.NetStandard/Extensions/QueryableExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,7 @@ internal static string GetOrderMethodName(ListSortDirection direction, bool alre
{
var expr = searchPredicate;
var entityParam = ExpressionHelper.BuildParameterExpression<TEntity>();
var searchValueConstant = Expression.Constant(globalSearchValue, typeof(string));
var searchValueConstant = ExpressionHelper.CreateConstantFilterExpression(globalSearchValue, typeof(string));
expression = (Expression<Func<TEntity, bool>>)Expression.Lambda(
Expression.Invoke(expr, entityParam, searchValueConstant),
entityParam);
Expand Down Expand Up @@ -233,7 +233,7 @@ internal static string GetOrderMethodName(ListSortDirection direction, bool alre
}

/// <summary>
/// Applies the search filter for each of the searchable <see cref="DataTablesRequest{TEntity, TEntityViewModel}.Columns"/>
/// Applies the search filter for each of the searchable <see cref="DataTablesRequest{TEntity, TEntityViewModel}.Columns"/>
/// where a search value is present.
/// </summary>
/// <typeparam name="TEntity">The type of the entity.</typeparam>
Expand Down Expand Up @@ -272,7 +272,7 @@ internal static string GetOrderMethodName(ListSortDirection direction, bool alre
{
var expr = searchPredicate;
var entityParam = ExpressionHelper.BuildParameterExpression<TEntity>();
var searchValueConstant = Expression.Constant(c.SearchValue, typeof(string));
var searchValueConstant = ExpressionHelper.CreateConstantFilterExpression(c.SearchValue, typeof(string));
expression = (Expression<Func<TEntity, bool>>)Expression.Lambda(
Expression.Invoke(expr, entityParam, searchValueConstant),
entityParam);
Expand Down
22 changes: 19 additions & 3 deletions DataTables.NetStandard/Util/ExpressionHelper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ internal static ParameterExpression BuildParameterExpression<TEntity>()

/// <summary>
/// Builds an <see cref="Expression"/> for the given <paramref name="propertyName"/>.
/// The property name has to be given as dot-separated property path, e.g. <code>Destination.Location.City</code>.
/// The property name has to be given as dot-separated property path, e.g. <c>Destination.Location.City</c>.
/// </summary>
/// <param name="param">The parameter.</param>
/// <param name="propertyName">Name of the property.</param>
Expand Down Expand Up @@ -85,7 +85,7 @@ internal static MemberExpression BuildPropertyExpression(ParameterExpression par
stringConstant = stringConstant.ToLower();
}

var someValue = Expression.Constant(stringConstant, typeof(string));
var someValue = CreateConstantFilterExpression(stringConstant, typeof(string));
var containsMethodExp = Expression.Call(exp, String_Contains, someValue);

var notNullExp = Expression.NotEqual(exp, Expression.Constant(null, typeof(object)));
Expand All @@ -112,12 +112,28 @@ internal static MemberExpression BuildPropertyExpression(ParameterExpression par
exp = Expression.Call(propertyExp, Object_ToString);
}

var regexExp = Expression.Constant(regex, typeof(string));
var regexExp = CreateConstantFilterExpression(regex, typeof(string));
var resultExp = Expression.Call(Regex_IsMatch, exp, regexExp);

var notNullExp = Expression.NotEqual(exp, Expression.Constant(null, typeof(object)));

return Expression.Lambda<Func<TEntity, bool>>(Expression.AndAlso(notNullExp, resultExp), parameterExp);
}

/// <summary>
/// Creates a constant filter expression of the given <paramref name="value"/> and converts the type to the given <paramref name="type"/>.
/// </summary>
/// <param name="value"></param>
/// <param name="type"></param>
internal static Expression CreateConstantFilterExpression(object value, Type type)
{
// The value is converted to anonymous function only returning the value itself.
Expression<Func<object>> valueExpression = () => value;

// Afterwards only the body of the function, which is the value, is converted to the delivered type.
// Therefore no Expression.Constant is necessary which lead to memory leaks, because EFCore caches such constants.
// Caching constants is not wrong, but creating constants of dynamic search values is wrong.
return Expression.Convert(valueExpression.Body, type);
}
}
}

0 comments on commit 732d6d9

Please sign in to comment.