Fix #116 and incorrect handling of invalid images file in ImageCache

flatisfy/filters/cache.py

@@ -106,9 +106,14 @@ class ImageCache(MemoryCache):
         if os.path.isfile(filepath):
             image = PIL.Image.open(filepath)
         else:
-            image = PIL.Image.open(BytesIO(requests.get(url).content))
+            req = requests.get(url)
+            try:
+                req.raise_for_status()
+                image = PIL.Image.open(BytesIO(req.content))
                 if self.storage_dir:
                     image.save(filepath, format=image.format)
+            except (requests.HTTPError, IOError):
+                return None
         return image
 
     def __init__(self, storage_dir=None):

flatisfy/filters/duplicates.py

@@ -68,6 +68,8 @@ def get_or_compute_photo_hash(photo, photo_cache):
     except KeyError:
         # Otherwise, get the image and compute the hash
         image = photo_cache.get(photo["url"])
+        if not image:
+            return None
         photo["hash"] = imagehash.average_hash(image)
         return photo["hash"]
 
@@ -88,7 +90,7 @@ def compare_photos(photo1, photo2, photo_cache, hash_threshold):
         hash2 = get_or_compute_photo_hash(photo2, photo_cache)
 
         return hash1 - hash2 < hash_threshold
-    except (IOError, requests.exceptions.RequestException):
+    except (IOError, requests.exceptions.RequestException, TypeError):
         return False
 
 

flatisfy/filters/images.py

@@ -32,6 +32,8 @@ def download_images(flats_list, config):
         )
         for photo in flat["photos"]:
             # Download photo
-            photo_cache.get(photo["url"])
+            image = photo_cache.get(photo["url"])
             # And store the local image
+            # Only add it if fetching was successful
+            if image:
                 photo["local"] = photo_cache.compute_filename(photo["url"])

flatisfy/tests.py

@@ -265,6 +265,38 @@ class TestPhotos(unittest.TestCase):
         ))
 
 
+class TestImageCache(unittest.TestCase):
+    """
+    Checks image cache is working as expected.
+    """
+    def __init__(self, *args, **kwargs):
+        self.IMAGE_CACHE = ImageCache(  # pylint: disable=invalid-name
+            storage_dir=tempfile.mkdtemp(prefix="flatisfy-")
+        )
+        super(TestImageCache, self).__init__(*args, **kwargs)
+
+    def test_invalid_url(self):
+        """
+        Check that it returns nothing on an invalid URL.
+        """
+        # See https://framagit.org/phyks/Flatisfy/issues/116.
+        self.assertIsNone(
+            self.IMAGE_CACHE.get("https://httpbin.org/status/404")
+        )
+        self.assertIsNone(
+            self.IMAGE_CACHE.get("https://httpbin.org/status/500")
+        )
+
+    def test_invalid_data(self):
+        """
+        Check that it returns nothing on an invalid data.
+        """
+        # See https://framagit.org/phyks/Flatisfy/issues/116.
+        self.assertIsNone(
+            self.IMAGE_CACHE.get("https://httpbin.org/")
+        )
+
+
 class TestDuplicates(unittest.TestCase):
     """
     Checks duplicates detection.
@@ -469,19 +501,9 @@ def run():
     """
     LOGGER.info("Running tests…")
     try:
-        suite = unittest.TestLoader().loadTestsFromTestCase(TestTexts)
+        for testsuite in [TestTexts, TestPhoneNumbers, TestImageCache,
-        result = unittest.TextTestRunner(verbosity=2).run(suite)
+                          TestDuplicates, TestPhotos]:
-        assert result.wasSuccessful()
+            suite = unittest.TestLoader().loadTestsFromTestCase(testsuite)
-
-        suite = unittest.TestLoader().loadTestsFromTestCase(TestPhoneNumbers)
-        result = unittest.TextTestRunner(verbosity=2).run(suite)
-        assert result.wasSuccessful()
-
-        suite = unittest.TestLoader().loadTestsFromTestCase(TestDuplicates)
-        result = unittest.TextTestRunner(verbosity=2).run(suite)
-        assert result.wasSuccessful()
-
-        suite = unittest.TestLoader().loadTestsFromTestCase(TestPhotos)
             result = unittest.TextTestRunner(verbosity=2).run(suite)
             assert result.wasSuccessful()
     except AssertionError: