Skip to content

Commit

Permalink
fix typing
Browse files Browse the repository at this point in the history
  • Loading branch information
geo-martino committed Jul 11, 2024
1 parent 987abe1 commit 8803f18
Show file tree
Hide file tree
Showing 10 changed files with 49 additions and 46 deletions.
20 changes: 17 additions & 3 deletions musify/libraries/remote/core/_response.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,22 @@
from abc import ABCMeta, abstractmethod
from typing import Any

from yarl import URL

from musify.base import MusifyObject


class RemoteResponse(MusifyObject, metaclass=ABCMeta):

__slots__ = ()

# noinspection PyPropertyDefinition,PyMethodParameters
@property
@abstractmethod
def kind(cls):
"""The type of remote object this class represents"""
raise NotImplementedError

@property
@abstractmethod
def response(self) -> dict[str, Any]:
Expand All @@ -24,11 +33,16 @@ def id(self) -> str:
"""The ID of this item/collection."""
raise NotImplementedError

# noinspection PyPropertyDefinition,PyMethodParameters
@property
@abstractmethod
def kind(cls):
"""The type of remote object this class represents"""
def url(self) -> URL:
"""The API URL of this item/collection."""
raise NotImplementedError

@property
@abstractmethod
def url_ext(self) -> URL | None:
"""The external URL of this item/collection."""
raise NotImplementedError

@abstractmethod
Expand Down
5 changes: 3 additions & 2 deletions musify/libraries/remote/core/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -104,8 +104,9 @@ async def __aenter__(self) -> Self:
except CacheError:
pass

if isinstance(self.handler.session, CachedSession):
for repository in self.handler.session.cache.values():
session = self.handler.session
if isinstance(session, CachedSession):
for repository in session.cache.values():
# all repositories must use the same payload handler as the request handler
# for it to function correctly
repository.settings.payload_handler = self.handler.payload_handler
Expand Down
14 changes: 0 additions & 14 deletions musify/libraries/remote/core/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,6 @@
from abc import ABCMeta, abstractmethod
from typing import Any, Self

from yarl import URL

from musify.base import MusifyItem
from musify.libraries.remote.core import RemoteResponse
from musify.libraries.remote.core.api import RemoteAPI
Expand Down Expand Up @@ -38,18 +36,6 @@ def has_uri(self) -> bool:
"""Does this item/collection have a valid URI that is not a local URI."""
raise NotImplementedError

@property
@abstractmethod
def url(self) -> URL:
"""The API URL of this item/collection."""
raise NotImplementedError

@property
@abstractmethod
def url_ext(self) -> URL | None:
"""The external URL of this item/collection."""
raise NotImplementedError

@property
def response(self) -> dict[str, Any]:
"""The API response for this object"""
Expand Down
5 changes: 3 additions & 2 deletions musify/libraries/remote/spotify/api/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -105,10 +105,11 @@ async def _response_test(self, response: ClientResponse) -> bool:

# noinspection PyAsyncCall
async def _setup_cache(self) -> None:
if not isinstance(self.handler.session, CachedSession):
session = self.handler.session
if not isinstance(session, CachedSession):
return

cache = self.handler.session.cache
cache = session.cache
cache.repository_getter = self._get_cache_repository

cache.create_repository(SpotifyRepositorySettings(name="tracks"))
Expand Down
14 changes: 8 additions & 6 deletions musify/libraries/remote/spotify/api/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
from collections.abc import Collection, MutableMapping, Iterable
from typing import Any

from aiorequestful.auth.oauth2 import OAuth2Authoriser
from aiorequestful.auth.oauth2 import AuthorisationCodeFlow
from aiorequestful.cache.backend.base import ResponseRepository
from aiorequestful.cache.exception import CacheError
from aiorequestful.cache.session import CachedSession
Expand All @@ -16,7 +16,7 @@
from musify.libraries.remote.core.types import RemoteObjectType


class SpotifyAPIBase(RemoteAPI[OAuth2Authoriser], metaclass=ABCMeta):
class SpotifyAPIBase(RemoteAPI[AuthorisationCodeFlow], metaclass=ABCMeta):
"""Base functionality required for all endpoint functions for the Spotify API"""

__slots__ = ()
Expand Down Expand Up @@ -101,11 +101,12 @@ async def _get_responses_from_cache(
:param id_list: List of IDs to append to the given URL.
:return: (Results from the cache, IDs found in the cache, IDs not found in the cache)
"""
if not isinstance(self.handler.session, CachedSession):
session = self.handler.session
if not isinstance(session, CachedSession):
self.handler.log("CACHE", url, message="Cache not configured, skipping...")
return [], [], id_list

repository = self.handler.session.cache.get_repository_from_url(url=url)
repository = session.cache.get_repository_from_url(url=url)
if repository is None:
self.handler.log("CACHE", url, message="No repository for this endpoint, skipping...")
return [], [], id_list
Expand All @@ -123,7 +124,8 @@ async def _get_responses_from_cache(

async def _cache_responses(self, method: str, responses: Iterable[dict[str, Any]]) -> None:
"""Persist ``results`` of a given ``method`` to the cache."""
if not isinstance(self.handler.session, CachedSession) or not responses:
session = self.handler.session
if not isinstance(session, CachedSession) or not responses:
return

# take all parts of href path, excluding ID
Expand All @@ -137,7 +139,7 @@ async def _cache_responses(self, method: str, responses: Iterable[dict[str, Any]
)
results_mapped = {(method.upper(), result[self.id_key]): result for result in responses}
url = next(iter(possible_urls))
repository: ResponseRepository = self.handler.session.cache.get_repository_from_url(url)
repository: ResponseRepository = session.cache.get_repository_from_url(url)
if repository is not None:
self.handler.log(
method="CACHE",
Expand Down
18 changes: 12 additions & 6 deletions tests/libraries/remote/core/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
from urllib.parse import unquote

import pytest
from aiorequestful.cache.backend import ResponseCache
from aiorequestful.cache.backend import ResponseCache, SQLiteCache
from yarl import URL

from musify.libraries.remote.core.api import RemoteAPI
Expand Down Expand Up @@ -40,6 +40,13 @@ def object_factory(self) -> RemoteObjectFactory:
"""Yield the object factory for objects of this remote service type as a pytest.fixture."""
raise NotImplementedError

@pytest.fixture
async def cache(self) -> ResponseCache:
"""Yields a valid :py:class:`ResponseCache` to use throughout tests in this suite as a pytest.fixture."""
async with SQLiteCache.connect_with_in_memory_db() as cache:
yield cache

# noinspection PyTestUnpassedFixture
@pytest.fixture
async def api_cache(self, api: RemoteAPI, cache: ResponseCache, api_mock: RemoteMock) -> RemoteAPI:
"""Yield an authorised :py:class:`RemoteAPI` object with a :py:class:`ResponseCache` configured."""
Expand Down Expand Up @@ -114,11 +121,10 @@ def assert_params(requests: Iterable[URL], params: dict[str, Any] | list[dict[st
assert k in url.query
assert unquote(url.query[k]) == params[k]

def test_context_management(self, api_cache: RemoteAPI):
session = api_cache.handler.session

assert session.cache.values()
for repository in session.cache.values():
@staticmethod
def test_context_management(api_cache: RemoteAPI, cache: ResponseCache):
assert cache.values()
for repository in cache.values():
assert repository.settings.payload_handler == api_cache.handler.payload_handler

assert api_cache.user_data
Expand Down
3 changes: 3 additions & 0 deletions tests/libraries/remote/spotify/api/test_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ class TestSpotifyAPI(SpotifyAPIFixtures):

items_key = SpotifyAPI.items_key

# noinspection PyTestUnpassedFixture
def test_init(self, cache: ResponseCache):
client_id = "CLIENT_ID"
client_secret = "CLIENT_SECRET"
Expand All @@ -41,6 +42,7 @@ def test_init(self, cache: ResponseCache):
assert api.handler.authoriser.user_request.params["scope"] == " ".join(scopes)
assert api.handler.authoriser.response.file_path == Path(token_file_path)

# noinspection PyTestUnpassedFixture
async def test_context_management(self, cache: ResponseCache, api_mock: SpotifyMock):
api = SpotifyAPI(cache=cache)
api.handler.authoriser.response.replace({
Expand Down Expand Up @@ -76,6 +78,7 @@ async def test_context_management(self, cache: ResponseCache, api_mock: SpotifyM
repository = choice(list(a.handler.session.cache.values()))
await repository.count() # just check this doesn't fail

# noinspection PyTestUnpassedFixture
async def test_cache_repository_getter(self, cache: ResponseCache, api_mock: SpotifyMock):
api = SpotifyAPI(cache=cache)
api.handler.authoriser.response.replace({
Expand Down
8 changes: 1 addition & 7 deletions tests/libraries/remote/spotify/api/testers.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
from typing import Any

import pytest
from aiorequestful.cache.backend import ResponseCache, SQLiteCache
from aiorequestful.cache.backend import ResponseCache
from aiorequestful.cache.backend.base import ResponseRepository

from musify.libraries.remote.core.api import RemoteAPI
Expand All @@ -24,12 +24,6 @@ def object_factory(self) -> SpotifyObjectFactory:
"""Yield the object factory for Spotify objects as a pytest.fixture."""
return SpotifyObjectFactory()

@pytest.fixture
async def cache(self) -> ResponseCache:
"""Yields a valid :py:class:`ResponseCache` to use throughout tests in this suite as a pytest.fixture."""
async with SQLiteCache.connect_with_in_memory_db() as cache:
yield cache

@pytest.fixture
async def repository(
self, object_type: RemoteObjectType, response: dict[str, Any], api_cache: SpotifyAPI, cache: ResponseCache
Expand Down
1 change: 1 addition & 0 deletions tests/libraries/remote/spotify/test_processors.py
Original file line number Diff line number Diff line change
Expand Up @@ -354,6 +354,7 @@ def matcher(self) -> ItemMatcher:
"""Yields a valid :py:class:`ItemMatcher` as a pytest.fixture."""
return ItemMatcher()

# noinspection PyTestUnpassedFixture
@pytest.fixture
def checker(self, matcher: ItemMatcher, api: SpotifyAPI, token_file_path: Path) -> RemoteItemChecker:
api.handler.authoriser.response.file_path = token_file_path
Expand Down
7 changes: 1 addition & 6 deletions tests/processors/test_download.py
Original file line number Diff line number Diff line change
Expand Up @@ -64,13 +64,8 @@ def test_opened_urls(
collections: list[BasicCollection[LocalTrack]],
mocker: MockerFixture,
):
def log_urls(url) -> None:
"""Log the opened URLs"""
nonlocal urls
urls.append(url)

urls = []
mocker.patch(f"{MODULE_ROOT}.processors.download.webopen", new=log_urls)
mocker.patch(f"{MODULE_ROOT}.processors.download.webopen", new=urls.append)

patch_input(values=[""] * sum(map(len, collections)), mocker=mocker)

Expand Down

0 comments on commit 8803f18

Please sign in to comment.