diff --git a/critiquebrainz/data/utils.py b/critiquebrainz/data/utils.py index a462b18a0..4d36a8104 100644 --- a/critiquebrainz/data/utils.py +++ b/critiquebrainz/data/utils.py @@ -14,7 +14,7 @@ def create_all(): - db.run_sql_script(os.path.join(ADMIN_SQL_DIR, 'create_extensions.sql')) + db.run_sql_script_without_transaction(os.path.join(ADMIN_SQL_DIR, 'create_extensions.sql')) db.run_sql_script(os.path.join(ADMIN_SQL_DIR, 'create_types.sql')) db.run_sql_script(os.path.join(ADMIN_SQL_DIR, 'create_tables.sql')) db.run_sql_script(os.path.join(ADMIN_SQL_DIR, 'create_primary_keys.sql')) diff --git a/critiquebrainz/db/__init__.py b/critiquebrainz/db/__init__.py index 3e03eadf5..7aed3d606 100644 --- a/critiquebrainz/db/__init__.py +++ b/critiquebrainz/db/__init__.py @@ -1,4 +1,5 @@ -from sqlalchemy import create_engine +from flask_debugtoolbar.panels import sqlalchemy +from sqlalchemy import create_engine, text from sqlalchemy.pool import NullPool # This value must be incremented after schema changes on exported tables! @@ -22,7 +23,22 @@ def init_db_engine(connect_str): def run_sql_script(sql_file_path): - with open(sql_file_path) as sql: - connection = engine.connect() - connection.execute(sql.read()) - connection.close() + with open(sql_file_path) as sql, engine.begin() as connection: + connection.execute(text(sql.read())) + + +def run_sql_script_without_transaction(sql_file_path): + with open(sql_file_path) as sql, engine.connect().execution_options(isolation_level="AUTOCOMMIT") as connection: + lines = sql.read().splitlines() + try: + for line in lines: + # TODO: Not a great way of removing comments. The alternative is to catch + # the exception sqlalchemy.exc.ProgrammingError "can't execute an empty query" + if line and not line.startswith("--"): + connection.execute(text(line)) + except sqlalchemy.exc.ProgrammingError as e: + print("Error: {}".format(e)) + return False + finally: + connection.close() + return True diff --git a/critiquebrainz/db/avg_rating.py b/critiquebrainz/db/avg_rating.py index 78576fbdf..76a9f0634 100644 --- a/critiquebrainz/db/avg_rating.py +++ b/critiquebrainz/db/avg_rating.py @@ -42,11 +42,12 @@ def update(entity_id, entity_type): # Calculate average rating and update it sum, count = row[0], row[1] + if count == 0: delete(entity_id, entity_type) return avg_rating = int(sum / count + 0.5) - with db.engine.connect() as connection: + with db.engine.begin() as connection: connection.execute(sqlalchemy.text(""" INSERT INTO avg_rating(entity_id, entity_type, rating, count) VALUES (:entity_id, :entity_type, :rating, :count) @@ -70,7 +71,7 @@ def delete(entity_id, entity_type): entity_id (uuid): ID of the entity entity_type (str): Type of the entity """ - with db.engine.connect() as connection: + with db.engine.begin() as connection: connection.execute(sqlalchemy.text(""" DELETE FROM avg_rating @@ -111,7 +112,7 @@ def get(entity_id, entity_type): "entity_type": entity_type }) - avg_rating = result.fetchone() + avg_rating = result.mappings().first() if not avg_rating: raise db_exceptions.NoDataFoundException("""No rating for the entity with ID: {id} and Type: {type}""". format(id=entity_id, type=entity_type)) diff --git a/critiquebrainz/db/comment.py b/critiquebrainz/db/comment.py index 58b9ec271..cd5d2e56a 100644 --- a/critiquebrainz/db/comment.py +++ b/critiquebrainz/db/comment.py @@ -36,7 +36,7 @@ def create(*, user_id, text, review_id, is_draft=False): Returns: the newly created comment in dict form """ - with db.engine.connect() as connection: + with db.engine.begin() as connection: result = connection.execute(sqlalchemy.text(""" INSERT INTO comment (user_id, review_id, is_draft) VALUES (:user_id, :review_id, :is_draft) @@ -46,8 +46,8 @@ def create(*, user_id, text, review_id, is_draft=False): 'review_id': review_id, 'is_draft': is_draft, }) - comment_id = result.fetchone()['id'] - db_comment_revision.create(comment_id, text) + comment_id = result.fetchone().id + db_comment_revision.create(connection, comment_id, text) return get_by_id(comment_id) @@ -100,7 +100,7 @@ def get_by_id(comment_id): 'comment_id': comment_id, }) - comment = result.fetchone() + comment = result.mappings().first() if not comment: raise db_exceptions.NoDataFoundException('Can\'t find comment with ID: {id}'.format(id=comment_id)) @@ -199,7 +199,7 @@ def list_comments(*, review_id=None, user_id=None, limit=20, offset=0, inc_hidde OFFSET :offset """.format(where_clause=filterstr)), query_vals) - rows = [dict(row) for row in result.fetchall()] + rows = [dict(row) for row in result.mappings()] for row in rows: row['last_revision'] = { 'id': row.pop('last_revision_id'), @@ -225,7 +225,7 @@ def delete(comment_id): Args: comment_id (uuid): the ID of the comment to be deleted. """ - with db.engine.connect() as connection: + with db.engine.begin() as connection: connection.execute(sqlalchemy.text(""" DELETE FROM comment @@ -266,15 +266,15 @@ def update(comment_id, *, text=None, is_draft=None, is_hidden=None): setstr = ', '.join(updates) update_data['comment_id'] = comment_id - with db.engine.connect() as connection: + with db.engine.begin() as connection: connection.execute(sqlalchemy.text(""" UPDATE comment SET {setstr} WHERE id = :comment_id """.format(setstr=setstr)), update_data) - if text is not None: - db_comment_revision.create(comment_id, text) + if text is not None: + db_comment_revision.create(connection, comment_id, text) def count_comments(*, review_id=None, user_id=None, is_hidden=None, is_draft=None): @@ -317,4 +317,4 @@ def count_comments(*, review_id=None, user_id=None, is_hidden=None, is_draft=Non FROM comment {where_condition} """.format(where_condition=filterstr)), filter_data) - return result.fetchone()[0] + return result.fetchone().count diff --git a/critiquebrainz/db/comment_revision.py b/critiquebrainz/db/comment_revision.py index 23b5e2597..5210790b1 100644 --- a/critiquebrainz/db/comment_revision.py +++ b/critiquebrainz/db/comment_revision.py @@ -21,7 +21,7 @@ import critiquebrainz.db as db -def create(comment_id, text): +def create(conn, comment_id, text): """ Create a new revision for comment with specified ID. Args: @@ -31,14 +31,13 @@ def create(comment_id, text): Returns: int: the ID of the new revision created. """ - with db.engine.connect() as connection: - result = connection.execute(sqlalchemy.text(""" - INSERT INTO comment_revision (comment_id, text) - VALUES (:comment_id, :text) - RETURNING id - """), { - 'comment_id': comment_id, - 'text': text, - }) + result = conn.execute(sqlalchemy.text(""" + INSERT INTO comment_revision (comment_id, text) + VALUES (:comment_id, :text) + RETURNING id + """), { + 'comment_id': comment_id, + 'text': text, + }) - return result.fetchone()['id'] + return result.fetchone().id diff --git a/critiquebrainz/db/license.py b/critiquebrainz/db/license.py index 91063f2f3..93a49b965 100644 --- a/critiquebrainz/db/license.py +++ b/critiquebrainz/db/license.py @@ -23,7 +23,7 @@ def create(*, id, full_name, info_url=None): "full_name": full_name, "info_url": info_url, } - with db.engine.connect() as connection: + with db.engine.begin() as connection: connection.execute(sqlalchemy.text(""" INSERT INTO license(id, full_name, info_url) VALUES (:id, :full_name, :info_url) @@ -37,7 +37,7 @@ def delete(*, id): Args: id (str): ID of the license. """ - with db.engine.connect() as connection: + with db.engine.begin() as connection: connection.execute(sqlalchemy.text(""" DELETE FROM license @@ -59,7 +59,7 @@ def get_licenses_list(connection): FROM license """) results = connection.execute(query) - return [dict(row) for row in results.fetchall()] + return [dict(row) for row in results.mappings()] def list_licenses(): diff --git a/critiquebrainz/db/moderation_log.py b/critiquebrainz/db/moderation_log.py index e9f2abb58..17c800de9 100644 --- a/critiquebrainz/db/moderation_log.py +++ b/critiquebrainz/db/moderation_log.py @@ -38,7 +38,7 @@ def create(*, admin_id, review_id=None, user_id=None, raise ValueError("No review ID or user ID specified.") if action not in AdminActions.get_all_actions(): raise ValueError("Please specify a valid action.") - with db.engine.connect() as connection: + with db.engine.begin() as connection: connection.execute(sqlalchemy.text(""" INSERT INTO moderation_log(admin_id, user_id, review_id, action, timestamp, reason) VALUES (:admin_id, :user_id, :review_id, :action, :timestamp, :reason) @@ -81,7 +81,7 @@ def list_logs(*, admin_id=None, limit=None, offset=None): """.format(where_clause=where_clause)) with db.engine.connect() as connection: result = connection.execute(query, filter_data) - count = result.fetchone()[0] + count = result.fetchone().count filter_data["limit"] = limit filter_data["offset"] = offset @@ -142,7 +142,7 @@ def list_logs(*, admin_id=None, limit=None, offset=None): with db.engine.connect() as connection: result = connection.execute(query, filter_data) logs = [] - for log in result.fetchall(): + for log in result.mappings(): log = dict(log) log["user"] = { "id": log["user_id"], diff --git a/critiquebrainz/db/oauth_client.py b/critiquebrainz/db/oauth_client.py index a7d0e8f04..ce122361d 100644 --- a/critiquebrainz/db/oauth_client.py +++ b/critiquebrainz/db/oauth_client.py @@ -29,7 +29,7 @@ def create(*, user_id, name, desc, website, redirect_uri): "redirect_uri": redirect_uri, } """ - with db.engine.connect() as connection: + with db.engine.begin() as connection: row = connection.execute(sqlalchemy.text(""" INSERT INTO oauth_client (client_id, client_secret, redirect_uri, user_id, name, "desc", website) @@ -45,7 +45,7 @@ def create(*, user_id, name, desc, website, redirect_uri): "website": website, "desc": desc, }) - return dict(row.fetchone()) + return dict(row.mappings().first()) def update(*, client_id, name=None, desc=None, website=None, redirect_uri=None): @@ -83,7 +83,7 @@ def update(*, client_id, name=None, desc=None, website=None, redirect_uri=None): """.format(update_str=update_str)) if update_str: - with db.engine.connect() as connection: + with db.engine.begin() as connection: connection.execute(query, update_data) @@ -93,7 +93,7 @@ def delete(client_id): Args: client_id(str): ID of the OAuth Client. """ - with db.engine.connect() as connection: + with db.engine.begin() as connection: connection.execute(sqlalchemy.text(""" DELETE FROM oauth_client @@ -134,7 +134,7 @@ def get_client(client_id): """), { "client_id": client_id, }) - row = result.fetchone() + row = result.mappings().first() if not row: raise db_exceptions.NoDataFoundException("Can't find OAuth client with ID: {id}".format(id=client_id)) - return dict(row) + return dict(row) diff --git a/critiquebrainz/db/oauth_grant.py b/critiquebrainz/db/oauth_grant.py index f48e96a02..7a9ea9a9a 100644 --- a/critiquebrainz/db/oauth_grant.py +++ b/critiquebrainz/db/oauth_grant.py @@ -35,13 +35,13 @@ def create(*, client_id, scopes, code, expires, redirect_uri, user_id): "user_id": user_id, "scopes": scopes, } - with db.engine.connect() as connection: + with db.engine.begin() as connection: result = connection.execute(sqlalchemy.text(""" INSERT INTO oauth_grant(client_id, code, expires, redirect_uri, scopes, user_id) VALUES (:client_id, :code, :expires, :redirect_uri, :scopes, :user_id) RETURNING id """), grant) - grant["id"] = result.fetchone()[0] + grant["id"] = result.first().id return grant @@ -97,8 +97,8 @@ def list_grants(*, client_id=None, code=None, limit=1, offset=None): LIMIT :limit OFFSET :offset """.format(where_clause=filterstr)), filter_data) - rows = results.fetchall() - return [dict(row) for row in rows] + rows = results.mappings() + return [dict(row) for row in rows] def delete(*, client_id, code): @@ -108,7 +108,7 @@ def delete(*, client_id, code): client_id (str): ID of the OAuth Client. code (str): The authorization code returned from authorization request. """ - with db.engine.connect() as connection: + with db.engine.begin() as connection: connection.execute(sqlalchemy.text(""" DELETE FROM oauth_grant @@ -136,9 +136,9 @@ def get_scopes(grant_id): """), { "grant_id": grant_id, }) - scopes = result.fetchone() + scopes = result.mappings().first() if not scopes: raise db_exceptions.NoDataFoundException("No grant exists with ID: {grant_id}".format(grant_id=grant_id)) - if scopes[0] is None: + if scopes["scopes"] is None: return list() - return scopes[0].split() + return scopes["scopes"].split() diff --git a/critiquebrainz/db/oauth_token.py b/critiquebrainz/db/oauth_token.py index 2398074d6..f686d7291 100644 --- a/critiquebrainz/db/oauth_token.py +++ b/critiquebrainz/db/oauth_token.py @@ -35,13 +35,13 @@ def create(*, client_id, scopes, access_token, refresh_token, expires, user_id): "user_id": user_id, "scopes": scopes, } - with db.engine.connect() as connection: + with db.engine.begin() as connection: result = connection.execute(sqlalchemy.text(""" INSERT INTO oauth_token(client_id, access_token, refresh_token, expires, scopes, user_id) VALUES (:client_id, :access_token, :refresh_token, :expires, :scopes, :user_id) RETURNING id """), token) - token["id"] = result.fetchone()[0] + token["id"] = result.first().id return token @@ -107,8 +107,8 @@ def list_tokens(*, client_id=None, refresh_token=None, access_token=None, limit= LIMIT :limit OFFSET :offset """.format(where_clause=filterstr)), filter_data) - rows = results.fetchall() - return [dict(row) for row in rows] + rows = results.mappings() + return [dict(row) for row in rows] def delete(*, client_id=None, refresh_token=None, user_id=None): @@ -128,7 +128,7 @@ def delete(*, client_id=None, refresh_token=None, user_id=None): filters.append("user_id = :user_id") filter_data["user_id"] = user_id filterstr = " AND ".join(filters) - with db.engine.connect() as connection: + with db.engine.begin() as connection: connection.execute(sqlalchemy.text(""" DELETE FROM oauth_token diff --git a/critiquebrainz/db/review.py b/critiquebrainz/db/review.py index 594f0ef18..26bca0a83 100644 --- a/critiquebrainz/db/review.py +++ b/critiquebrainz/db/review.py @@ -165,7 +165,7 @@ def get_by_ids(review_ids: List[uuid.UUID]): "review_ids": tuple(map(str, review_ids)), }) - reviews = result.fetchall() + reviews = result.mappings().fetchall() results = [] for review in reviews: @@ -210,7 +210,7 @@ def set_hidden_state(review_id, *, is_hidden): is_hidden (bool): True if review has to be hidden, False if not. """ review = get_by_id(review_id) - with db.engine.connect() as connection: + with db.engine.begin() as connection: connection.execute(sqlalchemy.text(""" UPDATE review SET is_hidden = :is_hidden @@ -242,7 +242,7 @@ def get_count(*, is_draft=False, is_hidden=False): "is_drafted": is_draft, "is_hidden": is_hidden, }) - return result.fetchone()[0] + return result.fetchone().count def update(review_id, *, drafted, text=None, rating=None, license_id=None, language=None, is_draft=None): @@ -290,13 +290,15 @@ def update(review_id, *, drafted, text=None, rating=None, license_id=None, langu WHERE id = :review_id """.format(setstr=setstr)) - with db.engine.connect() as connection: + with db.engine.begin() as connection: if setstr: updated_info["review_id"] = review_id connection.execute(query, updated_info) db_revision.create(connection, review_id, text, rating) - db_revision.update_rating(review_id) + db_revision.update_rating(review_id) + + with db.engine.begin() as connection: result = connection.execute(sqlalchemy.text(""" SELECT review.entity_id FROM review @@ -304,7 +306,7 @@ def update(review_id, *, drafted, text=None, rating=None, license_id=None, langu """), { "review_id": review_id, }) - review = dict(result.fetchone()) + review = dict(result.mappings().first()) invalidate_ws_entity_cache(review["entity_id"]) @@ -363,7 +365,7 @@ def create(*, entity_id, entity_type, user_id, is_draft, text=None, rating=None, else: published_on = datetime.now() - with db.engine.connect() as connection: + with db.engine.begin() as connection: result = connection.execute(sqlalchemy.text(""" INSERT INTO review (id, entity_id, entity_type, user_id, edits, is_draft, is_hidden, license_id, language, source, source_url, published_on) VALUES (:id, :entity_id, :entity_type, :user_id, :edits, :is_draft, :is_hidden, :license_id, :language, :source, :source_url, :published_on) @@ -562,7 +564,7 @@ def get_reviews_list(connection, *, inc_drafts=False, inc_hidden=False, entity_i filter_data["offset"] = offset results = connection.execute(query, filter_data) - rows = results.fetchall() + rows = results.mappings() rows = [dict(row) for row in rows] # Organise last revision info in reviews @@ -704,9 +706,8 @@ def get_popular_reviews_for_index(): "limit": defined_limit, "last_month": datetime.now() - timedelta(weeks=4) }) - reviews = results.fetchall() + reviews = [dict(review) for review in results.mappings()] - reviews = [dict(review) for review in reviews] if reviews: for review in reviews: review["rating"] = RATING_SCALE_1_5.get(review["rating"]) @@ -731,7 +732,7 @@ def delete(review_id): review_id: ID of the review to be deleted. """ review = get_by_id(review_id) - with db.engine.connect() as connection: + with db.engine.begin() as connection: connection.execute(sqlalchemy.text(""" DELETE FROM review @@ -757,7 +758,7 @@ def check_review_deleted(review_id) -> bool: "review_id": review_id, }) - return dict(result.fetchone())["exists"] + return result.first().exists def get_distinct_entities(connection): @@ -771,7 +772,7 @@ def get_distinct_entities(connection): """) results = connection.execute(query) - return {row[0] for row in results.fetchall()} + return {row["entity_id"] for row in results.mappings()} def distinct_entities(): @@ -808,5 +809,4 @@ def reviewed_entities(*, entity_ids, entity_type): "entity_type": entity_type, "entity_ids": tuple(entity_ids), }) - reviewed_ids = [str(row[0]) for row in results.fetchall()] - return reviewed_ids + return [str(row["entity_id"]) for row in results.mappings()] diff --git a/critiquebrainz/db/revision.py b/critiquebrainz/db/revision.py index bce09ee49..4d40cd02b 100644 --- a/critiquebrainz/db/revision.py +++ b/critiquebrainz/db/revision.py @@ -30,7 +30,7 @@ def get(review_id, limit=1, offset=0): "votes_negative": (int), } """ - with db.engine.connect() as connection: + with db.engine.begin() as connection: result = connection.execute(sqlalchemy.text(""" SELECT id, review_id, @@ -57,7 +57,7 @@ def get(review_id, limit=1, offset=0): "limit": limit }) - rows = result.fetchall() + rows = result.mappings().all() if not rows: raise db_exceptions.NoDataFoundException("Cannot find specified review.") rows = [dict(row) for row in rows] diff --git a/critiquebrainz/db/spam_report.py b/critiquebrainz/db/spam_report.py index aa32ce834..82a8cc0f6 100644 --- a/critiquebrainz/db/spam_report.py +++ b/critiquebrainz/db/spam_report.py @@ -38,8 +38,8 @@ def get(user_id, revision_id): "revision_id": revision_id, }) - report = result.fetchone() - return dict(report) if report else None + report = result.mappings().first() + return dict(report) if report else None def archive(user_id, revision_id): @@ -49,7 +49,7 @@ def archive(user_id, revision_id): user_id(uuid): ID of the reporter. revision_id(uuid): ID of the revision of the reported review. """ - with db.engine.connect() as connection: + with db.engine.begin() as connection: connection.execute(sqlalchemy.text(""" UPDATE spam_report SET is_archived = 'true' @@ -79,7 +79,7 @@ def create(revision_id, user_id, reason): "is_archived": (bool) } """ - with db.engine.connect() as connection: + with db.engine.begin() as connection: connection.execute(sqlalchemy.text(""" INSERT INTO spam_report (user_id, reason, revision_id, reported_at, is_archived) VALUES (:user_id, :reason, :revision_id, :reported_at, :is_archived) @@ -185,7 +185,7 @@ def list_reports(**kwargs): with db.engine.connect() as connection: result = connection.execute(query, filter_data) - spam_reports = result.fetchall() + spam_reports = result.mappings() if spam_reports: spam_reports = [dict(spam_report) for spam_report in spam_reports] for spam_report in spam_reports: diff --git a/critiquebrainz/db/statistics.py b/critiquebrainz/db/statistics.py index 652042ece..931c21adf 100644 --- a/critiquebrainz/db/statistics.py +++ b/critiquebrainz/db/statistics.py @@ -85,7 +85,7 @@ def get_users_with_review_count(from_date=date(1970, 1, 1), to_date=date.today() "to_date": to_date, }) - reviewers = result.fetchall() + reviewers = result.mappings() if not reviewers: raise db_exceptions.NoDataFoundException("Can't get users with review count!") reviewers = [dict(reviewer) for reviewer in reviewers] @@ -126,7 +126,7 @@ def get_users_with_vote_count(from_date=date(1970, 1, 1), to_date=date.today() + "to_date": to_date, }) - voters = result.fetchall() + voters = result.mappings() if not voters: raise db_exceptions.NoDataFoundException("Can't get users with vote count!") voters = [dict(voter) for voter in voters] @@ -172,7 +172,7 @@ def get_users_with_comment_count(from_date=date(1970, 1, 1), to_date=date.today( "to_date": to_date, }) - commenters = result.fetchall() + commenters = result.mappings() if not commenters: raise db_exceptions.NoDataFoundException("Can't get users with comment count!") commenters = [dict(commenter) for commenter in commenters] diff --git a/critiquebrainz/db/users.py b/critiquebrainz/db/users.py index 85c15873c..f6e7a9996 100644 --- a/critiquebrainz/db/users.py +++ b/critiquebrainz/db/users.py @@ -1,6 +1,5 @@ import uuid from datetime import datetime -from hashlib import md5 import sqlalchemy @@ -42,7 +41,7 @@ def get_many_by_mb_username(usernames): """.format(columns=','.join(USER_GET_COLUMNS))), { 'musicbrainz_usernames': usernames, }) - row = result.fetchall() + row = result.mappings() users = [dict(r) for r in row] return users @@ -62,7 +61,7 @@ def get_user_by_ref(user_ref): """.format(columns=','.join(USER_GET_COLUMNS))), { 'user_ref': user_ref, }) - row = result.fetchone() + row = result.mappings().first() if row: return dict(row) return None @@ -82,11 +81,10 @@ def get_user_by_id(connection, user_id): result = connection.execute(query, { "user_id": user_id }) - row = result.fetchone() - if not row: - return None - row = dict(row) - return row + row = result.mappings().first() + if row: + return dict(row) + return None def get_by_id(user_id): @@ -143,7 +141,7 @@ def create(**user_data): if user_data: raise TypeError('Unexpected **user_data: %r' % user_data) - with db.engine.connect() as connection: + with db.engine.begin() as connection: result = connection.execute(sqlalchemy.text(""" INSERT INTO "user" (id, display_name, email, created, musicbrainz_id, is_blocked, license_choice, musicbrainz_row_id) @@ -160,7 +158,7 @@ def create(**user_data): "license_choice": license_choice, "musicbrainz_row_id": musicbrainz_row_id, }) - new_id = result.fetchone()[0] + new_id = result.fetchone().id return get_by_id(new_id) @@ -190,11 +188,10 @@ def get_by_mbid(musicbrainz_username): """.format(columns=','.join(USER_GET_COLUMNS))), { "musicbrainz_username": musicbrainz_username, }) - row = result.fetchone() - if not row: - return None - row = dict(row) - return row + row = result.mappings().first() + if row: + return dict(row) + return None def get_or_create(musicbrainz_row_id, musicbrainz_username, new_user_data): @@ -256,7 +253,7 @@ def update_username(user, new_musicbrainz_id: str): WHERE id = :cb_id """.format(", ".join(updates)) - with db.engine.connect() as connection: + with db.engine.begin() as connection: connection.execute(sqlalchemy.text(query), { "cb_id": user["id"], "new_musicbrainz_id": new_musicbrainz_id, @@ -281,7 +278,7 @@ def total_count(): FROM "user" """)) - return result.fetchone()[0] + return result.fetchone().count def list_users(limit=None, offset=0): @@ -314,9 +311,8 @@ def list_users(limit=None, offset=0): "limit": limit, "offset": offset }) - rows = result.fetchall() - rows = [dict(row) for row in rows] - return rows + rows = result.mappings() + return [dict(row) for row in rows] def unblock(user_id): @@ -325,7 +321,7 @@ def unblock(user_id): Args: user_id(uuid): ID of user to be unblocked. """ - with db.engine.connect() as connection: + with db.engine.begin() as connection: connection.execute(sqlalchemy.text(""" UPDATE "user" SET is_blocked = 'false' @@ -341,7 +337,7 @@ def block(user_id): Args: user_id(uuid): ID of user to be blocked. """ - with db.engine.connect() as connection: + with db.engine.begin() as connection: connection.execute(sqlalchemy.text(""" UPDATE "user" SET is_blocked = 'true' @@ -372,8 +368,8 @@ def has_voted(user_id, review_id): "revision_id": last_revision['id'], "user_id": user_id }) - count = result.fetchone()[0] - return count > 0 + count = result.fetchone().count + return count > 0 def karma(user_id): @@ -449,8 +445,8 @@ def reviews(user_id): "user_id": user_id }) - rows = result.fetchall() - return [dict(row) for row in rows] + rows = result.mappings() + return [dict(row) for row in rows] def get_votes(user_id, from_date=datetime.utcfromtimestamp(0)): @@ -479,8 +475,8 @@ def get_votes(user_id, from_date=datetime.utcfromtimestamp(0)): "from_date": from_date }) - rows = result.fetchall() - return [dict(row) for row in rows] + rows = result.mappings() + return [dict(row) for row in rows] def get_reviews(user_id, from_date=datetime.utcfromtimestamp(0)): @@ -532,8 +528,8 @@ def get_reviews(user_id, from_date=datetime.utcfromtimestamp(0)): "from_date": from_date }) - rows = result.fetchall() - return [dict(row) for row in rows] + rows = result.mappings() + return [dict(row) for row in rows] def get_comments(user_id, from_date=datetime.utcfromtimestamp(0)): @@ -577,8 +573,8 @@ def get_comments(user_id, from_date=datetime.utcfromtimestamp(0)): "from_date": from_date }) - rows = result.fetchall() - return [dict(row) for row in rows] + rows = result.mappings() + return [dict(row) for row in rows] def update(user_id, user_new_info): @@ -608,7 +604,7 @@ def update(user_id, user_new_info): """.format(setstr)) if user_new_info: user_new_info["user_id"] = user_id - with db.engine.connect() as connection: + with db.engine.begin() as connection: connection.execute(query, user_new_info) @@ -619,7 +615,7 @@ def delete(user_id): Args: user_id(uuid): ID of the user to be deleted. """ - with db.engine.connect() as connection: + with db.engine.begin() as connection: connection.execute(sqlalchemy.text(""" DELETE FROM "user" @@ -662,8 +658,8 @@ def clients(user_id): "user_id": user_id }) - rows = result.fetchall() - return [dict(row) for row in rows] + rows = result.mappings() + return [dict(row) for row in rows] def tokens(user_id): @@ -703,8 +699,8 @@ def tokens(user_id): "user_id": user_id }) - rows = result.fetchall() - return [dict(row) for row in rows] + rows = result.mappings() + return [dict(row) for row in rows] def get_by_mb_row_id(musicbrainz_row_id, musicbrainz_id=None): @@ -734,7 +730,7 @@ def get_by_mb_row_id(musicbrainz_row_id, musicbrainz_id=None): WHERE musicbrainz_row_id = :musicbrainz_row_id {optional_filter} """.format(columns=','.join(USER_GET_COLUMNS), optional_filter=filter_str)), filter_data) - - if result.rowcount: - return dict(result.fetchone()) + row = result.mappings().first() + if row: + return dict(row) return None diff --git a/critiquebrainz/db/vote.py b/critiquebrainz/db/vote.py index 3f9eaa1b7..3e0b92680 100644 --- a/critiquebrainz/db/vote.py +++ b/critiquebrainz/db/vote.py @@ -31,7 +31,7 @@ def get(user_id, revision_id): "user_id": user_id, "revision_id": revision_id, }) - row = result.fetchone() + row = result.mappings().first() if not row: raise db_exceptions.NoDataFoundException("Cannot find specified vote.") return dict(row) @@ -47,7 +47,7 @@ def submit(user_id, revision_id, vote): revision_id (id): ID of a review revision that the vote is associated with. vote (bool): `False` if it's a negative vote, `True` if positive. """ - with db.engine.connect() as connection: + with db.engine.begin() as connection: connection.execute(sqlalchemy.text(""" INSERT INTO vote (user_id, revision_id, vote, rated_at) VALUES (:user_id, :revision_id, :vote, :rated_at) @@ -67,7 +67,7 @@ def delete(user_id, revision_id): user_id (uuid): ID of a user. revision_id (id): ID of a review revision that the vote is associated with. """ - with db.engine.connect() as connection: + with db.engine.begin() as connection: connection.execute(sqlalchemy.text(""" DELETE FROM vote WHERE user_id = :user_id AND revision_id = :revision_id @@ -85,4 +85,4 @@ def get_count(): SELECT count(*) FROM vote """)) - return result.fetchone()[0] + return result.fetchone().count diff --git a/critiquebrainz/frontend/external/bookbrainz_db/author.py b/critiquebrainz/frontend/external/bookbrainz_db/author.py index c38ff36d8..fc3afcd47 100644 --- a/critiquebrainz/frontend/external/bookbrainz_db/author.py +++ b/critiquebrainz/frontend/external/bookbrainz_db/author.py @@ -102,7 +102,7 @@ def fetch_multiple_authors(bbids: List[str]) -> dict: gender.name """), {'bbids': tuple(bbids)}) - authors = result.fetchall() + authors = result.mappings() results = {} for author in authors: author = dict(author) diff --git a/critiquebrainz/frontend/external/bookbrainz_db/common_entity.py b/critiquebrainz/frontend/external/bookbrainz_db/common_entity.py index 3fb797a3b..a68a8ae00 100644 --- a/critiquebrainz/frontend/external/bookbrainz_db/common_entity.py +++ b/critiquebrainz/frontend/external/bookbrainz_db/common_entity.py @@ -34,7 +34,7 @@ def get_authors_for_artist(artist_mbid) -> List: AND author.data_id IS NOT NULL GROUP BY bbid """), {'artist_mbid': artist_mbid, 'identifier_type': MB_ARTIST_IDENTIFIER_TYPE}) - authors = result.fetchall() + authors = result.mappings() author_bbids = [] for author in authors: @@ -76,7 +76,7 @@ def get_literary_works_for_work(work_mbid) -> List: GROUP BY bbid """), {'work_mbid': work_mbid, 'identifier_type': MB_WORK_IDENTIFIER_TYPE}) - literary_works = result.fetchall() + literary_works = result.mappings() work_bbids = [] for literary_work in literary_works: literary_work = dict(literary_work) diff --git a/critiquebrainz/frontend/external/bookbrainz_db/edition.py b/critiquebrainz/frontend/external/bookbrainz_db/edition.py index d04f009d2..e729a1f3e 100644 --- a/critiquebrainz/frontend/external/bookbrainz_db/edition.py +++ b/critiquebrainz/frontend/external/bookbrainz_db/edition.py @@ -79,7 +79,7 @@ def fetch_multiple_editions(bbids: List[str]) -> dict: format """), {'bbids': tuple(bbids)}) - editions = result.fetchall() + editions = result.mappings() results = {} for edition in editions: edition = dict(edition) diff --git a/critiquebrainz/frontend/external/bookbrainz_db/edition_group.py b/critiquebrainz/frontend/external/bookbrainz_db/edition_group.py index fe68b1f24..a9aba36be 100644 --- a/critiquebrainz/frontend/external/bookbrainz_db/edition_group.py +++ b/critiquebrainz/frontend/external/bookbrainz_db/edition_group.py @@ -78,7 +78,7 @@ def fetch_multiple_edition_groups(bbids: List[str]) -> dict: relationship_set_id """), {'bbids': tuple(bbids)}) - edition_groups = result.fetchall() + edition_groups = result.mappings() results = {} for edition_group in edition_groups: edition_group = dict(edition_group) @@ -123,7 +123,7 @@ def fetch_works_for_edition_group(bbid: uuid.UUID): AND rel.type_id = :relationship_type_id """), {'bbid': str(bbid), 'relationship_type_id': EDITION_WORK_CONTAINS_REL_ID}) - works = result.fetchall() + works = result.mappings() work_bbids = [] for work in works: diff --git a/critiquebrainz/frontend/external/bookbrainz_db/identifiers.py b/critiquebrainz/frontend/external/bookbrainz_db/identifiers.py index a88dcc8ae..b44460257 100644 --- a/critiquebrainz/frontend/external/bookbrainz_db/identifiers.py +++ b/critiquebrainz/frontend/external/bookbrainz_db/identifiers.py @@ -33,7 +33,7 @@ def fetch_bb_external_identifiers(identifier_set_id: int) -> List: LEFT JOIN identifier_type idtype on iden.type_id = idtype.id WHERE idens.set_id = :identifier_set_id """), {'identifier_set_id': identifier_set_id}) - identifiers = result.fetchall() + identifiers = result.mappings() identifiers = [dict(identifier) for identifier in identifiers] identifiers = process_bb_identifiers(identifiers) cache.set(bb_identifiers_key, identifiers, DEFAULT_CACHE_EXPIRATION) diff --git a/critiquebrainz/frontend/external/bookbrainz_db/literary_work.py b/critiquebrainz/frontend/external/bookbrainz_db/literary_work.py index b4cf2ac19..5a3cf10c5 100644 --- a/critiquebrainz/frontend/external/bookbrainz_db/literary_work.py +++ b/critiquebrainz/frontend/external/bookbrainz_db/literary_work.py @@ -103,7 +103,7 @@ def fetch_multiple_literary_works(bbids: List[str], work_type=None, limit=None, """.format(work_type_filter_string=work_type_filter_string) ), query_params) - literary_works = result.fetchall() + literary_works = result.mappings() results = {} for literary_work in literary_works: literary_work = dict(literary_work) @@ -144,7 +144,7 @@ def fetch_edition_groups_for_works(bbid: str) -> list: AND rel.type_id = :relationship_type_id """), {'bbid': str(bbid), 'relationship_type_id': EDITION_WORK_CONTAINS_REL_ID}) - edition_groups = result.fetchall() + edition_groups = result.mappings() edition_group_bbids = [] for edition_group in edition_groups: diff --git a/critiquebrainz/frontend/external/bookbrainz_db/publisher.py b/critiquebrainz/frontend/external/bookbrainz_db/publisher.py index 84f9b1f66..29589f303 100644 --- a/critiquebrainz/frontend/external/bookbrainz_db/publisher.py +++ b/critiquebrainz/frontend/external/bookbrainz_db/publisher.py @@ -70,7 +70,7 @@ def fetch_multiple_publishers(bbids: List[str]) -> dict: AND master = 't' """), {'bbids': tuple(bbids)}) - publishers = result.fetchall() + publishers = result.mappings() results = {} for publisher in publishers: publisher = dict(publisher) diff --git a/critiquebrainz/frontend/external/bookbrainz_db/redirects.py b/critiquebrainz/frontend/external/bookbrainz_db/redirects.py index cbb7b7d88..65c61db79 100644 --- a/critiquebrainz/frontend/external/bookbrainz_db/redirects.py +++ b/critiquebrainz/frontend/external/bookbrainz_db/redirects.py @@ -34,7 +34,7 @@ def get_redirected_bbid(bbid: str) -> str | None: FROM redirects """), {'bbid': bbid}) - redirect_bbids = result.fetchall() + redirect_bbids = result.mappings() results = [] for redirect_bbid in redirect_bbids: diff --git a/critiquebrainz/frontend/external/bookbrainz_db/relationships.py b/critiquebrainz/frontend/external/bookbrainz_db/relationships.py index 908fc36b8..9f9595a86 100644 --- a/critiquebrainz/frontend/external/bookbrainz_db/relationships.py +++ b/critiquebrainz/frontend/external/bookbrainz_db/relationships.py @@ -42,7 +42,7 @@ def fetch_relationships(relationship_set_id: int, relation_types_id: List) -> Li WHERE rels.set_id = :relationship_set_id AND reltype.id in :relation_types_id """), {'relationship_set_id': relationship_set_id, 'relation_types_id': tuple(relation_types_id)}) - relationships = result.fetchall() + relationships = result.mappings() relationships = [dict(relationship) for relationship in relationships] cache.set(key, relationships, DEFAULT_CACHE_EXPIRATION) diff --git a/critiquebrainz/frontend/external/bookbrainz_db/series.py b/critiquebrainz/frontend/external/bookbrainz_db/series.py index 94e0cc1b4..79806d2a6 100644 --- a/critiquebrainz/frontend/external/bookbrainz_db/series.py +++ b/critiquebrainz/frontend/external/bookbrainz_db/series.py @@ -74,7 +74,7 @@ def fetch_multiple_series(bbids: List[str]) -> dict: AND entity_type IS NOT NULL """), {'bbids': tuple(bbids)}) - data = result.fetchall() + data = result.mappings() results = {} for series in data: series = dict(series) diff --git a/docker/docker-compose.test.yml b/docker/docker-compose.test.yml index ca426ab67..fb042ff60 100644 --- a/docker/docker-compose.test.yml +++ b/docker/docker-compose.test.yml @@ -33,3 +33,4 @@ services: - critiquebrainz_redis environment: PGPASSWORD: critiquebrainz + SQLALCHEMY_WARN_20: 1 diff --git a/pytest.ini b/pytest.ini index d07376c90..534060909 100644 --- a/pytest.ini +++ b/pytest.ini @@ -1,3 +1,3 @@ [pytest] testpaths = critiquebrainz -addopts = --cov=critiquebrainz --no-cov-on-fail +addopts = --cov=critiquebrainz --no-cov-on-fail -W always::DeprecationWarning -W error::sqlalchemy.exc.Base20DeprecationWarning diff --git a/requirements.txt b/requirements.txt index b7f6773de..4d1d790e7 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,4 +1,4 @@ -brainzutils@git+https://github.com/metabrainz/brainzutils-python.git@v2.6.1 +brainzutils@git+https://github.com/metabrainz/brainzutils-python.git@v2.7.0 beautifulsoup4==4.8.0 click==8.1.3 Flask-Babel==2.0.0 @@ -30,8 +30,8 @@ sentry-sdk[flask]==1.5.8 redis==4.2.2 msgpack==0.5.6 requests==2.27.1 -SQLAlchemy==1.3.16 -mbdata@git+https://github.com/amCap1712/mbdata.git@upstream-schema-changes +SQLAlchemy==1.4.41 +mbdata@git+https://github.com/amCap1712/mbdata.git@fix-sqlalchemy-warnings sqlalchemy-dst==1.0.1 markupsafe==2.1.1 itsdangerous==2.1.2