Updare upon review

Signed-off-by: Alina Buzachis <[email protected]>

Author: alinabuzachis

core/controller_client.py

@@ -6,41 +6,24 @@
 from requests import Session
 from requests.auth import HTTPBasicAuth
 
-from pattern_service.settings.aap import get_aap_settings
+from pattern_service.settings.aap import AAP_PASSWORD
+from pattern_service.settings.aap import AAP_USERNAME
+from pattern_service.settings.aap import AAP_VALIDATE_CERTS
 
 logger = logging.getLogger(__name__)
 
-settings = get_aap_settings()
 
-_aap_session: Optional[Session] = None
+def get_http_session() -> Session:
+    """Creates and returns a new Session instance with AAP credentials."""
+    session = Session()
+    session.auth = HTTPBasicAuth(AAP_USERNAME, AAP_PASSWORD)
+    session.verify = AAP_VALIDATE_CERTS
+    session.headers.update({"Content-Type": "application/json"})
+    return session
 
 
-def get_http_session(force_refresh: bool = False) -> Session:
-    """Returns a cached Session instance with AAP credentials."""
-    global _aap_session
-    if _aap_session is None or force_refresh:
-        session = Session()
-        session.auth = HTTPBasicAuth(settings.username, settings.password)
-        session.verify = settings.verify_ssl
-        session.headers.update({"Content-Type": "application/json"})
-        _aap_session = session
-
-    return _aap_session
-
-
-def get(path: str, *, params: Optional[Dict] = None) -> requests.Response:
-    session = get_http_session()
-    url = f"{settings.url}{path}"
-    response = session.get(url, params=params, stream=True)
-    response.raise_for_status()
-
-    return response
-
-
-def build_collection_uri(collection: str, version: str) -> str:
-    """Builds the URI for a given collection and version."""
-    base_url = settings.url
-    path = "/api/galaxy/v3/plugin/ansible/content/published/collections/artifacts"
-    filename = f"{collection}-{version}.tar.gz"
-
-    return f"{base_url}{path}/{filename}"
+def get(url: str, *, params: Optional[Dict] = None) -> requests.Response:
+    with get_http_session() as session:
+        response = session.get(url, params=params, stream=True)
+        response.raise_for_status()
+        return response

core/models.py

@@ -94,3 +94,8 @@ class Meta:
 
     status: models.CharField = models.CharField(max_length=20, choices=status_choices)
     details: models.JSONField = models.JSONField(null=True, blank=True)
+
+    def update_task_status(self, status_: str, details: dict) -> None:
+        self.status = status_
+        self.details = details
+        self.save()

core/services.py

@@ -1,5 +1,46 @@
-from core.services import pattern_task
+import json
+import logging
+import os
+
+from .models import Pattern
+from .models import Task
+from .utils import build_collection_uri
+from .utils import download_collection
+
+logger = logging.getLogger(__name__)
 
 
 def run_pattern_task(pattern_id: int, task_id: int) -> None:
-    pattern_task(pattern_id, task_id)
+    """
+    Orchestrates downloading a collection and saving a pattern definition.
+    """
+    task = Task.objects.get(id=task_id)
+    try:
+        pattern = Pattern.objects.get(id=pattern_id)
+        task.update_task_status("Running", {"info": "Processing pattern"})
+        with download_collection(
+            pattern.collection_name, pattern.collection_version
+        ) as collection_path:
+            path_to_definition = os.path.join(
+                collection_path,
+                "extensions",
+                "patterns",
+                pattern.pattern_name,
+                "meta",
+                "pattern.json",
+            )
+            with open(path_to_definition, "r") as file:
+                definition = json.load(file)
+
+            pattern.pattern_definition = definition
+            pattern.collection_version_uri = build_collection_uri(
+                pattern.collection_name, pattern.collection_version
+            )
+            pattern.save(update_fields=["pattern_definition", "collection_version_uri"])
+        task.update_task_status("Completed", {"info": "Pattern processed successfully"})
+    except FileNotFoundError:
+        logger.error(f"Could not find pattern definition for task {task_id}")
+        task.update_task_status("Failed", {"error": "Pattern definition not found."})
+    except Exception as e:
+        logger.error(f"Task {task_id} failed: {e}")
+        task.update_task_status("Failed", {"error": str(e)})

