skip to Main Content

I have the following function that takes a BookForUpdateDto and updates the Book entity based on the updated properties. When I try to update items inside of a ICollection of the Book it works all right when adding one item or when removing one item, but when calling these operations right after another (first one call where one item from the collection is removed, then one call where another item is added), I get an exception on SaveChangesAsync() telling me "The database operation was expected to affect 1 row(s), but actually affected 0 row(s);".

This is my code:

    public async Task UpdateBookAsync(string email, BookForUpdateDto bookUpdateDto)
    {
        var user = await _userRepository.GetAsync(email, trackChanges: true);
        var book = user.Books.SingleOrDefault(book => book.BookId == bookUpdateDto.Guid);
        if (book == null)
        {
            const string message = "No book with this id exists";
            throw new CommonErrorException(404, message, 4);
        }
        await _bookRepository.LoadRelationShipsAsync(book);

        var dtoProperties = bookUpdateDto.GetType().GetProperties();
        foreach (var dtoProperty in dtoProperties)
        {
            // Manually handle certain properties
            switch (dtoProperty.Name)
            {
                // ...
                case "Highlights":
                {
                    Collection<Highlight> newHighlights = new();
                    foreach(var highlightInDto in bookUpdateDto.Highlights)
                    {
                        newHighlights.Add(_mapper.Map<Highlight>(highlightInDto));
                    }

                    book.Highlights = newHighlights;
                    continue;
                }
            }
            
            // Update any other property via reflection
            var value = dtoProperty.GetValue(bookUpdateDto);
            SetPropertyOnBook(book, dtoProperty.Name, value);
        }

        await _bookRepository.SaveChangesAsync();  // <-- Error is thrown here
    }

2

Answers


  1. If You add the new Highlights, updating directly _bookRepository.Highlights table (not books Collection), You can add every new row, without worry about Optimistic Concurrency. You can do:

     ...
    case "Highlights":
    {
       Collection<Highlight> newHighlights = new();
       foreach(var highlightInDto in bookUpdateDto.Highlights)
       {
            newHighlights.Add(_mapper.Map<Highlight>(highlightInDto));
       }
    
       foreach (Highlight hl in newHighlights)
       {
            hl.BookId = book.BookId;
            _bookRepository.Highlights.Add(hl);
       }    
       _bookRepository.SaveChanges();
       continue;
    }
    
    Login or Signup to reply.
  2. There are a few issues with this approach. The main one is trying to replace the child collection references by creating a new collection:

    foreach(var highlightInDto in bookUpdateDto.Highlights)
    {
        newHighlights.Add(_mapper.Map<Highlight>(highlightInDto));
    }
    
    book.Highlights = newHighlights;
    

    With singular references you replace items with instances tracked by the DbContext, with collections you need to add and remove references.

    So for instance, if your book has a collection of Highlights and you want to replace these values with a new set, you need to determine which highlights need to be added, and which might need to be replaced. If Highlights are references to existing records in a table as opposed to unique children "owned" by each book, then you need to load those references to the highlights to be added from the DbContext and associate them with the book.

    To address updating the Highlights on the book, assuming Highlights are references:

    var updatedHighlightIds = bookUpdateDto.Highlights
        .Select(x => x.HighlightId);
    var existingHighlightIds = book.Highlights
        .Select(x => x.HighlightId);
    var highlightIdsToAdd = updatedHighlightIds
        .Except(existingHighlightIds)
        .ToList();
    var highlightIdsToRemove = existingHighlightIds
        .Except(updatedHighlightIds)
        .ToList();
    
    foreach(var highlightId in highlightIdsToRemove)
    {
        book.Highlights.Remove(book.Highlights.First(x => x.HighlightId == highlightId));
    }
    
    if (highlightIdsToAdd.Any())
    {
        var highlights = await _context.Highlights
            .Where(x => highlightIdsToAdd.Contains(x.HighlightId))
            .ToListAsync();
        foreach(var highlight in highlights)
        {
            book.Highlights.Add(highlight);
        }
    }
    

    If instead highlights are something to be created as new references, owned solely by each book, then you could use a similar approach, but instead of fetching the highlights from the DbContext, new up the highlights that need to be added. In these cases the matching of highlights might not be done based on Highlight ID but rather something that uniquely identifies them.

    I do not recommend using per-entity Repository patterns such as the Generic Repository with EF. Patterns like this add no real value when working with EF and impose what can be significant limitations to what you can do. It isn’t clear what this method does:

    await _bookRepository.LoadRelationShipsAsync(book);
    

    but I am assuming it will explicitly load the Highlights and other relation references for the given book.

    Code like this:

    var user = await _userRepository.GetAsync(email, trackChanges: true);
    var book = user.Books.SingleOrDefault(book => book.BookId == bookUpdateDto.Guid);
    

    is particularly wasteful, as you are loading a user, then lazy-loading all of the Books for that user. Alternatively, using the DbContext/DbSet and navigation properties, this could be replaced with just:

    var book = await _context.Books
        .Include(x => x.Highlights)
        .SingleOrDefaultAsync(x => x.BookId == bookUpdateDto.Guid && x.User.Email == email);
    

    This would fetch just the single book and it’s highlights in one query hit. If you needed to include several relations you can append .AsSplitQuery() to address the possibility of a large Cartesian Product in the resulting single query.

    Login or Signup to reply.
Please signup or login to give your own answer.
Back To Top
Search