Skip to content

Commit

Permalink
Create generic DB_CACHE functions for db.Model classes (#1011)
Browse files Browse the repository at this point in the history
## Fixes issue
#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.
  • Loading branch information
michplunkett authored Aug 7, 2023
1 parent 6c08a17 commit 32eec2c
Show file tree
Hide file tree
Showing 9 changed files with 260 additions and 42 deletions.
9 changes: 7 additions & 2 deletions OpenOversight/app/main/model_view.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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()
Expand All @@ -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:
Expand Down
9 changes: 9 additions & 0 deletions OpenOversight/app/main/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
)
Expand Down Expand Up @@ -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!")
Expand Down Expand Up @@ -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:
Expand Down
43 changes: 9 additions & 34 deletions OpenOversight/app/models/database.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -13,21 +12,21 @@
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


db = SQLAlchemy()
jwt = JsonWebToken("HS512")

BaseModel = db.Model # type: DefaultMeta
BaseModel = db.Model

officer_links = db.Table(
"officer_links",
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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)
Expand All @@ -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()
Expand Down
44 changes: 44 additions & 0 deletions OpenOversight/app/models/database_cache.py
Original file line number Diff line number Diff line change
@@ -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]
2 changes: 1 addition & 1 deletion OpenOversight/app/templates/sort.html
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
6 changes: 3 additions & 3 deletions OpenOversight/app/utils/constants.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
2 changes: 1 addition & 1 deletion OpenOversight/tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ def __init__(
("IVANA", "", "TINKLE"),
("SEYMOUR", "", "BUTZ"),
("HAYWOOD", "U", "CUDDLEME"),
("BEA", "", "PROBLEM"),
("BEA", "", "O'PROBLEM"),
("URA", "", "SNOTBALL"),
("HUGH", "", "JASS"),
]
Expand Down
Loading

0 comments on commit 32eec2c

Please sign in to comment.