Linq-to-Sage Implementation

Following-up on my last question where I wrapped the Sage API with a familiar IRepository interface, I decided to push the abstraction a step further, and... implement an actual LINQ provider.

So my client code looks like this:

class Program
{
    static void Main(string[] args)
    {
        using (var context = new SageContext(/*redacted*/))
        {
            context.Open();
            var orders =
                from po in context.PurchaseOrderHeaders
                where po.Number.Contains("123") && po.Lines > 0
                select po;

            foreach (var po in orders)
            {
                Console.WriteLine("PO Number: {0} ({1:d}) - {2}", po.Number, po.OrderDate, po.Key);
            }
        }
        Console.ReadLine();
    }
}

Looks exactly like it would with Entity Framework? That's intentional. Except... I don't have fancypants navigation properties. Yet (to be honest, not sure I have the slightest idea of how I'd go about this).

So, a bare-bones SageContext looks like this:

public class SageContext : SageContextBase
{
    public SageContext(string userId, string password, string dbName) 
        : base(new SageCredential(userId, password, dbName))
    {
    }

    public ViewSet PurchaseOrderHeaders { get; set; }
    public ViewSet PurchaseOrderHeaderOptionalFields { get; set; }

    protected override void OnModelComposing()
    {
        //PurchaseOrderHeaders.View.Compose(new View[]{ /*compose views here*/ });
    }
}

Every ViewSet property is assigned a reference via reflection, in the base class (does that interfere with IDisposable?) - the SageCredential class isn't secure at all, I just added it because it nicely wraps closely-related values into a single-use object:

```
public abstract class SageContextBase : IDisposable
{
private const string AppId = "ABC";
private const string ProgramName = "ABC1234";
private const string AppVersion = "999";

private readonly SageCredentia

There are a number of missed opportunities that could be considered as bugs here.

Your Execute implementation in SageContextBase is poorly adapted from the MSDN example. Good job on making it generic, but this:

// The expression must represent a query over the data source. 
if (!(expression is MethodCallExpression))
{
    throw new InvalidProgramException("No query over the data source was specified.");
}

..is causing a weird issue. Consider:

var orders = from po in context.PurchaseOrderHeaders select po;

and:

var orders = context.PurchaseOrders;

Are these two snippets equivalent? No! The second one is a ConstantExpression without a MethodCallExpression, which makes your Execute method throw a surprising exception whenever there isn't an explicit projection / Select call.

Additionally, this part:

// Find the call to Where() and get the lambda expression predicate.
    var whereFinder = new InnermostWhereFinder();
    var whereExpression = whereFinder.GetInnermostWhere(expression);
    var lambdaExpression = (LambdaExpression)((UnaryExpression)(whereExpression.Arguments[1])).Operand;

Assumes there's always a Where clause in your query, which you probably don't want to assume, especially if you're simply selecting all rows from a small support view/table.

You can account for the ConstantExpression like this:

var constantExpression = expression as ConstantExpression;
if (constantExpression != null)
{
    if (constantExpression.Value is ViewSet)
    {
        return viewSet.Select(string.Empty);
    }
}

And you can account for a missing Where clause with a simple null check, like this:

var whereFinder = new InnermostWhereFinder();
var whereExpression = whereFinder.GetInnermostWhere(expression);
var filter = string.Empty;
if (whereExpression != null)
{

And now your client code can have this rather intuitive code run without any issues:

var foo = context.SomeEntity.ToList();

You haven't listed the InnermostWhereFinder class, but by the naming of the local variables involved I suspect it's not accounting for other filtering methods, and so these queries would throw potentially surprising exceptions:

var foo = context.Something.Single(s => s.Id == 42);
var foo = context.Something.SingleOrDefault(s => s.Id == 42);
var foo = context.Something.First(s => s.Id == 42);
var foo = context.Something.FirstOrDefault(s => s.Id == 42);
var foo = context.Something.Last(s => s.Id == 42);
var foo = context.Something.LastOrDefault(s => s.Id == 42);
var foo = context.Something.Count(s => s.Foo == "42");
var foo = context.Something.Any();
var foo = context.Something.All(s => s.IsActive);

The provider should delegate anything it can't handle to LINQ-to-Objects, instead of blowing up. Matt Warren has a series of articles on MSDN that go into more details than the little walkthrough you used to do this.

StackExchange Code Review Q#119339, answer score: 3