Produits Entr'ouvert » w.c.s.

Un travail d'implémentation est visible sur http://git.entrouvert.org/wcs.git/?h=wip/alternate-file-storage

Les objets PicklableUpload gagnent un attribut dictionnaire "storage_attrs" quand ils sont passés par RemoteOpaqueUploadStorage. Dans ce dictionnaire on a un redirect_url qui est l'URL vers laquelle rediriger l'utilisateur quand il veut ouvrir le fichier ; cette URL de redirection est signée au moment de clic, permettant un accès sécurisé à un système distant d'accès au fichier.

En export JSON, les fichiers distants ont un content vide mais on renvoie storage_attrs.

Ce qui me chiffonne un peu dans ma solution c'est les tests du genre « if hasattr(upload, 'storage_attrs') » pour savoir si un fichier est distant ou pas. Peut-être qu'on pourrait avoir des attributs plus explicites, genre upload.opaque ou upload.remote autre... mais ça va pas forcément rendre les choses plus simples. Idées bienvenues le cas échéant. (Dans le patch original de Frédéric il y avait un test sur un attribut upload.redirect_url)

Je verrais un truc comme upload.can_thumbnail(), upload.get_json_value() (pour les deux vus rapidement); et dans Upload alors la mécanique qui va déléguer la question à la classe de stockage.

est-ce que je me fais peur à moi-même ?

Lance les tests sans les hasattr pour voir ?

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

est-ce que je me fais peur à moi-même ?

Lance les tests sans les hasattr pour voir ?

  py3-pylint-coverage: commands succeeded
  congratulations :)

Courageux, j'ai donc refais le commit sans les hasattr. C'est dans la branche, voici l'interdiff :

diff --git a/wcs/fields.py b/wcs/fields.py
index 50583408..ebaf5ea4 100644
--- a/wcs/fields.py
+++ b/wcs/fields.py
@@ -1041,7 +1041,7 @@ class FileField(WidgetField):
         t = TemplateIO(html=True)
         t += htmltext('<div class="file-field">')
         t += htmltext('<a download="%s" href="[download]?f=%s">') % (value.base_filename, self.id)
-        if include_image_thumbnail and hasattr(value, 'can_thumbnail') and value.can_thumbnail():
+        if include_image_thumbnail and value.can_thumbnail():
             t += htmltext('<img alt="" src="[download]?f=%s&thumbnail=1"/>') % self.id
         t += htmltext('<span>%s</span>') % value
         t += htmltext('</a></div>')
diff --git a/wcs/forms/common.py b/wcs/forms/common.py
index af4486c3..4b9214e2 100644
--- a/wcs/forms/common.py
+++ b/wcs/forms/common.py
@@ -70,7 +70,7 @@ class FileDirectory(Directory):
         response = get_response()

         if self.thumbnails:
-            if hasattr(file, 'can_thumbnail') and file.can_thumbnail():
+            if file.can_thumbnail():
                 try:
                     thumbnail = misc.get_thumbnail(file.get_filename(),
                             content_type=file.content_type)
@@ -686,7 +686,7 @@ class FormStatusPage(Directory, FormTemplateMixin):
         file_url = 'files/%s/' % fn

         if get_request().form.get('thumbnail') == '1':
-            if hasattr(file, 'can_thumbnail') and file.can_thumbnail():
+            if file.can_thumbnail():
                 file_url += 'thumbnail/'
             else:
                 raise errors.TraversalError()

Ca te semble une façon correcte de faire ? (ajout du can_thumbnail: http://git.entrouvert.org/wcs.git/commit/?h=wip/39517-alternate-file-storage&id=c81c1bbc02907fce1b67996c4a31e2d70b46b56a)

Yep ça me parait bien.

Ça me fait voir quand même dans le commit cet autre bout avec un hasattr qui serait à remplacer par une méthode :

+        if hasattr(file, 'storage_attrs'):  # remote storage
             if not file.storage_attrs.get('redirect_url'):

La limite actuelle de l'implémentation reste la gestion des receive() et donc ce qui concerne l'upload de document via des formulaires de workflows et leur transfert dans les données de traitement, par exemple. Je pense cependant en faire un ticket séparé.

Ok mais faut ce ticket.

_~~

Il va falloir rebaser en faisant attention, notamment à ce bout :

+    def __setstate__(self, dict):
+        self.__dict__.update(dict)
+        if hasattr(self, 'data'):
+            # backward compatibility with older w.c.s. version
+            self.fp = StringIO(self.data)
+            del self.data

(qui est BytesIO dans master).

_~~

À repasser sur le branche j'aurais déplacé la partie "RemoteOpaqueUploadStorage" dans un commit séparé/final mais ok.

_~~

+    def get_redirect_url(self, upload, backoffice=False):
+        raise NotImplementedError('no get_redirect_url on AttachmentEvolutionPart object')

Sans doute un ticket également à faire pour gérer ça.

_~~

+        if hasattr(self, 'storage_attrs'):  # remote storage

Je serais pour s/remote storage/alternative storage/, même si on n'a pas de perspective particulière à ce sujet.

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

La limite actuelle de l'implémentation reste la gestion des receive() et donc ce qui concerne l'upload de document via des formulaires de workflows et leur transfert dans les données de traitement, par exemple. Je pense cependant en faire un ticket séparé.

Ok mais faut ce ticket.

Hop, #39976

Il va falloir rebaser en faisant attention, notamment à ce bout :

C'est rebasé avec ce BytesIO.

À repasser sur le branche j'aurais déplacé la partie "RemoteOpaqueUploadStorage" dans un commit séparé/final mais ok.

Ouf (je suis pas assez doué en rebase pour ça)

raise NotImplementedError('no get_redirect_url on AttachmentEvolutionPart object')

Sans doute un ticket également à faire pour gérer ça.

Tu veux dire gérer des fichiers distants au niveau des AttachmentEvolutionPart ?

Je serais pour s/remote storage/alternative storage/, même si on n'a pas de perspective particulière à ce sujet.

Yep c'est changé en "alternative storage" (sur la branche).

raise NotImplementedError('no get_redirect_url on AttachmentEvolutionPart object')

Sans doute un ticket également à faire pour gérer ça.

Tu veux dire gérer des fichiers distants au niveau des AttachmentEvolutionPart ?

Pas la moindre idée je suis juste parti sur l'idée que ce NotImplementedError était uniquement temporaire, en attendant "quelque chose". S'il a vocation à rester là je serais plutôt pour taper une autre exception, pour laquelle on recevra une trace si jamais le code se trouve appelé.

Nouvelle branche poussée avec des AssertionError à la place des NotImplementedError (pas d'URL distante pour les fichiers locaux, ni pour les fichiers enregistrés dans l'historique -- du moins pour ces derniers pas dans cette version où has_redirect_url est toujours False).