Download remote app icons in dedicated tasks

repomaker/models/remoteapp.py

@@ -11,9 +11,9 @@ from django.utils import timezone
 from django.utils.translation import ugettext_lazy as _
 from fdroidserver import net
 from hvad.models import TranslatedFields
-
 from repomaker import tasks
 from repomaker.utils import clean
+
 from .app import AbstractApp
 from .category import Category
 from .remoterepository import RemoteRepository
@@ -62,8 +62,6 @@ class RemoteApp(AbstractApp):
             self.author_name = app['authorName']
         if 'webSite' in app:
             self.website = app['webSite']
-        if 'icon' in app:
-            self._update_icon(app['icon'])
         if 'categories' in app:
             self._update_categories(app['categories'])
         if 'added' in app:
@@ -78,6 +76,11 @@ class RemoteApp(AbstractApp):
             # no localization available, translate in default language
             self.default_translate()
             self.save()
+        # do the icon last, because we require the app to be saved, so a pk exists
+        if 'icon' in app:
+            # Schedule icon updating task, because it takes too long within this task
+            # pylint: disable=unexpected-keyword-arg
+            tasks.update_remote_app_icon(self.pk, app['icon'], priority=-3)
         return True
 
     @staticmethod
@@ -98,7 +101,13 @@ class RemoteApp(AbstractApp):
             return True
         return False
 
-    def _update_icon(self, icon_name):
+    def update_icon(self, icon_name):
+        """
+        Updates the app's icon from the remote repository.
+        Should be run in a background task.
+
+        :param icon_name: The file name of the icon
+        """
         url = self.repo.url + '/icons-640/' + icon_name
         icon, etag = net.http_get(url, self.icon_etag)
         if icon is None:

repomaker/models/remoterepository.py

@@ -12,7 +12,6 @@ from django.db import models
 from django.dispatch import receiver
 from django.utils import timezone
 from fdroidserver import index, net
-
 from repomaker import tasks
 from repomaker.models.repository import AbstractRepository
 from repomaker.storage import get_remote_repo_path

repomaker/tasks.py

@@ -1,14 +1,13 @@
 import json
 import logging
 
+import repomaker.models
 from background_task import background
 from background_task.signals import task_failed
 from django.core.exceptions import ObjectDoesNotExist
 from django.dispatch import receiver
 from django.utils import timezone
 
-import repomaker.models
-
 
 @background(schedule=timezone.now())
 def update_repo(repo_id):
@@ -54,6 +53,16 @@ def update_remote_repo(remote_repo_id):
         remote_repo.save()
 
 
+@background(schedule=timezone.now())
+def update_remote_app_icon(remote_app_id, icon_name):
+    try:
+        remote_app = repomaker.models.RemoteApp.objects.get(pk=remote_app_id)
+    except ObjectDoesNotExist as e:
+        logging.warning('Remote App does not exist anymore, dropping task. (%s)', e)
+        return
+    remote_app.update_icon(icon_name)
+
+
 @background(schedule=timezone.now())
 def download_apk(apk_id, url):
     try:

repomaker/tests/models/test_remoteapp.py

@@ -90,7 +90,7 @@ class RemoteAppTestCase(TestCase):
 
         # update icon
         http_get.return_value = b'icon-data', 'new_etag'
-        self.app._update_icon('icon.png')  # pylint: disable=protected-access
+        self.app.update_icon('icon.png')  # pylint: disable=protected-access
         http_get.assert_called_once_with(self.repo.url + '/icons-640/icon.png', 'etag')
 
         # assert that old icon got deleted and new one was saved

repomaker/tests/models/test_repository.py

@@ -7,6 +7,7 @@ from unittest.mock import patch
 
 import sass_processor.processor
 import sass_processor.storage
+from background_task.models import Task
 from django.conf import settings
 from django.contrib.auth.models import User
 from django.core.files import File
@@ -315,6 +316,9 @@ class RepositoryTestCase(TestCase):
         This test creates a local repository with one app and one non-apk app
         and then imports it again as a remote repository.
         """
+        # remove pending background tasks
+        Task.objects.all().delete()
+
         # create repo
         repo = self.repo
         fake_repo_create(repo)
@@ -386,10 +390,10 @@ class RepositoryTestCase(TestCase):
         remote_repo.update_index()
         self.assertTrue(len(remote_repo.public_key) > 500)
 
-        # assert repo and app icon were also downloaded
-        self.assertEqual(3, get.call_count)
+        # assert repo icon were also downloaded
+        self.assertEqual(2, get.call_count)
         get.assert_called_with(  # last get call
-            'test_url' + '/icons-640/org.bitbucket.tickytacky.mirrormirror.2.png',
+            'test_url' + '/icons/default-repo-icon.png',
             headers={'User-Agent': 'F-Droid'}
         )
 
@@ -403,9 +407,18 @@ class RepositoryTestCase(TestCase):
         self.assertEqual('', remote_app.description_override)
         self.assertEqual(app.website, remote_app.website)
         self.assertEqual(app.author_name, remote_app.author_name)
-        self.assertTrue(remote_app.icon)
+        self.assertFalse(remote_app.icon)  # downloaded in extra task
         self.assertEqual({settings.LANGUAGE_CODE, 'de', 'en-us'},
                          set(remote_app.get_available_languages()))
+
+        # assert that remote app icon is scheduled to be downloaded in a new task
+        self.assertEqual(1, Task.objects.all().count())
+        task = Task.objects.all()[0]
+        self.assertEqual('repomaker.tasks.update_remote_app_icon', task.task_name)
+        self.assertJSONEqual(
+            '[[' + str(remote_app.pk) + ', "org.bitbucket.tickytacky.mirrormirror.2.png"], {}]',
+            task.task_params)
+
         # non-apk app
         remote_app2 = remote_apps[1]
         self.assertEqual(app2.name, remote_app2.name)

repomaker/tests/test_tasks.py

@@ -5,7 +5,6 @@ import requests
 from background_task.tasks import Task
 from django.contrib.auth.models import User
 from django.test import TestCase
-
 from repomaker import tasks
 from repomaker.models import Repository, RemoteRepository, App, RemoteApp, Apk, RemoteScreenshot
 
@@ -87,6 +86,25 @@ class TasksTest(TestCase):
         # assert that nothing was updated and published
         self.assertFalse(update_index.called)
 
+    @patch('repomaker.models.remoteapp.RemoteApp.update_icon')
+    def test_update_remote_app_icon(self, update_icon):
+        remote_app = RemoteApp.objects.create(
+            repo=RemoteRepository.objects.get(pk=1),
+            last_updated_date=datetime.fromtimestamp(0, timezone.utc)
+        )
+
+        tasks.update_remote_app_icon.now(remote_app.id, "icon name")
+
+        # assert that index of remote repository was updated
+        update_icon.assert_called_once_with("icon name")
+
+    @patch('repomaker.models.remoteapp.RemoteApp.update_icon')
+    def test_update_remote_app_icon_gone(self, update_icon):
+        tasks.update_remote_app_icon.now(1337, "icon name")  # this app ID doesn't exist (anymore?)
+
+        # assert that nothing was updated and published
+        self.assertFalse(update_icon.called)
+
     @patch('repomaker.models.apk.Apk.download')
     def test_download_apk(self, download):
         # create an APK

run.sh

@@ -2,6 +2,7 @@
 
 python3 manage.py runserver &
 PID=$!
+sleep 10
 python3 manage.py process_tasks
 pkill -P $PID
 kill $PID