From 5ab1a6f92406a50b5479f7ab38d3d4edf6c75794 Mon Sep 17 00:00:00 2001 From: Jonathan Green Date: Thu, 9 Sep 2021 14:28:31 -0300 Subject: [PATCH 1/7] Add additional tests for ODL - Test a feed with multiple license blocks. - Test a single license block with different numbers of licenses available. --- tests/files/odl/multiple_license.opds | 141 +++++++++++++++ tests/files/odl2/multiple_license.json | 235 +++++++++++++++++++++++++ tests/test_odl.py | 189 +++++++++++++++++--- tests/test_odl2.py | 18 +- 4 files changed, 552 insertions(+), 31 deletions(-) create mode 100644 tests/files/odl/multiple_license.opds create mode 100644 tests/files/odl2/multiple_license.json diff --git a/tests/files/odl/multiple_license.opds b/tests/files/odl/multiple_license.opds new file mode 100644 index 0000000000..96716d0eaf --- /dev/null +++ b/tests/files/odl/multiple_license.opds @@ -0,0 +1,141 @@ + + + https://market.example.com/api/libraries/harvest.atom + Example + 2021-09-09T14:19:33Z + /favicon.ico + + Example + https://market.example.com + support@example.zendesk.com + + +481 +100 + + +The Murmur of Bees +https://www.example.com/item/4154969 +urn:ISBN:9781542090490 +urn:ISBN:9781542040501 +476 + + Sofía Segovia + https://example.com/store/browse/recent.atom?author_id=519449&lang=en + + + Simon Bruni + https://example.com/store/browse/recent.atom?contributor_id=1447607&lang=en + +2021-06-14T17:38:53Z +2021-07-09T13:10:07Z +en +Amazon Publishing +2019-04-16 + +From a beguiling voice in Mexican fiction comes an astonishing novel—her first to be translated into English—about a mysterious child with the power to change a family’s history in a country on the verge of revolution. + +From the day that old Nana Reja found a baby abandoned under a bridge, the life of a small Mexican town forever changed. Disfigured and covered in a blanket of bees, little Simonopio is for some locals the stuff of superstition, a child kissed by the devil. But he is welcomed by landowners Francisco and Beatriz Morales, who adopt him and care for him as if he were their own. As he grows up, Simonopio becomes a cause for wonder to the Morales family, because when the uncannily gifted child closes his eyes, he can see what no one else can—visions of all that’s yet to come, both beautiful and dangerous. Followed by his protective swarm of bees and living to deliver his adoptive family from threats—both human and those of nature—Simonopio’s purpose in Linares will, in time, be divined. + +Set against the backdrop of the Mexican Revolution and the devastating influenza of 1918, +The Murmur of Bees +captures both the fate of a country in flux and the destiny of one family that has put their love, faith, and future in the unbelievable. + +476 pages +5 MB + + + + + + + + + + + + urn:uuid:ed3c2fe4-d44d-427e-afde-cf246d21ecd9 + application/epub+zip + text/html + 2021-06-14T19:46:17+02:00 + + {{expires_1}} + 40 + 10 + 5097600 + + + application/vnd.adobe.adept+xml + 6 + true + false + false + + + application/vnd.readium.lcp.license.v1.0+json + 6 + true + false + false + + + + + + urn:uuid:c9079f86-3cdb-4144-b106-04bd70973586 + application/epub+zip + text/html + 2021-07-09T15:08:55+02:00 + + {{expires_2}} + 40 + 10 + 5097600 + + + application/vnd.adobe.adept+xml + 6 + true + false + false + + + application/vnd.readium.lcp.license.v1.0+json + 6 + true + false + false + + + + + + urn:uuid:49f6bb8d-1912-4b95-8e62-a65d7c8c02fd + application/epub+zip + text/html + 2021-07-09T15:08:55+02:00 + + {{expires_3}} + 40 + 10 + 5097600 + + + application/vnd.adobe.adept+xml + 6 + true + false + false + + + application/vnd.readium.lcp.license.v1.0+json + 6 + true + false + false + + + + + + diff --git a/tests/files/odl2/multiple_license.json b/tests/files/odl2/multiple_license.json new file mode 100644 index 0000000000..e76f62acbf --- /dev/null +++ b/tests/files/odl2/multiple_license.json @@ -0,0 +1,235 @@ +{ + "metadata": { + "title": "Test", + "itemsPerPage": 10, + "currentPage": 1, + "numberOfItems": 100 + }, + "links": [ + { + "type": "application/opds+json", + "rel": "self", + "href": "https://market.feedbooks.com/api/libraries/harvest.json" + } + ], + "publications": [ + { + "metadata": { + "@type": "http://schema.org/Book", + "title": "The Murmur of Bees", + "language": "en", + "modified": "2021-07-09T13:10:07Z", + "published": "2019-04-16T00:00:00Z", + "numberOfPages": 476, + "identifier": "urn:ISBN:9781542090490", + "schema:workExample": [ + { + "@type": "http://schema.org/Book", + "schema:bookFormat": "http://schema.org/Hardcover", + "schema:isbn": "urn:ISBN:9781542040501" + } + ], + "author": [ + { + "name": "Sofía Segovia", + "links": [ + { + "type": "application/opds+json", + "href": "https://example.com/store/browse/recent.json?author_id=519449&lang=en" + } + ] + } + ], + "translator": [ + { + "name": "Simon Bruni", + "links": [ + { + "type": "application/opds+json", + "href": "https://example.com/store/browse/recent.json?contributor_id=1447607&lang=en" + } + ] + } + ], + "publisher": { + "name": "Amazon Publishing", + "links": [ + { + "type": "application/opds+json", + "href": "https://example.com/store/browse/recent.json?lang=en&publisher=Amazon+Publishing" + } + ] + }, + "description": "

\nFrom a beguiling voice in Mexican fiction comes an astonishing novel—her first to be translated into English—about a mysterious child with the power to change a family’s history in a country on the verge of revolution.\n

\n

From the day that old Nana Reja found a baby abandoned under a bridge, the life of a small Mexican town forever changed. Disfigured and covered in a blanket of bees, little Simonopio is for some locals the stuff of superstition, a child kissed by the devil. But he is welcomed by landowners Francisco and Beatriz Morales, who adopt him and care for him as if he were their own. As he grows up, Simonopio becomes a cause for wonder to the Morales family, because when the uncannily gifted child closes his eyes, he can see what no one else can—visions of all that’s yet to come, both beautiful and dangerous. Followed by his protective swarm of bees and living to deliver his adoptive family from threats—both human and those of nature—Simonopio’s purpose in Linares will, in time, be divined.

\n

\nSet against the backdrop of the Mexican Revolution and the devastating influenza of 1918,\nThe Murmur of Bees\ncaptures both the fate of a country in flux and the destiny of one family that has put their love, faith, and future in the unbelievable.\n

", + "subject": [ + { + "code": "FBFIC000000", + "name": "Fiction", + "scheme": "http://www.feedbooks.com/categories", + "links": [ + { + "type": "application/opds+json", + "href": "https://example.com/category/FBFIC000000.json?lang=en" + } + ] + }, + { + "code": "FBFIC014000", + "name": "Historical", + "scheme": "http://www.feedbooks.com/categories", + "links": [ + { + "type": "application/opds+json", + "href": "https://example.com/category/FBFIC014000.json?lang=en" + } + ] + }, + { + "code": "FBFIC019000", + "name": "Literary", + "scheme": "http://www.feedbooks.com/categories", + "links": [ + { + "type": "application/opds+json", + "href": "https://example.com/category/FBFIC019000.json?lang=en" + } + ] + } + ] + }, + "images": [ + { + "href": "https://covers.example.com/item/4154969.jpg?size=large", + "type": "image/jpeg", + "width": 260, + "height": 420 + }, + { + "href": "https://covers.example.com/item/4154969.jpg", + "type": "image/jpeg", + "width": 100, + "height": 180 + } + ], + "licenses": [ + { + "metadata": { + "identifier": "urn:uuid:ed3c2fe4-d44d-427e-afde-cf246d21ecd9", + "format": [ + "application/epub+zip", + "text/html" + ], + "created": "2021-06-14T19:46:17+02:00", + "terms": { + "expires": "{{expires_1}}", + "checkouts": 40, + "concurrency": 10, + "length": 5097600 + }, + "protection": { + "format": [ + "application/vnd.adobe.adept+xml", + "application/vnd.readium.lcp.license.v1.0+json" + ], + "devices": 6, + "copy": true, + "print": false, + "tts": false + } + }, + "links": [ + { + "rel": "http://opds-spec.org/acquisition/borrow", + "href": "https://license.example.com/loan/status/{?id,checkout_id,expires,patron_id,notification_url,passphrase,hint,hint_url}", + "type": "application/vnd.readium.license.status.v1.0+json", + "templated": true + }, + { + "rel": "self", + "href": "https://license.example.com/copy/status/?uuid=ed3c2fe4-d44d-427e-afde-cf246d21ecd9", + "type": "application/vnd.odl.info+json" + } + ] + }, + { + "metadata": { + "identifier": "urn:uuid:c9079f86-3cdb-4144-b106-04bd70973586", + "format": [ + "application/epub+zip", + "text/html" + ], + "created": "2021-07-09T15:08:55+02:00", + "terms": { + "expires": "{{expires_2}}", + "checkouts": 40, + "concurrency": 10, + "length": 5097600 + }, + "protection": { + "format": [ + "application/vnd.adobe.adept+xml", + "application/vnd.readium.lcp.license.v1.0+json" + ], + "devices": 6, + "copy": true, + "print": false, + "tts": false + } + }, + "links": [ + { + "rel": "http://opds-spec.org/acquisition/borrow", + "href": "https://license.example.com/loan/status/{?id,checkout_id,expires,patron_id,notification_url,passphrase,hint,hint_url}", + "type": "application/vnd.readium.license.status.v1.0+json", + "templated": true + }, + { + "rel": "self", + "href": "https://license.example.com/copy/status/?uuid=c9079f86-3cdb-4144-b106-04bd70973586", + "type": "application/vnd.odl.info+json" + } + ] + }, + { + "metadata": { + "identifier": "urn:uuid:49f6bb8d-1912-4b95-8e62-a65d7c8c02fd", + "format": [ + "application/epub+zip", + "text/html" + ], + "created": "2021-07-09T15:08:55+02:00", + "terms": { + "expires": "{{expires_3}}", + "checkouts": 40, + "concurrency": 10, + "length": 5097600 + }, + "protection": { + "format": [ + "application/vnd.adobe.adept+xml", + "application/vnd.readium.lcp.license.v1.0+json" + ], + "devices": 6, + "copy": true, + "print": false, + "tts": false + } + }, + "links": [ + { + "rel": "http://opds-spec.org/acquisition/borrow", + "href": "https://license.example.com/loan/status/{?id,checkout_id,expires,patron_id,notification_url,passphrase,hint,hint_url}", + "type": "application/vnd.readium.license.status.v1.0+json", + "templated": true + }, + { + "rel": "self", + "href": "https://license.example.com/copy/status/?uuid=49f6bb8d-1912-4b95-8e62-a65d7c8c02fd", + "type": "application/vnd.odl.info+json" + } + ] + } + ] + } + ] +} diff --git a/tests/test_odl.py b/tests/test_odl.py index a7028f90d4..a8ff5a484c 100644 --- a/tests/test_odl.py +++ b/tests/test_odl.py @@ -2,6 +2,7 @@ import json import os import urllib.parse +from typing import Callable, List, Tuple import dateutil import pytest @@ -29,10 +30,12 @@ ExternalIntegration, Hold, Hyperlink, + LicensePool, Loan, MediaTypes, Representation, RightsStatus, + Work ) from core.scripts import RunCollectionMonitorScript from core.testing import DatabaseTest @@ -1933,10 +1936,11 @@ def canonicalize_author_name(self, identifier, working_display_name): class TestODLExpiredItemsReaper(DatabaseTest, BaseODLTest): ODL_PROTOCOL = ODLAPI.NAME - ODL_FEED_FILENAME_WITH_SINGLE_ODL_LICENSE = "single_license.opds" - ODL_LICENSE_EXPIRATION_TIME_PLACEHOLDER = "{{expires}}" + ODL_FEED_FILENAME = None + ODL_LICENSE_EXPIRATION_TIME_PLACEHOLDER = [] ODL_REAPER_CLASS = ODLExpiredItemsReaper - SECONDS_PER_HOUR = 3600 + LICENSES_AVAILABLE = None + LICENSES_LEFT = None def _create_importer(self, collection, http_get): """Create a new ODL importer with the specified parameters. @@ -1959,30 +1963,27 @@ def _create_importer(self, collection, http_get): return importer - def _get_test_feed_with_single_odl_license(self, expires): - """Get the feed with a single ODL license with the specific expiration date. + def _get_test_feed(self, expires: List[datetime.datetime]) -> str: + """Get the feed and template the specific expiration dates. - :param expires: Expiration date of the ODL license - :type expires: datetime.datetime + :param expires: List of expiration dates for the ODL licenses in feed. - :return: Test ODL feed with a single ODL license with the specific expiration date - :rtype: str + :return: Test ODL feed. """ - feed = self.get_data(self.ODL_FEED_FILENAME_WITH_SINGLE_ODL_LICENSE) - feed = feed.replace(self.ODL_LICENSE_EXPIRATION_TIME_PLACEHOLDER, expires.isoformat()) + feed = self.get_data(self.ODL_FEED_FILENAME) + for idx, expire in enumerate(expires): + feed = feed.replace(self.ODL_LICENSE_EXPIRATION_TIME_PLACEHOLDER[idx], expire.isoformat()) return feed - def _import_test_feed_with_single_odl_license(self, expires): - """Import the test ODL feed with a single ODL license with the specific expiration date. + def _import_test_feed(self, expires: List[datetime.datetime]) -> Tuple[List[Edition], List[LicensePool], List[Work]]: + """Import the test ODL feed, templated with specific expiration dates. :param expires: Expiration date of the ODL license - :type expires: datetime.datetime :return: 3-tuple containing imported editions, license pools and works - :rtype: Tuple[List[Edition], List[LicensePool], List[Work]] """ - feed = self._get_test_feed_with_single_odl_license(expires) + feed = self._get_test_feed(expires) data_source = DataSource.lookup(self._db, "Feedbooks", autocreate=True) collection = MockODLAPI.mock_collection(self._db, protocol=self.ODL_PROTOCOL) collection.external_integration.set_setting( @@ -1991,7 +1992,8 @@ def _import_test_feed_with_single_odl_license(self, expires): ) license_status = { "checkouts": { - "available": 1 + "available": self.LICENSES_AVAILABLE, + "left": self.LICENSES_LEFT } } license_status_response = MagicMock(return_value=(200, {}, json.dumps(license_status))) @@ -2003,6 +2005,12 @@ def _import_test_feed_with_single_odl_license(self, expires): return imported_editions, imported_pools, imported_works + +class TestODLExpiredItemsReaperSingleLicense(TestODLExpiredItemsReaper): + ODL_FEED_FILENAME = "single_license.opds" + ODL_LICENSE_EXPIRATION_TIME_PLACEHOLDER = ["{{expires}}"] + LICENSES_AVAILABLE = 1 + @freeze_time("2021-01-01T00:00:00+00:00") def test_odl_importer_skips_expired_licenses(self): """Ensure ODLImporter skips expired licenses @@ -2010,10 +2018,8 @@ def test_odl_importer_skips_expired_licenses(self): # 1.1. Import the test feed with an expired ODL license. # The license expires 2021-01-01T00:01:00+01:00 that equals to 2010-01-01T00:00:00+00:00, the current time. # It means the license had already expired at the time of the import. - license_expiration_date = datetime.datetime(2021, 1, 1, 1, 0, 0, tzinfo=tzoffset(None, self.SECONDS_PER_HOUR)) - imported_editions, imported_pools, imported_works = self._import_test_feed_with_single_odl_license( - license_expiration_date - ) + license_expiration_date = dateutil.parser.isoparse("2021-01-01T00:01:00+01:00") + imported_editions, imported_pools, imported_works = self._import_test_feed([license_expiration_date]) # Commit to expire the SQLAlchemy cache. self._db.commit() @@ -2034,9 +2040,7 @@ def test_odl_reaper_removes_expired_licenses(self): # 1.1. Import the test feed with an ODL license that is still valid. # The license will be valid for one more day since this very moment. license_expiration_date = datetime_helpers.utc_now() + datetime.timedelta(days=1) - imported_editions, imported_pools, imported_works = self._import_test_feed_with_single_odl_license( - license_expiration_date - ) + imported_editions, imported_pools, imported_works = self._import_test_feed([license_expiration_date]) # Commit to expire the SQLAlchemy cache. self._db.commit() @@ -2067,9 +2071,8 @@ def test_odl_reaper_removes_expired_licenses(self): assert imported_pool.licenses_available == 1 # 4. Expire the license. - # Set the expiration date to 2021-01-01T00:01:00+01:00 - # that equals to 2010-01-01T00:00:00+00:00, the current time. - license.expires = datetime.datetime(2021, 1, 1, 1, 0, 0, tzinfo=tzoffset(None, self.SECONDS_PER_HOUR)) + # Set the expiration date to yesterday. + license.expires = datetime_helpers.utc_now() - datetime.timedelta(days=1) # 5.1. Run ODLExpiredItemsReaper again. This time it should remove the expired license. script.run() @@ -2090,3 +2093,135 @@ def test_odl_reaper_removes_expired_licenses(self): # 6.2. Ensure that number of licenses is still 0. assert imported_pool.licenses_owned == 0 assert imported_pool.licenses_available == 0 + + @freeze_time("2021-01-01T00:00:00+00:00") + def test_odl_reaper_removes_expired_licenses_with_multiple_available(self): + """Ensure ODLExpiredItemsReaper removes expired licenses.""" + # 1.1. Import the test feed with an ODL license that is still valid. Set the number of licenses + # available to 5 to make sure that the licenses are not available once the license expires. + license_expiration_date = datetime_helpers.utc_now() + datetime.timedelta(days=1) + self.LICENSES_AVAILABLE = 5 + imported_editions, imported_pools, imported_works = self._import_test_feed([license_expiration_date]) + + # Commit to expire the SQLAlchemy cache. + self._db.commit() + + # 1.2. Ensure that there is a license pool with available license. + assert len(imported_pools) == 1 + + [imported_pool] = imported_pools + assert imported_pool.licenses_available == self.LICENSES_AVAILABLE + + assert len(imported_pool.licenses) == 1 + [license] = imported_pool.licenses + assert license.expires == license_expiration_date + + # 2.1. Expire the license. + # Set the expiration date to yesterday. + license.expires = datetime_helpers.utc_now() - datetime.timedelta(days=1) + + # 2.2. Run ODLExpiredItemsReaper. It should remove the expired license. + script = RunCollectionMonitorScript(self.ODL_REAPER_CLASS, _db=self._db, cmd_args=["Test ODL Collection"]) + script.run() + + # Commit to expire the SQLAlchemy cache. + self._db.commit() + + # 2.3. Ensure that availability of the license pool was updated and now it doesn't have any available licenses. + assert imported_pool.licenses_available == 0 + + +class TestODLExpiredItemsReaperMultipleLicense(TestODLExpiredItemsReaper): + ODL_FEED_FILENAME = "multiple_license.opds" + ODL_LICENSE_EXPIRATION_TIME_PLACEHOLDER = ["{{expires_1}}", "{{expires_2}}", "{{expires_3}}"] + LICENSES_AVAILABLE = 8 + LICENSES_LEFT = 18 + LICENSES_OWNED = 10 + + @freeze_time("2021-01-01T00:00:00+00:00") + def test_odl_importer_skips_expired_licenses(self): + """Ensure ODLImporter skips expired licenses + and does not count them in the total number of available licenses.""" + # 1.1. Import the test feed with one expired ODL license and two valid licenses. + license_expiration_dates = [ + datetime_helpers.utc_now() - datetime.timedelta(days=1), # Expired + datetime_helpers.utc_now() + datetime.timedelta(days=1), # Valid + datetime_helpers.utc_now() + datetime.timedelta(weeks=12) # Valid + ] + imported_editions, imported_pools, imported_works = self._import_test_feed(license_expiration_dates) + + # Commit to expire the SQLAlchemy cache. + self._db.commit() + + # 1.2. Ensure that the license pool was successfully created + assert len(imported_pools) == 1 + [imported_pool] = imported_pools + + # 1.3. Ensure that the two valid licenses were imported + assert len(imported_pool.licenses) == 2 + + # 1.4 Make sure that 20 licenses are marked as owned (10 from each valid license) + assert imported_pool.licenses_owned == self.LICENSES_OWNED * 2 + assert imported_pool.licenses_available == self.LICENSES_AVAILABLE * 2 + + @freeze_time("2021-01-01T00:00:00+00:00") + def test_odl_reaper_removes_expired_licenses(self): + """Ensure ODLExpiredItemsReaper removes expired licenses.""" + # 1.1. Import the test feed with an ODL licenses that are still valid. + license_expiration_dates = [ + datetime_helpers.utc_now() + datetime.timedelta(days=1), + datetime_helpers.utc_now() + datetime.timedelta(days=60), + datetime_helpers.utc_now() + datetime.timedelta(days=365) + ] + imported_editions, imported_pools, imported_works = self._import_test_feed(license_expiration_dates) + + # Commit to expire the SQLAlchemy cache. + self._db.commit() + + # 1.2. Ensure that there is a license pool with available license. + assert len(imported_pools) == 1 + + [imported_pool] = imported_pools + assert imported_pool.licenses_owned == 3 * self.LICENSES_OWNED + assert imported_pool.licenses_available == 3 * self.LICENSES_AVAILABLE + assert len(imported_pool.licenses) == 3 + + [license1, license2, license3] = imported_pool.licenses + assert license1.expires == license_expiration_dates[0] + assert license2.expires == license_expiration_dates[1] + assert license3.expires == license_expiration_dates[2] + + # 2.1. Run ODLExpiredItemsReaper. This time nothing should happen since the license is still valid. + script = RunCollectionMonitorScript(self.ODL_REAPER_CLASS, _db=self._db, cmd_args=["Test ODL Collection"]) + script.run() + + # Commit to expire the SQLAlchemy cache. + self._db.commit() + + # 2.2. Ensure that availability of the license pool didn't change. + assert imported_pool.licenses_owned == 3 * self.LICENSES_OWNED + assert imported_pool.licenses_available == 3 * self.LICENSES_AVAILABLE + assert len(imported_pool.licenses) == 3 + + # 3. Expire the license. + license1.expires = datetime_helpers.utc_now() - datetime.timedelta(days=1) + + # 3.1. Run ODLExpiredItemsReaper again. This time it should remove the expired license. + script.run() + + # Commit to expire the SQLAlchemy cache. + self._db.commit() + + # 3.2. Ensure that availability of the license pool was updated + assert imported_pool.licenses_owned == 2 * self.LICENSES_OWNED + assert imported_pool.licenses_available == 2 * self.LICENSES_AVAILABLE + + # 4.1. Run ODLExpiredItemsReaper again to ensure that number of licenses won't become negative. + script.run() + + # Commit to expire the SQLAlchemy cache. + self._db.commit() + + # 4.2. Ensure that number of licenses is still 0. + assert imported_pool.licenses_owned == 2 * self.LICENSES_OWNED + assert imported_pool.licenses_available == 2 * self.LICENSES_AVAILABLE diff --git a/tests/test_odl2.py b/tests/test_odl2.py index 85d4370096..28579f68a7 100644 --- a/tests/test_odl2.py +++ b/tests/test_odl2.py @@ -28,7 +28,10 @@ from core.model.configuration import ConfigurationFactory, ConfigurationStorage from core.opds2_import import RWPMManifestParser from core.tests.test_opds2_import import OPDS2Test -from tests.test_odl import TestODLExpiredItemsReaper +from tests.test_odl import ( + TestODLExpiredItemsReaperMultipleLicense, + TestODLExpiredItemsReaperSingleLicense, +) class TestODL2Importer(OPDS2Test): @@ -253,13 +256,12 @@ def test(self): assert str(huck_finn_semantic_error) == huck_finn_failure.exception -class TestODL2ExpiredItemsReaper(TestODLExpiredItemsReaper): +class TestODL2ExpiredItemsReaper: __base_path = os.path.split(__file__)[0] resource_path = os.path.join(__base_path, "files", "odl2") - ODL_PROTOCOL = ODL2API.NAME - ODL_FEED_FILENAME_WITH_SINGLE_ODL_LICENSE = "single_license.json" ODL_REAPER_CLASS = ODL2ExpiredItemsReaper + ODL_PROTOCOL = ODL2API.NAME def _create_importer(self, collection, http_get): """Create a new ODL importer with the specified parameters. @@ -282,3 +284,11 @@ def _create_importer(self, collection, http_get): ) return importer + + +class TestODL2ExpiredItemsReaperSingleLicensee(TestODL2ExpiredItemsReaper, TestODLExpiredItemsReaperSingleLicense): + ODL_FEED_FILENAME = "single_license.json" + + +class TestODL2ExpiredItemsReaperMultipleLicense(TestODL2ExpiredItemsReaper, TestODLExpiredItemsReaperMultipleLicense): + ODL_FEED_FILENAME = "multiple_license.json" From 98116f6cc01d3a6b58f2ff96413030e7188a1cec Mon Sep 17 00:00:00 2001 From: Jonathan Green Date: Thu, 9 Sep 2021 18:19:39 -0300 Subject: [PATCH 2/7] Update test comments. --- tests/test_odl.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/test_odl.py b/tests/test_odl.py index a8ff5a484c..a388ee245e 100644 --- a/tests/test_odl.py +++ b/tests/test_odl.py @@ -2167,7 +2167,7 @@ def test_odl_importer_skips_expired_licenses(self): @freeze_time("2021-01-01T00:00:00+00:00") def test_odl_reaper_removes_expired_licenses(self): """Ensure ODLExpiredItemsReaper removes expired licenses.""" - # 1.1. Import the test feed with an ODL licenses that are still valid. + # 1.1. Import the test feed with ODL licenses that are not expired. license_expiration_dates = [ datetime_helpers.utc_now() + datetime.timedelta(days=1), datetime_helpers.utc_now() + datetime.timedelta(days=60), @@ -2216,12 +2216,12 @@ def test_odl_reaper_removes_expired_licenses(self): assert imported_pool.licenses_owned == 2 * self.LICENSES_OWNED assert imported_pool.licenses_available == 2 * self.LICENSES_AVAILABLE - # 4.1. Run ODLExpiredItemsReaper again to ensure that number of licenses won't become negative. + # 4.1. Run ODLExpiredItemsReaper again to make sure that licenses are not expired twice. script.run() # Commit to expire the SQLAlchemy cache. self._db.commit() - # 4.2. Ensure that number of licenses is still 0. + # 4.2. Ensure that number of licenses remains the same as in step 3.2. assert imported_pool.licenses_owned == 2 * self.LICENSES_OWNED assert imported_pool.licenses_available == 2 * self.LICENSES_AVAILABLE From c0f63c1698db4536d47f3bbda4a948932423f53a Mon Sep 17 00:00:00 2001 From: Viacheslav Bessonov Date: Wed, 15 Sep 2021 20:40:59 +0500 Subject: [PATCH 3/7] Update the ODL licensing model and add additional tests --- api/odl.py | 78 ++- ...license.opds => feed_template.opds.jinja2} | 73 +-- tests/files/odl/multiple_license.opds | 141 ------ tests/files/odl2/feed_template.json.jinja2 | 118 +++++ tests/files/odl2/multiple_license.json | 235 --------- tests/files/odl2/single_license.json | 110 ---- tests/test_odl.py | 473 ++++++++++++------ 7 files changed, 547 insertions(+), 681 deletions(-) rename tests/files/odl/{single_license.opds => feed_template.opds.jinja2} (69%) delete mode 100644 tests/files/odl/multiple_license.opds create mode 100644 tests/files/odl2/feed_template.json.jinja2 delete mode 100644 tests/files/odl2/multiple_license.json delete mode 100644 tests/files/odl2/single_license.json diff --git a/api/odl.py b/api/odl.py index 67c5e014b7..c330cc24c7 100644 --- a/api/odl.py +++ b/api/odl.py @@ -1,5 +1,6 @@ import datetime import json +import logging import uuid from io import StringIO @@ -844,9 +845,10 @@ def _detail_for_elementtree_entry(cls, parser, entry_tag, feed_url=None, do_get= data['medium'] = medium expires = None + total_checkouts = None remaining_checkouts = None - available_checkouts = None concurrent_checkouts = None + available_concurrent_checkouts = None checkout_link = None for link_tag in parser._xpath(odl_license_tag, 'odl:tlink') or []: @@ -866,17 +868,21 @@ def _detail_for_elementtree_entry(cls, parser, entry_tag, feed_url=None, do_get= odl_status_link = attrib.get("href") break - # If we found one, retrieve it and get licensing information about this book. - if odl_status_link: - ignore, ignore, response = do_get(odl_status_link, headers={}) - status = json.loads(response) - checkouts = status.get("checkouts", {}) - remaining_checkouts = checkouts.get("left") - available_checkouts = checkouts.get("available") - terms = parser._xpath(odl_license_tag, "odl:terms") if terms: + total_checkouts = subtag(terms[0], "odl:total_checkouts") concurrent_checkouts = subtag(terms[0], "odl:concurrent_checkouts") + + if total_checkouts is not None: + total_checkouts = int(total_checkouts) + + if total_checkouts <= 0: + logging.info( + f"License # {identifier} expired since " + f"the total number of checkouts is {total_checkouts}" + ) + continue + expires = subtag(terms[0], "odl:expires") if expires: @@ -884,10 +890,37 @@ def _detail_for_elementtree_entry(cls, parser, entry_tag, feed_url=None, do_get= now = util.datetime_helpers.utc_now() if expires <= now: - continue + logging.info( + f"License # {identifier} expired at {expires} (now is {now})" + ) + continue - licenses_owned += int(concurrent_checkouts or 0) - licenses_available += int(available_checkouts or 0) + # If we found one, retrieve it and get licensing information about this book. + if odl_status_link: + ignore, ignore, response = do_get(odl_status_link, headers={}) + status = json.loads(response) + checkouts = status.get("checkouts", {}) + remaining_checkouts = checkouts.get("left") + available_concurrent_checkouts = checkouts.get("available") + + if remaining_checkouts is None: + remaining_checkouts = total_checkouts + + if remaining_checkouts is not None: + remaining_checkouts = int(remaining_checkouts) + + if remaining_checkouts <= 0: + logging.info( + f"License # {identifier} expired since " + f"the remaining number of checkouts is {remaining_checkouts}" + ) + continue + + if available_concurrent_checkouts is None: + available_concurrent_checkouts = concurrent_checkouts + + licenses_owned += int(remaining_checkouts or 0) + licenses_available += int(available_concurrent_checkouts or 0) licenses.append(LicenseData( identifier=identifier, @@ -895,7 +928,7 @@ def _detail_for_elementtree_entry(cls, parser, entry_tag, feed_url=None, do_get= status_url=odl_status_link, expires=expires, remaining_checkouts=remaining_checkouts, - concurrent_checkouts=concurrent_checkouts, + concurrent_checkouts=available_concurrent_checkouts, )) if not data.get('circulation'): @@ -1552,17 +1585,18 @@ def __init__(self, _db, collection): def process_item(self, identifier): for licensepool in identifier.licensed_through: - licenses_owned = licensepool.licenses_owned - licenses_available = licensepool.licenses_available + remaining_checkouts = 0 # total number of checkouts across all the licenses in the pool + concurrent_checkouts = 0 # number of concurrent checkouts allowed across all the licenses in the pool for license in licensepool.licenses: - if license.is_expired: - licenses_owned -= 1 - licenses_available -= 1 - - if licenses_owned != licensepool.licenses_owned or licenses_available != licensepool.licenses_available: - licenses_owned = max(licenses_owned, 0) - licenses_available = max(licenses_available, 0) + if not license.is_expired: + remaining_checkouts += license.remaining_checkouts + concurrent_checkouts += license.concurrent_checkouts + + if remaining_checkouts != licensepool.licenses_owned or \ + concurrent_checkouts != licensepool.licenses_available: + licenses_owned = max(remaining_checkouts, 0) + licenses_available = max(concurrent_checkouts, 0) circulation_data = CirculationData( data_source=licensepool.data_source, diff --git a/tests/files/odl/single_license.opds b/tests/files/odl/feed_template.opds.jinja2 similarity index 69% rename from tests/files/odl/single_license.opds rename to tests/files/odl/feed_template.opds.jinja2 index e89a17d610..bcd040c4a5 100644 --- a/tests/files/odl/single_license.opds +++ b/tests/files/odl/feed_template.opds.jinja2 @@ -45,35 +45,48 @@ - - urn:uuid:c981d61e-26f4-4070-aaa8-83df952cf61b - application/epub+zip - text/html - http://www.cantook.net/ - 40.00 - cant-2461538-24501117858552614-libraries - 2020-03-02T20:20:17+01:00 - - {{expires}} - 1 - 5097600 - - - application/vnd.adobe.adept+xml - 6 - true - false - false - - - application/vnd.readium.lcp.license.v1.0+json - 6 - true - false - false - - - - + + {% for license in licenses %} + + urn:uuid:{{ license.identifier }} + application/epub+zip + text/html + http://www.cantook.net/ + 40.00 + cant-2461538-24501117858552614-libraries + 2020-03-02T20:20:17+01:00 + + {% if license.total_checkouts is not none %} + {{ license.total_checkouts }} + {% endif %} + + {% if license.concurrent_checkouts is not none %} + {{ license.concurrent_checkouts }} + {% endif %} + + {% if license.expires is not none %} + {{ license.expires.isoformat() }} + {% endif %} + + 5097600 + + + application/vnd.adobe.adept+xml + 6 + true + false + false + + + application/vnd.readium.lcp.license.v1.0+json + 6 + true + false + false + + + + + {% endfor %} \ No newline at end of file diff --git a/tests/files/odl/multiple_license.opds b/tests/files/odl/multiple_license.opds deleted file mode 100644 index 96716d0eaf..0000000000 --- a/tests/files/odl/multiple_license.opds +++ /dev/null @@ -1,141 +0,0 @@ - - - https://market.example.com/api/libraries/harvest.atom - Example - 2021-09-09T14:19:33Z - /favicon.ico - - Example - https://market.example.com - support@example.zendesk.com - - -481 -100 - - -The Murmur of Bees -https://www.example.com/item/4154969 -urn:ISBN:9781542090490 -urn:ISBN:9781542040501 -476 - - Sofía Segovia - https://example.com/store/browse/recent.atom?author_id=519449&lang=en - - - Simon Bruni - https://example.com/store/browse/recent.atom?contributor_id=1447607&lang=en - -2021-06-14T17:38:53Z -2021-07-09T13:10:07Z -en -Amazon Publishing -2019-04-16 - -From a beguiling voice in Mexican fiction comes an astonishing novel—her first to be translated into English—about a mysterious child with the power to change a family’s history in a country on the verge of revolution. - -From the day that old Nana Reja found a baby abandoned under a bridge, the life of a small Mexican town forever changed. Disfigured and covered in a blanket of bees, little Simonopio is for some locals the stuff of superstition, a child kissed by the devil. But he is welcomed by landowners Francisco and Beatriz Morales, who adopt him and care for him as if he were their own. As he grows up, Simonopio becomes a cause for wonder to the Morales family, because when the uncannily gifted child closes his eyes, he can see what no one else can—visions of all that’s yet to come, both beautiful and dangerous. Followed by his protective swarm of bees and living to deliver his adoptive family from threats—both human and those of nature—Simonopio’s purpose in Linares will, in time, be divined. - -Set against the backdrop of the Mexican Revolution and the devastating influenza of 1918, -The Murmur of Bees -captures both the fate of a country in flux and the destiny of one family that has put their love, faith, and future in the unbelievable. - -476 pages -5 MB - - - - - - - - - - - - urn:uuid:ed3c2fe4-d44d-427e-afde-cf246d21ecd9 - application/epub+zip - text/html - 2021-06-14T19:46:17+02:00 - - {{expires_1}} - 40 - 10 - 5097600 - - - application/vnd.adobe.adept+xml - 6 - true - false - false - - - application/vnd.readium.lcp.license.v1.0+json - 6 - true - false - false - - - - - - urn:uuid:c9079f86-3cdb-4144-b106-04bd70973586 - application/epub+zip - text/html - 2021-07-09T15:08:55+02:00 - - {{expires_2}} - 40 - 10 - 5097600 - - - application/vnd.adobe.adept+xml - 6 - true - false - false - - - application/vnd.readium.lcp.license.v1.0+json - 6 - true - false - false - - - - - - urn:uuid:49f6bb8d-1912-4b95-8e62-a65d7c8c02fd - application/epub+zip - text/html - 2021-07-09T15:08:55+02:00 - - {{expires_3}} - 40 - 10 - 5097600 - - - application/vnd.adobe.adept+xml - 6 - true - false - false - - - application/vnd.readium.lcp.license.v1.0+json - 6 - true - false - false - - - - - - diff --git a/tests/files/odl2/feed_template.json.jinja2 b/tests/files/odl2/feed_template.json.jinja2 new file mode 100644 index 0000000000..316feb9bb4 --- /dev/null +++ b/tests/files/odl2/feed_template.json.jinja2 @@ -0,0 +1,118 @@ +{ + "metadata": { + "title": "Test", + "itemsPerPage": 10, + "currentPage": 1, + "numberOfItems": 100 + }, + "links": [ + { + "type": "application/opds+json", + "rel": "self", + "href": "https://market.feedbooks.com/api/libraries/harvest.json" + } + ], + "publications": [ + { + "metadata": { + "@type": "http://schema.org/Book", + "title": "Moby-Dick", + "author": "Herman Melville", + "identifier": "urn:isbn:978-3-16-148410-0", + "language": "en", + "publisher": { + "name": "Test Publisher" + }, + "published": "2015-09-29T00:00:00Z", + "modified": "2015-09-29T17:00:00Z", + "subject": [ + { + "scheme": "http://schema.org/audience", + "code": "juvenile-fiction", + "name": "Juvenile Fiction", + "links": [] + } + ] + }, + "links": [ + { + "rel": "self", + "href": "http://example.org/publication.json", + "type": "application/opds-publication+json" + } + ], + "images": [ + { + "href": "http://example.org/cover.jpg", + "type": "image/jpeg", + "height": 1400, + "width": 800 + }, + { + "href": "http://example.org/cover-small.jpg", + "type": "image/jpeg", + "height": 700, + "width": 400 + }, + { + "href": "http://example.org/cover.svg", + "type": "image/svg+xml" + } + ], + "licenses": [ + {% for license in licenses %} + { + "metadata": { + "identifier": "urn:uuid:{{ license.identifier }}", + "format": [ + "application/epub+zip", + "text/html", + "application/audiobook+json; protection=http://www.feedbooks.com/audiobooks/access-restriction" + ], + "price": { + "currency": "USD", + "value": 7.99 + }, + "created": "2014-04-25T12:25:21+02:00", + "terms": { + {% if license.total_checkouts is not none %} + "checkouts": {{ license.total_checkouts }}, + {% endif %} + {% if license.concurrent_checkouts is not none %} + "concurrency": {{ license.concurrent_checkouts }}, + {% endif %} + {% if license.expires is not none %} + "expires": "{{ license.expires.isoformat() }}", + {% endif %} + "length": 5097600 + }, + "protection": { + "format": [ + "application/vnd.adobe.adept+xml", + "application/vnd.readium.lcp.license.v1.0+json" + ], + "devices": 6, + "copy": false, + "print": false, + "tts": false + } + }, + "links": [ + { + "rel": "http://opds-spec.org/acquisition/borrow", + "href": "http://www.example.com/get{?id,checkout_id,expires,patron_id,passphrase,hint,hint_url,notification_url}", + "type": "application/vnd.readium.license.status.v1.0+json", + "templated": true + }, + { + "rel": "self", + "href": "http://www.example.com/status/294024", + "type": "application/vnd.odl.info+json" + } + ] + } + {% endfor %} + ] + } + ] +} diff --git a/tests/files/odl2/multiple_license.json b/tests/files/odl2/multiple_license.json deleted file mode 100644 index e76f62acbf..0000000000 --- a/tests/files/odl2/multiple_license.json +++ /dev/null @@ -1,235 +0,0 @@ -{ - "metadata": { - "title": "Test", - "itemsPerPage": 10, - "currentPage": 1, - "numberOfItems": 100 - }, - "links": [ - { - "type": "application/opds+json", - "rel": "self", - "href": "https://market.feedbooks.com/api/libraries/harvest.json" - } - ], - "publications": [ - { - "metadata": { - "@type": "http://schema.org/Book", - "title": "The Murmur of Bees", - "language": "en", - "modified": "2021-07-09T13:10:07Z", - "published": "2019-04-16T00:00:00Z", - "numberOfPages": 476, - "identifier": "urn:ISBN:9781542090490", - "schema:workExample": [ - { - "@type": "http://schema.org/Book", - "schema:bookFormat": "http://schema.org/Hardcover", - "schema:isbn": "urn:ISBN:9781542040501" - } - ], - "author": [ - { - "name": "Sofía Segovia", - "links": [ - { - "type": "application/opds+json", - "href": "https://example.com/store/browse/recent.json?author_id=519449&lang=en" - } - ] - } - ], - "translator": [ - { - "name": "Simon Bruni", - "links": [ - { - "type": "application/opds+json", - "href": "https://example.com/store/browse/recent.json?contributor_id=1447607&lang=en" - } - ] - } - ], - "publisher": { - "name": "Amazon Publishing", - "links": [ - { - "type": "application/opds+json", - "href": "https://example.com/store/browse/recent.json?lang=en&publisher=Amazon+Publishing" - } - ] - }, - "description": "

