From a4ea5329fd5878d046da6f40d942013357200d82 Mon Sep 17 00:00:00 2001 From: Michele-Alberti <62114934+Michele-Alberti@users.noreply.github.com> Date: Tue, 2 Jan 2024 18:12:34 +0100 Subject: [PATCH] fix(guest_override-cache): move guest override from cache to db and collect some sql related calls inside models.py's classes guest override flag is now managed with models.get_flag and models.set_flag instead of using the state cache state cache is not reliable when multiple containers serve the app simultaneously (every container has a different panel.state.cache object add the optional parameter value_if_missing to models.get_flag to set a default value instead of returning None add a clean method and a read_as_df method to every table object (see models.py) to improve code readability add a clear_guest_override method to the class models.Flags to clean app every guest_override flag (those are saved on a per-user basis) add InvalidToken error handling in auth (this only happen if DATA_LUNCH_OAUTH_ENC_KEY changes but the guest user password is not updated for basic auth rename 'cache' to 'flags' in variables and properties for gui.py --- data_lunch_app/__init__.py | 21 +++- data_lunch_app/auth.py | 22 ++-- data_lunch_app/core.py | 72 ++++++------ data_lunch_app/gui.py | 60 +++++----- data_lunch_app/models.py | 218 ++++++++++++++++++++++++++++++++++++- 5 files changed, 313 insertions(+), 80 deletions(-) diff --git a/data_lunch_app/__init__.py b/data_lunch_app/__init__.py index cda0cb1..7eb01b7 100755 --- a/data_lunch_app/__init__.py +++ b/data_lunch_app/__init__.py @@ -42,10 +42,17 @@ def create_app(config: DictConfig) -> pn.Template: # Set the no_more_orders flag if it is None (not found in flags table) if models.get_flag(config=config, id="no_more_orders") is None: models.set_flag(config=config, id="no_more_orders", value=False) - # Set guest override if it is empty - guest_override = pn.state.cache.get( - f"{pn.state.user}_guest_override", False - ) + # Set guest override flag if it is None (not found in flags table) + # Guest override flag is per-user and is not set for guests + if ( + models.get_flag(config=config, id=f"{pn.state.user}_guest_override") + is None + ) and not auth.is_guest( + user=pn.state.user, config=config, allow_override=False + ): + models.set_flag( + config=config, id=f"{pn.state.user}_guest_override", value=False + ) # DASHBOARD BASE TEMPLATE log.debug("instantiate base template") @@ -95,7 +102,11 @@ def create_app(config: DictConfig) -> pn.Template: reload=False, ) gi.reload_on_guest_override( - toggle=guest_override, + toggle=models.get_flag( + config=config, + id=f"{pn.state.user}_guest_override", + value_if_missing=False, + ), reload=False, ) core.reload_menu( diff --git a/data_lunch_app/auth.py b/data_lunch_app/auth.py index c129165..d92942f 100644 --- a/data_lunch_app/auth.py +++ b/data_lunch_app/auth.py @@ -433,20 +433,14 @@ def list_users_guests_and_privileges(config: DictConfig) -> pd.DataFrame: """Join privileged users' table and credentials tables to list users, admins and guests. Returns a dataframe.""" - # Create session - engine = models.create_engine(config) # Query tables required to understand users and guests - df_privileged_users = pd.read_sql_table( - models.PrivilegedUsers.__tablename__, - engine, - schema=config.db.get("schema", None), + df_privileged_users = models.PrivilegedUsers.read_as_df( + config=config, index_col="user", ) - df_credentials = pd.read_sql_table( - models.Credentials.__tablename__, - engine, - schema=config.db.get("schema", None), + df_credentials = models.Credentials.read_as_df( + config=config, index_col="user", ) # Change admin column to privileges (used after join) @@ -468,10 +462,12 @@ def is_guest( The guest override chached value (per-user) can force the function to always return True. If allow_override is set to False the guest override value is ignored.""" - # Load guest override from state cache (if the button is pressed its value + # Load guest override from flag table (if the button is pressed its value # is True). If not available use False. - guest_override = pn.state.cache.get( - f"{pn.state.user}_guest_override", False + guest_override = models.get_flag( + config=config, + id=f"{pn.state.user}_guest_override", + value_if_missing=False, ) # If guest override is active always return true (user act like guest) diff --git a/data_lunch_app/core.py b/data_lunch_app/core.py index e28aa5d..4735346 100644 --- a/data_lunch_app/core.py +++ b/data_lunch_app/core.py @@ -22,6 +22,7 @@ # Authentication from . import auth +import cryptography.fernet # LOGGER ---------------------------------------------------------------------- log = logging.getLogger(__name__) @@ -67,26 +68,20 @@ def delete_files(config: DictConfig): def clean_tables(config: DictConfig): # Clean tables - session = models.create_session(config) - with session: - # Clean orders - num_rows_deleted = session.execute(delete(models.Orders)) - session.commit() - log.info(f"deleted {num_rows_deleted.rowcount} from table 'orders'") - # Clean menu - num_rows_deleted = session.execute(delete(models.Menu)) - session.commit() - log.info(f"deleted {num_rows_deleted.rowcount} from table 'menu'") - # Clean users - num_rows_deleted = session.execute(delete(models.Users)) - session.commit() - log.info(f"deleted {num_rows_deleted.rowcount} from table 'users'") - # Clean flags - models.set_flag(config=config, id="no_more_orders", value=False) - log.info("reset values in table 'flags'") - # Clean cache - pn.state.clear_caches() - log.info("cache cleaned") + # Clean orders + models.Orders.clear(config=config) + # Clean menu + models.Menu.clear(config=config) + # Clean users + models.Users.clear(config=config) + # Clean flags + models.Flags.clear_guest_override(config=config) + # Reset flags + models.set_flag(config=config, id="no_more_orders", value=False) + log.info("reset values in table 'flags'") + # Clean cache + pn.state.clear_caches() + log.info("cache cleaned") def set_guest_user_password(config: DictConfig) -> str: @@ -130,9 +125,20 @@ def set_guest_user_password(config: DictConfig) -> str: # Load from database session = models.create_session(config) with session: - guest_password = session.get( - models.Credentials, "guest" - ).password_encrypted.decrypt() + try: + guest_password = session.get( + models.Credentials, "guest" + ).password_encrypted.decrypt() + except cryptography.fernet.InvalidToken: + # Notify exception and suggest to reset guest user password + guest_password = "" + log.warning( + "Unable to decrypt 'guest' user password because an invalid token has been detected: reset password from backend" + ) + pn.state.notifications.warning( + "Unable to decrypt 'guest' user password
Invalid token detected: reset password from backend", + duration=config.panel.notifications.duration, + ) else: guest_password = "" @@ -290,9 +296,11 @@ def reload_menu( return - # Check guest override button status - gi.toggle_guest_override_button.value = pn.state.cache.get( - f"{pn.state.user}_guest_override", False + # Check guest override button status (if not in table use False) + gi.toggle_guest_override_button.value = models.get_flag( + config=config, + id=f"{pn.state.user}_guest_override", + value_if_missing=False, ) # Guest graphic configuration @@ -307,10 +315,8 @@ def reload_menu( # Reload menu engine = models.create_engine(config) - df = pd.read_sql_table( - models.Menu.__tablename__, - engine, - schema=config.db.get("schema", models.SCHEMA), + df = models.Menu.read_as_df( + config=config, index_col="id", ) df["order"] = False @@ -772,10 +778,8 @@ def df_list_by_lunch_time( # Create database engine and session engine = models.create_engine(config) # Read menu and save how menu items are sorted (first courses, second courses, etc.) - original_order = pd.read_sql_table( - models.Menu.__tablename__, - engine, - schema=config.db.get("schema", models.SCHEMA), + original_order = models.Menu.read_as_df( + config=config, index_col="id", ).item # Create session diff --git a/data_lunch_app/gui.py b/data_lunch_app/gui.py index cd68f91..7209c36 100644 --- a/data_lunch_app/gui.py +++ b/data_lunch_app/gui.py @@ -227,12 +227,16 @@ def reload_on_guest_override_callback( toggle: pnw.ToggleIcon, reload: bool = True ): # Update global variable that control guest override - # Only non guest can store this value in cache (guest users are - # always guests) + # Only non guest can store this value in 'flags' table (guest users + # are always guests, there is no use in sotring a flag for them) if not auth.is_guest( user=pn.state.user, config=config, allow_override=False ): - pn.state.cache[f"{pn.state.user}_guest_override"] = toggle + models.set_flag( + config=config, + id=f"{pn.state.user}_guest_override", + value=toggle, + ) # Show banner if override is active self.guest_override_alert.visible = toggle # Simply reload the menu when the toggle button value changes @@ -905,13 +909,12 @@ def __init__( self.users_tabulator = pn.widgets.Tabulator( value=auth.list_users_guests_and_privileges(config), ) - # Cache content - self.cache_content = pn.widgets.JSONEditor( - value=pn.state.cache, - name="Cache Content", - mode="view", - sizing_mode="stretch_both", + # Flags content (use empty dataframe to instantiate) + df_flags = models.Flags.read_as_df( + config=config, + index_col="id", ) + self.flags_content = pn.widgets.Tabulator(value=df_flags) # BUTTONS # Exit button @@ -942,9 +945,9 @@ def __init__( icon_size="2em", sizing_mode="stretch_width", ) - # Clear cache button - self.clear_cache_button = pnw.Button( - name="Clear Server Cache", + # Clear flags table button + self.clear_flags_button = pnw.Button( + name="Clear Guest Override Flags", button_type="danger", height=generic_button_height, icon="file-shredder", @@ -976,11 +979,11 @@ def __init__( self.delete_user_button, width=sidebar_width, ) - self.clear_cache_column = pn.Column( - pn.pane.HTML("Cache Content"), - self.cache_content, + self.clear_flags_column = pn.Column( + pn.pane.HTML("Flags Table Content"), + self.flags_content, pn.VSpacer(), - self.clear_cache_button, + self.clear_flags_button, width=sidebar_width, ) # Create for deleting users @@ -1022,7 +1025,7 @@ def __init__( sizing_mode="stretch_height", ) ) - self.backend_controls.append(self.clear_cache_column) + self.backend_controls.append(self.clear_flags_column) self.backend_controls.append( pn.pane.HTML( styles=dict(background="lightgray"), @@ -1089,27 +1092,34 @@ def delete_user_button_callback(self): lambda e: delete_user_button_callback(self) ) - # Clear cache callback - def clear_cache_button_callback(self): - pn.state.clear_caches() - pn.state.cache = {} + # Clear flags callback + def clear_flags_button_callback(self): + # Clear flags + num_rows_deleted = models.Flags.clear_guest_override(config=config) + # Reload and notify user self.reload_backend(config) pn.state.notifications.success( - "Cache cleared", + f"Guest override flags cleared
{num_rows_deleted} rows deleted", duration=config.panel.notifications.duration, ) - self.clear_cache_button.on_click( - lambda e: clear_cache_button_callback(self) + self.clear_flags_button.on_click( + lambda e: clear_flags_button_callback(self) ) # UTILITY FUNCTIONS # MAIN SECTION def reload_backend(self, config) -> None: + # Users and guests lists self.users_tabulator.value = auth.list_users_guests_and_privileges( config ) - self.cache_content.value = pn.state.cache + # Flags table content + df_flags = models.Flags.read_as_df( + config=config, + index_col="id", + ) + self.flags_content.value = df_flags def exit_backend(self) -> None: # Edit pathname to force exit diff --git a/data_lunch_app/models.py b/data_lunch_app/models.py index dec4f1c..c7ca070 100755 --- a/data_lunch_app/models.py +++ b/data_lunch_app/models.py @@ -3,6 +3,7 @@ import logging from omegaconf import DictConfig import pathlib +import pandas as pd from psycopg import Connection as ConnectionPostgresql from sqlite3 import Connection as ConnectionSqlite from sqlalchemy import ( @@ -16,6 +17,7 @@ Boolean, event, MetaData, + delete, ) from sqlalchemy.engine import Engine from sqlalchemy.exc import OperationalError @@ -169,6 +171,33 @@ class Menu(Data): passive_deletes=True, ) + @classmethod + def clear(self, config: DictConfig) -> int: + """Clear table and return deleted rows""" + + session = create_session(config) + with session: + # Clean menu + num_rows_deleted = session.execute(delete(self)) + session.commit() + log.info( + f"deleted {num_rows_deleted.rowcount} rows from table '{self.__tablename__}'" + ) + + return num_rows_deleted.rowcount + + @classmethod + def read_as_df(self, config: DictConfig, **kwargs) -> pd.DataFrame: + """Read table as pandas DataFrame""" + df = pd.read_sql_table( + self.__tablename__, + create_engine(config=config), + schema=config.db.get("schema", SCHEMA), + **kwargs, + ) + + return df + def __repr__(self): return f"" @@ -191,6 +220,33 @@ class Orders(Data): menu_item = relationship("Menu", back_populates="orders") note = relationship("Users", back_populates="orders", uselist=False) + @classmethod + def clear(self, config: DictConfig) -> int: + """Clear table and return deleted rows""" + + session = create_session(config) + with session: + # Clean menu + num_rows_deleted = session.execute(delete(self)) + session.commit() + log.info( + f"deleted {num_rows_deleted.rowcount} rows from table '{self.__tablename__}'" + ) + + return num_rows_deleted.rowcount + + @classmethod + def read_as_df(self, config: DictConfig, **kwargs) -> pd.DataFrame: + """Read table as pandas DataFrame""" + df = pd.read_sql_table( + self.__tablename__, + create_engine(config=config), + schema=config.db.get("schema", SCHEMA), + **kwargs, + ) + + return df + def __repr__(self): return f"" @@ -219,6 +275,33 @@ class Users(Data): passive_deletes=True, ) + @classmethod + def clear(self, config: DictConfig) -> int: + """Clear table and return deleted rows""" + + session = create_session(config) + with session: + # Clean menu + num_rows_deleted = session.execute(delete(self)) + session.commit() + log.info( + f"deleted {num_rows_deleted.rowcount} rows from table '{self.__tablename__}'" + ) + + return num_rows_deleted.rowcount + + @classmethod + def read_as_df(self, config: DictConfig, **kwargs) -> pd.DataFrame: + """Read table as pandas DataFrame""" + df = pd.read_sql_table( + self.__tablename__, + create_engine(config=config), + schema=config.db.get("schema", SCHEMA), + **kwargs, + ) + + return df + def __repr__(self): return f"" @@ -248,6 +331,33 @@ class Stats(Data): Integer, nullable=False, default=0, server_default="0" ) + @classmethod + def clear(self, config: DictConfig) -> int: + """Clear table and return deleted rows""" + + session = create_session(config) + with session: + # Clean menu + num_rows_deleted = session.execute(delete(self)) + session.commit() + log.info( + f"deleted {num_rows_deleted.rowcount} rows from table '{self.__tablename__}'" + ) + + return num_rows_deleted.rowcount + + @classmethod + def read_as_df(self, config: DictConfig, **kwargs) -> pd.DataFrame: + """Read table as pandas DataFrame""" + df = pd.read_sql_table( + self.__tablename__, + create_engine(config=config), + schema=config.db.get("schema", SCHEMA), + **kwargs, + ) + + return df + def __repr__(self): return f"" @@ -262,6 +372,50 @@ class Flags(Data): ) value = Column(Boolean, nullable=False) + @classmethod + def clear(self, config: DictConfig) -> int: + """Clear table and return deleted rows""" + + session = create_session(config) + with session: + # Clean menu + num_rows_deleted = session.execute(delete(self)) + session.commit() + log.info( + f"deleted {num_rows_deleted.rowcount} rows from table '{self.__tablename__}'" + ) + + return num_rows_deleted.rowcount + + @classmethod + def clear_guest_override(self, config: DictConfig) -> int: + """Clear 'guest_override' flags and return deleted rows""" + + session = create_session(config) + with session: + # Clean menu + num_rows_deleted = session.execute( + delete(self).where(self.id.like("%_guest_override")) + ) + session.commit() + log.info( + f"deleted {num_rows_deleted.rowcount} rows (guest override) from table '{self.__tablename__}'" + ) + + return num_rows_deleted.rowcount + + @classmethod + def read_as_df(self, config: DictConfig, **kwargs) -> pd.DataFrame: + """Read table as pandas DataFrame""" + df = pd.read_sql_table( + self.__tablename__, + create_engine(config=config), + schema=config.db.get("schema", SCHEMA), + **kwargs, + ) + + return df + def __repr__(self): return f"" @@ -278,6 +432,33 @@ class PrivilegedUsers(Data): Boolean, nullable=False, default=False, server_default=sql_false() ) + @classmethod + def clear(self, config: DictConfig) -> int: + """Clear table and return deleted rows""" + + session = create_session(config) + with session: + # Clean menu + num_rows_deleted = session.execute(delete(self)) + session.commit() + log.info( + f"deleted {num_rows_deleted.rowcount} rows from table '{self.__tablename__}'" + ) + + return num_rows_deleted.rowcount + + @classmethod + def read_as_df(self, config: DictConfig, **kwargs) -> pd.DataFrame: + """Read table as pandas DataFrame""" + df = pd.read_sql_table( + self.__tablename__, + create_engine(config=config), + schema=config.db.get("schema", SCHEMA), + **kwargs, + ) + + return df + def __repr__(self): return f"" @@ -298,6 +479,33 @@ class Credentials(Data): server_default=None, ) + @classmethod + def clear(self, config: DictConfig) -> int: + """Clear table and return deleted rows""" + + session = create_session(config) + with session: + # Clean menu + num_rows_deleted = session.execute(delete(self)) + session.commit() + log.info( + f"deleted {num_rows_deleted.rowcount} rows from table '{self.__tablename__}'" + ) + + return num_rows_deleted.rowcount + + @classmethod + def read_as_df(self, config: DictConfig, **kwargs) -> pd.DataFrame: + """Read table as pandas DataFrame""" + df = pd.read_sql_table( + self.__tablename__, + create_engine(config=config), + schema=config.db.get("schema", SCHEMA), + **kwargs, + ) + + return df + def __repr__(self): return f"" @@ -486,13 +694,17 @@ def set_flag(config: DictConfig, id: str, value: bool) -> None: session.commit() -def get_flag(config: DictConfig, id: str) -> bool | None: - """Get the value of a flag""" +def get_flag( + config: DictConfig, id: str, value_if_missing: bool | None = None +) -> bool | None: + """Get the value of a flag. + Optionally select the values to return if the flag is missing (default to None). + """ session = create_session(config) flag = session.get(Flags, id) if flag is None: - value = None + value = value_if_missing else: value = flag.value return value