diff --git a/redbot/cogs/audio/manager.py b/redbot/cogs/audio/manager.py index 6d5d3c6d701..bffaf2adf21 100644 --- a/redbot/cogs/audio/manager.py +++ b/redbot/cogs/audio/manager.py @@ -106,7 +106,28 @@ LAVALINK_VERSION_LINE_PRE35: Final[Pattern] = re.compile( rb"^Version:\s+(?P\S+)$", re.MULTILINE | re.VERBOSE ) -# used for LL 3.5-rc4 and newer +# used for LL versions >=3.5-rc4 but below 3.6. +# Since this only applies to historical version, this regex is based only on +# version numbers that actually existed, not ones that technically could. +LAVALINK_VERSION_LINE_PRE36: Final[Pattern] = re.compile( + rb""" + ^ + Version:\s+ + (?P + (?P3)\.(?P[0-5]) + # Before LL 3.6, when patch version == 0, it was stripped from the version string + (?:\.(?P[1-9]\d*))? + # Before LL 3.6, the dot in rc.N was optional + (?:-rc\.?(?P0|[1-9]\d*))? + # additional build metadata, can be used by our downstream Lavalink + # if we need to alter an upstream release + (?:\+red\.(?P[1-9]\d*))? + ) + $ + """, + re.MULTILINE | re.VERBOSE, +) +# used for LL 3.6 and newer # This regex is limited to the realistic usage in the LL version number, # not everything that could be a part of it according to the spec. # We can easily release an update to this regex in the future if it ever becomes necessary. @@ -115,11 +136,8 @@ ^ Version:\s+ (?P - (?P0|[1-9]\d*)\.(?P0|[1-9]\d*) - # Before LL 3.6, when patch version == 0, it was stripped from the version string - (?:\.(?P0|[1-9]\d*))? - # Before LL 3.6, the dot in rc.N was optional - (?:-rc\.?(?P0|[1-9]\d*))? + (?P0|[1-9]\d*)\.(?P0|[1-9]\d*)\.(?P0|[1-9]\d*) + (?:-rc\.(?P0|[1-9]\d*))? # additional build metadata, can be used by our downstream Lavalink # if we need to alter an upstream release (?:\+red\.(?P[1-9]\d*))? @@ -142,10 +160,16 @@ def __str__(self) -> None: def from_version_output(cls, output: bytes) -> Self: build_match = LAVALINK_BUILD_LINE.search(output) if build_match is None: - raise ValueError("Could not find Build line in the given `--version` output.") + raise ValueError( + "Could not find 'Build' line in the given `--version` output," + " or invalid build number given." + ) version_match = LAVALINK_VERSION_LINE_PRE35.search(output) if version_match is None: - raise ValueError("Could not find Version line in the given `--version` output.") + raise ValueError( + "Could not find 'Version' line in the given `--version` output," + " or invalid version number given." + ) return cls( raw_version=version_match["version"].decode(), build_number=int(build_match["build"]), @@ -206,16 +230,22 @@ def __init__( def __str__(self) -> None: version = f"{self.major}.{self.minor}.{self.patch}" if self.rc is not None: - version += f"-rc{self.rc}" + version += f"-rc.{self.rc}" if self.red: - version += f"_red{self.red}" + version += f"+red.{self.red}" return version @classmethod def from_version_output(cls, output: bytes) -> Self: match = LAVALINK_VERSION_LINE.search(output) if match is None: - raise ValueError("Could not find Version line in the given `--version` output.") + # >=3.5-rc4, <3.6 + match = LAVALINK_VERSION_LINE_PRE36.search(output) + if match is None: + raise ValueError( + "Could not find 'Version' line in the given `--version` output," + " or invalid version number given." + ) return LavalinkVersion( major=int(match["major"]), minor=int(match["minor"]), @@ -583,29 +613,34 @@ async def _is_up_to_date(self): stdout = (await _proc.communicate())[0] if (branch := LAVALINK_BRANCH_LINE.search(stdout)) is None: # Output is unexpected, suspect corrupted jarfile - return False + raise ValueError( + "Could not find 'Branch' line in the `--version` output," + " or invalid branch name given." + ) if (java := LAVALINK_JAVA_LINE.search(stdout)) is None: # Output is unexpected, suspect corrupted jarfile - return False + raise ValueError( + "Could not find 'JVM' line in the `--version` output," + " or invalid version number given." + ) if (lavaplayer := LAVALINK_LAVAPLAYER_LINE.search(stdout)) is None: # Output is unexpected, suspect corrupted jarfile - return False + raise ValueError( + "Could not find 'Lavaplayer' line in the `--version` output," + " or invalid version number given." + ) if (buildtime := LAVALINK_BUILD_TIME_LINE.search(stdout)) is None: # Output is unexpected, suspect corrupted jarfile - return False + raise ValueError( + "Could not find 'Build time' line in the `--version` output," + " or invalid build time given." + ) - if (build := LAVALINK_BUILD_LINE.search(stdout)) is not None: - try: - self._lavalink_version = LavalinkOldVersion.from_version_output(stdout) - except ValueError: - # Output is unexpected, suspect corrupted jarfile - return False - else: - try: - self._lavalink_version = LavalinkVersion.from_version_output(stdout) - except ValueError: - # Output is unexpected, suspect corrupted jarfile - return False + self._lavalink_version = ( + LavalinkOldVersion.from_version_output(stdout) + if LAVALINK_BUILD_LINE.search(stdout) is not None + else LavalinkVersion.from_version_output(stdout) + ) date = buildtime["build_time"].decode() date = date.replace(".", "/") self._lavalink_branch = branch["branch"].decode() @@ -616,7 +651,24 @@ async def _is_up_to_date(self): return self._up_to_date async def maybe_download_jar(self): - if not (self.lavalink_jar_file.exists() and await self._is_up_to_date()): + if not self.lavalink_jar_file.exists(): + log.info("Triggering first-time download of Lavalink...") + await self._download_jar() + return + + try: + up_to_date = await self._is_up_to_date() + except ValueError as exc: + log.warning("Failed to get Lavalink version: %s\nTriggering update...", exc) + await self._download_jar() + return + + if not up_to_date: + log.info( + "Lavalink version outdated, triggering update from %s to %s...", + self._lavalink_version, + self.JAR_VERSION, + ) await self._download_jar() async def wait_until_ready(self, timeout: Optional[float] = None): diff --git a/tests/cogs/audio/test_manager.py b/tests/cogs/audio/test_manager.py index 9afb86e5203..25deb8062ae 100644 --- a/tests/cogs/audio/test_manager.py +++ b/tests/cogs/audio/test_manager.py @@ -1,4 +1,5 @@ import itertools +from typing import Optional import pytest @@ -43,30 +44,88 @@ def test_old_ll_version_parsing( raw_version: str, raw_build_number: str, expected: LavalinkOldVersion ) -> None: line = b"Version: %b\nBuild: %b" % (raw_version.encode(), raw_build_number.encode()) - assert LavalinkOldVersion.from_version_output(line) + actual = LavalinkOldVersion.from_version_output(line) + assert actual == expected + assert str(actual) == f"{raw_version}_{raw_build_number}" + + +def _generate_ll_version_line(raw_version: str) -> bytes: + return b"Version: " + raw_version.encode() @pytest.mark.parametrize( - "raw_version,expected", + "raw_version,expected_str,expected", ( # older version format that allowed stripped `.0` and no dot in `rc.4`, used until LL 3.6 - ("3.5-rc4", LavalinkVersion(3, 5, rc=4)), - ("3.5", LavalinkVersion(3, 5)), + ("3.5-rc4", "3.5.0-rc.4", LavalinkVersion(3, 5, rc=4)), + ("3.5", "3.5.0", LavalinkVersion(3, 5)), # newer version format - ("3.6.0-rc.1", LavalinkVersion(3, 6, 0, rc=1)), + ("3.6.0-rc.1", None, LavalinkVersion(3, 6, 0, rc=1)), # downstream RC version with `+red.N` suffix - ("3.7.5-rc.1+red.1", LavalinkVersion(3, 7, 5, rc=1, red=1)), - ("3.7.5-rc.1+red.123", LavalinkVersion(3, 7, 5, rc=1, red=123)), + ("3.7.5-rc.1+red.1", None, LavalinkVersion(3, 7, 5, rc=1, red=1)), + ("3.7.5-rc.1+red.123", None, LavalinkVersion(3, 7, 5, rc=1, red=123)), # upstream stable version - ("3.7.5", LavalinkVersion(3, 7, 5)), + ("3.7.5", None, LavalinkVersion(3, 7, 5)), # downstream stable version with `+red.N` suffix - ("3.7.5+red.1", LavalinkVersion(3, 7, 5, red=1)), - ("3.7.5+red.123", LavalinkVersion(3, 7, 5, red=123)), + ("3.7.5+red.1", None, LavalinkVersion(3, 7, 5, red=1)), + ("3.7.5+red.123", None, LavalinkVersion(3, 7, 5, red=123)), + ), +) +def test_ll_version_parsing( + raw_version: str, expected_str: Optional[str], expected: LavalinkVersion +) -> None: + line = _generate_ll_version_line(raw_version) + actual = LavalinkVersion.from_version_output(line) + expected_str = expected_str or raw_version + assert actual == expected + assert str(actual) == expected_str + + +@pytest.mark.parametrize( + "raw_version", + ( + # 3.5.0-rc4 is first version to not have build number + # 3.5 stripped `.0` from version number + "3.5", + # RC version don't need a dot for RC versions... + "3.5-rc4", + # ...but that doesn't mean they can't + "3.5-rc.5", + # regular 3.5.x version + "3.5.5", + # one more RC version with non-zero patch version + "3.5.5-rc1", + ), +) +def test_ll_version_accepts_less_strict_below_3_6(raw_version: str) -> None: + line = _generate_ll_version_line(raw_version) + # check that the version can be parsed + LavalinkVersion.from_version_output(line) + + +@pytest.mark.parametrize( + "raw_version", + ( + # `.0` releases <3.6 had their `.0` stripped so this is not valid: + "3.5.0-rc4", + # 3.6 is first to require stricter format + "3.6.0-rc4", + "3.6", + # another single digit minor version newer than 3.6 + "3.7", + # double digit minor version + "3.11.3-rc1", + # newer major version + "4.0.0-rc5", + # double digit major version + "11.0.0-rc5", ), ) -def test_ll_version_parsing(raw_version: str, expected: LavalinkVersion) -> None: - line = b"Version: " + raw_version.encode() - assert LavalinkVersion.from_version_output(line) +def test_ll_version_rejects_less_strict_on_3_6_and_above(raw_version: str) -> None: + line = _generate_ll_version_line(raw_version) + + with pytest.raises(ValueError): + LavalinkVersion.from_version_output(line) def test_ll_version_comparison() -> None: