¿Debo verificar si existe algo en la base de datos y fallar rápidamente o esperar la excepción de la base de datos?

32

Tener dos clases:

public class Parent 
{
    public int Id { get; set; }
    public int ChildId { get; set; }
}

public class Child { ... }

Al asignar ChildIda Parentdebería comprobar primero si existe en la base de datos o esperar a que la base de datos a una excepción?

Por ejemplo (usando Entity Framework Core):

TENGA EN CUENTA que este tipo de comprobaciones ESTÁN EN TODA LA INTERNET incluso en los documentos oficiales de Microsoft: https://docs.microsoft.com/en-us/aspnet/mvc/overview/getting-started/getting-started-with-ef-using- mvc / handling-concurrency-with-the-entity-framework-in-an-asp-net-mvc-application # modify-the-department-controller pero hay un manejo adicional de excepciones paraSaveChanges

Además, tenga en cuenta que la intención principal de esta comprobación fue devolver un mensaje amigable y un estado HTTP conocido al usuario de la API y no ignorar por completo las excepciones de la base de datos. Y el único lugar en el que se lanzará una excepción es dentro SaveChangeso SaveChangesAsyncllamando ... para que no haya ninguna excepción cuando llame FindAsynco Any. Por lo tanto, si existe un elemento secundario pero se eliminó antes, SaveChangesAsyncse generará una excepción de concurrencia.

Hice esto debido a que la foreign key violationexcepción será mucho más difícil de formatear para mostrar "No se pudo encontrar el niño con ID {parent.ChildId}".

public async Task<ActionResult<Parent>> CreateParent(Parent parent)
{
    // is this code redundant?
   // NOTE: its probably better to use Any isntead of FindAsync because FindAsync selects *, and Any selects 1
    var child = await _db.Children.FindAsync(parent.ChildId);
    if (child == null)
       return NotFound($"Child with id {parent.ChildId} could not be found.");

    _db.Parents.Add(parent);    
    await _db.SaveChangesAsync();        

    return parent;
}

versus:

public async Task<ActionResult<Parent>> CreateParent(Parent parent)
{
    _db.Parents.Add(parent);
    await _db.SaveChangesAsync();  // handle exception somewhere globally when child with the specified id doesn't exist...  

    return parent;
}

El segundo ejemplo en Postgres arrojará un 23503 foreign_key_violationerror: https://www.postgresql.org/docs/9.4/static/errcodes-appendix.html

La desventaja de manejar excepciones de esta manera en ORM como EF es que solo funcionará con un backend de base de datos específico. Si alguna vez quisiste cambiar al servidor SQL o algo más, esto ya no funcionará porque el código de error cambiará.

No formatear la excepción correctamente para el usuario final podría exponer algunas cosas que no desea que nadie más que los desarrolladores vean.

Relacionado:

https://stackoverflow.com/questions/6171588/preventing-race-condition-of-if-exists-update-else-insert-in-entity-framework

https://stackoverflow.com/questions/4189954/implementing-if-not-exists-insert-using-entity-framework-without-race-conditions

https://stackoverflow.com/questions/308905/should-there-be-a-transaction-for-read-queries

