Fix collection_image unavailable handling (#180973)

homeassistant/components/collection_image/image.py

@@ -5,9 +5,10 @@ from pathlib import Path
 import random
 from typing import override
 
-from homeassistant.components.image import ImageEntity
+from homeassistant.components.image import DEFAULT_CONTENT_TYPE, ImageEntity
 from homeassistant.components.media_player import (
     BrowseError,
+    BrowseMedia,
     MediaClass,
     async_process_play_media_url,
 )
@@ -51,8 +52,6 @@ async def async_setup_entry(
 class CollectionImageImageEntity(ImageEntity):
     """Implement the image entity for Collection Image."""
 
-    _unavailable_logged: bool = False
-
     path: Path | None
 
     def __init__(
@@ -69,80 +68,80 @@ class CollectionImageImageEntity(ImageEntity):
         self._attr_name = name
         self.media_content_id = media_content_id
 
-    async def get_next_image(self) -> None:
-        """Update the image entity with the next image from the source media."""
-
+    def set_unavailable(self) -> None:
+        """Set the entity to unavailable state."""
+        self._attr_available = False
+        self.path = None
+        self._attr_image_url = UNDEFINED
         self._cached_image = None
+        self.async_write_ha_state()
 
-        def set_unavailable() -> None:
-            self._unavailable_logged = True
-            self._attr_available = False
-            self.path = None
-            self._attr_image_url = UNDEFINED
-            self.async_write_ha_state()
-
+    async def get_valid_images(self) -> list[BrowseMedia]:
+        """Given the configured media directory for the entity, get a list of all child images."""
         try:
             media = await async_browse_media(self.hass, self.media_content_id)
         except BrowseError as err:
-            if not self._unavailable_logged:
-                _LOGGER.info("%s: %s", self.entity_id, str(err))
-            set_unavailable()
-            return
+            _LOGGER.warning("%s: %s", self.entity_id, str(err))
+            return []
 
-        if media.children and (
-            filtered := [
-                item for item in media.children if item.media_class == MediaClass.IMAGE
-            ]
-        ):
-            child = random.choice(filtered)
-            try:
-                resolved = await async_resolve_media(
-                    self.hass, child.media_content_id, self.entity_id
-                )
-            except Unresolvable as err:
-                if not self._unavailable_logged:
-                    _LOGGER.info("%s: %s", self.entity_id, str(err))
-                set_unavailable()
-                return
-
-            if resolved.url:
-                self.path = None
-                self._attr_image_url = async_process_play_media_url(
-                    self.hass, resolved.url
-                )
-            else:
-                self.path = resolved.path
-                self._attr_image_url = UNDEFINED
-
-            self._attr_content_type = resolved.mime_type
-            self._attr_available = True
-            self._attr_image_last_updated = dt_util.utcnow()
-            if self._unavailable_logged:
-                _LOGGER.info(
-                    "%s: Has become available again",
-                    self.entity_id,
-                )
-            self._unavailable_logged = False
-            self.async_write_ha_state()
-            return
-
-        if not self._unavailable_logged:
-            _LOGGER.info(
+        images = [
+            item
+            for item in (media.children or [])
+            if item.media_class == MediaClass.IMAGE
+        ]
+        if not images:
+            _LOGGER.warning(
                 "%s: No valid images in %s",
                 self.entity_id,
                 self.media_content_id,
             )
-        set_unavailable()
-        return
+        return images
+
+    async def get_random_image(self) -> None:
+        """Update the image entity with a random image from the source media."""
+
+        filtered = await self.get_valid_images()
+        if not filtered:
+            self.set_unavailable()
+            return
+
+        child = random.choice(filtered)
+        self._attr_available = True
+        await self.update_image(child.media_content_id)
+
+    async def update_image(self, image_id: str):
+        """Update the entity from the image_id."""
+        self._cached_image = None
+        try:
+            resolved = await async_resolve_media(self.hass, image_id, self.entity_id)
+        except Unresolvable as err:
+            _LOGGER.warning("%s: %s", self.entity_id, str(err))
+            self._attr_image_last_updated = None
+            self.path = None
+            self._attr_image_url = UNDEFINED
+            self._attr_content_type = DEFAULT_CONTENT_TYPE
+            self.async_write_ha_state()
+            return
+
+        if resolved.url:
+            self.path = None
+            self._attr_image_url = async_process_play_media_url(self.hass, resolved.url)
+        else:
+            self.path = resolved.path
+            self._attr_image_url = UNDEFINED
+
+        self._attr_content_type = resolved.mime_type
+        self._attr_image_last_updated = dt_util.utcnow()
+        self.async_write_ha_state()
 
     @override
     async def async_added_to_hass(self) -> None:
         """Initialize the first image after entity has been created."""
 
-        async def get_next_image_on_start(_hass: HomeAssistant) -> None:
-            await self.get_next_image()
+        async def get_random_image_on_start(_hass: HomeAssistant) -> None:
+            await self.get_random_image()
 
-        self.async_on_remove(async_at_started(self.hass, get_next_image_on_start))
+        self.async_on_remove(async_at_started(self.hass, get_random_image_on_start))
 
     @override
     def image(self) -> bytes | None:

homeassistant/components/collection_image/services.py

@@ -19,5 +19,5 @@ def async_setup_services(hass: HomeAssistant) -> None:
         SERVICE_SHUFFLE,
         entity_domain=IMAGE_DOMAIN,
         schema={},
-        func="get_next_image",
+        func="get_random_image",
     )

tests/components/collection_image/test_image.py

@@ -9,7 +9,11 @@ import pytest
 
 from homeassistant.components.image import Image, async_get_image
 from homeassistant.components.media_source import BrowseMediaSource, PlayMedia
-from homeassistant.const import EVENT_HOMEASSISTANT_STARTED, STATE_UNAVAILABLE
+from homeassistant.const import (
+    EVENT_HOMEASSISTANT_STARTED,
+    STATE_UNAVAILABLE,
+    STATE_UNKNOWN,
+)
 from homeassistant.core import CoreState, HomeAssistant
 from homeassistant.exceptions import HomeAssistantError
 
@@ -216,7 +220,7 @@ async def test_unresolvable(
 
     state = hass.states.get(DEFAULT_ENTITY_ID)
 
-    assert state and state.state == STATE_UNAVAILABLE
+    assert state and state.state == STATE_UNKNOWN
 
     await hass.async_block_till_done(wait_background_tasks=True)
 

tests/components/collection_image/test_services.py

@@ -27,14 +27,14 @@ async def test_shuffle_action(
     config_entry: MockConfigEntry,
     mock_media_source,
 ) -> None:
-    """Test that shuffle calls get_next_image on the target entity."""
+    """Test that shuffle calls get_random_image on the target entity."""
     await _setup_integration(hass, config_entry)
 
     with patch.object(
         CollectionImageImageEntity,
-        "get_next_image",
+        "get_random_image",
         new_callable=AsyncMock,
-    ) as mock_get_next_image:
+    ) as mock_get_random_image:
         await hass.services.async_call(
             DOMAIN,
             "shuffle",
@@ -42,4 +42,4 @@ async def test_shuffle_action(
             blocking=True,
         )
 
-    mock_get_next_image.assert_awaited_once()
+    mock_get_random_image.assert_awaited_once()