rewrite fietsctl to use click

This makes it consistent with the other scripts (fietsupdate,
fietscron), and makes the argument parsing setup a bit nicer to read.

5 files changed, 175 insertions, 230 deletions

diff --git a/fietsboek/scripts/__init__.py b/fietsboek/scripts/__init__.py
index 5bb534f..ea36d96 100644
--- a/fietsboek/scripts/__init__.py
+++ b/fietsboek/scripts/__init__.py
@@ -1 +1,19 @@
-# package
+"""Various command line scripts to interact with the fietsboek installation."""
+import click
+
+# We keep this as a separate option that is added to each subcommand as Click
+# (unlike argparse) cannot handle "--help" without the required arguments of
+# the parent (so "fietsupdate update --help" would error out)
+# See also
+# https://github.com/pallets/click/issues/295
+# https://github.com/pallets/click/issues/814
+config_option = click.option(
+    "-c",
+    "--config",
+    type=click.Path(exists=True, dir_okay=False),
+    required=True,
+    help="Path to the Fietsboek configuration file",
+)
+
+
+__all__ = ["config_option"]
diff --git a/fietsboek/scripts/fietsctl.py b/fietsboek/scripts/fietsctl.py
index 2d6302e..0043862 100644
--- a/fietsboek/scripts/fietsctl.py
+++ b/fietsboek/scripts/fietsctl.py
@@ -1,19 +1,56 @@
 """Script to do maintenance work on a Fietsboek instance."""
-# pylint: disable=consider-using-f-string,unused-argument
-import argparse
-import getpass
-import sys
+# pylint: disable=too-many-arguments
+from typing import Optional
 
+import click
+from click_option_group import RequiredMutuallyExclusiveOptionGroup, optgroup
 from pyramid.paster import bootstrap, setup_logging
+from pyramid.scripting import AppEnvironment
 from sqlalchemy import select
 
 from .. import __VERSION__, models
+from . import config_option
 
 EXIT_OKAY = 0
 EXIT_FAILURE = 1
 
 
-def cmd_useradd(env, args):
+def setup(config_path: str) -> AppEnvironment:
+    """Sets up the logging and app environment for the scripts.
+
+    This - for example - sets up a transaction manager in
+    ``setup(...)["request"]``.
+
+    :param config_path: Path to the configuration file.
+    :return: The prepared environment.
+    """
+    setup_logging(config_path)
+    return bootstrap(config_path)
+
+
+@click.group(
+    help=__doc__,
+    context_settings={"help_option_names": ["-h", "--help"]},
+)
+@click.version_option(package_name="fietsboek")
+def cli():
+    """CLI main entry point."""
+
+
+@cli.command("useradd")
+@config_option
+@click.option("--email", help="email address of the user", prompt=True)
+@click.option("--name", help="name of the user", prompt=True)
+@click.option(
+    "--password",
+    help="password of the user",
+    prompt=True,
+    hide_input=True,
+    confirmation_prompt=True,
+)
+@click.option("--admin", help="make the new user an admin", is_flag=True)
+@click.pass_context
+def cmd_useradd(ctx: click.Context, config: str, email: str, name: str, password: str, admin: bool):
     """Create a new user.
 
     This user creation bypasses the "enable_account_registration" setting. It
@@ -26,15 +63,7 @@ def cmd_useradd(env, args):
 
     Note that this function does less input validation and should therefore be used with care!
     """
-    email = args.email
-    if not email:
-        email = input("Email address: ")
-    name = args.name
-    if not name:
-        name = input("Name: ")
-    password = args.password
-    if not password:
-        password = getpass.getpass()
+    env = setup(config)
 
     # The UNIQUE constraint only prevents identical emails from being inserted,
     # but does not take into account the case insensitivity. The least we
@@ -44,10 +73,10 @@ def cmd_useradd(env, args):
     with env["request"].tm:
         result = env["request"].dbsession.execute(query).scalar_one_or_none()
         if result is not None:
-            print("Error: The given email already exists!", file=sys.stderr)
-            return EXIT_FAILURE
+            click.echo("Error: The given email already exists!", err=True)
+            ctx.exit(EXIT_FAILURE)
 
