Merge pull request #398 from KSP-SpaceDock/beta

Author: DasSkelett

.github/workflows/mypy.yml

@@ -1,6 +1,8 @@
 name: Mypy
 
-on: [push, pull_request]
+on:
+- push
+- pull_request
 
 jobs:
   build:
@@ -15,6 +17,7 @@ jobs:
     - name: Install Dependencies
       run: |
         pip install mypy
+        pip install -r requirements-tests.txt
     - name: mypy
       run: |
         mypy KerbalStuff alembic/versions

.github/workflows/pytest.yml

@@ -1,6 +1,8 @@
 name: PyTest
 
-on: [push, pull_request]
+on:
+- push
+- pull_request
 
 jobs:
   build:

.gitignore

@@ -18,6 +18,7 @@ alembic.ini
 /spacedock-db
 /static
 content/
+.bash_history
 
 venv/
 build/

KerbalStuff/app.py

@@ -16,6 +16,7 @@
 from flaskext.markdown import Markdown
 from sqlalchemy import desc
 from werkzeug.exceptions import HTTPException, InternalServerError
+from jinja2 import ChainableUndefined
 
 from .blueprints.accounts import accounts
 from .blueprints.admin import admin
@@ -26,6 +27,7 @@
 from .blueprints.login_oauth import list_defined_oauths, login_oauth
 from .blueprints.mods import mods
 from .blueprints.profile import profiles
+from .middleware.session_interface import OnlyLoggedInSessionInterface
 from .celery import update_from_github
 from .common import first_paragraphs, many_paragraphs, json_output, jsonify_exception, dumb_object, sanitize_text
 from .config import _cfg, _cfgb, _cfgd, _cfgi, site_logger
@@ -49,11 +51,14 @@
         SESSION_COOKIE_SECURE=True,
         REMEMBER_COOKIE_SECURE=True
     )
+    # Render None and any accesses of its properties and sub-properties as the empty string instead of throwing exceptions
+    app.jinja_env.undefined = ChainableUndefined
 app.jinja_env.filters['first_paragraphs'] = first_paragraphs
 app.jinja_env.filters['bleach'] = sanitize_text
 app.jinja_env.auto_reload = app.debug
 app.secret_key = _cfg("secret-key")
 app.json_encoder = CustomJSONEncoder
+app.session_interface = OnlyLoggedInSessionInterface()
 Markdown(app, extensions=[KerbDown(), 'fenced_code'])
 login_manager = LoginManager(app)
 
