0
votes

i have problem that i cant solve. This is the scenario. i have two models, the 1st one is Question and the 2nd one is Answer. Also, i have DAL, BusinessEntities(view model) and BusinessServices class libraries with UnitOfWork and GenericRepository pattern. Problem is related to Edit action in that way when i edit(update) one model at the time it works but when i try to update Question model that has 3 Answers as list property i get this exception

//exception start Attaching an entity of type 'Quiz.DAL.Answer' failed because another entity of the same type already has the same primary key value. This can happen when using the 'Attach' method or setting the state of an entity to 'Unchanged' or 'Modified' if any entities in the graph have conflicting key values. This may be because some entities are new and have not yet received database-generated key values. In this case use the 'Add' method or the 'Added' entity state to track the graph and then set the state of non-new entities to 'Unchanged' or 'Modified' as appropriate. //exception end

exception occurs in GenericRepository class.

public class GenericRepository<TEntity> where TEntity : class
{
    internal QuizContext Context;
    internal DbSet<TEntity> DbSet;

    /// <summary>
    /// Public Constructor,initializes privately declared local variables.
    /// </summary>
    /// <param name="context"></param>
    public GenericRepository(QuizContext context)
    {
        this.Context = context;
        this.DbSet = context.Set<TEntity>();
    }
    public virtual void Update(TEntity entityToUpdate)
    {
        DbSet.Attach(entityToUpdate); //exception is invoked here
        Context.Entry(entityToUpdate).State = EntityState.Modified;
    } 
 }

ive checked if all data is posted from view to action and it is.

This is the method which is called from Edit action

public class QuestionServices : IQuestionServices
{
    private UnitOfWork _unitOfWork;
    public void UpdateQuestion(QuestionEntity questionEntity)
    {
        var questionDb = new Question
        {
            Id = questionEntity.Id,
            Name = questionEntity.Name
        };

        var answersDb = _unitOfWork.AnswerRepository.GetMany(a => 
        a.QuestionId == questionDb.Id).ToList();
        foreach (var a in questionEntity.Answers)
        {
            answersDb.Add(new Answer()
            {
                Id = a.Id,
                Name = a.Name,
                IsTrue = a.IsTrue,
                QuestionId = a.QuestionId,
                Question = questionDb
            });
        }

        unitOfWork.QuestionRepository.Update(questionDb);
        foreach (var answer in answersDb)
        {
            _unitOfWork.AnswerRepository.Update(answer);
        }
        _unitOfWork.Save();
}

edit view

@using (Html.BeginForm())
{
    @Html.AntiForgeryToken()

    <div class="form-horizontal">
        <h4>QuestionEntity</h4>
        <hr />
        @Html.ValidationSummary(true, "", new { @class = "text-danger" })
        @Html.HiddenFor(model => model.Id)

        <div class="form-group">
            @Html.LabelFor(model => model.Name, htmlAttributes: new { @class = "control-label col-md-2" })
            <div class="col-md-10">
                @Html.EditorFor(model => model.Name, new { htmlAttributes = new { @class = "form-control" } })
                @Html.ValidationMessageFor(model => model.Name, "", new { @class = "text-danger" })
            </div>
        </div>

        @for (int i=0; i<Model.Answers.Count; i++)
        {
            @Html.EditorFor(m=>m.Answers[i])
        }

        <div class="form-group">
            <div class="col-md-offset-2 col-md-10">
                <input type="submit" value="Save" class="btn btn-default" />
            </div>
        </div>
    </div>
}

editor template for answerEntity

<div class="form-horizontal"/>
@Html.HiddenFor(model=>model.Id)
@Html.HiddenFor(model=>model.QuestionId)
@Html.ValidationSummary(true, "", new { @class = "text-danger" })
<div class="form-group">
    @Html.LabelFor(model => model.Name, htmlAttributes: new { @class = "control-label col-md-2" })
    <div class="col-md-10">
        @Html.EditorFor(model => model.Name, new { htmlAttributes = new { @class = "form-control" } })
        @Html.ValidationMessageFor(model => model.Name, "", new { @class = "text-danger" })
    </div>
</div>
<div class="form-group">
    @Html.LabelFor(model => model.IsTrue, htmlAttributes: new { @class = "control-label col-md-2" })
    <div class="col-md-10">
        <div class="checkbox">
            @Html.EditorFor(model => model.IsTrue)
            @Html.ValidationMessageFor(model => model.IsTrue, "", new { @class = "text-danger" })
        </div>
    </div>
</div>

and finally the action method

