Projet

Général

Profil

Development #56537

prendre en charge les fichiers de type "texte" (txt, xml, svg,...) dans l'action document

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

Statut:
Fermé
Priorité:
Normal
Assigné à:
Version cible:
-
Début:
01 septembre 2021
Echéance:
% réalisé:

0%

Temps estimé:
Patch proposed:
Oui
Planning:
Non

Description

Parce que ce n'est pas bien dur à faire (juste appliquer le rendu de gabarit au fichier), et que ça permettrait de générer des documents parfois utiles.


Fichiers


Demandes liées

Lié à Passerelle - Development #56526: connection json → xmlRejeté31 août 2021

Actions

Révisions associées

Révision f2eaa892 (diff)
Ajouté par Benjamin Dauvergne il y a plus de 2 ans

wf/export_to_model: support XML templates (#56537)

Historique

#2

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

#3

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

(il y a quand même toute une série d'abus étranges que ça va amener, mais ok).

juste appliquer le rendu de gabarit au fichier

Parce que produire de l'XML à base de gabarit, ça n'a jamais vraiment été recommandé; pour s'éviter des galères j'ajouterais aussi une étape "vérifier un minimum ce qui a été produit", i.e. si on est sur de l'xml vérifier qu'il est valide bien formé, pareil pour du json, etc.

#4

Mis à jour par Benjamin Dauvergne il y a plus de 2 ans

L'implémentation minimale pour moi c'est de dire que si on ne détecte pas un fichier libre-office via libmagic, on est en mode texte. La validation xml si c'est du xml ça peut faire l'objet d'un autre ticket, si on s'aperçoit que c'est fréquent.

#5

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

Mon souhait ici est qu'il soit intégré dès le début cette validation.

#6

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

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

Mon souhait ici est qu'il soit intégré dès le début cette validation.

Je n'ai pas compris "dès le début". On ne pourra pas valider qu'un template Django destiné à produire du XML va bien le faire, le moindre "if" ou "for" va vite casser la logique du XML. On peut en revanche détecter si c'est bien un template Django à la syntaxe a priori valide, c'est de ça dont tu parlais ?

#7

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

"dès le début" c'était pour dire dès ce ticket, c'est mon objection à "ça peut faire l'objet d'un autre ticket, si on s'aperçoit que c'est fréquent.".

#8

Mis à jour par Benjamin Dauvergne il y a plus de 2 ans

Ok la vue de JSON me fait un peu peur, donc je pense qu'on va limiter ça à ".xml" dès le départ comme on a déjà une détection un peu bâtarde de RTF et OpenDocument à base de content_type/extension. Je n'ai pas du tout envie de débugger du JSON généré par un template. Je préférerai qu'on ouvre les vannes petit à petit suite à cette première expérience (genre si on voit un besoin de CSV, on l'envisagera à ce moment là).

#9

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

À noter quand même que sur du libreoffice on ne teste rien pas grand chose : upload.content_type.startswith('application/vnd.oasis.opendocument.')

Quelle serait l'idée de valider quelque chose ici ? Si le XML renvoyé est foireux, quelle importance, et autant voir le résultat planté plutôt que rien.

#10

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

C'est n'importe quoi de générer du XML à coup de print() et si on veut le faire malgré tout je pense que vérifier que ce qui est généré est bien formé est un minimum, que ça permet d'être averti dès w.c.s., et pas trois jours plus tard par un commentaire comme quoi l'application métier supposée absorber le fichier généré a fait une trace java de 2 kilomètres.

#11

Mis à jour par Benjamin Dauvergne il y a plus de 2 ans

  • Assigné à mis à Benjamin Dauvergne
#12

Mis à jour par Benjamin Dauvergne il y a plus de 2 ans

#13

Mis à jour par Benjamin Dauvergne il y a plus de 2 ans

Voilà, je suis allé au plus simple, ça n'accepte que les fichiers .xml et ça vérifie le format du fichier en sortie. Et en vrai pour tricher on peut toujours mettre un fichier qui s'appelle .xml mais qui contient du JSON ou du CSV et le renommer ensuite.

#14

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

Benjamin Dauvergne a écrit :

Voilà, je suis allé au plus simple, ça n'accepte que les fichiers .xml et ça vérifie le format du fichier en sortie. Et en vrai pour tricher on peut toujours mettre un fichier qui s'appelle .xml mais qui contient du JSON ou du CSV et le renommer ensuite.

Sur le test du type : upload.base_filename.endswith('.rtf')upload.base_filename.endswith('.xml')

raise UploadValidationError(_('Only RTF and OpenDocument files can be used')) : je préférerais ne pas cacher que XML est accepté, et l'ajouter dans le message.

raise TemplatingError(_('Error in template: output is not valid XML (%s)') % str(e)) : j'imagine que ça va planter le workflow qui va s'arrêter sur le statut, ça ne me va pas : les actions suivantes ne sont pas exécutées, risque de se retrouver dans un statut "technique" qui ne devrait pas être possible, mais surtout on ne verra pas le XML généré (pour comprendre le pépin). Perso je trouve qu'on devrait juste avoir le record_error, mais pas ce raise. Oui, il y aura un XML planté dans outstream, mais c'est mieux que de ne rien avoir. Et on comprendra la source de la trace java pourrie.

(Je vois qu'on fait ce raise aussi pour le mode RTF, comme ce n'est plus jamais utilisé on ne voit pas de soucis)

L'alternative serait d'ajouter un "saut en cas d'erreur" dans l'action, et bof.

#15

Mis à jour par Benjamin Dauvergne il y a plus de 2 ans

Thomas Noël a écrit :

Benjamin Dauvergne a écrit :

Voilà, je suis allé au plus simple, ça n'accepte que les fichiers .xml et ça vérifie le format du fichier en sortie. Et en vrai pour tricher on peut toujours mettre un fichier qui s'appelle .xml mais qui contient du JSON ou du CSV et le renommer ensuite.

Je dis une bêtise, ça plantera sur la validation XML, manque de glucose avant le repas.

Sur le test du type : upload.base_filename.endswith('.rtf')upload.base_filename.endswith('.xml')

Ok, j'ajoute un test. PS: au passage je vois que la partie avant ça n'est pas bonne non plus, il faut ignorer content_type dans ce cas là et j'ai ajouté un vague test de préfixe sur le contenu en plus du test sur l'extension.

raise UploadValidationError(_('Only RTF and OpenDocument files can be used')) : je préférerais ne pas cacher que XML est accepté, et l'ajouter dans le message.

Ok, un oubli.

raise TemplatingError(_('Error in template: output is not valid XML (%s)') % str(e)) : j'imagine que ça va planter le workflow qui va s'arrêter sur le statut, ça ne me va pas : les actions suivantes ne sont pas exécutées, risque de se retrouver dans un statut "technique" qui ne devrait pas être possible, mais surtout on ne verra pas le XML généré (pour comprendre le pépin). Perso je trouve qu'on devrait juste avoir le record_error, mais pas ce raise. Oui, il y aura un XML planté dans outstream, mais c'est mieux que de ne rien avoir. Et on comprendra la source de la trace java pourrie.

Ok, on aura quand même une erreur si le template Django est planté, ça va ça ? (sur une TemplateError on convertit en TemplatingError, je ne sais pas bien c'est utile d'ailleurs).

#16

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

Jenkins est tout rouge, sans doute y'a des trucs à revoir.

Ok pour la validation du XML qui donne juste un warning, très bien selon moi. Je reste gêné par le "raise" en bas de apply_text_template_to_formdata. Je comprends que c'est repris de apply_rtf_template_to_formdata (que personne plus jamais n'utilise) : pour moi ça va faire le process de génération du document va exploser en milieu de workflow, ça ne va pas. Je préfère qu'on fasse comme pour odt : on laisse filer, au pire y'a pas de doc généré, juste une erreur enregistrée, voilà c'est la vie.