@@ -154,7 +159,7 @@ def handle_generic_exception(e: Union[Exception, HTTPException]) -> Union[Tuple[
     site_logger.exception(e)
     try:
         db.rollback()
-        db.close()
+        # Session will be closed in app.teardown_request so templates can be rendered
     except:
         pass
 
@@ -166,13 +171,18 @@ def handle_generic_exception(e: Union[Exception, HTTPException]) -> Union[Tuple[
     else:
         if not isinstance(e, HTTPException):
             # Create an HTTPException so it has a code, name and description which we access in the template.
-            # We deliberately loose the original message here because it can contain confidential data.
+            # We deliberately lose the original message here because it can contain confidential data.
             e = InternalServerError()
         if e.description == werkzeug.exceptions.InternalServerError.description:
             e.description = "Clearly you've broken something. Maybe if you refresh no one will notice."
         return render_template("error_5XX.html", error=e), e.code or 500
 
 
+@app.teardown_request
+def teardown_request(exception: Optional[Exception]) -> None:
+    db.close()
+
+
 # I am unsure if this function is still needed or rather, if it still works.
 # TODO(Thomas): Investigate and remove
 @app.route('/ksp-profile-proxy/<fragment>')
@@ -280,7 +290,9 @@ def inject() -> Dict[str, Any]:
     if request.cookies.get('dismissed_donation') is not None:
         dismissed_donation = True
     return {
-        'announcements': get_announcement_posts(),
+        'announcements': (get_all_announcement_posts()
+                          if current_user
+                          else get_non_member_announcement_posts()),
         'many_paragraphs': many_paragraphs,
         'analytics_id': _cfg("google_analytics_id"),
         'analytics_domain': _cfg("google_analytics_domain"),
@@ -300,6 +312,7 @@ def inject() -> Dict[str, Any]:
         'url_for': url_for,
         'strftime': strftime,
         'site_name': _cfg('site-name'),
+        'caption': _cfg('caption'),
         'support_mail': _cfg('support-mail'),
         'source_code': _cfg('source-code'),
         'support_channels': _cfgd('support-channels'),
@@ -309,5 +322,11 @@ def inject() -> Dict[str, Any]:
     }
 
 
-def get_announcement_posts() -> List[BlogPost]:
-    return BlogPost.query.filter(BlogPost.announcement == True).order_by(desc(BlogPost.created)).all()
+def get_all_announcement_posts() -> List[BlogPost]:
+    return BlogPost.query.filter(BlogPost.announcement).order_by(desc(BlogPost.created)).all()
+
+
+def get_non_member_announcement_posts() -> List[BlogPost]:
+    return BlogPost.query.filter(
+        BlogPost.announcement, BlogPost.members_only != True
+    ).order_by(desc(BlogPost.created)).all()

KerbalStuff/blueprints/accounts.py

@@ -17,7 +17,7 @@
 from ..objects import Mod, User
 from ..search import get_mod_score
 
-accounts = Blueprint('accounts', __name__, template_folder='../../templates/accounts')
+accounts = Blueprint('accounts', __name__)
 
 
 @accounts.route("/register", methods=['GET', 'POST'])

KerbalStuff/blueprints/admin.py

@@ -17,7 +17,7 @@
 from ..email import send_bulk_email
 from ..objects import Mod, GameVersion, Game, Publisher, User
 
-admin = Blueprint('admin', __name__, template_folder='../../templates/admin')
+admin = Blueprint('admin', __name__)
 ITEMS_PER_PAGE = 10
 
 

KerbalStuff/blueprints/anonymous.py

@@ -1,17 +1,18 @@
 import os.path
 
 import werkzeug.wrappers
-from flask import Blueprint, render_template, send_from_directory, abort, request, Response
+from flask import Blueprint, render_template, abort, request, Response, make_response, send_file
 from flask_login import current_user
 from sqlalchemy import desc
+from datetime import timezone
 
 from ..common import dumb_object, paginate_query, get_paginated_mods, get_game_info, get_games, \
-    get_featured_mods, get_top_mods, get_new_mods, get_updated_mods
+    get_featured_mods, get_top_mods, get_new_mods, get_updated_mods, sendfile
 from ..config import _cfg
 from ..database import db
 from ..objects import Featured, Mod, ModVersion, User
 
-anonymous = Blueprint('anonymous', __name__, template_folder='../../templates/anonymous')
+anonymous = Blueprint('anonymous', __name__)
 
 
 @anonymous.route("/")
@@ -44,9 +45,10 @@ def game(gameshort: str) -> str:
 @anonymous.route("/content/<path:path>")
 def content(path: str) -> werkzeug.wrappers.Response:
     storage = _cfg('storage')
-    if not storage or not os.path.isfile(os.path.join(storage, path)):
+    if not storage:
         abort(404)
-    return send_from_directory(storage + "/", path)
+
+    return sendfile(path, True)
 
 
 @anonymous.route("/browse")
@@ -114,18 +116,19 @@ def browse_featured() -> str:
 
 @anonymous.route("/browse/featured.rss")
 def browse_featured_rss() -> Response:
-    mods = get_featured_mods(None, 30)
-    # Fix dates
-    for f in mods:
-        f.mod.created = f.created
-    mods = [dumb_object(f.mod) for f in mods]
-    db.rollback()
     site_name = _cfg('site-name')
     if not site_name:
         abort(404)
+    mods = []
+    for fm in get_featured_mods(None, 30):
+        # Add each mod but with created set to when it was featured
+        fmod = dumb_object(fm.mod)
+        fmod['created'] = fm.created.astimezone(timezone.utc)
+        mods.append(fmod)
     return Response(render_template("rss.xml", mods=mods, title="Featured mods on " + site_name,
                                     description="Featured mods on " + site_name,
-                                    url="/browse/featured"), mimetype="text/xml")
+                                    url="/browse/featured"),
+                    mimetype="text/xml")
 
 
 @anonymous.route("/browse/all")
@@ -162,7 +165,8 @@ def singlegame_browse_new_rss(gameshort: str) -> Response:
     mods = get_new_mods(ga.id, 30)
     return Response(render_template("rss.xml", mods=mods, title="New mods on " + site_name, ga=ga,
                                     description="The newest mods on " + site_name,
-                                    url="/browse/new"), mimetype="text/xml")
+                                    url="/browse/new"),
+                    mimetype="text/xml")
 
 
 @anonymous.route("/<gameshort>/browse/updated")
