asulwer / RulesEngine

Rules Engine with extensive Dynamic expression support
MIT License
26 stars 1 forks source link

RuleParameter.Value changes type of ExpandoObject values #53

Closed Michaelcombs closed 4 months ago

Michaelcombs commented 4 months ago

Steps to Reproduce

  1. Create and register a custom Action and configure it in a workflow.
  2. Create a Ruleparameter and set the Value to an ExpandoObject with data that will trigger rule success for the workflow. Call RuleEngine.ExecuteAllRulesAsync with the RuleParameter as an argument.
  3. Check the type of RuleParameter.Value in the custom action ( Actionbase.Run).

Expected Behavior

Cast RuleParameter.Value as ExpandoObject and access the properties and data.

Observed Behavior

The cast result is a null value. The underlying type of RuleParameter.Value is no longer an ExpandoObject. Utils.GetTypedObject is called in the constructor of RuleParameter which changes the type of the object to an anonymous runtime type.

Suggested Next Steps

Add a property to RuleParameter which preserves the original type and value of the Value property which is not change via Utils.GetTypedObject. (e.g. RuleProperty.OriginalValue). This will provide access to the RuleParameter input data in the same manner as the data was created.

This problem makes it difficult to safely access data from the RuleParameters (i.e. the input data) in the custom Action. I need to extract data from the RuleParameter that is optional. If I access the data as a dynamic, I cannot check to see if the optional property exists before access without exceptions.

I also attempted to create a new class, CustomRuleParameter, which inherits from RuleParaemter. I added a property, CustomRuleParameter.OriginalValue which preserves the original Value object. I then passed this object to RuleEngine.ExecuteAllRulesAsync. I was unable to cast the RuleParameter back to CustomRuleParameter in Actionbase.Run. The original RuleParameter is not preserved.

asulwer commented 4 months ago
dynamic input1 = new ExpandoObject();
input1.count = 1;
var inputs = new RuleParameter[] { new ("input1", input1) };

var ret = await bre.ExecuteAllRulesAsync(workflow.WorkflowName, inputs, cancellationToken);

this worked as expected. did you do something different?

asulwer commented 4 months ago

the ExpandoObject is changed to DynamicClass. The underlying nuget package for RulesEngine uses System.Linq.Dynamic.Core and they discussed, i think, this issue

asulwer commented 4 months ago

DynamicClass contains a couple of methods that you may find handy. Maybe this could help you? my test code below isn't perfect but maybe...

dynamic input1 = new ExpandoObject();
input1.count = 1;
var inputs = new RuleParameter[] { new ("input1", input1) };

DynamicClass dc = inputs[0].Value as DynamicClass;
var names = dc.GetDynamicMemberNames();
var value = dc.GetDynamicPropertyValue<int>("count");
Michaelcombs commented 4 months ago

This is a simpler demonstration of the problem:

dynamic input1 = new ExpandoObject(); input1.count = 1; RuleParameter rp = new RuleParameter("data", input1);

ExpandoObject data = rp.Value as ExpandoObject; //data is null here

The RuleParameter is used throughout the lifecycle of the workflow execution and passed back in Custom Actions. I think the RuleParameter should preserve the original type so it can be cast back to the original type and accessed in the same way in the Custom Action. This could be done with an additional property.

Thank you for the DynamicClass suggestion! I prefer this to potential exceptions from accessing properties that don't exist.

asulwer commented 4 months ago

i agree keeping the type as is. i will see what i can do

RenanCarlosPereira commented 4 months ago

The issue you are encountering revolves around the usage of the ExpandoObject and the way the RuleParameter class is handling dynamic objects. Specifically, the problem lies in the way RuleParameter stores and retrieves the dynamic objects, resulting in a loss of the expected type when casting back.

Detailed Breakdown

  1. Dynamic Object and RuleParameter:

    dynamic input1 = new ExpandoObject();
    input1.count = 1;
    RuleParameter rp = new RuleParameter("data", input1);
    
    ExpandoObject data = rp.Value as ExpandoObject;
    // data is null here

    Here, you create a dynamic object input1 and assign it to a RuleParameter. Later, you try to cast the rp.Value back to ExpandoObject, but it's null. This happens because RuleParameter is transforming the ExpandoObject into a typed object, and when you try to cast it back, it fails.

  2. RuleParameter Class:

    public class RuleParameter
    {
        public RuleParameter(string name, object value)
        {
            Value = Utils.GetTypedObject(value);
            Init(name, Value?.GetType());
        }
        // Other members and methods...
    }

    The constructor calls Utils.GetTypedObject(value), which transforms the ExpandoObject into a typed object using System.Dynamic.Linq.Core.

  3. Utils.GetTypedObject Method:

    public static object GetTypedObject(dynamic input)
    {
        if (input is ExpandoObject)
        {
            Type type = CreateAbstractClassType(input);
            return CreateObject(type, input);
        }
        else
        {
            return input;
        }
    }

    This method checks if the input is an ExpandoObject and transforms it into a strongly-typed object using CreateAbstractClassType and CreateObject methods.

  4. Transformation of ExpandoObject:

    • CreateAbstractClassType creates a type dynamically based on the properties of the ExpandoObject.
    • CreateObject creates an instance of that type and populates it with the values from the ExpandoObject.

Problem Summary

The core of the issue is that the RuleParameter class converts the ExpandoObject into a strongly-typed object during its initialization. This conversion makes it impossible to cast back to ExpandoObject later.

Suggested Solution

To address this issue, you could modify the RuleParameter class to preserve the original ExpandoObject along with the typed object. This way, you can access both the original dynamic object and the typed version when needed.

Here is an example modification:

public class RuleParameter
{
    public RuleParameter(string name, object value)
    {
        OriginalValue = value;
        Value = Utils.GetTypedObject(value);
        Init(name, Value?.GetType());
    }

    public Type Type { get; private set; }
    public string Name { get; private set; }
    public object Value { get; private set; }
    public object OriginalValue { get; private set; } // Preserve the original value
    public ParameterExpression ParameterExpression { get; private set; }

    private void Init(string name, Type type)
    {
        Name = name;
        Type = type ?? typeof(object);
        ParameterExpression = Expression.Parameter(Type, Name);
    }

    public static RuleParameter Create<T>(string name, T value)
    {
        var typedValue = Utils.GetTypedObject(value);
        var type = typedValue?.GetType() ?? typeof(T);
        return new RuleParameter(name, value); // Pass original value
    }
}

With this modification, you can access the original dynamic object:

dynamic input1 = new ExpandoObject();
input1.count = 1;
RuleParameter rp = new RuleParameter("data", input1);

ExpandoObject originalData = rp.OriginalValue as ExpandoObject;
// originalData will now hold the original ExpandoObject

This preserves the original ExpandoObject, allowing you to cast it back without losing the dynamic nature of the object. But this approach could

Note: If the rules engine mutates the object Value, OriginalValue and Value may diverge. We need to ensure that the rules engine does not mutate the Value object, otherwise, we need to consider additional synchronization logic if mutations are possible.

@Michaelcombs could you tell me a bit more about what you are thinking about when you want to access this object?

asulwer commented 4 months ago

OR

ExpandoObject data = inputs[0].Value.ToExpando();

@Michaelcombs @RenanCarlosPereira

I created an extension method for type object which can convert back to ExpandoObject. here is a summary of the methods that i would like to add to the associated PR

public static class RuleParameterExtensions
{
    /// <summary>
    /// Extension method that turns a dictionary of string and object to an ExpandoObject
    /// </summary>
    public static ExpandoObject ToExpando(this IDictionary<string, object> dictionary)
    {
        var expando = new ExpandoObject();
        var expandoDic = (IDictionary<string, object>)expando;

        // go through the items in the dictionary and copy over the key value pairs)
        foreach (var kvp in dictionary)
        {
            // if the value can also be turned into an ExpandoObject, then do it!
            if (kvp.Value is IDictionary<string, object>)
            {
                var expandoValue = ((IDictionary<string, object>)kvp.Value).ToExpando();
                expandoDic.Add(kvp.Key, expandoValue);
            }
            else if (kvp.Value is ICollection)
            {
                // iterate through the collection and convert any strin-object dictionaries
                // along the way into expando objects
                var itemList = new List<object>();
                foreach (var item in (ICollection)kvp.Value)
                {
                    if (item is IDictionary<string, object>)
                    {
                        var expandoItem = ((IDictionary<string, object>)item).ToExpando();
                        itemList.Add(expandoItem);
                    }
                    else
                    {
                        itemList.Add(item);
                    }
                }

                expandoDic.Add(kvp.Key, itemList);
            }
            else
            {
                expandoDic.Add(kvp);
            }
        }

        return expando;
    }

    public static ExpandoObject ToExpando(this object obj)
    {
        // Null-check
        Dictionary<string, object> expando = new Dictionary<string, object>();

        foreach (PropertyDescriptor property in TypeDescriptor.GetProperties(obj.GetType()))
        {
            expando.Add(property.Name, property.GetValue(obj));
        }

        return expando.ToExpando();
    }
}
asulwer commented 4 months ago

both suggestions could be used. i am going to see if the code can maintain the actual object as is instead of using one of the two suggestions. regardless, one of our suggestions could work?

Michaelcombs commented 4 months ago

@RenanCarlosPereira
in reference to your question "could you tell me a bit more about what you are thinking about when you want to access this object?"

  1. I route Mqtt JSON data payloads from sensors through rule engine workflows as ExpandoObjects via RuleParameters. I do not have control over the JSON format. There are many formats so I am not unloading the JSON into strongly typed objects.

  2. I am using custom action classes. Upon successful triggering of a rule, the RuleParameters used during rule evaluation are passed back into the custom action via the ActionBase.Run method.

  3. In the custom action I need to extract data from the RuleParameters, specifically a sensor id, to initiate subsequent processes. I would like to maintain consistent access to the data throughout the code. I often use extensions for ExpandoObject and JsonElement as well as validation utilities.

RenanCarlosPereira commented 4 months ago

thanks @Michaelcombs

here's a PR, feel free to take a look at it #55.

Summary of Changes

  1. Added OriginalValue Property:

    • Preserves the original ExpandoObject without converting its type.
  2. Implemented TryGetPropertyValue Method:

    • Safely accesses properties, returning a boolean indicating success and setting the out parameter to the property value or null if not found.
  3. Created Unit Tests:

    • Tests for TryGetPropertyValue behavior and ensuring OriginalValue is correctly set in the constructor.

Reason for Changes

The issue involved the RuleParameter class transforming ExpandoObject into a strongly-typed object, causing a loss of the expected type when casting back. The changes ensure that both the original dynamic object and the typed version are accessible, maintaining data integrity throughout the workflow execution.

asulwer commented 4 months ago

@Michaelcombs

@RenanCarlosPereira has created a potential solution to your issue. my solution is here IF anyone wants that added at a later date.

asulwer commented 4 months ago

@Michaelcombs i am going to assume this solved your issue? if so, then i would like to close this issue

Michaelcombs commented 4 months ago

Yes. Thank you!