0

I had a similar issue with this question, when updating a record using EF6.

I really thought I had cracked the whole updating thing, but now have to almost identical functions updating in what I think was an identical way. One works, the other doesn't. I have fixed the one that doesn't work by using Jamie's comment in the above question, but I'd like to understand if the function that works, really shouldn't and so is on borrowed time and I should make more like the 'fixed' one. Or, why the 'fixed' one didn't work in the first place. I even moved them into the same controller so that the database (DB) context was guaranteed the same. Have I missed something and they are not identical (functionally) at all?

It might also help some others out there that struggle with this as I did.

The function that works (cut down) is:

        [HttpPost]
    [Route("UpdateAddBusinessService")]
    public async Task<IHttpActionResult> UpdateAddBusinessService(BusinessServiceDTO servicetoupdateoradd)
    {

        ... pre check stuff...

        try
        {
            if (servicetoupdateoradd.Id != null) // This is an existing service to be updated - if Is Null then create new
            {
                BusinessService businessService = await db.BusinessServices.FindAsync(servicetoupdateoradd.Id);
                if (businessService != null)
                {
                    Mapper.Map(servicetoupdateoradd, businessService);
                    db.Entry(businessService).State = EntityState.Modified;
                    await db.SaveChangesAsync();
                    return Ok("Service Updated");
                }
                else

The function that doesn't work is:

        [HttpPost]
    [Route("UpdateImage")]
    public async Task<IHttpActionResult> UpdateImage(ImageDTO imageDTO)
    {
        ... pre check stuff ... 
        try
        {
            // First find the image
            // Image imagetoupdate = await db.Images.FindAsync(imageDTO.Id);  <<-This FAILS.
            Image imagetoupdate = db.Images.AsNoTracking().Single(x => x.Id == imageDTO.Id);  <<- This WORKS
            if (imagetoupdate != null)
            {
                imagetoupdate = Mapper.Map<ImageDTO, Image>(imageDTO); // Move the stuff over..
                db.Entry(imagetoupdate).State = EntityState.Modified;
                await db.SaveChangesAsync();
                return Ok();
            }

I wondered (as you will no doubt), if my Mapper function was doing anything, but I suspect not (without digging too deep, but I guess it could be), my Mapper.Config functions for the two DTO's are very similar:

            cfg.CreateMap<Image, ImageDTO>();
            cfg.CreateMap<ImageDTO, Image>();

and:

            cfg.CreateMap<BusinessService, BusinessServiceDTO>();
            cfg.CreateMap<BusinessServiceDTO, BusinessService>();

I would really just like to understand the 'correct' way of doing this so it doesn't bite me again. Thanks in advance.

EDIT: I was asked (quite reasonably) if the 'pre-check stuff' does anything to fetch the data, it doesn't, but there is a subtle difference, that I might have missed... This is from the BusinessService function that works:

        if (!ModelState.IsValid)
        {
            return BadRequest(ModelState);
        }
        string userid = User.Identity.GetUserId(); //Check user is valid
        if (servicetoupdateoradd.UserId != userid)
        {
            var message = "User Id Not found - Contact support";
            HttpResponseMessage err = new HttpResponseMessage() { StatusCode = HttpStatusCode.ExpectationFailed, ReasonPhrase = message };
            return ResponseMessage(err);
        }

This is from the UpdateImage function that didn't work:

        if (!ModelState.IsValid)
        {
            return BadRequest(ModelState);
        }
        string userid = User.Identity.GetUserId();
        SGGUser user = db.Users.Find(userid); // Use this find and not UserManager becuase its a different context and buggers up the file save
        if (user == null)
        {
            var message = "User Id Not found - Contact support";
            HttpResponseMessage err = new HttpResponseMessage() { StatusCode = HttpStatusCode.ExpectationFailed, ReasonPhrase = message };
            return ResponseMessage(err);
        }

I see that in this one, though I don't fetch the relevant data, I do use the 'db' context.. could that be it?? The Image object does contain a reference to the user, so maybe that does some magic in the background code?

Appologies, I just didn't want to clutter up the question too much...

Brett JB
  • 687
  • 7
  • 24

1 Answers1

3

This line:

Mapper.Map(servicetoupdateoradd, businessService);

and this line:

imagetoupdate = Mapper.Map<ImageDTO, Image>(imageDTO); // Move the stuff over..

look similar, but do two different things.

The first line will tell Automapper to copy values from the first object over to the second object reference using the mapping rules. The second line will tell Automapper to make a completely new reference to the entity with the mapped over values from the provided object and return it.

So in the first case, the entity reference is preserved to the one the DbContext knows about. The reference was loaded from a DbContext and should be tracking changes so you shouldn't even need to set it's entity state. In the second case, Automapper is creating an entirely new reference and assigning it over top the original reference. EF is treating that as a completely new instance and trying to attach it, resulting in it complaining because the context had already loaded that entity, you just overwrote the reference.

It should work if you change the second instance to:

Mapper.Map(imageDTO, imagetoupdate);
Steve Py
  • 26,149
  • 3
  • 25
  • 43
  • Probably could also work `businessService.State = EntityState.Detached;` to force the datacontext forgettting about it, but I like the "create a new object" approach. – Cleptus Oct 31 '19 at 11:21
  • Thank you, yes, this was the issue. I hadn't realised that using Mapper that way had a different outcome. Cheers. – Brett JB Oct 31 '19 at 14:59