From 33373bda57e0b81feed8c452510395409559dfe0 Mon Sep 17 00:00:00 2001 From: SpoinkyNL Date: Thu, 24 Sep 2020 00:02:53 +0200 Subject: [PATCH] Conditions - Added null-checks to accessors Conditions - Simplified operators, removing unnecessary expression trees --- .../Operators/EqualsConditionOperator.cs | 5 +- .../Operators/GreaterThanConditionOperator.cs | 5 +- .../GreaterThanOrEqualConditionOperator.cs | 5 +- .../Operators/LessThanConditionOperator.cs | 5 +- .../LessThanOrEqualConditionOperator.cs | 5 +- .../Operators/NotEqualConditionOperator.cs | 5 +- .../Operators/NotNullConditionOperator.cs | 23 +++ .../Operators/NullConditionOperator.cs | 23 +++ .../StringContainsConditionOperator.cs | 22 +-- .../StringEndsWithConditionOperator.cs | 22 +-- .../StringEqualsConditionOperator.cs | 16 +-- .../StringNotContainsConditionOperator.cs | 22 +-- .../StringNotEqualConditionOperator.cs | 16 +-- .../Operators/StringNullConditionOperator.cs | 24 ---- .../StringStartsWithConditionOperator.cs | 22 +-- .../Profile/Conditions/ConditionOperator.cs | 39 +---- .../DataModelConditionListPredicate.cs | 136 +++++++++--------- .../Conditions/DataModelConditionPredicate.cs | 92 +++++------- .../Modes/ConditionalDataBinding.cs | 5 +- .../DataBindings/Modes/DataBindingModifier.cs | 19 +-- .../Registration/ConditionOperatorService.cs | 6 +- .../Utilities/ExpressionUtilities.cs | 37 +++++ .../PluginDataModelExpansion.cs | 1 + .../DataModelConditionPredicateViewModel.cs | 9 +- 24 files changed, 253 insertions(+), 311 deletions(-) create mode 100644 src/Artemis.Core/DefaultTypes/Conditions/Operators/NotNullConditionOperator.cs create mode 100644 src/Artemis.Core/DefaultTypes/Conditions/Operators/NullConditionOperator.cs delete mode 100644 src/Artemis.Core/DefaultTypes/Conditions/Operators/StringNullConditionOperator.cs create mode 100644 src/Artemis.Core/Utilities/ExpressionUtilities.cs diff --git a/src/Artemis.Core/DefaultTypes/Conditions/Operators/EqualsConditionOperator.cs b/src/Artemis.Core/DefaultTypes/Conditions/Operators/EqualsConditionOperator.cs index 9981c4910..602afcd18 100644 --- a/src/Artemis.Core/DefaultTypes/Conditions/Operators/EqualsConditionOperator.cs +++ b/src/Artemis.Core/DefaultTypes/Conditions/Operators/EqualsConditionOperator.cs @@ -1,6 +1,5 @@ using System; using System.Collections.Generic; -using System.Linq.Expressions; namespace Artemis.Core.DefaultTypes { @@ -11,9 +10,9 @@ namespace Artemis.Core.DefaultTypes public override string Description => "Equals"; public override string Icon => "Equal"; - public override BinaryExpression CreateExpression(Expression leftSide, Expression rightSide) + public override bool Evaluate(object a, object b) { - return Expression.Equal(leftSide, rightSide); + return Equals(a, b); } } } \ No newline at end of file diff --git a/src/Artemis.Core/DefaultTypes/Conditions/Operators/GreaterThanConditionOperator.cs b/src/Artemis.Core/DefaultTypes/Conditions/Operators/GreaterThanConditionOperator.cs index 6735fd128..c33b0ab47 100644 --- a/src/Artemis.Core/DefaultTypes/Conditions/Operators/GreaterThanConditionOperator.cs +++ b/src/Artemis.Core/DefaultTypes/Conditions/Operators/GreaterThanConditionOperator.cs @@ -1,6 +1,5 @@ using System; using System.Collections.Generic; -using System.Linq.Expressions; namespace Artemis.Core.DefaultTypes { @@ -11,9 +10,9 @@ namespace Artemis.Core.DefaultTypes public override string Description => "Is greater than"; public override string Icon => "GreaterThan"; - public override BinaryExpression CreateExpression(Expression leftSide, Expression rightSide) + public override bool Evaluate(object a, object b) { - return Expression.GreaterThan(leftSide, rightSide); + return Convert.ToSingle(a) > Convert.ToSingle(b); } } } \ No newline at end of file diff --git a/src/Artemis.Core/DefaultTypes/Conditions/Operators/GreaterThanOrEqualConditionOperator.cs b/src/Artemis.Core/DefaultTypes/Conditions/Operators/GreaterThanOrEqualConditionOperator.cs index 2e950af71..ca09d2fdd 100644 --- a/src/Artemis.Core/DefaultTypes/Conditions/Operators/GreaterThanOrEqualConditionOperator.cs +++ b/src/Artemis.Core/DefaultTypes/Conditions/Operators/GreaterThanOrEqualConditionOperator.cs @@ -1,6 +1,5 @@ using System; using System.Collections.Generic; -using System.Linq.Expressions; namespace Artemis.Core.DefaultTypes { @@ -11,9 +10,9 @@ namespace Artemis.Core.DefaultTypes public override string Description => "Is greater than or equal to"; public override string Icon => "GreaterThanOrEqual"; - public override BinaryExpression CreateExpression(Expression leftSide, Expression rightSide) + public override bool Evaluate(object a, object b) { - return Expression.GreaterThanOrEqual(leftSide, rightSide); + return Convert.ToSingle(a) >= Convert.ToSingle(b); } } } \ No newline at end of file diff --git a/src/Artemis.Core/DefaultTypes/Conditions/Operators/LessThanConditionOperator.cs b/src/Artemis.Core/DefaultTypes/Conditions/Operators/LessThanConditionOperator.cs index 89b47acdd..9a05a487b 100644 --- a/src/Artemis.Core/DefaultTypes/Conditions/Operators/LessThanConditionOperator.cs +++ b/src/Artemis.Core/DefaultTypes/Conditions/Operators/LessThanConditionOperator.cs @@ -1,6 +1,5 @@ using System; using System.Collections.Generic; -using System.Linq.Expressions; namespace Artemis.Core.DefaultTypes { @@ -11,9 +10,9 @@ namespace Artemis.Core.DefaultTypes public override string Description => "Is less than"; public override string Icon => "LessThan"; - public override BinaryExpression CreateExpression(Expression leftSide, Expression rightSide) + public override bool Evaluate(object a, object b) { - return Expression.LessThan(leftSide, rightSide); + return Convert.ToSingle(a) < Convert.ToSingle(b); } } } \ No newline at end of file diff --git a/src/Artemis.Core/DefaultTypes/Conditions/Operators/LessThanOrEqualConditionOperator.cs b/src/Artemis.Core/DefaultTypes/Conditions/Operators/LessThanOrEqualConditionOperator.cs index 93b4ecd99..0cf0c6b17 100644 --- a/src/Artemis.Core/DefaultTypes/Conditions/Operators/LessThanOrEqualConditionOperator.cs +++ b/src/Artemis.Core/DefaultTypes/Conditions/Operators/LessThanOrEqualConditionOperator.cs @@ -1,6 +1,5 @@ using System; using System.Collections.Generic; -using System.Linq.Expressions; namespace Artemis.Core.DefaultTypes { @@ -11,9 +10,9 @@ namespace Artemis.Core.DefaultTypes public override string Description => "Is less than or equal to"; public override string Icon => "LessThanOrEqual"; - public override BinaryExpression CreateExpression(Expression leftSide, Expression rightSide) + public override bool Evaluate(object a, object b) { - return Expression.LessThanOrEqual(leftSide, rightSide); + return Convert.ToSingle(a) <= Convert.ToSingle(b); } } } \ No newline at end of file diff --git a/src/Artemis.Core/DefaultTypes/Conditions/Operators/NotEqualConditionOperator.cs b/src/Artemis.Core/DefaultTypes/Conditions/Operators/NotEqualConditionOperator.cs index b799a2afb..9d264e5c8 100644 --- a/src/Artemis.Core/DefaultTypes/Conditions/Operators/NotEqualConditionOperator.cs +++ b/src/Artemis.Core/DefaultTypes/Conditions/Operators/NotEqualConditionOperator.cs @@ -1,6 +1,5 @@ using System; using System.Collections.Generic; -using System.Linq.Expressions; namespace Artemis.Core.DefaultTypes { @@ -11,9 +10,9 @@ namespace Artemis.Core.DefaultTypes public override string Description => "Does not equal"; public override string Icon => "NotEqualVariant"; - public override BinaryExpression CreateExpression(Expression leftSide, Expression rightSide) + public override bool Evaluate(object a, object b) { - return Expression.NotEqual(leftSide, rightSide); + return !Equals(a, b); } } } \ No newline at end of file diff --git a/src/Artemis.Core/DefaultTypes/Conditions/Operators/NotNullConditionOperator.cs b/src/Artemis.Core/DefaultTypes/Conditions/Operators/NotNullConditionOperator.cs new file mode 100644 index 000000000..c14ebaa77 --- /dev/null +++ b/src/Artemis.Core/DefaultTypes/Conditions/Operators/NotNullConditionOperator.cs @@ -0,0 +1,23 @@ +using System; +using System.Collections.Generic; + +namespace Artemis.Core.DefaultTypes +{ + internal class NotNullConditionOperator : ConditionOperator + { + public NotNullConditionOperator() + { + SupportsRightSide = false; + } + + public override IReadOnlyCollection CompatibleTypes => new List {typeof(object)}; + + public override string Description => "Is not null"; + public override string Icon => "CheckboxMarkedCircleOutline"; + + public override bool Evaluate(object a, object b) + { + return a != null; + } + } +} \ No newline at end of file diff --git a/src/Artemis.Core/DefaultTypes/Conditions/Operators/NullConditionOperator.cs b/src/Artemis.Core/DefaultTypes/Conditions/Operators/NullConditionOperator.cs new file mode 100644 index 000000000..ffd8c0639 --- /dev/null +++ b/src/Artemis.Core/DefaultTypes/Conditions/Operators/NullConditionOperator.cs @@ -0,0 +1,23 @@ +using System; +using System.Collections.Generic; + +namespace Artemis.Core.DefaultTypes +{ + internal class NullConditionOperator : ConditionOperator + { + public NullConditionOperator() + { + SupportsRightSide = false; + } + + public override IReadOnlyCollection CompatibleTypes => new List {typeof(object)}; + + public override string Description => "Is null"; + public override string Icon => "Null"; + + public override bool Evaluate(object a, object b) + { + return a == null; + } + } +} \ No newline at end of file diff --git a/src/Artemis.Core/DefaultTypes/Conditions/Operators/StringContainsConditionOperator.cs b/src/Artemis.Core/DefaultTypes/Conditions/Operators/StringContainsConditionOperator.cs index a16c90e03..23c7e67f0 100644 --- a/src/Artemis.Core/DefaultTypes/Conditions/Operators/StringContainsConditionOperator.cs +++ b/src/Artemis.Core/DefaultTypes/Conditions/Operators/StringContainsConditionOperator.cs @@ -1,33 +1,21 @@ using System; using System.Collections.Generic; -using System.Linq.Expressions; -using System.Reflection; namespace Artemis.Core.DefaultTypes { internal class StringContainsConditionOperator : ConditionOperator { - private readonly MethodInfo _contains; - private readonly MethodInfo _toLower; - - public StringContainsConditionOperator() - { - _toLower = typeof(string).GetMethod("ToLower", new Type[] { }); - _contains = typeof(string).GetMethod("Contains", new[] {typeof(string)}); - } - public override IReadOnlyCollection CompatibleTypes => new List {typeof(string)}; public override string Description => "Contains"; public override string Icon => "Contain"; - public override BinaryExpression CreateExpression(Expression leftSide, Expression rightSide) + public override bool Evaluate(object a, object b) { - var contains = Expression.Equal( - Expression.Call(Expression.Call(leftSide, _toLower), _contains, Expression.Call(rightSide, _toLower)), - Expression.Constant(true) - ); - return AddNullChecks(leftSide, rightSide, contains); + var aString = (string) a; + var bString = (string) b; + + return bString != null && aString != null && aString.Contains(bString, StringComparison.InvariantCultureIgnoreCase); } } } \ No newline at end of file diff --git a/src/Artemis.Core/DefaultTypes/Conditions/Operators/StringEndsWithConditionOperator.cs b/src/Artemis.Core/DefaultTypes/Conditions/Operators/StringEndsWithConditionOperator.cs index a256fbf37..c193c9651 100644 --- a/src/Artemis.Core/DefaultTypes/Conditions/Operators/StringEndsWithConditionOperator.cs +++ b/src/Artemis.Core/DefaultTypes/Conditions/Operators/StringEndsWithConditionOperator.cs @@ -1,33 +1,21 @@ using System; using System.Collections.Generic; -using System.Linq.Expressions; -using System.Reflection; namespace Artemis.Core.DefaultTypes { internal class StringEndsWithConditionOperator : ConditionOperator { - private readonly MethodInfo _endsWith; - private readonly MethodInfo _toLower; - - public StringEndsWithConditionOperator() - { - _toLower = typeof(string).GetMethod("ToLower", new Type[] { }); - _endsWith = typeof(string).GetMethod("EndsWith", new[] {typeof(string)}); - } - public override IReadOnlyCollection CompatibleTypes => new List {typeof(string)}; public override string Description => "Ends with"; public override string Icon => "ContainEnd"; - public override BinaryExpression CreateExpression(Expression leftSide, Expression rightSide) + public override bool Evaluate(object a, object b) { - var endsWith = Expression.Equal( - Expression.Call(Expression.Call(leftSide, _toLower), _endsWith, Expression.Call(rightSide, _toLower)), - Expression.Constant(true) - ); - return AddNullChecks(leftSide, rightSide, endsWith); + var aString = (string) a; + var bString = (string) b; + + return bString != null && aString != null && aString.EndsWith(bString, StringComparison.InvariantCultureIgnoreCase); } } } \ No newline at end of file diff --git a/src/Artemis.Core/DefaultTypes/Conditions/Operators/StringEqualsConditionOperator.cs b/src/Artemis.Core/DefaultTypes/Conditions/Operators/StringEqualsConditionOperator.cs index b6e0b95f4..bea4e56ec 100644 --- a/src/Artemis.Core/DefaultTypes/Conditions/Operators/StringEqualsConditionOperator.cs +++ b/src/Artemis.Core/DefaultTypes/Conditions/Operators/StringEqualsConditionOperator.cs @@ -1,27 +1,21 @@ using System; using System.Collections.Generic; -using System.Linq.Expressions; -using System.Reflection; namespace Artemis.Core.DefaultTypes { internal class StringEqualsConditionOperator : ConditionOperator { - private readonly MethodInfo _toLower; - - public StringEqualsConditionOperator() - { - _toLower = typeof(string).GetMethod("ToLower", new Type[] { }); - } - public override IReadOnlyCollection CompatibleTypes => new List {typeof(string)}; public override string Description => "Equals"; public override string Icon => "Equal"; - public override BinaryExpression CreateExpression(Expression leftSide, Expression rightSide) + public override bool Evaluate(object a, object b) { - return Expression.Equal(Expression.Call(leftSide, _toLower), Expression.Call(rightSide, _toLower)); + var aString = (string) a; + var bString = (string) b; + + return string.Equals(aString, bString, StringComparison.InvariantCultureIgnoreCase); } } } \ No newline at end of file diff --git a/src/Artemis.Core/DefaultTypes/Conditions/Operators/StringNotContainsConditionOperator.cs b/src/Artemis.Core/DefaultTypes/Conditions/Operators/StringNotContainsConditionOperator.cs index 0ee241bd7..00337a44b 100644 --- a/src/Artemis.Core/DefaultTypes/Conditions/Operators/StringNotContainsConditionOperator.cs +++ b/src/Artemis.Core/DefaultTypes/Conditions/Operators/StringNotContainsConditionOperator.cs @@ -1,33 +1,21 @@ using System; using System.Collections.Generic; -using System.Linq.Expressions; -using System.Reflection; namespace Artemis.Core.DefaultTypes { internal class StringNotContainsConditionOperator : ConditionOperator { - private readonly MethodInfo _contains; - private readonly MethodInfo _toLower; - - public StringNotContainsConditionOperator() - { - _toLower = typeof(string).GetMethod("ToLower", new Type[] { }); - _contains = typeof(string).GetMethod("Contains", new[] {typeof(string)}); - } - public override IReadOnlyCollection CompatibleTypes => new List {typeof(string)}; public override string Description => "Does not contain"; public override string Icon => "FormatStrikethrough"; - public override BinaryExpression CreateExpression(Expression leftSide, Expression rightSide) + public override bool Evaluate(object a, object b) { - var notContains = Expression.Equal( - Expression.Call(Expression.Call(leftSide, _toLower), _contains, Expression.Call(rightSide, _toLower)), - Expression.Constant(false) - ); - return AddNullChecks(leftSide, rightSide, notContains); + var aString = (string) a; + var bString = (string) b; + + return bString != null && aString != null && !aString.Contains(bString, StringComparison.InvariantCultureIgnoreCase); } } } \ No newline at end of file diff --git a/src/Artemis.Core/DefaultTypes/Conditions/Operators/StringNotEqualConditionOperator.cs b/src/Artemis.Core/DefaultTypes/Conditions/Operators/StringNotEqualConditionOperator.cs index 4a1421967..2ce7a1a64 100644 --- a/src/Artemis.Core/DefaultTypes/Conditions/Operators/StringNotEqualConditionOperator.cs +++ b/src/Artemis.Core/DefaultTypes/Conditions/Operators/StringNotEqualConditionOperator.cs @@ -1,27 +1,21 @@ using System; using System.Collections.Generic; -using System.Linq.Expressions; -using System.Reflection; namespace Artemis.Core.DefaultTypes { internal class StringNotEqualConditionOperator : ConditionOperator { - private readonly MethodInfo _toLower; - - public StringNotEqualConditionOperator() - { - _toLower = typeof(string).GetMethod("ToLower", new Type[] { }); - } - public override IReadOnlyCollection CompatibleTypes => new List {typeof(string)}; public override string Description => "Does not equal"; public override string Icon => "NotEqualVariant"; - public override BinaryExpression CreateExpression(Expression leftSide, Expression rightSide) + public override bool Evaluate(object a, object b) { - return Expression.NotEqual(Expression.Call(leftSide, _toLower), Expression.Call(rightSide, _toLower)); + var aString = (string) a; + var bString = (string) b; + + return !string.Equals(aString, bString, StringComparison.InvariantCultureIgnoreCase); } } } \ No newline at end of file diff --git a/src/Artemis.Core/DefaultTypes/Conditions/Operators/StringNullConditionOperator.cs b/src/Artemis.Core/DefaultTypes/Conditions/Operators/StringNullConditionOperator.cs deleted file mode 100644 index 5ab353440..000000000 --- a/src/Artemis.Core/DefaultTypes/Conditions/Operators/StringNullConditionOperator.cs +++ /dev/null @@ -1,24 +0,0 @@ -using System; -using System.Collections.Generic; -using System.Linq.Expressions; - -namespace Artemis.Core.DefaultTypes -{ - internal class StringNullConditionOperator : ConditionOperator - { - public StringNullConditionOperator() - { - SupportsRightSide = false; - } - - public override IReadOnlyCollection CompatibleTypes => new List {typeof(string)}; - - public override string Description => "Is null"; - public override string Icon => "Null"; - - public override BinaryExpression CreateExpression(Expression leftSide, Expression rightSide) - { - return Expression.Equal(leftSide, Expression.Constant(null, leftSide.Type)); - } - } -} \ No newline at end of file diff --git a/src/Artemis.Core/DefaultTypes/Conditions/Operators/StringStartsWithConditionOperator.cs b/src/Artemis.Core/DefaultTypes/Conditions/Operators/StringStartsWithConditionOperator.cs index 7ae5bc06a..ebe63bee8 100644 --- a/src/Artemis.Core/DefaultTypes/Conditions/Operators/StringStartsWithConditionOperator.cs +++ b/src/Artemis.Core/DefaultTypes/Conditions/Operators/StringStartsWithConditionOperator.cs @@ -1,33 +1,21 @@ using System; using System.Collections.Generic; -using System.Linq.Expressions; -using System.Reflection; namespace Artemis.Core.DefaultTypes { internal class StringStartsWithConditionOperator : ConditionOperator { - private readonly MethodInfo _startsWith; - private readonly MethodInfo _toLower; - - public StringStartsWithConditionOperator() - { - _toLower = typeof(string).GetMethod("ToLower", new Type[] { }); - _startsWith = typeof(string).GetMethod("StartsWith", new[] {typeof(string)}); - } - public override IReadOnlyCollection CompatibleTypes => new List {typeof(string)}; public override string Description => "Starts with"; public override string Icon => "ContainStart"; - public override BinaryExpression CreateExpression(Expression leftSide, Expression rightSide) + public override bool Evaluate(object a, object b) { - var startsWith = Expression.Equal( - Expression.Call(Expression.Call(leftSide, _toLower), _startsWith, Expression.Call(rightSide, _toLower)), - Expression.Constant(true) - ); - return AddNullChecks(leftSide, rightSide, startsWith); + var aString = (string) a; + var bString = (string) b; + + return bString != null && aString != null && aString.StartsWith(bString, StringComparison.InvariantCultureIgnoreCase); } } } \ No newline at end of file diff --git a/src/Artemis.Core/Models/Profile/Conditions/ConditionOperator.cs b/src/Artemis.Core/Models/Profile/Conditions/ConditionOperator.cs index 5a07ca4e5..662902ee2 100644 --- a/src/Artemis.Core/Models/Profile/Conditions/ConditionOperator.cs +++ b/src/Artemis.Core/Models/Profile/Conditions/ConditionOperator.cs @@ -1,7 +1,6 @@ using System; using System.Collections.Generic; using System.Linq; -using System.Linq.Expressions; namespace Artemis.Core { @@ -47,40 +46,10 @@ namespace Artemis.Core } /// - /// Creates a binary expression comparing two types + /// Evaluates the operator on a and b /// - /// The parameter on the left side of the expression - /// The parameter on the right side of the expression - /// - public abstract BinaryExpression CreateExpression(Expression leftSide, Expression rightSide); - - /// - /// Wraps the provided expression in null-checks for the left and right side - /// - /// The resulting expression body looks like: - /// (a == null && b == null) || ((a != null && b != null) && <expression>) - /// - /// - /// The left side to check for nulls - /// The right side to check for nulls - /// The expression to wrap - /// The wrapped expression - protected BinaryExpression AddNullChecks(Expression leftSide, Expression rightSide, BinaryExpression expression) - { - var nullConst = Expression.Constant(null); - return Expression.OrElse( - Expression.AndAlso( - Expression.Equal(leftSide, nullConst), - Expression.Equal(rightSide, nullConst) - ), - Expression.AndAlso( - Expression.AndAlso( - Expression.NotEqual(leftSide, nullConst), - Expression.NotEqual(rightSide, nullConst) - ), - expression - ) - ); - } + /// The parameter on the left side of the expression + /// The parameter on the right side of the expression + public abstract bool Evaluate(object a, object b); } } \ No newline at end of file diff --git a/src/Artemis.Core/Models/Profile/Conditions/DataModelConditionListPredicate.cs b/src/Artemis.Core/Models/Profile/Conditions/DataModelConditionListPredicate.cs index db9b7778a..b68d18cbe 100644 --- a/src/Artemis.Core/Models/Profile/Conditions/DataModelConditionListPredicate.cs +++ b/src/Artemis.Core/Models/Profile/Conditions/DataModelConditionListPredicate.cs @@ -77,15 +77,8 @@ namespace Artemis.Core /// public object RightStaticValue { get; private set; } - /// - /// Gets the compiled expression that evaluates this predicate - /// - public Func CompiledListPredicate { get; private set; } - - /// - /// Gets the compiled expression that evaluates this predicate on an external right-side data model - /// - public Func CompiledExternalListPredicate { get; set; } + public Func LeftSideAccessor { get; set; } + public Func RightSideAccessor { get; set; } /// /// Updates the left side of the predicate @@ -215,28 +208,26 @@ namespace Artemis.Core return true; } - - #region IDisposable - - /// - protected override void Dispose(bool disposing) - { - ConditionOperatorStore.ConditionOperatorAdded -= ConditionOperatorStoreOnConditionOperatorAdded; - ConditionOperatorStore.ConditionOperatorRemoved -= ConditionOperatorStoreOnConditionOperatorRemoved; - - base.Dispose(disposing); - } - - #endregion - + internal override bool EvaluateObject(object target) { - if (PredicateType == ListRightSideType.Static && CompiledListPredicate != null) - return CompiledListPredicate(target); - if (PredicateType == ListRightSideType.DynamicList && CompiledListPredicate != null) - return CompiledListPredicate(target); - if (PredicateType == ListRightSideType.Dynamic && CompiledExternalListPredicate != null) - return CompiledExternalListPredicate(target, RightDataModel); + if (Operator == null || LeftSideAccessor == null || PredicateType != ListRightSideType.Static && RightSideAccessor == null) + return false; + + // Compare with a static value + if (PredicateType == ListRightSideType.Static) + { + if (!DataModelConditionList.ListType.IsValueType && RightStaticValue == null) + return false; + + return Operator.Evaluate(LeftSideAccessor(target), RightStaticValue); + } + + // Compare with dynamic values + if (PredicateType == ListRightSideType.Dynamic) + return Operator.Evaluate(LeftSideAccessor(target), RightSideAccessor(RightDataModel)); + if (PredicateType == ListRightSideType.DynamicList) + return Operator.Evaluate(LeftSideAccessor(target), RightSideAccessor(target)); return false; } @@ -308,6 +299,8 @@ namespace Artemis.Core private void Initialize() { + DataModelStore.DataModelAdded += DataModelStoreOnDataModelAdded; + DataModelStore.DataModelRemoved += DataModelStoreOnDataModelRemoved; ConditionOperatorStore.ConditionOperatorAdded += ConditionOperatorStoreOnConditionOperatorAdded; ConditionOperatorStore.ConditionOperatorRemoved += ConditionOperatorStoreOnConditionOperatorRemoved; @@ -375,18 +368,16 @@ namespace Artemis.Core private void CreateExpression() { - CompiledListPredicate = null; - if (Operator == null) return; // If the operator does not support a right side, create a static expression because the right side will simply be null if (PredicateType == ListRightSideType.DynamicList && Operator.SupportsRightSide) - CreateDynamicListExpression(); + CreateDynamicListAccessors(); else if (PredicateType == ListRightSideType.Dynamic && Operator.SupportsRightSide) - CreateDynamicExpression(); + CreateDynamicAccessors(); else - CreateStaticExpression(); + CreateStaticAccessors(); } private void ValidateOperator() @@ -452,7 +443,7 @@ namespace Artemis.Core RightStaticValue = null; } - private void CreateDynamicListExpression() + private void CreateDynamicListAccessors() { if (LeftPropertyPath == null || RightPropertyPath == null || Operator == null) return; @@ -467,12 +458,11 @@ namespace Artemis.Core if (rightSideAccessor.Type != leftSideAccessor.Type) rightSideAccessor = Expression.Convert(rightSideAccessor, leftSideAccessor.Type); - var conditionExpression = Operator.CreateExpression(leftSideAccessor, rightSideAccessor); - var lambda = Expression.Lambda>(conditionExpression, leftSideParameter); - CompiledListPredicate = lambda.Compile(); + LeftSideAccessor = Expression.Lambda>(leftSideAccessor, leftSideParameter).Compile(); + RightSideAccessor = Expression.Lambda>(rightSideAccessor, leftSideParameter).Compile(); } - private void CreateDynamicExpression() + private void CreateDynamicAccessors() { if (LeftPropertyPath == null || RightPropertyPath == null || RightDataModel == null || Operator == null) return; @@ -480,19 +470,18 @@ namespace Artemis.Core // List accessors share the same parameter because a list always contains one item per entry var leftSideParameter = Expression.Parameter(typeof(object), "listItem"); var leftSideAccessor = CreateListAccessor(LeftPropertyPath, leftSideParameter); - var rightSideAccessor = CreateAccessor(RightDataModel, RightPropertyPath, "right", out var rightSideParameter); + var rightSideAccessor = ExpressionUtilities.CreateDataModelAccessor(RightDataModel, RightPropertyPath, "right", out var rightSideParameter); // A conversion may be required if the types differ // This can cause issues if the DataModelConditionOperator wasn't accurate in it's supported types but that is not a concern here if (rightSideAccessor.Type != leftSideAccessor.Type) rightSideAccessor = Expression.Convert(rightSideAccessor, leftSideAccessor.Type); - var conditionExpression = Operator.CreateExpression(leftSideAccessor, rightSideAccessor); - var lambda = Expression.Lambda>(conditionExpression, leftSideParameter, rightSideParameter); - CompiledExternalListPredicate = lambda.Compile(); + LeftSideAccessor = Expression.Lambda>(leftSideAccessor, leftSideParameter).Compile(); + RightSideAccessor = Expression.Lambda>(rightSideAccessor, rightSideParameter).Compile(); } - private void CreateStaticExpression() + private void CreateStaticAccessors() { if (!DataModelConditionList.IsPrimitiveList && LeftPropertyPath == null || Operator == null) return; @@ -503,43 +492,51 @@ namespace Artemis.Core ? Expression.Convert(leftSideParameter, DataModelConditionList.ListType) : CreateListAccessor(LeftPropertyPath, leftSideParameter); - // If the left side is a value type but the input is empty, this isn't a valid expression - if (leftSideAccessor.Type.IsValueType && RightStaticValue == null) - return; - - // If the right side value is null, the constant type cannot be inferred and must be provided manually - var rightSideConstant = RightStaticValue != null - ? Expression.Constant(Convert.ChangeType(RightStaticValue, leftSideAccessor.Type)) - : Expression.Constant(null, leftSideAccessor.Type); - - var conditionExpression = Operator.CreateExpression(leftSideAccessor, rightSideConstant); - var lambda = Expression.Lambda>(conditionExpression, leftSideParameter); - CompiledListPredicate = lambda.Compile(); + LeftSideAccessor = Expression.Lambda>(leftSideAccessor, leftSideParameter).Compile(); + RightSideAccessor = null; } private Expression CreateListAccessor(string path, ParameterExpression listParameter) { - return path.Split('.').Aggregate( - Expression.Convert(listParameter, DataModelConditionList.ListType), // Cast to the appropriate type - Expression.Property - ); + // Create an expression that checks every part of the path for null + // In the same iteration, create the accessor + Expression source = Expression.Convert(listParameter, DataModelConditionList.ListType); + return ExpressionUtilities.CreateNullCheckedAccessor(source, path); } - private Expression CreateAccessor(DataModel dataModel, string path, string parameterName, out ParameterExpression parameter) + #region IDisposable + + /// + protected override void Dispose(bool disposing) { - var listType = dataModel.GetListTypeInPath(path); - if (listType != null) - throw new ArtemisCoreException($"Cannot create a regular accessor at path {path} because the path contains a list"); + DataModelStore.DataModelAdded -= DataModelStoreOnDataModelAdded; + DataModelStore.DataModelRemoved -= DataModelStoreOnDataModelRemoved; + ConditionOperatorStore.ConditionOperatorAdded -= ConditionOperatorStoreOnConditionOperatorAdded; + ConditionOperatorStore.ConditionOperatorRemoved -= ConditionOperatorStoreOnConditionOperatorRemoved; - parameter = Expression.Parameter(typeof(object), parameterName + "DataModel"); - return path.Split('.').Aggregate( - Expression.Convert(parameter, dataModel.GetType()), // Cast to the appropriate type - Expression.Property - ); + base.Dispose(disposing); } + #endregion + #region Event handlers + private void DataModelStoreOnDataModelAdded(object sender, DataModelStoreEvent e) + { + var dataModel = e.Registration.DataModel; + if (dataModel.PluginInfo.Guid == Entity.RightDataModelGuid && dataModel.ContainsPath(Entity.RightPropertyPath)) + UpdateRightSideDynamic(dataModel, Entity.RightPropertyPath); + } + + private void DataModelStoreOnDataModelRemoved(object sender, DataModelStoreEvent e) + { + if (RightDataModel == e.Registration.DataModel) + { + RightSideAccessor = null; + RightDataModel = null; + } + } + private void ConditionOperatorStoreOnConditionOperatorAdded(object sender, ConditionOperatorStoreEvent e) { var conditionOperator = e.Registration.ConditionOperator; @@ -552,7 +549,6 @@ namespace Artemis.Core if (e.Registration.ConditionOperator != Operator) return; Operator = null; - CompiledListPredicate = null; } #endregion diff --git a/src/Artemis.Core/Models/Profile/Conditions/DataModelConditionPredicate.cs b/src/Artemis.Core/Models/Profile/Conditions/DataModelConditionPredicate.cs index 6a63cd13f..246b54f58 100644 --- a/src/Artemis.Core/Models/Profile/Conditions/DataModelConditionPredicate.cs +++ b/src/Artemis.Core/Models/Profile/Conditions/DataModelConditionPredicate.cs @@ -73,15 +73,8 @@ namespace Artemis.Core /// public object RightStaticValue { get; private set; } - /// - /// Gets the compiled function that evaluates this predicate if it of a dynamic - /// - public Func CompiledDynamicPredicate { get; private set; } - - /// - /// Gets the compiled function that evaluates this predicate if it is of a static - /// - public Func CompiledStaticPredicate { get; private set; } + public Func LeftSideAccessor { get; set; } + public Func RightSideAccessor { get; set; } internal DataModelConditionPredicateEntity Entity { get; set; } @@ -109,7 +102,7 @@ namespace Artemis.Core ValidateOperator(); ValidateRightSide(); - CreateExpression(); + CreateAccessors(); } /// @@ -134,7 +127,7 @@ namespace Artemis.Core RightDataModel = dataModel; RightPropertyPath = path; - CreateExpression(); + CreateAccessors(); } /// @@ -167,7 +160,7 @@ namespace Artemis.Core else RightStaticValue = null; - CreateExpression(); + CreateAccessors(); } /// @@ -180,7 +173,7 @@ namespace Artemis.Core if (conditionOperator == null) { Operator = null; - CreateExpression(); + CreateAccessors(); return; } @@ -199,16 +192,28 @@ namespace Artemis.Core } Operator = conditionOperator; - CreateExpression(); + CreateAccessors(); } /// public override bool Evaluate() { - if (CompiledDynamicPredicate != null) - return CompiledDynamicPredicate(LeftDataModel, RightDataModel); - if (CompiledStaticPredicate != null) - return CompiledStaticPredicate(LeftDataModel); + if (Operator == null || LeftSideAccessor == null || PredicateType != ProfileRightSideType.Static && RightSideAccessor == null) + return false; + + // Compare with a static value + if (PredicateType == ProfileRightSideType.Static) + { + var leftSideValue = LeftSideAccessor(LeftDataModel); + if (leftSideValue.GetType().IsValueType && RightStaticValue == null) + return false; + + return Operator.Evaluate(leftSideValue, RightStaticValue); + } + + // Compare with dynamic values + if (PredicateType == ProfileRightSideType.Dynamic) + return Operator.Evaluate(LeftSideAccessor(LeftDataModel), RightSideAccessor(RightDataModel)); return false; } @@ -314,17 +319,14 @@ namespace Artemis.Core return Entity; } - private void CreateExpression() + private void CreateAccessors() { - CompiledDynamicPredicate = null; - CompiledStaticPredicate = null; - if (Operator == null) return; // If the operator does not support a right side, create a static expression because the right side will simply be null if (PredicateType == ProfileRightSideType.Dynamic && Operator.SupportsRightSide) - CreateDynamicExpression(); + CreateDynamicAccessors(); else CreateStaticExpression(); } @@ -360,22 +362,21 @@ namespace Artemis.Core } } - private void CreateDynamicExpression() + private void CreateDynamicAccessors() { if (LeftDataModel == null || RightDataModel == null || Operator == null) return; - var leftSideAccessor = CreateAccessor(LeftDataModel, LeftPropertyPath, "left", out var leftSideParameter); - var rightSideAccessor = CreateAccessor(RightDataModel, RightPropertyPath, "right", out var rightSideParameter); + var leftSideAccessor = ExpressionUtilities.CreateDataModelAccessor(LeftDataModel, LeftPropertyPath, "left", out var leftSideParameter); + var rightSideAccessor = ExpressionUtilities.CreateDataModelAccessor(RightDataModel, RightPropertyPath, "right", out var rightSideParameter); // A conversion may be required if the types differ // This can cause issues if the DataModelConditionOperator wasn't accurate in it's supported types but that is not a concern here if (rightSideAccessor.Type != leftSideAccessor.Type) rightSideAccessor = Expression.Convert(rightSideAccessor, leftSideAccessor.Type); - var conditionExpression = Operator.CreateExpression(leftSideAccessor, rightSideAccessor); - var lambda = Expression.Lambda>(conditionExpression, leftSideParameter, rightSideParameter); - CompiledDynamicPredicate = lambda.Compile(); + LeftSideAccessor = Expression.Lambda>(leftSideAccessor, leftSideParameter).Compile(); + RightSideAccessor = Expression.Lambda>(rightSideAccessor, rightSideParameter).Compile(); } private void CreateStaticExpression() @@ -383,34 +384,19 @@ namespace Artemis.Core if (LeftDataModel == null || Operator == null) return; - var leftSideAccessor = CreateAccessor(LeftDataModel, LeftPropertyPath, "left", out var leftSideParameter); + var leftSideAccessor = Expression.Convert( + ExpressionUtilities.CreateDataModelAccessor(LeftDataModel, LeftPropertyPath, "left", out var leftSideParameter), + typeof(object) + ); // If the left side is a value type but the input is empty, this isn't a valid expression if (leftSideAccessor.Type.IsValueType && RightStaticValue == null) return; - // If the right side value is null, the constant type cannot be inferred and must be provided manually - var rightSideConstant = RightStaticValue != null - ? Expression.Constant(Convert.ChangeType(RightStaticValue, leftSideAccessor.Type)) - : Expression.Constant(null, leftSideAccessor.Type); - - var conditionExpression = Operator.CreateExpression(leftSideAccessor, rightSideConstant); - var lambda = Expression.Lambda>(conditionExpression, leftSideParameter); - CompiledStaticPredicate = lambda.Compile(); + LeftSideAccessor = Expression.Lambda>(leftSideAccessor, leftSideParameter).Compile(); + RightSideAccessor = null; } - private Expression CreateAccessor(DataModel dataModel, string path, string parameterName, out ParameterExpression parameter) - { - var listType = dataModel.GetListTypeInPath(path); - if (listType != null) - throw new ArtemisCoreException($"Cannot create a regular accessor at path {path} because the path contains a list"); - - parameter = Expression.Parameter(typeof(object), parameterName + "DataModel"); - return path.Split('.').Aggregate( - Expression.Convert(parameter, dataModel.GetType()), // Cast to the appropriate type - Expression.Property - ); - } #region Event handlers @@ -427,13 +413,13 @@ namespace Artemis.Core { if (LeftDataModel == e.Registration.DataModel) { - CompiledDynamicPredicate = null; + LeftSideAccessor = null; LeftDataModel = null; } if (RightDataModel == e.Registration.DataModel) { - CompiledDynamicPredicate = null; + RightSideAccessor = null; RightDataModel = null; } } @@ -451,8 +437,6 @@ namespace Artemis.Core return; Operator = null; - CompiledStaticPredicate = null; - CompiledDynamicPredicate = null; } #endregion diff --git a/src/Artemis.Core/Models/Profile/DataBindings/Modes/ConditionalDataBinding.cs b/src/Artemis.Core/Models/Profile/DataBindings/Modes/ConditionalDataBinding.cs index 09dca0393..c1955ec8e 100644 --- a/src/Artemis.Core/Models/Profile/DataBindings/Modes/ConditionalDataBinding.cs +++ b/src/Artemis.Core/Models/Profile/DataBindings/Modes/ConditionalDataBinding.cs @@ -37,6 +37,8 @@ namespace Artemis.Core throw new ObjectDisposedException("ConditionalDataBinding"); var condition = Conditions.FirstOrDefault(c => c.Evaluate()); + if (condition != null) + Console.WriteLine(); return condition == null ? baseValue : condition.Value; } @@ -108,8 +110,7 @@ namespace Artemis.Core } #endregion - - + #region Storage /// diff --git a/src/Artemis.Core/Models/Profile/DataBindings/Modes/DataBindingModifier.cs b/src/Artemis.Core/Models/Profile/DataBindings/Modes/DataBindingModifier.cs index 52d64cd0d..5b9c1f1f5 100644 --- a/src/Artemis.Core/Models/Profile/DataBindings/Modes/DataBindingModifier.cs +++ b/src/Artemis.Core/Models/Profile/DataBindings/Modes/DataBindingModifier.cs @@ -300,25 +300,14 @@ namespace Artemis.Core return; // If the right side value is null, the constant type cannot be inferred and must be provided based on the data binding target - var parameterAccessor = CreateAccessor(ParameterDataModel, ParameterPropertyPath, "parameter", out var rightSideParameter); + var parameterAccessor = ExpressionUtilities.CreateDataModelAccessor( + ParameterDataModel, ParameterPropertyPath, "parameter", out var rightSideParameter + ); var lambda = Expression.Lambda>(Expression.Convert(parameterAccessor, typeof(object)), rightSideParameter); CompiledParameterAccessor = lambda.Compile(); } } - - private Expression CreateAccessor(DataModel dataModel, string path, string parameterName, out ParameterExpression parameter) - { - var listType = dataModel.GetListTypeInPath(path); - if (listType != null) - throw new ArtemisCoreException($"Cannot create a regular accessor at path {path} because the path contains a list"); - - parameter = Expression.Parameter(typeof(object), parameterName + "DataModel"); - return path.Split('.').Aggregate( - Expression.Convert(parameter, dataModel.GetType()), // Cast to the appropriate type - Expression.Property - ); - } - + #region Event handlers private void DataBindingModifierTypeStoreOnDataBindingModifierAdded(object sender, DataBindingModifierTypeStoreEvent e) diff --git a/src/Artemis.Core/Services/Registration/ConditionOperatorService.cs b/src/Artemis.Core/Services/Registration/ConditionOperatorService.cs index bfae42d34..bf99c5e1c 100644 --- a/src/Artemis.Core/Services/Registration/ConditionOperatorService.cs +++ b/src/Artemis.Core/Services/Registration/ConditionOperatorService.cs @@ -59,7 +59,11 @@ namespace Artemis.Core.Services RegisterConditionOperator(Constants.CorePluginInfo, new StringNotContainsConditionOperator()); RegisterConditionOperator(Constants.CorePluginInfo, new StringStartsWithConditionOperator()); RegisterConditionOperator(Constants.CorePluginInfo, new StringEndsWithConditionOperator()); - RegisterConditionOperator(Constants.CorePluginInfo, new StringNullConditionOperator()); + + // Null checks, at the bottom + // TODO: Implement a priority mechanism + RegisterConditionOperator(Constants.CorePluginInfo, new NullConditionOperator()); + RegisterConditionOperator(Constants.CorePluginInfo, new NotNullConditionOperator()); } } } \ No newline at end of file diff --git a/src/Artemis.Core/Utilities/ExpressionUtilities.cs b/src/Artemis.Core/Utilities/ExpressionUtilities.cs new file mode 100644 index 000000000..ca43a078f --- /dev/null +++ b/src/Artemis.Core/Utilities/ExpressionUtilities.cs @@ -0,0 +1,37 @@ +using System.Linq.Expressions; +using Artemis.Core.DataModelExpansions; + +namespace Artemis.Core +{ + internal static class ExpressionUtilities + { + internal static Expression CreateDataModelAccessor(DataModel dataModel, string path, string parameterName, out ParameterExpression parameter) + { + parameter = Expression.Parameter(typeof(object), parameterName + "DataModel"); + + // Create an expression that checks every part of the path for null + // In the same iteration, create the accessor + Expression source = Expression.Convert(parameter, dataModel.GetType()); + return CreateNullCheckedAccessor(source, path); + } + + internal static Expression CreateNullCheckedAccessor(Expression source, string path) + { + // Create an expression that checks every part of the path for null + // In the same iteration, create the accessor + Expression condition = null; + foreach (var memberName in path.Split('.')) + { + var notNull = Expression.NotEqual(source, Expression.Constant(null)); + condition = condition != null ? Expression.AndAlso(condition, notNull) : notNull; + source = Expression.PropertyOrField(source, memberName); + } + + if (condition == null) + throw new ArtemisCoreException($"Failed to create a null-check for path {path}"); + + // Combine the null check and the accessor in a conditional statement that returns the default for the type if null + return Expression.Condition(condition, source, Expression.Default(source.Type)); + } + } +} \ No newline at end of file diff --git a/src/Artemis.Plugins.DataModelExpansions.TestData/PluginDataModelExpansion.cs b/src/Artemis.Plugins.DataModelExpansions.TestData/PluginDataModelExpansion.cs index 38ece82fd..f07a415c3 100644 --- a/src/Artemis.Plugins.DataModelExpansions.TestData/PluginDataModelExpansion.cs +++ b/src/Artemis.Plugins.DataModelExpansions.TestData/PluginDataModelExpansion.cs @@ -1,4 +1,5 @@ using System; +using System.Linq; using Artemis.Core.DataModelExpansions; using Artemis.Plugins.DataModelExpansions.TestData.DataModels; using SkiaSharp; diff --git a/src/Artemis.UI/Screens/ProfileEditor/Conditions/DataModelConditionPredicateViewModel.cs b/src/Artemis.UI/Screens/ProfileEditor/Conditions/DataModelConditionPredicateViewModel.cs index 5a3bf7f66..e94e4e90e 100644 --- a/src/Artemis.UI/Screens/ProfileEditor/Conditions/DataModelConditionPredicateViewModel.cs +++ b/src/Artemis.UI/Screens/ProfileEditor/Conditions/DataModelConditionPredicateViewModel.cs @@ -128,10 +128,15 @@ namespace Artemis.UI.Screens.ProfileEditor.Conditions if (DataModelConditionPredicate.Operator == null) DataModelConditionPredicate.UpdateOperator(Operators.FirstOrDefault(o => o.SupportsType(leftSideType))); SelectedOperator = DataModelConditionPredicate.Operator; + if (!SelectedOperator.SupportsRightSide) + { + DisposeRightSideStatic(); + DisposeRightSideDynamic(); + } // Ensure the right side has the proper VM var targetType = LeftSideSelectionViewModel?.SelectedPropertyViewModel?.PropertyInfo?.PropertyType; - if (DataModelConditionPredicate.PredicateType == ProfileRightSideType.Dynamic) + if (DataModelConditionPredicate.PredicateType == ProfileRightSideType.Dynamic && SelectedOperator.SupportsRightSide) { DisposeRightSideStatic(); if (RightSideSelectionViewModel == null) @@ -147,7 +152,7 @@ namespace Artemis.UI.Screens.ProfileEditor.Conditions ); RightSideSelectionViewModel.FilterTypes = new[] {targetType}; } - else + else if (SelectedOperator.SupportsRightSide) { DisposeRightSideDynamic(); if (RightSideInputViewModel == null)