Développement #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)
Files
History
Updated by Benjamin Dauvergne about 4 years ago
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.
Updated by Frédéric Péters about 4 years ago
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 ?
Updated by Benjamin Dauvergne about 4 years ago
Oui oui si tu as ça sous la main sinon je le fais.
Updated by Frédéric Péters about 4 years ago
Non j'ai juste tapé le chunk ici sans tester ni rien.
Updated by Benjamin Dauvergne about 4 years ago
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
Updated by Benjamin Dauvergne about 4 years ago
- File 0001-api-check-members-presence-before-removing-them-5086.patch 0001-api-check-members-presence-before-removing-them-5086.patch added
- Status changed from Nouveau to Solution proposée
- Patch proposed changed from No to Yes
Updated by Benjamin Dauvergne about 4 years ago
- Status changed from Solution proposée to En cours
Je vais regarde ce qui est vraiment déclenché comme signaux.