From 5e3618716d2dd7ea71ecd041c5685e0fdb4bbc27 Mon Sep 17 00:00:00 2001 From: Ozzieisaacs Date: Mon, 7 Dec 2020 19:53:34 +0100 Subject: [PATCH] Fix missing session rollback on commit error --- cps/admin.py | 95 +++++++++++++++++++++++++++++++++++++---------- cps/config_sql.py | 34 +++++++++++++---- cps/kobo.py | 42 ++++++++++++++++----- cps/kobo_auth.py | 11 +++++- cps/oauth_bb.py | 11 +++++- cps/shelf.py | 7 +++- cps/ub.py | 14 +++++-- cps/web.py | 56 +++++++++++++++++++--------- 8 files changed, 208 insertions(+), 62 deletions(-) diff --git a/cps/admin.py b/cps/admin.py index f0a82e77..9b90a74e 100644 --- a/cps/admin.py +++ b/cps/admin.py @@ -206,7 +206,10 @@ def edit_domain(allow): vals = request.form.to_dict() answer = g.ubsession.query(ub.Registration).filter(ub.Registration.id == vals['pk']).first() answer.domain = vals['value'].replace('*', '%').replace('?', '_').lower() - g.ubsession.commit() + try: + g.ubsession.commit() + except OperationalError: + g.ubsession.rollback() return "" @@ -220,7 +223,10 @@ def add_domain(allow): if not check: new_domain = ub.Registration(domain=domain_name, allow=allow) g.ubsession.add(new_domain) - g.ubsession.commit() + try: + g.ubsession.commit() + except OperationalError: + g.ubsession.rollback() return "" @@ -230,12 +236,18 @@ def add_domain(allow): def delete_domain(): domain_id = request.form.to_dict()['domainid'].replace('*', '%').replace('?', '_').lower() g.ubsession.query(ub.Registration).filter(ub.Registration.id == domain_id).delete() - g.ubsession.commit() + try: + g.ubsession.commit() + except OperationalError: + g.ubsession.rollback() # If last domain was deleted, add all domains by default if not g.ubsession.query(ub.Registration).filter(ub.Registration.allow==1).count(): new_domain = ub.Registration(domain="%.%",allow=1) g.ubsession.add(new_domain) - g.ubsession.commit() + try: + g.ubsession.commit() + except OperationalError: + g.ubsession.rollback() return "" @@ -275,7 +287,10 @@ def edit_restriction(res_type): elementlist = usr.list_allowed_tags() elementlist[int(element['id'][1:])]=element['Element'] usr.allowed_tags = ','.join(elementlist) - g.ubsession.commit() + try: + g.ubsession.commit() + except OperationalError: + g.ubsession.rollback() if res_type == 3: # CColumn per user usr_id = os.path.split(request.referrer)[-1] if usr_id.isdigit() == True: @@ -285,7 +300,10 @@ def edit_restriction(res_type): elementlist = usr.list_allowed_column_values() elementlist[int(element['id'][1:])]=element['Element'] usr.allowed_column_value = ','.join(elementlist) - g.ubsession.commit() + try: + g.ubsession.commit() + except OperationalError: + g.ubsession.rollback() if element['id'].startswith('d'): if res_type == 0: # Tags as template elementlist = config.list_denied_tags() @@ -306,7 +324,10 @@ def edit_restriction(res_type): elementlist = usr.list_denied_tags() elementlist[int(element['id'][1:])]=element['Element'] usr.denied_tags = ','.join(elementlist) - g.ubsession.commit() + try: + g.ubsession.commit() + except OperationalError: + g.ubsession.rollback() if res_type == 3: # CColumn per user usr_id = os.path.split(request.referrer)[-1] if usr_id.isdigit() == True: @@ -316,7 +337,10 @@ def edit_restriction(res_type): elementlist = usr.list_denied_column_values() elementlist[int(element['id'][1:])]=element['Element'] usr.denied_column_value = ','.join(elementlist) - g.ubsession.commit() + try: + g.ubsession.commit() + except OperationalError: + g.ubsession.rollback() return "" def restriction_addition(element, list_func): @@ -362,10 +386,16 @@ def add_restriction(res_type): usr = current_user if 'submit_allow' in element: usr.allowed_tags = restriction_addition(element, usr.list_allowed_tags) - g.ubsession.commit() + try: + g.ubsession.commit() + except OperationalError: + g.ubsession.rollback() elif 'submit_deny' in element: usr.denied_tags = restriction_addition(element, usr.list_denied_tags) - g.ubsession.commit() + try: + g.ubsession.commit() + except OperationalError: + g.ubsession.rollback() if res_type == 3: # CustomC per user usr_id = os.path.split(request.referrer)[-1] if usr_id.isdigit() == True: @@ -374,10 +404,16 @@ def add_restriction(res_type): usr = current_user if 'submit_allow' in element: usr.allowed_column_value = restriction_addition(element, usr.list_allowed_column_values) - g.ubsession.commit() + try: + g.ubsession.commit() + except OperationalError: + g.ubsession.rollback() elif 'submit_deny' in element: usr.denied_column_value = restriction_addition(element, usr.list_denied_column_values) - g.ubsession.commit() + try: + g.ubsession.commit() + except OperationalError: + g.ubsession.rollback() return "" @admi.route("/ajax/deleterestriction/", methods=['POST']) @@ -407,10 +443,16 @@ def delete_restriction(res_type): usr = current_user if element['id'].startswith('a'): usr.allowed_tags = restriction_deletion(element, usr.list_allowed_tags) - g.ubsession.commit() + try: + g.ubsession.commit() + except OperationalError: + g.ubsession.rollback() elif element['id'].startswith('d'): usr.denied_tags = restriction_deletion(element, usr.list_denied_tags) - g.ubsession.commit() + try: + g.ubsession.commit() + except OperationalError: + g.ubsession.rollback() elif res_type == 3: # Columns per user usr_id = os.path.split(request.referrer)[-1] if usr_id.isdigit() == True: # select current user if admins are editing their own rights @@ -419,10 +461,16 @@ def delete_restriction(res_type): usr = current_user if element['id'].startswith('a'): usr.allowed_column_value = restriction_deletion(element, usr.list_allowed_column_values) - g.ubsession.commit() + try: + g.ubsession.commit() + except OperationalError: + g.ubsession.rollback() elif element['id'].startswith('d'): usr.denied_column_value = restriction_deletion(element, usr.list_denied_column_values) - g.ubsession.commit() + try: + g.ubsession.commit() + except OperationalError: + g.ubsession.rollback() return "" @@ -815,7 +863,10 @@ def _handle_new_user(to_save, content,languages, translations, kobo_support): content.allowed_column_value = config.config_allowed_column_value content.denied_column_value = config.config_denied_column_value g.ubsession.add(content) - g.ubsession.commit() + try: + g.ubsession.commit() + except OperationalError: + g.ubsession.rollback() flash(_(u"User '%(user)s' created", user=content.nickname), category="success") return redirect(url_for('admin.admin')) except IntegrityError: @@ -831,7 +882,10 @@ def _handle_edit_user(to_save, content,languages, translations, kobo_support): if g.ubsession.query(ub.User).filter(ub.User.role.op('&')(constants.ROLE_ADMIN) == constants.ROLE_ADMIN, ub.User.id != content.id).count(): g.ubsession.query(ub.User).filter(ub.User.id == content.id).delete() - g.ubsession.commit() + try: + g.ubsession.commit() + except OperationalError: + g.ubsession.rollback() flash(_(u"User '%(nick)s' deleted", nick=content.nickname), category="success") return redirect(url_for('admin.admin')) else: @@ -906,7 +960,10 @@ def _handle_edit_user(to_save, content,languages, translations, kobo_support): if "kindle_mail" in to_save and to_save["kindle_mail"] != content.kindle_mail: content.kindle_mail = to_save["kindle_mail"] try: - g.ubsession.commit() + try: + g.ubsession.commit() + except OperationalError: + g.ubsession.rollback() flash(_(u"User '%(nick)s' updated", nick=content.nickname), category="success") except IntegrityError: g.ubsession.rollback() diff --git a/cps/config_sql.py b/cps/config_sql.py index 3e531eef..bfcb81ad 100644 --- a/cps/config_sql.py +++ b/cps/config_sql.py @@ -22,8 +22,8 @@ import os import sys from sqlalchemy import exc, Column, String, Integer, SmallInteger, Boolean, BLOB, JSON +from sqlalchemy.exc import OperationalError from sqlalchemy.ext.declarative import declarative_base - from . import constants, cli, logger, ub @@ -247,7 +247,10 @@ class _ConfigSQL(object): if flask_settings == None: flask_settings = _Flask_Settings(os.urandom(32)) self._session.add(flask_settings) - self._session.commit() + try: + self._session.commit() + except OperationalError: + self._session.rollback() return flask_settings.flask_session_key @@ -308,7 +311,11 @@ class _ConfigSQL(object): log.warning("Log path %s not valid, falling back to default", self.config_logfile) self.config_logfile = logfile self._session.merge(s) - self._session.commit() + try: + self._session.commit() + except OperationalError as e: + log.error('Database error: %s', e) + self._session.rollback() def save(self): '''Apply all configuration values to the underlying storage.''' @@ -322,7 +329,11 @@ class _ConfigSQL(object): log.debug("_ConfigSQL updating storage") self._session.merge(s) - self._session.commit() + try: + self._session.commit() + except OperationalError as e: + log.error('Database error: %s', e) + self._session.rollback() self.load() def invalidate(self, error=None): @@ -363,7 +374,10 @@ def _migrate_table(session, orm_class): changed = True if changed: - session.commit() + try: + session.commit() + except OperationalError: + session.rollback() def autodetect_calibre_binary(): @@ -414,7 +428,10 @@ def load_configuration(): if not session.query(_Settings).count(): session.add(_Settings()) - session.commit() + try: + session.commit() + except OperationalError: + session.rollback() conf = _ConfigSQL(session) # Migrate from global restrictions to user based restrictions if bool(conf.config_default_show & constants.MATURE_CONTENT) and conf.config_denied_tags == "": @@ -422,5 +439,8 @@ def load_configuration(): conf.save() session.query(ub.User).filter(ub.User.mature_content != True). \ update({"denied_tags": conf.config_mature_content_tags}, synchronize_session=False) - session.commit() + try: + session.commit() + except OperationalError: + session.rollback() return conf diff --git a/cps/kobo.py b/cps/kobo.py index 47d10417..6c994525 100644 --- a/cps/kobo.py +++ b/cps/kobo.py @@ -44,6 +44,7 @@ from flask_login import current_user from werkzeug.datastructures import Headers from sqlalchemy import func from sqlalchemy.sql.expression import and_, or_ +from sqlalchemy.exc import OperationalError from sqlalchemy.orm import load_only from sqlalchemy.exc import StatementError import requests @@ -452,8 +453,10 @@ def HandleTagCreate(): items_unknown_to_calibre = add_items_to_shelf(items, shelf) if items_unknown_to_calibre: log.debug("Received request to add unknown books to a collection. Silently ignoring items.") - g.ubsession.commit() - + try: + g.ubsession.commit() + except OperationalError: + g.ubsession.rollback() return make_response(jsonify(str(shelf.uuid)), 201) @@ -485,7 +488,10 @@ def HandleTagUpdate(tag_id): shelf.name = name g.ubsession.merge(shelf) - g.ubsession.commit() + try: + g.ubsession.commit() + except OperationalError: + g.ubsession.rollback() return make_response(' ', 200) @@ -537,7 +543,10 @@ def HandleTagAddItem(tag_id): log.debug("Received request to add an unknown book to a collection. Silently ignoring item.") g.ubsession.merge(shelf) - g.ubsession.commit() + try: + g.ubsession.commit() + except OperationalError: + g.ubsession.rollback() return make_response('', 201) @@ -578,7 +587,10 @@ def HandleTagRemoveItem(tag_id): shelf.books.filter(ub.BookShelf.book_id == book.id).delete() except KeyError: items_unknown_to_calibre.append(item) - g.ubsession.commit() + try: + g.ubsession.commit() + except OperationalError: + g.ubsession.rollback() if items_unknown_to_calibre: log.debug("Received request to remove an unknown book to a collecition. Silently ignoring item.") @@ -624,7 +636,10 @@ def sync_shelves(sync_token, sync_results): "ChangedTag": tag }) sync_token.tags_last_modified = new_tags_last_modified - g.ubsession.commit() + try: + g.ubsession.commit() + except OperationalError: + g.ubsession.rollback() # Creates a Kobo "Tag" object from a ub.Shelf object @@ -705,7 +720,10 @@ def HandleStateRequest(book_uuid): abort(400, description="Malformed request data is missing 'ReadingStates' key") g.ubsession.merge(kobo_reading_state) - g.ubsession.commit() + try: + g.ubsession.commit() + except OperationalError: + g.ubsession.rollback() return jsonify({ "RequestResult": "Success", "UpdateResults": [update_results_response], @@ -743,7 +761,10 @@ def get_or_create_reading_state(book_id): kobo_reading_state.statistics = ub.KoboStatistics() book_read.kobo_reading_state = kobo_reading_state g.ubsession.add(book_read) - g.ubsession.commit() + try: + g.ubsession.commit() + except OperationalError: + g.ubsession.rollback() return book_read.kobo_reading_state @@ -846,7 +867,10 @@ def HandleBookDeletionRequest(book_uuid): archived_book.last_modified = datetime.datetime.utcnow() g.ubsession.merge(archived_book) - g.ubsession.commit() + try: + g.ubsession.commit() + except OperationalError: + g.ubsession.rollback() return ("", 204) diff --git a/cps/kobo_auth.py b/cps/kobo_auth.py index e936acce..dda5d018 100644 --- a/cps/kobo_auth.py +++ b/cps/kobo_auth.py @@ -66,6 +66,7 @@ from os import urandom from flask import g, Blueprint, url_for, abort, request from flask_login import login_user, login_required from flask_babel import gettext as _ +from sqlalchemy.exc import OperationalError from . import logger, ub, lm from .web import render_title_template @@ -147,7 +148,10 @@ def generate_auth_token(user_id): auth_token.token_type = 1 g.ubsession.add(auth_token) - g.ubsession.commit() + try: + g.ubsession.commit() + except OperationalError: + g.ubsession.rollback() return render_title_template( "generate_kobo_auth_url.html", title=_(u"Kobo Setup"), @@ -164,5 +168,8 @@ def delete_auth_token(user_id): # Invalidate any prevously generated Kobo Auth token for this user. g.ubsession.query(ub.RemoteAuthToken).filter(ub.RemoteAuthToken.user_id == user_id)\ .filter(ub.RemoteAuthToken.token_type==1).delete() - g.ubsession.commit() + try: + g.ubsession.commit() + except OperationalError: + g.ubsession.rollback() return "" diff --git a/cps/oauth_bb.py b/cps/oauth_bb.py index 1305c977..000a7376 100644 --- a/cps/oauth_bb.py +++ b/cps/oauth_bb.py @@ -32,6 +32,7 @@ from flask_dance.contrib.github import make_github_blueprint, github from flask_dance.contrib.google import make_google_blueprint, google from flask_login import login_user, current_user from sqlalchemy.orm.exc import NoResultFound +from sqlalchemy.exc import OperationalError from . import constants, logger, config, app, ub from .web import login_required @@ -104,12 +105,18 @@ if ub.oauth_support: oauthProvider.provider_name = "github" oauthProvider.active = False g.ubsession.add(oauthProvider) - g.ubsession.commit() + try: + g.ubsession.commit() + except OperationalError: + g.ubsession.rollback() oauthProvider = ub.OAuthProvider() oauthProvider.provider_name = "google" oauthProvider.active = False g.ubsession.add(oauthProvider) - g.ubsession.commit() + try: + g.ubsession.commit() + except OperationalError: + g.ubsession.rollback() oauth_ids = g.ubsession.query(ub.OAuthProvider).all() ele1 = dict(provider_name='github', diff --git a/cps/shelf.py b/cps/shelf.py index 8ab0b5d8..b312922d 100644 --- a/cps/shelf.py +++ b/cps/shelf.py @@ -322,8 +322,11 @@ def delete_shelf_helper(cur_shelf): g.ubsession.delete(cur_shelf) g.ubsession.query(ub.BookShelf).filter(ub.BookShelf.shelf == shelf_id).delete() g.ubsession.add(ub.ShelfArchive(uuid=cur_shelf.uuid, user_id=cur_shelf.user_id)) - g.ubsession.commit() - log.info("successfully deleted %s", cur_shelf) + try: + g.ubsession.commit() + log.info("successfully deleted %s", cur_shelf) + except OperationalError: + g.ubsession.rollback() diff --git a/cps/ub.py b/cps/ub.py index 9a7069dc..a1e67655 100644 --- a/cps/ub.py +++ b/cps/ub.py @@ -681,13 +681,19 @@ def update_download(book_id, user_id): if not check: new_download = Downloads(user_id=user_id, book_id=book_id) g.ubsession.add(new_download) - g.ubsession.commit() + try: + g.ubsession.commit() + except exc.OperationalError: + g.ubsession.rollback() # Delete non exisiting downloaded books in calibre-web's own database def delete_download(book_id): g.ubsession.query(Downloads).filter(book_id == Downloads.book_id).delete() - g.ubsession.commit() + try: + g.ubsession.commit() + except exc.OperationalError: + g.ubsession.rollback() # Generate user Guest (translated text), as anonymous user, no rights def create_anonymous_user(session): @@ -746,7 +752,7 @@ def init_db(app_db_path): session.close() -def dispose(): +'''def dispose(): global session old_session = session @@ -760,4 +766,4 @@ def dispose(): try: old_session.bind.dispose() except Exception: - pass + pass''' diff --git a/cps/web.py b/cps/web.py index 15d22d60..e93a5af8 100644 --- a/cps/web.py +++ b/cps/web.py @@ -434,7 +434,10 @@ def bookmark(book_id, book_format): ub.Bookmark.book_id == book_id, ub.Bookmark.format == book_format)).delete() if not bookmark_key: - g.ubsession.commit() + try: + g.ubsession.commit() + except OperationalError: + g.ubsession.rollback() return "", 204 lbookmark = ub.Bookmark(user_id=current_user.id, @@ -442,7 +445,10 @@ def bookmark(book_id, book_format): format=book_format, bookmark_key=bookmark_key) g.ubsession.merge(lbookmark) - g.ubsession.commit() + try: + g.ubsession.commit() + except OperationalError: + g.ubsession.rollback() return "", 201 @@ -467,7 +473,10 @@ def toggle_read(book_id): kobo_reading_state.statistics = ub.KoboStatistics() book.kobo_reading_state = kobo_reading_state g.ubsession.merge(book) - g.ubsession.commit() + try: + g.ubsession.commit() + except OperationalError: + g.ubsession.rollback() else: try: calibre_db.update_title_sort(config) @@ -501,7 +510,10 @@ def toggle_archived(book_id): archived_book = ub.ArchivedBook(user_id=current_user.id, book_id=book_id) archived_book.is_archived = True g.ubsession.merge(archived_book) - g.ubsession.commit() + try: + g.ubsession.commit() + except OperationalError: + g.ubsession.rollback() return "" @@ -1086,11 +1098,12 @@ def update_table_settings(): except AttributeError: pass g.ubsession.commit() - except InvalidRequestError: + except (InvalidRequestError, OperationalError): log.error("Invalid request received: %r ", request, ) return "Invalid request", 400 return "" + @web.route("/author") @login_required_if_no_ano def author_list(): @@ -1676,7 +1689,10 @@ def logout(): def remote_login(): auth_token = ub.RemoteAuthToken() g.ubsession.add(auth_token) - g.ubsession.commit() + try: + g.ubsession.commit() + except OperationalError: + g.ubsession.rollback() verify_url = url_for('web.verify_token', token=auth_token.auth_token, _external=true) log.debug(u"Remot Login request with token: %s", auth_token.auth_token) @@ -1708,7 +1724,10 @@ def verify_token(token): # Update token with user information auth_token.user_id = current_user.id auth_token.verified = True - g.ubsession.commit() + try: + g.ubsession.commit() + except OperationalError: + g.ubsession.rollback() flash(_(u"Success! Please return to your device"), category="success") log.debug(u"Remote Login token for userid %s verified", auth_token.user_id) @@ -1731,7 +1750,10 @@ def token_verified(): # Token expired elif datetime.now() > auth_token.expiration: g.ubsession.delete(auth_token) - g.ubsession.commit() + try: + g.ubsession.commit() + except OperationalError: + g.ubsession.rollback() data['status'] = 'error' data['message'] = _(u"Token has expired") @@ -1744,7 +1766,10 @@ def token_verified(): login_user(user) g.ubsession.delete(auth_token) - g.ubsession.commit() + try: + g.ubsession.commit() + except OperationalError: + g.ubsession.rollback() data['status'] = 'success' log.debug(u"Remote Login for userid %s succeded", user.id) @@ -1836,14 +1861,11 @@ def profile(): g.ubsession.rollback() flash(_(u"Found an existing account for this e-mail address."), category="error") log.debug(u"Found an existing account for this e-mail address.") - '''return render_title_template("user_edit.html", - content=current_user, - translations=translations, - kobo_support=kobo_support, - title=_(u"%(name)s's profile", name=current_user.nickname), - page="me", - registered_oauth=local_oauth_check, - oauth_status=oauth_status)''' + except OperationalError as e: + g.ubsession.rollback() + log.error("Database error: %s", e) + flash(_(u"Database error: %(error)s.", error=e), category="error") + return render_title_template("user_edit.html", translations=translations, profile=1,