@@ -184,7 +188,8 @@ def singlegame_browse_updated_rss(gameshort: str) -> Response:
     return Response(render_template("rss.xml", mods=mods, title="Recently updated on " + site_name, ga=ga,
                                     description="Mods on " +
                                     site_name + " updated recently",
-                                    url="/browse/updated"), mimetype="text/xml")
+                                    url="/browse/updated"),
+                    mimetype="text/xml")
 
 
 @anonymous.route("/<gameshort>/browse/top")
@@ -212,15 +217,16 @@ def singlegame_browse_featured_rss(gameshort: str) -> Response:
     if not site_name:
         abort(404)
     ga = get_game_info(short=gameshort)
-    mods = get_featured_mods(ga.id, 30)
-    # Fix dates
-    for f in mods:
-        f.mod.created = f.created
-    mods = [dumb_object(f.mod) for f in mods]
-    db.rollback()
+    mods = []
+    for fm in get_featured_mods(ga.id, 30):
+        # Add each mod but with created set to when it was featured
+        fmod = dumb_object(fm.mod)
+        fmod['created'] = fm.created.astimezone(timezone.utc)
+        mods.append(fmod)
     return Response(render_template("rss.xml", mods=mods, title="Featured mods on " + site_name, ga=ga,
                                     description="Featured mods on " + site_name,
-                                    url="/browse/featured"), mimetype="text/xml")
+                                    url="/browse/featured"),
+                    mimetype="text/xml")
 
 
 @anonymous.route("/<gameshort>/browse/all")

KerbalStuff/blueprints/api.py

@@ -6,7 +6,6 @@
 from functools import wraps
 from typing import Dict, Any, Callable, Optional, Tuple, Iterable, List, Union
 
-import bcrypt
 from flask import Blueprint, url_for, current_app, request, abort
 from flask_login import login_user, current_user
 from sqlalchemy import desc, asc
@@ -22,6 +21,7 @@
 from ..objects import GameVersion, Game, Publisher, Mod, Featured, User, ModVersion, SharedAuthor, \
     ModList
 from ..search import search_mods, search_users, typeahead_mods, get_mod_score
+from ..thumbnail import thumb_path_from_background_path
 
 api = Blueprint('api', __name__)
 
@@ -60,7 +60,7 @@ def mod_info(mod: Mod) -> Dict[str, Any]:
         "author": mod.user.username,
         "default_version_id": mod.default_version.id,
         "shared_authors": list(),
-        "background": mod.background,
+        "background": mod.background_url(_cfg('protocol'), _cfg('cdn-domain')),
         "bg_offset_y": mod.bgOffsetY,
         "license": mod.license,
         "website": mod.external_link,
