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

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)

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