Migrate Safe Apps icons to hosted images (#1065)

- `iconUrl` field in the `SafeApp` model is now an `ImageField` (db migration was added).
- Tests are adapted accordingly.

src/safe_apps/migrations/0011_alter_safeapp_icon_url.py

@@ -0,0 +1,25 @@
+# Generated by Django 4.2.2 on 2024-03-07 17:16
+
+from django.db import migrations, models
+
+import safe_apps.models
+
+
+class Migration(migrations.Migration):
+    dependencies = [
+        ("safe_apps", "0010_safe_apps_social_profiles"),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name="safeapp",
+            name="icon_url",
+            field=models.ImageField(
+                blank=True,
+                max_length=255,
+                null=True,
+                upload_to=safe_apps.models.safe_app_icon_path,
+                validators=[safe_apps.models.validate_safe_app_icon_size],
+            ),
+        ),
+    ]

src/safe_apps/models.py

@@ -1,6 +1,10 @@
+import os
 from enum import Enum
+from typing import IO, Union
 
 from django.contrib.postgres.fields import ArrayField
+from django.core.exceptions import ValidationError
+from django.core.files.images import get_image_dimensions
 from django.core.validators import RegexValidator
 from django.db import models
 
@@ -11,6 +15,21 @@
 )
 
 
+def safe_app_icon_path(instance: "SafeApp", filename: str) -> str:
+    _, file_extension = os.path.splitext(filename)
+    return f"safe_apps/{instance.app_id}/icon{file_extension}"
+
+
+def validate_safe_app_icon_size(image: Union[str, IO[bytes]]) -> None:
+    width, height = get_image_dimensions(image)
+    if not width or not height:
+        raise ValidationError(
+            f"Could not get image dimensions. Width={width}, Height={height}"
+        )
+    if width > 512 or height > 512:
+        raise ValidationError("Image width and height need to be at most 512 pixels")
+
+
 class Provider(models.Model):
     url = models.URLField(primary_key=True)
     name = models.CharField(max_length=200)
@@ -43,7 +62,13 @@ class AccessControlPolicy(str, Enum):
     )  # True if this safe-app should be visible from the view. False otherwise
     url = models.URLField()
     name = models.CharField(max_length=200)
-    icon_url = models.URLField()
+    icon_url = models.ImageField(
+        validators=[validate_safe_app_icon_size],
+        upload_to=safe_app_icon_path,
+        max_length=255,
+        null=True,
+        blank=True,
+    )
     description = models.CharField(max_length=200)
     chain_ids = ArrayField(models.PositiveBigIntegerField())
     provider = models.ForeignKey(

src/safe_apps/serializers.py

@@ -51,6 +51,7 @@ class SocialProfileSerializer(serializers.Serializer[SocialProfile]):
 
 class SafeAppsResponseSerializer(serializers.ModelSerializer[SafeApp]):
     id = serializers.IntegerField(source="app_id")
+    icon_url = serializers.ImageField(use_url=True)
     provider = ProviderSerializer()
     access_control = serializers.SerializerMethodField()
     tags = serializers.SerializerMethodField()

src/safe_apps/tests/factories.py

@@ -47,7 +47,7 @@ class Meta:
     visible = True
     url = factory.Faker("url")
     name = factory.Faker("company")
-    icon_url = factory.Faker("image_url")
+    icon_url = factory.django.ImageField(width=50, height=50)
     description = factory.Faker("catch_phrase")
     chain_ids = factory.Faker("pylist", nb_elements=2, value_types=(int,))
     provider = None

src/safe_apps/tests/test_models.py

@@ -1,3 +1,4 @@
+import factory
 from django.core.exceptions import ValidationError
 from django.test import TestCase
 
@@ -11,6 +12,36 @@
 )
 
 
+class IconTestCase(TestCase):
+    def test_icon_upload_path(self) -> None:
+        safe_app = SafeAppFactory.create()
+
+        self.assertEqual(
+            safe_app.icon_url.url, f"/media/safe_apps/{safe_app.app_id}/icon.jpg"
+        )
+
+    def test_icon_max_size_validation(self) -> None:
+        safe_app = SafeAppFactory.create(
+            icon_url=factory.django.ImageField(width=512, height=512)
+        )
+
+        safe_app.full_clean()  # should not rise any exception
+
+    def test_icon_width_greater_than_512(self) -> None:
+        with self.assertRaises(ValidationError):
+            safe_app = SafeAppFactory.create(
+                icon_url=factory.django.ImageField(width=513, height=50)
+            )
+            safe_app.full_clean()
+
+    def test_icon_height_greater_than_512(self) -> None:
+        with self.assertRaises(ValidationError):
+            safe_app = SafeAppFactory.create(
+                icon_url=factory.django.ImageField(width=50, height=513)
+            )
+            safe_app.full_clean()
+
+
 class ProviderTestCase(TestCase):
     def test_str_method_outputs_name_url(self) -> None:
         provider = ProviderFactory.create()

src/safe_apps/tests/test_signals.py

@@ -410,6 +410,9 @@ def test_on_tag_delete_with_safe_app(self) -> None:
             "utf-8"
         )
 
+    # Otherwise fails when testing with all suites
+    # TODO: above tests somehow leak
+    @responses.stop  # type: ignore
     @responses.activate
     def test_on_tag_update_with_multiple_safe_apps(self) -> None:
         chain_id_1 = fake.pyint()
@@ -522,7 +525,8 @@ def test_on_feature_delete_with_safe_app(self) -> None:
             "utf-8"
         )
 
-    # Otherwise fails when testing with all suites - above tests somehow leak
+    # Otherwise fails when testing with all suites
+    # TODO: above tests somehow leak
     @responses.stop  # type: ignore
     @responses.activate
     def test_on_feature_update_with_multiple_safe_apps(self) -> None:

src/safe_apps/tests/test_views.py

@@ -33,7 +33,7 @@ def test_json_payload_format(self) -> None:
                 "id": safe_app.app_id,
                 "url": safe_app.url,
                 "name": safe_app.name,
-                "iconUrl": safe_app.icon_url,
+                "iconUrl": f"http://testserver{safe_app.icon_url.url}",
                 "description": safe_app.description,
                 "chainIds": safe_app.chain_ids,
                 "provider": None,
@@ -61,7 +61,7 @@ def test_tags_payload(self) -> None:
                 "id": safe_app.app_id,
                 "url": safe_app.url,
                 "name": safe_app.name,
-                "iconUrl": safe_app.icon_url,
+                "iconUrl": f"http://testserver{safe_app.icon_url.url}",
                 "description": safe_app.description,
                 "chainIds": safe_app.chain_ids,
                 "provider": None,
@@ -89,7 +89,7 @@ def test_features_payload(self) -> None:
                 "id": safe_app.app_id,
                 "url": safe_app.url,
                 "name": safe_app.name,
-                "iconUrl": safe_app.icon_url,
+                "iconUrl": f"http://testserver{safe_app.icon_url.url}",
                 "description": safe_app.description,
                 "chainIds": safe_app.chain_ids,
                 "provider": None,
@@ -118,7 +118,7 @@ def test_social_profiles_payload(self) -> None:
                 "id": safe_app.app_id,
                 "url": safe_app.url,
                 "name": safe_app.name,
-                "iconUrl": safe_app.icon_url,
+                "iconUrl": f"http://testserver{safe_app.icon_url.url}",
                 "description": safe_app.description,
                 "chainIds": safe_app.chain_ids,
                 "provider": None,
@@ -152,7 +152,7 @@ def test_all_apps_returned(self) -> None:
                 "id": safe_app_1.app_id,
                 "url": safe_app_1.url,
                 "name": safe_app_1.name,
-                "iconUrl": safe_app_1.icon_url,
+                "iconUrl": f"http://testserver{safe_app_1.icon_url.url}",
                 "description": safe_app_1.description,
                 "chainIds": safe_app_1.chain_ids,
                 "provider": None,
@@ -168,7 +168,7 @@ def test_all_apps_returned(self) -> None:
                 "id": safe_app_2.app_id,
                 "url": safe_app_2.url,
                 "name": safe_app_2.name,
-                "iconUrl": safe_app_2.icon_url,
+                "iconUrl": f"http://testserver{safe_app_2.icon_url.url}",
                 "description": safe_app_2.description,
                 "chainIds": safe_app_2.chain_ids,
                 "provider": None,
