diff --git a/.devcontainer/devcontainer.json b/.devcontainer/devcontainer.json index abce9d3..fe652f4 100644 --- a/.devcontainer/devcontainer.json +++ b/.devcontainer/devcontainer.json @@ -12,7 +12,7 @@ "settings": { "python.linting.pylintPath": "/workspace/.venv/bin/pylint" }, - "extensions": ["ms-azuretools.vscode-docker"] + "extensions": ["ms-azuretools.vscode-docker", "ms-python.black-formatter"] } }, "remoteUser": "root" diff --git a/Dockerfile b/Dockerfile index 15cfc28..7058678 100644 --- a/Dockerfile +++ b/Dockerfile @@ -5,9 +5,6 @@ ENV PYTHONUNBUFFERED=1 WORKDIR /app - -FROM base as build - ENV PIP_DEFAULT_TIMEOUT=100 ENV PIP_DISABLE_PIP_VERSION_CHECK=1 ENV PIP_NO_CACHE_DIR=1 @@ -20,26 +17,31 @@ COPY pyproject.toml poetry.lock ./ RUN poetry config virtualenvs.in-project true RUN poetry install --no-interaction --no-root + +FROM base as build + COPY . . RUN poetry build FROM base AS execute +ENV PATH=/app/.venv/bin:$PATH COPY --from=build /app/dist . COPY --from=build /app/.venv ./.venv RUN ./.venv/bin/pip install *.whl COPY ./config ./config -ENV PATH=/app/.venv/bin:$PATH ENTRYPOINT ["python", "-m", "notifier"] -FROM execute AS test -RUN apt-get update && apt-get install -y default-mysql-client +FROM base AS test ENV PATH=/app/.venv/bin:$PATH +RUN apt-get update && apt-get install -y default-mysql-client COPY conftest.py . +COPY config/ config/ COPY tests/ tests/ -ENTRYPOINT ["pytest", "-vx"] +COPY notifier/ notifier/ +ENTRYPOINT ["pytest", "-vvx"] FROM amazon/aws-lambda-python:3.8 AS execute_lambda diff --git a/README.md b/README.md index f01de10..09dedd8 100644 --- a/README.md +++ b/README.md @@ -50,6 +50,13 @@ To start the notifier service locally: poetry run python3 -m notifier path_to_config_file path_to_auth_file ``` +Or with Docker: + +```shell +docker build --target execute --tag notifier:execute . +docker run --rm notifier:execute path_to_config_file path_to_auth_file +``` + The config file that my notifier instance uses is `config/config.toml`. A sample auth file with dummy secrets, used for CI tests, can be found at `config/auth.ci.toml`. diff --git a/docker-compose.test.yml b/docker-compose.test.yml index 56ff90a..357556c 100644 --- a/docker-compose.test.yml +++ b/docker-compose.test.yml @@ -7,7 +7,7 @@ services: context: . dockerfile: ./Dockerfile target: test - command: "--notifier-config config/config.toml --notifier-auth config/auth.compose.toml" + command: "--notifier-config config/config.toml --notifier-auth config/auth.compose.toml ${PYTEST_ARGS-}" links: - database depends_on: diff --git a/docs/database.md b/docs/database.md index 679a9a9..39591ec 100644 --- a/docs/database.md +++ b/docs/database.md @@ -33,7 +33,7 @@ Create the MySQL Server container: docker create --name notifier_mysql \ -p 3306:3306 \ -e MYSQL_ROOT_PASSWORD=root \ - mysql:5.6.17 + mysql:5.7.4 ``` For an ephemeral, development-only, containerised MySQL installation I'm diff --git a/lambda_function.py b/lambda_function.py index 234113f..c01393d 100644 --- a/lambda_function.py +++ b/lambda_function.py @@ -46,9 +46,9 @@ def lambda_handler(event, context): logger.debug("Lambda: starting main procedure") main( - read_local_config(local_config_path), - read_local_auth(local_auth_path), - [], + config=read_local_config(local_config_path), + auth=read_local_auth(local_auth_path), + execute_now=[], force_current_time=force_current_time, ) logger.info("Lambda finished") diff --git a/notifier/cli.py b/notifier/cli.py index 0fa400f..dd7ec72 100644 --- a/notifier/cli.py +++ b/notifier/cli.py @@ -12,11 +12,12 @@ def cli(): """Run main procedure as a command-line tool.""" args = read_command_line_arguments() main( - read_local_config(args.config), - read_local_auth(args.auth), - args.execute_now, - args.limit_wikis, - args.force_initial_search_timestamp, + config=read_local_config(args.config), + auth=read_local_auth(args.auth), + execute_now=args.execute_now, + limit_wikis=args.limit_wikis, + force_initial_search_timestamp=args.force_initial_search_timestamp, + dry_run=args.dry_run, ) @@ -49,5 +50,12 @@ def read_command_line_arguments(): type=int, help="""The lower timestamp to use when searching for posts.""", ) + parser.add_argument( + "--dry-run", + action="store_true", + default=False, + help="""A dry run will skip remote config acquisition, new post + acquisition, and will not actually send any notifications.""", + ) return parser.parse_args() diff --git a/notifier/database/drivers/base.py b/notifier/database/drivers/base.py index c83c9e5..03d4791 100644 --- a/notifier/database/drivers/base.py +++ b/notifier/database/drivers/base.py @@ -86,10 +86,20 @@ def get_user_configs(self, frequency: str) -> List[CachedUserConfig]: """ @abstractmethod - def store_user_configs(self, user_configs: List[RawUserConfig]) -> None: + def get_notifiable_users(self, frequency: str) -> List[str]: + """Get the list of IDs for users subscribed to the given channel + frequency who have at least one notification waiting for them. + """ + + @abstractmethod + def store_user_configs( + self, user_configs: List[RawUserConfig], *, overwrite_existing=True + ) -> None: """Caches user notification configurations. :param user_configs: List of configurations for all users. + :param overwrite_existing: Whether to overwrite the existing set of + user configs. Default true. If false, will append. """ @abstractmethod diff --git a/notifier/database/drivers/mysql.py b/notifier/database/drivers/mysql.py index 03f578b..00937f0 100644 --- a/notifier/database/drivers/mysql.py +++ b/notifier/database/drivers/mysql.py @@ -319,10 +319,23 @@ def get_user_configs(self, frequency: str) -> List[CachedUserConfig]: ] return user_configs - def store_user_configs(self, user_configs: List[RawUserConfig]) -> None: + def get_notifiable_users(self, frequency: str) -> List[str]: + user_ids = [ + cast(str, row["user_id"]) + for row in self.execute_named( + "get_user_ids_for_frequency_with_notifications", + {"frequency": frequency}, + ).fetchall() + ] + return user_ids + + def store_user_configs( + self, user_configs: List[RawUserConfig], *, overwrite_existing=True + ) -> None: with self.transaction() as cursor: - # Overwrite all current configs - self.execute_named("delete_user_configs", None, cursor) + if overwrite_existing: + # Overwrite all current configs + self.execute_named("delete_user_configs", None, cursor) for user_config in user_configs: self.execute_named( "store_user_config", diff --git a/notifier/database/queries/get_posts_in_subscribed_threads.sql b/notifier/database/queries/get_posts_in_subscribed_threads.sql index dc11bbe..f18dd75 100644 --- a/notifier/database/queries/get_posts_in_subscribed_threads.sql +++ b/notifier/database/queries/get_posts_in_subscribed_threads.sql @@ -16,15 +16,15 @@ SELECT FROM post INNER JOIN - thread ON post.thread_id = thread.id + thread ON thread.id = post.thread_id INNER JOIN - wiki ON thread.wiki_id = wiki.id + wiki ON wiki.id = thread.wiki_id INNER JOIN thread_first_post ON thread_first_post.thread_id = thread.id INNER JOIN - post AS first_post ON thread_first_post.post_id = first_post.id + post AS first_post_in_thread ON first_post_in_thread.id = thread_first_post.post_id LEFT JOIN - category ON thread.category_id = category.id + category ON category.id = thread.category_id LEFT JOIN manual_sub AS thread_sub ON ( thread_sub.user_id = %(user_id)s @@ -54,7 +54,7 @@ WHERE thread_sub.sub = 1 -- Get posts in threads started by the user - OR first_post.user_id = %(user_id)s + OR first_post_in_thread.user_id = %(user_id)s ) -- Remove posts in threads unsubscribed from diff --git a/notifier/database/queries/get_replies_to_subscribed_posts.sql b/notifier/database/queries/get_replies_to_subscribed_posts.sql index 61d5eeb..f33eed2 100644 --- a/notifier/database/queries/get_replies_to_subscribed_posts.sql +++ b/notifier/database/queries/get_replies_to_subscribed_posts.sql @@ -20,13 +20,13 @@ SELECT FROM post INNER JOIN - thread ON post.thread_id = thread.id + thread ON thread.id = post.thread_id INNER JOIN - wiki ON thread.wiki_id = wiki.id + wiki ON wiki.id = thread.wiki_id INNER JOIN - post AS parent_post ON post.parent_post_id = parent_post.id + post AS parent_post ON parent_post.id = post.parent_post_id LEFT JOIN - category ON thread.category_id = category.id + category ON category.id = thread.category_id LEFT JOIN manual_sub AS post_sub ON ( post_sub.user_id = %(user_id)s diff --git a/notifier/database/queries/get_user_ids_for_frequency_with_notifications.sql b/notifier/database/queries/get_user_ids_for_frequency_with_notifications.sql new file mode 100644 index 0000000..a7ea4de --- /dev/null +++ b/notifier/database/queries/get_user_ids_for_frequency_with_notifications.sql @@ -0,0 +1,72 @@ +SELECT + user_config.user_id AS user_id +FROM + user_config + LEFT JOIN + user_last_notified ON user_config.user_id = user_last_notified.user_id +WHERE + -- Only users on the given channel + user_config.frequency = %(frequency)s + + -- Only users with a notification waiting for them + AND EXISTS ( + SELECT NULL FROM + post + INNER JOIN + thread ON thread.id = post.thread_id + LEFT JOIN + post AS parent_post ON parent_post.id = post.parent_post_id + INNER JOIN + thread_first_post ON thread_first_post.thread_id = thread.id + INNER JOIN + post AS first_post_in_thread ON first_post_in_thread.id = thread_first_post.post_id + WHERE + -- Remove deleted posts + post.is_deleted = 0 + + -- Remove posts made by the user + AND post.user_id <> user_config.user_id + + -- Only posts posted since the user was last notified + AND post.posted_timestamp > user_last_notified.notified_timestamp + + -- Remove deleted threads + AND thread.is_deleted = 0 + + -- Only posts matching thread or post subscription criteria + AND ( + -- Posts in threads started by the user + first_post_in_thread.user_id = user_config.user_id + + -- Replies to posts made by the user + OR parent_post.user_id = user_config.user_id + + -- Posts in threads subscribed to and replies to posts subscribed to + OR EXISTS ( + SELECT NULL FROM + manual_sub + WHERE + manual_sub.user_id = user_config.user_id + AND manual_sub.thread_id = thread.id + AND ( + manual_sub.post_id IS NULL -- Threads + OR manual_sub.post_id = parent_post.id -- Post replies + ) + AND manual_sub.sub = 1 + ) + ) + + -- Remove posts/replies in/to threads/posts unsubscribed from + AND NOT EXISTS ( + SELECT NULL FROM + manual_sub + WHERE + manual_sub.user_id = user_config.user_id + AND manual_sub.thread_id = thread.id + AND ( + manual_sub.post_id IS NULL -- Threads + OR manual_sub.post_id = parent_post.id -- Post replies + ) + AND manual_sub.sub = -1 + ) + ) \ No newline at end of file diff --git a/notifier/emailer.py b/notifier/emailer.py index 3d14cbb..358f701 100644 --- a/notifier/emailer.py +++ b/notifier/emailer.py @@ -1,12 +1,26 @@ +import logging +from typing import cast + import yagmail +logger = logging.getLogger(__name__) + class Emailer: # pylint: disable=too-few-public-methods """Responsible for sending emails.""" - def __init__(self, gmail_username: str, gmail_password: str): - self.yag = yagmail.SMTP(gmail_username, gmail_password) + def __init__( + self, gmail_username: str, gmail_password: str, *, dry_run=False + ): + self.dry_run = dry_run + if dry_run: + self.yag = cast(yagmail.SMTP, object()) + else: + self.yag = yagmail.SMTP(gmail_username, gmail_password) def send(self, address: str, subject: str, body: str) -> None: """Send an email to an address.""" + if self.dry_run: + logger.warn("Dry run: email send was rejected") + return self.yag.send(address, subject, body, prettify_html=False) diff --git a/notifier/main.py b/notifier/main.py index f800b42..31edbdd 100644 --- a/notifier/main.py +++ b/notifier/main.py @@ -17,19 +17,22 @@ def main( + *, config: LocalConfig, auth: AuthConfig, execute_now: List[str] = None, limit_wikis: List[str] = None, force_initial_search_timestamp: int = None, force_current_time: str = None, + dry_run=False, ): - """Main executor, supposed to be called via command line.""" + """Main notifier application entrypoint.""" logger.info("The current time is %s", now) if force_current_time is not None: override_current_time(force_current_time) + logger.info("The current time is %s", now) # Database stores forum posts and caches subscriptions DatabaseDriver = resolve_driver_from_config(config["database"]["driver"]) @@ -44,7 +47,7 @@ def main( logger.info("Wikis will be limited to %s", limit_wikis) if execute_now is None: - logger.info("Starting in scheduled mode") + logger.info("execute_now not present. Starting in scheduled mode") # Scheduler is responsible for executing tasks at the right times scheduler = BlockingScheduler() @@ -52,12 +55,13 @@ def main( # Schedule the task scheduler.add_job( lambda: notify( - config, - auth, - pick_channels_to_notify(), - database, - limit_wikis, - force_initial_search_timestamp, + config=config, + auth=auth, + active_channels=pick_channels_to_notify(), + database=database, + limit_wikis=limit_wikis, + force_initial_search_timestamp=force_initial_search_timestamp, + dry_run=dry_run, ), CronTrigger.from_crontab(notification_channels["hourly"]), ) @@ -65,19 +69,22 @@ def main( # Start the service scheduler.start() else: - logger.info("Starting in instant execution mode") + logger.info( + "execute_now list provided. Starting in instant execution mode" + ) # Choose which channels to activate channels = pick_channels_to_notify(execute_now) # Run immediately and once only notify( - config, - auth, - channels, - database, - limit_wikis, - force_initial_search_timestamp, + config=config, + auth=auth, + active_channels=channels, + database=database, + limit_wikis=limit_wikis, + force_initial_search_timestamp=force_initial_search_timestamp, + dry_run=dry_run, ) print("Finished") diff --git a/notifier/notify.py b/notifier/notify.py index 1b72974..e27b75f 100644 --- a/notifier/notify.py +++ b/notifier/notify.py @@ -70,12 +70,14 @@ def pick_channels_to_notify(force_channels: List[str] = None) -> List[str]: def notify( + *, config: LocalConfig, auth: AuthConfig, active_channels: List[str], database: BaseDatabaseDriver, limit_wikis: List[str] = None, force_initial_search_timestamp: int = None, + dry_run=False, ): """Main task executor. Should be called as often as the most frequent notification digest. @@ -90,36 +92,53 @@ def notify( logger.warning("No active channels; aborting") return - connection = Connection(config, database.get_supported_wikis()) + connection = Connection( + config, database.get_supported_wikis(), dry_run=dry_run + ) - logger.info("Getting remote config...") - get_global_config(config, database, connection) - logger.info("Getting user config...") - get_user_config(config, database, connection) + if dry_run: + logger.info("Dry run: skipping remote config acquisition") + else: + logger.info("Getting remote config...") + get_global_config(config, database, connection) + logger.info("Getting user config...") + get_user_config(config, database, connection) - # Refresh the connection to add any newly-configured wikis - connection = Connection(config, database.get_supported_wikis()) + # Refresh the connection to add any newly-configured wikis + connection = Connection(config, database.get_supported_wikis()) - logger.info("Getting new posts...") - get_new_posts(database, connection, limit_wikis) + if dry_run: + logger.info("Dry run: skipping new post acquisition") + else: + logger.info("Getting new posts...") + get_new_posts(database, connection, limit_wikis) # Record the 'current' timestamp immediately after downloading posts current_timestamp = int(time.time()) # Get the password from keyring for login wikidot_password = auth["wikidot_password"] - connection.login(config["wikidot_username"], wikidot_password) + + if dry_run: + logger.info("Dry run: skipping Wikidot login") + else: + connection.login(config["wikidot_username"], wikidot_password) logger.info("Notifying...") notify_active_channels( active_channels, - current_timestamp, - config, - auth, - database, - connection, - force_initial_search_timestamp, + current_timestamp=current_timestamp, + config=config, + auth=auth, + database=database, + connection=connection, + force_initial_search_timestamp=force_initial_search_timestamp, + dry_run=dry_run, ) + if dry_run: + logger.info("Dry run: skipping cleanup") + return + # Notifications have been sent, so perform time-insensitive maintenance logger.info("Cleaning up...") @@ -137,40 +156,46 @@ def notify( def notify_active_channels( active_channels: Iterable[str], + *, current_timestamp: int, config: LocalConfig, auth: AuthConfig, database: BaseDatabaseDriver, connection: Connection, force_initial_search_timestamp: int = None, + dry_run=False, ): """Prepare and send notifications to all activated channels.""" digester = Digester(config["path"]["lang"]) - emailer = Emailer(config["gmail_username"], auth["gmail_password"]) + emailer = Emailer( + config["gmail_username"], auth["gmail_password"], dry_run=dry_run + ) for channel in active_channels: # Should this be asynchronous + parallel? notify_channel( channel, - current_timestamp, - force_initial_search_timestamp, + current_timestamp=current_timestamp, + force_initial_search_timestamp=force_initial_search_timestamp, config=config, database=database, connection=connection, digester=digester, emailer=emailer, + dry_run=dry_run, ) def notify_channel( channel: str, + *, current_timestamp: int, force_initial_search_timestamp: int = None, - *, config: LocalConfig, database: BaseDatabaseDriver, connection: Connection, digester: Digester, emailer: Emailer, + dry_run=False, ): """Compiles and sends notifications for all users in a given channel.""" logger.info("Activating channel %s", {"channel": channel}) @@ -180,6 +205,22 @@ def notify_channel( "Found users for channel %s", {"user_count": len(user_configs), "channel": channel}, ) + # Filter the users only to those with notifications waiting + logger.debug("Filtering users without notifications waiting...") + user_count_pre_filter = len(user_configs) + notifiable_user_ids = database.get_notifiable_users(channel) + user_configs = [ + user for user in user_configs if user["user_id"] in notifiable_user_ids + ] + logger.debug( + "Filtered users without notifications waiting %s", + { + "from_count": user_count_pre_filter, + "to_count": len(user_configs), + "removed_count": user_count_pre_filter - len(user_configs), + "users_with_waiting_notifs_count": len(notifiable_user_ids), + }, + ) # Notify each user on this frequency channel notified_users = 0 addresses: EmailAddresses = {} @@ -187,15 +228,16 @@ def notify_channel( try: notified_users += notify_user( user, - channel, - current_timestamp, - force_initial_search_timestamp, + channel=channel, + current_timestamp=current_timestamp, + force_initial_search_timestamp=force_initial_search_timestamp, config=config, database=database, connection=connection, digester=digester, emailer=emailer, addresses=addresses, + dry_run=dry_run, ) except SMTPAuthenticationError as error: logger.error( @@ -228,16 +270,17 @@ def notify_channel( def notify_user( user: CachedUserConfig, + *, channel: str, current_timestamp: int, force_initial_search_timestamp: int = None, - *, config: LocalConfig, database: BaseDatabaseDriver, connection: Connection, digester: Digester, emailer: Emailer, addresses: EmailAddresses, + dry_run=False, ) -> int: """Compiles and sends a notification for a single user. @@ -303,6 +346,14 @@ def notify_user( # Compile the digest subject, body = digester.for_user(user, posts) + if dry_run: + logger.info( + "Dry run: not sending or recording notification %s", + {"for_user": user["username"]}, + ) + # Still return true to indicate that the user would have been notified + return True + # Send the digests via PM to PM-subscribed users pm_inform_tag = "restricted-inbox" if user["delivery"] == "pm": diff --git a/notifier/wikiconnection.py b/notifier/wikiconnection.py index 02effdf..746512d 100644 --- a/notifier/wikiconnection.py +++ b/notifier/wikiconnection.py @@ -56,10 +56,20 @@ class Connection: MODULE_ATTEMPT_LIMIT = 3 def __init__( - self, config: LocalConfig, supported_wikis: List[SupportedWikiConfig] + self, + config: LocalConfig, + supported_wikis: List[SupportedWikiConfig], + *, + dry_run=False, ): """Connect to Wikidot.""" - self._session = requests.sessions.Session() + self.dry_run = dry_run + if self.dry_run: + # Theoretically the session will never be used in a dry run + logger.info("Dry run: Wikidot requests will be rejected") + self._session = cast(requests.sessions.Session, object()) + else: + self._session = requests.sessions.Session() self.supported_wikis = supported_wikis # Always add the 'base' wiki, if it's not already present if not any( @@ -86,6 +96,11 @@ def __init__( def post(self, url, **kwargs): """Make a POST request.""" + if self.dry_run: + logger.warn( + "Dry run: Wikidot request was rejected %s", {"to": url} + ) + return return self._session.request("POST", url, **kwargs) def module( diff --git a/tests/run_tests.sh b/tests/run_tests.sh new file mode 100755 index 0000000..7d9a78f --- /dev/null +++ b/tests/run_tests.sh @@ -0,0 +1,8 @@ +#!/usr/bin/env sh + +cleanup() { + docker compose -f docker-compose.test.yml stop +} +trap cleanup EXIT + +docker compose -f docker-compose.test.yml up --build notifier --attach notifier --attach database --exit-code-from notifier \ No newline at end of file diff --git a/tests/test_database.py b/tests/test_database.py index 1fd34f8..19e38b6 100644 --- a/tests/test_database.py +++ b/tests/test_database.py @@ -1,4 +1,4 @@ -from typing import Any, List, Tuple +from typing import Any, Callable, List, Optional, Set, Tuple import pytest @@ -8,19 +8,48 @@ from notifier.types import ( AuthConfig, LocalConfig, + PostReplyInfo, RawPost, RawUserConfig, Subscription, + SubscriptionCardinality, SupportedWikiConfig, ThreadInfo, ) -def construct(keys: List[str], all_values: List[Tuple[Any, ...]]): - """pass""" +def construct( + keys: List[str], + all_values: List[Tuple[Any, ...]], + *, + verify: Optional[Callable[[Any], bool]] = None, +): + """Constructs a DB entry from the given keys and value sets. + + Can also run a verification check on each value set given. + """ + if verify: + for values in all_values: + assert verify(values) return [dict(zip(keys, values)) for values in all_values] +def subs( + thread_id: str, + post_id: str = None, + direction: SubscriptionCardinality = 1, +) -> List[Subscription]: + """Shorthand for constructing a single (un)subscription for a user.""" + return construct( + ["thread_id", "post_id", "sub"], [(thread_id, post_id, direction)] + ) + + +def u(id: int, name: str, subs, unsubs, *, last_ts=1): + """Shorthand for making a user shorthand for construct.""" + return (str(id), name, "hourly", "en", "pm", last_ts, "", subs, unsubs) + + @pytest.fixture(scope="module") def sample_database( notifier_config: LocalConfig, notifier_auth: AuthConfig @@ -37,14 +66,6 @@ def sample_database( password=notifier_auth["mysql_password"], ) db.scrub_database() - subs: List[Subscription] = construct( - ["thread_id", "post_id", "sub"], - [("t-1", None, 1), ("t-3", "p-32", 1)], - ) - unsubs: List[Subscription] = construct( - ["thread_id", "post_id", "sub"], - [("t-4", None, -1)], - ) sample_user_configs: List[RawUserConfig] = construct( [ "user_id", @@ -57,7 +78,20 @@ def sample_database( "subscriptions", "unsubscriptions", ], - [("1", "MyUser", "hourly", "en", "pm", 1, "", subs, unsubs)], + [ + u( + 1, + "UserR1", + construct( + ["thread_id", "post_id", "sub"], + [("t-1", None, 1), ("t-3", "p-32", 1)], + ), + construct( + ["thread_id", "post_id", "sub"], + [("t-4", None, -1)], + ), + ) + ], ) sample_wikis: List[SupportedWikiConfig] = construct( ["id", "name", "secure"], [("my-wiki", "My Wiki", 1)] @@ -73,10 +107,11 @@ def sample_database( "created_timestamp", ], [ - ("t-1", "Thread 1", "my-wiki", None, None, "MyUser", 10, "p-11"), - ("t-2", "Thread 2", "my-wiki", None, None, "MyUser", 13, "p-21"), - ("t-3", "Thread 3", "my-wiki", None, None, "AUser", 16, "p-31"), - ("t-4", "Thread 4", "my-wiki", None, None, "MyUser", 50, "p-41"), + ("t-0", "Null thread", "my-wiki", None, None, "system", 0), + ("t-1", "Thread 1", "my-wiki", None, None, "UserR1", 10), + ("t-2", "Thread 2", "my-wiki", None, None, "UserR1", 13), + ("t-3", "Thread 3", "my-wiki", None, None, "UserD1", 16), + ("t-4", "Thread 4", "my-wiki", None, None, "UserR1", 50), ], ) sample_thread_first_posts = [ @@ -97,19 +132,22 @@ def sample_database( "username", ], [ - ("p-11", "t-1", None, 10, "Post 11", "", "1", "MyUser"), - ("p-12", "t-1", None, 20, "Post 12", "", "2", "AUser"), - ("p-111", "t-1", "p-11", 30, "Post 111", "", "2", "AUser"), - ("p-21", "t-2", None, 13, "Post 21", "", "1", "MyUser"), - ("p-211", "t-2", "p-21", 17, "Post 211", "", "2", "AUser"), - ("p-212", "t-2", "p-21", 20, "Post 212", "", "3", "BUser"), - ("p-2121", "t-2", "p-212", 23, "Post 2121", "", "1", "MyUser"), - ("p-31", "t-3", None, 16, "Post 31", "", "2", "AUser"), - ("p-32", "t-3", None, 21, "Post 32", "", "3", "BUser"), - ("p-321", "t-3", "p-32", 31, "Post 321", "", "2", "AUser"), - ("p-41", "t-4", None, 50, "Post 41", "", "1", "MyUser"), - ("p-411", "t-4", "p-41", 60, "Post 411", "", "3", "BUser"), - ("p-42", "t-4", None, 65, "Post 42", "", "3", "BUser"), + ("p-11", "t-1", None, 10, "Post 11", "", "1", "UserR1"), + ("p-111", "t-1", "p-11", 30, "Post 111", "", "2", "UserD1"), + ("p-12", "t-1", None, 20, "Post 12", "", "2", "UserD1"), + # + ("p-21", "t-2", None, 13, "Post 21", "", "1", "UserR1"), + ("p-211", "t-2", "p-21", 17, "Post 211", "", "2", "UserD1"), + ("p-212", "t-2", "p-21", 20, "Post 212", "", "3", "UserD2"), + ("p-2121", "t-2", "p-212", 23, "Post 2121", "", "1", "UserR1"), + # + ("p-31", "t-3", None, 16, "Post 31", "", "2", "UserD1"), + ("p-32", "t-3", None, 21, "Post 32", "", "3", "UserD2"), + ("p-321", "t-3", "p-32", 31, "Post 321", "", "2", "UserD1"), + # + ("p-41", "t-4", None, 50, "Post 41", "", "1", "UserR1"), + ("p-411", "t-4", "p-41", 60, "Post 411", "", "3", "UserD2"), + ("p-42", "t-4", None, 65, "Post 42", "", "3", "UserD2"), ], ) db.store_user_configs(sample_user_configs) @@ -123,7 +161,7 @@ def sample_database( return db -def titles(posts): +def titles(posts: List[PostReplyInfo]) -> Set[str]: """Get a set of post titles from a list of posts.""" return set(p["title"] for p in posts) @@ -252,3 +290,113 @@ def check_timestamp(n): }, ) check_timestamp(2) + + +def test_get_notifiable_users(sample_database: BaseDatabaseDriver): + """Test that the notifiable users list returns the correct set of users. + + The notifiable users utility lists directly from the database the set of + users who have unsent notifications waiting for them, for a given channel. + """ + # Let's add another test thread and some more subscribed users + sample_user_configs: List[RawUserConfig] = construct( + [ + "user_id", + "username", + "frequency", + "language", + "delivery", + "user_base_notified", + "tags", + "subscriptions", + "unsubscriptions", + ], + # Users scoped to this test are named like "Thread5User-" + [ + # Participated, but is manually unsubbed + u(50, "T5U-Unsub", [], subs("t-5", None, -1)), + # Did not participate, but is manually subbed + u(51, "T5U-!P-Sub", subs("t-5"), []), + # Posted but was not replied to + u(52, "T5U-Lonely", [], []), + # Started the thread + u(53, "T5U-Starter", [], []), + # Posted one reply, then replied to that reply + u(54, "T5U-SelfRep", [], []), + # Posted and was replied to + u(55, "T5U-Poster", [], []), + # Posted and was replied to, but is unsubbed from their post + u(56, "T5U-UnsubPost", [], subs("t-5", "p-54", -1)), + # Posted and was replied to, but has been notified already + u(57, "T5U-PrevNotif", [], [], last_ts=200), + # Irrelevant user who is subbed elsewhere + u(58, "T5U-Irrel", subs("t-0"), []), + ], + ) + sample_threads: List[ThreadInfo] = construct( + [ + "id", + "title", + "wiki_id", + "category_id", + "category_name", + "creator_username", + "created_timestamp", + ], + [("t-5", "Thread 5", "my-wiki", None, None, "T5U-Starter", 100)], + ) + + def verify_post_author_id_matches_username(post: Tuple[Any, ...]) -> bool: + """Check that the user ID+name pair match one of the scoped configs.""" + return any( + post[6] == user["user_id"] and post[7] == user["username"] + for user in sample_user_configs + ) + + sample_posts: List[RawPost] = construct( + [ + "id", + "thread_id", + "parent_post_id", + "posted_timestamp", + "title", + "snippet", + "user_id", + "username", + ], + [ + ("p-51", "t-5", None, 100, "Post 51", "", "53", "T5U-Starter"), + ("p-52", "t-5", None, 101, "Post 52", "", "55", "T5U-Poster"), + ("p-521", "t-5", "p-52", 102, "Post 521", "", "52", "T5U-Lonely"), + ("p-53", "t-5", None, 103, "Post 53", "", "54", "T5U-SelfRep"), + ("p-531", "t-5", "p-53", 104, "Post 531", "", "54", "T5U-SelfRep"), + ("p-54", "t-5", None, 106, "Post 54", "", "56", "T5U-UnsubPost"), + ("p-541", "t-5", "p-54", 105, "Post 541", "", "52", "T5U-Lonely"), + ("p-55", "t-5", None, 106, "Post 55", "", "50", "T5U-Unsub"), + ("p-551", "t-5", "p-55", 107, "Post 551", "", "52", "T5U-Lonely"), + ("p-56", "t-5", None, 108, "Post 56", "", "57", "T5U-PrevNotif"), + ("p-561", "t-5", "p-56", 109, "Post 561", "", "52", "T5U-Lonely"), + ], + verify=verify_post_author_id_matches_username, + ) + sample_database.store_user_configs( + sample_user_configs, overwrite_existing=False + ) + for thread in sample_threads: + sample_database.store_thread(thread) + sample_database.store_thread_first_post("t-5", "p-51") + for post in sample_posts: + sample_database.store_post(post) + + users_expected_to_have_notifications = { + "1", # UserR1 from base sample DB + "51", # T5U-!P-Sub + "53", # T5U-Starter + "55", # T5U-Poster + } + + users_with_notifications = sample_database.get_notifiable_users("hourly") + + assert ( + set(users_with_notifications) == users_expected_to_have_notifications + )