Repository: internetarchive/openlibrary
Issue category: Performance Issue

Cache Patron's Active Loans
<!-- A clear and concise description of what the problem is. Ex. I'm always frustrated when [...] -->

On several pages (e.g. LoanStatus) we fetch the patron's active loans (which can be expensive) to see if they've borrowed a book (e.g. on the book page).

Ideally, we'd cache this every 5 minutes and invalidate within the /borrow endpoint. The one "gotcha" is if a patron borrows a book from archive.org, Open Library doesn't know how to bust the cache, but that should be okay because presumably this will be fixed within the 5 minute window.

Exploration required into whether cache actually works for this and how performance is impacted.




Functions to edit:
openlibrary/core/lending.py:get_loans_of_user
openlibrary/core/models.py:User.get_loan_for
openlibrary/core/models.py:User.get_waiting_loan_for
openlibrary/plugins/upstream/borrow.py:borrow.POST
Functions added: openlibrary/core/lending.py:get_user_waiting_loans
openlibrary/core/models.py:User.get_user_waiting_loans

Patch:
diff --git a/openlibrary/core/lending.py b/openlibrary/core/lending.py
index 6162ed5b081..d7e2a1949cb 100644
--- a/openlibrary/core/lending.py
+++ b/openlibrary/core/lending.py
@@ -511,13 +511,53 @@ def _get_ia_loan(identifier, userid):
 
 def get_loans_of_user(user_key):
     """TODO: Remove inclusion of local data; should only come from IA"""
+    if 'env' not in web.ctx:
+        """For the get_cached_user_loans to call the API if no cache is present,
+        we have to fakeload the web.ctx
+        """
+        delegate.fakeload()
+
     account = OpenLibraryAccount.get(username=user_key.split('/')[-1])
 
     loandata = web.ctx.site.store.values(type='/type/loan', name='user', value=user_key)
     loans = [Loan(d) for d in loandata] + (_get_ia_loans_of_user(account.itemname))
+    # Set patron's loans in cache w/ now timestamp
+    get_cached_loans_of_user.memcache_set(
+        [user_key], {}, loans or [], time.time()
+    )  # rehydrate cache
     return loans
 
 
+get_cached_loans_of_user = cache.memcache_memoize(
+    get_loans_of_user,
+    key_prefix='lending.cached_loans',
+    timeout=5 * dateutil.MINUTE_SECS,  # time to live for cached loans = 5 minutes
+)
+
+
+def get_user_waiting_loans(user_key):
+    """Gets the waitingloans of the patron.
+
+    Returns [] if user has no waitingloans.
+    """
+    from .waitinglist import WaitingLoan
+
+    account = OpenLibraryAccount.get(key=user_key)
+    itemname = account.itemname
+    result = WaitingLoan.query(userid=itemname)
+    get_cached_user_waiting_loans.memcache_set(
+        [user_key], {}, result or {}, time.time()
+    )  # rehydrate cache
+    return result or []
+
+
+get_cached_user_waiting_loans = cache.memcache_memoize(
+    get_user_waiting_loans,
+    key_prefix='waitinglist.user_waiting_loans',
+    timeout=10 * dateutil.MINUTE_SECS,
+)
+
+
 def _get_ia_loans_of_user(userid):
     ia_loans = ia_lending_api.find_loans(userid=userid)
     return [Loan.from_ia_loan(d) for d in ia_loans]
diff --git a/openlibrary/core/models.py b/openlibrary/core/models.py
index d582db128c4..b4b3c1cd2a5 100644
--- a/openlibrary/core/models.py
+++ b/openlibrary/core/models.py
@@ -16,6 +16,7 @@
 
 # TODO: fix this. openlibrary.core should not import plugins.
 from openlibrary import accounts
+from openlibrary.core import lending
 from openlibrary.catalog import add_book
 from openlibrary.core.booknotes import Booknotes
 from openlibrary.core.bookshelves import Bookshelves
@@ -24,7 +25,7 @@
 from openlibrary.core.observations import Observations
 from openlibrary.core.ratings import Ratings
 from openlibrary.core.vendors import create_edition_from_amazon_metadata
-from openlibrary.utils import extract_numeric_id_from_olid
+from openlibrary.utils import extract_numeric_id_from_olid, dateutil
 from openlibrary.utils.isbn import to_isbn_13, isbn_13_to_isbn_10, canonical
 
 # Seed might look unused, but removing it causes an error :/
@@ -916,25 +917,51 @@ def has_borrowed(self, book):
         loan = self.get_loan_for(book.ocaid)
         return loan is not None
 
-    def get_loan_for(self, ocaid):
+    def get_loan_for(self, ocaid, use_cache=False):
         """Returns the loan object for given ocaid.
 
         Returns None if this user hasn't borrowed the given book.
         """
         from ..plugins.upstream import borrow
 
-        loans = borrow.get_loans(self)
+        loans = (
+            lending.get_cached_loans_of_user(self.key)
+            if use_cache
+            else lending.get_loans_of_user(self.key)
+        )
         for loan in loans:
             if ocaid == loan['ocaid']:
                 return loan
 
     def get_waiting_loan_for(self, ocaid):
         """
-        :param str or None ocaid:
+        :param str or None ocaid: edition ocaid
         :rtype: dict (e.g. {position: number})
         """
         return ocaid and WaitingLoan.find(self.key, ocaid)
 
+    def get_user_waiting_loans(self, ocaid=None, use_cache=False):
+        """
+        Similar to get_waiting_loan_for, but fetches and caches all of user's waiting loans
+        :param str or None ocaid: edition ocaid
+        :rtype: dict (e.g. {position: number})
+        """
+        all_user_waiting_loans = (
+            lending.get_cached_user_waiting_loans
+            if use_cache
+            else lending.get_user_waiting_loans
+        )(self.key)
+        if ocaid:
+            return next(
+                (
+                    loan
+                    for loan in all_user_waiting_loans
+                    if loan['identifier'] == ocaid
+                ),
+                None,
+            )
+        return all_user_waiting_loans
+
     def __repr__(self):
         return "<User: %s>" % repr(self.key)
 
diff --git a/openlibrary/core/waitinglist.py b/openlibrary/core/waitinglist.py
index 5381e4614b6..207bae4da44 100644
--- a/openlibrary/core/waitinglist.py
+++ b/openlibrary/core/waitinglist.py
@@ -16,6 +16,7 @@
 import logging
 import web
 from openlibrary.accounts.model import OpenLibraryAccount
+from openlibrary.core import cache
 from . import helpers as h
 from .sendmail import sendmail_with_template
 from . import db
diff --git a/openlibrary/macros/LoanStatus.html b/openlibrary/macros/LoanStatus.html
index b9649006622..7c6427a7378 100644
--- a/openlibrary/macros/LoanStatus.html
+++ b/openlibrary/macros/LoanStatus.html
@@ -17,7 +17,7 @@
 $ work_key = work_key or (doc.get('works') and doc.works[0].key)
 
 $ waiting_loan_start_time = time()
-$ waiting_loan = check_loan_status and ocaid and ctx.user and ctx.user.get_waiting_loan_for(ocaid)
+$ waiting_loan = check_loan_status and ocaid and ctx.user and ctx.user.get_user_waiting_loans(ocaid, use_cache=True)
 $ waiting_loan_total_time = time() - waiting_loan_start_time
 $ my_turn_to_borrow = waiting_loan and waiting_loan['status'] == 'available' and waiting_loan['position'] == 1
 
@@ -34,7 +34,7 @@
 
 $# Checks to see if patron has actively loan / waitlist for this book
 $ get_loan_for_start_time = time()
-$ user_loan = doc.get('loan') or (check_loan_status and ocaid and ctx.user and ctx.user.get_loan_for(ocaid))
+$ user_loan = doc.get('loan') or (check_loan_status and ocaid and ctx.user and ctx.user.get_loan_for(ocaid, use_cache=True))
 $ get_loan_for_total_time = time() - get_loan_for_start_time
 
 $ is_edition = doc.key.split('/')[1] == 'books'
diff --git a/openlibrary/plugins/upstream/borrow.py b/openlibrary/plugins/upstream/borrow.py
index f48c1656115..9e9827046c7 100644
--- a/openlibrary/plugins/upstream/borrow.py
+++ b/openlibrary/plugins/upstream/borrow.py
@@ -149,6 +149,9 @@ def POST(self, key):
             account = OpenLibraryAccount.get_by_email(user.email)
             ia_itemname = account.itemname if account else None
             s3_keys = web.ctx.site.store.get(account._key).get('s3_keys')
+            lending.get_cached_loans_of_user.memcache_delete(
+                user.key, {}
+            )  # invalidate cache for user loans
         if not user or not ia_itemname or not s3_keys:
             web.setcookie(config.login_cookie_name, "", expires=-1)
             redirect_url = (
@@ -165,10 +168,16 @@ def POST(self, key):
             user.update_loan_status()
             raise web.seeother(edition_redirect)
         elif action == 'join-waitinglist':
+            lending.get_cached_user_waiting_loans.memcache_delete(
+                user.key, {}
+            )  # invalidate cache for user waiting loans
             lending.s3_loan_api(s3_keys, ocaid=edition.ocaid, action='join_waitlist')
             stats.increment('ol.loans.joinWaitlist')
             raise web.redirect(edition_redirect)
         elif action == 'leave-waitinglist':
+            lending.get_cached_user_waiting_loans.memcache_delete(
+                user.key, {}
+            )  # invalidate cache for user waiting loans
             lending.s3_loan_api(s3_keys, ocaid=edition.ocaid, action='leave_waitlist')
             stats.increment('ol.loans.leaveWaitlist')
             raise web.redirect(edition_redirect)