Condition operators - Redesigned API to leverage generics

Condition operators - Updated default operators to use new API

src/Artemis.Core/Models/Profile/Conditions/Abstract/BaseConditionOperator.cs

@@ -0,0 +1,65 @@
+using System;
+
+namespace Artemis.Core
+{
+    /// <summary>
+    ///     Represents a condition operator that performs a boolean operation
+    ///     <para>
+    ///         To implement your own condition operator, inherit <see cref="ConditionOperator{TLeftSide, TRightSide}" /> or
+    ///         <see cref="ConditionOperator{TLeftSide}" />
+    ///     </para>
+    /// </summary>
+    public abstract class BaseConditionOperator
+    {
+        /// <summary>
+        ///     Gets or sets the description of this logical operator
+        /// </summary>
+        public abstract string Description { get; }
+
+        /// <summary>
+        ///     Gets or sets the icon of this logical operator
+        /// </summary>
+        public abstract string Icon { get; }
+
+        /// <summary>
+        ///     Gets the plugin info this condition operator belongs to
+        ///     <para>Note: Not set until after registering</para>
+        /// </summary>
+        public PluginInfo PluginInfo { get; internal set; }
+
+        /// <summary>
+        ///     Gets the left side type of this condition operator
+        /// </summary>
+        public abstract Type LeftSideType { get; }
+
+        /// <summary>
+        ///     Gets the right side type of this condition operator. May be null if the operator does not support a left side type
+        /// </summary>
+        public abstract Type? RightSideType { get; }
+
+        /// <summary>
+        ///     Returns whether the given type is supported by the operator
+        /// </summary>
+        /// <param name="type">The type to check for, must be either the same or be castable to the target type</param>
+        /// <param name="side">Which side of the operator to check, left or right</param>
+        public abstract bool SupportsType(Type type, ConditionParameterSide side);
+
+        /// <summary>
+        ///     Evaluates the condition with the input types being provided as objects
+        ///     <para>
+        ///         This leaves the caller responsible for the types matching <see cref="LeftSideType" /> and
+        ///         <see cref="RightSideType" />
+        ///     </para>
+        /// </summary>
+        /// <param name="leftSideValue">The left side value, type should match <see cref="LeftSideType" /></param>
+        /// <param name="rightSideValue">The right side value, type should match <see cref="RightSideType" /></param>
+        /// <returns>The result of the boolean condition's evaluation</returns>
+        internal abstract bool InternalEvaluate(object? leftSideValue, object? rightSideValue);
+    }
+
+    public enum ConditionParameterSide
+    {
+        Left,
+        Right
+    }
+}

src/Artemis.Core/Models/Profile/Conditions/Abstract/ConditionOperator.cs

@@ -0,0 +1,95 @@
+using System;
+
+namespace Artemis.Core
+{
+    /// <summary>
+    ///     Represents a condition operator that performs a boolean operation using a left- and right-side
+    /// </summary>
+    public abstract class ConditionOperator<TLeftSide, TRightSide> : BaseConditionOperator
+    {
+        /// <summary>
+        ///     Evaluates the operator on a and b
+        /// </summary>
+        /// <param name="a">The parameter on the left side of the expression</param>
+        /// <param name="b">The parameter on the right side of the expression</param>
+        public abstract bool Evaluate(TLeftSide a, TRightSide b);
+
+        /// <inheritdoc />
+        public override bool SupportsType(Type type, ConditionParameterSide side)
+        {
+            if (type == null)
+                return true;
+            if (side == ConditionParameterSide.Left)
+                return LeftSideType.IsCastableFrom(type);
+            return RightSideType.IsCastableFrom(type);
+        }
+
+        /// <inheritdoc />
+        internal override bool InternalEvaluate(object? leftSideValue, object? rightSideValue)
+        {
+            // TODO: Can we avoid boxing/unboxing?
+            TLeftSide leftSide;
+            if (leftSideValue != null)
+                leftSide = (TLeftSide) Convert.ChangeType(leftSideValue, typeof(TLeftSide));
+            else
+                leftSide = default;
+
+            TRightSide rightSide;
+            if (rightSideValue != null)
+                rightSide = (TRightSide) Convert.ChangeType(rightSideValue, typeof(TRightSide));
+            else
+                rightSide = default;
+
+            return Evaluate(leftSide, rightSide);
+        }
+
+        /// <inheritdoc />
+        public override Type LeftSideType => typeof(TLeftSide);
+
+        /// <inheritdoc />
+        public override Type RightSideType => typeof(TRightSide);
+    }
+
+    /// <summary>
+    ///     Represents a condition operator that performs a boolean operation using only a left side
+    /// </summary>
+    public abstract class ConditionOperator<TLeftSide> : BaseConditionOperator
+    {
+        /// <summary>
+        ///     Evaluates the operator on a and b
+        /// </summary>
+        /// <param name="a">The parameter on the left side of the expression</param>
+        public abstract bool Evaluate(TLeftSide a);
+
+        /// <inheritdoc />
+        public override bool SupportsType(Type type, ConditionParameterSide side)
+        {
+            if (type == null)
+                return true;
+            if (side == ConditionParameterSide.Left)
+                return LeftSideType.IsCastableFrom(type);
+            return false;
+        }
+
+        /// <inheritdoc />
+        internal override bool InternalEvaluate(object? leftSideValue, object? rightSideValue)
+        {
+            // TODO: Can we avoid boxing/unboxing?
+            TLeftSide leftSide;
+            if (leftSideValue != null)
+                leftSide = (TLeftSide) Convert.ChangeType(leftSideValue, typeof(TLeftSide));
+            else
+                leftSide = default;
+
+            return Evaluate(leftSide);
+        }
+
+        /// <inheritdoc />
+        public override Type LeftSideType => typeof(TLeftSide);
+
+        /// <summary>
+        ///     Always <c>null</c>, not applicable to this type of condition operator
+        /// </summary>
+        public override Type? RightSideType => null;
+    }
+}

