Projet

Général

Profil

Development #22376

API users, avoir la possibilité d'un get_or_create

Ajouté par Frédéric Péters il y a 10 mois. Mis à jour il y a 3 mois.

Statut:
Solution proposée
Priorité:
Normal
Assigné à:
Catégorie:
-
Version cible:
-
Début:
08 mar. 2018
Echéance:
% réalisé:

0%

Patch proposed:
Oui

Description

Un formulaire côté wcs qui demande quatre adresses email mais la même peut être réutilisée (parce que c'est genre "adresse du responsable du service A", "... du service B", etc. et que la même personne peut être en charge de différents services); un workflow qui doit créer pour chacune de ces adresses un compte, on peut construire ça avec un appel vérifiant l'existance et permettant de récupérer l'uuid existant si présent puis un autre appel de création si ce n'était pas le cas, mais ce serait plus simple d'avoir un appel type "get or create".

0001-add-get-or-create-users-api-call-22376.patch Voir (6,12 ko) Paul Marillonnet, 05 oct. 2018 16:04

Historique

#1 Mis à jour par Benjamin Dauvergne il y a 10 mois

Je verrai plutôt des préconditions, genre :

POST /api/users/

{
   ...
   'email': 'whatever@whatever.org',
   'preconditions': {
      'not_exists': [
         {
           'ou': 'agents',
           'email': 'whatever@whatever.org',
         }
       ]
    }
}

Si ça ne matche pas je retourne un 412 Precondition failed avec le contenu JSON explicatif.

#2 Mis à jour par Frédéric Péters il y a 10 mois

avec le contenu JSON explicatif

C'est-à-dire ?

Le truc important avec le get_or_create c'est qu'on récupère ainsi les bonnes infos de l'utilisateur d'une unique manière, qu'on peut ensuite utiliser le long du workflow de manière unique et simple, pour par exemple ensuite appeler un autre webservice en passant identifiant_de_l_appel_create_response_uuid, que l'utilisateur ait été créé ou pas.

#3 Mis à jour par Benjamin Dauvergne il y a 10 mois

Ok bon le gros souci c'est que faire un get_or_create sur des champs sans index d'unicité c'est juste impossible avec Django, le seul moyen c'est de faire comme dans w.c.s.:
  • passer en autocommit
  • boucler sur
    1. chercher via les champs uniques, si trouvé on retourne
    2. sinon créer un utilisateur, obtenir un id x
    3. faire un select, vérifier si x est l'id le plus petit si plusieurs objets trouvés, sinon supprimer x et recommencer en 1

À voir comment désactiver les transaction et les remettre à la pince à épiler où c'est nécessaire.

#4 Mis à jour par Paul Marillonnet il y a 10 mois

Benjamin Dauvergne a écrit :

  • boucler sur
    1. chercher via les champs uniques, si trouvé on retourne

Pourquoi via les champs uniques, alors que le get_or_create se fait potentiellement via des champs non uniques ?

2. sinon créer un utilisateur, obtenir un id x

Pourquoi commencer par créer l'utilisateur si on est en mode autocommit ?
C'est par souci de cohérence de la base ?
Qu'est-ce que ça change de faire le select et la vérification après la création de l'utilisateur ?

3. faire un select, vérifier si x est l'id le plus petit si plusieurs objets trouvés, sinon supprimer x et recommencer en 1

Ok

#5 Mis à jour par Benjamin Dauvergne il y a 4 mois

Paul Marillonnet a écrit :

Benjamin Dauvergne a écrit :

  • boucler sur
    1. chercher via les champs uniques, si trouvé on retourne

Pourquoi via les champs uniques, alors que le get_or_create se fait potentiellement via des champs non uniques ?

get_or_create() ne garantit pas l'unicité sur des champs non unique (il faut au moins couvrir un index d'unicité dans les critères, soit mono champ soit multi champ mais il faut le couvrir, i.e. que toutes ses colonnes soient citées).

2. sinon créer un utilisateur, obtenir un id x

Pourquoi commencer par créer l'utilisateur si on est en mode autocommit ?
C'est par souci de cohérence de la base ?
Qu'est-ce que ça change de faire le select et la vérification après la création de l'utilisateur ?

C'est pour simuler du verrouillage optimise sans verrou, en vrai c'est limite sans transactions c'est quasiment impossible d'assurer l'unicité sans index sauf à avoir une limite haute sur le temps d'exécution des différents clients.

3. faire un select, vérifier si x est l'id le plus petit si plusieurs objets trouvés, sinon supprimer x et recommencer en 1

Ok

En fait en autocommit il vaudrait mieux boucler sur quelque chose comme cela (exécution concurrente possible):

Temps Client 1 Client 2
1 COUNT x=1 -> 0
2 COUNT x=1 -> 0
3 INSERT x=1 -> id=a
4 INSERT x=1 -> id=b
5 COUNT x=1 -> 2
6 COUNT x=1 -> 2
7 DELETE id=a
8 wait random() ms; go to 1 DELETE id=b
9 wait random() ms; go to 1

