getRowId() retourne ''9999999999' au moment d'appeler le template editor

Tags: #<Tag:0x00007f80f76db4e0>

Bonjour,

Nous rencontrons un cas étrange :

Un de nos BO implémente la méthode isActionEnable et utilise le retour de getRowId() dans un SQL.
Ce code est appelé par Simplicité au moment d’afficher le template editor et à ce moment la valeur retournée par getRowId() est 9999999999, ce qui fait partir le SQL en erreur car la valeur est trop grande pour une int.

Le row-id maximal en base de donnée a ce moment là est 2092.

Cette valeur est la “dummy value” pour le type ID. Effectivement là elle n’est pas dans le range de integer.

On va corriger ça pour que la valeur reste dans le range de l’integer

Merci David pour cette réponse.

Et suite à cette modification, sur quelle critère je peux me baser pour détecter que l’appel à isActionEnable ne se fait pas dans le cas d’un affichage du formulaire et éviter de faire mon appel SQL ?

@francois est-ce que l’éditeur de template utilise une instance particulière ?

2092 me semble un peu faible.
La plage d’un int signé est effectivement -2147483648 … 2147483647
sur mysql les ID sont des int(11) donc c’est passé inaperçu.

Pour reconnaitre un objet dans l’éditeur de template, il faut regarder la présence d’un paramètre

obj.getParameter("_UI_EDIT_TEMPLATE_")!=null

car il faut nécessairement utiliser l’instance Main pour pas mal de raisons UI/ajax.

Simplicité se sert notamment de ce flag pour publier/afficher les champs hidden ou forbidden pour les placer quand même à l’écran (car la visibilité peut être contrainte dynamiquement).

Ceci dit personne ne doit jamais tester ce flag, votre code doit marcher quelle que soit la valeur du row_id (une fois la dummy value corrigée).

Ceci dit personne ne doit jamais tester ce flag, votre code doit marcher quelle que soit la valeur du row_id (une fois la dummy value corrigée).

Si je ne peux pas utiliser ce flag, est ce qu’il y en a un qui m’assure que je suis uniquement dans le cas d’un affichage du formulaire par un utilisateur et qu’il serait acceptable d’utiliser ?

Le flag est accessible donc vous pouvez l’utiliser, mais vous faites surement une erreur de conception ou de robustesse du code, car personne ne fait jamais ça. Il n’y a jamais de cas particulier “edition de template” vs “affichage normal” à gérer d’un point de vue métier.

Sinon expliquez nous pourquoi l’écran d’édition devrait faire des choses différentes de l’écran final.

Je pense que le pb est que le isActionEnable de l’objet implémente une logique qui utilise le row ID, or avec un dummy row ID d’un format inadapté (9999999999) ça posait un pb technique à cette logique.

Maintenant qu’il a un format plus “normal” ça devrait aller.

@francois, suite discussion de ce jour la question derrière la question c’est pourquoi le hook isActionEnable est appelé dans le contexte d’édition de template (donc avec des donnés dummy)

Dans leur cas ce hook implémente des traitements qui marchent quand on a a de vraies données mais pas avec des données dummy.

L’idée est bien de ne pas avoir a écrire dans le code un if (contexte d'édition de template) { return true; } else { /* vraie règle métier */ }

L’éditeur de template est une surcouche à l’affichage nominal du runtime.

Le runtime n’a pas non plus à tester qu’il est dans ce contexte pour chaque affichage de formulaire + appel de hooks = cela entrainera une dégradation des performances pour 99,99% des accès aux objets qui ne sont pas dans ce contexte.

Cependant, on peut faire des exceptions comme cela a dû être fait pour les champs cachés/interdits qui sont tout de même affichés dans ce contexte d’édition pour des vraies raisons de design. On peut ajouter isActionEnable, initUpdate… au détriment des performances : faire un test d’existence dans un hash des paramètres pour chaque action, ça doit pas être énorme mais multiplié par 99,99% des usages du formulaire (hors édition de template) ça fait beaucoup de secondes par jour au final.

Ceci étant, ça ne corrigera pas le pb de robustesse du code qui a été ajouté dans le isActionEnable, le code doit être robuste aux données et donc ne doit pas dépendre de l’existence d’un row_id en base.

Par exemple en cas de concurrence d’accès : un autre thread/utilisateur peut supprimer l’enregistrement au moment où l’autre utilisateur l’affiche. Le row_id (ou toute autre valeur, état…) qu’il soit dummy, invalide ou supprimé doit être géré dans un code robuste :

  • par un if (select(row_id)) ... ou équivalent d’existence de la resource accédée
  • ou par un try/catch { /* ignore */ } si un tel test n’est pas faisable, et si c’est juste un warning occasionnel à poubeliser (deleted record, editeur de template, date invalide…)

@david
Merci de me donner l’extrait du code qui a vitalement besoin d’un row_id pour ne pas planter.
On verra ce qu’on peut faire, mais on ne va pas impacter le runtime général pour un simple pb d’algorithme dans un cas particulier de hook mal isolé.

Je n’ai pas l’extrait de code en question.
@adrien-olivier pouvez vous fournir ça ?

Merci pour ces analyses de la robustesse de notre code, je vais m’éfforcer de répondre a vos question :

Sinon expliquez nous pourquoi l’écran d’édition devrait faire des choses différentes de l’écran final.

Car dans notre cas, en fonction de l’utilisateur, on affiche ou pas l’action mais que l’affichage des champs dans l’éditeur n’est pas conditionner par un utilisateur, sauf erreur de ma part.

Ceci étant, ça ne corrigera pas le pb de robustesse du code qui a été ajouté dans le isActionEnable, le code doit être robuste aux données et donc ne doit pas dépendre de l’existence d’un row_id en base.

Selon la définition du hook, sauf cas particulier dont il n’est pas question ici ( concurrence … ) , le row_id est soit null soit égal au row-id de l’objet actuel.

@david En fait ca ne seras pas nécessaire, nous avons modifié le code pour qu’il ne cause plus de problème en cas d’appel hors affichage d’un formulaire.

Le problème ne vient pas d’un accès concurent mais que le hook soit appelé dans un contexte inatendus au vu de sa définition ( isActionEnable devrait selon la documentation etre appelé avec un rowId soit existant, soit null mais pas avec une valeur dummy )

Ma question reformulée est donc :
Est-ce qu’il faut que l’on vérifie systématiquement les données que simplicité fournies dans nos hooks car il es prévus dans l’architecture de Simplicité d’appeler ces méthodes dans des contextes rompant avec la logique attendus ?

Pour répondre à la question finale, ce cas des données dummy de l’editeur de template est le seul cas où on se retrouve avec des données qui n’existent pas en base. @francois tu confirme ?

Reste que une recherche sur un row ID qui n’existe pas ne devrait pas faire pire que renvoyer un resultat vide. Donc on ne voit pas pourquoi cela pose un pb bloquant dans votre cas (en tout cas depuis que le dummy row ID ne vaut plus 999999999). Peut on voir le code en question pour comprendre ? Merci

Comme vu ensemble il y a une incompréhension, je voulais savoir si on pouvais se retrouver dans cette situation dans d’autre méthode ou a d’autre moment et donc si il fallait porter un point d’attention particulier a vérifier les données que l’on recupére.

Dans notre cas, nous avons utilisé le flag pour ne pas faire la requète, non pas pour ne pas avoir a traiter un retour null mais pour ne pas faire un appel sql inutile.

C’est pour ca que je n’ai pas mis le code que nous avons, pour ne pas que la discussion diverge sur comment faire pour éviter une NullPointerException.

Merci pour la réponse apportée, nous ferons attention au implémentation de isActionEnable dans le futur pour ne plus avoir de problème avec la dummy value de rowId.

Merci,

Nous avons bien noté votre besoin de ne pas appeler de hooks dans le contexte de l’édition de template, cela a bien du sens pour éviter le pb que vous nous remontez.

Il n’y pas de solution miracle à éditer une formulaire avec des fausses données + des faux droits pour tout voir + des hooks malins et des contraintes en back et en front qui modifient le comportement des visibilités à la volée, etc. Ca rejoint aussi une multitude de questions sur les données des unit tests + dummy data pour reproduire tous les cas possibles.

A mon avis il y a peu de hooks concernés, donc ça me semble faisable sans trop impacter les performance : il faudrait juste shunter les initUpdate, isXxxEnable, canReference et les contraintes.

Ca n’empêchera pas les vrais exceptions de type “null pointer” (concurrence d’accès, mémoire ou disque full…) et qu’il faut les traiter par un try/cactch ou un test d’existence même si l’appelant avait bien la ressource au moment de l’appel.

Dernier truc à savoir : Simplicité encapsule tous les appels de hook dans des try/catch pour s’isoler du code spécifique qui tomberait en erreur. Donc pas d’inquiétude, votre null-pointer n’a aucun effet indésirable sauf polluer les logs en design de template.