From 7732dd19e04e5d955b82e18ca95c3506de2e7f10 Mon Sep 17 00:00:00 2001 From: nishika26 Date: Tue, 1 Jul 2025 16:15:48 +0530 Subject: [PATCH 01/21] configure openai creds --- backend/app/api/routes/collections.py | 27 ++++++++++++++++++++++----- backend/app/crud/__init__.py | 1 + backend/app/crud/rag/open_ai.py | 2 +- 3 files changed, 24 insertions(+), 6 deletions(-) diff --git a/backend/app/api/routes/collections.py b/backend/app/api/routes/collections.py index 68e4b6a87..f709710b4 100644 --- a/backend/app/api/routes/collections.py +++ b/backend/app/api/routes/collections.py @@ -15,8 +15,13 @@ from app.api.deps import CurrentUser, SessionDep, CurrentUserOrgProject from app.core.cloud import AmazonCloudStorage from app.core.config import settings -from app.core.util import now, raise_from_unknown, post_callback -from app.crud import DocumentCrud, CollectionCrud, DocumentCollectionCrud +from app.core.util import now, raise_from_unknown, post_callback, configure_openai +from app.crud import ( + DocumentCrud, + CollectionCrud, + DocumentCollectionCrud, + get_provider_credential, +) from app.crud.rag import OpenAIVectorStoreCrud, OpenAIAssistantCrud from app.models import Collection, Document from app.models.collection import CollectionStatus @@ -180,12 +185,24 @@ def _backout(crud: OpenAIAssistantCrud, assistant_id: str): def do_create_collection( session: SessionDep, - current_user: CurrentUser, + current_user: CurrentUserOrgProject, request: CreationRequest, payload: ResponsePayload, ): start_time = time.time() - client = OpenAI(api_key=settings.OPENAI_API_KEY) + + credentials = get_provider_credential( + session=session, + org_id=current_user.organization_id, + provider="openai", + project_id=current_user.project_id, + ) + client, success = configure_openai(credentials) + if not success: + return APIResponse.failure_response( + error="OpenAI API key not configured for this organization." + ) + callback = ( SilentCallback(payload) if request.callback_url is None @@ -226,7 +243,7 @@ def do_create_collection( collection_crud._update(collection) elapsed = time.time() - start_time - logging.info( + logger.info( f"[do_create_collection] Collection created: {collection.id} | Time: {elapsed:.2f}s | " f"Files: {len(flat_docs)} | Sizes: {file_sizes_kb} KB | Types: {list(file_exts)}" ) diff --git a/backend/app/crud/__init__.py b/backend/app/crud/__init__.py index 6bf4d5da5..bb4dc484c 100644 --- a/backend/app/crud/__init__.py +++ b/backend/app/crud/__init__.py @@ -37,6 +37,7 @@ get_key_by_org, update_creds_for_org, remove_creds_for_org, + get_provider_credential, ) from .thread_results import upsert_thread_result, get_thread_result diff --git a/backend/app/crud/rag/open_ai.py b/backend/app/crud/rag/open_ai.py index 69479888f..beeaf73bb 100644 --- a/backend/app/crud/rag/open_ai.py +++ b/backend/app/crud/rag/open_ai.py @@ -91,7 +91,7 @@ def clean(self, resource): class OpenAICrud: def __init__(self, client=None): - self.client = client or OpenAI(api_key=settings.OPENAI_API_KEY) + self.client = client class OpenAIVectorStoreCrud(OpenAICrud): From 613b7ccb21f5ad95e8cbea6a3a1442d7c7591a3c Mon Sep 17 00:00:00 2001 From: nishika26 Date: Tue, 1 Jul 2025 16:50:22 +0530 Subject: [PATCH 02/21] test cases --- .../collections/test_create_collections.py | 25 +++++++---- .../tests/utils/collections_openai_mock.py | 43 +++++++++++++++++++ 2 files changed, 60 insertions(+), 8 deletions(-) create mode 100644 backend/app/tests/utils/collections_openai_mock.py diff --git a/backend/app/tests/api/routes/collections/test_create_collections.py b/backend/app/tests/api/routes/collections/test_create_collections.py index 6fb0855c2..aa3230327 100644 --- a/backend/app/tests/api/routes/collections/test_create_collections.py +++ b/backend/app/tests/api/routes/collections/test_create_collections.py @@ -3,18 +3,18 @@ import io import openai_responses -from sqlmodel import Session, select +from sqlmodel import Session from fastapi.testclient import TestClient -from openai import OpenAIError +from unittest.mock import MagicMock, patch from app.core.config import settings from app.tests.utils.document import DocumentStore -from app.tests.utils.utils import openai_credentials, get_user_from_api_key +from app.tests.utils.utils import get_user_from_api_key from app.main import app from app.crud.collection import CollectionCrud -from app.api.routes.collections import CreationRequest, ResponsePayload from app.seed_data.seed_data import seed_database from app.models.collection import CollectionStatus +from app.tests.utils.collections_openai_mock import get_mock_openai_client client = TestClient(app) @@ -51,16 +51,20 @@ def head_object(self, Bucket, Key): monkeypatch.setattr("boto3.client", lambda service: FakeS3Client()) -@pytest.mark.usefixtures("openai_credentials") +# @pytest.mark.usefixtures("openai_credentials") +@patch("app.api.routes.collections.configure_openai") +@patch("app.api.routes.collections.get_provider_credential") class TestCollectionRouteCreate: _n_documents = 5 - @openai_responses.mock() def test_create_collection_success( self, + mock_get_credential, + mock_configure_openai, client: TestClient, db: Session, ): + # Setup test documents store = DocumentStore(db) documents = store.fill(self._n_documents) doc_ids = [str(doc.id) for doc in documents] @@ -73,9 +77,14 @@ def test_create_collection_success( "temperature": 0.1, } original_api_key = "ApiKey No3x47A5qoIGhm0kVKjQ77dhCqEdWRIQZlEPzzzh7i8" - headers = {"X-API-KEY": original_api_key} + # Mock API key credentials + mock_get_credential.return_value = {"api_key": "test_api_key"} + + mock_openai_client = get_mock_openai_client() + mock_configure_openai.return_value = (mock_openai_client, True) + response = client.post( f"{settings.API_V1_STR}/collections/create", json=body, @@ -89,8 +98,8 @@ def test_create_collection_success( assert metadata["status"] == CollectionStatus.processing.value assert UUID(metadata["key"]) + # Confirm collection metadata in DB collection_id = UUID(metadata["key"]) - user = get_user_from_api_key(db, headers) collection = CollectionCrud(db, user.user_id).read_one(collection_id) diff --git a/backend/app/tests/utils/collections_openai_mock.py b/backend/app/tests/utils/collections_openai_mock.py new file mode 100644 index 000000000..7fba6bcba --- /dev/null +++ b/backend/app/tests/utils/collections_openai_mock.py @@ -0,0 +1,43 @@ +from unittest.mock import MagicMock + + +def get_mock_openai_client(): + mock_client = MagicMock() + + # Vector store + mock_vector_store = MagicMock() + mock_vector_store.id = "mock_vector_store_id" + mock_client.vector_stores.create.return_value = mock_vector_store + + # File upload + polling + mock_file_batch = MagicMock() + mock_file_batch.file_counts.completed = 2 + mock_file_batch.file_counts.total = 2 + mock_client.vector_stores.file_batches.upload_and_poll.return_value = ( + mock_file_batch + ) + + # File list + mock_client.vector_stores.files.list.return_value = {"data": []} + + # Assistant + mock_assistant = MagicMock() + mock_assistant.id = "mock_assistant_id" + mock_assistant.name = "Mock Assistant" + mock_assistant.model = "gpt-4o" + mock_assistant.instructions = "Mock instructions" + mock_client.beta.assistants.create.return_value = mock_assistant + + # Thread + mock_thread = MagicMock() + mock_thread.id = "mock_thread_id" + mock_client.beta.threads.create.return_value = mock_thread + + # Run + mock_run = MagicMock() + mock_run.id = "mock_run_id" + mock_run.status = "completed" + mock_client.beta.threads.runs.create.return_value = mock_run + mock_client.beta.threads.runs.retrieve.return_value = mock_run + + return mock_client From 49f428a96183371b83f3f3aaffcf282865911aaa Mon Sep 17 00:00:00 2001 From: nishika26 Date: Tue, 1 Jul 2025 16:55:27 +0530 Subject: [PATCH 03/21] cleaning up a little --- .../routes/collections/test_create_collections.py | 2 -- backend/app/tests/utils/collections_openai_mock.py | 12 ------------ 2 files changed, 14 deletions(-) diff --git a/backend/app/tests/api/routes/collections/test_create_collections.py b/backend/app/tests/api/routes/collections/test_create_collections.py index aa3230327..400d98275 100644 --- a/backend/app/tests/api/routes/collections/test_create_collections.py +++ b/backend/app/tests/api/routes/collections/test_create_collections.py @@ -51,7 +51,6 @@ def head_object(self, Bucket, Key): monkeypatch.setattr("boto3.client", lambda service: FakeS3Client()) -# @pytest.mark.usefixtures("openai_credentials") @patch("app.api.routes.collections.configure_openai") @patch("app.api.routes.collections.get_provider_credential") class TestCollectionRouteCreate: @@ -79,7 +78,6 @@ def test_create_collection_success( original_api_key = "ApiKey No3x47A5qoIGhm0kVKjQ77dhCqEdWRIQZlEPzzzh7i8" headers = {"X-API-KEY": original_api_key} - # Mock API key credentials mock_get_credential.return_value = {"api_key": "test_api_key"} mock_openai_client = get_mock_openai_client() diff --git a/backend/app/tests/utils/collections_openai_mock.py b/backend/app/tests/utils/collections_openai_mock.py index 7fba6bcba..7f9c064b2 100644 --- a/backend/app/tests/utils/collections_openai_mock.py +++ b/backend/app/tests/utils/collections_openai_mock.py @@ -28,16 +28,4 @@ def get_mock_openai_client(): mock_assistant.instructions = "Mock instructions" mock_client.beta.assistants.create.return_value = mock_assistant - # Thread - mock_thread = MagicMock() - mock_thread.id = "mock_thread_id" - mock_client.beta.threads.create.return_value = mock_thread - - # Run - mock_run = MagicMock() - mock_run.id = "mock_run_id" - mock_run.status = "completed" - mock_client.beta.threads.runs.create.return_value = mock_run - mock_client.beta.threads.runs.retrieve.return_value = mock_run - return mock_client From f64fa46fa2f6c6a9fc59db244a8811c9d91be203 Mon Sep 17 00:00:00 2001 From: nishika26 Date: Tue, 1 Jul 2025 17:12:18 +0530 Subject: [PATCH 04/21] additional test cases changes --- .../tests/crud/collections/test_crud_collection_delete.py | 8 ++++---- .../crud/collections/test_crud_collection_read_all.py | 2 +- .../crud/collections/test_crud_collection_read_one.py | 2 +- backend/app/tests/utils/collection.py | 2 +- backend/app/tests/utils/utils.py | 2 +- 5 files changed, 8 insertions(+), 8 deletions(-) diff --git a/backend/app/tests/crud/collections/test_crud_collection_delete.py b/backend/app/tests/crud/collections/test_crud_collection_delete.py index 58d552d11..c4d7c8bb2 100644 --- a/backend/app/tests/crud/collections/test_crud_collection_delete.py +++ b/backend/app/tests/crud/collections/test_crud_collection_delete.py @@ -17,7 +17,7 @@ class TestCollectionDelete: @openai_responses.mock() def test_delete_marks_deleted(self, db: Session): - client = OpenAI(api_key=settings.OPENAI_API_KEY) + client = OpenAI(api_key="test_api_key") assistant = OpenAIAssistantCrud(client) collection = get_collection(db, client) @@ -29,7 +29,7 @@ def test_delete_marks_deleted(self, db: Session): @openai_responses.mock() def test_delete_follows_insert(self, db: Session): - client = OpenAI(api_key=settings.OPENAI_API_KEY) + client = OpenAI(api_key="test_api_key") assistant = OpenAIAssistantCrud(client) collection = get_collection(db, client) @@ -41,7 +41,7 @@ def test_delete_follows_insert(self, db: Session): @openai_responses.mock() def test_cannot_delete_others_collections(self, db: Session): - client = OpenAI(api_key=settings.OPENAI_API_KEY) + client = OpenAI(api_key="test_api_key") assistant = OpenAIAssistantCrud(client) collection = get_collection(db, client) @@ -56,7 +56,7 @@ def test_delete_document_deletes_collections(self, db: Session): store = DocumentStore(db) documents = store.fill(1) - client = OpenAI(api_key=settings.OPENAI_API_KEY) + client = OpenAI(api_key="test_api_key") resources = [] for _ in range(self._n_collections): coll = get_collection(db, client) diff --git a/backend/app/tests/crud/collections/test_crud_collection_read_all.py b/backend/app/tests/crud/collections/test_crud_collection_read_all.py index 39eb36e99..d140c4e7e 100644 --- a/backend/app/tests/crud/collections/test_crud_collection_read_all.py +++ b/backend/app/tests/crud/collections/test_crud_collection_read_all.py @@ -18,7 +18,7 @@ def create_collections(db: Session, n: int): openai_mock = OpenAIMock() with openai_mock.router: - client = OpenAI(api_key=settings.OPENAI_API_KEY) + client = OpenAI(api_key="test_api_key") for _ in range(n): collection = get_collection(db, client) if crud is None: diff --git a/backend/app/tests/crud/collections/test_crud_collection_read_one.py b/backend/app/tests/crud/collections/test_crud_collection_read_one.py index 22edba532..909d1a0c7 100644 --- a/backend/app/tests/crud/collections/test_crud_collection_read_one.py +++ b/backend/app/tests/crud/collections/test_crud_collection_read_one.py @@ -17,7 +17,7 @@ def mk_collection(db: Session): openai_mock = OpenAIMock() with openai_mock.router: - client = OpenAI(api_key=settings.OPENAI_API_KEY) + client = OpenAI(api_key="Test_api_key") collection = get_collection(db, client) crud = CollectionCrud(db, collection.owner_id) return crud.create(collection, documents) diff --git a/backend/app/tests/utils/collection.py b/backend/app/tests/utils/collection.py index ded5c728d..e8674e020 100644 --- a/backend/app/tests/utils/collection.py +++ b/backend/app/tests/utils/collection.py @@ -40,7 +40,7 @@ def get_collection(db: Session, client=None): ) if client is None: - client = OpenAI(api_key=settings.OPENAI_API_KEY) + client = OpenAI(api_key="test_api_key") vector_store = client.vector_stores.create() assistant = client.beta.assistants.create( diff --git a/backend/app/tests/utils/utils.py b/backend/app/tests/utils/utils.py index 9cba7aaee..2c7114bea 100644 --- a/backend/app/tests/utils/utils.py +++ b/backend/app/tests/utils/utils.py @@ -19,7 +19,7 @@ @pytest.fixture(scope="class") def openai_credentials(): - settings.OPENAI_API_KEY = "sk-fake123" + OPENAI_API_KEY = "sk-fake123" def random_lower_string() -> str: From d8d67dcc02270a0d78397613c539b045ffc5d030 Mon Sep 17 00:00:00 2001 From: nishika26 Date: Wed, 2 Jul 2025 12:14:43 +0530 Subject: [PATCH 05/21] big changes .1 --- backend/app/api/routes/collections.py | 23 ++++++++++-- backend/app/api/routes/documents.py | 35 +++++++++++++++---- backend/app/crud/rag/open_ai.py | 4 ++- .../collections/test_create_collections.py | 3 +- .../documents/test_route_document_info.py | 13 ++++--- 5 files changed, 60 insertions(+), 18 deletions(-) diff --git a/backend/app/api/routes/collections.py b/backend/app/api/routes/collections.py index f709710b4..00abdf3d3 100644 --- a/backend/app/api/routes/collections.py +++ b/backend/app/api/routes/collections.py @@ -183,6 +183,18 @@ def _backout(crud: OpenAIAssistantCrud, assistant_id: str): ) +def mark_collection_failed(session, user_id, collection_id, reason: str): + try: + collection = CollectionCrud(session, user_id).read_one(collection_id) + collection.status = CollectionStatus.failed + collection.updated_at = now() + CollectionCrud(session, user_id)._update(collection) + except Exception as suberr: + logger.warning( + f"[do_create_collection] Failed to mark collection failed | {{'collection_id': '{collection_id}', 'reason': '{str(suberr)}'}}" + ) + + def do_create_collection( session: SessionDep, current_user: CurrentUserOrgProject, @@ -199,8 +211,15 @@ def do_create_collection( ) client, success = configure_openai(credentials) if not success: - return APIResponse.failure_response( - error="OpenAI API key not configured for this organization." + logger.error( + f"OpenAI API key not configured for org_id={current_user.organization_id}, project_id={current_user.project_id}" + ) + + mark_collection_failed( + session=session, + user_id=current_user.id, + collection_id=UUID(payload.key), + reason="OpenAI config failed", ) callback = ( diff --git a/backend/app/api/routes/documents.py b/backend/app/api/routes/documents.py index 03d2cd98e..c3f85ffc9 100644 --- a/backend/app/api/routes/documents.py +++ b/backend/app/api/routes/documents.py @@ -3,13 +3,14 @@ from typing import List from pathlib import Path -from fastapi import APIRouter, File, UploadFile, Query +from fastapi import APIRouter, File, UploadFile, Query, HTTPException from fastapi import Path as FastPath -from app.crud import DocumentCrud, CollectionCrud +from app.crud import DocumentCrud, CollectionCrud, get_provider_credential from app.models import Document +from app.core.util import configure_openai from app.utils import APIResponse, load_description -from app.api.deps import CurrentUser, SessionDep +from app.api.deps import CurrentUser, SessionDep, CurrentUserOrgProject from app.core.cloud import AmazonCloudStorage from app.crud.rag import OpenAIAssistantCrud @@ -65,10 +66,20 @@ def upload_doc( ) def remove_doc( session: SessionDep, - current_user: CurrentUser, + current_user: CurrentUserOrgProject, doc_id: UUID = FastPath(description="Document to delete"), ): - a_crud = OpenAIAssistantCrud() + credentials = get_provider_credential( + session=session, + org_id=current_user.organization_id, + provider="openai", + project_id=current_user.project_id, + ) + client, success = configure_openai(credentials) + if not success: + raise HTTPException(status_code=400, detail="OpenAI is not configured") + + a_crud = OpenAIAssistantCrud(client) d_crud = DocumentCrud(session, current_user.id) c_crud = CollectionCrud(session, current_user.id) @@ -84,10 +95,20 @@ def remove_doc( ) def permanent_delete_doc( session: SessionDep, - current_user: CurrentUser, + current_user: CurrentUserOrgProject, doc_id: UUID = FastPath(description="Document to permanently delete"), ): - a_crud = OpenAIAssistantCrud() + credentials = get_provider_credential( + session=session, + org_id=current_user.organization_id, + provider="openai", + project_id=current_user.project_id, + ) + client, success = configure_openai(credentials) + if not success: + raise HTTPException(status_code=400, detail="OpenAI is not configured") + + a_crud = OpenAIAssistantCrud(client) d_crud = DocumentCrud(session, current_user.id) c_crud = CollectionCrud(session, current_user.id) storage = AmazonCloudStorage(current_user) diff --git a/backend/app/crud/rag/open_ai.py b/backend/app/crud/rag/open_ai.py index beeaf73bb..21ff61ab9 100644 --- a/backend/app/crud/rag/open_ai.py +++ b/backend/app/crud/rag/open_ai.py @@ -90,7 +90,9 @@ def clean(self, resource): class OpenAICrud: - def __init__(self, client=None): + def __init__(self, client): + if client is None: + raise ValueError("OpenAI client is not configured") self.client = client diff --git a/backend/app/tests/api/routes/collections/test_create_collections.py b/backend/app/tests/api/routes/collections/test_create_collections.py index 400d98275..c1be75390 100644 --- a/backend/app/tests/api/routes/collections/test_create_collections.py +++ b/backend/app/tests/api/routes/collections/test_create_collections.py @@ -62,9 +62,10 @@ def test_create_collection_success( mock_configure_openai, client: TestClient, db: Session, + api_key_headers: dict[str, str], ): # Setup test documents - store = DocumentStore(db) + store = DocumentStore(db, api_key_headers) documents = store.fill(self._n_documents) doc_ids = [str(doc.id) for doc in documents] diff --git a/backend/app/tests/api/routes/documents/test_route_document_info.py b/backend/app/tests/api/routes/documents/test_route_document_info.py index 3f077935a..0d668f915 100644 --- a/backend/app/tests/api/routes/documents/test_route_document_info.py +++ b/backend/app/tests/api/routes/documents/test_route_document_info.py @@ -23,8 +23,9 @@ def test_response_is_success( db: Session, route: Route, crawler: WebCrawler, + api_key_headers: dict[str, str], ): - store = DocumentStore(db) + store = DocumentStore(db, api_key_headers) response = crawler.get(route.append(store.put())) assert response.is_success @@ -34,8 +35,9 @@ def test_info_reflects_database( db: Session, route: Route, crawler: WebCrawler, + api_key_headers: dict[str, str], ): - store = DocumentStore(db) + store = DocumentStore(db, api_key_headers) document = store.put() source = DocumentComparator(document) @@ -44,13 +46,10 @@ def test_info_reflects_database( assert source == target.data def test_cannot_info_unknown_document( - self, - db: Session, - route: Route, - crawler: Route, + self, db: Session, route: Route, crawler: Route, api_key_headers: dict[str, str] ): DocumentStore.clear(db) - maker = DocumentMaker(db) + maker = DocumentMaker(db, api_key_headers) response = crawler.get(route.append(next(maker))) assert response.is_error From 487166f32fa41fa51155403e354766ba6638af4f Mon Sep 17 00:00:00 2001 From: nishika26 Date: Wed, 2 Jul 2025 12:16:54 +0530 Subject: [PATCH 06/21] big change .2 --- .../documents/test_route_document_list.py | 6 +- .../test_route_document_permanent_remove.py | 35 +++++++-- .../documents/test_route_document_remove.py | 71 +++++++++++++++---- .../documents/test_route_document_upload.py | 6 +- backend/app/tests/conftest.py | 5 ++ .../test_crud_collection_create.py | 14 +++- .../test_crud_collection_delete.py | 14 +++- .../test_crud_collection_read_all.py | 23 ++++-- .../test_crud_collection_read_one.py | 22 ++++-- .../documents/test_crud_document_delete.py | 18 +++-- .../documents/test_crud_document_read_many.py | 12 +++- .../documents/test_crud_document_read_one.py | 18 +++-- .../documents/test_crud_document_update.py | 12 +++- backend/app/tests/utils/document.py | 25 ++++--- 14 files changed, 215 insertions(+), 66 deletions(-) diff --git a/backend/app/tests/api/routes/documents/test_route_document_list.py b/backend/app/tests/api/routes/documents/test_route_document_list.py index b488b9ae1..ed2d26166 100644 --- a/backend/app/tests/api/routes/documents/test_route_document_list.py +++ b/backend/app/tests/api/routes/documents/test_route_document_list.py @@ -47,8 +47,9 @@ def test_item_reflects_database( db: Session, route: QueryRoute, crawler: WebCrawler, + api_key_headers: dict[str, str], ): - store = DocumentStore(db) + store = DocumentStore(db, api_key_headers) source = DocumentComparator(store.put()) response = httpx_to_standard(crawler.get(route)) @@ -77,8 +78,9 @@ def test_skip_greater_than_limit_is_difference( db: Session, route: QueryRoute, crawler: WebCrawler, + api_key_headers: dict[str, str], ): - store = DocumentStore(db) + store = DocumentStore(db, api_key_headers) limit = len(store.fill(self._ndocs)) skip = limit // 2 diff --git a/backend/app/tests/api/routes/documents/test_route_document_permanent_remove.py b/backend/app/tests/api/routes/documents/test_route_document_permanent_remove.py index 8a6d353fe..5036223da 100644 --- a/backend/app/tests/api/routes/documents/test_route_document_permanent_remove.py +++ b/backend/app/tests/api/routes/documents/test_route_document_permanent_remove.py @@ -3,11 +3,14 @@ from urllib.parse import urlparse import pytest +from unittest.mock import patch from botocore.exceptions import ClientError from moto import mock_aws from sqlmodel import Session, select +from openai import OpenAI import openai_responses +from openai_responses import OpenAIMock from app.core.cloud import AmazonCloudStorageClient from app.core.config import settings @@ -20,6 +23,7 @@ crawler, ) from app.tests.utils.utils import openai_credentials +from app.seed_data.seed_data import seed_database @pytest.fixture @@ -27,6 +31,13 @@ def route(): return Route("remove") +@pytest.fixture(scope="function", autouse=True) +def load_seed_data(db): + """Load seed data before each test.""" + seed_database(db) + yield + + @pytest.fixture(scope="class") def aws_credentials(): os.environ["AWS_ACCESS_KEY_ID"] = "testing" @@ -36,22 +47,32 @@ def aws_credentials(): os.environ["AWS_DEFAULT_REGION"] = settings.AWS_DEFAULT_REGION -@pytest.mark.usefixtures("openai_credentials", "aws_credentials") -@mock_aws -class TestDocumentRoutePermanentRemove: +@pytest.mark.usefixtures("openai_credentials") +class TestDocumentRouteRemove: @openai_responses.mock() - def test_permanent_delete_document_from_s3( + @patch("app.api.routes.documents.get_provider_credential") + @patch("app.api.routes.documents.configure_openai") + def test_item_is_soft_removed( self, + mock_configure_openai, + mock_get_credential, db: Session, route: Route, crawler: WebCrawler, + api_key_headers: dict[str, str], ): + openai_mock = OpenAIMock() + with openai_mock.router: + client = OpenAI(api_key="test_key") + mock_get_credential.return_value = {"api_key": "sk-test-key"} + mock_configure_openai.return_value = (client, True) + # Setup AWS aws = AmazonCloudStorageClient() aws.create() # Setup document in DB and S3 - store = DocumentStore(db) + store = DocumentStore(db, api_key_headers) document = store.put() s3_key = Path(urlparse(document.object_store_url).path).relative_to("/") aws.client.put_object( @@ -60,6 +81,7 @@ def test_permanent_delete_document_from_s3( # Delete document response = crawler.delete(route.append(document, suffix="permanent")) + print(response) assert response.is_success db.refresh(document) @@ -82,10 +104,11 @@ def test_cannot_delete_nonexistent_document( db: Session, route: Route, crawler: WebCrawler, + api_key_headers: dict[str, str], ): DocumentStore.clear(db) - maker = DocumentMaker(db) + maker = DocumentMaker(db, api_key_headers) response = crawler.delete(route.append(next(maker), suffix="permanent")) assert response.is_error diff --git a/backend/app/tests/api/routes/documents/test_route_document_remove.py b/backend/app/tests/api/routes/documents/test_route_document_remove.py index 7b01cc2f4..b6f3c1c8c 100644 --- a/backend/app/tests/api/routes/documents/test_route_document_remove.py +++ b/backend/app/tests/api/routes/documents/test_route_document_remove.py @@ -1,6 +1,9 @@ import pytest import openai_responses +from openai_responses import OpenAIMock +from openai import OpenAI from sqlmodel import Session, select +from unittest.mock import patch from app.models import Document from app.tests.utils.document import ( @@ -12,6 +15,14 @@ ) from app.tests.utils.collection import get_collection from app.tests.utils.utils import openai_credentials +from app.seed_data.seed_data import seed_database + + +@pytest.fixture(scope="function", autouse=True) +def load_seed_data(db): + """Load seed data before each test.""" + seed_database(db) + yield @pytest.fixture @@ -22,44 +33,76 @@ def route(): @pytest.mark.usefixtures("openai_credentials") class TestDocumentRouteRemove: @openai_responses.mock() + @patch("app.api.routes.documents.get_provider_credential") + @patch("app.api.routes.documents.configure_openai") def test_response_is_success( self, + mock_configure_openai, + mock_get_credential, db: Session, route: Route, crawler: WebCrawler, + api_key_headers: dict[str, str], ): - store = DocumentStore(db) - response = crawler.get(route.append(store.put())) + openai_mock = OpenAIMock() + with openai_mock.router: + client = OpenAI(api_key="test_key") + mock_get_credential.return_value = {"api_key": "sk-test-key"} + mock_configure_openai.return_value = (client, True) - assert response.is_success + store = DocumentStore(db, api_key_headers) + response = crawler.get(route.append(store.put())) + + assert response.is_success @openai_responses.mock() + @patch("app.api.routes.documents.get_provider_credential") + @patch("app.api.routes.documents.configure_openai") def test_item_is_soft_removed( self, + mock_configure_openai, + mock_get_credential, db: Session, route: Route, crawler: WebCrawler, + api_key_headers: dict[str, str], ): - store = DocumentStore(db) - document = store.put() + openai_mock = OpenAIMock() + with openai_mock.router: + client = OpenAI(api_key="test_key") + mock_get_credential.return_value = {"api_key": "sk-test-key"} + mock_configure_openai.return_value = (client, True) + + store = DocumentStore(db, api_key_headers) + document = store.put() - crawler.get(route.append(document)) - db.refresh(document) - statement = select(Document).where(Document.id == document.id) - result = db.exec(statement).one() + crawler.get(route.append(document)) + db.refresh(document) + statement = select(Document).where(Document.id == document.id) + result = db.exec(statement).one() - assert result.deleted_at is not None + assert result.deleted_at is not None @openai_responses.mock() + @patch("app.api.routes.documents.get_provider_credential") + @patch("app.api.routes.documents.configure_openai") def test_cannot_remove_unknown_document( self, + mock_configure_openai, + mock_get_credential, db: Session, route: Route, crawler: WebCrawler, + api_key_headers: dict[str, str], ): - DocumentStore.clear(db) + openai_mock = OpenAIMock() + with openai_mock.router: + client = OpenAI(api_key="test_key") + mock_get_credential.return_value = {"api_key": "sk-test-key"} + mock_configure_openai.return_value = (client, True) - maker = DocumentMaker(db) - response = crawler.get(route.append(next(maker))) + DocumentStore.clear(db) + maker = DocumentMaker(db, api_key_headers) + response = crawler.get(route.append(next(maker))) - assert response.is_error + assert response.is_error diff --git a/backend/app/tests/api/routes/documents/test_route_document_upload.py b/backend/app/tests/api/routes/documents/test_route_document_upload.py index aa28d23f6..af6ce6ff2 100644 --- a/backend/app/tests/api/routes/documents/test_route_document_upload.py +++ b/backend/app/tests/api/routes/documents/test_route_document_upload.py @@ -25,7 +25,7 @@ def put(self, route: Route, scratch: Path): with scratch.open("rb") as fp: return self.client.post( str(route), - headers=self.superuser_token_headers, + headers=self.api_key_headers, files={ "src": (str(scratch), fp, mtype), }, @@ -45,8 +45,8 @@ def route(): @pytest.fixture -def uploader(client: TestClient, superuser_token_headers: dict[str, str]): - return WebUploader(client, superuser_token_headers) +def uploader(client: TestClient, api_key_headers: dict[str, str]): + return WebUploader(client, api_key_headers) @pytest.fixture(scope="class") diff --git a/backend/app/tests/conftest.py b/backend/app/tests/conftest.py index e2a6464d7..d15e7fd3a 100644 --- a/backend/app/tests/conftest.py +++ b/backend/app/tests/conftest.py @@ -56,3 +56,8 @@ def normal_user_token_headers(client: TestClient, db: Session) -> dict[str, str] return authentication_token_from_email( client=client, email=settings.EMAIL_TEST_USER, db=db ) + + +@pytest.fixture +def api_key_headers(): + return {"X-API-KEY": "ApiKey No3x47A5qoIGhm0kVKjQ77dhCqEdWRIQZlEPzzzh7i8"} diff --git a/backend/app/tests/crud/collections/test_crud_collection_create.py b/backend/app/tests/crud/collections/test_crud_collection_create.py index 564230b33..f413f5641 100644 --- a/backend/app/tests/crud/collections/test_crud_collection_create.py +++ b/backend/app/tests/crud/collections/test_crud_collection_create.py @@ -7,6 +7,14 @@ from app.tests.utils.document import DocumentStore from app.tests.utils.collection import get_collection from app.tests.utils.utils import openai_credentials +from app.seed_data.seed_data import seed_database + + +@pytest.fixture(scope="function", autouse=True) +def load_seed_data(db): + """Load seed data before each test.""" + seed_database(db) + yield @pytest.mark.usefixtures("openai_credentials") @@ -14,8 +22,10 @@ class TestCollectionCreate: _n_documents = 10 @openai_responses.mock() - def test_create_associates_documents(self, db: Session): - store = DocumentStore(db) + def test_create_associates_documents( + self, db: Session, api_key_headers: dict[str, str] + ): + store = DocumentStore(db, api_key_headers) documents = store.fill(self._n_documents) collection = get_collection(db) diff --git a/backend/app/tests/crud/collections/test_crud_collection_delete.py b/backend/app/tests/crud/collections/test_crud_collection_delete.py index c4d7c8bb2..4803ecd89 100644 --- a/backend/app/tests/crud/collections/test_crud_collection_delete.py +++ b/backend/app/tests/crud/collections/test_crud_collection_delete.py @@ -9,6 +9,14 @@ from app.tests.utils.document import DocumentStore from app.tests.utils.collection import get_collection, uuid_increment from app.tests.utils.utils import openai_credentials +from app.seed_data.seed_data import seed_database + + +@pytest.fixture(scope="function", autouse=True) +def load_seed_data(db): + """Load seed data before each test.""" + seed_database(db) + yield @pytest.mark.usefixtures("openai_credentials") @@ -52,8 +60,10 @@ def test_cannot_delete_others_collections(self, db: Session): crud.delete(collection, assistant) @openai_responses.mock() - def test_delete_document_deletes_collections(self, db: Session): - store = DocumentStore(db) + def test_delete_document_deletes_collections( + self, db: Session, api_key_headers: dict[str, str] + ): + store = DocumentStore(db, api_key_headers) documents = store.fill(1) client = OpenAI(api_key="test_api_key") diff --git a/backend/app/tests/crud/collections/test_crud_collection_read_all.py b/backend/app/tests/crud/collections/test_crud_collection_read_all.py index d140c4e7e..32f3ae260 100644 --- a/backend/app/tests/crud/collections/test_crud_collection_read_all.py +++ b/backend/app/tests/crud/collections/test_crud_collection_read_all.py @@ -9,11 +9,19 @@ from app.tests.utils.document import DocumentStore from app.tests.utils.collection import get_collection from app.tests.utils.utils import openai_credentials +from app.seed_data.seed_data import seed_database -def create_collections(db: Session, n: int): +@pytest.fixture(scope="function", autouse=True) +def load_seed_data(db): + """Load seed data before each test.""" + seed_database(db) + yield + + +def create_collections(db: Session, n: int, api_key_headers: dict[str, str]): crud = None - store = DocumentStore(db) + store = DocumentStore(db, api_key_headers) documents = store.fill(1) openai_mock = OpenAIMock() @@ -39,18 +47,19 @@ class TestCollectionReadAll: _ncollections = 5 def test_number_read_is_expected( - self, - db: Session, + self, db: Session, api_key_headers: dict[str, str] ): db.query(Collection).delete() - owner = create_collections(db, self._ncollections) + owner = create_collections(db, self._ncollections, api_key_headers) crud = CollectionCrud(db, owner) docs = crud.read_all() assert len(docs) == self._ncollections - def test_deleted_docs_are_excluded(self, db: Session): - owner = create_collections(db, self._ncollections) + def test_deleted_docs_are_excluded( + self, db: Session, api_key_headers: dict[str, str] + ): + owner = create_collections(db, self._ncollections, api_key_headers) crud = CollectionCrud(db, owner) assert all(x.deleted_at is None for x in crud.read_all()) diff --git a/backend/app/tests/crud/collections/test_crud_collection_read_one.py b/backend/app/tests/crud/collections/test_crud_collection_read_one.py index 909d1a0c7..01efd36fe 100644 --- a/backend/app/tests/crud/collections/test_crud_collection_read_one.py +++ b/backend/app/tests/crud/collections/test_crud_collection_read_one.py @@ -9,10 +9,18 @@ from app.tests.utils.document import DocumentStore from app.tests.utils.collection import get_collection, uuid_increment from app.tests.utils.utils import openai_credentials +from app.seed_data.seed_data import seed_database -def mk_collection(db: Session): - store = DocumentStore(db) +@pytest.fixture(scope="function", autouse=True) +def load_seed_data(db): + """Load seed data before each test.""" + seed_database(db) + yield + + +def mk_collection(db: Session, api_key_headers: dict[str, str]): + store = DocumentStore(db, api_key_headers) documents = store.fill(1) openai_mock = OpenAIMock() @@ -25,16 +33,18 @@ def mk_collection(db: Session): @pytest.mark.usefixtures("openai_credentials") class TestDatabaseReadOne: - def test_can_select_valid_id(self, db: Session): - collection = mk_collection(db) + def test_can_select_valid_id(self, db: Session, api_key_headers: dict[str, str]): + collection = mk_collection(db, api_key_headers) crud = CollectionCrud(db, collection.owner_id) result = crud.read_one(collection.id) assert result.id == collection.id - def test_cannot_select_others_collections(self, db: Session): - collection = mk_collection(db) + def test_cannot_select_others_collections( + self, db: Session, api_key_headers: dict[str, str] + ): + collection = mk_collection(db, api_key_headers) other = collection.owner_id + 1 crud = CollectionCrud(db, other) diff --git a/backend/app/tests/crud/documents/test_crud_document_delete.py b/backend/app/tests/crud/documents/test_crud_document_delete.py index 093294108..1b369c76a 100644 --- a/backend/app/tests/crud/documents/test_crud_document_delete.py +++ b/backend/app/tests/crud/documents/test_crud_document_delete.py @@ -7,11 +7,19 @@ from app.tests.utils.document import DocumentStore from app.core.exception_handlers import HTTPException +from app.seed_data.seed_data import seed_database + + +@pytest.fixture(scope="function", autouse=True) +def load_seed_data(db): + """Load seed data before each test.""" + seed_database(db) + yield @pytest.fixture -def document(db: Session): - store = DocumentStore(db) +def document(db: Session, api_key_headers: dict[str, str]): + store = DocumentStore(db, api_key_headers) document = store.put() crud = DocumentCrud(db, document.owner_id) @@ -31,8 +39,10 @@ def test_delete_marks_deleted(self, document: Document): def test_delete_follows_insert(self, document: Document): assert document.inserted_at <= document.deleted_at - def test_cannot_delete_others_documents(self, db: Session): - store = DocumentStore(db) + def test_cannot_delete_others_documents( + self, db: Session, api_key_headers: dict[str, str] + ): + store = DocumentStore(db, api_key_headers) document = store.put() other_owner_id = store.documents.owner_id + 1 diff --git a/backend/app/tests/crud/documents/test_crud_document_read_many.py b/backend/app/tests/crud/documents/test_crud_document_read_many.py index 1a6ac6a73..949a883bb 100644 --- a/backend/app/tests/crud/documents/test_crud_document_read_many.py +++ b/backend/app/tests/crud/documents/test_crud_document_read_many.py @@ -4,11 +4,19 @@ from app.crud import DocumentCrud from app.tests.utils.document import DocumentStore +from app.seed_data.seed_data import seed_database + + +@pytest.fixture(scope="function", autouse=True) +def load_seed_data(db): + """Load seed data before each test.""" + seed_database(db) + yield @pytest.fixture -def store(db: Session): - ds = DocumentStore(db) +def store(db: Session, api_key_headers: dict[str, str]): + ds = DocumentStore(db, api_key_headers) ds.fill(TestDatabaseReadMany._ndocs) return ds diff --git a/backend/app/tests/crud/documents/test_crud_document_read_one.py b/backend/app/tests/crud/documents/test_crud_document_read_one.py index 463ea02ea..4d62901dd 100644 --- a/backend/app/tests/crud/documents/test_crud_document_read_one.py +++ b/backend/app/tests/crud/documents/test_crud_document_read_one.py @@ -6,11 +6,19 @@ from app.tests.utils.document import DocumentStore from app.core.exception_handlers import HTTPException +from app.seed_data.seed_data import seed_database + + +@pytest.fixture(scope="function", autouse=True) +def load_seed_data(db): + """Load seed data before each test.""" + seed_database(db) + yield @pytest.fixture -def store(db: Session): - return DocumentStore(db) +def store(db: Session, api_key_headers: dict[str, str]): + return DocumentStore(db, api_key_headers) class TestDatabaseReadOne: @@ -34,12 +42,10 @@ def test_cannot_select_invalid_id(self, db: Session, store: DocumentStore): assert "Document not found" in str(exc_info.value.detail) def test_cannot_read_others_documents( - self, - db: Session, - store: DocumentStore, + self, db: Session, store: DocumentStore, api_key_headers: dict[str, str] ): document = store.put() - other = DocumentStore(db) + other = DocumentStore(db, api_key_headers) crud = DocumentCrud(db, other.owner) with pytest.raises(HTTPException) as exc_info: diff --git a/backend/app/tests/crud/documents/test_crud_document_update.py b/backend/app/tests/crud/documents/test_crud_document_update.py index a14805bcf..d7b74f707 100644 --- a/backend/app/tests/crud/documents/test_crud_document_update.py +++ b/backend/app/tests/crud/documents/test_crud_document_update.py @@ -4,11 +4,19 @@ from app.crud import DocumentCrud from app.tests.utils.document import DocumentMaker, DocumentStore +from app.seed_data.seed_data import seed_database + + +@pytest.fixture(scope="function", autouse=True) +def load_seed_data(db): + """Load seed data before each test.""" + seed_database(db) + yield @pytest.fixture -def documents(db: Session): - store = DocumentStore(db) +def documents(db: Session, api_key_headers: dict[str, str]): + store = DocumentStore(db, api_key_headers) return store.documents diff --git a/backend/app/tests/utils/document.py b/backend/app/tests/utils/document.py index 078ea1c0a..eb3431bdd 100644 --- a/backend/app/tests/utils/document.py +++ b/backend/app/tests/utils/document.py @@ -15,7 +15,7 @@ from app.models import Document from app.utils import APIResponse -from .utils import SequentialUuidGenerator, get_user_id_by_email +from .utils import SequentialUuidGenerator, get_user_id_by_email, get_user_from_api_key @ft.cache @@ -23,13 +23,18 @@ def _get_user_id_by_email(db: Session): return get_user_id_by_email(db) +def _get_user_from_api_key(db: Session, api_key_headers: dict[str, str]): + return get_user_from_api_key(db, api_key_headers) + + def httpx_to_standard(response: Response): return APIResponse(**response.json()) class DocumentMaker: - def __init__(self, db: Session): - self.owner_id = _get_user_id_by_email(db) + def __init__(self, db: Session, api_key_headers: dict[str, str]): + user = _get_user_from_api_key(db, api_key_headers) + self.owner_id = user.user_id self.index = SequentialUuidGenerator() def __iter__(self): @@ -58,9 +63,9 @@ def clear(db: Session): def owner(self): return self.documents.owner_id - def __init__(self, db: Session): + def __init__(self, db: Session, api_key_headers: dict[str, str]): self.db = db - self.documents = DocumentMaker(self.db) + self.documents = DocumentMaker(db, api_key_headers) self.clear(self.db) def put(self): @@ -113,18 +118,18 @@ def append(self, doc: Document, suffix: str = None): @dataclass class WebCrawler: client: TestClient - superuser_token_headers: dict[str, str] + api_key_headers: dict[str, str] def get(self, route: Route): return self.client.get( str(route), - headers=self.superuser_token_headers, + headers=self.api_key_headers, ) def delete(self, route: Route): return self.client.delete( str(route), - headers=self.superuser_token_headers, + headers=self.api_key_headers, ) @@ -158,5 +163,5 @@ def to_dict(self): @pytest.fixture -def crawler(client: TestClient, superuser_token_headers: dict[str, str]): - return WebCrawler(client, superuser_token_headers) +def crawler(client: TestClient, api_key_headers: dict[str, str]): + return WebCrawler(client, api_key_headers) From 765e15683ca2bbbfdcb1554240534e274beccf3d Mon Sep 17 00:00:00 2001 From: nishika26 Date: Wed, 2 Jul 2025 12:34:44 +0530 Subject: [PATCH 07/21] removing spaces --- .env.example | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.env.example b/.env.example index 8df496766..abea3f771 100644 --- a/.env.example +++ b/.env.example @@ -43,7 +43,7 @@ DOCKER_IMAGE_FRONTEND=frontend AWS_ACCESS_KEY_ID= AWS_SECRET_ACCESS_KEY= AWS_DEFAULT_REGION=ap-south-1 -AWS_S3_BUCKET_PREFIX = "bucket-prefix-name" +AWS_S3_BUCKET_PREFIX="bucket-prefix-name" # OpenAI From 7575920b87dc1d9655d707482807ad1ff0ba9d8e Mon Sep 17 00:00:00 2001 From: nishika26 Date: Wed, 2 Jul 2025 13:11:16 +0530 Subject: [PATCH 08/21] mock aws --- .../api/routes/documents/test_route_document_permanent_remove.py | 1 + 1 file changed, 1 insertion(+) diff --git a/backend/app/tests/api/routes/documents/test_route_document_permanent_remove.py b/backend/app/tests/api/routes/documents/test_route_document_permanent_remove.py index 5036223da..babd0dd9a 100644 --- a/backend/app/tests/api/routes/documents/test_route_document_permanent_remove.py +++ b/backend/app/tests/api/routes/documents/test_route_document_permanent_remove.py @@ -48,6 +48,7 @@ def aws_credentials(): @pytest.mark.usefixtures("openai_credentials") +@mock_aws class TestDocumentRouteRemove: @openai_responses.mock() @patch("app.api.routes.documents.get_provider_credential") From f0990fb5492d4855e856a1789bf1bea357147e31 Mon Sep 17 00:00:00 2001 From: nishika26 Date: Thu, 3 Jul 2025 22:02:23 +0530 Subject: [PATCH 09/21] logging --- backend/app/api/routes/collections.py | 39 +++++++++++---------------- backend/app/api/routes/documents.py | 6 +++++ 2 files changed, 21 insertions(+), 24 deletions(-) diff --git a/backend/app/api/routes/collections.py b/backend/app/api/routes/collections.py index 00abdf3d3..991cb9fd2 100644 --- a/backend/app/api/routes/collections.py +++ b/backend/app/api/routes/collections.py @@ -200,28 +200,10 @@ def do_create_collection( current_user: CurrentUserOrgProject, request: CreationRequest, payload: ResponsePayload, + client, ): start_time = time.time() - credentials = get_provider_credential( - session=session, - org_id=current_user.organization_id, - provider="openai", - project_id=current_user.project_id, - ) - client, success = configure_openai(credentials) - if not success: - logger.error( - f"OpenAI API key not configured for org_id={current_user.organization_id}, project_id={current_user.project_id}" - ) - - mark_collection_failed( - session=session, - user_id=current_user.id, - collection_id=UUID(payload.key), - reason="OpenAI config failed", - ) - callback = ( SilentCallback(payload) if request.callback_url is None @@ -297,6 +279,19 @@ def create_collection( request: CreationRequest, background_tasks: BackgroundTasks, ): + credentials = get_provider_credential( + session=session, + org_id=current_user.organization_id, + provider="openai", + project_id=current_user.project_id, + ) + client, success = configure_openai(credentials) + if not success: + logger.error( + f"[create_collection] OpenAI API key not configured for org_id={current_user.organization_id}, project_id={current_user.project_id}" + ) + raise HTTPException(status_code=400, detail="OpenAI is not configured") + this = inspect.currentframe() route = router.url_path_for(this.f_code.co_name) payload = ResponsePayload("processing", route) @@ -314,11 +309,7 @@ def create_collection( # 2. Launch background task background_tasks.add_task( - do_create_collection, - session, - current_user, - request, - payload, + do_create_collection, session, current_user, request, payload, client ) logger.info( diff --git a/backend/app/api/routes/documents.py b/backend/app/api/routes/documents.py index c3f85ffc9..60a30cbf7 100644 --- a/backend/app/api/routes/documents.py +++ b/backend/app/api/routes/documents.py @@ -77,6 +77,9 @@ def remove_doc( ) client, success = configure_openai(credentials) if not success: + logger.error( + f"[remove_doc] OpenAI API key not configured for org_id={current_user.organization_id}, project_id={current_user.project_id}" + ) raise HTTPException(status_code=400, detail="OpenAI is not configured") a_crud = OpenAIAssistantCrud(client) @@ -106,6 +109,9 @@ def permanent_delete_doc( ) client, success = configure_openai(credentials) if not success: + logger.error( + f"[permanent_delete_doc] OpenAI API key not configured for org_id={current_user.organization_id}, project_id={current_user.project_id}" + ) raise HTTPException(status_code=400, detail="OpenAI is not configured") a_crud = OpenAIAssistantCrud(client) From f91b613bdb6eec90c6307a265825cc26b29414a6 Mon Sep 17 00:00:00 2001 From: nishika26 Date: Mon, 21 Jul 2025 13:59:14 +0530 Subject: [PATCH 10/21] removed hardcoded api key header --- .../collections/test_create_collections.py | 18 ++++++++++-------- .../documents/test_route_document_info.py | 10 ++++------ .../documents/test_route_document_list.py | 6 ++---- .../test_route_document_permanent_remove.py | 6 ++---- .../documents/test_route_document_remove.py | 14 +++++--------- .../collections/test_crud_collection_create.py | 6 ++---- .../collections/test_crud_collection_delete.py | 6 ++---- .../test_crud_collection_read_all.py | 16 ++++++---------- .../test_crud_collection_read_one.py | 14 ++++++-------- .../documents/test_crud_document_delete.py | 10 ++++------ .../documents/test_crud_document_read_many.py | 4 ++-- .../documents/test_crud_document_read_one.py | 10 ++++------ .../documents/test_crud_document_update.py | 4 ++-- backend/app/tests/utils/document.py | 15 +++++---------- 14 files changed, 56 insertions(+), 83 deletions(-) diff --git a/backend/app/tests/api/routes/collections/test_create_collections.py b/backend/app/tests/api/routes/collections/test_create_collections.py index 462d74160..686ea2c2a 100644 --- a/backend/app/tests/api/routes/collections/test_create_collections.py +++ b/backend/app/tests/api/routes/collections/test_create_collections.py @@ -2,12 +2,10 @@ from uuid import UUID import io -import openai_responses from sqlmodel import Session from fastapi.testclient import TestClient from unittest.mock import MagicMock, patch - from app.core.config import settings from app.tests.utils.document import DocumentStore from app.tests.utils.utils import get_user_from_api_key @@ -44,16 +42,21 @@ def head_object(self, Bucket, Key): monkeypatch.setattr("boto3.client", lambda service: FakeS3Client()) -@patch("app.api.routes.collections.configure_openai") -@patch("app.api.routes.collections.get_provider_credential") class TestCollectionRouteCreate: _n_documents = 5 + @patch("app.api.routes.collections.configure_openai") + @patch("app.api.routes.collections.get_provider_credential") def test_create_collection_success( - self, client: TestClient, db: Session, normal_user_api_key_headers + self, + mock_get_credential, + mock_configure_openai, + client: TestClient, + db: Session, + normal_user_api_key_headers, ): # Setup test documents - store = DocumentStore(db, api_key_headers) + store = DocumentStore(db) documents = store.fill(self._n_documents) doc_ids = [str(doc.id) for doc in documents] @@ -64,11 +67,10 @@ def test_create_collection_success( "instructions": "Test collection assistant.", "temperature": 0.1, } - + headers = normal_user_api_key_headers mock_get_credential.return_value = {"api_key": "test_api_key"} - mock_openai_client = get_mock_openai_client() mock_configure_openai.return_value = (mock_openai_client, True) diff --git a/backend/app/tests/api/routes/documents/test_route_document_info.py b/backend/app/tests/api/routes/documents/test_route_document_info.py index 0d668f915..41b3ebc52 100644 --- a/backend/app/tests/api/routes/documents/test_route_document_info.py +++ b/backend/app/tests/api/routes/documents/test_route_document_info.py @@ -23,9 +23,8 @@ def test_response_is_success( db: Session, route: Route, crawler: WebCrawler, - api_key_headers: dict[str, str], ): - store = DocumentStore(db, api_key_headers) + store = DocumentStore(db) response = crawler.get(route.append(store.put())) assert response.is_success @@ -35,9 +34,8 @@ def test_info_reflects_database( db: Session, route: Route, crawler: WebCrawler, - api_key_headers: dict[str, str], ): - store = DocumentStore(db, api_key_headers) + store = DocumentStore(db) document = store.put() source = DocumentComparator(document) @@ -46,10 +44,10 @@ def test_info_reflects_database( assert source == target.data def test_cannot_info_unknown_document( - self, db: Session, route: Route, crawler: Route, api_key_headers: dict[str, str] + self, db: Session, route: Route, crawler: Route ): DocumentStore.clear(db) - maker = DocumentMaker(db, api_key_headers) + maker = DocumentMaker(db) response = crawler.get(route.append(next(maker))) assert response.is_error diff --git a/backend/app/tests/api/routes/documents/test_route_document_list.py b/backend/app/tests/api/routes/documents/test_route_document_list.py index ed2d26166..b488b9ae1 100644 --- a/backend/app/tests/api/routes/documents/test_route_document_list.py +++ b/backend/app/tests/api/routes/documents/test_route_document_list.py @@ -47,9 +47,8 @@ def test_item_reflects_database( db: Session, route: QueryRoute, crawler: WebCrawler, - api_key_headers: dict[str, str], ): - store = DocumentStore(db, api_key_headers) + store = DocumentStore(db) source = DocumentComparator(store.put()) response = httpx_to_standard(crawler.get(route)) @@ -78,9 +77,8 @@ def test_skip_greater_than_limit_is_difference( db: Session, route: QueryRoute, crawler: WebCrawler, - api_key_headers: dict[str, str], ): - store = DocumentStore(db, api_key_headers) + store = DocumentStore(db) limit = len(store.fill(self._ndocs)) skip = limit // 2 diff --git a/backend/app/tests/api/routes/documents/test_route_document_permanent_remove.py b/backend/app/tests/api/routes/documents/test_route_document_permanent_remove.py index babd0dd9a..e01937078 100644 --- a/backend/app/tests/api/routes/documents/test_route_document_permanent_remove.py +++ b/backend/app/tests/api/routes/documents/test_route_document_permanent_remove.py @@ -60,7 +60,6 @@ def test_item_is_soft_removed( db: Session, route: Route, crawler: WebCrawler, - api_key_headers: dict[str, str], ): openai_mock = OpenAIMock() with openai_mock.router: @@ -73,7 +72,7 @@ def test_item_is_soft_removed( aws.create() # Setup document in DB and S3 - store = DocumentStore(db, api_key_headers) + store = DocumentStore(db) document = store.put() s3_key = Path(urlparse(document.object_store_url).path).relative_to("/") aws.client.put_object( @@ -105,11 +104,10 @@ def test_cannot_delete_nonexistent_document( db: Session, route: Route, crawler: WebCrawler, - api_key_headers: dict[str, str], ): DocumentStore.clear(db) - maker = DocumentMaker(db, api_key_headers) + maker = DocumentMaker(db) response = crawler.delete(route.append(next(maker), suffix="permanent")) assert response.is_error diff --git a/backend/app/tests/api/routes/documents/test_route_document_remove.py b/backend/app/tests/api/routes/documents/test_route_document_remove.py index b6f3c1c8c..435febd7e 100644 --- a/backend/app/tests/api/routes/documents/test_route_document_remove.py +++ b/backend/app/tests/api/routes/documents/test_route_document_remove.py @@ -13,9 +13,8 @@ WebCrawler, crawler, ) -from app.tests.utils.collection import get_collection -from app.tests.utils.utils import openai_credentials from app.seed_data.seed_data import seed_database +from app.tests.utils.utils import openai_credentials @pytest.fixture(scope="function", autouse=True) @@ -42,7 +41,6 @@ def test_response_is_success( db: Session, route: Route, crawler: WebCrawler, - api_key_headers: dict[str, str], ): openai_mock = OpenAIMock() with openai_mock.router: @@ -50,7 +48,7 @@ def test_response_is_success( mock_get_credential.return_value = {"api_key": "sk-test-key"} mock_configure_openai.return_value = (client, True) - store = DocumentStore(db, api_key_headers) + store = DocumentStore(db) response = crawler.get(route.append(store.put())) assert response.is_success @@ -65,15 +63,14 @@ def test_item_is_soft_removed( db: Session, route: Route, crawler: WebCrawler, - api_key_headers: dict[str, str], ): openai_mock = OpenAIMock() with openai_mock.router: + mock_get_credential.return_value = {"api_key": "test-key"} client = OpenAI(api_key="test_key") - mock_get_credential.return_value = {"api_key": "sk-test-key"} mock_configure_openai.return_value = (client, True) - store = DocumentStore(db, api_key_headers) + store = DocumentStore(db) document = store.put() crawler.get(route.append(document)) @@ -93,7 +90,6 @@ def test_cannot_remove_unknown_document( db: Session, route: Route, crawler: WebCrawler, - api_key_headers: dict[str, str], ): openai_mock = OpenAIMock() with openai_mock.router: @@ -102,7 +98,7 @@ def test_cannot_remove_unknown_document( mock_configure_openai.return_value = (client, True) DocumentStore.clear(db) - maker = DocumentMaker(db, api_key_headers) + maker = DocumentMaker(db) response = crawler.get(route.append(next(maker))) assert response.is_error diff --git a/backend/app/tests/crud/collections/test_crud_collection_create.py b/backend/app/tests/crud/collections/test_crud_collection_create.py index f413f5641..003bbcad7 100644 --- a/backend/app/tests/crud/collections/test_crud_collection_create.py +++ b/backend/app/tests/crud/collections/test_crud_collection_create.py @@ -22,10 +22,8 @@ class TestCollectionCreate: _n_documents = 10 @openai_responses.mock() - def test_create_associates_documents( - self, db: Session, api_key_headers: dict[str, str] - ): - store = DocumentStore(db, api_key_headers) + def test_create_associates_documents(self, db: Session): + store = DocumentStore(db) documents = store.fill(self._n_documents) collection = get_collection(db) diff --git a/backend/app/tests/crud/collections/test_crud_collection_delete.py b/backend/app/tests/crud/collections/test_crud_collection_delete.py index 4803ecd89..a73cf8bda 100644 --- a/backend/app/tests/crud/collections/test_crud_collection_delete.py +++ b/backend/app/tests/crud/collections/test_crud_collection_delete.py @@ -60,10 +60,8 @@ def test_cannot_delete_others_collections(self, db: Session): crud.delete(collection, assistant) @openai_responses.mock() - def test_delete_document_deletes_collections( - self, db: Session, api_key_headers: dict[str, str] - ): - store = DocumentStore(db, api_key_headers) + def test_delete_document_deletes_collections(self, db: Session): + store = DocumentStore(db) documents = store.fill(1) client = OpenAI(api_key="test_api_key") diff --git a/backend/app/tests/crud/collections/test_crud_collection_read_all.py b/backend/app/tests/crud/collections/test_crud_collection_read_all.py index 32f3ae260..8dd003db9 100644 --- a/backend/app/tests/crud/collections/test_crud_collection_read_all.py +++ b/backend/app/tests/crud/collections/test_crud_collection_read_all.py @@ -19,9 +19,9 @@ def load_seed_data(db): yield -def create_collections(db: Session, n: int, api_key_headers: dict[str, str]): +def create_collections(db: Session, n: int): crud = None - store = DocumentStore(db, api_key_headers) + store = DocumentStore(db) documents = store.fill(1) openai_mock = OpenAIMock() @@ -46,20 +46,16 @@ def refresh(self, db: Session): class TestCollectionReadAll: _ncollections = 5 - def test_number_read_is_expected( - self, db: Session, api_key_headers: dict[str, str] - ): + def test_number_read_is_expected(self, db: Session): db.query(Collection).delete() - owner = create_collections(db, self._ncollections, api_key_headers) + owner = create_collections(db, self._ncollections) crud = CollectionCrud(db, owner) docs = crud.read_all() assert len(docs) == self._ncollections - def test_deleted_docs_are_excluded( - self, db: Session, api_key_headers: dict[str, str] - ): - owner = create_collections(db, self._ncollections, api_key_headers) + def test_deleted_docs_are_excluded(self, db: Session): + owner = create_collections(db, self._ncollections) crud = CollectionCrud(db, owner) assert all(x.deleted_at is None for x in crud.read_all()) diff --git a/backend/app/tests/crud/collections/test_crud_collection_read_one.py b/backend/app/tests/crud/collections/test_crud_collection_read_one.py index 01efd36fe..7dfa4992d 100644 --- a/backend/app/tests/crud/collections/test_crud_collection_read_one.py +++ b/backend/app/tests/crud/collections/test_crud_collection_read_one.py @@ -19,8 +19,8 @@ def load_seed_data(db): yield -def mk_collection(db: Session, api_key_headers: dict[str, str]): - store = DocumentStore(db, api_key_headers) +def mk_collection(db: Session): + store = DocumentStore(db) documents = store.fill(1) openai_mock = OpenAIMock() @@ -33,18 +33,16 @@ def mk_collection(db: Session, api_key_headers: dict[str, str]): @pytest.mark.usefixtures("openai_credentials") class TestDatabaseReadOne: - def test_can_select_valid_id(self, db: Session, api_key_headers: dict[str, str]): - collection = mk_collection(db, api_key_headers) + def test_can_select_valid_id(self, db: Session): + collection = mk_collection(db) crud = CollectionCrud(db, collection.owner_id) result = crud.read_one(collection.id) assert result.id == collection.id - def test_cannot_select_others_collections( - self, db: Session, api_key_headers: dict[str, str] - ): - collection = mk_collection(db, api_key_headers) + def test_cannot_select_others_collections(self, db: Session): + collection = mk_collection(db) other = collection.owner_id + 1 crud = CollectionCrud(db, other) diff --git a/backend/app/tests/crud/documents/test_crud_document_delete.py b/backend/app/tests/crud/documents/test_crud_document_delete.py index 1b369c76a..710e67571 100644 --- a/backend/app/tests/crud/documents/test_crud_document_delete.py +++ b/backend/app/tests/crud/documents/test_crud_document_delete.py @@ -18,8 +18,8 @@ def load_seed_data(db): @pytest.fixture -def document(db: Session, api_key_headers: dict[str, str]): - store = DocumentStore(db, api_key_headers) +def document(db: Session): + store = DocumentStore(db) document = store.put() crud = DocumentCrud(db, document.owner_id) @@ -39,10 +39,8 @@ def test_delete_marks_deleted(self, document: Document): def test_delete_follows_insert(self, document: Document): assert document.inserted_at <= document.deleted_at - def test_cannot_delete_others_documents( - self, db: Session, api_key_headers: dict[str, str] - ): - store = DocumentStore(db, api_key_headers) + def test_cannot_delete_others_documents(self, db: Session): + store = DocumentStore(db) document = store.put() other_owner_id = store.documents.owner_id + 1 diff --git a/backend/app/tests/crud/documents/test_crud_document_read_many.py b/backend/app/tests/crud/documents/test_crud_document_read_many.py index 949a883bb..72df8374c 100644 --- a/backend/app/tests/crud/documents/test_crud_document_read_many.py +++ b/backend/app/tests/crud/documents/test_crud_document_read_many.py @@ -15,8 +15,8 @@ def load_seed_data(db): @pytest.fixture -def store(db: Session, api_key_headers: dict[str, str]): - ds = DocumentStore(db, api_key_headers) +def store(db: Session): + ds = DocumentStore(db) ds.fill(TestDatabaseReadMany._ndocs) return ds diff --git a/backend/app/tests/crud/documents/test_crud_document_read_one.py b/backend/app/tests/crud/documents/test_crud_document_read_one.py index 4d62901dd..6a13570ba 100644 --- a/backend/app/tests/crud/documents/test_crud_document_read_one.py +++ b/backend/app/tests/crud/documents/test_crud_document_read_one.py @@ -17,8 +17,8 @@ def load_seed_data(db): @pytest.fixture -def store(db: Session, api_key_headers: dict[str, str]): - return DocumentStore(db, api_key_headers) +def store(db: Session): + return DocumentStore(db) class TestDatabaseReadOne: @@ -41,11 +41,9 @@ def test_cannot_select_invalid_id(self, db: Session, store: DocumentStore): assert exc_info.value.status_code == 404 assert "Document not found" in str(exc_info.value.detail) - def test_cannot_read_others_documents( - self, db: Session, store: DocumentStore, api_key_headers: dict[str, str] - ): + def test_cannot_read_others_documents(self, db: Session, store: DocumentStore): document = store.put() - other = DocumentStore(db, api_key_headers) + other = DocumentStore(db) crud = DocumentCrud(db, other.owner) with pytest.raises(HTTPException) as exc_info: diff --git a/backend/app/tests/crud/documents/test_crud_document_update.py b/backend/app/tests/crud/documents/test_crud_document_update.py index d7b74f707..e74a3dc77 100644 --- a/backend/app/tests/crud/documents/test_crud_document_update.py +++ b/backend/app/tests/crud/documents/test_crud_document_update.py @@ -15,8 +15,8 @@ def load_seed_data(db): @pytest.fixture -def documents(db: Session, api_key_headers: dict[str, str]): - store = DocumentStore(db, api_key_headers) +def documents(db: Session): + store = DocumentStore(db) return store.documents diff --git a/backend/app/tests/utils/document.py b/backend/app/tests/utils/document.py index 61f80e827..ab72d2cef 100644 --- a/backend/app/tests/utils/document.py +++ b/backend/app/tests/utils/document.py @@ -15,7 +15,7 @@ from app.models import Document from app.utils import APIResponse -from .utils import SequentialUuidGenerator, get_user_id_by_email, get_user_from_api_key +from .utils import SequentialUuidGenerator, get_user_id_by_email @ft.cache @@ -23,18 +23,13 @@ def _get_user_id_by_email(db: Session): return get_user_id_by_email(db) -def _get_user_from_api_key(db: Session, api_key_headers: dict[str, str]): - return get_user_from_api_key(db, api_key_headers) - - def httpx_to_standard(response: Response): return APIResponse(**response.json()) class DocumentMaker: - def __init__(self, db: Session, api_key_headers: dict[str, str]): - user = _get_user_from_api_key(db, api_key_headers) - self.owner_id = user.user_id + def __init__(self, db: Session): + self.owner_id = _get_user_id_by_email(db) self.index = SequentialUuidGenerator() def __iter__(self): @@ -63,9 +58,9 @@ def clear(db: Session): def owner(self): return self.documents.owner_id - def __init__(self, db: Session, api_key_headers: dict[str, str]): + def __init__(self, db: Session): self.db = db - self.documents = DocumentMaker(db, api_key_headers) + self.documents = DocumentMaker(self.db) self.clear(self.db) def put(self): From 1fe3c989e91708849fe9db4a3b8007d4fcf5262c Mon Sep 17 00:00:00 2001 From: nishika26 Date: Mon, 21 Jul 2025 14:01:55 +0530 Subject: [PATCH 11/21] formatting --- backend/app/tests/conftest.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/backend/app/tests/conftest.py b/backend/app/tests/conftest.py index 2fb670dae..2ae724d26 100644 --- a/backend/app/tests/conftest.py +++ b/backend/app/tests/conftest.py @@ -1,11 +1,8 @@ -from collections.abc import Generator - import pytest -import time - from fastapi.testclient import TestClient from sqlmodel import Session from sqlalchemy import event +from collections.abc import Generator from app.core.config import settings from app.core.db import engine @@ -62,6 +59,7 @@ def normal_user_token_headers(client: TestClient, db: Session) -> dict[str, str] client=client, email=settings.EMAIL_TEST_USER, db=db ) + @pytest.fixture(scope="function") def superuser_api_key_headers(db: Session) -> dict[str, str]: api_key = get_api_key_by_email(db, settings.FIRST_SUPERUSER) From 94435c0b1a90043320684ffd3532ba7fec449f83 Mon Sep 17 00:00:00 2001 From: nishika26 Date: Mon, 21 Jul 2025 14:17:33 +0530 Subject: [PATCH 12/21] removing seed database part --- .../documents/test_route_document_permanent_remove.py | 8 -------- .../api/routes/documents/test_route_document_remove.py | 8 -------- .../tests/crud/collections/test_crud_collection_create.py | 8 -------- .../tests/crud/collections/test_crud_collection_delete.py | 8 -------- .../crud/collections/test_crud_collection_read_all.py | 8 -------- .../crud/collections/test_crud_collection_read_one.py | 8 -------- .../app/tests/crud/documents/test_crud_document_delete.py | 8 -------- .../tests/crud/documents/test_crud_document_read_many.py | 8 -------- .../tests/crud/documents/test_crud_document_read_one.py | 8 -------- .../app/tests/crud/documents/test_crud_document_update.py | 8 -------- 10 files changed, 80 deletions(-) diff --git a/backend/app/tests/api/routes/documents/test_route_document_permanent_remove.py b/backend/app/tests/api/routes/documents/test_route_document_permanent_remove.py index e01937078..854f205bf 100644 --- a/backend/app/tests/api/routes/documents/test_route_document_permanent_remove.py +++ b/backend/app/tests/api/routes/documents/test_route_document_permanent_remove.py @@ -23,7 +23,6 @@ crawler, ) from app.tests.utils.utils import openai_credentials -from app.seed_data.seed_data import seed_database @pytest.fixture @@ -31,13 +30,6 @@ def route(): return Route("remove") -@pytest.fixture(scope="function", autouse=True) -def load_seed_data(db): - """Load seed data before each test.""" - seed_database(db) - yield - - @pytest.fixture(scope="class") def aws_credentials(): os.environ["AWS_ACCESS_KEY_ID"] = "testing" diff --git a/backend/app/tests/api/routes/documents/test_route_document_remove.py b/backend/app/tests/api/routes/documents/test_route_document_remove.py index 435febd7e..5c6c31752 100644 --- a/backend/app/tests/api/routes/documents/test_route_document_remove.py +++ b/backend/app/tests/api/routes/documents/test_route_document_remove.py @@ -13,17 +13,9 @@ WebCrawler, crawler, ) -from app.seed_data.seed_data import seed_database from app.tests.utils.utils import openai_credentials -@pytest.fixture(scope="function", autouse=True) -def load_seed_data(db): - """Load seed data before each test.""" - seed_database(db) - yield - - @pytest.fixture def route(): return Route("remove") diff --git a/backend/app/tests/crud/collections/test_crud_collection_create.py b/backend/app/tests/crud/collections/test_crud_collection_create.py index 003bbcad7..564230b33 100644 --- a/backend/app/tests/crud/collections/test_crud_collection_create.py +++ b/backend/app/tests/crud/collections/test_crud_collection_create.py @@ -7,14 +7,6 @@ from app.tests.utils.document import DocumentStore from app.tests.utils.collection import get_collection from app.tests.utils.utils import openai_credentials -from app.seed_data.seed_data import seed_database - - -@pytest.fixture(scope="function", autouse=True) -def load_seed_data(db): - """Load seed data before each test.""" - seed_database(db) - yield @pytest.mark.usefixtures("openai_credentials") diff --git a/backend/app/tests/crud/collections/test_crud_collection_delete.py b/backend/app/tests/crud/collections/test_crud_collection_delete.py index a73cf8bda..c4d7c8bb2 100644 --- a/backend/app/tests/crud/collections/test_crud_collection_delete.py +++ b/backend/app/tests/crud/collections/test_crud_collection_delete.py @@ -9,14 +9,6 @@ from app.tests.utils.document import DocumentStore from app.tests.utils.collection import get_collection, uuid_increment from app.tests.utils.utils import openai_credentials -from app.seed_data.seed_data import seed_database - - -@pytest.fixture(scope="function", autouse=True) -def load_seed_data(db): - """Load seed data before each test.""" - seed_database(db) - yield @pytest.mark.usefixtures("openai_credentials") diff --git a/backend/app/tests/crud/collections/test_crud_collection_read_all.py b/backend/app/tests/crud/collections/test_crud_collection_read_all.py index 8dd003db9..41056f3a9 100644 --- a/backend/app/tests/crud/collections/test_crud_collection_read_all.py +++ b/backend/app/tests/crud/collections/test_crud_collection_read_all.py @@ -9,14 +9,6 @@ from app.tests.utils.document import DocumentStore from app.tests.utils.collection import get_collection from app.tests.utils.utils import openai_credentials -from app.seed_data.seed_data import seed_database - - -@pytest.fixture(scope="function", autouse=True) -def load_seed_data(db): - """Load seed data before each test.""" - seed_database(db) - yield def create_collections(db: Session, n: int): diff --git a/backend/app/tests/crud/collections/test_crud_collection_read_one.py b/backend/app/tests/crud/collections/test_crud_collection_read_one.py index 7dfa4992d..909d1a0c7 100644 --- a/backend/app/tests/crud/collections/test_crud_collection_read_one.py +++ b/backend/app/tests/crud/collections/test_crud_collection_read_one.py @@ -9,14 +9,6 @@ from app.tests.utils.document import DocumentStore from app.tests.utils.collection import get_collection, uuid_increment from app.tests.utils.utils import openai_credentials -from app.seed_data.seed_data import seed_database - - -@pytest.fixture(scope="function", autouse=True) -def load_seed_data(db): - """Load seed data before each test.""" - seed_database(db) - yield def mk_collection(db: Session): diff --git a/backend/app/tests/crud/documents/test_crud_document_delete.py b/backend/app/tests/crud/documents/test_crud_document_delete.py index 710e67571..093294108 100644 --- a/backend/app/tests/crud/documents/test_crud_document_delete.py +++ b/backend/app/tests/crud/documents/test_crud_document_delete.py @@ -7,14 +7,6 @@ from app.tests.utils.document import DocumentStore from app.core.exception_handlers import HTTPException -from app.seed_data.seed_data import seed_database - - -@pytest.fixture(scope="function", autouse=True) -def load_seed_data(db): - """Load seed data before each test.""" - seed_database(db) - yield @pytest.fixture diff --git a/backend/app/tests/crud/documents/test_crud_document_read_many.py b/backend/app/tests/crud/documents/test_crud_document_read_many.py index 72df8374c..1a6ac6a73 100644 --- a/backend/app/tests/crud/documents/test_crud_document_read_many.py +++ b/backend/app/tests/crud/documents/test_crud_document_read_many.py @@ -4,14 +4,6 @@ from app.crud import DocumentCrud from app.tests.utils.document import DocumentStore -from app.seed_data.seed_data import seed_database - - -@pytest.fixture(scope="function", autouse=True) -def load_seed_data(db): - """Load seed data before each test.""" - seed_database(db) - yield @pytest.fixture diff --git a/backend/app/tests/crud/documents/test_crud_document_read_one.py b/backend/app/tests/crud/documents/test_crud_document_read_one.py index 6a13570ba..a3de8f49d 100644 --- a/backend/app/tests/crud/documents/test_crud_document_read_one.py +++ b/backend/app/tests/crud/documents/test_crud_document_read_one.py @@ -6,14 +6,6 @@ from app.tests.utils.document import DocumentStore from app.core.exception_handlers import HTTPException -from app.seed_data.seed_data import seed_database - - -@pytest.fixture(scope="function", autouse=True) -def load_seed_data(db): - """Load seed data before each test.""" - seed_database(db) - yield @pytest.fixture diff --git a/backend/app/tests/crud/documents/test_crud_document_update.py b/backend/app/tests/crud/documents/test_crud_document_update.py index e74a3dc77..a14805bcf 100644 --- a/backend/app/tests/crud/documents/test_crud_document_update.py +++ b/backend/app/tests/crud/documents/test_crud_document_update.py @@ -4,14 +4,6 @@ from app.crud import DocumentCrud from app.tests.utils.document import DocumentMaker, DocumentStore -from app.seed_data.seed_data import seed_database - - -@pytest.fixture(scope="function", autouse=True) -def load_seed_data(db): - """Load seed data before each test.""" - seed_database(db) - yield @pytest.fixture From 4801db36dab924593455c575e1efc588b32eb3d4 Mon Sep 17 00:00:00 2001 From: nishika26 Date: Mon, 21 Jul 2025 14:24:19 +0530 Subject: [PATCH 13/21] small fixes --- .../routes/documents/test_route_document_permanent_remove.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/backend/app/tests/api/routes/documents/test_route_document_permanent_remove.py b/backend/app/tests/api/routes/documents/test_route_document_permanent_remove.py index 854f205bf..b1ea8f7ac 100644 --- a/backend/app/tests/api/routes/documents/test_route_document_permanent_remove.py +++ b/backend/app/tests/api/routes/documents/test_route_document_permanent_remove.py @@ -39,9 +39,9 @@ def aws_credentials(): os.environ["AWS_DEFAULT_REGION"] = settings.AWS_DEFAULT_REGION -@pytest.mark.usefixtures("openai_credentials") +@pytest.mark.usefixtures("openai_credentials", "aws_credentials") @mock_aws -class TestDocumentRouteRemove: +class TestDocumentRoutePermanentRemove: @openai_responses.mock() @patch("app.api.routes.documents.get_provider_credential") @patch("app.api.routes.documents.configure_openai") From d87c09a161b6ce41d1c80acc0da70a9afdd0affd Mon Sep 17 00:00:00 2001 From: nishika26 Date: Thu, 24 Jul 2025 12:59:54 +0530 Subject: [PATCH 14/21] review fixes --- backend/app/api/routes/collections.py | 38 ++++--------------- backend/app/api/routes/documents.py | 30 +++------------ .../collections/test_collection_info.py | 3 -- .../collections/test_create_collections.py | 14 ++----- .../test_route_document_permanent_remove.py | 14 +++---- .../documents/test_route_document_remove.py | 33 ++++++---------- .../test_crud_collection_delete.py | 8 ++-- .../test_crud_collection_read_all.py | 8 ++-- .../test_crud_collection_read_one.py | 8 ++-- backend/app/tests/utils/utils.py | 5 ++- 10 files changed, 49 insertions(+), 112 deletions(-) diff --git a/backend/app/api/routes/collections.py b/backend/app/api/routes/collections.py index 991cb9fd2..c571cdc00 100644 --- a/backend/app/api/routes/collections.py +++ b/backend/app/api/routes/collections.py @@ -1,31 +1,28 @@ import inspect import logging import time -import warnings from uuid import UUID, uuid4 from typing import Any, List, Optional from dataclasses import dataclass, field, fields, asdict, replace -from openai import OpenAI, OpenAIError +from openai import OpenAIError, OpenAI from fastapi import APIRouter, HTTPException, BackgroundTasks, Query from fastapi import Path as FastPath from pydantic import BaseModel, Field, HttpUrl -from sqlalchemy.exc import NoResultFound, MultipleResultsFound, SQLAlchemyError +from sqlalchemy.exc import SQLAlchemyError from app.api.deps import CurrentUser, SessionDep, CurrentUserOrgProject from app.core.cloud import AmazonCloudStorage -from app.core.config import settings -from app.core.util import now, raise_from_unknown, post_callback, configure_openai +from app.core.util import now, post_callback, configure_openai from app.crud import ( DocumentCrud, CollectionCrud, DocumentCollectionCrud, - get_provider_credential, ) from app.crud.rag import OpenAIVectorStoreCrud, OpenAIAssistantCrud from app.models import Collection, Document from app.models.collection import CollectionStatus -from app.utils import APIResponse, load_description +from app.utils import APIResponse, load_description, get_openai_client logger = logging.getLogger(__name__) router = APIRouter(prefix="/collections", tags=["collections"]) @@ -183,24 +180,12 @@ def _backout(crud: OpenAIAssistantCrud, assistant_id: str): ) -def mark_collection_failed(session, user_id, collection_id, reason: str): - try: - collection = CollectionCrud(session, user_id).read_one(collection_id) - collection.status = CollectionStatus.failed - collection.updated_at = now() - CollectionCrud(session, user_id)._update(collection) - except Exception as suberr: - logger.warning( - f"[do_create_collection] Failed to mark collection failed | {{'collection_id': '{collection_id}', 'reason': '{str(suberr)}'}}" - ) - - def do_create_collection( session: SessionDep, current_user: CurrentUserOrgProject, request: CreationRequest, payload: ResponsePayload, - client, + client: OpenAI, ): start_time = time.time() @@ -279,18 +264,9 @@ def create_collection( request: CreationRequest, background_tasks: BackgroundTasks, ): - credentials = get_provider_credential( - session=session, - org_id=current_user.organization_id, - provider="openai", - project_id=current_user.project_id, + client = get_openai_client( + session, current_user.organization_id, current_user.project_id ) - client, success = configure_openai(credentials) - if not success: - logger.error( - f"[create_collection] OpenAI API key not configured for org_id={current_user.organization_id}, project_id={current_user.project_id}" - ) - raise HTTPException(status_code=400, detail="OpenAI is not configured") this = inspect.currentframe() route = router.url_path_for(this.f_code.co_name) diff --git a/backend/app/api/routes/documents.py b/backend/app/api/routes/documents.py index 60a30cbf7..c9c49f890 100644 --- a/backend/app/api/routes/documents.py +++ b/backend/app/api/routes/documents.py @@ -6,10 +6,10 @@ from fastapi import APIRouter, File, UploadFile, Query, HTTPException from fastapi import Path as FastPath -from app.crud import DocumentCrud, CollectionCrud, get_provider_credential +from app.crud import DocumentCrud, CollectionCrud from app.models import Document from app.core.util import configure_openai -from app.utils import APIResponse, load_description +from app.utils import APIResponse, load_description, get_openai_client from app.api.deps import CurrentUser, SessionDep, CurrentUserOrgProject from app.core.cloud import AmazonCloudStorage from app.crud.rag import OpenAIAssistantCrud @@ -69,18 +69,9 @@ def remove_doc( current_user: CurrentUserOrgProject, doc_id: UUID = FastPath(description="Document to delete"), ): - credentials = get_provider_credential( - session=session, - org_id=current_user.organization_id, - provider="openai", - project_id=current_user.project_id, + client = get_openai_client( + session, current_user.organization_id, current_user.project_id ) - client, success = configure_openai(credentials) - if not success: - logger.error( - f"[remove_doc] OpenAI API key not configured for org_id={current_user.organization_id}, project_id={current_user.project_id}" - ) - raise HTTPException(status_code=400, detail="OpenAI is not configured") a_crud = OpenAIAssistantCrud(client) d_crud = DocumentCrud(session, current_user.id) @@ -101,18 +92,9 @@ def permanent_delete_doc( current_user: CurrentUserOrgProject, doc_id: UUID = FastPath(description="Document to permanently delete"), ): - credentials = get_provider_credential( - session=session, - org_id=current_user.organization_id, - provider="openai", - project_id=current_user.project_id, + client = get_openai_client( + session, current_user.organization_id, current_user.project_id ) - client, success = configure_openai(credentials) - if not success: - logger.error( - f"[permanent_delete_doc] OpenAI API key not configured for org_id={current_user.organization_id}, project_id={current_user.project_id}" - ) - raise HTTPException(status_code=400, detail="OpenAI is not configured") a_crud = OpenAIAssistantCrud(client) d_crud = DocumentCrud(session, current_user.id) diff --git a/backend/app/tests/api/routes/collections/test_collection_info.py b/backend/app/tests/api/routes/collections/test_collection_info.py index ba31baf26..6cb39f068 100644 --- a/backend/app/tests/api/routes/collections/test_collection_info.py +++ b/backend/app/tests/api/routes/collections/test_collection_info.py @@ -4,12 +4,9 @@ from sqlmodel import Session from app.core.config import settings from app.models import Collection -from app.main import app from app.tests.utils.utils import get_user_from_api_key from app.models.collection import CollectionStatus -client = TestClient(app) - def create_collection( db, diff --git a/backend/app/tests/api/routes/collections/test_create_collections.py b/backend/app/tests/api/routes/collections/test_create_collections.py index 686ea2c2a..8e6a855dd 100644 --- a/backend/app/tests/api/routes/collections/test_create_collections.py +++ b/backend/app/tests/api/routes/collections/test_create_collections.py @@ -4,18 +4,15 @@ from sqlmodel import Session from fastapi.testclient import TestClient -from unittest.mock import MagicMock, patch +from unittest.mock import patch from app.core.config import settings from app.tests.utils.document import DocumentStore from app.tests.utils.utils import get_user_from_api_key -from app.main import app from app.crud.collection import CollectionCrud from app.models.collection import CollectionStatus from app.tests.utils.collections_openai_mock import get_mock_openai_client -client = TestClient(app) - @pytest.fixture(autouse=True) def mock_s3(monkeypatch): @@ -45,12 +42,10 @@ def head_object(self, Bucket, Key): class TestCollectionRouteCreate: _n_documents = 5 - @patch("app.api.routes.collections.configure_openai") - @patch("app.api.routes.collections.get_provider_credential") + @patch("app.api.routes.collections.get_openai_client") def test_create_collection_success( self, - mock_get_credential, - mock_configure_openai, + mock_get_openai_client, client: TestClient, db: Session, normal_user_api_key_headers, @@ -70,9 +65,8 @@ def test_create_collection_success( headers = normal_user_api_key_headers - mock_get_credential.return_value = {"api_key": "test_api_key"} mock_openai_client = get_mock_openai_client() - mock_configure_openai.return_value = (mock_openai_client, True) + mock_get_openai_client.return_value = mock_openai_client response = client.post( f"{settings.API_V1_STR}/collections/create", json=body, headers=headers diff --git a/backend/app/tests/api/routes/documents/test_route_document_permanent_remove.py b/backend/app/tests/api/routes/documents/test_route_document_permanent_remove.py index b1ea8f7ac..5773146b3 100644 --- a/backend/app/tests/api/routes/documents/test_route_document_permanent_remove.py +++ b/backend/app/tests/api/routes/documents/test_route_document_permanent_remove.py @@ -43,21 +43,18 @@ def aws_credentials(): @mock_aws class TestDocumentRoutePermanentRemove: @openai_responses.mock() - @patch("app.api.routes.documents.get_provider_credential") - @patch("app.api.routes.documents.configure_openai") - def test_item_is_soft_removed( + @patch("app.api.routes.documents.get_openai_client") + def test_permanent_delete_document_from_s3( self, - mock_configure_openai, - mock_get_credential, + mock_get_openai_client, db: Session, route: Route, crawler: WebCrawler, ): openai_mock = OpenAIMock() with openai_mock.router: - client = OpenAI(api_key="test_key") - mock_get_credential.return_value = {"api_key": "sk-test-key"} - mock_configure_openai.return_value = (client, True) + client = OpenAI(api_key=self.openai_api_key) + mock_get_openai_client.return_value = client # Setup AWS aws = AmazonCloudStorageClient() @@ -73,7 +70,6 @@ def test_item_is_soft_removed( # Delete document response = crawler.delete(route.append(document, suffix="permanent")) - print(response) assert response.is_success db.refresh(document) diff --git a/backend/app/tests/api/routes/documents/test_route_document_remove.py b/backend/app/tests/api/routes/documents/test_route_document_remove.py index 5c6c31752..2d1be6d95 100644 --- a/backend/app/tests/api/routes/documents/test_route_document_remove.py +++ b/backend/app/tests/api/routes/documents/test_route_document_remove.py @@ -24,21 +24,18 @@ def route(): @pytest.mark.usefixtures("openai_credentials") class TestDocumentRouteRemove: @openai_responses.mock() - @patch("app.api.routes.documents.get_provider_credential") - @patch("app.api.routes.documents.configure_openai") + @patch("app.api.routes.documents.get_openai_client") def test_response_is_success( self, - mock_configure_openai, - mock_get_credential, + mock_get_openai_client, db: Session, route: Route, crawler: WebCrawler, ): openai_mock = OpenAIMock() with openai_mock.router: - client = OpenAI(api_key="test_key") - mock_get_credential.return_value = {"api_key": "sk-test-key"} - mock_configure_openai.return_value = (client, True) + client = OpenAI(api_key=self.openai_api_key) + mock_get_openai_client.return_value = client store = DocumentStore(db) response = crawler.get(route.append(store.put())) @@ -46,21 +43,18 @@ def test_response_is_success( assert response.is_success @openai_responses.mock() - @patch("app.api.routes.documents.get_provider_credential") - @patch("app.api.routes.documents.configure_openai") + @patch("app.api.routes.documents.get_openai_client") def test_item_is_soft_removed( self, - mock_configure_openai, - mock_get_credential, + mock_get_openai_client, db: Session, route: Route, crawler: WebCrawler, ): openai_mock = OpenAIMock() with openai_mock.router: - mock_get_credential.return_value = {"api_key": "test-key"} - client = OpenAI(api_key="test_key") - mock_configure_openai.return_value = (client, True) + client = OpenAI(api_key=self.openai_api_key) + mock_get_openai_client.return_value = client store = DocumentStore(db) document = store.put() @@ -73,21 +67,18 @@ def test_item_is_soft_removed( assert result.deleted_at is not None @openai_responses.mock() - @patch("app.api.routes.documents.get_provider_credential") - @patch("app.api.routes.documents.configure_openai") + @patch("app.api.routes.documents.get_openai_client") def test_cannot_remove_unknown_document( self, - mock_configure_openai, - mock_get_credential, + mock_get_openai_client, db: Session, route: Route, crawler: WebCrawler, ): openai_mock = OpenAIMock() with openai_mock.router: - client = OpenAI(api_key="test_key") - mock_get_credential.return_value = {"api_key": "sk-test-key"} - mock_configure_openai.return_value = (client, True) + client = OpenAI(api_key=self.openai_api_key) + mock_get_openai_client.return_value = client DocumentStore.clear(db) maker = DocumentMaker(db) diff --git a/backend/app/tests/crud/collections/test_crud_collection_delete.py b/backend/app/tests/crud/collections/test_crud_collection_delete.py index c4d7c8bb2..a78f3935e 100644 --- a/backend/app/tests/crud/collections/test_crud_collection_delete.py +++ b/backend/app/tests/crud/collections/test_crud_collection_delete.py @@ -17,7 +17,7 @@ class TestCollectionDelete: @openai_responses.mock() def test_delete_marks_deleted(self, db: Session): - client = OpenAI(api_key="test_api_key") + client = OpenAI(api_key=self.openai_api_key) assistant = OpenAIAssistantCrud(client) collection = get_collection(db, client) @@ -29,7 +29,7 @@ def test_delete_marks_deleted(self, db: Session): @openai_responses.mock() def test_delete_follows_insert(self, db: Session): - client = OpenAI(api_key="test_api_key") + client = OpenAI(api_key=self.openai_api_key) assistant = OpenAIAssistantCrud(client) collection = get_collection(db, client) @@ -41,7 +41,7 @@ def test_delete_follows_insert(self, db: Session): @openai_responses.mock() def test_cannot_delete_others_collections(self, db: Session): - client = OpenAI(api_key="test_api_key") + client = OpenAI(api_key=self.openai_api_key) assistant = OpenAIAssistantCrud(client) collection = get_collection(db, client) @@ -56,7 +56,7 @@ def test_delete_document_deletes_collections(self, db: Session): store = DocumentStore(db) documents = store.fill(1) - client = OpenAI(api_key="test_api_key") + client = OpenAI(api_key=self.openai_api_key) resources = [] for _ in range(self._n_collections): coll = get_collection(db, client) diff --git a/backend/app/tests/crud/collections/test_crud_collection_read_all.py b/backend/app/tests/crud/collections/test_crud_collection_read_all.py index 41056f3a9..3dc979088 100644 --- a/backend/app/tests/crud/collections/test_crud_collection_read_all.py +++ b/backend/app/tests/crud/collections/test_crud_collection_read_all.py @@ -11,14 +11,14 @@ from app.tests.utils.utils import openai_credentials -def create_collections(db: Session, n: int): +def create_collections(db: Session, n: int, api_key: str): crud = None store = DocumentStore(db) documents = store.fill(1) openai_mock = OpenAIMock() with openai_mock.router: - client = OpenAI(api_key="test_api_key") + client = OpenAI(api_key=api_key) for _ in range(n): collection = get_collection(db, client) if crud is None: @@ -41,13 +41,13 @@ class TestCollectionReadAll: def test_number_read_is_expected(self, db: Session): db.query(Collection).delete() - owner = create_collections(db, self._ncollections) + owner = create_collections(db, self._ncollections, self.openai_api_key) crud = CollectionCrud(db, owner) docs = crud.read_all() assert len(docs) == self._ncollections def test_deleted_docs_are_excluded(self, db: Session): - owner = create_collections(db, self._ncollections) + owner = create_collections(db, self._ncollections, self.openai_api_key) crud = CollectionCrud(db, owner) assert all(x.deleted_at is None for x in crud.read_all()) diff --git a/backend/app/tests/crud/collections/test_crud_collection_read_one.py b/backend/app/tests/crud/collections/test_crud_collection_read_one.py index 909d1a0c7..4d453dc75 100644 --- a/backend/app/tests/crud/collections/test_crud_collection_read_one.py +++ b/backend/app/tests/crud/collections/test_crud_collection_read_one.py @@ -11,13 +11,13 @@ from app.tests.utils.utils import openai_credentials -def mk_collection(db: Session): +def mk_collection(db: Session, api_key: str): store = DocumentStore(db) documents = store.fill(1) openai_mock = OpenAIMock() with openai_mock.router: - client = OpenAI(api_key="Test_api_key") + client = OpenAI(api_key=api_key) collection = get_collection(db, client) crud = CollectionCrud(db, collection.owner_id) return crud.create(collection, documents) @@ -26,7 +26,7 @@ def mk_collection(db: Session): @pytest.mark.usefixtures("openai_credentials") class TestDatabaseReadOne: def test_can_select_valid_id(self, db: Session): - collection = mk_collection(db) + collection = mk_collection(db, self.openai_api_key) crud = CollectionCrud(db, collection.owner_id) result = crud.read_one(collection.id) @@ -34,7 +34,7 @@ def test_can_select_valid_id(self, db: Session): assert result.id == collection.id def test_cannot_select_others_collections(self, db: Session): - collection = mk_collection(db) + collection = mk_collection(db, self.openai_api_key) other = collection.owner_id + 1 crud = CollectionCrud(db, other) diff --git a/backend/app/tests/utils/utils.py b/backend/app/tests/utils/utils.py index 6543155dc..b4221f525 100644 --- a/backend/app/tests/utils/utils.py +++ b/backend/app/tests/utils/utils.py @@ -3,6 +3,7 @@ from uuid import UUID from typing import Type, TypeVar + import pytest from pydantic import EmailStr from fastapi.testclient import TestClient @@ -18,8 +19,8 @@ @pytest.fixture(scope="class") -def openai_credentials(): - OPENAI_API_KEY = "sk-fake123" +def openai_credentials(request): + request.cls.openai_api_key = "sk-fake123" def random_lower_string() -> str: From e30dcff19993c987bb8251fd2eb5c4f5ee54400e Mon Sep 17 00:00:00 2001 From: nishika26 Date: Thu, 24 Jul 2025 14:22:07 +0530 Subject: [PATCH 15/21] second review fix --- backend/app/api/routes/collections.py | 19 ++++++++++--------- backend/app/api/routes/documents.py | 3 +-- .../test_route_document_permanent_remove.py | 5 ++--- .../documents/test_route_document_remove.py | 8 +++----- .../test_crud_collection_delete.py | 10 ++++------ .../test_crud_collection_read_all.py | 11 ++++------- .../test_crud_collection_read_one.py | 11 ++++------- backend/app/tests/utils/utils.py | 5 ----- 8 files changed, 28 insertions(+), 44 deletions(-) diff --git a/backend/app/api/routes/collections.py b/backend/app/api/routes/collections.py index c571cdc00..8682cb69f 100644 --- a/backend/app/api/routes/collections.py +++ b/backend/app/api/routes/collections.py @@ -13,7 +13,7 @@ from app.api.deps import CurrentUser, SessionDep, CurrentUserOrgProject from app.core.cloud import AmazonCloudStorage -from app.core.util import now, post_callback, configure_openai +from app.core.util import now, post_callback from app.crud import ( DocumentCrud, CollectionCrud, @@ -297,9 +297,10 @@ def create_collection( def do_delete_collection( session: SessionDep, - current_user: CurrentUser, + current_user: CurrentUserOrgProject, request: DeletionRequest, payload: ResponsePayload, + client: OpenAI, ): if request.callback_url is None: callback = SilentCallback(payload) @@ -309,7 +310,7 @@ def do_delete_collection( collection_crud = CollectionCrud(session, current_user.id) try: collection = collection_crud.read_one(request.collection_id) - assistant = OpenAIAssistantCrud() + assistant = OpenAIAssistantCrud(client) data = collection_crud.delete(collection, assistant) logger.info( f"[do_delete_collection] Collection deleted successfully | {{'collection_id': '{collection.id}'}}" @@ -335,20 +336,20 @@ def do_delete_collection( ) def delete_collection( session: SessionDep, - current_user: CurrentUser, + current_user: CurrentUserOrgProject, request: DeletionRequest, background_tasks: BackgroundTasks, ): + client = get_openai_client( + session, current_user.organization_id, current_user.project_id + ) + this = inspect.currentframe() route = router.url_path_for(this.f_code.co_name) payload = ResponsePayload("processing", route) background_tasks.add_task( - do_delete_collection, - session, - current_user, - request, - payload, + do_delete_collection, session, current_user, request, payload, client ) logger.info( diff --git a/backend/app/api/routes/documents.py b/backend/app/api/routes/documents.py index c9c49f890..3924cabf5 100644 --- a/backend/app/api/routes/documents.py +++ b/backend/app/api/routes/documents.py @@ -3,12 +3,11 @@ from typing import List from pathlib import Path -from fastapi import APIRouter, File, UploadFile, Query, HTTPException +from fastapi import APIRouter, File, UploadFile, Query from fastapi import Path as FastPath from app.crud import DocumentCrud, CollectionCrud from app.models import Document -from app.core.util import configure_openai from app.utils import APIResponse, load_description, get_openai_client from app.api.deps import CurrentUser, SessionDep, CurrentUserOrgProject from app.core.cloud import AmazonCloudStorage diff --git a/backend/app/tests/api/routes/documents/test_route_document_permanent_remove.py b/backend/app/tests/api/routes/documents/test_route_document_permanent_remove.py index 5773146b3..10d11f3ec 100644 --- a/backend/app/tests/api/routes/documents/test_route_document_permanent_remove.py +++ b/backend/app/tests/api/routes/documents/test_route_document_permanent_remove.py @@ -22,7 +22,6 @@ WebCrawler, crawler, ) -from app.tests.utils.utils import openai_credentials @pytest.fixture @@ -39,7 +38,7 @@ def aws_credentials(): os.environ["AWS_DEFAULT_REGION"] = settings.AWS_DEFAULT_REGION -@pytest.mark.usefixtures("openai_credentials", "aws_credentials") +@pytest.mark.usefixtures("aws_credentials") @mock_aws class TestDocumentRoutePermanentRemove: @openai_responses.mock() @@ -53,7 +52,7 @@ def test_permanent_delete_document_from_s3( ): openai_mock = OpenAIMock() with openai_mock.router: - client = OpenAI(api_key=self.openai_api_key) + client = OpenAI(api_key="sk-test-key") mock_get_openai_client.return_value = client # Setup AWS diff --git a/backend/app/tests/api/routes/documents/test_route_document_remove.py b/backend/app/tests/api/routes/documents/test_route_document_remove.py index 2d1be6d95..292b2b10a 100644 --- a/backend/app/tests/api/routes/documents/test_route_document_remove.py +++ b/backend/app/tests/api/routes/documents/test_route_document_remove.py @@ -13,7 +13,6 @@ WebCrawler, crawler, ) -from app.tests.utils.utils import openai_credentials @pytest.fixture @@ -21,7 +20,6 @@ def route(): return Route("remove") -@pytest.mark.usefixtures("openai_credentials") class TestDocumentRouteRemove: @openai_responses.mock() @patch("app.api.routes.documents.get_openai_client") @@ -34,7 +32,7 @@ def test_response_is_success( ): openai_mock = OpenAIMock() with openai_mock.router: - client = OpenAI(api_key=self.openai_api_key) + client = OpenAI(api_key="sk-test-key") mock_get_openai_client.return_value = client store = DocumentStore(db) @@ -53,7 +51,7 @@ def test_item_is_soft_removed( ): openai_mock = OpenAIMock() with openai_mock.router: - client = OpenAI(api_key=self.openai_api_key) + client = OpenAI(api_key="sk-test-key") mock_get_openai_client.return_value = client store = DocumentStore(db) @@ -77,7 +75,7 @@ def test_cannot_remove_unknown_document( ): openai_mock = OpenAIMock() with openai_mock.router: - client = OpenAI(api_key=self.openai_api_key) + client = OpenAI(api_key="sk-test-key") mock_get_openai_client.return_value = client DocumentStore.clear(db) diff --git a/backend/app/tests/crud/collections/test_crud_collection_delete.py b/backend/app/tests/crud/collections/test_crud_collection_delete.py index a78f3935e..8029c5e80 100644 --- a/backend/app/tests/crud/collections/test_crud_collection_delete.py +++ b/backend/app/tests/crud/collections/test_crud_collection_delete.py @@ -8,16 +8,14 @@ from app.crud.rag import OpenAIAssistantCrud from app.tests.utils.document import DocumentStore from app.tests.utils.collection import get_collection, uuid_increment -from app.tests.utils.utils import openai_credentials -@pytest.mark.usefixtures("openai_credentials") class TestCollectionDelete: _n_collections = 5 @openai_responses.mock() def test_delete_marks_deleted(self, db: Session): - client = OpenAI(api_key=self.openai_api_key) + client = OpenAI(api_key="sk-test-key") assistant = OpenAIAssistantCrud(client) collection = get_collection(db, client) @@ -29,7 +27,7 @@ def test_delete_marks_deleted(self, db: Session): @openai_responses.mock() def test_delete_follows_insert(self, db: Session): - client = OpenAI(api_key=self.openai_api_key) + client = OpenAI(api_key="sk-test-key") assistant = OpenAIAssistantCrud(client) collection = get_collection(db, client) @@ -41,7 +39,7 @@ def test_delete_follows_insert(self, db: Session): @openai_responses.mock() def test_cannot_delete_others_collections(self, db: Session): - client = OpenAI(api_key=self.openai_api_key) + client = OpenAI(api_key="sk-test-key") assistant = OpenAIAssistantCrud(client) collection = get_collection(db, client) @@ -56,7 +54,7 @@ def test_delete_document_deletes_collections(self, db: Session): store = DocumentStore(db) documents = store.fill(1) - client = OpenAI(api_key=self.openai_api_key) + client = OpenAI(api_key="sk-test-key") resources = [] for _ in range(self._n_collections): coll = get_collection(db, client) diff --git a/backend/app/tests/crud/collections/test_crud_collection_read_all.py b/backend/app/tests/crud/collections/test_crud_collection_read_all.py index 3dc979088..b6f63b884 100644 --- a/backend/app/tests/crud/collections/test_crud_collection_read_all.py +++ b/backend/app/tests/crud/collections/test_crud_collection_read_all.py @@ -4,21 +4,19 @@ from sqlmodel import Session from app.crud import CollectionCrud -from app.core.config import settings from app.models import Collection from app.tests.utils.document import DocumentStore from app.tests.utils.collection import get_collection -from app.tests.utils.utils import openai_credentials -def create_collections(db: Session, n: int, api_key: str): +def create_collections(db: Session, n: int): crud = None store = DocumentStore(db) documents = store.fill(1) openai_mock = OpenAIMock() with openai_mock.router: - client = OpenAI(api_key=api_key) + client = OpenAI(api_key="sk-test-key") for _ in range(n): collection = get_collection(db, client) if crud is None: @@ -34,20 +32,19 @@ def refresh(self, db: Session): db.commit() -@pytest.mark.usefixtures("openai_credentials") class TestCollectionReadAll: _ncollections = 5 def test_number_read_is_expected(self, db: Session): db.query(Collection).delete() - owner = create_collections(db, self._ncollections, self.openai_api_key) + owner = create_collections(db, self._ncollections) crud = CollectionCrud(db, owner) docs = crud.read_all() assert len(docs) == self._ncollections def test_deleted_docs_are_excluded(self, db: Session): - owner = create_collections(db, self._ncollections, self.openai_api_key) + owner = create_collections(db, self._ncollections) crud = CollectionCrud(db, owner) assert all(x.deleted_at is None for x in crud.read_all()) diff --git a/backend/app/tests/crud/collections/test_crud_collection_read_one.py b/backend/app/tests/crud/collections/test_crud_collection_read_one.py index 4d453dc75..ed0d31ad4 100644 --- a/backend/app/tests/crud/collections/test_crud_collection_read_one.py +++ b/backend/app/tests/crud/collections/test_crud_collection_read_one.py @@ -8,25 +8,23 @@ from app.crud import CollectionCrud from app.tests.utils.document import DocumentStore from app.tests.utils.collection import get_collection, uuid_increment -from app.tests.utils.utils import openai_credentials -def mk_collection(db: Session, api_key: str): +def mk_collection(db: Session): store = DocumentStore(db) documents = store.fill(1) openai_mock = OpenAIMock() with openai_mock.router: - client = OpenAI(api_key=api_key) + client = OpenAI(api_key="sk-test-key") collection = get_collection(db, client) crud = CollectionCrud(db, collection.owner_id) return crud.create(collection, documents) -@pytest.mark.usefixtures("openai_credentials") class TestDatabaseReadOne: def test_can_select_valid_id(self, db: Session): - collection = mk_collection(db, self.openai_api_key) + collection = mk_collection(db) crud = CollectionCrud(db, collection.owner_id) result = crud.read_one(collection.id) @@ -34,8 +32,7 @@ def test_can_select_valid_id(self, db: Session): assert result.id == collection.id def test_cannot_select_others_collections(self, db: Session): - collection = mk_collection(db, self.openai_api_key) - + collection = mk_collection(db) other = collection.owner_id + 1 crud = CollectionCrud(db, other) with pytest.raises(NoResultFound): diff --git a/backend/app/tests/utils/utils.py b/backend/app/tests/utils/utils.py index b4221f525..df436a602 100644 --- a/backend/app/tests/utils/utils.py +++ b/backend/app/tests/utils/utils.py @@ -18,11 +18,6 @@ T = TypeVar("T") -@pytest.fixture(scope="class") -def openai_credentials(request): - request.cls.openai_api_key = "sk-fake123" - - def random_lower_string() -> str: return "".join(random.choices(string.ascii_lowercase, k=32)) From f633a5d7e817197b1d35e451368ae4a878c81156 Mon Sep 17 00:00:00 2001 From: nishika26 Date: Thu, 24 Jul 2025 14:28:31 +0530 Subject: [PATCH 16/21] create collection crud file --- .../app/tests/crud/collections/test_crud_collection_create.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/backend/app/tests/crud/collections/test_crud_collection_create.py b/backend/app/tests/crud/collections/test_crud_collection_create.py index 564230b33..d6feb064f 100644 --- a/backend/app/tests/crud/collections/test_crud_collection_create.py +++ b/backend/app/tests/crud/collections/test_crud_collection_create.py @@ -1,4 +1,3 @@ -import pytest import openai_responses from sqlmodel import Session, select @@ -6,10 +5,8 @@ from app.models import DocumentCollection from app.tests.utils.document import DocumentStore from app.tests.utils.collection import get_collection -from app.tests.utils.utils import openai_credentials -@pytest.mark.usefixtures("openai_credentials") class TestCollectionCreate: _n_documents = 10 From 3454a0064af73cac9af3a89733b65c9bb084f384 Mon Sep 17 00:00:00 2001 From: nishika26 Date: Thu, 24 Jul 2025 14:53:48 +0530 Subject: [PATCH 17/21] logging --- backend/app/crud/rag/open_ai.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/backend/app/crud/rag/open_ai.py b/backend/app/crud/rag/open_ai.py index 21ff61ab9..cdb644abc 100644 --- a/backend/app/crud/rag/open_ai.py +++ b/backend/app/crud/rag/open_ai.py @@ -92,7 +92,9 @@ def clean(self, resource): class OpenAICrud: def __init__(self, client): if client is None: + logger.error("[OpenAICrud] OpenAI client is not configured") raise ValueError("OpenAI client is not configured") + self.client = client From 4b5d8d8cc289dcd950ad99fd8f29c7cc2dd67aa7 Mon Sep 17 00:00:00 2001 From: nishika26 Date: Fri, 25 Jul 2025 10:33:52 +0530 Subject: [PATCH 18/21] formatting --- .../tests/api/routes/collections/test_create_collections.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/backend/app/tests/api/routes/collections/test_create_collections.py b/backend/app/tests/api/routes/collections/test_create_collections.py index 8e6a855dd..49b429e66 100644 --- a/backend/app/tests/api/routes/collections/test_create_collections.py +++ b/backend/app/tests/api/routes/collections/test_create_collections.py @@ -29,7 +29,7 @@ def stream(self, file_obj): return fake_file def get_file_size_kb(self, url: str) -> float: - return 1.0 # Simulate 1KB files + return 1.0 class FakeS3Client: def head_object(self, Bucket, Key): @@ -50,7 +50,6 @@ def test_create_collection_success( db: Session, normal_user_api_key_headers, ): - # Setup test documents store = DocumentStore(db) documents = store.fill(self._n_documents) doc_ids = [str(doc.id) for doc in documents] From 5b45d6add8d3db69fd524a1ab73c549902fa4d36 Mon Sep 17 00:00:00 2001 From: nishika26 Date: Fri, 25 Jul 2025 10:46:07 +0530 Subject: [PATCH 19/21] normal user to user --- .../tests/api/routes/collections/test_create_collections.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/backend/app/tests/api/routes/collections/test_create_collections.py b/backend/app/tests/api/routes/collections/test_create_collections.py index 49b429e66..bcc48283d 100644 --- a/backend/app/tests/api/routes/collections/test_create_collections.py +++ b/backend/app/tests/api/routes/collections/test_create_collections.py @@ -48,7 +48,7 @@ def test_create_collection_success( mock_get_openai_client, client: TestClient, db: Session, - normal_user_api_key_headers, + user_api_key_header, ): store = DocumentStore(db) documents = store.fill(self._n_documents) @@ -62,7 +62,7 @@ def test_create_collection_success( "temperature": 0.1, } - headers = normal_user_api_key_headers + headers = user_api_key_header mock_openai_client = get_mock_openai_client() mock_get_openai_client.return_value = mock_openai_client From d4afb586d87eefe4adc64941cfa19335ffbe33b3 Mon Sep 17 00:00:00 2001 From: nishika26 Date: Fri, 25 Jul 2025 14:38:01 +0530 Subject: [PATCH 20/21] normal user to user --- .../tests/api/routes/collections/test_create_collections.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/backend/app/tests/api/routes/collections/test_create_collections.py b/backend/app/tests/api/routes/collections/test_create_collections.py index bcc48283d..2bede71aa 100644 --- a/backend/app/tests/api/routes/collections/test_create_collections.py +++ b/backend/app/tests/api/routes/collections/test_create_collections.py @@ -11,7 +11,7 @@ from app.tests.utils.utils import get_user_from_api_key from app.crud.collection import CollectionCrud from app.models.collection import CollectionStatus -from app.tests.utils.collections_openai_mock import get_mock_openai_client +from app.tests.utils.openai import get_mock_openai_client_with_vector_store @pytest.fixture(autouse=True) @@ -64,7 +64,7 @@ def test_create_collection_success( headers = user_api_key_header - mock_openai_client = get_mock_openai_client() + mock_openai_client = get_mock_openai_client_with_vector_store() mock_get_openai_client.return_value = mock_openai_client response = client.post( From 016aa1648d198e99a0c4091575b7ca10cfe07bae Mon Sep 17 00:00:00 2001 From: nishika26 Date: Fri, 25 Jul 2025 14:38:51 +0530 Subject: [PATCH 21/21] moving the util function --- .../tests/utils/collections_openai_mock.py | 31 ------------------- backend/app/tests/utils/openai.py | 31 +++++++++++++++++++ 2 files changed, 31 insertions(+), 31 deletions(-) delete mode 100644 backend/app/tests/utils/collections_openai_mock.py diff --git a/backend/app/tests/utils/collections_openai_mock.py b/backend/app/tests/utils/collections_openai_mock.py deleted file mode 100644 index 7f9c064b2..000000000 --- a/backend/app/tests/utils/collections_openai_mock.py +++ /dev/null @@ -1,31 +0,0 @@ -from unittest.mock import MagicMock - - -def get_mock_openai_client(): - mock_client = MagicMock() - - # Vector store - mock_vector_store = MagicMock() - mock_vector_store.id = "mock_vector_store_id" - mock_client.vector_stores.create.return_value = mock_vector_store - - # File upload + polling - mock_file_batch = MagicMock() - mock_file_batch.file_counts.completed = 2 - mock_file_batch.file_counts.total = 2 - mock_client.vector_stores.file_batches.upload_and_poll.return_value = ( - mock_file_batch - ) - - # File list - mock_client.vector_stores.files.list.return_value = {"data": []} - - # Assistant - mock_assistant = MagicMock() - mock_assistant.id = "mock_assistant_id" - mock_assistant.name = "Mock Assistant" - mock_assistant.model = "gpt-4o" - mock_assistant.instructions = "Mock instructions" - mock_client.beta.assistants.create.return_value = mock_assistant - - return mock_client diff --git a/backend/app/tests/utils/openai.py b/backend/app/tests/utils/openai.py index 778d4804f..6f11bbf51 100644 --- a/backend/app/tests/utils/openai.py +++ b/backend/app/tests/utils/openai.py @@ -1,6 +1,7 @@ from typing import Optional import time +from unittest.mock import MagicMock from openai.types.beta import Assistant as OpenAIAssistant from openai.types.beta.assistant import ToolResources, ToolResourcesFileSearch from openai.types.beta.assistant_tool import FileSearchTool @@ -37,3 +38,33 @@ def mock_openai_assistant( top_p=1.0, reasoning_effort=None, ) + + +def get_mock_openai_client_with_vector_store(): + mock_client = MagicMock() + + # Vector store + mock_vector_store = MagicMock() + mock_vector_store.id = "mock_vector_store_id" + mock_client.vector_stores.create.return_value = mock_vector_store + + # File upload + polling + mock_file_batch = MagicMock() + mock_file_batch.file_counts.completed = 2 + mock_file_batch.file_counts.total = 2 + mock_client.vector_stores.file_batches.upload_and_poll.return_value = ( + mock_file_batch + ) + + # File list + mock_client.vector_stores.files.list.return_value = {"data": []} + + # Assistant + mock_assistant = MagicMock() + mock_assistant.id = "mock_assistant_id" + mock_assistant.name = "Mock Assistant" + mock_assistant.model = "gpt-4o" + mock_assistant.instructions = "Mock instructions" + mock_client.beta.assistants.create.return_value = mock_assistant + + return mock_client