Skip to content

Commit

Permalink
Update Google Photos to have a DataUpdateCoordinator for loading albu…
Browse files Browse the repository at this point in the history
…ms (#126443)

* Update Google Photos to have a data update coordiantor for loading albums

* Remove album from services

* Remove action string changes

* Revert services.yaml change

* Simplify integration by blocking startup on album loading

* Update homeassistant/components/google_photos/coordinator.py

---------

Co-authored-by: Joost Lekkerkerker <joostlek@outlook.com>
  • Loading branch information
allenporter and joostlek authored Sep 24, 2024
1 parent 741b025 commit 27bed0c
Show file tree
Hide file tree
Showing 9 changed files with 101 additions and 51 deletions.
5 changes: 4 additions & 1 deletion homeassistant/components/google_photos/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@

from . import api
from .const import DOMAIN
from .coordinator import GooglePhotosUpdateCoordinator
from .services import async_register_services
from .types import GooglePhotosConfigEntry

Expand Down Expand Up @@ -42,7 +43,9 @@ async def async_setup_entry(
raise ConfigEntryNotReady from err
except ClientError as err:
raise ConfigEntryNotReady from err
entry.runtime_data = GooglePhotosLibraryApi(auth)
coordinator = GooglePhotosUpdateCoordinator(hass, GooglePhotosLibraryApi(auth))
await coordinator.async_config_entry_first_refresh()
entry.runtime_data = coordinator

async_register_services(hass)

Expand Down
61 changes: 61 additions & 0 deletions homeassistant/components/google_photos/coordinator.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
"""Coordinator for fetching data from Google Photos API.
This coordinator fetches the list of Google Photos albums that were created by
Home Assistant, which for large libraries may take some time. The list of album
ids and titles is cached and this provides a method to refresh urls since they
are short lived.
"""

import asyncio
import datetime
import logging
from typing import Final

from google_photos_library_api.api import GooglePhotosLibraryApi
from google_photos_library_api.exceptions import GooglePhotosApiError
from google_photos_library_api.model import Album

from homeassistant.core import HomeAssistant
from homeassistant.helpers.update_coordinator import DataUpdateCoordinator, UpdateFailed

_LOGGER = logging.getLogger(__name__)

UPDATE_INTERVAL: Final = datetime.timedelta(hours=24)
ALBUM_PAGE_SIZE = 50


class GooglePhotosUpdateCoordinator(DataUpdateCoordinator[dict[str, str]]):
"""Coordinator for fetching Google Photos albums.
The `data` object is a dict from Album ID to Album title.
"""

def __init__(self, hass: HomeAssistant, client: GooglePhotosLibraryApi) -> None:
"""Initialize TaskUpdateCoordinator."""
super().__init__(
hass,
_LOGGER,
name="Google Photos",
update_interval=UPDATE_INTERVAL,
)
self.client = client

async def _async_update_data(self) -> dict[str, str]:
"""Fetch albums from API endpoint."""
albums: dict[str, str] = {}
try:
async for album_result in await self.client.list_albums(
page_size=ALBUM_PAGE_SIZE
):
for album in album_result.albums:
albums[album.id] = album.title
except GooglePhotosApiError as err:
_LOGGER.debug("Error listing albums: %s", err)
raise UpdateFailed(f"Error listing albums: {err}") from err
return albums

async def list_albums(self) -> list[Album]:
"""Return Albums with refreshed URLs based on the cached list of album ids."""
return await asyncio.gather(
*(self.client.get_album(album_id) for album_id in self.data)
)
14 changes: 4 additions & 10 deletions homeassistant/components/google_photos/media_source.py
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ async def async_resolve_media(self, item: MediaSourceItem) -> PlayMedia:
f"Could not resolve identiifer that is not a Photo: {identifier}"
)
entry = self._async_config_entry(identifier.config_entry_id)
client = entry.runtime_data
client = entry.runtime_data.client
media_item = await client.get_media_item(media_item_id=identifier.media_id)
if not media_item.mime_type:
raise BrowseError("Could not determine mime type of media item")
Expand Down Expand Up @@ -189,7 +189,8 @@ async def async_browse_media(self, item: MediaSourceItem) -> BrowseMediaSource:
# Determine the configuration entry for this item
identifier = PhotosIdentifier.of(item.identifier)
entry = self._async_config_entry(identifier.config_entry_id)
client = entry.runtime_data
coordinator = entry.runtime_data
client = coordinator.client

source = _build_account(entry, identifier)
if identifier.id_type is None:
Expand All @@ -202,15 +203,8 @@ async def async_browse_media(self, item: MediaSourceItem) -> BrowseMediaSource:
)
for special_album in SpecialAlbum
]
albums: list[Album] = []
try:
async for album_result in await client.list_albums(
page_size=ALBUM_PAGE_SIZE
):
albums.extend(album_result.albums)
except GooglePhotosApiError as err:
raise BrowseError(f"Error listing albums: {err}") from err