src/Artemis.Core/Models/Profile/Conditions/ConditionOperator.cs

@@ -1,55 +0,0 @@
-using System;
-using System.Collections.Generic;
-using System.Linq;
-
-namespace Artemis.Core
-{
-    /// <summary>
-    ///     A condition operator is used by the conditions system to perform a specific boolean check
-    /// </summary>
-    public abstract class ConditionOperator
-    {
-        /// <summary>
-        ///     Gets the plugin info this condition operator belongs to
-        ///     <para>Note: Not set until after registering</para>
-        /// </summary>
-        public PluginInfo PluginInfo { get; internal set; }
-
-        /// <summary>
-        ///     Gets the types this operator supports
-        /// </summary>
-        public abstract IReadOnlyCollection<Type> CompatibleTypes { get; }
-
-        /// <summary>
-        ///     Gets or sets the description of this logical operator
-        /// </summary>
-        public abstract string Description { get; }
-
-        /// <summary>
-        ///     Gets or sets the icon of this logical operator
-        /// </summary>
-        public abstract string Icon { get; }
-
-        /// <summary>
-        ///     Gets or sets whether this condition operator supports a right side, defaults to true
-        /// </summary>
-        public bool SupportsRightSide { get; protected set; } = true;
-
-        /// <summary>
-        ///     Returns whether the given type is supported by the operator
-        /// </summary>
-        public bool SupportsType(Type type)
-        {
-            if (type == null)
-                return true;
-            return CompatibleTypes.Any(t => t.IsCastableFrom(type));
-        }
-
-        /// <summary>
-        /// Evaluates the operator on a and b
-        /// </summary>
-        /// <param name="a">The parameter on the left side of the expression</param>
-        /// <param name="b">The parameter on the right side of the expression</param>
-        public abstract bool Evaluate(object? a, object? b);
-    }
-}

src/Artemis.Core/Models/Profile/Conditions/DataModelConditionListPredicate.cs

@@ -47,7 +47,7 @@ namespace Artemis.Core
         /// <summary>
         ///     Gets the operator
         /// </summary>
-        public ConditionOperator? Operator { get; private set; }
+        public BaseConditionOperator? Operator { get; private set; }
 
         /// <summary>
         ///     Gets the data model condition list this predicate belongs to