-    user = models.User(name=name, email=email, is_verified=True, is_admin=args.admin)
+    user = models.User(name=name, email=email, is_verified=True, is_admin=admin)
     user.set_password(password)
 
     with env["request"].tm:
@@ -56,11 +85,23 @@ def cmd_useradd(env, args):
         dbsession.flush()
         user_id = user.id
 
-    print(user_id)
-    return EXIT_OKAY
-
-
-def cmd_userdel(env, args):
+    click.echo(user_id)
+
+
+@cli.command("userdel")
+@config_option
+@click.option("--force", "-f", help="override the safety check", is_flag=True)
+@optgroup.group("User selection", cls=RequiredMutuallyExclusiveOptionGroup)
+@optgroup.option("--id", "-i", "id_", help="database ID of the user", type=int)
+@optgroup.option("--email", "-e", help="email of the user")
+@click.pass_context
+def cmd_userdel(
+    ctx: click.Context,
+    config: str,
+    force: bool,
+    id_: Optional[int],
+    email: Optional[str],
+):
     """Delete a user.
 
     This command deletes the user's account as well as any tracks associated
@@ -68,240 +109,118 @@ def cmd_userdel(env, args):
 
     This command is destructive and irreversibly deletes data.
     """
-    if args.id:
-        query = select(models.User).filter_by(id=args.id)
+    env = setup(config)
+    if id_ is not None:
+        query = select(models.User).filter_by(id=id_)
     else:
-        query = models.User.query_by_email(args.email)
+        query = models.User.query_by_email(email)
     with env["request"].tm:
         dbsession = env["request"].dbsession
         user = dbsession.execute(query).scalar_one_or_none()
         if user is None:
-            print("Error: No such user found.", file=sys.stderr)
-            return EXIT_FAILURE
-        print(user.name)
-        print(user.email)
-        if not args.force:
-            query = input("Really delete this user? [y/N] ")
-            if query not in {"Y", "y"}:
-                print("Aborted by user.")
-                return EXIT_FAILURE
+            click.echo("Error: No such user found.", err=True)
+            ctx.exit(EXIT_FAILURE)
+        click.echo(user.name)
+        click.echo(user.email)
+        if not force:
+            if not click.confirm("Really delete this user?"):
+                click.echo("Aborted by user.")
+                ctx.exit(EXIT_FAILURE)
         dbsession.delete(user)
-        print("User deleted")
-        return EXIT_OKAY
+        click.echo("User deleted")
 
 
-def cmd_userlist(env, args):
+@cli.command("userlist")
+@config_option
+def cmd_userlist(config: str):
     """Prints a listing of all user accounts.
 
-    The format is::
+    The format is:
 
-        [av] {ID} - {email} - {Name}
+        [av] {ID} - {email} - {name}
 
     with one line per user. The 'a' is added for admin accounts, the 'v' is added
     for verified users.
     """
+    env = setup(config)
     with env["request"].tm:
         dbsession = env["request"].dbsession
         users = dbsession.execute(select(models.User).order_by(models.User.id)).scalars()
         for user in users:
+            # pylint: disable=consider-using-f-string
             tag = "[{}{}]".format(
                 "a" if user.is_admin else "-",
                 "v" if user.is_verified else "-",
             )
-            print(f"{tag} {user.id} - {user.email} - {user.name}")
-    return EXIT_OKAY
-
-
-def cmd_passwd(env, args):
+            click.echo(f"{tag} {user.id} - {user.email} - {user.name}")
+
+
+@cli.command("passwd")
+@config_option
+@click.option("--password", help="password of the user")
+@optgroup.group("User selection", cls=RequiredMutuallyExclusiveOptionGroup)
+@optgroup.option("--id", "-i", "id_", help="database ID of the user", type=int)
+@optgroup.option("--email", "-e", help="email of the user")
+@click.pass_context
+def cmd_passwd(
+    ctx: click.Context,
+    config: str,
+    password: Optional[str],
+    id_: Optional[int],
+    email: Optional[str],
+):
     """Change the password of a user."""
-    if args.id:
-        query = select(models.User).filter_by(id=args.id)
+    env = setup(config)
+    if id_ is not None:
+        query = select(models.User).filter_by(id=id_)
     else:
-        query = models.User.query_by_email(args.email)
+        query = models.User.query_by_email(email)
     with env["request"].tm:
         dbsession = env["request"].dbsession
         user = dbsession.execute(query).scalar_one_or_none()
         if user is None:
-            print("Error: No such user found.", file=sys.stderr)
-            return EXIT_FAILURE
-        password = args.password
+            click.echo("Error: No such user found.", err=True)
+            ctx.exit(EXIT_FAILURE)
         if not password:
-            password = getpass.getpass()
-            repeat = getpass.getpass("Repeat password: ")
-            if password != repeat:
-                print("Error: Mismatched passwords.")
-                return EXIT_FAILURE
+            password = click.prompt("Password", hide_input=True, confirmation_prompt=True)
 
         user.set_password(password)
-        print(f"Changed password of {user.name} ({user.email})")
-        return EXIT_OKAY
+        click.echo(f"Changed password of {user.name} ({user.email})")
 
 
-def cmd_maintenance_mode(env, args):
+@cli.command("maintenance-mode")
+@config_option
+@click.option("--disable", help="disable the maintenance mode", is_flag=True)
+@click.argument("reason", required=False)
+@click.pass_context
+def cmd_maintenance_mode(ctx: click.Context, config: str, disable: bool, reason: Optional[str]):
     """Check the status of the maintenance mode, or activate or deactive it.
 
-    With neither `--disable` nor `reason` given, just checks the state of the
+    When REASON is given, enables the maintenance mode with the given text as
+    reason.
+
+    With neither --disable nor REASON given, just checks the state of the
     maintenance mode.
     """
+    env = setup(config)
     data_manager = env["request"].data_manager
-    if args.reason is None and not args.disable:
+    if disable and reason:
+        click.echo("Cannot enable and disable maintenance mode at the same time", err=True)
+        ctx.exit(EXIT_FAILURE)
+    elif not disable and not reason:
         maintenance = data_manager.maintenance_mode()
         if maintenance is None:
-            print("Maintenance mode is disabled")
+            click.echo("Maintenance mode is disabled")
         else:
-            print(f"Maintenance mode is enabled: {maintenance}")
-    elif args.disable:
+            click.echo(f"Maintenance mode is enabled: {maintenance}")
+    elif disable:
         (data_manager.data_dir / "MAINTENANCE").unlink()
     else:
-        (data_manager.data_dir / "MAINTENANCE").write_text(args.reason, encoding="utf-8")
-    return EXIT_OKAY
+        (data_manager.data_dir / "MAINTENANCE").write_text(reason, encoding="utf-8")
 
 
+@cli.command("version")
 def cmd_version():
     """Show the installed fietsboek version."""
     name = __name__.split(".", 1)[0]
     print(f"{name} {__VERSION__}")
-
-
-def parse_args(argv):
-    """Parse the given args.
-
-    :param argv: List of arguments.
-    :type argv: list[str]
-    :return: The parsed arguments.
-    :rtype: argparse.Namespace
-    """
-    parser = argparse.ArgumentParser(description=__doc__)
-    parser.add_argument(
-        "-c",
-        "--config",
-        dest="config_uri",
-        help="configuration file, e.g., development.ini",
-    )
-
-    subparsers = parser.add_subparsers(help="available subcommands", required=True)
-
-    p_version = subparsers.add_parser(
-        "version",
-        help="show the version",
-        description=cmd_version.__doc__,
-    )
-    p_version.set_defaults(func=cmd_version)
-
-    p_useradd = subparsers.add_parser(
-        "useradd",
-        help="create a new user",
-        description=cmd_useradd.__doc__,
-    )
-    p_useradd.add_argument(
-        "--email",
-        help="email address of the user",
-    )
-    p_useradd.add_argument(
-        "--name",
-        help="name of the user",
-    )
-    p_useradd.add_argument(
-        "--password",
-        help="password of the user",
-    )
-    p_useradd.add_argument(
-        "--admin",
-        action="store_true",
-        help="make the new user an admin",
-    )
-    p_useradd.set_defaults(func=cmd_useradd)
-
-    p_userdel = subparsers.add_parser(
-        "userdel",
-        help="delete a user account",
-        description=cmd_userdel.__doc__,
-    )
-    p_userdel.add_argument(
-        "--force",
-        "-f",
-        action="store_true",
-        help="override the safety check",
-    )
-    group = p_userdel.add_mutually_exclusive_group(required=True)
-    group.add_argument(
-        "--id",
-        "-i",
-        type=int,
-        help="database ID of the user",
-    )
-    group.add_argument(
-        "--email",
-        "-e",
-        help="email of the user",
-    )
-    p_userdel.set_defaults(func=cmd_userdel)
-
-    p_userlist = subparsers.add_parser(
-        "userlist",
-        help="list user accounts",
-        description=cmd_userlist.__doc__,
-    )
-    p_userlist.set_defaults(func=cmd_userlist)
-
-    p_passwd = subparsers.add_parser(
-        "passwd",
-        help="change user password",
-        description=cmd_passwd.__doc__,
-    )
-    p_passwd.add_argument(
-        "--password",
-        help="password of the user",
-    )
-    group = p_passwd.add_mutually_exclusive_group(required=True)
-    group.add_argument(
-        "--id",
-        "-i",
-        type=int,
-        help="database ID of the user",
-    )
-    group.add_argument(
-        "--email",
-        "-e",
-        help="email of the user",
-    )
-    p_passwd.set_defaults(func=cmd_passwd)
-
-    p_maintenance = subparsers.add_parser(
-        "maintenance-mode",
-        help="enable or disable the maintenance mode",
-        description=cmd_maintenance_mode.__doc__,
-    )
-    group = p_maintenance.add_mutually_exclusive_group(required=False)
-    group.add_argument(
-        "--disable",
-        action="store_true",
-        help="disable the maintenance mode",
-    )
-    group.add_argument(
-        "reason",
-        nargs="?",
-        help="reason for the maintenance",
-    )
-    p_maintenance.set_defaults(func=cmd_maintenance_mode)
-
-    return parser.parse_args(argv[1:]), parser
-
-
-def main(argv=None):
-    """Main entry point."""
-    if argv is None:
-        argv = sys.argv
-    args, parser = parse_args(argv)
-
-    if args.func == cmd_version:  # pylint: disable=comparison-with-callable
-        cmd_version()
-        sys.exit(EXIT_OKAY)
-
-    if not args.config_uri:
-        parser.error("the following arguments are required: -c/--config")
-
-    setup_logging(args.config_uri)
-    env = bootstrap(args.config_uri)
-
-    sys.exit(args.func(env, args))
diff --git a/fietsboek/updater/cli.py b/fietsboek/updater/cli.py
index d1b9a3e..f74dc40 100644
--- a/fietsboek/updater/cli.py
+++ b/fietsboek/updater/cli.py
@@ -10,22 +10,9 @@ import logging.config
 
 import click
 
+from ..scripts import config_option
 from . import Updater
 
