From aa2cfbb1b27ab7cc0e23dadedc770ff9ae839381 Mon Sep 17 00:00:00 2001 From: RishiDiwanTT <90382027+RishiDiwanTT@users.noreply.github.com> Date: Wed, 25 Oct 2023 12:20:03 +0530 Subject: [PATCH] PP-554 Remove cachedfeeds (#1476) * Removed the CachedFeeds table and supporting columns from Work Removed all mentions and uses of the said attributes and tables * Propagate Lane.max_cache_age down to the feed responses Since the caching is now outsourced to CDNs we should propagate the Lane's max_cache_age down to the CDN itself, so it can be cached correctly per the lane preference * Drop cachedfeedds and cache columns migration * Removed the max_cache_age attribute from facets --- .../20231020_7fceb9488bc6_drop_cachedfeeds.py | 95 +++ api/config.py | 13 - api/controller.py | 58 +- api/lanes.py | 7 - bin/cache_opds_blocks | 11 - bin/cache_opds_lane_facets | 11 - bin/database_reaper | 2 +- core/config.py | 3 - core/facets.py | 3 - core/feed/annotator/verbose.py | 2 - core/feed/opds.py | 3 +- core/lane.py | 128 +-- core/model/__init__.py | 6 +- core/model/cachedfeed.py | 398 +--------- core/model/edition.py | 4 - core/model/library.py | 8 - core/model/work.py | 24 +- core/monitor.py | 12 - core/scripts.py | 18 - docker/services/cron/cron.d/circulation | 1 - scripts.py | 413 +--------- tests/api/test_config.py | 8 - tests/api/test_controller_cm.py | 56 +- tests/api/test_controller_opdsfeed.py | 4 +- tests/api/test_lanes.py | 26 +- tests/api/test_scripts.py | 437 +---------- tests/core/models/test_cachedfeed.py | 731 ------------------ tests/core/models/test_listeners.py | 17 +- tests/core/test_lane.py | 88 +-- tests/core/test_monitor.py | 32 - tests/core/test_scripts.py | 34 - 31 files changed, 140 insertions(+), 2513 deletions(-) create mode 100644 alembic/versions/20231020_7fceb9488bc6_drop_cachedfeeds.py delete mode 100755 bin/cache_opds_blocks delete mode 100755 bin/cache_opds_lane_facets delete mode 100644 tests/core/models/test_cachedfeed.py diff --git a/alembic/versions/20231020_7fceb9488bc6_drop_cachedfeeds.py b/alembic/versions/20231020_7fceb9488bc6_drop_cachedfeeds.py new file mode 100644 index 0000000000..d96f239354 --- /dev/null +++ b/alembic/versions/20231020_7fceb9488bc6_drop_cachedfeeds.py @@ -0,0 +1,95 @@ +"""Drop cachedfeeds + +Revision ID: 7fceb9488bc6 +Revises: 0739d5558dda +Create Date: 2023-10-20 10:55:49.709820+00:00 + +""" +import sqlalchemy as sa +from sqlalchemy.dialects import postgresql + +from alembic import op + +# revision identifiers, used by Alembic. +revision = "7fceb9488bc6" +down_revision = "0739d5558dda" +branch_labels = None +depends_on = None + + +def upgrade() -> None: + op.drop_index("ix_cachedfeeds_lane_id", table_name="cachedfeeds") + op.drop_index("ix_cachedfeeds_library_id", table_name="cachedfeeds") + op.drop_index( + "ix_cachedfeeds_library_id_lane_id_type_facets_pagination", + table_name="cachedfeeds", + ) + op.drop_index("ix_cachedfeeds_timestamp", table_name="cachedfeeds") + op.drop_index("ix_cachedfeeds_work_id", table_name="cachedfeeds") + op.drop_table("cachedfeeds") + op.drop_column("works", "simple_opds_entry") + op.drop_column("works", "verbose_opds_entry") + op.drop_column("editions", "simple_opds_entry") + + +def downgrade() -> None: + op.add_column( + "works", + sa.Column( + "verbose_opds_entry", sa.VARCHAR(), autoincrement=False, nullable=True + ), + ) + op.add_column( + "works", + sa.Column( + "simple_opds_entry", sa.VARCHAR(), autoincrement=False, nullable=True + ), + ) + op.add_column( + "editions", + sa.Column( + "simple_opds_entry", sa.VARCHAR(), autoincrement=False, nullable=True + ), + ) + op.create_table( + "cachedfeeds", + sa.Column("id", sa.INTEGER(), autoincrement=True, nullable=False), + sa.Column("lane_id", sa.INTEGER(), autoincrement=False, nullable=True), + sa.Column( + "timestamp", + postgresql.TIMESTAMP(timezone=True), + autoincrement=False, + nullable=True, + ), + sa.Column("type", sa.VARCHAR(), autoincrement=False, nullable=False), + sa.Column("unique_key", sa.VARCHAR(), autoincrement=False, nullable=True), + sa.Column("facets", sa.VARCHAR(), autoincrement=False, nullable=True), + sa.Column("pagination", sa.VARCHAR(), autoincrement=False, nullable=False), + sa.Column("content", sa.VARCHAR(), autoincrement=False, nullable=True), + sa.Column("library_id", sa.INTEGER(), autoincrement=False, nullable=True), + sa.Column("work_id", sa.INTEGER(), autoincrement=False, nullable=True), + sa.ForeignKeyConstraint( + ["lane_id"], ["lanes.id"], name="cachedfeeds_lane_id_fkey" + ), + sa.ForeignKeyConstraint( + ["library_id"], ["libraries.id"], name="cachedfeeds_library_id_fkey" + ), + sa.ForeignKeyConstraint( + ["work_id"], ["works.id"], name="cachedfeeds_work_id_fkey" + ), + sa.PrimaryKeyConstraint("id", name="cachedfeeds_pkey"), + ) + op.create_index("ix_cachedfeeds_work_id", "cachedfeeds", ["work_id"], unique=False) + op.create_index( + "ix_cachedfeeds_timestamp", "cachedfeeds", ["timestamp"], unique=False + ) + op.create_index( + "ix_cachedfeeds_library_id_lane_id_type_facets_pagination", + "cachedfeeds", + ["library_id", "lane_id", "type", "facets", "pagination"], + unique=False, + ) + op.create_index( + "ix_cachedfeeds_library_id", "cachedfeeds", ["library_id"], unique=False + ) + op.create_index("ix_cachedfeeds_lane_id", "cachedfeeds", ["lane_id"], unique=False) diff --git a/api/config.py b/api/config.py index bb8a0609da..636fd6cc48 100644 --- a/api/config.py +++ b/api/config.py @@ -16,8 +16,6 @@ class Configuration(CoreConfiguration): - DEFAULT_OPDS_FORMAT = "simple_opds_entry" - # The list of patron web urls allowed to access this CM PATRON_WEB_HOSTNAMES = "patron_web_hostnames" @@ -282,14 +280,3 @@ def cipher(cls, key: bytes) -> PKCS1OAEP_Cipher: encrypt() (public key) or decrypt() (private key). """ return PKCS1_OAEP.new(RSA.import_key(key)) - - -# We changed Configuration.DEFAULT_OPDS_FORMAT, but the Configuration -# class from core still has the old value. Change that one to match, -# so that core code that checks this constant will get the right -# value. -# -# TODO: We should come up with a better solution for this, probably -# involving a registry of Configuration objects that returns the -# appropriate one in any situation. This is a source of subtle bugs. -CoreConfiguration.DEFAULT_OPDS_FORMAT = Configuration.DEFAULT_OPDS_FORMAT diff --git a/api/controller.py b/api/controller.py index 5508547e53..0c4d78b0e8 100644 --- a/api/controller.py +++ b/api/controller.py @@ -68,21 +68,11 @@ ) from core.feed.navigation import NavigationFeed from core.feed.opds import NavigationFacets -from core.lane import ( - BaseFacets, - Facets, - FeaturedFacets, - Lane, - Pagination, - SearchFacets, - WorkList, -) +from core.lane import Facets, FeaturedFacets, Lane, Pagination, SearchFacets, WorkList from core.marc import MARCExporter from core.metadata_layer import ContributorData from core.model import ( - Admin, Annotation, - CachedFeed, CirculationEvent, Collection, ConfigurationSetting, @@ -249,30 +239,6 @@ def load_facets_from_request(self, *args, **kwargs): ): return NO_SUCH_LANE.detailed(_("Lane does not exist")) - if ( - isinstance(facets, BaseFacets) - and getattr(facets, "max_cache_age", None) is not None - ): - # A faceting object was loaded, and it tried to do something nonstandard - # with caching. - - # Try to get the AdminSignInController, which is - # associated with the CirculationManager object by the - # admin interface in admin/controller. - # - # If the admin interface wasn't initialized for whatever - # reason, we'll default to assuming the user is not an - # authenticated admin. - authenticated = False - controller = getattr(self, "admin_sign_in_controller", None) - if controller: - admin = controller.authenticated_admin_from_request() - # If authenticated_admin_from_request returns anything other than an admin (probably - # a ProblemDetail), the user is not an authenticated admin. - if isinstance(admin, Admin): - authenticated = True - if not authenticated: - facets.max_cache_age = None return facets def reload_settings_if_changed(self): @@ -948,7 +914,7 @@ def feed(self, lane_identifier, feed_class=OPDSAcquisitionFeed): search_engine=search_engine, ) return feed.as_response( - max_age=int(max_age) if max_age else None, + max_age=int(max_age) if max_age else lane.max_cache_age(), mime_types=flask.request.accept_mimetypes, ) @@ -984,7 +950,7 @@ def navigation(self, lane_identifier): worklist=lane, annotator=annotator, facets=facets, - ).as_response() + ).as_response(max_age=lane.max_cache_age()) def crawlable_library_feed(self): """Build or retrieve a crawlable acquisition feed for the @@ -1077,7 +1043,9 @@ def _crawlable_feed( facets=facets, pagination=pagination, search_engine=search_engine, - ).as_response(mime_types=flask.request.accept_mimetypes) + ).as_response( + mime_types=flask.request.accept_mimetypes, max_age=worklist.max_cache_age() + ) def _load_search_facets(self, lane): entrypoints = list(flask.request.library.entrypoints) @@ -1163,7 +1131,9 @@ def search(self, lane_identifier, feed_class=OPDSAcquisitionFeed): ) if isinstance(response, ProblemDetail): return response - return response.as_response(mime_types=flask.request.accept_mimetypes) + return response.as_response( + mime_types=flask.request.accept_mimetypes, max_age=lane.max_cache_age() + ) def _qa_feed( self, feed_factory, feed_title, controller_name, facet_class, worklist_factory @@ -1215,7 +1185,7 @@ def _qa_feed( annotator=annotator, search_engine=search_engine, facets=facets, - max_age=CachedFeed.IGNORE_CACHE, + max_age=0, ) def qa_feed(self, feed_class=OPDSAcquisitionFeed): @@ -2024,7 +1994,7 @@ def contributor( pagination=pagination, annotator=annotator, search_engine=search_engine, - ).as_response() + ).as_response(max_age=lane.max_cache_age()) def permalink(self, identifier_type, identifier): """Serve an entry for a single book. @@ -2121,7 +2091,7 @@ def related( pagination=None, facets=facets, search_engine=search_engine, - ).as_response() + ).as_response(max_age=lane.max_cache_age()) def recommendations( self, @@ -2180,7 +2150,7 @@ def recommendations( pagination=pagination, annotator=annotator, search_engine=search_engine, - ).as_response() + ).as_response(max_age=lane.max_cache_age()) def series(self, series_name, languages, audiences, feed_class=OPDSAcquisitionFeed): """Serve a feed of books in a given series.""" @@ -2219,7 +2189,7 @@ def series(self, series_name, languages, audiences, feed_class=OPDSAcquisitionFe pagination=pagination, annotator=annotator, search_engine=search_engine, - ).as_response() + ).as_response(max_age=lane.max_cache_age()) class ProfileController(CirculationManagerController): diff --git a/api/lanes.py b/api/lanes.py index 41a0e32c54..2827469049 100644 --- a/api/lanes.py +++ b/api/lanes.py @@ -14,7 +14,6 @@ WorkList, ) from core.model import ( - CachedFeed, Contributor, DataSource, Edition, @@ -1037,7 +1036,6 @@ class RecommendationLane(WorkBasedLane): # Cache for 24 hours -- would ideally be much longer but availability # information goes stale. MAX_CACHE_AGE = 24 * 60 * 60 - CACHED_FEED_TYPE = CachedFeed.RECOMMENDATIONS_TYPE def __init__( self, library, work, display_name=None, novelist_api=None, parent=None @@ -1124,7 +1122,6 @@ class SeriesLane(DynamicLane): # Cache for 24 hours -- would ideally be longer but availability # information goes stale. MAX_CACHE_AGE = 24 * 60 * 60 - CACHED_FEED_TYPE = CachedFeed.SERIES_TYPE def __init__(self, library, series_name, parent=None, **kwargs): if not series_name: @@ -1185,7 +1182,6 @@ class ContributorLane(DynamicLane): # Cache for 24 hours -- would ideally be longer but availability # information goes stale. MAX_CACHE_AGE = 24 * 60 * 60 - CACHED_FEED_TYPE = CachedFeed.CONTRIBUTOR_TYPE def __init__( self, library, contributor, parent=None, languages=None, audiences=None @@ -1250,7 +1246,6 @@ class RelatedBooksLane(WorkBasedLane): service. """ - CACHED_FEED_TYPE = CachedFeed.RELATED_TYPE DISPLAY_NAME = "Related Books" ROUTE = "related_books" @@ -1354,8 +1349,6 @@ def _recommendation_sublane(self, _db, novelist_api): class CrawlableFacets(Facets): """A special Facets class for crawlable feeds.""" - CACHED_FEED_TYPE = CachedFeed.CRAWLABLE_TYPE - # These facet settings are definitive of a crawlable feed. # Library configuration settings don't matter. SETTINGS = { diff --git a/bin/cache_opds_blocks b/bin/cache_opds_blocks deleted file mode 100755 index 6de4efda6a..0000000000 --- a/bin/cache_opds_blocks +++ /dev/null @@ -1,11 +0,0 @@ -#!/usr/bin/env python -"""Refresh the top-level OPDS groups.""" -import os -import sys - -bin_dir = os.path.split(__file__)[0] -package_dir = os.path.join(bin_dir, "..") -sys.path.append(os.path.abspath(package_dir)) -from scripts import CacheOPDSGroupFeedPerLane - -CacheOPDSGroupFeedPerLane().run() diff --git a/bin/cache_opds_lane_facets b/bin/cache_opds_lane_facets deleted file mode 100755 index 2f1819a5f1..0000000000 --- a/bin/cache_opds_lane_facets +++ /dev/null @@ -1,11 +0,0 @@ -#!/usr/bin/env python -"""Refresh the OPDS lane facets.""" -import os -import sys - -bin_dir = os.path.split(__file__)[0] -package_dir = os.path.join(bin_dir, "..") -sys.path.append(os.path.abspath(package_dir)) -from scripts import CacheFacetListsPerLane - -CacheFacetListsPerLane().run() diff --git a/bin/database_reaper b/bin/database_reaper index 678a1576fe..d3aba0d44d 100755 --- a/bin/database_reaper +++ b/bin/database_reaper @@ -1,5 +1,5 @@ #!/usr/bin/env python -"""Remove miscellaneous expired things (Credentials, CachedFeeds, Loans, etc.) +"""Remove miscellaneous expired things (Credentials, Loans, etc.) from the database. """ import os diff --git a/core/config.py b/core/config.py index 2ae72cfd6f..e728877b51 100644 --- a/core/config.py +++ b/core/config.py @@ -73,9 +73,6 @@ class Configuration(ConfigurationConstants): # Configuration key for push notifications status PUSH_NOTIFICATIONS_STATUS = "push_notifications_status" - # Lane policies - DEFAULT_OPDS_FORMAT = "verbose_opds_entry" - # Integrations URL = "url" INTEGRATIONS = "integrations" diff --git a/core/facets.py b/core/facets.py index 3dd7600879..692967fca2 100644 --- a/core/facets.py +++ b/core/facets.py @@ -8,9 +8,6 @@ class FacetConstants: ENTRY_POINT_REL = "http://librarysimplified.org/terms/rel/entrypoint" ENTRY_POINT_FACET_GROUP_NAME = "entrypoint" - # Query arguments can change how long a feed is to be cached. - MAX_CACHE_AGE_NAME = "max_age" - # Subset the collection, roughly, by quality. COLLECTION_FACET_GROUP_NAME = "collection" COLLECTION_FULL = "full" diff --git a/core/feed/annotator/verbose.py b/core/feed/annotator/verbose.py index 965f36aa01..eabcb870a2 100644 --- a/core/feed/annotator/verbose.py +++ b/core/feed/annotator/verbose.py @@ -22,8 +22,6 @@ class VerboseAnnotator(Annotator): in great detail. """ - opds_cache_field = Work.verbose_opds_entry.name - def annotate_work_entry( self, entry: WorkEntry, updated: Optional[datetime] = None ) -> None: diff --git a/core/feed/opds.py b/core/feed/opds.py index 3943bce58a..b33e1a5447 100644 --- a/core/feed/opds.py +++ b/core/feed/opds.py @@ -11,7 +11,6 @@ from core.feed.serializer.opds2 import OPDS2Serializer from core.feed.types import FeedData, WorkEntry from core.lane import FeaturedFacets -from core.model.cachedfeed import CachedFeed from core.util.flask_util import OPDSEntryResponse, OPDSFeedResponse from core.util.opds_writer import OPDSMessage @@ -106,4 +105,4 @@ class UnfulfillableWork(Exception): class NavigationFacets(FeaturedFacets): - CACHED_FEED_TYPE = CachedFeed.NAVIGATION_TYPE + pass diff --git a/core/lane.py b/core/lane.py index 740a5ef105..59c0f9c651 100644 --- a/core/lane.py +++ b/core/lane.py @@ -29,7 +29,6 @@ aliased, backref, contains_eager, - defer, joinedload, query, relationship, @@ -43,7 +42,6 @@ from core.facets import FacetConstants from core.model import ( Base, - CachedFeed, Collection, CustomList, CustomListEntry, @@ -84,36 +82,14 @@ class BaseFacets(FacetConstants): This is intended solely for use as a base class. """ - # If the use of a certain faceting object has implications for the - # type of feed (the way FeaturedFacets always implies a 'groups' feed), - # set the type of feed here. This will override any CACHED_FEED_TYPE - # associated with the WorkList. - CACHED_FEED_TYPE: Optional[str] = None - - # By default, faceting objects have no opinion on how long the feeds - # generated using them should be cached. - max_cache_age = None - def items(self): """Yields a 2-tuple for every active facet setting. These tuples are used to generate URLs that can identify - specific facet settings, and to distinguish between CachedFeed - objects that represent the same feed with different facet - settings. + specific facet settings. """ return [] - @property - def cached(self): - """This faceting object's opinion on whether feeds should be cached. - - :return: A boolean, or None for 'no opinion'. - """ - if self.max_cache_age is None: - return None - return self.max_cache_age != 0 - @property def query_string(self): """A query string fragment that propagates all active facet @@ -166,24 +142,18 @@ class FacetsWithEntryPoint(BaseFacets): selected EntryPoint. """ - def __init__( - self, entrypoint=None, entrypoint_is_default=False, max_cache_age=None, **kwargs - ): + def __init__(self, entrypoint=None, entrypoint_is_default=False, **kwargs): """Constructor. :param entrypoint: An EntryPoint (optional). :param entrypoint_is_default: If this is True, then `entrypoint` is a default value and was not determined by a user's explicit choice. - :param max_cache_age: Any feeds generated by this faceting object - will be cached for this amount of time. The default is to have - no opinion and let the Worklist manage this. :param kwargs: Other arguments may be supplied based on user input, but the default implementation is to ignore them. """ self.entrypoint = entrypoint self.entrypoint_is_default = entrypoint_is_default - self.max_cache_age = max_cache_age self.constructor_kwargs = kwargs @classmethod @@ -208,7 +178,6 @@ def navigate(self, entrypoint): return self.__class__( entrypoint=entrypoint, entrypoint_is_default=False, - max_cache_age=self.max_cache_age, **self.constructor_kwargs, ) @@ -284,15 +253,9 @@ def _from_request( return entrypoint entrypoint, is_default = entrypoint - max_cache_age = get_argument(Facets.MAX_CACHE_AGE_NAME, None) - max_cache_age = cls.load_max_cache_age(max_cache_age) - if isinstance(max_cache_age, ProblemDetail): - return max_cache_age - return cls( entrypoint=entrypoint, entrypoint_is_default=is_default, - max_cache_age=max_cache_age, **extra_kwargs, ) @@ -322,48 +285,13 @@ def load_entrypoint(cls, name, valid_entrypoints, default=None): return default, True return ep, False - @classmethod - def load_max_cache_age(cls, value): - """Convert a value for the MAX_CACHE_AGE_NAME parameter to a value - that CachedFeed will understand. - - :param value: A string. - :return: For now, either CachedFeed.IGNORE_CACHE or None. - """ - if value is None: - return value - - try: - value = int(value) - except ValueError as e: - value = None - - # At the moment, the only acceptable value that can be set - # through the web is zero -- i.e. don't use the cache at - # all. We can't give web clients fine-grained control over - # the internal workings of our cache; the most we can do - # is give them the opportunity to opt out. - # - # Thus, any nonzero value will be ignored. - if value == 0: - value = CachedFeed.IGNORE_CACHE - else: - value = None - return value - def items(self): """Yields a 2-tuple for every active facet setting. - In this class that just means the entrypoint and any max_cache_age. + In this class that just means the entrypoint. """ if self.entrypoint: yield (self.ENTRY_POINT_FACET_GROUP_NAME, self.entrypoint.INTERNAL_NAME) - if self.max_cache_age not in (None, CachedFeed.CACHE_FOREVER): - if self.max_cache_age == CachedFeed.IGNORE_CACHE: - value = 0 - else: - value = self.max_cache_age - yield (self.MAX_CACHE_AGE_NAME, str(value)) def modify_search_filter(self, filter): """Modify the given external_search.Filter object @@ -641,7 +569,6 @@ def navigate( enabled_facets=self.facets_enabled_at_init, entrypoint=(entrypoint or self.entrypoint), entrypoint_is_default=False, - max_cache_age=self.max_cache_age, ) def items(self): @@ -973,9 +900,6 @@ class FeaturedFacets(FacetsWithEntryPoint): AcquisitionFeed.groups(). """ - # This Facets class is used exclusively for grouped feeds. - CACHED_FEED_TYPE = CachedFeed.GROUPS_TYPE - def __init__( self, minimum_featured_quality, entrypoint=None, random_seed=None, **kwargs ): @@ -1012,9 +936,7 @@ def navigate(self, minimum_featured_quality=None, entrypoint=None): minimum_featured_quality or self.minimum_featured_quality ) entrypoint = entrypoint or self.entrypoint - return self.__class__( - minimum_featured_quality, entrypoint, max_cache_age=self.max_cache_age - ) + return self.__class__(minimum_featured_quality, entrypoint) def modify_search_filter(self, filter): super().modify_search_filter(filter) @@ -1413,10 +1335,8 @@ class WorkList: def visible(self) -> bool: return True - def max_cache_age(self, type): - """Determine how long a feed for this WorkList should be cached - internally. - """ + def max_cache_age(self): + """Determine how long a feed for this WorkList should be cached.""" return self.MAX_CACHE_AGE @classmethod @@ -2426,7 +2346,6 @@ def base_query(cls, _db): # Apply optimizations. qu = cls._modify_loading(qu) - qu = cls._defer_unused_fields(qu) return qu @classmethod @@ -2475,18 +2394,6 @@ def only_show_ready_deliverable_works(self, _db, query, show_suppressed=False): query, show_suppressed=show_suppressed, collection_ids=self.collection_ids ) - @classmethod - def _defer_unused_fields(cls, query): - """Some applications use the simple OPDS entry and some - applications use the verbose. Whichever one we don't need, - we can stop from even being sent over from the - database. - """ - if Configuration.DEFAULT_OPDS_FORMAT == "simple_opds_entry": - return query.options(defer(Work.verbose_opds_entry)) - else: - return query.options(defer(Work.simple_opds_entry)) - def bibliographic_filter_clauses(self, _db, qu): """Create a SQLAlchemy filter that excludes books whose bibliographic metadata doesn't match what we're looking for. @@ -2813,13 +2720,6 @@ class Lane(Base, DatabaseBackedWorkList, HierarchyWorkList): # admin interface can see all the lanes, visible or not. _visible = Column("visible", Boolean, default=True, nullable=False) - # A Lane may have many CachedFeeds. - cachedfeeds: Mapped[List[CachedFeed]] = relationship( - "CachedFeed", - backref="lane", - cascade="all, delete-orphan", - ) - # A Lane may have many CachedMARCFiles. cachedmarcfiles: Mapped[List[CachedMARCFile]] = relationship( "CachedMARCFile", @@ -3032,22 +2932,6 @@ def uses_customlists(self): return True return False - def max_cache_age(self, type): - """Determine how long a feed for this WorkList should be cached - internally. - - :param type: The type of feed. - """ - if type == CachedFeed.GROUPS_TYPE: - # Generating grouped feeds on the fly for Lanes is not incredibly - # expensive, but it's slow enough that we prefer to regenerate - # them in the background (using force_refresh=True) rather - # than while someone is waiting for an HTTP response. - return CachedFeed.CACHE_FOREVER - - # Other than that, we have no opinion -- use the default. - return super().max_cache_age(type) - def update_size(self, _db, search_engine=None): """Update the stored estimate of the number of Works in this Lane.""" library = self.get_library(_db) diff --git a/core/model/__init__.py b/core/model/__init__.py index 8627923774..c05aaf8db7 100644 --- a/core/model/__init__.py +++ b/core/model/__init__.py @@ -515,11 +515,7 @@ def _bulk_operation(self): SAMLFederation, ) from core.model.admin import Admin, AdminRole -from core.model.cachedfeed import ( - CachedFeed, - CachedMARCFile, - WillNotGenerateExpensiveFeed, -) +from core.model.cachedfeed import CachedMARCFile from core.model.circulationevent import CirculationEvent from core.model.classification import Classification, Genre, Subject from core.model.collection import ( diff --git a/core/model/cachedfeed.py b/core/model/cachedfeed.py index cc0ac2093f..a6603caa8f 100644 --- a/core/model/cachedfeed.py +++ b/core/model/cachedfeed.py @@ -1,406 +1,16 @@ -# CachedFeed, WillNotGenerateExpensiveFeed +# Cached Marc Files from __future__ import annotations -import datetime -import logging -from collections import namedtuple -from typing import TYPE_CHECKING, Optional +from typing import TYPE_CHECKING -from sqlalchemy import Column, DateTime, ForeignKey, Index, Integer, Unicode +from sqlalchemy import Column, DateTime, ForeignKey, Integer from sqlalchemy.orm import Mapped, relationship -from sqlalchemy.sql.expression import and_ -from core.model import Base, flush, get_one, get_one_or_create -from core.model.work import Work -from core.util.datetime_helpers import utc_now -from core.util.flask_util import OPDSFeedResponse +from core.model import Base if TYPE_CHECKING: from core.model import Representation -# This named tuple makes it easy to manage the return value of -# CachedFeed._prepare_keys. -CachedFeedKeys = namedtuple( - "CachedFeedKeys", - [ - "feed_type", - "library", - "work", - "lane_id", - "unique_key", - "facets_key", - "pagination_key", - ], -) - - -class CachedFeed(Base): - __tablename__ = "cachedfeeds" - id = Column(Integer, primary_key=True) - - # Every feed is associated with a lane. If null, this is a feed - # for a WorkList. If work_id is also null, it's a feed for the - # top-level. - lane_id = Column(Integer, ForeignKey("lanes.id"), nullable=True, index=True) - - # Every feed has a timestamp reflecting when it was created. - timestamp = Column(DateTime(timezone=True), nullable=True, index=True) - - # A feed is of a certain type--such as 'page' or 'groups'. - type = Column(Unicode, nullable=False) - - # A feed associated with a WorkList can have a unique key. - # This should be null if the feed is associated with a Lane. - unique_key = Column(Unicode, nullable=True) - - # A 'page' feed is associated with a set of values for the facet - # groups. - facets = Column(Unicode, nullable=True) - - # A 'page' feed is associated with a set of values for pagination. - pagination = Column(Unicode, nullable=False) - - # The content of the feed. - content = Column(Unicode, nullable=True) - - # Every feed is associated with a Library. - library_id = Column(Integer, ForeignKey("libraries.id"), index=True) - - # A feed may be associated with a Work. - work_id = Column(Integer, ForeignKey("works.id"), nullable=True, index=True) - work: Mapped[Optional[Work]] = relationship("Work", back_populates="cached_feeds") - - # Distinct types of feeds that might be cached. - GROUPS_TYPE = "groups" - PAGE_TYPE = "page" - NAVIGATION_TYPE = "navigation" - CRAWLABLE_TYPE = "crawlable" - RELATED_TYPE = "related" - RECOMMENDATIONS_TYPE = "recommendations" - SERIES_TYPE = "series" - CONTRIBUTOR_TYPE = "contributor" - - # Special constants for cache durations. - CACHE_FOREVER = object() - IGNORE_CACHE = object() - - log = logging.getLogger("CachedFeed") - - @classmethod - def fetch( - cls, - _db, - worklist, - facets, - pagination, - refresher_method, - max_age=None, - raw=False, - **response_kwargs, - ): - """Retrieve a cached feed from the database if possible. - - Generate it from scratch and store it in the database if - necessary. - - Return it in the most useful form to the caller. - - :param _db: A database connection. - :param worklist: The WorkList associated with this feed. - :param facets: A Facets object that distinguishes this feed from - others (for instance, by its sort order). - :param pagination: A Pagination object that explains which - page of a larger feed is being cached. - :param refresher_method: A function to call if it turns out - the contents of the feed need to be regenerated. This - function must take no arguments and return an object that - implements __unicode__. (A Unicode string or an OPDSFeed is fine.) - :param max_age: If a cached feed is older than this, it will - be considered stale and regenerated. This may be either a - number of seconds or a timedelta. If no value is - specified, a default value will be calculated based on - WorkList and Facets configuration. Setting this value to - zero will force a refresh. - :param raw: If this is False (the default), a Response ready to be - converted into a Flask Response object will be returned. If this - is True, the CachedFeed object itself will be returned. In most - non-test situations the default is better. - - :return: A Response or CachedFeed containing up-to-date content. - """ - - # Gather the information necessary to uniquely identify this - # page of this feed. - keys = cls._prepare_keys(_db, worklist, facets, pagination) - - # Calculate the maximum cache age, converting from timedelta - # to seconds if necessary. - max_age = cls.max_cache_age(worklist, keys.feed_type, facets, max_age) - - # These arguments will probably be passed into get_one, and - # will be passed into get_one_or_create in the event of a cache - # miss. - - # TODO: this constraint_clause might not be necessary anymore. - # ISTR it was an attempt to avoid race conditions, and we do a - # better job of that now. - constraint_clause = and_(cls.content != None, cls.timestamp != None) - kwargs = dict( - on_multiple="interchangeable", - constraint=constraint_clause, - type=keys.feed_type, - library=keys.library, - work=keys.work, - lane_id=keys.lane_id, - unique_key=keys.unique_key, - facets=keys.facets_key, - pagination=keys.pagination_key, - ) - feed_data = None - if max_age is cls.IGNORE_CACHE or isinstance(max_age, int) and max_age <= 0: - # Don't even bother checking for a CachedFeed: we're - # just going to replace it. - feed_obj = None - else: - feed_obj = get_one(_db, cls, **kwargs) - - should_refresh = cls._should_refresh(feed_obj, max_age) - if should_refresh: - # This is a cache miss. Either feed_obj is None or - # it's no good. We need to generate a new feed. - feed_data = str(refresher_method()) - generation_time = utc_now() - - if max_age is not cls.IGNORE_CACHE: - # Having gone through all the trouble of generating - # the feed, we want to cache it in the database. - - # Since it can take a while to generate a feed, and we know - # that the feed in the database is stale, it's possible that - # another thread _also_ noticed that feed was stale, and - # generated a similar feed while we were working. - # - # To avoid a database error, fetch the feed _again_ from the - # database rather than assuming we have the up-to-date - # object. - feed_obj, is_new = get_one_or_create(_db, cls, **kwargs) - if feed_obj.timestamp is None or feed_obj.timestamp < generation_time: - # Either there was no contention for this object, or there - # was contention but our feed is more up-to-date than - # the other thread(s). Our feed takes priority. - feed_obj.content = feed_data - feed_obj.timestamp = generation_time - elif feed_obj: - feed_data = feed_obj.content - - if raw and feed_obj: - return feed_obj - - # We have the information necessary to create a useful - # response-type object. - # - # Set some defaults in case the caller didn't pass them in. - if isinstance(max_age, int): - response_kwargs.setdefault("max_age", max_age) - - if max_age == cls.IGNORE_CACHE: - # If we were asked to ignore our internal cache, we should - # also tell the client not to store this document in _its_ - # internal cache. - response_kwargs["max_age"] = 0 - - if keys.library and keys.library.has_root_lanes: - # If this feed is associated with a Library that guides - # patrons to different lanes based on their patron type, - # all CachedFeeds need to be treated as private (but - # cacheable) on the client side. Otherwise, a change of - # client credentials might cause a cached representation - # to be reused when it should have been discarded. - # - # TODO: it might be possible to make this decision in a - # more fine-grained way, which would allow intermediaries - # to cache these feeds. - response_kwargs["private"] = True - - return OPDSFeedResponse(response=feed_data, **response_kwargs) - - @classmethod - def feed_type(cls, worklist, facets): - """Determine the 'type' of the feed. - - This may be defined either by `worklist` or by `facets`, with - `facets` taking priority. - - :return: A string that can go into cachedfeeds.type. - """ - type = CachedFeed.PAGE_TYPE - if worklist: - type = worklist.CACHED_FEED_TYPE or type - if facets: - type = facets.CACHED_FEED_TYPE or type - return type - - @classmethod - def max_cache_age(cls, worklist, type, facets, override=None): - """Determine the number of seconds that a cached feed - of a given type can remain fresh. - - Order of precedence: `override`, `facets`, `worklist`. - - :param worklist: A WorkList which may have an opinion on this - topic. - :param type: The type of feed being generated. - :param facets: A faceting object that may have an opinion on this - topic. - :param override: A specific value passed in by the caller. This - may either be a number of seconds or a timedelta. - - :return: A number of seconds, or CACHE_FOREVER or IGNORE_CACHE - """ - value = override - if value is None and facets is not None: - value = facets.max_cache_age - if value is None and worklist is not None: - value = worklist.max_cache_age(type) - - if value in (cls.CACHE_FOREVER, cls.IGNORE_CACHE): - # Special caching rules apply. - return value - - if value is None: - # Assume the feed should not be cached at all. - value = 0 - - if isinstance(value, datetime.timedelta): - value = value.total_seconds() - return value - - @classmethod - def _should_refresh(cls, feed_obj, max_age): - """Should we try to get a new representation of this CachedFeed? - - :param feed_obj: A CachedFeed. This may be None, which is why - this is a class method. - - :param max_age: Either a number of seconds, or one of the constants - CACHE_FOREVER or IGNORE_CACHE. - """ - should_refresh = False - if feed_obj is None: - # If we didn't find a CachedFeed (maybe because we didn't - # bother looking), we must always refresh. - should_refresh = True - elif max_age == cls.IGNORE_CACHE: - # If we are ignoring the cache, we must always refresh. - should_refresh = True - elif max_age == cls.CACHE_FOREVER: - # If we found *anything*, and the cache time is CACHE_FOREVER, - # we will never refresh. - should_refresh = False - elif ( - feed_obj.timestamp - and feed_obj.timestamp + datetime.timedelta(seconds=max_age) <= utc_now() - ): - # Here it comes down to a date comparison: how old is the - # CachedFeed? - should_refresh = True - return should_refresh - - @classmethod - def _prepare_keys(cls, _db, worklist, facets, pagination): - """Prepare various unique keys that will go into the database - and be used to distinguish CachedFeeds from one another. - - This is kept in a helper method for ease of testing. - - :param worklist: A WorkList. - :param facets: A Facets object. - :param pagination: A Pagination object. - - :return: A CachedFeedKeys object. - """ - if not worklist: - raise ValueError("Cannot prepare a CachedFeed without a WorkList.") - - feed_type = cls.feed_type(worklist, facets) - - # The Library is the one associated with `worklist`. - library = worklist.get_library(_db) - - # A feed may be associated with a specific Work, - # e.g. recommendations for readers of that Work. - work = getattr(worklist, "work", None) - - # Either lane_id or unique_key must be set, but not both. - from core.lane import Lane - - if isinstance(worklist, Lane): - lane_id = worklist.id - unique_key = None - else: - lane_id = None - unique_key = worklist.unique_key - - facets_key = "" - if facets is not None: - if isinstance(facets.query_string, bytes): - facets_key = facets.query_string.decode("utf-8") - else: - facets_key = facets.query_string - - pagination_key = "" - if pagination is not None: - if isinstance(pagination.query_string, bytes): - pagination_key = pagination.query_string.decode("utf-8") - else: - pagination_key = pagination.query_string - - return CachedFeedKeys( - feed_type=feed_type, - library=library, - work=work, - lane_id=lane_id, - unique_key=unique_key, - facets_key=facets_key, - pagination_key=pagination_key, - ) - - def update(self, _db, content): - self.content = content - self.timestamp = utc_now() - flush(_db) - - def __repr__(self): - if self.content: - length = len(self.content) - else: - length = "No content" - return "".format( - self.id, - self.lane_id, - self.type, - self.facets, - self.pagination, - self.timestamp, - length, - ) - - -Index( - "ix_cachedfeeds_library_id_lane_id_type_facets_pagination", - CachedFeed.library_id, - CachedFeed.lane_id, - CachedFeed.type, - CachedFeed.facets, - CachedFeed.pagination, -) - - -class WillNotGenerateExpensiveFeed(Exception): - """This exception is raised when a feed is not cached, but it's too - expensive to generate. - """ - class CachedMARCFile(Base): """A record that a MARC file has been created and cached for a particular lane.""" diff --git a/core/model/edition.py b/core/model/edition.py index 6d6307cc5c..59c395d46b 100644 --- a/core/model/edition.py +++ b/core/model/edition.py @@ -136,10 +136,6 @@ class Edition(Base, EditionConstants): cover_full_url = Column(Unicode) cover_thumbnail_url = Column(Unicode) - # An OPDS entry containing all metadata about this entry that - # would be relevant to display to a library patron. - simple_opds_entry = Column(Unicode, default=None) - # Information kept in here probably won't be used. extra: Mapped[Dict[str, str]] = Column(MutableDict.as_mutable(JSON), default={}) diff --git a/core/model/library.py b/core/model/library.py index e56c9eb9d1..9b9e563f2b 100644 --- a/core/model/library.py +++ b/core/model/library.py @@ -49,7 +49,6 @@ from core.lane import Lane from core.model import ( # noqa: autoflake AdminRole, - CachedFeed, CachedMARCFile, CirculationEvent, Collection, @@ -109,13 +108,6 @@ class Library(Base, HasSessionCache): "AdminRole", back_populates="library", cascade="all, delete-orphan" ) - # A Library may have many CachedFeeds. - cachedfeeds: Mapped[List[CachedFeed]] = relationship( - "CachedFeed", - backref="library", - cascade="all, delete-orphan", - ) - # A Library may have many CachedMARCFiles. cachedmarcfiles: Mapped[List[CachedMARCFile]] = relationship( "CachedMARCFile", diff --git a/core/model/work.py b/core/model/work.py index c226c90265..8ce42a3964 100644 --- a/core/model/work.py +++ b/core/model/work.py @@ -59,12 +59,7 @@ # Import related models when doing type checking if TYPE_CHECKING: - from core.model import ( # noqa: autoflake - CachedFeed, - CustomListEntry, - Library, - LicensePool, - ) + from core.model import CustomListEntry, Library, LicensePool class WorkGenre(Base): @@ -148,12 +143,6 @@ class Work(Base): "CustomListEntry", backref="work" ) - # One Work may have multiple CachedFeeds, and if a CachedFeed - # loses its Work, it ceases to exist. - cached_feeds: Mapped[List[CachedFeed]] = relationship( - "CachedFeed", back_populates="work", cascade="all, delete-orphan" - ) - # One Work may participate in many WorkGenre assignments. genres = association_proxy("work_genres", "genre", creator=WorkGenre.from_genre) work_genres: Mapped[List[WorkGenre]] = relationship( @@ -220,15 +209,6 @@ class Work(Base): # will be made to make the Work presentation ready. presentation_ready_exception = Column(Unicode, default=None, index=True) - # A precalculated OPDS entry containing all metadata about this - # work that would be relevant to display to a library patron. - simple_opds_entry = Column(Unicode, default=None) - - # A precalculated OPDS entry containing all metadata about this - # work that would be relevant to display in a machine-to-machine - # integration context. - verbose_opds_entry = Column(Unicode, default=None) - # A precalculated MARC record containing metadata about this # work that would be relevant to display in a library's public # catalog. @@ -237,8 +217,6 @@ class Work(Base): # These fields are potentially large and can be deferred if you # don't need all the data in a Work. LARGE_FIELDS = [ - "simple_opds_entry", - "verbose_opds_entry", "marc_record", "summary_text", ] diff --git a/core/monitor.py b/core/monitor.py index 349c85c9fe..4f664757c9 100644 --- a/core/monitor.py +++ b/core/monitor.py @@ -12,7 +12,6 @@ from core.metadata_layer import TimestampData from core.model import ( Base, - CachedFeed, CirculationEvent, Collection, CollectionMissing, @@ -889,17 +888,6 @@ def query(self): # ReaperMonitors that do something specific. -class CachedFeedReaper(ReaperMonitor): - """Removed cached feeds older than thirty days.""" - - MODEL_CLASS = CachedFeed - TIMESTAMP_FIELD = "timestamp" - MAX_AGE = 30 - - -ReaperMonitor.REGISTRY.append(CachedFeedReaper) - - class CredentialReaper(ReaperMonitor): """Remove Credentials that expired more than a day ago.""" diff --git a/core/scripts.py b/core/scripts.py index 0c0c56a44b..da886168b7 100644 --- a/core/scripts.py +++ b/core/scripts.py @@ -27,7 +27,6 @@ from core.metadata_layer import TimestampData from core.model import ( BaseCoverageRecord, - CachedFeed, Collection, ConfigurationSetting, Contributor, @@ -1774,8 +1773,6 @@ def _optimized_query(self): ) .options( defer(Work.summary_text), - defer(Work.simple_opds_entry), - defer(Work.verbose_opds_entry), ) ) @@ -2495,7 +2492,6 @@ def run(self, cmd_args=None): self.out("\n") else: self.out("There are no libraries in the system -- that's a problem.") - self.delete_cached_feeds() self.out("\n") collections = parsed.collections or self._db.query(Collection) for collection in collections: @@ -2519,20 +2515,6 @@ def check_library(self, library): else: self.out(" Associated with %s lanes.", len(library.lanes)) - def delete_cached_feeds(self): - page_feeds = self._db.query(CachedFeed).filter( - CachedFeed.type != CachedFeed.GROUPS_TYPE - ) - page_feeds_count = page_feeds.count() - self.out( - "%d feeds in cachedfeeds table, not counting grouped feeds.", - page_feeds_count, - ) - if page_feeds_count: - self.out(" Deleting them all.") - page_feeds.delete() - self._db.commit() - def explain_collection(self, collection): self.out('Examining collection "%s"', collection.name) diff --git a/docker/services/cron/cron.d/circulation b/docker/services/cron/cron.d/circulation index 0f3a9ddf86..cea2902683 100644 --- a/docker/services/cron/cron.d/circulation +++ b/docker/services/cron/cron.d/circulation @@ -11,7 +11,6 @@ HOME=/var/www/circulation # These scripts update internal caches. # -*/30 * * * * root core/bin/run cache_opds_blocks >> /var/log/cron.log 2>&1 */30 * * * * root core/bin/run -d 15 search_index_refresh >> /var/log/cron.log 2>&1 10 0 * * * root core/bin/run search_index_clear >> /var/log/cron.log 2>&1 0 0 * * * root core/bin/run update_custom_list_size >> /var/log/cron.log 2>&1 diff --git a/scripts.py b/scripts.py index 4636a370b1..1fa5e82576 100644 --- a/scripts.py +++ b/scripts.py @@ -18,7 +18,6 @@ from api.axis import Axis360BibliographicCoverageProvider from api.bibliotheca import BibliothecaCirculationSweep from api.config import CannotLoadConfiguration, Configuration -from api.controller import CirculationManager from api.lanes import create_default_lanes from api.local_analytics_exporter import LocalAnalyticsExporter from api.marc import LibraryAnnotator as MARCLibraryAnnotator @@ -30,10 +29,8 @@ OPDSForDistributorsReaperMonitor, ) from api.overdrive import OverdriveAPI -from core.entrypoint import EntryPoint from core.external_search import ExternalSearchIndex -from core.feed.acquisition import OPDSAcquisitionFeed -from core.lane import Facets, FeaturedFacets, Lane, Pagination +from core.lane import Lane from core.marc import MARCExporter from core.model import ( LOCK_ID_DB_INIT, @@ -46,7 +43,6 @@ ExternalIntegration, Hold, Identifier, - Library, LicensePool, Loan, Patron, @@ -66,7 +62,6 @@ from core.service.container import container_instance from core.util import LanguageCodes from core.util.datetime_helpers import utc_now -from core.util.opds_writer import OPDSFeed class Script(CoreScript): @@ -149,412 +144,6 @@ def q(self): ) -class CacheRepresentationPerLane(TimestampScript, LaneSweeperScript): - name = "Cache one representation per lane" - - @classmethod - def arg_parser(cls, _db): - parser = LaneSweeperScript.arg_parser(_db) - parser.add_argument( - "--language", - help="Process only lanes that include books in this language.", - action="append", - ) - parser.add_argument( - "--max-depth", - help="Stop processing lanes once you reach this depth.", - type=int, - default=None, - ) - parser.add_argument( - "--min-depth", - help="Start processing lanes once you reach this depth.", - type=int, - default=1, - ) - return parser - - def __init__(self, _db=None, cmd_args=None, manager=None, *args, **kwargs): - """Constructor. - :param _db: A database connection. - :param cmd_args: A mock set of command-line arguments, to use instead - of looking at the actual command line. - :param testing: If this method creates a CirculationManager object, - this value will be passed in to its constructor as its value for - `testing`. - :param manager: A mock CirculationManager object, to use instead - of creating a new one (creating a CirculationManager object is - very time-consuming). - :param *args: Positional arguments to pass to the superconstructor. - :param **kwargs: Keyword arguments to pass to the superconstructor. - """ - - super().__init__(_db, *args, **kwargs) - self.parse_args(cmd_args) - if not manager: - manager = CirculationManager(self._db, self.services) - from api.app import app - - app.manager = manager - self.app = app - self.base_url = ConfigurationSetting.sitewide( - self._db, Configuration.BASE_URL_KEY - ).value - - def parse_args(self, cmd_args=None): - parser = self.arg_parser(self._db) - parsed = parser.parse_args(cmd_args) - self.languages = [] - if parsed.language: - for language in parsed.language: - alpha = LanguageCodes.string_to_alpha_3(language) - if alpha: - self.languages.append(alpha) - else: - self.log.warning("Ignored unrecognized language code %s", alpha) - self.max_depth = parsed.max_depth - self.min_depth = parsed.min_depth - - # Return the parsed arguments in case a subclass needs to - # process more args. - return parsed - - def should_process_lane(self, lane): - if not isinstance(lane, Lane): - return False - - language_ok = False - if not self.languages: - # We are considering lanes for every single language. - language_ok = True - - if not lane.languages: - # The lane has no language restrictions. - language_ok = True - - for language in self.languages: - if language in lane.languages: - language_ok = True - break - if not language_ok: - return False - - if self.max_depth is not None and lane.depth > self.max_depth: - return False - if self.min_depth is not None and lane.depth < self.min_depth: - return False - - return True - - def cache_url(self, annotator, lane, languages): - raise NotImplementedError() - - def generate_representation(self, *args, **kwargs): - raise NotImplementedError() - - # The generated document will probably be an OPDS acquisition - # feed. - ACCEPT_HEADER = OPDSFeed.ACQUISITION_FEED_TYPE - - cache_url_method = None - - def process_library(self, library): - begin = time.time() - client = self.app.test_client() - ctx = self.app.test_request_context(base_url=self.base_url) - ctx.push() - super().process_library(library) - ctx.pop() - end = time.time() - self.log.info( - "Processed library %s in %.2fsec", library.short_name, end - begin - ) - - def process_lane(self, lane): - """Generate a number of feeds for this lane. - One feed will be generated for each combination of Facets and - Pagination objects returned by facets() and pagination(). - """ - cached_feeds = [] - for facets in self.facets(lane): - for pagination in self.pagination(lane): - extra_description = "" - if facets: - extra_description += " Facets: %s." % facets.query_string - if pagination: - extra_description += " Pagination: %s." % pagination.query_string - self.log.info( - "Generating feed for %s.%s", lane.full_identifier, extra_description - ) - a = time.time() - feed = self.do_generate(lane, facets, pagination) - b = time.time() - if feed: - cached_feeds.append(feed) - self.log.info( - "Took %.2f sec to make %d bytes.", (b - a), len(feed.data) - ) - total_size = sum(len(x.data) for x in cached_feeds) - return cached_feeds - - def facets(self, lane): - """Yield a Facets object for each set of facets this - script is expected to handle. - :param lane: The lane under consideration. (Different lanes may have - different available facets.) - :yield: A sequence of Facets objects. - """ - yield None - - def pagination(self, lane): - """Yield a Pagination object for each page of a feed this - script is expected to handle. - :param lane: The lane under consideration. (Different lanes may have - different pagination rules.) - :yield: A sequence of Pagination objects. - """ - yield None - - -class CacheFacetListsPerLane(CacheRepresentationPerLane): - """Cache the first two pages of every relevant facet list for this lane.""" - - name = "Cache paginated OPDS feed for each lane" - - @classmethod - def arg_parser(cls, _db): - parser = CacheRepresentationPerLane.arg_parser(_db) - available = Facets.DEFAULT_ENABLED_FACETS[Facets.ORDER_FACET_GROUP_NAME] - order_help = "Generate feeds for this ordering. Possible values: %s." % ( - ", ".join(available) - ) - parser.add_argument( - "--order", - help=order_help, - action="append", - default=[], - ) - - available = Facets.DEFAULT_ENABLED_FACETS[Facets.AVAILABILITY_FACET_GROUP_NAME] - availability_help = ( - "Generate feeds for this availability setting. Possible values: %s." - % (", ".join(available)) - ) - parser.add_argument( - "--availability", - help=availability_help, - action="append", - default=[], - ) - - available = Facets.DEFAULT_ENABLED_FACETS[Facets.COLLECTION_FACET_GROUP_NAME] - collection_help = ( - "Generate feeds for this collection within each lane. Possible values: %s." - % (", ".join(available)) - ) - parser.add_argument( - "--collection", - help=collection_help, - action="append", - default=[], - ) - - available = [x.INTERNAL_NAME for x in EntryPoint.ENTRY_POINTS] - entrypoint_help = ( - "Generate feeds for this entry point within each lane. Possible values: %s." - % (", ".join(available)) - ) - parser.add_argument( - "--entrypoint", - help=entrypoint_help, - action="append", - default=[], - ) - - default_pages = 2 - parser.add_argument( - "--pages", - help="Number of pages to cache for each facet. Default: %d" % default_pages, - type=int, - default=default_pages, - ) - return parser - - def parse_args(self, cmd_args=None): - parsed = super().parse_args(cmd_args) - self.orders = parsed.order - self.availabilities = parsed.availability - self.collections = parsed.collection - self.entrypoints = parsed.entrypoint - self.pages = parsed.pages - return parsed - - def facets(self, lane): - """This script covers a user-specified combination of facets, but it - defaults to using every combination of available facets for - the given lane with a certain sort order. - This means every combination of availability, collection, and - entry point. - That's a whole lot of feeds, which is why this script isn't - actually used -- by the time we generate all of then, they've - expired. - """ - library = lane.get_library(self._db) - default_order = library.default_facet(Facets.ORDER_FACET_GROUP_NAME) - allowed_orders = library.enabled_facets(Facets.ORDER_FACET_GROUP_NAME) - chosen_orders = self.orders or [default_order] - - allowed_entrypoint_names = [x.INTERNAL_NAME for x in library.entrypoints] - default_entrypoint_name = None - if allowed_entrypoint_names: - default_entrypoint_name = allowed_entrypoint_names[0] - - chosen_entrypoints = self.entrypoints or allowed_entrypoint_names - - default_availability = library.default_facet( - Facets.AVAILABILITY_FACET_GROUP_NAME - ) - allowed_availabilities = library.enabled_facets( - Facets.AVAILABILITY_FACET_GROUP_NAME - ) - chosen_availabilities = self.availabilities or [default_availability] - - default_collection = library.default_facet(Facets.COLLECTION_FACET_GROUP_NAME) - allowed_collections = library.enabled_facets(Facets.COLLECTION_FACET_GROUP_NAME) - chosen_collections = self.collections or [default_collection] - - top_level = lane.parent is None - for entrypoint_name in chosen_entrypoints: - entrypoint = EntryPoint.BY_INTERNAL_NAME.get(entrypoint_name) - if not entrypoint: - logging.warning("Ignoring unknown entry point %s" % entrypoint_name) - continue - if not entrypoint_name in allowed_entrypoint_names: - logging.warning("Ignoring disabled entry point %s" % entrypoint_name) - continue - for order in chosen_orders: - if order not in allowed_orders: - logging.warning("Ignoring unsupported ordering %s" % order) - continue - for availability in chosen_availabilities: - if availability not in allowed_availabilities: - logging.warning( - "Ignoring unsupported availability %s" % availability - ) - continue - for collection in chosen_collections: - if collection not in allowed_collections: - logging.warning( - "Ignoring unsupported collection %s" % collection - ) - continue - facets = Facets( - library=library, - collection=collection, - availability=availability, - distributor=None, # All distributors always - collection_name=None, # All collections - entrypoint=entrypoint, - entrypoint_is_default=( - top_level - and entrypoint.INTERNAL_NAME == default_entrypoint_name - ), - order=order, - order_ascending=True, - ) - yield facets - - def pagination(self, lane): - """This script covers a user-specified number of pages.""" - page = Pagination.default() - for pagenum in range(0, self.pages): - yield page - page = page.next_page - if not page: - # There aren't enough books to fill `self.pages` - # pages. Stop working. - break - - def do_generate(self, lane, facets, pagination, feed_class=None): - feeds = [] - title = lane.display_name - library = lane.get_library(self._db) - annotator = self.app.manager.annotator(lane, facets=facets) - url = annotator.feed_url(lane, facets=facets, pagination=pagination) - feed_class = feed_class or OPDSAcquisitionFeed - return feed_class.page( - _db=self._db, - title=title, - url=url, - worklist=lane, - annotator=annotator, - pagination=pagination, - facets=facets, - search_engine=None, - ).as_response(max_age=0) - - -class CacheOPDSGroupFeedPerLane(CacheRepresentationPerLane): - name = "Cache OPDS grouped feed for each lane" - - def should_process_lane(self, lane): - # OPDS grouped feeds are only generated for lanes that have sublanes. - if not lane.children: - return False - if self.max_depth is not None and lane.depth > self.max_depth: - return False - return True - - def do_generate(self, lane, facets, pagination, feed_class=None): - title = lane.display_name - annotator = self.app.manager.annotator(lane, facets=facets) - url = annotator.groups_url(lane, facets) - feed_class = feed_class or OPDSAcquisitionFeed - - # Since grouped feeds are only cached for lanes that have sublanes, - # there's no need to consider the case of a lane with no sublanes, - # unlike the corresponding code in OPDSFeedController.groups() - return feed_class.groups( - _db=self._db, - title=title, - url=url, - worklist=lane, - annotator=annotator, - pagination=None, - facets=facets, - search_engine=None, - ).as_response(max_age=0) - - def facets(self, lane): - """Generate a Facets object for each of the library's enabled - entrypoints. - This is the only way grouped feeds are ever generated, so there is - no way to override this. - """ - top_level = lane.parent is None - library: Library = lane.get_library(self._db) - - # If the WorkList has explicitly defined EntryPoints, we want to - # create a grouped feed for each EntryPoint. Otherwise, we want - # to create a single grouped feed with no particular EntryPoint. - # - # We use library.entrypoints instead of lane.entrypoints - # because WorkList.entrypoints controls which entry points you - # can *switch to* from a given WorkList. We're handling the - # case where you switched further up the hierarchy and now - # you're navigating downwards. - entrypoints = list(library.entrypoints) or [None] - default_entrypoint = entrypoints[0] - for entrypoint in entrypoints: - facets = FeaturedFacets( - minimum_featured_quality=library.settings.minimum_featured_quality, - uses_customlists=lane.uses_customlists, - entrypoint=entrypoint, - entrypoint_is_default=(top_level and entrypoint is default_entrypoint), - ) - yield facets - - class CacheMARCFiles(LaneSweeperScript): """Generate and cache MARC files for each input library.""" diff --git a/tests/api/test_config.py b/tests/api/test_config.py index ac50a16dc0..affccc2b1a 100644 --- a/tests/api/test_config.py +++ b/tests/api/test_config.py @@ -10,7 +10,6 @@ from api.config import Configuration from core.config import CannotLoadConfiguration -from core.config import Configuration as CoreConfiguration from core.configuration.library import LibrarySettings from tests.fixtures.database import DatabaseTransactionFixture from tests.fixtures.files import FilesFixture @@ -136,13 +135,6 @@ def test_max_outstanding_fines( assert max_fines is not None assert 100 == max_fines.amount - def test_default_opds_format(self): - # Initializing the Configuration object modifies the corresponding - # object in core, so that core code will behave appropriately. - assert ( - Configuration.DEFAULT_OPDS_FORMAT == CoreConfiguration.DEFAULT_OPDS_FORMAT - ) - @patch.object(os, "environ", new=dict()) def test_fcm_credentials(self, notifications_files_fixture): invalid_json = "{ this is invalid JSON }" diff --git a/tests/api/test_controller_cm.py b/tests/api/test_controller_cm.py index 06ff4d81aa..ba91176cb9 100644 --- a/tests/api/test_controller_cm.py +++ b/tests/api/test_controller_cm.py @@ -10,7 +10,7 @@ LibraryAnnotator, ) from core.lane import Facets, WorkList -from core.model import Admin, CachedFeed, ConfigurationSetting, create +from core.model import ConfigurationSetting, create from core.model.discovery_service_registration import DiscoveryServiceRegistration from core.problem_details import * from core.util.problem_detail import ProblemDetail @@ -251,60 +251,6 @@ def __init__(self, *args, **kwargs): assert isinstance(annotator, CirculationManagerAnnotator) assert worklist == annotator.lane - def test_load_facets_from_request_disable_caching( - self, circulation_fixture: CirculationControllerFixture - ): - # Only an authenticated admin can ask to disable caching, - # and load_facets_from_request is where we enforce this. - class MockAdminSignInController: - # Pretend to be able to find (or not) an Admin authenticated - # to make the current request. - admin = None - - def authenticated_admin_from_request(self): - return self.admin - - admin = Admin() - controller = MockAdminSignInController() - - circulation_fixture.manager.admin_sign_in_controller = controller # type: ignore[assignment] - - with circulation_fixture.request_context_with_library("/"): - # If you don't specify a max cache age, nothing happens, - # whether or not you're an admin. - for value in INVALID_CREDENTIALS, admin: - controller.admin = value # type: ignore - facets = circulation_fixture.manager.load_facets_from_request() - assert None == facets.max_cache_age - - with circulation_fixture.request_context_with_library("/?max_age=0"): - # Not an admin, max cache age requested. - controller.admin = INVALID_CREDENTIALS # type: ignore - facets = circulation_fixture.manager.load_facets_from_request() - assert None == facets.max_cache_age - - # Admin, max age requested. This is the only case where - # nonstandard caching rules make it through - # load_facets_from_request(). - controller.admin = admin # type: ignore - facets = circulation_fixture.manager.load_facets_from_request() - assert CachedFeed.IGNORE_CACHE == facets.max_cache_age - - # Since the admin sign-in controller is part of the admin - # package and not the API proper, test a situation where, for - # whatever reason, that controller was never initialized. - del circulation_fixture.manager.admin_sign_in_controller - - # Now what controller.admin says doesn't matter, because the - # controller's not associated with the CirculationManager. - # But everything still basically works; you just can't - # disable the cache. - with circulation_fixture.request_context_with_library("/?max_age=0"): - for value in (INVALID_CREDENTIALS, admin): - controller.admin = value # type: ignore - facets = circulation_fixture.manager.load_facets_from_request() - assert None == facets.max_cache_age - def test_load_facets_from_request_denies_access_to_inaccessible_worklist( self, circulation_fixture: CirculationControllerFixture ): diff --git a/tests/api/test_controller_opdsfeed.py b/tests/api/test_controller_opdsfeed.py index 345eadcb82..31a50df4e2 100644 --- a/tests/api/test_controller_opdsfeed.py +++ b/tests/api/test_controller_opdsfeed.py @@ -17,7 +17,7 @@ from core.feed.navigation import NavigationFeed from core.feed.opds import NavigationFacets from core.lane import Facets, FeaturedFacets, Pagination, SearchFacets, WorkList -from core.model import CachedFeed, Edition +from core.model import Edition from core.util.flask_util import Response from tests.fixtures.api_controller import CirculationControllerFixture, WorkSpec from tests.fixtures.library import LibraryFixture @@ -748,7 +748,7 @@ def test__qa_feed(self, circulation_fixture: CirculationControllerFixture): assert expect_url == kwargs.pop("url") # type: ignore # These feeds are never to be cached. - assert CachedFeed.IGNORE_CACHE == kwargs.pop("max_age") # type: ignore + assert 0 == kwargs.pop("max_age") # type: ignore # To improve performance, a Pagination object was created that # limits each lane in the test feed to a single Work. diff --git a/tests/api/test_lanes.py b/tests/api/test_lanes.py index 7238f5821e..b4f762f886 100644 --- a/tests/api/test_lanes.py +++ b/tests/api/test_lanes.py @@ -31,14 +31,7 @@ from core.external_search import Filter from core.lane import DefaultSortOrderFacets, Facets, FeaturedFacets, Lane, WorkList from core.metadata_layer import ContributorData, Metadata -from core.model import ( - CachedFeed, - Contributor, - DataSource, - Edition, - ExternalIntegration, - create, -) +from core.model import Contributor, DataSource, Edition, ExternalIntegration, create from tests.fixtures.database import DatabaseTransactionFixture from tests.fixtures.library import LibraryFixture from tests.fixtures.search import ExternalSearchFixtureFake @@ -516,10 +509,6 @@ def related_books_fixture(db: DatabaseTransactionFixture) -> RelatedBooksFixture class TestRelatedBooksLane: - def test_feed_type(self, related_books_fixture: RelatedBooksFixture): - # All feeds from these lanes are cached as 'related works' feeds. - assert CachedFeed.RELATED_TYPE == RelatedBooksLane.CACHED_FEED_TYPE - def test_initialization(self, related_books_fixture: RelatedBooksFixture): # Asserts that a RelatedBooksLane won't be initialized for a work # without related books @@ -768,10 +757,6 @@ def test_default_sort_order(self, db: DatabaseTransactionFixture): class TestSeriesLane: - def test_feed_type(self): - # All feeds from these lanes are cached as series feeds. - assert CachedFeed.SERIES_TYPE == SeriesLane.CACHED_FEED_TYPE - def test_initialization(self, lane_fixture: LaneFixture): # An error is raised if SeriesLane is created with an empty string. pytest.raises(ValueError, SeriesLane, lane_fixture.db.default_library(), "") @@ -841,10 +826,6 @@ def test_default_sort_order(self, db: DatabaseTransactionFixture): class TestContributorLane: - def test_feed_type(self): - # All feeds of this type are cached as contributor feeds. - assert CachedFeed.CONTRIBUTOR_TYPE == ContributorLane.CACHED_FEED_TYPE - def test_initialization(self, lane_fixture: LaneFixture): with pytest.raises(ValueError) as excinfo: ContributorLane(lane_fixture.db.default_library(), None) @@ -925,11 +906,6 @@ def test_overview_facets(self, lane_fixture: LaneFixture): class TestCrawlableFacets: - def test_feed_type(self, db: DatabaseTransactionFixture): - # All crawlable feeds are cached as such, no matter what - # WorkList they come from. - assert CachedFeed.CRAWLABLE_TYPE == CrawlableFacets.CACHED_FEED_TYPE - def test_default(self, db: DatabaseTransactionFixture): facets = CrawlableFacets.default(db.default_library()) assert CrawlableFacets.COLLECTION_FULL == facets.collection diff --git a/tests/api/test_scripts.py b/tests/api/test_scripts.py index 225cade1ac..f66b0ecc10 100644 --- a/tests/api/test_scripts.py +++ b/tests/api/test_scripts.py @@ -15,9 +15,8 @@ from api.config import Configuration from api.marc import LibraryAnnotator as MARCLibraryAnnotator from api.novelist import NoveListAPI -from core.entrypoint import AudiobooksEntryPoint, EbooksEntryPoint -from core.external_search import ExternalSearchIndex, mock_search_index -from core.lane import Facets, FeaturedFacets, Pagination, WorkList +from core.external_search import ExternalSearchIndex +from core.lane import WorkList from core.marc import MARCExporter from core.model import ( LOCK_ID_DB_INIT, @@ -30,29 +29,21 @@ create, ) from core.util.datetime_helpers import datetime_utc, utc_now -from core.util.flask_util import OPDSFeedResponse, Response -from core.util.opds_writer import OPDSFeed from scripts import ( AdobeAccountIDResetScript, - CacheFacetListsPerLane, CacheMARCFiles, - CacheOPDSGroupFeedPerLane, - CacheRepresentationPerLane, GenerateShortTokenScript, InstanceInitializationScript, LanguageListScript, LocalAnalyticsExportScript, NovelistSnapshotScript, ) -from tests.api.mockapi.circulation import MockCirculationManager from tests.fixtures.library import LibraryFixture -from tests.fixtures.search import EndToEndSearchFixture, ExternalSearchFixtureFake -from tests.mocks.search import fake_hits +from tests.fixtures.search import EndToEndSearchFixture if TYPE_CHECKING: from tests.fixtures.authenticator import SimpleAuthIntegrationFixture from tests.fixtures.database import DatabaseTransactionFixture - from tests.fixtures.search import ExternalSearchFixture class TestAdobeAccountIDResetScript: @@ -120,428 +111,6 @@ def lane_script_fixture( return LaneScriptFixture(db, library_fixture) -class TestCacheRepresentationPerLane: - def test_should_process_lane(self, lane_script_fixture: LaneScriptFixture): - db = lane_script_fixture.db - - # Test that should_process_lane respects any specified - # language restrictions. - script = CacheRepresentationPerLane( - db.session, - [ - "--language=fre", - "--language=English", - "--language=none", - "--min-depth=0", - ], - manager=object(), - ) - assert ["fre", "eng"] == script.languages - - english_lane = db.lane(languages=["eng"]) - assert True == script.should_process_lane(english_lane) - - no_english_lane = db.lane(languages=["spa", "fre"]) - assert True == script.should_process_lane(no_english_lane) - - no_english_or_french_lane = db.lane(languages=["spa"]) - assert False == script.should_process_lane(no_english_or_french_lane) - - # Test that should_process_lane respects maximum depth - # restrictions. - script = CacheRepresentationPerLane( - db.session, ["--max-depth=0", "--min-depth=0"], manager=object() - ) - assert 0 == script.max_depth - - child = db.lane(display_name="sublane") - parent = db.lane(display_name="parent") - parent.sublanes = [child] - assert True == script.should_process_lane(parent) - assert False == script.should_process_lane(child) - - script = CacheRepresentationPerLane( - db.session, ["--min-depth=1"], manager=MockCirculationManager(db.session) - ) - assert 1 == script.min_depth - assert False == script.should_process_lane(parent) - assert True == script.should_process_lane(child) - - def test_process_lane(self, lane_script_fixture: LaneScriptFixture): - db = lane_script_fixture.db - # process_lane() calls do_generate() once for every - # combination of items yielded by facets() and pagination(). - - class MockFacets: - def __init__(self, query): - self.query = query - - @property - def query_string(self): - return self.query - - facets1 = MockFacets("facets1") - facets2 = MockFacets("facets2") - page1 = Pagination.default() - page2 = page1.next_page - - class Mock(CacheRepresentationPerLane): - generated = [] - - def do_generate(self, lane, facets, pagination): - value = (lane, facets, pagination) - response = Response("mock response") - response.value = value - self.generated.append(response) - return response - - def facets(self, lane): - yield facets1 - yield facets2 - - def pagination(self, lane): - yield page1 - yield page2 - - lane = db.lane() - script = Mock(db.session, manager=object(), cmd_args=[]) - generated = script.process_lane(lane) - assert generated == script.generated - - c1, c2, c3, c4 = (x.value for x in script.generated) - assert (lane, facets1, page1) == c1 - assert (lane, facets1, page2) == c2 - assert (lane, facets2, page1) == c3 - assert (lane, facets2, page2) == c4 - - def test_default_facets(self, lane_script_fixture: LaneScriptFixture): - db = lane_script_fixture.db - # By default, do_generate will only be called once, with facets=None. - script = CacheRepresentationPerLane(db.session, manager=object(), cmd_args=[]) - assert [None] == list(script.facets(object())) - - def test_default_pagination(self, lane_script_fixture: LaneScriptFixture): - db = lane_script_fixture.db - # By default, do_generate will only be called once, with pagination=None. - script = CacheRepresentationPerLane(db.session, manager=object(), cmd_args=[]) - assert [None] == list(script.pagination(object())) - - -class TestCacheFacetListsPerLane: - def test_arguments(self, lane_script_fixture: LaneScriptFixture): - db = lane_script_fixture.db - # Verify that command-line arguments become attributes of - # the CacheFacetListsPerLane object. - script = CacheFacetListsPerLane( - db.session, ["--order=title", "--order=added"], manager=object() - ) - assert ["title", "added"] == script.orders - script = CacheFacetListsPerLane( - db.session, - ["--availability=all", "--availability=always"], - manager=object(), - ) - assert ["all", "always"] == script.availabilities - - script = CacheFacetListsPerLane( - db.session, ["--collection=main", "--collection=full"], manager=object() - ) - assert ["main", "full"] == script.collections - - script = CacheFacetListsPerLane( - db.session, ["--entrypoint=Audio", "--entrypoint=Book"], manager=object() - ) - assert ["Audio", "Book"] == script.entrypoints - - script = CacheFacetListsPerLane(db.session, ["--pages=1"], manager=object()) - assert 1 == script.pages - - def test_facets( - self, lane_script_fixture: LaneScriptFixture, library_fixture: LibraryFixture - ): - db = lane_script_fixture.db - # Verify that CacheFacetListsPerLane.facets combines the items - # found in the attributes created by command-line parsing. - script = CacheFacetListsPerLane(db.session, manager=object(), cmd_args=[]) - script.orders = [Facets.ORDER_TITLE, Facets.ORDER_AUTHOR, "nonsense"] - script.entrypoints = [ - AudiobooksEntryPoint.INTERNAL_NAME, - "nonsense", - EbooksEntryPoint.INTERNAL_NAME, - ] - script.availabilities = [Facets.AVAILABLE_NOW, "nonsense"] - script.collections = [Facets.COLLECTION_FULL, "nonsense"] - - library = library_fixture.library() - - # EbooksEntryPoint is normally a valid entry point, but we're - # going to disable it for this library. - settings = library_fixture.mock_settings() - settings.enabled_entry_points = [AudiobooksEntryPoint.INTERNAL_NAME] - library.update_settings(settings) - - lane = db.lane(library=library) - - # We get one Facets object for every valid combination - # of parameters. Here there are 2*1*1*1 combinations. - f1, f2 = script.facets(lane) - - # The facets differ only in their .order. - assert Facets.ORDER_TITLE == f1.order - assert Facets.ORDER_AUTHOR == f2.order - - # All other fields are tied to the only acceptable values - # given in the script attributes. The first (and only) - # enabled entry point is treated as the default. - for f in f1, f2: - assert AudiobooksEntryPoint == f.entrypoint - assert True == f.entrypoint_is_default - assert Facets.AVAILABLE_NOW == f.availability - assert Facets.COLLECTION_FULL == f.collection - - # The first entry point is treated as the default only for WorkLists - # that have no parent. When the WorkList has a parent, the selected - # entry point is treated as an explicit choice -- navigating downward - # in the lane hierarchy ratifies the default value. - sublane = db.lane(parent=lane, library=library) - f1, f2 = script.facets(sublane) - for f in f1, f2: - assert False == f.entrypoint_is_default - - def test_pagination(self, lane_script_fixture: LaneScriptFixture): - db = lane_script_fixture.db - script = CacheFacetListsPerLane(db.session, manager=object(), cmd_args=[]) - script.pages = 3 - lane = db.lane() - p1, p2, p3 = script.pagination(lane) - pagination = Pagination.default() - assert pagination.query_string == p1.query_string - assert pagination.next_page.query_string == p2.query_string - assert pagination.next_page.next_page.query_string == p3.query_string - - def test_do_generate( - self, - lane_script_fixture: LaneScriptFixture, - end_to_end_search_fixture: EndToEndSearchFixture, - ): - db = lane_script_fixture.db - migration = end_to_end_search_fixture.external_search_index.start_migration() - assert migration is not None - migration.finish() - - # When it's time to generate a feed, AcquisitionFeed.page - # is called with the right arguments. - class MockAcquisitionFeed: - called_with = None - - @classmethod - def page(cls, **kwargs): - cls.called_with = kwargs - resp = MagicMock() - resp.as_response.return_value = "here's your feed" - return resp - - # Test our ability to generate a single feed. - script = CacheFacetListsPerLane(db.session, testing=True, cmd_args=[]) - facets = Facets.default(db.default_library()) - pagination = Pagination.default() - - with script.app.test_request_context("/"): - lane = db.lane() - result = script.do_generate( - lane, facets, pagination, feed_class=MockAcquisitionFeed - ) - assert "here's your feed" == result - - args = MockAcquisitionFeed.called_with - assert db.session == args["_db"] # type: ignore - assert lane == args["worklist"] # type: ignore - assert lane.display_name == args["title"] # type: ignore - - # The Pagination object was passed into - # MockAcquisitionFeed.page, and it was also used to make the - # feed URL (see below). - assert pagination == args["pagination"] # type: ignore - - # The Facets object was passed into - # MockAcquisitionFeed.page, and it was also used to make - # the feed URL and to create the feed annotator. - assert facets == args["facets"] # type: ignore - annotator = args["annotator"] # type: ignore - assert facets == annotator.facets - assert args["url"] == annotator.feed_url( # type: ignore - lane, facets=facets, pagination=pagination - ) - - # Try again without mocking AcquisitionFeed, to verify that - # we get a Flask Response containing an OPDS feed. - response = script.do_generate(lane, facets, pagination) - assert isinstance(response, OPDSFeedResponse) - assert OPDSFeed.ACQUISITION_FEED_TYPE == response.content_type - assert response.get_data(as_text=True).startswith(" timestamp1 - - # Since there was a matching CachedFeed in the database - # already, that CachedFeed was passed into _should_refresh -- - # previously this value was None. - assert (result1, 42) == Mock._should_refresh_called_with - - # Now try the scenario where the feed does not need to be refreshed. - clear_helpers() - Mock.SHOULD_REFRESH = False - result3 = m( - db.session, - worklist, - facets, - pagination, - refresher, - max_age, - raw=True, - ) - - # Not only do we have the same CachedFeed as before, but its - # timestamp and content are unchanged. - assert result3 == result2 - assert "This is feed #2" == result3.content - assert timestamp2 == result3.timestamp - - # If max_age ends up zero, we don't check for the existence of a - # cached feed before forging ahead. - Mock.MAX_CACHE_AGE = 0 - clear_helpers() - m( - db.session, - worklist, - facets, - pagination, - refresher, - max_age, - raw=True, - ) - - # A matching CachedFeed exists in the database, but we didn't - # even look for it, because we knew we'd be looking it up - # again after feed generation. - assert (None, 0) == Mock._should_refresh_called_with - - def test_no_race_conditions(self, db: DatabaseTransactionFixture): - # Why do we look up a CachedFeed again after feed generation? - # Well, let's see what happens if someone else messes around - # with the CachedFeed object _while the refresher is running_. - # - # This is a race condition that happens in real life. Rather - # than setting up a multi-threaded test, we can have the - # refresher itself simulate a background modification by - # messing around with the CachedFeed object we know will - # eventually be returned. - # - # The most up-to-date feed always wins, so background - # modifications will take effect only if they made the - # CachedFeed look _newer_ than the foreground process does. - facets = Facets.default(db.default_library()) - pagination = Pagination.default() - wl = WorkList() - wl.initialize(db.default_library()) - - m = CachedFeed.fetch - - # In this case, two simulated threads try to create the same - # CachedFeed at the same time. We end up with a single - # CachedFeed containing the result of the last code that ran. - def simultaneous_refresher(): - # This refresher method simulates another thread creating - # a CachedFeed for this feed while this thread's - # refresher is running. - def other_thread_refresher(): - return "Another thread made a feed." - - m( - db.session, - wl, - facets, - pagination, - other_thread_refresher, - 0, - raw=True, - ) - - return "Then this thread made a feed." - - # This will call simultaneous_refresher(), which will call - # CachedFeed.fetch() _again_, which will call - # other_thread_refresher(). - result = m( - db.session, - wl, - facets, - pagination, - simultaneous_refresher, - 0, - raw=True, - ) - - # We ended up with a single CachedFeed containing the - # latest information. - assert [result] == db.session.query(CachedFeed).all() - assert "Then this thread made a feed." == result.content - - # If two threads contend for an existing CachedFeed, the one that - # sets CachedFeed.timestamp to the later value wins. - # - # Here, the other thread wins by setting .timestamp on the - # existing CachedFeed to a date in the future. - now = utc_now() - tomorrow = now + datetime.timedelta(days=1) - yesterday = now - datetime.timedelta(days=1) - - def tomorrow_vs_now(): - result.content = "Someone in the background set tomorrow's content." - result.timestamp = tomorrow - return "Today's content can't compete." - - tomorrow_result = m( - db.session, - wl, - facets, - pagination, - tomorrow_vs_now, - 0, - raw=True, - ) - assert tomorrow_result == result - assert ( - "Someone in the background set tomorrow's content." - == tomorrow_result.content - ) - assert tomorrow_result.timestamp == tomorrow - - # Here, the other thread sets .timestamp to a date in the past, and - # it loses out to the (apparently) newer feed. - def yesterday_vs_now(): - result.content = "Someone in the background set yesterday's content." - result.timestamp = yesterday - return "Today's content is fresher." - - now_result = m( - db.session, - wl, - facets, - pagination, - yesterday_vs_now, - 0, - raw=True, - ) - - # We got the same CachedFeed we've been getting this whole - # time, but the outdated data set by the 'background thread' - # has been fixed. - assert result == now_result - assert "Today's content is fresher." == result.content - assert result.timestamp > yesterday - - # This shouldn't happen, but if the CachedFeed's timestamp or - # content are *cleared out* in the background, between the - # time the CacheFeed is fetched and the time the refresher - # finishes, then we don't know what's going on and we don't - # take chances. We create a whole new CachedFeed object for - # the updated version of the feed. - - # First, try the situation where .timestamp is cleared out in - # the background. - def timestamp_cleared_in_background(): - result.content = "Someone else sets content and clears timestamp." - result.timestamp = None - - return "Non-weird content." - - result2 = m( - db.session, - wl, - facets, - pagination, - timestamp_cleared_in_background, - 0, - raw=True, - ) - now = utc_now() - - # result2 is a brand new CachedFeed. - assert result2 != result - assert "Non-weird content." == result2.content - assert (now - result2.timestamp).total_seconds() < 2 - - # We let the background process do whatever it wants to do - # with the old one. - assert "Someone else sets content and clears timestamp." == result.content - assert None == result.timestamp - - # Next, test the situation where .content is cleared out. - def content_cleared_in_background(): - result2.content = None - result2.timestamp = tomorrow - - return "Non-weird content." - - result3 = m( - db.session, - wl, - facets, - pagination, - content_cleared_in_background, - 0, - raw=True, - ) - now = utc_now() - - # Again, a brand new CachedFeed. - assert result3 != result2 - assert result3 != result - assert "Non-weird content." == result3.content - assert (now - result3.timestamp).total_seconds() < 2 - - # Again, we let the background process have the old one for - # whatever weird thing it wants to do. - assert None == result2.content - assert tomorrow == result2.timestamp - - def test_response_format(self, db: DatabaseTransactionFixture): - # Verify that fetch() can be told to return an appropriate - # OPDSFeedResponse object. This is the default behavior, since - # it preserves some useful information that would otherwise be - # lost. - facets = Facets.default(db.default_library()) - pagination = Pagination.default() - wl = WorkList() - wl.initialize(db.default_library()) - - def refresh(): - return "Here's a feed." - - private = object() - r = CachedFeed.fetch( - db.session, - wl, - facets, - pagination, - refresh, - max_age=102, - private=private, - ) - assert isinstance(r, OPDSFeedResponse) - assert 200 == r.status_code - assert OPDSFeed.ACQUISITION_FEED_TYPE == r.content_type - assert 102 == r.max_age - assert "Here's a feed." == str(r) - - # The extra argument `private`, not used by CachedFeed.fetch, was - # passed on to the OPDSFeedResponse constructor. - assert private == r.private - - # The CachedFeed was created; just not returned. - cf = db.session.query(CachedFeed).one() - assert "Here's a feed." == cf.content - - # Try it again as a cache hit. - r = CachedFeed.fetch( - db.session, - wl, - facets, - pagination, - refresh, - max_age=102, - private=private, - ) - assert isinstance(r, OPDSFeedResponse) - assert 200 == r.status_code - assert OPDSFeed.ACQUISITION_FEED_TYPE == r.content_type - assert 102 == r.max_age - assert "Here's a feed." == str(r) - - # If we tell CachedFeed to cache its feed 'forever', that only - # applies to the _database_ cache. The client is told to cache - # the feed for the default period. - r = CachedFeed.fetch( - db.session, - wl, - facets, - pagination, - refresh, - max_age=CachedFeed.CACHE_FOREVER, - private=private, - ) - assert isinstance(r, OPDSFeedResponse) - assert OPDSFeed.DEFAULT_MAX_AGE == r.max_age - - # If the Library associated with the WorkList used in the feed - # has root lanes, `private` is always set to True, even if we - # asked for the opposite. - - from core.model import Library - - Library._has_root_lane_cache[db.default_library().id] = True - r = CachedFeed.fetch( - db.session, - wl, - facets, - pagination, - refresh, - private=False, - ) - assert isinstance(r, OPDSFeedResponse) - assert True == r.private - - # Tests of helper methods. - - def test_feed_type(self): - # Verify that a WorkList or a Facets object can determine the - # value to be stored in CachedFeed.type, with Facets taking - # priority. - class DontCare: - CACHED_FEED_TYPE = None - - class WorkList: - CACHED_FEED_TYPE = "from worklist" - - class Facets: - CACHED_FEED_TYPE = "from facets" - - m = CachedFeed.feed_type - - # The default type is PAGE_TYPE. - assert CachedFeed.PAGE_TYPE == m(None, None) - assert CachedFeed.PAGE_TYPE == m(DontCare, DontCare) - - # If `worklist` has an opinion and `facets` doesn't, we use that. - assert "from worklist" == m(WorkList, None) - assert "from worklist" == m(WorkList, DontCare) - - # If `facets` has an opinion`, it is always used. - assert "from facets" == m(DontCare, Facets) - assert "from facets" == m(None, Facets) - assert "from facets" == m(WorkList, Facets) - - def test_max_cache_age(self): - m = CachedFeed.max_cache_age - - # If override is provided, that value is always used. - assert 60 == m(None, None, None, 60) - assert 60 == m(None, None, None, datetime.timedelta(minutes=1)) - - # Otherwise, the faceting object gets a chance to weigh in. - class MockFacets: - max_cache_age = 22 - - facets = MockFacets() - assert 22 == m(None, "feed type", facets=facets) - - # If there is no override and the faceting object doesn't - # care, CachedFeed.max_cache_age depends on - # WorkList.max_cache_age. This method can return a few - # different data types. - class MockWorklist: - def max_cache_age(self, type): - return dict( - number=1, - timedelta=datetime.timedelta(seconds=2), - expensive=CachedFeed.CACHE_FOREVER, - dont_cache=None, - )[type] - - # The result is always either a number of seconds or - # CACHE_FOREVER. - wl = MockWorklist() - assert 1 == m(wl, "number", None) - assert 2 == m(wl, "timedelta", None) - assert 0 == m(wl, "dont_cache", None) - assert CachedFeed.CACHE_FOREVER == m(wl, "expensive", None) - - # The faceting object still takes precedence, assuming it has - # an opinion. - facets.max_cache_age = None - assert CachedFeed.CACHE_FOREVER == m(wl, "expensive", facets) - - facets.max_cache_age = 22 - assert 22 == m(wl, "expensive", facets) - - # And an override takes precedence over that. - assert 60 == m(wl, "expensive", facets, 60) - - def test__prepare_keys(self, db: DatabaseTransactionFixture): - # Verify the method that turns WorkList, Facets, and Pagination - # into a unique set of values for CachedFeed fields. - - # First, prepare some mock classes. - class MockCachedFeed(CachedFeed): - feed_type_called_with = None - - @classmethod - def feed_type(cls, worklist, facets): - cls.feed_type_called_with = (worklist, facets) - return "mock type" - - class MockFacets: - query_string = b"facets query string" - - class MockPagination: - query_string = b"pagination query string" - - m = MockCachedFeed._prepare_keys - # A WorkList of some kind is required. - with pytest.raises(ValueError) as excinfo: - m(db.session, None, MockFacets, MockPagination) - assert "Cannot prepare a CachedFeed without a WorkList." in str(excinfo.value) - - # Basic Lane case, no facets or pagination. - lane = db.lane() - - # The response object is a named tuple. feed_type, library and - # lane_id are the only members set. - keys = m(db.session, lane, None, None) - assert "mock type" == keys.feed_type - assert lane.library == keys.library - assert None == keys.work - assert lane.id == keys.lane_id - assert None == keys.unique_key - assert "" == keys.facets_key - assert "" == keys.pagination_key - - # When pagination and/or facets are available, facets_key and - # pagination_key are set appropriately. - keys = m(db.session, lane, MockFacets, MockPagination) - assert "facets query string" == keys.facets_key - assert "pagination query string" == keys.pagination_key - - # Now we can check that feed_type was obtained by passing - # `worklist` and `facets` into MockCachedFeed.feed_type. - assert "mock type" == keys.feed_type - assert (lane, MockFacets) == MockCachedFeed.feed_type_called_with - - # When a WorkList is used instead of a Lane, keys.lane_id is None - # but keys.unique_key is set to worklist.unique_key. - worklist = WorkList() - worklist.initialize( - library=db.default_library(), - display_name="wl", - languages=["eng", "spa"], - audiences=[Classifier.AUDIENCE_CHILDREN], - ) - - keys = m(db.session, worklist, None, None) - assert "mock type" == keys.feed_type - assert worklist.get_library(db.session) == keys.library - assert None == keys.work - assert None == keys.lane_id - assert "wl-eng,spa-Children" == keys.unique_key - assert keys.unique_key == worklist.unique_key - assert "" == keys.facets_key - assert "" == keys.pagination_key - - # When a WorkList is associated with a specific .work, - # that information is included as keys.work. - work = object() - worklist.work = work # type: ignore[attr-defined] - keys = m(db.session, worklist, None, None) - assert work == keys.work - - def test__should_refresh(self): - # Test the algorithm that tells whether a CachedFeed is stale. - m = CachedFeed._should_refresh - - # If there's no CachedFeed, we must always refresh. - assert True == m(None, object()) - - class MockCachedFeed: - def __init__(self, timestamp): - self.timestamp = timestamp - - now = utc_now() - - # This feed was generated five minutes ago. - five_minutes_old = MockCachedFeed(now - datetime.timedelta(minutes=5)) - - # This feed was generated a thousand years ago. - ancient = MockCachedFeed(now - datetime.timedelta(days=1000 * 365)) - - # If we intend to cache forever, then even a thousand-year-old - # feed shouldn't be refreshed. - assert False == m(ancient, CachedFeed.CACHE_FOREVER) - - # Otherwise, it comes down to a date comparison. - - # If we're caching a feed for ten minutes, then the - # five-minute-old feed should not be refreshed. - assert False == m(five_minutes_old, 600) - - # If we're caching a feed for only a few seconds (or not at all), - # then the five-minute-old feed should be refreshed. - assert True == m(five_minutes_old, 0) - assert True == m(five_minutes_old, 1) - - # Realistic end-to-end tests. - - def test_lifecycle_with_lane(self, db: DatabaseTransactionFixture): - facets = Facets.default(db.default_library()) - pagination = Pagination.default() - lane = db.lane("My Lane", languages=["eng", "chi"]) - - # Fetch a cached feed from the database. It comes out updated. - refresher = MockFeedGenerator() - args = (db.session, lane, facets, pagination, refresher) - feed = CachedFeed.fetch(*args, max_age=0, raw=True) - assert "This is feed #1" == feed.content - - assert pagination.query_string == feed.pagination - assert facets.query_string == feed.facets - assert lane.id == feed.lane_id - - # Fetch it again, with a high max_age, and it's cached! - feed = CachedFeed.fetch(*args, max_age=1000, raw=True) - assert "This is feed #1" == feed.content - - # Fetch it with a low max_age, and it gets updated again. - feed = CachedFeed.fetch(*args, max_age=0, raw=True) - assert "This is feed #2" == feed.content - - # The special constant CACHE_FOREVER means it's always cached. - feed = CachedFeed.fetch(*args, max_age=CachedFeed.CACHE_FOREVER, raw=True) - assert "This is feed #2" == feed.content - - def test_lifecycle_with_worklist(self, db: DatabaseTransactionFixture): - facets = Facets.default(db.default_library()) - pagination = Pagination.default() - lane = WorkList() - lane.initialize(db.default_library()) - - # Fetch a cached feed from the database. It comes out updated. - refresher = MockFeedGenerator() - args = (db.session, lane, facets, pagination, refresher) - feed = CachedFeed.fetch(*args, max_age=0, raw=True) - assert "This is feed #1" == feed.content - - assert pagination.query_string == feed.pagination - assert facets.query_string == feed.facets - assert None == feed.lane_id - assert lane.unique_key == feed.unique_key - - # Fetch it again, with a high max_age, and it's cached! - feed = CachedFeed.fetch(*args, max_age=1000, raw=True) - assert "This is feed #1" == feed.content - - # Fetch it with a low max_age, and it gets updated again. - feed = CachedFeed.fetch(*args, max_age=0, raw=True) - assert "This is feed #2" == feed.content - - # The special constant CACHE_FOREVER means it's always cached. - feed = CachedFeed.fetch(*args, max_age=CachedFeed.CACHE_FOREVER, raw=True) - assert "This is feed #2" == feed.content diff --git a/tests/core/models/test_listeners.py b/tests/core/models/test_listeners.py index 2fc0c6e7c8..508aa87046 100644 --- a/tests/core/models/test_listeners.py +++ b/tests/core/models/test_listeners.py @@ -5,13 +5,7 @@ from core import lane, model from core.config import Configuration -from core.model import ( - CachedFeed, - ConfigurationSetting, - Timestamp, - WorkCoverageRecord, - create, -) +from core.model import ConfigurationSetting, Timestamp, WorkCoverageRecord from core.model.listeners import site_configuration_has_changed from core.util.datetime_helpers import utc_now from tests.fixtures.database import DatabaseTransactionFixture @@ -219,15 +213,6 @@ def test_configuration_relevant_collection_change_updates_configuration( session.commit() data.mock.assert_was_called() - # Associating a CachedFeed with the library does _not_ call - # the method, because nothing changed on the Library object and - # we don't listen for 'append' events on Library.cachedfeeds. - create( - session, CachedFeed, type="page", pagination="", facets="", library=library - ) - session.commit() - data.mock.assert_was_not_called() - # NOTE: test_work.py:TestWork.test_reindex_on_availability_change # tests the circumstances under which a database change # requires that a Work's entry in the search index be diff --git a/tests/core/test_lane.py b/tests/core/test_lane.py index 9eac0b8699..8b1b4fc1e5 100644 --- a/tests/core/test_lane.py +++ b/tests/core/test_lane.py @@ -32,7 +32,6 @@ WorkList, ) from core.model import ( - CachedFeed, CustomList, DataSource, Edition, @@ -70,10 +69,8 @@ def test_items(self): assert [expect_items] == list(f.items()) assert "%s=%s" % expect_items == f.query_string - f.max_cache_age = 41 expect_items = [ (f.ENTRY_POINT_FACET_GROUP_NAME, ep.INTERNAL_NAME), - (f.MAX_CACHE_AGE_NAME, "41"), ] assert expect_items == list(f.items()) @@ -95,7 +92,7 @@ def test_navigate(self): old_entrypoint = object() kwargs = dict(extra_key="extra_value") facets = FacetsWithEntryPoint( - old_entrypoint, entrypoint_is_default=True, max_cache_age=123, **kwargs + old_entrypoint, entrypoint_is_default=True, **kwargs ) new_entrypoint = object() new_facets = facets.navigate(new_entrypoint) @@ -110,9 +107,6 @@ def test_navigate(self): # the new Facets object is not using a default EntryPoint. assert False == new_facets.entrypoint_is_default - # The max_cache_age was preserved. - assert 123 == new_facets.max_cache_age - # The keyword arguments used to create the original faceting # object were propagated to its constructor. assert kwargs == new_facets.constructor_kwargs @@ -153,12 +147,11 @@ def _from_request(cls, *args, **kwargs): assert expect == result def test__from_request(self): - # _from_request calls load_entrypoint() and - # load_max_cache_age() and instantiates the class with the - # result. + # _from_request calls load_entrypoint() and instantiates + # the class with the result. class MockFacetsWithEntryPoint(FacetsWithEntryPoint): - # Mock load_entrypoint() and load_max_cache_age() to + # Mock load_entrypoint() to # return whatever values we have set up ahead of time. @classmethod @@ -175,22 +168,15 @@ def load_entrypoint(cls, entrypoint_name, entrypoints, default=None): ) return cls.expect_load_entrypoint - @classmethod - def load_max_cache_age(cls, max_cache_age): - cls.load_max_cache_age_called_with = max_cache_age - return cls.expect_max_cache_age - # Mock the functions that pull information out of an HTTP # request. # EntryPoint.load_entrypoint pulls the facet group name and # the maximum cache age out of the 'request' and passes those - # values into load_entrypoint() and load_max_cache_age. + # values into load_entrypoint() def get_argument(key, default): if key == Facets.ENTRY_POINT_FACET_GROUP_NAME: return "entrypoint name from request" - elif key == Facets.MAX_CACHE_AGE_NAME: - return "max cache age from request" # FacetsWithEntryPoint.load_entrypoint does not use # get_header(). @@ -217,25 +203,19 @@ def m(): MockFacetsWithEntryPoint.expect_load_entrypoint = INVALID_INPUT assert INVALID_INPUT == m() - # Similarly if load_entrypoint() works but load_max_cache_age - # returns a ProblemDetail. expect_entrypoint = object() expect_is_default = object() MockFacetsWithEntryPoint.expect_load_entrypoint = ( expect_entrypoint, expect_is_default, ) - MockFacetsWithEntryPoint.expect_max_cache_age = INVALID_INPUT - assert INVALID_INPUT == m() # Next, test success. The return value of load_entrypoint() is # is passed as 'entrypoint' into the FacetsWithEntryPoint - # constructor. The object returned by load_max_cache_age is - # passed as 'max_cache_age'. + # constructor. # # The object returned by load_entrypoint() does not need to be a # currently enabled entrypoint for the library. - MockFacetsWithEntryPoint.expect_max_cache_age = 345 facets = m() assert isinstance(facets, FacetsWithEntryPoint) assert expect_entrypoint == facets.entrypoint @@ -245,13 +225,8 @@ def m(): ["Selectable entrypoints"], default_entrypoint, ) == MockFacetsWithEntryPoint.load_entrypoint_called_with - assert 345 == facets.max_cache_age assert dict(extra="extra kwarg") == facets.constructor_kwargs assert MockFacetsWithEntryPoint.selectable_entrypoints_called_with == config - assert ( - MockFacetsWithEntryPoint.load_max_cache_age_called_with - == "max cache age from request" - ) def test_load_entrypoint(self): audio = AudiobooksEntryPoint @@ -287,30 +262,6 @@ def test_load_entrypoint(self): # nothing. assert (None, True) == m(audio.INTERNAL_NAME, []) - def test_load_max_cache_age(self): - m = FacetsWithEntryPoint.load_max_cache_age - - # The two valid options for max_cache_age as loaded in from a request are - # IGNORE_CACHE (do not pull from cache) and None (no opinion). - assert None == m("") - assert None == m(None) - assert CachedFeed.IGNORE_CACHE == m(0) - assert CachedFeed.IGNORE_CACHE == m("0") - - # All other values are treated as 'no opinion'. - assert None == m("1") - assert None == m(2) - assert None == m("not a number") - - def test_cache_age(self): - # No matter what type of feed we ask about, the max_cache_age of a - # FacetsWithEntryPoint is whatever is stored in its .max_cache_age. - # - # This is true even for 'feed types' that make no sense. - max_cache_age = object() - facets = FacetsWithEntryPoint(max_cache_age=max_cache_age) - assert max_cache_age == facets.max_cache_age - def test_selectable_entrypoints(self): """The default implementation of selectable_entrypoints just returns the worklist's entrypoints. @@ -352,19 +303,6 @@ def _configure_facets(library, enabled, default): library.settings_dict[f"facets_default_{key}"] = value library._settings = None - def test_max_cache_age(self, db: DatabaseTransactionFixture): - # A default Facets object has no opinion on what max_cache_age - # should be. - facets = Facets( - db.default_library(), - Facets.COLLECTION_FULL, - Facets.AVAILABLE_ALL, - Facets.ORDER_TITLE, - Facets.DISTRIBUTOR_ALL, - Facets.COLLECTION_NAME_ALL, - ) - assert None == facets.max_cache_age - def test_facet_groups(self, db: DatabaseTransactionFixture): db.default_collection().data_source = DataSource.AMAZON facets = Facets( @@ -1291,11 +1229,6 @@ def test_constructor(self): assert entrypoint == facets.entrypoint assert True == facets.entrypoint_is_default - def test_feed_type(self): - # If a grouped feed is built via CachedFeed.fetch, it will be - # filed as a grouped feed. - assert CachedFeed.GROUPS_TYPE == FeaturedFacets.CACHED_FEED_TYPE - def test_default( self, db: DatabaseTransactionFixture, library_fixture: LibraryFixture ): @@ -2231,7 +2164,7 @@ def test_max_cache_age(self): # WorkList is the default cache age for any type of OPDS feed, # no matter what type of feed is being generated. wl = WorkList() - assert OPDSFeed.DEFAULT_MAX_AGE == wl.max_cache_age(object()) + assert OPDSFeed.DEFAULT_MAX_AGE == wl.max_cache_age() def test_filter(self, db: DatabaseTransactionFixture): # Verify that filter() calls modify_search_filter_hook() @@ -3011,13 +2944,9 @@ class Mock(DatabaseBackedWorkList): def _modify_loading(cls, qu): return [qu, "_modify_loading"] - @classmethod - def _defer_unused_fields(cls, qu): - return qu + ["_defer_unused_fields"] - result = Mock.base_query(db.session) - [base_query, m, d] = result + [base_query, m] = result expect = ( db.session.query(Work) .join(Work.license_pools) @@ -3026,7 +2955,6 @@ def _defer_unused_fields(cls, qu): ) assert str(expect) == str(base_query) assert "_modify_loading" == m - assert "_defer_unused_fields" == d def test_bibliographic_filter_clauses(self, db: DatabaseTransactionFixture): called = dict() diff --git a/tests/core/test_monitor.py b/tests/core/test_monitor.py index 55bf12d010..42104f12e5 100644 --- a/tests/core/test_monitor.py +++ b/tests/core/test_monitor.py @@ -6,7 +6,6 @@ from core.config import Configuration from core.metadata_layer import TimestampData from core.model import ( - CachedFeed, CirculationEvent, Collection, CollectionMissing, @@ -28,7 +27,6 @@ get_one_or_create, ) from core.monitor import ( - CachedFeedReaper, CirculationEventLocationScrubber, CollectionMonitor, CollectionReaper, @@ -1030,8 +1028,6 @@ def test_cutoff(self, db: DatabaseTransactionFixture): Time.time_eq(m.cutoff, utc_now() - m.MAX_AGE) def test_specific_reapers(self, db: DatabaseTransactionFixture): - assert CachedFeed.timestamp == CachedFeedReaper(db.session).timestamp_field - assert 30 == CachedFeedReaper.MAX_AGE assert Credential.expires == CredentialReaper(db.session).timestamp_field assert 1 == CredentialReaper.MAX_AGE assert ( @@ -1040,10 +1036,6 @@ def test_specific_reapers(self, db: DatabaseTransactionFixture): ) assert 60 == PatronRecordReaper.MAX_AGE - def test_where_clause(self, db: DatabaseTransactionFixture): - m = CachedFeedReaper(db.session) - assert "cachedfeeds.timestamp < :timestamp_1" == str(m.where_clause) - def test_run_once(self, db: DatabaseTransactionFixture): # Create four Credentials: two expired, two valid. expired1 = db.credential() @@ -1140,25 +1132,9 @@ def remove_work(self, work): for work in works: WorkCoverageRecord.add_for(work, operation="some operation") - # Each work has a CachedFeed. - for work in works: - feed = CachedFeed( - work=work, type="page", content="content", pagination="", facets="" - ) - db.session.add(feed) - - # Also create a CachedFeed that has no associated Work. - workless_feed = CachedFeed( - work=None, type="page", content="content", pagination="", facets="" - ) - db.session.add(workless_feed) - - db.session.commit() - # Run the reaper. s = MockSearchIndex() m = WorkReaper(db.session, search_index_client=s) - print(m.search_index_client) m.run_once() # Search index was updated @@ -1188,14 +1164,6 @@ def remove_work(self, work): assert 2 == len([x for x in l.entries if not x.work]) assert [has_license_pool] == [x.work for x in l.entries if x.work] - # The CachedFeeds associated with the reaped Works have been - # deleted. The surviving Work still has one, and the - # CachedFeed that didn't have a work in the first place is - # unaffected. - feeds = db.session.query(CachedFeed).all() - assert [workless_feed] == [x for x in feeds if not x.work] - assert [has_license_pool] == [x.work for x in feeds if x.work] - class TestCollectionReaper: def test_query(self, db: DatabaseTransactionFixture): diff --git a/tests/core/test_scripts.py b/tests/core/test_scripts.py index 289140cd33..a4a8f3b17b 100644 --- a/tests/core/test_scripts.py +++ b/tests/core/test_scripts.py @@ -18,7 +18,6 @@ from core.lane import Lane, WorkList from core.metadata_layer import TimestampData from core.model import ( - CachedFeed, Collection, ConfigurationSetting, Contributor, @@ -1723,39 +1722,6 @@ def test_check_library( assert " This library has no collections -- that's a problem." == no_collection assert " This library has no lanes -- that's a problem." == no_lanes - def test_delete_cached_feeds( - self, - db: DatabaseTransactionFixture, - end_to_end_search_fixture: EndToEndSearchFixture, - ): - groups = CachedFeed(type=CachedFeed.GROUPS_TYPE, pagination="") - db.session.add(groups) - not_groups = CachedFeed(type=CachedFeed.PAGE_TYPE, pagination="") - db.session.add(not_groups) - - assert 2 == db.session.query(CachedFeed).count() - - script = MockWhereAreMyBooks( - _db=db.session, search=end_to_end_search_fixture.external_search_index - ) - script.delete_cached_feeds() - how_many, theyre_gone = script.output - assert ( - "%d feeds in cachedfeeds table, not counting grouped feeds.", - [1], - ) == how_many - assert " Deleting them all." == theyre_gone - - # Call it again, and we don't see "Deleting them all". There aren't - # any to delete. - script.output = [] - script.delete_cached_feeds() - [how_many] = script.output - assert ( - "%d feeds in cachedfeeds table, not counting grouped feeds.", - [0], - ) == how_many - @staticmethod def check_explanation( db: DatabaseTransactionFixture,