Merge branch 'master' into CB-448-new-footer

Dockerfile

@@ -1,4 +1,4 @@
-FROM metabrainz/python:3.10-20220315 as critiquebrainz-base
+FROM metabrainz/python:3.11-20231006 as critiquebrainz-base
 
 ENV PYTHONUNBUFFERED 1
 
@@ -39,7 +39,7 @@ RUN mkdir -p /etc/apt/keyrings \
 
 RUN pip install --upgrade pip==21.0.1
 
-RUN pip install --no-cache-dir uWSGI==2.0.20
+RUN pip install --no-cache-dir uWSGI==2.0.23
 
 RUN mkdir /code
 WORKDIR /code

admin/sql/clear_tables.sql

@@ -0,0 +1,13 @@
+DELETE FROM comment_revision;
+DELETE FROM comment;
+DELETE FROM vote;
+DELETE FROM spam_report;
+DELETE FROM revision;
+DELETE FROM oauth_grant;
+DELETE FROM oauth_token;
+DELETE FROM oauth_client;
+DELETE FROM moderation_log;
+DELETE FROM review;
+DELETE FROM "user";
+DELETE FROM license;
+DELETE FROM avg_rating;

critiquebrainz/data/dump_manager.py

@@ -9,9 +9,10 @@
 from time import gmtime, strftime
 
 import click
+import orjson
 import sqlalchemy
 from flask import current_app, jsonify
-from flask.json import JSONEncoder
+from flask.json.provider import JSONProvider
 from psycopg2.sql import SQL, Identifier
 
 from critiquebrainz import db
@@ -147,7 +148,7 @@ def json(location, rotate=False):
     """
     create_path(location)
 
-    current_app.json_encoder = DumpJSONEncoder
+    current_app.json_encoder = OrJSONProvider
 
     print("Creating new archives...")
     with db.engine.begin() as connection:
@@ -541,16 +542,9 @@ def reset_sequence(table_names):
         connection.close()
 
 
-class DumpJSONEncoder(JSONEncoder):
-    """Custom JSON encoder for database dumps."""
+class OrJSONProvider(JSONProvider):
+    def dumps(self, obj, **kwargs):
+        return orjson.dumps(obj).decode()
 
-    def default(self, o):  # pylint: disable=method-hidden
-        try:
-            if isinstance(o, datetime):
-                return o.isoformat()
-            iterable = iter(o)
-        except TypeError:
-            pass
-        else:
-            return list(iterable)
-        return JSONEncoder.default(self, o)
+    def loads(self, s, **kwargs):
+        return orjson.loads(s)

critiquebrainz/data/testing.py

@@ -1,26 +1,5 @@
-import os
+from critiquebrainz.frontend.testing import FrontendTestCase
 
-from flask_testing import TestCase
 
-from critiquebrainz.data.utils import create_all, drop_tables, drop_types
-from critiquebrainz.frontend import create_app
-
-
-class DataTestCase(TestCase):
-
-    def create_app(self):
-        app = create_app(config_path=os.path.join(os.path.dirname(os.path.realpath(__file__)), '..', '..', 'test_config.py'))
-        return app
-
-    def setUp(self):
-        self.reset_db()
-        # TODO(roman): Add stuff form fixtures.
-
-    def tearDown(self):
-        pass
-
-    @staticmethod
-    def reset_db():
-        drop_tables()
-        drop_types()
-        create_all()
+class DataTestCase(FrontendTestCase):
+    pass

critiquebrainz/data/utils.py

@@ -30,6 +30,10 @@ def drop_types():
     db.run_sql_script(os.path.join(ADMIN_SQL_DIR, 'drop_types.sql'))
 
 
+def clear_tables():
+    db.run_sql_script(os.path.join(ADMIN_SQL_DIR, 'clear_tables.sql'))
+
+
 def explode_db_uri(uri):
     """Extracts database connection info from the URI.
 

critiquebrainz/db/users.py

@@ -4,7 +4,6 @@
 import sqlalchemy
 
 from critiquebrainz import db
