Feature Discussion about graphdiff HOT 3 CLOSED

Hi,

yes, there is an easier way. The mapping really is just a tree of GraphNodes representing what's normally specified using expressions. Of course you can replace the ConfigurationVisitor for your scenario and create GraphNodes directly, bypassing expressions and the ConfigurationVisitor entirely. Issue #9 proposes to do this using attributes on navigation properties.

As I'm not maintaining this repository I can't give you a definitive statement regarding the desirability of the proposed features. Personally I think the loading part could be a bit out-of-scope for GraphDiff while the async feature might be nice to have.

from graphdiff.

Finally circling back to this - had time to pick apart the ConfigurationVisitor.

Although the MVC-specific use case is definitely outside the scope of EFGraphDiff, the more I think about it the more the following seems in scope - at least to me - and I'm interested in thoughts:

1. Loading Entities using the same *Configuration as one would use to Update them.
2. Creating Blank Entities using the same *Configuration
3. Deleting Entities using the same *Configuration

For now I'm focused solely on (1), but I'm interested in feedback as to whether others would agree loading an Entity using this config seems in-bounds for this library. Since it's C# it's not like the increased code is harmful to implementers, and MVC is just one case where it'd have a lot of use.

To get more detailed, after looking at the code I see the ConfigurationVisitor loops over the entire Expression/method body to determine the kinds of collections that need to be saved. In other words, the way UpdateGraph() works, it requires an Expression inside an Expression like:

x => x
.OwnedCollection(m => m.Urls)
.OwnedCollection(m => m.ContactInfos)

It does this because the work it's going to do needs a List like this:

List<Expression<Func<TModel, TItem>>>

with strong types for TItem - differing types - all mixed into the same List, which would obviously be illegal in C#. So instead you have this Expression that's a sequence of statements, which the ConfigurationVisitor then loops over via Reflection and gathers these method call names and types, and determines handing that off to do the proper work of updating the appropriate collections and properties.

Assuming EFGraphDiff doesn't move to attributes as you mentioned, it would appear the best way to accomplish the Loading functionality I'd like would be to:

1. Expose the ConfigurationVisitor to the outside. This means taking it out of Internal, which is the main reason I want to check here first before I proceed.
2. Offer some extension point(s) in ConfigurationVisitor so the current functionality where it then creates the various OwnedEntityGraphNode, etc is just one potential use of it.
3. Use those extension point(s) internally to ensure the current functionality remains the same.
4. Use those extension point(s) in my code, which I could contribute back to EFGraphDiff or keep outside it, or.... something else? To accomplish this automated loading functionality.

I may just do this anyway and end up with my own fork that's never merged, but obviously that kind of forking can mean my version drifts farther and farther away from the trunk, so I want to check with the community here before I proceed.

from graphdiff.

Hi, I had a few hours this afternoon so I decided to give it a go. I like the suggestions, I've put in a PR to show the proposed solution. It's not finished but just an early look to get thoughts and suggestions. Can you take a look? https://github.com/refactorthis/GraphDiff/pull/58
cheers.

p.s. should also mention I'm thinking of adding a class whicih will allow you to register the default mappings for aggregates at app start which will be the default if provided no mapping configuration to UpdateGraph() (sort of like fluent api) There is also a Select for aggregates in the POC.

from graphdiff.