Projet

Général

Profil

Bug #5524

crash SQL lors d'une action "ajouter un rôle à l'usager"

Ajouté par Thomas Noël il y a plus de 9 ans. Mis à jour il y a plus de 9 ans.

Statut:
Fermé
Priorité:
Normal
Assigné à:
Frédéric Péters
Version cible:
-
Début:
17 septembre 2014
Echéance:
% réalisé:

0%

Temps estimé:
Patch proposed:
Oui
Planning:

Description

LINE 1: ...Test CAM Admin', roles = ARRAY[18, 13, 9, 22, 10, E'Comptes ...
                                                             ^
)
To: admin+dev.montpellier@entrouvert.com

Exception:
  type = '<class 'psycopg2.DataError'>', value = 'invalid input syntax for integer: "Comptes relis  un dossier famille" 
LINE 1: ...Test CAM Admin', roles = ARRAY[18, 13, 9, 22, 10, E'Comptes ...
                                                             ^
'

Stack trace (most recent call first):
  File "/usr/lib/python2.6/dist-packages/wcs/sql.py", line 963, in store
   961                                        self._table_name,
   962                                        ', '.join(['%s = %%(%s)s' % (x,x) for x in column_names]))
>  963             cur.execute(sql_statement, sql_dict)
   964             if cur.fetchone() is None:
   965                 column_names = sql_dict.keys()

  locals:
     sql_statement = 'UPDATE users SET f0 = %(f0)s, f1 = %(f1)s, f2 = %(f2)s, f3 = %(f3)s, f4 = %(f4)s, f5 = %(f5)s, f6 = %(f6)s, f7 = %(f7)s, f8 =