-from critiquebrainz.db import revision as db_revision
 
 
 USER_GET_COLUMNS = [
@@ -305,6 +304,7 @@ def list_users(limit=None, offset=0):
         result = connection.execute(sqlalchemy.text("""
             SELECT {columns}
               FROM "user"
+          ORDER BY musicbrainz_row_id
              LIMIT :limit
             OFFSET :offset
         """.format(columns=','.join(USER_GET_COLUMNS))), {
@@ -357,6 +357,7 @@ def has_voted(user_id, review_id):
     Returns:
         (bool): True if has voted else False.
     """
+    from critiquebrainz.db import revision as db_revision
     last_revision = db_revision.get(review_id, limit=1)[0]
     with db.engine.connect() as connection:
         result = connection.execute(sqlalchemy.text("""

critiquebrainz/frontend/babel.py

@@ -3,8 +3,6 @@
 
 
 def init_app(app, domain='messages'):
-    babel = Babel(app, default_domain=domain)
-
     app.config['LANGUAGES'] = {}
     for language in app.config['SUPPORTED_LANGUAGES']:
         app.config['LANGUAGES'][language] = Locale.parse(language).language_name
@@ -21,7 +19,6 @@ def after_this_request(f):
         g.after_request_callbacks.append(f)
         return f
 
-    @babel.localeselector
     def get_locale():  # pylint: disable=unused-variable
         supported_languages = app.config['SUPPORTED_LANGUAGES']
         language_arg = request.args.get('l')
@@ -38,3 +35,6 @@ def remember_language(response):  # pylint: disable=unused-variable
                 return language_cookie
 
         return request.accept_languages.best_match(supported_languages)
+
+    babel = Babel()
+    babel.init_app(app, default_domain=domain, locale_selector=get_locale)

critiquebrainz/frontend/external/__init__.py

@@ -6,10 +6,10 @@
 
 See documentation in each module for information about usage.
 """
-from flask import current_app, _app_ctx_stack
-
+from flask import current_app, g
 from critiquebrainz.frontend.external.entities import get_entity_by_id, get_multiple_entities
 
+
 class MBDataAccess(object):
     """A data access object which switches between database get methods or development versions
     This is useful because we won't show a review if we cannot find its metadata in the
@@ -37,19 +37,17 @@ def init_app(self, app):
 
     @property
     def get_entity_by_id(self):
-        ctx = _app_ctx_stack.top
-        if ctx is not None:
-            if not hasattr(ctx, 'get_entity_by_id'):
-                ctx.get_entity_by_id = self.get_entity_by_id_method
-            return ctx.get_entity_by_id
+        if not hasattr(g, 'get_entity_by_id'):
+            g.get_entity_by_id = self.get_entity_by_id_method
+        return g.get_entity_by_id
 
     @property
     def get_multiple_entities(self):
-        ctx = _app_ctx_stack.top
-        if ctx is not None:
-            if not hasattr(ctx, 'get_multiple_entities'):
-                ctx.get_multiple_entities = self.get_multiple_entities_method
-            return ctx.get_multiple_entities
+        if not hasattr(g, 'get_multiple_entities'):
+            g.get_multiple_entities = self.get_multiple_entities_method
+        return g.get_multiple_entities
+
+
 
 mbstore = MBDataAccess()
 
@@ -65,6 +63,7 @@ def development_get_multiple_entities(entities):
         data[mbid] = get_dummy_item(mbid, entity_type)
     return data
 
+
 def development_get_entity_by_id(entity_id, entity_type):
     """Same as get_entity_by_id, but always returns a dummy item if the requested entity
        isn't in the MusicBrainz database. Used in development with a sample database."""

critiquebrainz/frontend/external/bookbrainz_db/test/literary_work_test.py

@@ -15,7 +15,7 @@ def test_get_literary_work_by_bbid(self):
         literary_work_info = literary_work.get_literary_work_by_bbid(self.bbid1)
         self.assertEqual(literary_work_info["bbid"], self.bbid1)
         self.assertEqual(literary_work_info["name"], "Assassin's Creed: Brotherhood")
-        self.assertEqual(literary_work_info["sort_name"], "Brotherhood, Assassin's Creed:")
+        self.assertEqual(literary_work_info["sort_name"], "Assassin's Creed: Brotherhood")
         self.assertEqual(literary_work_info["work_type"], "Novel")
 
     def test_fetch_multiple_literary_works(self):

critiquebrainz/frontend/forms/review.py

@@ -47,8 +47,8 @@ def __init__(self, default_license_id='CC BY-SA 3.0', default_language='en', **k
         kwargs.setdefault('language', default_language)
         FlaskForm.__init__(self, **kwargs)
 
-    def validate(self):
-        if not super(ReviewEditForm, self).validate():
+    def validate(self, extra_validators=None):
+        if not super(ReviewEditForm, self).validate(extra_validators):
             return False
         if not self.text.data and not self.rating.data:
             self.text.errors.append("You must provide some text or a rating to complete this review.")

critiquebrainz/frontend/testing.py

@@ -1,34 +1,17 @@
 import os
-from critiquebrainz.ws.oauth import oauth
-
-from flask_testing import TestCase
 
-from critiquebrainz.data.utils import create_all, drop_tables, drop_types
 from critiquebrainz.frontend import create_app
+from critiquebrainz.testing import ServerTestCase
+from critiquebrainz.ws.oauth import oauth
 
 
-class FrontendTestCase(TestCase):
+class FrontendTestCase(ServerTestCase):
 
-    def create_app(self):
-        app = create_app(config_path=os.path.join(os.path.dirname(os.path.realpath(__file__)), '..', '..', 'test_config.py'))
+    @classmethod
+    def create_app(cls):
+        app = create_app(
+            config_path=os.path.join(os.path.dirname(os.path.realpath(__file__)), '..', '..', 'test_config.py')
+        )
         oauth.init_app(app)
+        app.config['TESTING'] = True
         return app
-
-    def setUp(self):
-        self.reset_db()
-        # TODO(roman): Add stuff form fixtures.
-
-    def tearDown(self):
-        pass
-
-    @staticmethod
-    def reset_db():
-        drop_tables()
-        drop_types()
-        create_all()
-
-    def temporary_login(self, user):
-        """Based on: http://stackoverflow.com/a/16238537."""
-        with self.client.session_transaction() as session:
-            session['_user_id'] = user.id
-            session['_fresh'] = True

critiquebrainz/frontend/views/profile.py

@@ -20,7 +20,7 @@ def edit():
             "license_choice": form.license_choice.data,
         })
         flash.success(gettext("Profile updated."))
-        return redirect(url_for('user.reviews', user_ref= current_user.user_ref))
+        return redirect(url_for('user.reviews', user_ref=current_user.user_ref))
 
     form.display_name.data = current_user.display_name
     form.email.data = current_user.email

critiquebrainz/frontend/views/review.py

@@ -1,6 +1,5 @@
 from math import ceil
 
-from brainzutils.musicbrainz_db.exceptions import NoDataFoundException
 from flask import Blueprint, render_template, request, redirect, url_for, jsonify
 from flask_babel import gettext, get_locale, lazy_gettext
 from flask_login import login_required, current_user

critiquebrainz/frontend/views/test/test_artist.py

@@ -1,9 +1,9 @@
 from unittest import mock
 
-from critiquebrainz.db.user import User
 from critiquebrainz.frontend.testing import FrontendTestCase
 from critiquebrainz.db import users as db_users
 
+
 def return_release_groups(*, artist_id, release_types=None, limit=None, offset=None):
     # pylint: disable=unused-argument
     if release_types == ['ep']:
@@ -30,6 +30,7 @@ def return_release_groups(*, artist_id, release_types=None, limit=None, offset=N
 class ArtistViewsTestCase(FrontendTestCase):
 
     def setUp(self):
+        from critiquebrainz.db.user import User
         super(ArtistViewsTestCase, self).setUp()
         self.reviewer = User(db_users.get_or_create(
             1, "Reviewer", new_user_data={"display_name": u"Reviewer"}

critiquebrainz/frontend/views/test/test_bb_author.py

@@ -6,6 +6,7 @@
 from critiquebrainz.db.user import User
 from critiquebrainz.frontend.testing import FrontendTestCase
 
+
 class AuthorViewsTestCase(FrontendTestCase):
 
     def setUp(self):

critiquebrainz/frontend/views/test/test_comment.py

@@ -96,13 +96,20 @@ def test_create(self):
 
         # blocked user should not be allowed to comment
         db_users.block(self.commenter.id)
+        # because the g global persists for entire duration of the test, we need to manually logout/login
+        # the user again for the current_user to be refreshed. in production, this would happen automatically
+        # as the current_user is loaded at the start of each request/request context.
+        self.temporary_login(self.commenter)
+
         response = self.client.post(
             url_for("comment.create"),
             data=payload,
             follow_redirects=True,
         )
         self.assertIn("You are not allowed to write new comments", str(response.data))
+
         db_users.unblock(self.commenter.id)
+        self.temporary_login(self.commenter)
 
         # comment with some text and a valid review_id must be saved
         response = self.client.post(

critiquebrainz/frontend/views/test/test_oauth.py

@@ -9,6 +9,7 @@
 
 class OauthTestCase(FrontendTestCase):
     def setUp(self):
+        super().setUp()
         from critiquebrainz.db.user import User
         self.user = User(db_users.get_or_create(2, "9371e5c7-5995-4471-a5a9-33481f897f9c", new_user_data={
             "display_name": u"User",

critiquebrainz/frontend/views/test/test_profile.py

@@ -31,6 +31,14 @@ def test_edit(self):
         response = self.client.post('/profile/edit', data=data,
                                     query_string=data, follow_redirects=True)
         self.assert200(response)
+
+        # because the g global persists for entire duration of the test, we need to manually logout/login
+        # the user again for the current_user to be refreshed. in production, this would happen automatically
+        # as the current_user is loaded at the start of each request/request context.
+        self.temporary_login(self.user)
+
+        response = self.client.get(f'/user/{self.user.id}')
+        self.assert200(response)
         self.assertIn(data['display_name'], str(response.data))
 
     def test_delete(self):

critiquebrainz/frontend/views/test/test_review.py

@@ -2,7 +2,7 @@
 from unittest.mock import patch
 from urllib.parse import urlparse
 
-from flask import current_app, url_for
+from flask import url_for
 
 import critiquebrainz.db.license as db_license
 import critiquebrainz.db.review as db_review

critiquebrainz/frontend/views/test/test_user.py

@@ -12,7 +12,9 @@ def setUp(self):
         self.user = User(db_users.get_or_create(1, "Tester", new_user_data={
             "display_name": u"Tester",
         }))
-        self.hacker = User(db_users.create(musicbrainz_row_id = 2, display_name = u"Hacker"))
+        self.hacker = User(db_users.get_or_create(2, "Hacker", new_user_data={
+            "display_name": u"Hacker",
+        }))
         self.admin = User(db_users.get_or_create(3, "Admin", new_user_data={
             "display_name": u"Admin",
         }))