Skip to content
Snippets Groups Projects
Commit be364ef4 authored by Benjamin Bertrand's avatar Benjamin Bertrand
Browse files

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
parent d2bb6124
No related branches found
No related tags found
No related merge requests found
...@@ -14,7 +14,6 @@ from flask_admin.contrib import sqla ...@@ -14,7 +14,6 @@ from flask_admin.contrib import sqla
from flask_admin.model.form import converts from flask_admin.model.form import converts
from flask_login import current_user from flask_login import current_user
from ..validators import IPNetwork, ICS_ID_RE from ..validators import IPNetwork, ICS_ID_RE
from .. import models
# Monkey patch flask-admin Unique validator to disable it # Monkey patch flask-admin Unique validator to disable it
...@@ -48,16 +47,6 @@ class AdminModelView(sqla.ModelView): ...@@ -48,16 +47,6 @@ class AdminModelView(sqla.ModelView):
def is_accessible(self): def is_accessible(self):
return current_user.is_authenticated and current_user.is_admin 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): class UserAdmin(AdminModelView):
can_create = False can_create = False
......
...@@ -210,10 +210,6 @@ def attributes_favorites(): ...@@ -210,10 +210,6 @@ def attributes_favorites():
@bp.route("/_retrieve_attributes_favorites") @bp.route("/_retrieve_attributes_favorites")
@login_required @login_required
def retrieve_attributes_favorites(): 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 = [ data = [
( (
favorite.base64_image(), favorite.base64_image(),
...@@ -275,10 +271,6 @@ def update_favorites(kind): ...@@ -275,10 +271,6 @@ def update_favorites(kind):
Add or remove the attribute from the favorites when the Add or remove the attribute from the favorites when the
checkbox is checked/unchecked in the attributes table 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: try:
model = getattr(models, kind) model = getattr(models, kind)
except AttributeError: except AttributeError:
......
...@@ -83,7 +83,6 @@ def get_temporary_ics_id(): ...@@ -83,7 +83,6 @@ def get_temporary_ics_id():
@login_manager.user_loader @login_manager.user_loader
@cache.memoize(timeout=1800)
def load_user(user_id): def load_user(user_id):
"""User loader callback for flask-login """User loader callback for flask-login
......
...@@ -22,8 +22,6 @@ from flask import ( ...@@ -22,8 +22,6 @@ from flask import (
from flask_login import login_user, logout_user, login_required, current_user from flask_login import login_user, logout_user, login_required, current_user
from flask_ldap3_login.forms import LDAPLoginForm from flask_ldap3_login.forms import LDAPLoginForm
from .forms import TokenForm from .forms import TokenForm
from ..extensions import cache, db
from ..models import load_user
from .. import tokens, utils from .. import tokens, utils
bp = Blueprint("user", __name__) bp = Blueprint("user", __name__)
...@@ -41,8 +39,6 @@ def login(): ...@@ -41,8 +39,6 @@ def login():
@bp.route("/logout") @bp.route("/logout")
@login_required @login_required
def logout(): def logout():
# Don't forget to remove the user from the cache
cache.delete_memoized(load_user, str(current_user.id))
logout_user() logout_user()
return redirect(url_for("user.login")) return redirect(url_for("user.login"))
...@@ -50,10 +46,6 @@ def logout(): ...@@ -50,10 +46,6 @@ def logout():
@bp.route("/profile", methods=["GET", "POST"]) @bp.route("/profile", methods=["GET", "POST"])
@login_required @login_required
def profile(): 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 # Try to get the generated token from the session
token = session.pop("generated_token", None) token = session.pop("generated_token", None)
form = TokenForm(request.form) form = TokenForm(request.form)
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment