Create generic DB_CACHE functions for db.Model classes (lucyparso…

…ns#1011)

lucyparsons#997

I created a cacheing file that allows us to remove values from the cache
when its underlying dataset has changed for all versions of a
`SQLAlchemy` `Model`.

 - [x] This branch is up-to-date with the `develop` branch.
 - [x] `pytest` passes on my local development environment.
 - [x] `pre-commit` passes on my local development environment.

OpenOversight/app/main/model_view.py

@@ -5,8 +5,10 @@
 from flask.views import MethodView
 from flask_login import current_user, login_required
 
-from OpenOversight.app.models.database import db
+from OpenOversight.app.models.database import Department, db
+from OpenOversight.app.models.database_cache import remove_database_cache_entry
 from OpenOversight.app.utils.auth import ac_or_admin_required
+from OpenOversight.app.utils.constants import KEY_DEPT_INCIDENTS_LAST_UPDATED
 from OpenOversight.app.utils.db import add_department_query
 from OpenOversight.app.utils.forms import set_dynamic_default
 
@@ -65,7 +67,6 @@ def new(self, form=None):
                     set_dynamic_default(form.department, current_user.dept_pref_rel)
             if hasattr(form, "created_by") and not form.created_by.data:
                 form.created_by.data = current_user.get_id()
-            # TODO: Determine whether creating counts as updating, seems redundant
             if hasattr(form, "last_updated_by"):
                 form.last_updated_by.data = current_user.get_id()
                 form.last_updated_at.data = datetime.datetime.now()
@@ -74,6 +75,11 @@ def new(self, form=None):
             new_obj = self.create_function(form)
             db.session.add(new_obj)
             db.session.commit()
+            if self.create_function.__name__ == "create_incident":
+                remove_database_cache_entry(
+                    Department(id=new_obj.department_id),
+                    KEY_DEPT_INCIDENTS_LAST_UPDATED,
+                )
             flash(f"{self.model_name} created!")
             return self.get_redirect_url(obj_id=new_obj.id)
         else:

OpenOversight/app/main/views.py

@@ -71,11 +71,14 @@
     User,
     db,
 )
+from OpenOversight.app.models.database_cache import remove_database_cache_entry
 from OpenOversight.app.utils.auth import ac_or_admin_required, admin_required
 from OpenOversight.app.utils.choices import AGE_CHOICES, GENDER_CHOICES, RACE_CHOICES
 from OpenOversight.app.utils.cloud import crop_image, upload_image_to_s3_and_store_in_db
 from OpenOversight.app.utils.constants import (
     ENCODING_UTF_8,
+    KEY_DEPT_ASSIGNMENTS_LAST_UPDATED,
+    KEY_DEPT_OFFICERS_LAST_UPDATED,
     KEY_OFFICERS_PER_PAGE,
     KEY_TIMEZONE,
 )
@@ -337,6 +340,9 @@ def add_assignment(officer_id):
             current_user.is_area_coordinator
             and officer.department_id == current_user.ac_department_id
         ):
+            remove_database_cache_entry(
+                Department(id=officer.department_id), KEY_DEPT_ASSIGNMENTS_LAST_UPDATED
+            )
             try:
                 add_new_assignment(officer_id, form)
                 flash("Added new assignment!")
@@ -922,6 +928,9 @@ def add_officer():
                 new_form_data[key] = "y"
         form = AddOfficerForm(new_form_data)
         officer = add_officer_profile(form, current_user)
+        remove_database_cache_entry(
+            Department(id=officer.department_id), KEY_DEPT_OFFICERS_LAST_UPDATED
+        )
         flash(f"New Officer {officer.last_name} added to OpenOversight")
         return redirect(url_for("main.submit_officer_images", officer_id=officer.id))
     else:

OpenOversight/app/models/database.py

@@ -5,29 +5,30 @@
 from decimal import Decimal
 
 from authlib.jose import JoseError, JsonWebToken
-from cachetools import TTLCache, cached
-from cachetools.keys import hashkey
+from cachetools import cached
 from flask import current_app
 from flask_login import UserMixin
 from flask_sqlalchemy import SQLAlchemy
 from sqlalchemy import CheckConstraint, UniqueConstraint, func
-
-# from flask_sqlalchemy.model import DefaultMeta
 from sqlalchemy.orm import validates
 from sqlalchemy.sql import func as sql_func
 from werkzeug.security import check_password_hash, generate_password_hash
 
+from OpenOversight.app.models.database_cache import DB_CACHE, db_model_cache_key
 from OpenOversight.app.utils.choices import GENDER_CHOICES, RACE_CHOICES
-from OpenOversight.app.utils.constants import ENCODING_UTF_8
+from OpenOversight.app.utils.constants import (
+    ENCODING_UTF_8,
+    KEY_DEPT_ASSIGNMENTS_LAST_UPDATED,
+    KEY_DEPT_INCIDENTS_LAST_UPDATED,
+    KEY_DEPT_OFFICERS_LAST_UPDATED,
+)
 from OpenOversight.app.validators import state_validator, url_validator
 
 
 db = SQLAlchemy()
 jwt = JsonWebToken("HS512")
 
-BaseModel = (
-    db.Model
-)  # This was here before but it's fucking with my IDE's typing - type: DefaultMeta (MJSB 2021-09-08)
+BaseModel = db.Model
 
 officer_links = db.Table(
     "officer_links",
@@ -58,30 +59,6 @@
 )
 
 
-# This is a last recently used cache that also utilizes a time-to-live function for each
-# value saved in it (12 hours).
-# TODO: Change this into a singleton so that we can clear values when updates happen
-date_updated_cache = TTLCache(maxsize=1024, ttl=12 * 60 * 60)
-
-
-def _date_updated_cache_key(update_type: str):
-    """Return a key function to calculate the cache key for Department
-    `latest_*_update` methods using the department id and a given update type.
-
-    Department.id is used instead of a Department obj because the default Python
-    __hash__ is unique per obj instance, meaning multiple instances of the same
-    department will have different hashes.
-
-    Update type is used in the hash to differentiate between the (currently) three
-    update types we compute per department.
-    """
-
-    def _cache_key(dept: "Department"):
-        return hashkey(dept.id, update_type)
-
-    return _cache_key
-
-
 class Department(BaseModel):
     __tablename__ = "departments"
     id = db.Column(db.Integer, primary_key=True)
@@ -117,7 +94,18 @@ def to_custom_dict(self):
             "unique_internal_identifier_label": self.unique_internal_identifier_label,
         }
 
-    @cached(cache=date_updated_cache, key=_date_updated_cache_key("incident"))
+    @cached(cache=DB_CACHE, key=db_model_cache_key(KEY_DEPT_ASSIGNMENTS_LAST_UPDATED))
+    def latest_assignment_update(self) -> datetime.date:
+        assignment_updated = (
+            db.session.query(func.max(Assignment.date_updated))
+            .join(Officer)
+            .filter(Assignment.officer_id == Officer.id)
+            .filter(Officer.department_id == self.id)
+            .scalar()
+        )
+        return assignment_updated.date() if assignment_updated else None
+
+    @cached(cache=DB_CACHE, key=db_model_cache_key(KEY_DEPT_INCIDENTS_LAST_UPDATED))
     def latest_incident_update(self) -> datetime.date:
         incident_updated = (
             db.session.query(func.max(Incident.date_updated))
@@ -126,7 +114,7 @@ def latest_incident_update(self) -> datetime.date:
         )
         return incident_updated.date() if incident_updated else None
 
-    @cached(cache=date_updated_cache, key=_date_updated_cache_key("officer"))
+    @cached(cache=DB_CACHE, key=db_model_cache_key(KEY_DEPT_OFFICERS_LAST_UPDATED))
     def latest_officer_update(self) -> datetime.date:
         officer_updated = (
             db.session.query(func.max(Officer.date_updated))
@@ -135,17 +123,6 @@ def latest_officer_update(self) -> datetime.date:
         )
         return officer_updated.date() if officer_updated else None
 