@@ -184,7 +184,7 @@ def test_all_apps_returned(self) -> None:
                 "id": safe_app_3.app_id,
                 "url": safe_app_3.url,
                 "name": safe_app_3.name,
-                "iconUrl": safe_app_3.icon_url,
+                "iconUrl": f"http://testserver{safe_app_3.icon_url.url}",
                 "description": safe_app_3.description,
                 "chainIds": safe_app_3.chain_ids,
                 "provider": None,
@@ -211,7 +211,7 @@ def test_all_apps_returned_on_empty_chain_id_value(self) -> None:
                 "id": safe_app_1.app_id,
                 "url": safe_app_1.url,
                 "name": safe_app_1.name,
-                "iconUrl": safe_app_1.icon_url,
+                "iconUrl": f"http://testserver{safe_app_1.icon_url.url}",
                 "description": safe_app_1.description,
                 "chainIds": safe_app_1.chain_ids,
                 "provider": None,
@@ -227,7 +227,7 @@ def test_all_apps_returned_on_empty_chain_id_value(self) -> None:
                 "id": safe_app_2.app_id,
                 "url": safe_app_2.url,
                 "name": safe_app_2.name,
-                "iconUrl": safe_app_2.icon_url,
+                "iconUrl": f"http://testserver{safe_app_2.icon_url.url}",
                 "description": safe_app_2.description,
                 "chainIds": safe_app_2.chain_ids,
                 "provider": None,
@@ -243,7 +243,7 @@ def test_all_apps_returned_on_empty_chain_id_value(self) -> None:
                 "id": safe_app_3.app_id,
                 "url": safe_app_3.url,
                 "name": safe_app_3.name,
-                "iconUrl": safe_app_3.icon_url,
+                "iconUrl": f"http://testserver{safe_app_3.icon_url.url}",
                 "description": safe_app_3.description,
                 "chainIds": safe_app_3.chain_ids,
                 "provider": None,
@@ -272,7 +272,7 @@ def test_apps_returned_on_filtered_chain_id(self) -> None:
                 "id": safe_app_4.app_id,
                 "url": safe_app_4.url,
                 "name": safe_app_4.name,
-                "iconUrl": safe_app_4.icon_url,
+                "iconUrl": f"http://testserver{safe_app_4.icon_url.url}",
                 "description": safe_app_4.description,
                 "chainIds": safe_app_4.chain_ids,
                 "provider": None,
@@ -288,7 +288,7 @@ def test_apps_returned_on_filtered_chain_id(self) -> None:
                 "id": safe_app_5.app_id,
                 "url": safe_app_5.url,
                 "name": safe_app_5.name,
-                "iconUrl": safe_app_5.icon_url,
+                "iconUrl": f"http://testserver{safe_app_5.icon_url.url}",
                 "description": safe_app_5.description,
                 "chainIds": safe_app_5.chain_ids,
                 "provider": None,
@@ -326,7 +326,7 @@ def test_apps_returned_on_same_chainid_key_pair(self) -> None:
                 "id": safe_app_1.app_id,
                 "url": safe_app_1.url,
                 "name": safe_app_1.name,
-                "iconUrl": safe_app_1.icon_url,
+                "iconUrl": f"http://testserver{safe_app_1.icon_url.url}",
                 "description": safe_app_1.description,
                 "chainIds": safe_app_1.chain_ids,
                 "provider": None,
@@ -357,7 +357,7 @@ def test_apps_returned_on_non_existent_client_url(self) -> None:
                 "id": safe_app.app_id,
                 "url": safe_app.url,
                 "name": safe_app.name,
-                "iconUrl": safe_app.icon_url,
+                "iconUrl": f"http://testserver{safe_app.icon_url.url}",
                 "description": safe_app.description,
                 "chainIds": safe_app.chain_ids,
                 "provider": None,
@@ -396,7 +396,7 @@ def test_apps_returned_on_empty_client_url(self) -> None:
                 "id": safe_app_1.app_id,
                 "url": safe_app_1.url,
                 "name": safe_app_1.name,
-                "iconUrl": safe_app_1.icon_url,
+                "iconUrl": f"http://testserver{safe_app_1.icon_url.url}",
                 "description": safe_app_1.description,
                 "chainIds": safe_app_1.chain_ids,
                 "provider": None,
