From 8ab248a34e3dce4383cc9338eb6d05268057b0c2 Mon Sep 17 00:00:00 2001
From: Ethan Dalool <git@voussoir.net>
Date: Tue, 5 Jan 2021 12:59:18 -0800
Subject: [PATCH] Improve naming and comments in FileEtagManager

---
 frontends/etiquette_flask/backend/caching.py | 70 ++++++++++++++------
 frontends/etiquette_flask/backend/common.py  | 11 +--
 2 files changed, 56 insertions(+), 25 deletions(-)

diff --git a/frontends/etiquette_flask/backend/caching.py b/frontends/etiquette_flask/backend/caching.py
index c732175..5565955 100644
--- a/frontends/etiquette_flask/backend/caching.py
+++ b/frontends/etiquette_flask/backend/caching.py
@@ -2,9 +2,10 @@ from voussoirkit import cacheclass
 
 import etiquette
 
-class FileCacheManager:
+class FileEtagManager:
     '''
-    The FileCacheManager serves ETag and Cache-Control headers for disk files.
+    The FileEtagManager serves ETag and Cache-Control headers for disk files to
+    enable client-side caching.
 
     We consider the following cases:
 
@@ -22,11 +23,50 @@ class FileCacheManager:
     file's mtime has changed since the last request.
     '''
     def __init__(self, maxlen, max_age, max_filesize):
+        '''
+        max_len:
+            The number of files to track in this cache.
+        max_age:
+            Integer number of seconds that will be send to the client as
+            Cache-Control:max-age=x
+        max_filesize:
+            Integer number of bytes. Because we use the file's MD5 as its etag,
+            you may wish to prevent the reading of large files. Files larger
+            than this size will not be etagged.
+        '''
         self.cache = cacheclass.Cache(maxlen=maxlen)
         self.max_age = int(max_age)
         self.max_filesize = max(int(max_filesize), 0) or None
 
-    def get(self, filepath):
+    def get_304_headers(self, request, filepath):
+        '''
+        Given a request object and a filepath that we would like to send back
+        as the response, check if the client's provided etag matches the
+        server's cached etag, and return the headers to be used in a 304
+        response (etag, cache-control).
+
+        If the client did not provide an etag, or their etag does not match the
+        current file, or the file cannot be cached, return None.
+        '''
+        client_etag = request.headers.get('If-None-Match', None)
+        if client_etag is None:
+            return None
+
+        server_value = self.get_file(filepath)
+        if server_value is None:
+            return None
+
+        server_etag = server_value.get_etag()
+        if client_etag != server_etag:
+            return None
+
+        return server_value.get_headers()
+
+    def get_file(self, filepath):
+        '''
+        Return a FileEtag object if the filepath can be cached, or None if it
+        cannot (size greater than max_filesize).
+        '''
         try:
             return self.cache[filepath]
         except KeyError:
@@ -35,26 +75,15 @@ class FileCacheManager:
         if (self.max_filesize is not None) and (filepath.size > self.max_filesize):
             return None
 
-        cache_file = CacheFile(filepath, max_age=self.max_age)
+        cache_file = FileEtag(filepath, max_age=self.max_age)
         self.cache[filepath] = cache_file
         return cache_file
 
-    def matches(self, request, filepath):
-        client_etag = request.headers.get('If-None-Match', None)
-        if client_etag is None:
-            return False
-
-        server_value = self.get(filepath)
-        if server_value is None:
-            return False
-
-        server_etag = server_value.get_etag()
-        if client_etag != server_etag:
-            return False
-
-        return server_value.get_headers()
-
-class CacheFile:
+class FileEtag:
+    '''
+    This class represents an individual disk file that is being managed by the
+    FileEtagManager.
+    '''
     def __init__(self, filepath, max_age):
         self.filepath = filepath
         self.max_age = int(max_age)
@@ -68,6 +97,7 @@ class CacheFile:
         if do_refresh:
             self._stored_hash_time = mtime
             self._stored_hash_value = etiquette.helpers.hash_file_md5(self.filepath)
+
         return self._stored_hash_value
 
     def get_headers(self):
diff --git a/frontends/etiquette_flask/backend/common.py b/frontends/etiquette_flask/backend/common.py
index 47b4827..17e9b51 100644
--- a/frontends/etiquette_flask/backend/common.py
+++ b/frontends/etiquette_flask/backend/common.py
@@ -45,7 +45,7 @@ site.debug = True
 site.localhost_only = False
 
 session_manager = sessions.SessionManager(maxlen=10000)
-file_cache_manager = caching.FileCacheManager(
+file_etag_manager = caching.FileEtagManager(
     maxlen=10000,
     max_filesize=5 * bytestring.MIBIBYTE,
     max_age=BROWSER_CACHE_DURATION,
@@ -240,7 +240,7 @@ def send_file(filepath, override_mimetype=None):
 
     file_size = filepath.size
 
-    headers = file_cache_manager.matches(request=request, filepath=filepath)
+    headers = file_etag_manager.get_304_headers(request=request, filepath=filepath)
     if headers:
         response = flask.Response(status=304, headers=headers)
         return response
@@ -292,9 +292,10 @@ def send_file(filepath, override_mimetype=None):
 
     outgoing_headers['Accept-Ranges'] = 'bytes'
     outgoing_headers['Content-Length'] = (range_max - range_min) + 1
-    cache_file = file_cache_manager.get(filepath)
-    if cache_file is not None:
-        outgoing_headers.update(cache_file.get_headers())
+
+    file_etag = file_etag_manager.get_file(filepath)
+    if file_etag is not None:
+        outgoing_headers.update(file_etag.get_headers())
 
     if request.method == 'HEAD':
         outgoing_data = bytes()