From 92bcc14fb1e023f4c44521400d23fe00f1b1d157 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20P=C3=A9ters?= <fpeters@entrouvert.com>
Date: Sun, 31 Oct 2021 13:49:21 +0100
Subject: [PATCH] misc: reference attachments using a relative path (#21731)

---
 tests/test_formdata.py     | 35 +++++++++++++++++++++++++++++++++++
 tests/workflow/test_all.py | 18 +++++++++---------
 wcs/workflows.py           | 27 ++++++++++++++++++---------
 3 files changed, 62 insertions(+), 18 deletions(-)

diff --git a/tests/test_formdata.py b/tests/test_formdata.py
index 2c394e6f2..693463ddb 100644
--- a/tests/test_formdata.py
+++ b/tests/test_formdata.py
@@ -1,5 +1,7 @@
 import collections
 import datetime
+import io
+import os.path
 import time
 from unittest import mock
 
@@ -23,6 +25,7 @@ from wcs.variables import LazyFormData
 from wcs.wf.register_comment import JournalEvolutionPart
 from wcs.wf.wscall import JournalWsCallErrorPart
 from wcs.workflows import (
+    AttachmentEvolutionPart,
     Workflow,
     WorkflowBackofficeFieldsFormDef,
     WorkflowCriticalityLevel,
@@ -3183,3 +3186,35 @@ def test_block_variables(pub):
     formdata.store()
     tmpl = Template('{{ form_var_block|getlist:"foo"|sum }} {{ form_var_block|getlist:"bar"|sum }}')
     assert tmpl.render(context) == '9 2'
+
+
+def test_attachment_part_path_migration(pub):
+    FormDef.wipe()
+    formdef = FormDef()
+    formdef.name = 'test'
+    formdef.fields = []
+    formdef.store()
+
+    formdef.data_class().wipe()
+    formdata = formdef.data_class()()
+    formdata.just_created()
+    formdata.status = 'wf-new'
+    formdata.evolution[-1].status = 'wf-new'
+    formdata.evolution[-1].parts = [
+        AttachmentEvolutionPart(
+            'hello.txt', fp=io.BytesIO(b'test'), content_type='text/plain', varname='testfile'
+        )
+    ]
+    formdata.store()
+    assert formdata.evolution[-1].parts[0].filename.startswith('attachments/')
+    assert os.path.exists(formdata.evolution[-1].parts[0].get_file_path())
+
+    # add full path as it was done before
+    formdata.evolution[-1].parts[0].filename = os.path.join(
+        pub.app_dir, formdata.evolution[-1].parts[0].filename
+    )
+    formdata.store()
+
+    # check it was converted to relative path
+    formdata = formdef.data_class().get(formdata.id)
+    assert formdata.evolution[-1].parts[0].filename.startswith('attachments/')
diff --git a/tests/workflow/test_all.py b/tests/workflow/test_all.py
index c529247fe..569fd827a 100644
--- a/tests/workflow/test_all.py
+++ b/tests/workflow/test_all.py
@@ -3417,7 +3417,7 @@ def test_export_to_model_image(pub, template_name):
     item.perform(formdata)
 
     assert formdata.evolution[-1].parts[-1].base_filename == 'template.odt'
-    with zipfile.ZipFile(formdata.evolution[-1].parts[0].filename, mode='r') as zfile:
+    with zipfile.ZipFile(formdata.evolution[-1].parts[0].get_file_path(), mode='r') as zfile:
         zinfo = zfile.getinfo('Pictures/10000000000000320000003276E9D46581B55C88.jpg')
     # check the image has been replaced by the one from the formdata
     assert zinfo.file_size == len(image_data)
@@ -3432,7 +3432,7 @@ def test_export_to_model_image(pub, template_name):
 
         item.perform(formdata)
 
-        with zipfile.ZipFile(formdata.evolution[-1].parts[0].filename, mode='r') as zfile:
+        with zipfile.ZipFile(formdata.evolution[-1].parts[0].get_file_path(), mode='r') as zfile:
             zinfo = zfile.getinfo('Pictures/10000000000000320000003276E9D46581B55C88.jpg')
         # check the original image has been left
         assert zinfo.file_size == 580
@@ -3476,7 +3476,7 @@ def test_export_to_model_qrcode(pub):
     item.perform(formdata)
 
     assert formdata.evolution[-1].parts[-1].base_filename == 'template.odt'
-    with zipfile.ZipFile(formdata.evolution[-1].parts[0].filename, mode='r') as zfile:
+    with zipfile.ZipFile(formdata.evolution[-1].parts[0].get_file_path(), mode='r') as zfile:
         # base template use a jpg images and export_to_model does not rename it
         # event when content is PNG, but it still works inside LibreOffice
         # which ignores the filename extension.
@@ -3583,7 +3583,7 @@ def test_export_to_model_django_template(pub):
     item.convert_to_pdf = False
     item.perform(formdata)
 
-    with open(formdata.evolution[0].parts[0].filename, 'rb') as fd:
+    with open(formdata.evolution[0].parts[0].get_file_path(), 'rb') as fd:
         with zipfile.ZipFile(fd) as zout:
             new_content = zout.read('content.xml')
     assert b'>foo-export-to-template-with-django<' in new_content
@@ -3592,7 +3592,7 @@ def test_export_to_model_django_template(pub):
     formdef.store()
     item.perform(formdata)
 
-    with open(formdata.evolution[0].parts[1].filename, 'rb') as fd:
+    with open(formdata.evolution[0].parts[1].get_file_path(), 'rb') as fd:
         with zipfile.ZipFile(fd) as zout:
             new_content = zout.read('content.xml')
     assert b'>Name with a \' simple quote<' in new_content
@@ -3601,7 +3601,7 @@ def test_export_to_model_django_template(pub):
     formdef.store()
     item.perform(formdata)
 
-    with open(formdata.evolution[0].parts[2].filename, 'rb') as fd:
+    with open(formdata.evolution[0].parts[2].get_file_path(), 'rb') as fd:
         with zipfile.ZipFile(fd) as zout:
             new_content = zout.read('content.xml')
     assert b'>A &lt;&gt; name<' in new_content
@@ -3639,7 +3639,7 @@ def test_export_to_model_xml(two_pubs):
         pub.substitutions.reset()
         pub.substitutions.feed(formdata)
         item.perform(formdata)
-        with open(formdata.evolution[0].parts[-1].filename) as fd:
+        with open(formdata.evolution[0].parts[-1].get_file_path()) as fd:
             return fd.read()
 
     # good XML
@@ -3738,7 +3738,7 @@ def test_export_to_model_form_details_section(pub, filename):
     item.convert_to_pdf = False
     item.perform(formdata)
 
-    with open(formdata.evolution[0].parts[0].filename, 'rb') as fd:
+    with open(formdata.evolution[0].parts[0].get_file_path(), 'rb') as fd:
         with zipfile.ZipFile(fd) as zout:
             new_content = force_text(zout.read('content.xml'))
     # section content has been removed
@@ -3762,7 +3762,7 @@ def test_export_to_model_form_details_section(pub, filename):
     assert 'XfooY, Xfoo2Y' in new_content
 
     if filename == 'template-form-details-no-styles.odt':
-        with open(formdata.evolution[0].parts[0].filename, 'rb') as fd:
+        with open(formdata.evolution[0].parts[0].get_file_path(), 'rb') as fd:
             with zipfile.ZipFile(fd) as zout:
                 new_styles = force_text(zout.read('styles.xml'))
         assert 'Field_20_Label' in new_styles
diff --git a/wcs/workflows.py b/wcs/workflows.py
index ca3961097..aa4917639 100644
--- a/wcs/workflows.py
+++ b/wcs/workflows.py
@@ -258,10 +258,16 @@ class AttachmentEvolutionPart(EvolutionPart):
             to=to,
         )
 
+    def get_file_path(self):
+        if os.path.isabs(self.filename):
+            return self.filename
+        else:
+            return os.path.join(get_publisher().app_dir, self.filename)
+
     def get_file_pointer(self):
         if self.filename.startswith('uuid-'):
             return None
-        return open(self.filename, 'rb')  # pylint: disable=consider-using-with
+        return open(self.get_file_path(), 'rb')  # pylint: disable=consider-using-with
 
     def __getstate__(self):
         odict = self.__dict__.copy()
@@ -274,20 +280,23 @@ class AttachmentEvolutionPart(EvolutionPart):
             return odict
 
         del odict['fp']
-        dirname = os.path.join(get_publisher().app_dir, 'attachments')
-        if not os.path.exists(dirname):
-            os.mkdir(dirname)
 
-        # there is not filename, or it was a temporary one: create it
+        # there is no filename, or it was a temporary one: create it
         if 'filename' not in odict or odict['filename'].startswith('uuid-'):
             filename = file_digest(self.fp)
-            dirname = os.path.join(dirname, filename[:4])
-            if not os.path.exists(dirname):
-                os.mkdir(dirname)
+            # create subdirectory with digest prefix as name
+            dirname = os.path.join('attachments', filename[:4])
+            os.makedirs(os.path.join(get_publisher().app_dir, dirname), exist_ok=True)
             odict['filename'] = os.path.join(dirname, filename)
             self.filename = odict['filename']
             self.fp.seek(0)
-            atomic_write(self.filename, self.fp)
+            atomic_write(self.get_file_path(), self.fp)
+        elif os.path.isabs(odict['filename']):
+            # current value is an absolute path, update it quietly to be a relative path
+            pub_app_path_prefix = os.path.join(get_publisher().app_dir, '')
+            if os.path.exists(odict['filename']) and odict['filename'].startswith(pub_app_path_prefix):
+                odict['filename'] = odict['filename'][len(pub_app_path_prefix) :]
+
         return odict
 
     def view(self):
-- 
2.33.1

