Merge pull request #11255 from nucleogenesis/enhancement--insufficien…

…t-storage-calc

Set insufficient storage DeviceStatus in ContentSyncHook.post_transfer

kolibri/core/content/kolibri_plugin.py

@@ -8,7 +8,10 @@
 from kolibri.core.content.tasks import enqueue_automatic_resource_import_if_needed
 from kolibri.core.content.utils.content_request import incomplete_downloads_queryset
 from kolibri.core.content.utils.content_request import process_metadata_import
+from kolibri.core.content.utils.content_request import StorageCalculator
 from kolibri.core.content.utils.content_request import synchronize_content_requests
+from kolibri.core.device.models import DeviceStatus
+from kolibri.core.device.models import LearnerDeviceStatus
 from kolibri.core.device.utils import get_device_setting
 from kolibri.core.discovery.hooks import NetworkLocationDiscoveryHook
 from kolibri.plugins.hooks import register_hook
@@ -80,8 +83,11 @@ def post_transfer(
         if incomplete_downloads_without_metadata.exists():
             process_metadata_import(incomplete_downloads_without_metadata)
 
-        # TODO: we need determine total space for new downloads and if there isn't sufficient space
-        # save the `LearnerDeviceStatus` with the insufficient storage status
+        calc = StorageCalculator(incomplete_downloads)
+        if local_is_single_user and not calc.is_space_sufficient():
+            LearnerDeviceStatus.save_learner_status(
+                single_user_id, DeviceStatus.InsufficientStorage
+            )
 
         enqueue_automatic_resource_import_if_needed()
 

kolibri/core/content/test/test_content_sync_hook.py

@@ -0,0 +1,44 @@
+import mock
+from morango.sync.context import SessionContext
+
+from kolibri.core.auth.sync_operations import KolibriSyncOperations
+from kolibri.core.content.kolibri_plugin import ContentSyncHook
+from kolibri.core.content.test.utils.test_content_request import (
+    IncompleteDownloadsQuerysetTestCase,
+)
+from kolibri.core.device.models import DeviceStatus
+
+
+class KolibriContentSyncHookTestCase(IncompleteDownloadsQuerysetTestCase):
+    def setUp(self):
+        super(KolibriContentSyncHookTestCase, self).setUp()
+        self.operation = KolibriSyncOperations()
+        self.context = mock.Mock(spec_set=SessionContext)()
+
+    @mock.patch("kolibri.core.device.models.LearnerDeviceStatus.save_learner_status")
+    @mock.patch("kolibri.core.content.utils.content_request.StorageCalculator")
+    def test_post_transfer_sets_insufficient_storage(
+        self,
+        mock_calc,
+        mock_save_learner_status,
+    ):
+        with mock.patch(
+            "kolibri.core.content.utils.settings.automatic_download_enabled",
+            return_value=True,
+        ):
+            with mock.patch(
+                "kolibri.core.content.utils.content_request.get_free_space_for_downloads",
+                return_value=0,
+            ):
+                self._create_resources(self.admin_request.contentnode_id)
+                hook = ContentSyncHook()
+                hook.post_transfer(
+                    self.facility.dataset_id,
+                    True,
+                    True,
+                    self.learner.id,
+                    self.context,
+                )
+                mock_save_learner_status.assert_called_with(
+                    self.learner.id, DeviceStatus.InsufficientStorage
+                )

kolibri/core/content/utils/content_request.py

@@ -486,6 +486,7 @@ def process_content_requests():
         LearnerDeviceStatus.clear_statuses()
     except InsufficientStorage as e:
         logger.warning(str(e))
+
         LearnerDeviceStatus.save_statuses(DeviceStatus.InsufficientStorage)
     except NoPeerAvailable as e:
         logger.warning(str(e))
@@ -649,19 +650,14 @@ def _process_content_requests(incomplete_downloads):
     """
     Processes content requests, both for downloading and removing content
     """
-    incomplete_downloads_with_metadata = incomplete_downloads.filter(has_metadata=True)
 
-    # obtain the incomplete removals, that do not have an associated download
-    incomplete_removals = incomplete_removals_queryset()
-    incomplete_sync_removals = incomplete_removals.filter(
-        reason=ContentRequestReason.SyncInitiated
-    )
-    incomplete_user_removals = incomplete_removals.filter(
-        reason=ContentRequestReason.UserInitiated
-    )
-    complete_user_downloads = ContentDownloadRequest.objects.filter(
-        status=ContentRequestStatus.Completed, reason=ContentRequestReason.UserInitiated
+    calc = StorageCalculator(incomplete_downloads)
+
+    incomplete_downloads_with_metadata = calc.incomplete_downloads.filter(
+        has_metadata=True
     )
+
+    # obtain the incomplete removals, that do not have an associated download
     # track failed so we can exclude them from the loop
     failed_ids = []
     has_processed_sync_removals = False
@@ -688,19 +684,28 @@ def _process_content_requests(incomplete_downloads):
                     free_space
                 )
             )