albums = await coordinator.list_albums()
source.children.extend(
_build_album(
album.title,
Expand Down
2 changes: 1 addition & 1 deletion homeassistant/components/google_photos/services.py
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ async def async_handle_upload(call: ServiceCall) -> ServiceResponse:
translation_placeholders={"target": DOMAIN},
)

client_api = config_entry.runtime_data
client_api = config_entry.runtime_data.client
upload_tasks = []
file_results = await hass.async_add_executor_job(
_read_file_contents, hass, call.data[CONF_FILENAME]
Expand Down
3 changes: 3 additions & 0 deletions homeassistant/components/google_photos/strings.json
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,9 @@
},
"api_error": {
"message": "Google Photos API responded with error: {message}"
},
"albums_failed": {
"message": "Cannot fetch albums from the Google Photos API"
}
},
"services": {
Expand Down
6 changes: 3 additions & 3 deletions homeassistant/components/google_photos/types.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
"""Google Photos types."""

from google_photos_library_api.api import GooglePhotosLibraryApi

from homeassistant.config_entries import ConfigEntry

type GooglePhotosConfigEntry = ConfigEntry[GooglePhotosLibraryApi]
from .coordinator import GooglePhotosUpdateCoordinator

type GooglePhotosConfigEntry = ConfigEntry[GooglePhotosUpdateCoordinator]
11 changes: 11 additions & 0 deletions tests/components/google_photos/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,17 @@ async def list_albums(*args: Any, **kwargs: Any) -> AsyncGenerator[ListAlbumResu
mock_api.list_albums.return_value.__aiter__ = list_albums
mock_api.list_albums.return_value.__anext__ = list_albums
mock_api.list_albums.side_effect = api_error

# Mock a point lookup by reading contents of the album fixture above
async def get_album(album_id: str, **kwargs: Any) -> Mock:
for album in load_json_object_fixture("list_albums.json", DOMAIN)["albums"]:
if album["id"] == album_id:
return Album.from_dict(album)
return None

mock_api.get_album = get_album
mock_api.get_album.side_effect = api_error

return mock_api


Expand Down
13 changes: 12 additions & 1 deletion tests/components/google_photos/test_init.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import time

from aiohttp import ClientError
from google_photos_library_api.exceptions import GooglePhotosApiError
import pytest

from homeassistant.components.google_photos.const import OAUTH2_TOKEN
Expand All @@ -20,6 +21,7 @@ async def test_setup(
config_entry: MockConfigEntry,
) -> None:
"""Test successful setup and unload."""
await hass.async_block_till_done()
assert config_entry.state is ConfigEntryState.LOADED

await hass.config_entries.async_unload(config_entry.entry_id)
Expand Down Expand Up @@ -68,7 +70,6 @@ async def test_expired_token_refresh_success(
config_entry: MockConfigEntry,
) -> None:
"""Test expired token is refreshed."""

assert config_entry.state is ConfigEntryState.LOADED
assert config_entry.data["token"]["access_token"] == "updated-access-token"
assert config_entry.data["token"]["expires_in"] == 3600
Expand Down Expand Up @@ -107,3 +108,13 @@ async def test_expired_token_refresh_failure(
"""Test failure while refreshing token with a transient error."""

assert config_entry.state is expected_state


@pytest.mark.usefixtures("setup_integration")
@pytest.mark.parametrize("api_error", [GooglePhotosApiError("some error")])
async def test_coordinator_init_failure(
hass: HomeAssistant,
config_entry: MockConfigEntry,
) -> None:
"""Test init failure to load albums."""
assert config_entry.state is ConfigEntryState.SETUP_RETRY
37 changes: 2 additions & 35 deletions tests/components/google_photos/test_media_source.py
Original file line number Diff line number Diff line change
Expand Up @@ -156,24 +156,6 @@ async def test_browse_invalid_path(hass: HomeAssistant) -> None:
)


@pytest.mark.usefixtures("setup_integration")
@pytest.mark.parametrize("api_error", [GooglePhotosApiError("some error")])
async def test_invalid_album_id(hass: HomeAssistant, mock_api: Mock) -> None:
"""Test browsing to an album id that does not exist."""
browse = await async_browse_media(hass, f"{URI_SCHEME}{DOMAIN}")
assert browse.domain == DOMAIN
assert browse.identifier is None
assert browse.title == "Google Photos"
assert [(child.identifier, child.title) for child in browse.children] == [
(CONFIG_ENTRY_ID, "Account Name")
]

with pytest.raises(BrowseError, match="Error listing media items"):
await async_browse_media(
hass, f"{URI_SCHEME}{DOMAIN}/{CONFIG_ENTRY_ID}/a/invalid-album-id"
)


@pytest.mark.usefixtures("setup_integration")
@pytest.mark.parametrize(
("identifier", "expected_error"),
Expand All @@ -193,8 +175,7 @@ async def test_missing_photo_id(


@pytest.mark.usefixtures("setup_integration", "mock_api")
@pytest.mark.parametrize("api_error", [GooglePhotosApiError("some error")])
async def test_list_albums_failure(hass: HomeAssistant) -> None:
async def test_list_media_items_failure(hass: HomeAssistant, mock_api: Mock) -> None:
"""Test browsing to an album id that does not exist."""
browse = await async_browse_media(hass, f"{URI_SCHEME}{DOMAIN}")
assert browse.domain == DOMAIN
Expand All @@ -204,21 +185,7 @@ async def test_list_albums_failure(hass: HomeAssistant) -> None:
(CONFIG_ENTRY_ID, "Account Name")
]

with pytest.raises(BrowseError, match="Error listing albums"):
await async_browse_media(hass, f"{URI_SCHEME}{DOMAIN}/{CONFIG_ENTRY_ID}")


@pytest.mark.usefixtures("setup_integration", "mock_api")
@pytest.mark.parametrize("api_error", [GooglePhotosApiError("some error")])
async def test_list_media_items_failure(hass: HomeAssistant) -> None:
"""Test browsing to an album id that does not exist."""
browse = await async_browse_media(hass, f"{URI_SCHEME}{DOMAIN}")
assert browse.domain == DOMAIN
assert browse.identifier is None
assert browse.title == "Google Photos"
assert [(child.identifier, child.title) for child in browse.children] == [
(CONFIG_ENTRY_ID, "Account Name")
]
mock_api.list_media_items.side_effect = GooglePhotosApiError("some error")

with pytest.raises(BrowseError, match="Error listing media items"):
await async_browse_media(
Expand Down

0 comments on commit 27bed0c

Please sign in to comment.