core/tasks.py

@@ -1,11 +1,8 @@
 import core.controller_client as cc
 
 
-def test_get_http_session_caches():
+def test_get_http_session():
     """Subsequent calls without force_refresh must return the *same* object."""
     s1 = cc.get_http_session()
     s2 = cc.get_http_session()
-    assert s1 is s2
-
-    s3 = cc.get_http_session(force_refresh=True)
-    assert s3 is not s1 and s3 is cc._aap_session
+    assert s1 is not s2

core/tests/test_controller_client.py

@@ -12,7 +12,7 @@
 from core.models import Pattern
 from core.models import PatternInstance
 from core.models import Task
-from core.services import pattern_task
+from core.tasks import run_pattern_task
 
 
 class SharedDataMixin:
@@ -78,12 +78,9 @@ def tearDown(self):
 
 
 class PatternTaskTest(SharedDataMixin, TestCase):
-    @patch(
-        "core.services.update_task_status",
-        wraps=pattern_task.__globals__["update_task_status"],
-    )
-    @patch("core.services.open", new_callable=mock_open, read_data='{"name": "test"}')
-    @patch("core.services.download_collection")
+    @patch("core.models.Task.update_task_status", autospec=True)
+    @patch("core.tasks.open", new_callable=mock_open, read_data='{"name": "test"}')
+    @patch("core.tasks.download_collection")
     def test_run_pattern_task_success(
         self, mock_download, mock_open_fn, mock_update_status
     ):
@@ -112,7 +109,7 @@ def test_run_pattern_task_success(
         ) as f:
             f.write(json.dumps({"name": "test"}))
 
