From ca265f450266937e2220d6c1bcba3bf1f9ce3495 Mon Sep 17 00:00:00 2001 From: Ansh Goyal Date: Fri, 5 Aug 2022 09:09:32 +0000 Subject: [PATCH 1/5] feat: Recursively find redirected BBIDs --- .../external/bookbrainz_db/redirects.py | 49 +++++++++++++++++++ 1 file changed, 49 insertions(+) create mode 100644 critiquebrainz/frontend/external/bookbrainz_db/redirects.py diff --git a/critiquebrainz/frontend/external/bookbrainz_db/redirects.py b/critiquebrainz/frontend/external/bookbrainz_db/redirects.py new file mode 100644 index 000000000..f49a30c25 --- /dev/null +++ b/critiquebrainz/frontend/external/bookbrainz_db/redirects.py @@ -0,0 +1,49 @@ +import uuid +from brainzutils import cache +import sqlalchemy +import critiquebrainz.frontend.external.bookbrainz_db as db +from critiquebrainz.frontend.external.bookbrainz_db import DEFAULT_CACHE_EXPIRATION + + +def get_redirected_bbid(bbid: uuid.UUID) -> str: + """ + Get the redirected BBID for a given BBID. + Args: + bbid: The BBID to get the redirected BBID for. + Returns: + The redirected BBID. + Returns None if the BBID is not redirected. + """ + bb_redirect_key = cache.gen_key('redirects', bbid) + results = cache.get(bb_redirect_key) + + if not results: + with db.bb_engine.connect() as connection: + result = connection.execute(sqlalchemy.text(""" + WITH RECURSIVE redirects AS ( + SELECT source_bbid, target_bbid + FROM entity_redirect + WHERE source_bbid = :bbid + UNION + SELECT er.source_bbid, er.target_bbid + FROM entity_redirect er + INNER JOIN redirects rd + ON rd.target_bbid = er.source_bbid + ) + SELECT target_bbid::text + FROM redirects + """), {'bbid': bbid}) + + redirect_bbids = result.fetchall() + + results = [] + for redirect_bbid in redirect_bbids: + redirect_bbids = dict(redirect_bbid) + results.append(redirect_bbid['target_bbid']) + + cache.set(bb_redirect_key, results, DEFAULT_CACHE_EXPIRATION) + + if results: + return results[-1] + else: + return None From 93ff8fffc343386db3dcf4e0766eaee316488b68 Mon Sep 17 00:00:00 2001 From: Ansh Goyal Date: Fri, 5 Aug 2022 09:10:30 +0000 Subject: [PATCH 2/5] feat: Redirect user to redirected BBID's page --- .../frontend/external/bookbrainz_db/edition_group.py | 1 + critiquebrainz/frontend/views/bb_edition_group.py | 8 +++++++- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/critiquebrainz/frontend/external/bookbrainz_db/edition_group.py b/critiquebrainz/frontend/external/bookbrainz_db/edition_group.py index 8c8ddf20f..2bacfd285 100644 --- a/critiquebrainz/frontend/external/bookbrainz_db/edition_group.py +++ b/critiquebrainz/frontend/external/bookbrainz_db/edition_group.py @@ -68,6 +68,7 @@ def fetch_multiple_edition_groups(bbids: List[uuid.UUID]) -> dict: LEFT JOIN author_credit_name acn ON acn.author_credit_id = edition_group.author_credit_id WHERE bbid in :bbids AND master = 't' + AND data_id IS NOT NULL GROUP BY bbid, edition_group.name, sort_name, diff --git a/critiquebrainz/frontend/views/bb_edition_group.py b/critiquebrainz/frontend/views/bb_edition_group.py index ff04645e8..8dd519136 100644 --- a/critiquebrainz/frontend/views/bb_edition_group.py +++ b/critiquebrainz/frontend/views/bb_edition_group.py @@ -1,9 +1,10 @@ -from flask import Blueprint, render_template, request +from flask import Blueprint, render_template, request, redirect, url_for from flask_login import current_user from werkzeug.exceptions import NotFound, BadRequest from flask_babel import gettext import critiquebrainz.db.review as db_review import critiquebrainz.frontend.external.bookbrainz_db.edition_group as bb_edition_group +import critiquebrainz.frontend.external.bookbrainz_db.redirects as bb_redirects from critiquebrainz.frontend.forms.rate import RatingEditForm from critiquebrainz.frontend.views import get_avg_rating, EDITION_GROUP_REVIEWS_LIMIT @@ -14,6 +15,11 @@ @bb_edition_group_bp.route('/') def entity(id): id = str(id) + + redirected_bbid = bb_redirects.get_redirected_bbid(id) + if redirected_bbid: + return redirect(url_for('bb_edition_group.entity', id=redirected_bbid)) + edition_group = bb_edition_group.get_edition_group_by_bbid(id) if edition_group is None: From 171d6a04bf7c613784602c5ddfd8eadea9aac635 Mon Sep 17 00:00:00 2001 From: Ansh Goyal Date: Tue, 16 Aug 2022 10:30:32 +0000 Subject: [PATCH 3/5] feat: Check for redirects only when entity is not found in database --- critiquebrainz/frontend/views/bb_edition_group.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/critiquebrainz/frontend/views/bb_edition_group.py b/critiquebrainz/frontend/views/bb_edition_group.py index 8dd519136..19a1c11d4 100644 --- a/critiquebrainz/frontend/views/bb_edition_group.py +++ b/critiquebrainz/frontend/views/bb_edition_group.py @@ -16,13 +16,13 @@ def entity(id): id = str(id) - redirected_bbid = bb_redirects.get_redirected_bbid(id) - if redirected_bbid: - return redirect(url_for('bb_edition_group.entity', id=redirected_bbid)) - edition_group = bb_edition_group.get_edition_group_by_bbid(id) if edition_group is None: + redirected_bbid = bb_redirects.get_redirected_bbid(id) + if redirected_bbid: + return redirect(url_for('bb_edition_group.entity', id=redirected_bbid)) + raise NotFound(gettext("Sorry, we couldn't find an edition group with that BookBrainz ID.")) try: From 66db55f80564b324f9993659f0a1244c4f486a73 Mon Sep 17 00:00:00 2001 From: Ansh Goyal Date: Tue, 16 Aug 2022 12:41:59 +0000 Subject: [PATCH 4/5] feat: Add Tests for BB Redirects --- .../external/bookbrainz_db/redirects.py | 2 +- .../bookbrainz_db/test/bb_redirects_test.py | 26 +++++++++++++++++++ 2 files changed, 27 insertions(+), 1 deletion(-) create mode 100644 critiquebrainz/frontend/external/bookbrainz_db/test/bb_redirects_test.py diff --git a/critiquebrainz/frontend/external/bookbrainz_db/redirects.py b/critiquebrainz/frontend/external/bookbrainz_db/redirects.py index f49a30c25..cc77f3300 100644 --- a/critiquebrainz/frontend/external/bookbrainz_db/redirects.py +++ b/critiquebrainz/frontend/external/bookbrainz_db/redirects.py @@ -14,7 +14,7 @@ def get_redirected_bbid(bbid: uuid.UUID) -> str: The redirected BBID. Returns None if the BBID is not redirected. """ - bb_redirect_key = cache.gen_key('redirects', bbid) + bb_redirect_key = cache.gen_key('bb_redirects', bbid) results = cache.get(bb_redirect_key) if not results: diff --git a/critiquebrainz/frontend/external/bookbrainz_db/test/bb_redirects_test.py b/critiquebrainz/frontend/external/bookbrainz_db/test/bb_redirects_test.py new file mode 100644 index 000000000..343bde924 --- /dev/null +++ b/critiquebrainz/frontend/external/bookbrainz_db/test/bb_redirects_test.py @@ -0,0 +1,26 @@ +from critiquebrainz.frontend.external.bookbrainz_db import redirects +from critiquebrainz.data.testing import DataTestCase + + +class BBRedirectsTestCase(DataTestCase): + + def setUp(self): + super(BBRedirectsTestCase, self).setUp() + self.bbid1 = '63a40e3d-54ff-4549-9637-24959ad89241' + self.bbid2 = 'dd40b465-931f-46ee-b2ae-28685b19f8d8' + self.bbid3 = 'e5c4e68b-bfce-4c77-9ca2-0f0a2d4d09f0' + + def test_single_bb_redirects(self): + # Test single redirect + redirected_bbid = redirects.get_redirected_bbid(self.bbid1) + self.assertEqual(redirected_bbid, 'ecdeb45d-c432-4347-94c3-f01acc799d4a') + + def test_multiple_bb_redirects(self): + # Test multiple redirects + redirected_bbid = redirects.get_redirected_bbid(self.bbid2) + self.assertEqual(redirected_bbid, '7e691222-6f78-4ad6-ad5d-1a671a319fbd') + + def test_no_bb_redirects(self): + # Test no redirects + redirected_bbid = redirects.get_redirected_bbid(self.bbid3) + self.assertEqual(redirected_bbid, None) From 7a591666329e500492fd496fc5d616acf4be42c4 Mon Sep 17 00:00:00 2001 From: Ansh Goyal Date: Tue, 16 Aug 2022 14:55:47 +0000 Subject: [PATCH 5/5] feat: Handle redirects for author, series and literary works --- critiquebrainz/frontend/views/bb_author.py | 6 +++++- critiquebrainz/frontend/views/bb_literary_work.py | 7 ++++++- critiquebrainz/frontend/views/bb_series.py | 7 ++++++- 3 files changed, 17 insertions(+), 3 deletions(-) diff --git a/critiquebrainz/frontend/views/bb_author.py b/critiquebrainz/frontend/views/bb_author.py index 20f075f43..5b9e64018 100644 --- a/critiquebrainz/frontend/views/bb_author.py +++ b/critiquebrainz/frontend/views/bb_author.py @@ -1,11 +1,12 @@ from datetime import datetime -from flask import Blueprint, render_template, request +from flask import Blueprint, render_template, request, redirect, url_for from flask_login import current_user from werkzeug.exceptions import NotFound, BadRequest from flask_babel import gettext import critiquebrainz.db.review as db_review import critiquebrainz.frontend.external.bookbrainz_db.author as bb_author import critiquebrainz.frontend.external.bookbrainz_db.literary_work as bb_literary_work +import critiquebrainz.frontend.external.bookbrainz_db.redirects as bb_redirects from critiquebrainz.frontend.forms.rate import RatingEditForm from critiquebrainz.frontend.views import get_avg_rating, AUTHOR_REVIEWS_LIMIT @@ -24,6 +25,9 @@ def entity(id): author = bb_author.get_author_by_bbid(id) if author is None: + redirected_bbid = bb_redirects.get_redirected_bbid(id) + if redirected_bbid: + return redirect(url_for('bb_author.entity', id=redirected_bbid)) raise NotFound(gettext("Sorry, we couldn't find an author with that BookBrainz ID.")) literary_work_type = request.args.get('literary_work_type') diff --git a/critiquebrainz/frontend/views/bb_literary_work.py b/critiquebrainz/frontend/views/bb_literary_work.py index 1b5c3409a..b77cea87b 100644 --- a/critiquebrainz/frontend/views/bb_literary_work.py +++ b/critiquebrainz/frontend/views/bb_literary_work.py @@ -1,9 +1,10 @@ -from flask import Blueprint, render_template, request +from flask import Blueprint, render_template, request, redirect, url_for from flask_login import current_user from werkzeug.exceptions import NotFound, BadRequest from flask_babel import gettext import critiquebrainz.db.review as db_review import critiquebrainz.frontend.external.bookbrainz_db.literary_work as bb_literary_work +import critiquebrainz.frontend.external.bookbrainz_db.redirects as bb_redirects from critiquebrainz.frontend.forms.rate import RatingEditForm from critiquebrainz.frontend.views import get_avg_rating, LITERARY_WORK_REVIEWS_LIMIT @@ -16,6 +17,10 @@ def entity(id): literary_work = bb_literary_work.get_literary_work_by_bbid(id) if literary_work is None: + redirected_bbid = bb_redirects.get_redirected_bbid(id) + if redirected_bbid: + return redirect(url_for('bb_literary_work.entity', id=redirected_bbid)) + raise NotFound(gettext("Sorry, we couldn't find a work with that BookBrainz ID.")) try: diff --git a/critiquebrainz/frontend/views/bb_series.py b/critiquebrainz/frontend/views/bb_series.py index 239d8d136..fbb01a3eb 100644 --- a/critiquebrainz/frontend/views/bb_series.py +++ b/critiquebrainz/frontend/views/bb_series.py @@ -1,9 +1,10 @@ -from flask import Blueprint, render_template, request +from flask import Blueprint, render_template, request, redirect, url_for from flask_login import current_user from werkzeug.exceptions import NotFound, BadRequest from flask_babel import gettext import critiquebrainz.db.review as db_review import critiquebrainz.frontend.external.bookbrainz_db.series as bb_series +import critiquebrainz.frontend.external.bookbrainz_db.redirects as bb_redirects from critiquebrainz.frontend.forms.rate import RatingEditForm from critiquebrainz.frontend.views import get_avg_rating, SERIES_REVIEWS_LIMIT @@ -16,6 +17,10 @@ def entity(id): series = bb_series.get_series_by_bbid(id) if series is None: + redirected_bbid = bb_redirects.get_redirected_bbid(id) + if redirected_bbid: + return redirect(url_for('bb_series.entity', id=redirected_bbid)) + raise NotFound(gettext("Sorry, we couldn't find a series with that BookBrainz ID.")) try: