12
\$\begingroup\$

Ok, before you ask: yes, I need to do this. Sort of.

I'm wrapping a 3rd-party API for data access, and I can't use an ORM, so I'm implementing this kind of thing:

public interface IRepository<TEntity> where TEntity : class, new()
{
    /// <summary>
    /// Projects all entities that match specified predicate into a <see cref="TEntity"/> instance.
    /// </summary>
    /// <param name="filter">A function expression that returns <c>true</c> for all entities to return.</param>
    /// <returns></returns>
    IEnumerable<TEntity> Select(Expression<Func<TEntity, bool>> filter);

    /// <summary>
    /// Projects the single that matches specified predicate into a <see cref="TEntity"/> instance.
    /// </summary>
    /// <exception cref="InvalidOperationException">Thrown when predicate matches more than a single result.</exception>
    /// <param name="filter">A function expression that returns <c>true</c> for the only entity to return.</param>
    /// <returns></returns>
    TEntity Single(Expression<Func<TEntity, bool>> filter);

    /// <summary>
    /// Updates the underlying <see cref="View"/> for the specified entity.
    /// </summary>
    /// <param name="entity">The existing entity with the modified property values.</param>
    void Update(TEntity entity);

    /// <summary>
    /// Deletes the specified entity from the underlying <see cref="View"/>.
    /// </summary>
    /// <param name="entity">The existing entity to remove.</param>
    void Delete(TEntity entity);

    /// <summary>
    /// Inserts a new entity into the underlying <see cref="View"/>.
    /// </summary>
    /// <param name="entity">A non-existing entity to create in the system.</param>
    void Insert(TEntity entity);
}

Notice the Expression<Func<TEntity, bool>> filter parameter of the Single and Select methods? That's so I can write this:

using (var repository = new PurchaseOrderRepository())
{
    var po = repository.Single(x => x.Number == "123456");
    //...
}

Instead of this:

_headerView.Browse("PONUMBER = \"123456\"", true);

So, this ToFilterExpression extension method allows me to nicely wrap this stringly-typed API with my own strongly-typed API, and hide all the nastiness behind a familiar IRepository abstraction.

Here's the extension method in question:

public static string ToFilterExpression<TEntity>(this Expression<Func<TEntity, bool>> expression) 
    where TEntity : class, new()
{
    if (expression == null)
    {
        return string.Empty;
    }

    var lambdaExpression = (LambdaExpression)expression;
    lambdaExpression = (LambdaExpression)Evaluator.PartialEval(lambdaExpression);

    var visitor = new FilterVisitor<TEntity>(lambdaExpression);
    var result = visitor.Filter;

    return result;
}

If you're curious, here's what the client code looks like:

static void Main(string[] args)
{
    using (var session = new Session())
    {
        session.Init(/*redacted*/);
        session.Open(/*redacted*/);

        using (var context = session.OpenDBLink(DBLinkType.Company, DBLinkFlags.ReadWrite))
        using (var repository = new PurchaseOrderHeadersRepository())
        {
            repository.Compose(context);
            var poNumber = "123456";
            var date = DateTime.Today.AddMonths(-1);
            var result = repository.Select(x => x.Number == poNumber && x.OrderDate >= date || x.Number.EndsWith("123"));

            foreach(var po in result)
            {
                Console.WriteLine("PO Number: {0}", po.Number);
            }
        }
    }

    Console.ReadLine();
}

...which looks pretty neat compared to what it would be without that wrapper API! The extension method produces this output:

"PONUMBER = 123456 AND ORDEREDON >= 20160105 OR PONUMBER LIKE \"%123\""

To achieve this, I implemented an ExpressionVisitor, adapting code from an MSDN article. Here's the visitor:

