bugy#184 changed REST script start request to streaming, to upload hu…

…ge files efficiently + added config for max http request size

.travis.yml

@@ -22,7 +22,7 @@ before_install:
 - sudo apt-get -y install python3-pip python3-setuptools
 install:
 - sudo pip3 install -r requirements.txt
-- sudo pip3 install ldap3
+- sudo pip3 install ldap3 requests parameterized
 - sudo pip3 install pyasn1 --upgrade
 - cd web-src
 - npm install

samples/configs/parameterized.json

@@ -2,7 +2,6 @@
   "name": "Very parameterized",
   "script_path": "scripts/parameterized.sh",
   "description": "This script does nothing except accepting a lot of parameters and printing them",
-  "requires_terminal": false,
   "allowed_users": [
     "*"
   ],

samples/scripts/parameterized.sh

@@ -37,17 +37,15 @@ printf '%s\n' "${args[@]}"
 echo
 
 if [ ! -z "$recurs_file" ]; then
-    echo "recurs_file=$recurs_file"
-    echo "md5="`md5sum "$recurs_file"`
+    echo "recurs_file="`md5sum "$recurs_file"`
 fi
 
 echo
 
 if [ -z "$my_file" ]; then
     echo '--file_upload is empty'
 else
-    echo '--file_upload content:'
-    cat "$my_file"
+    echo "--file_upload: "`md5sum "$my_file"`
 fi
 
 sleep 5

src/features/file_upload_feature.py

@@ -2,7 +2,7 @@
 import os
 
 from files.user_file_storage import UserFileStorage
-from utils import file_utils
+from utils.file_utils import normalize_path
 
 RESULT_FILES_FOLDER = 'uploadFiles'
 
@@ -16,11 +16,6 @@ def __init__(self, user_file_storage: UserFileStorage, temp_folder) -> None:
 
         user_file_storage.start_autoclean(self.folder, 1000 * 60 * 60 * 24)
 
-    def save_file(self, filename, body, username) -> str:
-        upload_folder = self.user_file_storage.prepare_new_folder(username, self.folder)
-        pref_result_path = os.path.join(upload_folder, filename)
-
-        result_path = file_utils.create_unique_filename(pref_result_path)
-        file_utils.write_file(result_path, body, True)
-
-        return file_utils.normalize_path(result_path)
+    def prepare_new_folder(self, username) -> str:
+        new_folder = self.user_file_storage.prepare_new_folder(username, self.folder)
+        return normalize_path(new_folder)

src/model/model_helper.py

@@ -115,6 +115,25 @@ def read_bool(value):
     return value.lower() == 'true'
 
 
+def read_int_from_config(key, config_obj, *, default=None):
+    value = config_obj.get(key)
+    if value is None:
+        return default
+
+    if isinstance(value, int) and not isinstance(value, bool):
+        return value
+
+    if isinstance(value, str):
+        if value.strip() == '':
+            return default
+        try:
+            return int(value)
+        except ValueError as e:
+            raise InvalidValueException(key, 'Invalid %s value: integer expected, but was: %s' % (key, value)) from e
+
+    raise InvalidValueTypeException('Invalid %s value: integer expected, but was: %s' % (key, repr(value)))
+
+
 def is_empty(value):
     return (not value) and (value != 0) and (value is not False)
 
@@ -197,3 +216,8 @@ class InvalidValueException(Exception):
     def __init__(self, param_name, validation_error) -> None:
         super().__init__(validation_error)
         self.param_name = param_name
+
+
+class InvalidValueTypeException(Exception):
+    def __init__(self, message) -> None:
+        super().__init__(message)

src/model/server_conf.py

@@ -5,7 +5,7 @@
 import utils.file_utils as file_utils
 from auth.authorization import ANY_USER
 from model import model_helper
-from model.model_helper import read_list
+from model.model_helper import read_list, read_int_from_config
 from utils.string_utils import strip
 
 LOGGER = logging.getLogger('server_conf')
