Validate Discord channel names

CHANGES.md

@@ -7,6 +7,10 @@
 * Change default behavior to automatically create missing Discord channels
   ([issue#23](https://github.com/richfromm/slack2discord/issues/23))
     * Former option `--create` is now `--no-create`
+* Validate Discord channel names
+  ([issue#24](https://github.com/richfromm/slack2discord/issues/24))
+* Begin adding automated tests, via pytest
+    * These also run automatically in GitHub
 
 ### 2.7
 

README.md

@@ -303,8 +303,13 @@ the required dev packages into your virtualenv:
 
     pip install -r requirements-dev.txt
 
-This will allow you to run
-[mypy](https://mypy.readthedocs.io/en/stable/) static typing checks:
+This will allow you to run automated tests via
+[pytest](https://docs.pytest.org/en/latest/):
+
+    pytest
+
+As well as [mypy](https://mypy.readthedocs.io/en/stable/) static
+typing checks:
 
     mypy slack2discord.py
 
@@ -318,7 +323,7 @@ checks:
 
     flake8 slack2discord.py slack2discord
 
-You can automatically run both checks with `./check.sh`. GitHub is configured
+You can automatically run all checks with `./check.sh`. GitHub is configured
 (via `.github/workflows/check.yaml` and [GitHub
 Actions](https://docs.github.com/en/actions)) to automatically run these checks
 on any PRs, to require the checks to pass before merging to `master`, and to
@@ -331,7 +336,7 @@ Some items I am considering:
 * Better error reporting, so that if an entire import is not
   successful, it is easier to resume in a way as to avoid duplicates.
 
-* Add unit tests
+* Add more automated tests
 
 * Ways to optimize file downloads:
     * Download multiple files asynchronously via using

check.sh

@@ -1,11 +1,12 @@
 #!/bin/bash
 
 # this assumes that all project dependencies, as well as all dev dependencies
-# (e.g. flake8 and mypy) have already been installed, e.g. via:
+# (e.g. flake8, mypy, and pytest) have already been installed, e.g. via:
 #    pip install -r requrements.txt
 #    pip install -r requrements-dev.txt
 
 # flake8 and mypy config options are specified in setup.cfg
+# (pytest can be as well, although there is not yet any pytest non-default config)
 
 # colored output: https://www.tutorialspoint.com/how-to-output-colored-text-to-a-linux-terminal
 red="\033[1;31m"
@@ -40,6 +41,18 @@ else
    check_result=1
 fi
 
+echo
+echo "Run pytest tests"
+echo "pytest"
+pytest
+pytest_result=$?
+if [[ $pytest_result -eq 0 ]]; then
+   echo -e "pytest: $pass"
+else
+   echo -e "pytest: $fail"
+   check_result=1
+fi
+
 echo
 if [[ $check_result -eq 0 ]]; then
    echo -e "Union of all checks: $pass"

requirements-dev.txt

@@ -1,3 +1,4 @@
+pytest
 mypy
 types-decorator
 types-requests

setup.cfg

@@ -3,3 +3,7 @@ show_error_codes = True
 
 [flake8]
 max-line-length = 99
+
+# pytest can be configured here via [tool:pytest]
+# although it is not recommended:
+# https://docs.pytest.org/en/latest/reference/customize.html#setup-cfg

slack2discord/client.py

@@ -2,6 +2,7 @@
 from decorator import decorator
 import logging
 from pprint import pprint
+from re import match
 from traceback import print_exc
 from typing import cast, Callable, NewType, Optional, Union, Sequence
 
@@ -314,6 +315,45 @@ async def post_messages(self) -> None:
         finally:
             await self.close()
 
+    @staticmethod
+    def valid_channel_name(channel_name: str) -> bool:
+        """
+        Return a boolean indicating if the input string is a legal Discord channel name.
+
+        The following criteria must be met:
+        * Names must contain only alphanumeric, dash (-), and/or underscore (_) characters
+        * Length must be between 1 and 100 chars (inclusive)
+        * Multiple dashes in a row (e.g. --) are not permitted
+
+        The actual reality of Discord channel creation is a bit more complex. Empirically, there
+        are numerous (undocumented?) cases where an input channel name does **not** fail to create
+        a channel, but the channel created does not have precisely the same name as the input. For
+        example:
+        * Multiple dashes in a row are converted into a single dash
+        * Space and tilde are converted to dash
+        * Tab and newline are removed
+        * All other symbols are removed
+
+        For our purposes, this is too complex to deal with, and we will fail to validate any input
+        that does not result in creating a channel with the same name as the input.
+        """
+        if not match(r'\A[A-Za-z0-9\-_]+\Z', channel_name):
+            logger.error("Discord channel name must contain only alphanumeric,"
+                         f" dash, and/or underscore: {channel_name}")
+            return False
+
+        if not (1 <= len(channel_name) <= 100):
+            logger.error(f"Discord channel name must be between 1 and 100 chars: {channel_name}")
+            return False
+
+        if match('.*--.*', channel_name):
+            logger.error("Discord channel name can not have multiple dashes"
+                         f" in a row: {channel_name}")
+            return False
+
+        # if none of the above problems were found, we're good
+        return True
+
     async def post_messages_to_channel(
             self,
             channel: Optional[discord.TextChannel],

slack2discord/parser.py

@@ -6,6 +6,8 @@
 from re import match, sub, Match
 from typing import cast, Any, NewType, Optional, Union
 
+# This import moved to within parse() to solve circular import problem
+# from .client import DiscordClient
 from .message import ParsedMessage
 
 
@@ -409,7 +411,22 @@ def parse(self) -> None:
         """
         self.parse_users()
 
+        # populate a map from slack to discord channel names
         self.set_channel_map()
+
+        # validate the discord channel names
+        # this import moved from the top of the file to solve circular import problem
+        from .client import DiscordClient
+        invalid_discord_channels = [discord_channel
+                                    for discord_channel in self.channel_map.values()
+                                    if not DiscordClient.valid_channel_name(discord_channel)]
+        if invalid_discord_channels:
+            fail_msg = f"Discord channel name(s) fail validation: {invalid_discord_channels}"
+            logger.error(fail_msg)
+            logger.info('You can use "--channel-file" to rename Slack channels when migrating.')
+            raise RuntimeError(fail_msg)
+
+        # iterate through the channel map, parsing the channels in the slack export
         for slack_channel, discord_channel in self.channel_map.items():
             self.parse_channel(slack_channel, discord_channel)
 

slack2discord/tests/test_client.py

@@ -0,0 +1,50 @@
+import pytest
+
+from ..client import DiscordClient
+
+
+class TestDiscordClient():
+    @pytest.mark.parametrize("channel_name", [
+        "general",
+        "foo-bar",
+        "foo_bar",
+        "foo-_bar",
+        "foo_-bar",
+        "foo__bar",
+        "foo-_-bar",
+        "foo_-_bar",
+        "foo-bar-baz",
+        "foo_bar_baz",
+        "foo-bar_baz",
+        "foo_bar-baz",
+        "a",
+        "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz1234567890-_",
+        "123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789",  # noqa: E501
+        "1234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890",  # noqa: E501
+    ])
+    def test_valid_channel_name(self, channel_name):
+        """
+        Test Discord channel names that pass validation
+        """
+        assert DiscordClient.valid_channel_name(channel_name)
+
+    @pytest.mark.parametrize("channel_name", [
+        "foo--bar",
+        "foo---bar",
+        "foo----bar",
+        "",
+        " ",
+        "foo bar",
+        "foo	bar",
+        "foo#bar",
+        "foo`-=bar",
+        "foo~!@#$%^&*()_+bar",
+        r"foo[]\;',./bar",  # don't want the backslash to be treated as an escape
+        'foo{}|:"<>?bar',
+        "12345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901",  # noqa: E501
+    ])
+    def test_invalid_channel_name(self, channel_name):
+        """
+        Test Discord channel names that fail validation
+        """
+        assert not DiscordClient.valid_channel_name(channel_name)