/// <summary>
/// Based on https://msdn.microsoft.com/en-us/library/bb546158.aspx
/// </summary>
internal class FilterVisitor<TEntity> : ExpressionVisitor
    where TEntity : class, new()
{
    private readonly Expression _expression;
    private string _filter;
    private readonly IList<EntityPropertyInfo<TEntity>> _properties;

    public FilterVisitor(Expression expression)
    {
        _expression = expression;
        _properties = typeof (TEntity).GetPropertyInfos<TEntity>().ToList();
    }

    public string Filter
    {
        get
        {
            if (_filter == null)
            {
                _filter = string.Empty;
                Visit(_expression);
            }
            return _filter;
        }
    }

    private readonly ExpressionType[] _binaryOperators =
    {
        ExpressionType.Equal,
        ExpressionType.NotEqual,
        ExpressionType.GreaterThan,
        ExpressionType.GreaterThanOrEqual,
        ExpressionType.LessThan,
        ExpressionType.LessThanOrEqual
    };

    private readonly IDictionary<ExpressionType, string> _binaryOperations = new Dictionary<ExpressionType, string>
    {
        { ExpressionType.Equal, " = " },
        { ExpressionType.NotEqual, " != " },
        { ExpressionType.GreaterThan, " > " },
        { ExpressionType.GreaterThanOrEqual, " >= " },
        { ExpressionType.LessThan, " < " },
        { ExpressionType.LessThanOrEqual, " <= " },
        { ExpressionType.AndAlso, " AND " },
        { ExpressionType.OrElse, " OR " },
    };

    private readonly Stack<string> _operators = new Stack<string>();
    protected override Expression VisitBinary(BinaryExpression b)
    {
        if (_binaryOperators.Contains(b.NodeType))
        {
            foreach (var property in _properties)
            {
                var name = property.Property.Name;
                if (ExpressionTreeHelpers.IsMemberEqualsValueExpression(b, typeof(TEntity), name, b.NodeType))
                {
                    var value = ExpressionTreeHelpers.GetValueFromEqualsExpression(b, typeof(TEntity), name, b.NodeType);
                    if (value is DateTime)
                    {
                        value = ((DateTime)value).ToString("yyyyMMdd");
                    }
                    _filter += property.FieldName + _binaryOperations[b.NodeType] + value;
                    if (_operators.Any())
                    {
                        _filter += _operators.Pop();
                    }
                    return b;
                }
            }
        }
        else if (b.NodeType == ExpressionType.AndAlso || b.NodeType == ExpressionType.OrElse)
        {
            _operators.Push(_binaryOperations[b.NodeType]);
        }

        return base.VisitBinary(b);
    }

    protected override Expression VisitMethodCall(MethodCallExpression m)
    {
        if (m.Method.DeclaringType == typeof(string))
        {
            if (m.Method.Name == "StartsWith")
            {
                foreach (var property in _properties)
                {
                    var name = property.Property.Name;
                    if (ExpressionTreeHelpers.IsSpecificMemberExpression(m.Object, typeof(TEntity), name))
                    {
                        _filter += property.FieldName + " LIKE \"" + ExpressionTreeHelpers.GetValueFromExpression(m.Arguments[0]) + "%\"";
                        return m;
                    }
                }
            }
            if (m.Method.Name == "EndsWith")
            {
                foreach (var property in _properties)
                {
                    var name = property.Property.Name;
                    if (ExpressionTreeHelpers.IsSpecificMemberExpression(m.Object, typeof(TEntity), name))
                    {
                        _filter += property.FieldName + " LIKE \"%" + ExpressionTreeHelpers.GetValueFromExpression(m.Arguments[0]) + "\"";
                        return m;
                    }
                }
            }
            if (m.Method.Name == "Contains")
            {
                foreach (var property in _properties)
                {
                    var name = property.Property.Name;
                    if (ExpressionTreeHelpers.IsSpecificMemberExpression(m.Object, typeof(TEntity), name))
                    {
                        _filter += property.FieldName + " LIKE \"%" + ExpressionTreeHelpers.GetValueFromExpression(m.Arguments[0]) + "%\"";
                        return m;
                    }
                }
            }
        }

        return base.VisitMethodCall(m);
    }
}

Obviously there are a number of things I could add and support additional constructs and method calls - but this is pretty much good enough for my immediate needs.

Is there a better way to do this?

\$\endgroup\$
  • \$\begingroup\$ Have you thought about sanitizing search strings? i.e. What if I supplied x.Number.EndsWith("123\" OR \"1\"=\"1") would it return everything? \$\endgroup\$ – RobH Feb 5 '16 at 8:50
  • 1
    \$\begingroup\$ Well, the data is coming from two legacy systems, I'm not concerned with "search filter injection" at all.. though it might of course be a concern if I had, say, a world-facing web UI using it... but I don't. \$\endgroup\$ – Mathieu Guindon Feb 5 '16 at 12:47
5
\$\begingroup\$

I wouldn't worry too much about the amount of things that aren't supported (yet/if ever) - it's impossible to cover everything in a scenario like this. One thing I would suggest is that you throw exceptions so the caller knows they're doing something unexpected:

protected override Expression VisitMethodCall(MethodCallExpression m)
{
    if (m.Method.DeclaringType == typeof(string))
    {
        // might work better as a switch with a default case. 
        if (m.Method.Name == "StartsWith")
        {
            // ...
        }
        if (m.Method.Name == "EndsWith")
        {
            // ...
        }
        if (m.Method.Name == "Contains")
        {
            // ...
        }
        throw new NotSupportedException("A meaningful error message");
    }

    throw new NotSupportedException("A meaningful error message");
}

"LIKE" should be a well named constant.


String.Format or string interpolation is nicer than concatenation:

var value = expressionTreeHelpers.GetValueFromExpression(m.Arguments[0]);
_filter += $"{property.FieldName} LIKE \"%{value}\"";

I'm afraid that's about the limit of what I can suggest at the moment. It seems like a good approach to me but I'm not exactly an expert at this kind of thing! I would suggest putting some search string validation/escaping in as it's generally a good idea to be cautious.

\$\endgroup\$
  • \$\begingroup\$ OMG this pretty much deserves a bounty. The lack of exceptions was causing the extension method to silently return an empty filter string, which made the Select call return every single entity. Thanks! \$\endgroup\$ – Mathieu Guindon Feb 5 '16 at 17:14
  • \$\begingroup\$ @Mat'sMug Glad it helped! :) \$\endgroup\$ – RobH Feb 5 '16 at 18:14
  • \$\begingroup\$ What would be really awesome is if, instead of throwing an exception right away, I would try to resolve the method call to a constant expression, and only throw when that is impossible. That way I wouldn't be throwing on, say, x => x.Description.Contains(foobar.ToLower()); I'd evaluate foobar.ToLower() instead. hmmm... \$\endgroup\$ – Mathieu Guindon Feb 5 '16 at 18:39
2
\$\begingroup\$

Since you're referring to methods on string, you could use the nameof operator:

if (m.Method.DeclaringType == typeof(string))
{
    if (m.Method.Name == nameof(string.StartsWith))
    {
        // ...
    }
    if (m.Method.Name == nameof(string.EndsWith))
    {
        // ...
    }
    if (m.Method.Name == nameof(string.Contains))
    {
        // ...
    }

If/when you add more supported methods, you get the intellisense for the method names as a result.

\$\endgroup\$
  • \$\begingroup\$ Nice one. Too bad I'm not using C# 6 for this. \$\endgroup\$ – Mathieu Guindon Feb 5 '16 at 18:41

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.