@@ -99,6 +99,8 @@ namespace Artemis.Core
         {
             if (path != null && !path.IsValid)
                 throw new ArtemisCoreException("Cannot update right side of predicate to an invalid path");
+            if (Operator != null && path != null && !Operator.SupportsType(path.GetPropertyType()!, ConditionParameterSide.Right))
+                throw new ArtemisCoreException($"Selected operator does not support right side of type {path.GetPropertyType()!.Name}");
 
             RightPath?.Dispose();
             RightPath = path != null ? new DataModelPath(path) : null;
@@ -123,7 +125,7 @@ namespace Artemis.Core
         ///     Updates the operator of the predicate and re-compiles the expression
         /// </summary>
         /// <param name="conditionOperator"></param>
-        public void UpdateOperator(ConditionOperator? conditionOperator)
+        public void UpdateOperator(BaseConditionOperator? conditionOperator)
         {
             if (conditionOperator == null)
             {
@@ -139,12 +141,13 @@ namespace Artemis.Core
             }
 
             Type leftType = LeftPath.GetPropertyType()!;
-            if (!conditionOperator.SupportsType(leftType))
+            // Left side can't go empty so enforce a match
+            if (!conditionOperator.SupportsType(leftType, ConditionParameterSide.Left))
                 throw new ArtemisCoreException($"Cannot apply operator {conditionOperator.GetType().Name} to this predicate because " +
                                                $"it does not support left side type {leftType.Name}");
 
-            if (conditionOperator.SupportsType(leftType))
-                Operator = conditionOperator;
+            Operator = conditionOperator;
+            ValidateRightSide();
         }
 
         /// <summary>
@@ -206,7 +209,7 @@ namespace Artemis.Core
                 if (leftSideValue != null && leftSideValue.GetType().IsValueType && RightStaticValue == null)
                     return false;
 
-                return Operator.Evaluate(leftSideValue, RightStaticValue);
+                return Operator.InternalEvaluate(leftSideValue, RightStaticValue);
             }
 
             if (RightPath == null || !RightPath.IsValid)
@@ -217,8 +220,8 @@ namespace Artemis.Core
             {
                 // If the path targets a property inside the list, evaluate on the list path value instead of the right path value
                 if (RightPath.Target is ListPredicateWrapperDataModel)
-                    return Operator.Evaluate(GetListPathValue(LeftPath, target), GetListPathValue(RightPath, target));
-                return Operator.Evaluate(GetListPathValue(LeftPath, target), RightPath.GetValue());
+                    return Operator.InternalEvaluate(GetListPathValue(LeftPath, target), GetListPathValue(RightPath, target));
+                return Operator.InternalEvaluate(GetListPathValue(LeftPath, target), RightPath.GetValue());
             }
 
             return false;
@@ -300,7 +303,7 @@ namespace Artemis.Core
             // Operator
             if (Entity.OperatorPluginGuid != null)
             {
-                ConditionOperator? conditionOperator = ConditionOperatorStore.Get(Entity.OperatorPluginGuid.Value, Entity.OperatorType)?.ConditionOperator;
+                BaseConditionOperator? conditionOperator = ConditionOperatorStore.Get(Entity.OperatorPluginGuid.Value, Entity.OperatorType)?.ConditionOperator;
                 if (conditionOperator != null)
                     UpdateOperator(conditionOperator);
             }
@@ -361,28 +364,31 @@ namespace Artemis.Core
                 return;
 
             Type leftType = LeftPath.GetPropertyType()!;
-            if (!Operator.SupportsType(leftType))
+            if (!Operator.SupportsType(leftType, ConditionParameterSide.Left))
                 Operator = null;
         }
 
         private void ValidateRightSide()
         {
-            Type? leftType = LeftPath?.GetPropertyType();
+            if (Operator == null)
+                return;
+
             if (PredicateType == ProfileRightSideType.Dynamic)
             {
                 if (RightPath == null || !RightPath.IsValid)
                     return;
 
                 Type rightSideType = RightPath.GetPropertyType()!;
-                if (leftType != null && !leftType.IsCastableFrom(rightSideType))
+                if (!Operator.SupportsType(rightSideType, ConditionParameterSide.Right))
                     UpdateRightSideDynamic(null);
             }
             else
             {
-                if (RightStaticValue != null && (leftType == null || leftType.IsCastableFrom(RightStaticValue.GetType())))
-                    UpdateRightSideStatic(RightStaticValue);
-                else
-                    UpdateRightSideStatic(null);
+                if (RightStaticValue == null)
+                    return;
+
+                if (!Operator.SupportsType(RightStaticValue.GetType(), ConditionParameterSide.Right))
+                    UpdateRightSideDynamic(null);
             }
         }
 
@@ -391,24 +397,27 @@ namespace Artemis.Core
             RightPath?.Dispose();
             RightPath = null;
 
