More robust subdomain handling. Implemented more unit tests.

apps/helpers.py

@@ -3,10 +3,11 @@
 from typing import Any, Optional
 
 import regex as re
+from django.conf import settings
 from django.core.exceptions import ObjectDoesNotExist, ValidationError
 from django.db import transaction
 
-from apps.types_.subdomain import SubdomainCandidateName, SubdomainTuple
+from apps.types_.subdomain import SubdomainCandidateName
 from studio.utils import get_logger
 
 from .models import Apps, BaseAppInstance, K8sUserAppStatus, Subdomain
@@ -353,7 +354,6 @@ def get_URI(instance):
     return URI
 
 
-# TODO: Status. Unit test this. mock deploy_resource()
 @transaction.atomic
 def create_instance_from_form(form, project, app_slug, app_id=None):
     """
@@ -435,7 +435,9 @@ def create_instance_from_form(form, project, app_slug, app_id=None):
 
     # TODO: Status. Check this use:
     setup_instance(instance, subdomain, app, project, user_action)
-    save_instance_and_related_data(instance, form)
+    id = save_instance_and_related_data(instance, form)
+
+    assert id > 0, "The instance id should be a positive int"
 
     if do_deploy:
         logger.debug(f"Now deploying resource app with app_id = {app_id}")
@@ -446,6 +448,8 @@ def create_instance_from_form(form, project, app_slug, app_id=None):
     else:
         logger.debug(f"Not re-deploying this app with app_id = {app_id}")
 
+    return id
+
 
 def get_subdomain_name(form):
     subdomain_tuple = form.cleaned_data.get("subdomain")
@@ -459,9 +463,22 @@ def get_or_create_status(instance, app_id):
     # return instance.app_status if app_id else AppStatus.objects.create()
 
 
-def handle_subdomain_change(instance, subdomain, subdomain_name):
+def handle_subdomain_change(instance: Any, subdomain: str, subdomain_name: str) -> None:
+    """
+    Detects if there has been a user-initiated subdomain change and if so,
+    then re-creates the app instance, also re-deploying the k8s resource.
+    """
     from .tasks import delete_resource
 
+    assert instance is not None, "instance is required"
+    # type(instance) is for example DashInstance
+
+    # assert instance.subdomain is not None, f"subdomain is required but is None for instance {instance.name}"
+    if instance.subdomain is None:
+        # TODO: Check this. It is possible for subdomain to be missing.
+        # But is this valid?
+        return
+
     if instance.subdomain.subdomain != subdomain_name:
         # The user modified the subdomain name
         # In this special case, we avoid async task.
@@ -503,12 +520,19 @@ def setup_instance(instance, subdomain, app, project, user_action=None, is_creat
     # instance.app_status = status
 
 
-def save_instance_and_related_data(instance, form):
+def save_instance_and_related_data(instance: Any, form: Any) -> int:
+    """
+    Saves a new or re-saves an existing app instance to the database.
+
+    Returns:
+    - int: The Id of the new or updated app instance.
+    """
     instance.save()
     form.save_m2m()
     instance.set_k8s_values()
     instance.url = get_URI(instance)
     instance.save(update_fields=["k8s_values", "url"])
+    return instance.id
 
 
 def validate_path_k8s_label_compatible(candidate: str) -> None:

apps/tests/test_app_helper_utils.py

@@ -1,99 +1,259 @@
 """This module is used to test the helper functions that are used by user app instance functionality."""
 
-import unittest
 from unittest.mock import patch
 
+import pytest
 from django.contrib.auth import get_user_model
 from django.test import TestCase
 
 from projects.models import Flavor, Project
 
 from ..app_registry import APP_REGISTRY
-from ..helpers import create_instance_from_form
-from ..models import Apps, JupyterInstance, K8sUserAppStatus
+from ..helpers import create_instance_from_form, get_subdomain_name
+from ..models import Apps, DashInstance, K8sUserAppStatus, Subdomain
+from ..types_.subdomain import SubdomainTuple
 
 User = get_user_model()
 
 
 class CreateAppInstanceTestCase(TestCase):
-    """Test case for testing create_instance_from_form"""
+    """
+    Test case for helper function create_instance_from_form
+    while creating a new user app instance.
+    """
 
     def setUp(self):
         self.user = User.objects.create_user("foo1", "[email protected]", "bar")
         self.project = Project.objects.create_project(name="test-app-creation", owner=self.user, description="")
+        self.flavor = Flavor.objects.create(name="flavor", project=self.project)
 
-    @unittest.expectedFailure
-    def test_create_instance_from_form_valid_input(self):
-        # TODO: Status
-
-        # Create the form data
-        flavor = Flavor.objects.filter(project=self.project).first()
+        self.app_slug = "dashapp"
 
-        app_slug = "dashapp"
+        self.app = Apps.objects.create(
+            name="Create App Test",
+            slug=self.app_slug,
+            user_can_delete=False,
+        )
 
+    def test_create_instance_from_form_valid_input(self):
+        # Create the form data
         data = {
-            "name": "app-form-name",
+            "name": "test-app-form-name",
             "description": "app-form-description",
-            "flavor": str(flavor.pk),
+            "flavor": str(self.flavor.pk),
             "access": "public",
             "port": 8000,
             "image": "some-image",
             "source_code_url": "https://someurlthatdoesnotexist.com",
         }
 
-        _, form_class = APP_REGISTRY.get(app_slug)
+        _, form_class = APP_REGISTRY.get(self.app_slug)
         form = form_class(data, project_pk=self.project.pk)
 
-        with patch("apps.tasks.delete_resource.delay") as mock_task:  # noqa: F841
-            create_instance_from_form(form, self.project, app_slug, app_id=None)
+        self.assertTrue(form.is_valid(), f"The form should be valid but has errors: {form.errors}")
 
-            pass
+        with patch("apps.tasks.deploy_resource.delay") as mock_task:
+            id = create_instance_from_form(form, self.project, self.app_slug, app_id=None)
 
-        self.assertTrue(1 == 0)
+            self.assertIsNotNone(id)
+            self.assertTrue(id > 0)
 
-    @unittest.expectedFailure
-    def test_create_instance_from_form_invalid_input(self):
-        # TODO: Status
-        with patch("apps.tasks.delete_resource.delay") as mock_task:  # noqa: F841
-            pass
+            # Get app instance and verify the status codes
+            app_instance = DashInstance.objects.get(pk=id)
 
-        self.assertTrue(1 == 0)
+            # Validate app instance properties
+            self.assertIsNotNone(app_instance)
+            self.assertEqual(app_instance.latest_user_action, "Creating")
+            self.assertIsNone(app_instance.k8s_user_app_status)
+            self.assertEqual(app_instance.name, data.get("name"))
+            self.assertEqual(app_instance.description, data.get("description"))
+            self.assertEqual(app_instance.access, data.get("access"))
+            self.assertEqual(app_instance.port, data.get("port"))
+            self.assertEqual(app_instance.image, data.get("image"))
+            self.assertEqual(app_instance.source_code_url, data.get("source_code_url"))
+
+            self.assertIsNotNone(app_instance.subdomain)
+            subdomain_name = app_instance.subdomain.subdomain
+            self.assertIsNotNone(subdomain_name)
+            # Example subdomain name pattern: rd5d576b4
+            self.assertTrue(
+                subdomain_name.startswith("r"), f"The subdomain should begin with r but was {subdomain_name}"
+            )
+
+            mock_task.assert_called_once()
 
 
 class UpdateExistingAppInstanceTestCase(TestCase):
-    """Test case for test create_instance_from_form and an existing user app instance."""
+    """
+    Test case for helper function create_instance_from_form
+    using an existing user app instance.
+    """
 
     def setUp(self) -> None:
         self.user = User.objects.create_user("foo1", "[email protected]", "bar")
+        self.project = Project.objects.create_project(name="test-app-updating", owner=self.user, description="")
+        self.flavor = Flavor.objects.create(name="flavor", project=self.project)
+
+        self.app_slug = "dashapp"
+
         self.app = Apps.objects.create(
-            name="Custom App",
-            slug="customapp",
+            name="Update App Test",
+            slug=self.app_slug,
+            user_can_delete=False,
         )
 
-        self.project = Project.objects.create_project(name="test-perm", owner=self.user, description="")
+        self.subdomain_name = "test-subdomain"
+        subdomain = Subdomain.objects.create(subdomain=self.subdomain_name)
 
         k8s_user_app_status = K8sUserAppStatus.objects.create()
-        self.app_instance = JupyterInstance.objects.create(
+        self.app_instance = DashInstance.objects.create(
             access="public",
             owner=self.user,
-            name="test_app_instance_public",
+            name="test-app-name-original",
             app=self.app,
             project=self.project,
+            subdomain=subdomain,
             k8s_user_app_status=k8s_user_app_status,
         )
 
-    @unittest.expectedFailure
-    def test_create_instance_from_form_valid_input(self):
-        # TODO: Status. Implement
-        with patch("apps.tasks.delete_resource.delay") as mock_task:  # noqa: F841
-            pass
+        self.assertIsNotNone(self.app_instance.id)
+        self.assertTrue(self.app_instance.id > 0)
+        self.assertIsNotNone(self.app_instance.subdomain)
+        self.assertIsNotNone(self.app_instance.subdomain.subdomain)
+
+    def test_update_instance_from_form_modify_port_should_redeploy(self):
+        """
+        Test function create_instance_from_form to update an existing app instance
+        to modify properties that should result in a re-deployment.
+        """
+
+        # Create the form data
+        # TODO: Need to investigate subdomain. Does it always need to be included?
+        # If included and the same, then get Subdomain already exists. Please choose another one.
+        data = {
+            "name": "test-app-name-new",
+            "description": "app-form-description",
+            "access": "public",
+            "port": 9999,
+            "image": "some-image",
+            "source_code_url": "https://someurlthatdoesnotexist.com",
+        }
+
+        _, form_class = APP_REGISTRY.get(self.app_slug)
+        form = form_class(data, project_pk=self.project.pk)
+
+        self.assertTrue(form.is_valid(), f"The form should be valid but has errors: {form.errors}")
+
+        with patch("apps.tasks.deploy_resource.delay") as mock_task:
+            id = create_instance_from_form(form, self.project, self.app_slug, app_id=self.app_instance.id)
+
+            self.assertIsNotNone(id)
+            self.assertTrue(id > 0)
+
+            # Get app instance and verify the status codes
+            app_instance = DashInstance.objects.get(pk=id)
+
+            # Validate app instance properties
+            self.assertIsNotNone(app_instance)
+            self.assertEqual(app_instance.latest_user_action, "Changing")
+            self.assertIsNone(app_instance.k8s_user_app_status)
+            self.assertEqual(app_instance.name, data.get("name"))
+            self.assertEqual(app_instance.description, data.get("description"))
+            self.assertEqual(app_instance.access, data.get("access"))
+            self.assertEqual(app_instance.port, data.get("port"))
+            self.assertEqual(app_instance.image, data.get("image"))
+            self.assertEqual(app_instance.source_code_url, data.get("source_code_url"))
+
+            mock_task.assert_called_once()
+
+    def test_update_instance_from_form_modify_image_should_redeploy(self):
+        """
+        Test function create_instance_from_form to update an existing app instance
+        to modify properties that should result in a re-deployment.
+        """
+
+        pass
+
+    def test_update_instance_from_form_modify_subdomain_should_redeploy(self):
+        """
+        Test function create_instance_from_form to update an existing app instance
+        to modify properties that should result in a re-deployment.
+        """
+
+        # is_created_by_user = False
+        # subdomain = SubdomainTuple(self.subdomain_name, is_created_by_user)
+
+        pass
+
+    def test_update_instance_from_form_modify_no_redeploy_values(self):
+        """
+        Test function create_instance_from_form to update an existing app instance
+        to modify only properties that do not result in a re-deployment.
+        """
+
+        # TODO:
+        pass
+
+        # mock_task.assert_ not called
+
+
+@pytest.mark.django_db
+def test_get_subdomain_name():
+    """Test function get_subdomain_name using form data with a subdomain."""
+
+    expected_subomain_name = "test-subdomain"
+    is_created_by_user = False
+    subdomain = SubdomainTuple(expected_subomain_name, is_created_by_user)
+
+    data = {
+        "name": "app-form-name",
+        "description": "app-form-description",
+        "access": "public",
+        "port": 9999,
+        "image": "some-image",
+        "source_code_url": "https://someurlthatdoesnotexist.com",
+        "subdomain": subdomain,
+    }
+
+    _, form_class = APP_REGISTRY.get("dashapp")
+    form = form_class(data)
+
+    assert form.is_valid(), f"The form should be valid but has errors: {form.errors}"
+
+    subdomain_name, is_created_by_user = get_subdomain_name(form)
+
+    assert (
+        subdomain_name == expected_subomain_name
+    ), f"The determined subdomain name {subdomain_name} should equal \
+        the expected name {expected_subomain_name}"
+
+    # The function overrides the input is_created_by_user setting it to True
+    # because the user specified the subdomain.
+    assert is_created_by_user is True, f"is_created_by_user should be True but was {is_created_by_user}"
+
+
+@pytest.mark.django_db
+def test_get_subdomain_name_no_subdomain_in_form():
+    """Test function get_subdomain_name using form data without subdomain."""
+
+    data = {
+        "name": "app-form-name",
+        "description": "app-form-description",
+        "access": "public",
+        "port": 9999,
+        "image": "some-image",
+        "source_code_url": "https://someurlthatdoesnotexist.com",
+    }
+
+    _, form_class = APP_REGISTRY.get("dashapp")
+    form = form_class(data)
 
-        self.assertTrue(1 == 0)
+    assert form.is_valid(), f"The form should be valid but has errors: {form.errors}"
 
-    @unittest.expectedFailure
-    def test_create_instance_from_form_invalid_input(self):
-        # TODO: Status. Implement
-        with patch("apps.tasks.delete_resource.delay") as mock_task:  # noqa: F841
-            pass
+    subdomain_name, is_created_by_user = get_subdomain_name(form)
 
-        self.assertTrue(1 == 0)
+    # The get_subdomain_name function sets a random release name if not specified by the user
+    assert subdomain_name is not None, "The subdomain should not be None"
+    # Example subdomain name pattern: rd5d576b4
+    assert subdomain_name.startswith("r"), f"The subdomain should begin with r but was {subdomain_name}"
+    assert is_created_by_user is False, f"is_created_by_user should be False but was {is_created_by_user}"