HenkKin / EntityCloner.Microsoft.EntityFrameworkCore

Cloning entities using Microsoft.EntityFrameworkCore
21 stars 3 forks source link

unable to cast hashset to ilist exception #7

Closed vigouredelaruse closed 1 year ago

vigouredelaruse commented 1 year ago

hello - given

cloneasync() throws the following

   at EntityCloner.Microsoft.EntityFrameworkCore.DbContextExtensions.InternalCloneCollection(DbContext source, Dictionary`2 references, Object parentEntity, Type collectionItemType, String definingNavigationName, IReadOnlyEntityType definingEntityType, IEnumerable collectionValue)
   at EntityCloner.Microsoft.EntityFrameworkCore.DbContextExtensions.ResetNavigationProperties(DbContext source, Object entity, String definingNavigationName, IReadOnlyEntityType definingEntityType, Dictionary`2 references, Object clonedEntity)
   at EntityCloner.Microsoft.EntityFrameworkCore.DbContextExtensions.InternalClone(DbContext source, Object entity, String definingNavigationName, IReadOnlyEntityType definingEntityType, Dictionary`2 references)
   at EntityCloner.Microsoft.EntityFrameworkCore.DbContextExtensions.<CloneAsync>d__3`1.MoveNext()
   at TheHorselessNewspaper.HostingModel.ContentEntities.Query.ContentCollections.ContentModelQueries`1.<Clone>d__20.MoveNext() in E:\src\horseless-core\src\entities\Model.Core.Taxa\TheHorselessNewspaper.Schemas.HostingModel\ContentEntities\Query\ContentCollections\ContentModelQueries.cs:line 589

please advise

HenkKin commented 1 year ago

Maybe you can try

public ICollection<AccessControlEntry> AccessControlEntries { get; set; } = new Collection<AccessControlEntry>();
vigouredelaruse commented 1 year ago

howdy; unfortunately i'd like to apply this functionality in a headless cms app development framework, where developers will extend the dbcontext entity model after the product is shipped; i won't be in control of how they model relationships if you see what i'm saying

i'm having some success with a fork https://github.com/HenkKin/EntityCloner.Microsoft.EntityFrameworkCore/compare/master...vigouredelaruse:EntityCloner.Microsoft.EntityFrameworkCore:ilist-replacement

i changed all the relationships in the test set to use hashset except for one and made some changes to the collection detection and population workflow

the changes i made currently pass most of the original unit tests except for ireadonly

unfortunately the unit tests for my own code currently clone everything except for many sided relationships; hoping to get that working today

vigouredelaruse commented 1 year ago

howdy - re these commits

question - regarding deep graph primary key and concurrency token reset my own test set graph resembles entity->relation->furtherNavigation->evenDeeperInThegraph

the ids and concurrency tokens at the root of the graph are reset but nowhere else. is that what's supported by the library? or does the library support such resets everywhere in the cloned graph?

in this line of thinking it might be useful to expose a delegate that lets consumers of the library apply customizations of the graph property reset

reason being the graphs i want to clone need case by case customization of the cloned graph

thanks

HenkKin commented 1 year ago

I think it can be much simpler. First I added test with hashset, it was failing with same exception. Then I fixed it. Test is now succeeded.

Here my changes

If this solves your problem, I can create a new release.

The reset is based on entity configuration and your includes. It should work in the entire (nested) graph. The idea is that only included entities are reset. Not included entities stay the same, so there id's should not be reset. Even foreign keys to not included entities must not be reset.

vigouredelaruse commented 1 year ago

thanks so much for looking into this. i'd hate to have to write this code from scratch

i took a look at your changes and i needed to make some further adjustments to get both test sets to complete

the summary is based around this difference in the way navigations are discovered. without this modification i get a shallow clone

            var entityName = entity.GetType().Name;
            var model = source.Model.FindEntityType(entity.GetType());
            if (model != null)
            {
                IEnumerable<ISkipNavigation> skipNavigations = model.GetSkipNavigations();

vs this

          foreach (var navigation in source.FindCurrentEntityType(entity.GetType(), definingNavigationName, definingEntityType).GetNavigations())

there is currently no support for applying configurations during dynamic discovery so the model is pure convention and the dbcontext has an incomplete set of static references to entities

my changes are rather verbose but if you like we can fold them in with a pr

thanks

HenkKin commented 1 year ago

I added your changes to my branch . I also changed some minor fixes. I also added some unittests (with many to many relations (skiprelations) which reflects these changes.

I personally never use DbSets on DbContext as properties (https://github.com/HenkKin/DataAccessClient/blob/master/DataAccessClientExample/DataLayer/ExampleDbContext.cs). I Always do it dynamically and have no problems with this library.

Are these changes working for you. Or are you missing some functionality?

Otherwise I want to create a new version.

vigouredelaruse commented 1 year ago

morning - so was able to take a quick look but there was still some issue with skip navigations not being populated

i will investigate today

vigouredelaruse commented 1 year ago

so, given your most recent commits to this branch

i think we've investigated the original scenario with enough rigour to advance thanks

HenkKin commented 1 year ago

Multi target project is under construction. It is almost ready with unittesting. After that i want to release a New version.

HenkKin commented 1 year ago

I published version 7.1.0 of this package.