Can't access members of objects other than the function parameter

RELEASE-NOTES.md

@@ -6,6 +6,8 @@
 
 * Supports RethinkDB's regular expression matching of strings, eg. ```table.Filter(o => o.Email != null && o.Email.Match(".*@(.*)") != null)```.  [Issue #107](https://github.com/mfenniak/rethinkdb-net/issues/107) & [PR #208](https://github.com/mfenniak/rethinkdb-net/pull/208)
 
+* Expressions can now reference member variables of server-side calculated data, eg. ```table.Filter(o => o.Email.Match(".*@(.*)").Groups[0].MatchedString == "google.com")``` would filter for all records with an e-mail address that has a domain of "google.com".  [PR #209](https://github.com/mfenniak/rethinkdb-net/pull/209)
+
 
 ## 0.10.0.0 (2015-04-14)
 

rethinkdb-net-test/Expressions/GenericTypeConstraintExpressionTests.cs

@@ -1,4 +1,5 @@
-using System.Runtime.Serialization;
+using System;
+using System.Runtime.Serialization;
 using NUnit.Framework;
 using FluentAssertions;
 using RethinkDb.DatumConverters;
@@ -28,12 +29,47 @@ public void TestFixtureSetUp()
         {
             datumConverterFactory = new AggregateDatumConverterFactory(
                 PrimitiveDatumConverterFactory.Instance,
-                DataContractDatumConverterFactory.Instance
+                DataContractDatumConverterFactory.Instance,
+                new TestInterfaceDatumConverterFactory()
             );
             expressionConverterFactory = new RethinkDb.Expressions.DefaultExpressionConverterFactory();
             queryConverter = new QueryConverter(datumConverterFactory, expressionConverterFactory);
         }
 
+        private class TestInterfaceDatumConverterFactory : AbstractDatumConverterFactory
+        {
+            public override bool TryGet<T>(IDatumConverterFactory rootDatumConverterFactory, out IDatumConverter<T> datumConverter)
+            {
+                datumConverter = null;
+                if (rootDatumConverterFactory == null)
+                    throw new ArgumentNullException("rootDatumConverterFactory");
+
+                if (typeof(T) != typeof(ITestInterface))
+                    return false;
+
+                datumConverter = (IDatumConverter<T>)new TestInterfaceDatumConverter();
+                return true;
+            }
+        }
+
+        private class TestInterfaceDatumConverter : AbstractReferenceTypeDatumConverter<ITestInterface>, IObjectDatumConverter
+        {
+            public override ITestInterface ConvertDatum(Datum datum)
+            {
+                throw new System.NotImplementedException();
+            }
+
+            public override Datum ConvertObject(ITestInterface value)
+            {
+                throw new System.NotImplementedException();
+            }
+
+            public string GetDatumFieldName(System.Reflection.MemberInfo memberInfo)
+            {
+                return "number";
+            }
+        }
+
         private static void AssertFunctionIsGetFieldSomeNumberSingleParameter(Term expr)
         {
             var funcTerm =

rethinkdb-net-test/Integration/MultiObjectTests.cs

@@ -1133,5 +1133,20 @@ public void FilterByRegexpMatches()
             }
             count.Should().Be(1);
         }
+
+        [Test]
+        public void FilterByRegexpGroup()
+        {
+            int count = 0;
+            foreach (var row in connection.Run(testTable.Filter(o => 
+                o.Name.Match("^([3])$") != null &&
+                o.Name.Match("^([3])$").Groups[0].MatchedString == "3"
+            )))
+            {
+                row.Name.Should().Be("3");
+                count += 1;
+            }
+            count.Should().Be(1);
+        }
     }
 }

rethinkdb-net/Expressions/BaseExpression.cs

