Skip to content

Commit 57059f5

Browse files
benjaminpiliaBenjamin PILIAleoguillaume
authored
refacto(user): POST /v1/admin/users endpoint clean archi refactoring (#867)
* refacto create_user repository method * refacto bootstrap admin user * add integration tests * BSR: add AccountExpiredHTTPException to endpoints documentation * refacto endpoints documentation * refacto create_user repo method * Update unit coverage badge * restore not admin exception documentation * hash password in dedicated function * Update unit coverage badge --------- Co-authored-by: Benjamin PILIA <benjamin.pilia@protonmail.com> Co-authored-by: benjaminpilia <benjaminpilia@users.noreply.github.com> Co-authored-by: leoguillaume <leo.ch.guillaume@gmail.com> Co-authored-by: leoguillaume <leoguillaume@users.noreply.github.com>
1 parent b2578a6 commit 57059f5

17 files changed

Lines changed: 195 additions & 97 deletions

File tree

.github/badges/coverage.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
{"schemaVersion":1,"label":"coverage","message":"56.24%","color":"red"}
1+
{"schemaVersion":1,"label":"coverage","message":"56.29%","color":"red"}

api/helpers/_usagetokenizer.py

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,14 @@
1010

1111

1212
class UsageTokenizer:
13-
USAGE_ENDPOINTS = [EndpointRoute.AUDIO_TRANSCRIPTIONS, EndpointRoute.CHAT_COMPLETIONS, EndpointRoute.EMBEDDINGS, EndpointRoute.OCR, EndpointRoute.RERANK, EndpointRoute.SEARCH]
13+
USAGE_ENDPOINTS = [
14+
EndpointRoute.AUDIO_TRANSCRIPTIONS,
15+
EndpointRoute.CHAT_COMPLETIONS,
16+
EndpointRoute.EMBEDDINGS,
17+
EndpointRoute.OCR,
18+
EndpointRoute.RERANK,
19+
EndpointRoute.SEARCH,
20+
]
1421

1522
def __init__(self, tokenizer: Tokenizer):
1623
if tokenizer == Tokenizer.TIKTOKEN_O200K_BASE:

api/infrastructure/fastapi/endpoints/admin/providers.py

Lines changed: 23 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -66,15 +66,17 @@
6666
path=EndpointRoute.ADMIN_PROVIDERS,
6767
dependencies=[Security(dependency=get_current_key)],
6868
status_code=201,
69-
responses=get_documentation_responses([
70-
InconsistentModelMaxContextLengthHTTPException,
71-
InconsistentModelVectorSizeHTTPException,
72-
InvalidProviderTypeHTTPException,
73-
ProviderNotReachableHTTPException,
74-
ProviderAlreadyExistsHTTPException,
75-
RouterNotFoundHTTPException,
76-
NotAdminUserHTTPException,
77-
]),
69+
responses=get_documentation_responses(
70+
exceptions=[
71+
InconsistentModelMaxContextLengthHTTPException,
72+
InconsistentModelVectorSizeHTTPException,
73+
InvalidProviderTypeHTTPException,
74+
ProviderNotReachableHTTPException,
75+
ProviderAlreadyExistsHTTPException,
76+
RouterNotFoundHTTPException,
77+
NotAdminUserHTTPException,
78+
]
79+
),
7880
)
7981
async def create_provider(
8082
body: CreateProviderBody,
@@ -143,7 +145,7 @@ async def create_provider(
143145
path=EndpointRoute.ADMIN_PROVIDERS + "/{provider_id}",
144146
dependencies=[Security(dependency=get_current_key)],
145147
status_code=200,
146-
responses=get_documentation_responses([NotAdminUserHTTPException, ProviderNotFoundHTTPException]),
148+
responses=get_documentation_responses([ProviderNotFoundHTTPException, NotAdminUserHTTPException]),
147149
)
148150
async def delete_provider(
149151
provider_id: int = Path(description="The ID of the provider to delete."),
@@ -185,15 +187,17 @@ async def delete_provider(
185187
path=EndpointRoute.ADMIN_PROVIDERS + "/{provider_id}",
186188
dependencies=[Security(dependency=get_current_key)],
187189
status_code=200,
188-
responses=get_documentation_responses([
189-
InconsistentModelMaxContextLengthHTTPException,
190-
InconsistentModelVectorSizeHTTPException,
191-
InvalidProviderTypeHTTPException,
192-
ProviderAlreadyExistsHTTPException,
193-
RouterNotFoundHTTPException,
194-
ProviderNotFoundHTTPException,
195-
NotAdminUserHTTPException,
196-
]),
190+
responses=get_documentation_responses(
191+
[
192+
InconsistentModelMaxContextLengthHTTPException,
193+
InconsistentModelVectorSizeHTTPException,
194+
InvalidProviderTypeHTTPException,
195+
ProviderAlreadyExistsHTTPException,
196+
RouterNotFoundHTTPException,
197+
ProviderNotFoundHTTPException,
198+
NotAdminUserHTTPException,
199+
]
200+
),
197201
)
198202
async def update_provider(
199203
provider_id: int = Path(description="The ID of the provider to update."),

api/infrastructure/fastapi/endpoints/admin/routers.py

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -221,12 +221,14 @@ async def delete_router(
221221
@router.patch(
222222
path=EndpointRoute.ADMIN_ROUTERS + "/{router_id}",
223223
dependencies=[Security(dependency=get_current_key)],
224-
responses=get_documentation_responses([
225-
RouterNotFoundHTTPException,
226-
NotAdminUserHTTPException,
227-
RouterAliasAlreadyExistsHTTPException,
228-
RouterAlreadyExistsHTTPException,
229-
]),
224+
responses=get_documentation_responses(
225+
[
226+
NotAdminUserHTTPException,
227+
RouterNotFoundHTTPException,
228+
RouterAliasAlreadyExistsHTTPException,
229+
RouterAlreadyExistsHTTPException,
230+
]
231+
),
230232
status_code=200,
231233
)
232234
async def update_router(

api/infrastructure/fastapi/endpoints/admin/users.py

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -30,12 +30,14 @@
3030
path=EndpointRoute.ADMIN_USERS,
3131
dependencies=[Security(dependency=get_current_key)],
3232
status_code=201,
33-
responses=get_documentation_responses([
34-
UserAlreadyExistsHTTPException,
35-
RoleNotFoundHTTPException,
36-
OrganizationNotFoundHTTPException,
37-
NotAdminUserHTTPException,
38-
]),
33+
responses=get_documentation_responses(
34+
[
35+
UserAlreadyExistsHTTPException,
36+
RoleNotFoundHTTPException,
37+
OrganizationNotFoundHTTPException,
38+
NotAdminUserHTTPException,
39+
]
40+
),
3941
)
4042
async def create_user(
4143
body: CreateUserBody = Body(description="The user creation request."),

api/infrastructure/postgres/_postgresusersrepository.py

Lines changed: 40 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33
import bcrypt
44
from sqlalchemy import Integer, cast, func, insert, select, update
5-
from sqlalchemy.exc import IntegrityError, NoResultFound
5+
from sqlalchemy.exc import IntegrityError
66
from sqlalchemy.ext.asyncio import AsyncSession
77

88
from api.domain.organization.errors import OrganizationNotFoundError
@@ -12,9 +12,7 @@
1212
from api.domain.user.entities import User
1313
from api.domain.user.errors import UserAlreadyExistsError, UserNotFoundError
1414
from api.infrastructure.postgres.decorators import with_lock
15-
from api.sql.models import Organization as OrganizationTable
1615
from api.sql.models import Permission as PermissionTable
17-
from api.sql.models import Role as RoleTable
1816
from api.sql.models import User as UserTable
1917
from api.utils.exceptions import UserNotFoundException
2018

@@ -44,6 +42,10 @@ def _row_to_user(row) -> User:
4442
updated=row.updated,
4543
)
4644

45+
@staticmethod
46+
def _hash_password(password: str) -> str:
47+
return bcrypt.hashpw(password.encode("utf-8"), bcrypt.gensalt()).decode("utf-8")
48+
4749
@with_lock(namespace="user", key="email")
4850
async def create_user(
4951
self,
@@ -58,55 +60,45 @@ async def create_user(
5860
expires: int | None = None,
5961
priority: int = 0,
6062
) -> User | UserAlreadyExistsError | RoleNotFoundError | OrganizationNotFoundError:
61-
result = await self.postgres_session.execute(select(RoleTable.id).where(RoleTable.id == role_id))
62-
try:
63-
result.scalar_one()
64-
except NoResultFound:
65-
return RoleNotFoundError(id=role_id)
66-
67-
if organization_id is not None:
68-
result = await self.postgres_session.execute(select(OrganizationTable.id).where(OrganizationTable.id == organization_id))
69-
try:
70-
result.scalar_one()
71-
except NoResultFound:
72-
return OrganizationNotFoundError(id=organization_id)
73-
74-
hashed_password = bcrypt.hashpw(password.encode("utf-8"), bcrypt.gensalt()).decode("utf-8")
63+
hashed_password = self._hash_password(password=password)
7564
expires_value = func.to_timestamp(expires) if expires is not None else None
7665

7766
try:
78-
async with self.postgres_session.begin_nested():
79-
result = await self.postgres_session.execute(
80-
insert(UserTable)
81-
.values(
82-
email=email,
83-
name=name,
84-
password=hashed_password,
85-
sub=sub,
86-
iss=iss,
87-
role_id=role_id,
88-
organization_id=organization_id,
89-
budget=budget,
90-
expires=expires_value,
91-
priority=priority,
92-
)
93-
.returning(
94-
UserTable.id,
95-
UserTable.email,
96-
UserTable.name,
97-
UserTable.sub,
98-
UserTable.iss,
99-
UserTable.role_id.label("role"),
100-
UserTable.organization_id.label("organization"),
101-
UserTable.budget,
102-
_unix_timestamp(UserTable.expires).label("expires"),
103-
_unix_timestamp(UserTable.created).label("created"),
104-
_unix_timestamp(UserTable.updated).label("updated"),
105-
UserTable.priority,
106-
)
67+
result = await self.postgres_session.execute(
68+
insert(UserTable)
69+
.values(
70+
email=email,
71+
name=name,
72+
password=hashed_password,
73+
sub=sub,
74+
iss=iss,
75+
role_id=role_id,
76+
organization_id=organization_id,
77+
budget=budget,
78+
expires=expires_value,
79+
priority=priority,
10780
)
108-
row = result.one()
109-
except IntegrityError:
81+
.returning(
82+
UserTable.id,
83+
UserTable.email,
84+
UserTable.name,
85+
UserTable.sub,
86+
UserTable.iss,
87+
UserTable.role_id.label("role"),
88+
UserTable.organization_id.label("organization"),
89+
UserTable.budget,
90+
_unix_timestamp(UserTable.expires).label("expires"),
91+
_unix_timestamp(UserTable.created).label("created"),
92+
_unix_timestamp(UserTable.updated).label("updated"),
93+
UserTable.priority,
94+
)
95+
)
96+
row = result.one()
97+
except IntegrityError as e:
98+
if "user_organization_id_fkey" in str(e.orig):
99+
return OrganizationNotFoundError(id=organization_id)
100+
if "user_role_id_fkey" in str(e.orig):
101+
return RoleNotFoundError(id=role_id)
110102
return UserAlreadyExistsError(email=email)
111103

112104
return self._row_to_user(row)

api/tests/integration/postgres/test_postgresrolesrepository.py renamed to api/tests/integration/endpoints/admin/users/__init__.py

File renamed without changes.
Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,101 @@
1+
from unittest.mock import AsyncMock
2+
3+
from httpx import AsyncClient
4+
import pytest
5+
import pytest_asyncio
6+
7+
from api.dependencies import create_user_use_case_factory
8+
from api.domain.organization.errors import OrganizationNotFoundError
9+
from api.domain.role.errors import RoleNotFoundError
10+
from api.domain.user.errors import UserAlreadyExistsError, UserIsNotAdminError
11+
from api.tests.helpers import create_key
12+
from api.tests.integration.factories.sql import RoleSQLFactory, UserSQLFactory
13+
from api.utils.variables import EndpointRoute
14+
15+
URL = f"/v1{EndpointRoute.ADMIN_USERS}"
16+
17+
18+
def _valid_body(role_id: int, **overrides) -> dict:
19+
body = {
20+
"email": "newuser@test.com",
21+
"password": "s3cr3t",
22+
"role": role_id,
23+
}
24+
body.update(overrides)
25+
return body
26+
27+
28+
@pytest.mark.asyncio(loop_scope="session")
29+
class TestCreateUser:
30+
@pytest_asyncio.fixture(autouse=True)
31+
async def setup(self, db_session):
32+
self.admin_user = UserSQLFactory(admin_user=True)
33+
self.token = await create_key(db_session, name="admin_token", user=self.admin_user)
34+
35+
async def test_happy_path(self, client: AsyncClient, db_session):
36+
role = RoleSQLFactory()
37+
await db_session.flush()
38+
39+
response = await client.post(
40+
url=URL,
41+
headers={"Authorization": f"Bearer {self.token.token}"},
42+
json=_valid_body(role_id=role.id),
43+
)
44+
45+
assert response.status_code == 201, response.text
46+
data = response.json()
47+
assert data["email"] == "newuser@test.com"
48+
assert isinstance(data["id"], int)
49+
assert data["role"] == role.id
50+
51+
@pytest.mark.parametrize(
52+
"use_case_result,expected_status,expected_detail",
53+
[
54+
(
55+
UserAlreadyExistsError(email="existing@test.com"),
56+
409,
57+
"User existing@test.com already exists.",
58+
),
59+
(
60+
RoleNotFoundError(id=99),
61+
404,
62+
"Role 99 not found.",
63+
),
64+
(
65+
OrganizationNotFoundError(id=99),
66+
404,
67+
"Organization 99 not found.",
68+
),
69+
(
70+
UserIsNotAdminError(),
71+
403,
72+
"User has no admin rights.",
73+
),
74+
],
75+
)
76+
async def test_error_maps_to_correct_http_status(self, client: AsyncClient, app, use_case_result, expected_status, expected_detail):
77+
mock_use_case = AsyncMock()
78+
mock_use_case.execute.return_value = use_case_result
79+
app.dependency_overrides[create_user_use_case_factory] = lambda: mock_use_case
80+
81+
response = await client.post(
82+
url=URL,
83+
headers={"Authorization": f"Bearer {self.token.token}"},
84+
json=_valid_body(role_id=1),
85+
)
86+
87+
assert response.status_code == expected_status
88+
assert response.json().get("detail") == expected_detail
89+
90+
@pytest.mark.parametrize(
91+
"headers,expected_status,expected_detail",
92+
[
93+
({}, 401, "Not authenticated"),
94+
({"Authorization": "Bearer invalid-token"}, 401, "Invalid API key."),
95+
],
96+
)
97+
async def test_auth(self, client: AsyncClient, headers, expected_status, expected_detail):
98+
response = await client.post(url=URL, headers=headers, json=_valid_body(role_id=1))
99+
100+
assert response.status_code == expected_status
101+
assert response.json().get("detail") == expected_detail

api/tests/integration/postgres/test_postgresproviderrepository.py

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -298,8 +298,7 @@ async def test_update_provider_should_persist_all_updatable_fields(self, reposit
298298

299299
# Act
300300
result = await repository.update_provider(
301-
domain_provider
302-
.with_router_id(router_2.id)
301+
domain_provider.with_router_id(router_2.id)
303302
.with_timeout(120)
304303
.with_model_hosting_zone(HostingZone.USA)
305304
.with_model_total_params(2_000_000)

api/tests/integration/postgres/user_repository/__init__.py

Whitespace-only changes.

0 commit comments

Comments
 (0)