-# We keep this as a separate option that is added to each subcommand as Click
-# (unlike argparse) cannot handle "--help" without the required arguments of
-# the parent (so "fietsupdate update --help" would error out)
-# See also
-# https://github.com/pallets/click/issues/295
-# https://github.com/pallets/click/issues/814
-config_option = click.option(
-    "-c",
-    "--config",
-    type=click.Path(exists=True, dir_okay=False),
-    required=True,
-    help="Path to the Fietsboek configuration file",
-)
-
 
 def user_confirm(verb):
     """Ask the user for confirmation before proceeding.
diff --git a/poetry.lock b/poetry.lock
index 12cebf3..50835cf 100644
--- a/poetry.lock
+++ b/poetry.lock
@@ -1,4 +1,4 @@
-# This file is automatically @generated by Poetry and should not be changed by hand.
+# This file is automatically @generated by Poetry 1.4.0 and should not be changed by hand.
 
 [[package]]
 name = "alabaster"
@@ -465,6 +465,26 @@ files = [
 colorama = {version = "*", markers = "platform_system == \"Windows\""}
 
 [[package]]
+name = "click-option-group"
+version = "0.5.5"
+description = "Option groups missing in Click"
+category = "main"
+optional = false
+python-versions = ">=3.6,<4"
+files = [
+    {file = "click-option-group-0.5.5.tar.gz", hash = "sha256:78ee474f07a0ca0ef6c0317bb3ebe79387aafb0c4a1e03b1d8b2b0be1e42fc78"},
+    {file = "click_option_group-0.5.5-py3-none-any.whl", hash = "sha256:0f8ca79bc9b1d6fcaafdbe194b17ba1a2dde44ddf19087235c3efed2ad288143"},
+]
+
+[package.dependencies]
+Click = ">=7.0,<9"
+
+[package.extras]
+docs = ["Pallets-Sphinx-Themes", "m2r2", "sphinx (>=3.0,<6)"]
+tests = ["pytest"]
+tests-cov = ["coverage (<6)", "coveralls", "pytest", "pytest-cov"]
+
+[[package]]
 name = "colorama"
 version = "0.4.6"
 description = "Cross-platform colored terminal text."
@@ -1835,7 +1855,7 @@ greenlet = {version = "!=0.4.17", markers = "python_version >= \"3\" and platfor
 
 [package.extras]
 aiomysql = ["aiomysql", "greenlet (!=0.4.17)"]
-aiosqlite = ["aiosqlite", "greenlet (!=0.4.17)", "typing_extensions (!=3.10.0.1)"]
+aiosqlite = ["aiosqlite", "greenlet (!=0.4.17)", "typing-extensions (!=3.10.0.1)"]
 asyncio = ["greenlet (!=0.4.17)"]
 asyncmy = ["asyncmy (>=0.2.3,!=0.2.4)", "greenlet (!=0.4.17)"]
 mariadb-connector = ["mariadb (>=1.0.1,!=1.1.2)"]
@@ -1845,14 +1865,14 @@ mssql-pyodbc = ["pyodbc"]
 mypy = ["mypy (>=0.910)", "sqlalchemy2-stubs"]
 mysql = ["mysqlclient (>=1.4.0)", "mysqlclient (>=1.4.0,<2)"]
 mysql-connector = ["mysql-connector-python"]
-oracle = ["cx_oracle (>=7)", "cx_oracle (>=7,<8)"]
+oracle = ["cx-oracle (>=7)", "cx-oracle (>=7,<8)"]
 postgresql = ["psycopg2 (>=2.7)"]
 postgresql-asyncpg = ["asyncpg", "greenlet (!=0.4.17)"]
 postgresql-pg8000 = ["pg8000 (>=1.16.6,!=1.29.0)"]
 postgresql-psycopg2binary = ["psycopg2-binary"]
 postgresql-psycopg2cffi = ["psycopg2cffi"]
 pymysql = ["pymysql", "pymysql (<1)"]
-sqlcipher = ["sqlcipher3_binary"]
+sqlcipher = ["sqlcipher3-binary"]
 
 [[package]]
 name = "termcolor"
@@ -2357,4 +2377,4 @@ test = ["zope.testing"]
 [metadata]
 lock-version = "2.0"
 python-versions = "^3.9"
-content-hash = "3daffa079370a50dea936dac52e997331ceefa0a67680845ed9ee623ee70312b"
+content-hash = "3a56e9d06c7becf27c7caf482ac2d58189c8b5768a2f7b0788ad2825d939595f"
diff --git a/pyproject.toml b/pyproject.toml
index 274d11e..e38f8aa 100644
--- a/pyproject.toml
+++ b/pyproject.toml
@@ -52,6 +52,7 @@ pydantic = "^1.10.2"
 termcolor = "^2.1.1"
 filelock = "^3.8.2"
 brotli = "^1.0.9"
+click-option-group = "^0.5.5"
 
 [tool.poetry.group.docs]
 optional = true
@@ -88,7 +89,7 @@ types-babel = "^2.11.0.7"
 types-redis = "^4.3.21.6"
 
 [tool.poetry.scripts]
-fietsctl = "fietsboek.scripts.fietsctl:main"
+fietsctl = "fietsboek.scripts.fietsctl:cli"
 fietscron = "fietsboek.scripts.fietscron:cli"
 fietsupdate = "fietsboek.updater.cli:cli"