@@ -167,8 +167,12 @@ protected Term SimpleMap(IDatumConverterFactory datumConverterFactory, Expressio
                     DefaultExpressionConverterFactory.ExpressionMappingDelegate<MemberExpression> memberAccessMapping;
                     if (expressionConverterFactory.TryGetMemberAccessMapping(member, out memberAccessMapping))
                         return memberAccessMapping(memberExpression, RecursiveMap, datumConverterFactory, expressionConverterFactory);
-                    else
-                        return AttemptClientSideConversion(datumConverterFactory, expr);
+
+                    Term serverSideTerm;
+                    if (ServerSideMemberAccess(datumConverterFactory, memberExpression, out serverSideTerm))
+                        return serverSideTerm;
+
+                    return AttemptClientSideConversion(datumConverterFactory, expr);
                 }
 
                 case ExpressionType.Conditional:
@@ -192,6 +196,52 @@ protected Term SimpleMap(IDatumConverterFactory datumConverterFactory, Expressio
             }
         }
 
+        private bool ServerSideMemberAccess(IDatumConverterFactory datumConverterFactory, MemberExpression memberExpression, out Term term)
+        {
+            term = null;
+
+            if (memberExpression.Expression == null)
+                // static member access; can't do that server-side, wouldn't want to either.
+                return false;
+
+            if (memberExpression.Expression.NodeType == ExpressionType.Constant)
+                // Accessing a constant; client-side conversion will be more efficient since we won't be round-tripping the object
+                return false;
+
+            IDatumConverter datumConverter;
+            if (!datumConverterFactory.TryGet(memberExpression.Expression.Type, out datumConverter))
+                // No datum converter for this type.
+                return false;
+
+            var fieldConverter = datumConverter as IObjectDatumConverter;
+            if (fieldConverter == null)
+                // doesn't implement IObjectDatumConverter, so we don't know how to map the MemberInfo to the field name
+                return false;
+
+            var datumFieldName = fieldConverter.GetDatumFieldName(memberExpression.Member);
+            if (string.IsNullOrEmpty(datumFieldName))
+                // At this point we're not returning false because we're expecting this should work; throwing an error instead.
+                throw new NotSupportedException(String.Format("Member {0} on type {1} could not be mapped to a datum field", memberExpression.Member.Name, memberExpression.Type));
+
+            var getAttrTerm = new Term()
+            {
+                type = Term.TermType.GET_FIELD
+            };
+            getAttrTerm.args.Add(RecursiveMap(memberExpression.Expression));
+            getAttrTerm.args.Add(new Term()
+            {
+                type = Term.TermType.DATUM,
+                datum = new Datum()
+                {
+                    type = Datum.DatumType.R_STR,
+                    r_str = datumFieldName
+                }
+            });
+
+            term = getAttrTerm;
+            return true;
+        }
+
         private Term AttemptClientSideConversion(IDatumConverterFactory datumConverterFactory, Expression expr)
         {
             try
@@ -205,7 +255,11 @@ private Term AttemptClientSideConversion(IDatumConverterFactory datumConverterFa
             }
             catch (InvalidOperationException ex)
             {
-                throw new InvalidOperationException("Failed to perform client-side evaluation of expression tree node; often this is caused by refering to a server-side variable in a node that is only supported w/ client-side evaluation", ex);
+                throw new InvalidOperationException(
+                    String.Format(
+                        "Failed to perform client-side evaluation of expression tree node '{0}'; this is caused by refering to a server-side variable in an expression tree that isn't convertible to ReQL logic",
+                        expr),
+                    ex);
             }
         }
 

rethinkdb-net/Expressions/SingleParameterLambda.cs

@@ -118,79 +118,29 @@ private Term MapExpressionToTerm(Expression expr)
                     };
                 }
 
