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

[WIP] (api) Follow xAPI spec for versioning #452

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
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
3 changes: 2 additions & 1 deletion src/ralph/api/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
from urllib.parse import urlparse

import sentry_sdk
from fastapi import Depends, FastAPI
from fastapi import Depends, Header, FastAPI

from ralph.conf import settings

Expand Down Expand Up @@ -39,6 +39,7 @@ def filter_transactions(event, hint): # pylint: disable=unused-argument
before_send_transaction=filter_transactions,
)


app = FastAPI()
app.include_router(statements.router)
app.include_router(health.router)
Expand Down
30 changes: 26 additions & 4 deletions src/ralph/api/routers/statements.py
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,7 @@ def strict_query_params(request: Request):
@router.get("/")
# pylint: disable=too-many-arguments, too-many-locals
async def get(
response: Response,
request: Request,
current_user: Annotated[AuthenticatedUser, Depends(get_authenticated_user)],
###
Expand Down Expand Up @@ -279,6 +280,9 @@ async def get(
LRS Specification:
https://github.com/adlnet/xAPI-Spec/blob/1.0.3/xAPI-Communication.md#213-get-statements
"""
# The LRS MUST include the "X-Experience-API-Version" header in every response
response.headers["X-Experience-API-Version"] = settings.LRS_HEADER_XAPI_VERSION

# Make sure the limit does not go above max from settings
limit = min(limit, settings.RUNSERVER_MAX_SEARCH_HITS_COUNT)

Expand Down Expand Up @@ -357,7 +361,7 @@ async def get(
# NB: There is an unhandled edge case where the total number of results is
# exactly a multiple of the "limit", in which case we'll offer an extra page
# with 0 results.
response = {}
more_query_parameters = {}
if len(query_result.statements) == limit:
# Search after relies on sorting info located in the last hit
path = request.url.path
Expand All @@ -370,7 +374,7 @@ async def get(
}
)

response.update(
more_query_parameters.update(
{
"more": ParseResult(
scheme="",
Expand All @@ -383,13 +387,23 @@ async def get(
}
)

return {**response, "statements": query_result.statements}
# Statements returned by an LRS MUST retain the version they are accepted with.
# If they lack a version, the version MUST be set to 1.0.0.
for statement in query_result.statements:
# Delete `version` if it is an empty string. Necessary for clickhouse.
if "version" in statement and not statement["version"]:
statement.pop("version")
statement["version"] = statement.get("version", settings.LRS_GET_STATEMENT_DEFAULT_XAPI_VERSION)


return {**more_query_parameters, "statements": query_result.statements}


@router.put("/", responses=POST_PUT_RESPONSES, status_code=status.HTTP_204_NO_CONTENT)
@router.put("", responses=POST_PUT_RESPONSES, status_code=status.HTTP_204_NO_CONTENT)
# pylint: disable=unused-argument
async def put(
response: Response,
current_user: Annotated[AuthenticatedUser, Depends(get_authenticated_user)],
statement: LaxStatement,
background_tasks: BackgroundTasks,
Expand All @@ -401,6 +415,10 @@ async def put(
LRS Specification:
https://github.com/adlnet/xAPI-Spec/blob/1.0.3/xAPI-Communication.md#211-put-statements
"""

# The LRS MUST include the "X-Experience-API-Version" header in every response
response.headers["X-Experience-API-Version"] = settings.LRS_HEADER_XAPI_VERSION

statement_as_dict = statement.dict(exclude_unset=True)
statement_id = str(statement_id)

Expand Down Expand Up @@ -459,10 +477,10 @@ async def put(
@router.post("/", responses=POST_PUT_RESPONSES)
@router.post("", responses=POST_PUT_RESPONSES)
async def post(
response: Response,
current_user: Annotated[AuthenticatedUser, Depends(get_authenticated_user)],
statements: Union[LaxStatement, List[LaxStatement]],
background_tasks: BackgroundTasks,
response: Response,
_=Depends(strict_query_params),
):
"""Store a set of statements (or a single statement as a single member of a set).
Expand All @@ -471,6 +489,10 @@ async def post(
LRS Specification:
https://github.com/adlnet/xAPI-Spec/blob/1.0.3/xAPI-Communication.md#212-post-statements
"""

# The LRS MUST include the "X-Experience-API-Version" header in every response
response.headers["X-Experience-API-Version"] = settings.LRS_HEADER_XAPI_VERSION

# As we accept both a single statement as a dict, and multiple statements as a list,
# we need to normalize the data into a list in all cases before we can process it.
if not isinstance(statements, list):
Expand Down
2 changes: 1 addition & 1 deletion src/ralph/backends/database/clickhouse.py
Original file line number Diff line number Diff line change
Expand Up @@ -398,7 +398,7 @@ def query_statements(self, params: RalphStatementsQuery) -> StatementQueryResult
# same timestamp, and also avoid sending the same event twice.
new_search_after = response[-1]["emission_time"].isoformat()
new_pit_id = str(response[-1]["event_id"])

return StatementQueryResult(
statements=[document["event"] for document in response],
search_after=new_search_after,
Expand Down
2 changes: 2 additions & 0 deletions src/ralph/conf.py
Original file line number Diff line number Diff line change
Expand Up @@ -381,6 +381,8 @@ class AuthBackends(Enum):
RUNSERVER_MAX_SEARCH_HITS_COUNT: int = 100
RUNSERVER_POINT_IN_TIME_KEEP_ALIVE: str = "1m"
RUNSERVER_PORT: int = 8100
LRS_HEADER_XAPI_VERSION: str = "1.0.3"
LRS_GET_STATEMENT_DEFAULT_XAPI_VERSION: str = "1.0.0"
LRS_RESTRICT_BY_AUTHORITY: bool = False
LRS_RESTRICT_BY_SCOPES: bool = False
SENTRY_CLI_TRACES_SAMPLE_RATE: float = 1.0
Expand Down
62 changes: 62 additions & 0 deletions tests/api/test_statements_get.py
Original file line number Diff line number Diff line change
Expand Up @@ -148,12 +148,14 @@ def test_api_statements_get_statements_mine(
"timestamp": (datetime.now() - timedelta(hours=1)).isoformat(),
"actor": agent_1,
"authority": agent_1,
"version": "1.0.0"
},
{
"id": "72c81e98-1763-4730-8cfc-f5ab34f1bad2",
"timestamp": datetime.now().isoformat(),
"actor": agent_1,
"authority": agent_2,
"version": "1.0.3"
},
]
insert_statements_and_monkeypatch_backend(statements)
Expand All @@ -164,6 +166,7 @@ def test_api_statements_get_statements_mine(
headers={"Authorization": f"Basic {credentials_1_bis}"},
)
assert response.status_code == 200
assert len(response.json()["statements"]) == 2
assert response.json() == {"statements": [statements[1], statements[0]]}

# No restriction on "mine" (explicit) : Return all statements
Expand Down Expand Up @@ -229,10 +232,12 @@ def test_api_statements_get_statements(
{
"id": "be67b160-d958-4f51-b8b8-1892002dbac6",
"timestamp": (datetime.now() - timedelta(hours=1)).isoformat(),
"version": "1.0.0"
},
{
"id": "72c81e98-1763-4730-8cfc-f5ab34f1bad2",
"timestamp": datetime.now().isoformat(),
"version": "1.0.0"
},
]
insert_statements_and_monkeypatch_backend(statements)
Expand All @@ -246,6 +251,39 @@ def test_api_statements_get_statements(
assert response.status_code == 200
assert response.json() == {"statements": [statements[1], statements[0]]}

# Test that version is in response header
assert response.headers["X-Experience-API-Version"] == "1.0.3"

def test_api_statements_get_statements_version(
insert_statements_and_monkeypatch_backend,
auth_credentials
):
"""Test that statements are returned with the proper version according to
xAPI specification (existing version or 1.0.0)."""

statements = [
{
"id": "be67b160-d958-4f51-b8b8-1892002dbac6",
"timestamp": (datetime.now() + timedelta(hours=1)).isoformat(),
"version": "1.0.3"
},
{
"id": "72c81e98-1763-4730-8cfc-f5ab34f1bad2",
"timestamp": datetime.now().isoformat(),
},
]
insert_statements_and_monkeypatch_backend(statements)

response = client.get(
"xAPI/statements", headers={"Authorization": f"Basic {auth_credentials}"}
)
assert response.status_code == 200

# Test that statement with existing `version` is unchanged
assert response.json()["statements"][0]["version"] == "1.0.3"

# Test that statement with no `version` is assigned 1.0.0
assert response.json()["statements"][1]["version"] == "1.0.0"

def test_api_statements_get_statements_ascending(
insert_statements_and_monkeypatch_backend, auth_credentials
Expand All @@ -259,10 +297,12 @@ def test_api_statements_get_statements_ascending(
{
"id": "be67b160-d958-4f51-b8b8-1892002dbac6",
"timestamp": (datetime.now() - timedelta(hours=1)).isoformat(),
"version": "1.0.0"
},
{
"id": "72c81e98-1763-4730-8cfc-f5ab34f1bad2",
"timestamp": datetime.now().isoformat(),
"version": "1.0.0"
},
]
insert_statements_and_monkeypatch_backend(statements)
Expand All @@ -288,10 +328,12 @@ def test_api_statements_get_statements_by_statement_id(
{
"id": "be67b160-d958-4f51-b8b8-1892002dbac6",
"timestamp": (datetime.now() - timedelta(hours=1)).isoformat(),
"version": "1.0.0"
},
{
"id": "72c81e98-1763-4730-8cfc-f5ab34f1bad2",
"timestamp": datetime.now().isoformat(),
"version": "1.0.0"
},
]
insert_statements_and_monkeypatch_backend(statements)
Expand Down Expand Up @@ -340,12 +382,14 @@ def test_api_statements_get_statements_by_agent(
"timestamp": datetime.now().isoformat(),
"actor": agent_1,
"authority": agent_1,
"version": "1.0.0"
},
{
"id": "72c81e98-1763-4730-8cfc-f5ab34f1bad2",
"timestamp": datetime.now().isoformat(),
"actor": agent_2,
"authority": agent_1,
"version": "1.0.0"
},
]
insert_statements_and_monkeypatch_backend(statements)
Expand All @@ -372,11 +416,13 @@ def test_api_statements_get_statements_by_verb(
"id": "be67b160-d958-4f51-b8b8-1892002dbac6",
"timestamp": datetime.now().isoformat(),
"verb": {"id": "http://adlnet.gov/expapi/verbs/experienced"},
"version": "1.0.0"
},
{
"id": "72c81e98-1763-4730-8cfc-f5ab34f1bad2",
"timestamp": datetime.now().isoformat(),
"verb": {"id": "http://adlnet.gov/expapi/verbs/played"},
"version": "1.0.0"
},
]
insert_statements_and_monkeypatch_backend(statements)
Expand Down Expand Up @@ -406,11 +452,13 @@ def test_api_statements_get_statements_by_activity(
"id": "be67b160-d958-4f51-b8b8-1892002dbac6",
"timestamp": datetime.now().isoformat(),
"object": activity_0,
"version": "1.0.0"
},
{
"id": "72c81e98-1763-4730-8cfc-f5ab34f1bad2",
"timestamp": datetime.now().isoformat(),
"object": activity_1,
"version": "1.0.0"
},
]
insert_statements_and_monkeypatch_backend(statements)
Expand Down Expand Up @@ -445,10 +493,12 @@ def test_api_statements_get_statements_since_timestamp(
{
"id": "be67b160-d958-4f51-b8b8-1892002dbac6",
"timestamp": (datetime.now() - timedelta(hours=1)).isoformat(),
"version": "1.0.0"
},
{
"id": "72c81e98-1763-4730-8cfc-f5ab34f1bad2",
"timestamp": datetime.now().isoformat(),
"version": "1.0.0"
},
]
insert_statements_and_monkeypatch_backend(statements)
Expand All @@ -475,10 +525,12 @@ def test_api_statements_get_statements_until_timestamp(
{
"id": "be67b160-d958-4f51-b8b8-1892002dbac6",
"timestamp": (datetime.now() - timedelta(hours=1)).isoformat(),
"version": "1.0.0"
},
{
"id": "72c81e98-1763-4730-8cfc-f5ab34f1bad2",
"timestamp": datetime.now().isoformat(),
"version": "1.0.0"
},
]
insert_statements_and_monkeypatch_backend(statements)
Expand Down Expand Up @@ -510,22 +562,27 @@ def test_api_statements_get_statements_with_pagination(
{
"id": "5d345b99-517c-4b54-848e-45010904b177",
"timestamp": (datetime.now() - timedelta(hours=4)).isoformat(),
"version": "1.0.0"
},
{
"id": "be67b160-d958-4f51-b8b8-1892002dbac6",
"timestamp": (datetime.now() - timedelta(hours=3)).isoformat(),
"version": "1.0.0"
},
{
"id": "be67b160-d958-4f51-b8b8-1892002dbac5",
"timestamp": (datetime.now() - timedelta(hours=2)).isoformat(),
"version": "1.0.0"
},
{
"id": "be67b160-d958-4f51-b8b8-1892002dbac4",
"timestamp": (datetime.now() - timedelta(hours=1)).isoformat(),
"version": "1.0.0"
},
{
"id": "72c81e98-1763-4730-8cfc-f5ab34f1bad2",
"timestamp": datetime.now().isoformat(),
"version": "1.0.0"
},
]
insert_statements_and_monkeypatch_backend(statements)
Expand Down Expand Up @@ -585,6 +642,7 @@ def test_api_statements_get_statements_with_pagination_and_query(
"display": {"en-US": "played"},
},
"timestamp": (datetime.now() - timedelta(hours=2)).isoformat(),
"version": "1.0.0"
},
{
"id": "be67b160-d958-4f51-b8b8-1892002dbac1",
Expand All @@ -593,6 +651,7 @@ def test_api_statements_get_statements_with_pagination_and_query(
"display": {"en-US": "played"},
},
"timestamp": (datetime.now() - timedelta(hours=1)).isoformat(),
"version": "1.0.0"
},
{
"id": "72c81e98-1763-4730-8cfc-f5ab34f1bad2",
Expand All @@ -601,6 +660,7 @@ def test_api_statements_get_statements_with_pagination_and_query(
"display": {"en-US": "played"},
},
"timestamp": datetime.now().isoformat(),
"version": "1.0.0"
},
]
insert_statements_and_monkeypatch_backend(statements)
Expand Down Expand Up @@ -640,10 +700,12 @@ def test_api_statements_get_statements_with_no_matching_statement(
{
"id": "be67b160-d958-4f51-b8b8-1892002dbac6",
"timestamp": (datetime.now() - timedelta(hours=1)).isoformat(),
"version": "1.0.0"
},
{
"id": "72c81e98-1763-4730-8cfc-f5ab34f1bad2",
"timestamp": datetime.now().isoformat(),
"version": "1.0.0"
},
]
insert_statements_and_monkeypatch_backend(statements)
Expand Down
3 changes: 3 additions & 0 deletions tests/api/test_statements_post.py
Original file line number Diff line number Diff line change
Expand Up @@ -304,6 +304,9 @@ def test_api_statements_post_statements_list_of_one(
response.json(), {"statements": [statement]}
)

# Test that version is in response header
assert response.headers["X-Experience-API-Version"] == "1.0.3"


@pytest.mark.parametrize(
"backend",
Expand Down
3 changes: 3 additions & 0 deletions tests/api/test_statements_put.py
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,9 @@ def test_api_statements_put_single_statement_directly(
)

assert response.status_code == 204

# Test that version is in response header
assert response.headers["X-Experience-API-Version"] == "1.0.3"

es.indices.refresh()

Expand Down