Skip to content

Commit

Permalink
Handle missing default mod version, don't close db before rendering t…
Browse files Browse the repository at this point in the history
…emplates
  • Loading branch information
HebaruSan committed Jul 29, 2021
1 parent 28411fa commit 7b5e378
Show file tree
Hide file tree
Showing 12 changed files with 38 additions and 15 deletions.
4 changes: 3 additions & 1 deletion .github/workflows/mypy.yml
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
name: Mypy

on: [push, pull_request]
on:
- push
- pull_request

jobs:
build:
Expand Down
4 changes: 3 additions & 1 deletion .github/workflows/pytest.yml
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
name: PyTest

on: [push, pull_request]
on:
- push
- pull_request

jobs:
build:
Expand Down
12 changes: 10 additions & 2 deletions KerbalStuff/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -50,6 +51,8 @@
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
Expand Down Expand Up @@ -156,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

Expand All @@ -168,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>')
Expand Down
2 changes: 1 addition & 1 deletion KerbalStuff/blueprints/mods.py
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ def mod(mod_id: int, mod_name: str) -> Union[str, werkzeug.wrappers.Response]:
editable = True
if not mod.published and not editable:
abort(403, 'Unfortunately we couldn\'t display the requested mod. Maybe it\'s not public yet?')
latest = mod.default_version
latest = mod.default_version or (mod.versions[0] if len(mod.versions) > 0 else None)
referral = request.referrer
if referral:
host = urlparse(referral).hostname
Expand Down
4 changes: 2 additions & 2 deletions KerbalStuff/ckan.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
import requests
import re
from flask import url_for
from typing import Dict, Iterable
from typing import Dict, Iterable, Optional

from .config import _cfg
from .objects import Mod, Game, GameVersion
Expand Down Expand Up @@ -58,7 +58,7 @@ def import_ksp_versions_from_ckan(ksp_game_id: int) -> None:
db.commit()


def ksp_versions_from_ckan() -> Iterable[str]:
def ksp_versions_from_ckan() -> Iterable[Optional[str]]:
builds = requests.get(CKAN_BUILDS_URL).json()
for _, full_version in builds['builds'].items():
m = MAJOR_MINOR_PATCH_PATTERN.match(full_version)
Expand Down
6 changes: 3 additions & 3 deletions KerbalStuff/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ def go(*args: str, **kwargs: int) -> werkzeug.wrappers.Response:
return ret
except:
db.rollback()
db.close()
# Session will be closed in app.teardown_request so templates can be rendered
raise

return go
Expand Down Expand Up @@ -96,7 +96,7 @@ def wrapper(*args: str, **kwargs: int) -> werkzeug.wrappers.Response:
return wrapper


def json_response(obj: Any, status: int = None) -> werkzeug.wrappers.Response:
def json_response(obj: Any, status: Optional[int] = None) -> werkzeug.wrappers.Response:
data = json.dumps(obj, cls=CustomJSONEncoder, separators=(',', ':'))
return Response(data, status=status, mimetype='application/json')

Expand Down Expand Up @@ -151,7 +151,7 @@ def get_page() -> int:
return 1


def get_paginated_mods(ga: Game = None, query: str = '', page_size: int = 30) -> Tuple[Iterable[Mod], int, int]:
def get_paginated_mods(ga: Optional[Game] = None, query: str = '', page_size: int = 30) -> Tuple[Iterable[Mod], int, int]:
page = get_page()
mods, total_pages = search_mods(ga.id if ga else None, query, page, page_size)
return mods, page, total_pages
Expand Down
2 changes: 1 addition & 1 deletion KerbalStuff/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ def _cfgb(k: str, default: bool = False) -> bool:
return strtobool(val) == 1 if val is not None else default


def _cfgd(k: str, default: Dict[str, str] = None) -> Dict[str, str]:
def _cfgd(k: str, default: Optional[Dict[str, str]] = None) -> Dict[str, str]:
if default is None:
default = {}
val = _cfg(k)
Expand Down
4 changes: 2 additions & 2 deletions KerbalStuff/email.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import html
from typing import Iterable, List, Dict
from typing import Iterable, List, Dict, Optional

from flask import url_for
from jinja2 import Template
Expand All @@ -10,7 +10,7 @@
from .config import _cfg, _cfgd


def send_confirmation(user: User, followMod: str = None) -> None:
def send_confirmation(user: User, followMod: Optional[str] = None) -> None:
site_name = _cfg('site-name')
if site_name:
with open("emails/confirm-account") as f:
Expand Down
8 changes: 8 additions & 0 deletions KerbalStuff/stubs/jinja2.pyi
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@

from jinja2.environment import Template as Template

class Undefined:
...

class ChainableUndefined(Undefined):
...
1 change: 1 addition & 0 deletions requirements-backend.txt
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ Flask-Login
Flask-Markdown
Flask-OAuthlib
future
GitPython
gunicorn
Jinja2<3 # See Flask
Markdown
Expand Down
2 changes: 1 addition & 1 deletion templates/layout.html
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@
</head>
<body class="{% if user %}logged-in{% endif %}">
{% block nav %}
<nav class="navbar navbar-default navbar-fixed-top {% if user and user.admin %}navbar-admin{% endif %}" role="navigation" >
<nav class="navbar navbar-default navbar-fixed-top {% if admin %}navbar-admin{% endif %}" role="navigation" >
<div class="container">
<div class="navbar-header">
<button type="button" class="navbar-toggle" data-toggle="collapse" data-target="#navbar-collapse">
Expand Down
4 changes: 3 additions & 1 deletion templates/mod.html
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,9 @@ <h2>
<span class="text-muted">
Game Version:
</span>
{{ latest.gameversion.friendly_version }}
{% if latest -%}
{{ latest.gameversion.friendly_version }}
{%- endif %}
</h2>
</div>
</div>
Expand Down

0 comments on commit 7b5e378

Please sign in to comment.