mirror of
https://github.com/Significant-Gravitas/AutoGPT.git
synced 2026-04-08 03:00:28 -04:00
refactor(copilot): DRY cache-bust helper, fast eviction test, unified JSON parse
Backend: - Extract _bust_copilot_cache() in creds_manager.py; create/update/delete now each call it once instead of repeating the try/except ImportError block - test_evicts_oldest_when_full: patch _CACHE_MAX_SIZE to 3 to avoid allocating 10 000 entries in CI; remove now-unused _CACHE_MAX_SIZE import Frontend: - Extract parseJson() helper shared by parseOutput and parseError in ConnectIntegrationTool.tsx, eliminating duplicated try/catch logic
This commit is contained in:
@@ -7,7 +7,6 @@ import pytest
|
||||
from pydantic import SecretStr
|
||||
|
||||
from backend.copilot.integration_creds import (
|
||||
_CACHE_MAX_SIZE,
|
||||
_NO_TOKEN,
|
||||
_NULL_CACHE_TTL,
|
||||
_TOKEN_CACHE_TTL,
|
||||
@@ -100,16 +99,17 @@ class TestCacheSet:
|
||||
assert value == "tok2"
|
||||
|
||||
def test_evicts_oldest_when_full(self):
|
||||
# Fill to max
|
||||
for i in range(_CACHE_MAX_SIZE):
|
||||
_cache_set((f"user-{i}", _PROVIDER), f"tok-{i}", _TOKEN_CACHE_TTL)
|
||||
assert len(_token_cache) == _CACHE_MAX_SIZE
|
||||
# Use a tiny cap so the test doesn't create 10 000 entries.
|
||||
with patch("backend.copilot.integration_creds._CACHE_MAX_SIZE", 3):
|
||||
for i in range(3):
|
||||
_cache_set((f"user-{i}", _PROVIDER), f"tok-{i}", _TOKEN_CACHE_TTL)
|
||||
assert len(_token_cache) == 3
|
||||
|
||||
# Adding one more should evict the oldest ("user-0")
|
||||
_cache_set(("user-new", _PROVIDER), "tok-new", _TOKEN_CACHE_TTL)
|
||||
assert len(_token_cache) == _CACHE_MAX_SIZE
|
||||
assert ("user-0", _PROVIDER) not in _token_cache
|
||||
assert ("user-new", _PROVIDER) in _token_cache
|
||||
# Adding one more should evict the oldest ("user-0")
|
||||
_cache_set(("user-new", _PROVIDER), "tok-new", _TOKEN_CACHE_TTL)
|
||||
assert len(_token_cache) == 3
|
||||
assert ("user-0", _PROVIDER) not in _token_cache
|
||||
assert ("user-new", _PROVIDER) in _token_cache
|
||||
|
||||
|
||||
class TestGetProviderToken:
|
||||
|
||||
@@ -25,6 +25,21 @@ logger = logging.getLogger(__name__)
|
||||
settings = Settings()
|
||||
|
||||
|
||||
def _bust_copilot_cache(user_id: str, provider: str) -> None:
|
||||
"""Remove the copilot token cache entry for *(user_id, provider)*.
|
||||
|
||||
Called after create/update/delete so that the next bash_exec command
|
||||
fetches fresh credentials instead of serving a stale TTL-cached value.
|
||||
Silently skipped if the copilot module is not installed.
|
||||
"""
|
||||
try:
|
||||
from backend.copilot.integration_creds import invalidate_user_provider_cache
|
||||
|
||||
invalidate_user_provider_cache(user_id, provider)
|
||||
except ImportError:
|
||||
pass # copilot module not installed (e.g. isolated test env)
|
||||
|
||||
|
||||
class IntegrationCredentialsManager:
|
||||
"""
|
||||
Handles the lifecycle of integration credentials.
|
||||
@@ -72,12 +87,7 @@ class IntegrationCredentialsManager:
|
||||
result = await self.store.add_creds(user_id, credentials)
|
||||
# Bust the copilot token cache so that the next bash_exec picks up the
|
||||
# new credential immediately instead of waiting for _NULL_CACHE_TTL.
|
||||
try:
|
||||
from backend.copilot.integration_creds import invalidate_user_provider_cache
|
||||
|
||||
invalidate_user_provider_cache(user_id, credentials.provider)
|
||||
except ImportError:
|
||||
pass # copilot module not installed (e.g. isolated test env)
|
||||
_bust_copilot_cache(user_id, credentials.provider)
|
||||
return result
|
||||
|
||||
async def exists(self, user_id: str, credentials_id: str) -> bool:
|
||||
@@ -178,12 +188,7 @@ class IntegrationCredentialsManager:
|
||||
async with self._locked(user_id, updated.id):
|
||||
await self.store.update_creds(user_id, updated)
|
||||
# Bust the copilot token cache so the updated credential is picked up immediately.
|
||||
try:
|
||||
from backend.copilot.integration_creds import invalidate_user_provider_cache
|
||||
|
||||
invalidate_user_provider_cache(user_id, updated.provider)
|
||||
except ImportError:
|
||||
pass # copilot module not installed (e.g. isolated test env)
|
||||
_bust_copilot_cache(user_id, updated.provider)
|
||||
|
||||
async def delete(self, user_id: str, credentials_id: str) -> None:
|
||||
# Read provider before deletion so we know which cache entry to bust.
|
||||
@@ -191,14 +196,7 @@ class IntegrationCredentialsManager:
|
||||
async with self._locked(user_id, credentials_id):
|
||||
await self.store.delete_creds_by_id(user_id, credentials_id)
|
||||
if creds:
|
||||
try:
|
||||
from backend.copilot.integration_creds import (
|
||||
invalidate_user_provider_cache,
|
||||
)
|
||||
|
||||
invalidate_user_provider_cache(user_id, creds.provider)
|
||||
except ImportError:
|
||||
pass # copilot module not installed (e.g. isolated test env)
|
||||
_bust_copilot_cache(user_id, creds.provider)
|
||||
|
||||
# -- Locking utilities -- #
|
||||
|
||||
|
||||
@@ -9,32 +9,29 @@ type Props = {
|
||||
part: ToolUIPart;
|
||||
};
|
||||
|
||||
function parseJson(raw: unknown): unknown {
|
||||
if (typeof raw === "string") {
|
||||
try {
|
||||
return JSON.parse(raw);
|
||||
} catch {
|
||||
return null;
|
||||
}
|
||||
}
|
||||
return raw;
|
||||
}
|
||||
|
||||
function parseOutput(raw: unknown): SetupRequirementsResponse | null {
|
||||
try {
|
||||
let parsed: unknown = raw;
|
||||
if (typeof raw === "string") {
|
||||
parsed = JSON.parse(raw);
|
||||
}
|
||||
if (parsed && typeof parsed === "object" && "setup_info" in parsed) {
|
||||
return parsed as SetupRequirementsResponse;
|
||||
}
|
||||
} catch {
|
||||
// ignore parse errors
|
||||
const parsed = parseJson(raw);
|
||||
if (parsed && typeof parsed === "object" && "setup_info" in parsed) {
|
||||
return parsed as SetupRequirementsResponse;
|
||||
}
|
||||
return null;
|
||||
}
|
||||
|
||||
function parseError(raw: unknown): string | null {
|
||||
try {
|
||||
let parsed: unknown = raw;
|
||||
if (typeof raw === "string") {
|
||||
parsed = JSON.parse(raw);
|
||||
}
|
||||
if (parsed && typeof parsed === "object" && "message" in parsed) {
|
||||
return String((parsed as { message: unknown }).message);
|
||||
}
|
||||
} catch {
|
||||
// ignore parse errors
|
||||
const parsed = parseJson(raw);
|
||||
if (parsed && typeof parsed === "object" && "message" in parsed) {
|
||||
return String((parsed as { message: unknown }).message);
|
||||
}
|
||||
return null;
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user