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

Handle StaleDataError for ODLNotificationController #2129

Merged
merged 2 commits into from
Oct 22, 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
11 changes: 10 additions & 1 deletion src/palace/manager/api/controller/odl_notification.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
from pydantic import ValidationError
from sqlalchemy import select
from sqlalchemy.orm import Session
from sqlalchemy.orm.exc import StaleDataError

from palace.manager.api.odl.api import OPDS2WithODLApi
from palace.manager.api.problem_details import (
Expand Down Expand Up @@ -101,6 +102,14 @@ def _process_notification(self, loan: Loan | None) -> Response:
# Once we move the OPDS2WithODL scripts to celery this should be possible.
# For now we just mark the loan as expired.
if not status_doc.active:
loan.end = utc_now()
try:
with self.db.begin_nested():
loan.end = utc_now()
except StaleDataError:
# This can happen if this callback happened while we were returning this
# item. We can fetch the loan, but it's deleted by the time we go to do
# the update. This is not a problem, as we were just marking the loan as
# completed anyway so we just continue.
...

return Response(status=204)
24 changes: 24 additions & 0 deletions tests/manager/api/controller/test_odl_notify.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
from unittest.mock import PropertyMock, create_autospec

import pytest
from flask import Response
from freezegun import freeze_time
from sqlalchemy.orm.exc import StaleDataError

from palace.manager.api.controller.odl_notification import ODLNotificationController
from palace.manager.api.odl.api import OPDS2WithODLApi
Expand All @@ -9,6 +12,7 @@
NO_ACTIVE_LOAN,
)
from palace.manager.core.problem_details import INVALID_INPUT
from palace.manager.integration.goals import Goals
from palace.manager.sqlalchemy.model.collection import Collection
from palace.manager.sqlalchemy.model.licensing import License
from palace.manager.sqlalchemy.model.patron import Loan, Patron
Expand Down Expand Up @@ -249,3 +253,23 @@ def test_notify_errors(
odl_fixture.controller.notify(
odl_fixture.patron_identifier, NON_EXISTENT_LICENSE_IDENTIFIER
)

def test__process_notification_already_deleted(
self,
odl_fixture: ODLFixture,
flask_app_fixture: FlaskAppFixture,
db: DatabaseTransactionFixture,
) -> None:
mock_loan = create_autospec(Loan)
type(mock_loan).end = PropertyMock(side_effect=StaleDataError())
mock_loan.license_pool.collection.integration_configuration.protocol = (
db.protocol_string(Goals.LICENSE_GOAL, OPDS2WithODLApi)
)
with flask_app_fixture.test_request_context(
"/",
method="POST",
library=odl_fixture.library,
data=odl_fixture.loan_status_document("revoked").model_dump_json(),
):
response = odl_fixture.controller._process_notification(mock_loan)
assert response.status_code == 204