From b981bfba7e11075ddc5aff9e9ef83556d0d5dbdb Mon Sep 17 00:00:00 2001 From: Andrey Date: Sat, 11 Feb 2017 20:33:41 +0200 Subject: [PATCH] Stop using entity_picture that is known to be bad. (#5856) * Stop using entity_picture that is known to be bad. * Only abandon image on 400 or 404 response * Return is_permanent_failure as a third part of response * Add debug printout * Fix lint * Fix lint --- .../components/media_player/__init__.py | 32 +++++++--- tests/components/media_player/test_demo.py | 62 ++++++++++++++++++- 2 files changed, 83 insertions(+), 11 deletions(-) diff --git a/homeassistant/components/media_player/__init__.py b/homeassistant/components/media_player/__init__.py index e3c3170f6f1..545ccd1c9d0 100644 --- a/homeassistant/components/media_player/__init__.py +++ b/homeassistant/components/media_player/__init__.py @@ -391,6 +391,7 @@ class MediaPlayerDevice(Entity): """ABC for media player devices.""" _access_token = None + _last_bad_image_url = None # pylint: disable=no-self-use # Implement these for your media player @@ -809,8 +810,7 @@ class MediaPlayerDevice(Entity): return None url = self.media_image_url - - if url is None: + if url in (None, self._last_bad_image_url): return None return ENTITY_IMAGE_URL.format( @@ -836,6 +836,14 @@ class MediaPlayerDevice(Entity): _async_fetch_image(self.hass, url), self.hass.loop ).result() + def set_last_bad_image_url(self, url): + """Save last bad image url.""" + should_update = self._last_bad_image_url != url + self._last_bad_image_url = url + if should_update: + _LOGGER.debug('%s marked as inaccessible', url) + self.schedule_update_ha_state() + @asyncio.coroutine def _async_fetch_image(hass, url): @@ -846,11 +854,10 @@ def _async_fetch_image(hass, url): cache_images = ENTITY_IMAGE_CACHE[ATTR_CACHE_IMAGES] cache_urls = ENTITY_IMAGE_CACHE[ATTR_CACHE_URLS] cache_maxsize = ENTITY_IMAGE_CACHE[ATTR_CACHE_MAXSIZE] - if url in cache_images: return cache_images[url] - content, content_type = (None, None) + content, content_type, is_permanent_failure = (None, None, False) websession = async_get_clientsession(hass) response = None try: @@ -859,6 +866,8 @@ def _async_fetch_image(hass, url): if response.status == 200: content = yield from response.read() content_type = response.headers.get(CONTENT_TYPE_HEADER) + elif response.status in (400, 404): + is_permanent_failure = True except asyncio.TimeoutError: pass @@ -868,9 +877,9 @@ def _async_fetch_image(hass, url): yield from response.release() if not content: - return (None, None) + return (None, None, is_permanent_failure) - cache_images[url] = (content, content_type) + cache_images[url] = (content, content_type, is_permanent_failure) cache_urls.append(url) while len(cache_urls) > cache_maxsize: @@ -881,7 +890,7 @@ def _async_fetch_image(hass, url): cache_urls = cache_urls[1:] - return content, content_type + return content, content_type, is_permanent_failure class MediaPlayerImageView(HomeAssistantView): @@ -907,12 +916,15 @@ class MediaPlayerImageView(HomeAssistantView): request.GET.get('token') == player.access_token) if not authenticated: - return web.Response(status=401) + return web.Response(status=401), - data, content_type = yield from _async_fetch_image( - request.app['hass'], player.media_image_url) + url = player.media_image_url + data, content_type, is_permanent_failure = \ + yield from _async_fetch_image(request.app['hass'], url) if data is None: + if is_permanent_failure: + player.set_last_bad_image_url(url) return web.Response(status=500) return web.Response(body=data, content_type=content_type) diff --git a/tests/components/media_player/test_demo.py b/tests/components/media_player/test_demo.py index 1e53245d8a5..ca53e9b66ca 100644 --- a/tests/components/media_player/test_demo.py +++ b/tests/components/media_player/test_demo.py @@ -254,7 +254,7 @@ class TestMediaPlayerWeb(unittest.TestCase): assert setup_component( self.hass, mp.DOMAIN, {'media_player': {'platform': 'demo'}}) - + mp.ENTITY_IMAGE_CACHE[mp.ATTR_CACHE_IMAGES].clear() self.hass.start() def tearDown(self): @@ -295,3 +295,63 @@ class TestMediaPlayerWeb(unittest.TestCase): state.attributes.get('entity_picture')) assert req.status_code == 200 assert req.text == fake_picture_data + + def test_media_image_proxy_500(self): + """Test the media server image proxy server .""" + class MockResponse(): + def __init__(self): + self.status = 500 + + @asyncio.coroutine + def release(self): + pass + + class MockWebsession(): + + @asyncio.coroutine + def get(self, url): + return MockResponse() + + def detach(self): + pass + + self.hass.data[DATA_CLIENTSESSION] = MockWebsession() + + assert self.hass.states.is_state(entity_id, 'playing') + state = self.hass.states.get(entity_id) + req = requests.get(HTTP_BASE_URL + + state.attributes.get('entity_picture')) + assert req.status_code == 500 + self.hass.block_till_done() + assert self.hass.states.get(entity_id).attributes.get( + 'entity_picture') is not None + + def test_media_image_proxy_404(self): + """Test the media server image proxy server .""" + class MockResponse(): + def __init__(self): + self.status = 404 + + @asyncio.coroutine + def release(self): + pass + + class MockWebsession(): + + @asyncio.coroutine + def get(self, url): + return MockResponse() + + def detach(self): + pass + + self.hass.data[DATA_CLIENTSESSION] = MockWebsession() + + assert self.hass.states.is_state(entity_id, 'playing') + state = self.hass.states.get(entity_id) + req = requests.get(HTTP_BASE_URL + + state.attributes.get('entity_picture')) + assert req.status_code == 500 + self.hass.block_till_done() + assert self.hass.states.get(entity_id).attributes.get( + 'entity_picture') is None