Code Review

Author: JonathanMagnan

GraphDiff/GraphDiff.Shared/IUpdateConfiguration.cs

@@ -89,7 +89,16 @@ public static IUpdateConfiguration<T> AssociatedCollection<T, T2>(this IUpdateCo
             return config;
         }
 
-        // NEED TEXT!
+        /// <summary>
+        /// States that the child collection is not a part of the aggregate. The parent's navigation property will be updated, but entity changes to the
+        /// child entities will not be saved.
+        /// </summary>
+        /// <typeparam name="T">The parent entity type</typeparam>
+        /// <typeparam name="T2">The child entity type </typeparam>
+        /// <param name="config">The configuration mapping</param>
+        /// <param name="expression">An expression specifying the child entity</param>
+        /// <param name="navExpression">An navigation expression specifying the parent entity</param>
+        /// <returns>Updated configuration mapping</returns>
         public static IUpdateConfiguration<T> AssociatedCollection<T, T2>(this IUpdateConfiguration<T> config, Expression<Func<T, ICollection<T2>>> expression, Expression<Func<T2, T>> navExpression)
         {
             return config;

GraphDiff/GraphDiff.Shared/Internal/ConfigurationVisitor.cs

@@ -13,7 +13,7 @@ internal class ConfigurationVisitor<T> : ExpressionVisitor
     {
         private GraphNode _currentMember;
         private string _currentMethod = "";
-        private bool isAssociatedCollectionWithNav = false;
+        private bool _isAssociatedCollectionWithNav;
 
         public GraphNode GetNodes(Expression<Func<IUpdateConfiguration<T>, object>> expression)
         {
@@ -27,19 +27,18 @@ protected override Expression VisitMember(MemberExpression memberExpression)
         {
             var accessor = GetMemberAccessor(memberExpression);
 
-            if (isAssociatedCollectionWithNav)
-			{
+            if (_isAssociatedCollectionWithNav)
+            {
                 _currentMember.AccessorCyclicNavigationProperty = accessor;
             }
-			else
-			{ 
+            else
+            {
                 var newMember = CreateNewMember(accessor);
 
                 _currentMember.Members.Push(newMember);
                 _currentMember = newMember;
             }
 
-
             return base.VisitMember(memberExpression);
         }
 
@@ -50,27 +49,30 @@ protected override Expression VisitMethodCall(MethodCallExpression expression)
             if (_currentMethod.Equals("AssociatedCollection") && expression.Arguments.Count == 3)
 			{  
                 Visit(expression.Arguments[1]);
-                isAssociatedCollectionWithNav = true;
+
                 try
-				{ 
+                {
+                    _isAssociatedCollectionWithNav = true;
                     Visit(expression.Arguments[2]);
-                    isAssociatedCollectionWithNav = false;
                 }
-                catch (Exception e)
-				{ 
-                    isAssociatedCollectionWithNav = false;
+                catch
+                {
                     throw;
-                } 
+                }
+                finally
+                {
+                    _isAssociatedCollectionWithNav = false;
+                }
             }
             else
-			{ 
+            {
                 // go left to right in the subtree (ignore first argument for now)
                 for (int i = 1; i < expression.Arguments.Count; i++)
                 {
                     Visit(expression.Arguments[i]);
                 }
             }
-             
+
             // go back up the tree and continue
             _currentMember = _currentMember.Parent;
             return Visit(expression.Arguments[0]);

GraphDiff/GraphDiff.Shared/Internal/Graph/GraphNode.cs

@@ -17,7 +17,7 @@ internal class GraphNode
         public GraphNode Parent { get; private set; }
         public Stack<GraphNode> Members { get; private set; }
         public bool? AllowDelete { get; set; }
-        
+
         protected readonly PropertyInfo Accessor;
 
         internal PropertyInfo AccessorCyclicNavigationProperty;
@@ -106,7 +106,7 @@ protected static IEnumerable<string> GetRequiredNavigationPropertyIncludes(DbCon
                     .Select(navigationProperty => ownIncludeString + "." + navigationProperty.Name);
         }
 
-        protected static void AttachCyclicNavigationProperty(IObjectContextAdapter context, object parent, object child, PropertyInfo navProperty = null)
+        protected static void AttachCyclicNavigationProperty(IObjectContextAdapter context, object parent, object child, PropertyInfo parentNavigationProperty = null)
         {
             if (parent == null || child == null) return;
 
@@ -115,18 +115,13 @@ protected static void AttachCyclicNavigationProperty(IObjectContextAdapter conte
 
             var navigationProperties = context.GetNavigationPropertiesForType(childType);
 
-            PropertyInfo parentNavigationProperty = null;
-
-            if (navProperty != null)
+            if (parentNavigationProperty == null)
 			{
-                parentNavigationProperty = navProperty;
-            }
-            else
-			{ 
+                // IF not parent property is specified, we take the first one if we found something
                 parentNavigationProperty = navigationProperties
-                      .Where(navigation => navigation.TypeUsage.EdmType.Name == parentType.Name)
-                      .Select(navigation => childType.GetProperty(navigation.Name, BindingFlags.Instance | BindingFlags.NonPublic | BindingFlags.Public))
-                      .FirstOrDefault();
+                    .Where(navigation => navigation.TypeUsage.EdmType.Name == parentType.Name)
+                    .Select(navigation => childType.GetProperty(navigation.Name, BindingFlags.Instance | BindingFlags.NonPublic | BindingFlags.Public))
+                    .FirstOrDefault();
             }
 
             if (parentNavigationProperty != null)

GraphDiff/Z.EntityFrameworkGraphDiff.labEF6/Request_ManyNav.cs

@@ -51,8 +51,10 @@ public static void Execute()
 				//		with.OwnedCollection(p => p.EntitySimples)));
 				//	context.UpdateGraph(entity, map => map.AssociatedEntity(c => c.EntitySimpleChild).AssociatedCollection(c => c.EntitySimpleLists, x => x.B));
 
+				//context.UpdateGraph(entity, map => map.AssociatedCollection(c => c.EntitySimpleLists).AssociatedEntity(c => c.EntitySimpleChild));
+
 				context.UpdateGraph(entity, map => map.AssociatedCollection(c => c.EntitySimpleLists, x => x.B).AssociatedEntity(c => c.EntitySimpleChild));
-			
+
 				//context.UpdateGraph(entity, map => map.AssociatedEntity(c => c.EntitySimpleChild).AssociatedCollection(c => c.EntitySimpleLists));
 				//context.UpdateGraph(entity, map => map.OwnedEntity(c => c.EntitySimpleChild).OwnedCollection(c => c.EntitySimpleLists));
 				context.SaveChanges();