Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

.Persist().InsertOrUpdate() generates equivalency expression that is always true #40

Closed
alewkowicz opened this issue Jun 13, 2017 · 9 comments

Comments

@alewkowicz
Copy link

alewkowicz commented Jun 13, 2017

This is related to a Stack Overflow post, here: https://stackoverflow.com/questions/44509445/automapper-collection-entityframework-causing-invalidoperationexception-after-pe

I copied the source for the InsertOrUpdate method from your source to my code file, and executed it as a local method. I found that, on inspection, the generated equivalency expression was always true:

image

Those values in the ToolTip are the IDs of the record I'm looking for, but obviously X == X will always be true. This line:

var to = _sourceSet.FirstOrDefault(equivExpr);

... will return the first record in the set. Since most of the time that's not the record I want, AutoMapper overwrites the Id value, which Entity Framework objects to, which generates the error in the Stack Overflow question referenced above.

(I was wrong when I said, in the question, that this only happened with some entity classes. It happens whenever the first entity in the set doesn't match the one I'm applying InsertOrUpdate to.)

The configuration from which the mapper instance is generated looks like this:

collectionConfig = new MapperConfiguration(cfg =>
{
    cfg.AddCollectionMappers();
         
    cfg.SetGeneratePropertyMaps<GenerateEntityFrameworkPrimaryKeyPropertyMaps<TruckTechContext>>();
    cfg.AddProfile(new CollectionProfile(cfg, dbContext));
                
});

The mapping of the relevant class looks like this:

            var vehicleMap = CreateMap<Vehicle, Vehicle>()
                .ForMember(veh => veh.LogVehicles, options => options.Ignore())
                .ForMember(veh => veh.InspectionReports, options => options.Ignore())
            ;

The EF context setup looks like this:

            modelBuilder.Entity<Vehicle>()
                .HasKey(v => v.Id)
                .Property(v => v.Id)
                .HasDatabaseGeneratedOption(System.ComponentModel.DataAnnotations.Schema.DatabaseGeneratedOption.None);

The definition of the ID field is as simple as it gets:

public int Id { get; set; }

But, if it's relevant, the Id is actually defined in a base class for the entity rather than in the Vehicle entity itself.

This is my first time submitting a GitHub issue. Let me know what more you need. Thank you.

@TylerCarlson1
Copy link
Member

TylerCarlson1 commented Jun 13, 2017

Do you have a VehicalDto class or are you trying to Map object to same type of itself?
Automapper doesn't support mapping to itself, as it causes so many problems.

My guess would be you are using the same types in your expression and that's the problem.
In Stack Overflow: "logs" is the output of an AutoMapper.Map of a collection.
What you should be doing is passing the dto into the persist call, not the entity it mapped to. That mapping will happen in the InsertOrUpdate call. Also that would explain why expression is always true. Both sides of the equals are Vehical parameters, so both get translated to their ID instead of v2.Id staying.

@alewkowicz
Copy link
Author

You're right, I was mapping a Vehicle to itself.

But, I'm getting the same problem with the VehicleDTO and the Vehicle. I would guess this is because, although they're different types, both objects derive from a common base class, and the base class is where the Id property is defined.

Does this sound likely? And, if so, is there a way around it?

I don't want to waste your time with unnecessary project detail, but the DTOs aren't true DTOs - they're really translations of a very complex JSON feed into an object graph. The database tables closely mirror them, and there are dozens of scalar data properties in some of the objects. The common base class saved me a lot of redundant definitions.

@TylerCarlson1
Copy link
Member

I would suggest making 2 base types one for Dto and one for Entity and it might work, because at that point they are 2 completely different properties and it shouldn't translate both of them in the expression.

If you can you can keep most of the base class I guess but make sure you have 2 classes with Id that the classes inherit from. If that doesn't work you might have to put ID in every class on the EF side to make it work.

@alewkowicz
Copy link
Author

I experimented some more and found that even though the Vehicle and VehicleDTO had a common base class, if I took the Id field (the key field) out of the base class and declared it separately in the two derived classes, everything worked. I think that's what you were suggesting in your reply.

(I now have the familiar problem of child objects being added to their respective DbSet<> when really they should be updated, but I already knew about that!)

Thank you. I guess you can consider this resolved.

Thanks again.

@TylerCarlson1
Copy link
Member

TylerCarlson1 commented Jun 14, 2017

Yes that was what I was suggesting.

Also AutoMapper.Collections should handle the child object collections for you. I don't know about cfg.AddProfile(new CollectionProfile(cfg, dbContext)); but cfg.AddCollectionMappers(); will insert/update/delete from existing child lists by matching by expression made by Primary Keys just like Vehical to VehicleDTO does.

It's designed so that if you have a single object to persist back, it should handle all inserts/updates/deletes for all it's children and their children's children without having to do any additional configuration other than cfg.SetGeneratePropertyMaps<GenerateEntityFrameworkPrimaryKeyPropertyMaps<TruckTechContext>>(); to tell the system to use Primary Key values to compare DTO and EF objects.

@alewkowicz
Copy link
Author

alewkowicz commented Jun 14, 2017

That's very interesting (and thanks for responding to that!) The situation I had - where I was getting duplicate key errors with a child collection - turned out to be due to duplicate records in the incoming data! So that was a false alarm.

But I have also found a case, with a child collection's child collection, where, if I explicitly load or enumerate the entities from the DBContext.Set<T> before doing my InsertOrUpdate, everything is fine. But if I don't do that, all the entities are set as Added, and I get duplicate key errors on insert.

So far it's just this one child collection where this is happening. It has a single-property primary key:

public int Id { get;set; }

... and I verified that the Id property is not coming from a common base class.

I also verified that all the objects have honest-to-goodness unique key values. As I said, if I call Load on the DbSet<T> property corresponding to the child-of-child class on the DbContext before doing my mapping, all is fine.

@alewkowicz
Copy link
Author

If a child object is updated, such that it's left in the state Unchanged, are its child objects even loaded during mapping? I would have expected they would be - a Customer can get new orders in an Orders collection even if nothing about the Customer information has changed - but now I'm wondering.

@TylerCarlson1
Copy link
Member

They should be loaded as you are mapping to them as well. Why they aren't loading is the question. Maybe you have lazy loading off and not explicitly loading them using Include. Or your relationship is not set up properly (Forgot to add virtual to the collection property in EF?), but if it's working for all the other classes and not that one I think it's something with EF and how you set it up.

Since you said each one is unique by ID for the children I want to point you to this https://stackoverflow.com/questions/11033348/is-it-possible-to-remove-child-from-collection-and-resolve-issues-on-savechanges. This is more to do with removing child objects from the list, where their Primary key needs to include the ID of its parent as well as it's own ID in order to be deleted instead of orphaned when you remove it from the parent's list.

@alewkowicz
Copy link
Author

You nailed it! (Again!) I left "virtual" off the collection property in one class!

I will read that link you included. I appreciate all your help! Thank you very much!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants