Crash de Qgis si je ferme la couche active et que je reclique sur "Edition" par exemple

WREATCHED commented 2 years ago

Si je supprime une couche du projet charger dans le gestionnaire de couche, et que je reclique sur le bouton "Edition" par exemple, Plume perd son identifiant de la couche et plante Qgis.

WREATCHED commented 2 years ago

Difficulté pour capter l'évènement de Qgis qui ferme une couche .....

WREATCHED commented 2 years ago

Résolu version 0.3.4 Bëta Pour résoudre complétement le problème chgt de gestion de sauvegarde Sauvegarde de l'identifiant de la couche est non plus de l'objet de type QgsVectorLayer Idem #58

alhyss commented 2 years ago

Je ferme cette issue et la #58, maintenant que tu as trouvé la solution 😃. Comme le problème apparaissait à cause des fonctionnalités implémentées pour la future v0.4 bêta, les utilisateurs n'y seront jamais confrontés.

alhyss commented 2 years ago

Je rouvre l'issue, puisque ce n'était finalement pas complètement réglé.

Salut @WREATCHED ! Comme tu me l'avais demandé, j'ai torturé Plume sur ces histoire de couches mémorisées susceptibles d'être perdues et autres cas similaires où Plume pourrait ne plus retrouver ses petits. J'étais sur un QGIS 3.22 (distribution de la communauté) pour m'épargner du temps de chargement par rapport aux 3.10 et 3.16, il faudra peut-être refaire la même chose avec d'autres versions avant de considérer le sujet comme réellement clos.

Une petite remarque avant que j'oublie. Je ne sais pas si c'est lié aux modifications que tu as faites ou si c'est l'effet 3.22, mais n'ai plus de problème avec le double-clic !

Pour pouvoir avancer dans les tests, je me suis permis quelques ajustements mineurs dans le code. Tu as peut-être déjà corrigé tout ça, mais je les liste ci-après histoire de tracer précisément le contexte dans lequel j'ai fait les tests.

J'ai ajouté QDockWidget et QTreeView à la liste des imports depuis PyQt5.QtWidgets dans bibli_plume.py.

from PyQt5.QtWidgets import (QAction, QMenu , QApplication, QMessageBox, QFileDialog, QTextEdit, QLineEdit, QMainWindow, QWidget, QDockWidget, QTreeView)

Petite coquille dans plume.bibli_plume.returnIfExisteInBrowser. Il manque un i dans self.navTreeView2.model(), qui devrait certainement être self.naviTreeView2.model().
```
    self.proxy_model = self.naviTreeView.model() \
        if self.mDic_LH["layerBeforeClickedBrowser"] == mNav1 \
        else self.naviTreeView2.model()
```
Au départ, j'avais un souci avec la pré-sélection depuis l'explorateur. La fiche était bien affichée à l'ouverture de Plume, mais si je tentais de passer en mode édition je me retrouvais avec l'avertissement de perte de couche.

J'ai résolu ça de manière assez draconienne, en partant du principe qu'il n'est finalement pas utile de vérifier que la couche est toujours indexée dans l'explorateur ou le panneau des couches. La seule chose qui importe est que le QgsVectorLayer stocké dans self.layerBeforeClicked[0] soit toujours valide. Donc j'ai notablement simplifié la partie try de plume.bibli_plume.gestionErreurExisteLegendeInterface, en mettant un test aussi peu coûteux que possible mais qui échouera à coup sûr si la couche a été supprimée :
```
    ret = True
    try: 
        self.layerBeforeClicked[0].id()
    except:
        ...
```
Dans plume.plume_ui.Ui_Dialog_plume.setupUi, j'ai remplacé
```
        if (self.layerBeforeClicked[0] != "" and self.layerBeforeClicked[0] != None) :
        try : 
            if self.layerBeforeClicked[0].type() != QgsMapLayer.VectorLayer : 
                self.layerBeforeClicked = ("", "")
        except : 
            self.layerBeforeClicked = ("", "")
```
par
```
        if self.layerBeforeClicked[0] is None:
        self.layerBeforeClicked = ('', '')
```
En pratique, ça devait fonctionner en l'état, mais je craignais que ce try / except ne rende certains cas problématiques plus difficiles à détecter. self.layerBeforeClicked[0] contient soit le résultat de plume.bibli_plume.returnIfExisteInLegendeInterface, soit celui de plume.bibli_plume.returnIfExisteInBrowser, et ces deux fonctions me semblent propres. Les deux renvoient un QgsVectorLayer si elles ont réussi à en construire un à partir des informations mémorisées, sinon None (auquel cas tu réinitialises ensuite self.layerBeforeClicked). Manipuler cet objet ne devrait jamais générer d'erreur, il n'y a pas non plus de raison d'avoir autre chose que None ou un QgsVectorLayer.
C'est plus de l'optimisation qu'autre chose (pour éviter de parcourir inutilement tout le dictionnaire), mais ta fonction plume.bibli_plume.returnIfExisteInLegendeInterface pourrait être simplifiée en :
```
def returnIfExisteInLegendeInterface(self, _IdLayerBeforeClick):
    layers = QgsProject.instance().mapLayers()
    return layers.get(_IdLayerBeforeClick)
```

Tests d'affichage multi-sources :

	Source	Test	Résultat
A1	Explorateur	Sélection d'une couche alors que Plume est ouvert, puis activation du mode édition.	Ok
A2	Explorateur 2	Sélection d'une couche alors que Plume est ouvert, puis activation du mode édition.	Ok
A3	Panneau des couches	Sélection d'une couche alors que Plume est ouvert, puis activation du mode édition.	Ok
A4	Explorateur	Sélection d'une couche avant l'ouverture de Plume, puis activation du mode édition.	Ok
A5	Explorateur 2	Sélection d'une couche avant l'ouverture de Plume, puis activation du mode édition.	Ok
A6	Panneau des couches	Sélection d'une couche avant l'ouverture de Plume, puis activation du mode édition.	Ok

Tests de suppression de couche :

	Source	Test	Résultat
B0	Panneau des couches	Suppression de la couche courante de Plume, puis activation du mode édition.	Ok : échec planifié, avertissement de l'utilisateur, réinitialisation.
B1	Panneau des couches	En mode lecture, suppression de la couche courante de Plume, puis changement de modèle.	Ok : échec planifié, avertissement de l'utilisateur, réinitialisation.
B2	Panneau des couches	En mode lecture, suppression de la couche courante de Plume, puis changement de langue.	Ok : échec planifié, avertissement de l'utilisateur, réinitialisation.
B3	Panneau des couches	En mode lecture, suppression de la couche courante de Plume, puis verrouillage.	Ok : échec planifié, avertissement de l'utilisateur, réinitialisation.
B4	Panneau des couches	En mode lecture, suppression de la couche courante de Plume, puis copie.	Ok : échec planifié, avertissement de l'utilisateur, réinitialisation.
B5	Panneau des couches	En mode lecture, suppression de la couche courante de Plume, puis export.	Ok : échec planifié, avertissement de l'utilisateur, réinitialisation.
B6	Panneau des couches	En mode lecture, suppression de la couche courante de Plume, puis ouverture de l'interface de configuration.	Ok : la boîte de dialogue s'ouvre.
B7	Panneau des couches	En mode lecture, suppression de la couche courante de Plume, puis ouverture de la documentation en ligne.	Ok : la documentation s'affiche.
B8	Panneau des couches	En mode lecture, suppression de la couche courante de Plume, puis ouverture de l'à propos'.	Ok : la boîte de dialogue s'ouvre.
B9	Panneau des couches	Activation du mode édition, suppression de la couche courante de Plume, puis désactivation du mode édition.	Après le retour à l'écran d'accueil, le mode édition reste actif, et surtout le verrouillage aussi, ce qui empêche de sélectionner une autre couche...
B10	Panneau des couches	Activation du mode édition, suppression de la couche courante de Plume, puis réinitialisation.	Idem test B9.
B11	Panneau des couches	Activation du mode édition, suppression de la couche courante de Plume, puis coller.	Idem test B9.
B12	Panneau des couches	Activation du mode édition, suppression de la couche courante de Plume, puis import depuis un fichier.	Idem test B9.
B13	Panneau des couches	Activation du mode édition, suppression de la couche courante de Plume, puis activation du mode traduction.	Idem test B9.
B14	Panneau des couches	Activation du mode édition, suppression de la couche courante de Plume, puis enregistrement.	Idem test B9.
B15	Panneau des couches	En mode lecture, verrouillage, puis suppression de la couche courante de Plume, puis déverrouillage.	Similaire aux tests B9 à B14. Le verrouillage reste actif, ce qui empêche de sélectionner une autre couche.
B16	Panneau des couches	En mode lecture, verrouillage, puis suppression de la couche courante de Plume, puis changement de modèle.	Idem test B15. Je ne fais pas d'autres tests de suppression avec verrouillage, partant du principe que le résultat sera le même quel que soit le bouton sur lequel je clique.
B17	Panneau des couches	Activation du mode édition et du mode traduction, suppression de la couche courante de Plume, puis manipulations dans le formulaire : bouton plus, bouton moins, changement de source, changement d'unité, changement de langue, bouton de calcul.	Ok : tout fonctionne normalement.
B18	Panneau des couches	Activation du mode édition, suppression de la couche courante de Plume, clic sur un bouton de visualisation de géométrie.	Idem B9 puisque le mode édition est actif.
B19	Panneau des couches	Activation du mode édition, suppression de la couche courante de Plume, utilisation du bouton d'aide à la saisie pour lancer un calcul côté PostGIS.	Idem B9 puisque le mode édition est actif. Je pars du principe que toutes les actions du menu auront le même effet et ne fais pas de test pour chacune.

En bref, il y aurait juste un petit ajustement à faire dans ta fonction de réinitialisation de l'interface qu'elle veille bien à revenir quoi qu'il arrive en mode lecture sans verrouillage, pour le reste ça m'a l'air en ordre.

Manipulations diverses des explorateurs :

	Source	Test	Résultat
C1	Explorateur	Verrouillage, suppression via l'explorateur de la connexion dont est issue la couche courante, bascule en mode édition, modification de la description, enregistrement.	Ok : pas d'erreur, comportement normal de Plume.
C2	Explorateur 2	Activation du mode édition, suppression via le Gestionnaire des sources de données de la connexion dont est issue la couche courante, bascule en mode édition, calcul du centroïde côté QGIS, enregistrement, désactivation du mode édition.	Ok : pas d'erreur, comportement normal de Plume.
C3	Explorateur 2	Activation du mode édition, fermeture de l'explorateur 2, réinitialisation de la fiche, enregistrement.	Ok : pas d'erreur, comportement normal de Plume.

Ces tests me semblent confirmer qu'à partir du moment où tu travailles sur un QgsVectorLayer que tu as généré toi même, ce qui est donc le cas lorsque la source est l'un des explorateurs, il n'y a pas à se préoccuper de ce qui se passe dans l'explorateur, car ça n'affecte pas le QgsVectorLayer. C'est ce qui rend légitime la simplification drastique de plume.bibli_plume.gestionErreurExisteLegendeInterface que j'évoquais précédemment.

Suppressions côté PostgreSQL :

	Source	Test	Résultat