-        pattern_task(pattern.id, task.id)
+        run_pattern_task(pattern.id, task.id)
 
         mock_update_status.assert_any_call(
             task, "Running", {"info": "Processing pattern"}
@@ -121,11 +118,8 @@ def test_run_pattern_task_success(
             task, "Completed", {"info": "Pattern processed successfully"}
         )
 
-    @patch(
-        "core.services.update_task_status",
-        wraps=pattern_task.__globals__["update_task_status"],
-    )
-    @patch("core.services.download_collection", side_effect=FileNotFoundError)
+    @patch("core.models.Task.update_task_status", autospec=True)
+    @patch("core.tasks.download_collection", side_effect=FileNotFoundError)
     def test_run_pattern_task_file_not_found(self, mock_download, mock_update_status):
         pattern = Pattern.objects.create(
             collection_name="demo.collection",
@@ -134,49 +128,55 @@ def test_run_pattern_task_file_not_found(self, mock_download, mock_update_status
         )
         task = Task.objects.create(status="Initiated", details={})
 
-        pattern_task(pattern.id, task.id)
+        run_pattern_task(pattern.id, task.id)
 
         mock_update_status.assert_called_with(
             task, "Failed", {"error": "Pattern definition not found."}
         )
 
-    @patch(
-        "core.services.download_collection", side_effect=Exception("Download failed")
-    )
+    @patch("core.tasks.download_collection", side_effect=Exception("Download failed"))
     def test_run_pattern_task_handles_download_failure(self, mock_download):
-        pattern_task(self.pattern.id, self.task.id)
+        run_pattern_task(self.pattern.id, self.task.id)
         self.task.refresh_from_db()
         self.assertEqual(self.task.status, "Failed")
         self.assertIn("Download failed", self.task.details.get("error", ""))
 
-    @patch(
-        "core.services.update_task_status",
-        wraps=pattern_task.__globals__["update_task_status"],
-    )
-    @patch("core.services.download_collection")
+    @patch("core.models.Task.update_task_status", autospec=True)
+    @patch("core.tasks.download_collection")
     def test_full_status_update_flow(self, mock_download, mock_update_status):
         temp_dir_path = self.create_temp_collection_dir()
         mock_download.return_value.__enter__.return_value = temp_dir_path
 
-        # Run the task
-        pattern_task(self.pattern.id, self.task.id)
+        def _side_effect(self_task, status_, details):
+            self_task.status = status_
+            self_task.details = details
+            self_task.save()
+
+        mock_update_status.side_effect = _side_effect
+
+        run_pattern_task(self.pattern.id, self.task.id)
 
-        # Verify calls to update_task_status
         expected_calls = [
             (self.task, "Running", {"info": "Processing pattern"}),
             (self.task, "Completed", {"info": "Pattern processed successfully"}),
         ]
-        actual_calls = [tuple(call.args) for call in mock_update_status.call_args_list]
+
+        actual_calls = [
+            (call_args[0][0], call_args[0][1], call_args[0][2])
+            for call_args in mock_update_status.call_args_list
+        ]
+
+        # Assert update_task_status calls
         for expected in expected_calls:
             self.assertIn(expected, actual_calls)
 
-        # Verify final DB state
+        # Assert final DB state
         self.task.refresh_from_db()
         self.assertEqual(self.task.status, "Completed")
         self.assertEqual(
             self.task.details.get("info"), "Pattern processed successfully"
         )
 
-        # Verify pattern_definition was updated and saved
+        # Assert pattern definition was updated
         self.pattern.refresh_from_db()
         self.assertEqual(self.pattern.pattern_definition, {"mock_key": "mock_value"})

core/tests/test_services.py renamed to core/tests/test_tasks.py

@@ -1,9 +1,3 @@
-import json
-import os
-import shutil
-import tempfile
-from unittest.mock import patch
-
 from django.urls import reverse
 from rest_framework import status
 from rest_framework.test import APITestCase
@@ -53,20 +47,6 @@ def setUpTestData(cls):
 
 
 class PatternViewSetTest(SharedDataMixin, APITestCase):
-    def create_temp_collection_dir(self):
-        temp_dir = tempfile.mkdtemp()
-        os.makedirs(
-            os.path.join(temp_dir, "extensions", "patterns", "new_pattern", "meta"),
-            exist_ok=True,
-        )
-        pattern_json_path = os.path.join(
-            temp_dir, "extensions", "patterns", "new_pattern", "meta", "pattern.json"
-        )
-        with open(pattern_json_path, "w") as f:
-            json.dump({"mock_key": "mock_value"}, f)
-        self.addCleanup(lambda: shutil.rmtree(temp_dir, ignore_errors=True))
-        return temp_dir
-
     def test_pattern_list_view(self):
         url = reverse("pattern-list")
         response = self.client.get(url)
@@ -80,11 +60,7 @@ def test_pattern_detail_view(self):
         self.assertEqual(response.status_code, status.HTTP_200_OK)
         self.assertEqual(response.data["collection_name"], "mynamespace.mycollection")
 
-    @patch("core.services.download_collection")
-    def test_pattern_create_view(self, mock_download_collection):
-        temp_dir = self.create_temp_collection_dir()  # Simulate a valid pattern.json
-        mock_download_collection.return_value.__enter__.return_value = temp_dir
-
+    def test_pattern_create_view(self):
         url = reverse("pattern-list")
         data = {
             "collection_name": "new.namespace.collection",
@@ -106,8 +82,9 @@ def test_pattern_create_view(self, mock_download_collection):
 
         # Task exists
         task = Task.objects.get(id=task_id)
-        self.assertEqual(task.status, "Completed")
-        self.assertEqual(task.details.get("info"), "Pattern processed successfully")
+        self.assertEqual(task.status, "Initiated")
+        self.assertEqual(task.details.get("model"), "Pattern")
+        self.assertEqual(task.details.get("id"), pattern.id)
 
     def test_pattern_delete_view(self):
         # Create a separate pattern for deletion