Merge branch 'master' of https://github.com/metabrainz/critiquebrainz into CB-427

Author: anshg1214

.github/workflows/tests.yml

@@ -11,6 +11,9 @@ jobs:
   test:
     name: Run test suite
     runs-on: ubuntu-latest
+    permissions:
+      checks: write
+      pull-requests: write
 
     steps:
     - uses: actions/checkout@v2
@@ -22,21 +25,11 @@ jobs:
       run: echo ${{ secrets.DOCKER_HUB_PASSWORD }} | docker login -u ${{ secrets.DOCKER_HUB_USERNAME }} --password-stdin
       continue-on-error: true
 
-    - name: Pull docker images
-      run: docker-compose -f docker/docker-compose.test.yml pull
-
-    - uses: satackey/action-docker-layer-caching@v0.0.11
-      continue-on-error: true
-      with:
-        key: critiquebrainz-prod-image-{hash}
-        restore-keys: |
-          critiquebrainz-prod-image-
-
     - name: Run tests
       run: ./test.sh
 
     - name: Publish Test Results
-      uses: EnricoMi/publish-unit-test-result-action@v1.11
+      uses: EnricoMi/publish-unit-test-result-action@v2
       if: ${{ always() }}
       with:
         files: reports/tests.xml
@@ -53,8 +46,5 @@ jobs:
         run: echo ${{ secrets.DOCKER_HUB_PASSWORD }} | docker login -u ${{ secrets.DOCKER_HUB_USERNAME }} --password-stdin
         continue-on-error: true
 
-      - uses: satackey/action-docker-layer-caching@v0.0.11
-        continue-on-error: true
-
       - name: Build production image
         run: docker build --build-arg GIT_COMMIT_SHA=HEAD .

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
 
@@ -29,13 +29,17 @@ RUN apt-get update \
     && rm -rf /var/lib/apt/lists/*
 
 # Node
-RUN curl -sL https://deb.nodesource.com/setup_16.x | bash - \
-   && apt-get install -y nodejs \
-   && rm -rf /var/lib/apt/lists/*
+ARG NODE_MAJOR=20
+RUN mkdir -p /etc/apt/keyrings \
+    && curl -fsSL https://deb.nodesource.com/gpgkey/nodesource-repo.gpg.key | gpg --dearmor -o /etc/apt/keyrings/nodesource.gpg \
+    && echo "deb [signed-by=/etc/apt/keyrings/nodesource.gpg] https://deb.nodesource.com/node_$NODE_MAJOR.x nodistro main" | tee /etc/apt/sources.list.d/nodesource.list \
+    && apt-get update \
+    && apt-get install -y nodejs \
+    && rm -rf /var/lib/apt/lists/*
 
 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

Dockerfile.webpack

@@ -1,4 +1,4 @@
-FROM node:16
+FROM node:20
 
 RUN mkdir /code
 WORKDIR /code

admin/schema_changes/23.sql

@@ -0,0 +1,3 @@
+BEGIN;
+CREATE INDEX ix_user_id ON "user" USING btree ((id::text));
+COMMIT;

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;

admin/sql/create_indexes.sql

@@ -3,5 +3,6 @@ BEGIN;
 CREATE INDEX ix_oauth_grant_code ON oauth_grant USING btree (code);
 CREATE INDEX ix_review_entity_id ON review USING btree (entity_id);
 CREATE INDEX ix_revision_review_id ON revision USING btree (review_id);
+CREATE INDEX ix_user_id ON "user" USING btree ((id::text));
 
 COMMIT;

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:
@@ -450,7 +451,26 @@ def importer(archive):
     with tarfile.open(archive, 'r:bz2') as archive:
         # TODO(roman): Read data from the archive without extracting it into temporary directory.
         temp_dir = tempfile.mkdtemp()
-        archive.extractall(temp_dir)
+        def is_within_directory(directory, target):
+            
+            abs_directory = os.path.abspath(directory)
+            abs_target = os.path.abspath(target)
+        
+            prefix = os.path.commonprefix([abs_directory, abs_target])
+            
+            return prefix == abs_directory
+        
+        def safe_extract(tar, path=".", members=None, *, numeric_owner=False):
+        
+            for member in tar.getmembers():
+                member_path = os.path.join(path, member.name)
+                if not is_within_directory(path, member_path):
+                    raise Exception("Attempted Path Traversal in Tar File")
+        
+            tar.extractall(path, members, numeric_owner=numeric_owner) 
+            
+        
+        safe_extract(archive, temp_dir)
 
         # Verifying schema version
         try:
@@ -522,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

@@ -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'))
@@ -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/__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

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))

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)
 
 
@@ -88,6 +88,7 @@ def get_by_id(comment_id):
                    "user".created as user_created,
                    "user".display_name,
                    "user".musicbrainz_id,
+                   COALESCE("user".musicbrainz_id, "user".id::text) as user_ref,
                    "user".is_blocked
               FROM comment c
               JOIN comment_revision cr ON c.id = cr.comment_id
@@ -99,9 +100,9 @@ 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))
+            raise db_exceptions.NoDataFoundException('Can’t find comment with ID: {id}'.format(id=comment_id))
 
         comment = dict(comment)
         comment['last_revision'] = {
@@ -115,6 +116,7 @@ def get_by_id(comment_id):
             'email': comment.pop('email'),
             'created': comment.pop('user_created'),
             'musicbrainz_username': comment.pop('musicbrainz_id'),
+            'user_ref': comment.pop('user_ref'),
             'is_blocked': comment.pop('is_blocked')
         })
         return comment
@@ -167,6 +169,7 @@ def list_comments(*, review_id=None, user_id=None, limit=20, offset=0, inc_hidde
                    "user".created as user_created,
                    "user".display_name,
                    "user".musicbrainz_id,
+                   COALESCE("user".musicbrainz_id, "user".id::text) as user_ref,
                    "user".is_blocked,
                    MIN(comment_revision.timestamp) as created,
                    latest_revision.id as last_revision_id,
@@ -196,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'),
@@ -208,6 +211,7 @@ def list_comments(*, review_id=None, user_id=None, limit=20, offset=0, inc_hidde
                 'display_name': row.pop('display_name'),
                 'is_blocked': row.pop('is_blocked'),
                 'musicbrainz_username': row.pop('musicbrainz_id'),
+                'user_ref': row.pop('user_ref'),
                 'email': row.pop('email'),
                 'created': row.pop('user_created'),
             })
@@ -221,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
@@ -262,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):
@@ -313,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