Merge pull request #636 from huard/fix-635

Create output directory before estimating available space on device

Author: huard

pywps/inout/storage/file.py

@@ -69,6 +69,14 @@ def __init__(self, output_path, output_url, copy_function=None):
         self.copy_function = copy_function
 
     def _do_store(self, output):
+        """Copy output to final storage location.
+
+        - Create output directory
+        - Check available file space
+        - Create output file name, taking care of possible duplicates
+        - Copy / link output in work directory to output directory
+        - Return store type, output path and output URL
+        """
         import platform
         import math
         import tempfile
@@ -78,6 +86,11 @@ def _do_store(self, output):
 
         request_uuid = output.uuid or uuid.uuid1()
 
+        # Create a target folder for each request
+        target = os.path.join(self.target, str(request_uuid))
+        if not os.path.exists(target):
+            os.makedirs(target)
+
         # st.blksize is not available in windows, skips the validation on windows
         if platform.system() != 'Windows':
             file_block_size = os.stat(file_name).st_blksize
@@ -91,11 +104,6 @@ def _do_store(self, output):
             if avail_size < actual_file_size:
                 raise NotEnoughStorage('Not enough space in {} to store {}'.format(self.target, file_name))
 
-        # create a target folder for each request
-        target = os.path.join(self.target, str(request_uuid))
-        if not os.path.exists(target):
-            os.makedirs(target)
-
         # build output name
         output_name, suffix = _build_output_name(output)
         # build tempfile in case of duplicates
@@ -116,7 +124,7 @@ def _do_store(self, output):
         url = self.url("{}/{}".format(request_uuid, just_file_name))
         LOGGER.info('File output URI: {}'.format(url))
 
-        return (STORE_TYPE.PATH, output_name, url)
+        return STORE_TYPE.PATH, output_name, url
 
     @staticmethod
     def copy(src, dst, copy_function=None):
@@ -134,7 +142,7 @@ def copy(src, dst, copy_function=None):
             try:
                 os.link(src, dst)
             except Exception:
-                LOGGER.warn("Could not create hardlink. Fallback to copy.")
+                LOGGER.warning("Could not create hardlink. Fallback to copy.")
                 FileStorage.copy(src, dst)
         else:
             shutil.copy2(src, dst)

tests/test_storage.py

@@ -2,33 +2,53 @@
 # Copyright 2018 Open Source Geospatial Foundation and others    #
 # licensed under MIT, Please consult LICENSE.txt for details     #
 ##################################################################
+import pytest
 
 from pywps.inout.storage.builder import StorageBuilder
 from pywps.inout.storage.file import FileStorage
 from pywps.inout.storage.s3 import S3Storage
 
 from pywps import configuration
 
+from pathlib import Path
 import unittest
+import tempfile
 
-class StorageBuilderTests(unittest.TestCase):
+
+@pytest.fixture
+def fake_output(tmp_path):
+    class FakeOutput(object):
+        """Fake output object for testing."""
+        def __init__(self):
+            self.identifier = "fake_output"
+            self.file = self._get_file()
+            self.uuid = None
+
+        def _get_file(self):
+            fn = tmp_path / 'file.tiff'
+            fn.touch()
+            return str(fn.absolute())
+
+    return FakeOutput()
+
+
+class TestStorageBuilder():
 
     def test_default_storage(self):
         storage = StorageBuilder.buildStorage()
-        self.assertIsInstance(storage, FileStorage)
-
+        assert isinstance(storage, FileStorage)
 
     def test_s3_storage(self):
         configuration.CONFIG.set('server', 'storagetype', 's3')
         storage = StorageBuilder.buildStorage()
-        self.assertIsInstance(storage, S3Storage)
-
-def load_tests(loader=None, tests=None, pattern=None):
-    """Load local tests
-    """
-    if not loader:
-        loader = unittest.TestLoader()
-    suite_list = [
-        loader.loadTestsFromTestCase(StorageBuilderTests)
-    ]
-    return unittest.TestSuite(suite_list)
+        assert isinstance(storage, S3Storage)
+
+    def test_recursive_directory_creation(self, fake_output):
+        """Test that outputpath is created."""
+        configuration.CONFIG.set('server', 'storagetype', 'file')
+        outputpath = Path(tempfile.gettempdir()) / "a" / "b" / "c"
+        configuration.CONFIG.set('server', 'outputpath', str(outputpath))
+        storage = StorageBuilder.buildStorage()
+
+        storage.store(fake_output)
+        assert outputpath.exists()