validate app name (#1186)

Author: dantownsend

piccolo/apps/app/commands/new.py

@@ -3,6 +3,7 @@
 import importlib
 import os
 import pathlib
+import string
 import sys
 import typing as t
 
@@ -33,24 +34,48 @@ def module_exists(module_name: str) -> bool:
         return True
 
 
+APP_NAME_ALLOWED_CHARACTERS = [*string.ascii_lowercase, *string.digits, "_"]
+
+
+def validate_app_name(app_name: str):
+    """
+    Make sure the app name is something which is a valid Python package name.
+
+    :raises ValueError:
+        If ``app_name`` isn't valid.
+
+    """
+    for char in app_name:
+        if not char.lower() in APP_NAME_ALLOWED_CHARACTERS:
+            raise ValueError(
+                f"The app name contains a disallowed character: `{char}`. "
+                "It must only include a-z, 0-9, and _ characters."
+            )
+
+
 def get_app_module(app_name: str, root: str) -> str:
     return ".".join([*pathlib.Path(root).parts, app_name, "piccolo_app"])
 
 
 def new_app(app_name: str, root: str = ".", register: bool = False):
     print(f"Creating {app_name} app ...")
 
-    app_root = os.path.join(root, app_name)
-
-    if os.path.exists(app_root):
-        sys.exit("Folder already exists - exiting.")
+    try:
+        validate_app_name(app_name=app_name)
+    except ValueError as exception:
+        sys.exit(str(exception))
 
     if module_exists(app_name):
         sys.exit(
             f"A module called {app_name} already exists - possibly a builtin "
             "Python module. Please choose a different name for your app."
         )
 
+    app_root = os.path.join(root, app_name)
+
+    if os.path.exists(app_root):
+        sys.exit("Folder already exists - exiting.")
+
     os.makedirs(app_root)
 
     with open(os.path.join(app_root, "__init__.py"), "w"):

tests/apps/app/commands/test_new.py

@@ -3,7 +3,12 @@
 import tempfile
 from unittest import TestCase
 
-from piccolo.apps.app.commands.new import get_app_module, module_exists, new
+from piccolo.apps.app.commands.new import (
+    get_app_module,
+    module_exists,
+    new,
+    validate_app_name,
+)
 
 
 class TestModuleExists(TestCase):
@@ -45,6 +50,22 @@ def test_new_with_clashing_name(self):
         )
 
 
+class TestValidateAppName(TestCase):
+
+    def test_validate_app_name(self):
+        """
+        Make sure only app names which work as valid Python package names are
+        allowed.
+        """
+        # Should be rejected:
+        for app_name in ("MY APP", "app/my_app", "my.app"):
+            with self.assertRaises(ValueError):
+                validate_app_name(app_name=app_name)
+
+        # Should work fine:
+        validate_app_name(app_name="music")
+
+
 class TestGetAppIdentifier(TestCase):
 
     def test_get_app_module(self):