Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Migrate to pydantic v2 and switch tests to Polyfactory #550

Merged
merged 1 commit into from
Apr 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,11 @@ and this project adheres to

## [Unreleased]

### Changed

- Upgrade `pydantic` to `2.7.0`
- Migrate model tests from hypothesis strategies to polyfactory

## [4.2.0] - 2024-04-08

### Added
Expand Down
5 changes: 3 additions & 2 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,8 @@ dependencies = [
# library (mostly models).
"importlib-metadata>=7.0.1, <8.0",
"langcodes>=3.2.0",
"pydantic[dotenv,email]>=1.10.0, <2.0",
"pydantic[email]>=2.5.3,<3.0",
"pydantic_settings>=2.1.0,<3.0",
"rfc3987>=1.3.0",
]
dynamic = ["version"]
Expand Down Expand Up @@ -92,7 +93,6 @@ dev = [
"black==24.3.0",
"cryptography==42.0.5",
"factory-boy==3.3.0",
"hypothesis<6.92.0", # pin as hypothesis 6.92.0 observability feature seems broken
"logging-gelf==0.0.32",
"mike==2.0.0",
"mkdocs==1.5.3",
Expand All @@ -102,6 +102,7 @@ dev = [
"moto==5.0.5",
"mypy==1.9.0",
"neoteroi-mkdocs==1.0.5",
"polyfactory==2.15.0",
"pyfakefs==5.4.0",
"pymdown-extensions==10.7.1",
"pytest<8.0.0", # pin as pytest-httpx<0.23.0 is not compatible with pytest 8.0.0
Expand Down
5 changes: 4 additions & 1 deletion src/ralph/api/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -51,4 +51,7 @@ async def whoami(
user: AuthenticatedUser = Depends(get_authenticated_user),
) -> Dict[str, Any]:
"""Return the current user's username along with their scopes."""
return {"agent": user.agent, "scopes": user.scopes}
return {
"agent": user.agent.model_dump(mode="json", exclude_none=True),
"scopes": user.scopes,
Leobouloc marked this conversation as resolved.
Show resolved Hide resolved
}
32 changes: 16 additions & 16 deletions src/ralph/api/auth/basic.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
"""Basic authentication & authorization related tools for the Ralph API."""

import logging
import os
from functools import lru_cache
from pathlib import Path
from threading import Lock
Expand All @@ -10,7 +11,7 @@
from cachetools import TTLCache, cached
from fastapi import Depends, HTTPException, status
from fastapi.security import HTTPBasic, HTTPBasicCredentials
from pydantic import BaseModel, root_validator
from pydantic import RootModel, model_validator
from starlette.authentication import AuthenticationError

from ralph.api.auth.user import AuthenticatedUser
Expand Down Expand Up @@ -40,45 +41,42 @@ class UserCredentials(AuthenticatedUser):
username: str


class ServerUsersCredentials(BaseModel):
class ServerUsersCredentials(RootModel[List[UserCredentials]]):
"""Custom root pydantic model.

Describe expected list of all server users credentials as stored in
the credentials file.

Attributes:
__root__ (List): Custom root consisting of the
root (List): Custom root consisting of the
list of all server users credentials.
"""

__root__: List[UserCredentials]

def __add__(self, other) -> Any: # noqa: D105
return ServerUsersCredentials.parse_obj(self.__root__ + other.__root__)
return ServerUsersCredentials.model_validate(self.root + other.root)

def __getitem__(self, item: int) -> UserCredentials: # noqa: D105
return self.__root__[item]
return self.root[item]

def __len__(self) -> int: # noqa: D105
return len(self.__root__)
return len(self.root)

def __iter__(self) -> Iterator[UserCredentials]: # noqa: D105
return iter(self.__root__)
return iter(self.root)

@root_validator
@classmethod
def ensure_unique_username(cls, values: Any) -> Any:
@model_validator(mode="after")
Leobouloc marked this conversation as resolved.
Show resolved Hide resolved
def ensure_unique_username(self) -> Any:
"""Every username should be unique among registered users."""
usernames = [entry.username for entry in values.get("__root__")]
usernames = [entry.username for entry in self.root]
if len(usernames) != len(set(usernames)):
raise ValueError(
"You cannot create multiple credentials with the same username"
)
return values
return self


@lru_cache()
def get_stored_credentials(auth_file: Path) -> ServerUsersCredentials:
def get_stored_credentials(auth_file: os.PathLike) -> ServerUsersCredentials:
"""Helper to read the credentials/scopes file.

Read credentials from JSON file and stored them to avoid reloading them with every
Expand All @@ -96,7 +94,9 @@ def get_stored_credentials(auth_file: Path) -> ServerUsersCredentials:
msg = "Credentials file <%s> not found."
logger.warning(msg, auth_file)
raise AuthenticationError(msg.format(auth_file))
return ServerUsersCredentials.parse_file(auth_file)

with open(auth_file, encoding=settings.LOCALE_ENCODING) as f:
return ServerUsersCredentials.model_validate_json(f.read())


@cached(
Expand Down
13 changes: 6 additions & 7 deletions src/ralph/api/auth/oidc.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
from fastapi.security import HTTPBearer, OpenIdConnect
from jose import ExpiredSignatureError, JWTError, jwt
from jose.exceptions import JWTClaimsError
from pydantic import AnyUrl, BaseModel, Extra
from pydantic import AnyUrl, BaseModel, ConfigDict
from typing_extensions import Annotated

from ralph.api.auth.user import AuthenticatedUser, UserScopes
Expand Down Expand Up @@ -45,14 +45,13 @@ class IDToken(BaseModel):

iss: str
sub: str
aud: Optional[str]
aud: Optional[str] = None
exp: int
iat: int
scope: Optional[str]
target: Optional[str]
scope: Optional[str] = None
target: Optional[str] = None

class Config: # noqa: D106
extra = Extra.ignore
model_config = ConfigDict(extra="ignore")


@lru_cache()
Expand Down Expand Up @@ -144,7 +143,7 @@ def get_oidc_user(
headers={"WWW-Authenticate": "Bearer"},
) from exc

id_token = IDToken.parse_obj(decoded_token)
id_token = IDToken.model_validate(decoded_token)

user = AuthenticatedUser(
agent={"openid": f"{id_token.iss}/{id_token.sub}"},
Expand Down
22 changes: 8 additions & 14 deletions src/ralph/api/auth/user.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
"""Authenticated user for the Ralph API."""

from typing import Dict, FrozenSet, Literal, Optional
from typing import FrozenSet, Literal, Optional

from pydantic import BaseModel
from pydantic import BaseModel, RootModel

from ralph.models.xapi.base.agents import BaseXapiAgent

Scope = Literal[
"statements/write",
Expand All @@ -18,7 +20,7 @@
]


class UserScopes(FrozenSet[Scope]):
class UserScopes(RootModel[FrozenSet[Scope]]):
"""Scopes available to users."""

def is_authorized(self, requested_scope: Scope):
Expand Down Expand Up @@ -47,19 +49,11 @@ def is_authorized(self, requested_scope: Scope):
}

expanded_user_scopes = set()
for scope in self:
for scope in self.root:
expanded_user_scopes.update(expanded_scopes.get(scope, {scope}))

return requested_scope in expanded_user_scopes

@classmethod
def __get_validators__(cls): # noqa: D105
def validate(value: FrozenSet[Scope]):
"""Transform value to an instance of UserScopes."""
return cls(value)

yield validate


class AuthenticatedUser(BaseModel):
"""Pydantic model for user authentication.
Expand All @@ -70,6 +64,6 @@ class AuthenticatedUser(BaseModel):
target (str or None): The target index or database to store statements into.
"""

agent: Dict
agent: BaseXapiAgent
scopes: UserScopes
target: Optional[str]
target: Optional[str] = None
6 changes: 3 additions & 3 deletions src/ralph/api/forwarding.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,16 +42,16 @@ async def forward_xapi_statements(
try:
# NB: post or put
req = await getattr(client, method)(
forwarding.url,
str(forwarding.url),
json=statements,
auth=(forwarding.basic_username, forwarding.basic_password),
timeout=forwarding.timeout,
)
req.raise_for_status()
msg = "Forwarded %s statements to %s with success."
if isinstance(statements, list):
logger.debug(msg, len(statements), forwarding.url)
logger.debug(msg, len(statements), str(forwarding.url))
else:
logger.debug(msg, 1, forwarding.url)
logger.debug(msg, 1, str(forwarding.url))
except (RequestError, HTTPStatusError) as error:
logger.error("Failed to forward xAPI statements. %s", error)
12 changes: 3 additions & 9 deletions src/ralph/api/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
from typing import Optional, Union
from uuid import UUID

from pydantic import AnyUrl, BaseModel, Extra
from pydantic import AnyUrl, BaseModel, ConfigDict

from ..models.xapi.base.agents import BaseXapiAgent
from ..models.xapi.base.groups import BaseXapiGroup
Expand All @@ -30,13 +30,7 @@ class BaseModelWithLaxConfig(BaseModel):
we receive statements through the API.
"""

class Config:
"""Enable extra properties.

Useful for not having to perform comprehensive validation.
"""

extra = Extra.allow
model_config = ConfigDict(extra="allow", coerce_numbers_to_str=True)


class LaxObjectField(BaseModelWithLaxConfig):
Expand Down Expand Up @@ -65,6 +59,6 @@ class LaxStatement(BaseModelWithLaxConfig):
"""

actor: Union[BaseXapiAgent, BaseXapiGroup]
id: Optional[UUID]
id: Optional[UUID] = None
object: LaxObjectField
verb: LaxVerbField
2 changes: 1 addition & 1 deletion src/ralph/api/routers/health.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ async def heartbeat(response: Response) -> Heartbeat:

Return a 200 if all checks are successful.
"""
statuses = Heartbeat.construct(
statuses = Heartbeat.model_construct(
database=await await_if_coroutine(BACKEND_CLIENT.status())
)
if not statuses.is_alive:
Expand Down
36 changes: 25 additions & 11 deletions src/ralph/api/routers/statements.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
status,
)
from fastapi.dependencies.models import Dependant
from pydantic import parse_obj_as
from pydantic import TypeAdapter
from pydantic.types import Json
from typing_extensions import Annotated

Expand Down Expand Up @@ -98,14 +98,17 @@ def _enrich_statement_with_authority(
) -> None:
# authority: Information about whom or what has asserted the statement is true.
# https://github.com/adlnet/xAPI-Spec/blob/master/xAPI-Data.md#249-authority
statement["authority"] = current_user.agent
statement["authority"] = current_user.agent.model_dump(
exclude_none=True, mode="json"
)


def _parse_agent_parameters(agent_obj: dict) -> AgentParameters:
"""Parse a dict and return an AgentParameters object to use in queries."""
# Transform agent to `dict` as FastAPI cannot parse JSON (seen as string)

agent = parse_obj_as(BaseXapiAgent, agent_obj)
adapter = TypeAdapter(BaseXapiAgent)
agent = adapter.validate_python(agent_obj)

agent_query_params = {}
if isinstance(agent, BaseXapiAgentWithMbox):
Expand All @@ -119,7 +122,7 @@ def _parse_agent_parameters(agent_obj: dict) -> AgentParameters:
agent_query_params["account__home_page"] = agent.account.homePage

# Overwrite `agent` field
return AgentParameters.construct(**agent_query_params)
return AgentParameters.model_construct(**agent_query_params)


def strict_query_params(request: Request) -> None:
Expand All @@ -141,7 +144,7 @@ def strict_query_params(request: Request) -> None:

@router.get("")
@router.get("/")
async def get( # noqa: PLR0913
async def get( # noqa: PLR0912,PLR0913
request: Request,
current_user: Annotated[
AuthenticatedUser,
Expand Down Expand Up @@ -169,7 +172,7 @@ async def get( # noqa: PLR0913
None,
description="Filter, only return Statements matching the specified Verb id",
),
activity: Optional[IRI] = Query(
activity: Optional[str] = Query(
quitterie-lcs marked this conversation as resolved.
Show resolved Hide resolved
None,
description=(
"Filter, only return Statements for which the Object "
Expand Down Expand Up @@ -334,7 +337,14 @@ async def get( # noqa: PLR0913
# Overwrite `agent` field
query_params["agent"] = _parse_agent_parameters(
json.loads(query_params["agent"])
)
).model_dump(mode="json", exclude_none=True)

# Coerce `verb` and `activity` as IRI
if query_params.get("verb"):
query_params["verb"] = IRI(query_params["verb"])

if query_params.get("activity"):
query_params["activity"] = IRI(query_params["activity"])

# mine: If using scopes, only restrict users with limited scopes
if settings.LRS_RESTRICT_BY_SCOPES:
Expand All @@ -346,7 +356,9 @@ async def get( # noqa: PLR0913

# Filter by authority if using `mine`
if mine:
query_params["authority"] = _parse_agent_parameters(current_user.agent)
query_params["authority"] = _parse_agent_parameters(
current_user.agent.model_dump(mode="json")
wilbrdt marked this conversation as resolved.
Show resolved Hide resolved
).model_dump(mode="json", exclude_none=True)

if "mine" in query_params:
query_params.pop("mine")
Expand All @@ -355,7 +367,7 @@ async def get( # noqa: PLR0913
try:
query_result = await await_if_coroutine(
BACKEND_CLIENT.query_statements(
params=RalphStatementsQuery.construct(
params=RalphStatementsQuery.model_construct(
**{**query_params, "limit": limit}
),
target=current_user.target,
Expand Down Expand Up @@ -418,7 +430,7 @@ async def put(
LRS Specification:
https://github.com/adlnet/xAPI-Spec/blob/1.0.3/xAPI-Communication.md#211-put-statements
"""
statement_as_dict = statement.dict(exclude_unset=True)
statement_as_dict = statement.model_dump(exclude_unset=True, mode="json")
statement_id = str(statement_id)

statement_as_dict.update(id=str(statement_as_dict.get("id", statement_id)))
Expand Down Expand Up @@ -516,7 +528,9 @@ async def post( # noqa: PLR0912

# Enrich statements before forwarding
statements_dict = {}
for statement in (x.dict(exclude_unset=True) for x in statements):
for statement in (
x.model_dump(exclude_unset=True, mode="json") for x in statements
):
_enrich_statement_with_id(statement)
# Requests with duplicate statement IDs are considered invalid
if statement["id"] in statements_dict:
Expand Down
Loading