Konrad
fuente
2
Compartir su investigación ayuda a todos . Cuéntanos qué has probado y por qué no satisfizo tus necesidades. Esto demuestra que te has tomado el tiempo para tratar de ayudarte a ti mismo, nos salva de reiterar respuestas obvias y, sobre todo, te ayuda a obtener una respuesta más específica y relevante. También vea Cómo preguntar
mosquito el
55
Como otros han mencionado, existe la posibilidad de que un registro se pueda insertar o eliminar simultáneamente con su comprobación de NotFound. Por esa razón, verificar primero parece una solución inaceptable. Si le preocupa escribir un manejo de excepciones específico de Postgres que no sea portátil a otros backends de bases de datos, intente estructurar el manejador de excepciones de tal manera que las funciones específicas de la base de datos (SQL, Postgres, etc.) puedan extender la funcionalidad principal
billrichards
3
Al revisar los comentarios, necesito decir esto: deja de pensar en lugares comunes . "Fail fast" no es una regla aislada y fuera de contexto que puede o debe seguirse a ciegas. Es una regla de oro. Siempre analice lo que realmente está tratando de lograr y luego considere cualquier técnica a la luz de si le ayuda a lograr ese objetivo o no. "Fail fast" lo ayuda a prevenir efectos secundarios no deseados. Y además, "fallar rápido" realmente significa "fallar tan pronto como pueda detectar que hay un problema". Ambas técnicas fallan tan pronto como se detecta un problema, por lo que debe considerar otras consideraciones.
jpmc26
1
@Konrad, ¿qué tienen que ver las excepciones con él? Deja de pensar en las condiciones de carrera como algo que vive en tu código: es una propiedad del universo. Cualquier cosa, cualquier cosa que toque un recurso que no controla por completo (por ejemplo, acceso directo a memoria, memoria compartida, base de datos, API REST, sistema de archivos, etc., etc.) más de una vez y espera que no se modifique tiene una posible condición de carrera. Diablos, tratamos esto en C, que ni siquiera tiene excepciones. Simplemente no se bifurque en el estado de un recurso que no controla si al menos una de las ramas se mete con el estado de ese recurso.
Jared Smith
1
@DanielPryden En mi pregunta, no dije que no quisiera manejar las excepciones de la base de datos (sé que las excepciones son inevitables). Creo que muchas personas lo malinterpretaron, quería tener un mensaje de error amigable para mi API web (para que lo lean los usuarios finales) Child with id {parent.ChildId} could not be found.. Y formatear "Violación de clave externa" creo que es peor en este caso.
Konrad

Respuestas:

3

Es una pregunta confusa, pero , debe verificar primero y no solo manejar una excepción de base de datos.

En primer lugar, en su ejemplo, está en la capa de datos, utilizando EF directamente en la base de datos para ejecutar SQL. Tu código es equivalente a correr

select * from children where id = x
//if no results, perform logic
insert into parents (blah)

La alternativa que sugiere es:

insert into parents (blah)
//if exception, perform logic

Usar la excepción para ejecutar la lógica condicional es lento y universalmente mal visto.

Tienes una condición de carrera y debes usar una transacción. Pero esto se puede hacer completamente en código.

using (var transaction = new TransactionScope())
{
    var child = await _db.Children.FindAsync(parent.ChildId);
    if (child == null) 
    {
       return NotFound($"Child with id {parent.ChildId} could not be found.");
    }

    _db.Parents.Add(parent);    
    await _db.SaveChangesAsync();        
    transaction.Complete();

    return parent;
}

La clave es preguntarse:

"¿Esperas que ocurra esta situación?"

Si no, entonces seguro, inserte y arroje una excepción. Pero solo maneje la excepción como cualquier otro error que pueda ocurrir.

Si espera que ocurra, NO es excepcional y debe verificar si el niño existe primero, respondiendo con el mensaje amistoso apropiado si no es así.

Editar : parece haber mucha controversia sobre esto. Antes de votar negativamente, considere:

A. ¿Qué pasaría si hubiera dos restricciones FK? ¿Recomendaría analizar el mensaje de excepción para determinar qué objeto faltaba?

B. Si tiene una falla, solo se ejecuta una instrucción SQL. Solo los hits generan el gasto adicional de una segunda consulta.

C. Por lo general, Id sería una clave sustituta. Es difícil imaginar una situación en la que conozca una y no esté seguro de que esté en la base de datos. La comprobación sería extraña. Pero, ¿qué pasa si es una clave natural que el usuario ha ingresado? Eso podría tener una alta probabilidad de no estar presente

Ewan
fuente
Los comentarios no son para discusión extendida; Esta conversación se ha movido al chat .
maple_shaft
1
¡Esto es totalmente incorrecto y engañoso! Son respuestas como esta las que producen malos profesionales contra los que siempre tengo que luchar. SELECT nunca bloquea una tabla, por lo que entre SELECT y INSERT, UPDATE o DELTE, el registro puede cambiar. Por lo tanto, es un mal diseño del software y un accidente que espera suceder en la producción.
Daniel Lobo
1
@DanielLobo transaccionescope arregla eso
Ewan
1
pruébalo si no me crees
Ewan
1
@yusha Tengo el código aquí mismo
Ewan
111

Comprobar la unicidad y luego configurar es un antipatrón; siempre puede suceder que la ID se inserte simultáneamente entre el tiempo de verificación y el tiempo de escritura. Las bases de datos están equipadas para tratar este problema a través de mecanismos como restricciones y transacciones; la mayoría de los lenguajes de programación no lo son. Por lo tanto, si valora la consistencia de los datos, déjelo al experto (la base de datos), es decir, inserte y capture una excepción si ocurre.

