EntityFramework adds duplicate related objects on SaveChanges

Question

Objects

public class Noun
{
    public int Id { get; set; }

    [MaxLength(128)]
    public string Name { get; set; }
    public bool Active { get; set; }

    public virtual Category Category { get; set; }
    public int CategoryId { get; set; }
}

public class Category
{
    public int Id { get; set; }

    [MaxLength(128)]
    public string Name { get; set; }
}

Repository

public Noun CreateNoun(string name, string categoryName)
{
    using (MyContext context = new MyContext())
    {
        Noun noun;
        Category category;

        lock (NounLock)
        {
            // don't create it if it already exists
            noun = context.Nouns
                .Include(t => t.Category)
                .FirstOrDefault(t => t.Name == name && t.Category.Name == categoryName);

            if (noun == null)
            {
                // make the category if it doesn't already exist
                lock (CategoryLock)
                {
                    category = context.Categories.FirstOrDefault(c => c.Name == categoryName);
                    if (category == null)
                    {
                        category = new Category() { Name = categoryName };
                        context.Categories.Add(category);
                        context.SaveChanges();
                    }
                }

                noun = new Noun()
                {
                    Name = name,
                    Category = category,
                    CategoryId = category.Id
                };

                context.Nouns.Add(noun);
            }
            else
            {
                category = noun.Category;
            }

            // make sure the noun is set as active
            noun.Active = true;

            context.Entry(category).State = EntityState.Unchanged;
            context.SaveChanges();

            return noun;
        }
    }
}

Context

internal class MyContext : DbContext
{ 
    public DbSet<Category> Categories { get; set; }
    public DbSet<Noun> Nouns { get; set; }

    public MyContext()
        : base("DefaultConnection")
    {
    }

    protected override void OnModelCreating(DbModelBuilder modelBuilder)
    {
        // Nouns
        modelBuilder.Entity<Noun>()
            .HasRequired(t => t.Category);
    }
}

Question

When calling CreateNoun(), when the noun with that category already exists, it should (based on what I think I am doing), just load the existing Noun from the db and mark it as active. But instead it inserts a new Noun, and a new Category. What am I doing wrong? I know it is probably a something small.

PS: The locks are static and in place because this is potentially used by a multi-threaded tenant

Example

Noun newNoun = repo.CreateNoun("name", "cat");
// should load existing noun from db, and set it as active, but instead duplicates it
Noun oldNoun = repo.CreateNoun("name", "cat"); 

Answer 1

You have some issues in the CreateNoun method, I did some changes that I'm going to explain later:

public Noun CreateNoun(string name, string categoryName)
{
   using (MyContext context = new MyContext())
   {
       Noun noun;
       Category category;

       lock (NounLock)
       {
          // don't create it if it already exists
          noun = context.Nouns
                    .FirstOrDefault(t => t.Name == name && t.Category.Name == categoryName);

          if (noun == null)
          {
             // make the category if it doesn't already exist
             lock (CategoryLock)
             {
               category = context.Categories.FirstOrDefault(c => c.Name == categoryName);
               if (category == null)
               {
                  category = new Category() { Name = categoryName };
                  context.Categories.Add(category);
                }
             }
             noun = new Noun()
             {
               Name = name,
               Category = category,
             };
             context.Nouns.Add(noun);
          }

          // make sure the noun is set as active
          noun.Active = true;

          context.SaveChanges();

          return noun;
       }
   }
}

• You don't need to call the Include, EF will load the Category property for you. This navegation property is virtual so, it will be lazy loaded .
• When you don't find the Category, you don't need to call the SaveChanges method. Before return the noum, you are calling this method, it will save all the changes that you did before.
• You don't need to set the Foreign Key CategoryId. You don't know if the category was loaded from DB or recently created. Just set the navegation property Category.
• You don't need to change the Category State

Another Recomendation:

In the OnModelCreating method you are configuring Category as required. The method HasRequired is used when you want to configure a relationship. In your case it would be like this:

 protected override void OnModelCreating(DbModelBuilder modelBuilder)
 {
        modelBuilder.Entity<Noun>()
            .HasRequired(t => t.Category).WithMany().HasForeignKey(n=>n.CategoryId);
 }

Update

As @Shoe said in his comment about the first point, If you want to use the Noun that is returned by this method and consult its Category, call the Include method when you search the Noum as you did before.

Answer 2

You haven't marked your keys in the models. Both Id properties need to be marked with [Key] and the Category property in the Noun class requires a [ForeignKey("CategoryId")] annotation. As it stands the duplicate check is always failing because t.Category.Name is always null.