Skip to content

Commit

Permalink
PP-554 Remove cachedfeeds (#1476)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
RishiDiwanTT authored Oct 25, 2023
1 parent 13c4b59 commit aa2cfbb
Show file tree
Hide file tree
Showing 31 changed files with 140 additions and 2,513 deletions.
95 changes: 95 additions & 0 deletions alembic/versions/20231020_7fceb9488bc6_drop_cachedfeeds.py
Original file line number Diff line number Diff line change
@@ -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)
13 changes: 0 additions & 13 deletions api/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down Expand Up @@ -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
58 changes: 14 additions & 44 deletions api/controller.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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):
Expand Down Expand Up @@ -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,
)

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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):
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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."""
Expand Down Expand Up @@ -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):
Expand Down
7 changes: 0 additions & 7 deletions api/lanes.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
WorkList,
)
from core.model import (
CachedFeed,
Contributor,
DataSource,
Edition,
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -1250,7 +1246,6 @@ class RelatedBooksLane(WorkBasedLane):
service.
"""

CACHED_FEED_TYPE = CachedFeed.RELATED_TYPE
DISPLAY_NAME = "Related Books"
ROUTE = "related_books"

Expand Down Expand Up @@ -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 = {
Expand Down
11 changes: 0 additions & 11 deletions bin/cache_opds_blocks

This file was deleted.

11 changes: 0 additions & 11 deletions bin/cache_opds_lane_facets

This file was deleted.

2 changes: 1 addition & 1 deletion bin/database_reaper
Original file line number Diff line number Diff line change
@@ -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
Expand Down
3 changes: 0 additions & 3 deletions core/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
3 changes: 0 additions & 3 deletions core/facets.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
2 changes: 0 additions & 2 deletions core/feed/annotator/verbose.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
3 changes: 1 addition & 2 deletions core/feed/opds.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -106,4 +105,4 @@ class UnfulfillableWork(Exception):


class NavigationFacets(FeaturedFacets):
CACHED_FEED_TYPE = CachedFeed.NAVIGATION_TYPE
pass
Loading

0 comments on commit aa2cfbb

Please sign in to comment.