Kilian Foth
fuente
34
comprobar y fallar no es más rápido que simplemente "intentar" y esperar lo mejor. El primero implica 2 operaciones a ser implementadas y realizadas por su sistema y 2 por el DB, mientras que el último solo implica una de ellas. La verificación se delega al servidor de base de datos. También implica un salto menos a la red y una tarea menos a la que debe asistir el DB. Podríamos pensar que una consulta más a la base de datos es asequible, pero a menudo nos olvidamos de pensar en grande. Piense en alta concurrencia desencadenando la consulta más de cien veces Podría duplicar todo el tráfico a la base de datos. Si eso importa depende de usted decidir.
Laiv
66
@Konrad Mi posición es que la opción correcta por defecto es una consulta que fallará por sí sola, y es el enfoque de verificación previa de consultas por separado el que tiene la carga de la prueba para justificarse. En cuanto a "convertirse en un problema": entonces está utilizando transacciones o de otra manera se asegura de estar a salvo de los errores de ToCToU , ¿verdad? Para mí no es obvio por el código publicado que usted es, pero si no lo es, entonces ya se ha convertido en un problema de la misma manera que una bomba de relojería se convierte en un problema mucho antes de que realmente explote.
mtraceur
44
@Konrad EF Core no va a colocar implícitamente tanto su cheque como el inserto en una transacción, deberá solicitarlo explícitamente. Sin la transacción, la verificación primero no tiene sentido ya que el estado de la base de datos puede cambiar entre la verificación y la inserción de todos modos. Incluso con una transacción, es posible que no evite que la base de datos cambie bajo sus pies. Hace unos años, nos encontramos con un problema usando EF con Oracle, donde aunque la base de datos lo admite, la entidad no estaba activando el bloqueo de los registros de lectura dentro de una transacción, y solo el inserto se trataba como transaccional.
Mr.Mindor
3
"Comprobar la unicidad y luego configurar es un antipatrón" No diría esto. Depende en gran medida de si puede suponer que no están ocurriendo otras modificaciones y de si la verificación produce algún resultado más útil (incluso solo un mensaje de error que realmente significa algo para el lector) cuando no existe. Con una base de datos que maneja solicitudes web simultáneas, no, no puede garantizar que no estén ocurriendo otras modificaciones, pero hay casos en los que es una suposición razonable.
jpmc26
55
Comprobar la unicidad primero no elimina la necesidad de manejar posibles fallas. Por otro lado, si una acción requeriría realizar varias operaciones, verificar si es probable que todas tengan éxito antes de comenzar cualquiera de ellas es a menudo mejor que realizar acciones que probablemente necesiten revertirse. Hacer las comprobaciones iniciales puede no evitar todas las situaciones en las que sería necesaria una reversión, pero podría ayudar a reducir la frecuencia de tales casos.
Supercat
38

Creo que lo que llamas "falla rápido" y lo que yo llamo no es lo mismo.

Decirle a la base de datos que haga un cambio y manejar la falla, eso es rápido. Su camino es complicado, lento y no particularmente confiable.

Esa técnica suya no es fallar rápidamente, es "verificación previa". A veces hay buenas razones, pero no cuando usa una base de datos.

gnasher729
fuente
1
Hay casos en los que necesita una segunda consulta cuando una clase depende de otra, por lo que no tiene otra opción en casos como ese.
Konrad
44
Pero no aquí. Y las consultas a la base de datos pueden ser bastante inteligentes, por lo que generalmente dudo de la "no elección".
gnasher729
1
Creo que también depende de la aplicación, si la creas solo para unos pocos usuarios, no debería hacer una diferencia y el código es más legible con 2 consultas.
Konrad
21
Está suponiendo que su base de datos está almacenando datos inconsistentes. En otras palabras, parece que no confías en tu base de datos y en la consistencia de los datos. Si ese fuera el caso, tiene un gran problema y su solución es una solución alternativa. Una solución paliativa destinada a ser anulada más pronto que tarde. Podría haber casos en los que se vea obligado a consumir un DB fuera de su control y administración. De otras aplicaciones. En esos casos, consideraría tales validaciones. En cualquier caso, @gnasher tiene razón, la suya no falla rápidamente o no es lo que entendemos como falla rápida.
Laiv
15

