From a3723d8b25b908e4848165d988bff0e0d3892c87 Mon Sep 17 00:00:00 2001 From: Jonathan Magnan Date: Wed, 23 Sep 2020 08:56:18 -0400 Subject: [PATCH] Code Review --- .../GraphDiff.Shared/IUpdateConfiguration.cs | 11 ++++++- .../Internal/ConfigurationVisitor.cs | 32 ++++++++++--------- .../Internal/Graph/GraphNode.cs | 19 ++++------- .../Request_ManyNav.cs | 4 ++- 4 files changed, 37 insertions(+), 29 deletions(-) diff --git a/GraphDiff/GraphDiff.Shared/IUpdateConfiguration.cs b/GraphDiff/GraphDiff.Shared/IUpdateConfiguration.cs index 3ae4349..41ace6c 100644 --- a/GraphDiff/GraphDiff.Shared/IUpdateConfiguration.cs +++ b/GraphDiff/GraphDiff.Shared/IUpdateConfiguration.cs @@ -89,7 +89,16 @@ public static IUpdateConfiguration AssociatedCollection(this IUpdateCo return config; } - // NEED TEXT! + /// + /// 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. + /// + /// The parent entity type + /// The child entity type + /// The configuration mapping + /// An expression specifying the child entity + /// An navigation expression specifying the parent entity + /// Updated configuration mapping public static IUpdateConfiguration AssociatedCollection(this IUpdateConfiguration config, Expression>> expression, Expression> navExpression) { return config; diff --git a/GraphDiff/GraphDiff.Shared/Internal/ConfigurationVisitor.cs b/GraphDiff/GraphDiff.Shared/Internal/ConfigurationVisitor.cs index 58688f0..d0cc228 100644 --- a/GraphDiff/GraphDiff.Shared/Internal/ConfigurationVisitor.cs +++ b/GraphDiff/GraphDiff.Shared/Internal/ConfigurationVisitor.cs @@ -13,7 +13,7 @@ internal class ConfigurationVisitor : ExpressionVisitor { private GraphNode _currentMember; private string _currentMethod = ""; - private bool isAssociatedCollectionWithNav = false; + private bool _isAssociatedCollectionWithNav; public GraphNode GetNodes(Expression, 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]); diff --git a/GraphDiff/GraphDiff.Shared/Internal/Graph/GraphNode.cs b/GraphDiff/GraphDiff.Shared/Internal/Graph/GraphNode.cs index 5cf85a2..5a6255e 100644 --- a/GraphDiff/GraphDiff.Shared/Internal/Graph/GraphNode.cs +++ b/GraphDiff/GraphDiff.Shared/Internal/Graph/GraphNode.cs @@ -17,7 +17,7 @@ internal class GraphNode public GraphNode Parent { get; private set; } public Stack Members { get; private set; } public bool? AllowDelete { get; set; } - + protected readonly PropertyInfo Accessor; internal PropertyInfo AccessorCyclicNavigationProperty; @@ -106,7 +106,7 @@ protected static IEnumerable 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) diff --git a/GraphDiff/Z.EntityFrameworkGraphDiff.labEF6/Request_ManyNav.cs b/GraphDiff/Z.EntityFrameworkGraphDiff.labEF6/Request_ManyNav.cs index 5f9c05d..a92b007 100644 --- a/GraphDiff/Z.EntityFrameworkGraphDiff.labEF6/Request_ManyNav.cs +++ b/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();