From 0078f90dc5c118a285ada35f262f85d5bfe7be6f Mon Sep 17 00:00:00 2001 From: melflitty Date: Mon, 31 Jul 2023 10:26:43 +0200 Subject: [PATCH 01/11] refactor: remove LinqKit --- Common/src/ArmoniK.Core.Common.csproj | 1 - Common/src/gRPC/ExpressionExt.cs | 29 +++++++++++++++++++ Common/src/gRPC/ListApplicationsRequestExt.cs | 16 +++++----- Common/src/gRPC/ListPartitionsRequestExt.cs | 18 +++++------- Common/src/gRPC/ListResultsRequestExt.cs | 18 +++++------- Common/src/gRPC/ListSessionsRequestExt.cs | 22 +++++++------- Common/src/gRPC/ListTasksRequestExt.cs | 22 +++++++------- Common/tests/Helpers/SimpleTaskTable.cs | 6 ++-- 8 files changed, 77 insertions(+), 55 deletions(-) create mode 100644 Common/src/gRPC/ExpressionExt.cs diff --git a/Common/src/ArmoniK.Core.Common.csproj b/Common/src/ArmoniK.Core.Common.csproj index 2626039ee..8b413265c 100644 --- a/Common/src/ArmoniK.Core.Common.csproj +++ b/Common/src/ArmoniK.Core.Common.csproj @@ -30,7 +30,6 @@ - diff --git a/Common/src/gRPC/ExpressionExt.cs b/Common/src/gRPC/ExpressionExt.cs new file mode 100644 index 000000000..80f0bc388 --- /dev/null +++ b/Common/src/gRPC/ExpressionExt.cs @@ -0,0 +1,29 @@ +using System; +using System.Linq.Expressions; +public static class ExpressionExt +{ + /// + /// Combines two predicate expressions using a logical AND condition + /// + /// The type of the input parameter the predicate expressions are evaluated on + /// The first predicate expression to combine + /// The second predicate expression to combine + /// A new predicate expression that represents the logical AND of the two expressions + public static Expression> ExpressionAnd(this Expression> expr1, + Expression> expr2) + => Expression.Lambda>(Expression.AndAlso(expr1.Body, + expr2.Body), + expr2.Parameters); + /// + /// Combines two predicate expressions using a logical OR condition + /// + /// The type of the input parameter the predicate expressions are evaluated on + /// The first predicate expression to combine + /// The second predicate expression to combine + /// A new predicate expression that represents the logical OR of the two expressions + public static Expression> ExpressionOr(this Expression> expr1, + Expression> expr2) + => Expression.Lambda>(Expression.OrElse(expr1.Body, + expr2.Body), + expr2.Parameters); + } diff --git a/Common/src/gRPC/ListApplicationsRequestExt.cs b/Common/src/gRPC/ListApplicationsRequestExt.cs index 5eff3fda9..08a8323ba 100644 --- a/Common/src/gRPC/ListApplicationsRequestExt.cs +++ b/Common/src/gRPC/ListApplicationsRequestExt.cs @@ -22,8 +22,6 @@ using ArmoniK.Api.gRPC.V1.Applications; using ArmoniK.Core.Common.Storage; -using LinqKit; - namespace ArmoniK.Core.Common.gRPC; public static class ListApplicationsRequestExt @@ -67,22 +65,25 @@ public static class ListApplicationsRequestExt /// the given message is not recognized public static Expression> ToApplicationFilter(this Filters filters) { - var predicate = PredicateBuilder.New(); + Expression> expr = data => false; + if (filters.Or == null || !filters.Or.Any()) { return data => true; } + foreach (var filtersAnd in filters.Or) { - var predicateAnd = PredicateBuilder.New(data => true); + Expression> exprAnd = data => true; + foreach (var filterField in filtersAnd.And) { switch (filterField.ValueConditionCase) { case FilterField.ValueConditionOneofCase.FilterString: - predicateAnd = predicateAnd.And(filterField.FilterString.Operator.ToFilter(filterField.Field.ApplicationField_.Field.ToField(), + exprAnd = exprAnd.ExpressionAnd(filterField.FilterString.Operator.ToFilter(filterField.Field.ApplicationField_.Field.ToField(), filterField.FilterString.Value)); break; case FilterField.ValueConditionOneofCase.None: @@ -91,9 +92,10 @@ public static Expression> ToApplicationFilter(this Filters } } - predicate = predicate.Or(predicateAnd); + expr = expr.ExpressionOr(exprAnd); } - return predicate; + + return expr; } } diff --git a/Common/src/gRPC/ListPartitionsRequestExt.cs b/Common/src/gRPC/ListPartitionsRequestExt.cs index 283aff6c5..fd69ddad3 100644 --- a/Common/src/gRPC/ListPartitionsRequestExt.cs +++ b/Common/src/gRPC/ListPartitionsRequestExt.cs @@ -23,8 +23,6 @@ using ArmoniK.Core.Common.Storage; -using LinqKit; - namespace ArmoniK.Core.Common.gRPC; public static class ListPartitionsRequestExt @@ -68,7 +66,7 @@ public static class ListPartitionsRequestExt /// the given message is not recognized public static Expression> ToPartitionFilter(this Filters filters) { - var predicate = PredicateBuilder.New(); + Expression> expr = data => false; if (filters.Or == null || !filters.Or.Any()) { @@ -77,25 +75,25 @@ public static Expression> ToPartitionFilter(this Filte foreach (var filtersAnd in filters.Or) { - var predicateAnd = PredicateBuilder.New(data => true); + Expression> exprAnd = data => true; foreach (var filterField in filtersAnd.And) { switch (filterField.ValueConditionCase) { case FilterField.ValueConditionOneofCase.FilterString: - predicateAnd = predicateAnd.And(filterField.FilterString.Operator.ToFilter(filterField.Field.ToField(), + exprAnd = exprAnd.ExpressionAnd(filterField.FilterString.Operator.ToFilter(filterField.Field.ToField(), filterField.FilterString.Value)); break; case FilterField.ValueConditionOneofCase.FilterNumber: - predicateAnd = predicateAnd.And(filterField.FilterNumber.Operator.ToFilter(filterField.Field.ToField(), + exprAnd = exprAnd.ExpressionAnd(filterField.FilterNumber.Operator.ToFilter(filterField.Field.ToField(), filterField.FilterNumber.Value)); break; case FilterField.ValueConditionOneofCase.FilterBoolean: - predicateAnd = predicateAnd.And(filterField.FilterBoolean.Operator.ToFilter(filterField.Field.ToField(), + exprAnd = exprAnd.ExpressionAnd(filterField.FilterBoolean.Operator.ToFilter(filterField.Field.ToField(), filterField.FilterBoolean.Value)); break; case FilterField.ValueConditionOneofCase.FilterArray: - predicateAnd = predicateAnd.And(filterField.FilterArray.Operator.ToFilter(filterField.Field.ToField(), + exprAnd = exprAnd.ExpressionAnd(filterField.FilterArray.Operator.ToFilter(filterField.Field.ToField(), filterField.FilterArray.Value)); break; case FilterField.ValueConditionOneofCase.None: @@ -104,9 +102,9 @@ public static Expression> ToPartitionFilter(this Filte } } - predicate = predicate.Or(predicateAnd); + expr = expr.ExpressionOr(exprAnd); } - return predicate; + return expr; } } diff --git a/Common/src/gRPC/ListResultsRequestExt.cs b/Common/src/gRPC/ListResultsRequestExt.cs index 2c88b0979..475ea0425 100644 --- a/Common/src/gRPC/ListResultsRequestExt.cs +++ b/Common/src/gRPC/ListResultsRequestExt.cs @@ -22,8 +22,6 @@ using ArmoniK.Api.gRPC.V1.Results; using ArmoniK.Core.Common.Storage; -using LinqKit; - namespace ArmoniK.Core.Common.gRPC; public static class ListResultsRequestExt @@ -66,7 +64,7 @@ public static class ListResultsRequestExt /// the given message is not recognized public static Expression> ToResultFilter(this Filters filters) { - var predicate = PredicateBuilder.New(); + Expression> expr = data => false; if (filters.Or == null || !filters.Or.Any()) { @@ -75,25 +73,25 @@ public static Expression> ToResultFilter(this Filters filters foreach (var filtersAnd in filters.Or) { - var predicateAnd = PredicateBuilder.New(data => true); + Expression> exprAnd = data => true; foreach (var filterField in filtersAnd.And) { switch (filterField.ValueConditionCase) { case FilterField.ValueConditionOneofCase.FilterString: - predicateAnd = predicateAnd.And(filterField.FilterString.Operator.ToFilter(filterField.Field.ToField(), + exprAnd = exprAnd.ExpressionAnd(filterField.FilterString.Operator.ToFilter(filterField.Field.ToField(), filterField.FilterString.Value)); break; case FilterField.ValueConditionOneofCase.FilterStatus: - predicateAnd = predicateAnd.And(filterField.FilterStatus.Operator.ToFilter(filterField.Field.ToField(), + exprAnd = exprAnd.ExpressionAnd(filterField.FilterStatus.Operator.ToFilter(filterField.Field.ToField(), filterField.FilterStatus.Value)); break; case FilterField.ValueConditionOneofCase.FilterDate: - predicateAnd = predicateAnd.And(filterField.FilterDate.Operator.ToFilter(filterField.Field.ToField(), + exprAnd = exprAnd.ExpressionAnd(filterField.FilterDate.Operator.ToFilter(filterField.Field.ToField(), filterField.FilterDate.Value.ToDateTime())); break; case FilterField.ValueConditionOneofCase.FilterArray: - predicateAnd = predicateAnd.And(filterField.FilterArray.Operator.ToFilter(filterField.Field.ToField(), + exprAnd = exprAnd.ExpressionAnd(filterField.FilterArray.Operator.ToFilter(filterField.Field.ToField(), filterField.FilterArray.Value)); break; case FilterField.ValueConditionOneofCase.None: @@ -102,9 +100,9 @@ public static Expression> ToResultFilter(this Filters filters } } - predicate = predicate.Or(predicateAnd); + expr = expr.ExpressionOr(exprAnd); } - return predicate; + return expr; } } diff --git a/Common/src/gRPC/ListSessionsRequestExt.cs b/Common/src/gRPC/ListSessionsRequestExt.cs index 42e9ef966..ae15ac83f 100644 --- a/Common/src/gRPC/ListSessionsRequestExt.cs +++ b/Common/src/gRPC/ListSessionsRequestExt.cs @@ -22,8 +22,6 @@ using ArmoniK.Api.gRPC.V1.Sessions; using ArmoniK.Core.Common.Storage; -using LinqKit; - namespace ArmoniK.Core.Common.gRPC; public static class ListSessionsRequestExt @@ -72,7 +70,7 @@ public static class ListSessionsRequestExt /// the given message is not recognized public static Expression> ToSessionDataFilter(this Filters filters) { - var predicate = PredicateBuilder.New(); + Expression> expr = data => false; if (filters.Or == null || !filters.Or.Any()) { @@ -81,33 +79,33 @@ public static Expression> ToSessionDataFilter(this Filte foreach (var filtersAnd in filters.Or) { - var predicateAnd = PredicateBuilder.New(data => true); + Expression> exprAnd = data => true; foreach (var filterField in filtersAnd.And) { switch (filterField.ValueConditionCase) { case FilterField.ValueConditionOneofCase.FilterString: - predicateAnd = predicateAnd.And(filterField.FilterString.Operator.ToFilter(filterField.Field.ToField(), + exprAnd = exprAnd.ExpressionAnd(filterField.FilterString.Operator.ToFilter(filterField.Field.ToField(), filterField.FilterString.Value)); break; case FilterField.ValueConditionOneofCase.FilterNumber: - predicateAnd = predicateAnd.And(filterField.FilterNumber.Operator.ToFilter(filterField.Field.ToField(), + exprAnd = exprAnd.ExpressionAnd(filterField.FilterNumber.Operator.ToFilter(filterField.Field.ToField(), filterField.FilterNumber.Value)); break; case FilterField.ValueConditionOneofCase.FilterBoolean: - predicateAnd = predicateAnd.And(filterField.FilterBoolean.Operator.ToFilter(filterField.Field.ToField(), + exprAnd = exprAnd.ExpressionAnd(filterField.FilterBoolean.Operator.ToFilter(filterField.Field.ToField(), filterField.FilterBoolean.Value)); break; case FilterField.ValueConditionOneofCase.FilterStatus: - predicateAnd = predicateAnd.And(filterField.FilterStatus.Operator.ToFilter(filterField.Field.ToField(), + exprAnd = exprAnd.ExpressionAnd(filterField.FilterStatus.Operator.ToFilter(filterField.Field.ToField(), filterField.FilterStatus.Value)); break; case FilterField.ValueConditionOneofCase.FilterDate: - predicateAnd = predicateAnd.And(filterField.FilterDate.Operator.ToFilter(filterField.Field.ToField(), + exprAnd = exprAnd.ExpressionAnd(filterField.FilterDate.Operator.ToFilter(filterField.Field.ToField(), filterField.FilterDate.Value.ToDateTime())); break; case FilterField.ValueConditionOneofCase.FilterArray: - predicateAnd = predicateAnd.And(filterField.FilterArray.Operator.ToFilter(filterField.Field.ToField(), + exprAnd = exprAnd.ExpressionAnd(filterField.FilterArray.Operator.ToFilter(filterField.Field.ToField(), filterField.FilterArray.Value)); break; case FilterField.ValueConditionOneofCase.None: @@ -116,9 +114,9 @@ public static Expression> ToSessionDataFilter(this Filte } } - predicate = predicate.Or(predicateAnd); + expr = expr.ExpressionOr(exprAnd); } - return predicate; + return expr; } } diff --git a/Common/src/gRPC/ListTasksRequestExt.cs b/Common/src/gRPC/ListTasksRequestExt.cs index a6651db3f..d403eb42c 100644 --- a/Common/src/gRPC/ListTasksRequestExt.cs +++ b/Common/src/gRPC/ListTasksRequestExt.cs @@ -24,8 +24,6 @@ using ArmoniK.Api.gRPC.V1.Tasks; using ArmoniK.Core.Common.Storage; -using LinqKit; - namespace ArmoniK.Core.Common.gRPC; public static class ListTasksRequestExt @@ -74,7 +72,7 @@ public static class ListTasksRequestExt /// the given message is not recognized public static Expression> ToTaskDataFilter(this Filters filters) { - var predicate = PredicateBuilder.New(); + Expression> expr = data => false; if (filters.Or == null || !filters.Or.Any()) { @@ -83,36 +81,36 @@ public static Expression> ToTaskDataFilter(this Filters fil foreach (var filtersAnd in filters.Or) { - var predicateAnd = PredicateBuilder.New(data => true); + Expression> exprAnd = data => true; foreach (var filterField in filtersAnd.And) { switch (filterField.ValueConditionCase) { case FilterField.ValueConditionOneofCase.FilterString: - predicateAnd = predicateAnd.And(filterField.FilterString.Operator.ToFilter(filterField.Field.ToField(), + exprAnd = exprAnd.ExpressionAnd(filterField.FilterString.Operator.ToFilter(filterField.Field.ToField(), filterField.FilterString.Value)); break; case FilterField.ValueConditionOneofCase.FilterNumber: - predicateAnd = predicateAnd.And(filterField.FilterNumber.Operator.ToFilter(filterField.Field.ToField(), + exprAnd = exprAnd.ExpressionAnd(filterField.FilterNumber.Operator.ToFilter(filterField.Field.ToField(), filterField.FilterNumber.Value)); break; case FilterField.ValueConditionOneofCase.FilterBoolean: - predicateAnd = predicateAnd.And(filterField.FilterBoolean.Operator.ToFilter(filterField.Field.ToField(), + exprAnd = exprAnd.ExpressionAnd(filterField.FilterBoolean.Operator.ToFilter(filterField.Field.ToField(), filterField.FilterBoolean.Value)); break; case FilterField.ValueConditionOneofCase.FilterStatus: - predicateAnd = predicateAnd.And(filterField.FilterStatus.Operator.ToFilter(filterField.Field.ToField(), + exprAnd = exprAnd.ExpressionAnd(filterField.FilterStatus.Operator.ToFilter(filterField.Field.ToField(), filterField.FilterStatus.Value)); break; case FilterField.ValueConditionOneofCase.FilterDate: var val = filterField.FilterDate.Value; - predicateAnd = predicateAnd.And(filterField.FilterDate.Operator.ToFilter(filterField.Field.ToField(), + exprAnd = exprAnd.ExpressionAnd(filterField.FilterDate.Operator.ToFilter(filterField.Field.ToField(), val == null ? null : val.ToDateTime())); break; case FilterField.ValueConditionOneofCase.FilterArray: - predicateAnd = predicateAnd.And(filterField.FilterArray.Operator.ToFilter(filterField.Field.ToField(), + exprAnd = exprAnd.ExpressionAnd(filterField.FilterArray.Operator.ToFilter(filterField.Field.ToField(), filterField.FilterArray.Value)); break; case FilterField.ValueConditionOneofCase.None: @@ -121,9 +119,9 @@ public static Expression> ToTaskDataFilter(this Filters fil } } - predicate = predicate.Or(predicateAnd); + expr = expr.ExpressionOr(exprAnd); } - return predicate; + return expr; } } diff --git a/Common/tests/Helpers/SimpleTaskTable.cs b/Common/tests/Helpers/SimpleTaskTable.cs index 632d5ccfd..c62514d6a 100644 --- a/Common/tests/Helpers/SimpleTaskTable.cs +++ b/Common/tests/Helpers/SimpleTaskTable.cs @@ -26,8 +26,6 @@ using ArmoniK.Core.Base.DataStructures; using ArmoniK.Core.Common.Storage; -using LinqKit; - using Microsoft.Extensions.Diagnostics.HealthChecks; using Microsoft.Extensions.Logging; @@ -186,7 +184,9 @@ public Task> FindTasksAsync(Expression> f TaskOptions, new Output(true, "")), - }.Select(selector.Invoke)); + }.Where(filter.Compile()) + .Select(selector.Compile())); + public Task UpdateOneTask(string taskId, ICollection<(Expression> selector, object? newValue)> updates, From 347723b9522f0f00bc845ff78dd158c4f2cce78d Mon Sep 17 00:00:00 2001 From: melflitty Date: Mon, 31 Jul 2023 11:08:30 +0200 Subject: [PATCH 02/11] style: reformat --- Common/src/gRPC/ExpressionExt.cs | 29 ++++++++++++++++++++++++----- 1 file changed, 24 insertions(+), 5 deletions(-) diff --git a/Common/src/gRPC/ExpressionExt.cs b/Common/src/gRPC/ExpressionExt.cs index 80f0bc388..e872251b1 100644 --- a/Common/src/gRPC/ExpressionExt.cs +++ b/Common/src/gRPC/ExpressionExt.cs @@ -1,29 +1,48 @@ +// This file is part of the ArmoniK project +// +// Copyright (C) ANEO, 2021-2023. All rights reserved. +// +// This program is free software: you can redistribute it and/or modify +// it under the terms of the GNU Affero General Public License as published +// by the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY, without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU Affero General Public License for more details. +// +// You should have received a copy of the GNU Affero General Public License +// along with this program. If not, see . + using System; using System.Linq.Expressions; + public static class ExpressionExt { /// - /// Combines two predicate expressions using a logical AND condition + /// Combines two predicate expressions using a logical AND condition /// /// The type of the input parameter the predicate expressions are evaluated on /// The first predicate expression to combine /// The second predicate expression to combine /// A new predicate expression that represents the logical AND of the two expressions public static Expression> ExpressionAnd(this Expression> expr1, - Expression> expr2) + Expression> expr2) => Expression.Lambda>(Expression.AndAlso(expr1.Body, expr2.Body), expr2.Parameters); + /// - /// Combines two predicate expressions using a logical OR condition + /// Combines two predicate expressions using a logical OR condition /// /// The type of the input parameter the predicate expressions are evaluated on /// The first predicate expression to combine /// The second predicate expression to combine /// A new predicate expression that represents the logical OR of the two expressions public static Expression> ExpressionOr(this Expression> expr1, - Expression> expr2) + Expression> expr2) => Expression.Lambda>(Expression.OrElse(expr1.Body, expr2.Body), expr2.Parameters); - } +} From 08a1ee512575a87d82d93c8ab0298b3013686433 Mon Sep 17 00:00:00 2001 From: melflitty Date: Mon, 31 Jul 2023 11:22:27 +0200 Subject: [PATCH 03/11] fix: invoke expression1 before combining --- Common/src/gRPC/ExpressionExt.cs | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/Common/src/gRPC/ExpressionExt.cs b/Common/src/gRPC/ExpressionExt.cs index e872251b1..fd1513a1d 100644 --- a/Common/src/gRPC/ExpressionExt.cs +++ b/Common/src/gRPC/ExpressionExt.cs @@ -30,8 +30,9 @@ public static class ExpressionExt public static Expression> ExpressionAnd(this Expression> expr1, Expression> expr2) => Expression.Lambda>(Expression.AndAlso(expr1.Body, - expr2.Body), - expr2.Parameters); + Expression.Invoke(expr2, + expr1.Parameters)), + expr1.Parameters); /// /// Combines two predicate expressions using a logical OR condition @@ -43,6 +44,7 @@ public static Expression> ExpressionAnd(this Expression> ExpressionOr(this Expression> expr1, Expression> expr2) => Expression.Lambda>(Expression.OrElse(expr1.Body, - expr2.Body), - expr2.Parameters); + Expression.Invoke(expr2, + expr1.Parameters)), + expr1.Parameters); } From d8cafd66bbd1981791e7b0bb3c1d92b273b789fa Mon Sep 17 00:00:00 2001 From: melflitty Date: Mon, 31 Jul 2023 11:25:53 +0200 Subject: [PATCH 04/11] fix: add new class to the namespace --- Common/src/gRPC/ExpressionExt.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Common/src/gRPC/ExpressionExt.cs b/Common/src/gRPC/ExpressionExt.cs index fd1513a1d..1f2867d40 100644 --- a/Common/src/gRPC/ExpressionExt.cs +++ b/Common/src/gRPC/ExpressionExt.cs @@ -18,6 +18,8 @@ using System; using System.Linq.Expressions; +namespace ArmoniK.Core.Common.gRPC; + public static class ExpressionExt { /// From b6ae04fc2ff3e131f902769fd436ec7c1a512905 Mon Sep 17 00:00:00 2001 From: melflitty Date: Mon, 31 Jul 2023 13:52:18 +0200 Subject: [PATCH 05/11] fix: use expression visitors --- Common/src/gRPC/ExpressionExt.cs | 56 +++++++++++++++++++++++++++----- 1 file changed, 48 insertions(+), 8 deletions(-) diff --git a/Common/src/gRPC/ExpressionExt.cs b/Common/src/gRPC/ExpressionExt.cs index 1f2867d40..0339512c3 100644 --- a/Common/src/gRPC/ExpressionExt.cs +++ b/Common/src/gRPC/ExpressionExt.cs @@ -31,10 +31,21 @@ public static class ExpressionExt /// A new predicate expression that represents the logical AND of the two expressions public static Expression> ExpressionAnd(this Expression> expr1, Expression> expr2) - => Expression.Lambda>(Expression.AndAlso(expr1.Body, - Expression.Invoke(expr2, - expr1.Parameters)), - expr1.Parameters); + { + var parameter = Expression.Parameter(typeof(T)); + + var leftVisitor = new ReplaceExpressionVisitor(expr1.Parameters[0], + parameter); + var left = leftVisitor.Visit(expr1.Body); + + var rightVisitor = new ReplaceExpressionVisitor(expr2.Parameters[0], + parameter); + var right = rightVisitor.Visit(expr2.Body); + + return Expression.Lambda>(Expression.AndAlso(left, + right), + parameter); + } /// /// Combines two predicate expressions using a logical OR condition @@ -45,8 +56,37 @@ public static Expression> ExpressionAnd(this Expression A new predicate expression that represents the logical OR of the two expressions public static Expression> ExpressionOr(this Expression> expr1, Expression> expr2) - => Expression.Lambda>(Expression.OrElse(expr1.Body, - Expression.Invoke(expr2, - expr1.Parameters)), - expr1.Parameters); + { + var parameter = Expression.Parameter(typeof(T)); + + var leftVisitor = new ReplaceExpressionVisitor(expr1.Parameters[0], + parameter); + var left = leftVisitor.Visit(expr1.Body); + + var rightVisitor = new ReplaceExpressionVisitor(expr2.Parameters[0], + parameter); + var right = rightVisitor.Visit(expr2.Body); + + return Expression.Lambda>(Expression.OrElse(left, + right), + parameter); + } + + private class ReplaceExpressionVisitor : ExpressionVisitor + { + private readonly Expression newValue_; + private readonly Expression oldValue_; + + public ReplaceExpressionVisitor(Expression oldValue, + Expression newValue) + { + oldValue_ = oldValue; + newValue_ = newValue; + } + + public override Expression Visit(Expression node) + => node == oldValue_ + ? newValue_ + : base.Visit(node); + } } From fd9973bd8bbc220622d84e9151dcbd3b03627c0e Mon Sep 17 00:00:00 2001 From: melflitty Date: Mon, 31 Jul 2023 14:23:09 +0200 Subject: [PATCH 06/11] refactor: use make binary --- Common/src/gRPC/ExpressionExt.cs | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/Common/src/gRPC/ExpressionExt.cs b/Common/src/gRPC/ExpressionExt.cs index 0339512c3..c2958e205 100644 --- a/Common/src/gRPC/ExpressionExt.cs +++ b/Common/src/gRPC/ExpressionExt.cs @@ -42,8 +42,9 @@ public static Expression> ExpressionAnd(this Expression>(Expression.AndAlso(left, - right), + return Expression.Lambda>(Expression.MakeBinary(ExpressionType.AndAlso, + left, + right), parameter); } @@ -54,8 +55,8 @@ public static Expression> ExpressionAnd(this Expression The first predicate expression to combine /// The second predicate expression to combine /// A new predicate expression that represents the logical OR of the two expressions - public static Expression> ExpressionOr(this Expression> expr1, - Expression> expr2) + public static Expression>? ExpressionOr(this Expression> expr1, + Expression> expr2) { var parameter = Expression.Parameter(typeof(T)); @@ -67,8 +68,9 @@ public static Expression> ExpressionOr(this Expression>(Expression.OrElse(left, - right), + return Expression.Lambda>(Expression.MakeBinary(ExpressionType.OrElse, + left, + right), parameter); } From f54923ee4674ee2716adc763d88e607118a35fda Mon Sep 17 00:00:00 2001 From: melflitty Date: Mon, 31 Jul 2023 14:28:28 +0200 Subject: [PATCH 07/11] refactor: use a generic function for the two conditions --- Common/src/gRPC/ExpressionExt.cs | 47 ++++++++++++++------------------ 1 file changed, 21 insertions(+), 26 deletions(-) diff --git a/Common/src/gRPC/ExpressionExt.cs b/Common/src/gRPC/ExpressionExt.cs index c2958e205..eef1508f9 100644 --- a/Common/src/gRPC/ExpressionExt.cs +++ b/Common/src/gRPC/ExpressionExt.cs @@ -22,15 +22,9 @@ namespace ArmoniK.Core.Common.gRPC; public static class ExpressionExt { - /// - /// Combines two predicate expressions using a logical AND condition - /// - /// The type of the input parameter the predicate expressions are evaluated on - /// The first predicate expression to combine - /// The second predicate expression to combine - /// A new predicate expression that represents the logical AND of the two expressions - public static Expression> ExpressionAnd(this Expression> expr1, - Expression> expr2) + private static Expression> MakeBinaryExpression(ExpressionType expressionType, + Expression> expr1, + Expression> expr2) { var parameter = Expression.Parameter(typeof(T)); @@ -42,12 +36,26 @@ public static Expression> ExpressionAnd(this Expression>(Expression.MakeBinary(ExpressionType.AndAlso, + return Expression.Lambda>(Expression.MakeBinary(expressionType, left, right), parameter); } + /// + /// Combines two predicate expressions using a logical AND condition + /// + /// The type of the input parameter the predicate expressions are evaluated on + /// The first predicate expression to combine + /// The second predicate expression to combine + /// A new predicate expression that represents the logical AND of the two expressions + public static Expression> ExpressionAnd(this Expression> expr1, + Expression> expr2) + => MakeBinaryExpression(ExpressionType.AndAlso, + expr1, + expr2); + + /// /// Combines two predicate expressions using a logical OR condition /// @@ -57,22 +65,9 @@ public static Expression> ExpressionAnd(this Expression A new predicate expression that represents the logical OR of the two expressions public static Expression>? ExpressionOr(this Expression> expr1, Expression> expr2) - { - var parameter = Expression.Parameter(typeof(T)); - - var leftVisitor = new ReplaceExpressionVisitor(expr1.Parameters[0], - parameter); - var left = leftVisitor.Visit(expr1.Body); - - var rightVisitor = new ReplaceExpressionVisitor(expr2.Parameters[0], - parameter); - var right = rightVisitor.Visit(expr2.Body); - - return Expression.Lambda>(Expression.MakeBinary(ExpressionType.OrElse, - left, - right), - parameter); - } + => MakeBinaryExpression(ExpressionType.OrElse, + expr1, + expr2); private class ReplaceExpressionVisitor : ExpressionVisitor { From a82a0d4117d8f7ec771157ad5d760980b91a4606 Mon Sep 17 00:00:00 2001 From: melflitty Date: Mon, 31 Jul 2023 14:32:37 +0200 Subject: [PATCH 08/11] fic: expressionor should not return null --- Common/src/gRPC/ExpressionExt.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Common/src/gRPC/ExpressionExt.cs b/Common/src/gRPC/ExpressionExt.cs index eef1508f9..c3843e1b5 100644 --- a/Common/src/gRPC/ExpressionExt.cs +++ b/Common/src/gRPC/ExpressionExt.cs @@ -63,8 +63,8 @@ public static Expression> ExpressionAnd(this Expression The first predicate expression to combine /// The second predicate expression to combine /// A new predicate expression that represents the logical OR of the two expressions - public static Expression>? ExpressionOr(this Expression> expr1, - Expression> expr2) + public static Expression> ExpressionOr(this Expression> expr1, + Expression> expr2) => MakeBinaryExpression(ExpressionType.OrElse, expr1, expr2); From 13a3683c917af6e599a4a42ab9af9018074b6a17 Mon Sep 17 00:00:00 2001 From: melflitty Date: Mon, 31 Jul 2023 14:55:01 +0200 Subject: [PATCH 09/11] refactor: handle case where node is null in Visit function --- Common/src/gRPC/ExpressionExt.cs | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/Common/src/gRPC/ExpressionExt.cs b/Common/src/gRPC/ExpressionExt.cs index c3843e1b5..285d2295f 100644 --- a/Common/src/gRPC/ExpressionExt.cs +++ b/Common/src/gRPC/ExpressionExt.cs @@ -81,9 +81,16 @@ public ReplaceExpressionVisitor(Expression oldValue, newValue_ = newValue; } - public override Expression Visit(Expression node) - => node == oldValue_ - ? newValue_ - : base.Visit(node); + public override Expression Visit(Expression? node) + { + if (node == null) + { + throw new ArgumentNullException(nameof(node)); + } + + return node == oldValue_ + ? newValue_ + : base.Visit(node); + } } } From a43f5101238e1af50f0c672385984abf352bb233 Mon Sep 17 00:00:00 2001 From: melflitty Date: Mon, 31 Jul 2023 15:14:59 +0200 Subject: [PATCH 10/11] fix: don't throw exception if node is null --- Common/src/gRPC/ExpressionExt.cs | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-) diff --git a/Common/src/gRPC/ExpressionExt.cs b/Common/src/gRPC/ExpressionExt.cs index 285d2295f..c3843e1b5 100644 --- a/Common/src/gRPC/ExpressionExt.cs +++ b/Common/src/gRPC/ExpressionExt.cs @@ -81,16 +81,9 @@ public ReplaceExpressionVisitor(Expression oldValue, newValue_ = newValue; } - public override Expression Visit(Expression? node) - { - if (node == null) - { - throw new ArgumentNullException(nameof(node)); - } - - return node == oldValue_ - ? newValue_ - : base.Visit(node); - } + public override Expression Visit(Expression node) + => node == oldValue_ + ? newValue_ + : base.Visit(node); } } From 449ed9a1c808f11d3e6bab4ed651d9f170c2940f Mon Sep 17 00:00:00 2001 From: melflitty Date: Mon, 31 Jul 2023 17:42:13 +0200 Subject: [PATCH 11/11] refactor: keep signature of Visit function(nullable parameter and return) --- Common/src/gRPC/ExpressionExt.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Common/src/gRPC/ExpressionExt.cs b/Common/src/gRPC/ExpressionExt.cs index c3843e1b5..13e88af58 100644 --- a/Common/src/gRPC/ExpressionExt.cs +++ b/Common/src/gRPC/ExpressionExt.cs @@ -37,8 +37,8 @@ private static Expression> MakeBinaryExpression(ExpressionType var right = rightVisitor.Visit(expr2.Body); return Expression.Lambda>(Expression.MakeBinary(expressionType, - left, - right), + left!, + right!), parameter); } @@ -81,7 +81,7 @@ public ReplaceExpressionVisitor(Expression oldValue, newValue_ = newValue; } - public override Expression Visit(Expression node) + public override Expression? Visit(Expression? node) => node == oldValue_ ? newValue_ : base.Visit(node);