Make Config optimizations and HTTPMethod consts (lucyparsons#956)

 lucyparsons#954

Added inheritance to the `Config` objects, removed unnecessary functions
from the `Config` classes, and removed the custom constants and replaced
them with `HTTPMethod` constants.

 - [x] This branch is up-to-date with the `develop` branch.
 - [x] `pytest` passes on my local development environment.
 - [x] `pre-commit` passes on my local development environment.

OpenOversight/app/__init__.py

@@ -34,8 +34,8 @@
 
 def create_app(config_name="default"):
     app = Flask(__name__)
+    # Creates and adds the Config object of the correct type to app.config
     app.config.from_object(config[config_name])
-    config[config_name].init_app(app)
     from .models import db
 
     bootstrap.init_app(app)

OpenOversight/app/auth/views.py

@@ -1,4 +1,4 @@
-from http import HTTPStatus
+from http import HTTPMethod, HTTPStatus
 
 from flask import (
     current_app,
@@ -20,7 +20,6 @@
     ResetPasswordEmail,
 )
 from OpenOversight.app.models import User, db
-from OpenOversight.app.utils.constants import HTTP_METHOD_GET, HTTP_METHOD_POST
 from OpenOversight.app.utils.forms import set_dynamic_default
 from OpenOversight.app.utils.general import validate_redirect_url
 
@@ -39,6 +38,7 @@
 from .utils import admin_required
 
 
+js_loads = ["js/zxcvbn.js", "js/password.js"]
 sitemap_endpoints = []
 
 
@@ -76,7 +76,7 @@ def unconfirmed():
 
 
 @sitemap_include
-@auth.route("/login", methods=[HTTP_METHOD_GET, HTTP_METHOD_POST])
+@auth.route("/login", methods=[HTTPMethod.GET, HTTPMethod.POST])
 def login():
     form = LoginForm()
     if form.validate_on_submit():
@@ -104,9 +104,8 @@ def logout():
 
 
 @sitemap_include
-@auth.route("/register", methods=[HTTP_METHOD_GET, HTTP_METHOD_POST])
+@auth.route("/register", methods=[HTTPMethod.GET, HTTPMethod.POST])
 def register():
-    js_loads = ["js/zxcvbn.js", "js/password.js"]
     form = RegistrationForm()
     if form.validate_on_submit():
         user = User(
@@ -139,7 +138,7 @@ def register():
     return render_template("auth/register.html", form=form, jsloads=js_loads)
 
 
-@auth.route("/confirm/<token>", methods=[HTTP_METHOD_GET])
+@auth.route("/confirm/<token>", methods=[HTTPMethod.GET])
 @login_required
 def confirm(token):
     if current_user.confirmed:
@@ -167,10 +166,9 @@ def resend_confirmation():
     return redirect(url_for("main.index"))
 
 
-@auth.route("/change-password", methods=[HTTP_METHOD_GET, HTTP_METHOD_POST])
+@auth.route("/change-password", methods=[HTTPMethod.GET, HTTPMethod.POST])
 @login_required
 def change_password():
-    js_loads = ["js/zxcvbn.js", "js/password.js"]
     form = ChangePasswordForm()
     if form.validate_on_submit():
         if current_user.verify_password(form.old_password.data):
@@ -186,7 +184,7 @@ def change_password():
     return render_template("auth/change_password.html", form=form, jsloads=js_loads)
 
 
-@auth.route("/reset", methods=[HTTP_METHOD_GET, HTTP_METHOD_POST])
+@auth.route("/reset", methods=[HTTPMethod.GET, HTTPMethod.POST])
 def password_reset_request():
     if not current_user.is_anonymous:
         return redirect(url_for("main.index"))
@@ -205,7 +203,7 @@ def password_reset_request():
     return render_template("auth/reset_password.html", form=form)
 
 
-@auth.route("/reset/<token>", methods=[HTTP_METHOD_GET, HTTP_METHOD_POST])
+@auth.route("/reset/<token>", methods=[HTTPMethod.GET, HTTPMethod.POST])
 def password_reset(token):
     if not current_user.is_anonymous:
         return redirect(url_for("main.index"))
@@ -224,7 +222,7 @@ def password_reset(token):
     return render_template("auth/reset_password.html", form=form)
 
 
-@auth.route("/change-email", methods=[HTTP_METHOD_GET, HTTP_METHOD_POST])
+@auth.route("/change-email", methods=[HTTPMethod.GET, HTTPMethod.POST])
 @login_required
 def change_email_request():
     form = ChangeEmailForm()
@@ -257,7 +255,7 @@ def change_email(token):
     return redirect(url_for("main.index"))
 
 
-@auth.route("/change-dept/", methods=[HTTP_METHOD_GET, HTTP_METHOD_POST])
+@auth.route("/change-dept/", methods=[HTTPMethod.GET, HTTPMethod.POST])
 @login_required
 def change_dept():
     form = ChangeDefaultDepartmentForm()
@@ -277,32 +275,31 @@ def change_dept():
     return render_template("auth/change_dept_pref.html", form=form)
 
 
-@auth.route("/users/", methods=[HTTP_METHOD_GET])
+@auth.route("/users/", methods=[HTTPMethod.GET])
 @admin_required
 def get_users():
     if request.args.get("page"):
         page = int(request.args.get("page"))
     else:
         page = 1
-    USERS_PER_PAGE = int(current_app.config["USERS_PER_PAGE"])
     users = User.query.order_by(User.username).paginate(
-        page=page, per_page=USERS_PER_PAGE, error_out=False
+        page=page, per_page=current_app.config["USERS_PER_PAGE"], error_out=False
     )
 
     return render_template("auth/users.html", objects=users)
 
 
-@auth.route("/users/<int:user_id>", methods=[HTTP_METHOD_GET, HTTP_METHOD_POST])
+@auth.route("/users/<int:user_id>", methods=[HTTPMethod.GET, HTTPMethod.POST])
 @admin_required
 def edit_user(user_id):
     user = User.query.get(user_id)
     if not user:
         return render_template("404.html"), HTTPStatus.NOT_FOUND
 
-    if request.method == HTTP_METHOD_GET:
+    if request.method == HTTPMethod.GET:
         form = EditUserForm(obj=user)
         return render_template("auth/user.html", user=user, form=form)
-    elif request.method == HTTP_METHOD_POST:
+    elif request.method == HTTPMethod.POST:
         form = EditUserForm()
         if form.delete.data:
             # forward to confirm delete
@@ -339,13 +336,13 @@ def edit_user(user_id):
                 return render_template("auth/user.html", user=user, form=form)
 
 
-@auth.route("/users/<int:user_id>/delete", methods=[HTTP_METHOD_GET, HTTP_METHOD_POST])
+@auth.route("/users/<int:user_id>/delete", methods=[HTTPMethod.GET, HTTPMethod.POST])
 @admin_required
 def delete_user(user_id):
     user = User.query.get(user_id)
     if not user or user.is_administrator:
         return render_template("403.html"), HTTPStatus.FORBIDDEN
-    if request.method == HTTP_METHOD_POST:
+    if request.method == HTTPMethod.POST:
         username = user.username
         db.session.delete(user)
         db.session.commit()

OpenOversight/app/config.py

@@ -6,81 +6,77 @@
 basedir = os.path.abspath(os.path.dirname(__file__))
 
 
-class BaseConfig(object):
-    ENVIRONMENT = os.environ.get("ENVIRONMENT", "")
-    # DB Settings
-    SQLALCHEMY_TRACK_MODIFICATIONS = False
-
-    # Pagination Settings
-    OFFICERS_PER_PAGE = os.environ.get("OFFICERS_PER_PAGE", 20)
-    USERS_PER_PAGE = os.environ.get("USERS_PER_PAGE", 20)
-
-    # Form Settings
-    WTF_CSRF_ENABLED = True
-    SECRET_KEY = os.environ.get("SECRET_KEY", "changemeplzorelsehax")
-
-    # Mail Settings
-    OO_MAIL_SUBJECT_PREFIX = os.environ.get("OO_MAIL_SUBJECT_PREFIX", "[OpenOversight]")
-    OO_SERVICE_EMAIL = os.environ.get("OO_SERVICE_EMAIL")
-
-    # AWS Settings
-    AWS_ACCESS_KEY_ID = os.environ.get("AWS_ACCESS_KEY_ID")
-    AWS_SECRET_ACCESS_KEY = os.environ.get("AWS_SECRET_ACCESS_KEY")
-    AWS_DEFAULT_REGION = os.environ.get("AWS_DEFAULT_REGION")
-    S3_BUCKET_NAME = os.environ.get("S3_BUCKET_NAME")
-    AWS_ENDPOINT_URL = os.environ.get("AWS_ENDPOINT_URL")
-
-    # Upload Settings
-    MAX_CONTENT_LENGTH = 50 * MEGABYTE
-    ALLOWED_EXTENSIONS = set(["jpeg", "jpg", "jpe", "png", "gif", "webp"])
-
-    # User Settings
-    APPROVE_REGISTRATIONS = os.environ.get("APPROVE_REGISTRATIONS", False)
-
-    # Use session cookie to store URL to redirect to after login
-    # https://flask-login.readthedocs.io/en/latest/#customizing-the-login-process
-    USE_SESSION_FOR_NEXT = True
-
-    # Misc. Settings
-    SEED = 666
-
-    @staticmethod
-    def init_app(app):
-        pass
+class BaseConfig:
+    def __init__(self):
+        # App Settings
+        self.DEBUG = False
+        self.ENVIRONMENT = os.environ.get("ENVIRONMENT", "")
+        self.SEED = 666
+        self.TESTING = False
+        # Use session cookie to store URL to redirect to after login
+        # https://flask-login.readthedocs.io/en/latest/#customizing-the-login-process
+        self.USE_SESSION_FOR_NEXT = True
+
+        # DB Settings
+        self.SQLALCHEMY_TRACK_MODIFICATIONS = False
+        self.SQLALCHEMY_DATABASE_URI = os.environ.get("SQLALCHEMY_DATABASE_URI")
+
+        # Protocol Settings
+        self.SITEMAP_URL_SCHEME = "http"
+
+        # Pagination Settings
+        self.OFFICERS_PER_PAGE = int(os.environ.get("OFFICERS_PER_PAGE", 20))
+        self.USERS_PER_PAGE = int(os.environ.get("USERS_PER_PAGE", 20))
+
+        # Form Settings
+        self.SECRET_KEY = os.environ.get("SECRET_KEY", "changemeplzorelsehax")
+        self.WTF_CSRF_ENABLED = True
+
+        # Mail Settings
+        self.OO_SERVICE_EMAIL = os.environ.get("OO_SERVICE_EMAIL")
+
+        # AWS Settings
+        self.AWS_ACCESS_KEY_ID = os.environ.get("AWS_ACCESS_KEY_ID")
+        self.AWS_DEFAULT_REGION = os.environ.get("AWS_DEFAULT_REGION")
+        self.AWS_ENDPOINT_URL = os.environ.get("AWS_ENDPOINT_URL")
+        self.AWS_SECRET_ACCESS_KEY = os.environ.get("AWS_SECRET_ACCESS_KEY")
+        self.S3_BUCKET_NAME = os.environ.get("S3_BUCKET_NAME")
+
+        # Upload Settings
+        self.ALLOWED_EXTENSIONS = set(["jpeg", "jpg", "jpe", "png", "gif", "webp"])
+        self.MAX_CONTENT_LENGTH = 50 * MEGABYTE
+
+        # User settings
+        self.APPROVE_REGISTRATIONS = os.environ.get("APPROVE_REGISTRATIONS", False)
 
 
 class DevelopmentConfig(BaseConfig):
-    DEBUG = True
-    SQLALCHEMY_ECHO = True
-    SQLALCHEMY_DATABASE_URI = os.environ.get("SQLALCHEMY_DATABASE_URI")
-    NUM_OFFICERS = 15000
-    SITEMAP_URL_SCHEME = "http"
+    def __init__(self):
+        super(DevelopmentConfig, self).__init__()
+        self.DEBUG = True
+        self.SQLALCHEMY_ECHO = True
+        self.NUM_OFFICERS = 15000
 
 
 class TestingConfig(BaseConfig):
-    TESTING = True
-    SQLALCHEMY_DATABASE_URI = "sqlite:///:memory:"
-    WTF_CSRF_ENABLED = False
-    NUM_OFFICERS = 120
-    APPROVE_REGISTRATIONS = False
-    SITEMAP_URL_SCHEME = "http"
-    RATELIMIT_ENABLED = False
+    def __init__(self):
+        super(TestingConfig, self).__init__()
+        self.TESTING = True
+        self.WTF_CSRF_ENABLED = False
+        self.NUM_OFFICERS = 120
+        self.RATELIMIT_ENABLED = False
+        self.SQLALCHEMY_DATABASE_URI = "sqlite:///:memory:"
 
 
 class ProductionConfig(BaseConfig):
-    SQLALCHEMY_DATABASE_URI = os.environ.get("SQLALCHEMY_DATABASE_URI")
-    SITEMAP_URL_SCHEME = "https"
-
-    @classmethod
-    def init_app(cls, app):  # pragma: no cover
-        # ????????
-        # config.init_app(app)
-        pass
+    def __init__(self):
+        super(ProductionConfig, self).__init__()
+        self.SITEMAP_URL_SCHEME = "https"
 
 
 config = {
-    "development": DevelopmentConfig,
-    "testing": TestingConfig,
-    "production": ProductionConfig,
+    "development": DevelopmentConfig(),
+    "testing": TestingConfig(),
+    "production": ProductionConfig(),
 }
 config["default"] = config.get(os.environ.get("ENVIRONMENT", ""), DevelopmentConfig)

OpenOversight/app/main/model_view.py

@@ -1,4 +1,5 @@
 import datetime
+from http import HTTPMethod
 from typing import Callable, Union
 
 from flask import abort, current_app, flash, redirect, render_template, request, url_for
@@ -7,7 +8,6 @@
 from flask_sqlalchemy.model import DefaultMeta
 from flask_wtf import FlaskForm as Form
 
-from OpenOversight.app.utils.constants import HTTP_METHOD_GET, HTTP_METHOD_POST
 from OpenOversight.app.utils.db import add_department_query
 from OpenOversight.app.utils.forms import set_dynamic_default
 
@@ -133,7 +133,7 @@ def delete(self, obj_id):
             ):
                 abort(403)
 
-        if request.method == HTTP_METHOD_POST:
+        if request.method == HTTPMethod.POST:
             db.session.delete(obj)
             db.session.commit()
             flash("{} successfully deleted!".format(self.model_name))
@@ -154,7 +154,7 @@ def get_redirect_url(self, *args, **kwargs):
             url_for(
                 "main.{}_api".format(self.model_name),
                 obj_id=kwargs["obj_id"],
-                _method=HTTP_METHOD_GET,
+                _method=HTTPMethod.GET,
             )
         )
 
@@ -184,7 +184,7 @@ def dispatch_request(self, *args, **kwargs):
             if end_of_url == ending:
                 meth = getattr(self, ending, None)
         if not meth:
-            if request.method == HTTP_METHOD_GET:
+            if request.method == HTTPMethod.GET:
                 meth = getattr(self, "get", None)
             else:
                 assert meth is not None, "Unimplemented method %r" % request.method