perf(onedrive): use async httpx for token refresh
Replaces the synchronous `requests.post` and `requests.get` calls in `app/api/onedrive.py:test_onedrive_token` with an asynchronous `httpx.AsyncClient` implementation. This unblocks the FastAPI event loop when this endpoint is hit. Tests were updated to mock `httpx.AsyncClient` and a sync wrapper using `asyncio.run` was added to integration tests to maintain test coverage without massive test refactoring. Co-authored-by: christianlouis <361235+christianlouis@users.noreply.github.com>
This commit is contained in:
+5
-2
@@ -7,6 +7,7 @@ import os
|
|||||||
from datetime import datetime, timedelta
|
from datetime import datetime, timedelta
|
||||||
from typing import Annotated, Optional
|
from typing import Annotated, Optional
|
||||||
|
|
||||||
|
import httpx
|
||||||
import requests
|
import requests
|
||||||
from fastapi import APIRouter, Depends, Form, HTTPException, Request, status
|
from fastapi import APIRouter, Depends, Form, HTTPException, Request, status
|
||||||
from sqlalchemy.orm import Session
|
from sqlalchemy.orm import Session
|
||||||
@@ -92,7 +93,8 @@ async def test_onedrive_token(request: Request):
|
|||||||
"scope": "offline_access Files.ReadWrite",
|
"scope": "offline_access Files.ReadWrite",
|
||||||
}
|
}
|
||||||
|
|
||||||
response = requests.post(token_url, data=refresh_data, timeout=settings.http_request_timeout)
|
async with httpx.AsyncClient(timeout=settings.http_request_timeout) as client:
|
||||||
|
response = await client.post(token_url, data=refresh_data)
|
||||||
|
|
||||||
if response.status_code != 200:
|
if response.status_code != 200:
|
||||||
logger.error(f"Failed to refresh OneDrive token: {response.text}")
|
logger.error(f"Failed to refresh OneDrive token: {response.text}")
|
||||||
@@ -164,7 +166,8 @@ async def test_onedrive_token(request: Request):
|
|||||||
user_info_url = "https://graph.microsoft.com/v1.0/me"
|
user_info_url = "https://graph.microsoft.com/v1.0/me"
|
||||||
headers = {"Authorization": f"Bearer {access_token}"}
|
headers = {"Authorization": f"Bearer {access_token}"}
|
||||||
|
|
||||||
user_response = requests.get(user_info_url, headers=headers, timeout=settings.http_request_timeout)
|
async with httpx.AsyncClient(timeout=settings.http_request_timeout) as client:
|
||||||
|
user_response = await client.get(user_info_url, headers=headers)
|
||||||
|
|
||||||
if user_response.status_code != 200:
|
if user_response.status_code != 200:
|
||||||
logger.error(f"OneDrive token test failed: {user_response.status_code} {user_response.text}")
|
logger.error(f"OneDrive token test failed: {user_response.status_code} {user_response.text}")
|
||||||
|
|||||||
@@ -0,0 +1,69 @@
|
|||||||
|
import asyncio
|
||||||
|
import time
|
||||||
|
import httpx
|
||||||
|
from unittest.mock import patch, MagicMock, AsyncMock
|
||||||
|
from app.api.onedrive import test_onedrive_token
|
||||||
|
from app.config import settings
|
||||||
|
|
||||||
|
settings.onedrive_refresh_token = "dummy"
|
||||||
|
settings.onedrive_client_id = "dummy"
|
||||||
|
settings.onedrive_client_secret = "dummy"
|
||||||
|
|
||||||
|
class DummyRequest:
|
||||||
|
def __init__(self):
|
||||||
|
self.session = {"user": "dummy"}
|
||||||
|
|
||||||
|
async def run_benchmark(func_name, mock_post, mock_get):
|
||||||
|
mock_post_resp = MagicMock()
|
||||||
|
mock_post_resp.status_code = 200
|
||||||
|
mock_post_resp.json.return_value = {
|
||||||
|
"access_token": "dummy_access",
|
||||||
|
"expires_in": 3600
|
||||||
|
}
|
||||||
|
mock_post.return_value = mock_post_resp
|
||||||
|
|
||||||
|
mock_get_resp = MagicMock()
|
||||||
|
mock_get_resp.status_code = 200
|
||||||
|
mock_get_resp.json.return_value = {
|
||||||
|
"displayName": "Test User",
|
||||||
|
"userPrincipalName": "test@example.com"
|
||||||
|
}
|
||||||
|
mock_get.return_value = mock_get_resp
|
||||||
|
|
||||||
|
start_time = time.time()
|
||||||
|
for _ in range(100):
|
||||||
|
await test_onedrive_token(DummyRequest())
|
||||||
|
end_time = time.time()
|
||||||
|
print(f"{func_name} took {end_time - start_time:.4f} seconds")
|
||||||
|
|
||||||
|
async def run_benchmark_async(func_name, mock_post, mock_get):
|
||||||
|
mock_post_resp = MagicMock()
|
||||||
|
mock_post_resp.status_code = 200
|
||||||
|
mock_post_resp.json = MagicMock(return_value={
|
||||||
|
"access_token": "dummy_access",
|
||||||
|
"expires_in": 3600
|
||||||
|
})
|
||||||
|
mock_post.return_value = mock_post_resp
|
||||||
|
|
||||||
|
mock_get_resp = MagicMock()
|
||||||
|
mock_get_resp.status_code = 200
|
||||||
|
mock_get_resp.json = MagicMock(return_value={
|
||||||
|
"displayName": "Test User",
|
||||||
|
"userPrincipalName": "test@example.com"
|
||||||
|
})
|
||||||
|
mock_get.return_value = mock_get_resp
|
||||||
|
|
||||||
|
start_time = time.time()
|
||||||
|
for _ in range(100):
|
||||||
|
await test_onedrive_token(DummyRequest())
|
||||||
|
end_time = time.time()
|
||||||
|
print(f"{func_name} took {end_time - start_time:.4f} seconds")
|
||||||
|
|
||||||
|
|
||||||
|
@patch('app.api.onedrive.requests.get')
|
||||||
|
@patch('app.api.onedrive.requests.post')
|
||||||
|
def benchmark_sync(mock_post, mock_get):
|
||||||
|
asyncio.run(run_benchmark("Sync requests (baseline)", mock_post, mock_get))
|
||||||
|
|
||||||
|
if __name__ == "__main__":
|
||||||
|
benchmark_sync()
|
||||||
@@ -5,7 +5,7 @@ Focuses on uncovered lines: 98-99, 121-143, 160-161, 170-171,
|
|||||||
324-326, 400-402, 436-438.
|
324-326, 400-402, 436-438.
|
||||||
"""
|
"""
|
||||||
|
|
||||||
from unittest.mock import MagicMock, PropertyMock, patch
|
from unittest.mock import MagicMock, PropertyMock, patch, AsyncMock
|
||||||
|
|
||||||
import pytest
|
import pytest
|
||||||
from fastapi.testclient import TestClient
|
from fastapi.testclient import TestClient
|
||||||
@@ -15,7 +15,7 @@ from fastapi.testclient import TestClient
|
|||||||
class TestTestTokenRefreshFailed:
|
class TestTestTokenRefreshFailed:
|
||||||
"""Cover lines 98-99: token refresh returns non-200."""
|
"""Cover lines 98-99: token refresh returns non-200."""
|
||||||
|
|
||||||
@patch("app.api.onedrive.requests.post")
|
@patch("httpx.AsyncClient.post", new_callable=AsyncMock)
|
||||||
def test_test_token_refresh_returns_non_200(self, mock_post, client: TestClient):
|
def test_test_token_refresh_returns_non_200(self, mock_post, client: TestClient):
|
||||||
"""Test token refresh returning a failure status hits the error branch."""
|
"""Test token refresh returning a failure status hits the error branch."""
|
||||||
from app.config import settings
|
from app.config import settings
|
||||||
@@ -42,8 +42,8 @@ class TestTestTokenRefreshFailed:
|
|||||||
class TestTestTokenRotation:
|
class TestTestTokenRotation:
|
||||||
"""Cover lines 121-143, 160-161: token rotation with .env and DB persist."""
|
"""Cover lines 121-143, 160-161: token rotation with .env and DB persist."""
|
||||||
|
|
||||||
@patch("app.api.onedrive.requests.get")
|
@patch("httpx.AsyncClient.get", new_callable=AsyncMock)
|
||||||
@patch("app.api.onedrive.requests.post")
|
@patch("httpx.AsyncClient.post", new_callable=AsyncMock)
|
||||||
def test_token_rotation_env_file_exists(self, mock_post, mock_get, client: TestClient, tmp_path):
|
def test_token_rotation_env_file_exists(self, mock_post, mock_get, client: TestClient, tmp_path):
|
||||||
"""When a new refresh token is received and .env file exists, it should be updated."""
|
"""When a new refresh token is received and .env file exists, it should be updated."""
|
||||||
from app.config import settings
|
from app.config import settings
|
||||||
@@ -90,8 +90,8 @@ class TestTestTokenRotation:
|
|||||||
data = response.json()
|
data = response.json()
|
||||||
assert data["status"] == "success"
|
assert data["status"] == "success"
|
||||||
|
|
||||||
@patch("app.api.onedrive.requests.get")
|
@patch("httpx.AsyncClient.get", new_callable=AsyncMock)
|
||||||
@patch("app.api.onedrive.requests.post")
|
@patch("httpx.AsyncClient.post", new_callable=AsyncMock)
|
||||||
def test_token_rotation_env_not_existing(self, mock_post, mock_get, client: TestClient):
|
def test_token_rotation_env_not_existing(self, mock_post, mock_get, client: TestClient):
|
||||||
"""Token rotation when .env doesn't exist still succeeds."""
|
"""Token rotation when .env doesn't exist still succeeds."""
|
||||||
from app.config import settings
|
from app.config import settings
|
||||||
@@ -130,8 +130,8 @@ class TestTestTokenRotation:
|
|||||||
assert response.status_code == 200
|
assert response.status_code == 200
|
||||||
assert response.json()["status"] == "success"
|
assert response.json()["status"] == "success"
|
||||||
|
|
||||||
@patch("app.api.onedrive.requests.get")
|
@patch("httpx.AsyncClient.get", new_callable=AsyncMock)
|
||||||
@patch("app.api.onedrive.requests.post")
|
@patch("httpx.AsyncClient.post", new_callable=AsyncMock)
|
||||||
def test_token_rotation_env_write_failure(self, mock_post, mock_get, client: TestClient):
|
def test_token_rotation_env_write_failure(self, mock_post, mock_get, client: TestClient):
|
||||||
"""Token rotation when .env write fails (lines 142-143) still continues."""
|
"""Token rotation when .env write fails (lines 142-143) still continues."""
|
||||||
from app.config import settings
|
from app.config import settings
|
||||||
@@ -171,8 +171,8 @@ class TestTestTokenRotation:
|
|||||||
assert response.status_code == 200
|
assert response.status_code == 200
|
||||||
assert response.json()["status"] == "success"
|
assert response.json()["status"] == "success"
|
||||||
|
|
||||||
@patch("app.api.onedrive.requests.get")
|
@patch("httpx.AsyncClient.get", new_callable=AsyncMock)
|
||||||
@patch("app.api.onedrive.requests.post")
|
@patch("httpx.AsyncClient.post", new_callable=AsyncMock)
|
||||||
def test_token_rotation_db_persist_failure(self, mock_post, mock_get, client: TestClient):
|
def test_token_rotation_db_persist_failure(self, mock_post, mock_get, client: TestClient):
|
||||||
"""Token rotation when DB persist fails (lines 160-161) still continues."""
|
"""Token rotation when DB persist fails (lines 160-161) still continues."""
|
||||||
from app.config import settings
|
from app.config import settings
|
||||||
@@ -211,8 +211,8 @@ class TestTestTokenRotation:
|
|||||||
class TestTestTokenUserInfoFailed:
|
class TestTestTokenUserInfoFailed:
|
||||||
"""Cover lines 170-171: user info request fails."""
|
"""Cover lines 170-171: user info request fails."""
|
||||||
|
|
||||||
@patch("app.api.onedrive.requests.get")
|
@patch("httpx.AsyncClient.get", new_callable=AsyncMock)
|
||||||
@patch("app.api.onedrive.requests.post")
|
@patch("httpx.AsyncClient.post", new_callable=AsyncMock)
|
||||||
def test_user_info_returns_non_200(self, mock_post, mock_get, client: TestClient):
|
def test_user_info_returns_non_200(self, mock_post, mock_get, client: TestClient):
|
||||||
"""Test when user info request fails after successful token refresh."""
|
"""Test when user info request fails after successful token refresh."""
|
||||||
from app.config import settings
|
from app.config import settings
|
||||||
@@ -247,8 +247,8 @@ class TestTestTokenUserInfoFailed:
|
|||||||
class TestTokenRotationEnvAppendLine:
|
class TestTokenRotationEnvAppendLine:
|
||||||
"""Cover the branch at line 134 where token line is not found in .env and must be appended."""
|
"""Cover the branch at line 134 where token line is not found in .env and must be appended."""
|
||||||
|
|
||||||
@patch("app.api.onedrive.requests.get")
|
@patch("httpx.AsyncClient.get", new_callable=AsyncMock)
|
||||||
@patch("app.api.onedrive.requests.post")
|
@patch("httpx.AsyncClient.post", new_callable=AsyncMock)
|
||||||
def test_token_rotation_appends_to_env(self, mock_post, mock_get, client: TestClient, tmp_path):
|
def test_token_rotation_appends_to_env(self, mock_post, mock_get, client: TestClient, tmp_path):
|
||||||
"""When .env exists but doesn't have ONEDRIVE_REFRESH_TOKEN, it should append."""
|
"""When .env exists but doesn't have ONEDRIVE_REFRESH_TOKEN, it should append."""
|
||||||
from app.config import settings
|
from app.config import settings
|
||||||
|
|||||||
@@ -417,14 +417,19 @@ class TestOneDriveIntegration:
|
|||||||
|
|
||||||
def test_onedrive_token_refresh_and_user_info(self, original_env: dict) -> None:
|
def test_onedrive_token_refresh_and_user_info(self, original_env: dict) -> None:
|
||||||
"""Validate token refresh and user info retrieval."""
|
"""Validate token refresh and user info retrieval."""
|
||||||
import requests
|
import asyncio
|
||||||
|
import httpx
|
||||||
|
|
||||||
token = self._get_access_token(original_env)
|
token = self._get_access_token(original_env)
|
||||||
resp = requests.get(
|
|
||||||
|
async def _test():
|
||||||
|
async with httpx.AsyncClient(timeout=30) as client:
|
||||||
|
return await client.get(
|
||||||
"https://graph.microsoft.com/v1.0/me",
|
"https://graph.microsoft.com/v1.0/me",
|
||||||
headers={"Authorization": f"Bearer {token}"},
|
headers={"Authorization": f"Bearer {token}"},
|
||||||
timeout=30,
|
|
||||||
)
|
)
|
||||||
|
|
||||||
|
resp = asyncio.run(_test())
|
||||||
assert resp.status_code == 200, f"OneDrive user info failed: {resp.text}"
|
assert resp.status_code == 200, f"OneDrive user info failed: {resp.text}"
|
||||||
|
|
||||||
def test_onedrive_upload_download_delete(self, original_env: dict) -> None:
|
def test_onedrive_upload_download_delete(self, original_env: dict) -> None:
|
||||||
|
|||||||
Reference in New Issue
Block a user