Jasiel Torres
Jasiel Torres

Reputation: 427

update child entities when updating a parent entity in EF

hello community I am trying to update the secondary data within a main class, the main class is quotation and within it is the cart class which contains references to two other classes, the cart articles class and products. I can now update the properties of the quotation class and I managed to insert a new article to the quotation but when inserting the new product the cart id becomes null, in the article cart table of the database I have the product id and the id of the cart.

How can I update the products within the quote?

How can I prevent the CarritoId from becoming null?

this is my code:

  [HttpPut]
    public async Task<ActionResult> Put(Cotizacion cotizacion)
    {
        var cotizacionoriginal = context.Cotizaciones.Where(x => x.CotizacionId == cotizacion.CotizacionId).FirstOrDefault();

        if (cotizacionoriginal != null)
        {
            context.Entry(cotizacionoriginal).State = EntityState.Detached;
        }

        context.Entry(cotizacion).State = EntityState.Modified;

        foreach (ArticuloCarrito articulo in cotizacion.Carrito.Articulos)
        {
            articulo.ProductoId = articulo.Producto.ProductoId;
            articulo.Producto = null;
            //articulo.Carrito.CarritoId = cotizacion.Carrito.CarritoId;
            context.ArticuloCarritos.Update(articulo);
            context.SaveChanges();
        }

        await context.SaveChangesAsync();
        return NoContent();
    }

this was inserted in the cart article table: enter image description here

the carritoid became null and I don't want this to happen

Upvotes: 0

Views: 1958

Answers (1)

Steve Py
Steve Py

Reputation: 35093

How can I prevent the CarritoId from becoming null?

Ultimately by not passing entities between controller, view, and back. It may look like you are sending an entity to the view and the entity is being sent back in the Put, but what you are sending is a serialized JSON block and casting it to look like an entity. Doing this leads to issues like this where your Model becomes a serialization of an entity graph that may have missing pieces and gets mangled when you start attaching it to a DbContext to track as an entity. This is likely the case you're seeing, the data that was sent to the view was either incomplete, or by attaching the top level you're expecting all the related entities to be attached & tracked which isn't often the case. Altering FK references also can lead to unexpected results rather than updating available navigation properties. It also makes your application vulnerable to tampering as a malicious client or browser add-in can modify the data being sent to your server to adjust values your UI doesn't even present. Modern browser debug tools make this a pretty simple task.

Ideally the controller and view should communicate with view models based on loaded entities. This can also streamline the amount of data being shuttled around to reduce the payload size.

To help mitigate this, approach it as if the method did not receive an entity back. You are loading the entity again anyways but in your case you are doing nothing with it except detaching it again to try and attach your passed in JSON object. For example this code:

var cotizacionoriginal = context.Cotizaciones.Where(x => x.CotizacionId == cotizacion.CotizacionId).FirstOrDefault();

if (cotizacionoriginal != null)
{
    context.Entry(cotizacionoriginal).State = EntityState.Detached;
}

... does absolutely nothing for you. It says "Attempt to find an object with this ID in the local cache or database, and if you find it, stop tracking it."

You're effectively going to the database for no reason. This doesn't even assert that the row exists because you're using an "OrDefault" variation.

For a start, this should read more like:

var cotizacionoriginal = context.Cotizaciones
    .Where(x => x.CotizacionId == cotizacion.CotizacionId)
    .Single();

This says "Load the one row from the local cache or database that has this CotizacionId". This asserts that there is actually a matching row in the database. If the record passed into the Put has an invalid ID this will throw an exception. We don't want to detach it.

One further detail. Since we are going to want to manipulate child collections and references in this object, we should eager-load them:

var cotizacionoriginal = context.Cotizaciones
    .Include(x => x.Carriyo)
        .ThenInclude(c => c.Articulo)
            .ThenInclude(a => a.Producto)
    .Where(x => x.CotizacionId == cotizacion.CotizacionId).Single();

With larger object graphs this can get rather wordy as you have to drill down each chain of related entities. A better approach is rather than updating a "whole" object graph at once, break it up into smaller legal operations where one entity relationship can be dealt with at a time.

The next step would be to validate the passed in values in your Put object. Does it appear to be complete or is anything out of place? At a minimum we should check the current session user ID to verify that they have access to this loaded Cortizacion row and have permissions to edit it. If not, throw an exception. The web site's exception handling should ensure that any serious exception, like attempting to access rows that don't exist or don't have permissions to, should be logged for admin review, and the current session should be ended. Someone may be tampering with the system or you have a bug which is resulting in possible data corruption. Either way it should be detected, reported, and fixed with the current session terminated as a precaution.

The last step would be to go through the passed in object graph and alter your "original" data to match. The important thing here is again, you cannot trust/treat the passed in parameters as "entities", only deserialized data that looks like an entity. So, if the Product changed in one of the referenced items, we will fetch a reference and update it.

foreach (ArticuloCarrito articulo in cotizacion.Carrito.Articulos)
{
    if (articulo.ArticuloId == 0)
    { // TODO: Handle adding a new article if that is supported.
    }
    else
    {
        var existingArticulo = existingCarrito.Articulos.Single(x => x.ArticuloId == articulo.ArticuloId);
        if (existingArticulo.ProductoId != articulo.Producto.ProductoId)
        {
            var producto = context.Productos.Single(x => x.ProductoId == articulo.Producto.ProductoId);
            existingArticulo.Producto = producto;
        }
    }
}
await context.SaveChangesAsync();

Optionally above we might check the Articulo to see if a new row has been added. (No ID yet) If we do have an ID, then we check the existing Carrito Articles for a matching item. If one is not found then this would result in an exception. Once we have one, we check if the Product ID has changed. If changed, we don't use the "Producto" passed in, as that is a deserialized JSON object, so we go to the Context to load a reference and set it on our existing row.

context.SaveChanges should only be called once per operation rather than inside the loop.

When copying across values from a detached, deserialized entity to a tracked entity, you can use:

context.Entry(existingArticulo).CurrentValues.SetValues(articulo); 

However, this should only be done if the values in the passed in object are validated. As far as I know this only updates value fields, not FKs or object references.

Hopefully this gives you some ideas on things to try to streamline the update process.

Upvotes: 1

Related Questions