-            // If the left side is empty simply apply the value, any validation will wait
-            if (LeftPath == null || !LeftPath.IsValid)
+            // If the operator is null simply apply the value, any validation will wait
+            if (Operator == null)
             {
                 RightStaticValue = staticValue;
                 return;
             }
-
-            // If the left path is valid we can expect a type
-            Type leftSideType = LeftPath.GetPropertyType()!;
+            // If the operator does not support a right side, always set it to null
+            if (Operator.RightSideType == null)
+            {
+                RightStaticValue = null;
+                return;
+            }
 
             // If not null ensure the types match and if not, convert it
-            if (staticValue != null && staticValue.GetType() == leftSideType)
+            if (staticValue != null && staticValue.GetType() == Operator.RightSideType)
                 RightStaticValue = staticValue;
             else if (staticValue != null)
-                RightStaticValue = Convert.ChangeType(staticValue, leftSideType);
+                RightStaticValue = Convert.ChangeType(staticValue, Operator.RightSideType);
             // If null create a default instance for value types or simply make it null for reference types
-            else if (leftSideType.IsValueType)
-                RightStaticValue = Activator.CreateInstance(leftSideType);
+            else if (Operator.RightSideType.IsValueType)
+                RightStaticValue = Activator.CreateInstance(Operator.RightSideType);
             else
                 RightStaticValue = null;
         }
@@ -426,7 +435,7 @@ namespace Artemis.Core
 
         private void ConditionOperatorStoreOnConditionOperatorAdded(object sender, ConditionOperatorStoreEvent e)
         {
-            ConditionOperator conditionOperator = e.Registration.ConditionOperator;
+            BaseConditionOperator conditionOperator = e.Registration.ConditionOperator;
             if (Entity.OperatorPluginGuid == conditionOperator.PluginInfo.Guid && Entity.OperatorType == conditionOperator.GetType().Name)
                 UpdateOperator(conditionOperator);
         }

src/Artemis.Core/Models/Profile/Conditions/DataModelConditionPredicate.cs

@@ -1,4 +1,5 @@
 using System;
+using System.IO;
 using Artemis.Core.DataModelExpansions;
 using Artemis.Storage.Entities.Profile.Abstract;
 using Artemis.Storage.Entities.Profile.Conditions;
@@ -43,7 +44,7 @@ namespace Artemis.Core
         /// <summary>
         ///     Gets the operator
         /// </summary>
-        public ConditionOperator? Operator { get; private set; }
+        public BaseConditionOperator? Operator { get; private set; }
 
         /// <summary>
         ///     Gets the path of the left property
@@ -87,6 +88,8 @@ namespace Artemis.Core
         {
             if (path != null && !path.IsValid)
                 throw new ArtemisCoreException("Cannot update right side of predicate to an invalid path");
+            if (Operator != null && path != null && !Operator.SupportsType(path.GetPropertyType()!, ConditionParameterSide.Right))
+                throw new ArtemisCoreException($"Selected operator does not support right side of type {path.GetPropertyType()!.Name}");
 
             RightPath?.Dispose();
             RightPath = path != null ? new DataModelPath(path) : null;
@@ -104,24 +107,27 @@ namespace Artemis.Core
             RightPath?.Dispose();
             RightPath = null;
 
-            // If the left side is empty simply apply the value, any validation will wait
-            if (LeftPath == null || !LeftPath.IsValid)
+            // If the operator is null simply apply the value, any validation will wait
+            if (Operator == null)
             {
                 RightStaticValue = staticValue;
                 return;
             }
-
-            // If the left path is valid we can expect a type
-            Type leftSideType = LeftPath.GetPropertyType()!;
+            // If the operator does not support a right side, always set it to null
+            if (Operator.RightSideType == null)
+            {
+                RightStaticValue = null;
+                return;
+            }
 
             // If not null ensure the types match and if not, convert it
-            if (staticValue != null && staticValue.GetType() == leftSideType)
+            if (staticValue != null && staticValue.GetType() == Operator.RightSideType)
                 RightStaticValue = staticValue;
             else if (staticValue != null)
-                RightStaticValue = Convert.ChangeType(staticValue, leftSideType);
+                RightStaticValue = Convert.ChangeType(staticValue, Operator.RightSideType);
             // If null create a default instance for value types or simply make it null for reference types
-            else if (leftSideType.IsValueType)
-                RightStaticValue = Activator.CreateInstance(leftSideType);
+            else if (Operator.RightSideType.IsValueType)
+                RightStaticValue = Activator.CreateInstance(Operator.RightSideType);
             else
                 RightStaticValue = null;
         }