%(f8)s, lasso_dump = %(lasso_dump)s, f0_display = %(f0_display)s, is_admin = %(is_admin)s, anonymous = %(anonymous)s, name_identifiers =
%(name_identifiers)s, identification_token = %(identification_token)s, name = %(name)s, roles = %(roles)s, email = %(email)s, last_seen =
%(last_seen)s WHERE id = %(id)s RETURNING id'
     cur = <cursor object at 0x355b148; closed: 0>
     sql_dict = {'f0': 'Monsieur', 'f1': 'IDP Test CAM', 'f2': 'Admin', 'f3': 'admin@entrouvert.com', 'f4': '0142350135', 'f5': '0643350135', 'f6':
'169 rue du chateau', 'f7': '75014', 'f8': 'PARIS', 'lasso_dump': '<Identity xmlns="http://www.entrouvert.org/namespaces/lasso/0.0" 
Version="2"><lasso:Federation xmlns:lasso="http://www.entrouvert.org/namespaces/lasso/0.0" xmlns:saml="urn:oasis:names:tc:SAML:2.0:assertion" 
RemoteProviderID="http://idp-montpellier.entrouvert.org/idp/saml2/metadata" FederationDumpVersion="2"><lasso:LocalNameIdentifier><saml:NameID
Format="urn:oasis:names:tc:SAML:2.0:nameid-format:persistent">_EB1870F05C82EBC5886CFF91B543D009</saml:NameID></lasso:LocalNameIdentifier></lasso:Fed
eration></Identity>', 'f0_display': None, 'is_admin': True, 'anonymous': False, 'name_identifiers': ['_EB1870F05C82EBC5886CFF91B543D009'], 'id': 19,
'identification_token': None, 'name': 'IDP Test CAM Admin', 'roles': [18, 13, 9, 22, 10, 'Comptes reli\xc3\xa9s \xc3\xa0 un dossi
 er famille'], 'email': 'admin@entrouvert.com', 'last_seen': (datetime.datetime(2014, 9, 17, 9, 14, 44),)}
     self = <SqlUser 'IDP Test CAM Admin' id:19>
     column_names = ['f0', 'f1', 'f2', 'f3', 'f4', 'f5', 'f6', 'f7', 'f8', 'lasso_dump', 'f0_display', 'is_admin', 'anonymous', 'name_identifiers',
'identification_token', 'name', 'roles', 'email', 'last_seen']
     x = 'last_seen'
     conn = <connection object at 0x383e7b0; dsn: 'dbname=montpellier user=wcs-au-quotidien', closed: 0>

Fichiers

creer-une-liaison-avec-un-compte-famille.wcs (4,9 ko) creer-une-liaison-avec-un-compte-famille.wcs Serghei Mihai, 17 septembre 2014 13:43
0001-workflows-improve-xml-import-export-of-roles-5524.patch (8,28 ko) 0001-workflows-improve-xml-import-export-of-roles-5524.patch Frédéric Péters, 17 septembre 2014 14:38
0001-workflows-improve-xml-import-export-of-roles-5524.patch (19,3 ko) 0001-workflows-improve-xml-import-export-of-roles-5524.patch Frédéric Péters, 19 septembre 2014 15:12
0001-workflows-improve-xml-import-export-of-roles-5524.patch (19,4 ko) 0001-workflows-improve-xml-import-export-of-roles-5524.patch Frédéric Péters, 20 septembre 2014 08:05

Historique

#1

Mis à jour par Serghei Mihai il y a plus de 9 ans

Quelques détails: le workflow a été importé depuis une autre instance de wcs et le rôle "Comptes liés à la famille" a été créé après l'import du workflow.
Donc comme l'id du rôle n'est pas trouvé, il semble que c'est son titre qui est envoyé dans la requete SQL.

#2

Mis à jour par Frédéric Péters il y a plus de 9 ans

Tu peux mettre en pièce jointe le fichier qui a été importé ?

#3

Mis à jour par Serghei Mihai il y a plus de 9 ans

Le voici.

#4

Mis à jour par Frédéric Péters il y a plus de 9 ans

Voilà du code qui améliore l'import/export XML des rôles : s'il y a un rôle qui matche sur le nom, on prend, sinon on prend le rôle qui matche sur l'id, et si celui-ci n'existe pas non plus, on crée à la volée un nouveau rôle.

#5

Mis à jour par Thomas Noël il y a plus de 9 ans

Frédéric Péters a écrit :

Voilà du code qui améliore l'import/export XML des rôles : s'il y a un rôle qui matche sur le nom, on prend, sinon on prend le rôle qui matche sur l'id, et si celui-ci n'existe pas non plus, on crée à la volée un nouveau rôle.

Ma proposition : si on trouve un rôle qui a le même nom, on le prend, sinon on créé un nouveau rôle. Oui, ça signifie ne plus utiliser l'id lors de l'import.

Ça me semble un fonctionnement plus compréhensible, le fonctionnement que tu proposes risque de déboucher sur «mais pourquoi sur ce site le rôle est bien importé alors que sur l'autre site ça prend le rôle "truc" existant ?».

#6

Mis à jour par Frédéric Péters il y a plus de 9 ans

Je garde en perspective l'utilisation de l'import/export xml pour un stockage lisible, à la place du pickle (#4739). Dans cette optique ça m'ennuierait d'avoir un rôle créé automatiquement au chargement d'un workflow, parce que le libellé du rôle initial aurait été modifié.

Bref, on est ennuyé par deux usages distincts de l'import/export xml: quand il se fait sur un même site vs quand il se fait d'un site à un autre.

#7

Mis à jour par Thomas Noël il y a plus de 9 ans

Pigé. Donc deux cas :
  1. import manuel via le web => rapprochement par le nom, sinon création (pas d'utilisation des id). Ca veut dire faire un import avec un "create_relations=True" (enfin c'est juste l'idée, le code exact est à voir).
  2. utilisation du XML par le logiciel => rapprochement par les id, sinon par le nom, sinon création.

Dernier cas : copier bêtement des fichiers XML dans le répertoire formdefs => risque de création de rôles s'ils ont changé entre temps (wcs pensera être dans le cas 2) Moi ça ne me dérange pas. A terme on pourrait imaginer un répertoire "import/" où on pourra placer les machins à importer, et faire tourner la logique du cas 1 pour ces objets là... (pour simplifier les copies d'objets entre sites).

#8

Mis à jour par Frédéric Péters il y a plus de 9 ans

En fait je me demande si simplement l'export par le web pourrait faire un include_ids=False (ce qui omettrait les attributs role_id).

#9

Mis à jour par Frédéric Péters il y a plus de 9 ans

Voilà un patch qui considère le include_id; et l'import fait via l'interface web ne les prend pas en compte (include_id=False).

#10

Mis à jour par Thomas Noël il y a plus de 9 ans

En première relecture (trop) rapide, je ne comprends pas l'ajout de "include_id=False" dans les arguments de init_with_xml (ligne 88 de wcs/wf/form.py) -- ou plutôt, je me dis que ça devrait peut-être impliquer un "include_id=include_id" dans l'appel à l'import du FormDef ligne 91 ? Ok j'avoue je suis un peu paumé sur l'import de formulaire dans l'import de workflows :-)

Sinon ça alourdit un peu le code, ce include_id qui traine partout, mais bon... pas d'autre idée :/

(Note : sauf si tu le sens bien comme ça, j'aimerais tagguer une 1.12.2 sans ce patch).

#11

Mis à jour par Frédéric Péters il y a plus de 9 ans

Thomas Noël a écrit :

En première relecture (trop) rapide, je ne comprends pas l'ajout de "include_id=False" dans les arguments de init_with_xml (ligne 88 de wcs/wf/form.py) -- ou plutôt, je me dis que ça devrait peut-être impliquer un "include_id=include_id" dans l'appel à l'import du FormDef ligne 91 ? Ok j'avoue je suis un peu paumé sur l'import de formulaire dans l'import de workflows :-)

Non, c'était explicite, c'est pour ça que j'ajoutais un commentaire, trop court et pas clair; dans le patch attaché j'allonge celui-ci, pour dire qu'on peut toujours avoir include_id=True à cet endroit parce que le formdef vit dans son propre espace isolé, il ne se trouve pas stocké à côté des autres formdefs, c'est la même situation que le formdef qui définit les champs utilisateur.

Sinon ça alourdit un peu le code, ce include_id qui traine partout, mais bon... pas d'autre idée :/

(Note : sauf si tu le sens bien comme ça, j'aimerais tagguer une 1.12.2 sans ce patch).

Dans la mesure où ça corrige un bug réellement rencontré, sur une fonctionnalité à la marge, avec des tests unitaires, je préférerais avoir la 1.12.2 avec ce patch.

#12

Mis à jour par Thomas Noël il y a plus de 9 ans

Frédéric Péters a écrit :

Dans la mesure où ça corrige un bug réellement rencontré, sur une fonctionnalité à la marge, avec des tests unitaires, je préférerais avoir la 1.12.2 avec ce patch.

Go

#13

Mis à jour par Frédéric Péters il y a plus de 9 ans

  • Statut changé de En cours à Résolu (à déployer)
commit 6063f68409ac7e18644704e32b7a5eb475d275d5
Author: Frédéric Péters <fpeters@entrouvert.com>
Date:   Wed Sep 17 14:36:08 2014 +0200

    workflows: improve xml import/export of roles (#5524)
#14

Mis à jour par Thomas Noël il y a plus de 9 ans

  • Statut changé de Résolu (à déployer) à Solution déployée
#15

Mis à jour par Thomas Noël il y a plus de 9 ans

  • Statut changé de Solution déployée à Fermé

Formats disponibles : Atom PDF