D1	Panneau des couches	Affichage de la fiche en mode lecture, suppression de la table côté PostgreSQL, bascule en mode édition.	Affichage d'une boîte de dialogue avec le message "PLUME a rencontré une erreur" sans aucune précision. Après clic sur le bouton Ok, erreur python dont je mets le détail ci-dessous.
D2	Explorateur	Affichage de la fiche en mode édition, suppression de la table côté PostgreSQL, modification du titre, enregistrement.	Affichage d'une boîte de dialogue avec le message "PLUME a rencontré une erreur" et une description de l'erreur (incomplète et pas très bien mise en forme - cf. ci-dessous). Après clic sur le bouton Ok, erreur python différente de la précédente et dont je mets le détail ci-dessous.
D3	Panneau des couches	Affichage de la fiche en mode lecture sans modèle, suppression d'un modèle, bascule sur le modèle supprimé, modification du descriptif, enregistrement.	Ok. Pas d'erreur. Comme le modèle n'existe plus, Plume ne trouve aucune catégorie associée dans `meta_template_categories_full`, donc toutes les métadonnées se retrouvent dans l'onglet Autres. C'est acceptable pour moi.
D4	Explorateur	Affichage en mode édition, suppression de PostGIS, exécution manuelle du calcul des référentiels (nécessite PostGIS).	Erreur python ci-dessous au moment de la génération du message d'erreur.
D5	Explorateur	Affichage en mode édition avec un modèle prévoyant du calcul automatique, suppression de PlumePg, réinitialisation de la fiche.	Impossible de supprimer PlumePg tant que Plume est actif (et même tant que la session QGIS reste ouverte, après la fermeture de Plume).
D6	Explorateur	Suppression d'une table côté PostgreSQL, activation de Plume, clic sur une couche issue de ladite table dans l'explorateur non actualisé.	Affichage d'une boîte de dialogue avec le message "PLUME a rencontré une erreur" et une description de l'erreur PostgreSQL (la relation n'existe pas). Après validation, affichage d'une seconde boîte de dialogue "PLUME a rencontré une erreur" avec le détail "la transaction est annulée, les commandes sont ignorées jusqu'à la fin du blocde la transaction". Cette boîte de dialogue apparaît de nouveau après validation, puis on arrive sur une erreur python (détail ci-dessous). Ensuite on repart au début de la boucle, avec la boîte de dialogue qui donne l'erreur PostgreSQL, et ainsi de suite... J'ai fini par en sortir, mais je ne sais pas trop comment.
D7	Panneau des couches	Suppression d'une table côté PostgreSQL, activation de Plume, clic sur une couche issue de ladite table dans le panneau de couches.	Idem D6. Sans trop de suprise, c'est indépendant de la source, puisque le problème apparaît au moment de la tentative d'import du descriptif PostgreSQL.

Erreur python rencontrée sur le test D1 :

TypeError: setEnabled(self, bool): argument 1 has unexpected type 'NoneType' 
Traceback (most recent call last):
  File "C:\Users/.../AppData/Roaming/QGIS/QGIS3\profiles\default/python/plugins\plume\plume_ui.py", line 327, in clickButtonsActions
    self.displayToolBar(*self.listIconToolBar)
  File "C:\Users/.../AppData/Roaming/QGIS/QGIS3\profiles\default/python/plugins\plume\plume_ui.py", line 884, in displayToolBar
    self.plumeEdit.setEnabled(r)
TypeError: setEnabled(self, bool): argument 1 has unexpected type 'NoneType'

Description de l'erreur PostgreSQL obtenue avec le test D2 :

la relation « e_cadastre_etalab.parcelles_bis » n'existe pasLINE 3:             WHERE pg_class.oid = '"e_cadastre_etalab"."parce...                                         ^

Le retour à la ligne entre "pas" et "LINE" a sauté. Pour autant que je puisse voir il était bien présent dans la valeur de l'attribut psycopg2.Error.pgcode, il doit y avoir moyen de corriger ça ?

Erreur python rencontrée sur le test D2 :

TypeError: 'NoneType' object is not subscriptable 
Traceback (most recent call last):
  File "C:\Users/.../AppData/Roaming/QGIS/QGIS3\profiles\default/python/plugins\plume\plume_ui.py", line 276, in clickButtonsActions
    bibli_plume.saveMetaIhm(self, self.schema, self.table)
  File "C:\Users/.../AppData/Roaming/QGIS/QGIS3\profiles\default/python/plugins\plume\bibli_plume.py", line 463, in saveMetaIhm
    mKeySql = (queries.query_update_table_comment(_schema, _table, relation_kind=kind[0]), (new_pg_description,))
TypeError: 'NoneType' object is not subscriptable

Erreur python rencontrée sur le test D4 :

TypeError: bad operand type for unary +: 'str' 
Traceback (most recent call last):
  File "C:\Users/.../AppData/Roaming/QGIS/QGIS3\profiles\default/python/plugins\plume\bibli_gene_objets.py", line 416, in 
    _mObjetQToolButton.clicked.connect(lambda : action_mObjetQToolButton_ComputeButton(self, _keyObjet, _valueObjet))
  File "C:\Users/.../AppData/Roaming/QGIS/QGIS3\profiles\default/python/plugins\plume\bibli_gene_objets.py", line 687, in action_mObjetQToolButton_ComputeButton
    zMess  = QtWidgets.QApplication.translate("plume_ui", "Au minimum, absence de l'extension : ", None) +  + _messDependances
TypeError: bad operand type for unary +: 'str'

Description de l'erreur PostgreSQL obtenue avec le test D6 :

la relation « e_cadastre_etalab.parcelles_bis » n'existe pasLINE 1: SELECT obj_description('"e_cadastre_etalab"."parcelles_bis"'...                               ^

Erreur python rencontrée sur le test D6 :

TypeError: 'NoneType' object is not subscriptable 
Traceback (most recent call last):
  File "C:\Users/.../AppData/Roaming/QGIS/QGIS3\profiles\default/python/plugins\plume\plume_ui.py", line 627, in retrieveInfoLayerBrowser
    self.displayToolBar(*self.listIconToolBar)
  File "C:\Users/.../AppData/Roaming/QGIS/QGIS3\profiles\default/python/plugins\plume\plume_ui.py", line 834, in displayToolBar
    result = r[0]
TypeError: 'NoneType' object is not subscriptable

Je crois que je viens de remettre le nez dans un sujet sur lequel tu ne t'étais pas repenché depuis le tout début...

À mon avis, le comportement en cas d'erreur PostgreSQL devrait être similaire au cas des couches disparues :

afficher une boîte de dialogue indiquant que PostgreSQL a renvoyé une erreur, avec le détail de celle-ci ;
réinitialiser l'interface.

Il y a sans doute un peu de boulot de ton côté pour le second point, même si j'espère que ce que tu as fait pour les couches disparues sera mutualisable. À mettre peut-être dans une issue spécifique, si tu penses que c'est un sujet à part entière ?

Par ailleurs, je vais probablement créer une issue pour l'apparence des boîtes de dialogue d'erreurs (au sens large). J'aimerais les harmoniser, tout en permettant à l'utilisateur de comprendre facilement si le problème est côté PostgreSQL ou côté QGIS (pour les couches disparues, je ne crois pas que nous ayons d'autre cas à date ?)... Je regarderai aussi la question de la mise en forme des messages d'erreur de PostgreSQL.

Le test D5 méritera aussi une issue. Ce n'est peut-être pas le plus gros problème qui soit (même si c'est casse pieds pour nos tests), mais ça ressemble à un curseur ouvert ou quelque chose du genre, ce qui ne serait pas très sain...

Cas tordus susceptibles de poser problème au mécanisme de pré-sélection :

	Source	Test	Résultat
E1	Panneau des couches	Affichage dans Plume des métadonnées d'une couche d'un projet, fermeture du projet, fermeture de Plume, réouverture du projet, lancement de Plume	Ok. C'est la première couche du panneau dont les métadonnées sont affichées (qui n'est pas celle qui était antérieurement sélectionnée et ce n'est pas dérangeant).
E2	Panneau des couches	Affichage dans Plume des métadonnées d'une couche d'un projet, fermeture du projet, ouverture d'un autre projet, lancement de Plume	Ok. Cette fois j'arrive sur l'écran d'accueil et pas sur la fiche de la première couche. J'ai reproduit cette différence de comportement entre E1 et E2 avec plusieurs projets différents. C'est curieux, mais sans aucune espèce de gravité, donc oublions.
E3	Panneau des couches	Affichage dans Plume des métadonnées d'une couche d'un projet, fermeture de QGIS, lancement de QGIS sur le même projet, lancement de Plume	Ok. Idem E1.
E4	Panneau des couches	Affichage dans Plume des métadonnées d'une couche d'un projet, fermeture de QGIS, lancement de QGIS sur un projet différent, lancement de Plume	Ok. Idem E2.
E5	Panneau des couches	Affichage dans Plume des métadonnées d'une couche d'un projet, fermeture de QGIS, lancement de QGIS sur un projet vierge, lancement de Plume	Ok. Plume s'ouvre sur l'écran d'accueil.
E6	Explorateur	Affichage dans Plume de métadonnées depuis l'explorateur, fermeture de QGIS, lancement de QGIS sur un projet vierge, lancement de Plume	Ok. Plume s'ouvre sur l'écran d'accueil.
E7		Suppression des paramètres `layerBeforeClicked` et `layerBeforeClickedWho` et `layerBeforeClickedBrowser` dans le fichier `QGIS3.ini` (simule la toute première exécution de Plume après l'installation), lancement de QGIS, lancement de Plume, chargement d'une fiche depuis l'explorateur.	Erreur python détaillée ci-dessous.
E8		Suppression de tous les paramètres dans le fichier `QGIS3.ini` (simule la toute première exécution de Plume après l'installation), lancement de QGIS, lancement de Plume, chargement d'une fiche depuis l'explorateur. NB: Test réalisé après correction de l'erreur rencontrée au test E7 (cf. ci-après), qui serait nécessairement apparue ici aussi.	Erreur python détaillée ci-dessous.

Erreur python rencontrée sur le test E7 :

AttributeError: 'NoneType' object has no attribute 'mapToSource' 
Traceback (most recent call last):
  File "C:\Users/.../AppData/Roaming/QGIS/QGIS3\profiles\default/python/plugins\plume\plume.py", line 133, in clickIHMplume2
    d = doplume_ui.Dialog()
  File "C:\Users/.../AppData/Roaming/QGIS/QGIS3\profiles\default/python/plugins\plume\doplume_ui.py", line 17, in __init__
    self.setupUi(self)
  File "C:\Users/.../AppData/Roaming/QGIS/QGIS3\profiles\default/python/plugins\plume\plume_ui.py", line 105, in setupUi
    self.layerBeforeClicked = (bibli_plume.returnIfExisteInBrowser(self, self.mDic_LH["layerBeforeClicked"]), self.mDic_LH["layerBeforeClickedWho"], self.mDic_LH["layerBeforeClickedBrowser"]) # Couche mémorisée avant ouverture et de l'origine
  File "C:\Users/.../AppData/Roaming/QGIS/QGIS3\profiles\default/python/plugins\plume\bibli_plume.py", line 187, in returnIfExisteInBrowser
    item = self.model.dataItem(self.proxy_model.mapToSource(index))
AttributeError: 'NoneType' object has no attribute 'mapToSource'

Celle-là, je l'ai corrigée pour pouvoir faire le test E8, en faisant en sorte que plume.bibli_plume.returnIfExisteInBrowser ne soit appelée que quand layerBeforeClickedWho est 'postgres', pas quand il est vide. NB: À la différence des modifications que j'ai listées en intro, j'ai changé ça au cours des tests, pas avant.

J'ai remplacé ça dans plume.plume_ui.Ui_Dialog_plume.setupUi

        #Management Click before open IHM 
        if self.mDic_LH["layerBeforeClickedWho"] == "qgis": 
           self.layerBeforeClicked = ...
        else :    
           self.layerBeforeClicked = ...

par ça

        #Management Click before open IHM 
        if self.mDic_LH["layerBeforeClickedWho"] == "qgis": 
           self.layerBeforeClicked = ...
        elif self.mDic_LH["layerBeforeClickedWho"] == "postgres":
           self.layerBeforeClicked = ... # idem ancien else
        else:
            self.layerBeforeClicked = ('', '')

Erreur python rencontrée sur le test E8 :

NameError: name 'QtWidgets' is not defined 
Traceback (most recent call last):
  File "C:\Users/leslie.lemaire/AppData/Roaming/QGIS/QGIS3\profiles\default/python/plugins\plume\plume.py", line 133, in clickIHMplume2
    d = doplume_ui.Dialog()
  File "C:\Users/leslie.lemaire/AppData/Roaming/QGIS/QGIS3\profiles\default/python/plugins\plume\doplume_ui.py", line 17, in __init__
    self.setupUi(self)
  File "C:\Users/leslie.lemaire/AppData/Roaming/QGIS/QGIS3\profiles\default/python/plugins\plume\plume_ui.py", line 97, in setupUi
    manageLibrary(bibli_plume.returnVersion(), self.versionPlumeBibli)
  File "C:\Users/leslie.lemaire/AppData/Roaming/QGIS/QGIS3\profiles\default/python/plugins\plume\bibli_install.py", line 12, in manageLibrary
    logging.debug(QtWidgets.QApplication.translate("bibli_install", "Installing Libraries ..."))
NameError: name 'QtWidgets' is not defined

J'ai ajouté l'import manquant dans bibli_install.py, ce qui a résolu le problème.

from PyQt5 import QtWidgets

alhyss commented 2 years ago

J'ai repensé à la meilleure manière de gérer les erreurs PostgreSQL et finalement je me demande s'il ne faut pas faire tout l'inverse de ce que j'évoquais : plutôt que de capturer les erreurs en amont et devoir ensuite gérer les effets de bord au cas par cas (fonctions qui ne renvoient pas le résultat attendu, etc.), laisser les erreurs se propager au maximum, jusqu'à la première méthode exécutée lorsque l'utilisateur fait une action. Je pense à toutes les méthodes plume.plume_ui.Ui_Dialog_plume.click... (plume.plume_ui.Ui_Dialog_plume.clickButtonsActions, plume.plume_ui.Ui_Dialog_plume.clickButtonsTemplateActions, etc., à l'exception de celles qui sont complètement indépendantes du serveur PostgreSQL), et il en faut aussi à l'initialisation, certainement au moins plume.plume_ui.Ui_Dialog_plume.retrieveInfoLayerQgis et plume.plume_ui.Ui_Dialog_plume.retrieveInfoLayerBrowser[^1] ?

[^1]: Certainement un peu plus que ça, parce que plume.plume_ui.Ui_Dialog_plume.setupUi appelle d'autres méthodes qui envoient des requêtes au serveur, genre plume.plume_ui.Ui_Dialog_plume.displayToolBar. Mais j'ai fait en sorte dans ce qui suit qu'il ne soit pas trop gênant de mettre le gestionnaire de contexte sur plusieurs méthodes qui s'exécutent successivement s'il n'est vraiment pas possible de l'éviter, donc on pourrait imaginer le mettre aussi sur plume.plume_ui.Ui_Dialog_plume.displayToolBar, par exemple.

Mon idée serait d'utiliser un gestionnaire contexte / context manager qui pour gérer à la fois la connexion PostgreSQL (en s'assurant de toujours la fermer quoi qu'il arrive) et les erreurs renvoyées par le serveur.

En super simplifié (cf. commentaires dans le code), il ressemblerait à ça :

from contextlib import contextmanager
import psycopg2

class Ui_Dialog_plume:
    ...
    @contextmanager
    def safe_pg_connection(self):
        if not getattr(self, 'mConnectEnCours', False) or self.mConnectEnCours.closed:
            self.mConnectEnCours = psycopg2.connect(self.connection_string, application_name='Plume')
            # création de la connexion au serveur PostgreSQL si elle n'est pas déjà définie
            # en plus compliqué bien sûr, il faudrait grosso modo reprendre
            # ici le contenu de plume.plume_ui.Ui_Dialog_plume.mTestConnect
            # avec les noms d'attributs qui vont bien à la place de ce  
            # qui correspondrait à self.uri.connectionInfo() 
        try:
            yield
        except psycopg2.Error as err:
            if err.diag:
                print(err.diag.message_primary)
            # ici on affiche le message à l'utilisateur
            # puis réinitialisation de l'interface
        finally:
            self.mConnectEnCours.close()

Ensuite, il peut s'utiliser comme ça :

>>> ui = Ui_Dialog_plume()
>>> ui.connection_string = 'host=localhost port=5432 dbname=metadata_dev user=postgres password=xxxxxx'
>>> with ui.safe_pg_connection():
...     with ui.mConnectEnCours.cursor() as cur:
...         cur.execute('SELECT bla FROM bla')
la relation « bla » n'existe pas
>>> ui.mConnectEnCours.closed
1

Ou dans une fonction :

def stupid_request(ui, my_request):
    with ui.safe_pg_connection():
        with ui.mConnectEnCours.cursor() as cur:
            cur.execute(my_request)
        print('et tant pis')

Quand on l'exécute :

>>> ui = Ui_Dialog_plume()
>>> ui.connection_string = 'host=localhost port=5432 dbname=metadata_dev user=postgres password=xxxxxx'
>>> stupid_request(ui, 'SELECT ceci FROM cela')
la relation « cela » n'existe pas

En imaginant que tout ceci soit une bonne idée, ça impliquerait les changements suivants dans ton code :

Créer le gestionnaire de contexte en tant que méthode de la classe plume.plume_ui.Ui_Dialog_plume, en réécrivant les parties que j'ai seulement esquissées.
Mettre le code de chacune des méthodes susmentionnées derrière un with self.safe_pg_connection():.
Supprimer toutes les créations, fermetures et tests de connexion PostgreSQL qui se faisaient ailleurs.
Supprimer le contrôle d'erreur dans plume.bibli_plume.executeSql, puisqu'il relèverait du gestionnaire de contexte. Un try / finally serait adapté ici, en fait, pour fermer le curseur même en cas d'erreur :
```
def executeSql(pointeur, _mKeySql, optionRetour=None):
    pointeurBase = pointeur.cursor()
    try:
        ... # pas de changement
    finally:
        pointeurBase.close()
    return result
```
A priori, la fonction n'aurait plus lieu de retourner d'informations sur les erreurs vu qu'elle n'en disposerait plus.

Bref, on en reparle.

alhyss commented 2 years ago

Hmm. Oublie ce que j'ai écrit sur la possibilité d'utiliser le gestionnaire de contexte dans des fonctions imbriquées. C'est un coup à perdre sa connexion, il ne faut surtout pas le faire. Le gestionnaire doit uniquement être appelé dans les méthodes chapeau, vraisemblablement les méthodes click... et setupUi ?

WREATCHED commented 2 years ago

La nouvelle gestion des connexions et des erreurs avec le Context Manager a permis de résoudre le problème de la suppression des objets coté PostgreSQL

DL : Faire les tests décrits dans l'issue pour validation définitive

WREATCHED commented 2 years ago

@ DL : Suivi de la recette des tests actuellement KO (si coché alors OK)

Tests d'affichage multi-sources :

	Source	Test	Résultat
- [x]	B9	Panneau des couches	Activation du mode édition, suppression de la couche courante de Plume, puis désactivation du mode édition.	Après le retour à l'écran d'accueil, le mode édition reste actif, et surtout le verrouillage aussi, ce qui empêche de sélectionner une autre couche...
- [x]	B10	Panneau des couches	Activation du mode édition, suppression de la couche courante de Plume, puis réinitialisation.	Idem test B9.
- [x]	B11	Panneau des couches	Activation du mode édition, suppression de la couche courante de Plume, puis coller.	Idem test B9.
- [x]	B12	Panneau des couches	Activation du mode édition, suppression de la couche courante de Plume, puis import depuis un fichier.	Idem test B9.
- [x]	B13	Panneau des couches	Activation du mode édition, suppression de la couche courante de Plume, puis activation du mode traduction.	Idem test B9.
- [x]	B14	Panneau des couches	Activation du mode édition, suppression de la couche courante de Plume, puis enregistrement.	Idem test B9.
- [x]	B15	Panneau des couches	En mode lecture, verrouillage, puis suppression de la couche courante de Plume, puis déverrouillage.	Similaire aux tests B9 à B14. Le verrouillage reste actif, ce qui empêche de sélectionner une autre couche.
- [x]	B16	Panneau des couches	En mode lecture, verrouillage, puis suppression de la couche courante de Plume, puis changement de modèle.	Idem test B15. Je ne fais pas d'autres tests de suppression avec verrouillage, partant du principe que le résultat sera le même quel que soit le bouton sur lequel je clique.
- [x]	B17	Panneau des couches	Activation du mode édition et du mode traduction, suppression de la couche courante de Plume, puis manipulations dans le formulaire : bouton plus, bouton moins, changement de source, changement d'unité, changement de langue, bouton de calcul.	Ok : tout fonctionne normalement.
- [x]	B18	Panneau des couches	Activation du mode édition, suppression de la couche courante de Plume, clic sur un bouton de visualisation de géométrie.	Idem B9 puisque le mode édition est actif.
- [x]	B19	Panneau des couches	Activation du mode édition, suppression de la couche courante de Plume, utilisation du bouton d'aide à la saisie pour lancer un calcul côté PostGIS.	Idem B9 puisque le mode édition est actif. Je pars du principe que toutes les actions du menu auront le même effet et ne fais pas de test pour chacune.

Manipulations diverses des explorateurs :

Suppressions côté PostgreSQL :

	Source	Test	Résultat
- [x]	D1	Panneau des couches	Affichage de la fiche en mode lecture, suppression de la table côté PostgreSQL, bascule en mode édition.	Affichage d'une boîte de dialogue avec le message "PLUME a rencontré une erreur" sans aucune précision. Après clic sur le bouton Ok, erreur python dont je mets le détail ci-dessous.
- [x]	D2	Explorateur	Affichage de la fiche en mode édition, suppression de la table côté PostgreSQL, modification du titre, enregistrement.	Affichage d'une boîte de dialogue avec le message "PLUME a rencontré une erreur" et une description de l'erreur (incomplète et pas très bien mise en forme - cf. ci-dessous). Après clic sur le bouton Ok, erreur python différente de la précédente et dont je mets le détail ci-dessous.
- [x]	D4	Explorateur	Affichage en mode édition, suppression de PostGIS, exécution manuelle du calcul des référentiels (nécessite PostGIS).	Erreur python ci-dessous au moment de la génération du message d'erreur.
- [x]	D5	Explorateur	Affichage en mode édition avec un modèle prévoyant du calcul automatique, suppression de PlumePg, réinitialisation de la fiche.	Impossible de supprimer PlumePg tant que Plume est actif (et même tant que la session QGIS reste ouverte, après la fermeture de Plume).
- [x]	D6	Explorateur	Suppression d'une table côté PostgreSQL, activation de Plume, clic sur une couche issue de ladite table dans l'explorateur non actualisé.	Affichage d'une boîte de dialogue avec le message "PLUME a rencontré une erreur" et une description de l'erreur PostgreSQL (la relation n'existe pas). Après validation, affichage d'une seconde boîte de dialogue "PLUME a rencontré une erreur" avec le détail "la transaction est annulée, les commandes sont ignorées jusqu'à la fin du blocde la transaction". Cette boîte de dialogue apparaît de nouveau après validation, puis on arrive sur une erreur python (détail ci-dessous). Ensuite on repart au début de la boucle, avec la boîte de dialogue qui donne l'erreur PostgreSQL, et ainsi de suite... J'ai fini par en sortir, mais je ne sais pas trop comment.
- [x]	D7	Panneau des couches	Suppression d'une table côté PostgreSQL, activation de Plume, clic sur une couche issue de ladite table dans le panneau de couches.	Idem D6. Sans trop de suprise, c'est indépendant de la source, puisque le problème apparaît au moment de la tentative d'import du descriptif PostgreSQL.

Cas tordus susceptibles de poser problème au mécanisme de pré-sélection :

	Source	Test	Résultat
- [x]	E1	Panneau des couches	Affichage dans Plume des métadonnées d'une couche d'un projet, fermeture du projet, fermeture de Plume, réouverture du projet, lancement de Plume	Ok. C'est la première couche du panneau dont les métadonnées sont affichées (qui n'est pas celle qui était antérieurement sélectionnée et ce n'est pas dérangeant).
- [x]	E2	Panneau des couches	Affichage dans Plume des métadonnées d'une couche d'un projet, fermeture du projet, ouverture d'un autre projet, lancement de Plume	Ok. Cette fois j'arrive sur l'écran d'accueil et pas sur la fiche de la première couche. J'ai reproduit cette différence de comportement entre E1 et E2 avec plusieurs projets différents. C'est curieux, mais sans aucune espèce de gravité, donc oublions.
- [x]	E3	Panneau des couches	Affichage dans Plume des métadonnées d'une couche d'un projet, fermeture de QGIS, lancement de QGIS sur le même projet, lancement de Plume	Ok. Idem E1.
- [x]	E4	Panneau des couches	Affichage dans Plume des métadonnées d'une couche d'un projet, fermeture de QGIS, lancement de QGIS sur un projet différent, lancement de Plume	Ok. Idem E2.
- [x]	E5	Panneau des couches	Affichage dans Plume des métadonnées d'une couche d'un projet, fermeture de QGIS, lancement de QGIS sur un projet vierge, lancement de Plume	Ok. Plume s'ouvre sur l'écran d'accueil.
- [x]	E6	Explorateur	Affichage dans Plume de métadonnées depuis l'explorateur, fermeture de QGIS, lancement de QGIS sur un projet vierge, lancement de Plume	Ok. Plume s'ouvre sur l'écran d'accueil.
- [x]	E7		Suppression des paramètres `layerBeforeClicked` et `layerBeforeClickedWho` et `layerBeforeClickedBrowser` dans le fichier `QGIS3.ini` (simule la toute première exécution de Plume après l'installation), lancement de QGIS, lancement de Plume, chargement d'une fiche depuis l'explorateur.	Erreur python détaillée ci-dessous.
- [x]	E8		Suppression de tous les paramètres dans le fichier `QGIS3.ini` (simule la toute première exécution de Plume après l'installation), lancement de QGIS, lancement de Plume, chargement d'une fiche depuis l'explorateur. NB: Test réalisé après correction de l'erreur rencontrée au test E7 (cf. ci-après), qui serait nécessairement apparue ici aussi.	Erreur python détaillée ci-dessous.

WREATCHED commented 2 years ago

Tests terminés, Corrections apportées pour les passer

Maintenant pas à l'abri d'une surprise, mais c'est déjà des tests parfois assez "tordus", donc je pense satisfaisant pour une éventuelle diffusion après la partie des requêtes.

MTES-MCT / metadata-postgresql

Crash de Qgis si je ferme la couche active et que je reclique sur "Edition" par exemple #59