I have these entities.
[Table("ServiceTickets", Schema = "dbo")]
public class ServiceTicket
{
[Key]
[DatabaseGenerated(DatabaseGeneratedOption.Identity)]
public int Id { get; set; }
public DateTime CreationDateTime { get; set; } = DateTime.Now;
[Column(TypeName = "varchar(1000)")]
public string Issue { get; set; } = string.Empty;
public DateTime? ReportedDateTime { get; set; }
public DateTime? ResolutionDateTime { get; set; }
public int CreatedBy { get; set; }
public int AttendedBy { get; set; }
public int ConfirmedBy { get; set; }
public int SrNumber { get; set; }
public int StatusId { get; set; }
public ServiceTicketSubInformation? ServiceTicketSubInformation { get; set; }
public virtual StatusEnum? StatusEnums { get; set; }
public virtual ICollection<WorkOrder>? WorkOrders { get; set; }
}
[Table("ServiceTicketSubInfo", Schema = "dbo")]
public class ServiceTicketSubInformation
{
[Key]
[DatabaseGenerated(DatabaseGeneratedOption.Identity)]
public int Id { get; set; }
public int LocationId { get; set; }
public int SubLocationId { get; set; }
public int ReporterId { get; set; }
}
[Table("WorkOrder", Schema = "dbo")]
public class WorkOrder
{
[Key]
[DatabaseGenerated(DatabaseGeneratedOption.Identity)]
public int Id { get; set; }
public DateTime? CreationDateTime { get; set; }
[Column(TypeName = "varchar(1000)")]
public string ActionTaken { get; set; } = string.Empty;
public DateTime? WorkStartTime { get; set; }
public DateTime? WorkCompleteTime { get; set; }
public int SrNumber { get; set; }
public int Status { get; set; }
}
[Table("Statuses", Schema = "dbo")]
public class StatusEnum
{
[Key]
[DatabaseGenerated(DatabaseGeneratedOption.Identity)]
public int Id { get; set; }
[Column(TypeName = "varchar(50)")]
public string Name { get; set; } = string.Empty;
}
It builds successfully, I need to insert a new record. So using Swagger, I change the values to this:
{
"id": 0,
"creationDateTime": "2022-07-24T12:41:49.666Z",
"issue": "My Issue",
"reportedDateTime": "2022-07-24T12:41:49.666Z",
"resolutionDateTime": "2022-07-24T12:41:49.666Z",
"createdBy": 1,
"attendedBy": 1,
"confirmedBy": 1,
"srNumber": 1,
"statusId": 1,
"serviceTicketSubInformation": {
"id": 1,
"locationId": 1,
"subLocationId": 1,
"reporterId": 1
},
"statusEnums": {
"id": 1,
"name": "Completed"
},
"workOrders": [
{
"id": 1,
"creationDateTime": "2022-07-24T12:41:49.666Z",
"actionTaken": "My Action Taken",
"workStartTime": "2022-07-24T12:41:49.666Z",
"workCompleteTime": "2022-07-24T12:41:49.666Z",
"srNumber": 1,
"status": 1
}
]
}
When I try to add
public async Task<ServiceTicket> UpsertServiceTicket(ServiceTicket model)
{
await context.ServiceTickets.AddAsync(model);
var result = await context.SaveChangesAsync();
model.Id = result;
return model;
}
I get this error:
Microsoft.EntityFrameworkCore.DbUpdateException: An error occurred while saving the entity changes. See the inner exception for details.
Microsoft.Data.SqlClient.SqlException (0x80131904): Cannot insert explicit value for identity column in table 'ServiceTicketSubInfo' when IDENTITY_INSERT is set to OFF.
at Microsoft.Data.SqlClient.SqlCommand.<>c.b__188_0(Task`1 result)
What am I doing wrong here?
CodePudding user response：
The issue is that your API is accepting models that look like Entities but aren't actually entities. This can work where you have an entity graph that is solely parent-child relationships, but can fall down the minute you introduce many-to-one references.
Think of it this way, if your API accepted a DTO object for the relationships, what would it look like and how would you process it?
The DTO would contain the details to create a new ServiceTicket, along with the detail to know which "SubInformation" applies to it. In your case you want to associate it with a SubInformation ID of #1.
This is different to a case where we want to Create a SubInformation. The issue is that while the ServiceTicket passed in contains something that looks like an existing SubInformation, it isn't an existing SubInformation as far as the DbContext servicing that request is concerned. It is a deserialized blob of JSON that the DbContext will treat as a new entity, and having an ID value means you're either going to get a duplicate exception, or EF would ignore the ID all-together and insert a new one with the next available ID, such as #13.
To address this, the DbContext needs to know that SubInformation #1 is referencing a Tracked, known entity:
A) The Hack:
public async Task<ServiceTicket> InsertServiceTicket(ServiceTicket model)
{
context.Attach(model.SubInformation);
await context.ServiceTickets.AddAsync(model);
var result = await context.SaveChangesAsync();
model.Id = result;
return model;
}
This assumes that you trust that the details in the SubInformation actually point to a valid row in the DB. If it doesn't then this operation will fail. This is also a common hack for when APIs/Controllers accept entities to be updated:
public async Task<ServiceTicket> UpdateServiceTicket(ServiceTicket model)
{
context.Attach(model);
context.Entry(model).State = EntityState.Modified;
var result = await context.SaveChangesAsync();
return model;
}
When attaching entities, one of the worst things you can do is set the EntityState to Modified in order to update all of the values. The issue is that you are explicitly trusting all of the values passed, which is extremely dangerous since it exposes your system to tampering in the request data.
B) Better:
public async Task<ServiceTicket> InsertServiceTicket(ServiceTicket model)
{
var subInformation = context.SubInformations.Single(x => x.Id == model.serviceTicketSubInformation.Id);
model.serviceTicketSubInformation = subInformation;
// Repeat for any and all other references... I.e. WorkOrders
await context.ServiceTickets.AddAsync(model);
var result = await context.SaveChangesAsync();
model.Id = result;
return model;
}
This solution queries for valid entities based on the IDs passed in and replaces the references with tracked entities. This way when we insert our service ticket, all references are validated and tracked. If we specify a SubInformation ID that doesn't exist, it will still throw an exception but the call stack will make it clear exactly what reference wasn't found rather than some error on SaveChanges. The issue with this approach is that it's easy to forget a reference, leading to repeated errors popping up as the system evolves.
C) Best, DTOs:
Using DTOs we can condense our payload for an Insert down to:
{
"id": 0,
"creationDateTime": "2022-07-24T12:41:49.666Z",
"issue": "My Issue",
"reportedDateTime": "2022-07-24T12:41:49.666Z",
"resolutionDateTime": "2022-07-24T12:41:49.666Z",
"createdBy": 1,
"attendedBy": 1,
"confirmedBy": 1,
"srNumber": 1,
"statusId": 1,
"serviceTicketSubInformationId": 1,
"workOrdersIds": [ 1 ],
}
When we go to insert our service ticket:
public async Task<ServiceTicketSummaryDTO> InsertServiceTicket(NewServiceTicketDTO model)
{
var subInformation = context.SubInformations.Single(x => x.Id == model.ServiceTicketSubInformationId);
var workOrders = await context.WorkOrders
.Where(x => model.WorkOrderIds.Contains(x.Id))
.ToListAsync();
if (workOrders.Count != model.WorkOrderIds.Count)
throw new ArgumentException("One or more work orders were not valid.");
var serviceTicket = _mapper.Map<ServiceTicket>(model);
serviceTicket.SubInformation = subInformation;
serviceTicket.WorkOrders = workOrders;
await context.ServiceTickets.AddAsync(serviceTicket);
await context.SaveChangesAsync();
return _mapper.Map<ServiceTicketSummaryDTO>(serviceTicket);
}
Here we accept a DTO for the details of a new Service Ticket. This DTO only needs to contain the IDs for any references which the call will validate and resolve before populating a new entity and passing back a DTO with details that the consumer will be interested in. (Either built from the entity we just created or querying the DBContext if there is more info needed.) This has the added benefit of cutting down the size of data being passed back and forth and ensures only data we expect to be altered is accepted.