#17

Mis à jour par Benjamin Dauvergne il y a plus de 2 ans

Thomas Noël a écrit :

Ok pour la validation du XML qui donne juste un warning, très bien selon moi. Je reste gêné par le "raise" en bas de apply_text_template_to_formdata. Je comprends que c'est repris de apply_rtf_template_to_formdata (que personne plus jamais n'utilise) : pour moi ça va faire le process de génération du document va exploser en milieu de workflow, ça ne va pas. Je préfère qu'on fasse comme pour odt : on laisse filer, au pire y'a pas de doc généré, juste une erreur enregistrée, voilà c'est la vie.

Bon en fait de toute façon template_on_formdata() ne peut pas lever TemplateError parce qu'il vérifie que la syntaxe est bonne et que le constructeur de Template ne reçoit pas raises=True concernant les erreurs durant le rendu, en fait c'est tout à inutile (ou alors peut-être en EZT, mais passons), je vire ça et ne met aucune contrôle de plus.

#18

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

Benjamin Dauvergne a écrit :

Thomas Noël a écrit :

Ok pour la validation du XML qui donne juste un warning, très bien selon moi. Je reste gêné par le "raise" en bas de apply_text_template_to_formdata. Je comprends que c'est repris de apply_rtf_template_to_formdata (que personne plus jamais n'utilise) : pour moi ça va faire le process de génération du document va exploser en milieu de workflow, ça ne va pas. Je préfère qu'on fasse comme pour odt : on laisse filer, au pire y'a pas de doc généré, juste une erreur enregistrée, voilà c'est la vie.

