Dois-je vérifier si quelque chose existe dans la base de données et échouer rapidement ou attendre une exception de base de données

32

Ayant deux classes:

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

public class Child { ... }

Lors de l'affectation ChildIdà, Parentdois-je d'abord vérifier s'il existe dans la base de données ou attendre que la base de données lève une exception?

Par exemple (en utilisant Entity Framework Core):

REMARQUE: ces types de vérifications sont TOUS SUR INTERNET, même sur les documents officiels de Microsoft: https://docs.microsoft.com/en-us/aspnet/mvc/overview/getting-started/getting-started-with-ef-using- mvc / traitement-concurrence-avec-l'entité-structure-dans-un-asp-net-mvc-application # modifier-le-contrôleur-département, mais il existe une gestion supplémentaire des exceptions pourSaveChanges

Notez également que l'objectif principal de cette vérification était de renvoyer un message convivial et un statut HTTP connu à l'utilisateur de l'API et de ne pas ignorer complètement les exceptions de base de données. Et la seule exception à être levée est à l'intérieur SaveChangesou SaveChangesAsyncappeler ... donc il n'y aura pas d'exception lorsque vous appelez FindAsyncou Any. Donc, si l'enfant existe mais a été supprimé avant, SaveChangesAsyncune exception d'accès simultané sera levée.

J'ai fait cela parce que l' foreign key violationexception sera beaucoup plus difficile à formater pour afficher "L'enfant avec l'id {parent.ChildId} est introuvable."

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;
}

contre:

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;
}

Le deuxième exemple dans Postgres va générer une 23503 foreign_key_violationerreur: https://www.postgresql.org/docs/9.4/static/errcodes-appendix.html

L'inconvénient de la gestion des exceptions de cette manière dans ORM comme EF est que cela ne fonctionnera qu'avec un système de base de données spécifique. Si vous avez toujours voulu passer au serveur SQL ou à quelque chose d'autre, cela ne fonctionnera plus car le code d'erreur sera modifié.

Ne pas formater correctement l'exception pour l'utilisateur final pourrait exposer des choses que vous ne voulez que les développeurs voient.

En relation:

https://stackoverflow.com/questions/6171588/preventing-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
la source
2
Partager vos recherches aide tout le monde . Dites-nous ce que vous avez essayé et pourquoi il ne répondait pas à vos besoins. Cela montre que vous avez pris le temps d'essayer de vous aider, que cela nous évite de répéter des réponses évidentes et, surtout, que cela vous aide à obtenir une réponse plus précise et plus pertinente. Voir aussi Comment poser une question
Gnat, le
5
Comme d'autres l'ont mentionné, il est possible qu'un enregistrement puisse être inséré ou supprimé simultanément à votre vérification de NotFound. Pour cette raison, vérifier d'abord semble être une solution inacceptable. Si vous souhaitez écrire des traitements d'exception spécifiques à Postgres qui ne soient pas portables vers d'autres systèmes de bases de données, essayez de structurer le gestionnaire d'exceptions de manière à ce que les fonctionnalités principales puissent être étendues à l'aide de classes spécifiques à la base de données (SQL, Postgres, etc.)
billrichards
3
En regardant à travers les commentaires, je dois dire ceci: arrêtez de penser en platitudes . "Fail fast" n'est pas une règle isolée hors contexte qui peut ou devrait être suivie aveuglément. C'est une règle de base. Analysez toujours ce que vous essayez réellement d’atteindre, puis envisagez toute technique à la lumière de la possibilité de l’atteindre ou non. "Fail fast" vous aide à prévenir les effets secondaires non souhaités. De plus, "échec rapide" signifie vraiment "échec dès que vous pouvez détecter un problème". Les deux techniques échouant dès qu'un problème est détecté, vous devez donc prendre en compte d'autres considérations.
jpmc26
1
@ Konrad Qu'est-ce que les exceptions ont à voir avec cela? Arrêtez de penser aux conditions de course comme à quelque chose qui habite votre code: c'est une propriété de l'univers. Tout ce qui touche une ressource qu’il ne contrôle pas complètement (accès direct à la mémoire, mémoire partagée, base de données, API REST, système de fichiers, etc.) plus d’une fois et s’attendant à ce qu’il soit inchangé présente une éventuelle condition de concurrence critique. Heck, nous traitons cela en C qui n'a même pas d' exceptions. Il suffit simplement de ne jamais créer de branche sur l'état d'une ressource que vous ne contrôlez pas si au moins une de ses branches est en désordre avec l'état de cette ressource.
Jared Smith
1
@DanielPryden Dans ma question, je n'ai pas dit que je ne voulais pas gérer les exceptions de base de données (je sais que les exceptions sont inévitables). Je pense que beaucoup de gens ont mal compris, je voulais un message d'erreur convivial pour mon API Web (que les utilisateurs finaux puissent lire), par exemple Child with id {parent.ChildId} could not be found.. Et le formatage "violation de clé étrangère" est selon moi pire dans ce cas.
Konrad