-                case ExpressionType.MemberAccess:
+                case ExpressionType.Convert:
                 {
-                    var memberExpr = (MemberExpression)expr;
-                    var member = memberExpr.Member;
-
-                    if (memberExpr.Expression == null)
-                    {
-                        return SimpleMap(datumConverterFactory, expr);
-                    }
-                    else if (memberExpr.Expression.NodeType == ExpressionType.Convert)
-                    {
-                        // In some cases the CLR can insert a type-cast when a generic type constrant is present on a
-                        // generic type that's a parameter.  We pretty much just ignore those casts.  It might be
-                        // valid to use the cast to switch to a different datum converter?, but the use-case isn't
-                        // really clear right now.  We do check that the type-cast makes sense for the parameter type,
-                        // but it's just to feel safer; it seems like the compiler should've made sure about that.
-
-                        var convertExpression = (UnaryExpression)memberExpr.Expression;
-                        if (convertExpression.Operand.NodeType != ExpressionType.Parameter)
-                            return SimpleMap(datumConverterFactory, expr);
-
-                        var parameterExpression = (ParameterExpression)convertExpression.Operand;
-                        if (!convertExpression.Type.IsAssignableFrom(parameterExpression.Type))
-                            throw new NotSupportedException(String.Format(
-                                "Cast on parameter expression not currently supported (from type {0} to type {1})",
-                                parameterExpression.Type, convertExpression.Type));
-                    }
-                    else if (memberExpr.Expression.NodeType != ExpressionType.Parameter)
-                    {
+                    // In some cases the CLR can insert a type-cast when a generic type constrant is present on a
+                    // generic type that's a parameter.  We pretty much just ignore those casts.  It might be
+                    // valid to use the cast to switch to a different datum converter?, but the use-case isn't
+                    // really clear right now.  We do check that the type-cast makes sense for the parameter type,
+                    // but it's just to feel safer; it seems like the compiler should've made sure about that.
+
+                    var convertExpression = (UnaryExpression)expr;
+                    if (convertExpression.Operand.NodeType != ExpressionType.Parameter)
+                        // If this isn't a cast on a Parameter, just drop it through to do continue processing.
                         return SimpleMap(datumConverterFactory, expr);
-                    }
 
-                    DefaultExpressionConverterFactory.ExpressionMappingDelegate<MemberExpression> memberAccessMapping;
-                    if (expressionConverterFactory.TryGetMemberAccessMapping(member, out memberAccessMapping))
-                        return memberAccessMapping(memberExpr, RecursiveMap, datumConverterFactory, expressionConverterFactory);
+                    // Otherwise; type-check it, and then just strip the Convert node out and recursivemap the inside.
+                    var parameterExpr = (ParameterExpression)convertExpression.Operand;
+                    if (!convertExpression.Type.IsAssignableFrom(parameterExpr.Type))
+                        throw new NotSupportedException(String.Format(
+                            "Cast on parameter expression not currently supported (from type {0} to type {1})",
+                            parameterExpr.Type, convertExpression.Type));
 
-                    var getAttrTerm = new Term() {
-                        type = Term.TermType.GET_FIELD
-                    };
-
-                    getAttrTerm.args.Add(new Term() {
-                        type = Term.TermType.VAR,
-                        args = {
-                            new Term() {
-                                type = Term.TermType.DATUM,
-                                datum = new Datum() {
-                                    type = Datum.DatumType.R_NUM,
-                                    r_num = 2
-                                },
-                            }
-                        }
-                    });
-
-                    var datumConverter = datumConverterFactory.Get<TParameter1>();
-                    var fieldConverter = datumConverter as IObjectDatumConverter;
-                    if (fieldConverter == null)
-                        throw new NotSupportedException("Cannot map member access into ReQL without implementing IObjectDatumConverter");
-
-                    var datumFieldName = fieldConverter.GetDatumFieldName(memberExpr.Member);
-                    if (string.IsNullOrEmpty(datumFieldName))
-                        throw new NotSupportedException(String.Format("Member {0} on type {1} could not be mapped to a datum field", memberExpr.Member.Name, memberExpr.Type));
-
-                    getAttrTerm.args.Add(new Term() {
-                        type = Term.TermType.DATUM,
-                        datum = new Datum() {
-                            type = Datum.DatumType.R_STR,
-                            r_str = datumFieldName
-                        }
-                    });
-
-                    return getAttrTerm;
+                    return RecursiveMap(parameterExpr);
                 }
-
+                
                 default:
                     return SimpleMap(datumConverterFactory, expr);
             }
@@ -199,4 +149,3 @@ private Term MapExpressionToTerm(Expression expr)
         #endregion
     }
 }
-