@@ -174,28 +174,41 @@ def _update_image(old_path: str, base_name: str, base_path: str) -> Optional[str
     full_path = os.path.join(storage, base_path)
     if not os.path.exists(full_path):
         os.makedirs(full_path)
-    try:
-        os.remove(os.path.join(storage, old_path))
-    except:
-        pass  # who cares
+
+    if old_path:
+        try_remove_file_and_folder(os.path.join(storage, old_path))
     f.save(os.path.join(full_path, filename))
     return os.path.join(base_path, filename)
 
 
+def try_remove_file_and_folder(path: str) -> None:
+    """Tries to remove a file and the containing folder if empty
+
+    :param path: An absolute path to the file
+    """
+    try:
+        os.remove(path)
+        # Remove the containing folder if empty
+        folder = os.path.dirname(path)
+        if not os.listdir(folder):
+            os.rmdir(folder)
+    except:
+        pass
+
+
 def _get_modversion_paths(mod_name: str, friendly_version: str) -> Tuple[str, str]:
     mod_name_sec = secure_filename(mod_name)
-    storage_base = os.path.join(f'{secure_filename(current_user.username)}_{current_user.id!s}',
-                                mod_name_sec)
+    base_path = os.path.join(current_user.base_path(), mod_name_sec)
     storage = _cfg('storage')
     if not storage:
-        return ('', '')
-    storage_path = os.path.join(storage, storage_base)
+        return '', ''
+    storage_path = os.path.join(storage, base_path)
     filename = f'{mod_name_sec}-{friendly_version}.zip'
     if not os.path.exists(storage_path):
         os.makedirs(storage_path)
     full_path = os.path.join(storage_path, filename)
     # Return tuple of (full path, relative path)
-    return (full_path, os.path.join(storage_base, filename))
+    return full_path, os.path.join(base_path, filename)
 
 
 def serialize_mod_list(mods: Iterable[Mod]) -> Iterable[Dict[str, Any]]:
@@ -458,13 +471,23 @@ def update_mod_background(mod_id: int) -> Dict[str, Any]:
     mod = _get_mod(mod_id)
     _check_mod_editable(mod)
     seq_mod_name = secure_filename(mod.name)
-    base_name = f'{seq_mod_name}-{time.time()!s}'
-    base_path = os.path.join(f'{secure_filename(mod.user.username)}_{mod.user.id!s}', seq_mod_name)
-    new_path = _update_image(mod.background, base_name, base_path)
+    base_name = f'{seq_mod_name}-{int(time.time())}'
+    old_path = mod.background
+    new_path = _update_image(old_path, base_name, mod.base_path())
     if new_path:
         mod.background = new_path
+        # Remove the old thumbnail
+        storage = _cfg('storage')
+        if storage:
+            if mod.thumbnail:
+                try_remove_file_and_folder(os.path.join(storage, mod.thumbnail))
+            if old_path and (calc_path := thumb_path_from_background_path(old_path)) != mod.thumbnail:
+                try_remove_file_and_folder(os.path.join(storage, calc_path))
+        mod.thumbnail = None
+        # Generate the new thumbnail
+        mod.background_thumb()
         notify_ckan(mod, 'update-background')
-        return {'path': '/content/' + new_path}
+        return {'path': mod.background_url(_cfg('protocol'), _cfg('cdn-domain'))}
     return {'path': None}
 
 
@@ -476,12 +499,13 @@ def update_user_background(username: str) -> Union[Dict[str, Any], Tuple[Dict[st
     if not current_user.admin and current_user.username != username:
         return {'error': True, 'reason': 'You are not authorized to edit this user\'s background'}, 403
     user = User.query.filter(User.username == username).first()
-    base_name = secure_filename(user.username)
-    base_path = f'{base_name}-{time.time()!s}_{user.id!s}'
-    new_path = _update_image(user.backgroundMedia, base_name, base_path)
+    seq_username = secure_filename(user.username)
+    base_name = f'{seq_username}-header-{int(time.time())}'
+    new_path = _update_image(user.backgroundMedia, base_name, user.base_path())
     if new_path:
         user.backgroundMedia = new_path
-        return {'path': '/content/' + new_path}
+        # The frontend needs the new path so it can show the updated image
+        return {'path': user.background_url(_cfg('protocol'), _cfg('cdn-domain'))}
     return {'path': None}
 
 
@@ -605,6 +629,7 @@ def create_mod() -> Tuple[Dict[str, Any], int]:
         return {'error': True, 'reason': 'Only users with public profiles may create mods.'}, 403
     mod_name = request.form.get('name')
     short_description = request.form.get('short-description')
+    description = request.form.get('description', default_description)
     mod_friendly_version = secure_filename(request.form.get('version', ''))
     # 'game' is deprecated, but kept for compatibility
     game_id = request.form.get('game-id') or request.form.get('game')
@@ -662,7 +687,7 @@ def create_mod() -> Tuple[Dict[str, Any], int]:
         mod = Mod(user=current_user,
                   name=mod_name,
                   short_description=short_description,
-                  description=default_description,
+                  description=description,
                   license=mod_licence,
                   ckan=(request.form.get('ckan', '').lower() in TRUE_STR),
                   game=game,
@@ -735,6 +760,7 @@ def update_mod(mod_id: int) -> Tuple[Dict[str, Any], int]:
         if mod.versions:
             version.sort_index = max(v.sort_index for v in mod.versions) + 1
         version.mod = mod
+        version.download_size = os.path.getsize(full_path)
         mod.default_version = version
         mod.updated = datetime.now()
         db.commit()