@@ -130,9 +136,8 @@ namespace Artemis.Core
         ///     Updates the operator of the predicate and re-compiles the expression
         /// </summary>
         /// <param name="conditionOperator"></param>
-        public void UpdateOperator(ConditionOperator? conditionOperator)
+        public void UpdateOperator(BaseConditionOperator? conditionOperator)
         {
-            // Calling CreateExpression will clear compiled expressions
             if (conditionOperator == null)
             {
                 Operator = null;
@@ -147,11 +152,13 @@ namespace Artemis.Core
             }
 
             Type leftType = LeftPath.GetPropertyType()!;
-            if (!conditionOperator.SupportsType(leftType))
+            // Left side can't go empty so enforce a match
+            if (!conditionOperator.SupportsType(leftType, ConditionParameterSide.Left))
                 throw new ArtemisCoreException($"Cannot apply operator {conditionOperator.GetType().Name} to this predicate because " +
                                                $"it does not support left side type {leftType.Name}");
 
             Operator = conditionOperator;
+            ValidateRightSide();
         }
 
         /// <inheritdoc />
@@ -167,14 +174,14 @@ namespace Artemis.Core
                 if (leftSideValue != null && leftSideValue.GetType().IsValueType && RightStaticValue == null)
                     return false;
 
-                return Operator.Evaluate(leftSideValue, RightStaticValue);
+                return Operator.InternalEvaluate(leftSideValue, RightStaticValue);
             }
 
             if (RightPath == null || !RightPath.IsValid)
                 return false;
 
             // Compare with dynamic values
-            return Operator.Evaluate(LeftPath.GetValue(), RightPath.GetValue());
+            return Operator.InternalEvaluate(LeftPath.GetValue(), RightPath.GetValue());
         }
 
         /// <inheritdoc />
@@ -237,7 +244,7 @@ namespace Artemis.Core
             // Operator
             if (Entity.OperatorPluginGuid != null)
             {
-                ConditionOperator? conditionOperator = ConditionOperatorStore.Get(Entity.OperatorPluginGuid.Value, Entity.OperatorType)?.ConditionOperator;
+                BaseConditionOperator? conditionOperator = ConditionOperatorStore.Get(Entity.OperatorPluginGuid.Value, Entity.OperatorType)?.ConditionOperator;
                 if (conditionOperator != null)
                     UpdateOperator(conditionOperator);
             }
@@ -292,28 +299,31 @@ namespace Artemis.Core
                 return;
 
             Type leftType = LeftPath.GetPropertyType()!;
-            if (!Operator.SupportsType(leftType))
+            if (!Operator.SupportsType(leftType, ConditionParameterSide.Left))
                 Operator = null;
         }
 
         private void ValidateRightSide()
         {
-            Type? leftType = LeftPath?.GetPropertyType();
+            if (Operator == null)
+                return;
+
             if (PredicateType == ProfileRightSideType.Dynamic)
             {
                 if (RightPath == null || !RightPath.IsValid)
                     return;
 
                 Type rightSideType = RightPath.GetPropertyType()!;
-                if (leftType != null && !leftType.IsCastableFrom(rightSideType))
+                if (!Operator.SupportsType(rightSideType, ConditionParameterSide.Right))
                     UpdateRightSideDynamic(null);
             }
             else
             {
-                if (RightStaticValue != null && (leftType == null || leftType.IsCastableFrom(RightStaticValue.GetType())))
-                    UpdateRightSideStatic(RightStaticValue);
-                else
-                    UpdateRightSideStatic(null);
+                if (RightStaticValue == null)
+                    return;
+
+                if (!Operator.SupportsType(RightStaticValue.GetType(), ConditionParameterSide.Right))
+                    UpdateRightSideDynamic(null);
             }
         }
 
@@ -321,7 +331,7 @@ namespace Artemis.Core
 
         private void ConditionOperatorStoreOnConditionOperatorAdded(object? sender, ConditionOperatorStoreEvent e)
         {
-            ConditionOperator conditionOperator = e.Registration.ConditionOperator;
+            BaseConditionOperator conditionOperator = e.Registration.ConditionOperator;
             if (Entity.OperatorPluginGuid == conditionOperator.PluginInfo.Guid && Entity.OperatorType == conditionOperator.GetType().Name)
                 UpdateOperator(conditionOperator);
         }