From 32eec2cb12a310ec72d4022019627f7862865d92 Mon Sep 17 00:00:00 2001
From: Michael Plunkett <5885605+michplunkett@users.noreply.github.com>
Date: Mon, 7 Aug 2023 12:00:06 -0500
Subject: [PATCH] Create generic `DB_CACHE` functions for `db.Model` classes
 (#1011)

## Fixes issue
https://github.com/lucyparsons/OpenOversight/issues/997

## Description of Changes
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`.

## Tests and linting
 - [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       |   9 +-
 OpenOversight/app/main/views.py            |   9 +
 OpenOversight/app/models/database.py       |  43 +----
 OpenOversight/app/models/database_cache.py |  44 +++++
 OpenOversight/app/templates/sort.html      |   2 +-
 OpenOversight/app/utils/constants.py       |   6 +-
 OpenOversight/tests/conftest.py            |   2 +-
 OpenOversight/tests/test_cache.py          | 185 +++++++++++++++++++++
 OpenOversight/tests/test_functional.py     |   2 +-
 9 files changed, 260 insertions(+), 42 deletions(-)
 create mode 100644 OpenOversight/app/models/database_cache.py
 create mode 100644 OpenOversight/tests/test_cache.py

diff --git a/OpenOversight/app/main/model_view.py b/OpenOversight/app/main/model_view.py
index 02a64842f..ddc466826 100644
--- a/OpenOversight/app/main/model_view.py
+++ b/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_TOTAL_INCIDENTS
 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,10 @@ 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_TOTAL_INCIDENTS
+                )
             flash(f"{self.model_name} created!")
             return self.get_redirect_url(obj_id=new_obj.id)
         else:
diff --git a/OpenOversight/app/main/views.py b/OpenOversight/app/main/views.py
index 762fec2df..75563355e 100644
--- a/OpenOversight/app/main/views.py
+++ b/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_TOTAL_ASSIGNMENTS,
+    KEY_DEPT_TOTAL_OFFICERS,
     KEY_OFFICERS_PER_PAGE,
     KEY_TIMEZONE,
 )
@@ -340,6 +343,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_TOTAL_ASSIGNMENTS
+            )
             try:
                 add_new_assignment(officer_id, form)
                 flash("Added new assignment!")
@@ -925,6 +931,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_TOTAL_OFFICERS
+        )
         flash(f"New Officer {officer.last_name} added to OpenOversight")
         return redirect(url_for("main.submit_officer_images", officer_id=officer.id))
     else:
diff --git a/OpenOversight/app/models/database.py b/OpenOversight/app/models/database.py
index 6c56872a8..d3f0f52fe 100644
--- a/OpenOversight/app/models/database.py
+++ b/OpenOversight/app/models/database.py
@@ -3,8 +3,7 @@
 from datetime import date
 
 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
@@ -13,13 +12,13 @@
 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,
-    HOUR,
-    KEY_TOTAL_ASSIGNMENTS,
-    KEY_TOTAL_INCIDENTS,
-    KEY_TOTAL_OFFICERS,
+    KEY_DEPT_TOTAL_ASSIGNMENTS,
+    KEY_DEPT_TOTAL_INCIDENTS,
+    KEY_DEPT_TOTAL_OFFICERS,
 )
 from OpenOversight.app.validators import state_validator, url_validator
 
@@ -27,7 +26,7 @@
 db = SQLAlchemy()
 jwt = JsonWebToken("HS512")
 
-BaseModel = db.Model  # type: DefaultMeta
+BaseModel = db.Model
 
 officer_links = db.Table(
     "officer_links",
@@ -57,30 +56,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
-DATABASE_CACHE = TTLCache(maxsize=1024, ttl=12 * HOUR)
-
-
-# TODO: In the singleton create functions for other model types.
-def _date_updated_cache_key(update_type: str):
-    """Return a key function to calculate the cache key for Department
-    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 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"
@@ -117,7 +92,7 @@ def to_custom_dict(self):
             "unique_internal_identifier_label": self.unique_internal_identifier_label,
         }
 
-    @cached(cache=DATABASE_CACHE, key=_date_updated_cache_key(KEY_TOTAL_ASSIGNMENTS))
+    @cached(cache=DB_CACHE, key=db_model_cache_key(KEY_DEPT_TOTAL_ASSIGNMENTS))
     def total_documented_assignments(self):
         return (
             db.session.query(Assignment.id)
@@ -126,13 +101,13 @@ def total_documented_assignments(self):
             .count()
         )
 
-    @cached(cache=DATABASE_CACHE, key=_date_updated_cache_key(KEY_TOTAL_INCIDENTS))
+    @cached(cache=DB_CACHE, key=db_model_cache_key(KEY_DEPT_TOTAL_INCIDENTS))
     def total_documented_incidents(self):
         return (
             db.session.query(Incident).filter(Incident.department_id == self.id).count()
         )
 
-    @cached(cache=DATABASE_CACHE, key=_date_updated_cache_key(KEY_TOTAL_OFFICERS))
+    @cached(cache=DB_CACHE, key=db_model_cache_key(KEY_DEPT_TOTAL_OFFICERS))
     def total_documented_officers(self):
         return (
             db.session.query(Officer).filter(Officer.department_id == self.id).count()
diff --git a/OpenOversight/app/models/database_cache.py b/OpenOversight/app/models/database_cache.py
new file mode 100644
index 000000000..3200dc360
--- /dev/null
+++ b/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]
diff --git a/OpenOversight/app/templates/sort.html b/OpenOversight/app/templates/sort.html
index 4bf6dcac3..7687f9427 100644
--- a/OpenOversight/app/templates/sort.html
+++ b/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"
diff --git a/OpenOversight/app/utils/constants.py b/OpenOversight/app/utils/constants.py
index ba1b33e20..316fd6471 100644
--- a/OpenOversight/app/utils/constants.py
+++ b/OpenOversight/app/utils/constants.py
@@ -2,9 +2,9 @@
 
 
 # Cache Key Constants
-KEY_TOTAL_ASSIGNMENTS = "total_assignments"
-KEY_TOTAL_INCIDENTS = "total_incidents"
-KEY_TOTAL_OFFICERS = "total_officers"
+KEY_DEPT_TOTAL_ASSIGNMENTS = "total_assignments"
+KEY_DEPT_TOTAL_INCIDENTS = "total_incidents"
+KEY_DEPT_TOTAL_OFFICERS = "total_officers"
 
 # Config Key Constants
 KEY_DATABASE_URI = "SQLALCHEMY_DATABASE_URI"
diff --git a/OpenOversight/tests/conftest.py b/OpenOversight/tests/conftest.py
index 90f685a72..22151e299 100644
--- a/OpenOversight/tests/conftest.py
+++ b/OpenOversight/tests/conftest.py
@@ -87,7 +87,7 @@ def __init__(
     ("IVANA", "", "TINKLE"),
     ("SEYMOUR", "", "BUTZ"),
     ("HAYWOOD", "U", "CUDDLEME"),
-    ("BEA", "", "PROBLEM"),
+    ("BEA", "", "O'PROBLEM"),
     ("URA", "", "SNOTBALL"),
     ("HUGH", "", "JASS"),
 ]
diff --git a/OpenOversight/tests/test_cache.py b/OpenOversight/tests/test_cache.py
new file mode 100644
index 000000000..92a9c1a54
--- /dev/null
+++ b/OpenOversight/tests/test_cache.py
@@ -0,0 +1,185 @@
+import random
+from datetime import date
+from http import HTTPStatus
+
+from flask import current_app, url_for
+
+from OpenOversight.app.main.forms import (
+    AddOfficerForm,
+    AssignmentForm,
+    IncidentForm,
+    LicensePlateForm,
+    LinkForm,
+    LocationForm,
+)
+from OpenOversight.app.models.database import Department, Incident, Job, Officer, User
+from OpenOversight.app.models.database_cache import (
+    DB_CACHE,
+    has_database_cache_entry,
+    model_key,
+)
+from OpenOversight.app.utils.choices import GENDER_CHOICES, RACE_CHOICES, STATE_CHOICES
+from OpenOversight.app.utils.constants import (
+    ENCODING_UTF_8,
+    KEY_DEPT_TOTAL_ASSIGNMENTS,
+    KEY_DEPT_TOTAL_INCIDENTS,
+    KEY_DEPT_TOTAL_OFFICERS,
+)
+from OpenOversight.app.utils.db import unit_choices
+from OpenOversight.tests.routes.route_helpers import login_admin, process_form_data
+
+
+def test_model_key(mockdata, faker):
+    """Test the model key generation with multiple Model inheriting classes."""
+    test_key = faker.uuid4()
+
+    test_officer = Officer(id=faker.random_number(digits=3))
+    test_officer_key = model_key(test_officer, test_key)
+    DB_CACHE[test_officer_key] = 1
+    assert has_database_cache_entry(test_officer, test_key)
+
+    test_department = Department(id=faker.random_number(digits=3))
+    test_department_key = model_key(test_department, test_key)
+    DB_CACHE[test_department_key] = 1
+    assert has_database_cache_entry(test_department, test_key)
+
+    test_incident = Incident(id=faker.random_number(digits=3))
+    test_incident_key = model_key(test_incident, test_key)
+    DB_CACHE[test_incident_key] = 1
+    assert has_database_cache_entry(test_incident, test_key)
+
+
+def test_total_documented_assignments(mockdata, client, faker):
+    with current_app.test_request_context():
+        login_admin(client)
+        department = Department.query.first()
+        department.total_documented_assignments()
+        department.total_documented_incidents()
+        department.total_documented_officers()
+
+        assert has_database_cache_entry(department, KEY_DEPT_TOTAL_ASSIGNMENTS) is True
+        assert has_database_cache_entry(department, KEY_DEPT_TOTAL_INCIDENTS) is True
+        assert has_database_cache_entry(department, KEY_DEPT_TOTAL_OFFICERS) is True
+
+        officer = Officer.query.first()
+        job = Job.query.filter_by(
+            department_id=officer.department_id, job_title="Police Officer"
+        ).one()
+        star_no = str(faker.random_number(digits=4))
+        form = AssignmentForm(
+            star_no=star_no,
+            job_title=job.id,
+            start_date=date(2019, 1, 1),
+            resign_date=date(2019, 12, 31),
+        )
+
+        rv = client.post(
+            url_for("main.add_assignment", officer_id=3),
+            data=form.data,
+            follow_redirects=True,
+        )
+
+        assert "Added new assignment" in rv.data.decode(ENCODING_UTF_8)
+        assert has_database_cache_entry(department, KEY_DEPT_TOTAL_ASSIGNMENTS) is False
+        assert has_database_cache_entry(department, KEY_DEPT_TOTAL_INCIDENTS) is True
+        assert has_database_cache_entry(department, KEY_DEPT_TOTAL_OFFICERS) is True
+
+
+def test_total_documented_incidents(mockdata, client, faker):
+    with current_app.test_request_context():
+        login_admin(client)
+        department = Department.query.first()
+        department.total_documented_assignments()
+        department.total_documented_incidents()
+        department.total_documented_officers()
+        user = User.query.first()
+
+        assert has_database_cache_entry(department, KEY_DEPT_TOTAL_ASSIGNMENTS) is True
+        assert has_database_cache_entry(department, KEY_DEPT_TOTAL_INCIDENTS) is True
+        assert has_database_cache_entry(department, KEY_DEPT_TOTAL_OFFICERS) is True
+
+        test_date = faker.date_time()
+
+        address_form = LocationForm(
+            street_name=faker.street_address(),
+            cross_street1=faker.street_address(),
+            city=faker.city(),
+            state=random.choice(STATE_CHOICES)[0],
+            zip_code=faker.postcode(),
+            created_by=user.id,
+        )
+        # These have to have a dropdown selected because if not, an empty Unicode
+        # string is sent, which does not mach the '' selector.
+        link_form = LinkForm(link_type="video", created_by=user.id)
+        license_plates_form = LicensePlateForm(state="AZ", created_by=user.id)
+        form = IncidentForm(
+            date_field=str(test_date.date()),
+            time_field=str(test_date.time()),
+            report_number="PPP Case 92",
+            description=faker.sentence(nb_words=15),
+            department=department.id,
+            address=address_form.data,
+            links=[link_form.data],
+            license_plates=[license_plates_form.data],
+            officers=[],
+            created_by=user.id,
+            last_updated_by=user.id,
+            last_updated_at=test_date,
+        )
+        data = process_form_data(form.data)
+
+        rv = client.post(
+            url_for("main.incident_api") + "new", data=data, follow_redirects=True
+        )
+
+        assert rv.status_code == HTTPStatus.OK
+        assert "created" in rv.data.decode(ENCODING_UTF_8)
+        assert has_database_cache_entry(department, KEY_DEPT_TOTAL_ASSIGNMENTS) is True
+        assert has_database_cache_entry(department, KEY_DEPT_TOTAL_INCIDENTS) is False
+        assert has_database_cache_entry(department, KEY_DEPT_TOTAL_OFFICERS) is True
+
+
+def test_total_documented_officers(mockdata, client, faker):
+    with current_app.test_request_context():
+        login_admin(client)
+        department = Department.query.first()
+        department.total_documented_assignments()
+        department.total_documented_incidents()
+        department.total_documented_officers()
+        user = User.query.filter_by(is_administrator=True).first()
+
+        assert has_database_cache_entry(department, KEY_DEPT_TOTAL_ASSIGNMENTS) is True
+        assert has_database_cache_entry(department, KEY_DEPT_TOTAL_INCIDENTS) is True
+        assert has_database_cache_entry(department, KEY_DEPT_TOTAL_OFFICERS) is True
+
+        links = [
+            LinkForm(url=faker.url(), link_type="link", created_by=user.id).data,
+            LinkForm(url=faker.url(), link_type="video", created_by=user.id).data,
+        ]
+        job = Job.query.filter_by(department_id=department.id).first()
+        last_name = faker.last_name()
+        form = AddOfficerForm(
+            first_name=faker.first_name(),
+            last_name=last_name,
+            middle_initial=faker.random_uppercase_letter(),
+            race=random.choice(RACE_CHOICES)[0],
+            gender=random.choice(GENDER_CHOICES)[0],
+            star_no=faker.random_number(digits=3),
+            job_id=job.id,
+            unit=random.choice(unit_choices()).id,
+            department=department.id,
+            birth_year=int(faker.year()),
+            links=links,
+            created_by=user.id,
+        )
+
+        rv = client.post(
+            url_for("main.add_officer"),
+            data=process_form_data(form.data),
+            follow_redirects=True,
+        )
+
+        assert f"New Officer {last_name} added" in rv.data.decode(ENCODING_UTF_8)
+        assert has_database_cache_entry(department, KEY_DEPT_TOTAL_ASSIGNMENTS) is True
+        assert has_database_cache_entry(department, KEY_DEPT_TOTAL_INCIDENTS) is True
+        assert has_database_cache_entry(department, KEY_DEPT_TOTAL_OFFICERS) is False
diff --git a/OpenOversight/tests/test_functional.py b/OpenOversight/tests/test_functional.py
index 2356eea82..aa93483a8 100644
--- a/OpenOversight/tests/test_functional.py
+++ b/OpenOversight/tests/test_functional.py
@@ -362,7 +362,7 @@ def test_image_classification_and_tagging(mockdata, browser, server_port):
 
     wait_for_page_load(browser)
     page_text = browser.find_element(By.TAG_NAME, "body").text
-    assert "All images have been classfied!" in page_text
+    assert "All images have been classified!" in page_text
 
     # 5. Identify the new officer in the uploaded image
     browser.get(f"http://localhost:{server_port}/cop_face/department/{dept_id}")