Ensuring another record does not already contain the same value for a field

Question

I am in the process of building a small content management system for my own academic purposes using C# MVC.

I have functionality to create page (list pages, delete pages etc. etc.)

At a basic level, I use an ID to verify all pages are unique. But as I am using MVC, a page is essentially a View - and it can contain razor. All this works no problem.

However my ViewName must also be unique (you cant have two views in the database with the same name). Also my ViewPath must be unique (you cant have two routes that are identical).

In my create page, I have checks (all working) to ensure a new view is created with a unique ID, and it checks prior to creating the view that both the ViewName and ViewPath are unique returning relevant errors if not.

All this works.

Now when I go to edit a view. The user has the option to alter the ViewName and ViewPath. So we need to ensure it is not altered to a ViewName and ViewPath that already exists. However as we are editing the view, the view itself already exists so we get these errors saying so.

So we need to check if we are saving to the same page so we look for the id of the record in the DB of the ViewName with the current ViewName (because ViewNames are unique, so we can get the ID), and verify that we are indeed saving to the same view. Then we can use this check to ensure that we don't show the above errors because we are simply changing the name of the view and it doesn't match any existing view.

The problem is I get the following error when I hit the line

db.Entry(view).State = EntityState.Modified;

The error is:

Additional information: Attaching an entity of type 'xxx' 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.

QUESTION: I don't know if I am over complicating my solution, if so, can someone please advise a better solution with an example.

If this is the correct "way" but I have just implemented it wrong somehow, can someone show me how to fix it.

Below is the POST method of the Edit controller action where I am having the problem:

public ActionResult Edit([Bind(Include = "Id,ViewName,ViewPath,ViewContent")] View view)
{
    View sameView1 = db.View.First(v => v.ViewName == view.ViewName);
    bool sameview2 = view.Id == sameView1.Id;

    bool ViewExists = db.View.Any(v => v.ViewName == view.ViewName);
    if (ViewExists && !sameview2)
    {
        ModelState.AddModelError("ViewName", UI_Prototype_MVC_Resources.ErrorViewAlreadyExists);
        return View(view);
    }

    bool PathExists = db.View.Any(v => v.ViewPath == view.ViewPath);
    if (PathExists && !sameview2)
    {
        ModelState.AddModelError("ViewPath", UI_Prototype_MVC_Resources.ErrorPathAlreadyExists);
        return View(view);
    }

    if (ViewExists && PathExists && sameview2)
    {
        if (ModelState.IsValid)
        {
            db.Entry(view).State = EntityState.Modified;
            db.SaveChanges();
            return RedirectToAction("Index");
        }
    }

    return View(view);
}

Below is the POST method of the Create controller action. This is working fine, it's just for reference encase it helps anyone:

public ActionResult Create(View view)
{
    bool ViewExists = db.View.Any(v => v.ViewName == view.ViewName);
    if (ViewExists)
    {
        ModelState.AddModelError("ViewName", UI_Prototype_MVC_Resources.ErrorViewAlreadyExists);
        return View(view);
    }

    bool PathExists = db.View.Any(v => v.ViewPath == view.ViewPath);
    if (PathExists)
    {
        ModelState.AddModelError("ViewPath", UI_Prototype_MVC_Resources.ErrorPathAlreadyExists);
        return View(view);
    }

    if (!ViewExists && !PathExists)
    {
        if (ModelState.IsValid)
        {
            view.ViewContent = "<p>Initial Content</p>";

            db.View.Add(view);
            db.SaveChanges();

            return RedirectToAction("Index");
        }
    }

    return View(view);
}

If adding comments would help any just let me know and I'll edit. Essentially its just reading the error strings from resource files. Its also just doing a couple of reads of the ViewName, ViewPath to check if a record already exists.

In the case of the Edit controller action, there is an additional check to pull back the current records id to verify that if we change the name that the record is still the same (so it doesn't match an existing record based on the ViewName).

Encase it helps any, I do have unique constraints on the ViewName and ViewPath fields in the DB. These will cause exceptions in the code in the cases where the fields are not unique and for some odd reason it is not handled in the code.

I can't help thinking there is too much complexity here?

Answer 1

Yes, your over complicating it and you can simplify your code and reduce the number of database calls by checking if its valid using

bool isViewNameInvalid = db.View.Any(v => v.ViewName == view.ViewName && v.Id != view.Id)

and ditto for ViewPath. You should also consider making both checks before returning the view so if the user has both errors, they do not need to make a extra submit in order t be informed of the 2nd error.

Note also that your View sameView1 = db.View.First(v => v.ViewName == view.ViewName); line of code may not be returning the View record you expect (it could be the one your editing or a previously created one with the same ViewName value) which could cause unexpected results.

Your code can be simplified to

public ActionResult Edit([Bind(Include = "Id,ViewName,ViewPath,ViewContent")] View view)
{
    bool isViewNameInvalid = db.View.Any(v => v.ViewName == view.ViewName && v.Id != view.Id)
    if (isViewNameInvalid)
    {
        ModelState.AddModelError("ViewName", UI_Prototype_MVC_Resources.ErrorViewAlreadyExists);
    }
    bool isViewPathInvalid = db.View.Any(v => v.ViewPath == view.ViewPath && v.Id != view.Id)
    if (isViewPathInvalid)
    {
        ModelState.AddModelError("ViewName", UI_Prototype_MVC_Resources.ErrorPathAlreadyExists);
    }
    if (!ModelState.IsValid)
    {
        return View(view);
    }
    // Save and redirect
    db.Entry(view).State = EntityState.Modified;
    db.SaveChanges();
    return RedirectToAction("Index")
}

As a side note, you could consider implementing a RemoteAttribute on the ViewName and ViewPath properties to give you client side validation (refer How to: Implement Remote Validation in ASP.NET MVC). In your case you would pass the Id value as additionalFields.

And since your editing data, I recommend you use a view model (and remove the [Bind] attribute - refer What is ViewModel in MVC?.