Esto comenzó como un comentario pero creció demasiado.

No, como han dicho las otras respuestas, este patrón no debe usarse. *

Cuando se trata de sistemas que usan componentes asincrónicos, siempre habrá una condición de carrera en la que la base de datos (o sistema de archivos u otro sistema asíncrono) puede cambiar entre la verificación y el cambio. Una verificación de este tipo simplemente no es una forma confiable de evitar el tipo de error que no desea manejar.
Peor que no ser suficiente, de un vistazo da la impresión de que debería evitar el error de registro duplicado dando una falsa sensación de seguridad.

Necesita el manejo de errores de todos modos.

En los comentarios, ha preguntado si necesita datos de múltiples fuentes.
Aún no.

La cuestión fundamental no desaparece si lo que desea verificar se vuelve más complejo.

Todavía necesita el manejo de errores de todos modos.

Incluso si esta verificación fuera una forma confiable de prevenir el error particular del que está tratando de protegerse, aún pueden ocurrir otros errores. ¿Qué sucede si pierde la conexión a la base de datos, o se queda sin espacio, o?

Es muy probable que aún necesite otro manejo de errores relacionado con la base de datos de todos modos. El manejo de este error en particular probablemente debería ser una pequeña parte de él.

Si necesita datos para determinar qué cambiar, obviamente tendrá que recopilarlos de alguna parte. (dependiendo de las herramientas que esté utilizando, probablemente haya mejores formas que las consultas separadas para recopilarlas) Si, al examinar los datos que recopiló, determina que no necesita realizar el cambio, después de todo, excelente, no realice cambio. Esta determinación está completamente separada de las preocupaciones de manejo de errores.

Todavía necesita el manejo de errores de todos modos.

Sé que estoy siendo repetitivo, pero creo que es importante aclarar esto. Ya he limpiado este desastre antes.

Fallará eventualmente. Cuando falla, será difícil y lento llegar al fondo. Resolver problemas que surgen de las condiciones de carrera es difícil. No suceden constantemente, por lo que será difícil o incluso imposible reproducirse de forma aislada. Para empezar, no introdujo el manejo de errores adecuado, por lo que es probable que no tenga mucho para continuar: tal vez el informe de un usuario final de algún texto críptico (que oye, estaba tratando de evitar que se vea en primer lugar). Tal vez una traza de pila que señala de nuevo a esa función que cuando la miras descaradamente niega que el error sea incluso posible.

* Puede haber razones comerciales válidas para realizar estas verificaciones existentes, como evitar que la aplicación duplique el trabajo costoso, pero no es un reemplazo adecuado para el manejo adecuado de errores.

Mr.Mindor
fuente
2

Creo que una cosa secundaria a tener en cuenta aquí: una de las razones por las que desea esto es para que pueda formatear un mensaje de error para que el usuario lo vea.

Recomiendo encarecidamente que:

a) muestre al usuario final el mismo mensaje de error genérico para cada error que ocurra.

b) registre la excepción real en algún lugar al que solo los desarrolladores puedan acceder (si está en un servidor) o en algún lugar que pueda enviarle mediante herramientas de informe de errores (si el cliente está implementado)

c) no intente formatear los detalles de excepción de error que registra a menos que pueda agregar más información útil. No querrás haber "formateado" accidentalmente la única información útil que hubieras podido usar para rastrear un problema.


En resumen, las excepciones están llenas de información técnica muy útil. Nada de esto debe ser para el usuario final y usted pierde esta información bajo su propio riesgo.

Arrozal
fuente
2
"muestra al usuario final el mismo mensaje de error genérico para cada error que ocurre". esa fue la razón principal, formatear la excepción para el usuario final parece una cosa horrible de hacer ...
Konrad
1
En cualquier sistema de base de datos razonable, debería poder averiguar mediante programación por qué algo ha fallado. No debería ser necesario analizar un mensaje de excepción. Y de manera más general: ¿quién dice que se debe mostrar un mensaje de error al usuario? Puede fallar la primera inserción y volver a intentar en un bucle hasta que tenga éxito (o hasta cierto límite de reintentos o tiempo). Y de hecho, el retroceso y el reintento es algo que querrá implementar eventualmente de todos modos.
Daniel Pryden