Merge pull request #395 from KSP-SpaceDock/alpha

α → β ⑨

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