From 91f69a37d6b0a34b52c6f4618022e7a875902a79 Mon Sep 17 00:00:00 2001
From: Emmanuel Cazenave <ecazenave@entrouvert.com>
Date: Tue, 11 Jan 2022 11:30:20 +0100
Subject: [PATCH] formdata: improve migration of attachments to absolute path
 (#60380)

---
 tests/test_formdata.py | 67 ++++++++++++++++++++++++++++++++++++++++++
 wcs/workflows.py       | 16 +++++-----
 2 files changed, 76 insertions(+), 7 deletions(-)

diff --git a/tests/test_formdata.py b/tests/test_formdata.py
index 837fee728..48068337c 100644
--- a/tests/test_formdata.py
+++ b/tests/test_formdata.py
@@ -3,6 +3,7 @@ import datetime
 import io
 import os.path
 import time
+import uuid
 from unittest import mock
 
 import pytest
@@ -19,6 +20,8 @@ from wcs.formdata import Evolution
 from wcs.formdef import FormDef
 from wcs.qommon import force_str
 from wcs.qommon.http_request import HTTPRequest
+from wcs.qommon.misc import file_digest
+from wcs.qommon.storage import atomic_write
 from wcs.qommon.substitution import CompatibilityNamesDict
 from wcs.qommon.template import Template
 from wcs.qommon.upload_storage import PicklableUpload
@@ -3428,3 +3431,67 @@ def test_attachment_part_path_migration(pub):
     # check it was converted to relative path
     formdata = formdef.data_class().get(formdata.id)
     assert formdata.evolution[-1].parts[0].filename.startswith('attachments/')
+
+    # More realistic situation :
+    # * an absolute path serialized to storage
+    # * fetch the formadata
+    # * store it again and check that the path is converted to a a relative path
+
+    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'
+    attachment_evolution_part = AttachmentEvolutionPart(
+        'hello.txt', fp=io.BytesIO(b'test'), content_type='text/plain', varname='testfile'
+    )
+    formdata.evolution[-1].parts = [attachment_evolution_part]
+
+    # monkeypatch so that the filename is stored as an absolute path
+    def get_state():
+        odict = attachment_evolution_part.__dict__.copy()
+        del odict['__getstate__']
+        if not odict.get('fp'):
+            if 'filename' not in odict:
+                # we need a filename as an identifier: create one from nothing
+                # instead of file_digest(self.fp) (see below)
+                odict['filename'] = 'uuid-%s' % uuid.uuid4()
+                attachment_evolution_part.filename = odict['filename']
+            return odict
+
+        del odict['fp']
+
+        # 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(attachment_evolution_part.fp)
+            # create subdirectory with digest prefix as name
+            dirname = os.path.join('attachments', filename[:4])
+            abs_dirname = os.path.join(get_publisher().app_dir, dirname)
+            os.makedirs(abs_dirname, exist_ok=True)
+            odict['filename'] = os.path.join(abs_dirname, filename)
+            attachment_evolution_part.filename = odict['filename']
+            attachment_evolution_part.fp.seek(0)
+            atomic_write(attachment_evolution_part.get_file_path(), attachment_evolution_part.fp)
+
+        return odict
+
+    attachment_evolution_part.__getstate__ = get_state
+
+    formdata.store()
+    # check that the path is absolute
+    assert formdata.evolution[-1].parts[0].filename.startswith(pub.APP_DIR)
+    assert os.path.exists(formdata.evolution[-1].parts[0].get_file_path())
+
+    # get a frech instance not monkeypatched
+    formdata = formdef.data_class().get(formdata.id)
+    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/wcs/workflows.py b/wcs/workflows.py
index 9a0b7c0f5..607a70c09 100644
--- a/wcs/workflows.py
+++ b/wcs/workflows.py
@@ -297,18 +297,20 @@ class AttachmentEvolutionPart(EvolutionPart):
 
     def __getstate__(self):
         odict = self.__dict__.copy()
-        if not odict.get('fp'):
-            if 'filename' not in odict:
-                # we need a filename as an identifier: create one from nothing
-                # instead of file_digest(self.fp) (see below)
-                odict['filename'] = 'uuid-%s' % uuid.uuid4()
-                self.filename = odict['filename']
+        if not odict.get('fp') and 'filename' not in odict:
+            # we need a filename as an identifier: create one from nothing
+            # instead of file_digest(self.fp) (see below)
+            odict['filename'] = 'uuid-%s' % uuid.uuid4()
+            self.filename = odict['filename']
             return odict
 
-        del odict['fp']
+        if 'fp' in odict:
+            del odict['fp']
 
         # there is no filename, or it was a temporary one: create it
         if 'filename' not in odict or odict['filename'].startswith('uuid-'):
+            if not self.fp:
+                return odict
             filename = file_digest(self.fp)
             # create subdirectory with digest prefix as name
             dirname = os.path.join('attachments', filename[:4])
-- 
2.30.2