    [HttpPost]
    [ValidateAntiForgeryToken]
    public ActionResult Edit([Bind(Include = "Id,Name,Answers")] 
    QuestionEntity questionEntity)
    {
        if (ModelState.IsValid)
        {
            //db.Entry(questionEntity).State = EntityState.Modified;
            //db.SaveChanges();
            _questionServices.UpdateQuestion(questionEntity);
            return RedirectToAction("Index");
        }
        return View(questionEntity);
    }

i believe this error is popping cuz i have to tell ef that i wanna update answers but ef thinks they are new and that they should be created not updated witch leads to the same primary key error or, i have more then one copy of answer in memory and that confuses ef.

any help is appreciated.

EDIT https://github.com/StefanIvovic/Quiz check it out

1
Apparently your answersDb list contains items with duplicate Id. In general you should detect the new/deleted/updated items and call the corresponding AnswerRepository methods. - Ivan Stoev
Well, I can pretty much guarantee the repository is part of your problem. It always is. Plain and simple, if you're using a repository with EF, you're doing it wrong. However, the root of your problem is likely in how the post data is being bound to the entity(ies) you're trying to save. We need to see code for that, though. - Chris Pratt
i updated post with view for edit - Stefan Ivovic
@IvanStoev thay dont have. each entity has its own PK ive checked via debugger. and they all have same QuestionId which is PK of Question - Stefan Ivovic

1 Answers

2
votes

No offense intended at all by this, because honestly, it's probably not your fault. There's so much bad information out there. All the same, this code is absolutely tragic. You've got three main problems.

  1. Don't use Bind. Like ever. Just stop. Right now. Yesterday, if possible. If you need to exclude properties from being posted use a view model, which is basically just a class, specifically tailored for the view, that only contains what the view needs. In this case, that would look something like:

    public class QuestionViewModel
    {
        public string Name { get; set; }
        public List<Answer> Answers { get; set; }
    }
    

    Notice that: 1) Id is not included. You should never allow the id to be posted. The id should come from the URI, since that's what makes it a "universal resource identifier". 2) Answer, likewise, should probably be using a view model here, so your property should most like be List<AnswerViewModel>, but I didn't want to overwhelm you too much.

  2. Never, ever, ever, EVER save anything created by the post directly to your database. This is actually solved by using view models, since you necessarily must save something different than what was posted. Nevertheless, you should always map post data onto the entity you intend to save. This is even more important when doing edits, as you should always pull the entity fresh from the database, modify that instance, and then save that instance. In this way, even if you don't use view models, you still don't even need Bind as no user can actually get anything they post to be saved to the database unless you explicitly allow that, by mapping the posted property to the entity property you're going to save.

  3. Finally, the repository/unit of work pattern is completely superfluous with an ORM like Entity Framework. EF actually already implements these patterns: the DbContext is your unit of work and each DbSet is a repository. Adding a layer on top of this does nothing but obfuscate what's happening and subsequently makes your application infinitely more difficult to maintain. I've heard a million and one excuses why people think you should still implement these patterns, but they're all either misguided ("You can't test without it!" Wrong. EF is 100% testable.) or are better solved by a different patterns (such as Command Query Responsibility Segregation (CQRS) or the Service Layer Pattern).