Réponses:

3

Plutôt une question confuse, mais OUI vous devriez vérifier d'abord et ne pas manipuler simplement une exception de base de données.

Tout d’abord, dans votre exemple, vous vous trouvez au niveau de la couche de données, en utilisant EF directement sur la base de données pour exécuter SQL. Votre code est équivalent à courir

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

L'alternative que vous suggérez est:

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

L'utilisation de l'exception pour exécuter une logique conditionnelle est lente et universellement mal vue.

Vous avez une condition de concurrence critique et devez utiliser une transaction. Mais cela peut être entièrement fait dans le code.

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;
}

L'essentiel est de vous demander:

"Vous attendez-vous à ce que cette situation se produise?"

Si ce n'est pas le cas, insérez-le et jetez une exception. Mais gérez simplement l'exception comme toute autre erreur pouvant survenir.

Si vous vous attendez à ce que cela se produise, cela n’est PAS exceptionnel et vous devriez vérifier si l’enfant existe en premier, en répondant avec le message convivial approprié, dans le cas contraire.

Edit - Il semble y avoir beaucoup de controverse à ce sujet. Avant que vous ne votiez vers le bas, considérez:

A. Et s'il y avait deux contraintes FK. Recommanderiez-vous l'analyse du message d'exception pour déterminer quel objet manquait?

B. Si vous avez un échec, une seule instruction SQL est exécutée. Ce ne sont que les hits qui entraînent la dépense supplémentaire d'une seconde requête.

C. Habituellement, Id serait une clé de substitution. Il est difficile d’imaginer une situation dans laquelle vous en connaissez une et où vous n'êtes pas sûr que ce soit sur la base de données. Vérifier serait étrange. Mais que se passe-t-il s'il s'agit d'une clé naturelle saisie par l'utilisateur? Cela pourrait avoir une grande chance de ne pas être présent

Ewan
la source
Les commentaires ne sont pas pour une discussion prolongée; cette conversation a été déplacée pour discuter .
maple_shaft
1
C'est totalement faux et trompeur! Ce sont des réponses comme celle-ci qui produisent de mauvais professionnels contre lesquels je dois toujours lutter. SELECT ne verrouille jamais une table. Par conséquent, l'enregistrement peut changer entre les commandes SELECT et INSERT, UPDATE ou DELTE. Il s’agit donc d’un logiciel médiocre digne et d’un accident en attente de production.
Daniel Lobo
1
@DanielLobo transactionscope corrige cela
Ewan
1
testez-le si vous ne me croyez pas
Ewan
1
@yusha j'ai le code ici
Ewan
111

La vérification de l'unicité et du réglage est un anti-modèle; il peut toujours arriver que l'ID soit inséré simultanément entre l'heure de contrôle et l'heure d'écriture. Les bases de données sont équipées pour traiter ce problème par le biais de mécanismes tels que des contraintes et des transactions; la plupart des langages de programmation ne le sont pas. Par conséquent, si vous accordez de la valeur à la cohérence des données, laissez-le à l'expert (la base de données), c'est-à-dire faites l'insertion et capturez une exception si cela se produit.

Kilian Foth
la source
34
vérifier et échouer n'est pas plus rapide que simplement "essayer" et espérer le meilleur. Ancien implique 2 opérations à implémenter et à exécuter par votre système et 2 opérations par la base de données, tandis que la dernière en implique une seule. La vérification est déléguée au serveur de base de données. Cela implique également un saut de moins dans le réseau et une tâche de moins à la charge de la base de données. Nous pourrions penser qu’une autre requête à la base de données est abordable, mais nous oublions souvent de penser en gros. Pensez en cas de forte simultanéité pour déclencher la requête cent fois plus. Cela pourrait piéger tout le trafic vers la base de données. Si cela compte, c'est à vous de décider.
Laiv
6
@ Konrad Ma position est que le choix correct par défaut est une requête qui échouera d'elle-même, et que c'est l' approche de pré-enregistrement de requête distincte qui a la charge de la preuve de se justifier. Pour ce qui est « devenu un problème »: si vous êtes utilisez les transactions ou autrement assurer que vous êtes en sécurité contre les erreurs ToCToU , non? D'après le code affiché, ce n'est pas évident pour moi, mais si vous ne l'êtes pas, c'est déjà devenu un problème, tout comme une bombe à retardement bien avant qu'elle n'explose.
mtraceur
4
@Konrad EF Core ne va pas implicitement mettre votre chèque et l'insertion dans une transaction, vous devrez le demander explicitement. Sans la transaction, la vérification préalable est inutile car l’état de la base de données peut changer entre la vérification et l’insertion. Même avec une transaction, vous ne pouvez pas empêcher la base de données de changer sous vos pieds. Il y a quelques années, nous avons rencontré un problème d'utilisation d'EF avec Oracle. Bien que la base de données le prenne en charge, Entity ne déclenchait pas le verrouillage des enregistrements lus dans une transaction et seul l'insert était traité comme transactionnel.
Mr.Mindor le
3
"Vérifier l'unicité et le réglage est un antipattern" Je ne dirais pas cela. Cela dépend fortement de savoir si vous pouvez supposer qu'aucune autre modification n'est en train de se produire et si le contrôle produit un résultat plus utile (même un simple message d'erreur qui signifie quelque chose pour le lecteur) lorsqu'il n'existe pas. Avec une base de données traitant des demandes Web simultanées, non, vous ne pouvez pas garantir que d'autres modifications ne se produiront pas, mais il existe des cas où cela est une hypothèse raisonnable.
jpmc26
5
Contrôler d'abord l'unicité n'élimine pas la nécessité de gérer les défaillances possibles. D'autre part, si une action nécessiterait d' effectuer plusieurs opérations, vérifier si tous doivent probablement réussir avant de commencer l' un d'eux est souvent mieux que d' effectuer des actions qui pourraient probablement besoin d'être annulées. Effectuer les vérifications initiales n’évitera peut-être pas toutes les situations dans lesquelles un retour en arrière serait nécessaire, mais cela pourrait contribuer à réduire la fréquence de tels cas.
Supercat
38

Je pense que ce que vous appelez «échec rapide» et ce que j'appelle n'est pas la même chose.

Dire à la base de données de faire un changement et gérer l'échec, c'est rapide. Votre chemin est compliqué, lent et pas particulièrement fiable.

Votre technique n’est pas un échec rapide, c’est un contrôle en amont. Il y a parfois de bonnes raisons, mais pas lorsque vous utilisez une base de données.

gnasher729
la source
1
Il y a des cas où vous avez besoin d'une seconde requête lorsqu'une classe dépend d'une autre, vous n'avez donc pas le choix dans de tels cas.
Konrad
4
Mais pas ici. Et les requêtes dans les bases de données peuvent être assez astucieuses, alors je doute généralement du «pas de choix».
gnasher729
1
Je pense que cela dépend aussi de l'application. Si vous le créez uniquement pour quelques utilisateurs, cela ne devrait pas faire de différence et le code est plus lisible avec 2 requêtes.
Konrad
21
Vous supposez que votre base de données stocke des données incohérentes. En d'autres termes, on dirait que vous ne faites pas confiance à votre base de données et à la cohérence des données. Si c'était le cas, vous avez un très gros problème et votre solution est un contournement. Une solution palliative destinée à être rejetée plus tôt que plus tard. Il peut arriver que vous soyez obligé de consommer une BD hors de votre contrôle et de votre gestion. A partir d'autres applications. Dans ces cas, je considérerais de telles validations. Dans tous les cas, @Glasher a raison, le vôtre n’échoue pas rapidement ou c’est ce que nous comprenons comme échec rapide.
Laiv
15

Cela a commencé comme un commentaire mais est devenu trop grand.

Non, comme l'ont indiqué les autres réponses, ce schéma ne doit pas être utilisé. *

Lorsqu'il s'agit de systèmes utilisant des composants asynchrones, il y aura toujours une situation de concurrence critique dans laquelle la base de données (ou le système de fichiers ou un autre système asynchrone) peut changer entre la vérification et la modification. Une vérification de ce type n’est tout simplement pas un moyen fiable d’empêcher le type d’erreur que vous ne voulez pas traiter.
Pire que ne pas être suffisant, cela donne en un coup d’œil l’impression qu’il faut éviter que le double des enregistrements ne donne un faux sentiment de sécurité.

De toute façon, vous avez besoin du traitement des erreurs.

Dans les commentaires, vous avez demandé si vous aviez besoin de données provenant de plusieurs sources.
Toujours pas.

La question fondamentale ne disparaît pas si ce que vous voulez vérifier devient plus complexe.

De toute façon, vous avez toujours besoin du traitement des erreurs.

Même si cette vérification était un moyen fiable d’éviter l’erreur particulière contre laquelle vous essayez de vous protéger, d’autres erreurs peuvent quand même se produire. Que se passe-t-il si vous perdez la connexion à la base de données, si elle manque d'espace ou?

De toute façon, vous avez probablement toujours besoin d'une autre gestion d'erreur liée à la base de données. Le traitement de cette erreur particulière devrait probablement en être une petite partie.

Si vous avez besoin de données pour déterminer ce qu’il faut changer, vous devrez évidemment les collecter quelque part. (En fonction des outils que vous utilisez, il existe probablement de meilleurs moyens que de les collecter séparément.) Si, en examinant les données que vous avez collectées, vous déterminez que vous n'avez pas besoin de faire le changement après tout, tant mieux, ne faites pas de même. changement. Cette détermination est complètement distincte des problèmes de traitement des erreurs.

De toute façon, vous avez toujours besoin du traitement des erreurs.

Je sais que je suis répétitif, mais j'estime qu'il est important de le préciser. J'ai nettoyé ce gâchis avant.

Cela finira par échouer. En cas d'échec, il sera difficile et long d'aller au fond des choses. Il est difficile de résoudre les problèmes liés aux conditions de course. Ils ne se produisent pas régulièrement, il sera donc difficile voire impossible de reproduire de manière isolée. Pour commencer, vous n'avez pas mis en place la gestion correcte des erreurs, vous n'aurez donc probablement pas grand chose à faire: peut-être qu'un utilisateur final rapportera un texte crypté (que vous tentiez d'empêcher de voir dès le départ). Une trace de pile qui renvoie peut-être à cette fonction et qui nie de façon flagrante l'erreur devrait même être possible.

* Il peut exister des raisons commerciales valables pour exécuter ces vérifications existantes, par exemple pour empêcher l'application de dupliquer un travail coûteux, mais cela ne constitue pas une solution de remplacement appropriée pour une gestion correcte des erreurs.

Mr.Mindor
la source
2

Je pense qu’un élément secondaire à noter ici - une des raisons pour lesquelles vous voulez que cela soit afin que vous puissiez formater un message d’erreur pour que l’utilisateur puisse le voir.

Je vous recommande vivement de:

a) montrer à l'utilisateur final le même message d'erreur générique pour chaque erreur qui se produit.

b) enregistrez l'exception réelle à un endroit auquel seuls les développeurs peuvent accéder (si sur un serveur) ou à un endroit qui peut vous être envoyé par des outils de rapport d'erreur (si le client est déployé)

c) n'essayez pas de formater les détails de l'exception d'erreur que vous enregistrez, sauf si vous pouvez ajouter d'autres informations utiles. Vous ne voulez pas avoir accidentellement «formaté» la seule information utile que vous auriez pu utiliser pour suivre un problème.


En bref, les exceptions regorgent d’informations techniques très utiles. Cela ne devrait en aucun cas être destiné à l'utilisateur final et vous perdez cette information à vos risques et périls.

Paddy
la source
2
"affiche le même message d'erreur générique à l'utilisateur final pour chaque erreur qui se produit." c’était la raison principale, formater l’exception pour l’utilisateur final ressemble à une chose horrible à faire ..
Konrad
1
Dans tout système de base de données raisonnable, vous devriez être capable de trouver par programme pourquoi quelque chose a échoué. Il ne devrait pas être nécessaire d'analyser un message d'exception. Et plus généralement: qui dit qu'un message d'erreur doit être affiché à l'utilisateur? Vous pouvez échouer lors de la première insertion et réessayer dans une boucle jusqu'à ce que vous réussissiez (ou jusqu'à une certaine limite de tentatives ou de temps). Et, en fait, vous souhaiterez de toute façon implémenter un retour en arrière et une nouvelle tentative.
Daniel Pryden