Bon en fait de toute façon template_on_formdata() ne peut pas lever TemplateError parce qu'il vérifie que la syntaxe est bonne et que le constructeur de Template ne reçoit pas raises=True concernant les erreurs durant le rendu, en fait c'est tout à inutile (ou alors peut-être en EZT, mais passons), je vire ça et ne met aucune contrôle de plus.

Ca me va d'aller avec ça.

Il me reste le petit « xml_prefix == b'<' » qui impose donc que le document ne commence jamais par du Django... en vrai je pense que ça peut s'entendre comme condition. Je pensais même même être plus méchants encore et imposer «

#19

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

Au passage, ajouter "XML" dans le

raise UploadValidationError(_('Only RTF and OpenDocument files can be used'))
#20

Mis à jour par Benjamin Dauvergne il y a plus de 2 ans

J'ai carrément viré RTF du message, on va dire que c'est la dépréciation qui commence.

#21

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

Ah, zut, raté ça, lorsque le rendu n'est pas XML, plutôt que "Error in template for export to model", dire plutôt "Le résultat du gabarit n'est pas un document XML valide" (enfin en anglais, mais tu connais mes performances).

Après ça je pourrais très certainement valider.

#22

Mis à jour par Benjamin Dauvergne il y a plus de 2 ans

Je commence à en avoir un peu ras le cul de vous, allez vous jeter dans les plantes vertes.

#23

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

  • Statut changé de Solution proposée à Solution validée

Benjamin Dauvergne a écrit :

Je commence à en avoir un peu ras le cul de vous, allez vous jeter dans les plantes vertes.

Ah, y'en a, super !

En vrai, merci à toi pour ce dev (me retrouver juste relecteur-râleur fût confortable)

#24

Mis à jour par Benjamin Dauvergne il y a plus de 2 ans

  • Statut changé de Solution validée à Résolu (à déployer)
commit f2eaa89247504a75e848ff32a23dbe2436a21b0a
Author: Benjamin Dauvergne <bdauvergne@entrouvert.com>
Date:   Thu Sep 2 11:43:30 2021 +0200

    wf/export_to_model: support XML templates (#56537)
#26

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

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

Formats disponibles : Atom PDF