Merge pull request #714 from christianlouis/perf/async-onedrive-token-refresh-7215258063318287629
⚡ Async OneDrive token validation using httpx
This commit is contained in:
+21
-19
@@ -6,7 +6,7 @@ import logging
|
||||
from datetime import datetime, timedelta
|
||||
from typing import Annotated, Optional
|
||||
|
||||
import requests
|
||||
import httpx
|
||||
from fastapi import APIRouter, Depends, Form, HTTPException, Request, status
|
||||
from sqlalchemy.orm import Session
|
||||
|
||||
@@ -92,17 +92,18 @@ async def test_onedrive_token(request: Request):
|
||||
"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:
|
||||
logger.error(f"Failed to refresh OneDrive token: {response.text}")
|
||||
return {
|
||||
"status": "error",
|
||||
"message": "Refresh token has expired or is invalid",
|
||||
"needs_reauth": True,
|
||||
}
|
||||
if response.status_code != 200:
|
||||
logger.error(f"Failed to refresh OneDrive token: {response.text}")
|
||||
return {
|
||||
"status": "error",
|
||||
"message": "Refresh token has expired or is invalid",
|
||||
"needs_reauth": True,
|
||||
}
|
||||
|
||||
token_data = response.json()
|
||||
token_data = response.json()
|
||||
access_token = token_data.get("access_token")
|
||||
expires_in = token_data.get("expires_in", 3600) # Default to 1 hour if not specified
|
||||
|
||||
@@ -139,17 +140,18 @@ async def test_onedrive_token(request: Request):
|
||||
user_info_url = "https://graph.microsoft.com/v1.0/me"
|
||||
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:
|
||||
logger.error(f"OneDrive token test failed: {user_response.status_code} {user_response.text}")
|
||||
return {
|
||||
"status": "error",
|
||||
"message": f"Token validation failed with status {user_response.status_code}: {user_response.text}",
|
||||
}
|
||||
if user_response.status_code != 200:
|
||||
logger.error(f"OneDrive token test failed: {user_response.status_code} {user_response.text}")
|
||||
return {
|
||||
"status": "error",
|
||||
"message": f"Token validation failed with status {user_response.status_code}: {user_response.text}",
|
||||
}
|
||||
|
||||
# Get user info
|
||||
user_info = user_response.json()
|
||||
# Get user info
|
||||
user_info = user_response.json()
|
||||
display_name = user_info.get("displayName", "Unknown user")
|
||||
email = user_info.get("userPrincipalName", "Unknown email")
|
||||
|
||||
|
||||
@@ -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.
|
||||
"""
|
||||
|
||||
from unittest.mock import MagicMock, PropertyMock, patch
|
||||
from unittest.mock import AsyncMock, MagicMock, PropertyMock, patch
|
||||
|
||||
import pytest
|
||||
from fastapi.testclient import TestClient
|
||||
@@ -15,7 +15,7 @@ from fastapi.testclient import TestClient
|
||||
class TestTestTokenRefreshFailed:
|
||||
"""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):
|
||||
"""Test token refresh returning a failure status hits the error branch."""
|
||||
from app.config import settings
|
||||
@@ -42,8 +42,8 @@ class TestTestTokenRefreshFailed:
|
||||
class TestTestTokenRotation:
|
||||
"""Cover lines 121-143, 160-161: token rotation with .env and DB persist."""
|
||||
|
||||
@patch("app.api.onedrive.requests.get")
|
||||
@patch("app.api.onedrive.requests.post")
|
||||
@patch("httpx.AsyncClient.get", new_callable=AsyncMock)
|
||||
@patch("httpx.AsyncClient.post", new_callable=AsyncMock)
|
||||
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."""
|
||||
from app.config import settings
|
||||
@@ -90,8 +90,8 @@ class TestTestTokenRotation:
|
||||
data = response.json()
|
||||
assert data["status"] == "success"
|
||||
|
||||
@patch("app.api.onedrive.requests.get")
|
||||
@patch("app.api.onedrive.requests.post")
|
||||
@patch("httpx.AsyncClient.get", new_callable=AsyncMock)
|
||||
@patch("httpx.AsyncClient.post", new_callable=AsyncMock)
|
||||
def test_token_rotation_env_not_existing(self, mock_post, mock_get, client: TestClient):
|
||||
"""Token rotation when .env doesn't exist still succeeds."""
|
||||
from app.config import settings
|
||||
@@ -130,8 +130,8 @@ class TestTestTokenRotation:
|
||||
assert response.status_code == 200
|
||||
assert response.json()["status"] == "success"
|
||||
|
||||
@patch("app.api.onedrive.requests.get")
|
||||
@patch("app.api.onedrive.requests.post")
|
||||
@patch("httpx.AsyncClient.get", new_callable=AsyncMock)
|
||||
@patch("httpx.AsyncClient.post", new_callable=AsyncMock)
|
||||
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."""
|
||||
from app.config import settings
|
||||
@@ -171,8 +171,8 @@ class TestTestTokenRotation:
|
||||
assert response.status_code == 200
|
||||
assert response.json()["status"] == "success"
|
||||
|
||||
@patch("app.api.onedrive.requests.get")
|
||||
@patch("app.api.onedrive.requests.post")
|
||||
@patch("httpx.AsyncClient.get", new_callable=AsyncMock)
|
||||
@patch("httpx.AsyncClient.post", new_callable=AsyncMock)
|
||||
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."""
|
||||
from app.config import settings
|
||||
@@ -211,8 +211,8 @@ class TestTestTokenRotation:
|
||||
class TestTestTokenUserInfoFailed:
|
||||
"""Cover lines 170-171: user info request fails."""
|
||||
|
||||
@patch("app.api.onedrive.requests.get")
|
||||
@patch("app.api.onedrive.requests.post")
|
||||
@patch("httpx.AsyncClient.get", new_callable=AsyncMock)
|
||||
@patch("httpx.AsyncClient.post", new_callable=AsyncMock)
|
||||
def test_user_info_returns_non_200(self, mock_post, mock_get, client: TestClient):
|
||||
"""Test when user info request fails after successful token refresh."""
|
||||
from app.config import settings
|
||||
@@ -247,8 +247,8 @@ class TestTestTokenUserInfoFailed:
|
||||
class TestTokenRotationEnvAppendLine:
|
||||
"""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("app.api.onedrive.requests.post")
|
||||
@patch("httpx.AsyncClient.get", new_callable=AsyncMock)
|
||||
@patch("httpx.AsyncClient.post", new_callable=AsyncMock)
|
||||
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."""
|
||||
from app.config import settings
|
||||
|
||||
@@ -1,7 +1,7 @@
|
||||
"""Comprehensive unit tests for app/api/onedrive.py module."""
|
||||
|
||||
from datetime import timedelta
|
||||
from unittest.mock import MagicMock, patch
|
||||
from unittest.mock import AsyncMock, MagicMock, patch
|
||||
|
||||
import pytest
|
||||
|
||||
@@ -48,8 +48,8 @@ class TestExchangeOneDriveToken:
|
||||
class TestTestOneDriveToken:
|
||||
"""Tests for GET /onedrive/test-token endpoint."""
|
||||
|
||||
@patch("app.api.onedrive.requests.post")
|
||||
@patch("app.api.onedrive.requests.get")
|
||||
@patch("httpx.AsyncClient.post", new_callable=AsyncMock)
|
||||
@patch("httpx.AsyncClient.get", new_callable=AsyncMock)
|
||||
def test_test_token_success(self, mock_get, mock_post):
|
||||
"""Test successful token validation."""
|
||||
from app.config import settings
|
||||
@@ -79,7 +79,7 @@ class TestTestOneDriveToken:
|
||||
# Should return success
|
||||
pass
|
||||
|
||||
@patch("app.api.onedrive.requests.post")
|
||||
@patch("httpx.AsyncClient.post", new_callable=AsyncMock)
|
||||
def test_test_token_not_configured(self, mock_post):
|
||||
"""Test when credentials are not configured."""
|
||||
from app.config import settings
|
||||
@@ -88,7 +88,7 @@ class TestTestOneDriveToken:
|
||||
# Should return error
|
||||
pass
|
||||
|
||||
@patch("app.api.onedrive.requests.post")
|
||||
@patch("httpx.AsyncClient.post", new_callable=AsyncMock)
|
||||
def test_test_token_refresh_failed(self, mock_post):
|
||||
"""Test when token refresh fails."""
|
||||
from app.config import settings
|
||||
@@ -104,8 +104,8 @@ class TestTestOneDriveToken:
|
||||
# Should return error with needs_reauth
|
||||
pass
|
||||
|
||||
@patch("app.api.onedrive.requests.post")
|
||||
@patch("app.api.onedrive.requests.get")
|
||||
@patch("httpx.AsyncClient.post", new_callable=AsyncMock)
|
||||
@patch("httpx.AsyncClient.get", new_callable=AsyncMock)
|
||||
def test_test_token_user_info_failed(self, mock_get, mock_post):
|
||||
"""Test when user info request fails."""
|
||||
from app.config import settings
|
||||
@@ -128,8 +128,8 @@ class TestTestOneDriveToken:
|
||||
# Should return error
|
||||
pass
|
||||
|
||||
@patch("app.api.onedrive.requests.post")
|
||||
@patch("app.api.onedrive.requests.get")
|
||||
@patch("httpx.AsyncClient.post", new_callable=AsyncMock)
|
||||
@patch("httpx.AsyncClient.get", new_callable=AsyncMock)
|
||||
@patch("builtins.open", create=True)
|
||||
@patch("os.path.exists")
|
||||
def test_test_token_updates_refresh_token(self, mock_exists, mock_open, mock_get, mock_post):
|
||||
@@ -167,8 +167,8 @@ class TestTestOneDriveToken:
|
||||
# Should update refresh token in memory and file
|
||||
pass
|
||||
|
||||
@patch("app.api.onedrive.requests.post")
|
||||
@patch("app.api.onedrive.requests.get")
|
||||
@patch("httpx.AsyncClient.post", new_callable=AsyncMock)
|
||||
@patch("httpx.AsyncClient.get", new_callable=AsyncMock)
|
||||
def test_test_token_expiration_info(self, mock_get, mock_post):
|
||||
"""Test that expiration info is included."""
|
||||
from app.config import settings
|
||||
@@ -195,7 +195,7 @@ class TestTestOneDriveToken:
|
||||
# token_info should include expiration details
|
||||
pass
|
||||
|
||||
@patch("app.api.onedrive.requests.post")
|
||||
@patch("httpx.AsyncClient.post", new_callable=AsyncMock)
|
||||
def test_test_token_exception_handling(self, mock_post):
|
||||
"""Test handling of exceptions."""
|
||||
from app.config import settings
|
||||
|
||||
@@ -417,14 +417,20 @@ class TestOneDriveIntegration:
|
||||
|
||||
def test_onedrive_token_refresh_and_user_info(self, original_env: dict) -> None:
|
||||
"""Validate token refresh and user info retrieval."""
|
||||
import requests
|
||||
import asyncio
|
||||
|
||||
import httpx
|
||||
|
||||
token = self._get_access_token(original_env)
|
||||
resp = requests.get(
|
||||
"https://graph.microsoft.com/v1.0/me",
|
||||
headers={"Authorization": f"Bearer {token}"},
|
||||
timeout=30,
|
||||
)
|
||||
|
||||
async def _test():
|
||||
async with httpx.AsyncClient(timeout=30) as client:
|
||||
return await client.get(
|
||||
"https://graph.microsoft.com/v1.0/me",
|
||||
headers={"Authorization": f"Bearer {token}"},
|
||||
)
|
||||
|
||||
resp = asyncio.run(_test())
|
||||
assert resp.status_code == 200, f"OneDrive user info failed: {resp.text}"
|
||||
|
||||
def test_onedrive_upload_download_delete(self, original_env: dict) -> None:
|
||||
|
||||
Reference in New Issue
Block a user