Development #50861
api memberships roles, ne pas écrire si pas de changement
0%
Description
Parce que ces API sont appelées en masse, et que le .add/.remove entraine visiblement .save() qui entraine provisionning, peut-être pourrait-on imaginer quelque chose de ce type :
@@ -916,12 +916,14 @@ class RoleMembershipAPI(ExceptionHandlerMixin, APIView):
raise PermissionDenied(u'User not allowed to change role')
def post(self, request, *args, **kwargs):
- self.role.members.add(self.member)
+ if self.member not in self.role.members:
+ self.role.members.add(self.member)
return Response({'result': 1, 'detail': _('User successfully added to role')},
status=status.HTTP_201_CREATED)
def delete(self, request, *args, **kwargs):
- self.role.members.remove(self.member)
+ if self.member in self.role.members:
+ self.role.members.remove(self.member)
return Response({'result': 1, 'detail': _('User successfully removed from role')},
status=status.HTTP_200_OK)
(pas testé, et peut-être qu'il serait souhaité quelque chose de plus compliqué dans hobo)
Fichiers
Historique
Mis à jour par Benjamin Dauvergne il y a environ 3 ans
Frédéric Péters a écrit :
Parce que ces API sont appelées en masse, et que le .add/.remove entraine visiblement .save() qui entraine provisionning, peut-être pourrait-on imaginer quelque chose de ce type :
[...]
Oui. Ça n'entraîne pas de save() sur User mais sur Role.members.throug. Ces changements d'appartenance à des rôles entraînent effectivement un provisionning, pour que les applications soient à jour.
(pas testé, et peut-être qu'il serait souhaité quelque chose de plus compliqué dans hobo)
Je ne pense pas qu'on puisse faire beaucoup mieux juste dans hobo; on a besoin de ce provisionning. Une seule amélioration possible ce serait avec une API batch pour créer/supprimer plusieurs liaisons rôles/utilisateurs d'un coup, ça ne produira rien si on parle en face de l'action ajouter un rôle. On pourrait aussi créer des messages de provisionning spécifiques moins coûteux à construire mais je ne sais pas si il y aura vraiment un gain.
Mis à jour par Frédéric Péters il y a environ 3 ans
Oui.
Juste pour être sûr : oui un patch qui reprendrait le chunk indiqué (après l'avoir testé etc.) peut être attaché à ce ticket ?
Mis à jour par Benjamin Dauvergne il y a environ 3 ans
Oui oui si tu as ça sous la main sinon je le fais.
Mis à jour par Frédéric Péters il y a environ 3 ans
Non j'ai juste tapé le chunk ici sans tester ni rien.
Mis à jour par Benjamin Dauvergne il y a environ 3 ans
Bon finalement après contrôle du code de Django, Role.members.add ne produit pas de provisionning si l'action est inutile, car il contrôle les membres déjà présent pour éviter de faire des INSERT inutile (m2m_changed est quand même émis mais pk_set est vide)[1]. Par contre dans Role.members.delete ce contrôle n'est pas fait, le verbe SQL DELETE étant idempotent.
1 https://git.entrouvert.org/hobo.git/tree/hobo/agent/authentic2/provisionning.py#n390
Mis à jour par Benjamin Dauvergne il y a environ 3 ans
- Fichier 0001-api-check-members-presence-before-removing-them-5086.patch 0001-api-check-members-presence-before-removing-them-5086.patch ajouté
- Statut changé de Nouveau à Solution proposée
- Patch proposed changé de Non à Oui
Mis à jour par Benjamin Dauvergne il y a environ 3 ans
- Statut changé de Solution proposée à En cours
Je vais regarde ce qui est vraiment déclenché comme signaux.