From be364ef4b44997ed8806a17cc6fb60eeb6509feb Mon Sep 17 00:00:00 2001 From: Benjamin Bertrand <benjamin.bertrand@esss.se> Date: Mon, 13 May 2019 10:26:35 +0200 Subject: [PATCH] Remove caching of user retrieval The caching of the load_user function already created several bugs (INFRA 908/1018). It requires to think about adding (or deleting in the admin view) the current_user to the session. This is easy to miss. It saves some database queries but we don't need this optimization. Performances are not impacted and are good enough so far. Removing the caching makes the application more robust and easier to maintain. JIRA INFRA-1025 #action In Progress --- app/admin/views.py | 11 ----------- app/inventory/views.py | 8 -------- app/models.py | 1 - app/user/views.py | 8 -------- 4 files changed, 28 deletions(-) diff --git a/app/admin/views.py b/app/admin/views.py index e954918..5839cab 100644 --- a/app/admin/views.py +++ b/app/admin/views.py @@ -14,7 +14,6 @@ from flask_admin.contrib import sqla from flask_admin.model.form import converts from flask_login import current_user from ..validators import IPNetwork, ICS_ID_RE -from .. import models # Monkey patch flask-admin Unique validator to disable it @@ -48,16 +47,6 @@ class AdminModelView(sqla.ModelView): def is_accessible(self): return current_user.is_authenticated and current_user.is_admin - def update_model(self, form, model): - # Remove the current user object added by flask-admin from the session - # Adding the current_user object later raises an exception otherwise: - # sqlalchemy.exc.InvalidRequestError: Can't attach instance <User at 0x7f6f0e39ce10>; - # another instance with key (<class 'app.models.User'>, (1,), None) is already present in this session. - for obj in self.session: - if isinstance(obj, models.User) and obj.id == current_user.id: - self.session.expunge(obj) - return super().update_model(form, model) - class UserAdmin(AdminModelView): can_create = False diff --git a/app/inventory/views.py b/app/inventory/views.py index c5ff9b6..25a82ad 100644 --- a/app/inventory/views.py +++ b/app/inventory/views.py @@ -210,10 +210,6 @@ def attributes_favorites(): @bp.route("/_retrieve_attributes_favorites") @login_required def retrieve_attributes_favorites(): - if current_user not in db.session: - # If the current user is cached, it won't be in the sqlalchemy session - # Add it to access the user favorite attributes relationship - db.session.add(current_user) data = [ ( favorite.base64_image(), @@ -275,10 +271,6 @@ def update_favorites(kind): Add or remove the attribute from the favorites when the checkbox is checked/unchecked in the attributes table """ - if current_user not in db.session: - # If the current user is cached, it won't be in the sqlalchemy session - # Add it to access the user favorite attributes relationship - db.session.add(current_user) try: model = getattr(models, kind) except AttributeError: diff --git a/app/models.py b/app/models.py index 12b9ebe..ed5d6ed 100644 --- a/app/models.py +++ b/app/models.py @@ -83,7 +83,6 @@ def get_temporary_ics_id(): @login_manager.user_loader -@cache.memoize(timeout=1800) def load_user(user_id): """User loader callback for flask-login diff --git a/app/user/views.py b/app/user/views.py index c85f5b0..9dcae10 100644 --- a/app/user/views.py +++ b/app/user/views.py @@ -22,8 +22,6 @@ from flask import ( from flask_login import login_user, logout_user, login_required, current_user from flask_ldap3_login.forms import LDAPLoginForm from .forms import TokenForm -from ..extensions import cache, db -from ..models import load_user from .. import tokens, utils bp = Blueprint("user", __name__) @@ -41,8 +39,6 @@ def login(): @bp.route("/logout") @login_required def logout(): - # Don't forget to remove the user from the cache - cache.delete_memoized(load_user, str(current_user.id)) logout_user() return redirect(url_for("user.login")) @@ -50,10 +46,6 @@ def logout(): @bp.route("/profile", methods=["GET", "POST"]) @login_required def profile(): - if current_user not in db.session: - # If the current user is cached, it won't be in the sqlalchemy session - # Add it to access the user.tokens relationship in the template - db.session.add(current_user) # Try to get the generated token from the session token = session.pop("generated_token", None) form = TokenForm(request.form) -- GitLab