From 99f1d7c91387faf336a84a7a222c8a9a9fbb1425 Mon Sep 17 00:00:00 2001
From: "google-labs-jules[bot]"
 <161369871+google-labs-jules[bot]@users.noreply.github.com>
Date: Thu, 4 Jun 2026 07:00:45 +0000
Subject: [PATCH] [FIX] N+1 Query in import_configs

This commit addresses a performance issue in the restore_config endpoint
where each configuration in the uploaded JSON was triggering a separate
database query.

Changes:
- Implemented pre-fetching of all WebhookConfig objects using an IN query.
- Use a dictionary mapping for O(1) lookups during the restore process.
- Added a Redis mock to tests/conftest.py to ensure tests run reliably in environments without a live Redis server.
- Added tests/test_restore_config.py to verify the functionality and prevent regressions.

Co-authored-by: arumes31 <114224498+arumes31@users.noreply.github.com>
---
 hookwise/api.py              |  9 ++++-
 tests/conftest.py            | 17 ++++++++-
 tests/test_restore_config.py | 72 ++++++++++++++++++++++++++++++++++++
 3 files changed, 96 insertions(+), 2 deletions(-)
 create mode 100644 tests/test_restore_config.py

diff --git a/hookwise/api.py b/hookwise/api.py
index c9f3177..facfdcd 100644
--- a/hookwise/api.py
+++ b/hookwise/api.py
@@ -920,11 +920,18 @@ def restore_config() -> Any:
             return jsonify({"status": "error", "message": "No file"}), 400
         try:
             data = json.load(file)
+            # Pre-fetch all configs in data to avoid N+1 queries
+            config_ids = [c["id"] for c in data if "id" in c]
+            existing_configs = {
+                cfg.id: cfg for cfg in WebhookConfig.query.filter(WebhookConfig.id.in_(config_ids)).all()
+            }
+
             for c in data:
-                config = WebhookConfig.query.get(c["id"])
+                config = existing_configs.get(c["id"])
                 if not config:
                     config = WebhookConfig(id=c["id"])
                     db.session.add(config)
+                    existing_configs[c["id"]] = config
                 fields = [
                     "name",
                     "customer_id_default",
diff --git a/tests/conftest.py b/tests/conftest.py
index 5399d8a..16d638e 100644
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -1,4 +1,5 @@
 import os
+from unittest.mock import MagicMock, patch
 
 os.environ["SOCKETIO_ASYNC_MODE"] = "threading"
 os.environ["SECRET_KEY"] = "test-secret"
@@ -9,8 +10,22 @@
 
 import pytest
 
-
 @pytest.fixture(autouse=True, scope="session")
 def setup_test_env():
     # Already set at top level, but kept for clarity
     pass
+
+@pytest.fixture(autouse=True)
+def mock_redis():
+    with patch("hookwise.extensions.redis_client") as mock_ext_redis,          patch("hookwise.tasks.redis_client") as mock_tasks_redis,          patch("hookwise.api.redis_client") as mock_api_redis:
+
+        # Make redis_client.get return None by default to avoid maintenance mode
+        mock_ext_redis.get.return_value = None
+        mock_tasks_redis.get.return_value = None
+        mock_api_redis.get.return_value = None
+
+        yield {
+            "ext": mock_ext_redis,
+            "tasks": mock_tasks_redis,
+            "api": mock_api_redis
+        }
diff --git a/tests/test_restore_config.py b/tests/test_restore_config.py
new file mode 100644
index 0000000..2cd3d16
--- /dev/null
+++ b/tests/test_restore_config.py
@@ -0,0 +1,72 @@
+import json
+import pytest
+from io import BytesIO
+from hookwise.extensions import db
+from hookwise.models import WebhookConfig
+from unittest.mock import patch
+
+@pytest.fixture
+def app():
+    from hookwise import create_app
+    app = create_app()
+    app.config["TESTING"] = True
+    app.config["WTF_CSRF_ENABLED"] = False
+    app.config["SQLALCHEMY_DATABASE_URI"] = "sqlite:///:memory:"
+    # Ensure we have a secret key for session
+    app.config["SECRET_KEY"] = "test-secret"
+    return app
+
+@pytest.fixture
+def client(app):
+    with app.app_context():
+        db.create_all()
+        yield app.test_client()
+        db.session.remove()
+        db.drop_all()
+
+def test_restore_config_functionality(client, app):
+    with app.app_context():
+        # Create some existing configs with string IDs
+        id1 = "config-1-id"
+        id2 = "config-2-id"
+        id3 = "config-3-id"
+
+        c1 = WebhookConfig(id=id1, name="Config 1", board="Board 1")
+        c2 = WebhookConfig(id=id2, name="Config 2", board="Board 2")
+        db.session.add_all([c1, c2])
+        db.session.commit()
+
+        # Data to restore: 1 update, 1 same, 1 new
+        restore_data = [
+            {"id": id1, "name": "Config 1 Updated", "board": "Board 1 New"},
+            {"id": id2, "name": "Config 2", "board": "Board 2"},
+            {"id": id3, "name": "New Config 3", "board": "Board 3"}
+        ]
+
+        data = BytesIO(json.dumps(restore_data).encode("utf-8"))
+
+        # Bypass auth
+        with client.session_transaction() as sess:
+            sess["user_id"] = "admin"
+
+        response = client.post(
+            "/admin/restore",
+            data={"backup_file": (data, "backup.json")},
+            content_type="multipart/form-data"
+        )
+
+        assert response.status_code == 200
+        assert response.json["status"] == "success"
+
+        # Verify updates in DB
+        updated_c1 = WebhookConfig.query.get(id1)
+        assert updated_c1.name == "Config 1 Updated"
+        assert updated_c1.board == "Board 1 New"
+
+        updated_c2 = WebhookConfig.query.get(id2)
+        assert updated_c2.name == "Config 2"
+
+        new_c3 = WebhookConfig.query.get(id3)
+        assert new_c3 is not None
+        assert new_c3.name == "New Config 3"
+        assert new_c3.board == "Board 3"