-    @cached(cache=date_updated_cache, key=_date_updated_cache_key("assignment"))
-    def latest_assignment_update(self) -> datetime.date:
-        assignment_updated = (
-            db.session.query(func.max(Assignment.date_updated))
-            .join(Officer)
-            .filter(Assignment.officer_id == Officer.id)
-            .filter(Officer.department_id == self.id)
-            .scalar()
-        )
-        return assignment_updated.date() if assignment_updated else None
-
 
 class Job(BaseModel):
     __tablename__ = "jobs"

OpenOversight/app/models/database_cache.py

@@ -0,0 +1,44 @@
+from cachetools import TTLCache
+from cachetools.keys import hashkey
+from flask_sqlalchemy.model import Model
+
+from OpenOversight.app.utils.constants import HOUR
+
+
+DB_CACHE = TTLCache(maxsize=1024, ttl=12 * HOUR)
+
+
+def model_key(model: Model, update_type: str):
+    """Create unique db.Model key."""
+    return hashkey(model.id, update_type, model.__class__.__name__)
+
+
+def db_model_cache_key(update_type: str):
+    """Return a key function to calculate the cache key for db.Model
+    methods using the db.Model id and a given update type.
+
+    db.Model.id is used instead of a db.Model obj because the default Python
+    __hash__ is unique per obj instance, meaning multiple instances of the same
+    department will have different hashes.
+
+    Update type is used in the hash to differentiate between the update types we compute
+    per department.
+    """
+
+    def _cache_key(model: Model):
+        return model_key(model, update_type)
+
+    return _cache_key
+
+
+def has_database_cache_entry(model: Model, update_type: str) -> bool:
+    """db.Model key exists in cache."""
+    key = model_key(model, update_type)
+    return key in DB_CACHE.keys()
+
+
+def remove_database_cache_entry(model: Model, update_type: str) -> None:
+    """Remove db.Model key from cache if it exists."""
+    key = model_key(model, update_type)
+    if key in DB_CACHE.keys():
+        del DB_CACHE[key]

OpenOversight/app/templates/sort.html

@@ -80,7 +80,7 @@ <h3>Your account has been disabled due to too many incorrect classifications/tag
              role="button">Email us to get it enabled again</a>
         </p>
       {% else %}
-        <h3>All images have been classfied!</h3>
+        <h3>All images have been classified!</h3>
         <p>
           <a href="{{ url_for("main.submit_data") }}"
              class="btn btn-lg btn-primary"

OpenOversight/app/utils/constants.py

@@ -2,15 +2,9 @@
 
 
 # Cache Key Constants
-KEY_DEPT_ALL_ASSIGNMENTS = "all_department_assignments"
-KEY_DEPT_ALL_INCIDENTS = "all_department_incidents"
-KEY_DEPT_ALL_LINKS = "all_department_links"
-KEY_DEPT_ALL_NOTES = "all_department_notes"
-KEY_DEPT_ALL_OFFICERS = "all_department_officers"
-KEY_DEPT_ALL_SALARIES = "all_department_salaries"
-KEY_DEPT_TOTAL_ASSIGNMENTS = "total_department_assignments"
-KEY_DEPT_TOTAL_INCIDENTS = "total_department_incidents"
-KEY_DEPT_TOTAL_OFFICERS = "total_department_officers"
+KEY_DEPT_ASSIGNMENTS_LAST_UPDATED = "assignments_last_updated"
+KEY_DEPT_INCIDENTS_LAST_UPDATED = "incidents_last_updated"
+KEY_DEPT_OFFICERS_LAST_UPDATED = "officers_last_updated"
 
 # Database Key Constants
 KEY_DB_CREATOR = "creator"

OpenOversight/tests/conftest.py

@@ -76,7 +76,7 @@ def __init__(self, name, short_name, state="", unique_internal_identifier_label=
     ("IVANA", "", "TINKLE"),
     ("SEYMOUR", "", "BUTZ"),
     ("HAYWOOD", "U", "CUDDLEME"),
-    ("BEA", "", "PROBLEM"),
+    ("BEA", "", "O'PROBLEM"),
     ("URA", "", "SNOTBALL"),
     ("HUGH", "", "JASS"),
 ]