diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 8f22746f..9b996b5e 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -12,40 +12,38 @@ jobs: fail-fast: false matrix: include: - - name: "Test: Python 3.9" - python: "3.9" - tox: py39 - - name: "Test: Python 3.10" - python: "3.10" - tox: py310 - name: "Test: Python 3.11" python: "3.11" tox: py311 + - name: "Test: Python 3.12" + python: "3.12" + tox: py312 coverage: true - - name: "Lint: check-manifest" - python: "3.11" - tox: check-manifest - - name: "Lint: flake8" - python: "3.11" - tox: flake8 + - name: "Lint: ruff lint" + python: "3.12" + tox: ruff-lint + - name: "Lint: ruff format" + python: "3.12" + tox: ruff-format name: ${{ matrix.name }} runs-on: ubuntu-22.04 container: ghcr.io/mopidy/ci:latest steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 - name: Fix home dir permissions to enable pip caching run: chown -R root /github/home - - uses: actions/setup-python@v4 + - uses: actions/setup-python@v5 with: python-version: ${{ matrix.python }} cache: pip - cache-dependency-path: setup.cfg - run: python -m pip install pygobject tox - run: python -m tox -e ${{ matrix.tox }} if: ${{ ! matrix.coverage }} - run: python -m tox -e ${{ matrix.tox }} -- --cov-report=xml if: ${{ matrix.coverage }} - - uses: codecov/codecov-action@v3 + - uses: codecov/codecov-action@v4 if: ${{ matrix.coverage }} + with: + token: ${{ secrets.CODECOV_TOKEN }} diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index f1dbed8d..133b557a 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -9,14 +9,14 @@ jobs: runs-on: ubuntu-22.04 steps: - - uses: actions/checkout@v3 - - uses: actions/setup-python@v4 - with: - python-version: "3.11" - - name: "Install dependencies" - run: python3 -m pip install build - - name: "Build package" - run: python3 -m build - - uses: pypa/gh-action-pypi-publish@v1.8.1 - with: - password: ${{ secrets.PYPI_TOKEN }} + - uses: actions/checkout@v4 + - uses: actions/setup-python@v5 + with: + python-version: "3.12" + - name: "Install dependencies" + run: python3 -m pip install build + - name: "Build package" + run: python3 -m build + - uses: pypa/gh-action-pypi-publish@release/v1 + with: + password: ${{ secrets.PYPI_TOKEN }} diff --git a/.gitignore b/.gitignore index 03640c9d..dadde9df 100644 --- a/.gitignore +++ b/.gitignore @@ -1,9 +1,8 @@ +*.egg-info *.pyc -/.coverage -/.mypy_cache/ -/.pytest_cache/ -/.tox/ -/*.egg-info -/build/ -/dist/ -/MANIFEST +.coverage +.mypy_cache/ +.pytest_cache/ +.tox/ +build/ +dist/ diff --git a/MANIFEST.in b/MANIFEST.in deleted file mode 100644 index 9d9fd9b3..00000000 --- a/MANIFEST.in +++ /dev/null @@ -1,14 +0,0 @@ -include *.py -include *.rst -include .mailmap -include LICENSE -include MANIFEST.in -include pyproject.toml -include tox.ini - -recursive-include .github * - -include mopidy_*/ext.conf - -recursive-include tests *.py -recursive-include tests/data * diff --git a/pyproject.toml b/pyproject.toml index 04a65244..97c13b46 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,17 +1,112 @@ [build-system] -requires = ["setuptools >= 30.3.0", "wheel"] +requires = ["setuptools >= 66", "setuptools-scm >= 7.1"] +build-backend = "setuptools.build_meta" -[tool.black] -target-version = ["py39", "py310", "py311"] -line-length = 80 +[project] +name = "Mopidy-Spotify" +description = "Mopidy extension for playing music from Spotify" +readme = "README.rst" +requires-python = ">= 3.11" +license = { text = "Apache-2.0" } +authors = [{ name = "Stein Magnus Jodal", email = "stein.magnus@jodal.no" }] +maintainers = [{ name = "Nick Steel", email = "nick@nsteel.co.uk" }] +classifiers = [ + "Environment :: No Input/Output (Daemon)", + "Intended Audience :: End Users/Desktop", + "License :: OSI Approved :: Apache Software License", + "Operating System :: OS Independent", + "Programming Language :: Python :: 3", + "Programming Language :: Python :: 3.11", + "Programming Language :: Python :: 3.12", + "Topic :: Multimedia :: Sound/Audio :: Players", +] +dynamic = ["version"] +dependencies = [ + "mopidy >= 3.4.0", + "pykka >= 4.0", + "requests >= 2.20.0", + "setuptools >= 66", +] +[project.optional-dependencies] +lint = ["ruff"] +test = ["pytest", "pytest-cov", "responses"] +typing = [] +dev = ["mopidy-spotify[lint,test,typing]", "tox"] -[tool.isort] -multi_line_output = 3 -include_trailing_comma = true -force_grid_wrap = 0 -use_parentheses = true -line_length = 88 -known_tests = "tests" -sections = "FUTURE,STDLIB,THIRDPARTY,FIRSTPARTY,TESTS,LOCALFOLDER" +[project.urls] +Source = "https://github.com/mopidy/mopidy-spotify" +Issues = "https://github.com/mopidy/mopidy-spotify/issues" + +[project.entry-points."mopidy.ext"] +spotify = "mopidy_spotify:Extension" + + +[tool.ruff] +target-version = "py311" + +[tool.ruff.lint] +select = [ + "A", # flake8-builtins + "ANN", # flake8-annotations + "ARG", # flake8-unused-arguments + "B", # flake8-bugbear + "C4", # flake8-comprehensions + "C90", # mccabe + "D", # pydocstyle + "DTZ", # flake8-datetimez + "E", # pycodestyle + "ERA", # eradicate + "F", # pyflakes + "FBT", # flake8-boolean-trap + "I", # isort + "INP", # flake8-no-pep420 + "ISC", # flake8-implicit-str-concat + "N", # pep8-naming + "PGH", # pygrep-hooks + "PIE", # flake8-pie + "PLC", # pylint convention + "PLE", # pylint error + "PLR", # pylint refactor + "PLW", # pylint warning + "PT", # flake8-pytest-style + "PTH", # flake8-use-pathlib + "Q", # flake8-quotes + "RET", # flake8-return + "RSE", # flake8-raise + "RUF", # ruff + "SIM", # flake8-simplify + "SLF", # flake8-self + "T20", # flake8-print + "TCH", # flake8-type-checking + "TID", # flake8-tidy-imports + "TRY", # tryceratops + "UP", # pyupgrade + "W", # pycodestyle +] +ignore = [ + "ANN", # flake8-annotations # TODO: Add types + "ANN101", # missing-type-self + "ANN102", # missing-type-cls + "ANN401", # any-type + "D", # pydocstyle + "ISC001", # single-line-implicit-string-concatenation + "SLF001", # private-member-access + "TRY003", # raise-vanilla-args +] + +[tool.ruff.lint.per-file-ignores] +"tests/*" = [ + "ANN", # flake8-annotations + "ARG", # flake8-unused-arguments + "D", # pydocstyle + "FBT", # flake8-boolean-trap + "PLR0913", # too-many-arguments + "PLR2004", # magic-value-comparison + "PT027", # pytest-unittest-raises-assertion + "TRY002", # raise-vanilla-class +] + + +[tool.setuptools_scm] diff --git a/setup.cfg b/setup.cfg deleted file mode 100644 index a755a557..00000000 --- a/setup.cfg +++ /dev/null @@ -1,84 +0,0 @@ -[metadata] -name = Mopidy-Spotify -version = 5.0.0a1 -url = https://github.com/mopidy/mopidy-spotify -author = Stein Magnus Jodal -author_email = stein.magnus@jodal.no -license = Apache License, Version 2.0 -license_file = LICENSE -description = Mopidy extension for playing music from Spotify -long_description = file: README.rst -classifiers = - Environment :: No Input/Output (Daemon) - Intended Audience :: End Users/Desktop - License :: OSI Approved :: Apache Software License - Operating System :: OS Independent - Programming Language :: Python :: 3 - Programming Language :: Python :: 3.9 - Programming Language :: Python :: 3.10 - Programming Language :: Python :: 3.11 - Topic :: Multimedia :: Sound/Audio :: Players - - -[options] -zip_safe = False -include_package_data = True -packages = find: -python_requires = >= 3.9 -install_requires = - Mopidy >= 3.4.0 - Pykka >= 2.0.1 - requests >= 2.20.0 - setuptools - - -[options.extras_require] -lint = - black - check-manifest - flake8 - flake8-black - flake8-bugbear - flake8-isort - isort -test = - pytest - pytest-cov - responses -dev = - %(lint)s - %(test)s - tox - - -[options.packages.find] -exclude = - tests - tests.* - - -[options.entry_points] -mopidy.ext = - spotify = mopidy_spotify:Extension - - -[flake8] -application-import-names = mopidy_spotify, tests -max-line-length = 80 -exclude = .git, .tox, build -select = - # Regular flake8 rules - C, E, F, W - # flake8-bugbear rules - B - # B950: line too long (soft speed limit) - B950 - # pep8-naming rules - N -ignore = - # E203: whitespace before ':' (not PEP8 compliant) - E203 - # E501: line too long (replaced by B950) - E501 - # W503: line break before binary operator (not PEP8 compliant) - W503 diff --git a/setup.py b/setup.py deleted file mode 100644 index 60684932..00000000 --- a/setup.py +++ /dev/null @@ -1,3 +0,0 @@ -from setuptools import setup - -setup() diff --git a/mopidy_spotify/__init__.py b/src/mopidy_spotify/__init__.py similarity index 94% rename from mopidy_spotify/__init__.py rename to src/mopidy_spotify/__init__.py index 601ff3e1..5b3130c6 100644 --- a/mopidy_spotify/__init__.py +++ b/src/mopidy_spotify/__init__.py @@ -1,9 +1,9 @@ import pathlib +from importlib.metadata import version -import pkg_resources from mopidy import config, ext -__version__ = pkg_resources.get_distribution("Mopidy-Spotify").version +__version__ = version("Mopidy-Spotify") class Extension(ext.Extension): diff --git a/mopidy_spotify/backend.py b/src/mopidy_spotify/backend.py similarity index 94% rename from mopidy_spotify/backend.py rename to src/mopidy_spotify/backend.py index be79b9f6..73c51795 100644 --- a/mopidy_spotify/backend.py +++ b/src/mopidy_spotify/backend.py @@ -50,6 +50,4 @@ def on_source_setup(self, source): source.set_property("cache-credentials", self._credentials_dir) if self._config["allow_cache"]: source.set_property("cache-files", self._cache_location) - source.set_property( - "cache-max-size", self._config["cache_size"] * 1048576 - ) + source.set_property("cache-max-size", self._config["cache_size"] * 1048576) diff --git a/mopidy_spotify/browse.py b/src/mopidy_spotify/browse.py similarity index 69% rename from mopidy_spotify/browse.py rename to src/mopidy_spotify/browse.py index 63b03182..981ee934 100644 --- a/mopidy_spotify/browse.py +++ b/src/mopidy_spotify/browse.py @@ -3,8 +3,8 @@ from mopidy import models from mopidy_spotify import playlists, translator -from mopidy_spotify.web import WebLink from mopidy_spotify.utils import flatten +from mopidy_spotify.web import WebLink logger = logging.getLogger(__name__) @@ -35,48 +35,55 @@ ] -BROWSE_DIR_URIS = set( +BROWSE_DIR_URIS = { u.uri - for u in [ROOT_DIR] - + _ROOT_DIR_CONTENTS - + _TOP_LIST_DIR_CONTENTS - + _YOUR_MUSIC_DIR_CONTENTS - + _PLAYLISTS_DIR_CONTENTS -) - - -def browse(*, config, session, web_client, uri): + for u in [ + ROOT_DIR, + *_ROOT_DIR_CONTENTS, + *_TOP_LIST_DIR_CONTENTS, + *_YOUR_MUSIC_DIR_CONTENTS, + *_PLAYLISTS_DIR_CONTENTS, + ] +} + + +def browse( # noqa: C901, PLR0911, PLR0912 + *, + config, # noqa: ARG001 + session, # noqa: ARG001 + web_client, + uri, +): if uri == ROOT_DIR.uri: return _ROOT_DIR_CONTENTS - elif uri == _TOP_LIST_DIR.uri: + if uri == _TOP_LIST_DIR.uri: return _TOP_LIST_DIR_CONTENTS - elif uri == _YOUR_MUSIC_DIR.uri: + if uri == _YOUR_MUSIC_DIR.uri: return _YOUR_MUSIC_DIR_CONTENTS - elif uri == _PLAYLISTS_DIR.uri: + if uri == _PLAYLISTS_DIR.uri: return _PLAYLISTS_DIR_CONTENTS if web_client is None or not web_client.logged_in: return [] # TODO: Support for category browsing. - if uri.startswith("spotify:user:") or uri.startswith("spotify:playlist:"): + if uri.startswith(("spotify:user:", "spotify:playlist:")): return _browse_playlist(web_client, uri) - elif uri.startswith("spotify:album:"): + if uri.startswith("spotify:album:"): return _browse_album(web_client, uri) - elif uri.startswith("spotify:artist:"): + if uri.startswith("spotify:artist:"): return _browse_artist(web_client, uri) - elif uri.startswith("spotify:top:"): + if uri.startswith("spotify:top:"): parts = uri.replace("spotify:top:", "").split(":") - if len(parts) == 1: - return _browse_toplist_user(web_client, variant=parts[0]) - else: + if len(parts) != 1: logger.info(f"Failed to browse {uri!r}: Toplist URI parsing failed") return [] - elif uri.startswith("spotify:your:"): + return _browse_toplist_user(web_client, variant=parts[0]) + if uri.startswith("spotify:your:"): parts = uri.replace("spotify:your:", "").split(":") if len(parts) == 1: return _browse_your_music(web_client, variant=parts[0]) - elif uri.startswith("spotify:playlists:"): + if uri.startswith("spotify:playlists:"): parts = uri.replace("spotify:playlists:", "").split(":") if len(parts) == 1: return _browse_playlists(web_client, variant=parts[0]) @@ -86,7 +93,12 @@ def browse(*, config, session, web_client, uri): def _browse_playlist(web_client, uri): - return playlists.playlist_lookup(web_client, uri, None, as_items=True) + return playlists.playlist_lookup( + web_client, + uri, + bitrate=None, + as_items=True, + ) def _browse_album(web_client, uri): @@ -132,12 +144,11 @@ def _browse_toplist_user(web_client, variant): if page ] ) - if variant == "tracks": - return list( - translator.web_to_track_refs(items, check_playable=False) - ) - else: - return list(translator.web_to_artist_refs(items)) + match variant: + case "tracks": + return list(translator.web_to_track_refs(items, check_playable=False)) + case "artists": + return list(translator.web_to_artist_refs(items)) else: return [] @@ -157,35 +168,35 @@ def _load_your_music(web_client, variant): if not page: continue items = page.get("items", []) - for item in items: - yield item + yield from items def _browse_your_music(web_client, variant): items = _load_your_music(web_client, variant) - if variant == "tracks": - return list(translator.web_to_track_refs(items)) - elif variant == "albums": - return list(translator.web_to_album_refs(items)) - else: - return [] + match variant: + case "tracks": + return list(translator.web_to_track_refs(items)) + case "albums": + return list(translator.web_to_album_refs(items)) + case _: + return [] def _browse_playlists(web_client, variant): if not web_client.logged_in: return [] - if variant == "featured": - items = flatten( - [ - page.get("playlists", {}).get("items", []) - for page in web_client.get_all( - "browse/featured-playlists", - params={"limit": 50}, - ) - if page - ] - ) - return list(translator.to_playlist_refs(items)) - else: + if variant != "featured": return [] + + items = flatten( + [ + page.get("playlists", {}).get("items", []) + for page in web_client.get_all( + "browse/featured-playlists", + params={"limit": 50}, + ) + if page + ] + ) + return list(translator.to_playlist_refs(items)) diff --git a/mopidy_spotify/distinct.py b/src/mopidy_spotify/distinct.py similarity index 57% rename from mopidy_spotify/distinct.py rename to src/mopidy_spotify/distinct.py index aec78256..75d56cc7 100644 --- a/mopidy_spotify/distinct.py +++ b/src/mopidy_spotify/distinct.py @@ -13,37 +13,38 @@ def get_distinct(config, playlists, web_client, field, query=None): if web_client is None or not web_client.logged_in: return set() - if field == "artist": - result = _get_distinct_artists(config, playlists, web_client, query) - elif field == "albumartist": - result = _get_distinct_albumartists( - config, playlists, web_client, query - ) - elif field == "album": - result = _get_distinct_albums(config, playlists, web_client, query) - elif field == "date": - result = _get_distinct_dates(config, playlists, web_client, query) - else: - result = set() + match field: + case "artist": + result = _get_distinct_artists(config, playlists, web_client, query) + case "albumartist": + result = _get_distinct_albumartists(config, playlists, web_client, query) + case "album": + result = _get_distinct_albums(config, playlists, web_client, query) + case "date": + result = _get_distinct_dates(config, playlists, web_client, query) + case _: + result = set() return result - {None} def _get_distinct_artists(config, playlists, web_client, query): logger.debug(f"Getting distinct artists: {query}") + if query: search_result = _get_search(config, web_client, query, artist=True) return {artist.name for artist in search_result.artists} - else: - return { - artist.name - for track in _get_playlist_tracks(config, playlists, web_client) - for artist in track.artists - } + + return { + artist.name + for track in _get_playlist_tracks(config, playlists, web_client) + for artist in track.artists + } def _get_distinct_albumartists(config, playlists, web_client, query): logger.debug(f"Getting distinct albumartists: {query}") + if query: search_result = _get_search(config, web_client, query, album=True) return { @@ -52,30 +53,32 @@ def _get_distinct_albumartists(config, playlists, web_client, query): for artist in album.artists if album.artists } - else: - return { - artists.name - for track in _get_playlist_tracks(config, playlists, web_client) - for artists in track.album.artists - if track.album and track.album.artists - } + + return { + artists.name + for track in _get_playlist_tracks(config, playlists, web_client) + for artists in track.album.artists + if track.album and track.album.artists + } def _get_distinct_albums(config, playlists, web_client, query): logger.debug(f"Getting distinct albums: {query}") + if query: search_result = _get_search(config, web_client, query, album=True) return {album.name for album in search_result.albums} - else: - return { - track.album.name - for track in _get_playlist_tracks(config, playlists, web_client) - if track.album - } + + return { + track.album.name + for track in _get_playlist_tracks(config, playlists, web_client) + if track.album + } def _get_distinct_dates(config, playlists, web_client, query): logger.debug(f"Getting distinct album years: {query}") + if query: search_result = _get_search(config, web_client, query, album=True) return { @@ -83,16 +86,22 @@ def _get_distinct_dates(config, playlists, web_client, query): for album in search_result.albums if album.date not in (None, "0") } - else: - return { - f"{track.album.date}" - for track in _get_playlist_tracks(config, playlists, web_client) - if track.album and track.album.date not in (None, 0) - } - -def _get_search( - config, web_client, query, album=False, artist=False, track=False + return { + f"{track.album.date}" + for track in _get_playlist_tracks(config, playlists, web_client) + if track.album and track.album.date not in (None, 0) + } + + +def _get_search( # noqa: PLR0913 + config, + web_client, + query, + *, + album=False, + artist=False, + track=False, ): types = [] if album: @@ -102,15 +111,23 @@ def _get_search( if track: types.append("track") - return search.search(config, web_client, query, types=types) + return search.search( + config, + web_client, + query=query, + types=types, + ) -def _get_playlist_tracks(config, playlists, web_client): +def _get_playlist_tracks( + config, + playlists, + web_client, # noqa: ARG001 +): if not playlists or not config["allow_playlists"]: return for playlist_ref in playlists.as_list(): playlist = playlists.lookup(playlist_ref.uri) if playlist: - for track in playlist.tracks: - yield track + yield from playlist.tracks diff --git a/mopidy_spotify/ext.conf b/src/mopidy_spotify/ext.conf similarity index 100% rename from mopidy_spotify/ext.conf rename to src/mopidy_spotify/ext.conf diff --git a/mopidy_spotify/images.py b/src/mopidy_spotify/images.py similarity index 73% rename from mopidy_spotify/images.py rename to src/mopidy_spotify/images.py index b7cfd8f1..05f99875 100644 --- a/mopidy_spotify/images.py +++ b/src/mopidy_spotify/images.py @@ -36,48 +36,50 @@ def get_images(web_client, uris): def _parse_uri(uri): if uri in BROWSE_DIR_URIS: - return # These are internal to the extension. + return None # These are internal to the extension. try: parsed_uri = urllib.parse.urlparse(uri) uri_type, uri_id = None, None - if parsed_uri.scheme == "spotify": - fragments = parsed_uri.path.split(":") - if len(fragments) < 2: - raise ValueError("Too few fragments") - uri_type, uri_id = parsed_uri.path.split(":")[:2] - elif parsed_uri.scheme in ("http", "https"): - if parsed_uri.netloc in ("open.spotify.com", "play.spotify.com"): - uri_type, uri_id = parsed_uri.path.split("/")[1:3] + match parsed_uri.scheme: + case "spotify": + match parsed_uri.path.split(":"): + case uri_type, uri_id, *_: + pass + case _: + raise ValueError("Too few arguments") # noqa: TRY301 + case "http" | "https": + if parsed_uri.netloc in ("open.spotify.com", "play.spotify.com"): + uri_type, uri_id = parsed_uri.path.split("/")[1:3] supported_types = ("track", "album", "artist", "playlist") if uri_type: if uri_type not in supported_types: - logger.warning( - f"Unsupported image type '{uri_type}' in {repr(uri)}" - ) - return - elif uri_id: + logger.warning(f"Unsupported image type '{uri_type}' in {uri!r}") + return None + if uri_id: return { "uri": uri, "type": uri_type, "id": uri_id, "key": (uri_type, uri_id), } - raise ValueError("Unknown error") + raise ValueError("Unknown error") # noqa: TRY301 except Exception as e: - logger.exception(f"Could not parse {repr(uri)} as a Spotify URI ({e})") + logger.exception(f"Could not parse {uri!r} as a Spotify URI ({e!s})") # noqa: TRY401 def _process_uri(web_client, uri): data = web_client.get(f"{uri['type']}s/{uri['id']}") - _cache[uri["key"]] = tuple( - web_to_image(i) for i in data.get("images") or [] - ) + _cache[uri["key"]] = tuple(web_to_image(i) for i in data.get("images") or []) return {uri["uri"]: _cache[uri["key"]]} -def _process_uris(web_client, uri_type, uris): +def _process_uris( # noqa: C901 + web_client, + uri_type, + uris, +): result = {} ids = [u["id"] for u in uris] ids_to_uris = {u["id"]: u for u in uris} @@ -113,8 +115,7 @@ def _process_uris(web_client, uri_type, uris): album_key = album["key"] if album_key not in _cache: _cache[album_key] = tuple( - web_to_image(i) - for i in item["album"].get("images") or [] + web_to_image(i) for i in item["album"].get("images") or [] ) _cache[uri["key"]] = _cache[album_key] else: diff --git a/mopidy_spotify/library.py b/src/mopidy_spotify/library.py similarity index 85% rename from mopidy_spotify/library.py rename to src/mopidy_spotify/library.py index 4c1f0d73..63d52593 100644 --- a/mopidy_spotify/library.py +++ b/src/mopidy_spotify/library.py @@ -37,11 +37,16 @@ def get_images(self, uris): def lookup(self, uri): return lookup.lookup(self._config, self._backend._web_client, uri) - def search(self, query=None, uris=None, exact=False): + def search( + self, + query=None, + uris=None, + exact=False, # noqa: FBT002 + ): return search.search( self._config, self._backend._web_client, - query, - uris, - exact, + query=query, + uris=uris, + exact=exact, ) diff --git a/mopidy_spotify/lookup.py b/src/mopidy_spotify/lookup.py similarity index 65% rename from mopidy_spotify/lookup.py rename to src/mopidy_spotify/lookup.py index f92c6e7e..d1c3ea2b 100644 --- a/mopidy_spotify/lookup.py +++ b/src/mopidy_spotify/lookup.py @@ -1,14 +1,18 @@ import logging from mopidy_spotify import browse, playlists, translator, utils -from mopidy_spotify.web import LinkType, WebLink, WebError +from mopidy_spotify.web import LinkType, WebError, WebLink logger = logging.getLogger(__name__) _VARIOUS_ARTISTS_URI = "spotify:artist:0LyfQWJT6nXafLPZqxe9Of" -def lookup(config, web_client, uri): +def lookup( # noqa: PLR0911 + config, + web_client, + uri, +): if web_client is None or not web_client.logged_in: return [] @@ -19,26 +23,23 @@ def lookup(config, web_client, uri): return [] try: - if link.type == LinkType.PLAYLIST: - return _lookup_playlist(config, web_client, link) - elif link.type == LinkType.YOUR: - return list(_lookup_your(config, web_client, link)) - elif link.type == LinkType.TRACK: - return list(_lookup_track(config, web_client, link)) - elif link.type == LinkType.ALBUM: - return list(_lookup_album(config, web_client, link)) - elif link.type == LinkType.ARTIST: - with utils.time_logger("Artist lookup"): - return list(_lookup_artist(config, web_client, link)) - else: - logger.info( - f"Failed to lookup {uri!r}: Cannot handle {link.type!r}" - ) - return [] + match link.type: + case LinkType.PLAYLIST: + return _lookup_playlist(config, web_client, link) + case LinkType.YOUR: + return list(_lookup_your(config, web_client, link)) + case LinkType.TRACK: + return list(_lookup_track(config, web_client, link)) + case LinkType.ALBUM: + return list(_lookup_album(config, web_client, link)) + case LinkType.ARTIST: + with utils.time_logger("Artist lookup"): + return list(_lookup_artist(config, web_client, link)) + case _: + logger.info(f"Failed to lookup {uri!r}: Cannot handle {link.type!r}") + return [] except WebError as exc: - logger.info( - f"Failed to lookup Spotify {link.type.value} {link.uri!r}: {exc}" - ) + logger.info(f"Failed to lookup Spotify {link.type.value} {link.uri!r}: {exc}") return [] @@ -58,9 +59,7 @@ def _lookup_album(config, web_client, link): if web_album == {}: raise WebError("Invalid album response") - yield from translator.web_to_album_tracks( - web_album, bitrate=config["bitrate"] - ) + yield from translator.web_to_album_tracks(web_album, bitrate=config["bitrate"]) def _lookup_artist(config, web_client, link): @@ -75,14 +74,14 @@ def _lookup_artist(config, web_client, link): break if is_various_artists: continue - yield from translator.web_to_album_tracks( - web_album, bitrate=config["bitrate"] - ) + yield from translator.web_to_album_tracks(web_album, bitrate=config["bitrate"]) def _lookup_playlist(config, web_client, link): playlist = playlists.playlist_lookup( - web_client, link.uri, config["bitrate"] + web_client, + link.uri, + bitrate=config["bitrate"], ) if playlist is None: raise WebError("Invalid playlist response") @@ -100,9 +99,7 @@ def _lookup_your(config, web_client, link): for item in items: # The extra level here is to also support "saved track objects". web_track = item.get("track", item) - track = translator.web_to_track( - web_track, bitrate=config["bitrate"] - ) + track = translator.web_to_track(web_track, bitrate=config["bitrate"]) if track is not None: yield track elif variant == "albums": diff --git a/mopidy_spotify/playlists.py b/src/mopidy_spotify/playlists.py similarity index 90% rename from mopidy_spotify/playlists.py rename to src/mopidy_spotify/playlists.py index 493d602c..d3d7f0fd 100644 --- a/mopidy_spotify/playlists.py +++ b/src/mopidy_spotify/playlists.py @@ -4,7 +4,6 @@ from mopidy_spotify import translator, utils - logger = logging.getLogger(__name__) @@ -38,12 +37,12 @@ def lookup(self, uri): with utils.time_logger(f"playlists.lookup({uri!r})", logging.DEBUG): return self._get_playlist(uri) - def _get_playlist(self, uri, as_items=False): + def _get_playlist(self, uri, *, as_items=False): return playlist_lookup( self._backend._web_client, uri, - self._backend._bitrate, - as_items, + bitrate=self._backend._bitrate, + as_items=as_items, ) def refresh(self): @@ -72,16 +71,22 @@ def save(self, playlist): pass # TODO -def playlist_lookup(web_client, uri, bitrate, as_items=False): +def playlist_lookup( + web_client, + uri, + *, + bitrate, + as_items=False, +): if web_client is None or not web_client.logged_in: - return + return None logger.debug(f'Fetching Spotify playlist "{uri!r}"') web_playlist = web_client.get_playlist(uri) if web_playlist == {}: logger.error(f"Failed to lookup Spotify playlist URI {uri!r}") - return + return None playlist = translator.to_playlist( web_playlist, @@ -91,6 +96,6 @@ def playlist_lookup(web_client, uri, bitrate, as_items=False): ) # TODO: cache the Mopidy tracks? And handle as_items here instead of translator if playlist is None: - return + return None return playlist diff --git a/mopidy_spotify/search.py b/src/mopidy_spotify/search.py similarity index 85% rename from mopidy_spotify/search.py rename to src/mopidy_spotify/search.py index e3d87510..53b0a6fb 100644 --- a/mopidy_spotify/search.py +++ b/src/mopidy_spotify/search.py @@ -10,11 +10,12 @@ logger = logging.getLogger(__name__) -def search( +def search( # noqa: PLR0913 config, web_client, + *, query=None, - uris=None, + uris=None, # noqa: ARG001 exact=False, types=_SEARCH_TYPES, ): @@ -27,7 +28,7 @@ def search( if "uri" in query: return _search_by_uri(config, web_client, query) - sp_query = translator.sp_search_query(query, exact) + sp_query = translator.sp_search_query(query, exact=exact) if not sp_query: logger.debug("Ignored search with empty query") return models.SearchResult(uri="spotify:search") @@ -45,7 +46,7 @@ def search( config["search_track_count"], ) - if search_count > 50: + if search_count > 50: # noqa: PLR2004 logger.warning( "Spotify currently allows maximum 50 search results of each type. " "Please set the config values spotify/search_album_count, " @@ -67,9 +68,7 @@ def search( albums = ( [ translator.web_to_album(web_album) - for web_album in result["albums"]["items"][ - : config["search_album_count"] - ] + for web_album in result["albums"]["items"][: config["search_album_count"]] ] if "albums" in result else [] @@ -91,18 +90,14 @@ def search( tracks = ( [ translator.web_to_track(web_track) - for web_track in result["tracks"]["items"][ - : config["search_track_count"] - ] + for web_track in result["tracks"]["items"][: config["search_track_count"]] ] if "tracks" in result else [] ) tracks = [x for x in tracks if x] - return models.SearchResult( - uri=uri, albums=albums, artists=artists, tracks=tracks - ) + return models.SearchResult(uri=uri, albums=albums, artists=artists, tracks=tracks) def _search_by_uri(config, web_client, query): diff --git a/mopidy_spotify/translator.py b/src/mopidy_spotify/translator.py similarity index 79% rename from mopidy_spotify/translator.py rename to src/mopidy_spotify/translator.py index 8cfe49a5..044200ce 100644 --- a/mopidy_spotify/translator.py +++ b/src/mopidy_spotify/translator.py @@ -1,27 +1,26 @@ -import collections import logging +from collections.abc import Hashable from mopidy import models logger = logging.getLogger(__name__) -class memoized: # noqa N801 +class memoized: # noqa: N801 def __init__(self, func): self.func = func self.cache = {} def __call__(self, *args, **kwargs): # NOTE Only args, not kwargs, are part of the memoization key. - if not isinstance(args, collections.abc.Hashable): + if not isinstance(args, Hashable): return self.func(*args, **kwargs) if args in self.cache: return self.cache[args] - else: - value = self.func(*args, **kwargs) - if value is not None: - self.cache[args] = value - return value + value = self.func(*args, **kwargs) + if value is not None: + self.cache[args] = value + return value # TODO: memoize web functions? @@ -29,7 +28,7 @@ def __call__(self, *args, **kwargs): def web_to_artist_ref(web_artist): if not valid_web_data(web_artist, "artist"): - return + return None uri = web_artist["uri"] return models.Ref.artist(uri=uri, name=web_artist.get("name", uri)) @@ -44,7 +43,7 @@ def web_to_artist_refs(web_artists): def web_to_album_ref(web_album): if not valid_web_data(web_album, "album"): - return + return None if "name" in web_album: artists = web_album.get("artists", []) @@ -60,9 +59,10 @@ def web_to_album_ref(web_album): def web_to_album_refs(web_albums): for web_album in web_albums: - # The extra level here is to also support "saved album objects". - web_album = web_album.get("album", web_album) - ref = web_to_album_ref(web_album) + ref = web_to_album_ref( + # The extra level here is to also support "saved album objects". + web_album.get("album", web_album), + ) if ref is not None: yield ref @@ -77,7 +77,7 @@ def valid_web_data(data, object_type): def web_to_track_ref(web_track, *, check_playable=True): if not valid_web_data(web_track, "track"): - return + return None # Web API track relinking guide says to use original URI. # libspotfy will handle any relinking when track is loaded for playback. @@ -85,22 +85,25 @@ def web_to_track_ref(web_track, *, check_playable=True): if check_playable and not web_track.get("is_playable", False): logger.debug(f"{uri!r} is not playable") - return + return None return models.Ref.track(uri=uri, name=web_track.get("name", uri)) def web_to_track_refs(web_tracks, *, check_playable=True): for web_track in web_tracks: - # The extra level here is to also support "saved track objects". - web_track = web_track.get("track", web_track) - ref = web_to_track_ref(web_track, check_playable=check_playable) + ref = web_to_track_ref( + # The extra level here is to also support "saved track objects". + web_track.get("track", web_track), + check_playable=check_playable, + ) if ref is not None: yield ref def to_playlist( web_playlist, + *, username=None, bitrate=None, as_ref=False, @@ -112,7 +115,7 @@ def to_playlist( web_tracks = web_playlist.get("tracks", {}).get("items") or [] if as_items and not isinstance(web_tracks, list): - return + return None if as_items: return list(web_to_track_refs(web_tracks)) @@ -128,7 +131,7 @@ def to_playlist( def to_playlist_ref(web_playlist, username=None): if not valid_web_data(web_playlist, "playlist"): - return + return None name = web_playlist.get("name", web_playlist["uri"]) @@ -156,19 +159,19 @@ def to_playlist_refs(web_playlists, username=None): } -def sp_search_query(query, exact=False): +def sp_search_query(query, *, exact=False): """Translate a Mopidy search query to a Spotify search query""" result = [] for field, values in query.items(): - field = SEARCH_FIELD_MAP.get(field, field) + field = SEARCH_FIELD_MAP.get(field, field) # noqa: PLW2901 if field is None: continue for value in values: if field == "year": - value = _transform_year(value) + value = _transform_year(value) # noqa: PLW2901 if value is not None: result.append(f"{field}:{value}") elif field == "any": @@ -176,13 +179,10 @@ def sp_search_query(query, exact=False): result.append(f'"{value}"') else: result.append(value) + elif exact: + result.append(f'{field}:"{value}"') else: - if exact: - result.append(f'{field}:"{value}"') - else: - result.append( - " ".join(f"{field}:{word}" for word in value.split()) - ) + result.append(" ".join(f"{field}:{word}" for word in value.split())) return " ".join(result) @@ -191,15 +191,13 @@ def _transform_year(date): try: return int(date.split("-")[0]) except ValueError: - logger.debug( - f'Excluded year from search query: Cannot parse date "{date}"' - ) + logger.debug(f'Excluded year from search query: Cannot parse date "{date}"') def web_to_artist(web_artist): ref = web_to_artist_ref(web_artist) if ref is None: - return + return None return models.Artist(uri=ref.uri, name=ref.name) @@ -216,20 +214,16 @@ def web_to_album_tracks(web_album, bitrate=None): if not isinstance(web_tracks, list): return [] - tracks = [ - web_to_track(web_track, bitrate, album) for web_track in web_tracks - ] + tracks = [web_to_track(web_track, bitrate, album) for web_track in web_tracks] return [t for t in tracks if t] def web_to_album(web_album): ref = web_to_album_ref(web_album) if ref is None: - return + return None - artists = [ - web_to_artist(web_artist) for web_artist in web_album.get("artists", []) - ] + artists = [web_to_artist(web_artist) for web_artist in web_album.get("artists", [])] artists = [a for a in artists if a] name = web_album.get("name", "Unknown album") @@ -239,16 +233,15 @@ def web_to_album(web_album): def int_or_none(inp): if inp is not None: return int(float(inp)) + return None def web_to_track(web_track, bitrate=None, album=None): ref = web_to_track_ref(web_track) if ref is None: - return + return None - artists = [ - web_to_artist(web_artist) for web_artist in web_track.get("artists", []) - ] + artists = [web_to_artist(web_artist) for web_artist in web_track.get("artists", [])] artists = [a for a in artists if a] if album is None: diff --git a/mopidy_spotify/utils.py b/src/mopidy_spotify/utils.py similarity index 100% rename from mopidy_spotify/utils.py rename to src/mopidy_spotify/utils.py diff --git a/mopidy_spotify/web.py b/src/mopidy_spotify/web.py similarity index 83% rename from mopidy_spotify/web.py rename to src/mopidy_spotify/web.py index 23da1d0d..51dd11fd 100644 --- a/mopidy_spotify/web.py +++ b/src/mopidy_spotify/web.py @@ -5,10 +5,10 @@ import time import urllib.parse from dataclasses import dataclass -from datetime import datetime -from enum import Enum, unique -from typing import Optional +from datetime import UTC, datetime from email.utils import parsedate_to_datetime +from enum import Enum, unique +from http import HTTPStatus import requests @@ -32,8 +32,9 @@ class OAuthClientError(Exception): class OAuthClient: - def __init__( + def __init__( # noqa: PLR0913 self, + *, base_url, refresh_url, client_id=None, @@ -77,7 +78,7 @@ def get(self, path, cache=None, *args, **kwargs): ignore_expiry = kwargs.pop("ignore_expiry", False) if cache is not None and path in cache: cached_result = cache.get(path) - if cached_result.still_valid(ignore_expiry): + if cached_result.still_valid(ignore_expiry=ignore_expiry): return cached_result kwargs.setdefault("headers", {}).update(cached_result.etag_headers) @@ -87,7 +88,7 @@ def get(self, path, cache=None, *args, **kwargs): if self._should_refresh_token(): self._refresh_token() except OAuthTokenRefreshError as e: - logger.error(e) + logger.error(e) # noqa: TRY400 return WebResponse(None, None) # Make sure our headers always override user supplied ones. @@ -126,13 +127,13 @@ def _refresh_token(self): if result is None: raise OAuthTokenRefreshError("Unknown error.") - elif result.get("error"): + if result.get("error"): raise OAuthTokenRefreshError( f"{result['error']} {result.get('error_description', '')}" ) - elif not result.get("access_token"): + if not result.get("access_token"): raise OAuthTokenRefreshError("missing access_token") - elif result.get("token_type") != "Bearer": + if result.get("token_type") != "Bearer": raise OAuthTokenRefreshError( f"wrong token_type: {result.get('token_type')}" ) @@ -154,6 +155,7 @@ def _request_with_retries(self, method, url, *args, **kwargs): try_until = time.time() + self._timeout + status_code = None result = None backoff_time = 0 @@ -163,7 +165,7 @@ def _request_with_retries(self, method, url, *args, **kwargs): # Give up if we don't have any timeout left after sleeping. if backoff_time > remaining_timeout: break - elif backoff_time > 0: + if backoff_time > 0: time.sleep(backoff_time) try: @@ -180,10 +182,8 @@ def _request_with_retries(self, method, url, *args, **kwargs): backoff_time = self._parse_retry_after(response) result = WebResponse.from_requests(prepared_request, response) - if status_code and 400 <= status_code < 600: - logger.debug( - f"Fetching {prepared_request.url} failed: {status_code}" - ) + if status_code and 400 <= status_code < 600: # noqa: PLR2004 + logger.debug(f"Fetching {prepared_request.url} failed: {status_code}") # Filter out cases where we should not retry. if status_code and status_code not in self._retry_statuses: @@ -197,11 +197,10 @@ def _request_with_retries(self, method, url, *args, **kwargs): # Decide how long to sleep in the next iteration. backoff_time = backoff_time or (2**i * self._backoff_factor) logger.error( - f"Retrying {prepared_request.url} in {backoff_time:.3f} " - "seconds." + f"Retrying {prepared_request.url} in {backoff_time:.3f} " "seconds." ) - if status_code == 401: + if status_code == HTTPStatus.UNAUTHORIZED: self._authorization_failed = True logger.error( "Authorization failed, not attempting Spotify API " @@ -221,19 +220,15 @@ def _prepare_url(self, url, *args, **kwargs): query = urllib.parse.parse_qsl(u.query, keep_blank_values=True) else: scheme, netloc = b.scheme, b.netloc - path = os.path.normpath(os.path.join(b.path, u.path)) + path = os.path.normpath(os.path.join(b.path, u.path)) # noqa: PTH118 query = urllib.parse.parse_qsl(b.query, keep_blank_values=True) - query.extend( - urllib.parse.parse_qsl(u.query, keep_blank_values=True) - ) + query.extend(urllib.parse.parse_qsl(u.query, keep_blank_values=True)) for key, value in kwargs.items(): query.append((key, value)) encoded_query = urllib.parse.urlencode(dict(query)) - return urllib.parse.urlunsplit( - (scheme, netloc, path, encoded_query, "") - ) + return urllib.parse.urlunsplit((scheme, netloc, path, encoded_query, "")) def _normalise_query_string(self, url, params=None): u = urllib.parse.urlsplit(url) @@ -244,9 +239,7 @@ def _normalise_query_string(self, url, params=None): query.update(params) sorted_unique_query = sorted(query.items()) encoded_query = urllib.parse.urlencode(sorted_unique_query) - return urllib.parse.urlunsplit( - (scheme, netloc, path, encoded_query, "") - ) + return urllib.parse.urlunsplit((scheme, netloc, path, encoded_query, "")) def _parse_retry_after(self, response): """Parse Retry-After header from response if it is set.""" @@ -257,7 +250,7 @@ def _parse_retry_after(self, response): elif re.match(r"^\s*[0-9]+\s*$", value): seconds = int(value) else: - now = datetime.utcnow() + now = datetime.now(tz=UTC).replace(tzinfo=None) try: date_tuple = parsedate_to_datetime(value) seconds = (date_tuple - now).total_seconds() @@ -267,7 +260,15 @@ def _parse_retry_after(self, response): class WebResponse(dict): - def __init__(self, url, data, expires=0.0, etag=None, status_code=400): + def __init__( # noqa: PLR0913 + self, + url, + data, + *, + expires=0.0, + etag=None, + status_code=400, + ): self._from_cache = False self.url = url self._expires = expires @@ -281,7 +282,13 @@ def from_requests(cls, request, response): expires = cls._parse_cache_control(response) etag = cls._parse_etag(response) json = cls._decode(response) - return cls(request.url, json, expires, etag, response.status_code) + return cls( + request.url, + json, + expires=expires, + etag=etag, + status_code=response.status_code, + ) @staticmethod def _decode(response): @@ -292,7 +299,7 @@ def _decode(response): return response.json() except ValueError as e: url = response.request.url - logger.error(f"JSON decoding {url} failed: {e}") + logger.error(f"JSON decoding {url} failed: {e}") # noqa: TRY400 return None @staticmethod @@ -304,10 +311,7 @@ def _parse_cache_control(response): seconds = 0 else: max_age = re.match(r".*max-age=\s*([0-9]+)\s*", value) - if not max_age: - seconds = 0 - else: - seconds = int(max_age.groups()[0]) + seconds = 0 if not max_age else int(max_age.groups()[0]) return time.time() + seconds @staticmethod @@ -321,10 +325,12 @@ def _parse_etag(response): # Format is string of ASCII characters placed between double quotes # but can seemingly also include hyphen characters. etag = re.match(r'^(W/)?("[!#-~]+")$', value) - if etag and len(etag.groups()) == 2: + if etag and len(etag.groups()) == 2: # noqa: PLR2004 return etag.groups()[1] - def still_valid(self, ignore_expiry=False): + return None + + def still_valid(self, *, ignore_expiry=False): if ignore_expiry: result = True status = "forced" @@ -340,30 +346,29 @@ def still_valid(self, ignore_expiry=False): @property def status_unchanged(self): - return self._from_cache or 304 == self._status_code + return self._from_cache or self._status_code == HTTPStatus.NOT_MODIFIED @property def status_ok(self): - return self._status_code >= 200 and self._status_code < 400 + return self._status_code >= 200 and self._status_code < 400 # noqa: PLR2004 @property def etag_headers(self): - if self._etag is not None: - return {"If-None-Match": self._etag} - else: + if self._etag is None: return {} + return {"If-None-Match": self._etag} def updated(self, response): self._from_cache = False if self._etag is None: return False - elif self.url != response.url: + if self.url != response.url: logger.error(f"ETag mismatch (different URI) for {self} {response}") return False - elif not response.status_ok: + if not response.status_ok: logger.debug(f"ETag mismatch (bad response) for {self} {response}") return False - elif response._status_code != 304: + if response._status_code != HTTPStatus.NOT_MODIFIED: _trace(f"ETag mismatch for {self} {response}") return False @@ -376,7 +381,7 @@ def updated(self, response): def __str__(self): return ( f"URL: {self.url} " - f"expires at: {datetime.fromtimestamp(self._expires)} " + f"expires at: {datetime.fromtimestamp(self._expires, tz=UTC)} " f"[ETag: {self._etag}]" ) @@ -390,9 +395,7 @@ class SpotifyOAuthClient(OAuthClient): "next,items(track(type,uri,name,duration_ms,disc_number,track_number," "artists,album,is_playable,linked_from.uri))" ) - PLAYLIST_FIELDS = ( - f"name,owner(id),type,uri,snapshot_id,tracks({TRACK_FIELDS})," - ) + PLAYLIST_FIELDS = f"name,owner(id),type,uri,snapshot_id,tracks({TRACK_FIELDS})," DEFAULT_EXTRA_EXPIRY = 10 def __init__(self, *, client_id, client_secret, proxy_config): @@ -424,18 +427,15 @@ def login(self): if self.user_id is None: logger.error("Failed to load Spotify user profile") return False - else: - logger.info(f"Logged into Spotify Web API as {self.user_id}") - return True + logger.info(f"Logged into Spotify Web API as {self.user_id}") + return True @property def logged_in(self): return self.user_id is not None def get_user_playlists(self): - pages = self.get_all( - f"users/{self.user_id}/playlists", params={"limit": 50} - ) + pages = self.get_all(f"users/{self.user_id}/playlists", params={"limit": 50}) for page in pages: yield from page.get("items", []) @@ -467,11 +467,9 @@ def get_playlist(self, uri): try: parsed = WebLink.from_uri(uri) if parsed.type != LinkType.PLAYLIST: - raise ValueError( - f"Could not parse {uri!r} as a Spotify playlist URI" - ) + raise ValueError(f"Could not parse {uri!r} as a Spotify playlist URI") # noqa: TRY301 except ValueError as exc: - logger.error(exc) + logger.error(exc) # noqa: TRY400 return {} playlist = self.get_one( @@ -491,7 +489,7 @@ def get_album(self, web_link): ) return self._with_all_tracks(album) - def get_artist_albums(self, web_link, all_tracks=True): + def get_artist_albums(self, web_link, *, all_tracks=True): if web_link.type != LinkType.ARTIST: logger.error("Expecting Spotify artist URI") return [] @@ -506,7 +504,7 @@ def get_artist_albums(self, web_link, all_tracks=True): try: web_link = WebLink.from_uri(album.get("uri")) except ValueError as exc: - logger.error(exc) + logger.error(exc) # noqa: TRY400 continue yield self.get_album(web_link) else: @@ -527,11 +525,12 @@ def get_track(self, web_link): logger.error("Expecting Spotify track URI") return {} - return self.get_one( - f"tracks/{web_link.id}", params={"market": "from_token"} - ) + return self.get_one(f"tracks/{web_link.id}", params={"market": "from_token"}) - def clear_cache(self, extra_expiry=None): + def clear_cache( + self, + extra_expiry=None, # noqa: ARG002 + ): self._cache.clear() @@ -548,8 +547,8 @@ class LinkType(Enum): class WebLink: uri: str type: LinkType - id: Optional[str] = None - owner: Optional[str] = None + id: str | None = None + owner: str | None = None @classmethod def from_uri(cls, uri): @@ -568,22 +567,18 @@ def from_uri(cls, uri): # Strip out empty parts to ensure we are strict about URI parsing. parts = [p for p in parts if p.strip()] - if len(parts) == 2 and parts[0] in ( - "track", - "album", - "artist", - "playlist", - ): - return cls(uri, LinkType(parts[0]), parts[1], None) - elif len(parts) == 2 and parts[0] == "your": - return cls(uri, LinkType(parts[0])) - elif len(parts) == 3 and parts[0] == "user" and parts[2] == "starred": - if parsed_uri.scheme == "spotify": - return cls(uri, LinkType.PLAYLIST, None, parts[1]) - elif len(parts) == 3 and parts[0] == "playlist": - return cls(uri, LinkType.PLAYLIST, parts[2], parts[1]) - elif len(parts) == 4 and parts[0] == "user" and parts[2] == "playlist": - return cls(uri, LinkType.PLAYLIST, parts[3], parts[1]) + match parts: + case [type, id] if type in ("track", "album", "artist", "playlist"): + return cls(uri, LinkType(type), id, None) + case ["your", _]: + return cls(uri, LinkType.YOUR) + case ["user", owner, "starred"]: + if parsed_uri.scheme == "spotify": + return cls(uri, LinkType.PLAYLIST, None, owner) + case ["playlist", owner, id]: + return cls(uri, LinkType.PLAYLIST, id, owner) + case ["user", owner, "playlist", id]: + return cls(uri, LinkType.PLAYLIST, id, owner) raise ValueError(f"Could not parse {uri!r} as a Spotify URI") diff --git a/tests/conftest.py b/tests/conftest.py index 5b146daa..83557903 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -3,17 +3,16 @@ import pytest from mopidy import backend as backend_api from mopidy import models - from mopidy_spotify import backend, library, utils, web -@pytest.fixture +@pytest.fixture() def caplog(caplog): caplog.set_level(utils.TRACE) return caplog -@pytest.fixture +@pytest.fixture() def config(tmp_path): return { "core": { @@ -39,14 +38,14 @@ def config(tmp_path): } -@pytest.fixture +@pytest.fixture() def web_mock(): patcher = mock.patch.object(backend, "web", spec=web) yield patcher.start() patcher.stop() -@pytest.fixture +@pytest.fixture() def web_search_mock(web_album_mock_base, web_artist_mock, web_track_mock): return { "albums": {"items": [web_album_mock_base]}, @@ -55,7 +54,7 @@ def web_search_mock(web_album_mock_base, web_artist_mock, web_track_mock): } -@pytest.fixture +@pytest.fixture() def web_search_mock_large(web_album_mock, web_artist_mock, web_track_mock): return { "albums": {"items": [web_album_mock] * 10}, @@ -64,12 +63,12 @@ def web_search_mock_large(web_album_mock, web_artist_mock, web_track_mock): } -@pytest.fixture +@pytest.fixture() def web_artist_mock(): return {"name": "ABBA", "uri": "spotify:artist:abba", "type": "artist"} -@pytest.fixture +@pytest.fixture() def web_track_mock_base(web_artist_mock): return { "artists": [web_artist_mock], @@ -83,7 +82,7 @@ def web_track_mock_base(web_artist_mock): } -@pytest.fixture +@pytest.fixture() def web_album_mock_base(web_artist_mock): return { "name": "DEF 456", @@ -94,16 +93,16 @@ def web_album_mock_base(web_artist_mock): } -@pytest.fixture +@pytest.fixture() def web_album_mock(web_album_mock_base, web_track_mock_base): return { **web_album_mock_base, - **{"tracks": {"items": [web_track_mock_base] * 10}}, + "tracks": {"items": [web_track_mock_base] * 10}, "is_playable": True, } -@pytest.fixture +@pytest.fixture() def web_album_mock_base2(web_artist_mock): return { "name": "XYZ 789", @@ -114,24 +113,24 @@ def web_album_mock_base2(web_artist_mock): } -@pytest.fixture +@pytest.fixture() def web_album_mock2(web_album_mock_base2, web_track_mock_base): return { **web_album_mock_base2, - **{"tracks": {"items": [web_track_mock_base] * 2}}, + "tracks": {"items": [web_track_mock_base] * 2}, "is_playable": True, } -@pytest.fixture +@pytest.fixture() def web_track_mock(web_track_mock_base, web_album_mock_base): return { **web_track_mock_base, - **{"album": web_album_mock_base}, + "album": web_album_mock_base, } -@pytest.fixture +@pytest.fixture() def web_response_mock(web_track_mock): return web.WebResponse( "https://api.spotify.com/v1/tracks/abc", @@ -141,13 +140,13 @@ def web_response_mock(web_track_mock): ) -@pytest.fixture +@pytest.fixture() def web_response_mock_etag(web_response_mock): web_response_mock._etag = '"1234"' return web_response_mock -@pytest.fixture +@pytest.fixture() def web_oauth_mock(): return { "access_token": "NgCXRK...MzYjw", @@ -157,7 +156,7 @@ def web_oauth_mock(): } -@pytest.fixture +@pytest.fixture() def web_playlist_mock(web_track_mock): return { "owner": {"id": "alice"}, @@ -169,12 +168,12 @@ def web_playlist_mock(web_track_mock): } -@pytest.fixture +@pytest.fixture() def mopidy_artist_mock(): return models.Artist(name="ABBA", uri="spotify:artist:abba") -@pytest.fixture +@pytest.fixture() def mopidy_album_mock(mopidy_artist_mock): return models.Album( artists=[mopidy_artist_mock], @@ -184,7 +183,7 @@ def mopidy_album_mock(mopidy_artist_mock): ) -@pytest.fixture +@pytest.fixture() def web_client_mock(): web_client_mock = mock.MagicMock(spec=web.SpotifyOAuthClient) web_client_mock.user_id = "alice" @@ -192,7 +191,7 @@ def web_client_mock(): return web_client_mock -@pytest.fixture +@pytest.fixture() def backend_mock(config, web_client_mock): backend_mock = mock.Mock(spec=backend.SpotifyBackend) backend_mock._config = config @@ -201,7 +200,7 @@ def backend_mock(config, web_client_mock): return backend_mock -@pytest.fixture +@pytest.fixture() def backend_listener_mock(): patcher = mock.patch.object( backend_api, "BackendListener", spec=backend_api.BackendListener @@ -210,6 +209,6 @@ def backend_listener_mock(): patcher.stop() -@pytest.fixture +@pytest.fixture() def provider(backend_mock): return library.SpotifyLibraryProvider(backend_mock) diff --git a/tests/test_backend.py b/tests/test_backend.py index 7a1dd38e..9756880f 100644 --- a/tests/test_backend.py +++ b/tests/test_backend.py @@ -1,8 +1,6 @@ -from unittest import mock -from unittest import skip +from unittest import mock, skip from mopidy import backend as backend_api - from mopidy_spotify import backend, library, playlists from mopidy_spotify.backend import SpotifyPlaybackProvider @@ -102,9 +100,7 @@ def test_on_start_refreshes_playlists(web_mock, config, caplog): assert backend.playlists._loaded -def test_on_start_doesnt_refresh_playlists_if_not_allowed( - web_mock, config, caplog -): +def test_on_start_doesnt_refresh_playlists_if_not_allowed(web_mock, config, caplog): config["spotify"]["allow_playlists"] = False backend = get_backend(config) diff --git a/tests/test_browse.py b/tests/test_browse.py index f45bb813..163b378b 100644 --- a/tests/test_browse.py +++ b/tests/test_browse.py @@ -2,7 +2,6 @@ import pytest from mopidy import models - from mopidy_spotify.browse import BROWSE_DIR_URIS @@ -17,13 +16,8 @@ def test_browse_root_directory(provider): assert len(results) == 3 assert models.Ref.directory(uri="spotify:top", name="Top lists") in results - assert ( - models.Ref.directory(uri="spotify:your", name="Your music") in results - ) - assert ( - models.Ref.directory(uri="spotify:playlists", name="Playlists") - in results - ) + assert models.Ref.directory(uri="spotify:your", name="Your music") in results + assert models.Ref.directory(uri="spotify:playlists", name="Playlists") in results def test_browse_dir_uris(provider): @@ -51,13 +45,9 @@ def test_browse_top_lists_directory(provider): results = provider.browse("spotify:top") assert len(results) == 2 + assert models.Ref.directory(uri="spotify:top:tracks", name="Top tracks") in results assert ( - models.Ref.directory(uri="spotify:top:tracks", name="Top tracks") - in results - ) - assert ( - models.Ref.directory(uri="spotify:top:artists", name="Top artists") - in results + models.Ref.directory(uri="spotify:top:artists", name="Top artists") in results ) @@ -66,12 +56,10 @@ def test_browse_your_music_directory(provider): assert len(results) == 2 assert ( - models.Ref.directory(uri="spotify:your:tracks", name="Your tracks") - in results + models.Ref.directory(uri="spotify:your:tracks", name="Your tracks") in results ) assert ( - models.Ref.directory(uri="spotify:your:albums", name="Your albums") - in results + models.Ref.directory(uri="spotify:your:albums", name="Your albums") in results ) @@ -94,9 +82,7 @@ def test_browse_playlist(web_client_mock, web_playlist_mock, provider): "spotify:user:alice:playlist:foo" ) assert len(results) == 1 - assert results[0] == models.Ref.track( - uri="spotify:track:abc", name="ABC 123" - ) + assert results[0] == models.Ref.track(uri="spotify:track:abc", name="ABC 123") @pytest.mark.parametrize( @@ -125,14 +111,10 @@ def test_browse_album(web_client_mock, web_album_mock, provider): results = provider.browse("spotify:album:def") assert len(results) == 10 - assert results[0] == models.Ref.track( - uri="spotify:track:abc", name="ABC 123" - ) + assert results[0] == models.Ref.track(uri="spotify:track:abc", name="ABC 123") -def test_browse_album_bad_uri( - web_client_mock, web_album_mock, provider, caplog -): +def test_browse_album_bad_uri(web_client_mock, web_album_mock, provider, caplog): web_client_mock.get_album.return_value = web_album_mock results = provider.browse("spotify:album:def:xyz") @@ -160,9 +142,7 @@ def test_browse_artist( mock.ANY, all_tracks=False ) assert len(results) == 4 - assert results[0] == models.Ref.track( - uri="spotify:track:abc", name="ABC 123" - ) + assert results[0] == models.Ref.track(uri="spotify:track:abc", name="ABC 123") assert results[3] == models.Ref.album( uri="spotify:album:def", name="ABBA - DEF 456" ) @@ -226,14 +206,10 @@ def test_browse_personal_top_tracks(web_client_mock, web_track_mock, provider): "me/top/tracks", params={"limit": 50} ) assert len(results) == 4 - assert results[0] == models.Ref.track( - uri="spotify:track:abc", name="ABC 123" - ) + assert results[0] == models.Ref.track(uri="spotify:track:abc", name="ABC 123") -def test_browse_personal_top_artists( - web_client_mock, web_artist_mock, provider -): +def test_browse_personal_top_artists(web_client_mock, web_artist_mock, provider): web_client_mock.get_all.return_value = [ {"items": [web_artist_mock, web_artist_mock]}, {"items": [web_artist_mock, web_artist_mock]}, @@ -245,9 +221,7 @@ def test_browse_personal_top_artists( "me/top/artists", params={"limit": 50} ) assert len(results) == 4 - assert results[0] == models.Ref.artist( - uri="spotify:artist:abba", name="ABBA" - ) + assert results[0] == models.Ref.artist(uri="spotify:artist:abba", name="ABBA") def test_browse_your_music_when_offline_web(web_client_mock, provider): @@ -269,8 +243,7 @@ def test_browse_your_music_tracks_unknown(provider, caplog): assert len(results) == 0 assert ( - "Failed to browse 'spotify:your:tracks:foobar': Unknown URI type" - in caplog.text + "Failed to browse 'spotify:your:tracks:foobar': Unknown URI type" in caplog.text ) @@ -295,9 +268,7 @@ def test_browse_your_music_tracks(web_client_mock, web_track_mock, provider): "me/tracks", params={"market": "from_token", "limit": 50} ) assert results == [results[0]] * 4 - assert results[0] == models.Ref.track( - uri="spotify:track:abc", name="ABC 123" - ) + assert results[0] == models.Ref.track(uri="spotify:track:abc", name="ABC 123") def test_browse_your_music_albums(web_client_mock, web_album_mock, provider): @@ -318,9 +289,7 @@ def test_browse_your_music_albums(web_client_mock, web_album_mock, provider): ) -def test_browse_playlists_featured( - web_client_mock, web_playlist_mock, provider -): +def test_browse_playlists_featured(web_client_mock, web_playlist_mock, provider): web_client_mock.get_all.return_value = [ {"playlists": {"items": [web_playlist_mock]}}, {"playlists": {"items": [web_playlist_mock]}}, diff --git a/tests/test_distinct.py b/tests/test_distinct.py index 35f353f4..4501e0e4 100644 --- a/tests/test_distinct.py +++ b/tests/test_distinct.py @@ -2,11 +2,10 @@ import pytest from mopidy import models +from mopidy_spotify import distinct, playlists, search -from mopidy_spotify import distinct, search, playlists - -@pytest.fixture +@pytest.fixture() def web_client_mock_with_playlists( web_client_mock, web_playlist_mock, @@ -20,7 +19,7 @@ def web_client_mock_with_playlists( return web_client_mock -@pytest.fixture +@pytest.fixture() def search_mock(mopidy_album_mock, mopidy_artist_mock): patcher = mock.patch.object(distinct, "search", spec=search) search_mock = patcher.start() @@ -47,14 +46,13 @@ def test_get_distinct_unsupported_field_types_returns_nothing(provider, field): @pytest.mark.parametrize( - "field,expected", + ("field", "expected"), [ ("artist", {"ABBA"}), ("albumartist", {"ABBA"}), ("album", {"DEF 456"}), ], ) -# ("date", {"2001"}), def test_get_distinct_without_query_when_playlists_enabled( web_client_mock_with_playlists, provider, field, expected ): @@ -88,7 +86,7 @@ def test_get_distinct_without_query_returns_nothing_when_playlists_disabled( @pytest.mark.parametrize( - "field,query,expected,types", + ("field", "query", "expected", "types"), [ ( "artist", @@ -128,5 +126,8 @@ def test_get_distinct_with_query( ): assert provider.get_distinct(field, query) == expected search_mock.search.assert_called_once_with( - mock.ANY, mock.ANY, query, types=types + mock.ANY, + mock.ANY, + query=query, + types=types, ) diff --git a/tests/test_images.py b/tests/test_images.py index d4fd9744..04c7bec9 100644 --- a/tests/test_images.py +++ b/tests/test_images.py @@ -1,10 +1,9 @@ import pytest from mopidy import models - from mopidy_spotify import images -@pytest.fixture +@pytest.fixture() def img_provider(provider): images._cache = {} return provider @@ -102,9 +101,7 @@ def test_get_track_images(web_client_mock, img_provider): "id": "41shEpOKyyadtG6lDclooa", "album": { "uri": "spotify:album:1utFPuvgBHXzLJdqhCDOkg", - "images": [ - {"height": 640, "url": "img://1/a", "width": 640} - ], + "images": [{"height": 640, "url": "img://1/a", "width": 640}], }, } ] @@ -136,9 +133,7 @@ def test_get_track_images_bad_album_uri(web_client_mock, img_provider): "id": "41shEpOKyyadtG6lDclooa", "album": { "uri": "spotify:bad-data", - "images": [ - {"height": 640, "url": "img://1/a", "width": 640} - ], + "images": [{"height": 640, "url": "img://1/a", "width": 640}], }, } ] @@ -162,9 +157,7 @@ def test_get_relinked_track_images(web_client_mock, img_provider): }, "album": { "uri": "spotify:album:1utFPuvgBHXzLJdqhCDOkg", - "images": [ - {"height": 640, "url": "img://1/a", "width": 640} - ], + "images": [{"height": 640, "url": "img://1/a", "width": 640}], }, } ] @@ -197,9 +190,7 @@ def test_get_playlist_image(web_client_mock, img_provider): result = img_provider.get_images(uris) - web_client_mock.get.assert_called_once_with( - "playlists/41shEpOKyyadtG6lDclooa" - ) + web_client_mock.get.assert_called_once_with("playlists/41shEpOKyyadtG6lDclooa") assert len(result) == 1 assert sorted(result.keys()) == ["spotify:playlist:41shEpOKyyadtG6lDclooa"] @@ -221,9 +212,7 @@ def test_results_are_cached(web_client_mock, img_provider): "id": "41shEpOKyyadtG6lDclooa", "album": { "uri": "spotify:album:1utFPuvgBHXzLJdqhCDOkg", - "images": [ - {"height": 640, "url": "img://1/a", "width": 640} - ], + "images": [{"height": 640, "url": "img://1/a", "width": 640}], }, } ] @@ -322,8 +311,6 @@ def test_service_returns_none_result(web_client_mock, img_provider): def test_service_returns_none_result_playlist(web_client_mock, img_provider): web_client_mock.get.return_value = {"images": None} - result = img_provider.get_images( - ["spotify:playlist:41shEpOKyyadtG6lDclooa"] - ) + result = img_provider.get_images(["spotify:playlist:41shEpOKyyadtG6lDclooa"]) assert result == {"spotify:playlist:41shEpOKyyadtG6lDclooa": ()} diff --git a/tests/test_lookup.py b/tests/test_lookup.py index da76edfb..0264bb62 100644 --- a/tests/test_lookup.py +++ b/tests/test_lookup.py @@ -33,8 +33,7 @@ def test_lookup_of_unhandled_uri(provider, caplog): assert len(results) == 0 assert ( "Failed to lookup 'spotify:invalid:something': " - "Could not parse 'spotify:invalid:something' as a Spotify URI" - in caplog.text + "Could not parse 'spotify:invalid:something' as a Spotify URI" in caplog.text ) @@ -140,9 +139,7 @@ def test_lookup_of_artist_uri_ignores_compilations( def test_lookup_of_artist_uri_ignores_various_artists_albums( web_client_mock, web_album_mock, provider ): - web_album_mock["artists"][0][ - "uri" - ] = "spotify:artist:0LyfQWJT6nXafLPZqxe9Of" + web_album_mock["artists"][0]["uri"] = "spotify:artist:0LyfQWJT6nXafLPZqxe9Of" web_client_mock.get_artist_albums.return_value = [web_album_mock] results = provider.lookup("spotify:artist:abba") @@ -155,9 +152,7 @@ def test_lookup_of_playlist_uri(web_client_mock, web_playlist_mock, provider): results = provider.lookup("spotify:playlist:alice:foo") - web_client_mock.get_playlist.assert_called_once_with( - "spotify:playlist:alice:foo" - ) + web_client_mock.get_playlist.assert_called_once_with("spotify:playlist:alice:foo") assert len(results) == 1 track = results[0] @@ -166,9 +161,7 @@ def test_lookup_of_playlist_uri(web_client_mock, web_playlist_mock, provider): assert track.bitrate == 160 -def test_lookup_of_playlist_uri_empty_response( - web_client_mock, provider, caplog -): +def test_lookup_of_playlist_uri_empty_response(web_client_mock, provider, caplog): web_client_mock.get_playlist.return_value = None results = provider.lookup("spotify:playlist:alice:foo") @@ -232,6 +225,5 @@ def test_lookup_of_invalid_your_uri(provider, caplog): assert len(results) == 0 assert ( - "Failed to lookup 'spotify:your:tracks:invalid': Could not parse" - in caplog.text + "Failed to lookup 'spotify:your:tracks:invalid': Could not parse" in caplog.text ) diff --git a/tests/test_playback.py b/tests/test_playback.py index b8dbb570..575e74e0 100644 --- a/tests/test_playback.py +++ b/tests/test_playback.py @@ -3,28 +3,24 @@ import pytest from mopidy import audio from mopidy import backend as backend_api - from mopidy_spotify import backend -@pytest.fixture +@pytest.fixture() def audio_mock(): - audio_mock = mock.Mock(spec=audio.Audio) - return audio_mock + return mock.Mock(spec=audio.Audio) -@pytest.fixture +@pytest.fixture() def backend_mock(config): backend_mock = mock.Mock(spec=backend.SpotifyBackend) backend_mock._config = config return backend_mock -@pytest.fixture +@pytest.fixture() def provider(audio_mock, backend_mock): - return backend.SpotifyPlaybackProvider( - audio=audio_mock, backend=backend_mock - ) + return backend.SpotifyPlaybackProvider(audio=audio_mock, backend=backend_mock) def test_is_a_playback_provider(provider): diff --git a/tests/test_playlists.py b/tests/test_playlists.py index 8021335c..bc3be7f5 100644 --- a/tests/test_playlists.py +++ b/tests/test_playlists.py @@ -3,11 +3,10 @@ import pytest from mopidy import backend as backend_api from mopidy.models import Ref - from mopidy_spotify import playlists -@pytest.fixture +@pytest.fixture() def web_client_mock(web_client_mock, web_track_mock): web_playlist1 = { "owner": {"id": "alice"}, @@ -40,7 +39,7 @@ def get_playlist(*args, **kwargs): return web_client_mock -@pytest.fixture +@pytest.fixture() def provider(backend_mock, web_client_mock): backend_mock._web_client = web_client_mock provider = playlists.SpotifyPlaylistsProvider(backend_mock) @@ -81,9 +80,7 @@ def test_as_list_when_playlist_wont_translate(provider): assert len(result) == 2 - assert result[0] == Ref.playlist( - uri="spotify:user:alice:playlist:foo", name="Foo" - ) + assert result[0] == Ref.playlist(uri="spotify:user:alice:playlist:foo", name="Foo") assert result[1] == Ref.playlist( uri="spotify:user:bob:playlist:baz", name="Baz (by bob)" ) @@ -223,17 +220,14 @@ def test_lookup_of_playlist_with_other_owner(provider): assert playlist.name == "Baz (by bob)" -def test_playlist_lookup_when_link_invalid( - web_client_mock, web_playlist_mock, caplog -): +def test_playlist_lookup_when_link_invalid(web_client_mock, web_playlist_mock, caplog): web_client_mock.get_playlist.return_value = web_playlist_mock playlist = playlists.playlist_lookup( - web_client_mock, "spotify:in:valid", None + web_client_mock, + "spotify:in:valid", + bitrate=None, ) assert playlist is None - assert ( - "Failed to lookup Spotify playlist URI 'spotify:in:valid'" - in caplog.text - ) + assert "Failed to lookup Spotify playlist URI 'spotify:in:valid'" in caplog.text diff --git a/tests/test_search.py b/tests/test_search.py index 7328fc16..99264dcd 100644 --- a/tests/test_search.py +++ b/tests/test_search.py @@ -1,5 +1,4 @@ from mopidy import models - from mopidy_spotify import search @@ -38,9 +37,7 @@ def test_search_by_single_uri(web_client_mock, web_track_mock, provider): def test_search_by_multiple_uris(web_client_mock, web_track_mock, provider): web_client_mock.get_track.return_value = web_track_mock - result = provider.search( - {"uri": ["spotify:track:abc", "spotify:track:abc"]} - ) + result = provider.search({"uri": ["spotify:track:abc", "spotify:track:abc"]}) assert isinstance(result, models.SearchResult) assert result.uri == "spotify:search" @@ -182,15 +179,13 @@ def test_sets_api_limit_to_track_count_when_max( assert len(result.tracks) == 6 -def test_sets_types_parameter( - web_client_mock, web_search_mock_large, provider, config -): +def test_sets_types_parameter(web_client_mock, web_search_mock_large, provider, config): web_client_mock.get.return_value = web_search_mock_large search.search( config["spotify"], web_client_mock, - {"any": ["ABBA"]}, + query={"any": ["ABBA"]}, types=["album", "artist"], ) @@ -205,9 +200,7 @@ def test_sets_types_parameter( ) -def test_sets_market_parameter( - web_client_mock, web_search_mock_large, provider -): +def test_sets_market_parameter(web_client_mock, web_search_mock_large, provider): web_client_mock.get.return_value = web_search_mock_large provider.search({"any": ["ABBA"]}) diff --git a/tests/test_translator.py b/tests/test_translator.py index 1778d342..b6696fd8 100644 --- a/tests/test_translator.py +++ b/tests/test_translator.py @@ -1,8 +1,8 @@ -import pytest -from mopidy import models from unittest import mock from unittest.mock import patch, sentinel +import pytest +from mopidy import models from mopidy_spotify import translator @@ -46,7 +46,7 @@ def test_bad_artists_filtered(self, web_artist_mock, web_track_mock): assert refs[0].name == "ABBA" -class TestValidWebData(object): +class TestValidWebData: def test_returns_false_if_none(self): assert translator.valid_web_data(None, "track") is False @@ -126,9 +126,7 @@ def test_returns_bare_albums(self, web_album_mock): assert refs[0].name == "ABBA - DEF 456" def test_bad_albums_filtered(self, web_album_mock, web_artist_mock): - refs = list( - translator.web_to_album_refs([{}, web_album_mock, web_artist_mock]) - ) + refs = list(translator.web_to_album_refs([{}, web_album_mock, web_artist_mock])) assert len(refs) == 1 @@ -207,9 +205,7 @@ def test_returns_top_list_tracks(self, web_track_mock): assert refs[0].name == "ABC 123" def test_bad_tracks_filtered(self, web_artist_mock, web_track_mock): - refs = list( - translator.web_to_track_refs([{}, web_track_mock, web_artist_mock]) - ) + refs = list(translator.web_to_track_refs([{}, web_track_mock, web_artist_mock])) assert len(refs) == 1 @@ -349,9 +345,7 @@ def test_returns_playlist_refs(self, web_playlist_mock): assert refs[0].name == "Foo" def test_bad_playlist_filtered(self, web_playlist_mock): - refs = list( - translator.to_playlist_refs([{}, web_playlist_mock, {"foo": 1}]) - ) + refs = list(translator.to_playlist_refs([{}, web_playlist_mock, {"foo": 1}])) assert len(refs) == 1 @@ -381,9 +375,7 @@ def test_artist_maps_to_artist(self): assert query == "artist:ABBA artist:ACDC" def test_artist_maps_to_artist_exact(self): - query = translator.sp_search_query( - {"artist": ["ABBA", "ACDC"]}, exact=True - ) + query = translator.sp_search_query({"artist": ["ABBA", "ACDC"]}, exact=True) assert query == 'artist:"ABBA" artist:"ACDC"' @@ -409,9 +401,7 @@ def test_album_maps_to_album(self): assert query == "album:Greatest album:Hits" def test_album_maps_to_album_exact(self): - query = translator.sp_search_query( - {"album": ["Greatest Hits"]}, exact=True - ) + query = translator.sp_search_query({"album": ["Greatest Hits"]}, exact=True) assert query == 'album:"Greatest Hits"' @@ -446,10 +436,7 @@ def test_date_is_ignored_if_not_parseable(self, caplog): query = translator.sp_search_query({"date": ["abc"]}) assert query == "" - assert ( - 'Excluded year from search query: Cannot parse date "abc"' - in caplog.text - ) + assert 'Excluded year from search query: Cannot parse date "abc"' in caplog.text def test_anything_can_be_combined(self): query = translator.sp_search_query( @@ -559,9 +546,7 @@ def test_ignores_invalid_artists(self, web_album_mock, web_artist_mock): assert album.name == "DEF 456" assert list(album.artists) == artists - def test_returns_empty_artists_list_if_artist_is_empty( - self, web_album_mock - ): + def test_returns_empty_artists_list_if_artist_is_empty(self, web_album_mock): web_album_mock["artists"] = [] album = translator.web_to_album(web_album_mock) @@ -736,7 +721,7 @@ def test_web_to_image_no_dimensions(): @pytest.mark.parametrize( - "height,width", + ("height", "width"), [ (600, 400), (600.0, 400.0), diff --git a/tests/test_web.py b/tests/test_web.py index e589f1d7..6839dc0a 100644 --- a/tests/test_web.py +++ b/tests/test_web.py @@ -1,17 +1,16 @@ import urllib +from datetime import UTC, datetime from unittest import mock -from datetime import datetime +import mopidy_spotify import pytest import requests import responses -from responses import matchers - -import mopidy_spotify from mopidy_spotify import web +from responses import matchers -@pytest.fixture +@pytest.fixture() def oauth_client(config): return web.OAuthClient( base_url="https://api.spotify.com/v1", @@ -23,7 +22,7 @@ def oauth_client(config): ) -@pytest.fixture +@pytest.fixture() def mock_time(): patcher = mock.patch.object(web.time, "time") mock_time = patcher.start() @@ -31,17 +30,17 @@ def mock_time(): patcher.stop() -@pytest.fixture -def mock_utcnow(): +@pytest.fixture() +def mock_now(): patcher = mock.patch("mopidy_spotify.web.datetime") mock_datetime = patcher.start() - mock_datetime.utcnow = mock.Mock() - mock_datetime.side_effect = lambda *args, **kw: datetime(*args, **kw) - yield mock_datetime.utcnow + mock_datetime.now = mock.Mock() + mock_datetime.side_effect = lambda *args, **kw: datetime(*args, **kw, tzinfo=UTC) + yield mock_datetime.now patcher.stop() -@pytest.fixture +@pytest.fixture() def skip_refresh_token(): patcher = mock.patch.object( web.OAuthClient, "_should_refresh_token", return_value=False @@ -73,7 +72,7 @@ def test_user_agent(oauth_client): @pytest.mark.parametrize( - "header,expected", + ("header", "expected"), [ (None, 0), ("", 0), @@ -89,8 +88,8 @@ def test_user_agent(oauth_client): ("foobar", 0), ], ) -def test_parse_retry_after(oauth_client, mock_utcnow, header, expected): - mock_utcnow.return_value = datetime(2022, 12, 7, 11, 27, 26, 0) +def test_parse_retry_after(oauth_client, mock_now, header, expected): + mock_now.return_value = datetime(2022, 12, 7, 11, 27, 26, 0, tzinfo=UTC) mock_response = mock.Mock(headers={"Retry-After": header}) result = oauth_client._parse_retry_after(mock_response) @@ -109,10 +108,7 @@ def test_request_exception(oauth_client, caplog): result = oauth_client.get("tracks/abc") assert result == {} - assert ( - "Fetching https://auth.mopidy.com/spotify/token failed: foo" - in caplog.text - ) + assert "Fetching https://auth.mopidy.com/spotify/token failed: foo" in caplog.text @responses.activate @@ -134,17 +130,10 @@ def test_get_uses_new_access_token( result = oauth_client.get("tracks/abc") assert len(responses.calls) == 2 + assert responses.calls[0].request.url == "https://auth.mopidy.com/spotify/token" + assert responses.calls[1].request.url == "https://api.spotify.com/v1/tracks/abc" assert ( - responses.calls[0].request.url - == "https://auth.mopidy.com/spotify/token" - ) - assert ( - responses.calls[1].request.url - == "https://api.spotify.com/v1/tracks/abc" - ) - assert ( - responses.calls[1].request.headers["Authorization"] - == "Bearer NgCXRK...MzYjw" + responses.calls[1].request.headers["Authorization"] == "Bearer NgCXRK...MzYjw" ) assert oauth_client._headers["Authorization"] == "Bearer NgCXRK...MzYjw" @@ -174,14 +163,8 @@ def test_get_uses_existing_access_token( result = oauth_client.get("tracks/abc") assert len(responses.calls) == 1 - assert ( - responses.calls[0].request.url - == "https://api.spotify.com/v1/tracks/abc" - ) - assert ( - responses.calls[0].request.headers["Authorization"] - == "Bearer 01234...abcde" - ) + assert responses.calls[0].request.url == "https://api.spotify.com/v1/tracks/abc" + assert responses.calls[0].request.headers["Authorization"] == "Bearer 01234...abcde" assert oauth_client._headers["Authorization"] == "Bearer 01234...abcde" assert result["uri"] == "spotify:track:abc" @@ -207,32 +190,22 @@ def test_bad_client_credentials(oauth_client): @responses.activate def test_auth_returns_invalid_json(oauth_client, caplog): - responses.add( - responses.POST, "https://auth.mopidy.com/spotify/token", body="scope" - ) + responses.add(responses.POST, "https://auth.mopidy.com/spotify/token", body="scope") result = oauth_client.get("tracks/abc") assert result == {} - assert ( - "JSON decoding https://auth.mopidy.com/spotify/token failed" - in caplog.text - ) + assert "JSON decoding https://auth.mopidy.com/spotify/token failed" in caplog.text @responses.activate def test_spotify_returns_invalid_json(skip_refresh_token, oauth_client, caplog): - responses.add( - responses.GET, "https://api.spotify.com/v1/tracks/abc", body="abc" - ) + responses.add(responses.GET, "https://api.spotify.com/v1/tracks/abc", body="abc") result = oauth_client.get("tracks/abc") assert result == {} - assert ( - "JSON decoding https://api.spotify.com/v1/tracks/abc failed" - in caplog.text - ) + assert "JSON decoding https://api.spotify.com/v1/tracks/abc failed" in caplog.text @responses.activate @@ -247,9 +220,7 @@ def test_auth_offline(oauth_client, caplog): result = oauth_client.get("tracks/abc") assert result == {} - assert ( - "Fetching https://auth.mopidy.com/spotify/token failed" in caplog.text - ) + assert "Fetching https://auth.mopidy.com/spotify/token failed" in caplog.text @responses.activate @@ -269,9 +240,7 @@ def test_spotify_offline(web_oauth_mock, oauth_client, caplog): result = oauth_client.get("tracks/abc") assert result == {} - assert ( - "Fetching https://api.spotify.com/v1/tracks/abc failed" in caplog.text - ) + assert "Fetching https://api.spotify.com/v1/tracks/abc failed" in caplog.text @responses.activate @@ -315,7 +284,7 @@ def test_auth_wrong_token_type(web_oauth_mock, oauth_client, caplog): @pytest.mark.parametrize( - "header,expected", + ("header", "expected"), [ ("no-store", 100), ("max-age=1", 101), @@ -335,7 +304,7 @@ def test_parse_cache_control(mock_time, header, expected): @pytest.mark.parametrize( - "header,expected", + ("header", "expected"), [ ("", None), ('" "', None), @@ -356,7 +325,7 @@ def test_parse_etag(header, expected): @pytest.mark.parametrize( - "status_code,expected", [(200, True), (301, True), (400, False)] + ("status_code", "expected"), [(200, True), (301, True), (400, False)] ) def test_web_response_status_ok(status_code, expected): response = web.WebResponse("https://foo.com", {}, status_code=status_code) @@ -364,7 +333,7 @@ def test_web_response_status_ok(status_code, expected): @pytest.mark.parametrize( - "status_code,expected", + ("status_code", "expected"), [(200, False), (301, False), (304, True), (400, False)], ) def test_web_response_status_unchanged(status_code, expected): @@ -388,7 +357,7 @@ def test_web_response_status_unchanged_from_cache(): @pytest.mark.parametrize( - "etag,expected", + ("etag", "expected"), [ ('"1234"', {"If-None-Match": '"1234"'}), ("fish", {"If-None-Match": "fish"}), @@ -401,7 +370,7 @@ def test_web_response_etag_headers(etag, expected): @pytest.mark.parametrize( - "etag,status,expected,expected_etag,expected_msg", + ("etag", "status", "expected", "expected_etag", "expected_msg"), [ ("abcd", 200, False, "abcd", "ETag mismatch"), ("abcd", 404, False, "abcd", "ETag mismatch"), @@ -428,7 +397,7 @@ def test_web_response_etag_updated_different(web_response_mock_etag, caplog): @pytest.mark.parametrize( - "cache,ok,expected", + ("cache", "ok", "expected"), [ (None, False, False), (None, True, False), @@ -443,7 +412,7 @@ def test_should_cache_response(oauth_client, cache, ok, expected): @pytest.mark.parametrize( - "path,params,expected", + ("path", "params", "expected"), [ ("tracks/abc?foo=bar&foo=5", None, "tracks/abc?foo=5"), ("tracks/abc?foo=bar&bar=9", None, "tracks/abc?bar=9&foo=bar"), @@ -458,9 +427,7 @@ def test_normalise_query_string(oauth_client, path, params, expected): @responses.activate -def test_web_response( - web_track_mock, mock_time, skip_refresh_token, oauth_client -): +def test_web_response(web_track_mock, mock_time, skip_refresh_token, oauth_client): responses.add( responses.GET, "https://api.spotify.com/v1/tracks/abc", @@ -545,7 +512,7 @@ def test_cache_response_ignore_expiry( mock_time.return_value = 9999 assert not web_response_mock.still_valid() - assert web_response_mock.still_valid(True) + assert web_response_mock.still_valid(ignore_expiry=True) assert "Cached data forced for" in caplog.text result = oauth_client.get( @@ -556,9 +523,7 @@ def test_cache_response_ignore_expiry( @responses.activate -def test_dont_cache_bad_status( - web_track_mock, skip_refresh_token, oauth_client -): +def test_dont_cache_bad_status(web_track_mock, skip_refresh_token, oauth_client): cache = {} responses.add( responses.GET, @@ -589,16 +554,13 @@ def test_cache_key_uses_path(web_track_mock, skip_refresh_token, oauth_client): @responses.activate -def test_cache_normalised_query_string( - mock_time, skip_refresh_token, oauth_client -): +def test_cache_normalised_query_string(mock_time, skip_refresh_token, oauth_client): cache = {} url = "https://api.spotify.com/v1/tracks/abc" responses.add( responses.GET, url, json={"uri": "foobar"}, - # match_querystring=True, match=[matchers.query_string_matcher("b=bar&f=foo")], ) responses.add( @@ -620,7 +582,7 @@ def test_cache_normalised_query_string( assert "tracks/abc?b=bar&f=cat" in cache -@pytest.mark.parametrize("status,unchanged", [(304, True), (200, False)]) +@pytest.mark.parametrize(("status", "unchanged"), [(304, True), (200, False)]) @responses.activate def test_cache_expired_with_etag( web_response_mock_etag, @@ -649,9 +611,7 @@ def test_cache_expired_with_etag( @responses.activate -def test_cache_miss_no_etag( - web_response_mock_etag, skip_refresh_token, oauth_client -): +def test_cache_miss_no_etag(web_response_mock_etag, skip_refresh_token, oauth_client): cache = {"tracks/abc": web_response_mock_etag} responses.add( responses.GET, @@ -726,7 +686,7 @@ def test_updated_responses_changed(web_response_mock, oauth_client, mock_time): assert not result.status_unchanged -@pytest.fixture +@pytest.fixture() def spotify_client(config): client = web.SpotifyOAuthClient( client_id=config["spotify"]["client_id"], @@ -758,7 +718,7 @@ def playlist_tracks_parms(): ) -@pytest.fixture +@pytest.fixture() def bar_playlist(playlist_parms): return { "href": url(f"playlists/bar?{playlist_parms}"), @@ -766,7 +726,7 @@ def bar_playlist(playlist_parms): } -@pytest.fixture +@pytest.fixture() def foo_playlist_tracks(playlist_tracks_parms): return { "href": url(f"playlists/foo/tracks?{playlist_tracks_parms}"), @@ -774,7 +734,7 @@ def foo_playlist_tracks(playlist_tracks_parms): } -@pytest.fixture +@pytest.fixture() def foo_playlist(playlist_parms, foo_playlist_tracks): return { "href": url(f"playlists/foo?{playlist_parms}"), @@ -782,7 +742,7 @@ def foo_playlist(playlist_parms, foo_playlist_tracks): } -@pytest.fixture +@pytest.fixture() def foo_album_next_tracks(): params = urllib.parse.urlencode({"market": "from_token", "offset": 3}) return { @@ -792,7 +752,7 @@ def foo_album_next_tracks(): } -@pytest.fixture +@pytest.fixture() def foo_album(foo_album_next_tracks): params = urllib.parse.urlencode({"market": "from_token"}) return { @@ -805,12 +765,12 @@ def foo_album(foo_album_next_tracks): } -@pytest.fixture +@pytest.fixture() def foo_album_response(foo_album): return web.WebResponse(foo_album["href"], foo_album) -@pytest.fixture +@pytest.fixture() def artist_albums_mock(web_album_mock_base, web_album_mock_base2): params = urllib.parse.urlencode( {"market": "from_token", "include_groups": "single,album"} @@ -841,7 +801,7 @@ def test_track_required_fields(self, field): @pytest.mark.parametrize( "field", - [("name"), ("type"), ("uri"), ("name"), ("snapshot_id"), ("tracks")], + [("name"), ("type"), ("uri"), ("snapshot_id"), ("tracks")], ) def test_playlist_required_fields(self, field): assert field in web.SpotifyOAuthClient.PLAYLIST_FIELDS @@ -872,10 +832,7 @@ def test_configures_proxy(self): def test_configures_urls(self, spotify_client): assert spotify_client._base_url == "https://api.spotify.com/v1" - assert ( - spotify_client._refresh_url - == "https://auth.mopidy.com/spotify/token" - ) + assert spotify_client._refresh_url == "https://auth.mopidy.com/spotify/token" @responses.activate def test_login_alice(self, spotify_client, caplog): @@ -925,7 +882,7 @@ def test_get_one_increased_expiry(self, mock_time, spotify_client): result = spotify_client.get_one("foo") - assert 1000 + spotify_client.DEFAULT_EXTRA_EXPIRY == result._expires + assert result._expires == 1000 + spotify_client.DEFAULT_EXTRA_EXPIRY @responses.activate def test_get_one_retry_header(self, spotify_client, caplog): @@ -941,15 +898,12 @@ def test_get_one_retry_header(self, spotify_client, caplog): assert result == {} assert ( - "Retrying https://api.spotify.com/v1/foo in 66.000 seconds." - in caplog.text + "Retrying https://api.spotify.com/v1/foo in 66.000 seconds." in caplog.text ) @responses.activate def test_get_all(self, spotify_client): - responses.add( - responses.GET, url("page1"), json={"n": 1, "next": "page2"} - ) + responses.add(responses.GET, url("page1"), json={"n": 1, "next": "page2"}) responses.add(responses.GET, url("page2"), json={"n": 2}) results = list(spotify_client.get_all("page1")) @@ -1012,9 +966,7 @@ def test_get_user_playlists(self, spotify_client): assert [f"playlist{i}" for i in range(6)] == results @responses.activate - def test_with_all_tracks_error( - self, spotify_client, foo_album_response, caplog - ): + def test_with_all_tracks_error(self, spotify_client, foo_album_response, caplog): responses.add( responses.GET, foo_album_response["tracks"]["next"], @@ -1073,7 +1025,7 @@ def test_with_all_tracks_uses_cached_tracks_when_unchanged( @responses.activate @pytest.mark.parametrize( - "uri,success", + ("uri", "success"), [ ("spotify:user:alice:playlist:bar", True), ("spotify:user:alice:playlist:fake", False), @@ -1169,7 +1121,7 @@ def test_get_playlist_collates_tracks( assert result["tracks"]["items"] == [1, 2, 3, 4, 5] @pytest.mark.parametrize( - "uri,msg", + ("uri", "msg"), [ ("spotify:artist:foo", "Spotify playlist"), ("my-bad-uri", "Spotify"), @@ -1186,9 +1138,7 @@ def test_clear_cache(self, spotify_client): assert {} == spotify_client._cache - @pytest.mark.parametrize( - "user_id,expected", [("alice", True), (None, False)] - ) + @pytest.mark.parametrize(("user_id", "expected"), [("alice", True), (None, False)]) def test_logged_in(self, spotify_client, user_id, expected): spotify_client.user_id = user_id @@ -1259,9 +1209,7 @@ def test_get_artist_albums( ) link = web.WebLink.from_uri("spotify:artist:abba") - results = list( - spotify_client.get_artist_albums(link, all_tracks=all_tracks) - ) + results = list(spotify_client.get_artist_albums(link, all_tracks=all_tracks)) if all_tracks: assert len(responses.calls) == 3 @@ -1347,7 +1295,7 @@ def test_get_artist_albums_value_error( @responses.activate @pytest.mark.parametrize( - "uri,success", + ("uri", "success"), [ ("spotify:track:abc", True), ("spotify:track:xyz", False), @@ -1411,7 +1359,7 @@ def test_get_artist_top_tracks_invalid_uri( @pytest.mark.parametrize( - "uri,type_,id_", + ("uri", "type_", "id_"), [ ("spotify:playlist:foo", web.LinkType.PLAYLIST, "foo"), ("spotify:track:bar", web.LinkType.TRACK, "bar"), @@ -1430,7 +1378,7 @@ def test_weblink_from_uri_spotify_uri(uri, type_, id_): @pytest.mark.parametrize( - "uri,id_,owner", + ("uri", "id_", "owner"), [ ("spotify:user:alice:playlist:foo", "foo", "alice"), ("spotify:user:alice:starred", None, "alice"), @@ -1462,8 +1410,7 @@ def test_weblink_from_uri_playlist(uri, id_, owner): ], ) def test_weblink_from_uri_raises(uri): - with pytest.raises(ValueError) as excinfo: - result = web.WebLink.from_uri(uri) - assert result is None + with pytest.raises(ValueError, match=r"^Could not parse.*") as excinfo: + web.WebLink.from_uri(uri) assert f"Could not parse {uri!r} as a Spotify URI" in str(excinfo.value) diff --git a/tox.ini b/tox.ini index a5ad7c1a..8fbb9c96 100644 --- a/tox.ini +++ b/tox.ini @@ -1,5 +1,5 @@ [tox] -envlist = py39, py310, py311, check-manifest, flake8 +envlist = py311, py312, ruff-lint, ruff-format [testenv] sitepackages = true @@ -10,10 +10,10 @@ commands = --cov=mopidy_spotify --cov-report=term-missing \ {posargs} -[testenv:check-manifest] +[testenv:ruff-lint] deps = .[lint] -commands = python -m check_manifest +commands = python -m ruff check . -[testenv:flake8] +[testenv:ruff-format] deps = .[lint] -commands = python -m flake8 --show-source --statistics +commands = python -m ruff format .