views:

283

answers:

5

I get this error

Cannot add an entity with a key that is already in use

when I try to save an Item

[AcceptVerbs(HttpVerbs.Post)]
public ActionResult Edit(Item item)
{
  Global.DataContext.Items.Attach(item);
  Global.DataContext.SubmitChanges();

  return View(item);
}

That's because I cannot attach the item to the global DataContext.

Is it possible to save an item without creating a new DataContext, and without having to assign each field of the item manually?

(I am very new to LINQ)

EDIT: I realised static DataContext would cause problems thanks to the comments below, it is now like this

public static AbcDataContext DataContext
{
  get
  {
    if (!HttpContext.Current.Items.Contains("DataContext"))
      HttpContext.Current.Items["DataContext"] = new AbcDataContext(ConnectionString);
    return (AbcDataContext)HttpContext.Current.Items["DataContext"];
  }
}

(Rex might not agree to that, but I can't be bothered changing the whole code at the moment - may be later)

+3  A: 

Don't have a global/static DataContext, that is setting yourself up for pain. A DataContext should represent a single logical transaction ("get in, do x/y/z and get out"). They are cheap to create and easy to dispose; there is absolutely no reason to try to minimize them, much less keep a global/static one.

Rex M
I did that before, but it gave a lot of sync problems, where an update to a DataContext is not reflected in the other.
@aximili you went in the wrong direction to solve the problem. You should keep DCs alive even less time and use another layer to keep your entities synchronized.
Rex M
How is that done then? Isn't it more tedious? My code now seems to be a lot simpler with a global DataContext.
@aximili but unstable and not scalable. The DataContext is not thread-safe and uses a single database connection. And of course you run into problems like this one. Your question is "how do I fix this error", the answer is don't do what you are doing.
Rex M
The above question is not really a problem though. I am currently assigning each field manually and it works fine. I was just wondering if it could be easier. But is there any other potential problems? And how do you "use another layer to keep your entities synchronized"?
@aximili the first way to get you 95% there is to ensure your DataContexts are only alive long enough to fetch the data and then close. If you need to ensure a particular entity is always up-to-date on every thread, then you should use something like the HttpRuntime cache to share the single instance of that entity across all threads; or implement a token system so that a thread must have the token before it is allowed to change the entity, and after the entity is changed all threads are notified that they should re-load their copy.
Rex M
I see... do you have a sample code to use the HttpRuntime cache? Actually I think this answers my other question, do you mind copying your answer there so I can mark it as an answerhttp://stackoverflow.com/questions/2664910/linq-to-sql-does-not-update-when-data-has-changed-in-databaseand thanks Rex!
@aximili no problem, feel free to hit me up if you need any more clarifications. I'll post an example of using the cache
Rex M
A: 

According to this discussion about the same problem, it seems to be a type-mapping bug that may be worked around if you delete the Item class in the designer and just drag over the table into the designer again.

Mark Cidade
I think that is about inserting a new record. I am trying to update an existing record.
+3  A: 

Suppose the primary key of your Item class is ItemId.

Suppose the ItemID for the instance you are attempting to update is 5.

The DataContext has seen an original state for ItemID 5, so it won't let you Attach(). http://msdn.microsoft.com/en-us/library/bb300517.aspx

In this version of Attach, the entity is assumed to be in its original value state. After calling this method, you can then update its fields, for example with additional data sent from the client.

There's three normal ways to perform an update in LinqToSql.

If the parameter to this Edit method was originally loaded up from the DataContext, then all you need to do is:

public ActionResult Edit(Item item) 
{
  Global.DataContext.SubmitChanges(); 
  return View(item); 
} 

The DataContext tracks changes against objects that it loaded. As a nasty side effect, any modified objects that was loaded by the DataContext are also going to be updated. This is a big reason to not use a single app level DataContext.

If the parameter to this Edit method was new'd up in your code, loaded by a different DataContext, or passed to your code (in other words, the instance has no attached DataContext) then you can do either of these:

public ActionResult Edit(Item item) 
{
  using(MyDataContext dc = new MyDataContext())
  {
//this new DataContext has never heard of my item, so I may Attach.
    dc.Items.Attach(item);
//this loads the database record in behind your changes
// to allow optimistic concurrency to work.
//if you turn off the optimistic concurrency in your item class
// then you won't have to do this
    dc.Refresh(item, KeepCurrentValues);
    dc.SubmitChanges(); 
  }
  return View(item); 
} 

public ActionResult Edit(Item item) 
{
  original = Global.DataContext.Items.Single(x => x.ItemID = item.ItemID)
  //play the changes against the original object.
  original.Property1 = item.Property1;
  original.Property2 = item.Property2;
  Global.DataContext.SubmitChanges(); 
  return View(item); 
} 

With your question answered, allow me to echo the concern that others have stated for using a static DataContext. This is a poor practice and goes against Microsoft's intended use of the DataContext class. http://msdn.microsoft.com/en-us/library/system.data.linq.datacontext.aspx

In general, a DataContext instance is designed to last for one "unit of work" however your application defines that term. A DataContext is lightweight and is not expensive to create. A typical LINQ to SQL application creates DataContext instances at method scope or as a member of short-lived classes that represent a logical set of related database operations.

David B
That got me excited, but it still returns the same error message even after I attach the original item. Thanks about the static, you're right.
Ok, removed the false bits and replaced with more solutions.
David B
+1  A: 

static global DataContext? If my understanding of your question is correct, this will result in everyone connecting to you app sharing the same data context which will cause lot of security/sync issues. Avoid it.

Raj Kaimal
I think you're right, thanks
A: 

DataContext discussion. Note I'm not commenting on your code.

DataContexts implement IDisposable, and therefore you should be disposing of the data context when it's no longer needed. Your website works well enough in development, but in production you will get nailed. You might as well do it right before your code gets too entrenched and changing it will be a big hassle. At best you'll just develop bad habits.

A better alternative to what you've written is to have your own controller base class that manages the lifetime for you.

public class MyBaseController : System.Web.Mvc.Controller
{
    private AbcDataContext abcDataContext;

    protected AbcDataContext DataContext
    {
        get 
        {   // lazy-create of DataContext
            if (abcDataContext == null)
                abcDataContext = new AbcDataContext(ConnectionString);

            return abcDataContext;
        }
    }

    protected override void Dispose(bool disposing)
    {
        base.Dispose(disposing);
        if (disposing)
        {
            if( abcDataContext != null )
                abcDataContext.Dispose();
        }
    }
}

which allows you to do

public class MyController : MyBaseController
{
    [AcceptVerbs(HttpVerbs.Post)]
    public ActionResult Edit(Item item)
    {
        DataContext.Items.Attach(item);
        DataContext.SubmitChanges();

        return View(item);
    }
}

While this works, I personally find it can get annoying and tricky.


Best: If you're going to follow MVC as you're supposed to, you should populate the model completely and not rely on lazy-loading entities. The best way to achieve this is to get rid of your DataContext as soon as you can.

Typically, we enforce this at a code level via the following pattern:

using( var dc = new AbcDataContext(ConnectionString))
{
    var itemUpdater = new ItemUpdater(dc);
    item = itemUpdater.Update(item);
}
return View(item);

The idea is that you will get an ObjectDisposedException if your view attempts to get any additional data via lazy-loading.

Robert Paulson