@@ -26,6 +26,7 @@ def __init__(self) -> None:
         self.trusted_ips = []
         self.user_groups = None
         self.admin_users = []
+        self.max_request_size_mb = None
 
     def get_port(self):
         return self.port
@@ -129,6 +130,8 @@ def from_json(conf_path, temp_folder):
     config.user_groups = user_groups
     config.admin_users = admin_users
 
+    config.max_request_size_mb = read_int_from_config('max_request_size', json_object, default=10)
+
     return config
 
 

src/tests/file_upload_feature_test.py

@@ -1,4 +1,5 @@
 import os
+import time
 import unittest
 
 from features.file_upload_feature import FileUploadFeature
@@ -17,16 +18,17 @@ def tearDown(self):
         test_utils.cleanup()
         self.__storage._stop_autoclean()
 
-    def test_create_file(self):
-        file_path = self.upload_feature.save_file('my_file.txt', b'test text', 'userX')
+    def test_prepare_new_folder(self):
+        file_path = self.upload_feature.prepare_new_folder('userX')
         self.assertTrue(os.path.exists(file_path))
 
-    def test_content_in_created_file(self):
-        file_path = self.upload_feature.save_file('my_file.txt', b'My text', 'userX')
-        content = file_utils.read_file(file_path)
-        self.assertEqual('My text', content)
+    def test_prepare_new_folder_different_users(self):
+        path1 = self.upload_feature.prepare_new_folder('userX')
+        path2 = self.upload_feature.prepare_new_folder('userY')
+        self.assertNotEqual(path1, path2)
 
-    def test_same_filename(self):
-        file_path1 = self.upload_feature.save_file('my_file.txt', b'some text', 'userX')
-        file_path2 = self.upload_feature.save_file('my_file.txt', b'some text', 'userX')
+    def test_prepare_new_folder_twice(self):
+        file_path1 = self.upload_feature.prepare_new_folder('userX')
+        time.sleep(0.1)
+        file_path2 = self.upload_feature.prepare_new_folder('userX')
         self.assertNotEqual(file_path1, file_path2)

src/tests/model_helper_test.py

@@ -4,7 +4,7 @@
 from config.constants import FILE_TYPE_FILE, FILE_TYPE_DIR
 from model import model_helper
 from model.model_helper import read_list, read_dict, fill_parameter_values, resolve_env_vars, \
-    InvalidFileException, read_bool_from_config
+    InvalidFileException, read_bool_from_config, InvalidValueException, InvalidValueTypeException
 from tests import test_utils
 from tests.test_utils import create_parameter_model, set_env_value
 
@@ -282,3 +282,35 @@ def setUp(self):
 
     def tearDown(self):
         test_utils.cleanup()
+
+
+class TestReadIntFromConfig(unittest.TestCase):
+    def test_normal_int_value(self):
+        value = model_helper.read_int_from_config('abc', {'abc': 123})
+        self.assertEqual(123, value)
+
+    def test_zero_int_value(self):
+        value = model_helper.read_int_from_config('abc', {'abc': 0})
+        self.assertEqual(0, value)
+
+    def test_string_value(self):
+        value = model_helper.read_int_from_config('abc', {'abc': '-666'})
+        self.assertEqual(-666, value)
+
+    def test_string_value_when_invalid(self):
+        self.assertRaises(InvalidValueException, model_helper.read_int_from_config, 'abc', {'abc': '1000b'})
+
+    def test_unsupported_type(self):
+        self.assertRaises(InvalidValueTypeException, model_helper.read_int_from_config, 'abc', {'abc': True})
+
+    def test_default_value(self):
+        value = model_helper.read_int_from_config('my_key', {'abc': 100})
+        self.assertIsNone(value)
+
+    def test_default_value_explicit(self):
+        value = model_helper.read_int_from_config('my_key', {'abc': 100}, default=5)
+        self.assertEqual(5, value)
+
+    def test_default_value_when_empty_string(self):
+        value = model_helper.read_int_from_config('my_key', {'my_key': ' '}, default=9999)
+        self.assertEqual(9999, value)

