50

Revised entire post.

I'm trying to post the following JSON POST request via Fiddler:

{Username:"Bob", FirstName:"Foo", LastName:"Bar", Password:"123", Headline:"Tuna"}

However I'm getting this error:

Message "Cannot insert the value NULL into column 'Id', table 'xxx_f8dc97e46f8b49c2b825439607e89b59.dbo.User'; column does not allow nulls. INSERT fails.\r\nThe statement has been terminated." string

Though if I manually send a random Id along with the request then all is good. Like so:

{Id:"1", Username:"Bob", FirstName:"Foo", LastName:"Bar", Password:"123", Headline:"Tuna"}

Why does Entity Framework not generate and auto increment the Id's? My POCO class is as follows:

public class User
{
    [Key]
    [DatabaseGenerated(DatabaseGeneratedOption.Identity)]
    public string Id { get; set; }
    public string Username { get; set; }
    public string FirstName { get; set; }
    public string LastName { get; set; }
    public string Password { get; set; }
    public string Headline { get; set; }
    public virtual ICollection<Connection> Connections { get; set; }
    public virtual ICollection<Address> Addresses { get; set; }
    public virtual ICollection<Phonenumber> Phonenumbers { get; set; }
    public virtual ICollection<Email> Emails { get; set; }
    public virtual ICollection<Position> Positions { get; set; }
}

public class Connection
{
    public string ConnectionId { get; set; }
    public int UserId { get; set; }
    public virtual User User { get; set; }
}

public class Phonenumber
{
    public string Id { get; set; }
    public string Number { get; set; }
    public int Cycle { get; set; }
    public int UserId { get; set; }
    public User User { get; set; }
}

Here is the controller method. When in debug mode and I send the request via Fiddler it breaks at db.SaveChanges(); and gives the error seen a bit above.

    // POST api/xxx/create
    [ActionName("create")]
    public HttpResponseMessage PostUser(User user)
    {
        if (ModelState.IsValid)
        {
            db.Users.Add(user);
            db.SaveChanges();

            HttpResponseMessage response = Request.CreateResponse(HttpStatusCode.Created, user);
            response.Headers.Location = new Uri(Url.Link("DefaultApi", new { id = user.Id }));
            return response;
        }
        else
        {
            return Request.CreateErrorResponse(HttpStatusCode.BadRequest, ModelState);
        }
    }

What's wrong?

Solution

Change string Id to int instead and remove Data annotations. Renamed the Id to UserId, still following convention, and made changes where necessary in other POCO's to match up with the changes.

afuzzyllama
  • 6,538
  • 5
  • 47
  • 64
brk
  • 1,396
  • 1
  • 15
  • 25

2 Answers2

59

This is a guess :)

Is it because the ID is a string? What happens if you change it to int?

I mean:

 public int Id { get; set; }
tymtam
  • 31,798
  • 8
  • 86
  • 126
  • 1
    I feel really silly but it's quite obvious to me now that it should have been `int`. It's just that when I started everything was pretty much a `string` and I sort of forgot about it. Thank you ;) – brk Apr 18 '13 at 13:02
  • 1
    Another hint would be **not** to forget that the fields/DB Columns need to be expressed as **properties** in your Model and not just plain fields/members. – Dr1Ku Oct 01 '13 at 08:26
9

You have a bad table design. You can't autoincrement a string, that doesn't make any sense. You have basically two options:

1.) change type of ID to int instead of string
2.) not recommended!!! - handle autoincrement by yourself. You first need to get the latest value from the database, parse it to the integer, increment it and attach it to the entity as a string again. VERY BAD idea

First option requires to change every table that has a reference to this table, BUT it's worth it.

walther
  • 13,466
  • 5
  • 41
  • 67
  • 1
    You are absolutely right, it was bad design of me. Thank you for your input :) – brk Apr 18 '13 at 13:00
  • Re: not recommended!!! - I don't agree with this. Autoincrement done by the db means that you have to hit the db to create a record which is not always great. But that's an another topic... – tymtam Apr 19 '13 at 01:23
  • 1
    @Tymek, erm, either way you have to hit the database to determine which Id should be next. But, if you let the database to handle it for you, you simply pass other parameters to the db and db figures out the value for you during handling the insert.If you handle the autoincrementing by yourself, you have to get the latest id from the database BEFORE you actually send a query to insert a record. That's one extra trip to db! Not mentioning the situation when your database gets accessed by more than one application/user at a time, that's really a nightmare to solve. – walther Apr 19 '13 at 09:41
  • @Tymek, I really can't see a situation when this might be a good idea. – walther Apr 19 '13 at 09:42
  • I was thinking about generating id yourself without hitting the database at all (remember, it doesn't need to be id++). For some of the reasons why autoincrement (and more genral the db handling ids) is not so good ave a look at http://ayende.com/blog/3915/nhibernate-avoid-identity-generator-when-possible In my view autoincrement sooner or later causes pain. – tymtam Apr 19 '13 at 13:00
  • 1
    @Tymek, yes, Id can be anything, but how do you know which id isn't taken if you don't hit the database first to check?? It has to be unique, so you need to have a mechanism that will take care of this. The more complex situation, the more pain implementing it, especially in concurrent scenarios (like ASP.NET, what is this question about), where you need to handle this for all users that are accessing your db at once. What if you create an Id, that is taken? You pass it to the database and you get an error. What do you tell the user? "Oops, we generated a wrong Id, let us try again"?? – walther Apr 19 '13 at 13:08
  • @Tymek, there's one word that fits that idea - reinventing the wheel. It usually comes with breaking other parts of the system, like Unit of work that Ayende mentions as well. Furthermore, what if there's a requirement to make the database available to other applications? What if they don't want to use nhibernate? Taking db responsibilities away from the db is never a good idea, doesn't matter who says otherwise... – walther Apr 19 '13 at 13:12
  • In the context of this question you are right. For a more general context of id generation this is not the right place to discuss the issue. – tymtam Apr 20 '13 at 13:30
  • 1
    Guid can be stored as string and you don't need to hit the database to see if already exists. – Bart Calixto Feb 13 '15 at 22:44
  • @BartCalixto Correct. IDs are great for projects that aren't going to scale horizontally. GUID's are necessary to scale horizontally. – Oscar Jan 18 '23 at 03:41