@@ -412,7 +412,7 @@ def test_apps_returned_on_empty_client_url(self) -> None:
                 "id": safe_app_2.app_id,
                 "url": safe_app_2.url,
                 "name": safe_app_2.name,
-                "iconUrl": safe_app_2.icon_url,
+                "iconUrl": f"http://testserver{safe_app_2.icon_url.url}",
                 "description": safe_app_2.description,
                 "chainIds": safe_app_2.chain_ids,
                 "provider": None,
@@ -439,7 +439,7 @@ def test_apps_returned_on_same_client_url_key_pair(self) -> None:
                 "id": safe_app_1.app_id,
                 "url": safe_app_1.url,
                 "name": safe_app_1.name,
-                "iconUrl": safe_app_1.icon_url,
+                "iconUrl": f"http://testserver{safe_app_1.icon_url.url}",
                 "description": safe_app_1.description,
                 "chainIds": safe_app_1.chain_ids,
                 "provider": None,
@@ -480,7 +480,7 @@ def test_apps_returned_on_filtered_client_url(self) -> None:
                 "id": safe_app_1.app_id,
                 "url": safe_app_1.url,
                 "name": safe_app_1.name,
-                "iconUrl": safe_app_1.icon_url,
+                "iconUrl": f"http://testserver{safe_app_1.icon_url.url}",
                 "description": safe_app_1.description,
                 "chainIds": safe_app_1.chain_ids,
                 "provider": None,
@@ -497,7 +497,7 @@ def test_apps_returned_on_filtered_client_url(self) -> None:
                 "id": safe_app_3.app_id,
                 "url": safe_app_3.url,
                 "name": safe_app_3.name,
-                "iconUrl": safe_app_3.icon_url,
+                "iconUrl": f"http://testserver{safe_app_3.icon_url.url}",
                 "description": safe_app_3.description,
                 "chainIds": safe_app_3.chain_ids,
                 "provider": None,
@@ -514,7 +514,7 @@ def test_apps_returned_on_filtered_client_url(self) -> None:
                 "id": safe_app_2.app_id,
                 "url": safe_app_2.url,
                 "name": safe_app_2.name,
-                "iconUrl": safe_app_2.icon_url,
+                "iconUrl": f"http://testserver{safe_app_2.icon_url.url}",
                 "description": safe_app_2.description,
                 "chainIds": safe_app_2.chain_ids,
                 "provider": None,
@@ -545,7 +545,7 @@ def test_provider_returned_in_response(self) -> None:
                 "id": safe_app.app_id,
                 "url": safe_app.url,
                 "name": safe_app.name,
-                "iconUrl": safe_app.icon_url,
+                "iconUrl": f"http://testserver{safe_app.icon_url.url}",
                 "description": safe_app.description,
                 "chainIds": safe_app.chain_ids,
                 "provider": {"name": provider.name, "url": provider.url},
@@ -573,7 +573,7 @@ def test_provider_not_returned_in_response(self) -> None:
                 "id": safe_app.app_id,
                 "url": safe_app.url,
                 "name": safe_app.name,
-                "iconUrl": safe_app.icon_url,
+                "iconUrl": f"http://testserver{safe_app.icon_url.url}",
                 "description": safe_app.description,
                 "chainIds": safe_app.chain_ids,
                 "provider": None,
@@ -603,7 +603,7 @@ def test_should_cache_response(self) -> None:
                 "id": safe_app_1.app_id,
                 "url": safe_app_1.url,
                 "name": safe_app_1.name,
-                "iconUrl": safe_app_1.icon_url,
+                "iconUrl": f"http://testserver{safe_app_1.icon_url.url}",
                 "description": safe_app_1.description,
                 "chainIds": safe_app_1.chain_ids,
                 "provider": None,
@@ -636,7 +636,7 @@ def test_visible_safe_app_is_shown(self) -> None:
                 "id": visible_safe_app.app_id,
                 "url": visible_safe_app.url,
                 "name": visible_safe_app.name,
-                "iconUrl": visible_safe_app.icon_url,
+                "iconUrl": f"http://testserver{visible_safe_app.icon_url.url}",
                 "description": visible_safe_app.description,
                 "chainIds": visible_safe_app.chain_ids,
                 "provider": None,