On fait un select pour voir si ce qu'on veut insérer est là, sinon on insère, on revérifie, sin on plus d'une ligne, on supprime celle qu'on vient d'insérer et on recommence en attendant un délai aléatoire (c'est un peu comme Ethernet, https://fr.wikipedia.org/wiki/Carrier_Sense_Multiple_Access_with_Collision_Detection).

C'est plus sûr que mon idée de garder l'id le plus petit (dans le cas où c'est le client avec l'id le plus grand qui fait la vérification en premier, i.e. si 6 se déroule avant 5 et que b > a alors ça ne marche pas, il vaut mieux dans tous le cas que celui qui détecte le doublon ré-essaye).

Ça marche parce qu'on est sûr qu'en autocommit 3 ou 4 s'exécute en premier et entièrement, donc en cas d'exécution entrecoupées, un des deux clients verra forcément un doublon.

On peut avoir un doublon en cas de crash entre le INSERT et la deuxième vérification, mais c'est plus improbable.

Il ne faut surtout pas exécuter tout cela dans une transaction parce qu'avec le niveau d'isolation par défaut de postgres (READ COMITTED) et bien les deux transactions peuvent faire tout cela sans se voir (READ COMITTED, on ne voit que les écritures "comittés" pendant la transaction).

#6 Mis à jour par Paul Marillonnet il y a 4 mois

  • Assigné à mis à Paul Marillonnet

#7 Mis à jour par Paul Marillonnet il y a 4 mois

Merci Benjamin pour ces explications. Je vais partir là dessus.

Et oui en effet, il y a du code similaire dans wcs, je viens de découvrir cela.
Par exemple ici :
https://git.entrouvert.org/wcs.git/tree/wcs/sql.py#n831

Edit: ici l'exemple choisi n'est peut-être pas le meilleur, parce que le passage en mode autocommit se fait dans la fonction appelante.
Mais l'idée est là je crois.

#8 Mis à jour par Paul Marillonnet il y a 4 mois

J'ai du mal à m'imaginer quelle serait l'interface viable et implémentable à l'aide du framework REST Django.

Benjamin, est-ce que ta première remarque dans ce ticket reste valable ? (introduire un champ 'preconditions' dans le payload JSON -- auquel cas, ce serait plutôt un "create_or_get")

Si oui, est-ce envisageable du point de vue de la cohérence de l'API, de renvoyer dans le payload, en plus du message explicatif de la réponse HTTP 412, le JSON de l'utilisateur déjà existant ?

Ou, alternative, virer ce champ "preconditions", et patcher l'API users pour renvoyer un HTTP 409 Conflict, en réponse au POST si l'utilisateur existe déjà, avec en payload le JSON correspondant à cet utilisateur ?

#9 Mis à jour par Paul Marillonnet il y a 4 mois

Je voudrais bien partir, côté API, sur l'option "renvoyer un HTTP 409 avec un JSON décrivant le compte utilisateur, lorsque l'on tente de le créer alors qu'il existe déjà".
Et donc, pour faire un "get or create" côté appel WS, il faudra faire un POST avec les champs de l'usager que l'on souhaite créer s'il n'existe as déjà (ce serait donc plutôt un "create or get").
On se ramène bien à un seul appel. Est-ce c'est bon ou bien ça pose problème côté w.c.s. ?

#10 Mis à jour par Paul Marillonnet il y a 4 mois

Discussion avec Thomas là-dessus.
Est-ce qu'on pourrait rajouter, dans le dico envoyé à l'API, un champ destiné à authentic2.api_views.BaseUserSerializer.create ?
Par exemple :

{   
    'email': 'pmar@eo.org',
    'last_name': '...',

    'get_or_create_by': 'email', <---- provoque un User.objects.get_or_create(email='pmar@eo.org', defaults=validated_data)

}

#11 Mis à jour par Benjamin Dauvergne il y a 4 mois

Je serai plus pour un ou des paramètres dans la query string, genre ?get-or-create-email=xyz@example.com&get-or-create-ou__slug=zob, juste qu'il va falloir parser tout ça et vérifier que c'est juste (que ou__slug ça existe, ou alors faire une whitelist dans un premier temps, username, email et ou__slug devraient suffire).

#12 Mis à jour par Paul Marillonnet il y a 3 mois

Benjamin Dauvergne a écrit :

Je serai plus pour un ou des paramètres dans la query string, genre ?get-or-create-email=xyz@example.com&get-or-create-ou__slug=zob, juste qu'il va falloir parser tout ça et vérifier que c'est juste (que ou__slug ça existe, ou alors faire une whitelist dans un premier temps, username, email et ou__slug devraient suffire).

Voilà ma compréhension du truc. Est-ce que c'est bien ça que tu voulais dire ?

Formats disponibles : Atom PDF