-            if not has_processed_sync_removals and incomplete_sync_removals.exists():
+            if (
+                not has_processed_sync_removals
+                and calc.incomplete_sync_removals.exists()
+            ):
                 # process, then repeat
                 has_processed_sync_removals = True
                 logger.info("Processing sync-initiated content removal requests")
-                process_content_removal_requests(incomplete_sync_removals)
+                process_content_removal_requests(calc.incomplete_sync_removals)
                 continue
-            if not has_processed_user_removals and incomplete_user_removals.exists():
+            if (
+                not has_processed_user_removals
+                and calc.incomplete_user_removals.exists()
+            ):
                 # process, then repeat
                 has_processed_user_removals = True
                 logger.info("Processing user-initiated content removal requests")
-                process_content_removal_requests(incomplete_user_removals)
+                process_content_removal_requests(calc.incomplete_user_removals)
                 continue
-            if not has_processed_user_downloads and complete_user_downloads.exists():
+            if (
+                not has_processed_user_downloads
+                and calc.complete_user_downloads.exists()
+            ):
                 # process, then repeat
                 has_processed_user_downloads = True
                 process_user_downloads_for_removal()
@@ -939,3 +944,44 @@ def process_content_removal_requests(queryset):
     remaining_pending = queryset.filter(status=ContentRequestStatus.Pending)
     _remove_corresponding_download_requests(remaining_pending)
     remaining_pending.update(status=ContentRequestStatus.Completed)
+
+
+class StorageCalculator:
+    def __init__(self, incomplete_downloads_queryset):
+        incomplete_removals = incomplete_removals_queryset()
+
+        self.incomplete_downloads = incomplete_downloads_queryset
+        self.incomplete_sync_removals = incomplete_removals.filter(
+            reason=ContentRequestReason.SyncInitiated
+        ).annotate(
+            total_size=_total_size_annotation(available=True),
+        )
+
+        self.incomplete_user_removals = incomplete_removals.filter(
+            reason=ContentRequestReason.UserInitiated
+        ).annotate(
+            total_size=_total_size_annotation(available=True),
+        )
+
+        self.complete_user_downloads = ContentDownloadRequest.objects.filter(
+            status=ContentRequestStatus.Completed,
+            reason=ContentRequestReason.UserInitiated,
+        ).annotate(
+            total_size=_total_size_annotation(available=True),
+        )
+        self.free_space = 0
+
+    def _calculate_space_available(self):
+        free_space = get_free_space_for_downloads(
+            completed_size=_total_size(completed_downloads_queryset())
+        )
+        free_space -= _total_size(self.incomplete_downloads)
+        free_space += _total_size(self.incomplete_sync_removals)
+        free_space += _total_size(self.incomplete_user_removals)
+        free_space += _total_size(self.complete_user_downloads)
+
+        self.free_space = free_space
+
+    def is_space_sufficient(self):
+        self._calculate_space_available()
+        return self.free_space > _total_size(self.incomplete_downloads)