\nFrom a beguiling voice in Mexican fiction comes an astonishing novel—her first to be translated into English—about a mysterious child with the power to change a family’s history in a country on the verge of revolution.\n

\n

From the day that old Nana Reja found a baby abandoned under a bridge, the life of a small Mexican town forever changed. Disfigured and covered in a blanket of bees, little Simonopio is for some locals the stuff of superstition, a child kissed by the devil. But he is welcomed by landowners Francisco and Beatriz Morales, who adopt him and care for him as if he were their own. As he grows up, Simonopio becomes a cause for wonder to the Morales family, because when the uncannily gifted child closes his eyes, he can see what no one else can—visions of all that’s yet to come, both beautiful and dangerous. Followed by his protective swarm of bees and living to deliver his adoptive family from threats—both human and those of nature—Simonopio’s purpose in Linares will, in time, be divined.

\n

\nSet against the backdrop of the Mexican Revolution and the devastating influenza of 1918,\nThe Murmur of Bees\ncaptures both the fate of a country in flux and the destiny of one family that has put their love, faith, and future in the unbelievable.\n

", - "subject": [ - { - "code": "FBFIC000000", - "name": "Fiction", - "scheme": "http://www.feedbooks.com/categories", - "links": [ - { - "type": "application/opds+json", - "href": "https://example.com/category/FBFIC000000.json?lang=en" - } - ] - }, - { - "code": "FBFIC014000", - "name": "Historical", - "scheme": "http://www.feedbooks.com/categories", - "links": [ - { - "type": "application/opds+json", - "href": "https://example.com/category/FBFIC014000.json?lang=en" - } - ] - }, - { - "code": "FBFIC019000", - "name": "Literary", - "scheme": "http://www.feedbooks.com/categories", - "links": [ - { - "type": "application/opds+json", - "href": "https://example.com/category/FBFIC019000.json?lang=en" - } - ] - } - ] - }, - "images": [ - { - "href": "https://covers.example.com/item/4154969.jpg?size=large", - "type": "image/jpeg", - "width": 260, - "height": 420 - }, - { - "href": "https://covers.example.com/item/4154969.jpg", - "type": "image/jpeg", - "width": 100, - "height": 180 - } - ], - "licenses": [ - { - "metadata": { - "identifier": "urn:uuid:ed3c2fe4-d44d-427e-afde-cf246d21ecd9", - "format": [ - "application/epub+zip", - "text/html" - ], - "created": "2021-06-14T19:46:17+02:00", - "terms": { - "expires": "{{expires_1}}", - "checkouts": 40, - "concurrency": 10, - "length": 5097600 - }, - "protection": { - "format": [ - "application/vnd.adobe.adept+xml", - "application/vnd.readium.lcp.license.v1.0+json" - ], - "devices": 6, - "copy": true, - "print": false, - "tts": false - } - }, - "links": [ - { - "rel": "http://opds-spec.org/acquisition/borrow", - "href": "https://license.example.com/loan/status/{?id,checkout_id,expires,patron_id,notification_url,passphrase,hint,hint_url}", - "type": "application/vnd.readium.license.status.v1.0+json", - "templated": true - }, - { - "rel": "self", - "href": "https://license.example.com/copy/status/?uuid=ed3c2fe4-d44d-427e-afde-cf246d21ecd9", - "type": "application/vnd.odl.info+json" - } - ] - }, - { - "metadata": { - "identifier": "urn:uuid:c9079f86-3cdb-4144-b106-04bd70973586", - "format": [ - "application/epub+zip", - "text/html" - ], - "created": "2021-07-09T15:08:55+02:00", - "terms": { - "expires": "{{expires_2}}", - "checkouts": 40, - "concurrency": 10, - "length": 5097600 - }, - "protection": { - "format": [ - "application/vnd.adobe.adept+xml", - "application/vnd.readium.lcp.license.v1.0+json" - ], - "devices": 6, - "copy": true, - "print": false, - "tts": false - } - }, - "links": [ - { - "rel": "http://opds-spec.org/acquisition/borrow", - "href": "https://license.example.com/loan/status/{?id,checkout_id,expires,patron_id,notification_url,passphrase,hint,hint_url}", - "type": "application/vnd.readium.license.status.v1.0+json", - "templated": true - }, - { - "rel": "self", - "href": "https://license.example.com/copy/status/?uuid=c9079f86-3cdb-4144-b106-04bd70973586", - "type": "application/vnd.odl.info+json" - } - ] - }, - { - "metadata": { - "identifier": "urn:uuid:49f6bb8d-1912-4b95-8e62-a65d7c8c02fd", - "format": [ - "application/epub+zip", - "text/html" - ], - "created": "2021-07-09T15:08:55+02:00", - "terms": { - "expires": "{{expires_3}}", - "checkouts": 40, - "concurrency": 10, - "length": 5097600 - }, - "protection": { - "format": [ - "application/vnd.adobe.adept+xml", - "application/vnd.readium.lcp.license.v1.0+json" - ], - "devices": 6, - "copy": true, - "print": false, - "tts": false - } - }, - "links": [ - { - "rel": "http://opds-spec.org/acquisition/borrow", - "href": "https://license.example.com/loan/status/{?id,checkout_id,expires,patron_id,notification_url,passphrase,hint,hint_url}", - "type": "application/vnd.readium.license.status.v1.0+json", - "templated": true - }, - { - "rel": "self", - "href": "https://license.example.com/copy/status/?uuid=49f6bb8d-1912-4b95-8e62-a65d7c8c02fd", - "type": "application/vnd.odl.info+json" - } - ] - } - ] - } - ] -} diff --git a/tests/files/odl2/single_license.json b/tests/files/odl2/single_license.json deleted file mode 100644 index 695f12f51e..0000000000 --- a/tests/files/odl2/single_license.json +++ /dev/null @@ -1,110 +0,0 @@ -{ - "metadata": { - "title": "Test", - "itemsPerPage": 10, - "currentPage": 1, - "numberOfItems": 100 - }, - "links": [ - { - "type": "application/opds+json", - "rel": "self", - "href": "https://market.feedbooks.com/api/libraries/harvest.json" - } - ], - "publications": [ - { - "metadata": { - "@type": "http://schema.org/Book", - "title": "Moby-Dick", - "author": "Herman Melville", - "identifier": "urn:isbn:978-3-16-148410-0", - "language": "en", - "publisher": { - "name": "Test Publisher" - }, - "published": "2015-09-29T00:00:00Z", - "modified": "2015-09-29T17:00:00Z", - "subject": [ - { - "scheme": "http://schema.org/audience", - "code": "juvenile-fiction", - "name": "Juvenile Fiction", - "links": [] - } - ] - }, - "links": [ - { - "rel": "self", - "href": "http://example.org/publication.json", - "type": "application/opds-publication+json" - } - ], - "images": [ - { - "href": "http://example.org/cover.jpg", - "type": "image/jpeg", - "height": 1400, - "width": 800 - }, - { - "href": "http://example.org/cover-small.jpg", - "type": "image/jpeg", - "height": 700, - "width": 400 - }, - { - "href": "http://example.org/cover.svg", - "type": "image/svg+xml" - } - ], - "licenses": [ - { - "metadata": { - "identifier": "urn:uuid:f7847120-fc6f-11e3-8158-56847afe9799", - "format": [ - "application/epub+zip", - "text/html", - "application/audiobook+json; protection=http://www.feedbooks.com/audiobooks/access-restriction" - ], - "price": { - "currency": "USD", - "value": 7.99 - }, - "created": "2014-04-25T12:25:21+02:00", - "terms": { - "checkouts": 1, - "expires": "{{expires}}", - "concurrency": 1, - "length": 5097600 - }, - "protection": { - "format": [ - "application/vnd.adobe.adept+xml", - "application/vnd.readium.lcp.license.v1.0+json" - ], - "devices": 6, - "copy": false, - "print": false, - "tts": false - } - }, - "links": [ - { - "rel": "http://opds-spec.org/acquisition/borrow", - "href": "http://www.example.com/get{?id,checkout_id,expires,patron_id,passphrase,hint,hint_url,notification_url}", - "type": "application/vnd.readium.license.status.v1.0+json", - "templated": true - }, - { - "rel": "self", - "href": "http://www.example.com/status/294024", - "type": "application/vnd.odl.info+json" - } - ] - } - ] - } - ] -} diff --git a/tests/test_odl.py b/tests/test_odl.py index a388ee245e..7d74c35ba6 100644 --- a/tests/test_odl.py +++ b/tests/test_odl.py @@ -2,13 +2,15 @@ import json import os import urllib.parse -from typing import Callable, List, Tuple +import uuid +from typing import List, Tuple, Optional import dateutil import pytest -from dateutil.tz import tzoffset from freezegun import freeze_time -from mock import MagicMock +from jinja2 import Environment, FileSystemLoader, select_autoescape +from mock import MagicMock, PropertyMock, patch +from parameterized import parameterized from api.circulation_exceptions import * from api.odl import ( @@ -35,8 +37,7 @@ MediaTypes, Representation, RightsStatus, - Work -) + Work) from core.scripts import RunCollectionMonitorScript from core.testing import DatabaseTest from core.util import datetime_helpers @@ -1310,7 +1311,7 @@ def canonicalize_author_name(self, identifier, working_display_name): return working_display_name metadata_client = MockMetadataClient() - warrior_time_limited = dict(checkouts=dict(available=1)) + warrior_time_limited = dict(checkouts=dict(left=52, available=1)) canadianity_loan_limited = dict(checkouts=dict(left=40, available=10)) canadianity_perpetual = dict(checkouts=dict(available=1)) midnight_loan_limited_1 = dict(checkouts=dict(left=20, available=1)) @@ -1370,7 +1371,7 @@ def do_get(url, headers): assert Representation.EPUB_MEDIA_TYPE == lpdm.delivery_mechanism.content_type assert DeliveryMechanism.ADOBE_DRM == lpdm.delivery_mechanism.drm_scheme assert RightsStatus.IN_COPYRIGHT == lpdm.rights_status.uri - assert 1 == warrior_pool.licenses_owned + assert 52 == warrior_pool.licenses_owned # 52 remaining checkouts in the License Info Document assert 1 == warrior_pool.licenses_available [license] = warrior_pool.licenses assert "1" == license.identifier @@ -1387,7 +1388,7 @@ def do_get(url, headers): assert datetime.datetime( 2019, 3, 31, 3, 13, 35, tzinfo=dateutil.tz.tzoffset("", 3600*2) ) == license.expires - assert None == license.remaining_checkouts + assert 52 == license.remaining_checkouts # 52 remaining checkouts in the License Info Document assert 1 == license.concurrent_checkouts # This item is an open access audiobook. @@ -1421,7 +1422,7 @@ def do_get(url, headers): assert Representation.EPUB_MEDIA_TYPE == lpdm.delivery_mechanism.content_type assert DeliveryMechanism.ADOBE_DRM == lpdm.delivery_mechanism.drm_scheme assert RightsStatus.IN_COPYRIGHT == lpdm.rights_status.uri - assert 11 == canadianity_pool.licenses_owned + assert 40 == canadianity_pool.licenses_owned # 40 remaining checkouts in the License Info Document assert 11 == canadianity_pool.licenses_available [license1, license2] = sorted(canadianity_pool.licenses, key=lambda x: x.identifier) assert "2" == license1.identifier @@ -1453,7 +1454,7 @@ def do_get(url, headers): [lpdm.delivery_mechanism.drm_scheme for lpdm in lpdms]) assert ([RightsStatus.IN_COPYRIGHT, RightsStatus.IN_COPYRIGHT] == [lpdm.rights_status.uri for lpdm in lpdms]) - assert 2 == midnight_pool.licenses_owned + assert 72 == midnight_pool.licenses_owned # 20 + 52 remaining checkouts in corresponding License Info Documents assert 2 == midnight_pool.licenses_available [license1, license2] = sorted(midnight_pool.licenses, key=lambda x: x.identifier) assert "4" == license1.identifier @@ -1930,30 +1931,126 @@ def canonicalize_author_name(self, identifier, working_display_name): assert Representation.EPUB_MEDIA_TYPE == lpdm.delivery_mechanism.content_type assert DeliveryMechanism.ADOBE_DRM == lpdm.delivery_mechanism.drm_scheme assert RightsStatus.IN_COPYRIGHT == lpdm.rights_status.uri - [borrow_link] = [l for l in essex_pool.identifier.links if l.rel == Hyperlink.BORROW] - assert 'http://localhost:6500/AL/works/URI/http://www.feedbooks.com/item/1946289/borrow' == borrow_link.resource.url + [borrow_link] = [ + l for l in essex_pool.identifier.links if l.rel == Hyperlink.BORROW + ] + assert ( + "http://localhost:6500/AL/works/URI/http://www.feedbooks.com/item/1946289/borrow" + == borrow_link.resource.url + ) + + +class TestLicense: + """Represents an ODL license.""" + + def __init__( + self, + identifier: Optional[str] = None, + total_checkouts: Optional[int] = None, + concurrent_checkouts: Optional[int] = None, + expires: Optional[datetime.datetime] = None, + ) -> None: + """Initialize a new instance of TestLicense class. + + :param identifier: License's identifier + :param total_checkouts: Total number of checkouts before a license expires + :param concurrent_checkouts: Number of concurrent checkouts allowed + :param expires: Date & time when a license expires + """ + self._identifier: str = identifier if identifier else str(uuid.uuid1()) + self._total_checkouts: Optional[int] = total_checkouts + self._concurrent_checkouts: int = concurrent_checkouts + self._expires: Optional[datetime.datetime] = expires + + @property + def identifier(self) -> str: + """Return the license's identifier. + + :return: License's identifier + """ + return self._identifier + + @property + def total_checkouts(self) -> Optional[int]: + """Return the total number of checkouts before a license expires. + + :return: Total number of checkouts before a license expires + """ + return self._total_checkouts + + @property + def concurrent_checkouts(self) -> Optional[int]: + """Return the number of concurrent checkouts allowed. + + :return: Number of concurrent checkouts allowed + """ + return self._concurrent_checkouts + + @property + def expires(self) -> Optional[datetime.datetime]: + """Return the date & time when a license expires. + + :return: Date & time when a license expires + """ + return self._expires + + +class TestLicenseInfo: + """Represents information about the current state of a license stored in the License Info Document.""" + + def __init__( + self, remaining_checkouts: int, available_concurrent_checkouts: int + ) -> None: + """Initialize a new instance of TestLicenseInfo class. + + :param remaining_checkouts: Total number of checkouts left for a License + :param available_concurrent_checkouts: Number of concurrent checkouts currently available + """ + self._remaining_checkouts: int = remaining_checkouts + self._available_concurrent_checkouts: int = available_concurrent_checkouts + + @property + def remaining_checkouts(self) -> int: + """Return the total number of checkouts left for a License. + + :return: Total number of checkouts left for a License + """ + return self._remaining_checkouts + + @property + def available_concurrent_checkouts(self) -> int: + """Return the number of concurrent checkouts currently available. + + :return: Number of concurrent checkouts currently available + """ + return self._available_concurrent_checkouts + + def __str__(self) -> str: + """Return a JSON representation of a part of the License Info Document.""" + return json.dumps( + { + "checkouts": { + "left": self.remaining_checkouts, + "available": self.available_concurrent_checkouts, + } + } + ) class TestODLExpiredItemsReaper(DatabaseTest, BaseODLTest): ODL_PROTOCOL = ODLAPI.NAME - ODL_FEED_FILENAME = None - ODL_LICENSE_EXPIRATION_TIME_PLACEHOLDER = [] + ODL_TEMPLATE_DIR = "files/odl" + ODL_TEMPLATE_FILENAME = "feed_template.opds.jinja2" ODL_REAPER_CLASS = ODLExpiredItemsReaper - LICENSES_AVAILABLE = None - LICENSES_LEFT = None def _create_importer(self, collection, http_get): """Create a new ODL importer with the specified parameters. :param collection: Collection object - :type collection: core.model.collection.Collection - :param http_get: Use this method to make an HTTP GET request. This can be replaced with a stub method for testing purposes. - :type http_get: Callable :return: ODLImporter object - :rtype: ODLImporter """ importer = ODLImporter( self._db, @@ -1963,63 +2060,92 @@ def _create_importer(self, collection, http_get): return importer - def _get_test_feed(self, expires: List[datetime.datetime]) -> str: - """Get the feed and template the specific expiration dates. + def _get_test_feed(self, licenses: List[TestLicense]) -> str: + """Get the test ODL feed with specific licensing information. - :param expires: List of expiration dates for the ODL licenses in feed. + :param licenses: List of ODL licenses - :return: Test ODL feed. + :return: Test ODL feed """ - feed = self.get_data(self.ODL_FEED_FILENAME) - for idx, expire in enumerate(expires): - feed = feed.replace(self.ODL_LICENSE_EXPIRATION_TIME_PLACEHOLDER[idx], expire.isoformat()) + env = Environment( + loader=FileSystemLoader(self.ODL_TEMPLATE_DIR), autoescape=select_autoescape() + ) + template = env.get_template(self.ODL_TEMPLATE_FILENAME) + feed = template.render(licenses=licenses) return feed - def _import_test_feed(self, expires: List[datetime.datetime]) -> Tuple[List[Edition], List[LicensePool], List[Work]]: - """Import the test ODL feed, templated with specific expiration dates. + def _import_test_feed( + self, + licenses: List[TestLicense], + license_infos: Optional[List[Optional[TestLicenseInfo]]] = None, + ) -> Tuple[List[Edition], List[LicensePool], List[Work]]: + """Import the test ODL feed with specific licensing information. - :param expires: Expiration date of the ODL license + :param licenses: List of ODL licenses + :param license_infos: List of License Info Documents :return: 3-tuple containing imported editions, license pools and works """ - feed = self._get_test_feed(expires) + feed = self._get_test_feed(licenses) data_source = DataSource.lookup(self._db, "Feedbooks", autocreate=True) collection = MockODLAPI.mock_collection(self._db, protocol=self.ODL_PROTOCOL) collection.external_integration.set_setting( - Collection.DATA_SOURCE_NAME_SETTING, - data_source.name + Collection.DATA_SOURCE_NAME_SETTING, data_source.name + ) + license_status_response = MagicMock( + side_effect=[ + (200, {}, str(license_status) if license_status else "{}") for license_status in license_infos + ] + if license_infos + else [(200, {}, {})] ) - license_status = { - "checkouts": { - "available": self.LICENSES_AVAILABLE, - "left": self.LICENSES_LEFT - } - } - license_status_response = MagicMock(return_value=(200, {}, json.dumps(license_status))) importer = self._create_importer(collection, license_status_response) - imported_editions, imported_pools, imported_works, _ = ( - importer.import_from_feed(feed) - ) + ( + imported_editions, + imported_pools, + imported_works, + _, + ) = importer.import_from_feed(feed) return imported_editions, imported_pools, imported_works class TestODLExpiredItemsReaperSingleLicense(TestODLExpiredItemsReaper): - ODL_FEED_FILENAME = "single_license.opds" - ODL_LICENSE_EXPIRATION_TIME_PLACEHOLDER = ["{{expires}}"] - LICENSES_AVAILABLE = 1 - + @parameterized.expand([ + ( + "expiration_date_in_the_past", + # The license expires 2021-01-01T00:01:00+01:00 that equals to 2010-01-01T00:00:00+00:00, the current time. + # It means the license had already expired at the time of the import. + TestLicense(expires=dateutil.parser.isoparse("2021-01-01T00:01:00+01:00")) + ), + ( + "total_checkouts_is_zero", + TestLicense(total_checkouts=0) + ), + ( + "remaining_checkouts_is_zero", + TestLicense(total_checkouts=10, concurrent_checkouts=5), + TestLicenseInfo(remaining_checkouts=0, available_concurrent_checkouts=0) + ) + ]) @freeze_time("2021-01-01T00:00:00+00:00") - def test_odl_importer_skips_expired_licenses(self): + def test_odl_importer_skips_expired_licenses( + self, + _, + test_license: TestLicense, + test_license_info: Optional[TestLicenseInfo] = None + ) -> None: """Ensure ODLImporter skips expired licenses - and does not count them in the total number of available licenses.""" + and does not count them in the total number of available licenses.""" # 1.1. Import the test feed with an expired ODL license. # The license expires 2021-01-01T00:01:00+01:00 that equals to 2010-01-01T00:00:00+00:00, the current time. # It means the license had already expired at the time of the import. - license_expiration_date = dateutil.parser.isoparse("2021-01-01T00:01:00+01:00") - imported_editions, imported_pools, imported_works = self._import_test_feed([license_expiration_date]) + imported_editions, imported_pools, imported_works = self._import_test_feed( + [test_license], + [test_license_info] + ) # Commit to expire the SQLAlchemy cache. self._db.commit() @@ -2039,8 +2165,29 @@ def test_odl_reaper_removes_expired_licenses(self): # 1.1. Import the test feed with an ODL license that is still valid. # The license will be valid for one more day since this very moment. - license_expiration_date = datetime_helpers.utc_now() + datetime.timedelta(days=1) - imported_editions, imported_pools, imported_works = self._import_test_feed([license_expiration_date]) + # The feed declares that there 10 checkouts available in total + # but the License Info Document shows that there are only 9 available at the moment of import. + total_checkouts = 10 + available_concurrent_checkouts = 5 + remaining_checkouts = 9 + license_expiration_date = datetime_helpers.utc_now() + datetime.timedelta( + days=1 + ) + imported_editions, imported_pools, imported_works = self._import_test_feed( + [ + TestLicense( + expires=license_expiration_date, + total_checkouts=total_checkouts, + concurrent_checkouts=available_concurrent_checkouts, + ) + ], + [ + TestLicenseInfo( + remaining_checkouts=remaining_checkouts, + available_concurrent_checkouts=available_concurrent_checkouts, + ) + ], + ) # Commit to expire the SQLAlchemy cache. self._db.commit() @@ -2049,8 +2196,8 @@ def test_odl_reaper_removes_expired_licenses(self): assert len(imported_pools) == 1 [imported_pool] = imported_pools - assert imported_pool.licenses_owned == 1 - assert imported_pool.licenses_available == 1 + assert imported_pool.licenses_owned == remaining_checkouts + assert imported_pool.licenses_available == available_concurrent_checkouts assert len(imported_pool.licenses) == 1 [license] = imported_pool.licenses @@ -2060,95 +2207,95 @@ def test_odl_reaper_removes_expired_licenses(self): loan, _ = license.loan_to(patron) # 3.1. Run ODLExpiredItemsReaper. This time nothing should happen since the license is still valid. - script = RunCollectionMonitorScript(self.ODL_REAPER_CLASS, _db=self._db, cmd_args=["Test ODL Collection"]) + script = RunCollectionMonitorScript( + self.ODL_REAPER_CLASS, _db=self._db, cmd_args=["Test ODL Collection"] + ) script.run() # Commit to expire the SQLAlchemy cache. self._db.commit() # 3.2. Ensure that availability of the license pool didn't change. - assert imported_pool.licenses_owned == 1 - assert imported_pool.licenses_available == 1 + assert imported_pool.licenses_owned == remaining_checkouts + assert imported_pool.licenses_available == available_concurrent_checkouts # 4. Expire the license. - # Set the expiration date to yesterday. - license.expires = datetime_helpers.utc_now() - datetime.timedelta(days=1) - - # 5.1. Run ODLExpiredItemsReaper again. This time it should remove the expired license. - script.run() - - # Commit to expire the SQLAlchemy cache. - self._db.commit() - - # 5.2. Ensure that availability of the license pool was updated and now it doesn't have any available licenses. - assert imported_pool.licenses_owned == 0 - assert imported_pool.licenses_available == 0 - - # 6.1. Run ODLExpiredItemsReaper again to ensure that number of licenses won't become negative. - script.run() - - # Commit to expire the SQLAlchemy cache. - self._db.commit() - - # 6.2. Ensure that number of licenses is still 0. - assert imported_pool.licenses_owned == 0 - assert imported_pool.licenses_available == 0 - - @freeze_time("2021-01-01T00:00:00+00:00") - def test_odl_reaper_removes_expired_licenses_with_multiple_available(self): - """Ensure ODLExpiredItemsReaper removes expired licenses.""" - # 1.1. Import the test feed with an ODL license that is still valid. Set the number of licenses - # available to 5 to make sure that the licenses are not available once the license expires. - license_expiration_date = datetime_helpers.utc_now() + datetime.timedelta(days=1) - self.LICENSES_AVAILABLE = 5 - imported_editions, imported_pools, imported_works = self._import_test_feed([license_expiration_date]) - - # Commit to expire the SQLAlchemy cache. - self._db.commit() - - # 1.2. Ensure that there is a license pool with available license. - assert len(imported_pools) == 1 + with patch("core.model.License.is_expired", new_callable=PropertyMock) as is_expired: + is_expired.return_value = True - [imported_pool] = imported_pools - assert imported_pool.licenses_available == self.LICENSES_AVAILABLE + # 5.1. Run ODLExpiredItemsReaper again. This time it should remove the expired license. + script.run() - assert len(imported_pool.licenses) == 1 - [license] = imported_pool.licenses - assert license.expires == license_expiration_date + # Commit to expire the SQLAlchemy cache. + self._db.commit() - # 2.1. Expire the license. - # Set the expiration date to yesterday. - license.expires = datetime_helpers.utc_now() - datetime.timedelta(days=1) + # 5.2. Ensure that availability of the license pool was updated + # and now it doesn't have any available licenses. + assert imported_pool.licenses_owned == 0 + assert imported_pool.licenses_available == 0 - # 2.2. Run ODLExpiredItemsReaper. It should remove the expired license. - script = RunCollectionMonitorScript(self.ODL_REAPER_CLASS, _db=self._db, cmd_args=["Test ODL Collection"]) - script.run() + # 6.1. Run ODLExpiredItemsReaper again to ensure that number of licenses won't become negative. + script.run() - # Commit to expire the SQLAlchemy cache. - self._db.commit() + # Commit to expire the SQLAlchemy cache. + self._db.commit() - # 2.3. Ensure that availability of the license pool was updated and now it doesn't have any available licenses. - assert imported_pool.licenses_available == 0 + # 6.2. Ensure that number of licenses is still 0. + assert imported_pool.licenses_owned == 0 + assert imported_pool.licenses_available == 0 class TestODLExpiredItemsReaperMultipleLicense(TestODLExpiredItemsReaper): - ODL_FEED_FILENAME = "multiple_license.opds" - ODL_LICENSE_EXPIRATION_TIME_PLACEHOLDER = ["{{expires_1}}", "{{expires_2}}", "{{expires_3}}"] - LICENSES_AVAILABLE = 8 - LICENSES_LEFT = 18 - LICENSES_OWNED = 10 - @freeze_time("2021-01-01T00:00:00+00:00") def test_odl_importer_skips_expired_licenses(self): """Ensure ODLImporter skips expired licenses - and does not count them in the total number of available licenses.""" + and does not count them in the total number of available licenses.""" # 1.1. Import the test feed with one expired ODL license and two valid licenses. - license_expiration_dates = [ - datetime_helpers.utc_now() - datetime.timedelta(days=1), # Expired - datetime_helpers.utc_now() + datetime.timedelta(days=1), # Valid - datetime_helpers.utc_now() + datetime.timedelta(weeks=12) # Valid - ] - imported_editions, imported_pools, imported_works = self._import_test_feed(license_expiration_dates) + remaining_checkouts = 9 + available_concurrent_checkouts = 5 + imported_editions, imported_pools, imported_works = self._import_test_feed( + [ + TestLicense( # Expired + total_checkouts=10, # (expiry date in the past) + concurrent_checkouts=5, + expires=datetime_helpers.utc_now() - datetime.timedelta(days=1), + ), + TestLicense( # Expired + total_checkouts=0, # (total_checkouts is 0) + concurrent_checkouts=0, + expires=datetime_helpers.utc_now() + datetime.timedelta(days=1), + ), + TestLicense( # Expired + total_checkouts=10, # (remaining_checkout is 0) + concurrent_checkouts=5, + expires=datetime_helpers.utc_now() + datetime.timedelta(days=1), + ), + TestLicense( # Valid + total_checkouts=10, + concurrent_checkouts=5, + expires=datetime_helpers.utc_now() + datetime.timedelta(days=2), + ), + TestLicense( # Valid + total_checkouts=10, + concurrent_checkouts=5, + expires=datetime_helpers.utc_now() + datetime.timedelta(weeks=12), + ), + ], + [ + TestLicenseInfo( + remaining_checkouts=0, + available_concurrent_checkouts=0 + ), + TestLicenseInfo( + remaining_checkouts=remaining_checkouts, + available_concurrent_checkouts=available_concurrent_checkouts + ), + TestLicenseInfo( + remaining_checkouts=remaining_checkouts, + available_concurrent_checkouts=available_concurrent_checkouts + ) + ], + ) # Commit to expire the SQLAlchemy cache. self._db.commit() @@ -2161,19 +2308,49 @@ def test_odl_importer_skips_expired_licenses(self): assert len(imported_pool.licenses) == 2 # 1.4 Make sure that 20 licenses are marked as owned (10 from each valid license) - assert imported_pool.licenses_owned == self.LICENSES_OWNED * 2 - assert imported_pool.licenses_available == self.LICENSES_AVAILABLE * 2 + assert imported_pool.licenses_owned == remaining_checkouts * 2 + assert imported_pool.licenses_available == available_concurrent_checkouts * 2 @freeze_time("2021-01-01T00:00:00+00:00") def test_odl_reaper_removes_expired_licenses(self): """Ensure ODLExpiredItemsReaper removes expired licenses.""" # 1.1. Import the test feed with ODL licenses that are not expired. - license_expiration_dates = [ - datetime_helpers.utc_now() + datetime.timedelta(days=1), - datetime_helpers.utc_now() + datetime.timedelta(days=60), - datetime_helpers.utc_now() + datetime.timedelta(days=365) - ] - imported_editions, imported_pools, imported_works = self._import_test_feed(license_expiration_dates) + total_checkouts = 10 + remaining_checkouts = 9 + available_concurrent_checkouts = 5 + imported_editions, imported_pools, imported_works = self._import_test_feed( + [ + TestLicense( + total_checkouts=total_checkouts, + concurrent_checkouts=available_concurrent_checkouts, + expires=datetime_helpers.utc_now() + datetime.timedelta(days=1), + ), + TestLicense( + total_checkouts=total_checkouts, + concurrent_checkouts=available_concurrent_checkouts, + expires=datetime_helpers.utc_now() + datetime.timedelta(days=2), + ), + TestLicense( + total_checkouts=total_checkouts, + concurrent_checkouts=available_concurrent_checkouts, + expires=datetime_helpers.utc_now() + datetime.timedelta(weeks=12), + ), + ], + [ + TestLicenseInfo( + remaining_checkouts=total_checkouts, + available_concurrent_checkouts=available_concurrent_checkouts + ), + TestLicenseInfo( + remaining_checkouts=remaining_checkouts, + available_concurrent_checkouts=available_concurrent_checkouts + ), + TestLicenseInfo( + remaining_checkouts=remaining_checkouts, + available_concurrent_checkouts=available_concurrent_checkouts + ) + ], + ) # Commit to expire the SQLAlchemy cache. self._db.commit() @@ -2182,26 +2359,34 @@ def test_odl_reaper_removes_expired_licenses(self): assert len(imported_pools) == 1 [imported_pool] = imported_pools - assert imported_pool.licenses_owned == 3 * self.LICENSES_OWNED - assert imported_pool.licenses_available == 3 * self.LICENSES_AVAILABLE assert len(imported_pool.licenses) == 3 [license1, license2, license3] = imported_pool.licenses - assert license1.expires == license_expiration_dates[0] - assert license2.expires == license_expiration_dates[1] - assert license3.expires == license_expiration_dates[2] + assert license1.remaining_checkouts == total_checkouts + assert license1.concurrent_checkouts == available_concurrent_checkouts + + assert license2.remaining_checkouts == remaining_checkouts + assert license2.concurrent_checkouts == available_concurrent_checkouts + + assert license3.remaining_checkouts == remaining_checkouts + assert license3.concurrent_checkouts == available_concurrent_checkouts + + assert imported_pool.licenses_owned == total_checkouts + 2 * remaining_checkouts + assert imported_pool.licenses_available == 3 * available_concurrent_checkouts # 2.1. Run ODLExpiredItemsReaper. This time nothing should happen since the license is still valid. - script = RunCollectionMonitorScript(self.ODL_REAPER_CLASS, _db=self._db, cmd_args=["Test ODL Collection"]) + script = RunCollectionMonitorScript( + self.ODL_REAPER_CLASS, _db=self._db, cmd_args=["Test ODL Collection"] + ) script.run() # Commit to expire the SQLAlchemy cache. self._db.commit() # 2.2. Ensure that availability of the license pool didn't change. - assert imported_pool.licenses_owned == 3 * self.LICENSES_OWNED - assert imported_pool.licenses_available == 3 * self.LICENSES_AVAILABLE assert len(imported_pool.licenses) == 3 + assert imported_pool.licenses_owned == total_checkouts + 2 * remaining_checkouts + assert imported_pool.licenses_available == 3 * available_concurrent_checkouts # 3. Expire the license. license1.expires = datetime_helpers.utc_now() - datetime.timedelta(days=1) @@ -2213,8 +2398,9 @@ def test_odl_reaper_removes_expired_licenses(self): self._db.commit() # 3.2. Ensure that availability of the license pool was updated - assert imported_pool.licenses_owned == 2 * self.LICENSES_OWNED - assert imported_pool.licenses_available == 2 * self.LICENSES_AVAILABLE + assert len(imported_pool.licenses) == 3 + assert imported_pool.licenses_owned == 2 * remaining_checkouts + assert imported_pool.licenses_available == 2 * available_concurrent_checkouts # 4.1. Run ODLExpiredItemsReaper again to make sure that licenses are not expired twice. script.run() @@ -2223,5 +2409,6 @@ def test_odl_reaper_removes_expired_licenses(self): self._db.commit() # 4.2. Ensure that number of licenses remains the same as in step 3.2. - assert imported_pool.licenses_owned == 2 * self.LICENSES_OWNED - assert imported_pool.licenses_available == 2 * self.LICENSES_AVAILABLE + assert len(imported_pool.licenses) == 3 + assert imported_pool.licenses_owned == 2 * remaining_checkouts + assert imported_pool.licenses_available == 2 * available_concurrent_checkouts From 33f501275bd096224c9c0dadb05f70b667c0fa7d Mon Sep 17 00:00:00 2001 From: Viacheslav Bessonov Date: Thu, 16 Sep 2021 02:50:57 +0500 Subject: [PATCH 4/7] Fix ODL 2.x tests --- api/odl.py | 195 +++++++++++---------- api/odl2.py | 57 +++--- tests/files/odl2/feed_template.json.jinja2 | 2 +- tests/test_odl.py | 15 +- tests/test_odl2.py | 20 ++- 5 files changed, 145 insertions(+), 144 deletions(-) diff --git a/api/odl.py b/api/odl.py index c330cc24c7..fa45154262 100644 --- a/api/odl.py +++ b/api/odl.py @@ -3,6 +3,7 @@ import logging import uuid from io import StringIO +from typing import Callable, Optional import dateutil import feedparser @@ -15,12 +16,7 @@ from core import util from core.analytics import Analytics -from core.metadata_layer import ( - CirculationData, - FormatData, - LicenseData, - TimestampData, -) +from core.metadata_layer import CirculationData, FormatData, LicenseData, TimestampData from core.model import ( Collection, ConfigurationSetting, @@ -33,38 +29,20 @@ LicensePool, Loan, MediaTypes, + Representation, RightsStatus, Session, get_one, get_one_or_create, - Representation) -from core.monitor import ( - CollectionMonitor, - IdentifierSweepMonitor) -from core.opds_import import ( - OPDSXMLParser, - OPDSImporter, - OPDSImportMonitor, -) -from core.testing import ( - DatabaseTest, - MockRequestsResponse, -) -from core.util.datetime_helpers import ( - utc_now, -) -from core.util.http import ( - HTTP, - BadResponseException, - RemoteIntegrationException, ) +from core.monitor import CollectionMonitor, IdentifierSweepMonitor +from core.opds_import import OPDSImporter, OPDSImportMonitor, OPDSXMLParser +from core.testing import DatabaseTest, MockRequestsResponse +from core.util.datetime_helpers import utc_now +from core.util.http import HTTP, BadResponseException, RemoteIntegrationException from core.util.string_helpers import base64 -from .circulation import ( - BaseCirculationAPI, - LoanInfo, - FulfillmentInfo, - HoldInfo, -) + +from .circulation import BaseCirculationAPI, FulfillmentInfo, HoldInfo, LoanInfo from .circulation_exceptions import * from .shared_collection import BaseSharedCollectionAPI @@ -787,6 +765,82 @@ class ODLImporter(OPDSImporter): # about the license. LICENSE_INFO_DOCUMENT_MEDIA_TYPE = 'application/vnd.odl.info+json' + @classmethod + def parse_license( + cls, + identifier: str, + total_checkouts: Optional[int], + concurrent_checkouts: Optional[int], + expires: Optional[datetime.datetime], + checkout_link: Optional[str], + odl_status_link: Optional[str], + do_get: Callable + ) -> Optional[LicenseData]: + remaining_checkouts = None + available_concurrent_checkouts = None + + while True: + if total_checkouts is not None: + total_checkouts = int(total_checkouts) + + if total_checkouts <= 0: + logging.info( + f"License # {identifier} expired since " + f"the total number of checkouts is {total_checkouts}" + ) + break + + if expires: + if not isinstance(expires, datetime.datetime): + expires = dateutil.parser.parse(expires) + + expires = util.datetime_helpers.to_utc(expires) + now = util.datetime_helpers.utc_now() + + if expires <= now: + logging.info( + f"License # {identifier} expired at {expires} (now is {now})" + ) + break + + if odl_status_link: + status_code, _, response = do_get( + odl_status_link, headers={} + ) + + if status_code < 400: + status = json.loads(response) + checkouts = status.get("checkouts", {}) + remaining_checkouts = checkouts.get("left") + available_concurrent_checkouts = checkouts.get("available") + + if remaining_checkouts is None: + remaining_checkouts = total_checkouts + + if remaining_checkouts is not None: + remaining_checkouts = int(remaining_checkouts) + + if remaining_checkouts <= 0: + logging.info( + f"License # {identifier} expired since " + f"the remaining number of checkouts is {remaining_checkouts}" + ) + break + + if available_concurrent_checkouts is None: + available_concurrent_checkouts = concurrent_checkouts + + return LicenseData( + identifier=identifier, + checkout_url=checkout_link, + status_url=odl_status_link, + expires=expires, + remaining_checkouts=remaining_checkouts, + concurrent_checkouts=available_concurrent_checkouts, + ) + + return None + @classmethod def _detail_for_elementtree_entry(cls, parser, entry_tag, feed_url=None, do_get=None): do_get = do_get or Representation.cautious_http_get @@ -844,12 +898,6 @@ def _detail_for_elementtree_entry(cls, parser, entry_tag, feed_url=None, do_get= data['medium'] = medium - expires = None - total_checkouts = None - remaining_checkouts = None - concurrent_checkouts = None - available_concurrent_checkouts = None - checkout_link = None for link_tag in parser._xpath(odl_license_tag, 'odl:tlink') or []: rel = link_tag.attrib.get("rel") @@ -868,68 +916,33 @@ def _detail_for_elementtree_entry(cls, parser, entry_tag, feed_url=None, do_get= odl_status_link = attrib.get("href") break + expires = None + total_checkouts = None + concurrent_checkouts = None + terms = parser._xpath(odl_license_tag, "odl:terms") if terms: total_checkouts = subtag(terms[0], "odl:total_checkouts") concurrent_checkouts = subtag(terms[0], "odl:concurrent_checkouts") - - if total_checkouts is not None: - total_checkouts = int(total_checkouts) - - if total_checkouts <= 0: - logging.info( - f"License # {identifier} expired since " - f"the total number of checkouts is {total_checkouts}" - ) - continue - expires = subtag(terms[0], "odl:expires") - if expires: - expires = util.datetime_helpers.to_utc(dateutil.parser.parse(expires)) - now = util.datetime_helpers.utc_now() - - if expires <= now: - logging.info( - f"License # {identifier} expired at {expires} (now is {now})" - ) - continue - - # If we found one, retrieve it and get licensing information about this book. - if odl_status_link: - ignore, ignore, response = do_get(odl_status_link, headers={}) - status = json.loads(response) - checkouts = status.get("checkouts", {}) - remaining_checkouts = checkouts.get("left") - available_concurrent_checkouts = checkouts.get("available") - - if remaining_checkouts is None: - remaining_checkouts = total_checkouts - - if remaining_checkouts is not None: - remaining_checkouts = int(remaining_checkouts) - - if remaining_checkouts <= 0: - logging.info( - f"License # {identifier} expired since " - f"the remaining number of checkouts is {remaining_checkouts}" - ) - continue + license = cls.parse_license( + identifier, + total_checkouts, + concurrent_checkouts, + expires, + checkout_link, + odl_status_link, + do_get + ) - if available_concurrent_checkouts is None: - available_concurrent_checkouts = concurrent_checkouts + if not license: + continue - licenses_owned += int(remaining_checkouts or 0) - licenses_available += int(available_concurrent_checkouts or 0) + licenses_owned += int(license.remaining_checkouts or 0) + licenses_available += int(license.concurrent_checkouts or 0) - licenses.append(LicenseData( - identifier=identifier, - checkout_url=checkout_link, - status_url=odl_status_link, - expires=expires, - remaining_checkouts=remaining_checkouts, - concurrent_checkouts=available_concurrent_checkouts, - )) + licenses.append(license) if not data.get('circulation'): data['circulation'] = dict() diff --git a/api/odl2.py b/api/odl2.py index f22e5e3abf..dfbb279c47 100644 --- a/api/odl2.py +++ b/api/odl2.py @@ -1,14 +1,13 @@ import json import logging +from api.odl import ODLAPI, ODLExpiredItemsReaper, ODLImporter from contextlib2 import contextmanager from flask_babel import lazy_gettext as _ from webpub_manifest_parser.odl import ODLFeedParserFactory from webpub_manifest_parser.opds2.registry import OPDS2LinkRelationsRegistry -from api.odl import ODLAPI, ODLExpiredItemsReaper -from core import util -from core.metadata_layer import FormatData, LicenseData +from core.metadata_layer import FormatData from core.model import DeliveryMechanism, Edition, MediaTypes, RightsStatus from core.model.configuration import ( ConfigurationAttributeType, @@ -209,11 +208,6 @@ def _extract_publication_metadata(self, feed, publication, data_source_name): ) ) - expires = None - remaining_checkouts = None - available_checkouts = None - concurrent_checkouts = None - checkout_link = first_or_default( license.links.get_by_rel(OPDS2LinkRelationsRegistry.BORROW.key) ) @@ -226,41 +220,32 @@ def _extract_publication_metadata(self, feed, publication, data_source_name): if odl_status_link: odl_status_link = odl_status_link.href - if odl_status_link: - status_code, _, response = self.http_get( - odl_status_link, headers={} - ) - - if status_code < 400: - status = json.loads(response) - checkouts = status.get("checkouts", {}) - remaining_checkouts = checkouts.get("left") - available_checkouts = checkouts.get("available") + expires = None + total_checkouts = None + concurrent_checkouts = None if license.metadata.terms: - expires = license.metadata.terms.expires + total_checkouts = license.metadata.terms.checkouts concurrent_checkouts = license.metadata.terms.concurrency + expires = license.metadata.terms.expires - if expires: - expires = util.datetime_helpers.to_utc(expires) - now = util.datetime_helpers.utc_now() + license = ODLImporter.parse_license( + identifier, + total_checkouts, + concurrent_checkouts, + expires, + checkout_link, + odl_status_link, + self.http_get + ) - if expires <= now: - continue + if not license: + continue - licenses_owned += int(concurrent_checkouts or 0) - licenses_available += int(available_checkouts or 0) + licenses_owned += int(license.remaining_checkouts or 0) + licenses_available += int(license.concurrent_checkouts or 0) - licenses.append( - LicenseData( - identifier=identifier, - checkout_url=checkout_link, - status_url=odl_status_link, - expires=expires, - remaining_checkouts=remaining_checkouts, - concurrent_checkouts=concurrent_checkouts, - ) - ) + licenses.append(license) metadata.circulation.licenses_owned = licenses_owned metadata.circulation.licenses_available = licenses_available diff --git a/tests/files/odl2/feed_template.json.jinja2 b/tests/files/odl2/feed_template.json.jinja2 index 316feb9bb4..0c44af253e 100644 --- a/tests/files/odl2/feed_template.json.jinja2 +++ b/tests/files/odl2/feed_template.json.jinja2 @@ -110,7 +110,7 @@ "type": "application/vnd.odl.info+json" } ] - } + }{{ ", " if not loop.last else "" }} {% endfor %} ] } diff --git a/tests/test_odl.py b/tests/test_odl.py index 7d74c35ba6..0c5852e863 100644 --- a/tests/test_odl.py +++ b/tests/test_odl.py @@ -3,15 +3,10 @@ import os import urllib.parse import uuid -from typing import List, Tuple, Optional +from typing import List, Optional, Tuple import dateutil import pytest -from freezegun import freeze_time -from jinja2 import Environment, FileSystemLoader, select_autoescape -from mock import MagicMock, PropertyMock, patch -from parameterized import parameterized - from api.circulation_exceptions import * from api.odl import ( ODLAPI, @@ -23,6 +18,11 @@ SharedODLAPI, SharedODLImporter, ) +from freezegun import freeze_time +from jinja2 import Environment, FileSystemLoader, select_autoescape +from mock import MagicMock, PropertyMock, patch +from parameterized import parameterized + from core.model import ( Collection, ConfigurationSetting, @@ -37,7 +37,8 @@ MediaTypes, Representation, RightsStatus, - Work) + Work, +) from core.scripts import RunCollectionMonitorScript from core.testing import DatabaseTest from core.util import datetime_helpers diff --git a/tests/test_odl2.py b/tests/test_odl2.py index 28579f68a7..ddb02a7647 100644 --- a/tests/test_odl2.py +++ b/tests/test_odl2.py @@ -3,7 +3,10 @@ import os import requests_mock +from api.odl import ODLImporter +from api.odl2 import ODL2API, ODL2APIConfiguration, ODL2ExpiredItemsReaper, ODL2Importer from freezegun import freeze_time +from mock import MagicMock from webpub_manifest_parser.core.ast import PresentationMetadata from webpub_manifest_parser.odl import ODLFeedParserFactory from webpub_manifest_parser.odl.ast import ODLPublication @@ -11,8 +14,6 @@ ODL_PUBLICATION_MUST_CONTAIN_EITHER_LICENSES_OR_OA_ACQUISITION_LINK_ERROR, ) -from api.odl import ODLImporter -from api.odl2 import ODL2API, ODL2APIConfiguration, ODL2ExpiredItemsReaper, ODL2Importer from core.coverage import CoverageFailure from core.model import ( Contribution, @@ -29,6 +30,7 @@ from core.opds2_import import RWPMManifestParser from core.tests.test_opds2_import import OPDS2Test from tests.test_odl import ( + TestODLExpiredItemsReaper, TestODLExpiredItemsReaperMultipleLicense, TestODLExpiredItemsReaperSingleLicense, ) @@ -256,12 +258,11 @@ def test(self): assert str(huck_finn_semantic_error) == huck_finn_failure.exception -class TestODL2ExpiredItemsReaper: - __base_path = os.path.split(__file__)[0] - resource_path = os.path.join(__base_path, "files", "odl2") - - ODL_REAPER_CLASS = ODL2ExpiredItemsReaper +class TestODL2ExpiredItemsReaper(TestODLExpiredItemsReaper): ODL_PROTOCOL = ODL2API.NAME + ODL_TEMPLATE_DIR = "files/odl2" + ODL_TEMPLATE_FILENAME = "feed_template.json.jinja2" + ODL_REAPER_CLASS = ODL2ExpiredItemsReaper def _create_importer(self, collection, http_get): """Create a new ODL importer with the specified parameters. @@ -282,13 +283,14 @@ def _create_importer(self, collection, http_get): parser=RWPMManifestParser(ODLFeedParserFactory()), http_get=http_get, ) + importer._is_identifier_allowed = MagicMock(return_value=True) return importer class TestODL2ExpiredItemsReaperSingleLicensee(TestODL2ExpiredItemsReaper, TestODLExpiredItemsReaperSingleLicense): - ODL_FEED_FILENAME = "single_license.json" + pass class TestODL2ExpiredItemsReaperMultipleLicense(TestODL2ExpiredItemsReaper, TestODLExpiredItemsReaperMultipleLicense): - ODL_FEED_FILENAME = "multiple_license.json" + pass From aa4ad79d8e6f5b3e2f9381cba58f0e6fb234e877 Mon Sep 17 00:00:00 2001 From: Viacheslav Bessonov Date: Thu, 16 Sep 2021 19:38:52 +0500 Subject: [PATCH 5/7] Update ODL_TEMPLATE_DIR to an absolute path --- tests/test_odl.py | 2 +- tests/test_odl2.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/test_odl.py b/tests/test_odl.py index 0c5852e863..c76b1bba8e 100644 --- a/tests/test_odl.py +++ b/tests/test_odl.py @@ -2040,7 +2040,7 @@ def __str__(self) -> str: class TestODLExpiredItemsReaper(DatabaseTest, BaseODLTest): ODL_PROTOCOL = ODLAPI.NAME - ODL_TEMPLATE_DIR = "files/odl" + ODL_TEMPLATE_DIR = os.path.join(BaseODLTest.base_path, "files", "odl") ODL_TEMPLATE_FILENAME = "feed_template.opds.jinja2" ODL_REAPER_CLASS = ODLExpiredItemsReaper diff --git a/tests/test_odl2.py b/tests/test_odl2.py index ddb02a7647..f5af923120 100644 --- a/tests/test_odl2.py +++ b/tests/test_odl2.py @@ -260,7 +260,7 @@ def test(self): class TestODL2ExpiredItemsReaper(TestODLExpiredItemsReaper): ODL_PROTOCOL = ODL2API.NAME - ODL_TEMPLATE_DIR = "files/odl2" + ODL_TEMPLATE_DIR = os.path.join(TestODLExpiredItemsReaper.base_path, "files", "odl2") ODL_TEMPLATE_FILENAME = "feed_template.json.jinja2" ODL_REAPER_CLASS = ODL2ExpiredItemsReaper From b49b1693d0f5f4fb18c5757395407e0e3f6d00b9 Mon Sep 17 00:00:00 2001 From: Viacheslav Bessonov Date: Mon, 27 Sep 2021 21:11:44 +0500 Subject: [PATCH 6/7] Address comments --- api/odl.py | 33 ++++++++++++++++--- ...te.opds.jinja2 => feed_template.xml.jinja} | 2 +- ...e.json.jinja2 => feed_template.json.jinja} | 0 tests/test_odl.py | 27 ++++++++++----- tests/test_odl2.py | 10 +++--- 5 files changed, 54 insertions(+), 18 deletions(-) rename tests/files/odl/{feed_template.opds.jinja2 => feed_template.xml.jinja} (99%) rename tests/files/odl2/{feed_template.json.jinja2 => feed_template.json.jinja} (100%) diff --git a/api/odl.py b/api/odl.py index fa45154262..5e07b63708 100644 --- a/api/odl.py +++ b/api/odl.py @@ -776,9 +776,26 @@ def parse_license( odl_status_link: Optional[str], do_get: Callable ) -> Optional[LicenseData]: + """Check the license's attributes passed as parameters: + - if they're correct, turn them into a LicenseData object + - otherwise, return a None + + :param identifier: License's identifier + :param total_checkouts: Total number of checkouts before the license expires + :param concurrent_checkouts: Number of concurrent checkouts allowed for this license + :param expires: Date & time until the license is valid + :param checkout_link: License's checkout link + :param odl_status_link: License Info Document's link + :param do_get: Callback performing HTTP GET method + + :return: LicenseData if all the license's attributes are correct, None, otherwise + """ remaining_checkouts = None available_concurrent_checkouts = None + # This cycle ends in two different cases: + # - when at least one of the parameters is invalid; in this case, the method returns None. + # - when all the parameters are valid; in this case, the method returns a LicenseData object. while True: if total_checkouts is not None: total_checkouts = int(total_checkouts) @@ -813,6 +830,12 @@ def parse_license( checkouts = status.get("checkouts", {}) remaining_checkouts = checkouts.get("left") available_concurrent_checkouts = checkouts.get("available") + else: + logging.warning( + f"License # {identifier}'s Info Document is not available. " + f"Status link failed with {status_code} code" + ) + break if remaining_checkouts is None: remaining_checkouts = total_checkouts @@ -1601,10 +1624,12 @@ def process_item(self, identifier): remaining_checkouts = 0 # total number of checkouts across all the licenses in the pool concurrent_checkouts = 0 # number of concurrent checkouts allowed across all the licenses in the pool - for license in licensepool.licenses: - if not license.is_expired: - remaining_checkouts += license.remaining_checkouts - concurrent_checkouts += license.concurrent_checkouts + # 0 is a starting point, + # we're going through all the valid licenses in the pool and count up available checkouts. + for license_pool_license in licensepool.licenses: + if not license_pool_license.is_expired: + remaining_checkouts += license_pool_license.remaining_checkouts + concurrent_checkouts += license_pool_license.concurrent_checkouts if remaining_checkouts != licensepool.licenses_owned or \ concurrent_checkouts != licensepool.licenses_available: diff --git a/tests/files/odl/feed_template.opds.jinja2 b/tests/files/odl/feed_template.xml.jinja similarity index 99% rename from tests/files/odl/feed_template.opds.jinja2 rename to tests/files/odl/feed_template.xml.jinja index bcd040c4a5..0c1580d7cc 100644 --- a/tests/files/odl/feed_template.opds.jinja2 +++ b/tests/files/odl/feed_template.xml.jinja @@ -89,4 +89,4 @@ {% endfor %} - \ No newline at end of file + diff --git a/tests/files/odl2/feed_template.json.jinja2 b/tests/files/odl2/feed_template.json.jinja similarity index 100% rename from tests/files/odl2/feed_template.json.jinja2 rename to tests/files/odl2/feed_template.json.jinja diff --git a/tests/test_odl.py b/tests/test_odl.py index c76b1bba8e..3bc8352731 100644 --- a/tests/test_odl.py +++ b/tests/test_odl.py @@ -1960,7 +1960,7 @@ def __init__( """ self._identifier: str = identifier if identifier else str(uuid.uuid1()) self._total_checkouts: Optional[int] = total_checkouts - self._concurrent_checkouts: int = concurrent_checkouts + self._concurrent_checkouts: Optional[int] = concurrent_checkouts self._expires: Optional[datetime.datetime] = expires @property @@ -2039,9 +2039,11 @@ def __str__(self) -> str: class TestODLExpiredItemsReaper(DatabaseTest, BaseODLTest): + """Base class for all ODL reaper tests.""" + ODL_PROTOCOL = ODLAPI.NAME ODL_TEMPLATE_DIR = os.path.join(BaseODLTest.base_path, "files", "odl") - ODL_TEMPLATE_FILENAME = "feed_template.opds.jinja2" + ODL_TEMPLATE_FILENAME = "feed_template.xml.jinja" ODL_REAPER_CLASS = ODLExpiredItemsReaper def _create_importer(self, collection, http_get): @@ -2114,6 +2116,8 @@ def _import_test_feed( class TestODLExpiredItemsReaperSingleLicense(TestODLExpiredItemsReaper): + """Class testing that the ODL 1.x reaper correctly processes publications with a single license.""" + @parameterized.expand([ ( "expiration_date_in_the_past", @@ -2139,10 +2143,13 @@ def test_odl_importer_skips_expired_licenses( test_license_info: Optional[TestLicenseInfo] = None ) -> None: """Ensure ODLImporter skips expired licenses - and does not count them in the total number of available licenses.""" + and does not count them in the total number of available licenses. + + :param test_license: An example of an expired ODL license + :param test_license_info: An example of an ODL License Info Document belonging to an expired ODL license + (if required) + """ # 1.1. Import the test feed with an expired ODL license. - # The license expires 2021-01-01T00:01:00+01:00 that equals to 2010-01-01T00:00:00+00:00, the current time. - # It means the license had already expired at the time of the import. imported_editions, imported_pools, imported_works = self._import_test_feed( [test_license], [test_license_info] @@ -2247,11 +2254,13 @@ def test_odl_reaper_removes_expired_licenses(self): class TestODLExpiredItemsReaperMultipleLicense(TestODLExpiredItemsReaper): + """Class testing that the ODL 1.x reaper correctly processes publications with multiple licenses.""" + @freeze_time("2021-01-01T00:00:00+00:00") def test_odl_importer_skips_expired_licenses(self): """Ensure ODLImporter skips expired licenses and does not count them in the total number of available licenses.""" - # 1.1. Import the test feed with one expired ODL license and two valid licenses. + # 1.1. Import the test feed with three expired ODL licenses and two valid licenses. remaining_checkouts = 9 available_concurrent_checkouts = 5 imported_editions, imported_pools, imported_works = self._import_test_feed( @@ -2305,10 +2314,10 @@ def test_odl_importer_skips_expired_licenses(self): assert len(imported_pools) == 1 [imported_pool] = imported_pools - # 1.3. Ensure that the two valid licenses were imported + # 1.3. Ensure that the two valid licenses were imported. assert len(imported_pool.licenses) == 2 - # 1.4 Make sure that 20 licenses are marked as owned (10 from each valid license) + # 1.4 Make sure that the license statistics is correct and include only checkouts owned by two valid licenses. assert imported_pool.licenses_owned == remaining_checkouts * 2 assert imported_pool.licenses_available == available_concurrent_checkouts * 2 @@ -2398,7 +2407,7 @@ def test_odl_reaper_removes_expired_licenses(self): # Commit to expire the SQLAlchemy cache. self._db.commit() - # 3.2. Ensure that availability of the license pool was updated + # 3.2. Ensure that availability of the license pool was updated. assert len(imported_pool.licenses) == 3 assert imported_pool.licenses_owned == 2 * remaining_checkouts assert imported_pool.licenses_available == 2 * available_concurrent_checkouts diff --git a/tests/test_odl2.py b/tests/test_odl2.py index f5af923120..cadf2c0d88 100644 --- a/tests/test_odl2.py +++ b/tests/test_odl2.py @@ -259,9 +259,11 @@ def test(self): class TestODL2ExpiredItemsReaper(TestODLExpiredItemsReaper): + """Base class for all ODL 2.x reaper tests.""" + ODL_PROTOCOL = ODL2API.NAME ODL_TEMPLATE_DIR = os.path.join(TestODLExpiredItemsReaper.base_path, "files", "odl2") - ODL_TEMPLATE_FILENAME = "feed_template.json.jinja2" + ODL_TEMPLATE_FILENAME = "feed_template.json.jinja" ODL_REAPER_CLASS = ODL2ExpiredItemsReaper def _create_importer(self, collection, http_get): @@ -288,9 +290,9 @@ def _create_importer(self, collection, http_get): return importer -class TestODL2ExpiredItemsReaperSingleLicensee(TestODL2ExpiredItemsReaper, TestODLExpiredItemsReaperSingleLicense): - pass +class TestODL2ExpiredItemsReaperSingleLicense(TestODL2ExpiredItemsReaper, TestODLExpiredItemsReaperSingleLicense): + """Class testing that the ODL 2.x reaper correctly processes publications with a single license.""" class TestODL2ExpiredItemsReaperMultipleLicense(TestODL2ExpiredItemsReaper, TestODLExpiredItemsReaperMultipleLicense): - pass + """Class testing that the ODL 2.x reaper correctly processes publications with multiple licenses.""" From 590595a9667c2938e680a37310e05f4024af61a5 Mon Sep 17 00:00:00 2001 From: Viacheslav Bessonov Date: Fri, 1 Oct 2021 02:14:29 +0500 Subject: [PATCH 7/7] Consider 200 and 201 to be the only success status codes when downloading an LCP License Status Document --- api/odl.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/odl.py b/api/odl.py index 5e07b63708..7ab1c2ee5c 100644 --- a/api/odl.py +++ b/api/odl.py @@ -825,7 +825,7 @@ def parse_license( odl_status_link, headers={} ) - if status_code < 400: + if status_code in (200, 201): status = json.loads(response) checkouts = status.get("checkouts", {}) remaining_checkouts = checkouts.get("left")