Add tests for FileQuestion

src/yunohost/tests/test_questions.py

@@ -14,7 +14,8 @@ from yunohost.utils.config import (
     PasswordQuestion,
     DomainQuestion,
     PathQuestion,
-    BooleanQuestion
+    BooleanQuestion,
+    FileQuestion
 )
 from yunohost.utils.error import YunohostError, YunohostValidationError
 
@@ -62,6 +63,23 @@ def test_question_string():
     assert out.value == "some_value"
 
 
+def test_question_string_from_query_string():
+
+    questions = [
+        {
+            "name": "some_string",
+            "type": "string",
+        }
+    ]
+    answers = "foo=bar&some_string=some_value&lorem=ipsum"
+
+    out = ask_questions_and_parse_answers(questions, answers)[0]
+
+    assert out.name == "some_string"
+    assert out.type == "string"
+    assert out.value == "some_value"
+
+
 def test_question_string_default_type():
     questions = [
         {
@@ -1916,7 +1934,13 @@ def test_question_number_input_test_ask_with_help():
 
 
 def test_question_display_text():
-    questions = [{"name": "some_app", "type": "display_text", "ask": "foobar"}]
+    questions = [
+            {
+                "name": "some_app",
+                "type": "display_text",
+                "ask": "foobar"
+            }
+    ]
     answers = {}
 
     with patch.object(sys, "stdout", new_callable=StringIO) as stdout, patch.object(
@@ -1926,6 +1950,72 @@ def test_question_display_text():
         assert "foobar" in stdout.getvalue()
 
 
+def test_question_file_from_cli():
+
+    FileQuestion.clean_upload_dirs()
+
+    filename = "/tmp/ynh_test_question_file"
+    os.system(f"rm -f {filename}")
+    os.system(f"echo helloworld > {filename}")
+
+    questions = [
+        {
+            "name": "some_file",
+            "type": "file",
+        }
+    ]
+    answers = {"some_file": filename}
+
+    out = ask_questions_and_parse_answers(questions, answers)[0]
+
+    assert out.name == "some_file"
+    assert out.type == "file"
+
+    # The file is supposed to be copied somewhere else
+    assert out.value != filename
+    assert out.value.startswith("/tmp/")
+    assert os.path.exists(out.value)
+    assert "helloworld" in open(out.value).read().strip()
+
+    FileQuestion.clean_upload_dirs()
+
+    assert not os.path.exists(out.value)
+
+
+def test_question_file_from_api():
+
+    FileQuestion.clean_upload_dirs()
+
+    from base64 import b64encode
+
+    b64content = b64encode("helloworld".encode())
+    questions = [
+        {
+            "name": "some_file",
+            "type": "file",
+        }
+    ]
+    answers = {"some_file": b64content}
+
+    interface_type_bkp = Moulinette.interface.type
+    try:
+        Moulinette.interface.type = "api"
+        out = ask_questions_and_parse_answers(questions, answers)[0]
+    finally:
+        Moulinette.interface.type = interface_type_bkp
+
+    assert out.name == "some_file"
+    assert out.type == "file"
+
+    assert out.value.startswith("/tmp/")
+    assert os.path.exists(out.value)
+    assert "helloworld" in open(out.value).read().strip()
+
+    FileQuestion.clean_upload_dirs()
+
+    assert not os.path.exists(out.value)
+
+
 def test_normalize_boolean_nominal():
 
     assert BooleanQuestion.normalize("yes") == 1

src/yunohost/utils/config.py

@@ -31,6 +31,7 @@ from moulinette.interfaces.cli import colorize
 from moulinette import Moulinette, m18n
 from moulinette.utils.log import getActionLogger
 from moulinette.utils.filesystem import (
+    read_file,
     write_to_file,
     read_toml,
     read_yaml,
@@ -167,6 +168,9 @@ class ConfigPanel:
             raise
         finally:
             # Delete files uploaded from API
+            # FIXME : this is currently done in the context of config panels,
+            # but could also happen in the context of app install ... (or anywhere else
+            # where we may parse args etc...)
             FileQuestion.clean_upload_dirs()
 
         self._reload_services()
@@ -969,10 +973,9 @@ class FileQuestion(Question):
     @classmethod
     def clean_upload_dirs(cls):
         # Delete files uploaded from API
-        if Moulinette.interface.type == "api":
+        for upload_dir in cls.upload_dirs:
-            for upload_dir in cls.upload_dirs:
+            if os.path.exists(upload_dir):
-                if os.path.exists(upload_dir):
+                shutil.rmtree(upload_dir)
-                    shutil.rmtree(upload_dir)
 
     def __init__(self, question):
         super().__init__(question)
@@ -984,12 +987,13 @@ class FileQuestion(Question):
 
         super()._prevalidate()
 
-        if not self.value or not os.path.exists(str(self.value)):
+        if Moulinette.interface.type != "api":
-            raise YunohostValidationError(
+            if not self.value or not os.path.exists(str(self.value)):
-                "app_argument_invalid",
+                raise YunohostValidationError(
-                name=self.name,
+                    "app_argument_invalid",
-                error=m18n.n("file_does_not_exist", path=str(self.value)),
+                    name=self.name,
-            )
+                    error=m18n.n("file_does_not_exist", path=str(self.value)),
+                )
 
     def _post_parse_value(self):
         from base64 import b64decode
@@ -997,22 +1001,21 @@ class FileQuestion(Question):
         if not self.value:
             return self.value
 
-        # FIXME : in the cli case, don't we want to also copy the file
+        upload_dir = tempfile.mkdtemp(prefix="ynh_filequestion_")
-        # to a tmp work dir ?
+        _, file_path = tempfile.mkstemp(dir=upload_dir)
+
+        FileQuestion.upload_dirs += [upload_dir]
+
+        logger.debug(f"Saving file {self.name} for file question into {file_path}")
+        if Moulinette.interface.type != "api":
+            content = read_file(str(self.value), file_mode="rb")
 
         if Moulinette.interface.type == "api":
+            content = b64decode(self.value)
 
-            upload_dir = tempfile.mkdtemp(prefix="tmp_configpanel_")
+        write_to_file(file_path, content, file_mode="wb")
-            _, file_path = tempfile.mkstemp(prefix="foobar", dir=upload_dir)
 
-            FileQuestion.upload_dirs += [upload_dir]
+        self.value = file_path
-            logger.debug(f"Save uploaded file from API into {file_path}")
-
-            content = self.value
-
-            write_to_file(file_path, b64decode(content), file_mode="wb")
-
-            self.value = file_path
 
         return self.value