src/tests/server_conf_test.py

@@ -105,6 +105,20 @@ def tearDown(self):
         test_utils.cleanup()
 
 
+class TestMaxRequestSize(unittest.TestCase):
+    def test_int_value(self):
+        config = _from_json({'max_request_size': 5})
+        self.assertEqual(5, config.max_request_size_mb)
+
+    def test_string_value(self):
+        config = _from_json({'max_request_size': '123'})
+        self.assertEqual(123, config.max_request_size_mb)
+
+    def test_default_value(self):
+        config = _from_json({})
+        self.assertEqual(10, config.max_request_size_mb)
+
+
 def _from_json(content):
     json_obj = json.dumps(content)
     conf_path = os.path.join(test_utils.temp_folder, 'conf.json')

src/tests/test_utils.py

@@ -4,6 +4,7 @@
 import stat
 import threading
 import uuid
+from collections import Set
 
 import utils.file_utils as file_utils
 import utils.os_utils as os_utils
@@ -15,6 +16,7 @@
 temp_folder = 'tests_temp'
 _original_env = {}
 
+
 def create_file(filepath, overwrite=False):
     if not os.path.exists(temp_folder):
         os.makedirs(temp_folder)
@@ -54,14 +56,14 @@ def create_dir(dir_path):
 
 def setup():
     if os.path.exists(temp_folder):
-        _rmtree()
+        _rmtree(temp_folder)
 
     os.makedirs(temp_folder)
 
 
 def cleanup():
     if os.path.exists(temp_folder):
-        _rmtree()
+        _rmtree(temp_folder)
 
     os_utils.reset_os()
 
@@ -74,7 +76,7 @@ def cleanup():
     _original_env.clear()
 
 
-def _rmtree():
+def _rmtree(folder):
     exception = None
 
     def on_rm_error(func, path, exc_info):
@@ -87,7 +89,7 @@ def on_rm_error(func, path, exc_info):
             if exception is None:
                 exception = e
 
-    shutil.rmtree(temp_folder, onerror=on_rm_error)
+    shutil.rmtree(folder, onerror=on_rm_error)
     if exception:
         raise exception
 
@@ -134,7 +136,6 @@ def create_script_param_config(
         file_recursive=None,
         file_type=None,
         file_extensions=None):
-
     conf = {'name': param_name}
 
     if type is not None:
@@ -308,6 +309,42 @@ def set_env_value(key, value):
     os.environ[key] = value
 
 
+def assert_large_dict_equal(expected, actual, testcase):
+    if len(expected) < 20 and len(actual) < 20:
+        testcase.assertEqual(expected, actual)
+        return
+
+    if expected == actual:
+        return
+
+    diff_expected = {}
+    diff_actual = {}
+    too_large_diff = False
+
+    all_keys = set()
+    all_keys.update(expected.keys())
+    all_keys.update(actual.keys())
+    for key in all_keys:
+        expected_value = expected.get(key)
+        actual_value = actual.get(key)
+
+        if expected_value == actual_value:
+            continue
+
+        diff_expected[key] = expected_value
+        diff_actual[key] = actual_value
+
+        if len(diff_expected) >= 50:
+            too_large_diff = True
+            break
+
+    message = 'Showing only different elements'
+    if too_large_diff:
+        message += ' (limited to 50)'
+
+    testcase.assertEqual(diff_expected, diff_actual, message)
+
+
 class _MockProcessWrapper(ProcessWrapper):
     def __init__(self, executor, command, working_directory):
         super().__init__(command, working_directory)
@@ -374,4 +411,3 @@ def is_allowed(self, user_id, allowed_users):
 
     def is_admin(self, user_id):
         return True
-